patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] net/mlx5: fix Rx descriptor status returned value
@ 2020-03-13  9:56 Didier Pallard
  2020-03-16 16:05 ` [dpdk-stable] [dpdk-dev] " Slava Ovsiienko
  0 siblings, 1 reply; 8+ messages in thread
From: Didier Pallard @ 2020-03-13  9:56 UTC (permalink / raw)
  To: dev; +Cc: stable

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-13  9:56 [dpdk-stable] [PATCH] net/mlx5: fix Rx descriptor status returned value Didier Pallard
@ 2020-03-16 16:05 ` Slava Ovsiienko
  2020-03-16 17:24   ` Didier Pallard
  0 siblings, 1 reply; 8+ messages in thread
From: Slava Ovsiienko @ 2020-03-16 16:05 UTC (permalink / raw)
  To: Didier Pallard, dev; +Cc: stable, Matan Azrad

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-16 16:05 ` [dpdk-stable] [dpdk-dev] " Slava Ovsiienko
@ 2020-03-16 17:24   ` Didier Pallard
  2020-03-17  8:33     ` Slava Ovsiienko
  0 siblings, 1 reply; 8+ messages in thread
From: Didier Pallard @ 2020-03-16 17:24 UTC (permalink / raw)
  To: Slava Ovsiienko; +Cc: dev, stable, Matan Azrad

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
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-16 17:24   ` Didier Pallard
@ 2020-03-17  8:33     ` Slava Ovsiienko
  2020-03-17  9:19       ` Didier Pallard
  0 siblings, 1 reply; 8+ messages in thread
From: Slava Ovsiienko @ 2020-03-17  8:33 UTC (permalink / raw)
  To: Didier Pallard; +Cc: dev, stable, Matan Azrad

>> From: Didier Pallard <didier.pallard@6wind.com> 
>> Sent: Monday, March 16, 2020 19:24
>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
>> Cc: dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
>> 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.

Yes, it is intended in queue setup routine. But, for mlx5 we have a bug for regular mlx5_rx_burst if scattering is enabled,
the Rx queue is created with size wqe_n elements, should be wqe_n << sges_n instead, to be able to receive the requested
number of packets (wqe_n). I think we must fix. Would you like to update your patch, or should I provide mine? 

>> 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
To estimate - the number of "DPDK descriptors" should be multiplied by the maximal length of scattered packet chain.

With best regards, Slava

> -----Original Message-----
> From: dev <mailto:dev-bounces@dpdk.org> On Behalf Of Didier Pallard
> Sent: Friday, March 13, 2020 11:57
> To: mailto:dev@dpdk.org
> Cc: mailto: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: mailto:stable@dpdk.org
> Signed-off-by: Didier Pallard <mailto: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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-17  8:33     ` Slava Ovsiienko
@ 2020-03-17  9:19       ` Didier Pallard
  2020-03-17  9:25         ` Slava Ovsiienko
  0 siblings, 1 reply; 8+ messages in thread
From: Didier Pallard @ 2020-03-17  9:19 UTC (permalink / raw)
  To: Slava Ovsiienko; +Cc: dev, stable, Matan Azrad

well, please do if you don't mind,
You will validate it quicker, and I'm currently working on a
different topic.
Btw, do you think it's possible to have an implementation of
[r,t]x_status_descriptor functions for vectorized implementation?
thanks
didier

On Tue, Mar 17, 2020 at 9:33 AM Slava Ovsiienko <viacheslavo@mellanox.com>
wrote:

> >> From: Didier Pallard <didier.pallard@6wind.com>
> >> Sent: Monday, March 16, 2020 19:24
> >> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> >> Cc: dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@mellanox.com>
> >> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status
> returned value
> >> 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.
>
> Yes, it is intended in queue setup routine. But, for mlx5 we have a bug
> for regular mlx5_rx_burst if scattering is enabled,
> the Rx queue is created with size wqe_n elements, should be wqe_n <<
> sges_n instead, to be able to receive the requested
> number of packets (wqe_n). I think we must fix. Would you like to update
> your patch, or should I provide mine?
>
> >> 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
> To estimate - the number of "DPDK descriptors" should be multiplied by the
> maximal length of scattered packet chain.
>
> With best regards, Slava
>
> > -----Original Message-----
> > From: dev <mailto:dev-bounces@dpdk.org> On Behalf Of Didier Pallard
> > Sent: Friday, March 13, 2020 11:57
> > To: mailto:dev@dpdk.org
> > Cc: mailto: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: mailto:stable@dpdk.org
> > Signed-off-by: Didier Pallard <mailto: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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-17  9:19       ` Didier Pallard
@ 2020-03-17  9:25         ` Slava Ovsiienko
  2020-03-17  9:41           ` Didier Pallard
  2020-10-29 11:29           ` Didier Pallard
  0 siblings, 2 replies; 8+ messages in thread
From: Slava Ovsiienko @ 2020-03-17  9:25 UTC (permalink / raw)
  To: Didier Pallard; +Cc: dev, stable, Matan Azrad

From: Didier Pallard <didier.pallard@6wind.com>
Sent: Tuesday, March 17, 2020 11:19
To: Slava Ovsiienko <viacheslavo@mellanox.com>
Cc: dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
>>>well, please do if you don't mind,
OK, I will.

>>>You will validate it quicker, and I'm currently working on a different topic.
>>>Btw, do you think it's possible to have an implementation of [r,t]x_status_descriptor functions for vectorized implementation?
>>>thanks
Yes, we are planning this update.

With best regards, Slava


didier

On Tue, Mar 17, 2020 at 9:33 AM Slava Ovsiienko <viacheslavo@mellanox.com<mailto:viacheslavo@mellanox.com>> wrote:
>> From: Didier Pallard <didier.pallard@6wind.com<mailto:didier.pallard@6wind.com>>
>> Sent: Monday, March 16, 2020 19:24
>> To: Slava Ovsiienko <viacheslavo@mellanox.com<mailto:viacheslavo@mellanox.com>>
>> Cc: dev@dpdk.org<mailto:dev@dpdk.org>; stable@dpdk.org<mailto:stable@dpdk.org>; Matan Azrad <matan@mellanox.com<mailto:matan@mellanox.com>>
>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
>> 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.

Yes, it is intended in queue setup routine. But, for mlx5 we have a bug for regular mlx5_rx_burst if scattering is enabled,
the Rx queue is created with size wqe_n elements, should be wqe_n << sges_n instead, to be able to receive the requested
number of packets (wqe_n). I think we must fix. Would you like to update your patch, or should I provide mine?

>> 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
To estimate - the number of "DPDK descriptors" should be multiplied by the maximal length of scattered packet chain.

With best regards, Slava

> -----Original Message-----
> From: dev <mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>> On Behalf Of Didier Pallard
> Sent: Friday, March 13, 2020 11:57
> To: mailto:dev@dpdk.org<mailto:dev@dpdk.org>
> Cc: mailto:stable@dpdk.org<mailto: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: mailto:stable@dpdk.org<mailto:stable@dpdk.org>
> Signed-off-by: Didier Pallard <mailto:didier.pallard@6wind.com<mailto: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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-17  9:25         ` Slava Ovsiienko
@ 2020-03-17  9:41           ` Didier Pallard
  2020-10-29 11:29           ` Didier Pallard
  1 sibling, 0 replies; 8+ messages in thread
From: Didier Pallard @ 2020-03-17  9:41 UTC (permalink / raw)
  To: Slava Ovsiienko; +Cc: dev, stable, Matan Azrad

OK, thank you
best regards
didier

On Tue, Mar 17, 2020 at 10:25 AM Slava Ovsiienko <viacheslavo@mellanox.com>
wrote:

> *From:* Didier Pallard <didier.pallard@6wind.com>
> *Sent:* Tuesday, March 17, 2020 11:19
> *To:* Slava Ovsiienko <viacheslavo@mellanox.com>
> *Cc:* dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@mellanox.com>
> *Subject:* Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status
> returned value
>
> >>>well, please do if you don't mind,
>
> OK, I will.
>
>
>
> >>>You will validate it quicker, and I'm currently working on a
> different topic.
>
> >>>Btw, do you think it's possible to have an implementation of
> [r,t]x_status_descriptor functions for vectorized implementation?
>
> >>>thanks
>
> Yes, we are planning this update.
>
>
>
> With best regards, Slava
>
>
>
>
>
> didier
>
>
>
> On Tue, Mar 17, 2020 at 9:33 AM Slava Ovsiienko <viacheslavo@mellanox.com>
> wrote:
>
> >> From: Didier Pallard <didier.pallard@6wind.com>
> >> Sent: Monday, March 16, 2020 19:24
> >> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> >> Cc: dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@mellanox.com>
> >> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status
> returned value
> >> 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.
>
> Yes, it is intended in queue setup routine. But, for mlx5 we have a bug
> for regular mlx5_rx_burst if scattering is enabled,
> the Rx queue is created with size wqe_n elements, should be wqe_n <<
> sges_n instead, to be able to receive the requested
> number of packets (wqe_n). I think we must fix. Would you like to update
> your patch, or should I provide mine?
>
> >> 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
> To estimate - the number of "DPDK descriptors" should be multiplied by the
> maximal length of scattered packet chain.
>
> With best regards, Slava
>
> > -----Original Message-----
> > From: dev <mailto:dev-bounces@dpdk.org> On Behalf Of Didier Pallard
> > Sent: Friday, March 13, 2020 11:57
> > To: mailto:dev@dpdk.org
> > Cc: mailto: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: mailto:stable@dpdk.org
> > Signed-off-by: Didier Pallard <mailto: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
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status returned value
  2020-03-17  9:25         ` Slava Ovsiienko
  2020-03-17  9:41           ` Didier Pallard
@ 2020-10-29 11:29           ` Didier Pallard
  1 sibling, 0 replies; 8+ messages in thread
From: Didier Pallard @ 2020-10-29 11:29 UTC (permalink / raw)
  To: Slava Ovsiienko; +Cc: dev, stable, Matan Azrad

Hi slava,

Is this fixed in main branch now?
If yes could you please point me out the proper commit?

Thanks
Didier

On 3/17/20 10:25 AM, Slava Ovsiienko wrote:
>
> *From:* Didier Pallard <didier.pallard@6wind.com>
> *Sent:* Tuesday, March 17, 2020 11:19
> *To:* Slava Ovsiienko <viacheslavo@mellanox.com>
> *Cc:* dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@mellanox.com>
> *Subject:* Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor status 
> returned value
>
> >>>well, please do if you don't mind,
>
> OK, I will.
>
> >>>You will validate it quicker, and I'm currently working on a 
> different topic.
>
> >>>Btw, do you think it's possible to have an implementation of 
> [r,t]x_status_descriptor functions for vectorized implementation?
>
> >>>thanks
>
> Yes, we are planning this update.
>
> With best regards, Slava
>
> didier
>
> On Tue, Mar 17, 2020 at 9:33 AM Slava Ovsiienko 
> <viacheslavo@mellanox.com <mailto:viacheslavo@mellanox.com>> wrote:
>
>     >> From: Didier Pallard <didier.pallard@6wind.com
>     <mailto:didier.pallard@6wind.com>>
>     >> Sent: Monday, March 16, 2020 19:24
>     >> To: Slava Ovsiienko <viacheslavo@mellanox.com
>     <mailto:viacheslavo@mellanox.com>>
>     >> Cc: dev@dpdk.org <mailto:dev@dpdk.org>; stable@dpdk.org
>     <mailto:stable@dpdk.org>; Matan Azrad <matan@mellanox.com
>     <mailto:matan@mellanox.com>>
>     >> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Rx descriptor
>     status returned value
>     >> 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.
>
>     Yes, it is intended in queue setup routine. But, for mlx5 we have
>     a bug for regular mlx5_rx_burst if scattering is enabled,
>     the Rx queue is created with size wqe_n elements, should be wqe_n
>     << sges_n instead, to be able to receive the requested
>     number of packets (wqe_n). I think we must fix. Would you like to
>     update your patch, or should I provide mine?
>
>     >> 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
>     To estimate - the number of "DPDK descriptors" should be
>     multiplied by the maximal length of scattered packet chain.
>
>     With best regards, Slava
>
>     > -----Original Message-----
>     > From: dev <mailto:dev-bounces@dpdk.org
>     <mailto:dev-bounces@dpdk.org>> On Behalf Of Didier Pallard
>     > Sent: Friday, March 13, 2020 11:57
>     > To: mailto:dev@dpdk.org <mailto:dev@dpdk.org>
>     > Cc: mailto:stable@dpdk.org <mailto: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: mailto:stable@dpdk.org <mailto:stable@dpdk.org>
>     > Signed-off-by: Didier Pallard <mailto:didier.pallard@6wind.com
>     <mailto: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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-29 11:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13  9:56 [dpdk-stable] [PATCH] net/mlx5: fix Rx descriptor status returned value Didier Pallard
2020-03-16 16:05 ` [dpdk-stable] [dpdk-dev] " Slava Ovsiienko
2020-03-16 17:24   ` Didier Pallard
2020-03-17  8:33     ` Slava Ovsiienko
2020-03-17  9:19       ` Didier Pallard
2020-03-17  9:25         ` Slava Ovsiienko
2020-03-17  9:41           ` Didier Pallard
2020-10-29 11:29           ` Didier Pallard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).