From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 157394306E;
	Tue, 15 Aug 2023 10:16:29 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id DEB8A427E9;
	Tue, 15 Aug 2023 10:16:28 +0200 (CEST)
Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187])
 by mails.dpdk.org (Postfix) with ESMTP id 5FA6E406B6
 for <dev@dpdk.org>; Tue, 15 Aug 2023 10:16:27 +0200 (CEST)
Received: from kwepemm600004.china.huawei.com (unknown [172.30.72.55])
 by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4RQ3wP6W0RzrSPD;
 Tue, 15 Aug 2023 16:15:01 +0800 (CST)
Received: from [10.67.103.231] (10.67.103.231) by
 kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2507.31; Tue, 15 Aug 2023 16:16:21 +0800
Message-ID: <96dc828d-3787-13a9-0b75-4ac1bd2097b1@huawei.com>
Date: Tue, 15 Aug 2023 16:16:20 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [RFC] ethdev: introduce maximum Rx buffer size
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>, <dev@dpdk.org>
CC: <thomas@monjalon.net>, <ferruh.yigit@amd.com>, <liuyonglong@huawei.com>
References: <20230808040234.12947-1-lihuisong@huawei.com>
 <38ba7f44-675f-a668-2233-5f089795fa53@oktetlabs.ru>
From: "lihuisong (C)" <lihuisong@huawei.com>
In-Reply-To: <38ba7f44-675f-a668-2233-5f089795fa53@oktetlabs.ru>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.67.103.231]
X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To
 kwepemm600004.china.huawei.com (7.193.23.242)
X-CFilter-Loop: Reflected
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

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@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 */
>>   };
>
> .