Thread (12 messages) 12 messages, 2 authors, 5d ago

Re: [PATCH net-next v3 3/5] net/sched: act_gate: cap the number of scheduling entries

flat view

From: netdev-bot+sashiko@kernel.org
Date: 2026-10-14 09:15:42

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] Concern (rejected after checking): the commit message relies on
  "net/sched: act_api: report an oversized action dump instead of…

--

net/sched: act_gate: cap the number of scheduling entries

This commit caps the number of TCA_GATE_ONE_ENTRY elements that
parse_gate_list() accepts at GATE_ENTRIES_MAX (1024). Larger lists are
rejected with -E2BIG and an extack message. The limit is based on the
SJA1105 schedule-table capacity and on the 16-bit nla_len limit of a
nested attribute.

Link: https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/ (local)
Why 1024 despite the dump budget:
1024 is a SJA1105 schedule-table capacity, not a dump-budget value. The
kernel clamps a dump skb to SKB_WITH_OVERHEAD(32768), so only ~800-890
entries of this list fit "tc actions ls"; the top of the admitted band
therefore installs but is not listed. That is accepted deliberately:
a unicast RTM_GETACTION reply is pre-sized from the action's advertised
size (budgeted per entry since the parent fix), so "tc actions get"
enumerates the whole band, and "net/sched: act_api: report an oversized
action dump instead of silently truncating it" makes the list view fail
loudly with -EMSGSIZE and an extack rather than silently truncating.
[Severity: Low]
This is a minor point about the commit message, not about the code.

The paragraph reads as if the -EMSGSIZE behaviour is already in place.
The referenced commit is the last patch of this series, though. At this
commit, tcf_dump_walker() in net/sched/act_api.c still drops an action
that does not fit:

net/sched/act_api.c:tcf_dump_walker() {
    ...
		if (err < 0) {
			index--;
			nlmsg_trim(skb, nest);
			goto done;
		}
    ...
}

So at this bisect point, a gate with roughly 890-1024 entries is still
silently left out of "tc actions ls". That is not a regression: the
truncation already happened before this patch, when gates of any size
were allowed. This patch only narrows the range that triggers it, and
the final patch of the series makes it fail loudly.

Could the message say that the act_api change comes later in this
series?

Could "the parent fix" also name the commit it means, presumably
cfa165cbfbed, which added the per-entry get_fill_size budget?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v3.r0.20261012193813%40mojatatu.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help