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 DE033A0C41; Sat, 9 Oct 2021 13:43:15 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6F21740042; Sat, 9 Oct 2021 13:43:15 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id A189340040 for ; Sat, 9 Oct 2021 13:43:13 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.55]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4HRNSM07fvzVmVt; Sat, 9 Oct 2021 19:41:39 +0800 (CST) Received: from dggema767-chm.china.huawei.com (10.1.198.209) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.8; Sat, 9 Oct 2021 19:43:11 +0800 Received: from [10.67.103.231] (10.67.103.231) by dggema767-chm.china.huawei.com (10.1.198.209) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2308.8; Sat, 9 Oct 2021 19:43:10 +0800 Message-ID: <5f1104a7-2e57-d95d-447d-81616f5dc8ea@huawei.com> Date: Sat, 9 Oct 2021 19:43:10 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 To: Ferruh Yigit CC: , Thomas Monjalon , Andrew Rybchenko References: <20211001143624.3744505-1-ferruh.yigit@intel.com> <20211007165626.2941995-1-ferruh.yigit@intel.com> <20211007165626.2941995-5-ferruh.yigit@intel.com> From: "lihuisong (C)" In-Reply-To: <20211007165626.2941995-5-ferruh.yigit@intel.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggema767-chm.china.huawei.com (10.1.198.209) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v5 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" 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 > Signed-off-by: Ferruh Yigit > --- > 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. As modified by this patch, dev_info->min_mtu is calculated based on RTE_ETHER_MIN_LEN. > + > + 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. > 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()