From mboxrd@z Thu Jan 1 00:00:00 1970 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) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 May 2018 08:05:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,415,1520924400"; d="scan'208";a="42044931" Received: from pgsmsx106.gar.corp.intel.com ([10.221.44.98]) by orsmga007.jf.intel.com with ESMTP; 18 May 2018 08:05:57 -0700 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.194]) by PGSMSX106.gar.corp.intel.com ([169.254.9.197]) with mapi id 14.03.0319.002; Fri, 18 May 2018 23:05:56 +0800 From: "Dai, Wei" To: "Zhang, Qi Z" , "Wu, Yanglong" , "dev@dpdk.org" Thread-Topic: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port Thread-Index: AQHT7no15G6H+aMKiEaVz3zyrhKo9qQ0lWuAgAC926D//5NigIAAlRig 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> In-Reply-To: <039ED4275CED7440929022BC67E70611531B488A@SHSMSX103.ccr.corp.intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmM0NWVkZDgtOTFiNy00ODY5LTg1MGEtYzU4NTJhYzZkMzI0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoib3NENms3YXhkSUxcL0g2Sm1aWTlZbUMzNm16M09uY1NrbDZtb1VQNzJBNFdRUWlMdXdRaTFtTFwvSFN6cStKXC9QdyJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [172.30.20.206] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port 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: Fri, 18 May 2018 15:06:00 -0000 > -----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()