[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240112175338.e5Ipk3C2@linutronix.de>
Date: Fri, 12 Jan 2024 18:53:38 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: "Kiyanovski, Arthur" <akiyano@...zon.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>,
Boqun Feng <boqun.feng@...il.com>,
Daniel Borkmann <daniel@...earbox.net>,
Eric Dumazet <edumazet@...gle.com>,
Frederic Weisbecker <frederic@...nel.org>,
Ingo Molnar <mingo@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Waiman Long <longman@...hat.com>, Will Deacon <will@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
"Arinzon, David" <darinzon@...zon.com>,
Igor Russkikh <irusskikh@...vell.com>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Michael Chan <michael.chan@...adcom.com>,
"Dagan, Noam" <ndagan@...zon.com>,
"Bshara, Saeed" <saeedb@...zon.com>,
"Agroskin, Shay" <shayagr@...zon.com>,
Sunil Goutham <sgoutham@...vell.com>
Subject: Re: RE: [PATCH net-next 17/24] net: amazon, aquanti, broadcom,
cavium, engleder: Use nested-BH locking for XDP redirect.
On 2023-12-16 22:09:07 [+0000], Kiyanovski, Arthur wrote:
> Hi Sebastian,
Arthur,
> I would like to make sure I understand correctly the difference in this patch
> between ena and atlantic drivers.
>
> In the atlantic driver the change you've made seems like the best change
> in terms of making the critical section as small as possible.
>
> You could have done exactly the same thing with ena, but you chose instead
> to let ena release the lock at the end of the function, which in case of an XDP_TX
> may make the critical section considerably longer than in the atlantic solution.
>
> If I understand correctly (quote from your commit message "This does not
> always work because some drivers (cpsw, atlantic) invoke xdp_do_flush()
> in the same context"), in the case of atlantic you had to go for the more
> code-altering change, because if you simply used guard() you would include
> the xdp_do_flush() in the critical section, but in the case of ena xdp_do_flush()
> is called after the function ends so guard is good enough.
>
> Questions:
> 1. Did I understand correctly the difference in solution choice between atlantic
> and ena?
Yes. I could have moved the "XDP_REDIRECT" case right after
bpf_prog_run_xdp() and use "scope guard" to make it slim in the ena
driver. I just made "this" because it was simpler I did not want to
spent unnecessarily cycles on it especially if I have to maintain for a
few releases.
> 2. As far as I can see the guard() solution looks good for ena except for (maybe?)
> XDP_TX, where the critical section becomes a bit long. Can you please explain,
> why you think it is still good enough for ena to use the guard() solution instead
> of doing the more code-altering atlantic solution?
Well, it was simpler/ quicker. If this approach would have been accepted
and this long section a problem then it could have been shorten
afterwards. Maybe a another function/ method could be introduced since
this pattern fits ~90% of all drivers.
However it looks like touching all drivers is not what we want so
avoiding spending a lot of cycles on it in the first place wasn't that
bad. (Also it was the third iteration until I got all details right).
> Thanks!
> Arthur
>
Sebastian
Powered by blists - more mailing lists