[PATCH net v2 0/3] vxlan: vnifilter: fix VNI add/update notifications

[PATCH net v2 0/3] vxlan: vnifilter: fix VNI add/update notifications

[Andy Roulin]

When a vxlan device has vnifilter enabled, userspace observers
(e.g., bridge monitor vni) miss VNI add events and see spurious
notifications on no-op VNI re-adds.

Patch 1 fixes the missing notification on VNI add: vxlan_vni_add()
guarded the notification on a 'changed' flag that vxlan_vni_update_group()
only sets when a multicast group or remote is supplied, so VNIs added
without a group (e.g., L3 VXLAN) were silently created.

Patch 2 fixes the spurious notification on VNI update: vxlan_vni_update()
tested 'if (changed)' against a bool pointer instead of dereferencing it,
so every re-add produced a notification regardless of whether anything
actually changed.

Patch 3 adds a selftest covering both bugs along with a few related
cases (add with remote, remote update, delete-nonexistent).

Changes since v1:
- Retarget to net.
- Patch 3: improved vni_notify_check helper based on review by
  sashiko.dev:
  * Bump pre-cmd sleep 0.1s -> 0.5s.
  * Add /proc/$monitor_pid liveness check and ksft_skip if
    iproute2 doesn't support 'bridge monitor vni'.
  * Capture and propagate "$@"'s exit status so check_err $?
    actually validates the bridge command's return.

v1: https://lore.kernel.org/netdev/20260518165700.1975478-1-aroulin@nvidia.com/

Andy Roulin (3):
  vxlan: vnifilter: send notification on VNI add
  vxlan: vnifilter: fix spurious notification on VNI update
  selftests: net: add vxlan vnifilter notification test

 drivers/net/vxlan/vxlan_vnifilter.c           |   5 +-
 tools/testing/selftests/net/Makefile          |   1 +
 .../net/test_vxlan_vnifilter_notify.sh        | 184 ++++++++++++++++++
 3 files changed, 187 insertions(+), 3 deletions(-)
 create mode 100755 tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh

-- 
2.43.0

[PATCH net v2 1/3] vxlan: vnifilter: send notification on VNI add

[Andy Roulin]

When a new VNI is added to a vxlan device with vnifilter enabled,
no RTM_NEWTUNNEL notification is sent to userspace. This means
'bridge monitor vni' never shows VNI add events, even though
VNI delete events are reported correctly.

The bug is in vxlan_vni_add(), where the notification is guarded by
'if (changed)'. The 'changed' flag is set by vxlan_vni_update_group()
only when the multicast group or remote IP is modified, but for a
new VNI added without a group (e.g. in L3 VxLAN interface scenarios),
the function returns early without setting changed=true. Since this
is a new VNI, the notification should be sent unconditionally.

The notification is not guarded by the return value of
vxlan_vni_update_group() because, at this point, the VNI has already
been inserted into the hash table and list with no rollback on error.
The VNI will be visible in 'bridge vni show' regardless, so userspace
should be informed. This is consistent with vxlan_vni_del() which also
notifies unconditionally.

The 'if (changed)' guard remains correct in vxlan_vni_update(), which
handles the case where a VNI already exists and is being re-added --
there, we only want to notify if the group/remote actually changed.

Reproducer:

 # ip link add vxlan100 type vxlan dstport 4789 local 10.0.0.1 \
      nolearning external vnifilter
 # ip link set vxlan100 up
 # bridge monitor vni &
 # bridge vni add vni 1000 dev vxlan100    # no notification
 # bridge vni delete vni 1000 dev vxlan100 # notification received

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Reported-by: Chirag Shah <chirag@nvidia.com>
Signed-off-by: Andy Roulin <aroulin@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Assisted-by: Claude:claude-opus-4-6
---
 drivers/net/vxlan/vxlan_vnifilter.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 2042369379ffc..f2a202d468928 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -759,8 +759,7 @@ static int vxlan_vni_add(struct vxlan_dev *vxlan,
 	err = vxlan_vni_update_group(vxlan, vninode, group, true, &changed,
 				     extack);
 
-	if (changed)
-		vxlan_vnifilter_notify(vxlan, vninode, RTM_NEWTUNNEL);
+	vxlan_vnifilter_notify(vxlan, vninode, RTM_NEWTUNNEL);
 
 	return err;
 }
-- 
2.43.0

[PATCH net v2 2/3] vxlan: vnifilter: fix spurious notification on VNI update

[Andy Roulin]

When a VNI is re-added with the same attributes (e.g. same group or no
group), vxlan_vni_update() sends a spurious RTM_NEWTUNNEL notification
even though nothing changed.

The bug is that 'if (changed)' tests whether the pointer is non-NULL,
not the bool value it points to. Since every caller passes a valid
pointer, the condition is always true and the notification fires
unconditionally.

Fix by dereferencing the pointer: 'if (*changed)'.

Reproducer:

 # ip link add vxlan100 type vxlan dstport 4789 local 10.0.0.1 \
      nolearning external vnifilter
 # ip link set vxlan100 up
 # bridge monitor vni &
 # bridge vni add vni 1000 dev vxlan100
 # bridge vni add vni 1000 dev vxlan100  # spurious notification

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Signed-off-by: Andy Roulin <aroulin@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Assisted-by: Claude:claude-opus-4-6
---
 drivers/net/vxlan/vxlan_vnifilter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index f2a202d468928..3e76f4e210944 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -661,7 +661,7 @@ static int vxlan_vni_update(struct vxlan_dev *vxlan,
 	if (ret)
 		return ret;
 
-	if (changed)
+	if (*changed)
 		vxlan_vnifilter_notify(vxlan, vninode, RTM_NEWTUNNEL);
 
 	return 0;
-- 
2.43.0

[PATCH net v2 3/3] selftests: net: add vxlan vnifilter notification test

[Andy Roulin]

Add a selftest for VXLAN vnifilter netlink notifications that verifies
RTM_NEWTUNNEL and RTM_DELTUNNEL are sent correctly when VNIs are added,
deleted, or updated, and that no spurious notifications are sent when
a VNI is re-added with the same attributes.

Signed-off-by: Andy Roulin <aroulin@nvidia.com>
Acked-by: Petr Machata <petrm@nvidia.com>
Assisted-by: Claude:claude-opus-4-6
---

Notes:
    Changes since v1:
    - Improved vni_notify_check helper based on review by sashiko.dev:
      * Bump pre-cmd sleep 0.1s -> 0.5s.
      * Add /proc/$monitor_pid liveness check and ksft_skip if
        iproute2 doesn't support 'bridge monitor vni'.
      * Capture and propagate "$@"'s exit status so check_err
        $? actually validates the bridge command's return.

 tools/testing/selftests/net/Makefile          |   1 +
 .../net/test_vxlan_vnifilter_notify.sh        | 184 ++++++++++++++++++
 2 files changed, 185 insertions(+)
 create mode 100755 tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index f3da38c54d276..2ed7d803eb548 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -109,6 +109,7 @@ TEST_PROGS := \
 	test_vxlan_nh.sh \
 	test_vxlan_nolocalbypass.sh \
 	test_vxlan_under_vrf.sh \
+	test_vxlan_vnifilter_notify.sh \
 	test_vxlan_vnifiltering.sh \
 	tfo_passive.sh \
 	traceroute.sh \
diff --git a/tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh b/tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh
new file mode 100755
index 0000000000000..9d51a9e02ae0c
--- /dev/null
+++ b/tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh
@@ -0,0 +1,184 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# shellcheck disable=SC2034,SC2154,SC2317,SC2329
+#
+# Test for VXLAN vnifilter netlink notifications (RTM_NEWTUNNEL /
+# RTM_DELTUNNEL).
+#
+# Verifies that:
+# - Adding a new VNI sends a notification
+# - Adding a new VNI with a remote sends a notification
+# - Deleting a VNI sends a notification
+# - Re-adding an existing VNI with the same attributes does not send
+#   a spurious notification
+# - Updating an existing VNI's remote sends a notification
+# - Deleting a non-existent VNI does not send a notification
+
+source lib.sh
+
+require_command bridge
+
+VXLAN_DEV=vxlan100
+
+ALL_TESTS="
+	test_vni_add_notify
+	test_vni_add_remote_notify
+	test_vni_del_notify
+	test_vni_readd_no_notify
+	test_vni_update_remote_notify
+	test_vni_del_nonexistent_no_notify
+"
+
+setup_prepare()
+{
+	setup_ns NS1
+	defer cleanup_all_ns
+
+	ip -n "$NS1" link add $VXLAN_DEV type vxlan dstport 4789 \
+		local 10.0.0.1 nolearning external vnifilter
+	ip -n "$NS1" link set $VXLAN_DEV up
+}
+
+# Run bridge monitor in the background, execute a command, then count
+# the notification lines.
+# Usage: vni_notify_check <command> [args...]
+# Sets: NOTIFY_COUNT with the number of notifications observed.
+vni_notify_check()
+{
+	local tmpf cmd_ret monitor_pid
+
+	tmpf=$(mktemp)
+	defer rm "$tmpf"
+
+	defer_scope_push
+		ip netns exec "$NS1" bridge monitor vni > "$tmpf" 2>/dev/null &
+		monitor_pid=$!
+		defer kill_process "$monitor_pid"
+
+		sleep 0.5
+		if [ ! -e "/proc/$monitor_pid" ]; then
+			RET=$ksft_skip
+			log_test "iproute2 'bridge monitor vni' not supported"
+			return "$RET"
+		fi
+
+		"$@"
+		cmd_ret=$?
+		sleep 0.2
+	defer_scope_pop
+
+	NOTIFY_COUNT=$(grep -c "$VXLAN_DEV" "$tmpf")
+	NOTIFY_COUNT=${NOTIFY_COUNT:-0}
+	return "$cmd_ret"
+}
+
+# Adding a brand new VNI should produce a notification.
+test_vni_add_notify()
+{
+	RET=0
+
+	vni_notify_check \
+		bridge -n "$NS1" vni add vni 1000 dev "$VXLAN_DEV"
+	check_err $? "Failed to add VNI"
+
+	[ "$NOTIFY_COUNT" -eq 1 ]
+	check_err $? "Expected 1 notification for VNI add, got $NOTIFY_COUNT"
+
+	bridge -n "$NS1" vni delete vni 1000 dev "$VXLAN_DEV" 2>/dev/null
+
+	log_test "VNI add sends notification"
+}
+
+# Adding a VNI with a remote should produce a notification.
+test_vni_add_remote_notify()
+{
+	RET=0
+
+	vni_notify_check \
+		bridge -n "$NS1" vni add vni 4000 remote 10.0.0.2 dev "$VXLAN_DEV"
+	check_err $? "Failed to add VNI with remote"
+
+	[ "$NOTIFY_COUNT" -eq 1 ]
+	check_err $? "Expected 1 notification for VNI add with remote, got $NOTIFY_COUNT"
+
+	bridge -n "$NS1" vni delete vni 4000 dev "$VXLAN_DEV"
+
+	log_test "VNI add with remote sends notification"
+}
+
+# Deleting a VNI should produce a notification.
+test_vni_del_notify()
+{
+	RET=0
+
+	bridge -n "$NS1" vni add vni 2000 dev "$VXLAN_DEV"
+
+	vni_notify_check \
+		bridge -n "$NS1" vni delete vni 2000 dev "$VXLAN_DEV"
+	check_err $? "Failed to delete VNI"
+
+	[ "$NOTIFY_COUNT" -eq 1 ]
+	check_err $? "Expected 1 notification for VNI del, got $NOTIFY_COUNT"
+
+	log_test "VNI delete sends notification"
+}
+
+# Re-adding an existing VNI with the same attributes should not produce
+# a notification.
+test_vni_readd_no_notify()
+{
+	RET=0
+
+	bridge -n "$NS1" vni add vni 3000 dev "$VXLAN_DEV"
+
+	vni_notify_check \
+		bridge -n "$NS1" vni add vni 3000 dev "$VXLAN_DEV"
+	check_err $? "Failed to re-add VNI"
+
+	[ "$NOTIFY_COUNT" -eq 0 ]
+	check_err $? "Expected 0 notifications for VNI re-add, got $NOTIFY_COUNT"
+
+	bridge -n "$NS1" vni delete vni 3000 dev "$VXLAN_DEV"
+
+	log_test "VNI re-add does not send spurious notification"
+}
+
+# Updating an existing VNI's remote should produce a notification.
+test_vni_update_remote_notify()
+{
+	RET=0
+
+	bridge -n "$NS1" vni add vni 5000 remote 10.0.0.2 dev "$VXLAN_DEV"
+
+	vni_notify_check \
+		bridge -n "$NS1" vni add vni 5000 remote 10.0.0.3 dev "$VXLAN_DEV"
+	check_err $? "Failed to update VNI remote"
+
+	[ "$NOTIFY_COUNT" -eq 1 ]
+	check_err $? "Expected 1 notification for VNI remote update, got $NOTIFY_COUNT"
+
+	bridge -n "$NS1" vni delete vni 5000 dev "$VXLAN_DEV"
+
+	log_test "VNI remote update sends notification"
+}
+
+# Deleting a non-existent VNI should not produce a notification.
+test_vni_del_nonexistent_no_notify()
+{
+	RET=0
+
+	vni_notify_check \
+		bridge -n "$NS1" vni delete vni 9999 dev "$VXLAN_DEV" 2>/dev/null
+
+	[ "$NOTIFY_COUNT" -eq 0 ]
+	check_err $? "Expected 0 notifications for non-existent VNI del, got $NOTIFY_COUNT"
+
+	log_test "Non-existent VNI delete does not send notification"
+}
+
+trap defer_scopes_cleanup EXIT
+
+setup_prepare
+tests_run
+
+exit "$EXIT_STATUS"
-- 
2.43.0

Re: [PATCH net v2 3/3] selftests: net: add vxlan vnifilter notification test

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/

Simon says:

There is also an AI-generated review of this patch on sashiko.dev.
And AI-generated reviews of other patches of this series.
But I believe what is forwarded here covers what should
be considered in the context of this patch-set.

---
selftests: net: add vxlan vnifilter notification test

This patch adds a selftest that exercises VXLAN vnifilter netlink
notifications by running bridge monitor vni in the background while
adding, updating, deleting and re-adding VNIs, then counting the
matching lines in the captured output.

> diff --git a/tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh b/tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh
> new file mode 100755
> index 000000000000..9d51a9e02ae0
> --- /dev/null
> +++ b/tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh
> @@ -0,0 +1,184 @@

[ ... ]

> +# Run bridge monitor in the background, execute a command, then count
> +# the notification lines.
> +# Usage: vni_notify_check <command> [args...]
> +# Sets: NOTIFY_COUNT with the number of notifications observed.
> +vni_notify_check()
> +{
> +	local tmpf cmd_ret monitor_pid
> +
> +	tmpf=$(mktemp)
> +	defer rm "$tmpf"
> +
> +	defer_scope_push
> +		ip netns exec "$NS1" bridge monitor vni > "$tmpf" 2>/dev/null &
> +		monitor_pid=$!
> +		defer kill_process "$monitor_pid"
> +
> +		sleep 0.5
> +		if [ ! -e "/proc/$monitor_pid" ]; then
> +			RET=$ksft_skip
> +			log_test "iproute2 'bridge monitor vni' not supported"
> +			return "$RET"
> +		fi

[Medium]
Could the skip path here turn a legitimate skip into a failure?

When bridge monitor vni is not supported, the helper sets RET=$ksft_skip,
calls log_test from inside the helper, and then returns ksft_skip (4).
Every caller follows the helper with:

    vni_notify_check \
        bridge -n "$NS1" vni add vni 1000 dev "$VXLAN_DEV"
    check_err $? "Failed to add VNI"

Since ksft_skip is non-zero, check_err treats the return as a failure
and calls ret_set_ksft_status with ksft_fail. Per the merge weights in
ksft_status_merge (pass < xfail < skip < fail), that demotes RET from
skip to fail.

NOTIFY_COUNT is also never assigned because the function returned
before the grep, so the next:

    [ "$NOTIFY_COUNT" -eq 1 ]
    check_err $? "Expected 1 notification for VNI add, got $NOTIFY_COUNT"

also records a failure, and the caller's log_test then prints FAIL for
the real test name in addition to the SKIP printed by the helper. The
script ends up reporting both a SKIP and a FAIL line for the same case
and exits with a failure status.

Two related observations on this same path:

The bare return is issued between defer_scope_push and defer_scope_pop,
so defer_scope_pop is skipped on the skip path. Does that leave
__DEFER__SCOPE_ID off by one and the registered defer kill_process
"$monitor_pid" only unwound through the EXIT trap rather than the
normal scope unwind?

Would it be cleaner to signal skip back to the caller without going
through check_err (for example, set RET plus a flag and have the caller
short-circuit), and either move log_test to the caller or restructure
the skip path so it does not straddle defer_scope_push/defer_scope_pop?
fdb_notify.sh uses a similar pattern without mixing log_test and an
early return inside the scope.

> +
> +		"$@"
> +		cmd_ret=$?
> +		sleep 0.2
> +	defer_scope_pop
> +
> +	NOTIFY_COUNT=$(grep -c "$VXLAN_DEV" "$tmpf")
> +	NOTIFY_COUNT=${NOTIFY_COUNT:-0}
> +	return "$cmd_ret"
> +}

[Low]
Does this counting strategy actually distinguish RTM_NEWTUNNEL from
RTM_DELTUNNEL?

The commit message states:

    verifies that RTM_NEWTUNNEL and RTM_DELTUNNEL are sent correctly
    when VNIs are added, deleted, or updated

but NOTIFY_COUNT comes from:

    NOTIFY_COUNT=$(grep -c "$VXLAN_DEV" "$tmpf")

which matches both add and delete events emitted by bridge monitor vni.
A regression that emitted RTM_NEWTUNNEL on delete (or vice versa) would
still produce NOTIFY_COUNT=1 and pass every positive test. Would
differentiating the two (for instance, by separately counting lines
that begin with "Deleted " versus those that do not) make the test
match what the commit message describes?