[RFC] ethdev: introduce maximum Rx buffer size

[lihuisong (C)]

Hi Andrew,
Thanks for your review.


在 2023/8/11 20:07, Andrew Rybchenko 写道:
> On 8/8/23 07:02, Huisong Li wrote:
>> The Rx buffer size stands for the size hardware supported to receive
>> packets in one mbuf. The "min_rx_bufsize" is the minimum buffer hardware
>> supported in Rx. Actually, some engines also have the maximum buffer
>> specification, like, hns3. For these engines, the available data size
>> of one mbuf in Rx also depends on the maximum buffer hardware supported.
>> So introduce maximum Rx buffer size in struct rte_eth_dev_info to report
>> user to avoid memory waste.
>
> I think that the field should be defined as for informational purposes
> only (highlighted in comments). I.e. if application specifies larger Rx
> buffer, driver should accept it and just pass smaller value value to HW.
Ok, will add it.
> Also I think it would be useful to log warning from Rx queue setup
> if provided Rx buffer is larger than maximum reported by the driver.
Ack
>
>>
>> Signed-off-by: Huisong Li <lihuisong at huawei.com>
>> ---
>>   lib/ethdev/rte_ethdev.c | 1 +
>>   lib/ethdev/rte_ethdev.h | 4 ++--
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 0840d2b594..6d1b92e607 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -3689,6 +3689,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct 
>> rte_eth_dev_info *dev_info)
>>       dev_info->min_mtu = RTE_ETHER_MIN_LEN - RTE_ETHER_HDR_LEN -
>>           RTE_ETHER_CRC_LEN;
>>       dev_info->max_mtu = UINT16_MAX;
>> +    dev_info->max_rx_bufsize = UINT32_MAX;
>>         if (*dev->dev_ops->dev_infos_get == NULL)
>>           return -ENOTSUP;
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 04a2564f22..1f0ab9c5d8 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -1779,8 +1779,8 @@ struct rte_eth_dev_info {
>>       struct rte_eth_switch_info switch_info;
>>       /** Supported error handling mode. */
>>       enum rte_eth_err_handle_mode err_handle_mode;
>> -
>> -    uint64_t reserved_64s[2]; /**< Reserved for future fields */
>> +    uint32_t max_rx_bufsize; /**< Maximum size of Rx buffer. */
>
> IMHO, comment should be aligned similar to comments below.
> Since the next release is ABI breaking, I think it should be put
> nearby min_rx_bufsize to make it easier to notice it.
Yes, let's put min/max_rx_bufsize together.
>
>> +    uint32_t reserved_32s[3]; /**< Reserved for future fields */
>>       void *reserved_ptrs[2];   /**< Reserved for future fields */
>>   };
>
> .