* [PATCH net-next v3 1/6] vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest
2026-09-27 21:52 [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory Ali Firas
@ 2026-09-27 21:52 ` Ali Firas
2026-09-30 3:52 ` netdev-bot+sashiko
2026-09-27 21:52 ` [PATCH net-next v3 2/6] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ali Firas @ 2026-09-27 21:52 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, shuah, linux-kselftest, linux-kernel, Ali Firas
vxlan_vnifilter_process() parses the message with vni_filter_policy,
whose VXLAN_VNIFILTER_ENTRY is a bare NLA_NESTED with no nested policy
attached. The entry attributes are therefore validated only later, one
entry at a time, by the nla_parse_nested() inside
vxlan_process_vni_filter(). Entries are parsed as they are dispatched:
in a message whose first entry is valid and whose second is not, the
first entry is applied and its RTM_NEWTUNNEL notification sent before
the second is rejected.
Link the nest to vni_filter_entry_policy with NLA_POLICY_NESTED() so the
whole message is validated up front, before any entry is acted on. A
message carrying an invalid entry is now rejected as a unit and installs
nothing.
This reorders two faults. The nest is validated before the device is
looked up and before the vnifilter flag is checked, so a request
rejected by the entry policy, aimed at a missing or non-vnifilter
device, now returns that policy error where it previously returned
-ENODEV or -EOPNOTSUPP. The request was invalid either way; only the
errno an operator sees changes.
The nla_parse_nested() in vxlan_process_vni_filter() stays: it is what
fills the per-entry attribute table the handler reads. It can no longer
fail on a message that has reached it.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/netdev/20260921150904.65a704eb@kernel.org/
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
Notes:
v3: new patch. Link the nest with NLA_POLICY_NESTED(), as Jakub asked.
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 dd94085e0886..d391ec579661 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -467,7 +467,7 @@ static const struct nla_policy vni_filter_entry_policy[VXLAN_VNIFILTER_ENTRY_MAX
};
static const struct nla_policy vni_filter_policy[VXLAN_VNIFILTER_MAX + 1] = {
- [VXLAN_VNIFILTER_ENTRY] = { .type = NLA_NESTED },
+ [VXLAN_VNIFILTER_ENTRY] = NLA_POLICY_NESTED(vni_filter_entry_policy),
};
static int vxlan_update_default_fdb_entry(struct vxlan_dev *vxlan, __be32 vni,
--
2.53.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next v3 1/6] vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest
2026-09-27 21:52 ` [PATCH net-next v3 1/6] vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest Ali Firas
@ 2026-09-30 3:52 ` netdev-bot+sashiko
0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 3:52 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, shuah, linux-kselftest, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The commit message says more than the patch does.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, and later patches
in the series fix it (baseline decision: discard).
--
vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest
This patch links the VXLAN_VNIFILTER_ENTRY nest in vni_filter_policy to
vni_filter_entry_policy with NLA_POLICY_NESTED(). The entry attributes are
then checked by nlmsg_parse() in vxlan_vnifilter_process() before any entry
is dispatched. The goal is to reject a message that carries an invalid
entry as a whole.
Link: https://lore.kernel.org/netdev/20260921150904.65a704eb@kernel.org/
> Link the nest to vni_filter_entry_policy with NLA_POLICY_NESTED() so the
> whole message is validated up front, before any entry is acted on. A
> message carrying an invalid entry is now rejected as a unit and installs
> nothing.
[Severity: High]
Is this claim accurate? NLA_POLICY_NESTED() moves only the attribute
policy checks (type, length, unknown attributes) ahead of dispatch. Two
other checks that make an entry invalid still run one entry at a time in
vxlan_process_vni_filter():
drivers/net/vxlan/vxlan_vnifilter.c:vxlan_process_vni_filter() {
...
if (!vni_start && !vni_end) {
NL_SET_ERR_MSG_ATTR(extack, nlvnifilter,
"vni start nor end found in vni entry");
return -EINVAL;
}
...
if (vxlan_addr_multicast(&group) && !vxlan->default_dst.remote_ifindex) {
NL_SET_ERR_MSG(extack,
"Local interface required for multicast remote group");
return -EINVAL;
}
...
}
An empty VXLAN_VNIFILTER_ENTRY nest gets through the new policy, because
validate_nla() returns before it applies the nested policy:
lib/nlattr.c:validate_nla() {
...
case NLA_NESTED:
/* a nested attributes is allowed to be empty; if its not,
* it must have a size of at least NLA_HDRLEN.
*/
if (attrlen == 0)
break;
...
}
An entry with only START=0, or with only GROUP, also gets through.
vxlan_vnifilter_process() applies entries in order, stops at the first
error and does not roll anything back:
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;
}
Take an RTM_NEWTUNNEL to a vnifilter device with entry1 {START=100} and
entry2 {}. Entry2 could also be {START=0}, or {START=200, GROUP=239.1.1.1}
on a device with no remote_ifindex. That message passes nlmsg_parse().
Then:
vxlan_vnifilter_process()
vxlan_process_vni_filter(entry1)
vxlan_vni_add_del()
vxlan_vni_add()
inserts VNI 100, vxlan_vnifilter_notify(..., RTM_NEWTUNNEL)
vxlan_process_vni_filter(entry2)
returns -EINVAL
RTM_DELTUNNEL works the same way: earlier deletes are applied and notified.
Isn't this the same "first entry valid, second invalid" case the commit
message says is now fixed?
Later in the series, the vxlan_vnifilter_check_msg() pre-pass from "vxlan:
vnifilter: bound the number of VNIs one request may touch" counts an empty
entry as one VNI. It does neither of these checks, so the case can still
happen at the end of the series.
Should the commit message say that only policy-level invalid entries are
now all-or-nothing? The alternative is to run the START/END presence check
and the multicast/remote_ifindex check over every entry before any entry is
applied.
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index dd94085e08865..d391ec5796610 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -467,7 +467,7 @@ static const struct nla_policy vni_filter_entry_policy[VXLAN_VNIFILTER_ENTRY_MAX
> };
>
> static const struct nla_policy vni_filter_policy[VXLAN_VNIFILTER_MAX + 1] = {
> - [VXLAN_VNIFILTER_ENTRY] = { .type = NLA_NESTED },
> + [VXLAN_VNIFILTER_ENTRY] = NLA_POLICY_NESTED(vni_filter_entry_policy),
> };
[Severity: High]
This isn't a bug introduced by this patch, but at this commit the entry
policy that is now linked still has no range check on START and END:
[VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 },
[VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 },
vxlan_process_vni_filter() hands any values other than both-zero to
vxlan_vni_add_del(). That function loops with a signed counter:
int v, err = 0;
...
for (v = start_vni; v <= end_vni; v++) {
With START=0xffffffff and no END, v starts at -1. Compared with the __u32
end_vni it becomes 0xffffffff, and v++ then climbs up from 0.
Can this loop run for a very long time under RTNL? Each iteration allocates
a vxlan_vni_node with per-CPU stats and sends a notification. The loop only
stops on an error.
VNIs >= VXLAN_N_VID are also accepted and stored, so they alias on the
wire.
Later patches in this series fix this. "vxlan: vnifilter: reject VNIs
outside the 24-bit space" adds NLA_POLICY_FULL_RANGE(NLA_U32,
&vni_filter_vni_range) to START and END, and makes the loop counter u32.
"vxlan: vnifilter: bound the number of VNIs one request may touch" caps the
total span at VXLAN_VNI_FILTER_MSG_MAX before dispatch.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927215209.2581830-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v3 2/6] vxlan: vnifilter: reject VNIs outside the 24-bit space
2026-09-27 21:52 [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory Ali Firas
2026-09-27 21:52 ` [PATCH net-next v3 1/6] vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest Ali Firas
@ 2026-09-27 21:52 ` Ali Firas
2026-09-30 3:52 ` netdev-bot+sashiko
2026-09-27 21:52 ` [PATCH net-next v3 3/6] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ali Firas @ 2026-09-27 21:52 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, shuah, linux-kselftest, linux-kernel, Ali Firas
VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END are 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 has vni_start == vni_end ==
0xffffffff and looks like a single VNI; v is then -1, the comparison
promotes it to 0xffffffff and passes, v++ makes v 0, and the loop walks
the space upwards from there, creating a VNI node and a per-CPU stats
block per iteration under rtnl_lock. Any range ending at 0xffffffff
behaves the same way.
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 keeps its own rhashtable slot while
being truncated on the wire.
Range-validate both attributes against the 24-bit space, as vxlan_mdb.c
already does for its own VNI attributes. With the nest now linked to
this policy, an out-of-range endpoint is rejected during netlink policy
validation, before vxlan_process_vni_filter() runs and before anything
is allocated. How many VNIs a single in-range request may span is a
separate question, bounded by the next patch; that limit is not a policy
check and does run in the handler.
Make the loop counter u32 as well. With the range bounded it is no
longer what keeps the loop finite, but it drops the undefined signed
overflow past INT_MAX and matches the u32 vni that vxlan_vni_add() and
vxlan_vni_del() already take.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
Notes:
v3: was 1/5. No code change; the changelog now says the out-of-range rejection lands at policy validation, which holds for a multi-entry message once patch 1 links the nest.
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 d391ec579661..9cffaf4998a9 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] 13+ messages in thread* Re: [PATCH net-next v3 2/6] vxlan: vnifilter: reject VNIs outside the 24-bit space
2026-09-27 21:52 ` [PATCH net-next v3 2/6] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
@ 2026-09-30 3:52 ` netdev-bot+sashiko
0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 3:52 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, shuah, linux-kselftest, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 (3)
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] This commit fixes a real denial-of-service that users can
trigger, but it has no Fixes: tag and no stable marking.
Pre-existing issues:
- [Medium] At this commit, both endpoints are limited to 0..0xffffff, but
a single RTM_NEWTUNNEL entry such as START=0 END=0xffffff (or END…
- [Low] The vxlan FDB netlink path takes NDA_VNI and NDA_SRC_VNI with
nla_get_u32() and never checks them against VXLAN_N_VID.
- [Low] A reversed range (START > END, e.g. START=100 END=50) passes
vxlan_process_vni_filter(), which rejects only `!vni_start &&…
- [Low] VNI 0 can be added as part of a range but cannot be added,
updated, or deleted on its own.
--
vxlan: vnifilter: reject VNIs outside the 24-bit space
This commit adds NLA_POLICY_FULL_RANGE validation to the
VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END attributes. Both
are limited to 0..VXLAN_N_VID - 1. It also changes the loop counter in
vxlan_vni_add_del() from int to u32, which stops the loop from running
without bound when end_vni is 0xffffffff.
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index d391ec5796610..9cffaf4998a98 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)),
> };
[Severity: Medium]
This fixes a problem that users can trigger, but the trailers have no
Fixes: tag:
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
The bare NLA_U32 policy, the int loop counter and the loop in
vxlan_vni_add_del() all come from:
f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Before this patch, a single RTM_NEWTUNNEL entry with START=0xffffffff
passed the !vni_start && !vni_end check in vxlan_process_vni_filter(). The
loop then stopped only when vxlan_vni_add() failed. That means either
-ENOMEM, or -E2BIG from the rhashtable: vxlan_vni_rht_params.max_size is
VXLAN_N_VID, so the limit is about 2^25 nodes.
Each iteration allocates a vxlan_vni_node and a per-CPU stats block, does
an rhashtable insert and sends a notification, all under rtnl_lock.
RTM_NEWTUNNEL only needs CAP_NET_ADMIN in the user namespace that owns the
netns.
Should this carry a Fixes: tag for f9c4bb0b245c so that stable kernels
pick it up?
A related point: the changelog says:
With the nest now linked to this policy, an out-of-range endpoint is
rejected during netlink policy validation, before
vxlan_process_vni_filter() runs and before anything is allocated.
That depends on the preceding patch, "vxlan: vnifilter: validate the
VXLAN_VNIFILTER_ENTRY nest". If this patch is backported alone, the
rejection happens in the nla_parse_nested() call inside
vxlan_process_vni_filter(). By then, earlier entries in the same message
have already been applied.
If both patches are meant for stable, could the changelog state this
dependency?
[Severity: Low]
This isn't a bug introduced by this patch, but the FDB netlink path seems
to have the same class of problem. vxlan_fdb_parse() in
drivers/net/vxlan/vxlan_core.c only checks the length of NDA_VNI and
NDA_SRC_VNI:
if (tb[NDA_VNI]) {
if (nla_len(tb[NDA_VNI]) != sizeof(u32)) {
NL_SET_ERR_MSG(extack, "Invalid vni");
return -EINVAL;
}
*vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
...
*src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
The FDB get and bulk flush paths do not check the range either.
A remote_vni >= VXLAN_N_VID can be stored, and vxlan_vni_field() then drops
its top byte on transmit. A src_vni that large can never match a received
24-bit VNI.
IFLA_VXLAN_ID, vxlan_mdb.c and now vnifilter all enforce the 24-bit bound.
Would a follow-up doing the same for the FDB attributes make sense? Note
that vxlan_core.c uses cpu_to_be32(VXLAN_N_VID) internally as a sentinel
remote_vni.
> @@ -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);
>
[Severity: Medium]
This is a pre-existing issue, but with both endpoints capped at 0xffffff,
one entry with START=0 END=0xffffff still makes the loop in
vxlan_vni_add_del() run about 16.7M times under rtnl_lock, with no
cond_resched(). An entry with END alone does the same, because vni_start
then stays 0:
for (v = start_vni; v <= end_vni; v++) {
switch (cmd) {
case RTM_NEWTUNNEL:
err = vxlan_vni_add(vxlan, vg, v, group, extack);
Each iteration walks the netns vxlan_list in vxlan_vni_in_use(). It also
allocates a node and a per-CPU stats block with plain GFP_KERNEL, which is
not charged to memcg. A single message can carry many such entries.
The changelog leaves the span limit to the next patch, and later patches
in the series handle it:
- "vxlan: vnifilter: bound the number of VNIs one request may touch"
adds VXLAN_VNI_FILTER_MSG_MAX (4096). The limit is summed across all
entries and checked in vxlan_vnifilter_check_msg() before the dispatch
loop in vxlan_vnifilter_process().
- "vxlan: vnifilter: account per-VNI memory to memcg" switches
vxlan_vni_alloc() and the per-CPU stats allocation to
GFP_KERNEL_ACCOUNT.
[Severity: Low]
This isn't a bug introduced by this patch, but a reversed range still
returns success. vxlan_process_vni_filter() only rejects the case where
both endpoints are zero:
if (!vni_start && !vni_end) {
So START=100 END=50 reaches the loop in vxlan_vni_add_del(). The body
never runs, and 0 is returned for both RTM_NEWTUNNEL and RTM_DELTUNNEL.
The new range policy checks each endpoint on its own, not their order.
Later in the series, vxlan_vnifilter_check_msg() documents this as
intended:
/* A start above the end selects no VNI and costs nothing; leave it
behaving as it does today. */
The selftest in that series also checks that an inverted range is
accepted as a no-op.
Is a success reply for a request that does nothing the intended uAPI
here?
[Severity: Low]
This is also a pre-existing issue: VNI 0 is handled inconsistently. With
START=0 END=1, the zero check passes and the loop calls vxlan_vni_add() for
v == 0, which inserts VNI 0.
A later request for VNI 0 alone (START=0, or START=0 END=0) is rejected in
vxlan_process_vni_filter():
if (!vni_start && !vni_end) {
NL_SET_ERR_MSG_ATTR(extack, nlvnifilter,
"vni start nor end found in vni entry");
return -EINVAL;
}
The check tests the endpoint values rather than whether the attributes
are present.
As a result, VNI 0 added through a range can only be removed by another
range that covers it, or by tearing down the device. VNI 0 is otherwise
valid: IFLA_VXLAN_ID accepts it, and the minimum of the new
vni_filter_vni_range is 0.
Should this check look at whether vattrs[VXLAN_VNIFILTER_ENTRY_START] and
vattrs[VXLAN_VNIFILTER_ENTRY_END] are present instead?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927215209.2581830-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v3 3/6] vxlan: vnifilter: bound the number of VNIs one request may touch
2026-09-27 21:52 [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory Ali Firas
2026-09-27 21:52 ` [PATCH net-next v3 1/6] vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest Ali Firas
2026-09-27 21:52 ` [PATCH net-next v3 2/6] vxlan: vnifilter: reject VNIs outside the 24-bit space Ali Firas
@ 2026-09-27 21:52 ` Ali Firas
2026-09-30 3:52 ` netdev-bot+sashiko
2026-09-27 21:52 ` [PATCH net-next v3 4/6] vxlan: vnifilter: clamp the dumped VNI range to the request limit Ali Firas
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ali Firas @ 2026-09-27 21:52 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, shuah, linux-kselftest, linux-kernel, Ali Firas
A single RTM_NEWTUNNEL or RTM_DELTUNNEL message can ask for the whole
24-bit space in one range. vxlan_vni_add_del() then walks it one VNI at
a time under rtnl_lock, holding the lock for the length of the walk.
That walk is the cost being bounded, not the memory: an add allocates a
VNI node and a per-CPU stats block per VNI, a delete creates neither,
but both walk the same span and hold rtnl the same way, so both are
bounded.
The span of one VXLAN_VNIFILTER_ENTRY is not the quantity to bound. An
entry carrying just START and END is 20 bytes on the wire and a message
may carry many of them, vxlan_process_vni_filter() being called once per
entry, so bounding each entry alone would still let one message ask for
thousands of times the limit. Sum the spans of every entry and reject
the message as a whole in vxlan_vnifilter_check_msg(), before the
dispatch loop: entries are applied and notified one at a time, so a
limit enforced during dispatch would return an error only after every
preceding entry had been acted on. The range extraction is factored
into vxlan_vni_filter_entry_range() so the count and the range later
acted on cannot drift.
The limit is 4096, following the usable VLAN ID space, since vnifilter
is mainly used on bridged devices where the VNI is derived from the
VLAN. It bounds one request, not how many VNIs a device may hold: the
whole space can still be installed in more than one request, and a
device holding it is still torn down in one step by
vxlan_vnigroup_uninit(), which is device teardown, not a netlink
message, and is not subject to this cap.
This is a policy narrowing of what a single message may ask for, not a
fix for a crash or corruption, and is not a backport candidate.
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
Notes:
v3: was 2/5. Symmetric cap kept (bounds add and delete); changelog reframed around the rtnl walk, and the dump-range asymmetry moved to patch 4.
drivers/net/vxlan/vxlan_vnifilter.c | 89 ++++++++++++++++++++++++++---
1 file changed, 81 insertions(+), 8 deletions(-)
diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 9cffaf4998a9..13f4e115701a 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 one request touching more VNIs than the VLAN ID space has
+ * no practical use, while an unbounded request walks the 24-bit space one VNI
+ * at a time 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,78 @@ 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 request touching more than VXLAN_VNI_FILTER_MSG_MAX VNIs before any
+ * of its entries is acted on. Both add and delete walk the span one VNI at a
+ * time under rtnl_lock, so both are bounded. 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 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
+ * 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 +937,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 +1044,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] 13+ messages in thread* Re: [PATCH net-next v3 3/6] vxlan: vnifilter: bound the number of VNIs one request may touch
2026-09-27 21:52 ` [PATCH net-next v3 3/6] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
@ 2026-09-30 3:52 ` netdev-bot+sashiko
0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 3:52 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, shuah, linux-kselftest, linux-kernel
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The changelog justifies capping delete as well as add by saying
the two "both walk the same span and hold rtnl the same way".
- [Low] The commit message says the 4096 limit follows "the usable VLAN
ID space".
- [Low] At this commit, the dump path can emit a VXLAN_VNIFILTER_ENTRY
that the new request parser refuses.
--
vxlan: vnifilter: bound the number of VNIs one request may touch
This patch caps the number of VNIs that one RTM_NEWTUNNEL or RTM_DELTUNNEL
message may add or delete at 4096. The count is summed across all
VXLAN_VNIFILTER_ENTRY attributes and checked in a new
vxlan_vnifilter_check_msg() before any entry is applied. The range
extraction moves into vxlan_vni_filter_entry_range() so the check and the
dispatch path use the same range.
> That walk is the cost being bounded, not the memory: an add allocates a
> VNI node and a per-CPU stats block per VNI, a delete creates neither,
> but both walk the same span and hold rtnl the same way, so both are
> bounded.
[Severity: Low]
Is "both walk the same span" accurate for delete? vxlan_vni_del() returns
-ENOENT for a VNI that is not installed:
drivers/net/vxlan/vxlan_vnifilter.c:vxlan_vni_del() {
...
vninode = rhashtable_lookup_fast(&vg->vni_hash, &v,
vxlan_vni_rht_params);
if (!vninode) {
err = -ENOENT;
goto out;
}
...
}
vxlan_vni_add_del() then stops the walk at the first error:
if (err)
goto out;
So an RTM_DELTUNNEL walk covers the full span only when every VNI in it is
installed. That worst case can happen after several capped adds, so there
is still a reason to bound delete. The wording could be more precise,
though.
The changelog says that installs can be split across requests and that
vxlan_vnigroup_uninit() is exempt. It does not say that a user who
installed a contiguous run of more than 4096 VNIs can no longer delete it
in one message. The same applies to re-issuing RTM_NEWTUNNEL over that run
to change the group. vxlan_vnifilter_check_msg() charges the full requested
span, whatever is actually installed.
Could the changelog spell out this user-visible narrowing?
> The limit is 4096, following the usable VLAN ID space, since vnifilter
> is mainly used on bridged devices where the VNI is derived from the
> VLAN.
[Severity: Low]
This isn't a bug, but is "usable" the right word here? The usable 802.1Q
VLAN IDs are 1..4094. br_vlan_valid_id() in net/bridge/br_private.h only
accepts:
bool ret = vid > 0 && vid < VLAN_VID_MASK;
4096 is the size of the full 12-bit VID field, which includes the reserved
IDs 0 and 4095. The in-code comment on VXLAN_VNI_FILTER_MSG_MAX says "the
VLAN ID space", which reads fine.
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 9cffaf4998a98..13f4e115701ac 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
[ ... ]
> +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) {
[ ... ]
> + 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: Low]
At this point in the series, can a dump emit an entry that this check then
rejects?
vxlan_vnifilter_dump_dev() merges any contiguous run of VNIs with the same
remote into a single START/END entry. It puts no bound on the span:
drivers/net/vxlan/vxlan_vnifilter.c:vxlan_vnifilter_dump_dev() {
...
if (!dump_stats && vnirange(vend, v) == 1 &&
vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) {
goto update_end;
...
}
A device can still hold more than 4096 contiguous VNIs installed over
several requests, because vxlan_vni_rht_params uses
.max_size = VXLAN_N_VID. If such a dumped entry is replayed in one
RTM_NEWTUNNEL, vxlan_vnifilter_check_msg() returns -EINVAL.
The next patch in the series, "vxlan: vnifilter: clamp the dumped VNI
range to the request limit", fixes this. It adds
vnirange(vbegin, v) < VXLAN_VNI_FILTER_MSG_MAX to the merge condition, and
the selftest commit checks the dump/replay round trip.
Would it make sense to put the clamp before this patch, or fold it in, so
dump and request stay consistent at every commit in the series?
> + }
> +
> + return 0;
> +}
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927215209.2581830-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v3 4/6] vxlan: vnifilter: clamp the dumped VNI range to the request limit
2026-09-27 21:52 [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory Ali Firas
` (2 preceding siblings ...)
2026-09-27 21:52 ` [PATCH net-next v3 3/6] vxlan: vnifilter: bound the number of VNIs one request may touch Ali Firas
@ 2026-09-27 21:52 ` Ali Firas
2026-09-30 3:52 ` netdev-bot+sashiko
2026-09-27 21:52 ` [PATCH net-next v3 5/6] vxlan: vnifilter: account per-VNI memory to memcg Ali Firas
2026-09-27 21:52 ` [PATCH net-next v3 6/6] selftests: net: test the vxlan vnifilter request limit and dump replay Ali Firas
5 siblings, 1 reply; 13+ messages in thread
From: Ali Firas @ 2026-09-27 21:52 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, shuah, linux-kselftest, linux-kernel, Ali Firas
vxlan_vnifilter_dump_dev() coalesces a contiguous run of VNIs sharing a
remote into one VXLAN_VNIFILTER_ENTRY with no upper bound on the span. A
device may hold the whole 24-bit space, populated by several requests,
and dump it as a single entry START..END. Now that a single request is
bounded to VXLAN_VNI_FILTER_MSG_MAX VNIs, replaying such an entry in one
RTM_NEWTUNNEL is rejected: the kernel emits an entry it will not read
back, so a dump/replay of a device's VNI configuration fails.
Clamp a merged run to VXLAN_VNI_FILTER_MSG_MAX VNIs so every entry the
dump produces is one the input path accepts. The run is broken in the
merge condition, by ending it once it reaches the limit even when the
next VNI is contiguous and shares the remote; the two representations
then agree on the same bound.
Resume across netlink message boundaries is unchanged. cb->args[1]
counts the VNI nodes already dumped and is advanced only when a
completed entry is written; a run is still a gapless block, so its node
count equals vnirange() + 1 as before, and the clamp only moves where a
run ends. A device with more contiguous VNIs than fit in one skb still
resumes correctly, now split into limit-sized entries rather than one.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
Notes:
v3: new patch. Clamp the dumped run to the request limit so its output replays.
drivers/net/vxlan/vxlan_vnifilter.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 13f4e115701a..92ea1fc94f45 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -382,6 +382,7 @@ static int vxlan_vnifilter_dump_dev(const struct net_device *dev,
continue;
}
if (!dump_stats && vnirange(vend, v) == 1 &&
+ vnirange(vbegin, v) < VXLAN_VNI_FILTER_MSG_MAX &&
vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) {
goto update_end;
} else {
--
2.53.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next v3 4/6] vxlan: vnifilter: clamp the dumped VNI range to the request limit
2026-09-27 21:52 ` [PATCH net-next v3 4/6] vxlan: vnifilter: clamp the dumped VNI range to the request limit Ali Firas
@ 2026-09-30 3:52 ` netdev-bot+sashiko
0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 3:52 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, shuah, linux-kselftest, 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
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The patch limits each dumped VXLAN_VNIFILTER_ENTRY to
VXLAN_VNI_FILTER_MSG_MAX (4096) VNIs.
- [Low] The dump/replay mismatch this patch fixes comes from the previous
patch in the same unmerged series, commit "vxlan: vnifilter: bound the…
--
vxlan: vnifilter: clamp the dumped VNI range to the request limit
This commit caps each VXLAN_VNIFILTER_ENTRY produced by
vxlan_vnifilter_dump_dev() at VXLAN_VNI_FILTER_MSG_MAX VNIs. It does this
by ending a merged run of contiguous VNIs once the run reaches the limit.
The goal is for dumped entries to be accepted when they are sent back
through RTM_NEWTUNNEL.
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 13f4e115701ac..92ea1fc94f458 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -382,6 +382,7 @@ static int vxlan_vnifilter_dump_dev(const struct net_device *dev,
> continue;
> }
> if (!dump_stats && vnirange(vend, v) == 1 &&
> + vnirange(vbegin, v) < VXLAN_VNI_FILTER_MSG_MAX &&
> vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) {
> goto update_end;
> } else {
[Severity: Medium]
Does this clamp work at the same granularity as the input-side limit? It
bounds each entry. However, vxlan_vnifilter_check_msg() adds up the spans
of every VXLAN_VNIFILTER_ENTRY in a message:
drivers/net/vxlan/vxlan_vnifilter.c:vxlan_vnifilter_check_msg() {
...
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;
}
...
}
The comment above the define also describes a per-message limit:
/* Maximum number of VNIs one RTM_NEWTUNNEL or RTM_DELTUNNEL message may add or
* delete, summed over all of its VXLAN_VNIFILTER_ENTRY attributes. ...
vxlan_vnifilter_dump_dev() still opens one RTM_NEWTUNNEL nlmsg per device
per skb. It keeps adding entries until the skb is full, so nothing limits
the total number of VNIs in one dumped message.
For example, say a device holds VNIs 1-8192, added with two requests. The
dump now produces one message with the entries 1-4096 and 4097-8192.
Wouldn't resending that message unchanged still fail with "Request asks for
more than 4096 VNIs"?
The commit message says:
the two representations then agree on the same bound.
Is that accurate? After this change, replay works only if each entry goes
in its own request, which is what the selftest in this series does. No
later patch in the series changes vxlan_vnifilter_dump_dev().
One option is for the dump to count VNIs per nlmsg, end the message early
and resume through cb->args. The other is for the commit message to state
that the guarantee applies per entry only.
[Severity: Low]
The final state of the series is fine. However, the mismatch fixed here
comes from the earlier patch in this series, "vxlan: vnifilter: bound the
number of VNIs one request may touch". The commit message says so:
Now that a single request is bounded to VXLAN_VNI_FILTER_MSG_MAX VNIs,
replaying such an entry in one RTM_NEWTUNNEL is rejected
At that earlier commit, vxlan_vnifilter_dump_dev() still merges a
contiguous run of any length into one entry. A fully populated device
therefore dumps as a single 1-16777215 entry, and
vxlan_vnifilter_check_msg() rejects it.
Could this clamp be squashed into the limit patch, or moved before it, so
that each commit in the series is consistent for bisection? A Fixes: tag
would not be appropriate, since the cause is not upstream.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927215209.2581830-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v3 5/6] vxlan: vnifilter: account per-VNI memory to memcg
2026-09-27 21:52 [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory Ali Firas
` (3 preceding siblings ...)
2026-09-27 21:52 ` [PATCH net-next v3 4/6] vxlan: vnifilter: clamp the dumped VNI range to the request limit Ali Firas
@ 2026-09-27 21:52 ` Ali Firas
2026-09-30 3:52 ` netdev-bot+sashiko
2026-09-27 21:52 ` [PATCH net-next v3 6/6] selftests: net: test the vxlan vnifilter request limit and dump replay Ali Firas
5 siblings, 1 reply; 13+ messages in thread
From: Ali Firas @ 2026-09-27 21:52 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, shuah, linux-kselftest, 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, so neither is charged
to the cgroup of the process that created the VNI. A netdevice's own
queues, ethtool state and NAPI config are already GFP_KERNEL_ACCOUNT in
alloc_netdev_mqs() (commit c948f51c1654 ("memcg: enable accounting for
net_device and Tx/Rx queues")), so these per-VNI allocations are the
unaccounted part of a device that a cgroup can grow one VNI at a time.
Charge both to the caller's cgroup with GFP_KERNEL_ACCOUNT in
vxlan_vni_alloc(). __GFP_ACCOUNT only charges an allocation made from a
task in a non-root memcg, so the sole caller, which already handles a
NULL return, is unaffected outside a memcg.
The charge lands in the caller's memcg: adding VNIs from inside one
moves each node into the accounted slab and raises the cgroup's per-CPU
memory, while the unaccounted slab is unchanged.
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
Notes:
v3: replaces 3/5 and 4/5. Account the per-VNI node and its per-CPU stats in vxlan_vni_alloc().
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 92ea1fc94f45..3087877ed1cb 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -711,10 +711,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] 13+ messages in thread* Re: [PATCH net-next v3 5/6] vxlan: vnifilter: account per-VNI memory to memcg
2026-09-27 21:52 ` [PATCH net-next v3 5/6] vxlan: vnifilter: account per-VNI memory to memcg Ali Firas
@ 2026-09-30 3:52 ` netdev-bot+sashiko
0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 3:52 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, shuah, linux-kselftest, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The per-VNI accounting in commit "vxlan: vnifilter: account per-
VNI memory to memcg" is incomplete, and its commit message overstates…
- [Low] The commit message credits commit c948f51c1654 ("memcg: enable
accounting for net_device and Tx/Rx queues") with making "a…
--
vxlan: vnifilter: account per-VNI memory to memcg
This commit changes the struct vxlan_vni_node allocation and its per-CPU
stats block in vxlan_vni_alloc() from GFP_KERNEL to GFP_KERNEL_ACCOUNT.
Per-VNI memory added through the vnifilter netlink interface is then
charged to the caller's memcg.
> A netdevice's own
> queues, ethtool state and NAPI config are already GFP_KERNEL_ACCOUNT in
> alloc_netdev_mqs() (commit c948f51c1654 ("memcg: enable accounting for
> net_device and Tx/Rx queues")), so these per-VNI allocations are the
> unaccounted part of a device that a cgroup can grow one VNI at a time.
[Severity: Low]
Is this attribution accurate? Commit c948f51c1654 seems to change only
three allocations to GFP_KERNEL_ACCOUNT:
- the rx queues in netif_alloc_rx_queues()
- the tx queues in netif_alloc_netdev_queues()
- the net_device itself in alloc_netdev_mqs()
The accounted allocations of dev->ethtool, dev->cfg and dev->napi_config
in alloc_netdev_mqs() came from later commits:
net/core/dev.c:alloc_netdev_mqs() {
...
dev->ethtool = kzalloc_obj(*dev->ethtool, GFP_KERNEL_ACCOUNT);
...
dev->cfg = kzalloc_obj(*dev->cfg, GFP_KERNEL_ACCOUNT);
...
dev->napi_config = kvzalloc(napi_config_sz, GFP_KERNEL_ACCOUNT);
...
}
Could the reference be corrected?
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 92ea1fc94f458..3087877ed1cb9 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -711,10 +711,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);
[Severity: High]
Is the per-VNI accounting complete? The commit message calls these two
allocations "the unaccounted part of a device that a cgroup can grow one
VNI at a time".
However, vxlan_vni_add(), the only caller, then creates a default FDB
entry for each VNI that has a remote address:
vxlan_vni_add()
vxlan_vni_alloc() <- now accounted
vxlan_vni_update_group(..., create=true, ...)
vxlan_update_default_fdb_entry()
spin_lock_bh(&vxlan->hash_lock)
vxlan_fdb_update(..., NLM_F_APPEND | NLM_F_CREATE, ...)
vxlan_fdb_create()
vxlan_fdb_alloc()
vxlan_fdb_append()
In vxlan_vni_update_group(), the remote address is the per-VNI group. If
no group is given and the device has a default remote, it is
vxlan->default_dst.remote_ip.
None of the allocations on that path are accounted:
drivers/net/vxlan/vxlan_core.c:vxlan_fdb_alloc() {
...
f = kmalloc_obj(*f, GFP_ATOMIC);
...
}
drivers/net/vxlan/vxlan_core.c:vxlan_fdb_append() {
...
rd = kmalloc_obj(*rd, GFP_ATOMIC);
...
dst_cache_init(&rd->dst_cache, GFP_ATOMIC | __GFP_NOWARN);
...
}
The dst_cache_init() call makes a per-CPU allocation of struct
dst_cache_pcpu. Per CPU, that is about the same size as the
vxlan_vni_stats_pcpu block this patch now charges.
vxlan_fdb_create() only enforces cfg.addrmax when it is non-zero, and it
defaults to 0. A task with CAP_NET_ADMIN in its own netns might be inside
a user namespace, for example.
Could such a task still add VNIs one at a time and pin slab and per-CPU
memory that is never charged to its memcg?
vxlan_fdb_alloc() and vxlan_fdb_append() are also used by vxlan_snoop()
learning in softirq context. Charging current would be wrong there, so
any accounting would have to be limited to the netlink-driven create
path, for example by using GFP_ATOMIC | __GFP_ACCOUNT only there. No
later patch in this series seems to touch drivers/.
If extending the accounting isn't planned, could the commit message be
narrowed to what the patch actually covers?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927215209.2581830-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v3 6/6] selftests: net: test the vxlan vnifilter request limit and dump replay
2026-09-27 21:52 [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory Ali Firas
` (4 preceding siblings ...)
2026-09-27 21:52 ` [PATCH net-next v3 5/6] vxlan: vnifilter: account per-VNI memory to memcg Ali Firas
@ 2026-09-27 21:52 ` Ali Firas
2026-09-30 3:52 ` netdev-bot+sashiko
5 siblings, 1 reply; 13+ messages in thread
From: Ali Firas @ 2026-09-27 21:52 UTC (permalink / raw)
To: netdev, idosch
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, horms, razor,
roopa, shuah, linux-kselftest, linux-kernel, Ali Firas
Extend the vnifilter tests for the request-size limit, the 24-bit VNI
range, and the dump/replay round trip the limit requires.
The API test gains: the largest accepted add and one VNI past it; a
rejected oversized add and a rejected multi-entry add whose entries are
each under the limit but sum over it, each checked to install nothing
(the range is asserted absent first, since vxlan_vni_add() folds an
existing VNI into the update path and would return 0 either way). The cap
is symmetric, so it also checks a maximum-size delete accepted and a
max+1 delete rejected, with every VNI in the max+1 range installed first
so the refusal can only come from the cap; a within-cap delete of a
never-installed range, which must fail on the missing VNI; an inverted
range accepted as a no-op; and the 24-bit bound exercised through END,
not only START.
bridge(8) places one VXLAN_VNIFILTER_ENTRY per comma-separated item into
a single message, so the multi-entry case is reachable without a
hand-built netlink message.
vxlan_vnifilter_dump_replay() builds a contiguous run twice the limit
from two requests, then replays every range the dump reports into a
second device and requires all to be accepted and the two dumps to
match. Without the dump clamp the run dumps as one over-limit entry that
replay rejects; with it the run dumps as limit-sized entries that
replay.
vxlan_vnifilter_api() had no teardown of its own device and namespace;
add veth-host and the test netns to cleanup(), which runs on EXIT, so a
failing case does not leave them or its entries behind.
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Assisted-by: LLM
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
Notes:
v3: was 5/5. Add the request-limit, 24-bit-range, symmetric-delete and dump/replay cases; give the test its own netns teardown.
.../selftests/net/test_vxlan_vnifiltering.sh | 114 +++++++++++++++++-
1 file changed, 113 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
index 8deacc565afa..f48bc861bb88 100755
--- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
+++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
@@ -84,6 +84,7 @@ ret=0
# all tests in this script. Can be overridden with -t option
TESTS="
vxlan_vnifilter_api
+ vxlan_vnifilter_dump_replay
vxlan_vnifilter_datapath
vxlan_vnifilter_datapath_pervni
vxlan_vnifilter_datapath_mgroup
@@ -163,8 +164,9 @@ check_vm_connectivity() {
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
+ ip link del veth-host 2>/dev/null || true
- cleanup_ns $hv_1 $hv_2 $vm_11 $vm_21 $vm_12 $vm_22 $vm_31 $vm_32
+ cleanup_ns $hv_1 $hv_2 $vm_11 $vm_21 $vm_12 $vm_22 $vm_31 $vm_32 $testns
}
trap cleanup EXIT
@@ -371,6 +373,116 @@ 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 add at most 4096 VNIs. bridge(8) puts one
+ # VXLAN_VNIFILTER_ENTRY per comma-separated item into a single message,
+ # so both the single-entry and the multi-entry paths are reachable here.
+
+ # The largest accepted add, and one VNI more rejected.
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14095"
+ log_test $? 0 "Add a request of the maximum VNI count"
+
+ # The rejected oversized add must install nothing. Assert the range is
+ # absent first: vxlan_vni_add() folds an already-present VNI into the
+ # update path and returns 0, so a later probe cannot tell "installed
+ # nothing" from "installed part" unless it started absent.
+ run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 20000"
+ log_test $? 1 "VNI 20000 absent before the oversized add"
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 20000-24096"
+ log_test $? 255 "Cannot add a request over the maximum VNI count"
+ run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 20000"
+ log_test $? 1 "The rejected oversized add installed nothing"
+
+ # Two entries each under the limit but summing over it: the per-message
+ # total is what is bounded, not the span of one entry. This is the shape
+ # a per-entry check would have let through.
+ run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 30000"
+ log_test $? 1 "VNI 30000 absent before the oversized multi-entry add"
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 30000-32047,32048-34097"
+ log_test $? 255 "Cannot add a multi-entry request summing over the maximum"
+ run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 30000"
+ log_test $? 1 "The rejected multi-entry add installed nothing"
+
+ # The cap is symmetric: a delete may touch at most the maximum too.
+ # Install a maximum-size range and the VNI past it, so the max+1 delete
+ # below has every VNI present and can only be refused by the cap, not by
+ # a missing VNI.
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 40000-44095"
+ log_test $? 0 "Populate a maximum-size range to delete"
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 44096"
+ log_test $? 0 "Add the VNI past the maximum range"
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 40000-44096"
+ log_test $? 255 "Cannot delete a request over the maximum VNI count"
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 40000-44095"
+ log_test $? 0 "Delete a request of the maximum VNI count"
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 44096"
+ log_test $? 0 "Delete the VNI past the maximum range"
+
+ # A within-cap delete of a never-installed range reaches the handler and
+ # fails there on the missing VNI, not on the cap.
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 50000-50010"
+ log_test $? 255 "Cannot delete a range that was never installed"
+
+ # A start above the end selects nothing and is accepted as a no-op.
+ # Use a VNI no earlier case installs, so the absence check is meaningful.
+ run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 55000"
+ log_test $? 1 "VNI 55000 absent before the inverted range"
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 55000-50000"
+ log_test $? 0 "An inverted range is accepted as a no-op"
+ run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 55000"
+ log_test $? 1 "The inverted range installed nothing"
+
+ # The VXLAN header carries 24 bits. The bound is on both endpoints, so a
+ # range whose END alone leaves the space is rejected too.
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777215"
+ log_test $? 0 "Add the highest VNI the header can carry"
+ run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 16777215"
+ log_test $? 0 "Delete the highest VNI the header can carry"
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777215-16777216"
+ log_test $? 255 "Cannot add a range whose END leaves the 24-bit space"
+ run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 100-4294967295"
+ log_test $? 255 "Cannot add a range whose END wraps past the 24-bit space"
+}
+
+# A device may hold a contiguous run longer than one request's limit, built
+# from several requests. The dump coalesces it, so the dump must break the run
+# into entries each no larger than the limit, or the configuration it reports
+# cannot be replayed. Install such a run, then feed every range the dump
+# reports back into a second device and require all to be accepted.
+vxlan_vnifilter_dump_replay()
+{
+ local opts="external vnifilter local 172.16.0.1 dev veth-testns"
+ local range rc=0 src_ranges dst_ranges
+
+ cleanup_vnifilter_api &>/dev/null
+ setup_vnifilter_api
+
+ # The destination is on a different dstport so re-adding the same VNIs
+ # does not collide with the source under vxlan_vni_in_use().
+ run_cmd "ip -netns $testns link add vxlan-src type vxlan $opts dstport 4789"
+ log_test $? 0 "dump/replay: create source device"
+ run_cmd "ip -netns $testns link add vxlan-dst type vxlan $opts dstport 4790"
+ log_test $? 0 "dump/replay: create destination device"
+
+ # 8192 contiguous VNIs sharing the default remote: one run, twice the
+ # limit, installed in two accepted requests.
+ run_cmd "bridge -netns $testns vni add dev vxlan-src vni 10000-14095"
+ run_cmd "bridge -netns $testns vni add dev vxlan-src vni 14096-18191"
+ log_test $? 0 "dump/replay: populate a run larger than the limit"
+
+ for range in $(bridge -netns $testns vni show dev vxlan-src | \
+ grep -oE '[0-9]+-[0-9]+|[0-9]{2,}'); do
+ bridge -netns $testns vni add dev vxlan-dst vni "$range" \
+ 2>/dev/null || rc=$?
+ done
+ log_test $rc 0 "dump/replay: every dumped entry is accepted on replay"
+
+ src_ranges=$(bridge -netns $testns vni show dev vxlan-src | \
+ grep -oE '[0-9]+-[0-9]+|[0-9]{2,}' | sort)
+ dst_ranges=$(bridge -netns $testns vni show dev vxlan-dst | \
+ grep -oE '[0-9]+-[0-9]+|[0-9]{2,}' | sort)
+ [ -n "$src_ranges" ] && [ "$src_ranges" = "$dst_ranges" ]
+ log_test $? 0 "dump/replay: source and destination dump the same ranges"
}
# Sanity test vnifilter datapath
--
2.53.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next v3 6/6] selftests: net: test the vxlan vnifilter request limit and dump replay
2026-09-27 21:52 ` [PATCH net-next v3 6/6] selftests: net: test the vxlan vnifilter request limit and dump replay Ali Firas
@ 2026-09-30 3:52 ` netdev-bot+sashiko
0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-30 3:52 UTC (permalink / raw)
To: alishmery18
Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev,
horms, razor, roopa, shuah, linux-kselftest, linux-kernel
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] No test rejects a VNI because its START is outside the 24-bit
space, even though the commit message says the bound is "exercised…
- [Low] In vxlan_vnifilter_dump_replay()
(test_vxlan_vnifiltering.sh:469-471), two run_cmd calls populate the…
- [Low] vxlan_vnifilter_dump_replay() only compares the source dump with
the destination dump.
- [Low] Every absence and "installed nothing" check in
vxlan_vnifilter_api() (lines 389, 393, 399, 403, 428, 432) runs…
--
selftests: net: test the vxlan vnifilter request limit and dump replay
This extends test_vxlan_vnifiltering.sh to cover the vnifilter request-size
limit, the 24-bit VNI range, and symmetric add/delete caps. It adds a new
vxlan_vnifilter_dump_replay() test that builds a run twice the limit,
replays the dumped ranges into a second device, and compares the two dumps.
It also adds veth-host and the test netns to cleanup().
> diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> index 8deacc565afa1..f48bc861bb88e 100755
> --- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
> +++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
[ ... ]
> @@ -371,6 +373,116 @@ vxlan_vnifilter_api()
[ ... ]
> + # The rejected oversized add must install nothing. Assert the range is
> + # absent first: vxlan_vni_add() folds an already-present VNI into the
> + # update path and returns 0, so a later probe cannot tell "installed
> + # nothing" from "installed part" unless it started absent.
> + run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 20000"
> + log_test $? 1 "VNI 20000 absent before the oversized add"
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 20000-24096"
> + log_test $? 255 "Cannot add a request over the maximum VNI count"
> + run_cmd "bridge -netns $testns vni show dev vxlan-ext1 | grep -qw 20000"
> + log_test $? 1 "The rejected oversized add installed nothing"
[Severity: Low]
Can these absence checks pass without looking at the device at all?
run_cmd() does this:
out=$(eval $cmd $stderr)
rc=$?
That returns the status of the last command in the pipeline, which is
grep. The script does not set pipefail.
Suppose bridge vni show fails or prints nothing, for example because of a
regression in the dump path. grep -qw 20000 then exits 1 on empty input,
and log_test $? 1 reports OK.
The same pattern is used for every absence and "installed nothing" check
in vxlan_vnifilter_api(), for VNIs 20000, 30000 and 55000.
Could the dump status be checked on its own, or pipefail be enabled for
these commands?
[ ... ]
> + # The VXLAN header carries 24 bits. The bound is on both endpoints, so a
> + # range whose END alone leaves the space is rejected too.
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777215"
> + log_test $? 0 "Add the highest VNI the header can carry"
> + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 16777215"
> + log_test $? 0 "Delete the highest VNI the header can carry"
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 16777215-16777216"
> + log_test $? 255 "Cannot add a range whose END leaves the 24-bit space"
> + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 100-4294967295"
> + log_test $? 255 "Cannot add a range whose END wraps past the 24-bit space"
> +}
[Severity: Low]
The commit message says:
and the 24-bit bound exercised through END,
not only START.
Does any case here reject a VNI because its START is out of range? START
only appears as an accepted value (vni 16777215). Both rejected cases go
out of range through END only.
The 100-4294967295 case also seems to be rejected by the request cap
whatever the policy says. In vxlan_vnifilter_check_msg():
vnis += vni_end - vni_start + 1;
if (vnis > VXLAN_VNI_FILTER_MSG_MAX) {
The span is far over 4096, so this returns -EINVAL whether or not the
NLA_POLICY_FULL_RANGE entries in vni_filter_entry_policy exist. So only
16777215-16777216 tests the 24-bit policy.
The case the series' "reject VNIs outside the 24-bit space" patch is
mainly about, a lone START=0xffffffff, has no test. That request has
vnis == 1, so the cap accepts it. Only the START policy then keeps
vxlan_vni_add_del() out of this endless loop:
for (v = start_vni; v <= end_vni; v++) {
Would a case such as vni 16777216 expecting 255 be worth adding? It
covers START without risking a hang if it regresses.
> +
> +# A device may hold a contiguous run longer than one request's limit, built
> +# from several requests. The dump coalesces it, so the dump must break the run
> +# into entries each no larger than the limit, or the configuration it reports
> +# cannot be replayed. Install such a run, then feed every range the dump
> +# reports back into a second device and require all to be accepted.
> +vxlan_vnifilter_dump_replay()
> +{
[ ... ]
> + # 8192 contiguous VNIs sharing the default remote: one run, twice the
> + # limit, installed in two accepted requests.
> + run_cmd "bridge -netns $testns vni add dev vxlan-src vni 10000-14095"
> + run_cmd "bridge -netns $testns vni add dev vxlan-src vni 14096-18191"
> + log_test $? 0 "dump/replay: populate a run larger than the limit"
[Severity: Low]
Is the exit status of the first add ever checked? The second run_cmd
overwrites $?, so log_test only sees the 14096-18191 add.
If the first add fails and the second succeeds, vxlan-src holds only
14096-18191. That is exactly 4096 VNIs, and it dumps as one entry the
input path accepts, with or without this clamp in
vxlan_vnifilter_dump_dev():
vnirange(vbegin, v) < VXLAN_VNI_FILTER_MSG_MAX &&
Replay then succeeds and the two dumps match, so all three dump/replay
checks report OK without testing the clamp.
This also applies when the test runs alone with
-t vxlan_vnifilter_dump_replay. In that case the API test's max-size add
check does not run first.
Could each add get its own log_test?
> +
> + for range in $(bridge -netns $testns vni show dev vxlan-src | \
> + grep -oE '[0-9]+-[0-9]+|[0-9]{2,}'); do
> + bridge -netns $testns vni add dev vxlan-dst vni "$range" \
> + 2>/dev/null || rc=$?
> + done
> + log_test $rc 0 "dump/replay: every dumped entry is accepted on replay"
> +
> + src_ranges=$(bridge -netns $testns vni show dev vxlan-src | \
> + grep -oE '[0-9]+-[0-9]+|[0-9]{2,}' | sort)
> + dst_ranges=$(bridge -netns $testns vni show dev vxlan-dst | \
> + grep -oE '[0-9]+-[0-9]+|[0-9]{2,}' | sort)
> + [ -n "$src_ranges" ] && [ "$src_ranges" = "$dst_ranges" ]
> + log_test $? 0 "dump/replay: source and destination dump the same ranges"
[Severity: Low]
Would this comparison catch a dump that drops part of the run? Both sides
come from the same vxlan_vnifilter_dump_dev(), and neither is compared
with the 10000-18191 range that was installed.
Suppose the clamp regressed so the dump stopped at the first limit
boundary and reported only 10000-14095, with no new entry starting at
14096. The replay loop would install 10000-14095 on vxlan-dst, and both
dumps would print 10000-14095.
Both the "every dumped entry is accepted on replay" and "source and
destination dump the same ranges" checks would pass, even though half
the configuration was missing from the dump.
Could src_ranges also be compared against the expected
10000-14095 14096-18191?
> }
>
> # Sanity test vnifilter datapath
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927215209.2581830-1-alishmery18%40gmail.com
^ permalink raw reply [flat|nested] 13+ messages in thread