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 4C369A034F; Tue, 12 Oct 2021 01:54:55 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BA01641161; Tue, 12 Oct 2021 01:54:51 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id CEAC64115B for ; Tue, 12 Oct 2021 01:54:48 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10134"; a="290490987" X-IronPort-AV: E=Sophos;i="5.85,366,1624345200"; d="scan'208";a="290490987" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Oct 2021 16:54:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,366,1624345200"; d="scan'208";a="591518389" Received: from silpixa00399752.ir.intel.com (HELO silpixa00399752.ger.corp.intel.com) ([10.237.222.27]) by orsmga004.jf.intel.com with ESMTP; 11 Oct 2021 16:54:46 -0700 From: Ferruh Yigit To: Thomas Monjalon , Andrew Rybchenko Cc: Ferruh Yigit , dev@dpdk.org, Huisong Li , Konstantin Ananyev Date: Tue, 12 Oct 2021 00:53:43 +0100 Message-Id: <20211011235345.851742-5-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011235345.851742-1-ferruh.yigit@intel.com> References: <20211001143624.3744505-1-ferruh.yigit@intel.com> <20211011235345.851742-1-ferruh.yigit@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v6 5/6] ethdev: unify MTU checks 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 Sender: "dev" 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. Default 'dev_info->min_mtu' (the one set by ethdev if driver doesn't provide one), changed to ('RTE_ETHER_MIN_LEN' - overhead). Previously it was 'RTE_ETHER_MIN_MTU' which is min MTU for IPv4 packets. Since the intention is to provide min MTU corresponding minimum frame size, new default value suits better. Suggested-by: Huisong Li Signed-off-by: Ferruh Yigit Acked-by: Konstantin Ananyev --- v6: * Commit log updated to document new default 'dev_info->min_mtu' value --- lib/ethdev/rte_ethdev.c | 91 +++++++++++++++++++++++++---------------- lib/ethdev/rte_ethdev.h | 2 +- 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index ea90bb19680a..2cf50f3abc94 100644 --- a/lib/ethdev/rte_ethdev.c +++ b/lib/ethdev/rte_ethdev.c @@ -1350,6 +1350,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) +{ + uint32_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; + } + + 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) @@ -1357,8 +1398,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, struct rte_eth_dev *dev; struct rte_eth_dev_info dev_info; struct rte_eth_conf orig_conf; - uint32_t max_rx_pktlen; - uint32_t overhead_len; int diag; int ret; uint16_t old_mtu; @@ -1407,10 +1446,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, if (ret != 0) goto rollback; - /* Get the real Ethernet overhead length */ - overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen, - dev_info.max_mtu); - /* If number of queues specified by application for both Rx and Tx is * zero, use driver preferred values. This cannot be done individually * as it is valid for either Tx or Rx (but not both) to be zero. @@ -1477,26 +1512,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; @@ -1505,6 +1527,12 @@ 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) { + uint32_t max_rx_pktlen; + uint32_t overhead_len; + + 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, @@ -3417,7 +3445,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; dev_info->max_mtu = UINT16_MAX; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP); @@ -3623,21 +3652,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 5d64cd6d9504..d7d01d99640c 100644 --- a/lib/ethdev/rte_ethdev.h +++ b/lib/ethdev/rte_ethdev.h @@ -3026,7 +3026,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() -- 2.31.1