linux-kernel - Re: [PATCH net] xsk: correct tx_ring_empty

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: <CAJ8uoz1JxhXFkzW8n_Dud8SR-4zE7gim5vS_UZHELiA7d0k+wQ@mail.gmail.com>
Date: Tue, 1 Apr 2025 08:57:39 +0200
From: Magnus Karlsson <magnus.karlsson@...il.com>
To: Wang Liang <wangliang74@...wei.com>
Cc: Stanislav Fomichev <stfomichev@...il.com>, bjorn@...nel.org, magnus.karlsson@...el.com, 
	maciej.fijalkowski@...el.com, jonathan.lemon@...il.com, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, 
	ast@...nel.org, daniel@...earbox.net, hawk@...nel.org, 
	john.fastabend@...il.com, yuehaibing@...wei.com, zhangchangzhong@...wei.com, 
	netdev@...r.kernel.org, bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] xsk: correct tx_ring_empty_descs count statistics

On Tue, 1 Apr 2025 at 04:36, Wang Liang <wangliang74@...wei.com> wrote:
>
>
> 在 2025/4/1 6:03, Stanislav Fomichev 写道:
> > On 03/31, Stanislav Fomichev wrote:
> >> On 03/29, Wang Liang wrote:
> >>> The tx_ring_empty_descs count may be incorrect, when set the XDP_TX_RING
> >>> option but do not reserve tx ring. Because xsk_poll() try to wakeup the
> >>> driver by calling xsk_generic_xmit() for non-zero-copy mode. So the
> >>> tx_ring_empty_descs count increases once the xsk_poll()is called:
> >>>
> >>>    xsk_poll
> >>>      xsk_generic_xmit
> >>>        __xsk_generic_xmit
> >>>          xskq_cons_peek_desc
> >>>            xskq_cons_read_desc
> >>>              q->queue_empty_descs++;

Sorry, but I do not understand how to reproduce this error. So you
first issue a setsockopt with the XDP_TX_RING option and then you do
not "reserve tx ring". What does that last "not reserve tx ring" mean?
No mmap() of that ring, or something else? I guess you have bound the
socket with a bind()? Some pseudo code on how to reproduce this would
be helpful. Just want to understand so I can help. Thank you.

> >>>
> >>> To avoid this count error, add check for tx descs before send msg in poll.
> >>>
> >>> Fixes: df551058f7a3 ("xsk: Fix crash in poll when device does not support ndo_xsk_wakeup")
> >>> Signed-off-by: Wang Liang <wangliang74@...wei.com>
> >> Acked-by: Stanislav Fomichev <sdf@...ichev.me>
> > Hmm, wait, I stumbled upon xskq_has_descs again and it looks only at
> > cached prod/cons. How is it supposed to work when the actual tx
> > descriptor is posted? Is there anything besides xskq_cons_peek_desc from
> > __xsk_generic_xmit that refreshes cached_prod?
>
>
> Yes, you are right!
>
> How about using xskq_cons_nb_entries() to check free descriptors?
>
> Like this:
>
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index e5d104ce7b82..babb7928d335 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -993,7 +993,7 @@ static __poll_t xsk_poll(struct file *file, struct
> socket *sock,
>          if (pool->cached_need_wakeup) {
>                  if (xs->zc)
>                          xsk_wakeup(xs, pool->cached_need_wakeup);
> -               else if (xs->tx)
> +               else if (xs->tx && xskq_cons_nb_entries(xs->tx, 1))
>                          /* Poll needs to drive Tx also in copy mode */
>                          xsk_generic_xmit(sk);
>          }
>
>