mbox series

[net-next,0/3] Plug holes in DSA's software bridging support

Message ID 20210823180242.2842161-1-vladimir.oltean@nxp.com
Headers show
Series Plug holes in DSA's software bridging support | expand

Message

Vladimir Oltean Aug. 23, 2021, 6:02 p.m. UTC 
This series addresses some oddities reported by Alvin while he was
working on the new rtl8365mb driver (a driver which does not implement
bridge offloading for now, and relies on software bridging).

First is that DSA behaves, in the lack of a .port_bridge_join method, as
if the operation succeeds, and does not kick off its internal procedures
for software bridging (the same procedures that were written for indirect
software bridging, meaning bridging with an unoffloaded software LAG).

Second is that even after being patched to treat ports with software
bridging as standalone, we still don't get rid of bridge VLANs, even
though we have code to ignore them, that code manages to get bypassed.
This is in fact a recurring issue which was brought up by Tobias
Waldekranz a while ago, but the solution never made it to the git tree.

After debugging with Florian the last time:
https://patchwork.kernel.org/project/netdevbpf/patch/20210320225928.2481575-3-olteanv@gmail.com/
I became very concerned about sending these patches to stable kernels.
They are relatively large reworks, and they are only tested properly on
net-next.

A few commands on my test vehicle which has ds->vlan_filtering_is_global
set to true:

| Nothing is committed to hardware when we add VLAN 100 on a standalone
| port
$ ip link add link sw0p2 name sw0p2.100 type vlan id 100
# When a neighbor port joins a VLAN-aware bridge, VLAN filtering gets
# enabled globally on the switch. This replays the VLAN 100 from
# sw0p2.100 and also installs VLAN 1 from the bridge on sw0p0.
$ ip link add br0 type bridge vlan_filtering 1 && ip link set sw0p0 master br0
[   97.948087] sja1105 spi2.0: Reset switch and programmed static config. Reason: VLAN filtering
[   97.957989] sja1105 spi2.0: sja1105_bridge_vlan_add: port 2 vlan 100
[   97.964442] sja1105 spi2.0: sja1105_bridge_vlan_add: port 4 vlan 100
[   97.971202] device sw0p0 entered promiscuous mode
[   97.976129] sja1105 spi2.0: sja1105_bridge_vlan_add: port 0 vlan 1
[   97.982640] sja1105 spi2.0: sja1105_bridge_vlan_add: port 4 vlan 1
| We can see that sw0p2, the standalone port, is now filtering because
| of the bridge
$ ethtool -k sw0p2 | grep vlan
rx-vlan-filter: on [fixed]
| When we make the bridge VLAN-unaware, the 8021q upper sw0p2.100 is
| uncomitted from hardware. The VLANs managed by the bridge still remain
| committed to hardware, because they are managed by the bridge.
$ ip link set br0 type bridge vlan_filtering 0
[  134.218869] sja1105 spi2.0: Reset switch and programmed static config. Reason: VLAN filtering
[  134.228913] sja1105 spi2.0: sja1105_bridge_vlan_del: port 2 vlan 100
# And now the standalone port is not filtering anymore.
ethtool -k sw0p2 | grep vlan
rx-vlan-filter: off [fixed]

The same test with .port_bridge_join and .port_bridge_leave commented
out from this driver:

| Not a flinch
$ ip link add link sw0p2 name sw0p2.100 type vlan id 100
$ ip link add br0 type bridge vlan_filtering 1 && ip link set sw0p0 master br0
Warning: dsa_core: Offloading not supported.
$ ethtool -k sw0p2 | grep vlan
rx-vlan-filter: off [fixed]
$ ip link set br0 type bridge vlan_filtering 0
$ ethtool -k sw0p2 | grep vlan
rx-vlan-filter: off [fixed]

Vladimir Oltean (3):
  net: dsa: properly fall back to software bridging
  net: dsa: don't advertise 'rx-vlan-filter' when not needed
  net: dsa: let drivers state that they need VLAN filtering while
    standalone

 drivers/net/dsa/hirschmann/hellcreek.c |  1 +
 include/net/dsa.h                      |  3 +
 net/dsa/dsa_priv.h                     |  2 +
 net/dsa/port.c                         | 42 +++++++++++++-
 net/dsa/slave.c                        | 79 +++++++++++++++++++++++++-
 net/dsa/switch.c                       | 27 ++++++---
 6 files changed, 143 insertions(+), 11 deletions(-)

Comments

Vladimir Oltean Aug. 23, 2021, 7:45 p.m. UTC | #1 
On Mon, Aug 23, 2021 at 09:02:39PM +0300, Vladimir Oltean wrote:
> This series addresses some oddities reported by Alvin while he was
> working on the new rtl8365mb driver (a driver which does not implement
> bridge offloading for now, and relies on software bridging).

I will resubmit for 3 reasons:

- I left an unused variable:
  https://patchwork.hopto.org/static/nipa/536059/12453371/build_32bit/stderr

- After wrapping up the testing with unoffloaded bridge ports I got a
  kernel panic when rebooting the board. This is because in
  dsa_port_pre_bridge_leave we call switchdev_bridge_port_unoffload on a
  NULL brport_dev. We must only call switchdev_bridge_port_unoffload if
  dp->bridge_dev is not NULL.

- I need to correct this phrase in the commit message of patch 2:
  | In turn, having dp->bridge_dev = NULL makes the following things go wrong:
                                  ~~~
                                  != NULL