From: Joe Damato <jdamato@fastly.com>
To: Samiullah Khawaja <skhawaja@google.com>
Cc: Martin Karsten <mkarsten@uwaterloo.ca>,
Jakub Kicinski <kuba@kernel.org>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>,
almasrymina@google.com, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v3 0/4] Add support to do threaded napi busy poll
Date: Thu, 6 Feb 2025 05:54:05 -0800 [thread overview]
Message-ID: <Z6S-_b7XwT9ebpIZ@LQ3V64L9R2> (raw)
In-Reply-To: <CAAywjhS69zRTBM7ZLNR08kL+anYuffppzU5ZuNORxKGQgo7_TA@mail.gmail.com>
On Wed, Feb 05, 2025 at 08:43:48PM -0800, Samiullah Khawaja wrote:
> On Wed, Feb 5, 2025 at 5:15 PM Martin Karsten <mkarsten@uwaterloo.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@uwaterloo.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.
I'm going to reproduce this on a GCP instance like you illustrated
in your cover letter, but there is a tremendous lack of detail to
get the system into a starting state.
As Martin explains in his follow: I am not asking for a generic
installer script for onload. I can definitely compile that program
on a GCP instance myself.
What we are both asking for is more information on how the system is
setup or, as Martin has repeatedly asked for, a script that gets the
system into its initial state for each experiment to streamline
reproduction of results.
I'm confused why your responses seem so strongly opposed to what we
are asking for?
Neither I nor Martin are against a new mechanism for busy polling
being introduced; we are both saying that a general purpose
mechanism that burns 100% CPU even when the network is idle should
be carefully considered. Wouldn't you agree?
The best way to carefully consider it would be to include more data
and more measurements of important system metrics, like CPU cycle
counts, CPU consumption, etc.
I don't intend to speak for Martin, but I'm asking for (and I think
he'd agree):
- More rigorous analysis, with more test cases and data (like CPU
consumption, socket counts, message sizes, etc)
- Better documentation of how one configures the system, possibly
using a script. It's OK if the script only works on the exact
GCP instance you used; I'd like to create that GCP instance and
know 100% that I have the same starting state as you when I run
the test.
- If more test cases are going to be tested and the data extracted
for a cover letter, a script that generates this in a standard
format with all of the metrics would be very helpful to ensure
we are all measuring this the same way, using the same command
line tools, with the same arguments. In other words, if you
decide to run "perf" to count cycles or cache misses or whatever
it is, capturing that in a script ensures we are all measuring
this the same way.
If you think this is unreasonable, please let me know; your
responses suggest you are opposed to more detailed analysis and I am
trying to understand why that might be.
next prev parent reply other threads:[~2025-02-06 13:54 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-05 0:10 [PATCH net-next v3 0/4] Add support to do threaded napi busy poll Samiullah Khawaja
2025-02-05 0:10 ` [PATCH net-next v3 1/4] Add support to set napi threaded for individual napi Samiullah Khawaja
2025-02-05 2:50 ` Joe Damato
2025-02-05 23:10 ` David Laight
2025-02-06 18:40 ` Joe Damato
2025-02-05 0:10 ` [PATCH net-next v3 2/4] net: Create separate gro_flush helper function Samiullah Khawaja
2025-02-05 2:55 ` Joe Damato
2025-02-05 0:10 ` [PATCH net-next v3 3/4] Extend napi threaded polling to allow kthread based busy polling Samiullah Khawaja
2025-02-05 9:08 ` Paul Barker
2025-02-06 3:40 ` Samudrala, Sridhar
2025-02-05 0:10 ` [PATCH net-next v3 4/4] selftests: Add napi threaded busy poll test in `busy_poller` Samiullah Khawaja
2025-02-05 0:14 ` [PATCH net-next v3 0/4] Add support to do threaded napi busy poll Samiullah Khawaja
2025-02-05 1:32 ` Martin Karsten
2025-02-05 20:35 ` Samiullah Khawaja
2025-02-05 22:06 ` Joe Damato
2025-02-06 0:45 ` Samiullah Khawaja
2025-02-06 13:42 ` Joe Damato
2025-02-06 22:49 ` Samiullah Khawaja
2025-02-06 22:58 ` Joe Damato
2025-02-06 1:15 ` Martin Karsten
2025-02-06 4:43 ` Samiullah Khawaja
2025-02-06 4:50 ` Martin Karsten
2025-02-06 6:43 ` Samiullah Khawaja
2025-02-06 14:00 ` Joe Damato
2025-02-06 13:54 ` Joe Damato [this message]
2025-02-05 3:18 ` Joe Damato
2025-02-06 21:19 ` Joe Damato
2025-02-06 22:06 ` Samiullah Khawaja
2025-02-06 22:48 ` Joe Damato
2025-02-07 3:13 ` Samiullah Khawaja
2025-02-07 3:50 ` Joe Damato
2025-02-11 2:52 ` Martin Karsten
2025-02-06 5:36 ` Dave Taht
2025-02-06 5:49 ` Samiullah Khawaja
2025-02-06 5:57 ` Dave Taht
2025-02-06 14:01 ` Joe Damato
2025-02-06 19:50 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z6S-_b7XwT9ebpIZ@LQ3V64L9R2 \
--to=jdamato@fastly.com \
--cc=almasrymina@google.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=mkarsten@uwaterloo.ca \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=skhawaja@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox