[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <68556722b5c47_3ffda429453@willemb.c.googlers.com.notmuch>
Date: Fri, 20 Jun 2025 09:50:26 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jason Xing <kerneljasonxing@...il.com>,
Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net,
edumazet@...gle.com,
pabeni@...hat.com,
bjorn@...nel.org,
magnus.karlsson@...el.com,
maciej.fijalkowski@...el.com,
jonathan.lemon@...il.com,
sdf@...ichev.me,
ast@...nel.org,
daniel@...earbox.net,
hawk@...nel.org,
john.fastabend@...il.com,
joe@...a.to,
willemdebruijn.kernel@...il.com,
bpf@...r.kernel.org,
netdev@...r.kernel.org,
Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v3] net: xsk: introduce XDP_MAX_TX_BUDGET
set/getsockopt
Jason Xing wrote:
> On Thu, Jun 19, 2025 at 11:09 PM Jakub Kicinski <kuba@...nel.org> wrote:
> >
> > On Thu, 19 Jun 2025 17:04:40 +0800 Jason Xing wrote:
> > > @@ -424,7 +421,9 @@ bool xsk_tx_peek_desc(struct xsk_buff_pool *pool, struct xdp_desc *desc)
> > > rcu_read_lock();
> > > again:
> > > list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) {
> > > - if (xs->tx_budget_spent >= MAX_PER_SOCKET_BUDGET) {
> > > + int max_budget = READ_ONCE(xs->max_tx_budget);
> > > +
> > > + if (xs->tx_budget_spent >= max_budget) {
> > > budget_exhausted = true;
> > > continue;
> > > }
> > > @@ -779,7 +778,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> > > static int __xsk_generic_xmit(struct sock *sk)
> > > {
> > > struct xdp_sock *xs = xdp_sk(sk);
> > > - u32 max_batch = TX_BATCH_SIZE;
> > > + u32 max_budget = READ_ONCE(xs->max_tx_budget);
> >
> > Hm, maybe a question to Stan / Willem & other XSK experts but are these
> > two max values / code paths really related? Question 2 -- is generic
> > XSK a legit optimization target, legit enough to add uAPI?
>
> I'm not an expert but my take is:
> #1, I don't see the correlation actually while I don't see any reason
> to use the different values for both of them.
> #2, These two definitions are improvement points because whether to do
> the real send is driven by calling sendto(). Enlarging a little bit of
> this value could save many times of calling sendto(). As for the uAPI,
> I don't know if it's worth it, sorry. If not, the previous version 2
> patch (regarding per-netns policy) will be revived.
>
> So I will leave those two questions to XSK experts as well.
You're proposing the code change, so I think it's on you to make
this argument?
> #2 quantification
> It's really hard to do so mainly because of various stacks implemented
> in the user-space. AF_XDP is providing a fundamental mechanism only
> and its upper layer is prosperous.
I think it's a hard sell to argue adding a tunable, if no plausible
recommendation can be given on how the tunable is to be used.
It's not necessary, and most cases infeasible, to give a heuristic
that fits all possible users. But at a minimum the one workload that
prompted the patch. What value do you set it to and how did you
arrive at that number?
Powered by blists - more mailing lists