Will send new version later. Thanks.
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, September 13, 2018 21:27
> To: Li, Xiaoyun <xiaoyun...@intel.com>; Xing, Beilei <beilei.x...@intel.com>;
> Zhang, Qi Z <qi.z.zh...@intel.com>
> Cc: dev@dpdk.org; Yang, Zhiyong <zhiyong.y...@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v5] net/i40e: add interface to choose latest
> vector path
>
> 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.
OK. Will update the commit log.
>
> > 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
OK.
>
>
> 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?
OK. Will take that name.
>
> <...>
>
> > @@ -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?
OK. Will generalize it to users not VPP users.
>
> > +
> > 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.
OK. I am preparing patch about that.
>
> > +
> > + 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()
OK.
>
> <...>
>
> > @@ -12527,4 +12570,5 @@
> RTE_PMD_REGISTER_PARAM_STRING(net_i40e,
> > ETH_I40E_FLOATING_VEB_ARG "=1"
> > ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
> > 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 ?
Yes. Will correct it.
