On 9/27/26 23:52, Ali Firas wrote:
The VNI filter interface accepts a START/END range with no bound on
either endpoint and no bound on how many VNIs one message may ask for,
and the memory it allocates per VNI is not charged to the caller's
cgroup.
Since v2, Jakub asked that the VXLAN_VNIFILTER_ENTRY nest be linked to
its policy with NLA_POLICY_NESTED() as patch 1:
https://lore.kernel.org/netdev/[email protected]/
Patch 1 does that. Without it the entry attributes were validated only
as each entry was dispatched, so a multi-entry message whose later
entry was invalid had the earlier entries applied and notified before
the message was rejected. With the nest linked the whole message is
validated up front and a bad entry rejects the message as a unit and
installs nothing.
Patches 2 and 3 bound a single request: patch 2 range-validates both
endpoints to the 24-bit VNI space, and patch 3 caps the number of VNIs
one request may add or delete, summed over its entries, at 4096. Both
add and delete walk the span one VNI at a time under rtnl_lock, so both
are bounded; that walk is the cost, not the memory.
Patch 4 clamps the dump. vxlan_vnifilter_dump_dev() coalesces a
contiguous run with no bound, so a device populated by several requests
could dump a single entry that patch 3 then refuses on replay. Patch 4
clamps the merged run to the same limit, so dump output is always
re-enterable; the selftest installs more than the limit, dumps it, and
replays what the dump reported.
Patch 5 charges the per-VNI node and its per-CPU stats block to the
cgroup of the task that created the VNI, so the memory a device grows one
VNI at a time is accounted the way the device's own queues, ethtool
state and NAPI config already are (commit c948f51c1654 ("memcg: enable
accounting for net_device and Tx/Rx queues")).
Patch 6 adds the selftests.
The cap is symmetric, applied to add and delete alike, which is what
Ido asked for when he agreed to a 4k limit. Patch 4 is what makes that
safe: because the dump is clamped to the same constant, the kernel never
reports a contiguous run that its own input path would reject, so a
device holding more than 4096 VNIs can still be torn down by replaying
what "bridge vni show" reports. Device teardown itself
(vxlan_vnigroup_uninit()) is not a netlink message and is not subject to
the cap.
uAPI changes, all of them:
- a VNI at or above 2^24, previously accepted and stored (and
truncated on the wire), is now rejected with -ERANGE;
- a single request asking to add or delete more than 4096 VNIs, summed
over its entries, is now rejected with -EINVAL -- "bridge vni add
dev X vni 1-10000" used to be accepted;
- a request rejected by the entry policy, aimed at a missing or
non-vnifilter device, now returns that policy error rather than
-ENODEV or -EOPNOTSUPP, because the nest is validated before the
device is resolved (patch 1);
- "bridge vni show" now splits a contiguous run longer than the limit
into several entries instead of one (patch 4); the set of VNIs it
reports is unchanged.
This is a policy and hardening change, not a fix for a crash or
corruption; it targets net-next and should not be backported.
One pre-existing semantic is worth review: an entry that carries only
END and no START is treated as the range [0, END], so a lone END near
the top of the space is a whole-space request that the cap now rejects;
whether END-without-START should mean that is left as an open question.
Not addressed here:
- vxlan_vni_add_del() still leaves the earlier VNIs of a range
installed if an allocation fails partway through it; with the cap
that is now bounded to fewer than 4096 VNIs. A fix needs a
transaction and overlaps a separate rollback change, so it is left
out.
- a lone VNI 0 dumps as a START-only entry the input path refuses
("vni start nor end found"), and a stats dump carries per-entry
stats the input path refuses; both predate this series and are
unrelated to the limit.
Sashiko complain WRT partial accounting of patch 5/6 looks legit.
Also it would make sense to reword a bit the commit message of patch 1 and
2 to reflect the above.
/P
