Requesting extra documentation on how to use and modify SWX pipeline

[dung Phan]

Hello everyone,
Is there any extra developer resource besides "doc.dpdk.org"? I am trying
to customize the SWX pipeline to add extra functionalities when the
pipeline is configured with a P4C compiled spec file.
I am currently having little luck trying to comprehend /lib/pipeline and
what document i did find (particularly at doc.dpdk.org) is hardly
comprehensive

Any help or suggestion welcome, thank you in advance for your time

BR,
Zung

Re: [RFC PATCH v1 0/4] dts: add dts api docs

[Juraj Linkeš]

Hi Bruce, Thomas,

The meson integration is kinda all over the place. I wanted to use the
existing conf.py Sphinx config file, but I also wanted to keep the docs
separated (because of extra DTS api docs dependencies), so the various
pieces are in different places (the config file in one place, meson code in
dts directory and generated Sphinx docs are in a new directory in the api
build dir, separate from the rest of the Sphinx html).

The big thing here is that I didn't figure out how to separate the dts api
build from the rest of the docs. I don't know how the -Denable_docs option
is supposed to work. I wanted to use -Denable_dts_docs in the same fashion
to decouple the builds, but it doesn't seem to work. Reading the code I
think the original option doesn't actually do anything - does it work? How
is it supposed to work?

Thanks,
Juraj

On Thu, Mar 23, 2023 at 11:40 AM Juraj Linkeš 
wrote:

> Augment the meson build system with dts api generation. The api docs are
> generated from Python docstrings in DTS using Sphinx. The format of
> choice is the Google format [0].
>
> The guide html sphinx configuration is used to preserve the same style.
>
> The build requires the same Python version and dependencies as DTS,
> because Sphinx imports the Python modules. Dependencies are installed
> using Poetry from the dts directory:
>
> poetry install --with docs
>
> After installing, enter the Poetry shell:
>
> poetry shell
>
> And then run the build:
> ninja -C  doc
>
> There's only one properly documented module that serves as a
> demonstration of the style - framework.testbed_model.node.
>
> I didn't figure out how to separate dts build from the rest of the docs,
> which I think is required because of the different dependencies.
> I thought the enable_docs option would do this, so I added
> enable_dts_docs, but it doesn't seem to be working. Requesting comment
> on this.
>
> [0]
> https://google.github.io/styleguide/pyguide.html#s3.8.4-comments-in-classes
>
> Juraj Linkeš (4):
>   dts: code adjustments for sphinx
>   dts: add doc generation dependencies
>   dts: add doc generation
>   dts: format docstrigs to google format
>
>  doc/api/meson.build   |   1 +
>  doc/guides/conf.py|  22 +-
>  doc/guides/meson.build|   1 +
>  doc/guides/tools/dts.rst  |  29 +
>  doc/meson.build   |   5 -
>  dts/doc-index.rst |  20 +
>  dts/framework/config/__init__.py  |  11 +
>  .../{testbed_model/hw => config}/cpu.py   |  13 +
>  dts/framework/dts.py  |   8 +-
>  dts/framework/remote_session/__init__.py  |   3 +-
>  dts/framework/remote_session/linux_session.py |   2 +-
>  dts/framework/remote_session/os_session.py|  12 +-
>  .../remote_session/remote/__init__.py |  16 -
>  .../{remote => }/remote_session.py|   0
>  .../{remote => }/ssh_session.py   |   0
>  dts/framework/settings.py |  55 +-
>  dts/framework/testbed_model/__init__.py   |  10 +-
>  dts/framework/testbed_model/hw/__init__.py|  27 -
>  dts/framework/testbed_model/node.py   | 164 ++--
>  dts/framework/testbed_model/sut_node.py   |   9 +-
>  .../testbed_model/{hw => }/virtual_device.py  |   0
>  dts/main.py   |   3 +-
>  dts/meson.build   |  50 ++
>  dts/poetry.lock   | 770 --
>  dts/pyproject.toml|   7 +
>  dts/tests/TestSuite_hello_world.py|   6 +-
>  meson.build   |   6 +
>  meson_options.txt |   2 +
>  28 files changed, 1027 insertions(+), 225 deletions(-)
>  create mode 100644 dts/doc-index.rst
>  rename dts/framework/{testbed_model/hw => config}/cpu.py (95%)
>  delete mode 100644 dts/framework/remote_session/remote/__init__.py
>  rename dts/framework/remote_session/{remote => }/remote_session.py (100%)
>  rename dts/framework/remote_session/{remote => }/ssh_session.py (100%)
>  delete mode 100644 dts/framework/testbed_model/hw/__init__.py
>  rename dts/framework/testbed_model/{hw => }/virtual_device.py (100%)
>  create mode 100644 dts/meson.build
>
> --
> 2.30.2
>
>

[RFC v2 0/3] add frequency adjustment support for PTP timesync

[Simei Su]

This patchset cover below parts:
(1)Introduce a new timesync API called "rte_eth_timesync_adjust_freq" that
   enables frequency adjustment during PTP timesync. This new API aligns with
   the kernel PTP which already supports frequency adjustment. It brings DPDK
   closer in alignment with the kernel's best practice.

(2)Refine the ptpclient application by applying a PI algorithm that leverages
   the new API to further improve timesync accuracy. These changes doesn't break
   original solution and creates a more accurate solution for DPDK-based high
   accuracy PTP. We have provided significant improvements for timesync accuracy
   on e810 and we believe these improvements will also benefit other devices.

The original command for starting ptpclient is:
./build/examples/dpdk-ptpclient -a :81:00.0 -c 1 -n 3 -- -T 0 -p 0x1

The command with PI algorithm is:
./build/examples/dpdk-ptpclient -a :81:00.0 -c 1 -n 3 -- -T 0 -p 0x1 -- 
controller=pi

[RFC v2 1/3] ethdev: add frequency adjustment API.
[RFC v2 2/3] examples/ptpclient: refine application.
[RFC v2 3/3] examples/ptpclient: add frequency adjustment support.

v2:
* Remove the ice PMD part from the RFC.
* Add description in cover letter.
* Refine commit log in patch.

Simei Su (3):
  ethdev: add frequency adjustment API
  examples/ptpclient: refine application
  examples/ptpclient: add frequency adjustment support

 examples/ptpclient/ptpclient.c   | 222 +--
 lib/ethdev/ethdev_driver.h   |   5 +
 lib/ethdev/ethdev_trace.h|   9 ++
 lib/ethdev/ethdev_trace_points.c |   3 +
 lib/ethdev/rte_ethdev.c  |  18 
 lib/ethdev/rte_ethdev.h  |  19 
 6 files changed, 245 insertions(+), 31 deletions(-)

-- 
2.9.5

[RFC v2 1/3] ethdev: add frequency adjustment API

[Simei Su]

This patch introduces a new timesync API "rte_eth_timesync_adjust_freq"
which enables frequency adjustment during PTP timesync.

Signed-off-by: Simei Su 
---
 lib/ethdev/ethdev_driver.h   |  5 +
 lib/ethdev/ethdev_trace.h|  9 +
 lib/ethdev/ethdev_trace_points.c |  3 +++
 lib/ethdev/rte_ethdev.c  | 18 ++
 lib/ethdev/rte_ethdev.h  | 19 +++
 5 files changed, 54 insertions(+)

diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 2c9d615..b1451d2 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -633,6 +633,9 @@ typedef int (*eth_timesync_read_tx_timestamp_t)(struct 
rte_eth_dev *dev,
 /** @internal Function used to adjust the device clock. */
 typedef int (*eth_timesync_adjust_time)(struct rte_eth_dev *dev, int64_t);
 
+/** @internal Function used to adjust the clock frequency. */
+typedef int (*eth_timesync_adjust_freq)(struct rte_eth_dev *dev, int64_t);
+
 /** @internal Function used to get time from the device clock. */
 typedef int (*eth_timesync_read_time)(struct rte_eth_dev *dev,
  struct timespec *timestamp);
@@ -1344,6 +1347,8 @@ struct eth_dev_ops {
eth_timesync_read_tx_timestamp_t timesync_read_tx_timestamp;
/** Adjust the device clock */
eth_timesync_adjust_time   timesync_adjust_time;
+   /** Adjust the clock frequency */
+   eth_timesync_adjust_freq   timesync_adjust_freq;
/** Get the device clock time */
eth_timesync_read_time timesync_read_time;
/** Set the device clock time */
diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
index 3dc7d02..d92554b 100644
--- a/lib/ethdev/ethdev_trace.h
+++ b/lib/ethdev/ethdev_trace.h
@@ -2196,6 +2196,15 @@ RTE_TRACE_POINT_FP(
rte_trace_point_emit_int(ret);
 )
 
+/* Called in loop in examples/ptpclient */
+RTE_TRACE_POINT_FP(
+   rte_eth_trace_timesync_adjust_freq,
+   RTE_TRACE_POINT_ARGS(uint16_t port_id, int64_t ppm, int ret),
+   rte_trace_point_emit_u16(port_id);
+   rte_trace_point_emit_i64(ppm);
+   rte_trace_point_emit_int(ret);
+)
+
 /* Called in loop in app/test-flow-perf */
 RTE_TRACE_POINT_FP(
rte_flow_trace_create,
diff --git a/lib/ethdev/ethdev_trace_points.c b/lib/ethdev/ethdev_trace_points.c
index 61010ca..c01b5d3 100644
--- a/lib/ethdev/ethdev_trace_points.c
+++ b/lib/ethdev/ethdev_trace_points.c
@@ -406,6 +406,9 @@ 
RTE_TRACE_POINT_REGISTER(rte_eth_trace_timesync_read_tx_timestamp,
 RTE_TRACE_POINT_REGISTER(rte_eth_trace_timesync_adjust_time,
lib.ethdev.timesync_adjust_time)
 
+RTE_TRACE_POINT_REGISTER(rte_eth_trace_timesync_adjust_freq,
+   lib.ethdev.timesync_adjust_freq)
+
 RTE_TRACE_POINT_REGISTER(rte_eth_trace_timesync_read_time,
lib.ethdev.timesync_read_time)
 
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 4d03255..f5934bb 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6017,6 +6017,24 @@ rte_eth_timesync_adjust_time(uint16_t port_id, int64_t 
delta)
 }
 
 int
+rte_eth_timesync_adjust_freq(uint16_t port_id, int64_t ppm)
+{
+   struct rte_eth_dev *dev;
+   int ret;
+
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+   dev = &rte_eth_devices[port_id];
+
+   if (*dev->dev_ops->timesync_adjust_freq == NULL)
+   return -ENOTSUP;
+   ret = eth_err(port_id, (*dev->dev_ops->timesync_adjust_freq)(dev, ppm));
+
+   rte_eth_trace_timesync_adjust_freq(port_id, ppm, ret);
+
+   return ret;
+}
+
+int
 rte_eth_timesync_read_time(uint16_t port_id, struct timespec *timestamp)
 {
struct rte_eth_dev *dev;
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 99fe9e2..9737461 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5102,6 +5102,25 @@ int rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
 int rte_eth_timesync_adjust_time(uint16_t port_id, int64_t delta);
 
 /**
+ * Adjust the clock increment rate on an Ethernet device.
+ *
+ * This is usually used in conjunction with other Ethdev timesync functions to
+ * synchronize the device time using the IEEE1588/802.1AS protocol.
+ *
+ * @param port_id
+ *  The port identifier of the Ethernet device.
+ * @param ppm
+ *  Parts per million with 16-bit fractional field
+ *
+ * @return
+ *   - 0: Success.
+ *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
+ *   - -ENOTSUP: The function is not supported by the Ethernet driver.
+ */
+int rte_eth_timesync_adjust_freq(uint16_t port_id, int64_t ppm);
+
+/**
  * Read the time from the timesync clock on an Ethernet device.
  *
  * This is usually used in conjunction with other Ethdev timesync functions to
-- 
2.9.5

[RFC v2 2/3] examples/ptpclient: refine application

[Simei Su]

This patch reworks code to split delay request message parsing
from follow up message parsing which doesn't break original logic.

Signed-off-by: Simei Su 
Signed-off-by: Wenjun Wu 
---
 examples/ptpclient/ptpclient.c | 48 --
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index cdf2da6..74a1bf5 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -382,21 +382,11 @@ parse_sync(struct ptpv2_data_slave_ordinary *ptp_data, 
uint16_t rx_tstamp_idx)
 static void
 parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
 {
-   struct rte_ether_hdr *eth_hdr;
-   struct rte_ether_addr eth_addr;
struct ptp_header *ptp_hdr;
-   struct clock_id *client_clkid;
struct ptp_message *ptp_msg;
-   struct delay_req_msg *req_msg;
-   struct rte_mbuf *created_pkt;
struct tstamp *origin_tstamp;
-   struct rte_ether_addr eth_multicast = ether_multicast;
-   size_t pkt_size;
-   int wait_us;
struct rte_mbuf *m = ptp_data->m;
-   int ret;
 
-   eth_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
ptp_hdr = (struct ptp_header *)(rte_pktmbuf_mtod(m, char *)
+ sizeof(struct rte_ether_hdr));
if (memcmp(&ptp_data->master_clock_id,
@@ -413,6 +403,26 @@ parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
ptp_data->tstamp1.tv_sec =
((uint64_t)ntohl(origin_tstamp->sec_lsb)) |
(((uint64_t)ntohs(origin_tstamp->sec_msb)) << 32);
+}
+
+static void
+send_delay_request(struct ptpv2_data_slave_ordinary *ptp_data)
+{
+   struct rte_ether_hdr *eth_hdr;
+   struct rte_ether_addr eth_addr;
+   struct ptp_header *ptp_hdr;
+   struct clock_id *client_clkid;
+   struct delay_req_msg *req_msg;
+   struct rte_mbuf *created_pkt;
+   struct rte_ether_addr eth_multicast = ether_multicast;
+   size_t pkt_size;
+   int wait_us;
+   struct rte_mbuf *m = ptp_data->m;
+   int ret;
+
+   eth_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+   ptp_hdr = (struct ptp_header *)(rte_pktmbuf_mtod(m, char *)
+   + sizeof(struct rte_ether_hdr));
 
if (ptp_data->seqID_FOLLOWUP == ptp_data->seqID_SYNC) {
ret = rte_eth_macaddr_get(ptp_data->portid, ð_addr);
@@ -550,12 +560,6 @@ parse_drsp(struct ptpv2_data_slave_ordinary *ptp_data)
((uint64_t)ntohl(rx_tstamp->sec_lsb)) |
(((uint64_t)ntohs(rx_tstamp->sec_msb)) << 32);
 
-   /* Evaluate the delta for adjustment. */
-   ptp_data->delta = delta_eval(ptp_data);
-
-   rte_eth_timesync_adjust_time(ptp_data->portid,
-ptp_data->delta);
-
ptp_data->current_ptp_port = ptp_data->portid;
 
/* Update kernel time if enabled in app parameters. */
@@ -568,6 +572,16 @@ parse_drsp(struct ptpv2_data_slave_ordinary *ptp_data)
}
 }
 
+static void
+ptp_adjust_time(struct ptpv2_data_slave_ordinary *ptp_data)
+{
+   /* Evaluate the delta for adjustment. */
+   ptp_data->delta = delta_eval(ptp_data);
+
+   rte_eth_timesync_adjust_time(ptp_data->portid,
+ptp_data->delta);
+}
+
 /* This function processes PTP packets, implementing slave PTP IEEE1588 L2
  * functionality.
  */
@@ -594,9 +608,11 @@ parse_ptp_frames(uint16_t portid, struct rte_mbuf *m) {
break;
case FOLLOW_UP:
parse_fup(&ptp_data);
+   send_delay_request(&ptp_data);
break;
case DELAY_RESP:
parse_drsp(&ptp_data);
+   ptp_adjust_time(&ptp_data);
print_clock_info(&ptp_data);
break;
default:
-- 
2.9.5

[RFC v2 3/3] examples/ptpclient: add frequency adjustment support

[Simei Su]

This patch applys PI servo algorithm to leverage frequency adjustment
API to improve PTP timesync accuracy.

The command for starting ptpclient with PI algorithm is:
./build/examples/dpdk-ptpclient -a :81:00.0 -c 1 -n 3 -- -T 0 -p 0x1
--controller=pi

Signed-off-by: Simei Su 
Signed-off-by: Wenjun Wu 
---
 examples/ptpclient/ptpclient.c | 178 +
 1 file changed, 161 insertions(+), 17 deletions(-)

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index 74a1bf5..3d074af 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -43,6 +43,28 @@
 #define KERNEL_TIME_ADJUST_LIMIT  2
 #define PTP_PROTOCOL 0x88F7
 
+#define KP 0.7
+#define KI 0.3
+
+enum servo_state {
+   SERVO_UNLOCKED,
+   SERVO_JUMP,
+   SERVO_LOCKED,
+};
+
+struct pi_servo {
+   double offset[2];
+   double local[2];
+   double drift;
+   int count;
+};
+
+enum controller_mode {
+   MODE_NONE,
+   MODE_PI,
+   MAX_ALL
+} mode;
+
 struct rte_mempool *mbuf_pool;
 uint32_t ptp_enabled_port_mask;
 uint8_t ptp_enabled_port_nb;
@@ -132,6 +154,9 @@ struct ptpv2_data_slave_ordinary {
uint8_t ptpset;
uint8_t kernel_time_set;
uint16_t current_ptp_port;
+   int64_t master_offset;
+   int64_t path_delay;
+   struct pi_servo *servo;
 };
 
 static struct ptpv2_data_slave_ordinary ptp_data;
@@ -290,36 +315,44 @@ print_clock_info(struct ptpv2_data_slave_ordinary 
*ptp_data)
ptp_data->tstamp3.tv_sec,
(ptp_data->tstamp3.tv_nsec));
 
-   printf("\nT4 - Master Clock.  %lds %ldns ",
+   printf("\nT4 - Master Clock.  %lds %ldns\n",
ptp_data->tstamp4.tv_sec,
(ptp_data->tstamp4.tv_nsec));
 
-   printf("\nDelta between master and slave clocks:%"PRId64"ns\n",
+   if (mode == MODE_NONE) {
+   printf("\nDelta between master and slave clocks:%"PRId64"ns\n",
ptp_data->delta);
 
-   clock_gettime(CLOCK_REALTIME, &sys_time);
-   rte_eth_timesync_read_time(ptp_data->current_ptp_port, &net_time);
+   clock_gettime(CLOCK_REALTIME, &sys_time);
+   rte_eth_timesync_read_time(ptp_data->current_ptp_port,
+  &net_time);
 
-   time_t ts = net_time.tv_sec;
+   time_t ts = net_time.tv_sec;
 
-   printf("\n\nComparison between Linux kernel Time and PTP:");
+   printf("\n\nComparison between Linux kernel Time and PTP:");
 
-   printf("\nCurrent PTP Time: %.24s %.9ld ns",
+   printf("\nCurrent PTP Time: %.24s %.9ld ns",
ctime(&ts), net_time.tv_nsec);
 
-   nsec = (int64_t)timespec64_to_ns(&net_time) -
+   nsec = (int64_t)timespec64_to_ns(&net_time) -
(int64_t)timespec64_to_ns(&sys_time);
-   ptp_data->new_adj = ns_to_timeval(nsec);
+   ptp_data->new_adj = ns_to_timeval(nsec);
+
+   gettimeofday(&ptp_data->new_adj, NULL);
 
-   gettimeofday(&ptp_data->new_adj, NULL);
+   time_t tp = ptp_data->new_adj.tv_sec;
 
-   time_t tp = ptp_data->new_adj.tv_sec;
+   printf("\nCurrent SYS Time: %.24s %.6ld ns",
+   ctime(&tp), ptp_data->new_adj.tv_usec);
 
-   printf("\nCurrent SYS Time: %.24s %.6ld ns",
-   ctime(&tp), ptp_data->new_adj.tv_usec);
+   printf("\nDelta between PTP and Linux Kernel 
time:%"PRId64"ns\n",
+   nsec);
+   }
 
-   printf("\nDelta between PTP and Linux Kernel time:%"PRId64"ns\n",
-   nsec);
+   if (mode == MODE_PI) {
+   printf("path delay: %"PRId64"ns\n", ptp_data->path_delay);
+   printf("master offset: %"PRId64"ns\n", ptp_data->master_offset);
+   }
 
printf("[Ctrl+C to quit]\n");
 
@@ -405,6 +438,76 @@ parse_fup(struct ptpv2_data_slave_ordinary *ptp_data)
(((uint64_t)ntohs(origin_tstamp->sec_msb)) << 32);
 }
 
+static double
+pi_sample(struct pi_servo *s, double offset, double local_ts,
+ enum servo_state *state)
+{
+   double ppb = 0.0;
+
+   switch (s->count) {
+   case 0:
+   s->offset[0] = offset;
+   s->local[0] = local_ts;
+   *state = SERVO_UNLOCKED;
+   s->count = 1;
+   break;
+   case 1:
+   s->offset[1] = offset;
+   s->local[1] = local_ts;
+   *state = SERVO_UNLOCKED;
+   s->count = 2;
+   break;
+   case 2:
+   s->drift += (s->offset[1] - s->offset[0]) /
+   (s->local[1] - s->local[0]);
+   *state = SERVO_UNLOCKED;
+   s->count = 3;
+   break;
+   case 3:
+   *state = SERVO_JUMP;
+ 

Re: Testpmd/l3fwd port shutdown failure on Arm Altra systems

[Juraj Linkeš]

Hello Qiming, Beilei,

Could you please help us debug this issue? Anything that would help with
getting to the bottom of anything that could go wrong during port
init/cleanup would be appreciated - extra eal/testpmd options or even code
changes (such as where could add extra debug messages).

Thanks,
Juraj

On Wed, Mar 8, 2023 at 7:25 AM Juraj Linkeš 
wrote:

> Hello Qiming, Beilei,
>
> Another reminder - are you looking at this by any chance?
>
> The high level short description is that testpmd/l3fwd breaks a link
> between two servers while VPP (using DPDK) doesn't. This leads us to
> believe there's a problem with testpmd/l3fwd/i40e driver in DPDK.
>
> Thanks,
> Juraj
>
> On Tue, Feb 21, 2023 at 12:18 PM Juraj Linkeš
>  wrote:
> >
> > Hi Qiming,
> >
> > Just a friendly reminder, would you please take a look?
> >
> > Thanks,
> > Juraj
> >
> >
> > On Tue, Feb 7, 2023 at 3:10 AM Xing, Beilei 
> wrote:
> > >
> > > Hi Qiming,
> > >
> > > Could you please help on this? Thanks.
> > >
> > > BR,
> > > Beilei
> > >
> > > > -Original Message-
> > > > From: Juraj Linkeš 
> > > > Sent: Monday, February 6, 2023 4:53 PM
> > > > To: Singh, Aman Deep ; Zhang, Yuying
> > > > ; Xing, Beilei 
> > > > Cc: dev@dpdk.org; Ruifeng Wang ; Zhang, Lijian
> > > > ; Honnappa Nagarahalli
> > > > 
> > > > Subject: Re: Testpmd/l3fwd port shutdown failure on Arm Altra systems
> > > >
> > > > Hello i40e and testpmd maintainers,
> > > >
> > > > A gentle reminder - would you please advise how to debug the issue
> described
> > > > below?
> > > >
> > > > Thanks,
> > > > Juraj
> > > >
> > > > On Fri, Jan 20, 2023 at 1:07 PM Juraj Linkeš
> 
> > > > wrote:
> > > > >
> > > > > Adding the logfile.
> > > > >
> > > > >
> > > > >
> > > > > One thing that's in the logs but didn't explicitly mention is the
> DPDK version
> > > > we've tried this with:
> > > > >
> > > > > EAL: RTE Version: 'DPDK 22.07.0'
> > > > >
> > > > >
> > > > >
> > > > > We also tried earlier versions going back to 21.08, with no luck.
> I also did a
> > > > quick check on 22.11, also with no luck.
> > > > >
> > > > >
> > > > >
> > > > > Juraj
> > > > >
> > > > >
> > > > >
> > > > > From: Juraj Linkeš
> > > > > Sent: Friday, January 20, 2023 12:56 PM
> > > > > To: 'aman.deep.si...@intel.com' ;
> > > > > 'yuying.zh...@intel.com' ; Xing, Beilei
> > > > > 
> > > > > Cc: dev@dpdk.org; Ruifeng Wang ; 'Lijian
> Zhang'
> > > > > ; 'Honnappa Nagarahalli'
> > > > > 
> > > > > Subject: Testpmd/l3fwd port shutdown failure on Arm Altra systems
> > > > >
> > > > >
> > > > >
> > > > > Hello i40e and testpmd maintainers,
> > > > >
> > > > >
> > > > >
> > > > > We're hitting an issue with DPDK testpmd on Ampere Altra servers
> in FD.io
> > > > lab.
> > > > >
> > > > >
> > > > >
> > > > > A bit of background: along with VPP performance tests (which uses
> DPDK),
> > > > we're running a small number of basic DPDK testpmd and l3fwd tests
> in FD.io
> > > > as well. This is to catch any performance differences due to VPP
> updating its
> > > > DPDK version.
> > > > >
> > > > >
> > > > >
> > > > > We're running both l3fwd tests and testpmd tests. The Altra
> servers are two
> > > > socket and the topology is TG -> DUT1 -> DUT2 -> TG, traffic flows
> in both
> > > > directions, but nothing gets forwarded (with a slight caveat - put a
> pin in this).
> > > > There's nothing special in the tests, just forwarding traffic. The
> NIC we're
> > > > testing is xl710-QDA2.
> > > > >
> > > > >
> > > > >
> > > > > The same tests are passing on all other testbeds - we have various
> two node
> > > > (1 DUT, 1 TG) and three node (2 DUT, 1 TG) Intel and Arm testbeds
> and with
> > > > various NICs (Intel 700 and 800 series and the Intel testbeds use
> some
> > > > Mellanox NICs as well). We don't have quite the same combination of
> another
> > > > three node topology with the same NIC though, so it looks like
> something with
> > > > testpmd/l3fwd and xl710-QDA2 on Altra servers.
> > > > >
> > > > >
> > > > >
> > > > > VPP performance tests are passing, but l3fwd and testpmd fail.
> This leads us
> > > > to believe to it's a software issue, but there could something wrong
> with the
> > > > hardware. I'll talk about testpmd from now on, but as far we can
> tell, the
> > > > behavior is the same for testpmd and l3fwd.
> > > > >
> > > > >
> > > > >
> > > > > Getting back to the caveat mentioned earlier, there seems to be
> something
> > > > wrong with port shutdown. When running testpmd on a testbed that
> hasn't
> > > > been used for a while it seems that all ports are up right away (we
> don't see
> > > > any "Port 0|1: link state change event") and the setup works fine
> (forwarding
> > > > works). After restarting testpmd (restarting on one server is
> sufficient), the
> > > > ports between DUT1 and DUT2 (but not between DUTs and TG) go down and
> > > > are not usable in DPDK, VPP or in Linux (with i40e kernel driver)
> for a while
> > > > (measured in minutes, sometimes dozens of minutes; the duration

[PATCH v2] version: 23.07-rc0

[David Marchand]

Start a new release cycle with empty release notes.
Bump version and ABI minor.

Signed-off-by: David Marchand 
---
Changes since v1:
- fix ABI reference git repository,

---
 .github/workflows/build.yml|   3 +-
 ABI_VERSION|   2 +-
 VERSION|   2 +-
 doc/guides/rel_notes/index.rst |   1 +
 doc/guides/rel_notes/release_23_07.rst | 138 +
 5 files changed, 142 insertions(+), 4 deletions(-)
 create mode 100644 doc/guides/rel_notes/release_23_07.rst

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index e24e47a216..edd39cbd62 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -26,8 +26,7 @@ jobs:
   MINGW: ${{ matrix.config.cross == 'mingw' }}
   MINI: ${{ matrix.config.mini != '' }}
   PPC64LE: ${{ matrix.config.cross == 'ppc64le' }}
-  REF_GIT_REPO: https://dpdk.org/git/dpdk-stable
-  REF_GIT_TAG: v22.11.1
+  REF_GIT_TAG: v23.03
   RISCV64: ${{ matrix.config.cross == 'riscv64' }}
   RUN_TESTS: ${{ contains(matrix.config.checks, 'tests') }}
 
diff --git a/ABI_VERSION b/ABI_VERSION
index a12b18e437..3c8ce91a46 100644
--- a/ABI_VERSION
+++ b/ABI_VERSION
@@ -1 +1 @@
-23.1
+23.2
diff --git a/VERSION b/VERSION
index 533bf9aa13..d3c78a13bf 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-23.03.0
+23.07.0-rc0
diff --git a/doc/guides/rel_notes/index.rst b/doc/guides/rel_notes/index.rst
index 57475a8158..d8dfa621ec 100644
--- a/doc/guides/rel_notes/index.rst
+++ b/doc/guides/rel_notes/index.rst
@@ -8,6 +8,7 @@ Release Notes
 :maxdepth: 1
 :numbered:
 
+release_23_07
 release_23_03
 release_22_11
 release_22_07
diff --git a/doc/guides/rel_notes/release_23_07.rst 
b/doc/guides/rel_notes/release_23_07.rst
new file mode 100644
index 00..a9b1293689
--- /dev/null
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -0,0 +1,138 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+   Copyright 2023 The DPDK contributors
+
+.. include:: 
+
+DPDK Release 23.07
+==
+
+.. **Read this first.**
+
+   The text in the sections below explains how to update the release notes.
+
+   Use proper spelling, capitalization and punctuation in all sections.
+
+   Variable and config names should be quoted as fixed width text:
+   ``LIKE_THIS``.
+
+   Build the docs and view the output file to ensure the changes are correct::
+
+  ninja -C build doc
+  xdg-open build/doc/guides/html/rel_notes/release_23_07.html
+
+
+New Features
+
+
+.. This section should contain new features added in this release.
+   Sample format:
+
+   * **Add a title in the past tense with a full stop.**
+
+ Add a short 1-2 sentence description in the past tense.
+ The description should be enough to allow someone scanning
+ the release notes to understand the new feature.
+
+ If the feature adds a lot of sub-features you can use a bullet list
+ like this:
+
+ * Added feature foo to do something.
+ * Enhanced feature bar to do something else.
+
+ Refer to the previous release notes for examples.
+
+ Suggested order in release notes items:
+ * Core libs (EAL, mempool, ring, mbuf, buses)
+ * Device abstraction libs and PMDs (ordered alphabetically by vendor name)
+   - ethdev (lib, PMDs)
+   - cryptodev (lib, PMDs)
+   - eventdev (lib, PMDs)
+   - etc
+ * Other libs
+ * Apps, Examples, Tools (if significant)
+
+ This section is a comment. Do not overwrite or remove it.
+ Also, make sure to start the actual text at the margin.
+ ===
+
+
+Removed Items
+-
+
+.. This section should contain removed items in this release. Sample format:
+
+   * Add a short 1-2 sentence description of the removed item
+ in the past tense.
+
+   This section is a comment. Do not overwrite or remove it.
+   Also, make sure to start the actual text at the margin.
+   ===
+
+
+API Changes
+---
+
+.. This section should contain API changes. Sample format:
+
+   * sample: Add a short 1-2 sentence description of the API change
+ which was announced in the previous releases and made in this release.
+ Start with a scope label like "ethdev:".
+ Use fixed width quotes for ``function_names`` or ``struct_names``.
+ Use the past tense.
+
+   This section is a comment. Do not overwrite or remove it.
+   Also, make sure to start the actual text at the margin.
+   ===
+
+
+ABI Changes
+---
+
+.. This section should contain ABI changes. Sample format:
+
+   * sample: Add a short 1-2 sentence description of the ABI change
+ which was announced in the previous releases and made in this release.
+ Start with a scope label like "ethdev:".
+ Use fixed width quotes for ``function_names`` or ``

Re: [RFC PATCH v1 0/4] dts: add dts api docs

[Bruce Richardson]

On Mon, Apr 03, 2023 at 11:17:06AM +0200, Juraj Linkeš wrote:
>Hi Bruce, Thomas,
>The meson integration is kinda all over the place. I wanted to use the
>existing conf.py Sphinx config file, but I also wanted to keep the docs
>separated (because of extra DTS api docs dependencies), so the various
>pieces are in different places (the config file in one place, meson
>code in dts directory and generated Sphinx docs are in a new directory
>in the api build dir, separate from the rest of the Sphinx html).
>The big thing here is that I didn't figure out how to separate the dts
>api build from the rest of the docs. I don't know how the -Denable_docs
>option is supposed to work. I wanted to use -Denable_dts_docs in the
>same fashion to decouple the builds, but it doesn't seem to work.
>Reading the code I think the original option doesn't actually do
>anything - does it work? How is it supposed to work?
>Thanks,
>Juraj

The enable_docs option works by selectively enabling the doc build tasks
using the "build_by_default" parameter on them. 
See http://git.dpdk.org/dpdk/tree/doc/guides/meson.build#n23 for an
example. The custom_target for sphinx is not a dependency of any other
task, so whether it gets run or not depends entirely on whether the
"build_by_default" and/or "install" options are set.

As usual, there may be other stuff that needs cleaning up on this, but
that's how it works for now, anyway. [And it does actually work, last I
tested it :-)]

/Bruce

Re: [PATCH v2 00/20] Enable lock annotations on most libraries and drivers

[David Marchand]

Hello Tyler,

On Thu, Mar 2, 2023 at 9:52 AM David Marchand  wrote:
> On Mon, Feb 27, 2023 at 5:13 PM Gaëtan Rivet  wrote:
> > Ah ok, so if I understand correctly, DPDK would need to declare some
> > '__rte_lockable rte_mutex' and associated functions for transparent support,
> > to wrap above the pthread API.
>
> Yes, this is what I had in mind for the mid/long term but it was too
> late for 23.03 after -rc1.
>
> The Windows porting effort will probably need this abstraction too as
> we are trying to stop relying on the pthread API.
> I don't see this item in Microsoft roadmap, though.

Do you have an opinion on this topic?

Thanks.

-- 
David Marchand

[PATCH v1 1/2] dts: fabric requirements

[Juraj Linkeš]

Replace pexpect with Fabric.

Signed-off-by: Juraj Linkeš 
---
 dts/poetry.lock| 553 +++--
 dts/pyproject.toml |   2 +-
 2 files changed, 490 insertions(+), 65 deletions(-)

diff --git a/dts/poetry.lock b/dts/poetry.lock
index 0b2a007d4d..a800efcba1 100644
--- a/dts/poetry.lock
+++ b/dts/poetry.lock
@@ -1,3 +1,5 @@
+# This file is automatically @generated by Poetry and should not be changed by 
hand.
+
 [[package]]
 name = "attrs"
 version = "22.1.0"
@@ -5,12 +7,51 @@ description = "Classes Without Boilerplate"
 category = "main"
 optional = false
 python-versions = ">=3.5"
+files = [
+{file = "attrs-22.1.0-py2.py3-none-any.whl", hash = 
"sha256:86efa402f67bf2df34f51a335487cf46b1ec130d02b8d39fd248abfd30da551c"},
+{file = "attrs-22.1.0.tar.gz", hash = 
"sha256:29adc2665447e5191d0e7c568fde78b21f9672d344281d0c6e1ab085429b22b6"},
+]
+
+[package.extras]
+dev = ["cloudpickle", "coverage[toml] (>=5.0.2)", "furo", "hypothesis", "mypy 
(>=0.900,!=0.940)", "pre-commit", "pympler", "pytest (>=4.3.0)", 
"pytest-mypy-plugins", "sphinx", "sphinx-notfound-page", "zope.interface"]
+docs = ["furo", "sphinx", "sphinx-notfound-page", "zope.interface"]
+tests = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy 
(>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", 
"zope.interface"]
+tests-no-zope = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", 
"mypy (>=0.900,!=0.940)", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins"]
+
+[[package]]
+name = "bcrypt"
+version = "4.0.1"
+description = "Modern password hashing for your software and your servers"
+category = "main"
+optional = false
+python-versions = ">=3.6"
+files = [
+{file = "bcrypt-4.0.1-cp36-abi3-macosx_10_10_universal2.whl", hash = 
"sha256:b1023030aec778185a6c16cf70f359cbb6e0c289fd564a7cfa29e727a1c38f8f"},
+{file = 
"bcrypt-4.0.1-cp36-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.manylinux_2_24_aarch64.whl",
 hash = 
"sha256:08d2947c490093a11416df18043c27abe3921558d2c03e2076ccb28a116cb6d0"},
+{file = 
"bcrypt-4.0.1-cp36-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash 
= "sha256:0eaa47d4661c326bfc9d08d16debbc4edf78778e6aaba29c1bc7ce67214d4410"},
+{file = 
"bcrypt-4.0.1-cp36-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = 
"sha256:ae88eca3024bb34bb3430f964beab71226e761f51b912de5133470b649d82344"},
+{file = "bcrypt-4.0.1-cp36-abi3-manylinux_2_24_x86_64.whl", hash = 
"sha256:a522427293d77e1c29e303fc282e2d71864579527a04ddcfda6d4f8396c6c36a"},
+{file = "bcrypt-4.0.1-cp36-abi3-manylinux_2_28_aarch64.whl", hash = 
"sha256:fbdaec13c5105f0c4e5c52614d04f0bca5f5af007910daa8b6b12095edaa67b3"},
+{file = "bcrypt-4.0.1-cp36-abi3-manylinux_2_28_x86_64.whl", hash = 
"sha256:ca3204d00d3cb2dfed07f2d74a25f12fc12f73e606fcaa6975d1f7ae69cacbb2"},
+{file = "bcrypt-4.0.1-cp36-abi3-musllinux_1_1_aarch64.whl", hash = 
"sha256:089098effa1bc35dc055366740a067a2fc76987e8ec75349eb9484061c54f535"},
+{file = "bcrypt-4.0.1-cp36-abi3-musllinux_1_1_x86_64.whl", hash = 
"sha256:e9a51bbfe7e9802b5f3508687758b564069ba937748ad7b9e890086290d2f79e"},
+{file = "bcrypt-4.0.1-cp36-abi3-win32.whl", hash = 
"sha256:2caffdae059e06ac23fce178d31b4a702f2a3264c20bfb5ff541b338194d8fab"},
+{file = "bcrypt-4.0.1-cp36-abi3-win_amd64.whl", hash = 
"sha256:8a68f4341daf7522fe8d73874de8906f3a339048ba406be6ddc1b3ccb16fc0d9"},
+{file = 
"bcrypt-4.0.1-pp37-pypy37_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", 
hash = 
"sha256:bf4fa8b2ca74381bb5442c089350f09a3f17797829d958fad058d6e44d9eb83c"},
+{file = "bcrypt-4.0.1-pp37-pypy37_pp73-manylinux_2_24_x86_64.whl", hash = 
"sha256:67a97e1c405b24f19d08890e7ae0c4f7ce1e56a712a016746c8b2d7732d65d4b"},
+{file = "bcrypt-4.0.1-pp37-pypy37_pp73-manylinux_2_28_x86_64.whl", hash = 
"sha256:b3b85202d95dd568efcb35b53936c5e3b3600c7cdcc6115ba461df3a8e89f38d"},
+{file = 
"bcrypt-4.0.1-pp38-pypy38_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", 
hash = 
"sha256:cbb03eec97496166b704ed663a53680ab57c5084b2fc98ef23291987b525cb7d"},
+{file = "bcrypt-4.0.1-pp38-pypy38_pp73-manylinux_2_24_x86_64.whl", hash = 
"sha256:5ad4d32a28b80c5fa6671ccfb43676e8c1cc232887759d1cd7b6f56ea4355215"},
+{file = "bcrypt-4.0.1-pp38-pypy38_pp73-manylinux_2_28_x86_64.whl", hash = 
"sha256:b57adba8a1444faf784394de3436233728a1ecaeb6e07e8c22c8848f179b893c"},
+{file = 
"bcrypt-4.0.1-pp39-pypy39_pp73-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", 
hash = 
"sha256:705b2cea8a9ed3d55b4491887ceadb0106acf7c6387699fca771af56b1cdeeda"},
+{file = "bcrypt-4.0.1-pp39-pypy39_pp73-manylinux_2_24_x86_64.whl", hash = 
"sha256:2b3ac11cf45161628f1f3733263e63194f22664bf4d0c0f3ab34099c02134665"},
+{file = "bcrypt-4.0.1-pp39-pypy39_pp73-manylinux_2_28_x86_64.whl", hash = 
"sha256:3100851841186c25f127731b9fa11909ab7b1df6fc4b9f8353f4f1fd952fbf71"},
+{file = "bcrypt-4.0.1.tar.gz", hash = 
"sha256:27d375903ac8261cfe4047f6709d16f7d18d39b

[PATCH v1 2/2] dts: replace pexpect with fabric

[Juraj Linkeš]

Pexpect is not a dedicated SSH connection library while Fabric is. With
Fabric, all SSH-related logic is provided and we can just focus on
what's DTS specific.

Signed-off-by: Juraj Linkeš 
---
 doc/guides/tools/dts.rst  |  29 +-
 dts/conf.yaml |   2 +-
 dts/framework/exception.py|  10 +-
 dts/framework/remote_session/linux_session.py |  31 +-
 dts/framework/remote_session/os_session.py|  51 +++-
 dts/framework/remote_session/posix_session.py |  48 +--
 .../remote_session/remote/remote_session.py   |  35 ++-
 .../remote_session/remote/ssh_session.py  | 287 ++
 dts/framework/testbed_model/sut_node.py   |  12 +-
 dts/framework/utils.py|   9 -
 10 files changed, 237 insertions(+), 277 deletions(-)

diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index ebd6dceb6a..d15826c098 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -95,9 +95,14 @@ Setting up DTS environment
 
 #. **SSH Connection**
 
-   DTS uses Python pexpect for SSH connections between DTS environment and the 
other hosts.
-   The pexpect implementation is a wrapper around the ssh command in the DTS 
environment.
-   This means it'll use the SSH agent providing the ssh command and its keys.
+   DTS uses the Fabric Python library for SSH connections between DTS 
environment
+   and the other hosts.
+   The authentication method used is pubkey authentication.
+   Fabric tries to use a passed key/certificate,
+   then any key it can with through an SSH agent,
+   then any "id_rsa", "id_dsa" or "id_ecdsa" key discoverable in ``~/.ssh/``
+   (with any matching OpenSSH-style certificates).
+   DTS doesn't pass any keys, so Fabric tries to use the other two methods.
 
 
 Setting up System Under Test
@@ -132,6 +137,21 @@ There are two areas that need to be set up on a System 
Under Test:
  It's possible to use the hugepage configuration already present on the 
SUT.
  If you wish to do so, don't specify the hugepage configuration in the DTS 
config file.
 
+#. **User with administrator privileges**
+
+.. _sut_admin_user:
+
+   DTS needs administrator privileges to run DPDK applications (such as 
testpmd) on the SUT.
+   The SUT user must be able run commands in privileged mode without asking 
for password.
+   On most Linux distributions, it's a matter of setting up passwordless sudo:
+
+   #. Run ``sudo visudo`` and check that it contains ``%sudo   ALL=(ALL:ALL) 
ALL``.
+
+   #. Add the SUT user to the sudo group with:
+
+   .. code-block:: console
+
+  sudo usermod -aG sudo 
 
 Running DTS
 ---
@@ -151,7 +171,8 @@ which is a template that illustrates what can be configured 
in DTS:
  :start-at: executions:
 
 
-The user must be root or any other user with prompt starting with ``#``.
+The user must have :ref:`administrator privileges `
+which don't require password authentication.
 The other fields are mostly self-explanatory
 and documented in more detail in 
``dts/framework/config/conf_yaml_schema.json``.
 
diff --git a/dts/conf.yaml b/dts/conf.yaml
index a9bd8a3ecf..129801d87c 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -16,7 +16,7 @@ executions:
 nodes:
   - name: "SUT 1"
 hostname: sut1.change.me.localhost
-user: root
+user: dtsuser
 arch: x86_64
 os: linux
 lcores: ""
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index ca353d98fc..44ff4e979a 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -62,13 +62,19 @@ class SSHConnectionError(DTSError):
 """
 
 host: str
+errors: list[str]
 severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
 
-def __init__(self, host: str):
+def __init__(self, host: str, errors: list[str] | None = None):
 self.host = host
+self.errors = [] if errors is None else errors
 
 def __str__(self) -> str:
-return f"Error trying to connect with {self.host}"
+message = f"Error trying to connect with {self.host}."
+if self.errors:
+message += f" Errors encountered while retrying: {', 
'.join(self.errors)}"
+
+return message
 
 
 class SSHSessionDeadError(DTSError):
diff --git a/dts/framework/remote_session/linux_session.py 
b/dts/framework/remote_session/linux_session.py
index a1e3bc3a92..f13f399121 100644
--- a/dts/framework/remote_session/linux_session.py
+++ b/dts/framework/remote_session/linux_session.py
@@ -14,10 +14,11 @@ class LinuxSession(PosixSession):
 The implementation of non-Posix compliant parts of Linux remote sessions.
 """
 
+def _get_privileged_command(self, command: str) -> str:
+return f"sudo -- sh -c '{command}'"
+
 def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:
-cpu_info = self.remote_session.send_command(
-"lscpu -p=CPU,CORE,SOCKET,NODE|grep -v \\#"
-).stdout
+cpu_info = self.send

Re: [RFC v2 2/2] eal: add high-performance timer facility

[Mattias Rönnblom]

On 2023-03-22 13:18, Morten Brørup wrote:
>> From: Mattias Rönnblom [mailto:mattias.ronnb...@ericsson.com]
>> Sent: Wednesday, 15 March 2023 18.04
> 
>> +++ b/lib/htimer/rte_htimer.h
>> @@ -0,0 +1,68 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2023 Ericsson AB
>> + */
>> +
>> +#ifndef _RTE_HTIMER_H_
>> +#define _RTE_HTIMER_H_
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +struct rte_htimer;
>> +
>> +typedef void (*rte_htimer_cb_t)(struct rte_htimer *, void *);
>> +
>> +struct rte_htimer {
>> +/**
>> + * Absolute timer expiration time (in ticks).
>> + */
>> +uint64_t expiration_time;
>> +/**
>> + * Time between expirations (in ticks). Zero for one-shot timers.
>> + */
>> +uint64_t period;
>> +/**
>> + * Owning lcore. May safely be read from any thread.
>> + */
>> +uint32_t owner_lcore_id;
>> +/**
>> + * The current state of the timer.
>> + */
>> +uint32_t state:4;
>> +/**
>> + * Flags set on this timer.
>> + */
>> +uint32_t flags:28;
>> +/**
>> + * User-specified callback function pointer.
>> + */
>> +rte_htimer_cb_t cb;
>> +/**
>> + * Argument for user callback.
>> + */
>> +void *cb_arg;
>> +/**
>> + * Pointers used to add timer to various internal lists.
>> + */
>> +LIST_ENTRY(rte_htimer) entry;
>> +};
> 
> If the rte_htimer structure is supposed to be used in some other data 
> structure, e.g. in a TCP/IP flow structure, it seems unnecessarily bloated.
> 
> Generally, if there is no significant performance benefit to the "period" 
> feature, please remove it.
> 
> Let's say that this library is used for handling the timers of flows in an IP 
> stack, then the vast majority of timers will be timers related to flows. I 
> would prefer if this high-performance timer library is optimized for such 
> high-volume use cases, rather than offering generic features for low-volume 
> use cases.
> 
> And if one HTW instance is used for a single purpose (e.g. the IP stack state 
> machine), both "cb" and "cb_arg" can be removed: The application can derive 
> the pointer to the flow by the using container_of() with the pointer to the 
> rte_htimer, and the cb_arg will effectively be a shadow variable of the 
> flow's state anyway (if not just a pointer to the flow).
> 
> Here's an idea, which will offer both: For the high-volume single-purpose use 
> cases you could provide a struct rte_htimer_core without the generic fields, 
> and for the generic use cases, you could provide a struct rte_htimer 
> containing a struct rte_htimer_core and the additional fields for generic use.
> 
>> 

Good points.

I will look into:
a) making  public
b) split rte_htimer into two timer structs (where the now-public 
rte_htw_timer struct may be used from the rte_htimer_timer struct).
c) ...where the htw timer struct won't have any callbacks
d) merge rte_htimer_timer.h into rte_htimer.h.
e) remove the periodic feature, at least from the core timer wheel

+
>> +#define RTE_HTIMER_FLAG_ABSOLUTE_TIME RTE_BIT32(0)
>> +#define RTE_HTIMER_FLAG_PERIODICAL RTE_BIT32(1)
>> +#define RTE_HTIMER_FLAG_TIME_TICK RTE_BIT32(2)
>> +#define RTE_HTIMER_FLAG_TIME_TSC RTE_BIT32(3)
>> +
>> +#define RTE_HTIMER_STATE_PENDING 1
>> +#define RTE_HTIMER_STATE_EXPIRED 2
>> +#define RTE_HTIMER_STATE_CANCELED 3
>> +
>> +LIST_HEAD(rte_htimer_list, rte_htimer);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif /* _RTE_HTIMER_H_ */

Re: [PATCH v1 1/2] dts: fabric requirements

[Thomas Monjalon]

03/04/2023 13:46, Juraj Linkeš:
> Replace pexpect with Fabric.

You should squash these lines with the move to Fabric.

> Signed-off-by: Juraj Linkeš 
> ---
>  dts/poetry.lock| 553 +++--

Do we really need *all* these lines?
I see a lot of lines about Windows and MacOSX which are not supported in DTS.
It is so long that it looks impossible to review.

[PATCH 1/2] eventdev: add bulk type event ring operations

[Mattias Rönnblom]

Introduce bulk enqueue and dequeue operations into the
 API, to supplement the already-existing burst
calls.

Signed-off-by: Mattias Rönnblom 
---
 lib/eventdev/rte_event_ring.h | 74 +++
 1 file changed, 74 insertions(+)

diff --git a/lib/eventdev/rte_event_ring.h b/lib/eventdev/rte_event_ring.h
index 7efa6b..f9cf19ae16 100644
--- a/lib/eventdev/rte_event_ring.h
+++ b/lib/eventdev/rte_event_ring.h
@@ -67,6 +67,80 @@ rte_event_ring_free_count(const struct rte_event_ring *r)
return rte_ring_free_count(&r->r);
 }
 
+
+/**
+ * Enqueue several objects on a ring.
+ *
+ * This function calls the multi-producer or the single-producer
+ * version depending on the default behavior that was specified at
+ * ring creation time (see flags).
+ *
+ * @param r
+ *   pointer to the event ring
+ * @param events
+ *   pointer to an array of struct rte_event objects
+ * @param n
+ *   The number of events in the array to enqueue
+ * @param free_space
+ *   if non-NULL, returns the amount of space in the ring after the
+ *   enqueue operation has completed
+ * @return
+ *   the number of objects enqueued, either 0 or n
+ */
+static __rte_always_inline unsigned int
+rte_event_ring_enqueue_bulk(struct rte_event_ring *r,
+   const struct rte_event *events,
+   unsigned int n, uint16_t *free_space)
+{
+   unsigned int num;
+   uint32_t space;
+
+   num = rte_ring_enqueue_bulk_elem(&r->r, events,
+sizeof(struct rte_event), n,
+&space);
+
+   if (free_space != NULL)
+   *free_space = space;
+
+   return num;
+}
+
+/**
+ * Dequeue a set of events from a ring
+ *
+ * Note: this API does not work with pointers to events, rather it copies
+ * the events themselves to the destination ``events`` buffer.
+ *
+ * @param r
+ *   pointer to the event ring
+ * @param events
+ *   pointer to an array to hold the struct rte_event objects
+ * @param n
+ *   number of events that can be held in the ``events`` array
+ * @param available
+ *   if non-null, is updated to indicate the number of events remaining in
+ *   the ring once the dequeue has completed
+ * @return
+ *   the number of objects dequeued, either 0 or n
+ */
+static __rte_always_inline unsigned int
+rte_event_ring_dequeue_bulk(struct rte_event_ring *r,
+struct rte_event *events,
+unsigned int n, uint16_t *available)
+{
+   unsigned int num;
+   uint32_t remaining;
+
+   num = rte_ring_dequeue_bulk_elem(&r->r, events,
+sizeof(struct rte_event), n,
+&remaining);
+
+   if (available != NULL)
+   *available = remaining;
+
+   return num;
+}
+
 /**
  * Enqueue a set of events onto a ring
  *
-- 
2.34.1

[PATCH 2/2] event/dsw: replace burst with bulk enqueue

[Mattias Rönnblom]

An enqueue operation to a DSW port's input ring is guaranteed to
succeed, an thus a bulk type enqueue (instead of a burst enqueue) may
be used. There is also need not check the return code of such calls.

This change shaves off a handful of CPU cycles worth of latency per
enqueued event.

Signed-off-by: Mattias Rönnblom 
---
 drivers/event/dsw/dsw_event.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/event/dsw/dsw_event.c b/drivers/event/dsw/dsw_event.c
index 9932caf2ee..90d298a255 100644
--- a/drivers/event/dsw/dsw_event.c
+++ b/drivers/event/dsw/dsw_event.c
@@ -590,7 +590,6 @@ dsw_port_transmit_buffered(struct dsw_evdev *dsw, struct 
dsw_port *source_port,
struct dsw_port *dest_port = &(dsw->ports[dest_port_id]);
uint16_t *buffer_len = &source_port->out_buffer_len[dest_port_id];
struct rte_event *buffer = source_port->out_buffer[dest_port_id];
-   uint16_t enqueued = 0;
 
if (*buffer_len == 0)
return;
@@ -598,13 +597,8 @@ dsw_port_transmit_buffered(struct dsw_evdev *dsw, struct 
dsw_port *source_port,
/* The rings are dimensioned to fit all in-flight events (even
 * on a single ring), so looping will work.
 */
-   do {
-   enqueued +=
-   rte_event_ring_enqueue_burst(dest_port->in_ring,
-buffer+enqueued,
-*buffer_len-enqueued,
-NULL);
-   } while (unlikely(enqueued != *buffer_len));
+   rte_event_ring_enqueue_bulk(dest_port->in_ring, buffer, *buffer_len,
+   NULL);
 
(*buffer_len) = 0;
 }
-- 
2.34.1

[PATCH v2] devtools: add script to check for non inclusive naming

[Stephen Hemminger]

Shell script to find use of words that not be used.
By default it prints matches.  The -q (quiet) option
is used to just count. There is also -l option
which lists lines matching (like grep -l).

Uses the word lists from Inclusive Naming Initiative
see https://inclusivenaming.org/word-lists/

Examples:
 $ ./devtools/check-naming-policy.sh -q
 Total files: 37 errors, 90 warnings, 2 suggestions

 $ ./devtools/check-naming-policy.sh -q -l lib/eal
 Total lines: 32 errors, 8 warnings, 0 suggestions

Add MAINTAINERS file entry for the new tool and resort
the list files back into to alphabetic order

Signed-off-by: Stephen Hemminger 
---
v2 - fix typo in words
   - add subtree (pathspec) option
   - update maintainers file (and fix alphabetic order)

 MAINTAINERS |   8 ++-
 devtools/check-naming-policy.sh | 107 
 devtools/naming/tier1.txt   |   8 +++
 devtools/naming/tier2.txt   |   1 +
 devtools/naming/tier3.txt   |   4 ++
 5 files changed, 125 insertions(+), 3 deletions(-)
 create mode 100755 devtools/check-naming-policy.sh
 create mode 100644 devtools/naming/tier1.txt
 create mode 100644 devtools/naming/tier2.txt
 create mode 100644 devtools/naming/tier3.txt

diff --git a/MAINTAINERS b/MAINTAINERS
index 8df23e50999f..b5881113ba85 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -83,26 +83,28 @@ Developers and Maintainers Tools
 M: Thomas Monjalon 
 F: MAINTAINERS
 F: devtools/build-dict.sh
-F: devtools/check-abi.sh
 F: devtools/check-abi-version.sh
+F: devtools/check-abi.sh
 F: devtools/check-doc-vs-code.sh
 F: devtools/check-dup-includes.sh
-F: devtools/check-maintainers.sh
 F: devtools/check-forbidden-tokens.awk
 F: devtools/check-git-log.sh
+F: devtools/check-maintainers.sh
+F: devtools/check-naming-policy.sh
 F: devtools/check-spdx-tag.sh
 F: devtools/check-symbol-change.sh
 F: devtools/check-symbol-maps.sh
 F: devtools/checkpatches.sh
 F: devtools/get-maintainer.sh
 F: devtools/git-log-fixes.sh
+F: devtools/libabigail.abignore
 F: devtools/load-devel-config
+F: devtools/naming/
 F: devtools/parse-flow-support.sh
 F: devtools/process-iwyu.py
 F: devtools/update-abi.sh
 F: devtools/update-patches.py
 F: devtools/update_version_map_abi.py
-F: devtools/libabigail.abignore
 F: devtools/words-case.txt
 F: license/
 F: .editorconfig
diff --git a/devtools/check-naming-policy.sh b/devtools/check-naming-policy.sh
new file mode 100755
index ..90347b415652
--- /dev/null
+++ b/devtools/check-naming-policy.sh
@@ -0,0 +1,107 @@
+#! /bin/bash
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2023 Stephen Hemminger
+#
+# This script scans the source tree and creates list of files
+# containing words that are recommended to bavoide by the
+# Inclusive Naming Initiative.
+# See: https://inclusivenaming.org/word-lists/
+#
+# The options are:
+#   -q = quiet mode, produces summary count only
+#   -l = show lines instead of files with recommendations
+#   -v = verbose, show a header between each tier
+#
+# Default is to scan all of DPDK source and documentation.
+# Optional pathspec can be used to limit specific tree.
+#
+#  Example:
+#check-naming-policy.sh -q doc/*
+#
+
+errors=0
+warnings=0
+suggestions=0
+quiet=false
+veborse=false
+lines='-l'
+
+print_usage () {
+echo "usage: $(basename $0) [-l] [-q] [-v] []"
+exit 1
+}
+
+# Locate word list files
+selfdir=$(dirname $(readlink -f $0))
+words=$selfdir/naming
+
+# These give false positives
+skipfiles=( ':^devtools/naming/' \
+   ':^doc/guides/rel_notes/' \
+   ':^doc/guides/contributing/coding_style.rst' \
+   ':^doc/guides/prog_guide/glossary.rst' \
+)
+# These are obsolete
+skipfiles+=( \
+   ':^drivers/net/liquidio/' \
+   ':^drivers/net/bnx2x/' \
+   ':^lib/table/' \
+   ':^lib/port/' \
+   ':^lib/pipeline/' \
+   ':^examples/pipeline/' \
+)
+
+#
+# check_wordlist wordfile description
+check_wordlist() {
+local list=$words/$1
+local description=$2
+
+git grep -i $lines -f $list -- ${skipfiles[@]} $pathspec > $tmpfile
+count=$(wc -l < $tmpfile)
+if ! $quiet; then
+   if [ $count -gt 0 ]; then
+   if $verbose; then
+   echo $description
+   echo $description | tr '[:print:]' '-'
+   fi
+   cat $tmpfile
+   echo
+   fi
+fi
+return $count
+}
+
+while getopts lqvh ARG ; do
+   case $ARG in
+   l ) lines= ;;
+   q ) quiet=true ;;
+   v ) verbose=true ;;
+   h ) print_usage ; exit 0 ;;
+   ? ) print_usage ; exit 1 ;;
+   esac
+done
+shift $(($OPTIND - 1))
+
+tmpfile=$(mktemp -t dpdk.checknames.XX)
+trap 'rm -f -- "$tmpfile"' INT TERM HUP EXIT
+
+pathspec=$*
+
+check_wordlist tier1.txt "Tier 1: Replace immediately"
+errors=$?
+
+check_wordlist tier2.txt "Tier 2: Strongly consider replacing"
+warnings=$?
+
+check_wordlist tier3.txt "Tier 3: Recommend 

Re: [EXT] [PATCH] vhost: add device op to offload the interrupt kick

[Eelco Chaudron]

On 27 Mar 2023, at 18:35, Maxime Coquelin wrote:

> On 3/27/23 18:04, Eelco Chaudron wrote:
>>
>>
>> On 27 Mar 2023, at 17:16, Gowrishankar Muthukrishnan wrote:
>>
>>> Hi Eelco,
>>>
 +void
 +rte_vhost_notify_guest(int vid, uint16_t queue_id) {
 +  struct virtio_net *dev = get_device(vid);
 +  struct vhost_virtqueue *vq;
 +
 +  if (!dev ||  queue_id >= VHOST_MAX_VRING)
 +  return;
 +
 +  vq = dev->virtqueue[queue_id];
 +  if (!vq)
 +  return;
 +
 +  rte_spinlock_lock(&vq->access_lock);
 +
>>>
>>> Is spin lock needed here before system call ?
>>
>> I assumed access_lock is protecting all the following fields in this 
>> structure, so I need the lock to read the vq->callfd, however, I can/should 
>> move the eventfd_write outside of the lock.
>
> The FD might be closed between the check and the call to eventfd_write
> though, but I agree this is not optimal to call the eventfd_write under
> the spinlock in your case, as you will block the pmd thread if it tries
> to enqueue/dequeue packets on this queue, defeating the purpose of this
> patch.
>
> Maybe the solution is to change to read-write locks for the access_lock
> spinlock. The datapath (rte_vhost_enqueue_burst/rte_vhost_dequeue_burst)
> and this API would use the read version, meaning they won't lock each
> other, and the control path (lib/vhost/vhost_user.c) will use the write
> version.
>
> Does that make sense?

Hi Maxime, I prepped a patch, but not the read/write part yet, 
https://github.com/chaudron/dpdk/commit/d51c93b4ff08b43daed33e3c0fee193a6d039c25#.

I was thinking that even a read/write lock does not work (or maybe we need a 
combination of taking the read and write lock). As we need to update 
statistics, which need protection.
For example here you see the split (with just two locks, but the sys call could 
be called in the read lock):

https://github.com/chaudron/dpdk/blob/d51c93b4ff08b43daed33e3c0fee193a6d039c25/lib/vhost/vhost.c#L1493

The best would be to not have a lock when calling the system call, but that 
does not seem safe. I do not see a clear solution, guess also as I have some 
trouble understanding the lock rules around vq’s.

Do you have some more insights to share? I can ping you offline and discuss 
this.

Cheers,

Eelco

>>
 +  if (vq->callfd >= 0)
 +  eventfd_write(vq->callfd, (eventfd_t)1);
 +
 +  rte_spinlock_unlock(&vq->access_lock);
 +}
 +
>>>
>>> Thanks.
>>

Re: [PATCH v1 1/2] dts: fabric requirements

[Juraj Linkeš]

On Mon, Apr 3, 2023 at 2:33 PM Thomas Monjalon  wrote:

> 03/04/2023 13:46, Juraj Linkeš:
> > Replace pexpect with Fabric.
>
> You should squash these lines with the move to Fabric.
>
> > Signed-off-by: Juraj Linkeš 
> > ---
> >  dts/poetry.lock| 553 +++--
>
> Do we really need *all* these lines?
> I see a lot of lines about Windows and MacOSX which are not supported in
> DTS.
> It is so long that it looks impossible to review.
>
>
This is a generated file and doesn't need to be reviewed. I separated the
dependencies part so that the code part is easier to review. If you want, I
can squash the two commits.

Re: [PATCH v2 00/20] Enable lock annotations on most libraries and drivers

[Tyler Retzlaff]

On Mon, Apr 03, 2023 at 12:52:06PM +0200, David Marchand wrote:
> Hello Tyler,
> 
> On Thu, Mar 2, 2023 at 9:52 AM David Marchand  
> wrote:
> > On Mon, Feb 27, 2023 at 5:13 PM Gaëtan Rivet  wrote:
> > > Ah ok, so if I understand correctly, DPDK would need to declare some
> > > '__rte_lockable rte_mutex' and associated functions for transparent 
> > > support,
> > > to wrap above the pthread API.
> >
> > Yes, this is what I had in mind for the mid/long term but it was too
> > late for 23.03 after -rc1.
> >
> > The Windows porting effort will probably need this abstraction too as
> > we are trying to stop relying on the pthread API.
> > I don't see this item in Microsoft roadmap, though.
> 
> Do you have an opinion on this topic?

Sorry David I got distracted I'll review the thread again.  I think with
Windows toolsets we left off with expanding empty for now?

Anyway, I'll take another pass today if I get a chance.

> 
> Thanks.
> 
> -- 
> David Marchand

Re: [PATCH v1 1/2] dts: fabric requirements

[Thomas Monjalon]

03/04/2023 16:56, Juraj Linkeš:
> On Mon, Apr 3, 2023 at 2:33 PM Thomas Monjalon  wrote:
> 
> > 03/04/2023 13:46, Juraj Linkeš:
> > > Replace pexpect with Fabric.
> >
> > You should squash these lines with the move to Fabric.
> >
> > > Signed-off-by: Juraj Linkeš 
> > > ---
> > >  dts/poetry.lock| 553 +++--
> >
> > Do we really need *all* these lines?
> > I see a lot of lines about Windows and MacOSX which are not supported in
> > DTS.
> > It is so long that it looks impossible to review.
> >
> >
> This is a generated file and doesn't need to be reviewed.

In general, I don't like storing generated files.

> I separated the
> dependencies part so that the code part is easier to review. If you want, I
> can squash the two commits.

What happens if we manually remove the useless lines?

Re: [EXT] [PATCH] vhost: add device op to offload the interrupt kick

[Maxime Coquelin]

Hi Eelco,

On 4/3/23 16:51, Eelco Chaudron wrote:



On 27 Mar 2023, at 18:35, Maxime Coquelin wrote:


On 3/27/23 18:04, Eelco Chaudron wrote:



On 27 Mar 2023, at 17:16, Gowrishankar Muthukrishnan wrote:


Hi Eelco,


+void
+rte_vhost_notify_guest(int vid, uint16_t queue_id) {
+   struct virtio_net *dev = get_device(vid);
+   struct vhost_virtqueue *vq;
+
+   if (!dev ||  queue_id >= VHOST_MAX_VRING)
+   return;
+
+   vq = dev->virtqueue[queue_id];
+   if (!vq)
+   return;
+
+   rte_spinlock_lock(&vq->access_lock);
+


Is spin lock needed here before system call ?


I assumed access_lock is protecting all the following fields in this structure, so 
I need the lock to read the vq->callfd, however, I can/should move the 
eventfd_write outside of the lock.


The FD might be closed between the check and the call to eventfd_write
though, but I agree this is not optimal to call the eventfd_write under
the spinlock in your case, as you will block the pmd thread if it tries
to enqueue/dequeue packets on this queue, defeating the purpose of this
patch.

Maybe the solution is to change to read-write locks for the access_lock
spinlock. The datapath (rte_vhost_enqueue_burst/rte_vhost_dequeue_burst)
and this API would use the read version, meaning they won't lock each
other, and the control path (lib/vhost/vhost_user.c) will use the write
version.

Does that make sense?


Hi Maxime, I prepped a patch, but not the read/write part yet, 
https://github.com/chaudron/dpdk/commit/d51c93b4ff08b43daed33e3c0fee193a6d039c25#.

I was thinking that even a read/write lock does not work (or maybe we need a 
combination of taking the read and write lock). As we need to update 
statistics, which need protection.
For example here you see the split (with just two locks, but the sys call could 
be called in the read lock):

https://github.com/chaudron/dpdk/blob/d51c93b4ff08b43daed33e3c0fee193a6d039c25/lib/vhost/vhost.c#L1493

The best would be to not have a lock when calling the system call, but that 
does not seem safe. I do not see a clear solution, guess also as I have some 
trouble understanding the lock rules around vq’s.


The lock is indeed required. Maybe we can use read-lock, and use atomic
operations for counters that could be accessed by several threads?



Do you have some more insights to share? I can ping you offline and discuss 
this.


Sure, I'll be happy to discuss more about this.

Thanks,
Maxime


Cheers,

Eelco




+   if (vq->callfd >= 0)
+   eventfd_write(vq->callfd, (eventfd_t)1);
+
+   rte_spinlock_unlock(&vq->access_lock);
+}
+


Thanks.

Re: [PATCH v2 00/20] Enable lock annotations on most libraries and drivers

[Tyler Retzlaff]

On Mon, Apr 03, 2023 at 12:52:06PM +0200, David Marchand wrote:
> Hello Tyler,
> 
> On Thu, Mar 2, 2023 at 9:52 AM David Marchand  
> wrote:
> > On Mon, Feb 27, 2023 at 5:13 PM Gaëtan Rivet  wrote:
> > > Ah ok, so if I understand correctly, DPDK would need to declare some
> > > '__rte_lockable rte_mutex' and associated functions for transparent 
> > > support,
> > > to wrap above the pthread API.
> >
> > Yes, this is what I had in mind for the mid/long term but it was too
> > late for 23.03 after -rc1.
> >
> > The Windows porting effort will probably need this abstraction too as
> > we are trying to stop relying on the pthread API.
> > I don't see this item in Microsoft roadmap, though.
> 
> Do you have an opinion on this topic?

Okay, trying to grok the question here. If the question is do we want to
introduce a mutex/condition variable and lock/unlock signal/wait
abstraction?

I would certainly like to see reduced conditional compilation that
applications have to perform for the platform features. I also really
would like to purge the remaining pthread_{mutex,condvar} shim since it
is unsightly.

With msvc I think we could probably achieve the same with C11 threads but
I haven't investigated feasability and said with no investigation older
glibc may not provide the optional feature.

In the absence of C11 threads we can provide an rte_ abstraction but I
don't think I can put it on the roadmap until basic msvc support is
stood up (a question of resource prioritization as always).

I could commit to looking at it once msvc and atomics changes are merged
the earliest possible time frame for that is the start of the 23.11
cycle. If that happens at decent velocity I could even see adding it to
23.11 roadmap.

For now if someone else decides to introduce an abstraction I would just
caution strongly not to remove __rte_experimental from any API added
until I get a chance to focus on it.

ty

> 
> Thanks.
> 
> -- 
> David Marchand

[PATCH 0/2] improve code portability

[Tyler Retzlaff]

Improve portability of telemetry code to allow it to be compiled by msvc
unconditionally.

Remove use of VLA and instead dynamically allocate. MSVC will never
implement VLAs due to misuse / security concerns. 

Remove use of ranged based initializer (a gcc extension) instead just
explicitly initialize individual array elements.

Tyler Retzlaff (2):
  telemetry: use malloc instead of variable length array
  telemetry: use portable syntax to initialize array

 lib/telemetry/telemetry_data.c | 20 ++--
 lib/telemetry/telemetry_json.h | 32 +++-
 2 files changed, 37 insertions(+), 15 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] telemetry: use malloc instead of variable length array

[Tyler Retzlaff]

Replace use of variable length array optional standard feature to
improve portability.

Signed-off-by: Tyler Retzlaff 
---
 lib/telemetry/telemetry_json.h | 32 +++-
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h
index 744bbfe..c375e97 100644
--- a/lib/telemetry/telemetry_json.h
+++ b/lib/telemetry/telemetry_json.h
@@ -30,18 +30,23 @@
 static inline int
 __json_snprintf(char *buf, const int len, const char *format, ...)
 {
-   char tmp[len];
+   char *tmp = malloc(len);
va_list ap;
-   int ret;
+   int ret = 0;
+
+   if (tmp == NULL)
+   return ret;
 
va_start(ap, format);
ret = vsnprintf(tmp, sizeof(tmp), format, ap);
va_end(ap);
if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
strcpy(buf, tmp);
-   return ret;
}
-   return 0; /* nothing written or modified */
+
+   free(tmp);
+
+   return ret;
 }
 
 static const char control_chars[0x20] = {
@@ -60,13 +65,17 @@
 static inline int
 __json_format_str(char *buf, const int len, const char *prefix, const char 
*str, const char *suffix)
 {
-   char tmp[len];
int tmpidx = 0;
+   int ret = 0;
+   char *tmp = malloc(len);
+
+   if (tmp == NULL)
+   return ret;
 
while (*prefix != '\0' && tmpidx < len)
tmp[tmpidx++] = *prefix++;
if (tmpidx >= len)
-   return 0;
+   goto cleanup;
 
while (*str != '\0') {
if (*str < (int)RTE_DIM(control_chars)) {
@@ -85,19 +94,24 @@
 * escaped character like "\n" without overflowing
 */
if (tmpidx > len - 2)
-   return 0;
+   goto cleanup;
str++;
}
 
while (*suffix != '\0' && tmpidx < len)
tmp[tmpidx++] = *suffix++;
if (tmpidx >= len)
-   return 0;
+   goto cleanup;
 
tmp[tmpidx] = '\0';
 
strcpy(buf, tmp);
-   return tmpidx;
+   ret = tmpidx;
+
+cleanup:
+   free(tmp);
+
+   return ret;
 }
 
 /* Copies an empty array into the provided buffer. */
-- 
1.8.3.1

[PATCH 2/2] telemetry: use portable syntax to initialize array

[Tyler Retzlaff]

Use of ranges in designated initialization are a non-standard gcc
extension. Manually expand the ranges used in designated initialization
to eliminate the use of non-standard features.

Signed-off-by: Tyler Retzlaff 
---
 lib/telemetry/telemetry_data.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
index 2bac2de..87e5f18 100644
--- a/lib/telemetry/telemetry_data.c
+++ b/lib/telemetry/telemetry_data.c
@@ -152,12 +152,20 @@
 static bool
 valid_name(const char *name)
 {
-   char allowed[128] = {
-   ['0' ... '9'] = 1,
-   ['A' ... 'Z'] = 1,
-   ['a' ... 'z'] = 1,
-   ['_'] = 1,
-   ['/'] = 1,
+   static const char allowed[128] = {
+   ['0'] = 1, ['1'] = 1, ['2'] = 1, ['3'] = 1, ['4'] = 1,
+   ['5'] = 1, ['6'] = 1, ['7'] = 1, ['8'] = 1, ['9'] = 1,
+   ['A'] = 1, ['B'] = 1, ['C'] = 1, ['D'] = 1, ['E'] = 1,
+   ['F'] = 1, ['G'] = 1, ['H'] = 1, ['I'] = 1, ['J'] = 1,
+   ['K'] = 1, ['L'] = 1, ['M'] = 1, ['N'] = 1, ['O'] = 1,
+   ['P'] = 1, ['Q'] = 1, ['R'] = 1, ['S'] = 1, ['T'] = 1,
+   ['U'] = 1, ['V'] = 1, ['W'] = 1, ['X'] = 1, ['Y'] = 1,
+   ['Z'] = 1, ['a'] = 1, ['b'] = 1, ['c'] = 1, ['d'] = 1,
+   ['e'] = 1, ['f'] = 1, ['g'] = 1, ['h'] = 1, ['i'] = 1,
+   ['j'] = 1, ['k'] = 1, ['l'] = 1, ['m'] = 1, ['n'] = 1,
+   ['o'] = 1, ['p'] = 1, ['q'] = 1, ['r'] = 1, ['s'] = 1,
+   ['t'] = 1, ['u'] = 1, ['v'] = 1, ['w'] = 1, ['x'] = 1,
+   ['y'] = 1, ['z'] = 1, ['_'] = 1, ['/'] = 1,
};
while (*name != '\0') {
if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name] == 
0)
-- 
1.8.3.1

Re: [PATCH 0/2] improve code portability

[Bruce Richardson]

On Mon, Apr 03, 2023 at 09:30:22AM -0700, Tyler Retzlaff wrote:
> Improve portability of telemetry code to allow it to be compiled by msvc
> unconditionally.
> 
> Remove use of VLA and instead dynamically allocate. MSVC will never
> implement VLAs due to misuse / security concerns. 
> 
> Remove use of ranged based initializer (a gcc extension) instead just
> explicitly initialize individual array elements.
> 
> Tyler Retzlaff (2):
>   telemetry: use malloc instead of variable length array
>   telemetry: use portable syntax to initialize array
> 
Is this worth doing, given that DPDK telemetry uses a unix domain socket
for it's connectivity, which would not be available on windows anyway?
I don't particularly like these patches as:
* The removal of the VLAs means we will potentially be doing a *lot* of
  malloc and free calls inside the telemetry code. It may not be a data
  path or particularly performance critical, but I know for things like CPU
  busyness, users may want to call telemetry functions hundreds (or
  potentially thousands) of times a second. It also makes the code slightly
  harder to read, and introduces the possibility of us having memory leaks.
* The second patch just makes the code uglier. True, it's non-standard, but
  it really does make the code a whole lot more readable and managable. If
  we need to make this standards-conforming, then I think we need to drop
  the "const", and do runtime init of this array with loops for the ranges.

All that said, if we do have a path to get telemetry working on windows, I
think we can work together to get a suitable patchset in for it.

/Bruce

Re: [PATCH 1/2] telemetry: use malloc instead of variable length array

[Tyler Retzlaff]

On Mon, Apr 03, 2023 at 09:30:23AM -0700, Tyler Retzlaff wrote:
> Replace use of variable length array optional standard feature to
> improve portability.
> 
> Signed-off-by: Tyler Retzlaff 
> ---
>  lib/telemetry/telemetry_json.h | 32 +++-
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h
> index 744bbfe..c375e97 100644
> --- a/lib/telemetry/telemetry_json.h
> +++ b/lib/telemetry/telemetry_json.h
> @@ -30,18 +30,23 @@
>  static inline int
>  __json_snprintf(char *buf, const int len, const char *format, ...)
>  {
> - char tmp[len];
> + char *tmp = malloc(len);
>   va_list ap;
> - int ret;
> + int ret = 0;
> +
> + if (tmp == NULL)
> + return ret;
>  
>   va_start(ap, format);
>   ret = vsnprintf(tmp, sizeof(tmp), format, ap);

I mistakenly pushed an old rev, i'll fix this.

sorry for the noise.

Re: [PATCH 0/2] improve code portability

[Tyler Retzlaff]

On Mon, Apr 03, 2023 at 06:04:08PM +0100, Bruce Richardson wrote:
> On Mon, Apr 03, 2023 at 09:30:22AM -0700, Tyler Retzlaff wrote:
> > Improve portability of telemetry code to allow it to be compiled by msvc
> > unconditionally.
> > 
> > Remove use of VLA and instead dynamically allocate. MSVC will never
> > implement VLAs due to misuse / security concerns. 
> > 
> > Remove use of ranged based initializer (a gcc extension) instead just
> > explicitly initialize individual array elements.
> > 
> > Tyler Retzlaff (2):
> >   telemetry: use malloc instead of variable length array
> >   telemetry: use portable syntax to initialize array
> > 

Bruce I was hoping to solicit your response specifically on this series so
thanks.

> Is this worth doing, given that DPDK telemetry uses a unix domain socket
> for it's connectivity, which would not be available on windows anyway?
> I don't particularly like these patches as:
> * The removal of the VLAs means we will potentially be doing a *lot* of
>   malloc and free calls inside the telemetry code. It may not be a data
>   path or particularly performance critical, but I know for things like CPU
>   busyness, users may want to call telemetry functions hundreds (or
>   potentially thousands) of times a second. It also makes the code slightly
>   harder to read, and introduces the possibility of us having memory leaks.

Yeah, malloc/free at high frequency is a bit ugly we could use alloca but I
think equally as unsafe as VLAs.

Is the preference here to just not compile any of this code for Windows
regardless of compiler? If that is preferred I can look at refactoring
to do that.

Another option is I could use conditionally compiled code narrowly here
using a Windows only API malloca which is vaguely safer?

> * The second patch just makes the code uglier. True, it's non-standard, but
>   it really does make the code a whole lot more readable and managable. If
>   we need to make this standards-conforming, then I think we need to drop
>   the "const", and do runtime init of this array with loops for the ranges.

I considered this. I thought the preference was to preserve const but
if initialization in a loop is preferrable i'll do that instead.

> 
> All that said, if we do have a path to get telemetry working on windows, I
> think we can work together to get a suitable patchset in for it.

It's unfortunate that EAL depends on telemetry but it doesn't work on
Windows (right now). Telemetry is unfortunately not of much value if the
code doesn't build and run so thus a lower priority.

What I can ask for here (if the community accepts) is getting the code
to compile, that's what unblocks progress for now and we can re-visit
making telemetry work properly ~later.

Thanks

> 
> /Bruce

[PATCH v2] improve code portability

[Tyler Retzlaff]

Improve portability of telemetry code to allow it to be compiled by msvc
unconditionally.

Remove use of ranged based initializer (a gcc extension). Instead initialize
using for loops over [0..9] [A..Z] [a..z] [_/] on first call.

v2:
  * Initialize valid character array using for loop instead of explicit
initialization of each element.
  * Drop patch removing VLAs, in a separate series a conditionally compiled
change will be introduced.

Tyler Retzlaff (1):
  telemetry: use portable syntax to initialize array

 lib/telemetry/telemetry_data.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

-- 
1.8.3.1

[PATCH v2] telemetry: use portable syntax to initialize array

[Tyler Retzlaff]

Use of ranges in designated initialization are a non-standard gcc
extension. Use loops to initialize permitted characters on first use.

Signed-off-by: Tyler Retzlaff 
---
 lib/telemetry/telemetry_data.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
index 2bac2de..d845186 100644
--- a/lib/telemetry/telemetry_data.c
+++ b/lib/telemetry/telemetry_data.c
@@ -152,13 +152,21 @@
 static bool
 valid_name(const char *name)
 {
-   char allowed[128] = {
-   ['0' ... '9'] = 1,
-   ['A' ... 'Z'] = 1,
-   ['a' ... 'z'] = 1,
-   ['_'] = 1,
-   ['/'] = 1,
-   };
+   static bool initialized;
+   static char allowed[128];
+
+   if (!initialized) {
+   int index;
+   for (index = '0'; index <= '9'; index++)
+   allowed[index] = 1;
+   for (index = 'A'; index <= 'Z'; index++)
+   allowed[index] = 1;
+   for (index = 'a'; index <= 'Z'; index++)
+   allowed[index] = 1;
+   allowed[(int)'_'] = allowed[(int)'/'] = 1;
+   initialized = true;
+   }
+
while (*name != '\0') {
if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name] == 
0)
return false;
-- 
1.8.3.1

[PATCH v3] improve code portability

[Tyler Retzlaff]

Improve portability of telemetry code to allow it to be compiled by msvc
unconditionally.

Remove use of ranged based initializer (a gcc extension). Instead initialize
using for loops over [0..9] [A..Z] [a..z] [_/] on first call.

v3:
  * Move int index declaration to top of function
  * Fix typo 'Z' -> 'z' in last for loop
v2:
  * Initialize valid character array using for loop instead of explicit
initialization of each element.
  * Drop patch removing VLAs, in a separate series a conditionally compiled
change will be introduced.

Tyler Retzlaff (1):
  telemetry: use portable syntax to initialize array

 lib/telemetry/telemetry_data.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

-- 
1.8.3.1

[PATCH v3] telemetry: use portable syntax to initialize array

[Tyler Retzlaff]

Use of ranges in designated initialization are a non-standard gcc
extension. Use loops to initialize permitted characters on first use.

Signed-off-by: Tyler Retzlaff 
---
 lib/telemetry/telemetry_data.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
index 2bac2de..562b387 100644
--- a/lib/telemetry/telemetry_data.c
+++ b/lib/telemetry/telemetry_data.c
@@ -152,13 +152,21 @@
 static bool
 valid_name(const char *name)
 {
-   char allowed[128] = {
-   ['0' ... '9'] = 1,
-   ['A' ... 'Z'] = 1,
-   ['a' ... 'z'] = 1,
-   ['_'] = 1,
-   ['/'] = 1,
-   };
+   int index;
+   static bool initialized;
+   static char allowed[128];
+
+   if (!initialized) {
+   for (index = '0'; index <= '9'; index++)
+   allowed[index] = 1;
+   for (index = 'A'; index <= 'Z'; index++)
+   allowed[index] = 1;
+   for (index = 'a'; index <= 'z'; index++)
+   allowed[index] = 1;
+   allowed[(int)'_'] = allowed[(int)'/'] = 1;
+   initialized = true;
+   }
+
while (*name != '\0') {
if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name] == 
0)
return false;
-- 
1.8.3.1

Re: [PATCH 1/2] telemetry: use malloc instead of variable length array

[Stephen Hemminger]

On Mon,  3 Apr 2023 09:30:23 -0700
Tyler Retzlaff  wrote:

>  __json_snprintf(char *buf, const int len, const char *format, ...)
>  {
> - char tmp[len];
> + char *tmp = malloc(len);
>   va_list ap;
> - int ret;
> + int ret = 0;
> +
> + if (tmp == NULL)
> + return ret;
>  
>   va_start(ap, format);
>   ret = vsnprintf(tmp, sizeof(tmp), format, ap);
>   va_end(ap);
>   if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
>   strcpy(buf, tmp);
> - return ret;
>   }
> - return 0; /* nothing written or modified */
> +
> + free(tmp);
> +
> + return ret;
>  }

Not sure why it needs a tmp buffer anyway?

Re: [PATCH 1/2] telemetry: use malloc instead of variable length array

[Tyler Retzlaff]

On Mon, Apr 03, 2023 at 01:19:12PM -0700, Stephen Hemminger wrote:
> On Mon,  3 Apr 2023 09:30:23 -0700
> Tyler Retzlaff  wrote:
> 
> >  __json_snprintf(char *buf, const int len, const char *format, ...)
> >  {
> > -   char tmp[len];
> > +   char *tmp = malloc(len);
> > va_list ap;
> > -   int ret;
> > +   int ret = 0;
> > +
> > +   if (tmp == NULL)
> > +   return ret;
> >  
> > va_start(ap, format);
> > ret = vsnprintf(tmp, sizeof(tmp), format, ap);
> > va_end(ap);
> > if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
   ^^^
also this seems redundant. when is len != sizeof(tmp) here?

> > strcpy(buf, tmp);
> > -   return ret;
> > }
> > -   return 0; /* nothing written or modified */
> > +
> > +   free(tmp);
> > +
> > +   return ret;
> >  }
> 
> Not sure why it needs a tmp buffer anyway?

yeah, there a are a few question marks in this code. i've removed this
patch from the series for now. v3 doesn't touch this file anymore.

Re: [PATCH v2] eal: introduce atomics abstraction

[Mattias Rönnblom]

On 2023-02-08 22:43, Tyler Retzlaff wrote:

Introduce atomics abstraction that permits optional use of standard C11
atomics when meson is provided the new enable_stdatomics=true option.



Terminology nitpicking: I don't think these functions provide any 
abstraction at all. They are just wrappers.



Signed-off-by: Tyler Retzlaff 
---
  config/meson.build | 11 
  lib/eal/arm/include/rte_atomic_32.h|  6 ++-
  lib/eal/arm/include/rte_atomic_64.h|  6 ++-
  lib/eal/include/generic/rte_atomic.h   | 96 +-
  lib/eal/loongarch/include/rte_atomic.h |  6 ++-
  lib/eal/ppc/include/rte_atomic.h   |  6 ++-
  lib/eal/riscv/include/rte_atomic.h |  6 ++-
  lib/eal/x86/include/rte_atomic.h   |  8 ++-
  meson_options.txt  |  2 +
  9 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 26f3168..25dd628 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -255,6 +255,17 @@ endif
  # add -include rte_config to cflags
  add_project_arguments('-include', 'rte_config.h', language: 'c')
  
+stdc_atomics_enabled = get_option('enable_stdatomics')

+dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
+
+if stdc_atomics_enabled
+if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
+add_project_arguments('-std=gnu11', language: 'c')
+else
+add_project_arguments('-std=c11', language: 'c')
+endif
+endif
+
  # enable extra warnings and disable any unwanted warnings
  # -Wall is added by default at warning level 1, and -Wextra
  # at warning level 2 (DPDK default)
diff --git a/lib/eal/arm/include/rte_atomic_32.h 
b/lib/eal/arm/include/rte_atomic_32.h
index c00ab78..7088a12 100644
--- a/lib/eal/arm/include/rte_atomic_32.h
+++ b/lib/eal/arm/include/rte_atomic_32.h
@@ -34,9 +34,13 @@
  #define rte_io_rmb() rte_rmb()
  
  static __rte_always_inline void

-rte_atomic_thread_fence(int memorder)
+rte_atomic_thread_fence(rte_memory_order memorder)
  {
+#ifdef RTE_STDC_ATOMICS
+   atomic_thread_fence(memorder);
+#else
__atomic_thread_fence(memorder);
+#endif
  }
  
  #ifdef __cplusplus

diff --git a/lib/eal/arm/include/rte_atomic_64.h 
b/lib/eal/arm/include/rte_atomic_64.h
index 6047911..7f02c57 100644
--- a/lib/eal/arm/include/rte_atomic_64.h
+++ b/lib/eal/arm/include/rte_atomic_64.h
@@ -38,9 +38,13 @@
  #define rte_io_rmb() rte_rmb()
  
  static __rte_always_inline void

-rte_atomic_thread_fence(int memorder)
+rte_atomic_thread_fence(rte_memory_order memorder)
  {
+#ifdef RTE_STDC_ATOMICS
+   atomic_thread_fence(memorder);
+#else
__atomic_thread_fence(memorder);
+#endif
  }
  
  /* 128 bit atomic operations -*/

diff --git a/lib/eal/include/generic/rte_atomic.h 
b/lib/eal/include/generic/rte_atomic.h
index f5c49a9..392d928 100644
--- a/lib/eal/include/generic/rte_atomic.h
+++ b/lib/eal/include/generic/rte_atomic.h
@@ -110,6 +110,100 @@
  
  #endif /* __DOXYGEN__ */
  
+#ifdef RTE_STDC_ATOMICS

+
+#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L || 
defined(__STDC_NO_ATOMICS__)
+#error compiler does not support C11 standard atomics
+#else
+#include 
+#endif
+
+#define __rte_atomic _Atomic
+
+typedef int rte_memory_order;
+
+#define rte_memory_order_relaxed memory_order_relaxed
+#define rte_memory_order_consume memory_order_consume
+#define rte_memory_order_acquire memory_order_acquire
+#define rte_memory_order_release memory_order_release
+#define rte_memory_order_acq_rel memory_order_acq_rel
+#define rte_memory_order_seq_cst memory_order_seq_cst
+


Would this be better of as an enum, rather than a typedef? If typedef, 
it should have the "_t" postfix. Also, the #define should be all-caps.



+#define rte_atomic_store_explicit(obj, desired, order) \
+   atomic_store_explicit(obj, desired, order)
+


Drop "explicit" from all the names. It's just noise. Also, the memory 
orders have very long names.


We haven't even move all DPDK code over from the old API, to using GCC 
C11 built-ins, and now we are switching to a new API?



+#define rte_atomic_load_explicit(obj, order) \
+   atomic_load_explicit(obj, order)
+
+#define rte_atomic_exchange_explicit(obj, desired, order) \
+   atomic_exchange_explicit(obj, desired, order)
+
+#define rte_atomic_compare_exchange_strong_explicit(obj, expected, desired, 
success, fail) \
+   atomic_compare_exchange_strong_explicit(obj, expected, desired, 
success, fail)
+
+#define rte_atomic_compare_exchange_weak_explicit(obj, expected, desired, 
success, fail) \
+   atomic_compare_exchange_weak_explicit(obj, expected, desired, success, 
fail)
+
+#define rte_atomic_fetch_add_explicit(obj, arg, order) \
+   atomic_fetch_add_explicit(obj, arg, order)
+
+#define rte_atomic_fetch_sub_explicit(obj, arg, order) \
+   atomic_fetch_sub_explicit(obj, arg, order)
+
+#define rte_atomic_fetch_or_explicit(obj, arg, order) \
+   atomic_fetch_or_explici

Re: [PATCH v2] eal: introduce atomics abstraction

[Mattias Rönnblom]

On 2023-02-09 09:05, Morten Brørup wrote:

From: Tyler Retzlaff [mailto:roret...@linux.microsoft.com]
Sent: Wednesday, 8 February 2023 22.44

Introduce atomics abstraction that permits optional use of standard C11
atomics when meson is provided the new enable_stdatomics=true option.

Signed-off-by: Tyler Retzlaff 
---


Looks good. A few minor suggestions about implementation only.

With or without suggested modifications,

Acked-by: Morten Brørup 



  config/meson.build | 11 
  lib/eal/arm/include/rte_atomic_32.h|  6 ++-
  lib/eal/arm/include/rte_atomic_64.h|  6 ++-
  lib/eal/include/generic/rte_atomic.h   | 96
+-
  lib/eal/loongarch/include/rte_atomic.h |  6 ++-
  lib/eal/ppc/include/rte_atomic.h   |  6 ++-
  lib/eal/riscv/include/rte_atomic.h |  6 ++-
  lib/eal/x86/include/rte_atomic.h   |  8 ++-
  meson_options.txt  |  2 +
  9 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 26f3168..25dd628 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -255,6 +255,17 @@ endif
  # add -include rte_config to cflags
  add_project_arguments('-include', 'rte_config.h', language: 'c')

+stdc_atomics_enabled = get_option('enable_stdatomics')
+dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
+
+if stdc_atomics_enabled
+if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
+add_project_arguments('-std=gnu11', language: 'c')
+else
+add_project_arguments('-std=c11', language: 'c')
+endif
+endif
+
  # enable extra warnings and disable any unwanted warnings
  # -Wall is added by default at warning level 1, and -Wextra
  # at warning level 2 (DPDK default)
diff --git a/lib/eal/arm/include/rte_atomic_32.h
b/lib/eal/arm/include/rte_atomic_32.h
index c00ab78..7088a12 100644
--- a/lib/eal/arm/include/rte_atomic_32.h
+++ b/lib/eal/arm/include/rte_atomic_32.h
@@ -34,9 +34,13 @@
  #define rte_io_rmb() rte_rmb()

  static __rte_always_inline void
-rte_atomic_thread_fence(int memorder)
+rte_atomic_thread_fence(rte_memory_order memorder)
  {
+#ifdef RTE_STDC_ATOMICS
+   atomic_thread_fence(memorder);
+#else
__atomic_thread_fence(memorder);
+#endif
  }

  #ifdef __cplusplus
diff --git a/lib/eal/arm/include/rte_atomic_64.h
b/lib/eal/arm/include/rte_atomic_64.h
index 6047911..7f02c57 100644
--- a/lib/eal/arm/include/rte_atomic_64.h
+++ b/lib/eal/arm/include/rte_atomic_64.h
@@ -38,9 +38,13 @@
  #define rte_io_rmb() rte_rmb()

  static __rte_always_inline void
-rte_atomic_thread_fence(int memorder)
+rte_atomic_thread_fence(rte_memory_order memorder)
  {
+#ifdef RTE_STDC_ATOMICS
+   atomic_thread_fence(memorder);
+#else
__atomic_thread_fence(memorder);
+#endif
  }

  /* 128 bit atomic operations -
*/
diff --git a/lib/eal/include/generic/rte_atomic.h
b/lib/eal/include/generic/rte_atomic.h
index f5c49a9..392d928 100644
--- a/lib/eal/include/generic/rte_atomic.h
+++ b/lib/eal/include/generic/rte_atomic.h
@@ -110,6 +110,100 @@

  #endif /* __DOXYGEN__ */

+#ifdef RTE_STDC_ATOMICS
+
+#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L ||
defined(__STDC_NO_ATOMICS__)
+#error compiler does not support C11 standard atomics
+#else
+#include 
+#endif
+
+#define __rte_atomic _Atomic
+
+typedef int rte_memory_order;


I would prefer enum for rte_memory_order:

typedef enum {
 rte_memory_order_relaxed = memory_order_relaxed,
 rte_memory_order_consume = memory_order_consume,
 rte_memory_order_acquire = memory_order_acquire,
 rte_memory_order_release = memory_order_release,
 rte_memory_order_acq_rel = memory_order_acq_rel,
 rte_memory_order_seq_cst = memory_order_seq_cst
} rte_memory_order;


+
+#define rte_memory_order_relaxed memory_order_relaxed
+#define rte_memory_order_consume memory_order_consume
+#define rte_memory_order_acquire memory_order_acquire
+#define rte_memory_order_release memory_order_release
+#define rte_memory_order_acq_rel memory_order_acq_rel
+#define rte_memory_order_seq_cst memory_order_seq_cst
+
+#define rte_atomic_store_explicit(obj, desired, order) \
+   atomic_store_explicit(obj, desired, order)
+
+#define rte_atomic_load_explicit(obj, order) \
+   atomic_load_explicit(obj, order)
+
+#define rte_atomic_exchange_explicit(obj, desired, order) \
+   atomic_exchange_explicit(obj, desired, order)
+
+#define rte_atomic_compare_exchange_strong_explicit(obj, expected,
desired, success, fail) \
+   atomic_compare_exchange_strong_explicit(obj, expected, desired,
success, fail)
+
+#define rte_atomic_compare_exchange_weak_explicit(obj, expected,
desired, success, fail) \
+   atomic_compare_exchange_weak_explicit(obj, expected, desired,
success, fail)
+
+#define rte_atomic_fetch_add_explicit(obj, arg, order) \
+   atomic_fetch_add_explicit(obj, arg, order)
+
+#define rte_atomic_fetch_sub_explicit(obj, arg, order) \
+   atomic_fetch_sub_e

RE: [PATCH v2] eal: introduce atomics abstraction

[Honnappa Nagarahalli]

> -Original Message-
> From: Mattias Rönnblom 
> Sent: Monday, April 3, 2023 4:12 PM
> To: Tyler Retzlaff ; dev@dpdk.org
> Cc: david.march...@redhat.com; tho...@monjalon.net;
> m...@smartsharesystems.com; Honnappa Nagarahalli
> ; bruce.richard...@intel.com
> Subject: Re: [PATCH v2] eal: introduce atomics abstraction
> 
> On 2023-02-08 22:43, Tyler Retzlaff wrote:
> > Introduce atomics abstraction that permits optional use of standard
> > C11 atomics when meson is provided the new enable_stdatomics=true option.
> >
> 
> Terminology nitpicking: I don't think these functions provide any abstraction 
> at
> all. They are just wrappers.
This patch should be deprecated. As per the decision at Techboard, we will move 
to using atomic APIs from stdatomics.h

> 
> > Signed-off-by: Tyler Retzlaff 
> > ---
> >   config/meson.build | 11 
> >   lib/eal/arm/include/rte_atomic_32.h|  6 ++-
> >   lib/eal/arm/include/rte_atomic_64.h|  6 ++-
> >   lib/eal/include/generic/rte_atomic.h   | 96
> +-
> >   lib/eal/loongarch/include/rte_atomic.h |  6 ++-
> >   lib/eal/ppc/include/rte_atomic.h   |  6 ++-
> >   lib/eal/riscv/include/rte_atomic.h |  6 ++-
> >   lib/eal/x86/include/rte_atomic.h   |  8 ++-
> >   meson_options.txt  |  2 +
> >   9 files changed, 139 insertions(+), 8 deletions(-)
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > 26f3168..25dd628 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -255,6 +255,17 @@ endif
> >   # add -include rte_config to cflags
> >   add_project_arguments('-include', 'rte_config.h', language: 'c')
> >
> > +stdc_atomics_enabled = get_option('enable_stdatomics')
> > +dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
> > +
> > +if stdc_atomics_enabled
> > +if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
> > +add_project_arguments('-std=gnu11', language: 'c') else
> > +add_project_arguments('-std=c11', language: 'c') endif endif
> > +
> >   # enable extra warnings and disable any unwanted warnings
> >   # -Wall is added by default at warning level 1, and -Wextra
> >   # at warning level 2 (DPDK default)
> > diff --git a/lib/eal/arm/include/rte_atomic_32.h
> > b/lib/eal/arm/include/rte_atomic_32.h
> > index c00ab78..7088a12 100644
> > --- a/lib/eal/arm/include/rte_atomic_32.h
> > +++ b/lib/eal/arm/include/rte_atomic_32.h
> > @@ -34,9 +34,13 @@
> >   #define rte_io_rmb() rte_rmb()
> >
> >   static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >   {
> > +#ifdef RTE_STDC_ATOMICS
> > +   atomic_thread_fence(memorder);
> > +#else
> > __atomic_thread_fence(memorder);
> > +#endif
> >   }
> >
> >   #ifdef __cplusplus
> > diff --git a/lib/eal/arm/include/rte_atomic_64.h
> > b/lib/eal/arm/include/rte_atomic_64.h
> > index 6047911..7f02c57 100644
> > --- a/lib/eal/arm/include/rte_atomic_64.h
> > +++ b/lib/eal/arm/include/rte_atomic_64.h
> > @@ -38,9 +38,13 @@
> >   #define rte_io_rmb() rte_rmb()
> >
> >   static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >   {
> > +#ifdef RTE_STDC_ATOMICS
> > +   atomic_thread_fence(memorder);
> > +#else
> > __atomic_thread_fence(memorder);
> > +#endif
> >   }
> >
> >   /* 128 bit atomic operations
> > -*/ diff --git
> > a/lib/eal/include/generic/rte_atomic.h
> > b/lib/eal/include/generic/rte_atomic.h
> > index f5c49a9..392d928 100644
> > --- a/lib/eal/include/generic/rte_atomic.h
> > +++ b/lib/eal/include/generic/rte_atomic.h
> > @@ -110,6 +110,100 @@
> >
> >   #endif /* __DOXYGEN__ */
> >
> > +#ifdef RTE_STDC_ATOMICS
> > +
> > +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L ||
> > +defined(__STDC_NO_ATOMICS__) #error compiler does not support C11
> > +standard atomics #else #include  #endif
> > +
> > +#define __rte_atomic _Atomic
> > +
> > +typedef int rte_memory_order;
> > +
> > +#define rte_memory_order_relaxed memory_order_relaxed #define
> > +rte_memory_order_consume memory_order_consume #define
> > +rte_memory_order_acquire memory_order_acquire #define
> > +rte_memory_order_release memory_order_release #define
> > +rte_memory_order_acq_rel memory_order_acq_rel #define
> > +rte_memory_order_seq_cst memory_order_seq_cst
> > +
> 
> Would this be better of as an enum, rather than a typedef? If typedef, it 
> should
> have the "_t" postfix. Also, the #define should be all-caps.
> 
> > +#define rte_atomic_store_explicit(obj, desired, order) \
> > +   atomic_store_explicit(obj, desired, order)
> > +
> 
> Drop "explicit" from all the names. It's just noise. Also, the memory orders 
> have
> very long names.
> 
> We haven't even move all DPDK code over from the old API, to using GCC
> C11 built-ins, and now we are switching to a new API?
> 
> > +#define rte_atomic_load_explicit(obj, 

Re: [PATCH v2] eal: introduce atomics abstraction

[Mattias Rönnblom]

On 2023-02-09 18:38, Tyler Retzlaff wrote:

On Thu, Feb 09, 2023 at 09:04:16AM +, Bruce Richardson wrote:

On Wed, Feb 08, 2023 at 01:43:38PM -0800, Tyler Retzlaff wrote:

Introduce atomics abstraction that permits optional use of standard C11
atomics when meson is provided the new enable_stdatomics=true option.

Signed-off-by: Tyler Retzlaff 
---
  config/meson.build | 11 
  lib/eal/arm/include/rte_atomic_32.h|  6 ++-
  lib/eal/arm/include/rte_atomic_64.h|  6 ++-
  lib/eal/include/generic/rte_atomic.h   | 96 +-
  lib/eal/loongarch/include/rte_atomic.h |  6 ++-
  lib/eal/ppc/include/rte_atomic.h   |  6 ++-
  lib/eal/riscv/include/rte_atomic.h |  6 ++-
  lib/eal/x86/include/rte_atomic.h   |  8 ++-
  meson_options.txt  |  2 +
  9 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 26f3168..25dd628 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -255,6 +255,17 @@ endif
  # add -include rte_config to cflags
  add_project_arguments('-include', 'rte_config.h', language: 'c')
  
+stdc_atomics_enabled = get_option('enable_stdatomics')

+dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
+
+if stdc_atomics_enabled
+if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
+add_project_arguments('-std=gnu11', language: 'c')


Is there a reason for using gnu11 on gcc and clang, rather than limiting
ourselves to proper c11 support?


there is code using posix extensions, there are two ways to use them
without emitting warnings.

1. -std=gnu11 (to get C11 with GNU extensions)

-- or --

2. -std=c11 and then in the source files consuming the C11 GNU
extensions do some dance with various GNUC macros before including
various stdxxx.h headers to enable the extensions for the translation
unit.

i vaguely recall that if you try to do a test build with -std=c11 over
the whole tree with meson --werror it will highlight the exact code i'm
talking about.

selfishly i'd be happy to see (2) done and potentially eliminate the use
of the extensions, but i didn't want to be disruptive as a part of this
change.



Some of the GNU extensions are useful, and have no equivalent in C11.

Statement expressions is one example that comes to mind.

I think you can get such code through GCC even with -std=c11, but not if 
combined with -pedantic.

Re: [PATCH 1/2] eal: provide leading and trailing zero bit count abstraction

[Mattias Rönnblom]

On 2022-11-23 23:14, Tyler Retzlaff wrote:
> Provide an abstraction for leading and trailing zero bit counting
> functions to hide compiler specific intrinsics and builtins.
> 
> Signed-off-by: Tyler Retzlaff 
> ---
>   lib/eal/include/meson.build|   1 +
>   lib/eal/include/rte_bitcount.h | 257 
> +
>   2 files changed, 258 insertions(+)
>   create mode 100644 lib/eal/include/rte_bitcount.h
> 
> diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
> index cfcd40a..8ff1d65 100644
> --- a/lib/eal/include/meson.build
> +++ b/lib/eal/include/meson.build
> @@ -5,6 +5,7 @@ includes += include_directories('.')
>   
>   headers += files(
>   'rte_alarm.h',
> +'rte_bitcount.h',

Maybe rte_bitops.h should be the home of these new wrappers.

>   'rte_bitmap.h',
>   'rte_bitops.h',
>   'rte_branch_prediction.h',
> diff --git a/lib/eal/include/rte_bitcount.h b/lib/eal/include/rte_bitcount.h
> new file mode 100644
> index 000..2f4dd38
> --- /dev/null
> +++ b/lib/eal/include/rte_bitcount.h
> @@ -0,0 +1,257 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2022 Microsoft Corporation
> + */
> +
> +#ifndef EAL_BITCOUNT_H
> +#define EAL_BITCOUNT_H
> +
> +#include 
> +
> +#ifdef RTE_TOOLCHAIN_MSVC
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned

unsigned int

If you want to be __builtin_clz()-compatible, you should stay with "int" 
as the return type.

If you don't, you might want to change more than just the return type. 
In particular, it would be useful to have size-specific variants 
(although in practice they are on most [all?] DPDK-supported platforms).

unsigned int
rte_clz(uint32_t v);

or rte_clz32(), to follow the naming convention from some other wrapper 
RFC you posted.

> +rte_clz(unsigned int v)
> +{
> + unsigned long rv;
> +
> + (void)_BitScanReverse(&rv, v);
> +
> + return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clzl(unsigned long v)
> +{
> + unsigned long rv;
> +
> + (void)_BitScanReverse(&rv, v);
> +
> + return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clzll(unsigned long long v)
> +{
> + unsigned long rv;
> +
> + (void)_BitScanReverse64(&rv, v);
> +
> + return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctz(unsigned int v)
> +{
> + unsigned long rv;
> +
> + (void)_BitScanForward(&rv, v);
> +
> + return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctzl(unsigned long v)
> +{
> + unsigned long rv;
> +
> + (void)_BitScanForward(&rv, v);
> +
> + return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctzll(unsigned long long v)
> +{
> + unsigned long rv;
> +
> + (void)_BitScanForward64(&rv, v);
> +
> + return (unsigned)rv;
> +}
> +
> +#else
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clz(unsigned int v)
> +{
> + return (unsigned)__builtin_clz(v);
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leadi

[PATCH 0/9] msvc integration changes

[Tyler Retzlaff]

In accordance with draft plan
http://mails.dpdk.org/archives/web/2023-February/002023.html
introduces conditionally compiled code to enable building with MSVC that
_does not_ require C99/C11 meaning it can be integrated now.

This series covers minimal changes for item #2 in draft plan for EAL
dependencies kvargs, telemetry and consumed EAL public headers.

Note if any patch in the series requires in-depth discussion I'll
detach it from this series for separate submission & more focused
discussion so it doesn't block the entire series.

Tyler Retzlaff (9):
  eal: use rdtsc intrinsic when compiling with msvc
  eal: use rtm and xtest intrinsics when compiling with msvc
  eal: use barrier intrinsics when compiling with msvc
  eal: typedef cpu flag enum as int for msvc
  eal: hide GCC extension based alignment markers
  eal: expand most macros to empty when using msvc
  eal: exclude exposure of rte atomic APIs for MSVC builds
  telemetry: disable json print formatting with msvc
  telemetry: avoid expanding versioned symbol macros on msvc

 lib/eal/include/generic/rte_atomic.h| 11 ++
 lib/eal/include/generic/rte_cpuflags.h  | 12 ++-
 lib/eal/include/rte_branch_prediction.h |  8 +++
 lib/eal/include/rte_common.h| 37 +
 lib/eal/include/rte_compat.h| 20 ++
 lib/eal/x86/include/rte_atomic.h| 14 -
 lib/eal/x86/include/rte_cycles.h|  8 +++
 lib/eal/x86/include/rte_rtm.h   | 19 +
 lib/telemetry/telemetry_data.c  | 16 ++
 lib/telemetry/telemetry_json.h  |  6 ++
 10 files changed, 145 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH 1/9] eal: use rdtsc intrinsic when compiling with msvc

[Tyler Retzlaff]

Inline assembly is not supported for msvc x64 instead use __rdtsc
intrinsic.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/x86/include/rte_cycles.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/eal/x86/include/rte_cycles.h b/lib/eal/x86/include/rte_cycles.h
index a461a4d..0c142ce 100644
--- a/lib/eal/x86/include/rte_cycles.h
+++ b/lib/eal/x86/include/rte_cycles.h
@@ -6,6 +6,10 @@
 #ifndef _RTE_CYCLES_X86_64_H_
 #define _RTE_CYCLES_X86_64_H_
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#include 
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -23,6 +27,7 @@
 static inline uint64_t
 rte_rdtsc(void)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
union {
uint64_t tsc_64;
RTE_STD_C11
@@ -47,6 +52,9 @@
 "=a" (tsc.lo_32),
 "=d" (tsc.hi_32));
return tsc.tsc_64;
+#else
+   return __rdtsc();
+#endif
 }
 
 static inline uint64_t
-- 
1.8.3.1

[PATCH 3/9] eal: use barrier intrinsics when compiling with msvc

[Tyler Retzlaff]

Inline assembly is not supported for msvc x64 instead use
_{Read,Write,ReadWrite}Barrier() intrinsics.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/include/generic/rte_atomic.h |  4 
 lib/eal/x86/include/rte_atomic.h | 10 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/generic/rte_atomic.h 
b/lib/eal/include/generic/rte_atomic.h
index 234b268..e973184 100644
--- a/lib/eal/include/generic/rte_atomic.h
+++ b/lib/eal/include/generic/rte_atomic.h
@@ -116,9 +116,13 @@
  * Guarantees that operation reordering does not occur at compile time
  * for operations directly before and after the barrier.
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #definerte_compiler_barrier() do { \
asm volatile ("" : : : "memory");   \
 } while(0)
+#else
+#define rte_compiler_barrier() _ReadWriteBarrier()
+#endif
 
 /**
  * Synchronization fence between threads based on the specified memory order.
diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
index f2ee1a9..5cce9ba 100644
--- a/lib/eal/x86/include/rte_atomic.h
+++ b/lib/eal/x86/include/rte_atomic.h
@@ -27,9 +27,13 @@
 
 #definerte_rmb() _mm_lfence()
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define rte_smp_wmb() rte_compiler_barrier()
-
 #define rte_smp_rmb() rte_compiler_barrier()
+#else
+#define rte_smp_wmb() _WriteBarrier()
+#define rte_smp_rmb() _ReadBarrier()
+#endif
 
 /*
  * From Intel Software Development Manual; Vol 3;
@@ -66,11 +70,15 @@
 static __rte_always_inline void
 rte_smp_mb(void)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifdef RTE_ARCH_I686
asm volatile("lock addl $0, -128(%%esp); " ::: "memory");
 #else
asm volatile("lock addl $0, -128(%%rsp); " ::: "memory");
 #endif
+#else
+   rte_compiler_barrier();
+#endif
 }
 
 #define rte_io_mb() rte_mb()
-- 
1.8.3.1

[PATCH 2/9] eal: use rtm and xtest intrinsics when compiling with msvc

[Tyler Retzlaff]

Inline assembly is not supported for msvc x64 instead use _xbegin,
_xend, _xabort and _xtest intrinsics.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/x86/include/rte_rtm.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/lib/eal/x86/include/rte_rtm.h b/lib/eal/x86/include/rte_rtm.h
index 36bf498..26672cb 100644
--- a/lib/eal/x86/include/rte_rtm.h
+++ b/lib/eal/x86/include/rte_rtm.h
@@ -5,6 +5,9 @@
 #ifndef _RTE_RTM_H_
 #define _RTE_RTM_H_ 1
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#include 
+#endif
 
 /* Official RTM intrinsics interface matching gcc/icc, but works
on older gcc compatible compilers and binutils. */
@@ -28,31 +31,47 @@
 static __rte_always_inline
 unsigned int rte_xbegin(void)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
unsigned int ret = RTE_XBEGIN_STARTED;
 
asm volatile(".byte 0xc7,0xf8 ; .long 0" : "+a" (ret) :: "memory");
return ret;
+#else
+   return _xbegin();
+#endif
 }
 
 static __rte_always_inline
 void rte_xend(void)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
 asm volatile(".byte 0x0f,0x01,0xd5" ::: "memory");
+#else
+   _xend();
+#endif
 }
 
 /* not an inline function to workaround a clang bug with -O0 */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define rte_xabort(status) do { \
asm volatile(".byte 0xc6,0xf8,%P0" :: "i" (status) : "memory"); \
 } while (0)
+#else
+#define rte_xabort(status) _xabort(status)
+#endif
 
 static __rte_always_inline
 int rte_xtest(void)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
unsigned char out;
 
asm volatile(".byte 0x0f,0x01,0xd6 ; setnz %0" :
"=r" (out) :: "memory");
return out;
+#else
+   return _xtest();
+#endif
 }
 
 #ifdef __cplusplus
-- 
1.8.3.1

[PATCH 4/9] eal: typedef cpu flag enum as int for msvc

[Tyler Retzlaff]

Forward declaration of a typedef is a non-standard extension and is not
supported by msvc. Use an int instead.

Abstract the use of the int/enum rte_cpu_flag_t in function parameter
lists by re-typdefing the enum rte_cpu_flag_t to the rte_cpu_flag_t
identifier.

Remove the use of __extension__ on function prototypes where
rte_cpu_flag_t appared in parameter lists, it is sufficient to have the
conditionally compiled __extension__ at the non-standard forward
declaration site.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/include/generic/rte_cpuflags.h | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/eal/include/generic/rte_cpuflags.h 
b/lib/eal/include/generic/rte_cpuflags.h
index d35551e..87ab03c 100644
--- a/lib/eal/include/generic/rte_cpuflags.h
+++ b/lib/eal/include/generic/rte_cpuflags.h
@@ -44,8 +44,12 @@ struct rte_cpu_intrinsics {
 /**
  * Enumeration of all CPU features supported
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 __extension__
-enum rte_cpu_flag_t;
+typedef enum rte_cpu_flag_t rte_cpu_flag_t;
+#else
+typedef int rte_cpu_flag_t;
+#endif
 
 /**
  * Get name of CPU flag
@@ -56,9 +60,8 @@ struct rte_cpu_intrinsics {
  * flag name
  * NULL if flag ID is invalid
  */
-__extension__
 const char *
-rte_cpu_get_flag_name(enum rte_cpu_flag_t feature);
+rte_cpu_get_flag_name(rte_cpu_flag_t feature);
 
 /**
  * Function for checking a CPU flag availability
@@ -70,9 +73,8 @@ struct rte_cpu_intrinsics {
  * 0 if flag is not available
  * -ENOENT if flag is invalid
  */
-__extension__
 int
-rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature);
+rte_cpu_get_flag_enabled(rte_cpu_flag_t feature);
 
 /**
  * This function checks that the currently used CPU supports the CPU features
-- 
1.8.3.1

[PATCH 7/9] eal: exclude exposure of rte atomic APIs for MSVC builds

[Tyler Retzlaff]

It's discouraged to use rte_atomics APIs instead standard APIs should be
used from C11. Since MSVC is a new toolchain/platform combination block
visibility of the rte_atomic APIs from day 1.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/include/generic/rte_atomic.h | 7 +++
 lib/eal/x86/include/rte_atomic.h | 4 
 2 files changed, 11 insertions(+)

diff --git a/lib/eal/include/generic/rte_atomic.h 
b/lib/eal/include/generic/rte_atomic.h
index e973184..1964697 100644
--- a/lib/eal/include/generic/rte_atomic.h
+++ b/lib/eal/include/generic/rte_atomic.h
@@ -131,6 +131,8 @@
 
 /*- 16 bit atomic operations 
-*/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Atomic compare and set.
  *
@@ -1038,8 +1040,11 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 }
 #endif
 
+#endif
+
 /* 128 bit atomic operations 
-*/
 
+
 /**
  * 128-bit integer structure.
  */
@@ -1049,8 +1054,10 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
union {
uint64_t val[2];
 #ifdef RTE_ARCH_64
+#ifndef RTE_TOOLCHAIN_MSVC
__extension__ __int128 int128;
 #endif
+#endif
};
 } __rte_aligned(16) rte_int128_t;
 
diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
index 5cce9ba..20f3380 100644
--- a/lib/eal/x86/include/rte_atomic.h
+++ b/lib/eal/x86/include/rte_atomic.h
@@ -87,6 +87,8 @@
 
 #define rte_io_rmb() rte_compiler_barrier()
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Synchronization fence between threads based on the specified memory order.
  *
@@ -283,6 +285,8 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t 
*v)
 #include "rte_atomic_64.h"
 #endif
 
+#endif
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1

[PATCH 8/9] telemetry: disable json print formatting with msvc

[Tyler Retzlaff]

VLAs are unsafe and will never be implemented in MSVC. When compiling
with MSVC just return immediately indicating 0 output characters
formatted.

For now telemetry doesn't work on Windows, we will revisit support for
the telemetry library sometime after we establish the DPDK unit tests.

Signed-off-by: Tyler Retzlaff 
---
 lib/telemetry/telemetry_json.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h
index 744bbfe..d847003 100644
--- a/lib/telemetry/telemetry_json.h
+++ b/lib/telemetry/telemetry_json.h
@@ -30,6 +30,7 @@
 static inline int
 __json_snprintf(char *buf, const int len, const char *format, ...)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
char tmp[len];
va_list ap;
int ret;
@@ -41,6 +42,7 @@
strcpy(buf, tmp);
return ret;
}
+#endif
return 0; /* nothing written or modified */
 }
 
@@ -60,6 +62,7 @@
 static inline int
 __json_format_str(char *buf, const int len, const char *prefix, const char 
*str, const char *suffix)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
char tmp[len];
int tmpidx = 0;
 
@@ -98,6 +101,9 @@
 
strcpy(buf, tmp);
return tmpidx;
+#else
+   return 0;
+#endif
 }
 
 /* Copies an empty array into the provided buffer. */
-- 
1.8.3.1

[PATCH 9/9] telemetry: avoid expanding versioned symbol macros on msvc

[Tyler Retzlaff]

Windows does not support versioned symbols. Fortunately Windows also
doesn't have an exported stable ABI.

Export rte_tel_data_add_array_int -> rte_tel_data_add_array_int_24
and rte_tel_data_add_dict_int -> rte_tel_data_add_dict_int_v24
functions.

Windows does have a way to achieve similar versioning for symbols but it
is not a simple #define so it will be done as a work package later.

Signed-off-by: Tyler Retzlaff 
---
 lib/telemetry/telemetry_data.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
index 2bac2de..284c16e 100644
--- a/lib/telemetry/telemetry_data.c
+++ b/lib/telemetry/telemetry_data.c
@@ -82,8 +82,16 @@
 /* mark the v23 function as the older version, and v24 as the default version 
*/
 VERSION_SYMBOL(rte_tel_data_add_array_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_array_int, _v24, 24);
+#ifndef RTE_TOOLCHAIN_MSVC
 MAP_STATIC_SYMBOL(int rte_tel_data_add_array_int(struct rte_tel_data *d,
int64_t x), rte_tel_data_add_array_int_v24);
+#else
+int
+rte_tel_data_add_array_int(struct rte_tel_data *d, int64_t x)
+{
+   return rte_tel_data_add_array_int_v24(d, x);
+}
+#endif
 
 int
 rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)
@@ -220,8 +228,16 @@
 /* mark the v23 function as the older version, and v24 as the default version 
*/
 VERSION_SYMBOL(rte_tel_data_add_dict_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_dict_int, _v24, 24);
+#ifndef RTE_TOOLCHAIN_MSVC
 MAP_STATIC_SYMBOL(int rte_tel_data_add_dict_int(struct rte_tel_data *d,
const char *name, int64_t val), rte_tel_data_add_dict_int_v24);
+#else
+int
+rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int64_t 
val)
+{
+   return rte_tel_data_add_dict_int_v24(d, name, val);
+}
+#endif
 
 int
 rte_tel_data_add_dict_uint(struct rte_tel_data *d,
-- 
1.8.3.1

[PATCH 6/9] eal: expand most macros to empty when using msvc

[Tyler Retzlaff]

For now expand a lot of common rte macros empty. The catch here is we
need to test that most of the macros do what they should but at the same
time they are blocking work needed to bootstrap of the unit tests.

Later we will return and provide (where possible) expansions that work
correctly for msvc and where not possible provide some alternate macros
to achieve the same outcome.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/include/rte_branch_prediction.h |  8 
 lib/eal/include/rte_common.h| 33 +
 lib/eal/include/rte_compat.h| 20 
 3 files changed, 61 insertions(+)

diff --git a/lib/eal/include/rte_branch_prediction.h 
b/lib/eal/include/rte_branch_prediction.h
index 0256a9d..3589c97 100644
--- a/lib/eal/include/rte_branch_prediction.h
+++ b/lib/eal/include/rte_branch_prediction.h
@@ -25,7 +25,11 @@
  *
  */
 #ifndef likely
+#ifndef RTE_TOOLCHAIN_MSVC
 #define likely(x)  __builtin_expect(!!(x), 1)
+#else
+#define likely(x)  (!!(x) == 1)
+#endif
 #endif /* likely */
 
 /**
@@ -39,7 +43,11 @@
  *
  */
 #ifndef unlikely
+#ifndef RTE_TOOLCHAIN_MSVC
 #define unlikely(x)__builtin_expect(!!(x), 0)
+#else
+#define unlikely(x)(!!(x) == 0)
+#endif
 #endif /* unlikely */
 
 #ifdef __cplusplus
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 2f464e3..a724e22 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -65,7 +65,11 @@
 /**
  * Force alignment
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_aligned(a) __attribute__((__aligned__(a)))
+#else
+#define __rte_aligned(a)
+#endif
 
 #ifdef RTE_ARCH_STRICT_ALIGN
 typedef uint64_t unaligned_uint64_t __rte_aligned(1);
@@ -88,8 +92,13 @@
 #define __rte_may_alias __attribute__((__may_alias__))
 
 /*** Macro to mark functions and fields scheduled for removal */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_deprecated   __attribute__((__deprecated__))
 #define __rte_deprecated_msg(msg)  __attribute__((__deprecated__(msg)))
+#else
+#define __rte_deprecated
+#define __rte_deprecated_msg(msg)
+#endif
 
 /**
  *  Macro to mark macros and defines scheduled for removal
@@ -117,7 +126,11 @@
 /**
  * short definition to mark a function parameter unused
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_unused __attribute__((__unused__))
+#else
+#define __rte_unused
+#endif
 
 /**
  * Mark pointer as restricted with regard to pointer aliasing.
@@ -141,6 +154,7 @@
  * even if the underlying stdio implementation is ANSI-compliant,
  * so this must be overridden.
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #if RTE_CC_IS_GNU
 #define __rte_format_printf(format_index, first_arg) \
__attribute__((format(gnu_printf, format_index, first_arg)))
@@ -148,6 +162,9 @@
 #define __rte_format_printf(format_index, first_arg) \
__attribute__((format(printf, format_index, first_arg)))
 #endif
+#else
+#define __rte_format_printf(format_index, first_arg)
+#endif
 
 /**
  * Tells compiler that the function returns a value that points to
@@ -222,7 +239,11 @@ static void __attribute__((destructor(RTE_PRIO(prio)), 
used)) func(void)
 /**
  * Hint never returning function
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_noreturn __attribute__((noreturn))
+#else
+#define __rte_noreturn
+#endif
 
 /**
  * Issue a warning in case the function's return value is ignored.
@@ -247,12 +268,20 @@ static void __attribute__((destructor(RTE_PRIO(prio)), 
used)) func(void)
  *  }
  * @endcode
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_warn_unused_result __attribute__((warn_unused_result))
+#else
+#define __rte_warn_unused_result
+#endif
 
 /**
  * Force a function to be inlined
  */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_always_inline inline __attribute__((always_inline))
+#else
+#define __rte_always_inline
+#endif
 
 /**
  * Force a function to be noinlined
@@ -437,7 +466,11 @@ static void __attribute__((destructor(RTE_PRIO(prio)), 
used)) func(void)
 #define RTE_CACHE_LINE_MIN_SIZE 64
 
 /** Force alignment to cache line. */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_cache_aligned __rte_aligned(RTE_CACHE_LINE_SIZE)
+#else
+#define __rte_cache_aligned
+#endif
 
 /** Force minimum cache line alignment. */
 #define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index fc9fbaa..6a4b5ee 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -12,14 +12,22 @@
 
 #ifndef ALLOW_EXPERIMENTAL_API
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_experimental \
 __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
 section(".text.experimental")))
+#else
+#define __rte_experimental
+#endif
 
 #else
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_experimental \
 __attribute__((section(".text.experimental")))
+#else
+#define __rte_experimental
+#endif
 
 #endif
 
@@ -30,23 +38,35 @@
 
 #if !defined ALLOW_INTERNAL_API && __has_attribute(err

[PATCH 5/9] eal: hide GCC extension based alignment markers

[Tyler Retzlaff]

When compiling with msvc don't expose typedefs used as alignment
markers.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/include/rte_common.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 15765b4..2f464e3 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -460,6 +460,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), 
used)) func(void)
 
 /*** Structure alignment markers /
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /** Generic marker for any place in a structure. */
 __extension__ typedef void*RTE_MARKER[0];
 /** Marker for 1B alignment in a structure. */
@@ -471,6 +473,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), 
used)) func(void)
 /** Marker for 8B alignment in a structure. */
 __extension__ typedef uint64_t RTE_MARKER64[0];
 
+#endif
+
 /**
  * Combines 32b inputs most significant set bits into the least
  * significant bits to construct a value with the same MSBs as x
-- 
1.8.3.1

Re: [PATCH] doc/ark: replace word segregation

[Luca Boccassi]

On Sun, 2 Apr 2023 at 23:54, Stephen Hemminger
 wrote:
>
> The word "segregation" brings up troubling memorys.
> Instead, use the term seperation which is what DPDK flow docs use.
>
> Signed-off-by: Stephen Hemminger 
> ---
>  doc/guides/nics/ark.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/guides/nics/ark.rst b/doc/guides/nics/ark.rst
> index de8fbbccc304..ab08214df1e2 100644
> --- a/doc/guides/nics/ark.rst
> +++ b/doc/guides/nics/ark.rst
> @@ -353,7 +353,7 @@ Supported Features
>
>  * Dynamic ARK PMD extensions
>  * Dynamic per-queue MBUF (re)sizing up to 32KB
> -* SR-IOV, VF-based queue-segregation
> +* SR-IOV, VF-based queue-seperation

Did you mean 'separation' by any chance?

Re: [PATCH v2] devtools: add script to check for non inclusive naming

[Luca Boccassi]

On Mon, 3 Apr 2023 at 15:47, Stephen Hemminger
 wrote:
>
> Shell script to find use of words that not be used.
> By default it prints matches.  The -q (quiet) option
> is used to just count. There is also -l option
> which lists lines matching (like grep -l).
>
> Uses the word lists from Inclusive Naming Initiative
> see https://inclusivenaming.org/word-lists/
>
> Examples:
>  $ ./devtools/check-naming-policy.sh -q
>  Total files: 37 errors, 90 warnings, 2 suggestions
>
>  $ ./devtools/check-naming-policy.sh -q -l lib/eal
>  Total lines: 32 errors, 8 warnings, 0 suggestions
>
> Add MAINTAINERS file entry for the new tool and resort
> the list files back into to alphabetic order
>
> Signed-off-by: Stephen Hemminger 
> ---
> v2 - fix typo in words
>- add subtree (pathspec) option
>- update maintainers file (and fix alphabetic order)

There's a json file on the website, how about downloading that on the
fly rather than storing a local copy that will go out of date?
https://inclusivenaming.org/word-lists/index.json

[PATCH v8 0/3] eal: provide abstracted bit counting functions

[Tyler Retzlaff]

As discussed technical board meeting 2023-02-22
http://mails.dpdk.org/archives/dev/2023-February/263516.html

We will bring support in pieces for the MSVC compiler, there will be
some abstractions and functions introduced before the compiler is
capable of compiling DPDK in order to make parallel progress
while waiting for standard atomics in 23.07.

A higher level plan / order of work is available in the Microsoft
roadmap for 23.07 and 23.11.

note:
Bruce Richardson previous acks have been preserved but be aware of
the two additional functions introduced in v7. If you wish to withdraw
your ack, please let me know but I believe the 2 additions are consistent
with previous.

v8:
  * squish patch including rte_bitops.h in lib/pipeline into
first patch.

v7:
  * add 2 additional counting functions rte_popcount{32,64}
including basic unit tests
  * fix patch 1 title link (CI complained too long)
  * add test_bitcount.c entry to MAINTAINERS file

v6:
  * remove stray #include 

v5:
  * fix implementation of msvc versions of rte_clz{32,64}
incorrect use of _BitscanReverse{,64} index.
  * fix and expand unit test to exercise full range of counting
over uint{32,64}_t input values. (which would have caught
above mistake).
  * reduce commit title length
  * correct commit author

v4:
  * combine unit test commit into function addition commit

v3:
  * rename to use 32/64 instead of l/ll suffixes
  * add new functions to rte_bitops.h instead of new header
  * move other bit functions from rte_common.h to rte_bitops.h

v2:
  * use unsigned int instead of unsigned (checkpatches)
  * match multiple include guard naming convention to rte_common.h
  * add explicit extern "C" linkage to rte_bitcount.h
note: not really needed but checkpatches required
  * add missing space around '-'

Tyler Retzlaff (3):
  eal: move bit count functions to bitops header
  eal: provide abstracted bit count functions
  maintainers: add bitcount test under EAL API and common code

 MAINTAINERS  |   1 +
 app/test/meson.build |   2 +
 app/test/test_bitcount.c | 136 
 app/test/test_common.c   |   1 +
 lib/eal/common/rte_reciprocal.c  |   1 +
 lib/eal/include/rte_bitops.h | 532 +++
 lib/eal/include/rte_common.h | 293 -
 lib/pipeline/rte_swx_pipeline_internal.h |   1 +
 8 files changed, 674 insertions(+), 293 deletions(-)
 create mode 100644 app/test/test_bitcount.c

-- 
1.8.3.1

[PATCH v8 1/3] eal: move bit count functions to bitops header

[Tyler Retzlaff]

Move the following inline functions from rte_common.h to rte_bitops.h

  rte_combine32ms1b
  rte_combine64ms1b
  rte_bsf32
  rte_bsf32_safe
  rte_bsf64
  rte_bsf64_safe
  rte_fls_u32
  rte_fls_u64
  rte_is_power_of_2
  rte_align32pow2
  rte_align32prevpow2
  rte_align64pow2
  rte_align64prevpow2
  rte_log2_u32
  rte_log2_u64

Signed-off-by: Tyler Retzlaff 
Acked-by: Ferruh Yigit 
Acked-by: Morten Brørup 
Acked-by: Bruce Richardson 
---
 app/test/test_common.c   |   1 +
 lib/eal/common/rte_reciprocal.c  |   1 +
 lib/eal/include/rte_bitops.h | 292 ++
 lib/eal/include/rte_common.h | 293 ---
 lib/pipeline/rte_swx_pipeline_internal.h |   1 +
 5 files changed, 295 insertions(+), 293 deletions(-)

diff --git a/app/test/test_common.c b/app/test/test_common.c
index f89e1eb..cf4a2c7 100644
--- a/app/test/test_common.c
+++ b/app/test/test_common.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
diff --git a/lib/eal/common/rte_reciprocal.c b/lib/eal/common/rte_reciprocal.c
index 42dfa44..d47dc47 100644
--- a/lib/eal/common/rte_reciprocal.c
+++ b/lib/eal/common/rte_reciprocal.c
@@ -8,6 +8,7 @@
 #include 
 
 #include 
+#include 
 
 #include "rte_reciprocal.h"
 
diff --git a/lib/eal/include/rte_bitops.h b/lib/eal/include/rte_bitops.h
index f50dbe4..531479e 100644
--- a/lib/eal/include/rte_bitops.h
+++ b/lib/eal/include/rte_bitops.h
@@ -275,6 +275,298 @@
return val & mask;
 }
 
+/**
+ * Combines 32b inputs most significant set bits into the least
+ * significant bits to construct a value with the same MSBs as x
+ * but all 1's under it.
+ *
+ * @param x
+ *The integer whose MSBs need to be combined with its LSBs
+ * @return
+ *The combined value.
+ */
+static inline uint32_t
+rte_combine32ms1b(uint32_t x)
+{
+   x |= x >> 1;
+   x |= x >> 2;
+   x |= x >> 4;
+   x |= x >> 8;
+   x |= x >> 16;
+
+   return x;
+}
+
+/**
+ * Combines 64b inputs most significant set bits into the least
+ * significant bits to construct a value with the same MSBs as x
+ * but all 1's under it.
+ *
+ * @param v
+ *The integer whose MSBs need to be combined with its LSBs
+ * @return
+ *The combined value.
+ */
+static inline uint64_t
+rte_combine64ms1b(uint64_t v)
+{
+   v |= v >> 1;
+   v |= v >> 2;
+   v |= v >> 4;
+   v |= v >> 8;
+   v |= v >> 16;
+   v |= v >> 32;
+
+   return v;
+}
+
+/**
+ * Searches the input parameter for the least significant set bit
+ * (starting from zero).
+ * If a least significant 1 bit is found, its bit index is returned.
+ * If the content of the input parameter is zero, then the content of the 
return
+ * value is undefined.
+ * @param v
+ * input parameter, should not be zero.
+ * @return
+ * least significant set bit in the input parameter.
+ */
+static inline uint32_t
+rte_bsf32(uint32_t v)
+{
+   return (uint32_t)__builtin_ctz(v);
+}
+
+/**
+ * Searches the input parameter for the least significant set bit
+ * (starting from zero). Safe version (checks for input parameter being zero).
+ *
+ * @warning ``pos`` must be a valid pointer. It is not checked!
+ *
+ * @param v
+ * The input parameter.
+ * @param pos
+ * If ``v`` was not 0, this value will contain position of least 
significant
+ * bit within the input parameter.
+ * @return
+ * Returns 0 if ``v`` was 0, otherwise returns 1.
+ */
+static inline int
+rte_bsf32_safe(uint32_t v, uint32_t *pos)
+{
+   if (v == 0)
+   return 0;
+
+   *pos = rte_bsf32(v);
+   return 1;
+}
+
+/**
+ * Searches the input parameter for the least significant set bit
+ * (starting from zero).
+ * If a least significant 1 bit is found, its bit index is returned.
+ * If the content of the input parameter is zero, then the content of the 
return
+ * value is undefined.
+ * @param v
+ * input parameter, should not be zero.
+ * @return
+ * least significant set bit in the input parameter.
+ */
+static inline uint32_t
+rte_bsf64(uint64_t v)
+{
+   return (uint32_t)__builtin_ctzll(v);
+}
+
+/**
+ * Searches the input parameter for the least significant set bit
+ * (starting from zero). Safe version (checks for input parameter being zero).
+ *
+ * @warning ``pos`` must be a valid pointer. It is not checked!
+ *
+ * @param v
+ * The input parameter.
+ * @param pos
+ * If ``v`` was not 0, this value will contain position of least 
significant
+ * bit within the input parameter.
+ * @return
+ * Returns 0 if ``v`` was 0, otherwise returns 1.
+ */
+static inline int
+rte_bsf64_safe(uint64_t v, uint32_t *pos)
+{
+   if (v == 0)
+   return 0;
+
+   *pos = rte_bsf64(v);
+   return 1;
+}
+
+/**
+ * Return the last (most-significant) bit set.
+ *
+ * @note The last (most significant) bit is at position 32.
+ * @note rte_fls_u32(0) = 0, rte_fls_u32(1) = 1, rte_fls_u32(0x8000) = 

[PATCH v8 3/3] maintainers: add bitcount test under EAL API and common code

[Tyler Retzlaff]

List app/test/test_bitcount.c under EAL API and common code.

Signed-off-by: Tyler Retzlaff 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8df23e5..c2995bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -155,6 +155,7 @@ F: doc/guides/prog_guide/env_abstraction_layer.rst
 F: app/test/test_alarm.c
 F: app/test/test_atomic.c
 F: app/test/test_barrier.c
+F: app/test/test_bitcount.c
 F: app/test/test_byteorder.c
 F: app/test/test_common.c
 F: app/test/test_cpuflags.c
-- 
1.8.3.1

[PATCH v8 2/3] eal: provide abstracted bit count functions

[Tyler Retzlaff]

Provide abstracted bit counting functions for count, leading and
trailing bits in v to hide compiler specific intrinsics and builtins.

Include basic unit test of following functions added.

rte_clz32
rte_clz64
rte_ctz32
rte_ctz64
rte_popcount32
rte_popcount64

Signed-off-by: Tyler Retzlaff 
Acked-by: Morten Brørup 
Acked-by: Bruce Richardson 
---
 app/test/meson.build |   2 +
 app/test/test_bitcount.c | 136 
 lib/eal/include/rte_bitops.h | 240 +++
 3 files changed, 378 insertions(+)
 create mode 100644 app/test/test_bitcount.c

diff --git a/app/test/meson.build b/app/test/meson.build
index b9b5432..dafd509 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -13,6 +13,7 @@ test_sources = files(
 'test_alarm.c',
 'test_atomic.c',
 'test_barrier.c',
+'test_bitcount.c',
 'test_bitops.c',
 'test_bitmap.c',
 'test_bpf.c',
@@ -161,6 +162,7 @@ test_deps += ['bus_pci', 'bus_vdev']
 fast_tests = [
 ['acl_autotest', true, true],
 ['atomic_autotest', false, true],
+['bitcount_autotest', true, true],
 ['bitmap_autotest', true, true],
 ['bpf_autotest', true, true],
 ['bpf_convert_autotest', true, true],
diff --git a/app/test/test_bitcount.c b/app/test/test_bitcount.c
new file mode 100644
index 000..5287ef7
--- /dev/null
+++ b/app/test/test_bitcount.c
@@ -0,0 +1,136 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include 
+#include 
+
+#include 
+#include 
+
+#include "test.h"
+
+RTE_LOG_REGISTER(bitcount_logtype_test, test.bitcount, INFO);
+
+static int
+test_clz32(void)
+{
+   size_t leading;
+   uint32_t v = 0x;
+
+   for (leading = 0; v; leading++) {
+   RTE_TEST_ASSERT(rte_clz32(v) == leading,
+   "Unexpected count.");
+   v >>= 1;
+   }
+
+   return 0;
+}
+
+static int
+test_clz64(void)
+{
+   size_t leading;
+   uint64_t v = 0x;
+
+   for (leading = 0; v; leading++) {
+   RTE_TEST_ASSERT(rte_clz64(v) == leading,
+   "Unexpected count.");
+   v >>= 1;
+   }
+
+   return 0;
+}
+
+static int
+test_ctz32(void)
+{
+   size_t trailing;
+   uint32_t v = 1;
+
+   for (trailing = 0; v; trailing++) {
+   RTE_TEST_ASSERT(rte_ctz32(v) == trailing,
+   "Unexpected count.");
+   v <<= 1;
+   }
+
+   return 0;
+}
+
+static int
+test_ctz64(void)
+{
+   size_t trailing;
+   uint64_t v = 1;
+
+   for (trailing = 0; v; trailing++) {
+   RTE_TEST_ASSERT(rte_ctz64(v) == trailing,
+   "Unexpected count.");
+   v <<= 1;
+   }
+
+   return 0;
+}
+
+static int
+test_popcount32(void)
+{
+   size_t shift;
+   uint32_t v = 0;
+   const size_t bits = sizeof(v) * CHAR_BIT;
+
+   for (shift = 0; shift < bits; shift++) {
+   RTE_TEST_ASSERT(rte_popcount32(v) == shift,
+   "Unexpected count.");
+   v <<= 1;
+   v |= 1;
+   }
+
+   RTE_TEST_ASSERT(rte_popcount32(v) == bits,
+   "Unexpected count.");
+
+   return 0;
+}
+
+static int
+test_popcount64(void)
+{
+   size_t shift;
+   uint64_t v = 0;
+   const size_t bits = sizeof(v) * CHAR_BIT;
+
+   for (shift = 0; shift < bits; shift++) {
+   RTE_TEST_ASSERT(rte_popcount64(v) == shift,
+   "Unexpected count.");
+   v <<= 1;
+   v |= 1;
+   }
+
+   RTE_TEST_ASSERT(rte_popcount64(v) == bits,
+   "Unexpected count.");
+
+   return 0;
+}
+
+static struct unit_test_suite bitcount_test_suite = {
+   .suite_name = "bitcount autotest",
+   .setup = NULL,
+   .teardown = NULL,
+   .unit_test_cases = {
+   TEST_CASE(test_clz32),
+   TEST_CASE(test_clz64),
+   TEST_CASE(test_ctz32),
+   TEST_CASE(test_ctz64),
+   TEST_CASE(test_popcount32),
+   TEST_CASE(test_popcount64),
+   TEST_CASES_END()
+   }
+};
+
+static int
+test_bitcount(void)
+{
+   return unit_test_suite_runner(&bitcount_test_suite);
+}
+
+REGISTER_TEST_COMMAND(bitcount_autotest, test_bitcount);
diff --git a/lib/eal/include/rte_bitops.h b/lib/eal/include/rte_bitops.h
index 531479e..d45aa54 100644
--- a/lib/eal/include/rte_bitops.h
+++ b/lib/eal/include/rte_bitops.h
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2020 Arm Limited
+ * Copyright(c) 2010-2019 Intel Corporation
+ * Copyright(c) 2023 Microsoft Corporation
  */
 
 #ifndef _RTE_BITOPS_H_
@@ -275,6 +277,244 @@
return val & mask;
 }
 
+#ifdef RTE_TOOLCHAIN_MSVC
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior not

[PATCH v2] devtools: stop compiler atomics with no C11 equivalent

[Tyler Retzlaff]

Refrain from using compiler __atomic_{add,and,nand,or,sub,xor}_fetch()
to ease future adoption of C11 standard atomics.

Signed-off-by: Tyler Retzlaff 
---
 devtools/checkpatches.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index a07bbc8..1b6841b 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -119,6 +119,14 @@ check_forbidden_additions() { # 
-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
"$1" || res=1
 
+   # refrain from using compiler __atomic_{add,and,nand,or,sub,xor}_fetch()
+   awk -v FOLDERS="lib drivers app examples" \
+   -v EXPRESSIONS="__atomic_(add|and|nand|or|sub|xor)_fetch\\\(" \
+   -v RET_ON_FAIL=1 \
+   -v MESSAGE='Using __atomic_op_fetch use __atomic_fetch_op 
instead' \
+   -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
+   "$1" || res=1
+
# forbid use of __reserved which is a reserved keyword in Windows 
system headers
awk -v FOLDERS="lib drivers app examples" \
-v EXPRESSIONS='\\<__reserved\\>' \
-- 
1.8.3.1

RE: Testpmd/l3fwd port shutdown failure on Arm Altra systems

[Yang, Qiming]

Hi, Juraj
I don’t know VPP. Can I narrow down your question? Do means you run testpmd and 
l3fwd by these cmd in an ARM system but crush?
> sudo build/app/dpdk-testpmd -v -l 1,2 -a 0004:04:00.1 -a 0004:04:00.0
> > > > --in-memory -- -i --forward-mode=io --burst=64 --txq=1 --rxq=1
> > > > --tx-offloads=0x0 --numa --auto-start --total-num-mbufs=32768
> > > > --nb-ports=2 --portmask=0x3 --max-pkt-len=1518 --mbuf-size=16384
> > > > --nb-cores=1
> > > >
> > > >
> > > >
> > > > And l3fwd (with different macs on the other server):
> > > >
> > > > sudo /tmp/openvpp-testing/dpdk/build/examples/dpdk-l3fwd -v -l 1,2 -a
> > > > 0004:04:00.0 -a 0004:04:00.1 --in-memory -- --parse-ptype
> > > > --eth-dest="0,40:a6:b7:85:e7:79" --eth-dest="1,3c:fd:fe:c3:e7:a1"
> > > > --config="(0, 0, 2),(1, 0, 2)" -P -L -p 0x3

Qiming

From: Juraj Linkeš 
Sent: Monday, April 3, 2023 5:27 PM
To: Xing, Beilei 
Cc: Singh, Aman Deep ; Zhang, Yuying 
; Yang, Qiming ; dev@dpdk.org; 
Ruifeng Wang ; Zhang, Lijian ; 
Honnappa Nagarahalli 
Subject: Re: Testpmd/l3fwd port shutdown failure on Arm Altra systems

Hello Qiming, Beilei,

Could you please help us debug this issue? Anything that would help with 
getting to the bottom of anything that could go wrong during port init/cleanup 
would be appreciated - extra eal/testpmd options or even code changes (such as 
where could add extra debug messages).

Thanks,
Juraj

On Wed, Mar 8, 2023 at 7:25 AM Juraj Linkeš 
mailto:juraj.lin...@pantheon.tech>> wrote:
Hello Qiming, Beilei,

Another reminder - are you looking at this by any chance?

The high level short description is that testpmd/l3fwd breaks a link
between two servers while VPP (using DPDK) doesn't. This leads us to
believe there's a problem with testpmd/l3fwd/i40e driver in DPDK.

Thanks,
Juraj

On Tue, Feb 21, 2023 at 12:18 PM Juraj Linkeš
mailto:juraj.lin...@pantheon.tech>> wrote:
>
> Hi Qiming,
>
> Just a friendly reminder, would you please take a look?
>
> Thanks,
> Juraj
>
>
> On Tue, Feb 7, 2023 at 3:10 AM Xing, Beilei 
> mailto:beilei.x...@intel.com>> wrote:
> >
> > Hi Qiming,
> >
> > Could you please help on this? Thanks.
> >
> > BR,
> > Beilei
> >
> > > -Original Message-
> > > From: Juraj Linkeš 
> > > mailto:juraj.lin...@pantheon.tech>>
> > > Sent: Monday, February 6, 2023 4:53 PM
> > > To: Singh, Aman Deep 
> > > mailto:aman.deep.si...@intel.com>>; Zhang, 
> > > Yuying
> > > mailto:yuying.zh...@intel.com>>; Xing, Beilei 
> > > mailto:beilei.x...@intel.com>>
> > > Cc: dev@dpdk.org; Ruifeng Wang 
> > > mailto:ruifeng.w...@arm.com>>; Zhang, Lijian
> > > mailto:lijian.zh...@arm.com>>; Honnappa Nagarahalli
> > > mailto:honnappa.nagaraha...@arm.com>>
> > > Subject: Re: Testpmd/l3fwd port shutdown failure on Arm Altra systems
> > >
> > > Hello i40e and testpmd maintainers,
> > >
> > > A gentle reminder - would you please advise how to debug the issue 
> > > described
> > > below?
> > >
> > > Thanks,
> > > Juraj
> > >
> > > On Fri, Jan 20, 2023 at 1:07 PM Juraj Linkeš 
> > > mailto:juraj.lin...@pantheon.tech>>
> > > wrote:
> > > >
> > > > Adding the logfile.
> > > >
> > > >
> > > >
> > > > One thing that's in the logs but didn't explicitly mention is the DPDK 
> > > > version
> > > we've tried this with:
> > > >
> > > > EAL: RTE Version: 'DPDK 22.07.0'
> > > >
> > > >
> > > >
> > > > We also tried earlier versions going back to 21.08, with no luck. I 
> > > > also did a
> > > quick check on 22.11, also with no luck.
> > > >
> > > >
> > > >
> > > > Juraj
> > > >
> > > >
> > > >
> > > > From: Juraj Linkeš
> > > > Sent: Friday, January 20, 2023 12:56 PM
> > > > To: 'aman.deep.si...@intel.com' 
> > > > mailto:aman.deep.si...@intel.com>>;
> > > > 'yuying.zh...@intel.com' 
> > > > mailto:yuying.zh...@intel.com>>; Xing, Beilei
> > > > mailto:beilei.x...@intel.com>>
> > > > Cc: dev@dpdk.org; Ruifeng Wang 
> > > > mailto:ruifeng.w...@arm.com>>; 'Lijian Zhang'
> > > > mailto:lijian.zh...@arm.com>>; 'Honnappa 
> > > > Nagarahalli'
> > > > mailto:honnappa.nagaraha...@arm.com>>
> > > > Subject: Testpmd/l3fwd port shutdown failure on Arm Altra systems
> > > >
> > > >
> > > >
> > > > Hello i40e and testpmd maintainers,
> > > >
> > > >
> > > >
> > > > We're hitting an issue with DPDK testpmd on Ampere Altra servers in 
> > > > FD.io
> > > lab.
> > > >
> > > >
> > > >
> > > > A bit of background: along with VPP performance tests (which uses DPDK),
> > > we're running a small number of basic DPDK testpmd and l3fwd tests in 
> > > FD.io
> > > as well. This is to catch any performance differences due to VPP updating 
> > > its
> > > DPDK version.
> > > >
> > > >
> > > >
> > > > We're running both l3fwd tests and testpmd tests. The Altra servers are 
> > > > two
> > > socket and the topology is TG -> DUT1 -> DUT2 -> TG, traffic flows in both
> > > directions, but nothing gets forwarded (with a slight caveat - put a pin 
> > > in this).
> > > There'

Re: [PATCH] doc/ark: replace word segregation

[Stephen Hemminger]

On Tue, 4 Apr 2023 00:06:16 +0100
Luca Boccassi  wrote:

> >  * Dynamic per-queue MBUF (re)sizing up to 32KB
> > -* SR-IOV, VF-based queue-segregation
> > +* SR-IOV, VF-based queue-seperation  
> 
> Did you mean 'separation' by any chance

Yup. It is one of the words I always misspell...

Re: [PATCH v2] devtools: add script to check for non inclusive naming

[Stephen Hemminger]

On Tue, 4 Apr 2023 00:08:30 +0100
Luca Boccassi  wrote:

> On Mon, 3 Apr 2023 at 15:47, Stephen Hemminger
>  wrote:
> >
> > Shell script to find use of words that not be used.
> > By default it prints matches.  The -q (quiet) option
> > is used to just count. There is also -l option
> > which lists lines matching (like grep -l).
> >
> > Uses the word lists from Inclusive Naming Initiative
> > see https://inclusivenaming.org/word-lists/
> >
> > Examples:
> >  $ ./devtools/check-naming-policy.sh -q
> >  Total files: 37 errors, 90 warnings, 2 suggestions
> >
> >  $ ./devtools/check-naming-policy.sh -q -l lib/eal
> >  Total lines: 32 errors, 8 warnings, 0 suggestions
> >
> > Add MAINTAINERS file entry for the new tool and resort
> > the list files back into to alphabetic order
> >
> > Signed-off-by: Stephen Hemminger 
> > ---
> > v2 - fix typo in words
> >- add subtree (pathspec) option
> >- update maintainers file (and fix alphabetic order)  
> 
> There's a json file on the website, how about downloading that on the
> fly rather than storing a local copy that will go out of date?
> https://inclusivenaming.org/word-lists/index.json

Ok, but that would mean using python and would also mean that terms like
segreation which are not on the official list would not be caught

Re: [PATCH v2] eal: introduce atomics abstraction

[Tyler Retzlaff]

On Mon, Apr 03, 2023 at 11:11:35PM +0200, Mattias Rönnblom wrote:
> On 2023-02-08 22:43, Tyler Retzlaff wrote:
> >Introduce atomics abstraction that permits optional use of standard C11
> >atomics when meson is provided the new enable_stdatomics=true option.
> >
> 
> Terminology nitpicking: I don't think these functions provide any
> abstraction at all. They are just wrappers.
> 
> >Signed-off-by: Tyler Retzlaff 
> >---
> >  config/meson.build | 11 
> >  lib/eal/arm/include/rte_atomic_32.h|  6 ++-
> >  lib/eal/arm/include/rte_atomic_64.h|  6 ++-
> >  lib/eal/include/generic/rte_atomic.h   | 96 
> > +-
> >  lib/eal/loongarch/include/rte_atomic.h |  6 ++-
> >  lib/eal/ppc/include/rte_atomic.h   |  6 ++-
> >  lib/eal/riscv/include/rte_atomic.h |  6 ++-
> >  lib/eal/x86/include/rte_atomic.h   |  8 ++-
> >  meson_options.txt  |  2 +
> >  9 files changed, 139 insertions(+), 8 deletions(-)
> >
> >diff --git a/config/meson.build b/config/meson.build
> >index 26f3168..25dd628 100644
> >--- a/config/meson.build
> >+++ b/config/meson.build
> >@@ -255,6 +255,17 @@ endif
> >  # add -include rte_config to cflags
> >  add_project_arguments('-include', 'rte_config.h', language: 'c')
> >+stdc_atomics_enabled = get_option('enable_stdatomics')
> >+dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
> >+
> >+if stdc_atomics_enabled
> >+if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
> >+add_project_arguments('-std=gnu11', language: 'c')
> >+else
> >+add_project_arguments('-std=c11', language: 'c')
> >+endif
> >+endif
> >+
> >  # enable extra warnings and disable any unwanted warnings
> >  # -Wall is added by default at warning level 1, and -Wextra
> >  # at warning level 2 (DPDK default)
> >diff --git a/lib/eal/arm/include/rte_atomic_32.h 
> >b/lib/eal/arm/include/rte_atomic_32.h
> >index c00ab78..7088a12 100644
> >--- a/lib/eal/arm/include/rte_atomic_32.h
> >+++ b/lib/eal/arm/include/rte_atomic_32.h
> >@@ -34,9 +34,13 @@
> >  #define rte_io_rmb() rte_rmb()
> >  static __rte_always_inline void
> >-rte_atomic_thread_fence(int memorder)
> >+rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> >+#ifdef RTE_STDC_ATOMICS
> >+atomic_thread_fence(memorder);
> >+#else
> > __atomic_thread_fence(memorder);
> >+#endif
> >  }
> >  #ifdef __cplusplus
> >diff --git a/lib/eal/arm/include/rte_atomic_64.h 
> >b/lib/eal/arm/include/rte_atomic_64.h
> >index 6047911..7f02c57 100644
> >--- a/lib/eal/arm/include/rte_atomic_64.h
> >+++ b/lib/eal/arm/include/rte_atomic_64.h
> >@@ -38,9 +38,13 @@
> >  #define rte_io_rmb() rte_rmb()
> >  static __rte_always_inline void
> >-rte_atomic_thread_fence(int memorder)
> >+rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> >+#ifdef RTE_STDC_ATOMICS
> >+atomic_thread_fence(memorder);
> >+#else
> > __atomic_thread_fence(memorder);
> >+#endif
> >  }
> >  /* 128 bit atomic operations 
> > -*/
> >diff --git a/lib/eal/include/generic/rte_atomic.h 
> >b/lib/eal/include/generic/rte_atomic.h
> >index f5c49a9..392d928 100644
> >--- a/lib/eal/include/generic/rte_atomic.h
> >+++ b/lib/eal/include/generic/rte_atomic.h
> >@@ -110,6 +110,100 @@
> >  #endif /* __DOXYGEN__ */
> >+#ifdef RTE_STDC_ATOMICS
> >+
> >+#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L || 
> >defined(__STDC_NO_ATOMICS__)
> >+#error compiler does not support C11 standard atomics
> >+#else
> >+#include 
> >+#endif
> >+
> >+#define __rte_atomic _Atomic
> >+
> >+typedef int rte_memory_order;
> >+
> >+#define rte_memory_order_relaxed memory_order_relaxed
> >+#define rte_memory_order_consume memory_order_consume
> >+#define rte_memory_order_acquire memory_order_acquire
> >+#define rte_memory_order_release memory_order_release
> >+#define rte_memory_order_acq_rel memory_order_acq_rel
> >+#define rte_memory_order_seq_cst memory_order_seq_cst
> >+
> 
> Would this be better of as an enum, rather than a typedef? If
> typedef, it should have the "_t" postfix. Also, the #define should
> be all-caps.
> 
> >+#define rte_atomic_store_explicit(obj, desired, order) \
> >+atomic_store_explicit(obj, desired, order)
> >+
> 
> Drop "explicit" from all the names. It's just noise. Also, the
> memory orders have very long names.
> 
> We haven't even move all DPDK code over from the old API, to using
> GCC C11 built-ins, and now we are switching to a new API?

This series has been marked obsolete/abandoned.  As per technical board
DPDK will move straight to standard C atomics without transitioning with
these macros.

Thanks

RE: ARM power-saving measurements

[Feifei Wang]

Hi, Alex

Thanks for your attention.
Following I will share how to measure the power savings in Arm:

1.Test Setup:
1.1  ensure the dpdk version has been applied the wfe patch:
http://patches.dpdk.org/project/dpdk/cover/20230220085109.3463640-1-feifei.wa...@arm.com/
This is due to dpdk cannot support l3fwd-power  'power monitor' in arm before 
this patch

1.2 enable wfe instructions in arm:
dpdk/config/arm/meson.build:
'RTE_ARM_USE_WFE', false -> 'RTE_ARM_USE_WFE', true

1.3 build l3fwd-power
meson -Dexamples=l3fwd-power build  &&  cd build  &&  ninja

2. Test Command:
L3fwd-power uses 'power monitor' mode
sudo ./examples/dpdk-l3fwd-power -l 14-15 -n 4 -a :13:00.0 -a :13:00.1 
-- -p 0x3 -P --config='(0,0,14),(1,0,15)' --pmd-mgmt=monitor
note: power monitor can just support more than 1 cores.

3. Power saving count:
For arm, wfe instructions can let CPU go into sleeping mode if it is always 
idle.
Thus, we have 2 ways to collect power saving info.

3.1 Using perf to record dynamic instruction number within 1 second:
$sudo perf stat -C , and then we can get CPU instructions number during 
a fixed time slot.
For examples:
Performance counter stats for 'CPU(s) 14':
  3,506.56 msec cpu-clock #1.000 CPUs utilized
 0  context-switches  #0.000 K/sec
 0  cpu-migrations#0.000 K/sec
 0  page-faults   #0.000 K/sec
 8,768,815,778  cycles#2.501 GHz
26,071,802  instructions  #0.00  insn per cycle
 branches
   104,982  branch-misses

   3.506978873 seconds time elapsed

Dynamic instructions number for 1 second = 26,071,802/3.506978873

If we disable wfe in arm, we can find 'dynamic instructions number for 1 second'
increases significantly. This also means CPU is running at full load.

3.2 Get CPU power form BMC:
For ampere altra and thunderx2 server, we can look up CPU timely power from BMC.
For example, when we run l3fwd-power in thunderx2, server 
information->power->consumption:
---
CPU0 Current Power Consumption  69 W | 235 BTU/hr
CPU1 Current Power Consumption  0 W | 0 BTU/hr
CPU Power Limit Management Activated
CPU Power Limit in Watts(1-150) 
---
If we disable wfe, CPU0 Current Power Consumption will increase, then we can 
know how much power we can save.

Best Regards
Feifei

> -Original Message-
> From: Alexander Kozyrev 
> Sent: Saturday, April 1, 2023 1:29 AM
> To: Feifei Wang ; Ruifeng Wang
> 
> Cc: dev@dpdk.org
> Subject: RE: ARM power-saving measurements
> 
> Hi Feifei/Ruifeng, any advice on power saving measurements on ARM?
> 
> Regards,
> Alex
> 
> > -Original Message-
> > From: Alexander Kozyrev
> > Sent: March 13, 2023 18:08
> > To: Feifei Wang ; ruifeng.w...@arm.com
> > Subject: ARM power-saving measurements
> >
> > Hi Feifei/ Ruifeng, I can see that power management support on ARM has
> > been integrated successfully.
> > Could you please share the knowledge on how to measure the power
> > savings on this architecture?
> > What are the tools available there? Unfortunately, turbostat is not
> > supported on ARM. How did you test your feature?
> >
> > Regards,
> > Alex

Re: [dpdk-dev][PATCH] drivers: optimize the build time for cnxk

[Jerin Jacob]

On Fri, Nov 11, 2022 at 10:53 AM  wrote:
>
> From: Kiran Kumar K 
>
> While building cnxk, if build platform is cn9k, cn10k files
> are also being compiled and vice versa. This is causing more
> build time. Adding changes to avoid this by checking the
> platform and compile only platform specific files. If no
> platform is provided, both cn9k and cn10k files will be compiled.
>
> Signed-off-by: Kiran Kumar K 


Applied to dpdk-next-net-mrvl/for-next-net. Thanks


> ---
>  drivers/event/cnxk/cn9k_eventdev.c | 16 
>  drivers/event/cnxk/cnxk_eventdev.c | 14 ++
>  drivers/event/cnxk/cnxk_eventdev.h |  1 +
>  drivers/event/cnxk/meson.build | 22 ++
>  drivers/net/cnxk/meson.build   | 14 ++
>  5 files changed, 47 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/event/cnxk/cn9k_eventdev.c 
> b/drivers/event/cnxk/cn9k_eventdev.c
> index f5a42a86f8..7b09f27644 100644
> --- a/drivers/event/cnxk/cn9k_eventdev.c
> +++ b/drivers/event/cnxk/cn9k_eventdev.c
> @@ -6,7 +6,6 @@
>  #include "cnxk_eventdev.h"
>  #include "cnxk_worker.h"
>
> -#define CN9K_DUAL_WS_NB_WS 2
>  #define CN9K_DUAL_WS_PAIR_ID(x, id) (((x)*CN9K_DUAL_WS_NB_WS) + id)
>
>  #define CN9K_SET_EVDEV_DEQ_OP(dev, deq_op, deq_ops)  
>   \
> @@ -239,21 +238,6 @@ cn9k_sso_hws_reset(void *arg, void *hws)
> ws->swtag_req = 0;
>  }
>
> -void
> -cn9k_sso_set_rsrc(void *arg)
> -{
> -   struct cnxk_sso_evdev *dev = arg;
> -
> -   if (dev->dual_ws)
> -   dev->max_event_ports = dev->sso.max_hws / CN9K_DUAL_WS_NB_WS;
> -   else
> -   dev->max_event_ports = dev->sso.max_hws;
> -   dev->max_event_queues =
> -   dev->sso.max_hwgrp > RTE_EVENT_MAX_QUEUES_PER_DEV ?
> - RTE_EVENT_MAX_QUEUES_PER_DEV :
> - dev->sso.max_hwgrp;
> -}
> -
>  static int
>  cn9k_sso_rsrc_init(void *arg, uint8_t hws, uint8_t hwgrp)
>  {
> diff --git a/drivers/event/cnxk/cnxk_eventdev.c 
> b/drivers/event/cnxk/cnxk_eventdev.c
> index db62d32a81..efa9359ce6 100644
> --- a/drivers/event/cnxk/cnxk_eventdev.c
> +++ b/drivers/event/cnxk/cnxk_eventdev.c
> @@ -623,3 +623,17 @@ cnxk_sso_remove(struct rte_pci_device *pci_dev)
>  {
> return rte_event_pmd_pci_remove(pci_dev, cnxk_sso_fini);
>  }
> +
> +void
> +cn9k_sso_set_rsrc(void *arg)
> +{
> +   struct cnxk_sso_evdev *dev = arg;
> +
> +   if (dev->dual_ws)
> +   dev->max_event_ports = dev->sso.max_hws / CN9K_DUAL_WS_NB_WS;
> +   else
> +   dev->max_event_ports = dev->sso.max_hws;
> +   dev->max_event_queues = dev->sso.max_hwgrp > 
> RTE_EVENT_MAX_QUEUES_PER_DEV ?
> +   RTE_EVENT_MAX_QUEUES_PER_DEV :
> +   dev->sso.max_hwgrp;
> +}
> diff --git a/drivers/event/cnxk/cnxk_eventdev.h 
> b/drivers/event/cnxk/cnxk_eventdev.h
> index 738e335ea4..fdbcfb4640 100644
> --- a/drivers/event/cnxk/cnxk_eventdev.h
> +++ b/drivers/event/cnxk/cnxk_eventdev.h
> @@ -56,6 +56,7 @@
>  #define CNXK_TAG_IS_HEAD(x)(BIT_ULL(35) & x)
>
>  #define CN9K_SSOW_GET_BASE_ADDR(_GW) ((_GW)-SSOW_LF_GWS_OP_GET_WORK0)
> +#define CN9K_DUAL_WS_NB_WS  2
>
>  #define CN10K_GW_MODE_NONE 0
>  #define CN10K_GW_MODE_PREF 1
> diff --git a/drivers/event/cnxk/meson.build b/drivers/event/cnxk/meson.build
> index aa42ab3a90..227c6ae7a8 100644
> --- a/drivers/event/cnxk/meson.build
> +++ b/drivers/event/cnxk/meson.build
> @@ -8,11 +8,17 @@ if not is_linux or not dpdk_conf.get('RTE_ARCH_64')
>  subdir_done()
>  endif
>
> +if meson.is_cross_build()
> +soc_type = meson.get_cross_property('platform', '')
> +else
> +soc_type = platform
> +endif
> +
> +if soc_type != 'cn9k' and soc_type != 'cn10k'
> +soc_type = 'all'
> +endif
> +
>  sources = files(
> -'cn9k_eventdev.c',
> -'cn9k_worker.c',
> -'cn10k_eventdev.c',
> -'cn10k_worker.c',
>  'cnxk_eventdev.c',
>  'cnxk_eventdev_adptr.c',
>  'cnxk_eventdev_selftest.c',
> @@ -21,7 +27,10 @@ sources = files(
>  'cnxk_tim_worker.c',
>  )
>
> +if soc_type == 'cn9k' or soc_type == 'all'
>  sources += files(
> +'cn9k_eventdev.c',
> +'cn9k_worker.c',
>  'deq/cn9k/deq_0_15_burst.c',
>  'deq/cn9k/deq_16_31_burst.c',
>  'deq/cn9k/deq_32_47_burst.c',
> @@ -320,8 +329,12 @@ sources += files(
>  'tx/cn9k/tx_96_111_dual_seg.c',
>  'tx/cn9k/tx_112_127_dual_seg.c',
>  )
> +endif
>
> +if soc_type == 'cn10k' or soc_type == 'all'
>  sources += files(
> +'cn10k_eventdev.c',
> +'cn10k_worker.c',
>  'deq/cn10k/deq_0_15_burst.c',
>  'deq/cn10k/deq_16_31_burst.c',
>  'deq/cn10k/deq_32_47_burst.c',
> @@ -470,6 +483,7 @@ sources += files(
>  'tx/cn10k/tx_96_111_seg.c',
>  'tx/cn10k/tx_112_127_seg.c',
>  )
> +endif
>
>  extra_flags = 

[PATCH] net/hns3: fix build warning

[jerinj]

From: Jerin Jacob 

aarch64 gcc 12.2.0 build complain with below warning[1].
Move the new_link initialization upwards to fix the warning.

Compiling C object drivers/libtmp_rte_net_hns3.a.p/net_hns3_hns3_ethdev.c.o
drivers/net/hns3/hns3_ethdev.c: In function ‘hns3_dev_link_update’:
drivers/net/hns3/hns3_ethdev.c:2249:1: warning: ‘new_link’ may be
used uninitialized [-Wmaybe-uninitialized]

Fixes: 64308555d5bf ("net/hns3: fix link status when port is stopped")
Cc: sta...@dpdk.org

Signed-off-by: Jerin Jacob 
---
 drivers/net/hns3/hns3_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 36896f8989..a872cb8dd7 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2257,6 +2257,7 @@ hns3_dev_link_update(struct rte_eth_dev *eth_dev, int 
wait_to_complete)
struct rte_eth_link new_link;
int ret;
 
+   memset(&new_link, 0, sizeof(new_link));
/* When port is stopped, report link down. */
if (eth_dev->data->dev_started == 0) {
new_link.link_autoneg = mac->link_autoneg;
@@ -2280,7 +2281,6 @@ hns3_dev_link_update(struct rte_eth_dev *eth_dev, int 
wait_to_complete)
rte_delay_ms(HNS3_LINK_CHECK_INTERVAL);
} while (retry_cnt--);
 
-   memset(&new_link, 0, sizeof(new_link));
hns3_setup_linkstatus(eth_dev, &new_link);
 
 out:
-- 
2.40.0