Re: [PATCH] net/rps: consolidate RPS dispatch into netif_rps() helpers

[Paolo Abeni]

On 7/2/26 5:28 PM, Jemmy Wong wrote:
> From: "Jemmy Wong" <jemmywong512@xxxxxxxxx>
> 
> 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