From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dai, Wei" Subject: Re: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port Date: Fri, 18 May 2018 15:05:55 +0000 Message-ID: <49759EB36A64CF4892C1AFEC9231E8D66CF8361B@PGSMSX111.gar.corp.intel.com> References: <20180517055205.58766-1-yanglong.wu@intel.com> <20180518072341.27051-1-yanglong.wu@intel.com> <039ED4275CED7440929022BC67E70611531B46A1@SHSMSX103.ccr.corp.intel.com> <49759EB36A64CF4892C1AFEC9231E8D66CF833FC@PGSMSX111.gar.corp.intel.com> <039ED4275CED7440929022BC67E70611531B488A@SHSMSX103.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable To: "Zhang, Qi Z" , "Wu, Yanglong" , "dev@dpdk.org" Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 22AEFD08E for ; Fri, 18 May 2018 17:05:59 +0200 (CEST) In-Reply-To: <039ED4275CED7440929022BC67E70611531B488A@SHSMSX103.ccr.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Zhang, Qi Z > Sent: Friday, May 18, 2018 8:37 PM > To: Dai, Wei ; Wu, Yanglong > ; dev@dpdk.org > Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per po= rt >=20 > Hi Daiwei: >=20 > > -----Original Message----- > > From: Dai, Wei > > Sent: Friday, May 18, 2018 7:07 PM > > To: Zhang, Qi Z ; Wu, Yanglong > > ; dev@dpdk.org > > Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per > > port > > > > > -----Original Message----- > > > From: Zhang, Qi Z > > > Sent: Friday, May 18, 2018 3:46 PM > > > To: Wu, Yanglong ; dev@dpdk.org > > > Cc: Dai, Wei > > > Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for > > > per port > > > > > > > -----Original Message----- > > > > From: Wu, Yanglong > > > > Sent: Friday, May 18, 2018 3:24 PM > > > > To: dev@dpdk.org > > > > Cc: Zhang, Qi Z ; Dai, Wei > > > > ; Wu, Yanglong > > > > Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per > > > > port > > > > > > > > rxq->offload should synchronize with dev_conf > > > > > > > > Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue > > > > offloading in > > > > VF") > > > > Signed-off-by: Yanglong Wu > > > > > > Acked-by: Qi Zhang > > > > The release date is coming soon. > > Sorry, I have to NACK it. > > VLAN strip is per-queue feature, > > If it is disabled on port level, it still can be enabled on queue level= . > > So the else branches still should be removed. >=20 > Remove the else branch will not disable all queues if some queue is enabl= ed > at queue configure level, I think this is not user expected. > The purpose of i40e_vlan_offload_set here is to disable all queue's vlan = strip, > though vlan strip is per queue offload and some queue may be enabled at > queue configure level, I don't know why we can't disable them in this > function. >=20 > Thanks > Qi As VLAN_STRIP has already been advertised to per-queue offloading on ixgbe = 82599/X540/X550. The else branches will break this per-queue feature. The issues is from the testpmd command "vlan set strip on " Which is meant to enable/disable VLAN_STRIP on whole port on the fly.=20 The call stack is as follows: rx_vlan_strip_set(portid_t port_id, int on) rte_eth_dev_set_vlan_offload(port_id, vlan_offload); //modify dev->data->d= ev_conf.rxmode.offloads dev->dev_ops->vlan_offload_set(dev, mask) ixgbe_vlan_offload_set(dev, mask) ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev) ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue= , bool on) As the VLAN_STRIP is per-queue capability, ixgbe_vlan_hw_strip_config() onl= y get it from rxq->offloads which hasn't been update in rte_eth_dev_set_vla= n_offload(port_id, vlan_offload); And the ixgbe_vlan_offload_set() is also be called by ixgbe_dev_start() whi= ch is normal called after dev_configure() and queue_setup(). These are 2 different path to config VLAN_STRIP. So we can add a function ixgbe_vlan_offload_config() to let them go differe= nt way as follows: dev->dev_ops->vlan_offload_set(dev, mask) -> point to new function ixgbe_vl= an_offload_config() ixgbe_vlan_offload_config(dev, mask) { if vlan_strip is configured on whole port { update dev->data->dev_conf.rxmode.offloads update rxq->offloads on all queues } Ixgbe_vlan_offload_set() } Ixgbe_dev_start() ixgbe_vlan_offload_set()