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 C679543279; Fri, 3 Nov 2023 13:37:35 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B13744027F; Fri, 3 Nov 2023 13:37:35 +0100 (CET) Received: from agw.arknetworks.am (agw.arknetworks.am [79.141.165.80]) by mails.dpdk.org (Postfix) with ESMTP id 2F0F540273 for ; Fri, 3 Nov 2023 13:37:34 +0100 (CET) Received: from debian (unknown [78.109.75.26]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by agw.arknetworks.am (Postfix) with ESMTPSA id 3DF0CE0179; Fri, 3 Nov 2023 16:37:33 +0400 (+04) DKIM-Filter: OpenDKIM Filter v2.11.0 agw.arknetworks.am 3DF0CE0179 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arknetworks.am; s=default; t=1699015053; bh=j1frj/7FIvAUkR0lO+KCJ+JDHUAfEKQYLVr5FCCEb5A=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=kIimiEBpuQ7PoZF4vKU9J0MLACIgmiH9xKuyc6oXaAE44jDMkyecrByt7j0cVIVDx h9xCheEyt2lkYVj3Uyoc4dWXuxvirAOR4XUmjpdxq9lBrg+LKFh6BSZwvFh6Dqsdbt vvIGVBcC3gFPFvQMUwDNu8k730tMO8i0oATg18rtyypAcLVNF0EgYiT5Pe7DKCalQT R/fMTEMBNKaIXS9TxM8LHUrWlk6i6M57l/6EFwfBPYs48HV0yh3+fbpBUgHZWphIu/ 1YBDXoJ9Z7ZonLG3qTdBUtLwFDymYdul4XFv7CdUZcx4Hzpvukn7C7d1hf4TLVFTfa 9sUqjr3q4eigQ== Date: Fri, 3 Nov 2023 16:37:24 +0400 (+04) From: Ivan Malov To: Huisong Li cc: dev@dpdk.org, Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , liuyonglong@huawei.com Subject: Re: [PATCH v5 1/3] ethdev: introduce maximum Rx buffer size In-Reply-To: <20231103102759.18886-2-lihuisong@huawei.com> Message-ID: <94b5b3fc-1006-0069-c772-fefeffffc990@arknetworks.am> References: <20230808040234.12947-1-lihuisong@huawei.com> <20231103102759.18886-1-lihuisong@huawei.com> <20231103102759.18886-2-lihuisong@huawei.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323328-1234145993-1699015052=:5778" 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 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323328-1234145993-1699015052=:5778 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT Hi, (minor notes below) On Fri, 3 Nov 2023, Huisong Li wrote: > The "min_rx_bufsize" in struct rte_eth_dev_info stands for the minimum > Rx buffer size supported by hardware. Actually, some engines also have > the maximum Rx buffer specification, like, hns3, i40e and so on. If mbuf > data room size in mempool is greater then the maximum Rx buffer size > per descriptor supported by HW, the data size application used in each > mbuf is just as much as the maximum Rx buffer size instead of the whole > data room size. > > So introduce maximum Rx buffer size which is not enforced just to > report user to avoid memory waste. In addition, fix the comment for > the "min_rx_bufsize" to make it be more specific. Consider: So introduce maximum Rx buffer size. It does not enforce anything; its sole purpose is to inform the user of possible memory waste. > > Signed-off-by: Huisong Li > Acked-by: Chengwen Feng > Acked-by: Morten Brørup > --- > doc/guides/rel_notes/release_23_11.rst | 7 +++++++ > lib/ethdev/rte_ethdev.c | 8 ++++++++ > lib/ethdev/rte_ethdev.h | 10 +++++++++- > 3 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst > index 95db98d098..d4f7d5b266 100644 > --- a/doc/guides/rel_notes/release_23_11.rst > +++ b/doc/guides/rel_notes/release_23_11.rst > @@ -122,6 +122,13 @@ New Features > a group's miss actions, which are the actions to be performed on packets > that didn't match any of the flow rules in the group. > > +* **Added maximum Rx buffer size to report.** Consider: Added maximum Rx buffer size reporting. > + > + Introduced the ``max_rx_bufsize`` field representing the maximum Rx > + buffer size per descriptor supported by HW in structure ``rte_eth_dev_info`` > + to report user and to avoid wasting space of mempool. > + Its value is UINT32_MAX if driver doesn't report it. > + > * **Updated Amazon ena (Elastic Network Adapter) net driver.** > > * Upgraded ENA HAL to latest version. > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > index af23ac0ad0..24b708f772 100644 > --- a/lib/ethdev/rte_ethdev.c > +++ b/lib/ethdev/rte_ethdev.c > @@ -2112,6 +2112,7 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id, > struct rte_eth_dev *dev; > struct rte_eth_dev_info dev_info; > struct rte_eth_rxconf local_conf; > + uint32_t buf_data_size; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > @@ -2158,6 +2159,12 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id, > return ret; > > mbp_buf_size = rte_pktmbuf_data_room_size(mp); > + buf_data_size = mbp_buf_size - RTE_PKTMBUF_HEADROOM; > + if (buf_data_size > dev_info.max_rx_bufsize) > + RTE_ETHDEV_LOG(DEBUG, > + "For port_id=%u, the mbuf data buffer size (%u) is bigger than " Consider: is larger than > + "max buffer size (%u) device can utilize, so mbuf size can be reduced.\n", > + port_id, buf_data_size, dev_info.max_rx_bufsize); > } else if (rx_conf != NULL && rx_conf->rx_nseg > 0) { > const struct rte_eth_rxseg_split *rx_seg; > uint16_t n_seg; > @@ -3757,6 +3764,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 a53dd5a1ef..7133b73d26 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -1723,7 +1723,15 @@ struct rte_eth_dev_info { > uint16_t min_mtu; /**< Minimum MTU allowed */ > uint16_t max_mtu; /**< Maximum MTU allowed */ > const uint32_t *dev_flags; /**< Device flags */ > - uint32_t min_rx_bufsize; /**< Minimum size of Rx buffer. */ > + /** Minimum Rx buffer size per descriptor supported by HW. */ > + uint32_t min_rx_bufsize; > + /** > + * Maximum Rx buffer size per descriptor supported by HW. > + * The value is not enforced, information only to application to > + * optimize mbuf size. Its value is UINT32_MAX when not specified > + * by the driver. Consider: The value is not enforced. It's a hint for the application to optimise mbuf size on mempool allocation. If the driver does not restrict the maximum buffer size, reported value will be UINT32_MAX. > + */ > + uint32_t max_rx_bufsize; > uint32_t max_rx_pktlen; /**< Maximum configurable length of Rx pkt. */ > /** Maximum configurable size of LRO aggregated packet. */ > uint32_t max_lro_pkt_size; > -- > 2.33.0 > > Other than that, Acked-by: Ivan Malov Thank you. --8323328-1234145993-1699015052=:5778--