Re: [PATCH net-next 1/2] net: dsa: rtl8366rb: support bridge offloading

[Vladimir Oltean <olteanv@gmail.com>]

On Mon, Aug 30, 2021 at 11:22:11PM +0200, Linus Walleij wrote:
> On Mon, Aug 30, 2021 at 10:12 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> 
> > > +/* Port isolation registers */
> > > +#define RTL8366RB_PORT_ISO_BASE              0x0F08
> > > +#define RTL8366RB_PORT_ISO(pnum)     (RTL8366RB_PORT_ISO_BASE + (pnum))
> > > +#define RTL8366RB_PORT_ISO_EN                BIT(0)
> > > +#define RTL8366RB_PORT_ISO_PORTS_MASK        GENMASK(7, 1)
> >
> > If RTL8366RB_NUM_PORTS is 6, then why is RTL8366RB_PORT_ISO_PORTS_MASK a
> > 7-bit field?
> 
> It's a 6 bit field actually from bit 1 to bit 7 just shifted up one
> bit because bit 0 is "enable".

Understood the part about bit 0 being "ENABLE".
But from bit 1 to bit 7, I count 7 bits set....

> > Also, it would be nice if you could do some minimal isolation at the
> > level of the FDB lookup. Currently, if I am not mistaken, a port will
> > perform FDB lookup even if it is standalone, and it might find an FDB
> > entry for a given {MAC DA, VLAN ID} pair that belongs to a port outside
> > of its isolation mask, so forwarding will be blocked and that packet
> > will be dropped (instead of the expected behavior which is for that
> > packet to be forwarded to the CPU).
> >
> > Normally the expectation is that this FDB-level isolation can be achieved
> > by configuring the VLANs of one bridge to use a filter ID that is
> > different from the VLANs of another bridge, and the port-based default
> > VLAN of standalone ports to use yet another filter ID. This is yet
> > another reason to disable learning on standalone ports, so that their
> > filter ID never contains any FDB entry, and packets are always flooded
> > to their only possible destination, the CPU port.
> >
> > Currently in DSA we do not offer a streamlined way for you to determine
> > what filter ID to use for a certain VLAN belonging to a certain bridge,
> > but at the very least you can test FDB isolation between standalone
> > ports and bridged ports. The simplest way to do that, assuming you
> > already have a forwarding setup with 2 switch ports swp0 and swp1, is to
> > enable CONFIG_BONDING=y, and then:
> >
> > ip link add br0 type bridge
> > ip link set bond0 master br0
> > ip link set swp1 master bond0
> > ip link set swp0 master br0
> >
> > Then ping between station A attached to swp0 and station B attached to
> > swp1.
> >
> > Because swp1 cannot offload bond0, it will fall back to software
> > forwarding and act as standalone, i.e. what you had up till now.
> > With hardware address learning enabled on swp0 (a port that offloads
> > br0), it will learn station A's source MAC address. Then when swp1 needs
> > to send a packet to station A's destination MAC address, it would be
> > tempted to look up the FDB, find that address, and forward to swp0. But
> > swp0 is isolated from swp1. If you use a filter ID for standalone ports
> > and another filter ID for bridged ports you will avoid that problem, and
> > you will also lay the groundwork for the full FDB isolation even between
> > bridges that will be coming during the next development cycle.
> >
> > If you feel that the second part is too much for now, you can just add
> > the extra callbacks for address learning and flushing (although I do
> > have some genuine concerns about how reliable was the software forwarding
> > with this driver, seeing that right now it enables hardware learning
> > unconditionally). Is there something that isolates FDB lookups already?
> 
> Ugh that was massive, I'm not that smart ;)
> 
> I kinda understand it but have no idea how to achieve this with
> the current hardware, driver and vendor code mess.
> 
> I prefer to fix the first part for now.

Okay, no problem, I suppose FDB isolation can be revisited as part of
the larger rework I've got planned for the next kernel.