This patchset provides several bug fixes for rx queue count calculation for mlx5 driver. --- V3: - fix spelling mistakes - fix second commit for mprq case V2: * squash first patch and second patch * fix wrong init of used for compressed cqes Didier Pallard (1): net/mlx5: fix Rx descriptor status returned value Maxime Leroy (1): net/mlx5: fix Rx queue count calculation drivers/net/mlx5/mlx5_rxtx.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) -- 2.27.0
The commit d2d57605522d ("net/mlx5: fix Rx queue count calculation") is incorrect because the count calculation is wrong for the next cqe: Example: Compressed Set of packets 1 | Compressed Set of packets 2 C | a | e0 | e1 | e2 | e3 | e4 | e5 | C | a | e0 There are 2 compressed set of packets in the first queue. For the first set, n is computed correctly. But for the second, n is not computed properly. Because the zip context is for the first set. The second set is not yet decompressed, so there are no context. To fix the issue, we should only use the zip context for the first CQEs series. Fixes: d2d57605522d ("net/mlx5: fix Rx queue count calculation") Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com> --- drivers/net/mlx5/mlx5_rxtx.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 844a1c63..2733dcd3 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -462,11 +462,18 @@ rx_queue_count(struct mlx5_rxq_data *rxq) { struct rxq_zip *zip = &rxq->zip; volatile struct mlx5_cqe *cqe; - unsigned int cq_ci = rxq->cq_ci; const unsigned int cqe_n = (1 << rxq->cqe_n); const unsigned int cqe_cnt = cqe_n - 1; - unsigned int used = 0; + unsigned int cq_ci, used; + /* if we are processing a compressed cqe */ + if (zip->ai) { + used = zip->cqe_cnt - zip->ai; + cq_ci = zip->cq_ci; + } else { + used = 0; + cq_ci = rxq->cq_ci; + } cqe = &(*rxq->cqes)[cq_ci & cqe_cnt]; while (check_cqe(cqe, cqe_n, cq_ci) != MLX5_CQE_STATUS_HW_OWN) { int8_t op_own; @@ -474,10 +481,7 @@ rx_queue_count(struct mlx5_rxq_data *rxq) op_own = cqe->op_own; if (MLX5_CQE_FORMAT(op_own) == MLX5_COMPRESSED) - if (unlikely(zip->ai)) - n = zip->cqe_cnt - zip->ai; - else - n = rte_be_to_cpu_32(cqe->byte_cnt); + n = rte_be_to_cpu_32(cqe->byte_cnt); else n = 1; cq_ci += n; -- 2.27.0
From: Didier Pallard <didier.pallard@6wind.com> Three 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. - The number of cqes is equals to (1U << rxq->elts_n) - 1 in non mqrt mode. The range returned by rx_queue_count should be the number of entries used in queue, so it ranges from 0 to max number of entries in queue, not this number minus one. - For MQRT mode, we need to take acount of the number of strd. Fixes: 8788fec1f269 ("net/mlx5: implement descriptor status API") Signed-off-by: Didier Pallard <didier.pallard@6wind.com> Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- drivers/net/mlx5/mlx5_rxtx.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 2733dcd3..2ecf901f 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -463,6 +463,9 @@ rx_queue_count(struct mlx5_rxq_data *rxq) struct rxq_zip *zip = &rxq->zip; volatile struct mlx5_cqe *cqe; const unsigned int cqe_n = (1 << rxq->cqe_n); + const unsigned int sges_n = (1 << rxq->sges_n); + const unsigned int elts_n = (1 << rxq->elts_n); + const unsigned int strd_n = (1 << rxq->strd_num_n); const unsigned int cqe_cnt = cqe_n - 1; unsigned int cq_ci, used; @@ -488,7 +491,7 @@ rx_queue_count(struct mlx5_rxq_data *rxq) used += n; cqe = &(*rxq->cqes)[cq_ci & cqe_cnt]; } - used = RTE_MIN(used, cqe_n); + used = RTE_MIN(used * sges_n, elts_n * strd_n); return used; } -- 2.27.0
> -----Original Message-----
> From: Maxime Leroy <maxime.leroy@6wind.com>
> Sent: Monday, November 16, 2020 19:03
> To: Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Alexander Kozyrev
> <akozyrev@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-N?lio Laranjeiro
> <nelio.laranjeiro@6wind.com>
> Subject: [PATCH v3 1/2] net/mlx5: fix Rx queue count calculation
>
> The commit d2d57605522d ("net/mlx5: fix Rx queue count calculation") is
> incorrect because the count calculation is wrong for the next cqe:
>
> Example:
>
> Compressed Set of packets 1 | Compressed Set of packets 2
> C | a | e0 | e1 | e2 | e3 | e4 | e5 | C | a | e0
>
> There are 2 compressed set of packets in the first queue. For the first set, n is
> computed correctly.
>
> But for the second, n is not computed properly. Because the zip context is for
> the first set. The second set is not yet decompressed, so there are no context.
>
> To fix the issue, we should only use the zip context for the first CQEs series.
>
> Fixes: d2d57605522d ("net/mlx5: fix Rx queue count calculation")
> Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Hi, Maxime It seems there are some typos (see below). Beside these ones - ACKed. > -----Original Message----- > From: Maxime Leroy <maxime.leroy@6wind.com> > Sent: Monday, November 16, 2020 19:03 > To: Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; > Slava Ovsiienko <viacheslavo@nvidia.com>; Olivier Matz > <olivier.matz@6wind.com> > Cc: dev@dpdk.org; Didier Pallard <didier.pallard@6wind.com> > Subject: [PATCH v3 2/2] net/mlx5: fix Rx descriptor status returned value > > From: Didier Pallard <didier.pallard@6wind.com> > > Three 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. > - The number of cqes is equals to (1U << rxq->elts_n) - 1 in non mqrt > mode. The range returned by rx_queue_count should be the number of Should it be "bullet number 3" ? > entries used in queue, so it ranges from 0 to max number of entries > in queue, not this number minus one. WARNING:TYPO_SPELLING: 'acount' may be misspelled - perhaps 'account'? Should it be - "to take into account?" Should it be "MPRQ" ? > - For MQRT mode, we need to take acount of the number of strd. > > Fixes: 8788fec1f269 ("net/mlx5: implement descriptor status API") > Signed-off-by: Didier Pallard <didier.pallard@6wind.com> > Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Hi Slava, On Mon, Nov 16, 2020 at 9:03 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote: > > Hi, Maxime > > It seems there are some typos (see below). > Beside these ones - ACKed. > > > -----Original Message----- > > From: Maxime Leroy <maxime.leroy@6wind.com> > > Sent: Monday, November 16, 2020 19:03 > > To: Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; > > Slava Ovsiienko <viacheslavo@nvidia.com>; Olivier Matz > > <olivier.matz@6wind.com> > > Cc: dev@dpdk.org; Didier Pallard <didier.pallard@6wind.com> > > Subject: [PATCH v3 2/2] net/mlx5: fix Rx descriptor status returned value > > > > From: Didier Pallard <didier.pallard@6wind.com> > > > > Three 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. > > - The number of cqes is equals to (1U << rxq->elts_n) - 1 in non mqrt > > mode. The range returned by rx_queue_count should be the number of > > Should it be "bullet number 3" ? This bullet is for SPRQ mode. The bullet number 3 is for MPRQ mode. Not sure to understand your point. > > > entries used in queue, so it ranges from 0 to max number of entries > > in queue, not this number minus one. > > WARNING:TYPO_SPELLING: 'acount' may be misspelled - perhaps 'account'? > Should it be - "to take into account?" > Should it be "MPRQ" ? I will fix it in V4. Thanks for the review. > > > - For MQRT mode, we need to take acount of the number of strd. > > > > Fixes: 8788fec1f269 ("net/mlx5: implement descriptor status API") > > Signed-off-by: Didier Pallard <didier.pallard@6wind.com> > > Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> > Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> >
Hi, Maxime > -----Original Message----- > From: Maxime Leroy <maxime.leroy@6wind.com> > Sent: Tuesday, November 17, 2020 10:52 > To: Slava Ovsiienko <viacheslavo@nvidia.com> > Cc: Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; > Olivier Matz <olivier.matz@6wind.com>; dev@dpdk.org; Didier Pallard > <didier.pallard@6wind.com> > Subject: Re: [PATCH v3 2/2] net/mlx5: fix Rx descriptor status returned value > > Hi Slava, > > On Mon, Nov 16, 2020 at 9:03 PM Slava Ovsiienko <viacheslavo@nvidia.com> > wrote: > > > > Hi, Maxime > > > > It seems there are some typos (see below). > > Beside these ones - ACKed. > > > > > -----Original Message----- > > > From: Maxime Leroy <maxime.leroy@6wind.com> > > > Sent: Monday, November 16, 2020 19:03 > > > To: Matan Azrad <matan@nvidia.com>; Shahaf Shuler > > > <shahafs@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; > > > Olivier Matz <olivier.matz@6wind.com> > > > Cc: dev@dpdk.org; Didier Pallard <didier.pallard@6wind.com> > > > Subject: [PATCH v3 2/2] net/mlx5: fix Rx descriptor status returned > > > value > > > > > > From: Didier Pallard <didier.pallard@6wind.com> > > > > > > Three 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. > > > - The number of cqes is equals to (1U << rxq->elts_n) - 1 in non mqrt > > > mode. The range returned by rx_queue_count should be the number of > > > > Should it be "bullet number 3" ? > > This bullet is for SPRQ mode. The bullet number 3 is for MPRQ mode. > Not sure to understand your point. Sorry, my bad - Now I see the third bullet at the MPRQ, so - please, disregard this my point: > > Should it be "bullet number 3" ? With best regards, Slava > > > > > entries used in queue, so it ranges from 0 to max number of entries > > > in queue, not this number minus one. > > > > WARNING:TYPO_SPELLING: 'acount' may be misspelled - perhaps 'account'? > > Should it be - "to take into account?" > > Should it be "MPRQ" ? > > I will fix it in V4. > > Thanks for the review. > > > > > > - For MQRT mode, we need to take acount of the number of strd. > > > > > > Fixes: 8788fec1f269 ("net/mlx5: implement descriptor status API") > > > Signed-off-by: Didier Pallard <didier.pallard@6wind.com> > > > Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> > > Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> > >