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 comment above VXLAN_VNI_FILTER_MSG_MAX in
drivers/net/vxlan/vxlan_vnifilter.c (lines 20-24) describes the order…
--
vxlan: vnifilter: clamp the dumped VNI range to the request limit
This commit adds VXLAN_VNI_FILTER_MSG_MAX and limits
vxlan_vnifilter_dump_dev() in two ways. A coalesced run of VNIs ends once
it reaches the limit, and each netlink message has a VNI budget. As a
result, no dumped message holds more VNIs than the input path accepts in
one request after a later patch in the series.
> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c
> b/drivers/net/vxlan/vxlan_vnifilter.c
> index 9e15f4de75c99..5beec29eb84a8 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -17,6 +17,13 @@
>
> #include "vxlan_private.h"
>
> +/* The dump coalesces a contiguous run of VNIs into entries, and packs
> entries
> + * into netlink messages, of at most this many VNIs each, so every message
> the
> + * dump emits stays within what the input path will accept on replay. A later
> + * patch bounds a single RTM_NEWTUNNEL/RTM_DELTUNNEL request to the same
> number.
> + */
> +#define VXLAN_VNI_FILTER_MSG_MAX 4096
[Severity: Low]
This isn't a bug, but will the "A later patch bounds a single
RTM_NEWTUNNEL/RTM_DELTUNNEL request to the same number" sentence go stale
once the series is merged?
At the end of the series this comment is unchanged. By then, the commit
"vxlan: vnifilter: bound the number of VNIs one request may touch" has
added the request cap. vxlan_vnifilter_process() calls
vxlan_vnifilter_check_msg(), which does this:
if (vnis > VXLAN_VNI_FILTER_MSG_MAX) {
Once merged, "a later patch" no longer points to anything. The comment
also doesn't say where the input-side limit is enforced.
Could the comment describe the invariant instead? For example, "must
match the per-request limit enforced by vxlan_vnifilter_check_msg()".
Another option is to move that sentence into the patch that adds the
cap.
The dump and input paths agree in the final tree, so this is only about
the comment text.
[ ... ]
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014123814.388557-1-alishmery18%40gmail.com
