On Fri, 28 Jun 2019 at 15:30, Ido Schimmel <ido...@idosch.org> wrote: > > On Tue, Jun 25, 2019 at 11:49:29PM +0300, Vladimir Oltean wrote: > > A number of DSA drivers (BCM53XX, Microchip KSZ94XX, Mediatek MT7530 > > at the very least), as well as Mellanox Spectrum (I didn't look at all > > the pure switchdev drivers) try to restore the pvid to a default value > > on .port_vlan_del. > > I don't know about DSA drivers, but that's not what mlxsw is doing. If > the VLAN that is configured as PVID is deleted from the bridge port, the > driver instructs the port to discard untagged and prio-tagged packets. > This is consistent with the bridge driver's behavior. > > We do have a flow the "restores" the PVID, but that's when a port is > unlinked from its bridge master. The PVID we set is 4095 which cannot be > configured by the 8021q / bridge driver. This is due to the way the > underlying hardware works. Even if a port is not bridged and used purely > for routing, packets still do L2 lookup first which sends them directly > to the router block. If PVID is not configured, untagged packets could > not be routed. Obviously, at egress we strip this VLAN. > > > Sure, the port stops receiving traffic when its pvid is a VLAN ID that > > is not installed in its hw filter, but as far as the bridge core is > > concerned, this is to be expected: > > > > # bridge vlan add dev swp2 vid 100 pvid untagged > > # bridge vlan > > port vlan ids > > swp5 1 PVID Egress Untagged > > > > swp2 1 Egress Untagged > > 100 PVID Egress Untagged > > > > swp3 1 PVID Egress Untagged > > > > swp4 1 PVID Egress Untagged > > > > br0 1 PVID Egress Untagged > > # ping 10.0.0.1 > > PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data. > > 64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.682 ms > > 64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.299 ms > > 64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=0.251 ms > > 64 bytes from 10.0.0.1: icmp_seq=4 ttl=64 time=0.324 ms > > 64 bytes from 10.0.0.1: icmp_seq=5 ttl=64 time=0.257 ms > > ^C > > --- 10.0.0.1 ping statistics --- > > 5 packets transmitted, 5 received, 0% packet loss, time 4188ms > > rtt min/avg/max/mdev = 0.251/0.362/0.682/0.163 ms > > # bridge vlan del dev swp2 vid 100 > > # bridge vlan > > port vlan ids > > swp5 1 PVID Egress Untagged > > > > swp2 1 Egress Untagged > > > > swp3 1 PVID Egress Untagged > > > > swp4 1 PVID Egress Untagged > > > > br0 1 PVID Egress Untagged > > > > # ping 10.0.0.1 > > PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data. > > ^C > > --- 10.0.0.1 ping statistics --- > > 8 packets transmitted, 0 received, 100% packet loss, time 7267ms > > > > What is the consensus here? Is there a reason why the bridge driver > > doesn't take care of this? > > Take care of what? :) Always maintaining a PVID on the bridge port? It's > completely OK not to have a PVID. >
Yes, I didn't think it through during the first email. I came to the same conclusion in the second one. > > Do switchdev drivers have to restore the pvid to always be > > operational, even if their state becomes inconsistent with the upper > > dev? Is it just 'nice to have'? What if VID 1 isn't in the hw filter > > either (perfectly legal)? > > Are you saying that DSA drivers always maintain a PVID on the bridge > port and allow untagged traffic to ingress regardless of the bridge > driver's configuration? If so, I think this needs to be fixed. Well, not at the DSA core level. But for Microchip: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/microchip/ksz9477.c#n576 For Broadcom: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/b53/b53_common.c#n1376 For Mediatek: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/mt7530.c#n1196 There might be others as well. Thanks, -Vladimir
