From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH v2] i40e: modify the meaning of single VLAN type Date: Tue, 21 Jun 2016 12:28:11 +0100 Message-ID: <20160621112811.GA19572@bricha3-MOBL3> References: <1464247695-4694-1-git-send-email-beilei.xing@intel.com> <1465805012-2907-1-git-send-email-beilei.xing@intel.com> <20160621102930.GA21016@bricha3-MOBL3> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Beilei Xing , jingjing.wu@intel.com, dev@dpdk.org, thomas.monjalon@6wind.com, nhorman@tuxdriver.com To: Panu Matilainen Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 81734B418 for ; Tue, 21 Jun 2016 13:28:15 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jun 21, 2016 at 02:06:38PM +0300, Panu Matilainen wrote: > On 06/21/2016 01:29 PM, Bruce Richardson wrote: > >On Mon, Jun 13, 2016 at 04:03:32PM +0800, Beilei Xing wrote: > >>In current i40e codebase, if single VLAN header is added in a packet, > >>it's treated as inner VLAN. Generally, a single VLAN header is > >>treated as the outer VLAN header. So change corresponding register > >>for single VLAN. > >>At the meanwhile, change the meanings of inner VLAN and outer VLAN. > >> > >>Signed-off-by: Beilei Xing > > > >This patch changes the ABI, since an app written to the original API as specified > >e.g. to set a single vlan header, would no longer work with this change. > >Therefore, even though the original behaviour was inconsistent with other drivers > >it may still need to be preserved. > > > >I'm thinking that we may need to provide appropriately versioned copies of the > >vlan_offload_set and vlan_tpid_set functions for backward compatibility with > >the old ABI. > > > >Any other comments or thoughts on this? > >Neil, Thomas, Panu - is this fix something that we need to provide backward > >version-compatibility for, or given that the functions are being called through > >a generic ethdev API mean that this can just go in as a straight bug-fix? > > Since it's currently inconsistent with everything else, I'd just call it a > bug-fix and leave it at that. > Yep, makes sense. > Besides, I dont think you could version it via the ordinary means even if > you wanted to, due to the way its called through eth_dev_ops etc. > Good point, never thought of that! :-( > - Panu - Thanks for the guidance. /Bruce