Re: [PATCH v6 net-next 3/8] vxlan: vnifilter: signal interrupted RTM_GETTUNNEL dumps

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Changelog/code mismatch: the commit message claims that "an in-
  place group update in vxlan_vni_update() (which splits or merges…

--

vxlan: vnifilter: signal interrupted RTM_GETTUNNEL dumps

This adds a per-netns generation counter that is bumped when the set of
vxlan devices in a netns changes, when a vni_list entry is added or
removed, and when a vni group is updated in place.  The counter is fed to
nl_dump_check_consistent() in vxlan_vnifilter_dump_dev() so user space
receives NLM_F_DUMP_INTR and can retry the RTM_GETTUNNEL dump.

Quoting the changelog:

>     A concurrent __vxlan_vni_add_list(), which inserts sorted by VNI,
>     __vxlan_vni_del_list(), or an in-place group update in
>     vxlan_vni_update() (which splits or merges coalesced VNI ranges) shifts
>     the second cursor, duplicating or skipping entries.

[Severity: Low]
Is the part about vxlan_vni_update() accurate?  The two other mechanisms
named there (list insert/delete moving cb->args[1], device
register/unregister moving cb->args[0]) do look real, but the claim that a
group update shifts the second cursor does not seem to match
vxlan_vnifilter_dump_dev().

cb->args[1] is a plain ordinal into vg->vni_list, advanced either one node
at a time for skipped nodes:

	list_for_each_entry_rcu(v, &vg->vni_list, vlist) {
		if (idx < s_idx) {
			idx++;
			continue;
		}

or in whole-node units when a coalesced range is flushed:

			idx += vnirange(vbegin, vend) + 1;
			vbegin = v;

Coalescing only folds strictly consecutive VNIs:

		if (!dump_stats && vnirange(vend, v) == 1 &&
		    vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) {

so vnirange(vbegin, vend) + 1 is exactly the number of vni_list nodes
merged into the emitted message, whether or not the ranges are split
differently on the next pass.

A group update only rewrites vninode->remote_ip; it adds no node, removes
no node, and cannot change a node's VNI, so no node's ordinal moves.
Doesn't a resumed dump therefore skip exactly the nodes it already
emitted, with nothing duplicated and nothing lost?

What a concurrent group update can do is make the whole dump internally
stale, for example an earlier skb reporting VNI X inside range [A-B] with
the old group while a later skb reports the new one.  That is a content
consistency argument rather than a cursor shift argument.  Could the
changelog be reworded to describe that instead for this bump site?

> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 0a04e8875f7dc..181a7614be5cd 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c

[ ... ]

> @@ -571,8 +587,11 @@ static int vxlan_vni_update_group(struct vxlan_dev *vxlan,
>  	if (ret)
>  		goto out;
>  
> -	if (group)
> +	if (group) {
>  		memcpy(&vninode->remote_ip, group, sizeof(vninode->remote_ip));
> +		if (!create)
> +			vxlan_vnifilter_seq_inc(dev_net(vxlan->dev));
> +	}
>  
>  	if (vxlan->dev->flags & IFF_UP) {
>  		if (vxlan_addr_multicast(&old_remote_ip) &&

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922181102.3989489-1-edumazet%40google.com