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 E30F5A052A; Mon, 25 Jan 2021 13:38:16 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D2394140F15; Mon, 25 Jan 2021 13:38:16 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mails.dpdk.org (Postfix) with ESMTP id 4FEC8140F12 for ; Mon, 25 Jan 2021 13:38:15 +0100 (CET) IronPort-SDR: KZy+2E2TfpciGoE2L2Pb1abAzKSs2InHkAOqI4H8QZF1qo+q8RZxHCUpCvsKxOR4IIvaCDrr8S 1OZqczZnq6zQ== X-IronPort-AV: E=McAfee;i="6000,8403,9874"; a="264541154" X-IronPort-AV: E=Sophos;i="5.79,373,1602572400"; d="scan'208";a="264541154" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jan 2021 04:38:14 -0800 IronPort-SDR: 1SR/qLquLj+LIuej9xhivt1ztLiGLpoG3SRNL0HfjuudWuwGa81BvPJ1xTqMw+dq2PVhpng/Nt 7EtIlbM0KkSw== X-IronPort-AV: E=Sophos;i="5.79,373,1602572400"; d="scan'208";a="361477546" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.243.89]) ([10.213.243.89]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jan 2021 04:38:12 -0800 To: "Yang, SteveX" , Huisong Li , "dev@dpdk.org" Cc: "Lu, Wenzhuo" , "Li, Xiaoyun" , "Iremonger, Bernard" , "thomas@monjalon.net" , "andrew.rybchenko@oktetlabs.ru" , "Yang, Qiming" , "oulijun@huawei.com" , "huangdaode@huawei.com" , Lijun Ou References: <20201223085152.20866-1-stevex.yang@intel.com> <20210122090110.50453-1-stevex.yang@intel.com> <20210122090110.50453-2-stevex.yang@intel.com> <25b3da8b-a193-d130-0bb5-da2526a48743@huawei.com> From: Ferruh Yigit Message-ID: Date: Mon, 25 Jan 2021 12:38:08 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 1/3] ethdev: fix MTU doesn't update when jumbo frame disabled 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" On 1/25/2021 9:49 AM, Yang, SteveX wrote: > Hi Huisong, > > Thanks for your review. > > The validity of the pair should be > checked from application layer (e.g.: testpmd), > > and the RTE layer should keep open enough to adapt the high-layer requirement. > > I’m not sure if exists some applications/NICs that treat ‘packet size < 1500’ as > JUMBO_FRAME. If so, that also can work as expect with current code. > > @Yigit, Ferruh , please correct me if something > understand wrong. > Hi Huisong, Agree that there is a grey area in the API, the question is if 'JUMBO_FRAME' is set, can application set the 'max_rx_pkt_len' less than "RTE_ETHER_MTU + overhead_len". Lijun (cc'ed) has the same concern. The API documentation, and checks in the 'rte_eth_dev_configure()' enables setting this for a long time, I am reluctant to add this limitation now. Although agree that application should set 'JUMBO_FRAME' properly based on requested 'MTU' value. > BTW, there perhaps are some confused problems about jumbo frame and > max_rx_pkt_len, and Ferruh has scheduled to re-factor this part at release 21.11. > > If you’re interesting about it, please refer to following link: [RFC,v2] doc: > announce max Rx packet len field deprecation - Patchwork (dpdk.org) > > > Thanks & Regards, > > Steve Yang. > > *From:* Huisong Li > *Sent:* Monday, January 25, 2021 3:12 PM > *To:* Yang, SteveX ; dev@dpdk.org > *Cc:* Lu, Wenzhuo ; Li, Xiaoyun ; > Iremonger, Bernard ; thomas@monjalon.net; Yigit, > Ferruh ; andrew.rybchenko@oktetlabs.ru; Yang, Qiming > ; oulijun@huawei.com; huangdaode@huawei.com > *Subject:* Re: [dpdk-dev] [PATCH v3 1/3] ethdev: fix MTU doesn't update when > jumbo frame disabled > > Hi Steve, > > In the current modification, the MTU is updated based on 'max_rx_pkt_len' > regardless of whether jumbo frame is enabled. > > Now, MTU is correct when jumbo frmae is disabled. However, when jumbo frame is > enabled, the MTU value may be inconsistent with > > the definition of the enabled jumbo frame. Like: > > 1/ DEV_RX_OFFLOAD_JUMBO_FRAME is set; > > 2/ max_rx_pkt_len = 1200 > > 3/ dev->data->mtu = 1200 - overhead_len(18) = 1182 > > In rte_eth_dev_configure API, the check for 'max_rx_pkt_len' is as follows: > > if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {  //jumbo frame enabled >         if (dev_conf->rxmode.max_rx_pkt_len > dev_info.max_rx_pktlen) { >             xxxx >             goto rollback; >         } else if (*dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN*) { >             xxxx >             goto rollback; >         } > } else { //jumbo frame disabled > >         if (pktlen < RTE_ETHER_MIN_MTU + overhead_len || >                     pktlen > RTE_ETHER_MTU + overhead_len) >                         /* Use default value */ >                         dev->data->dev_conf.rxmode.max_rx_pkt_len = >                                                 RTE_ETHER_MTU + overhead_len; > > } > > Since the applicatin sets DEV_RX_OFFLOAD_JUMBO_FRAME to enable jumbo frame, and > the framework API needs to update > > the MTU based on 'max_rx_pkt_len', but the framework API uses > *RTE_ETHER_MIN_LEN(64)* to verify the boundary value of > > 'max_rx_pkt_len', instead of "RTE_ETHER_MTU + overhead_len".  As far as I know, > if the applicatin sets DEV_RX_OFFLOAD_JUMBO_FRAME > > and 'max_rx_pkt_len' is 1200, the framework API or driver should return a > failure. As mentioned in this patch set, the jumbo frame > > offload is set only when 'max_rx_pkt_len' requested is greater than > "RTE_ETHER_MTU + eth_overhead" in testpmd. > > I really don't understand it.  How do you understand this behavior? > > Thanks. > > 在 2021/1/22 17:01, Steve Yang 写道: > > The MTU value should be updated to 'max_rx_pkt_len - overhead' > > no matter if the JUMBO FRAME offload enabled. If not update this MTU, > > use will get the wrong MTU info via some command. > > E.g.: 'show port info all' in testpmd tool. > > Actually, the 'max_rx_pkt_len' has been used for other purposes in many > > places now, even though the 'max_rx_pkt_len' is expected 'Only used if > > JUMBO_FRAME enabled'. > > For examples, > > 'max_rx_pkt_len' perhaps can be used as the 'rx_ctx.rxmax' in i40e. > > Fixes: bf0f90d92d30 ("ethdev: fix max Rx packet length check") > > Signed-off-by: Steve Yang > > --- > > lib/librte_ethdev/rte_ethdev.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > > index daf5f24f7e..42857e3b67 100644 > > --- a/lib/librte_ethdev/rte_ethdev.c > > +++ b/lib/librte_ethdev/rte_ethdev.c > > @@ -1421,10 +1421,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, > >                        ret = -EINVAL; > >                        goto rollback; > >                } > > - > > -               /* Scale the MTU size to adapt max_rx_pkt_len */ > > -               dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len - > > -                              overhead_len; > >        } else { > >                uint16_t pktlen = dev_conf->rxmode.max_rx_pkt_len; > >                if (pktlen < RTE_ETHER_MIN_MTU + overhead_len || > > @@ -1434,6 +1430,10 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, > >                                               RTE_ETHER_MTU + overhead_len; > >        } > > > > +       /* Scale the MTU size to adapt max_rx_pkt_len */ > > +       dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len - > > +                              overhead_len; > > + > >        /* > >         * If LRO is enabled, check that the maximum aggregated packet > >         * size is supported by the configured device. >