From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 749EBA0524; Fri, 27 Nov 2020 18:08:58 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AA3EFC94E; Fri, 27 Nov 2020 18:08:55 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id F146DC93C for ; Fri, 27 Nov 2020 18:08:52 +0100 (CET) IronPort-SDR: vQldb6Ww74Tml6or8M7QAJ54XcpdGtjQd8CKGFiVV2oGoSCcRK2IhTo6yXVr9hACjWK6lysK+2 bVZulmgat+uw== X-IronPort-AV: E=McAfee;i="6000,8403,9818"; a="172571354" X-IronPort-AV: E=Sophos;i="5.78,375,1599548400"; d="scan'208";a="172571354" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Nov 2020 09:08:49 -0800 IronPort-SDR: ZO/A4r8UKR4pqRYP3Am6e3cIogHyN5bLugode4hfQC2viKtM2qJ3gsVwYxntzWLB+iLMaee2q1 TESL+qIcDrTA== X-IronPort-AV: E=Sophos;i="5.78,375,1599548400"; d="scan'208";a="537700470" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.6.231]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 27 Nov 2020 09:08:46 -0800 Date: Fri, 27 Nov 2020 17:08:42 +0000 From: Bruce Richardson To: Andrew Rybchenko Cc: Ferruh Yigit , "Yang, SteveX" , "Ananyev, Konstantin" , "dev@dpdk.org" , Thomas Monjalon , "Zhang, Qi Z" , Ajit Khaparde , "jerinj@marvell.com" , Viacheslav Ovsiienko , Matan Azrad , "Xing, Beilei" , "Lu, Wenzhuo" , "Iremonger, Bernard" , "Yang, Qiming" , "mdr@ashroe.eu" , "nhorman@tuxdriver.com" Message-ID: <20201127170842.GH1561@bricha3-MOBL.ger.corp.intel.com> References: <20201028030334.30300-1-stevex.yang@intel.com> <20201102085234.72779-1-stevex.yang@intel.com> <20201102085234.72779-3-stevex.yang@intel.com> <67bb56e7-d12f-d364-1cc4-dfefd20739b0@oktetlabs.ru> <8721bb8d-e361-f815-257f-08d531e23fba@intel.com> <1e5d6121-0650-3d48-1482-d15f4a598aff@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v8 2/2] doc: annouce deprecation of jumbo frame flag condition X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 Fri, Nov 27, 2020 at 03:19:43PM +0300, Andrew Rybchenko wrote: > On 11/24/20 8:46 PM, Ferruh Yigit wrote: > > On 11/3/2020 9:07 AM, Yang, SteveX wrote: > >>> -----Original Message----- > >>> From: Ananyev, Konstantin > >>> Sent: Tuesday, November 3, 2020 12:05 AM > >>> To: Yigit, Ferruh ; Andrew Rybchenko > >>> ; Yang, SteveX ; > >>> dev@dpdk.org > >>> Cc: Xing, Beilei ; Lu, Wenzhuo > >>> ; Iremonger, Bernard > >>> ; Yang, Qiming ; > >>> mdr@ashroe.eu; nhorman@tuxdriver.com > >>> Subject: RE: [dpdk-dev] [PATCH v8 2/2] doc: annouce deprecation of jumbo > >>> frame flag condition > >>> > >>>> On 11/2/2020 1:18 PM, Andrew Rybchenko wrote: > >>>>> On 11/2/20 11:52 AM, SteveX Yang wrote: > >>>>>> Annouce to replace 'RTE_ETHER_MAX_LEN' with 'RTE_ETHER_MTU' as type > >>>>>> condition of jumbo frame. Involved scopes: > >>>>>> - rte_ethdev; > >>>>>> - app, e.g.: test-pmd, test-eventdev; > >>>>>> - examples, e.g.: ipsec-secgw, l3fwd, vhost; > >>>>>> - net PMDs which support VLAN tag(s) within overhead, e.g.: i40e, > >>>>>> ixgbe; > >>>>>> > >>>>>> Signed-off-by: SteveX Yang > >>>>>> --- > >>>>>>    doc/guides/rel_notes/deprecation.rst | 12 ++++++++++++ > >>>>>>    1 file changed, 12 insertions(+) > >>>>>> > >>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst > >>>>>> b/doc/guides/rel_notes/deprecation.rst > >>>>>> index 2e082499b..fae139f01 100644 > >>>>>> --- a/doc/guides/rel_notes/deprecation.rst > >>>>>> +++ b/doc/guides/rel_notes/deprecation.rst > >>>>>> @@ -138,6 +138,18 @@ Deprecation Notices > >>>>>>      will be limited to maximum 256 queues. > >>>>>>      Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed. > >>>>>> +* ethdev: Offload flag ``DEV_RX_OFFLOAD_JUMBO_FRAME`` will be set > >>>>>> +according to > >>>>>> +  ``RTE_ETHER_MTU`` in next release. Currently, the jumbo frame > >>>>>> +uses the > >>>>>> +  ``RTE_ETHER_MAX_LEN`` as boundary condition. When the MTU (1500) > >>>>>> +set, the > >>>>>> +  frame type of rx packet will be different if used different > >>>>>> +overhead, it will > >>>>>> +  cause the consistency issue. Hence, using fixed value > >>>>>> +``RTE_ETHER_MTU`` can > >>>>>> +  avoid this issue. > >>>>>> +  Following scopes will be changed: > >>>>>> +  - ``rte_ethdev`` > >>>>>> +  - ``app``, e.g.: ``test-pmd``, ``test-eventdev``; > >>>>>> +  - ``examples``, e.g.: ``ipsec-secgw``, ``l3fwd``, ``vhost``; > >>>>>> +  - net PMDs which support VLAN tag(s) within overhead, e.g.: > >>>>>> +``i40e``; > >>>>>> + > >>>>>>    * cryptodev: support for using IV with all sizes is added, J0 > >>>>>> still can > >>>>>>      be used but only when IV length in following structs > >>>>>> ``rte_crypto_auth_xform``, > >>>>>>      ``rte_crypto_aead_xform`` is set to zero. When IV length is > >>>>>> greater or equal > >>>>>> > >>>>> > >>>>> If so, what's the point to have the offload? May be just deprecate > >>>>> and later remove it? > >>>>> > >>> > >>> Same thought actually, can we remove DEV_RX_OFFLOAD_JUMBO_FRAME > >>> flag completely? > >>> PMD can decide based on provided MTU size does segmentation, etc. is > >>> needed. > >>> > >> > >> Yes, it seems can be removed when base on MTU size. > >> I've checked related code of using DEV_RX_OFFLOAD_JUMBO_FRAME. > >> Most of them use for checking boundary of max packet length and set > >> 'dev->data->mtu'. > >> > > > > Steve already sent the RFC for above fix: > > https://patches.dpdk.org/patch/84486/ > > > > We can consider removing 'DEV_RX_OFFLOAD_JUMBO_FRAME' but anyway it is > > for 21.11. > > > > This deprecation notice is required to fix the ethdev in next release, > > as in the above RFC. > > > > I cc'ed a few more relevant people, can you please review this > > deprecation notice. > > > > Thanks, > > ferruh > > > > > >>>> > >>>> Above just changes the condition of what is called jumbo frame, nothing more. > >>>> > >>>> ethdev assumes max frame size (without jumbo frame support) can be > >>>> 'RTE_ETHER_MAX_LEN' (1518) > >>>> > >>>> But a PMD can support double VLAN, and it can have RTE_ETHER_MAX_LEN + > >>>> 8 bytes frame size and still may not support jumbo frame. > >>>> > >>>> In that case ethdev overwrites the frame size as RTE_ETHER_MAX_LEN, > >>>> and this will reduce the supported MTU to 1492 (instead of expected > >>>> 1500). > >>>> Above deprecation notice is to fix this. > > My problem with the deprecation notice is that I don't actually > understand what will be done to address it. > > The idea explained by Ferruh in details makes sense. But I > don't understand how the deprecation notice description > corresponding to it. I read > "Offload flag ``DEV_RX_OFFLOAD_JUMBO_FRAME`` will be set.." > as an enforcement of the offload flag based on other > parameters. I think it is incorrect. Or I still don't > understand something... > > Looking at [1] adds more confusion since I don't understand why > we care about dev_conf->rxmode.max_rx_pkt_len when JUMBO_FRAME > offload is disabled. As far as I know JUMBO_FRAME offload > enable means that driver should take a look at it and apply. > Otherwise, just ignore it. > I agree with the comment here - my understanding is the same that if the JUMBO_FRAME offload flag is not set, then the max_rx_pkt_len should be ignored (which for me implies that it should be set to 0 or similar sentinal value in ethdev to ensure drivers don't accidentally use it). In terms of the deprecation notice, I also think it's fairly confusing, and after talking with Ferruh, I'm not convinced we need one. It seems that the planned changes based on this are just bug fixes, where packets that should not have been dropped were dropped. Perhaps someone could comment on the specific behaviour change that would affect apps (where it's not just plain buggy behaviour!) However, it does appear that this area is in need of clean-up generally. The suggestion to drop the jumbo frame flag, packet_len/mtu value from the ethdev config, and just use the existing API calls, sounds interesting. If that is not the approach taken, I'd like to see the existing approach kept, so that a zero-initialized config is acceptable for packet size setting, i.e. no jumbo frame flag and zero max-length == default ethernet MTU. Just my 2c. /Bruce