[dpdk-dev] [PATCH] net/i40e: fix crash when calling i40e_vsi_delete_mac
Ananyev, Konstantin
konstantin.ananyev at intel.com
Mon Apr 15 14:21:08 CEST 2019
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 at entry=
> 0x400052804b40, addr=addr at 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 at 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 at 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 at entry=
> 0xad3940 <rte_eth_devices+16512>, event=event at entry=
> RTE_ETH_EVENT_INTR_LSC, ret_param=ret_param at 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 at 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 at dpdk.org
>
> Signed-off-by: Yunjian Wang <wangyunjian at 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
>
More information about the dev
mailing list