Re: [PATCH 12/16] crypto/dpaa2_sec: fix bitmask truncation

2024-11-17 Thread Hemant Agrawal 



On 15-11-2024 11:35, Stephen Hemminger wrote:

The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: a77db24643b7 ("crypto/dpaa2_sec: support atomic queues")
Cc: ashish.j...@nxp.com
Cc: sta...@dpdk.org

Signed-off-by: Stephen Hemminger 
---
  drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c 
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index ec6577f64c..7ad8fd47dd 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -1491,8 +1491,8 @@ dpaa2_sec_enqueue_burst(void *qp, struct rte_crypto_op 
**ops,
if (*dpaa2_seqn((*ops)->sym->m_src)) {
if (*dpaa2_seqn((*ops)->sym->m_src) & 
QBMAN_ENQUEUE_FLAG_DCA) {
DPAA2_PER_LCORE_DQRR_SIZE--;
-   DPAA2_PER_LCORE_DQRR_HELD &= ~(1 <<
-   *dpaa2_seqn((*ops)->sym->m_src) &
+   DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) 
<<
+   *dpaa2_seqn((*ops)->sym->m_src) 
&
QBMAN_EQCR_DCA_IDXMASK);
}
flags[loop] = *dpaa2_seqn((*ops)->sym->m_src);
@@ -1772,7 +1772,7 @@ dpaa2_sec_set_enqueue_descriptor(struct dpaa2_queue 
*dpaa2_q,
dq_idx = *dpaa2_seqn(m) - 1;
qbman_eq_desc_set_dca(eqdesc, 1, dq_idx, 0);
DPAA2_PER_LCORE_DQRR_SIZE--;
-   DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dq_idx);
+   DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << dq_idx);
}
*dpaa2_seqn(m) = DPAA2_INVALID_MBUF_SEQN;
  }
@@ -4055,7 +4055,7 @@ dpaa2_sec_process_atomic_event(struct qbman_swp *swp 
__rte_unused,
dqrr_index = qbman_get_dqrr_idx(dq);
*dpaa2_seqn(crypto_op->sym->m_src) = QBMAN_ENQUEUE_FLAG_DCA | 
dqrr_index;
DPAA2_PER_LCORE_DQRR_SIZE++;
-   DPAA2_PER_LCORE_DQRR_HELD |= 1 << dqrr_index;
+   DPAA2_PER_LCORE_DQRR_HELD |= UINT64_C(1) << dqrr_index;
DPAA2_PER_LCORE_DQRR_MBUF(dqrr_index) = crypto_op->sym->m_src;
ev->event_ptr = crypto_op;
  }

Acked-by: Hemant Agrawal

Re: [PATCH 1/1] doc: virtual function MTU settings has no meaning

2024-11-17 Thread Raslan Darawsheh 
Hi,

From: Slava Ovsiienko 
Sent: Monday, October 28, 2024 4:45 PM
To: dev@dpdk.org
Cc: Raslan Darawsheh; Matan Azrad; Suanming Mou
Subject: [PATCH 1/1] doc: virtual function MTU settings has no meaning

There is the mlx5 NIC limitations - configuring MTU
for PCI Virtual Function has no meaning. The actual maximal
packet size in VF's receiving is limited by MTU configured
on the related PCI Physical Function, the DPDK datapath
running over VF should be prepared to handle packets
of this maximal size.

Signed-off-by: Viacheslav Ovsiienko 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

RE: [EXTERNAL] [PATCH 4/4] net/bnx2x: fix duplicate branch

2024-11-17 Thread Jerin Jacob 



> -Original Message-
> From: Stephen Hemminger 
> Sent: Tuesday, November 12, 2024 11:14 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Julien Aube
> ; Harish Patil 
> Subject: [EXTERNAL] [PATCH 4/4] net/bnx2x: fix duplicate branch
> 
> Coverity spotted that both legs of the conditional are the same. Looking at
> kernel driver there is additional code there, but the kernel driver supports 
> Wake
> On Lan, and DPDK does not. Coverity issue: 362072 Fixes: 540a211084a7
> ("bnx2x: driver 
> Coverity spotted that both legs of the conditional are the same.
> Looking at kernel driver there is additional code there, but the kernel driver
> supports Wake On Lan, and DPDK does not.
> 
> Coverity issue: 362072
> Fixes: 540a211084a7 ("bnx2x: driver core")
> 
> Signed-off-by: Stephen Hemminger 

Added Cc: sta...@dpdk.org and  Series applied to dpdk-next-net-mrvl/for-main. 
Thanks

Re: [PATCH 14/16] event/dpaa: fix bitmask truncation

2024-11-17 Thread Hemant Agrawal 



On 15-11-2024 11:35, Stephen Hemminger wrote:

More bitmask truncation from mask computation.

Fixes: 0ee17f79ebd0 ("event/dpaa: add enqueue/dequeue")
Cc: sunil.k...@nxp.com
Cc: sta...@dpdk.org

Signed-off-by: Stephen Hemminger 
---
  drivers/event/dpaa/dpaa_eventdev.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c 
b/drivers/event/dpaa/dpaa_eventdev.c
index 853cc1ecf9..400e0ecd1c 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -102,7 +102,7 @@ dpaa_event_enqueue_burst(void *port, const struct rte_event 
ev[],
qman_dca_index(ev[i].impl_opaque, 0);
mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
-   DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+   DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
DPAA_PER_LCORE_DQRR_SIZE--;
break;
default:
@@ -199,11 +199,11 @@ dpaa_event_dequeue_burst(void *port, struct rte_event 
ev[],
/* Check if there are atomic contexts to be released */
i = 0;
while (DPAA_PER_LCORE_DQRR_SIZE) {
-   if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+   if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << i)) {
qman_dca_index(i, 0);
mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
-   DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+   DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
DPAA_PER_LCORE_DQRR_SIZE--;
}
i++;
@@ -263,11 +263,11 @@ dpaa_event_dequeue_burst_intr(void *port, struct 
rte_event ev[],
/* Check if there are atomic contexts to be released */
i = 0;
while (DPAA_PER_LCORE_DQRR_SIZE) {
-   if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+   if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << i)) {
qman_dca_index(i, 0);
mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
-   DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+   DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
DPAA_PER_LCORE_DQRR_SIZE--;
}
i++;

Acked-by: Hemant Agrawal

Re: [PATCH 15/16] net/dpaa: fix bitmask truncation

2024-11-17 Thread Hemant Agrawal 

Acked-by: Hemant Agrawal 

On 15-11-2024 11:35, Stephen Hemminger wrote:

The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: 5e7455931442 ("net/dpaa: support Rx queue configurations with eventdev")
Cc: sunil.k...@nxp.com
Cc: sta...@dpdk.org

Signed-off-by: Stephen Hemminger 
---
  drivers/net/dpaa/dpaa_rxtx.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_rxtx.c b/drivers/net/dpaa/dpaa_rxtx.c
index 247e7b92ba..05bd73becf 100644
--- a/drivers/net/dpaa/dpaa_rxtx.c
+++ b/drivers/net/dpaa/dpaa_rxtx.c
@@ -842,7 +842,7 @@ dpaa_rx_cb_atomic(void *event,
/* Save active dqrr entries */
index = DQRR_PTR2IDX(dqrr);
DPAA_PER_LCORE_DQRR_SIZE++;
-   DPAA_PER_LCORE_DQRR_HELD |= 1 << index;
+   DPAA_PER_LCORE_DQRR_HELD |= UINT64_C(1) << index;
DPAA_PER_LCORE_DQRR_MBUF(index) = mbuf;
ev->impl_opaque = index + 1;
*dpaa_seqn(mbuf) = (uint32_t)index + 1;
@@ -1338,13 +1338,12 @@ dpaa_eth_queue_tx(void *q, struct rte_mbuf **bufs, 
uint16_t nb_bufs)
seqn = *dpaa_seqn(mbuf);
if (seqn != DPAA_INVALID_MBUF_SEQN) {
index = seqn - 1;
-   if (DPAA_PER_LCORE_DQRR_HELD & (1 << index)) {
+   if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << 
index)) {
flags[loop] =
   ((index & QM_EQCR_DCA_IDXMASK) << 8);
flags[loop] |= QMAN_ENQUEUE_FLAG_DCA;
DPAA_PER_LCORE_DQRR_SIZE--;
-   DPAA_PER_LCORE_DQRR_HELD &=
-   ~(1 << index);
+   DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) 
<< index);
}
}

[PATCH v16 2/4] pmu: support reading ARM PMU events in runtime

2024-11-17 Thread Tomasz Duszynski 
Add support for reading ARM PMU events in runtime.

Signed-off-by: Tomasz Duszynski 
---
 app/test/test_pmu.c |  4 ++
 lib/pmu/meson.build |  8 
 lib/pmu/pmu_arm64.c | 94 +
 lib/pmu/rte_pmu.h   |  4 ++
 lib/pmu/rte_pmu_pmc_arm64.h | 30 
 5 files changed, 140 insertions(+)
 create mode 100644 lib/pmu/pmu_arm64.c
 create mode 100644 lib/pmu/rte_pmu_pmc_arm64.h

diff --git a/app/test/test_pmu.c b/app/test/test_pmu.c
index 464e5068ec..0dc33eeccc 100644
--- a/app/test/test_pmu.c
+++ b/app/test/test_pmu.c
@@ -13,6 +13,10 @@ test_pmu_read(void)
int tries = 10, event;
uint64_t val = 0;
 
+#if defined(RTE_ARCH_ARM64)
+   name = "cpu_cycles";
+#endif
+
if (name == NULL) {
printf("PMU not supported on this arch\n");
return TEST_SKIPPED;
diff --git a/lib/pmu/meson.build b/lib/pmu/meson.build
index f173b6f55c..94619c63c2 100644
--- a/lib/pmu/meson.build
+++ b/lib/pmu/meson.build
@@ -10,4 +10,12 @@ endif
 headers = files('rte_pmu.h')
 sources = files('rte_pmu.c')
 
+indirect_headers += files(
+'rte_pmu_pmc_arm64.h',
+)
+
+if dpdk_conf.has('RTE_ARCH_ARM64')
+sources += files('pmu_arm64.c')
+endif
+
 deps += ['log']
diff --git a/lib/pmu/pmu_arm64.c b/lib/pmu/pmu_arm64.c
new file mode 100644
index 00..3b72009cff
--- /dev/null
+++ b/lib/pmu/pmu_arm64.c
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2024 Marvell International Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "pmu_private.h"
+
+#define PERF_USER_ACCESS_PATH "/proc/sys/kernel/perf_user_access"
+
+static int restore_uaccess;
+
+static int
+read_attr_int(const char *path, int *val)
+{
+   char buf[BUFSIZ];
+   int ret, fd;
+
+   fd = open(path, O_RDONLY);
+   if (fd == -1)
+   return -errno;
+
+   ret = read(fd, buf, sizeof(buf));
+   if (ret == -1) {
+   close(fd);
+
+   return -errno;
+   }
+
+   *val = strtol(buf, NULL, 10);
+   close(fd);
+
+   return 0;
+}
+
+static int
+write_attr_int(const char *path, int val)
+{
+   char buf[BUFSIZ];
+   int num, ret, fd;
+
+   fd = open(path, O_WRONLY);
+   if (fd == -1)
+   return -errno;
+
+   num = snprintf(buf, sizeof(buf), "%d", val);
+   ret = write(fd, buf, num);
+   if (ret == -1) {
+   close(fd);
+
+   return -errno;
+   }
+
+   close(fd);
+
+   return 0;
+}
+
+int
+pmu_arch_init(void)
+{
+   int ret;
+
+   ret = read_attr_int(PERF_USER_ACCESS_PATH, &restore_uaccess);
+   if (ret)
+   return ret;
+
+   /* user access already enabled */
+   if (restore_uaccess == 1)
+   return 0;
+
+   return write_attr_int(PERF_USER_ACCESS_PATH, 1);
+}
+
+void
+pmu_arch_fini(void)
+{
+   write_attr_int(PERF_USER_ACCESS_PATH, restore_uaccess);
+}
+
+void
+pmu_arch_fixup_config(uint64_t config[3])
+{
+   /* select 64 bit counters */
+   config[1] |= RTE_BIT64(0);
+   /* enable userspace access */
+   config[1] |= RTE_BIT64(1);
+}
diff --git a/lib/pmu/rte_pmu.h b/lib/pmu/rte_pmu.h
index 06731c866f..6e073f11ea 100644
--- a/lib/pmu/rte_pmu.h
+++ b/lib/pmu/rte_pmu.h
@@ -37,6 +37,10 @@ extern "C" {
 #include 
 #include 
 
+#if defined(RTE_ARCH_ARM64)
+#include "rte_pmu_pmc_arm64.h"
+#endif
+
 /** Maximum number of events in a group */
 #define RTE_MAX_NUM_GROUP_EVENTS 8
 
diff --git a/lib/pmu/rte_pmu_pmc_arm64.h b/lib/pmu/rte_pmu_pmc_arm64.h
new file mode 100644
index 00..39165bbc20
--- /dev/null
+++ b/lib/pmu/rte_pmu_pmc_arm64.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2024 Marvell.
+ */
+#ifndef _RTE_PMU_PMC_ARM64_H_
+#define _RTE_PMU_PMC_ARM64_H_
+
+#include 
+
+static __rte_always_inline uint64_t
+rte_pmu_pmc_read(int index)
+{
+   uint64_t val;
+
+   if (index == 31) {
+   /* CPU Cycles (0x11) must be read via pmccntr_el0 */
+   asm volatile("mrs %0, pmccntr_el0" : "=r" (val));
+   } else {
+   asm volatile(
+   "msr pmselr_el0, %x0\n"
+   "mrs %0, pmxevcntr_el0\n"
+   : "=r" (val)
+   : "rZ" (index)
+   );
+   }
+
+   return val;
+}
+#define rte_pmu_pmc_read rte_pmu_pmc_read
+
+#endif /* _RTE_PMU_PMC_ARM64_H_ */
-- 
2.34.1

Re: [PATCH v6 00/14] use rte_strerror() for rte_errno

2024-11-17 Thread lihuisong (C) 

Series-Acked-by: Huisong Li 

在 2024/11/14 19:39, Dengdui Huang 写道:

Whether strerror() needs to be replaced needs further discussion,
but rte_error should not be used by strerror() because rte_errno
may be an RTE-specific error code.

This patchset use rte_strerror() instead of strerror() for rte_errno.

Also, I tried to add it to checkpatch, but rte_errno can be used as an argument 
to any function
and the checkpatch script should not return an error, so I didn't do that.

Dengdui Huang (14):
   eal: fix use rte errno for libc function
   eal: use rte strerror
   pdump: use rte strerror
   bus/vdev: use rte strerror
   common/mlx5: use rte strerror
   net/af_xdp: use rte strerror
   net/bonding: use rte strerror
   net/failsafe: use rte strerror
   net/memif: use rte strerror
   net/mlx4: use rte strerror
   net/mlx5: use rte strerror
   net/tap: use rte strerror
   app/dumpcap: use rte strerror
   test: use rte strerror

  app/dumpcap/main.c|  2 +-
  app/test/test_bpf.c   | 10 +-
  drivers/bus/vdev/vdev.c   |  2 +-
  drivers/common/mlx5/linux/mlx5_nl.c   | 10 +-
  drivers/common/mlx5/mlx5_common.c |  6 +++---
  drivers/net/af_xdp/rte_eth_af_xdp.c   |  2 +-
  drivers/net/bonding/rte_eth_bond_flow.c   |  4 ++--
  drivers/net/failsafe/failsafe.c   |  2 +-
  drivers/net/failsafe/failsafe_eal.c   |  2 +-
  drivers/net/failsafe/failsafe_flow.c  |  4 ++--
  drivers/net/memif/rte_eth_memif.c |  2 +-
  drivers/net/mlx4/mlx4.c   |  2 +-
  drivers/net/mlx4/mlx4_ethdev.c| 18 +-
  drivers/net/mlx4/mlx4_rxq.c   |  4 ++--
  drivers/net/mlx4/mlx4_txq.c   | 10 +-
  drivers/net/mlx5/hws/mlx5dr_matcher.c |  2 +-
  drivers/net/mlx5/linux/mlx5_ethdev_os.c   | 22 +++---
  drivers/net/mlx5/linux/mlx5_os.c  | 16 
  drivers/net/mlx5/mlx5.c   |  8 
  drivers/net/mlx5/mlx5_mac.c   |  2 +-
  drivers/net/mlx5/mlx5_rxmode.c|  8 
  drivers/net/mlx5/mlx5_stats.c |  2 +-
  drivers/net/mlx5/mlx5_testpmd.c   | 10 +-
  drivers/net/mlx5/mlx5_trigger.c   | 12 ++--
  drivers/net/mlx5/mlx5_vlan.c  |  2 +-
  drivers/net/mlx5/windows/mlx5_ethdev_os.c |  2 +-
  drivers/net/mlx5/windows/mlx5_os.c| 10 +-
  drivers/net/tap/rte_eth_tap.c |  4 ++--
  lib/eal/linux/eal_dev.c   |  2 +-
  lib/eal/unix/eal_unix_memory.c|  2 +-
  lib/pdump/rte_pdump.c |  2 +-
  31 files changed, 93 insertions(+), 93 deletions(-)

Re: [PATCH v3 10/10] app/test-dma-perf: fix parsing of DMA address

2024-11-17 Thread fengchengwen 
On 2024/11/16 4:06, Stephen Hemminger wrote:
> There was useless loop when looking at the DMA address.
> It looks like it was meant to skip whitespace before
> calling strtok.
> 
> Good time to replace strtok with strtok_r as well.

Please delete this line, with this fixed:
Acked-by: Chengwen Feng 

> 
> Link: https://pvs-studio.com/en/blog/posts/cpp/1179/
> 
> Fixes: 623dc9364dc6 ("app/dma-perf: introduce DMA performance test")
> Cc: cheng1.ji...@intel.com
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Stephen Hemminger 
> Acked-by: Bruce Richardson 
> ---
>  app/test-dma-perf/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
> index 18219918cc..8d46b1258b 100644
> --- a/app/test-dma-perf/main.c
> +++ b/app/test-dma-perf/main.c
> @@ -229,8 +229,8 @@ parse_lcore_dma(struct test_configure *test_case, const 
> char *value)
>   return -1;
>   addrs = input;
>  
> - while (*addrs == '\0')
> - addrs++;
> + while (isspace(*addrs))
> + ++addrs;
>   if (*addrs == '\0') {
>   fprintf(stderr, "No input DMA addresses\n");
>   ret = -1;

RE: [PATCH v3 04/10] app/test: avoid duplicate initialization

2024-11-17 Thread Gujjar, Abhinandan S 



> -Original Message-
> From: Stephen Hemminger 
> Sent: Saturday, November 16, 2024 1:37 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ;
> jerin.ja...@caviumnetworks.com; Richardson, Bruce
> ; Gujjar, Abhinandan S
> 
> Subject: [PATCH v3 04/10] app/test: avoid duplicate initialization
> 
> The event_dev_config initialization had duplicate assignments to the same
> element. Change to use structure initialization so that compiler will catch 
> this
> type of bug.
> 
> Link: https://pvs-studio.com/en/blog/posts/cpp/1179/
> 
> Fixes: f8f9d233ea0e ("test/eventdev: add unit tests")
> Cc: jerin.ja...@caviumnetworks.com
> 
> Signed-off-by: Stephen Hemminger 
Acked-by: Abhinandan Gujjar 


> Acked-by: Bruce Richardson 
> ---
>  app/test/test_event_crypto_adapter.c | 24 ++--
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/app/test/test_event_crypto_adapter.c
> b/app/test/test_event_crypto_adapter.c
> index 9d38a66bfa..ab24e30a97 100644
> --- a/app/test/test_event_crypto_adapter.c
> +++ b/app/test/test_event_crypto_adapter.c
> @@ -1154,21 +1154,17 @@ configure_cryptodev(void)
> 
>  static inline void
>  evdev_set_conf_values(struct rte_event_dev_config *dev_conf,
> - struct rte_event_dev_info *info)
> +   const struct rte_event_dev_info *info)
>  {
> - memset(dev_conf, 0, sizeof(struct rte_event_dev_config));
> - dev_conf->dequeue_timeout_ns = info->min_dequeue_timeout_ns;
> - dev_conf->nb_event_ports = NB_TEST_PORTS;
> - dev_conf->nb_event_queues = NB_TEST_QUEUES;
> - dev_conf->nb_event_queue_flows = info->max_event_queue_flows;
> - dev_conf->nb_event_port_dequeue_depth =
> - info->max_event_port_dequeue_depth;
> - dev_conf->nb_event_port_enqueue_depth =
> - info->max_event_port_enqueue_depth;
> - dev_conf->nb_event_port_enqueue_depth =
> - info->max_event_port_enqueue_depth;
> - dev_conf->nb_events_limit =
> - info->max_num_events;
> + *dev_conf = (struct rte_event_dev_config) {
> + .dequeue_timeout_ns = info->min_dequeue_timeout_ns,
> + .nb_event_ports = NB_TEST_PORTS,
> + .nb_event_queues = NB_TEST_QUEUES,
> + .nb_event_queue_flows = info->max_event_queue_flows,
> + .nb_event_port_dequeue_depth = info-
> >max_event_port_dequeue_depth,
> + .nb_event_port_enqueue_depth = info-
> >max_event_port_enqueue_depth,
> + .nb_events_limit = info->max_num_events,
> + };
>  }
> 
>  static int
> --
> 2.45.2

[PATCH v16 0/4] add support for self monitoring

2024-11-17 Thread Tomasz Duszynski 
This series adds self monitoring support i.e allows to configure and
read performance measurement unit (PMU) counters in runtime without
using perf utility. This has certain advantages when application runs on
isolated cores running dedicated tasks.

Events can be read directly using rte_pmu_read() or using dedicated
tracepoint rte_eal_trace_pmu_read(). The latter will cause events to be
stored inside CTF file.

By design, all enabled events are grouped together and the same group
is attached to lcores that use self monitoring funtionality.

Events are enabled by names, which need to be read from standard
location under sysfs i.e

/sys/bus/event_source/devices/PMU/events

where PMU is a core pmu i.e one measuring cpu events. As of today
raw events are not supported.

v16:
- fix build issue related to ALLOW_EXPERIMENTAL_API not always defined
- rename tracepoints upfront to make it easier to decouple for tracing
- minor doc improvements
- make sure non EAL threads can't read PMU
v15:
- add some basic logs related to API failures
- get rid of thread-local-storage
- do not support MT-safety (which was buggy anyway) in management APIs 
(rte_pmu_init(),
  rte_pmu_fini(), rte_pmu_add_{event,events_by_pattern}() as it impacts 
rte_pmu_read()
  performance because more logic needs to be incorporated to handle all corner 
cases
- improve documentation slightly
- various other improvements here and there
v14:
- replace __atomic_xxx with rte_atomic_xxx
- rebase to dpdk/main since that's a new feature
v13:
- allow init/fini calling from multiple contexts
- get rid of conditional compilation and return erors in case APIs are
  used on unsupported OS
v12:
- rebase old series
- slightly refresh existing documentation
- make sure compiler won't optimize accesses to certain variables during
  event readout
- drop previous Acked-by to respin a fresh review cycle
v11:
- skip fast test in case init fails
v10:
- check permissions before using counters
- do not use internal symbols in exported functions
- address review comments
v9:
- fix 'maybe-uninitialized' warning reported by CI
v8:
- just rebase series
v7:
- use per-lcore event group instead of global table index by lcore-id
- don't add pmu_autotest to fast tests because due to lack of suported
  on every arch
v6:
- move codebase to the separate library
- address review comments
v5:
- address review comments
- fix sign extension while reading pmu on x86
- fix regex mentioned in doc
- various minor changes/improvements here and there
v4:
- fix freeing mem detected by debug_autotest
v3:
- fix shared build
v2:
- fix problems reported by test build infra

Tomasz Duszynski (4):
  lib: add generic support for reading PMU events
  pmu: support reading ARM PMU events in runtime
  pmu: support reading Intel x86_64 PMU events in runtime
  eal: add PMU support to tracing library

 MAINTAINERS  |   5 +
 app/test/meson.build |   1 +
 app/test/test_pmu.c  |  55 +++
 app/test/test_trace_perf.c   |  10 +
 doc/api/doxy-api-index.md|   3 +-
 doc/api/doxy-api.conf.in |   1 +
 doc/guides/prog_guide/profile_app.rst|  31 ++
 doc/guides/prog_guide/trace_lib.rst  |  32 ++
 doc/guides/rel_notes/release_24_11.rst   |   7 +
 lib/eal/common/eal_common_trace.c|   5 +-
 lib/eal/common/eal_common_trace_pmu.c|  38 ++
 lib/eal/common/eal_common_trace_points.c |   5 +
 lib/eal/common/eal_trace.h   |   4 +
 lib/eal/common/meson.build   |   1 +
 lib/eal/include/rte_eal_trace.h  |  11 +
 lib/eal/meson.build  |   3 +
 lib/eal/version.map  |   1 +
 lib/meson.build  |   1 +
 lib/pmu/meson.build  |  22 +
 lib/pmu/pmu_arm64.c  |  94 
 lib/pmu/pmu_private.h|  32 ++
 lib/pmu/rte_pmu.c| 536 +++
 lib/pmu/rte_pmu.h| 250 +++
 lib/pmu/rte_pmu_pmc_arm64.h  |  30 ++
 lib/pmu/rte_pmu_pmc_x86_64.h |  24 +
 lib/pmu/version.map  |  15 +
 26 files changed, 1215 insertions(+), 2 deletions(-)
 create mode 100644 app/test/test_pmu.c
 create mode 100644 lib/eal/common/eal_common_trace_pmu.c
 create mode 100644 lib/pmu/meson.build
 create mode 100644 lib/pmu/pmu_arm64.c
 create mode 100644 lib/pmu/pmu_private.h
 create mode 100644 lib/pmu/rte_pmu.c
 create mode 100644 lib/pmu/rte_pmu.h
 create mode 100644 lib/pmu/rte_pmu_pmc_arm64.h
 create mode 100644 lib/pmu/rte_pmu_pmc_x86_64.h
 create mode 100644 lib/pmu/version.map

--
2.34.1

[PATCH v16 1/4] lib: add generic support for reading PMU events

2024-11-17 Thread Tomasz Duszynski 
Add support for programming PMU counters and reading their values
in runtime bypassing kernel completely.

This is especially useful in cases where CPU cores are isolated
i.e run dedicated tasks. In such cases one cannot use standard
perf utility without sacrificing latency and performance.

Signed-off-by: Tomasz Duszynski 
---
 MAINTAINERS|   5 +
 app/test/meson.build   |   1 +
 app/test/test_pmu.c|  49 +++
 doc/api/doxy-api-index.md  |   3 +-
 doc/api/doxy-api.conf.in   |   1 +
 doc/guides/prog_guide/profile_app.rst  |  26 ++
 doc/guides/rel_notes/release_24_11.rst |   7 +
 lib/eal/meson.build|   3 +
 lib/meson.build|   1 +
 lib/pmu/meson.build|  13 +
 lib/pmu/pmu_private.h  |  32 ++
 lib/pmu/rte_pmu.c  | 473 +
 lib/pmu/rte_pmu.h  | 226 
 lib/pmu/version.map|  14 +
 14 files changed, 853 insertions(+), 1 deletion(-)
 create mode 100644 app/test/test_pmu.c
 create mode 100644 lib/pmu/meson.build
 create mode 100644 lib/pmu/pmu_private.h
 create mode 100644 lib/pmu/rte_pmu.c
 create mode 100644 lib/pmu/rte_pmu.h
 create mode 100644 lib/pmu/version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index f84ca3ea68..940dce9940 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1854,6 +1854,11 @@ M: Nithin Dabilpuram 
 M: Pavan Nikhilesh 
 F: lib/node/
 
+PMU - EXPERIMENTAL
+M: Tomasz Duszynski 
+F: lib/pmu/
+F: app/test/test_pmu*
+
 
 Test Applications
 -
diff --git a/app/test/meson.build b/app/test/meson.build
index 40f22a54d5..7533eadcdf 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -143,6 +143,7 @@ source_file_deps = {
 'test_pmd_perf.c': ['ethdev', 'net'] + packet_burst_generator_deps,
 'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
 'test_pmd_ring_perf.c': ['ethdev', 'net_ring', 'bus_vdev'],
+'test_pmu.c': ['pmu'],
 'test_power.c': ['power'],
 'test_power_cpufreq.c': ['power'],
 'test_power_intel_uncore.c': ['power'],
diff --git a/app/test/test_pmu.c b/app/test/test_pmu.c
new file mode 100644
index 00..464e5068ec
--- /dev/null
+++ b/app/test/test_pmu.c
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2024 Marvell International Ltd.
+ */
+
+#include 
+
+#include "test.h"
+
+static int
+test_pmu_read(void)
+{
+   const char *name = NULL;
+   int tries = 10, event;
+   uint64_t val = 0;
+
+   if (name == NULL) {
+   printf("PMU not supported on this arch\n");
+   return TEST_SKIPPED;
+   }
+
+   if (rte_pmu_init() < 0)
+   return TEST_FAILED;
+
+   event = rte_pmu_add_event(name);
+   while (tries--)
+   val += rte_pmu_read(event);
+
+   rte_pmu_fini();
+
+   return val ? TEST_SUCCESS : TEST_FAILED;
+}
+
+static struct unit_test_suite pmu_tests = {
+   .suite_name = "pmu autotest",
+   .setup = NULL,
+   .teardown = NULL,
+   .unit_test_cases = {
+   TEST_CASE(test_pmu_read),
+   TEST_CASES_END()
+   }
+};
+
+static int
+test_pmu(void)
+{
+   return unit_test_suite_runner(&pmu_tests);
+}
+
+REGISTER_FAST_TEST(pmu_autotest, true, true, test_pmu);
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index f0193502bc..a0eb6fad73 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -241,7 +241,8 @@ The public API headers are grouped by topics:
   [log](@ref rte_log.h),
   [errno](@ref rte_errno.h),
   [trace](@ref rte_trace.h),
-  [trace_point](@ref rte_trace_point.h)
+  [trace_point](@ref rte_trace_point.h),
+  [pmu](@ref rte_pmu.h)
 
 - **misc**:
   [EAL config](@ref rte_eal.h),
diff --git a/doc/api/doxy-api.conf.in b/doc/api/doxy-api.conf.in
index d23352d300..f26317d346 100644
--- a/doc/api/doxy-api.conf.in
+++ b/doc/api/doxy-api.conf.in
@@ -69,6 +69,7 @@ INPUT   = @TOPDIR@/doc/api/doxy-api-index.md \
   @TOPDIR@/lib/pdcp \
   @TOPDIR@/lib/pdump \
   @TOPDIR@/lib/pipeline \
+  @TOPDIR@/lib/pmu \
   @TOPDIR@/lib/port \
   @TOPDIR@/lib/power \
   @TOPDIR@/lib/ptr_compress \
diff --git a/doc/guides/prog_guide/profile_app.rst 
b/doc/guides/prog_guide/profile_app.rst
index a6b5fb4d5e..ecb90a0d94 100644
--- a/doc/guides/prog_guide/profile_app.rst
+++ b/doc/guides/prog_guide/profile_app.rst
@@ -7,6 +7,32 @@ Profile Your Application
 The following sections describe methods of profiling DPDK applications on
 different architectures.
 
+Performance counter based profiling
+---
+
+Majority of architectures support some performance monitoring unit (PMU).
+Such unit provides programmable

[PATCH v16 4/4] eal: add PMU support to tracing library

2024-11-17 Thread Tomasz Duszynski 
In order to profile app one needs to store significant amount of samples
somewhere for an analysis later on. Since trace library supports
storing data in a CTF format lets take advantage of that and add a
dedicated PMU tracepoint.

Signed-off-by: Tomasz Duszynski 
---
 app/test/test_trace_perf.c   | 10 
 doc/guides/prog_guide/profile_app.rst|  5 ++
 doc/guides/prog_guide/trace_lib.rst  | 32 +++
 lib/eal/common/eal_common_trace.c|  5 +-
 lib/eal/common/eal_common_trace_pmu.c| 38 ++
 lib/eal/common/eal_common_trace_points.c |  5 ++
 lib/eal/common/eal_trace.h   |  4 ++
 lib/eal/common/meson.build   |  1 +
 lib/eal/include/rte_eal_trace.h  | 11 
 lib/eal/version.map  |  1 +
 lib/pmu/rte_pmu.c| 67 +++-
 lib/pmu/rte_pmu.h| 24 +++--
 lib/pmu/version.map  |  1 +
 13 files changed, 198 insertions(+), 6 deletions(-)
 create mode 100644 lib/eal/common/eal_common_trace_pmu.c

diff --git a/app/test/test_trace_perf.c b/app/test/test_trace_perf.c
index 8257cc02be..28f908ce40 100644
--- a/app/test/test_trace_perf.c
+++ b/app/test/test_trace_perf.c
@@ -114,6 +114,10 @@ worker_fn_##func(void *arg) \
 #define GENERIC_DOUBLE rte_eal_trace_generic_double(3.6)
 #define GENERIC_STR rte_eal_trace_generic_str("hello world")
 #define VOID_FP app_dpdk_test_fp()
+#ifdef RTE_LIB_PMU
+/* 0 corresponds first event passed via --trace= */
+#define READ_PMU rte_pmu_trace_read(0)
+#endif
 
 WORKER_DEFINE(GENERIC_VOID)
 WORKER_DEFINE(GENERIC_U64)
@@ -122,6 +126,9 @@ WORKER_DEFINE(GENERIC_FLOAT)
 WORKER_DEFINE(GENERIC_DOUBLE)
 WORKER_DEFINE(GENERIC_STR)
 WORKER_DEFINE(VOID_FP)
+#ifdef RTE_LIB_PMU
+WORKER_DEFINE(READ_PMU)
+#endif
 
 static void
 run_test(const char *str, lcore_function_t f, struct test_data *data, size_t 
sz)
@@ -174,6 +181,9 @@ test_trace_perf(void)
run_test("double", worker_fn_GENERIC_DOUBLE, data, sz);
run_test("string", worker_fn_GENERIC_STR, data, sz);
run_test("void_fp", worker_fn_VOID_FP, data, sz);
+#ifdef RTE_LIB_PMU
+   run_test("read_pmu", worker_fn_READ_PMU, data, sz);
+#endif
 
rte_free(data);
return TEST_SUCCESS;
diff --git a/doc/guides/prog_guide/profile_app.rst 
b/doc/guides/prog_guide/profile_app.rst
index ecb90a0d94..8e9cddb652 100644
--- a/doc/guides/prog_guide/profile_app.rst
+++ b/doc/guides/prog_guide/profile_app.rst
@@ -33,6 +33,11 @@ As of now implementation imposes certain limitations:
 
 * Each EAL lcore measures same group of events
 
+Alternatively tracing library can be used which offers dedicated tracepoint
+``rte_pmu_trace_read()``.
+
+Refer to :doc:`../prog_guide/trace_lib` for more details.
+
 
 Profiling on x86
 
diff --git a/doc/guides/prog_guide/trace_lib.rst 
b/doc/guides/prog_guide/trace_lib.rst
index d9b17abe90..1cbfd42656 100644
--- a/doc/guides/prog_guide/trace_lib.rst
+++ b/doc/guides/prog_guide/trace_lib.rst
@@ -46,6 +46,7 @@ DPDK tracing library features
   trace format and is compatible with ``LTTng``.
   For detailed information, refer to
   `Common Trace Format `_.
+- Support reading PMU events on ARM64 and x86-64 (Intel)
 
 How to add a tracepoint?
 
@@ -139,6 +140,37 @@ the user must use ``RTE_TRACE_POINT_FP`` instead of 
``RTE_TRACE_POINT``.
 ``RTE_TRACE_POINT_FP`` is compiled out by default and it can be enabled using
 the ``enable_trace_fp`` option for meson build.
 
+PMU tracepoint
+--
+
+Performance monitoring unit (PMU) event values can be read from hardware
+registers using predefined ``rte_pmu_read`` tracepoint.
+
+Tracing is enabled via ``--trace`` EAL option by passing both expression
+matching PMU tracepoint name i.e ``lib.eal.pmu.read`` and expression
+``e=ev1[,ev2,...]`` matching particular events::
+
+--trace='.*pmu.read\|e=cpu_cycles,l1d_cache'
+
+Event names are available under ``/sys/bus/event_source/devices/PMU/events``
+directory, where ``PMU`` is a placeholder for either a ``cpu`` or a directory
+containing ``cpus``.
+
+In contrary to other tracepoints this does not need any extra variables
+added to source files. Instead, caller passes index which follows the order of
+events specified via ``--trace`` parameter. In the following example index 
``0``
+corresponds to ``cpu_cyclces`` while index ``1`` corresponds to ``l1d_cache``.
+
+.. code-block:: c
+
+ ...
+ rte_pmu_trace_read(0);
+ rte_pmu_trace_read(1);
+ ...
+
+PMU tracing support must be explicitly enabled using the ``enable_trace_fp``
+option for meson build.
+
 Event record mode
 -
 
diff --git a/lib/eal/common/eal_common_trace.c 
b/lib/eal/common/eal_common_trace.c
index 918f49bf4f..9be8724ec4 100644
--- a/lib/eal/common/eal_common_trace.c
+++ b/lib/eal/common/eal_common_trace.c
@@ -72,8 +72,10 @@ eal_trace_init(void)
goto free_meta;

[RFC] Revert "vhost: use imported VDUSE uAPI header"

2024-11-17 Thread Stephen Hemminger 
The file vduse.h does not have a license that is compatiable
with current DPDK license policy.

This reverts commit 9fec3f0569087de0129c7f2badaf5be2776e.

Signed-off-by: Stephen Hemminger 
---
 kernel/linux/uapi/linux/vduse.h | 353 
 lib/vhost/meson.build   |   5 +-
 lib/vhost/vduse.c   |   2 +-
 lib/vhost/vduse.h   |  22 ++
 4 files changed, 27 insertions(+), 355 deletions(-)
 delete mode 100644 kernel/linux/uapi/linux/vduse.h

diff --git a/kernel/linux/uapi/linux/vduse.h b/kernel/linux/uapi/linux/vduse.h
deleted file mode 100644
index 11bd48c72c..00
--- a/kernel/linux/uapi/linux/vduse.h
+++ /dev/null
@@ -1,353 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI_VDUSE_H_
-#define _UAPI_VDUSE_H_
-
-#include 
-
-#define VDUSE_BASE 0x81
-
-/* The ioctls for control device (/dev/vduse/control) */
-
-#define VDUSE_API_VERSION  0
-
-/*
- * Get the version of VDUSE API that kernel supported (VDUSE_API_VERSION).
- * This is used for future extension.
- */
-#define VDUSE_GET_API_VERSION  _IOR(VDUSE_BASE, 0x00, __u64)
-
-/* Set the version of VDUSE API that userspace supported. */
-#define VDUSE_SET_API_VERSION  _IOW(VDUSE_BASE, 0x01, __u64)
-
-/**
- * struct vduse_dev_config - basic configuration of a VDUSE device
- * @name: VDUSE device name, needs to be NUL terminated
- * @vendor_id: virtio vendor id
- * @device_id: virtio device id
- * @features: virtio features
- * @vq_num: the number of virtqueues
- * @vq_align: the allocation alignment of virtqueue's metadata
- * @reserved: for future use, needs to be initialized to zero
- * @config_size: the size of the configuration space
- * @config: the buffer of the configuration space
- *
- * Structure used by VDUSE_CREATE_DEV ioctl to create VDUSE device.
- */
-struct vduse_dev_config {
-#define VDUSE_NAME_MAX 256
-   char name[VDUSE_NAME_MAX];
-   __u32 vendor_id;
-   __u32 device_id;
-   __u64 features;
-   __u32 vq_num;
-   __u32 vq_align;
-   __u32 reserved[13];
-   __u32 config_size;
-   __u8 config[];
-};
-
-/* Create a VDUSE device which is represented by a char device 
(/dev/vduse/$NAME) */
-#define VDUSE_CREATE_DEV   _IOW(VDUSE_BASE, 0x02, struct vduse_dev_config)
-
-/*
- * Destroy a VDUSE device. Make sure there are no more references
- * to the char device (/dev/vduse/$NAME).
- */
-#define VDUSE_DESTROY_DEV  _IOW(VDUSE_BASE, 0x03, char[VDUSE_NAME_MAX])
-
-/* The ioctls for VDUSE device (/dev/vduse/$NAME) */
-
-/**
- * struct vduse_iotlb_entry - entry of IOTLB to describe one IOVA region 
[start, last]
- * @offset: the mmap offset on returned file descriptor
- * @start: start of the IOVA region
- * @last: last of the IOVA region
- * @perm: access permission of the IOVA region
- *
- * Structure used by VDUSE_IOTLB_GET_FD ioctl to find an overlapped IOVA 
region.
- */
-struct vduse_iotlb_entry {
-   __u64 offset;
-   __u64 start;
-   __u64 last;
-#define VDUSE_ACCESS_RO 0x1
-#define VDUSE_ACCESS_WO 0x2
-#define VDUSE_ACCESS_RW 0x3
-   __u8 perm;
-};
-
-/*
- * Find the first IOVA region that overlaps with the range [start, last]
- * and return the corresponding file descriptor. Return -EINVAL means the
- * IOVA region doesn't exist. Caller should set start and last fields.
- */
-#define VDUSE_IOTLB_GET_FD _IOWR(VDUSE_BASE, 0x10, struct 
vduse_iotlb_entry)
-
-/*
- * Get the negotiated virtio features. It's a subset of the features in
- * struct vduse_dev_config which can be accepted by virtio driver. It's
- * only valid after FEATURES_OK status bit is set.
- */
-#define VDUSE_DEV_GET_FEATURES _IOR(VDUSE_BASE, 0x11, __u64)
-
-/**
- * struct vduse_config_data - data used to update configuration space
- * @offset: the offset from the beginning of configuration space
- * @length: the length to write to configuration space
- * @buffer: the buffer used to write from
- *
- * Structure used by VDUSE_DEV_SET_CONFIG ioctl to update device
- * configuration space.
- */
-struct vduse_config_data {
-   __u32 offset;
-   __u32 length;
-   __u8 buffer[];
-};
-
-/* Set device configuration space */
-#define VDUSE_DEV_SET_CONFIG   _IOW(VDUSE_BASE, 0x12, struct vduse_config_data)
-
-/*
- * Inject a config interrupt. It's usually used to notify virtio driver
- * that device configuration space has changed.
- */
-#define VDUSE_DEV_INJECT_CONFIG_IRQ_IO(VDUSE_BASE, 0x13)
-
-/**
- * struct vduse_vq_config - basic configuration of a virtqueue
- * @index: virtqueue index
- * @max_size: the max size of virtqueue
- * @reserved: for future use, needs to be initialized to zero
- *
- * Structure used by VDUSE_VQ_SETUP ioctl to setup a virtqueue.
- */
-struct vduse_vq_config {
-   __u32 index;
-   __u16 max_size;
-   __u16 reserved[13];
-};
-
-/*
- * Setup the specified virtqueue. Make sure all virtqueues have been
- * configured before the device is attached to vDPA bus.
- */
-#define

Re: [PATCH v3 09/11] net/ntnic: remove unnecessary void cast

2024-11-17 Thread Serhii Iliushyk 
>On 14.11.2024, 04:38, "Stephen Hemminger" wrote:
>
>
>There is no need to cast memset to void.
>
>
>Signed-off-by: Stephen Hemminger >
>---
>drivers/net/ntnic/nim/i2c_nim.c | 2 +-
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c | 2 +-
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 4 ++--
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 4 ++--
>drivers/net/ntnic/ntnic_ethdev.c | 2 +-
>17 files changed, 31 insertions(+), 31 deletions(-)
>
>
>diff --git a/drivers/net/ntnic/nim/i2c_nim.c b/drivers/net/ntnic/nim/i2c_nim.c
>index e6f7755ded..f9fec5f767 100644
>--- a/drivers/net/ntnic/nim/i2c_nim.c
>+++ b/drivers/net/ntnic/nim/i2c_nim.c
>@@ -292,7 +292,7 @@ static int qsfp_nim_state_build(nim_i2c_ctx_t *ctx, 
>sfp_nim_state_t *state)
>assert(ctx && state);
>assert(ctx->nim_id != NT_NIM_UNKNOWN && "Nim is not initialized");
>
>
>- (void)memset(state, 0, sizeof(*state));
>+ memset(state, 0, sizeof(*state));
>
>
>switch (ctx->nim_id) {
>case 12U:
>diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c 
>b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c
>index d3213593e1..649a060682 100644
>--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c
>+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c
>@@ -18,7 +18,7 @@ struct cat_nthw *cat_nthw_new(void)
>struct cat_nthw *p = malloc(sizeof(struct cat_nthw));
>
>
>if (p)
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>
>
>return p;
>}
>@@ -26,7 +26,7 @@ struct cat_nthw *cat_nthw_new(void)
>void cat_nthw_delete(struct cat_nthw *p)
>{
>if (p) {
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>free(p);
>}
>}
>diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c 
>b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c
>index c1b73179a8..dc3582c9f5 100644
>--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c
>+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c
>@@ -22,7 +22,7 @@ struct csu_nthw *csu_nthw_new(void)
>struct csu_nthw *p = malloc(sizeof(struct csu_nthw));
>
>
>if (p)
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>
>
>return p;
>}
>@@ -30,7 +30,7 @@ struct csu_nthw *csu_nthw_new(void)
>void csu_nthw_delete(struct csu_nthw *p)
>{
>if (p) {
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>free(p);
>}
>}
>diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c 
>b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c
>index 8855978349..7647949bc7 100644
>--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c
>+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c
>@@ -19,7 +19,7 @@ struct flm_nthw *flm_nthw_new(void)
>struct flm_nthw *p = malloc(sizeof(struct flm_nthw));
>
>
>if (p)
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>
>
>return p;
>}
>@@ -27,7 +27,7 @@ struct flm_nthw *flm_nthw_new(void)
>void flm_nthw_delete(struct flm_nthw *p)
>{
>if (p) {
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>free(p);
>}
>}
>diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c 
>b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c
>index 4a8b17101d..d0e65c071d 100644
>--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c
>+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c
>@@ -22,7 +22,7 @@ struct hfu_nthw *hfu_nthw_new(void)
>struct hfu_nthw *p = malloc(sizeof(struct hfu_nthw));
>
>
>if (p)
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>
>
>return p;
>}
>@@ -30,7 +30,7 @@ struct hfu_nthw *hfu_nthw_new(void)
>void hfu_nthw_delete(struct hfu_nthw *p)
>{
>if (p) {
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>free(p);
>}
>}
>diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c 
>b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c
>index 80ead2729a..caf84790cd 100644
>--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c
>+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c
>@@ -23,7 +23,7 @@ struct hsh_nthw *hsh_nthw_new(void)
>struct hsh_nthw *p = malloc(sizeof(struct hsh_nthw));
>
>
>if (p)
>- (void)memset(p, 0, sizeof(*p));
>+ memset(p, 0, sizeof(*p));
>
>
>return p;
>}
>@@ -31,7 +31,7 @@ struct hsh_nthw *hsh_nthw_new(void)
>void hsh_nthw_delete(struct hsh_nthw *p)
>{
>if

Re: [PATCH v3 10/11] net/ntnic: check result of malloc

2024-11-17 Thread Serhii Iliushyk 
On 14.11.2024, 04:38, "Stephen Hemminger" wrote:
>
>
>Need to check the result of malloc() before calling memset.
>This is only place in this driver that forgot, other code
>does check.
>
>
>Signed-off-by: Stephen Hemminger >
>---
>drivers/net/ntnic/nthw/nthw_rac.c | 4 +++-
>1 file changed, 3 insertions(+), 1 deletion(-)
>
>
>diff --git a/drivers/net/ntnic/nthw/nthw_rac.c 
>b/drivers/net/ntnic/nthw/nthw_rac.c
>index ca6aba6db2..f275e64da3 100644
>--- a/drivers/net/ntnic/nthw/nthw_rac.c
>+++ b/drivers/net/ntnic/nthw/nthw_rac.c
>@@ -31,7 +31,9 @@
>nthw_rac_t *nthw_rac_new(void)
>{
>nthw_rac_t *p = malloc(sizeof(nthw_rac_t));
>- memset(p, 0, sizeof(nthw_rac_t));
>+
>+ if (p)
>+ memset(p, 0, sizeof(nthw_rac_t));
>return p;
>}
>
>
>-- 
>2.45.2
>
>

Hi Stephen,

Thanks a lot for noticing and fixing this issue.
We appreciate it.

Reviewed-by: Serhii Iliushyk

Re: [PATCH v3 11/11] net/ntnic: remove unnecessary memset

2024-11-17 Thread Serhii Iliushyk 
>On 14.11.2024, 04:38, "Stephen Hemminger" wrote:
>
>
>Calling memset before free() has no effect and will be flagged
>by security parsing tools as a potential bug. None of these data
>structures have sensitive information.
>
>
>Signed-off-by: Stephen Hemminger >
>---
>drivers/net/ntnic/nthw/core/nthw_hif.c | 5 +
>drivers/net/ntnic/nthw/core/nthw_iic.c | 5 +
>drivers/net/ntnic/nthw/core/nthw_pcie3.c | 5 +
>drivers/net/ntnic/nthw/core/nthw_rpf.c | 5 +
>drivers/net/ntnic/nthw/core/nthw_sdc.c | 5 +
>drivers/net/ntnic/nthw/core/nthw_si5340.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 1 -
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 5 +
>drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 5 +
>20 files changed, 19 insertions(+), 77 deletions(-)
>
>
>diff --git a/drivers/net/ntnic/nthw/core/nthw_hif.c 
>b/drivers/net/ntnic/nthw/core/nthw_hif.c
>index 9f699e4f94..d702257d76 100644
>--- a/drivers/net/ntnic/nthw/core/nthw_hif.c
>+++ b/drivers/net/ntnic/nthw/core/nthw_hif.c
>@@ -23,10 +23,7 @@ nthw_hif_t *nthw_hif_new(void)
>
>
>void nthw_hif_delete(nthw_hif_t *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(nthw_hif_t));
>- free(p);
>- }
>+ free(p);
>}
>
>
>int nthw_hif_init(nthw_hif_t *p, nthw_fpga_t *p_fpga, int n_instance)
>diff --git a/drivers/net/ntnic/nthw/core/nthw_iic.c 
>b/drivers/net/ntnic/nthw/core/nthw_iic.c
>index 269754c24a..a98ec659c4 100644
>--- a/drivers/net/ntnic/nthw/core/nthw_iic.c
>+++ b/drivers/net/ntnic/nthw/core/nthw_iic.c
>@@ -253,10 +253,7 @@ int nthw_iic_init(nthw_iic_t *p, nthw_fpga_t *p_fpga, int 
>n_iic_instance,
>
>
>void nthw_iic_delete(nthw_iic_t *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(nthw_iic_t));
>- free(p);
>- }
>+ free(p);
>}
>
>
>int nthw_iic_set_retry_params(nthw_iic_t *p, const int n_poll_delay, const int 
>n_bus_ready_retry,
>diff --git a/drivers/net/ntnic/nthw/core/nthw_pcie3.c 
>b/drivers/net/ntnic/nthw/core/nthw_pcie3.c
>index 5997ebb419..a5833e166c 100644
>--- a/drivers/net/ntnic/nthw/core/nthw_pcie3.c
>+++ b/drivers/net/ntnic/nthw/core/nthw_pcie3.c
>@@ -24,10 +24,7 @@ nthw_pcie3_t *nthw_pcie3_new(void)
>
>
>void nthw_pcie3_delete(nthw_pcie3_t *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(nthw_pcie3_t));
>- free(p);
>- }
>+ free(p);
>}
>
>
>int nthw_pcie3_init(nthw_pcie3_t *p, nthw_fpga_t *p_fpga, int n_instance)
>diff --git a/drivers/net/ntnic/nthw/core/nthw_rpf.c 
>b/drivers/net/ntnic/nthw/core/nthw_rpf.c
>index 1ed4d7b4e0..d5c19e312b 100644
>--- a/drivers/net/ntnic/nthw/core/nthw_rpf.c
>+++ b/drivers/net/ntnic/nthw/core/nthw_rpf.c
>@@ -22,10 +22,7 @@ nthw_rpf_t *nthw_rpf_new(void)
>
>
>void nthw_rpf_delete(nthw_rpf_t *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(nthw_rpf_t));
>- free(p);
>- }
>+ free(p);
>}
>
>
>int nthw_rpf_init(nthw_rpf_t *p, nthw_fpga_t *p_fpga, int n_instance)
>diff --git a/drivers/net/ntnic/nthw/core/nthw_sdc.c 
>b/drivers/net/ntnic/nthw/core/nthw_sdc.c
>index fc73e6957c..e32d87b967 100644
>--- a/drivers/net/ntnic/nthw/core/nthw_sdc.c
>+++ b/drivers/net/ntnic/nthw/core/nthw_sdc.c
>@@ -22,10 +22,7 @@ nthw_sdc_t *nthw_sdc_new(void)
>
>
>void nthw_sdc_delete(nthw_sdc_t *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(nthw_sdc_t));
>- free(p);
>- }
>+ free(p);
>}
>
>
>int nthw_sdc_init(nthw_sdc_t *p, nthw_fpga_t *p_fpga, int n_instance)
>diff --git a/drivers/net/ntnic/nthw/core/nthw_si5340.c 
>b/drivers/net/ntnic/nthw/core/nthw_si5340.c
>index 05cadc0bf4..ceaa58e0f7 100644
>--- a/drivers/net/ntnic/nthw/core/nthw_si5340.c
>+++ b/drivers/net/ntnic/nthw/core/nthw_si5340.c
>@@ -44,10 +44,7 @@ int nthw_si5340_init(nthw_si5340_t *p, nthw_iic_t 
>*p_nthw_iic, uint8_t n_iic_add
>
>
>void nthw_si5340_delete(nthw_si5340_t *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(nthw_si5340_t));
>- free(p);
>- }
>+ free(p);
>}
>
>
>/*
>diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c 
>b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c
>index 649a060682..776d4986f7 100644
>--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c
>+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c
>@@ -25,10 +25,7 @@ struct cat_nthw *cat_nthw_new(void)
>
>
>void cat_nthw_delete(struct cat_nthw *p)
>{
>- if (p) {
>- memset(p, 0, sizeof(*p));
>- free(p);
>- }
>+ free(p);
>}
>
>
>void cat_nthw_set_debug_mode(struct c

Re: rte_fib network order bug

2024-11-17 Thread Vladimir Medvedkin 
Hi all,

[Robin] > I had not understood that it was *only* the lookups that were
network order
[Morten] >When I saw the byte order flag the first time, it was not clear
to me either that it only affected lookups - I too thought it covered the
entire API of the library. This needs to be emphasized in the description
of the flag. And the flag's name should contain LOOKUP
[Morten] > And/or rename RTE_FIB_F_NETWORK_ORDER to
RTE_FIB_F_NETWORK_ORDER_LOOKUP or similar.

There is a clear comment for this flag that it has effects on lookup.
Repeating the statement with an exclamation mark seems too much. Moreover,
at first this flag was named "RTE_FIB_FLAG_LOOKUP_BE" and it was suggested
for renaming here:
https://inbox.dpdk.org/dev/d4swpkoprd5z.87yiet3y...@redhat.com/

[Morten] >Control plane API should use CPU byte order ... adding it
(support for network byte order) to the RIB library would be nice too.
I'm not sure if I understood you correctly here, RIB is a control plane
library.

[Robin] > an IPv4 address is *not* an integer. It should be treated as an
opaque value.
I don't agree here. IPv4 is 32 bits of information. CPUs usually can treat
32 bits of information as an integer, which is really useful.

[Morten] > Treating IPv4 addresses as byte arrays would allow simple
memcmp() for range comparison
How is it possible for a general case? For example, I need to test IP
addresses against range 1.1.1.7 - 10.20.30.37.

[Robin] >Also for consistency with IPv6, I really think that *all*
addresses should be dealt in their network form.
There is no such a problem as byte order mismatch for IPv6 addresses since
they can not be treated by modern CPUs as an native integer type.

[Robin] >But it (RTE_IPV4) will always generate addresses in *host order*.
Which means they cannot be used in IPv4 headers without passing them
through htonl().
RTE_IPV4 is not limited by setting IPv4 headers values.

[Robin] >Maybe we could revert that patch and defer a complete change of
the rib/fib APIs to only expose network order addresses?
I don't agree with that. Don't limit yourself to just manipulating network
headers.

[Robin] >Thinking about it some more. Having a flag for such a drastic
change in behaviour does not seem right.
This flag is optional. I don't see any problems with that.

In general, here we just have different perspectives on the problem. I can
see and understand your point.
My considerations are:
- The vast majority of the longest prefix match algorithms works with
addresses in host byte order (binary trees, multibit tries, DXR, except
only hash based lookup)
- If you do byteswap two or more times - If you run byteswap two or more
times, you are probably doing something wrong in terms of computations

So, feel free to submit patches adding this feature to the control plane
API, but let's consider:
- default behaviour should remain the same. Why? At least because for my
usecases I'd like to have "data representation" (byte swap) outside of the
library. Not to mention ABI/API breakage
-  IPv4 should stay as uint32_t. C doesn't know such a thing as byte order,
it knows about size and signedness. rte_be32_t is just a hint for us -
humans :)


пт, 15 нояб. 2024 г. в 17:00, Stephen Hemminger :

> On Fri, 15 Nov 2024 15:28:33 +0100
> "Robin Jarry"  wrote:
>
> > Morten Brørup, Nov 15, 2024 at 14:52:
> > > Robin, you've totally won me over on this endian discussion. :-)
> > > Especially the IPv6 comparison make it clear why IPv4 should also be
> > > network byte order.
> > >
> > > API/ABI stability is a pain... we're stuck with host endian IPv4
> > > addresses; e.g. for the RTE_IPV4() macro, which I now agree produces
> > > the wrong endian value (on little endian CPUs).
> >
> > At least for 24.11 it is too late. But maybe we could make it right for
> > the next LTS?
> >
> > >> Vladimir, could we at least consider adding a real network order mode
> > >> for the rib and fib libraries? So that we can have consistent APIs
> > >> between IPv4 and IPv6?
> > >
> > > And/or rename RTE_FIB_F_NETWORK_ORDER to
> > > RTE_FIB_F_NETWORK_ORDER_LOOKUP or similar. This is important if real
> > > network order mode is added (now or later)!
> >
> > Maybe we could revert that patch and defer a complete change of the
> > rib/fib APIs to only expose network order addresses? It would be an ABI
> > breakage but if properly announced in advance, it should be possible.
> >
> > Thinking about it some more. Having a flag for such a drastic change in
> > behaviour does not seem right.
>
> It was a mistake for DPDK to define its own data structures for IP
> addresses.
> Would have been much better to stick with the legacy what BSD, Linux (and
> Windows)
> uses in API. 'struct in_addr' and 'struct in6_addr'
>
> Reinvention did not help users.
>


-- 
Regards,
Vladimir

Re: [PATCH] eal: fix lcore variables documentation

2024-11-17 Thread Mattias Rönnblom 

On 2024-11-14 18:09, Thomas Monjalon wrote:

The lcore variables API is new in DPDK 24.11,
that's why the function rte_lcore_var_alloc() was marked experimental.
To be clearer, the whole header file (including all macros)
is marked experimental.

A change decreased the default buffer size from 1 MB to 128 kB,
missing to update the documentation, which is fixed here.

Fixes: 5bce9bed67ad ("eal: add static per-lcore memory allocation facility")
Fixes: f2fd6c2e080c ("config: limit lcore variable maximum size to 128k")

Signed-off-by: Thomas Monjalon 
---
  doc/guides/prog_guide/lcore_var.rst | 2 +-
  lib/eal/include/rte_lcore_var.h | 2 ++
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/lcore_var.rst 
b/doc/guides/prog_guide/lcore_var.rst
index b492ad12c6..3d9384bc33 100644
--- a/doc/guides/prog_guide/lcore_var.rst
+++ b/doc/guides/prog_guide/lcore_var.rst
@@ -240,7 +240,7 @@ and huge pages for lcore variables:
and their use would result in a significant amount of memory going to waste.
An example: ~256 kB worth of lcore variables are allocated
by DPDK libraries, PMDs and the application.
-  ``RTE_MAX_LCORE_VAR`` is set to 1 MB and ``RTE_MAX_LCORE`` to 128.
+  ``RTE_MAX_LCORE_VAR`` is set to 128 kB and ``RTE_MAX_LCORE`` to 128.
With 4 kB OS pages, only the first ~64 pages of each of the 128 per-lcore 
id slices
in the (only) ``lcore_var_buffer`` will actually be resident (paged in).
Here, demand paging saves ~98 MB of memory.
diff --git a/lib/eal/include/rte_lcore_var.h b/lib/eal/include/rte_lcore_var.h
index 28d88cd89b..0216a67cab 100644
--- a/lib/eal/include/rte_lcore_var.h
+++ b/lib/eal/include/rte_lcore_var.h
@@ -15,6 +15,8 @@
   *
   * Please refer to the lcore variables' programmer's guide
   * for an overview of this API and its implementation.
+ *
+ * EXPERIMENTAL: this API may change, or be removed, without prior notice.
   */
  
  #include 


Acked-by: Mattias Rönnblom 

Thanks!