* Re: [PATCH] net/nfp: fix the issue about set the vlan strip offload off
2022-04-19 8:38 [PATCH] net/nfp: fix the issue about set the vlan strip offload off Peng Zhang
@ 2022-05-20 15:20 ` Ferruh Yigit
2022-05-23 21:16 ` Niklas Söderlund
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2022-05-20 15:20 UTC (permalink / raw)
To: Peng Zhang, dev, niklas.soderlund; +Cc: stable, Yong Xu, Chaoyong He
On 4/19/2022 9:38 AM, Peng Zhang wrote:
> When set the vlan_strip_offload off, it doesn't work.
> Because we can't update the msg in the nic.
>
> This patch will fix this error.
>
> Fixes: d4a27a3b092a ("nfp: add basic features")
> Cc: stable@dpdk.org
>
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Signed-off-by: Yong Xu <yong.xu@corigine.com>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Hi Niklas, Reminder of this patch waiting for your review.
> ---
> drivers/net/nfp/nfp_common.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index b26770dbfb..ab4cfda389 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -968,19 +968,18 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> int ret;
>
> hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> - new_ctrl = 0;
> + new_ctrl = hw->ctrl;
>
> - /* Enable vlan strip if it is not configured yet */
> - if ((mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
> - !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> - new_ctrl = hw->ctrl | NFP_NET_CFG_CTRL_RXVLAN;
> -
> - /* Disable vlan strip just if it is configured */
> - if (!(mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
> - (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> - new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
> + if (mask & RTE_ETH_VLAN_STRIP_OFFLOAD) {
> + /* Disable vlan strip just if it is configured */
> + if (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)
> + new_ctrl = new_ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
> + /* Enable vlan strip if it is not configured yet */
> + else
> + new_ctrl = new_ctrl | NFP_NET_CFG_CTRL_RXVLAN;
> + }
>
> - if (new_ctrl == 0)
> + if (new_ctrl == hw->ctrl)
> return 0;
>
> update = NFP_NET_CFG_UPDATE_GEN;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/nfp: fix the issue about set the vlan strip offload off
2022-04-19 8:38 [PATCH] net/nfp: fix the issue about set the vlan strip offload off Peng Zhang
2022-05-20 15:20 ` Ferruh Yigit
@ 2022-05-23 21:16 ` Niklas Söderlund
2022-05-24 12:57 ` Ferruh Yigit
2022-05-26 14:34 ` [PATCH v2] net/nfp: fix bug of disable VLAN strip Peng Zhang
3 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2022-05-23 21:16 UTC (permalink / raw)
To: Peng Zhang; +Cc: dev, stable, Yong Xu, Chaoyong He
Hi Peng,
Thanks for your work.
On 2022-04-19 10:38:57 +0200, Peng Zhang wrote:
> When set the vlan_strip_offload off, it doesn't work.
> Because we can't update the msg in the nic.
>
> This patch will fix this error.
>
> Fixes: d4a27a3b092a ("nfp: add basic features")
> Cc: stable@dpdk.org
>
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Signed-off-by: Yong Xu <yong.xu@corigine.com>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> ---
> drivers/net/nfp/nfp_common.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index b26770dbfb..ab4cfda389 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -968,19 +968,18 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> int ret;
>
> hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> - new_ctrl = 0;
> + new_ctrl = hw->ctrl;
>
> - /* Enable vlan strip if it is not configured yet */
> - if ((mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
> - !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> - new_ctrl = hw->ctrl | NFP_NET_CFG_CTRL_RXVLAN;
> -
> - /* Disable vlan strip just if it is configured */
> - if (!(mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
> - (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> - new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
> + if (mask & RTE_ETH_VLAN_STRIP_OFFLOAD) {
> + /* Disable vlan strip just if it is configured */
> + if (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)
> + new_ctrl = new_ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
> + /* Enable vlan strip if it is not configured yet */
> + else
> + new_ctrl = new_ctrl | NFP_NET_CFG_CTRL_RXVLAN;
> + }
>
> - if (new_ctrl == 0)
> + if (new_ctrl == hw->ctrl)
> return 0;
>
> update = NFP_NET_CFG_UPDATE_GEN;
> --
> 2.27.0
>
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/nfp: fix the issue about set the vlan strip offload off
2022-04-19 8:38 [PATCH] net/nfp: fix the issue about set the vlan strip offload off Peng Zhang
2022-05-20 15:20 ` Ferruh Yigit
2022-05-23 21:16 ` Niklas Söderlund
@ 2022-05-24 12:57 ` Ferruh Yigit
2022-05-26 14:34 ` [PATCH v2] net/nfp: fix bug of disable VLAN strip Peng Zhang
3 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2022-05-24 12:57 UTC (permalink / raw)
To: Peng Zhang, Niklas Söderlund
Cc: niklas.soderlund, stable, Yong Xu, Chaoyong He, dev
On 4/19/2022 9:38 AM, Peng Zhang wrote:
> When set the vlan_strip_offload off, it doesn't work.
> Because we can't update the msg in the nic.
>
> This patch will fix this error.
>
> Fixes: d4a27a3b092a ("nfp: add basic features")
> Cc: stable@dpdk.org
>
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Signed-off-by: Yong Xu <yong.xu@corigine.com>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
> drivers/net/nfp/nfp_common.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index b26770dbfb..ab4cfda389 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -968,19 +968,18 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> int ret;
>
> hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> - new_ctrl = 0;
> + new_ctrl = hw->ctrl;
>
> - /* Enable vlan strip if it is not configured yet */
> - if ((mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
> - !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> - new_ctrl = hw->ctrl | NFP_NET_CFG_CTRL_RXVLAN;
> -
> - /* Disable vlan strip just if it is configured */
> - if (!(mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
> - (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> - new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
> + if (mask & RTE_ETH_VLAN_STRIP_OFFLOAD) {
> + /* Disable vlan strip just if it is configured */
> + if (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)
> + new_ctrl = new_ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
> + /* Enable vlan strip if it is not configured yet */
> + else
> + new_ctrl = new_ctrl | NFP_NET_CFG_CTRL_RXVLAN;
> + }
Hi Peng,
I think both old and new implementations are not correct, although new
one is on the right track to fix.
First 'mask' is filled with ethdev layer and can get one of below values:
RTE_ETH_VLAN_STRIP_MASK
RTE_ETH_VLAN_FILTER_MASK
RTE_ETH_VLAN_EXTEND_MASK
RTE_ETH_QINQ_STRIP_MASK
So PMDs should check 'mask' against those values.
'RTE_ETH_VLAN_*_OFFLOAD' are the values passed by application to the
ethdev layer. (Why ethdev layer converts them to different set of
macros, I don't know :/ )
And PMD 'vlan_offload_set()' dev_ops having one of 'RTE_ETH_VLAN_*_MASK'
macro means that config is changed, this doesn't say yet if it is
enabled or disabled.
'dev->data->dev_conf.rxmode' needs to be checked to decide to enable or
disable a config.
Please check 'rte_eth_dev_set_vlan_offload()' API implementation for
details.
>
> - if (new_ctrl == 0)
> + if (new_ctrl == hw->ctrl)
> return 0;
>
> update = NFP_NET_CFG_UPDATE_GEN;
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] net/nfp: fix bug of disable VLAN strip
2022-04-19 8:38 [PATCH] net/nfp: fix the issue about set the vlan strip offload off Peng Zhang
` (2 preceding siblings ...)
2022-05-24 12:57 ` Ferruh Yigit
@ 2022-05-26 14:34 ` Peng Zhang
2022-05-31 13:27 ` Ferruh Yigit
3 siblings, 1 reply; 6+ messages in thread
From: Peng Zhang @ 2022-05-26 14:34 UTC (permalink / raw)
To: dev
Cc: niklas.soderlund, oss-drivers, Peng Zhang, stable, Yong Xu, Chaoyong He
"vlan set strip off 0" can't work, due to the incorrect usage
of the mask, it just represent that the status is changed or not,
not represent offloaded or not, so that the driver send the
error control message to the nic.
Now, by first inspect the mask of things that changed, and then
change the requested state if VLAN stripping according the
requested offload status. So this change can fix this bug.
Fixes: d4a27a3b092a ("nfp: add basic features")
Cc: stable@dpdk.org
Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Signed-off-by: Yong Xu <yong.xu@corigine.com>
Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
drivers/net/nfp/nfp_common.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index b26770dbfb..57c50ec475 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -965,22 +965,25 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
{
uint32_t new_ctrl, update;
struct nfp_net_hw *hw;
+ struct rte_eth_conf *dev_conf;
int ret;
hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- new_ctrl = 0;
-
- /* Enable vlan strip if it is not configured yet */
- if ((mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
- !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
- new_ctrl = hw->ctrl | NFP_NET_CFG_CTRL_RXVLAN;
+ dev_conf = &dev->data->dev_conf;
+ new_ctrl = hw->ctrl;
- /* Disable vlan strip just if it is configured */
- if (!(mask & RTE_ETH_VLAN_STRIP_OFFLOAD) &&
- (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
- new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
+ /*
+ * Vlan stripping setting
+ * Enable or disable VLAN stripping
+ */
+ if (mask & RTE_ETH_VLAN_STRIP_MASK) {
+ if (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
+ new_ctrl |= NFP_NET_CFG_CTRL_RXVLAN;
+ else
+ new_ctrl &= ~NFP_NET_CFG_CTRL_RXVLAN;
+ }
- if (new_ctrl == 0)
+ if (new_ctrl == hw->ctrl)
return 0;
update = NFP_NET_CFG_UPDATE_GEN;
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] net/nfp: fix bug of disable VLAN strip
2022-05-26 14:34 ` [PATCH v2] net/nfp: fix bug of disable VLAN strip Peng Zhang
@ 2022-05-31 13:27 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2022-05-31 13:27 UTC (permalink / raw)
To: Peng Zhang
Cc: niklas.soderlund, oss-drivers, stable, Yong Xu, Chaoyong He, dev
On 5/26/2022 3:34 PM, Peng Zhang wrote:
> "vlan set strip off 0" can't work, due to the incorrect usage
> of the mask, it just represent that the status is changed or not,
> not represent offloaded or not, so that the driver send the
> error control message to the nic.
>
> Now, by first inspect the mask of things that changed, and then
> change the requested state if VLAN stripping according the
> requested offload status. So this change can fix this bug.
>
> Fixes: d4a27a3b092a ("nfp: add basic features")
> Cc:stable@dpdk.org
>
> Signed-off-by: Peng Zhang<peng.zhang@corigine.com>
> Signed-off-by: Yong Xu<yong.xu@corigine.com>
> Signed-off-by: Chaoyong He<chaoyong.he@corigine.com>
> Signed-off-by: Niklas Söderlund<niklas.soderlund@corigine.com>
Applied to dpdk-next-net/main, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread