Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value

[Didier Pallard <didier.pallard@6wind.com>]

Well, you're right, another way to fix the problem could be to set up the
queue size assuming the provided number
is a number of packets in queue rather than a number of mbufs in queue.
But not sure it's better, it's also important for the application/user to
know the number of mbufs that could fit in a rx/tx queue,
whatever the number of packets that it covers, since it is very important
to size the memory pools correctly to avoid any
mbuf shortage during system life.
Thanks
Didier

On Mon, Mar 16, 2020 at 5:05 PM Slava Ovsiienko <viacheslavo@mellanox.com>
wrote:

> Hi, Didier
>
> First, thank you for the patch.
>
> If we have a look at the description of rte_eth_rx_queue_count(): "Get the
> number of used descriptors of a rx queue".
> It means the DPDK generic descriptors, not PMD specific ones. "DPDK
> descriptor" means the entity which can handle one packet.
> rte_eth_rx_queue_count() should return the potential number of packets
> could be fetched from the Rx queue on the next rx_burst() call.
> Application should know anything about PMD descriptors, it must be
> isolated. So, rx_queue_count() should return the number of expected
> packets not hardware entries. That's why the value being returned is
> compared with elts, not with HW desctriptors.
>
> As for -1 - I agree, should be fixed.
>
> It seems we have another bug - the Rx queue is created with number of
> hardware descriptors which does not correspond the requested packets
> in case of multi-segment packets  (requested desc is divided by
> (1<<sges_n), it seems it should not).
>
> With best regards, Slava
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Didier Pallard
> > Sent: Friday, March 13, 2020 11:57
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned
> value
> >
> > Two bugs in rx_queue_count function:
> > - One entry may contain several segments, so 'used' must be multiplied
> >   by number of segments per entry to properly reflect the queue usage.
> > - rx_queue_count returns the number of entries used in queue, so it
> ranges
> >   from 0 to max number of entries in queue, not this number minus
> >   one.
> >
> > Fixes: 8788fec1f269 ("net/mlx5: implement descriptor status API")
> > Cc: stable@dpdk.org
> > Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> > ---
> >  drivers/net/mlx5/mlx5_rxtx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index
> > 5ac63da8039d..17f80c25443e 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.c
> > +++ b/drivers/net/mlx5/mlx5_rxtx.c
> > @@ -500,7 +500,7 @@ rx_queue_count(struct mlx5_rxq_data *rxq)
> >               used += n;
> >               cqe = &(*rxq->cqes)[cq_ci & cqe_cnt];
> >       }
> > -     used = RTE_MIN(used, (1U << rxq->elts_n) - 1);
> > +     used = RTE_MIN(used * (1 << rxq->sges_n), 1U << rxq->elts_n);
> >       return used;
> >  }
> >
> > --
> > 2.24.1
>
>