linux-kernel - Re: [PATCH net-next v5 7/7] docs: networking: Describe irq suspension

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZykRdK6WgfR_4p5X@LQ3V64L9R2>
Date: Mon, 4 Nov 2024 10:24:52 -0800
From: Joe Damato <jdamato@...tly.com>
To: Bagas Sanjaya <bagasdotme@...il.com>
Cc: netdev@...r.kernel.org, hdanton@...a.com, pabeni@...hat.com,
	namangulati@...gle.com, edumazet@...gle.com,
	amritha.nambiar@...el.com, sridhar.samudrala@...el.com,
	sdf@...ichev.me, peter@...eblog.net, m2shafiei@...terloo.ca,
	bjorn@...osinc.com, hch@...radead.org, willy@...radead.org,
	willemdebruijn.kernel@...il.com, skhawaja@...gle.com,
	kuba@...nel.org, Martin Karsten <mkarsten@...terloo.ca>,
	"David S. Miller" <davem@...emloft.net>,
	Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
	Linux Documentation <linux-doc@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux BPF <bpf@...r.kernel.org>
Subject: Re: [PATCH net-next v5 7/7] docs: networking: Describe irq suspension

On Mon, Nov 04, 2024 at 05:52:52PM +0700, Bagas Sanjaya wrote:
> On Sun, Nov 03, 2024 at 05:24:09AM +0000, Joe Damato wrote:
> > +It is important to note that choosing a large value for ``gro_flush_timeout``
> > +will defer IRQs to allow for better batch processing, but will induce latency
> > +when the system is not fully loaded. Choosing a small value for
> > +``gro_flush_timeout`` can cause interference of the user application which is
> > +attempting to busy poll by device IRQs and softirq processing. This value
> > +should be chosen carefully with these tradeoffs in mind. epoll-based busy
> > +polling applications may be able to mitigate how much user processing happens
> > +by choosing an appropriate value for ``maxevents``.
> > +
> > +Users may want to consider an alternate approach, IRQ suspension, to help deal
>                                                                      to help dealing
> > +with these tradeoffs.
> > +

Thanks for the careful review. I read this sentence a few times and
perhaps my English grammar isn't great, but I think it should be
one of:

Users may want to consider an alternate approach, IRQ suspension, to
help deal with these tradeoffs.  (the original)

or

Users may want to consider an alternate approach, IRQ suspension,
which can help to deal with these tradeoffs.

or

Users may want to consider an alternate approach, IRQ suspension,
which can help when dealing with these tradeoffs.

I am thinking of leaving the original unless you have a strong
preference? My apologies if I've gotten the grammar wrong here :)

Please let me know.

> > <snipped>...
> > +There are essentially three possible loops for network processing and
> > +packet delivery:
> > +
> > +1) hardirq -> softirq   -> napi poll; basic interrupt delivery
> > +
> > +2)   timer -> softirq   -> napi poll; deferred irq processing
> > +
> > +3)   epoll -> busy-poll -> napi poll; busy looping
> 
> The loops list are parsed inconsistently due to tabs between the
> enumerators and list items. I have to expand them into single space
> (along with number reference fix to follow the output):

Thank you for doing that. I'll take the suggested patch below and
apply it for our v6.

> ---- >8 ----
> diff --git a/Documentation/networking/napi.rst b/Documentation/networking/napi.rst
> index bbd58bcc430fab..848cb19f0becc1 100644
> --- a/Documentation/networking/napi.rst
> +++ b/Documentation/networking/napi.rst
> @@ -375,23 +375,21 @@ epoll finds no events, the setting of ``gro_flush_timeout`` and
>  There are essentially three possible loops for network processing and
>  packet delivery:
>  
> -1) hardirq -> softirq   -> napi poll; basic interrupt delivery
> +1) hardirq -> softirq -> napi poll; basic interrupt delivery
> +2) timer -> softirq -> napi poll; deferred irq processing
> +3) epoll -> busy-poll -> napi poll; busy looping
>  
> -2)   timer -> softirq   -> napi poll; deferred irq processing
> -
> -3)   epoll -> busy-poll -> napi poll; busy looping
> -
> -Loop 2) can take control from Loop 1), if ``gro_flush_timeout`` and
> +Loop 2 can take control from Loop 1, if ``gro_flush_timeout`` and
>  ``napi_defer_hard_irqs`` are set.
>  
> -If ``gro_flush_timeout`` and ``napi_defer_hard_irqs`` are set, Loops 2)
> -and 3) "wrestle" with each other for control.
> +If ``gro_flush_timeout`` and ``napi_defer_hard_irqs`` are set, Loops 2
> +and 3 "wrestle" with each other for control.
>  
> -During busy periods, ``irq-suspend-timeout`` is used as timer in Loop 2),
> -which essentially tilts network processing in favour of Loop 3).
> +During busy periods, ``irq-suspend-timeout`` is used as timer in Loop 2,
> +which essentially tilts network processing in favour of Loop 3.
>  
> -If ``gro_flush_timeout`` and ``napi_defer_hard_irqs`` are not set, Loop 3)
> -cannot take control from Loop 1).
> +If ``gro_flush_timeout`` and ``napi_defer_hard_irqs`` are not set, Loop 3
> +cannot take control from Loop 1.
>  
>  Therefore, setting ``gro_flush_timeout`` and ``napi_defer_hard_irqs`` is
>  the recommended usage, because otherwise setting ``irq-suspend-timeout``
> 
> Thanks.
> 
> -- 
> An old man doll... just what I always wanted! - Clara