[dpdk-dev] [PATCH] net/ixgbe: fix VLAN strip setting fail for per port

[dpdk-dev] [PATCH] net/ixgbe: fix VLAN strip setting fail for per port

[Yanglong Wu]

VLAN-strip should be set by both per port and per queue

Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in VF")
Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index f5006bc94..f27e777fe 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5228,9 +5228,11 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 
 	/* VF function only support hw strip feature, others are not support */
 	if (mask & ETH_VLAN_STRIP_MASK) {
+		on = !!(dev->data->dev_conf.rxmode.offloads &
+				DEV_RX_OFFLOAD_VLAN_STRIP);
 		for (i = 0; i < dev->data->nb_rx_queues; i++) {
 			rxq = dev->data->rx_queues[i];
-			on = !!(rxq->offloads &	DEV_RX_OFFLOAD_VLAN_STRIP);
+			on |= !!(rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 			ixgbevf_vlan_strip_queue_set(dev, i, on);
 		}
 	}
-- 
2.11.0

Re: [dpdk-dev] [PATCH] net/ixgbe: fix VLAN strip setting fail for per port

[Zhang, Qi Z]

Hi Yanglong:

> -----Original Message-----
> From: Wu, Yanglong
> Sent: Thursday, May 17, 2018 1:52 PM
> To: dev@dpdk.org
> Cc: Dai, Wei <wei.dai@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Yanglong <yanglong.wu@intel.com>
> Subject: [PATCH] net/ixgbe: fix VLAN strip setting fail for per port
> 
> VLAN-strip should be set by both per port and per queue
> 
> Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in
> VF")
> Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index f5006bc94..f27e777fe 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -5228,9 +5228,11 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev,
> int mask)
> 
>  	/* VF function only support hw strip feature, others are not support */
>  	if (mask & ETH_VLAN_STRIP_MASK) {
> +		on = !!(dev->data->dev_conf.rxmode.offloads &
> +				DEV_RX_OFFLOAD_VLAN_STRIP);

In ixgbe_rx_qeueu_setup, I saw rxq->offload is assigned by rx_conf->offloads | dev->data->dev_conf.rxmode.offloads;
So queue offload it already cover port offload, looks redundant here , could give more explain why this is necessary?

Regards
Qi

>  		for (i = 0; i < dev->data->nb_rx_queues; i++) {
>  			rxq = dev->data->rx_queues[i];
> -			on = !!(rxq->offloads &	DEV_RX_OFFLOAD_VLAN_STRIP);
> +			on |= !!(rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);


>  			ixgbevf_vlan_strip_queue_set(dev, i, on);
>  		}
>  	}
> --
> 2.11.0

[dpdk-dev] [DPDK] net/ixgbe: fix VLAN strip setting fail for per port

[Yanglong Wu]

rxq->offload should synchronize with dev_conf

Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in VF")
Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
---
V2:
rework as comments
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index f5006bc94..94d28878a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2114,6 +2114,14 @@ ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_rx_queues; i++) {
 			rxq = dev->data->rx_queues[i];
 			ctrl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
+
+			/* rxq->offload should synchronize with dev_conf*/
+			if (dev->data->dev_conf.rxmode.offloads &
+					DEV_RX_OFFLOAD_VLAN_STRIP)
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			else
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+
 			if (rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) {
 				ctrl |= IXGBE_RXDCTL_VME;
 				on = TRUE;
@@ -5230,6 +5238,14 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		for (i = 0; i < dev->data->nb_rx_queues; i++) {
 			rxq = dev->data->rx_queues[i];
+
+			/* rxq->offload should synchronize with dev_conf*/
+			if (dev->data->dev_conf.rxmode.offloads &
+						DEV_RX_OFFLOAD_VLAN_STRIP)
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			else
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+
 			on = !!(rxq->offloads &	DEV_RX_OFFLOAD_VLAN_STRIP);
 			ixgbevf_vlan_strip_queue_set(dev, i, on);
 		}
-- 
2.11.0

[dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Yanglong Wu]

rxq->offload should synchronize with dev_conf

Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in VF")
Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
---
v2:
rework as comments asked
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index f5006bc94..94d28878a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2114,6 +2114,14 @@ ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev)
 		for (i = 0; i < dev->data->nb_rx_queues; i++) {
 			rxq = dev->data->rx_queues[i];
 			ctrl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
+
+			/* rxq->offload should synchronize with dev_conf*/
+			if (dev->data->dev_conf.rxmode.offloads &
+					DEV_RX_OFFLOAD_VLAN_STRIP)
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			else
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+
 			if (rxq->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) {
 				ctrl |= IXGBE_RXDCTL_VME;
 				on = TRUE;
@@ -5230,6 +5238,14 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		for (i = 0; i < dev->data->nb_rx_queues; i++) {
 			rxq = dev->data->rx_queues[i];
+
+			/* rxq->offload should synchronize with dev_conf*/
+			if (dev->data->dev_conf.rxmode.offloads &
+						DEV_RX_OFFLOAD_VLAN_STRIP)
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			else
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+
 			on = !!(rxq->offloads &	DEV_RX_OFFLOAD_VLAN_STRIP);
 			ixgbevf_vlan_strip_queue_set(dev, i, on);
 		}
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Zhang, Qi Z]

> -----Original Message-----
> From: Wu, Yanglong
> Sent: Friday, May 18, 2018 3:24 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wu,
> Yanglong <yanglong.wu@intel.com>
> 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 <yanglong.wu@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Dai, Wei]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, May 18, 2018 3:46 PM
> To: Wu, Yanglong <yanglong.wu@intel.com>; dev@dpdk.org
> Cc: Dai, Wei <wei.dai@intel.com>
> 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 <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>;
> > Wu, Yanglong <yanglong.wu@intel.com>
> > 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 <yanglong.wu@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>

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.

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Zhang, Qi Z]

Hi Daiwei:

> -----Original Message-----
> From: Dai, Wei
> Sent: Friday, May 18, 2018 7:07 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Yanglong
> <yanglong.wu@intel.com>; 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 <yanglong.wu@intel.com>; dev@dpdk.org
> > Cc: Dai, Wei <wei.dai@intel.com>
> > 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 <qi.z.zhang@intel.com>; Dai, Wei
> > > <wei.dai@intel.com>; Wu, Yanglong <yanglong.wu@intel.com>
> > > 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 <yanglong.wu@intel.com>
> >
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> 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.

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

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Ferruh Yigit]

On 5/18/2018 1:36 PM, Zhang, Qi Z wrote:
> Hi Daiwei:
> 
>> -----Original Message-----
>> From: Dai, Wei
>> Sent: Friday, May 18, 2018 7:07 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Yanglong
>> <yanglong.wu@intel.com>; 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 <yanglong.wu@intel.com>; dev@dpdk.org
>>> Cc: Dai, Wei <wei.dai@intel.com>
>>> 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 <qi.z.zhang@intel.com>; Dai, Wei
>>>> <wei.dai@intel.com>; Wu, Yanglong <yanglong.wu@intel.com>
>>>> 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 <yanglong.wu@intel.com>
>>>
>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>
>> 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.
> 
> 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. 

rte_eth_dev_set_vlan_offload() API requests changes on VLAN related offloading
configuration, this API should cause:
1) Update configuration structure according API input
2) Apply new configuration to hardware

1) is done by API for port offloads.

2) is done by PMD "vlan_offload_set" dev_ops implementation.

But queue offload configuration struct is not updated anywhere, I guess this
patch is filling that gap. Otherwise wrong configuration will be applied to
hardware.


And there is still a defect with this patch, although I think it is minor, if
vlan is a queue offload and not enabled in configure but enabled for some set of
queues, when application asks to disable that offload, since rxmode.offloads
doesn't have it API will think it is already disable and don't send a request to
PMD, and this will keeps vlan offload enabled in some queues.
But comparing the what this patch fixes, I guess the remaining one is smaller
defect.


Perhaps it is better to update queue offload configuration too in API and PMD
only applies the configuration to hardware.

The rte_eth_dev_set_vlan_offload() API is not queue offload friendly, it may
disable/enable the offload for all queues if vlan offload is reported in queue
offload capability.

BUT, what do you think go with PMD update for this release, and if required put
a deprecation notice for the API and do the API change into next release?


> Thanks
> Qi
> 

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Dai, Wei]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, May 18, 2018 8:37 PM
> To: Dai, Wei <wei.dai@intel.com>; Wu, Yanglong
> <yanglong.wu@intel.com>; 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 <qi.z.zhang@intel.com>; Wu, Yanglong
> > <yanglong.wu@intel.com>; 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 <yanglong.wu@intel.com>; dev@dpdk.org
> > > Cc: Dai, Wei <wei.dai@intel.com>
> > > 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 <qi.z.zhang@intel.com>; Dai, Wei
> > > > <wei.dai@intel.com>; Wu, Yanglong <yanglong.wu@intel.com>
> > > > 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 <yanglong.wu@intel.com>
> > >
> > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> > 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.
> 
> 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

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 <port_id>"
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)

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);

And the ixgbe_vlan_offload_set() is also be called by ixgbe_dev_start() which 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 different way as follows:

dev->dev_ops->vlan_offload_set(dev, mask) -> point to new function 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()
		}

Ixgbe_dev_start()
	ixgbe_vlan_offload_set()

[dpdk-dev] [PATCH v3] net/ixgbe: config VLAN strip on the fly

[Wei Dai]

The old ixgbe_vlan_offload_set() is called by
rte_eth_dev_set_vlan_offload() which is meant to config VLAN
strip/filter/extend on all queues.
This old fucntion is also called by rte_eth_dev_start()/ixgbe_dev_start()
which need support per-queue VALN strip on only parts of queues.
So add new function ixgbe_vlan_offload_config() = old
ixgbe_vlan_offload_set().
New ixgbe_vlan_offload_set =  codes to align VLAN strip flags on all
queues with port level setting + ixgbe_vlan_offload_configure().

Signed-off-by: Wei Dai <wei.dai@intel.com>
Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 69 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7219f02..deb9baa 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -189,6 +189,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev,
 		uint16_t queue, bool on);
 static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue,
 		int on);
+static int ixgbe_vlan_offload_config(struct rte_eth_dev *dev, int mask);
 static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue);
 static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue);
@@ -246,6 +247,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev,
 		uint16_t queue, int on);
+static int ixgbevf_vlan_offload_config(struct rte_eth_dev *dev, int mask);
 static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on);
 static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
@@ -1974,10 +1976,13 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 
 	rxq = dev->data->rx_queues[queue];
 
-	if (on)
+	if (on) {
 		rxq->vlan_flags = PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED;
-	else
+		rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+	} else {
 		rxq->vlan_flags = PKT_RX_VLAN;
+		rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+	}
 }
 
 static void
@@ -2130,7 +2135,7 @@ ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev)
 }
 
 static int
-ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+ixgbe_vlan_offload_config(struct rte_eth_dev *dev, int mask)
 {
 	struct rte_eth_rxmode *rxmode;
 	rxmode = &dev->data->dev_conf.rxmode;
@@ -2156,6 +2161,32 @@ ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
+static int
+ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	uint16_t i;
+	struct rte_eth_rxmode *rxmode;
+	struct ixgbe_rx_queue *rxq;
+
+	if (mask & ETH_VLAN_STRIP_MASK) {
+		rxmode = &dev->data->dev_conf.rxmode;
+		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+		else
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+	}
+
+	ixgbe_vlan_offload_config(dev, mask);
+
+	return 0;
+}
+
 static void
 ixgbe_vmdq_vlan_hw_filter_enable(struct rte_eth_dev *dev)
 {
@@ -2577,7 +2608,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 
 	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
 		ETH_VLAN_EXTEND_MASK;
-	err = ixgbe_vlan_offload_set(dev, mask);
+	err = ixgbe_vlan_offload_config(dev, mask);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Unable to set VLAN offload");
 		goto error;
@@ -5022,7 +5053,7 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 	/* Set HW strip */
 	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
 		ETH_VLAN_EXTEND_MASK;
-	err = ixgbevf_vlan_offload_set(dev, mask);
+	err = ixgbevf_vlan_offload_config(dev, mask);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err);
 		ixgbe_dev_clear_queues(dev);
@@ -5220,7 +5251,7 @@ ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on)
 }
 
 static int
-ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+ixgbevf_vlan_offload_config(struct rte_eth_dev *dev, int mask)
 {
 	struct ixgbe_rx_queue *rxq;
 	uint16_t i;
@@ -5238,6 +5269,32 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
+static int
+ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	uint16_t i;
+	struct rte_eth_rxmode *rxmode;
+	struct ixgbe_rx_queue *rxq;
+
+	if (mask & ETH_VLAN_STRIP_MASK) {
+		rxmode = &dev->data->dev_conf.rxmode;
+		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+		else
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+
+		ixgbevf_vlan_offload_config(dev, mask);
+	}
+
+	return 0;
+}
+
 int
 ixgbe_vt_check(struct ixgbe_hw *hw)
 {
-- 
2.7.5

[dpdk-dev] [PATCH v4] net/ixgbe: config VLAN strip on the fly

[Wei Dai]

The old ixgbe_vlan_offload_set() is called by
rte_eth_dev_set_vlan_offload() which is meant to config VLAN
strip/filter/extend on all queues.
This old function is also called by rte_eth_dev_start()/ixgbe_dev_start()
which need support per-queue VALN strip on only parts of queues.
So add new function ixgbe_vlan_offload_config() = old
ixgbe_vlan_offload_set().
New ixgbe_vlan_offload_set =  codes to align VLAN strip flags on all
queues with port level setting + ixgbe_vlan_offload_configure().

Signed-off-by: Wei Dai <wei.dai@intel.com>
Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>

---
v4:
  fix typo error in git log message

v3:
  keep vlan strip as per-queue capability
  and support config vlan on port level on the fly

v2:
  rework as comments asked
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 69 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7219f02..deb9baa 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -189,6 +189,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev,
 		uint16_t queue, bool on);
 static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue,
 		int on);
+static int ixgbe_vlan_offload_config(struct rte_eth_dev *dev, int mask);
 static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue);
 static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue);
@@ -246,6 +247,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev,
 		uint16_t queue, int on);
+static int ixgbevf_vlan_offload_config(struct rte_eth_dev *dev, int mask);
 static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on);
 static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
@@ -1974,10 +1976,13 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 
 	rxq = dev->data->rx_queues[queue];
 
-	if (on)
+	if (on) {
 		rxq->vlan_flags = PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED;
-	else
+		rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+	} else {
 		rxq->vlan_flags = PKT_RX_VLAN;
+		rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+	}
 }
 
 static void
@@ -2130,7 +2135,7 @@ ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev)
 }
 
 static int
-ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+ixgbe_vlan_offload_config(struct rte_eth_dev *dev, int mask)
 {
 	struct rte_eth_rxmode *rxmode;
 	rxmode = &dev->data->dev_conf.rxmode;
@@ -2156,6 +2161,32 @@ ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
+static int
+ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	uint16_t i;
+	struct rte_eth_rxmode *rxmode;
+	struct ixgbe_rx_queue *rxq;
+
+	if (mask & ETH_VLAN_STRIP_MASK) {
+		rxmode = &dev->data->dev_conf.rxmode;
+		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+		else
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+	}
+
+	ixgbe_vlan_offload_config(dev, mask);
+
+	return 0;
+}
+
 static void
 ixgbe_vmdq_vlan_hw_filter_enable(struct rte_eth_dev *dev)
 {
@@ -2577,7 +2608,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 
 	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
 		ETH_VLAN_EXTEND_MASK;
-	err = ixgbe_vlan_offload_set(dev, mask);
+	err = ixgbe_vlan_offload_config(dev, mask);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Unable to set VLAN offload");
 		goto error;
@@ -5022,7 +5053,7 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 	/* Set HW strip */
 	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
 		ETH_VLAN_EXTEND_MASK;
-	err = ixgbevf_vlan_offload_set(dev, mask);
+	err = ixgbevf_vlan_offload_config(dev, mask);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err);
 		ixgbe_dev_clear_queues(dev);
@@ -5220,7 +5251,7 @@ ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on)
 }
 
 static int
-ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+ixgbevf_vlan_offload_config(struct rte_eth_dev *dev, int mask)
 {
 	struct ixgbe_rx_queue *rxq;
 	uint16_t i;
@@ -5238,6 +5269,32 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
+static int
+ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	uint16_t i;
+	struct rte_eth_rxmode *rxmode;
+	struct ixgbe_rx_queue *rxq;
+
+	if (mask & ETH_VLAN_STRIP_MASK) {
+		rxmode = &dev->data->dev_conf.rxmode;
+		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+		else
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+
+		ixgbevf_vlan_offload_config(dev, mask);
+	}
+
+	return 0;
+}
+
 int
 ixgbe_vt_check(struct ixgbe_hw *hw)
 {
-- 
2.7.5

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port

[Dai, Wei]

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 <qi.z.zhang@intel.com>; Wu, Yanglong
> <yanglong.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for
> per port
> 
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Friday, May 18, 2018 8:37 PM
> > To: Dai, Wei <wei.dai@intel.com>; Wu, Yanglong
> > <yanglong.wu@intel.com>; 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 <qi.z.zhang@intel.com>; Wu, Yanglong
> > > <yanglong.wu@intel.com>; 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 <yanglong.wu@intel.com>; dev@dpdk.org
> > > > Cc: Dai, Wei <wei.dai@intel.com>
> > > > 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 <qi.z.zhang@intel.com>; Dai, Wei
> > > > > <wei.dai@intel.com>; Wu, Yanglong <yanglong.wu@intel.com>
> > > > > 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 <yanglong.wu@intel.com>
> > > >
> > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > >
> > > 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.
> >
> > 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
> 
> 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 <port_id>"
> 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)
> 
> 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);
> 
> And the ixgbe_vlan_offload_set() is also be called by ixgbe_dev_start() which
> 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
> different way as follows:
> 
> 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()
> 		}
> 
> Ixgbe_dev_start()
> 	ixgbe_vlan_offload_set()
> 

Re: [dpdk-dev] [PATCH v4] net/ixgbe: config VLAN strip on the fly

[Zhang, Qi Z]

> -----Original Message-----
> From: Dai, Wei
> Sent: Saturday, May 19, 2018 12:09 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Yanglong <yanglong.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v4] net/ixgbe: config VLAN strip on the fly
> 
> The old ixgbe_vlan_offload_set() is called by
> rte_eth_dev_set_vlan_offload() which is meant to config VLAN
> strip/filter/extend on all queues.
> This old function is also called by rte_eth_dev_start()/ixgbe_dev_start()

OK, I think this is what we missed in previous patch, good capture, thanks!

> which need support per-queue VALN strip on only parts of queues.
> So add new function ixgbe_vlan_offload_config() = old
> ixgbe_vlan_offload_set().
> New ixgbe_vlan_offload_set =  codes to align VLAN strip flags on all queues
> with port level setting + ixgbe_vlan_offload_configure().
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>

Though the code can be improved to remove some redundant , but I think it's not a big deal right now.
BTW, I think we still need the fixed line, so it will be added during apply.

Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in VF")

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

[dpdk-dev] [PATCH v5] net/ixgbe: fix to config VLAN strip on the fly

[Wei Dai]

The old ixgbe_vlan_offload_set() is called by
rte_eth_dev_set_vlan_offload() which is meant to config VLAN
strip/filter/extend on all queues.
This old function is also called by rte_eth_dev_start()/ixgbe_dev_start()
which need support per-queue VALN strip on only parts of queues.

So add new function ixgbe_vlan_offload_config() =
old ixgbe_vlan_offload_set().
This new function is called by ixgbe_dev_start() to support VLAN strip
per-queue configuration.

New ixgbe_vlan_offload_set() = codes to align VLAN strip flags on all
queues with port level setting + new ixgbe_vlan_offload_configure().
The 2nd function is called by rte_eth_dev_set_vlan_offload to support
configure VLAN strip on all queues of whole port.

Fixes: 216f78f4d53f ("net/ixgbe: support VLAN strip per queue offloading in PF")
Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in VF")

Cc: stable@dpdk.org

Signed-off-by: Wei Dai <weid.dai@intel.com>
Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

---
v5:
  reduce redundant codes
  add Fixes in git log message

v4:
  fix typo error in git log message

v3:
  keep vlan strip as per-queue capability
  and support config vlan on port level on the fly

v2:
  rework as comments asked
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 61 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7219f02..0b150cb 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -189,6 +189,9 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev,
 		uint16_t queue, bool on);
 static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue,
 		int on);
+static void ixgbe_config_vlan_strip_on_all_queues(struct rte_eth_dev *dev,
+						  int mask);
+static int ixgbe_vlan_offload_config(struct rte_eth_dev *dev, int mask);
 static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue);
 static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue);
@@ -246,6 +249,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev,
 		uint16_t queue, int on);
+static int ixgbevf_vlan_offload_config(struct rte_eth_dev *dev, int mask);
 static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on);
 static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
@@ -1974,10 +1978,13 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 
 	rxq = dev->data->rx_queues[queue];
 
-	if (on)
+	if (on) {
 		rxq->vlan_flags = PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED;
-	else
+		rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+	} else {
 		rxq->vlan_flags = PKT_RX_VLAN;
+		rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+	}
 }
 
 static void
@@ -2129,8 +2136,30 @@ ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev)
 	}
 }
 
+static void
+ixgbe_config_vlan_strip_on_all_queues(struct rte_eth_dev *dev, int mask)
+{
+	uint16_t i;
+	struct rte_eth_rxmode *rxmode;
+	struct ixgbe_rx_queue *rxq;
+
+	if (mask & ETH_VLAN_STRIP_MASK) {
+		rxmode = &dev->data->dev_conf.rxmode;
+		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+		else
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				rxq->offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+			}
+	}
+}
+
 static int
-ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+ixgbe_vlan_offload_config(struct rte_eth_dev *dev, int mask)
 {
 	struct rte_eth_rxmode *rxmode;
 	rxmode = &dev->data->dev_conf.rxmode;
@@ -2156,6 +2185,16 @@ ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
+static int
+ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	ixgbe_config_vlan_strip_on_all_queues(dev, mask);
+
+	ixgbe_vlan_offload_config(dev, mask);
+
+	return 0;
+}
+
 static void
 ixgbe_vmdq_vlan_hw_filter_enable(struct rte_eth_dev *dev)
 {
@@ -2577,7 +2616,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 
 	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
 		ETH_VLAN_EXTEND_MASK;
-	err = ixgbe_vlan_offload_set(dev, mask);
+	err = ixgbe_vlan_offload_config(dev, mask);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Unable to set VLAN offload");
 		goto error;
@@ -5022,7 +5061,7 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 	/* Set HW strip */
 	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
 		ETH_VLAN_EXTEND_MASK;
-	err = ixgbevf_vlan_offload_set(dev, mask);
+	err = ixgbevf_vlan_offload_config(dev, mask);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err);
 		ixgbe_dev_clear_queues(dev);
@@ -5220,7 +5259,7 @@ ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on)
 }
 
 static int
-ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+ixgbevf_vlan_offload_config(struct rte_eth_dev *dev, int mask)
 {
 	struct ixgbe_rx_queue *rxq;
 	uint16_t i;
@@ -5238,6 +5277,16 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
+static int
+ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+{
+	ixgbe_config_vlan_strip_on_all_queues(dev, mask);
+
+	ixgbevf_vlan_offload_config(dev, mask);
+
+	return 0;
+}
+
 int
 ixgbe_vt_check(struct ixgbe_hw *hw)
 {
-- 
2.7.5

Re: [dpdk-dev] [PATCH v4] net/ixgbe: config VLAN strip on the fly

[Dai, Wei]

Hi, Zhang Qi
Thanks for your feedback.
I've just submitted v5 patch to follow your guidance.

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Saturday, May 19, 2018 8:19 AM
> To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Yanglong
> <yanglong.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v4] net/ixgbe: config VLAN strip on the fly
> 
> 
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Saturday, May 19, 2018 12:09 AM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Wu, Yanglong <yanglong.wu@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [PATCH v4] net/ixgbe: config VLAN strip on the fly
> >
> > The old ixgbe_vlan_offload_set() is called by
> > rte_eth_dev_set_vlan_offload() which is meant to config VLAN
> > strip/filter/extend on all queues.
> > This old function is also called by
> > rte_eth_dev_start()/ixgbe_dev_start()
> 
> OK, I think this is what we missed in previous patch, good capture, thanks!
> 
> > which need support per-queue VALN strip on only parts of queues.
> > So add new function ixgbe_vlan_offload_config() = old
> > ixgbe_vlan_offload_set().
> > New ixgbe_vlan_offload_set =  codes to align VLAN strip flags on all
> > queues with port level setting + ixgbe_vlan_offload_configure().
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
> 
> Though the code can be improved to remove some redundant , but I think
> it's not a big deal right now.
> BTW, I think we still need the fixed line, so it will be added during apply.
> 
> Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in
> VF")
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v5] net/ixgbe: fix to config VLAN strip on the fly

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> Sent: Saturday, May 19, 2018 6:11 PM
> To: Zhang, Qi Z; Yigit, Ferruh; Lu, Wenzhuo; Ananyev, Konstantin; Wu,
> Yanglong
> Cc: dev@dpdk.org; Dai, Wei; stable@dpdk.org; Wei Dai
> Subject: [dpdk-dev] [PATCH v5] net/ixgbe: fix to config VLAN strip on the fly
> 
> The old ixgbe_vlan_offload_set() is called by
> rte_eth_dev_set_vlan_offload() which is meant to config VLAN
> strip/filter/extend on all queues.
> This old function is also called by rte_eth_dev_start()/ixgbe_dev_start()
> which need support per-queue VALN strip on only parts of queues.
> 
> So add new function ixgbe_vlan_offload_config() = old
> ixgbe_vlan_offload_set().
> This new function is called by ixgbe_dev_start() to support VLAN strip per-
> queue configuration.
> 
> New ixgbe_vlan_offload_set() = codes to align VLAN strip flags on all queues
> with port level setting + new ixgbe_vlan_offload_configure().
> The 2nd function is called by rte_eth_dev_set_vlan_offload to support
> configure VLAN strip on all queues of whole port.
> 
> Fixes: 216f78f4d53f ("net/ixgbe: support VLAN strip per queue offloading in PF")
> Fixes: 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading in
> VF")
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <weid.dai@intel.com>
> Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied to dpdk-next-net-intel, thanks!

/Helin