Re: [dpdk-dev] [PATCH v2] net/af_packet: fix ignoring full ring on tx

[Tudor Cornea <tudor.cornea@gmail.com>]

Hi Ferruh,

What you described above looks like a ring buffer with single producer and
> single consumer, and producer overwrites the not consumed items.


Indeed. This is also my understanding of the bug.
I am going to try to isolate the issue, and should probably be able to come
up with a script in a few days.

Our of curiosity, are you using an modified af_packet implementation in
> kernel
> for above described usage?


We are currently using an Ubuntu-based distro with a 4.15 Linux kernel.
We don't have any kernel patches for the af_packet implementation to my
knowledge (probably excepting patches that are back-ported by Ubuntu
maintainers from newer releases).


On Mon, 20 Sept 2021 at 20:44, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 9/13/2021 2:45 PM, Tudor Cornea wrote:
> > The poll call can return POLLERR which is ignored, or it can return
> > POLLOUT, even if there are no free frames in the mmap-ed area.
> >
> > We can account for both of these cases by re-checking if the next
> > frame is empty before writing into it.
> >
> > Signed-off-by: Mihai Pogonaru <pogonarumihai@gmail.com>
> > Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> > ---
> >  drivers/net/af_packet/rte_eth_af_packet.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c
> b/drivers/net/af_packet/rte_eth_af_packet.c
> > index b73b211..087c196 100644
> > --- a/drivers/net/af_packet/rte_eth_af_packet.c
> > +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> > @@ -216,6 +216,25 @@ eth_af_packet_tx(void *queue, struct rte_mbuf
> **bufs, uint16_t nb_pkts)
> >                   (poll(&pfd, 1, -1) < 0))
> >                       break;
> >
> > +             /*
> > +              * Poll can return POLLERR if the interface is down
> > +              *
> > +              * It will almost always return POLLOUT, even if there
> > +              * are no extra buffers available
> > +              *
> > +              * This happens, because packet_poll() calls
> datagram_poll()
> > +              * which checks the space left in the socket buffer and,
> > +              * in the case of packet_mmap, the default socket buffer
> length
> > +              * doesn't match the requested size for the tx_ring.
> > +              * As such, there is almost always space left in socket
> buffer,
> > +              * which doesn't seem to be correlated to the requested
> size
> > +              * for the tx_ring in packet_mmap.
> > +              *
> > +              * This results in poll() returning POLLOUT.
> > +              */
> > +             if (ppd->tp_status != TP_STATUS_AVAILABLE)
> > +                     break;
> > +
>
> If 'POLLOUT' doesn't indicate that there is space in the buffer, what is
> the
> point of the 'poll()' at all?
>
> What can we test/reproduce the mentioned behavior? Or is there a way to
> fix the
> behavior of poll() or use an alternative of it?
>
>
> OK to break on the 'POLLERR', I guess it can be detected in the
> 'pfd.revent'.
>
>
> >               /* copy the tx frame data */
> >               pbuf = (uint8_t *) ppd + TPACKET2_HDRLEN -
> >                       sizeof(struct sockaddr_ll);
> >
>
>