netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
@ 2024-12-18 17:15 Petr Machata
  2024-12-18 17:15 ` [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles Petr Machata
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Petr Machata @ 2024-12-18 17:15 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev
  Cc: Simon Horman, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	Ido Schimmel, Petr Machata, mlxsw

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/cover.1660100506.git.sevinj.aghayeva@gmail.com/

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

-- 
2.47.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles
  2024-12-18 17:15 [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
@ 2024-12-18 17:15 ` Petr Machata
  2024-12-20 12:27   ` Nikolay Aleksandrov
  2024-12-18 17:15 ` [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Petr Machata @ 2024-12-18 17:15 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev
  Cc: Simon Horman, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	Ido Schimmel, Petr Machata, mlxsw

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.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_vlan.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 89f51ea4cabe..b728b71e693f 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -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 {
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
  2024-12-18 17:15 [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
  2024-12-18 17:15 ` [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles Petr Machata
@ 2024-12-18 17:15 ` Petr Machata
  2024-12-20 12:27   ` Nikolay Aleksandrov
  2024-12-18 17:15 ` [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers Petr Machata
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Petr Machata @ 2024-12-18 17:15 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev
  Cc: Simon Horman, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	Ido Schimmel, Petr Machata, mlxsw

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_binding toggles on VLAN uppers.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br.c         |  7 +++++++
 net/bridge/br_private.h |  9 +++++++++
 net/bridge/br_vlan.c    | 24 ++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/net/bridge/br.c b/net/bridge/br.c
index 2cab878e0a39..183fcb362f9e 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -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)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 9853cfbb9d14..29d6ec45cf41 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -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;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index b728b71e693f..d9a69ec9affe 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -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)
 {
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers
  2024-12-18 17:15 [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
  2024-12-18 17:15 ` [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles Petr Machata
  2024-12-18 17:15 ` [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
@ 2024-12-18 17:15 ` Petr Machata
  2024-12-20 12:28   ` Nikolay Aleksandrov
  2024-12-18 17:15 ` [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest Petr Machata
  2024-12-20 21:30 ` [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING patchwork-bot+netdevbpf
  4 siblings, 1 reply; 10+ messages in thread
From: Petr Machata @ 2024-12-18 17:15 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev
  Cc: Simon Horman, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	Ido Schimmel, Petr Machata, mlxsw, Shuah Khan, linux-kselftest

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 <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
CC: Shuah Khan <shuah@kernel.org>
CC: linux-kselftest@vger.kernel.org

---
 tools/testing/selftests/net/lib.sh | 31 ++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index 2cd5c743b2d9..0bd9a038a1f0 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -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 "$@"
+}
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest
  2024-12-18 17:15 [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
                   ` (2 preceding siblings ...)
  2024-12-18 17:15 ` [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers Petr Machata
@ 2024-12-18 17:15 ` Petr Machata
  2024-12-20 12:28   ` Nikolay Aleksandrov
  2024-12-20 21:30 ` [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING patchwork-bot+netdevbpf
  4 siblings, 1 reply; 10+ messages in thread
From: Petr Machata @ 2024-12-18 17:15 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev
  Cc: Simon Horman, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	Ido Schimmel, Petr Machata, mlxsw, Shuah Khan, linux-kselftest

Add a test that exercises bridge binding.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
CC: Shuah Khan <shuah@kernel.org>
CC: linux-kselftest@vger.kernel.org

---
 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

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index f09bd96cc978..73ee88d6b043 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -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
diff --git a/tools/testing/selftests/net/vlan_bridge_binding.sh b/tools/testing/selftests/net/vlan_bridge_binding.sh
new file mode 100755
index 000000000000..e7cb8c678bde
--- /dev/null
+++ b/tools/testing/selftests/net/vlan_bridge_binding.sh
@@ -0,0 +1,256 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+source lib.sh
+
+ALL_TESTS="
+	test_binding_on
+	test_binding_off
+	test_binding_toggle_on
+	test_binding_toggle_off
+	test_binding_toggle_on_when_upper_down
+	test_binding_toggle_off_when_upper_down
+	test_binding_toggle_on_when_lower_down
+	test_binding_toggle_off_when_lower_down
+"
+
+setup_prepare()
+{
+	local port
+
+	ip_link_add br up type bridge vlan_filtering 1
+
+	for port in d1 d2 d3; do
+		ip_link_add $port type veth peer name r$port
+		ip_link_set_up $port
+		ip_link_set_up r$port
+		ip_link_set_master $port br
+	done
+
+	bridge_vlan_add vid 11 dev br self
+	bridge_vlan_add vid 11 dev d1 master
+
+	bridge_vlan_add vid 12 dev br self
+	bridge_vlan_add vid 12 dev d2 master
+
+	bridge_vlan_add vid 13 dev br self
+	bridge_vlan_add vid 13 dev d1 master
+	bridge_vlan_add vid 13 dev d2 master
+
+	bridge_vlan_add vid 14 dev br self
+	bridge_vlan_add vid 14 dev d1 master
+	bridge_vlan_add vid 14 dev d2 master
+	bridge_vlan_add vid 14 dev d3 master
+}
+
+operstate_is()
+{
+	local dev=$1; shift
+	local expect=$1; shift
+
+	local operstate=$(ip -j link show $dev | jq -r .[].operstate)
+	if [[ $operstate == UP ]]; then
+		operstate=1
+	elif [[ $operstate == DOWN || $operstate == LOWERLAYERDOWN ]]; then
+		operstate=0
+	fi
+	echo -n $operstate
+	[[ $operstate == $expect ]]
+}
+
+check_operstate()
+{
+	local dev=$1; shift
+	local expect=$1; shift
+	local operstate
+
+	operstate=$(busywait 1000 \
+			operstate_is "$dev" "$expect")
+	check_err $? "Got operstate of $operstate, expected $expect"
+}
+
+add_one_vlan()
+{
+	local link=$1; shift
+	local id=$1; shift
+
+	ip_link_add $link.$id link $link type vlan id $id "$@"
+}
+
+add_vlans()
+{
+	add_one_vlan br 11 "$@"
+	add_one_vlan br 12 "$@"
+	add_one_vlan br 13 "$@"
+	add_one_vlan br 14 "$@"
+}
+
+set_vlans()
+{
+	ip link set dev br.11 "$@"
+	ip link set dev br.12 "$@"
+	ip link set dev br.13 "$@"
+	ip link set dev br.14 "$@"
+}
+
+down_netdevs()
+{
+	local dev
+
+	for dev in "$@"; do
+		ip_link_set_down $dev
+	done
+}
+
+check_operstates()
+{
+	local opst_11=$1; shift
+	local opst_12=$1; shift
+	local opst_13=$1; shift
+	local opst_14=$1; shift
+
+	check_operstate br.11 $opst_11
+	check_operstate br.12 $opst_12
+	check_operstate br.13 $opst_13
+	check_operstate br.14 $opst_14
+}
+
+do_test_binding()
+{
+	local inject=$1; shift
+	local what=$1; shift
+	local opsts_d1=$1; shift
+	local opsts_d2=$1; shift
+	local opsts_d12=$1; shift
+	local opsts_d123=$1; shift
+
+	RET=0
+
+	defer_scope_push
+		down_netdevs d1
+		$inject
+		check_operstates $opsts_d1
+	defer_scope_pop
+
+	defer_scope_push
+		down_netdevs d2
+		$inject
+		check_operstates $opsts_d2
+	defer_scope_pop
+
+	defer_scope_push
+		down_netdevs d1 d2
+		$inject
+		check_operstates $opsts_d12
+	defer_scope_pop
+
+	defer_scope_push
+		down_netdevs d1 d2 d3
+		$inject
+		check_operstates $opsts_d123
+	defer_scope_pop
+
+	log_test "Test bridge_binding $what"
+}
+
+do_test_binding_on()
+{
+	local inject=$1; shift
+	local what=$1; shift
+
+	do_test_binding "$inject" "$what"	\
+			"0 1 1 1"		\
+			"1 0 1 1"		\
+			"0 0 0 1"		\
+			"0 0 0 0"
+}
+
+do_test_binding_off()
+{
+	local inject=$1; shift
+	local what=$1; shift
+
+	do_test_binding "$inject" "$what"	\
+			"1 1 1 1"		\
+			"1 1 1 1"		\
+			"1 1 1 1"		\
+			"0 0 0 0"
+}
+
+test_binding_on()
+{
+	add_vlans bridge_binding on
+	set_vlans up
+	do_test_binding_on : "on"
+}
+
+test_binding_off()
+{
+	add_vlans bridge_binding off
+	set_vlans up
+	do_test_binding_off : "off"
+}
+
+test_binding_toggle_on()
+{
+	add_vlans bridge_binding off
+	set_vlans up
+	set_vlans type vlan bridge_binding on
+	do_test_binding_on : "off->on"
+}
+
+test_binding_toggle_off()
+{
+	add_vlans bridge_binding on
+	set_vlans up
+	set_vlans type vlan bridge_binding off
+	do_test_binding_off : "on->off"
+}
+
+dfr_set_binding_on()
+{
+	set_vlans type vlan bridge_binding on
+	defer set_vlans type vlan bridge_binding off
+}
+
+dfr_set_binding_off()
+{
+	set_vlans type vlan bridge_binding off
+	defer set_vlans type vlan bridge_binding on
+}
+
+test_binding_toggle_on_when_lower_down()
+{
+	add_vlans bridge_binding off
+	set_vlans up
+	do_test_binding_on dfr_set_binding_on "off->on when lower down"
+}
+
+test_binding_toggle_off_when_lower_down()
+{
+	add_vlans bridge_binding on
+	set_vlans up
+	do_test_binding_off dfr_set_binding_off "on->off when lower down"
+}
+
+test_binding_toggle_on_when_upper_down()
+{
+	add_vlans bridge_binding off
+	set_vlans type vlan bridge_binding on
+	set_vlans up
+	do_test_binding_on : "off->on when upper down"
+}
+
+test_binding_toggle_off_when_upper_down()
+{
+	add_vlans bridge_binding on
+	set_vlans type vlan bridge_binding off
+	set_vlans up
+	do_test_binding_off : "on->off when upper down"
+}
+
+trap defer_scopes_cleanup EXIT
+setup_prepare
+tests_run
+
+exit $EXIT_STATUS
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles
  2024-12-18 17:15 ` [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles Petr Machata
@ 2024-12-20 12:27   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2024-12-20 12:27 UTC (permalink / raw)
  To: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev
  Cc: Simon Horman, Roopa Prabhu, bridge, Ido Schimmel, mlxsw

On 12/18/24 19:15, Petr Machata wrote:
> 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.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/bridge/br_vlan.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 89f51ea4cabe..b728b71e693f 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -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 {

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
  2024-12-18 17:15 ` [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
@ 2024-12-20 12:27   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2024-12-20 12:27 UTC (permalink / raw)
  To: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev
  Cc: Simon Horman, Roopa Prabhu, bridge, Ido Schimmel, mlxsw

On 12/18/24 19:15, Petr Machata 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 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.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/bridge/br.c         |  7 +++++++
>  net/bridge/br_private.h |  9 +++++++++
>  net/bridge/br_vlan.c    | 24 ++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/net/bridge/br.c b/net/bridge/br.c
> index 2cab878e0a39..183fcb362f9e 100644
> --- a/net/bridge/br.c
> +++ b/net/bridge/br.c
> @@ -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)
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 9853cfbb9d14..29d6ec45cf41 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -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;
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index b728b71e693f..d9a69ec9affe 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -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)
>  {

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers
  2024-12-18 17:15 ` [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers Petr Machata
@ 2024-12-20 12:28   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2024-12-20 12:28 UTC (permalink / raw)
  To: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev
  Cc: Simon Horman, Roopa Prabhu, bridge, Ido Schimmel, mlxsw,
	Shuah Khan, linux-kselftest

On 12/18/24 19:15, Petr Machata wrote:
> 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 <petrm@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
> CC: Shuah Khan <shuah@kernel.org>
> CC: linux-kselftest@vger.kernel.org
> 
> ---
>  tools/testing/selftests/net/lib.sh | 31 ++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index 2cd5c743b2d9..0bd9a038a1f0 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -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 "$@"
> +}

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest
  2024-12-18 17:15 ` [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest Petr Machata
@ 2024-12-20 12:28   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2024-12-20 12:28 UTC (permalink / raw)
  To: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev
  Cc: Simon Horman, Roopa Prabhu, bridge, Ido Schimmel, mlxsw,
	Shuah Khan, linux-kselftest

On 12/18/24 19:15, Petr Machata wrote:
> Add a test that exercises bridge binding.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
> CC: Shuah Khan <shuah@kernel.org>
> CC: linux-kselftest@vger.kernel.org
> 
> ---
>  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
> 

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING
  2024-12-18 17:15 [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
                   ` (3 preceding siblings ...)
  2024-12-18 17:15 ` [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest Petr Machata
@ 2024-12-20 21:30 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-12-20 21:30 UTC (permalink / raw)
  To: Petr Machata
  Cc: davem, edumazet, kuba, pabeni, netdev, horms, roopa, razor,
	bridge, idosch, mlxsw

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

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_binding toggles 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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-12-20 21:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-18 17:15 [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
2024-12-18 17:15 ` [PATCH net-next 1/4] net: bridge: Extract a helper to handle bridge_binding toggles Petr Machata
2024-12-20 12:27   ` Nikolay Aleksandrov
2024-12-18 17:15 ` [PATCH net-next 2/4] net: bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING Petr Machata
2024-12-20 12:27   ` Nikolay Aleksandrov
2024-12-18 17:15 ` [PATCH net-next 3/4] selftests: net: lib: Add a couple autodefer helpers Petr Machata
2024-12-20 12:28   ` Nikolay Aleksandrov
2024-12-18 17:15 ` [PATCH net-next 4/4] selftests: net: Add a VLAN bridge binding selftest Petr Machata
2024-12-20 12:28   ` Nikolay Aleksandrov
2024-12-20 21:30 ` [PATCH net-next 0/4] bridge: Handle changes in VLAN_FLAG_BRIDGE_BINDING patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).