Re: [dpdk-dev] [EXT] [PATCH v1] ticketlock: ticket based to improve fairness

[Jerin Jacob Kollanukkaran]

On Sun, 2019-01-13 at 22:46 +0800, Gavin Hu wrote:
> ---
> ---
> From: Joyce Kong 
> 
> The spinlock implementation is unfair, some threads may take locks
> aggressively while leaving the other threads starving for long time.
> As
> shown in the following test, within same period of time, there are
> threads taking locks much more times than the others.
> 
> The ticketlock gives each waiting thread a ticket and they can take
> the
> lock one by one, first come, first serviced, this avoids starvation
> for
> too long time and is more predictable.
> 
> *** spinlock_autotest without this patch ***
> Core [0] count = 89
> Core [1] count = 84
> Core [2] count = 94
> ...
> Core [208] count = 171
> Core [209] count = 152
> Core [210] count = 161
> Core [211] count = 187
> 
> *** spinlock_autotest with this patch ***

This comment needs to be updated as it is new API now
and we need add test case for the same. I think,
making modification to exiting spinlock test case would be fine.


> Core [0] count = 534
> Core [1] count = 533
> Core [2] count = 542
> ...
> Core [208] count = 554
> Core [209] count = 556
> Core [210] count = 555
> Core [211] count = 551
> 
> Signed-off-by: Joyce Kong 
> Signed-off-by: Gavin Hu 
> ---
> 

Re: [dpdk-dev] [EXT] Default cacheline size for ARM

[Jerin Jacob Kollanukkaran]

On Mon, 2019-01-14 at 07:47 +, Honnappa Nagarahalli wrote:
> > On Sat, 2019-01-05 at 22:47 +, Honnappa Nagarahalli wrote:
> > > > On Fri, 2019-01-04 at 19:59 +, Yongseok Koh wrote:
> > > > > ---
> > > > > 
> > > > > 
> > > > > ---
> > > > > Hi,
> > > > > 
> > > > > The cacheline size (RTE_CACHE_LINE_SIZE) for ARM CPUs is set
> > > > > to be
> > > > > 128B by default. Mellanox's BlueField is an ARM CPU having
> > > > > Cortex-A72
> > > > > and its CL size is 64B.
> > > Just wondering how many devices are out there with 128B cache
> > > line? I
> > > also have not heard about any future devices with 128B cache
> > > line. If
> > > the majority is 64B, why not keep 64B as the default?
> > 
> > The problem is, In the armv8 spec the cache line size is
> > IMPLEMENTATION
> > DEFINED. Marvell's embedded processors has 128B CL and Server
> > processors
> > has 64B CL.
> > 
> > Assuming the defconfig_arm64-armv8a-linuxapp-gcc will be used by
> > distro
> > folks then that configuration should support all the devices with
> > armv8.1 spec. For instance, marvells new chips are armv8.2 but we
> > can not
> > make that as default.
> What will happen in the future when we will have v9, v10 etc? I think
> the distro(generic/binary portable) config should get rid of v8.

Will it be too much overhead if the image is same for v8, v9 and v10.
I think, we can inline with what distro folks are doing for 
other packages, I think, DPDK package does not need any exception.

Re: [dpdk-dev] [EXT] Re: [PATCH v3] build: use generic march on arm64 when using 'default' machine

[Jerin Jacob Kollanukkaran]

On Fri, 2019-01-11 at 10:45 +, Luca Boccassi wrote:
> External Email
> 
> ---
> ---
> On Mon, 2019-01-07 at 14:11 +, Luca Boccassi wrote:
> > When building for generic distribution we need a stable baseline
> > architecture, or depending on the build worker the result will
> > vary.
> > 
> > Force the default flags if the user explicitly sets machine=default
> > at configuration time.
> > 
> > Fixes: b1d48c41189a ("build: support ARM with meson")
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Luca Boccassi 
> > Acked-by: Bruce Richardson 
> > ---
> > v2: fix typo in commit message, remove variable used only once
> > v3: put back temporary variable, as "machine" gets overwritten
> > by the function and loses the original value before we need
> > it.
> > 
> >  config/arm/meson.build | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > index dae55d6b2..614139534 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -6,6 +6,7 @@
> >  march_opt = '-march=@0@'.format(machine)
> >  
> >  arm_force_native_march = false
> > +arm_force_default_march = (machine == 'default')
> >  
> >  machine_args_generic = [
> > ['default', ['-march=armv8-a+crc+crypto']],
> > @@ -105,7 +106,11 @@ else
> > cmd_generic = ['generic', '', '', 'default', '']
> > cmd_output = cmd_generic # Set generic by default
> > machine_args = [] # Clear previous machine args
> > -   if not meson.is_cross_build()
> > +   if arm_force_default_march and not meson.is_cross_build()
> > +   machine = impl_generic
> > +   cmd_output = cmd_generic
> > +   impl_pn = 'default'
> > +   elif not meson.is_cross_build()
> > # The script returns ['Implementer', 'Variant',
> > 'Architecture',
> > # 'Primary Part number', 'Revision']
> > detect_vendor = find_program(join_paths(
> 
> Any chance for a quick review from the arm maintainers/devs? Thanks!

Looks good.

Could you please document the procedure to build generic image some
where in the DPDK documentation.

I guess the procedure will be same for x86 and arm64. Right?

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Varghese, Vipin]

Hi Qi Z Zhang,

snipped
> >
> > Hi,
> >
> > Thanks Kevin for redirecting to the maintainer. Appreciate the help.
> >
> > Thanks Qi Z Zhang for the update. But the reason for request is
> > because I did not find the update in patches for 'release notes, faq or 
> > i40e'
> documentation.
> > Hence, I was forced to assume this is known bug.
> >
> > Will wait to see an update on either release notes, faq or i40e
> documentation.
> 
> Sorry I still didn't get your point
> The issue is not a knowing issue, as a common bug, it is observed by somebody
> and report to dev team.
> So I didn't see anything we can update on the i40e documentation since the
> issue is fixed.
> And it also does not impact any user experience, (no new feature, no knowing
> issue fix) so I didn’t see the point to update release notes also.

For the last couple of months, we have been receiving queries from customers 
like 'security issue for Side band channel, spectre, mail box, ring 
communication, vhost interface etc'. As per the update 'PMD from malicious VF 
message attack', having this tracked in release notes, faq or i40e serves the 
purpose of information update. So in my humble opinion security update patch 
should be mentioned in documentation with working Firmware.

I will leave this community opinion of either updating in a common place or PMD 
for such tracking.

> 
> 
> >
> > Thanks
> > Vipin Varghese
> >
> > > -Original Message-
> > > From: Zhang, Qi Z
> > > Sent: Friday, January 11, 2019 6:59 PM
> > > To: Varghese, Vipin ; Kevin Traynor
> > > ; Wang, Haiyue ;
> > > dev@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > validation on the VF messages
> > >
> > > Hi Vipin:
> > >
> > > > -Original Message-
> > > > From: Varghese, Vipin
> > > > Sent: Friday, January 11, 2019 10:54 AM
> > > > To: Kevin Traynor ; Wang, Haiyue
> > > > ; dev@dpdk.org; Zhang, Qi Z
> > > > 
> > > > Subject: RE: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > > validation on the VF messages
> > > >
> > > > Hi Kevin,
> > > >
> > > > A question, since the patch is fixing issue for 'i40e vf'  should
> > > > not the sections for 'known limitations' or 'i40e PMD' be updated too?
> > >
> > > The patch is going to fix some issue not be recorded as knowing
> > > limitation previously, so I didn’t see the necessary to update the doc.
> > > But please let me know if I missed your point.
> > >
> > > >
> > > > Thanks
> > > > Vipin Varghese
> > > >
> > > > > -Original Message-
> > > > > From: dev  On Behalf Of Kevin Traynor
> > > > > Sent: Thursday, January 10, 2019 11:18 PM
> > > > > To: Wang, Haiyue ; dev@dpdk.org; Zhang,
> > > > > Qi Z 
> > > > > Subject: Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic
> > > > > validation on the VF messages
> > > > >
> > > > > On 01/10/2019 12:07 PM, Haiyue Wang wrote:
> > > > > > Do the VF message basic validation such as OPCODE message
> > > > > > length check, some special OPCODE message format check, to
> > > > > > protect the i40e PMD from malicious VF message attack.
> > > > > >
> > > > > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > > > > >
> > > > >
> > > > > Missing Cc: sta...@dpdk.org ? or there is some reason not to backport?
> > > > >
> > > > > > Signed-off-by: Haiyue Wang 
> > > > > > ---
> > > > > >  drivers/net/i40e/i40e_pf.c | 25 +
> > > > > >  1 file changed, 25 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/net/i40e/i40e_pf.c
> > > > > > b/drivers/net/i40e/i40e_pf.c index 092e0d3..d6e83e3 100644
> > > > > > --- a/drivers/net/i40e/i40e_pf.c
> > > > > > +++ b/drivers/net/i40e/i40e_pf.c
> > > > > > @@ -1295,6 +1295,7 @@
> > > > > > uint16_t vf_id = abs_vf_id - hw->func_caps.vf_base_id;
> > > > > > struct rte_pmd_i40e_mb_event_param ret_param;
> > > > > > bool b_op = TRUE;
> > > > > > +   int ret;
> > > > > >
> > > > > > if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > > > > > PMD_DRV_LOG(ERR, "invalid argument"); @@ -1309,6
> > > > > +1310,30 @@
> > > > > > return;
> > > > > > }
> > > > > >
> > > > > > +   /* perform basic checks on the msg */
> > > > > > +   ret = virtchnl_vc_validate_vf_msg(&vf->version, opcode, msg,
> > > > > > +msglen);
> > > > > > +
> > > > > > +   /* perform additional checks specific to this driver */
> > > > > > +   if (opcode == VIRTCHNL_OP_CONFIG_RSS_KEY) {
> > > > > > +   struct virtchnl_rss_key *vrk = (struct virtchnl_rss_key
> > > > > > +*)msg;
> > > > > > +
> > > > > > +   if (vrk->key_len != ((I40E_PFQF_HKEY_MAX_INDEX + 1)
> * 4))
> > > > > > +   ret = VIRTCHNL_ERR_PARAM;
> > > > > > +   } else if (opcode == VIRTCHNL_OP_CONFIG_RSS_LUT) {
> > > > > > +   struct virtchnl_rss_lut *vrl = (struct virtchnl_rss_lut
> > > > > > +*)msg;
> > > > > > +
> > > > > > +   if (vrl->lut_entries != ((I40E_VFQF_HLUT1_MAX_INDEX
> + 1) *
> > > > > 4))
> > > > > > +   ret = VIRTCHNL_ERR_PARAM;

[dpdk-dev] [PATCH] net/i40e: fix FPK VF overwrite PF RSS LUT

[Yanglong Wu]

i40e_aq_set_rss_lut should set rigister according
to the vsi->type. if not, VF may overwrite the rigister
of PF

Fixes: d0a349409bd7 ("i40e: support AQ based RSS config")
Cc: sta...@dpdk.org

Signed-off-by: Wu Yanglong 
---
 drivers/net/i40e/i40e_ethdev.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index a6b97e164..d4d99edcd 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -4183,8 +4183,13 @@ i40e_get_rss_lut(struct i40e_vsi *vsi, uint8_t *lut, 
uint16_t lut_size)
return -EINVAL;
 
if (pf->flags & I40E_FLAG_RSS_AQ_CAPABLE) {
-   ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, TRUE,
- lut, lut_size);
+   if (vsi->type == I40E_VSI_SRIOV) {
+   ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, FALSE,
+ lut, lut_size);
+   } else {
+   ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, TRUE,
+ lut, lut_size);
+   }
if (ret) {
PMD_DRV_LOG(ERR, "Failed to get RSS lookup table");
return ret;
-- 
2.11.0

Re: [dpdk-dev] [pull-request] next-crypto 19.02 rc2

[Thomas Monjalon]

11/01/2019 18:49, Thomas Monjalon:
> 11/01/2019 07:08, Pablo de Lara:
> >   http://dpdk.org/git/next/dpdk-next-crypto 
> 
> "Meson" compilation is not working if IPSec_MB is not installed.

This meson issue has been fixed with help of Pablo.

Another issue has been fixed in vhost-crypto patches (moving change
from one patch to another for fixing build).

> "Make" compilation has no such big issue but it does not respect
> EXTRA_CFLAGS when assuming IMB_HDR = /usr/include/intel-ipsec-mb.h

This makefile issue should be fixed later.

The fixed tree is integrated in master.

[dpdk-dev] [PATCH v2] test/hash: reset iter and found in perf test

[Dharmik Thakkar]

Reset 'iter' and 'tbl_rw_test_param.found' on each iteration
to give correct result for lost and duplicated keys.

This patch also changes the default return value of the test to -1
when not enough resources are provided.

Fixes: 0eb3726ebcf14 ("test/hash: add test for read/write concurrency")
Cc: sta...@dpdk.org

Signed-off-by: Dharmik Thakkar 
Reviewed-by: Ferruh Yigit 
Reviewed-by: Yipeng Wang 
--
v2:
* Update commit message
* Change 'found' array type to uint8_t
* Restore declaration of 'iter' to the start
---
 test/test/test_hash_readwrite.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index 6b695ce6e444..480ae97d857d 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -39,7 +39,7 @@ static struct perf htm_results, non_htm_results;
 
 struct {
uint32_t *keys;
-   uint32_t *found;
+   uint8_t *found;
uint32_t num_insert;
uint32_t rounded_tot_insert;
struct rte_hash *h;
@@ -126,7 +126,7 @@ init_params(int use_ext, int use_htm, int use_jhash)
unsigned int i;
 
uint32_t *keys = NULL;
-   uint32_t *found = NULL;
+   uint8_t *found = NULL;
struct rte_hash *handle;
 
struct rte_hash_parameters hash_params = {
@@ -173,7 +173,7 @@ init_params(int use_ext, int use_htm, int use_jhash)
goto err;
}
 
-   found = rte_zmalloc(NULL, sizeof(uint32_t) * TOTAL_ENTRY, 0);
+   found = rte_zmalloc(NULL, sizeof(uint8_t) * TOTAL_ENTRY, 0);
if (found == NULL) {
printf("RTE_ZMALLOC failed\n");
goto err;
@@ -361,7 +361,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int 
use_htm,
 
const void *next_key;
void *next_data;
-   uint32_t iter = 0;
+   uint32_t iter;
int use_jhash = 0;
 
uint32_t duplicated_keys = 0;
@@ -536,6 +536,8 @@ test_hash_readwrite_perf(struct perf *perf_results, int 
use_htm,
 
rte_eal_mp_wait_lcore();
 
+   iter = 0;
+   memset(tbl_rw_test_param.found, 0, TOTAL_ENTRY);
while (rte_hash_iterate(tbl_rw_test_param.h,
&next_key, &next_data, &iter) >= 0) {
/* Search for the key in the list of keys added .*/
@@ -619,7 +621,7 @@ test_hash_readwrite_main(void)
if (rte_lcore_count() <= 2) {
printf("More than two lcores are required "
"to do read write test\n");
-   return 0;
+   return -1;
}
 
RTE_LCORE_FOREACH_SLAVE(core_id) {
-- 
2.17.1

[dpdk-dev] mbuf file in BPF test

[Thomas Monjalon]

Hi Konstantin,

There is an outdated copy of rte_mbuf.h in test/bpf/mbuf.h.
It is bringing confusion when grepping code.
Would it be possible to remove this file?

[dpdk-dev] [Bug 188] significant multi-core performance degradation with SR-IOV/macswap in some cases

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=188

Bug ID: 188
   Summary: significant multi-core performance degradation with
SR-IOV/macswap in some cases
   Product: DPDK
   Version: 19.02
  Hardware: x86
OS: Linux
Status: CONFIRMED
  Severity: critical
  Priority: Normal
 Component: testpmd
  Assignee: dev@dpdk.org
  Reporter: alia...@mellanox.com
  Target Milestone: ---

Hi,

I'm seeing a performance degradation with testpmd running with multiple cores.
I saw the issue with ConnectX-3, ConnectX-4 Lx, and ConnectX-5.

### For ConnectX-3:
- With 2 Cores, I see a degradation from 19.2 mpps to 13.5 mpps.

testpmd command:
`MLX4_INLINE_RECV_SIZE=64 ./build/app/test-pmd/testpmd -c 0x7 -n 4 -w
0002:00:02.0 --vdev=net_vdev_netvsc,ignore=1 --socket-mem=2048 --
--port-numa-config=0,0 --socket-num=0 --burst=64 --txd=256 --rxd=256
--mbcache=512  --rxq=2 --txq=2 --nb-cores=2 -i -a --forward-mode=macswap`


- With 4 Cores, I see a degradation from 19.2 mpps - 13.9 mpps.

testpmd command:
`MLX4_INLINE_RECV_SIZE=64 ./build/app/test-pmd/testpmd -c 0x1f -n 4 -w
0002:00:02.0 --vdev=net_vdev_netvsc,ignore=1 --socket-mem=2048 --
--port-numa-config=0,0 --socket-num=0 --burst=64 --txd=256 --rxd=256
--mbcache=512  --rxq=2 --txq=2 --nb-cores=4 -i -a --forward-mode=macswap`


### For ConnectX-4 Lx:

- With 2 Cores, I see a degradation from 32.9 mpps to 26.8 mpps.

testpmd command:
`./build/app/test-pmd/testpmd -c 0x7 -n 4 -w 0003:00:02.0
--vdev="net_vdev_netvsc,ignore=1" -- --burst=64 --txd=1024 --rxd=256
--mbcache=512 --rxq=2 --txq=2 --nb-cores=2 --rss-udp --port-topology=chained
--forward-mode=macswap -i -a`

- With 4 Cores, I see a degradation from 34.8 mpps to 19 mpps.

testpmd command:
`./build/app/test-pmd/testpmd -c 0x1f -n 4 -w 0003:00:02.0
--vdev="net_vdev_netvsc,ignore=1" -- --burst=64 --txd=8192 --rxd=256
--mbcache=512 --rxq=4 --txq=4 --nb-cores=4 --rss-udp --port-topology=chained
--forward-mode=macswap -i -a`


Server specs:
OS: Ubuntu 16.04.2 LTS
Kernel: 4.15.0-rc3

I'm using IXIA as a traffic generator, and to get the forwarding performance.

DPDK version: master:a89fbda

The cause of the degradation is the patch:

```
commit 62b52877adbe8d3ad19615b0827e7725e51feadc
Author: Qi Zhang 
Date:   Sun Dec 16 08:58:35 2018 +0800

app/testpmd: batch MAC swap for performance on x86

Do four packets macswap in same loop iterate to squeeze more
CPU cycles.

Signed-off-by: Qi Zhang 
Reviewed-by: Ferruh Yigit 
Acked-by: Bernard Iremonger 
```

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

Re: [dpdk-dev] [PATCH 1/2] mbuf: remove deprecated macro

[Thomas Monjalon]

14/01/2019 08:40, Andrew Rybchenko:
> On 1/14/19 8:20 AM, Yongseok Koh wrote:
> > RTE_MBUF_INDIRECT() is replaced with RTE_MBUF_CLONED() and removed.
> > This macro was deprecated in release 18.05 when EXT_ATTACHED_MBUF was
> > introduced.
> >
> > Signed-off-by: Yongseok Koh 
> 
> Acked-by: Andrew Rybchenko 
> 
> <...>
> 
> > diff --git a/doc/guides/rel_notes/release_19_02.rst 
> > b/doc/guides/rel_notes/release_19_02.rst
> > index 5a46f1acdc..c4ad072412 100644
> > --- a/doc/guides/rel_notes/release_19_02.rst
> > +++ b/doc/guides/rel_notes/release_19_02.rst
> > @@ -210,6 +210,9 @@ API Changes
> > function from now on. Failed to do so will cause
> > ``rte_cryptodev_sym_session_create()`` function call return error.
> >   
> > +* mbuf: ``RTE_MBUF_INDIRECT()``, which was deprecated in 18.05, was 
> > replaced
> > +  with ``RTE_MBUF_CLONED()`` and removed in 19.02.
> > +
> 
> May be order in New Features should be applicable in API changes as well.

Yes
Usually I take care of it when applying, but would be better to be fixed
in the patch itself :)

Re: [dpdk-dev] [EXT] Re: [PATCH v3] build: use generic march on arm64 when using 'default' machine

[Luca Boccassi]

On Mon, 2019-01-14 at 08:09 +, Jerin Jacob Kollanukkaran wrote:
> On Fri, 2019-01-11 at 10:45 +, Luca Boccassi wrote:
> > External Email
> > 
> > ---
> > ---
> > On Mon, 2019-01-07 at 14:11 +, Luca Boccassi wrote:
> > > When building for generic distribution we need a stable baseline
> > > architecture, or depending on the build worker the result will
> > > vary.
> > > 
> > > Force the default flags if the user explicitly sets
> > > machine=default
> > > at configuration time.
> > > 
> > > Fixes: b1d48c41189a ("build: support ARM with meson")
> > > Cc: sta...@dpdk.org
> > > 
> > > Signed-off-by: Luca Boccassi 
> > > Acked-by: Bruce Richardson 
> > > ---
> > > v2: fix typo in commit message, remove variable used only once
> > > v3: put back temporary variable, as "machine" gets overwritten
> > > by the function and loses the original value before we need
> > > it.
> > > 
> > >  config/arm/meson.build | 7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > index dae55d6b2..614139534 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -6,6 +6,7 @@
> > >  march_opt = '-march=@0@'.format(machine)
> > >  
> > >  arm_force_native_march = false
> > > +arm_force_default_march = (machine == 'default')
> > >  
> > >  machine_args_generic = [
> > >   ['default', ['-march=armv8-a+crc+crypto']],
> > > @@ -105,7 +106,11 @@ else
> > >   cmd_generic = ['generic', '', '', 'default', '']
> > >   cmd_output = cmd_generic # Set generic by default
> > >   machine_args = [] # Clear previous machine args
> > > - if not meson.is_cross_build()
> > > + if arm_force_default_march and not
> > > meson.is_cross_build()
> > > + machine = impl_generic
> > > + cmd_output = cmd_generic
> > > + impl_pn = 'default'
> > > + elif not meson.is_cross_build()
> > >   # The script returns ['Implementer', 'Variant',
> > > 'Architecture',
> > >   # 'Primary Part number', 'Revision']
> > >   detect_vendor = find_program(join_paths(
> > 
> > Any chance for a quick review from the arm maintainers/devs?
> > Thanks!
> 
> Looks good.
> 
> Could you please document the procedure to build generic image some
> where in the DPDK documentation.
> 
> I guess the procedure will be same for x86 and arm64. Right?

The "machine" meson option is already described in meson_options.txt,
is that enough?

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH 1/2] app/testpmd: fix wrong pointer reference in macswap

[Yongseok Koh]

The pointer is misused and could set wrong MAC address. As a result, some
of packets can be dropped in receiver side due to MAC address mismatch.
This can be shown as performance degradation.

Bugzilla ID: 188

Fixes: 62b52877adbe ("app/testpmd: batch MAC swap for performance on x86")
Cc: qi.z.zh...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Yongseok Koh 
---
 app/test-pmd/macswap_sse.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/macswap_sse.h b/app/test-pmd/macswap_sse.h
index 7d268bfbb0..662fe79cf9 100644
--- a/app/test-pmd/macswap_sse.h
+++ b/app/test-pmd/macswap_sse.h
@@ -76,7 +76,7 @@ do_macswap(struct rte_mbuf *pkts[], uint16_t nb,
eth_hdr[0] = rte_pktmbuf_mtod(mb[0], struct ether_hdr *);
 
/* Swap dest and src mac addresses. */
-   addr0 = _mm_loadu_si128((__m128i *)eth_hdr);
+   addr0 = _mm_loadu_si128((__m128i *)eth_hdr[0]);
addr0 = _mm_shuffle_epi8(addr0, shfl_msk);
_mm_storeu_si128((__m128i *)eth_hdr[0], addr0);
 
-- 
2.11.0

[dpdk-dev] [PATCH 2/2] app/testpmd: fix missing prefetch in macswap mode

[Yongseok Koh]

Prefetching packet was missing when do_macswap() was optimized.

Fixes: 62b52877adbe ("app/testpmd: batch MAC swap for performance on x86")
Cc: qi.z.zh...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Yongseok Koh 
---
 app/test-pmd/macswap_sse.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/app/test-pmd/macswap_sse.h b/app/test-pmd/macswap_sse.h
index 662fe79cf9..2b6e7324da 100644
--- a/app/test-pmd/macswap_sse.h
+++ b/app/test-pmd/macswap_sse.h
@@ -35,6 +35,13 @@ do_macswap(struct rte_mbuf *pkts[], uint16_t nb,
r = nb;
 
while (r >= 4) {
+   if (r >= 8) {
+   rte_prefetch0(rte_pktmbuf_mtod(pkts[i + 4], void *));
+   rte_prefetch0(rte_pktmbuf_mtod(pkts[i + 5], void *));
+   rte_prefetch0(rte_pktmbuf_mtod(pkts[i + 6], void *));
+   rte_prefetch0(rte_pktmbuf_mtod(pkts[i + 7], void *));
+   }
+
mb[0] = pkts[i++];
eth_hdr[0] = rte_pktmbuf_mtod(mb[0], struct ether_hdr *);
addr0 = _mm_loadu_si128((__m128i *)eth_hdr[0]);
-- 
2.11.0

[dpdk-dev] [PATCH] ip_pipeline: add QinQ PPPoE encapsulation action

[Marjanovic,Nemanja]

Add implementation of QinQ PPPoE packet encapsulation action.

Signed-off-by: Nemanja Marjanovic 
---
 examples/ip_pipeline/cli.c | 43 +-
 lib/librte_net/rte_ether.h |  1 +
 lib/librte_pipeline/rte_table_action.c | 61 ++
 lib/librte_pipeline/rte_table_action.h | 16 +++
 4 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c
index 910386282..dcc98ee65 100644
--- a/examples/ip_pipeline/cli.c
+++ b/examples/ip_pipeline/cli.c
@@ -1290,11 +1290,13 @@ cmd_table_action_profile(char **tokens,
 
p.encap.encap_mask = 1LLU << 
RTE_TABLE_ACTION_ENCAP_VXLAN;
n_extra_tokens = 5;
-   } else {
+   } else if (strcmp(tokens[t0 + 1], "qinq_pppoe") == 0)
+   p.encap.encap_mask =
+   1LLU << RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE;
+   else {
snprintf(out, out_size, MSG_ARG_MISMATCH, "encap");
return;
}
-
p.action_mask |= 1LLU << RTE_TABLE_ACTION_ENCAP;
t0 += 2 + n_extra_tokens;
} /* encap */
@@ -3391,6 +3393,43 @@ parse_table_action_encap(char **tokens,
return 1 + 9;
}
 
+   /* qinq_pppoe */
+   if (n_tokens && (strcmp(tokens[0], "qinq_pppoe") == 0)) {
+   uint32_t svlan_pcp, svlan_dei, svlan_vid;
+   uint32_t cvlan_pcp, cvlan_dei, cvlan_vid;
+
+   if ((n_tokens < 10) ||
+   parse_mac_addr(tokens[1],
+   &a->encap.qinq_pppoe.ether.da) ||
+   parse_mac_addr(tokens[2],
+   &a->encap.qinq_pppoe.ether.sa) ||
+   parser_read_uint32(&svlan_pcp, tokens[3]) ||
+   (svlan_pcp > 0x7) ||
+   parser_read_uint32(&svlan_dei, tokens[4]) ||
+   (svlan_dei > 0x1) ||
+   parser_read_uint32(&svlan_vid, tokens[5]) ||
+   (svlan_vid > 0xFFF) ||
+   parser_read_uint32(&cvlan_pcp, tokens[6]) ||
+   (cvlan_pcp > 0x7) ||
+   parser_read_uint32(&cvlan_dei, tokens[7]) ||
+   (cvlan_dei > 0x1) ||
+   parser_read_uint32(&cvlan_vid, tokens[8]) ||
+   (cvlan_vid > 0xFFF) ||
+   
parser_read_uint16(&a->encap.qinq_pppoe.pppoe.session_id,
+   tokens[9]))
+   return 0;
+
+   a->encap.qinq_pppoe.svlan.pcp = svlan_pcp & 0x7;
+   a->encap.qinq_pppoe.svlan.dei = svlan_dei & 0x1;
+   a->encap.qinq_pppoe.svlan.vid = svlan_vid & 0xFFF;
+   a->encap.qinq_pppoe.cvlan.pcp = cvlan_pcp & 0x7;
+   a->encap.qinq_pppoe.cvlan.dei = cvlan_dei & 0x1;
+   a->encap.qinq_pppoe.cvlan.vid = cvlan_vid & 0xFFF;
+   a->encap.type = RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE;
+   a->action_mask |= 1 << RTE_TABLE_ACTION_ENCAP;
+   return 1 + 10;
+   }
+
/* mpls */
if (n_tokens && (strcmp(tokens[0], "mpls") == 0)) {
uint32_t label, tc, ttl;
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index c2c5e249f..212ead64a 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -300,6 +300,7 @@ struct vxlan_hdr {
 #define ETHER_TYPE_ARP  0x0806 /**< Arp Protocol. */
 #define ETHER_TYPE_RARP 0x8035 /**< Reverse Arp Protocol. */
 #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
+#define ETHER_TYPE_PPPoE 0x8864 /**< PPPoE Session Stage */
 #define ETHER_TYPE_QINQ 0x88A8 /**< IEEE 802.1ad QinQ tagging. */
 #define ETHER_TYPE_ETAG 0x893F /**< IEEE 802.1BR E-Tag. */
 #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time Protocol. */
diff --git a/lib/librte_pipeline/rte_table_action.c 
b/lib/librte_pipeline/rte_table_action.c
index 7c7c8dd82..78cc7a7a2 100644
--- a/lib/librte_pipeline/rte_table_action.c
+++ b/lib/librte_pipeline/rte_table_action.c
@@ -432,6 +432,7 @@ encap_valid(enum rte_table_action_encap_type encap)
case RTE_TABLE_ACTION_ENCAP_MPLS:
case RTE_TABLE_ACTION_ENCAP_PPPOE:
case RTE_TABLE_ACTION_ENCAP_VXLAN:
+   case RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE:
return 1;
default:
return 0;
@@ -532,6 +533,13 @@ struct encap_vxlan_ipv6_vlan_data {
struct vxlan_hdr vxlan;
 } __attribute__((__packed__));
 
+struct encap_qinq_pppoe_data {
+   struct ether_hdr ether;
+   struct vlan_hdr svlan;
+   struct vlan_hdr cvlan;
+   struct pppoe_ppp_hdr pppoe_ppp;
+} __attribute__((__packed__));
+
 static size_t
 encap_data_size(struct rte_table_action_encap_config *encap)
 

Re: [dpdk-dev] [PATCH v2] app/testpmd: add tos and ttl field to vxlan encapsulation

[Iremonger, Bernard]

+Ori, Adrien 

> -Original Message-
> From: Shahaf Shuler [mailto:shah...@mellanox.com]
> Sent: Monday, January 14, 2019 6:04 AM
> To: Slava Ovsiienko ; sha...@mellanox.com;
> Yigit, Ferruh ; Lu, Wenzhuo ;
> Wu, Jingjing ; Iremonger, Bernard
> 
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] app/testpmd: add tos and ttl field to vxlan
> encapsulation
> 
> Adding some relevant folks.
> 
> Any comments or concerns on this patch?
> 
> Sunday, January 13, 2019 4:40 PM, Viacheslav Ovsiienko:
> > Subject: [dpdk-dev] [PATCH v2] app/testpmd: add tos and ttl field to
> > vxlan encapsulation
> >
> > The new testpmd set vxlan-tos-ttl command is added. It allows to
> > specify tos and tll fields for encapsulation IP header.
> >
> > IPv4 VXLAN outer header:
> >
> >   testpmd> set vxlan-tos-ttl ip-version ipv4 vni 4 udp-src 4
> >udp-dst 4 ip-tos 0 ip-ttl 255 ip-src 127.0.0.1
> >ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11
> >eth-dst 22:22:22:22:22:22
> >
> > IPv6 VXLAN outer header:
> >   testpmd> set vxlan-tos-ttl ip-version ipv6 vni 4 udp-src 4
> >udp-dst 4 ip-tos 0 ip-ttl 255 ::1 ip-dst ::
> >eth-src 11:11:11:11:11:11 eth-dst
> >22:22:22:22:22:22
> >
> > Note: ip-ttl parameter corresponds the nop_limits field for IPv6.
> >
> > Signed-off-by: Viacheslav Ovsiienko 
> > ---
> >
> > v2:
> >   The patchset is splitted into two parts, one is related to mlx5 PMD, and
> >   another is update for testpmd, these parts are going to be merged into
> >   different next branches.
> >
> > v1:
> >   Original patchset:
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> > ches.dpdk.org%2Fproject%2Fdpdk%2Flist%2F%3Fseries%3D2974&data
> > =02%7C01%7Cshahafs%40mellanox.com%7C8d911e89ae384e23a1f908d6796
> >
> 50b0f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636829872232
> 5
> > 87143&sdata=EMLbTbGYyxdYdEdZ4IFsvaKoDRD4oQrmWThJD9Xqsr8%3
> > D&reserved=0
> >
> >  app/test-pmd/cmdline.c  | 63
> > +
> >  app/test-pmd/cmdline_flow.c | 32 +++
> >  app/test-pmd/testpmd.c  |  3 ++
> >  app/test-pmd/testpmd.h  |  3 ++
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 16 
> >  5 files changed, 117 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 3ddc3e0..9e9e898 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -794,6 +794,12 @@ static void cmd_help_long_parsed(void
> > *parsed_result,
> > " eth-dst (eth-dst)\n"
> > "   Configure the VXLAN encapsulation for
> > flows.\n\n"
> >
> > +   "vxlan-tos-ttl ip-version (ipv4|ipv6) vni (vni) udp-src"
> > +   " (udp-src) udp-dst (udp-dst) ip-tos (ip-tos) ip-ttl 
> > (ip-
> > ttl)"
> > +   " ip-src (ip-src) ip-dst (ip-dst) eth-src (eth-src)"
> > +   " eth-dst (eth-dst)\n"
> > +   "   Configure the VXLAN encapsulation for
> > flows.\n\n"
> > +
> > "nvgre ip-version (ipv4|ipv6) tni (tni) ip-src"
> > " (ip-src) ip-dst (ip-dst) eth-src (eth-src) eth-dst"
> > " (eth-dst)\n"
> > @@ -15034,6 +15040,8 @@ struct cmd_set_vxlan_result {
> > cmdline_ipaddr_t ip_src;
> > cmdline_ipaddr_t ip_dst;
> > uint16_t tci;
> > +   uint8_t tos;
> > +   uint8_t ttl;
> > struct ether_addr eth_src;
> > struct ether_addr eth_dst;
> >  };
> > @@ -15042,6 +15050,9 @@ struct cmd_set_vxlan_result {
> > TOKEN_STRING_INITIALIZER(struct cmd_set_vxlan_result, set, "set");
> > cmdline_parse_token_string_t cmd_set_vxlan_vxlan =
> > TOKEN_STRING_INITIALIZER(struct cmd_set_vxlan_result, vxlan,
> > "vxlan");
> > +cmdline_parse_token_string_t cmd_set_vxlan_vxlan_tos_ttl =
> > +   TOKEN_STRING_INITIALIZER(struct cmd_set_vxlan_result, vxlan,
> > +"vxlan-tos-ttl");
> >  cmdline_parse_token_string_t cmd_set_vxlan_vxlan_with_vlan =
> > TOKEN_STRING_INITIALIZER(struct cmd_set_vxlan_result, vxlan,
> >  "vxlan-with-vlan");
> > @@ -15066,6 +15077,16 @@ struct cmd_set_vxlan_result {
> >  "udp-dst");
> >  cmdline_parse_token_num_t cmd_set_vxlan_udp_dst_value =
> > TOKEN_NUM_INITIALIZER(struct cmd_set_vxlan_result, udp_dst,
> UINT16);
> > +cmdline_parse_token_string_t cmd_set_vxlan_ip_tos =
> > +   TOKEN_STRING_INITIALIZER(struct cmd_set_vxlan_result,
> > pos_token,
> > +"ip-tos");
> > +cmdline_parse_token_num_t cmd_set_vxlan_ip_tos_value =
> > +   TOKEN_NUM_INITIALIZER(struct cmd_set_vxlan_result, tos, UINT8);
> > +cmdline_parse_token_string_t cmd_set_vxlan_ip_ttl =
> > +   TOKEN_STRING_INITIALIZER(struct cmd_set_vxlan_result,
> > pos_token,
> > +"ip-ttl");
> > +cmdline_parse_token_num_t cmd_set

Re: [dpdk-dev] [PATCH v3] config: add Mellanox BlueField ARMv8 SoC

[Yongseok Koh]

> On Jan 13, 2019, at 10:13 PM, Honnappa Nagarahalli 
>  wrote:
> 
> Similarly, the commit heading subject line also needs to change.
> 
>> 
>>> 
>>> BlueField is Mellanox's new SoC based on ARMv8. BlueField integrates
>>  
>> ^^ Above should be
>> changed to armv8

Good to know that, thanks, Honnappa.

Thomas, will you take care of it when you merge (hope you will :-)?
Shall I send out a new version?

Thanks,
Yongseok
>> 
>>> Mellanox ConnectX-5 interconnect and Cortex-A72 cores into a single device.
>>> 
>>> 
>> http://www.mellanox.com/page/products_dyn?product_family=256&mtag=s
>>> oc_overview
>>> 
>>> Signed-off-by: Yongseok Koh 
>>> ---
>>> 
>>> v3:
>>> * remove redundant mlx5 config items
>>> 
>>> v2:
>>> * remove config items which can be used for external PCIe device
>>> 
>>> config/defconfig_arm64-bluefield-linuxapp-gcc | 18 ++
>>> 1 file changed, 18 insertions(+)
>>> create mode 100644 config/defconfig_arm64-bluefield-linuxapp-gcc
>>> 
>>> diff --git a/config/defconfig_arm64-bluefield-linuxapp-gcc
>>> b/config/defconfig_arm64-bluefield-linuxapp-gcc
>>> new file mode 100644
>>> index 00..dd252c0e47
>>> --- /dev/null
>>> +++ b/config/defconfig_arm64-bluefield-linuxapp-gcc
>>> @@ -0,0 +1,18 @@
>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright 2019 Mellanox
>>> +Technologies, Ltd #
>>> +
>>> +#include "defconfig_arm64-armv8a-linuxapp-gcc"
>>> +
>>> +# Mellanox BlueField
>>> +CONFIG_RTE_ARCH_ARM_TUNE="cortex-a72"
>>> +
>>> +CONFIG_RTE_MAX_NUMA_NODES=1
>>> +CONFIG_RTE_CACHE_LINE_SIZE=64
>>> +
>>> +# UMA architecture
>>> +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
>>> +CONFIG_RTE_LIBRTE_VHOST_NUMA=n
>>> +
>>> +# PMD for ConnectX-5
>>> +CONFIG_RTE_LIBRTE_MLX5_PMD=y
>>> --
>>> 2.11.0
> 

Re: [dpdk-dev] [PATCH 1/2] vhost: fix memory leak on realloc failure

[Maxime Coquelin]

On 1/14/19 4:34 AM, Tiwei Bie wrote:

When realloc() fails, the original block isn't freed.

Fixes: e246896178e6 ("vhost: get guest/host physical address mappings")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/vhost_user.c | 3 +++
  1 file changed, 3 insertions(+)




Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 2/2] examples/vhost: fix realloc failure

[Maxime Coquelin]

On 1/14/19 4:34 AM, Tiwei Bie wrote:

Fixes: ad0eef4d2203 ("examples/vhost: support multiple socket files")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  examples/vhost/main.c | 8 
  1 file changed, 8 insertions(+)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

[dpdk-dev] [PATCH v2] ip_pipeline: add QinQ PPPoE encapsulation action

[Marjanovic,Nemanja]

Add implementation of QinQ PPPoE packet encapsulation action.

Signed-off-by: Nemanja Marjanovic 
---
v2:Fix changes from check patch
---
 examples/ip_pipeline/cli.c | 43 +-
 lib/librte_net/rte_ether.h |  1 +
 lib/librte_pipeline/rte_table_action.c | 61 ++
 lib/librte_pipeline/rte_table_action.h | 16 +++
 4 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c
index 910386282..dcc98ee65 100644
--- a/examples/ip_pipeline/cli.c
+++ b/examples/ip_pipeline/cli.c
@@ -1290,11 +1290,13 @@ cmd_table_action_profile(char **tokens,
 
p.encap.encap_mask = 1LLU << 
RTE_TABLE_ACTION_ENCAP_VXLAN;
n_extra_tokens = 5;
-   } else {
+   } else if (strcmp(tokens[t0 + 1], "qinq_pppoe") == 0)
+   p.encap.encap_mask =
+   1LLU << RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE;
+   else {
snprintf(out, out_size, MSG_ARG_MISMATCH, "encap");
return;
}
-
p.action_mask |= 1LLU << RTE_TABLE_ACTION_ENCAP;
t0 += 2 + n_extra_tokens;
} /* encap */
@@ -3391,6 +3393,43 @@ parse_table_action_encap(char **tokens,
return 1 + 9;
}
 
+   /* qinq_pppoe */
+   if (n_tokens && (strcmp(tokens[0], "qinq_pppoe") == 0)) {
+   uint32_t svlan_pcp, svlan_dei, svlan_vid;
+   uint32_t cvlan_pcp, cvlan_dei, cvlan_vid;
+
+   if ((n_tokens < 10) ||
+   parse_mac_addr(tokens[1],
+   &a->encap.qinq_pppoe.ether.da) ||
+   parse_mac_addr(tokens[2],
+   &a->encap.qinq_pppoe.ether.sa) ||
+   parser_read_uint32(&svlan_pcp, tokens[3]) ||
+   (svlan_pcp > 0x7) ||
+   parser_read_uint32(&svlan_dei, tokens[4]) ||
+   (svlan_dei > 0x1) ||
+   parser_read_uint32(&svlan_vid, tokens[5]) ||
+   (svlan_vid > 0xFFF) ||
+   parser_read_uint32(&cvlan_pcp, tokens[6]) ||
+   (cvlan_pcp > 0x7) ||
+   parser_read_uint32(&cvlan_dei, tokens[7]) ||
+   (cvlan_dei > 0x1) ||
+   parser_read_uint32(&cvlan_vid, tokens[8]) ||
+   (cvlan_vid > 0xFFF) ||
+   
parser_read_uint16(&a->encap.qinq_pppoe.pppoe.session_id,
+   tokens[9]))
+   return 0;
+
+   a->encap.qinq_pppoe.svlan.pcp = svlan_pcp & 0x7;
+   a->encap.qinq_pppoe.svlan.dei = svlan_dei & 0x1;
+   a->encap.qinq_pppoe.svlan.vid = svlan_vid & 0xFFF;
+   a->encap.qinq_pppoe.cvlan.pcp = cvlan_pcp & 0x7;
+   a->encap.qinq_pppoe.cvlan.dei = cvlan_dei & 0x1;
+   a->encap.qinq_pppoe.cvlan.vid = cvlan_vid & 0xFFF;
+   a->encap.type = RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE;
+   a->action_mask |= 1 << RTE_TABLE_ACTION_ENCAP;
+   return 1 + 10;
+   }
+
/* mpls */
if (n_tokens && (strcmp(tokens[0], "mpls") == 0)) {
uint32_t label, tc, ttl;
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index c2c5e249f..212ead64a 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -300,6 +300,7 @@ struct vxlan_hdr {
 #define ETHER_TYPE_ARP  0x0806 /**< Arp Protocol. */
 #define ETHER_TYPE_RARP 0x8035 /**< Reverse Arp Protocol. */
 #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
+#define ETHER_TYPE_PPPoE 0x8864 /**< PPPoE Session Stage */
 #define ETHER_TYPE_QINQ 0x88A8 /**< IEEE 802.1ad QinQ tagging. */
 #define ETHER_TYPE_ETAG 0x893F /**< IEEE 802.1BR E-Tag. */
 #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time Protocol. */
diff --git a/lib/librte_pipeline/rte_table_action.c 
b/lib/librte_pipeline/rte_table_action.c
index 7c7c8dd82..78cc7a7a2 100644
--- a/lib/librte_pipeline/rte_table_action.c
+++ b/lib/librte_pipeline/rte_table_action.c
@@ -432,6 +432,7 @@ encap_valid(enum rte_table_action_encap_type encap)
case RTE_TABLE_ACTION_ENCAP_MPLS:
case RTE_TABLE_ACTION_ENCAP_PPPOE:
case RTE_TABLE_ACTION_ENCAP_VXLAN:
+   case RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE:
return 1;
default:
return 0;
@@ -532,6 +533,13 @@ struct encap_vxlan_ipv6_vlan_data {
struct vxlan_hdr vxlan;
 } __attribute__((__packed__));
 
+struct encap_qinq_pppoe_data {
+   struct ether_hdr ether;
+   struct vlan_hdr svlan;
+   struct vlan_hdr cvlan;
+   struct pppoe_ppp_hdr pppoe_ppp;
+} __attribute__((__packed__));
+
 static size_t
 encap_data_size(struct rte

Re: [dpdk-dev] [PATCH v3] config: add Mellanox BlueField ARMv8 SoC

[Thomas Monjalon]

14/01/2019 11:11, Yongseok Koh:
> > On Jan 13, 2019, at 10:13 PM, Honnappa Nagarahalli 
> >  wrote:
> > 
> > Similarly, the commit heading subject line also needs to change.
> > 
> >> 
> >>> 
> >>> BlueField is Mellanox's new SoC based on ARMv8. BlueField integrates
> >>
> >>   ^^ Above should be
> >> changed to armv8
> 
> Good to know that, thanks, Honnappa.
> 
> Thomas, will you take care of it when you merge (hope you will :-)?
> Shall I send out a new version?

Yes, that's fine, I will take care.

Re: [dpdk-dev] [PATCH 3/4] net/enic: support multicast filtering

[Ferruh Yigit]

On 1/12/2019 4:49 AM, Hyong Youb Kim wrote:
> On Fri, Jan 11, 2019 at 03:46:47PM +, Ferruh Yigit wrote:
>> On 12/10/2018 6:28 PM, Hyong Youb Kim wrote:
>>> The VIC hardware has 64 MAC filters per vNIC, which can be either
>>> unicast or multicast. Use one half for unicast and the other half for
>>> multicast, as the VIC kernel drivers for Linux and Windows do.
>>>
>>> Signed-off-by: Hyong Youb Kim 
>>> Reviewed-by: John Daley 
>>
>> <...>
>>
>>> +static void debug_log_add_del_addr(struct ether_addr *addr, bool add)
>>> +{
>>> +   char mac_str[ETHER_ADDR_FMT_SIZE];
>>> +   if (rte_log_get_level(enicpmd_logtype_init) == RTE_LOG_DEBUG) {
>>> +   ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
>>> +   PMD_INIT_LOG(ERR, " %s address %s\n",
>>> +add ? "add" : "remove", mac_str);
>>> +   }
>>> +}
>>
>> Why logging with 'ERR' level after checking if 'DEBUG' level is set?
> 
> Thanks for pointing this out. Our mistake. Should be DEBUG.
> 
>> And why need that check at all?
> 
> The outer check ("is log level debug?") is there to make this function
> to do nothing (including ether_format_addr) if log level > debug. I
> could not find a good way to combine the level test,
> ether_format_addr, and rte_log into one clean macro..
> 
> Since this function is on a slow path, I think I can just remove the
> level check and do something like the following. Would this satisfy
> your concern?
> 
> char mac_str[ETHER_ADDR_FMT_SIZE];
> ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
> PMD_INIT_LOG(DEBUG, " %s address %s\n", add ? "add" : "remove", mac_str);

Yes, I think this is better, thanks.

[dpdk-dev] [PATCH v5] eal: fix core number validation

[Hari Kumar Vemula]

When incorrect core value or range provided,
as part of -l command line option, a crash occurs.

Added valid range checks to fix the crash.

Added ut check for negative core values.
Added unit test case for invalid core number range.

Fixes: d888cb8b9613 ("eal: add core list input format")
Cc: sta...@dpdk.org

Signed-off-by: Hari Kumar Vemula 
--
v5: corrected unit test case for -l option
v4: Used RTE_MAX_LCORE for max core check
v3: Added unit test cases for invalid core number range
v2: Replace strtoul with strtol
Modified log message
---
 lib/librte_eal/common/eal_common_options.c |  9 +++--
 test/test/test_eal_flags.c | 15 ++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c 
b/lib/librte_eal/common/eal_common_options.c
index 6e3a83b98..14f40c62c 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -592,7 +592,9 @@ eal_parse_corelist(const char *corelist)
if (*corelist == '\0')
return -1;
errno = 0;
-   idx = strtoul(corelist, &end, 10);
+   idx = strtol(corelist, &end, 10);
+   if (idx < 0 || idx >= (int)cfg->lcore_count)
+   return -1;
if (errno || end == NULL)
return -1;
while (isblank(*end))
@@ -1103,6 +1105,7 @@ eal_parse_common_option(int opt, const char *optarg,
 {
static int b_used;
static int w_used;
+   struct rte_config *cfg = rte_eal_get_configuration();
 
switch (opt) {
/* blacklist */
@@ -1145,7 +1148,9 @@ eal_parse_common_option(int opt, const char *optarg,
/* corelist */
case 'l':
if (eal_parse_corelist(optarg) < 0) {
-   RTE_LOG(ERR, EAL, "invalid core list\n");
+   RTE_LOG(ERR, EAL,
+   "invalid core list, please check core numbers 
are in [0, %u] range\n",
+   cfg->lcore_count-1);
return -1;
}
 
diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
index 2acab9d69..4dc22ec36 100644
--- a/test/test/test_eal_flags.c
+++ b/test/test/test_eal_flags.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -513,6 +514,16 @@ test_missing_c_flag(void)
const char *argv25[] = { prgname, prefix, mp_flag,
 "-n", "3", "--lcores",
 "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
+   /* core number is negative value */
+   const char * const argv26[] = { prgname, prefix, mp_flag,
+   "-n", "3", "-l", "-5" };
+   const char * const argv27[] = { prgname, prefix, mp_flag,
+   "-n", "3", "-l", "-5-7" };
+   /* core number is maximum value */
+   const char * const argv28[] = { prgname, prefix, mp_flag,
+   "-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
+   const char * const argv29[] = { prgname, prefix, mp_flag,
+   "-n", "3", "-l", "1-"RTE_STR(RTE_MAX_LCORE) };
 
if (launch_proc(argv2) != 0) {
printf("Error - "
@@ -556,7 +567,9 @@ test_missing_c_flag(void)
launch_proc(argv18) == 0 || launch_proc(argv19) == 0 ||
launch_proc(argv20) == 0 || launch_proc(argv21) == 0 ||
launch_proc(argv21) == 0 || launch_proc(argv22) == 0 ||
-   launch_proc(argv23) == 0 || launch_proc(argv24) == 0) {
+   launch_proc(argv23) == 0 || launch_proc(argv24) == 0 ||
+   launch_proc(argv26) == 0 || launch_proc(argv27) == 0 ||
+   launch_proc(argv28) == 0 || launch_proc(argv29) == 0) {
printf("Error - "
   "process ran without error with invalid --lcore flag\n");
return -1;
-- 
2.17.2

Re: [dpdk-dev] [EXT] Re: [PATCH v3] build: use generic march on arm64 when using 'default' machine

[Jerin Jacob Kollanukkaran]

On Mon, 2019-01-14 at 09:58 +, Luca Boccassi wrote:
> On Mon, 2019-01-14 at 08:09 +, Jerin Jacob Kollanukkaran wrote:
> > On Fri, 2019-01-11 at 10:45 +, Luca Boccassi wrote:
> > > External Email
> > > 
> > > ---
> > > 
> > > ---
> > > On Mon, 2019-01-07 at 14:11 +, Luca Boccassi wrote:
> > > > When building for generic distribution we need a stable
> > > > baseline
> > > > architecture, or depending on the build worker the result will
> > > > vary.
> > > > 
> > > > Force the default flags if the user explicitly sets
> > > > machine=default
> > > > at configuration time.
> > > > 
> > > > Fixes: b1d48c41189a ("build: support ARM with meson")
> > > > Cc: sta...@dpdk.org
> > > > 
> > > > Signed-off-by: Luca Boccassi 
> > > > Acked-by: Bruce Richardson 
> > > > ---
> > > > v2: fix typo in commit message, remove variable used only once
> > > > v3: put back temporary variable, as "machine" gets overwritten
> > > > by the function and loses the original value before we need
> > > > it.
> > > > 
> > > >  config/arm/meson.build | 7 ++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > index dae55d6b2..614139534 100644
> > > > --- a/config/arm/meson.build
> > > > +++ b/config/arm/meson.build
> > > > @@ -6,6 +6,7 @@
> > > >  march_opt = '-march=@0@'.format(machine)
> > > >  
> > > >  arm_force_native_march = false
> > > > +arm_force_default_march = (machine == 'default')
> > > >  
> > > >  machine_args_generic = [
> > > > ['default', ['-march=armv8-a+crc+crypto']],
> > > > @@ -105,7 +106,11 @@ else
> > > > cmd_generic = ['generic', '', '', 'default', '']
> > > > cmd_output = cmd_generic # Set generic by default
> > > > machine_args = [] # Clear previous machine args
> > > > -   if not meson.is_cross_build()
> > > > +   if arm_force_default_march and not
> > > > meson.is_cross_build()
> > > > +   machine = impl_generic
> > > > +   cmd_output = cmd_generic
> > > > +   impl_pn = 'default'
> > > > +   elif not meson.is_cross_build()
> > > > # The script returns ['Implementer', 'Variant',
> > > > 'Architecture',
> > > > # 'Primary Part number', 'Revision']
> > > > detect_vendor = find_program(join_paths(
> > > 
> > > Any chance for a quick review from the arm maintainers/devs?
> > > Thanks!
> > 
> > Looks good.
> > 
> > Could you please document the procedure to build generic image some
> > where in the DPDK documentation.
> > 
> > I guess the procedure will be same for x86 and arm64. Right?
> 
> The "machine" meson option is already described in meson_options.txt,
> is that enough?

I meant, Documenting the steps to create the generic image i.e -
Dmachine=default scheme to use generic build for meson based
distribution build.

Is it same for x86 too?

meson configure -Dmachine=default
meson build
cd build
ninja
ninja install


> 

Re: [dpdk-dev] [PATCH] net/nfp: add CPP bridge as service

[Ferruh Yigit]

On 1/13/2019 9:41 PM, Thomas Monjalon wrote:
> 11/01/2019 17:42, Ferruh Yigit:
>> On 1/11/2019 1:25 PM, Alejandro Lucero wrote:
>>> The Netronome's Network Flow Processor chip is highly programmable
>>> with the goal of processing packets at high speed. Processing units
>>> and other chip components are available from the host through the
>>> PCIe CPP(Command Push Pull bus) interface. The NFP PF PMD configures
>>> a CPP handler for setting up and working with vNICs, perform actions
>>> like link up or down, or accessing extended stats from the MAC component.
>>>
>>> There exist NFP host tools which access the NFP components for
>>> programming and debugging but they require the CPP interface. When the
>>> PMD is bound to the PF, the DPDK app owns the CPP interface, so these
>>> host tools can not access the NFP through other means like NFP kernel
>>> drivers.
>>>
>>> This patch adds a CPP bridge using the rte_service API which can be
>>> enabled by a DPDK app. Interestingly, DPDK clients like OVS will not
>>> enable specific service cores, but this can be performed with a
>>> secondary process specifically enabling this CPP bridge service and
>>> therefore giving access to the NFP to those host tools.
>>>
>>> v2:
>>>  - Avoid printfs for debugging
>>>  - fix compilation problems for powerpc
>>>
>>> Signed-off-by: Alejandro Lucero 
>>
>> Applied to dpdk-next-net/master, thanks.
> 
> It does not compile with 32-bit toolchain.
> 
> Please check the occurences of %lu, thanks.

Hi Thomas,

We aware the build error, but let it because nfp doesn't support 32-bit.

But I just recognized that it is enabled by default on 32-bit default configs,
we should disable them.


Hi Alejandro,

Can you please disable nfp driver explicitly on
'defconfig_i686-native-linuxapp-*' config files, perhaps also on
'defconfig_x86_x32-native-linuxapp-gcc' too?

I will drop the existing patch from next-net.

And if it is possible to fix the build error, specially if it is just for %lu of
the logging, I prefer the fix against the config update, but it is up to you.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2] ip_pipeline: add QinQ PPPoE encapsulation action

[Dumitrescu, Cristian]

Hi Nemanja,

> -Original Message-
> From: Marjanovic, Nemanja
> Sent: Monday, January 14, 2019 10:23 AM
> To: dev@dpdk.org
> Cc: Singh, Jasvinder ; Dumitrescu, Cristian
> ; Marjanovic, Nemanja
> 
> Subject: [PATCH v2] ip_pipeline: add QinQ PPPoE encapsulation action
> 
> Add implementation of QinQ PPPoE packet encapsulation action.
> 
> Signed-off-by: Nemanja Marjanovic 
> ---
> v2:Fix changes from check patch
> ---
>  examples/ip_pipeline/cli.c | 43 +-
>  lib/librte_net/rte_ether.h |  1 +
>  lib/librte_pipeline/rte_table_action.c | 61 ++
>  lib/librte_pipeline/rte_table_action.h | 16 +++
>  4 files changed, 119 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/ip_pipeline/cli.c b/examples/ip_pipeline/cli.c
> index 910386282..dcc98ee65 100644
> --- a/examples/ip_pipeline/cli.c
> +++ b/examples/ip_pipeline/cli.c

We need to add support for this new packet encapsulation in the Soft NIC CLI, 
we don't want it to get out of sync with IP pipeline.
Basically, add essentially the same CLI code in 
drivers/net/softnic/rte_eth_softnic_cli.c, can you please do this?

> @@ -1290,11 +1290,13 @@ cmd_table_action_profile(char **tokens,
> 
>   p.encap.encap_mask = 1LLU <<
> RTE_TABLE_ACTION_ENCAP_VXLAN;
>   n_extra_tokens = 5;
> - } else {
> + } else if (strcmp(tokens[t0 + 1], "qinq_pppoe") == 0)
> + p.encap.encap_mask =
> + 1LLU <<
> RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE;
> + else {
>   snprintf(out, out_size, MSG_ARG_MISMATCH,
> "encap");
>   return;
>   }
> -

Please put back this blank line.

>   p.action_mask |= 1LLU << RTE_TABLE_ACTION_ENCAP;
>   t0 += 2 + n_extra_tokens;
>   } /* encap */
> @@ -3391,6 +3393,43 @@ parse_table_action_encap(char **tokens,
>   return 1 + 9;
>   }
> 
> + /* qinq_pppoe */
> + if (n_tokens && (strcmp(tokens[0], "qinq_pppoe") == 0)) {
> + uint32_t svlan_pcp, svlan_dei, svlan_vid;
> + uint32_t cvlan_pcp, cvlan_dei, cvlan_vid;
> +
> + if ((n_tokens < 10) ||
> + parse_mac_addr(tokens[1],
> + &a->encap.qinq_pppoe.ether.da) ||
> + parse_mac_addr(tokens[2],
> + &a->encap.qinq_pppoe.ether.sa) ||
> + parser_read_uint32(&svlan_pcp, tokens[3]) ||
> + (svlan_pcp > 0x7) ||
> + parser_read_uint32(&svlan_dei, tokens[4]) ||
> + (svlan_dei > 0x1) ||
> + parser_read_uint32(&svlan_vid, tokens[5]) ||
> + (svlan_vid > 0xFFF) ||
> + parser_read_uint32(&cvlan_pcp, tokens[6]) ||
> + (cvlan_pcp > 0x7) ||
> + parser_read_uint32(&cvlan_dei, tokens[7]) ||
> + (cvlan_dei > 0x1) ||
> + parser_read_uint32(&cvlan_vid, tokens[8]) ||
> + (cvlan_vid > 0xFFF) ||
> + parser_read_uint16(&a-
> >encap.qinq_pppoe.pppoe.session_id,
> + tokens[9]))
> + return 0;
> +
> + a->encap.qinq_pppoe.svlan.pcp = svlan_pcp & 0x7;
> + a->encap.qinq_pppoe.svlan.dei = svlan_dei & 0x1;
> + a->encap.qinq_pppoe.svlan.vid = svlan_vid & 0xFFF;
> + a->encap.qinq_pppoe.cvlan.pcp = cvlan_pcp & 0x7;
> + a->encap.qinq_pppoe.cvlan.dei = cvlan_dei & 0x1;
> + a->encap.qinq_pppoe.cvlan.vid = cvlan_vid & 0xFFF;
> + a->encap.type = RTE_TABLE_ACTION_ENCAP_QINQ_PPPOE;
> + a->action_mask |= 1 << RTE_TABLE_ACTION_ENCAP;
> + return 1 + 10;
> + }
> +

Please add this new encap in the cmd_table_action_profile_help string, as well 
in the comment describing the table action options (search for "table_action 
::= ") in this file.

>   /* mpls */
>   if (n_tokens && (strcmp(tokens[0], "mpls") == 0)) {
>   uint32_t label, tc, ttl;
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index c2c5e249f..212ead64a 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -300,6 +300,7 @@ struct vxlan_hdr {
>  #define ETHER_TYPE_ARP  0x0806 /**< Arp Protocol. */
>  #define ETHER_TYPE_RARP 0x8035 /**< Reverse Arp Protocol. */
>  #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
> +#define ETHER_TYPE_PPPoE 0x8864 /**< PPPoE Session Stage */

Please add both PPPoE session and PPPoE discovery ethertypes, and call them 
accordingly. Please keep the existing naming convention, i.e. keep names all 
upper case for the purpose of these macros: PPPOE_DISCOVERY and PPPOE_SESSION 
(as opposed to lover case "o" in PPPoE).

>  #define ETHE

Re: [dpdk-dev] [PATCH v3] config: add Mellanox BlueField ARMv8 SoC

[Thomas Monjalon]

10/01/2019 19:39, Yongseok Koh:
> BlueField is Mellanox's new SoC based on ARMv8. BlueField integrates
> Mellanox ConnectX-5 interconnect and Cortex-A72 cores into a single device.
> 
> http://www.mellanox.com/page/products_dyn?product_family=256&mtag=soc_overview
> 
> Signed-off-by: Yongseok Koh 

Applied with armv8 wording, thanks

Re: [dpdk-dev] [dpdk-stable] [PATCH] build: use static deps of libs for pkg-config libs.private

[Thomas Monjalon]

11/01/2019 17:12, Luca Boccassi:
> Dependencies of the RTE libraries were not being added to the
> Requires.private field of the pc file since the variable used for
> dynamic linking was passed to the related field of pkg.generate.
> Use the static one so that dependencies are included.
> 
> Fixes: 57ae0ec62620 ("build: add dependency on telemetry to apps with meson")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Luca Boccassi 
> ---
> split from the other series, it's a stand-alone non-invasive change

Applied, thanks

Re: [dpdk-dev] [dpdk-stable] [PATCH] build: fix incorrect variable name in error message

[Thomas Monjalon]

10/01/2019 14:10, Luca Boccassi:
> On Thu, 2019-01-10 at 10:29 +, Bruce Richardson wrote:
> > The variable name in the error message had an extra '_' which caused
> > an actual meson error when the message would otherwise be printed to
> > give meaningful information about what was going wrong.
> > 
> > Fixes: 203b61dc5e18 ("build: improve error message for missing
> > dependency")
> > CC: sta...@dpdk.org
> > 
> > Signed-off-by: Bruce Richardson 
> 
> Acked-by: Luca Boccassi 

Applied, thanks

Re: [dpdk-dev] [EXT] Re: [PATCH v3] build: use generic march on arm64 when using 'default' machine

[Luca Boccassi]

On Mon, 2019-01-14 at 10:35 +, Jerin Jacob Kollanukkaran wrote:
> On Mon, 2019-01-14 at 09:58 +, Luca Boccassi wrote:
> > On Mon, 2019-01-14 at 08:09 +, Jerin Jacob Kollanukkaran wrote:
> > > On Fri, 2019-01-11 at 10:45 +, Luca Boccassi wrote:
> > > > External Email
> > > > 
> > > > ---
> > > > 
> > > > ---
> > > > On Mon, 2019-01-07 at 14:11 +, Luca Boccassi wrote:
> > > > > When building for generic distribution we need a stable
> > > > > baseline
> > > > > architecture, or depending on the build worker the result
> > > > > will
> > > > > vary.
> > > > > 
> > > > > Force the default flags if the user explicitly sets
> > > > > machine=default
> > > > > at configuration time.
> > > > > 
> > > > > Fixes: b1d48c41189a ("build: support ARM with meson")
> > > > > Cc: sta...@dpdk.org
> > > > > 
> > > > > Signed-off-by: Luca Boccassi 
> > > > > Acked-by: Bruce Richardson 
> > > > > ---
> > > > > v2: fix typo in commit message, remove variable used only
> > > > > once
> > > > > v3: put back temporary variable, as "machine" gets
> > > > > overwritten
> > > > > by the function and loses the original value before we
> > > > > need
> > > > > it.
> > > > > 
> > > > >  config/arm/meson.build | 7 ++-
> > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > > index dae55d6b2..614139534 100644
> > > > > --- a/config/arm/meson.build
> > > > > +++ b/config/arm/meson.build
> > > > > @@ -6,6 +6,7 @@
> > > > >  march_opt = '-march=@0@'.format(machine)
> > > > >  
> > > > >  arm_force_native_march = false
> > > > > +arm_force_default_march = (machine == 'default')
> > > > >  
> > > > >  machine_args_generic = [
> > > > >   ['default', ['-march=armv8-a+crc+crypto']],
> > > > > @@ -105,7 +106,11 @@ else
> > > > >   cmd_generic = ['generic', '', '', 'default', '']
> > > > >   cmd_output = cmd_generic # Set generic by default
> > > > >   machine_args = [] # Clear previous machine args
> > > > > - if not meson.is_cross_build()
> > > > > + if arm_force_default_march and not
> > > > > meson.is_cross_build()
> > > > > + machine = impl_generic
> > > > > + cmd_output = cmd_generic
> > > > > + impl_pn = 'default'
> > > > > + elif not meson.is_cross_build()
> > > > >   # The script returns ['Implementer',
> > > > > 'Variant',
> > > > > 'Architecture',
> > > > >   # 'Primary Part number', 'Revision']
> > > > >   detect_vendor = find_program(join_paths(
> > > > 
> > > > Any chance for a quick review from the arm maintainers/devs?
> > > > Thanks!
> > > 
> > > Looks good.
> > > 
> > > Could you please document the procedure to build generic image
> > > some
> > > where in the DPDK documentation.
> > > 
> > > I guess the procedure will be same for x86 and arm64. Right?
> > 
> > The "machine" meson option is already described in
> > meson_options.txt,
> > is that enough?
> 
> I meant, Documenting the steps to create the generic image i.e -
> Dmachine=default scheme to use generic build for meson based
> distribution build.
> 
> Is it same for x86 too?
> 
> meson configure -Dmachine=default
> meson build
> cd build
> ninja
> ninja install

Yes that's correct, for any architecture "-Dmachine=default" will pick
a stable baseline.
I can send a separate patch to update the doc with that command.

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [dpdk-stable] [PATCH] test-meson-build: fix check for whether meson has run

[Thomas Monjalon]

10/01/2019 14:10, Luca Boccassi:
> On Thu, 2019-01-10 at 10:37 +, Bruce Richardson wrote:
> > The current check to see whether we need to call meson or just ninja
> > simply checked if the build directory existed. However, if meson was
> > run
> > but failed, the build directory would still exist. We can fix this by
> > instead checking for the build.ninja file inside the directory. Once
> > that
> > is present, we can use ninja safely and let it worry about rerunning
> > meson if necessary.
> > 
> > Fixes: a55277a788df ("devtools: add test script for meson builds")
> > CC: sta...@dpdk.org
> > 
> > Signed-off-by: Bruce Richardson 
> 
> Acked-by: Luca Boccassi 

Applied, thanks

[dpdk-dev] [PATCH] build: mention machine=default and its use in documentation

[Luca Boccassi]

Signed-off-by: Luca Boccassi 
---
 doc/build-sdk-meson.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/doc/build-sdk-meson.txt b/doc/build-sdk-meson.txt
index 29a8bd387..2efff58e2 100644
--- a/doc/build-sdk-meson.txt
+++ b/doc/build-sdk-meson.txt
@@ -87,6 +87,8 @@ Project-specific options are passed used -Doption=value::
 
meson -Denable_docs=true fullbuild  # build and install docs
 
+   meson -Dmachine=default  # user builder-independent baseline -march
+
 Examples of setting the same options using meson configure::
 
meson configure -Dwerror=true
@@ -103,6 +105,9 @@ should be used to change the build settings within the 
directory, and when
 ``ninja`` is called to do the build itself, it will trigger the necessary
 re-scan from meson.
 
+NOTE: machine=default will use the minimum required -march on all architectures
+regardless of the capabilities of the machine where the build is happening.
+
 As well as those settings taken from ``meson configure``, other options
 such as the compiler to use can be passed via environment variables. For
 example::
-- 
2.20.1

Re: [dpdk-dev] [PATCH v4 5/5] meson: add cross build targets for thunderx2 and octeontx2

[Thomas Monjalon]

09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> From: Pavan Nikhilesh 
> 
> Add Thunderx2 and Octeontx2 SoC cross build targets.
> meson build --cross-file config/arm/arm64___
> 
> Signed-off-by: Pavan Nikhilesh 
> ---
>  config/arm/arm64_octeontx2_linuxapp_gcc | 14 ++
>  config/arm/arm64_thunderx2_linuxapp_gcc | 14 ++
>  2 files changed, 28 insertions(+)

Don't you think it could be merged in patches 3 & 4
for more clarity?

Re: [dpdk-dev] [PATCH v4 2/5] meson: add infra to support machine specific flags

[Thomas Monjalon]

09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> From: Pavan Nikhilesh 
> 
> Currently, RTE_* flags are set based on the implementer ID but there might
> be some micro arch specific differences from the same vendor
> eg. CACHE_LINESIZE.
> Add support to set micro arch specific flags.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Pavan Nikhilesh 
> ---
>  flags_cavium = [
> - ['RTE_MACHINE', '"thunderx"'],
>   ['RTE_CACHE_LINE_SIZE', 128],
>   ['RTE_MAX_NUMA_NODES', 2],
>   ['RTE_MAX_LCORE', 96],
> - ['RTE_MAX_VFIO_GROUPS', 128],
> - ['RTE_USE_C11_MEM_MODEL', false]]
> + ['RTE_MAX_VFIO_GROUPS', 128]]
[...]
> +flags_thunderx_extra = [
> + ['RTE_MACHINE', '"thunderx"'],
> + ['RTE_USE_C11_MEM_MODEL', false]]
[..]
> +machine_args_cavium = [
> + ['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> + ['native', ['-march=native']],
> + ['0xa1', ['-mcpu=thunderxt88'], flags_thunderx_extra],
> + ['0xa2', ['-mcpu=thunderxt81'], flags_thunderx_extra],
> + ['0xa3', ['-mcpu=thunderxt83'], flags_thunderx_extra]]

This patch is not only adding infra, it also change some Cavium config.
What about splitting, and explaining the change of RTE_USE_C11_MEM_MODEL?

Re: [dpdk-dev] [PATCH v4 1/5] mk: introduce helper to check valid compiler argument

[Thomas Monjalon]

09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> From: Jerin Jacob 
> 
> Introduce rte_cc_has_argument() Makefile helper to
> check a given argument is support by the compiler.
> 
> Example Usage:
> 
> include $(RTE_SDK)/mk/rte.helper.mk
> MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)
> 
> This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
> if it is only supported by the compiler. The use case for such
> scheme is to enable the mcpu optimization if the compiler
> supports else it needs to compile the source code without
> any errors.
> 
> This patch also moves inclusion of toolchain's rte.vars.mk
> to before the machine's rte.vars.mk inclusion to make
> correct CC available for the cross compile case.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Pavan Nikhilesh 
> ---
> --- /dev/null
> +++ b/mk/rte.helper.mk
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Marvell International Ltd
> +
> +# rte_cc_has_argument
> +# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-avx512f)
> +# Return the argument if the argument is supported by the compiler.
> +#
> +define rte_cc_has_argument
> + $(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2> /dev/null && 
> rm -f tmp$$ && echo $(1) | xargs echo -n)
> +endef

What is tmp$$ ?

If the command is interrupted in the middle, temp file is not cleaned.
We could fix it with "trap".
Is it possible to just avoid creating a temporary file?

Re: [dpdk-dev] [PATCH] maintainers: resign from maintainership of distributor library

[Thomas Monjalon]

11/01/2019 12:48, Bruce Richardson:
> The distributor library doesn't see much in the way of changes, and Dave
> is well able to manage the library on his own, so remove my name against
> it in the MAINTAINERS file.
> 
> Signed-off-by: Bruce Richardson 

Applied

Re: [dpdk-dev] [PATCH v4] mk: fix scope of disabling AVX512F support

[Thomas Monjalon]

Hi,
Few details reviewed below,

07/01/2019 17:49, Ferruh Yigit:
> --- a/doc/guides/rel_notes/known_issues.rst
> +++ b/doc/guides/rel_notes/known_issues.rst
> +AVX-512 support disabled
> +
> +
> +**Description**:
> +   ``AVX-512`` support has been disabled on some conditions.
> +   This shouldn't be confused with ``CONFIG_RTE_ENABLE_AVX512`` config 
> option which is already
> +   disabled by default. This config option defines if ``AVX-512`` specific 
> implementations of
> +   some file to be used or not. What has been disabled is compiler feature 
> to produce ``AVX-512``
> +   instructions from any source code.
> +
> +   On DPDK v18.11 ``AVX-512`` disabled for all ``GCC`` builds which reported 
> to cause a performance
> +   drop.

*is* disabled

> +
> +   On DPDK v19.02 ``AVX-512`` disable scope reduced to ``GCC`` and 
> ``binutils version 2.30`` based

*is* reduced

> +   on information accured from the GCC community defect.
> +
> +**Reason**:
> +   Generated ``AVX-512`` code cause crash:
> +   https://bugs.dpdk.org/show_bug.cgi?id=97
> +   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096
> +
> +**Resolution/Workaround**:
> +   Update ``binutils`` to newer version than ``2.30``.
> +   Use different compiler, like ``clang`` for this case.

These are 2 possible workarounds. Should we say "Or" ?

> +
> +**Affected Environment/Platform**:
> +``GCC`` and ``binutils version 2.30``.
> --- a/doc/guides/rel_notes/release_19_02.rst
> +++ b/doc/guides/rel_notes/release_19_02.rst
> @@ -276,6 +276,19 @@ Known Issues
> Also, make sure to start the actual text at the margin.
> =
>  
> +* ``AVX-512`` support has been disabled for ``GCC`` builds when ``binutils 
> 2.30``
> +  is detected [1] because of a crash [2]. This can affect ``native`` machine 
> type
> +  build targets on the platforms that support ``AVX512F`` like ``Intel 
> Skylake``
> +  processors, and can cause a possible performance drop. The immediate 
> workaround
> +  is to use ``clang`` compiler on these platforms.
> +  Initial workaround in DPDK v18.11 was to disable ``AVX-512`` support for 
> ``GCC``
> +  completely, but based on information on defect submitted to GCC community 
> [3],
> +  issue has been identified as ``binutils 2.30`` issue. Since currently only 
> GCC
> +  generates ``AVX-512`` instructions, scope limited to ``GCC`` and 
> ``binutils 2.30``

*the* scope *is* limited

> +
> +  - [1]: Commit ("mk: fix scope of disabling AVX512F support")
> +  - [2]: https://bugs.dpdk.org/show_bug.cgi?id=97
> +  - [3]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096
>  

Space missing here

>  Tested Platforms
>  
> --- a/mk/toolchain/gcc/rte.toolchain-compat.mk
> +++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
> +LD_VERSION = $(shell $(LD) -v)
> +# disable AVX512F support for GCC & binutils 2.30 as a workaround for Bug 97
> +ifneq ($(filter 2.30%,$(LD_VERSION)),)
> +FORCE_DISABLE_AVX512 := y
> +# print warning only once for librte_eal
> +ifneq ($(filter %librte_eal,$(lastword $(CURDIR))),)

Do we need lastword for CURDIR?

Re: [dpdk-dev] [EXT] Re: [PATCH v4 1/5] mk: introduce helper to check valid compiler argument

[Jerin Jacob Kollanukkaran]

On Mon, 2019-01-14 at 12:35 +0100, Thomas Monjalon wrote:
> ---
> ---
> 09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> > From: Jerin Jacob 
> > 
> > Introduce rte_cc_has_argument() Makefile helper to
> > check a given argument is support by the compiler.
> > 
> > Example Usage:
> > 
> > include $(RTE_SDK)/mk/rte.helper.mk
> > MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)
> > 
> > This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
> > if it is only supported by the compiler. The use case for such
> > scheme is to enable the mcpu optimization if the compiler
> > supports else it needs to compile the source code without
> > any errors.
> > 
> > This patch also moves inclusion of toolchain's rte.vars.mk
> > to before the machine's rte.vars.mk inclusion to make
> > correct CC available for the cross compile case.
> > 
> > Signed-off-by: Jerin Jacob 
> > Signed-off-by: Pavan Nikhilesh 
> > ---
> > --- /dev/null
> > +++ b/mk/rte.helper.mk
> > @@ -0,0 +1,12 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2018 Marvell International Ltd
> > +
> > +# rte_cc_has_argument
> > +# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-
> > avx512f)
> > +# Return the argument if the argument is supported by the
> > compiler.
> > +#
> > +define rte_cc_has_argument
> > +   $(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2>
> > /dev/null && rm -f tmp$$ && echo $(1) | xargs echo -n)
> > +endef
> 
> What is tmp$$ ?

It is created per process with pid value.

> 
> If the command is interrupted in the middle, temp file is not
> cleaned.

Yes. I think we can move to RTE_OUTPUT. Even it is not cleaned 
then it is file, I think, that would be easiest solution.

> We could fix it with "trap".

Can we do it in Makefile? 

> Is it possible to just avoid creating a temporary file?

I tried it but gcc creates one.

> 
> 
> 

Re: [dpdk-dev] [PATCH] maintainer: claim maintainership of Hashes

[Thomas Monjalon]

09/01/2019 10:14, Bruce Richardson:
> On Tue, Jan 08, 2019 at 07:08:34AM -0800, Yipeng Wang wrote:
> > Add Yipeng and Sameh as additional maintainers.
> > 
> > Signed-off-by: Yipeng Wang 
> > Signed-off-by: Sameh Gobriel 
> > ---
> >  MAINTAINERS | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> 
> Acked-by: Bruce Richardson 
> 
> Thanks for stepping up.

Applied, thanks

[dpdk-dev] [PATCH] ipsec: fix invalid assert condition

[Konstantin Ananyev]

fix invalid RTE_ASSERT condition in rsn_update_finish()

Fixes: c0308cd89554 ("ipsec: rework SA replay window/SQN for MT environment")

Reported-by: Ferruh Yigit 
Signed-off-by: Konstantin Ananyev 
---
 lib/librte_ipsec/ipsec_sqn.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ipsec/ipsec_sqn.h b/lib/librte_ipsec/ipsec_sqn.h
index 7de10bef5..a3ae7e2de 100644
--- a/lib/librte_ipsec/ipsec_sqn.h
+++ b/lib/librte_ipsec/ipsec_sqn.h
@@ -333,7 +333,7 @@ rsn_update_finish(struct rte_ipsec_sa *sa, struct 
replay_sqn *rsn)
 
n = sa->sqn.inb.wridx;
RTE_ASSERT(n != sa->sqn.inb.rdidx);
-   RTE_ASSERT(rsn - sa->sqn.inb.rsn == n);
+   RTE_ASSERT(rsn == sa->sqn.inb.rsn[n]);
 
rte_rwlock_write_unlock(&rsn->rwl);
sa->sqn.inb.rdidx = n;
-- 
2.17.1

Re: [dpdk-dev] [EXT] Re: [PATCH v4 1/5] mk: introduce helper to check valid compiler argument

[Thomas Monjalon]

14/01/2019 12:56, Jerin Jacob Kollanukkaran:
> On Mon, 2019-01-14 at 12:35 +0100, Thomas Monjalon wrote:
> > ---
> > ---
> > 09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> > > From: Jerin Jacob 
> > > 
> > > Introduce rte_cc_has_argument() Makefile helper to
> > > check a given argument is support by the compiler.
> > > 
> > > Example Usage:
> > > 
> > > include $(RTE_SDK)/mk/rte.helper.mk
> > > MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)
> > > 
> > > This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
> > > if it is only supported by the compiler. The use case for such
> > > scheme is to enable the mcpu optimization if the compiler
> > > supports else it needs to compile the source code without
> > > any errors.
> > > 
> > > This patch also moves inclusion of toolchain's rte.vars.mk
> > > to before the machine's rte.vars.mk inclusion to make
> > > correct CC available for the cross compile case.
> > > 
> > > Signed-off-by: Jerin Jacob 
> > > Signed-off-by: Pavan Nikhilesh 
> > > ---
> > > --- /dev/null
> > > +++ b/mk/rte.helper.mk
> > > @@ -0,0 +1,12 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause
> > > +# Copyright(c) 2018 Marvell International Ltd
> > > +
> > > +# rte_cc_has_argument
> > > +# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-
> > > avx512f)
> > > +# Return the argument if the argument is supported by the
> > > compiler.
> > > +#
> > > +define rte_cc_has_argument
> > > + $(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2>
> > > /dev/null && rm -f tmp$$ && echo $(1) | xargs echo -n)
> > > +endef
> > 
> > What is tmp$$ ?
> 
> It is created per process with pid value.

I see. The file is in current directory with name tmp + PID.

> > If the command is interrupted in the middle, temp file is not
> > cleaned.
> 
> Yes. I think we can move to RTE_OUTPUT. Even it is not cleaned 
> then it is file, I think, that would be easiest solution.
> 
> > We could fix it with "trap".
> 
> Can we do it in Makefile? 

Yes, it is just one more command separated with ;

> > Is it possible to just avoid creating a temporary file?
> 
> I tried it but gcc creates one.

It does not create a file in my test.

A better command is:
$(CC) -E $(1) -xc /dev/null >/dev/null
Then you can just check the return value.

If you want rte_cc_has_argument returns a string or empty as true/false,
you can do this:
$(CC) -E $(1) -xc /dev/null >/dev/null && echo $(1)

Re: [dpdk-dev] [PATCH] net/ice: fix vlan filter Tx issue

[Zhang, Qi Z]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Qiming Yang
> Sent: Monday, January 14, 2019 9:36 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming 
> Subject: [dpdk-dev] [PATCH] net/ice: fix vlan filter Tx issue
> 
> This patch fixed Tx error and allowed untagged packets in when vlan filter on.
> 
> Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops")
> 
> Signed-off-by: Qiming Yang 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [dpdk-dev] [PATCH] lib: add warning for NULL pointer

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Qiming Yang
> Sent: Wednesday, December 5, 2018 11:56 AM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] lib: add warning for NULL pointer
> 
> May return NULL when manage tries for packet in acl library. So
> this patch added warning for the NULL pointer return.
> 
> Fixes: 074f54ad03ee ("acl: fix build and runtime for default target")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Qiming Yang 
> ---
>  lib/librte_acl/acl_run.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_acl/acl_run.h b/lib/librte_acl/acl_run.h
> index bf7842d..6c718c0 100644
> --- a/lib/librte_acl/acl_run.h
> +++ b/lib/librte_acl/acl_run.h
> @@ -143,6 +143,9 @@ acl_start_next_trie(struct acl_flow_data *flows, struct 
> parms *parms, int n,
>   flows->num_packets * flows->categories);
>   }
> 
> + if (flows->last_cmplt == NULL)
> + RTE_LOG(WARNING, MALLOC, "packet tries allocate 
> failed");
> +

alloc_completion() should never retrun NULL
(unless something is totally broken).
So no need to handle such situation.
If you desire - feel free to add
RTE_ASSERT(flows->last_cmplt != NULL)
here.
Konstantin

>   /* set completion parameters and starting index for this slot */
>   parms[n].cmplt = flows->last_cmplt;
>   transition =
> --
> 2.9.5

Re: [dpdk-dev] [PATCH] net/i40e: fix FPK VF overwrite PF RSS LUT

[Zhang, Qi Z]

> -Original Message-
> From: Wu, Yanglong
> Sent: Monday, January 14, 2019 4:24 PM
> To: dev@dpdk.org
> Cc: Hall, Ryan E ; Zhang, Qi Z ;
> Wu, Yanglong ; sta...@dpdk.org
> Subject: [PATCH] net/i40e: fix FPK VF overwrite PF RSS LUT

I think this is not a just for FPK, but for all i40e family right?
> 
> i40e_aq_set_rss_lut should set rigister according to the vsi->type. if not, VF
> may overwrite the rigister of PF
> 
> Fixes: d0a349409bd7 ("i40e: support AQ based RSS config")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Wu Yanglong 

It should be "Yanglong Wu"

> ---
>  drivers/net/i40e/i40e_ethdev.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index a6b97e164..d4d99edcd 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -4183,8 +4183,13 @@ i40e_get_rss_lut(struct i40e_vsi *vsi, uint8_t *lut,
> uint16_t lut_size)
>   return -EINVAL;
> 
>   if (pf->flags & I40E_FLAG_RSS_AQ_CAPABLE) {
> - ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, TRUE,
> -   lut, lut_size);
> + if (vsi->type == I40E_VSI_SRIOV) {
> + ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, FALSE,
> +   lut, lut_size);
> + } else {
> + ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, TRUE,
> +   lut, lut_size);
> + }

You can simplify above code to below single line

ret = i40e_aq_get_rss_lut(hw, vsi->vsi_id, vsi->type != I40E_VSI_SRIOV, lut, 
lut_size);

>   if (ret) {
>   PMD_DRV_LOG(ERR, "Failed to get RSS lookup table");
>   return ret;
> --
> 2.11.0

Re: [dpdk-dev] [PATCH v1] net/i40e: perform basic validation on the VF messages

[Zhang, Qi Z]

> -Original Message-
> From: Wang, Haiyue
> Sent: Thursday, January 10, 2019 8:08 PM
> To: dev@dpdk.org; Zhang, Qi Z 
> Cc: Wang, Haiyue 
> Subject: [PATCH v1] net/i40e: perform basic validation on the VF messages
> 
> Do the VF message basic validation such as OPCODE message length check,
> some special OPCODE message format check, to protect the i40e PMD from
> malicious VF message attack.
> 
> Fixes: 4861cde46116 ("i40e: new poll mode driver")

> 
> Signed-off-by: Haiyue Wang 

Acked-by: Qi Zhang 

Cc: sta...@dpdk.org added when

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [dpdk-dev] [PATCH v2] examples/ip_fragmentation: support big packets

[Ananyev, Konstantin]

> 
> In some vendors the RX and TX configuration must be the same, therefore
> the MTU size need to be equal to max_rx_pkt_len.
> The MTU is the largest size packet in bytes that can be sent on the
> network, therefore before changing this parameter, the NIC could not
> receive packets larger than 1500 bytes, which is the default MTU size.
> In addition, scatter-gather need to be enabled in order to receive
> frames bigger than mbuf size.
> 
> Signed-off-by: Noa Ezra 
> ---
> v2: move set_mtu() to be done after dev_configure().
> 
>  examples/ip_fragmentation/main.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/examples/ip_fragmentation/main.c 
> b/examples/ip_fragmentation/main.c
> index 17a877d..8d789b4 100644
> --- a/examples/ip_fragmentation/main.c
> +++ b/examples/ip_fragmentation/main.c
> @@ -141,6 +141,7 @@ struct lcore_queue_conf {
>   .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
>   .split_hdr_size = 0,
>   .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> +  DEV_RX_OFFLOAD_SCATTER |
>DEV_RX_OFFLOAD_JUMBO_FRAME),
>   },
>   .txmode = {
> @@ -935,6 +936,16 @@ struct rte_lpm6_config lpm6_config = {
>   ret, portid);
>   }
> 
> + /* set the mtu to the maximum received packet size */
> + ret = rte_eth_dev_set_mtu(portid,
> + local_port_conf.rxmode.max_rx_pkt_len);
> + if (ret < 0) {
> + printf("\n");
> + rte_exit(EXIT_FAILURE, "Set MTU failed: "
> + "err=%d, port=%d\n",
> + ret, portid);
> + }
> +
>   ret = rte_eth_dev_adjust_nb_rx_tx_desc(portid, &nb_rxd,
>   &nb_txd);
>   if (ret < 0) {
> --

Acked-by: Konstantin Ananyev 

> 1.8.3.1

Re: [dpdk-dev] [PATCH v2] kni:fix build failure as "ndo_change_mtu_rh74" not found in RHEL8

[Thomas Monjalon]

19/12/2018 14:47, Ferruh Yigit:
> On 12/19/2018 7:07 AM, Xiao Liang wrote:
> > 'ndo_change_mtu_rh74' was changed to 'ndo_change_mtu' in RHEL8.
> > 
> > Build error log:
> > /home/dpdk-18.11/kernel/linux/kni/compat.h:107:24: error: ‘const struct
> > net_device_ops’ has no member named ‘ndo_change_mtu_rh74’; did you mean
> > ‘ndo_change_mtu’?
> >  #define ndo_change_mtu ndo_change_mtu_rh74
> > 
> > ^~~
> > Signed-off-by: Xiao Liang 
> 
> Suggested title: 'kni: fix build on RHEL 8'
> 
> And a request to backport:
> Cc: sta...@dpdk.org
> 
> Acked-by: Ferruh Yigit 

Applied, thanks

Re: [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat

[Ferruh Yigit]

On 1/14/2019 6:04 AM, Chaitanya Babu Talluri wrote:
> Strcat does not check the destination length and there might be
> chances of string overflow so insted of strcat, strncat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri 

<...>

> @@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user 
> *desc)
>* driver is used because that implies root user.
>*/
>   home_path = getenv("HOME");
> - lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
> + lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
> sizeof(char));
>  
> - strcat(lockfile, home_path);
> - strcat(lockfile, "/.lock_nfp_secondary");
> + strncat(lockfile, home_path, LOCKFILE_HOME_PATH);
> + strncat(lockfile, lockname, strlen(lockfile));

I guess this need to be 'LOCKFILE_HOME_PATH - strlen(lockfile) - 1' instead.
But also this can be implemented as 'snprintf()'

Since 'lockfile' allocated dynamically based on sizes of existing strings, using
'lockname' instead of "/.lock_nfp_secondary" will show that there won't be any
overflow but tools still may be complaining about 'strcat' usage.

Re: [dpdk-dev] [PATCH] build: mention machine=default and its use in documentation

[Christian Ehrhardt]

On Mon, Jan 14, 2019 at 1:23 PM Luca Boccassi  wrote:
>
> Signed-off-by: Luca Boccassi 
> ---
>  doc/build-sdk-meson.txt | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/build-sdk-meson.txt b/doc/build-sdk-meson.txt
> index 29a8bd387..2efff58e2 100644
> --- a/doc/build-sdk-meson.txt
> +++ b/doc/build-sdk-meson.txt
> @@ -87,6 +87,8 @@ Project-specific options are passed used -Doption=value::
>
> meson -Denable_docs=true fullbuild  # build and install docs
>
> +   meson -Dmachine=default  # user builder-independent baseline -march

Thanks for driving all the proper builder independent config Luca.
In this line I'd assume you meant "use" not "user" right?

Other than that:
Reviewed-by: Christian Ehrhardt 

> +
>  Examples of setting the same options using meson configure::
>
> meson configure -Dwerror=true
> @@ -103,6 +105,9 @@ should be used to change the build settings within the 
> directory, and when
>  ``ninja`` is called to do the build itself, it will trigger the necessary
>  re-scan from meson.
>
> +NOTE: machine=default will use the minimum required -march on all 
> architectures
> +regardless of the capabilities of the machine where the build is happening.
> +
>  As well as those settings taken from ``meson configure``, other options
>  such as the compiler to use can be passed via environment variables. For
>  example::
> --
> 2.20.1
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

Re: [dpdk-dev] [PATCH] kni: fix build on RHEL8 and RHEL7.6-ALT

[Thomas Monjalon]

21/12/2018 18:02, Ferruh Yigit:
> On 12/21/2018 4:27 PM, David Zeng wrote:
> > Signed-off-by: David Zeng 
> 
> Acked-by: Ferruh Yigit 

Applied on top of another fix:
http://git.dpdk.org/dpdk/commit/?id=4c820b22
and renamed:
kni: fix build on RHEL8 for arm and Power9

Thanks

Re: [dpdk-dev] [PATCH] ipsec: fix invalid assert condition

[Ferruh Yigit]

On 1/14/2019 12:07 PM, Konstantin Ananyev wrote:
> fix invalid RTE_ASSERT condition in rsn_update_finish()
> 
> Fixes: c0308cd89554 ("ipsec: rework SA replay window/SQN for MT environment")
> 
> Reported-by: Ferruh Yigit 
> Signed-off-by: Konstantin Ananyev 

Reviewed-by: Ferruh Yigit 

Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix wrong pointer reference in macswap

[Iremonger, Bernard]

> -Original Message-
> From: Yongseok Koh [mailto:ys...@mellanox.com]
> Sent: Monday, January 14, 2019 10:00 AM
> To: Lu, Wenzhuo ; Wu, Jingjing
> ; Iremonger, Bernard 
> Cc: dev@dpdk.org; Zhang, Qi Z ; sta...@dpdk.org
> Subject: [PATCH 1/2] app/testpmd: fix wrong pointer reference in macswap
> 
> The pointer is misused and could set wrong MAC address. As a result, some of
> packets can be dropped in receiver side due to MAC address mismatch.
> This can be shown as performance degradation.
> 
> Bugzilla ID: 188
> 
> Fixes: 62b52877adbe ("app/testpmd: batch MAC swap for performance on x86")
> Cc: qi.z.zh...@intel.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yongseok Koh 
Acked-by: Bernard Iremonger 

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/sfc: fix typo in preprocessor check for macro defined

[Ferruh Yigit]

On 1/13/2019 2:50 PM, Andrew Rybchenko wrote:
> ISP2() macro function misspelled in a check just before it.
> 
> Fixes: e1b944598579 ("net/sfc: build libefx")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Andrew Rybchenko 

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

Re: [dpdk-dev] [PATCH 2/2] app/testpmd: fix missing prefetch in macswap mode

[Iremonger, Bernard]

> -Original Message-
> From: Yongseok Koh [mailto:ys...@mellanox.com]
> Sent: Monday, January 14, 2019 10:00 AM
> To: Lu, Wenzhuo ; Wu, Jingjing
> ; Iremonger, Bernard 
> Cc: dev@dpdk.org; Zhang, Qi Z ; sta...@dpdk.org
> Subject: [PATCH 2/2] app/testpmd: fix missing prefetch in macswap mode
> 
> Prefetching packet was missing when do_macswap() was optimized.
> 
> Fixes: 62b52877adbe ("app/testpmd: batch MAC swap for performance on x86")
> Cc: qi.z.zh...@intel.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yongseok Koh 
Acked-by: Bernard Iremonger 

[dpdk-dev] [PATCH v2] build: mention machine=default and its use in documentation

[Luca Boccassi]

Document the new value, as it's useful for distributions and users
who need to use a stable baseline -march

Signed-off-by: Luca Boccassi 
Reviewed-by: Christian Ehrhardt 
---
v2: fix typo, add commit body and reviewed-by

 doc/build-sdk-meson.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/doc/build-sdk-meson.txt b/doc/build-sdk-meson.txt
index 29a8bd387..ab33ca82a 100644
--- a/doc/build-sdk-meson.txt
+++ b/doc/build-sdk-meson.txt
@@ -87,6 +87,8 @@ Project-specific options are passed used -Doption=value::
 
meson -Denable_docs=true fullbuild  # build and install docs
 
+   meson -Dmachine=default  # use builder-independent baseline -march
+
 Examples of setting the same options using meson configure::
 
meson configure -Dwerror=true
@@ -103,6 +105,9 @@ should be used to change the build settings within the 
directory, and when
 ``ninja`` is called to do the build itself, it will trigger the necessary
 re-scan from meson.
 
+NOTE: machine=default will use the minimum required -march on all architectures
+regardless of the capabilities of the machine where the build is happening.
+
 As well as those settings taken from ``meson configure``, other options
 such as the compiler to use can be passed via environment variables. For
 example::
-- 
2.20.1

Re: [dpdk-dev] [PATCH] build: mention machine=default and its use in documentation

[Luca Boccassi]

On Mon, 2019-01-14 at 15:32 +0200, Christian Ehrhardt wrote:
> On Mon, Jan 14, 2019 at 1:23 PM Luca Boccassi 
> wrote:
> > 
> > Signed-off-by: Luca Boccassi 
> > ---
> >  doc/build-sdk-meson.txt | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/doc/build-sdk-meson.txt b/doc/build-sdk-meson.txt
> > index 29a8bd387..2efff58e2 100644
> > --- a/doc/build-sdk-meson.txt
> > +++ b/doc/build-sdk-meson.txt
> > @@ -87,6 +87,8 @@ Project-specific options are passed used
> > -Doption=value::
> > 
> > meson -Denable_docs=true fullbuild  # build and install
> > docs
> > 
> > +   meson -Dmachine=default  # user builder-independent
> > baseline -march
> 
> Thanks for driving all the proper builder independent config Luca.
> In this line I'd assume you meant "use" not "user" right?
> 
> Other than that:
> Reviewed-by: Christian Ehrhardt 

Yes indeed it's a typo, fixed in v2, thanks

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH] ipsec: fix invalid assert condition

[Thomas Monjalon]

14/01/2019 14:37, Ferruh Yigit:
> On 1/14/2019 12:07 PM, Konstantin Ananyev wrote:
> > fix invalid RTE_ASSERT condition in rsn_update_finish()
> > 
> > Fixes: c0308cd89554 ("ipsec: rework SA replay window/SQN for MT 
> > environment")
> > 
> > Reported-by: Ferruh Yigit 
> > Signed-off-by: Konstantin Ananyev 
> 
> Reviewed-by: Ferruh Yigit 

Applied, thanks

Re: [dpdk-dev] [PATCH v8] eal_interrupts: add option for pending callback unregister

[Thomas Monjalon]

Someone for a review please?

17/12/2018 13:30, Jakub Grajciar:
> use case: if callback is used to receive message form socket,
> and the message received is disconnect/error, this callback needs
> to be unregistered, but cannot because it is still active.
> 
> With this patch it is possible to mark the callback to be
> unregistered once the interrupt process is done with this
> interrupt source.
> 
> Signed-off-by: Jakub Grajciar 
> ---
>  .../common/include/rte_interrupts.h   | 32 +++
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c  | 85 ++-
>  lib/librte_eal/rte_eal_version.map|  1 +
>  3 files changed, 116 insertions(+), 2 deletions(-)
> 
> v2:
> - fix coding syle
> 
> v3:
> - fix locks
> 
> v4:
> - update version map
> 
> v6:
> - add _rte_experimental to rte_intr_callback_unregister_pending
> 
> v8:
> - fix compilation errors
> 
> diff --git a/lib/librte_eal/common/include/rte_interrupts.h 
> b/lib/librte_eal/common/include/rte_interrupts.h
> index d751a6378..13d73a137 100644
> --- a/lib/librte_eal/common/include/rte_interrupts.h
> +++ b/lib/librte_eal/common/include/rte_interrupts.h
> @@ -6,6 +6,7 @@
>  #define _RTE_INTERRUPTS_H_
>  
>  #include 
> +#include 
>  
>  /**
>   * @file
> @@ -24,6 +25,13 @@ struct rte_intr_handle;
>  /** Function to be registered for the specific interrupt */
>  typedef void (*rte_intr_callback_fn)(void *cb_arg);
>  
> +/**
> + * Function to call after a callback is unregistered.
> + * Can be used to close fd and free cb_arg.
> + */
> +typedef void (*rte_intr_unregister_callback_fn)(struct rte_intr_handle 
> *intr_handle,
> + void *cb_arg);
> +
>  #include "rte_eal_interrupts.h"
>  
>  /**
> @@ -61,6 +69,30 @@ int rte_intr_callback_register(const struct 
> rte_intr_handle *intr_handle,
>  int rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
>   rte_intr_callback_fn cb, void *cb_arg);
>  
> +/**
> + * It unregisters the callback according to the specified interrupt handle,
> + * after it's no longer acive. Failes if source is not active.
> + *
> + * @param intr_handle
> + *  pointer to the interrupt handle.
> + * @param cb
> + *  callback address.
> + * @param cb_arg
> + *  address of parameter for callback, (void *)-1 means to remove all
> + *  registered which has the same callback address.
> + * @param ucb_fn
> + *  callback to call before cb is unregistered (optional).
> + *  can be used to close fd and free cb_arg.
> + *
> + * @return
> + *  - On success, return the number of callback entities marked for remove.
> + *  - On failure, a negative value.
> + */
> +int __rte_experimental
> +rte_intr_callback_unregister_pending(const struct rte_intr_handle 
> *intr_handle,
> + rte_intr_callback_fn cb_fn, void *cb_arg,
> + rte_intr_unregister_callback_fn ucb_fn);
> +
>  /**
>   * It enables the interrupt for the specified handle.
>   *
> diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c 
> b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> index cbac451e1..be0dd4490 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> @@ -76,6 +76,8 @@ struct rte_intr_callback {
>   TAILQ_ENTRY(rte_intr_callback) next;
>   rte_intr_callback_fn cb_fn;  /**< callback address */
>   void *cb_arg;/**< parameter for callback */
> + uint8_t pending_delete;  /**< delete after callback is called */
> + rte_intr_unregister_callback_fn ucb_fn; /**< fn to call before cb is 
> deleted */
>  };
>  
>  struct rte_intr_source {
> @@ -472,6 +474,8 @@ rte_intr_callback_register(const struct rte_intr_handle 
> *intr_handle,
>   }
>   callback->cb_fn = cb;
>   callback->cb_arg = cb_arg;
> + callback->pending_delete = 0;
> + callback->ucb_fn = NULL;
>  
>   rte_spinlock_lock(&intr_lock);
>  
> @@ -518,6 +522,57 @@ rte_intr_callback_register(const struct rte_intr_handle 
> *intr_handle,
>   return ret;
>  }
>  
> +int __rte_experimental
> +rte_intr_callback_unregister_pending(const struct rte_intr_handle 
> *intr_handle,
> + rte_intr_callback_fn cb_fn, void *cb_arg,
> + rte_intr_unregister_callback_fn ucb_fn)
> +{
> + int ret;
> + struct rte_intr_source *src;
> + struct rte_intr_callback *cb, *next;
> +
> + /* do parameter checking first */
> + if (intr_handle == NULL || intr_handle->fd < 0) {
> + RTE_LOG(ERR, EAL,
> + "Unregistering with invalid input parameter\n");
> + return -EINVAL;
> + }
> +
> + rte_spinlock_lock(&intr_lock);
> +
> + /* check if the insterrupt source for the fd is existent */
> + TAILQ_FOREACH(src, &intr_sources, next)
> + if (src->intr_handle.fd == intr_handle->fd)
> + break;
> +
> + /* No interrupt source r

Re: [dpdk-dev] [PATCH] eal: turn off getopt_long error messages

[Thomas Monjalon]

Keith, it seems you missed my questions below:

19/12/2018 21:35, Thomas Monjalon:
> 16/12/2018 17:45, Keith Wiles:
> > When using dpdk register option api when parsing for log level
> > the opterr flags was still set to one causing an error message
> > from getopt_long(). Set opterr to zero to disable error messages.
> 
> Please could you be more specific?
> Which function call? Which error message?
> 
> We need also Fixes: and stable tags for backport.
> Thanks

Re: [dpdk-dev] [PATCH v2 7/7] net/tap: don't print pointer in info message

[Ferruh Yigit]

On 1/11/2019 8:35 PM, Stephen Hemminger wrote:
> Printing pointer in log is uninformative (unless in a debugger),
> instead print the assigned kernel device name which correlates
> well with what TAP is doing.
> 
> Signed-off-by: Stephen Hemminger 

Reviewed-by: Ferruh Yigit 

Re: [dpdk-dev] [PATCH v2 0/6] net/tap: fixes and cleanups

[Ferruh Yigit]

On 1/11/2019 8:35 PM, Stephen Hemminger wrote:
> The tap device (used by vdev_netvsc on Azure) has a bug that
> prevents it working with primary/secondary process model because
> the device name generation assumed a single process.  The fix for
> this is to have the kernel assign the device name (patch #5).
> 
> While investigating this, found a number of other small issues
> that should be cleaned up as well.
> 
> v2
>   - encorporate Keith's style feedback and ack
>   - use strlcpy instead of snprintf when copying "dtap%d"
> since it makes intent clearer
>   - add another message cleanup
> 
> Stephen Hemminger (7):
>   net/tap: use strlcpy for interface name
>   net/tap: allow full length names
>   net/tap: check interface name in kvargs
>   net/tap: lower the priority of log messages
>   net/tap: let kernel choose tun device name
>   net/tap: get rid of global tuntap_name
>   net/tap: don't print pointer in info message

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] eal: fix strdup usages in internal config

[Thomas Monjalon]

10/01/2019 14:38, Anatoly Burakov:
> Currently, we use strdup in a few places to store command-line
> parameter values for certain internal config values. There are
> several issues with that.
> 
> First of all, they're never freed, so memory ends up leaking
> either after EAL exit, or when these command-line options are
> supplied multiple times.
> 
> Second of all, they're defined as `const char *`, so they
> *cannot* be freed even if we wanted to.
> 
> Finally, strdup may return NULL, which will be stored in the
> config. For most fields, NULL is a valid value, but for the
> default prefix, the value is always expected to be valid.
> 
> To fix all of this, three things are done. First, we change
> the definitions of these values to `char *` as opposed to
> `const char *`. This does not break the ABI, and previous
> code assumes constness (which is more restrictive), so it's
> safe to do so.
> 
> Then, fix all usages of strdup to check return value, and add
> a cleanup function that will free the memory occupied by
> these strings, as well as freeing them before assigning a new
> value to prevent leaks when parameter is specified multiple
> times.
> 
> And finally, add an internal API to query hugefile prefix, so
> that, absent of a valid value, a default value will be
> returned, and also fix up all usages of hugefile prefix to
> use this API instead of accessing hugefile prefix directly.
> 
> Bugzilla ID: 108
> 
> Signed-off-by: Anatoly Burakov 

Applied, thanks

Re: [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat

[Bruce Richardson]

On Mon, Jan 14, 2019 at 06:04:35AM +, Chaitanya Babu Talluri wrote:
> Strcat does not check the destination length and there might be
> chances of string overflow so insted of strcat, strncat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri 
> ---
>  drivers/net/bnx2x/bnx2x.c  | 6 --
>  drivers/net/i40e/i40e_ethdev.c | 6 --
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 8 +---
>  test/test/test_cryptodev.c | 3 ++-
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 4c775c163..0e1e6447a 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>   for (i = 0; i < 5; i++)
>   if (flags & (1 << i)) {
> - strcat(flag_str, flag[i]);
> + strncat(flag_str, flag[i],
> + BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
>   flags ^= (1 << i);
>   }
>   if (flags) {
>   static char unknown[BNX2X_INFO_STR_MAX];
>   snprintf(unknown, 32, "Unknown flag mask %x", flags);
> - strcat(flag_str, unknown);
> + strncat(flag_str, unknown,
> + BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
>   }
>   return flag_str;
>  }

While I believe this is correct, this (and the changes below) would be a
lot neater using strlcat function. We should perhaps look to add strlcat
alongside strlcpy for DPDK use.

/Bruce

Re: [dpdk-dev] [PATCH] eal/bsdapp: don't clean up files at startup

[Thomas Monjalon]

10/01/2019 12:34, Anatoly Burakov:
> On FreeBSD, closing the file descriptor drops the lock even if the
> file descriptor was mmap'ed. This leads to the cleanup at the end
> of EAL init to remove fbarray files that are still in use by the
> process itself.
> 
> However, instead of working around this issue, we can take advantage
> of the fact that FreeBSD doesn't really create any per-process
> files in the first place, so no cleanup is actually needed.
> 
> Fixes: 0a529578f162 ("eal: clean up unused files on initialization")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Anatoly Burakov 

Applied, thanks

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Thomas Monjalon]

13/01/2019 16:38, Nithin Kumar Dabilpuram:
> Currently this api doesn't report error if name is
> truncated and so user is not sure about uniqueness
> of name. This change reports error to help user.
> 
> Signed-off-by: Nithin Dabilpuram 
> ---
> + if (rc >= RTE_MEMZONE_NAMESIZE) {
> + RTE_ETHDEV_LOG(ERR, "truncated name");

It would be better understandable from an user perspective by saying
"ring name too long".
And it would be even better with \n at the end ;)

> + rte_errno = ENAMETOOLONG;
> + return NULL;
> + }

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vfio: don't unregister callback in secondaries

[Thomas Monjalon]

10/01/2019 17:33, Anatoly Burakov:
> Callbacks are only registered in the primary, so do not attempt to
> unregister callbacks in secondary processes.
> 
> Fixes: 43e463137154 ("vfio: support memory event callbacks")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Anatoly Burakov 

Applied, thanks

Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix wrong pointer reference in macswap

[Ferruh Yigit]

On 1/14/2019 1:39 PM, Iremonger, Bernard wrote:
>> -Original Message-
>> From: Yongseok Koh [mailto:ys...@mellanox.com]
>> Sent: Monday, January 14, 2019 10:00 AM
>> To: Lu, Wenzhuo ; Wu, Jingjing
>> ; Iremonger, Bernard 
>> Cc: dev@dpdk.org; Zhang, Qi Z ; sta...@dpdk.org
>> Subject: [PATCH 1/2] app/testpmd: fix wrong pointer reference in macswap
>>
>> The pointer is misused and could set wrong MAC address. As a result, some of
>> packets can be dropped in receiver side due to MAC address mismatch.
>> This can be shown as performance degradation.
>>
>> Bugzilla ID: 188
>>
>> Fixes: 62b52877adbe ("app/testpmd: batch MAC swap for performance on x86")
>> Cc: qi.z.zh...@intel.com
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Yongseok Koh 
> Acked-by: Bernard Iremonger 

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] memalloc: fix variable shadowing

[Thomas Monjalon]

08/01/2019 17:33, Anatoly Burakov:
> A local variable ``flags`` was shadowing another variable from outer
> scope. Fix this by renaming the variable and make it const.
> 
> Fixes: c127be93f619 ("mem: support using memfd segments for in-memory mode")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Anatoly Burakov 

Applied, thanks

Re: [dpdk-dev] [PATCH v5] eal: fix core number validation

[David Marchand]

On Mon, Jan 14, 2019 at 11:31 AM Hari Kumar Vemula <
hari.kumarx.vem...@intel.com> wrote:

> diff --git a/test/test/test_eal_flags.c b/test/test/test_eal_flags.c
> index 2acab9d69..4dc22ec36 100644
> --- a/test/test/test_eal_flags.c
> +++ b/test/test/test_eal_flags.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>
> +#include 
>  #include 
>  #include 
>
> @@ -513,6 +514,16 @@ test_missing_c_flag(void)
> const char *argv25[] = { prgname, prefix, mp_flag,
>  "-n", "3", "--lcores",
>  "0-1,2@(5-7),(3-5)@(0,2),(0,6),7"};
> +   /* core number is negative value */
> +   const char * const argv26[] = { prgname, prefix, mp_flag,
> +   "-n", "3", "-l", "-5" };
> +   const char * const argv27[] = { prgname, prefix, mp_flag,
> +   "-n", "3", "-l", "-5-7" };
> +   /* core number is maximum value */
> +   const char * const argv28[] = { prgname, prefix, mp_flag,
> +   "-n", "3", "-l", RTE_STR(RTE_MAX_LCORE) };
> +   const char * const argv29[] = { prgname, prefix, mp_flag,
> +   "-n", "3", "-l",
> "1-"RTE_STR(RTE_MAX_LCORE) };
>

Please move this block with the other "-l" options.
You can add my Reviewed-by tag with this.


Thanks.

-- 
David Marchand

[dpdk-dev] [PATCH] examples/vhost: fix a typo

[Rami Rosen]

This minor patch fixes a typo in examples/vhost/main.c.

Fixes: d19533e86f1d ("examples/vhost: copy old vhost example")

Signed-off-by: Rami Rosen 
---
 examples/vhost/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index dc9ea10..c4bdd3c 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1220,7 +1220,7 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 
 /*
  * A new device is added to a data core. First the device is added to the main 
linked list
- * and the allocated to a specific data core.
+ * and then allocated to a specific data core.
  */
 static int
 new_device(int vid)
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] memalloc: fix pointer dereferencing

[Thomas Monjalon]

08/01/2019 17:46, Anatoly Burakov:
> The original code was supposed to overwrite the value pointed to
> by the pointer, but the new one is instead overwriting the
> pointer value itself, which has no effect outside that function.
> Fix it by adding a pointer dereference.
> 
> Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Anatoly Burakov 

Applied with below title, thanks:
mem: fix storing old policy

Re: [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible

[Olivier Matz]

On Fri, Jan 11, 2019 at 02:33:16PM +, Dumitrescu, Cristian wrote:
> 
> 
> > -Original Message-
> > From: Van Haaren, Harry
> > Sent: Friday, January 11, 2019 11:33 AM
> > To: dev@dpdk.org
> > Cc: Van Haaren, Harry ; Pattan, Reshma
> > ; Dumitrescu, Cristian
> > ; olivier.m...@6wind.com;
> > tho...@monjalon.net; step...@networkplumber.org;
> > gavin...@arm.com; honnappa.nagaraha...@arm.com
> > Subject: [PATCH v3] mbuf: fix compile by making sched struct visible
> > 
> > Although C compilation works with the struct rte_mbuf_sched
> > declared inside the struct rte_mbuf namespace, C++ fails to
> > compile. This fix moves the rte_mbuf_sched struct up to the
> > global namespace, instead of declaring it inside the struct
> > mbuf namespace.
> > 
> > The struct rte_mbuf_sched is being used on the stack in
> > rte_mbuf_sched_get() and as a cast in _set(). For this
> > reason, it must be exposed as an available type.
> > 
> > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> > 
> > Signed-off-by: Harry van Haaren 
> > 
> > ---
> > 
> > v3:
> >  - Update comment in mbuf to state size of struct sched (Crisitian)
> > 
> > v2:
> >  - Different solution, not applicable, v3 based on v1 (ML discussion)
> > 
> > Cc: reshma.pat...@intel.com
> > Cc: cristian.dumitre...@intel.com
> > Cc: olivier.m...@6wind.com
> > Cc: tho...@monjalon.net
> > Cc: step...@networkplumber.org
> > Cc: gavin...@arm.com
> > Cc: honnappa.nagaraha...@arm.com
> > ---
> >  lib/librte_mbuf/rte_mbuf.h | 23 +--
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index bc562dc8a..d53b68253 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -468,6 +468,17 @@ __extension__
> >  typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8
> > bytes
> > * with a single assignment */
> > 
> > +struct rte_mbuf_sched {
> > +   uint32_t queue_id;   /**< Queue ID. */
> > +   uint8_t traffic_class;
> > +   /**< Traffic class ID. Traffic class 0
> > +* is the highest priority traffic class.
> > +*/
> > +   uint8_t color;
> > +   /**< Color. @see enum rte_color.*/
> > +   uint16_t reserved;   /**< Reserved. */
> > +}; /**< Hierarchical scheduler */
> > +
> >  /**
> >   * The generic rte_mbuf, containing a packet mbuf.
> >   */
> > @@ -574,16 +585,8 @@ struct rte_mbuf {
> >  * on PKT_RX_FDIR_* flag in ol_flags.
> >  */
> > } fdir; /**< Filter identifier if FDIR enabled */
> > -   struct rte_mbuf_sched {
> > -   uint32_t queue_id;   /**< Queue ID. */
> > -   uint8_t traffic_class;
> > -   /**< Traffic class ID. Traffic class 0
> > -* is the highest priority traffic class.
> > -*/
> > -   uint8_t color;
> > -   /**< Color. @see enum rte_color.*/
> > -   uint16_t reserved;   /**< Reserved. */
> > -   } sched; /**< Hierarchical scheduler */
> > +   struct rte_mbuf_sched sched;
> > +   /**< Hierarchical scheduler : 8 bytes */
> > struct {
> > uint32_t reserved1;
> > uint16_t reserved2;
> > --
> > 2.17.1
> 
> Acked-by: Cristian Dumitrescu 
> 
> Of course, final say belongs to Olivier, so Olivier please let un know your 
> vote.

Acked-by: Olivier Matz 

[dpdk-dev] [PATCH 0/2] net/enic: two trivial fixes

[Hyong Youb Kim]

Hyong Youb Kim (2):
  net/enic: remove useless include
  net/enic: remove redundant log level check

 drivers/net/enic/enic_clsf.c   | 2 --
 drivers/net/enic/enic_ethdev.c | 9 -
 drivers/net/enic/enic_main.c   | 1 -
 3 files changed, 4 insertions(+), 8 deletions(-)

-- 
2.16.2

[dpdk-dev] [PATCH 2/2] net/enic: remove redundant log level check

[Hyong Youb Kim]

Fixes: 8d496995346c ("net/enic: support multicast filtering")
Cc: sta...@dpdk.org

Suggested-by: Ferruh Yigit 
Signed-off-by: Hyong Youb Kim 
---
 drivers/net/enic/enic_ethdev.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index fbbec2266..8d14d8ac7 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -647,11 +647,10 @@ static int enicpmd_set_mac_addr(struct rte_eth_dev 
*eth_dev,
 static void debug_log_add_del_addr(struct ether_addr *addr, bool add)
 {
char mac_str[ETHER_ADDR_FMT_SIZE];
-   if (rte_log_get_level(enicpmd_logtype_init) == RTE_LOG_DEBUG) {
-   ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
-   PMD_INIT_LOG(ERR, " %s address %s\n",
-add ? "add" : "remove", mac_str);
-   }
+
+   ether_format_addr(mac_str, ETHER_ADDR_FMT_SIZE, addr);
+   PMD_INIT_LOG(DEBUG, " %s address %s\n",
+add ? "add" : "remove", mac_str);
 }
 
 static int enicpmd_set_mc_addr_list(struct rte_eth_dev *eth_dev,
-- 
2.16.2

[dpdk-dev] [PATCH 1/2] net/enic: remove useless include

[Hyong Youb Kim]

libgen.h is not used, so do not include it.

Fixes: fefed3d1e62c ("enic: new driver")
Cc: sta...@dpdk.org

Signed-off-by: Hyong Youb Kim 
---
 drivers/net/enic/enic_clsf.c | 2 --
 drivers/net/enic/enic_main.c | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index 9d95201ec..9e9e548c2 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -3,8 +3,6 @@
  * Copyright 2007 Nuova Systems, Inc.  All rights reserved.
  */
 
-#include 
-
 #include 
 #include 
 #include 
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 621a984a0..2652949a2 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -8,7 +8,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
-- 
2.16.2

Re: [dpdk-dev] [PATCH v2 7/7] net/tap: don't print pointer in info message

[Wiles, Keith]

> On Jan 11, 2019, at 2:35 PM, Stephen Hemminger  
> wrote:
> 
> Printing pointer in log is uninformative (unless in a debugger),
> instead print the assigned kernel device name which correlates
> well with what TAP is doing.
> 
> Signed-off-by: Stephen Hemminger 
> ---
> drivers/net/tap/rte_eth_tap.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 705c90dadee5..586c8a952df9 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -853,6 +853,8 @@ tap_dev_stop(struct rte_eth_dev *dev)
> static int
> tap_dev_configure(struct rte_eth_dev *dev)
> {
> + struct pmd_internals *pmd = dev->data->dev_private;
> +
>   if (dev->data->nb_rx_queues > RTE_PMD_TAP_MAX_QUEUES) {
>   TAP_LOG(ERR,
>   "%s: number of rx queues %d exceeds max num of queues 
> %d",
> @@ -870,11 +872,11 @@ tap_dev_configure(struct rte_eth_dev *dev)
>   return -1;
>   }
> 
> - TAP_LOG(INFO, "%s: %p: TX configured queues number: %u",
> - dev->device->name, (void *)dev, dev->data->nb_tx_queues);
> + TAP_LOG(INFO, "%s: %s: TX configured queues number: %u",
> + dev->device->name, pmd->name, dev->data->nb_tx_queues);
> 
> - TAP_LOG(INFO, "%s: %p: RX configured queues number: %u",
> - dev->device->name, (void *)dev, dev->data->nb_rx_queues);
> + TAP_LOG(INFO, "%s: %s: RX configured queues number: %u",
> + dev->device->name, pmd->name, dev->data->nb_rx_queues);

Acked-by: Keith Wiles 
> 
>   return 0;
> }
> -- 
> 2.20.1
> 

Regards,
Keith

[dpdk-dev] [PATCH] doc: update description in qat compression guide

[Tomasz Jozwiak]

Added limitations description for the QAT compression PMD.
Updated intermediate buffers description in qat.rst file.

Signed-off-by: Tomasz Jozwiak 
---
 doc/guides/compressdevs/qat_comp.rst | 4 
 doc/guides/cryptodevs/qat.rst| 6 --
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/doc/guides/compressdevs/qat_comp.rst 
b/doc/guides/compressdevs/qat_comp.rst
index 567a1fc..5631cb1 100644
--- a/doc/guides/compressdevs/qat_comp.rst
+++ b/doc/guides/compressdevs/qat_comp.rst
@@ -35,6 +35,10 @@ Limitations
 * Compressdev level 0, no compression, is not supported.
 * Queue pairs are not thread-safe (that is, within a single queue pair, RX and 
TX from different lcores is not supported).
 * No BSD support as BSD QAT kernel driver not available.
+* Number of segments in mbuf chains in the op must be <= 
RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS from the config file.
+* When using Deflate dynamic huffman encoding for compression, the input size 
(op.src.length)
+  must be < CONFIG_RTE_PMD_QAT_COMP_IM_BUFFER_SIZE from the config file,
+  see :ref:`building_qat_config` for more details.
 
 
 Installation
diff --git a/doc/guides/cryptodevs/qat.rst b/doc/guides/cryptodevs/qat.rst
index 9fb9f01..d692c17 100644
--- a/doc/guides/cryptodevs/qat.rst
+++ b/doc/guides/cryptodevs/qat.rst
@@ -145,6 +145,8 @@ Quick instructions for QAT compressdev PMD are as follows:
make
 
 
+.. _building_qat_config:
+
 Build Configuration
 ~~~
 
@@ -180,8 +182,8 @@ An extra (max_inflight_ops x 16) bytes per queue_pair will 
be used for every inc
 QAT compression PMD needs intermediate buffers to support Deflate compression
 with Dynamic Huffman encoding. CONFIG_RTE_PMD_QAT_COMP_IM_BUFFER_SIZE
 specifies the size of a single buffer, the PMD will allocate a multiple of 
these,
-plus some extra space for associated meta-data. For GEN2 devices, 20 buffers 
plus
-1472 bytes are allocated.
+plus some extra space for associated meta-data. For GEN2 devices, 20 buffers 
are
+allocated while for GEN1 devices, 12 buffers are allocated, plus 1472 bytes 
overhead.
 
 .. Note::
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH 2/2] mbuf: remove experimental tag of external buffer attachment

[Olivier Matz]

On Mon, Jan 14, 2019 at 10:42:21AM +0300, Andrew Rybchenko wrote:
> On 1/14/19 8:20 AM, Yongseok Koh wrote:
> > Remove the experimental tag of rte_pktmbuf_attach_extbuf() which was
> > introduced in 18.05.
> > 
> > Signed-off-by: Yongseok Koh 
> 
> Acked-by: Andrew Rybchenko 
> 

Acked-by: Olivier Matz 

Re: [dpdk-dev] [PATCH 1/2] mbuf: remove deprecated macro

[Olivier Matz]

On Mon, Jan 14, 2019 at 10:50:50AM +0100, Thomas Monjalon wrote:
> 14/01/2019 08:40, Andrew Rybchenko:
> > On 1/14/19 8:20 AM, Yongseok Koh wrote:
> > > RTE_MBUF_INDIRECT() is replaced with RTE_MBUF_CLONED() and removed.
> > > This macro was deprecated in release 18.05 when EXT_ATTACHED_MBUF was
> > > introduced.
> > >
> > > Signed-off-by: Yongseok Koh 
> > 
> > Acked-by: Andrew Rybchenko 

Acked-by: Olivier Matz 

> > 
> > <...>
> > 
> > > diff --git a/doc/guides/rel_notes/release_19_02.rst 
> > > b/doc/guides/rel_notes/release_19_02.rst
> > > index 5a46f1acdc..c4ad072412 100644
> > > --- a/doc/guides/rel_notes/release_19_02.rst
> > > +++ b/doc/guides/rel_notes/release_19_02.rst
> > > @@ -210,6 +210,9 @@ API Changes
> > > function from now on. Failed to do so will cause
> > > ``rte_cryptodev_sym_session_create()`` function call return error.
> > >   
> > > +* mbuf: ``RTE_MBUF_INDIRECT()``, which was deprecated in 18.05, was 
> > > replaced
> > > +  with ``RTE_MBUF_CLONED()`` and removed in 19.02.
> > > +
> > 
> > May be order in New Features should be applicable in API changes as well.
> 
> Yes
> Usually I take care of it when applying, but would be better to be fixed
> in the patch itself :)
> 
> 

Re: [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible

[Thomas Monjalon]

14/01/2019 15:58, Olivier Matz:
> On Fri, Jan 11, 2019 at 02:33:16PM +, Dumitrescu, Cristian wrote:
> > From: Van Haaren, Harry
> > > 
> > > Although C compilation works with the struct rte_mbuf_sched
> > > declared inside the struct rte_mbuf namespace, C++ fails to
> > > compile. This fix moves the rte_mbuf_sched struct up to the
> > > global namespace, instead of declaring it inside the struct
> > > mbuf namespace.
> > > 
> > > The struct rte_mbuf_sched is being used on the stack in
> > > rte_mbuf_sched_get() and as a cast in _set(). For this
> > > reason, it must be exposed as an available type.
> > > 
> > > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> > > 
> > > Signed-off-by: Harry van Haaren 
> > > 
> > > ---
> > > 
> > > v3:
> > >  - Update comment in mbuf to state size of struct sched (Crisitian)
> > > 
> > > v2:
> > >  - Different solution, not applicable, v3 based on v1 (ML discussion)
> > 
> > Acked-by: Cristian Dumitrescu 
> > 
> > Of course, final say belongs to Olivier, so Olivier please let un know your 
> > vote.
> 
> Acked-by: Olivier Matz 

Applied, thanks

Re: [dpdk-dev] [PATCH] examples/vhost: fix a typo

[Gavin Hu (Arm Technology China)]

> -Original Message-
> From: dev  On Behalf Of Rami Rosen
> Sent: Monday, January 14, 2019 10:48 PM
> To: dev@dpdk.org
> Cc: maxime.coque...@redhat.com; huawei@intel.com;
> tiwei@intel.com; Rami Rosen 
> Subject: [dpdk-dev] [PATCH] examples/vhost: fix a typo
>
> This minor patch fixes a typo in examples/vhost/main.c.
>
> Fixes: d19533e86f1d ("examples/vhost: copy old vhost example")
>
> Signed-off-by: Rami Rosen 
> ---
>  examples/vhost/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index dc9ea10..c4bdd3c 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -1220,7 +1220,7 @@ static void virtio_tx_offload(struct rte_mbuf *m)
>
>  /*
>   * A new device is added to a data core. First the device is added to the
> main linked list
> - * and the allocated to a specific data core.
> + * and then allocated to a specific data core.
>   */
>  static int
>  new_device(int vid)
> --
> 1.8.3.1

Reviewed-by: Gavin Hu 

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.

Re: [dpdk-dev] [PATCH 1/2] mbuf: remove deprecated macro

[Thomas Monjalon]

14/01/2019 16:23, Olivier Matz:
> On Mon, Jan 14, 2019 at 10:50:50AM +0100, Thomas Monjalon wrote:
> > 14/01/2019 08:40, Andrew Rybchenko:
> > > On 1/14/19 8:20 AM, Yongseok Koh wrote:
> > > > RTE_MBUF_INDIRECT() is replaced with RTE_MBUF_CLONED() and removed.
> > > > This macro was deprecated in release 18.05 when EXT_ATTACHED_MBUF was
> > > > introduced.
> > > >
> > > > Signed-off-by: Yongseok Koh 
> > > 
> > > Acked-by: Andrew Rybchenko 
> 
> Acked-by: Olivier Matz 

Applied, thanks

Re: [dpdk-dev] [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)

[Eads, Gage]

From: Andrew Rybchenko [mailto:arybche...@solarflare.com]
Sent: Sunday, January 13, 2019 6:19 AM
To: Eads, Gage ; dev@dpdk.org
Cc: olivier.m...@6wind.com; Richardson, Bruce ; 
Ananyev, Konstantin 
Subject: Re: [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)

On 1/10/19 11:55 PM, Gage Eads wrote:

This operation can be used for non-blocking algorithms, such as a

non-blocking stack or ring.



Signed-off-by: Gage Eads 

---

 .../common/include/arch/x86/rte_atomic_64.h| 22 ++

 1 file changed, 22 insertions(+)



diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h 
b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h

index fd2ec9c53..34c2addf8 100644

--- a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h

+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h

@@ -34,6 +34,7 @@

 /*

  * Inspired from FreeBSD src/sys/amd64/include/atomic.h

  * Copyright (c) 1998 Doug Rabson

+ * Copyright (c) 2019 Intel Corporation

  * All rights reserved.

  */



@@ -208,4 +209,25 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)

 }

 #endif



+static inline int

+rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t *src)

+{

+  uint8_t res;

+

+  asm volatile (

+MPLOCKED

+"cmpxchg16b %[dst];"

+" sete %[res]"

+: [dst] "=m" (*dst),

+  [res] "=r" (res)

+: "c" (src[1]),

+  "b" (src[0]),

+  "m" (*dst),

+  "d" (exp[1]),

+  "a" (exp[0])

+: "memory");

+

+  return res;

+}

+

 #endif /* _RTE_ATOMIC_X86_64_

Is it OK to add it to rte_atomic_64.h header which is for 64-bit integer ops?

Andrew.

I believe this file is for atomic operations specific to x86_64 builds, but not 
necessarily limited to 64-bit operations (note that rte_atomic_32.h contains 
64-bit operations specific to 32-bit builds). At least, that’s how I 
interpreted it.

Re: [dpdk-dev] [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)

[Eads, Gage]

> -Original Message-
> From: Varghese, Vipin
> Sent: Sunday, January 13, 2019 10:29 PM
> To: Andrew Rybchenko ; Eads, Gage
> ; dev@dpdk.org
> Cc: olivier.m...@6wind.com; Richardson, Bruce ;
> Ananyev, Konstantin 
> Subject: RE: [dpdk-dev] [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)
> 
> Hi Gage,
> 
> snipped
> > > @@ -208,4 +209,25 @@ static inline void
> > rte_atomic64_clear(rte_atomic64_t *v)
> > >   }
> > >   #endif
> > >
> > > +static inline int
> > > +rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t
> > > +*src) {
> > > + uint8_t res;
> > > +
> > > + asm volatile (
> > > +   MPLOCKED
> > > +   "cmpxchg16b %[dst];"
> > > +   " sete %[res]"
> > > +   : [dst] "=m" (*dst),
> > > + [res] "=r" (res)
> > > +   : "c" (src[1]),
> > > + "b" (src[0]),
> > > + "m" (*dst),
> > > + "d" (exp[1]),
> > > + "a" (exp[0])
> > > +   : "memory");
> Since update depends upon on the 'set|unset' value of ZF, should we first set 
> ZF
> to 0?
> 
> Apologies in advance if it is internally taken care by 'sete'.

cmpxchg16b will set the ZF if the compared values are equal, else it will clear 
the ZF, so there's no need to initialize the ZF.

Source: https://www.felixcloutier.com/x86/cmpxchg8b:cmpxchg16b

Thanks,
Gage

[dpdk-dev] [PATCH v5] mk: fix scope of disabling AVX512F support

[Ferruh Yigit]

AVX512 was disabled for GCC because of Bugzilla issue 97 [1],
the GCC defect submitted for the issue [2] highlighted that this is
a known binutils version 2.30 issue.

Narrowed the scope of no-avx512 to the this specific binutils version.

[1]
https://bugs.dpdk.org/show_bug.cgi?id=97

[2]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096

Fixes: 8d07c82b239f ("mk: disable gcc AVX512F support")
Cc: sta...@dpdk.org

Signed-off-by: Ferruh Yigit 
---
Cc: Tom Barbette 
Cc: Thomas Monjalon 
Cc: Yongseok Koh 
Cc: Konstantin Ananyev 
Cc: Bruce Richardson 

v2:
* Added warning message (print only once for eal)
* Moved decision details to compat file, kept execution in cpuflags

v3:
* replace 'ld' with '$(LD)' for cross build
* added meson support

v4:
* Document this limitation in release notes
Cc: Vipin Varghese 

v5:
* Grammar fix
* remove $(lastword.. ) from makefile
* add 'OR' between possible workarounds
---
 config/meson.build   |  8 ++
 doc/guides/rel_notes/known_issues.rst| 35 
 doc/guides/rel_notes/release_19_02.rst   | 15 ++
 mk/rte.cpuflags.mk   |  4 +--
 mk/toolchain/gcc/rte.toolchain-compat.mk | 10 +++
 5 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index db32499b3..40802fc88 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -43,6 +43,14 @@ toolchain = cc.get_id()
 dpdk_conf.set_quoted('RTE_TOOLCHAIN', toolchain)
 dpdk_conf.set('RTE_TOOLCHAIN_' + toolchain.to_upper(), 1)
 
+# get binutils version for the workaround of Bug 97
+ldver = run_command('ld', '-v').stdout().strip()
+if ldver.contains('2.30')
+   if cc.has_argument('-mno-avx512f')
+   machine_args += '-mno-avx512f'
+   endif
+endif
+
 add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
 dpdk_extra_ldflags += '-Wl,--no-as-needed'
 
diff --git a/doc/guides/rel_notes/known_issues.rst 
b/doc/guides/rel_notes/known_issues.rst
index a1face9c1..358dfa321 100644
--- a/doc/guides/rel_notes/known_issues.rst
+++ b/doc/guides/rel_notes/known_issues.rst
@@ -826,3 +826,38 @@ Kernel crash when hot-unplug igb_uio device while DPDK 
application is running
 
 **Driver/Module**:
``igb_uio`` module.
+
+
+AVX-512 support disabled
+
+
+**Description**:
+   ``AVX-512`` support has been disabled on some conditions.
+   This shouldn't be confused with ``CONFIG_RTE_ENABLE_AVX512`` config option 
which is already
+   disabled by default. This config option defines if ``AVX-512`` specific 
implementations of
+   some file to be used or not. What has been disabled is compiler feature to 
produce ``AVX-512``
+   instructions from any source code.
+
+   On DPDK v18.11 ``AVX-512`` is disabled for all ``GCC`` builds which 
reported to cause a performance
+   drop.
+
+   On DPDK v19.02 ``AVX-512`` disable scope is reduced to ``GCC`` and 
``binutils version 2.30`` based
+   on information accured from the GCC community defect.
+
+**Reason**:
+   Generated ``AVX-512`` code cause crash:
+   https://bugs.dpdk.org/show_bug.cgi?id=97
+   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096
+
+**Resolution/Workaround**:
+   * Update ``binutils`` to newer version than ``2.30``.
+
+   OR
+
+   * Use different compiler, like ``clang`` for this case.
+
+**Affected Environment/Platform**:
+``GCC`` and ``binutils version 2.30``.
+
+**Driver/Module**:
+ALL.
diff --git a/doc/guides/rel_notes/release_19_02.rst 
b/doc/guides/rel_notes/release_19_02.rst
index d542110dd..a89042963 100644
--- a/doc/guides/rel_notes/release_19_02.rst
+++ b/doc/guides/rel_notes/release_19_02.rst
@@ -339,6 +339,21 @@ Known Issues
Also, make sure to start the actual text at the margin.
=
 
+* ``AVX-512`` support has been disabled for ``GCC`` builds when ``binutils 
2.30``
+  is detected [1] because of a crash [2]. This can affect ``native`` machine 
type
+  build targets on the platforms that support ``AVX512F`` like ``Intel 
Skylake``
+  processors, and can cause a possible performance drop. The immediate 
workaround
+  is to use ``clang`` compiler on these platforms.
+  Initial workaround in DPDK v18.11 was to disable ``AVX-512`` support for 
``GCC``
+  completely, but based on information on defect submitted to GCC community 
[3],
+  issue has been identified as ``binutils 2.30`` issue. Since currently only 
GCC
+  generates ``AVX-512`` instructions, the scope is limited to ``GCC`` and
+  ``binutils 2.30``
+
+  - [1]: Commit ("mk: fix scope of disabling AVX512F support")
+  - [2]: https://bugs.dpdk.org/show_bug.cgi?id=97
+  - [3]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096
+
 
 Tested Platforms
 
diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
index c3291b17a..541211c61 100644
--- a/mk/rte.cpuflags.mk
+++ b/mk/rte.cpuflags.mk
@@ -69,8 +69,8 @@ ifneq ($(filter $(AUTO_CPUFLAGS),__AVX512F__),)
 ifeq ($(CONFIG_RTE

Re: [dpdk-dev] [PATCH v3 1/2] mbuf: add function returning default buffer address

[Olivier Matz]

Hi,

On Fri, Jan 11, 2019 at 01:48:12PM +0100, David Marchand wrote:
> On Fri, Jan 11, 2019 at 12:57 PM Bruce Richardson <
> bruce.richard...@intel.com> wrote:
> 
> > On Fri, Jan 11, 2019 at 02:17:04PM +0300, Andrew Rybchenko wrote:
> > > Olivier, David,
> > >
> > > could you take a look at naming suggested below and share your thoughts.
> > > My fear is that rte_mbuf_buf_addr() is too generic and true for direct
> > mbuf
> > > only. That's why I'd like to highlight it in the function name.
> > >
> >
> > I would tend to agree with that concern.
> >
> 
> I understand your concern as well.
> 
> The only usecase we have so far is for drivers on the rx side, so
> implicitely direct mbufs.
> But from a api user pov, explicit is always better.
> 
> I will let Olivier have the last word :-)

Thanks Andrew for pointing this out.

However I agree with Yongseok: we already have many functions that
applies to direct mbufs that don't have "direct" in their names.

In my opinion, rte_mbuf_buf_addr() is a good name, but I think the
doxygen comment could be improved a bit to state that it returns the
pointer to the embedded data. I also think that a small comment
explaining why the mp arg is required would be helpful.

Thanks,
Olivier

[dpdk-dev] [PATCH] ipsec: fix compilation problem

[Konstantin Ananyev]

gcc 6.2 and 6.4 fails to compile lib/librte_ipsec/sa.c
with the following errors:
/local/kananye1/dpdk.org/lib/librte_ipsec/sa.c:
 In function âinline_outb_tun_pkt_processâ:
 dpdk.org/x86_64-native-linuxapp-gcc/include/rte_memcpy.h:337:2:
 error: array subscript is above array bounds [-Werror=array-bounds]
  rte_mov32((uint8_t *)dst + 1 * 32, (const uint8_t *)src + 1 * 32);
  ^
 ...

It complains about the following lines of code:
esp_outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t

/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + sa->hdr_len);
iv = (uint64_t *)(esph + 1);
rte_memcpy(iv, ivp, sa->iv_len);

While I believe it is a false positive,
it is too excessive to use rte_memcpy() here,
as IV length could be only 0/8/16 bytes.
So introduce small helper function to copy IV and use it
instead of rte_memcpy().

Fixes: 4d7ea3e1459b ("ipsec: implement SA data-path API")
Signed-off-by: Konstantin Ananyev 
---
 lib/librte_ipsec/crypto.h | 25 +
 lib/librte_ipsec/sa.c |  6 +++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/lib/librte_ipsec/crypto.h b/lib/librte_ipsec/crypto.h
index 61f5c1433..b5f264831 100644
--- a/lib/librte_ipsec/crypto.h
+++ b/lib/librte_ipsec/crypto.h
@@ -76,6 +76,31 @@ gen_iv(uint64_t iv[IPSEC_MAX_IV_QWORD], rte_be64_t sqn)
iv[1] = 0;
 }
 
+/*
+ * Helper routine to copy IV
+ * Righ now we support only algorithms with IV length equals 0/8/16 bytes.
+ */
+static inline void
+copy_iv(uint64_t dst[IPSEC_MAX_IV_QWORD],
+   const uint64_t src[IPSEC_MAX_IV_QWORD], uint32_t len)
+{
+   RTE_BUILD_BUG_ON(IPSEC_MAX_IV_SIZE != 2 * sizeof(uint64_t));
+
+   switch (len) {
+   case IPSEC_MAX_IV_SIZE:
+   dst[1] = src[1];
+   /* fallthrough */
+   case sizeof(uint64_t):
+   dst[0] = src[0];
+   /* fallthrough */
+   case 0:
+   break;
+   default:
+   /* should never happen */
+   RTE_ASSERT(NULL);
+   }
+}
+
 /*
  * from RFC 4303 3.3.2.1.4:
  * If the ESN option is enabled for the SA, the high-order 32
diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
index 8d4ce1ac6..5f55c2a4e 100644
--- a/lib/librte_ipsec/sa.c
+++ b/lib/librte_ipsec/sa.c
@@ -526,7 +526,7 @@ esp_outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, 
rte_be64_t sqc,
/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + sa->hdr_len);
iv = (uint64_t *)(esph + 1);
-   rte_memcpy(iv, ivp, sa->iv_len);
+   copy_iv(iv, ivp, sa->iv_len);
 
esph->spi = sa->spi;
esph->seq = sqn_low32(sqc);
@@ -689,7 +689,7 @@ esp_outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, 
rte_be64_t sqc,
/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + uhlen);
iv = (uint64_t *)(esph + 1);
-   rte_memcpy(iv, ivp, sa->iv_len);
+   copy_iv(iv, ivp, sa->iv_len);
 
esph->spi = sa->spi;
esph->seq = sqn_low32(sqc);
@@ -821,7 +821,7 @@ esp_inb_tun_cop_prepare(struct rte_crypto_op *cop,
ivc = rte_crypto_op_ctod_offset(cop, uint64_t *, sa->iv_ofs);
ivp = rte_pktmbuf_mtod_offset(mb, uint64_t *,
pofs + sizeof(struct esp_hdr));
-   rte_memcpy(ivc, ivp, sa->iv_len);
+   copy_iv(ivc, ivp, sa->iv_len);
}
return 0;
 }
-- 
2.17.1

Re: [dpdk-dev] [PATCH] doc: update description in qat compression guide

[Trahe, Fiona]

> -Original Message-
> From: Jozwiak, TomaszX
> Sent: Monday, January 14, 2019 3:11 PM
> To: dev@dpdk.org; Trahe, Fiona ; Jozwiak, TomaszX
> ; De Lara Guarch, Pablo 
> 
> Subject: [PATCH] doc: update description in qat compression guide
> 
> Added limitations description for the QAT compression PMD.
> Updated intermediate buffers description in qat.rst file.
> 
> Signed-off-by: Tomasz Jozwiak 
Acked-by: Fiona Trahe 

[dpdk-dev] [PATCH] eal: fix build of external apps with clang on armv8

[Ilya Maximets]

In case DPDK built using GCC, RTE_TOOLCHAIN_CLANG is not defined.
But 'rte_atomic.h' is a generic header that included to the
external apps like OVS while building with DPDK. As a result,
clang build of OVS fails on ARMv8 if DPDK built using gcc:

include/generic/rte_atomic.h:215:9: error:
implicit declaration of function '__atomic_exchange_2'
is invalid in C99
include/generic/rte_atomic.h:494:9: error:
implicit declaration of function '__atomic_exchange_4'
is invalid in C99
include/generic/rte_atomic.h:772:9: error:
implicit declaration of function '__atomic_exchange_8'
is invalid in C99

We need to check for current compiler, not the compiler used for
DPDK build.

Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
Cc: sta...@dpdk.org

Signed-off-by: Ilya Maximets 
---
 lib/librte_eal/common/include/generic/rte_atomic.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h 
b/lib/librte_eal/common/include/generic/rte_atomic.h
index b99ba4688..d0c464fb1 100644
--- a/lib/librte_eal/common/include/generic/rte_atomic.h
+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
@@ -212,7 +212,7 @@ rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val);
 static inline uint16_t
 rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
 {
-#if defined(RTE_ARCH_ARM64) && defined(RTE_TOOLCHAIN_CLANG)
+#if defined(RTE_ARCH_ARM64) && defined(__clang__)
return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
 #else
return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
@@ -495,7 +495,7 @@ rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val);
 static inline uint32_t
 rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
 {
-#if defined(RTE_ARCH_ARM64) && defined(RTE_TOOLCHAIN_CLANG)
+#if defined(RTE_ARCH_ARM64) && defined(__clang__)
return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
 #else
return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
@@ -777,7 +777,7 @@ rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val);
 static inline uint64_t
 rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
 {
-#if defined(RTE_ARCH_ARM64) && defined(RTE_TOOLCHAIN_CLANG)
+#if defined(RTE_ARCH_ARM64) && defined(__clang__)
return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
 #else
return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
-- 
2.17.1

Re: [dpdk-dev] [PATCH 2/3] mempool/nb_stack: add non-blocking stack mempool

[Eads, Gage]

> -Original Message-
> From: Andrew Rybchenko [mailto:arybche...@solarflare.com]
> Sent: Sunday, January 13, 2019 7:31 AM
> To: Eads, Gage ; dev@dpdk.org
> Cc: olivier.m...@6wind.com; Richardson, Bruce ;
> Ananyev, Konstantin 
> Subject: Re: [PATCH 2/3] mempool/nb_stack: add non-blocking stack mempool
> 
> Hi Gage,
> 
> In general looks very good.
> 
> Have you considered to make nb_lifo.h a library to be reusable outside of the
> mempool driver?

I'm certainly open to it, if the community can benefit. Due to the difficulty 
of adding a new lib/ directory (I believe this requires Tech Board approval), 
I'll defer that work to a separate patchset.

> 
> There are few notes below.
> 
> Thanks,
> Andrew.
> 
> On 1/10/19 11:55 PM, Gage Eads wrote:
> 
> 
>   This commit adds support for non-blocking (linked list based) stack
> mempool
>   handler. The stack uses a 128-bit compare-and-swap instruction, and
> thus is
>   limited to x86_64. The 128-bit CAS atomically updates the stack top
> pointer
>   and a modification counter, which protects against the ABA problem.
> 
>   In mempool_perf_autotest the lock-based stack outperforms the non-
> blocking
>   handler*, however:
>   - For applications with preemptible pthreads, a lock-based stack's
> worst-case performance (i.e. one thread being preempted while
> holding the spinlock) is much worse than the non-blocking stack's.
>   - Using per-thread mempool caches will largely mitigate the
> performance
> difference.
> 
>   *Test setup: x86_64 build with default config, dual-socket Xeon E5-2699
> v4,
>   running on isolcpus cores with a tickless scheduler. The lock-based
> stack's
>   rate_persec was 1x-3.5x the non-blocking stack's.
> 
>   Signed-off-by: Gage Eads 
> 
>   ---
>MAINTAINERS|   4 +
>config/common_base |   1 +
>drivers/mempool/Makefile   |   1 +
>drivers/mempool/nb_stack/Makefile  |  30 +
>drivers/mempool/nb_stack/meson.build   |   4 +
>drivers/mempool/nb_stack/nb_lifo.h | 132
> +
>drivers/mempool/nb_stack/rte_mempool_nb_stack.c| 125
> +++
>.../nb_stack/rte_mempool_nb_stack_version.map  |   4 +
>mk/rte.app.mk  |   1 +
>9 files changed, 302 insertions(+)
>create mode 100644 drivers/mempool/nb_stack/Makefile
>create mode 100644 drivers/mempool/nb_stack/meson.build
>create mode 100644 drivers/mempool/nb_stack/nb_lifo.h
>create mode 100644
> drivers/mempool/nb_stack/rte_mempool_nb_stack.c
>create mode 100644
> drivers/mempool/nb_stack/rte_mempool_nb_stack_version.map
> 
>   diff --git a/MAINTAINERS b/MAINTAINERS
>   index 470f36b9c..5519d3323 100644
>   --- a/MAINTAINERS
>   +++ b/MAINTAINERS
>   @@ -416,6 +416,10 @@ M: Artem V. Andreev
>  
>M: Andrew Rybchenko 
> 
>F: drivers/mempool/bucket/
> 
>   +Non-blocking stack memory pool
>   +M: Gage Eads  
>   +F: drivers/mempool/nb_stack/
>   +
> 
>Bus Drivers
>---
>   diff --git a/config/common_base b/config/common_base
>   index 964a6956e..40ce47312 100644
>   --- a/config/common_base
>   +++ b/config/common_base
>   @@ -728,6 +728,7 @@ CONFIG_RTE_DRIVER_MEMPOOL_BUCKET=y
>CONFIG_RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB=64
>CONFIG_RTE_DRIVER_MEMPOOL_RING=y
>CONFIG_RTE_DRIVER_MEMPOOL_STACK=y
>   +CONFIG_RTE_DRIVER_MEMPOOL_NB_STACK=y
> 
> 
> Typically it is alphabetically sorted.

Will fix.

> 
> 
> 
>#
># Compile PMD for octeontx fpa mempool device
>   diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
>   index 28c2e8360..aeae3ac12 100644
>   --- a/drivers/mempool/Makefile
>   +++ b/drivers/mempool/Makefile
>   @@ -13,5 +13,6 @@ endif
>DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += ring
>DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += stack
>DIRS-$(CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL) += octeontx
>   +DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_NB_STACK) += nb_stack
> 
> 
> Typically it is alphabetically sorted. Yes, already broken, but, please, put 
> it
> before ring.
> 

Sure, will do.

> 
> 
> 
>include $(RTE_SDK)/mk/rte.subdir.mk
>   diff --git a/drivers/mempool/nb_stack/Makefile
> b/drivers/mempool/nb_stack/Makefile
>   new file mode 100644
>   index 0..38b45f4f5
>   --- /dev/null
>   +++ b/drivers/mempool/nb_stack/Makefile
>   @@ -0,0 +1,30 @@
>   +# SPDX-License-Identifier: BSD-3-Clause
>   +# Copyright(c) 2019 Intel Corporation
>   +
> 

Re: [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat

[Stephen Hemminger]

On Mon, 14 Jan 2019 13:29:38 +
Ferruh Yigit  wrote:

> On 1/14/2019 6:04 AM, Chaitanya Babu Talluri wrote:
> > Strcat does not check the destination length and there might be
> > chances of string overflow so insted of strcat, strncat is used.
> > 
> > Fixes: 540a211084 ("bnx2x: driver core")
> > Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> > Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> > Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> > Cc: sta...@dpdk.org
> > 
> > Signed-off-by: Chaitanya Babu Talluri   
> 
> <...>
> 
> > @@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct 
> > nfp_pcie_user *desc)
> >  * driver is used because that implies root user.
> >  */
> > home_path = getenv("HOME");
> > -   lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
> > +   lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
> >   sizeof(char));
> >  
> > -   strcat(lockfile, home_path);
> > -   strcat(lockfile, "/.lock_nfp_secondary");
> > +   strncat(lockfile, home_path, LOCKFILE_HOME_PATH);
> > +   strncat(lockfile, lockname, strlen(lockfile));  
> 
> I guess this need to be 'LOCKFILE_HOME_PATH - strlen(lockfile) - 1' instead.
> But also this can be implemented as 'snprintf()'
> 
> Since 'lockfile' allocated dynamically based on sizes of existing strings, 
> using
> 'lockname' instead of "/.lock_nfp_secondary" will show that there won't be any
> overflow but tools still may be complaining about 'strcat' usage.
> 
> 

Why not use vasprintf() instead of doing manual construction?

Re: [dpdk-dev] [PATCH] eal: fix build of external apps with clang on armv8

[Thomas Monjalon]

14/01/2019 17:14, Ilya Maximets:
> In case DPDK built using GCC, RTE_TOOLCHAIN_CLANG is not defined.
> But 'rte_atomic.h' is a generic header that included to the
> external apps like OVS while building with DPDK. As a result,
> clang build of OVS fails on ARMv8 if DPDK built using gcc:
> 
> include/generic/rte_atomic.h:215:9: error:
> implicit declaration of function '__atomic_exchange_2'
> is invalid in C99
> include/generic/rte_atomic.h:494:9: error:
> implicit declaration of function '__atomic_exchange_4'
> is invalid in C99
> include/generic/rte_atomic.h:772:9: error:
> implicit declaration of function '__atomic_exchange_8'
> is invalid in C99
> 
> We need to check for current compiler, not the compiler used for
> DPDK build.

Right, API cannot rely on internal build system configuration.

> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ilya Maximets 

Acked-by: Thomas Monjalon 

Re: [dpdk-dev] [PATCH v5] mk: fix scope of disabling AVX512F support

[Thomas Monjalon]

14/01/2019 16:49, Ferruh Yigit:
> AVX512 was disabled for GCC because of Bugzilla issue 97 [1],
> the GCC defect submitted for the issue [2] highlighted that this is
> a known binutils version 2.30 issue.
> 
> Narrowed the scope of no-avx512 to the this specific binutils version.
> 
> [1]
> https://bugs.dpdk.org/show_bug.cgi?id=97
> 
> [2]
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096
> 
> Fixes: 8d07c82b239f ("mk: disable gcc AVX512F support")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ferruh Yigit 
> ---
> Cc: Tom Barbette 
> Cc: Thomas Monjalon 
> Cc: Yongseok Koh 
> Cc: Konstantin Ananyev 
> Cc: Bruce Richardson 
> 
> v2:
> * Added warning message (print only once for eal)
> * Moved decision details to compat file, kept execution in cpuflags
> 
> v3:
> * replace 'ld' with '$(LD)' for cross build
> * added meson support
> 
> v4:
> * Document this limitation in release notes
> Cc: Vipin Varghese 
> 
> v5:
> * Grammar fix
> * remove $(lastword.. ) from makefile
> * add 'OR' between possible workarounds

Acked-by: Thomas Monjalon 

Applied, thanks

Re: [dpdk-dev] [PATCH 0/2] net/enic: two trivial fixes

[Ferruh Yigit]

On 1/14/2019 3:01 PM, Hyong Youb Kim wrote:
> Hyong Youb Kim (2):
>   net/enic: remove useless include
>   net/enic: remove redundant log level check

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH] eal: fix build of external apps with clang on armv8

[Richardson, Bruce]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ilya Maximets
> Sent: Monday, January 14, 2019 4:15 PM
> To: dev@dpdk.org; Thomas Monjalon 
> Cc: Pavan Nikhilesh ; Ilya Maximets
> ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] eal: fix build of external apps with clang on
> armv8
> 
> In case DPDK built using GCC, RTE_TOOLCHAIN_CLANG is not defined.
> But 'rte_atomic.h' is a generic header that included to the external apps
> like OVS while building with DPDK. As a result, clang build of OVS fails
> on ARMv8 if DPDK built using gcc:
> 
> include/generic/rte_atomic.h:215:9: error:
> implicit declaration of function '__atomic_exchange_2'
> is invalid in C99
> include/generic/rte_atomic.h:494:9: error:
> implicit declaration of function '__atomic_exchange_4'
> is invalid in C99
> include/generic/rte_atomic.h:772:9: error:
> implicit declaration of function '__atomic_exchange_8'
> is invalid in C99
> 
> We need to check for current compiler, not the compiler used for DPDK
> build.
> 
> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/librte_eal/common/include/generic/rte_atomic.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h
> b/lib/librte_eal/common/include/generic/rte_atomic.h
> index b99ba4688..d0c464fb1 100644
> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> @@ -212,7 +212,7 @@ rte_atomic16_exchange(volatile uint16_t *dst, uint16_t
> val);  static inline uint16_t  rte_atomic16_exchange(volatile uint16_t
> *dst, uint16_t val)  { -#if defined(RTE_ARCH_ARM64) &&
> defined(RTE_TOOLCHAIN_CLANG)
> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>   return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>   return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST); @@ -495,7
> +495,7 @@ rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val);
> static inline uint32_t  rte_atomic32_exchange(volatile uint32_t *dst,
> uint32_t val)  { -#if defined(RTE_ARCH_ARM64) &&
> defined(RTE_TOOLCHAIN_CLANG)
> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>   return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>   return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST); @@ -777,7
> +777,7 @@ rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val);
> static inline uint64_t  rte_atomic64_exchange(volatile uint64_t *dst,
> uint64_t val)  { -#if defined(RTE_ARCH_ARM64) &&
> defined(RTE_TOOLCHAIN_CLANG)
> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>   return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>   return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
> --
> 2.17.1

Is this really architecture-specific? Would the same issue not occur on e.g. 
x86 or PPC?

Re: [dpdk-dev] [PATCH] eal: fix build of external apps with clang on armv8

[Honnappa Nagarahalli]

> 
> In case DPDK built using GCC, RTE_TOOLCHAIN_CLANG is not defined.
> But 'rte_atomic.h' is a generic header that included to the external apps like
> OVS while building with DPDK. As a result, clang build of OVS fails on ARMv8

   ^^
Typo, should be armv8.
Commit '40fd87486799' should have caught the typo issue. I don't want to do 
this for every patch 😊 Does 'check-git-log.sh' run automatically?

> if DPDK built using gcc:
> 
> include/generic/rte_atomic.h:215:9: error:
> implicit declaration of function '__atomic_exchange_2'
> is invalid in C99
> include/generic/rte_atomic.h:494:9: error:
> implicit declaration of function '__atomic_exchange_4'
> is invalid in C99
> include/generic/rte_atomic.h:772:9: error:
> implicit declaration of function '__atomic_exchange_8'
> is invalid in C99
> 
> We need to check for current compiler, not the compiler used for DPDK build.
> 
> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/librte_eal/common/include/generic/rte_atomic.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h
> b/lib/librte_eal/common/include/generic/rte_atomic.h
> index b99ba4688..d0c464fb1 100644
> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> @@ -212,7 +212,7 @@ rte_atomic16_exchange(volatile uint16_t *dst,
> uint16_t val);  static inline uint16_t  rte_atomic16_exchange(volatile 
> uint16_t
> *dst, uint16_t val)  { -#if defined(RTE_ARCH_ARM64) &&
> defined(RTE_TOOLCHAIN_CLANG)
> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>   return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>   return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST); @@ -
> 495,7 +495,7 @@ rte_atomic32_exchange(volatile uint32_t *dst, uint32_t
> val);  static inline uint32_t  rte_atomic32_exchange(volatile uint32_t *dst,
> uint32_t val)  { -#if defined(RTE_ARCH_ARM64) &&
> defined(RTE_TOOLCHAIN_CLANG)
> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>   return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>   return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST); @@ -
> 777,7 +777,7 @@ rte_atomic64_exchange(volatile uint64_t *dst, uint64_t
> val);  static inline uint64_t  rte_atomic64_exchange(volatile uint64_t *dst,
> uint64_t val)  { -#if defined(RTE_ARCH_ARM64) &&
> defined(RTE_TOOLCHAIN_CLANG)
> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>   return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>   return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
> --
> 2.17.1

Re: [dpdk-dev] [EXT] [PATCH v1] ticketlock: ticket based to improve fairness

[Gavin Hu (Arm Technology China)]

> -Original Message-
> From: Jerin Jacob Kollanukkaran 
> Sent: Monday, January 14, 2019 4:00 PM
> To: Gavin Hu (Arm Technology China) ;
> dev@dpdk.org
> Cc: step...@networkplumber.org; Honnappa Nagarahalli
> ; tho...@monjalon.net; nd
> ; Joyce Kong (Arm Technology China)
> ; hemant.agra...@nxp.com
> Subject: Re: [EXT] [PATCH v1] ticketlock: ticket based to improve fairness
> 
> On Sun, 2019-01-13 at 22:46 +0800, Gavin Hu wrote:
> > ---
> > ---
> > From: Joyce Kong 
> >
> > The spinlock implementation is unfair, some threads may take locks
> > aggressively while leaving the other threads starving for long time.
> > As
> > shown in the following test, within same period of time, there are
> > threads taking locks much more times than the others.
> >
> > The ticketlock gives each waiting thread a ticket and they can take
> > the
> > lock one by one, first come, first serviced, this avoids starvation
> > for
> > too long time and is more predictable.
> >
> > *** spinlock_autotest without this patch ***
> > Core [0] count = 89
> > Core [1] count = 84
> > Core [2] count = 94
> > ...
> > Core [208] count = 171
> > Core [209] count = 152
> > Core [210] count = 161
> > Core [211] count = 187
> >
> > *** spinlock_autotest with this patch ***
> 
> This comment needs to be updated as it is new API now
> and we need add test case for the same. I think,
> making modification to exiting spinlock test case would be fine.

Thanks for you comments, we will do it and send a v2.

Re: [dpdk-dev] [EXT] [PATCH v3 5/6] spinlock: reimplement with atomic one-way barrier builtins

[Gavin Hu (Arm Technology China)]

> > > > > *sl);  static
> > > > > inline void  rte_spinlock_lock(rte_spinlock_t *sl)  {
> > > > > - while (__sync_lock_test_and_set(&sl->locked, 1))
> > > > > - while(sl->locked)
> > > > > + int exp = 0;
> > > > > +
> > > > > + while (!__atomic_compare_exchange_n(&sl->locked, &exp,
> > > > > 1, 0,
> > > > > + __ATOMIC_ACQUIRE,
> > > > > __ATOMIC_RELAXED))
> > > > {
> > > >
> > > > How about remove explict exp = 0 and change to
> > > > __atomic_test_and_set(flag, __ATOMIC_ACQUIRE);
> > >
> > > Yes, __atomic_test_and_set means simpler code and better, but
> > > __atomic_test_and_set takes the first argument as a pointer to type
> > > bool or
> > > char, in our case, sl->locked is of type uint32.
> > > We can force it to uint8, or just pass in the 32bit pointer, only
> > > one byte/bit is
> > > really used in this case, is that ok?
> > >
> > > "It should be only used for operands of type bool or char. For
> > > other types only
> > > part of the value may be set."
> > > https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/_005f_005fatomic-
> > > Builtins.html
> > >
> > > From performance perspective, in our testing, the performance was
> > > very close,
> > > compared to __atomic.
> > If performance is close, I suggest we go with the existing patch.
> > Changing sl->locked to bool/char would be an ABI change and will
> > affect x86 TM based implementation as well.
> > Jerin, what do you think?
> 
> Looks good to me.
> 
I tested __atomic_test_and_test based patch(I did not push this, it is in 
internal review), it caused 10 times performance degradation on ThunderX2.
In the internal review, Ola Liljedahl's comment well explained the degradation:
"Unlike the old code, the new code will write the lock location (and thus get 
exclusive access to the whole cache line) repeatedly while it is busy waiting. 
This is bad for the lock owner if it is accessing other data located in the 
same cache line as the lock (which is often the case)."
The real difference is __atomic_test_and_test keeps writing the lock 
location(whether it is locked or not) and monopolizing the cache line, it is 
bad to the lock owner and other contenders. 

Re: [dpdk-dev] TX unable to enqueue packets to NIC due to no free TX descriptor

[Soni, Shivam]

I doubled the mempool size to 65535 but the issue is not resolved.

On 1/11/19, 4:27 PM, "dev on behalf of Soni, Shivam"  wrote:

Hi Stephen,

Thanks for the reply.

Our mbuf pool is big enough. We have 2 RX cores, 2 TX cores and 8 worker 
cores.
NTxd and NRxd is 1024 each and we have 16 Rx rings (shared between Rx and 
workers) and 8 Tx rings (between Tx and workers)
Mempool cache size is 256 and burst size is 32.

So overall calculation comes out to be 
((NIC_RX_QUEUE_SIZE * RX_LCORES) + (NIC_TX_QUEUE_SIZE * TX_LCORES) + \
  (WORKER_RX_RING_SIZE * RX_LCORES * 
NAT_WORKER_LCORES) + (WORKER_TX_RING_SIZE * NAT_WORKER_LCORES) + \
  ((MBUF_ARRAY_SIZE + CACHE_SIZE) * (RX_LCORES + 
TX_LCORES + NAT_WORKER_LCORES)))

With this the  mbuf pool size should be 32128. To round off as power of 2 
we have kept mbuf pool size as 32767.

Also the incoming packet rate Is pretty low.

For testing I have doubled the pool size for now. Not sure whether this 
will solve the issue.

Thanks.

On 1/11/19, 3:38 PM, "Stephen Hemminger"  wrote:

On Fri, 11 Jan 2019 22:10:39 +
"Soni, Shivam"  wrote:

> Hi All,
> 
> We are trying to debug and fix an issue. After the deployment, in few 
of the hosts we see an issue where TX is unable to enqueue packets to NIC. On 
rebouncing or restarting our packet processor daemon, issue gets resolved.
> 
> We are using IntelDPDK version 17.11.4 and i40e drivers.
> 
> On looking into driver’s code, we found that whenever the issue is 
happening the value for nb_tx_free is ‘0’. And then it tries to free the buffer 
by calling function ‘i40e_tx_free_bufs’.
> 
> This method returns early as the buffer its trying to free says it 
hasn’t finished transmitting yet. The method returns at this if condition:
> 
> /* check DD bits on threshold descriptor */
> if ((txq->tx_ring[txq->tx_next_dd].cmd_type_offset_bsz &
> rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) !=
> rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE)) {
> return 0;
> }
> 
> Hence nb_tx_free remains 0.
> 
> Our tx descriptor count is 1024.
> 
> How can we fix this issue.  Can someone help us out here please

Use bigger mbuf pool.  For safety the mbuf pool has to be big enough
for Nports * (NRxd + NTxd) + NCore * (mbuf_pool_cache_size + burst_size)

Each NIC might get full receive ring and full transmit ring
and each active core might be processing a burst of packets and have
free buffers sitting in the mbuf pool cache. This doesn't account for 
additional
mbuf's created if doing things like reassembly, encryption, 
re-encapsulation, or compression

Anything smaller and your application is relying on statistical averages
to never see resource exhaustion; overcommitment

Re: [dpdk-dev] [PATCH] net/nfp: add CPP bridge as service

[Alejandro Lucero]

On Mon, Jan 14, 2019 at 10:40 AM Ferruh Yigit 
wrote:

> On 1/13/2019 9:41 PM, Thomas Monjalon wrote:
> > 11/01/2019 17:42, Ferruh Yigit:
> >> On 1/11/2019 1:25 PM, Alejandro Lucero wrote:
> >>> The Netronome's Network Flow Processor chip is highly programmable
> >>> with the goal of processing packets at high speed. Processing units
> >>> and other chip components are available from the host through the
> >>> PCIe CPP(Command Push Pull bus) interface. The NFP PF PMD configures
> >>> a CPP handler for setting up and working with vNICs, perform actions
> >>> like link up or down, or accessing extended stats from the MAC
> component.
> >>>
> >>> There exist NFP host tools which access the NFP components for
> >>> programming and debugging but they require the CPP interface. When the
> >>> PMD is bound to the PF, the DPDK app owns the CPP interface, so these
> >>> host tools can not access the NFP through other means like NFP kernel
> >>> drivers.
> >>>
> >>> This patch adds a CPP bridge using the rte_service API which can be
> >>> enabled by a DPDK app. Interestingly, DPDK clients like OVS will not
> >>> enable specific service cores, but this can be performed with a
> >>> secondary process specifically enabling this CPP bridge service and
> >>> therefore giving access to the NFP to those host tools.
> >>>
> >>> v2:
> >>>  - Avoid printfs for debugging
> >>>  - fix compilation problems for powerpc
> >>>
> >>> Signed-off-by: Alejandro Lucero 
> >>
> >> Applied to dpdk-next-net/master, thanks.
> >
> > It does not compile with 32-bit toolchain.
> >
> > Please check the occurences of %lu, thanks.
>
> Hi Thomas,
>
> We aware the build error, but let it because nfp doesn't support 32-bit.
>
> But I just recognized that it is enabled by default on 32-bit default
> configs,
> we should disable them.
>
>
> Hi Alejandro,
>
> Can you please disable nfp driver explicitly on
> 'defconfig_i686-native-linuxapp-*' config files, perhaps also on
> 'defconfig_x86_x32-native-linuxapp-gcc' too?
>
> I will drop the existing patch from next-net.
>
>
Ok. I'll do asap.


> And if it is possible to fix the build error, specially if it is just for
> %lu of
> the logging, I prefer the fix against the config update, but it is up to
> you.
>
>
I did not see any logging error/warning when compiling nor any when using
checkpatch. I have used a gcc 7.3.1 (Ubuntu) and a 8.2.1 (RH). What are you
using for triggering such error?


> Thanks,
> ferruh
>

[dpdk-dev] [PATCH] config: remove NFP PMD from 32-bits builds

[Alejandro Lucero]

Signed-off-by: Alejandro Lucero 
---
 config/defconfig_i686-native-linuxapp-gcc| 5 +
 config/defconfig_i686-native-linuxapp-icc| 5 +
 config/defconfig_x86_x32-native-linuxapp-gcc | 5 +
 3 files changed, 15 insertions(+)

diff --git a/config/defconfig_i686-native-linuxapp-gcc 
b/config/defconfig_i686-native-linuxapp-gcc
index 1178fe35b..9b7086859 100644
--- a/config/defconfig_i686-native-linuxapp-gcc
+++ b/config/defconfig_i686-native-linuxapp-gcc
@@ -47,5 +47,10 @@ CONFIG_RTE_LIBRTE_PMD_ZUC=n
 #
 CONFIG_RTE_LIBRTE_AVP_PMD=n
 
+#
+# NFP PMD is not supported on 32-bit
+#
+CONFIG_RTE_LIBRTE_NFP_PMD=n
+
 # 32-bit doesn't break up memory in lists, but does have VA allocation limit
 CONFIG_RTE_MAX_MEM_MB=2048
diff --git a/config/defconfig_i686-native-linuxapp-icc 
b/config/defconfig_i686-native-linuxapp-icc
index 016c73f30..17defd803 100644
--- a/config/defconfig_i686-native-linuxapp-icc
+++ b/config/defconfig_i686-native-linuxapp-icc
@@ -47,5 +47,10 @@ CONFIG_RTE_LIBRTE_PMD_ZUC=n
 #
 CONFIG_RTE_LIBRTE_AVP_PMD=n
 
+#
+# NFP PMD is not supported on 32-bit
+#
+CONFIG_RTE_LIBRTE_NFP_PMD=n
+
 # 32-bit doesn't break up memory in lists, but does have VA allocation limit
 CONFIG_RTE_MAX_MEM_MB=2048
diff --git a/config/defconfig_x86_x32-native-linuxapp-gcc 
b/config/defconfig_x86_x32-native-linuxapp-gcc
index 57d000dc5..2c8419f14 100644
--- a/config/defconfig_x86_x32-native-linuxapp-gcc
+++ b/config/defconfig_x86_x32-native-linuxapp-gcc
@@ -27,5 +27,10 @@ CONFIG_RTE_LIBRTE_SFC_EFX_PMD=n
 #
 CONFIG_RTE_LIBRTE_AVP_PMD=n
 
+#
+# NFP PMD is not supported on 32-bit
+#
+CONFIG_RTE_LIBRTE_NFP_PMD=n
+
 # 32-bit doesn't break up memory in lists, but does have VA allocation limit
 CONFIG_RTE_MAX_MEM_MB=2048
-- 
2.17.1