net/ixgbe: fix VLAN strip setting fail for per port
PATCH 1/1 net/ixgbe: fix VLAN strip setting fail for per port
VLAN-strip should be set by both per port and per queue
--- drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/ixgbe/ixgbe_ethdev.c +3 -1
@@ -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 Ð_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); } }
Signature · 1 line
2.11.0
Hi Yanglong:
Quoted 23 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Thursday, May 17, 2018 1:52 PM
To: [email protected]
Cc: Dai, Wei <[email protected]>; Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH] net/ixgbe: fix VLAN strip setting fail for per portVLAN-strip should be set by both per port and per queue
Fixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading inVF")
Signed-off-by Yanglong Wu <[email protected]>--- drivers/net/ixgbe/ixgbe_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/net/ixgbe/ixgbe_ethdev.c +0
-0
drivers/net/ixgbe/ixgbe_ethdev.c ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, · line 5233
Quoted 6 lines
drivers/net/ixgbe/ixgbe_ethdev.c +2
-0@@ -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 Ð_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
Quoted 11 lines
drivers/net/ixgbe/ixgbe_ethdev.c +1
-1for (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 );drivers/net/ixgbe/ixgbe_ethdev.c +0
-1ixgbevf_vlan_strip_queue_set (dev, i, on); } } --2.11.0
rxq->offload should synchronize with dev_conf
---
V2:
rework as comments
--- drivers/net/ixgbe/ixgbe_ethdev.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/net/ixgbe/ixgbe_ethdev.c +16 -0
@@ -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 Ð_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); }
Signature · 1 line
2.11.0
PATCH v2 1/1 net/ixgbe: fix VLAN strip setting fail for per port
In reply to Yanglong Wurxq->offload should synchronize with dev_conf
---
v2:
rework as comments asked
--- drivers/net/ixgbe/ixgbe_ethdev.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/net/ixgbe/ixgbe_ethdev.c +16 -0
@@ -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 Ð_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); }
Signature · 1 line
2.11.0
Quoted 8 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Friday, May 18, 2018 3:24 PM
To: [email protected]
Cc: Zhang, Qi Z <[email protected]>; Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port
Quoted 5 lines
rxq->offloadshould synchronize withdev_confFixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queue offloading inVF")
Signed-off-by Yanglong Wu <[email protected]>
Quoted 16 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 3:46 PM
To: Wu, Yanglong <[email protected]>; [email protected]
Cc: Dai, Wei <[email protected]>
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 9 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Friday, May 18, 2018 3:24 PM
To: [email protected]
Cc: Zhang, Qi Z <[email protected]>; Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port
Quoted 8 lines
Quoted 6 lines
rxq->offloadshould synchronize withdev_confFixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queueoffloading in
VF")Signed-off-by Yanglong Wu <[email protected]>Acked-by Qi Zhang <[email protected]>
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.
Hi Daiwei:
Quoted 31 lines
-----Original Message-----
From: Dai, Wei
Sent: Friday, May 18, 2018 7:07 PM
To: Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 24 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 3:46 PM
To: Wu, Yanglong <[email protected]>; [email protected]
Cc: Dai, Wei <[email protected]>
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 15 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Friday, May 18, 2018 3:24 PM
To: [email protected]
Cc: Zhang, Qi Z <[email protected]>; Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port
rxq->offloadshould synchronize withdev_confFixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queueoffloading in
VF")Signed-off-by Yanglong Wu <[email protected]>
Quoted 7 lines
Quoted 1 line
Acked-by Qi Zhang <[email protected]>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
Quoted 35 lines · Qi Z
Hi Daiwei:
Quoted 33 lines
-----Original Message-----
From: Dai, Wei
Sent: Friday, May 18, 2018 7:07 PM
To: Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 25 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 3:46 PM
To: Wu, Yanglong <[email protected]>; [email protected]
Cc: Dai, Wei <[email protected]>
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 9 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Friday, May 18, 2018 3:24 PM
To: [email protected]
Cc: Zhang, Qi Z <[email protected]>; Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 6 lines
rxq->offloadshould synchronize withdev_confFixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queueoffloading in
VF")Signed-off-by Yanglong Wu <[email protected]>Acked-by Qi Zhang <[email protected]>
Quoted 8 lines
Quoted 5 lines
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 ofi40e_vlan_offload_sethere 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?
Quoted 3 lines
Thanks
Qi
Quoted 56 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 8:37 PM
To: Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portHi Daiwei:
Quoted 32 lines
-----Original Message-----
From: Dai, Wei
Sent: Friday, May 18, 2018 7:07 PM
To: Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 24 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 3:46 PM
To: Wu, Yanglong <[email protected]>; [email protected]
Cc: Dai, Wei <[email protected]>
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 15 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Friday, May 18, 2018 3:24 PM
To: [email protected]
Cc: Zhang, Qi Z <[email protected]>; Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port
rxq->offloadshould synchronize withdev_confFixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queueoffloading in
VF")Signed-off-by Yanglong Wu <[email protected]>Quoted 7 lines
Quoted 1 line
Acked-by Qi Zhang <[email protected]>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 ofi40e_vlan_offload_sethere 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.
Quoted 2 lines
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()
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.
Quoted 108 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Dai, Wei
Sent: Friday, May 18, 2018 11:06 PM
To: Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 57 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 8:37 PM
To: Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portHi Daiwei:
Quoted 32 lines
-----Original Message-----
From: Dai, Wei
Sent: Friday, May 18, 2018 7:07 PM
To: Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; [email protected]
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 24 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Friday, May 18, 2018 3:46 PM
To: Wu, Yanglong <[email protected]>; [email protected]
Cc: Dai, Wei <[email protected]>
Subject: RE: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per portQuoted 15 lines
-----Original Message-----
From: Wu, Yanglong
Sent: Friday, May 18, 2018 3:24 PM
To: [email protected]
Cc: Zhang, Qi Z <[email protected]>; Dai, Wei <[email protected]>; Wu, Yanglong <[email protected]>
Subject: [PATCH v2] net/ixgbe: fix VLAN strip setting fail for per port
rxq->offloadshould synchronize withdev_confFixes 860a94d3c692 ("net/ixgbe: support VLAN strip per queueoffloading in
VF")Signed-off-by Yanglong Wu <[email protected]>Quoted 7 lines
Quoted 1 line
Acked-by Qi Zhang <[email protected]>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_sethere 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.Quoted 2 lines
Thanks
QiAs
VLAN_STRIPhas 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/disableVLAN_STRIPon 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); //modifydev->data->dev_conf.rxmode.offloadsdev->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_tqueue, bool on)As the
VLAN_STRIPis per-queue capability,ixgbe_vlan_hw_strip_config()only get it fromrxq->offloadswhich hasn't been update inrte_eth_dev_set_vlan_offload(port_id, vlan_offload);And the
ixgbe_vlan_offload_set()is also be called byixgbe_dev_start()which is normal called afterdev_configure()andqueue_setup().These are 2 different path to config
VLAN_STRIP.
So we can add a functionixgbe_vlan_offload_config()to let them go different way as follows:
dev->dev_ops->vlan_offload_set(dev, mask)-> point to new functiondev->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()
PATCH v3 1/1 net/ixgbe: config VLAN strip on the fly
In reply to Yanglong WuThe 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().
--- drivers/net/ixgbe/ixgbe_ethdev.c | 69 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-)
drivers/net/ixgbe/ixgbe_ethdev.c +63 -6
@@ -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 Ð_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 Ð_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) {
Signature · 1 line
2.7.5
PATCH v4 1/1 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() 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().
---
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(-)
drivers/net/ixgbe/ixgbe_ethdev.c +63 -6
@@ -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 Ð_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 Ð_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) {
Signature · 1 line
2.7.5
Re: [PATCH v4] net/ixgbe: config VLAN strip on the fly
Quoted 11 lines
-----Original Message-----
From: Dai, Wei
Sent: Saturday, May 19, 2018 12:09 AM
To: Lu, Wenzhuo <[email protected]>; Ananyev, Konstantin <[email protected]>; Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; Yigit, Ferruh <[email protected]>
Cc: [email protected]; Dai, Wei <[email protected]>
Subject: [PATCH v4] net/ixgbe: config VLAN strip on the fly
Quoted 4 lines
The old
ixgbe_vlan_offload_set()is called byrte_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!
Quoted 8 lines
which need support per-queue VALN strip on only parts of queues. So add new function
ixgbe_vlan_offload_config()= oldixgbe_vlan_offload_set().
Newixgbe_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 <[email protected]>Signed-off-by Yanglong Wu <[email protected]>
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.
Re: [PATCH v4] net/ixgbe: config VLAN strip on the fly
Hi, Zhang Qi
Thanks for your feedback.
I've just submitted v5 patch to follow your guidance.
Quoted 46 lines
-----Original Message-----
From: Zhang, Qi Z
Sent: Saturday, May 19, 2018 8:19 AM
To: Dai, Wei <[email protected]>; Lu, Wenzhuo <[email protected]>; Ananyev, Konstantin <[email protected]>; Wu, Yanglong <[email protected]>; Yigit, Ferruh <[email protected]>
Cc: [email protected]
Subject: RE: [PATCH v4] net/ixgbe: config VLAN strip on the flyQuoted 10 lines
-----Original Message-----
From: Dai, Wei
Sent: Saturday, May 19, 2018 12:09 AM
To: Lu, Wenzhuo <[email protected]>; Ananyev, Konstantin <[email protected]>; Zhang, Qi Z <[email protected]>; Wu, Yanglong <[email protected]>; Yigit, Ferruh <[email protected]>
Cc: [email protected]; Dai, Wei <[email protected]>
Subject: [PATCH v4] net/ixgbe: config VLAN strip on the flyQuoted 5 lines
The old
ixgbe_vlan_offload_set()is called byrte_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!
Quoted 8 lines
which need support per-queue VALN strip on only parts of queues. So add new function
ixgbe_vlan_offload_config()= oldixgbe_vlan_offload_set().
Newixgbe_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 <[email protected]>Signed-off-by Yanglong Wu <[email protected]>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 inVF")
Acked-by Qi Zhang <[email protected]>
PATCH v5 1/1 net/ixgbe: fix to config VLAN strip on the fly
In reply to Wei DaiThe 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.
---
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(-)
drivers/net/ixgbe/ixgbe_ethdev.c +55 -6
@@ -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 Ð_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) {
Signature · 1 line
2.7.5
Zhang, Helin
AppliedRe: [PATCH v5] net/ixgbe: fix to config VLAN strip on the fly
Quoted 28 lines
-----Original Message-----
From: dev [mailto:[email protected]] 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: [email protected]; Dai, Wei; [email protected]; Wei Dai
Subject: [dpdk-dev] [PATCH v5] net/ixgbe: fix to config VLAN strip on the flyThe old
ixgbe_vlan_offload_set()is called byrte_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()= oldixgbe_vlan_offload_set().
This new function is called byixgbe_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 + newixgbe_vlan_offload_configure(). The 2nd function is called byrte_eth_dev_set_vlan_offloadto 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
Quoted 8 lines
VF")
Signed-off-by Wei Dai <[email protected]>Signed-off-by Yanglong Wu <[email protected]>Acked-by Qi Zhang <[email protected]>
Applied to dpdk-next-net-intel, thanks!
/Helin