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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210830220155.5nbtm6khoivend6f@skbuf>
Date:   Tue, 31 Aug 2021 01:01:55 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        DENG Qingfang <dqfext@...il.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Mauri Sandberg <sandberg@...lfence.com>
Subject: Re: [PATCH net-next 1/2] net: dsa: rtl8366rb: support bridge
 offloading

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@...il.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.