From: Ferruh Yigit <ferruh.yigit@amd.com>
To: "lihuisong (C)" <lihuisong@huawei.com>,
dev@dpdk.org, "Thomas Monjalon" <thomas@monjalon.net>,
"Andrew Rybchenko" <andrew.rybchenko@oktetlabs.ru>,
"Morten Brørup" <mb@smartsharesystems.com>,
"Stephen Hemminger" <stephen@networkplumber.org>
Cc: liuyonglong@huawei.com
Subject: Re: [PATCH v4 1/3] ethdev: introduce maximum Rx buffer size
Date: Fri, 3 Nov 2023 03:30:52 +0000 [thread overview]
Message-ID: <769abd53-d93b-440e-b542-aee3e244f147@amd.com> (raw)
In-Reply-To: <85d5dacf-4cb2-3eb9-87e0-29da61726b80@huawei.com>
On 11/3/2023 2:21 AM, lihuisong (C) wrote:
>
> 在 2023/11/3 0:35, Ferruh Yigit 写道:
>> On 11/2/2023 12:16 PM, 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.
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>> ---
>>> doc/guides/rel_notes/release_23_11.rst | 7 +++++++
>>> lib/ethdev/rte_ethdev.c | 7 +++++++
>>> lib/ethdev/rte_ethdev.h | 10 +++++++++-
>>> 3 files changed, 23 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.**
>>> +
>>> + 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..03539bb6c3 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;
>>>
>> I guess 'vld' is valid, but that is not accurate. Can we rename it
>> something like 'buf_data_size'?
> Ack
>>
>>> 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,
>>> + "For port_id=%u the max data size per mbuf in Rx is
>>> %u instead of the whole data room(%u).\n",
>>> + port_id, dev_info.max_rx_bufsize, vld_bufsize);
>>>
>> This log gives some information, but it is not clear what user should
>> do. My concern is this can confuse end users (not developers but users
>> of dpdk application).
>>
>> Since audience of this message is DPDK application developer, most of
>> the times end user don't have anything to do with this log, what do you
>> think to change log level to 'debug'?
> Ack
>>
>> Also can we add some action to it, something like:
>> "mbuf data buffer size is bigger than what device can utilize, consider
>> reducing mbuf size."
> What do you think of this log, Ferruh?
> "The mbuf data buffer size (%u) is bigger than the max size (%u) device
> with port_id=%u can utilize, consider reducing mbuf size.\n",
>
Can move port_id to the beginning, and can make wording passive, what about:
"For port_id=%u, the mbuf data buffer size (%u) is bigger than max
buffer size (%u) device can utilize, mbuf size can be reduced.\n"
>>
>>
>>
>>> } 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 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.
>>> + */
>>> + 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;
>> .
next prev parent reply other threads:[~2023-11-03 3:31 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-08 4:02 [RFC] " Huisong Li
2023-08-11 12:07 ` Andrew Rybchenko
2023-08-15 8:16 ` lihuisong (C)
2023-08-15 11:10 ` [PATCH v1 0/3] " Huisong Li
2023-08-15 11:10 ` [PATCH v1 1/3] ethdev: " Huisong Li
2023-09-28 15:56 ` Ferruh Yigit
2023-10-24 12:21 ` lihuisong (C)
2023-08-15 11:10 ` [PATCH v1 2/3] app/testpmd: add maximum Rx buffer size display Huisong Li
2023-08-15 11:10 ` [PATCH v1 3/3] net/hns3: report maximum buffer size Huisong Li
2023-10-27 4:15 ` [PATCH v2 0/3] introduce maximum Rx " Huisong Li
2023-10-27 4:15 ` [PATCH v2 1/3] ethdev: " Huisong Li
2023-10-27 6:27 ` fengchengwen
2023-10-27 7:40 ` Morten Brørup
2023-10-28 1:23 ` lihuisong (C)
2023-10-27 4:15 ` [PATCH v2 2/3] app/testpmd: add maximum Rx buffer size display Huisong Li
2023-10-27 6:28 ` fengchengwen
2023-10-27 4:15 ` [PATCH v2 3/3] net/hns3: report maximum buffer size Huisong Li
2023-10-27 6:17 ` fengchengwen
2023-10-28 1:48 ` [PATCH v3 0/3] introduce maximum Rx " Huisong Li
2023-10-28 1:48 ` [PATCH v3 1/3] ethdev: " Huisong Li
2023-10-29 15:43 ` Stephen Hemminger
2023-10-30 3:08 ` lihuisong (C)
2023-10-28 1:48 ` [PATCH v3 2/3] app/testpmd: add maximum Rx buffer size display Huisong Li
2023-10-28 1:48 ` [PATCH v3 3/3] net/hns3: report maximum buffer size Huisong Li
2023-10-29 15:48 ` [PATCH v3 0/3] introduce maximum Rx " Stephen Hemminger
2023-10-30 1:25 ` lihuisong (C)
2023-10-30 18:48 ` Stephen Hemminger
2023-10-31 2:57 ` lihuisong (C)
2023-10-31 7:48 ` Morten Brørup
2023-10-31 15:40 ` Stephen Hemminger
2023-11-01 2:36 ` lihuisong (C)
2023-11-01 16:08 ` Stephen Hemminger
2023-11-02 1:59 ` lihuisong (C)
2023-11-02 16:23 ` Ferruh Yigit
2023-11-02 16:51 ` Morten Brørup
2023-11-02 17:05 ` Ferruh Yigit
2023-11-02 17:12 ` Morten Brørup
2023-11-02 17:35 ` Ferruh Yigit
2023-11-03 2:13 ` lihuisong (C)
2023-11-02 12:16 ` [PATCH v4 " Huisong Li
2023-11-02 12:16 ` [PATCH v4 1/3] ethdev: " Huisong Li
2023-11-02 16:35 ` Ferruh Yigit
2023-11-03 2:21 ` lihuisong (C)
2023-11-03 3:30 ` Ferruh Yigit [this message]
2023-11-03 6:27 ` lihuisong (C)
2023-11-02 12:16 ` [PATCH v4 2/3] app/testpmd: add maximum Rx buffer size display Huisong Li
2023-11-02 16:42 ` Ferruh Yigit
2023-11-03 2:39 ` lihuisong (C)
2023-11-03 3:53 ` Ferruh Yigit
2023-11-03 6:37 ` lihuisong (C)
2023-11-02 12:16 ` [PATCH v4 3/3] net/hns3: report maximum buffer size Huisong Li
2023-11-03 10:27 ` [PATCH v5 0/3] introduce maximum Rx " Huisong Li
2023-11-03 10:27 ` [PATCH v5 1/3] ethdev: " Huisong Li
2023-11-03 12:37 ` Ivan Malov
2023-11-03 10:27 ` [PATCH v5 2/3] app/testpmd: add maximum Rx buffer size display Huisong Li
2023-11-03 10:27 ` [PATCH v5 3/3] net/hns3: report maximum buffer size Huisong Li
2023-11-03 11:53 ` [PATCH v5 0/3] introduce maximum Rx " Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=769abd53-d93b-440e-b542-aee3e244f147@amd.com \
--to=ferruh.yigit@amd.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=lihuisong@huawei.com \
--cc=liuyonglong@huawei.com \
--cc=mb@smartsharesystems.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).