From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 4B4D912008 for ; Fri, 18 May 2018 18:10:56 +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 orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 May 2018 09:10:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,415,1520924400"; d="scan'208";a="42059779" Received: from pgsmsx102.gar.corp.intel.com ([10.221.44.80]) by orsmga007.jf.intel.com with ESMTP; 18 May 2018 09:10:53 -0700 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.194]) by PGSMSX102.gar.corp.intel.com ([169.254.6.245]) with mapi id 14.03.0319.002; Sat, 19 May 2018 00:10:52 +0800 From: "Dai, Wei" To: "Dai, Wei" , "Zhang, Qi Z" , "Wu, Yanglong" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port Thread-Index: AQHT7rnQLIklRhOUcEuf/rXOXbVtD6Q1p6uw Date: Fri, 18 May 2018 16:10:51 +0000 Message-ID: <49759EB36A64CF4892C1AFEC9231E8D66CF8366A@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> <49759EB36A64CF4892C1AFEC9231E8D66CF8361B@PGSMSX111.gar.corp.intel.com> In-Reply-To: <49759EB36A64CF4892C1AFEC9231E8D66CF8361B@PGSMSX111.gar.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 16:10:56 -0000 As 18.05 release is coming soon. I'd like to submit http://dpdk.org/dev/patchwork/patch/40226/ in reply to yanglong's v2 patch for quick review and validation. Thanks for your understanding. > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dai, Wei > Sent: Friday, May 18, 2018 11:06 PM > To: Zhang, Qi Z ; Wu, Yanglong > ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail= for > per port >=20 > > -----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 > > port > > > > Hi Daiwei: > > > > > -----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 lev= el. > > > So the else branches still should be removed. > > > > Remove the else branch will not disable all queues if some queue is > > enabled 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. > > > > Thanks > > Qi >=20 > 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. >=20 > 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. > 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->dev_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) >=20 > As the VLAN_STRIP is per-queue capability, ixgbe_vlan_hw_strip_config() > only get it from rxq->offloads which hasn't been update in > rte_eth_dev_set_vlan_offload(port_id, vlan_offload); >=20 > And the ixgbe_vlan_offload_set() is also be called by ixgbe_dev_start() w= hich > is normal called after dev_configure() and queue_setup(). >=20 > These are 2 different path to config VLAN_STRIP. > So we can add a function ixgbe_vlan_offload_config() to let them go > different way as follows: >=20 > dev->dev_ops->vlan_offload_set(dev, mask) -> point to new function > dev->ixgbe_vlan_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() > } >=20 > Ixgbe_dev_start() > ixgbe_vlan_offload_set() >=20