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

[Stephen Hemminger <stephen@networkplumber.org>]

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.