Re: [dpdk-dev] [PATCH v4 1/4] regexdev: introduce regexdev subsystem

[Ori Kam]

Hi Thomas,

> -Original Message-
> From: Thomas Monjalon 
> Sent: Monday, July 6, 2020 12:19 AM
> To: jer...@marvell.com; Ori Kam 
> Cc: xiang.w.w...@intel.com; dev@dpdk.org; g...@marvell.com;
> pbhagavat...@marvell.com; Shahaf Shuler ;
> hemant.agra...@nxp.com; Opher Reviv ; Alex
> Rosenbaum ; dov...@marvell.com;
> pkap...@marvell.com; nipun.gu...@nxp.com; bruce.richard...@intel.com;
> yang.a.h...@intel.com; harry.ch...@intel.com; gu.ji...@zte.com.cn;
> shanjia...@chinatelecom.cn; zhangy@chinatelecom.cn;
> lixin...@huachentel.com; wush...@inspur.com; yuying...@yxlink.com;
> fanchengg...@sunyainfo.com; davidf...@tencent.com;
> liuzho...@chinaunicom.cn; zhaoyon...@huawei.com; o...@yunify.com;
> j...@netgate.com; hongjun...@intel.com; d...@ntop.org; f...@napatech.com;
> arthur...@lionic.com
> Subject: Re: [dpdk-dev] [PATCH v4 1/4] regexdev: introduce regexdev
> subsystem
> 
> 02/07/2020 09:46, Ori Kam:
> > From: Jerin Jacob 
> > --- a/config/common_base
> > +++ b/config/common_base
> >  #
> > +# Compile regex device support
> > +#
> > +CONFIG_RTE_LIBRTE_REGEXDEV=y
> > +
> > +#
> >  # Compile librte_ring
> >  #
> >  CONFIG_RTE_LIBRTE_RING=y
> > @@ -1141,3 +1146,4 @@ CONFIG_RTE_APP_CRYPTO_PERF=y
> >  # Compile the eventdev application
> >  #
> >  CONFIG_RTE_APP_EVENTDEV=y
> > +
> 
> Why this empty line?
> 
Sure.

> > --- a/doc/api/doxy-api-index.md
> > +++ b/doc/api/doxy-api-index.md
> > @@ -26,6 +26,7 @@ The public API headers are grouped by topics:
> >[event_timer_adapter](@ref rte_event_timer_adapter.h),
> >[event_crypto_adapter]   (@ref rte_event_crypto_adapter.h),
> >[rawdev] (@ref rte_rawdev.h),
> > +  [regexdev]   (@ref rte_regexdev.h),
> >[metrics](@ref rte_metrics.h),
> 
> Please move regexdev after [compress].
> 

Sure.

> > --- a/doc/api/doxy-api.conf.in
> > +++ b/doc/api/doxy-api.conf.in
> > @@ -61,6 +61,7 @@ INPUT   = @TOPDIR@/doc/api/doxy-api-
> index.md \
> >@TOPDIR@/lib/librte_rcu \
> >@TOPDIR@/lib/librte_reorder \
> >@TOPDIR@/lib/librte_rib \
> > +  @TOPDIR@/lib/librte_regexdev \
> >@TOPDIR@/lib/librte_ring \
> 
> What is the ordering here?
> 
Will move it above rib.

> > --- a/doc/guides/prog_guide/index.rst
> > +++ b/doc/guides/prog_guide/index.rst
> > @@ -72,3 +72,4 @@ Programmer's Guide
> >  lto
> >  profile_app
> >  glossary
> > +regexdev_lib
> 
> Why adding it at the end?
> I would suggest adding regexdev after compressdev.
> 
Sure.

> > --- /dev/null
> > +++ b/doc/guides/prog_guide/regexdev_lib.rst
> 
> Please remove the _lib suffix in this filename.
> 
Sure.

> [...]
> > +The dequeue API uses the same format as the enqueue API of processed but
> > +the ``nb_ops`` and ``ops`` parameters are now used to specify the max
> processed
> > +operations the user wishes to retrieve and the location in which to store
> them.
> > +The API call returns the actual number of processed operations returned,
> this
> > +can never be larger than ``nb_ops``.
> > +
> 
> Please avoid empty line at end of files.
> 
Sure.

> > --- /dev/null
> > +++ b/lib/librte_regexdev/Makefile
> > @@ -0,0 +1,31 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(C) 2019 Marvell International Ltd.
> > +# Copyright(C) 2020 Mellanox International Ltd.
> 
> Mellanox copyright is wrong. It should be:
> Copyright 2020 Mellanox Technologies, Ltd
>
Will fix.
 
> > --- /dev/null
> > +++ b/lib/librte_regexdev/meson.build
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2020 Mellanox Corporation
> 
> Wrong copyright. Please check them all.
> 
Will fix.

> > +
> > +allow_experimental_apis = true
> 
> Internal libraries do not need such flag anymore.
> 
Will remove.

> > +sources = files('rte_regexdev.c')
> > +headers = files('rte_regexdev.h')
> > +deps += ['mbuf']
> [...]
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -25,7 +25,7 @@ libraries = [
> > 'gro', 'gso', 'ip_frag', 'jobstats',
> > 'kni', 'latencystats', 'lpm', 'member',
> > 'power', 'pdump', 'rawdev',
> > -   'rib', 'reorder', 'sched', 'security', 'stack', 'vhost',
> > +   'regexdev', 'rib', 'reorder', 'sched', 'security', 'stack', 'vhost',
> 
> strange choice :)
> I would have added regex at the end of the previous line which is shorter.
> 
Will change.
> 

Re: [dpdk-dev] [PATCH v6 1/3] eal: disable function versioning on Windows

[Fady Bader]

> -Original Message-
> From: Thomas Monjalon 
> Sent: Sunday, July 5, 2020 11:24 PM
> To: Fady Bader 
> Cc: dev@dpdk.org; Tasnim Bashar ; Tal Shnaiderman
> ; Yohad Tor ;
> dmitry.kozl...@gmail.com; harini.ramakrish...@microsoft.com;
> ocard...@microsoft.com; pallavi.ka...@intel.com; ranjit.me...@intel.com;
> olivier.m...@6wind.com; arybche...@solarflare.com; m...@ashroe.eu;
> nhor...@tuxdriver.com
> Subject: Re: [dpdk-dev] [PATCH v6 1/3] eal: disable function versioning on
> Windows
> 
> 05/07/2020 15:47, Fady Bader:
> > Function versioning implementation is not supported by Windows.
> > Function versioning was disabled on Windows.
> 
> was -> is
> 
> > Signed-off-by: Fady Bader 
> > ---
> >  lib/librte_eal/include/rte_function_versioning.h | 2 +-
> >  lib/meson.build  | 5 +
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> As suggested by Ray, we should add a note in the documentation about the ABI
> compatibility. Because we have no function versioning, we cannot ensure ABI
> compatibility on Windows.
> 
> I recommend adding this text in doc/guides/windows_gsg/intro.rst under
> "Limitations":
> "
> The :doc:`../contributing/abi_policy` cannot be respected for Windows.
> Minor ABI versions may be incompatible
> because function versioning is not supported on Windows.
> "

Ok, I'll send a new patch with the changes soon.

> 

Re: [dpdk-dev] [PATCH v4 0/4] add RegEx class

[Ori Kam]

Hi Thomas

> -Original Message-
> From: Thomas Monjalon 
> Subject: Re: [dpdk-dev] [PATCH v4 0/4] add RegEx class
> 
> 02/07/2020 09:45, Ori Kam:
> >  config/common_base   |8 +
> >  config/meson.build   |1 +
> >  doc/api/doxy-api-index.md|1 +
> >  doc/api/doxy-api.conf.in |1 +
> >  doc/guides/prog_guide/index.rst  |1 +
> >  doc/guides/prog_guide/regexdev_lib.rst   |  177 +++
> >  lib/Makefile |2 +
> >  lib/librte_regexdev/Makefile |   33 +
> >  lib/librte_regexdev/meson.build  |   10 +
> >  lib/librte_regexdev/rte_regexdev.c   |  568 ++
> >  lib/librte_regexdev/rte_regexdev.h   | 1534
> ++
> >  lib/librte_regexdev/rte_regexdev_core.h  |  184 +++
> >  lib/librte_regexdev/rte_regexdev_driver.h|   59 +
> >  lib/librte_regexdev/rte_regexdev_version.map |   26 +
> >  lib/meson.build  |2 +-
> >  meson_options.txt|2 +
> 
> When adding a new library, a lot of files have to be modified:
>   - maintainers
>   - doxygen index and config
>   - doc guide
>   - compilation files
>   - new test application
>   - new example application
> 
> The update of MAINTAINERS is missing.
> The applications are missing but I think they can be added later.

Will update MAINTAINERS.
> 
Best,
Ori

Re: [dpdk-dev] [PATCH v4 3/4] regexdev: add regexdev core functions

[Thomas Monjalon]

06/07/2020 08:07, Ori Kam:
> From: Thomas Monjalon 
> > 02/07/2020 09:46, Ori Kam:
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -30,6 +30,8 @@ option('max_lcores', type: 'integer', value: 128,
> > >   description: 'maximum number of cores/threads supported by EAL')
> > >  option('max_numa_nodes', type: 'integer', value: 4,
> > >   description: 'maximum number of NUMA nodes supported by EAL')
> > > +option('max_regexdev_devs', type: 'integer', value: 32,
> > > + description: 'maximum number of RegEx devices')
> > 
> > Do we really want to add such option in meson?
> > Some other classes do not expose any option here.
> > I tend to think it should be hidden.
> > If the max is really varying, it should be dynamic.
> > By the way, the maximum number of ethdev ports should be made infinite
> > with a dynamic array.
> > 
> > Bruce, any opinion please?
> > 
> Why this is just like ethdev:
> option('max_ethports', type: 'integer', value: 32,
> description: 'maximum number of Ethernet devices')

Ethdev is the only class exposing such option.
And we already have some requests to replace it with
on-demand runtime dynamic size.
That's why it's better not exposing such bad config.
Anyway we are probably not going to need more than 32 regex engines
in the near future. It gives us time to switch to a dynamic model.

[dpdk-dev] [PATCH v6 2/2] ethdev: fix VLAN offloads set if no relative capabilities

[Wei Hu (Xavier)]

Currently, there is a potential problem that calling the API function
rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the
driver does not support. If the PMD driver does not support certain VLAN
hardware offloads and does not check for it, the hardware setting will
not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads
will be turned on.

It is supposed to check the hardware capabilities to decide whether the
relative callback needs to be called just like the behavior in the API
function named rte_eth_dev_configure. And it is also needed to cleanup
duplicated checks which are done in some PMDs. Also, note that it is
behaviour change for some PMDs which simply ignore (with error/warning log
message) unsupported VLAN offloads, but now it will fail.

Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
Fixes: 0ebce6129bc6 ("net/dpaa2: support new ethdev offload APIs")
Fixes: f9416bbafd98 ("net/enic: remove VLAN filter handler")
Fixes: 4f7d9e383e5c ("fm10k: update vlan offload features")
Fixes: fdba3bf15c7b ("net/hinic: add VLAN filter and offload")
Fixes: b96fb2f0d22b ("net/i40e: handle QinQ strip")
Fixes: d4a27a3b092a ("nfp: add basic features")
Fixes: 56139e85abec ("net/octeontx: support VLAN filter offload")
Fixes: ba1b3b081edf ("net/octeontx2: support VLAN offloads")
Fixes: d87246a43759 ("net/qede: enable and disable VLAN filtering")
Cc: sta...@dpdk.org

Signed-off-by: Chengchang Tang 
Signed-off-by: Wei Hu (Xavier) 
Acked-by: Andrew Rybchenko 
Acked-by: Hyong Youb Kim 
Acked-by: Sachin Saxena 
Acked-by: Xiaoyun wang 
Acked-by: Harman Kalra 
---
v5 -> v6: add the related history patch into the Fixes commit log.
v4 -> v5: no change.
v3 -> v4: Delete "next_mask" label and modify the function that when the
  offload is not supported the function fail.
v2 -> v3: Add __rte_unused to avoid unused parameter 'dev' and 'mask'
  warning.
v1 -> v2: Cleanup duplicated checks which are done in some PMDs.
---
 drivers/net/dpaa2/dpaa2_ethdev.c   | 12 +++-
 drivers/net/enic/enic_ethdev.c | 12 
 drivers/net/fm10k/fm10k_ethdev.c   | 23 ++-
 drivers/net/hinic/hinic_pmd_ethdev.c   |  6 --
 drivers/net/i40e/i40e_ethdev.c |  5 -
 drivers/net/nfp/nfp_net.c  |  5 -
 drivers/net/octeontx/octeontx_ethdev_ops.c | 10 --
 drivers/net/octeontx2/otx2_vlan.c  |  5 -
 drivers/net/qede/qede_ethdev.c |  3 ---
 lib/librte_ethdev/rte_ethdev.c | 21 +
 10 files changed, 26 insertions(+), 76 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index a1f1919..489d744 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -145,7 +145,7 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 {
struct dpaa2_dev_priv *priv = dev->data->dev_private;
struct fsl_mc_io *dpni = dev->process_private;
-   int ret;
+   int ret = 0;
 
PMD_INIT_FUNC_TRACE();
 
@@ -153,7 +153,7 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
/* VLAN Filter not avaialble */
if (!priv->max_vlan_filters) {
DPAA2_PMD_INFO("VLAN filter not available");
-   goto next_mask;
+   return -ENOTSUP;
}
 
if (dev->data->dev_conf.rxmode.offloads &
@@ -166,14 +166,8 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
if (ret < 0)
DPAA2_PMD_INFO("Unable to set vlan filter = %d", ret);
}
-next_mask:
-   if (mask & ETH_VLAN_EXTEND_MASK) {
-   if (dev->data->dev_conf.rxmode.offloads &
-   DEV_RX_OFFLOAD_VLAN_EXTEND)
-   DPAA2_PMD_INFO("VLAN extend offload not supported");
-   }
 
-   return 0;
+   return ret;
 }
 
 static int
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 6a3580f..30a599d 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -367,18 +367,6 @@ static int enicpmd_vlan_offload_set(struct rte_eth_dev 
*eth_dev, int mask)
enic->ig_vlan_strip_en = 0;
}
 
-   if ((mask & ETH_VLAN_FILTER_MASK) &&
-   (offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
-   dev_warning(enic,
-   "Configuration of VLAN filter is not supported\n");
-   }
-
-   if ((mask & ETH_VLAN_EXTEND_MASK) &&
-   (offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)) {
-   dev_warning(enic,
-   "Configuration of extended VLAN is not supported\n");
-   }
-
return enic_set_vlan_strip(enic);
 }
 
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index cb0dd3b..b574693 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/

[dpdk-dev] [PATCH v6 1/2] ethdev: fix data room size verification in Rx queue setup

[Wei Hu (Xavier)]

In the rte_eth_rx_queue_setup API function, the local variable named
mbp_buf_size, which is the data room size of the input parameter mp,
is checked to guarantee that each memory chunck used for net device
in the mbuf is bigger than the min_rx_bufsize. But if mbp_buf_size is
less than RTE_PKTMBUF_HEADROOM, the value of the following  statement
will be a large number since the mbp_buf_size is a unsigned value.
mbp_buf_size - RTE_PKTMBUF_HEADROOM
As a result, it will cause a segment fault in this situation.

This patch fixes it by modify the check condition to guarantee that the
local varibale named mbp_buf_size is bigger than RTE_PKTMBUF_HEADROOM.

Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org

Signed-off-by: Chengchang Tang 
Signed-off-by: Wei Hu (Xavier) 
Reviewed-by: Andrew Rybchenko 
Acked-by: Sachin Saxena 
---
v2 -> v6: No change.
v1 -> v2: Simplify the check condition of mbp_buf_size according to
  Andrew Rybchenko's comment.
---
 lib/librte_ethdev/rte_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index d06b7f9..50c3f18 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1820,7 +1820,7 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t 
rx_queue_id,
}
mbp_buf_size = rte_pktmbuf_data_room_size(mp);
 
-   if ((mbp_buf_size - RTE_PKTMBUF_HEADROOM) < dev_info.min_rx_bufsize) {
+   if (mbp_buf_size < dev_info.min_rx_bufsize + RTE_PKTMBUF_HEADROOM) {
RTE_ETHDEV_LOG(ERR,
"%s mbuf_data_room_size %d < %d 
(RTE_PKTMBUF_HEADROOM=%d + min_rx_bufsize(dev)=%d)\n",
mp->name, (int)mbp_buf_size,
-- 
2.7.4

[dpdk-dev] [PATCH v6 0/2] ethdev: minor bugfixes

[Wei Hu (Xavier)]

This series are minor bugfixes for rte_ethdev.c.

Wei Hu (Xavier) (2):
  ethdev: fix data room size verification in Rx queue setup
  ethdev: fix VLAN offloads set if no relative capabilities

 drivers/net/dpaa2/dpaa2_ethdev.c   | 12 +++-
 drivers/net/enic/enic_ethdev.c | 12 
 drivers/net/fm10k/fm10k_ethdev.c   | 23 ++-
 drivers/net/hinic/hinic_pmd_ethdev.c   |  6 --
 drivers/net/i40e/i40e_ethdev.c |  5 -
 drivers/net/nfp/nfp_net.c  |  5 -
 drivers/net/octeontx/octeontx_ethdev_ops.c | 10 --
 drivers/net/octeontx2/otx2_vlan.c  |  5 -
 drivers/net/qede/qede_ethdev.c |  3 ---
 lib/librte_ethdev/rte_ethdev.c | 23 ++-
 10 files changed, 27 insertions(+), 77 deletions(-)

-- 
2.7.4

Re: [dpdk-dev] [dpdk-stable] [PATCH] bugfix: rte_raw_checksum

[Olivier Matz]

Hi Hongzhi,

I suggest the following title instead:

  net: fix checksum on big endian CPUs

On Wed, Jun 24, 2020 at 05:11:19PM +0200, Morten Brørup wrote:
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Wednesday, June 24, 2020 5:04 PM
> > 
> > 24/06/2020 15:00, Morten Brørup:
> > > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > Sent: Wednesday, June 24, 2020 2:22 PM
> > > >
> > > > 27/05/2020 15:40, guohongzhi:
> > > > > From: Hongzhi Guo 
> > > > >
> > > > > __rte_raw_cksum should consider Big Endian.
> > > >
> > > > We need to explain the logic in the commit log.

Here is a suggestion of commit log:

  With current code, the checksum of odd-length buffers is wrong on
  big endian CPUs: the last byte is not properly summed to the
  accumulator.

  Fix this by left-shifting the remaining byte by 8. For instance,
  if the last byte is 0x42, we should add 0x4200 to the accumulator
  on big endian CPUs.

  This change is similar to what is suggested in Errata 3133 of
  RFC 1071.

Can you please submit a new version with the 2 changes above?

> > >
> > > Having grown up with big endian CPUs, reading the final byte like
> > this is obvious to me. I struggle understanding the little endian way
> > of reading the last byte. (Not really anymore, but back when little
> > endian was unfamiliar to me I would have struggled.)
> > >
> > > An RFC (I can't remember which) describes why the same checksum
> > calculation code works on both big and little endian CPUs. Is it this
> > explanation you are asking for?
> > 
> > This explanation may be interesting.
> > 
> 
> RFC 1071, especially chapter 3.
> 
> Please note that big endian is considered "Normal" order in the RFC. :-)

There is an errata for this RFC about the C code:
see https://www.rfc-editor.org/errata/eid3133

> > 
> > > > > Signed-off-by: Hongzhi Guo 
> > > > > ---
> > > > > +#if (RTE_BYTE_ORDER == RTE_BIG_ENDIAN)
> > > > > + sum += *((const uint8_t *)u16_buf) << 8;
> > > > > +#else
> > > > >   sum += *((const uint8_t *)u16_buf);
> > > > > +#endif
> > > >
> > > > *((const uint8_t *)u16_buf) should be an uint8_t.
> > > > What is the expected behaviour of shifting 8 bits of a byte?
> > >
> > > Yes, the value will be an uint8_t type. But the shift operation will
> > cause the compiler to promote the type to int before shifting it.
> > 
> > This is the explanation I was looking for :-)
> > 
> > 
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH] bugfix: rte_raw_checksum

[Olivier Matz]

On Mon, Jul 06, 2020 at 09:36:25AM +0200, Olivier Matz wrote:
> Hi Hongzhi,
> 
> I suggest the following title instead:
> 
>   net: fix checksum on big endian CPUs
> 
> On Wed, Jun 24, 2020 at 05:11:19PM +0200, Morten Brørup wrote:
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon
> > > Sent: Wednesday, June 24, 2020 5:04 PM
> > > 
> > > 24/06/2020 15:00, Morten Brørup:
> > > > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > > Sent: Wednesday, June 24, 2020 2:22 PM
> > > > >
> > > > > 27/05/2020 15:40, guohongzhi:
> > > > > > From: Hongzhi Guo 
> > > > > >
> > > > > > __rte_raw_cksum should consider Big Endian.
> > > > >
> > > > > We need to explain the logic in the commit log.
> 
> Here is a suggestion of commit log:
> 
>   With current code, the checksum of odd-length buffers is wrong on
>   big endian CPUs: the last byte is not properly summed to the
>   accumulator.
> 
>   Fix this by left-shifting the remaining byte by 8. For instance,
>   if the last byte is 0x42, we should add 0x4200 to the accumulator
>   on big endian CPUs.
> 
>   This change is similar to what is suggested in Errata 3133 of
>   RFC 1071.
> 
> Can you please submit a new version with the 2 changes above?

One more thing, please also add:

  Fixes: 6006818cfb26 ("net: new checksum functions")
  Cc: sta...@dpdk.org

Thanks
Olivier

> 
> > > >
> > > > Having grown up with big endian CPUs, reading the final byte like
> > > this is obvious to me. I struggle understanding the little endian way
> > > of reading the last byte. (Not really anymore, but back when little
> > > endian was unfamiliar to me I would have struggled.)
> > > >
> > > > An RFC (I can't remember which) describes why the same checksum
> > > calculation code works on both big and little endian CPUs. Is it this
> > > explanation you are asking for?
> > > 
> > > This explanation may be interesting.
> > > 
> > 
> > RFC 1071, especially chapter 3.
> > 
> > Please note that big endian is considered "Normal" order in the RFC. :-)
> 
> There is an errata for this RFC about the C code:
> see https://www.rfc-editor.org/errata/eid3133
> 
> > > 
> > > > > > Signed-off-by: Hongzhi Guo 
> > > > > > ---
> > > > > > +#if (RTE_BYTE_ORDER == RTE_BIG_ENDIAN)
> > > > > > +   sum += *((const uint8_t *)u16_buf) << 8;
> > > > > > +#else
> > > > > > sum += *((const uint8_t *)u16_buf);
> > > > > > +#endif
> > > > >
> > > > > *((const uint8_t *)u16_buf) should be an uint8_t.
> > > > > What is the expected behaviour of shifting 8 bits of a byte?
> > > >
> > > > Yes, the value will be an uint8_t type. But the shift operation will
> > > cause the compiler to promote the type to int before shifting it.
> > > 
> > > This is the explanation I was looking for :-)
> > > 
> > > 
> > 

[dpdk-dev] [PATCH v2 1/6] lib/eal: add a common wrapper for restricted pointers

[Joyce Kong]

The 'restrict' keyword is recognized in C99, while type qulifier
'__restrict' compiles ok in C with all language levels. This patch
is to add a wrapper defining '__rte_restrict' with 'restrict' and
'__restrict' to be supported by all compilers.

Signed-off-by: Joyce Kong 
---
 lib/librte_eal/include/rte_common.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_eal/include/rte_common.h 
b/lib/librte_eal/include/rte_common.h
index 0843ce69e..cda32c056 100644
--- a/lib/librte_eal/include/rte_common.h
+++ b/lib/librte_eal/include/rte_common.h
@@ -103,6 +103,16 @@ typedef uint16_t unaligned_uint16_t;
  */
 #define __rte_unused __attribute__((__unused__))
 
+/**
+ * Define a wrapper for restricted pointers which can be supported
+ * by all compilers.
+ */
+#if __STDC_VERSION__ >= 199901
+#define __rte_restrict restrict
+#else
+#define __rte_restrict __restrict
+#endif
+
 /**
  * definition to mark a variable or function parameter as used so
  * as to avoid a compiler warning
-- 
2.27.0

[dpdk-dev] [PATCH v2 2/6] net/virtio: restrict pointer aliasing for NEON vpmd

[Joyce Kong]

Restrict pointer aliasing to allow the compiler to vectorize loops
more aggressively.

With this patch, a 9.6% improvement is observed in throughput for
the virtio-net PVP case, and a 2.4% perf improvement in throughput
for the virtio-user PVP case. All performance data are measured
under the 0.001% acceptable packet loss with 2 cores on the vhost
side.

Signed-off-by: Joyce Kong 
Reviewed-by: Phil Yang 
---
 drivers/net/virtio/virtio_rxtx_simple_neon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c 
b/drivers/net/virtio/virtio_rxtx_simple_neon.c
index 363e2b330..5febfb0f5 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
@@ -36,8 +36,8 @@
  * - nb_pkts < RTE_VIRTIO_DESC_PER_LOOP, just return no packet
  */
 uint16_t
-virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
-   uint16_t nb_pkts)
+virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf
+   **__rte_restrict rx_pkts, uint16_t nb_pkts)
 {
struct virtnet_rx *rxvq = rx_queue;
struct virtqueue *vq = rxvq->vq;
-- 
2.27.0

[dpdk-dev] [PATCH v2 3/6] lib/vhost: restrict pointer aliasing for packed vpmd

[Joyce Kong]

Restrict pointer aliasing to allow the compiler to vectorize loop
more aggressively.

With this patch, a 9.6% improvement is observed in throughput for
the packed virtio-net PVP case, and a 2.8% improvement in throughput
for the packed virtio-user PVP case. All performance data are measured
under 0.001% acceptable packet loss with 1 core on both vhost and
virtio side.

Signed-off-by: Joyce Kong 
Reviewed-by: Phil Yang 
---
 drivers/net/virtio/virtio_rxtx_simple_neon.c |  5 +++--
 lib/librte_vhost/virtio_net.c| 14 +++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx_simple_neon.c 
b/drivers/net/virtio/virtio_rxtx_simple_neon.c
index 5febfb0f5..31824a931 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_neon.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_neon.c
@@ -36,8 +36,9 @@
  * - nb_pkts < RTE_VIRTIO_DESC_PER_LOOP, just return no packet
  */
 uint16_t
-virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf
-   **__rte_restrict rx_pkts, uint16_t nb_pkts)
+virtio_recv_pkts_vec(void *rx_queue,
+   struct rte_mbuf **__rte_restrict rx_pkts,
+   uint16_t nb_pkts)
 {
struct virtnet_rx *rxvq = rx_queue;
struct virtqueue *vq = rxvq->vq;
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 751c1f373..e60358251 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -1133,8 +1133,8 @@ virtio_dev_rx_single_packed(struct virtio_net *dev,
 
 static __rte_noinline uint32_t
 virtio_dev_rx_packed(struct virtio_net *dev,
-struct vhost_virtqueue *vq,
-struct rte_mbuf **pkts,
+struct vhost_virtqueue *__rte_restrict vq,
+struct rte_mbuf **__rte_restrict pkts,
 uint32_t count)
 {
uint32_t pkt_idx = 0;
@@ -1219,7 +1219,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
 
 uint16_t
 rte_vhost_enqueue_burst(int vid, uint16_t queue_id,
-   struct rte_mbuf **pkts, uint16_t count)
+   struct rte_mbuf **__rte_restrict pkts, uint16_t count)
 {
struct virtio_net *dev = get_device(vid);
 
@@ -2124,9 +2124,9 @@ free_zmbuf(struct vhost_virtqueue *vq)
 
 static __rte_noinline uint16_t
 virtio_dev_tx_packed_zmbuf(struct virtio_net *dev,
-  struct vhost_virtqueue *vq,
+  struct vhost_virtqueue *__rte_restrict vq,
   struct rte_mempool *mbuf_pool,
-  struct rte_mbuf **pkts,
+  struct rte_mbuf **__rte_restrict pkts,
   uint32_t count)
 {
uint32_t pkt_idx = 0;
@@ -2160,9 +2160,9 @@ virtio_dev_tx_packed_zmbuf(struct virtio_net *dev,
 
 static __rte_noinline uint16_t
 virtio_dev_tx_packed(struct virtio_net *dev,
-struct vhost_virtqueue *vq,
+struct vhost_virtqueue *__rte_restrict vq,
 struct rte_mempool *mbuf_pool,
-struct rte_mbuf **pkts,
+struct rte_mbuf **__rte_restrict pkts,
 uint32_t count)
 {
uint32_t pkt_idx = 0;
-- 
2.27.0

[dpdk-dev] [PATCH v2 0/6] Restrict pointer aliasing with a common wrapper

[Joyce Kong]

As the 'restrict' keyword is recognized in C99, this patchset is to
add a wrapper defining '__rte_restrict' which can be supported by
all compilers. Then replace the existing 'restrict' and '__restrict'
in different vpmds, and optimize vhost/virtio with restricted pointer
aliasing for more aggressive loops vectorization.

The vhost/virtio optimization patches were benchmarked by running PVP
case on ThunderX2 platform and showed positive performance results.

Joyce Kong (6):
  lib/eal: add a wrapper to define restricted pointers
  net/virtio: restrict pointer aliasing for NEON vpmd
  lib/vhost: restrict pointer aliasing for packed vpmd
  net/i40e: replace restrict with rte restrict
  examples/performance-thread: replace restrict with wrapper
  net/mlx5: replace restrict keyword with rte restrict

 drivers/net/i40e/i40e_rxtx_vec_neon.c |  17 +-
 drivers/net/mlx5/mlx5_rxtx.c  | 208 +-
 drivers/net/virtio/virtio_rxtx_simple_neon.c  |   5 +-
 .../pthread_shim/pthread_shim.c   |  12 +-
 lib/librte_eal/include/rte_common.h   |  10 +
 lib/librte_vhost/virtio_net.c |  14 +-
 6 files changed, 139 insertions(+), 127 deletions(-)

-- 
2.27.0

[dpdk-dev] [PATCH v2 5/6] examples/performance-thread: replace restrict with wrapper

[Joyce Kong]

'__rte_restrict' is a common wrapper for restricted pointers which
can be supported by all compilers. Use '__rte_restrict' instead of
'__restrict' for code consistency.

Signed-off-by: Joyce Kong 
---
 .../performance-thread/pthread_shim/pthread_shim.c   | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/examples/performance-thread/pthread_shim/pthread_shim.c 
b/examples/performance-thread/pthread_shim/pthread_shim.c
index 93e8dca3f..bbc076584 100644
--- a/examples/performance-thread/pthread_shim/pthread_shim.c
+++ b/examples/performance-thread/pthread_shim/pthread_shim.c
@@ -341,9 +341,9 @@ int pthread_cond_signal(pthread_cond_t *cond)
 }
 
 int
-pthread_cond_timedwait(pthread_cond_t *__restrict cond,
-  pthread_mutex_t *__restrict mutex,
-  const struct timespec *__restrict time)
+pthread_cond_timedwait(pthread_cond_t *__rte_restrict cond,
+  pthread_mutex_t *__rte_restrict mutex,
+  const struct timespec *__rte_restrict time)
 {
NOT_IMPLEMENTED;
return _sys_pthread_funcs.f_pthread_cond_timedwait(cond, mutex, time);
@@ -362,10 +362,10 @@ int pthread_cond_wait(pthread_cond_t *cond, 
pthread_mutex_t *mutex)
 }
 
 int
-pthread_create(pthread_t *__restrict tid,
-   const pthread_attr_t *__restrict attr,
+pthread_create(pthread_t *__rte_restrict tid,
+   const pthread_attr_t *__rte_restrict attr,
lthread_func_t func,
-  void *__restrict arg)
+  void *__rte_restrict arg)
 {
if (override) {
int lcore = -1;
-- 
2.27.0

[dpdk-dev] [PATCH v2 4/6] net/i40e: replace restrict with rte restrict

[Joyce Kong]

'__rte_restrict' is a common wrapper for restricted pointers which
can be supported by all compilers. Use '__rte_restrict' instead of
'__restrict' for code consistency.

Signed-off-by: Joyce Kong 
---
 drivers/net/i40e/i40e_rxtx_vec_neon.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx_vec_neon.c 
b/drivers/net/i40e/i40e_rxtx_vec_neon.c
index 1dfd0478b..4574139d5 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_neon.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
@@ -172,8 +172,8 @@ desc_to_olflags_v(struct i40e_rx_queue *rxq, uint64x2_t 
descs[4],
 #define I40E_UINT16_BIT (CHAR_BIT * sizeof(uint16_t))
 
 static inline void
-desc_to_ptype_v(uint64x2_t descs[4], struct rte_mbuf **__restrict rx_pkts,
-   uint32_t *__restrict ptype_tbl)
+desc_to_ptype_v(uint64x2_t descs[4], struct rte_mbuf **__rte_restrict rx_pkts,
+   uint32_t *__rte_restrict ptype_tbl)
 {
int i;
uint8_t ptype;
@@ -194,8 +194,9 @@ desc_to_ptype_v(uint64x2_t descs[4], struct rte_mbuf 
**__restrict rx_pkts,
  *   numbers of DD bits
  */
 static inline uint16_t
-_recv_raw_pkts_vec(struct i40e_rx_queue *__restrict rxq, struct rte_mbuf
-   **__restrict rx_pkts, uint16_t nb_pkts, uint8_t *split_packet)
+_recv_raw_pkts_vec(struct i40e_rx_queue *__rte_restrict rxq,
+  struct rte_mbuf **__rte_restrict rx_pkts,
+  uint16_t nb_pkts, uint8_t *split_packet)
 {
volatile union i40e_rx_desc *rxdp;
struct i40e_rx_entry *sw_ring;
@@ -432,8 +433,8 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *__restrict rxq, 
struct rte_mbuf
  *   numbers of DD bits
  */
 uint16_t
-i40e_recv_pkts_vec(void *__restrict rx_queue,
-   struct rte_mbuf **__restrict rx_pkts, uint16_t nb_pkts)
+i40e_recv_pkts_vec(void *__rte_restrict rx_queue,
+   struct rte_mbuf **__rte_restrict rx_pkts, uint16_t nb_pkts)
 {
return _recv_raw_pkts_vec(rx_queue, rx_pkts, nb_pkts, NULL);
 }
@@ -504,8 +505,8 @@ vtx(volatile struct i40e_tx_desc *txdp, struct rte_mbuf 
**pkt,
 }
 
 uint16_t
-i40e_xmit_fixed_burst_vec(void *__restrict tx_queue,
-   struct rte_mbuf **__restrict tx_pkts, uint16_t nb_pkts)
+i40e_xmit_fixed_burst_vec(void *__rte_restrict tx_queue,
+   struct rte_mbuf **__rte_restrict tx_pkts, uint16_t nb_pkts)
 {
struct i40e_tx_queue *txq = (struct i40e_tx_queue *)tx_queue;
volatile struct i40e_tx_desc *txdp;
-- 
2.27.0

[dpdk-dev] [PATCH v2 6/6] net/mlx5: replace restrict keyword with rte restrict

[Joyce Kong]

The 'restrict' keyword is recognized in C99, which might have
some issues with old compilers. It is better to use the wrapper
'__rte_restrict' which can be supported by all compilers for
restricted pointers.

Signed-off-by: Joyce Kong 
---
 drivers/net/mlx5/mlx5_rxtx.c | 208 +--
 1 file changed, 104 insertions(+), 104 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index e4106bf0a..894f441f3 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -113,13 +113,13 @@ mlx5_queue_state_modify(struct rte_eth_dev *dev,
struct mlx5_mp_arg_queue_state_modify *sm);
 
 static inline void
-mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *restrict tcp,
-   volatile struct mlx5_cqe *restrict cqe,
+mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *__rte_restrict tcp,
+   volatile struct mlx5_cqe *__rte_restrict cqe,
uint32_t phcsum);
 
 static inline void
-mlx5_lro_update_hdr(uint8_t *restrict padd,
-   volatile struct mlx5_cqe *restrict cqe,
+mlx5_lro_update_hdr(uint8_t *__rte_restrict padd,
+   volatile struct mlx5_cqe *__rte_restrict cqe,
uint32_t len);
 
 uint32_t mlx5_ptype_table[] __rte_cache_aligned = {
@@ -374,7 +374,7 @@ mlx5_set_swp_types_table(void)
  *   Software Parser flags are set by pointer.
  */
 static __rte_always_inline uint32_t
-txq_mbuf_to_swp(struct mlx5_txq_local *restrict loc,
+txq_mbuf_to_swp(struct mlx5_txq_local *__rte_restrict loc,
uint8_t *swp_flags,
unsigned int olx)
 {
@@ -747,7 +747,7 @@ check_err_cqe_seen(volatile struct mlx5_err_cqe *err_cqe)
  *   the error completion entry is handled successfully.
  */
 static int
-mlx5_tx_error_cqe_handle(struct mlx5_txq_data *restrict txq,
+mlx5_tx_error_cqe_handle(struct mlx5_txq_data *__rte_restrict txq,
 volatile struct mlx5_err_cqe *err_cqe)
 {
if (err_cqe->syndrome != MLX5_CQE_SYNDROME_WR_FLUSH_ERR) {
@@ -1508,8 +1508,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, 
uint16_t pkts_n)
  *   The L3 pseudo-header checksum.
  */
 static inline void
-mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *restrict tcp,
-   volatile struct mlx5_cqe *restrict cqe,
+mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *__rte_restrict tcp,
+   volatile struct mlx5_cqe *__rte_restrict cqe,
uint32_t phcsum)
 {
uint8_t l4_type = (rte_be_to_cpu_16(cqe->hdr_type_etc) &
@@ -1550,8 +1550,8 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *restrict tcp,
  *   The packet length.
  */
 static inline void
-mlx5_lro_update_hdr(uint8_t *restrict padd,
-   volatile struct mlx5_cqe *restrict cqe,
+mlx5_lro_update_hdr(uint8_t *__rte_restrict padd,
+   volatile struct mlx5_cqe *__rte_restrict cqe,
uint32_t len)
 {
union {
@@ -1965,7 +1965,7 @@ mlx5_check_vec_rx_support(struct rte_eth_dev *dev 
__rte_unused)
  *   compile time and may be used for optimization.
  */
 static __rte_always_inline void
-mlx5_tx_free_mbuf(struct rte_mbuf **restrict pkts,
+mlx5_tx_free_mbuf(struct rte_mbuf **__rte_restrict pkts,
  unsigned int pkts_n,
  unsigned int olx __rte_unused)
 {
@@ -2070,7 +2070,7 @@ mlx5_tx_free_mbuf(struct rte_mbuf **restrict pkts,
  *   compile time and may be used for optimization.
  */
 static __rte_always_inline void
-mlx5_tx_free_elts(struct mlx5_txq_data *restrict txq,
+mlx5_tx_free_elts(struct mlx5_txq_data *__rte_restrict txq,
  uint16_t tail,
  unsigned int olx __rte_unused)
 {
@@ -2111,8 +2111,8 @@ mlx5_tx_free_elts(struct mlx5_txq_data *restrict txq,
  *   compile time and may be used for optimization.
  */
 static __rte_always_inline void
-mlx5_tx_copy_elts(struct mlx5_txq_data *restrict txq,
- struct rte_mbuf **restrict pkts,
+mlx5_tx_copy_elts(struct mlx5_txq_data *__rte_restrict txq,
+ struct rte_mbuf **__rte_restrict pkts,
  unsigned int pkts_n,
  unsigned int olx __rte_unused)
 {
@@ -2148,7 +2148,7 @@ mlx5_tx_copy_elts(struct mlx5_txq_data *restrict txq,
  *   compile time and may be used for optimization.
  */
 static __rte_always_inline void
-mlx5_tx_comp_flush(struct mlx5_txq_data *restrict txq,
+mlx5_tx_comp_flush(struct mlx5_txq_data *__rte_restrict txq,
   volatile struct mlx5_cqe *last_cqe,
   unsigned int olx __rte_unused)
 {
@@ -2179,7 +2179,7 @@ mlx5_tx_comp_flush(struct mlx5_txq_data *restrict txq,
  * routine smaller, simple and faster - from experiments.
  */
 static void
-mlx5_tx_handle_completion(struct mlx5_txq_data *restrict txq,
+mlx5_tx_handle_completion(struct mlx5_txq_data *__rte_restrict txq,
  unsigned int olx __rte_unused)
 {
unsigned int co

[dpdk-dev] [PATCH v2] app/flow-perf: fix condition of hairpin queues setup

[Wisam Jaddo]

The hairpin queue is the one that start from normal rxq,
and will be less than nr_queues where nr_queues is the
sum of normal and hairpin

Fixes: bf3688f1e816 ("app/flow-perf: add insertion rate calculation")
Cc: wis...@mellanox.com

Signed-off-by: Wisam Jaddo 
Reviewed-by: Asaf Penso 

---
v2:
* Add documentation of hairpin peering and allocating logic.
* Add documentation for some variables.
---
 app/test-flow-perf/main.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
index e155e49c37..8f12ee10f1 100644
--- a/app/test-flow-perf/main.c
+++ b/app/test-flow-perf/main.c
@@ -1012,8 +1012,26 @@ init_port(void)
rte_strerror(-ret), port_id);
 
if (hairpinq != 0) {
+   /* Each hairpin queue setup need a hairpin configuration
+* object, which determine the TX path for hairpin.
+*
+* The peering here represent the TX side, which mean 
the
+* peer.port represent TX port, and peer.queue represent
+* tx_queue.
+*
+* So if RXQ=4 and TXQ=4, and first hairpin_q is 4 after
+* [0, 1, 2, 3], then tx_queue is TXQ+i which is 4 as 
well.
+*
+* hairpinq: represent the number of hairpin queues 
needed
+* to be initialized.
+*
+* In 0 case means no hairpin queues needed which is the
+* default.
+*
+* hairpin_q: represent hairpin queue id to be 
initialized.
+*/
for (hairpin_q = RXQ_NUM, std_queue = 0;
-   std_queue < nr_queues;
+   hairpin_q < nr_queues;
hairpin_q++, std_queue++) {
hairpin_conf.peers[0].port = port_id;
hairpin_conf.peers[0].queue =
@@ -1028,7 +1046,7 @@ init_port(void)
}
 
for (hairpin_q = TXQ_NUM, std_queue = 0;
-   std_queue < nr_queues;
+   hairpin_q < nr_queues;
hairpin_q++, std_queue++) {
hairpin_conf.peers[0].port = port_id;
hairpin_conf.peers[0].queue =
-- 
2.17.1

Re: [dpdk-dev] [PATCH] bugfix: udptcp_checksum should tread tcp and udp differently

[Olivier Matz]

Hi,

Here is a suggestion for the title:

  net: fix uneeded replacement of 0 by  for TCP checksum

The commit log looks good to me, but you can add:

  Fixes: 6006818cfb26 ("net: new checksum functions")
  Cc: sta...@dpdk.org

On Wed, May 27, 2020 at 05:36:59PM +0200, Morten Brørup wrote:
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Wednesday, May 27, 2020 5:03 PM
> > 
> > On Wed, 27 May 2020 22:41:27 +0800
> > guohongzhi  wrote:
> >
> > > --- a/lib/librte_net/rte_ip.h
> > > +++ b/lib/librte_net/rte_ip.h
> > > @@ -324,8 +324,7 @@ rte_ipv4_phdr_cksum(const struct rte_ipv4_hdr 
> > > *ipv4_hdr, uint64_t ol_flags)
> > >   * @param l4_hdr
> > >   *   The pointer to the beginning of the L4 header.
> > >   * @return
> > > - *   The complemented checksum to set in the IP packet
> > > - *   or 0 on error
> > > + *   The complemented checksum to set in the IP packet.
> > >   */
> > >  static inline uint16_t
> > >  rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void 
> > > *l4_hdr)

It still returns 0 when ipv4_hdr->total_length < sizeof(struct rte_ipv4_hdr).
I suggest to keep this case in the API comment, maybe like this:

  The complemented checksum to set in the IP packet
  or 0 if the IP length is invalid in the header.


> > > + /* For Udp, if the computed checksum is zero,
> > > +  * it is transmitted as all ones.RFC768
> > > +  */
> > > + if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
> > >   cksum = 0x;
> > >
> > 
> > 
> > The comment should be reformatted to be clearer.
> > 
> > Maybe:
> > /*
> >  * Per RFC768:
> >  * If the computed  checksum  is zero,it is transmitted  as all
> > ones
> >  * (the equivalent  in one's complement  arithmetic).
> >  */
> > 
> 
> But without the double spaces. :-)

I agree, Stephen's wording looks better, can you please use it?

> > There is no special case required here, it is true for TCP as well.
> 
> I disagree. I researched this topic when Hongzhi Guo initially submitted the 
> patches, and have only seen the special exception mentioned in RFC 768 
> (describing UDP), where a transmitted value of 0x means that the checksum 
> has not been generated. None of the RFCs regarding Internet Checksum or TCP 
> checksum mentions this special case.
> 
> > In TCP it appears 0 is allowed but not generally used. Most
> > implementations
> > use common checksum for both TCP and UDP
> 
> Then most implementations are wrong.
> 
> Jon Postel famously wrote: "Be liberal in what you accept, and conservative 
> in what you send."
> 
> This function is in the transmission path, so we should calculate it 
> correctly.
> 
> In the receive path, when checking the checksum as described in RFC 1071, any 
> of the two values (0x or 0x) in the Checksum field will yield the 
> correct result. Which is why most implementations can get away with doing it 
> wrong.

I agree with Morten, it looks more strict like this.

[dpdk-dev] [PATCH] app/flow-perf: fix typo in usage

[Wisam Jaddo]

>From hairping-rss into hairpin-rss

Fixes: bf3688f1e816 ("app/flow-perf: add insertion rate calculation")
Cc: wis...@mellanox.com

Signed-off-by: Wisam Jaddo 
---
 app/test-flow-perf/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
index 8f12ee10f1..cca7785b57 100644
--- a/app/test-flow-perf/main.c
+++ b/app/test-flow-perf/main.c
@@ -136,7 +136,7 @@ usage(char *progname)
printf("  --set-tag: add set tag action in flow actions\n");
printf("  --drop: add drop action in flow actions\n");
printf("  --hairpin-queue=N: add hairpin-queue action in flow 
actions\n");
-   printf("  --hairpin-rss=N: add hairping-rss action in flow actions\n");
+   printf("  --hairpin-rss=N: add hairpin-rss action in flow actions\n");
 }
 
 static void
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4 3/4] regexdev: add regexdev core functions

[Bruce Richardson]

On Mon, Jul 06, 2020 at 09:03:49AM +0200, Thomas Monjalon wrote:
> 06/07/2020 08:07, Ori Kam:
> > From: Thomas Monjalon 
> > > 02/07/2020 09:46, Ori Kam:
> > > > --- a/meson_options.txt
> > > > +++ b/meson_options.txt
> > > > @@ -30,6 +30,8 @@ option('max_lcores', type: 'integer', value: 128,
> > > > description: 'maximum number of cores/threads supported by EAL')
> > > >  option('max_numa_nodes', type: 'integer', value: 4,
> > > > description: 'maximum number of NUMA nodes supported by EAL')
> > > > +option('max_regexdev_devs', type: 'integer', value: 32,
> > > > +   description: 'maximum number of RegEx devices')
> > > 
> > > Do we really want to add such option in meson?
> > > Some other classes do not expose any option here.
> > > I tend to think it should be hidden.
> > > If the max is really varying, it should be dynamic.
> > > By the way, the maximum number of ethdev ports should be made infinite
> > > with a dynamic array.
> > > 
> > > Bruce, any opinion please?
> > > 
> > Why this is just like ethdev:
> > option('max_ethports', type: 'integer', value: 32,
> > description: 'maximum number of Ethernet devices')
> 
> Ethdev is the only class exposing such option.
> And we already have some requests to replace it with
> on-demand runtime dynamic size.
> That's why it's better not exposing such bad config.
> Anyway we are probably not going to need more than 32 regex engines
> in the near future. It gives us time to switch to a dynamic model.
>
I would tend to agree. In general I do not like adding new config options
so I'd prefer this not be exposed unless necessary. For ethdev, since it is
by far the mostly widely used, and since we know that people have some
varying port requirements, we have exposed this, while other classes have
not, indicating that it's probably not necessary for most device
categories.

/Bruce 

[dpdk-dev] [PATCH 1/2] devtools: fix filename in forbidden token check

[David Marchand]

Fix displayed filename by adjusting the extraction from the patch.

Before:
Warning in /lib/librte_eal/linux/eal.c:

After:
Warning in lib/librte_eal/linux/eal.c:

Fixes: 7413e7f2aeb3 ("devtools: alert on new calls to exit from libs")
Cc: sta...@dpdk.org

Signed-off-by: David Marchand 
---
 devtools/check-forbidden-tokens.awk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/check-forbidden-tokens.awk 
b/devtools/check-forbidden-tokens.awk
index 8c89de3d4e..f86cbe8dc1 100755
--- a/devtools/check-forbidden-tokens.awk
+++ b/devtools/check-forbidden-tokens.awk
@@ -62,7 +62,7 @@ BEGIN {
 }
 END {
if (count > 0) {
-   print "Warning in " substr(last_file,6) ":"
+   print "Warning in " substr(last_file,7) ":"
print MESSAGE
exit RET_ON_FAIL
}
-- 
2.23.0

[dpdk-dev] [PATCH 2/2] devtools: fix forbidden token

[David Marchand]

An expression with a space is split by the awk script resulting in
false positive for any patch matching any of the two part of the
expression.
Fix this by using [[:space:]].

Fixes: 43e73483a4b8 ("devtools: forbid variable declaration inside for")

Signed-off-by: David Marchand 
---
 devtools/checkpatches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 27ab1252b7..58021aa5dd 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -71,7 +71,7 @@ check_forbidden_additions() { # 
 
# forbid variable declaration inside "for" loop
awk -v FOLDERS='.' \
-   -v EXPRESSIONS='for *\\((char|u?int|unsigned|s?size_t)' \
+   -v 
EXPRESSIONS='for[[:space:]]*\\((char|u?int|unsigned|s?size_t)' \
-v RET_ON_FAIL=1 \
-v MESSAGE='Declaring a variable inside for()' \
-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
-- 
2.23.0

Re: [dpdk-dev] [PATCH] app/flow-perf: fix condition of hairpin queues setup

[Wisam Monther]

Hi,

>-Original Message-
>From: Thomas Monjalon 
>Sent: Sunday, July 5, 2020 11:40 PM
>To: Wisam Monther 
>Cc: Jack Min ; david.march...@redhat.com;
>dev@dpdk.org; Asaf Penso ;
>arybche...@solarflare.com
>Subject: Re: [dpdk-dev] [PATCH] app/flow-perf: fix condition of hairpin queues
>setup
>
>30/06/2020 10:10, Wisam Jaddo:
>> The hairpin queue is the one that start from normal rxq, and will be
>> less than nr_queues where nr_queues is the sum of normal and hairpin
>>
>> Fixes: bf3688f1e816 ("app/flow-perf: add insertion rate calculation")
>> Cc: wis...@mellanox.com
>>
>> Signed-off-by: Wisam Jaddo 
>
>You should take this opportunity to document the logic for the allocation and
>peering of hairpin queues.
>
>It would be good to add short code comments for the variables as well.
>It confusing to have hairpinq and hairpin_q variables.
>
>Currently, we cannot really understand whether this fix is good or not.

I've sent V2 with the fix + those documentation.

>
>On hairpin topic, I suggest fixing this typo:
>   hairping-rss -> hairpin-rss

I've provided another patch to fix this typo:
http://patches.dpdk.org/patch/73162/

>
>

Re: [dpdk-dev] [dpdk-stable] [PATCH] doc: fix references to /dev/huge

[Sarosh Arif]

On Mon, Jul 6, 2020 at 1:44 AM Thomas Monjalon  wrote:

> 23/06/2020 07:55, Sarosh Arif:
> > change /dev/huge to /dev/hugepages
> >
> > Bugzilla ID: 492
> > Signed-off-by: Sarosh Arif 
>
> Please could you explain why /dev/hugepages must be used,
> in the commit log for the record?
>

 The path /dev/huge does not exist, the correct path is  /dev/hugepages, so
for the sake of accuracy in the documentation it should be changed.

>
> FYI, a similar change was done in the website by Stephen.
>

This bug was submitted by Stephen on bugzilla, I took notice of it and
submitted a patch to correct it. This bug is not yet corrected on the
website.

Re: [dpdk-dev] [dpdk-techboard] [PATCH v3 0/3] Experimental/internal libraries cleanup

[Bruce Richardson]

On Sun, Jul 05, 2020 at 09:55:41PM +0200, Thomas Monjalon wrote:
> +Cc maintainers of the problematic libraries:
>   - librte_fib
>   - librte_rib
>   - librte_gro
>   - librte_member
>   - librte_rawdev
> 
> 26/06/2020 10:16, David Marchand:
> > Following discussions on the mailing list and the 05/20 TB meeting, here
> > is a series that drops the special versioning for non stable libraries.
> > 
> > Two notes:
> > 
> > - RIB/FIB library is not referenced in the API doxygen index, is this
> >   intentional?
> 
> Vladimir please, could you fix the miss in the doxygen index?
> 
> > - I inspected MAINTAINERS: librte_gro, librte_member and librte_rawdev are
> >   announced as experimental while their functions are part of the 20
> >   stable ABI (in .map files + no __rte_experimental marking).
> >   Their fate must be discussed.
> 
> I would suggest removing EXPERIMENTAL flag for gro, member and rawdev.
> They are probably already considered stable for a lot of users.
> Maintainers, are you OK to follow the ABI compatibility rules
> for these libraries? Do you feel these libraries are mature enough?
>

I think things being added to the official ABI is good. For these, I wonder
if waiting till the 20.11 release is the best time to officially mark them
as stable, rather than doing so now? 

Re: [dpdk-dev] [PATCH] mbuf: use c11 atomics for refcnt operations

[Phil Yang]

> -Original Message-
> From: David Marchand 
> Sent: Friday, July 3, 2020 11:39 PM
> To: Phil Yang 
> Cc: dev ; Olivier Matz ; David
> Christensen ; Honnappa Nagarahalli
> ; Ruifeng Wang
> ; nd 
> Subject: Re: [dpdk-dev] [PATCH] mbuf: use c11 atomics for refcnt operations
> 
> On Thu, Jun 11, 2020 at 12:26 PM Phil Yang  wrote:
> >
> > Use c11 atomics with explicit ordering instead of rte_atomic ops which
> > enforce unnecessary barriers on aarch64.
> >
> > Signed-off-by: Phil Yang 
> > Reviewed-by: Ruifeng Wang 
> 
> I did not look at the details, but this patch is refused by the ABI
> check in Travis.

Thanks, David.
The ABI issue is the name of 'rte_mbuf_ext_shared_info::refcnt_atomic' changed 
to 'rte_mbuf_ext_shared_info::refcnt' at rte_mbuf_core.h.
I made this change just to simplify the name of the variable.

Revert the 'rte_mbuf_ext_shared_info::refcnt' to refcnt_atomic can fix this 
issue.
I will update it in v2.

Thanks,
Phil

> 
> 
> --
> David Marchand

Re: [dpdk-dev] [dpdk-techboard] [PATCH v3 0/3] Experimental/internal libraries cleanup

[Thomas Monjalon]

06/07/2020 10:02, Bruce Richardson:
> On Sun, Jul 05, 2020 at 09:55:41PM +0200, Thomas Monjalon wrote:
> > +Cc maintainers of the problematic libraries:
> > - librte_fib
> > - librte_rib
> > - librte_gro
> > - librte_member
> > - librte_rawdev
> > 
> > 26/06/2020 10:16, David Marchand:
> > > Following discussions on the mailing list and the 05/20 TB meeting, here
> > > is a series that drops the special versioning for non stable libraries.
> > > 
> > > Two notes:
> > > 
> > > - RIB/FIB library is not referenced in the API doxygen index, is this
> > >   intentional?
> > 
> > Vladimir please, could you fix the miss in the doxygen index?
> > 
> > > - I inspected MAINTAINERS: librte_gro, librte_member and librte_rawdev are
> > >   announced as experimental while their functions are part of the 20
> > >   stable ABI (in .map files + no __rte_experimental marking).
> > >   Their fate must be discussed.
> > 
> > I would suggest removing EXPERIMENTAL flag for gro, member and rawdev.
> > They are probably already considered stable for a lot of users.
> > Maintainers, are you OK to follow the ABI compatibility rules
> > for these libraries? Do you feel these libraries are mature enough?
> >
> 
> I think things being added to the official ABI is good. For these, I wonder
> if waiting till the 20.11 release is the best time to officially mark them
> as stable, rather than doing so now? 

They are already not marked as experimental symbols...
I think we should remove confusion in the MAINTAINERS file.

Re: [dpdk-dev] [PATCH v2] app/flow-perf: fix condition of hairpin queues setup

[Thomas Monjalon]

06/07/2020 09:53, Wisam Jaddo:
> The hairpin queue is the one that start from normal rxq,
> and will be less than nr_queues where nr_queues is the
> sum of normal and hairpin
> 
> Fixes: bf3688f1e816 ("app/flow-perf: add insertion rate calculation")
> Cc: wis...@mellanox.com
> 
> Signed-off-by: Wisam Jaddo 
> Reviewed-by: Asaf Penso 
> 
> ---
> v2:
> * Add documentation of hairpin peering and allocating logic.
> * Add documentation for some variables.
> ---
> --- a/app/test-flow-perf/main.c
> +++ b/app/test-flow-perf/main.c
> @@ -1012,8 +1012,26 @@ init_port(void)
>   rte_strerror(-ret), port_id);
>  
>   if (hairpinq != 0) {
> + /* Each hairpin queue setup need a hairpin configuration
> +  * object, which determine the TX path for hairpin.
> +  *
> +  * The peering here represent the TX side, which mean 
> the
> +  * peer.port represent TX port, and peer.queue represent
> +  * tx_queue.
> +  *
> +  * So if RXQ=4 and TXQ=4, and first hairpin_q is 4 after
> +  * [0, 1, 2, 3], then tx_queue is TXQ+i which is 4 as 
> well.
> +  *
> +  * hairpinq: represent the number of hairpin queues 
> needed
> +  * to be initialized.
> +  *
> +  * In 0 case means no hairpin queues needed which is the
> +  * default.
> +  *
> +  * hairpin_q: represent hairpin queue id to be 
> initialized.
> +  */

Variables doc should be on variable declaration.

Re: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get status message

[Adrian Moreno]

On 7/5/20 3:15 PM, Xia, Chenbo wrote:
> Hi Adrian,
> 
>> -Original Message-
>> From: dev  On Behalf Of Adrian Moreno
>> Sent: Thursday, July 2, 2020 4:33 PM
>> To: dev@dpdk.org; Ye, Xiaolong ;
>> shah...@mellanox.com; ma...@mellanox.com; maxime.coque...@redhat.com;
>> Wang, Xiao W ; viachesl...@mellanox.com
>> Cc: jasow...@redhat.com; l...@redhat.com; Adrian Moreno
>> 
>> Subject: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get status
>> message
>>
>> This patch adds support to the new Virtio device get status Vhost-user 
>> message.
>>
>> The driver can send this new message to read the device status.
>>
>> One of the uses of this message is to ensure the feature negotiation has
>> succeded.  According to the virtio spec, after completing the feature 
>> negotiation,
>> the driver sets the FEATURE_OK status bit and re-reads it to ensure the 
>> device
>> has accepted the features.
>>
>> This patch also clears the FEATURE_OK status bit if the feature negotiation 
>> has
>> failed to let the driver know about his failure.
>>
>> Signed-off-by: Adrian Moreno 
>> ---
>>  lib/librte_vhost/vhost.h  |  2 ++
>>  lib/librte_vhost/vhost_user.c | 32 
>> lib/librte_vhost/vhost_user.h |  1 +
>>  3 files changed, 35 insertions(+)
>>
>> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index
>> 25d31c71b..e743821cc 100644
>> --- a/lib/librte_vhost/vhost.h
>> +++ b/lib/librte_vhost/vhost.h
>> @@ -32,6 +32,8 @@
>>  #define VIRTIO_DEV_BUILTIN_VIRTIO_NET 4
>>  /* Used to indicate that the device has its own data path and configured */
>> #define VIRTIO_DEV_VDPA_CONFIGURED 8
>> +/* Used to indicate that the feature negotiation failed */ #define
>> +VIRTIO_DEV_FEATURES_FAILED 16
>>
>>  /* Backend value set by guest. */
>>  #define VIRTIO_DEV_STOPPED -1
>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c 
>> index
>> 8d3d13913..00da7bf18 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -88,6 +88,7 @@ static const char *vhost_message_str[VHOST_USER_MAX]
>> = {
>>  [VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD",
>>  [VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD",
>>  [VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS",
>> +[VHOST_USER_GET_STATUS] = "VHOST_USER_GET_STATUS",
>>  };
>>
>>  static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg); @@ -339,6
>> +340,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct
>> VhostUserMsg *msg,
>>  VHOST_LOG_CONFIG(ERR,
>>  "(%d) received invalid negotiated features.\n",
>>  dev->vid);
>> +dev->flags |= VIRTIO_DEV_FEATURES_FAILED;
>> +dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
>> +
>>  return RTE_VHOST_MSG_RESULT_ERR;
>>  }
>>
>> @@ -402,6 +406,7 @@ vhost_user_set_features(struct virtio_net **pdev, struct
>> VhostUserMsg *msg,
>>  if (vdpa_dev)
>>  vdpa_dev->ops->set_features(dev->vid);
>>
>> +dev->flags &= ~VIRTIO_DEV_FEATURES_FAILED;
>>  return RTE_VHOST_MSG_RESULT_OK;
>>  }
>>
>> @@ -2458,6 +2463,22 @@ vhost_user_postcopy_end(struct virtio_net **pdev,
>> struct VhostUserMsg *msg,
>>  return RTE_VHOST_MSG_RESULT_REPLY;
>>  }
>>
>> +static int
>> +vhost_user_get_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
>> +  int main_fd __rte_unused)
>> +{
>> +struct virtio_net *dev = *pdev;
>> +
>> +if (validate_msg_fds(msg, 0) != 0)
>> +return RTE_VHOST_MSG_RESULT_ERR;
>> +
>> +msg->payload.u64 = dev->status;
>> +msg->size = sizeof(msg->payload.u64);
>> +msg->fd_num = 0;
>> +
>> +return RTE_VHOST_MSG_RESULT_OK;
> 
> Should this 'RESULT_OK' be 'RESULT_REPLY' since get_status msg needs a reply?
> 
Right! Thanks for catching it.
I'll update.

> Thanks!
> Chenbo
> 
>> +}
>> +
>>  static int
>>  vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
>>  int main_fd __rte_unused)
>> @@ -2476,6 +2497,16 @@ vhost_user_set_status(struct virtio_net **pdev,
>> struct VhostUserMsg *msg,
>>
>>  dev->status = msg->payload.u64;
>>
>> +if ((dev->status & VIRTIO_DEVICE_STATUS_FEATURES_OK) &&
>> +(dev->flags & VIRTIO_DEV_FEATURES_FAILED)) {
>> +VHOST_LOG_CONFIG(ERR, "FEATURES_OK bit is set but feature
>> negotiation failed\n");
>> +/*
>> + * Clear the bit to let the driver know about the feature
>> + * negotiation failure
>> + */
>> +dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
>> +}
>> +
>>  VHOST_LOG_CONFIG(INFO, "New device status(0x%08x):\n"
>>  "\t-ACKNOWLEDGE: %u\n"
>>  "\t-DRIVER: %u\n"
>> @@ -2527,6 +2558,7 @@ static vhost_message_handler_t
>> vhost_message_handlers[VHOST_USER_MAX] = {
>>  [VHOST_USER_GET_INFLIGHT_FD] = vhost_user_get_infl

Re: [dpdk-dev] [PATCH v5 1/3] eal: disable function versioning on Windows

[Bruce Richardson]

On Sun, Jul 05, 2020 at 02:46:27PM +0300, Fady Bader wrote:
> Function versioning implementation is not supported by Windows.
> Function versioning was disabled on Windows.
> 
> Signed-off-by: Fady Bader 
> ---
>  lib/librte_eal/include/rte_function_versioning.h | 2 +-
>  lib/meson.build  | 5 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/include/rte_function_versioning.h 
> b/lib/librte_eal/include/rte_function_versioning.h
> index f588f2643b..9890551ba1 100644
> --- a/lib/librte_eal/include/rte_function_versioning.h
> +++ b/lib/librte_eal/include/rte_function_versioning.h
> @@ -7,7 +7,7 @@
>  #define _RTE_FUNCTION_VERSIONING_H_
>  #include 
>  
> -#ifndef RTE_USE_FUNCTION_VERSIONING
> +#if !defined RTE_USE_FUNCTION_VERSIONING && !defined RTE_EXEC_ENV_WINDOWS
>  #error Use of function versioning disabled, is 
> "use_function_versioning=true" in meson.build?
>  #endif
>  
> diff --git a/lib/meson.build b/lib/meson.build
> index c1b9e1633f..a1ab582a51 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -107,6 +107,11 @@ foreach l:libraries
>   shared_dep = declare_dependency(include_directories: 
> includes)
>   static_dep = shared_dep
>   else
> + if is_windows and use_function_versioning
> + message('@0@: Function versioning is not 
> supported by Windows.'
> + .format(name))
> + use_function_versioning = false
> + endif
>  
>   if use_function_versioning
>   cflags += '-DRTE_USE_FUNCTION_VERSIONING'
> -- 

I wonder if this patch can be simplified as follows.

Presumably the use of the RTE_USE_FUNCTION_VERSIONING cflag doesn't cause
any problems building on windows, since it's basically nothing except a
flag to indicate the build is configured properly, so that block can be
left intact. Instead what is needed to be disabled is the
RTE_BUILD_SHARED_LIB setting so that we use the static macros. Therefore,
I think the same result can be got by just changing the following line
in lib/meson.build:

@@ -138,7 +138,7 @@ foreach l:libraries
include_directories: includes,
dependencies: static_deps)

-   if not use_function_versioning
+   if not use_function_versioning or is_windows
# use pre-build objects to build shared lib
sources = []
objs += 
static_lib.extract_all_objects(recursive: false)

Then no error message disabling is needed in windows. I also don't think a
message about function versioning not being supported on windows is
necessary. It's not something the user can do anything about himself
anyway. If we want such a message I think it should just be printed once at
the start of the configuration process, rather than each time we hit a
versioned library.

Regards,
/Bruce

[dpdk-dev] [PATCH v3 00/27] update e1000 base code

[Guinan Sun]

update e1000 base code.

source code of e1000 driver:
cid-gigabit.2020.06.05.tar.gz released by the team which develop
basic drivers for any e1000 NIC.

changelog in ND share repo:
>From 99bddf09773a ("e1000_shared: Remove #ifdef CLARKVILLE_HW")
To 64edeeac42a7 ("e1000-shared: Fix LTR algorithm for i225 device")
Tested-by: Bo Chen 
---
v3:
* Merge some patches.
* Modify some commit messages.

v2:
* Remove codes about i225.

Guinan Sun (27):
  net/e1000/base: i210 slow system clock update
  net/e1000/base: add ICL device ID
  net/e1000/base: introduce flags
  net/e1000/base: add support for i211
  net/e1000/base: expose xmdio methods
  net/e1000/base: fall through explicitly
  net/e1000/base: add function parameter descriptions
  net/e1000/base: improve code style and fix klocwork errors
  net/e1000/base: modify HW level time sync mechanisms
  net/e1000/base: remove duplicated codes
  net/e1000/base: expose MAC functions
  net/e1000/base: add define to PCIm function state
  net/e1000/base: add missing register defines
  net/e1000/base: increased timeout for ME ULP exit
  net/e1000/base: add missing device ID
  net/e1000/base: expose more future extended NVM
  net/e1000/base: remove useless statement
  net/e1000/base: add missed define for VFTA
  net/e1000/base: modify flow control setup
  net/e1000/base: led blinking fix for i210
  net/e1000/base: expose new FEXTNVM registers and masks
  net/e1000/base: add support for Nahum10
  net/e1000/base: add ADL device ID
  net/e1000/base: introduce DPGFR register
  net/e1000/base: cleanup pre-processor tags
  net/e1000/base: modify copyright
  net/e1000/base: update version

 drivers/net/e1000/Makefile |   1 +
 drivers/net/e1000/base/README  |   4 +-
 drivers/net/e1000/base/e1000_80003es2lan.c |   3 +-
 drivers/net/e1000/base/e1000_80003es2lan.h |   2 +-
 drivers/net/e1000/base/e1000_82540.c   |   2 +-
 drivers/net/e1000/base/e1000_82541.c   |   2 +-
 drivers/net/e1000/base/e1000_82541.h   |   2 +-
 drivers/net/e1000/base/e1000_82542.c   |   2 +-
 drivers/net/e1000/base/e1000_82543.c   |   2 +-
 drivers/net/e1000/base/e1000_82543.h   |   2 +-
 drivers/net/e1000/base/e1000_82571.c   |   2 +-
 drivers/net/e1000/base/e1000_82571.h   |   2 +-
 drivers/net/e1000/base/e1000_82575.c   | 521 +++--
 drivers/net/e1000/base/e1000_82575.h   |  95 +---
 drivers/net/e1000/base/e1000_api.c |  14 +-
 drivers/net/e1000/base/e1000_api.h |   3 +-
 drivers/net/e1000/base/e1000_base.c| 190 
 drivers/net/e1000/base/e1000_base.h| 127 +
 drivers/net/e1000/base/e1000_defines.h |  27 +-
 drivers/net/e1000/base/e1000_hw.h  |  17 +-
 drivers/net/e1000/base/e1000_i210.c| 101 +---
 drivers/net/e1000/base/e1000_i210.h|   6 +-
 drivers/net/e1000/base/e1000_ich8lan.c | 115 ++---
 drivers/net/e1000/base/e1000_ich8lan.h |  27 +-
 drivers/net/e1000/base/e1000_mac.c |  13 +-
 drivers/net/e1000/base/e1000_mac.h |   5 +-
 drivers/net/e1000/base/e1000_manage.c  |   6 +-
 drivers/net/e1000/base/e1000_manage.h  |   3 +-
 drivers/net/e1000/base/e1000_mbx.c |   7 +-
 drivers/net/e1000/base/e1000_mbx.h |   2 +-
 drivers/net/e1000/base/e1000_nvm.c |  16 +-
 drivers/net/e1000/base/e1000_nvm.h |   2 +-
 drivers/net/e1000/base/e1000_phy.c |  86 +++-
 drivers/net/e1000/base/e1000_phy.h |   7 +-
 drivers/net/e1000/base/e1000_regs.h|  39 +-
 drivers/net/e1000/base/e1000_vf.c  |   4 +-
 drivers/net/e1000/base/e1000_vf.h  |   2 +-
 drivers/net/e1000/base/meson.build |   1 +
 drivers/net/e1000/igb_rxtx.c   |   2 +-
 39 files changed, 805 insertions(+), 659 deletions(-)
 create mode 100644 drivers/net/e1000/base/e1000_base.c
 create mode 100644 drivers/net/e1000/base/e1000_base.h

-- 
2.17.1

[dpdk-dev] [PATCH v3 01/27] net/e1000/base: i210 slow system clock update

[Guinan Sun]

This code is required for the update for system clock.

Signed-off-by: Todd Fujinaka 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_i210.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_i210.c 
b/drivers/net/e1000/base/e1000_i210.c
index 9298223c3..d9cd1a084 100644
--- a/drivers/net/e1000/base/e1000_i210.c
+++ b/drivers/net/e1000/base/e1000_i210.c
@@ -900,6 +900,8 @@ STATIC s32 e1000_pll_workaround_i210(struct e1000_hw *hw)
u16 nvm_word, phy_word, pci_word, tmp_nvm;
int i;
 
+   /* Get PHY semaphore */
+   hw->phy.ops.acquire(hw);
/* Get and set needed register values */
wuc = E1000_READ_REG(hw, E1000_WUC);
mdicnfg = E1000_READ_REG(hw, E1000_MDICNFG);
@@ -915,8 +917,11 @@ STATIC s32 e1000_pll_workaround_i210(struct e1000_hw *hw)
phy_word = E1000_PHY_PLL_UNCONF;
for (i = 0; i < E1000_MAX_PLL_TRIES; i++) {
/* check current state directly from internal PHY */
-   e1000_read_phy_reg_gs40g(hw, (E1000_PHY_PLL_FREQ_PAGE |
-E1000_PHY_PLL_FREQ_REG), &phy_word);
+   e1000_write_phy_reg_mdic(hw, GS40G_PAGE_SELECT, 0xFC);
+   usec_delay(20);
+   e1000_read_phy_reg_mdic(hw, E1000_PHY_PLL_FREQ_REG, &phy_word);
+   usec_delay(20);
+   e1000_write_phy_reg_mdic(hw, GS40G_PAGE_SELECT, 0);
if ((phy_word & E1000_PHY_PLL_UNCONF)
!= E1000_PHY_PLL_UNCONF) {
ret_val = E1000_SUCCESS;
@@ -950,6 +955,8 @@ STATIC s32 e1000_pll_workaround_i210(struct e1000_hw *hw)
}
/* restore MDICNFG setting */
E1000_WRITE_REG(hw, E1000_MDICNFG, mdicnfg);
+   /* Release PHY semaphore */
+   hw->phy.ops.release(hw);
return ret_val;
 }
 
-- 
2.17.1

[dpdk-dev] [PATCH v3 03/27] net/e1000/base: introduce flags

[Guinan Sun]

Introduce flags to make flexible adjusting
number of outstanding requests.

Signed-off-by: Sasha Neftin 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_ich8lan.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.h 
b/drivers/net/e1000/base/e1000_ich8lan.h
index 699a92c4b..9c21396c3 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.h
+++ b/drivers/net/e1000/base/e1000_ich8lan.h
@@ -103,8 +103,8 @@
 #define NVM_SIZE_MULTIPLIER 4096  /*multiplier for NVMS field*/
 #define E1000_FLASH_BASE_ADDR 0xE000 /*offset of NVM access regs*/
 #define E1000_CTRL_EXT_NVMVS 0x3 /*NVM valid sector */
-#define E1000_TARC0_CB_MULTIQ_3_REQ(1 << 28 | 1 << 29)
-#define E1000_TARC0_CB_MULTIQ_2_REQ(1 << 29)
+#define E1000_TARC0_CB_MULTIQ_3_REQ0x3000
+#define E1000_TARC0_CB_MULTIQ_2_REQ0x2000
 #define PCIE_ICH8_SNOOP_ALLPCIE_NO_SNOOP_ALL
 
 #define E1000_ICH_RAR_ENTRIES  7
-- 
2.17.1

[dpdk-dev] [PATCH v3 02/27] net/e1000/base: add ICL device ID

[Guinan Sun]

This patch contains a preliminary support for new LAN device ID.

Signed-off-by: Lotem Leder 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_api.c | 4 
 drivers/net/e1000/base/e1000_hw.h  | 4 
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_api.c 
b/drivers/net/e1000/base/e1000_api.c
index 718952801..a93e8ff03 100644
--- a/drivers/net/e1000/base/e1000_api.c
+++ b/drivers/net/e1000/base/e1000_api.c
@@ -284,6 +284,10 @@ s32 e1000_set_mac_type(struct e1000_hw *hw)
case E1000_DEV_ID_PCH_CNP_I219_V6:
case E1000_DEV_ID_PCH_CNP_I219_LM7:
case E1000_DEV_ID_PCH_CNP_I219_V7:
+   case E1000_DEV_ID_PCH_ICP_I219_LM8:
+   case E1000_DEV_ID_PCH_ICP_I219_V8:
+   case E1000_DEV_ID_PCH_ICP_I219_LM9:
+   case E1000_DEV_ID_PCH_ICP_I219_V9:
mac->type = e1000_pch_cnp;
break;
case E1000_DEV_ID_82575EB_COPPER:
diff --git a/drivers/net/e1000/base/e1000_hw.h 
b/drivers/net/e1000/base/e1000_hw.h
index 9793b724e..933a9204c 100644
--- a/drivers/net/e1000/base/e1000_hw.h
+++ b/drivers/net/e1000/base/e1000_hw.h
@@ -120,6 +120,10 @@ struct e1000_hw;
 #define E1000_DEV_ID_PCH_CNP_I219_V6   0x15BE
 #define E1000_DEV_ID_PCH_CNP_I219_LM7  0x15BB
 #define E1000_DEV_ID_PCH_CNP_I219_V7   0x15BC
+#define E1000_DEV_ID_PCH_ICP_I219_LM8  0x15DF
+#define E1000_DEV_ID_PCH_ICP_I219_V8   0x15E0
+#define E1000_DEV_ID_PCH_ICP_I219_LM9  0x15E1
+#define E1000_DEV_ID_PCH_ICP_I219_V9   0x15E2
 #define E1000_DEV_ID_82576 0x10C9
 #define E1000_DEV_ID_82576_FIBER   0x10E6
 #define E1000_DEV_ID_82576_SERDES  0x10E7
-- 
2.17.1

[dpdk-dev] [PATCH v3 04/27] net/e1000/base: add support for i211

[Guinan Sun]

Add support SF/FW syncronization.
Add support to print PBA when using flashless.

Signed-off-by: Todd Fujinaka 
Signed-off-by: Sasha Neftin 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_82575.c | 4 ++--
 drivers/net/e1000/base/e1000_nvm.c   | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_82575.c 
b/drivers/net/e1000/base/e1000_82575.c
index 4c3611c6d..8db287251 100644
--- a/drivers/net/e1000/base/e1000_82575.c
+++ b/drivers/net/e1000/base/e1000_82575.c
@@ -435,7 +435,7 @@ STATIC s32 e1000_init_mac_params_82575(struct e1000_hw *hw)
if ((mac->type == e1000_i210) || (mac->type == e1000_i211))
mac->ops.init_hw = e1000_init_hw_i210;
else
-   mac->ops.init_hw = e1000_init_hw_82575;
+   mac->ops.init_hw = e1000_init_hw_82575;
/* link setup */
mac->ops.setup_link = e1000_setup_link_generic;
/* physical interface link setup */
@@ -486,7 +486,7 @@ STATIC s32 e1000_init_mac_params_82575(struct e1000_hw *hw)
/* acquire SW_FW sync */
mac->ops.acquire_swfw_sync = e1000_acquire_swfw_sync_82575;
mac->ops.release_swfw_sync = e1000_release_swfw_sync_82575;
-   if (mac->type >= e1000_i210) {
+   if (mac->type == e1000_i210 || mac->type == e1000_i211) {
mac->ops.acquire_swfw_sync = e1000_acquire_swfw_sync_i210;
mac->ops.release_swfw_sync = e1000_release_swfw_sync_i210;
}
diff --git a/drivers/net/e1000/base/e1000_nvm.c 
b/drivers/net/e1000/base/e1000_nvm.c
index 56e2db122..4d4a8e04b 100644
--- a/drivers/net/e1000/base/e1000_nvm.c
+++ b/drivers/net/e1000/base/e1000_nvm.c
@@ -749,8 +749,9 @@ s32 e1000_read_pba_string_generic(struct e1000_hw *hw, u8 
*pba_num,
 
DEBUGFUNC("e1000_read_pba_string_generic");
 
-   if ((hw->mac.type >= e1000_i210) &&
-   !e1000_get_flash_presence_i210(hw)) {
+   if ((hw->mac.type == e1000_i210 ||
+hw->mac.type == e1000_i211) &&
+!e1000_get_flash_presence_i210(hw)) {
DEBUGOUT("Flashless no PBA string\n");
return -E1000_ERR_NVM_PBA_SECTION;
}
-- 
2.17.1

[dpdk-dev] [PATCH v3 06/27] net/e1000/base: fall through explicitly

[Guinan Sun]

Found some inconsistent code comments when it came to when we "fall
through", so made them more consistent and non-repetitive.

This patch adds/changes fall through comments to address new warnings
produced by gcc 7.

Signed-off-by: Jeff Kirsher 
Signed-off-by: Todd Fujinaka 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_82575.c | 11 +--
 drivers/net/e1000/base/e1000_mbx.c   |  1 +
 drivers/net/e1000/base/e1000_phy.c   |  1 +
 drivers/net/e1000/base/e1000_vf.c|  2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_82575.c 
b/drivers/net/e1000/base/e1000_82575.c
index 8db287251..7c562258a 100644
--- a/drivers/net/e1000/base/e1000_82575.c
+++ b/drivers/net/e1000/base/e1000_82575.c
@@ -1560,14 +1560,21 @@ STATIC s32 e1000_setup_copper_link_82575(struct 
e1000_hw *hw)
}
switch (hw->phy.type) {
case e1000_phy_i210:
+   /* Fall through */
case e1000_phy_m88:
switch (hw->phy.id) {
case I347AT4_E_PHY_ID:
+   /* Fall through */
case M88E1112_E_PHY_ID:
+   /* Fall through */
case M88E1340M_E_PHY_ID:
+   /* Fall through */
case M88E1543_E_PHY_ID:
+   /* Fall through */
case M88E1512_E_PHY_ID:
+   /* Fall through */
case I210_I_PHY_ID:
+   /* Fall through */
ret_val = e1000_copper_link_setup_m88_gen2(hw);
break;
default:
@@ -1653,7 +1660,7 @@ STATIC s32 e1000_setup_serdes_link_82575(struct e1000_hw 
*hw)
case E1000_CTRL_EXT_LINK_MODE_1000BASE_KX:
/* disable PCS autoneg and support parallel detect only */
pcs_autoneg = false;
-   /* fall through to default case */
+   /* Fall through */
default:
if (hw->mac.type == e1000_82575 ||
hw->mac.type == e1000_82576) {
@@ -1779,7 +1786,7 @@ STATIC s32 e1000_get_media_type_82575(struct e1000_hw *hw)
dev_spec->sgmii_active = true;
break;
}
-   /* fall through for I2C based SGMII */
+   /* Fall through for I2C based SGMII */
case E1000_CTRL_EXT_LINK_MODE_PCIE_SERDES:
/* read media type from SFP EEPROM */
ret_val = e1000_set_sfp_media_type_82575(hw);
diff --git a/drivers/net/e1000/base/e1000_mbx.c 
b/drivers/net/e1000/base/e1000_mbx.c
index 6fae6767f..4cac3642e 100644
--- a/drivers/net/e1000/base/e1000_mbx.c
+++ b/drivers/net/e1000/base/e1000_mbx.c
@@ -755,6 +755,7 @@ s32 e1000_init_mbx_params_pf(struct e1000_hw *hw)
mbx->stats.reqs = 0;
mbx->stats.acks = 0;
mbx->stats.rsts = 0;
+   /* Fall through */
default:
return E1000_SUCCESS;
}
diff --git a/drivers/net/e1000/base/e1000_phy.c 
b/drivers/net/e1000/base/e1000_phy.c
index e8d922147..be4786794 100644
--- a/drivers/net/e1000/base/e1000_phy.c
+++ b/drivers/net/e1000/base/e1000_phy.c
@@ -1274,6 +1274,7 @@ s32 e1000_copper_link_setup_m88_gen2(struct e1000_hw *hw)
phy_data |= M88E1000_PSCR_AUTO_X_1000T;
break;
}
+   /* Fall through */
case 0:
default:
phy_data |= M88E1000_PSCR_AUTO_X_MODE;
diff --git a/drivers/net/e1000/base/e1000_vf.c 
b/drivers/net/e1000/base/e1000_vf.c
index 543fa7741..dc109d5e0 100644
--- a/drivers/net/e1000/base/e1000_vf.c
+++ b/drivers/net/e1000/base/e1000_vf.c
@@ -462,8 +462,10 @@ s32 e1000_promisc_set_vf(struct e1000_hw *hw, enum 
e1000_promisc_type type)
break;
case e1000_promisc_enabled:
msgbuf |= E1000_VF_SET_PROMISC_MULTICAST;
+   /* fall-through */
case e1000_promisc_unicast:
msgbuf |= E1000_VF_SET_PROMISC_UNICAST;
+   /* fall-through */
case e1000_promisc_disabled:
break;
default:
-- 
2.17.1

[dpdk-dev] [PATCH v3 05/27] net/e1000/base: expose xmdio methods

[Guinan Sun]

move read and write xmdio methods to e1000_phy.

Signed-off-by: Neftin Sasha 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_i210.c | 71 
 drivers/net/e1000/base/e1000_i210.h |  4 --
 drivers/net/e1000/base/e1000_phy.c  | 72 +
 drivers/net/e1000/base/e1000_phy.h  |  5 ++
 4 files changed, 77 insertions(+), 75 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_i210.c 
b/drivers/net/e1000/base/e1000_i210.c
index d9cd1a084..5c2fe8a3e 100644
--- a/drivers/net/e1000/base/e1000_i210.c
+++ b/drivers/net/e1000/base/e1000_i210.c
@@ -815,77 +815,6 @@ STATIC s32 e1000_valid_led_default_i210(struct e1000_hw 
*hw, u16 *data)
return ret_val;
 }
 
-/**
- *  __e1000_access_xmdio_reg - Read/write XMDIO register
- *  @hw: pointer to the HW structure
- *  @address: XMDIO address to program
- *  @dev_addr: device address to program
- *  @data: pointer to value to read/write from/to the XMDIO address
- *  @read: boolean flag to indicate read or write
- **/
-STATIC s32 __e1000_access_xmdio_reg(struct e1000_hw *hw, u16 address,
-   u8 dev_addr, u16 *data, bool read)
-{
-   s32 ret_val;
-
-   DEBUGFUNC("__e1000_access_xmdio_reg");
-
-   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAC, dev_addr);
-   if (ret_val)
-   return ret_val;
-
-   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAAD, address);
-   if (ret_val)
-   return ret_val;
-
-   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAC, E1000_MMDAC_FUNC_DATA |
-dev_addr);
-   if (ret_val)
-   return ret_val;
-
-   if (read)
-   ret_val = hw->phy.ops.read_reg(hw, E1000_MMDAAD, data);
-   else
-   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAAD, *data);
-   if (ret_val)
-   return ret_val;
-
-   /* Recalibrate the device back to 0 */
-   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAC, 0);
-   if (ret_val)
-   return ret_val;
-
-   return ret_val;
-}
-
-/**
- *  e1000_read_xmdio_reg - Read XMDIO register
- *  @hw: pointer to the HW structure
- *  @addr: XMDIO address to program
- *  @dev_addr: device address to program
- *  @data: value to be read from the EMI address
- **/
-s32 e1000_read_xmdio_reg(struct e1000_hw *hw, u16 addr, u8 dev_addr, u16 *data)
-{
-   DEBUGFUNC("e1000_read_xmdio_reg");
-
-   return __e1000_access_xmdio_reg(hw, addr, dev_addr, data, true);
-}
-
-/**
- *  e1000_write_xmdio_reg - Write XMDIO register
- *  @hw: pointer to the HW structure
- *  @addr: XMDIO address to program
- *  @dev_addr: device address to program
- *  @data: value to be written to the XMDIO address
- **/
-s32 e1000_write_xmdio_reg(struct e1000_hw *hw, u16 addr, u8 dev_addr, u16 data)
-{
-   DEBUGFUNC("e1000_read_xmdio_reg");
-
-   return __e1000_access_xmdio_reg(hw, addr, dev_addr, &data, false);
-}
-
 /**
  * e1000_pll_workaround_i210
  * @hw: pointer to the HW structure
diff --git a/drivers/net/e1000/base/e1000_i210.h 
b/drivers/net/e1000/base/e1000_i210.h
index c6aa2a17b..940eee21a 100644
--- a/drivers/net/e1000/base/e1000_i210.h
+++ b/drivers/net/e1000/base/e1000_i210.h
@@ -17,10 +17,6 @@ s32 e1000_read_invm_version(struct e1000_hw *hw,
struct e1000_fw_version *invm_ver);
 s32 e1000_acquire_swfw_sync_i210(struct e1000_hw *hw, u16 mask);
 void e1000_release_swfw_sync_i210(struct e1000_hw *hw, u16 mask);
-s32 e1000_read_xmdio_reg(struct e1000_hw *hw, u16 addr, u8 dev_addr,
-u16 *data);
-s32 e1000_write_xmdio_reg(struct e1000_hw *hw, u16 addr, u8 dev_addr,
- u16 data);
 s32 e1000_init_hw_i210(struct e1000_hw *hw);
 
 #define E1000_STM_OPCODE   0xDB00
diff --git a/drivers/net/e1000/base/e1000_phy.c 
b/drivers/net/e1000/base/e1000_phy.c
index 956c06747..e8d922147 100644
--- a/drivers/net/e1000/base/e1000_phy.c
+++ b/drivers/net/e1000/base/e1000_phy.c
@@ -4229,3 +4229,75 @@ bool e1000_is_mphy_ready(struct e1000_hw *hw)
 
return ready;
 }
+
+/**
+ *  __e1000_access_xmdio_reg - Read/write XMDIO register
+ *  @hw: pointer to the HW structure
+ *  @address: XMDIO address to program
+ *  @dev_addr: device address to program
+ *  @data: pointer to value to read/write from/to the XMDIO address
+ *  @read: boolean flag to indicate read or write
+ **/
+STATIC s32 __e1000_access_xmdio_reg(struct e1000_hw *hw, u16 address,
+   u8 dev_addr, u16 *data, bool read)
+{
+   s32 ret_val;
+
+   DEBUGFUNC("__e1000_access_xmdio_reg");
+
+   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAC, dev_addr);
+   if (ret_val)
+   return ret_val;
+
+   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAAD, address);
+   if (ret_val)
+   return ret_val;
+
+   ret_val = hw->phy.ops.write_reg(hw, E1000_MMDAC, E1000_MMDAC_FUNC_DAT

[dpdk-dev] [PATCH v3 07/27] net/e1000/base: add function parameter descriptions

[Guinan Sun]

Add function parameter descriptions to address gcc 7 warnings.

Signed-off-by: Todd Fujinaka 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_82575.c | 11 +--
 drivers/net/e1000/base/e1000_mac.c   |  8 
 drivers/net/e1000/base/e1000_mbx.c   |  4 
 drivers/net/e1000/base/e1000_nvm.c   |  7 +++
 drivers/net/e1000/base/e1000_phy.c   |  6 ++
 5 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_82575.c 
b/drivers/net/e1000/base/e1000_82575.c
index 7c562258a..750f1cf01 100644
--- a/drivers/net/e1000/base/e1000_82575.c
+++ b/drivers/net/e1000/base/e1000_82575.c
@@ -2764,7 +2764,7 @@ STATIC s32 e1000_update_nvm_checksum_i350(struct e1000_hw 
*hw)
 /**
  *  __e1000_access_emi_reg - Read/write EMI register
  *  @hw: pointer to the HW structure
- *  @addr: EMI address to program
+ *  @address: EMI address to program
  *  @data: pointer to value to read/write from/to the EMI address
  *  @read: boolean flag to indicate read or write
  **/
@@ -2991,8 +2991,8 @@ s32 e1000_initialize_M88E1543_phy(struct e1000_hw *hw)
 /**
  *  e1000_set_eee_i350 - Enable/disable EEE support
  *  @hw: pointer to the HW structure
- *  @adv1g: boolean flag enabling 1G EEE advertisement
- *  @adv100m: boolean flag enabling 100M EEE advertisement
+ *  @adv1G: boolean flag enabling 1G EEE advertisement
+ *  @adv100M: boolean flag enabling 100M EEE advertisement
  *
  *  Enable/disable EEE based on setting in dev_spec structure.
  *
@@ -3046,8 +3046,8 @@ s32 e1000_set_eee_i350(struct e1000_hw *hw, bool adv1G, 
bool adv100M)
 /**
  *  e1000_set_eee_i354 - Enable/disable EEE support
  *  @hw: pointer to the HW structure
- *  @adv1g: boolean flag enabling 1G EEE advertisement
- *  @adv100m: boolean flag enabling 100M EEE advertisement
+ *  @adv1G: boolean flag enabling 1G EEE advertisement
+ *  @adv100M: boolean flag enabling 100M EEE advertisement
  *
  *  Enable/disable EEE legacy mode based on setting in dev_spec structure.
  *
@@ -3703,7 +3703,6 @@ STATIC s32 e1000_set_i2c_data(struct e1000_hw *hw, u32 
*i2cctl, bool data)
 
 /**
  *  e1000_get_i2c_data - Reads the I2C SDA data bit
- *  @hw: pointer to hardware structure
  *  @i2cctl: Current value of I2CCTL register
  *
  *  Returns the I2C data bit value
diff --git a/drivers/net/e1000/base/e1000_mac.c 
b/drivers/net/e1000/base/e1000_mac.c
index d0e1fa58b..48384b284 100644
--- a/drivers/net/e1000/base/e1000_mac.c
+++ b/drivers/net/e1000/base/e1000_mac.c
@@ -75,6 +75,8 @@ void e1000_null_mac_generic(struct e1000_hw E1000_UNUSEDARG 
*hw)
 /**
  *  e1000_null_link_info - No-op function, return 0
  *  @hw: pointer to the HW structure
+ *  @s: dummy variable
+ *  @d: dummy variable
  **/
 s32 e1000_null_link_info(struct e1000_hw E1000_UNUSEDARG *hw,
 u16 E1000_UNUSEDARG *s, u16 E1000_UNUSEDARG *d)
@@ -98,6 +100,8 @@ bool e1000_null_mng_mode(struct e1000_hw E1000_UNUSEDARG *hw)
 /**
  *  e1000_null_update_mc - No-op function, return void
  *  @hw: pointer to the HW structure
+ *  @h: dummy variable
+ *  @a: dummy variable
  **/
 void e1000_null_update_mc(struct e1000_hw E1000_UNUSEDARG *hw,
  u8 E1000_UNUSEDARG *h, u32 E1000_UNUSEDARG a)
@@ -110,6 +114,8 @@ void e1000_null_update_mc(struct e1000_hw E1000_UNUSEDARG 
*hw,
 /**
  *  e1000_null_write_vfta - No-op function, return void
  *  @hw: pointer to the HW structure
+ *  @a: dummy variable
+ *  @b: dummy variable
  **/
 void e1000_null_write_vfta(struct e1000_hw E1000_UNUSEDARG *hw,
   u32 E1000_UNUSEDARG a, u32 E1000_UNUSEDARG b)
@@ -122,6 +128,8 @@ void e1000_null_write_vfta(struct e1000_hw E1000_UNUSEDARG 
*hw,
 /**
  *  e1000_null_rar_set - No-op function, return 0
  *  @hw: pointer to the HW structure
+ *  @h: dummy variable
+ *  @a: dummy variable
  **/
 int e1000_null_rar_set(struct e1000_hw E1000_UNUSEDARG *hw,
u8 E1000_UNUSEDARG *h, u32 E1000_UNUSEDARG a)
diff --git a/drivers/net/e1000/base/e1000_mbx.c 
b/drivers/net/e1000/base/e1000_mbx.c
index 4cac3642e..3f3b326c6 100644
--- a/drivers/net/e1000/base/e1000_mbx.c
+++ b/drivers/net/e1000/base/e1000_mbx.c
@@ -7,6 +7,7 @@
 /**
  *  e1000_null_mbx_check_for_flag - No-op function, return 0
  *  @hw: pointer to the HW structure
+ *  @mbx_id: id of mailbox to read
  **/
 STATIC s32 e1000_null_mbx_check_for_flag(struct e1000_hw E1000_UNUSEDARG *hw,
 u16 E1000_UNUSEDARG mbx_id)
@@ -20,6 +21,9 @@ STATIC s32 e1000_null_mbx_check_for_flag(struct e1000_hw 
E1000_UNUSEDARG *hw,
 /**
  *  e1000_null_mbx_transact - No-op function, return 0
  *  @hw: pointer to the HW structure
+ *  @msg: The message buffer
+ *  @size: Length of buffer
+ *  @mbx_id: id of mailbox to read
  **/
 STATIC s32 e1000_null_mbx_transact(struct e1000_hw E1000_UNUSEDARG *hw,
   u32 E1000_UNUSEDARG *msg,
diff --git a/drivers/net/e1000/base/e1000_nvm.c 
b/drivers/net/e1000/base/e1000_nvm

Re: [dpdk-dev] [PATCH] maintainers: update for netcope sze/nfb

[Ferruh Yigit]

On 6/18/2020 3:29 PM, Martin Špinler wrote:
> Acked-by: Martin Spinler 
>> Setting Martin Spinler as new and only maintainer for Netcope
>> libsze2/nfb drivers
>>
>> Signed-off-by: Jakub Neruda 
> 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v3 09/27] net/e1000/base: modify HW level time sync mechanisms

[Guinan Sun]

Add additinal configuration space access to allow HW
level time sync mechanism.

Signed-off-by: Efimov Evgeny 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_ich8lan.c | 18 ++
 drivers/net/e1000/base/e1000_ich8lan.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.c 
b/drivers/net/e1000/base/e1000_ich8lan.c
index 823dda5f5..1dc29553e 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -4896,6 +4896,7 @@ STATIC s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
u16 kum_cfg;
u32 ctrl, reg;
s32 ret_val;
+   u16 pci_cfg;
 
DEBUGFUNC("e1000_reset_hw_ich8lan");
 
@@ -4956,11 +4957,28 @@ STATIC s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
e1000_gate_hw_phy_config_ich8lan(hw, true);
}
ret_val = e1000_acquire_swflag_ich8lan(hw);
+
+   /* Read from EXTCNF_CTRL in e1000_acquire_swflag_ich8lan function
+* may occur during global reset and cause system hang.
+* Configuration space access creates the needed delay.
+* Write to E1000_STRAP RO register E1000_PCI_VENDOR_ID_REGISTER value
+* insures configuration space read is done before global reset.
+*/
+   e1000_read_pci_cfg(hw, E1000_PCI_VENDOR_ID_REGISTER, &pci_cfg);
+   E1000_WRITE_REG(hw, E1000_STRAP, pci_cfg);
DEBUGOUT("Issuing a global reset to ich8lan\n");
E1000_WRITE_REG(hw, E1000_CTRL, (ctrl | E1000_CTRL_RST));
/* cannot issue a flush here because it hangs the hardware */
msec_delay(20);
 
+   /* Configuration space access improve HW level time sync mechanism.
+* Write to E1000_STRAP RO register E1000_PCI_VENDOR_ID_REGISTER
+* value to insure configuration space read is done
+* before any access to mac register.
+*/
+   e1000_read_pci_cfg(hw, E1000_PCI_VENDOR_ID_REGISTER, &pci_cfg);
+   E1000_WRITE_REG(hw, E1000_STRAP, pci_cfg);
+
/* Set Phy Config Counter to 50msec */
if (hw->mac.type == e1000_pch2lan) {
reg = E1000_READ_REG(hw, E1000_FEXTNVM3);
diff --git a/drivers/net/e1000/base/e1000_ich8lan.h 
b/drivers/net/e1000/base/e1000_ich8lan.h
index 9c21396c3..bfee467b3 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.h
+++ b/drivers/net/e1000/base/e1000_ich8lan.h
@@ -287,6 +287,7 @@
 /* Receive Address Initial CRC Calculation */
 #define E1000_PCH_RAICC(_n)(0x05F50 + ((_n) * 4))
 
+#define E1000_PCI_VENDOR_ID_REGISTER   0x00
 #if defined(QV_RELEASE) || !defined(NO_PCH_LPT_B0_SUPPORT)
 #define E1000_PCI_REVISION_ID_REG  0x08
 #endif /* defined(QV_RELEASE) || !defined(NO_PCH_LPT_B0_SUPPORT) */
-- 
2.17.1

[dpdk-dev] [PATCH v3 10/27] net/e1000/base: remove duplicated codes

[Guinan Sun]

Add two files base.c and base.h to reduce the redundancy
in the silicon family code.
Remove the code duplication from e1000_82575 files.
Clean family specific functions from base.
Fix up a stray and duplicate function declaration.

Signed-off-by: Jeff Kirsher 
Signed-off-by: Sasha Neftin 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/Makefile |   1 +
 drivers/net/e1000/base/e1000_82575.c   | 475 -
 drivers/net/e1000/base/e1000_82575.h   |  91 +
 drivers/net/e1000/base/e1000_api.h |   1 -
 drivers/net/e1000/base/e1000_base.c| 190 ++
 drivers/net/e1000/base/e1000_base.h| 127 +++
 drivers/net/e1000/base/e1000_defines.h |  14 +-
 drivers/net/e1000/base/e1000_hw.h  |   1 +
 drivers/net/e1000/base/e1000_i210.c|   2 +-
 drivers/net/e1000/base/e1000_regs.h|  23 +-
 drivers/net/e1000/base/meson.build |   1 +
 drivers/net/e1000/igb_rxtx.c   |   2 +-
 12 files changed, 512 insertions(+), 416 deletions(-)
 create mode 100644 drivers/net/e1000/base/e1000_base.c
 create mode 100644 drivers/net/e1000/base/e1000_base.h

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index 9fb038cf0..f186f8d0e 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -50,6 +50,7 @@ VPATH += $(SRCDIR)/base
 #
 # all source are stored in SRCS-y
 #
+SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_base.c
 SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_80003es2lan.c
 SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_82540.c
 SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_82541.c
diff --git a/drivers/net/e1000/base/e1000_82575.c 
b/drivers/net/e1000/base/e1000_82575.c
index 410f2db41..f6b9f629f 100644
--- a/drivers/net/e1000/base/e1000_82575.c
+++ b/drivers/net/e1000/base/e1000_82575.c
@@ -17,8 +17,6 @@
 
 STATIC s32  e1000_init_phy_params_82575(struct e1000_hw *hw);
 STATIC s32  e1000_init_mac_params_82575(struct e1000_hw *hw);
-STATIC s32  e1000_acquire_phy_82575(struct e1000_hw *hw);
-STATIC void e1000_release_phy_82575(struct e1000_hw *hw);
 STATIC s32  e1000_acquire_nvm_82575(struct e1000_hw *hw);
 STATIC void e1000_release_nvm_82575(struct e1000_hw *hw);
 STATIC s32  e1000_check_for_link_82575(struct e1000_hw *hw);
@@ -30,6 +28,7 @@ STATIC s32  e1000_phy_hw_reset_sgmii_82575(struct e1000_hw 
*hw);
 STATIC s32  e1000_read_phy_reg_sgmii_82575(struct e1000_hw *hw, u32 offset,
   u16 *data);
 STATIC s32  e1000_reset_hw_82575(struct e1000_hw *hw);
+STATIC s32 e1000_init_hw_82575(struct e1000_hw *hw);
 STATIC s32  e1000_reset_hw_82580(struct e1000_hw *hw);
 STATIC s32  e1000_read_phy_reg_82580(struct e1000_hw *hw,
 u32 offset, u16 *data);
@@ -55,10 +54,8 @@ STATIC s32  e1000_get_pcs_speed_and_duplex_82575(struct 
e1000_hw *hw,
 STATIC s32  e1000_get_phy_id_82575(struct e1000_hw *hw);
 STATIC void e1000_release_swfw_sync_82575(struct e1000_hw *hw, u16 mask);
 STATIC bool e1000_sgmii_active_82575(struct e1000_hw *hw);
-STATIC s32  e1000_reset_init_script_82575(struct e1000_hw *hw);
 STATIC s32  e1000_read_mac_addr_82575(struct e1000_hw *hw);
 STATIC void e1000_config_collision_dist_82575(struct e1000_hw *hw);
-STATIC void e1000_power_down_phy_copper_82575(struct e1000_hw *hw);
 STATIC void e1000_shutdown_serdes_link_82575(struct e1000_hw *hw);
 STATIC void e1000_power_up_serdes_link_82575(struct e1000_hw *hw);
 STATIC s32 e1000_set_pcie_completion_timeout(struct e1000_hw *hw);
@@ -127,8 +124,8 @@ STATIC bool e1000_sgmii_uses_mdio_82575(struct e1000_hw *hw)
 }
 
 /**
- *  e1000_init_phy_params_82575 - Init PHY func ptrs.
- *  @hw: pointer to the HW structure
+ * e1000_init_phy_params_82575 - Initialize PHY function ptrs
+ * @hw: pointer to the HW structure
  **/
 STATIC s32 e1000_init_phy_params_82575(struct e1000_hw *hw)
 {
@@ -146,17 +143,17 @@ STATIC s32 e1000_init_phy_params_82575(struct e1000_hw 
*hw)
goto out;
}
 
-   phy->ops.power_up   = e1000_power_up_phy_copper;
-   phy->ops.power_down = e1000_power_down_phy_copper_82575;
+   phy->ops.power_up   = e1000_power_up_phy_copper;
+   phy->ops.power_down = e1000_power_down_phy_copper_base;
 
phy->autoneg_mask   = AUTONEG_ADVERTISE_SPEED_DEFAULT;
phy->reset_delay_us = 100;
 
-   phy->ops.acquire= e1000_acquire_phy_82575;
+   phy->ops.acquire= e1000_acquire_phy_base;
phy->ops.check_reset_block = e1000_check_reset_block_generic;
phy->ops.commit = e1000_phy_sw_reset_generic;
phy->ops.get_cfg_done   = e1000_get_cfg_done_82575;
-   phy->ops.release= e1000_release_phy_82575;
+   phy->ops.release= e1000_release_phy_base;
 
ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
 
@@ -203,76 +200,39 @@ STATIC s32 e1000_init_phy_params_82575(struct e1000_hw 
*hw)
case I347AT4_E_PHY_ID:
case M88E1112_E_PHY_ID:
case M88E1340M_E_PHY_ID:
+  

[dpdk-dev] [PATCH v3 11/27] net/e1000/base: expose MAC functions

[Guinan Sun]

Now the functions are being accessed outside of the file, we need
to properly expose them for silicon families to use.

Signed-off-by: Jeff Kirsher 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_mac.c | 3 +--
 drivers/net/e1000/base/e1000_mac.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_mac.c 
b/drivers/net/e1000/base/e1000_mac.c
index 48384b284..1a2abd9a2 100644
--- a/drivers/net/e1000/base/e1000_mac.c
+++ b/drivers/net/e1000/base/e1000_mac.c
@@ -7,7 +7,6 @@
 STATIC s32 e1000_validate_mdi_setting_generic(struct e1000_hw *hw);
 STATIC void e1000_set_lan_id_multi_port_pcie(struct e1000_hw *hw);
 STATIC void e1000_config_collision_dist_generic(struct e1000_hw *hw);
-STATIC int e1000_rar_set_generic(struct e1000_hw *hw, u8 *addr, u32 index);
 
 /**
  *  e1000_init_mac_ops_generic - Initialize MAC function pointers
@@ -448,7 +447,7 @@ s32 e1000_check_alt_mac_addr_generic(struct e1000_hw *hw)
  *  Sets the receive address array register at index to the address passed
  *  in by addr.
  **/
-STATIC int e1000_rar_set_generic(struct e1000_hw *hw, u8 *addr, u32 index)
+int e1000_rar_set_generic(struct e1000_hw *hw, u8 *addr, u32 index)
 {
u32 rar_low, rar_high;
 
diff --git a/drivers/net/e1000/base/e1000_mac.h 
b/drivers/net/e1000/base/e1000_mac.h
index bbd2a7388..35a691f63 100644
--- a/drivers/net/e1000/base/e1000_mac.h
+++ b/drivers/net/e1000/base/e1000_mac.h
@@ -41,6 +41,7 @@ s32  e1000_led_on_generic(struct e1000_hw *hw);
 s32  e1000_led_off_generic(struct e1000_hw *hw);
 void e1000_update_mc_addr_list_generic(struct e1000_hw *hw,
   u8 *mc_addr_list, u32 mc_addr_count);
+int e1000_rar_set_generic(struct e1000_hw *hw, u8 *addr, u32 index);
 s32  e1000_set_default_fc_generic(struct e1000_hw *hw);
 s32  e1000_set_fc_watermarks_generic(struct e1000_hw *hw);
 s32  e1000_setup_fiber_serdes_link_generic(struct e1000_hw *hw);
-- 
2.17.1

[dpdk-dev] [PATCH v3 12/27] net/e1000/base: add define to PCIm function state

[Guinan Sun]

Added define to pcim function state.

Signed-off-by: Lifshits Vitaly 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_defines.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/e1000/base/e1000_defines.h 
b/drivers/net/e1000/base/e1000_defines.h
index 06759cc3b..41a2b5c3f 100644
--- a/drivers/net/e1000/base/e1000_defines.h
+++ b/drivers/net/e1000/base/e1000_defines.h
@@ -316,6 +316,7 @@
 #define E1000_STATUS_PCIX_SPEED_66 0x /* PCI-X bus spd 50-66MHz */
 #define E1000_STATUS_PCIX_SPEED_1000x4000 /* PCI-X bus spd 66-100MHz */
 #define E1000_STATUS_PCIX_SPEED_1330x8000 /* PCI-X bus spd 100-133MHz*/
+#define E1000_STATUS_PCIM_STATE0x4000 /* PCIm function 
state */
 
 #define SPEED_10   10
 #define SPEED_100  100
-- 
2.17.1

[dpdk-dev] [PATCH v3 14/27] net/e1000/base: increased timeout for ME ULP exit

[Guinan Sun]

Due timing issues in WHL and since recovery by host is
not always supported, increased timeout for Manageability Engine(ME)
to finish Ultra Low Power(ULP) exit flow for Nahum before timer expiration.

Signed-off-by: Efrati Nir 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_ich8lan.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.c 
b/drivers/net/e1000/base/e1000_ich8lan.c
index 1dc29553e..b79e3bad8 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -1268,6 +1268,7 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool 
to_sx)
 s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
 {
s32 ret_val = E1000_SUCCESS;
+   u8 ulp_exit_timeout = 30;
u32 mac_reg;
u16 phy_reg;
int i = 0;
@@ -1289,10 +1290,12 @@ s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool 
force)
E1000_WRITE_REG(hw, E1000_H2ME, mac_reg);
}
 
-   /* Poll up to 300msec for ME to clear ULP_CFG_DONE. */
+   if (hw->mac.type == e1000_pch_cnp)
+   ulp_exit_timeout = 100;
+
while (E1000_READ_REG(hw, E1000_FWSM) &
   E1000_FWSM_ULP_CFG_DONE) {
-   if (i++ == 30) {
+   if (i++ == ulp_exit_timeout) {
ret_val = -E1000_ERR_PHY;
goto out;
}
-- 
2.17.1

[dpdk-dev] [PATCH v3 08/27] net/e1000/base: improve code style and fix klocwork errors

[Guinan Sun]

This patch fixes coding style and klocwork errors.

Fix typo in piece of code of NVM access for SPT.
And cleans up the remaining instances in the shared code
where it was not adhering to the Linux code standard.
Wrong description was found in the mentioned file, so fix them.
Remove shadowing variable declarations.

Relating to operands in bitwise operations having different sizes.
Unreachable code since *clock_in_i2c_* always return success.
Don't return unused s32 and don't check for constants.

Signed-off-by: Jeff Kirsher 
Signed-off-by: Neftin Sasha 
Signed-off-by: Lifshits Vitaly 
Signed-off-by: Robert Konklewski 
Signed-off-by: Doug Dziggel 
Signed-off-by: Todd Fujinaka 
Signed-off-by: Jacob Keller 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_80003es2lan.c |  1 -
 drivers/net/e1000/base/e1000_82575.c   | 18 --
 drivers/net/e1000/base/e1000_i210.c| 10 +++---
 drivers/net/e1000/base/e1000_ich8lan.c | 40 --
 drivers/net/e1000/base/e1000_nvm.c |  2 +-
 drivers/net/e1000/base/e1000_phy.c |  5 ++-
 6 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_80003es2lan.c 
b/drivers/net/e1000/base/e1000_80003es2lan.c
index bcfda9415..b795491c0 100644
--- a/drivers/net/e1000/base/e1000_80003es2lan.c
+++ b/drivers/net/e1000/base/e1000_80003es2lan.c
@@ -1210,7 +1210,6 @@ STATIC s32 e1000_setup_copper_link_80003es2lan(struct 
e1000_hw *hw)
 /**
  *  e1000_cfg_on_link_up_80003es2lan - es2 link configuration after link-up
  *  @hw: pointer to the HW structure
- *  @duplex: current duplex setting
  *
  *  Configure the KMRN interface by applying last minute quirks for
  *  10/100 operation.
diff --git a/drivers/net/e1000/base/e1000_82575.c 
b/drivers/net/e1000/base/e1000_82575.c
index 750f1cf01..410f2db41 100644
--- a/drivers/net/e1000/base/e1000_82575.c
+++ b/drivers/net/e1000/base/e1000_82575.c
@@ -75,10 +75,10 @@ STATIC void e1000_clear_vfta_i350(struct e1000_hw *hw);
 
 STATIC void e1000_i2c_start(struct e1000_hw *hw);
 STATIC void e1000_i2c_stop(struct e1000_hw *hw);
-STATIC s32 e1000_clock_in_i2c_byte(struct e1000_hw *hw, u8 *data);
+STATIC void e1000_clock_in_i2c_byte(struct e1000_hw *hw, u8 *data);
 STATIC s32 e1000_clock_out_i2c_byte(struct e1000_hw *hw, u8 data);
 STATIC s32 e1000_get_i2c_ack(struct e1000_hw *hw);
-STATIC s32 e1000_clock_in_i2c_bit(struct e1000_hw *hw, bool *data);
+STATIC void e1000_clock_in_i2c_bit(struct e1000_hw *hw, bool *data);
 STATIC s32 e1000_clock_out_i2c_bit(struct e1000_hw *hw, bool data);
 STATIC void e1000_raise_i2c_clk(struct e1000_hw *hw, u32 *i2cctl);
 STATIC void e1000_lower_i2c_clk(struct e1000_hw *hw, u32 *i2cctl);
@@ -1084,7 +1084,7 @@ STATIC void e1000_release_swfw_sync_82575(struct e1000_hw 
*hw, u16 mask)
; /* Empty */
 
swfw_sync = E1000_READ_REG(hw, E1000_SW_FW_SYNC);
-   swfw_sync &= ~mask;
+   swfw_sync &= (u32)~mask;
E1000_WRITE_REG(hw, E1000_SW_FW_SYNC, swfw_sync);
 
e1000_put_hw_semaphore_generic(hw);
@@ -3300,9 +3300,7 @@ s32 e1000_read_i2c_byte_generic(struct e1000_hw *hw, u8 
byte_offset,
if (status != E1000_SUCCESS)
goto fail;
 
-   status = e1000_clock_in_i2c_byte(hw, data);
-   if (status != E1000_SUCCESS)
-   goto fail;
+   e1000_clock_in_i2c_byte(hw, data);
 
status = e1000_clock_out_i2c_bit(hw, nack);
if (status != E1000_SUCCESS)
@@ -3466,7 +3464,7 @@ STATIC void e1000_i2c_stop(struct e1000_hw *hw)
  *
  *  Clocks in one byte data via I2C data/clock
  **/
-STATIC s32 e1000_clock_in_i2c_byte(struct e1000_hw *hw, u8 *data)
+STATIC void e1000_clock_in_i2c_byte(struct e1000_hw *hw, u8 *data)
 {
s32 i;
bool bit = 0;
@@ -3478,8 +3476,6 @@ STATIC s32 e1000_clock_in_i2c_byte(struct e1000_hw *hw, 
u8 *data)
e1000_clock_in_i2c_bit(hw, &bit);
*data |= bit << i;
}
-
-   return E1000_SUCCESS;
 }
 
 /**
@@ -3568,7 +3564,7 @@ STATIC s32 e1000_get_i2c_ack(struct e1000_hw *hw)
  *
  *  Clocks in one bit via I2C data/clock
  **/
-STATIC s32 e1000_clock_in_i2c_bit(struct e1000_hw *hw, bool *data)
+STATIC void e1000_clock_in_i2c_bit(struct e1000_hw *hw, bool *data)
 {
u32 i2cctl = E1000_READ_REG(hw, E1000_I2CPARAMS);
 
@@ -3586,8 +3582,6 @@ STATIC s32 e1000_clock_in_i2c_bit(struct e1000_hw *hw, 
bool *data)
 
/* Minimum low period of clock is 4.7 us */
usec_delay(E1000_I2C_T_LOW);
-
-   return E1000_SUCCESS;
 }
 
 /**
diff --git a/drivers/net/e1000/base/e1000_i210.c 
b/drivers/net/e1000/base/e1000_i210.c
index 5c2fe8a3e..a6d8e3b34 100644
--- a/drivers/net/e1000/base/e1000_i210.c
+++ b/drivers/net/e1000/base/e1000_i210.c
@@ -117,7 +117,7 @@ void e1000_release_swfw_sync_i210(struct e1000_hw *hw, u16 
mask)
; /* Empty */
 
swfw_sync = E1000_READ_REG(hw, E1000_SW_FW_SYNC);
-   

[dpdk-dev] [PATCH v3 18/27] net/e1000/base: add missed define for VFTA

[Guinan Sun]

VLAN filtering using the VFTA (VLAN Filter Table Array) and
should be initialized prior to setting rx mode.

Signed-off-by: Sasha Neftin 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_defines.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/e1000/base/e1000_defines.h 
b/drivers/net/e1000/base/e1000_defines.h
index 41a2b5c3f..3d015fb20 100644
--- a/drivers/net/e1000/base/e1000_defines.h
+++ b/drivers/net/e1000/base/e1000_defines.h
@@ -1392,6 +1392,7 @@
 #define E1000_MDIC_ERROR   0x4000
 #define E1000_MDIC_DEST0x8000
 
+#define E1000_VFTA_BLOCK_SIZE  8
 /* SerDes Control */
 #define E1000_GEN_CTL_READY0x8000
 #define E1000_GEN_CTL_ADDRESS_SHIFT8
-- 
2.17.1

[dpdk-dev] [PATCH v3 17/27] net/e1000/base: remove useless statement

[Guinan Sun]

As it stands now, this fix will not change the current functionality
of the code. In addition, we remove the comment that seemed to be
a copy/paste from a separate implementation.

Signed-off-by: Jeb Cramer 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_ich8lan.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.c 
b/drivers/net/e1000/base/e1000_ich8lan.c
index b79e3bad8..9b9cc7d90 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -4113,13 +4113,6 @@ STATIC s32 e1000_update_nvm_checksum_spt(struct e1000_hw 
*hw)
if (ret_val)
goto release;
 
-   /* And invalidate the previously valid segment by setting
-* its signature word (0x13) high_byte to 0b. This can be
-* done without an erase because flash erase sets all bits
-* to 1's. We can write 1's to 0's without an erase
-*/
-   act_offset = (old_bank_offset + E1000_ICH_NVM_SIG_WORD) * 2 + 1;
-
/* offset in words but we read dword*/
act_offset = old_bank_offset + E1000_ICH_NVM_SIG_WORD - 1;
ret_val = e1000_read_flash_dword_ich8lan(hw, act_offset, &dword);
-- 
2.17.1

[dpdk-dev] [PATCH v3 15/27] net/e1000/base: add missing device ID

[Guinan Sun]

Adding Intel(R) I210 Gigabit Network Connection 15F6 device ID for SGMII
flashless automotive device.

Signed-off-by: Kamil Bednarczyk 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_api.c | 1 +
 drivers/net/e1000/base/e1000_hw.h  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_api.c 
b/drivers/net/e1000/base/e1000_api.c
index a93e8ff03..a9a4493ac 100644
--- a/drivers/net/e1000/base/e1000_api.c
+++ b/drivers/net/e1000/base/e1000_api.c
@@ -326,6 +326,7 @@ s32 e1000_set_mac_type(struct e1000_hw *hw)
break;
case E1000_DEV_ID_I210_COPPER_FLASHLESS:
case E1000_DEV_ID_I210_SERDES_FLASHLESS:
+   case E1000_DEV_ID_I210_SGMII_FLASHLESS:
case E1000_DEV_ID_I210_COPPER:
case E1000_DEV_ID_I210_COPPER_OEM1:
case E1000_DEV_ID_I210_COPPER_IT:
diff --git a/drivers/net/e1000/base/e1000_hw.h 
b/drivers/net/e1000/base/e1000_hw.h
index 688174200..b71cb4d4d 100644
--- a/drivers/net/e1000/base/e1000_hw.h
+++ b/drivers/net/e1000/base/e1000_hw.h
@@ -158,6 +158,7 @@ struct e1000_hw;
 #define E1000_DEV_ID_I210_SGMII0x1538
 #define E1000_DEV_ID_I210_COPPER_FLASHLESS 0x157B
 #define E1000_DEV_ID_I210_SERDES_FLASHLESS 0x157C
+#define E1000_DEV_ID_I210_SGMII_FLASHLESS  0x15F6
 #define E1000_DEV_ID_I211_COPPER   0x1539
 #define E1000_DEV_ID_I354_BACKPLANE_1GBPS  0x1F40
 #define E1000_DEV_ID_I354_SGMII0x1F41
-- 
2.17.1

[dpdk-dev] [PATCH v3 16/27] net/e1000/base: expose more future extended NVM

[Guinan Sun]

Future extended NVM 5 (five) required for a Dynamic Power Gating
control in the MAC.

Signed-off-by: Sasha Neftin 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/e1000/base/e1000_regs.h 
b/drivers/net/e1000/base/e1000_regs.h
index 4af9e1746..71c8d28b0 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -35,6 +35,7 @@
 #define E1000_FEXTNVM  0x00028  /* Future Extended NVM - RW */
 #define E1000_FEXTNVM3 0x0003C  /* Future Extended NVM 3 - RW */
 #define E1000_FEXTNVM4 0x00024  /* Future Extended NVM 4 - RW */
+#define E1000_FEXTNVM5 0x00014  /* Future Extended NVM 5 - RW */
 #define E1000_FEXTNVM6 0x00010  /* Future Extended NVM 6 - RW */
 #define E1000_FEXTNVM7 0x000E4  /* Future Extended NVM 7 - RW */
 #define E1000_FEXTNVM9 0x5BB4  /* Future Extended NVM 9 - RW */
-- 
2.17.1

[dpdk-dev] [PATCH v3 13/27] net/e1000/base: add missing register defines

[Guinan Sun]

Added defines for the EEC, SHADOWINF and FLFWUPDATE registers needed for
the nvmupd_validate_offset function to correctly validate the NVM update
offset.

Signed-off-by: Jeff Kirsher 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_regs.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_regs.h 
b/drivers/net/e1000/base/e1000_regs.h
index 9edd3c528..4af9e1746 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -140,6 +140,8 @@
 #define E1000_EMIDATA  0x11 /* Extended Memory Indirect Data */
 /* Shadow Ram Write Register - RW */
 #define E1000_SRWR 0x12018
+#define E1000_EEC_REG  0x12010
+
 #define E1000_I210_FLMNGCTL0x12038
 #define E1000_I210_FLMNGDATA   0x1203C
 #define E1000_I210_FLMNGCNT0x12040
@@ -150,6 +152,9 @@
 
 #define E1000_I210_FLA 0x1201C
 
+#define E1000_SHADOWINF0x12068
+#define E1000_FLFWUPDATE   0x12108
+
 #define E1000_INVM_DATA_REG(_n)(0x12120 + 4*(_n))
 #define E1000_INVM_SIZE64 /* Number of INVM Data Registers */
 
-- 
2.17.1

[dpdk-dev] [PATCH v3 20/27] net/e1000/base: led blinking fix for i210

[Guinan Sun]

Added initialization of identification LED.

Signed-off-by: Maciej Hefczyc 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_i210.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_i210.c 
b/drivers/net/e1000/base/e1000_i210.c
index 553803261..d32b0f089 100644
--- a/drivers/net/e1000/base/e1000_i210.c
+++ b/drivers/net/e1000/base/e1000_i210.c
@@ -925,6 +925,7 @@ STATIC s32 e1000_get_cfg_done_i210(struct e1000_hw *hw)
 s32 e1000_init_hw_i210(struct e1000_hw *hw)
 {
s32 ret_val;
+   struct e1000_mac_info *mac = &hw->mac;
 
DEBUGFUNC("e1000_init_hw_i210");
if ((hw->mac.type >= e1000_i210) &&
@@ -934,6 +935,10 @@ s32 e1000_init_hw_i210(struct e1000_hw *hw)
return ret_val;
}
hw->phy.ops.get_cfg_done = e1000_get_cfg_done_i210;
+
+   /* Initialize identification LED */
+   ret_val = mac->ops.id_led_init(hw);
+
ret_val = e1000_init_hw_base(hw);
return ret_val;
 }
-- 
2.17.1

[dpdk-dev] [PATCH v3 19/27] net/e1000/base: modify flow control setup

[Guinan Sun]

Customers had a problem with large pings after connected standby.
This is due to the requirement of maintaining link after CS - the driver
blocks resets during "AdapterStart" and skips flow control setup.
Added condition in e1000_setup_link_ich8lan.c function that always setup
flow control, and setup physical interface only when no need to block
resets.

Signed-off-by: Efrati Nir 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_ich8lan.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.c 
b/drivers/net/e1000/base/e1000_ich8lan.c
index 9b9cc7d90..85344ebeb 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -5200,9 +5200,6 @@ STATIC s32 e1000_setup_link_ich8lan(struct e1000_hw *hw)
 
DEBUGFUNC("e1000_setup_link_ich8lan");
 
-   if (hw->phy.ops.check_reset_block(hw))
-   return E1000_SUCCESS;
-
/* ICH parts do not have a word in the NVM to determine
 * the default flow control setting, so we explicitly
 * set it to full.
@@ -5218,10 +5215,12 @@ STATIC s32 e1000_setup_link_ich8lan(struct e1000_hw *hw)
DEBUGOUT1("After fix-ups FlowControl is now = %x\n",
hw->fc.current_mode);
 
-   /* Continue to configure the copper link. */
-   ret_val = hw->mac.ops.setup_physical_interface(hw);
-   if (ret_val)
-   return ret_val;
+   if (!hw->phy.ops.check_reset_block(hw)) {
+   /* Continue to configure the copper link. */
+   ret_val = hw->mac.ops.setup_physical_interface(hw);
+   if (ret_val)
+   return ret_val;
+   }
 
E1000_WRITE_REG(hw, E1000_FCTTV, hw->fc.pause_time);
if ((hw->phy.type == e1000_phy_82578) ||
-- 
2.17.1

[dpdk-dev] [PATCH v3 22/27] net/e1000/base: add support for Nahum10

[Guinan Sun]

Add support to a new MAC type (for Nahum10).

Signed-off-by: Roman Fridlyand 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_hw.h  | 1 +
 drivers/net/e1000/base/e1000_ich8lan.c | 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_hw.h 
b/drivers/net/e1000/base/e1000_hw.h
index b71cb4d4d..11acec27e 100644
--- a/drivers/net/e1000/base/e1000_hw.h
+++ b/drivers/net/e1000/base/e1000_hw.h
@@ -212,6 +212,7 @@ enum e1000_mac_type {
e1000_pch_lpt,
e1000_pch_spt,
e1000_pch_cnp,
+   e1000_pch_adp,
e1000_82575,
e1000_82576,
e1000_82580,
diff --git a/drivers/net/e1000/base/e1000_ich8lan.c 
b/drivers/net/e1000/base/e1000_ich8lan.c
index 85344ebeb..61dcc1e61 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -318,6 +318,7 @@ STATIC s32 e1000_init_phy_workarounds_pchlan(struct 
e1000_hw *hw)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
if (e1000_phy_is_accessible_pchlan(hw))
break;
 
@@ -467,6 +468,7 @@ STATIC s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
/* In case the PHY needs to be in mdio slow mode,
 * set slow mode and try to get the PHY id again.
 */
@@ -772,12 +774,11 @@ STATIC s32 e1000_init_mac_params_ich8lan(struct e1000_hw 
*hw)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
-#ifndef NO_NON_BLOCKING_PHY_MTA_UPDATE_SUPPORT
+   case e1000_pch_adp:
/* multicast address update for pch2 */
mac->ops.update_mc_addr_list =
e1000_update_mc_addr_list_pch2lan;
/* fall-through */
-#endif
case e1000_pchlan:
 #if defined(QV_RELEASE) || !defined(NO_PCH_LPT_B0_SUPPORT)
/* save PCH revision_id */
@@ -1764,6 +1765,7 @@ void e1000_init_function_pointers_ich8lan(struct e1000_hw 
*hw)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
hw->phy.ops.init_params = e1000_init_phy_params_pchlan;
break;
default:
@@ -2231,6 +2233,7 @@ STATIC s32 e1000_sw_lcd_config_ich8lan(struct e1000_hw 
*hw)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
sw_cfg_mask = E1000_FEXTNVM_SW_CONFIG_ICH8M;
break;
default:
@@ -3358,6 +3361,7 @@ STATIC s32 e1000_valid_nvm_bank_detect_ich8lan(struct 
e1000_hw *hw, u32 *bank)
switch (hw->mac.type) {
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
bank1_offset = nvm->flash_bank_size;
act_offset = E1000_ICH_NVM_SIG_WORD;
 
@@ -4329,6 +4333,7 @@ STATIC s32 e1000_validate_nvm_checksum_ich8lan(struct 
e1000_hw *hw)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
word = NVM_COMPAT;
valid_csum_mask = NVM_COMPAT_VALID_CSUM;
break;
-- 
2.17.1

[dpdk-dev] [PATCH v3 21/27] net/e1000/base: expose new FEXTNVM registers and masks

[Guinan Sun]

Adding defines for FEXTNVM8 and FEXTNVM12 registers with new masks for
future use.

Signed-off-by: Nir Efrati 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_ich8lan.h | 3 ++-
 drivers/net/e1000/base/e1000_regs.h| 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/e1000/base/e1000_ich8lan.h 
b/drivers/net/e1000/base/e1000_ich8lan.h
index bfee467b3..3be3ee246 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.h
+++ b/drivers/net/e1000/base/e1000_ich8lan.h
@@ -92,11 +92,12 @@
 #if !defined(EXTERNAL_RELEASE) || defined(ULP_SUPPORT)
 #define E1000_FEXTNVM7_DISABLE_SMB_PERST   0x0020
 #endif /* !EXTERNAL_RELEASE || ULP_SUPPORT */
+#define E1000_FEXTNVM8_UNBIND_DPG_FROM_MPHY0x0400
 #define E1000_FEXTNVM9_IOSFSB_CLKGATE_DIS  0x0800
 #define E1000_FEXTNVM9_IOSFSB_CLKREQ_DIS   0x1000
 #define E1000_FEXTNVM11_DISABLE_PB_READ0x0200
 #define E1000_FEXTNVM11_DISABLE_MULR_FIX   0x2000
-
+#define E1000_FEXTNVM12_DONT_WAK_DPG_CLKREQ0x1000
 /* bit24: RXDCTL thresholds granularity: 0 - cache lines, 1 - descriptors */
 #define E1000_RXDCTL_THRESH_UNIT_DESC  0x0100
 
diff --git a/drivers/net/e1000/base/e1000_regs.h 
b/drivers/net/e1000/base/e1000_regs.h
index 71c8d28b0..8ee1c768c 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -38,8 +38,10 @@
 #define E1000_FEXTNVM5 0x00014  /* Future Extended NVM 5 - RW */
 #define E1000_FEXTNVM6 0x00010  /* Future Extended NVM 6 - RW */
 #define E1000_FEXTNVM7 0x000E4  /* Future Extended NVM 7 - RW */
+#define E1000_FEXTNVM8 0x5BB0  /* Future Extended NVM 8 - RW */
 #define E1000_FEXTNVM9 0x5BB4  /* Future Extended NVM 9 - RW */
 #define E1000_FEXTNVM110x5BBC  /* Future Extended NVM 11 - RW */
+#define E1000_FEXTNVM120x5BC0  /* Future Extended NVM 12 - RW */
 #define E1000_PCIEANACFG   0x00F18 /* PCIE Analog Config */
 #define E1000_FCT  0x00030  /* Flow Control Type - RW */
 #define E1000_CONNSW   0x00034  /* Copper/Fiber switch control - RW */
-- 
2.17.1

[dpdk-dev] [PATCH v3 24/27] net/e1000/base: introduce DPGFR register

[Guinan Sun]

Defined DPGFR, Dynamic Power Gate Force Control Register.

Signed-off-by: Vitaly Lifshits 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/e1000/base/e1000_regs.h 
b/drivers/net/e1000/base/e1000_regs.h
index 8ee1c768c..d3bad9752 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -43,6 +43,7 @@
 #define E1000_FEXTNVM110x5BBC  /* Future Extended NVM 11 - RW */
 #define E1000_FEXTNVM120x5BC0  /* Future Extended NVM 12 - RW */
 #define E1000_PCIEANACFG   0x00F18 /* PCIE Analog Config */
+#define E1000_DPGFR0x00FAC /* Dynamic Power Gate Force Control Register */
 #define E1000_FCT  0x00030  /* Flow Control Type - RW */
 #define E1000_CONNSW   0x00034  /* Copper/Fiber switch control - RW */
 #define E1000_VET  0x00038  /* VLAN Ether Type - RW */
-- 
2.17.1

[dpdk-dev] [PATCH v3 23/27] net/e1000/base: add ADL device ID

[Guinan Sun]

Add new device ID for Alder Lake brand.

Signed-off-by: Sasha Neftin 
Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_api.c | 7 +++
 drivers/net/e1000/base/e1000_hw.h  | 4 
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/e1000/base/e1000_api.c 
b/drivers/net/e1000/base/e1000_api.c
index a9a4493ac..352e31932 100644
--- a/drivers/net/e1000/base/e1000_api.c
+++ b/drivers/net/e1000/base/e1000_api.c
@@ -290,6 +290,12 @@ s32 e1000_set_mac_type(struct e1000_hw *hw)
case E1000_DEV_ID_PCH_ICP_I219_V9:
mac->type = e1000_pch_cnp;
break;
+   case E1000_DEV_ID_PCH_ADL_I219_LM16:
+   case E1000_DEV_ID_PCH_ADL_I219_V16:
+   case E1000_DEV_ID_PCH_ADL_I219_LM17:
+   case E1000_DEV_ID_PCH_ADL_I219_V17:
+   mac->type = e1000_pch_adp;
+   break;
case E1000_DEV_ID_82575EB_COPPER:
case E1000_DEV_ID_82575EB_FIBER_SERDES:
case E1000_DEV_ID_82575GB_QUAD_COPPER:
@@ -443,6 +449,7 @@ s32 e1000_setup_init_funcs(struct e1000_hw *hw, bool 
init_device)
case e1000_pch_lpt:
case e1000_pch_spt:
case e1000_pch_cnp:
+   case e1000_pch_adp:
e1000_init_function_pointers_ich8lan(hw);
break;
case e1000_82575:
diff --git a/drivers/net/e1000/base/e1000_hw.h 
b/drivers/net/e1000/base/e1000_hw.h
index 11acec27e..85d09feae 100644
--- a/drivers/net/e1000/base/e1000_hw.h
+++ b/drivers/net/e1000/base/e1000_hw.h
@@ -124,6 +124,10 @@ struct e1000_hw;
 #define E1000_DEV_ID_PCH_ICP_I219_V8   0x15E0
 #define E1000_DEV_ID_PCH_ICP_I219_LM9  0x15E1
 #define E1000_DEV_ID_PCH_ICP_I219_V9   0x15E2
+#define E1000_DEV_ID_PCH_ADL_I219_LM16 0x1A1E
+#define E1000_DEV_ID_PCH_ADL_I219_V16  0x1A1F
+#define E1000_DEV_ID_PCH_ADL_I219_LM17 0x1A1C
+#define E1000_DEV_ID_PCH_ADL_I219_V17  0x1A1D
 #define E1000_DEV_ID_82576 0x10C9
 #define E1000_DEV_ID_82576_FIBER   0x10E6
 #define E1000_DEV_ID_82576_SERDES  0x10E7
-- 
2.17.1

[dpdk-dev] [PATCH v3 25/27] net/e1000/base: cleanup pre-processor tags

[Guinan Sun]

The codes has been exposed correctly, so remove pre-processor tags.

Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_82575.h   |  2 --
 drivers/net/e1000/base/e1000_defines.h |  9 +
 drivers/net/e1000/base/e1000_hw.h  |  4 
 drivers/net/e1000/base/e1000_ich8lan.c | 19 ---
 drivers/net/e1000/base/e1000_ich8lan.h | 17 +
 drivers/net/e1000/base/e1000_mac.h |  2 --
 drivers/net/e1000/base/e1000_manage.c  |  4 ++--
 drivers/net/e1000/base/e1000_regs.h|  5 +++--
 8 files changed, 7 insertions(+), 55 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_82575.h 
b/drivers/net/e1000/base/e1000_82575.h
index 14887e25f..10383b03f 100644
--- a/drivers/net/e1000/base/e1000_82575.h
+++ b/drivers/net/e1000/base/e1000_82575.h
@@ -26,7 +26,6 @@
 #define E1000_SW_SYNCH_MB  0x0100
 #define E1000_STAT_DEV_RST_SET 0x0010
 
-#ifdef E1000_BIT_FIELDS
 struct e1000_adv_data_desc {
__le64 buffer_addr;/* Address of the descriptor's data buffer */
union {
@@ -89,7 +88,6 @@ struct e1000_adv_context_desc {
} fields;
} l4_setup;
 };
-#endif
 
 /* SRRCTL bit definitions */
 #define E1000_SRRCTL_BSIZEPKT_SHIFT10 /* Shift _right_ */
diff --git a/drivers/net/e1000/base/e1000_defines.h 
b/drivers/net/e1000/base/e1000_defines.h
index 3d015fb20..c09635344 100644
--- a/drivers/net/e1000/base/e1000_defines.h
+++ b/drivers/net/e1000/base/e1000_defines.h
@@ -128,9 +128,7 @@
E1000_RXDEXT_STATERR_CXE |  \
E1000_RXDEXT_STATERR_RXE)
 
-#if !defined(EXTERNAL_RELEASE) || defined(E1000E_MQ)
 #define E1000_MRQC_ENABLE_RSS_2Q   0x0001
-#endif /* !EXTERNAL_RELEASE || E1000E_MQ */
 #define E1000_MRQC_RSS_FIELD_MASK  0x
 #define E1000_MRQC_RSS_FIELD_IPV4_TCP  0x0001
 #define E1000_MRQC_RSS_FIELD_IPV4  0x0002
@@ -1221,9 +1219,7 @@
 #define PCIE_LINK_SPEED_5000   0x02
 #define PCIE_DEVICE_CONTROL2_16ms  0x0005
 
-#ifndef ETH_ADDR_LEN
 #define ETH_ADDR_LEN   6
-#endif
 
 #define PHY_REVISION_MASK  0xFFF0
 #define MAX_PHY_REG_ADDRESS0x1F  /* 5 bit address bus (0-0x1F) */
@@ -1490,10 +1486,7 @@
 /* Lan ID bit field offset in status register */
 #define E1000_STATUS_LAN_ID_OFFSET 2
 #define E1000_VFTA_ENTRIES 128
-#ifndef E1000_UNUSEDARG
+
 #define E1000_UNUSEDARG
-#endif /* E1000_UNUSEDARG */
-#ifndef ERROR_REPORT
 #define ERROR_REPORT(fmt)  do { } while (0)
-#endif /* ERROR_REPORT */
 #endif /* _E1000_DEFINES_H_ */
diff --git a/drivers/net/e1000/base/e1000_hw.h 
b/drivers/net/e1000/base/e1000_hw.h
index 85d09feae..6e2697248 100644
--- a/drivers/net/e1000/base/e1000_hw.h
+++ b/drivers/net/e1000/base/e1000_hw.h
@@ -931,7 +931,6 @@ struct e1000_shadow_ram {
 
 #define E1000_SHADOW_RAM_WORDS 2048
 
-#ifdef ULP_SUPPORT
 /* I218 PHY Ultra Low Power (ULP) states */
 enum e1000_ulp_state {
e1000_ulp_state_unknown,
@@ -939,7 +938,6 @@ enum e1000_ulp_state {
e1000_ulp_state_on,
 };
 
-#endif /* ULP_SUPPORT */
 struct e1000_dev_spec_ich8lan {
bool kmrn_lock_loss_workaround_enabled;
struct e1000_shadow_ram shadow_ram[E1000_SHADOW_RAM_WORDS];
@@ -949,12 +947,10 @@ struct e1000_dev_spec_ich8lan {
bool disable_k1_off;
bool eee_disable;
u16 eee_lp_ability;
-#ifdef ULP_SUPPORT
enum e1000_ulp_state ulp_state;
bool ulp_capability_disabled;
bool during_suspend_flow;
bool during_dpg_exit;
-#endif /* ULP_SUPPORT */
u16 lat_enc;
u16 max_ltr_enc;
bool smbus_disable;
diff --git a/drivers/net/e1000/base/e1000_ich8lan.c 
b/drivers/net/e1000/base/e1000_ich8lan.c
index 61dcc1e61..bf77bfa1d 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -51,11 +51,9 @@ STATIC bool e1000_check_mng_mode_pchlan(struct e1000_hw *hw);
 STATIC int  e1000_rar_set_pch2lan(struct e1000_hw *hw, u8 *addr, u32 index);
 STATIC int  e1000_rar_set_pch_lpt(struct e1000_hw *hw, u8 *addr, u32 index);
 STATIC s32 e1000_sw_lcd_config_ich8lan(struct e1000_hw *hw);
-#ifndef NO_NON_BLOCKING_PHY_MTA_UPDATE_SUPPORT
 STATIC void e1000_update_mc_addr_list_pch2lan(struct e1000_hw *hw,
  u8 *mc_addr_list,
  u32 mc_addr_count);
-#endif /* NO_NON_BLOCKING_PHY_MTA_UPDATE_SUPPORT */
 STATIC s32  e1000_check_reset_block_ich8lan(struct e1000_hw *hw);
 STATIC s32  e1000_phy_hw_reset_ich8lan(struct e1000_hw *hw);
 STATIC s32  e1000_set_lplu_state_pchlan(struct e1000_hw *hw, bool active);
@@ -297,13 +295,11 @@ STATIC s32 e1000_init_phy_workarounds_pchlan(struct 
e1000_hw *hw)
 */
e1000_gate_hw_phy_config_ich8lan(hw, true);
 
-#ifdef ULP_SUPPORT
/* It is not possible to be certain of the current state of ULP
 * so forcibly disable it.
 */
hw->dev_spe

[dpdk-dev] [PATCH v3 27/27] net/e1000/base: update version

[Guinan Sun]

Update base code version in readme.

Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/README | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/base/README b/drivers/net/e1000/base/README
index 56738d001..b84ee5ad6 100644
--- a/drivers/net/e1000/base/README
+++ b/drivers/net/e1000/base/README
@@ -1,9 +1,9 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2016 Intel Corporation
+ * Copyright(c) 2010-2020 Intel Corporation
  */
 
 This directory contains source code of FreeBSD em & igb drivers of version
-cid-shared-code.2016.11.22 released by ND. The sub-directory of base/
+cid-gigabit.2020.06.05.tar.gz released by ND. The sub-directory of base/
 contains the original source package.
 
 This driver is valid for the product(s) listed below
-- 
2.17.1

Re: [dpdk-dev] Epoll installation in FreeBSD

[Bruce Richardson]

On Sun, Jul 05, 2020 at 03:27:30PM +0500, Ibtisam Tariq wrote:
> Hey.
> 
> I've tried to install a new copy of FreeBSD. But it can't find the
> sys/epoll.h header file.
> I have searched and sys/epoll.h is not available on FreeBSD.
> 
> The output of meson builds on FreeBSD.
> --
> Has header "sys/epoll.h" : NO
> examples/meson.build:109.2: ERROR: Problem encountered: Cannot build
> requested example "ip_pipeline"
> --
> 
> How can I solve this problem?
> 
The ip_pipeline example application is indeed not buildable on FreeBSD,
sadly, so there is no way to solve this problem without modifying the code
to not require the use of epoll. To get a clean DPDK build on FreeBSD you
need to remove this example from the list of examples you are requesting to
be built as part of the SDK build.

One suggestion to find all the examples which are working on FreeBSD is to
pass "-Dexamples=all" to meson, rather than requesting each by name. This
will attempt to build all examples, but won't error out on any that are not
buildable, instead it will just skip them.

Regards,
/Bruce

[dpdk-dev] [PATCH v3 26/27] net/e1000/base: modify copyright

[Guinan Sun]

Modify the copyright of the file header.

Signed-off-by: Guinan Sun 
---
 drivers/net/e1000/base/e1000_80003es2lan.c | 2 +-
 drivers/net/e1000/base/e1000_80003es2lan.h | 2 +-
 drivers/net/e1000/base/e1000_82540.c   | 2 +-
 drivers/net/e1000/base/e1000_82541.c   | 2 +-
 drivers/net/e1000/base/e1000_82541.h   | 2 +-
 drivers/net/e1000/base/e1000_82542.c   | 2 +-
 drivers/net/e1000/base/e1000_82543.c   | 2 +-
 drivers/net/e1000/base/e1000_82543.h   | 2 +-
 drivers/net/e1000/base/e1000_82571.c   | 2 +-
 drivers/net/e1000/base/e1000_82571.h   | 2 +-
 drivers/net/e1000/base/e1000_82575.c   | 2 +-
 drivers/net/e1000/base/e1000_82575.h   | 2 +-
 drivers/net/e1000/base/e1000_api.c | 2 +-
 drivers/net/e1000/base/e1000_api.h | 2 +-
 drivers/net/e1000/base/e1000_defines.h | 2 +-
 drivers/net/e1000/base/e1000_hw.h  | 2 +-
 drivers/net/e1000/base/e1000_i210.c| 2 +-
 drivers/net/e1000/base/e1000_i210.h| 2 +-
 drivers/net/e1000/base/e1000_ich8lan.c | 2 +-
 drivers/net/e1000/base/e1000_ich8lan.h | 2 +-
 drivers/net/e1000/base/e1000_mac.c | 2 +-
 drivers/net/e1000/base/e1000_mac.h | 2 +-
 drivers/net/e1000/base/e1000_manage.c  | 2 +-
 drivers/net/e1000/base/e1000_manage.h  | 3 +--
 drivers/net/e1000/base/e1000_mbx.c | 2 +-
 drivers/net/e1000/base/e1000_mbx.h | 2 +-
 drivers/net/e1000/base/e1000_nvm.c | 2 +-
 drivers/net/e1000/base/e1000_nvm.h | 2 +-
 drivers/net/e1000/base/e1000_phy.c | 2 +-
 drivers/net/e1000/base/e1000_phy.h | 2 +-
 drivers/net/e1000/base/e1000_regs.h| 2 +-
 drivers/net/e1000/base/e1000_vf.c  | 2 +-
 drivers/net/e1000/base/e1000_vf.h  | 2 +-
 33 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_80003es2lan.c 
b/drivers/net/e1000/base/e1000_80003es2lan.c
index b795491c0..243bc6fe3 100644
--- a/drivers/net/e1000/base/e1000_80003es2lan.c
+++ b/drivers/net/e1000/base/e1000_80003es2lan.c
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 /* 80003ES2LAN Gigabit Ethernet Controller (Copper)
diff --git a/drivers/net/e1000/base/e1000_80003es2lan.h 
b/drivers/net/e1000/base/e1000_80003es2lan.h
index f77beb253..74264362b 100644
--- a/drivers/net/e1000/base/e1000_80003es2lan.h
+++ b/drivers/net/e1000/base/e1000_80003es2lan.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 #ifndef _E1000_80003ES2LAN_H_
diff --git a/drivers/net/e1000/base/e1000_82540.c 
b/drivers/net/e1000/base/e1000_82540.c
index 0d14f6e07..068f9f68f 100644
--- a/drivers/net/e1000/base/e1000_82540.c
+++ b/drivers/net/e1000/base/e1000_82540.c
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 /*
diff --git a/drivers/net/e1000/base/e1000_82541.c 
b/drivers/net/e1000/base/e1000_82541.c
index eb80873f5..74efd8b6f 100644
--- a/drivers/net/e1000/base/e1000_82541.c
+++ b/drivers/net/e1000/base/e1000_82541.c
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 /*
diff --git a/drivers/net/e1000/base/e1000_82541.h 
b/drivers/net/e1000/base/e1000_82541.h
index 2f343f182..72fc07d0e 100644
--- a/drivers/net/e1000/base/e1000_82541.h
+++ b/drivers/net/e1000/base/e1000_82541.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 #ifndef _E1000_82541_H_
diff --git a/drivers/net/e1000/base/e1000_82542.c 
b/drivers/net/e1000/base/e1000_82542.c
index 9351ed722..fd473c1c6 100644
--- a/drivers/net/e1000/base/e1000_82542.c
+++ b/drivers/net/e1000/base/e1000_82542.c
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 /*
diff --git a/drivers/net/e1000/base/e1000_82543.c 
b/drivers/net/e1000/base/e1000_82543.c
index dfde2a8b9..ca273b436 100644
--- a/drivers/net/e1000/base/e1000_82543.c
+++ b/drivers/net/e1000/base/e1000_82543.c
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 /*
diff --git a/drivers/net/e1000/base/e1000_82543.h 
b/drivers/net/e1000/base/e1000_82543.h
index 51a421190..cf81e4e84 100644
--- a/drivers/net/e1000/base/e1000_82543.h
+++ b/drivers/net/e1000/base/e1000_82543.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2001 - 2015 Intel Corporation
+ * Copyright(c) 2001-2020 Intel Corporation
  */
 
 #ifndef _E1000_82543_H_
diff --git a/drivers/net/e1000/base/e1000_82571.c 
b/d

Re: [dpdk-dev] [PATCH] maintainers: update for netvsc

[Ferruh Yigit]

On 6/30/2020 4:04 PM, Haiyang Zhang wrote:
> 
> 
>> -Original Message-
>> From: lon...@linuxonhyperv.com 
>> Sent: Thursday, June 25, 2020 4:30 PM
>> To: KY Srinivasan ; Haiyang Zhang
>> ; Stephen Hemminger 
>> Cc: dev@dpdk.org; Long Li 
>> Subject: [PATCH] maintainers: update for netvsc
>>
>> From: Long Li 
>>
>> Add Long Li as additional maintainer.
>>
>> Signed-off-by: Long Li 
>> ---
>>  MAINTAINERS | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 15485c97c..9d63a128a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -775,6 +775,7 @@ Microsoft Hyper-V netvsc - EXPERIMENTAL
>>  M: Stephen Hemminger 
>>  M: K. Y. Srinivasan 
>>  M: Haiyang Zhang 
>> +M: Long Li 
>>  F: drivers/net/netvsc/
>>  F: doc/guides/nics/netvsc.rst
>>  F: doc/guides/nics/features/netvsc.ini
> 
> Acked-by: Haiyang Zhang 
> 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] maintainers: update for vmbus

[Ferruh Yigit]

On 7/2/2020 1:04 AM, Stephen Hemminger wrote:
> On Thu, 25 Jun 2020 16:52:16 -0700
> Long Li  wrote:
> 
>> From: Long Li 
>>
>> Add Long Li as additional maintainer.
>>
>> Signed-off-by: Long Li 
> 
> Acked-by: Stephen Hemminger 
> 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v5 1/4] build: disable vhost NUMA for arm32

[Juraj Linkeš]

config/defconfig_arm-armv7a-linux-gcc specifies that
RTE_LIBRTE_VHOST_NUMA is not supported on arm32, so disable it in meson.

Signed-off-by: Juraj Linkeš 
---
 lib/librte_vhost/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/meson.build b/lib/librte_vhost/meson.build
index 882a0eaf4..73dc05f86 100644
--- a/lib/librte_vhost/meson.build
+++ b/lib/librte_vhost/meson.build
@@ -5,7 +5,7 @@ if not is_linux
build = false
reason = 'only supported on linux'
 endif
-if has_libnuma == 1
+if has_libnuma == 1 and not dpdk_conf.has('RTE_ARCH_ARMv7')
dpdk_conf.set10('RTE_LIBRTE_VHOST_NUMA', true)
 endif
 if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
-- 
2.20.1

[dpdk-dev] [PATCH v5 2/4] build: add arm32 meson build flags

[Juraj Linkeš]

Base the flags on config/defconfig_arm-armv7a-linuxapp-gcc.
Omit driver flags which can be built on arm32.

Signed-off-by: Juraj Linkeš 
---
 config/arm/meson.build | 135 ++---
 1 file changed, 74 insertions(+), 61 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 8728051d5..b02fc95d9 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -34,6 +34,11 @@ flags_generic = [
['RTE_MAX_LCORE', 256],
['RTE_USE_C11_MEM_MODEL', true],
['RTE_CACHE_LINE_SIZE', 128]]
+flags_generic_arm32 = [
+   ['RTE_MACHINE', '"armv7a"'],
+   ['RTE_MAX_LCORE', 128],
+   ['RTE_USE_C11_MEM_MODEL', false],
+   ['RTE_CACHE_LINE_SIZE', 64]]
 flags_arm = [
['RTE_MACHINE', '"armv8a"'],
['RTE_MAX_LCORE', 16],
@@ -63,6 +68,10 @@ flags_armada = [
['RTE_MAX_LCORE', 16]]
 
 flags_default_extra = []
+flags_default_arm32_extra = [
+['RTE_ARCH_ARM_NEON_MEMCPY', false],
+['RTE_ARCH_STRICT_ALIGN', true],
+['RTE_EAL_NUMA_AWARE_HUGEPAGES', false]]
 flags_n1sdp_extra = [
['RTE_MACHINE', '"n1sdp"'],
['RTE_MAX_NUMA_NODES', 1],
@@ -99,6 +108,9 @@ machine_args_generic = [
['0xd0b', ['-mcpu=cortex-a76']],
['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'], 
flags_n1sdp_extra]]
 
+machine_args_generic_arm32 = [
+['default_arm32', ['-march=armv7-a', '-mtune=cortex-a9', 
'-mfpu=neon'], flags_default_arm32_extra]]
+
 machine_args_cavium = [
['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
['native', ['-march=native']],
@@ -114,6 +126,7 @@ machine_args_emag = [
 
 ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
 impl_generic = ['Generic armv8', flags_generic, machine_args_generic]
+impl_generic_arm32 = ['Generic armv7', flags_generic_arm32, 
machine_args_generic_arm32]
 impl_0x41 = ['Arm', flags_arm, machine_args_generic]
 impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
 impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
@@ -136,74 +149,74 @@ if not dpdk_conf.get('RTE_ARCH_64')
dpdk_conf.set('RTE_ARCH_ARMv7', 1)
# the minimum architecture supported, armv7-a, needs the following,
# mk/machine/armv7a/rte.vars.mk sets it too
-   machine_args += '-mfpu=neon'
 else
dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
dpdk_conf.set('RTE_ARCH_ARM64', 1)
+endif
 
-   machine = []
-   cmd_generic = ['generic', '', '', 'default', '']
-   cmd_output = cmd_generic # Set generic by default
-   machine_args = [] # Clear previous machine args
-   if arm_force_default_march and not meson.is_cross_build()
+machine = []
+machine_args = [] # Clear previous machine args
+cmd_generic = ['generic', '', '', 'default', '']
+cmd_output = cmd_generic # Set generic by default
+if arm_force_default_march and not meson.is_cross_build()
+   machine = impl_generic
+   impl_pn = 'default'
+elif not meson.is_cross_build()
+   # The script returns ['Implementer', 'Variant', 'Architecture',
+   # 'Primary Part number', 'Revision']
+   detect_vendor = find_program(join_paths(
+   meson.current_source_dir(), 'armv8_machine.py'))
+   cmd = run_command(detect_vendor.path())
+   if cmd.returncode() == 0
+   cmd_output = cmd.stdout().to_lower().strip().split(' ')
+   endif
+   # Set to generic if variable is not found
+   machine = get_variable('impl_' + cmd_output[0], ['generic'])
+   if machine[0] == 'generic'
machine = impl_generic
-   impl_pn = 'default'
-   elif not meson.is_cross_build()
-   # The script returns ['Implementer', 'Variant', 'Architecture',
-   # 'Primary Part number', 'Revision']
-   detect_vendor = find_program(join_paths(
-   meson.current_source_dir(), 'armv8_machine.py'))
-   cmd = run_command(detect_vendor.path())
-   if cmd.returncode() == 0
-   cmd_output = cmd.stdout().to_lower().strip().split(' ')
-   endif
-   # Set to generic if variable is not found
-   machine = get_variable('impl_' + cmd_output[0], ['generic'])
-   if machine[0] == 'generic'
-   machine = impl_generic
-   cmd_output = cmd_generic
-   endif
-   impl_pn = cmd_output[3]
-   if arm_force_native_march == true
-   impl_pn = 'native'
-   endif
-   else
-   impl_id = meson.get_cross_property('implementor_id', 'generic')
-   impl_pn = meson.get_cross_property('implementor_pn', 'default')
-   machine = get_variable('impl_' + impl_id)
+   cmd_output = cmd_generic
endif
-
-   # Apply Common Defaults. These settings may be overwritten by machine

[dpdk-dev] [PATCH v5 0/4] aarch64 -> arm32 cross compilation support

[Juraj Linkeš]

Add support for arm32 cross build in meson
and add aarch64 -> arm32 cross build to Travis.

The patchset makes use of existing options in
config/defconfig_arm-armv7a-linux-gcc a ports those to the meson build
system.

The aarch64 -> arm32 build currently fails in l3fwd example and ixgbe
driver without series 9609:
http://patches.dpdk.org/project/dpdk/list/?series=9609

Tested here after rebasing on top of series 9609:
https://travis-ci.com/github/jlinkes/dpdk/builds/162648822

v4:
Removed disabled drivers which actually build on arm32.
Also tested the patchset with series 9609 which fixes underlying
failures.

v5:
Changed the condition for running test-null.sh in ci.
Re-uploaded after underlying fixes have been committed.

Juraj Linkeš (4):
  build: disable vhost NUMA for arm32
  build: add arm32 meson build flags
  build: add arm32 meson cross file
  ci: add aarch64 -> arm32 cross compiling jobs

 .ci/linux-build.sh  |   7 +-
 .travis.yml |  19 +
 config/arm/arm_armv7a_linux_gcc |  17 
 config/arm/meson.build  | 135 +---
 lib/librte_vhost/meson.build|   2 +-
 5 files changed, 117 insertions(+), 63 deletions(-)
 create mode 100644 config/arm/arm_armv7a_linux_gcc

-- 
2.20.1

[dpdk-dev] [PATCH v5 4/4] ci: add aarch64 -> arm32 cross compiling jobs

[Juraj Linkeš]

Add two jobs (static and shared libs), both building on aarch64 and
producing 32 bit arm binaries. Do not run tests in these jobs.

Signed-off-by: Juraj Linkeš 
---
 .ci/linux-build.sh |  7 ++-
 .travis.yml| 19 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index d079801d7..e5407fb37 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -33,6 +33,11 @@ if [ "$AARCH64" = "1" ]; then
 OPTS="$OPTS --cross-file config/arm/arm64_armv8_linux_gcc"
 fi
 
+if [ "$ARM" = "1" ]; then
+# convert the arch specifier
+OPTS="$OPTS --cross-file config/arm/arm_armv7a_linux_gcc"
+fi
+
 if [ "$BUILD_DOCS" = "1" ]; then
 OPTS="$OPTS -Denable_docs=true"
 fi
@@ -53,7 +58,7 @@ OPTS="$OPTS --buildtype=debugoptimized"
 meson build --werror $OPTS
 ninja -C build
 
-if [ "$AARCH64" != "1" ]; then
+if [ "$(uname -m)" = "x86_64" ]; then
 devtools/test-null.sh
 fi
 
diff --git a/.travis.yml b/.travis.yml
index 14f812423..baba29539 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -21,6 +21,10 @@ _aarch64_packages: &aarch64_packages
   - *required_packages
   - [gcc-aarch64-linux-gnu, libc6-dev-arm64-cross, 
pkg-config-aarch64-linux-gnu]
 
+_arm_packages: &arm_packages
+  - *required_packages
+  - [gcc-arm-linux-gnueabihf, libc6-dev-armhf-cross, 
pkg-config-arm-linux-gnueabihf]
+
 _libabigail_build_packages: &libabigail_build_packages
   - [autoconf, automake, libtool, pkg-config, libxml2-dev, libdw-dev]
 
@@ -124,6 +128,21 @@ jobs:
 packages:
   - *required_packages
   - *libabigail_build_packages
+  # aarch64 cross-compiling arm jobs
+  - env: DEF_LIB="shared" ARM=1
+arch: arm64
+compiler: gcc
+addons:
+  apt:
+packages:
+  - *arm_packages
+  - env: DEF_LIB="static" ARM=1
+arch: arm64
+compiler: gcc
+addons:
+  apt:
+packages:
+  - *arm_packages
   # aarch64 clang jobs
   - env: DEF_LIB="static"
 arch: arm64
-- 
2.20.1

[dpdk-dev] [PATCH v5 3/4] build: add arm32 meson cross file

[Juraj Linkeš]

Use arm-linux-gnueabihf- toolset which comes with standard packages on
most used systems, such as Ubuntu and CentOS.

Signed-off-by: Juraj Linkeš 
---
 config/arm/arm_armv7a_linux_gcc | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 config/arm/arm_armv7a_linux_gcc

diff --git a/config/arm/arm_armv7a_linux_gcc b/config/arm/arm_armv7a_linux_gcc
new file mode 100644
index 0..39566d2ea
--- /dev/null
+++ b/config/arm/arm_armv7a_linux_gcc
@@ -0,0 +1,17 @@
+[binaries]
+c = 'arm-linux-gnueabihf-gcc'
+cpp = 'arm-linux-gnueabihf-cpp'
+ar = 'arm-linux-gnueabihf-gcc-ar'
+strip = 'arm-linux-gnueabihf-strip'
+pkgconfig = 'arm-linux-gnueabihf-pkg-config'
+pcap-config = ''
+
+[host_machine]
+system = 'linux'
+cpu_family = 'arm'
+cpu = 'armv7-a'
+endian = 'little'
+
+[properties]
+implementor_id = 'generic_arm32'
+implementor_pn = 'default_arm32'
-- 
2.20.1

[dpdk-dev] [PATCH v3] app/flow-perf: fix condition of hairpin queues setup

[Wisam Jaddo]

The hairpin queue is the one that start from normal rxq,
and will be less than nr_queues where nr_queues is the
sum of normal and hairpin

Fixes: bf3688f1e816 ("app/flow-perf: add insertion rate calculation")
Cc: wis...@mellanox.com

Signed-off-by: Wisam Jaddo 
Reviewed-by: Asaf Penso 

---
v3:
* Add variable documentation variable declaration.

v2:
* Add documentation of hairpin peering and allocating logic.
* Add documentation for some variables.
---
 app/test-flow-perf/main.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
index e155e49c37..2200767e0d 100644
--- a/app/test-flow-perf/main.c
+++ b/app/test-flow-perf/main.c
@@ -59,6 +59,13 @@ static struct rte_mempool *mbuf_mp;
 static uint32_t nb_lcores;
 static uint32_t flows_count;
 static uint32_t iterations_number;
+/*
+ * hairpinq: represent the number of hairpin queues needed
+ * to be initialized.
+ *
+ * In 0 case means no hairpin queues needed which is the
+ * default.
+ */
 static uint32_t hairpinq;
 static uint32_t nb_lcores;
 
@@ -929,6 +936,7 @@ init_port(void)
 {
int ret;
uint16_t std_queue;
+   /* hairpin_q represent hairpin queue id to be initialized. */
uint16_t hairpin_q;
uint16_t port_id;
uint16_t nr_ports;
@@ -1012,8 +1020,19 @@ init_port(void)
rte_strerror(-ret), port_id);
 
if (hairpinq != 0) {
+   /*
+* Each hairpin queue setup need a hairpin configuration
+* object, which determine the TX path for hairpin.
+*
+* The peering here represent the TX side, which mean 
the
+* peer.port represent TX port, and peer.queue represent
+* tx_queue.
+*
+* So if RXQ=4 and TXQ=4, and first hairpin_q is 4 after
+* [0, 1, 2, 3], then tx_queue is TXQ+i which is 4 as 
well.
+*/
for (hairpin_q = RXQ_NUM, std_queue = 0;
-   std_queue < nr_queues;
+   hairpin_q < nr_queues;
hairpin_q++, std_queue++) {
hairpin_conf.peers[0].port = port_id;
hairpin_conf.peers[0].queue =
@@ -1028,7 +1047,7 @@ init_port(void)
}
 
for (hairpin_q = TXQ_NUM, std_queue = 0;
-   std_queue < nr_queues;
+   hairpin_q < nr_queues;
hairpin_q++, std_queue++) {
hairpin_conf.peers[0].port = port_id;
hairpin_conf.peers[0].queue =
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 1/7] ethdev: introduce sample action for rte flow

[Jerin Jacob]

On Sun, Jul 5, 2020 at 10:22 AM Matan Azrad  wrote:
>
>
>
> From: Jerin Jacob:
> > On Sun, Jul 5, 2020 at 12:56 AM Matan Azrad  wrote:
> > >
> > > Hi all
> > >
> > > From: Jerin Jacob:
> > > > On Fri, Jul 3, 2020 at 8:57 PM Thomas Monjalon 
> > > > wrote:
> > > > >
> > > > > 03/07/2020 17:08, Jerin Jacob:
> > > > > > On Fri, Jul 3, 2020 at 8:25 PM Matan Azrad 
> > > > wrote:
> > > > > > > From: Jerin Jacob:
> > > > > > > > When adding overlapping API(rte_eth_mirror_rule_set()) in
> > > > > > > > the same library(ethdev).
> > > > > > > > Please depreciate the old API.
> > > > > > > > We should not have two separate paths for the same function
> > > > > > > > in the same ethdev library. It is pain for app and driver 
> > > > > > > > developers.
> > > > > > >
> > > > > > > What are about all the other rte_flow parallel configuration
> > > > > > > APIs in
> > > > ethdev:
> > > > > > >  promiscuous_enable;
> > > > > > > promiscuous_disable;
> > > > > > > allmulticast_enable;
> > > > > > > allmulticast_disable;
> > > > > > > mac_addr_remove;
> > > > > > > mac_addr_add;
> > > > > > > mac_addr_set;
> > > > > > > set_mc_addr_list;
> > > > > > > vlan_filter_set;
> > > > > > > vlan_tpid_set;
> > > > > > > vlan_strip_queue_set;
> > > > > > > vlan_offload_set;
> > > > > > > vlan_pvid_set;
> > > > > > > udp_tunnel_port_add;
> > > > > > > udp_tunnel_port_del;
> > > > > > > ...
> > > > > > >
> > > > > > > These APIs can be replaced easily by rte_flow API.
> > > > > > > Do you think we need to deprecate all?
> > > > > >
> > > > > > I think, basic stuff like below can have separate API.
> > > > > > 1)  promiscuous_enable;
> > > > > > 2) promiscuous_disable;
> > > > > > 3) allmulticast_enable;
> > > > > > 4) allmulticast_disable;
> > > > > > 5) mac_addr_remove;
> > > > > > 6) mac_addr_add;
> > > > > > 7) mac_addr_set;
> > > > > > 8) set_mc_addr_list;
> > > > >
> > > > > "Basic" is not a precise definition :)
> > > >
> > > > Yep.
> > > >
> > > > > I would say port-level configuration should remain out of rte_flow
> > > > > API.
> > >
> > > Thomas, Can you explain what is port-level?
> > > Everything in rte_flow is per port...
> > >
> > > Also, can you give reasons for your claim?
> > >
> > > > +1.
> > > > In addition that, I would say anything needs to configured at "per-flow"
> > > > granularity use rte_flow.
> > >
> > > Jerin, What do you mean "per-flow" ?
> >
> > In Terms of  NIC HW features, Typical HW will have
> > a) Basic "port" level configuration like
> > - enable/disable promiscuous
>
> What is "port level", everything in rte_flow is also per port...
>
> > b) Advance HW's will have CAM based flow filtering. IMO, CAM related stuff
> > should go to rte_flow.
>
> It is HW internal, I don't think all HWs use the same logic here.
> Since rte_flow is generic for all filtering methods, It is good candidate API 
> for all HWs.
>
> > This is to enable,  The very basic PMD(without advanced features) should
> > work with port level basic APIs(i.e without rte_flow)
>
> What is "basic"? Do you mean simple match and simple action?
> As I said, Also rte_flow is port level API - so "port level" is not good term 
> here.
>
> As you said " When adding overlapping API(rte_eth_mirror_rule_set()) in the 
> same library(ethdev). Please depreciate the old API."
>
> Different APIs to do the same thing is not good, especially in packet 
> filtering:
> What should we do if we have conflicts?
> For example: legacy filtering APIs say to receive packet A and rte_flow says 
> to drop it.
>
> Don't you think it complicates more the user API understanding, also the PMD 
> implementations?
>
>
> > I have seen promiscuous, mac address handling is part of basic NIC HW(i.e
> > NICs without advanced CAM filters).
> > That's my reasoning for the split.
>
> As I said, the nic HW specific implementation should not affect the API.
> I don't think we need to split API and to complicate the user because of it.
>
> IMO, It is better to have one generic API for packet filtering:
> It is clearer, simpler, generic and classic.
> The user and PMD need to understand only one filtering API and that’s it (no 
> need to combine between multiple filtering APIs).
>
> I know this is big change but we can do it in modular way.
> It reminds me the big change that was done in Rx\Tx offload configurations:
> So, when offload became more popular we modularly forced users to replace the 
> offload API.
> Also here, flow filtering becomes popular so maybe this is the 
> time(20.08-20.11) to force changes in the old APIs.
>
> > > Everything in traffic filtering\actions is per flow, for example:
> > > Promiscuous: flow create 0 ingress pattern eth / end actions queue
> > > index 0 / end
> >
> > IMO, it is not an accurate representation of promiscuous enable. It needs to
> > send the traffic to all queues and patterns is not just eth.
>
> Yes, if legacy RSS is configured we need to replace the above queue action by 
> rss action as I wrote before.(did you read it just below?)
>
>

Re: [dpdk-dev] [PATCH v2 3/3] eal/windows: librte_net build on Windows

[Fady Bader]

Hi Microsoft team,

Can you please check Thomas's comment below regarding header licensing (of ip.h 
and in.h) and verify that it can be used in the dpdk project or do I need to 
change anything?

Headers are taken from: 
Ip.h: https://code.woboq.org/qt5/include/netinet/ip.h.html
In.h: https://code.woboq.org/qt5/include/netinet/in.h.html

> -Original Message-
> From: Thomas Monjalon 
> Sent: Tuesday, June 16, 2020 12:04 PM
> To: Fady Bader 
> Cc: dev@dpdk.org; Tasnim Bashar ; Tal Shnaiderman
> ; Yohad Tor ;
> dmitry.kozl...@gmail.com; harini.ramakrish...@microsoft.com;
> ocard...@microsoft.com; ranjit.me...@intel.com; olivier.m...@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2 3/3] eal/windows: librte_net build on
> Windows
> 
> 10/06/2020 14:00, Fady Bader:
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2020 Mellanox Technologies, Ltd  */
> 
> Isn't it a different copyright?
> Where does it come from?
> 
> > +
> > +#ifndef _IN_H_
> > +#define _IN_H_
> > +
> > +#define IPPROTO_IP 0
> > +#define IPPROTO_HOPOPTS 0
> > +#defineIPPROTO_IPV4 4 /* IPv4 encapsulation */
> > +#defineIPPROTO_IPIP IPPROTO_IPV4  /* for compatibility */
> > +#define IPPROTO_TCP 6
> > +#define IPPROTO_UDP 17
> > +#defineIPPROTO_IPV6 41/* IP6 header */
> > +#defineIPPROTO_ROUTING 43 /* IP6 routing header */
> > +#defineIPPROTO_FRAGMENT 44/* IP6 fragmentation header */
> > +#defineIPPROTO_GRE 47 /* General Routing Encap. */
> > +#defineIPPROTO_ESP 50 /* IP6 Encap Sec. Payload */
> > +#defineIPPROTO_AH 51  /* IP6 Auth Header */
> > +#define IPPROTO_NONE 59/* IPv6 no next header */
> > +#defineIPPROTO_DSTOPTS 60 /* IP6 destination option */
> > +#define IPPROTO_SCTP 132   /* Stream Control Transmission Protocol 
> > */
> 
> There are some tabs instead of space. Please double check.
> 

Re: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get status message

[Adrian Moreno]

On 7/6/20 5:22 AM, Xia, Chenbo wrote:
> Hi Adrian,
> 
>> -Original Message-
>> From: dev  On Behalf Of Adrian Moreno
>> Sent: Thursday, July 2, 2020 4:33 PM
>> To: dev@dpdk.org; Ye, Xiaolong ;
>> shah...@mellanox.com; ma...@mellanox.com; maxime.coque...@redhat.com;
>> Wang, Xiao W ; viachesl...@mellanox.com
>> Cc: jasow...@redhat.com; l...@redhat.com; Adrian Moreno
>> 
>> Subject: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get status
>> message
>>
>> This patch adds support to the new Virtio device get status Vhost-user 
>> message.
>>
>> The driver can send this new message to read the device status.
>>
>> One of the uses of this message is to ensure the feature negotiation has
>> succeded.  According to the virtio spec, after completing the feature 
>> negotiation,
>> the driver sets the FEATURE_OK status bit and re-reads it to ensure the 
>> device
>> has accepted the features.
>>
>> This patch also clears the FEATURE_OK status bit if the feature negotiation 
>> has
>> failed to let the driver know about his failure.
>>
>> Signed-off-by: Adrian Moreno 
>> ---
>>  lib/librte_vhost/vhost.h  |  2 ++
>>  lib/librte_vhost/vhost_user.c | 32 
>> lib/librte_vhost/vhost_user.h |  1 +
>>  3 files changed, 35 insertions(+)
>>
>> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index
>> 25d31c71b..e743821cc 100644
>> --- a/lib/librte_vhost/vhost.h
>> +++ b/lib/librte_vhost/vhost.h
>> @@ -32,6 +32,8 @@
>>  #define VIRTIO_DEV_BUILTIN_VIRTIO_NET 4
>>  /* Used to indicate that the device has its own data path and configured */
>> #define VIRTIO_DEV_VDPA_CONFIGURED 8
>> +/* Used to indicate that the feature negotiation failed */ #define
>> +VIRTIO_DEV_FEATURES_FAILED 16
>>
>>  /* Backend value set by guest. */
>>  #define VIRTIO_DEV_STOPPED -1
>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c 
>> index
>> 8d3d13913..00da7bf18 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -88,6 +88,7 @@ static const char *vhost_message_str[VHOST_USER_MAX]
>> = {
>>  [VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD",
>>  [VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD",
>>  [VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS",
>> +[VHOST_USER_GET_STATUS] = "VHOST_USER_GET_STATUS",
>>  };
>>
>>  static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg); @@ -339,6
>> +340,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct
>> VhostUserMsg *msg,
>>  VHOST_LOG_CONFIG(ERR,
>>  "(%d) received invalid negotiated features.\n",
>>  dev->vid);
>> +dev->flags |= VIRTIO_DEV_FEATURES_FAILED;
>> +dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
>> +
>>  return RTE_VHOST_MSG_RESULT_ERR;
>>  }
>>
>> @@ -402,6 +406,7 @@ vhost_user_set_features(struct virtio_net **pdev, struct
>> VhostUserMsg *msg,
>>  if (vdpa_dev)
>>  vdpa_dev->ops->set_features(dev->vid);
>>
>> +dev->flags &= ~VIRTIO_DEV_FEATURES_FAILED;
>>  return RTE_VHOST_MSG_RESULT_OK;
>>  }
>>
>> @@ -2458,6 +2463,22 @@ vhost_user_postcopy_end(struct virtio_net **pdev,
>> struct VhostUserMsg *msg,
>>  return RTE_VHOST_MSG_RESULT_REPLY;
>>  }
>>
>> +static int
>> +vhost_user_get_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
>> +  int main_fd __rte_unused)
>> +{
>> +struct virtio_net *dev = *pdev;
>> +
>> +if (validate_msg_fds(msg, 0) != 0)
>> +return RTE_VHOST_MSG_RESULT_ERR;
>> +
>> +msg->payload.u64 = dev->status;
>> +msg->size = sizeof(msg->payload.u64);
>> +msg->fd_num = 0;
>> +
>> +return RTE_VHOST_MSG_RESULT_OK;
>> +}
>> +
>>  static int
>>  vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
>>  int main_fd __rte_unused)
>> @@ -2476,6 +2497,16 @@ vhost_user_set_status(struct virtio_net **pdev,
>> struct VhostUserMsg *msg,
>>
>>  dev->status = msg->payload.u64;
>>
>> +if ((dev->status & VIRTIO_DEVICE_STATUS_FEATURES_OK) &&
>> +(dev->flags & VIRTIO_DEV_FEATURES_FAILED)) {
>> +VHOST_LOG_CONFIG(ERR, "FEATURES_OK bit is set but feature
>> negotiation failed\n");
>> +/*
>> + * Clear the bit to let the driver know about the feature
>> + * negotiation failure
>> + */
>> +dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
>> +}
>> +
> 
> There's a coding style issue because of above '}' alignment. Could you fix 
> this?
> 
> Another thing I'm not sure: if above condition happens, should it be treated
> as err? If set status is with replay-ack (this will happen, right?), would 
> QEMU
> like to know this status is not set? As QEMU should know it during 
> SET_FEATURES,
> I'm not sure whether this will also need NACK when reply-ack enabled. What's
> your opinion?
> 

My interpretation was t

Re: [dpdk-dev] [PATCH 2/2] devtools: fix forbidden token

[Thomas Monjalon]

06/07/2020 10:00, David Marchand:
> An expression with a space is split by the awk script resulting in
> false positive for any patch matching any of the two part of the
> expression.
> Fix this by using [[:space:]].
> 
> Fixes: 43e73483a4b8 ("devtools: forbid variable declaration inside for")
> 
> Signed-off-by: David Marchand 

Applied, thanks

Re: [dpdk-dev] [PATCH v5 1/4] build: disable vhost NUMA for arm32

[David Marchand]

On Mon, Jul 6, 2020 at 10:29 AM Juraj Linkeš  wrote:
>
> config/defconfig_arm-armv7a-linux-gcc specifies that
> RTE_LIBRTE_VHOST_NUMA is not supported on arm32, so disable it in meson.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  lib/librte_vhost/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/meson.build b/lib/librte_vhost/meson.build
> index 882a0eaf4..73dc05f86 100644
> --- a/lib/librte_vhost/meson.build
> +++ b/lib/librte_vhost/meson.build
> @@ -5,7 +5,7 @@ if not is_linux
> build = false
> reason = 'only supported on linux'
>  endif
> -if has_libnuma == 1
> +if has_libnuma == 1 and not dpdk_conf.has('RTE_ARCH_ARMv7')

Is the libnuma detection invalid in the first place?


> dpdk_conf.set10('RTE_LIBRTE_VHOST_NUMA', true)
>  endif
>  if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
> --
> 2.20.1
>


-- 
David Marchand

Re: [dpdk-dev] [PATCH] add flow shared action API

[Jerin Jacob]

On Sun, Jul 5, 2020 at 3:56 PM Ori Kam  wrote:
>
> Hi Jerin,
> PSB,
>
> Thanks,
> Ori
>
> > -Original Message-
> > From: Jerin Jacob 
> > Sent: Saturday, July 4, 2020 3:33 PM
> > dpdk-dev 
> > Subject: Re: [dpdk-dev] [PATCH] add flow shared action API
> >
> > On Sat, Jul 4, 2020 at 3:40 PM Andrey Vesnovaty
> >  wrote:
> > >
> > >
> > > Thanks,
> > >
> > > Andrey Vesnovaty
> > > (+972)526775512 | Skype: andrey775512
> > >
>
>
> [..Nip ..]
>
> > > I need to mention the locking issue once again.
> > > If there is a need to maintain "shared session" in the generic rte_flow 
> > > layer
> > all
> > > calls to flow_create() with shared action & all delete need to take
> > sharedsession
> > > management locks at least for verification. Lock partitioning is also bit
> > problematic
> > > since one flow may have more than one shared action.
> >
> > Then, I think better approach would be to introduce
> > rte_flow_action_update() public
> > API which can either take "const struct rte_flow_action []" OR shared
> > context ID, to cater to
> > both cases or something on similar lines. This would allow HW's
> > without have  the shared context ID
> > to use the action update.
>
> Can you please explain your idea?

I see two types of HW schemes supporting action updates without going
through call `rte_flow_destroy()` and call `rte_flow_create()`
- The shared HW action context feature
- The HW has "pattern" and "action" mapped to different HW objects and
action can be updated any time.
Other than above-mentioned RSS use case, another use case would be to
a) create rte_flow and set the action as DROP (Kind of reserving the HW object)
b) Update the action only when the rest of the requirements ready.

Any API schematic that supports both notions of HW is fine with me.


> As I can see if we use the flow_action array it may result in bugs.
> For example, the application created two flows with the same RSS (not using 
> the context)
> Then he wants to change one flow to use different RSS, but the result will 
> that both flows
> will be changed.

Sorry. I don't quite follow this.

> Also this will enforce the PMD to keep track on all flows which will have 
> memory penalty for
> some PMDs.

Re: [dpdk-dev] [PATCH v4 1/2] vhost: introduce async enqueue registration API

[Fu, Patrick]

Hi,

> -Original Message-
> From: Liu, Yong 
> Sent: Monday, July 6, 2020 11:06 AM
> To: Fu, Patrick ; dev@dpdk.org;
> maxime.coque...@redhat.com; Xia, Chenbo ; Wang,
> Zhihong 
> Cc: Fu, Patrick ; Wang, Yinan
> ; Jiang, Cheng1 ; Liang,
> Cunming 
> Subject: RE: [dpdk-dev] [PATCH v4 1/2] vhost: introduce async enqueue
> registration API
> 
> Hi Patrick,
> Few comments are inline,  others are fine to me.
> 
> Regards,
> Marvin
> 
> > diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index
> > 0d822d6..58ee3ef 100644
> > --- a/lib/librte_vhost/vhost.c
> > +++ b/lib/librte_vhost/vhost.c
> > @@ -332,8 +332,13 @@
> >  {
> > if (vq_is_packed(dev))
> > rte_free(vq->shadow_used_packed);
> > -   else
> > +   else {
> > rte_free(vq->shadow_used_split);
> > +   if (vq->async_pkts_pending)
> > +   rte_free(vq->async_pkts_pending);
> > +   if (vq->async_pending_info)
> > +   rte_free(vq->async_pending_info);
> 
> Missed pointer set and feature set to 0.
> 
This memory free statement is part of vq free operation, which should be safe 
to leave the vq member pointer just as is

> > +   if (vq->async_pkts_pending) {
> > +   rte_free(vq->async_pkts_pending);
> > +   vq->async_pkts_pending = 0;
> > +   }
> > +
> > +   if (vq->async_pending_info) {
> > +   rte_free(vq->async_pending_info);
> > +   vq->async_pending_info = 0;
> > +   }
> > +
> 
> Please unify the async pending pointer check and free logic and pointer
> should be set to NULL.
> 
Will change it in the next patch version

Thanks,

Patrick

Re: [dpdk-dev] [PATCH v3 3/5] app/testpmd: re-implement commands by using private API

[Yang, Qiming]

> -Original Message-
> From: Di, ChenxuX 
> Sent: Wednesday, July 1, 2020 16:25
> To: dev@dpdk.org
> Cc: Yang, Qiming ; Guo, Jia ; Di,
> ChenxuX 
> Subject: [PATCH v3 3/5] app/testpmd: re-implement commands by using
> private API
> 
> The legacy filter API will be superseded. This patch use private api to change
> the implementation of commands global_config  gre-key-len
>  and show port fdir 
> 
> Signed-off-by: Chenxu Di 
> ---
>  app/test-pmd/cmdline.c |  6 
>  app/test-pmd/config.c  | 65 +++---
> 
>  2 files changed, 61 insertions(+), 10 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 996a49876..466c54aa9 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -9287,6 +9287,12 @@ cmd_global_config_parsed(void *parsed_result,
>   conf.cfg.gre_key_len = res->len;
>   ret = rte_eth_dev_filter_ctrl(res->port_id, RTE_ETH_FILTER_NONE,
> RTE_ETH_FILTER_SET, &conf);
> +
> +#ifdef RTE_LIBRTE_I40E_PMD
> + if (ret == -ENOTSUP)
> + ret = rte_pmd_i40e_set_gre_key_len(res->port_id, res-
> >len); #endif
> +
>   if (ret != 0)
>   printf("Global config error\n");
>  }
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> a7112c998..ed341c715 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -3727,6 +3727,50 @@ print_fdir_flow_type(uint32_t flow_types_mask)
>   printf("\n");
>  }
> 
> +static int
> +get_fdir_info(portid_t port_id, struct rte_eth_fdir_info *fdir_info) {
> + int ret;
> +
> + ret = rte_eth_dev_filter_supported(port_id,
> +RTE_ETH_FILTER_FDIR);
> + if (!ret)
> + rte_eth_dev_filter_ctrl(port_id, RTE_ETH_FILTER_FDIR,
> + RTE_ETH_FILTER_INFO, fdir_info);
> +
> +#ifdef RTE_LIBRTE_I40E_PMD
> + if (ret == -ENOTSUP)
> + ret = rte_pmd_i40e_get_fdir_info(port_id, fdir_info); #endif

No return value check.

> #ifdef
> +RTE_LIBRTE_IXGBE_PMD
> + if (ret == -ENOTSUP)
> + ret = rte_pmd_ixgbe_get_fdir_info(port_id, fdir_info);
> #endif

We'd better use
If(ret == -ENOTSUP)
#ifdef RTE_LIBRTE_I40E_PMD
...
#endif
#ifdef RTE_LIBRTE_IXGBE_PMD
...
#endif

Same issue in patch 5

> + return ret;
> +}
> +
> +static int
> +get_fdir_stat(portid_t port_id, struct rte_eth_fdir_stats *fdir_stat) {
> + int ret;
> +
> + ret = rte_eth_dev_filter_supported(port_id,
> +RTE_ETH_FILTER_FDIR);
> + if (!ret)
> + rte_eth_dev_filter_ctrl(port_id, RTE_ETH_FILTER_FDIR,
> + RTE_ETH_FILTER_STATS, fdir_stat);
> +
> +#ifdef RTE_LIBRTE_I40E_PMD
> + if (ret == -ENOTSUP)
> + ret = rte_pmd_i40e_get_fdir_stats(port_id, fdir_stat); #endif
> #ifdef
> +RTE_LIBRTE_IXGBE_PMD
> + if (ret == -ENOTSUP)
> + ret = rte_pmd_ixgbe_get_fdir_stats(port_id, fdir_stat);
> #endif

Same as above

> + return ret;
> +}
> +
>  void
>  fdir_get_infos(portid_t port_id)
>  {
> @@ -3738,19 +3782,20 @@ fdir_get_infos(portid_t port_id)
> 
>   if (port_id_is_invalid(port_id, ENABLED_WARN))
>   return;
> - ret = rte_eth_dev_filter_supported(port_id, RTE_ETH_FILTER_FDIR);
> - if (ret < 0) {
> - printf("\n FDIR is not supported on port %-2d\n",
> - port_id);
> - return;
> - }
> 
>   memset(&fdir_info, 0, sizeof(fdir_info));
> - rte_eth_dev_filter_ctrl(port_id, RTE_ETH_FILTER_FDIR,
> -RTE_ETH_FILTER_INFO, &fdir_info);
> + ret = get_fdir_info(port_id, &fdir_info);
> + if (ret) {
> + printf("Get fdir infos error: (%s)\n", strerror(-ret));
> + return;
> + }
>   memset(&fdir_stat, 0, sizeof(fdir_stat));
> - rte_eth_dev_filter_ctrl(port_id, RTE_ETH_FILTER_FDIR,
> -RTE_ETH_FILTER_STATS, &fdir_stat);
> + ret = get_fdir_stat(port_id, &fdir_stat);

Why you add new function here, it's no need to do that

> + if (ret) {
> + printf("Get fdir status error: (%s)\n", strerror(-ret));
> + return;
> + }
> +
>   printf("\n  %s FDIR infos for port %-2d %s\n",
>  fdir_stats_border, port_id, fdir_stats_border);
>   printf("  MODE: ");
> --
> 2.17.1

Re: [dpdk-dev] [PATCH v5 2/4] build: add arm32 meson build flags

[Jerin Jacob]

On Mon, Jul 6, 2020 at 1:59 PM Juraj Linkeš  wrote:
>
> Base the flags on config/defconfig_arm-armv7a-linuxapp-gcc.
> Omit driver flags which can be built on arm32.
>
> Signed-off-by: Juraj Linkeš 

Hi Juraj,

Not strictly specific to this patch. Just to understand, How armv7
support has been used?
- Is it for Running arm32 program on arm64 machines?
- Is it for Native DPDK support from arm32. If so, What kind of PMD
supports native arm32 DPDK?

Or some other use case?

I would like to understand the arm32 use case, so we can review it at
that angle.

> ---
>  config/arm/meson.build | 135 ++---
>  1 file changed, 74 insertions(+), 61 deletions(-)
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 8728051d5..b02fc95d9 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -34,6 +34,11 @@ flags_generic = [
> ['RTE_MAX_LCORE', 256],
> ['RTE_USE_C11_MEM_MODEL', true],
> ['RTE_CACHE_LINE_SIZE', 128]]
> +flags_generic_arm32 = [
> +   ['RTE_MACHINE', '"armv7a"'],
> +   ['RTE_MAX_LCORE', 128],
> +   ['RTE_USE_C11_MEM_MODEL', false],
> +   ['RTE_CACHE_LINE_SIZE', 64]]
>  flags_arm = [
> ['RTE_MACHINE', '"armv8a"'],
> ['RTE_MAX_LCORE', 16],
> @@ -63,6 +68,10 @@ flags_armada = [
> ['RTE_MAX_LCORE', 16]]
>
>  flags_default_extra = []
> +flags_default_arm32_extra = [
> +['RTE_ARCH_ARM_NEON_MEMCPY', false],
> +['RTE_ARCH_STRICT_ALIGN', true],
> +['RTE_EAL_NUMA_AWARE_HUGEPAGES', false]]
>  flags_n1sdp_extra = [
> ['RTE_MACHINE', '"n1sdp"'],
> ['RTE_MAX_NUMA_NODES', 1],
> @@ -99,6 +108,9 @@ machine_args_generic = [
> ['0xd0b', ['-mcpu=cortex-a76']],
> ['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'], 
> flags_n1sdp_extra]]
>
> +machine_args_generic_arm32 = [
> +['default_arm32', ['-march=armv7-a', '-mtune=cortex-a9', 
> '-mfpu=neon'], flags_default_arm32_extra]]
> +
>  machine_args_cavium = [
> ['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> ['native', ['-march=native']],
> @@ -114,6 +126,7 @@ machine_args_emag = [
>
>  ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
>  impl_generic = ['Generic armv8', flags_generic, machine_args_generic]
> +impl_generic_arm32 = ['Generic armv7', flags_generic_arm32, 
> machine_args_generic_arm32]
>  impl_0x41 = ['Arm', flags_arm, machine_args_generic]
>  impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
>  impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
> @@ -136,74 +149,74 @@ if not dpdk_conf.get('RTE_ARCH_64')
> dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> # the minimum architecture supported, armv7-a, needs the following,
> # mk/machine/armv7a/rte.vars.mk sets it too
> -   machine_args += '-mfpu=neon'
>  else
> dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> dpdk_conf.set('RTE_ARCH_ARM64', 1)
> +endif
>
> -   machine = []
> -   cmd_generic = ['generic', '', '', 'default', '']
> -   cmd_output = cmd_generic # Set generic by default
> -   machine_args = [] # Clear previous machine args
> -   if arm_force_default_march and not meson.is_cross_build()
> +machine = []
> +machine_args = [] # Clear previous machine args
> +cmd_generic = ['generic', '', '', 'default', '']
> +cmd_output = cmd_generic # Set generic by default
> +if arm_force_default_march and not meson.is_cross_build()
> +   machine = impl_generic
> +   impl_pn = 'default'
> +elif not meson.is_cross_build()
> +   # The script returns ['Implementer', 'Variant', 'Architecture',
> +   # 'Primary Part number', 'Revision']
> +   detect_vendor = find_program(join_paths(
> +   meson.current_source_dir(), 'armv8_machine.py'))
> +   cmd = run_command(detect_vendor.path())
> +   if cmd.returncode() == 0
> +   cmd_output = cmd.stdout().to_lower().strip().split(' ')
> +   endif
> +   # Set to generic if variable is not found
> +   machine = get_variable('impl_' + cmd_output[0], ['generic'])
> +   if machine[0] == 'generic'
> machine = impl_generic
> -   impl_pn = 'default'
> -   elif not meson.is_cross_build()
> -   # The script returns ['Implementer', 'Variant', 
> 'Architecture',
> -   # 'Primary Part number', 'Revision']
> -   detect_vendor = find_program(join_paths(
> -   meson.current_source_dir(), 
> 'armv8_machine.py'))
> -   cmd = run_command(detect_vendor.path())
> -   if cmd.returncode() == 0
> -   cmd_output = cmd.stdout().to_lower().strip().split(' 
> ')
> -   endif
> -   # Set to generic if variable is not found
> -   machine = get_variable('impl_' + cmd_output[0], ['generic'])
> -   if machine[0] == 'generic'
> -  

Re: [dpdk-dev] [PATCH v5 4/4] ci: add aarch64 -> arm32 cross compiling jobs

[Juraj Linkeš]

> -Original Message-
> From: Juraj Linkeš 
> Sent: Monday, July 6, 2020 10:28 AM
> To: bruce.richard...@intel.com; acon...@redhat.com;
> maicolgabr...@hotmail.com
> Cc: dev@dpdk.org; Juraj Linkeš 
> Subject: [PATCH v5 4/4] ci: add aarch64 -> arm32 cross compiling jobs
> 
> Add two jobs (static and shared libs), both building on aarch64 and producing 
> 32
> bit arm binaries. Do not run tests in these jobs.
> 
> Signed-off-by: Juraj Linkeš 
> ---
>  .ci/linux-build.sh |  7 ++-
>  .travis.yml| 19 +++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 
> d079801d7..e5407fb37
> 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -33,6 +33,11 @@ if [ "$AARCH64" = "1" ]; then
>  OPTS="$OPTS --cross-file config/arm/arm64_armv8_linux_gcc"
>  fi
> 
> +if [ "$ARM" = "1" ]; then
> +# convert the arch specifier
> +OPTS="$OPTS --cross-file config/arm/arm_armv7a_linux_gcc"
> +fi
> +
>  if [ "$BUILD_DOCS" = "1" ]; then
>  OPTS="$OPTS -Denable_docs=true"
>  fi
> @@ -53,7 +58,7 @@ OPTS="$OPTS --buildtype=debugoptimized"
>  meson build --werror $OPTS
>  ninja -C build
> 
> -if [ "$AARCH64" != "1" ]; then
> +if [ "$(uname -m)" = "x86_64" ]; then
>  devtools/test-null.sh
>  fi
> 

Now that I think about this a bit more, shouldn't this check be "if this is a 
native build, run test-null.sh?". If so, should we do something like this?
if file build/app/dpdk-testpmd | sed 's/-/_/g' | grep -q "$(uname -m)"; then
devtools/test-null.sh
fi

The sed is there because file returns x86-64 and uname returns x86_64.

What do you think?

> diff --git a/.travis.yml b/.travis.yml
> index 14f812423..baba29539 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -21,6 +21,10 @@ _aarch64_packages: &aarch64_packages
>- *required_packages
>- [gcc-aarch64-linux-gnu, libc6-dev-arm64-cross, pkg-config-aarch64-linux-
> gnu]
> 
> +_arm_packages: &arm_packages
> +  - *required_packages
> +  - [gcc-arm-linux-gnueabihf, libc6-dev-armhf-cross,
> +pkg-config-arm-linux-gnueabihf]
> +
>  _libabigail_build_packages: &libabigail_build_packages
>- [autoconf, automake, libtool, pkg-config, libxml2-dev, libdw-dev]
> 
> @@ -124,6 +128,21 @@ jobs:
>  packages:
>- *required_packages
>- *libabigail_build_packages
> +  # aarch64 cross-compiling arm jobs
> +  - env: DEF_LIB="shared" ARM=1
> +arch: arm64
> +compiler: gcc
> +addons:
> +  apt:
> +packages:
> +  - *arm_packages
> +  - env: DEF_LIB="static" ARM=1
> +arch: arm64
> +compiler: gcc
> +addons:
> +  apt:
> +packages:
> +  - *arm_packages
># aarch64 clang jobs
>- env: DEF_LIB="static"
>  arch: arm64
> --
> 2.20.1

Re: [dpdk-dev] [PATCH v4 1/2] eal: add WC store functions

[Nicolau, Radu]

Hi David, thanks for reviewing!

Some comments inline.


On 7/3/2020 4:19 PM, David Marchand wrote:

On Thu, Jul 2, 2020 at 11:24 AM Radu Nicolau  wrote:

+static inline void
+rte_write32_wc(uint32_t value, volatile void *addr);

This is a new API, and even if inlined, it should be marked experimental.

Will do.

Is volatile necessary?
Yes, most of these functions will be called on mmio addresses/volatile 
pointers. All other io functions have it.


+static inline void
+rte_write32_wc_relaxed(uint32_t value, volatile void *addr);
+
+
Double empty line (there are some other in this patch that I won't flag again).

I will check all occurrences.




  #endif /* __DOXYGEN__ */

  #ifndef RTE_OVERRIDE_IO_H
@@ -345,6 +378,20 @@ rte_write64(uint64_t value, volatile void *addr)
 rte_write64_relaxed(value, addr);
  }

+#ifndef RTE_NATIVE_WRITE32_WC

Missing return type, this causes build failure on anything but x86.

Yes, got lost in the copy/paste. I will fix it in the next version




+rte_write32_wc(uint32_t value, volatile void *addr)
+{
+   rte_write32(value, addr);
+}
+
+static __rte_always_inline void
+rte_write32_wc_relaxed(uint32_t value, volatile void *addr)
+{
+   rte_write32_relaxed(value, addr);
+}
+#endif /* RTE_NATIVE_WRITE32_WC */
+
+
  #endif /* RTE_OVERRIDE_IO_H */

  #endif /* _RTE_IO_H_ */
diff --git a/lib/librte_eal/x86/include/rte_io.h 
b/lib/librte_eal/x86/include/rte_io.h
index 2db71b1..c95ed67 100644
--- a/lib/librte_eal/x86/include/rte_io.h
+++ b/lib/librte_eal/x86/include/rte_io.h
@@ -9,8 +9,64 @@
  extern "C" {
  #endif

+#include "rte_cpuflags.h"

Inclusion of this header should be out of the extern "C" block.
Why? It is used elsewhere inside the extern "C" block e.g. 
x86/rte_spinlock.h




+
+#define RTE_NATIVE_WRITE32_WC
  #include "generic/rte_io.h"

+/**
+ * @internal
+ * MOVDIRI wrapper.
+ */
+static __rte_always_inline void
+_rte_x86_movdiri(uint32_t value, volatile void *addr)
+{
+   asm volatile(
+   /* MOVDIRI */
+   ".byte 0x40, 0x0f, 0x38, 0xf9, 0x02"
+   :
+   : "a" (value), "d" (addr));
+}
+
+static __rte_always_inline void
+rte_write32_wc(uint32_t value, volatile void *addr)
+{
+   static int _x86_movdiri_flag = -1;
+   if (_x86_movdiri_flag == 1) {
+   rte_wmb();
+   _rte_x86_movdiri(value, addr);
+   } else if (_x86_movdiri_flag == 0) {
+   rte_write32(value, addr);
+   } else {
+   _x86_movdiri_flag =
+   (rte_cpu_get_flag_enabled(RTE_CPUFLAG_MOVDIRI) > 0);

Can't this cpu flag check be moved in a constructor?
This would avoid this copy/paste.
We evaluated this approach but it creates more problems than if fixes - 
it will need a variable that needs to be exported and there is no good 
place to put it.




+   if (_x86_movdiri_flag == 1) {
+   rte_wmb();
+   _rte_x86_movdiri(value, addr);
+   } else {
+   rte_write32(value, addr);
+   }
+   }
+}
+
+static __rte_always_inline void
+rte_write32_wc_relaxed(uint32_t value, volatile void *addr)
+{
+   static int _x86_movdiri_flag = -1;

Same check with a static variable with the same name.

It should be no problem, they are static local variables.



I wonder if wrapping all of this in a single function would be more elegant.
Then rte_write32_wc(|_relaxed) would call it with a flag.
Yes, it will be more elegant but also it will cost more, it was written 
like this to minimize the number of branches taken for the movdiri path.

Re: [dpdk-dev] [PATCH v5 00/13] rte_log registration usage improvement

[Jerin Jacob]

On Fri, Jul 3, 2020 at 7:26 PM David Marchand  wrote:
>
> On Wed, Jul 1, 2020 at 2:34 PM  wrote:
> >
> > From: Jerin Jacob 
> >
> > This patch series improves the rte_log registration code snippet by
> > avoiding duplication of the code around registration by
> > introducing RTE_LOG_REGISTER macro.
> >

>
> Squashed as a single commit since the conversion patches do not give
> more insight than the first change.
> Removed unnecessary some double empty lines or at the end of files.
> And applied.
>
> Note: I spotted some left behind logtype declarations for
> mlx5_vdpa_logtype, rte_graph_logtype, librawdev_logtype and
> stack_logtype.
> Could you send a followup patch if there are unneeded?

Yes. I will send it. Thanks for check.

>
> Thanks for the cleanup.
>
>
> --
> David Marchand
>

Re: [dpdk-dev] [PATCH 1/2] devtools: fix filename in forbidden token check

[Arnon Warshavsky]

On Mon, Jul 6, 2020 at 11:00 AM David Marchand 
wrote:

> Fix displayed filename by adjusting the extraction from the patch.
>
> Before:
> Warning in /lib/librte_eal/linux/eal.c:
>
> After:
> Warning in lib/librte_eal/linux/eal.c:
>
> Fixes: 7413e7f2aeb3 ("devtools: alert on new calls to exit from libs")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand 
> ---
>  devtools/check-forbidden-tokens.awk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/devtools/check-forbidden-tokens.awk
> b/devtools/check-forbidden-tokens.awk
> index 8c89de3d4e..f86cbe8dc1 100755
> --- a/devtools/check-forbidden-tokens.awk
> +++ b/devtools/check-forbidden-tokens.awk
> @@ -62,7 +62,7 @@ BEGIN {
>  }
>  END {
> if (count > 0) {
> -   print "Warning in " substr(last_file,6) ":"
> +   print "Warning in " substr(last_file,7) ":"
> print MESSAGE
> exit RET_ON_FAIL
> }
> --
> 2.23.0
>
>
Acked-By: Arnon Warshavsky 

Re: [dpdk-dev] [pull-request] next-eventdev 20.08 RC1

[Thomas Monjalon]

05/07/2020 05:41, Jerin Jacob Kollanukkaran:
>   http://dpdk.org/git/next/dpdk-next-eventdev
> 
> 
> Harman Kalra (1):
>   event/octeontx: fix memory corruption
> 
> Harry van Haaren (1):
>   examples/eventdev_pipeline: fix 32-bit coremask logic
> 
> Pavan Nikhilesh (3):
>   event/octeontx2: fix device reconfigure
>   event/octeontx2: fix sub event type violation
>   event/octeontx2: improve datapath memory locality

Pulled patches above.

> Phil Yang (4):
>   eventdev: fix race condition on timer list counter
>   eventdev: use c11 atomics for lcore timer armed flag
>   eventdev: remove redundant code
>   eventdev: relax smp barriers with c11 atomics

I cannot merge this C11 series because of an ABI breakage:
http://mails.dpdk.org/archives/test-report/2020-July/140440.html

[dpdk-dev] [PATCH] net/ice: fix incorrect error log in generic flow

[Shougang Wang]

When create a rss rule with void action, the error log is "Invalid
input set". This patch fix the issue by adding check for the type of
first actions item.

Fixes: 7615a6895009 ("net/ice: rework for generic flow enabling")
Cc: sta...@dpdk.org

Signed-off-by: Shougang Wang 
---
 drivers/net/ice/ice_generic_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_generic_flow.c 
b/drivers/net/ice/ice_generic_flow.c
index ad103d0e8..80ef0e323 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -1873,7 +1873,7 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
return -rte_errno;
}
 
-   if (!actions) {
+   if (!actions || actions->type == RTE_FLOW_ACTION_TYPE_END) {
rte_flow_error_set(error, EINVAL,
   RTE_FLOW_ERROR_TYPE_ACTION_NUM,
   NULL, "NULL action.");
-- 
2.17.1

Re: [dpdk-dev] [dpdk-dev v4 1/4] cryptodev: add symmetric crypto data-path APIs

[Zhang, Roy Fan]

Hi Akhil,

> -Original Message-
> From: Akhil Goyal 
> Sent: Saturday, July 4, 2020 7:16 PM
> To: Zhang, Roy Fan ; dev@dpdk.org;
> ano...@marvell.com; asoma...@amd.com; ruifeng.w...@arm.com;
> Nagadheeraj Rottela ; Michael Shamis
> ; Ankur Dwivedi ; Jay
> Zhou ; De Lara Guarch, Pablo
> 
> Cc: Trahe, Fiona ; Bronowski, PiotrX
> ; Ananyev, Konstantin
> ; Thomas Monjalon
> 
> Subject: RE: [dpdk-dev v4 1/4] cryptodev: add symmetric crypto data-path
> APIs
> 
> Hi Fan,
> 
> > +
> > +/**
> > + * Asynchronous operation job descriptor.
> > + * Used by HW crypto devices direct API call that supports such activity
> > + **/
> > +struct rte_crypto_sym_job {
> > +   union {
> > +   /**
> > +* When RTE_CRYPTO_HW_ENQ_FLAG_IS_SGL bit is set in
> flags,
> > sgl
> > +* field is used as input data. Otherwise data_iova is
> > +* used.
> > +**/
> > +   rte_iova_t data_iova;
> > +   struct rte_crypto_sgl *sgl;
> > +   };
> > +   union {
> > +   /**
> > +* Different than cryptodev ops, all ofs and len fields have
> > +* the unit of bytes (including Snow3G/Kasumi/Zuc.
> > +**/
> > +   struct {
> > +   uint32_t cipher_ofs;
> > +   uint32_t cipher_len;
> > +   } cipher_only;
> > +   struct {
> > +   uint32_t auth_ofs;
> > +   uint32_t auth_len;
> > +   rte_iova_t digest_iova;
> > +   } auth_only;
> > +   struct {
> > +   uint32_t aead_ofs;
> > +   uint32_t aead_len;
> > +   rte_iova_t tag_iova;
> > +   uint8_t *aad;
> > +   rte_iova_t aad_iova;
> > +   } aead;
> > +   struct {
> > +   uint32_t cipher_ofs;
> > +   uint32_t cipher_len;
> > +   uint32_t auth_ofs;
> > +   uint32_t auth_len;
> > +   rte_iova_t digest_iova;
> > +   } chain;
> > +   };
> > +   uint8_t *iv;
> > +   rte_iova_t iv_iova;
> > +};
> 
> NACK,
> Why do you need this structure definitions again when you have similar ones
> (the ones used in CPU crypto) available for the same purpose.
> In case of CPU crypto, there were 2 main requirements
> - synchronous API instead of enq +deq
> - raw buffers.
> 

As you may have seen the structure definition is hW centric with the
IOVA addresses all over. Also as you will from the patch series the operation 
is 
Per operation basis instead of operating in a burst. The external application
may sooner know when a specific enqueue is failed. 

> Now for this patchset, the requirement is
> - raw buffers
> - asynchronous APIs
> 
> The data structure for raw buffers and crypto related offsets are already
> defined
> So they should be reused.
> And I believe with some changes in rte_crypto_op  and rte_crypto_sym_op,
> We can support raw buffers with the same APIs.
> Instead of m_src and m_dst, raw buffer data structures can be combined in a
> Union and some of the fields in the rte_crypto_op can be left NULL in case of
> raw buffers.
> 

This is a good point but we still face too many unnecessary fields to be NULL, 
such as
digest pointers, I have given a lot thought to this structure. Hopefully it 
covers
all vendor's HW symmetric crypto needs and in the same time it well squeeze 
the required HW addresses into 1 cacheline, instead of rte_crypto_op + 
rte_crypto_sym_op 3 cacheline footprint. Another purpose of the structure design
is the structure buffer can be taken from stack and can be used to fill all
jobs to the PMD HW.

> 
> > +/* Struct for user to perform HW specific enqueue/dequeue function calls
> */
> > +struct rte_crypto_hw_ops {
> > +   /* Driver written queue pair data pointer, should NOT be alterred by
> > +* the user.
> > +*/
> > +   void *qp;
> > +   /* Function handler to enqueue AEAD job */
> > +   rte_crypto_hw_enq_cb_fn enqueue_aead;
> > +   /* Function handler to enqueue cipher only job */
> > +   rte_crypto_hw_enq_cb_fn enqueue_cipher;
> > +   /* Function handler to enqueue auth only job */
> > +   rte_crypto_hw_enq_cb_fn enqueue_auth;
> > +   /* Function handler to enqueue cipher + hash chaining job */
> > +   rte_crypto_hw_enq_cb_fn enqueue_chain;
> > +   /* Function handler to query processed jobs */
> > +   rte_crypto_hw_query_processed query_processed;
> > +   /* Function handler to dequeue one job and return opaque data
> stored
> > */
> > +   rte_crypto_hw_deq_one_cb_fn dequeue_one;
> > +   /* Function handler to dequeue many jobs */
> > +   rte_crypto_hw_deq_many_cb_fn dequeue_many;
> > +   /* Reserved */
> > +   void *reserved[8];
> > +};
> 
> Why do we need such callbacks in the library?
> These should be inside the drivers, or else we do the same for
> Legacy case as well. The pain of finding the correct enq function for
> Appropriate crypto operation is already handled by

Re: [dpdk-dev] [PATCH v2 4/4] eventdev: relax smp barriers with c11 atomics

[Thomas Monjalon]

02/07/2020 07:26, Phil Yang:
> The implementation-specific opaque data is shared between arm and cancel
> operations. The state flag acts as a guard variable to make sure the
> update of opaque data is synchronized. This patch uses c11 atomics with
> explicit one way memory barrier instead of full barriers rte_smp_w/rmb()
> to synchronize the opaque data between timer arm and cancel threads.

I think we should write C11 (uppercase).

Please, in your explanations, try to be more specific.
Naming fields may help to make things clear.

[...]
> --- a/lib/librte_eventdev/rte_event_timer_adapter.h
> +++ b/lib/librte_eventdev/rte_event_timer_adapter.h
> @@ -467,7 +467,7 @@ struct rte_event_timer {
>*  - op: RTE_EVENT_OP_NEW
>*  - event_type: RTE_EVENT_TYPE_TIMER
>*/
> - volatile enum rte_event_timer_state state;
> + enum rte_event_timer_state state;
>   /**< State of the event timer. */

Why do you remove the volatile keyword?
It is not explained in the commit log.

This change is triggering a warning in the ABI check:
http://mails.dpdk.org/archives/test-report/2020-July/140440.html
Moving from volatile to non-volatile is probably not an issue.
I expect the code generated for the volatile case to work the same
in non-volatile case. Do you confirm?

In any case, we need an explanation and an ABI check exception.

Re: [dpdk-dev] [PATCH v5 01/51] net/bnxt: add basic infrastructure for VF reps

[Ferruh Yigit]

On 7/3/2020 10:01 PM, Ajit Khaparde wrote:
> From: Somnath Kotur 
> 
> Defines data structures and code to init/uninit
> VF representors during pci_probe and pci_remove
> respectively.
> Most of the dev_ops for the VF representor are just
> stubs for now and will be will be filled out in next patch.
> 
> To create a representor using testpmd:
> testpmd -c 0xff -wB:D.F,representor=1 -- -i
> testpmd -c 0xff -w05:02.0,representor=[1] -- -i
> 
> To create a representor using ovs-dpdk:
> 1. Firt add the trusted VF port to a bridge
> ovs-vsctl add-port ovsbr0 vf_rep1 -- set Interface vf_rep1 type=dpdk
> options:dpdk-devargs=:06:02.0
> 2. Add the representor port to the bridge
> ovs-vsctl add-port ovsbr0 vf_rep1 -- set Interface vf_rep1 type=dpdk
> options:dpdk-devargs=:06:02.0,representor=1
> 
> Signed-off-by: Somnath Kotur 
> Signed-off-by: Venkat Duvvuru 
> Reviewed-by: Kalesh AP 
> Reviewed-by: Ajit Khaparde 

<...>

>  static int bnxt_pci_remove(struct rte_pci_device *pci_dev)
>  {
> - if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> - return rte_eth_dev_pci_generic_remove(pci_dev,
> - bnxt_dev_uninit);
> - else
> + struct rte_eth_dev *eth_dev;
> +
> + eth_dev = rte_eth_dev_allocated(pci_dev->device.name);
> + if (!eth_dev)
> + return ENODEV; /* Invoked typically only by OVS-DPDK, by the
> + * time it comes here the eth_dev is already
> + * deleted by rte_eth_dev_close(), so returning
> + * +ve value will atleast help in proper cleanup
> + */

Why returning a positive error value? It hides the error since the caller of the
function does a "< 0" check.
Better to be more explicit and return '0' if an error is not intendent in this 
case.

Re: [dpdk-dev] [PATCH v5 00/51] net/bnxt: add features for host-based flow management

[Ferruh Yigit]

On 7/3/2020 10:01 PM, Ajit Khaparde wrote:
> v1->v2:
>  - update commit message
>  - rebase patches against latest changes in the tree
>  - fix signed-off-by tags
>  - update release notes
> 
> v2->v3:
>  - fix compilation issues
> 
> v3->v4:
>  - rebase against latest dpdk-next-net
> 
> v4->v5:
>  - fix uninitlalized variable in patch [29/51]
>  - rebase against latest dpdk-next-net
> 
> Ajit Khaparde (1):
>   doc: update release notes
> 
> Jay Ding (5):
>   net/bnxt: implement support for TCAM access
>   net/bnxt: support two level priority for TCAMs
>   net/bnxt: add external action alloc and free
>   net/bnxt: implement IF tables set and get
>   net/bnxt: add global config set and get APIs
> 
> Kishore Padmanabha (8):
>   net/bnxt: integrate with the latest tf core changes
>   net/bnxt: add support for if table processing
>   net/bnxt: disable Tx vector mode if truflow is enabled
>   net/bnxt: add index opcode and operand to mapper table
>   net/bnxt: add support for global resource templates
>   net/bnxt: add support for internal exact match entries
>   net/bnxt: add support for conditional execution of mapper tables
>   net/bnxt: add VF-rep and stat templates
> 
> Lance Richardson (1):
>   net/bnxt: initialize parent PF information
> 
> Michael Wildt (7):
>   net/bnxt: add multi device support
>   net/bnxt: update multi device design support
>   net/bnxt: multiple device implementation
>   net/bnxt: update identifier with remap support
>   net/bnxt: update RM with residual checker
>   net/bnxt: update table get to use new design
>   net/bnxt: add TF register and unregister
> 
> Mike Baucom (1):
>   net/bnxt: add support for internal encap records
> 
> Peter Spreadborough (7):
>   net/bnxt: add support for exact match
>   net/bnxt: modify EM insert and delete to use HWRM direct
>   net/bnxt: add HCAPI interface support
>   net/bnxt: support EM and TCAM lookup with table scope
>   net/bnxt: update RM to support HCAPI only
>   net/bnxt: remove table scope from session
>   net/bnxt: add support for EEM System memory
> 
> Randy Schacher (2):
>   net/bnxt: add core changes for EM and EEM lookups
>   net/bnxt: align CFA resources with RM
> 
> Shahaji Bhosle (2):
>   net/bnxt: support bulk table get and mirror
>   net/bnxt: support two-level priority for TCAMs
> 
> Somnath Kotur (7):
>   net/bnxt: add basic infrastructure for VF reps
>   net/bnxt: add support for VF-reps data path
>   net/bnxt: get IDs for VF-Rep endpoint
>   net/bnxt: parse reps along with other dev-args
>   net/bnxt: create default flow rules for the VF-rep
>   net/bnxt: add ULP Flow counter Manager
>   net/bnxt: add support for count action in flow query
> 
> Venkat Duvvuru (10):
>   net/bnxt: modify port db dev interface
>   net/bnxt: get port and function info
>   net/bnxt: add support for hwrm port phy qcaps
>   net/bnxt: modify port db to handle more info
>   net/bnxt: enable port MAC qcfg command for trusted VF
>   net/bnxt: enhancements for port db
>   net/bnxt: manage VF to VFR conduit
>   net/bnxt: fill mapper parameters with default rules
>   net/bnxt: add port default rules for ingress and egress
>   net/bnxt: fill cfa action in the Tx descriptor

Hi Ajit,

There are checkpatch warnings and some spelling errors (checkpatch will show
them by default when 'codespell' is installed), can you please check them?

Re: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get status message

[Xia, Chenbo]

Hi Adrian,

> -Original Message-
> From: Adrian Moreno 
> Sent: Monday, July 6, 2020 4:49 PM
> To: Xia, Chenbo ; dev@dpdk.org;
> shah...@mellanox.com; ma...@mellanox.com; maxime.coque...@redhat.com;
> Wang, Xiao W ; viachesl...@mellanox.com
> Cc: jasow...@redhat.com; l...@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get 
> status
> message
> 
> 
> 
> On 7/6/20 5:22 AM, Xia, Chenbo wrote:
> > Hi Adrian,
> >
> >> -Original Message-
> >> From: dev  On Behalf Of Adrian Moreno
> >> Sent: Thursday, July 2, 2020 4:33 PM
> >> To: dev@dpdk.org; Ye, Xiaolong ;
> >> shah...@mellanox.com; ma...@mellanox.com;
> maxime.coque...@redhat.com;
> >> Wang, Xiao W ; viachesl...@mellanox.com
> >> Cc: jasow...@redhat.com; l...@redhat.com; Adrian Moreno
> >> 
> >> Subject: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get
> >> status message
> >>
> >> This patch adds support to the new Virtio device get status Vhost-user
> message.
> >>
> >> The driver can send this new message to read the device status.
> >>
> >> One of the uses of this message is to ensure the feature negotiation
> >> has succeded.  According to the virtio spec, after completing the
> >> feature negotiation, the driver sets the FEATURE_OK status bit and
> >> re-reads it to ensure the device has accepted the features.
> >>
> >> This patch also clears the FEATURE_OK status bit if the feature
> >> negotiation has failed to let the driver know about his failure.
> >>
> >> Signed-off-by: Adrian Moreno 
> >> ---
> >>  lib/librte_vhost/vhost.h  |  2 ++
> >>  lib/librte_vhost/vhost_user.c | 32 
> >> lib/librte_vhost/vhost_user.h |  1 +
> >>  3 files changed, 35 insertions(+)
> >>
> >> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> >> index 25d31c71b..e743821cc 100644
> >> --- a/lib/librte_vhost/vhost.h
> >> +++ b/lib/librte_vhost/vhost.h
> >> @@ -32,6 +32,8 @@
> >>  #define VIRTIO_DEV_BUILTIN_VIRTIO_NET 4
> >>  /* Used to indicate that the device has its own data path and
> >> configured */ #define VIRTIO_DEV_VDPA_CONFIGURED 8
> >> +/* Used to indicate that the feature negotiation failed */ #define
> >> +VIRTIO_DEV_FEATURES_FAILED 16
> >>
> >>  /* Backend value set by guest. */
> >>  #define VIRTIO_DEV_STOPPED -1
> >> diff --git a/lib/librte_vhost/vhost_user.c
> >> b/lib/librte_vhost/vhost_user.c index
> >> 8d3d13913..00da7bf18 100644
> >> --- a/lib/librte_vhost/vhost_user.c
> >> +++ b/lib/librte_vhost/vhost_user.c
> >> @@ -88,6 +88,7 @@ static const char
> >> *vhost_message_str[VHOST_USER_MAX]
> >> = {
> >>[VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD",
> >>[VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD",
> >>[VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS",
> >> +  [VHOST_USER_GET_STATUS] = "VHOST_USER_GET_STATUS",
> >>  };
> >>
> >>  static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg);
> >> @@ -339,6
> >> +340,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct
> >> VhostUserMsg *msg,
> >>VHOST_LOG_CONFIG(ERR,
> >>"(%d) received invalid negotiated features.\n",
> >>dev->vid);
> >> +  dev->flags |= VIRTIO_DEV_FEATURES_FAILED;
> >> +  dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
> >> +
> >>return RTE_VHOST_MSG_RESULT_ERR;
> >>}
> >>
> >> @@ -402,6 +406,7 @@ vhost_user_set_features(struct virtio_net **pdev,
> >> struct VhostUserMsg *msg,
> >>if (vdpa_dev)
> >>vdpa_dev->ops->set_features(dev->vid);
> >>
> >> +  dev->flags &= ~VIRTIO_DEV_FEATURES_FAILED;
> >>return RTE_VHOST_MSG_RESULT_OK;
> >>  }
> >>
> >> @@ -2458,6 +2463,22 @@ vhost_user_postcopy_end(struct virtio_net
> >> **pdev, struct VhostUserMsg *msg,
> >>return RTE_VHOST_MSG_RESULT_REPLY;
> >>  }
> >>
> >> +static int
> >> +vhost_user_get_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
> >> +int main_fd __rte_unused)
> >> +{
> >> +  struct virtio_net *dev = *pdev;
> >> +
> >> +  if (validate_msg_fds(msg, 0) != 0)
> >> +  return RTE_VHOST_MSG_RESULT_ERR;
> >> +
> >> +  msg->payload.u64 = dev->status;
> >> +  msg->size = sizeof(msg->payload.u64);
> >> +  msg->fd_num = 0;
> >> +
> >> +  return RTE_VHOST_MSG_RESULT_OK;
> >> +}
> >> +
> >>  static int
> >>  vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
> >>int main_fd __rte_unused)
> >> @@ -2476,6 +2497,16 @@ vhost_user_set_status(struct virtio_net
> >> **pdev, struct VhostUserMsg *msg,
> >>
> >>dev->status = msg->payload.u64;
> >>
> >> +  if ((dev->status & VIRTIO_DEVICE_STATUS_FEATURES_OK) &&
> >> +  (dev->flags & VIRTIO_DEV_FEATURES_FAILED)) {
> >> +  VHOST_LOG_CONFIG(ERR, "FEATURES_OK bit is set but feature
> >> negotiation failed\n");
> >> +  /*
> >> +   * Clear the bit to let the driver know about the feature
> >> +   * negotiation failure
> >> +  

Re: [dpdk-dev] [PATCH v5 1/3] eal: disable function versioning on Windows

[Fady Bader]

> -Original Message-
> From: Bruce Richardson 
> Sent: Monday, July 6, 2020 11:20 AM
> To: Fady Bader 
> Cc: dev@dpdk.org; Thomas Monjalon ; Tasnim Bashar
> ; Tal Shnaiderman ; Yohad Tor
> ; dmitry.kozl...@gmail.com;
> harini.ramakrish...@microsoft.com; ocard...@microsoft.com;
> pallavi.ka...@intel.com; ranjit.me...@intel.com; olivier.m...@6wind.com;
> arybche...@solarflare.com; m...@ashroe.eu; nhor...@tuxdriver.com
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] eal: disable function versioning on
> Windows
> 
> On Sun, Jul 05, 2020 at 02:46:27PM +0300, Fady Bader wrote:
> > Function versioning implementation is not supported by Windows.
> > Function versioning was disabled on Windows.
> >
> > Signed-off-by: Fady Bader 
> > ---
> >  lib/librte_eal/include/rte_function_versioning.h | 2 +-
> >  lib/meson.build  | 5 +
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/include/rte_function_versioning.h
> > b/lib/librte_eal/include/rte_function_versioning.h
> > index f588f2643b..9890551ba1 100644
> > --- a/lib/librte_eal/include/rte_function_versioning.h
> > +++ b/lib/librte_eal/include/rte_function_versioning.h
> > @@ -7,7 +7,7 @@
> >  #define _RTE_FUNCTION_VERSIONING_H_
> >  #include 
> >
> > -#ifndef RTE_USE_FUNCTION_VERSIONING
> > +#if !defined RTE_USE_FUNCTION_VERSIONING && !defined
> > +RTE_EXEC_ENV_WINDOWS
> >  #error Use of function versioning disabled, is 
> > "use_function_versioning=true"
> in meson.build?
> >  #endif
> >
> > diff --git a/lib/meson.build b/lib/meson.build index
> > c1b9e1633f..a1ab582a51 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -107,6 +107,11 @@ foreach l:libraries
> > shared_dep = declare_dependency(include_directories:
> includes)
> > static_dep = shared_dep
> > else
> > +   if is_windows and use_function_versioning
> > +   message('@0@: Function versioning is not
> supported by Windows.'
> > +   .format(name))
> > +   use_function_versioning = false
> > +   endif
> >
> > if use_function_versioning
> > cflags += '-DRTE_USE_FUNCTION_VERSIONING'
> > --
> 
> I wonder if this patch can be simplified as follows.
> 
> Presumably the use of the RTE_USE_FUNCTION_VERSIONING cflag doesn't
> cause any problems building on windows, since it's basically nothing except a 
> flag
> to indicate the build is configured properly, so that block can be left 
> intact.
> Instead what is needed to be disabled is the RTE_BUILD_SHARED_LIB setting so
> that we use the static macros. Therefore, I think the same result can be got 
> by
> just changing the following line in lib/meson.build:
> 
> @@ -138,7 +138,7 @@ foreach l:libraries
> include_directories: includes,
> dependencies: static_deps)
> 
> -   if not use_function_versioning
> +   if not use_function_versioning or is_windows
> # use pre-build objects to build shared lib
> sources = []
> objs += 
> static_lib.extract_all_objects(recursive: false)

Good point, the patch is much simpler now.
I'll send a new version soon.

> 
> Then no error message disabling is needed in windows. I also don't think a
> message about function versioning not being supported on windows is
> necessary. It's not something the user can do anything about himself anyway. 
> If
> we want such a message I think it should just be printed once at the start of 
> the
> configuration process, rather than each time we hit a versioned library.

It was added in order to avoid confusions in case Linux developers try to run 
the 
project on windows. I'll print it once.

> 
> Regards,
> /Bruce

[dpdk-dev] [PATCH 0/3] Minor rawdev fixes and enhancements

[Bruce Richardson]

Improve the rawdev info_get function to return the numa socket id value to
the user, and to allow querying basic info of an unknown device by allowing
generic info to be returned without having to know the device type.

Also add in the missing dump function to the map file.

Bruce Richardson (3):
  rawdev: allow calling info function for unknown rawdevs
  rawdev: return NUMA socket id to the user
  rawdev: fix missing dump function in map file

 lib/librte_rawdev/rte_rawdev.c   | 14 +++---
 lib/librte_rawdev/rte_rawdev.h   |  8 +++-
 lib/librte_rawdev/rte_rawdev_version.map |  1 +
 3 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH 3/3] rawdev: fix missing dump function in map file

[Bruce Richardson]

The rte_rawdev_dump function was missing from the map file, meaning it was
unavailable for use when linking dynamically.

Fixes: c88b3f2558ed ("rawdev: introduce raw device library")
Cc: shreyansh.j...@nxp.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 lib/librte_rawdev/rte_rawdev_version.map | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_rawdev/rte_rawdev_version.map 
b/lib/librte_rawdev/rte_rawdev_version.map
index d847c9e0d..63b54f598 100644
--- a/lib/librte_rawdev/rte_rawdev_version.map
+++ b/lib/librte_rawdev/rte_rawdev_version.map
@@ -5,6 +5,7 @@ DPDK_20.0 {
rte_rawdev_configure;
rte_rawdev_count;
rte_rawdev_dequeue_buffers;
+   rte_rawdev_dump;
rte_rawdev_enqueue_buffers;
rte_rawdev_firmware_load;
rte_rawdev_firmware_status_get;
-- 
2.25.1

[dpdk-dev] [PATCH 1/3] rawdev: allow calling info function for unknown rawdevs

[Bruce Richardson]

To call the rte_rawdev_info_get() function, the user currently has to know
the underlying type of the device in order to pass an appropriate structure
or buffer as the dev_private pointer in the info structure. By allowing a
NULL value for this field, we can skip getting the device-specific info and
just return the generic info - including the device name and driver, which
can be used to determine the device type - to the user.

This ensures that basic info can be get for all rawdevs, without knowing
the type, and even if the info driver API call has not been implemented for
the device.

Cc: sta...@dpdk.org
Signed-off-by: Bruce Richardson 
---
 lib/librte_rawdev/rte_rawdev.c | 6 --
 lib/librte_rawdev/rte_rawdev.h | 8 +++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c
index aec61f425..b18638435 100644
--- a/lib/librte_rawdev/rte_rawdev.c
+++ b/lib/librte_rawdev/rte_rawdev.c
@@ -90,8 +90,10 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info 
*dev_info)
 
rawdev = &rte_rawdevs[dev_id];
 
-   RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP);
-   (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private);
+   if (dev_info->dev_private != NULL) {
+   RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, 
-ENOTSUP);
+   (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private);
+   }
 
if (dev_info) {
 
diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h
index ed011ca22..7fb6cb188 100644
--- a/lib/librte_rawdev/rte_rawdev.h
+++ b/lib/librte_rawdev/rte_rawdev.h
@@ -77,7 +77,13 @@ struct rte_rawdev_info;
  *
  * @param[out] dev_info
  *   A pointer to a structure of type *rte_rawdev_info* to be filled with the
- *   contextual information of the device.
+ *   contextual information of the device. The dev_info->dev_private field
+ *   should point to an appropriate buffer space for holding the device-
+ *   specific info for that hardware.
+ *   If the dev_private field is set to NULL, then the device-specific info
+ *   function will not be called and only basic information about the device
+ *   will be returned. This can be used to safely query the type of a rawdev
+ *   instance without needing to know the size of the private data to return.
  *
  * @return
  *   - 0: Success, driver updates the contextual information of the raw device
-- 
2.25.1

[dpdk-dev] [PATCH 2/3] rawdev: return NUMA socket id to the user

[Bruce Richardson]

The rawdev info struct has a socket_id field which was not filled in.

We can also omit the checks for the parameter struct being null, since
that is previously checked in the function.

Cc: sta...@dpdk.org
Signed-off-by: Bruce Richardson 
---
 lib/librte_rawdev/rte_rawdev.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c
index b18638435..9ee160455 100644
--- a/lib/librte_rawdev/rte_rawdev.c
+++ b/lib/librte_rawdev/rte_rawdev.c
@@ -95,11 +95,9 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info 
*dev_info)
(*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private);
}
 
-   if (dev_info) {
-
-   dev_info->driver_name = rawdev->driver_name;
-   dev_info->device = rawdev->device;
-   }
+   dev_info->driver_name = rawdev->driver_name;
+   dev_info->device = rawdev->device;
+   dev_info->socket_id = rawdev->socket_id;
 
return 0;
 }
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get status message

[Adrian Moreno]

On 7/6/20 12:29 PM, Xia, Chenbo wrote:
> Hi Adrian,
> 
>> -Original Message-
>> From: Adrian Moreno 
>> Sent: Monday, July 6, 2020 4:49 PM
>> To: Xia, Chenbo ; dev@dpdk.org;
>> shah...@mellanox.com; ma...@mellanox.com; maxime.coque...@redhat.com;
>> Wang, Xiao W ; viachesl...@mellanox.com
>> Cc: jasow...@redhat.com; l...@redhat.com
>> Subject: Re: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get 
>> status
>> message
>>
>>
>>
>> On 7/6/20 5:22 AM, Xia, Chenbo wrote:
>>> Hi Adrian,
>>>
 -Original Message-
 From: dev  On Behalf Of Adrian Moreno
 Sent: Thursday, July 2, 2020 4:33 PM
 To: dev@dpdk.org; Ye, Xiaolong ;
 shah...@mellanox.com; ma...@mellanox.com;
>> maxime.coque...@redhat.com;
 Wang, Xiao W ; viachesl...@mellanox.com
 Cc: jasow...@redhat.com; l...@redhat.com; Adrian Moreno
 
 Subject: [dpdk-dev] [PATCH v2 6/8] vhost: add support for virtio get
 status message

 This patch adds support to the new Virtio device get status Vhost-user
>> message.

 The driver can send this new message to read the device status.

 One of the uses of this message is to ensure the feature negotiation
 has succeded.  According to the virtio spec, after completing the
 feature negotiation, the driver sets the FEATURE_OK status bit and
 re-reads it to ensure the device has accepted the features.

 This patch also clears the FEATURE_OK status bit if the feature
 negotiation has failed to let the driver know about his failure.

 Signed-off-by: Adrian Moreno 
 ---
  lib/librte_vhost/vhost.h  |  2 ++
  lib/librte_vhost/vhost_user.c | 32 
 lib/librte_vhost/vhost_user.h |  1 +
  3 files changed, 35 insertions(+)

 diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
 index 25d31c71b..e743821cc 100644
 --- a/lib/librte_vhost/vhost.h
 +++ b/lib/librte_vhost/vhost.h
 @@ -32,6 +32,8 @@
  #define VIRTIO_DEV_BUILTIN_VIRTIO_NET 4
  /* Used to indicate that the device has its own data path and
 configured */ #define VIRTIO_DEV_VDPA_CONFIGURED 8
 +/* Used to indicate that the feature negotiation failed */ #define
 +VIRTIO_DEV_FEATURES_FAILED 16

  /* Backend value set by guest. */
  #define VIRTIO_DEV_STOPPED -1
 diff --git a/lib/librte_vhost/vhost_user.c
 b/lib/librte_vhost/vhost_user.c index
 8d3d13913..00da7bf18 100644
 --- a/lib/librte_vhost/vhost_user.c
 +++ b/lib/librte_vhost/vhost_user.c
 @@ -88,6 +88,7 @@ static const char
 *vhost_message_str[VHOST_USER_MAX]
 = {
[VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD",
[VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD",
[VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS",
 +  [VHOST_USER_GET_STATUS] = "VHOST_USER_GET_STATUS",
  };

  static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg);
 @@ -339,6
 +340,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct
 VhostUserMsg *msg,
VHOST_LOG_CONFIG(ERR,
"(%d) received invalid negotiated features.\n",
dev->vid);
 +  dev->flags |= VIRTIO_DEV_FEATURES_FAILED;
 +  dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
 +
return RTE_VHOST_MSG_RESULT_ERR;
}

 @@ -402,6 +406,7 @@ vhost_user_set_features(struct virtio_net **pdev,
 struct VhostUserMsg *msg,
if (vdpa_dev)
vdpa_dev->ops->set_features(dev->vid);

 +  dev->flags &= ~VIRTIO_DEV_FEATURES_FAILED;
return RTE_VHOST_MSG_RESULT_OK;
  }

 @@ -2458,6 +2463,22 @@ vhost_user_postcopy_end(struct virtio_net
 **pdev, struct VhostUserMsg *msg,
return RTE_VHOST_MSG_RESULT_REPLY;
  }

 +static int
 +vhost_user_get_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
 +int main_fd __rte_unused)
 +{
 +  struct virtio_net *dev = *pdev;
 +
 +  if (validate_msg_fds(msg, 0) != 0)
 +  return RTE_VHOST_MSG_RESULT_ERR;
 +
 +  msg->payload.u64 = dev->status;
 +  msg->size = sizeof(msg->payload.u64);
 +  msg->fd_num = 0;
 +
 +  return RTE_VHOST_MSG_RESULT_OK;
 +}
 +
  static int
  vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
int main_fd __rte_unused)
 @@ -2476,6 +2497,16 @@ vhost_user_set_status(struct virtio_net
 **pdev, struct VhostUserMsg *msg,

dev->status = msg->payload.u64;

 +  if ((dev->status & VIRTIO_DEVICE_STATUS_FEATURES_OK) &&
 +  (dev->flags & VIRTIO_DEV_FEATURES_FAILED)) {
 +  VHOST_LOG_CONFIG(ERR, "FEATURES_OK bit is set but feature
 negotiation failed\n");
 +  /*
 +   * Clear the bit to let the driver know about the 

Re: [dpdk-dev] [PATCH v4 01/10] eal: introduce macros for getting valuefor bit

[Morten Brørup]

> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Parav Pandit
> Sent: Friday, July 3, 2020 3:47 PM
> 
> There are several drivers which duplicate bit generation macro.
> Introduce a generic bit macros so that such drivers avoid redefining
> same in multiple drivers.
> 
> Signed-off-by: Parav Pandit 
> Acked-by: Matan Azrad 
> ---
> Changelog:
> v1->v2:
>  - Addressed comments from Thomas and Gaten.
>  - Avoided new file, added macro to rte_bitops.h
> ---
>  lib/librte_eal/include/rte_bitops.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_eal/include/rte_bitops.h
> b/lib/librte_eal/include/rte_bitops.h
> index 740927f3b..d72c7cd93 100644
> --- a/lib/librte_eal/include/rte_bitops.h
> +++ b/lib/librte_eal/include/rte_bitops.h
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
> 
> +#define RTE_BIT(bit_num) (1UL << (bit_num))

Is the return value 32 or 64 bit, or is intended to depend on the target 
architecture?

Please be explicit by using UINT32_C(1) or UINT64_C(1) instead of 1UL, if you 
want a specific size.

It could be a static inline __attribute__((__pure__)) function instead of a 
macro, but it's not important for me.

The macro/function needs a description for the documentation.

I'm also concerned about the name of the macro being too generic. But the 
effort of changing all the drivers where it is being used already could be too 
big if the name changes too.

And the macro/function is new, so shouldn't it - in theory - be marked as 
experimental?

> +
>  /* 32-bit relaxed operations -
> ---*/
> 
>  /**
> --
> 2.26.2
> 

[dpdk-dev] 答复: Questions about rte_flow_create APIs

[oulijun]

Ok, thanks.  I have adjusted.

发件人: Ori Kam [mailto:or...@mellanox.com]
发送时间: 2020年7月5日 19:58
收件人: oulijun ; ferruh.yi...@intel.com
抄送: dev@dpdk.org; us...@dpdk.org
主题: RE: Questions about rte_flow_create APIs

Hi Oulijun,

First small comment, I think you sent the mail in HTML format,
please next time send it as plain text.


From: oulijun mailto:ouli...@huawei.com>>
Sent: Friday, July 3, 2020 9:45 AM
To: Ori Kam mailto:or...@mellanox.com>>; 
ferruh.yi...@intel.com
Cc: dev@dpdk.org; us...@dpdk.org
Subject: Questions about rte_flow_create APIs

Hi, Guys
  I am analyzing rte_flow_create API and test and is testing server parametric 
scenario.
  When use testpmd and the following flow creat cmd:

  testpmd> flow create 0 ingress pattern end actions rss func simple_xor  / end

why the contents of rte_flow_action_rss is not same acquisition method after 
run the flowing codes.
struct rte_flow_action_rss conf;

rss_conf = action->conf;

the Value of unspecified parameter is not same acquisition method.

The rss_conf.type is default;
The rss_conf.key_len is deault;
The rss_conf.queue_num is deault;

However, the rss_conf.key is random and unpredictable

The result is that when the user does not specify the rss key, the driver will 
fill a random value into the hardware。
The rss key value found by the user is different from the first value, which 
results in the user not being able to use it correctly.
I think the reasonable behavior is that either the rss rule was not 
successfully created or the content of the rss key that was queried has not 
changed

testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
RSS key:
4439796BB54C5023B675EA5B124F9F30B8A2C03DDFDC4D02A08C9B334AF64A4C05C6FA343958D8557D99583AE138C92E81150366
testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss types 
ipv4-udp end queues end / end
Flow rule #0 created
testpmd> show port 0 rss-hash key
RSS functions:
all ipv4-udp udp
RSS key:
74657374706D6427732064656661756C74205253532068617368206B65792C206F7665727269646520697420666F722062657474

What do you think?
[Ori] I think this is a testpmd only issue, since it uses the same structure 
when it pass it to the pmd.
you can look at it in also in a different way.
1. the application create first flow  using empty rss action, this mean that 
the flow gets the default RSS functions,
2. the application insert the flow you gave with different RSS function than 
the original one.
3. the application create a new flow with empty RSS, no the flow is created 
using the new RSS function and not the old one.
So I don’t think this is a true issue.
Best,
Ori


Thanks
Lijun Ou

[dpdk-dev] [PATCH v2 2/2] test/service: add test for service active on two lcores

[Andrew Rybchenko]

From: Igor Romanov 

The test checks that the service may be active API works
when there are two cores: a non-service lcore and a service one.

The API notes to take care when checking the status of a running
service, but the test setup allows for a safe usage in that case.

Signed-off-by: Igor Romanov 
Signed-off-by: Andrew Rybchenko 
---
 app/test/test_service_cores.c | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c
index 981e212130..80dc80ff9c 100644
--- a/app/test/test_service_cores.c
+++ b/app/test/test_service_cores.c
@@ -910,6 +910,55 @@ service_may_be_active(void)
return unregister_all();
 }
 
+/* check service may be active when service is running on a second lcore */
+static int
+service_active_two_cores(void)
+{
+   const uint32_t sid = 0;
+   int i;
+
+   uint32_t lcore = rte_get_next_lcore(/* start core */ -1,
+   /* skip master */ 1,
+   /* wrap */ 0);
+   uint32_t slcore = rte_get_next_lcore(/* start core */ lcore,
+/* skip master */ 1,
+/* wrap */ 0);
+
+   /* start the service on the second available lcore */
+   TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 1),
+   "Starting valid service failed");
+   TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore),
+   "Add service core failed when not in use before");
+   TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore, 1),
+   "Enabling valid service on valid core failed");
+   TEST_ASSERT_EQUAL(0, rte_service_lcore_start(slcore),
+   "Service core start after add failed");
+
+   /* ensures core really is running the service function */
+   TEST_ASSERT_EQUAL(1, service_lcore_running_check(),
+   "Service core expected to poll service but it didn't");
+
+   /* ensures that service may be active reports running state */
+   TEST_ASSERT_EQUAL(1, rte_service_may_be_active(sid),
+   "Service may be active did not report running state");
+
+   /* stop the service */
+   TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0),
+   "Error: Service stop returned non-zero");
+
+   /* give the service 100ms to stop running */
+   for (i = 0; i < 100; i++) {
+   if (!rte_service_may_be_active(sid))
+   break;
+   rte_delay_ms(SERVICE_DELAY);
+   }
+
+   TEST_ASSERT_EQUAL(0, rte_service_may_be_active(sid),
+ "Error: Service not stopped after 100ms");
+
+   return unregister_all();
+}
+
 static struct unit_test_suite service_tests  = {
.suite_name = "service core test suite",
.setup = testsuite_setup,
@@ -931,6 +980,7 @@ static struct unit_test_suite service_tests  = {
TEST_CASE_ST(dummy_register, NULL, service_app_lcore_mt_safe),
TEST_CASE_ST(dummy_register, NULL, service_app_lcore_mt_unsafe),
TEST_CASE_ST(dummy_register, NULL, service_may_be_active),
+   TEST_CASE_ST(dummy_register, NULL, service_active_two_cores),
TEST_CASES_END() /**< NULL terminate unit test array */
}
 };
-- 
2.17.1

[dpdk-dev] [PATCH v2 1/2] service: fix wrong lcore indices

[Andrew Rybchenko]

From: Igor Romanov 

The service core list is populated, but not used. Incorrect
lcore states are examined for a service.

Use the populated list to iterate over service cores.

Fixes: e30dd31847d2 ("service: add mechanism for quiescing")
Cc: sta...@dpdk.org

Signed-off-by: Igor Romanov 
Signed-off-by: Andrew Rybchenko 
---
 lib/librte_eal/common/rte_service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/rte_service.c 
b/lib/librte_eal/common/rte_service.c
index 6123a2124d..e2795f857e 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -422,7 +422,7 @@ rte_service_may_be_active(uint32_t id)
return -EINVAL;
 
for (i = 0; i < lcore_count; i++) {
-   if (lcore_states[i].service_active_on_lcore[id])
+   if (lcore_states[ids[i]].service_active_on_lcore[id])
return 1;
}
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH] service: fix wrong lcore indexes

[Andrew Rybchenko]

On 7/4/20 6:10 PM, David Marchand wrote:
> On Sat, Jul 4, 2020 at 5:07 PM Honnappa Nagarahalli
>  wrote:
>>
>> Hi Andrew/Igor,
>> A effective test case is missing for this, can you please add a test 
>> case? Otherwise it looks good.
> 
> +1, I was about to reply this.

Done in v2

[dpdk-dev] [PATCH v3 1/8] vhost: fix virtio ready flag check

[Adrian Moreno]

From: Maxime Coquelin 

Before checking whether the device is ready is done
a check on whether the RUNNING flag is set. Then the
READY flag is set if virtio_is_ready() returns true.

While it seems to not cause any issue, it makes more
sense to check whether the READY flag is set and not
the RUNNING one.

Fixes: c0674b1bc898 ("vhost: move the device ready check at proper place")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---
 lib/librte_vhost/vhost_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 6039a8fdb..5750dde6d 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -2825,7 +2825,7 @@ vhost_user_msg_handler(int vid, int fd)
}
 
 
-   if (!(dev->flags & VIRTIO_DEV_RUNNING) && virtio_is_ready(dev)) {
+   if (!(dev->flags & VIRTIO_DEV_READY) && virtio_is_ready(dev)) {
dev->flags |= VIRTIO_DEV_READY;
 
if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
-- 
2.26.2