> -----Original Message----- > From: Julien Meunier [mailto:julien.meunier at 6wind.com] > Sent: Wednesday, February 3, 2016 7:32 PM > To: Zhang, Helin <helin.zhang at intel.com> > Cc: dev at 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 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
