Re: [dpdk-dev] [RFC] ethdev: add a field for rte_eth_rxq_info

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 6/24/2020 4:48 AM, Chengchang Tang wrote:
> 
> On 2020/6/23 17:30, Andrew Rybchenko wrote:
>> On 6/23/20 9:48 AM, Chengchang Tang wrote:
>>> In common practice, PMD configure the rx_buf_size according to the data
>>> room size of the object in mempool. But in fact the final value is related
>>> to the specifications of hw, and its values will affect the number of
>>> fragments in recieving pkts.
>>>
>>> At present, we seem to have no way to espose relevant information to upper
>>> layer users.
>>>
>>> Add a field named rx_bufsize in rte_eth_rxq_info to indicate the buffer
>>> size used in recieving pkts for hw.
>>>
>>
>> I'm OK with the change in general.
>> I'm unsure which name to use: 'rx_buf_size' or 'rx_bursize',
>> since I found both 'min_rx_buf_size' and 'min_rx_bufsize' in
>> ethdev.
>>
>> I think it is important to update PMDs which provides the
>> information to fill the field in.
> 
> My plan is to divide the subsequent series into two patches,
> one to modify rte_eth_rxq_info, and one to add our hns3 PMD
> implementation of rxq_info_get. Should i update all the PMDs
> that provide this information and test programs such as
> testpmd at the same time?

Hi Chengchang, Andrew,

No objection to the change, but it should be crystal clear what is added. These
are for PMD developers to implement and when it is not clear we end up having
different implementations and inconsistencies.

There is already some confusion for the Rx packet size etc.. my concern is
adding more to it, here all we have is "size of RX buffer." comment, I think we
need more.
Adding a PMD implementation and testpmd updates helps to clarify the
intention/usage, so I suggest sending them as a single patch with this one.

Updating all PMDs is a bigger ask and sometimes too hard because of lack of
knowledge on the internals of other PMDs, although this is causing feature gaps
time to time, we are not mandating this to developers, so please update as many
PMD as you can, that you are confident, rest should be done by their maintainers.

>>
>>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>
>>> ---
>>>  lib/librte_ethdev/rte_ethdev.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>>> index 0f6d053..82b7e98 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -1306,6 +1306,7 @@ struct rte_eth_rxq_info {
>>>  	struct rte_eth_rxconf conf; /**< queue config parameters. */
>>>  	uint8_t scattered_rx;       /**< scattered packets RX supported. */
>>>  	uint16_t nb_desc;           /**< configured number of RXDs. */
>>> +	uint16_t rx_bufsize;        /**< size of RX buffer. */
>>>  } __rte_cache_min_aligned;
>>>
>>>  /**
>>> --
>>> 2.7.4
>>>
>>
>>
>> .
>>
>