* [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
* 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 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
* [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
* 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 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
* [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 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
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®