diff mbox series

net/mlx5: fix Rx descriptor status returned value

Message ID 20200313095659.19000-1-didier.pallard@6wind.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix Rx descriptor status returned value |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed

Commit Message

Didier Pallard March 13, 2020, 9:56 a.m. UTC 
  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(-)

Comments

Slava Ovsiienko March 16, 2020, 4:05 p.m. UTC | #1 
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
Didier Pallard March 16, 2020, 5:24 p.m. UTC | #2 
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
>
>
Slava Ovsiienko March 17, 2020, 8:33 a.m. UTC | #3 
>> 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
Didier Pallard March 17, 2020, 9:19 a.m. UTC | #4 
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
>
Slava Ovsiienko March 17, 2020, 9:25 a.m. UTC | #5 
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
Didier Pallard March 17, 2020, 9:41 a.m. UTC | #6 
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
>
>
Didier Pallard Oct. 29, 2020, 11:29 a.m. UTC | #7 
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
>
diff mbox series

Patch

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