Re: [PATCH net] xsk: fix __xsk_generic_xmit() error code when cq is full

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2025/2/25 1:14, Magnus Karlsson 写道:
On Mon, 24 Feb 2025 at 17:00, Stanislav Fomichev <stfomichev@xxxxxxxxx> wrote:
On 02/24, Magnus Karlsson wrote:
On Sat, 22 Feb 2025 at 10:18, Wang Liang <wangliang74@xxxxxxxxxx> wrote:
When the cq reservation is failed, the error code is not set which is
initialized to zero in __xsk_generic_xmit(). That means the packet is not
send successfully but sendto() return ok.

Set the error code and make xskq_prod_reserve_addr()/xskq_prod_reserve()
return values more meaningful when the queue is full.
Hi Wang,

I agree that this would have been a really good idea if it was
implemented from day one, but now I do not dare to change this since
it would be changing the uapi. Let us say you have the following quite
common code snippet for sending a packet with AF_XDP in skb mode:

err = sendmsg();
if (err && err != -EAGAIN && err != -EBUSY)
     goto die_due_to_error;
continue with code

This code would with your change go and die suddenly when the
completion ring is full instead of working. Maybe there is a piece of
code that cleans the completion ring after these lines of code and
next time sendmsg() is called, the packet will get sent, so the
application used to work.

So I say: let us not do this. But if anyone has another opinion, please share.
Can we return -EBUSY from this 'if (xsk_cq_reserve_addr_locked())' case as
well?
That is a good idea! Though I would return -EAGAIN. When -EBUSY is
returned, the buffer was consumed but not sent. But -EAGAIN means that
the user just has to perform then sendmsg() again and that is exactly
what the user has to do here too.


Thank you for the suggestion!
Changing the uapi is indeed a high-risk act. Return -EAGAIN is a much better choice. The cq is full usually because it is not released in time, try to send msg again is appropriate. I will send a new patch later, and look forward to getting more advice. Thanks.






[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux