From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E03EAA09EF for ; Tue, 12 Jan 2021 08:08:24 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D2556140D44; Tue, 12 Jan 2021 08:08:24 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id ED2AB140CE9; Tue, 12 Jan 2021 08:08:21 +0100 (CET) IronPort-SDR: CGi2E5l5imyxLqxO+exNcYEqGl5Ta33rK7U4XhKbXB+a7Z+FFp1qp1pCYV6cfaDLx4K2Impkqa M28FhIVvR8/Q== X-IronPort-AV: E=McAfee;i="6000,8403,9861"; a="196615053" X-IronPort-AV: E=Sophos;i="5.79,340,1602572400"; d="scan'208";a="196615053" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jan 2021 23:08:20 -0800 IronPort-SDR: o169bvZybQ86SuLXUjD0KwTgO3oibvS7/AJI6eY5rjT0jGcWmhVy8l5GYj5T8++PQ8THmzWnl9 U738ycqyDWGQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.79,340,1602572400"; d="scan'208";a="351793553" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga006.jf.intel.com with ESMTP; 11 Jan 2021 23:08:20 -0800 Received: from shsmsx602.ccr.corp.intel.com (10.109.6.142) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 11 Jan 2021 23:08:19 -0800 Received: from shsmsx603.ccr.corp.intel.com (10.109.6.143) by SHSMSX602.ccr.corp.intel.com (10.109.6.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 12 Jan 2021 15:08:17 +0800 Received: from shsmsx603.ccr.corp.intel.com ([10.109.6.143]) by SHSMSX603.ccr.corp.intel.com ([10.109.6.143]) with mapi id 15.01.1713.004; Tue, 12 Jan 2021 15:08:17 +0800 From: "Yu, DapengX" To: "Wu, Jingjing" , "Xu, Ting" , "Zhang, Qi Z" , "Xing, Beilei" CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH] net/iavf: fix vector id assignment Thread-Index: AQHW3niNwiU9Aex5OkiIEAB/XvgsLaodDOuAgAYH3gCAAATLgIAAiwdw Date: Tue, 12 Jan 2021 07:08:17 +0000 Message-ID: <5c614223a23046c098f5bcf3c03cedff@intel.com> References: <20201230065347.90115-1-dapengx.yu@intel.com> <20210108102111.7519-1-dapengx.yu@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-stable] [PATCH] net/iavf: fix vector id assignment X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi Jingjing, I double checked that the max_vectors assignment statement is already ahead= of RTE_MIN. And this patch do the same thing as http://patchwork.dpdk.org/patch/86118/.= Since patch 86118 is ahead of this one, I guess merging 86118 is preferre= d. -----Original Message----- From: Wu, Jingjing=20 Sent: Tuesday, January 12, 2021 2:44 PM To: Xu, Ting ; Yu, DapengX ; Zhang= , Qi Z ; Xing, Beilei Cc: dev@dpdk.org; Yu, DapengX ; stable@dpdk.org Subject: RE: [PATCH] net/iavf: fix vector id assignment > -----Original Message----- > From: Xu, Ting > Sent: Tuesday, January 12, 2021 2:27 PM > To: Yu, DapengX ; Zhang, Qi Z=20 > ; Wu, Jingjing ; Xing,=20 > Beilei > Cc: dev@dpdk.org; Yu, DapengX ; stable@dpdk.org > Subject: RE: [PATCH] net/iavf: fix vector id assignment >=20 > > -----Original Message----- > > From: dapengx.yu@intel.com > > Sent: Friday, January 8, 2021 6:21 PM > > To: Zhang, Qi Z ; Wu, Jingjing=20 > > ; Xing, Beilei ; Xu,=20 > > Ting > > Cc: dev@dpdk.org; Yu, DapengX ; > stable@dpdk.org > > Subject: [PATCH] net/iavf: fix vector id assignment > > > > From: YU DAPENG > > > > The number of MSI-X interrupts on Rx shall be the minimal value of=20 > > the number of available MSI-X interrupts per VF - 1 (the 1 is for=20 > > miscellaneous > > interrupt) and the number of configured Rx queues. > > The current code break the rule because the number of available=20 > > MSI-X interrupts is used as the first value, but code does not subtract= 1 from it. > > > > In normal situation, the first value is larger than the second value. > > So each queue can be assigned a unique vector_id. > > > > For example: 17 available MSI-X interrupts, and 16 available Rx=20 > > queues per VF; but only 4 Rx queues are configured when device is start= ed. > > vector_id:0 is for misc interrupt, vector_id:1 for Rx queue0, > > vector_id:2 for Rx queue1, vector_id:3 for Rx queue2, vector_id:4=20 > > for Rx queue3. > > > > Current code breaks the rule in this normal situation, because when=20 > > assign vector_ids to interrupt handle, for example, it does not=20 > > assign > > vector_id:4 to the queue3, but assign vector_id:1 to it, because the=20 > > condition used causes vector_id wrap around too early. > > >=20 > Hi, Dapeng, >=20 > Could you please further explain in which condition will this error happe= n? > Seems it requires vf->nb_msix =3D 3 to make it happen, but I do not=20 > notice such situation. > I know it may be an example, is there any more specific case? >=20 > Thanks. >=20 > > In iavf_config_irq_map(), the current code does not write data into=20 > > the last element of vecmap[], because of the previous code break. > > Which cause wrong data is sent to PF with opcode=20 > > VIRTCHNL_OP_CONFIG_IRQ_MAP and cause > > error: VIRTCHNL_STATUS_ERR_PARAM(-5). > > > > If kernel driver supports large VFs (up to 256 queues), different=20 > > queues can be assigned same vector_id. > > > > In order to adapt to large VFs and avoid wrapping early, the=20 > > condition is replaced from vec >=3D vf->nb_msix to vec >=3D vf->vf_res-= >max_vectors. > > > > Fixes: d6bde6b5eae9 ("net/avf: enable Rx interrupt") > > Cc: stable@dpdk.org > > > > Signed-off-by: YU DAPENG > > --- > > drivers/net/iavf/iavf_ethdev.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/iavf/iavf_ethdev.c=20 > > b/drivers/net/iavf/iavf_ethdev.c index 7e3c26a94..d730bb156 100644 > > --- a/drivers/net/iavf/iavf_ethdev.c > > +++ b/drivers/net/iavf/iavf_ethdev.c > > @@ -483,6 +483,7 @@ static int iavf_config_rx_queues_irqs(struct=20 > > rte_eth_dev *dev, struct iavf_qv_map *qv_map; uint16_t interval,=20 > > i; int vec; > > +uint16_t max_vectors; > > > > if (rte_intr_cap_multiple(intr_handle) && > > dev->data->dev_conf.intr_conf.rxq) { @@ -570,15 +571,16 @@=20 > > static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev, > > /* If Rx interrupt is reuquired, and we can use > > * multi interrupts, then the vec is from 1 > > */ > > -vf->nb_msix =3D RTE_MIN(vf->vf_res->max_vectors, > > - intr_handle->nb_efd); > > +max_vectors =3D > > +vf->vf_res->max_vectors - > > IAVF_RX_VEC_START; Looks it is the same fix as http://patchwork.dpdk.org/patch/86118 /. I think this line need to be moved to ahead of RTE_MIN? And the RTE_MIN(max= _vectors, intr_handle->nb_efd); > > +vf->nb_msix =3D RTE_MIN(max_vectors, intr_handle- > > >nb_efd); > > vf->msix_base =3D IAVF_RX_VEC_START; > > vec =3D IAVF_RX_VEC_START; > > for (i =3D 0; i < dev->data->nb_rx_queues; i++) { qv_map[i].queue_id= =20 > > =3D i; qv_map[i].vector_id =3D vec; intr_handle->intr_vec[i] =3D vec+= +;=20 > > -if (vec >=3D vf->nb_msix) > > +if (vec >=3D vf->vf_res->max_vectors) > > vec =3D IAVF_RX_VEC_START; > > } > > vf->qv_map =3D qv_map; > > -- > > 2.27.0