From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "lihuisong (C)" <lihuisong@huawei.com>
Cc: <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [dpdk-dev] [PATCH v5 5/6] ethdev: unify MTU checks
Date: Mon, 11 Oct 2021 21:15:20 +0100 [thread overview]
Message-ID: <8afc3490-84b1-37e8-6261-ec535cf98e80@intel.com> (raw)
In-Reply-To: <5f1104a7-2e57-d95d-447d-81616f5dc8ea@huawei.com>
On 10/9/2021 12:43 PM, lihuisong (C) wrote:
> Hi, Ferruh
>
> 在 2021/10/8 0:56, Ferruh Yigit 写道:
>> Both 'rte_eth_dev_configure()' & 'rte_eth_dev_set_mtu()' sets MTU but
>> have slightly different checks. Like one checks min MTU against
>> RTE_ETHER_MIN_MTU and other RTE_ETHER_MIN_LEN.
>>
>> Checks moved into common function to unify the checks. Also this has
>> benefit to have common error logs.
>>
>> Suggested-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> lib/ethdev/rte_ethdev.c | 82 ++++++++++++++++++++++++++---------------
>> lib/ethdev/rte_ethdev.h | 2 +-
>> 2 files changed, 54 insertions(+), 30 deletions(-)
>>
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index c2b624aba1a0..0a6e952722ae 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -1336,6 +1336,47 @@ eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu)
>> return overhead_len;
>> }
>> +/* rte_eth_dev_info_get() should be called prior to this function */
>> +static int
>> +eth_dev_validate_mtu(uint16_t port_id, struct rte_eth_dev_info *dev_info,
>> + uint16_t mtu)
>> +{
>> + uint16_t overhead_len;
>> + uint32_t frame_size;
>> +
>> + if (mtu < dev_info->min_mtu) {
>> + RTE_ETHDEV_LOG(ERR,
>> + "MTU (%u) < device min MTU (%u) for port_id %u\n",
>> + mtu, dev_info->min_mtu, port_id);
>> + return -EINVAL;
>> + }
>> + if (mtu > dev_info->max_mtu) {
>> + RTE_ETHDEV_LOG(ERR,
>> + "MTU (%u) > device max MTU (%u) for port_id %u\n",
>> + mtu, dev_info->max_mtu, port_id);
>> + return -EINVAL;
>> + }
>> +
>> + overhead_len = eth_dev_get_overhead_len(dev_info->max_rx_pktlen,
>> + dev_info->max_mtu);
>> + frame_size = mtu + overhead_len;
>> + if (frame_size < RTE_ETHER_MIN_LEN) {
>> + RTE_ETHDEV_LOG(ERR,
>> + "Frame size (%u) < min frame size (%u) for port_id %u\n",
>> + frame_size, RTE_ETHER_MIN_LEN, port_id);
>> + return -EINVAL;
>> + }
>> +
>> + if (frame_size > dev_info->max_rx_pktlen) {
>> + RTE_ETHDEV_LOG(ERR,
>> + "Frame size (%u) > device max frame size (%u) for port_id %u\n",
>> + frame_size, dev_info->max_rx_pktlen, port_id);
>> + return -EINVAL;
>> + }
>
> This function is used to verify the MTU. So "frame_size" is redundant.
>
Yes it is redundant for the drivers that both announce 'max_rx_pktlen' & 'max_mtu',
but stil some drivers doesn't announce the 'max_mtu' values and default value
'UINT16_MAX' is set by ethdev, specially virtual drivers.
That is why I kept both to be in safe side.
> As modified by this patch, dev_info->min_mtu is calculated based on RTE_ETHER_MIN_LEN.
>
And for the min check, for the default 'min_mtu' check is redundant, but for the cases
driver sets "min_mtu < (RTE_ETHER_MIN_LEN - overhead_len)" second check becomes
different limit. I don't know if this happens at all in practice but I think it
doesn't hurt to have both checks to be on safe side.
>> +
>> + return 0;
>> +}
>> +
>> int
>> rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>> const struct rte_eth_conf *dev_conf)
>> @@ -1463,26 +1504,13 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>> goto rollback;
>> }
>> - /*
>> - * Check that the maximum RX packet length is supported by the
>> - * configured device.
>> - */
>> if (dev_conf->rxmode.mtu == 0)
>> dev->data->dev_conf.rxmode.mtu = RTE_ETHER_MTU;
>> - max_rx_pktlen = dev->data->dev_conf.rxmode.mtu + overhead_len;
>> - if (max_rx_pktlen > dev_info.max_rx_pktlen) {
>> - RTE_ETHDEV_LOG(ERR,
>> - "Ethdev port_id=%u max_rx_pktlen %u > max valid value %u\n",
>> - port_id, max_rx_pktlen, dev_info.max_rx_pktlen);
>> - ret = -EINVAL;
>> - goto rollback;
>> - } else if (max_rx_pktlen < RTE_ETHER_MIN_LEN) {
>> - RTE_ETHDEV_LOG(ERR,
>> - "Ethdev port_id=%u max_rx_pktlen %u < min valid value %u\n",
>> - port_id, max_rx_pktlen, RTE_ETHER_MIN_LEN);
>> - ret = -EINVAL;
>> +
>> + ret = eth_dev_validate_mtu(port_id, &dev_info,
>> + dev->data->dev_conf.rxmode.mtu);
>> + if (ret != 0)
>> goto rollback;
>> - }
>> dev->data->mtu = dev->data->dev_conf.rxmode.mtu;
>> @@ -1491,6 +1519,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>> * size is supported by the configured device.
>> */
>> if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
>> + overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen,
>> + dev_info.max_mtu);
>> + max_rx_pktlen = dev->data->dev_conf.rxmode.mtu + overhead_len;
>> if (dev_conf->rxmode.max_lro_pkt_size == 0)
>> dev->data->dev_conf.rxmode.max_lro_pkt_size = max_rx_pktlen;
>> ret = eth_dev_check_lro_pkt_size(port_id,
>> @@ -3437,7 +3468,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>> dev_info->rx_desc_lim = lim;
>> dev_info->tx_desc_lim = lim;
>> dev_info->device = dev->device;
>> - dev_info->min_mtu = RTE_ETHER_MIN_MTU;
>> + dev_info->min_mtu = RTE_ETHER_MIN_LEN - RTE_ETHER_HDR_LEN -
>> + RTE_ETHER_CRC_LEN;
> I suggest that the adjustment to the minimum mtu size is also explicitly reflected in the commit log.
ack, I will
>> dev_info->max_mtu = UINT16_MAX;
>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
>> @@ -3643,21 +3675,13 @@ rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
>> * which relies on dev->dev_ops->dev_infos_get.
>> */
>> if (*dev->dev_ops->dev_infos_get != NULL) {
>> - uint16_t overhead_len;
>> - uint32_t frame_size;
>> -
>> ret = rte_eth_dev_info_get(port_id, &dev_info);
>> if (ret != 0)
>> return ret;
>> - if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
>> - return -EINVAL;
>> -
>> - overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen,
>> - dev_info.max_mtu);
>> - frame_size = mtu + overhead_len;
>> - if (mtu < RTE_ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen)
>> - return -EINVAL;
>> + ret = eth_dev_validate_mtu(port_id, &dev_info, mtu);
>> + if (ret != 0)
>> + return ret;
>> }
>> ret = (*dev->dev_ops->mtu_set)(dev, mtu);
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 4d0f956a4b28..50e124ff631f 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -3056,7 +3056,7 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>> * };
>> *
>> * device = dev->device
>> - * min_mtu = RTE_ETHER_MIN_MTU
>> + * min_mtu = RTE_ETHER_MIN_LEN - RTE_ETHER_HDR_LEN - RTE_ETHER_CRC_LEN
>> * max_mtu = UINT16_MAX
>> *
>> * The following fields will be populated if support for dev_infos_get()
next prev parent reply other threads:[~2021-10-11 20:15 UTC|newest]
Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-09 17:29 [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length Ferruh Yigit
2021-07-09 17:29 ` [dpdk-dev] [PATCH 2/4] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-07-13 13:48 ` Andrew Rybchenko
2021-07-21 12:26 ` Ferruh Yigit
2021-07-18 7:49 ` Xu, Rosen
2021-07-19 14:38 ` Ajit Khaparde
2021-07-09 17:29 ` [dpdk-dev] [PATCH 3/4] ethdev: move check to library for MTU set Ferruh Yigit
2021-07-13 13:56 ` Andrew Rybchenko
2021-07-18 7:52 ` Xu, Rosen
2021-07-09 17:29 ` [dpdk-dev] [PATCH 4/4] ethdev: remove jumbo offload flag Ferruh Yigit
2021-07-13 14:07 ` Andrew Rybchenko
2021-07-21 12:26 ` Ferruh Yigit
2021-07-21 12:39 ` Ferruh Yigit
2021-07-18 7:53 ` Xu, Rosen
2021-07-13 12:47 ` [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length Andrew Rybchenko
2021-07-21 16:46 ` Ferruh Yigit
2021-07-22 1:31 ` Ajit Khaparde
2021-07-22 10:27 ` Ferruh Yigit
2021-07-22 10:38 ` Andrew Rybchenko
2021-07-18 7:45 ` Xu, Rosen
2021-07-19 3:35 ` Huisong Li
2021-07-21 15:29 ` Ferruh Yigit
2021-07-22 7:21 ` Huisong Li
2021-07-22 10:12 ` Ferruh Yigit
2021-07-22 10:15 ` Andrew Rybchenko
2021-07-22 14:43 ` Stephen Hemminger
2021-09-17 1:08 ` Min Hu (Connor)
2021-09-17 8:04 ` Ferruh Yigit
2021-09-17 8:16 ` Min Hu (Connor)
2021-09-17 8:17 ` Min Hu (Connor)
2021-07-22 17:21 ` [dpdk-dev] [PATCH v2 1/6] " Ferruh Yigit
2021-07-22 17:21 ` [dpdk-dev] [PATCH v2 2/6] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-07-22 17:21 ` [dpdk-dev] [PATCH v2 3/6] ethdev: move check to library for MTU set Ferruh Yigit
2021-07-22 17:21 ` [dpdk-dev] [PATCH v2 4/6] ethdev: remove jumbo offload flag Ferruh Yigit
2021-07-22 17:21 ` [dpdk-dev] [PATCH v2 5/6] ethdev: unify MTU checks Ferruh Yigit
2021-07-23 3:29 ` Huisong Li
2021-07-22 17:21 ` [dpdk-dev] [PATCH v2 6/6] examples/ip_reassembly: remove unused parameter Ferruh Yigit
2021-10-01 14:36 ` [dpdk-dev] [PATCH v3 1/6] ethdev: fix max Rx packet length Ferruh Yigit
2021-10-01 14:36 ` [dpdk-dev] [PATCH v3 2/6] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-10-04 5:08 ` Somnath Kotur
2021-10-01 14:36 ` [dpdk-dev] [PATCH v3 3/6] ethdev: move check to library for MTU set Ferruh Yigit
2021-10-04 5:09 ` Somnath Kotur
2021-10-01 14:36 ` [dpdk-dev] [PATCH v3 4/6] ethdev: remove jumbo offload flag Ferruh Yigit
[not found] ` <CAOBf=muYkU2dwgi3iC8Q7pdSNTJsMUwWYdXj14KeN_=_mUGa0w@mail.gmail.com>
2021-10-04 7:55 ` Somnath Kotur
2021-10-05 16:48 ` Ferruh Yigit
2021-10-01 14:36 ` [dpdk-dev] [PATCH v3 5/6] ethdev: unify MTU checks Ferruh Yigit
2021-10-01 14:36 ` [dpdk-dev] [PATCH v3 6/6] examples/ip_reassembly: remove unused parameter Ferruh Yigit
2021-10-01 15:07 ` [dpdk-dev] [PATCH v3 1/6] ethdev: fix max Rx packet length Stephen Hemminger
2021-10-05 16:46 ` Ferruh Yigit
2021-10-05 17:16 ` [dpdk-dev] [PATCH v4 " Ferruh Yigit
2021-10-05 17:16 ` [dpdk-dev] [PATCH v4 2/6] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-10-08 8:39 ` Xu, Rosen
2021-10-05 17:16 ` [dpdk-dev] [PATCH v4 3/6] ethdev: move check to library for MTU set Ferruh Yigit
2021-10-05 17:16 ` [dpdk-dev] [PATCH v4 4/6] ethdev: remove jumbo offload flag Ferruh Yigit
2021-10-08 8:38 ` Xu, Rosen
2021-10-05 17:16 ` [dpdk-dev] [PATCH v4 5/6] ethdev: unify MTU checks Ferruh Yigit
2021-10-05 17:16 ` [dpdk-dev] [PATCH v4 6/6] examples/ip_reassembly: remove unused parameter Ferruh Yigit
2021-10-05 22:07 ` [dpdk-dev] [PATCH v4 1/6] ethdev: fix max Rx packet length Ajit Khaparde
2021-10-06 6:08 ` Somnath Kotur
2021-10-08 8:36 ` Xu, Rosen
2021-10-10 6:30 ` Matan Azrad
2021-10-11 21:59 ` Ferruh Yigit
2021-10-12 7:03 ` Matan Azrad
2021-10-12 11:03 ` Ferruh Yigit
2021-10-07 16:56 ` [dpdk-dev] [PATCH v5 " Ferruh Yigit
2021-10-07 16:56 ` [dpdk-dev] [PATCH v5 2/6] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-10-08 17:20 ` Ananyev, Konstantin
2021-10-09 10:58 ` lihuisong (C)
2021-10-07 16:56 ` [dpdk-dev] [PATCH v5 3/6] ethdev: move check to library for MTU set Ferruh Yigit
2021-10-08 17:19 ` Ananyev, Konstantin
2021-10-07 16:56 ` [dpdk-dev] [PATCH v5 4/6] ethdev: remove jumbo offload flag Ferruh Yigit
2021-10-08 17:11 ` Ananyev, Konstantin
2021-10-09 11:09 ` lihuisong (C)
2021-10-10 5:46 ` Matan Azrad
2021-10-07 16:56 ` [dpdk-dev] [PATCH v5 5/6] ethdev: unify MTU checks Ferruh Yigit
2021-10-08 16:51 ` Ananyev, Konstantin
2021-10-11 19:50 ` Ferruh Yigit
2021-10-09 11:43 ` lihuisong (C)
2021-10-11 20:15 ` Ferruh Yigit [this message]
2021-10-12 4:02 ` lihuisong (C)
2021-10-07 16:56 ` [dpdk-dev] [PATCH v5 6/6] examples/ip_reassembly: remove unused parameter Ferruh Yigit
2021-10-08 16:53 ` Ananyev, Konstantin
2021-10-08 15:57 ` [dpdk-dev] [PATCH v5 1/6] ethdev: fix max Rx packet length Ananyev, Konstantin
2021-10-11 19:47 ` Ferruh Yigit
2021-10-09 10:56 ` lihuisong (C)
2021-10-11 23:53 ` [dpdk-dev] [PATCH v6 " Ferruh Yigit
2021-10-11 23:53 ` [dpdk-dev] [PATCH v6 2/6] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-10-11 23:53 ` [dpdk-dev] [PATCH v6 3/6] ethdev: move check to library for MTU set Ferruh Yigit
2021-10-11 23:53 ` [dpdk-dev] [PATCH v6 4/6] ethdev: remove jumbo offload flag Ferruh Yigit
2021-10-12 17:20 ` Hyong Youb Kim (hyonkim)
2021-10-13 7:16 ` Michał Krawczyk
2021-10-11 23:53 ` [dpdk-dev] [PATCH v6 5/6] ethdev: unify MTU checks Ferruh Yigit
2021-10-12 5:58 ` Andrew Rybchenko
2021-10-11 23:53 ` [dpdk-dev] [PATCH v6 6/6] examples/ip_reassembly: remove unused parameter Ferruh Yigit
2021-10-12 6:02 ` [dpdk-dev] [PATCH v6 1/6] ethdev: fix max Rx packet length Andrew Rybchenko
2021-10-12 9:42 ` Ananyev, Konstantin
2021-10-13 7:08 ` Xu, Rosen
2021-10-15 1:31 ` Hyong Youb Kim (hyonkim)
2021-10-16 0:24 ` Ferruh Yigit
2021-10-18 8:54 ` Ferruh Yigit
2021-10-18 13:48 ` [dpdk-dev] [PATCH v7 " Ferruh Yigit
2021-10-18 13:48 ` [dpdk-dev] [PATCH v7 2/6] ethdev: move jumbo frame offload check to library Ferruh Yigit
2021-10-18 13:48 ` [dpdk-dev] [PATCH v7 3/6] ethdev: move check to library for MTU set Ferruh Yigit
2021-10-18 13:48 ` [dpdk-dev] [PATCH v7 4/6] ethdev: remove jumbo offload flag Ferruh Yigit
2021-10-21 0:43 ` Thomas Monjalon
2021-10-22 11:25 ` Ferruh Yigit
2021-10-22 11:29 ` Andrew Rybchenko
2021-10-18 13:48 ` [dpdk-dev] [PATCH v7 5/6] ethdev: unify MTU checks Ferruh Yigit
2021-10-18 13:48 ` [dpdk-dev] [PATCH v7 6/6] examples/ip_reassembly: remove unused parameter Ferruh Yigit
2021-10-18 17:31 ` [dpdk-dev] [PATCH v7 1/6] ethdev: fix max Rx packet length Ferruh Yigit
2021-11-05 14:19 ` Xueming(Steven) Li
2021-11-05 14:39 ` 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=8afc3490-84b1-37e8-6261-ec535cf98e80@intel.com \
--to=ferruh.yigit@intel.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=lihuisong@huawei.com \
--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).