mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request
       [not found] <20260907141001.GA708129@shredder>
@ 2026-09-09  9:26 ` Ali Firas
  2026-09-09  9:26   ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ali Firas @ 2026-09-09  9:26 UTC (permalink / raw)
  To: netdev, idosch
  Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa,
	linux-kernel, Ali Firas

A single RTM_NEWTUNNEL or RTM_DELTUNNEL message can currently ask the
vnifilter code for the whole 24-bit VNI space, and vxlan_vni_add_del()
loops over that span under rtnl_lock creating one VNI node and one
per-CPU stats block per iteration. That is reachable by an unprivileged
user in a user+network namespace and it costs roughly 2.1 GiB + 1 GiB
per possible CPU (128 B slab plus 64 B per possible CPU per VNI): on
2, 4 and 8 CPU guests every full-range request
ended in a global OOM, with no errno returned because the caller is
itself OOM-killed, and with unrelated root-owned processes killed on the
way down. Because rtnl_lock is global rather than per-netns, it also
stalled network configuration everywhere: a plain "ip link add dummy0
type dummy" in a different network namespace takes 0.011 s normally,
4.472 s while a 1,000,000 VNI request runs, and never completed at all
during a full-range one. Patch 1 caps the span of a single request at
4096 VNIs, which closes both. The cap is on one request, not on the
device: a device can still hold the whole VNI space, it just takes more
than one message.

Patch 2 charges the VNI node and its per-CPU stats to the caller's memcg.
With the cap in place this is no longer the primary defence, but nothing
limits how many capped requests a task may issue, so an unprivileged user
can still accumulate VNIs 4096 at a time with none of it charged to them.
It is also the same class fix as commit 1beb81947eb4 ("net/sched: account
classifier filter allocations to memcg"). One limitation is worth stating
up front: try_charge() reclaims and then invokes the memcg OOM killer
rather than returning -ENOMEM, so accounting confines the blast radius
without producing a graceful failure.

Patch 3 adds the selftest coverage Ido asked for, in the existing API
test: a range of exactly the maximum is accepted and one VNI more is
rejected, for both add and delete.

The limit is a driver-local constant rather than VLAN_N_VID: the values
coincide today, but a bound on a VXLAN netlink request is not a count of
VLAN IDs, and coupling them would make a change to one silently change
the other.

Patch 1 does tighten uAPI: a request spanning more than 4096 VNIs used
to succeed and now returns -EINVAL. Ido's assessment was that the limit
is unlikely to break anyone, since vnifilter is mainly used on bridged
VXLAN devices where the VNI is derived from the VLAN, capped at 4094.
I am sending the series to net because the stall is reachable by an
unprivileged user and crosses namespaces.

What I am less sure about is that Fixes: in net means this reaches
stable, where a script issuing one large range would start failing
across a point release. If that is the wrong trade, I am happy to respin
patch 1 against net-next without the Fixes tag.

Two things the series does not address. A single-VNI request with
START == END >= VXLAN_N_VID still passes the span check, and
vxlan_vni_field() shifts without masking, so such an entry is silently
truncated on the wire while holding its own rhashtable slot. That wants
a netlink policy range check, which I will send separately to net-next
as a pure uAPI tightening.

Measured worst case at the cap, on a 2 CPU / 2G guest: a request of
exactly 4096 VNIs takes 0.031 s to add and 0.022 s to delete, and the
cross-namespace "ip link add" blocks for 0.024 s during it. The old
full-range request is now rejected outright with

  Error: VNI range spans more than 4096 VNIs.

for both add and delete.

The whole selftest file passes before and after: 27 tests passed and 0
failed on the base, 31 passed and 0 failed with the series applied.

The 4096 limit follows Ido Schimmel's suggestion:
https://lore.kernel.org/netdev/20260907141001.GA708129@shredder/

Ali Firas (3):
  vxlan: vnifilter: limit the VNI range of a single request
  vxlan: vnifilter: account VNI node and per-CPU stats to memcg
  selftests: net: test the vxlan vnifilter VNI range limit

 drivers/net/vxlan/vxlan_vnifilter.c           | 24 +++++++++++++++++--
 .../selftests/net/test_vxlan_vnifiltering.sh  | 13 ++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

-- 
2.53.0


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

* [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request
  2026-09-09  9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas
@ 2026-09-09  9:26   ` Ali Firas
  2026-09-10  9:38     ` netdev-bot+sashiko
  2026-09-15  0:31     ` Jakub Kicinski
  2026-09-09  9:26   ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas
  2026-09-09  9:26   ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas
  2 siblings, 2 replies; 9+ messages in thread
From: Ali Firas @ 2026-09-09  9:26 UTC (permalink / raw)
  To: netdev, idosch
  Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa,
	linux-kernel, Ali Firas

VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END are parsed
without any bound on how far apart they are, so a single RTM_NEWTUNNEL
message can ask for the whole 24-bit VNI space. vxlan_vni_add_del() then
loops over that span creating one VNI node and one per-CPU stats block
per iteration, all under rtnl_lock.

Two things follow from that, both reachable by an unprivileged user in a
user+network namespace, since adding VNIs only requires CAP_NET_ADMIN in
the network namespace's user namespace:

  - the allocation is unbounded. Each VNI costs 128 bytes of slab plus
    64 bytes per possible CPU, so a full in-range request costs roughly
    2.1 GiB + 1 GiB per possible CPU. Measured in a QEMU guest on 2, 4
    and 8 CPU configurations, every one of them ends in a global OOM
    with the allocating task in vxlan_vnifilter_process(). No errno is
    returned because the calling process is itself OOM-killed, and the
    OOM killer also killed unrelated root-owned processes.

  - rtnl_lock is held for the entire loop. rtnl is global rather than
    per-netns, so unrelated network configuration blocks everywhere for
    as long as the request runs. Measured with a plain "ip link add
    dummy0 type dummy" in a different network namespace: it takes 0.011
    s normally, 4.472 s while a 1,000,000 VNI request runs, and during a
    full-range request it never completes at all.

Cap the span of one request at 4096 VNIs. The limit is on a single
request, not on how many VNIs a device may hold: a device can still be
populated with the whole VNI space, it just takes more than one message.
The value follows from how the interface is used in practice, on bridged
VXLAN devices where the VNI is derived from the VLAN and so cannot
exceed the 4094 usable VLAN IDs.

The limit is written as a driver-local constant rather than reusing
VLAN_N_VID. The two numbers coincide today, but a bound on a VXLAN
netlink request is not a count of VLAN IDs, and tying them together
would make a change to one silently change the other.

The check sits in vxlan_process_vni_filter(), where the span is known
and before any VNI is created, so it rejects the request before any work
is done. Both RTM_NEWTUNNEL and RTM_DELTUNNEL reach vxlan_vni_add_del()
through this one function, so a single check covers add and delete.

The span is inclusive, so START=0 END=4095 is 4096 VNIs and is accepted,
while START=0 END=4096 is 4097 and is rejected. A request carrying only
END has START default to 0 and is bounded the same way. A start above
the end selects no VNI at all and is deliberately left behaving as it
does today, rather than being turned into an error by unsigned
wraparound.

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
 drivers/net/vxlan/vxlan_vnifilter.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index dd94085e0886..f18ce0e1e741 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -17,6 +17,14 @@
 
 #include "vxlan_private.h"
 
+/* Maximum number of VNIs a single RTM_NEWTUNNEL or RTM_DELTUNNEL request may
+ * span.  VNI filtering is mainly used on bridged VXLAN devices where the VNI
+ * is derived from the VLAN, so a span wider than the VLAN ID space has no
+ * practical use, while an unbounded span lets one netlink message create up
+ * to 2^24 VNIs under rtnl_lock.
+ */
+#define VXLAN_VNI_FILTER_RANGE_MAX	4096
+
 static inline int vxlan_vni_cmp(struct rhashtable_compare_arg *arg,
 				const void *ptr)
 {
@@ -869,6 +877,17 @@ static int vxlan_process_vni_filter(struct vxlan_dev *vxlan,
 		return -EINVAL;
 	}
 
+	/* Only bound a well-formed range; a start above the end selects no
+	 * VNI at all and is left behaving as before.
+	 */
+	if (vni_end >= vni_start &&
+	    vni_end - vni_start >= VXLAN_VNI_FILTER_RANGE_MAX) {
+		NL_SET_ERR_MSG_ATTR_FMT(extack, nlvnifilter,
+					"VNI range spans more than %u VNIs",
+					VXLAN_VNI_FILTER_RANGE_MAX);
+		return -EINVAL;
+	}
+
 	if (vattrs[VXLAN_VNIFILTER_ENTRY_GROUP]) {
 		group.sin.sin_addr.s_addr =
 			nla_get_in_addr(vattrs[VXLAN_VNIFILTER_ENTRY_GROUP]);
-- 
2.53.0


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

* [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg
  2026-09-09  9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas
  2026-09-09  9:26   ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas
@ 2026-09-09  9:26   ` Ali Firas
  2026-09-10  9:38     ` netdev-bot+sashiko
  2026-09-15  0:31     ` Jakub Kicinski
  2026-09-09  9:26   ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas
  2 siblings, 2 replies; 9+ messages in thread
From: Ali Firas @ 2026-09-09  9:26 UTC (permalink / raw)
  To: netdev, idosch
  Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa,
	linux-kernel, Ali Firas

vxlan_vni_alloc() allocates a struct vxlan_vni_node and a per-CPU stats
block for every VNI, both with plain GFP_KERNEL. Neither carries
__GFP_ACCOUNT, so the memory is not charged to the cgroup of the process
that asked for it.

With the range of a single request now capped, one message can no longer
exhaust memory on its own. This is no longer the primary defence, but it
still matters: nothing limits how many capped requests a task may issue,
so an unprivileged user in a user+network namespace can still accumulate
an arbitrary number of VNIs, 4096 at a time, and none of it is charged
to them.

Per VNI the add path allocates 128 bytes of slab, an exact fit in
kmalloc-128 and measured at exactly 1.000 objects per VNI, plus 64 bytes
per possible CPU for the stats block. The per-CPU term is the one that
grows: 256 bytes per VNI on a 2-CPU host, but 4.2 KB per VNI on a
64-CPU one.

Charging both allocations confines the damage to the caller's cgroup.
The kill becomes CONSTRAINT_MEMCG with oom_memcg set to that cgroup,
memory.stat attributes both the slab and the percpu bytes to it, and the
host survives what previously took it down.

One limitation is worth stating plainly: try_charge() reclaims and then
invokes the memcg OOM killer rather than returning -ENOMEM, so the
request does not fail gracefully, the caller is killed. Accounting
confines the blast radius, it does not turn this into a clean error.

For a caller not under a memcg limit there is no change. With no limit
set, the same workload installs the same number of VNIs to within 0.4%,
fails at the same point, and a bounded add of 1,000,000 VNIs costs an
identical 128 bytes of slab and 64 bytes per CPU. The objects simply
move from kmalloc-128 to kmalloc-cg-128.

Conditions to recreate the bug:
- CONFIG_VXLAN, CONFIG_MEMCG.
- Unprivileged user in a fresh user+network namespace (unshare -Urn),
  or root with CAP_NET_ADMIN.
- Create a vnifilter-enabled vxlan device and add VNIs in a loop
  (e.g. ip link add vx0 type vxlan external vnifilter dstport 4789,
  then repeated bridge vni add ... commands) while watching a
  memcg-limited cgroup: system slab and percpu grow far faster than
  memory.current, pinning kernel memory outside memcg charging.

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
 drivers/net/vxlan/vxlan_vnifilter.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index f18ce0e1e741..3d6718ec3f55 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -703,10 +703,11 @@ static struct vxlan_vni_node *vxlan_vni_alloc(struct vxlan_dev *vxlan,
 {
 	struct vxlan_vni_node *vninode;
 
-	vninode = kzalloc_obj(*vninode);
+	vninode = kzalloc_obj(*vninode, GFP_KERNEL_ACCOUNT);
 	if (!vninode)
 		return NULL;
-	vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu);
+	vninode->stats = __netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu,
+						   GFP_KERNEL_ACCOUNT);
 	if (!vninode->stats) {
 		kfree(vninode);
 		return NULL;
-- 
2.53.0


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

* [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit
  2026-09-09  9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas
  2026-09-09  9:26   ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas
  2026-09-09  9:26   ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas
@ 2026-09-09  9:26   ` Ali Firas
  2026-09-10  9:39     ` netdev-bot+sashiko
  2 siblings, 1 reply; 9+ messages in thread
From: Ali Firas @ 2026-09-09  9:26 UTC (permalink / raw)
  To: netdev, idosch
  Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa,
	linux-kernel, Ali Firas

A single RTM_NEWTUNNEL or RTM_DELTUNNEL request may span at most 4096
VNIs. Nothing covered the range functionality at all, so add the
boundary cases to the existing API test: a range of exactly the maximum
is accepted and one VNI more is rejected, for both add and delete.

The range used sits above the VNIs the surrounding tests already
install, so it does not disturb them.

Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
 .../selftests/net/test_vxlan_vnifiltering.sh        | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
index 8deacc565afa..464ff353d6c0 100755
--- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
+++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
@@ -371,6 +371,19 @@ vxlan_vnifilter_api()
 	# change vxlan vnifilter flag
 	run_cmd "ip -netns $testns link set dev vxlan-ext1 type vxlan external novnifilter"
 	log_test $? 2 "Cannot unset vnifilter flag on a device"
+
+	# a single request may span at most 4096 vnis
+	run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14095"
+	log_test $? 0 "Add vni range of maximum size"
+
+	run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14096"
+	log_test $? 255 "Cannot add vni range larger than maximum"
+
+	run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14096"
+	log_test $? 255 "Cannot delete vni range larger than maximum"
+
+	run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14095"
+	log_test $? 0 "Delete vni range of maximum size"
 }
 
 # Sanity test vnifilter datapath
-- 
2.53.0


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

* Re: [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request
  2026-09-09  9:26   ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas
@ 2026-09-10  9:38     ` netdev-bot+sashiko
  2026-09-15  0:31     ` Jakub Kicinski
  1 sibling, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10  9:38 UTC (permalink / raw)
  To: alishmery18
  Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
	razor, roopa, linux-kernel

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 2 · Medium: 1 · Low: 0

- [High] The new cap is enforced per VXLAN_VNIFILTER_ENTRY attribute, not
  per netlink request, so the bug class the commit claims to close stays…
- [High] The added check can be bypassed entirely because
  vxlan_vni_add_del() (drivers/net/vxlan/vxlan_vnifilter.c:820) iterates…
- [Medium] The kernel now emits VNI-filter messages it will refuse to
  parse.

--

vxlan: vnifilter: limit the VNI range of a single request

This adds a driver-local constant VXLAN_VNI_FILTER_RANGE_MAX of 4096 and
rejects a VXLAN_VNIFILTER_ENTRY_START/END pair whose span is wider than
that, in vxlan_process_vni_filter().  The stated goal is to stop one
RTM_NEWTUNNEL or RTM_DELTUNNEL message from creating up to 2^24 VNIs
under rtnl_lock.

> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index dd94085e0886..f18ce0e1e741 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -17,6 +17,14 @@
>  
>  #include "vxlan_private.h"
>  
> +/* Maximum number of VNIs a single RTM_NEWTUNNEL or RTM_DELTUNNEL request may
> + * span.  VNI filtering is mainly used on bridged VXLAN devices where the VNI
> + * is derived from the VLAN, so a span wider than the VLAN ID space has no
> + * practical use, while an unbounded span lets one netlink message create up
> + * to 2^24 VNIs under rtnl_lock.
> + */
> +#define VXLAN_VNI_FILTER_RANGE_MAX	4096
> +
>  static inline int vxlan_vni_cmp(struct rhashtable_compare_arg *arg,
>  				const void *ptr)
>  {

[ ... ]

> @@ -869,6 +877,17 @@ static int vxlan_process_vni_filter(struct vxlan_dev *vxlan,
>  		return -EINVAL;
>  	}
>  
> +	/* Only bound a well-formed range; a start above the end selects no
> +	 * VNI at all and is left behaving as before.
> +	 */
> +	if (vni_end >= vni_start &&
> +	    vni_end - vni_start >= VXLAN_VNI_FILTER_RANGE_MAX) {
> +		NL_SET_ERR_MSG_ATTR_FMT(extack, nlvnifilter,
> +					"VNI range spans more than %u VNIs",
> +					VXLAN_VNI_FILTER_RANGE_MAX);
> +		return -EINVAL;
> +	}
> +

[Severity: High]

Is this bound per request, or per VXLAN_VNIFILTER_ENTRY attribute?
vxlan_process_vni_filter() is called once per entry from
vxlan_vnifilter_process():

	nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh,
				 sizeof(*tmsg), rem) {
		err = vxlan_process_vni_filter(vxlan, attr, nlh->nlmsg_type,
					       extack);
		vnis++;
		if (err)
			break;
	}

The vnis counter is only used afterwards to emit "No vnis found to
process", it is never a budget, and vni_filter_policy places no limit on
how many nested VXLAN_VNIFILTER_ENTRY attributes a message may carry:

	static const struct nla_policy vni_filter_policy[VXLAN_VNIFILTER_MAX + 1] = {
		[VXLAN_VNIFILTER_ENTRY] = { .type = NLA_NESTED },
	};

A minimal entry is a 4 byte nest header plus an 8 byte START and an 8
byte END, so a message that fits inside the default sk_sndbuf can carry
on the order of ten thousand entries, each of which passes the new check
while spanning 4096 VNIs.  That sums to more than the entire 24-bit space
in one message.  The handlers are registered with flags 0:

	{THIS_MODULE, PF_BRIDGE, RTM_NEWTUNNEL, vxlan_vnifilter_process, NULL, 0},
	{THIS_MODULE, PF_BRIDGE, RTM_DELTUNNEL, vxlan_vnifilter_process, NULL, 0},

so rtnl_lock is held for the whole loop, with no lock drop and no
cond_resched(), and each iteration of vxlan_vni_add_del() still does
kzalloc_obj() plus netdev_alloc_pcpu_stats() per VNI in vxlan_vni_alloc().

Would the bound need to accumulate the span across all entries of the
message, or be applied in vxlan_vnifilter_process() before any entry is
handed to vxlan_process_vni_filter()?

The commit message says:

  "The check sits in vxlan_process_vni_filter(), where the span is known
  and before any VNI is created, so it rejects the request before any
  work is done."

Since entries are parsed and applied one at a time, an oversized entry in
the middle of a message returns -EINVAL only after all preceding entries
have already created their VNI nodes and sent their notifications.  Should
that sentence be adjusted, along with the comment above the constant which
describes the value as a cap on what "a single RTM_NEWTUNNEL or
RTM_DELTUNNEL request may span"?

[Severity: High]

Can this check be skipped entirely with a large START value?
vni_filter_entry_policy declares the two attributes without any maximum:

	[VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 },
	[VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 },

so START = 0xffffffff and no END gives vni_start == vni_end ==
0xffffffff.  The "!vni_start && !vni_end" test above does not fire, and
here vni_end - vni_start is 0, so this looks like a one VNI request and is
accepted.

vxlan_vni_add_del() then iterates with a signed counter:

	int v, err = 0;
	...
	for (v = start_vni; v <= end_vni; v++) {

v starts at -1, and in "v <= end_vni" it is converted to unsigned against
the __u32 end_vni, so 0xffffffff <= 0xffffffff is true, a VNI is created,
v++ makes v == 0, and 0 <= 0xffffffff is still true.  Does the loop then
keep walking upwards from 0, creating a node plus per-CPU stats plus a
notification per iteration under rtnl_lock, until -ENOMEM or until
rhashtable returns -E2BIG (vxlan_vni_rht_params has .max_size =
VXLAN_N_VID, so max_elems is about 33.5M)?  vxlan_vni_in_use() returns 0
for a namespace holding a single vnifilter device, so nothing seems to
abort it earlier.  Once v passes INT_MAX the increment is also signed
overflow.

The same applies to any range that ends at 0xffffffff, for example
START = 0xfffff001 with END = 0xffffffff, whose span is 4095.  Would
bounding the absolute values (or making the loop counter unsigned) be
needed in addition to bounding the difference?

[Severity: Medium]

Does this make the dump output unparseable by the same code path?
vxlan_vnifilter_dump_dev() merges an unbounded run of contiguous VNIs
that share a remote into one VXLAN_VNIFILTER_ENTRY:

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

and then calls vxlan_fill_vni_filter_entry(skb, vbegin, vend, dump_stats)
once for the merged run, emitting a single START/END pair with no clamp on
the span.  The commit message keeps it legal for a device to hold the whole
VNI space through several requests, so a device populated with, say,
100000 contiguous VNIs via 25 accepted requests is dumped as one entry
with END - START == 99999.  Replaying that entry, or deleting the run with
one RTM_DELTUNNEL, now returns -EINVAL with "VNI range spans more than
4096 VNIs".

Should the dump side chunk merged runs to the same maximum span, or should
the input restriction be expressed as something other than a limit on the
START/END span?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909092645.3105263-1-alishmery18%40gmail.com

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

* Re: [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg
  2026-09-09  9:26   ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas
@ 2026-09-10  9:38     ` netdev-bot+sashiko
  2026-09-15  0:31     ` Jakub Kicinski
  1 sibling, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10  9:38 UTC (permalink / raw)
  To: alishmery18
  Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
	razor, roopa, linux-kernel

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] The commit message's quantitative model of the per-CPU charge is
  wrong/incomplete.
- [Low] The commit message states categorically that 'try_charge()
  reclaims and then invokes the memcg OOM killer rather than returning…

--

vxlan: vnifilter: account VNI node and per-CPU stats to memcg

This changes vxlan_vni_alloc() to allocate the struct vxlan_vni_node with
GFP_KERNEL_ACCOUNT and to switch the per-VNI stats block from
netdev_alloc_pcpu_stats() to __netdev_alloc_pcpu_stats() with
GFP_KERNEL_ACCOUNT, so both allocations are charged to the caller's memcg.

Both comments below are about the numbers and the failure behaviour
described in the changelog, not about the code change itself.

> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index f18ce0e1e741..3d6718ec3f55 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -703,10 +703,11 @@ static struct vxlan_vni_node *vxlan_vni_alloc(struct vxlan_dev *vxlan,
>  {
>  	struct vxlan_vni_node *vninode;
>  
> -	vninode = kzalloc_obj(*vninode);
> +	vninode = kzalloc_obj(*vninode, GFP_KERNEL_ACCOUNT);
>  	if (!vninode)
>  		return NULL;
> -	vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu);
> +	vninode->stats = __netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu,
> +						   GFP_KERNEL_ACCOUNT);
>  	if (!vninode->stats) {
>  		kfree(vninode);
>  		return NULL;

[Severity: Low]
Is the stated cost model for this per-CPU allocation complete?  The
changelog says:

    "plus 64 bytes per possible CPU for the stats block. The per-CPU term
     is the one that grows: 256 bytes per VNI on a 2-CPU host, but 4.2 KB
     per VNI on a 64-CPU one."

Once __GFP_ACCOUNT is set, the amount charged comes from
pcpu_obj_full_size() in mm/percpu-internal.h:

	size_t extra_size = 0;

	#ifdef CONFIG_MEMCG
		if (!mem_cgroup_kmem_disabled())
			extra_size += size / PCPU_MIN_ALLOC_SIZE * sizeof(struct obj_cgroup *);
	#endif

		return size * num_possible_cpus() + extra_size;

For a 64-byte object on a 64-bit build that extra term is 64 / 4 * 8 =
128 bytes, which is what makes the quoted measurements add up
(64 * 2 + 128 = 256, 64 * 64 + 128 = 4224).  Would it be clearer to state
the obj_cgroup metadata term alongside the per-CPU term, since the stated
formula and the stated measurements otherwise disagree?

Also, is the 64-byte figure architecture independent?  struct
vxlan_vni_stats_pcpu in include/net/vxlan.h embeds a u64_stats_sync:

	struct vxlan_vni_stats_pcpu {
		struct vxlan_vni_stats stats;
		struct u64_stats_sync syncp;
	};

That member is empty on 64-bit but carries a seqcount on 32-bit, so the
object is larger than 64 bytes there.

[Severity: Low]
The changelog states:

    "try_charge() reclaims and then invokes the memcg OOM killer rather
     than returning -ENOMEM, so the request does not fail gracefully, the
     caller is killed"

Is that categorical?  In try_charge_memcg() in mm/memcontrol.c, when
mem_cgroup_oom() returns false (no killable task, oom_lock bypass, an
existing OOM victim with MMF_OOM_SKIP, or task_in_memcg_oom(current)),
control reaches:

	nomem:
		if (!(gfp_mask & (__GFP_NOFAIL | __GFP_HIGH)))
			return -ENOMEM;

GFP_KERNEL_ACCOUNT carries neither __GFP_NOFAIL nor __GFP_HIGH, so the
charge can return -ENOMEM, vxlan_vni_alloc() then returns NULL and the
netlink request does fail with -ENOMEM rather than the caller being
killed.

Separately, when the memcg OOM killer does run it picks a victim inside
the constrained cgroup, which need not be the allocating task, and
memory.oom.group can extend the kill to the whole cgroup.

Would it be more accurate to say the add may trigger memcg reclaim and
then either a memcg OOM kill of one or more tasks in the cgroup, or a
plain -ENOMEM?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909092645.3105263-1-alishmery18%40gmail.com

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

* Re: [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit
  2026-09-09  9:26   ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas
@ 2026-09-10  9:39     ` netdev-bot+sashiko
  0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10  9:39 UTC (permalink / raw)
  To: alishmery18
  Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
	razor, roopa, linux-kernel

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 · Low: 0

- [High] The series' stated guarantee — "A single RTM_NEWTUNNEL or
  RTM_DELTUNNEL request may span at most 4096 VNIs" (this selftest's…
- [Medium] The new "Cannot delete vni range larger than maximum" case
  (tools/testing/selftests/net/test_vxlan_vnifiltering.sh:382-383) has…

--

selftests: net: test the vxlan vnifilter VNI range limit

This adds four cases to vxlan_vnifilter_api() in
test_vxlan_vnifiltering.sh covering the new 4096-VNI cap on a vnifilter
request: an add and a delete of exactly the maximum span are expected to
succeed, and an add and a delete one VNI larger are expected to fail.
No Link: tags were present in the commit header.

> diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> index 8deacc565afa..464ff353d6c0 100755
> --- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> +++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> @@ -371,6 +371,19 @@ vxlan_vnifilter_api()
>  	# change vxlan vnifilter flag
>  	run_cmd "ip -netns $testns link set dev vxlan-ext1 type vxlan external novnifilter"
>  	log_test $? 2 "Cannot unset vnifilter flag on a device"
> +
> +	# a single request may span at most 4096 vnis
> +	run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14095"
> +	log_test $? 0 "Add vni range of maximum size"
> +
> +	run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14096"
> +	log_test $? 255 "Cannot add vni range larger than maximum"

[Severity: High]
The comment here, and the commit message wording "A single RTM_NEWTUNNEL
or RTM_DELTUNNEL request may span at most 4096 VNIs", describe a
per-request property.  Is that what the companion patch actually
enforces?

The check lives in vxlan_process_vni_filter():

	if (vni_end >= vni_start &&
	    vni_end - vni_start >= VXLAN_VNI_FILTER_RANGE_MAX) {
		NL_SET_ERR_MSG_ATTR_FMT(extack, nlvnifilter,
					"VNI range spans more than %u VNIs",
					VXLAN_VNI_FILTER_RANGE_MAX);
		return -EINVAL;
	}

and vxlan_vnifilter_process() calls that helper once per
VXLAN_VNIFILTER_ENTRY attribute present in the message:

	nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh,
				 sizeof(*tmsg), rem) {
		err = vxlan_process_vni_filter(vxlan, attr, nlh->nlmsg_type,
					       extack);
		vnis++;
		if (err)
			break;
	}

So a message carrying N entries, each an individually legal 4096-VNI
span, still results in N * 4096 add/del operations inside one rtnl_lock
hold.  An entry nest with only START and END is roughly 20 bytes on the
wire, and netlink_sendmsg() accepts messages up to sk_sndbuf/2, so
thousands of entries fit in a single request.

Since iproute2 emits exactly one VXLAN_VNIFILTER_ENTRY per message, can
any of the four new cases observe a multi-entry request?  They all
exercise the per-entry path only.

Would it make sense either to accumulate the span across entries in
vxlan_vnifilter_process(), or to reword the comment above
VXLAN_VNI_FILTER_RANGE_MAX, the commit messages and these test names to
say "per entry"?

> +
> +	run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14096"
> +	log_test $? 255 "Cannot delete vni range larger than maximum"
> +
> +	run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14095"
> +	log_test $? 0 "Delete vni range of maximum size"

[Severity: Medium]
Can the "Cannot delete vni range larger than maximum" case distinguish
the range limit from a plain missing-VNI error?

Only 10000-14095 were installed by the first case, so the delete of
10000-14096 has two ways to return non-zero.  With the cap present,
vxlan_process_vni_filter() returns -EINVAL and nothing is touched.
With the cap removed, vxlan_vni_add_del() deletes 10000-14095 and then
vxlan_vni_del() hits:

	vninode = rhashtable_lookup_fast(&vg->vni_hash, &v,
					 vxlan_vni_rht_params);
	if (!vninode) {
		err = -ENOENT;
		goto out;
	}

which the loop propagates.  iproute2 maps either errno to exit status
255, the same coarse value the pre-existing "vni add dev vxlan-ext2 vni
200" case already expects, so log_test $? 255 passes in both cases.

In the no-cap case the 4096 VNIs are already gone, so the next
assertion, "Delete vni range of maximum size" expecting 0, is the one
that fails.  Does that not report the regression under the wrong test
name?

Would deleting a span whose every VNI exists (for example installing
10000-14096 with two capped adds first), or matching the extack text
rather than only the exit status, make the case specific to the limit?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909092645.3105263-1-alishmery18%40gmail.com

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

* Re: [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request
  2026-09-09  9:26   ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas
  2026-09-10  9:38     ` netdev-bot+sashiko
@ 2026-09-15  0:31     ` Jakub Kicinski
  1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-09-15  0:31 UTC (permalink / raw)
  To: Ali Firas
  Cc: netdev, idosch, pabeni, davem, edumazet, andrew+netdev, razor,
	roopa, linux-kernel

On Wed,  9 Sep 2026 12:26:43 +0300 Ali Firas wrote:
> VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END are parsed
> without any bound on how far apart they are, so a single RTM_NEWTUNNEL
> message can ask for the whole 24-bit VNI space. vxlan_vni_add_del() then
> loops over that span creating one VNI node and one per-CPU stats block
> per iteration, all under rtnl_lock.

The AI review looks legit.
Please target net-next without a fixes tag for this.
Please do _not_ send the next version in reply to a previous posting.

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

* Re: [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg
  2026-09-09  9:26   ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas
  2026-09-10  9:38     ` netdev-bot+sashiko
@ 2026-09-15  0:31     ` Jakub Kicinski
  1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-09-15  0:31 UTC (permalink / raw)
  To: Ali Firas
  Cc: netdev, idosch, pabeni, davem, edumazet, andrew+netdev, razor,
	roopa, linux-kernel

On Wed,  9 Sep 2026 12:26:44 +0300 Ali Firas wrote:
> -	vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu);
> +	vninode->stats = __netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu,
> +						   GFP_KERNEL_ACCOUNT);

You can make netdev_alloc_pcpu_stats() use _ACCOUNT, I reckon.
Again, no Fixes tag needed here, this is really not a serious issue.

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

end of thread, other threads:[~2026-09-15  0:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20260907141001.GA708129@shredder>
2026-09-09  9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas
2026-09-09  9:26   ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas
2026-09-10  9:38     ` netdev-bot+sashiko
2026-09-15  0:31     ` Jakub Kicinski
2026-09-09  9:26   ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas
2026-09-10  9:38     ` netdev-bot+sashiko
2026-09-15  0:31     ` Jakub Kicinski
2026-09-09  9:26   ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas
2026-09-10  9:39     ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®