Skip to contentSkip to conversation

vxlan: vnifilter: fix VNI add/update notifications

PATCH Applied Andy Roulin 4 participants 7 messages Jun 2 – Jun 4

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover vxlan: vnifilter: fix VNI add/update notifications

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/[email protected]/

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
Signature · 1 line
2.43.0
#1 lore raw

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

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")
Assisted-by Claude:claude-opus-4-6
---
 drivers/net/vxlan/vxlan_vnifilter.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/vxlan/vxlan_vnifilter.c +1 -2
index 2042369379ffc..f2a202d468928 100644
@@ -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;
 }
Signature · 1 line
2.43.0
#2 lore raw

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

In reply to 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")
Assisted-by Claude:claude-opus-4-6
---
 drivers/net/vxlan/vxlan_vnifilter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/vxlan/vxlan_vnifilter.c +1 -1
index f2a202d468928..3e76f4e210944 100644
@@ -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;
Signature · 1 line
2.43.0
#3 lore raw

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

In reply to 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.

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
tools/testing/selftests/net/Makefile +1 -0
index f3da38c54d276..2ed7d803eb548 100644
@@ -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 \
tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh new file +184 -0

184 lines not on this page · raw message

Signature · 1 line
2.43.0
reviewer
#4 lore raw

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

From: 'Simon Horman' <[email protected]>

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.

Quoted 6 lines
tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh new file +0 -0
new file mode 100755
index 000000000000..9d51a9e02ae0
@@ -0,0 +1,184 @@

[ ... ]

Quoted 12 lines
tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh new file +12 -0
+# 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

tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh line 63

Quoted 10 lines
tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh new file +10 -0
+		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.

tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh line 73

Quoted 10 lines
tools/testing/selftests/net/test_vxlan_vnifilter_notify.sh new file +10 -0
+
+		"$@"
+		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?

maintainer
#5 lore raw

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

Quoted 10 lines · On Thu, 4 Jun 2026 14:19:25 +0100 Simon Horman wrote:

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.

FWIW doesn't read like a blocker to me, please follow up if necessary.

#6 lore raw
In reply to Andy Roulin
This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Tue, 2 Jun 2026 11:51:35 -0700 you wrote:

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.

[...]

Here is the summary with links:
- [net,v2,1/3] vxlan: vnifilter: send notification on VNI add

    https://git.kernel.org/netdev/net/c/aa6ca1c5c338
  - [net,v2,2/3] vxlan: vnifilter: fix spurious notification on VNI update
    https://git.kernel.org/netdev/net/c/84683b5b60c7
  - [net,v2,3/3] selftests: net: add vxlan vnifilter notification test
    https://git.kernel.org/netdev/net/c/8df1c84e7272

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html