From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id BD62CAAC8 for ; Tue, 17 Apr 2018 00:13:48 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 27CC221193; Mon, 16 Apr 2018 18:13:48 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 16 Apr 2018 18:13:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=IYk+jC6oLU6FjZyzZqyL9lB6Hd 32/cN7fYtg1xBcGC0=; b=ATenxd/luzFyH0mm1hsSF81nY8vaMQKJkMX1pVsp25 LRGJ7tDKJKPYg3upW0Z76ixlYkhhkk13YfwYMUCImuoP/R07l+w3UAsUzrJsG1c6 /H7QK60Oa6ktoNVt95DVQp+hzH9BGLo2PxY+EH56fwq0cgjF5RGENzLH23S7kfer o= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=IYk+jC 6oLU6FjZyzZqyL9lB6Hd32/cN7fYtg1xBcGC0=; b=a3+GM3t9/6l9eebVxxnFnp Hi+8sls5JbvT165HLrsF2L4IapmW1keR0z59pOjrqW9vj3Pqr/wtjLsMuVmEBsZc bHWUTYONeukttQE/IujINz+jCCwVd9uvk8QrPhaMs9afMNkwTtP40c4ErZSCn+xt f+Fsql49F6VWdg7jSgwX2QlBbO8M5pgTbTvkOeZBrHnzsNAkBdcPC4QyLtT95jsP a0nitkQAnW8M6Wb8igXwTxbJqZaPBzBOxC/u/kl/FodPAqQRosTAGaWZIl456SMQ zS4cGOlYpShSP3Shr3lh6bXqGlA6123ug9OOckmUMevJnzTbRnIhHxNjVkXVZ+UA == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 1D46CE4394; Mon, 16 Apr 2018 18:13:47 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit , Shahaf Shuler Cc: dev@dpdk.org, Neil Horman , John McNamara , Marko Kovacevic Date: Tue, 17 Apr 2018 00:13:45 +0200 Message-ID: <5080470.gr8dEl1KFW@xps> In-Reply-To: <5271b825-cfcc-e098-d34a-dd92f79d3a47@intel.com> References: <20180320112631.107105-1-ferruh.yigit@intel.com> <5271b825-cfcc-e098-d34a-dd92f79d3a47@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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: Mon, 16 Apr 2018 22:13:49 -0000 16/04/2018 19:23, Ferruh Yigit: > On 4/1/2018 8:10 AM, Shahaf Shuler wrote: > > Thursday, March 29, 2018 4:32 PM, Ferruh Yigit: > >> 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. > >>> > >>> 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" > > > > I think the above line ... > > > >> > >> v18.11: > >> - Remove the CRC_STRIP flag completely > > > > Should be here. > > > > It is better to change the default behavior only once the STRIP_CRC flags is removed. > > Without it on v18.08 you break application that wants to have the CRC, and on v18.11 you break the application which actually used it. > > It is better to have all the application changes in one release - 18.11. > > > >> > >> I think this is more proper but takes more time. > > > > Me too. > > > >> > >> > >> 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. > > > > but what about those which do support? > > You break application which uses PMDs which support this offload for the sake of the PMD which don't have this capability. > > > > I think #1 is the clean one. > > > Any decision here? So will we go with first version of this patch [1]? > > [1] > https://dpdk.org/dev/patchwork/patch/36288/ Please do a v3 according to what Shahaf is proposing: - add KEEP_CRC in 18.08 + implement in PMDs + translate ~STRIP_CRC - remove STRIP_CRC in 18.11 + toggle default to stripping Or, are we able to do it quickly in 18.05-rc1, and remove in 18.08 with other offloads?