From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id D087B2AA0 for ; Fri, 7 Sep 2018 11:23:40 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Sep 2018 02:23:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,341,1531810800"; d="scan'208";a="68263582" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga007.fm.intel.com with ESMTP; 07 Sep 2018 02:23:37 -0700 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 7 Sep 2018 02:23:37 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 7 Sep 2018 02:23:37 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.205]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.143]) with mapi id 14.03.0319.002; Fri, 7 Sep 2018 17:23:35 +0800 From: "Li, Xiaoyun" To: "Zhang, Qi Z" , "Xing, Beilei" CC: "dev@dpdk.org" , "Yang, Zhiyong" , "Richardson, Bruce" , "Hunt, David" Thread-Topic: [PATCH v3] net/i40e: add interface to choose latest vector path Thread-Index: AQHUREUcX+8tWlum30GE0wRfaP0chKThF10AgAN43vA= Date: Fri, 7 Sep 2018 09:23:34 +0000 Message-ID: References: <1535595399-430873-1-git-send-email-xiaoyun.li@intel.com> <20180904113939.80849-1-xiaoyun.li@intel.com> <039ED4275CED7440929022BC67E7061153280503@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <039ED4275CED7440929022BC67E7061153280503@SHSMSX103.ccr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODg0OTI0MGEtNjljYi00ZTE1LTk1ZDMtMDA5NTBhNTZiMjc2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiT0FLQXp1RU1FRDRXQWhtVTBVWHdHZDJoSXF0MlwvaFdPcWlwMnBjZ1NEY0VxK2t5Ykw5OVI2TnNOWTNHNm5XSkMifQ== x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: add interface to choose latest vector path 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: Fri, 07 Sep 2018 09:23:41 -0000 OK. Will send v4 later. Thanks. > -----Original Message----- > From: Zhang, Qi Z > Sent: Wednesday, September 5, 2018 20:22 > To: Li, Xiaoyun ; Xing, Beilei > Cc: dev@dpdk.org; Yang, Zhiyong ; Richardson, > Bruce ; Hunt, David > Subject: RE: [PATCH v3] net/i40e: add interface to choose latest vector p= ath >=20 > Hi Xiaoyun: >=20 > > -----Original Message----- > > From: Li, Xiaoyun > > Sent: Tuesday, September 4, 2018 7:40 PM > > To: Xing, Beilei ; Zhang, Qi Z > > > > Cc: dev@dpdk.org; Yang, Zhiyong ; Richardson, > > Bruce ; Hunt, David > > ; Li, Xiaoyun > > Subject: [PATCH v3] net/i40e: add interface to choose latest vector > > path > > > > Right now, vector path is limited to only use on later platform. > > This patch adds a devarg enable-latest-vec to allow the users to use > > the latest vector path that the platform supported. Namely, using AVX2 > > vector path on broadwell is possible. > > > > Signed-off-by: Xiaoyun Li > > --- > > v3: > > * Polish the doc and commit log. > > v2: > > * Correct the calling of the wrong function last time. > > * Fix seg fault bug. > > > > doc/guides/nics/i40e.rst | 8 ++++++ > > doc/guides/rel_notes/release_18_11.rst | 4 +++ > > drivers/net/i40e/i40e_ethdev.c | 38 > > ++++++++++++++++++++++++++ > > drivers/net/i40e/i40e_ethdev.h | 1 + > > drivers/net/i40e/i40e_rxtx.c | 27 ++++++++++++++++++ > > 5 files changed, 78 insertions(+) > > > > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index > > 65d87f869..6158e7c34 100644 > > --- a/doc/guides/nics/i40e.rst > > +++ b/doc/guides/nics/i40e.rst > > @@ -163,6 +163,14 @@ Runtime Config Options > > Currently hot-plugging of representor ports is not supported so all > required > > representors must be specified on the creation of the PF. > > > > +- ``Enable latest vector`` (default ``disable``) > > + > > + Vector path was limited to use only on later platform. But users > > + may want the latest vector path. For example, VPP users may want to > > + use > > + AVX2 vector path on HSW/BDW because it can get better perf. So > > + ``devargs`` parameter ``enable-latest-vec`` is introduced, for examp= le:: > > + -w 84:00.0,enable-latest-vec=3D1 >=20 >=20 > How about "use_latest_vec" or "use-lastest-vpmd"? >=20 > > + > > Driver compilation and testing > > ------------------------------ > > > > diff --git a/doc/guides/rel_notes/release_18_11.rst > > b/doc/guides/rel_notes/release_18_11.rst > > index 3ae6b3f58..f8b0f3189 100644 > > --- a/doc/guides/rel_notes/release_18_11.rst > > +++ b/doc/guides/rel_notes/release_18_11.rst > > @@ -54,6 +54,10 @@ New Features > > Also, make sure to start the actual text at the margin. > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > +* **Added a devarg to eable the latest vector path.** > > + A new devarg ``enable-latest-vec`` was introduced to allow users to > > +choose > > + the latest vector path that the platform supported. For example, > > +VPP users > > + can use AVX2 vector path on BDW/HSW to get better performance. > > > > API Changes > > ----------- > > diff --git a/drivers/net/i40e/i40e_ethdev.c > > b/drivers/net/i40e/i40e_ethdev.c index 85a6a867f..16b5345fb 100644 > > --- a/drivers/net/i40e/i40e_ethdev.c > > +++ b/drivers/net/i40e/i40e_ethdev.c > > @@ -12513,6 +12513,44 @@ i40e_config_rss_filter(struct i40e_pf *pf, > > return 0; > > } > > > > +#define ETH_I40E_ENABLE_LATEST_VEC "enable-latest-vec" >=20 > This should be defined along with other exist devargs, please check > ETH_I40E_SUPPORT_MULTI_DRIVER for reference Also it should be > registered with RTE_PMD_REGISTER_PARAM_STRING. >=20 > > + > > +bool > > +i40e_parse_latest_vec(struct rte_eth_dev *dev) { > > + static const char *const valid_keys[] =3D { > > + ETH_I40E_ENABLE_LATEST_VEC, NULL}; > > + int enable_latest_vec; > > + struct rte_kvargs *kvlist; > > + > > + if (!dev->device->devargs) > > + return 0; > > + > > + kvlist =3D rte_kvargs_parse(dev->device->devargs->args, valid_keys); > > + if (!kvlist) > > + return -EINVAL; > > + > > + if (!rte_kvargs_count(kvlist, ETH_I40E_ENABLE_LATEST_VEC)) > > + return 0; > > + > > + if (rte_kvargs_count(kvlist, ETH_I40E_ENABLE_LATEST_VEC) > 1) > > + PMD_DRV_LOG(WARNING, "More than one argument \"%s\" > and > > only " > > + "the first one is used !", > > + ETH_I40E_ENABLE_LATEST_VEC); > > + > > + enable_latest_vec =3D atoi((&kvlist->pairs[0])->value); > > + > > + rte_kvargs_free(kvlist); > > + > > + if (enable_latest_vec !=3D 0 && enable_latest_vec !=3D 1) > > + PMD_DRV_LOG(WARNING, "Value should be 0 or 1, set it as > 1!"); > > + > > + if (enable_latest_vec) > > + return true; > > + else > > + return false; > > +} >=20 > We call rte_kvargs_parse in different place for different parameter which= is > not necessary. > it's better to merge them into one parse_devargs function at dev_init and > then all corresponding field of i40e_adapter can be configured at the sam= e > place. > Though this is not this patch's scope, but it's better to introduce a fie= ld like > i40e_adapter->use_latest_vec and in i40e_parse_latest_vec, it just assign= the > value which could be used later. > This will make things easy for future code clean and also it is not neces= sary to > call i40e_parse_latest_vec multiple time in set_rx/tx_function. >=20 > > + > > RTE_INIT(i40e_init_log) > > { > > i40e_logtype_init =3D rte_log_register("pmd.net.i40e.init"); > > diff --git a/drivers/net/i40e/i40e_ethdev.h > > b/drivers/net/i40e/i40e_ethdev.h index 3fffe5a55..cdf68cd93 100644 > > --- a/drivers/net/i40e/i40e_ethdev.h > > +++ b/drivers/net/i40e/i40e_ethdev.h > > @@ -1243,6 +1243,7 @@ int i40e_config_rss_filter(struct i40e_pf *pf, > > struct i40e_rte_flow_rss_conf *conf, bool add); int > > i40e_vf_representor_init(struct rte_eth_dev *ethdev, void > > *init_params); int i40e_vf_representor_uninit(struct rte_eth_dev > > *ethdev); > > +bool i40e_parse_latest_vec(struct rte_eth_dev *dev); > > > > #define I40E_DEV_TO_PCI(eth_dev) \ > > RTE_DEV_TO_PCI((eth_dev)->device) > > diff --git a/drivers/net/i40e/i40e_rxtx.c > > b/drivers/net/i40e/i40e_rxtx.c index > > 2a28ee348..75f8ec284 100644 > > --- a/drivers/net/i40e/i40e_rxtx.c > > +++ b/drivers/net/i40e/i40e_rxtx.c > > @@ -2960,6 +2960,15 @@ i40e_set_rx_function(struct rte_eth_dev *dev) > > if > (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) > > dev->rx_pkt_burst =3D > > i40e_recv_scattered_pkts_vec_avx2; > > + /* > > + * Give users chance to use the latest vector path > > + * that the platform supported. > > + */ > > + if (i40e_parse_latest_vec(dev)) { > > + if > (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2)) > > + dev->rx_pkt_burst =3D > > + i40e_recv_scattered_pkts_vec_avx2; > > + } >=20 >=20 > How about: >=20 > if (adapter->use_latest_vec) > dev->pkt_burst =3D get_latest_rx_vec(); > else > dev->pkt_burst =3D get_recommended_rx_vec(); >=20 > this make code easy to understand and in future, if we have avx512 vpmd, > it's easy for us to modify the code. >=20 >=20 > > #endif > > } else { > > PMD_INIT_LOG(DEBUG, "Using a Scattered with bulk > " > > @@ -2989,6 +2998,15 @@ i40e_set_rx_function(struct rte_eth_dev *dev) > > */ > > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) > > dev->rx_pkt_burst =3D i40e_recv_pkts_vec_avx2; > > + /* > > + * Give users chance to use the latest vector path > > + * that the platform supported. > > + */ > > + if (i40e_parse_latest_vec(dev)) { > > + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2)) > > + dev->rx_pkt_burst =3D > > + i40e_recv_pkts_vec_avx2; > > + } >=20 > Same as above > > #endif > > } else if (ad->rx_bulk_alloc_allowed) { > > PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions > are " > > @@ -3083,6 +3101,15 @@ i40e_set_tx_function(struct rte_eth_dev *dev) > > */ > > if > (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F)) > > dev->tx_pkt_burst =3D > i40e_xmit_pkts_vec_avx2; > > + /* > > + * Give users chance to use the latest vector path > > + * that the platform supported. > > + */ > > + if (i40e_parse_latest_vec(dev)) { > > + if > (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2)) > > + dev->tx_pkt_burst =3D > > + i40e_xmit_pkts_vec_avx2; > > + } >=20 > Same as above >=20 > > #endif > > } else { > > PMD_INIT_LOG(DEBUG, "Simple tx finally be used."); > > -- > > 2.17.1 >=20 > Regards > Qi