diff mbox series

[v2,1/3] ethdev: introduce maximum Rx buffer size

Message ID 20231027041523.14518-2-lihuisong@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series introduce maximum Rx buffer size |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

lihuisong (C) Oct. 27, 2023, 4:15 a.m. UTC 
  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. If mbuf data room size
in mempool is greater then the maximum Rx buffer size supported by HW,
the data size application used in each mbuf is just as much as the maximum
Rx buffer size supported by HW 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.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 7 +++++++
 lib/ethdev/rte_ethdev.h | 9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

fengchengwen Oct. 27, 2023, 6:27 a.m. UTC | #1 
Acked-by: Chengwen Feng <fengchengwen@huawei.com>


On 2023/10/27 12:15, 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. If mbuf data room size
> in mempool is greater then the maximum Rx buffer size supported by HW,
> the data size application used in each mbuf is just as much as the maximum
> Rx buffer size supported by HW 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.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>

...
Morten Brørup Oct. 27, 2023, 7:40 a.m. UTC | #2 
> From: Huisong Li [mailto:lihuisong@huawei.com]
> Sent: Friday, 27 October 2023 06.15
> 
> 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. If mbuf data room size
> in mempool is greater then the maximum Rx buffer size supported by HW,
> the data size application used in each mbuf is just as much as the
> maximum
> Rx buffer size supported by HW 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.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---

[...]

> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1723,7 +1723,14 @@ 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;

The comment was moved above min_rx_bufsize, so you must use "/** " instead of "/**< ".

> +	/**
> +	 * Maximum Rx buffer size per descriptor supported by HW.
> +	 * The value is not enforced, information only to application to
> +	 * optimize mbuf size.
> +	 */
> +	uint32_t max_rx_bufsize;

The comment should mention that the value is UINT32_MAX when not specified by the driver.

With those to changes,

Acked-by: Morten Brørup <mb@smartsharesystems.com>
lihuisong (C) Oct. 28, 2023, 1:23 a.m. UTC | #3 
在 2023/10/27 15:40, Morten Brørup 写道:
>> From: Huisong Li [mailto:lihuisong@huawei.com]
>> Sent: Friday, 27 October 2023 06.15
>>
>> 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. If mbuf data room size
>> in mempool is greater then the maximum Rx buffer size supported by HW,
>> the data size application used in each mbuf is just as much as the
>> maximum
>> Rx buffer size supported by HW 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.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
> [...]
>
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -1723,7 +1723,14 @@ 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;
> The comment was moved above min_rx_bufsize, so you must use "/** " instead of "/**< ".
You are right. will fix it in next version.
>
>> +	/**
>> +	 * Maximum Rx buffer size per descriptor supported by HW.
>> +	 * The value is not enforced, information only to application to
>> +	 * optimize mbuf size.
>> +	 */
>> +	uint32_t max_rx_bufsize;
> The comment should mention that the value is UINT32_MAX when not specified by the driver.
Ack
>
> With those to changes,
>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>
>
> .
diff mbox series

Patch

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9dabcb5ae2..eb657a984e 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 vld_bufsize;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
@@ -2158,6 +2159,11 @@  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);
+		vld_bufsize = mbp_buf_size - RTE_PKTMBUF_HEADROOM;
+		if (vld_bufsize > dev_info.max_rx_bufsize)
+			RTE_ETHDEV_LOG(INFO,
+				"Ethdev port_id=%u, the data size application used in each mbuf is just %u instead of the whole data room(%u).\n",
+				port_id, dev_info.max_rx_bufsize, vld_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 +3763,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 2fd3cd808d..5dfb455b2c 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1723,7 +1723,14 @@  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.
+	 */
+	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;