| Message ID | 1454421057-6602-1-git-send-email-julien.meunier@6wind.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 8924C95DD; Tue, 2 Feb 2016 14:51:23 +0100 (CET) Received: from mail-wm0-f43.google.com (mail-wm0-f43.google.com [74.125.82.43]) by dpdk.org (Postfix) with ESMTP id 4F4C895DC for <dev@dpdk.org>; Tue, 2 Feb 2016 14:51:22 +0100 (CET) Received: by mail-wm0-f43.google.com with SMTP id 128so119109834wmz.1 for <dev@dpdk.org>; Tue, 02 Feb 2016 05:51:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=F6EVyHzABenNZ1kELIJXs7MTIKkTvN31naxG8pzfLDg=; b=u5tLBgx+KHacmVZBtlk+17kr0hOjv4AFMV5pNkv0y7wnVWtsnBsl4X7Ln87iJhM9qM NoL1bFrXuCRyCiKy05V6MNqYlcRvTrBJ2tvXeA0/sdl/asHT4DG0mwGW65ZhZL79OQoh l6OUnAaCBFSegI33p7cnKz7ZLSLHkHaPC+z+YoYbcWnO5mXAi7EUdyfkrwEs2EAvsboH uXQIxof60SywKdJhb4Av+3Uc5x+8WL4tG19MUAKhGi7rMYoRhexsRzd93Zmp3R7WGgwe m+M5kGnMgVVF/KfzwcoIVx9YB+z1NfdEiK0+F1cNF+UHjlFTivP077+eK66fKjl1lqk6 61lg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=F6EVyHzABenNZ1kELIJXs7MTIKkTvN31naxG8pzfLDg=; b=aHeocVerNE9K2xEew0LHG8JeGYJECuSHSPEm5w1pj6vYEK2nu2wSB1Pqr4i7VHwWEX V4HSr8qwi9BibURy0sC1urjQELJqmn4coQYDoCKLNoVMf/8mO0IiEFxMxQU4gwEW4u6E HI/IW2UGERRXVQNm69BdIKoFOb90hZaS0xbsLYL4Tl21gZIpdsOPBny/VIKqaJXwYSD0 /BrvZcyEEjVOrf100w1LBNNKmY1SDnmBP5CjvpsTbwb5wj08GmutziQPQhMFbeckp4N0 hG0+sr9qip2G+tGvSI0N/201B1TOuoa1a/9xfT4N6nb2a/BHHmiIDxPnwXdO/d6LdDJV j2bA== X-Gm-Message-State: AG10YOSMcx3rS65QPo9lWyrf+L8X2R7RaAT1efEH51TVFQQURvpLlpy0Niz+qrCanj+MGo+r X-Received: by 10.194.9.200 with SMTP id c8mr11545591wjb.63.1454421082106; Tue, 02 Feb 2016 05:51:22 -0800 (PST) Received: from grendel.vm.6wind.com. (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id os7sm1622690wjc.18.2016.02.02.05.51.21 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 02 Feb 2016 05:51:21 -0800 (PST) From: Julien Meunier <julien.meunier@6wind.com> To: helin.zhang@intel.com Date: Tue, 2 Feb 2016 14:50:57 +0100 Message-Id: <1454421057-6602-1-git-send-email-julien.meunier@6wind.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1453137515-19195-1-git-send-email-julien.meunier@6wind.com> References: <1453137515-19195-1-git-send-email-julien.meunier@6wind.com> Cc: dev@dpdk.org Subject: [dpdk-dev] [PATCH v2] i40e: fix vlan filtering X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Julien Meunier
Feb. 2, 2016, 1:50 p.m. UTC
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(-)
Comments
> -----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
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
> -----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
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.
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
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);