On Tue, Mar 28, 2023 at 9:59 PM Felix Fietkau <nbd@xxxxxxxx> wrote:
>
> When dealing with few flows or an imbalance on CPU utilization, static RPS
> CPU assignment can be too inflexible. Add support for enabling threaded NAPI
> for backlog processing in order to allow the scheduler to better balance
> processing. This helps better spread the load across idle CPUs.
>
> Signed-off-by: Felix Fietkau <nbd@xxxxxxxx>
> ---
> v2:
> - initialize sd->backlog.poll_list in order fix switching backlogs to threaded
> that have not been scheduled before
> PATCH:
> - add missing process_queue_empty initialization
> - fix kthread leak
> - add documentation
> RFC v3:
> - make patch more generic, applies to backlog processing in general
> - fix process queue access on flush
> RFC v2:
> - fix rebase error in rps locking
> Documentation/admin-guide/sysctl/net.rst | 9 +++
> Documentation/networking/scaling.rst | 20 ++++++
> include/linux/netdevice.h | 2 +
> net/core/dev.c | 83 ++++++++++++++++++++++--
> net/core/sysctl_net_core.c | 27 ++++++++
> 5 files changed, 136 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> index 466c560b0c30..6d037633a52f 100644
> --- a/Documentation/admin-guide/sysctl/net.rst
> +++ b/Documentation/admin-guide/sysctl/net.rst
> @@ -47,6 +47,15 @@ Table : Subdirectories in /proc/sys/net
> 1. /proc/sys/net/core - Network core options
> ============================================
>
> +backlog_threaded
> +----------------
> +
> +This offloads processing of backlog (input packets steered by RPS, or
> +queued because the kernel is receiving more than it can handle on the
> +incoming CPU) to threads (one for each CPU) instead of processing them
> +in softirq context. This can improve load balancing by allowing the
> +scheduler to better spread the load across idle CPUs.
> +
> bpf_jit_enable
> --------------
>
> diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
> index 3d435caa3ef2..ded6fc713304 100644
> --- a/Documentation/networking/scaling.rst
> +++ b/Documentation/networking/scaling.rst
> @@ -244,6 +244,26 @@ Setting net.core.netdev_max_backlog to either 1000 or 10000
> performed well in experiments.
>
>
> +Threaded Backlog
> +~~~~~~~~~~~~~~~~
> +
> +When dealing with few flows or an imbalance on CPU utilization, static
> +RPS CPU assignment can be too inflexible. Making backlog processing
> +threaded can improve load balancing by allowing the scheduler to spread
> +the load across idle CPUs.
> +
> +
> +Suggested Configuration
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +If you have CPUs fully utilized with network processing, you can enable
> +threaded backlog processing by setting /proc/sys/net/core/backlog_threaded
> +to 1. Afterwards, RPS CPU configuration bits no longer refer to CPU
> +numbers, but to backlog threads named napi/backlog-<n>.
> +If necessary, you can change the CPU affinity of these threads to limit
> +them to specific CPU cores.
> +
> +
> RFS: Receive Flow Steering
> ==========================
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 18a5be6ddd0f..953876cb0e92 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -527,6 +527,7 @@ static inline bool napi_complete(struct napi_struct *n)
> }
>
> int dev_set_threaded(struct net_device *dev, bool threaded);
> +int backlog_set_threaded(bool threaded);
>
> /**
> * napi_disable - prevent NAPI from scheduling
> @@ -3217,6 +3218,7 @@ struct softnet_data {
> unsigned int cpu;
> unsigned int input_queue_tail;
> #endif
> + unsigned int process_queue_empty;
Hmmm... probably better close to input_queue_head, to share a
dedicated cache line already dirtied
with input_queue_head_incr()
I also think we could avoid adding this new field.
Use instead input_queue_head, latching its value in void
flush_backlog() and adding sd->process_queue length ?
Then waiting for (s32)(input_queue_head - latch) >= 0 ?
> /*
> * Inline a custom version of __napi_complete().
> - * only current cpu owns and manipulates this napi,
> - * and NAPI_STATE_SCHED is the only possible flag set
> - * on backlog.
> + * only current cpu owns and manipulates this napi.
> * We can use a plain write instead of clear_bit(),
> * and we dont need an smp_mb() memory barrier.
> */
> - napi->state = 0;
> + napi->state &= ~(NAPIF_STATE_SCHED |
> + NAPIF_STATE_SCHED_THREADED);
> again = false;
> } else {
> skb_queue_splice_tail_init(&sd->input_pkt_queue,
> @@ -6350,6 +6370,55 @@ int dev_set_threaded(struct net_device *dev, bool threaded)
> }
> EXPORT_SYMBOL(dev_set_threaded);
>
> +int backlog_set_threaded(bool threaded)
> +{
> + static bool backlog_threaded;
> + int err = 0;
> + int i;
> +
> + if (backlog_threaded == threaded)
> + return 0;
> +
> + for_each_possible_cpu(i) {
> + struct softnet_data *sd = &per_cpu(softnet_data, i);
> + struct napi_struct *n = &sd->backlog;
> +
> + if (n->thread)
> + continue;
> + n->thread = kthread_run(napi_threaded_poll, n, "napi/backlog-%d", i);
> + if (IS_ERR(n->thread)) {
> + err = PTR_ERR(n->thread);
> + pr_err("kthread_run failed with err %d\n", err);
> + n->thread = NULL;
> + threaded = false;
> + break;
> + }
> +
> + }
> +
> + backlog_threaded = threaded;
> +
> + /* Make sure kthread is created before THREADED bit
> + * is set.
> + */
> + smp_mb__before_atomic();
> +
> + for_each_possible_cpu(i) {
> + struct softnet_data *sd = &per_cpu(softnet_data, i);
> + struct napi_struct *n = &sd->backlog;
> + unsigned long flags;
> +
> + rps_lock_irqsave(sd, &flags);
> + if (threaded)
> + n->state |= NAPIF_STATE_THREADED;
> + else
> + n->state &= ~NAPIF_STATE_THREADED;
> + rps_unlock_irq_restore(sd, &flags);
> + }
> +
> + return err;
> +}
> +
> void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
> int (*poll)(struct napi_struct *, int), int weight)
> {
> @@ -11108,6 +11177,9 @@ static int dev_cpu_dead(unsigned int oldcpu)
> raise_softirq_irqoff(NET_TX_SOFTIRQ);
> local_irq_enable();
>
> + if (test_bit(NAPI_STATE_THREADED, &oldsd->backlog.state))
> + return 0;
> +
> #ifdef CONFIG_RPS
> remsd = oldsd->rps_ipi_list;
> oldsd->rps_ipi_list = NULL;
> @@ -11411,6 +11483,7 @@ static int __init net_dev_init(void)
> INIT_CSD(&sd->defer_csd, trigger_rx_softirq, sd);
> spin_lock_init(&sd->defer_lock);
>
> + INIT_LIST_HEAD(&sd->backlog.poll_list);
> init_gro_hash(&sd->backlog);
> sd->backlog.poll = process_backlog;
> sd->backlog.weight = weight_p;
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index 74842b453407..77114cd0b021 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -30,6 +30,7 @@ static int int_3600 = 3600;
> static int min_sndbuf = SOCK_MIN_SNDBUF;
> static int min_rcvbuf = SOCK_MIN_RCVBUF;
> static int max_skb_frags = MAX_SKB_FRAGS;
> +static int backlog_threaded;
>
> static int net_msg_warn; /* Unused, but still a sysctl */
>
> @@ -188,6 +189,23 @@ static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
> }
> #endif /* CONFIG_RPS */
>
> +static int backlog_threaded_sysctl(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos)
> +{
> + static DEFINE_MUTEX(backlog_threaded_mutex);
> + int ret;
> +
> + mutex_lock(&backlog_threaded_mutex);
> +
> + ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> + if (write && !ret)
> + ret = backlog_set_threaded(backlog_threaded);
> +
> + mutex_unlock(&backlog_threaded_mutex);
> +
> + return ret;
> +}
> +
> #ifdef CONFIG_NET_FLOW_LIMIT
> static DEFINE_MUTEX(flow_limit_update_mutex);
>
> @@ -532,6 +550,15 @@ static struct ctl_table net_core_table[] = {
> .proc_handler = rps_sock_flow_sysctl
> },
> #endif
> + {
> + .procname = "backlog_threaded",
> + .data = &backlog_threaded,
> + .maxlen = sizeof(unsigned int),
> + .mode = 0644,
> + .proc_handler = backlog_threaded_sysctl,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = SYSCTL_ONE
> + },
> #ifdef CONFIG_NET_FLOW_LIMIT
> {
> .procname = "flow_limit_cpu_bitmap",
> --
> 2.39.0
>
