RE: [RFC 7/7] raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE

RE: [RFC 7/7] raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE

[Xu, Rosen]

Hi,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, January 28, 2025 2:04 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Xu, Rosen
> <rosen.xu@intel.com>
> Subject: [RFC 7/7] raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE
> 
> Prefer the EAL version over local version of macro.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/raw/ifpga/base/opae_osdep.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/raw/ifpga/base/opae_osdep.h
> b/drivers/raw/ifpga/base/opae_osdep.h
> index e35a21c80e..b483d00a54 100644
> --- a/drivers/raw/ifpga/base/opae_osdep.h
> +++ b/drivers/raw/ifpga/base/opae_osdep.h
> @@ -11,6 +11,7 @@
> 
>  #ifdef RTE_LIB_EAL
>  #include "osdep_rte/osdep_generic.h"
> +#include <rte_queue.h>
>  #else
>  #include "osdep_raw/osdep_generic.h"
>  #endif
> --
> 2.45.2
Reviewed-by: Rosen Xu <rosen.xu@intel.com>

[PATCH v2 0/7] Introduce FOREACH_SAFE macros

[Stephen Hemminger]

This series adds common macros for safe iteration over lists.
It is a subset copy of the macros from FreeBSD that are
missing from the Linux header sys/queue.h

Chose this over several other options:
  - let each driver define their own as needed.
    One Intel driver got it wrong, others will as well.
  - rename all the queue macros to RTE_XXX variants.
    Seems like useless renaming and confusion.
  - Several distros have libbsd package with the correct macros.
    But adding yet another dependency to DPDK would be annoying
    for something this basic.

There are more macros in FreeBSD header that could be useful,
but we can add those later as needed here.

v2 - rebase after Intel PMD reorg

Stephen Hemminger (7):
  eal: add queue macro extensions from FreeBSD
  net/qede: fix use after free
  bus/fslmc: fix use after free
  net/bnxt: fix use after free
  net/iavf: replace local version of TAILQ_FOREACH_SAFE
  vhost: replace open coded TAILQ_FOREACH_SAFE
  raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE

 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |   5 +-
 drivers/net/bnxt/bnxt_filter.c           |   8 +-
 drivers/net/intel/iavf/iavf_vchnl.c      |   8 +-
 drivers/net/qede/qede_ethdev.h           |   3 +-
 drivers/net/qede/qede_filter.c           |  13 +-
 drivers/raw/ifpga/base/opae_osdep.h      |   1 +
 lib/eal/include/meson.build              |   3 +-
 lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
 lib/vhost/socket.c                       |  11 +-
 9 files changed, 193 insertions(+), 33 deletions(-)
 create mode 100644 lib/eal/include/rte_queue.h

-- 
2.47.2

[PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Stephen Hemminger]

The Linux version of sys/queue.h is frozen at an older version
and is missing the _SAFE macro variants. Several drivers started
introducing the own workarounds for this. Should be handled in EAL.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/meson.build |   3 +-
 lib/eal/include/rte_queue.h | 174 ++++++++++++++++++++++++++++++++++++
 2 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 lib/eal/include/rte_queue.h

diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index d903577caa..75bf09381f 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -19,8 +19,8 @@ headers += files(
         'rte_eal.h',
         'rte_eal_memconfig.h',
         'rte_eal_trace.h',
-        'rte_errno.h',
         'rte_epoll.h',
+        'rte_errno.h',
         'rte_fbarray.h',
         'rte_hexdump.h',
         'rte_hypervisor.h',
@@ -38,6 +38,7 @@ headers += files(
         'rte_pci_dev_features.h',
         'rte_per_lcore.h',
         'rte_pflock.h',
+        'rte_queue.h',
         'rte_random.h',
         'rte_reciprocal.h',
         'rte_seqcount.h',
diff --git a/lib/eal/include/rte_queue.h b/lib/eal/include/rte_queue.h
new file mode 100644
index 0000000000..7ec807b15d
--- /dev/null
+++ b/lib/eal/include/rte_queue.h
@@ -0,0 +1,174 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ *
+ * Original Copyright (c) 1991, 1993
+ *	The Regents of the University of California.  All rights reserved.
+ */
+
+#ifndef _RTE_QUEUE_H_
+#define _RTE_QUEUE_H_
+
+/**
+ * @file
+ *  Defines macro's that exist in the FreeBSD version of queue.h
+ *  which are missing in other versions.
+ */
+
+#include <sys/queue.h>
+
+/*
+ * This file defines four types of data structures: singly-linked lists,
+ * singly-linked tail queues, lists and tail queues.
+ *
+ * Below is a summary of implemented functions where:
+ *  o  means the macro exists in original version
+ *  +  means the macro is added here
+ *  -  means the macro is not available
+ *  s  means the macro is available but is slow (runs in O(n) time)
+ *
+ *				SLIST	LIST	STAILQ	TAILQ
+ * _HEAD			o	o	o	o
+ * _HEAD_INITIALIZER		o	o	o	o
+ * _ENTRY			o	o	o	o
+ * _INIT			o	o	o	o
+ * _EMPTY			o	o	o	o
+ * _FIRST			o	o	o	o
+ * _NEXT			o	o	o	o
+ * _FOREACH			o	o	o	o
+ * _FOREACH_FROM		+	+	+	+
+ * _FOREACH_SAFE		+	+	+	+
+ * _FOREACH_FROM_SAFE		+	+	+	+
+ * _FOREACH_REVERSE		-	-	-	o
+ * _FOREACH_REVERSE_FROM	-	-	-	+
+ * _FOREACH_REVERSE_SAFE	-	-	-       +
+ * _FOREACH_REVERSE_FROM_SAFE	-	-	-	+
+ * _INSERT_HEAD			o	o	o	o
+ * _INSERT_BEFORE		-	o	-	o
+ * _INSERT_AFTER		o	o	o	o
+ * _INSERT_TAIL			-	-	o	o
+ * _CONCAT			s	s	o	o
+ * _REMOVE_AFTER		o	-	o	-
+ * _REMOVE_HEAD			o	o	o	o
+ * _REMOVE			s	o	s	o
+ *
+ */
+
+
+/*
+ * Singly-linked List declarations.
+ */
+#ifndef SLIST_FOREACH_FROM
+#define	SLIST_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : SLIST_FIRST((head)));		\
+	    (var);							\
+	    (var) = SLIST_NEXT((var), field))
+#endif
+
+#ifndef SLIST_FOREACH_SAFE
+#define	SLIST_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = SLIST_FIRST((head));				\
+	    (var) && ((tvar) = SLIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef SLIST_FOREACH_FROM_SAFE
+#define	SLIST_FOREACH_FROM_SAFE(var, head, field, tvar)			\
+	for ((var) = ((var) ? (var) : SLIST_FIRST((head)));		\
+	    (var) && ((tvar) = SLIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+
+/*
+ * Singly-linked Tail queue declarations.
+ */
+#ifndef STAILQ_FOREACH_FROM
+#define	STAILQ_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : STAILQ_FIRST((head)));		\
+	   (var);							\
+	   (var) = STAILQ_NEXT((var), field))
+#endif
+
+#ifndef STAILQ_FOREACH_SAFE
+#define	STAILQ_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = STAILQ_FIRST((head));				\
+	    (var) && ((tvar) = STAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef STAILQ_FOREACH_FROM_SAFE
+#define	STAILQ_FOREACH_FROM_SAFE(var, head, field, tvar)		\
+	for ((var) = ((var) ? (var) : STAILQ_FIRST((head)));		\
+	    (var) && ((tvar) = STAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+/*
+ * List declarations.
+ */
+#ifndef LIST_FOREACH_FROM
+#define	LIST_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : LIST_FIRST((head)));		\
+	    (var);							\
+	    (var) = LIST_NEXT((var), field))
+#endif
+
+#ifndef LIST_FOREACH_SAFE
+#define	LIST_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = LIST_FIRST((head));				\
+	    (var) && ((tvar) = LIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef LIST_FOREACH_FROM_SAFE
+#define	LIST_FOREACH_FROM_SAFE(var, head, field, tvar)			\
+	for ((var) = ((var) ? (var) : LIST_FIRST((head)));		\
+	    (var) && ((tvar) = LIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+/*
+ * Tail queue declarations.
+ */
+#ifndef TAILQ_FOREACH_FROM
+#define TAILQ_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : TAILQ_FIRST((head)));		\
+	    (var);							\
+	    (var) = TAILQ_NEXT((var), field))
+#endif
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = TAILQ_FIRST((head));				\
+	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef TAILQ_FOREACH_FROM_SAFE
+#define TAILQ_FOREACH_FROM_SAFE(var, head, field, tvar)			\
+	for ((var) = ((var) ? (var) : TAILQ_FIRST((head)));		\
+	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef TAILQ_FOREACH_REVERSE_FROM
+#define TAILQ_FOREACH_REVERSE_FROM(var, head, headname, field)		\
+	for ((var) = ((var) ? (var) : TAILQ_LAST((head), headname));	\
+	    (var);							\
+	    (var) = TAILQ_PREV((var), headname, field))
+#endif
+
+#ifndef TAILQ_FOREACH_REVERSE_SAFE
+#define TAILQ_FOREACH_REVERSE_SAFE(var, head, headname, field, tvar)	\
+	for ((var) = TAILQ_LAST((head), headname);			\
+	    (var) && ((tvar) = TAILQ_PREV((var), headname, field), 1);	\
+	    (var) = (tvar))
+#endif
+
+#ifndef TAILQ_FOREACH_REVERSE_FROM_SAFE
+#define TAILQ_FOREACH_REVERSE_FROM_SAFE(var, head, headname, field, tvar)\
+	for ((var) = ((var) ? (var) : TAILQ_LAST((head), headname));	\
+	    (var) && ((tvar) = TAILQ_PREV((var), headname, field), 1);	\
+	    (var) = (tvar))
+#endif
+
+#endif /* !_RTE_QUEUE_H_ */
-- 
2.47.2

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Thomas Monjalon]

14/02/2025 18:20, Stephen Hemminger:
> The Linux version of sys/queue.h is frozen at an older version
> and is missing the _SAFE macro variants. Several drivers started
> introducing the own workarounds for this. Should be handled in EAL.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

We may want to unify with lib/eal/windows/include/sys/queue.h

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Stephen Hemminger]

On Wed, 19 Feb 2025 15:55:17 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 14/02/2025 18:20, Stephen Hemminger:
> > The Linux version of sys/queue.h is frozen at an older version
> > and is missing the _SAFE macro variants. Several drivers started
> > introducing the own workarounds for this. Should be handled in EAL.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> We may want to unify with lib/eal/windows/include/sys/queue.h

Do we want to just not use the local Linux version of queue.h entirely?
Maybe just put in current FreeBSD one? It has lots more useful variations.

The point is to avoid broken versions (like one Intel driver) or
having same macro in multiple places.

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Thomas Monjalon]

22/02/2025 00:05, Stephen Hemminger:
> On Wed, 19 Feb 2025 15:55:17 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 14/02/2025 18:20, Stephen Hemminger:
> > > The Linux version of sys/queue.h is frozen at an older version
> > > and is missing the _SAFE macro variants. Several drivers started
> > > introducing the own workarounds for this. Should be handled in EAL.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> > 
> > We may want to unify with lib/eal/windows/include/sys/queue.h
> 
> Do we want to just not use the local Linux version of queue.h entirely?
> Maybe just put in current FreeBSD one? It has lots more useful variations.

Yes but it should be global in EAL, no need to duplicate it for EAL Windows and others.

> The point is to avoid broken versions (like one Intel driver) or
> having same macro in multiple places.

Got it.

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Stephen Hemminger]

On Wed, 19 Feb 2025 15:55:17 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 14/02/2025 18:20, Stephen Hemminger:
> > The Linux version of sys/queue.h is frozen at an older version
> > and is missing the _SAFE macro variants. Several drivers started
> > introducing the own workarounds for this. Should be handled in EAL.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> We may want to unify with lib/eal/windows/include/sys/queue.h
> 
> 

Not sure, we have several options here:
  1. Keep using Linux sys/queue.h and add enhancements in rte_queue.h
  2. Make rte_queue.h a copy of FreeBSD version of queue.h (that is what Windows did)
  3. Use the bsd version of queue.h.
     On Debian/Ubuntu this in libbsd-dev package and referred to as bsd/sys/queue.h

I chose #1 as simplest. But all of these could work. #3 means DPDK has least new
code, but adds another dependency to the build.

No matter what, should add to checkpatch to block any new files that include sys/queue.h
directly.

RE: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 13 March 2025 00.02
> 
> On Wed, 19 Feb 2025 15:55:17 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 14/02/2025 18:20, Stephen Hemminger:
> > > The Linux version of sys/queue.h is frozen at an older version
> > > and is missing the _SAFE macro variants. Several drivers started
> > > introducing the own workarounds for this. Should be handled in EAL.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> > We may want to unify with lib/eal/windows/include/sys/queue.h
> >
> >
> 
> Not sure, we have several options here:
>   1. Keep using Linux sys/queue.h and add enhancements in rte_queue.h
>   2. Make rte_queue.h a copy of FreeBSD version of queue.h (that is
> what Windows did)
>   3. Use the bsd version of queue.h.
>      On Debian/Ubuntu this in libbsd-dev package and referred to as
> bsd/sys/queue.h
> 
> I chose #1 as simplest. But all of these could work. #3 means DPDK has
> least new
> code, but adds another dependency to the build.

+1 to #2:
If Linux sys/queue.h is frozen, and is a subset of FreeBSD queue.h, #2 would consolidate all the queue macros in one file.
That seems easier to navigate (for a developer/reviewer looking at the macro definitions/implementations) than including sys/queue.h, queue.h or lib/eal/windows/include/sys/queue.h depending on O/S, and then defining the missing macros in rte_queue.h.
A comment at the top of the rte_queue.h file could mention that it is a copy of the FreeBSD queue.h file.

-1 to #3; it adds an unnecessary dependency.

> 
> No matter what, should add to checkpatch to block any new files that
> include sys/queue.h
> directly.

+1 to this.

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Bruce Richardson]

On Thu, Mar 13, 2025 at 09:00:09AM +0100, Morten Brørup wrote:
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Thursday, 13 March 2025 00.02
> > 
> > On Wed, 19 Feb 2025 15:55:17 +0100
> > Thomas Monjalon <thomas@monjalon.net> wrote:
> > 
> > > 14/02/2025 18:20, Stephen Hemminger:
> > > > The Linux version of sys/queue.h is frozen at an older version
> > > > and is missing the _SAFE macro variants. Several drivers started
> > > > introducing the own workarounds for this. Should be handled in EAL.
> > > >
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > >
> > > We may want to unify with lib/eal/windows/include/sys/queue.h
> > >
> > >
> > 
> > Not sure, we have several options here:
> >   1. Keep using Linux sys/queue.h and add enhancements in rte_queue.h
> >   2. Make rte_queue.h a copy of FreeBSD version of queue.h (that is
> > what Windows did)
> >   3. Use the bsd version of queue.h.
> >      On Debian/Ubuntu this in libbsd-dev package and referred to as
> > bsd/sys/queue.h
> > 
> > I chose #1 as simplest. But all of these could work. #3 means DPDK has
> > least new
> > code, but adds another dependency to the build.
> 
> +1 to #2:
> If Linux sys/queue.h is frozen, and is a subset of FreeBSD queue.h, #2 would consolidate all the queue macros in one file.
> That seems easier to navigate (for a developer/reviewer looking at the macro definitions/implementations) than including sys/queue.h, queue.h or lib/eal/windows/include/sys/queue.h depending on O/S, and then defining the missing macros in rte_queue.h.
> A comment at the top of the rte_queue.h file could mention that it is a copy of the FreeBSD queue.h file.
> 
> -1 to #3; it adds an unnecessary dependency.
> 
> > 
> > No matter what, should add to checkpatch to block any new files that
> > include sys/queue.h
> > directly.
> 
> +1 to this.
> 

I agree with Morten, #2 looks the best option to me too.
/Bruce

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Thomas Monjalon]

13/03/2025 09:56, Bruce Richardson:
> On Thu, Mar 13, 2025 at 09:00:09AM +0100, Morten Brørup wrote:
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Thursday, 13 March 2025 00.02
> > > 
> > > On Wed, 19 Feb 2025 15:55:17 +0100
> > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > 
> > > > 14/02/2025 18:20, Stephen Hemminger:
> > > > > The Linux version of sys/queue.h is frozen at an older version
> > > > > and is missing the _SAFE macro variants. Several drivers started
> > > > > introducing the own workarounds for this. Should be handled in EAL.
> > > > >
> > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > >
> > > > We may want to unify with lib/eal/windows/include/sys/queue.h
> > > >
> > > >
> > > 
> > > Not sure, we have several options here:
> > >   1. Keep using Linux sys/queue.h and add enhancements in rte_queue.h
> > >   2. Make rte_queue.h a copy of FreeBSD version of queue.h (that is
> > > what Windows did)
> > >   3. Use the bsd version of queue.h.
> > >      On Debian/Ubuntu this in libbsd-dev package and referred to as
> > > bsd/sys/queue.h
> > > 
> > > I chose #1 as simplest. But all of these could work. #3 means DPDK has
> > > least new
> > > code, but adds another dependency to the build.
> > 
> > +1 to #2:
> > If Linux sys/queue.h is frozen, and is a subset of FreeBSD queue.h, #2 would consolidate all the queue macros in one file.
> > That seems easier to navigate (for a developer/reviewer looking at the macro definitions/implementations) than including sys/queue.h, queue.h or lib/eal/windows/include/sys/queue.h depending on O/S, and then defining the missing macros in rte_queue.h.
> > A comment at the top of the rte_queue.h file could mention that it is a copy of the FreeBSD queue.h file.
> > 
> > -1 to #3; it adds an unnecessary dependency.
> > 
> > > 
> > > No matter what, should add to checkpatch to block any new files that
> > > include sys/queue.h
> > > directly.
> > 
> > +1 to this.
> > 
> 
> I agree with Morten, #2 looks the best option to me too.

It means we want to remove lib/eal/windows/include/sys/queue.h, right?

RE: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> 
> 13/03/2025 09:56, Bruce Richardson:
> > On Thu, Mar 13, 2025 at 09:00:09AM +0100, Morten Brørup wrote:
> > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > > Sent: Thursday, 13 March 2025 00.02
> > > >
> > > > On Wed, 19 Feb 2025 15:55:17 +0100
> > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > >
> > > > > 14/02/2025 18:20, Stephen Hemminger:
> > > > > > The Linux version of sys/queue.h is frozen at an older
> version
> > > > > > and is missing the _SAFE macro variants. Several drivers
> started
> > > > > > introducing the own workarounds for this. Should be handled
> in EAL.
> > > > > >
> > > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > >
> > > > > We may want to unify with lib/eal/windows/include/sys/queue.h
> > > > >
> > > > >
> > > >
> > > > Not sure, we have several options here:
> > > >   1. Keep using Linux sys/queue.h and add enhancements in
> rte_queue.h
> > > >   2. Make rte_queue.h a copy of FreeBSD version of queue.h (that
> is
> > > > what Windows did)
> > > >   3. Use the bsd version of queue.h.
> > > >      On Debian/Ubuntu this in libbsd-dev package and referred to
> as
> > > > bsd/sys/queue.h
> > > >
> > > > I chose #1 as simplest. But all of these could work. #3 means
> DPDK has
> > > > least new
> > > > code, but adds another dependency to the build.
> > >
> > > +1 to #2:
> > > If Linux sys/queue.h is frozen, and is a subset of FreeBSD queue.h,
> #2 would consolidate all the queue macros in one file.
> > > That seems easier to navigate (for a developer/reviewer looking at
> the macro definitions/implementations) than including sys/queue.h,
> queue.h or lib/eal/windows/include/sys/queue.h depending on O/S, and
> then defining the missing macros in rte_queue.h.
> > > A comment at the top of the rte_queue.h file could mention that it
> is a copy of the FreeBSD queue.h file.
> > >
> > > -1 to #3; it adds an unnecessary dependency.
> > >
> > > >
> > > > No matter what, should add to checkpatch to block any new files
> that
> > > > include sys/queue.h
> > > > directly.
> > >
> > > +1 to this.
> > >
> >
> > I agree with Morten, #2 looks the best option to me too.
> 
> It means we want to remove lib/eal/windows/include/sys/queue.h, right?

Correct.
All queue macros are consolidated into rte_queue.h.

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Stephen Hemminger]

On Thu, 13 Mar 2025 15:41:33 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 
> > 13/03/2025 09:56, Bruce Richardson:  
> > > On Thu, Mar 13, 2025 at 09:00:09AM +0100, Morten Brørup wrote:  
> > > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > > > Sent: Thursday, 13 March 2025 00.02
> > > > >
> > > > > On Wed, 19 Feb 2025 15:55:17 +0100
> > > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > >  
> > > > > > 14/02/2025 18:20, Stephen Hemminger:  
> > > > > > > The Linux version of sys/queue.h is frozen at an older  
> > version  
> > > > > > > and is missing the _SAFE macro variants. Several drivers  
> > started  
> > > > > > > introducing the own workarounds for this. Should be handled  
> > in EAL.  
> > > > > > >
> > > > > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> > > > > >
> > > > > > We may want to unify with lib/eal/windows/include/sys/queue.h
> > > > > >
> > > > > >  
> > > > >
> > > > > Not sure, we have several options here:
> > > > >   1. Keep using Linux sys/queue.h and add enhancements in  
> > rte_queue.h  
> > > > >   2. Make rte_queue.h a copy of FreeBSD version of queue.h (that  
> > is  
> > > > > what Windows did)
> > > > >   3. Use the bsd version of queue.h.
> > > > >      On Debian/Ubuntu this in libbsd-dev package and referred to  
> > as  
> > > > > bsd/sys/queue.h
> > > > >
> > > > > I chose #1 as simplest. But all of these could work. #3 means  
> > DPDK has  
> > > > > least new
> > > > > code, but adds another dependency to the build.  
> > > >
> > > > +1 to #2:
> > > > If Linux sys/queue.h is frozen, and is a subset of FreeBSD queue.h,  
> > #2 would consolidate all the queue macros in one file.  
> > > > That seems easier to navigate (for a developer/reviewer looking at  
> > the macro definitions/implementations) than including sys/queue.h,
> > queue.h or lib/eal/windows/include/sys/queue.h depending on O/S, and
> > then defining the missing macros in rte_queue.h.  
> > > > A comment at the top of the rte_queue.h file could mention that it  
> > is a copy of the FreeBSD queue.h file.  
> > > >
> > > > -1 to #3; it adds an unnecessary dependency.
> > > >  
> > > > >
> > > > > No matter what, should add to checkpatch to block any new files  
> > that  
> > > > > include sys/queue.h
> > > > > directly.  
> > > >
> > > > +1 to this.
> > > >  
> > >
> > > I agree with Morten, #2 looks the best option to me too.  
> > 
> > It means we want to remove lib/eal/windows/include/sys/queue.h, right?  
> 
> Correct.
> All queue macros are consolidated into rte_queue.h.
> 

I have been trying this, and it possible but there is some fallout.
If application includes <sys/queue.h> and other DPDK headers include <rte_queue.h>
there are declaration conflicts. Ends up being a compilation failure.
Tedious, but not hard to fix all of DPDK to use rte_queue.h instead of sys/queue.h
but what about user applications?

Since that would be API break, best to put it off to 25.11.

RE: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 13 March 2025 17.25
> 
> On Thu, 13 Mar 2025 15:41:33 +0100
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > >
> > > 13/03/2025 09:56, Bruce Richardson:
> > > > On Thu, Mar 13, 2025 at 09:00:09AM +0100, Morten Brørup wrote:
> > > > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > > > > Sent: Thursday, 13 March 2025 00.02
> > > > > >
> > > > > > On Wed, 19 Feb 2025 15:55:17 +0100
> > > > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > >
> > > > > > > 14/02/2025 18:20, Stephen Hemminger:
> > > > > > > > The Linux version of sys/queue.h is frozen at an older
> > > version
> > > > > > > > and is missing the _SAFE macro variants. Several drivers
> > > started
> > > > > > > > introducing the own workarounds for this. Should be
> handled
> > > in EAL.
> > > > > > > >
> > > > > > > > Signed-off-by: Stephen Hemminger
> <stephen@networkplumber.org>
> > > > > > >
> > > > > > > We may want to unify with
> lib/eal/windows/include/sys/queue.h
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > Not sure, we have several options here:
> > > > > >   1. Keep using Linux sys/queue.h and add enhancements in
> > > rte_queue.h
> > > > > >   2. Make rte_queue.h a copy of FreeBSD version of queue.h
> (that
> > > is
> > > > > > what Windows did)
> > > > > >   3. Use the bsd version of queue.h.
> > > > > >      On Debian/Ubuntu this in libbsd-dev package and referred
> to
> > > as
> > > > > > bsd/sys/queue.h
> > > > > >
> > > > > > I chose #1 as simplest. But all of these could work. #3 means
> > > DPDK has
> > > > > > least new
> > > > > > code, but adds another dependency to the build.
> > > > >
> > > > > +1 to #2:
> > > > > If Linux sys/queue.h is frozen, and is a subset of FreeBSD
> queue.h,
> > > #2 would consolidate all the queue macros in one file.
> > > > > That seems easier to navigate (for a developer/reviewer looking
> at
> > > the macro definitions/implementations) than including sys/queue.h,
> > > queue.h or lib/eal/windows/include/sys/queue.h depending on O/S,
> and
> > > then defining the missing macros in rte_queue.h.
> > > > > A comment at the top of the rte_queue.h file could mention that
> it
> > > is a copy of the FreeBSD queue.h file.
> > > > >
> > > > > -1 to #3; it adds an unnecessary dependency.
> > > > >
> > > > > >
> > > > > > No matter what, should add to checkpatch to block any new
> files
> > > that
> > > > > > include sys/queue.h
> > > > > > directly.
> > > > >
> > > > > +1 to this.
> > > > >
> > > >
> > > > I agree with Morten, #2 looks the best option to me too.
> > >
> > > It means we want to remove lib/eal/windows/include/sys/queue.h,
> right?
> >
> > Correct.
> > All queue macros are consolidated into rte_queue.h.
> >
> 
> I have been trying this, and it possible but there is some fallout.
> If application includes <sys/queue.h> and other DPDK headers include
> <rte_queue.h>
> there are declaration conflicts. Ends up being a compilation failure.
> Tedious, but not hard to fix all of DPDK to use rte_queue.h instead of
> sys/queue.h
> but what about user applications?

Perhaps this could work:

If _SYS_QUEUE_H_ is defined in rte_queue.h, including <sys/queue.h> after <rte_queue.h> will be harmless.

If each macro in rte_queue.h is surrounded by #ifndef MACRONAME, including <rte_queue.h> after <sys/queue.h> will be harmless.
Alternatively, surround a group of macros in rte_queue.h by #ifndef _SYS_QUEUE_H_.

> 
> Since that would be API break, best to put it off to 25.11.

It might be impossible prohibiting user libraries from including <sys/queue.h>, especially if some 3rd party library includes <sys/queue.h> in the library's header file. The application developer would not want to modify the header files of 3rd party libraries.

Re: [PATCH v2 1/7] eal: add queue macro extensions from FreeBSD

[Stephen Hemminger]

On Thu, 13 Mar 2025 17:40:26 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Thursday, 13 March 2025 17.25
> > 
> > On Thu, 13 Mar 2025 15:41:33 +0100
> > Morten Brørup <mb@smartsharesystems.com> wrote:
> >   
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > >
> > > > 13/03/2025 09:56, Bruce Richardson:  
> > > > > On Thu, Mar 13, 2025 at 09:00:09AM +0100, Morten Brørup wrote:  
> > > > > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > > > > > Sent: Thursday, 13 March 2025 00.02
> > > > > > >
> > > > > > > On Wed, 19 Feb 2025 15:55:17 +0100
> > > > > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > > >  
> > > > > > > > 14/02/2025 18:20, Stephen Hemminger:  
> > > > > > > > > The Linux version of sys/queue.h is frozen at an older  
> > > > version  
> > > > > > > > > and is missing the _SAFE macro variants. Several drivers  
> > > > started  
> > > > > > > > > introducing the own workarounds for this. Should be  
> > handled  
> > > > in EAL.  
> > > > > > > > >
> > > > > > > > > Signed-off-by: Stephen Hemminger  
> > <stephen@networkplumber.org>  
> > > > > > > >
> > > > > > > > We may want to unify with  
> > lib/eal/windows/include/sys/queue.h  
> > > > > > > >
> > > > > > > >  
> > > > > > >
> > > > > > > Not sure, we have several options here:
> > > > > > >   1. Keep using Linux sys/queue.h and add enhancements in  
> > > > rte_queue.h  
> > > > > > >   2. Make rte_queue.h a copy of FreeBSD version of queue.h  
> > (that  
> > > > is  
> > > > > > > what Windows did)
> > > > > > >   3. Use the bsd version of queue.h.
> > > > > > >      On Debian/Ubuntu this in libbsd-dev package and referred  
> > to  
> > > > as  
> > > > > > > bsd/sys/queue.h
> > > > > > >
> > > > > > > I chose #1 as simplest. But all of these could work. #3 means  
> > > > DPDK has  
> > > > > > > least new
> > > > > > > code, but adds another dependency to the build.  
> > > > > >
> > > > > > +1 to #2:
> > > > > > If Linux sys/queue.h is frozen, and is a subset of FreeBSD  
> > queue.h,  
> > > > #2 would consolidate all the queue macros in one file.  
> > > > > > That seems easier to navigate (for a developer/reviewer looking  
> > at  
> > > > the macro definitions/implementations) than including sys/queue.h,
> > > > queue.h or lib/eal/windows/include/sys/queue.h depending on O/S,  
> > and  
> > > > then defining the missing macros in rte_queue.h.  
> > > > > > A comment at the top of the rte_queue.h file could mention that  
> > it  
> > > > is a copy of the FreeBSD queue.h file.  
> > > > > >
> > > > > > -1 to #3; it adds an unnecessary dependency.
> > > > > >  
> > > > > > >
> > > > > > > No matter what, should add to checkpatch to block any new  
> > files  
> > > > that  
> > > > > > > include sys/queue.h
> > > > > > > directly.  
> > > > > >
> > > > > > +1 to this.
> > > > > >  
> > > > >
> > > > > I agree with Morten, #2 looks the best option to me too.  
> > > >
> > > > It means we want to remove lib/eal/windows/include/sys/queue.h,  
> > right?  
> > >
> > > Correct.
> > > All queue macros are consolidated into rte_queue.h.
> > >  
> > 
> > I have been trying this, and it possible but there is some fallout.
> > If application includes <sys/queue.h> and other DPDK headers include
> > <rte_queue.h>
> > there are declaration conflicts. Ends up being a compilation failure.
> > Tedious, but not hard to fix all of DPDK to use rte_queue.h instead of
> > sys/queue.h
> > but what about user applications?  
> 
> Perhaps this could work:
> 
> If _SYS_QUEUE_H_ is defined in rte_queue.h, including <sys/queue.h> after <rte_queue.h> will be harmless.
> 
> If each macro in rte_queue.h is surrounded by #ifndef MACRONAME, including <rte_queue.h> after <sys/queue.h> will be harmless.
> Alternatively, surround a group of macros in rte_queue.h by #ifndef _SYS_QUEUE_H_.

Most applications include system headers (ie. sys/queue.h) then DPDK headers (rte_queue.h)
> > 
> > Since that would be API break, best to put it off to 25.11.  
> 
> It might be impossible prohibiting user libraries from including <sys/queue.h>, especially if some 3rd party library includes <sys/queue.h> in the library's header file. The application developer would not want to modify the header files of 3rd party libraries.
> 

It is possible to bracket every macro with #ifdef to avoid collision
but gets messy.

[PATCH v2 2/7] net/qede: fix use after free

[Stephen Hemminger]

The loop cleaning up flowdir resources was using SLIST_FOREACH
but the inner loop would call rte_free. Found by building with
address sanitizer undefined check.

Also remove needless initialization, and null check.

Fixes: f5765f66f9bb ("net/qede: refactor flow director into generic aRFS")
Cc: shahed.shaikh@cavium.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/qede/qede_ethdev.h |  3 +--
 drivers/net/qede/qede_filter.c | 13 +++++--------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
index 3015c18504..b502658425 100644
--- a/drivers/net/qede/qede_ethdev.h
+++ b/drivers/net/qede/qede_ethdev.h
@@ -8,8 +8,7 @@
 #ifndef _QEDE_ETHDEV_H_
 #define _QEDE_ETHDEV_H_
 
-#include <sys/queue.h>
-
+#include <rte_queue.h>
 #include <rte_ether.h>
 #include <ethdev_driver.h>
 #include <ethdev_pci.h>
diff --git a/drivers/net/qede/qede_filter.c b/drivers/net/qede/qede_filter.c
index 14fb4338e9..c3f74c89d9 100644
--- a/drivers/net/qede/qede_filter.c
+++ b/drivers/net/qede/qede_filter.c
@@ -154,15 +154,12 @@ int qede_check_fdir_support(struct rte_eth_dev *eth_dev)
 void qede_fdir_dealloc_resc(struct rte_eth_dev *eth_dev)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
-	struct qede_arfs_entry *tmp = NULL;
+	struct qede_arfs_entry *tmp, *tmp2;
 
-	SLIST_FOREACH(tmp, &qdev->arfs_info.arfs_list_head, list) {
-		if (tmp) {
-			rte_memzone_free(tmp->mz);
-			SLIST_REMOVE(&qdev->arfs_info.arfs_list_head, tmp,
-				     qede_arfs_entry, list);
-			rte_free(tmp);
-		}
+	SLIST_FOREACH_SAFE(tmp, &qdev->arfs_info.arfs_list_head, list, tmp2) {
+		rte_memzone_free(tmp->mz);
+		SLIST_REMOVE(&qdev->arfs_info.arfs_list_head, tmp, qede_arfs_entry, list);
+		rte_free(tmp);
 	}
 }
 
-- 
2.47.2

[PATCH v2 3/7] bus/fslmc: fix use after free

[Stephen Hemminger]

The cleanup loop would deference the dpio_dev after freeing.
Use TAILQ_FOREACH_SAFE to fix that.
Found by building with sanitizer undefined flag.

Fixes: e55d0494ab98 ("bus/fslmc: support secondary process")
Cc: shreyansh.jain@nxp.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 2dfcf7a498..6ae15c2054 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -15,7 +15,6 @@
 #include <signal.h>
 #include <pthread.h>
 #include <sys/types.h>
-#include <sys/queue.h>
 #include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
@@ -27,6 +26,7 @@
 #include <ethdev_driver.h>
 #include <rte_malloc.h>
 #include <rte_memcpy.h>
+#include <rte_queue.h>
 #include <rte_string_fns.h>
 #include <rte_cycles.h>
 #include <rte_kvargs.h>
@@ -403,6 +403,7 @@ dpaa2_create_dpio_device(int vdev_fd,
 	struct rte_dpaa2_device *obj)
 {
 	struct dpaa2_dpio_dev *dpio_dev = NULL;
+	struct dpaa2_dpio_dev *dpio_tmp;
 	struct vfio_region_info reg_info = { .argsz = sizeof(reg_info)};
 	struct qbman_swp_desc p_des;
 	struct dpio_attr attr;
@@ -588,7 +589,7 @@ dpaa2_create_dpio_device(int vdev_fd,
 	rte_free(dpio_dev);
 
 	/* For each element in the list, cleanup */
-	TAILQ_FOREACH(dpio_dev, &dpio_dev_list, next) {
+	TAILQ_FOREACH_SAFE(dpio_dev, &dpio_dev_list, next, dpio_tmp) {
 		if (dpio_dev->dpio) {
 			dpio_disable(dpio_dev->dpio, CMD_PRI_LOW,
 				dpio_dev->token);
-- 
2.47.2

RE: [PATCH v2 3/7] bus/fslmc: fix use after free

[Hemant Agrawal]

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

[PATCH v2 4/7] net/bnxt: fix use after free

[Stephen Hemminger]

The filter cleanup loop was using STAILQ_FOREACH and rte_free
and would dereference the filter after free.

Found by build with -Dbsanitize=address,undefined

Fixes: e8fe0e067b68 ("net/bnxt: fix allocation of PF info struct")
Cc: ajit.khaparde@broadcom.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 7b90ba651f..f083f3aa94 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -3,14 +3,12 @@
  * All rights reserved.
  */
 
-#include <sys/queue.h>
-
 #include <rte_byteorder.h>
 #include <rte_log.h>
 #include <rte_malloc.h>
 #include <rte_flow.h>
 #include <rte_flow_driver.h>
-#include <rte_tailq.h>
+#include <rte_queue.h>
 
 #include "bnxt.h"
 #include "bnxt_filter.h"
@@ -151,7 +149,9 @@ void bnxt_free_filter_mem(struct bnxt *bp)
 	bp->filter_info = NULL;
 
 	for (i = 0; i < bp->pf->max_vfs; i++) {
-		STAILQ_FOREACH(filter, &bp->pf->vf_info[i].filter, next) {
+		struct bnxt_filter_info *tmp;
+
+		STAILQ_FOREACH_SAFE(filter, &bp->pf->vf_info[i].filter, next, tmp) {
 			rte_free(filter);
 			STAILQ_REMOVE(&bp->pf->vf_info[i].filter, filter,
 				      bnxt_filter_info, next);
-- 
2.47.2

[PATCH v2 5/7] net/iavf: replace local version of TAILQ_FOREACH_SAFE

[Stephen Hemminger]

Now in EAL as rte_queue.h

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/intel/iavf/iavf_vchnl.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 6feca8435e..0b7c35250c 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -13,6 +13,7 @@
 #include <rte_byteorder.h>
 #include <rte_common.h>
 #include <rte_os_shim.h>
+#include <rte_queue.h>
 
 #include <rte_debug.h>
 #include <rte_alarm.h>
@@ -52,13 +53,6 @@ static struct iavf_event_handler event_handler = {
 	.fd = {-1, -1},
 };
 
-#ifndef TAILQ_FOREACH_SAFE
-#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
-	for ((var) = TAILQ_FIRST((head)); \
-		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
-	(var) = (tvar))
-#endif
-
 static uint32_t
 iavf_dev_event_handle(void *param __rte_unused)
 {
-- 
2.47.2

[PATCH v2 6/7] vhost: replace open coded TAILQ_FOREACH_SAFE

[Stephen Hemminger]

Proper macro is now in EAL rte_queue.h use it instead.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/vhost/socket.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
index 4c6c551c3d..dc9c9da9d1 100644
--- a/lib/vhost/socket.c
+++ b/lib/vhost/socket.c
@@ -10,10 +10,10 @@
 #include <string.h>
 #include <sys/socket.h>
 #include <sys/un.h>
-#include <sys/queue.h>
 #include <errno.h>
 #include <fcntl.h>
 
+#include <rte_queue.h>
 #include <rte_thread.h>
 #include <rte_log.h>
 
@@ -456,14 +456,7 @@ vhost_user_client_reconnect(void *arg __rte_unused)
 	while (1) {
 		pthread_mutex_lock(&reconn_list.mutex);
 
-		/*
-		 * An equal implementation of TAILQ_FOREACH_SAFE,
-		 * which does not exist on all platforms.
-		 */
-		for (reconn = TAILQ_FIRST(&reconn_list.head);
-		     reconn != NULL; reconn = next) {
-			next = TAILQ_NEXT(reconn, next);
-
+		TAILQ_FOREACH_SAFE(reconn, &reconn_list.head, next, next) {
 			ret = vhost_user_connect_nonblock(reconn->vsocket->path, reconn->fd,
 						(struct sockaddr *)&reconn->un,
 						sizeof(reconn->un));
-- 
2.47.2

[PATCH v2 7/7] raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE

[Stephen Hemminger]

Prefer the EAL version over local version of macro.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
---
 drivers/raw/ifpga/base/opae_osdep.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/raw/ifpga/base/opae_osdep.h b/drivers/raw/ifpga/base/opae_osdep.h
index e35a21c80e..b483d00a54 100644
--- a/drivers/raw/ifpga/base/opae_osdep.h
+++ b/drivers/raw/ifpga/base/opae_osdep.h
@@ -11,6 +11,7 @@
 
 #ifdef RTE_LIB_EAL
 #include "osdep_rte/osdep_generic.h"
+#include <rte_queue.h>
 #else
 #include "osdep_raw/osdep_generic.h"
 #endif
-- 
2.47.2

RE: [PATCH v2 0/7] Introduce FOREACH_SAFE macros

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 14 February 2025 18.20
> 
> This series adds common macros for safe iteration over lists.
> It is a subset copy of the macros from FreeBSD that are
> missing from the Linux header sys/queue.h
> 
> Chose this over several other options:
>   - let each driver define their own as needed.
>     One Intel driver got it wrong, others will as well.
>   - rename all the queue macros to RTE_XXX variants.
>     Seems like useless renaming and confusion.
>   - Several distros have libbsd package with the correct macros.
>     But adding yet another dependency to DPDK would be annoying
>     for something this basic.
> 
> There are more macros in FreeBSD header that could be useful,
> but we can add those later as needed here.

100 % agree with all of the above.
Especially that adding RTE_ prefix to "industry standard" macros would be silly.

And all patches in this series LGTM.

Series-acked-by: Morten Brørup <mb@smartsharesystems.com>

[PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Stephen Hemminger]

This series adds common macros for safe iteration over lists.
It is a subset copy of the macros from FreeBSD that are
missing from the Linux header sys/queue.h

Chose this over several other options:
  - let each driver define their own as needed.
    One Intel driver got it wrong, others will as well.
  - rename all the queue macros to RTE_XXX variants.
    Seems like useless renaming and confusion.
  - Several distros have libbsd package with the correct macros.
    But adding yet another dependency to DPDK would be annoying
    for something this basic.

There are more macros in FreeBSD header that could be useful,
but we can add those later as needed here.

v3 - add a few more open coded loops that can use a macro

Stephen Hemminger (9):
  eal: add queue macro extensions from FreeBSD
  net/qede: fix use after free
  bus/fslmc: fix use after free
  net/bnxt: fix use after free
  net/iavf: replace local version of TAILQ_FOREACH_SAFE
  vhost: replace open coded TAILQ_FOREACH_SAFE
  raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE
  net/i40e: replace open coded version of LIST_FOREACH_SAFE
  net/ice: replace open coded version of LIST_FOREACH_SAFE

 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |   5 +-
 drivers/net/bnxt/bnxt_filter.c           |   8 +-
 drivers/net/intel/i40e/i40e_ethdev.c     |   9 +-
 drivers/net/intel/iavf/iavf_vchnl.c      |   8 +-
 drivers/net/intel/ice/ice_ethdev.c       |   9 +-
 drivers/net/qede/qede_ethdev.h           |   3 +-
 drivers/net/qede/qede_filter.c           |  13 +-
 drivers/raw/ifpga/base/ifpga_enumerate.c |   4 +-
 drivers/raw/ifpga/base/opae_osdep.h      |   1 +
 lib/eal/include/meson.build              |   3 +-
 lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
 lib/vhost/socket.c                       |  11 +-
 12 files changed, 200 insertions(+), 48 deletions(-)
 create mode 100644 lib/eal/include/rte_queue.h

-- 
2.47.2

[PATCH v3 1/9] eal: add queue macro extensions from FreeBSD

[Stephen Hemminger]

The Linux version of sys/queue.h is frozen at an older version
and is missing the _SAFE macro variants. Several drivers started
introducing the own workarounds for this. Should be handled in EAL.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/meson.build |   3 +-
 lib/eal/include/rte_queue.h | 174 ++++++++++++++++++++++++++++++++++++
 2 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 lib/eal/include/rte_queue.h

diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index d903577caa..75bf09381f 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -19,8 +19,8 @@ headers += files(
         'rte_eal.h',
         'rte_eal_memconfig.h',
         'rte_eal_trace.h',
-        'rte_errno.h',
         'rte_epoll.h',
+        'rte_errno.h',
         'rte_fbarray.h',
         'rte_hexdump.h',
         'rte_hypervisor.h',
@@ -38,6 +38,7 @@ headers += files(
         'rte_pci_dev_features.h',
         'rte_per_lcore.h',
         'rte_pflock.h',
+        'rte_queue.h',
         'rte_random.h',
         'rte_reciprocal.h',
         'rte_seqcount.h',
diff --git a/lib/eal/include/rte_queue.h b/lib/eal/include/rte_queue.h
new file mode 100644
index 0000000000..7ec807b15d
--- /dev/null
+++ b/lib/eal/include/rte_queue.h
@@ -0,0 +1,174 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ *
+ * Original Copyright (c) 1991, 1993
+ *	The Regents of the University of California.  All rights reserved.
+ */
+
+#ifndef _RTE_QUEUE_H_
+#define _RTE_QUEUE_H_
+
+/**
+ * @file
+ *  Defines macro's that exist in the FreeBSD version of queue.h
+ *  which are missing in other versions.
+ */
+
+#include <sys/queue.h>
+
+/*
+ * This file defines four types of data structures: singly-linked lists,
+ * singly-linked tail queues, lists and tail queues.
+ *
+ * Below is a summary of implemented functions where:
+ *  o  means the macro exists in original version
+ *  +  means the macro is added here
+ *  -  means the macro is not available
+ *  s  means the macro is available but is slow (runs in O(n) time)
+ *
+ *				SLIST	LIST	STAILQ	TAILQ
+ * _HEAD			o	o	o	o
+ * _HEAD_INITIALIZER		o	o	o	o
+ * _ENTRY			o	o	o	o
+ * _INIT			o	o	o	o
+ * _EMPTY			o	o	o	o
+ * _FIRST			o	o	o	o
+ * _NEXT			o	o	o	o
+ * _FOREACH			o	o	o	o
+ * _FOREACH_FROM		+	+	+	+
+ * _FOREACH_SAFE		+	+	+	+
+ * _FOREACH_FROM_SAFE		+	+	+	+
+ * _FOREACH_REVERSE		-	-	-	o
+ * _FOREACH_REVERSE_FROM	-	-	-	+
+ * _FOREACH_REVERSE_SAFE	-	-	-       +
+ * _FOREACH_REVERSE_FROM_SAFE	-	-	-	+
+ * _INSERT_HEAD			o	o	o	o
+ * _INSERT_BEFORE		-	o	-	o
+ * _INSERT_AFTER		o	o	o	o
+ * _INSERT_TAIL			-	-	o	o
+ * _CONCAT			s	s	o	o
+ * _REMOVE_AFTER		o	-	o	-
+ * _REMOVE_HEAD			o	o	o	o
+ * _REMOVE			s	o	s	o
+ *
+ */
+
+
+/*
+ * Singly-linked List declarations.
+ */
+#ifndef SLIST_FOREACH_FROM
+#define	SLIST_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : SLIST_FIRST((head)));		\
+	    (var);							\
+	    (var) = SLIST_NEXT((var), field))
+#endif
+
+#ifndef SLIST_FOREACH_SAFE
+#define	SLIST_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = SLIST_FIRST((head));				\
+	    (var) && ((tvar) = SLIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef SLIST_FOREACH_FROM_SAFE
+#define	SLIST_FOREACH_FROM_SAFE(var, head, field, tvar)			\
+	for ((var) = ((var) ? (var) : SLIST_FIRST((head)));		\
+	    (var) && ((tvar) = SLIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+
+/*
+ * Singly-linked Tail queue declarations.
+ */
+#ifndef STAILQ_FOREACH_FROM
+#define	STAILQ_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : STAILQ_FIRST((head)));		\
+	   (var);							\
+	   (var) = STAILQ_NEXT((var), field))
+#endif
+
+#ifndef STAILQ_FOREACH_SAFE
+#define	STAILQ_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = STAILQ_FIRST((head));				\
+	    (var) && ((tvar) = STAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef STAILQ_FOREACH_FROM_SAFE
+#define	STAILQ_FOREACH_FROM_SAFE(var, head, field, tvar)		\
+	for ((var) = ((var) ? (var) : STAILQ_FIRST((head)));		\
+	    (var) && ((tvar) = STAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+/*
+ * List declarations.
+ */
+#ifndef LIST_FOREACH_FROM
+#define	LIST_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : LIST_FIRST((head)));		\
+	    (var);							\
+	    (var) = LIST_NEXT((var), field))
+#endif
+
+#ifndef LIST_FOREACH_SAFE
+#define	LIST_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = LIST_FIRST((head));				\
+	    (var) && ((tvar) = LIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef LIST_FOREACH_FROM_SAFE
+#define	LIST_FOREACH_FROM_SAFE(var, head, field, tvar)			\
+	for ((var) = ((var) ? (var) : LIST_FIRST((head)));		\
+	    (var) && ((tvar) = LIST_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+/*
+ * Tail queue declarations.
+ */
+#ifndef TAILQ_FOREACH_FROM
+#define TAILQ_FOREACH_FROM(var, head, field)				\
+	for ((var) = ((var) ? (var) : TAILQ_FIRST((head)));		\
+	    (var);							\
+	    (var) = TAILQ_NEXT((var), field))
+#endif
+
+#ifndef TAILQ_FOREACH_SAFE
+#define TAILQ_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = TAILQ_FIRST((head));				\
+	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef TAILQ_FOREACH_FROM_SAFE
+#define TAILQ_FOREACH_FROM_SAFE(var, head, field, tvar)			\
+	for ((var) = ((var) ? (var) : TAILQ_FIRST((head)));		\
+	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
+#ifndef TAILQ_FOREACH_REVERSE_FROM
+#define TAILQ_FOREACH_REVERSE_FROM(var, head, headname, field)		\
+	for ((var) = ((var) ? (var) : TAILQ_LAST((head), headname));	\
+	    (var);							\
+	    (var) = TAILQ_PREV((var), headname, field))
+#endif
+
+#ifndef TAILQ_FOREACH_REVERSE_SAFE
+#define TAILQ_FOREACH_REVERSE_SAFE(var, head, headname, field, tvar)	\
+	for ((var) = TAILQ_LAST((head), headname);			\
+	    (var) && ((tvar) = TAILQ_PREV((var), headname, field), 1);	\
+	    (var) = (tvar))
+#endif
+
+#ifndef TAILQ_FOREACH_REVERSE_FROM_SAFE
+#define TAILQ_FOREACH_REVERSE_FROM_SAFE(var, head, headname, field, tvar)\
+	for ((var) = ((var) ? (var) : TAILQ_LAST((head), headname));	\
+	    (var) && ((tvar) = TAILQ_PREV((var), headname, field), 1);	\
+	    (var) = (tvar))
+#endif
+
+#endif /* !_RTE_QUEUE_H_ */
-- 
2.47.2

[PATCH v3 2/9] net/qede: fix use after free

[Stephen Hemminger]

The loop cleaning up flowdir resources was using SLIST_FOREACH
but the inner loop would call rte_free. Found by building with
address sanitizer undefined check.

Also remove needless initialization, and null check.

Fixes: f5765f66f9bb ("net/qede: refactor flow director into generic aRFS")
Cc: shahed.shaikh@cavium.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/qede/qede_ethdev.h |  3 +--
 drivers/net/qede/qede_filter.c | 13 +++++--------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
index 3015c18504..b502658425 100644
--- a/drivers/net/qede/qede_ethdev.h
+++ b/drivers/net/qede/qede_ethdev.h
@@ -8,8 +8,7 @@
 #ifndef _QEDE_ETHDEV_H_
 #define _QEDE_ETHDEV_H_
 
-#include <sys/queue.h>
-
+#include <rte_queue.h>
 #include <rte_ether.h>
 #include <ethdev_driver.h>
 #include <ethdev_pci.h>
diff --git a/drivers/net/qede/qede_filter.c b/drivers/net/qede/qede_filter.c
index 14fb4338e9..c3f74c89d9 100644
--- a/drivers/net/qede/qede_filter.c
+++ b/drivers/net/qede/qede_filter.c
@@ -154,15 +154,12 @@ int qede_check_fdir_support(struct rte_eth_dev *eth_dev)
 void qede_fdir_dealloc_resc(struct rte_eth_dev *eth_dev)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
-	struct qede_arfs_entry *tmp = NULL;
+	struct qede_arfs_entry *tmp, *tmp2;
 
-	SLIST_FOREACH(tmp, &qdev->arfs_info.arfs_list_head, list) {
-		if (tmp) {
-			rte_memzone_free(tmp->mz);
-			SLIST_REMOVE(&qdev->arfs_info.arfs_list_head, tmp,
-				     qede_arfs_entry, list);
-			rte_free(tmp);
-		}
+	SLIST_FOREACH_SAFE(tmp, &qdev->arfs_info.arfs_list_head, list, tmp2) {
+		rte_memzone_free(tmp->mz);
+		SLIST_REMOVE(&qdev->arfs_info.arfs_list_head, tmp, qede_arfs_entry, list);
+		rte_free(tmp);
 	}
 }
 
-- 
2.47.2

[PATCH v3 3/9] bus/fslmc: fix use after free

[Stephen Hemminger]

The cleanup loop would deference the dpio_dev after freeing.
Use TAILQ_FOREACH_SAFE to fix that.
Found by building with sanitizer undefined flag.

Fixes: e55d0494ab98 ("bus/fslmc: support secondary process")
Cc: shreyansh.jain@nxp.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
index 2dfcf7a498..6ae15c2054 100644
--- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
+++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
@@ -15,7 +15,6 @@
 #include <signal.h>
 #include <pthread.h>
 #include <sys/types.h>
-#include <sys/queue.h>
 #include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
@@ -27,6 +26,7 @@
 #include <ethdev_driver.h>
 #include <rte_malloc.h>
 #include <rte_memcpy.h>
+#include <rte_queue.h>
 #include <rte_string_fns.h>
 #include <rte_cycles.h>
 #include <rte_kvargs.h>
@@ -403,6 +403,7 @@ dpaa2_create_dpio_device(int vdev_fd,
 	struct rte_dpaa2_device *obj)
 {
 	struct dpaa2_dpio_dev *dpio_dev = NULL;
+	struct dpaa2_dpio_dev *dpio_tmp;
 	struct vfio_region_info reg_info = { .argsz = sizeof(reg_info)};
 	struct qbman_swp_desc p_des;
 	struct dpio_attr attr;
@@ -588,7 +589,7 @@ dpaa2_create_dpio_device(int vdev_fd,
 	rte_free(dpio_dev);
 
 	/* For each element in the list, cleanup */
-	TAILQ_FOREACH(dpio_dev, &dpio_dev_list, next) {
+	TAILQ_FOREACH_SAFE(dpio_dev, &dpio_dev_list, next, dpio_tmp) {
 		if (dpio_dev->dpio) {
 			dpio_disable(dpio_dev->dpio, CMD_PRI_LOW,
 				dpio_dev->token);
-- 
2.47.2

[PATCH v3 4/9] net/bnxt: fix use after free

[Stephen Hemminger]

The filter cleanup loop was using STAILQ_FOREACH and rte_free
and would dereference the filter after free.

Found by build with -Dbsanitize=address,undefined

Fixes: e8fe0e067b68 ("net/bnxt: fix allocation of PF info struct")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_filter.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 7b90ba651f..f083f3aa94 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -3,14 +3,12 @@
  * All rights reserved.
  */
 
-#include <sys/queue.h>
-
 #include <rte_byteorder.h>
 #include <rte_log.h>
 #include <rte_malloc.h>
 #include <rte_flow.h>
 #include <rte_flow_driver.h>
-#include <rte_tailq.h>
+#include <rte_queue.h>
 
 #include "bnxt.h"
 #include "bnxt_filter.h"
@@ -151,7 +149,9 @@ void bnxt_free_filter_mem(struct bnxt *bp)
 	bp->filter_info = NULL;
 
 	for (i = 0; i < bp->pf->max_vfs; i++) {
-		STAILQ_FOREACH(filter, &bp->pf->vf_info[i].filter, next) {
+		struct bnxt_filter_info *tmp;
+
+		STAILQ_FOREACH_SAFE(filter, &bp->pf->vf_info[i].filter, next, tmp) {
 			rte_free(filter);
 			STAILQ_REMOVE(&bp->pf->vf_info[i].filter, filter,
 				      bnxt_filter_info, next);
-- 
2.47.2

[PATCH v3 5/9] net/iavf: replace local version of TAILQ_FOREACH_SAFE

[Stephen Hemminger]

Now in EAL as rte_queue.h

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/intel/iavf/iavf_vchnl.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 6feca8435e..0b7c35250c 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -13,6 +13,7 @@
 #include <rte_byteorder.h>
 #include <rte_common.h>
 #include <rte_os_shim.h>
+#include <rte_queue.h>
 
 #include <rte_debug.h>
 #include <rte_alarm.h>
@@ -52,13 +53,6 @@ static struct iavf_event_handler event_handler = {
 	.fd = {-1, -1},
 };
 
-#ifndef TAILQ_FOREACH_SAFE
-#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \
-	for ((var) = TAILQ_FIRST((head)); \
-		(var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
-	(var) = (tvar))
-#endif
-
 static uint32_t
 iavf_dev_event_handle(void *param __rte_unused)
 {
-- 
2.47.2

[PATCH v3 6/9] vhost: replace open coded TAILQ_FOREACH_SAFE

[Stephen Hemminger]

Proper macro is now in EAL rte_queue.h use it instead.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/vhost/socket.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
index 4c6c551c3d..dc9c9da9d1 100644
--- a/lib/vhost/socket.c
+++ b/lib/vhost/socket.c
@@ -10,10 +10,10 @@
 #include <string.h>
 #include <sys/socket.h>
 #include <sys/un.h>
-#include <sys/queue.h>
 #include <errno.h>
 #include <fcntl.h>
 
+#include <rte_queue.h>
 #include <rte_thread.h>
 #include <rte_log.h>
 
@@ -456,14 +456,7 @@ vhost_user_client_reconnect(void *arg __rte_unused)
 	while (1) {
 		pthread_mutex_lock(&reconn_list.mutex);
 
-		/*
-		 * An equal implementation of TAILQ_FOREACH_SAFE,
-		 * which does not exist on all platforms.
-		 */
-		for (reconn = TAILQ_FIRST(&reconn_list.head);
-		     reconn != NULL; reconn = next) {
-			next = TAILQ_NEXT(reconn, next);
-
+		TAILQ_FOREACH_SAFE(reconn, &reconn_list.head, next, next) {
 			ret = vhost_user_connect_nonblock(reconn->vsocket->path, reconn->fd,
 						(struct sockaddr *)&reconn->un,
 						sizeof(reconn->un));
-- 
2.47.2

[PATCH v3 7/9] raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE

[Stephen Hemminger]

Prefer the EAL version over local version of macro.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
---
 drivers/raw/ifpga/base/ifpga_enumerate.c | 4 +---
 drivers/raw/ifpga/base/opae_osdep.h      | 1 +
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
index 61eb6601ea..085fb6db40 100644
--- a/drivers/raw/ifpga/base/ifpga_enumerate.c
+++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
@@ -725,9 +725,7 @@ static void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
 		return;
 
 	/* remove all device feature lists in the list. */
-	for (dfl = TAILQ_FIRST(&info->dfls);
-		dfl && (tmp = TAILQ_NEXT(dfl, node), 1);
-		dfl = tmp) {
+	TAILQ_FOREACH_SAFE(dfl, &info->dfls, node, tmp) {
 		TAILQ_REMOVE(&info->dfls, dfl, node);
 		opae_free(dfl);
 	}
diff --git a/drivers/raw/ifpga/base/opae_osdep.h b/drivers/raw/ifpga/base/opae_osdep.h
index e35a21c80e..b483d00a54 100644
--- a/drivers/raw/ifpga/base/opae_osdep.h
+++ b/drivers/raw/ifpga/base/opae_osdep.h
@@ -11,6 +11,7 @@
 
 #ifdef RTE_LIB_EAL
 #include "osdep_rte/osdep_generic.h"
+#include <rte_queue.h>
 #else
 #include "osdep_raw/osdep_generic.h"
 #endif
-- 
2.47.2

[PATCH v3 8/9] net/i40e: replace open coded version of LIST_FOREACH_SAFE

[Stephen Hemminger]

Use macro instead.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/intel/i40e/i40e_ethdev.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
index 1c5ab35a8b..616053dd5b 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.c
+++ b/drivers/net/intel/i40e/i40e_ethdev.c
@@ -29,6 +29,7 @@
 #include <rte_hash_crc.h>
 #include <rte_bitmap.h>
 #include <rte_os_shim.h>
+#include <rte_queue.h>
 
 #include "i40e_logs.h"
 #include "base/i40e_prototype.h"
@@ -5096,16 +5097,12 @@ i40e_res_pool_destroy(struct i40e_res_pool_info *pool)
 	if (pool == NULL)
 		return;
 
-	for (entry = LIST_FIRST(&pool->alloc_list);
-			entry && (next_entry = LIST_NEXT(entry, next), 1);
-			entry = next_entry) {
+	LIST_FOREACH_SAFE(entry, &pool->alloc_list, next, next_entry) {
 		LIST_REMOVE(entry, next);
 		rte_free(entry);
 	}
 
-	for (entry = LIST_FIRST(&pool->free_list);
-			entry && (next_entry = LIST_NEXT(entry, next), 1);
-			entry = next_entry) {
+	LIST_FOREACH_SAFE(entry, &pool->free_list, next, next_entry) {
 		LIST_REMOVE(entry, next);
 		rte_free(entry);
 	}
-- 
2.47.2

Re: [PATCH v3 8/9] net/i40e: replace open coded version of LIST_FOREACH_SAFE

[Burakov, Anatoly]

On 3/13/2025 12:15 AM, Stephen Hemminger wrote:
> Use macro instead.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
-- 
Thanks,
Anatoly

[PATCH v3 9/9] net/ice: replace open coded version of LIST_FOREACH_SAFE

[Stephen Hemminger]

Use macro instead.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/intel/ice/ice_ethdev.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 3cdfa16f77..72129f5e0e 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -15,6 +15,7 @@
 
 #include <rte_tailq.h>
 #include <rte_os_shim.h>
+#include <rte_queue.h>
 
 #include "eal_firmware.h"
 
@@ -884,16 +885,12 @@ ice_res_pool_destroy(struct ice_res_pool_info *pool)
 	if (!pool)
 		return;
 
-	for (entry = LIST_FIRST(&pool->alloc_list);
-	     entry && (next_entry = LIST_NEXT(entry, next), 1);
-	     entry = next_entry) {
+	LIST_FOREACH_SAFE(entry, &pool->alloc_list, next, next_entry) {
 		LIST_REMOVE(entry, next);
 		rte_free(entry);
 	}
 
-	for (entry = LIST_FIRST(&pool->free_list);
-	     entry && (next_entry = LIST_NEXT(entry, next), 1);
-	     entry = next_entry) {
+	LIST_FOREACH_SAFE(entry, &pool->free_list, next, next_entry) {
 		LIST_REMOVE(entry, next);
 		rte_free(entry);
 	}
-- 
2.47.2

Re: [PATCH v3 9/9] net/ice: replace open coded version of LIST_FOREACH_SAFE

[Burakov, Anatoly]

On 3/13/2025 12:15 AM, Stephen Hemminger wrote:
> Use macro instead.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Stephen Hemminger]

On Wed, 12 Mar 2025 16:15:29 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> This series adds common macros for safe iteration over lists.
> It is a subset copy of the macros from FreeBSD that are
> missing from the Linux header sys/queue.h
> 
> Chose this over several other options:
>   - let each driver define their own as needed.
>     One Intel driver got it wrong, others will as well.
>   - rename all the queue macros to RTE_XXX variants.
>     Seems like useless renaming and confusion.
>   - Several distros have libbsd package with the correct macros.
>     But adding yet another dependency to DPDK would be annoying
>     for something this basic.
> 
> There are more macros in FreeBSD header that could be useful,
> but we can add those later as needed here.
> 
> v3 - add a few more open coded loops that can use a macro
> 
> Stephen Hemminger (9):
>   eal: add queue macro extensions from FreeBSD
>   net/qede: fix use after free
>   bus/fslmc: fix use after free
>   net/bnxt: fix use after free
>   net/iavf: replace local version of TAILQ_FOREACH_SAFE
>   vhost: replace open coded TAILQ_FOREACH_SAFE
>   raw/ifpga: use EAL version of TAILQ_FOREACH_SAFE
>   net/i40e: replace open coded version of LIST_FOREACH_SAFE
>   net/ice: replace open coded version of LIST_FOREACH_SAFE
> 
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c |   5 +-
>  drivers/net/bnxt/bnxt_filter.c           |   8 +-
>  drivers/net/intel/i40e/i40e_ethdev.c     |   9 +-
>  drivers/net/intel/iavf/iavf_vchnl.c      |   8 +-
>  drivers/net/intel/ice/ice_ethdev.c       |   9 +-
>  drivers/net/qede/qede_ethdev.h           |   3 +-
>  drivers/net/qede/qede_filter.c           |  13 +-
>  drivers/raw/ifpga/base/ifpga_enumerate.c |   4 +-
>  drivers/raw/ifpga/base/opae_osdep.h      |   1 +
>  lib/eal/include/meson.build              |   3 +-
>  lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++

Revisiting this and wondering about naming...
The file rte_queue.h is not really DPDK (ie not related to runtime environment).
Thinking of calling it bsd_queue.h as a comprimise

RE: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, 18 August 2025 18.34
> 
> On Wed, 12 Mar 2025 16:15:29 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> > This series adds common macros for safe iteration over lists.
> > It is a subset copy of the macros from FreeBSD that are
> > missing from the Linux header sys/queue.h
> >
> > Chose this over several other options:
> >   - let each driver define their own as needed.
> >     One Intel driver got it wrong, others will as well.
> >   - rename all the queue macros to RTE_XXX variants.
> >     Seems like useless renaming and confusion.
> >   - Several distros have libbsd package with the correct macros.
> >     But adding yet another dependency to DPDK would be annoying
> >     for something this basic.
> >
> > There are more macros in FreeBSD header that could be useful,
> > but we can add those later as needed here.
> >
> >  lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
> 
> Revisiting this and wondering about naming...
> The file rte_queue.h is not really DPDK (ie not related to runtime
> environment).
> Thinking of calling it bsd_queue.h as a compromise

Since it replaces sys/queue.h, then maybe sys_queue.h (or rte_sys_queue.h).

But more importantly:
It is not really DPDK, and thus shouldn't really be part of the EAL.
So here's an idea:
As part of de-bloating the EAL, can we somehow add a new directory structure for independent "libraries" like this?
And treat this rte_queue.h file as a "header file only" library, and put it there.
Then, build wise, the EAL could depend on this "library".

Re: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Burakov, Anatoly]

On 8/20/2025 8:42 AM, Morten Brørup wrote:
>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> Sent: Monday, 18 August 2025 18.34
>>
>> On Wed, 12 Mar 2025 16:15:29 -0700
>> Stephen Hemminger <stephen@networkplumber.org> wrote:
>>
>>> This series adds common macros for safe iteration over lists.
>>> It is a subset copy of the macros from FreeBSD that are
>>> missing from the Linux header sys/queue.h
>>>
>>> Chose this over several other options:
>>>    - let each driver define their own as needed.
>>>      One Intel driver got it wrong, others will as well.
>>>    - rename all the queue macros to RTE_XXX variants.
>>>      Seems like useless renaming and confusion.
>>>    - Several distros have libbsd package with the correct macros.
>>>      But adding yet another dependency to DPDK would be annoying
>>>      for something this basic.
>>>
>>> There are more macros in FreeBSD header that could be useful,
>>> but we can add those later as needed here.
>>>
>>>   lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
>>
>> Revisiting this and wondering about naming...
>> The file rte_queue.h is not really DPDK (ie not related to runtime
>> environment).
>> Thinking of calling it bsd_queue.h as a compromise
> 
> Since it replaces sys/queue.h, then maybe sys_queue.h (or rte_sys_queue.h).
> 
> But more importantly:
> It is not really DPDK, and thus shouldn't really be part of the EAL.
> So here's an idea:
> As part of de-bloating the EAL, can we somehow add a new directory structure for independent "libraries" like this?
> And treat this rte_queue.h file as a "header file only" library, and put it there.
> Then, build wise, the EAL could depend on this "library".
> 

IMO it depends on what you mean by "EAL". EAL is environment abstraction 
layer, and this header abstracts OS, thereby meeting description of an 
"environment abstraction layer"?

-- 
Thanks,
Anatoly

RE: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Morten Brørup]

> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> Sent: Wednesday, 27 August 2025 16.14
> 
> On 8/20/2025 8:42 AM, Morten Brørup wrote:
> >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >> Sent: Monday, 18 August 2025 18.34
> >>
> >> On Wed, 12 Mar 2025 16:15:29 -0700
> >> Stephen Hemminger <stephen@networkplumber.org> wrote:
> >>
> >>> This series adds common macros for safe iteration over lists.
> >>> It is a subset copy of the macros from FreeBSD that are
> >>> missing from the Linux header sys/queue.h
> >>>
> >>> Chose this over several other options:
> >>>    - let each driver define their own as needed.
> >>>      One Intel driver got it wrong, others will as well.
> >>>    - rename all the queue macros to RTE_XXX variants.
> >>>      Seems like useless renaming and confusion.
> >>>    - Several distros have libbsd package with the correct macros.
> >>>      But adding yet another dependency to DPDK would be annoying
> >>>      for something this basic.
> >>>
> >>> There are more macros in FreeBSD header that could be useful,
> >>> but we can add those later as needed here.
> >>>
> >>>   lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
> >>
> >> Revisiting this and wondering about naming...
> >> The file rte_queue.h is not really DPDK (ie not related to runtime
> >> environment).
> >> Thinking of calling it bsd_queue.h as a compromise
> >
> > Since it replaces sys/queue.h, then maybe sys_queue.h (or rte_sys_queue.h).
> >
> > But more importantly:
> > It is not really DPDK, and thus shouldn't really be part of the EAL.
> > So here's an idea:
> > As part of de-bloating the EAL, can we somehow add a new directory structure
> for independent "libraries" like this?
> > And treat this rte_queue.h file as a "header file only" library, and put it
> there.
> > Then, build wise, the EAL could depend on this "library".
> >
> 
> IMO it depends on what you mean by "EAL". EAL is environment abstraction
> layer, and this header abstracts OS, thereby meeting description of an
> "environment abstraction layer"?

This library (header file) is generic, and has zero interaction with the hardware and OS, so it's not an environment abstraction.
The EAL has become a dump for "everything else" that isn't an individual library with its own subdirectory of the /lib directory.
IMO, it would be nice if we could separate generic utility libraries from the EAL.

Re: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Thomas Monjalon]

27/08/2025 17:08, Morten Brørup:
> > From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> > Sent: Wednesday, 27 August 2025 16.14
> > 
> > On 8/20/2025 8:42 AM, Morten Brørup wrote:
> > >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > >> Sent: Monday, 18 August 2025 18.34
> > >>
> > >> On Wed, 12 Mar 2025 16:15:29 -0700
> > >> Stephen Hemminger <stephen@networkplumber.org> wrote:
> > >>
> > >>> This series adds common macros for safe iteration over lists.
> > >>> It is a subset copy of the macros from FreeBSD that are
> > >>> missing from the Linux header sys/queue.h
> > >>>
> > >>> Chose this over several other options:
> > >>>    - let each driver define their own as needed.
> > >>>      One Intel driver got it wrong, others will as well.
> > >>>    - rename all the queue macros to RTE_XXX variants.
> > >>>      Seems like useless renaming and confusion.
> > >>>    - Several distros have libbsd package with the correct macros.
> > >>>      But adding yet another dependency to DPDK would be annoying
> > >>>      for something this basic.
> > >>>
> > >>> There are more macros in FreeBSD header that could be useful,
> > >>> but we can add those later as needed here.
> > >>>
> > >>>   lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
> > >>
> > >> Revisiting this and wondering about naming...
> > >> The file rte_queue.h is not really DPDK (ie not related to runtime
> > >> environment).
> > >> Thinking of calling it bsd_queue.h as a compromise
> > >
> > > Since it replaces sys/queue.h, then maybe sys_queue.h (or rte_sys_queue.h).
> > >
> > > But more importantly:
> > > It is not really DPDK, and thus shouldn't really be part of the EAL.
> > > So here's an idea:
> > > As part of de-bloating the EAL, can we somehow add a new directory structure
> > for independent "libraries" like this?
> > > And treat this rte_queue.h file as a "header file only" library, and put it
> > there.
> > > Then, build wise, the EAL could depend on this "library".
> > >
> > 
> > IMO it depends on what you mean by "EAL". EAL is environment abstraction
> > layer, and this header abstracts OS, thereby meeting description of an
> > "environment abstraction layer"?
> 
> This library (header file) is generic, and has zero interaction with the hardware and OS, so it's not an environment abstraction.

I disagree here, it is something due by the OS libc,
but not reliably available everywhere.

> The EAL has become a dump for "everything else" that isn't an individual library with its own subdirectory of the /lib directory.
> IMO, it would be nice if we could separate generic utility libraries from the EAL.

I agree with the goal of having a thinner EAL.

I'm not sure about this one.

RE: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 27 August 2025 17.12
> 
> 27/08/2025 17:08, Morten Brørup:
> > > From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> > > Sent: Wednesday, 27 August 2025 16.14
> > >
> > > On 8/20/2025 8:42 AM, Morten Brørup wrote:
> > > >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > >> Sent: Monday, 18 August 2025 18.34
> > > >>
> > > >> On Wed, 12 Mar 2025 16:15:29 -0700
> > > >> Stephen Hemminger <stephen@networkplumber.org> wrote:
> > > >>
> > > >>> This series adds common macros for safe iteration over lists.
> > > >>> It is a subset copy of the macros from FreeBSD that are
> > > >>> missing from the Linux header sys/queue.h
> > > >>>
> > > >>> Chose this over several other options:
> > > >>>    - let each driver define their own as needed.
> > > >>>      One Intel driver got it wrong, others will as well.
> > > >>>    - rename all the queue macros to RTE_XXX variants.
> > > >>>      Seems like useless renaming and confusion.
> > > >>>    - Several distros have libbsd package with the correct macros.
> > > >>>      But adding yet another dependency to DPDK would be annoying
> > > >>>      for something this basic.
> > > >>>
> > > >>> There are more macros in FreeBSD header that could be useful,
> > > >>> but we can add those later as needed here.
> > > >>>
> > > >>>   lib/eal/include/rte_queue.h              | 174
> +++++++++++++++++++++++
> > > >>
> > > >> Revisiting this and wondering about naming...
> > > >> The file rte_queue.h is not really DPDK (ie not related to runtime
> > > >> environment).
> > > >> Thinking of calling it bsd_queue.h as a compromise
> > > >
> > > > Since it replaces sys/queue.h, then maybe sys_queue.h (or
> rte_sys_queue.h).
> > > >
> > > > But more importantly:
> > > > It is not really DPDK, and thus shouldn't really be part of the EAL.
> > > > So here's an idea:
> > > > As part of de-bloating the EAL, can we somehow add a new directory
> structure
> > > for independent "libraries" like this?
> > > > And treat this rte_queue.h file as a "header file only" library, and put
> it
> > > there.
> > > > Then, build wise, the EAL could depend on this "library".
> > > >
> > >
> > > IMO it depends on what you mean by "EAL". EAL is environment abstraction
> > > layer, and this header abstracts OS, thereby meeting description of an
> > > "environment abstraction layer"?
> >
> > This library (header file) is generic, and has zero interaction with the
> hardware and OS, so it's not an environment abstraction.
> 
> I disagree here, it is something due by the OS libc,
> but not reliably available everywhere.
> 
> > The EAL has become a dump for "everything else" that isn't an individual
> library with its own subdirectory of the /lib directory.
> > IMO, it would be nice if we could separate generic utility libraries from
> the EAL.
> 
> I agree with the goal of having a thinner EAL.
> 
> I'm not sure about this one.

I thought this file might be a good place to start separating utilities from abstraction.
However, a well designed roadmap to re-organize the EAL is probably better than starting with this specific library.

Sorry (not sorry) about the noise. I will keep reminding the community about the bloated EAL from time to time. :-)

Re: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Bruce Richardson]

On Wed, Aug 27, 2025 at 05:08:50PM +0200, Morten Brørup wrote:
> > From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> > Sent: Wednesday, 27 August 2025 16.14
> > 
> > On 8/20/2025 8:42 AM, Morten Brørup wrote:
> > >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > >> Sent: Monday, 18 August 2025 18.34
> > >>
> > >> On Wed, 12 Mar 2025 16:15:29 -0700
> > >> Stephen Hemminger <stephen@networkplumber.org> wrote:
> > >>
> > >>> This series adds common macros for safe iteration over lists.
> > >>> It is a subset copy of the macros from FreeBSD that are
> > >>> missing from the Linux header sys/queue.h
> > >>>
> > >>> Chose this over several other options:
> > >>>    - let each driver define their own as needed.
> > >>>      One Intel driver got it wrong, others will as well.
> > >>>    - rename all the queue macros to RTE_XXX variants.
> > >>>      Seems like useless renaming and confusion.
> > >>>    - Several distros have libbsd package with the correct macros.
> > >>>      But adding yet another dependency to DPDK would be annoying
> > >>>      for something this basic.
> > >>>
> > >>> There are more macros in FreeBSD header that could be useful,
> > >>> but we can add those later as needed here.
> > >>>
> > >>>   lib/eal/include/rte_queue.h              | 174 +++++++++++++++++++++++
> > >>
> > >> Revisiting this and wondering about naming...
> > >> The file rte_queue.h is not really DPDK (ie not related to runtime
> > >> environment).
> > >> Thinking of calling it bsd_queue.h as a compromise
> > >
> > > Since it replaces sys/queue.h, then maybe sys_queue.h (or rte_sys_queue.h).
> > >
> > > But more importantly:
> > > It is not really DPDK, and thus shouldn't really be part of the EAL.
> > > So here's an idea:
> > > As part of de-bloating the EAL, can we somehow add a new directory structure
> > for independent "libraries" like this?
> > > And treat this rte_queue.h file as a "header file only" library, and put it
> > there.
> > > Then, build wise, the EAL could depend on this "library".
> > >
> > 
> > IMO it depends on what you mean by "EAL". EAL is environment abstraction
> > layer, and this header abstracts OS, thereby meeting description of an
> > "environment abstraction layer"?
> 
> This library (header file) is generic, and has zero interaction with the hardware and OS, so it's not an environment abstraction.
> The EAL has become a dump for "everything else" that isn't an individual library with its own subdirectory of the /lib directory.
> IMO, it would be nice if we could separate generic utility libraries from the EAL.
>
We could start fairly easily by extracting out generic utility headers, no?

/Bruce 

Re: [PATCH v3 0/9] introduce common FOREACH_SAFE macros

[Stephen Hemminger]

On Wed, 27 Aug 2025 16:55:12 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> > This library (header file) is generic, and has zero interaction with the hardware and OS, so it's not an environment abstraction.
> > The EAL has become a dump for "everything else" that isn't an individual library with its own subdirectory of the /lib directory.
> > IMO, it would be nice if we could separate generic utility libraries from the EAL.
> >  
> We could start fairly easily by extracting out generic utility headers, no?

Vendoring OS headers is a bad, bad idea.