Subject: Re: [PATCH net-next v2 3/3] ipv4: Centralize TOS matching

From: Guillaume Nault <[email-masked]>
Date: Tue, 3 Sep 2024 11:02:34 +0200

On Mon, Sep 02, 2024 at 10:09:21PM +0300, Ido Schimmel wrote:
> On Mon, Sep 02, 2024 at 10:50:17AM -0600, David Ahern wrote:
> > On 8/14/24 6:52 AM, Ido Schimmel wrote:
> > > diff --git a/include/uapi/linux/in_route.h b/include/uapi/linux/in_route.h
> > > index 0cc2c23b47f8..10bdd7e7107f 100644
> > > --- a/include/uapi/linux/in_route.h
> > > +++ b/include/uapi/linux/in_route.h
> > > @@ -2,6 +2,8 @@
> > >  #ifndef _LINUX_IN_ROUTE_H
> > >  #define _LINUX_IN_ROUTE_H
> > >  
> > > +#include <linux/ip.h>
> > > +
> > >  /* IPv4 routing cache flags */
> > >  
> > >  #define RTCF_DEAD	RTNH_F_DEAD
> > 
> > This breaks compile of iproute2 (on Ubuntu 22.04 at least):
> 
> Sorry about that. Some definitions in include/uapi/linux/ip.h conflict
> with those in /usr/include/netinet/ip.h.
> 
> Guillaume, any objections going back to v1 [1]?

No objection. Let's go back to v1. Any other attempt to fix the
situation would probably require ugly workarounds.

> [1]
> https://lore.kernel.org/netdev/[email-masked]/
> 
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 269ec10f63e4..967e4dc555fa 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -22,6 +22,7 @@
>  #include <linux/percpu.h>
>  #include <linux/notifier.h>
>  #include <linux/refcount.h>
> +#include <linux/ip.h>
>  #include <linux/in_route.h>
>  
>  struct fib_config {
> diff --git a/include/uapi/linux/in_route.h b/include/uapi/linux/in_route.h
> index 10bdd7e7107f..0cc2c23b47f8 100644
> --- a/include/uapi/linux/in_route.h
> +++ b/include/uapi/linux/in_route.h
> @@ -2,8 +2,6 @@
>  #ifndef _LINUX_IN_ROUTE_H
>  #define _LINUX_IN_ROUTE_H
>  
> -#include <linux/ip.h>
> -
>  /* IPv4 routing cache flags */
>  
>  #define RTCF_DEAD      RTNH_F_DEAD
> 
> > 
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:25: warning: "IPTOS_TOS" redefined
> >    25 | #define IPTOS_TOS(tos)          ((tos)&IPTOS_TOS_MASK)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:212: note: this is the location of the
> > previous definition
> >   212 | #define IPTOS_TOS(tos)          ((tos) & IPTOS_TOS_MASK)
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:29: warning: "IPTOS_MINCOST" redefined
> >    29 | #define IPTOS_MINCOST           0x02
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:217: note: this is the location of the
> > previous definition
> >   217 | #define IPTOS_MINCOST           IPTOS_LOWCOST
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:31: warning: "IPTOS_PREC_MASK" redefined
> >    31 | #define IPTOS_PREC_MASK         0xE0
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:222: note: this is the location of the
> > previous definition
> >   222 | #define IPTOS_PREC_MASK                 IPTOS_CLASS_MASK
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:32: warning: "IPTOS_PREC" redefined
> >    32 | #define IPTOS_PREC(tos)         ((tos)&IPTOS_PREC_MASK)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:223: note: this is the location of the
> > previous definition
> >   223 | #define IPTOS_PREC(tos)                 IPTOS_CLASS(tos)
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:33: warning: "IPTOS_PREC_NETCONTROL" redefined
> >    33 | #define IPTOS_PREC_NETCONTROL           0xe0
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:224: note: this is the location of the
> > previous definition
> >   224 | #define IPTOS_PREC_NETCONTROL           IPTOS_CLASS_CS7
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:34: warning: "IPTOS_PREC_INTERNETCONTROL"
> > redefined
> >    34 | #define IPTOS_PREC_INTERNETCONTROL      0xc0
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:225: note: this is the location of the
> > previous definition
> >   225 | #define IPTOS_PREC_INTERNETCONTROL      IPTOS_CLASS_CS6
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:35: warning: "IPTOS_PREC_CRITIC_ECP" redefined
> >    35 | #define IPTOS_PREC_CRITIC_ECP           0xa0
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:226: note: this is the location of the
> > previous definition
> >   226 | #define IPTOS_PREC_CRITIC_ECP           IPTOS_CLASS_CS5
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:36: warning: "IPTOS_PREC_FLASHOVERRIDE" redefined
> >    36 | #define IPTOS_PREC_FLASHOVERRIDE        0x80
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:227: note: this is the location of the
> > previous definition
> >   227 | #define IPTOS_PREC_FLASHOVERRIDE        IPTOS_CLASS_CS4
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:37: warning: "IPTOS_PREC_FLASH" redefined
> >    37 | #define IPTOS_PREC_FLASH                0x60
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:228: note: this is the location of the
> > previous definition
> >   228 | #define IPTOS_PREC_FLASH                IPTOS_CLASS_CS3
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:38: warning: "IPTOS_PREC_IMMEDIATE" redefined
> >    38 | #define IPTOS_PREC_IMMEDIATE            0x40
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:229: note: this is the location of the
> > previous definition
> >   229 | #define IPTOS_PREC_IMMEDIATE            IPTOS_CLASS_CS2
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:39: warning: "IPTOS_PREC_PRIORITY" redefined
> >    39 | #define IPTOS_PREC_PRIORITY             0x20
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:230: note: this is the location of the
> > previous definition
> >   230 | #define IPTOS_PREC_PRIORITY             IPTOS_CLASS_CS1
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:40: warning: "IPTOS_PREC_ROUTINE" redefined
> >    40 | #define IPTOS_PREC_ROUTINE              0x00
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:231: note: this is the location of the
> > previous definition
> >   231 | #define IPTOS_PREC_ROUTINE              IPTOS_CLASS_CS0
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:48: warning: "IPOPT_COPIED" redefined
> >    48 | #define IPOPT_COPIED(o)         ((o)&IPOPT_COPY)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:240: note: this is the location of the
> > previous definition
> >   240 | #define IPOPT_COPIED(o)         ((o) & IPOPT_COPY)
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:49: warning: "IPOPT_CLASS" redefined
> >    49 | #define IPOPT_CLASS(o)          ((o)&IPOPT_CLASS_MASK)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:241: note: this is the location of the
> > previous definition
> >   241 | #define IPOPT_CLASS(o)          ((o) & IPOPT_CLASS_MASK)
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:50: warning: "IPOPT_NUMBER" redefined
> >    50 | #define IPOPT_NUMBER(o)         ((o)&IPOPT_NUMBER_MASK)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:242: note: this is the location of the
> > previous definition
> >   242 | #define IPOPT_NUMBER(o)         ((o) & IPOPT_NUMBER_MASK)
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:54: warning: "IPOPT_MEASUREMENT" redefined
> >    54 | #define IPOPT_MEASUREMENT       0x40
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:247: note: this is the location of the
> > previous definition
> >   247 | #define IPOPT_MEASUREMENT       IPOPT_DEBMEAS
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:57: warning: "IPOPT_END" redefined
> >    57 | #define IPOPT_END       (0 |IPOPT_CONTROL)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:251: note: this is the location of the
> > previous definition
> >   251 | #define IPOPT_END               IPOPT_EOL
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:58: warning: "IPOPT_NOOP" redefined
> >    58 | #define IPOPT_NOOP      (1 |IPOPT_CONTROL)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:253: note: this is the location of the
> > previous definition
> >   253 | #define IPOPT_NOOP              IPOPT_NOP
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:59: warning: "IPOPT_SEC" redefined
> >    59 | #define IPOPT_SEC       (2 |IPOPT_CONTROL|IPOPT_COPY)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:259: note: this is the location of the
> > previous definition
> >   259 | #define IPOPT_SEC               IPOPT_SECURITY
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:60: warning: "IPOPT_LSRR" redefined
> >    60 | #define IPOPT_LSRR      (3 |IPOPT_CONTROL|IPOPT_COPY)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:260: note: this is the location of the
> > previous definition
> >   260 | #define IPOPT_LSRR              131             /* loose source
> > route */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:61: warning: "IPOPT_TIMESTAMP" redefined
> >    61 | #define IPOPT_TIMESTAMP (4 |IPOPT_MEASUREMENT)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:257: note: this is the location of the
> > previous definition
> >   257 | #define IPOPT_TIMESTAMP         IPOPT_TS
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:63: warning: "IPOPT_RR" redefined
> >    63 | #define IPOPT_RR        (7 |IPOPT_CONTROL)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:255: note: this is the location of the
> > previous definition
> >   255 | #define IPOPT_RR                7               /* record packet
> > route */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:64: warning: "IPOPT_SID" redefined
> >    64 | #define IPOPT_SID       (8 |IPOPT_CONTROL|IPOPT_COPY)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:262: note: this is the location of the
> > previous definition
> >   262 | #define IPOPT_SID               IPOPT_SATID
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:65: warning: "IPOPT_SSRR" redefined
> >    65 | #define IPOPT_SSRR      (9 |IPOPT_CONTROL|IPOPT_COPY)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:263: note: this is the location of the
> > previous definition
> >   263 | #define IPOPT_SSRR              137             /* strict source
> > route */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:66: warning: "IPOPT_RA" redefined
> >    66 | #define IPOPT_RA        (20|IPOPT_CONTROL|IPOPT_COPY)
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:264: note: this is the location of the
> > previous definition
> >   264 | #define IPOPT_RA                148             /* router alert */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:77: warning: "IPOPT_NOP" redefined
> >    77 | #define IPOPT_NOP IPOPT_NOOP
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:252: note: this is the location of the
> > previous definition
> >   252 | #define IPOPT_NOP               1               /* no operation */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:78: warning: "IPOPT_EOL" redefined
> >    78 | #define IPOPT_EOL IPOPT_END
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:250: note: this is the location of the
> > previous definition
> >   250 | #define IPOPT_EOL               0               /* end of option
> > list */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:79: warning: "IPOPT_TS" redefined
> >    79 | #define IPOPT_TS  IPOPT_TIMESTAMP
> >       |
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:256: note: this is the location of the
> > previous definition
> >   256 | #define IPOPT_TS                68              /* timestamp */
> >       |
> > In file included from ../include/uapi/linux/in_route.h:5,
> >                  from iproute.c:19:
> > ../include/uapi/linux/ip.h:87:8: error: redefinition of ‘struct iphdr’
> >    87 | struct iphdr {
> >       |        ^~~~~
> > In file included from iproute.c:17:
> > /usr/include/netinet/ip.h:44:8: note: originally defined here
> >    44 | struct iphdr
> >       |        ^~~~~
>