[RFC,net-next,1/2] net: Use SMP threads for backlog NAPI.

  Backlog NAPI is a per-CPU NAPI struct only (with no device behind it)
used by drivers which don't do NAPI them self and RPS.
The non-NAPI driver use the CPU local backlog NAPI. If RPS is enabled
then a flow for the skb is computed and based on the flow the skb can be
enqueued on a remote CPU. Scheduling/ raising the softirq (for backlog's
NAPI) on the remote CPU isn't trivial because the softirq is only
scheduled on the local CPU and performed after the hardirq is done.
In order to schedule a softirq on the remote CPU, an IPI is sent to the
remote CPU which schedules the backlog-NAPI on the then local CPU.

On PREEMPT_RT interrupts are force-threaded. The soft interrupts are
raised within the interrupt thread and processed after the interrupt
handler completed still within the context of the interrupt thread. The
softirq is handled in the context where it originated.

With force-threaded interrupts enabled, ksoftirqd is woken up if a
softirq is raised from hardirq context. This is the case if it is raised
from an IPI. Additionally there is a warning on PREEMPT_RT if the
softirq is raised from the idle thread.
This was done for two reasons:
- With threaded interrupts the processing should happen in thread
  context (where it originated) and ksoftirqd is the only thread for
  this context if raised from hardirq. Using the currently running task
  instead would "punish" a random task.
- Once ksoftirqd is active it consumes all further softirqs until it
  stops running. This changed recently and is no longer the case.

Instead of keeping the backlog NAPI in ksoftirqd (in force-threaded/
PREEMPT_RT setups) I am proposing NAPI-threads for backlog.
The "proper" setup with threaded-NAPI is not doable because the threads
are not pinned to an individual CPU and can be modified by the user.
Additionally a dummy network device would have to be assigned. Also
CPU-hotplug has to be considered if additional CPUs show up.
All this can be probably done/ solved but the smpboot-threads already
provide this infrastructure.

Create NAPI-threads for backlog. The thread runs the inner loop from
napi_threaded_poll(), the wait part is different.
Since there are now per-CPU threads for backlog the remote IPI for
signaling is not needed and can be removed. The NAPI for backlog can
always be scheduled as it ends in waking the corresponding thread.
Since "deferred skb free" use a similar IPI mechanism for signaling, it
is also using the backlog threads.

This makes NAPI threads mandatory for backlog and it can not be
disabled. The other visibile part with RPS (or backlog usage in general)
is that it becomes now visible in `top' while earlier it would remain
unaccounted.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/netdevice.h |   8 --
 net/core/dev.c            | 226 +++++++++++++-------------------------
 net/core/net-procfs.c     |   2 +-
 net/core/skbuff.c         |   2 +-
 4 files changed, 79 insertions(+), 159 deletions(-)
  

On 2023-08-23 15:35:41 [+0200], Paolo Abeni wrote:
> On Mon, 2023-08-14 at 11:35 +0200, Sebastian Andrzej Siewior wrote:
> > @@ -4781,7 +4733,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> >  		 * We can use non atomic operation since we own the queue lock
> >  		 */
> >  		if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
> > -			napi_schedule_rps(sd);
> > +			__napi_schedule_irqoff(&sd->backlog);
> >  		goto enqueue;
> >  	}
> >  	reason = SKB_DROP_REASON_CPU_BACKLOG;
> 
> I *think* that the above could be quite dangerous when cpu ==
> smp_processor_id() - that is, with plain veth usage.
> 
> Currently, each packet runs into the rx path just after
> enqueue_to_backlog()/tx completes.
> 
> With this patch there will be a burst effect, where the backlog thread
> will run after a few (several) packets will be enqueued, when the
> process scheduler will decide - note that the current CPU is already
> hosting a running process, the tx thread.
> 
> The above can cause packet drops (due to limited buffering) or very
> high latency (due to long burst), even in non overload situation, quite
> hard to debug.
> 
> I think the above needs to be an opt-in, but I guess that even RT
> deployments doing some packet forwarding will not be happy with this
> on.

I've been looking at this again and have been thinking what you said
here. I think part of the problem is that we lack a policy/ mechanism
when a DoS is happening and what to do.

Before commit d15121be74856 ("Revert "softirq: Let ksoftirqd do its
job"") when a lot of network packets are processed then processing is
moved to ksoftirqd and continues based on how the scheduler schedules
the SCHED_OTHER ksoftirqd task. This avoids lock-ups of the system and
it can do something else in between. Any interrupt will not continue the
outstanding softirq backlog but wait for ksoftirqd. So it basically
avoids the networking overload. It throttles the throughput if needed.

This isn't the case after that commit. Now, the CPU can be stuck with
processing networking packets if the packets come in fast enough. Even
if ksoftirqd is woken up, the next interrupt (say the timer) will
continue with at least one round.
By using NAPI-threads it is possible to give the control back to the
scheduler which can throttle the NAPI processing in favour of other
threads that ask for CPU. As you pointed out, waking the thread does not
guarantee that it will immediately do the NAPI work. It can be delayed
based on current load on the system.

This could be influenced by assigning the NAPI-thread a SCHED_FIFO
priority. Based on the priority it could be ensured that the thread
starts right away or "later" if something else is more important.
However, this opens the DoS window again: The scheduler will put the
NAPI thread on CPU as long as it asks for it with no throttling.

If we could somehow define a DoS condition once we are overwhelmed with
packets, then we could act on it and throttle it. This in turn would
allow a SCHED_FIFO priority without the fear of a lockup if the system
is flooded with packets.

> Cheers,
> 
> Paolo

Sebastian
  

Hi!

On Wed, 2023-09-20 at 17:57 +0200, Sebastian Andrzej Siewior wrote:
> On 2023-08-23 15:35:41 [+0200], Paolo Abeni wrote:
> > On Mon, 2023-08-14 at 11:35 +0200, Sebastian Andrzej Siewior wrote:
> > > @@ -4781,7 +4733,7 @@ static int enqueue_to_backlog(struct
> > > sk_buff *skb, int cpu,
> > >  		 * We can use non atomic operation since we own
> > > the queue lock
> > >  		 */
> > >  		if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd-
> > > >backlog.state))
> > > -			napi_schedule_rps(sd);
> > > +			__napi_schedule_irqoff(&sd->backlog);
> > >  		goto enqueue;
> > >  	}
> > >  	reason = SKB_DROP_REASON_CPU_BACKLOG;
> > 
> > I *think* that the above could be quite dangerous when cpu ==
> > smp_processor_id() - that is, with plain veth usage.
> > 
> > Currently, each packet runs into the rx path just after
> > enqueue_to_backlog()/tx completes.
> > 
> > With this patch there will be a burst effect, where the backlog
> > thread
> > will run after a few (several) packets will be enqueued, when the
> > process scheduler will decide - note that the current CPU is
> > already
> > hosting a running process, the tx thread.
> > 
> > The above can cause packet drops (due to limited buffering) or very
> > high latency (due to long burst), even in non overload situation,
> > quite
> > hard to debug.
> > 
> > I think the above needs to be an opt-in, but I guess that even RT
> > deployments doing some packet forwarding will not be happy with
> > this
> > on.
> 
> I've been looking at this again and have been thinking what you said
> here. I think part of the problem is that we lack a policy/ mechanism
> when a DoS is happening and what to do.
> 
> Before commit d15121be74856 ("Revert "softirq: Let ksoftirqd do its
> job"") when a lot of network packets are processed then processing is
> moved to ksoftirqd and continues based on how the scheduler schedules
> the SCHED_OTHER ksoftirqd task. This avoids lock-ups of the system
> and
> it can do something else in between. Any interrupt will not continue
> the
> outstanding softirq backlog but wait for ksoftirqd. So it basically
> avoids the networking overload. It throttles the throughput if
> needed.
> 
> This isn't the case after that commit. Now, the CPU can be stuck with
> processing networking packets if the packets come in fast enough.
> Even
> if ksoftirqd is woken up, the next interrupt (say the timer) will
> continue with at least one round.
> By using NAPI-threads it is possible to give the control back to the
> scheduler which can throttle the NAPI processing in favour of other
> threads that ask for CPU. As you pointed out, waking the thread does
> not
> guarantee that it will immediately do the NAPI work. It can be
> delayed
> based on current load on the system.
> 
> This could be influenced by assigning the NAPI-thread a SCHED_FIFO
> priority. Based on the priority it could be ensured that the thread
> starts right away or "later" if something else is more important.
> However, this opens the DoS window again: The scheduler will put the
> NAPI thread on CPU as long as it asks for it with no throttling.
> 
> If we could somehow define a DoS condition once we are overwhelmed
> with
> packets, then we could act on it and throttle it. This in turn would
> allow a SCHED_FIFO priority without the fear of a lockup if the
> system
> is flooded with packets.

Can this be avoided if we reuse gro_flush_timeout as the maximum time
the NAPI thread can be scheduled?

> 
> > Cheers,
> > 
> > Paolo
> 
> Sebastian

Ferenc
  

On 2023-09-21 12:41:33 [+0200], Ferenc Fejes wrote:
> Hi!
Hi,

> > If we could somehow define a DoS condition once we are overwhelmed
> > with
> > packets, then we could act on it and throttle it. This in turn would
> > allow a SCHED_FIFO priority without the fear of a lockup if the
> > system
> > is flooded with packets.
> 
> Can this be avoided if we reuse gro_flush_timeout as the maximum time
> the NAPI thread can be scheduled?

First your run time needs to be accounted somehow. I observed that some
cards/ systems tend pull often a few packets on each interrupt and
others pull more packets at a time.
So probably packets in a time frame would make sense. Maybe even plus
packet size assuming larger packets require more processing time.

If you run at SCHED_OTHER you don't care, you can keep it running. With
SCHED_FIFO you would need to decide:
- how much is too much
- what to do once you reach too much

Once you reach too much you could:
- change the scheduling policy to SCHED_OTHER and keep going until it is
  no longer "too much in a given period" so you can flip it back.

- stop processing for a period of time and risk packet loss which is
  defined as better than to continue.

- pulling packets and dropping them instead of injecting into the stack.
  Using xdp/ebpf might be easy since there is an API for that. One could
  even peek at packets to decide if some can be kept.
  This would rely on the fact that the system can do this quick enough
  under a DoS condition.

> 
> Ferenc

Sebastian
  

On Wed, 2023-09-20 at 17:57 +0200, Sebastian Andrzej Siewior wrote:
> On 2023-08-23 15:35:41 [+0200], Paolo Abeni wrote:
> > On Mon, 2023-08-14 at 11:35 +0200, Sebastian Andrzej Siewior wrote:
> > > @@ -4781,7 +4733,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
> > >  		 * We can use non atomic operation since we own the queue lock
> > >  		 */
> > >  		if (!__test_and_set_bit(NAPI_STATE_SCHED, &sd->backlog.state))
> > > -			napi_schedule_rps(sd);
> > > +			__napi_schedule_irqoff(&sd->backlog);
> > >  		goto enqueue;
> > >  	}
> > >  	reason = SKB_DROP_REASON_CPU_BACKLOG;
> > 
> > I *think* that the above could be quite dangerous when cpu ==
> > smp_processor_id() - that is, with plain veth usage.
> > 
> > Currently, each packet runs into the rx path just after
> > enqueue_to_backlog()/tx completes.
> > 
> > With this patch there will be a burst effect, where the backlog thread
> > will run after a few (several) packets will be enqueued, when the
> > process scheduler will decide - note that the current CPU is already
> > hosting a running process, the tx thread.
> > 
> > The above can cause packet drops (due to limited buffering) or very
> > high latency (due to long burst), even in non overload situation, quite
> > hard to debug.
> > 
> > I think the above needs to be an opt-in, but I guess that even RT
> > deployments doing some packet forwarding will not be happy with this
> > on.
> 
> I've been looking at this again and have been thinking what you said
> here. I think part of the problem is that we lack a policy/ mechanism
> when a DoS is happening and what to do.
> 
> Before commit d15121be74856 ("Revert "softirq: Let ksoftirqd do its
> job"") when a lot of network packets are processed then processing is
> moved to ksoftirqd and continues based on how the scheduler schedules
> the SCHED_OTHER ksoftirqd task. This avoids lock-ups of the system and
> it can do something else in between. Any interrupt will not continue the
> outstanding softirq backlog but wait for ksoftirqd. So it basically
> avoids the networking overload. It throttles the throughput if needed.
> 
> This isn't the case after that commit. Now, the CPU can be stuck with
> processing networking packets if the packets come in fast enough. Even
> if ksoftirqd is woken up, the next interrupt (say the timer) will
> continue with at least one round.
> By using NAPI-threads it is possible to give the control back to the
> scheduler which can throttle the NAPI processing in favour of other
> threads that ask for CPU. As you pointed out, waking the thread does not
> guarantee that it will immediately do the NAPI work. It can be delayed
> based on current load on the system.
> 
> This could be influenced by assigning the NAPI-thread a SCHED_FIFO
> priority. Based on the priority it could be ensured that the thread
> starts right away or "later" if something else is more important.
> However, this opens the DoS window again: The scheduler will put the
> NAPI thread on CPU as long as it asks for it with no throttling.
> 
> If we could somehow define a DoS condition once we are overwhelmed with
> packets, then we could act on it and throttle it. This in turn would
> allow a SCHED_FIFO priority without the fear of a lockup if the system
> is flooded with packets.

I declare ENOCOFFEE before starting, be warned! 

I fear this is becoming a bit too theoretical, but we can infer a DoS
condition if the napi thread enqueues somewhere (socket buffer, qdisc,
tx ring, ???) a packet and the queue utilization is "high" (say > 75%
of max).

I have no idea how to throttle a FIFO thread retaining its priority.

More importantly, this kind of configuration is not really viable for a
generic !PREEMPT_RT build, while the concern I have with napi threaded
backlog/serving the backlog with ksoftirqd applies there.

Cheers,

Paolo