[PATCH net-next] bridge: Allow deleting FDB entries with non-existent VLAN

[PATCH net-next] bridge: Allow deleting FDB entries with non-existent VLAN

[Ido Schimmel]

It is currently impossible to delete individual FDB entries (as opposed
to flushing) that were added with a VLAN that no longer exists:

 # ip link add name dummy1 up type dummy
 # ip link add name br1 up type bridge vlan_filtering 1
 # ip link set dev dummy1 master br1
 # bridge fdb add 00:11:22:33:44:55 dev dummy1 master static vlan 1
 # bridge vlan del vid 1 dev dummy1
 # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
 00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
 # bridge fdb del 00:11:22:33:44:55 dev dummy1 master vlan 1
 RTNETLINK answers: Invalid argument
 # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
 00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static

This is in contrast to MDB entries that can be deleted after the VLAN
was deleted:

 # bridge vlan add vid 10 dev dummy1
 # bridge mdb add dev br1 port dummy1 grp 239.1.1.1 permanent vid 10
 # bridge vlan del vid 10 dev dummy1
 # bridge mdb get dev br1 grp 239.1.1.1 vid 10
 dev br1 port dummy1 grp 239.1.1.1 permanent vid 10
 # bridge mdb del dev br1 port dummy1 grp 239.1.1.1 permanent vid 10
 # bridge mdb get dev br1 grp 239.1.1.1 vid 10
 Error: bridge: MDB entry not found.

Align the two interfaces and allow user space to delete FDB entries that
were added with a VLAN that no longer exists:

 # ip link add name dummy1 up type dummy
 # ip link add name br1 up type bridge vlan_filtering 1
 # ip link set dev dummy1 master br1
 # bridge fdb add 00:11:22:33:44:55 dev dummy1 master static vlan 1
 # bridge vlan del vid 1 dev dummy1
 # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
 00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
 # bridge fdb del 00:11:22:33:44:55 dev dummy1 master vlan 1
 # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
 Error: Fdb entry not found.

Add a selftest to make sure this behavior does not regress:

 # ./rtnetlink.sh -t kci_test_fdb_del
 PASS: bridge fdb del

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Andy Roulin <aroulin@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 net/bridge/br_fdb.c                      |  9 ++----
 tools/testing/selftests/net/rtnetlink.sh | 40 ++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 1cd7bade9b3b..77f110035df1 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1319,7 +1319,6 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
 {
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_port *p = NULL;
-	struct net_bridge_vlan *v;
 	struct net_bridge *br;
 	int err;
 
@@ -1338,14 +1337,10 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
 	}
 
 	if (vid) {
-		v = br_vlan_find(vg, vid);
-		if (!v) {
-			pr_info("bridge: RTM_DELNEIGH with unconfigured vlan %d on %s\n", vid, dev->name);
-			return -EINVAL;
-		}
-
 		err = __br_fdb_delete(br, p, addr, vid);
 	} else {
+		struct net_bridge_vlan *v;
+
 		err = -ENOENT;
 		err &= __br_fdb_delete(br, p, addr, 0);
 		if (!vg || !vg->num_vlans)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 87dce3efe31e..6e216d7a8e2f 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -25,6 +25,7 @@ ALL_TESTS="
 	kci_test_ipsec
 	kci_test_ipsec_offload
 	kci_test_fdb_get
+	kci_test_fdb_del
 	kci_test_neigh_get
 	kci_test_bridge_parent_id
 	kci_test_address_proto
@@ -1065,6 +1066,45 @@ kci_test_fdb_get()
 	end_test "PASS: bridge fdb get"
 }
 
+kci_test_fdb_del()
+{
+	local test_mac=de:ad:be:ef:13:37
+	local dummydev="dummy1"
+	local brdev="test-br0"
+	local ret=0
+
+	run_cmd_grep 'bridge fdb get' bridge fdb help
+	if [ $? -ne 0 ]; then
+		end_test "SKIP: fdb del tests: iproute2 too old"
+		return $ksft_skip
+	fi
+
+	setup_ns testns
+	if [ $? -ne 0 ]; then
+		end_test "SKIP fdb del tests: cannot add net namespace $testns"
+		return $ksft_skip
+	fi
+	IP="ip -netns $testns"
+	BRIDGE="bridge -netns $testns"
+	run_cmd $IP link add $dummydev type dummy
+	run_cmd $IP link add name $brdev type bridge vlan_filtering 1
+	run_cmd $IP link set dev $dummydev master $brdev
+	run_cmd $BRIDGE fdb add $test_mac dev $dummydev master static vlan 1
+	run_cmd $BRIDGE vlan del vid 1 dev $dummydev
+	run_cmd $BRIDGE fdb get $test_mac br $brdev vlan 1
+	run_cmd $BRIDGE fdb del $test_mac dev $dummydev master vlan 1
+	run_cmd_fail $BRIDGE fdb get $test_mac br $brdev vlan 1
+
+	ip netns del $testns &>/dev/null
+
+	if [ $ret -ne 0 ]; then
+		end_test "FAIL: bridge fdb del"
+		return 1
+	fi
+
+	end_test "PASS: bridge fdb del"
+}
+
 kci_test_neigh_get()
 {
 	dstmac=de:ad:be:ef:13:37
-- 
2.47.0

Re: [PATCH net-next] bridge: Allow deleting FDB entries with non-existent VLAN

[Nikolay Aleksandrov]

On Tue, Nov 05, 2024 at 03:39:54PM +0200, Ido Schimmel wrote:
> It is currently impossible to delete individual FDB entries (as opposed
> to flushing) that were added with a VLAN that no longer exists:
> 
>  # ip link add name dummy1 up type dummy
>  # ip link add name br1 up type bridge vlan_filtering 1
>  # ip link set dev dummy1 master br1
>  # bridge fdb add 00:11:22:33:44:55 dev dummy1 master static vlan 1
>  # bridge vlan del vid 1 dev dummy1
>  # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
>  00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
>  # bridge fdb del 00:11:22:33:44:55 dev dummy1 master vlan 1
>  RTNETLINK answers: Invalid argument
>  # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
>  00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
> 
> This is in contrast to MDB entries that can be deleted after the VLAN
> was deleted:
> 
>  # bridge vlan add vid 10 dev dummy1
>  # bridge mdb add dev br1 port dummy1 grp 239.1.1.1 permanent vid 10
>  # bridge vlan del vid 10 dev dummy1
>  # bridge mdb get dev br1 grp 239.1.1.1 vid 10
>  dev br1 port dummy1 grp 239.1.1.1 permanent vid 10
>  # bridge mdb del dev br1 port dummy1 grp 239.1.1.1 permanent vid 10
>  # bridge mdb get dev br1 grp 239.1.1.1 vid 10
>  Error: bridge: MDB entry not found.
> 
> Align the two interfaces and allow user space to delete FDB entries that
> were added with a VLAN that no longer exists:
> 
>  # ip link add name dummy1 up type dummy
>  # ip link add name br1 up type bridge vlan_filtering 1
>  # ip link set dev dummy1 master br1
>  # bridge fdb add 00:11:22:33:44:55 dev dummy1 master static vlan 1
>  # bridge vlan del vid 1 dev dummy1
>  # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
>  00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
>  # bridge fdb del 00:11:22:33:44:55 dev dummy1 master vlan 1
>  # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
>  Error: Fdb entry not found.
> 
> Add a selftest to make sure this behavior does not regress:
> 
>  # ./rtnetlink.sh -t kci_test_fdb_del
>  PASS: bridge fdb del
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Andy Roulin <aroulin@nvidia.com>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> ---
>  net/bridge/br_fdb.c                      |  9 ++----
>  tools/testing/selftests/net/rtnetlink.sh | 40 ++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 7 deletions(-)
> 
 
Nice catch, I'd even queue it for -net. :)
Of course we should be able to delete anything.

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

Re: [PATCH net-next] bridge: Allow deleting FDB entries with non-existent VLAN

[patchwork-bot+netdevbpf]

Hello:

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

On Tue, 5 Nov 2024 15:39:54 +0200 you wrote:
> It is currently impossible to delete individual FDB entries (as opposed
> to flushing) that were added with a VLAN that no longer exists:
> 
>  # ip link add name dummy1 up type dummy
>  # ip link add name br1 up type bridge vlan_filtering 1
>  # ip link set dev dummy1 master br1
>  # bridge fdb add 00:11:22:33:44:55 dev dummy1 master static vlan 1
>  # bridge vlan del vid 1 dev dummy1
>  # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
>  00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
>  # bridge fdb del 00:11:22:33:44:55 dev dummy1 master vlan 1
>  RTNETLINK answers: Invalid argument
>  # bridge fdb get 00:11:22:33:44:55 br br1 vlan 1
>  00:11:22:33:44:55 dev dummy1 vlan 1 master br1 static
> 
> [...]

Here is the summary with links:
  - [net-next] bridge: Allow deleting FDB entries with non-existent VLAN
    https://git.kernel.org/netdev/net-next/c/774ca6d3bf24

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html