Hi, Matan
On 2020/9/1 23:33, Matan Azrad wrote:
>
> Hi Chengchang
>
> Please see some question below.
>
> From: Chengchang Tang
>> Add a field named rx_buf_size in rte_eth_rxq_info to indicate the buffer size
>> used in receiving packets for HW.
>>
>> In this way, upper-layer users can get this information by calling
>> rte_eth_rx_queue_info_get.
>>
>> Signed-off-by: Chengchang Tang <tangchengch...@huawei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
>> Acked-by: Andrew Rybchenko <arybche...@solarflare.com>
>> ---
>> lib/librte_ethdev/rte_ethdev.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index 70295d7..9fed5cb 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1420,6 +1420,8 @@ 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. */
>> + /**< buffer size used for hardware when receive packets. */
>> + uint16_t rx_buf_size;
>
> Is it the maximum supported Rx buffer by the HW?
> If yes, maybe max_rx_buf_size is better name?
No, it is the Rx buffer size currently used by HW.
IMHO, the structure rte_eth_rxq_info and associated query API are mainly used
to query HW configurations
at runtime or after queue is configured/setup. Therefore, the content of this
structure should be the
current HW configuration.
> Maybe document that 0 means - no limitation by HW?
Yes, there is no need to fill this filed for HW that has no restrictions on it.
I'll add it in v4.
> Must application read it in order to know if its datapath should handle
> multi-segment buffers?
I think it's more appropriate to use scattered_rx to determine if multi-segment
buffers should be handled.
>
> Maybe it will be good to force application to configure scatter when this
> field is valid and smaller than max_rx_pkt_len\max_lro.. (<= room size)...
>
>
>> } __rte_cache_min_aligned;
>>
>> /**
