Re: [dpdk-dev] [PATCH] net/virtio: remove handling of zero desc number on RxQ setup

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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