Re: [dpdk-dev] [PATCH] eal/x86: get hypervisor name

[Jerin Jacob]

-Original Message-
> Date: Thu, 30 Nov 2017 22:47:20 +0100
> From: Thomas Monjalon 
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> X-Mailer: git-send-email 2.15.0
> 
> The CPUID instruction is catched by hypervisor which can return
> a flag indicating one is running, and its name.
> 
> Suggested-by: Stephen Hemminger 
> Signed-off-by: Thomas Monjalon 
> ---
> warning: to be tested
> ---
>  lib/librte_eal/common/arch/arm/rte_cpuflags.c  |  6 +
>  lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +
>  lib/librte_eal/common/arch/x86/rte_cpuflags.c  | 30 
> ++
>  .../common/include/arch/x86/rte_cpuflags.h |  1 +
>  .../common/include/generic/rte_cpuflags.h  | 14 ++
>  lib/librte_eal/rte_eal_version.map |  9 ++-
>  6 files changed, 65 insertions(+), 1 deletion(-)
>   RTE_CPUFLAG_FPU,/**< FPU */
> diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h 
> b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> index c1c5551fc..3832fb851 100644
> --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> @@ -93,4 +93,18 @@ rte_cpu_check_supported(void);
>  int
>  rte_cpu_is_supported(void);
>  
> +enum rte_hypervisor {
> + RTE_HYPERVISOR_NONE,
> + RTE_HYPERVISOR_KVM,
> + RTE_HYPERVISOR_HYPERV,
> + RTE_HYPERVISOR_VMWARE,
> + RTE_HYPERVISOR_UNKNOWN
> +};
> +
> +/**
> + * Get the type of hypervisor it is running on.
> + */
> +enum rte_hypervisor
> +rte_hypervisor_get_name(void);

Cc: chao...@linux.vnet.ibm.com

IMO, cpu_flag area is the not the correct abstraction to get
the hypervisor name. It is x86 specific. I think, correct
usage will be to call hypervisor specific APIs like KVM_GET_API_VERSION
https://lwn.net/Articles/658511/

BTW, What is the need for an DPDK application to know the 
hypervisor name? What action an DPDK application should
take based on hypervisor name? if is not interest of data plane
application why it needs to be abstracted in DPDK?

Re: [dpdk-dev] [PATCH 5/7] net/mrvl: add extra error logs

[Tomasz Duszynski]

On Fri, Dec 01, 2017 at 11:32:15AM +0800, Jianbo Liu wrote:
> The 11/30/2017 14:32, Tomasz Duszynski wrote:
> > Add extra error logs in a few places.
> >
> > Signed-off-by: Tomasz Duszynski 
> > ---
> >  drivers/net/mrvl/mrvl_ethdev.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
> > index 92cc283..ed97831 100644
> > --- a/drivers/net/mrvl/mrvl_ethdev.c
> > +++ b/drivers/net/mrvl/mrvl_ethdev.c
> > @@ -431,8 +431,10 @@ mrvl_dev_start(struct rte_eth_dev *dev)
> >   priv->bpool_min_size = priv->nb_rx_queues * MRVL_BURST_SIZE * 2;
> >
> >   ret = pp2_ppio_init(&priv->ppio_params, &priv->ppio);
> > - if (ret)
> > + if (ret) {
> > + RTE_LOG(ERR, PMD, "Failed to init ppio\n");
> >   return ret;
> > + }
> >
> >   /*
> >* In case there are some some stale uc/mc mac addresses flush them
> > @@ -467,8 +469,8 @@ mrvl_dev_start(struct rte_eth_dev *dev)
> >   if (mrvl_qos_cfg) {
> >   ret = mrvl_start_qos_mapping(priv);
> >   if (ret) {
> > - pp2_ppio_deinit(priv->ppio);
> > - return ret;
> > + RTE_LOG(ERR, PMD, "Failed to setup QoS mapping\n");
> > + goto out;
> >   }
> >   }
> >
>
> Do you need to print error log as well if mrvl_dev_set_link_up fails?

Right, extra log can be added for consistency.

>
> > @@ -478,6 +480,7 @@ mrvl_dev_start(struct rte_eth_dev *dev)
> >
> >   return 0;
> >  out:
> > + RTE_LOG(ERR, PMD, "Failed to start device\n");
> >   pp2_ppio_deinit(priv->ppio);
> >   return ret;
> >  }
> > --
> > 2.7.4
> >
>
> --
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.

--
- Tomasz Duszyński

[dpdk-dev] [PATCH 3/7] ethdev: separate driver APIs

[Ferruh Yigit]

Create a rte_ethdev_driver.h file and move PMD specific APIs here.
Drivers updated to include this new header file.

There is no update in header content and since ethdev.h included by
ethdev_driver.h, nothing changed from driver point of view, only
logically grouping of APIs. From applications point of view they can't
access to driver specific APIs anymore and they shouldn't.

More PMD specific data structures still remain in ethdev.h because of
inline functions in header use them. Those will be handled separately.

Signed-off-by: Ferruh Yigit 
---
 drivers/bus/dpaa/dpaa_bus.c |   2 +-
 drivers/bus/dpaa/include/fman.h |   2 +-
 drivers/bus/fslmc/fslmc_bus.c   |   2 +-
 drivers/bus/fslmc/fslmc_vfio.c  |   2 +-
 drivers/bus/fslmc/portal/dpaa2_hw_dpbp.c|   2 +-
 drivers/bus/fslmc/portal/dpaa2_hw_dpci.c|   2 +-
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c|   2 +-
 drivers/event/dpaa2/dpaa2_eventdev.c|   2 +-
 drivers/event/dpaa2/dpaa2_hw_dpcon.c|   2 +-
 drivers/event/octeontx/ssovf_evdev.c|   2 +-
 drivers/mempool/dpaa2/dpaa2_hw_mempool.c|   2 +-
 drivers/net/af_packet/rte_eth_af_packet.c   |   2 +-
 drivers/net/ark/ark_ethdev_rx.h |   2 +-
 drivers/net/ark/ark_ethdev_tx.h |   2 +-
 drivers/net/ark/ark_ext.h   |   2 +-
 drivers/net/ark/ark_global.h|   2 +-
 drivers/net/ark/ark_pktchkr.c   |   2 +-
 drivers/net/ark/ark_pktgen.c|   2 +-
 drivers/net/avp/avp_ethdev.c|   2 +-
 drivers/net/bnx2x/bnx2x_ethdev.h|   2 +-
 drivers/net/bnxt/bnxt.h |   2 +-
 drivers/net/bnxt/bnxt_ethdev.c  |   2 +-
 drivers/net/bnxt/bnxt_stats.h   |   2 +-
 drivers/net/bnxt/rte_pmd_bnxt.c |   2 +-
 drivers/net/bnxt/rte_pmd_bnxt.h |   2 +-
 drivers/net/bonding/rte_eth_bond_api.c  |   2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c  |   2 +-
 drivers/net/bonding/rte_eth_bond_private.h  |   2 +-
 drivers/net/cxgbe/base/t4_hw.c  |   2 +-
 drivers/net/cxgbe/cxgbe_ethdev.c|   2 +-
 drivers/net/cxgbe/cxgbe_main.c  |   2 +-
 drivers/net/cxgbe/sge.c |   2 +-
 drivers/net/dpaa/dpaa_ethdev.c  |   2 +-
 drivers/net/dpaa/dpaa_ethdev.h  |   2 +-
 drivers/net/dpaa/dpaa_rxtx.c|   2 +-
 drivers/net/dpaa2/base/dpaa2_hw_dpni.c  |   2 +-
 drivers/net/dpaa2/dpaa2_ethdev.c|   2 +-
 drivers/net/dpaa2/dpaa2_rxtx.c  |   2 +-
 drivers/net/e1000/em_ethdev.c   |   2 +-
 drivers/net/e1000/em_rxtx.c |   2 +-
 drivers/net/e1000/igb_ethdev.c  |   2 +-
 drivers/net/e1000/igb_flow.c|   2 +-
 drivers/net/e1000/igb_pf.c  |   2 +-
 drivers/net/e1000/igb_rxtx.c|   2 +-
 drivers/net/ena/ena_ethdev.c|   2 +-
 drivers/net/enic/enic_clsf.c|   2 +-
 drivers/net/enic/enic_ethdev.c  |   2 +-
 drivers/net/enic/enic_flow.c|   2 +-
 drivers/net/enic/enic_main.c|   2 +-
 drivers/net/enic/enic_res.c |   2 +-
 drivers/net/enic/enic_rxtx.c|   2 +-
 drivers/net/failsafe/failsafe.c |   2 +-
 drivers/net/failsafe/failsafe_ops.c |   2 +-
 drivers/net/failsafe/failsafe_private.h |   2 +-
 drivers/net/failsafe/failsafe_rxtx.c|   2 +-
 drivers/net/fm10k/fm10k_ethdev.c|   2 +-
 drivers/net/fm10k/fm10k_rxtx.c  |   2 +-
 drivers/net/fm10k/fm10k_rxtx_vec.c  |   2 +-
 drivers/net/i40e/i40e_ethdev.c  |   2 +-
 drivers/net/i40e/i40e_ethdev_vf.c   |   2 +-
 drivers/net/i40e/i40e_fdir.c|   2 +-
 drivers/net/i40e/i40e_flow.c|   2 +-
 drivers/net/i40e/i40e_pf.c  |   2 +-
 drivers/net/i40e/i40e_rxtx.c|   2 +-
 drivers/net/i40e/i40e_rxtx_vec_altivec.c|   2 +-
 drivers/net/i40e/i40e_rxtx_vec_common.h |   2 +-
 drivers/net/i40e/i40e_rxtx_vec_neon.c   |   2 +-
 drivers/net/i40e/i40e_rxtx_vec_sse.c|   2 +-
 drivers/net/i40e/rte_pmd_i40e.h |   2 +-
 drivers/net/ixgbe/ixgbe_bypass.c|   2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c|   2 +-
 drivers/net/ixgbe/ixgbe_fdir.c  |   2 +-
 drivers/net/ixgbe/ixgbe_flow.c  |   2 +-
 drivers/net/ixgbe/ixgbe_ipsec.c |   2 +-
 drivers/net/ixgbe/ixgbe_pf.c|   2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c  |   2 +-
 drivers/net/ixgbe/ixgbe_rxtx_vec_c

[dpdk-dev] [PATCH v2 2/2] Change root makefile license to SPDX tag

[Hemant Agrawal]

Signed-off-by: Hemant Agrawal 
---
 GNUmakefile | 28 +---
 Makefile| 28 +---
 2 files changed, 2 insertions(+), 54 deletions(-)

diff --git a/GNUmakefile b/GNUmakefile
index 45b7fbb..5b07b61 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -1,33 +1,7 @@
-#   BSD LICENSE
 #
 #   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
-#   All rights reserved.
 #
-#   Redistribution and use in source and binary forms, with or without
-#   modification, are permitted provided that the following conditions
-#   are met:
-#
-# * Redistributions of source code must retain the above copyright
-#   notice, this list of conditions and the following disclaimer.
-# * Redistributions in binary form must reproduce the above copyright
-#   notice, this list of conditions and the following disclaimer in
-#   the documentation and/or other materials provided with the
-#   distribution.
-# * Neither the name of Intel Corporation nor the names of its
-#   contributors may be used to endorse or promote products derived
-#   from this software without specific prior written permission.
-#
-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#   SPDX-License-Identifier:BSD-3-Clause
 
 #
 # Head Makefile for compiling rte SDK
diff --git a/Makefile b/Makefile
index f4b807e..a58b364 100644
--- a/Makefile
+++ b/Makefile
@@ -1,33 +1,7 @@
-#   BSD LICENSE
 #
 #   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
-#   All rights reserved.
 #
-#   Redistribution and use in source and binary forms, with or without
-#   modification, are permitted provided that the following conditions
-#   are met:
-#
-# * Redistributions of source code must retain the above copyright
-#   notice, this list of conditions and the following disclaimer.
-# * Redistributions in binary form must reproduce the above copyright
-#   notice, this list of conditions and the following disclaimer in
-#   the documentation and/or other materials provided with the
-#   distribution.
-# * Neither the name of Intel Corporation nor the names of its
-#   contributors may be used to endorse or promote products derived
-#   from this software without specific prior written permission.
-#
-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#   SPDX-License-Identifier:BSD-3-Clause
 
 .error Error please compile using GNU Make (gmake)
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 1/2] Introducing SPDX License Identifiers

[Hemant Agrawal]

The DPDK uses the Open Source BSD-3-Clause license for the core libraries
and drivers. The kernel components are naturally GPLv2 licensed.

Many of the files in the DPDK source code contain the full text of the
applicable license. For example, most of the BSD-3-Clause files contain a
full copy of the BSD-3-Clause license text.

Including big blocks of License headers in all files blows up the source
code with mostly redundant information.  An additional problem is that even
the same licenses are referred to by a number of slightly varying text
blocks (full, abbreviated, different indentation, line wrapping and/or
white space, with obsolete address information, ...) which makes validation
and automatic processing a nightmare.

To make this easier, DPDK is adpoting the use of a single line reference to
Unique License Identifiers in source files as defined by the Linux
Foundation's SPDX project [1].

Adding license information in this fashion, rather than adding full license
text, can be more efficient for developers; decreases errors; and improves
automated detection of licenses. The current set of valid, predefined SPDX
identifiers is set forth on the SPDX License List[2]
at https://spdx.org/licenses/.

For example, to label a file as subject to the BSD-3-Clause license,
the following text would be used:

Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
SPDX-License-Identifier:BSD-3-Clause

To label a file as GPL-2.0 (e.g., for code that runs in the kernel), the
following text would be used:

Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
SPDX-License-Identifier:GPL-2.0

To label a file as dual-licensed with BSD-3-Clause and GPL-2.0 (e.g., for
code that is shared between the kernel and userspace), the following text
would be used:

Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
SPDX-License-Identifier:BSD-3-Clause OR GPL-2.0

To label a file as dual-licensed with BSD-3-Clause and LGPL-2.1 (e.g., for
code that is shared between the kernel and userspace), the following text
would be used:

Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
SPDX-License-Identifier:BSD-3-Clause OR LGPL-2.1

Note: Any new file contributions in DPDK shall adhere to the above scheme.
It is also being recommended to replace the existing license text in the
code with SPDX-License-Identifiers.

Note 2: DPDK currently adhere to it's IP policies[3]. Any exception to this
shall be approved by DPDK tech board and DPDK Governing Board. Steps for
any exception approval:
1. Mention the appropriate license identifier form SPDX. If the license is
   not listed in SPDX Licenses. It is the submitters responsibiliity to get
   it first listed.
2. Get the required approval from the DPDK Technical Board. Technical board
   may advise the author to check alternate means first. If no other
   alternatives are found and the merit of the contributions are important
   for DPDK's mission, it may decide on such exception with two-thirds vote
   of the members.
3. Technical board then approach Governing board for such limited approval
   for the given contribution only.

Any approvals shall be documented in "Licenses/exceptions.txt" with record
dates.

Note 3: Projects like U-boot have been been using SPDX License Idenfiers
successfully [2]. They have been referered in implementing SPDX based
guidelines in DPDK.

Note 4: From the legal point of view, this patch is supposed to be only a
change to the textual representation of the license information, but in no
way any change to the actual license terms. With this patch applied, all
files will still be licensed under the same terms they were before.

Signed-off-by: Hemant Agrawal 
Acked-by: Stephen Hemminger 
---
 LICENSE.GPL | 339 
 LICENSE.LGPL| 502 
 Licenses/Exceptions.txt |  12 +
 Licenses/README |  82 ++
 Licenses/bsd-3-clause.txt   |   9 +
 Licenses/gpl-2.0.txt| 339 
 Licenses/lgpl-2.1.txt   | 502 
 README  |   5 +-
 doc/guides/contributing/patches.rst |  36 +++
 9 files changed, 983 insertions(+), 843 deletions(-)
 delete mode 100644 LICENSE.GPL
 delete mode 100644 LICENSE.LGPL
 create mode 100644 Licenses/Exceptions.txt
 create mode 100644 Licenses/README
 create mode 100644 Licenses/bsd-3-clause.txt
 create mode 100644 Licenses/gpl-2.0.txt
 create mode 100644 Licenses/lgpl-2.1.txt

diff --git a/LICENSE.GPL b/LICENSE.GPL
deleted file mode 100644
index d511905..000
--- a/LICENSE.GPL
+++ /dev/null
@@ -1,339 +0,0 @@
-   GNU GENERAL PUBLIC LICENSE
-  Version 2, June 1991
-
- Copyright (C) 1989, 1991 Free Software Foundation, Inc.,
- 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- Everyone is permitted to copy and distribute verbatim copies
- of this license documen

Re: [dpdk-dev] [PATCH 2/2] Change root makefile license to SPDX tag

[Hemant Agrawal]

On 11/27/2017 5:31 PM, Bruce Richardson wrote:

On Mon, Nov 27, 2017 at 01:16:04PM +0530, Hemant Agrawal wrote:

Signed-off-by: Hemant Agrawal 
---
 GNUmakefile | 27 +--
 Makefile| 27 +--
 2 files changed, 2 insertions(+), 52 deletions(-)

diff --git a/GNUmakefile b/GNUmakefile
index 45b7fbb..2602531 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -1,33 +1,8 @@
-#   BSD LICENSE
 #
 #   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
 #   All rights reserved.
 #


As part of this cleanup, I think it might be also useful to remove the
duplicate "All rights reserved" on the files. There is surely no need to
have the statement duplicated.


Done.
BTW, this patch will need an ACK from Intel :)


/Bruce

Re: [dpdk-dev] [PATCH] bus/vdev: add custom scan hook

[Thomas Monjalon]

Hi,

01/12/2017 06:48, Tan, Jianfeng:
> Hi Thomas,
> 
> Please help us to understand why we need this.
> 
> 
> On 12/1/2017 8:36 AM, Thomas Monjalon wrote:
> > The scan callback allows to spawn a vdev automatically
> > given some custom scan rules.
> 
> These two new APIs (rte_vdev_add_custom_scan and 
> rte_vdev_remove_custom_scan) are called by applications?

Yes, the application can use it but it can also be used by a DPDK
driver or library.

> If so, why not just constructing them in the parameters before passing 
> to rte_eal_init?

It is not only for initialization because it will also work when
we will have some hotplug mechanism where the scan is run during run-time.

> > It is especially useful to create a TAP device automatically
> > connected to a netdevice as remote.
> 
> It sounds like an use case. Without this patch, I suppose we can already 
> do this?

Yes we can already update the devargs list.
But this hook will help to do it on hotplug events.

Re: [dpdk-dev] [PATCH] eal/x86: get hypervisor name

[Thomas Monjalon]

01/12/2017 09:12, Jerin Jacob:
> -Original Message-
> > Date: Thu, 30 Nov 2017 22:47:20 +0100
> > From: Thomas Monjalon 
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> > X-Mailer: git-send-email 2.15.0
> > 
> > The CPUID instruction is catched by hypervisor which can return
> > a flag indicating one is running, and its name.
> > 
> > Suggested-by: Stephen Hemminger 
> > Signed-off-by: Thomas Monjalon 
> > ---
> > warning: to be tested
> > ---
> >  lib/librte_eal/common/arch/arm/rte_cpuflags.c  |  6 +
> >  lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +
> >  lib/librte_eal/common/arch/x86/rte_cpuflags.c  | 30 
> > ++
> >  .../common/include/arch/x86/rte_cpuflags.h |  1 +
> >  .../common/include/generic/rte_cpuflags.h  | 14 ++
> >  lib/librte_eal/rte_eal_version.map |  9 ++-
> >  6 files changed, 65 insertions(+), 1 deletion(-)
> > RTE_CPUFLAG_FPU,/**< FPU */
> > diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h 
> > b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > index c1c5551fc..3832fb851 100644
> > --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > @@ -93,4 +93,18 @@ rte_cpu_check_supported(void);
> >  int
> >  rte_cpu_is_supported(void);
> >  
> > +enum rte_hypervisor {
> > +   RTE_HYPERVISOR_NONE,
> > +   RTE_HYPERVISOR_KVM,
> > +   RTE_HYPERVISOR_HYPERV,
> > +   RTE_HYPERVISOR_VMWARE,
> > +   RTE_HYPERVISOR_UNKNOWN
> > +};
> > +
> > +/**
> > + * Get the type of hypervisor it is running on.
> > + */
> > +enum rte_hypervisor
> > +rte_hypervisor_get_name(void);
> 
> Cc: chao...@linux.vnet.ibm.com
> 
> IMO, cpu_flag area is the not the correct abstraction to get
> the hypervisor name. It is x86 specific. I think, correct
> usage will be to call hypervisor specific APIs like KVM_GET_API_VERSION
> https://lwn.net/Articles/658511/

I think it is quite logical because the CPU is virtualized by the hypervisor.
It is similar to get the CPU model, but for a different ring level.

> BTW, What is the need for an DPDK application to know the 
> hypervisor name? What action an DPDK application should
> take based on hypervisor name? if is not interest of data plane
> application why it needs to be abstracted in DPDK?

I see two usages for now.
We can automate the use of the specific VMware TSC (it is an option currently).
We can adapt the device management policy on Hyper-V without waiting a device 
scan.

[dpdk-dev] [PATCH] net/i40e: add fdir nvgre parameters check

[Wei Zhao]

Add mask parameters check in nvgre parser for flow API.

Fixes: 30965ca341278 ("net/i40e: add NVGRE flow parsing")

Signed-off-by: Wei Zhao 
---
 drivers/net/i40e/i40e_flow.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 7e4936e..05e54f1 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -3610,6 +3610,41 @@ i40e_flow_parse_nvgre_pattern(__rte_unused struct 
rte_eth_dev *dev,
   "Invalid TNI mask");
return -rte_errno;
}
+   if (nvgre_mask->protocol &&
+   nvgre_mask->protocol != 0x) {
+   rte_flow_error_set(error, EINVAL,
+   RTE_FLOW_ERROR_TYPE_ITEM,
+   item,
+   "Invalid NVGRE item");
+   return -rte_errno;
+   }
+   if (nvgre_mask->c_k_s_rsvd0_ver &&
+   nvgre_mask->c_k_s_rsvd0_ver !=
+   rte_cpu_to_be_16(0x3000)) {
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM,
+  item,
+  "Invalid NVGRE item");
+   return -rte_errno;
+   }
+   if (nvgre_spec->c_k_s_rsvd0_ver !=
+   rte_cpu_to_be_16(0x2000) &&
+   nvgre_mask->c_k_s_rsvd0_ver) {
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM,
+  item,
+  "Invalid NVGRE item");
+   return -rte_errno;
+   }
+   if (nvgre_mask->protocol &&
+   nvgre_spec->protocol !=
+   rte_cpu_to_be_16(0x6558)) {
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM,
+  item,
+  "Invalid NVGRE item");
+   return -rte_errno;
+   }
rte_memcpy(((uint8_t *)&tenant_id_be + 1),
   nvgre_spec->tni, 3);
filter->tenant_id =
-- 
2.9.3

Re: [dpdk-dev] [PATCH 3/7] ethdev: separate driver APIs

[Hemant Agrawal]

On 12/1/2017 7:59 AM, Ferruh Yigit wrote:

...

diff --git a/lib/librte_ether/rte_ethdev_driver.h 
b/lib/librte_ether/rte_ethdev_driver.h
new file mode 100644
index 0..3e77d1439
--- /dev/null
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -0,0 +1,163 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *   All rights reserved.


You can remove one of the all rights reserved.
This is also an issue in your next patch for rte_ethdev_core.h

Also, as Shreyansh mentioned, Why not start with SPDX tags instead of 
full license text?


...

+/**
+ * @internal Executes all the user application registered callbacks for
+ * the specific device. It is for DPDK internal user only. User
+ * application should not call it directly.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ * @param event
+ *  Eth device interrupt event type.
+ * @param cb_arg
+ *  callback parameter.
+ * @param ret_param
+ *  To pass data back to user application.
+ *  This allows the user application to decide if a particular function
+ *  is permitted or not.
+ *
+ * @return
+ *  int
+ */
+int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
+   enum rte_eth_event_type event, void *cb_arg, void *ret_param);
+
+/**
+ * Create memzone for HW rings.


Like all other functions, you can also add "@internal" for this as well.

Acked-by: Hemant Agrawal 

Re: [dpdk-dev] [PATCH 2/7] ethdev: fix port id storage

[Hemant Agrawal]

On 12/1/2017 7:59 AM, Ferruh Yigit wrote:

port_id is now 16bits, update function parameter according.

Fixes: 4c270218aa26 ("ethdev: support security APIs")
Cc: sta...@dpdk.org
Cc: declan.dohe...@intel.com

Signed-off-by: Ferruh Yigit 
---
Cc: Boris Pismenny 
Cc: Aviad Yehezkel 
Cc: Radu Nicolau 
Cc: Declan Doherty 
---
 lib/librte_ether/rte_ethdev.c | 2 +-
 lib/librte_ether/rte_ethdev.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 318af2869..eed3d3005 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -301,7 +301,7 @@ rte_eth_dev_socket_id(uint16_t port_id)
 }

 void *
-rte_eth_dev_get_sec_ctx(uint8_t port_id)
+rte_eth_dev_get_sec_ctx(uint16_t port_id)
 {
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, NULL);
return rte_eth_devices[port_id].security_ctx;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index e620c3706..59189096e 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1745,7 +1745,7 @@ struct rte_eth_dev {
 } __rte_cache_aligned;

 void *
-rte_eth_dev_get_sec_ctx(uint8_t port_id);
+rte_eth_dev_get_sec_ctx(uint16_t port_id);

 struct rte_eth_dev_sriov {
uint8_t active;   /**< SRIOV is active with 16, 32 or 64 
pools */


Acked-by: Hemant Agrawal 

Re: [dpdk-dev] [PATCH 1/7] net/mrvl: sync compilation with musdk-17.10

[Tomasz Duszynski]

On Fri, Dec 01, 2017 at 11:29:07AM +0800, Jianbo Liu wrote:
> The 11/30/2017 14:32, Tomasz Duszynski wrote:
> > Followig changes are needed to switch to musdk-17.10:
> >
> > - With a new version of the musdk library it's no longer necessary to
> >   explicitly define MVCONF_ARCH_DMA_ADDR_T_64BIT and
> >   CONF_PP2_BPOOL_COOKIE_SIZE.
> >
> >   Proper defines are autogenerated by ./configure script based on
> >   passed options and available after mv_autogen_comp_flags.h inclusion.
> >
> > - API used to set promiscuous mode was renamed. Thus in order to
> >   compile against the latest library new API must be used.
> >
> > Signed-off-by: Tomasz Duszynski 
> > ---
> >  drivers/net/mrvl/Makefile  | 4 ++--
> >  drivers/net/mrvl/mrvl_ethdev.c | 5 +++--
> >  drivers/net/mrvl/mrvl_ethdev.h | 1 +
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/mrvl/Makefile b/drivers/net/mrvl/Makefile
> > index 815c3ba..f75e53c 100644
> > --- a/drivers/net/mrvl/Makefile
> > +++ b/drivers/net/mrvl/Makefile
> > @@ -51,8 +51,8 @@ EXPORT_MAP := rte_pmd_mrvl_version.map
> >
> >  # external library dependencies
> >  CFLAGS += -I$(LIBMUSDK_PATH)/include
> > -CFLAGS += -DMVCONF_ARCH_DMA_ADDR_T_64BIT
> > -CFLAGS += -DCONF_PP2_BPOOL_COOKIE_SIZE=32
> > +CFLAGS += -DMVCONF_TYPES_PUBLIC
> > +CFLAGS += -DMVCONF_DMA_PHYS_ADDR_T_PUBLIC
> >  CFLAGS += $(WERROR_FLAGS)
> >  CFLAGS += -O3
> >  LDLIBS += -L$(LIBMUSDK_PATH)/lib
> > diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
> > index 2936165..4fac797 100644
> > --- a/drivers/net/mrvl/mrvl_ethdev.c
> > +++ b/drivers/net/mrvl/mrvl_ethdev.c
> > @@ -47,6 +47,7 @@
> >  #undef container_of
> >  #endif
> >
> > +#include 
>
> Is it needed as you also included this file in mrvl_ethdev.h?
> I think you can move all the MUSDK headers to mrvl_ethdev.h to avoid the
> duplication.

It's needed here as well because it needs to be included before other MUSDK
includes. So either mrvl_ethdev.h can be moved a little bit
or all MUSDK related headers can go to mrvl_ethdev.h as you suggest.

The latter option seems to be better choice though.

>
> >  #include 
> >  #include 
> >  #include 
> > @@ -690,7 +691,7 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev)
> >   struct mrvl_priv *priv = dev->data->dev_private;
> >   int ret;
> >
> > - ret = pp2_ppio_set_uc_promisc(priv->ppio, 1);
> > + ret = pp2_ppio_set_promisc(priv->ppio, 1);
> >   if (ret)
> >   RTE_LOG(ERR, PMD, "Failed to enable promiscuous mode\n");
> >  }
> > @@ -724,7 +725,7 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev)
> >   struct mrvl_priv *priv = dev->data->dev_private;
> >   int ret;
> >
> > - ret = pp2_ppio_set_uc_promisc(priv->ppio, 0);
> > + ret = pp2_ppio_set_promisc(priv->ppio, 0);
> >   if (ret)
> >   RTE_LOG(ERR, PMD, "Failed to disable promiscuous mode\n");
> >  }
> > diff --git a/drivers/net/mrvl/mrvl_ethdev.h b/drivers/net/mrvl/mrvl_ethdev.h
> > index 2a4ab5a..252e7a3 100644
> > --- a/drivers/net/mrvl/mrvl_ethdev.h
> > +++ b/drivers/net/mrvl/mrvl_ethdev.h
> > @@ -36,6 +36,7 @@
> >  #define _MRVL_ETHDEV_H_
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
> > --
> > 2.7.4
> >
>
> --
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.

--
- Tomasz Duszyński

Re: [dpdk-dev] [PATCH] eal/x86: get hypervisor name

[Jerin Jacob]

-Original Message-
> Date: Fri, 01 Dec 2017 09:52:26 +0100
> From: Thomas Monjalon 
> To: Jerin Jacob 
> Cc: dev@dpdk.org, chao...@linux.vnet.ibm.com
> Subject: Re: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> 
> 01/12/2017 09:12, Jerin Jacob:
> > -Original Message-
> > > Date: Thu, 30 Nov 2017 22:47:20 +0100
> > > From: Thomas Monjalon 
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] eal/x86: get hypervisor name
> > > X-Mailer: git-send-email 2.15.0
> > > 
> > > The CPUID instruction is catched by hypervisor which can return
> > > a flag indicating one is running, and its name.
> > > 
> > > Suggested-by: Stephen Hemminger 
> > > Signed-off-by: Thomas Monjalon 
> > > ---
> > > warning: to be tested
> > > ---
> > >  lib/librte_eal/common/arch/arm/rte_cpuflags.c  |  6 +
> > >  lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c   |  6 +
> > >  lib/librte_eal/common/arch/x86/rte_cpuflags.c  | 30 
> > > ++
> > >  .../common/include/arch/x86/rte_cpuflags.h |  1 +
> > >  .../common/include/generic/rte_cpuflags.h  | 14 ++
> > >  lib/librte_eal/rte_eal_version.map |  9 ++-
> > >  6 files changed, 65 insertions(+), 1 deletion(-)
> > >   RTE_CPUFLAG_FPU,/**< FPU */
> > > diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h 
> > > b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > > index c1c5551fc..3832fb851 100644
> > > --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > > +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> > > @@ -93,4 +93,18 @@ rte_cpu_check_supported(void);
> > >  int
> > >  rte_cpu_is_supported(void);
> > >  
> > > +enum rte_hypervisor {
> > > + RTE_HYPERVISOR_NONE,
> > > + RTE_HYPERVISOR_KVM,
> > > + RTE_HYPERVISOR_HYPERV,
> > > + RTE_HYPERVISOR_VMWARE,
> > > + RTE_HYPERVISOR_UNKNOWN
> > > +};
> > > +
> > > +/**
> > > + * Get the type of hypervisor it is running on.
> > > + */
> > > +enum rte_hypervisor
> > > +rte_hypervisor_get_name(void);
> > 
> > Cc: chao...@linux.vnet.ibm.com
> > 
> > IMO, cpu_flag area is the not the correct abstraction to get
> > the hypervisor name. It is x86 specific. I think, correct
> > usage will be to call hypervisor specific APIs like KVM_GET_API_VERSION
> > https://lwn.net/Articles/658511/
> 
> I think it is quite logical because the CPU is virtualized by the hypervisor.
> It is similar to get the CPU model, but for a different ring level.

At least on non x86, it is not exposed as CPU flags. IMO,
*hypervisor*.h file makes more sense rather than rte_cpuflags.h.

> 
> > BTW, What is the need for an DPDK application to know the 
> > hypervisor name? What action an DPDK application should
> > take based on hypervisor name? if is not interest of data plane
> > application why it needs to be abstracted in DPDK?
> 
> I see two usages for now.
> We can automate the use of the specific VMware TSC (it is an option 
> currently).

In my view this use case needed by the DPDK "implementation" not the
"application". So internal API makes more sense to me.

> We can adapt the device management policy on Hyper-V without waiting a device 
> scan.

I am not sure about this? Looks like a candidate for library implementation
not for the "application"

Re: [dpdk-dev] [PATCH] [RFC] ether: standardize getting the port by name

[Gaëtan Rivet]

On Thu, Nov 30, 2017 at 10:44:58PM +0100, Thomas Monjalon wrote:
> 30/11/2017 22:21, Stephen Hemminger:
> > On Thu, 30 Nov 2017 18:35:11 +0100
> > Thomas Monjalon  wrote:
> > 
> > > 30/11/2017 18:15, Stephen Hemminger:
> > > > Some thoughts.
> > > > 1) Not all devices are PCI; look at recent VMBUS  
> > > 
> > > Yes, we need a syntax which works for every devices.
> > > I suggest to use the prefix "pci:" before the PCI id.
> > > We need also a prefix and ids for NXP buses.
> > > We could use "vmbus:" before VMBUS ids.
> > > How VMBUS ids look like?
> > > 

rte_devargs are easily accessible, user-readable. Only thing missing
would be requiring a 1-1 mapping between an rte_devargs and a port, thus
requiring PMDs to have at least one version of a device string that
would probe a single port (as is done with port= in mlx4).

Implementing an rte_devargs to rte_device in rte_bus is simple enough,
and this would allow implementing an rte_devargs to port_id in rte_eth.

What am I missing?

> > > > 2) The name may have to be set before MAC address is determined on 
> > > > boot.  
> > > 
> > > I don't understand this comment.
> > > Do you mean MAC may be unknown when starting DPDK?
> > 
> > The MAC be known by the hardware, but the device would have to be
> > created before using  hardware to read it.
> 
> Indeed, it is a problem if we want to use this syntax for blacklist.
> 
> 
> > > > 3) The names themselves are not persistent or human friendly. This is 
> > > > hard
> > > >see the effort udev goes to.  
> > > 
> > > Yes udev has a syntax to identify devices. It can be inspiring.
> > > Qemu may also be inspiring:
> > >   https://github.com/qemu/qemu/blob/master/docs/qdev-device-use.txt
> 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH] contigmem: include to fix FreeBSD build

[Bruce Richardson]

On Thu, Nov 30, 2017 at 02:00:38PM +0800, kefu chai wrote:
> otherwise the build fails with
> 
> In file included from contigmem.c:57:
> /usr/srcs/head/src/sys/vm/vm_phys.h:122:10: error: use of undeclared
> identifier 'vm_cnt'
> return (vm_cnt.v_free_count += adj);
> ^
> 1 error generated.
> *** Error code 1
> 
> Signed-off-by: Kefu Chai 
> ---

The commit log could do with being updated to note that the error occurs
with, currently unreleased, FreeBSD 12  Otherwise:

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object

[Bruce Richardson]

On Fri, Dec 01, 2017 at 02:29:57AM +, Ferruh Yigit wrote:
> "struct rte_eth_rxtx_callback" is defined as internal data structure but
> used in public APIs.
> 
> Checking the API documentation shows that intention was using this
> object as opaque object. Data structure only used in delete APIs which
> doesn't require to know the internals of the data structure.
> 
> Converting callback parameter in API to void pointer should not require
> any modification in user application because this data structure was
> already marked as internal and only should be used as pointer in
> application.
> 
> Signed-off-by: Ferruh Yigit 
> ---

I disagree on this patch. The structure itself is not exposed, only the
name, since it is only passed around as a pointer, so there is no need
to change the parameters to void pointer. It's a named opaque type.

[dpdk-dev] [PATCH v2 0/2] ethdev: add GENEVE to flow API

[Andrew Rybchenko]

v2:
 - add after ESP to avoid ABI breakage
 - make default mask to include VNI only
 - minor style fixes

Roman Zhukov (2):
  ethdev: add GENEVE flow pattern item
  app/testpmd: support GENEVE pattern item in flow rules

 app/test-pmd/cmdline_flow.c | 31 +
 app/test-pmd/config.c   |  1 +
 doc/guides/prog_guide/rte_flow.rst  | 12 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  5 +
 lib/librte_ether/rte_flow.c |  1 +
 lib/librte_ether/rte_flow.h | 30 
 6 files changed, 80 insertions(+)

-- 
2.7.4

[dpdk-dev] [PATCH v2 1/2] ethdev: add GENEVE flow pattern item

[Andrew Rybchenko]

From: Roman Zhukov 

Add new pattern item RTE_FLOW_ITEM_TYPE_GENEVE in flow API.
Add default mask for the item.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
---
 doc/guides/prog_guide/rte_flow.rst | 12 
 lib/librte_ether/rte_flow.c|  1 +
 lib/librte_ether/rte_flow.h| 30 ++
 3 files changed, 43 insertions(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index d158be5..5b8f9c5 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -980,6 +980,18 @@ Matches an ESP header.
 - ``hdr``: ESP header definition (``rte_esp.h``).
 - Default ``mask`` matches SPI only.
 
+Item: ``GENEVE``
+^^^
+
+Matches a GENEVE header.
+
+- ``ver_opt_len_o_c_rsvd0``: version (2b), length of the options fields (6b),
+  OAM packet (1b), critical options present (1b), reserved 0 (6b).
+- ``protocol``: protocol type.
+- ``vni``: virtual network identifier.
+- ``rsvd1``: reserved, normally 0x00.
+- Default ``mask`` matches VNI only.
+
 Actions
 ~~~
 
diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
index 6659063..913d1a5 100644
--- a/lib/librte_ether/rte_flow.c
+++ b/lib/librte_ether/rte_flow.c
@@ -81,6 +81,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = 
{
MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
MK_FLOW_ITEM(E_TAG, sizeof(struct rte_flow_item_e_tag)),
MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
+   MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)),
 };
 
 /** Generate flow_action[] entry. */
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 47c88ea..e0402cf 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -344,6 +344,13 @@ enum rte_flow_item_type {
 * See struct rte_flow_item_esp.
 */
RTE_FLOW_ITEM_TYPE_ESP,
+
+   /**
+* Matches a GENEVE header.
+*
+* See struct rte_flow_item_geneve.
+*/
+   RTE_FLOW_ITEM_TYPE_GENEVE,
 };
 
 /**
@@ -813,6 +820,29 @@ static const struct rte_flow_item_esp 
rte_flow_item_esp_mask = {
 #endif
 
 /**
+ * RTE_FLOW_ITEM_TYPE_GENEVE.
+ *
+ * Matches a GENEVE header.
+ */
+struct rte_flow_item_geneve {
+   /**
+* Version (2b), length of the options fields (6b), OAM packet (1b),
+* critical options present (1b), reserved 0 (6b).
+*/
+   rte_be16_t ver_opt_len_o_c_rsvd0;
+   rte_be16_t protocol; /**< Protocol type. */
+   uint8_t vni[3]; /**< Virtual Network Identifier. */
+   uint8_t rsvd1; /**< Reserved, normally 0x00. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_GENEVE. */
+#ifndef __cplusplus
+static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = {
+   .vni = "\xff\xff\xff",
+};
+#endif
+
+/**
  * Matching pattern item definition.
  *
  * A pattern is formed by stacking items starting from the lowest protocol
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/2] app/testpmd: support GENEVE pattern item in flow rules

[Andrew Rybchenko]

From: Roman Zhukov 

Add the ability to match VNI and protocol fields of GENEVE protocol header.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
---
 app/test-pmd/cmdline_flow.c | 31 +
 app/test-pmd/config.c   |  1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  5 +
 3 files changed, 37 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index df16d2a..561e057 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -175,6 +175,9 @@ enum index {
ITEM_GTP_TEID,
ITEM_GTPC,
ITEM_GTPU,
+   ITEM_GENEVE,
+   ITEM_GENEVE_VNI,
+   ITEM_GENEVE_PROTO,
 
/* Validate/create actions. */
ACTIONS,
@@ -460,6 +463,7 @@ static const enum index next_item[] = {
ITEM_GTP,
ITEM_GTPC,
ITEM_GTPU,
+   ITEM_GENEVE,
ZERO,
 };
 
@@ -603,6 +607,13 @@ static const enum index item_gtp[] = {
ZERO,
 };
 
+static const enum index item_geneve[] = {
+   ITEM_GENEVE_VNI,
+   ITEM_GENEVE_PROTO,
+   ITEM_NEXT,
+   ZERO,
+};
+
 static const enum index next_action[] = {
ACTION_END,
ACTION_VOID,
@@ -1470,6 +1481,26 @@ static const struct token token_list[] = {
.next = NEXT(item_gtp),
.call = parse_vc,
},
+   [ITEM_GENEVE] = {
+   .name = "geneve",
+   .help = "match GENEVE header",
+   .priv = PRIV_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)),
+   .next = NEXT(item_geneve),
+   .call = parse_vc,
+   },
+   [ITEM_GENEVE_VNI] = {
+   .name = "vni",
+   .help = "virtual network identifier",
+   .next = NEXT(item_geneve, NEXT_ENTRY(UNSIGNED), item_param),
+   .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_geneve, vni)),
+   },
+   [ITEM_GENEVE_PROTO] = {
+   .name = "protocol",
+   .help = "GENEVE protocol type",
+   .next = NEXT(item_geneve, NEXT_ENTRY(UNSIGNED), item_param),
+   .args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_geneve,
+protocol)),
+   },
 
/* Validate/create actions. */
[ACTIONS] = {
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index cd2ac11..86ca3aa 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -973,6 +973,7 @@ static const struct {
MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
MK_FLOW_ITEM(GTPU, sizeof(struct rte_flow_item_gtp)),
+   MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)),
 };
 
 /** Compute storage space needed by item specification. */
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 9789139..2b00be8 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3107,6 +3107,11 @@ This section lists supported pattern items and their 
attributes, if any.
 
   - ``teid {unsigned}``: tunnel endpoint identifier.
 
+- ``geneve``: match GENEVE header.
+
+  - ``vni {unsigned}``: virtual network identifier.
+  - ``protocol {unsigned}``: protocol type.
+
 Actions list
 
 
-- 
2.7.4

[dpdk-dev] [PATCH] maintainers: claim responsibility for virtio PMD

[Tiwei Bie]

Add myself as co-maintainer for virtio driver.

Signed-off-by: Tiwei Bie 
---
Hi Yuanhan and Maxime,

As there will be more and more virtio related stuffs (e.g. vhost-pci,
virtio crypto, vhost crypto) in DPDK, more maintaining efforts will be
needed for virtio/vhost. So I volunteer to co-maintain virtio driver.
Hope this could help!

Best regards,
Tiwei Bie

 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f0baeb423..0627aa345 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -528,6 +528,7 @@ F: doc/guides/nics/features/vhost.ini
 Virtio PMD
 M: Yuanhan Liu 
 M: Maxime Coquelin 
+M: Tiwei Bie 
 T: git://dpdk.org/next/dpdk-next-virtio
 F: drivers/net/virtio/
 F: doc/guides/nics/virtio.rst
-- 
2.13.3

[dpdk-dev] [PATCH 2/2] eal/x86: Use lock-prefixed instructions to reduce cost of rte_smp_mb()

[Konstantin Ananyev]

On x86 it  is possible to use lock-prefixed instructions to get
the similar effect as mfence.
As pointed by Java guys, on most modern HW that gives a better
performance than using mfence:
https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
That patch adopts that technique for rte_smp_mb() implementation.
On BDW 2.2 mb_autotest on single lcore reports 2X cycle reduction,
i.e. from ~110 to ~55 cycles per operation.

Signed-off-by: Konstantin Ananyev 
---
 .../common/include/arch/x86/rte_atomic.h   | 45 +-
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h 
b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
index 4eac66631..07b7fa7f7 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
@@ -55,12 +55,53 @@ extern "C" {
 
 #definerte_rmb() _mm_lfence()
 
-#define rte_smp_mb() rte_mb()
-
 #define rte_smp_wmb() rte_compiler_barrier()
 
 #define rte_smp_rmb() rte_compiler_barrier()
 
+/*
+ * From Intel Software Development Manual; Vol 3;
+ * 8.2.2 Memory Ordering in P6 and More Recent Processor Families:
+ * ...
+ * . Reads are not reordered with other reads.
+ * . Writes are not reordered with older reads.
+ * . Writes to memory are not reordered with other writes,
+ *   with the following exceptions:
+ *   . streaming stores (writes) executed with the non-temporal move
+ * instructions (MOVNTI, MOVNTQ, MOVNTDQ, MOVNTPS, and MOVNTPD); and
+ *   . string operations (see Section 8.2.4.1).
+ *  ...
+ * . Reads may be reordered with older writes to different locations but not
+ * with older writes to the same location.
+ * . Reads or writes cannot be reordered with I/O instructions,
+ * locked instructions, or serializing instructions.
+ * . Reads cannot pass earlier LFENCE and MFENCE instructions.
+ * . Writes ... cannot pass earlier LFENCE, SFENCE, and MFENCE instructions.
+ * . LFENCE instructions cannot pass earlier reads.
+ * . SFENCE instructions cannot pass earlier writes ...
+ * . MFENCE instructions cannot pass earlier reads, writes ...
+ *
+ * As pointed by Java guys, that makes possible to use lock-prefixed
+ * instructions to get the same effect as mfence and on most modern HW
+ * that gives a better perfomarnce than using mfence:
+ * https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
+ * So below we use that technique for rte_smp_mb() implementation.
+ */
+
+#ifdef RTE_ARCH_I686
+#defineRTE_SP  RTE_STR(esp)
+#else
+#defineRTE_SP  RTE_STR(rsp)
+#endif
+
+#define RTE_MB_DUMMY_MEMP  "-128(%%" RTE_SP ")"
+
+static __rte_always_inline void
+rte_smp_mb(void)
+{
+   asm volatile("lock addl $0," RTE_MB_DUMMY_MEMP "; " ::: "memory");
+}
+
 #define rte_io_mb() rte_mb()
 
 #define rte_io_wmb() rte_compiler_barrier()
-- 
2.13.5

[dpdk-dev] [PATCH 1/2] test/test: introduce new test-case for rte_smp_mb()

[Konstantin Ananyev]

Simple functional test for rte_smp_mb() implementations.
Also when executed on a single lcore could be used as rough
estimation how many cycles particular implementation of rte_smp_mb()
might take.

Signed-off-by: Konstantin Ananyev 
---
 test/test/Makefile  |   1 +
 test/test/test_mb.c | 278 
 2 files changed, 279 insertions(+)
 create mode 100644 test/test/test_mb.c

diff --git a/test/test/Makefile b/test/test/Makefile
index bb54c9808..3134283a8 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -95,6 +95,7 @@ SRCS-y += test_spinlock.c
 SRCS-y += test_memory.c
 SRCS-y += test_memzone.c
 SRCS-y += test_bitmap.c
+SRCS-y += test_mb.c
 
 SRCS-y += test_ring.c
 SRCS-y += test_ring_perf.c
diff --git a/test/test/test_mb.c b/test/test/test_mb.c
new file mode 100644
index 0..5526a0366
--- /dev/null
+++ b/test/test/test_mb.c
@@ -0,0 +1,278 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+ /*
+  * This is a simple functional test for rte_smp_mb() implementation.
+  * I.E. make sure that LOAD and STORE operations that precede the
+  * rte_smp_mb() call are globally visible across the lcores
+  * before the the LOAD and STORE operations that follows it.
+  * The test uses simple implementation of Peterson's lock algorithm
+  * for two execution units to make sure that rte_smp_mb() prevents
+  * store-load reordering to happen.
+  * Also when executed on a single lcore could be used as a approxiamate
+  * estimation of number of cycles particular implementation of rte_smp_mb()
+  * will take.
+  */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "test.h"
+
+#define ADD_MAX8
+#define ITER_MAX   0x100
+
+enum plock_use_type {
+   USE_MB,
+   USE_SMP_MB,
+   USE_NUM
+};
+
+struct plock {
+   volatile uint32_t flag[2];
+   volatile uint32_t victim;
+   enum plock_use_type utype;
+};
+
+struct plock_test {
+   struct plock lock;
+   uint32_t val;
+   uint32_t iter;
+};
+
+struct lcore_plock_test {
+   struct plock_test *pt[2];
+   uint32_t sum[2];
+   uint32_t iter;
+   uint32_t lc;
+};
+
+static inline void
+store_load_barrier(uint32_t utype)
+{
+   if (utype == USE_MB)
+   rte_mb();
+   else if (utype == USE_SMP_MB)
+   rte_smp_mb();
+   else
+   RTE_VERIFY(0);
+}
+
+static void
+plock_lock(struct plock *l, uint32_t self)
+{
+   uint32_t other;
+
+   other = self ^ 1;
+
+   l->flag[self] = 1;
+   l->victim = self;
+
+   store_load_barrier(l->utype);
+
+   while (l->flag[other] == 1 && l->victim == self)
+   rte_pause();
+}
+
+static void
+plock_unlock(struct plock *l, uint32_t self)
+{
+   rte_smp_wmb();
+   l->flag[self] = 0;
+}
+
+static void
+plock_reset(struct plock *l, enum plock_use_type utype)
+{
+   memset(l, 0, sizeof(*l));
+   l->utype = utype;
+}
+
+static void
+plock_add(struct plock_test *pt, uint32_t self, uint32_t n)
+{
+   plock_lock(&pt->lock, self);
+   pt->iter++;
+   pt->val += n;
+   plock_unlock(&pt->lock, self);
+}
+
+static int
+plock_test1_lcore(void *data)
+{

Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Friday, December 1, 2017 10:33 AM
> To: Yigit, Ferruh 
> Cc: Thomas Monjalon ; dev@dpdk.org; 
> vl...@cloudius-systems.com
> Subject: Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object
> 
> On Fri, Dec 01, 2017 at 02:29:57AM +, Ferruh Yigit wrote:
> > "struct rte_eth_rxtx_callback" is defined as internal data structure but
> > used in public APIs.
> >
> > Checking the API documentation shows that intention was using this
> > object as opaque object. Data structure only used in delete APIs which
> > doesn't require to know the internals of the data structure.
> >
> > Converting callback parameter in API to void pointer should not require
> > any modification in user application because this data structure was
> > already marked as internal and only should be used as pointer in
> > application.
> >
> > Signed-off-by: Ferruh Yigit 
> > ---
> 
> I disagree on this patch. The structure itself is not exposed, only the
> name, since it is only passed around as a pointer, so there is no need
> to change the parameters to void pointer. It's a named opaque type.

Personally I think it would be better to do visa-versa: 
make rte_eth_add_(rx|tx)_callback() to return struct rte_eth_rxtx_callback *
instead of void *.
Konstantin

[dpdk-dev] [PATCH] contigmem: include to fix FreeBSD build

[kefu chai]

otherwise the build fails with FreeBSD 12, like

In file included from contigmem.c:57:
/usr/srcs/head/src/sys/vm/vm_phys.h:122:10: error: use of undeclared
identifier 'vm_cnt'
return (vm_cnt.v_free_count += adj);
^
1 error generated.
*** Error code 1

Signed-off-by: Kefu Chai 
---
 lib/librte_eal/bsdapp/contigmem/contigmem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_eal/bsdapp/contigmem/contigmem.c
b/lib/librte_eal/bsdapp/contigmem/contigmem.c
index e8fb9087d..9676d8b35 100644
--- a/lib/librte_eal/bsdapp/contigmem/contigmem.c
+++ b/lib/librte_eal/bsdapp/contigmem/contigmem.c
@@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 

 #include 

--
2.15.0

Re: [dpdk-dev] [PATCH] maintainers: claim responsibility for virtio PMD

[Yuanhan Liu]

On Fri, Dec 01, 2017 at 07:04:35PM +0800, Tiwei Bie wrote:
> Add myself as co-maintainer for virtio driver.
> 
> Signed-off-by: Tiwei Bie 
> ---
> Hi Yuanhan and Maxime,
> 
> As there will be more and more virtio related stuffs (e.g. vhost-pci,
> virtio crypto, vhost crypto) in DPDK, more maintaining efforts will be
> needed for virtio/vhost. So I volunteer to co-maintain virtio driver.
> Hope this could help!

I'm glad that you are offering help! And

Acked-by: Yuanhan Liu 

--yliu
> 
> Best regards,
> Tiwei Bie
> 
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f0baeb423..0627aa345 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -528,6 +528,7 @@ F: doc/guides/nics/features/vhost.ini
>  Virtio PMD
>  M: Yuanhan Liu 
>  M: Maxime Coquelin 
> +M: Tiwei Bie 
>  T: git://dpdk.org/next/dpdk-next-virtio
>  F: drivers/net/virtio/
>  F: doc/guides/nics/virtio.rst
> -- 
> 2.13.3

Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership

[Neil Horman]

On Thu, Nov 30, 2017 at 02:24:43PM +0100, Gaëtan Rivet wrote:
> Hello Matan, Neil,
> 
> I like the port ownership concept. I think it is needed to clarify some
> operations and should be useful to several subsystems.
> 
> This patch could certainly be sub-divided however, and your current 1/5
> should probably come after this one.
> 
> Some comments inline.
> 
> On Thu, Nov 30, 2017 at 07:36:11AM -0500, Neil Horman wrote:
> > On Tue, Nov 28, 2017 at 11:57:58AM +, Matan Azrad wrote:
> > > The ownership of a port is implicit in DPDK.
> > > Making it explicit is better from the next reasons:
> > > 1. It may be convenient for multi-process applications to know which
> > >process is in charge of a port.
> > > 2. A library could work on top of a port.
> > > 3. A port can work on top of another port.
> > > 
> > > Also in the fail-safe case, an issue has been met in testpmd.
> > > We need to check that the user is not trying to use a port which is
> > > already managed by fail-safe.
> > > 
> > > Add ownership mechanism to DPDK Ethernet devices to avoid multiple
> > > management of a device by different DPDK entities.
> > > 
> > > A port owner is built from owner id(number) and owner name(string) while
> > > the owner id must be unique to distinguish between two identical entity
> > > instances and the owner name can be any name.
> > > The name helps to logically recognize the owner by different DPDK
> > > entities and allows easy debug.
> > > Each DPDK entity can allocate an owner unique identifier and can use it
> > > and its preferred name to owns valid ethdev ports.
> > > Each DPDK entity can get any port owner status to decide if it can
> > > manage the port or not.
> > > 
> > > The current ethdev internal port management is not affected by this
> > > feature.
> > > 
> 
> The internal port management is not affected, but the external interface
> is, however. In order to respect port ownership, applications are forced
> to modify their port iterator, as shown by your testpmd patch.
> 
> I think it would be better to modify the current RTE_ETH_FOREACH_DEV to call
> RTE_FOREACH_DEV_OWNED_BY, and introduce a default owner that would
> represent the application itself (probably with the ID 0 and an owner
> string ""). Only with specific additional configuration should this
> default subset of ethdev be divided.
> 
> This would make this evolution seamless for applications, at no cost to
> the complexity of the design.
> 
> > > Signed-off-by: Matan Azrad 
> > 
> > 
> > This seems fairly racy.  What if one thread attempts to set ownership on a 
> > port,
> > while another is checking it on another cpu in parallel.  It doesn't seem 
> > like
> > it will protect against that at all. It also doesn't protect against the
> > possibility of multiple threads attempting to poll for rx in parallel, 
> > which I
> > think was part of Thomas's origional statement regarding port ownership (he
> > noted that the lockless design implied only a single thread should be 
> > allowed to
> > poll for receive or make configuration changes at a time.
> > 
> > Neil
> > 
> 
> Isn't this race already there for any configuration operation / polling
> function? The DPDK arch is expecting applications to solve it. Why should
> port ownership be designed differently from other DPDK components?
> 
Yes, but that doesn't mean it should exist in purpituity, nor does it mean that
your new api should contain it as well.

> Embedding checks for port ownership within operations will force
> everyone to bear their costs, even those not interested in using this
> API. These checks should be kept outside, within the entity claiming
> ownership of the port, in the form of using the proper port iterator
> IMO.
> 
No.  At the very least, you need to make the API itself exclusive.  That is to
say, you should at least ensure that a port ownership get call doesn't race with
a port ownership set call.  It seems rediculous to just leave that sort of
locking as an exercize to the user.

Neil

> > > ---
> > >  doc/guides/prog_guide/poll_mode_drv.rst |  12 +++-
> > >  lib/librte_ether/rte_ethdev.c   | 121 
> > > 
> > >  lib/librte_ether/rte_ethdev.h   |  86 +++
> > >  lib/librte_ether/rte_ethdev_version.map |  12 
> > >  4 files changed, 230 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/doc/guides/prog_guide/poll_mode_drv.rst 
> > > b/doc/guides/prog_guide/poll_mode_drv.rst
> > > index 6a0c9f9..af639a1 100644
> > > --- a/doc/guides/prog_guide/poll_mode_drv.rst
> > > +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> > > @@ -156,7 +156,7 @@ concurrently on the same tx queue without SW lock. 
> > > This PMD feature found in som
> > >  
> > >  See `Hardware Offload`_ for ``DEV_TX_OFFLOAD_MT_LOCKFREE`` capability 
> > > probing details.
> > >  
> > > -Device Identification and Configuration
> > > +Device Identification, Ownership  and Configuration
> > >  --

Re: [dpdk-dev] [PATCH] maintainers: claim responsibility for virtio PMD

[Tiwei Bie]

On Fri, Dec 01, 2017 at 08:06:04PM +0800, Yuanhan Liu wrote:
> On Fri, Dec 01, 2017 at 07:04:35PM +0800, Tiwei Bie wrote:
> > Add myself as co-maintainer for virtio driver.
> > 
> > Signed-off-by: Tiwei Bie 
> > ---
> > Hi Yuanhan and Maxime,
> > 
> > As there will be more and more virtio related stuffs (e.g. vhost-pci,
> > virtio crypto, vhost crypto) in DPDK, more maintaining efforts will be
> > needed for virtio/vhost. So I volunteer to co-maintain virtio driver.
> > Hope this could help!
> 
> I'm glad that you are offering help! And
> 
> Acked-by: Yuanhan Liu 
> 

Cool! Thank you so much! ;-)

Best regards,
Tiwei Bie

Re: [dpdk-dev] 16.11.4 (LTS) patches review and test

[Luca Boccassi]

Hi all,

Unfortunately the result of some of the regression tests (from AT&T) are
not available yet due to a lab move. Regression tests from Intel look good
(thanks for the help!) but just to be safe I'll delay tagging 16.11.4 by a
week.

Kind regards,
Luca Boccassi

On 21 Nov 2017 14:27, "Luca Boccassi"  wrote:

Hi all,

Here is a list of patches targeted for LTS release 16.11.4. Please
help review and test. The planned date for the final release is December
the 1st.
Before that, please shout if anyone has objections with these patches being
applied.

These patches are located at branch 16.11 of dpdk-stable repo:
http://dpdk.org/browse/dpdk-stable/

Thanks.

Kind regards,
Luca Boccassi

---
Aaron Conole (1):
  net/enic: fix assignment

Ajit Khaparde (9):
  net/bnxt: fix an issue with broadcast traffic
  net/bnxt: set checksum offload flags correctly
  net/bnxt: update status of Rx IP/L4 CKSUM
  net/bnxt: fix interrupt handler
  net/bnxt: fix Tx offload capability
  net/bnxt: fix Rx offload capability
  net/bnxt: fix a bit shift operation
  net/bnxt: fix a potential null pointer dereference
  net/bnxt: fix link handling and configuration

Alejandro Lucero (2):
  net/nfp: fix RSS
  net/nfp: fix stats struct initial value

Andrey Chilikin (1):
  net/i40e: fix flexible payload configuration

Bruce Richardson (2):
  cmdline: fix warning for unused return value
  eal/bsd: fix missing interrupt stub functions

Chas Williams (1):
  net/vmxnet3: fix memory leak when releasing queues

Congwen Zhang (1):
  net/cxgbe: fix memory leak

Daniel Mrzyglod (2):
  net/virtio: fix untrusted scalar value
  test/pmd_perf: fix crash with multiple devices

David Harton (2):
  net/vmxnet3: fix MAC address set
  net/i40e: fix i40evf MAC filter table

Declan Doherty (1):
  net/bonding: fix LACP slave deactivate behavioral

Edward Makarov (1):
  net/mlx5: fix link speed bitmasks

Ferruh Yigit (4):
  kni: fix ethtool build with kernel 4.11
  buildtools: fix icc build
  config: fix bnx2x option for armv7a
  net/qede: fix icc build

Hemant Agrawal (1):
  examples/l2fwd-crypto: fix uninitialized errno value

Ian Stokes (1):
  cryptodev: fix build with -Ofast

Ilya V. Matveychikov (1):
  pdump: fix possible mbuf leak on failure

Jacek Piasecki (1):
  examples/performance-thread: check thread creation

Jasvinder Singh (1):
  examples/qos_sched: fix uninitialized config

Jerin Jacob (1):
  timer: use 64-bit specific code on more platforms

Jia He (1):
  ring: guarantee load/load order in enqueue and dequeue

Jianbo Liu (1):
  net/i40e: fix Rx packets number for NEON

Jingjing Wu (3):
  net/i40e: fix memory leak if VF init fails
  net/i40e/base: fix bool definition
  net/i40e: fix variable assignment

John Daley (1):
  net/enic: fix packet loss after MTU change

Kuba Kozak (2):
  vfio: fix close unchecked file descriptor
  examples/l3fwd-acl: check fseek return

Lee Roberts (1):
  kni: fix build on RHEL 7.4

Li Han (1):
  app/testpmd: fix invalid port id parameters

Lukasz Majczak (1):
  eal: fix auxv open check for ARM and PPC

Matan Azrad (1):
  net/mlx5: fix probe failure report

Michal Jastrzebski (1):
  net/vmxnet3: fix dereference before null check

Nikhil Rao (1):
  eal/x86: fix atomic cmpset

Nirmoy Das (1):
  kni: fix build on SLE12 SP3

Nélio Laranjeiro (4):
  net/mlx5: fix clang build
  net/mlx5: improve stack usage during link update
  net/mlx5: fix clang compilation error
  app/testpmd: fix RSS structure initialisation

Olivier Matz (7):
  net: fix inner L2 length in packet type parser
  uio: fix compilation with -Og
  cmdline: fix compilation with -Og
  net/virtio: fix mbuf port for simple Rx function
  net/virtio: fix queue setup consistency
  net/virtio: fix compilation with -Og
  lpm6: fix compilation with -Og

Omri Mor (1):
  usertools: fix device binding with python 3

Ophir Munk (1):
  app/testpmd: fix forwarding between non consecutive ports

Pablo de Lara (3):
  hash: fix eviction counter
  crypto/qat: fix SHA512-HMAC supported key size
  app/testpmd: fix topology error message

Patrick MacArthur (1):
  eal: copy raw strings taken from command line

Qi Zhang (4):
  net/i40e: fix flow control watermark mismatch
  net/i40e: fix packet count for PF
  net/i40e: fix mbuf free in vector Tx
  net/i40e: fix mirror with firmware 6.0

Radoslaw Biernacki (1):
  test/memzone: fix memory leak

Rasesh Mody (7):
  net/qede/base: fix to use a passed ptt handle
  net/qede/base: fix macros to check chip revision/metal
  net/qede/base: fix API return types
  net/qede/base: fix number of app table entries
  net/qede/base: fix for VF malicious indication
  net/qede/base: fix return code to align with FW
  net/qede/base: fix division 

Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object

[Bruce Richardson]

On Fri, Dec 01, 2017 at 11:22:12AM +, Ananyev, Konstantin wrote:
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Friday, December 1, 2017 10:33 AM
> > To: Yigit, Ferruh 
> > Cc: Thomas Monjalon ; dev@dpdk.org; 
> > vl...@cloudius-systems.com
> > Subject: Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object
> > 
> > On Fri, Dec 01, 2017 at 02:29:57AM +, Ferruh Yigit wrote:
> > > "struct rte_eth_rxtx_callback" is defined as internal data structure but
> > > used in public APIs.
> > >
> > > Checking the API documentation shows that intention was using this
> > > object as opaque object. Data structure only used in delete APIs which
> > > doesn't require to know the internals of the data structure.
> > >
> > > Converting callback parameter in API to void pointer should not require
> > > any modification in user application because this data structure was
> > > already marked as internal and only should be used as pointer in
> > > application.
> > >
> > > Signed-off-by: Ferruh Yigit 
> > > ---
> > 
> > I disagree on this patch. The structure itself is not exposed, only the
> > name, since it is only passed around as a pointer, so there is no need
> > to change the parameters to void pointer. It's a named opaque type.
> 
> Personally I think it would be better to do visa-versa: 
> make rte_eth_add_(rx|tx)_callback() to return struct rte_eth_rxtx_callback *
> instead of void *.
> Konstantin
> 
I didn't realise that it did, so definite +1 to that suggestion.

Re: [dpdk-dev] [PATCH] contigmem: include to fix FreeBSD build

[Bruce Richardson]

On Fri, Dec 01, 2017 at 07:22:39PM +0800, kefu chai wrote:
> otherwise the build fails with FreeBSD 12, like
> 
> In file included from contigmem.c:57:
> /usr/srcs/head/src/sys/vm/vm_phys.h:122:10: error: use of undeclared
> identifier 'vm_cnt'
> return (vm_cnt.v_free_count += adj);
> ^
> 1 error generated.
> *** Error code 1
> 
> Signed-off-by: Kefu Chai 
> ---
As before:

Acked-by: Bruce Richardson 

NOTE: for minor changes to patches, feel free to include an acked-by tag
on the patch submission if the previous version had already been acked.
Also, it's good practice to have updated versions marked with a V2
number and posted in reply to the original submission. See dev page on
dpdk.org for more details. http://dpdk.org/dev/

Re: [dpdk-dev] [PATCH] maintainers: claim responsibility for virtio PMD

[Maxime Coquelin]

On 12/01/2017 12:04 PM, Tiwei Bie wrote:

Add myself as co-maintainer for virtio driver.

Signed-off-by: Tiwei Bie 
---
Hi Yuanhan and Maxime,

As there will be more and more virtio related stuffs (e.g. vhost-pci,
virtio crypto, vhost crypto) in DPDK, more maintaining efforts will be
needed for virtio/vhost. So I volunteer to co-maintain virtio driver.
Hope this could help!

Best regards,
Tiwei Bie

  MAINTAINERS | 1 +
  1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f0baeb423..0627aa345 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -528,6 +528,7 @@ F: doc/guides/nics/features/vhost.ini
  Virtio PMD
  M: Yuanhan Liu 
  M: Maxime Coquelin 
+M: Tiwei Bie 
  T: git://dpdk.org/next/dpdk-next-virtio
  F: drivers/net/virtio/
  F: doc/guides/nics/virtio.rst



Acked-by: Maxime Coquelin 

Thanks!
Maxime

Re: [dpdk-dev] [PATCH] maintainers: claim responsibility for virtio PMD

[Tiwei Bie]

On Fri, Dec 01, 2017 at 02:46:46PM +0100, Maxime Coquelin wrote:
> On 12/01/2017 12:04 PM, Tiwei Bie wrote:
> > Add myself as co-maintainer for virtio driver.
> > 
> > Signed-off-by: Tiwei Bie 
> > ---
> > Hi Yuanhan and Maxime,
> > 
> > As there will be more and more virtio related stuffs (e.g. vhost-pci,
> > virtio crypto, vhost crypto) in DPDK, more maintaining efforts will be
> > needed for virtio/vhost. So I volunteer to co-maintain virtio driver.
> > Hope this could help!
> > 
> > Best regards,
> > Tiwei Bie
> > 
> >   MAINTAINERS | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f0baeb423..0627aa345 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -528,6 +528,7 @@ F: doc/guides/nics/features/vhost.ini
> >   Virtio PMD
> >   M: Yuanhan Liu 
> >   M: Maxime Coquelin 
> > +M: Tiwei Bie 
> >   T: git://dpdk.org/next/dpdk-next-virtio
> >   F: drivers/net/virtio/
> >   F: doc/guides/nics/virtio.rst
> > 
> 
> Acked-by: Maxime Coquelin 
> 

Thank you very much! ;-)

Best regards,
Tiwei Bie

[dpdk-dev] [dpdk-announce] features review and integration for 18.02

[Thomas Monjalon]

Reminder of the planning for the release 18.02:
- Proposal deadline: November 30, 2017
- Integration deadline: January 10, 2018
- Release: February 5, 2018

The proposal period is now over.
It is time to focus on proposals which were done.
It means everybody should review details and comment the ideas,
before they are integrated for 18.02.

In order to help your patches being reviewed, you should make sure
that the maintainers of the code are the recipients of the patches.
Otherwise you can add them while asking for comments.
Note:
You can use the file MAINTAINERS or the tool devtools/get-maintainer.sh.

Then the maintainers of the code are responsible to make sure the patches
are well reviewed. Everybody can help giving some comments.

At the end, the git tree maintainers (also called committers) will be
responsible to integrate the patches on time.

More details on the process:
http://dpdk.org/doc/guides/contributing/patches.html#the-review-process

Do not hesitate to ping when progress is too slow,
and to help making progress on reviewing your peers.

Collaboration is the key to succeed.
Thank you

[dpdk-dev] [RFC PATCH 1/3] ethdev: introduce eth_queue_local

[Konstantin Ananyev]

Introduce a separate data structure (rte_eth_queue_local)
to store local to given process (i.e. no-shareable) information
for each configured rx/tx queue.
Memory for that structure is allocated/freed dynamically during
rte_eth_dev_configure().
Put a placeholder pointers for queue specific (rx|tx)_pkt_burst(),
tx_pkt_prepare() functions inside that structure.
Move rx/tx callback related information inside that structure.
That introduces a change in current behavior: all callbacks for
un-configured queues will be automatically removed.
Let say: rte_eth_dev_configure(port, 0, 0, ...);
would wipe out all installed callbacks for that port.


Signed-off-by: Konstantin Ananyev 
---
 lib/librte_ether/rte_ethdev.c | 228 +-
 lib/librte_ether/rte_ethdev.h |  85 +++-
 2 files changed, 216 insertions(+), 97 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 318af2869..ff8571d60 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -241,6 +241,14 @@ rte_eth_dev_allocate(const char *name)
return eth_dev;
 }
 
+static struct rte_eth_queue_local *
+alloc_queue_local(uint16_t nb_queues)
+{
+   struct rte_eth_queue_local *ql;
+   ql = rte_zmalloc(NULL, sizeof(ql[0]) * nb_queues, RTE_CACHE_LINE_SIZE);
+   return ql;
+}
+
 /*
  * Attach to a port already registered by the primary process, which
  * makes sure that the same device would have the same port id both
@@ -269,6 +277,16 @@ rte_eth_dev_attach_secondary(const char *name)
eth_dev = eth_dev_get(i);
RTE_ASSERT(eth_dev->data->port_id == i);
 
+   /*
+* obviously it is quite error prone:
+* if the primary process will decide to (re)configure device later,
+* there is no way for secondary one to notice that and update
+* it's local data (queue_local, rx|tx_pkt_burst, etc.).
+* We probably need an eth_dev_configure_secondary() or so.
+*/
+   eth_dev->rx_ql = alloc_queue_local(eth_dev->data->nb_rx_queues);
+   eth_dev->tx_ql = alloc_queue_local(eth_dev->data->nb_tx_queues);
+
return eth_dev;
 }
 
@@ -444,52 +462,92 @@ rte_eth_dev_detach(uint16_t port_id, char *name)
return ret;
 }
 
+/*
+ * Helper routine - removes all registered RX/TX queue callbacks
+ * from the FIFO list.
+ * It is a caller responsibility to make sure no actual RX/TX happens
+ * simultanesoulsy on that queue.
+ */
+static void
+free_rxtx_cbs(struct rte_eth_rxtx_callback *cbs)
+{
+   struct rte_eth_rxtx_callback *cb, *next;
+
+   for (cb = cbs; cb != NULL; cb = next) {
+   next = cb->next;
+   rte_free(cb);
+   }
+}
+
+static void
+reset_queue_local(struct rte_eth_queue_local *ql)
+{
+   free_rxtx_cbs(ql->cbs);
+   memset(ql, 0, sizeof(*ql));
+}
+
 static int
 rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)
 {
-   uint16_t old_nb_queues = dev->data->nb_rx_queues;
+   uint32_t diff, i, old_nb_queues;
+   struct rte_eth_queue_local *rlq;
void **rxq;
-   unsigned i;
 
-   if (dev->data->rx_queues == NULL && nb_queues != 0) { /* first time 
configuration */
-   dev->data->rx_queues = rte_zmalloc("ethdev->rx_queues",
-   sizeof(dev->data->rx_queues[0]) * nb_queues,
-   RTE_CACHE_LINE_SIZE);
-   if (dev->data->rx_queues == NULL) {
-   dev->data->nb_rx_queues = 0;
-   return -(ENOMEM);
-   }
-   } else if (dev->data->rx_queues != NULL && nb_queues != 0) { /* 
re-configure */
-   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release, 
-ENOTSUP);
+   old_nb_queues = dev->data->nb_rx_queues;
+
+   /* same # of queues nothing need to be done */
+   if (nb_queues == old_nb_queues)
+   return 0;
+
+   rxq = dev->data->rx_queues;
+   rlq = dev->rx_ql;
 
-   rxq = dev->data->rx_queues;
+   /* reduce # of queues */
+   if (old_nb_queues > nb_queues) {
 
-   for (i = nb_queues; i < old_nb_queues; i++)
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release,
+   -ENOTSUP);
+
+   dev->data->nb_rx_queues = nb_queues;
+
+   /* free resources used by the queues we are going to remove */
+   for (i = nb_queues; i < old_nb_queues; i++) {
+   reset_queue_local(rlq + i);
(*dev->dev_ops->rx_queue_release)(rxq[i]);
-   rxq = rte_realloc(rxq, sizeof(rxq[0]) * nb_queues,
-   RTE_CACHE_LINE_SIZE);
-   if (rxq == NULL)
-   return -(ENOMEM);
-   if (nb_queues > old_nb_queues) {
-   uint16_t new_qs = nb_queues - old_nb_queues;
-
-   memset(rxq + old_nb_queues, 0,
- 

[dpdk-dev] [RFC PATCH 0/3] *** SUBJECT HERE ***

[Konstantin Ananyev]

The series introduces 2 main changes:

1.Introduce a separate data structure (rte_eth_queue_local)
to store local to given process (i.e. no-shareable) information
for each configured rx/tx queue.
Memory for that structure is allocated/freed dynamically during
rte_eth_dev_configure().
Reserve a space for queue specific (rx|tx)_pkt_burst(),
tx_pkt_prepare() function pointers inside that structure.
Move rx/tx callback related information inside that structure.
That introduces a change in current behavior: all callbacks for
un-configured queues will be automatically removed.
Also as size of struct rte_eth_dev changes that patch is an ABI breakage,
so deprecation notice for 18.05 is filled.
Further suggestions how to introduce the same functionality
without ABI breakage are welcome.

2. Make it safe to remove rx/tx callback at runtime.
Right now it is not possible for the application to figure out
when it is safe to free removed callback handle and
associated with it resources(unless the queue is stopped).
That's probably not a big problem if all callbacks are static
hange through whole application lifetime)
and/or application doesn't allocate any resources for the callback handler.
Though if callbacks have to be added/removed dynamically and
callback handler would require more resources to operate properly -
then it might become an issue.
So patch #2 fixes that problem - now as soon as
rte_eth_remove_(rx|tx)_callback() completes successfully, application
can safely free all associated with the removed callback resources.

Performance impact:
If application doesn't use RX/TX callbacks, then the tests I run didn't
reveal any performance degradation.
Though if application do use RX/TX callbacks - patch #2 does introduce
some slowdown.
 
To be more specific here, on BDW (E5-2699 v4) 2.2GHz, 4x10Gb (X520-4)
with http://dpdk.org/dev/patchwork/patch/31864/ patch installed I got:
1) testpmd ... --latencystats=1 - slowdown < 1%
2) examples//l3fwd ... --parse-ptype - - slowdown < 1%
3) examples/rxtx_callbacks - slowdown ~8%
All that in terms of packet throughput (Mpps).

Ability to safely remove callbacks at runtime implies
some sort of synchronization.
Even I tried to make it as light as possible,
probably some slowdown is unavoidable.
Of course instead of introducing these changes at rte_ethdev layer
similar technique could be applied on individual callback basis.
In that case it would be up to callback writer/installer to decide
does he/she need a removable at runtime callback or not.
Though in that case, each installed callback might introduce extra
synchronization overhead and slowdown.

Konstantin Ananyev (3):
  ethdev: introduce eth_queue_local
  ethdev: make it safe to remove rx/tx callback at runtime
  doc: ethdev ABI change deprecation notice

 doc/guides/rel_notes/deprecation.rst |   5 +
 lib/librte_ether/rte_ethdev.c| 390 ++-
 lib/librte_ether/rte_ethdev.h| 174 
 3 files changed, 387 insertions(+), 182 deletions(-)

-- 
2.13.5

[dpdk-dev] [RFC PATCH 3/3] doc: ethdev ABI change deprecation notice

[Konstantin Ananyev]

Signed-off-by: Konstantin Ananyev 
---
 doc/guides/rel_notes/deprecation.rst | 5 +
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 13e85432f..038b55fd5 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -8,6 +8,11 @@ API and ABI deprecation notices are to be posted here.
 Deprecation Notices
 ---
 
+* ethdev: an ABI changes for ``rte_ethdev`` are planned in v18.05.
+  The size and layout of struct rte_eth_dev will change.
+  Mainly to accommodate queue specific RX/TX function pointers, plus
+  reorganize RX/TX callback related information.
+
 * eal: several API and ABI changes are planned for ``rte_devargs`` in v18.02.
   The format of device command line parameters will change. The bus will need
   to be explicitly stated in the device declaration. The enum ``rte_devtype``
-- 
2.13.5

[dpdk-dev] [RFC PATCH 2/3] ethdev: make it safe to remove rx/tx callback at runtime

[Konstantin Ananyev]

Right now it is not possible for the application to know when it is safe
to free removed callback handle and associated with it resources
(unless the queue is stopped).
That patch changes that behavior to:
- if the rte_eth_remove_(rx|tx)_callback() completes successfully,
then it will automatically free the callback handle and the user can safely
release any associated with the removed callback resources.

Signed-off-by: Konstantin Ananyev 
---
 lib/librte_ether/rte_ethdev.c | 180 +++---
 lib/librte_ether/rte_ethdev.h | 127 ++---
 2 files changed, 194 insertions(+), 113 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ff8571d60..30b23b9b2 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -482,7 +482,7 @@ free_rxtx_cbs(struct rte_eth_rxtx_callback *cbs)
 static void
 reset_queue_local(struct rte_eth_queue_local *ql)
 {
-   free_rxtx_cbs(ql->cbs);
+   free_rxtx_cbs(ql->cbs.head);
memset(ql, 0, sizeof(*ql));
 }
 
@@ -3172,6 +3172,79 @@ rte_eth_dev_filter_ctrl(uint16_t port_id, enum 
rte_filter_type filter_type,
return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
 }
 
+#ifdef RTE_ETHDEV_RXTX_CALLBACKS
+
+/*
+ * Helper routine - contains common code to add RX/TX queue callbacks
+ * to the list.
+ */
+static struct rte_eth_rxtx_callback *
+add_rxtx_callback(struct rte_eth_rxtx_cbs *cbs, int32_t first,
+   struct rte_eth_rxtx_callback *cb, rte_spinlock_t *lock)
+{
+   struct rte_eth_rxtx_callback *tail, **pt;
+
+   rte_spinlock_lock(lock);
+
+   /* Add callback to the head of the list. */
+   if (first != 0) {
+   cb->next = cbs->head;
+   rte_smp_wmb();
+   cbs->head = cb;
+
+   /* Add callback to the tail of the list. */
+   } else {
+   for (pt = &cbs->head; *pt != NULL; pt = &tail->next)
+   tail = *pt;
+
+   *pt = cb;
+   }
+
+   rte_spinlock_unlock(lock);
+   return cb;
+}
+
+/*
+ * Helper routine - contains common code to delete RX/TX queue callbacks
+ * from the FIFO list.
+ */
+static int
+del_rxtx_callback(struct rte_eth_rxtx_cbs *cbs,
+   struct rte_eth_rxtx_callback *user_cb, rte_spinlock_t *lock)
+{
+   int32_t ret;
+   struct rte_eth_rxtx_callback *cb, **prev_cb;
+
+   ret = -EINVAL;
+   rte_spinlock_lock(lock);
+
+   for (prev_cb = &cbs->head; *prev_cb != NULL; prev_cb = &cb->next) {
+
+   cb = *prev_cb;
+   if (cb == user_cb) {
+   /* Remove the user cb from the callback list. */
+   *prev_cb = cb->next;
+   ret = 0;
+   break;
+   }
+   }
+
+   rte_spinlock_unlock(lock);
+
+   /*
+* first make sure datapath doesn't use removed callback anymore,
+* then free the callback structure.
+*/
+   if (ret == 0) {
+   __rte_eth_rxtx_cbs_wait(cbs);
+   rte_free(cb);
+   }
+
+   return ret;
+}
+
+#endif /* RTE_ETHDEV_RXTX_CALLBACKS */
+
 void *
 rte_eth_add_rx_callback(uint16_t port_id, uint16_t queue_id,
rte_rx_callback_fn fn, void *user_param)
@@ -3180,14 +3253,16 @@ rte_eth_add_rx_callback(uint16_t port_id, uint16_t 
queue_id,
rte_errno = ENOTSUP;
return NULL;
 #endif
+   struct rte_eth_rxtx_callback *cb;
+
/* check input parameters */
if (!rte_eth_dev_is_valid_port(port_id) || fn == NULL ||
queue_id >= rte_eth_devices[port_id].data->nb_rx_queues) {
rte_errno = EINVAL;
return NULL;
}
-   struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
 
+   cb = rte_zmalloc(NULL, sizeof(*cb), 0);
if (cb == NULL) {
rte_errno = ENOMEM;
return NULL;
@@ -3196,22 +3271,8 @@ rte_eth_add_rx_callback(uint16_t port_id, uint16_t 
queue_id,
cb->fn.rx = fn;
cb->param = user_param;
 
-   rte_spinlock_lock(&rte_eth_rx_cb_lock);
-   /* Add the callbacks in fifo order. */
-   struct rte_eth_rxtx_callback *tail =
-   rte_eth_devices[port_id].rx_ql[queue_id].cbs;
-
-   if (!tail) {
-   rte_eth_devices[port_id].rx_ql[queue_id].cbs = cb;
-
-   } else {
-   while (tail->next)
-   tail = tail->next;
-   tail->next = cb;
-   }
-   rte_spinlock_unlock(&rte_eth_rx_cb_lock);
-
-   return cb;
+   return add_rxtx_callback(&rte_eth_devices[port_id].rx_ql[queue_id].cbs,
+   0, cb, &rte_eth_rx_cb_lock);
 }
 
 void *
@@ -3222,6 +3283,8 @@ rte_eth_add_first_rx_callback(uint16_t port_id, uint16_t 
queue_id,
rte_errno = ENOTSUP;
return NULL;
 #endif
+   struct rte_eth_rxtx_callback *cb;
+
/* check input parameters */

[dpdk-dev] [RFC PATCH 0/3] ethdev: few changes in rte_ethdev layer

[Konstantin Ananyev]

The series introduces 2 main changes:

1.Introduce a separate data structure (rte_eth_queue_local)
to store local to given process (i.e. no-shareable) information
for each configured rx/tx queue.
Memory for that structure is allocated/freed dynamically during
rte_eth_dev_configure().
Reserve a space for queue specific (rx|tx)_pkt_burst(),
tx_pkt_prepare() function pointers inside that structure.
Move rx/tx callback related information inside that structure.
That introduces a change in current behavior: all callbacks for
un-configured queues will be automatically removed.
Also as size of struct rte_eth_dev changes that patch is an ABI breakage,
so deprecation notice for 18.05 is filled.
Further suggestions how to introduce the same functionality
without ABI breakage are welcome.

2. Make it safe to remove rx/tx callback at runtime.
Right now it is not possible for the application to figure out
when it is safe to free removed callback handle and
associated with it resources(unless the queue is stopped).
That's probably not a big problem if all callbacks are static
hange through whole application lifetime)
and/or application doesn't allocate any resources for the callback handler.
Though if callbacks have to be added/removed dynamically and
callback handler would require more resources to operate properly -
then it might become an issue.
So patch #2 fixes that problem - now as soon as
rte_eth_remove_(rx|tx)_callback() completes successfully, application
can safely free all associated with the removed callback resources.

Performance impact:
If application doesn't use RX/TX callbacks, then the tests I run didn't
reveal any performance degradation.
Though if application do use RX/TX callbacks - patch #2 does introduce
some slowdown.
 
To be more specific here, on BDW (E5-2699 v4) 2.2GHz, 4x10Gb (X520-4)
with http://dpdk.org/dev/patchwork/patch/31864/ patch installed I got:
1) testpmd ... --latencystats=1 - slowdown < 1%
2) examples//l3fwd ... --parse-ptype - - slowdown < 1%
3) examples/rxtx_callbacks - slowdown ~8%
All that in terms of packet throughput (Mpps).

Ability to safely remove callbacks at runtime implies
some sort of synchronization.
Even I tried to make it as light as possible,
probably some slowdown is unavoidable.
Of course instead of introducing these changes at rte_ethdev layer
similar technique could be applied on individual callback basis.
In that case it would be up to callback writer/installer to decide
does he/she need a removable at runtime callback or not.
Though in that case, each installed callback might introduce extra
synchronization overhead and slowdown.

Konstantin Ananyev (3):
  ethdev: introduce eth_queue_local
  ethdev: make it safe to remove rx/tx callback at runtime
  doc: ethdev ABI change deprecation notice

 doc/guides/rel_notes/deprecation.rst |   5 +
 lib/librte_ether/rte_ethdev.c| 390 ++-
 lib/librte_ether/rte_ethdev.h| 174 
 3 files changed, 387 insertions(+), 182 deletions(-)

-- 
2.13.5

Re: [dpdk-dev] [RFC PATCH 0/3] *** SUBJECT HERE ***

[Ananyev, Konstantin]

Oops sorry, resending with proper subject.

> -Original Message-
> From: Ananyev, Konstantin
> Sent: Friday, December 1, 2017 2:48 PM
> To: dev@dpdk.org; dev@dpdk.org
> Cc: Ananyev, Konstantin 
> Subject: [RFC PATCH 0/3] *** SUBJECT HERE ***
> 
> The series introduces 2 main changes:
> 
> 1.Introduce a separate data structure (rte_eth_queue_local)
> to store local to given process (i.e. no-shareable) information
> for each configured rx/tx queue.
> Memory for that structure is allocated/freed dynamically during
> rte_eth_dev_configure().
> Reserve a space for queue specific (rx|tx)_pkt_burst(),
> tx_pkt_prepare() function pointers inside that structure.
> Move rx/tx callback related information inside that structure.
> That introduces a change in current behavior: all callbacks for
> un-configured queues will be automatically removed.
> Also as size of struct rte_eth_dev changes that patch is an ABI breakage,
> so deprecation notice for 18.05 is filled.
> Further suggestions how to introduce the same functionality
> without ABI breakage are welcome.
> 
> 2. Make it safe to remove rx/tx callback at runtime.
> Right now it is not possible for the application to figure out
> when it is safe to free removed callback handle and
> associated with it resources(unless the queue is stopped).
> That's probably not a big problem if all callbacks are static
> hange through whole application lifetime)
> and/or application doesn't allocate any resources for the callback handler.
> Though if callbacks have to be added/removed dynamically and
> callback handler would require more resources to operate properly -
> then it might become an issue.
> So patch #2 fixes that problem - now as soon as
> rte_eth_remove_(rx|tx)_callback() completes successfully, application
> can safely free all associated with the removed callback resources.
> 
> Performance impact:
> If application doesn't use RX/TX callbacks, then the tests I run didn't
> reveal any performance degradation.
> Though if application do use RX/TX callbacks - patch #2 does introduce
> some slowdown.
> 
> To be more specific here, on BDW (E5-2699 v4) 2.2GHz, 4x10Gb (X520-4)
> with http://dpdk.org/dev/patchwork/patch/31864/ patch installed I got:
> 1) testpmd ... --latencystats=1 - slowdown < 1%
> 2) examples//l3fwd ... --parse-ptype - - slowdown < 1%
> 3) examples/rxtx_callbacks - slowdown ~8%
> All that in terms of packet throughput (Mpps).
> 
> Ability to safely remove callbacks at runtime implies
> some sort of synchronization.
> Even I tried to make it as light as possible,
> probably some slowdown is unavoidable.
> Of course instead of introducing these changes at rte_ethdev layer
> similar technique could be applied on individual callback basis.
> In that case it would be up to callback writer/installer to decide
> does he/she need a removable at runtime callback or not.
> Though in that case, each installed callback might introduce extra
> synchronization overhead and slowdown.
> 
> Konstantin Ananyev (3):
>   ethdev: introduce eth_queue_local
>   ethdev: make it safe to remove rx/tx callback at runtime
>   doc: ethdev ABI change deprecation notice
> 
>  doc/guides/rel_notes/deprecation.rst |   5 +
>  lib/librte_ether/rte_ethdev.c| 390 
> ++-
>  lib/librte_ether/rte_ethdev.h| 174 
>  3 files changed, 387 insertions(+), 182 deletions(-)
> 
> --
> 2.13.5

Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: add target queues in flow actions

[Anoob Joseph]

Hi Nelio,

On 30-11-2017 17:58, Nelio Laranjeiro wrote:

Hi Annob,

On Thu, Nov 30, 2017 at 04:16:23PM +0530, Anoob wrote:

On 11/29/2017 06:20 PM, Nelio Laranjeiro wrote:

Hi Anoob,

On Wed, Nov 29, 2017 at 06:00:38PM +0530, Anoob wrote:

 Hi Nelio,

 Since support of RSS with inline crypto/protocol is hardware
 implementation dependent, it would be better if there is some sort of
 capability check before setting the flow parameters in the application.

 If the hardware doesn't support RSS with inline processing, then the RSS
 flow action will have to be ignored in the driver. This wouldn't look
 right from application's point of view. And also the PMD would need
 application-specific logic to handle such cases, which may not scale well.

There is a real issue here, RTE_FLOW API needs a terminal action, security is
not one [1] you must have one of the followings: QUEUE, DROP, RSS, PF,
VF or PASSTHRU.

Flow API does not work with "capabilities" as the application can verify
the rule using the validate().  If it cannot be validated the
application can test another kind of rule until the PMD returns a
success.

Here, I am proposing the RSS as RSS with a single queue is equivalent to queue.

On Mellanox NIC we need the RSS or QUEUE in ingress and for Egress PASSTHRU
is good.

What are your needs?

Thanks for the clarification. Understood the issue here. On Cavium hardware
SECURITY will be terminating.

You should finalise with PASSTHRU to be compliant with the API,
otherwise application makers won't understand why it does not work
according to the API implementation.
Cavium hardware would be supporting only terminating actions. So 
PASSTHRU will not be supported.

So a better approach would be to first check from the application
(using rte_flow_verify()) if SECURITY is terminating action. If it
fails, then application can do RSS/QUEUE. That should solve
the issue.



I think we have an agreement here, in order the final action to be
tested:

  1. PASSTHRU
  2. RSS
  3. QUEUE

If those 3 fails, the functions fails to create the rule, the first
succeeding is the one applied.
PASSTHRU itself is non-terminating, right? So I'm not sure, how a check 
with PASSTHRU would help us. SECURITY will be terminating action on 
Cavium hardware. So, the first check could be without any addition. If 
that fails, RSS. And then QUEUE. That should be fine.


Any thoughts?
Anoob


Do you agree?

Thanks,

[dpdk-dev] [PATCH] examples/eventdev_pipeline: add Rx adapter support

[Pavan Nikhilesh]

Use event Rx adapter for packets Rx instead of explicit producer logic.
Use service run iter function for granular control instead of using dedicated
service lcore.

Signed-off-by: Pavan Nikhilesh 
---
 examples/eventdev_pipeline_sw_pmd/main.c | 149 +--
 1 file changed, 82 insertions(+), 67 deletions(-)

diff --git a/examples/eventdev_pipeline_sw_pmd/main.c 
b/examples/eventdev_pipeline_sw_pmd/main.c
index 5f431d87d..59cfea0d8 100644
--- a/examples/eventdev_pipeline_sw_pmd/main.c
+++ b/examples/eventdev_pipeline_sw_pmd/main.c
@@ -46,6 +46,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #define MAX_NUM_STAGES 8
@@ -78,6 +79,7 @@ struct fastpath_data {
uint32_t tx_lock;
uint32_t sched_lock;
uint32_t evdev_service_id;
+   uint32_t rxadptr_service_id;
bool rx_single;
bool tx_single;
bool sched_single;
@@ -105,6 +107,7 @@ struct config_data {
unsigned int worker_cq_depth;
int16_t next_qid[MAX_NUM_STAGES+2];
int16_t qid[MAX_NUM_STAGES];
+   uint8_t rx_adapter_id;
 };

 static struct config_data cdata = {
@@ -193,53 +196,12 @@ consumer(void)
return 0;
 }

-static int
-producer(void)
-{
-   static uint8_t eth_port;
-   struct rte_mbuf *mbufs[BATCH_SIZE+2];
-   struct rte_event ev[BATCH_SIZE+2];
-   uint32_t i, num_ports = prod_data.num_nic_ports;
-   int32_t qid = prod_data.qid;
-   uint8_t dev_id = prod_data.dev_id;
-   uint8_t port_id = prod_data.port_id;
-   uint32_t prio_idx = 0;
-
-   const uint16_t nb_rx = rte_eth_rx_burst(eth_port, 0, mbufs, BATCH_SIZE);
-   if (++eth_port == num_ports)
-   eth_port = 0;
-   if (nb_rx == 0) {
-   rte_pause();
-   return 0;
-   }
-
-   for (i = 0; i < nb_rx; i++) {
-   ev[i].flow_id = mbufs[i]->hash.rss;
-   ev[i].op = RTE_EVENT_OP_NEW;
-   ev[i].sched_type = cdata.queue_type;
-   ev[i].queue_id = qid;
-   ev[i].event_type = RTE_EVENT_TYPE_ETHDEV;
-   ev[i].sub_event_type = 0;
-   ev[i].priority = RTE_EVENT_DEV_PRIORITY_NORMAL;
-   ev[i].mbuf = mbufs[i];
-   RTE_SET_USED(prio_idx);
-   }
-
-   const int nb_tx = rte_event_enqueue_burst(dev_id, port_id, ev, nb_rx);
-   if (nb_tx != nb_rx) {
-   for (i = nb_tx; i < nb_rx; i++)
-   rte_pktmbuf_free(mbufs[i]);
-   }
-
-   return 0;
-}
-
 static inline void
 schedule_devices(unsigned int lcore_id)
 {
if (fdata->rx_core[lcore_id] && (fdata->rx_single ||
rte_atomic32_cmpset(&(fdata->rx_lock), 0, 1))) {
-   producer();
+   rte_service_run_iter_on_app_lcore(fdata->rxadptr_service_id, 1);
rte_atomic32_clear((rte_atomic32_t *)&(fdata->rx_lock));
}

@@ -553,6 +515,79 @@ parse_app_args(int argc, char **argv)
}
 }

+static inline void
+init_rx_adapter(uint16_t nb_ports)
+{
+   int i;
+   int ret;
+   uint8_t evdev_id = 0;
+   uint8_t port_needed = 0;
+   struct rte_event_dev_info dev_info;
+
+   ret = rte_event_dev_info_get(evdev_id, &dev_info);
+
+   struct rte_event_port_conf rx_p_conf = {
+   .dequeue_depth = 8,
+   .enqueue_depth = 8,
+   .new_event_threshold = 1200,
+   };
+
+   if (rx_p_conf.dequeue_depth > dev_info.max_event_port_dequeue_depth)
+   rx_p_conf.dequeue_depth = dev_info.max_event_port_dequeue_depth;
+   if (rx_p_conf.enqueue_depth > dev_info.max_event_port_enqueue_depth)
+   rx_p_conf.enqueue_depth = dev_info.max_event_port_enqueue_depth;
+
+   ret = rte_event_eth_rx_adapter_create(cdata.rx_adapter_id, evdev_id,
+   &rx_p_conf);
+if (ret)
+ rte_exit(EXIT_FAILURE, "failed to create rx adapter[%d]",
+cdata.rx_adapter_id);
+
+   struct rte_event_eth_rx_adapter_queue_conf queue_conf = {
+   .ev.sched_type = cdata.queue_type,
+   .ev.queue_id = cdata.qid[0],
+   };
+
+   for (i = 0; i < nb_ports; i++) {
+   uint32_t cap;
+
+   ret = rte_event_eth_rx_adapter_caps_get(evdev_id, i, &cap);
+   if (ret)
+   rte_exit(EXIT_FAILURE,
+   "failed to get event rx adapter "
+   "capabilities");
+   /* Producer needs port. */
+   port_needed |= !(cap &
+   RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT);
+
+   ret = rte_event_eth_rx_adapter_queue_add(cdata.rx_adapter_id, i,
+   -1, &queue_conf);
+   if (ret)
+   rte_exit(EXIT_FAILURE,
+   "Failed to add queues to Rx adapter");
+   }
+
+   if (port_need

[dpdk-dev] [PATCH 0/7] Sync with MUSDK-17.10

[Tomasz Duszynski]

This patchset brings following changes:

o Sync with MUSDK-17.10. Latest version of the library comes with many
  improvements and fixes thus switching to it is beneficial.

o A few code and documentation updates.

Changes since v1:
o Add extra error log in case setting link up fails.
o Cram all MUSDK related headers into the same the driver header.

Tomasz Duszynski (7):
  net/mrvl: sync compilation with musdk-17.10
  net/mrvl: query link status using library API
  net/mrvl: do not enable port after setting MAC address
  net/mrvl: check if ppio is initialized
  net/mrvl: add extra error logs
  devtools/test-build: add MRVL NET PMD to test-build
  net/mrvl: update MRVL NET PMD documentation

 devtools/test-build.sh |  1 +
 doc/guides/nics/mrvl.rst   | 69 ---
 drivers/net/mrvl/Makefile  |  4 +-
 drivers/net/mrvl/mrvl_ethdev.c | 92 +-
 drivers/net/mrvl/mrvl_ethdev.h |  5 +++
 5 files changed, 117 insertions(+), 54 deletions(-)

--
2.7.4

[dpdk-dev] [PATCH 1/7] net/mrvl: sync compilation with musdk-17.10

[Tomasz Duszynski]

Followig changes are needed to switch to musdk-17.10:

- With a new version of the musdk library it's no longer necessary to
  explicitly define MVCONF_ARCH_DMA_ADDR_T_64BIT and
  CONF_PP2_BPOOL_COOKIE_SIZE.

  Proper defines are autogenerated by ./configure script based on
  passed options and available after mv_autogen_comp_flags.h inclusion.

- API used to set promiscuous mode was renamed. Thus in order to
  compile against the latest library new API must be used.

Signed-off-by: Tomasz Duszynski 
---
 drivers/net/mrvl/Makefile  | 4 ++--
 drivers/net/mrvl/mrvl_ethdev.c | 8 ++--
 drivers/net/mrvl/mrvl_ethdev.h | 5 +
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mrvl/Makefile b/drivers/net/mrvl/Makefile
index 815c3ba..f75e53c 100644
--- a/drivers/net/mrvl/Makefile
+++ b/drivers/net/mrvl/Makefile
@@ -51,8 +51,8 @@ EXPORT_MAP := rte_pmd_mrvl_version.map
 
 # external library dependencies
 CFLAGS += -I$(LIBMUSDK_PATH)/include
-CFLAGS += -DMVCONF_ARCH_DMA_ADDR_T_64BIT
-CFLAGS += -DCONF_PP2_BPOOL_COOKIE_SIZE=32
+CFLAGS += -DMVCONF_TYPES_PUBLIC
+CFLAGS += -DMVCONF_DMA_PHYS_ADDR_T_PUBLIC
 CFLAGS += $(WERROR_FLAGS)
 CFLAGS += -O3
 LDLIBS += -L$(LIBMUSDK_PATH)/lib
diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index 2936165..a1ae2c1 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -47,10 +47,6 @@
 #undef container_of
 #endif
 
-#include 
-#include 
-#include 
-
 #include 
 #include 
 #include 
@@ -690,7 +686,7 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
-   ret = pp2_ppio_set_uc_promisc(priv->ppio, 1);
+   ret = pp2_ppio_set_promisc(priv->ppio, 1);
if (ret)
RTE_LOG(ERR, PMD, "Failed to enable promiscuous mode\n");
 }
@@ -724,7 +720,7 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
-   ret = pp2_ppio_set_uc_promisc(priv->ppio, 0);
+   ret = pp2_ppio_set_promisc(priv->ppio, 0);
if (ret)
RTE_LOG(ERR, PMD, "Failed to disable promiscuous mode\n");
 }
diff --git a/drivers/net/mrvl/mrvl_ethdev.h b/drivers/net/mrvl/mrvl_ethdev.h
index 2a4ab5a..8a647a5 100644
--- a/drivers/net/mrvl/mrvl_ethdev.h
+++ b/drivers/net/mrvl/mrvl_ethdev.h
@@ -36,7 +36,12 @@
 #define _MRVL_ETHDEV_H_
 
 #include 
+
+#include 
+#include 
+#include 
 #include 
+#include 
 #include 
 
 /** Maximum number of rx queues per port */
-- 
2.7.4

[dpdk-dev] [PATCH 2/7] net/mrvl: query link status using library API

[Tomasz Duszynski]

Up to now link status was updated unconditionally during
link_up()/link_down() calls thus one was never sure about
it's true status.

Using dedicated library api makes sure the true link status is set.

Signed-off-by: Tomasz Duszynski 
---
 drivers/net/mrvl/mrvl_ethdev.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index a1ae2c1..47f12b8 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -361,8 +361,6 @@ mrvl_dev_set_link_up(struct rte_eth_dev *dev)
if (ret)
pp2_ppio_disable(priv->ppio);
 
-   dev->data->dev_link.link_status = ETH_LINK_UP;
-
return ret;
 }
 
@@ -379,15 +377,8 @@ static int
 mrvl_dev_set_link_down(struct rte_eth_dev *dev)
 {
struct mrvl_priv *priv = dev->data->dev_private;
-   int ret;
-
-   ret = pp2_ppio_disable(priv->ppio);
-   if (ret)
-   return ret;
-
-   dev->data->dev_link.link_status = ETH_LINK_DOWN;
 
-   return ret;
+   return pp2_ppio_disable(priv->ppio);
 }
 
 /**
@@ -628,9 +619,10 @@ mrvl_link_update(struct rte_eth_dev *dev, int 
wait_to_complete __rte_unused)
 * TODO
 * once MUSDK provides necessary API use it here
 */
+   struct mrvl_priv *priv = dev->data->dev_private;
struct ethtool_cmd edata;
struct ifreq req;
-   int ret, fd;
+   int ret, fd, link_up;
 
edata.cmd = ETHTOOL_GSET;
 
@@ -670,6 +662,8 @@ mrvl_link_update(struct rte_eth_dev *dev, int 
wait_to_complete __rte_unused)
 ETH_LINK_HALF_DUPLEX;
dev->data->dev_link.link_autoneg = edata.autoneg ? ETH_LINK_AUTONEG :
   ETH_LINK_FIXED;
+   pp2_ppio_get_link_state(priv->ppio, &link_up);
+   dev->data->dev_link.link_status = link_up ? ETH_LINK_UP : ETH_LINK_DOWN;
 
return 0;
 }
-- 
2.7.4

[dpdk-dev] [PATCH 3/7] net/mrvl: do not enable port after setting MAC address

[Tomasz Duszynski]

Setting enabled port's mac address caused it to stop receiving
packets. Now as that issue is fixed in library renabling port
is no longer necessary.

Signed-off-by: Tomasz Duszynski 
---
 drivers/net/mrvl/mrvl_ethdev.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index 47f12b8..c44a2bc 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -822,15 +822,14 @@ static void
 mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
struct mrvl_priv *priv = dev->data->dev_private;
+   int ret;
 
-   pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes);
-   /*
-* TODO
-* Port stops sending packets if pp2_ppio_set_mac_addr()
-* was called after pp2_ppio_enable(). As a quick fix issue
-* enable port once again.
-*/
-   pp2_ppio_enable(priv->ppio);
+   ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes);
+   if (ret) {
+   char buf[ETHER_ADDR_FMT_SIZE];
+   ether_format_addr(buf, sizeof(buf), mac_addr);
+   RTE_LOG(ERR, PMD, "Failed to set mac to %s\n", buf);
+   }
 }
 
 /**
-- 
2.7.4

[dpdk-dev] [PATCH 4/7] net/mrvl: check if ppio is initialized

[Tomasz Duszynski]

Uninitialized ppio cannot be passed to MUSDK library routines as
application will crash.

Fix this by first checking whether ppio has been initialized.

Signed-off-by: Tomasz Duszynski 
---
 drivers/net/mrvl/mrvl_ethdev.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index c44a2bc..40f2ead 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -324,6 +324,9 @@ mrvl_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
if (mtu < ETHER_MIN_MTU || mru > MRVL_PKT_SIZE_MAX)
return -EINVAL;
 
+   if (!priv->ppio)
+   return -EPERM;
+
ret = pp2_ppio_set_mru(priv->ppio, mru);
if (ret)
return ret;
@@ -346,6 +349,9 @@ mrvl_dev_set_link_up(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
+   if (!priv->ppio)
+   return -EPERM;
+
ret = pp2_ppio_enable(priv->ppio);
if (ret)
return ret;
@@ -378,6 +384,9 @@ mrvl_dev_set_link_down(struct rte_eth_dev *dev)
 {
struct mrvl_priv *priv = dev->data->dev_private;
 
+   if (!priv->ppio)
+   return -EPERM;
+
return pp2_ppio_disable(priv->ppio);
 }
 
@@ -624,6 +633,9 @@ mrvl_link_update(struct rte_eth_dev *dev, int 
wait_to_complete __rte_unused)
struct ifreq req;
int ret, fd, link_up;
 
+   if (!priv->ppio)
+   return -EPERM;
+
edata.cmd = ETHTOOL_GSET;
 
strcpy(req.ifr_name, dev->data->name);
@@ -680,6 +692,9 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
+   if (!priv->ppio)
+   return;
+
ret = pp2_ppio_set_promisc(priv->ppio, 1);
if (ret)
RTE_LOG(ERR, PMD, "Failed to enable promiscuous mode\n");
@@ -697,6 +712,9 @@ mrvl_allmulticast_enable(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
+   if (!priv->ppio)
+   return;
+
ret = pp2_ppio_set_mc_promisc(priv->ppio, 1);
if (ret)
RTE_LOG(ERR, PMD, "Failed enable all-multicast mode\n");
@@ -714,6 +732,9 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
+   if (!priv->ppio)
+   return;
+
ret = pp2_ppio_set_promisc(priv->ppio, 0);
if (ret)
RTE_LOG(ERR, PMD, "Failed to disable promiscuous mode\n");
@@ -731,6 +752,9 @@ mrvl_allmulticast_disable(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
+   if (!priv->ppio)
+   return;
+
ret = pp2_ppio_set_mc_promisc(priv->ppio, 0);
if (ret)
RTE_LOG(ERR, PMD, "Failed to disable all-multicast mode\n");
@@ -751,6 +775,9 @@ mrvl_mac_addr_remove(struct rte_eth_dev *dev, uint32_t 
index)
char buf[ETHER_ADDR_FMT_SIZE];
int ret;
 
+   if (!priv->ppio)
+   return;
+
ret = pp2_ppio_remove_mac_addr(priv->ppio,
   dev->data->mac_addrs[index].addr_bytes);
if (ret) {
@@ -787,6 +814,9 @@ mrvl_mac_addr_add(struct rte_eth_dev *dev, struct 
ether_addr *mac_addr,
/* For setting index 0, mrvl_mac_addr_set() should be used.*/
return -1;
 
+   if (!priv->ppio)
+   return -EPERM;
+
/*
 * Maximum number of uc addresses can be tuned via kernel module mvpp2x
 * parameter uc_filter_max. Maximum number of mc addresses is then
@@ -824,6 +854,9 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct 
ether_addr *mac_addr)
struct mrvl_priv *priv = dev->data->dev_private;
int ret;
 
+   if (!priv->ppio)
+   return;
+
ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes);
if (ret) {
char buf[ETHER_ADDR_FMT_SIZE];
@@ -851,6 +884,9 @@ mrvl_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *stats)
uint64_t drop_mac = 0;
unsigned int i, idx, ret;
 
+   if (!priv->ppio)
+   return -EPERM;
+
for (i = 0; i < dev->data->nb_rx_queues; i++) {
struct mrvl_rxq *rxq = dev->data->rx_queues[i];
struct pp2_ppio_inq_statistics rx_stats;
@@ -943,6 +979,9 @@ mrvl_stats_reset(struct rte_eth_dev *dev)
struct mrvl_priv *priv = dev->data->dev_private;
int i;
 
+   if (!priv->ppio)
+   return;
+
for (i = 0; i < dev->data->nb_rx_queues; i++) {
struct mrvl_rxq *rxq = dev->data->rx_queues[i];
 
@@ -1099,6 +1138,9 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t 
vlan_id, int on)
 {
struct mrvl_priv *priv = dev->data->dev_private;
 
+   if (!priv->ppio)
+

[dpdk-dev] [PATCH 6/7] devtools/test-build: add MRVL NET PMD to test-build

[Tomasz Duszynski]

Add MRVL NET PMD to test build tool.

Signed-off-by: Tomasz Duszynski 
---
 devtools/test-build.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/devtools/test-build.sh b/devtools/test-build.sh
index 092d3a7..78609c9 100755
--- a/devtools/test-build.sh
+++ b/devtools/test-build.sh
@@ -197,6 +197,7 @@ config () #   
sed -ri   's,(SCHED_.*=)n,\1y,' $1/.config
test -z "$LIBMUSDK_PATH" || \
sed -ri's,(PMD_MRVL_CRYPTO=)n,\1y,' $1/.config
+   sed -ri   's,(MRVL_PMD=)n,\1y,' $1/.config
build_config_hook $1 $2 $3
 
# Explicit enabler/disabler (uppercase)
-- 
2.7.4

[dpdk-dev] [PATCH 7/7] net/mrvl: update MRVL NET PMD documentation

[Tomasz Duszynski]

Update MRVL NET PMD documentation.

Signed-off-by: Tomasz Duszynski 
---
 doc/guides/nics/mrvl.rst | 69 +++-
 1 file changed, 45 insertions(+), 24 deletions(-)

diff --git a/doc/guides/nics/mrvl.rst b/doc/guides/nics/mrvl.rst
index fbfdf47..67b254c 100644
--- a/doc/guides/nics/mrvl.rst
+++ b/doc/guides/nics/mrvl.rst
@@ -87,31 +87,40 @@ Limitations
 Prerequisites
 -
 
-- Custom Linux Kernel sources available
-  `here 
`__.
+- Custom Linux Kernel sources
 
-- Out of tree `mvpp2x_sysfs` kernel module sources available
-  `here 
`__.
+  .. code-block:: console
 
-- MUSDK (Marvell User-Space SDK) sources available
-  `here 
`__.
+ git clone https://github.com/MarvellEmbeddedProcessors/linux-marvell.git 
-b linux-4.4.52-armada-17.10
 
-MUSDK is a light-weight library that provides direct access to Marvell's
-PPv2 (Packet Processor v2). Alternatively prebuilt MUSDK library can be
-requested from `Marvell Extranet `_. Once
-approval has been granted, library can be found by typing ``musdk`` in
-the search box.
+- Out of tree `mvpp2x_sysfs` kernel module sources
 
-MUSDK must be configured with the following features:
+  .. code-block:: console
 
-.. code-block:: console
+ git clone https://github.com/MarvellEmbeddedProcessors/mvpp2x-marvell.git 
-b mvpp2x-armada-17.10
 
-   --enable-bpool-dma=64
+- MUSDK (Marvell User-Space SDK) sources
+
+  .. code-block:: console
+
+ git clone https://github.com/MarvellEmbeddedProcessors/musdk-marvell.git 
-b musdk-armada-17.10
+
+  MUSDK is a light-weight library that provides direct access to Marvell's
+  PPv2 (Packet Processor v2). Alternatively prebuilt MUSDK library can be
+  requested from `Marvell Extranet `_. Once
+  approval has been granted, library can be found by typing ``musdk`` in
+  the search box.
+
+  MUSDK must be configured with the following features:
+
+  .. code-block:: console
+
+ --enable-bpool-dma=64
 
 - DPDK environment
 
-Follow the DPDK :ref:`Getting Started Guide for Linux ` to setup
-DPDK environment.
+  Follow the DPDK :ref:`Getting Started Guide for Linux ` to setup
+  DPDK environment.
 
 
 Config File Options
@@ -123,11 +132,6 @@ The following options can be modified in the ``config`` 
file.
 
 Toggle compilation of the librte_pmd_mrvl driver.
 
-- ``CONFIG_RTE_MRVL_MUSDK_DMA_MEMSIZE`` (default ``41943040``)
-
-Size in bytes of the contiguous memory region that MUSDK will allocate
-for run-time DMA-able data buffers.
-
 
 QoS Configuration
 -
@@ -142,7 +146,7 @@ Configuration syntax
 
[port  default]
default_tc = 
-   qos_mode = 
+   mapping_priority = 
 
[port  tc ]
rxq = 
@@ -160,7 +164,7 @@ Where:
 
 - : Default traffic class (e.g. 0)
 
-- : QoS priority for mapping (`ip`, `vlan`, `ip/vlan` or 
`vlan/ip`).
+- : QoS priority for mapping (`ip`, `vlan`, `ip/vlan` or 
`vlan/ip`).
 
 - : Traffic Class to be configured.
 
@@ -218,9 +222,26 @@ Building DPDK
 Driver needs precompiled MUSDK library during compilation. Please consult
 ``doc/musdk_get_started.txt`` for the detailed build instructions.
 
+.. code-block:: console
+
+   export CROSS_COMPILE=/bin/aarch64-linux-gnu-
+   ./bootstrap
+   ./configure --enable-bpool-dma=64
+   make install
+
+MUSDK will be installed to `usr/local` under current directory.
+For the detailed build instructions please consult 
``doc/musdk_get_started.txt``.
+
 Before the DPDK build process the environmental variable ``LIBMUSDK_PATH`` with
 the path to the MUSDK installation directory needs to be exported.
 
+.. code-block:: console
+
+   export LIBMUSDK_PATH=/usr/local
+   export CROSS=aarch64-linux-gnu-
+   make config T=arm64-armv8a-linuxapp-gcc
+   sed -ri 's,(MRVL_PMD=)n,\1y,' build/.config
+   make
 
 Usage Example
 -
@@ -242,7 +263,7 @@ Additionally interfaces used by DPDK application need to be 
put up:
 .. code-block:: console
 
ip link set eth0 up
-   ip link set eth1 up
+   ip link set eth2 up
 
 In order to run testpmd example application following command can be used:
 
-- 
2.7.4

[dpdk-dev] [PATCH 5/7] net/mrvl: add extra error logs

[Tomasz Duszynski]

Add extra error logs in a few places.

Signed-off-by: Tomasz Duszynski 
---
 drivers/net/mrvl/mrvl_ethdev.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index 40f2ead..127ce44 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -426,8 +426,10 @@ mrvl_dev_start(struct rte_eth_dev *dev)
priv->bpool_min_size = priv->nb_rx_queues * MRVL_BURST_SIZE * 2;
 
ret = pp2_ppio_init(&priv->ppio_params, &priv->ppio);
-   if (ret)
+   if (ret) {
+   RTE_LOG(ERR, PMD, "Failed to init ppio\n");
return ret;
+   }
 
/*
 * In case there are some some stale uc/mc mac addresses flush them
@@ -462,17 +464,20 @@ mrvl_dev_start(struct rte_eth_dev *dev)
if (mrvl_qos_cfg) {
ret = mrvl_start_qos_mapping(priv);
if (ret) {
-   pp2_ppio_deinit(priv->ppio);
-   return ret;
+   RTE_LOG(ERR, PMD, "Failed to setup QoS mapping\n");
+   goto out;
}
}
 
ret = mrvl_dev_set_link_up(dev);
-   if (ret)
+   if (ret) {
+   RTE_LOG(ERR, PMD, "Failed to set link up\n");
goto out;
+   }
 
return 0;
 out:
+   RTE_LOG(ERR, PMD, "Failed to start device\n");
pp2_ppio_deinit(priv->ppio);
return ret;
 }
-- 
2.7.4

[dpdk-dev] [PATCH] net/i40e: do not turn on flexible payload on driver init

[Kirill Rybalchenko]

Function i40e_GLQF_reg_init() overwrites global register for
flexible payload, forcing extraction of first 16 bytes of
L2/L3/L4 payload to the field vector even if flexible payload
is not used by an application. Such unconditional turn on of
flexible payload effectively disables ability to use outer
IP Destination address for RSS/FDIR for tunnelled packets,
as flexible payload overwrites outer IP destination address
on the field vector.

Now flexible payload turned on only when flow director is
enabled and configured.

v1:
Global registers will be set only when payload is enabled.
They will be reset if payload is disabled or on port reset
(uninit).

Signed-off-by: Kirill Rybalchenko 
---
 drivers/net/i40e/i40e_ethdev.c | 32 +---
 drivers/net/i40e/i40e_ethdev.h |  1 +
 drivers/net/i40e/i40e_fdir.c   |  1 +
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 811cc9f..d6a207f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -701,17 +701,6 @@ RTE_PMD_REGISTER_KMOD_DEP(net_i40e, "* igb_uio | 
uio_pci_generic | vfio-pci");
 static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 {
/*
-* Force global configuration for flexible payload
-* to the first 16 bytes of the corresponding L2/L3/L4 paylod.
-* This should be removed from code once proper
-* configuration API is added to avoid configuration conflicts
-* between ports of the same device.
-*/
-   I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x00E0);
-   I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x00E3);
-   I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x00E6);
-
-   /*
 * Initialize registers for parsing packet type of QinQ
 * This should be removed from code once proper
 * configuration API is added to avoid configuration conflicts
@@ -1435,6 +1424,24 @@ i40e_rm_fdir_filter_list(struct i40e_pf *pf)
}
 }
 
+void i40e_flex_payload_reg_cfg(struct i40e_hw *hw, bool enable)
+{
+   if (enable) {
+   /*
+* Set global configuration for flexible payload
+* to the first 16 bytes of the corresponding L2/L3/L4 paylod.
+*/
+   I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x00E0);
+   I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x00E3);
+   I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x00E6);
+   } else {
+   /* Disable flexible payload in global configuration */
+   I40E_WRITE_REG(hw, I40E_GLQF_ORT(33), 0x);
+   I40E_WRITE_REG(hw, I40E_GLQF_ORT(34), 0x);
+   I40E_WRITE_REG(hw, I40E_GLQF_ORT(35), 0x);
+   }
+}
+
 static int
 eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 {
@@ -1478,6 +1485,9 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
hw->fc.requested_mode = I40E_FC_NONE;
i40e_set_fc(hw, &aq_fail, TRUE);
 
+   /* Disable flexible payload in global configuration */
+   i40e_flex_payload_reg_cfg(hw, false);
+
/* uninitialize pf host driver */
i40e_pf_host_uninit(dev);
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index cd67453..2cbe9cb 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1198,6 +1198,7 @@ int i40e_dcb_init_configure(struct rte_eth_dev *dev, bool 
sw_dcb);
 int i40e_flush_queue_region_all_conf(struct rte_eth_dev *dev,
struct i40e_hw *hw, struct i40e_pf *pf, uint16_t on);
 void i40e_init_queue_region_conf(struct rte_eth_dev *dev);
+void i40e_flex_payload_reg_cfg(struct i40e_hw *hw, bool enable);
 
 #define I40E_DEV_TO_PCI(eth_dev) \
RTE_DEV_TO_PCI((eth_dev)->device)
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 3d7170d..0ae8fbd 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -671,6 +671,7 @@ i40e_fdir_configure(struct rte_eth_dev *dev)
return -EINVAL;
}
/* configure flex payload */
+   i40e_flex_payload_reg_cfg(hw, !!conf->nb_payloads);
for (i = 0; i < conf->nb_payloads; i++)
i40e_set_flx_pld_cfg(pf, &conf->flex_set[i]);
/* configure flex mask*/
-- 
2.5.5

Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: add target queues in flow actions

[Nelio Laranjeiro]

Hi Annob

On Fri, Dec 01, 2017 at 08:34:04PM +0530, Anoob Joseph wrote:

> > 
> > I think we have an agreement here, in order the final action to be
> > tested:
> > 
> >   1. PASSTHRU
> >   2. RSS
> >   3. QUEUE
> > 
> > If those 3 fails, the functions fails to create the rule, the first
> > succeeding is the one applied.
> PASSTHRU itself is non-terminating, right? So I'm not sure, how a check with
> PASSTHRU would help us. SECURITY will be terminating action on Cavium
> hardware. So, the first check could be without any addition. If that fails,
> RSS. And then QUEUE. That should be fine.

Agreed, assuming PASSTHRU [1] is not mandatory (i.e. explicit) when the
rule action list is not terminal.

A small suggestion, you should accept/ignore such action in your PMD,
the API also allows applications to add it.

Thanks,

[1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#action-types

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH] mbuf: cleanup rte_pktmbuf_lastseg(), remove useless variable

[Olivier MATZ]

Hi Ilya,

On Tue, Nov 14, 2017 at 05:44:49PM +0400, Ilya Matveychikov wrote:
> Fixes: af75078fece3 ("first public release")
> Cc: intel.com

The Cc tag seems wrong, should be removed.

> Signed-off-by: Ilya V. Matveychikov 
> ---
> 
> There is no reason to have local variable m2 or am I wrong?

Yes, you're right. Can you please say it in the commit log?

Thanks,
Olivier

> 
>  lib/librte_mbuf/rte_mbuf.h | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 7e326bbc2..be79e3728 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1538,12 +1538,10 @@ static inline uint16_t rte_pktmbuf_tailroom(const 
> struct rte_mbuf *m)
>   */
>  static inline struct rte_mbuf *rte_pktmbuf_lastseg(struct rte_mbuf *m)
>  {
> - struct rte_mbuf *m2 = (struct rte_mbuf *)m;
> -
>   __rte_mbuf_sanity_check(m, 1);
> - while (m2->next != NULL)
> - m2 = m2->next;
> - return m2;
> + while (m->next != NULL)
> + m = m->next;
> + return m;
>  }
> 
>  /**
> --
> 2.15.0

Re: [dpdk-dev] [PATCH 2/2] mbuf: reset nb_segs of chained packet

[Olivier MATZ]

Hi Ilya,

On Thu, Nov 16, 2017 at 11:15:18PM +0400, Ilya Matveychikov wrote:
> 
> > On Nov 16, 2017, at 9:01 PM, Stephen Hemminger  
> > wrote:
> > 
> > On Thu, 16 Nov 2017 18:05:35 +0400
> > Ilya Matveychikov  wrote:
> > 
> >> Fixes: 139debc42dc0 ("mbuf: move chaining from ip_frag library")
> >> Cc: simon.kagst...@netinsight.net
> >> 
> >> Signed-off-by: Ilya V. Matveychikov 
> >> ---
> >> lib/librte_mbuf/rte_mbuf.h | 5 -
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> >> index ce8a05ddf..2126dc94b 100644
> >> --- a/lib/librte_mbuf/rte_mbuf.h
> >> +++ b/lib/librte_mbuf/rte_mbuf.h
> >> @@ -1828,9 +1828,12 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf 
> >> *head, struct rte_mbuf *tail
> >>head->nb_segs += tail->nb_segs;
> >>head->pkt_len += tail->pkt_len;
> >> 
> >> -  /* pkt_len is only set in the head */
> >> +  /* nb_segs and pkt_len are only set in the head */
> >> +  tail->nb_segs = 1;
> >>tail->pkt_len = tail->data_len;
> >> 
> >> +  __rte_mbuf_sanity_check(head, 1);
> >> +
> >>return 0;
> >> }
> > 
> > My understanding is that nb_segs and pkt_len are only valid
> > in head. For other packets in the chain nb_segs and pkt_len
> > can be anything.
> 
> So why not to keep them in consistency with multi-seg logic?
> I mean that pkt_len/nb_segs for the head always have meaning but for
> the rest of chain pkt_len is the same as data_len and nb_segs := 1
> 

Stephen is right: like most mbuf fields, nb_segs and pkt_len are only
valid for the first mbuf of the chain.

What would be the advantage of changing this?
In addition, I think it would require to do the same change that in many
places, like drivers that build multi-seg mbufs.

If you are fixing an issue, please describe it in the commit log.

Olivier

Re: [dpdk-dev] [PATCH 1/2] mbuf: check sanity of data_len and pkt_len as well

[Olivier MATZ]

Hi Ilya,

On Thu, Nov 16, 2017 at 06:04:43PM +0400, Ilya Matveychikov wrote:
> Signed-off-by: Ilya V. Matveychikov 

Please, add a commit log.

> ---
>  lib/librte_mbuf/rte_mbuf.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 7543662f7..491685c36 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -202,8 +202,7 @@ rte_pktmbuf_pool_create(const char *name, unsigned n,
>  void
>  rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
>  {
> - const struct rte_mbuf *m_seg;
> - unsigned int nb_segs;
> + unsigned int nb_segs, pkt_len;
> 
>   if (m == NULL)
>   rte_panic("mbuf is NULL\n");
> @@ -220,18 +219,26 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int 
> is_header)
>   if ((cnt == 0) || (cnt == UINT16_MAX))
>   rte_panic("bad ref cnt\n");
> 
> + /* data_len supposed to be not more than pkt_len */
> + if (m->data_len > m->pkt_len)
> + rte_panic("bad data_len\n");
> +

This check should only be done if is_header == 1.

Re: [dpdk-dev] rte_ctrlmbuf_init() and CTRL_MBUF_FLAG are not used - shouldn't they be removed and deprecated ?

[Olivier MATZ]

Hi Kevin,

On Sat, Nov 25, 2017 at 01:43:17PM +0200, Kevin Wilson wrote:
> Hi all,
> 
> >Let's see if someone complains, and if no, we can plan to remove it
> >for 17.11.
> 
> Well, unless I missed it, I did not encounter any complaints, and
> 17.11 was already released.
> And the status is of course the same (meaning that in the DPDK tree it
> is not used).
> Maybe now that 17.11 was just released several days ago, it is a good
> time to start deprecation
> process for these control MBUF stuff ? I know for sure that for DPDK
> newbies who read the programmer's guide
> and encounter it, this is a bit confusing (like in
> 
> "2.3.3. Network Packet Buffer Management (librte_mbuf)"
> in
>  
> http://dpdk.org/doc/guides/prog_guide/overview.html#network-packet-buffer-management-librte-mbuf)
> ...
> ...
> "This library provides an API to allocate/free mbufs, manipulate
> control message buffers (ctrlmbuf) which
> are generic message buffers, and packet buffers (pktmbuf) which are
> used to carry network packets."
> ...
> ...
> and of course also confusing those who delve into the code.
> 

I agree.
I'll send a RFC soon, and a deprecation notice for 18.02, planning
a removal for 18.05.

Thanks for spotting it again.

Olivier

Re: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of device stop/start

[Fischetti, Antonio]

Hi All,
I've got an update on this.
I could replicate the same issue by using testpmd + a VM (= Virtual Machine).

The test topology I'm using is:


[Traffic gen][NIC port #0][testpmd][vhost port #2]+
  | 
  |
 [testpmd in
the VM]
  |
  |
[Traffic gen][NIC port #1][testpmd][vhost port #3]+


So there's no OvS now in the picture: one testpmd running on the host
and one testpmd running on the VM.

The issue is that no packet goes through testpmd in the VM.
It seems this is happening after this patch was upstreamed.

Please note
---
To replicate this issue both the next 2 conditions must be met:
 - the traffic is already being sent before launching testpmd in the VM
 - there are at least 2 forwarding lcores.

I found that a way to fix this is:

diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index c3a536f..17dd87d 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -80,7 +80,7 @@ virtqueue_flush(struct virtqueue *vq)
rte_pktmbuf_free(dxp->cookie);
dxp->cookie = NULL;
}
-   vq->vq_used_cons_idx++;
+   //vq->vq_used_cons_idx++;
vq_ring_free_chain(vq, desc_idx);
}

or alternatively by using a commit before 

commit d8227497ec5c3de75fe378e09fc9673ae097fa73
Author: Tiwei Bie 
Date:   Fri Oct 20 10:09:28 2017 +0800

net/virtio: flush Rx queues on start


Other details of my testbench below.

- testpmd on the host has 2 forwarding lcores: #2 and #3 
(otherwise the issue doesn't show up with 1 fwd lcore).

- the VM (qemu) is using lcore #10

- the VM has 2 vhost ports, with 1 queue each

- in this case testpmd on the host uses vhostuser ports. 
In the other testbench with OvS - see previous email - OvS 
was using vhostuserclient ports and the VM was acting as a 
vhost server. The issue happens in both cases.

For further info, please let me know.


Thanks,
Antonio


> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Fischetti, Antonio
> Sent: Tuesday, November 14, 2017 5:39 PM
> To: Bie, Tiwei ; dev@dpdk.org
> Cc: y...@fridaylinux.org; maxime.coque...@redhat.com;
> jfreim...@redhat.com; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior
> of device stop/start
> 
> Hi Tiwei,
> 
> I'm doing some regression tests with v17.11-rc4. I ran
> into a hitch with testpmd running into a guest VM. It happens
> that no packet gets forwarded by testpmd.
> The issue seems to appear after this patch was upstreamed.
> 
> I saw there's a way to make it work, ie by avoiding to
> increment the last consumed descriptor:
> 
> --- a/drivers/net/virtio/virtqueue.c
> +++ b/drivers/net/virtio/virtqueue.c
> @@ -80,7 +80,7 @@ virtqueue_flush(struct virtqueue *vq)
> rte_pktmbuf_free(dxp->cookie);
> dxp->cookie = NULL;
> }
> -   vq->vq_used_cons_idx++;
> +   //vq->vq_used_cons_idx++;
> vq_ring_free_chain(vq, desc_idx);
> 
> Not quite sure if this change make any sense to you?
> 
> Some details below.
> 
> The issue appears only if the traffic generator is already
> sending packets before I launch testpmd in the guest.
> 
> In my testbench I have Open-vSwitch (OvS-DPDK) which launches
> a VM with 2 vhostuserclient ports (vhu0 and vhu1), each with
> a single queue.
> My OvS has 2 physical ports: dpdk0 and dpdk1.
> dpdk0 forwards packets back and forth from/to the generator
> to/from vhu0.
> Similarly, dpdk1 forwards packets back and forth from/to the generator
> to/from vhu1.
> 
> In OvS there are 2 different PMD threads serving the 2
> vhostuserclient ports.
> 
> While the traffic generator is already sending packets, in the
> guest VM I launch
>   ./testpmd -c 0x3 -n 4 --socket-mem 512 -- --burst=64 -i --
> txqflags=0xf00 --disable-hw-vlan
> 
> The issue is that I see no packet received on the traffic generator
> and in fact testpmd shows
> 
> -- Forward statistics for port 0  --
> 
>   RX-packets: 0  RX-dropped: 0 RX-total: 0
>   TX-packets: 0  TX-dropped: 0 TX-total: 0
>   --
> --
> 
>   -- Forward statistics for port 1  
> --
>   RX-packets: 0  RX-dropped: 0 RX-total: 0
>   TX-packets: 0  TX-dropped: 0 TX-total: 0
>   

Re: [dpdk-dev] [PATCH 2/2] eal/x86: Use lock-prefixed instructions to reduce cost of rte_smp_mb()

[Stephen Hemminger]

On Fri,  1 Dec 2017 11:12:51 +
Konstantin Ananyev  wrote:

> On x86 it  is possible to use lock-prefixed instructions to get
> the similar effect as mfence.
> As pointed by Java guys, on most modern HW that gives a better
> performance than using mfence:
> https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
> That patch adopts that technique for rte_smp_mb() implementation.
> On BDW 2.2 mb_autotest on single lcore reports 2X cycle reduction,
> i.e. from ~110 to ~55 cycles per operation.
> 
> Signed-off-by: Konstantin Ananyev 
> ---
>  .../common/include/arch/x86/rte_atomic.h   | 45 
> +-
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h 
> b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> index 4eac66631..07b7fa7f7 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> @@ -55,12 +55,53 @@ extern "C" {
>  
>  #define  rte_rmb() _mm_lfence()
>  
> -#define rte_smp_mb() rte_mb()
> -
>  #define rte_smp_wmb() rte_compiler_barrier()
>  
>  #define rte_smp_rmb() rte_compiler_barrier()
>  
> +/*
> + * From Intel Software Development Manual; Vol 3;
> + * 8.2.2 Memory Ordering in P6 and More Recent Processor Families:
> + * ...
> + * . Reads are not reordered with other reads.
> + * . Writes are not reordered with older reads.
> + * . Writes to memory are not reordered with other writes,
> + *   with the following exceptions:
> + *   . streaming stores (writes) executed with the non-temporal move
> + * instructions (MOVNTI, MOVNTQ, MOVNTDQ, MOVNTPS, and MOVNTPD); and
> + *   . string operations (see Section 8.2.4.1).
> + *  ...
> + * . Reads may be reordered with older writes to different locations but not
> + * with older writes to the same location.
> + * . Reads or writes cannot be reordered with I/O instructions,
> + * locked instructions, or serializing instructions.
> + * . Reads cannot pass earlier LFENCE and MFENCE instructions.
> + * . Writes ... cannot pass earlier LFENCE, SFENCE, and MFENCE instructions.
> + * . LFENCE instructions cannot pass earlier reads.
> + * . SFENCE instructions cannot pass earlier writes ...
> + * . MFENCE instructions cannot pass earlier reads, writes ...
> + *
> + * As pointed by Java guys, that makes possible to use lock-prefixed
> + * instructions to get the same effect as mfence and on most modern HW
> + * that gives a better perfomarnce than using mfence:
> + * https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
> + * So below we use that technique for rte_smp_mb() implementation.
> + */
> +
> +#ifdef RTE_ARCH_I686
> +#define  RTE_SP  RTE_STR(esp)
> +#else
> +#define  RTE_SP  RTE_STR(rsp)
> +#endif
> +
> +#define RTE_MB_DUMMY_MEMP"-128(%%" RTE_SP ")"
> +
> +static __rte_always_inline void
> +rte_smp_mb(void)
> +{
> + asm volatile("lock addl $0," RTE_MB_DUMMY_MEMP "; " ::: "memory");
> +}
> +
>  #define rte_io_mb() rte_mb()
>  
>  #define rte_io_wmb() rte_compiler_barrier()

The lock instruction is a stronger barrier than the compiler barrier
and has worse performance impact. Are you sure it is necessary to use it in 
DPDK.
Linux kernel has successfully used simple compiler reodering barrier for years.

Don't confuse rte_smp_mb with the required barrier for talking to I/O devices.

[dpdk-dev] [PATCH 2/2] doc: Fix typo

[Roy Franz]

Fix trivial typo (an -> and) in description of service core masks.

Signed-off-by: Roy Franz 
---
 doc/guides/prog_guide/service_cores.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/service_cores.rst 
b/doc/guides/prog_guide/service_cores.rst
index 3a029ba9c..5a8d8941a 100644
--- a/doc/guides/prog_guide/service_cores.rst
+++ b/doc/guides/prog_guide/service_cores.rst
@@ -55,7 +55,7 @@ Service Core Initialization
 There are two methods to having service cores in a DPDK application, either by
 using the service coremask, or by dynamically adding cores using the API.
 The simpler of the two is to pass the `-s` coremask argument to EAL, which will
-take any cores available in the main DPDK coremask, an if the bits are also set
+take any cores available in the main DPDK coremask, and if the bits are also 
set
 in the service coremask the cores become service-cores instead of DPDK
 application lcores.
 
-- 
2.11.0

[dpdk-dev] [PATCH 1/2] doc: Clarify wording regarding actions and flow rules.

[Roy Franz]

Current wording regarding actions and flow rules doesn't make sense.

Signed-off-by: Roy Franz 
---
Maybe a better word than 'assigned' could be chosen?  (attached,
 associated with, etc)  I'm happy to spin this if needed.


 doc/guides/prog_guide/rte_flow.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index d158be5e4..9de6d32d3 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -984,7 +984,7 @@ Actions
 ~~~
 
 Each possible action is represented by a type. Some have associated
-configuration structures. Several actions combined in a list can be affected
+configuration structures. Several actions combined in a list can be assigned
 to a flow rule. That list is not ordered.
 
 They fall in three categories:
-- 
2.11.0

[dpdk-dev] [PATCH 0/4] dpdk: enhance EXPERIMENTAL api tagging

[Neil Horman]

Hey all-
A few days ago, I was lamenting the fact that, when reviewing patches I
would frequently complain about ABI changes that were actually considered safe
because they were part of the EXPERIMENTAL api set.  John M. asked me then what
I might do to improve the situation, and the following patch set is a proposal
that I've come up with.

In thinking about the problem I identified two issues that I think we
can improve on in this area:

1) Make experimental api calls more visible in the source code.  That is to say,
when reviewing patches, it would be nice to have some sort of visual reference
that indicates that the changes being made are part of an experimental API and
therefore ABI concerns need not be addressed

2) Make experimenal api usage more visible to consumers of the DPDK, so that
they can make a more informed decision about the API's they consume in their
application.  We make an effort to document all the experimental API's, but
there is no guarantee that a user will check the documentation before making use
of a new library.

This patch set attempts to achieve both of the above goals.  To do this I've
added an __experimental macro tag, suitable for inserting into api forward
declarations and definitions.  

The presence of the tag in the header and c files where the api code resides
increases the likelyhood that any patch submitted against them will include the
tag in the context, making it clear to reviewers that ABI stability isn't a
concern here.


Also, This tag marks each function it is used on with an attibute causing any
use of the fuction to emit a warning during the build
with a message indicating that the API call in question is not yet part of the
stable interface.  Developers can then make an informed decision to suppress
that warning or not.

Because there is internal use of several experimental API's, this set also
includes a new override macro ALLOW_EXPERIMENTAL_FUNCTIONS to automatically
suprress these warnings.  I think its fair to assume that, for internal use, we
almost always want to suppress these warnings, as by definition any change to
the apis (even their removal) must be done in parallel with an appropriate
change in the calling locations, lest the dpdk build itself break.

Neil

[dpdk-dev] [PATCH 4/4] Makefiles: Add experimental tag check and warnings to trigger on use

[Neil Horman]

Add checks during build to ensure that all symbols in the EXPERIMENTAL
version map section have __experimental tags on their definitions, and
enable the warnings needed to announce their use.  Also add a
ALLOW_EXPERIMENTAL_FUNCTIONS variable check to allow for in-tree dpdk
libraries to override those checks.

Signed-off-by: Neil Horman 
CC: Thomas Monjalon 
CC: "Mcnamara, John" 
---
 app/test-eventdev/Makefile |  2 ++
 app/test-pmd/Makefile  |  2 ++
 drivers/event/sw/Makefile  |  2 ++
 drivers/net/failsafe/Makefile  |  2 ++
 drivers/net/ixgbe/Makefile |  2 ++
 examples/eventdev_pipeline_sw_pmd/Makefile |  2 ++
 examples/flow_classify/Makefile|  2 ++
 examples/ipsec-secgw/Makefile  |  2 ++
 examples/service_cores/Makefile|  2 ++
 lib/librte_eal/linuxapp/Makefile   |  2 ++
 lib/librte_eal/linuxapp/eal/Makefile   |  4 
 lib/librte_eventdev/Makefile   |  3 +++
 lib/librte_security/Makefile   |  3 +++
 mk/internal/rte.compile-pre.mk | 14 --
 mk/toolchain/clang/rte.vars.mk |  2 +-
 mk/toolchain/gcc/rte.vars.mk   |  2 +-
 mk/toolchain/icc/rte.vars.mk   |  4 ++--
 17 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/app/test-eventdev/Makefile b/app/test-eventdev/Makefile
index dcb2ac476..e68870828 100644
--- a/app/test-eventdev/Makefile
+++ b/app/test-eventdev/Makefile
@@ -32,6 +32,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 APP = dpdk-test-eventdev
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 
diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index d21308fcd..f0262a369 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -33,6 +33,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
 
 ifeq ($(CONFIG_RTE_TEST_PMD),y)
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 #
 # library name
 #
diff --git a/drivers/event/sw/Makefile b/drivers/event/sw/Makefile
index 2f2b67bac..0711ce46e 100644
--- a/drivers/event/sw/Makefile
+++ b/drivers/event/sw/Makefile
@@ -33,6 +33,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # library name
 LIB = librte_pmd_sw_event.a
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 # build flags
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
diff --git a/drivers/net/failsafe/Makefile b/drivers/net/failsafe/Makefile
index ea2a8fe46..795da9d56 100644
--- a/drivers/net/failsafe/Makefile
+++ b/drivers/net/failsafe/Makefile
@@ -36,6 +36,8 @@ LIB = librte_pmd_failsafe.a
 
 EXPORT_MAP := rte_pmd_failsafe_version.map
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 LIBABIVER := 1
 
 # Sources are stored in SRCS-y
diff --git a/drivers/net/ixgbe/Makefile b/drivers/net/ixgbe/Makefile
index 511a64eb0..b8431600a 100644
--- a/drivers/net/ixgbe/Makefile
+++ b/drivers/net/ixgbe/Makefile
@@ -36,6 +36,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
 #
 LIB = librte_pmd_ixgbe.a
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 
diff --git a/examples/eventdev_pipeline_sw_pmd/Makefile 
b/examples/eventdev_pipeline_sw_pmd/Makefile
index de4e22c88..4272caf93 100644
--- a/examples/eventdev_pipeline_sw_pmd/Makefile
+++ b/examples/eventdev_pipeline_sw_pmd/Makefile
@@ -32,6 +32,8 @@ ifeq ($(RTE_SDK),)
 $(error "Please define RTE_SDK environment variable")
 endif
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 # Default target, can be overridden by command line or environment
 RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
diff --git a/examples/flow_classify/Makefile b/examples/flow_classify/Makefile
index eecdde14c..b04747c40 100644
--- a/examples/flow_classify/Makefile
+++ b/examples/flow_classify/Makefile
@@ -33,6 +33,8 @@ ifeq ($(RTE_SDK),)
 $(error "Please define RTE_SDK environment variable")
 endif
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 # Default target, can be overridden by command line or environment
 RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
diff --git a/examples/ipsec-secgw/Makefile b/examples/ipsec-secgw/Makefile
index 9fd33cb7f..90a4feca1 100644
--- a/examples/ipsec-secgw/Makefile
+++ b/examples/ipsec-secgw/Makefile
@@ -33,6 +33,8 @@ ifeq ($(RTE_SDK),)
$(error "Please define RTE_SDK environment variable")
 endif
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 # Default target, can be overridden by command line or environment
 RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
diff --git a/examples/service_cores/Makefile b/examples/service_cores/Makefile
index bd4a345dc..ba731e259 100644
--- a/examples/service_cores/Makefile
+++ b/examples/service_cores/Makefile
@@ -32,6 +32,8 @@ ifeq ($(RTE_SDK),)
 $(error "Please define RTE_SDK environment variable")
 endif
 
+ALLOW_EXPERIMENTAL_FUNCTIONS := 1
+
 # Default target, can be overridden by command line or environment
 RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
diff --git a/lib/librte_eal/linuxapp/Makefile b/lib/librte_eal/linuxapp/Makefile
index 2ebdf3139..11ee59e42 100644
--- a/lib/librte_eal/linuxapp/Makefile
+++ b/lib/librte_eal/l

[dpdk-dev] [PATCH 1/4] buildtools: Add tool to check EXPERIMENTAL api exports

[Neil Horman]

This tools reads the given version map for a directory, and checks to ensure
that, for each symbol listed in the export list, the corresponding definition is
tagged as __experimental, erroring out if its not.  In this way, we can ensure
that the EXPERIMENTAL api is kept in sync with the tags

Signed-off-by: Neil Horman 
CC: Thomas Monjalon 
CC: "Mcnamara, John" 
---
 buildtools/experimentalsyms.sh | 34 ++
 1 file changed, 34 insertions(+)
 create mode 100755 buildtools/experimentalsyms.sh

diff --git a/buildtools/experimentalsyms.sh b/buildtools/experimentalsyms.sh
new file mode 100755
index 0..dc01f71ba
--- /dev/null
+++ b/buildtools/experimentalsyms.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+MAPFILE=$1
+OBJFILE=$2
+
+if [ -d $MAPFILE ]
+then
+   exit 0
+fi
+
+if [ -d $OBJFILE ]
+then
+   exit 0
+fi
+
+for i in `awk 'BEGIN {found=0}
+   /.*EXPERIMENTAL.*/ {found=1}
+   /.*}.*;/ {found=0}
+   /.*;/ {if (found == 1) print $1}' $MAPFILE`
+do
+   SYM=`echo $i | sed -e"s/;//"`
+   objdump -t $OBJFILE | grep -q "\.text.*$SYM"
+   IN_TEXT=$?
+   objdump -t $OBJFILE | grep -q "\.text\.experimental.*$SYM"
+   IN_EXP=$?
+   if [ $IN_TEXT -eq 0 -a $IN_EXP -ne 0 ]
+   then
+   echo "$SYM is not flagged as experimental, but is listed in 
version map"
+   echo "Please add __experimental to the definition of $SYM"
+   exit 1
+   fi  
+done
+exit 0
+
-- 
2.14.3

[dpdk-dev] [PATCH 3/4] dpdk: add __experimental tag to appropriate api calls

[Neil Horman]

Append the __experimental tag to api calls appearing in the EXPERIMENTAL
section of their libraries version map

Signed-off-by: Neil Horman 
CC: Thomas Monjalon 
CC: "Mcnamara, John" 
---
 lib/librte_eal/common/eal_common_dev.c |  5 ++-
 lib/librte_eal/common/eal_common_devargs.c |  7 ++--
 lib/librte_eal/common/include/rte_dev.h|  5 ++-
 lib/librte_eal/common/include/rte_devargs.h|  7 ++--
 lib/librte_eal/common/include/rte_service.h| 40 +-
 .../common/include/rte_service_component.h | 10 ++---
 lib/librte_eal/common/rte_service.c| 49 +++---
 lib/librte_eal/linuxapp/eal/eal.c  |  1 +
 lib/librte_ether/rte_mtr.c | 25 +--
 lib/librte_ether/rte_mtr.h | 26 ++--
 lib/librte_flow_classify/rte_flow_classify.c   | 13 +++---
 lib/librte_flow_classify/rte_flow_classify.h   | 11 ++---
 lib/librte_security/rte_security.c | 16 +++
 lib/librte_security/rte_security.h | 23 +-
 14 files changed, 124 insertions(+), 114 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c 
b/lib/librte_eal/common/eal_common_dev.c
index dda8f5835..8a46ccab6 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -133,7 +134,7 @@ full_dev_name(const char *bus, const char *dev, const char 
*args)
return name;
 }
 
-int rte_eal_hotplug_add(const char *busname, const char *devname,
+int __experimental rte_eal_hotplug_add(const char *busname, const char 
*devname,
const char *devargs)
 {
struct rte_bus *bus;
@@ -203,7 +204,7 @@ int rte_eal_hotplug_add(const char *busname, const char 
*devname,
return ret;
 }
 
-int rte_eal_hotplug_remove(const char *busname, const char *devname)
+int __experimental rte_eal_hotplug_remove(const char *busname, const char 
*devname)
 {
struct rte_bus *bus;
struct rte_device *dev;
diff --git a/lib/librte_eal/common/eal_common_devargs.c 
b/lib/librte_eal/common/eal_common_devargs.c
index 6ac88d6ab..8e46d54bd 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -85,7 +86,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name)
return strncmp(bus->name, name, strlen(bus->name));
 }
 
-int
+int __experimental
 rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
 {
struct rte_bus *bus = NULL;
@@ -139,7 +140,7 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs 
*da)
return 0;
 }
 
-int
+int __experimental
 rte_eal_devargs_insert(struct rte_devargs *da)
 {
int ret;
@@ -188,7 +189,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char 
*devargs_str)
return -1;
 }
 
-int
+int __experimental
 rte_eal_devargs_remove(const char *busname, const char *devname)
 {
struct rte_devargs *d;
diff --git a/lib/librte_eal/common/include/rte_dev.h 
b/lib/librte_eal/common/include/rte_dev.h
index 9342e0cbd..e3e52dcb8 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -49,6 +49,7 @@ extern "C" {
 #include 
 #include 
 
+#include 
 #include 
 
 __attribute__((format(printf, 2, 0)))
@@ -209,7 +210,7 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @return
  *   0 on success, negative on error.
  */
-int rte_eal_hotplug_add(const char *busname, const char *devname,
+int __experimental rte_eal_hotplug_add(const char *busname, const char 
*devname,
const char *devargs);
 
 /**
@@ -225,7 +226,7 @@ int rte_eal_hotplug_add(const char *busname, const char 
*devname,
  * @return
  *   0 on success, negative on error.
  */
-int rte_eal_hotplug_remove(const char *busname, const char *devname);
+int __experimental rte_eal_hotplug_remove(const char *busname, const char 
*devname);
 
 /**
  * Device comparison function.
diff --git a/lib/librte_eal/common/include/rte_devargs.h 
b/lib/librte_eal/common/include/rte_devargs.h
index 58d585df6..7148949c9 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -50,6 +50,7 @@ extern "C" {
 
 #include 
 #include 
+#include 
 #include 
 
 /**
@@ -136,7 +137,7 @@ int rte_eal_parse_devargs_str(const char *devargs_str,
  *   - 0 on success.
  *   - Negative errno on error.
  */
-int
+int __experimental
 rte_eal_devargs_parse(const char *dev,
  struct rte_devargs *da);
 
@@ -150,7 +151,7 @@ rte_eal_devargs_parse(const char *dev,
  *   - 0 on success
  *   - Negative on error.
  */
-int
+int __experimental
 rte_eal_devargs_insert(struct rte_devargs *da);
 
 /**
@@ -193,7 +194,7 @@ int rte_eal_devargs_add(enum rte_de

[dpdk-dev] [PATCH 2/4] compat: Add __experimental macro

[Neil Horman]

The __experimental macro tags a given exported function as being part of
the EXPERIMENTAL api.  Use of this tag will cause any caller of the
function (that isn't removed by dead code elimination) to emit a warning
that the user is making use of an API whos stabilty isn't guaranteed.
It also places the function in the .text.experimental section, which is
used to validate the tag against the corresponding library version map

Signed-off-by: Neil Horman 
CC: Thomas Monjalon 
CC: "Mcnamara, John" 
---
 lib/librte_compat/rte_compat.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index 41e8032ba..c7c967d86 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -101,5 +101,7 @@
  */
 #endif
 
+#define __experimental \
+__attribute__(( deprecated("Symbol is not yet part of stable abi"), 
section(".text.experimental") ))
 
 #endif /* _RTE_COMPAT_H_ */
-- 
2.14.3

Re: [dpdk-dev] [RFC v1] doc compression API for DPDK

[Trahe, Fiona]

> -Original Message-
> From: Verma, Shally [mailto:shally.ve...@cavium.com]
> Sent: Thursday, November 30, 2017 11:13 AM
> To: Trahe, Fiona ; dev@dpdk.org
> Cc: Athreya, Narayana Prasad ; Challa, 
> Mahipal
> 
> Subject: Re: [RFC v1] doc compression API for DPDK
> 
> HI Fiona
> 
> > -Original Message-
> > From: Trahe, Fiona [mailto:fiona.tr...@intel.com]
> > Sent: 28 November 2017 00:25
> > To: Verma, Shally ; dev@dpdk.org; Athreya,
> > Narayana Prasad ; Challa, Mahipal
> > ; De Lara Guarch, Pablo
> > 
> > Cc: Trahe, Fiona 
> > Subject: RE: [RFC v1] doc compression API for DPDK
> >
> > Hi Shally,
> >
> > > -Original Message-
> > > From: Verma, Shally [mailto:shally.ve...@cavium.com]
> > > Sent: Tuesday, October 31, 2017 11:39 AM
> > > To: dev@dpdk.org; Trahe, Fiona ; Athreya,
> > Narayana Prasad
> > > ; Challa, Mahipal
> > 
> > > Subject: [RFC v1] doc compression API for DPDK
> > >
> > > HI Fiona
> > >
> > > This is an RFC document to brief our understanding and requirements on
> > compression API proposal in
> > > DPDK. It is based on "[RFC] Compression API in DPDK
> > http://dpdk.org/ml/archives/dev/2017-
> > > October/079377.html".
> > > Intention of this document is to align on concepts built into compression
> > API, its usage and identify further
> > > requirements.
> > >
> > > Going further it could be a base to Compression Module Programmer
> > Guide.
> > >
> > > Current scope is limited to
> > > - definition of the terminology which makes up foundation of compression
> > API
> > > - typical API flow expected to use by applications
> > >
> > > Overview
> > > 
> > > A. Notion of a session in compression API
> > > ==
> > > A Session is per device logical entity which is setup with chained-xforms 
> > > to
> > be performed on burst
> > > operations where individual entry contains operation type
> > (decompress/compress) and related parameter.
> > > A typical Session parameter includes:
> > > - compress / decompress
> > > - dev_id
> > > - compression algorithm and other related parameters
> > > - mempool - for use by session for runtime requirement
> > > - and any other associated private data maintained by session
> > >
> > > Application can setup multiple sessions on a device as dictated by
> > dev_info.nb_sessions or
> > > nb_session_per_qp.
> > >
> > [Fiona] The session design is modelled on the cryptodev session design and
> > so allows
> > to create a session which can be used on different driver types. E.g.  a
> > session could be set up and initialised
> > to run on a QuickAssist device and a Software device. This may be useful for
> > stateless
> > requests, and enable load-balancing. For stateful flows the session should 
> > be
> > set up for
> > only one specific driver-type as the state information will be stored in the
> > private data specific to the driver-type
> > and not transferrable between driver-types.
> > So a session
> >  - is not per-device
> >  - has no dev_id
> >  - has no mempool stored in it - the pool is created by the application, 
> > the lib
> > can retrieve the pool from the object with rte_mempool_from_obj()
> >  - does not have a limit number per device, just per qp, i.e. there is no
> > dev_info.nb_sessions, just dev_info.max_nb_sessions_per_qp
> >
> > Do you think any of this needs to be changed?
> 
> [Shally] Please help confirm following before I could answer this.
> 
> In cryptodev, session holds an drivers array initialized on it where each can 
> be setup to perform
> same/different operation in its private_data.
> On mapping it to compression it mean, a session:
> - Will not retain any of the info as mentioned above (xform, mempool, algos 
> et el). All such information is
> maintained as part of associated device driver private data.
[Fiona] exactly

> - App can use same session to set compress xform and decompress xform on 
> devices but if both devices
> maps to same driver_id then only either is effective (whichever is set first)?
[Fiona] No,  the intention is that the session is initialised for all drivers, 
and so for all devices, using the same xform. So it should only be initialised 
to either compress or decompress. 
The intent being that an application can prepare an operation (stateless) 
independently of the driver & device it's targeting, and then choose where to 
send it, possibly based on which device is not busy.


> 
> Is this understanding correct?
> 
> >
> > > B. Notion of burst operations in compression API
> > >  ===
> > > struct rte_comp_op defines compression/decompression operational
> > parameter and makes up one single
> > > element of burst. This is both an input/output parameter.
> > > PMD gets source, destination and checksum information at input and
> > updated it with bytes consumed and
> > > produced at output.
> > [Fiona] Agreed
> >
> > > Once enqueued for processing, rte_comp_op *cannot be reused* until its
> > status is set 

[dpdk-dev] [RFC PATCH v5 3/5] eventtimer: add config variable for adapter

[Erik Gabriel Carrillo]

This commit introduces a configuration variable that can
be used to enable or disable compilation of the event timer
adapter.

Signed-off-by: Erik Gabriel Carrillo 
---
 config/common_base | 1 +
 drivers/event/sw/sw_evdev.c| 4 
 lib/librte_eventdev/Makefile   | 6 +++---
 lib/librte_eventdev/rte_eventdev_pmd.h | 6 ++
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/config/common_base b/config/common_base
index 91a2f0f..09d2a62 100644
--- a/config/common_base
+++ b/config/common_base
@@ -574,6 +574,7 @@ CONFIG_RTE_LIBRTE_EVENTDEV=y
 CONFIG_RTE_LIBRTE_EVENTDEV_DEBUG=n
 CONFIG_RTE_EVENT_MAX_DEVS=16
 CONFIG_RTE_EVENT_MAX_QUEUES_PER_DEV=64
+CONFIG_RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER=y
 CONFIG_RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER_DEBUG=n
 
 #
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index 94da675..69050cf 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -435,6 +435,7 @@ sw_eth_rx_adapter_caps_get(const struct rte_eventdev *dev,
return 0;
 }
 
+#ifdef RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER
 static int
 sw_timer_adapter_caps_get(const struct rte_eventdev *dev,
  uint64_t flags,
@@ -450,6 +451,7 @@ sw_timer_adapter_caps_get(const struct rte_eventdev *dev,
 
return 0;
 }
+#endif
 
 static void
 sw_info_get(struct rte_eventdev *dev, struct rte_event_dev_info *info)
@@ -771,7 +773,9 @@ sw_probe(struct rte_vdev_device *vdev)
 
.eth_rx_adapter_caps_get = sw_eth_rx_adapter_caps_get,
 
+#ifdef RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER
.timer_adapter_caps_get = sw_timer_adapter_caps_get,
+#endif
 
.xstats_get = sw_xstats_get,
.xstats_get_names = sw_xstats_get_names,
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index f3f05c2..2e47fa5 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -45,7 +45,7 @@ LDLIBS += -lrte_eal -lrte_ring -lrte_ethdev -lrte_hash
 SRCS-y += rte_eventdev.c
 SRCS-y += rte_event_ring.c
 SRCS-y += rte_event_eth_rx_adapter.c
-SRCS-y += rte_event_timer_adapter.c
+SRCS-$(CONFIG_RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER) += rte_event_timer_adapter.c
 
 # export include files
 SYMLINK-y-include += rte_eventdev.h
@@ -54,8 +54,8 @@ SYMLINK-y-include += rte_eventdev_pmd_pci.h
 SYMLINK-y-include += rte_eventdev_pmd_vdev.h
 SYMLINK-y-include += rte_event_ring.h
 SYMLINK-y-include += rte_event_eth_rx_adapter.h
-SYMLINK-y-include += rte_event_timer_adapter.h
-SYMLINK-y-include += rte_event_timer_adapter_pmd.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER)-include += 
rte_event_timer_adapter.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER)-include += 
rte_event_timer_adapter_pmd.h
 
 # versioning export map
 EXPORT_MAP := rte_eventdev_version.map
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h 
b/lib/librte_eventdev/rte_eventdev_pmd.h
index 321aef2..91e1f47 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -52,7 +52,9 @@ extern "C" {
 #include 
 
 #include "rte_eventdev.h"
+#ifdef RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER
 #include "rte_event_timer_adapter_pmd.h"
+#endif
 
 /* Logging Macros */
 #define RTE_EDEV_LOG_ERR(...) \
@@ -467,6 +469,7 @@ typedef int (*eventdev_eth_rx_adapter_caps_get_t)
 
 struct rte_event_eth_rx_adapter_queue_conf *queue_conf;
 
+#ifdef RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER
 /**
  * Retrieve the event device's timer adapter capabilities, as well as the ops
  * structure that an event timer adapter should call through to enter the
@@ -497,6 +500,7 @@ typedef int (*eventdev_timer_adapter_caps_get_t)(
uint64_t flags,
uint32_t *caps,
const struct rte_event_timer_adapter_ops **ops);
+#endif
 
 /**
  * Add ethernet Rx queues to event device. This callback is invoked if
@@ -683,8 +687,10 @@ struct rte_eventdev_ops {
eventdev_eth_rx_adapter_stats_reset eth_rx_adapter_stats_reset;
/**< Reset ethernet Rx stats */
 
+#ifdef RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER
eventdev_timer_adapter_caps_get_t timer_adapter_caps_get;
/**< Get timer adapter capabilities */
+#endif
 };
 
 /**
-- 
2.6.4

[dpdk-dev] [RFC PATCH v5 5/5] test: add event timer adapter auto-test

[Erik Gabriel Carrillo]

This commit adds enough test infrastructure to exercise the
allocation/deallocation routines of the event timer adapter library
minimally.

Signed-off-by: Erik Gabriel Carrillo 
---
 test/test/Makefile   |   1 +
 test/test/test_event_timer_adapter.c | 249 +++
 2 files changed, 250 insertions(+)
 create mode 100644 test/test/test_event_timer_adapter.c

diff --git a/test/test/Makefile b/test/test/Makefile
index bb54c98..9448aef 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -210,6 +210,7 @@ ifeq ($(CONFIG_RTE_LIBRTE_EVENTDEV),y)
 SRCS-y += test_eventdev.c
 SRCS-y += test_event_ring.c
 SRCS-y += test_event_eth_rx_adapter.c
+SRCS-y += test_event_timer_adapter.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += test_eventdev_sw.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += test_eventdev_octeontx.c
 endif
diff --git a/test/test/test_event_timer_adapter.c 
b/test/test/test_event_timer_adapter.c
new file mode 100644
index 000..d0ea066
--- /dev/null
+++ b/test/test/test_event_timer_adapter.c
@@ -0,0 +1,249 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "test.h"
+
+/* Example from RFC */
+#define NB_TEST_EVENT_TIMERS 4
+
+static int evdev;
+//struct rte_event_timer_adapter *g_adapter;
+struct rte_event_timer *g_evtimer;
+struct rte_mempool *g_event_timer_pool;
+
+static inline void
+devconf_set_default_sane_values(struct rte_event_dev_config *dev_conf,
+   struct rte_event_dev_info *info)
+{
+   memset(dev_conf, 0, sizeof(struct rte_event_dev_config));
+   dev_conf->dequeue_timeout_ns = info->min_dequeue_timeout_ns;
+   /* Leave a port for the adapter to allocate */
+   dev_conf->nb_event_ports = info->max_event_ports - 1;
+   dev_conf->nb_event_queues = info->max_event_queues;
+   dev_conf->nb_event_queue_flows = info->max_event_queue_flows;
+   dev_conf->nb_event_port_dequeue_depth =
+   info->max_event_port_dequeue_depth;
+   dev_conf->nb_event_port_enqueue_depth =
+   info->max_event_port_enqueue_depth;
+   dev_conf->nb_event_port_enqueue_depth =
+   info->max_event_port_enqueue_depth;
+   dev_conf->nb_events_limit =
+   info->max_num_events;
+}
+
+static int
+configure_event_dev(void)
+{
+   struct rte_event_dev_config devconf;
+   int ret;
+   const char *eventdev_name = "event_sw0";
+   struct rte_event_dev_info info;
+   int i;
+
+   evdev = rte_event_dev_get_dev_id(eventdev_name);
+   if (evdev < 0) {
+   if (rte_vdev_init(eventdev_name, NULL) < 0) {
+   printf("Error creating eventdev\n");
+   return TEST_FAILED;
+   }
+   evdev = rte_event_dev_get_dev_id(eventdev_name);
+   if (evdev < 0) {
+   printf("Error finding newly created eventdev\n");
+   return TEST_FAILED;
+   }
+   }
+
+   ret = rte_event_dev_info_get(evdev, &info);
+   TEST_ASSERT_SUCCESS(ret, "Failed to get event dev info");
+
+   devconf_set_default_sane_values(&devconf, &info);
+
+   ret = rte_event_dev_configure(evdev, &devconf);

[dpdk-dev] [RFC PATCH v5 1/5] eventtimer: introduce event timer adapter

[Erik Gabriel Carrillo]

Signed-off-by: Erik Gabriel Carrillo 
---
 doc/api/doxy-api-index.md |   1 +
 lib/librte_eventdev/Makefile  |   1 +
 lib/librte_eventdev/rte_event_timer_adapter.h | 518 ++
 lib/librte_eventdev/rte_eventdev.h|   4 +-
 4 files changed, 522 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_eventdev/rte_event_timer_adapter.h

diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 3492702..3110658 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -46,6 +46,7 @@ The public API headers are grouped by topics:
   [security]   (@ref rte_security.h),
   [eventdev]   (@ref rte_eventdev.h),
   [event_eth_rx_adapter]   (@ref rte_event_eth_rx_adapter.h),
+  [event_timer_adapter](@ref rte_event_timer_adapter.h),
   [metrics](@ref rte_metrics.h),
   [bitrate](@ref rte_bitrate.h),
   [latency](@ref rte_latencystats.h),
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index 5ac22cd..6ef7c1c 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -53,6 +53,7 @@ SYMLINK-y-include += rte_eventdev_pmd_pci.h
 SYMLINK-y-include += rte_eventdev_pmd_vdev.h
 SYMLINK-y-include += rte_event_ring.h
 SYMLINK-y-include += rte_event_eth_rx_adapter.h
+SYMLINK-y-include += rte_event_timer_adapter.h
 
 # versioning export map
 EXPORT_MAP := rte_eventdev_version.map
diff --git a/lib/librte_eventdev/rte_event_timer_adapter.h 
b/lib/librte_eventdev/rte_event_timer_adapter.h
new file mode 100644
index 000..2a16a77
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_timer_adapter.h
@@ -0,0 +1,518 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright 2017 Cavium, Inc.
+ *   Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef __RTE_EVENT_TIMER_ADAPTER_H__
+#define __RTE_EVENT_TIMER_ADAPTER_H__
+
+/**
+ * @file
+ *
+ * RTE Event Timer Adapter
+ *
+ * An event timer adapter has the following abstract working model:
+ *
+ *   timer_tick_ns
+ *   +
+ *  +---+|
+ *  |   ||
+ *  +---+ bkt 0 +v---+
+ *  |   |   ||
+ *  |   +---+|
+ *  +---+---++---+---+  +---+---+---+---+
+ *  |   ||   |  |   |   |   |   |
+ *  | bkt n || bkt 1 |<-> t0| t1| t2| tn|
+ *  |   ||   |  |   |   |   |   |
+ *  +---+---++---+---+  +---+---+---+---+
+ *  | Timer adapter  |
+ *  +---+---++---+---+
+ *  |   ||   |
+ *  | bkt 4 || bkt 2 |<--- Current bucket
+ *  |   ||   |
+ *  +---+---++---+---+
+ *   |  +---+   |
+ *   |  |   |   |
+ *   +--+ bkt 3 +---+
+ *  |   |
+ *  +---+
+ *
+ * - It has a virtual monotonically increasing 64-bit timer adapter clock based
+ *   on *enum rte_event_timer_adapter_clk_src* clock source. The clock source
+ *   could be a CPU clock, or a platform depended ex

[dpdk-dev] [RFC PATCH v5 2/5] eventtimer: add common code

[Erik Gabriel Carrillo]

This commit adds the logic that is shared by all event timer adapter
drivers; the common code handles instance allocation and some
initialization.

Signed-off-by: Erik Gabriel Carrillo 
---
 config/common_base|   1 +
 drivers/event/sw/sw_evdev.c   |  18 +
 lib/librte_eventdev/Makefile  |   2 +
 lib/librte_eventdev/rte_event_timer_adapter.c | 407 ++
 lib/librte_eventdev/rte_event_timer_adapter_pmd.h | 159 +
 lib/librte_eventdev/rte_eventdev.h|   3 +
 lib/librte_eventdev/rte_eventdev_pmd.h|  35 ++
 lib/librte_eventdev/rte_eventdev_version.map  |  15 +-
 8 files changed, 639 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_eventdev/rte_event_timer_adapter.c
 create mode 100644 lib/librte_eventdev/rte_event_timer_adapter_pmd.h

diff --git a/config/common_base b/config/common_base
index e74febe..91a2f0f 100644
--- a/config/common_base
+++ b/config/common_base
@@ -574,6 +574,7 @@ CONFIG_RTE_LIBRTE_EVENTDEV=y
 CONFIG_RTE_LIBRTE_EVENTDEV_DEBUG=n
 CONFIG_RTE_EVENT_MAX_DEVS=16
 CONFIG_RTE_EVENT_MAX_QUEUES_PER_DEV=64
+CONFIG_RTE_LIBRTE_EVENTDEV_TIMER_ADAPTER_DEBUG=n
 
 #
 # Compile PMD for skeleton event device
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index fd11079..94da675 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -435,6 +435,22 @@ sw_eth_rx_adapter_caps_get(const struct rte_eventdev *dev,
return 0;
 }
 
+static int
+sw_timer_adapter_caps_get(const struct rte_eventdev *dev,
+ uint64_t flags,
+ uint32_t *caps,
+ const struct rte_event_timer_adapter_ops **ops)
+{
+   RTE_SET_USED(dev);
+   RTE_SET_USED(flags);
+   *caps = 0;
+
+   /* Use default SW ops */
+   *ops = NULL;
+
+   return 0;
+}
+
 static void
 sw_info_get(struct rte_eventdev *dev, struct rte_event_dev_info *info)
 {
@@ -755,6 +771,8 @@ sw_probe(struct rte_vdev_device *vdev)
 
.eth_rx_adapter_caps_get = sw_eth_rx_adapter_caps_get,
 
+   .timer_adapter_caps_get = sw_timer_adapter_caps_get,
+
.xstats_get = sw_xstats_get,
.xstats_get_names = sw_xstats_get_names,
.xstats_get_by_name = sw_xstats_get_by_name,
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index 6ef7c1c..f3f05c2 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -45,6 +45,7 @@ LDLIBS += -lrte_eal -lrte_ring -lrte_ethdev -lrte_hash
 SRCS-y += rte_eventdev.c
 SRCS-y += rte_event_ring.c
 SRCS-y += rte_event_eth_rx_adapter.c
+SRCS-y += rte_event_timer_adapter.c
 
 # export include files
 SYMLINK-y-include += rte_eventdev.h
@@ -54,6 +55,7 @@ SYMLINK-y-include += rte_eventdev_pmd_vdev.h
 SYMLINK-y-include += rte_event_ring.h
 SYMLINK-y-include += rte_event_eth_rx_adapter.h
 SYMLINK-y-include += rte_event_timer_adapter.h
+SYMLINK-y-include += rte_event_timer_adapter_pmd.h
 
 # versioning export map
 EXPORT_MAP := rte_eventdev_version.map
diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c 
b/lib/librte_eventdev/rte_event_timer_adapter.c
new file mode 100644
index 000..5315058
--- /dev/null
+++ b/lib/librte_eventdev/rte_event_timer_adapter.c
@@ -0,0 +1,407 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING

[dpdk-dev] [RFC PATCH v5 4/5] eventtimer: add default software implementation stub

[Erik Gabriel Carrillo]

If an eventdev PMD does not wish to provide event timer adapter ops
definitions, the library will fall back to a default software
implementation whose entry points are added by this commit.

Signed-off-by: Erik Gabriel Carrillo 
---
 lib/librte_eventdev/rte_event_timer_adapter.c | 107 ++
 1 file changed, 107 insertions(+)

diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c 
b/lib/librte_eventdev/rte_event_timer_adapter.c
index 5315058..d5a58ff 100644
--- a/lib/librte_eventdev/rte_event_timer_adapter.c
+++ b/lib/librte_eventdev/rte_event_timer_adapter.c
@@ -48,6 +48,8 @@
 
 static struct rte_event_timer_adapter adapters[MAX_EVENT_TIMER_ADAPTERS];
 
+const struct rte_event_timer_adapter_ops sw_event_adapter_timer_ops;
+
 static inline int
 adapter_valid(const struct rte_event_timer_adapter *adapter)
 {
@@ -211,6 +213,12 @@ rte_event_timer_adapter_create_ext(
}
}
 
+   /* If eventdev PMD did not provide ops, use default software
+* implementation.
+*/
+   if (adapter->ops == NULL)
+   adapter->ops = &sw_event_adapter_timer_ops;
+
/* Allow driver to do some setup */
FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -ENOTSUP);
ret = adapter->ops->init(adapter);
@@ -318,6 +326,12 @@ rte_event_timer_adapter_lookup(uint16_t adapter_id)
return NULL;
}
 
+   /* If eventdev PMD did not provide ops, use default software
+* implementation.
+*/
+   if (adapter->ops == NULL)
+   adapter->ops = &sw_event_adapter_timer_ops;
+
/* Set fast-path function pointers */
adapter->arm_burst = adapter->ops->arm_burst;
adapter->arm_tmo_tick_burst = adapter->ops->arm_tmo_tick_burst;
@@ -405,3 +419,96 @@ rte_event_timer_cancel_burst(const struct 
rte_event_timer_adapter *adapter,
 
return adapter->cancel_burst(adapter, event_timers, nb_event_timers);
 }
+
+/*
+ * Software event timer adapter ops definitions
+ */
+
+static int
+sw_event_timer_adapter_init(struct rte_event_timer_adapter *adapter)
+{
+   RTE_SET_USED(adapter);
+
+   return 0;
+}
+
+static int
+sw_event_timer_adapter_uninit(struct rte_event_timer_adapter *adapter)
+{
+   RTE_SET_USED(adapter);
+
+   return 0;
+}
+
+static int
+sw_event_timer_adapter_start(const struct rte_event_timer_adapter *adapter)
+{
+   RTE_SET_USED(adapter);
+
+   return 0;
+}
+
+static int
+sw_event_timer_adapter_stop(const struct rte_event_timer_adapter *adapter)
+{
+   RTE_SET_USED(adapter);
+
+   return 0;
+}
+
+static void
+sw_event_timer_adapter_get_info(const struct rte_event_timer_adapter *adapter,
+   struct rte_event_timer_adapter_info *adapter_info)
+{
+   RTE_SET_USED(adapter);
+   RTE_SET_USED(adapter_info);
+}
+
+static int
+sw_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
+struct rte_event_timer **evtims,
+uint16_t nb_evtims)
+{
+   RTE_SET_USED(adapter);
+   RTE_SET_USED(evtims);
+   RTE_SET_USED(nb_evtims);
+
+   return 0;
+}
+
+static int
+sw_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
+   struct rte_event_timer **evtims,
+   uint16_t nb_evtims)
+{
+   RTE_SET_USED(adapter);
+   RTE_SET_USED(evtims);
+   RTE_SET_USED(nb_evtims);
+
+   return 0;
+}
+
+static int
+sw_event_timer_arm_tmo_tick_burst(const struct rte_event_timer_adapter 
*adapter,
+ struct rte_event_timer **tims,
+ uint64_t timeout_tick,
+ uint16_t nb_tims)
+{
+   RTE_SET_USED(adapter);
+   RTE_SET_USED(tims);
+   RTE_SET_USED(timeout_tick);
+   RTE_SET_USED(nb_tims);
+
+   return 0;
+}
+
+const struct rte_event_timer_adapter_ops sw_event_adapter_timer_ops = {
+   .init = sw_event_timer_adapter_init,
+   .uninit = sw_event_timer_adapter_uninit,
+   .start = sw_event_timer_adapter_start,
+   .stop = sw_event_timer_adapter_stop,
+   .get_info = sw_event_timer_adapter_get_info,
+   .arm_burst = sw_event_timer_arm_burst,
+   .arm_tmo_tick_burst = sw_event_timer_arm_tmo_tick_burst,
+   .cancel_burst = sw_event_timer_cancel_burst,
+};
-- 
2.6.4

[dpdk-dev] [RFC PATCH v5 0/5] eventtimer: introduce event timer adapter

[Erik Gabriel Carrillo]

This set of RFC patches contains a reworked version of the "front-end" of 
the event timer adapter, i.e., the API and common logic.

This patch set produces the following checkpatch warning: 
"macro with flow control". I have left the macros in since such usage seems
common in DPDK.

v5
- Addressed comments on previous version from Pavan:
  - renamed rte_event_timer_adapter_driver.h to rte_event_timer_adapter_pmd.h
  - moved contents of sw_event_timer_adapter.c into rte_event_timer_adapter.c
  - added flags parameter to timer_adapter_caps_get() call
  - added DEBUG config variable to conditionally compile run-time checks on
datapath
  - fixed license text and file description
- Also added a config variable to enable/disable compilation of event timer
  adapter - feedback on whether this is desirable is appreciated

v4
- Split changes into multiple patches for easier review

v3
- Reworked allocation and ops organization in common code based on feedback
  received from Jerin and Pavan. This will allow fast-path function pointers to 
  be dereferenced with one level of indirection with pointers valid in primary
  and secondary processes.
- Moved default software implementation from sw_evdev directory to eventdev
  library directory, which will allow it to be used by any eventdev PMD as an
  alternative to providing its own definitions.
- Reverted occurrences of id back to pointer to adapter struct in library API
- Added rte_event_timer_adapter_lookup() function back in

v2
- Added ops structure and stubbed out plugin for SW impl
- Added unit test stubs
- Replaced occurrences of "wheel" in API with "adapter"
- Replaced occurrences of pointer to struct rte_event_timer_adapter with ids
- Removed rte_event_timer_adapter_lookup() function
- Replaced RTE_EVENT_TIMER_SUCCESS_{ARM,CANCEL} states with
  RTE_EVENT_TIMER_ARMED

Erik Gabriel Carrillo (4):
  eventtimer: introduce event timer adapter
  eventtimer: add common code
  eventtimer: add config variable for adapter
  eventtimer: add default software implementation stub
  test: add event timer adapter auto-test

 config/common_base|   2 +
 doc/api/doxy-api-index.md |   1 +
 drivers/event/sw/sw_evdev.c   |  22 +
 lib/librte_eventdev/Makefile  |   3 +
 lib/librte_eventdev/rte_event_timer_adapter.c | 514 +
 lib/librte_eventdev/rte_event_timer_adapter.h | 518 ++
 lib/librte_eventdev/rte_event_timer_adapter_pmd.h | 159 +++
 lib/librte_eventdev/rte_eventdev.h|   7 +-
 lib/librte_eventdev/rte_eventdev_pmd.h|  41 ++
 lib/librte_eventdev/rte_eventdev_version.map  |  15 +-
 test/test/Makefile|   1 +
 test/test/test_event_timer_adapter.c  | 249 +++
 12 files changed, 1529 insertions(+), 3 deletions(-)
 create mode 100644 lib/librte_eventdev/rte_event_timer_adapter.c
 create mode 100644 lib/librte_eventdev/rte_event_timer_adapter.h
 create mode 100644 lib/librte_eventdev/rte_event_timer_adapter_pmd.h
 create mode 100644 test/test/test_event_timer_adapter.c

-- 
2.6.4

Re: [dpdk-dev] [RFC PATCH v4 2/4] eventtimer: add common code

[Carrillo, Erik G]

> -Original Message-
> From: Pavan Nikhilesh Bhagavatula
> [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Thursday, November 30, 2017 11:13 PM
> To: Carrillo, Erik G 
> Cc: dev@dpdk.org
> Subject: Re: [RFC PATCH v4 2/4] eventtimer: add common code
> 
> On Thu, Nov 30, 2017 at 08:59:19PM +, Carrillo, Erik G wrote:
> > Hi Pavan,
> >
> > Thanks for the review;  I'm working on addressing the comments and have
> the following question (inline):
> >
> > <... snipped ...>
> >
> > > > +   adapter->data->mz = mz;
> > > > +   adapter->data->event_dev_id = conf->event_dev_id;
> > > > +   adapter->data->id = adapter_id;
> > > > +   adapter->data->socket_id = conf->socket_id;
> > > > +   adapter->data->conf = *conf;  /* copy conf structure */
> > > > +
> > > > +   /* Query eventdev PMD for timer adapter capabilities and ops */
> > > > +   ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > > > +  &adapter->data->caps,
> > > > +  &adapter->ops);
> > >
> > > The underlying driver needs info about the adapter flags i.e.
> > > RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES and
> > > RTE_EVENT_TIMER_ADAPTER_F_SP_PUT so we need to pass those too
> conf-
> > > >flags.
> >
> > By "underlying driver", are you referring to the eventdev PMD, or the
> event timer adapter "driver" (i.e., the set of ops functions)?
> >
> > If the latter, the adapter "driver" will have a chance to inspect the flags
> when adapter->ops->init() is called below, since it can look at the flags
> through the adapter arg.
> >
> 
> I was refering to the timer driver, the presence of flag
> RTE_EVENT_TIMER_ADAPTER_F_SP_PUT would suggest the driver to use a
> multi thread unsafe arm/cancel data path API and it would set a different
> function pointers to adapter->arm_burst etc.
> 
> I dont think in the current scheme this is possible. Currently, if we see
> mempool it inspects flags before setting ops.
> 
> Hope this clears things up.
> 
> -Pavan

Yes, I see your point now.  I agree that it would be useful to allow different 
ops structures to be selected based on the flags that are set in addition to 
being able to inspect the flags within the ops functions themselves.  I have 
made the change in the follow-up patch series.

Thanks,
Gabriel

> 
> > If the former, will the eventdev PMD consider the flags when deciding
> whether or not to provide an ops definition in the timer_adapter_caps_get()
> call?
> >
> > >
> > > > +   if (ret < 0) {
> > > > +   rte_errno = -EINVAL;
> > > > +   return NULL;
> > > > +   }
> > > > +
> > > > +   if (!(adapter->data->caps &
> > > > + RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)) {
> > > > +   FUNC_PTR_OR_NULL_RET_WITH_ERRNO(conf_cb, -EINVAL);
> > > > +   ret = conf_cb(adapter->data->id, adapter->data-
> > > >event_dev_id,
> > > > + &adapter->data->event_port_id, conf_arg);
> > > > +   if (ret < 0) {
> > > > +   rte_errno = -EINVAL;
> > > > +   return NULL;
> > > > +   }
> > > > +   }
> > > > +
> > > > +   /* Allow driver to do some setup */
> > > > +   FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -
> > > ENOTSUP);
> > > > +   ret = adapter->ops->init(adapter);
> > > > +   if (ret < 0) {
> > > > +   rte_errno = -EINVAL;
> > > > +   return NULL;
> > > > +   }
> > > > +
> > > > +   /* Set fast-path function pointers */
> > > > +   adapter->arm_burst = adapter->ops->arm_burst;
> > > > +   adapter->arm_tmo_tick_burst = adapter->ops-
> > > >arm_tmo_tick_burst;
> > > > +   adapter->cancel_burst = adapter->ops->cancel_burst;
> > > > +
> > > > +   adapter->allocated = 1;
> > > > +
> > > > +   return adapter;
> > > > +}
> >
> > <... snipped ...>

Re: [dpdk-dev] [PATCH 2/3] power: switching to unbuffered access for /sys files

[Radoslaw Biernacki]

Hi David,

Sorry for log delay.
I will make V3 with your suggestions.
Thanks.

On 23 November 2017 at 15:42, Hunt, David  wrote:

> Hi Radoslaw,
>
>
>
> On 11/11/2017 6:55 PM, Radoslaw Biernacki wrote:
>
>> This patch fixes the bug caused by improper use of buffered stdio file
>> access for switching the CPU frequency and governor. When using
>> buffered stdio, each fwrite() must use fflush() and the return code
>> must be verified. Also fseek() is needed.  Therefore it is better to
>> use unbuffered mode or use plain open()/write() functions.  This fix
>> use second approach. This not only remove need for ffush() but also
>> remove need for fseek() operations.  This patch also reuse some code
>> around power_set_governor_userspace() and
>> power_set_governor_userspace() functions.
>>
>> Fixes: 445c6528b55f ("power: common interface for guest and host")
>> CC: sta...@dpdk.org
>>
>> Signed-off-by: Radoslaw Biernacki 
>> ---
>>   lib/librte_power/rte_power_acpi_cpufreq.c | 211
>> +-
>>   1 file changed, 91 insertions(+), 120 deletions(-)
>>
>> diff --git a/lib/librte_power/rte_power_acpi_cpufreq.c
>> b/lib/librte_power/rte_power_acpi_cpufreq.c
>> index 3d0872f..f811bd3 100644
>> --- a/lib/librte_power/rte_power_acpi_cpufreq.c
>> +++ b/lib/librte_power/rte_power_acpi_cpufreq.c
>> @@ -30,7 +30,7 @@
>>*   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>> USE
>>*   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>> DAMAGE.
>>*/
>> -
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -47,6 +47,12 @@
>>   #include "rte_power_acpi_cpufreq.h"
>>   #include "rte_power_common.h"
>>   +#define min(_x, _y) ({ \
>> +   typeof(_x) _min1 = (_x); \
>> +   typeof(_y) _min2 = (_y); \
>> +   (void) (&_min1 == &_min2); \
>> +   _min1 < _min2 ? _min1 : _min2; })
>> +
>>   #ifdef RTE_LIBRTE_POWER_DEBUG
>>   #define POWER_DEBUG_TRACE(fmt, args...) do { \
>> RTE_LOG(ERR, POWER, "%s: " fmt, __func__, ## args); \
>> @@ -88,7 +94,7 @@ struct rte_power_info {
>> unsigned lcore_id;   /**< Logical core id */
>> uint32_t freqs[RTE_MAX_LCORE_FREQS]; /**< Frequency array */
>> uint32_t nb_freqs;   /**< number of available
>> freqs */
>> -   FILE *f; /**< FD of scaling_setspeed
>> */
>> +   int fd;  /**< FD of scaling_setspeed
>> */
>> char governor_ori[32];   /**< Original governor name
>> */
>> uint32_t curr_idx;   /**< Freq index in freqs
>> array */
>> volatile uint32_t state; /**< Power in use state */
>> @@ -105,6 +111,9 @@ static struct rte_power_info
>> lcore_power_info[RTE_MAX_LCORE];
>>   static int
>>   set_freq_internal(struct rte_power_info *pi, uint32_t idx)
>>   {
>> +   char buf[BUFSIZ];
>> +   int count, ret;
>> +
>> if (idx >= RTE_MAX_LCORE_FREQS || idx >= pi->nb_freqs) {
>> RTE_LOG(ERR, POWER, "Invalid frequency index %u, which "
>> "should be less than %u\n", idx,
>> pi->nb_freqs);
>> @@ -117,17 +126,14 @@ set_freq_internal(struct rte_power_info *pi,
>> uint32_t idx)
>> POWER_DEBUG_TRACE("Freqency[%u] %u to be set for lcore %u\n",
>> idx, pi->freqs[idx], pi->lcore_id);
>> -   if (fseek(pi->f, 0, SEEK_SET) < 0) {
>> -   RTE_LOG(ERR, POWER, "Fail to set file position indicator
>> to 0 "
>> -   "for setting frequency for lcore %u\n",
>> pi->lcore_id);
>> +   count = snprintf(buf, sizeof(buf), "%u", pi->freqs[idx]);
>> +   assert((size_t)count < sizeof(buf)-1);
>> +   ret = write(pi->fd, buf, count);
>> +   if (ret != count) {
>> +   RTE_LOG(ERR, POWER, "Fail to write new frequency (%s) for
>> "
>> +   "lcore %u\n", buf, pi->lcore_id);
>> return -1;
>> }
>> -   if (fprintf(pi->f, "%u", pi->freqs[idx]) < 0) {
>> -   RTE_LOG(ERR, POWER, "Fail to write new frequency for "
>> -   "lcore %u\n", pi->lcore_id);
>> -   return -1;
>> -   }
>> -   fflush(pi->f);
>> pi->curr_idx = idx;
>> return 1;
>> @@ -139,90 +145,109 @@ set_freq_internal(struct rte_power_info *pi,
>> uint32_t idx)
>>* governor will be saved for rolling back.
>>*/
>>   static int
>> -power_set_governor_userspace(struct rte_power_info *pi)
>> +power_set_governor(unsigned int lcore_id, const char *new_gov, char
>> *old_gov,
>> +  size_t old_gov_len)
>>   {
>> -   FILE *f;
>> +   int fd;
>> +   int count, len;
>> int ret = -1;
>> char buf[BUFSIZ];
>> char fullpath[PATH_MAX];
>> -   char *s;
>> -   int val;
>> snprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_GOVERNOR,
>> -   pi->lcore

Re: [dpdk-dev] [PATCH 1/7] ethdev: remove unused flag from header

[Vlad Zolotarov]

resending after registering with the new email domain ;)
Please, see my comments below.

On 12/01/2017 05:17 PM, Vlad Zolotarov wrote:



On 11/30/2017 09:29 PM, Ferruh Yigit wrote:

remove RTE_ETHDEV_HAS_LRO_SUPPORT flag from header.

Flag seems added with the patch that adds LRO support, and intention
looks like giving a pointer to application that library supports LRO.


Exactly. Removing this flag may make the existing application "think" 
that LRO is not supported.

Why do you want to remove it to begin with?


Fixes: 8eecb3295aed ("ixgbe: add LRO support")
Cc:vl...@cloudius-systems.com

Signed-off-by: Ferruh Yigit
---
  lib/librte_ether/rte_ethdev.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 341c2d624..e620c3706 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -172,9 +172,6 @@ extern "C" {
  
  #include 
  
-/* Use this macro to check if LRO API is supported */

-#define RTE_ETHDEV_HAS_LRO_SUPPORT
-
  #include 
  #include 
  #include 

Re: [dpdk-dev] [PATCH 2/2] eal/x86: Use lock-prefixed instructions to reduce cost of rte_smp_mb()

[Ananyev, Konstantin]

Hi Stephen,

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 1, 2017 6:04 PM
> To: Ananyev, Konstantin 
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] eal/x86: Use lock-prefixed instructions 
> to reduce cost of rte_smp_mb()
> 
> On Fri,  1 Dec 2017 11:12:51 +
> Konstantin Ananyev  wrote:
> 
> > On x86 it  is possible to use lock-prefixed instructions to get
> > the similar effect as mfence.
> > As pointed by Java guys, on most modern HW that gives a better
> > performance than using mfence:
> > https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
> > That patch adopts that technique for rte_smp_mb() implementation.
> > On BDW 2.2 mb_autotest on single lcore reports 2X cycle reduction,
> > i.e. from ~110 to ~55 cycles per operation.
> >
> > Signed-off-by: Konstantin Ananyev 
> > ---
> >  .../common/include/arch/x86/rte_atomic.h   | 45 
> > +-
> >  1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h 
> > b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> > index 4eac66631..07b7fa7f7 100644
> > --- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> > +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> > @@ -55,12 +55,53 @@ extern "C" {
> >
> >  #definerte_rmb() _mm_lfence()
> >
> > -#define rte_smp_mb() rte_mb()
> > -
> >  #define rte_smp_wmb() rte_compiler_barrier()
> >
> >  #define rte_smp_rmb() rte_compiler_barrier()
> >
> > +/*
> > + * From Intel Software Development Manual; Vol 3;
> > + * 8.2.2 Memory Ordering in P6 and More Recent Processor Families:
> > + * ...
> > + * . Reads are not reordered with other reads.
> > + * . Writes are not reordered with older reads.
> > + * . Writes to memory are not reordered with other writes,
> > + *   with the following exceptions:
> > + *   . streaming stores (writes) executed with the non-temporal move
> > + * instructions (MOVNTI, MOVNTQ, MOVNTDQ, MOVNTPS, and MOVNTPD); and
> > + *   . string operations (see Section 8.2.4.1).
> > + *  ...
> > + * . Reads may be reordered with older writes to different locations but 
> > not
> > + * with older writes to the same location.
> > + * . Reads or writes cannot be reordered with I/O instructions,
> > + * locked instructions, or serializing instructions.
> > + * . Reads cannot pass earlier LFENCE and MFENCE instructions.
> > + * . Writes ... cannot pass earlier LFENCE, SFENCE, and MFENCE 
> > instructions.
> > + * . LFENCE instructions cannot pass earlier reads.
> > + * . SFENCE instructions cannot pass earlier writes ...
> > + * . MFENCE instructions cannot pass earlier reads, writes ...
> > + *
> > + * As pointed by Java guys, that makes possible to use lock-prefixed
> > + * instructions to get the same effect as mfence and on most modern HW
> > + * that gives a better perfomarnce than using mfence:
> > + * https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
> > + * So below we use that technique for rte_smp_mb() implementation.
> > + */
> > +
> > +#ifdef RTE_ARCH_I686
> > +#defineRTE_SP  RTE_STR(esp)
> > +#else
> > +#defineRTE_SP  RTE_STR(rsp)
> > +#endif
> > +
> > +#define RTE_MB_DUMMY_MEMP  "-128(%%" RTE_SP ")"
> > +
> > +static __rte_always_inline void
> > +rte_smp_mb(void)
> > +{
> > +   asm volatile("lock addl $0," RTE_MB_DUMMY_MEMP "; " ::: "memory");
> > +}
> > +
> >  #define rte_io_mb() rte_mb()
> >
> >  #define rte_io_wmb() rte_compiler_barrier()
> 
> The lock instruction is a stronger barrier than the compiler barrier
> and has worse performance impact. Are you sure it is necessary to use it in 
> DPDK.
> Linux kernel has successfully used simple compiler reodering barrier for 
> years.

Where do you see compiler barrier?
Right now for x86 rte_smp_mb()==rte_mb()==mfence.
So I am replacing mfence with 'lock add'.
As comment above says - on most modern x86 systems it is faster,
while allow to preserve memory ordering.
Konstantin 

> 
> Don't confuse rte_smp_mb with the required barrier for talking to I/O devices.

Re: [dpdk-dev] [PATCH 1/7] ethdev: remove unused flag from header

[Ferruh Yigit]

On 12/1/2017 2:17 PM, Vlad Zolotarov wrote:
> 
> 
> On 11/30/2017 09:29 PM, Ferruh Yigit wrote:
>> remove RTE_ETHDEV_HAS_LRO_SUPPORT flag from header.
>>
>> Flag seems added with the patch that adds LRO support, and intention
>> looks like giving a pointer to application that library supports LRO.
> 
> Exactly. Removing this flag may make the existing application "think" that LRO
> is not supported.
> Why do you want to remove it to begin with?

I agree that this is a little controversial, and I don't have a strong opinion
about it, I thought twice before sending or not sending the patch :)

This flag can be useful for the applications that use different versions of the
DPDK library (with the ones does and doesn't support LRO), so that application
can be more portable.
I would understand a dynamic approach, which would be useful for distributed
applications that you don't know and can't control what version of the DPDK
library used.
But here to benefit from this flag you need to compile your application against
DPDK library, and if you are compiling it you already know if your DPDK supports
LRO or not. And this is not something keeps changing platform to platform etc,
after a specific release LRO is always supported.

And this is the only capability support flag in the ethdev, there are many new
features introduced in each release but they are not advertised by a capability
flag. Only having LRO flag can be confusing.

_Perhaps_ DPDK should have a way to expose the supported features, and a dynamic
runtime way can be better option than compile time macros, but it should be
generic nothing specific to LRO support.

> 
>>
>> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>> Cc: vl...@cloudius-systems.com
>>
>> Signed-off-by: Ferruh Yigit 
>> ---
>>  lib/librte_ether/rte_ethdev.h | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> index 341c2d624..e620c3706 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -172,9 +172,6 @@ extern "C" {
>>  
>>  #include 
>>  
>> -/* Use this macro to check if LRO API is supported */
>> -#define RTE_ETHDEV_HAS_LRO_SUPPORT
>> -
>>  #include 
>>  #include 
>>  #include 
> 

Re: [dpdk-dev] [PATCH 3/7] ethdev: separate driver APIs

[Ferruh Yigit]

On 12/1/2017 12:59 AM, Hemant Agrawal wrote:
> On 12/1/2017 7:59 AM, Ferruh Yigit wrote:
> 
> ...
>> diff --git a/lib/librte_ether/rte_ethdev_driver.h 
>> b/lib/librte_ether/rte_ethdev_driver.h
>> new file mode 100644
>> index 0..3e77d1439
>> --- /dev/null
>> +++ b/lib/librte_ether/rte_ethdev_driver.h
>> @@ -0,0 +1,163 @@
>> +/*-
>> + *   BSD LICENSE
>> + *
>> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
>> + *   All rights reserved.
> 
> You can remove one of the all rights reserved.
> This is also an issue in your next patch for rte_ethdev_core.h
> 
> Also, as Shreyansh mentioned, Why not start with SPDX tags instead of 
> full license text?

Good idea, I will update in next version.

> 
> ...
>> +/**
>> + * @internal Executes all the user application registered callbacks for
>> + * the specific device. It is for DPDK internal user only. User
>> + * application should not call it directly.
>> + *
>> + * @param dev
>> + *  Pointer to struct rte_eth_dev.
>> + * @param event
>> + *  Eth device interrupt event type.
>> + * @param cb_arg
>> + *  callback parameter.
>> + * @param ret_param
>> + *  To pass data back to user application.
>> + *  This allows the user application to decide if a particular function
>> + *  is permitted or not.
>> + *
>> + * @return
>> + *  int
>> + */
>> +int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
>> +enum rte_eth_event_type event, void *cb_arg, void *ret_param);
>> +
>> +/**
>> + * Create memzone for HW rings.
> 
> Like all other functions, you can also add "@internal" for this as well.

OK.

> 
> Acked-by: Hemant Agrawal 
> 

Re: [dpdk-dev] [PATCH 2/7] ethdev: fix port id storage

[Ferruh Yigit]

On 11/30/2017 6:29 PM, Ferruh Yigit wrote:
> port_id is now 16bits, update function parameter according.
> 
> Fixes: 4c270218aa26 ("ethdev: support security APIs")
> Cc: sta...@dpdk.org
> Cc: declan.dohe...@intel.com
> 
> Signed-off-by: Ferruh Yigit 
> ---
> Cc: Boris Pismenny 
> Cc: Aviad Yehezkel 
> Cc: Radu Nicolau 
> Cc: Declan Doherty 
> ---

<...>

>  
>  void *
> -rte_eth_dev_get_sec_ctx(uint8_t port_id);
> +rte_eth_dev_get_sec_ctx(uint16_t port_id);

Hi Declan,

Since this is a public API, it needs API documentation. Can you please send a
patch to add doxygen comments the the API?

Thanks,
ferruh

>  
>  struct rte_eth_dev_sriov {
>   uint8_t active;   /**< SRIOV is active with 16, 32 or 64 
> pools */
> 

Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object

[Ferruh Yigit]

On 12/1/2017 5:17 AM, Bruce Richardson wrote:
> On Fri, Dec 01, 2017 at 11:22:12AM +, Ananyev, Konstantin wrote:
>>
>>
>>> -Original Message-
>>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson
>>> Sent: Friday, December 1, 2017 10:33 AM
>>> To: Yigit, Ferruh 
>>> Cc: Thomas Monjalon ; dev@dpdk.org; 
>>> vl...@cloudius-systems.com
>>> Subject: Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object
>>>
>>> On Fri, Dec 01, 2017 at 02:29:57AM +, Ferruh Yigit wrote:
 "struct rte_eth_rxtx_callback" is defined as internal data structure but
 used in public APIs.

 Checking the API documentation shows that intention was using this
 object as opaque object. Data structure only used in delete APIs which
 doesn't require to know the internals of the data structure.

 Converting callback parameter in API to void pointer should not require
 any modification in user application because this data structure was
 already marked as internal and only should be used as pointer in
 application.

 Signed-off-by: Ferruh Yigit 
 ---
>>>
>>> I disagree on this patch. The structure itself is not exposed, only the
>>> name, since it is only passed around as a pointer, so there is no need
>>> to change the parameters to void pointer. It's a named opaque type.
>>
>> Personally I think it would be better to do visa-versa: 
>> make rte_eth_add_(rx|tx)_callback() to return struct rte_eth_rxtx_callback *
>> instead of void *.
>> Konstantin
>>
> I didn't realise that it did, so definite +1 to that suggestion.

No issue on having a named opaque type, but unfortunately struct is exposed
because of inline functions again.
It has been moved into rte_ethdev_core.h but accessible by applications.

And since intention is an opaque type, because of "void *" return types, I
thought it is better to hide type completely so that application can't access
details.

Re: [dpdk-dev] [PATCH] mmap(2) returns MAP_FAILED, not NULL, on failure

[Ferruh Yigit]

On 11/30/2017 10:51 PM, Michael McConville wrote:
> Signed-off-by: Michael McConville 

Reviewed-by: Ferruh Yigit 


Hi Michael,

Thanks you for the patch, can you please check the contribution guide [1] for
expected patch format for next patches.

[1]
http://dpdk.org/doc/guides/contributing/patches.html

<...>

Re: [dpdk-dev] [PATCH] mmap(2) returns MAP_FAILED, not NULL, on failure

[Ferruh Yigit]

On 12/1/2017 4:50 PM, Ferruh Yigit wrote:
> On 11/30/2017 10:51 PM, Michael McConville wrote:
>> Signed-off-by: Michael McConville 
> 
> Reviewed-by: Ferruh Yigit 
> 
> 
> Hi Michael,
> 
> Thanks you for the patch, can you please check the contribution guide [1] for
> expected patch format for next patches.

Ahh, there is checkpatch warning as well, can you make a new version of the
patch with updated patch title and warning fixed?

> 
> [1]
> http://dpdk.org/doc/guides/contributing/patches.html
> 
> <...>
> 

Re: [dpdk-dev] [PATCH] mmap(2) returns MAP_FAILED, not NULL, on failure

[Ferruh Yigit]

On 11/30/2017 10:51 PM, Michael McConville wrote:
> Signed-off-by: Michael McConville 
> ---
> lib/librte_eal/bsdapp/eal/eal_memory.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c 
> b/lib/librte_eal/bsdapp/eal/eal_memory.c
> index 6ba058578..2c8a4b592 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_memory.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c
> @@ -155,7 +155,7 @@ rte_eal_hugepage_attach(void)
>/* Map the shared hugepage_info into the process address spaces */
>hpi = mmap(NULL, sizeof(struct hugepage_info), PROT_READ, MAP_PRIVATE,
>fd_hugepage_info, 0);
> -   if (hpi == NULL) {
> +   if (hpi == MAP_FAILED) {

Hi Matej,

Can you fix same thing in szedata2 PMD please [1] ?

[1]
http://dpdk.org/browse/dpdk/tree/drivers/net/szedata2/rte_eth_szedata2.c?h=v17.11#n1556

>RTE_LOG(ERR, EAL, "Could not mmap %s\n", 
> eal_hugepage_info_path());
>goto error;
>}
> 

[dpdk-dev] [Bug 5] This is a Bug, Please Disabled Create new Account

[bugzilla]

http://dpdk.org/tracker/show_bug.cgi?id=5

Bug ID: 5
   Summary: This is a Bug, Please Disabled Create new Account
   Product: DPDK
   Version: unspecified
  Hardware: All
OS: All
Status: CONFIRMED
  Severity: critical
  Priority: Normal
 Component: other
  Assignee: dev@dpdk.org
  Reporter: akabane...@gmail.com
  Target Milestone: ---

Created attachment 1
  --> http://dpdk.org/tracker/attachment.cgi?id=1&action=edit
Okay

Oh No, Please Disabled !!

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of device stop/start

[Tiwei Bie]

On Wed, Nov 15, 2017 at 01:38:32AM +0800, Fischetti, Antonio wrote:
> Hi Tiwei,
> 
> I'm doing some regression tests with v17.11-rc4. I ran 
> into a hitch with testpmd running into a guest VM. It happens 
> that no packet gets forwarded by testpmd.
> The issue seems to appear after this patch was upstreamed.
> 
> I saw there's a way to make it work, ie by avoiding to 
> increment the last consumed descriptor:
> 
> --- a/drivers/net/virtio/virtqueue.c
> +++ b/drivers/net/virtio/virtqueue.c
> @@ -80,7 +80,7 @@ virtqueue_flush(struct virtqueue *vq)
> rte_pktmbuf_free(dxp->cookie);
> dxp->cookie = NULL;
> }
> -   vq->vq_used_cons_idx++;
> +   //vq->vq_used_cons_idx++;
> vq_ring_free_chain(vq, desc_idx);
> 
> Not quite sure if this change make any sense to you?
> 
> Some details below.
> 
> The issue appears only if the traffic generator is already 
> sending packets before I launch testpmd in the guest.
> 
> In my testbench I have Open-vSwitch (OvS-DPDK) which launches 
> a VM with 2 vhostuserclient ports (vhu0 and vhu1), each with 
> a single queue.
> My OvS has 2 physical ports: dpdk0 and dpdk1.
> dpdk0 forwards packets back and forth from/to the generator
> to/from vhu0.
> Similarly, dpdk1 forwards packets back and forth from/to the generator
> to/from vhu1.
> 
> In OvS there are 2 different PMD threads serving the 2 
> vhostuserclient ports.
> 
> While the traffic generator is already sending packets, in the 
> guest VM I launch 
>   ./testpmd -c 0x3 -n 4 --socket-mem 512 -- --burst=64 -i --txqflags=0xf00 
> --disable-hw-vlan
> 
> The issue is that I see no packet received on the traffic generator 
> and in fact testpmd shows
> 
> -- Forward statistics for port 0  --
>   RX-packets: 0  RX-dropped: 0 RX-total: 0
>   TX-packets: 0  TX-dropped: 0 TX-total: 0
>   
> 
>   -- Forward statistics for port 1  --
>   RX-packets: 0  RX-dropped: 0 RX-total: 0
>   TX-packets: 0  TX-dropped: 0 TX-total: 0
>   
> 
>   +++ Accumulated forward statistics for all ports+++
>   RX-packets: 0  RX-dropped: 0 RX-total: 0
>   TX-packets: 0  TX-dropped: 0 TX-total: 0
>   
> 
> Please let me know if I missed something or if you need 
> more info on my testbench.
> 

Hi Antonio,

I'm very sorry, I missed this mail before.. Thank you so much for
reporting this issue and the detailed info. I'll look into this!

Best regards,
Tiwei Bie

> 
> Thanks,
> Antonio
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tiwei Bie
> > Sent: Friday, October 20, 2017 3:09 AM
> > To: dev@dpdk.org
> > Cc: y...@fridaylinux.org; maxime.coque...@redhat.com;
> > jfreim...@redhat.com; sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of
> > device stop/start
> > 
> > After starting a device, the driver shouldn't deliver the
> > packets that already existed before the device is started
> > to applications. Otherwise it will lead to incorrect packet
> > collection for port state. This patch fixes this issue by
> > flushing the Rx queues when starting the device.
> > 
> > Fixes: a85786dc816f ("virtio: fix states handling during
> > initialization")
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Tiwei Bie 
> > Reviewed-by: Jens Freimann 
> > ---
> > v2:
> > - Use the existing `for` loop
> > - Improve the commit log
> > 
> >  drivers/net/virtio/virtio_ethdev.c |  2 ++
> >  drivers/net/virtio/virtio_rxtx.c   |  2 +-
> >  drivers/net/virtio/virtqueue.c | 25 +
> >  drivers/net/virtio/virtqueue.h |  5 +
> >  4 files changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> > b/drivers/net/virtio/virtio_ethdev.c
> > index 42c2836..9ccb0f4 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -1819,6 +1819,8 @@ virtio_dev_start(struct rte_eth_dev *dev)
> > 
> > for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > rxvq = dev->data->rx_queues[i];
> > +   /* Flush the old packets */
> > +   virtqueue_flush(rxvq->vq);
> > virtqueue_notify(rxvq->vq);
> > }
> > 
> > diff --git a/drivers/net/virtio/virtio_rxtx.c
> > b/drivers/net/virtio/virtio_rxtx.c
> > index 609b413..f5b6f94 100644
> > --- a/drivers/net/virtio/virtio_rxtx.c
> > +++ b/drivers/net/virtio/virtio_rxtx.c
> > @@ -80,7 +80,7 @@ virtio_dev_rx_queue_done(void *rxq, uint16_t offset)
> > retu

Re: [dpdk-dev] [PATCH] net/bonding: fix bonding in 8023ad mode

[Ferruh Yigit]

On 9/4/2017 6:37 AM, radu.nicolau at intel.com (Radu Nicolau) wrote:
> On 8/8/2017 1:56 PM, Jacek Piasecki wrote:
>> This patch blocks possibility to set master bonding by
>> rte_eth_bond_mode_set() in 802.3ad mode, as the API
>> doesn't prevent this.
>>
>> Fixes: 6d72657ce379 ("net/bonding: add other aggregator modes")
>> Cc: danielx.t.mrzyglod at intel.com
>>
>> Signed-off-by: Jacek Piasecki 
> Reviewed-by:  Radu Nicolau 

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

Re: [dpdk-dev] [PATCH] net/bnxt: remove unused rx_tpa from bnxt_rx_queue{}

[Ferruh Yigit]

On 11/28/2017 2:50 PM, Ferruh Yigit wrote:
> On 11/14/2017 4:42 AM, Ilya Matveychikov wrote:
> 
> Please add maintainer to cc next time.
> 
>> Signed-off-by: Ilya V. Matveychikov 
> 
> Reviewed-by: Ferruh Yigit 

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

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/nfp: fix MTU settings

[Ferruh Yigit]

On 11/24/2017 6:23 AM, Alejandro Lucero wrote:
> The wrong mtu length was used for configuring the hardware. The
> max_rx_pktlen reported was also wrong.
> 
> Fixes: defb9a5dd156 ("nfp: introduce driver initialization")
Cc: sta...@dpdk.org

> 
> Signed-off-by: Alejandro Lucero 

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

Re: [dpdk-dev] [PATCH 1/7] ethdev: remove unused flag from header

[Vlad Zolotarov]

On 12/01/2017 06:51 PM, Ferruh Yigit wrote:

On 12/1/2017 2:17 PM, Vlad Zolotarov wrote:


On 11/30/2017 09:29 PM, Ferruh Yigit wrote:

remove RTE_ETHDEV_HAS_LRO_SUPPORT flag from header.

Flag seems added with the patch that adds LRO support, and intention
looks like giving a pointer to application that library supports LRO.

Exactly. Removing this flag may make the existing application "think" that LRO
is not supported.
Why do you want to remove it to begin with?

I agree that this is a little controversial, and I don't have a strong opinion
about it, I thought twice before sending or not sending the patch :)

This flag can be useful for the applications that use different versions of the
DPDK library (with the ones does and doesn't support LRO), so that application
can be more portable.
I would understand a dynamic approach, which would be useful for distributed
applications that you don't know and can't control what version of the DPDK
library used.
But here to benefit from this flag you need to compile your application against
DPDK library, and if you are compiling it you already know if your DPDK supports
LRO or not. And this is not something keeps changing platform to platform etc,
after a specific release LRO is always supported.


True but DPDK is usually not a part of your source tree - it (should) 
come as a library and currently there isn't any proper way to query 
feature set.
This flag was also added after a lot of consideration as a rather ugly 
compromise since there wasn't (and there isn't) a proper way to do that 
at that (this) time. See more on that below.




And this is the only capability support flag in the ethdev, there are many new
features introduced in each release but they are not advertised by a capability
flag. Only having LRO flag can be confusing.

_Perhaps_ DPDK should have a way to expose the supported features, and a dynamic
runtime way can be better option than compile time macros, but it should be
generic nothing specific to LRO support.


IMO it's not "perhaps" it's a "must" _however_ you can't remove this 
flag without giving some other tool to do the same.
Querying a DPDK version would be a wrong direction because some Linux 
distributions (yes, I'm talking about you, Ubuntu) tend to have it's own 
trees with their own backports and these trees may be light years ahead 
in their patch level compared to vanilla trees with the same version.





Fixes: 8eecb3295aed ("ixgbe: add LRO support")
Cc: vl...@cloudius-systems.com

Signed-off-by: Ferruh Yigit 
---
  lib/librte_ether/rte_ethdev.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 341c2d624..e620c3706 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -172,9 +172,6 @@ extern "C" {
  
  #include 
  
-/* Use this macro to check if LRO API is supported */

-#define RTE_ETHDEV_HAS_LRO_SUPPORT
-
  #include 
  #include 
  #include 

Re: [dpdk-dev] [PATCH v2] net/virtio: fix an incorrect behavior of device stop/start

[Tiwei Bie]

Hi Antonio,

On Sat, Dec 02, 2017 at 01:17:58AM +0800, Fischetti, Antonio wrote:
> Hi All,
> I've got an update on this.
> I could replicate the same issue by using testpmd + a VM (= Virtual Machine).
> 
> The test topology I'm using is:
> 
> 
> [Traffic gen][NIC port #0][testpmd][vhost port #2]+
>   | 
>   |
>  [testpmd in
> the VM]
>   |
>   |
> [Traffic gen][NIC port #1][testpmd][vhost port #3]+
> 
> 
> So there's no OvS now in the picture: one testpmd running on the host
> and one testpmd running on the VM.
> 
> The issue is that no packet goes through testpmd in the VM.
> It seems this is happening after this patch was upstreamed.
> 
> Please note
> ---
> To replicate this issue both the next 2 conditions must be met:
>  - the traffic is already being sent before launching testpmd in the VM
>  - there are at least 2 forwarding lcores.
> 

Thanks again for reporting this issue with such detailed info!

Due to the current environment I have, I tried a slightly different
topo setup, instead of the above you described, my topo setup is:

[testpmd1/txonly][vhost port #0]+
|
|
   [testpmd in
  the VM]
|
|
[testpmd2/rxonly][vhost port #1]+

That is to say, I'm using testpmd/{txonly,rxonly} directly without
external traffic generators.

Below is the script I'm using to launch the testpmds:

# testpmd1:

sudo ./x86_64-native-linuxapp-gcc/app/testpmd -l 1,2 \
--socket-mem 1024,1024 \
--file-prefix=vhost0 \
--no-pci \
--vdev=net_vhost0,iface=/tmp/socket-0,queues=1 \
-- \
--port-topology=chained \
-i \
--nb-cores=1 \
--forward-mode=txonly

# testpmd2:

sudo ./x86_64-native-linuxapp-gcc/app/testpmd -l 3,4 \
--socket-mem 1024,1024 \
--file-prefix=vhost1 \
--no-pci \
--vdev=net_vhost1,iface=/tmp/socket-1,queues=1 \
-- \
--port-topology=chained \
-i \
--nb-cores=1 \
--forward-mode=rxonly

# testpmd in the VM:

sudo \
./x86_64-native-linuxapp-gcc/app/testpmd -l 1,2,3 \
--socket-mem 512 \
-- \
--burst=64 -i --txqflags=0xf00 --disable-hw-vlan \
--nb-cores=2 \
--forward-mode=mac

Unfortunately, I failed to reproduce this issue.

I'm starting the traffics of the testpmd1 and testpmd2 (i.e. txonly
fwd in testpmd1 and rxonly fwd in testpmd2) before launching the
testpmd in the VM.

And I'm using 2 forwarding lcores for the testpmd in the VM:

testpmd> show config fwd
mac packet forwarding - ports=2 - cores=2 - streams=2 - NUMA support enabled, 
MP over anonymous pages disabled
Logical Core 2 (socket 0) forwards packets on 1 streams:
  RX P=0/Q=0 (socket 0) -> TX P=1/Q=0 (socket 0) peer=02:00:00:00:00:01
Logical Core 3 (socket 0) forwards packets on 1 streams:
  RX P=1/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00

I can see the traffics are forwarded once the fwd is started
in the testpmd in the VM:

testpmd> show port stats all

   NIC statistics for port 0  
  RX-packets: 52865632   RX-missed: 0  RX-bytes:  3383400448
  RX-errors: 0
  RX-nombuf:  0
  TX-packets: 0  TX-errors: 0  TX-bytes:  0

  Throughput (since last show)
  Rx-pps:  6040178
  Tx-pps:0
  

   NIC statistics for port 1  
  RX-packets: 0  RX-missed: 0  RX-bytes:  0
  RX-errors: 0
  RX-nombuf:  0
  TX-packets: 52865568   TX-errors: 0  TX-bytes:  3383396352

  Throughput (since last show)
  Rx-pps:0
  Tx-pps:  6040177
  

The DPDK version I'm using is: 18.02-rc0

Do you see anything I missed? Or can you reproduce the issue with the
setup I'm using?

> I found that a way to fix this is:
> 
> diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
> index c3a536f..17dd87d 100644
> --- a/drivers/net/virtio/virtqueue.c
> +++ b/drivers/net/virtio/virtqueue.c
> @@ -80,7 +80,7 @@ virtqueue_flush(struct virtqueue *vq)
>

Re: [dpdk-dev] [PATCH] arch/arm: optimization for memcpy on AArch64

[Pavan Nikhilesh Bhagavatula]

On Mon, Nov 27, 2017 at 03:49:45PM +0800, Herbert Guan wrote:
> This patch provides an option to do rte_memcpy() using 'restrict'
> qualifier, which can induce GCC to do optimizations by using more
> efficient instructions, providing some performance gain over memcpy()
> on some AArch64 platforms/enviroments.
>
> The memory copy performance differs between different AArch64
> platforms. And a more recent glibc (e.g. 2.23 or later)
> can provide a better memcpy() performance compared to old glibc
> versions. It's always suggested to use a more recent glibc if
> possible, from which the entire system can get benefit. If for some
> reason an old glibc has to be used, this patch is provided for an
> alternative.
>
> This implementation can improve memory copy on some AArch64
> platforms, when an old glibc (e.g. 2.19, 2.17...) is being used.
> It is disabled by default and needs "RTE_ARCH_ARM64_MEMCPY"
> defined to activate. It's not always proving better performance
> than memcpy() so users need to run DPDK unit test
> "memcpy_perf_autotest" and customize parameters in "customization
> section" in rte_memcpy_64.h for best performance.
>
> Compiler version will also impact the rte_memcpy() performance.
> It's observed on some platforms and with the same code, GCC 7.2.0
> compiled binary can provide better performance than GCC 4.8.5. It's
> suggested to use GCC 5.4.0 or later.
>
> Signed-off-by: Herbert Guan 
> ---
>  .../common/include/arch/arm/rte_memcpy_64.h| 193 
> +
>  1 file changed, 193 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/arch/arm/rte_memcpy_64.h 
> b/lib/librte_eal/common/include/arch/arm/rte_memcpy_64.h
> index b80d8ba..1f42b3c 100644
> --- a/lib/librte_eal/common/include/arch/arm/rte_memcpy_64.h
> +++ b/lib/librte_eal/common/include/arch/arm/rte_memcpy_64.h
> @@ -42,6 +42,197 @@
>
>  #include "generic/rte_memcpy.h"
>
> +#ifdef RTE_ARCH_ARM64_MEMCPY

There is an existing flag for arm32 to enable neon based memcpy
RTE_ARCH_ARM_NEON_MEMCPY we could reuse that here as restrict does the same.

> +#include 
> +#include 
> +
> +/***
> + * The memory copy performance differs on different AArch64 
> micro-architectures.
> + * And the most recent glibc (e.g. 2.23 or later) can provide a better 
> memcpy()
> + * performance compared to old glibc versions. It's always suggested to use a
> + * more recent glibc if possible, from which the entire system can get 
> benefit.
> + *
> + * This implementation improves memory copy on some aarch64 
> micro-architectures,
> + * when an old glibc (e.g. 2.19, 2.17...) is being used. It is disabled by
> + * default and needs "RTE_ARCH_ARM64_MEMCPY" defined to activate. It's not
> + * always providing better performance than memcpy() so users need to run 
> unit
> + * test "memcpy_perf_autotest" and customize parameters in customization 
> section
> + * below for best performance.
> + *
> + * Compiler version will also impact the rte_memcpy() performance. It's 
> observed
> + * on some platforms and with the same code, GCC 7.2.0 compiled binaries can
> + * provide better performance than GCC 4.8.5 compiled binaries.
> + 
> **/
> +
> +/**
> + * Beginning of customization section
> + **/
> +#define ALIGNMENT_MASK 0x0F
> +#ifndef RTE_ARCH_ARM64_MEMCPY_STRICT_ALIGN
> +// Only src unalignment will be treaed as unaligned copy
> +#define IS_UNALIGNED_COPY(dst, src) ((uintptr_t)(dst) & ALIGNMENT_MASK)

We can use existing `rte_is_aligned` function instead.

> +#else
> +// Both dst and src unalignment will be treated as unaligned copy
> +#define IS_UNALIGNED_COPY(dst, src) \
> + (((uintptr_t)(dst) | (uintptr_t)(src)) & ALIGNMENT_MASK)
> +#endif
> +
> +
> +// If copy size is larger than threshold, memcpy() will be used.
> +// Run "memcpy_perf_autotest" to determine the proper threshold.
> +#define ALIGNED_THRESHOLD   ((size_t)(0x))
> +#define UNALIGNED_THRESHOLD ((size_t)(0x))
> +
> +
> +/**
> + * End of customization section
> + **/
> +#ifdef RTE_TOOLCHAIN_GCC
> +#if (GCC_VERSION < 50400)
> +#warning "The GCC version is quite old, which may result in sub-optimal \
> +performance of the compiled code. It is suggested that at least GCC 5.4.0 \
> +be used."
> +#endif
> +#endif
> +
> +static inline void __attribute__ ((__always_inline__))
use __rte_always_inline instead.
> +rte_mov16(uint8_t *restrict dst, const uint8_t *restrict src)
> +{
> + __int128 * restrict dst128 = (__int128 * restrict)dst;
> + const __int128 * restrict src128 = (const __int128 * restrict)src;
> + *dst128 = *src128;
> +}
> +
> +static inline void __attribute__ ((__always_inline__))
> +rte_mov64(uint8_t *restrict dst, const uint