Re: [PATCH net-next 0/3] bridge: dsa: switchdev: mv88e6xxx: Implement local_receive bridge flag

Vladimir Oltean
2022-03-18T16:28:39Z

On Fri, Mar 18, 2022 at 05:03:31PM +0100, Tobias Waldekranz wrote:
> On Fri, Mar 18, 2022 at 14:44, Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Fri, Mar 18, 2022 at 01:09:08PM +0100, Tobias Waldekranz wrote:
> >> >> > So have you seriously considered making the bridge ports that operate in
> >> >> > 'dumb hub' mode have a pvid which isn't installed as a 'self' entry on
> >> >> > the bridge device?
> >> >>
> >> >> Just so there's no confusion, you mean something like...
> >> >>
> >> >> ip link add dev br0 type bridge vlan_filtering 1 vlan_default_pvid 0
> >> >>
> >> >> for p in swp0 swp1; do
> >> >> ip link set dev $p master br0
> >> >> bridge vlan add dev $p vid 1 pvid untagged
> >> >> done
> >> >>
> >> >> ... right?
> >> >>
> >> >> In that case, the repeater is no longer transparent with respect to
> >> >> tagged packets, which the application requires.
> >> >
> >> > If you are sure that there exists one VLAN ID which is never used (like
> >> > 4094), what you could do is you could set the port pvids to that VID
> >> > instead of 1, and add the entire VLAN_N_VID range sans that VID in the
> >> > membership list of the two ports, as egress-tagged.
> >>
> >> Yeah, I've thought about this too. If the device's only role is to act
> >> as a repeater, then you can get away with it. But you will have consumed
> >> all rows in the VTU and half of the rows in the ATU (we add an entry for
> >> the broadcast address in every FID). So if you want to use your other
> >> ports for regular bridging you're left with a very limited feature set.
> >
> > But VLANs in other bridges would reuse the same FIDs, at least in the
> > current mv88e6xxx implementation with no FDB isolation, no? So even
> > though the VTU is maxed out, it wouldn't get 'more' maxed out.
>
> I'm pretty sure that mv88e6xxx won't allow the same VID to be configured
> on multiple bridges. A quick test seems to support that:
>
> root@coronet:~# ip link add dev br0 type bridge vlan_filtering 1
> root@coronet:~# ip link add dev br1 type bridge vlan_filtering 1
> root@coronet:~# ip link set dev br0 up
> root@coronet:~# ip link set dev br1 up
> root@coronet:~# ip link set dev swp1 master br0
> root@coronet:~# ip link set dev swp2 master br1
> RTNETLINK answers: Operation not supported

Ok, I forgot about mv88e6xxx_port_check_hw_vlan() even though I was
there on multiple occasions. Thanks for reminding me.

> > As for the broadcast address needing to be present in the ATU, honestly
> > I don't know too much about that. I see that some switches have a
> > FloodBC bit, wouldn't that be useful?
>
> mv88e6xxx can handle broadcast in two ways:
>
> 1. Always flood broadcast, independent of all other settings.
>
> 2. Treat broadcast as multicast, only allow flooding if unknown
> multicast is allowed on the port, or if there's an entry in the ATU
> (making it known) that allows it.
>
> The kernel driver uses (2), because that is the only way (I know of)
> that we can support the BCAST_FLOOD flag. In order to make BCAST_FLOOD
> independent of MCAST_FLOOD, we have to load an entry allowing bc to
> egress on all ports by default. De Morgan comes back to guide us once
> more :)

Ok, so this alternative falls flat on its face due to excessive resource
usage. Next...

Does your application require bridged foreign interfaces with the other
switch ports? In other words, is there a reason to keep the CPU port in
the flood domain of the switch, other than current software limitations?