From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id BAE3637A6 for ; Wed, 10 Feb 2016 16:53:55 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP; 10 Feb 2016 07:53:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,426,1449561600"; d="scan'208";a="45648401" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.159]) by fmsmga004.fm.intel.com with SMTP; 10 Feb 2016 07:53:52 -0800 Received: by (sSMTP sendmail emulation); Wed, 10 Feb 2016 15:53:51 +0025 Date: Wed, 10 Feb 2016 15:53:51 +0000 From: Bruce Richardson To: Tom Kiely Message-ID: <20160210155351.GC10620@bricha3-MOBL3> References: <1449853163-25673-1-git-send-email-stephen@networkplumber.org> <2601191342CEEE43887BDE71AB97725836AD3A0B@irsmsx105.ger.corp.intel.com> <20151214112516.35bbc1f8@xeon-e3> <2601191342CEEE43887BDE71AB97725836AD3A5C@irsmsx105.ger.corp.intel.com> <20151214133512.48593b49@xeon-e3> <2601191342CEEE43887BDE71AB97725836AD3DA2@irsmsx105.ger.corp.intel.com> <56728ED2.8010509@brocade.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56728ED2.8010509@brocade.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Feb 2016 15:53:56 -0000 On Thu, Dec 17, 2015 at 10:30:42AM +0000, Tom Kiely wrote: > Sorry for the delay in replying to this thread. I was on vacation for the > last 3 days. Please see inline for my comments. > > On 12/15/2015 02:37 PM, Ananyev, Konstantin wrote: > > > >>-----Original Message----- > >>From: Stephen Hemminger [mailto:stephen@networkplumber.org] > >>Sent: Monday, December 14, 2015 9:35 PM > >>To: Ananyev, Konstantin > >>Cc: Zhang, Helin; dev@dpdk.org; Tom Kiely > >>Subject: Re: [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers. > >> > >>On Mon, 14 Dec 2015 19:57:10 +0000 > >>"Ananyev, Konstantin" wrote: > >> > >>> > >>>>-----Original Message----- > >>>>From: Stephen Hemminger [mailto:stephen@networkplumber.org] > >>>>Sent: Monday, December 14, 2015 7:25 PM > >>>>To: Ananyev, Konstantin > >>>>Cc: Zhang, Helin; dev@dpdk.org; Tom Kiely > >>>>Subject: Re: [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers. > >>>> > >>>>On Mon, 14 Dec 2015 19:12:26 +0000 > >>>>"Ananyev, Konstantin" wrote: > >>>> > >>>>> > >>>>>>From: Stephen Hemminger [mailto:stephen@networkplumber.org] > >>>>>>Sent: Friday, December 11, 2015 4:59 PM > >>>>>>To: Zhang, Helin; Ananyev, Konstantin > >>>>>>Cc: dev@dpdk.org; Tom Kiely; Stephen Hemminger > >>>>>>Subject: [PATCH] ixgbe: Discard SRIOV transparent vlan packet headers. > >>>>>> > >>>>>>From: Tom Kiely > >>>>>> > >>>>>>SRIOV VFs support "transparent" vlans. Traffic from/to a VM > >>>>>>associated with a VF is tagged/untagged with the specified > >>>>>>vlan in a manner intended to be totally transparent to the VM. > >>>>>> > >>>>>>The vlan is specified by "ip link set vf vlan ". > >>>>>>The VM is not configured for any vlan on the VF and the VM > >>>>>>should never see these transparent vlan headers for that reason. > >>>>>> > >>>>>>However, in practice these vlan headers are being received by > >>>>>>the VM which discards the packets as that vlan is unknown to it. > >>>>>>The Linux kernel explicitly discards such vlan headers but DPDK > >>>>>>does not. > >>>>>>This patch mirrors the kernel behaviour for SRIOV VFs only > >>>>> > >>>>>I have few concerns about that approach: > >>>>> > >>>>>1. I don't think vlan_tci info should *always* be stripped by vf RX routine. > >>>>>There could be configurations when that information might be needed by upper layer. > >>>>>Let say VF can be member of 2 or more VLANs and upper layer would like to have that information > >>>>>for further processing. > >>>>>Or special mirror VF, that does traffic snnoping, or something else. > >>>>>2. Proposed implementation would introduce a slowdown for all VF RX routines. > >>>>>3. From the description it seems like the aim is to clear VLAN information for the RX packet. > >>>>>Though the patch actually clears VLAN info only for the RX packet whose VLAN tag is not present inside SW copy of VFTA table. > >>>>>Which makes no much point to me: > >>>>>If VLAN is not present in HW VFTA table, then packet with that VLAN tag will be discarded by HW anyway. > >>>>>If it is present inside VFTA table (both SW & HW), then VLAN information would be preserved with and without the patch. > >>>>> > >>>>>If you need to clear VLAN information, why not to do it on the upper layer - inside your application itself? > >>>>>Either create some sort of wrapper around rx_burst(), or setup an RX call-back for your VF device. > >>>>> > >>>>>Konstantin > >>>> > >>>>The aim is to get SRIOV to work when the transparent VLAN tag feature is used. > >>>>Please talk to the Linux driver team. Similar code exists there in ixgbevf_process_skb_fields. > >>> > >>>Ah ok, I realised what you are trying to achieve now: > >>>You setup HW VFTA[] from the PF, so from VF point of view SW copy of the VFTA[] remains unset. > >>>So HW will pass VLAN packet in, but then SW will clear VLAN tag. > >>>Ok, that clears #3 above, but I think #1,2 still remain. > >>On the host, what configured is a vlan tag per VF per guest > >> > >>Tom had more info in the original mail. > >> > >>http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/28932 > >> > >>>>The other option is have a copy of all the receive logic which is only > >>>>used by VF code. > >>>Why that's the only option? > >>>Why can't you clear that VLAN information above the PMD layer? > >>>Keep/obtain a copy of VFTA[] somewhere on the upper layer, > >>>and do actual clear after rx_burst() returns? > >>>Konstantin > >>The problem is that the guest is supposed to not see the VLAN tags (it has no reason to), > >>but the hardware leaves a VLAN tag on there. > >Yes, I understand what you are trying to achieve. > > What I am trying to say: > >1. VLAN tag removing shouldn't be forced for all VFs. > >I think there are scenarios where existing behaviour (keeping vlan_tci and ol_flags intact) are what people need. > >One example would be mirror VF doing other VFs traffic snooping. > >Probably some other cases too. > >2. The way you implemented it - it might cause a RX performance degradation (specially for VF). > >That's why I think it better to be implemented on top of PMD: > >i.e: some sort of wrapper that checks all packets returned by rx_burst() and clears vlan_tci if needed. > >That would give you desired behaviour and keep current implementation intact. > > > >Konstantin > > > > > >Hi Konstantin, > To address your comments: > > (1) Only tags corresponding to VLANs that the client knows nothing about are > stripped. These tags are not intended to be seen by the client. > Maybe your concern would be addressed by disabling this functionality when > snooping in the same way that vlan offloading is disabled ? > I think further analysis is required here on our part. > (2) In relation to performance, for the non-SRIOV case, the hit is one "if" > per packet to test whether the functionality is enabled or not. We saw no > significant performance impact for the SRIOV case. > Moving the functionality above PMD is certainly something that we can > examine. > > Thanks, > Hi Tom, Stephen, Konstantin, have we reached a consensus on this patch? From what Tom says above on point 1, it seems to me like this some re-evaluation is going to be done and a patch for this issue will be sent again at a later date. Is that correct? Regards, /Bruce