From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: wangyunjian <wangyunjian@huawei.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "xudingke@huawei.com" <xudingke@huawei.com>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix crash when calling i40e_vsi_delete_mac
Date: Mon, 15 Apr 2019 12:21:08 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB9772580148A97E3D@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <1555149291-12432-1-git-send-email-wangyunjian@huawei.com>
Hi,
> Now the macvlan filter list may be accessed in the same time by two
> different threads and may cause a lot of optional errors. This patch
> protects the macvlan filter access with a spinlock.
>
> Call Trace:
> #1 0x00007ffb4cbe2e3c in i40e_vsi_delete_mac (vsi=vsi@entry=
> 0x400052804b40, addr=addr@entry=0x7ffb47672244) at /usr/src/
> debug/dpdk-18.11/drivers/net/i40e/i40e_ethdev.c:7266
> #2 0x00007ffb4cbe342b in i40e_set_default_mac_addr (dev=<optimized out>,
> mac_addr=0x400052a6618d) at /usr/src/debug/dpdk-18.11/drivers/net/
> i40e/i40e_ethdev.c:11893
> #3 0x00007ffb4f569d4a in rte_eth_dev_default_mac_addr_set (port_id=
> <optimized out>, addr=addr@entry=0x400052a6618d) at /usr/src/debug/
> dpdk-18.11/lib/librte_ethdev/rte_ethdev.c:3366
> #4 0x00007ffb4d0bb403 in mac_address_slaves_update (bonded_eth_dev=
> bonded_eth_dev@entry=0xacf8c0 <rte_eth_devices>) at /usr/src/debug/
> dpdk-18.11/drivers/net/bonding/rte_eth_bond_pmd.c:1854
> #5 0x00007ffb4d0bd221 in bond_ethdev_lsc_event_callback (port_id=
> <optimized out>, type=<optimized out>, param=<optimized out>,
> ret_param= <optimized out>) at /usr/src/debug/dpdk-18.11/drivers/
> net/bonding/rte_eth_bond_pmd.c:3076
> #6 0x00007ffb4f56aa09 in _rte_eth_dev_callback_process (dev=dev@entry=
> 0xad3940 <rte_eth_devices+16512>, event=event@entry=
> RTE_ETH_EVENT_INTR_LSC, ret_param=ret_param@entry=0x0)
> at /usr/src/debug/dpdk-18.11/lib/librte_ethdev/rte_ethdev.c:3699
> #7 0x00007ffb4cbb99f1 in i40e_dev_handle_aq_msg (dev=dev@entry=0xad3940
> <rte_eth_devices+16512>) at /usr/src/debug/dpdk-18.11/drivers/net/
> i40e/i40e_ethdev.c:6573
> #8 0x00007ffb4cbdfbed in i40e_dev_alarm_handler (param=0xad3940
> <rte_eth_devices+16512>) at /usr/src/debug/dpdk-18.11/drivers/net/
> i40e/i40e_ethdev.c:6681
> #9 0x00007ffb4fb9766f in eal_alarm_callback (arg=<optimized out>) at
> /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/eal_alarm.c:90
> #10 0x00007ffb4fb95dd2 in eal_intr_process_interrupts (nfds=<optimized
> out>, events=<optimized out>) at /usr/src/debug/dpdk-18.11/lib/
> librte_eal/linuxapp/eal/eal_interrupts.c:886
> #11 eal_intr_handle_interrupts (totalfds=<optimized out>, pfd=20) at
> /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/
> eal_interrupts.c:946
> #12 eal_intr_thread_main (arg=<optimized out>) at /usr/src/debug/
> dpdk-18.11/lib/librte_eal/linuxapp/eal/eal_interrupts.c:1035
> #13 0x00007ffb4b208dd5 in start_thread () from /usr/lib64/libpthread.so.0
> #14 0x00007ffb4981659d in clone () from /usr/lib64/libc.so.6
That is not specific to i40e or macvlan filter.
If inside your app several threads concurrently access/modify NIC config,
then you need to provide some synchronization mechanism for them.
DPDK ethdev API (as most others) on itself doesn't provide any synchronization,
leaving it up to the upper layer to choose the most appropriate one.
Konstantin
>
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> Cc: stable@dpdk.org
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 28 +++++++++++++++++++++++++---
> drivers/net/i40e/i40e_ethdev.h | 1 +
> drivers/net/i40e/i40e_pf.c | 6 ++++++
> drivers/net/i40e/rte_pmd_i40e.c | 12 ++++++++++++
> 4 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5b01dc1..e4f6818 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -4036,8 +4036,9 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> vsi = pf->main_vsi;
> else
> vsi = pf->vmdq[pool - 1].vsi;
> -
> + rte_spinlock_lock(&vsi->mac_list_lock);
> ret = i40e_vsi_add_mac(vsi, &mac_filter);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> if (ret != I40E_SUCCESS) {
> PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter");
> return -ENODEV;
> @@ -4075,7 +4076,9 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> }
> vsi = pf->vmdq[i - 1].vsi;
> }
> + rte_spinlock_lock(&vsi->mac_list_lock);
> ret = i40e_vsi_delete_mac(vsi, macaddr);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
>
> if (ret) {
> PMD_DRV_LOG(ERR, "Failed to remove MACVLAN filter");
> @@ -4138,7 +4141,9 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> ETHER_ADDR_LEN);
>
> mac_filter.filter_type = filter->filter_type;
> + rte_spinlock_lock(&vf->vsi->mac_list_lock);
> ret = i40e_vsi_add_mac(vf->vsi, &mac_filter);
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> if (ret != I40E_SUCCESS) {
> PMD_DRV_LOG(ERR, "Failed to add MAC filter.");
> return -1;
> @@ -4147,7 +4152,9 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> } else {
> rte_memcpy(hw->mac.addr, hw->mac.perm_addr,
> ETHER_ADDR_LEN);
> + rte_spinlock_lock(&vf->vsi->mac_list_lock);
> ret = i40e_vsi_delete_mac(vf->vsi, &filter->mac_addr);
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> if (ret != I40E_SUCCESS) {
> PMD_DRV_LOG(ERR, "Failed to delete MAC filter.");
> return -1;
> @@ -5266,9 +5273,11 @@ static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
> }
>
> /* Remove all macvlan filters of the VSI */
> + rte_spinlock_lock(&vsi->mac_list_lock);
> i40e_vsi_remove_all_macvlan_filter(vsi);
> TAILQ_FOREACH_SAFE(f, &vsi->mac_list, next, temp)
> rte_free(f);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
>
> if (vsi->type != I40E_VSI_MAIN &&
> ((vsi->type != I40E_VSI_SRIOV) ||
> @@ -5346,7 +5355,9 @@ static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
> rte_memcpy(&mac->addr_bytes, hw->mac.perm_addr,
> ETH_ADDR_LEN);
> f->mac_info.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> + rte_spinlock_lock(&vsi->mac_list_lock);
> TAILQ_INSERT_TAIL(&vsi->mac_list, f, next);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> vsi->mac_num++;
>
> return ret;
> @@ -5354,7 +5365,10 @@ static int i40e_pf_config_vf_rxq_number(struct rte_eth_dev *dev)
> rte_memcpy(&filter.mac_addr,
> (struct ether_addr *)(hw->mac.perm_addr), ETH_ADDR_LEN);
> filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> - return i40e_vsi_add_mac(vsi, &filter);
> + rte_spinlock_lock(&vsi->mac_list_lock);
> + ret = i40e_vsi_add_mac(vsi, &filter);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> + return ret;
> }
>
> /*
> @@ -5521,6 +5535,7 @@ struct i40e_vsi *
> return NULL;
> }
> TAILQ_INIT(&vsi->mac_list);
> + rte_spinlock_init(&vsi->mac_list_lock);
> vsi->type = type;
> vsi->adapter = I40E_PF_TO_ADAPTER(pf);
> vsi->max_macaddrs = I40E_NUM_MACADDR_MAX;
> @@ -5816,8 +5831,9 @@ struct i40e_vsi *
> /* MAC/VLAN configuration */
> rte_memcpy(&filter.mac_addr, &broadcast, ETHER_ADDR_LEN);
> filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> -
> + rte_spinlock_lock(&vsi->mac_list_lock);
> ret = i40e_vsi_add_mac(vsi, &filter);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> if (ret != I40E_SUCCESS) {
> PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter");
> goto fail_msix_alloc;
> @@ -5866,6 +5882,7 @@ struct i40e_vsi *
> i = 0;
>
> /* Remove all existing mac */
> + rte_spinlock_lock(&vsi->mac_list_lock);
> TAILQ_FOREACH_SAFE(f, &vsi->mac_list, next, temp) {
> mac_filter[i] = f->mac_info;
> ret = i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr);
> @@ -5889,6 +5906,7 @@ struct i40e_vsi *
> }
>
> DONE:
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> rte_free(mac_filter);
> return ret;
> }
> @@ -11953,12 +11971,14 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> return -EINVAL;
> }
>
> + rte_spinlock_lock(&vsi->mac_list_lock);
> TAILQ_FOREACH(f, &vsi->mac_list, next) {
> if (is_same_ether_addr(&pf->dev_addr, &f->mac_info.mac_addr))
> break;
> }
>
> if (f == NULL) {
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> PMD_DRV_LOG(ERR, "Failed to find filter for default mac");
> return -EIO;
> }
> @@ -11966,11 +11986,13 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> mac_filter = f->mac_info;
> ret = i40e_vsi_delete_mac(vsi, &mac_filter.mac_addr);
> if (ret != I40E_SUCCESS) {
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> PMD_DRV_LOG(ERR, "Failed to delete mac filter");
> return -EIO;
> }
> memcpy(&mac_filter.mac_addr, mac_addr, ETH_ADDR_LEN);
> ret = i40e_vsi_add_mac(vsi, &mac_filter);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> if (ret != I40E_SUCCESS) {
> PMD_DRV_LOG(ERR, "Failed to add mac filter");
> return -EIO;
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 930eb9a..060be26 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -368,6 +368,7 @@ struct i40e_vsi {
> uint16_t mac_num; /* Total mac number */
> uint32_t vfta[I40E_VFTA_SIZE]; /* VLAN bitmap */
> struct i40e_mac_filter_list mac_list; /* macvlan filter list */
> + rte_spinlock_t mac_list_lock; /* macvlan filter list lock*/
> /* specific VSI-defined parameters, SRIOV stored the vf_id */
> uint32_t user_param;
> uint16_t seid; /* The seid of VSI itself */
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> index 91be450..faa80cc 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -845,11 +845,14 @@
> mac = (struct ether_addr *)(addr_list->list[i].addr);
> rte_memcpy(&filter.mac_addr, mac, ETHER_ADDR_LEN);
> filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> + rte_spinlock_lock(&vf->vsi->mac_list_lock);
> if (is_zero_ether_addr(mac) ||
> i40e_vsi_add_mac(vf->vsi, &filter)) {
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> ret = I40E_ERR_INVALID_MAC_ADDR;
> goto send_msg;
> }
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> }
>
> send_msg:
> @@ -887,11 +890,14 @@
>
> for (i = 0; i < addr_list->num_elements; i++) {
> mac = (struct ether_addr *)(addr_list->list[i].addr);
> + rte_spinlock_lock(&vf->vsi->mac_list_lock);
> if(is_zero_ether_addr(mac) ||
> i40e_vsi_delete_mac(vf->vsi, mac)) {
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> ret = I40E_ERR_INVALID_MAC_ADDR;
> goto send_msg;
> }
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> }
>
> send_msg:
> diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
> index 7ae78e4..c5983c9 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -360,7 +360,9 @@
> }
>
> /* remove all the MAC and VLAN first */
> + rte_spinlock_lock(&vsi->mac_list_lock);
> ret = i40e_vsi_rm_mac_filter(vsi);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> if (ret) {
> PMD_INIT_LOG(ERR, "Failed to remove MAC filters.");
> return ret;
> @@ -390,7 +392,9 @@
> }
>
> /* add all the MAC and VLAN back */
> + rte_spinlock_lock(&vsi->mac_list_lock);
> ret = i40e_vsi_restore_mac_filter(vsi);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
> if (ret)
> return ret;
> if (vsi->vlan_anti_spoof_on || vsi->vlan_filter_on) {
> @@ -563,10 +567,12 @@
> ether_addr_copy(mac_addr, &vf->mac_addr);
>
> /* Remove all existing mac */
> + rte_spinlock_lock(&vsi->mac_list_lock);
> TAILQ_FOREACH_SAFE(f, &vsi->mac_list, next, temp)
> if (i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr)
> != I40E_SUCCESS)
> PMD_DRV_LOG(WARNING, "Delete MAC failed");
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
>
> return 0;
> }
> @@ -609,7 +615,9 @@
> ether_addr_copy(&null_mac_addr, &vf->mac_addr);
>
> /* Remove the mac */
> + rte_spinlock_lock(&vsi->mac_list_lock);
> i40e_vsi_delete_mac(vsi, mac_addr);
> + rte_spinlock_unlock(&vsi->mac_list_lock);
>
> return 0;
> }
> @@ -764,6 +772,7 @@ int rte_pmd_i40e_set_vf_broadcast(uint16_t port, uint16_t vf_id,
> return -EINVAL;
> }
>
> + rte_spinlock_lock(&vsi->mac_list_lock);
> if (on) {
> rte_memcpy(&filter.mac_addr, &broadcast, ETHER_ADDR_LEN);
> filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> @@ -771,6 +780,7 @@ int rte_pmd_i40e_set_vf_broadcast(uint16_t port, uint16_t vf_id,
> } else {
> ret = i40e_vsi_delete_mac(vsi, &broadcast);
> }
> + rte_spinlock_unlock(&vsi->mac_list_lock);
>
> if (ret != I40E_SUCCESS && ret != I40E_ERR_PARAM) {
> ret = -ENOTSUP;
> @@ -2388,7 +2398,9 @@ int rte_pmd_i40e_ptype_mapping_replace(uint16_t port,
>
> mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
> ether_addr_copy(mac_addr, &mac_filter.mac_addr);
> + rte_spinlock_lock(&vf->vsi->mac_list_lock);
> ret = i40e_vsi_add_mac(vsi, &mac_filter);
> + rte_spinlock_unlock(&vf->vsi->mac_list_lock);
> if (ret != I40E_SUCCESS) {
> PMD_DRV_LOG(ERR, "Failed to add MAC filter.");
> return -1;
> --
> 1.8.3.1
>
next prev parent reply other threads:[~2019-04-15 12:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-13 9:54 wangyunjian
2019-04-13 9:54 ` wangyunjian
2019-04-15 12:21 ` Ananyev, Konstantin [this message]
2019-04-15 12:21 ` Ananyev, Konstantin
2019-04-15 12:50 ` Zhang, Qi Z
2019-04-15 12:50 ` Zhang, Qi Z
2019-04-16 2:05 ` wangyunjian
2019-04-16 2:05 ` wangyunjian
2019-04-23 3:39 ` Zhang, Qi Z
2019-04-23 3:39 ` Zhang, Qi Z
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB9772580148A97E3D@irsmsx105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
--cc=wangyunjian@huawei.com \
--cc=xudingke@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).