Re: [PATCH] rps: process the skb directly if rps cpu not changed

From: Jakub Kicinski
Date: Tue Mar 21 2023 - 22:55:08 EST


On Tue, 21 Mar 2023 20:12:29 +0800 (CST) yang.yang29@xxxxxxxxxx wrote:
> The measured result shows the patch brings 50% reduction of NET_RX softirqs.
> The test was done on the QEMU environment with two-core CPU by iperf3.
> taskset 01 iperf3 -c 192.168.2.250 -t 3 -u -R;
> taskset 02 iperf3 -c 192.168.2.250 -t 3 -u -R;
>
> Previous RPS:
> CPU0 CPU1

this header looks misalinged

> NET_RX: 45 0 (before iperf3 testing)
> NET_RX: 1095 241 (after iperf3 testing)
>
> Patched RPS:
> CPU0 CPU1
> NET_RX: 28 4 (before iperf3 testing)
> NET_RX: 573 32 (after iperf3 testing)

This table is really confusing. What's the unit, how is it measured
and why are you showing before/after rather than the delta?

> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7853192563d..c33ddac3c012 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5666,8 +5666,9 @@ static int netif_receive_skb_internal(struct sk_buff *skb)
> if (static_branch_unlikely(&rps_needed)) {
> struct rps_dev_flow voidflow, *rflow = &voidflow;
> int cpu = get_rps_cpu(skb->dev, skb, &rflow);
> + int current_cpu = smp_processor_id();
>
> - if (cpu >= 0) {
> + if (cpu >= 0 && cpu != current_cpu) {
> ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
> rcu_read_unlock();
> return ret;
> @@ -5699,8 +5700,9 @@ void netif_receive_skb_list_internal(struct list_head *head)
> 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);
> + int current_cpu = smp_processor_id();

This does not have to be in the loop.

>
> - if (cpu >= 0) {
> + if (cpu >= 0 && cpu != current_cpu) {

Please answer Yunsheng's question as well..