On Mon, Aug 19, 2019 at 08:15:03PM +0300, Vladimir Oltean wrote: > On 6/28/19 7:45 PM, Florian Fainelli wrote: > > On 6/28/19 5:37 AM, Vladimir Oltean wrote: > > > 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. > > > > That sounds bogus indeed, and I bet that the two other drivers just > > followed the b53 driver there. I will have to test this again and come > > up with a patch eventually. > > > > When the port leaves the bridge we do bring it back into a default PVID > > (which is different than the bridge's default PVID) so that part should > > be okay. > > > > Adding a few more networking people. > So my flow is something like this: > - Boot a board with a DSA switch > - Bring all interfaces up > - Enslave all interfaces to br0 > - Enable vlan_filtering on br0 > > What VIDs should be installed into the ports' hw filters, and what should > the pvid be at this point? > Should the switch ports pass any traffic? > At this point, 'bridge vlan' shows a confusing: > port vlan ids > eth0 1 PVID Egress Untagged > > swp5 1 PVID Egress Untagged > > swp2 1 PVID Egress Untagged > > swp3 1 PVID Egress Untagged > > swp4 1 PVID Egress Untagged > > br0 1 PVID Egress Untagged > for all ports, but the .port_vlan_add callback is nowhere to be found.
The bridge adds a PVID on the port when it is enslaved to the bridge. The configuration only takes effect when VLAN filtering is enabled. I'm looking at dsa_port_vlan_add() and it seems that it does not propagate the VLAN call when VLAN filtering is disabled. This explains why you never see the callback. I assume that if you configure the bridge with VLAN filtering enabled and then enslave a port, then everything works fine. mlxsw avoids the situation by forbidding the toggling of VLAN filtering on the bridge when its ports are enslaved. > > Whose responsibility is it for the switch to pass traffic without any > further 'bridge vlan' command? What is the mechanism through which this > should work? > > What if I do: > sudo bridge vlan add vid 100 dev swp2 pvid untagged > echo 0 | sudo tee /sys/class/net/br0/bridge/vlan_filtering > echo 1 | sudo tee /sys/class/net/br0/bridge/vlan_filtering > What pvid should there be on swp2 now? > 'bridge vlan' shows: > port vlan ids > eth0 1 PVID Egress Untagged > > 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 > If the 'bridge vlan' output is correct, whose responsibility is it to > restore this pvid? I suggest to follow mlxsw and avoid this mess. You can support both VLAN filtering enable / disable without supporting dynamically toggling the option. > > More context: the sja1105 driver is somewhat similar to the mlxsw in that > VLAN awareness cannot be truly disabled. Arid details aside, in both cases, > achieving "VLAN-unaware"-like behavior involves manipulating the pvid in > both cases. But it appears that the bridge core does expect: > (1) that the driver performs a default VLAN initialization which matches its > own, without them ever communicating. But because switchdev/DSA drivers > start off in standalone mode, vlan_filtering=0 comes first, hence the > non-standard pvid. Through what mechanism is the bridge-expected pvid > supposed to get restored upon flipping vlan_filtering? > (2) that toggling VLAN filtering off and on has no other state upon the > underlying driver than enabling and disabling VLAN awareness. The VLAN hw > filter table is assumed to be unchanged. Is this a correct assumption? > > Thanks, > -Vladimir
