* [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup
@ 2021-08-20 12:47 Andrew Rybchenko
2021-09-13 19:25 ` Maxime Coquelin
2021-09-14 11:24 ` Maxime Coquelin
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Rybchenko @ 2021-08-20 12:47 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia, Shreyansh Jain, Remy Horton,
Ferruh Yigit, Thomas Monjalon
Cc: dev, Ivan Ilchenko, stable
From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
Rx queue setup callback allows to use the whole ring when
descriptor number argument equals zero. There's no point to
handle zero in any way since RTE Rx queue setup function
rte_eth_rx_queue_setup() doesn't pass zero using fallback
values.
Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
Cc: stable@dpdk.org
Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
drivers/net/virtio/virtio_rxtx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 8a48fba5cc..18f03c9fc9 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -706,7 +706,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
vq->vq_free_thresh = rx_free_thresh;
- if (nb_desc == 0 || nb_desc > vq->vq_nentries)
+ if (nb_desc > vq->vq_nentries)
nb_desc = vq->vq_nentries;
vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup
2021-08-20 12:47 [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup Andrew Rybchenko
@ 2021-09-13 19:25 ` Maxime Coquelin
2021-09-14 6:40 ` Andrew Rybchenko
2021-09-14 11:24 ` Maxime Coquelin
1 sibling, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2021-09-13 19:25 UTC (permalink / raw)
To: Andrew Rybchenko, Chenbo Xia, Shreyansh Jain, Remy Horton,
Ferruh Yigit, Thomas Monjalon
Cc: dev, Ivan Ilchenko, stable
On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>
> Rx queue setup callback allows to use the whole ring when
> descriptor number argument equals zero. There's no point to
> handle zero in any way since RTE Rx queue setup function
> rte_eth_rx_queue_setup() doesn't pass zero using fallback
> values.
>
> Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> drivers/net/virtio/virtio_rxtx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 8a48fba5cc..18f03c9fc9 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -706,7 +706,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
> }
> vq->vq_free_thresh = rx_free_thresh;
>
> - if (nb_desc == 0 || nb_desc > vq->vq_nentries)
> + if (nb_desc > vq->vq_nentries)
> nb_desc = vq->vq_nentries;
> vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
>
>
Is that really a fix?
I see it more like an optimization in a cold path, so maybe it is not
worth backporting?
Other than that:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup
2021-09-13 19:25 ` Maxime Coquelin
@ 2021-09-14 6:40 ` Andrew Rybchenko
2021-09-14 7:26 ` Maxime Coquelin
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Rybchenko @ 2021-09-14 6:40 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia, Shreyansh Jain, Remy Horton,
Ferruh Yigit, Thomas Monjalon
Cc: dev, Ivan Ilchenko, stable
On 9/13/21 10:25 PM, Maxime Coquelin wrote:
>
>
> On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
>> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>>
>> Rx queue setup callback allows to use the whole ring when
>> descriptor number argument equals zero. There's no point to
>> handle zero in any way since RTE Rx queue setup function
>> rte_eth_rx_queue_setup() doesn't pass zero using fallback
>> values.
>>
>> Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> ---
>> drivers/net/virtio/virtio_rxtx.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>> index 8a48fba5cc..18f03c9fc9 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -706,7 +706,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>> }
>> vq->vq_free_thresh = rx_free_thresh;
>>
>> - if (nb_desc == 0 || nb_desc > vq->vq_nentries)
>> + if (nb_desc > vq->vq_nentries)
>> nb_desc = vq->vq_nentries;
>> vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
>>
>>
>
> Is that really a fix?
> I see it more like an optimization in a cold path, so maybe it is not
> worth backporting?
The main idea is not an optimization, but simplification of
the code to make it easier to understand. Less special
cases is better.
I agree that it does not make sense to backport it.
> Other than that:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Andrew.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup
2021-09-14 6:40 ` Andrew Rybchenko
@ 2021-09-14 7:26 ` Maxime Coquelin
2021-09-14 7:33 ` Andrew Rybchenko
0 siblings, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2021-09-14 7:26 UTC (permalink / raw)
To: Andrew Rybchenko, Chenbo Xia, Shreyansh Jain, Remy Horton,
Ferruh Yigit, Thomas Monjalon
Cc: dev, Ivan Ilchenko, stable
On 9/14/21 8:40 AM, Andrew Rybchenko wrote:
> On 9/13/21 10:25 PM, Maxime Coquelin wrote:
>>
>>
>> On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
>>> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>>>
>>> Rx queue setup callback allows to use the whole ring when
>>> descriptor number argument equals zero. There's no point to
>>> handle zero in any way since RTE Rx queue setup function
>>> rte_eth_rx_queue_setup() doesn't pass zero using fallback
>>> values.
>>>
>>> Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> ---
>>> drivers/net/virtio/virtio_rxtx.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>>> index 8a48fba5cc..18f03c9fc9 100644
>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>> @@ -706,7 +706,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>> }
>>> vq->vq_free_thresh = rx_free_thresh;
>>>
>>> - if (nb_desc == 0 || nb_desc > vq->vq_nentries)
>>> + if (nb_desc > vq->vq_nentries)
>>> nb_desc = vq->vq_nentries;
>>> vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
>>>
>>>
>>
>> Is that really a fix?
>> I see it more like an optimization in a cold path, so maybe it is not
>> worth backporting?
>
> The main idea is not an optimization, but simplification of
> the code to make it easier to understand. Less special
> cases is better.
>
> I agree that it does not make sense to backport it.
Ok, thanks. I'll will remove the Fixes tag while applying, no need to
resubmit.
Maxime
>
>> Other than that:
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Thanks,
> Andrew.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup
2021-09-14 7:26 ` Maxime Coquelin
@ 2021-09-14 7:33 ` Andrew Rybchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Rybchenko @ 2021-09-14 7:33 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia, Shreyansh Jain, Remy Horton,
Ferruh Yigit, Thomas Monjalon
Cc: dev, Ivan Ilchenko, stable
On 9/14/21 10:26 AM, Maxime Coquelin wrote:
>
>
> On 9/14/21 8:40 AM, Andrew Rybchenko wrote:
>> On 9/13/21 10:25 PM, Maxime Coquelin wrote:
>>>
>>>
>>> On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
>>>> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>>>>
>>>> Rx queue setup callback allows to use the whole ring when
>>>> descriptor number argument equals zero. There's no point to
>>>> handle zero in any way since RTE Rx queue setup function
>>>> rte_eth_rx_queue_setup() doesn't pass zero using fallback
>>>> values.
>>>>
>>>> Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> ---
>>>> drivers/net/virtio/virtio_rxtx.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>>>> index 8a48fba5cc..18f03c9fc9 100644
>>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>>> @@ -706,7 +706,7 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>>> }
>>>> vq->vq_free_thresh = rx_free_thresh;
>>>>
>>>> - if (nb_desc == 0 || nb_desc > vq->vq_nentries)
>>>> + if (nb_desc > vq->vq_nentries)
>>>> nb_desc = vq->vq_nentries;
>>>> vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
>>>>
>>>>
>>>
>>> Is that really a fix?
>>> I see it more like an optimization in a cold path, so maybe it is not
>>> worth backporting?
>>
>> The main idea is not an optimization, but simplification of
>> the code to make it easier to understand. Less special
>> cases is better.
>>
>> I agree that it does not make sense to backport it.
>
> Ok, thanks. I'll will remove the Fixes tag while applying, no need to
> resubmit.
Thanks,
Andrew.
> Maxime
>>
>>> Other than that:
>>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> Thanks,
>> Andrew.
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup
2021-08-20 12:47 [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup Andrew Rybchenko
2021-09-13 19:25 ` Maxime Coquelin
@ 2021-09-14 11:24 ` Maxime Coquelin
1 sibling, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2021-09-14 11:24 UTC (permalink / raw)
To: Andrew Rybchenko, Chenbo Xia, Shreyansh Jain, Remy Horton,
Ferruh Yigit, Thomas Monjalon
Cc: dev, Ivan Ilchenko, stable
On 8/20/21 2:47 PM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
>
> Rx queue setup callback allows to use the whole ring when
> descriptor number argument equals zero. There's no point to
> handle zero in any way since RTE Rx queue setup function
> rte_eth_rx_queue_setup() doesn't pass zero using fallback
> values.
>
> Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> drivers/net/virtio/virtio_rxtx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied to dpdk-next-virtio/main.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-14 11:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 12:47 [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup Andrew Rybchenko
2021-09-13 19:25 ` Maxime Coquelin
2021-09-14 6:40 ` Andrew Rybchenko
2021-09-14 7:26 ` Maxime Coquelin
2021-09-14 7:33 ` Andrew Rybchenko
2021-09-14 11:24 ` Maxime Coquelin
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).