From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 4685B2BC7 for ; Thu, 29 Mar 2018 15:32:19 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Mar 2018 06:32:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,376,1517904000"; d="scan'208";a="32464739" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.63]) ([10.237.221.63]) by fmsmga002.fm.intel.com with ESMTP; 29 Mar 2018 06:32:16 -0700 To: Shahaf Shuler , Thomas Monjalon Cc: Neil Horman , John McNamara , Marko Kovacevic , "dev@dpdk.org" References: <20180320112631.107105-1-ferruh.yigit@intel.com> <20180321194730.52068-2-ferruh.yigit@intel.com> <15555561.lU3UqzoiBB@xps> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR1xgUJB33UawAKCRD5M+tD3xNhH3TID/wNjq1doFXg49WaM7ZXez/1 qwj7U6FQ2eUhlEPX8dXc52cu+iQk8/mssXQtANEx+jndxfvlkikBNgI+mr3m/ho54uQWFZd7 Bv0DVVkLHNkLWK6tT6iISdCgXrQuOv8y5rucEGkJ8dDPsp8Mqr4sBjavRBpczphoa3miKusA HzUEo0SYuHUPmBlbmvKv4PkA5quWtXpkgV2HZ1bW9X0dQkAqEVioAPm5Q6pXJODfV8kaZUtJ z4swEtBnK9XAWm2HccF7KmFh0vv+Zh9lEMnEFt7UPXngY6+xCTo6xV1IVc8EZfDjOip8I4h2 ALMaLgrZwA9VVmHCVOrnO3UZqfGehrwz4O3sUrcmIzxjk3gfBmVRCBfuWGchSpWtZ99U6V1a OEuG+ymyjBDtyymW+KSvmCBl2gIxZHvZFvSRRdDObkkUIskSJ95f/6HBOESRZBOiV9GEAhC5 gI9OAKKF8HQaN/r5KJrkzscjQcjIdV1jXhTkfQ4wH3GJiVM5JxsKsEjjRw7yfSJo2GBEGqMe KMwMLWg4f1DgVtjOuGf10isu+29MvapW2IxKYaHcVc2vHfWbDi2AvBj/VAzKILWbTEgI1VL5 zKpo5p6X2O55oEyeflDiAzrUfvLqB4vmTyqXtW6PdLyZC7kXIzmNu6EBVx9oSgy3CADw5saN 0La9OoCAc7Tn+s7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKA== Message-ID: <315855d9-2a6a-667a-4032-62c947323b02@intel.com> Date: Thu, 29 Mar 2018 14:32:15 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 2/2] ethdev: add new offload flag to keep CRC 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: , X-List-Received-Date: Thu, 29 Mar 2018 13:32:20 -0000 On 3/29/2018 8:56 AM, Shahaf Shuler wrote: > Thursday, March 29, 2018 10:43 AM, Thomas Monjalon: >> 29/03/2018 07:38, Shahaf Shuler: >>> Wednesday, March 21, 2018 9:48 PM, Ferruh Yigit: >>>> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. >>>> >>>> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release but >>>> default behavior in PMDs is to strip the CRC independent from this flag. >>>> >>>> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed: >>>> - Setting both KEEP_CRC & CRC_STRIP is INVALID >>>> - Setting only CRC_STRIP PMD should strip the CRC >>>> - Setting only KEEP_CRC PMD should keep the CRC >>>> - Not setting both PMD should strip the CRC >>> >>> We cannot have such default behavior, it may break existing applications. >>> >>> The API of ethdev offloads says (even though it is not well documented) : >> DEV_RX_OFFLOAD_CRC_STRIP (emphasis the STRIP). >>> meaning, if set -> STRIP, if not set -> don't strip. I am aware to at least one >> application which wants to have the CRC, and for that purpose it naturally >> don't set the offload flag. >>> >>> The fact some PMDs lack the ability to toggle the CRC stripping should be >> exposed in the "limitations" section of their related guide. >>> >>> Up to here, this is the behavior as defined by the API. >>> >>> Now, we want to change it, and I think it makes sense. However I think we >> should take similar approach to what we did with the ethdev offloads API: >>> 1. at first stage a new offload flag is exposed DEV_RX_OFFLOAD_KEEP_CRC >> and implemented on the PMDs. >> >> This is what is proposed above, except that we change the default >> behaviour. >> If we introduce a new flag which is the contrary of the old one, we need to >> choose which one is the default, so which one has no effect. > > The default behavior should remain as long as we don't deprecate the DEV_RX_OFFLOAD_CRC_STRIP flag. > > Logic should be : > Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed: > - Setting both KEEP_CRC & CRC_STRIP is INVALID - enforced by ethdev. > - Setting only CRC_STRIP PMD should strip the CRC > - Setting only KEEP_CRC PMD should keep the CRC > - Not setting both PMD should *not* strip the CRC Hi Shahaf, I think we have two options, 1- This is v1 of this patch and also what you suggest: v18.05: - Send deprecation notice v18.08: - Add KEEP_CRC flag - Change the meaning not setting CRC_STRIP to "strip the CRC" v18.11: - Remove the CRC_STRIP flag completely I think this is more proper but takes more time. 2- Based on all the reality that all devices are doing CRC strip already: v18.05: - Add KEEP_CRC flag - Change the meaning not setting CRC_STRIP to "strip the CRC" v18.08: - Remove the CRC_STRIP flag completely With option two since not setting both KEEP_CRC & CRC_STRIP will mean "strip the CRC", it won't require a change in PMDs, only we can pay attention to get new PMDs according plan. This can be more problematic for the case that application ask for keeping CRC, because the way to say this was not setting CRC_STRIP, it changed to setting KEEP_CRC without notification. This can be less problem since many PMD already doesn't support keep crc. Are you aware of any use case that is broken with option 2? > >> >>> 2. there is a conversion function on ethdev. Which for example converts >> ~DEV_RX_OFFLOAD_CRC_STRIP -> DEV_RX_OFFLOAD_KEEP_CRC for the >> PMDs. >>> 3. deprecation of DEV_RX_OFFLOAD_CRC_STRIP for applications and >> remove of the conversion functions. >> >> >> >