[PATCH] i40e: fix vlan filtering

[PATCH] i40e: fix vlan filtering

[Julien Meunier]

VLAN filtering was always performed, even if hw_vlan_filter was
disabled. During device initialization, default filter
RTE_MACVLAN_PERFECT_MATCH was applied. In this situation, all incoming
VLAN frames were dropped by the card (increase of the register RUPP - Rx
Unsupported Protocol).

In order to restore default behavior, if HW VLAN filtering is activated,
set a filter to match MAC and VLAN. If not, set a filter to only match
MAC.

Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 drivers/net/i40e/i40e_ethdev.c | 39 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |  1 +
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index bf6220d..ef9d578 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *vsi = pf->main_vsi;
 
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+			i40e_vsi_config_vlan_filter(vsi, TRUE);
+		else
+			i40e_vsi_config_vlan_filter(vsi, FALSE);
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
@@ -4156,6 +4163,34 @@ fail_mem:
 	return NULL;
 }
 
+/* Configure vlan filter on or off */
+int
+i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
+{
+	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
+	struct i40e_mac_filter_info filter;
+	int ret;
+
+	rte_memcpy(&filter.mac_addr,
+		   (struct ether_addr *)(hw->mac.perm_addr), ETH_ADDR_LEN);
+	ret = i40e_vsi_delete_mac(vsi, &filter.mac_addr);
+
+	if (on) {
+		/* Filter to match MAC and VLAN */
+		filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	} else {
+		/* Filter to match only MAC */
+		filter.filter_type = RTE_MAC_PERFECT_MATCH;
+	}
+
+	ret |= i40e_vsi_add_mac(vsi, &filter);
+
+	if (ret)
+		PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
+			    on ? "enable" : "disable");
+	return ret;
+}
+
 /* Configure vlan stripping on or off */
 int
 i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on)
@@ -4203,9 +4238,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)
 {
 	struct rte_eth_dev_data *data = dev->data;
 	int ret;
+	int mask = 0;
 
 	/* Apply vlan offload setting */
-	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
+	i40e_vlan_offload_set(dev, mask);
 
 	/* Apply double-vlan setting, not implemented yet */
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 1f9792b..5505d72 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
+int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
 uint64_t i40e_config_hena(uint64_t flags);
 uint64_t i40e_parse_hena(uint64_t flags);
 enum i40e_status_code i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
-- 
2.1.4

Re: [PATCH] i40e: fix vlan filtering

[Zhang, Helin]

Hi Julien

Nice of seeing your patch for this issue, which has been in our issue list.
I will ask a validation here for your patch. My comments inlined.
Thank you very much!

Regards,
Helin

-----Original Message-----
From: Julien Meunier [mailto:julien.meunier@6wind.com] 
Sent: Tuesday, January 19, 2016 1:19 AM
To: Zhang, Helin <helin.zhang@intel.com>
Cc: dev@dpdk.org
Subject: [PATCH] i40e: fix vlan filtering

VLAN filtering was always performed, even if hw_vlan_filter was disabled. During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH was applied. In this situation, all incoming VLAN frames were dropped by the card (increase of the register RUPP - Rx Unsupported Protocol).

In order to restore default behavior, if HW VLAN filtering is activated, set a filter to match MAC and VLAN. If not, set a filter to only match MAC.

Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 drivers/net/i40e/i40e_ethdev.c | 39 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |  1 +
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index bf6220d..ef9d578 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *vsi = pf->main_vsi;
 
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+			i40e_vsi_config_vlan_filter(vsi, TRUE);
+		else
+			i40e_vsi_config_vlan_filter(vsi, FALSE);
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
@@ -4156,6 +4163,34 @@ fail_mem:
 	return NULL;
 }
 
+/* Configure vlan filter on or off */
+int
+i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
+	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
+	struct i40e_mac_filter_info filter;
+	int ret;
+
+	rte_memcpy(&filter.mac_addr,
+		   (struct ether_addr *)(hw->mac.perm_addr), ETH_ADDR_LEN);
It would be better to use ether_addr_copy() for mac copy.

+	ret = i40e_vsi_delete_mac(vsi, &filter.mac_addr);
It would be better to add debug messages here for failure case.

+
+	if (on) {
+		/* Filter to match MAC and VLAN */
+		filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	} else {
+		/* Filter to match only MAC */
+		filter.filter_type = RTE_MAC_PERFECT_MATCH;
+	}
+
+	ret |= i40e_vsi_add_mac(vsi, &filter);
+
+	if (ret)
+		PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
+			    on ? "enable" : "disable");
+	return ret;
+}
+
 /* Configure vlan stripping on or off */  int  i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@ -4203,9 +4238,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
 	struct rte_eth_dev_data *data = dev->data;
 	int ret;
+	int mask = 0;
 
 	/* Apply vlan offload setting */
-	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
+	i40e_vlan_offload_set(dev, mask);
 
 	/* Apply double-vlan setting, not implemented yet */
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h index 1f9792b..5505d72 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);  int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);  int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
+int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
 uint64_t i40e_config_hena(uint64_t flags);  uint64_t i40e_parse_hena(uint64_t flags);  enum i40e_status_code i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
--
2.1.4

Re: [PATCH] i40e: fix vlan filtering

[Pei, Yulong]

It works as expected, thanks.

Tested-by Yulong.pei@intel.com

Best Regards
Yulong Pei

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
Sent: Tuesday, January 19, 2016 1:19 AM
To: Zhang, Helin <helin.zhang@intel.com>
Cc: dev@dpdk.org
Subject: [dpdk-dev] [PATCH] i40e: fix vlan filtering

VLAN filtering was always performed, even if hw_vlan_filter was disabled. During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH was applied. In this situation, all incoming VLAN frames were dropped by the card (increase of the register RUPP - Rx Unsupported Protocol).

In order to restore default behavior, if HW VLAN filtering is activated, set a filter to match MAC and VLAN. If not, set a filter to only match MAC.

Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 drivers/net/i40e/i40e_ethdev.c | 39 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |  1 +
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index bf6220d..ef9d578 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *vsi = pf->main_vsi;
 
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+			i40e_vsi_config_vlan_filter(vsi, TRUE);
+		else
+			i40e_vsi_config_vlan_filter(vsi, FALSE);
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
@@ -4156,6 +4163,34 @@ fail_mem:
 	return NULL;
 }
 
+/* Configure vlan filter on or off */
+int
+i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
+	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
+	struct i40e_mac_filter_info filter;
+	int ret;
+
+	rte_memcpy(&filter.mac_addr,
+		   (struct ether_addr *)(hw->mac.perm_addr), ETH_ADDR_LEN);
+	ret = i40e_vsi_delete_mac(vsi, &filter.mac_addr);
+
+	if (on) {
+		/* Filter to match MAC and VLAN */
+		filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	} else {
+		/* Filter to match only MAC */
+		filter.filter_type = RTE_MAC_PERFECT_MATCH;
+	}
+
+	ret |= i40e_vsi_add_mac(vsi, &filter);
+
+	if (ret)
+		PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
+			    on ? "enable" : "disable");
+	return ret;
+}
+
 /* Configure vlan stripping on or off */  int  i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@ -4203,9 +4238,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
 	struct rte_eth_dev_data *data = dev->data;
 	int ret;
+	int mask = 0;
 
 	/* Apply vlan offload setting */
-	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
+	i40e_vlan_offload_set(dev, mask);
 
 	/* Apply double-vlan setting, not implemented yet */
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h index 1f9792b..5505d72 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);  int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);  int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
+int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
 uint64_t i40e_config_hena(uint64_t flags);  uint64_t i40e_parse_hena(uint64_t flags);  enum i40e_status_code i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
--
2.1.4

Re: [PATCH] i40e: fix vlan filtering

[Zhang, Helin]

> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@6wind.com]
> Sent: Tuesday, January 19, 2016 1:19 AM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: [PATCH] i40e: fix vlan filtering
> 
> VLAN filtering was always performed, even if hw_vlan_filter was disabled.
> During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH
> was applied. In this situation, all incoming VLAN frames were dropped by the
> card (increase of the register RUPP - Rx Unsupported Protocol).
> 
> In order to restore default behavior, if HW VLAN filtering is activated, set a
> filter to match MAC and VLAN. If not, set a filter to only match MAC.
> 
> Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 39
> ++++++++++++++++++++++++++++++++++++++-
>  drivers/net/i40e/i40e_ethdev.h |  1 +
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index bf6220d..ef9d578 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> int mask)
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> >dev_private);
>  	struct i40e_vsi *vsi = pf->main_vsi;
> 
> +	if (mask & ETH_VLAN_FILTER_MASK) {
> +		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
> +			i40e_vsi_config_vlan_filter(vsi, TRUE);
> +		else
> +			i40e_vsi_config_vlan_filter(vsi, FALSE);
> +	}
> +
>  	if (mask & ETH_VLAN_STRIP_MASK) {
>  		/* Enable or disable VLAN stripping */
>  		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
> @@ -4156,6 +4163,34 @@ fail_mem:
>  	return NULL;
>  }
> 
> +/* Configure vlan filter on or off */
> +int
> +i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
> +	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
> +	struct i40e_mac_filter_info filter;
> +	int ret;
> +
> +	rte_memcpy(&filter.mac_addr,
> +		   (struct ether_addr *)(hw->mac.perm_addr),
> ETH_ADDR_LEN);
> +	ret = i40e_vsi_delete_mac(vsi, &filter.mac_addr);
> +
> +	if (on) {
> +		/* Filter to match MAC and VLAN */
> +		filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> +	} else {
> +		/* Filter to match only MAC */
> +		filter.filter_type = RTE_MAC_PERFECT_MATCH;
> +	}
> +
> +	ret |= i40e_vsi_add_mac(vsi, &filter);
How would it be if multiple mac addresses has been configured?
I think this might be ignored in the code changes, right?

Regards,
Helin

> +
> +	if (ret)
> +		PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
> +			    on ? "enable" : "disable");
> +	return ret;
> +}
> +
>  /* Configure vlan stripping on or off */  int
> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@ -4203,9
> +4238,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
>  	struct rte_eth_dev_data *data = dev->data;
>  	int ret;
> +	int mask = 0;
> 
>  	/* Apply vlan offload setting */
> -	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
> +	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
> +	i40e_vlan_offload_set(dev, mask);
> 
>  	/* Apply double-vlan setting, not implemented yet */
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.h
> b/drivers/net/i40e/i40e_ethdev.h index 1f9792b..5505d72 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi
> *vsi);  int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
>  			   struct i40e_vsi_vlan_pvid_info *info);  int
> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
> +int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
>  uint64_t i40e_config_hena(uint64_t flags);  uint64_t
> i40e_parse_hena(uint64_t flags);  enum i40e_status_code
> i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
> --
> 2.1.4

Re: [PATCH] i40e: fix vlan filtering

[Julien Meunier]

Hello,

Yes, you are right. Even if VLAN filtering is configured most of the 
time during initialization, we should managed the case of multiple MAC 
addresses already configured.

I will send you a v2 patch with this modification, use ether_addr_copy 
and add additional debug messages.

Regards,

On 01/20/2016 06:00 AM, Zhang, Helin wrote:
>> -----Original Message-----
>> From: Julien Meunier [mailto:julien.meunier@6wind.com]
>> Sent: Tuesday, January 19, 2016 1:19 AM
>> To: Zhang, Helin
>> Cc:dev@dpdk.org
>> Subject: [PATCH] i40e: fix vlan filtering
>>
>> VLAN filtering was always performed, even if hw_vlan_filter was disabled.
>> During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH
>> was applied. In this situation, all incoming VLAN frames were dropped by the
>> card (increase of the register RUPP - Rx Unsupported Protocol).
>>
>> In order to restore default behavior, if HW VLAN filtering is activated, set a
>> filter to match MAC and VLAN. If not, set a filter to only match MAC.
>>
>> Signed-off-by: Julien Meunier<julien.meunier@6wind.com>
>> Signed-off-by: David Marchand<david.marchand@6wind.com>
>> ---
>>   drivers/net/i40e/i40e_ethdev.c | 39
>> ++++++++++++++++++++++++++++++++++++++-
>>   drivers/net/i40e/i40e_ethdev.h |  1 +
>>   2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index bf6220d..ef9d578 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
>> int mask)
>>   	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
>>> dev_private);
>>   	struct i40e_vsi *vsi = pf->main_vsi;
>>
>> +	if (mask & ETH_VLAN_FILTER_MASK) {
>> +		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
>> +			i40e_vsi_config_vlan_filter(vsi, TRUE);
>> +		else
>> +			i40e_vsi_config_vlan_filter(vsi, FALSE);
>> +	}
>> +
>>   	if (mask & ETH_VLAN_STRIP_MASK) {
>>   		/* Enable or disable VLAN stripping */
>>   		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
>> @@ -4156,6 +4163,34 @@ fail_mem:
>>   	return NULL;
>>   }
>>
>> +/* Configure vlan filter on or off */
>> +int
>> +i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
>> +	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
>> +	struct i40e_mac_filter_info filter;
>> +	int ret;
>> +
>> +	rte_memcpy(&filter.mac_addr,
>> +		   (struct ether_addr *)(hw->mac.perm_addr),
>> ETH_ADDR_LEN);
>> +	ret = i40e_vsi_delete_mac(vsi, &filter.mac_addr);
>> +
>> +	if (on) {
>> +		/* Filter to match MAC and VLAN */
>> +		filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
>> +	} else {
>> +		/* Filter to match only MAC */
>> +		filter.filter_type = RTE_MAC_PERFECT_MATCH;
>> +	}
>> +
>> +	ret |= i40e_vsi_add_mac(vsi, &filter);
> How would it be if multiple mac addresses has been configured?
> I think this might be ignored in the code changes, right?
>
> Regards,
> Helin
>
>> +
>> +	if (ret)
>> +		PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
>> +			    on ? "enable" : "disable");
>> +	return ret;
>> +}
>> +
>>   /* Configure vlan stripping on or off */  int
>> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@ -4203,9
>> +4238,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
>>   	struct rte_eth_dev_data *data = dev->data;
>>   	int ret;
>> +	int mask = 0;
>>
>>   	/* Apply vlan offload setting */
>> -	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
>> +	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
>> +	i40e_vlan_offload_set(dev, mask);
>>
>>   	/* Apply double-vlan setting, not implemented yet */
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.h
>> b/drivers/net/i40e/i40e_ethdev.h index 1f9792b..5505d72 100644
>> --- a/drivers/net/i40e/i40e_ethdev.h
>> +++ b/drivers/net/i40e/i40e_ethdev.h
>> @@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi
>> *vsi);  int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
>>   			   struct i40e_vsi_vlan_pvid_info *info);  int
>> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
>> +int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
>>   uint64_t i40e_config_hena(uint64_t flags);  uint64_t
>> i40e_parse_hena(uint64_t flags);  enum i40e_status_code
>> i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
>> --
>> 2.1.4

-- 
Julien MEUNIER
6WIND

[PATCH v2] i40e: fix vlan filtering

[Julien Meunier]

VLAN filtering was always performed, even if hw_vlan_filter was
disabled. During device initialization, default filter
RTE_MACVLAN_PERFECT_MATCH was applied. In this situation, all incoming
VLAN frames were dropped by the card (increase of the register RUPP - Rx
Unsupported Protocol).

In order to restore default behavior, if HW VLAN filtering is activated,
set a filter to match MAC and VLAN. If not, set a filter to only match
MAC.

Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
---
Changes since v1:
- use ether_addr_copy() for mac copy
- add more debug messages in case of failure
- update all existing filters when multiple mac addresses have been configured
- when adding new mac address, use correct filter

TODO:
- i40e_update_default_filter_setting always forces to
  RTE_MACVLAN_PERFECT_MATCH.
  => The type of filter should be changed according to vlan filter setting.

- What happens if vlan filter setting changes when various filters are already
  set like RTE_MACVLAN_PERFECT_MATCH, RTE_MACVLAN_PERFECT_MATCH,
  RTE_MAC_HASH_MATCH, RTE_MACVLAN_HASH_MATCH ?
  => With testpmd, it is possible to add manually these filters. But when
  changing vlan filter setting, all previous filter set manually are overriden.
---
drivers/net/i40e/i40e_ethdev.c | 73 ++++++++++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  1 +
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index bf6220d..64d6ada 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *vsi = pf->main_vsi;
 
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+			i40e_vsi_config_vlan_filter(vsi, TRUE);
+		else
+			i40e_vsi_config_vlan_filter(vsi, FALSE);
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
@@ -2583,7 +2590,10 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 	}
 
 	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
-	mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+		mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	else
+		mac_filter.filter_type = RTE_MAC_PERFECT_MATCH;
 
 	if (pool == 0)
 		vsi = pf->main_vsi;
@@ -4156,6 +4166,63 @@ fail_mem:
 	return NULL;
 }
 
+/* Configure vlan filter on or off */
+int
+i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
+{
+	int i, num;
+	struct i40e_mac_filter *f;
+	struct i40e_mac_filter_info *mac_filter;
+	enum rte_mac_filter_type desired_filter;
+	int ret = I40E_SUCCESS;
+
+	if (on) {
+		/* Filter to match MAC and VLAN */
+		desired_filter = RTE_MACVLAN_PERFECT_MATCH;
+	} else {
+		/* Filter to match only MAC */
+		desired_filter = RTE_MAC_PERFECT_MATCH;
+	}
+
+	num = vsi->mac_num;
+
+	mac_filter = rte_zmalloc("mac_filter_info_data",
+				 num * sizeof(*mac_filter), 0);
+	if (mac_filter == NULL) {
+		PMD_DRV_LOG(ERR, "failed to allocate memory");
+		return I40E_ERR_NO_MEMORY;
+	}
+
+	i = 0;
+
+	/* Remove all existing mac */
+	TAILQ_FOREACH(f, &vsi->mac_list, next) {
+		mac_filter[i] = f->mac_info;
+		ret = i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
+		if (ret) {
+			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
+				    on ? "enable" : "disable");
+			goto DONE;
+		}
+		i++;
+	}
+
+	/* Override with new filter */
+	for (i = 0; i < num; i++) {
+		mac_filter[i].filter_type = desired_filter;
+		ret = i40e_vsi_add_mac(vsi, &mac_filter[i]);
+		if (ret) {
+			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
+				    on ? "enable" : "disable");
+			goto DONE;
+		}
+	}
+
+DONE:
+	rte_free(mac_filter);
+	return ret;
+}
+
 /* Configure vlan stripping on or off */
 int
 i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on)
@@ -4203,9 +4270,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)
 {
 	struct rte_eth_dev_data *data = dev->data;
 	int ret;
+	int mask = 0;
 
 	/* Apply vlan offload setting */
-	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
+	i40e_vlan_offload_set(dev, mask);
 
 	/* Apply double-vlan setting, not implemented yet */
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 1f9792b..5505d72 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
+int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
 uint64_t i40e_config_hena(uint64_t flags);
 uint64_t i40e_parse_hena(uint64_t flags);
 enum i40e_status_code i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
-- 
2.1.4

Re: [PATCH v2] i40e: fix vlan filtering

[Zhang, Helin]

> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@6wind.com]
> Sent: Tuesday, February 2, 2016 9:51 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v2] i40e: fix vlan filtering
> 
> VLAN filtering was always performed, even if hw_vlan_filter was disabled.
> During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH
> was applied. In this situation, all incoming VLAN frames were dropped by the
> card (increase of the register RUPP - Rx Unsupported Protocol).
> 
> In order to restore default behavior, if HW VLAN filtering is activated, set a
> filter to match MAC and VLAN. If not, set a filter to only match MAC.
> 
> Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
> ---
> Changes since v1:
> - use ether_addr_copy() for mac copy
> - add more debug messages in case of failure
> - update all existing filters when multiple mac addresses have been configured
> - when adding new mac address, use correct filter
> 
> TODO:
> - i40e_update_default_filter_setting always forces to
>   RTE_MACVLAN_PERFECT_MATCH.
>   => The type of filter should be changed according to vlan filter setting.
> 
> - What happens if vlan filter setting changes when various filters are already
>   set like RTE_MACVLAN_PERFECT_MATCH,
> RTE_MACVLAN_PERFECT_MATCH,
>   RTE_MAC_HASH_MATCH, RTE_MACVLAN_HASH_MATCH ?
>   => With testpmd, it is possible to add manually these filters. But when
>   changing vlan filter setting, all previous filter set manually are overriden.
> ---
> drivers/net/i40e/i40e_ethdev.c | 73
> ++++++++++++++++++++++++++++++++++++++++--
>  drivers/net/i40e/i40e_ethdev.h |  1 +
>  2 files changed, 72 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index bf6220d..64d6ada 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
> mask)
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct i40e_vsi *vsi = pf->main_vsi;
> 
> +	if (mask & ETH_VLAN_FILTER_MASK) {
> +		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
> +			i40e_vsi_config_vlan_filter(vsi, TRUE);
> +		else
> +			i40e_vsi_config_vlan_filter(vsi, FALSE);
> +	}
> +
>  	if (mask & ETH_VLAN_STRIP_MASK) {
>  		/* Enable or disable VLAN stripping */
>  		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
> @@ -2583,7 +2590,10 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
>  	}
> 
>  	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
> -	mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> +	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
> +		mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> +	else
> +		mac_filter.filter_type = RTE_MAC_PERFECT_MATCH;
> 
>  	if (pool == 0)
>  		vsi = pf->main_vsi;
> @@ -4156,6 +4166,63 @@ fail_mem:
>  	return NULL;
>  }
> 
> +/* Configure vlan filter on or off */
> +int
> +i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
> +	int i, num;
> +	struct i40e_mac_filter *f;
> +	struct i40e_mac_filter_info *mac_filter;
> +	enum rte_mac_filter_type desired_filter;
> +	int ret = I40E_SUCCESS;
> +
> +	if (on) {
> +		/* Filter to match MAC and VLAN */
> +		desired_filter = RTE_MACVLAN_PERFECT_MATCH;
> +	} else {
> +		/* Filter to match only MAC */
> +		desired_filter = RTE_MAC_PERFECT_MATCH;
> +	}
> +
> +	num = vsi->mac_num;
> +
> +	mac_filter = rte_zmalloc("mac_filter_info_data",
> +				 num * sizeof(*mac_filter), 0);
> +	if (mac_filter == NULL) {
> +		PMD_DRV_LOG(ERR, "failed to allocate memory");
> +		return I40E_ERR_NO_MEMORY;
> +	}
> +
> +	i = 0;
> +
> +	/* Remove all existing mac */
> +	TAILQ_FOREACH(f, &vsi->mac_list, next) {
> +		mac_filter[i] = f->mac_info;
> +		ret = i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
> +		if (ret) {
> +			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
INFO should he changed to ERR?

> +				    on ? "enable" : "disable");
> +			goto DONE;
> +		}
> +		i++;
> +	}
> +
> +	/* Override with new filter */
> +	for (i = 0; i < num; i++) {
> +		mac_filter[i].filter_type = desired_filter;
> +		ret = i40e_vsi_add_mac(vsi, &mac_filter[i]);
> +		if (ret) {
> +			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
INFO should he changed to ERR?

All others looks good to me. Thanks!

Helin
> +				    on ? "enable" : "disable");
> +			goto DONE;
> +		}
> +	}
> +
> +DONE:
> +	rte_free(mac_filter);
> +	return ret;
> +}
> +
>  /* Configure vlan stripping on or off */  int
> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@ -4203,9
> +4270,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
>  	struct rte_eth_dev_data *data = dev->data;
>  	int ret;
> +	int mask = 0;
> 
>  	/* Apply vlan offload setting */
> -	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
> +	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
> +	i40e_vlan_offload_set(dev, mask);
> 
>  	/* Apply double-vlan setting, not implemented yet */
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 1f9792b..5505d72 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
> int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
>  			   struct i40e_vsi_vlan_pvid_info *info);  int
> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
> +int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
>  uint64_t i40e_config_hena(uint64_t flags);  uint64_t
> i40e_parse_hena(uint64_t flags);  enum i40e_status_code
> i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
> --
> 2.1.4

Re: [PATCH v2] i40e: fix vlan filtering

[Julien Meunier]

Hello,

INFO log level is used in order to keep code homogeneity: 
i40e_vsi_config_vlan_stripping or i40e_dev_init_vlan use this log level 
during failure for example.

Tell me if ERR log level for VLAN filtering issue must be set.

On 02/03/2016 02:15 AM, Zhang, Helin wrote:
>
>> -----Original Message-----
>> From: Julien Meunier [mailto:julien.meunier@6wind.com]
>> Sent: Tuesday, February 2, 2016 9:51 PM
>> To: Zhang, Helin <helin.zhang@intel.com>
>> Cc: dev@dpdk.org
>> Subject: [PATCH v2] i40e: fix vlan filtering
>>
>> VLAN filtering was always performed, even if hw_vlan_filter was disabled.
>> During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH
>> was applied. In this situation, all incoming VLAN frames were dropped by the
>> card (increase of the register RUPP - Rx Unsupported Protocol).
>>
>> In order to restore default behavior, if HW VLAN filtering is activated, set a
>> filter to match MAC and VLAN. If not, set a filter to only match MAC.
>>
>> Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
>> ---
>> Changes since v1:
>> - use ether_addr_copy() for mac copy
>> - add more debug messages in case of failure
>> - update all existing filters when multiple mac addresses have been configured
>> - when adding new mac address, use correct filter
>>
>> TODO:
>> - i40e_update_default_filter_setting always forces to
>>    RTE_MACVLAN_PERFECT_MATCH.
>>    => The type of filter should be changed according to vlan filter setting.
>>
>> - What happens if vlan filter setting changes when various filters are already
>>    set like RTE_MACVLAN_PERFECT_MATCH,
>> RTE_MACVLAN_PERFECT_MATCH,
>>    RTE_MAC_HASH_MATCH, RTE_MACVLAN_HASH_MATCH ?
>>    => With testpmd, it is possible to add manually these filters. But when
>>    changing vlan filter setting, all previous filter set manually are overriden.
>> ---
>> drivers/net/i40e/i40e_ethdev.c | 73
>> ++++++++++++++++++++++++++++++++++++++++--
>>   drivers/net/i40e/i40e_ethdev.h |  1 +
>>   2 files changed, 72 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index bf6220d..64d6ada 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int
>> mask)
>>   	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>>   	struct i40e_vsi *vsi = pf->main_vsi;
>>
>> +	if (mask & ETH_VLAN_FILTER_MASK) {
>> +		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
>> +			i40e_vsi_config_vlan_filter(vsi, TRUE);
>> +		else
>> +			i40e_vsi_config_vlan_filter(vsi, FALSE);
>> +	}
>> +
>>   	if (mask & ETH_VLAN_STRIP_MASK) {
>>   		/* Enable or disable VLAN stripping */
>>   		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
>> @@ -2583,7 +2590,10 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
>>   	}
>>
>>   	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
>> -	mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
>> +	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
>> +		mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
>> +	else
>> +		mac_filter.filter_type = RTE_MAC_PERFECT_MATCH;
>>
>>   	if (pool == 0)
>>   		vsi = pf->main_vsi;
>> @@ -4156,6 +4166,63 @@ fail_mem:
>>   	return NULL;
>>   }
>>
>> +/* Configure vlan filter on or off */
>> +int
>> +i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
>> +	int i, num;
>> +	struct i40e_mac_filter *f;
>> +	struct i40e_mac_filter_info *mac_filter;
>> +	enum rte_mac_filter_type desired_filter;
>> +	int ret = I40E_SUCCESS;
>> +
>> +	if (on) {
>> +		/* Filter to match MAC and VLAN */
>> +		desired_filter = RTE_MACVLAN_PERFECT_MATCH;
>> +	} else {
>> +		/* Filter to match only MAC */
>> +		desired_filter = RTE_MAC_PERFECT_MATCH;
>> +	}
>> +
>> +	num = vsi->mac_num;
>> +
>> +	mac_filter = rte_zmalloc("mac_filter_info_data",
>> +				 num * sizeof(*mac_filter), 0);
>> +	if (mac_filter == NULL) {
>> +		PMD_DRV_LOG(ERR, "failed to allocate memory");
>> +		return I40E_ERR_NO_MEMORY;
>> +	}
>> +
>> +	i = 0;
>> +
>> +	/* Remove all existing mac */
>> +	TAILQ_FOREACH(f, &vsi->mac_list, next) {
>> +		mac_filter[i] = f->mac_info;
>> +		ret = i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
>> +		if (ret) {
>> +			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
> INFO should he changed to ERR?
>
>> +				    on ? "enable" : "disable");
>> +			goto DONE;
>> +		}
>> +		i++;
>> +	}
>> +
>> +	/* Override with new filter */
>> +	for (i = 0; i < num; i++) {
>> +		mac_filter[i].filter_type = desired_filter;
>> +		ret = i40e_vsi_add_mac(vsi, &mac_filter[i]);
>> +		if (ret) {
>> +			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
> INFO should he changed to ERR?
>
> All others looks good to me. Thanks!
>
> Helin
>> +				    on ? "enable" : "disable");
>> +			goto DONE;
>> +		}
>> +	}
>> +
>> +DONE:
>> +	rte_free(mac_filter);
>> +	return ret;
>> +}
>> +
>>   /* Configure vlan stripping on or off */  int
>> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@ -4203,9
>> +4270,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
>>   	struct rte_eth_dev_data *data = dev->data;
>>   	int ret;
>> +	int mask = 0;
>>
>>   	/* Apply vlan offload setting */
>> -	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
>> +	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
>> +	i40e_vlan_offload_set(dev, mask);
>>
>>   	/* Apply double-vlan setting, not implemented yet */
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
>> index 1f9792b..5505d72 100644
>> --- a/drivers/net/i40e/i40e_ethdev.h
>> +++ b/drivers/net/i40e/i40e_ethdev.h
>> @@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
>> int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
>>   			   struct i40e_vsi_vlan_pvid_info *info);  int
>> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
>> +int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
>>   uint64_t i40e_config_hena(uint64_t flags);  uint64_t
>> i40e_parse_hena(uint64_t flags);  enum i40e_status_code
>> i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
>> --
>> 2.1.4

-- 
Julien MEUNIER
6WIND

Re: [PATCH v2] i40e: fix vlan filtering

[Zhang, Helin]

> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@6wind.com]
> Sent: Wednesday, February 3, 2016 7:32 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] i40e: fix vlan filtering
> 
> Hello,
> 
> INFO log level is used in order to keep code homogeneity:
> i40e_vsi_config_vlan_stripping or i40e_dev_init_vlan use this log level during
> failure for example.
> 
> Tell me if ERR log level for VLAN filtering issue must be set.
There is a failure, and may result in uncertain behaviors which cannot be ignored.
I'd suggest to use ERR but not INFO, though I am not so confident on that.
Could Thomas help give some guidance on that?

Regards,
Helin

> 
> On 02/03/2016 02:15 AM, Zhang, Helin wrote:
> >
> >> -----Original Message-----
> >> From: Julien Meunier [mailto:julien.meunier@6wind.com]
> >> Sent: Tuesday, February 2, 2016 9:51 PM
> >> To: Zhang, Helin <helin.zhang@intel.com>
> >> Cc: dev@dpdk.org
> >> Subject: [PATCH v2] i40e: fix vlan filtering
> >>
> >> VLAN filtering was always performed, even if hw_vlan_filter was disabled.
> >> During device initialization, default filter
> >> RTE_MACVLAN_PERFECT_MATCH was applied. In this situation, all
> >> incoming VLAN frames were dropped by the card (increase of the register
> RUPP - Rx Unsupported Protocol).
> >>
> >> In order to restore default behavior, if HW VLAN filtering is
> >> activated, set a filter to match MAC and VLAN. If not, set a filter to only
> match MAC.
> >>
> >> Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
> >> ---
> >> Changes since v1:
> >> - use ether_addr_copy() for mac copy
> >> - add more debug messages in case of failure
> >> - update all existing filters when multiple mac addresses have been
> >> configured
> >> - when adding new mac address, use correct filter
> >>
> >> TODO:
> >> - i40e_update_default_filter_setting always forces to
> >>    RTE_MACVLAN_PERFECT_MATCH.
> >>    => The type of filter should be changed according to vlan filter setting.
> >>
> >> - What happens if vlan filter setting changes when various filters are already
> >>    set like RTE_MACVLAN_PERFECT_MATCH,
> RTE_MACVLAN_PERFECT_MATCH,
> >>    RTE_MAC_HASH_MATCH, RTE_MACVLAN_HASH_MATCH ?
> >>    => With testpmd, it is possible to add manually these filters. But when
> >>    changing vlan filter setting, all previous filter set manually are
> overriden.
> >> ---
> >> drivers/net/i40e/i40e_ethdev.c | 73
> >> ++++++++++++++++++++++++++++++++++++++++--
> >>   drivers/net/i40e/i40e_ethdev.h |  1 +
> >>   2 files changed, 72 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >> b/drivers/net/i40e/i40e_ethdev.c index bf6220d..64d6ada 100644
> >> --- a/drivers/net/i40e/i40e_ethdev.c
> >> +++ b/drivers/net/i40e/i40e_ethdev.c
> >> @@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> >> int
> >> mask)
> >>   	struct i40e_pf *pf =
> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> >>   	struct i40e_vsi *vsi = pf->main_vsi;
> >>
> >> +	if (mask & ETH_VLAN_FILTER_MASK) {
> >> +		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
> >> +			i40e_vsi_config_vlan_filter(vsi, TRUE);
> >> +		else
> >> +			i40e_vsi_config_vlan_filter(vsi, FALSE);
> >> +	}
> >> +
> >>   	if (mask & ETH_VLAN_STRIP_MASK) {
> >>   		/* Enable or disable VLAN stripping */
> >>   		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
> >> @@ -2583,7 +2590,10 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
> >>   	}
> >>
> >>   	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr,
> ETHER_ADDR_LEN);
> >> -	mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> >> +	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
> >> +		mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> >> +	else
> >> +		mac_filter.filter_type = RTE_MAC_PERFECT_MATCH;
> >>
> >>   	if (pool == 0)
> >>   		vsi = pf->main_vsi;
> >> @@ -4156,6 +4166,63 @@ fail_mem:
> >>   	return NULL;
> >>   }
> >>
> >> +/* Configure vlan filter on or off */ int
> >> +i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on) {
> >> +	int i, num;
> >> +	struct i40e_mac_filter *f;
> >> +	struct i40e_mac_filter_info *mac_filter;
> >> +	enum rte_mac_filter_type desired_filter;
> >> +	int ret = I40E_SUCCESS;
> >> +
> >> +	if (on) {
> >> +		/* Filter to match MAC and VLAN */
> >> +		desired_filter = RTE_MACVLAN_PERFECT_MATCH;
> >> +	} else {
> >> +		/* Filter to match only MAC */
> >> +		desired_filter = RTE_MAC_PERFECT_MATCH;
> >> +	}
> >> +
> >> +	num = vsi->mac_num;
> >> +
> >> +	mac_filter = rte_zmalloc("mac_filter_info_data",
> >> +				 num * sizeof(*mac_filter), 0);
> >> +	if (mac_filter == NULL) {
> >> +		PMD_DRV_LOG(ERR, "failed to allocate memory");
> >> +		return I40E_ERR_NO_MEMORY;
> >> +	}
> >> +
> >> +	i = 0;
> >> +
> >> +	/* Remove all existing mac */
> >> +	TAILQ_FOREACH(f, &vsi->mac_list, next) {
> >> +		mac_filter[i] = f->mac_info;
> >> +		ret = i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
> >> +		if (ret) {
> >> +			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
> > INFO should he changed to ERR?
> >
> >> +				    on ? "enable" : "disable");
> >> +			goto DONE;
> >> +		}
> >> +		i++;
> >> +	}
> >> +
> >> +	/* Override with new filter */
> >> +	for (i = 0; i < num; i++) {
> >> +		mac_filter[i].filter_type = desired_filter;
> >> +		ret = i40e_vsi_add_mac(vsi, &mac_filter[i]);
> >> +		if (ret) {
> >> +			PMD_DRV_LOG(INFO, "Update VSI failed to %s vlan filter",
> > INFO should he changed to ERR?
> >
> > All others looks good to me. Thanks!
> >
> > Helin
> >> +				    on ? "enable" : "disable");
> >> +			goto DONE;
> >> +		}
> >> +	}
> >> +
> >> +DONE:
> >> +	rte_free(mac_filter);
> >> +	return ret;
> >> +}
> >> +
> >>   /* Configure vlan stripping on or off */  int
> >> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on) @@
> >> -4203,9
> >> +4270,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)  {
> >>   	struct rte_eth_dev_data *data = dev->data;
> >>   	int ret;
> >> +	int mask = 0;
> >>
> >>   	/* Apply vlan offload setting */
> >> -	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
> >> +	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
> >> +	i40e_vlan_offload_set(dev, mask);
> >>
> >>   	/* Apply double-vlan setting, not implemented yet */
> >>
> >> diff --git a/drivers/net/i40e/i40e_ethdev.h
> >> b/drivers/net/i40e/i40e_ethdev.h index 1f9792b..5505d72 100644
> >> --- a/drivers/net/i40e/i40e_ethdev.h
> >> +++ b/drivers/net/i40e/i40e_ethdev.h
> >> @@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi
> >> *vsi); int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
> >>   			   struct i40e_vsi_vlan_pvid_info *info);  int
> >> i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
> >> +int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
> >>   uint64_t i40e_config_hena(uint64_t flags);  uint64_t
> >> i40e_parse_hena(uint64_t flags);  enum i40e_status_code
> >> i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
> >> --
> >> 2.1.4
> 
> --
> Julien MEUNIER
> 6WIND

Re: [PATCH v2] i40e: fix vlan filtering

[Stephen Hemminger]

On Thu, 4 Feb 2016 00:41:00 +0000
"Zhang, Helin" <helin.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Julien Meunier [mailto:julien.meunier@6wind.com]
> > Sent: Wednesday, February 3, 2016 7:32 PM
> > To: Zhang, Helin <helin.zhang@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH v2] i40e: fix vlan filtering
> > 
> > Hello,
> > 
> > INFO log level is used in order to keep code homogeneity:
> > i40e_vsi_config_vlan_stripping or i40e_dev_init_vlan use this log level during
> > failure for example.
> > 
> > Tell me if ERR log level for VLAN filtering issue must be set.  
> There is a failure, and may result in uncertain behaviors which cannot be ignored.
> I'd suggest to use ERR but not INFO, though I am not so confident on that.
> Could Thomas help give some guidance on that?

Sounds like NOTICE level.

Re: [PATCH v2] i40e: fix vlan filtering

[Thomas Monjalon]

2016-02-04 00:41, Zhang, Helin:
> From: Julien Meunier [mailto:julien.meunier@6wind.com]
> > INFO log level is used in order to keep code homogeneity:
> > i40e_vsi_config_vlan_stripping or i40e_dev_init_vlan use this log level during
> > failure for example.
> > 
> > Tell me if ERR log level for VLAN filtering issue must be set.
> There is a failure, and may result in uncertain behaviors which cannot be ignored.
> I'd suggest to use ERR but not INFO, though I am not so confident on that.
> Could Thomas help give some guidance on that?

Helin, about i40e internals, I follow your guidance :)
If you consider it as a real error, yes we must log an error.

About code homogeneity, Julien seems to have seen some errors in the
driver which should be logged as such, right?
It may be a separate effort to fix them.
Thanks

[PATCH v3] i40e: fix vlan filtering

[Julien Meunier]

VLAN filtering was always performed, even if hw_vlan_filter was
disabled. During device initialization, default filter
RTE_MACVLAN_PERFECT_MATCH was applied. In this situation, all incoming
VLAN frames were dropped by the card (increase of the register RUPP - Rx
Unsupported Protocol).

In order to restore default behavior, if HW VLAN filtering is activated,
set a filter to match MAC and VLAN. If not, set a filter to only match
MAC.

Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
---
Changes since v2:
- switch log level from INFO to ERR in case of failure

Changes since v1:
- use ether_addr_copy() for mac copy
- add more debug messages in case of failure
- update all existing filters when multiple mac addresses have been configured
- when adding new mac address, use correct filter

TODO:
- i40e_update_default_filter_setting always forces to
  RTE_MACVLAN_PERFECT_MATCH.
  => The type of filter should be changed according to vlan filter setting.

- What happens if vlan filter setting changes when various filters are already
  set like RTE_MACVLAN_PERFECT_MATCH, RTE_MACVLAN_PERFECT_MATCH,
  RTE_MAC_HASH_MATCH, RTE_MACVLAN_HASH_MATCH ?
  => With testpmd, it is possible to add manually these filters. But when
  changing vlan filter setting, all previous filter set manually are overriden.
---
 drivers/net/i40e/i40e_ethdev.c | 73 ++++++++++++++++++++++++++++++++++++++++--
 drivers/net/i40e/i40e_ethdev.h |  1 +
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index bf6220d..750206b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2332,6 +2332,13 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *vsi = pf->main_vsi;
 
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+			i40e_vsi_config_vlan_filter(vsi, TRUE);
+		else
+			i40e_vsi_config_vlan_filter(vsi, FALSE);
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		if (dev->data->dev_conf.rxmode.hw_vlan_strip)
@@ -2583,7 +2590,10 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 	}
 
 	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
-	mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	if (dev->data->dev_conf.rxmode.hw_vlan_filter)
+		mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	else
+		mac_filter.filter_type = RTE_MAC_PERFECT_MATCH;
 
 	if (pool == 0)
 		vsi = pf->main_vsi;
@@ -4156,6 +4166,63 @@ fail_mem:
 	return NULL;
 }
 
+/* Configure vlan filter on or off */
+int
+i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on)
+{
+	int i, num;
+	struct i40e_mac_filter *f;
+	struct i40e_mac_filter_info *mac_filter;
+	enum rte_mac_filter_type desired_filter;
+	int ret = I40E_SUCCESS;
+
+	if (on) {
+		/* Filter to match MAC and VLAN */
+		desired_filter = RTE_MACVLAN_PERFECT_MATCH;
+	} else {
+		/* Filter to match only MAC */
+		desired_filter = RTE_MAC_PERFECT_MATCH;
+	}
+
+	num = vsi->mac_num;
+
+	mac_filter = rte_zmalloc("mac_filter_info_data",
+				 num * sizeof(*mac_filter), 0);
+	if (mac_filter == NULL) {
+		PMD_DRV_LOG(ERR, "failed to allocate memory");
+		return I40E_ERR_NO_MEMORY;
+	}
+
+	i = 0;
+
+	/* Remove all existing mac */
+	TAILQ_FOREACH(f, &vsi->mac_list, next) {
+		mac_filter[i] = f->mac_info;
+		ret = i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Update VSI failed to %s vlan filter",
+				    on ? "enable" : "disable");
+			goto DONE;
+		}
+		i++;
+	}
+
+	/* Override with new filter */
+	for (i = 0; i < num; i++) {
+		mac_filter[i].filter_type = desired_filter;
+		ret = i40e_vsi_add_mac(vsi, &mac_filter[i]);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Update VSI failed to %s vlan filter",
+				    on ? "enable" : "disable");
+			goto DONE;
+		}
+	}
+
+DONE:
+	rte_free(mac_filter);
+	return ret;
+}
+
 /* Configure vlan stripping on or off */
 int
 i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on)
@@ -4203,9 +4270,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)
 {
 	struct rte_eth_dev_data *data = dev->data;
 	int ret;
+	int mask = 0;
 
 	/* Apply vlan offload setting */
-	i40e_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
+	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
+	i40e_vlan_offload_set(dev, mask);
 
 	/* Apply double-vlan setting, not implemented yet */
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 1f9792b..5505d72 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -551,6 +551,7 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
+int i40e_vsi_config_vlan_filter(struct i40e_vsi *vsi, bool on);
 uint64_t i40e_config_hena(uint64_t flags);
 uint64_t i40e_parse_hena(uint64_t flags);
 enum i40e_status_code i40e_fdir_setup_tx_resources(struct i40e_pf *pf);
-- 
2.1.4

Re: [PATCH v3] i40e: fix vlan filtering

[Zhang, Helin]

> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@6wind.com]
> Sent: Thursday, February 4, 2016 7:02 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v3] i40e: fix vlan filtering
> 
> VLAN filtering was always performed, even if hw_vlan_filter was disabled.
> During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH
> was applied. In this situation, all incoming VLAN frames were dropped by the
> card (increase of the register RUPP - Rx Unsupported Protocol).
> 
> In order to restore default behavior, if HW VLAN filtering is activated, set a
> filter to match MAC and VLAN. If not, set a filter to only match MAC.
> 
> Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

Re: [PATCH v3] i40e: fix vlan filtering

[Bruce Richardson]

On Fri, Feb 05, 2016 at 12:20:15AM +0000, Zhang, Helin wrote:
> 
> 
> > -----Original Message-----
> > From: Julien Meunier [mailto:julien.meunier@6wind.com]
> > Sent: Thursday, February 4, 2016 7:02 PM
> > To: Zhang, Helin <helin.zhang@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [PATCH v3] i40e: fix vlan filtering
> > 
> > VLAN filtering was always performed, even if hw_vlan_filter was disabled.
> > During device initialization, default filter RTE_MACVLAN_PERFECT_MATCH
> > was applied. In this situation, all incoming VLAN frames were dropped by the
> > card (increase of the register RUPP - Rx Unsupported Protocol).
> > 
> > In order to restore default behavior, if HW VLAN filtering is activated, set a
> > filter to match MAC and VLAN. If not, set a filter to only match MAC.
> > 
> > Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>
> 
Applied to dpdk-next-net/rel_16_04

Thanks,
/Bruce