From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 949931B81C for ; Tue, 24 Oct 2017 18:09:34 +0200 (CEST) Received: from lfbn-1-6068-189.w90-110.abo.wanadoo.fr ([90.110.3.189] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1e71rR-0002G1-JB; Tue, 24 Oct 2017 18:15:26 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Tue, 24 Oct 2017 18:09:23 +0200 Date: Tue, 24 Oct 2017 18:09:23 +0200 From: Olivier MATZ To: Ferruh Yigit Cc: dev@dpdk.org, thomas@monjalon.net Message-ID: <20171024160922.fu7qiciondyikgvb@platinum> References: <20171023121623.27466-1-olivier.matz@6wind.com> <4171e3c8-31eb-4c40-117b-87ac6a585764@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4171e3c8-31eb-4c40-117b-87ac6a585764@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] mbuf: rename deprecated VLAN flags 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: Tue, 24 Oct 2017 16:09:34 -0000 Hi Ferruh, On Mon, Oct 23, 2017 at 07:08:25PM -0700, Ferruh Yigit wrote: > On 10/23/2017 5:16 AM, Olivier Matz wrote: > > PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated for a while. > > As explained in [1], these flags were kept to let the applications and > > PMDs move to the new flag. There is also a need to support Rx vlan > > offload without vlan strip (at least for the ixgbe driver). > > > > This patch renames the old flags for this feature, knowing that some > > PMDs were using PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT to indicate that > > the vlan tci has been saved in the mbuf structure. > > So this patch merges two steps, > - remove old flags > - Add new flag to separate stripped and saved vlan info. > > Overall looks like a good idea. > > Just to confirm, in old usage, "PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED", > both were meaning same thing, right. And "PKT_RX_VLAN_PKT" kept for backward > compatibility. > So previous "PKT_RX_VLAN_PKT" was way to say "stripped and saved", converting it > to "PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED" as done in this patch should be safe. > Only may be missing cases that only "PKT_RX_VLAN" is valid. Before this patch: - PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED: stripped and saved - PKT_RX_VLAN_STRIPPED: unused, but also means stripped and saved - PKT_RX_VLAN_PKT: undefined, but present in several drivers. In some of them, I think it means 'not stripped and saved'. After this patch: - PKT_RX_VLAN: not stripped and saved - PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED: stripped and saved - PKT_RX_VLAN_STRIPPED: same (stripped and saved), PKT_RX_VLAN is implied I think we can forbid to have this flag alone. > And is "PKT_RX_VLAN_STRIPPED" implies the "PKT_RX_VLAN"? This was mentioned is > the initial proposal, but it looks like that is not the case in this patch. Yes, I can clarify it in the documentation. > [...] > > This patch has no impact on applications that do not > > use the deprecated flags. > > I guess this is only true if "PKT_RX_VLAN_STRIPPED" implies the "PKT_RX_VLAN" >>From what I see, currently PKT_RX_VLAN_STRIPPED is always used with PKT_RX_VLAN_PKT in PMDs, so I think there will be not impact. > > For other applications, renaming > > the flag would restore the same behavior. > > Not sure, for an old application, using "PKT_RX_VLAN_PKT", can't just rename to > "PKT_RX_VLAN" and use as it is because now it doesn't say if vlan stripped or > not. So old applications seems may be effected, if so this may require to be > notified in advance. Old applications using PKT_RX_VLAN_PKT have been notified since 16.11 that the flag is deprecated. If the application is relying on the undocumented fact that PKT_RX_VLAN_PKT means "not stripped and saved" on some PMDs, it will continue to work as is after renaming. > > But, since we are > > close to the release, applying it early after the release could > > also be considered. > > Is there any benefit to be in this release, one think I can think of is 17.11 > being LTS, is there any other? > > And what do you think doing in two steps, > - in this release remove deprecated flags > - in the beginning of the next release introduce the new flags I think one good side effect of having only one patch is that it doesn't break applications that were using an undocumented behavior of a PMD (it just requires a rename). Olivier