* [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory
@ 2026-09-16 19:34 Ali Firas
2026-09-16 19:34 ` [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Ali Firas @ 2026-09-16 19:34 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, linux-kernel, Ali Firas
The VNI filter interface accepts a START/END range with no bound on
either endpoint and no bound on the total a single message may ask for,
and the memory it allocates per VNI is not charged to the caller's
cgroup.
Patches 1 and 2 bound the request. The first range-validates both
endpoints against the 24-bit VNI space, which also removes a loop whose
counter is signed while the bound it is compared against is not. The
second caps the total number of VNIs one message may span at 4096,
summed across every VXLAN_VNIFILTER_ENTRY rather than per entry, since
a message may carry any number of entries. Neither bounds how many VNIs
a device may hold.
Patch 3 makes netdev_alloc_pcpu_stats() use GFP_KERNEL_ACCOUNT, as
suggested on v1; it affects 34 call sites in 25 files, all of which
already handle a NULL return. Patch 4 accounts the VNI node itself.
Patch 5 adds selftests for the new limits.
v2:
- target net-next, drop the Fixes tags, and post as a new thread, per
review of v1
- split the range validation out of the cap into its own patch
- cap the per-message total instead of the per-entry span
- account in netdev_alloc_pcpu_stats() rather than at the call site
- trim the changelogs
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
Ali Firas (5):
vxlan: vnifilter: reject VNIs outside the 24-bit space
vxlan: vnifilter: bound the number of VNIs one request may touch
net: account per-CPU netdev stats to memcg
vxlan: vnifilter: account the VNI node to memcg
selftests: net: test the vxlan vnifilter VNI limit
drivers/net/vxlan/vxlan_vnifilter.c | 103 ++++++++++++++++--
include/linux/netdevice.h | 2 +-
.../selftests/net/test_vxlan_vnifiltering.sh | 34 ++++++
3 files changed, 126 insertions(+), 13 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space
2026-09-16 19:34 [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory Ali Firas
@ 2026-09-16 19:34 ` Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 2/5] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ali Firas @ 2026-09-16 19:34 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, linux-kernel, Ali Firas
VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END are declared as
bare NLA_U32, so neither is bounded before vxlan_process_vni_filter()
hands them to vxlan_vni_add_del():
int v, err = 0;
...
for (v = start_vni; v <= end_vni; v++) {
v is int and end_vni is __u32, so the comparison is done unsigned. A
request carrying only START=0xffffffff gives vni_start == vni_end ==
0xffffffff, which looks like a single-VNI request. v is then -1,
"v <= end_vni" converts it to 0xffffffff and passes, v++ makes v 0, and
the loop walks the whole space upwards from there, allocating a VNI node
and a per-CPU stats block per iteration under rtnl_lock. Any range
ending at 0xffffffff behaves the same way, including narrow ones such as
START=0xfffff001 END=0xffffffff.
Separately, a VNI at or above VXLAN_N_VID is accepted and stored even
though the VXLAN header carries only 24 bits: vxlan_vni_field() shifts
without masking, so such an entry occupies its own rhashtable slot while
being truncated on the wire.
Range-validate both attributes against the 24-bit VNI space, which is
what vxlan_mdb.c already does for its own VNI attributes. The request is
then rejected during netlink policy validation, before
vxlan_process_vni_filter() is reached, and nothing is allocated. Make
the loop counter u32 while touching this: that is not what terminates
the loop, the policy check is, but it removes the signed overflow past
INT_MAX and matches the u32 vni parameter vxlan_vni_add() already takes.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
drivers/net/vxlan/vxlan_vnifilter.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index dd94085e0886..5aaaaeee8110 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -459,9 +459,15 @@ static int vxlan_vnifilter_dump(struct sk_buff *skb, struct netlink_callback *cb
return err;
}
+static const struct netlink_range_validation vni_filter_vni_range = {
+ .max = VXLAN_N_VID - 1,
+};
+
static const struct nla_policy vni_filter_entry_policy[VXLAN_VNIFILTER_ENTRY_MAX + 1] = {
- [VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 },
- [VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 },
+ [VXLAN_VNIFILTER_ENTRY_START] = NLA_POLICY_FULL_RANGE(NLA_U32,
+ &vni_filter_vni_range),
+ [VXLAN_VNIFILTER_ENTRY_END] = NLA_POLICY_FULL_RANGE(NLA_U32,
+ &vni_filter_vni_range),
[VXLAN_VNIFILTER_ENTRY_GROUP] = NLA_POLICY_EXACT_LEN(sizeof_field(struct iphdr, daddr)),
[VXLAN_VNIFILTER_ENTRY_GROUP6] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
};
@@ -814,7 +820,8 @@ static int vxlan_vni_add_del(struct vxlan_dev *vxlan, __u32 start_vni,
int cmd, struct netlink_ext_ack *extack)
{
struct vxlan_vni_group *vg;
- int v, err = 0;
+ int err = 0;
+ u32 v;
vg = rtnl_dereference(vxlan->vnigrp);
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 2/5] vxlan: vnifilter: bound the number of VNIs one request may touch
2026-09-16 19:34 [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory Ali Firas
2026-09-16 19:34 ` [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
@ 2026-09-16 19:34 ` Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 3/5] net: account per-CPU netdev stats to memcg Ali Firas
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ali Firas @ 2026-09-16 19:34 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, linux-kernel, Ali Firas
With both endpoints bounded to the 24-bit space, a single RTM_NEWTUNNEL
or RTM_DELTUNNEL message can still ask for all of it.
vxlan_vni_add_del() loops over the span creating one VNI node and one
per-CPU stats block per iteration, all under rtnl_lock.
The span of one VXLAN_VNIFILTER_ENTRY is not the quantity to bound.
vxlan_vnifilter_process() calls vxlan_process_vni_filter() once per
entry, vni_filter_policy places no limit on how many entries the nest
may carry, and an entry carrying just START and END is 20 bytes on the
wire, so bounding each entry on its own would still let one message ask
for thousands of times the bound. Sum the spans of every entry and
reject the message as a whole in vxlan_vnifilter_check_msg(), before the
dispatch loop rather than inside it: entries are applied and notified
one at a time, so a limit enforced during dispatch would return -EINVAL
only after every preceding entry had already created its VNIs and sent
its notifications.
The limit is 4096, which follows from how the interface is used on
bridged VXLAN devices where the VNI is derived from the VLAN and so
cannot exceed the usable VLAN ID space. It bounds one message, not how
many VNIs a device may hold: a device can still be populated with the
whole space, it just takes more than one message. It is a driver-local
constant rather than VLAN_N_VID because a bound on a VXLAN netlink
request is not a count of VLAN IDs. One asymmetry is deliberate:
vxlan_vnifilter_dump_dev() merges a contiguous run sharing a remote into
a single entry with no clamp, so a device populated by several accepted
requests can dump as one entry this check refuses on replay. Chunking
the dump would not remove that, since the same run split into capped
entries still exceeds the limit when they arrive in one message.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
drivers/net/vxlan/vxlan_vnifilter.c | 88 ++++++++++++++++++++++++++---
1 file changed, 80 insertions(+), 8 deletions(-)
diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 5aaaaeee8110..9a1baca39d8b 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -17,6 +17,15 @@
#include "vxlan_private.h"
+/* Maximum number of VNIs one RTM_NEWTUNNEL or RTM_DELTUNNEL message may add or
+ * delete, summed over all of its VXLAN_VNIFILTER_ENTRY attributes. VNI
+ * filtering is mainly used on bridged VXLAN devices where the VNI is derived
+ * from the VLAN, so a message touching more VNIs than the VLAN ID space has no
+ * practical use, while an unbounded message can walk the whole 24-bit space
+ * under rtnl_lock.
+ */
+#define VXLAN_VNI_FILTER_MSG_MAX 4096
+
static inline int vxlan_vni_cmp(struct rhashtable_compare_arg *arg,
const void *ptr)
{
@@ -846,12 +855,77 @@ static int vxlan_vni_add_del(struct vxlan_dev *vxlan, __u32 start_vni,
return err;
}
+/* Derive the VNI range one VXLAN_VNIFILTER_ENTRY selects. Shared so that the
+ * count taken by vxlan_vnifilter_check_msg() cannot drift from the range
+ * vxlan_process_vni_filter() then acts on.
+ */
+static void vxlan_vni_filter_entry_range(struct nlattr **vattrs, u32 *vni_start,
+ u32 *vni_end)
+{
+ *vni_start = 0;
+ *vni_end = 0;
+
+ if (vattrs[VXLAN_VNIFILTER_ENTRY_START]) {
+ *vni_start = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_START]);
+ *vni_end = *vni_start;
+ }
+
+ if (vattrs[VXLAN_VNIFILTER_ENTRY_END])
+ *vni_end = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_END]);
+}
+
+/* Reject a message asking for more than VXLAN_VNI_FILTER_MSG_MAX VNIs before
+ * any of its entries is acted on. Entries are applied one at a time and each
+ * one notifies as it goes, so a limit checked inside the dispatch loop would
+ * leave the entries ahead of the offending one already applied.
+ */
+static int vxlan_vnifilter_check_msg(const struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *vattrs[VXLAN_VNIFILTER_ENTRY_MAX + 1];
+ struct nlattr *attr;
+ u32 vnis = 0;
+ int err, rem;
+
+ nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh,
+ sizeof(struct tunnel_msg), rem) {
+ u32 vni_start, vni_end;
+
+ err = nla_parse_nested(vattrs, VXLAN_VNIFILTER_ENTRY_MAX, attr,
+ vni_filter_entry_policy, extack);
+ if (err)
+ return err;
+
+ vxlan_vni_filter_entry_range(vattrs, &vni_start, &vni_end);
+
+ /* A start above the end selects no VNI at all and costs
+ * nothing; leave it behaving as it does today.
+ */
+ if (vni_end < vni_start)
+ continue;
+
+ /* vni_filter_entry_policy has already bounded both endpoints
+ * to below VXLAN_N_VID, so one entry adds at most VXLAN_N_VID
+ * and vnis cannot wrap before the test below rejects it.
+ */
+ vnis += vni_end - vni_start + 1;
+ if (vnis > VXLAN_VNI_FILTER_MSG_MAX) {
+ NL_SET_ERR_MSG_ATTR_FMT(extack, attr,
+ "Request asks for more than %u VNIs",
+ VXLAN_VNI_FILTER_MSG_MAX);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int vxlan_process_vni_filter(struct vxlan_dev *vxlan,
struct nlattr *nlvnifilter,
int cmd, struct netlink_ext_ack *extack)
{
struct nlattr *vattrs[VXLAN_VNIFILTER_ENTRY_MAX + 1];
- u32 vni_start = 0, vni_end = 0;
+ u32 vni_start, vni_end;
union vxlan_addr group;
int err;
@@ -862,13 +936,7 @@ static int vxlan_process_vni_filter(struct vxlan_dev *vxlan,
if (err)
return err;
- if (vattrs[VXLAN_VNIFILTER_ENTRY_START]) {
- vni_start = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_START]);
- vni_end = vni_start;
- }
-
- if (vattrs[VXLAN_VNIFILTER_ENTRY_END])
- vni_end = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_END]);
+ vxlan_vni_filter_entry_range(vattrs, &vni_start, &vni_end);
if (!vni_start && !vni_end) {
NL_SET_ERR_MSG_ATTR(extack, nlvnifilter,
@@ -975,6 +1043,10 @@ static int vxlan_vnifilter_process(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!(vxlan->cfg.flags & VXLAN_F_VNIFILTER))
return -EOPNOTSUPP;
+ err = vxlan_vnifilter_check_msg(nlh, extack);
+ if (err)
+ return err;
+
nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh,
sizeof(*tmsg), rem) {
err = vxlan_process_vni_filter(vxlan, attr, nlh->nlmsg_type,
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 3/5] net: account per-CPU netdev stats to memcg
2026-09-16 19:34 [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory Ali Firas
2026-09-16 19:34 ` [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
2026-09-16 19:34 ` [PATCH net-next v2 2/5] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
@ 2026-09-16 19:34 ` Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 4/5] vxlan: vnifilter: account the VNI node " Ali Firas
2026-09-16 19:34 ` [PATCH net-next v2 5/5] selftests: net: test the vxlan vnifilter VNI limit Ali Firas
4 siblings, 1 reply; 11+ messages in thread
From: Ali Firas @ 2026-09-16 19:34 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, linux-kernel, Ali Firas
netdev_alloc_pcpu_stats() allocates a per-CPU stats block with plain
GFP_KERNEL, so it is not charged to the cgroup of the process that
created the object it belongs to. The netdev's own queue arrays, ethtool
state and NAPI config are already allocated with GFP_KERNEL_ACCOUNT in
alloc_netdev_mqs(), which leaves the stats block allocated alongside
them as the unaccounted part of the same device.
Make the macro use GFP_KERNEL_ACCOUNT. This affects 34 call sites in 25
files; all of them already test the return value and propagate -ENOMEM,
so none of them has to be excluded. __GFP_ACCOUNT only charges an
allocation made from a task in a non-root memcg, so callers that run at
boot or from a driver probe are unaffected in practice. The devm_ and
explicit-gfp forms of the macro are left alone.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
include/linux/netdevice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1f0710eef185..7c659012355d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3171,7 +3171,7 @@ static inline void dev_dstats_tx_dropped(struct net_device *dev)
})
#define netdev_alloc_pcpu_stats(type) \
- __netdev_alloc_pcpu_stats(type, GFP_KERNEL)
+ __netdev_alloc_pcpu_stats(type, GFP_KERNEL_ACCOUNT)
#define devm_netdev_alloc_pcpu_stats(dev, type) \
({ \
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 4/5] vxlan: vnifilter: account the VNI node to memcg
2026-09-16 19:34 [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory Ali Firas
` (2 preceding siblings ...)
2026-09-16 19:34 ` [PATCH net-next v2 3/5] net: account per-CPU netdev stats to memcg Ali Firas
@ 2026-09-16 19:34 ` Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 5/5] selftests: net: test the vxlan vnifilter VNI limit Ali Firas
4 siblings, 1 reply; 11+ messages in thread
From: Ali Firas @ 2026-09-16 19:34 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, linux-kernel, Ali Firas
vxlan_vni_alloc() allocates a struct vxlan_vni_node for every VNI with
plain GFP_KERNEL. The per-CPU stats block it allocates next is now
charged to the caller's cgroup, but the node itself is not.
Use GFP_KERNEL_ACCOUNT. The only caller already handles a NULL return.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
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 9a1baca39d8b..fbff52c450c0 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -710,7 +710,7 @@ 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);
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 5/5] selftests: net: test the vxlan vnifilter VNI limit
2026-09-16 19:34 [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory Ali Firas
` (3 preceding siblings ...)
2026-09-16 19:34 ` [PATCH net-next v2 4/5] vxlan: vnifilter: account the VNI node " Ali Firas
@ 2026-09-16 19:34 ` Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
4 siblings, 1 reply; 11+ messages in thread
From: Ali Firas @ 2026-09-16 19:34 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, linux-kernel, Ali Firas
Extend the vnifilter API test with the largest accepted range and one
VNI more rejected, for both add and delete, and with the 24-bit boundary
the first patch enforces.
The oversized delete is written so that it can only fail on the limit.
If it covered VNIs that were never installed, a kernel without the limit
would reach vxlan_vni_del(), fail with -ENOENT on the first missing VNI,
and iproute2 would map that to the same exit status the test expects, so
the case would pass while the limit was gone. Installing the range first
makes every VNI of the oversized delete exist, leaving the limit as the
only reason for it to fail.
bridge(8) sends one VXLAN_VNIFILTER_ENTRY per message, so these cases
exercise the single-entry path only; the per-message total across
several entries is not reachable from iproute2.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
.../selftests/net/test_vxlan_vnifiltering.sh | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
index 8deacc565afa..7cd4acc76ed6 100755
--- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
+++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
@@ -371,6 +371,40 @@ 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 touch at most 4096 vnis in total. bridge(8)
+ # sends one range per message, so these cover the one-entry case; the
+ # total across several entries of one message is not reachable from
+ # iproute2.
+ 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"
+
+ # install the one vni past that range as well, so that the oversized
+ # delete below can only fail on the limit and not on a missing vni
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 14096"
+ log_test $? 0 "Add the vni past the maximum range"
+
+ 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"
+
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 14096"
+ log_test $? 0 "Delete the vni past the maximum range"
+
+ # the vxlan header carries 24 bits, so a vni above that is rejected
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777215"
+ log_test $? 0 "Add the highest vni the vxlan header can carry"
+
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 16777215"
+ log_test $? 0 "Delete the highest vni the vxlan header can carry"
+
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777216"
+ log_test $? 255 "Cannot add a vni the vxlan header cannot carry"
}
# Sanity test vnifilter datapath
--
2.53.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space
2026-09-16 19:34 ` [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
@ 2026-09-19 0:21 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 0:21 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 2 (1) · Low: 0
- [Medium] Missing `Fixes:` tag.
- [Medium] Commit message inaccuracy: the message claims the offending
request "is then rejected during netlink policy validation, before…
Pre-existing issues:
- [Medium] Residual per-request allocation storm under rtnl_lock after
this patch alone: the new policy caps START/END at VXLAN_N_VID-1,…
--
vxlan: vnifilter: reject VNIs outside the 24-bit space
The patch adds a netlink_range_validation bound of VXLAN_N_VID - 1 to the
VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END attributes in
vni_filter_entry_policy, and changes the vxlan_vni_add_del() loop counter
from int to u32.
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
A few questions on the commit message and on the remaining behaviour of
the loop.
> Assisted-by: LLM
> Signed-off-by: Ali Firas <alishmery18@gmail.com>
[Severity: Medium]
Should this carry a Fixes: tag? The bug being closed is reachable from a
plain RTM_NEWTUNNEL/RTM_DELTUNNEL message and makes vxlan_vni_add_del()
walk most of the 32-bit space allocating a struct vxlan_vni_node plus a
per-CPU stats block per iteration under rtnl_lock, so stable trees would
want it.
Both the "int v" loop and the unbounded NLA_U32 START/END policy arrived
with the file itself:
Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Without that tag, backport tooling has nothing to key off.
> Range-validate both attributes against the 24-bit VNI space, which is
> what vxlan_mdb.c already does for its own VNI attributes. The request is
> then rejected during netlink policy validation, before
> vxlan_process_vni_filter() is reached, and nothing is allocated.
[Severity: Medium]
Is "before vxlan_process_vni_filter() is reached, and nothing is
allocated" accurate for a multi-entry message?
The new range bound is only enforced by the nla_parse_nested() call
inside vxlan_process_vni_filter():
drivers/net/vxlan/vxlan_vnifilter.c:vxlan_process_vni_filter() {
err = nla_parse_nested(vattrs,
VXLAN_VNIFILTER_ENTRY_MAX,
nlvnifilter, vni_filter_entry_policy,
extack);
...
}
The top-level nlmsg_parse() in vxlan_vnifilter_process() uses
vni_filter_policy, where the nest has no nested_policy pointer, and
lib/nlattr.c only recurses when pt->nested_policy is set:
static const struct nla_policy vni_filter_policy[VXLAN_VNIFILTER_MAX + 1] = {
[VXLAN_VNIFILTER_ENTRY] = { .type = NLA_NESTED },
};
So for a message like [ENTRY: START=1 END=100][ENTRY: START=0xffffffff],
does the first entry get fully applied (100 vxlan_vni_node plus per-CPU
stats allocations, rhashtable inserts and RTM_NEWTUNNEL notifications)
before the second one returns -ERANGE, with vxlan_vnifilter_process()
merely breaking out of nlmsg_for_each_attr_type() and leaving that state
installed?
"Nothing is allocated" appears to hold only for a single-entry request.
Noting for the series: the later patch "vxlan: vnifilter: bound the number
of VNIs one request may touch" adds vxlan_vnifilter_check_msg(), which
parses every entry against vni_filter_entry_policy before the dispatch
loop, so the partial-application behaviour is gone by the end of the
series. The wording in this patch's own message, and therefore in a
standalone stable backport of just this patch, still describes something
else.
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index dd94085e08865..5aaaaeee81107 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -459,9 +459,15 @@ static int vxlan_vnifilter_dump(struct sk_buff *skb, struct netlink_callback *cb
> return err;
> }
>
> +static const struct netlink_range_validation vni_filter_vni_range = {
> + .max = VXLAN_N_VID - 1,
> +};
> +
> static const struct nla_policy vni_filter_entry_policy[VXLAN_VNIFILTER_ENTRY_MAX + 1] = {
> - [VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 },
> - [VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 },
> + [VXLAN_VNIFILTER_ENTRY_START] = NLA_POLICY_FULL_RANGE(NLA_U32,
> + &vni_filter_vni_range),
> + [VXLAN_VNIFILTER_ENTRY_END] = NLA_POLICY_FULL_RANGE(NLA_U32,
> + &vni_filter_vni_range),
[Severity: Medium]
This isn't a bug introduced by this patch, but the per-request allocation
behaviour described in the commit message survives it in a narrower form,
since the policy bounds each endpoint and not the distance between them.
vxlan_process_vni_filter() leaves vni_start at 0 when only END is
present:
if (vattrs[VXLAN_VNIFILTER_ENTRY_START]) {
vni_start = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_START]);
vni_end = vni_start;
}
if (vattrs[VXLAN_VNIFILTER_ENTRY_END])
vni_end = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_END]);
So a single entry carrying only END=0xffffff passes the new policy and
still drives roughly 2^24 iterations of vxlan_vni_add_del() ->
vxlan_vni_add() -> vxlan_vni_alloc(), each doing a GFP_KERNEL
kzalloc_obj() of struct vxlan_vni_node plus netdev_alloc_pcpu_stats() over
nr_cpu_ids, an rhashtable insert and an RTM_NEWTUNNEL notification skb.
There is no cond_resched() and no fatal-signal check in that loop, and the
handlers are registered without RTNL_FLAG_DOIT_UNLOCKED, so rtnl_lock is
held throughout. Is that span still intended to be unbounded here?
Also, when an allocation fails part way through the range the loop takes
the goto out path and the nodes already inserted into vg->vni_hash and
vg->vni_list stay behind. That predates this patch as well.
The follow-on patch "vxlan: vnifilter: bound the number of VNIs one
request may touch" caps the summed span at VXLAN_VNI_FILTER_MSG_MAX before
the dispatch loop, and the memcg accounting patches later in the series
change both allocations to GFP_KERNEL_ACCOUNT, so this is addressed within
the series.
> [VXLAN_VNIFILTER_ENTRY_GROUP] = NLA_POLICY_EXACT_LEN(sizeof_field(struct iphdr, daddr)),
> [VXLAN_VNIFILTER_ENTRY_GROUP6] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
> };
> @@ -814,7 +820,8 @@ static int vxlan_vni_add_del(struct vxlan_dev *vxlan, __u32 start_vni,
> int cmd, struct netlink_ext_ack *extack)
> {
> struct vxlan_vni_group *vg;
> - int v, err = 0;
> + int err = 0;
> + u32 v;
>
> vg = rtnl_dereference(vxlan->vnigrp);
>
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916193449.2552039-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/5] vxlan: vnifilter: bound the number of VNIs one request may touch
2026-09-16 19:34 ` [PATCH net-next v2 2/5] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
@ 2026-09-19 0:21 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 0:21 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 1
- [Medium] The kernel's own dump output is no longer accepted by the
kernel's own input path.
- [Low] Ambiguous framing: the changelog describes a concrete kernel
defect — one netlink message making vxlan_vni_add_del()…
--
vxlan: vnifilter: bound the number of VNIs one request may touch
The patch adds vxlan_vnifilter_check_msg(), which sums the START/END span
of every VXLAN_VNIFILTER_ENTRY in an RTM_NEWTUNNEL or RTM_DELTUNNEL
message and rejects the message with -EINVAL if the total exceeds a new
driver-local limit of 4096. The range extraction used by
vxlan_process_vni_filter() is factored out into
vxlan_vni_filter_entry_range() so both sides compute the same range.
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
Two questions, one about the changelog and one about the new check.
> vxlan: vnifilter: bound the number of VNIs one request may touch
>
> With both endpoints bounded to the 24-bit space, a single RTM_NEWTUNNEL
> or RTM_DELTUNNEL message can still ask for all of it.
> vxlan_vni_add_del() loops over the span creating one VNI node and one
> per-CPU stats block per iteration, all under rtnl_lock.
[Severity: Low]
This part of the changelog reads like the description of a user triggerable
defect: one netlink message makes vxlan_vni_add_del() iterate over up to
the whole 24-bit space, allocating a vxlan_vni_node plus a per-CPU stats
block per iteration while holding rtnl_lock.
There is no Fixes: tag naming the commit that introduced that loop
(f9c4bb0b245c "vxlan: vni filtering support on collect metadata device"),
and no statement that this is a policy/hardening change that should not be
backported.
Since the patch also narrows what userspace may send in one message, could
the changelog say which of the two it is, so stable triage does not have to
guess?
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 5aaaaeee81107..9a1baca39d8b0 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -17,6 +17,15 @@
>
> #include "vxlan_private.h"
>
> +/* Maximum number of VNIs one RTM_NEWTUNNEL or RTM_DELTUNNEL message may add or
> + * delete, summed over all of its VXLAN_VNIFILTER_ENTRY attributes. VNI
> + * filtering is mainly used on bridged VXLAN devices where the VNI is derived
> + * from the VLAN, so a message touching more VNIs than the VLAN ID space has no
> + * practical use, while an unbounded message can walk the whole 24-bit space
> + * under rtnl_lock.
> + */
> +#define VXLAN_VNI_FILTER_MSG_MAX 4096
> +
[ ... ]
> @@ -846,12 +855,77 @@ static int vxlan_vni_add_del(struct vxlan_dev *vxlan, __u32 start_vni,
[ ... ]
> +static int vxlan_vnifilter_check_msg(const struct nlmsghdr *nlh,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *vattrs[VXLAN_VNIFILTER_ENTRY_MAX + 1];
> + struct nlattr *attr;
> + u32 vnis = 0;
> + int err, rem;
> +
> + nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh,
> + sizeof(struct tunnel_msg), rem) {
[ ... ]
> + /* vni_filter_entry_policy has already bounded both endpoints
> + * to below VXLAN_N_VID, so one entry adds at most VXLAN_N_VID
> + * and vnis cannot wrap before the test below rejects it.
> + */
> + vnis += vni_end - vni_start + 1;
> + if (vnis > VXLAN_VNI_FILTER_MSG_MAX) {
> + NL_SET_ERR_MSG_ATTR_FMT(extack, attr,
> + "Request asks for more than %u VNIs",
> + VXLAN_VNI_FILTER_MSG_MAX);
> + return -EINVAL;
> + }
> + }
[Severity: Medium]
Does this make the driver emit a VXLAN_VNIFILTER_ENTRY that it will then
refuse to parse back?
The changelog states that a device may still hold the whole 24-bit space as
long as it is populated by several messages. On dump,
vxlan_vnifilter_dump_dev() merges every contiguous run sharing a remote
into one entry with no clamp:
if (!dump_stats && vnirange(vend, v) == 1 &&
vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) {
goto update_end;
and vxlan_fill_vni_filter_entry() serializes that whole run into the same
START/END pair that update requests use:
if (nla_put_u32(skb, VXLAN_VNIFILTER_ENTRY_START, vs))
goto out_err;
if (ve && nla_put_u32(skb, VXLAN_VNIFILTER_ENTRY_END, ve))
goto out_err;
So a device populated with more than 4096 contiguous VNIs sharing a remote
dumps as a single entry whose span vxlan_vnifilter_check_msg() rejects with
-EINVAL on replay. Should the dump side be clamped to the same constant so
the two representations agree?
A second question on the same check: it does not look at nlh->nlmsg_type,
so RTM_DELTUNNEL is bounded as well. vxlan_vni_del() only looks the VNI up
and frees it:
vninode = rhashtable_lookup_fast(&vg->vni_hash, &v,
vxlan_vni_rht_params);
if (!vninode) {
err = -ENOENT;
Since the delete path allocates nothing, is bounding RTM_DELTUNNEL
intended? As written, a device holding more than 4096 VNIs can no longer
be torn down with the single range the dump reported.
> +
> + return 0;
> +}
> +
[ ... ]
> @@ -975,6 +1043,10 @@ static int vxlan_vnifilter_process(struct sk_buff *skb, struct nlmsghdr *nlh,
> if (!(vxlan->cfg.flags & VXLAN_F_VNIFILTER))
> return -EOPNOTSUPP;
>
> + err = vxlan_vnifilter_check_msg(nlh, extack);
> + if (err)
> + return err;
> +
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916193449.2552039-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 3/5] net: account per-CPU netdev stats to memcg
2026-09-16 19:34 ` [PATCH net-next v2 3/5] net: account per-CPU netdev stats to memcg Ali Firas
@ 2026-09-19 0:21 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 0:21 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)
- [Medium] The accounting change only covers the plain
`netdev_alloc_pcpu_stats()` form, so per-CPU stats belonging to one…
Pre-existing issues:
- [Low] Concern that the change is a memcg-isolation bug fix and
therefore lacks a required `Fixes:` tag identifying the commit that…
--
net: account per-CPU netdev stats to memcg
This changes the plain netdev_alloc_pcpu_stats() macro to allocate with
GFP_KERNEL_ACCOUNT instead of GFP_KERNEL, so the per-CPU stats block is
charged to the memcg of the task that created the object. The devm_ and
explicit-gfp variants of the macro are unchanged.
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 1f0710eef185b..7c659012355dd 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3171,7 +3171,7 @@ static inline void dev_dstats_tx_dropped(struct net_device *dev)
> })
>
> #define netdev_alloc_pcpu_stats(type) \
> - __netdev_alloc_pcpu_stats(type, GFP_KERNEL)
> + __netdev_alloc_pcpu_stats(type, GFP_KERNEL_ACCOUNT)
[Severity: Medium]
Does this move the accounting asymmetry rather than remove it? Several
callers allocate more than one per-CPU stats block for the same object,
and only the ones going through this macro become charged.
net/sched/sch_generic.c:qdisc_alloc() does:
sch->cpu_bstats =
netdev_alloc_pcpu_stats(struct gnet_stats_basic_sync);
if (!sch->cpu_bstats)
goto errout1;
sch->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
so cpu_bstats is now charged while cpu_qstats stays uncharged for the same
qdisc created by tc qdisc add.
net/sched/act_api.c:tcf_idr_create() has the same split:
p->cpu_bstats_hw = netdev_alloc_pcpu_stats(struct gnet_stats_basic_sync);
if (!p->cpu_bstats_hw)
goto err2;
p->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
Two of the three per-CPU blocks of one tc action get charged, the third
does not.
The commit message says:
The devm_ and explicit-gfp forms of the macro are left alone.
but it does not mention these adjacent alloc_percpu() companions inside the
same functions. Were they deliberately left out, or just missed?
Related, the same net_device field ends up charged or not depending on which
macro a driver picks: dev->tstats is charged when it comes from
netdev_do_alloc_pcpu_stats() in net/core/dev.c, and uncharged when a driver
uses devm_netdev_alloc_pcpu_stats(), as in liteeth_probe()
(drivers/net/ethernet/litex/litex_liteeth.c), stmmac_dvr_probe()
(drivers/net/ethernet/stmicro/stmmac/stmmac_main.c) and
ax88796c_probe() (drivers/net/ethernet/asix/ax88796c_main.c).
And the user-triggered netlink path in net/ipv6/seg6_local.c still builds
per-CPU SRv6 counters with plain GFP_KERNEL:
#define seg6_local_alloc_pcpu_counters(__gfp) \
__netdev_alloc_pcpu_stats(struct pcpu_seg6_local_counters, \
((__gfp) | __GFP_ZERO))
...
pcounters = seg6_local_alloc_pcpu_counters(GFP_KERNEL);
reached from parse_nla_counters() during route parsing. Should that wrapper
add __GFP_ACCOUNT too, or is leaving it out intentional?
[Severity: Low]
This isn't a bug introduced by this patch, but the missing accounting in
netdev_alloc_pcpu_stats() is a pre-existing condition, and one review pass
asked whether a Fixes: tag pointing at 3c7be18ac9a0 ("mm: memcg/percpu:
account percpu memory to memory cgroups") is needed here.
Checking the closest precedent argues against it: commit c948f51c1654
("memcg: enable accounting for net_device and Tx/Rx queues"), which
introduced the GFP_KERNEL_ACCOUNT usage in alloc_netdev_mqs() and
netif_alloc_rx_queues() that this patch cites as its parity target, carries
no Fixes: tag and went in through net-next as an enhancement. 3c7be18ac9a0
also never claimed to charge allocations that omit __GFP_ACCOUNT, so it does
not look like a valid Fixes: target.
Is treating this as an enhancement without a Fixes: tag the intent?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916193449.2552039-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 4/5] vxlan: vnifilter: account the VNI node to memcg
2026-09-16 19:34 ` [PATCH net-next v2 4/5] vxlan: vnifilter: account the VNI node " Ali Firas
@ 2026-09-19 0:21 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 0:21 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 0 · Low: 1
- [Low] Missing Fixes: tag — the concern asserts that because the patch
corrects observable memory-controller behavior for a long-lived per-…
--
vxlan: vnifilter: account the VNI node to memcg
This switches the struct vxlan_vni_node allocation in vxlan_vni_alloc()
from plain GFP_KERNEL to GFP_KERNEL_ACCOUNT, so the per-VNI node is
charged to the caller's cgroup like the per-CPU stats block that follows
it.
Link: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 9a1baca39d8b0..fbff52c450c07 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -710,7 +710,7 @@ 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);
[Severity: Low]
This isn't a bug, but one review pass suggested this change in
vxlan_vni_alloc() should carry:
Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
on the grounds that it changes observable memory-controller behaviour for a
long-lived per-VNI allocation.
Is a Fixes tag wanted here? The comparable precedent, commit c948f51c1654
("memcg: enable accounting for net_device and Tx/Rx queues"), added
GFP_KERNEL_ACCOUNT to the netdev allocations this builds on and carries no
Fixes tag, and the sibling patch accounting the per-CPU netdev stats in this
series does not carry one either.
There is also a behavioural side to it: with GFP_KERNEL_ACCOUNT, VNI
creation that previously succeeded can now return -ENOMEM once the calling
cgroup hits its limit, which may not be desirable to backport to stable.
> if (!vninode)
> return NULL;
> vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu);
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916193449.2552039-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 5/5] selftests: net: test the vxlan vnifilter VNI limit
2026-09-16 19:34 ` [PATCH net-next v2 5/5] selftests: net: test the vxlan vnifilter VNI limit Ali Firas
@ 2026-09-19 0:21 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 0:21 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, linux-kernel
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:
Critical: 0 · High: 1 · Medium: 1 · Low: 1 (1)
- [High] The two sides of the vnifilter netlink object model now
disagree: `vxlan_vnifilter_dump_dev()`…
- [Medium] The new "24-bit boundary" cases exercise
VXLAN_VNIFILTER_ENTRY_START only.
- [Low] The oversized-add assertion at
tools/testing/selftests/net/test_vxlan_vnifiltering.sh:382-383 only…
Pre-existing issues:
- [Low] vxlan_vnifilter_api() creates veth-host and $testns via
setup_vnifilter_api() (test_vxlan_vnifiltering.sh:299-304) but never…
--
selftests: net: test the vxlan vnifilter VNI limit
This extends vxlan_vnifilter_api() in test_vxlan_vnifiltering.sh with cases
for the largest accepted VNI range, one VNI past it (for add and delete),
and the 24-bit VNI boundary. All new cases go through bridge(8) and check
only the command exit status.
v1: https://lore.kernel.org/netdev/20260909092645.3105263-1-alishmery18@gmail.com/
> diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> index 8deacc565afa1..7cd4acc76ed6c 100755
> --- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> +++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> @@ -371,6 +371,40 @@ 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 touch at most 4096 vnis in total. bridge(8)
> + # sends one range per message, so these cover the one-entry case; the
> + # total across several entries of one message is not reachable from
> + # iproute2.
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14095"
> + log_test $? 0 "Add vni range of maximum size"
[Severity: High]
Does this pair of commands leave the device in a state the kernel can dump
but no longer accept back?
vxlan_vnifilter_dump_dev() in drivers/net/vxlan/vxlan_vnifilter.c coalesces
any contiguous run of VNIs sharing a remote into one
VXLAN_VNIFILTER_ENTRY, with no upper bound on the span:
if (!dump_stats && vnirange(vend, v) == 1 &&
vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) {
goto update_end;
while vxlan_vnifilter_check_msg() rejects an RTM_NEWTUNNEL whose entries sum
to more than VXLAN_VNI_FILTER_MSG_MAX:
if (vnis > VXLAN_VNI_FILTER_MSG_MAX) {
NL_SET_ERR_MSG_ATTR_FMT(extack, attr,
"Request asks for more than %u VNIs",
VXLAN_VNI_FILTER_MSG_MAX);
return -EINVAL;
This test installs 10000-14095 and then 14096 (both with the default
all-zero remote), which is exactly a 4097-VNI contiguous run. On
RTM_GETTUNNEL the dump then emits a single entry START=10000 END=14096, and
replaying that one entry in a single RTM_NEWTUNNEL now fails with -EINVAL.
Would a save/restore or dump/replay consumer of bridge vni show break on
that?
Separately, is the limit itself an rtnetlink narrowing? A single request
such as bridge vni add dev X vni 1-10000 was accepted before and now
returns -EINVAL.
> +
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14096"
> + log_test $? 255 "Cannot add vni range larger than maximum"
> +
> + # install the one vni past that range as well, so that the oversized
> + # delete below can only fail on the limit and not on a missing vni
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 14096"
> + log_test $? 0 "Add the vni past the maximum range"
[Severity: Low]
Can this follow-up command distinguish "the rejected message installed
nothing" from "the rejected message installed part of the range"?
vxlan_vni_add() in drivers/net/vxlan/vxlan_vnifilter.c short-circuits an
already present VNI into the update path:
if (vxlan_vnifilter_lookup(vxlan, v))
return vxlan_vni_update(vxlan, vg, v, group, &changed, extack);
so vni add 14096 returns 0 whether or not 14096 was already created by the
rejected 10000-14096 request, and the later vni del 10000-14095 returns 0
either way.
The comment above vxlan_vnifilter_check_msg() states the message is
rejected "before any of its entries is acted on", which is the property this
block does not observe. Would checking that 14096 is absent first (a bridge
vni show grep, or requiring vni del 14096 to fail) before installing it make
the sequence detect a limit that moved into the per-entry dispatch loop?
> +
> + 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"
> +
> + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 14096"
> + log_test $? 0 "Delete the vni past the maximum range"
[Severity: Low]
This isn't a bug introduced by this patch, but vxlan_vnifilter_api() has no
teardown on exit, and the new block adds up to 4097 entries that depend on
these del commands succeeding.
setup_vnifilter_api() creates veth-host and $testns, while the generic
cleanup() installed by trap cleanup EXIT removes only veth-hv-1, the
vethhv-* pair and the hv_*/vm_* namespaces:
cleanup() {
ip link del veth-hv-1 2>/dev/null || true
ip link del vethhv-11 vethhv-12 vethhv-21 vethhv-22 2>/dev/null || true
cleanup_ns $hv_1 $hv_2 $vm_11 $vm_21 $vm_12 $vm_22 $vm_31 $vm_32
}
cleanup_vnifilter_api() does remove them, but it is only called at the start
of vxlan_vnifilter_api(), so it cleans up the previous run. If one of the
new commands returns an unexpected status on a kernel without the limit (the
oversized add succeeding, or the final vni add 16777216 succeeding), the
entries survive together with $testns and the veth pair. Could
cleanup_vnifilter_api() be called at the end of the test, or registered in
the trap?
> +
> + # the vxlan header carries 24 bits, so a vni above that is rejected
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777215"
> + log_test $? 0 "Add the highest vni the vxlan header can carry"
> +
> + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 16777215"
> + log_test $? 0 "Delete the highest vni the vxlan header can carry"
> +
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777216"
> + log_test $? 255 "Cannot add a vni the vxlan header cannot carry"
[Severity: Medium]
Do these three cases reach VXLAN_VNIFILTER_ENTRY_END at all?
They are all single-VNI commands, so the message carries START only. The
kernel change bounds both attributes:
[VXLAN_VNIFILTER_ENTRY_START] = NLA_POLICY_FULL_RANGE(NLA_U32,
&vni_filter_vni_range),
[VXLAN_VNIFILTER_ENTRY_END] = NLA_POLICY_FULL_RANGE(NLA_U32,
&vni_filter_vni_range),
but the unbounded walk in vxlan_vni_add_del() under rtnl_lock is reached
through END, for example bridge vni add dev X vni 100-4294967295 (START=100,
END=0xffffffff).
No case in the file drives END past VXLAN_N_VID - 1, so the whole new block
still passes if the range check on VXLAN_VNIFILTER_ENTRY_END is dropped.
Would adding something like vni 16777215-16777216, or vni 100-4294967295,
cover the vector that motivated the change?
> }
>
> # Sanity test vnifilter datapath
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916193449.2552039-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-19 0:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 19:34 [PATCH net-next v2 0/5] vxlan: vnifilter: bound one request and account per-VNI memory Ali Firas
2026-09-16 19:34 ` [PATCH net-next v2 1/5] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 2/5] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 3/5] net: account per-CPU netdev stats to memcg Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 4/5] vxlan: vnifilter: account the VNI node " Ali Firas
2026-09-19 0:21 ` netdev-bot+sashiko
2026-09-16 19:34 ` [PATCH net-next v2 5/5] selftests: net: test the vxlan vnifilter VNI limit Ali Firas
2026-09-19 0:21 ` 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®