diff mbox series

[v2] ethdev: fix QinQ strip offload support

Message ID 1555486846-20764-1-git-send-email-viveksharma@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] ethdev: fix QinQ strip offload support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Vivek Kumar Sharma April 17, 2019, 7:40 a.m. UTC 
  From: Vivek Sharma <viveksharma@marvell.com>

Enable missing support for QinQ strip rx offload
in vlan offload set/get methods.

Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion")
Cc: stable@dpdk.org

Signed-off-by: Vivek Sharma <viveksharma@marvell.com>
---
v2:
* Use pointer to dereference dev->data->dev_conf.rxmode.offloads 
 
 lib/librte_ethdev/rte_ethdev.c | 55 +++++++++++++++++++++++-------------------
 lib/librte_ethdev/rte_ethdev.h |  5 +++-
 2 files changed, 34 insertions(+), 26 deletions(-)

Comments

Thomas Monjalon April 17, 2019, 8:34 a.m. UTC | #1 
17/04/2019 09:40, viveksharma@marvell.com:
> From: Vivek Sharma <viveksharma@marvell.com>
> 
> Enable missing support for QinQ strip rx offload
> in vlan offload set/get methods.
> 
> Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion")
> Cc: stable@dpdk.org

Not sure it is a fix.
The commit mentioned above allows some kind of offload config.
You are extending the offload config with support in
rte_eth_dev_set_vlan_offload().
Vivek Kumar Sharma April 18, 2019, 7:38 a.m. UTC | #2 
>> From: Vivek Sharma <viveksharma@marvell.com>
>>
>> Enable missing support for QinQ strip rx offload
>> in vlan offload set/get methods.
>>
>> Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion")
>> Cc: stable@dpdk.org

>Not sure it is a fix.
>The commit mentioned above allows some kind of offload config.
>You are extending the offload config with support in
>rte_eth_dev_set_vlan_offload().

DEV_RX_OFFLOAD_QINQ_STRIP was introduced in cc9d0456b870 ("i40e: support double vlan stripping and insertion"). But, the means to utilize this capability by enabling this rx offload for an ethdev was missing from the patch. The current patch fixes that missing functionality so that user can enable QinQ strip rx offload for capable devices.
Thomas Monjalon April 18, 2019, 8:07 a.m. UTC | #3 
18/04/2019 09:38, Vivek Kumar Sharma:
> >> From: Vivek Sharma <viveksharma@marvell.com>
> >>
> >> Enable missing support for QinQ strip rx offload
> >> in vlan offload set/get methods.
> >>
> >> Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion")
> >> Cc: stable@dpdk.org
> 
> >Not sure it is a fix.
> >The commit mentioned above allows some kind of offload config.
> >You are extending the offload config with support in
> >rte_eth_dev_set_vlan_offload().
> 
> DEV_RX_OFFLOAD_QINQ_STRIP was introduced in cc9d0456b870 ("i40e: support double vlan stripping and insertion"). But, the means to utilize this capability by enabling this rx offload for an ethdev was missing from the patch. The current patch fixes that missing functionality so that user can enable QinQ strip rx offload for capable devices. 

It was possible to utilize the offload by setting the flag in mbuf.
You are adding a way to configure it at port level. So it is not a fix.
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 243beb4..3e770bc 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2723,53 +2723,56 @@  rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
 	int mask = 0;
 	int cur, org = 0;
 	uint64_t orig_offloads;
+	uint64_t *dev_offloads;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
 	/* save original values in case of failure */
 	orig_offloads = dev->data->dev_conf.rxmode.offloads;
+	dev_offloads = &dev->data->dev_conf.rxmode.offloads;
 
 	/*check which option changed by application*/
 	cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD);
-	org = !!(dev->data->dev_conf.rxmode.offloads &
-		 DEV_RX_OFFLOAD_VLAN_STRIP);
+	org = !!(*dev_offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 	if (cur != org) {
 		if (cur)
-			dev->data->dev_conf.rxmode.offloads |=
-				DEV_RX_OFFLOAD_VLAN_STRIP;
+			*dev_offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
 		else
-			dev->data->dev_conf.rxmode.offloads &=
-				~DEV_RX_OFFLOAD_VLAN_STRIP;
+			*dev_offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
 		mask |= ETH_VLAN_STRIP_MASK;
 	}
 
 	cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD);
-	org = !!(dev->data->dev_conf.rxmode.offloads &
-		 DEV_RX_OFFLOAD_VLAN_FILTER);
+	org = !!(*dev_offloads & DEV_RX_OFFLOAD_VLAN_FILTER);
 	if (cur != org) {
 		if (cur)
-			dev->data->dev_conf.rxmode.offloads |=
-				DEV_RX_OFFLOAD_VLAN_FILTER;
+			*dev_offloads |= DEV_RX_OFFLOAD_VLAN_FILTER;
 		else
-			dev->data->dev_conf.rxmode.offloads &=
-				~DEV_RX_OFFLOAD_VLAN_FILTER;
+			*dev_offloads &= ~DEV_RX_OFFLOAD_VLAN_FILTER;
 		mask |= ETH_VLAN_FILTER_MASK;
 	}
 
 	cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD);
-	org = !!(dev->data->dev_conf.rxmode.offloads &
-		 DEV_RX_OFFLOAD_VLAN_EXTEND);
+	org = !!(*dev_offloads & DEV_RX_OFFLOAD_VLAN_EXTEND);
 	if (cur != org) {
 		if (cur)
-			dev->data->dev_conf.rxmode.offloads |=
-				DEV_RX_OFFLOAD_VLAN_EXTEND;
+			*dev_offloads |= DEV_RX_OFFLOAD_VLAN_EXTEND;
 		else
-			dev->data->dev_conf.rxmode.offloads &=
-				~DEV_RX_OFFLOAD_VLAN_EXTEND;
+			*dev_offloads &= ~DEV_RX_OFFLOAD_VLAN_EXTEND;
 		mask |= ETH_VLAN_EXTEND_MASK;
 	}
 
+	cur = !!(offload_mask & ETH_QINQ_STRIP_OFFLOAD);
+	org = !!(*dev_offloads & DEV_RX_OFFLOAD_QINQ_STRIP);
+	if (cur != org) {
+		if (cur)
+			*dev_offloads |= DEV_RX_OFFLOAD_QINQ_STRIP;
+		else
+			*dev_offloads &= ~DEV_RX_OFFLOAD_QINQ_STRIP;
+		mask |= ETH_QINQ_STRIP_MASK;
+	}
+
 	/*no change*/
 	if (mask == 0)
 		return ret;
@@ -2778,7 +2781,7 @@  rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
 	ret = (*dev->dev_ops->vlan_offload_set)(dev, mask);
 	if (ret) {
 		/* hit an error restore  original values */
-		dev->data->dev_conf.rxmode.offloads = orig_offloads;
+		*dev_offloads = orig_offloads;
 	}
 
 	return eth_err(port_id, ret);
@@ -2788,23 +2791,25 @@  int
 rte_eth_dev_get_vlan_offload(uint16_t port_id)
 {
 	struct rte_eth_dev *dev;
+	uint64_t *dev_offloads;
 	int ret = 0;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+	dev_offloads = &dev->data->dev_conf.rxmode.offloads;
 
-	if (dev->data->dev_conf.rxmode.offloads &
-	    DEV_RX_OFFLOAD_VLAN_STRIP)
+	if (*dev_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
 		ret |= ETH_VLAN_STRIP_OFFLOAD;
 
-	if (dev->data->dev_conf.rxmode.offloads &
-	    DEV_RX_OFFLOAD_VLAN_FILTER)
+	if (*dev_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		ret |= ETH_VLAN_FILTER_OFFLOAD;
 
-	if (dev->data->dev_conf.rxmode.offloads &
-	    DEV_RX_OFFLOAD_VLAN_EXTEND)
+	if (*dev_offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)
 		ret |= ETH_VLAN_EXTEND_OFFLOAD;
 
+	if (*dev_offloads & DEV_RX_OFFLOAD_QINQ_STRIP)
+		ret |= DEV_RX_OFFLOAD_QINQ_STRIP;
+
 	return ret;
 }
 
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 40a068f..c1792f4 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -550,11 +550,13 @@  struct rte_eth_rss_conf {
 #define ETH_VLAN_STRIP_OFFLOAD   0x0001 /**< VLAN Strip  On/Off */
 #define ETH_VLAN_FILTER_OFFLOAD  0x0002 /**< VLAN Filter On/Off */
 #define ETH_VLAN_EXTEND_OFFLOAD  0x0004 /**< VLAN Extend On/Off */
+#define ETH_QINQ_STRIP_OFFLOAD   0x0008 /**< QINQ Strip On/Off */
 
 /* Definitions used for mask VLAN setting */
 #define ETH_VLAN_STRIP_MASK   0x0001 /**< VLAN Strip  setting mask */
 #define ETH_VLAN_FILTER_MASK  0x0002 /**< VLAN Filter  setting mask*/
 #define ETH_VLAN_EXTEND_MASK  0x0004 /**< VLAN Extend  setting mask*/
+#define ETH_QINQ_STRIP_MASK   0x0008 /**< QINQ Strip  setting mask */
 #define ETH_VLAN_ID_MAX       0x0FFF /**< VLAN ID is in lower 12 bits*/
 
 /* Definitions used for receive MAC address   */
@@ -965,7 +967,8 @@  struct rte_eth_conf {
 				 DEV_RX_OFFLOAD_TCP_CKSUM)
 #define DEV_RX_OFFLOAD_VLAN (DEV_RX_OFFLOAD_VLAN_STRIP | \
 			     DEV_RX_OFFLOAD_VLAN_FILTER | \
-			     DEV_RX_OFFLOAD_VLAN_EXTEND)
+			     DEV_RX_OFFLOAD_VLAN_EXTEND | \
+			     DEV_RX_OFFLOAD_QINQ_STRIP)
 
 /*
  * If new Rx offload capabilities are defined, they also must be