From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id B5AE8A046B for ; Mon, 24 Jun 2019 15:33:20 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F35514CC5; Mon, 24 Jun 2019 15:33:18 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id DBDBE378E for ; Mon, 24 Jun 2019 15:33:17 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us5.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 032104C0067; Mon, 24 Jun 2019 13:33:13 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 24 Jun 2019 14:33:06 +0100 To: "Ananyev, Konstantin" , Ivan Malov , Olivier Matz CC: "dev@dpdk.org" References: <20190412150542.12026-1-ivan.malov@oktetlabs.ru> <20190621103419.11626-1-ivan.malov@oktetlabs.ru> <2601191342CEEE43887BDE71AB97725801689E31B2@IRSMSX104.ger.corp.intel.com> <4462101f-5250-12f8-db88-0c3af619017a@solarflare.com> <2601191342CEEE43887BDE71AB97725801689E3F8B@IRSMSX104.ger.corp.intel.com> From: Andrew Rybchenko Message-ID: Date: Mon, 24 Jun 2019 16:33:03 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.1 MIME-Version: 1.0 In-Reply-To: <2601191342CEEE43887BDE71AB97725801689E3F8B@IRSMSX104.ger.corp.intel.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24708.003 X-TM-AS-Result: No-22.218100-8.000000-10 X-TMASE-MatchedRID: 9vvqFUF7IWnmLzc6AOD8DfHkpkyUphL9MHi1Ydy2WEj6eV5+LAaaX3ZQ rrFTqfg/Z+g/9suVejVqnCONYgFlZ9tRKUVZdq2UhUy0TABax1xp4xorO9dSmWz3xnx8b/qRKYK OOxNxwUXlOyb8gIQzWGNQNBwI/n1bjHHp3pfy0nqiAZ3zAhQYgg51SAUUhKsU33Nl3elSfsrt8r FozbGqQPFy+KnMGfULipEV96MQGTdtdBnQBR4+Bl9hr1b06OK1K/YFZTiDf+oELMPQNzyJS/trU qc5Y/E8i+92hb0TzFucbe1vlefuCBESp0PoosxqGUlF/M3Dxp9ezmeoa8MJ82mycYYiBYyZ2Qfv bWTVtgMjk5nJ43ka/fcziLR7wxmATBZpkDublHfBLypRtAo4yOjYK7HrnxE3wubD3SFbWztGIzS vkOhmQFf4DQPnm1aEV3dxVfkt91XyUmwGNE3Swsb1bQPrwFnMHS44sLaHAPLg91xayX4L8wF7bi wY8YH/pEYBIO560cqv8TAvJrvuV3Ho2p42VhyY5rnnDTmVbKog/CIfleX9D4qxMBrs575iLFlXZ d2HA7CdfbUqhtxNN/rap8Wg8afC0+Gv1tc3Op1mNCKOCsW/OivnqYJI3/w7f1gRz0Kj/4PdKUSB W7I322+5ieh24ZYRkZOl7WKIImrvXOvQVlExsEY41YX/o/8KdMrO7yc0I/yrEHfaj14ZyX08MjL XrejbsuLl1vGNTtHm3tWjXwzQZ+1VPC4tOnbF7oz8WBG9IxQ= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--22.218100-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24708.003 X-MDID: 1561383196-Jwo1Faa0GTp2 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] mbuf: outer offsets must be zero for non-tunnel packets 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 6/24/19 3:59 PM, Ananyev, Konstantin wrote: > Hi Andrew, > >> Hi Konstantin, >> >> On 6/21/19 2:10 PM, Ananyev, Konstantin wrote: >> Hi Ivan, >> >> Make sure that outer L2 and L3 header length fields are >> equal to zero for non-tunnel packets in order to ensure >> consistent and predictable behaviour in network drivers. >> Explain this expectation in comments to help developers. >> >> Signed-off-by: Ivan Malov >> Reviewed-by: Andrew Rybchenko >> --- >> >>> Not sure it is a good idea: >>> 1) it is a change in public API behavior (requirements). >> I would say that it is a clarification. Yes, in terms of rte_validate_tx_offload() >> behaviour is it is a change. The area looks grey and we just want to make >> it either black or white. What is the alternative? Say that outer_l2_len and >> outer_l3_len content is undefined if packet is not tunnelled and drivers >> must check (ol_flags & PKT_TX_TUNNEL_MASK) != 0 before usage these fields? > Yes, that was my thought. > As I understand, that what is implied right now. > Otherwise any app that setups tx_offload fieds for rte_eth_tx_burst() > need to be changed? > >> bnxt, fm10k, i40e, ixgbe (depends on PKT_TX_OUTER_IP_CKSUM in fact, but >> not PKT_TX_TUNNEL_MASK) and ice use these fields w/o tunnel checks (if >> I read code correctly). >> >> enic, mlx4, mlx5, qede and sfc use them in the case of tunnel packet only. >> >> I.e. 5 vs 5. >> >> >>> 2) why these 2 particular tx_offload fields only? >>> If we'll follow that logic we should enforce same rule for other >>> tx_offload fileds (tso, l4_len, l3_len, etc.) >> Because it is about tunnel packets and outer_l2_len and outer_l3_len >> should be either undefined or 0 for non-tunnel packets. > I understand that, but I think rules for setting/treating tx_offload fields > should be the same for all fields. > We either allow any tx_offload field to be undefined when related > bit(s) in ol_flags are not set, or we need to force people to setup > whole 64-bit tx_offload value if any of related TX flags are set. Yes, I agree. >>> Personally I don't think there will be much gain from it. >>> Might be better and easier just to fix offending drivers that make wrong assumptions. >> We would prefer to define as the patch suggests since it allows >> to avoid conditions. > It does, and it might simplify things for PMDs... > But as I said above, it would need changes in the apps that > do use tx_offload fileds for TX, right? Yes, it sounds bad. OK, we have discussed it and Ivan will send v2 which simply clarify comments. It looks like there is no simple conditions when we should require (in validate function) non-zero outer_l2/l3_len. Thanks a lot, Andrew.