Re: [dpdk-dev] [dpdk-stable] [PATCH 1/4] common/mlx5: share UAR allocation routine
10/11/2020 17:04, Viacheslav Ovsiienko:
> This patch introduces the routine to allocate the UAR (User
> Access Region) with various memory mapping types. The origin
> patch being fixed provided the UAR allocation workaround
> for the mlx5 net PMD only. As it was found the other mlx5
> based drivers - vdpa and regex are affected by the issue
> as well and must be fixed.
>
> Fixes: a0bfe9d56f74 ("net/mlx5: fix UAR memory mapping type")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Viacheslav Ovsiienko
> Acked-by: Matan Azrad
Series applied, thanks
Re: [dpdk-dev] [PATCH v2] gro: fix an error when packet is IPv6
> > For VxLAN packets, GRO will mistakenly reassemble them
> > if inner L3 is IPv6, inner L4 is TCP or UDP, and outer L3
> > is IPv4 because the value of IS_IPV4_VXLAN_TCP4/UDP4_PKT
> > is true for them.
> >
> > This fix makes sure IS_IPV4_TCP_PKT, IS_IPV4_UDP_PKT,
> > IS_IPV4_VXLAN_TCP4_PKT and IS_IPV4_VXLAN_UDP4_PKT can make
> > decision precisely.
> >
> > Fixes: e2d811063673 ("gro: support VXLAN UDP/IPv4")
> > Fixes: 1ca5e6740852 ("gro: support UDP/IPv4")
> > Fixes: 9e0b9d2ec0f4 ("gro: support VxLAN GRO")
> > Fixes: 0d2cbe59b719 ("lib/gro: support TCP/IPv4")
> > Signed-off-by: Yi Yang
> Acked-by: Jiayu Hu
Added missing Cc: sta...@dpdk.org
Tried to be more precise in the title:
"gro: fix packet type detection with IPv6 tunnel"
Applied, thanks
Re: [dpdk-dev] [dpdk-stable] [PATCH] config: enable packet prefetching with Meson
13/11/2020 16:05, Bruce Richardson:
> On Fri, Nov 13, 2020 at 03:52:12PM +0100, Maxime Coquelin wrote:
> > With Make build system, RTE_PMD_PACKET_PREFETCH was enabled
> > by default. It got lost when transitioning to Meson build
> > system.
> >
> > In order to avoid performance changes, this patch enables
> > packet prefetching in rte_config.h.
> >
> > Reported-by: Marvin Liu
> > Suggested-by: David Marchand
> > Signed-off-by: Maxime Coquelin
> > ---
> >
> > Hi Bruce,
> >
> > We were not sure whether adding below Fixes tag so that it is
> > backported to LTSes. What do you think?
> >
> > Fixes: 9314afb68a53 ("drivers: add infrastructure for meson build")
> > Cc: sta...@dpdk.org
> >
> > config/rte_config.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> I view it as a gap in the transition from make to meson, so +1 for
> adding these. Stable maintainers can then decide themselves on whether to
> accept the patch or not.
Applied with backport info, thanks.
Re: [dpdk-dev] [dpdk-stable] [PATCH] config: enable packet prefetching with Meson
14/11/2020 09:51, Thomas Monjalon: > 13/11/2020 16:05, Bruce Richardson: > > On Fri, Nov 13, 2020 at 03:52:12PM +0100, Maxime Coquelin wrote: > > > With Make build system, RTE_PMD_PACKET_PREFETCH was enabled > > > by default. It got lost when transitioning to Meson build > > > system. > > > > > > In order to avoid performance changes, this patch enables > > > packet prefetching in rte_config.h. > > > > > > Reported-by: Marvin Liu > > > Suggested-by: David Marchand > > > Signed-off-by: Maxime Coquelin [...] > > Applied with backport info, thanks. Note that it is added back for "compatibility", but I am still in favour of dropping this config option, replaced by arch decision if any: http://inbox.dpdk.org/dev/3677226.MZCibFMyqQ@thomas/ The decision of such optimization should be done in DPDK project, not in the hand of the packager.
[dpdk-dev] [PATCH v3 00/11] Examples compilation fixes
I tested external compilation for all possible examples (in my env). The result is this series. -- David Marchand Changelog since v2: - reworked patch 4 check on crypto driver availability, - added patch 5 to fix misalignment with other examples makefiles, - dropped last patch on checking all examples, Changelog since v1: - added patch 7 to fix performance-thread example, - reworked last patch by introducing a new -Dexamples=buildall special value and removed exceptions for examples recently fixed in main thanks to Bruce, David Marchand (11): examples/fips_validation: fix build with pkg-config examples/ipsec-gw: fix build with pkg-config examples/kni: fix build with pkg-config examples/l2fwd-crypto: fix build with pkg-config examples/l3fwd-graph: fix static build examples/l3fwd-graph: fix pkg-config usage examples/ntb: fix clean target examples/performance-thread: fix build with pkg-config examples/vhost_blk: fix build with pkg-config examples/rxtx_callbacks: fix build with pkg-config examples: restore trace point examples/cmdline/Makefile | 2 ++ examples/distributor/Makefile | 2 ++ examples/ethtool/ethtool-app/Makefile | 2 ++ examples/eventdev_pipeline/Makefile| 2 ++ examples/fips_validation/Makefile | 2 ++ examples/flow_filtering/Makefile | 2 ++ examples/helloworld/Makefile | 2 ++ examples/ioat/Makefile | 2 ++ examples/ip_reassembly/Makefile| 2 ++ examples/ipsec-secgw/Makefile | 1 + examples/ipv4_multicast/Makefile | 2 ++ examples/kni/Makefile | 2 ++ examples/l2fwd-cat/Makefile| 2 ++ examples/l2fwd-crypto/Makefile | 6 ++ examples/l2fwd-event/Makefile | 2 ++ examples/l2fwd-jobstats/Makefile | 2 ++ examples/l2fwd-keepalive/Makefile | 2 ++ examples/l2fwd-keepalive/ka-agent/Makefile | 2 ++ examples/l3fwd-acl/Makefile| 2 ++ examples/l3fwd-graph/Makefile | 6 +++--- examples/link_status_interrupt/Makefile| 2 ++ examples/multi_process/client_server_mp/mp_client/Makefile | 2 ++ examples/multi_process/client_server_mp/mp_server/Makefile | 2 ++ examples/multi_process/hotplug_mp/Makefile | 2 ++ examples/multi_process/simple_mp/Makefile | 2 ++ examples/multi_process/symmetric_mp/Makefile | 2 ++ examples/ntb/Makefile | 2 +- examples/packet_ordering/Makefile | 2 ++ examples/performance-thread/l3fwd-thread/main.c| 5 + examples/ptpclient/Makefile| 2 ++ examples/qos_sched/Makefile| 2 ++ examples/rxtx_callbacks/Makefile | 2 ++ examples/server_node_efd/node/Makefile | 2 ++ examples/server_node_efd/server/Makefile | 2 ++ examples/service_cores/Makefile| 2 ++ examples/skeleton/Makefile | 2 ++ examples/timer/Makefile| 2 ++ examples/vhost_blk/vhost_blk.c | 6 ++ examples/vm_power_manager/Makefile | 2 ++ examples/vm_power_manager/guest_cli/Makefile | 2 ++ examples/vmdq/Makefile | 2 ++ examples/vmdq_dcb/Makefile | 2 ++ 42 files changed, 94 insertions(+), 4 deletions(-) -- 2.23.0
[dpdk-dev] [PATCH v3 02/11] examples/ipsec-gw: fix build with pkg-config
flow.c: In function ‘parse_flow_tokens’:
flow.c:153:23: error: taking address of packed member of ‘struct
rte_ipv4_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
153 | if (ipv4_addr_cpy(&rule->ipv4.spec.hdr.src_addr,
| ^
flow.c:154:9: error: taking address of packed member of ‘struct
rte_ipv4_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
154 | &rule->ipv4.mask.hdr.src_addr,
| ^
flow.c:170:23: error: taking address of packed member of ‘struct
rte_ipv4_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
170 | if (ipv4_addr_cpy(&rule->ipv4.spec.hdr.dst_addr,
| ^
flow.c:171:9: error: taking address of packed member of ‘struct
rte_ipv4_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
171 | &rule->ipv4.mask.hdr.dst_addr,
| ^
cc1: all warnings being treated as errors
Meson build is fine since we waive those warnings.
Replicate it for make.
Fixes: 8e693616fcb2 ("examples/ipsec-secgw: enable flow based distribution")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
examples/ipsec-secgw/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/ipsec-secgw/Makefile b/examples/ipsec-secgw/Makefile
index 5e6cb51eac..7670cc3684 100644
--- a/examples/ipsec-secgw/Makefile
+++ b/examples/ipsec-secgw/Makefile
@@ -42,6 +42,7 @@ LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
CFLAGS += -DALLOW_EXPERIMENTAL_API
+CFLAGS += -Wno-address-of-packed-member
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
--
2.23.0
[dpdk-dev] [PATCH v3 01/11] examples/fips_validation: fix build with pkg-config
When this example started using rte_cryptodev_sym_session_pool_create,
the part for pkg-config builds was not updated.
Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
examples/fips_validation/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/examples/fips_validation/Makefile
b/examples/fips_validation/Makefile
index 7ba9bcfea6..8f82a4c6c5 100644
--- a/examples/fips_validation/Makefile
+++ b/examples/fips_validation/Makefile
@@ -36,6 +36,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
--
2.23.0
[dpdk-dev] [PATCH v3 03/11] examples/kni: fix build with pkg-config
rm -f build/kni build/kni-static build/kni-shared
test -d build && rmdir -p build || true
[...]
/usr/bin/ld: /tmp/cc72ssnK.o: undefined reference to symbol
'pthread_join@@GLIBC_2.2.5'
This example explicitly call pthread API and should be linked against
the pthread library.
Fixes: 724beb913b44 ("examples/kni: monitor and update link state continually")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
examples/kni/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/examples/kni/Makefile b/examples/kni/Makefile
index fa9fa85319..bbf3bcae12 100644
--- a/examples/kni/Makefile
+++ b/examples/kni/Makefile
@@ -27,6 +27,8 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+LDFLAGS += -pthread
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
--
2.23.0
[dpdk-dev] [PATCH v3 05/11] examples/l3fwd-graph: fix static build
This example missed the rework from commit 8549295db07b
("build/pkg-config: improve static linking flags").
Fixes: 08bd1a174461 ("examples/l3fwd-graph: add graph-based l3fwd skeleton")
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
examples/l3fwd-graph/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l3fwd-graph/Makefile b/examples/l3fwd-graph/Makefile
index 368ac21147..1e4f0600ae 100644
--- a/examples/l3fwd-graph/Makefile
+++ b/examples/l3fwd-graph/Makefile
@@ -24,7 +24,7 @@ PKGCONF=pkg-config --define-prefix
PC_FILE := $(shell $(PKGCONF) --path libdpdk)
CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk) -DALLOW_EXPERIMENTAL_API
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
-LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
+LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
--
2.23.0
[dpdk-dev] [PATCH v3 04/11] examples/l2fwd-crypto: fix build with pkg-config
Two issues fixed here.
First add the experimental flag.
Then fix a link issue with the crypto scheduler driver:
/usr/bin/ld: /tmp/cchr7aHA.o: in function `main':
main.c:(.text.startup+0x1673): undefined reference to
`rte_cryptodev_scheduler_workers_get'
collect2: error: ld returned 1 exit status
Fixes: e3bcb99a5e13 ("examples/l2fwd-crypto: limit number of sessions")
Fixes: 261bbff75e34 ("examples: use separate crypto session mempools")
Signed-off-by: David Marchand
---
Changelog since v2:
- reworked config check,
---
examples/l2fwd-crypto/Makefile | 6 ++
1 file changed, 6 insertions(+)
diff --git a/examples/l2fwd-crypto/Makefile b/examples/l2fwd-crypto/Makefile
index 4953ee2b95..7731eccd03 100644
--- a/examples/l2fwd-crypto/Makefile
+++ b/examples/l2fwd-crypto/Makefile
@@ -26,6 +26,12 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+CONFIG_DEFINES = $(shell $(CC) $(CFLAGS) -dM -E - < /dev/null)
+ifneq ($(findstring RTE_CRYPTO_SCHEDULER,$(CONFIG_DEFINES)),)
+LDFLAGS_SHARED += -lrte_crypto_scheduler
+endif
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
--
2.23.0
[dpdk-dev] [PATCH v3 06/11] examples/l3fwd-graph: fix pkg-config usage
This example missed the fixes from commit 69b1bb49ed82
("examples: hide error for missing pkg-config path flag") and
commit 12a652a02b08 ("examples: fix build with old pkg-config").
Fixes: 08bd1a174461 ("examples/l3fwd-graph: add graph-based l3fwd skeleton")
Signed-off-by: David Marchand
---
examples/l3fwd-graph/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/l3fwd-graph/Makefile b/examples/l3fwd-graph/Makefile
index 1e4f0600ae..6e3d0bca06 100644
--- a/examples/l3fwd-graph/Makefile
+++ b/examples/l3fwd-graph/Makefile
@@ -19,9 +19,9 @@ shared: build/$(APP)-shared
static: build/$(APP)-static
ln -sf $(APP)-static build/$(APP)
-PKGCONF=pkg-config --define-prefix
+PKGCONF ?= pkg-config
-PC_FILE := $(shell $(PKGCONF) --path libdpdk)
+PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk) -DALLOW_EXPERIMENTAL_API
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
--
2.23.0
[dpdk-dev] [PATCH v3 08/11] examples/performance-thread: fix build with pkg-config
main.c: In function ‘lthread_tx’:
main.c:2091:25: error: implicit declaration of function ‘sched_getcpu’;
did you mean ‘sched_getparam’? [-Werror=implicit-function-declaration]
2091 | tx_conf->conf.cpu_id = sched_getcpu();
| ^~~~
| sched_getparam
cc1: all warnings being treated as errors
Explicitly pass _GNU_SOURCE and include missing header (rather than
rely on automagic inclusion from other system headers).
Fixes: d48415e1fee3 ("examples/performance-thread: add l3fwd-thread app")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
---
examples/performance-thread/l3fwd-thread/main.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/examples/performance-thread/l3fwd-thread/main.c
b/examples/performance-thread/l3fwd-thread/main.c
index 7bf61db6be..4d82fb82ef 100644
--- a/examples/performance-thread/l3fwd-thread/main.c
+++ b/examples/performance-thread/l3fwd-thread/main.c
@@ -2,6 +2,10 @@
* Copyright(c) 2010-2016 Intel Corporation
*/
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
#include
#include
#include
@@ -12,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
--
2.23.0
[dpdk-dev] [PATCH v3 07/11] examples/ntb: fix clean target
When introducing this example, the cleanup from commit 7e9562a107f1
("examples: fix make clean when using pkg-config") was missed.
Fixes: c5eebf85badc ("examples/ntb: add example for NTB")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
Acked-by: Xiaoyun Li
---
examples/ntb/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/ntb/Makefile b/examples/ntb/Makefile
index 4675570fd2..d35dabc471 100644
--- a/examples/ntb/Makefile
+++ b/examples/ntb/Makefile
@@ -42,4 +42,4 @@ build:
.PHONY: clean
clean:
rm -f build/$(APP) build/$(APP)-static build/$(APP)-shared
- rmdir --ignore-fail-on-non-empty build
+ test -d build && rmdir -p build || true
--
2.23.0
[dpdk-dev] [PATCH v3 09/11] examples/vhost_blk: fix build with pkg-config
vhost_blk.c: In function ‘ctrlr_worker’:
vhost_blk.c:543:2: warning: implicit declaration of function ‘CPU_ZERO’
[-Wimplicit-function-declaration]
543 | CPU_ZERO(&cpuset);
| ^~~~
vhost_blk.c:544:2: warning: implicit declaration of function ‘CPU_SET’
[-Wimplicit-function-declaration]
544 | CPU_SET(0, &cpuset);
| ^~~
vhost_blk.c:545:2: warning: implicit declaration of function
‘pthread_setaffinity_np’ [-Wimplicit-function-declaration]
545 | pthread_setaffinity_np(thread, sizeof(cpu_set_t), &cpuset);
| ^~
/usr/bin/ld: /tmp/cczpiMWH.o: in function `ctrlr_worker':
vhost_blk.c:(.text+0x1076): undefined reference to `CPU_ZERO'
/usr/bin/ld: vhost_blk.c:(.text+0x1082): undefined reference to
`CPU_SET'
collect2: error: ld returned 1 exit status
gmake: *** [Makefile:34: build/vhost-blk-shared] Error 1
Explicitly pass _GNU_SOURCE and include missing headers (rather than
rely on automagic inclusion from other system headers).
Fixes: c19beb3f38cd ("examples/vhost_blk: introduce vhost storage sample")
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
examples/vhost_blk/vhost_blk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/examples/vhost_blk/vhost_blk.c b/examples/vhost_blk/vhost_blk.c
index 8f5d61a589..bb293d492f 100644
--- a/examples/vhost_blk/vhost_blk.c
+++ b/examples/vhost_blk/vhost_blk.c
@@ -2,6 +2,12 @@
* Copyright(c) 2010-2019 Intel Corporation
*/
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include
+#include
+
#include
#include
#include
--
2.23.0
[dpdk-dev] [PATCH v3 11/11] examples: restore trace point
Before make removal, those examples were built with experimental flag
for tracepoints to be compiled in but the pkg-config part of those
makefiles were missed.
Fixes: 78d44153de8f ("ethdev: add tracepoints")
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
Changelog since v2:
- added eventdev_pipeline,
---
examples/cmdline/Makefile | 2 ++
examples/distributor/Makefile | 2 ++
examples/ethtool/ethtool-app/Makefile | 2 ++
examples/eventdev_pipeline/Makefile| 2 ++
examples/flow_filtering/Makefile | 2 ++
examples/helloworld/Makefile | 2 ++
examples/ioat/Makefile | 2 ++
examples/ip_reassembly/Makefile| 2 ++
examples/ipv4_multicast/Makefile | 2 ++
examples/l2fwd-cat/Makefile| 2 ++
examples/l2fwd-event/Makefile | 2 ++
examples/l2fwd-jobstats/Makefile | 2 ++
examples/l2fwd-keepalive/Makefile | 2 ++
examples/l2fwd-keepalive/ka-agent/Makefile | 2 ++
examples/l3fwd-acl/Makefile| 2 ++
examples/link_status_interrupt/Makefile| 2 ++
examples/multi_process/client_server_mp/mp_client/Makefile | 2 ++
examples/multi_process/client_server_mp/mp_server/Makefile | 2 ++
examples/multi_process/hotplug_mp/Makefile | 2 ++
examples/multi_process/simple_mp/Makefile | 2 ++
examples/multi_process/symmetric_mp/Makefile | 2 ++
examples/packet_ordering/Makefile | 2 ++
examples/ptpclient/Makefile| 2 ++
examples/qos_sched/Makefile| 2 ++
examples/server_node_efd/node/Makefile | 2 ++
examples/server_node_efd/server/Makefile | 2 ++
examples/service_cores/Makefile| 2 ++
examples/skeleton/Makefile | 2 ++
examples/timer/Makefile| 2 ++
examples/vm_power_manager/Makefile | 2 ++
examples/vm_power_manager/guest_cli/Makefile | 2 ++
examples/vmdq/Makefile | 2 ++
examples/vmdq_dcb/Makefile | 2 ++
33 files changed, 66 insertions(+)
diff --git a/examples/cmdline/Makefile b/examples/cmdline/Makefile
index d8f51fb39b..09da84ba0b 100644
--- a/examples/cmdline/Makefile
+++ b/examples/cmdline/Makefile
@@ -26,6 +26,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
diff --git a/examples/distributor/Makefile b/examples/distributor/Makefile
index 4116064581..d7615f9a32 100644
--- a/examples/distributor/Makefile
+++ b/examples/distributor/Makefile
@@ -26,6 +26,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
diff --git a/examples/ethtool/ethtool-app/Makefile
b/examples/ethtool/ethtool-app/Makefile
index 5ebeb200a8..93ef5c27c3 100644
--- a/examples/ethtool/ethtool-app/Makefile
+++ b/examples/ethtool/ethtool-app/Makefile
@@ -31,6 +31,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED += $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC += $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
diff --git a/examples/eventdev_pipeline/Makefile
b/examples/eventdev_pipeline/Makefile
index 05c6f865ad..f5072a2b0c 100644
--- a/examples/eventdev_pipeline/Makefile
+++ b/examples/eventdev_pipeline/Makefile
@@ -28,6 +28,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
diff --git a/examples/flow_filtering/Makefile b/examples/flow_filtering/Makefile
index 0577e52985..9bc9179346 100644
--- a/examples/flow_filtering/Makefile
+++ b/examples/flow_filtering/Makefile
@@ -24,6 +24,8 @@ CFLAGS += -O3 $(s
[dpdk-dev] [PATCH v3 10/11] examples/rxtx_callbacks: fix build with pkg-config
This example is missing the experimental flag since it uses an
experimental API.
Fixes: cd1dadeb9b2a ("examples/rxtx_callbacks: support HW timestamp")
Cc: sta...@dpdk.org
Signed-off-by: David Marchand
Acked-by: Bruce Richardson
---
examples/rxtx_callbacks/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/examples/rxtx_callbacks/Makefile b/examples/rxtx_callbacks/Makefile
index bac3015d34..a618cdf751 100644
--- a/examples/rxtx_callbacks/Makefile
+++ b/examples/rxtx_callbacks/Makefile
@@ -26,6 +26,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
--
2.23.0
Re: [dpdk-dev] [dpdk-stable] [PATCH 1/4] common/mlx5: share UAR allocation routine
10/11/2020 17:04, Viacheslav Ovsiienko:
> +void *
> +mlx5_devx_alloc_uar(void *ctx, int mapping)
> +{
> + void *uar;
> + uint32_t retry, uar_mapping;
> + void *base_addr;
> +
> + for (retry = 0; retry < MLX5_ALLOC_UAR_RETRY; ++retry) {
> +#ifdef MLX5DV_UAR_ALLOC_TYPE_NC
> + /* Control the mapping type according to the settings. */
> + uar_mapping = (mapping < 0) ?
> + MLX5DV_UAR_ALLOC_TYPE_NC : mapping;
> +#else
> + /*
> + * It seems we have no way to control the memory mapping type
> + * for the UAR, the default "Write-Combining" type is supposed.
> + */
> + uar_mapping = 0;
> +#endif
A failure was reported by the CI:
error: unused parameter ‘mapping’
It is fixed while merging by adding the below in the #else case:
RTE_SET_USED(mapping);
Please take care of CI results!
Re: [dpdk-dev] [PATCH] doc/rel_notes: remove old deprecation for meson version
On Fri, Nov 13, 2020 at 5:17 PM Bruce Richardson wrote: > > DPDK has been using meson 0.47 for some time now, so we can safely > remove the note calling out this fact. > > Signed-off-by: Bruce Richardson > --- > doc/guides/rel_notes/deprecation.rst | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > index efb09f0c5..e04cab7a1 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -19,12 +19,6 @@ Deprecation Notices >``NET``, ``CRYPTO``, ``VDPA``. The old macros are deprecated and will be >removed in a future release. > > -* meson: The minimum supported version of meson for configuring and building > - DPDK will be increased to v0.47.1 (from 0.41) from DPDK 19.05 onwards. For > - those users with a version earlier than 0.47.1, an updated copy of meson > - can be got using the ``pip3`` tool (or ``python3 -m pip``) for downloading > - python packages. > - > * kvargs: The function ``rte_kvargs_process`` will get a new parameter >for returning key match count. It will ease handling of no-match case. > Acked-by: David Marchand -- David Marchand
Re: [dpdk-dev] [dpdk-stable] [PATCH] config: enable packet prefetching with Meson
On Sat, Nov 14, 2020 at 10:00 AM Thomas Monjalon wrote:
>
> 14/11/2020 09:51, Thomas Monjalon:
> > 13/11/2020 16:05, Bruce Richardson:
> > > On Fri, Nov 13, 2020 at 03:52:12PM +0100, Maxime Coquelin wrote:
> > > > With Make build system, RTE_PMD_PACKET_PREFETCH was enabled
> > > > by default. It got lost when transitioning to Meson build
> > > > system.
> > > >
> > > > In order to avoid performance changes, this patch enables
> > > > packet prefetching in rte_config.h.
> > > >
> > > > Reported-by: Marvin Liu
> > > > Suggested-by: David Marchand
> > > > Signed-off-by: Maxime Coquelin
> [...]
> >
> > Applied with backport info, thanks.
>
> Note that it is added back for "compatibility",
> but I am still in favour of dropping this config option,
> replaced by arch decision if any:
> http://inbox.dpdk.org/dev/3677226.MZCibFMyqQ@thomas/
>
> The decision of such optimization should be done in DPDK project,
> not in the hand of the packager.
I am for dropping this too.
And for cleaning more prefetch-related stuff, like:
#if 1
#define RTE_PMD_USE_PREFETCH
#endif
#ifdef RTE_PMD_USE_PREFETCH
#define rte_em_prefetch(p) rte_prefetch0(p)
#else
#define rte_em_prefetch(p) do {} while(0)
#endif
This has been copied into other drivers.
The igc driver forgot(?) to force #define this macro, so it just
copied unused code.
--
David Marchand
Re: [dpdk-dev] [PATCH 5/7] examples/qos_sched: enhance getopt_long usage
On Wed, Nov 11, 2020 at 9:17 AM Ibtisam Tariq wrote: > > Instead of using getopt_long return value, strcmp was used to > compare the input parameters with the struct option array. This > patch get rid of all those strcmp by directly binding each longopt > with an int enum. This is to improve readability and consistency in > all examples. > > Bugzilla ID: 238 > Cc: step...@networkplumber.org > > Reported-by: David Marchand > Signed-off-by: Ibtisam Tariq This patch breaks compilation with clang: https://travis-ci.com/github/ovsrobot/dpdk/jobs/433053506#L1019 You can remove this str_is helper. -- David Marchand
[dpdk-dev] [PATCH v2] app/testpmd: fix testpmd packets dump overlapping
When testpmd enabled the verbosity for the received packets, if two packets
was received at the same time, for example, sampling packet and normal
packet, the dump output of these packets may be overlapping due to multiple
core handled the multiple queues simultaneously.
The patch uses one string buffer that collects all the packet dump output
into this buffer and then printout it at last, that guarantee to printout
separately the dump output per packet.
Fixes: d862c45 ("app/testpmd: move dumping packets to a separate function")
Signed-off-by: Jiawei Wang
---
v2:
* Print dump output of per packet instead of per burst.
* Add the checking for return value of 'snprintf' and exit if required size
exceed the print buffer size.
* Update the log messages.
---
app/test-pmd/util.c | 378
1 file changed, 295 insertions(+), 83 deletions(-)
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 649bf8f..ffae590 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -12,15 +12,20 @@
#include
#include
#include
+#include
#include "testpmd.h"
-static inline void
-print_ether_addr(const char *what, const struct rte_ether_addr *eth_addr)
+#define MAX_STRING_LEN 8192
+
+static inline int
+print_ether_addr(const char *what, const struct rte_ether_addr *eth_addr,
+char print_buf[], int buf_size)
{
char buf[RTE_ETHER_ADDR_FMT_SIZE];
+
rte_ether_format_addr(buf, RTE_ETHER_ADDR_FMT_SIZE, eth_addr);
- printf("%s%s", what, buf);
+ return snprintf(print_buf, buf_size, "%s%s", what, buf);
}
static inline bool
@@ -74,13 +79,18 @@
uint32_t vx_vni;
const char *reason;
int dynf_index;
+ int buf_size = MAX_STRING_LEN;
+ char print_buf[buf_size];
+ int cur_len = 0;
+ memset(print_buf, 0, sizeof(print_buf));
if (!nb_pkts)
return;
- printf("port %u/queue %u: %s %u packets\n",
- port_id, queue,
- is_rx ? "received" : "sent",
- (unsigned int) nb_pkts);
+ cur_len += snprintf(print_buf + cur_len, buf_size - cur_len,
+ "port %u/queue %u: %s %u packets\n",
+ port_id, queue,
+ is_rx ? "received" : "sent",
+ (unsigned int) nb_pkts);
for (i = 0; i < nb_pkts; i++) {
int ret;
struct rte_flow_error error;
@@ -93,95 +103,269 @@
is_encapsulation = RTE_ETH_IS_TUNNEL_PKT(packet_type);
ret = rte_flow_get_restore_info(port_id, mb, &info, &error);
if (!ret) {
- printf("restore info:");
+ cur_len += snprintf(print_buf + cur_len,
+ buf_size - cur_len,
+ "restore info:");
+ if (cur_len >= buf_size)
+ goto error;
if (info.flags & RTE_FLOW_RESTORE_INFO_TUNNEL) {
struct port_flow_tunnel *port_tunnel;
port_tunnel = port_flow_locate_tunnel
(port_id, &info.tunnel);
- printf(" - tunnel");
- if (port_tunnel)
- printf(" #%u", port_tunnel->id);
- else
- printf(" %s", "-none-");
- printf(" type %s",
- port_flow_tunnel_type(&info.tunnel));
+ cur_len += snprintf(print_buf + cur_len,
+ buf_size - cur_len,
+ " - tunnel");
+ if (cur_len >= buf_size)
+ goto error;
+ if (port_tunnel) {
+ cur_len += snprintf(print_buf + cur_len,
+ buf_size-cur_len,
+ " #%u",
+ port_tunnel->id);
+ if (cur_len >= buf_size)
+ goto error;
+ } else {
+ cur_len += snprintf(print_buf + cur_len,
+ buf_size - cur_len,
+ " %s",
+ "-none-");
+ if (cur_len >= buf_size)
+ goto error;
+
Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors
13/11/2020 15:52, Gregory Etelson: > + ret = mlx5_flow_group_to_table(dev, tunnel, jump_data->group, > + &flow_table, grp_info, error); The parameter grp_info is a struct passed as value. I believe it should be passed as a pointer. I see some other functions are passing arrays with [] syntax, which does not make sense in parameters. I should be a simple pointer. Matan, Slava, as maintainers, what are your policies in mlx5 code?
Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection
13/11/2020 15:31, Juraj Linkeš:
> Add an option to automatically discover the host's numa and cpu counts
> and use those values for a non cross-build.
> Give users the option to override the per-arch default values or values
> from cross files by specifying them on the command line with -Dmax_lcores
> and -Dmax_numa_nodes.
>
> Signed-off-by: Juraj Linkeš
> Reviewed-by: Honnappa Nagarahalli
[...]
> create mode 100644 buildtools/get_cpu_count.py
> create mode 100644 buildtools/get_numa_count.py
These new files should be added in the file MAINTAINERS.
The recommended pattern is to use - as word separator in script filenames.
I'm also worried there is no more review on such general change,
especially the description in meson_options.txt:
> +option('max_lcores', type: 'integer', value: 0,
> + description: 'maximum number of cores/threads supported by EAL. Set
> to positive integer to overwrite per-arch or cross-compilation defaults. Set
> to -1 to detect the number of cores on the build machine.')
> +option('max_numa_nodes', type: 'integer', value: 0,
> + description: 'maximum number of NUMA nodes supported by EAL. Set to
> positive integer to overwrite per-arch or cross-compilation defaults. Set to
> -1 to detect the number of numa
I didn't think this series is changing options for all archs.
It was supposed to be an Arm-only rework.
Clearly it is too late for such change in 20.11.
Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors
> 13/11/2020 15:52, Gregory Etelson:
> > + ret = mlx5_flow_group_to_table(dev, tunnel, jump_data->group,
> > + &flow_table, grp_info, error);
>
> The parameter grp_info is a struct passed as value.
> I believe it should be passed as a pointer.
struct flow_grp_info is a 64 bit-field:
struct flow_grp_info {
uint64_t external:1;
uint64_t transfer:1;
uint64_t fdb_def_rule:1;
/* force standard group translation */
uint64_t std_tbl_fix:1;
uint64_t skip_scale:1;
};
Since mlx5_flow_group_to_table() does not change bits configuration,
there is no need to pass this type as a pointer.
>
> I see some other functions are passing arrays with [] syntax, which does
> not make sense in parameters. I should be a simple pointer.
>
I agree that arrays should not be passed with [] syntax.
However, I kept existing rte flow style - see rte_flow_create
> Matan, Slava, as maintainers, what are your policies in mlx5 code?
>
Re: [dpdk-dev] [PATCH v2 1/5] net/mlx5: fix tunnel offload object allocation
> 13/11/2020 15:52, Gregory Etelson:
> > The original patch allocated tunnel offload objects with invalid
> > indexes. As the result, PMD tunnel object allocation failed.
> >
> > In this patch indexed pool provides both an index and memory for a new
> > tunnel offload object.
> > Also tunnel offload ipool moved to dv enabled code only.
> >
> > Fixes: f2e8093 ("net/mlx5: use indexed pool as id generator")
> >
> > Signed-off-by: Gregory Etelson 2
>
> Typo above
>
> > Acked-by: Viacheslav Ovsiienko
>
> This patch is breaking compilation in non-DV mode.
> You should move it at the end of this series, after moving all in DV
> areas.
Patch "[PATCH v2 5/5] net/mlx5: fix non-dv compilation errors" in that series
resolves the compilation error.
To restore tunnel offload functionality all 5 patches must be applied.
Re: [dpdk-dev] [EXT] [PATCH v12 14/14] config: fix Arm implementer and its SoCs
Reviewed-by: Liron Himi
-Original Message-
From: Juraj Linkeš
Sent: Friday, 13 November 2020 16:31
To: bruce.richard...@intel.com; ruifeng.w...@arm.com;
honnappa.nagaraha...@arm.com; phil.y...@arm.com; vcchu...@amazon.com;
dharmik.thak...@arm.com; jerinjac...@gmail.com; hemant.agra...@nxp.com;
ajit.khapa...@broadcom.com; ferruh.yi...@intel.com
Cc: dev@dpdk.org; Juraj Linkeš ; Liron Himi
; ys...@mellanox.com
Subject: [EXT] [PATCH v12 14/14] config: fix Arm implementer and its SoCs
External Email
--
Fix the implementer and part number of DPAA and ARMADA SoCs.
The current values of 16 cores and 1 NUMA node don't cover all SoCs from the
Arm implementer, e.g. Taishan 2280 has 64 cores and 4 NUMA nodes.
Increase these to 64 and 4 to widen the coverage.
Add configuration to SoC options where smaller values are needed.
Fixes: 6ec78c2463ac ("build: add meson support for dpaaX platforms")
Cc: hemant.agra...@nxp.com
Fixes: dd1cd845c102 ("config: add Marvell ARMADA based on armv8-a")
Cc: lir...@marvell.com
Fixes: d97108a33231 ("config: change defaults of armv8")
Cc: ys...@mellanox.com
Signed-off-by: Juraj Linkeš
Reviewed-by: Honnappa Nagarahalli
---
config/arm/meson.build | 54 ++
1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/config/arm/meson.build b/config/arm/meson.build index
1738e36c3..f25a7dbab 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -75,8 +75,8 @@ implementer_arm = {
['RTE_MACHINE', '"armv8a"'],
['RTE_USE_C11_MEM_MODEL', true],
['RTE_CACHE_LINE_SIZE', 64],
- ['RTE_MAX_LCORE', 16],
- ['RTE_MAX_NUMA_NODES', 1]
+ ['RTE_MAX_LCORE', 64],
+ ['RTE_MAX_NUMA_NODES', 4]
],
'part_number_config': part_number_config_arm } @@ -146,38 +146,12 @@
implementer_ampere = {
}
}
-implementer_marvell = {
- 'description': 'Marvell ARMADA',
- 'flags': [
- ['RTE_MACHINE', '"armv8a"'],
- ['RTE_CACHE_LINE_SIZE', 64],
- ['RTE_MAX_LCORE', 16],
- ['RTE_MAX_NUMA_NODES', 1]
- ],
- 'part_number_config': part_number_config_arm
-}
-
-implementer_dpaa = {
- 'description': 'NXP DPAA',
- 'flags': [
- ['RTE_MACHINE', '"dpaa"'],
- ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false],
- ['RTE_USE_C11_MEM_MODEL', true],
- ['RTE_CACHE_LINE_SIZE', 64],
- ['RTE_MAX_LCORE', 16],
- ['RTE_MAX_NUMA_NODES', 1]
- ],
- 'part_number_config': part_number_config_arm
-}
-
## Arm implementers (ID from MIDR in Arm Architecture Reference Manual)
implementers = {
'generic': implementer_generic,
'0x41': implementer_arm,
'0x43': implementer_cavium,
- '0x50': implementer_ampere,
- '0x56': implementer_marvell,
- 'dpaa': implementer_dpaa
+ '0x50': implementer_ampere
}
# soc specific aarch64 flags have the highest priority @@ -190,8 +164,12 @@
soc_generic = {
soc_armada = {
'description': 'Marvell ARMADA',
- 'implementer': '0x56',
+ 'implementer': '0x41',
'part_number': '0xd08',
+ 'flags': [
+ ['RTE_MAX_LCORE', 16],
+ ['RTE_MAX_NUMA_NODES', 1]
+ ],
'numa': false,
'disabled_drivers': ['bus/dpaa', 'bus/fslmc', 'common/dpaax'] } @@
-200,13 +178,23 @@ soc_bluefield = {
'description': 'NVIDIA BlueField',
'implementer': '0x41',
'part_number': '0xd08',
+ 'flags': [
+ ['RTE_MAX_LCORE', 16],
+ ['RTE_MAX_NUMA_NODES', 1]
+ ],
'numa': false
}
soc_dpaa = {
'description': 'NXP DPAA',
- 'implementer': 'dpaa',
+ 'implementer': '0x41',
'part_number': '0xd08',
+ 'flags': [
+ ['RTE_MACHINE', '"dpaa"'],
+ ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false],
+ ['RTE_MAX_LCORE', 16],
+ ['RTE_MAX_NUMA_NODES', 1]
+ ],
'numa': false
}
@@ -243,6 +231,10 @@ soc_octeontx2 = {
soc_stingray = {
'description': 'Broadcom Stingray',
'implementer': '0x41',
+ 'flags': [
+ ['RTE_MAX_LCORE', 16],
+ ['RTE_MAX_NUMA_NODES', 1]
+ ],
'part_number': '0xd08',
'numa': false
}
--
2.20.1
Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors
14/11/2020 18:41, Gregory Etelson:
> > 13/11/2020 15:52, Gregory Etelson:
> > > + ret = mlx5_flow_group_to_table(dev, tunnel, jump_data->group,
> > > + &flow_table, grp_info, error);
> >
> > The parameter grp_info is a struct passed as value.
> > I believe it should be passed as a pointer.
>
> struct flow_grp_info is a 64 bit-field:
> struct flow_grp_info {
> uint64_t external:1;
> uint64_t transfer:1;
> uint64_t fdb_def_rule:1;
> /* force standard group translation */
> uint64_t std_tbl_fix:1;
> uint64_t skip_scale:1;
> };
> Since mlx5_flow_group_to_table() does not change bits configuration,
> there is no need to pass this type as a pointer.
I feel passing struct as pointer is a better practice.
> > I see some other functions are passing arrays with [] syntax, which does
> > not make sense in parameters. I should be a simple pointer.
>
> I agree that arrays should not be passed with [] syntax.
> However, I kept existing rte flow style - see rte_flow_create
I recommend not following this bad code style.
Later other occurences should be fixed.
> > Matan, Slava, as maintainers, what are your policies in mlx5 code?
Re: [dpdk-dev] [PATCH v2 1/5] net/mlx5: fix tunnel offload object allocation
14/11/2020 18:47, Gregory Etelson:
> > 13/11/2020 15:52, Gregory Etelson:
> > > The original patch allocated tunnel offload objects with invalid
> > > indexes. As the result, PMD tunnel object allocation failed.
> > >
> > > In this patch indexed pool provides both an index and memory for a new
> > > tunnel offload object.
> > > Also tunnel offload ipool moved to dv enabled code only.
> > >
> > > Fixes: f2e8093 ("net/mlx5: use indexed pool as id generator")
> > >
> > > Signed-off-by: Gregory Etelson 2
> >
> > Typo above
> >
> > > Acked-by: Viacheslav Ovsiienko
> >
> > This patch is breaking compilation in non-DV mode.
> > You should move it at the end of this series, after moving all in DV
> > areas.
>
> Patch "[PATCH v2 5/5] net/mlx5: fix non-dv compilation errors" in that series
> resolves the compilation error.
> To restore tunnel offload functionality all 5 patches must be applied.
All patches must compile.
That's why I recommend moving this patch later in the series.
Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors
> 14/11/2020 18:41, Gregory Etelson:
> > > 13/11/2020 15:52, Gregory Etelson:
> > > > + ret = mlx5_flow_group_to_table(dev, tunnel, jump_data-
> >group,
> > > > + &flow_table, grp_info,
> > > > + error);
> > >
> > > The parameter grp_info is a struct passed as value.
> > > I believe it should be passed as a pointer.
> >
> > struct flow_grp_info is a 64 bit-field:
> > struct flow_grp_info {
> > uint64_t external:1;
> > uint64_t transfer:1;
> > uint64_t fdb_def_rule:1;
> > /* force standard group translation */
> > uint64_t std_tbl_fix:1;
> > uint64_t skip_scale:1;
> > };
> > Since mlx5_flow_group_to_table() does not change bits configuration,
> > there is no need to pass this type as a pointer.
>
> I feel passing struct as pointer is a better practice.
>
The parameter in question is 64 bit unsigned long value.
Structure coating is a syntactic sugar, because
C language does not have bit-field types.
In general, if structure size does not exceed 64 bit it can be passed
by value.
Passing it by reference would create unnecessary indirect access.
> > > I see some other functions are passing arrays with [] syntax, which
> > > does not make sense in parameters. I should be a simple pointer.
> >
> > I agree that arrays should not be passed with [] syntax.
> > However, I kept existing rte flow style - see rte_flow_create
>
> I recommend not following this bad code style.
> Later other occurences should be fixed.
>
> > > Matan, Slava, as maintainers, what are your policies in mlx5 code?
>
>
Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors
14/11/2020 19:31, Gregory Etelson:
> > 14/11/2020 18:41, Gregory Etelson:
> > > > 13/11/2020 15:52, Gregory Etelson:
> > > > > + ret = mlx5_flow_group_to_table(dev, tunnel, jump_data-
> > >group,
> > > > > + &flow_table, grp_info,
> > > > > + error);
> > > >
> > > > The parameter grp_info is a struct passed as value.
> > > > I believe it should be passed as a pointer.
> > >
> > > struct flow_grp_info is a 64 bit-field:
> > > struct flow_grp_info {
> > > uint64_t external:1;
> > > uint64_t transfer:1;
> > > uint64_t fdb_def_rule:1;
> > > /* force standard group translation */
> > > uint64_t std_tbl_fix:1;
> > > uint64_t skip_scale:1;
> > > };
> > > Since mlx5_flow_group_to_table() does not change bits configuration,
> > > there is no need to pass this type as a pointer.
> >
> > I feel passing struct as pointer is a better practice.
>
> The parameter in question is 64 bit unsigned long value.
> Structure coating is a syntactic sugar, because
> C language does not have bit-field types.
> In general, if structure size does not exceed 64 bit it can be passed
> by value.
> Passing it by reference would create unnecessary indirect access.
Did you measure a performance difference?
This current code triggers this compiler note:
drivers/net/mlx5/mlx5_flow.c:7106:1: note:
parameter passing for argument of type 'struct flow_grp_info' changed in GCC 9.1
I'm afraid it can be a problem.
In general we don't have such note on the whole DPDK code base.
[dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
The ETOOMANYREFS errno is missing from the Windows clang build is it used in initialization of flow error structures. The commit will define it as it is done in the minGW Windows build. Signed-off-by: Tal Shnaiderman --- lib/librte_eal/windows/include/rte_os.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/librte_eal/windows/include/rte_os.h b/lib/librte_eal/windows/include/rte_os.h index 569ed92d51..2a91ebf6a1 100644 --- a/lib/librte_eal/windows/include/rte_os.h +++ b/lib/librte_eal/windows/include/rte_os.h @@ -90,6 +90,7 @@ eal_strerror(int code) } #define strerror eal_strerror +#define ETOOMANYREFS WSAETOOMANYREFS #endif /* RTE_TOOLCHAIN_GCC */ -- 2.16.1.windows.4
Re: [dpdk-dev] [PATCH v1 59/72] net/mlx5/windows: support VF PCI address
> Subject: Re: [dpdk-dev] [PATCH v1 59/72] net/mlx5/windows: support VF PCI
> address
>
> On Tue, Oct 27, 2020 at 11:23:22PM +, Ophir Munk wrote:
> > From: Tal Shnaiderman
> >
> > Support VF BDF scanning by checking both the BDF and raw BDF provided
> > by DevX. In Linux a PCI address is formatted as: domain, bus, device,
> > function (DBDF). This is right for both a PF and a VF. In Windows a
> > PF also has a DBDF format, but the domain is always 0, while a VF has
> > a special "domain" called "Virtual PCI Bus, Serial" (for example:
> > "Virtual PCI Bus Slot 2 Serial 2") or segment. The full VF format
> > under Windows is called raw DBF. Windows special domain must be
> > considered and DevX must be called to support it.
> >
> > Signed-off-by: Tal Shnaiderman
> > ---
> > drivers/net/mlx5/windows/mlx5_os.c | 67
> > --
> > 1 file changed, 64 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/windows/mlx5_os.c
> > b/drivers/net/mlx5/windows/mlx5_os.c
> > index f9b469f..4374b05 100644
> > --- a/drivers/net/mlx5/windows/mlx5_os.c
> > +++ b/drivers/net/mlx5/windows/mlx5_os.c
> > @@ -901,6 +901,68 @@ mlx5_os_set_allmulti(struct rte_eth_dev *dev, int
> > enable) }
> > + * @return
> > + * 1 on Device match, 0 on mismatch, rte_errno code on failure.
> > + */
> > +static int
> > +mlx5_match_devx_devices_to_addr(struct devx_device_bdf *devx_bdf,
> > + struct rte_pci_addr *addr)
> > +{
> > + err = mlx5_glue->query_device(devx_bdf, &mlx5_dev);
> > + if (err) {
> > + DRV_LOG(ERR, "query_device failed");
> > + rte_errno = err;
> > + return 0;
>
> Return rte_errno.
Actually updating rte_errno is enough in the function, but the caller need to
check it, I'll fix in v2.
>
> > + }
> > + if (mlx5_match_devx_bdf_to_addr(&mlx5_dev.raw_bdf, addr))
> > + return 1;
> > + return 0;
> > +}
> > +
> > --
> > 2.8.4
Re: [dpdk-dev] [PATCH v1 48/72] net/mlx5/windows: support link update
> Subject: Re: [dpdk-dev] [PATCH v1 48/72] net/mlx5/windows: support link
> update
>
> On Tue, Oct 27, 2020 at 11:23:11PM +, Ophir Munk wrote:
> > From: Tal Shnaiderman
> >
> > Add support for mlx5_link_update() to get link speed and link state.
> > Other parameters are currently hard-coded.
> >
> > Signed-off-by: Tal Shnaiderman
> > ---
> > drivers/net/mlx5/windows/mlx5_ethdev_os.c | 39
> > +++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/net/mlx5/windows/mlx5_ethdev_os.c
> > b/drivers/net/mlx5/windows/mlx5_ethdev_os.c
> > index 4925fd8..0c45101 100644
> > --- a/drivers/net/mlx5/windows/mlx5_ethdev_os.c
> > +++ b/drivers/net/mlx5/windows/mlx5_ethdev_os.c
> > @@ -171,6 +171,45 @@ mlx5_os_read_dev_counters(struct rte_eth_dev
> > *dev, uint64_t *stats) }
> >
> > /**
> > + * @return
> > + * 0 if link status was not updated, positive if it was, a negative errno
> > + * value otherwise and rte_errno is set.
> > + */
> > +int
> > +mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete) {
> > + RTE_SET_USED(wait_to_complete);
> > + struct mlx5_priv *priv;
> > + mlx5_context_st *context_obj;
> > + struct rte_eth_link dev_link;
> > + int ret;
> > +
> > + ret = 0;
> > + if (!dev) {
> > + rte_errno = EINVAL;
> > + return rte_errno;
>
> Should this be "return -rte_errno", as per the function description above?
Right, will fix in v2.
>
> > + }
> > + priv = dev->data->dev_private;
> > + context_obj = (mlx5_context_st *)priv->sh->ctx;
> > 2.8.4
Re: [dpdk-dev] [PATCH v1 42/72] common/mlx5/windows: add DevX UAR getters
> Subject: Re: [dpdk-dev] [PATCH v1 42/72] common/mlx5/windows: add
> DevX UAR getters
>
> On Tue, Oct 27, 2020 at 11:23:05PM +, Ophir Munk wrote:
> > The following getters are added: mlx5_os_get_devx_uar_mmap_offset,
> > mlx5_os_get_devx_uar_base_addr, mlx5_os_get_devx_uar_reg_addr,
> > mlx5_os_get_devx_uar_page_id. This commit is the Windows equivalent
> > of the Linux implementation in (1).
> >
> > (1)
> > commit 8638e19a10aa ("net/mlx5: remove more DV dependencies")
> >
> > Signed-off-by: Ophir Munk
> > ---
> > drivers/common/mlx5/windows/mlx5_common_os.h | 74
> >
> > 1 file changed, 74 insertions(+)
> >
> > diff --git a/drivers/common/mlx5/windows/mlx5_common_os.h
> > b/drivers/common/mlx5/windows/mlx5_common_os.h
> > + * Get mmap offset. Given a pointer to an DevX UAR object of type
> > + * 'struct mlx5dv_devx_uar *' - return its mmap offset.
> > + *
> > + * @param[in] uar
> > + *Pointer to UAR object.
> > + *
> > + * @return
> > + *The mmap offset if uar is valid, 0 otherwise.
> > + */
> > +static inline off_t
> > +mlx5_os_get_devx_uar_mmap_offset(void *uar) {
> > + if (!uar)
> > + return 0;
> > + return 0;
>
> Should we return uar->mmap_off here?
In Windows, this function will always return 0 as mmap offset is unneeded, I'll
update the function docu and change it to reflect it, thanks.
>
> > +}
> > +
> > +/**
> > + * Get base addr pointer. Given a pointer to an UAR object of type
> > + * 'struct mlx5dv_devx_uar *' - return its base address.
> > + *
Re: [dpdk-dev] [PATCH v1 34/72] net/mlx5/windows: implement mlx5 mac addr add
> Subject: Re: [dpdk-dev] [PATCH v1 34/72] net/mlx5/windows: implement
> mlx5 mac addr add
>
> On Tue, Oct 27, 2020 at 11:22:57PM +, Ophir Munk wrote:
> > From: Tal Shnaiderman
> >
> > Get the list of MAC addresses and verify if the input mac parameter
> > already exists. If not - return -ENOTSUP (as Windows does not support
> > adding new MAC addresses). If the MAC address exists (EEXIST) return 0
> > (the equivalent of Linux implementation of this API).
> >
> > Signed-off-by: Tal Shnaiderman
> > ---
> > drivers/net/mlx5/windows/mlx5_os.c | 36
> >
> > 1 file changed, 36 insertions(+)
> >
> > diff --git a/drivers/net/mlx5/windows/mlx5_os.c
> > b/drivers/net/mlx5/windows/mlx5_os.c
> > index b0cc9f3..6e27474 100644
> > --- a/drivers/net/mlx5/windows/mlx5_os.c
> > +++ b/drivers/net/mlx5/windows/mlx5_os.c
> > @@ -183,6 +183,42 @@ mlx5_os_mac_addr_remove(struct rte_eth_dev
> *dev,
> > uint32_t index) }
> >
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise
> > + */
> > +int
> > +mlx5_os_mac_addr_add(struct rte_eth_dev *dev, struct rte_ether_addr
> *mac,
> > +uint32_t index)
> > +{
> > + (void)index;
> > + struct rte_ether_addr lmac;
> > +
> > + if (mlx5_get_mac(dev, &lmac.addr_bytes)) {
> > + DRV_LOG(ERR,
> > + "port %u cannot get MAC address, is mlx5_en"
> > + " loaded? (errno: %s)",
> > + dev->data->port_id, strerror(rte_errno));
> > + return rte_errno;
>
> Should it be "return -rte_errno" here?
Actually, mlx5_get_mac return a negative value in rte_errno in case of failure.
>
> > + }
> > + if (memcmp(&lmac, mac, sizeof(struct rte_ether_addr))) {
> > + DRV_LOG(ERR,
> > + "adding new mac address to device is unsupported");
> > + return -ENOTSUP;
> > + }
> > + return 0;
> > +}
> > +
> > +/**
> > * Modify a VF MAC address
> > * Currently it has no support under Windows.
> > *
> > --
> > 2.8.4
Re: [dpdk-dev] [PATCH v1 27/72] common/mlx5/windows: add OS alloc/dealloc pd
> Subject: Re: [dpdk-dev] [PATCH v1 27/72] common/mlx5/windows: add OS
> alloc/dealloc pd
>
> On Tue, Oct 27, 2020 at 11:22:50PM +, Ophir Munk wrote:
> > From: Tal Shnaiderman
> >
> > Implement Windows API mlx5_os_alloc_pd() and mlx5_os_dealloc_pd().
> > They are equivalent to the Linux implementation.
> >
> > Signed-off-by: Tal Shnaiderman
> > Acked-by: Matan Azrad
> > ---
> > drivers/common/mlx5/rte_common_mlx5_exports.def | 3 +-
> > drivers/common/mlx5/windows/mlx5_common_os.c| 47
> +
> > drivers/common/mlx5/windows/mlx5_common_os.h| 3 ++
> > drivers/common/mlx5/windows/mlx5_win_ext.h | 6
> > 4 files changed, 58 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/common/mlx5/rte_common_mlx5_exports.def
> > b/drivers/common/mlx5/rte_common_mlx5_exports.def
> > + * @return
> > + *The mlx5_pd if pd is valid, NULL and errno otherwise.
> > + */
> > +void *
> > +mlx5_os_alloc_pd(void *ctx)
> > +{
> > + struct mlx5_pd *ppd = mlx5_malloc(MLX5_MEM_ZERO,
> > + sizeof(struct mlx5_pd), 0, SOCKET_ID_ANY);
> > + if (!ppd)
> > + return NULL;
> > +
> > + struct mlx5_devx_obj *obj = mlx5_devx_cmd_alloc_pd(ctx);
> > + if (!obj)
>
> Free ppd here, to avoid memory leak.
Right, will do in v2.
>
> > + return NULL;
> > +
> > + ppd->obj = obj;
> > + ppd->pdn = obj->id;
> > + ppd->devx_ctx = ctx;
> > + return ppd;
> > +}
> > +
Re: [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
On Sat, 14 Nov 2020 23:11:56 +0200, Tal Shnaiderman wrote: > The ETOOMANYREFS errno is missing from the Windows clang build > is it used in initialization of flow error structures. "is it" -> "it is" > The commit will define it as it is done in the minGW Windows build. "The commit will" is unnecessary. "minGW" -> "MinGW" > Signed-off-by: Tal Shnaiderman > --- > lib/librte_eal/windows/include/rte_os.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/librte_eal/windows/include/rte_os.h > b/lib/librte_eal/windows/include/rte_os.h > index 569ed92d51..2a91ebf6a1 100644 > --- a/lib/librte_eal/windows/include/rte_os.h > +++ b/lib/librte_eal/windows/include/rte_os.h > @@ -90,6 +90,7 @@ eal_strerror(int code) > } > > #define strerror eal_strerror > +#define ETOOMANYREFS WSAETOOMANYREFS > > #endif /* RTE_TOOLCHAIN_GCC */ Should be #define ETOOMANYREFS 10059 /* WSAETOOMANYREFS */ for all toolchains: 1. Users of librte_ethdev, who check for ETOOMANYREFS, may not wish to include because of its defines that break librte_net headers. 2. Now that I looked closely, MinGW-w64's #define ETOOMANYREFS WSAETOOMANYREFS is under #if 0 clause (for documentation?). Apologies for earlier misinformation.
Re: [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
> Subject: Re: [PATCH] eal/windows: definition for ETOOMANYREFS errno > > External email: Use caution opening links or attachments > > > On Sat, 14 Nov 2020 23:11:56 +0200, Tal Shnaiderman wrote: > > The ETOOMANYREFS errno is missing from the Windows clang build is it > > used in initialization of flow error structures. > > "is it" -> "it is" > > > The commit will define it as it is done in the minGW Windows build. > > "The commit will" is unnecessary. > > "minGW" -> "MinGW" > > > Signed-off-by: Tal Shnaiderman > > --- > > lib/librte_eal/windows/include/rte_os.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/lib/librte_eal/windows/include/rte_os.h > > b/lib/librte_eal/windows/include/rte_os.h > > index 569ed92d51..2a91ebf6a1 100644 > > --- a/lib/librte_eal/windows/include/rte_os.h > > +++ b/lib/librte_eal/windows/include/rte_os.h > > @@ -90,6 +90,7 @@ eal_strerror(int code) } > > > > #define strerror eal_strerror > > +#define ETOOMANYREFS WSAETOOMANYREFS > > > > #endif /* RTE_TOOLCHAIN_GCC */ > > Should be #define ETOOMANYREFS 10059 /* WSAETOOMANYREFS */ for all > toolchains: > > 1. Users of librte_ethdev, who check for ETOOMANYREFS, may not wish to > include because of its defines that break librte_net headers. > > 2. Now that I looked closely, MinGW-w64's #define ETOOMANYREFS > WSAETOOMANYREFS is under #if 0 clause (for documentation?). Apologies > for earlier misinformation. Thank you for the comments Dmitry, will send a v2 promptly.
[dpdk-dev] [PATCH v2] eal/windows: definition for ETOOMANYREFS errno
The ETOOMANYREFS errno is missing from the Windows build.
it is used in initialization of flow error structures.
The commit will define it with the same error code used by
WSAETOOMANYREFS.
Signed-off-by: Tal Shnaiderman
---
v2: log message fix, apply errno to both Windows builds
and remove dependency on winsock2.h [DmitryK]
---
---
lib/librte_eal/windows/include/rte_os.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_eal/windows/include/rte_os.h
b/lib/librte_eal/windows/include/rte_os.h
index 569ed92d51..8300ea483a 100644
--- a/lib/librte_eal/windows/include/rte_os.h
+++ b/lib/librte_eal/windows/include/rte_os.h
@@ -51,6 +51,8 @@ extern "C" {
/* as in */
typedef long long ssize_t;
+#define ETOOMANYREFS 10059 /* WSAETOOMANYREFS */
+
#ifndef RTE_TOOLCHAIN_GCC
static inline int
--
2.16.1.windows.4
Re: [dpdk-dev] [PATCH v2] eal/windows: definition for ETOOMANYREFS errno
On Sun, 15 Nov 2020 00:21:29 +0200, Tal Shnaiderman wrote: > The ETOOMANYREFS errno is missing from the Windows build. > it is used in initialization of flow error structures. > > The commit will define it with the same error code used by > WSAETOOMANYREFS. > > Signed-off-by: Tal Shnaiderman > > --- > v2: log message fix, apply errno to both Windows builds > and remove dependency on winsock2.h [DmitryK] Acked-by: Dmitry Kozlyuk
Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: fix non-dv compilation errors
> 14/11/2020 19:31, Gregory Etelson:
> > > 14/11/2020 18:41, Gregory Etelson:
> > > > > 13/11/2020 15:52, Gregory Etelson:
> > > > > > + ret = mlx5_flow_group_to_table(dev, tunnel, jump_data-
> > > >group,
> > > > > > + &flow_table, grp_info,
> > > > > > + error);
> > > > >
> > > > > The parameter grp_info is a struct passed as value.
> > > > > I believe it should be passed as a pointer.
> > > >
> > > > struct flow_grp_info is a 64 bit-field:
> > > > struct flow_grp_info {
> > > > uint64_t external:1;
> > > > uint64_t transfer:1;
> > > > uint64_t fdb_def_rule:1;
> > > > /* force standard group translation */
> > > > uint64_t std_tbl_fix:1;
> > > > uint64_t skip_scale:1;
> > > > };
> > > > Since mlx5_flow_group_to_table() does not change bits
> > > > configuration, there is no need to pass this type as a pointer.
> > >
> > > I feel passing struct as pointer is a better practice.
> >
> > The parameter in question is 64 bit unsigned long value.
> > Structure coating is a syntactic sugar, because C language does not
> > have bit-field types.
> > In general, if structure size does not exceed 64 bit it can be passed
> > by value.
> > Passing it by reference would create unnecessary indirect access.
>
> Did you measure a performance difference?
>
> This current code triggers this compiler note:
> drivers/net/mlx5/mlx5_flow.c:7106:1: note:
> parameter passing for argument of type 'struct flow_grp_info' changed in
> GCC 9.1
>
> I'm afraid it can be a problem.
> In general we don't have such note on the whole DPDK code base.
>
I'll handle this issue in DV compilation patch.
