From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 73A057EB4 for ; Thu, 13 Sep 2018 15:27:16 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2018 06:27:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,369,1531810800"; d="scan'208";a="88443871" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.56]) ([10.237.221.56]) by fmsmga004.fm.intel.com with ESMTP; 13 Sep 2018 06:27:13 -0700 To: Xiaoyun Li , beilei.xing@intel.com, qi.z.zhang@intel.com Cc: dev@dpdk.org, zhiyong.yang@intel.com References: <1535595399-430873-1-git-send-email-xiaoyun.li@intel.com> <20180912101202.37950-1-xiaoyun.li@intel.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: <4958c6fd-2c51-0d36-a079-a3c4406a6b68@intel.com> Date: Thu, 13 Sep 2018 14:27:11 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180912101202.37950-1-xiaoyun.li@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5] 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: Thu, 13 Sep 2018 13:27:17 -0000 On 9/12/2018 11:12 AM, Xiaoyun Li wrote: > Right now, vector path is limited to only use on later platform. i40e supports vector instructions for intel, arm and powerpc, this behavior is only for Intel vector drivers, can be good to clarify, also it can better to explain a little more what "limited to only use on later platform" means. > This patch adds a devarg use-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. Again, this is for intel only, and can you put a matrix to clarify what is supported: no devarg: Machine PMD avx512 avx2 avx2 sse4.2 sse4.2 sse4.2 < sse4.2 not supported with devarg: Machine PMD avx512 avx2 avx2 avx2 sse4.2 sse4.2 < sse4.2 not supported And I am not sure about name of the devarg "use-latest-vec", I can see it has been discussed already. What about "use-latest-supported-vec"? Too verbose? Do you have any other suggestion? <...> > @@ -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. > > +- ``Use 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 ``use-latest-vec`` is > + introduced, for example:: > + -w 84:00.0,use-latest-vec=1 Do we need to name a specific consumer of DPDK on i40e document? Why not say any application? > + > 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..34af591a2 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. > ========================================================= > > +* **Added a devarg to use the latest vector path.** > + A new devarg ``use-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. Same, do we need to name a specific consumer of DPDK on release notes? <...> > @@ -1201,6 +1203,46 @@ i40e_aq_debug_write_global_register(struct i40e_hw *hw, > return i40e_aq_debug_write_register(hw, reg_addr, reg_val, cmd_details); > } > > +static int > +i40e_parse_latest_vec(struct rte_eth_dev *dev) > +{ > + struct i40e_adapter *ad = > + I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private); > + int kvargs_count, use_latest_vec; > + struct rte_kvargs *kvlist; > + > + ad->use_latest_vec = false; > + > + if (!dev->device->devargs) > + return 0; > + > + kvlist = rte_kvargs_parse(dev->device->devargs->args, valid_keys); > + if (!kvlist) > + return -EINVAL; Agree with Qi to prevent rte_kvargs_parse() call for each devarg, in the future. > + > + kvargs_count = rte_kvargs_count(kvlist, ETH_I40E_USE_LATEST_VEC); > + if (!kvargs_count) { > + rte_kvargs_free(kvlist); > + return 0; > + } > + > + if (kvargs_count > 1) > + PMD_DRV_LOG(WARNING, "More than one argument \"%s\" and only " > + "the first one is used !", > + ETH_I40E_USE_LATEST_VEC); > + > + use_latest_vec = atoi((&kvlist->pairs[0])->value); Instead of accessing directly kvlist internals, please use rte_kvargs_process() <...> > @@ -12527,4 +12570,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_i40e, > ETH_I40E_FLOATING_VEB_ARG "=1" > ETH_I40E_FLOATING_VEB_LIST_ARG "=" > ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16" > - ETH_I40E_SUPPORT_MULTI_DRIVER "=1"); > + ETH_I40E_SUPPORT_MULTI_DRIVER "=1" > + ETH_I40E_USE_LATEST_VEC "=1"); = 0|1 ?