mbox series

[RFC,0/7] Introduce FreeBSD macros for SAFE iteration

Message ID 20250127180842.97907-1-stephen@networkplumber.org (mailing list archive)
Headers
Series Introduce FreeBSD macros for SAFE iteration |

Message

Stephen Hemminger Jan. 27, 2025, 6:03 p.m. UTC
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.

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/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
  

Comments

Bruce Richardson Jan. 27, 2025, 6:16 p.m. UTC | #1
On Mon, Jan 27, 2025 at 10:03:54AM -0800, Stephen Hemminger 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.
> 

Actually, I wouldn't be that quick to eliminate the last option. It may
give us some additional options for simplification. For example, the
strlcpy and strlcat functions are in libbsd too, and if we had that as
mandatory dependency, perhaps we could remove some extra code there too?

/Bruce
  
Stephen Hemminger Jan. 27, 2025, 6:43 p.m. UTC | #2
On Mon, 27 Jan 2025 18:16:18 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Mon, Jan 27, 2025 at 10:03:54AM -0800, Stephen Hemminger 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.
> >   
> 
> Actually, I wouldn't be that quick to eliminate the last option. It may
> give us some additional options for simplification. For example, the
> strlcpy and strlcat functions are in libbsd too, and if we had that as
> mandatory dependency, perhaps we could remove some extra code there too?
> 
> /Bruce
> 

I would be ok with using libbsd but only if we didn't have to keep a parallel
copy for all the other compiler and OS variants. And would it be global or
a per-driver dependency?
  
Morten Brørup Jan. 27, 2025, 7:29 p.m. UTC | #3
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, 27 January 2025 19.44
> 
> On Mon, 27 Jan 2025 18:16:18 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Mon, Jan 27, 2025 at 10:03:54AM -0800, Stephen Hemminger 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.
> > >
> >
> > Actually, I wouldn't be that quick to eliminate the last option. It
> may
> > give us some additional options for simplification. For example, the
> > strlcpy and strlcat functions are in libbsd too, and if we had that
> as
> > mandatory dependency, perhaps we could remove some extra code there
> too?
> >
> > /Bruce
> >
> 
> I would be ok with using libbsd but only if we didn't have to keep a
> parallel
> copy for all the other compiler and OS variants. And would it be global
> or
> a per-driver dependency?

+1 to providing our own implementations of relevant libbsd features in the DPDK EAL, rather than depending on the entire libbsd (and libbsd-dev for development). Providing these features as part of a "utilities library" (which is currently integrated into the EAL) is better for non-Unix environments.

Furthermore, libbsd has plenty of stuff we don't need:
https://manpages.debian.org/testing/libbsd-dev/index.html
  
Stephen Hemminger Jan. 27, 2025, 11:14 p.m. UTC | #4
On Mon, 27 Jan 2025 20:29:55 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Monday, 27 January 2025 19.44
> > 
> > On Mon, 27 Jan 2025 18:16:18 +0000
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >   
> > > On Mon, Jan 27, 2025 at 10:03:54AM -0800, Stephen Hemminger 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.
> > > >  
> > >
> > > Actually, I wouldn't be that quick to eliminate the last option. It  
> > may  
> > > give us some additional options for simplification. For example, the
> > > strlcpy and strlcat functions are in libbsd too, and if we had that  
> > as  
> > > mandatory dependency, perhaps we could remove some extra code there  
> > too?  
> > >
> > > /Bruce
> > >  
> > 
> > I would be ok with using libbsd but only if we didn't have to keep a
> > parallel
> > copy for all the other compiler and OS variants. And would it be global
> > or
> > a per-driver dependency?  
> 
> +1 to providing our own implementations of relevant libbsd features in the DPDK EAL, rather than depending on the entire libbsd (and libbsd-dev for development). Providing these features as part of a "utilities library" (which is currently integrated into the EAL) is better for non-Unix environments.
> 
> Furthermore, libbsd has plenty of stuff we don't need:
> https://manpages.debian.org/testing/libbsd-dev/index.html

The red-black tries in libbsd are very useful. In one product we used it as a way
to manage LPM rules, since the current DPDK model is O(N^2) and works terribly in
a router with 3M routes.