net/i40e: fix crash when calling i40e_vsi_delete_mac
Checks
Commit Message
From: Yunjian Wang <wangyunjian@huawei.com>
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
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(-)
Comments
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
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Monday, April 15, 2019 8:21 PM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> Cc: xudingke@huawei.com; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix crash when calling
> i40e_vsi_delete_mac
>
> 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
+1
>
> 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
Thanks. Now the lsc thread isn't controled by the upper layer.
Do you have any idea to fix it?
Thanks,
Yunjian
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of wangyunjian
> Sent: Tuesday, April 16, 2019 10:05 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: xudingke <xudingke@huawei.com>; stable@dpdk.org; Lilijun (Jerry,
> Cloud Networking) <jerry.lilijun@huawei.com>; Zhang, Jerry
> <jerry.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix crash when calling
> i40e_vsi_delete_mac
>
> >
> > 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
>
> Thanks. Now the lsc thread isn't controled by the upper layer.
> Do you have any idea to fix it?
What do you mean "the lsc thread isn't controlled by the upper layer"?
you may invoke ethdev APIs in LSC callback function, and use some lock to prevent race condition,
>
> Thanks,
> Yunjian
@@ -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;
@@ -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 */
@@ -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:
@@ -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;