Re: [PATCH net-next v3 0/6] vxlan: vnifilter: bound a single request and account per-VNI memory

[Paolo Abeni <pabeni@redhat.com>]

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/20260921150904.65a704eb@kernel.org/
> 
> 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