Skip to contentSkip to conversation

net/i40e: issue with ADD VLAN from Guest

PATCH Superseded Souvik Dey 4 participants 13 messages Dec 9, 2020 – Jan 5, 2021 Network drivers

lore
Share & tools
lore

Souvik Dey

author
#0 lore raw

PATCH 1/1 net/i40e: issue with ADD VLAN from Guest

In case of i40evf pmd, when ADD_VLAN is sent down the linux i40e driver, along with add the vlan the kernel driver also enables the vlan stripping by default.
This might have issues if the app configured DEV_RX_OFFLOAD_VLAN_STRIP as off at the port configuration. The app after adding the VLAN will expect the VLAN to be coming in the received packets but, due to VLAN_STRIP enabled at the PF, it will get stripped.
This behavior of the Linux driver causes confussion with the DPDK app using i40e_pmd. So it is better to reconfigure the vlan_offload, which checks for DEV_RX_OFFLOAD_VLAN_STRIP flag in the dev_conf and enables or disables the vlan strip in the PF.
Also rte_eth_dev_set_vlan_offload() to disable VLAN_STRIP cannot be used from the application, as this will only work for the first time when original and current confi mismatch, but for all subsequent call it will ignore it.

---
 drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +12 -1
index c26b036..2ecf74b 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	args.out_buffer = vf->aq_resp;
 	args.out_size = I40E_AQ_BUF_SZ;
 	err = i40evf_execute_vf_cmd(dev, &args);
-	if (err)
+	if (err) {
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+		return err;
+	}
+	/*
+	 * In linux kernel driver on receiving ADD_VLAN it enables
+	 * VLAN_STRIP by default. So reconfigure the vlan_offload
+	 * as it was done by the app earlier.
+	 */
+	err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	if (err)
+		PMD_DRV_LOG(ERR, "fail to execute command disable_vlan_strip "
+			"as a part of OP_ADD_VLAN");
 
 	return err;
 }
Signature · 1 line
2.9.3.windows.1
#1 lore raw

hi, souvik

From: Souvik Dey <[email protected]>
Sent: Thursday, December 10, 2020 1:55 AM
To: Xing, Beilei <[email protected]>; Guo, Jia <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Souvik Dey <[email protected]>
Subject: [PATCH] net/i40e: issue with ADD VLAN from Guest

In case of i40evf pmd, when ADD_VLAN is sent down the linux i40e driver, along with add the vlan the kernel driver also enables the vlan stripping by default.
This might have issues if the app configured DEV_RX_OFFLOAD_VLAN_STRIP as off at the port configuration. The app after adding the VLAN will expect the VLAN to be coming in the received packets but, due to VLAN_STRIP enabled at the PF, it will get stripped.
This behavior of the Linux driver causes confussion with the DPDK app using i40e_pmd. So it is better to reconfigure the vlan_offload, which checks for DEV_RX_OFFLOAD_VLAN_STRIP flag in the dev_conf and enables or disables the vlan strip in the PF.
Also rte_eth_dev_set_vlan_offload() to disable VLAN_STRIP cannot be used from the application, as this will only work for the first time when original and current confi mismatch, but for all subsequent call it will ignore it.

Thanks for your patch and the clear detail interpret, what you said is reset the configuration about vlan strip that
would be change by the pf driver when adding vlan from vf, so I think concentrate<http://www.baidu.com/link?url=FKtXEQdgI9iuD4HN0uZIpf5hHCbYVGlPJx-jzQ-bGjXQiZutjIi6XT6rPlEoGfM13HvgvT9Gt88OAPmEKXNitp-rKK4k50M6di1HF4bDMxa&wd=&eqid=e7f451dc0001a611000000045fd2e0e1> the commit log to be more simple in your way would be enough.

---
drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-

1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +0 -0
index c26b036..2ecf74b 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)

args.out_buffer = vf->aq_resp;
args.out_size = I40E_AQ_BUF_SZ;
err = i40evf_execute_vf_cmd(dev, &args);
- if (err)
+ if (err) {
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ return err;

Would it be more code clean to use “goto err;”?

+ }
+ /*

/* -> /**, that would what I suggest.

+ * In linux kernel driver on receiving ADD_VLAN it enables
+ * VLAN_STRIP by default. So reconfigure the vlan_offload
+ * as it was done by the app earlier.
+ */
+ err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to execute command disable_vlan_strip "
+ "as a part of OP_ADD_VLAN");

If it is not coming as a command, please refine this log, such as “fail to set vlan strip.”

return err;
}
--
2.9.3.windows.1

________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

#2 lore raw

Hi Guo,
Thanks for the comments. I will upload a v2 of the patch.

--
Regards,
Souvik

From: Guo, Jia <[email protected]>
Sent: Thursday, December 10, 2020 10:08 PM
To: Dey, Souvik <[email protected]>; Xing, Beilei <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]
Subject: RE: [PATCH] net/i40e: issue with ADD VLAN from Guest

________________________________
NOTICE: This email was received from an EXTERNAL sender
________________________________

hi, souvik

From: Souvik Dey <[email protected]<mailto:[email protected]>>
Sent: Thursday, December 10, 2020 1:55 AM
To: Xing, Beilei <[email protected]<mailto:[email protected]>>; Guo, Jia <[email protected]<mailto:[email protected]>>; Zhang, Qi Z <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>; Souvik Dey <[email protected]<mailto:[email protected]>>
Subject: [PATCH] net/i40e: issue with ADD VLAN from Guest

In case of i40evf pmd, when ADD_VLAN is sent down the linux i40e driver, along with add the vlan the kernel driver also enables the vlan stripping by default.
This might have issues if the app configured DEV_RX_OFFLOAD_VLAN_STRIP as off at the port configuration. The app after adding the VLAN will expect the VLAN to be coming in the received packets but, due to VLAN_STRIP enabled at the PF, it will get stripped.
This behavior of the Linux driver causes confussion with the DPDK app using i40e_pmd. So it is better to reconfigure the vlan_offload, which checks for DEV_RX_OFFLOAD_VLAN_STRIP flag in the dev_conf and enables or disables the vlan strip in the PF.
Also rte_eth_dev_set_vlan_offload() to disable VLAN_STRIP cannot be used from the application, as this will only work for the first time when original and current confi mismatch, but for all subsequent call it will ignore it.

Thanks for your patch and the clear detail interpret, what you said is reset the configuration about vlan strip that
would be change by the pf driver when adding vlan from vf, so I think concentrate<http://www.baidu.com/link?url=FKtXEQdgI9iuD4HN0uZIpf5hHCbYVGlPJx-jzQ-bGjXQiZutjIi6XT6rPlEoGfM13HvgvT9Gt88OAPmEKXNitp-rKK4k50M6di1HF4bDMxa&wd=&eqid=e7f451dc0001a611000000045fd2e0e1> the commit log to be more simple in your way would be enough.

---
drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-

1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +0 -0
index c26b036..2ecf74b 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)

args.out_buffer = vf->aq_resp;
args.out_size = I40E_AQ_BUF_SZ;
err = i40evf_execute_vf_cmd(dev, &args);
- if (err)
+ if (err) {
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ return err;

Would it be more code clean to use “goto err;”?

+ }
+ /*

/* -> /**, that would what I suggest.

+ * In linux kernel driver on receiving ADD_VLAN it enables
+ * VLAN_STRIP by default. So reconfigure the vlan_offload
+ * as it was done by the app earlier.
+ */
+ err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to execute command disable_vlan_strip "
+ "as a part of OP_ADD_VLAN");

If it is not coming as a command, please refine this log, such as “fail to set vlan strip.”

return err;
}
--
2.9.3.windows.1

________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

author
#3 lore raw

PATCH v2 1/1 net/i40e: issue with ADD VLAN from Guest

In reply to Souvik Dey

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

---
v2:
* Simplied the commit log.
* goto err; is not required as it has only one more return path and there is no cleanup required apart from just return err.
* Updated the comment start from /* -> /**
* Changed the error log in case vlan strip command fails.

---
 drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +12 -1
index c26b036..d3dbcb5 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	args.out_buffer = vf->aq_resp;
 	args.out_size = I40E_AQ_BUF_SZ;
 	err = i40evf_execute_vf_cmd(dev, &args);
-	if (err)
+	if (err) {
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+		return err;
+	}
+	/**
+	 * In linux kernel driver on receiving ADD_VLAN it enables
+	 * VLAN_STRIP by default. So reconfigure the vlan_offload
+	 * as it was done by the app earlier.
+	 */
+	err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	if (err)
+		PMD_DRV_LOG(ERR, "fail to set vlan_strip "
+			"as a part of OP_ADD_VLAN");
 
 	return err;
 }
Signature · 1 line
2.9.3.windows.1
#4 lore raw

From: Souvik Dey <[email protected]>
Sent: Saturday, December 12, 2020 9:05 PM
To: Xing, Beilei <[email protected]>; Guo, Jia <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Souvik Dey <[email protected]>
Subject: [PATCH v2] net/i40e: issue with ADD VLAN from Guest

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

---
v2:
* Simplied the commit log.
* goto err; is not required as it has only one more return path and there is no cleanup required apart from just return err.
* Updated the comment start from /* -> /**
* Changed the error log in case vlan strip command fails.
---
drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-

1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +0 -0
index c26b036..d3dbcb5 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)

args.out_buffer = vf->aq_resp;
args.out_size = I40E_AQ_BUF_SZ;
err = i40evf_execute_vf_cmd(dev, &args);
- if (err)
+ if (err) {
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ return err;
+ }
+ /**
+ * In linux kernel driver on receiving ADD_VLAN it enables
+ * VLAN_STRIP by default. So reconfigure the vlan_offload
+ * as it was done by the app earlier.
+ */
+ err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to set vlan_strip "
+ "as a part of OP_ADD_VLAN");

What I mean is that “PMD_DRV_LOG(ERR, "fail to set vlan strip");” is enough, since it will make the driver log to be more simple. And also avoid a quoted string split across lines which let the second line could not hit your expectation. Other look good, if no other comment, please add my ACK in your next version, thanks.

return err;
}
--
2.9.3.windows.1

________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

#5 lore raw

Hi Guo,

Ok sure will remove the extra log and submit the version 3. I have not received any other comments , so will also add your ACK in the next version then.

--
Regards,
Souvik

From: Guo, Jia <[email protected]>
Sent: Monday, December 14, 2020 9:24 PM
To: Dey, Souvik <[email protected]>; Xing, Beilei <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]
Subject: RE: [PATCH v2] net/i40e: issue with ADD VLAN from Guest

________________________________
NOTICE: This email was received from an EXTERNAL sender
________________________________

From: Souvik Dey <[email protected]<mailto:[email protected]>>
Sent: Saturday, December 12, 2020 9:05 PM
To: Xing, Beilei <[email protected]<mailto:[email protected]>>; Guo, Jia <[email protected]<mailto:[email protected]>>; Zhang, Qi Z <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>; Souvik Dey <[email protected]<mailto:[email protected]>>
Subject: [PATCH v2] net/i40e: issue with ADD VLAN from Guest

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

---
v2:
* Simplied the commit log.
* goto err; is not required as it has only one more return path and there is no cleanup required apart from just return err.
* Updated the comment start from /* -> /**
* Changed the error log in case vlan strip command fails.
---
drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-

1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +0 -0
index c26b036..d3dbcb5 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)

args.out_buffer = vf->aq_resp;
args.out_size = I40E_AQ_BUF_SZ;
err = i40evf_execute_vf_cmd(dev, &args);
- if (err)
+ if (err) {
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ return err;
+ }
+ /**
+ * In linux kernel driver on receiving ADD_VLAN it enables
+ * VLAN_STRIP by default. So reconfigure the vlan_offload
+ * as it was done by the app earlier.
+ */
+ err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to set vlan_strip "
+ "as a part of OP_ADD_VLAN");

What I mean is that “PMD_DRV_LOG(ERR, "fail to set vlan strip");” is enough, since it will make the driver log to be more simple. And also avoid a quoted string split across lines which let the second line could not hit your expectation. Other look good, if no other comment, please add my ACK in your next version, thanks.

return err;
}
--
2.9.3.windows.1

________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

#6 lore raw

Hi Guo,
Raised the v3 of the patch , but was not sure how to put the ACK. If you don’t mind can you please ACK the same. Thanks.
https://patchwork.dpdk.org/patch/85210/

--
Regards,
Souvik

From: Dey, Souvik
Sent: Tuesday, December 15, 2020 8:16 AM
To: Guo, Jia <[email protected]>; Xing, Beilei <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]
Subject: RE: [PATCH v2] net/i40e: issue with ADD VLAN from Guest

Hi Guo,

Ok sure will remove the extra log and submit the version 3. I have not received any other comments , so will also add your ACK in the next version then.

--
Regards,
Souvik

From: Guo, Jia <[email protected]<mailto:[email protected]>>
Sent: Monday, December 14, 2020 9:24 PM
To: Dey, Souvik <[email protected]<mailto:[email protected]>>; Xing, Beilei <[email protected]<mailto:[email protected]>>; Zhang, Qi Z <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>
Subject: RE: [PATCH v2] net/i40e: issue with ADD VLAN from Guest

________________________________
NOTICE: This email was received from an EXTERNAL sender
________________________________

From: Souvik Dey <[email protected]<mailto:[email protected]>>
Sent: Saturday, December 12, 2020 9:05 PM
To: Xing, Beilei <[email protected]<mailto:[email protected]>>; Guo, Jia <[email protected]<mailto:[email protected]>>; Zhang, Qi Z <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>; Souvik Dey <[email protected]<mailto:[email protected]>>
Subject: [PATCH v2] net/i40e: issue with ADD VLAN from Guest

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

---
v2:
* Simplied the commit log.
* goto err; is not required as it has only one more return path and there is no cleanup required apart from just return err.
* Updated the comment start from /* -> /**
* Changed the error log in case vlan strip command fails.
---
drivers/net/i40e/i40e_ethdev_vf.c | 13 ++++++++++++-

1 file changed, 12 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +0 -0
index c26b036..d3dbcb5 100644
@@ -1078,8 +1078,19 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)

args.out_buffer = vf->aq_resp;
args.out_size = I40E_AQ_BUF_SZ;
err = i40evf_execute_vf_cmd(dev, &args);
- if (err)
+ if (err) {
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ return err;
+ }
+ /**
+ * In linux kernel driver on receiving ADD_VLAN it enables
+ * VLAN_STRIP by default. So reconfigure the vlan_offload
+ * as it was done by the app earlier.
+ */
+ err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to set vlan_strip "
+ "as a part of OP_ADD_VLAN");

What I mean is that “PMD_DRV_LOG(ERR, "fail to set vlan strip");” is enough, since it will make the driver log to be more simple. And also avoid a quoted string split across lines which let the second line could not hit your expectation. Other look good, if no other comment, please add my ACK in your next version, thanks.

return err;
}
--
2.9.3.windows.1

________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

author
#7 lore raw

PATCH v3 1/1 net/i40e: issue with ADD VLAN from Guest

In reply to Souvik Dey

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

---
v3:
* Changed the error log in case vlan strip command fails as per the latest comment.
---
v2:
* Simplied the commit log.
* goto err; is not required as it has only one more return path and there is no cleanup required apart from just return err.
* Updated the comment start from /* -> /**
* Changed the error log in case vlan strip command fails.
---

 drivers/net/i40e/i40e_ethdev_vf.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +11 -1
index c26b036..2faee1d 100644
@@ -1078,8 +1078,18 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	args.out_buffer = vf->aq_resp;
 	args.out_size = I40E_AQ_BUF_SZ;
 	err = i40evf_execute_vf_cmd(dev, &args);
-	if (err)
+	if (err) {
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+		return err;
+	}
+	/**
+	 * In linux kernel driver on receiving ADD_VLAN it enables
+	 * VLAN_STRIP by default. So reconfigure the vlan_offload
+	 * as it was done by the app earlier.
+	 */
+	err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	if (err)
+		PMD_DRV_LOG(ERR, "fail to set vlan_strip");
 
 	return err;
 }
Signature · 1 line
2.9.3.windows.1
Acked-by Jeff Guo on v3 1/1
#8 lore raw

From: Souvik Dey <[email protected]>
Sent: Tuesday, December 15, 2020 9:28 PM
To: Xing, Beilei <[email protected]>; Guo, Jia <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Souvik Dey <[email protected]>
Subject: [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

---
v3:
* Changed the error log in case vlan strip command fails as per the latest comment.
---
v2:
* Simplied the commit log.
* goto err; is not required as it has only one more return path and there is no cleanup required apart from just return err.
* Updated the comment start from /* -> /**
* Changed the error log in case vlan strip command fails.
---

drivers/net/i40e/i40e_ethdev_vf.c | 12 +++++++++++-

1 file changed, 11 insertions(+), 1 deletion(-)
drivers/net/i40e/i40e_ethdev_vf.c +0 -0
index c26b036..2faee1d 100644
@@ -1078,8 +1078,18 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)

args.out_buffer = vf->aq_resp;
args.out_size = I40E_AQ_BUF_SZ;
err = i40evf_execute_vf_cmd(dev, &args);
- if (err)
+ if (err) {
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ return err;
+ }
+ /**
+ * In linux kernel driver on receiving ADD_VLAN it enables
+ * VLAN_STRIP by default. So reconfigure the vlan_offload
+ * as it was done by the app earlier.
+ */
+ err = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to set vlan_strip");

return err;
}
--
2.9.3.windows.1

________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

Acked-by Jeff Guo Applied
#9 lore raw

From: Guo, Jia <[email protected]>
Sent: Wednesday, December 16, 2020 10:10 AM
To: Souvik Dey <[email protected]>; Xing, Beilei <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]
Subject: RE: [PATCH v3] net/i40e: issue with ADD VLAN from Guest

From: Souvik Dey <[email protected]<mailto:[email protected]>>
Sent: Tuesday, December 15, 2020 9:28 PM
To: Xing, Beilei <[email protected]<mailto:[email protected]>>; Guo, Jia <[email protected]<mailto:[email protected]>>; Zhang, Qi Z <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>; Souvik Dey <[email protected]<mailto:[email protected]>>
Subject: [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

Applied to dpdk-next-net-intel

Thanks
Qi
________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________

#10 lore raw
In reply to Guo, Jia
Quoted 8 lines · Jia

From: Souvik Dey <[email protected]>
Sent: Tuesday, December 15, 2020 9:28 PM
To: Xing, Beilei <[email protected]>; Guo, Jia <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Souvik Dey <[email protected]>
Subject: [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Quoted 8 lines

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf. Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

I suggest title: "net/i40e: fix VLAN stripping in VF"
Will update the title and some wording in the commit log while merging.

And I assume this should be backported, so will add [email protected] tag, but can you please confirm the Linux PF behavior was always same?

And if the Linux PF behavior was always same, should we add the DPDK commit as fixes commit, @Qi, @Jeff, what do you think?

Other question is, does Linux PF and DPDK PF behave differently on enabling VLAN stripping and should it be unified to be consistent?

#11 lore raw
Quoted 33 lines

-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Tuesday, January 5, 2021 12:43 AM
To: Guo, Jia <[email protected]>; Souvik Dey <[email protected]>; Xing, Beilei <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Kevin Traynor <[email protected]>; Luca Boccassi <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Quoted 9 lines · Jia

From: Souvik Dey <[email protected]>
Sent: Tuesday, December 15, 2020 9:28 PM
To: Xing, Beilei <[email protected]>; Guo, Jia <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Souvik Dey <[email protected]>
Subject: [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Quoted 8 lines

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf.
Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

I suggest title: "net/i40e: fix VLAN stripping in VF"
Will update the title and some wording in the commit log while merging.

Quoted 8 lines

And I assume this should be backported, so will add [email protected] tag, but can you please confirm the Linux PF behavior was always same?

And if the Linux PF behavior was always same, should we add the DPDK commit as fixes commit, @Qi, @Jeff, what do you think?

Other question is, does Linux PF and DPDK PF behave differently on enabling VLAN stripping and should it be unified to be consistent?

Yes, it does have different behave when enabling VLAN stripping in DPDK PF and Linux PF, and not always the same in linux PFs, but this patch set could be an workaround in DPDK PF for the compatibility.

#12 lore raw
Quoted 36 lines · Jia
Quoted 36 lines

-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Tuesday, January 5, 2021 12:43 AM
To: Guo, Jia <[email protected]>; Souvik Dey <[email protected]>; Xing, Beilei <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Kevin Traynor <[email protected]>; Luca Boccassi <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Quoted 9 lines · Jia

From: Souvik Dey <[email protected]>
Sent: Tuesday, December 15, 2020 9:28 PM
To: Xing, Beilei <[email protected]>; Guo, Jia <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Souvik Dey <[email protected]>
Subject: [PATCH v3] net/i40e: issue with ADD VLAN from Guest

Quoted 8 lines

Reset the configuration of vlan strip that would be change by the pf kernel driver when adding vlan from vf.
Application cannot use rte_eth_dev_set_vlan_offload() to set the VLAN_STRIP, as this will only work for the first time when original and current config mismatch, but for all subsequent call it will be ignored.

I suggest title: "net/i40e: fix VLAN stripping in VF"
Will update the title and some wording in the commit log while merging.

And I assume this should be backported, so will add [email protected] tag, but can you please confirm the Linux PF behavior was always same?

Quoted 8 lines
Quoted 5 lines

And if the Linux PF behavior was always same, should we add the DPDK commit as fixes commit, @Qi, @Jeff, what do you think?

Other question is, does Linux PF and DPDK PF behave differently on enabling VLAN stripping and should it be unified to be consistent?

Yes, it does have different behave when enabling VLAN stripping in DPDK PF and Linux PF, and not always the same in linux PFs, but this patch set could be an workaround in DPDK PF for the compatibility.

OK, and since the patch is safe from Linux PF point of view, I will add following fixline, which is the initial DPDK code that adds VF VLAN add support:

Fixes 4861cde46116 ("i40e: new poll mode driver")

--
Thanks,
ferruh