RE: [PATCH] app/testpmd: fix closing softnic port before ethdev ports

[Jangra, Yogesh]

-Original Message-
From: Stephen Hemminger 
Sent: Friday, March 10, 2023 10:15 PM
To: Ferruh Yigit 
Cc: Dumitrescu, Cristian ; Jangra, Yogesh 
; Singh, Aman Deep ; Zhang, 
Yuying ; dev@dpdk.org; R, Kamalakannan 
; Suresh Narayane, Harshad 

Subject: Re: [PATCH] app/testpmd: fix closing softnic port before ethdev ports



On Fri, 10 Mar 2023 13:58:52 +

Ferruh Yigit mailto:ferruh.yi...@amd.com>> wrote:



> >>

> >> Why not fix the misbehaving drivers, instead of working around for

> >> softnic, as Stephen suggested?

> >>

> >> Is there a list of problematic drivers?

> >>

> >

> > Ferruh, I think this is not a reasonable request. We don't have the 
> > expertise to fix all drivers, not the hardware to test all drivers.

> >

>

> Please don't make it over dramatic ;), this is not about having

> expertise in all drivers or having their hardware to test.

>

> You claim some drivers does free up their resources on stop() and

> continue to polling from them cause segfault. Action is move resource

> free from stop() to close().

>

> And my intention was not request a fix from you, if you have any

> particular misbehaving drivers, I can facilitate a fix from those

> driver maintainers.

> Eventually drivers freeing resources in stop() is a bigger problem and

> can hit other applications too, this is not just testpmd problem.



Lets all work together to resolve this.

I and others are willing to review and fix drivers you identify as problematic.

If this is a common problem, ideally we can update CI infrastructure to test 
shutdown and resource issues more thoroughly via ASAN builds etc.







Hi Stephen,



I am getting segmentation error while stopping only the physical port .

I run testpmd application using Soft NIC driver. I used one physical port and 
Soft NIC is running on the top of that.



In my scenario, I started the traffic on the port and when I stop the port I am 
getting segmentation error in rte_eth_rx_burst api.

This shows the order of stopping the port is casing the issue. As stopping the 
port is not handling the service thread stop.



Please refer screenshot for reference.



[cid:image001.png@01D958CD.BE580600]



Thanks & Regards,

Yogesh

Re: [PATCH v2 1/1] test/mbuf: fix mbuf autotest when mbuf debug is enabled

[Olivier Matz]

On Thu, Mar 16, 2023 at 10:14:56PM +0300, Pavel Ivashchenko wrote:
> How to reproduce:
> 
> 1. Define RTE_LIBRTE_MBUF_DEBUG
> 2. MALLOC_PERTURB_=178 DPDK_TEST=mbuf_autotest gdb --args 
> obj-x86_64-linux-gnu/app/test/dpdk-test --file-prefix=mbuf_autotest
> 
>PANIC in rte_mbuf_sanity_check():
>bad pkt_len
> 
>...
>#6  0x77d3d4cc in rte_mbuf_sanity_check (m=m@entry=0x17f8c3400, 
> is_header=is_header@entry=1) at ../lib/mbuf/rte_mbuf.c:384
>#7  0x55653d57 in rte_pktmbuf_free (m=0x17f8c3400) at 
> ../lib/mbuf/rte_mbuf.h:1385
>#8  0x5565c7a6 in test_nb_segs_and_next_reset () at 
> ../app/test/test_mbuf.c:2752
>#9  test_mbuf () at ../app/test/test_mbuf.c:2967
>...
> 
>(gdb) frame 6
>#6  0x77d3d4cc in rte_mbuf_sanity_check (m=m@entry=0x17f8c3400, 
> is_header=is_header@entry=1) at ../lib/mbuf/rte_mbuf.c:384
>384rte_panic("%s\n", reason);
>(gdb) p/d m->pkt_len
>$4 = 1500
> 
> Fixes: efc6f9104c80 ("mbuf: fix reset on mbuf free")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Pavel Ivashchenko 

Acked-by: Olivier Matz 

Re: [PATCH v2 1/1] test/mbuf: fix mbuf autotest when mbuf debug is enabled

[David Marchand]

On Fri, Mar 17, 2023 at 9:03 AM Olivier Matz  wrote:
> On Thu, Mar 16, 2023 at 10:14:56PM +0300, Pavel Ivashchenko wrote:
> > How to reproduce:
> >
> > 1. Define RTE_LIBRTE_MBUF_DEBUG
> > 2. MALLOC_PERTURB_=178 DPDK_TEST=mbuf_autotest gdb --args 
> > obj-x86_64-linux-gnu/app/test/dpdk-test --file-prefix=mbuf_autotest
> >
> >PANIC in rte_mbuf_sanity_check():
> >bad pkt_len
> >
> >...
> >#6  0x77d3d4cc in rte_mbuf_sanity_check (m=m@entry=0x17f8c3400, 
> > is_header=is_header@entry=1) at ../lib/mbuf/rte_mbuf.c:384
> >#7  0x55653d57 in rte_pktmbuf_free (m=0x17f8c3400) at 
> > ../lib/mbuf/rte_mbuf.h:1385
> >#8  0x5565c7a6 in test_nb_segs_and_next_reset () at 
> > ../app/test/test_mbuf.c:2752
> >#9  test_mbuf () at ../app/test/test_mbuf.c:2967
> >...
> >
> >(gdb) frame 6
> >#6  0x77d3d4cc in rte_mbuf_sanity_check (m=m@entry=0x17f8c3400, 
> > is_header=is_header@entry=1) at ../lib/mbuf/rte_mbuf.c:384
> >384rte_panic("%s\n", reason);
> >(gdb) p/d m->pkt_len
> >$4 = 1500
> >
> > Fixes: efc6f9104c80 ("mbuf: fix reset on mbuf free")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Pavel Ivashchenko 
> Acked-by: Olivier Matz 

Applied, thanks.


-- 
David Marchand

[PATCH] net/iavf: fix handle VF reset

[Zhichao Zeng]

This patch adds detection to avoid repeated calls to dev_reset.

And enables the iavf_dev_watchdog to detect some VF reset without PF event
(VIRTCHNL_EVENT_RESET_IMPENDING), one of which is after an NVM update.

Fixes: e74e1bb6280d ("net/iavf: enable port reset")
Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without 
interrupt")
Cc: sta...@dpdk.org

Signed-off-by: Zhichao Zeng 
---
 drivers/net/iavf/iavf.h|  2 +-
 drivers/net/iavf/iavf_ethdev.c |  9 +
 drivers/net/iavf/iavf_vchnl.c  | 17 -
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index aa18650ffa..309c1a50e4 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -31,7 +31,7 @@
 
 #define IAVF_NUM_MACADDR_MAX  64
 
-#define IAVF_DEV_WATCHDOG_PERIOD 0
+#define IAVF_DEV_WATCHDOG_PERIOD 5 /* 0 means disabled*/
 
 #define IAVF_DEFAULT_RX_PTHRESH  8
 #define IAVF_DEFAULT_RX_HTHRESH  8
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f6d68403ce..8a5ac6b2f6 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -299,6 +299,7 @@ iavf_dev_watchdog(void *cb_arg)
"VF \"%s\" reset event detected by watchdog",
adapter->vf.eth_dev->data->name);
 
+   adapter->vf.link_up = false;
/* enter reset state with VFLR event */
adapter->vf.vf_reset = true;
 
@@ -2806,7 +2807,15 @@ static int
 iavf_dev_reset(struct rte_eth_dev *dev)
 {
int ret;
+   struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+   ret = iavf_check_vf_reset_done(hw);
+   if (ret) {
+   PMD_DRV_LOG(ERR, "Wait too long for reset done!!!\n");
+   return ret;
+   }
 
+   PMD_DRV_LOG(DEBUG, "Start dev_reset ...\n");
ret = iavf_dev_uninit(dev);
if (ret)
return ret;
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 9adaadb173..bea8d476dd 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -260,8 +260,12 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, 
uint16_t buf_len,
vf->link_up ? "up" : "down");
break;
case VIRTCHNL_EVENT_RESET_IMPENDING:
-   vf->vf_reset = true;
-   PMD_DRV_LOG(INFO, "VF is resetting");
+   vf->link_up = false;
+   if (!vf->vf_reset) {
+   vf->vf_reset = true;
+   iavf_dev_event_post(vf->eth_dev, 
RTE_ETH_EVENT_INTR_RESET, NULL, 0);
+   PMD_DRV_LOG(INFO, "VF is resetting");
+   }
break;
case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
vf->dev_closed = true;
@@ -433,9 +437,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t 
*msg,
switch (pf_msg->event) {
case VIRTCHNL_EVENT_RESET_IMPENDING:
PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
-   vf->vf_reset = true;
-   iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
- NULL, 0);
+   vf->link_up = false;
+   if (!vf->vf_reset) {
+   vf->vf_reset = true;
+   iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+   NULL, 0);
+   }
break;
case VIRTCHNL_EVENT_LINK_CHANGE:
PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
-- 
2.25.1

RE: [PATCH v4] net/ice: fix ice dcf control thread crash

[Zhang, Qi Z]

> -Original Message-
> From: Ye, MingjinX 
> Sent: Friday, March 17, 2023 1:10 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org; Zhou, YidingX
> ; Ye, MingjinX ; Zhang,
> Ke1X ; Zhang, Qi Z 
> Subject: [PATCH v4] net/ice: fix ice dcf control thread crash
> 
> The control thread accesses the hardware resources after the resources were
> released, which results in a segment error.
> 
> The 'ice-reset' threads are detached, so thread resources cannot be
> reclaimed by `pthread_join` calls.
> 
> This commit synchronizes the number of 'ice-reset' threads by adding two
> variables (the 'vsi_update_thread_num' static global and the
> 'vsi_thread_lock' static global spinlock). When releasing HW resources, we
> clear the event callback function. That makes these threads exit quickly.
> After the number of 'ice-reset' threads decreased to be 0, we release
> resources.
> 
> Fixes: 3b3757bda3c3 ("net/ice: get VF hardware index in DCF")
> Fixes: 931ee54072b1 ("net/ice: support QoS bandwidth config after VF reset
> in DCF")
> Fixes: c7e1a1a3bfeb ("net/ice: refactor DCF VLAN handling")
> Fixes: 0b02c9519432 ("net/ice: handle PF initialization by DCF")
> Fixes: b71573ec2fc2 ("net/ice: retry getting VF VSI map after failure")
> Fixes: 7564d5509611 ("net/ice: add DCF hardware initialization")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ke Zhang 
> Signed-off-by: Mingjin Ye 
> ---
> v2: add pthread_exit() for windows
> ---
> v3: Optimization. It is unsafe for a thread to forcibly exit, which will cause
> the spin lock to not be released correctly
> ---
> v4: Safely wait for all event threads to end
> ---
>  drivers/net/ice/ice_dcf.c| 18 ++--
>  drivers/net/ice/ice_dcf.h|  1 +
>  drivers/net/ice/ice_dcf_parent.c | 37 
>  3 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c index
> 1c3d22ae0f..169520f5bb 100644
> --- a/drivers/net/ice/ice_dcf.c
> +++ b/drivers/net/ice/ice_dcf.c
> @@ -543,6 +543,8 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw
> *hw)
>   ice_dcf_disable_irq0(hw);
> 
>   for (;;) {
> + if (hw->vc_event_msg_cb == NULL)
> + break;
>   if (ice_dcf_get_vf_resource(hw) == 0 &&
>   ice_dcf_get_vf_vsi_map(hw) >= 0) {
>   err = 0;
> @@ -555,8 +557,10 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw
> *hw)
>   rte_delay_ms(ICE_DCF_ARQ_CHECK_TIME);
>   }
> 
> - rte_intr_enable(pci_dev->intr_handle);
> - ice_dcf_enable_irq0(hw);
> + if (hw->vc_event_msg_cb != NULL) {
> + rte_intr_enable(pci_dev->intr_handle);
> + ice_dcf_enable_irq0(hw);
> + }
> 
>   rte_spinlock_unlock(&hw->vc_cmd_send_lock);
> 
> @@ -749,6 +753,12 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev,
> struct ice_dcf_hw *hw)
>   struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
>   struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> 
> + /* Clear event callbacks, `VIRTCHNL_EVENT_DCF_VSI_MAP_UPDATE`
> +  * event will be ignored and all running `ice-thread` threads
> +  * will exit quickly.
> +  */
> + hw->vc_event_msg_cb = NULL;
> +
>   if (hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS)
>   if (hw->tm_conf.committed) {
>   ice_dcf_clear_bw(hw);
> @@ -760,6 +770,10 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev,
> struct ice_dcf_hw *hw)
>   rte_intr_callback_unregister(intr_handle,
>ice_dcf_dev_interrupt_handler, hw);
> 
> + /* Wait for all `ice-thread` threads to exit. */
> + while (ice_dcf_event_handle_num() > 0)
> + rte_delay_ms(ICE_DCF_ARQ_CHECK_TIME);
> +
>   ice_dcf_mode_disable(hw);
>   iavf_shutdown_adminq(&hw->avf);
> 
> diff --git a/drivers/net/ice/ice_dcf.h b/drivers/net/ice/ice_dcf.h index
> 7f42ebabe9..6c636a7497 100644
> --- a/drivers/net/ice/ice_dcf.h
> +++ b/drivers/net/ice/ice_dcf.h
> @@ -143,6 +143,7 @@ int ice_dcf_execute_virtchnl_cmd(struct ice_dcf_hw
> *hw,  int ice_dcf_send_aq_cmd(void *dcf_hw, struct ice_aq_desc *desc,
>   void *buf, uint16_t buf_size);
>  int ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw);
> +int ice_dcf_event_handle_num(void);
>  int ice_dcf_init_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw);
> void ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw);
> int ice_dcf_configure_rss_key(struct ice_dcf_hw *hw); diff --git
> a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
> index 01e390ddda..0ff08e179e 100644
> --- a/drivers/net/ice/ice_dcf_parent.c
> +++ b/drivers/net/ice/ice_dcf_parent.c
> @@ -14,6 +14,9 @@
> 
>  #define ICE_DCF_VSI_UPDATE_SERVICE_INTERVAL  10 /* us */
>  static rte_spinlock_t vsi_update_lock = RTE_SPINLOCK_INITIALIZER;
> +static rte_spinlock_t vsi_thread_lock = RTE_SP

Re: [PATCH v5 2/2] eal: fix failure path race setting new thread affinity

[David Marchand]

On Thu, Mar 16, 2023 at 1:07 AM Tyler Retzlaff
 wrote:
>
> In rte_thread_create setting affinity after pthread_create may fail.
> Such a failure should result in the entire rte_thread_create failing
> but doesn't.
>
> Additionally if there is a failure to set affinity a race exists where
> the creating thread will free ctx and depending on scheduling of the new
> thread it may also free ctx (double free).
>
> Resolve the above by setting the affinity from the newly created thread
> using a condition variable to signal the completion of the thread
> start wrapper having completed.
>
> Since we are now waiting for the thread start wrapper to complete we can
> allocate the thread start wrapper context on the stack. While here clean
> up the variable naming in the context to better highlight the fields of
> the context require synchronization between the creating and created
> thread.
>
> Fixes: ce6e911d20f6 ("eal: add thread lifetime API")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Tyler Retzlaff 
> ---
>  lib/eal/unix/rte_thread.c | 70 
> +--
>  1 file changed, 43 insertions(+), 27 deletions(-)
>
> diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> index 37ebfcf..5992b04 100644
> --- a/lib/eal/unix/rte_thread.c
> +++ b/lib/eal/unix/rte_thread.c
> @@ -16,9 +16,14 @@ struct eal_tls_key {
> pthread_key_t thread_index;
>  };
>
> -struct thread_routine_ctx {
> +struct thread_start_context {
> rte_thread_func thread_func;
> -   void *routine_args;
> +   void *thread_args;
> +   const rte_thread_attr_t *thread_attr;
> +   pthread_mutex_t wrapper_mutex;
> +   pthread_cond_t wrapper_cond;
> +   int wrapper_ret;
> +   volatile int wrapper_done;

One question.

I see that wrapper_done is accessed under wrapper_mutex.
Is volatile needed?

(nit: a boolean is probably enough too)

I was thinking of squashing below diff:

diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 5992b04a45..5ab5267ca3 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -23,7 +23,7 @@ struct thread_start_context {
pthread_mutex_t wrapper_mutex;
pthread_cond_t wrapper_cond;
int wrapper_ret;
-   volatile int wrapper_done;
+   bool wrapper_done;
 };

 static int
@@ -101,7 +101,7 @@ thread_start_wrapper(void *arg)

pthread_mutex_lock(&ctx->wrapper_mutex);
ctx->wrapper_ret = ret;
-   ctx->wrapper_done = 1;
+   ctx->wrapper_done = true;
pthread_cond_signal(&ctx->wrapper_cond);
pthread_mutex_unlock(&ctx->wrapper_mutex);

@@ -127,6 +127,7 @@ rte_thread_create(rte_thread_t *thread_id,
.thread_func = thread_func,
.thread_args = args,
.thread_attr = thread_attr,
+   .wrapper_done = false,
.wrapper_mutex = PTHREAD_MUTEX_INITIALIZER,
.wrapper_cond = PTHREAD_COND_INITIALIZER,
};
@@ -151,7 +152,6 @@ rte_thread_create(rte_thread_t *thread_id,
goto cleanup;
}

-
if (thread_attr->priority ==
RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
ret = ENOTSUP;
@@ -183,7 +183,7 @@ rte_thread_create(rte_thread_t *thread_id,
}

pthread_mutex_lock(&ctx.wrapper_mutex);
-   while (ctx.wrapper_done != 1)
+   while (!ctx.wrapper_done)
pthread_cond_wait(&ctx.wrapper_cond, &ctx.wrapper_mutex);
ret = ctx.wrapper_ret;
pthread_mutex_unlock(&ctx.wrapper_mutex);


The rest lgtmn thanks Tyler.



-- 
David Marchand

Re: [PATCH 2/2] app/testpmd: add testpmd based sleeping

[Anthony Harivel]

Ferruh Yigit, Mar 16, 2023 at 18:05:
> Hi Anthony,
>
> What is the motivation here?

Hi Ferruh,

AFAIK testpmd is the reference tool used for CI and tests whether it is
for functional or performance tests and I think it would be in
everyone's interest to consume less CPU during them. Moreover, all
patches coming to the ML are going through validation tests and this
could reduce the maintenance cost of the project.

AES-GCM crypto design review

[Suanming Mou]

BEGIN:VCALENDAR
METHOD:REQUEST
PRODID:Microsoft Exchange Server 2010
VERSION:2.0
BEGIN:VTIMEZONE
TZID:China Standard Time
BEGIN:STANDARD
DTSTART:16010101T00
TZOFFSETFROM:+0800
TZOFFSETTO:+0800
END:STANDARD
BEGIN:DAYLIGHT
DTSTART:16010101T00
TZOFFSETFROM:+0800
TZOFFSETTO:+0800
END:DAYLIGHT
END:VTIMEZONE
BEGIN:VEVENT
ORGANIZER;CN=Suanming Mou:mailto:suanmi...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Matan Azra
 d:mailto:ma...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Andrey Ves
 novaty:mailto:andr...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Slava Ovsi
 ienko:mailto:viachesl...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Ori Kam:ma
 ilto:or...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Salaheddin
  Badawi:mailto:sbad...@nvidia.com
ATTENDEE;ROLE=OPT-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Raslan Dar
 awsheh:mailto:rasl...@nvidia.com
ATTENDEE;ROLE=OPT-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Asaf Penso
 :mailto:as...@nvidia.com
ATTENDEE;ROLE=OPT-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=dev@dpdk.o
 rg:mailto:dev@dpdk.org
DESCRIPTION;LANGUAGE=en-US:Placeholder\n___
 _\nMicrosoft Teams meeting\nJo
 in on your computer\, mobile app or room device\nClick here to join the me
 eting\nMeeting ID: 246 651 372 142\nPasscode: fMKop
 X\nDownload Teams | Join on the web\nJoin with a video conferencing device\nte...@vc.nvidia.com\nVideo Conference ID: 117 530 971 3\nAlternate VTC i
 nstructions\nOr call in (
 audio only)\n+86 400 919 8546\,\,799024045#   China\, All locations\nPhone Conference ID: 799 024 045#\nFind a loca
 l number | Reset PIN\nLearn More | Meeting opt
 ions\n
 \n\n
UID:04008200E00074C5B7101A82E0086090DE840459D901000
 01000B9F66064B0866B40A098FE2021389D66
SUMMARY;LANGUAGE=en-US:AES-GCM crypto design review
DTSTART;TZID=China Standard Time:20230321T163000
DTEND;TZID=China Standard Time:20230321T173000
CLASS:PUBLIC
PRIORITY:5
DTSTAMP:20230317T111955Z
TRANSP:OPAQUE
STATUS:CONFIRMED
SEQUENCE:0
LOCATION;LANGUAGE=en-US:Microsoft Teams Meeting
X-MICROSOFT-CDO-APPT-SEQUENCE:0
X-MICROSOFT-CDO-OWNERAPPTID:1559312359
X-MICROSOFT-CDO-BUSYSTATUS:TENTATIVE
X-MICROSOFT-CDO-INTENDEDSTATUS:FREE
X-MICROSOFT-CDO-ALLDAYEVENT:FALSE
X-MICROSOFT-CDO-IMPORTANCE:1
X-MICROSOFT-CDO-INSTTYPE:0
X-MICROSOFT-SKYPETEAMSMEETINGURL:https://teams.microsoft.com/l/meetup-join/
 19%3ameeting_ZjE0YTZmNDUtYWQ4Mi00MzEyLWJjZWUtZmIyNDU0NTRmMzk0%40thread.v2/
 0?context=%7b%22Tid%22%3a%2243083d15-7273-40c1-b7db-39efd9ccc17a%22%2c%22O
 id%22%3a%228d96c045-dd88-4303-b9d9-40c1bb74d5d3%22%7d
X-MICROSOFT-SCHEDULINGSERVICEUPDATEURL:https://api.scheduler.teams.microsof
 t.com/teams/43083d15-7273-40c1-b7db-39efd9ccc17a/8d96c045-dd88-4303-b9d9-4
 0c1bb74d5d3/19_meeting_ZjE0YTZmNDUtYWQ4Mi00MzEyLWJjZWUtZmIyNDU0NTRmMzk0@th
 read.v2/0
X-MICROSOFT-SKYPETEAMSPROPERTIES:{"cid":"19:meeting_ZjE0YTZmNDUtYWQ4Mi00MzE
 yLWJjZWUtZmIyNDU0NTRmMzk0@thread.v2"\,"private":true\,"type":0\,"mid":0\,"
 rid":0\,"uid":null}
X-MICROSOFT-ONLINEMEETINGCONFLINK:conf:sip:suanmi...@nvidia.com\;gruu\;opaq
 ue=app:conf:focus:id:teams:2:0!19:meeting_ZjE0YTZmNDUtYWQ4Mi00MzEyLWJjZWUt
 ZmIyNDU0NTRmMzk0-thread.v2!8d96c045dd884303b9d940c1bb74d5d3!43083d15727340
 c1b7db39efd9ccc17a
X-MICROSOFT-DONOTFORWARDMEETING:FALSE
X-MICROSOFT-DISALLOW-COUNTER:FALSE
X-MICROSOFT-LOCATIONS:[ { "DisplayName" : "Microsoft Teams Meeting"\, "Loca
 tionAnnotation" : ""\, "LocationSource" : 0\, "Unresolved" : false\, "Loca
 tionUri" : "" } ]
BEGIN:VALARM
DESCRIPTION:REMINDER
TRIGGER;RELATED=START:-PT18H
ACTION:DISPLAY
END:VALARM
END:VEVENT
END:VCALENDAR

Canceled: AES-GCM crypto design review

[Suanming Mou]

BEGIN:VCALENDAR
METHOD:CANCEL
PRODID:Microsoft Exchange Server 2010
VERSION:2.0
BEGIN:VTIMEZONE
TZID:China Standard Time
BEGIN:STANDARD
DTSTART:16010101T00
TZOFFSETFROM:+0800
TZOFFSETTO:+0800
END:STANDARD
BEGIN:DAYLIGHT
DTSTART:16010101T00
TZOFFSETFROM:+0800
TZOFFSETTO:+0800
END:DAYLIGHT
END:VTIMEZONE
BEGIN:VEVENT
ORGANIZER;CN=Suanming Mou:mailto:suanmi...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Matan Azra
 d:mailto:ma...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Andrey Ves
 novaty:mailto:andr...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Slava Ovsi
 ienko:mailto:viachesl...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Ori Kam:ma
 ilto:or...@nvidia.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Salaheddin
  Badawi:mailto:sbad...@nvidia.com
ATTENDEE;ROLE=OPT-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Raslan Dar
 awsheh:mailto:rasl...@nvidia.com
ATTENDEE;ROLE=OPT-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Asaf Penso
 :mailto:as...@nvidia.com
ATTENDEE;ROLE=OPT-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=dev@dpdk.o
 rg:mailto:dev@dpdk.org
DESCRIPTION;LANGUAGE=en-US:Placeholder\n___
 _\nMicrosoft Teams meeting\nJo
 in on your computer\, mobile app or room device\nClick here to join the me
 eting\nMeeting ID: 246 651 372 142\nPasscode: fMKop
 X\nDownload Teams | Join on the web\nJoin with a video conferencing device\nte...@vc.nvidia.com\nVideo Conference ID: 117 530 971 3\nAlternate VTC i
 nstructions\nOr call in (
 audio only)\n+86 400 919 8546\,\,799024045#   China\, All locations\nPhone Conference ID: 799 024 045#\nFind a loca
 l number | Reset PIN\nLearn More | Meeting opt
 ions\n
 \n\n
UID:04008200E00074C5B7101A82E0086090DE840459D901000
 01000B9F66064B0866B40A098FE2021389D66
SUMMARY;LANGUAGE=en-US:Canceled: AES-GCM crypto design review
DTSTART;TZID=China Standard Time:20230321T163000
DTEND;TZID=China Standard Time:20230321T173000
CLASS:PUBLIC
PRIORITY:1
DTSTAMP:20230317T112210Z
TRANSP:TRANSPARENT
STATUS:CANCELLED
SEQUENCE:1
LOCATION;LANGUAGE=en-US:Microsoft Teams Meeting
X-MICROSOFT-CDO-APPT-SEQUENCE:1
X-MICROSOFT-CDO-OWNERAPPTID:1559312359
X-MICROSOFT-CDO-BUSYSTATUS:FREE
X-MICROSOFT-CDO-INTENDEDSTATUS:FREE
X-MICROSOFT-CDO-ALLDAYEVENT:FALSE
X-MICROSOFT-CDO-IMPORTANCE:2
X-MICROSOFT-CDO-INSTTYPE:0
X-MICROSOFT-ONLINEMEETINGINFORMATION:{"OnlineMeetingChannelId":null\,"Onlin
 eMeetingProvider":3}
X-MICROSOFT-SKYPETEAMSMEETINGURL:https://teams.microsoft.com/l/meetup-join/
 19%3ameeting_ZjE0YTZmNDUtYWQ4Mi00MzEyLWJjZWUtZmIyNDU0NTRmMzk0%40thread.v2/
 0?context=%7b%22Tid%22%3a%2243083d15-7273-40c1-b7db-39efd9ccc17a%22%2c%22O
 id%22%3a%228d96c045-dd88-4303-b9d9-40c1bb74d5d3%22%7d
X-MICROSOFT-SCHEDULINGSERVICEUPDATEURL:https://api.scheduler.teams.microsof
 t.com/teams/43083d15-7273-40c1-b7db-39efd9ccc17a/8d96c045-dd88-4303-b9d9-4
 0c1bb74d5d3/19_meeting_ZjE0YTZmNDUtYWQ4Mi00MzEyLWJjZWUtZmIyNDU0NTRmMzk0@th
 read.v2/0
X-MICROSOFT-SKYPETEAMSPROPERTIES:{"cid":"19:meeting_ZjE0YTZmNDUtYWQ4Mi00MzE
 yLWJjZWUtZmIyNDU0NTRmMzk0@thread.v2"\,"private":true\,"type":0\,"mid":0\,"
 rid":0\,"uid":null}
X-MICROSOFT-ONLINEMEETINGCONFLINK:conf:sip:suanmi...@nvidia.com\;gruu\;opaq
 ue=app:conf:focus:id:teams:2:0!19:meeting_ZjE0YTZmNDUtYWQ4Mi00MzEyLWJjZWUt
 ZmIyNDU0NTRmMzk0-thread.v2!8d96c045dd884303b9d940c1bb74d5d3!43083d15727340
 c1b7db39efd9ccc17a
X-MICROSOFT-DONOTFORWARDMEETING:FALSE
X-MICROSOFT-DISALLOW-COUNTER:FALSE
END:VEVENT
END:VCALENDAR

[dpdk-dev] [PATCH] doc: deprecation notice to remove net/bnx2x driver

[jerinj]

From: Jerin Jacob 

Starting from DPDK 23.07, the Marvell QLogic bnx2x driver
will be removed. This decision has been made to alleviate the burden of
maintaining a discontinued product.

Signed-off-by: Jerin Jacob 
---
 doc/guides/rel_notes/deprecation.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 872847e938..d3d8d0011c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -135,3 +135,6 @@ Deprecation Notices
   Its removal has been postponed to let potential users report interest
   in maintaining it.
   In the absence of such interest, this library will be removed in DPDK 23.11.
+
+* net/bnx2x: Starting from DPDK 23.07, the Marvell QLogic bnx2x driver will be 
removed.
+  This decision has been made to alleviate the burden of maintaining a 
discontinued product.
-- 
2.40.0

Re: [PATCH v5 2/2] eal: fix failure path race setting new thread affinity

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 11:45:08AM +0100, David Marchand wrote:
> On Thu, Mar 16, 2023 at 1:07 AM Tyler Retzlaff
>  wrote:
> >
> > In rte_thread_create setting affinity after pthread_create may fail.
> > Such a failure should result in the entire rte_thread_create failing
> > but doesn't.
> >
> > Additionally if there is a failure to set affinity a race exists where
> > the creating thread will free ctx and depending on scheduling of the new
> > thread it may also free ctx (double free).
> >
> > Resolve the above by setting the affinity from the newly created thread
> > using a condition variable to signal the completion of the thread
> > start wrapper having completed.
> >
> > Since we are now waiting for the thread start wrapper to complete we can
> > allocate the thread start wrapper context on the stack. While here clean
> > up the variable naming in the context to better highlight the fields of
> > the context require synchronization between the creating and created
> > thread.
> >
> > Fixes: ce6e911d20f6 ("eal: add thread lifetime API")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tyler Retzlaff 
> > ---
> >  lib/eal/unix/rte_thread.c | 70 
> > +--
> >  1 file changed, 43 insertions(+), 27 deletions(-)
> >
> > diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> > index 37ebfcf..5992b04 100644
> > --- a/lib/eal/unix/rte_thread.c
> > +++ b/lib/eal/unix/rte_thread.c
> > @@ -16,9 +16,14 @@ struct eal_tls_key {
> > pthread_key_t thread_index;
> >  };
> >
> > -struct thread_routine_ctx {
> > +struct thread_start_context {
> > rte_thread_func thread_func;
> > -   void *routine_args;
> > +   void *thread_args;
> > +   const rte_thread_attr_t *thread_attr;
> > +   pthread_mutex_t wrapper_mutex;
> > +   pthread_cond_t wrapper_cond;
> > +   int wrapper_ret;
> > +   volatile int wrapper_done;
> 
> One question.
> 
> I see that wrapper_done is accessed under wrapper_mutex.
> Is volatile needed?

I'm not entirely certain. i'm being cautious since i can conceive of the
load in the loop being optimized as a single load by the compiler. but
again i'm not sure, i always like to learn if someone knows better.

> 
> (nit: a boolean is probably enough too)

I have no issue with it being a _Bool if you want to adjust it for that
i certainly don't object. ordinarily i would use _Bool but a lot of dpdk
code seems to prefer int so that's why i chose it. if we use the macro
bool then we should include stdbool.h directly into this translation
unit.

> 
> I was thinking of squashing below diff:

Yeah, no objection. you can decide if you want to keep the volatile or
not and add the stdbool.h include.

Thanks for reviewing, appreciate it.

> 
> diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> index 5992b04a45..5ab5267ca3 100644
> --- a/lib/eal/unix/rte_thread.c
> +++ b/lib/eal/unix/rte_thread.c
> @@ -23,7 +23,7 @@ struct thread_start_context {
> pthread_mutex_t wrapper_mutex;
> pthread_cond_t wrapper_cond;
> int wrapper_ret;
> -   volatile int wrapper_done;
> +   bool wrapper_done;
>  };
> 
>  static int
> @@ -101,7 +101,7 @@ thread_start_wrapper(void *arg)
> 
> pthread_mutex_lock(&ctx->wrapper_mutex);
> ctx->wrapper_ret = ret;
> -   ctx->wrapper_done = 1;
> +   ctx->wrapper_done = true;
> pthread_cond_signal(&ctx->wrapper_cond);
> pthread_mutex_unlock(&ctx->wrapper_mutex);
> 
> @@ -127,6 +127,7 @@ rte_thread_create(rte_thread_t *thread_id,
> .thread_func = thread_func,
> .thread_args = args,
> .thread_attr = thread_attr,
> +   .wrapper_done = false,
> .wrapper_mutex = PTHREAD_MUTEX_INITIALIZER,
> .wrapper_cond = PTHREAD_COND_INITIALIZER,
> };
> @@ -151,7 +152,6 @@ rte_thread_create(rte_thread_t *thread_id,
> goto cleanup;
> }
> 
> -
> if (thread_attr->priority ==
> RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> ret = ENOTSUP;
> @@ -183,7 +183,7 @@ rte_thread_create(rte_thread_t *thread_id,
> }
> 
> pthread_mutex_lock(&ctx.wrapper_mutex);
> -   while (ctx.wrapper_done != 1)
> +   while (!ctx.wrapper_done)
> pthread_cond_wait(&ctx.wrapper_cond, &ctx.wrapper_mutex);
> ret = ctx.wrapper_ret;
> pthread_mutex_unlock(&ctx.wrapper_mutex);
> 
> 
> The rest lgtmn thanks Tyler.
> 
> 
> 
> -- 
> David Marchand

[PATCH] bus/auxiliary: support cleanup callback

[Xueming Li]

The bus cleanup callback is used to sunset all devices on bus
gracefully. This patch supports the callback by unplug all
devices on auxiliary bus.

Signed-off-by: Xueming Li 
---
 drivers/bus/auxiliary/auxiliary_common.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/bus/auxiliary/auxiliary_common.c 
b/drivers/bus/auxiliary/auxiliary_common.c
index ff1369353a..19bf523660 100644
--- a/drivers/bus/auxiliary/auxiliary_common.c
+++ b/drivers/bus/auxiliary/auxiliary_common.c
@@ -236,6 +236,7 @@ auxiliary_probe(void)
return (probed && probed == failed) ? -1 : 0;
 }
 
+
 static int
 auxiliary_parse(const char *name, void *addr)
 {
@@ -337,6 +338,26 @@ auxiliary_unplug(struct rte_device *dev)
return ret;
 }
 
+static int
+auxiliary_cleanup(void)
+{
+   struct rte_auxiliary_device *dev, *tmp_dev;
+   int error = 0;
+
+   RTE_TAILQ_FOREACH_SAFE(dev, &auxiliary_bus.device_list, next, tmp_dev) {
+   struct rte_auxiliary_driver *drv = dev->driver;
+   int ret = 0;
+
+   ret = auxiliary_unplug(&dev->device);
+   if (ret < 0) {
+   rte_errno = errno;
+   error = -1;
+   }
+   }
+
+   return error;
+}
+
 static int
 auxiliary_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t 
len)
 {
@@ -406,6 +427,7 @@ struct rte_auxiliary_bus auxiliary_bus = {
.bus = {
.scan = auxiliary_scan,
.probe = auxiliary_probe,
+   .cleanup = auxiliary_cleanup,
.find_device = auxiliary_find_device,
.plug = auxiliary_plug,
.unplug = auxiliary_unplug,
-- 
2.25.1

[PATCH v2] bus/auxiliary: support cleanup callback

[Xueming Li]

The bus cleanup callback is used to sunset all devices on bus
gracefully. This patch supports the callback by unplug all
devices on auxiliary bus.

Signed-off-by: Xueming Li 
---
 drivers/bus/auxiliary/auxiliary_common.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/bus/auxiliary/auxiliary_common.c 
b/drivers/bus/auxiliary/auxiliary_common.c
index ff1369353a..a66cc85d86 100644
--- a/drivers/bus/auxiliary/auxiliary_common.c
+++ b/drivers/bus/auxiliary/auxiliary_common.c
@@ -236,6 +236,7 @@ auxiliary_probe(void)
return (probed && probed == failed) ? -1 : 0;
 }
 
+
 static int
 auxiliary_parse(const char *name, void *addr)
 {
@@ -337,6 +338,25 @@ auxiliary_unplug(struct rte_device *dev)
return ret;
 }
 
+static int
+auxiliary_cleanup(void)
+{
+   struct rte_auxiliary_device *dev, *tmp_dev;
+   int error = 0;
+
+   RTE_TAILQ_FOREACH_SAFE(dev, &auxiliary_bus.device_list, next, tmp_dev) {
+   int ret;
+
+   ret = auxiliary_unplug(&dev->device);
+   if (ret < 0) {
+   rte_errno = errno;
+   error = -1;
+   }
+   }
+
+   return error;
+}
+
 static int
 auxiliary_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t 
len)
 {
@@ -406,6 +426,7 @@ struct rte_auxiliary_bus auxiliary_bus = {
.bus = {
.scan = auxiliary_scan,
.probe = auxiliary_probe,
+   .cleanup = auxiliary_cleanup,
.find_device = auxiliary_find_device,
.plug = auxiliary_plug,
.unplug = auxiliary_unplug,
-- 
2.25.1

Re: [PATCH v4 1/2] testpmd: go back to using cmdline_interact

[Olivier Matz]

Hi Stephen,

Thank you for having a look at this.

On Wed, Mar 15, 2023 at 10:31:31AM -0700, Stephen Hemminger wrote:
> The cmdline library poll function is broken on Windows
> and was never tested, don't use it.
> 
> Instead, use sigaction() to cancel read character on Unix OS's
> and a new helper to cancel I/O on Windows.
> 
> Fixes: 0fd1386c30c3 ("app/testpmd: cleanup cleanly from signal")
> Bugzilla ID: 1180
> Signed-off-by: Stephen Hemminger 
> ---
>  app/test-pmd/cmdline.c   | 27 ++-
>  app/test-pmd/testpmd.c   | 11 +++
>  lib/cmdline/cmdline.h| 10 ++
>  lib/cmdline/cmdline_os_unix.c|  5 +
>  lib/cmdline/cmdline_os_windows.c | 14 ++
>  lib/cmdline/cmdline_private.h|  2 +-
>  lib/cmdline/version.map  |  3 +++
>  7 files changed, 58 insertions(+), 14 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 6fa870dc329b..072437d9bfcf 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -66,6 +66,7 @@
>  #include "cmdline_tm.h"
>  #include "bpf_cmd.h"
>  
> +static struct cmdline *testpmd_cl;
>  static cmdline_parse_ctx_t *main_ctx;
>  static TAILQ_HEAD(, testpmd_driver_commands) driver_commands_head =
>   TAILQ_HEAD_INITIALIZER(driver_commands_head);
> @@ -13028,26 +13029,26 @@ cmdline_read_from_file(const char *filename)
>   printf("Read CLI commands from %s\n", filename);
>  }
>  
> +void
> +prompt_exit(void)
> +{
> + cmdline_cancel(testpmd_cl);
> + cmdline_quit(testpmd_cl);
> +}
> +
>  /* prompt function, called from main on MAIN lcore */
>  void
>  prompt(void)
>  {
> - struct cmdline *cl;
> -
> - cl = cmdline_stdin_new(main_ctx, "testpmd> ");
> - if (cl == NULL)
> + testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
> + if (testpmd_cl == NULL) {
> + fprintf(stderr,
> + "Failed to create stdin based cmdline context\n");
>   return;
> -
> - /* loop until signal or quit command */
> - while (f_quit == 0 && cl_quit == 0) {
> - int status = cmdline_poll(cl);
> -
> - if (status < 0 || status == RDLINE_EXITED)
> - break;
>   }
>  
> - cmdline_quit(cl);
> - cmdline_stdin_exit(cl);
> + cmdline_interact(testpmd_cl);
> + cmdline_stdin_exit(testpmd_cl);
>  }
>  
>  void
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 2ce19ed47ab4..5cb6f9252395 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -4469,6 +4469,7 @@ static void
>  signal_handler(int signum __rte_unused)
>  {
>   f_quit = 1;
> + prompt_exit();
>  }
>  
>  int
> @@ -4479,8 +4480,18 @@ main(int argc, char** argv)
>   uint16_t count;
>   int ret;
>  
> +#ifdef RTE_EXEC_ENV_WINDOWS
>   signal(SIGINT, signal_handler);
>   signal(SIGTERM, signal_handler);
> +#else
> + /* Want read() not to be restarted on signal */
> + struct sigaction action = {
> + .sa_handler = signal_handler,
> + };
> +
> + sigaction(SIGINT, &action, NULL);
> + sigaction(SIGTERM, &action, NULL);
> +#endif

If we have to change this in testpmd, we'll have to do the same in other
applications.

See my comment in patch 2.

>  
>   testpmd_logtype = rte_log_register("testpmd");
>   if (testpmd_logtype < 0)
> diff --git a/lib/cmdline/cmdline.h b/lib/cmdline/cmdline.h
> index b14355ef5121..2a1721cf9712 100644
> --- a/lib/cmdline/cmdline.h
> +++ b/lib/cmdline/cmdline.h
> @@ -60,6 +60,16 @@ int cmdline_poll(struct cmdline *cl);
>  void cmdline_interact(struct cmdline *cl);
>  void cmdline_quit(struct cmdline *cl);
>  
> +
> +/**
> + * This function causes the read() in cmdline_interact to exit.
> + *
> + * @param cl
> + *   The command line object.
> + */
> +__rte_experimental
> +void cmdline_cancel(struct cmdline *cl);
> +

The help says this function causes the read() in cmdline_interact to
exit, but the unix implementation is empty.

Maybe we should instead explain in what condition this function must
called. Also, shouldn't this function call cmdline_quit() too to avoid
the user to do it?

>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/cmdline/cmdline_os_unix.c b/lib/cmdline/cmdline_os_unix.c
> index 64a945a34fb3..5f9839a15f98 100644
> --- a/lib/cmdline/cmdline_os_unix.c
> +++ b/lib/cmdline/cmdline_os_unix.c
> @@ -51,3 +51,8 @@ cmdline_vdprintf(int fd, const char *format, va_list op)
>  {
>   return vdprintf(fd, format, op);
>  }
> +
> +void
> +cmdline_cancel(__rte_unused struct cmdline *cl)
> +{
> +}
> diff --git a/lib/cmdline/cmdline_os_windows.c 
> b/lib/cmdline/cmdline_os_windows.c
> index 73ed9ba290b8..80863bfc8a00 100644
> --- a/lib/cmdline/cmdline_os_windows.c
> +++ b/lib/cmdline/cmdline_os_windows.c
> @@ -203,3 +203,17 @@ cmdline_vdprintf(int fd, const char *format, va_list op)
>  
>   return ret;
>  }
> +
> +void
> +cmdline_cancel(struct cmdline *cl)
> 

Re: [PATCH v4 2/2] testpmd: enable interrupt in interactive mode

[Olivier Matz]

On Wed, Mar 15, 2023 at 10:31:32AM -0700, Stephen Hemminger wrote:
> The setting in terminal handling for both Unix style and
> Windows was not ensuring that Ctrl-C character would
> cause interrupt.
> 
> This is a first release bug. Testpmd interactive mode has
> always disabled control-c handling on Linux.

This was a design choice, not a bug. This design choice is discussable
today (at that time, dpdk was also running in baremetal without signals
or interrupt). The idea was to behave like a shell, i.e. ctrl-c just
clears the current line.

We may want to change this behavior (I remember an old discussion where
Bruce stated that he would prefer ctrl-c to kill the program), but it
will have an impact on all cmdline users, so to me it has to be
announced.


> 
> Fixes: af75078fece3 ("first public release")
> Signed-off-by: Stephen Hemminger 
> ---
>  lib/cmdline/cmdline_os_unix.c| 3 ++-
>  lib/cmdline/cmdline_os_windows.c | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/cmdline/cmdline_os_unix.c b/lib/cmdline/cmdline_os_unix.c
> index 5f9839a15f98..356c5b3f3ca2 100644
> --- a/lib/cmdline/cmdline_os_unix.c
> +++ b/lib/cmdline/cmdline_os_unix.c
> @@ -16,7 +16,8 @@ terminal_adjust(struct cmdline *cl)
>   tcgetattr(0, &cl->oldterm);
>  
>   memcpy(&term, &cl->oldterm, sizeof(term));
> - term.c_lflag &= ~(ICANON | ECHO | ISIG);
> + term.c_lflag &= ~(ICANON | ECHO);
> + term.c_lflag |= ISIG;
>   tcsetattr(0, TCSANOW, &term);
>  
>   setbuf(stdin, NULL);
> diff --git a/lib/cmdline/cmdline_os_windows.c 
> b/lib/cmdline/cmdline_os_windows.c
> index 80863bfc8a00..8cff3b175747 100644
> --- a/lib/cmdline/cmdline_os_windows.c
> +++ b/lib/cmdline/cmdline_os_windows.c
> @@ -32,10 +32,10 @@ terminal_adjust(struct cmdline *cl)
>   mode &= ~(
>   ENABLE_LINE_INPUT |  /* no line buffering */
>   ENABLE_ECHO_INPUT |  /* no echo */
> - ENABLE_PROCESSED_INPUT | /* pass Ctrl+C to program */
>   ENABLE_MOUSE_INPUT | /* no mouse events */
>   ENABLE_WINDOW_INPUT);/* no window resize events */
> - mode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
> + mode |= ENABLE_VIRTUAL_TERMINAL_INPUT |
> + ENABLE_PROCESSED_INPUT; /* Ctrl C processed by the 
> system */
>   SetConsoleMode(handle, mode);
>   }
>  
> -- 
> 2.39.2
> 

Re: [PATCH 2/2] app/testpmd: add testpmd based sleeping

[Stephen Hemminger]

On Fri, 17 Mar 2023 12:09:04 +0100
"Anthony Harivel"  wrote:

> Ferruh Yigit, Mar 16, 2023 at 18:05:
> > Hi Anthony,
> >
> > What is the motivation here?  
> 
> Hi Ferruh,
> 
> AFAIK testpmd is the reference tool used for CI and tests whether it is
> for functional or performance tests and I think it would be in
> everyone's interest to consume less CPU during them. Moreover, all
> patches coming to the ML are going through validation tests and this
> could reduce the maintenance cost of the project.

But it introduces another variable, and the performance would vary based on
sleeping and HW interaction. I think testpmd should just run with 100% CPU,
and leave the heuristics stuff to some of the example applications.

Re: [PATCH v4 2/2] testpmd: enable interrupt in interactive mode

[Stephen Hemminger]

On Fri, 17 Mar 2023 17:20:59 +0100
Olivier Matz  wrote:

> On Wed, Mar 15, 2023 at 10:31:32AM -0700, Stephen Hemminger wrote:
> > The setting in terminal handling for both Unix style and
> > Windows was not ensuring that Ctrl-C character would
> > cause interrupt.
> > 
> > This is a first release bug. Testpmd interactive mode has
> > always disabled control-c handling on Linux.  
> 
> This was a design choice, not a bug. This design choice is discussable
> today (at that time, dpdk was also running in baremetal without signals
> or interrupt). The idea was to behave like a shell, i.e. ctrl-c just
> clears the current line.
> 
> We may want to change this behavior (I remember an old discussion where
> Bruce stated that he would prefer ctrl-c to kill the program), but it
> will have an impact on all cmdline users, so to me it has to be
> announced.


Ok, my motivation was to be able to test interrupt in testpmd
interactive mode. Without this change, it requires sending SIGINT
from another process.  Plus almost all programs that have an
interactive mode accept control-c to interrupt.

I split this patch off since it doesn't impact the bugfix
around testpmd and interrupts.

Re: [PATCH v4 1/2] testpmd: go back to using cmdline_interact

[Stephen Hemminger]

On Fri, 17 Mar 2023 17:20:48 +0100
Olivier Matz  wrote:

> >  
> > +#ifdef RTE_EXEC_ENV_WINDOWS
> > signal(SIGINT, signal_handler);
> > signal(SIGTERM, signal_handler);
> > +#else
> > +   /* Want read() not to be restarted on signal */
> > +   struct sigaction action = {
> > +   .sa_handler = signal_handler,
> > +   };
> > +
> > +   sigaction(SIGINT, &action, NULL);
> > +   sigaction(SIGTERM, &action, NULL);
> > +#endif  
> 
> If we have to change this in testpmd, we'll have to do the same in other
> applications.

The only a couple other program combining signal() and cmdline_interact().
These programs all exit from signal handler and never return from it.

In examples/ntb, the program is using the signal suicide (lets kill myself)
model, which will work.  Not sure why it bothers just to print a message.

In examples/vdpa, the program is trapping signal and calling close on ports.
This is not signal safe, but that is up to the vdpa maintainers to address.

Also, examples/vm_power_management is calling channel_XXX_exit() routines
which is not signal safe. Ditto, need the maintainers to address that.

> > +__rte_experimental
> > +void cmdline_cancel(struct cmdline *cl);
> > +  
> 
> The help says this function causes the read() in cmdline_interact to
> exit, but the unix implementation is empty.
> 
> Maybe we should instead explain in what condition this function must
> called. Also, shouldn't this function call cmdline_quit() too to avoid
> the user to do it?

Prefer to have function not to do implicit quit.

PS: cmdline functions need better documentation.

[PATCH v5] testpmd: go back to using cmdline_interact

[Stephen Hemminger]

The cmdline_poll() function is broken and was
not fully tested, don't use it.

Instead, use sigaction() to cancel read character on Unix OS's
and a new helper to cancel I/O on Windows.

Fixes: 0fd1386c30c3 ("app/testpmd: cleanup cleanly from signal")
Bugzilla ID: 1180
Signed-off-by: Stephen Hemminger 
---
v5 - Do not need to expose cmdline_cancel() as user api
 can make cmdline_quit() do it.

 Change to control C handling is separate issue and
 should be discussed more.

 app/test-pmd/cmdline.c   | 26 +-
 app/test-pmd/testpmd.c   | 11 +++
 lib/cmdline/cmdline.c|  1 +
 lib/cmdline/cmdline_os_unix.c|  6 ++
 lib/cmdline/cmdline_os_windows.c | 14 ++
 lib/cmdline/cmdline_private.h|  5 -
 6 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 6fa870dc329b..7b20bef4e9ba 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -66,6 +66,7 @@
 #include "cmdline_tm.h"
 #include "bpf_cmd.h"
 
+static struct cmdline *testpmd_cl;
 static cmdline_parse_ctx_t *main_ctx;
 static TAILQ_HEAD(, testpmd_driver_commands) driver_commands_head =
TAILQ_HEAD_INITIALIZER(driver_commands_head);
@@ -13028,26 +13029,25 @@ cmdline_read_from_file(const char *filename)
printf("Read CLI commands from %s\n", filename);
 }
 
+void
+prompt_exit(void)
+{
+   cmdline_quit(testpmd_cl);
+}
+
 /* prompt function, called from main on MAIN lcore */
 void
 prompt(void)
 {
-   struct cmdline *cl;
-
-   cl = cmdline_stdin_new(main_ctx, "testpmd> ");
-   if (cl == NULL)
+   testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
+   if (testpmd_cl == NULL) {
+   fprintf(stderr,
+   "Failed to create stdin based cmdline context\n");
return;
-
-   /* loop until signal or quit command */
-   while (f_quit == 0 && cl_quit == 0) {
-   int status = cmdline_poll(cl);
-
-   if (status < 0 || status == RDLINE_EXITED)
-   break;
}
 
-   cmdline_quit(cl);
-   cmdline_stdin_exit(cl);
+   cmdline_interact(testpmd_cl);
+   cmdline_stdin_exit(testpmd_cl);
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 2ce19ed47ab4..5cb6f9252395 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4469,6 +4469,7 @@ static void
 signal_handler(int signum __rte_unused)
 {
f_quit = 1;
+   prompt_exit();
 }
 
 int
@@ -4479,8 +4480,18 @@ main(int argc, char** argv)
uint16_t count;
int ret;
 
+#ifdef RTE_EXEC_ENV_WINDOWS
signal(SIGINT, signal_handler);
signal(SIGTERM, signal_handler);
+#else
+   /* Want read() not to be restarted on signal */
+   struct sigaction action = {
+   .sa_handler = signal_handler,
+   };
+
+   sigaction(SIGINT, &action, NULL);
+   sigaction(SIGTERM, &action, NULL);
+#endif
 
testpmd_logtype = rte_log_register("testpmd");
if (testpmd_logtype < 0)
diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index 8ad0690d8533..355c7d8ca635 100644
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -173,6 +173,7 @@ cmdline_quit(struct cmdline *cl)
 {
if (!cl)
return;
+   cmdline_cancel(cl);
rdline_quit(&cl->rdl);
 }
 
diff --git a/lib/cmdline/cmdline_os_unix.c b/lib/cmdline/cmdline_os_unix.c
index 64a945a34fb3..9a4ec4e33477 100644
--- a/lib/cmdline/cmdline_os_unix.c
+++ b/lib/cmdline/cmdline_os_unix.c
@@ -51,3 +51,9 @@ cmdline_vdprintf(int fd, const char *format, va_list op)
 {
return vdprintf(fd, format, op);
 }
+
+/* This function is not needed on Linux, instead use sigaction() */
+void
+cmdline_cancel(__rte_unused struct cmdline *cl)
+{
+}
diff --git a/lib/cmdline/cmdline_os_windows.c b/lib/cmdline/cmdline_os_windows.c
index 73ed9ba290b8..80863bfc8a00 100644
--- a/lib/cmdline/cmdline_os_windows.c
+++ b/lib/cmdline/cmdline_os_windows.c
@@ -203,3 +203,17 @@ cmdline_vdprintf(int fd, const char *format, va_list op)
 
return ret;
 }
+
+void
+cmdline_cancel(struct cmdline *cl)
+{
+   if (!cl)
+   return;
+
+   /* force the outstanding read on console to exit */
+   if (cl->oldterm.is_console_input) {
+   HANDLE handle = (HANDLE)_get_osfhandle(cl->s_in);
+
+   CancelIoEx(handle, NULL);
+   }
+}
diff --git a/lib/cmdline/cmdline_private.h b/lib/cmdline/cmdline_private.h
index a3271c76934a..86a46cdea61a 100644
--- a/lib/cmdline/cmdline_private.h
+++ b/lib/cmdline/cmdline_private.h
@@ -24,7 +24,7 @@
 #define RDLINE_HISTORY_MAX_LINE 64
 
 struct rdline {
-   enum rdline_status status;
+   volatile enum rdline_status status;
/* rdline bufs */
struct cirbuf left;
struct cirbuf right;
@@ -90,6 +90,9 @@ int cmdline_poll_char(struct cmdline *cl);
 /* Read one character from 

RE: [dpdk-dev] [PATCH] doc: deprecation notice to remove net/bnx2x driver

[Alok Prasad]

> -Original Message-
> From: jer...@marvell.com 
> Sent: 17 March 2023 18:00
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; david.march...@redhat.com; ferruh.yi...@amd.com; 
> andrew.rybche...@oktetlabs.ru; Alok Prasad
> ; Devendra Singh Rawat ; Jerin 
> Jacob Kollanukkaran 
> Subject: [dpdk-dev] [PATCH] doc: deprecation notice to remove net/bnx2x driver
> 
> From: Jerin Jacob 
> 
> Starting from DPDK 23.07, the Marvell QLogic bnx2x driver
> will be removed. This decision has been made to alleviate the burden of
> maintaining a discontinued product.
> 
> Signed-off-by: Jerin Jacob 
> ---
>  doc/guides/rel_notes/deprecation.rst | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst 
> b/doc/guides/rel_notes/deprecation.rst
> index 872847e938..d3d8d0011c 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -135,3 +135,6 @@ Deprecation Notices
>Its removal has been postponed to let potential users report interest
>in maintaining it.
>In the absence of such interest, this library will be removed in DPDK 
> 23.11.
> +
> +* net/bnx2x: Starting from DPDK 23.07, the Marvell QLogic bnx2x driver will 
> be removed.
> +  This decision has been made to alleviate the burden of maintaining a 
> discontinued product.
> --
> 2.40.0

Thanks Jerin!

Acked-by: Alok Prasad 

Re: [PATCH v5 2/2] eal: fix failure path race setting new thread affinity

[David Marchand]

On Fri, Mar 17, 2023 at 3:50 PM Tyler Retzlaff
 wrote:
> > > -struct thread_routine_ctx {
> > > +struct thread_start_context {
> > > rte_thread_func thread_func;
> > > -   void *routine_args;
> > > +   void *thread_args;
> > > +   const rte_thread_attr_t *thread_attr;
> > > +   pthread_mutex_t wrapper_mutex;
> > > +   pthread_cond_t wrapper_cond;
> > > +   int wrapper_ret;
> > > +   volatile int wrapper_done;
> >
> > One question.
> >
> > I see that wrapper_done is accessed under wrapper_mutex.
> > Is volatile needed?
>
> I'm not entirely certain. i'm being cautious since i can conceive of the
> load in the loop being optimized as a single load by the compiler. but
> again i'm not sure, i always like to learn if someone knows better.

After an interesting discussion with Dodji on C99 and side effects
(5.1.2.3/2 and 5.1.2.3/3), I am a bit more convinced that we don't
need this volatile.


>
> >
> > (nit: a boolean is probably enough too)
>
> I have no issue with it being a _Bool if you want to adjust it for that
> i certainly don't object. ordinarily i would use _Bool but a lot of dpdk
> code seems to prefer int so that's why i chose it. if we use the macro
> bool then we should include stdbool.h directly into this translation
> unit.
>
> >
> > I was thinking of squashing below diff:
>
> Yeah, no objection. you can decide if you want to keep the volatile or
> not and add the stdbool.h include.
>
> Thanks for reviewing, appreciate it.

This is a fix but this v5 had an additional change in affinity setting
(switching to rte_thread_set_affinity()).
To be on the safe side wrt backport, I'll also revert to calling
rte_thread_set_affinity_by_id as this is what was being used before.
And this removes the need for patch 1.

Sending a v6 soon, so that it goes through the CI before rc3.


-- 
David Marchand

[PATCH v6] eal/unix: fix thread creation

[David Marchand]

From: Tyler Retzlaff 

In rte_thread_create setting affinity after pthread_create may fail.
Such a failure should result in the entire rte_thread_create failing
but doesn't.

Additionally if there is a failure to set affinity a race exists where
the creating thread will free ctx and depending on scheduling of the new
thread it may also free ctx (double free).

Resolve the above by setting the affinity from the newly created thread
using a condition variable to signal the completion of the thread
start wrapper having completed.

Since we are now waiting for the thread start wrapper to complete we can
allocate the thread start wrapper context on the stack. While here clean
up the variable naming in the context to better highlight the fields of
the context require synchronization between the creating and created
thread.

Fixes: ce6e911d20f6 ("eal: add thread lifetime API")
Cc: sta...@dpdk.org

Signed-off-by: Tyler Retzlaff 
Signed-off-by: David Marchand 
---
Changes since v5:
- dropped volatile and switched to boolean for wrapper_done,
- reverted to rte_thread_set_affinity_by_id() call,

---
 lib/eal/unix/rte_thread.c | 73 ---
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 37ebfcfca1..f4076122a4 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -16,9 +17,14 @@ struct eal_tls_key {
pthread_key_t thread_index;
 };
 
-struct thread_routine_ctx {
+struct thread_start_context {
rte_thread_func thread_func;
-   void *routine_args;
+   void *thread_args;
+   const rte_thread_attr_t *thread_attr;
+   pthread_mutex_t wrapper_mutex;
+   pthread_cond_t wrapper_cond;
+   int wrapper_ret;
+   bool wrapper_done;
 };
 
 static int
@@ -81,13 +87,29 @@ thread_map_os_priority_to_eal_priority(int policy, int 
os_pri,
 }
 
 static void *
-thread_func_wrapper(void *arg)
+thread_start_wrapper(void *arg)
 {
-   struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+   struct thread_start_context *ctx = (struct thread_start_context *)arg;
+   rte_thread_func thread_func = ctx->thread_func;
+   void *thread_args = ctx->thread_args;
+   int ret = 0;
+
+   if (ctx->thread_attr != NULL && CPU_COUNT(&ctx->thread_attr->cpuset) > 
0) {
+   ret = rte_thread_set_affinity_by_id(rte_thread_self(), 
&ctx->thread_attr->cpuset);
+   if (ret != 0)
+   RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id 
failed\n");
+   }
 
-   free(arg);
+   pthread_mutex_lock(&ctx->wrapper_mutex);
+   ctx->wrapper_ret = ret;
+   ctx->wrapper_done = true;
+   pthread_cond_signal(&ctx->wrapper_cond);
+   pthread_mutex_unlock(&ctx->wrapper_mutex);
 
-   return (void *)(uintptr_t)ctx.thread_func(ctx.routine_args);
+   if (ret != 0)
+   return NULL;
+
+   return (void *)(uintptr_t)thread_func(thread_args);
 }
 
 int
@@ -98,20 +120,18 @@ rte_thread_create(rte_thread_t *thread_id,
int ret = 0;
pthread_attr_t attr;
pthread_attr_t *attrp = NULL;
-   struct thread_routine_ctx *ctx;
struct sched_param param = {
.sched_priority = 0,
};
int policy = SCHED_OTHER;
-
-   ctx = calloc(1, sizeof(*ctx));
-   if (ctx == NULL) {
-   RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context 
allocations\n");
-   ret = ENOMEM;
-   goto cleanup;
-   }
-   ctx->routine_args = args;
-   ctx->thread_func = thread_func;
+   struct thread_start_context ctx = {
+   .thread_func = thread_func,
+   .thread_args = args,
+   .thread_attr = thread_attr,
+   .wrapper_done = false,
+   .wrapper_mutex = PTHREAD_MUTEX_INITIALIZER,
+   .wrapper_cond = PTHREAD_COND_INITIALIZER,
+   };
 
if (thread_attr != NULL) {
ret = pthread_attr_init(&attr);
@@ -133,7 +153,6 @@ rte_thread_create(rte_thread_t *thread_id,
goto cleanup;
}
 
-
if (thread_attr->priority ==
RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
ret = ENOTSUP;
@@ -158,24 +177,22 @@ rte_thread_create(rte_thread_t *thread_id,
}
 
ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
-   thread_func_wrapper, ctx);
+   thread_start_wrapper, &ctx);
if (ret != 0) {
RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
goto cleanup;
}
 
-   if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
-   ret = rte_thread_set_affinity_by_id(*thread_id,
-   &thread_attr->cpuset);
-   if (ret != 0) {
-   

Re: [PATCH 2/2] app/testpmd: add testpmd based sleeping

[Robin Jarry]

Anthony Harivel, Mar 16, 2023 at 16:14:
> Sleep for an incremental amount of time if the fwd engine has processed
> less than at least half a burst of packets (i.e 16pkts with default
> setting) on a polling iteration of testpmd.
>
> Upon detecting the threshold of >= 16 pkts on an Rxq, reset the sleep
> time to zero (i.e. no sleep).
>
> Sleep time will be increased on each iteration where the low load
> conditions remain up to a total of the max sleep time which is set by
> the user with the "--max-sleep-us NUM" command line argument or when in
> interactive "mode set max_sleep NUM".
>
> The default max_sleep value is 0, which means that no sleeps will occur
> and the default behaviour is unchanged from previously.
>
> Testing has been performed on AMD EPYC 7702 server with --nb-cores 12.
> The results were obtained via turbostat for each individual lcore:
>
> max_sleep 0 ==   
>   idle4Mpps   16Mpps   Bursts
> === ==   
> C1-state %   0000
> C2-state %   0000
> % usage100  100  100  100
> Watt / core   1.14 1.18 1.19 1.14
> === ==   
>
> max_sleep 500   ==   
>   idle4Mpps   16Mpps   Bursts
> === ==   
> C1-state %  99   85   74 98.6
> C2-state %   0000
> % usage  1   15   261
> Watt / core   0.04 0.18 0.28 0 04
> === ==   
>
> max_sleep 1000  ==   
>   idle4Mpps   16Mpps   Bursts
> === ==   
> C1-state %   0   85   74  0.3
> C2-state %  9900 97.6
> % usage  1   15   251
> Watt / core   0.02 0.18 0.28 0 02
> === ==   
>
> On most cases, the consumption of the cores is greatly improved while
> still performing zero packet loss.
>
> Latency test has been performed on each tests above. The CPU has a C1
> latency of 1us and a C2 latency of 400us. On the worst case scenario, Tx
> Burst of thousands packets every seconds, the following latency in us
> (micro seconds) has been observed:
>
> ===  = ==
> max_sleep  0   500   1000
> ---  - --
> max latency   14   560   1260
> min latency5 5  6
> Avg latency7   305617
> ===  = ==
>
> link: 
> https://www.github.com/torvalds/linux/tree/master/tools/power/x86/turbostat
> Signed-off-by: Anthony Harivel 

Given the amount of time testpmd is used in local development and
automated testing, adding such an option is a great addition to save CPU
power.

Thanks Anthony.

Reviewed-by: Robin Jarry 

[PATCH 0/7] replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

This series covers the libraries and drivers that are built on Windows.

The code has be converted to use the __atomic builtins but there are
additional during conversion i notice that there may be some issues
that need to be addressed.

I'll comment in the patches where my concerns are so the maintainers
may comment.

Tyler Retzlaff (7):
  ring: replace rte atomics with GCC builtin atomics
  stack: replace rte atomics with GCC builtin atomics
  dma/idxd: replace rte atomics with GCC builtin atomics
  net/ice: replace rte atomics with GCC builtin atomics
  net/ixgbe: replace rte atomics with GCC builtin atomics
  net/null: replace rte atomics with GCC builtin atomics
  net/ring: replace rte atomics with GCC builtin atomics

 drivers/dma/idxd/idxd_internal.h |  3 +--
 drivers/dma/idxd/idxd_pci.c  |  6 +++---
 drivers/net/ice/ice_dcf.c|  1 -
 drivers/net/ice/ice_dcf_ethdev.c |  1 -
 drivers/net/ice/ice_ethdev.c | 10 ++
 drivers/net/ixgbe/ixgbe_bypass.c |  1 -
 drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++--
 drivers/net/ixgbe/ixgbe_ethdev.h |  3 ++-
 drivers/net/ixgbe/ixgbe_flow.c   |  1 -
 drivers/net/ixgbe/ixgbe_rxtx.c   |  1 -
 drivers/net/null/rte_eth_null.c  | 20 ++--
 drivers/net/ring/rte_eth_ring.c  | 20 ++--
 lib/ring/rte_ring_core.h |  1 -
 lib/ring/rte_ring_generic_pvt.h  | 10 ++
 lib/stack/rte_stack_lf_generic.h | 11 +--
 15 files changed, 49 insertions(+), 52 deletions(-)

-- 
1.8.3.1

[PATCH 1/7] ring: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 lib/ring/rte_ring_core.h|  1 -
 lib/ring/rte_ring_generic_pvt.h | 10 ++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
index 82b2370..b9c7860 100644
--- a/lib/ring/rte_ring_core.h
+++ b/lib/ring/rte_ring_core.h
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/lib/ring/rte_ring_generic_pvt.h b/lib/ring/rte_ring_generic_pvt.h
index 5acb6e5..f9a15b6 100644
--- a/lib/ring/rte_ring_generic_pvt.h
+++ b/lib/ring/rte_ring_generic_pvt.h
@@ -92,8 +92,9 @@
if (is_sp)
r->prod.head = *new_head, success = 1;
else
-   success = rte_atomic32_cmpset(&r->prod.head,
-   *old_head, *new_head);
+   success = __atomic_compare_exchange_n(&r->prod.head,
+   old_head, *new_head, 0,
+   __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
} while (unlikely(success == 0));
return n;
 }
@@ -162,8 +163,9 @@
rte_smp_rmb();
success = 1;
} else {
-   success = rte_atomic32_cmpset(&r->cons.head, *old_head,
-   *new_head);
+   success = __atomic_compare_exchange_n(&r->cons.head,
+   old_head, *new_head, 0,
+   __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
}
} while (unlikely(success == 0));
return n;
-- 
1.8.3.1

[PATCH 3/7] dma/idxd: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 drivers/dma/idxd/idxd_internal.h | 3 +--
 drivers/dma/idxd/idxd_pci.c  | 6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/idxd/idxd_internal.h b/drivers/dma/idxd/idxd_internal.h
index 180a858..53a0c8e 100644
--- a/drivers/dma/idxd/idxd_internal.h
+++ b/drivers/dma/idxd/idxd_internal.h
@@ -7,7 +7,6 @@
 
 #include 
 #include 
-#include 
 
 #include "idxd_hw_defs.h"
 
@@ -34,7 +33,7 @@ struct idxd_pci_common {
rte_spinlock_t lk;
 
uint8_t wq_cfg_sz;
-   rte_atomic16_t ref_count;
+   int16_t ref_count;
volatile struct rte_idxd_bar0 *regs;
volatile uint32_t *wq_regs_base;
volatile struct rte_idxd_grpcfg *grp_regs;
diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
index 781fa02..e869d33 100644
--- a/drivers/dma/idxd/idxd_pci.c
+++ b/drivers/dma/idxd/idxd_pci.c
@@ -6,7 +6,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "idxd_internal.h"
 
@@ -136,7 +135,8 @@
/* if this is the last WQ on the device, disable the device and free
 * the PCI struct
 */
-   is_last_wq = rte_atomic16_dec_and_test(&idxd->u.pci->ref_count);
+   is_last_wq = __atomic_fetch_sub(&idxd->u.pci->ref_count, 1,
+   __ATOMIC_SEQ_CST) - 1 == 0;
if (is_last_wq) {
/* disable the device */
err_code = idxd_pci_dev_command(idxd, idxd_disable_dev);
@@ -350,7 +350,7 @@
free(idxd.u.pci);
return ret;
}
-   rte_atomic16_inc(&idxd.u.pci->ref_count);
+   __atomic_fetch_add(&idxd.u.pci->ref_count, 1, __ATOMIC_SEQ_CST);
}
 
return 0;
-- 
1.8.3.1

[PATCH 2/7] stack: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 lib/stack/rte_stack_lf_generic.h | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/stack/rte_stack_lf_generic.h b/lib/stack/rte_stack_lf_generic.h
index 7fa29ce..3ef0f74 100644
--- a/lib/stack/rte_stack_lf_generic.h
+++ b/lib/stack/rte_stack_lf_generic.h
@@ -26,8 +26,7 @@
 * elements. If the mempool is near-empty to the point that this is a
 * concern, the user should consider increasing the mempool size.
 */
-   return (unsigned int)rte_atomic64_read((rte_atomic64_t *)
-   &s->stack_lf.used.len);
+   return __atomic_load_n(&s->stack_lf.used.len, __ATOMIC_SEQ_CST);
 }
 
 static __rte_always_inline void
@@ -68,7 +67,7 @@
__ATOMIC_RELAXED);
} while (success == 0);
 
-   rte_atomic64_add((rte_atomic64_t *)&list->len, num);
+   __atomic_fetch_add(&list->len, num, __ATOMIC_SEQ_CST);
 }
 
 static __rte_always_inline struct rte_stack_lf_elem *
@@ -82,14 +81,14 @@
 
/* Reserve num elements, if available */
while (1) {
-   uint64_t len = rte_atomic64_read((rte_atomic64_t *)&list->len);
+   uint64_t len = __atomic_load_n(&list->len, __ATOMIC_SEQ_CST);
 
/* Does the list contain enough elements? */
if (unlikely(len < num))
return NULL;
 
-   if (rte_atomic64_cmpset((volatile uint64_t *)&list->len,
-   len, len - num))
+   if (__atomic_compare_exchange_n(&list->len, &len, len - num,
+   0, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
break;
}
 
-- 
1.8.3.1

[PATCH 4/7] net/ice: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/ice/ice_dcf.c|  1 -
 drivers/net/ice/ice_dcf_ethdev.c |  1 -
 drivers/net/ice/ice_ethdev.c | 10 ++
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
index 1c3d22a..80d2cbd 100644
--- a/drivers/net/ice/ice_dcf.c
+++ b/drivers/net/ice/ice_dcf.c
@@ -14,7 +14,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/net/ice/ice_dcf_ethdev.c b/drivers/net/ice/ice_dcf_ethdev.c
index dcbf2af..13ff245 100644
--- a/drivers/net/ice/ice_dcf_ethdev.c
+++ b/drivers/net/ice/ice_dcf_ethdev.c
@@ -11,7 +11,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 9a88cf9..bdf4569 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -3927,8 +3927,9 @@ static int ice_init_rss(struct ice_pf *pf)
struct rte_eth_link *dst = link;
struct rte_eth_link *src = &dev->data->dev_link;
 
-   if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-   *(uint64_t *)src) == 0)
+   if (!__atomic_compare_exchange_n((uint64_t *)dst,
+   (uint64_t *)dst, *(uint64_t *)src, 0,
+   __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
return -1;
 
return 0;
@@ -3941,8 +3942,9 @@ static int ice_init_rss(struct ice_pf *pf)
struct rte_eth_link *dst = &dev->data->dev_link;
struct rte_eth_link *src = link;
 
-   if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
-   *(uint64_t *)src) == 0)
+   if (!__atomic_compare_exchange_n((uint64_t *)dst,
+   (uint64_t *)dst, *(uint64_t *)src, 0,
+   __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
return -1;
 
return 0;
-- 
1.8.3.1

[PATCH 6/7] net/null: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/null/rte_eth_null.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 47d9554..195c3bd 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -37,8 +37,8 @@ struct null_queue {
struct rte_mempool *mb_pool;
struct rte_mbuf *dummy_packet;
 
-   rte_atomic64_t rx_pkts;
-   rte_atomic64_t tx_pkts;
+   int64_t rx_pkts;
+   int64_t tx_pkts;
 };
 
 struct pmd_options {
@@ -101,7 +101,7 @@ struct pmd_internals {
bufs[i]->port = h->internals->port_id;
}
 
-   rte_atomic64_add(&(h->rx_pkts), i);
+   __atomic_fetch_add(&h->rx_pkts, i, __ATOMIC_SEQ_CST);
 
return i;
 }
@@ -128,7 +128,7 @@ struct pmd_internals {
bufs[i]->port = h->internals->port_id;
}
 
-   rte_atomic64_add(&(h->rx_pkts), i);
+   __atomic_fetch_add(&h->rx_pkts, i, __ATOMIC_SEQ_CST);
 
return i;
 }
@@ -152,7 +152,7 @@ struct pmd_internals {
for (i = 0; i < nb_bufs; i++)
rte_pktmbuf_free(bufs[i]);
 
-   rte_atomic64_add(&(h->tx_pkts), i);
+   __atomic_fetch_add(&h->tx_pkts, i, __ATOMIC_SEQ_CST);
 
return i;
 }
@@ -174,7 +174,7 @@ struct pmd_internals {
rte_pktmbuf_free(bufs[i]);
}
 
-   rte_atomic64_add(&(h->tx_pkts), i);
+   __atomic_fetch_add(&h->tx_pkts, i, __ATOMIC_SEQ_CST);
 
return i;
 }
@@ -317,7 +317,7 @@ struct pmd_internals {
RTE_DIM(internal->rx_null_queues)));
for (i = 0; i < num_stats; i++) {
igb_stats->q_ipackets[i] =
-   internal->rx_null_queues[i].rx_pkts.cnt;
+   internal->rx_null_queues[i].rx_pkts;
rx_total += igb_stats->q_ipackets[i];
}
 
@@ -326,7 +326,7 @@ struct pmd_internals {
RTE_DIM(internal->tx_null_queues)));
for (i = 0; i < num_stats; i++) {
igb_stats->q_opackets[i] =
-   internal->tx_null_queues[i].tx_pkts.cnt;
+   internal->tx_null_queues[i].tx_pkts;
tx_total += igb_stats->q_opackets[i];
}
 
@@ -347,9 +347,9 @@ struct pmd_internals {
 
internal = dev->data->dev_private;
for (i = 0; i < RTE_DIM(internal->rx_null_queues); i++)
-   internal->rx_null_queues[i].rx_pkts.cnt = 0;
+   internal->rx_null_queues[i].rx_pkts = 0;
for (i = 0; i < RTE_DIM(internal->tx_null_queues); i++)
-   internal->tx_null_queues[i].tx_pkts.cnt = 0;
+   internal->tx_null_queues[i].tx_pkts = 0;
 
return 0;
 }
-- 
1.8.3.1

[PATCH 7/7] net/ring: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/ring/rte_eth_ring.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index e8bc9b6..15d4a3d 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -44,8 +44,8 @@ enum dev_action {
 
 struct ring_queue {
struct rte_ring *rng;
-   rte_atomic64_t rx_pkts;
-   rte_atomic64_t tx_pkts;
+   int64_t rx_pkts;
+   int64_t tx_pkts;
 };
 
 struct pmd_internals {
@@ -80,9 +80,9 @@ struct pmd_internals {
const uint16_t nb_rx = (uint16_t)rte_ring_dequeue_burst(r->rng,
ptrs, nb_bufs, NULL);
if (r->rng->flags & RING_F_SC_DEQ)
-   r->rx_pkts.cnt += nb_rx;
+   r->rx_pkts += nb_rx;
else
-   rte_atomic64_add(&(r->rx_pkts), nb_rx);
+   __atomic_fetch_add(&r->rx_pkts, nb_rx, __ATOMIC_SEQ_CST);
return nb_rx;
 }
 
@@ -94,9 +94,9 @@ struct pmd_internals {
const uint16_t nb_tx = (uint16_t)rte_ring_enqueue_burst(r->rng,
ptrs, nb_bufs, NULL);
if (r->rng->flags & RING_F_SP_ENQ)
-   r->tx_pkts.cnt += nb_tx;
+   r->tx_pkts += nb_tx;
else
-   rte_atomic64_add(&(r->tx_pkts), nb_tx);
+   __atomic_fetch_add(&r->tx_pkts, nb_tx, __ATOMIC_SEQ_CST);
return nb_tx;
 }
 
@@ -184,13 +184,13 @@ struct pmd_internals {
 
for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
i < dev->data->nb_rx_queues; i++) {
-   stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt;
+   stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts;
rx_total += stats->q_ipackets[i];
}
 
for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
i < dev->data->nb_tx_queues; i++) {
-   stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt;
+   stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts;
tx_total += stats->q_opackets[i];
}
 
@@ -207,9 +207,9 @@ struct pmd_internals {
struct pmd_internals *internal = dev->data->dev_private;
 
for (i = 0; i < dev->data->nb_rx_queues; i++)
-   internal->rx_ring_queues[i].rx_pkts.cnt = 0;
+   internal->rx_ring_queues[i].rx_pkts = 0;
for (i = 0; i < dev->data->nb_tx_queues; i++)
-   internal->tx_ring_queues[i].tx_pkts.cnt = 0;
+   internal->tx_ring_queues[i].tx_pkts = 0;
 
return 0;
 }
-- 
1.8.3.1

[PATCH 5/7] net/ixgbe: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

Replace the use of rte_atomic.h types and functions, instead use GCC
supplied C++11 memory model builtins.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/ixgbe/ixgbe_bypass.c |  1 -
 drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++--
 drivers/net/ixgbe/ixgbe_ethdev.h |  3 ++-
 drivers/net/ixgbe/ixgbe_flow.c   |  1 -
 drivers/net/ixgbe/ixgbe_rxtx.c   |  1 -
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_bypass.c b/drivers/net/ixgbe/ixgbe_bypass.c
index 94f34a2..f615d18 100644
--- a/drivers/net/ixgbe/ixgbe_bypass.c
+++ b/drivers/net/ixgbe/ixgbe_bypass.c
@@ -3,7 +3,6 @@
  */
 
 #include 
-#include 
 #include 
 #include "ixgbe_ethdev.h"
 #include "ixgbe_bypass_api.h"
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 88118bc..3efb5ff 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1127,7 +1127,7 @@ struct rte_ixgbe_xstats_name_off {
return 0;
}
 
-   rte_atomic32_clear(&ad->link_thread_running);
+   __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
ixgbe_parse_devargs(eth_dev->data->dev_private,
pci_dev->device.devargs);
rte_eth_copy_pci_info(eth_dev, pci_dev);
@@ -1625,7 +1625,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev 
*eth_dev)
return 0;
}
 
-   rte_atomic32_clear(&ad->link_thread_running);
+   __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
ixgbevf_parse_devargs(eth_dev->data->dev_private,
  pci_dev->device.devargs);
 
@@ -4186,7 +4186,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
struct ixgbe_adapter *ad = dev->data->dev_private;
uint32_t timeout = timeout_ms ? timeout_ms : WARNING_TIMEOUT;
 
-   while (rte_atomic32_read(&ad->link_thread_running)) {
+   while (__atomic_load_n(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
msec_delay(1);
timeout--;
 
@@ -4222,7 +4222,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
ixgbe_setup_link(hw, speed, true);
 
intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
-   rte_atomic32_clear(&ad->link_thread_running);
+   __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
return NULL;
 }
 
@@ -4317,7 +4317,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
if (link_up == 0) {
if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
ixgbe_dev_wait_setup_link_complete(dev, 0);
-   if 
(rte_atomic32_test_and_set(&ad->link_thread_running)) {
+   if (__atomic_test_and_set(&ad->link_thread_running, 
__ATOMIC_SEQ_CST)) {
/* To avoid race condition between threads, set
 * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
 * when there is no link thread running.
@@ -4330,7 +4330,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
dev) < 0) {
PMD_DRV_LOG(ERR,
"Create link thread failed!");
-   
rte_atomic32_clear(&ad->link_thread_running);
+   
__atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
}
} else {
PMD_DRV_LOG(ERR,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 48290af..2ca6998 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -6,6 +6,7 @@
 #define _IXGBE_ETHDEV_H_
 
 #include 
+#include 
 #include 
 
 #include "base/ixgbe_type.h"
@@ -510,7 +511,7 @@ struct ixgbe_adapter {
 */
uint8_t pflink_fullchk;
uint8_t mac_ctrl_frame_fwd;
-   rte_atomic32_t link_thread_running;
+   bool link_thread_running;
pthread_t link_thread_tid;
 };
 
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index eac81ee..687341c 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -18,7 +18,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index c9d6ca9..8d7251d 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -27,7 +27,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.8.3.1

Re: [PATCH 1/7] ring: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 01:19:42PM -0700, Tyler Retzlaff wrote:
> Replace the use of rte_atomic.h types and functions, instead use GCC
> supplied C++11 memory model builtins.
> 
> Signed-off-by: Tyler Retzlaff 
> ---
>  lib/ring/rte_ring_core.h|  1 -
>  lib/ring/rte_ring_generic_pvt.h | 10 ++
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
> index 82b2370..b9c7860 100644
> --- a/lib/ring/rte_ring_core.h
> +++ b/lib/ring/rte_ring_core.h
> @@ -31,7 +31,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/lib/ring/rte_ring_generic_pvt.h b/lib/ring/rte_ring_generic_pvt.h
> index 5acb6e5..f9a15b6 100644
> --- a/lib/ring/rte_ring_generic_pvt.h
> +++ b/lib/ring/rte_ring_generic_pvt.h
> @@ -92,8 +92,9 @@
>   if (is_sp)
>   r->prod.head = *new_head, success = 1;
>   else
> - success = rte_atomic32_cmpset(&r->prod.head,
> - *old_head, *new_head);
> + success = __atomic_compare_exchange_n(&r->prod.head,
> + old_head, *new_head, 0,
> + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
>   } while (unlikely(success == 0));
>   return n;
>  }
> @@ -162,8 +163,9 @@
>   rte_smp_rmb();
>   success = 1;
>   } else {
> - success = rte_atomic32_cmpset(&r->cons.head, *old_head,
> - *new_head);
> + success = __atomic_compare_exchange_n(&r->cons.head,
> + old_head, *new_head, 0,
> + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
>   }
>   } while (unlikely(success == 0));
>   return n;

just something i noticed and not related to this change.

i note that old_head for both __rte_ring_move_prod_head and
__rte_ring_move_con_head are performing a non-atomic load to
initialize `*old_head` probably not the best idea.

Re: [PATCH 4/7] net/ice: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 01:19:45PM -0700, Tyler Retzlaff wrote:
> Replace the use of rte_atomic.h types and functions, instead use GCC
> supplied C++11 memory model builtins.
> 
> Signed-off-by: Tyler Retzlaff 
> ---
>  drivers/net/ice/ice_dcf.c|  1 -
>  drivers/net/ice/ice_dcf_ethdev.c |  1 -
>  drivers/net/ice/ice_ethdev.c | 10 ++
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
> index 1c3d22a..80d2cbd 100644
> --- a/drivers/net/ice/ice_dcf.c
> +++ b/drivers/net/ice/ice_dcf.c
> @@ -14,7 +14,6 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/net/ice/ice_dcf_ethdev.c 
> b/drivers/net/ice/ice_dcf_ethdev.c
> index dcbf2af..13ff245 100644
> --- a/drivers/net/ice/ice_dcf_ethdev.c
> +++ b/drivers/net/ice/ice_dcf_ethdev.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index 9a88cf9..bdf4569 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -3927,8 +3927,9 @@ static int ice_init_rss(struct ice_pf *pf)
>   struct rte_eth_link *dst = link;
>   struct rte_eth_link *src = &dev->data->dev_link;
>  
> - if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> - *(uint64_t *)src) == 0)
> + if (!__atomic_compare_exchange_n((uint64_t *)dst,
> + (uint64_t *)dst, *(uint64_t *)src, 0,
> + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
>   return -1;
>  
>   return 0;
> @@ -3941,8 +3942,9 @@ static int ice_init_rss(struct ice_pf *pf)
>   struct rte_eth_link *dst = &dev->data->dev_link;
>   struct rte_eth_link *src = link;
>  
> - if (rte_atomic64_cmpset((uint64_t *)dst, *(uint64_t *)dst,
> - *(uint64_t *)src) == 0)
> + if (!__atomic_compare_exchange_n((uint64_t *)dst,
> + (uint64_t *)dst, *(uint64_t *)src, 0,
> + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST))
>   return -1;
>  

*(uint64_t *)dst for the second parameter look like a bug to me,
a non-atomic load will be generated.

probably this code should be corrected by performing __atomic_load_n(dst, ...)
to a stack variable and then performing the cmpset/compare_exchange.

Re: [PATCH 6/7] net/null: replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 01:19:47PM -0700, Tyler Retzlaff wrote:
> Replace the use of rte_atomic.h types and functions, instead use GCC
> supplied C++11 memory model builtins.
> 
> Signed-off-by: Tyler Retzlaff 
> ---
>  drivers/net/null/rte_eth_null.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> index 47d9554..195c3bd 100644
> --- a/drivers/net/null/rte_eth_null.c
> +++ b/drivers/net/null/rte_eth_null.c
> @@ -37,8 +37,8 @@ struct null_queue {
>   struct rte_mempool *mb_pool;
>   struct rte_mbuf *dummy_packet;
>  
> - rte_atomic64_t rx_pkts;
> - rte_atomic64_t tx_pkts;
> + int64_t rx_pkts;
> + int64_t tx_pkts;
>  };
>  
>  struct pmd_options {
> @@ -101,7 +101,7 @@ struct pmd_internals {
>   bufs[i]->port = h->internals->port_id;
>   }
>  
> - rte_atomic64_add(&(h->rx_pkts), i);
> + __atomic_fetch_add(&h->rx_pkts, i, __ATOMIC_SEQ_CST);
>  
>   return i;
>  }
> @@ -128,7 +128,7 @@ struct pmd_internals {
>   bufs[i]->port = h->internals->port_id;
>   }
>  
> - rte_atomic64_add(&(h->rx_pkts), i);
> + __atomic_fetch_add(&h->rx_pkts, i, __ATOMIC_SEQ_CST);
>  
>   return i;
>  }
> @@ -152,7 +152,7 @@ struct pmd_internals {
>   for (i = 0; i < nb_bufs; i++)
>   rte_pktmbuf_free(bufs[i]);
>  
> - rte_atomic64_add(&(h->tx_pkts), i);
> + __atomic_fetch_add(&h->tx_pkts, i, __ATOMIC_SEQ_CST);
>  
>   return i;
>  }
> @@ -174,7 +174,7 @@ struct pmd_internals {
>   rte_pktmbuf_free(bufs[i]);
>   }
>  
> - rte_atomic64_add(&(h->tx_pkts), i);
> + __atomic_fetch_add(&h->tx_pkts, i, __ATOMIC_SEQ_CST);
>  
>   return i;
>  }
> @@ -317,7 +317,7 @@ struct pmd_internals {
>   RTE_DIM(internal->rx_null_queues)));
>   for (i = 0; i < num_stats; i++) {
>   igb_stats->q_ipackets[i] =
> - internal->rx_null_queues[i].rx_pkts.cnt;
> + internal->rx_null_queues[i].rx_pkts;
>   rx_total += igb_stats->q_ipackets[i];
>   }
>  
> @@ -326,7 +326,7 @@ struct pmd_internals {
>   RTE_DIM(internal->tx_null_queues)));
>   for (i = 0; i < num_stats; i++) {
>   igb_stats->q_opackets[i] =
> - internal->tx_null_queues[i].tx_pkts.cnt;
> + internal->tx_null_queues[i].tx_pkts;
>   tx_total += igb_stats->q_opackets[i];
>   }
>  

these variables are operated on with atomic builtins in other places
yet here they are being non-atomically loaded. should probably be using
_atomic_load_n(...)

> @@ -347,9 +347,9 @@ struct pmd_internals {
>  
>   internal = dev->data->dev_private;
>   for (i = 0; i < RTE_DIM(internal->rx_null_queues); i++)
> - internal->rx_null_queues[i].rx_pkts.cnt = 0;
> + internal->rx_null_queues[i].rx_pkts = 0;
>   for (i = 0; i < RTE_DIM(internal->tx_null_queues); i++)
> - internal->tx_null_queues[i].tx_pkts.cnt = 0;
> + internal->tx_null_queues[i].tx_pkts = 0;

same thing, these should probably be __atomic_store_n(...)

Re: [PATCH v5] testpmd: go back to using cmdline_interact

[Olivier Matz]

On Fri, Mar 17, 2023 at 09:59:41AM -0700, Stephen Hemminger wrote:
> The cmdline_poll() function is broken and was
> not fully tested, don't use it.
> 
> Instead, use sigaction() to cancel read character on Unix OS's
> and a new helper to cancel I/O on Windows.
> 
> Fixes: 0fd1386c30c3 ("app/testpmd: cleanup cleanly from signal")
> Bugzilla ID: 1180
> Signed-off-by: Stephen Hemminger 

Acked-by: Olivier Matz 

Thank you

Re: [PATCH v5 2/2] eal: fix failure path race setting new thread affinity

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 07:51:25PM +0100, David Marchand wrote:
> On Fri, Mar 17, 2023 at 3:50 PM Tyler Retzlaff
>  wrote:
> > > > -struct thread_routine_ctx {
> > > > +struct thread_start_context {
> > > > rte_thread_func thread_func;
> > > > -   void *routine_args;
> > > > +   void *thread_args;
> > > > +   const rte_thread_attr_t *thread_attr;
> > > > +   pthread_mutex_t wrapper_mutex;
> > > > +   pthread_cond_t wrapper_cond;
> > > > +   int wrapper_ret;
> > > > +   volatile int wrapper_done;
> > >
> > > One question.
> > >
> > > I see that wrapper_done is accessed under wrapper_mutex.
> > > Is volatile needed?
> >
> > I'm not entirely certain. i'm being cautious since i can conceive of the
> > load in the loop being optimized as a single load by the compiler. but
> > again i'm not sure, i always like to learn if someone knows better.
> 
> After an interesting discussion with Dodji on C99 and side effects
> (5.1.2.3/2 and 5.1.2.3/3), I am a bit more convinced that we don't
> need this volatile.

Thanks for the references, based on the reading i agree we can drop the
volatile.

> 
> 
> >
> > >
> > > (nit: a boolean is probably enough too)
> >
> > I have no issue with it being a _Bool if you want to adjust it for that
> > i certainly don't object. ordinarily i would use _Bool but a lot of dpdk
> > code seems to prefer int so that's why i chose it. if we use the macro
> > bool then we should include stdbool.h directly into this translation
> > unit.
> >
> > >
> > > I was thinking of squashing below diff:
> >
> > Yeah, no objection. you can decide if you want to keep the volatile or
> > not and add the stdbool.h include.
> >
> > Thanks for reviewing, appreciate it.
> 
> This is a fix but this v5 had an additional change in affinity setting
> (switching to rte_thread_set_affinity()).
> To be on the safe side wrt backport, I'll also revert to calling
> rte_thread_set_affinity_by_id as this is what was being used before.
> And this removes the need for patch 1.

Is it worth merging the const patch but not backporting? I'm not fussed
either way.

> 
> Sending a v6 soon, so that it goes through the CI before rc3.

Yes, great.

Thanks David!

> 
> 
> -- 
> David Marchand

Re: [PATCH v6] eal/unix: fix thread creation

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 07:52:29PM +0100, David Marchand wrote:
> From: Tyler Retzlaff 
> 
> In rte_thread_create setting affinity after pthread_create may fail.
> Such a failure should result in the entire rte_thread_create failing
> but doesn't.
> 
> Additionally if there is a failure to set affinity a race exists where
> the creating thread will free ctx and depending on scheduling of the new
> thread it may also free ctx (double free).
> 
> Resolve the above by setting the affinity from the newly created thread
> using a condition variable to signal the completion of the thread
> start wrapper having completed.
> 
> Since we are now waiting for the thread start wrapper to complete we can
> allocate the thread start wrapper context on the stack. While here clean
> up the variable naming in the context to better highlight the fields of
> the context require synchronization between the creating and created
> thread.
> 
> Fixes: ce6e911d20f6 ("eal: add thread lifetime API")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Tyler Retzlaff 
> Signed-off-by: David Marchand 
> ---

Looks good to me, not sure if you need a Reviewed-by: from me for the
changes but here is one anyway.

v5
Reviewed-by: Tyler Retzlaff 

Re: [PATCH 0/7] replace rte atomics with GCC builtin atomics

[Stephen Hemminger]

On Fri, 17 Mar 2023 13:19:41 -0700
Tyler Retzlaff  wrote:

> Replace the use of rte_atomic.h types and functions, instead use GCC
> supplied C++11 memory model builtins.
> 
> This series covers the libraries and drivers that are built on Windows.
> 
> The code has be converted to use the __atomic builtins but there are
> additional during conversion i notice that there may be some issues
> that need to be addressed.

I don't think all these cmpset need to use SEQ_CST.
Especially for the places where it is used a loop, might
be more efficient with some of the other memory models.

Re: [PATCH 0/7] replace rte atomics with GCC builtin atomics

[Tyler Retzlaff]

On Fri, Mar 17, 2023 at 02:42:26PM -0700, Stephen Hemminger wrote:
> On Fri, 17 Mar 2023 13:19:41 -0700
> Tyler Retzlaff  wrote:
> 
> > Replace the use of rte_atomic.h types and functions, instead use GCC
> > supplied C++11 memory model builtins.
> > 
> > This series covers the libraries and drivers that are built on Windows.
> > 
> > The code has be converted to use the __atomic builtins but there are
> > additional during conversion i notice that there may be some issues
> > that need to be addressed.
> 
> I don't think all these cmpset need to use SEQ_CST.
> Especially for the places where it is used a loop, might
> be more efficient with some of the other memory models.

i agree.

however, i'm not trying to improve the code with this change, just
decouple it from rte_atomics.h so trying my best to avoid any
unnecessary semantic change.

certainly if the maintainers of this code wish to weaken the ordering
where appropriate after the change is merged they can do so and handily
this change has enabled them to do so easily allowing them to test just
their change in isolation.

[PATCH 2/6] net/ixgbe: use rte thread API

[Tyler Retzlaff]

Update driver to use rte thread API where available instead of pthread
as a prerequisite to removing pthread stubs on Windows.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 10 +-
 drivers/net/ixgbe/ixgbe_ethdev.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 88118bc..3abe96e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -236,7 +236,7 @@ static int ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev);
 static void ixgbe_dev_interrupt_handler(void *param);
 static void ixgbe_dev_interrupt_delayed_handler(void *param);
-static void *ixgbe_dev_setup_link_thread_handler(void *param);
+static uint32_t ixgbe_dev_setup_link_thread_handler(void *param);
 static int ixgbe_dev_wait_setup_link_complete(struct rte_eth_dev *dev,
  uint32_t timeout_ms);
 
@@ -4203,7 +4203,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
return 1;
 }
 
-static void *
+static uint32_t
 ixgbe_dev_setup_link_thread_handler(void *param)
 {
struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
@@ -4214,7 +4214,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
u32 speed;
bool autoneg = false;
 
-   pthread_detach(pthread_self());
+   rte_thread_detach(rte_thread_self());
speed = hw->phy.autoneg_advertised;
if (!speed)
ixgbe_get_link_capabilities(hw, &speed, &autoneg);
@@ -4223,7 +4223,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
 
intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
rte_atomic32_clear(&ad->link_thread_running);
-   return NULL;
+   return 0;
 }
 
 /*
@@ -4323,7 +4323,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused 
struct rte_eth_dev *dev,
 * when there is no link thread running.
 */
intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
-   if (rte_ctrl_thread_create(&ad->link_thread_tid,
+   if 
(rte_thread_create_control(&ad->link_thread_tid,
"ixgbe-link-handler",
NULL,
ixgbe_dev_setup_link_thread_handler,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 48290af..4332b2c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -511,7 +511,7 @@ struct ixgbe_adapter {
uint8_t pflink_fullchk;
uint8_t mac_ctrl_frame_fwd;
rte_atomic32_t link_thread_running;
-   pthread_t link_thread_tid;
+   rte_thread_t link_thread_tid;
 };
 
 struct ixgbe_vf_representor {
-- 
1.8.3.1

[PATCH 0/6] windows: remove most pthread lifetime shim functions

[Tyler Retzlaff]

Adopt rte thread APIs in code built for Windows to decouple it from the
pthread shim.

Remove most of the pthread_xxx lifetime shim functions, only
pthread_create remains while we wait for rte_ctrl_thread_create removal.

Tyler Retzlaff (6):
  dma/skeleton: use rte thread API
  net/ixgbe: use rte thread API
  net/ice: use rte thread API
  net/iavf: use rte thread API
  eal: use rte thread API
  windows: remove most pthread lifetime shim functions

 drivers/dma/skeleton/skeleton_dmadev.c | 15 +++---
 drivers/dma/skeleton/skeleton_dmadev.h |  4 +-
 drivers/net/iavf/iavf_vchnl.c  | 12 ++---
 drivers/net/ice/ice_dcf_parent.c   | 11 ++--
 drivers/net/ixgbe/ixgbe_ethdev.c   | 10 ++--
 drivers/net/ixgbe/ixgbe_ethdev.h   |  2 +-
 lib/eal/common/eal_common_thread.c |  4 +-
 lib/eal/windows/eal.c  |  2 +-
 lib/eal/windows/eal_interrupts.c   | 12 ++---
 lib/eal/windows/include/pthread.h  | 99 --
 10 files changed, 36 insertions(+), 135 deletions(-)

-- 
1.8.3.1

[PATCH 3/6] net/ice: use rte thread API

[Tyler Retzlaff]

Update driver to use rte thread API where available instead of pthread
as a prerequisite to removing pthread stubs on Windows.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/ice/ice_dcf_parent.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
index 01e390d..3175d18 100644
--- a/drivers/net/ice/ice_dcf_parent.c
+++ b/drivers/net/ice/ice_dcf_parent.c
@@ -4,7 +4,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -115,7 +114,7 @@ struct ice_dcf_reset_event_param {
pf_vsi_idx, vsi_ctx->vsi_num);
 }
 
-static void*
+static uint32_t
 ice_dcf_vsi_update_service_handler(void *param)
 {
struct ice_dcf_reset_event_param *reset_param = param;
@@ -124,7 +123,7 @@ struct ice_dcf_reset_event_param {
container_of(hw, struct ice_dcf_adapter, real_hw);
struct ice_adapter *parent_adapter = &adapter->parent;
 
-   pthread_detach(pthread_self());
+   rte_thread_detach(rte_thread_self());
 
rte_delay_us(ICE_DCF_VSI_UPDATE_SERVICE_INTERVAL);
 
@@ -154,7 +153,7 @@ struct ice_dcf_reset_event_param {
 
free(param);
 
-   return NULL;
+   return 0;
 }
 
 static void
@@ -163,7 +162,7 @@ struct ice_dcf_reset_event_param {
 #define THREAD_NAME_LEN16
struct ice_dcf_reset_event_param *param;
char name[THREAD_NAME_LEN];
-   pthread_t thread;
+   rte_thread_t thread;
int ret;
 
param = malloc(sizeof(*param));
@@ -177,7 +176,7 @@ struct ice_dcf_reset_event_param {
param->vf_id = vf_id;
 
snprintf(name, sizeof(name), "ice-reset-%u", vf_id);
-   ret = rte_ctrl_thread_create(&thread, name, NULL,
+   ret = rte_thread_create_control(&thread, name, NULL,
 ice_dcf_vsi_update_service_handler, param);
if (ret != 0) {
PMD_DRV_LOG(ERR, "Failed to start the thread for reset 
handling");
-- 
1.8.3.1

[PATCH 1/6] dma/skeleton: use rte thread API

[Tyler Retzlaff]

Update driver to use rte thread API where available instead of pthread
as a prerequisite to removing pthread stubs on Windows.

Signed-off-by: Tyler Retzlaff 
---
 drivers/dma/skeleton/skeleton_dmadev.c | 15 ---
 drivers/dma/skeleton/skeleton_dmadev.h |  4 ++--
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/skeleton/skeleton_dmadev.c 
b/drivers/dma/skeleton/skeleton_dmadev.c
index daf35ec..2ec10db 100644
--- a/drivers/dma/skeleton/skeleton_dmadev.c
+++ b/drivers/dma/skeleton/skeleton_dmadev.c
@@ -5,6 +5,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 #include 
 #include 
@@ -53,7 +55,7 @@
return 0;
 }
 
-static void *
+static uint32_t
 cpucopy_thread(void *param)
 {
 #define SLEEP_THRESHOLD1
@@ -81,7 +83,7 @@
(void)rte_ring_enqueue(hw->desc_completed, (void *)desc);
}
 
-   return NULL;
+   return 0;
 }
 
 static void
@@ -126,7 +128,7 @@
rte_mb();
 
snprintf(name, sizeof(name), "dma_skel_%d", dev->data->dev_id);
-   ret = rte_ctrl_thread_create(&hw->thread, name, NULL,
+   ret = rte_thread_create_control(&hw->thread, name, NULL,
 cpucopy_thread, dev);
if (ret) {
SKELDMA_LOG(ERR, "Start cpucopy thread fail!");
@@ -135,8 +137,7 @@
 
if (hw->lcore_id != -1) {
cpuset = rte_lcore_cpuset(hw->lcore_id);
-   ret = pthread_setaffinity_np(hw->thread, sizeof(cpuset),
-&cpuset);
+   ret = rte_thread_get_affinity_by_id(hw->thread, &cpuset);
if (ret)
SKELDMA_LOG(WARNING,
"Set thread affinity lcore = %d fail!",
@@ -154,8 +155,8 @@
hw->exit_flag = true;
rte_delay_ms(1);
 
-   (void)pthread_cancel(hw->thread);
-   pthread_join(hw->thread, NULL);
+   (void)pthread_cancel((pthread_t)hw->thread.opaque_id);
+   rte_thread_join(hw->thread, NULL);
 
return 0;
 }
diff --git a/drivers/dma/skeleton/skeleton_dmadev.h 
b/drivers/dma/skeleton/skeleton_dmadev.h
index 6f89400..8670a68 100644
--- a/drivers/dma/skeleton/skeleton_dmadev.h
+++ b/drivers/dma/skeleton/skeleton_dmadev.h
@@ -5,9 +5,9 @@
 #ifndef SKELETON_DMADEV_H
 #define SKELETON_DMADEV_H
 
-#include 
 
 #include 
+#include 
 
 #define SKELDMA_ARG_LCORE  "lcore"
 
@@ -21,7 +21,7 @@ struct skeldma_desc {
 struct skeldma_hw {
int lcore_id; /* cpucopy task affinity core */
int socket_id;
-   pthread_t thread; /* cpucopy task thread */
+   rte_thread_t thread; /* cpucopy task thread */
volatile int exit_flag; /* cpucopy task exit flag */
 
struct skeldma_desc *desc_mem;
-- 
1.8.3.1

[PATCH 4/6] net/iavf: use rte thread API

[Tyler Retzlaff]

Update driver to use rte thread API where available instead of pthread
as a prerequisite to removing pthread stubs on Windows.

Signed-off-by: Tyler Retzlaff 
---
 drivers/net/iavf/iavf_vchnl.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 9adaadb..7a2be22 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -42,7 +42,7 @@ struct iavf_event_element {
 
 struct iavf_event_handler {
uint32_t ndev;
-   pthread_t tid;
+   rte_thread_t tid;
int fd[2];
pthread_mutex_t lock;
TAILQ_HEAD(event_list, iavf_event_element) pending;
@@ -59,7 +59,7 @@ struct iavf_event_handler {
(var) = (tvar))
 #endif
 
-static void *
+static uint32_t
 iavf_dev_event_handle(void *param __rte_unused)
 {
struct iavf_event_handler *handler = &event_handler;
@@ -84,7 +84,7 @@ struct iavf_event_handler {
}
}
 
-   return NULL;
+   return 0;
 }
 
 static void
@@ -135,7 +135,7 @@ struct iavf_event_handler {
TAILQ_INIT(&handler->pending);
pthread_mutex_init(&handler->lock, NULL);
 
-   if (rte_ctrl_thread_create(&handler->tid, "iavf-event-thread",
+   if (rte_thread_create_control(&handler->tid, "iavf-event-thread",
NULL, iavf_dev_event_handle, NULL)) {
__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED);
return -1;
@@ -152,14 +152,14 @@ struct iavf_event_handler {
if (__atomic_sub_fetch(&handler->ndev, 1, __ATOMIC_RELAXED) != 0)
return;
 
-   int unused = pthread_cancel(handler->tid);
+   int unused = pthread_cancel((pthread_t)handler->tid.opaque_id);
RTE_SET_USED(unused);
close(handler->fd[0]);
close(handler->fd[1]);
handler->fd[0] = -1;
handler->fd[1] = -1;
 
-   pthread_join(handler->tid, NULL);
+   rte_thread_join(handler->tid, NULL);
pthread_mutex_destroy(&handler->lock);
 
struct iavf_event_element *pos, *save_next;
-- 
1.8.3.1

[PATCH 5/6] eal: use rte thread API

[Tyler Retzlaff]

Update driver to use rte thread API where available instead of pthread
as a prerequisite to removing pthread stubs on Windows.

There is a single pthread_create still in use until
rte_ctrl_thread_create is removed.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/common/eal_common_thread.c |  4 ++--
 lib/eal/windows/eal.c  |  2 +-
 lib/eal/windows/eal_interrupts.c   | 12 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/eal/common/eal_common_thread.c 
b/lib/eal/common/eal_common_thread.c
index 079a385..e3aad2c 100644
--- a/lib/eal/common/eal_common_thread.c
+++ b/lib/eal/common/eal_common_thread.c
@@ -176,7 +176,7 @@ unsigned rte_socket_id(void)
 
ret = eal_thread_dump_current_affinity(cpuset, sizeof(cpuset));
RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
-   lcore_id, (uintptr_t)pthread_self(), cpuset,
+   lcore_id, rte_thread_self().opaque_id, cpuset,
ret == 0 ? "" : "...");
 
rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
@@ -330,7 +330,7 @@ static uint32_t control_thread_start(void *arg)
/* Check if the control thread encountered an error */
if (ctrl_thread_status == CTRL_THREAD_ERROR) {
/* ctrl thread is exiting */
-   pthread_join(*thread, NULL);
+   rte_thread_join((rte_thread_t){(uintptr_t)*thread}, NULL);
}
 
ret = params->ret;
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index e7d405b..89da0c0 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -416,7 +416,7 @@ enum rte_proc_type_t
 
ret = eal_thread_dump_current_affinity(cpuset, sizeof(cpuset));
RTE_LOG(DEBUG, EAL, "Main lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
-   config->main_lcore, (uintptr_t)pthread_self(), cpuset,
+   config->main_lcore, rte_thread_self().opaque_id, cpuset,
ret == 0 ? "" : "...");
 
RTE_LCORE_FOREACH_WORKER(i) {
diff --git a/lib/eal/windows/eal_interrupts.c b/lib/eal/windows/eal_interrupts.c
index bb0585c..49c4b96 100644
--- a/lib/eal/windows/eal_interrupts.c
+++ b/lib/eal/windows/eal_interrupts.c
@@ -9,7 +9,7 @@
 
 #define IOCP_KEY_SHUTDOWN UINT32_MAX
 
-static pthread_t intr_thread;
+static rte_thread_t intr_thread;
 
 static HANDLE intr_iocp;
 static HANDLE intr_thread_handle;
@@ -33,7 +33,7 @@
return 0;
 }
 
-static void *
+static uint32_t
 eal_intr_thread_main(LPVOID arg __rte_unused)
 {
bool finished = false;
@@ -78,12 +78,12 @@
intr_thread_handle = NULL;
 
 cleanup:
-   intr_thread = 0;
+   intr_thread.opaque_id = 0;
 
CloseHandle(intr_iocp);
intr_iocp = NULL;
 
-   return NULL;
+   return 0;
 }
 
 int
@@ -98,7 +98,7 @@
return -1;
}
 
-   ret = rte_ctrl_thread_create(&intr_thread, "eal-intr-thread", NULL,
+   ret = rte_thread_create_control(&intr_thread, "eal-intr-thread", NULL,
eal_intr_thread_main, NULL);
if (ret != 0) {
rte_errno = -ret;
@@ -111,7 +111,7 @@
 int
 rte_thread_is_intr(void)
 {
-   return pthread_equal(intr_thread, pthread_self());
+   return rte_thread_equal(intr_thread, rte_thread_self());
 }
 
 int
-- 
1.8.3.1

[PATCH 6/6] windows: remove most pthread lifetime shim functions

[Tyler Retzlaff]

Remove most of the pthread_xxx lifetime shim functions, only
pthread_create remains while we wait for rte_ctrl_thread_create removal.

Signed-off-by: Tyler Retzlaff 
---
 lib/eal/windows/include/pthread.h | 99 ---
 1 file changed, 99 deletions(-)

diff --git a/lib/eal/windows/include/pthread.h 
b/lib/eal/windows/include/pthread.h
index f7cf0e9..051b931 100644
--- a/lib/eal/windows/include/pthread.h
+++ b/lib/eal/windows/include/pthread.h
@@ -42,92 +42,6 @@
!DeleteSynchronizationBarrier(barrier)
 #define pthread_cancel(thread) !TerminateThread((HANDLE) thread, 0)
 
-/* pthread function overrides */
-#define pthread_self() \
-   ((pthread_t)GetCurrentThreadId())
-
-
-static inline int
-pthread_equal(pthread_t t1, pthread_t t2)
-{
-   return t1 == t2;
-}
-
-static inline int
-pthread_setaffinity_np(pthread_t threadid, size_t cpuset_size,
-   rte_cpuset_t *cpuset)
-{
-   DWORD_PTR ret = 0;
-   HANDLE thread_handle;
-
-   if (cpuset == NULL || cpuset_size == 0)
-   return -1;
-
-   thread_handle = OpenThread(THREAD_ALL_ACCESS, FALSE, threadid);
-   if (thread_handle == NULL) {
-   RTE_LOG_WIN32_ERR("OpenThread()");
-   return -1;
-   }
-
-   ret = SetThreadAffinityMask(thread_handle, *cpuset->_bits);
-   if (ret == 0) {
-   RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
-   goto close_handle;
-   }
-
-close_handle:
-   if (CloseHandle(thread_handle) == 0) {
-   RTE_LOG_WIN32_ERR("CloseHandle()");
-   return -1;
-   }
-   return (ret == 0) ? -1 : 0;
-}
-
-static inline int
-pthread_getaffinity_np(pthread_t threadid, size_t cpuset_size,
-   rte_cpuset_t *cpuset)
-{
-   /* Workaround for the lack of a GetThreadAffinityMask()
-*API in Windows
-*/
-   DWORD_PTR prev_affinity_mask;
-   HANDLE thread_handle;
-   DWORD_PTR ret = 0;
-
-   if (cpuset == NULL || cpuset_size == 0)
-   return -1;
-
-   thread_handle = OpenThread(THREAD_ALL_ACCESS, FALSE, threadid);
-   if (thread_handle == NULL) {
-   RTE_LOG_WIN32_ERR("OpenThread()");
-   return -1;
-   }
-
-   /* obtain previous mask by setting dummy mask */
-   prev_affinity_mask = SetThreadAffinityMask(thread_handle, 0x1);
-   if (prev_affinity_mask == 0) {
-   RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
-   goto close_handle;
-   }
-
-   /* set it back! */
-   ret = SetThreadAffinityMask(thread_handle, prev_affinity_mask);
-   if (ret == 0) {
-   RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
-   goto close_handle;
-   }
-
-   memset(cpuset, 0, cpuset_size);
-   *cpuset->_bits = prev_affinity_mask;
-
-close_handle:
-   if (CloseHandle(thread_handle) == 0) {
-   RTE_LOG_WIN32_ERR("SetThreadAffinityMask()");
-   return -1;
-   }
-   return (ret == 0) ? -1 : 0;
-}
-
 static inline int
 pthread_create(void *threadid, const void *threadattr, void *threadfunc,
void *args)
@@ -145,19 +59,6 @@
 }
 
 static inline int
-pthread_detach(__rte_unused pthread_t thread)
-{
-   return 0;
-}
-
-static inline int
-pthread_join(__rte_unused pthread_t thread,
-   __rte_unused void **value_ptr)
-{
-   return 0;
-}
-
-static inline int
 pthread_mutex_init(pthread_mutex_t *mutex,
   __rte_unused pthread_mutexattr_t *attr)
 {
-- 
1.8.3.1

[PATCH 1/2] net/mana: avoid unnecessary assignments in data path

[longli]

From: Long Li 

Unnecessary assignments involve memset and waste CPU cycles.
Removing them to reduce CPU usage.

Fixes: 517ed6e2d590 ("net/mana: add basic driver with build environment")
Cc: sta...@dpdk.org
Signed-off-by: Long Li 
---
 drivers/net/mana/gdma.c | 11 ++-
 drivers/net/mana/mana.h |  2 +-
 drivers/net/mana/rx.c   |  9 -
 drivers/net/mana/tx.c   | 17 ++---
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/net/mana/gdma.c b/drivers/net/mana/gdma.c
index 3d4039014f..0922463ef9 100644
--- a/drivers/net/mana/gdma.c
+++ b/drivers/net/mana/gdma.c
@@ -123,7 +123,7 @@ write_scatter_gather_list(uint8_t *work_queue_head_pointer,
 int
 gdma_post_work_request(struct mana_gdma_queue *queue,
   struct gdma_work_request *work_req,
-  struct gdma_posted_wqe_info *wqe_info)
+  uint32_t *wqe_size_in_bu)
 {
uint32_t client_oob_size =
work_req->inline_oob_size_in_bytes >
@@ -149,14 +149,7 @@ gdma_post_work_request(struct mana_gdma_queue *queue,
DRV_LOG(DEBUG, "client_oob_size %u sgl_data_size %u wqe_size %u",
client_oob_size, sgl_data_size, wqe_size);
 
-   if (wqe_info) {
-   wqe_info->wqe_index =
-   ((queue->head * GDMA_WQE_ALIGNMENT_UNIT_SIZE) &
-(queue->size - 1)) / GDMA_WQE_ALIGNMENT_UNIT_SIZE;
-   wqe_info->unmasked_queue_offset = queue->head;
-   wqe_info->wqe_size_in_bu =
-   wqe_size / GDMA_WQE_ALIGNMENT_UNIT_SIZE;
-   }
+   *wqe_size_in_bu = wqe_size / GDMA_WQE_ALIGNMENT_UNIT_SIZE;
 
wq_buffer_pointer = gdma_get_wqe_pointer(queue);
wq_buffer_pointer += write_dma_client_oob(wq_buffer_pointer, work_req,
diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h
index 4a05238a96..d4a1ba8492 100644
--- a/drivers/net/mana/mana.h
+++ b/drivers/net/mana/mana.h
@@ -459,7 +459,7 @@ int mana_rq_ring_doorbell(struct mana_rxq *rxq, uint8_t 
arm);
 
 int gdma_post_work_request(struct mana_gdma_queue *queue,
   struct gdma_work_request *work_req,
-  struct gdma_posted_wqe_info *wqe_info);
+  uint32_t *wqe_size_in_bu);
 uint8_t *gdma_get_wqe_pointer(struct mana_gdma_queue *queue);
 
 uint16_t mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **rx_pkts,
diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c
index 55247889c1..bdbd11c5f9 100644
--- a/drivers/net/mana/rx.c
+++ b/drivers/net/mana/rx.c
@@ -52,8 +52,8 @@ mana_alloc_and_post_rx_wqe(struct mana_rxq *rxq)
 {
struct rte_mbuf *mbuf = NULL;
struct gdma_sgl_element sgl[1];
-   struct gdma_work_request request = {0};
-   struct gdma_posted_wqe_info wqe_info = {0};
+   struct gdma_work_request request;
+   uint32_t wqe_size_in_bu;
struct mana_priv *priv = rxq->priv;
int ret;
struct mana_mr_cache *mr;
@@ -72,7 +72,6 @@ mana_alloc_and_post_rx_wqe(struct mana_rxq *rxq)
}
 
request.gdma_header.struct_size = sizeof(request);
-   wqe_info.gdma_header.struct_size = sizeof(wqe_info);
 
sgl[0].address = rte_cpu_to_le_64(rte_pktmbuf_mtod(mbuf, uint64_t));
sgl[0].memory_key = mr->lkey;
@@ -87,14 +86,14 @@ mana_alloc_and_post_rx_wqe(struct mana_rxq *rxq)
request.flags = 0;
request.client_data_unit = NOT_USING_CLIENT_DATA_UNIT;
 
-   ret = gdma_post_work_request(&rxq->gdma_rq, &request, &wqe_info);
+   ret = gdma_post_work_request(&rxq->gdma_rq, &request, &wqe_size_in_bu);
if (!ret) {
struct mana_rxq_desc *desc =
&rxq->desc_ring[rxq->desc_ring_head];
 
/* update queue for tracking pending packets */
desc->pkt = mbuf;
-   desc->wqe_size_in_bu = wqe_info.wqe_size_in_bu;
+   desc->wqe_size_in_bu = wqe_size_in_bu;
rxq->desc_ring_head = (rxq->desc_ring_head + 1) % rxq->num_desc;
} else {
DRV_LOG(ERR, "failed to post recv ret %d", ret);
diff --git a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c
index 300bf27cc1..a7ee47c582 100644
--- a/drivers/net/mana/tx.c
+++ b/drivers/net/mana/tx.c
@@ -208,8 +208,8 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
for (uint16_t pkt_idx = 0; pkt_idx < nb_pkts; pkt_idx++) {
struct rte_mbuf *m_pkt = tx_pkts[pkt_idx];
struct rte_mbuf *m_seg = m_pkt;
-   struct transmit_oob_v2 tx_oob = {0};
-   struct one_sgl sgl = {0};
+   struct transmit_oob_v2 tx_oob;
+   struct one_sgl sgl;
uint16_t seg_idx;
 
/* Drop the packet if it exceeds max segments */
@@ -263,6 +263,8 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, 
uint16_t nb_pkts)
tx_oob.short_oob.tx_compute_TCP_ch

[PATCH 2/2] net/mana: optimize completion queue polling by processing a batch at a time

[longli]

From: Long Li 

We can poll completion queues in a batch to speed up completion processing.
Also, the completion data doesn't need to be copied out of the hardware
queue and they can be passed as pointers to be consumed by the RX/TX code.

Fixes: 517ed6e2d590 ("net/mana: add basic driver with build environment")
Cc: sta...@dpdk.org
Signed-off-by: Long Li 
---
 drivers/net/mana/gdma.c | 62 ++---
 drivers/net/mana/mana.c | 22 +++
 drivers/net/mana/mana.h | 25 +++--
 drivers/net/mana/rx.c   | 21 +-
 drivers/net/mana/tx.c   | 11 +---
 5 files changed, 80 insertions(+), 61 deletions(-)

diff --git a/drivers/net/mana/gdma.c b/drivers/net/mana/gdma.c
index 0922463ef9..db1571a5c8 100644
--- a/drivers/net/mana/gdma.c
+++ b/drivers/net/mana/gdma.c
@@ -252,45 +252,51 @@ mana_ring_doorbell(void *db_page, enum gdma_queue_types 
queue_type,
 /*
  * Poll completion queue for completions.
  */
-int
-gdma_poll_completion_queue(struct mana_gdma_queue *cq, struct gdma_comp *comp)
+uint32_t
+gdma_poll_completion_queue(struct mana_gdma_queue *cq,
+  struct gdma_comp *gdma_comp, uint32_t max_comp)
 {
struct gdma_hardware_completion_entry *cqe;
-   uint32_t head = cq->head % cq->count;
uint32_t new_owner_bits, old_owner_bits;
uint32_t cqe_owner_bits;
+   uint32_t num_comp = 0;
struct gdma_hardware_completion_entry *buffer = cq->buffer;
 
-   cqe = &buffer[head];
-   new_owner_bits = (cq->head / cq->count) & COMPLETION_QUEUE_OWNER_MASK;
-   old_owner_bits = (cq->head / cq->count - 1) &
-   COMPLETION_QUEUE_OWNER_MASK;
-   cqe_owner_bits = cqe->owner_bits;
+   while (num_comp < max_comp) {
+   cqe = &buffer[cq->head % cq->count];
+   new_owner_bits = (cq->head / cq->count) &
+   COMPLETION_QUEUE_OWNER_MASK;
+   old_owner_bits = (cq->head / cq->count - 1) &
+   COMPLETION_QUEUE_OWNER_MASK;
+   cqe_owner_bits = cqe->owner_bits;
+
+   DRV_LOG(DEBUG, "comp cqe bits 0x%x owner bits 0x%x",
+   cqe_owner_bits, old_owner_bits);
+
+   /* No new entry */
+   if (cqe_owner_bits == old_owner_bits)
+   break;
+
+   if (cqe_owner_bits != new_owner_bits) {
+   DRV_LOG(ERR, "CQ overflowed, ID %u cqe 0x%x new 0x%x",
+   cq->id, cqe_owner_bits, new_owner_bits);
+   break;
+   }
 
-   DRV_LOG(DEBUG, "comp cqe bits 0x%x owner bits 0x%x",
-   cqe_owner_bits, old_owner_bits);
+   gdma_comp[num_comp].cqe_data = cqe->dma_client_data;
+   num_comp++;
 
-   if (cqe_owner_bits == old_owner_bits)
-   return 0; /* No new entry */
+   cq->head++;
 
-   if (cqe_owner_bits != new_owner_bits) {
-   DRV_LOG(ERR, "CQ overflowed, ID %u cqe 0x%x new 0x%x",
-   cq->id, cqe_owner_bits, new_owner_bits);
-   return -1;
+   DRV_LOG(DEBUG, "comp new 0x%x old 0x%x cqe 0x%x wq %u sq %u 
head %u",
+   new_owner_bits, old_owner_bits, cqe_owner_bits,
+   cqe->wq_num, cqe->is_sq, cq->head);
}
 
-   /* Ensure checking owner bits happens before reading from CQE */
+   /* Make sure the CQE owner bits are checked before we access the data
+* in CQE
+*/
rte_rmb();
 
-   comp->work_queue_number = cqe->wq_num;
-   comp->send_work_queue = cqe->is_sq;
-
-   memcpy(comp->completion_data, cqe->dma_client_data, 
GDMA_COMP_DATA_SIZE);
-
-   cq->head++;
-
-   DRV_LOG(DEBUG, "comp new 0x%x old 0x%x cqe 0x%x wq %u sq %u head %u",
-   new_owner_bits, old_owner_bits, cqe_owner_bits,
-   comp->work_queue_number, comp->send_work_queue, cq->head);
-   return 1;
+   return num_comp;
 }
diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 8a782c0d63..2463f34c1e 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -487,6 +487,15 @@ mana_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t 
queue_idx,
goto fail;
}
 
+   txq->gdma_comp_buf = rte_malloc_socket("mana_txq_comp",
+   sizeof(*txq->gdma_comp_buf) * nb_desc,
+   RTE_CACHE_LINE_SIZE, socket_id);
+   if (!txq->gdma_comp_buf) {
+   DRV_LOG(ERR, "failed to allocate txq comp");
+   ret = -ENOMEM;
+   goto fail;
+   }
+
ret = mana_mr_btree_init(&txq->mr_btree,
 MANA_MR_BTREE_PER_QUEUE_N, socket_id);
if (ret) {
@@ -506,6 +515,7 @@ mana_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t 
queue_idx,
return 0;
 
 fail:
+   rte_fr