Message-ID: <Z6TAZK4TrTbhm1SB@LQ3V64L9R2>
Date: Thu, 6 Feb 2025 06:00:04 -0800
From: Joe Damato <jdamato@...tly.com>
To: Samiullah Khawaja <skhawaja@...gle.com>
Cc: Martin Karsten <mkarsten@...terloo.ca>,
	Jakub Kicinski <kuba@...nel.org>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	almasrymina@...gle.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 0/4] Add support to do threaded napi busy poll

On Wed, Feb 05, 2025 at 10:43:43PM -0800, Samiullah Khawaja wrote:
> On Wed, Feb 5, 2025 at 8:50 PM Martin Karsten <mkarsten@...terloo.ca> wrote:
> >
> > On 2025-02-05 23:43, Samiullah Khawaja wrote:
> > > On Wed, Feb 5, 2025 at 5:15 PM Martin Karsten <mkarsten@...terloo.ca> wrote:
> > >>
> > >> On 2025-02-05 17:06, Joe Damato wrote:
> > >>> On Wed, Feb 05, 2025 at 12:35:00PM -0800, Samiullah Khawaja wrote:
> > >>>> On Tue, Feb 4, 2025 at 5:32 PM Martin Karsten <mkarsten@...terloo.ca> wrote:
> > >>>>>
> > >>>>> On 2025-02-04 19:10, Samiullah Khawaja wrote:
> > >>
> > >> [snip]
> > >>
> > >>>>> Note that I don't dismiss the approach out of hand. I just think it's
> > >>>>> important to properly understand the purported performance improvements.
> > >>>> I think the performance improvements are apparent with the data I
> > >>>> provided, I purposefully used more sockets to show the real
> > >>>> differences in tail latency with this revision.
> > >>>
> > >>> Respectfully, I don't agree that the improvements are "apparent." I
> > >>> think my comments and Martin's comments both suggest that the cover
> > >>> letter does not make the improvements apparent.
> > >>>
> > >>>> Also one thing that you are probably missing here is that the change
> > >>>> here also has an API aspect, that is it allows a user to drive napi
> > >>>> independent of the user API or protocol being used.
> > >>>
> > >>> I'm not missing that part; I'll let Martin speak for himself but I
> > >>> suspect he also follows that part.
> > >>
> > >> Yes, the API aspect is quite interesting. In fact, Joe has given you
> > >> pointers how to split this patch into multiple incremental steps, the
> > >> first of which should be uncontroversial.
> > >>
> > >> I also just read your subsequent response to Joe. He has captured the
> > >> relevant concerns very well and I don't understand why you refuse to
> > >> document your complete experiment setup for transparency and
> > >> reproducibility. This shouldn't be hard.
> > > I think I have provided all the setup details and pointers to
> > > components. I appreciate that you want to reproduce the results and If
> > > you really really want to set it up then start by setting up onload on
> > > your platform. I cannot provide a generic installer script for onload
> > > that _claims_ to set it up on an arbitrary platform (with arbitrary
> > > NIC and environment). If it works on your platform (on top of AF_XDP)
> > > then from that point you can certainly build neper and run it using
> > > the command I shared.
> >
> > This is not what I have asked. Installing onload and neper is a given.
> > At least, I need the irq routing and potential thread affinity settings
> > at the server. Providing a full experiment script would be appreciated,
> > but at least the server configuration needs to be specified.
> - There is only 1 rx/tx queue and IRQs are deferred as mentioned in
> the cover letter. I have listed the following command for you to
> configure your netdev with 1 queue pair.
> ```
> sudo ethtool -L eth0 rx 1 tx 1
> ```
> - There is no special interrupt routing, there is only 1 queue pair
> and IDPF shares the same IRQ for both queues. The results remain the
> same whatever core I pin this IRQ to, I tested with core 2, 3 and 23
> (random) on my machine. This is mostly irrelevant since interrupts are
> deferred in both tests. I don't know how your NIC uses IRQs and
> whether the tx and rx are combined, so you might have to figure that
> part out. Sorry about that.

Again, you can assume we are using the same GCP instance you used.

If pinning the IRQ to other cores has no impact on the results, why
not include data that shows that in your cover letter?

> - I moved my napi polling thread to core 2 and as you can see in the
> command I shared I run neper on core 3-10. I enable threaded napi at
> device level for ease of use as I have only 1 queue pair and they both
> share a single NAPI on idpf. Probably different for your platform. I
> use following command,
> ```
>   echo 2 | sudo tee /sys/class/net/eth0/threaded
>   NAPI_T=$(ps -ef | grep napi | grep -v grep | awk '{ print $2 }')
>   sudo chrt -o  -p 0 $NAPI_T
>   sudo taskset -pc 2 $NAPI_T
> ```

I must have missed it in the original cover letter because I didn't
know that you had moved NAPI polling to core 2.

So: 
  - your base case is testing share one CPU core, where IRQ/softirq
    interference is likely to occur when network load is high

  - your test case is testing a 2 CPU setup, where one core does
    NAPI processing with BH disabled so that IRQ/sofitrq cannot
    interfere

Is that accurate? It may not be, and if not please let me know.

I never heard back on my response to the cover letter so I have no
idea if my understanding of how this works, what is being tested,
and the machine's configuration is accurate.

I think this points to more detail required in the cover letter
given how none of this seems particularly obvious to either Martin
or I.

Powered by blists - more mailing lists