[dpdk-dev] [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 at 6wind.com]
>> Sent: Tuesday, February 2, 2016 9:51 PM
>> To: Zhang, Helin <helin.zhang at intel.com>
>> Cc: dev at 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 at 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