All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/rps: consolidate RPS dispatch into netif_rps() helpers
@ 2026-07-02 15:28 Jemmy Wong
  2026-07-07 15:48 ` [RESEND PATCH] net/core: " Jemmy Wong
  2026-07-10  9:59 ` [PATCH] net/rps: " Paolo Abeni
  0 siblings, 2 replies; 5+ messages in thread
From: Jemmy Wong @ 2026-07-02 15:28 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev
  Cc: Simon Horman, Andrew Lunn, Kuniyuki Iwashima, Stanislav Fomichev,
	Samiullah Khawaja, Hangbin Liu, Krishna Kumar, Jemmy Wong,
	linux-kernel

From: "Jemmy Wong" <jemmywong512@gmail.com>

The RPS steering logic in netif_rx_internal(), netif_receive_skb_internal()
and netif_receive_skb_list_internal() was open-coded three times, each with
its own #ifdef CONFIG_RPS block and manual rcu_read_lock()/unlock() pairs.

Factor it into two helpers, netif_rps() for the single-skb path and
netif_rps_list() for the list path, and switch the callers to
guard(rcu)/scoped_guard(rcu). A new internal NET_RX_UNHANDLED sentinel lets
a helper report "RPS did not take this skb" so the caller falls back to the
local enqueue / __netif_receive_skb() path; it never escapes to callers.

netif_rps_list() keeps the early static_branch_unlikely(&rps_needed) bail
out so the list is not needlessly walked and re-spliced when RPS is
compiled in but disabled.

No functional change intended.

Signed-off-by: Jemmy Wong <jemmywong512@gmail.com>
---
 include/linux/netdevice.h |  5 ++-
 net/core/dev.c            | 94 +++++++++++++++++++--------------------
 2 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9981d637f8b5..c265b78082e3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -93,8 +93,9 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 void netdev_sw_irq_coalesce_default_on(struct net_device *dev);

 /* Backlog congestion levels */
-#define NET_RX_SUCCESS		0	/* keep 'em coming, baby */
-#define NET_RX_DROP		1	/* packet dropped */
+#define NET_RX_UNHANDLED	-1
+#define NET_RX_SUCCESS		0
+#define NET_RX_DROP		1

 #define MAX_NEST_DEV 8

diff --git a/net/core/dev.c b/net/core/dev.c
index 4b3d5cfdf6e0..259f8c8e5657 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5426,6 +5426,38 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
 	return NET_RX_DROP;
 }

+static inline int netif_rps(struct sk_buff *skb)
+{
+#ifdef CONFIG_RPS
+	if (static_branch_unlikely(&rps_needed)) {
+		struct rps_dev_flow voidflow, *rflow = &voidflow;
+		int cpu = get_rps_cpu(skb->dev, skb, &rflow);
+
+		if (cpu >= 0)
+			return enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
+	}
+#endif
+	return NET_RX_UNHANDLED;
+}
+
+static inline void netif_rps_list(struct list_head *head)
+{
+#ifdef CONFIG_RPS
+	struct sk_buff *skb, *next;
+	LIST_HEAD(undo_list);
+
+	if (!static_branch_unlikely(&rps_needed))
+		return;
+
+	list_for_each_entry_safe(skb, next, head, list) {
+		skb_list_del_init(skb);
+		if (netif_rps(skb) == NET_RX_UNHANDLED)
+			list_add_tail(&skb->list, &undo_list);
+	}
+	list_splice_init(&undo_list, head);
+#endif
+}
+
 static struct netdev_rx_queue *netif_get_rxqueue(struct sk_buff *skb)
 {
 	struct net_device *dev = skb->dev;
@@ -5695,33 +5727,20 @@ EXPORT_SYMBOL_GPL(do_xdp_generic);

 static int netif_rx_internal(struct sk_buff *skb)
 {
-	int ret;
+	int ret = NET_RX_UNHANDLED;
+	unsigned int qtail;

 	net_timestamp_check(READ_ONCE(net_hotdata.tstamp_prequeue), skb);

 	trace_netif_rx(skb);

-#ifdef CONFIG_RPS
-	if (static_branch_unlikely(&rps_needed)) {
-		struct rps_dev_flow voidflow, *rflow = &voidflow;
-		int cpu;
-
-		rcu_read_lock();
-
-		cpu = get_rps_cpu(skb->dev, skb, &rflow);
-		if (cpu < 0)
-			cpu = smp_processor_id();
+	scoped_guard(rcu)
+		ret = netif_rps(skb);
+	if (ret != NET_RX_UNHANDLED)
+		return ret;

-		ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
+	ret = enqueue_to_backlog(skb, smp_processor_id(), &qtail);

-		rcu_read_unlock();
-	} else
-#endif
-	{
-		unsigned int qtail;
-
-		ret = enqueue_to_backlog(skb, smp_processor_id(), &qtail);
-	}
 	return ret;
 }

@@ -6389,21 +6408,12 @@ static int netif_receive_skb_internal(struct sk_buff *skb)
 	if (skb_defer_rx_timestamp(skb))
 		return NET_RX_SUCCESS;

-	rcu_read_lock();
-#ifdef CONFIG_RPS
-	if (static_branch_unlikely(&rps_needed)) {
-		struct rps_dev_flow voidflow, *rflow = &voidflow;
-		int cpu = get_rps_cpu(skb->dev, skb, &rflow);
+	guard(rcu)();
+	ret = netif_rps(skb);
+	if (ret != NET_RX_UNHANDLED)
+		return ret;

-		if (cpu >= 0) {
-			ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
-			rcu_read_unlock();
-			return ret;
-		}
-	}
-#endif
 	ret = __netif_receive_skb(skb);
-	rcu_read_unlock();
 	return ret;
 }

@@ -6421,23 +6431,9 @@ void netif_receive_skb_list_internal(struct list_head *head)
 	}
 	list_splice_init(&sublist, head);

-	rcu_read_lock();
-#ifdef CONFIG_RPS
-	if (static_branch_unlikely(&rps_needed)) {
-		list_for_each_entry_safe(skb, next, head, list) {
-			struct rps_dev_flow voidflow, *rflow = &voidflow;
-			int cpu = get_rps_cpu(skb->dev, skb, &rflow);
-
-			if (cpu >= 0) {
-				/* Will be handled, remove from list */
-				skb_list_del_init(skb);
-				enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
-			}
-		}
-	}
-#endif
+	guard(rcu)();
+	netif_rps_list(head);
 	__netif_receive_skb_list(head);
-	rcu_read_unlock();
 }

 /**
--
2.25.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RESEND PATCH] net/core: consolidate RPS dispatch into netif_rps() helpers
  2026-07-02 15:28 [PATCH] net/rps: consolidate RPS dispatch into netif_rps() helpers Jemmy Wong
@ 2026-07-07 15:48 ` Jemmy Wong
  2026-07-20 23:25   ` Jakub Kicinski
  2026-07-10  9:59 ` [PATCH] net/rps: " Paolo Abeni
  1 sibling, 1 reply; 5+ messages in thread
From: Jemmy Wong @ 2026-07-07 15:48 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, andrew+netdev, davem, edumazet, kuba, pabeni, horms,
	Jemmy Wong

From: "Jemmy Wong" <jemmywong512@gmail.com>

The RPS steering logic in netif_rx_internal(), netif_receive_skb_internal()
and netif_receive_skb_list_internal() was open-coded three times, each with
its own #ifdef CONFIG_RPS block and manual rcu_read_lock()/unlock() pairs.

Factor it into two helpers, netif_rps() for the single-skb path and
netif_rps_list() for the list path, and switch the callers to
guard(rcu)/scoped_guard(rcu). A new internal NET_RX_UNHANDLED sentinel lets
a helper report "RPS did not take this skb" so the caller falls back to the
local enqueue / __netif_receive_skb() path; it never escapes to callers.

netif_rps_list() keeps the early static_branch_unlikely(&rps_needed) bail
out so the list is not needlessly walked and re-spliced when RPS is
compiled in but disabled.

No functional change intended.

Signed-off-by: Jemmy Wong <jemmywong512@gmail.com>
---
 include/linux/netdevice.h |  5 ++-
 net/core/dev.c            | 94 +++++++++++++++++++--------------------
 2 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9981d637f8b5..c265b78082e3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -93,8 +93,9 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 void netdev_sw_irq_coalesce_default_on(struct net_device *dev);

 /* Backlog congestion levels */
-#define NET_RX_SUCCESS		0	/* keep 'em coming, baby */
-#define NET_RX_DROP		1	/* packet dropped */
+#define NET_RX_UNHANDLED	-1
+#define NET_RX_SUCCESS		0
+#define NET_RX_DROP		1

 #define MAX_NEST_DEV 8

diff --git a/net/core/dev.c b/net/core/dev.c
index 4b3d5cfdf6e0..259f8c8e5657 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5426,6 +5426,38 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
 	return NET_RX_DROP;
 }

+static inline int netif_rps(struct sk_buff *skb)
+{
+#ifdef CONFIG_RPS
+	if (static_branch_unlikely(&rps_needed)) {
+		struct rps_dev_flow voidflow, *rflow = &voidflow;
+		int cpu = get_rps_cpu(skb->dev, skb, &rflow);
+
+		if (cpu >= 0)
+			return enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
+	}
+#endif
+	return NET_RX_UNHANDLED;
+}
+
+static inline void netif_rps_list(struct list_head *head)
+{
+#ifdef CONFIG_RPS
+	struct sk_buff *skb, *next;
+	LIST_HEAD(undo_list);
+
+	if (!static_branch_unlikely(&rps_needed))
+		return;
+
+	list_for_each_entry_safe(skb, next, head, list) {
+		skb_list_del_init(skb);
+		if (netif_rps(skb) == NET_RX_UNHANDLED)
+			list_add_tail(&skb->list, &undo_list);
+	}
+	list_splice_init(&undo_list, head);
+#endif
+}
+
 static struct netdev_rx_queue *netif_get_rxqueue(struct sk_buff *skb)
 {
 	struct net_device *dev = skb->dev;
@@ -5695,33 +5727,20 @@ EXPORT_SYMBOL_GPL(do_xdp_generic);

 static int netif_rx_internal(struct sk_buff *skb)
 {
-	int ret;
+	int ret = NET_RX_UNHANDLED;
+	unsigned int qtail;

 	net_timestamp_check(READ_ONCE(net_hotdata.tstamp_prequeue), skb);

 	trace_netif_rx(skb);

-#ifdef CONFIG_RPS
-	if (static_branch_unlikely(&rps_needed)) {
-		struct rps_dev_flow voidflow, *rflow = &voidflow;
-		int cpu;
-
-		rcu_read_lock();
-
-		cpu = get_rps_cpu(skb->dev, skb, &rflow);
-		if (cpu < 0)
-			cpu = smp_processor_id();
+	scoped_guard(rcu)
+		ret = netif_rps(skb);
+	if (ret != NET_RX_UNHANDLED)
+		return ret;

-		ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
+	ret = enqueue_to_backlog(skb, smp_processor_id(), &qtail);

-		rcu_read_unlock();
-	} else
-#endif
-	{
-		unsigned int qtail;
-
-		ret = enqueue_to_backlog(skb, smp_processor_id(), &qtail);
-	}
 	return ret;
 }

@@ -6389,21 +6408,12 @@ static int netif_receive_skb_internal(struct sk_buff *skb)
 	if (skb_defer_rx_timestamp(skb))
 		return NET_RX_SUCCESS;

-	rcu_read_lock();
-#ifdef CONFIG_RPS
-	if (static_branch_unlikely(&rps_needed)) {
-		struct rps_dev_flow voidflow, *rflow = &voidflow;
-		int cpu = get_rps_cpu(skb->dev, skb, &rflow);
+	guard(rcu)();
+	ret = netif_rps(skb);
+	if (ret != NET_RX_UNHANDLED)
+		return ret;

-		if (cpu >= 0) {
-			ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
-			rcu_read_unlock();
-			return ret;
-		}
-	}
-#endif
 	ret = __netif_receive_skb(skb);
-	rcu_read_unlock();
 	return ret;
 }

@@ -6421,23 +6431,9 @@ void netif_receive_skb_list_internal(struct list_head *head)
 	}
 	list_splice_init(&sublist, head);

-	rcu_read_lock();
-#ifdef CONFIG_RPS
-	if (static_branch_unlikely(&rps_needed)) {
-		list_for_each_entry_safe(skb, next, head, list) {
-			struct rps_dev_flow voidflow, *rflow = &voidflow;
-			int cpu = get_rps_cpu(skb->dev, skb, &rflow);
-
-			if (cpu >= 0) {
-				/* Will be handled, remove from list */
-				skb_list_del_init(skb);
-				enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
-			}
-		}
-	}
-#endif
+	guard(rcu)();
+	netif_rps_list(head);
 	__netif_receive_skb_list(head);
-	rcu_read_unlock();
 }

 /**
--
2.25.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/rps: consolidate RPS dispatch into netif_rps() helpers
  2026-07-02 15:28 [PATCH] net/rps: consolidate RPS dispatch into netif_rps() helpers Jemmy Wong
  2026-07-07 15:48 ` [RESEND PATCH] net/core: " Jemmy Wong
@ 2026-07-10  9:59 ` Paolo Abeni
  1 sibling, 0 replies; 5+ messages in thread
From: Paolo Abeni @ 2026-07-10  9:59 UTC (permalink / raw)
  To: Jemmy Wong, David S . Miller, Eric Dumazet, Jakub Kicinski,
	netdev
  Cc: Simon Horman, Andrew Lunn, Kuniyuki Iwashima, Stanislav Fomichev,
	Samiullah Khawaja, Hangbin Liu, Krishna Kumar, linux-kernel

On 7/2/26 5:28 PM, Jemmy Wong wrote:
> From: "Jemmy Wong" <jemmywong512@gmail.com>
> 
> The RPS steering logic in netif_rx_internal(), netif_receive_skb_internal()
> and netif_receive_skb_list_internal() was open-coded three times, each with
> its own #ifdef CONFIG_RPS block and manual rcu_read_lock()/unlock() pairs.
> 
> Factor it into two helpers, netif_rps() for the single-skb path and
> netif_rps_list() for the list path, and switch the callers to
> guard(rcu)/scoped_guard(rcu). 

Please be aware of:

https://elixir.bootlin.com/linux/v7.1.2/source/Documentation/process/maintainer-netdev.rst#L400

> @@ -5695,33 +5727,20 @@ EXPORT_SYMBOL_GPL(do_xdp_generic);
> 
>  static int netif_rx_internal(struct sk_buff *skb)
>  {
> -	int ret;
> +	int ret = NET_RX_UNHANDLED;
> +	unsigned int qtail;
> 
>  	net_timestamp_check(READ_ONCE(net_hotdata.tstamp_prequeue), skb);
> 
>  	trace_netif_rx(skb);
> 
> -#ifdef CONFIG_RPS
> -	if (static_branch_unlikely(&rps_needed)) {
> -		struct rps_dev_flow voidflow, *rflow = &voidflow;
> -		int cpu;
> -
> -		rcu_read_lock();
> -
> -		cpu = get_rps_cpu(skb->dev, skb, &rflow);
> -		if (cpu < 0)
> -			cpu = smp_processor_id();
> +	scoped_guard(rcu)
> +		ret = netif_rps(skb);
> +	if (ret != NET_RX_UNHANDLED)
> +		return ret;

This function is performance critical and RCU lock is not a no-op. I
*think* this will add an unneeded rcp barrier when RPS is compile
enabled and `static_branch_unlikely(&rps_needed)` evaluate to false.

At very least you should prove that the generated code is no worse than
the current one.

/P


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RESEND PATCH] net/core: consolidate RPS dispatch into netif_rps() helpers
  2026-07-07 15:48 ` [RESEND PATCH] net/core: " Jemmy Wong
@ 2026-07-20 23:25   ` Jakub Kicinski
  2026-07-21 16:05     ` Jemmy Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2026-07-20 23:25 UTC (permalink / raw)
  To: Jemmy Wong
  Cc: netdev, linux-kernel, andrew+netdev, davem, edumazet, pabeni,
	horms

On Tue,  7 Jul 2026 23:48:55 +0800 Jemmy Wong wrote:
> The RPS steering logic in netif_rx_internal(), netif_receive_skb_internal()
> and netif_receive_skb_list_internal() was open-coded three times, each with
> its own #ifdef CONFIG_RPS block and manual rcu_read_lock()/unlock() pairs.
> 
> Factor it into two helpers, netif_rps() for the single-skb path and
> netif_rps_list() for the list path, and switch the callers to
> guard(rcu)/scoped_guard(rcu). A new internal NET_RX_UNHANDLED sentinel lets
> a helper report "RPS did not take this skb" so the caller falls back to the
> local enqueue / __netif_receive_skb() path; it never escapes to callers.
> 
> netif_rps_list() keeps the early static_branch_unlikely(&rps_needed) bail
> out so the list is not needlessly walked and re-spliced when RPS is
> compiled in but disabled.
> 
> No functional change intended.

You haven't read Paolo's reply, please go away.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RESEND PATCH] net/core: consolidate RPS dispatch into netif_rps() helpers
  2026-07-20 23:25   ` Jakub Kicinski
@ 2026-07-21 16:05     ` Jemmy Wong
  0 siblings, 0 replies; 5+ messages in thread
From: Jemmy Wong @ 2026-07-21 16:05 UTC (permalink / raw)
  To: Jakub Kicinski, pabeni
  Cc: Jemmy Wong, netdev, linux-kernel, andrew+netdev, davem, edumazet,
	horms


> On Jul 21, 2026, at 7:25 AM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Tue,  7 Jul 2026 23:48:55 +0800 Jemmy Wong wrote:
>> The RPS steering logic in netif_rx_internal(), netif_receive_skb_internal()
>> and netif_receive_skb_list_internal() was open-coded three times, each with
>> its own #ifdef CONFIG_RPS block and manual rcu_read_lock()/unlock() pairs.
>> 
>> Factor it into two helpers, netif_rps() for the single-skb path and
>> netif_rps_list() for the list path, and switch the callers to
>> guard(rcu)/scoped_guard(rcu). A new internal NET_RX_UNHANDLED sentinel lets
>> a helper report "RPS did not take this skb" so the caller falls back to the
>> local enqueue / __netif_receive_skb() path; it never escapes to callers.
>> 
>> netif_rps_list() keeps the early static_branch_unlikely(&rps_needed) bail
>> out so the list is not needlessly walked and re-spliced when RPS is
>> compiled in but disabled.
>> 
>> No functional change intended.
> 
> You haven't read Paolo's reply, please go away.

Hi Jakub,

Apologies for the confusion — I am new to the netdev subsystem and
may have handled the process incorrectly.

I did read Paolo's reply and sent v2 [1] the following day to address
his two concerns:

 1. Dropped guard()/scoped_guard() in favour of explicit
    rcu_read_lock()/rcu_read_unlock() per the cleanup.h guidance [2].

 2. Moved the RCU section inside the static_branch_unlikely() check so
    the hot path (!rps_needed) acquires no lock. Verified with objdump
    and bloat-o-meter that the fast path is instruction-identical to base.

I should have replied here first before posting v2 — apologies for
that process mistake.

After re-reading the maintainer-netdev.rst guide I also noticed:

 - v2 subject is missing the target tree prefix (should be
   "[PATCH net-next v2]").

 - The patch is a standalone refactoring with no functional change,
   which the guide discourages [3]. If it does not carry enough value
   on its own, I am happy to drop it or fold it into a future
   functional change.

[1] https://lore.kernel.org/all/20260711121009.76842-1-jemmywong512@gmail.com/
[2] https://elixir.bootlin.com/linux/v7.1.2/source/Documentation/process/maintainer-netdev.rst#L400
[3] https://elixir.bootlin.com/linux/v7.1.2/source/Documentation/process/maintainer-netdev.rst#L416

Thanks,
Jemmy Wong


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-21 16:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02 15:28 [PATCH] net/rps: consolidate RPS dispatch into netif_rps() helpers Jemmy Wong
2026-07-07 15:48 ` [RESEND PATCH] net/core: " Jemmy Wong
2026-07-20 23:25   ` Jakub Kicinski
2026-07-21 16:05     ` Jemmy Wong
2026-07-10  9:59 ` [PATCH] net/rps: " Paolo Abeni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.