From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 , CC: , , References: <20230808040234.12947-1-lihuisong@huawei.com> <38ba7f44-675f-a668-2233-5f089795fa53@oktetlabs.ru> From: "lihuisong (C)" 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 >> --- >>   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 */ >>   }; > > .