From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id D052C5942; Mon, 15 Apr 2019 14:21:12 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Apr 2019 05:21:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,353,1549958400"; d="scan'208";a="140800632" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga008.fm.intel.com with ESMTP; 15 Apr 2019 05:21:10 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.31]) by IRSMSX101.ger.corp.intel.com ([169.254.1.115]) with mapi id 14.03.0415.000; Mon, 15 Apr 2019 13:21:09 +0100 From: "Ananyev, Konstantin" To: wangyunjian , "dev@dpdk.org" CC: "xudingke@huawei.com" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] net/i40e: fix crash when calling i40e_vsi_delete_mac Thread-Index: AQHU8d8IjuCfzE+MZEaWqavVP6qkOaY9JZow Date: Mon, 15 Apr 2019 12:21:08 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580148A97E3D@irsmsx105.ger.corp.intel.com> References: <1555149291-12432-1-git-send-email-wangyunjian@huawei.com> In-Reply-To: <1555149291-12432-1-git-send-email-wangyunjian@huawei.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTg3MjU4ZjEtNGU1Yy00ZjZiLWIxYmEtNjczZTFkNzYyOWZkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiMXliU0JUeWZ1bXpHdWczRWoyWnc2ZHhsTVg2YjNDYXpYYWlveXRDbjBMUnV0YTZuQ0J4V3ppXC9MWFFDbkxZaE4ifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix crash when calling i40e_vsi_delete_mac X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Apr 2019 12:21:13 -0000 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. >=20 > Call Trace: > #1 0x00007ffb4cbe2e3c in i40e_vsi_delete_mac (vsi=3Dvsi@entry=3D > 0x400052804b40, addr=3Daddr@entry=3D0x7ffb47672244) at /usr/src/ > debug/dpdk-18.11/drivers/net/i40e/i40e_ethdev.c:7266 > #2 0x00007ffb4cbe342b in i40e_set_default_mac_addr (dev=3D, > mac_addr=3D0x400052a6618d) 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=3D > , addr=3Daddr@entry=3D0x400052a6618d) at /usr/src/de= bug/ > dpdk-18.11/lib/librte_ethdev/rte_ethdev.c:3366 > #4 0x00007ffb4d0bb403 in mac_address_slaves_update (bonded_eth_dev=3D > bonded_eth_dev@entry=3D0xacf8c0 ) at /usr/src/debu= g/ > dpdk-18.11/drivers/net/bonding/rte_eth_bond_pmd.c:1854 > #5 0x00007ffb4d0bd221 in bond_ethdev_lsc_event_callback (port_id=3D > , type=3D, param=3D, > ret_param=3D ) 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=3Ddev@entr= y=3D > 0xad3940 , event=3Devent@entry=3D > RTE_ETH_EVENT_INTR_LSC, ret_param=3Dret_param@entry=3D0x0) > at /usr/src/debug/dpdk-18.11/lib/librte_ethdev/rte_ethdev.c:3699 > #7 0x00007ffb4cbb99f1 in i40e_dev_handle_aq_msg (dev=3Ddev@entry=3D0xa= d3940 > ) at /usr/src/debug/dpdk-18.11/drivers/net/ > i40e/i40e_ethdev.c:6573 > #8 0x00007ffb4cbdfbed in i40e_dev_alarm_handler (param=3D0xad3940 > ) at /usr/src/debug/dpdk-18.11/drivers/net/ > i40e/i40e_ethdev.c:6681 > #9 0x00007ffb4fb9766f in eal_alarm_callback (arg=3D) at > /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/eal_alarm.c:9= 0 > #10 0x00007ffb4fb95dd2 in eal_intr_process_interrupts (nfds=3D out>, events=3D) at /usr/src/debug/dpdk-18.11/lib/ > librte_eal/linuxapp/eal/eal_interrupts.c:886 > #11 eal_intr_handle_interrupts (totalfds=3D, pfd=3D20) a= t > /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/ > eal_interrupts.c:946 > #12 eal_intr_thread_main (arg=3D) 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 synchronizat= ion, leaving it up to the upper layer to choose the most appropriate one. Konstantin >=20 > Fixes: 4861cde46116 ("i40e: new poll mode driver") > Cc: stable@dpdk.org >=20 > Signed-off-by: Yunjian Wang > --- > 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(-) >=20 > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethde= v.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 s= truct rte_eth_dev *dev, > vsi =3D pf->main_vsi; > else > vsi =3D pf->vmdq[pool - 1].vsi; > - > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vsi, &mac_filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > if (ret !=3D 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 s= truct rte_eth_dev *dev, > } > vsi =3D pf->vmdq[i - 1].vsi; > } > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D i40e_vsi_delete_mac(vsi, macaddr); > + rte_spinlock_unlock(&vsi->mac_list_lock); >=20 > if (ret) { > PMD_DRV_LOG(ERR, "Failed to remove MACVLAN filter"); > @@ -4138,7 +4141,9 @@ static int i40e_dev_xstats_get_names(__rte_unused s= truct rte_eth_dev *dev, > ETHER_ADDR_LEN); >=20 > mac_filter.filter_type =3D filter->filter_type; > + rte_spinlock_lock(&vf->vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vf->vsi, &mac_filter); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > if (ret !=3D 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 s= truct 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 =3D i40e_vsi_delete_mac(vf->vsi, &filter->mac_addr); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > if (ret !=3D 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) > } >=20 > /* 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); >=20 > if (vsi->type !=3D I40E_VSI_MAIN && > ((vsi->type !=3D 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 =3D 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++; >=20 > 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 =3D RTE_MACVLAN_PERFECT_MATCH; > - return i40e_vsi_add_mac(vsi, &filter); > + rte_spinlock_lock(&vsi->mac_list_lock); > + ret =3D i40e_vsi_add_mac(vsi, &filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > + return ret; > } >=20 > /* > @@ -5521,6 +5535,7 @@ struct i40e_vsi * > return NULL; > } > TAILQ_INIT(&vsi->mac_list); > + rte_spinlock_init(&vsi->mac_list_lock); > vsi->type =3D type; > vsi->adapter =3D I40E_PF_TO_ADAPTER(pf); > vsi->max_macaddrs =3D 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 =3D RTE_MACVLAN_PERFECT_MATCH; > - > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vsi, &filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > if (ret !=3D I40E_SUCCESS) { > PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter"); > goto fail_msix_alloc; > @@ -5866,6 +5882,7 @@ struct i40e_vsi * > i =3D 0; >=20 > /* Remove all existing mac */ > + rte_spinlock_lock(&vsi->mac_list_lock); > TAILQ_FOREACH_SAFE(f, &vsi->mac_list, next, temp) { > mac_filter[i] =3D f->mac_info; > ret =3D i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr); > @@ -5889,6 +5906,7 @@ struct i40e_vsi * > } >=20 > 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; > } >=20 > + 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; > } >=20 > if (f =3D=3D 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 =3D f->mac_info; > ret =3D i40e_vsi_delete_mac(vsi, &mac_filter.mac_addr); > if (ret !=3D 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 =3D i40e_vsi_add_mac(vsi, &mac_filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > if (ret !=3D 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_ethde= v.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 =3D (struct ether_addr *)(addr_list->list[i].addr); > rte_memcpy(&filter.mac_addr, mac, ETHER_ADDR_LEN); > filter.filter_type =3D 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 =3D I40E_ERR_INVALID_MAC_ADDR; > goto send_msg; > } > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > } >=20 > send_msg: > @@ -887,11 +890,14 @@ >=20 > for (i =3D 0; i < addr_list->num_elements; i++) { > mac =3D (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 =3D I40E_ERR_INVALID_MAC_ADDR; > goto send_msg; > } > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > } >=20 > send_msg: > diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i= 40e.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 @@ > } >=20 > /* remove all the MAC and VLAN first */ > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D 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 @@ > } >=20 > /* add all the MAC and VLAN back */ > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D 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); >=20 > /* 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) > !=3D I40E_SUCCESS) > PMD_DRV_LOG(WARNING, "Delete MAC failed"); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); >=20 > return 0; > } > @@ -609,7 +615,9 @@ > ether_addr_copy(&null_mac_addr, &vf->mac_addr); >=20 > /* Remove the mac */ > + rte_spinlock_lock(&vsi->mac_list_lock); > i40e_vsi_delete_mac(vsi, mac_addr); > + rte_spinlock_unlock(&vsi->mac_list_lock); >=20 > return 0; > } > @@ -764,6 +772,7 @@ int rte_pmd_i40e_set_vf_broadcast(uint16_t port, uint= 16_t vf_id, > return -EINVAL; > } >=20 > + rte_spinlock_lock(&vsi->mac_list_lock); > if (on) { > rte_memcpy(&filter.mac_addr, &broadcast, ETHER_ADDR_LEN); > filter.filter_type =3D RTE_MACVLAN_PERFECT_MATCH; > @@ -771,6 +780,7 @@ int rte_pmd_i40e_set_vf_broadcast(uint16_t port, uint= 16_t vf_id, > } else { > ret =3D i40e_vsi_delete_mac(vsi, &broadcast); > } > + rte_spinlock_unlock(&vsi->mac_list_lock); >=20 > if (ret !=3D I40E_SUCCESS && ret !=3D I40E_ERR_PARAM) { > ret =3D -ENOTSUP; > @@ -2388,7 +2398,9 @@ int rte_pmd_i40e_ptype_mapping_replace(uint16_t por= t, >=20 > mac_filter.filter_type =3D RTE_MACVLAN_PERFECT_MATCH; > ether_addr_copy(mac_addr, &mac_filter.mac_addr); > + rte_spinlock_lock(&vf->vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vsi, &mac_filter); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > if (ret !=3D I40E_SUCCESS) { > PMD_DRV_LOG(ERR, "Failed to add MAC filter."); > return -1; > -- > 1.8.3.1 >=20 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 5FBC4A00E6 for ; Mon, 15 Apr 2019 14:21:18 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 108CA1B1EF; Mon, 15 Apr 2019 14:21:15 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id D052C5942; Mon, 15 Apr 2019 14:21:12 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Apr 2019 05:21:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,353,1549958400"; d="scan'208";a="140800632" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga008.fm.intel.com with ESMTP; 15 Apr 2019 05:21:10 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.31]) by IRSMSX101.ger.corp.intel.com ([169.254.1.115]) with mapi id 14.03.0415.000; Mon, 15 Apr 2019 13:21:09 +0100 From: "Ananyev, Konstantin" To: wangyunjian , "dev@dpdk.org" CC: "xudingke@huawei.com" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] net/i40e: fix crash when calling i40e_vsi_delete_mac Thread-Index: AQHU8d8IjuCfzE+MZEaWqavVP6qkOaY9JZow Date: Mon, 15 Apr 2019 12:21:08 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580148A97E3D@irsmsx105.ger.corp.intel.com> References: <1555149291-12432-1-git-send-email-wangyunjian@huawei.com> In-Reply-To: <1555149291-12432-1-git-send-email-wangyunjian@huawei.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTg3MjU4ZjEtNGU1Yy00ZjZiLWIxYmEtNjczZTFkNzYyOWZkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiMXliU0JUeWZ1bXpHdWczRWoyWnc2ZHhsTVg2YjNDYXpYYWlveXRDbjBMUnV0YTZuQ0J4V3ppXC9MWFFDbkxZaE4ifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix crash when calling i40e_vsi_delete_mac X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190415122108.yQ9Mls2RrU51USv4jbL4YMFuDyGnFz5igy9vDJD_gKQ@z> 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. >=20 > Call Trace: > #1 0x00007ffb4cbe2e3c in i40e_vsi_delete_mac (vsi=3Dvsi@entry=3D > 0x400052804b40, addr=3Daddr@entry=3D0x7ffb47672244) at /usr/src/ > debug/dpdk-18.11/drivers/net/i40e/i40e_ethdev.c:7266 > #2 0x00007ffb4cbe342b in i40e_set_default_mac_addr (dev=3D, > mac_addr=3D0x400052a6618d) 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=3D > , addr=3Daddr@entry=3D0x400052a6618d) at /usr/src/de= bug/ > dpdk-18.11/lib/librte_ethdev/rte_ethdev.c:3366 > #4 0x00007ffb4d0bb403 in mac_address_slaves_update (bonded_eth_dev=3D > bonded_eth_dev@entry=3D0xacf8c0 ) at /usr/src/debu= g/ > dpdk-18.11/drivers/net/bonding/rte_eth_bond_pmd.c:1854 > #5 0x00007ffb4d0bd221 in bond_ethdev_lsc_event_callback (port_id=3D > , type=3D, param=3D, > ret_param=3D ) 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=3Ddev@entr= y=3D > 0xad3940 , event=3Devent@entry=3D > RTE_ETH_EVENT_INTR_LSC, ret_param=3Dret_param@entry=3D0x0) > at /usr/src/debug/dpdk-18.11/lib/librte_ethdev/rte_ethdev.c:3699 > #7 0x00007ffb4cbb99f1 in i40e_dev_handle_aq_msg (dev=3Ddev@entry=3D0xa= d3940 > ) at /usr/src/debug/dpdk-18.11/drivers/net/ > i40e/i40e_ethdev.c:6573 > #8 0x00007ffb4cbdfbed in i40e_dev_alarm_handler (param=3D0xad3940 > ) at /usr/src/debug/dpdk-18.11/drivers/net/ > i40e/i40e_ethdev.c:6681 > #9 0x00007ffb4fb9766f in eal_alarm_callback (arg=3D) at > /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/eal_alarm.c:9= 0 > #10 0x00007ffb4fb95dd2 in eal_intr_process_interrupts (nfds=3D out>, events=3D) at /usr/src/debug/dpdk-18.11/lib/ > librte_eal/linuxapp/eal/eal_interrupts.c:886 > #11 eal_intr_handle_interrupts (totalfds=3D, pfd=3D20) a= t > /usr/src/debug/dpdk-18.11/lib/librte_eal/linuxapp/eal/ > eal_interrupts.c:946 > #12 eal_intr_thread_main (arg=3D) 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 synchronizat= ion, leaving it up to the upper layer to choose the most appropriate one. Konstantin >=20 > Fixes: 4861cde46116 ("i40e: new poll mode driver") > Cc: stable@dpdk.org >=20 > Signed-off-by: Yunjian Wang > --- > 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(-) >=20 > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethde= v.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 s= truct rte_eth_dev *dev, > vsi =3D pf->main_vsi; > else > vsi =3D pf->vmdq[pool - 1].vsi; > - > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vsi, &mac_filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > if (ret !=3D 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 s= truct rte_eth_dev *dev, > } > vsi =3D pf->vmdq[i - 1].vsi; > } > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D i40e_vsi_delete_mac(vsi, macaddr); > + rte_spinlock_unlock(&vsi->mac_list_lock); >=20 > if (ret) { > PMD_DRV_LOG(ERR, "Failed to remove MACVLAN filter"); > @@ -4138,7 +4141,9 @@ static int i40e_dev_xstats_get_names(__rte_unused s= truct rte_eth_dev *dev, > ETHER_ADDR_LEN); >=20 > mac_filter.filter_type =3D filter->filter_type; > + rte_spinlock_lock(&vf->vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vf->vsi, &mac_filter); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > if (ret !=3D 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 s= truct 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 =3D i40e_vsi_delete_mac(vf->vsi, &filter->mac_addr); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > if (ret !=3D 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) > } >=20 > /* 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); >=20 > if (vsi->type !=3D I40E_VSI_MAIN && > ((vsi->type !=3D 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 =3D 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++; >=20 > 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 =3D RTE_MACVLAN_PERFECT_MATCH; > - return i40e_vsi_add_mac(vsi, &filter); > + rte_spinlock_lock(&vsi->mac_list_lock); > + ret =3D i40e_vsi_add_mac(vsi, &filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > + return ret; > } >=20 > /* > @@ -5521,6 +5535,7 @@ struct i40e_vsi * > return NULL; > } > TAILQ_INIT(&vsi->mac_list); > + rte_spinlock_init(&vsi->mac_list_lock); > vsi->type =3D type; > vsi->adapter =3D I40E_PF_TO_ADAPTER(pf); > vsi->max_macaddrs =3D 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 =3D RTE_MACVLAN_PERFECT_MATCH; > - > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vsi, &filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > if (ret !=3D I40E_SUCCESS) { > PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter"); > goto fail_msix_alloc; > @@ -5866,6 +5882,7 @@ struct i40e_vsi * > i =3D 0; >=20 > /* Remove all existing mac */ > + rte_spinlock_lock(&vsi->mac_list_lock); > TAILQ_FOREACH_SAFE(f, &vsi->mac_list, next, temp) { > mac_filter[i] =3D f->mac_info; > ret =3D i40e_vsi_delete_mac(vsi, &f->mac_info.mac_addr); > @@ -5889,6 +5906,7 @@ struct i40e_vsi * > } >=20 > 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; > } >=20 > + 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; > } >=20 > if (f =3D=3D 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 =3D f->mac_info; > ret =3D i40e_vsi_delete_mac(vsi, &mac_filter.mac_addr); > if (ret !=3D 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 =3D i40e_vsi_add_mac(vsi, &mac_filter); > + rte_spinlock_unlock(&vsi->mac_list_lock); > if (ret !=3D 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_ethde= v.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 =3D (struct ether_addr *)(addr_list->list[i].addr); > rte_memcpy(&filter.mac_addr, mac, ETHER_ADDR_LEN); > filter.filter_type =3D 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 =3D I40E_ERR_INVALID_MAC_ADDR; > goto send_msg; > } > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > } >=20 > send_msg: > @@ -887,11 +890,14 @@ >=20 > for (i =3D 0; i < addr_list->num_elements; i++) { > mac =3D (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 =3D I40E_ERR_INVALID_MAC_ADDR; > goto send_msg; > } > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > } >=20 > send_msg: > diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i= 40e.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 @@ > } >=20 > /* remove all the MAC and VLAN first */ > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D 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 @@ > } >=20 > /* add all the MAC and VLAN back */ > + rte_spinlock_lock(&vsi->mac_list_lock); > ret =3D 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); >=20 > /* 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) > !=3D I40E_SUCCESS) > PMD_DRV_LOG(WARNING, "Delete MAC failed"); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); >=20 > return 0; > } > @@ -609,7 +615,9 @@ > ether_addr_copy(&null_mac_addr, &vf->mac_addr); >=20 > /* Remove the mac */ > + rte_spinlock_lock(&vsi->mac_list_lock); > i40e_vsi_delete_mac(vsi, mac_addr); > + rte_spinlock_unlock(&vsi->mac_list_lock); >=20 > return 0; > } > @@ -764,6 +772,7 @@ int rte_pmd_i40e_set_vf_broadcast(uint16_t port, uint= 16_t vf_id, > return -EINVAL; > } >=20 > + rte_spinlock_lock(&vsi->mac_list_lock); > if (on) { > rte_memcpy(&filter.mac_addr, &broadcast, ETHER_ADDR_LEN); > filter.filter_type =3D RTE_MACVLAN_PERFECT_MATCH; > @@ -771,6 +780,7 @@ int rte_pmd_i40e_set_vf_broadcast(uint16_t port, uint= 16_t vf_id, > } else { > ret =3D i40e_vsi_delete_mac(vsi, &broadcast); > } > + rte_spinlock_unlock(&vsi->mac_list_lock); >=20 > if (ret !=3D I40E_SUCCESS && ret !=3D I40E_ERR_PARAM) { > ret =3D -ENOTSUP; > @@ -2388,7 +2398,9 @@ int rte_pmd_i40e_ptype_mapping_replace(uint16_t por= t, >=20 > mac_filter.filter_type =3D RTE_MACVLAN_PERFECT_MATCH; > ether_addr_copy(mac_addr, &mac_filter.mac_addr); > + rte_spinlock_lock(&vf->vsi->mac_list_lock); > ret =3D i40e_vsi_add_mac(vsi, &mac_filter); > + rte_spinlock_unlock(&vf->vsi->mac_list_lock); > if (ret !=3D I40E_SUCCESS) { > PMD_DRV_LOG(ERR, "Failed to add MAC filter."); > return -1; > -- > 1.8.3.1 >=20