Re: [PATCH] net: mark skb NAPI IDs independently of busy polling

[Jacob Keller <jacob.e.keller@intel.com>]

On 11/5/2026 12:04 AM, bot+bpf-ci@kernel.org wrote:
>> net: mark skb NAPI IDs independently of busy polling
> 
> TL;DR: Commit message only: it blames e68b6e50fa35 for gating skb NAPI IDs on
> busy polling, but that gating predates it; 97dc7cd92ac6 introduced the
> dependency. Suggests rewording.
> 
>>     net: mark skb NAPI IDs independently of busy polling
>>
>>     Commit 97dc7cd92ac6 ("ptp: Support late timestamp determination")
>>     added support for deferring RX timestamp conversion to
>>     ndo_get_tstamp(). When the timestamp is consumed, get_timestamp()
>>     uses the skb's NAPI ID to find the device that owns the timestamp
>>     cookie and invoke its callback.
>>     Commit e68b6e50fa35 ("udp: enable busy polling for all sockets")
>>     gated NAPI ID bookkeeping with CONFIG_NET_RX_BUSY_POLL. As a result,
>>     when busy polling is disabled, the skb may not carry a usable NAPI ID.
>>     The device lookup then fails, so the driver callback is not invoked.
>>     get_timestamp() falls back to hwtstamp, which does not contain the
>>     converted timestamp, resulting in an incorrect RX hardware timestamp.
>>     Record skb NAPI IDs independently of busy-poll support to support
>>     drivers that use deferred timestamp conversion and future driver
>>     features that need to look up a device by its NAPI ID.
>>     Signed-off-by: Ding Meng <meng.ding@siemens.com>
>>     Reviewed-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> 
> This isn't a bug, but is e68b6e50fa35 the right commit to name here?
> 
> Before that commit, __skb_mark_napi_id() / skb_mark_napi_id() was already
> inside '#ifdef CONFIG_NET_RX_BUSY_POLL' with an empty stub in the #else
> branch, and struct sk_buff already had napi_id under
> '#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)'.  That gating
> goes back to 8b80cda536ea ("net: rename ll methods to busy-poll").
> 
> e68b6e50fa35 only moved the socket-side sk_mark_napi_id() out of the ifdef
> block and added sk_mark_napi_id_once().  Both act on sk->sk_napi_id, not on
> the skb.
> 
> It is also about six years older than 97dc7cd92ac6, so the message reads as
> if a later change broke the deferred-timestamp lookup.  In fact
> get_timestamp() has depended on a busy-poll-only skb field (skb_napi_id()
> returns 0 when CONFIG_NET_RX_BUSY_POLL=n) since 97dc7cd92ac6 added it.
> 
> Could the message say that the skb NAPI ID was always busy-poll-only and
> that 97dc7cd92ac6 started depending on it?
> 

I think the AI is reading a bit much into the commit message. Clearly
the bug is that 97dc7cd92ac6 depends on busy polling. The wording could
be slightly more precise to not imply that e68b6e50fa35 actually
introduced a problem, but I think that is a bit of a nitpick, and not
worth a reroll of this work unless something else needs to change.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41989099174