Linux Ethernet Bridging - Jan 2021 - [Bridge] [PATCH net] net: mrp: use stp state as substitute for unimplemented mrp state

On Mon, Jan 18, 2021 at 09:20:36PM +0100, Horatiu Vultur
wrote:
> The 01/18/2021 19:46, Vladimir Oltean wrote:
> >
> > On Mon, Jan 18, 2021 at 07:56:18PM +0100, Horatiu Vultur wrote:
> > > The reason was to stay away from STP, because you can't run
these two
> > > protocols at the same time. Even though in SW, we reuse
port's state.
> > > In our driver(which is not upstreamed), we currently implement
> > > SWITCHDEV_ATTR_ID_MRP_PORT_STATE and just call the
> > > SWITCHDEV_ATTR_ID_PORT_STP_STATE.
> >
> > And isn't Rasmus's approach reasonable, in that it allows
unmodified
> > switchdev drivers to offload MRP port states without creating
> > unnecessary code churn?
>
> I am sorry but I don't see this as the correct solution. In my opinion,
> I would prefer to have 3 extra lines in the driver and have a better
> view of what is happening. Than having 2 calls in the driver for
> different protocols.
I think the question boils down to: is a MRP-unaware driver expected to
work with the current bridge MRP code?
> If it is not a problem to have STP calls when you configure the MRP,
> then why not just remove SWITCHDEV_ATTR_ID_MRP_PORT_STATE?
Good question, why not?

The 01/18/2021 21:27, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe
> 
> On Mon, Jan 18, 2021 at 09:20:36PM +0100, Horatiu Vultur wrote:
> > The 01/18/2021 19:46, Vladimir Oltean wrote:
> > >
> > > On Mon, Jan 18, 2021 at 07:56:18PM +0100, Horatiu Vultur wrote:
> > > > The reason was to stay away from STP, because you can't
run these two
> > > > protocols at the same time. Even though in SW, we reuse
port's state.
> > > > In our driver(which is not upstreamed), we currently
implement
> > > > SWITCHDEV_ATTR_ID_MRP_PORT_STATE and just call the
> > > > SWITCHDEV_ATTR_ID_PORT_STP_STATE.
> > >
> > > And isn't Rasmus's approach reasonable, in that it allows
unmodified
> > > switchdev drivers to offload MRP port states without creating
> > > unnecessary code churn?
> >
> > I am sorry but I don't see this as the correct solution. In my
opinion,
> > I would prefer to have 3 extra lines in the driver and have a better
> > view of what is happening. Than having 2 calls in the driver for
> > different protocols.
> 
> I think the question boils down to: is a MRP-unaware driver expected to
> work with the current bridge MRP code?
If the driver has switchdev support, then is not expected to work with
the current bridge MRP code.

For example, the Ocelot driver, it has switchdev support but no MRP
support so this is not expected to work.  The main reason is that MRP is
using as DMAC 01:15:4E:00:00:0x(where x is between 1-4) so then when
these frames will arrive to HW then they will just be flooded which is
the wrong behavior.

But, the Ocelot which is not MRP aware, it can behave as MRP node if the
callbacks are implemented. For example, in MRP you have a notion of MRC
(Client) which needs to forward MRP Test frames between 2 ports and copy
to CPU MRP TopologyChange frames and forward these frames between 2
ports. Then using some TCAM rules(match on DMAC and source port) you can
implement this because you can differentiate between Test and Topology
frames by using the last byte of the DMAC.
> 
> > If it is not a problem to have STP calls when you configure the MRP,
> > then why not just remove SWITCHDEV_ATTR_ID_MRP_PORT_STATE?
> 
> Good question, why not?
-- 
/Horatiu