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

[Gerhard Engleder <gerhard@engleder-embedded.com>]

On 05.11.26 08:16, Ding Meng wrote:
> 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>
> ---
> In the discussion of the issue reported at
> https://lore.kernel.org/netdev/20260630112056.885071-1-meng.ding@siemens.com/
> this previous patch added an igc-specific fallback for configurations
> with CONFIG_NET_RX_BUSY_POLL disabled. It populated skb's hwtstamp
> field with a timestamp converted using the default hardware timer,
> allowing get_timestamp() to return it without invoking ndo_get_tstamp().
> But Jakub Kicinski noted that the proposed fix appeared to be a
> driver-specific workaround for a generic problem. The engleder driver
> would also be affected by the same issue.
> 
> This patch introduces a new configuration option to enable NAPI ID
> bookkeeping independently of busy polling. The option is enabled
> whenever busy polling is enabled, preserving the existing behavior.
> It is also selected by the igc and engleder drivers, ensuring that
> skbs carry a NAPI ID even when busy polling is disabled.
> 
> Another approach considered was to require drivers using deferred
> timestamp conversion, such as igc and engleder, to enable busy polling.
> However, this would introduce an unnecessary dependency between two
> independent features: hardware timestamp reporting should not require
> busy polling to be enabled.
> 
> Timestamping has been tested only with igc; it has not been tested
> with engleder.
> 
>   Documentation/networking/timestamping.rst | 11 ++++++++---
>   drivers/net/ethernet/engleder/Kconfig     |  1 +
>   drivers/net/ethernet/intel/Kconfig        |  1 +
>   include/linux/skbuff.h                    |  4 ++--
>   include/net/busy_poll.h                   |  2 +-
>   include/trace/events/net.h                |  2 +-
>   net/Kconfig                               |  4 ++++
>   net/core/filter.c                         |  2 +-
>   net/core/skbuff.c                         |  2 +-
>   9 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> index 2162c4f2b28a6..dbe40a1972639 100644
> --- a/Documentation/networking/timestamping.rst
> +++ b/Documentation/networking/timestamping.rst
> @@ -278,9 +278,14 @@ SOF_TIMESTAMPING_OPT_PKTINFO:
>     packets with hardware timestamps. The message contains struct
>     scm_ts_pktinfo, which supplies the index of the real interface which
>     received the packet and its length at layer 2. A valid (non-zero)
> -  interface index will be returned only if CONFIG_NET_RX_BUSY_POLL is
> -  enabled and the driver is using NAPI. The struct contains also two
> -  other fields, but they are reserved and undefined.
> +  interface index will be returned only if CONFIG_SKB_MARK_NAPI_ID is
> +  enabled and the driver is using NAPI. This hidden configuration is
> +  selected automatically by busy-poll support and by drivers that need
> +  skb NAPI IDs to resolve deferred hardware timestamps. Drivers that
> +  use deferred timestamp conversion must select it so the networking
> +  stack can find the device and call its timestamp callback even when
> +  busy polling is disabled. The struct contains also two other fields,
> +  but they are reserved and undefined.
>   
>   SOF_TIMESTAMPING_OPT_TX_SWHW:
>     Request both hardware and software timestamps for outgoing packets
> diff --git a/drivers/net/ethernet/engleder/Kconfig b/drivers/net/ethernet/engleder/Kconfig
> index 3df6bf476ae7e..9700a243bea11 100644
> --- a/drivers/net/ethernet/engleder/Kconfig
> +++ b/drivers/net/ethernet/engleder/Kconfig
> @@ -22,6 +22,7 @@ config TSNEP
>   	depends on PTP_1588_CLOCK_OPTIONAL
>   	select PHYLIB
>   	select PAGE_POOL
> +	select SKB_MARK_NAPI_ID
>   	help
>   	  Support for the Engleder TSN endpoint Ethernet MAC IP Core.
>   
> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
> index 780f113986ea8..7fb63f351fc20 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -376,6 +376,7 @@ config IGC
>   	depends on PCI
>   	depends on PTP_1588_CLOCK_OPTIONAL
>   	depends on ETHTOOL_NETLINK
> +	select SKB_MARK_NAPI_ID
>   	help
>   	  This driver supports Intel(R) Ethernet Controller I225-LM/I225-V
>   	  family of adapters.
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6787fcf6ac3e4..164c862f7a403 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1059,7 +1059,7 @@ struct sk_buff {
>   			__u16	vlan_tci;
>   		};
>   	};
> -#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
> +#if defined(CONFIG_SKB_MARK_NAPI_ID) || defined(CONFIG_XPS)
>   	union {
>   		unsigned int	napi_id;
>   		unsigned int	sender_cpu;
> @@ -1267,7 +1267,7 @@ static inline bool skb_pkt_type_ok(u32 ptype)
>    */
>   static inline unsigned int skb_napi_id(const struct sk_buff *skb)
>   {
> -#ifdef CONFIG_NET_RX_BUSY_POLL
> +#ifdef CONFIG_SKB_MARK_NAPI_ID
>   	return skb->napi_id;
>   #else
>   	return 0;
> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> index 6e172d0f6ef5f..f7c0e62c6a7d7 100644
> --- a/include/net/busy_poll.h
> +++ b/include/net/busy_poll.h
> @@ -130,7 +130,7 @@ static inline void sk_busy_loop(struct sock *sk, int nonblock)
>   static inline void __skb_mark_napi_id(struct sk_buff *skb,
>   				      const struct gro_node *gro)
>   {
> -#ifdef CONFIG_NET_RX_BUSY_POLL
> +#ifdef CONFIG_SKB_MARK_NAPI_ID
>   	/* If the skb was already marked with a valid NAPI ID, avoid overwriting
>   	 * it.
>   	 */
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index dbc2c5598e35a..99ed7645242eb 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -208,7 +208,7 @@ DECLARE_EVENT_CLASS(net_dev_rx_verbose_template,
>   
>   	TP_fast_assign(
>   		__assign_str(name);
> -#ifdef CONFIG_NET_RX_BUSY_POLL
> +#ifdef CONFIG_SKB_MARK_NAPI_ID
>   		__entry->napi_id = napi_id_valid(skb->napi_id) ?
>   				   skb->napi_id : 0;
>   #else
> diff --git a/net/Kconfig b/net/Kconfig
> index 76ab44aa439a6..8fa7586dc08ca 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -346,9 +346,13 @@ config CGROUP_NET_CLASSID
>   	  Cgroup subsystem for use as general purpose socket classid marker that is
>   	  being used in cls_cgroup and for netfilter matching.
>   
> +config SKB_MARK_NAPI_ID
> +	bool
> +
>   config NET_RX_BUSY_POLL
>   	bool
>   	default y if !PREEMPT_RT || (PREEMPT_RT && !NETCONSOLE)
> +	select SKB_MARK_NAPI_ID
>   
>   config BQL
>   	bool
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b7ac145b68b1f..e2218c18622b8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -10274,7 +10274,7 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
>   		break;
>   
>   	case offsetof(struct __sk_buff, napi_id):
> -#if defined(CONFIG_NET_RX_BUSY_POLL)
> +#if defined(CONFIG_SKB_MARK_NAPI_ID)
>   		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->src_reg,
>   				      bpf_target_off(struct sk_buff, napi_id, 4,
>   						     target_size));
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index eb9fcc4b1fd1a..386a32cdc0cc8 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1581,7 +1581,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>   #ifdef CONFIG_NETWORK_SECMARK
>   	CHECK_SKB_FIELD(secmark);
>   #endif
> -#ifdef CONFIG_NET_RX_BUSY_POLL
> +#ifdef CONFIG_SKB_MARK_NAPI_ID
>   	CHECK_SKB_FIELD(napi_id);
>   #endif
>   #ifdef CONFIG_XPS
> 
> base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e

Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>