bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
Subsystems (4)
cover bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
When bridge binding is enabled on a VLAN netdevice, its link state should track bridge ports that are members of the corresponding VLAN. This works for a newly-added netdevices. However toggling the option does not have the effect of enabling or disabling the behavior as appropriate.
In this patchset, have bridge react to bridge_binding toggles on VLAN uppers.
There has been another attempt at supporting this behavior in 2022 by Sevinj Aghayeva [0]. A discussion ensued that informed how this new patchset is constructed, namely that the logic is in the bridge as opposed to the 8021q driver, and the bridge reacts to NETDEV_CHANGE events on the 8021q upper.
Patches #1 and #2 contain the implementation, patches #3 and #4 a selftest.
[0] https://lore.kernel.org/netdev/[email protected]/
Petr Machata (4):
net: bridge: Extract a helper to handle bridge_binding toggles
net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
selftests: net: lib: Add a couple autodefer helpers
selftests: net: Add a VLAN bridge binding selftest
net/bridge/br.c | 7 + net/bridge/br_private.h | 9 + net/bridge/br_vlan.c | 44 ++- tools/testing/selftests/net/Makefile | 1 + tools/testing/selftests/net/lib.sh | 31 ++- .../selftests/net/vlan_bridge_binding.sh | 256 ++++++++++++++++++ 6 files changed, 340 insertions(+), 8 deletions(-) create mode 100755 tools/testing/selftests/net/vlan_bridge_binding.sh
Signature · 1 line
2.47.0
PATCH 1/4 net: bridge: Extract a helper to handle bridge_binding toggles
Currently, the BROPT_VLAN_BRIDGE_BINDING bridge option is only toggled when VLAN devices are added on top of a bridge or removed from it. Extract the toggling of the option to a function so that it could be invoked by a subsequent patch when the state of an upper VLAN device changes.
--- net/bridge/br_vlan.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
net/bridge/br_vlan.c +14 -6
@@ -1664,6 +1664,18 @@ static void br_vlan_set_all_vlan_dev_state(struct net_bridge_port *p) } } +static void br_vlan_toggle_bridge_binding (struct net_device *br_dev, +bool enable) +{ +struct net_bridge *br =netdev_priv (br_dev); + +if (enable) +br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , true); +else +br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , +br_vlan_has_upper_bind_vlan_dev (br_dev)); +} +static void br_vlan_upper_change (struct net_device *dev,struct net_device *upper_dev,bool linking)
@@ -1673,13 +1685,9 @@ static void br_vlan_upper_change(struct net_device *dev,if (!br_vlan_is_bind_vlan_dev (upper_dev))return ; -if (linking) { +br_vlan_toggle_bridge_binding (dev, linking); +if (linking)br_vlan_set_vlan_dev_state (br, upper_dev); -br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , true); - }else { -br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , -br_vlan_has_upper_bind_vlan_dev (dev)); - } }struct br_vlan_link_state_walk_data {
Signature · 1 line
2.47.0
Re: [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles
Quoted 40 lines · Petr Machata
Currently, the
BROPT_VLAN_BRIDGE_BINDINGbridge option is only toggled when VLAN devices are added on top of a bridge or removed from it. Extract the toggling of the option to a function so that it could be invoked by a subsequent patch when the state of an upper VLAN device changes.Signed-off-by Petr Machata <[email protected]>Reviewed-by Ido Schimmel <[email protected]>--- net/bridge/br_vlan.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)net/bridge/br_vlan.c +13
-1@@ -1664,6 +1664,18 @@ static void br_vlan_set_all_vlan_dev_state(struct net_bridge_port *p) } } +static void br_vlan_toggle_bridge_binding (struct net_device *br_dev, +bool enable) +{ +struct net_bridge *br =netdev_priv (br_dev); + +if (enable) +br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , true); +else +br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , +br_vlan_has_upper_bind_vlan_dev (br_dev)); +} +static void br_vlan_upper_change (struct net_device *dev,struct net_device *upper_dev,bool linking)@@ -1673,13 +1685,9 @@ static void br_vlan_upper_change(struct net_device *dev,if (!br_vlan_is_bind_vlan_dev (upper_dev))return ; -if (linking) { +br_vlan_toggle_bridge_binding (dev, linking);
net/bridge/br_vlan.c static void br_vlan_upper_change(struct net_device *dev, · line 1693
Quoted 10 lines
net/bridge/br_vlan.c +1
-5+if (linking)br_vlan_set_vlan_dev_state (br, upper_dev); -br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , true); - }else { -br_opt_toggle (br,BROPT_VLAN_BRIDGE_BINDING , -br_vlan_has_upper_bind_vlan_dev (dev)); - } }struct br_vlan_link_state_walk_data {
PATCH 2/4 net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
In reply to Petr MachataWhen bridge binding is enabled on a VLAN netdevice, its link state should track bridge ports that are members of the corresponding VLAN. This works for newly-added netdevices. However toggling the option does not have the effect of enabling or disabling the behavior as appropriate.
In this patch, react to bridge_binding toggles on VLAN uppers.
--- net/bridge/br.c | 7 +++++++ net/bridge/br_private.h | 9 +++++++++ net/bridge/br_vlan.c | 24 ++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
net/bridge/br.c +7 -0
@@ -51,6 +51,13 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v } } +if (is_vlan_dev (dev)) { +struct net_device *real_dev =vlan_dev_real_dev (dev); + +if (netif_is_bridge_master (real_dev)) +br_vlan_vlan_upper_event (real_dev, dev, event); + } +/* not a port of a bridge */ p =br_port_get_rtnl (dev);if (!p)
net/bridge/br_private.h +9 -0
@@ -1571,6 +1571,9 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,void br_vlan_port_event (struct net_bridge_port *p,unsigned long event);int br_vlan_bridge_event (struct net_device *dev,unsigned long event,void *ptr); +void br_vlan_vlan_upper_event (struct net_device *br_dev, +struct net_device *vlan_dev, +unsigned long event);int br_vlan_rtnl_init (void );void br_vlan_rtnl_uninit (void ); void br_vlan_notify(const struct net_bridge *br,
@@ -1802,6 +1805,12 @@ static inline int br_vlan_bridge_event(struct net_device *dev,return 0 ; } +static inline void br_vlan_vlan_upper_event (struct net_device *br_dev, +struct net_device *vlan_dev, +unsigned long event) +{ +} +static inline int br_vlan_rtnl_init (void ) {return 0 ;
net/bridge/br_vlan.c +24 -0
@@ -1772,6 +1772,30 @@ int br_vlan_bridge_event(struct net_device *dev, unsigned long event, void *ptr)return ret; } +void br_vlan_vlan_upper_event (struct net_device *br_dev, +struct net_device *vlan_dev, +unsigned long event) +{ +struct vlan_dev_priv *vlan =vlan_dev_priv (vlan_dev); +struct net_bridge *br =netdev_priv (br_dev); +bool bridge_binding; + +switch (event) { +case NETDEV_CHANGE : +case NETDEV_UP : +break ; +default : +return ; + } + + bridge_binding = vlan->flags &VLAN_FLAG_BRIDGE_BINDING ; +br_vlan_toggle_bridge_binding (br_dev, bridge_binding); +if (bridge_binding) +br_vlan_set_vlan_dev_state (br, vlan_dev); +else if (!bridge_binding &&netif_carrier_ok (br_dev)) +netif_carrier_on (vlan_dev); +} +/* Must be protected by RTNL. */ void br_vlan_port_event (struct net_bridge_port *p,unsigned long event) {
Signature · 1 line
2.47.0
Re: [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
Quoted 85 lines · Petr Machata
When bridge binding is enabled on a VLAN netdevice, its link state should track bridge ports that are members of the corresponding VLAN. This works for newly-added netdevices. However toggling the option does not have the effect of enabling or disabling the behavior as appropriate.
In this patch, react to
bridge_bindingtoggles on VLAN uppers.Signed-off-by Petr Machata <[email protected]>Reviewed-by Ido Schimmel <[email protected]>--- net/bridge/br.c | 7 +++++++ net/bridge/br_private.h | 9 +++++++++ net/bridge/br_vlan.c | 24 ++++++++++++++++++++++++ 3 files changed, 40 insertions(+)net/bridge/br.c +7
-0@@ -51,6 +51,13 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v } } +if (is_vlan_dev (dev)) { +struct net_device *real_dev =vlan_dev_real_dev (dev); + +if (netif_is_bridge_master (real_dev)) +br_vlan_vlan_upper_event (real_dev, dev, event); + } +/* not a port of a bridge */ p =br_port_get_rtnl (dev);if (!p)net/bridge/br_private.h +9
-0@@ -1571,6 +1571,9 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,void br_vlan_port_event (struct net_bridge_port *p,unsigned long event);int br_vlan_bridge_event (struct net_device *dev,unsigned long event,void *ptr); +void br_vlan_vlan_upper_event (struct net_device *br_dev, +struct net_device *vlan_dev, +unsigned long event);int br_vlan_rtnl_init (void );void br_vlan_rtnl_uninit (void ); void br_vlan_notify(const struct net_bridge *br,@@ -1802,6 +1805,12 @@ static inline int br_vlan_bridge_event(struct net_device *dev,return 0 ; } +static inline void br_vlan_vlan_upper_event (struct net_device *br_dev, +struct net_device *vlan_dev, +unsigned long event) +{ +} +static inline int br_vlan_rtnl_init (void ) {return 0 ;net/bridge/br_vlan.c +17
-0@@ -1772,6 +1772,30 @@ int br_vlan_bridge_event(struct net_device *dev, unsigned long event, void *ptr)return ret; } +void br_vlan_vlan_upper_event (struct net_device *br_dev, +struct net_device *vlan_dev, +unsigned long event) +{ +struct vlan_dev_priv *vlan =vlan_dev_priv (vlan_dev); +struct net_bridge *br =netdev_priv (br_dev); +bool bridge_binding; + +switch (event) { +case NETDEV_CHANGE : +case NETDEV_UP : +break ; +default : +return ; + } + + bridge_binding = vlan->flags &VLAN_FLAG_BRIDGE_BINDING ;
net/bridge/br_vlan.c int br_vlan_bridge_event(struct net_device *dev, unsigned long event, void *ptr) · line 1801
Quoted 10 lines
net/bridge/br_vlan.c +7
-0+br_vlan_toggle_bridge_binding (br_dev, bridge_binding); +if (bridge_binding) +br_vlan_set_vlan_dev_state (br, vlan_dev); +else if (!bridge_binding &&netif_carrier_ok (br_dev)) +netif_carrier_on (vlan_dev); +} +/* Must be protected by RTNL. */ void br_vlan_port_event (struct net_bridge_port *p,unsigned long event) {
PATCH 3/4 selftests: net: lib: Add a couple autodefer helpers
In reply to Petr MachataAlongside the helper ip_link_set_up(), one to set the link down will be useful as well. Add a helper to determine the link state as well, ip_link_is_up(), and use it to short-circuit any changes if the state is already the desired one.
Furthermore, add a helper bridge_vlan_add().
---
CC: Shuah Khan <[email protected]>
CC: [email protected]
--- tools/testing/selftests/net/lib.sh | 31 ++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
tools/testing/selftests/net/lib.sh +29 -2
@@ -477,12 +477,33 @@ ip_link_set_addr() defer ip link set dev "$name" address "$old_addr" } +ip_link_is_up() +{ + local name=$1; shift + + local state=$(ip -j link show "$name" | + jq -r '(.[].flags[] | select(. == "UP")) // "DOWN"') + [[ $state == "UP" ]] +} + ip_link_set_up() { local name=$1; shift - ip link set dev "$name" up - defer ip link set dev "$name" down + if ! ip_link_is_up "$name"; then + ip link set dev "$name" up + defer ip link set dev "$name" down + fi +} + +ip_link_set_down() +{ + local name=$1; shift + + if ip_link_is_up "$name"; then + ip link set dev "$name" down + defer ip link set dev "$name" up + fi } ip_addr_add()
@@ -498,3 +519,9 @@ ip_route_add() ip route add "$@" defer ip route del "$@" } + +bridge_vlan_add() +{ + bridge vlan add "$@" + defer bridge vlan del "$@" +}
Signature · 1 line
2.47.0
Re: [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers
Quoted 58 lines · Petr Machata
Alongside the helper
ip_link_set_up(), one to set the link down will be useful as well. Add a helper to determine the link state as well,ip_link_is_up(), and use it to short-circuit any changes if the state is already the desired one.Furthermore, add a helper
bridge_vlan_add().Signed-off-by Petr Machata <[email protected]>Reviewed-by Ido Schimmel <[email protected]>---
CC: Shuah Khan <[email protected]>
CC: [email protected]--- tools/testing/selftests/net/lib.sh | 31 ++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)tools/testing/selftests/net/lib.sh +23
-2@@ -477,12 +477,33 @@ ip_link_set_addr() defer ip link set dev "$name" address "$old_addr" } +ip_link_is_up() +{ + local name=$1; shift + + local state=$(ip -j link show "$name" | + jq -r '(.[].flags[] | select(. == "UP")) // "DOWN"') + [[ $state == "UP" ]] +} + ip_link_set_up() { local name=$1; shift - ip link set dev "$name" up - defer ip link set dev "$name" down + if ! ip_link_is_up "$name"; then + ip link set dev "$name" up + defer ip link set dev "$name" down + fi +} + +ip_link_set_down() +{ + local name=$1; shift + + if ip_link_is_up "$name"; then + ip link set dev "$name" down + defer ip link set dev "$name" up + fi } ip_addr_add()
tools/testing/selftests/net/lib.sh ip_route_add() · line 527
Quoted 9 lines
tools/testing/selftests/net/lib.sh +6
-0@@ -498,3 +519,9 @@ ip_route_add() ip route add "$@" defer ip route del "$@" } + +bridge_vlan_add() +{ + bridge vlan add "$@" + defer bridge vlan del "$@" +}
PATCH 4/4 selftests: net: Add a VLAN bridge binding selftest
In reply to Petr MachataAdd a test that exercises bridge binding.
---
CC: Shuah Khan <[email protected]>
CC: [email protected]
--- tools/testing/selftests/net/Makefile | 1 + .../selftests/net/vlan_bridge_binding.sh | 256 ++++++++++++++++++ 2 files changed, 257 insertions(+) create mode 100755 tools/testing/selftests/net/vlan_bridge_binding.sh
tools/testing/selftests/net/Makefile +1 -0
@@ -96,6 +96,7 @@ TEST_PROGS += test_bridge_backup_port.sh TEST_PROGS += fdb_flush.sh fdb_notify.sh TEST_PROGS += fq_band_pktlimit.sh TEST_PROGS += vlan_hw_filter.sh +TEST_PROGS += vlan_bridge_binding.sh TEST_PROGS += bpf_offload.py TEST_PROGS += ipv6_route_update_soft_lockup.sh TEST_PROGS += busy_poll_test.sh
tools/testing/selftests/net/vlan_bridge_binding.sh new file +256 -0
256 lines not on this page · raw message
Signature · 1 line
2.47.0
Re: [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest
Quoted 8 lines · Petr Machata
Add a test that exercises bridge binding.
Signed-off-by Petr Machata <[email protected]>Reviewed-by Ido Schimmel <[email protected]>---
CC: Shuah Khan <[email protected]>
CC: [email protected]
Quoted 6 lines
--- tools/testing/selftests/net/Makefile | 1 + .../selftests/net/vlan_bridge_binding.sh | 256 ++++++++++++++++++ 2 files changed, 257 insertions(+) create mode 100755 tools/testing/selftests/net/vlan_bridge_binding.sh
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Quoted 9 lines · On Wed, 18 Dec 2024 18:15:55 +0100 you wrote:
When bridge binding is enabled on a VLAN netdevice, its link state should track bridge ports that are members of the corresponding VLAN. This works for a newly-added netdevices. However toggling the option does not have the effect of enabling or disabling the behavior as appropriate.
In this patchset, have bridge react to
bridge_bindingtoggles on VLAN uppers.[...]
Here is the summary with links:
- [net-next,1/4] net: bridge: Extract a helper to handle bridge_binding toggles
https://git.kernel.org/netdev/net-next/c/f284424dc17b - [net-next,2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING https://git.kernel.org/netdev/net-next/c/3abd45122c72 - [net-next,3/4] selftests: net: lib: Add a couple autodefer helpers https://git.kernel.org/netdev/net-next/c/976d248bd333 - [net-next,4/4] selftests: net: Add a VLAN bridge binding selftest https://git.kernel.org/netdev/net-next/c/dca12e9ab760
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html