On Thu, Jul 09, 2020 at 09:30:42PM +0200, Magnus Karlsson wrote: > On Thu, Jul 9, 2020 at 7:10 PM Magnus Karlsson > <magnus.karls...@gmail.com> wrote: > > > > On Thu, Jul 9, 2020 at 7:06 PM Jonathan Lemon <jonathan.le...@gmail.com> > > wrote: > > > > > > On Thu, Jul 09, 2020 at 11:45:51AM +0200, Magnus Karlsson wrote: > > > > In the skb Tx path, transmission of a packet is performed with > > > > dev_direct_xmit(). When QUEUE_STATE_FROZEN is set in the transmit > > > > routines, it returns NETDEV_TX_BUSY signifying that it was not > > > > possible to send the packet now, please try later. Unfortunately, the > > > > xsk transmit code discarded the packet and returned EBUSY to the > > > > application. Fix this unnecessary packet loss, by not discarding the > > > > packet and return EAGAIN. As EAGAIN is returned to the application, it > > > > can then retry the send operation and the packet will finally be sent > > > > as we will likely not be in the QUEUE_STATE_FROZEN state anymore. So > > > > EAGAIN tells the application that the packet was not discarded from > > > > the Tx ring and that it needs to call send() again. EBUSY, on the > > > > other hand, signifies that the packet was not sent and discarded from > > > > the Tx ring. The application needs to put the packet on the Tx ring > > > > again if it wants it to be sent. > > > > > > Doesn't the original code leak the skb if NETDEV_TX_BUSY is returned? > > > I'm not seeing where it was released. The new code looks correct. > > > > You are correct. Should also have mentioned that in the commit message. > > Jonathan, > > Some context here. The bug report from Arkadiusz started out with the > unnecessary packet loss. While fixing it, I discovered that it was > actually leaking memory too. If you want, I can send a v2 that has a > commit message that mentions both problems? Let me know what you > prefer.
I think it would be best to mention both problems for the benefit of future readers. -- Jonathan
