All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guillaume Nault <gnault@redhat.com>
To: Ido Schimmel <idosch@nvidia.com>
Cc: David Ahern <dsahern@kernel.org>,
	netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
	davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
	edumazet@google.com, pablo@netfilter.org, kadlec@netfilter.org,
	fw@strlen.de
Subject: Re: [PATCH net-next v2 3/3] ipv4: Centralize TOS matching
Date: Tue, 3 Sep 2024 11:02:34 +0200	[thread overview]
Message-ID: <ZtbQqsYIwo12AhOm@debian> (raw)
In-Reply-To: <ZtYNYTKSihLYowLO@shredder.mtl.com>

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/ZqYsrgnWwdQb1zgp@shredder.mtl.com/
> 
> 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
> >       |        ^~~~~
> 


  reply	other threads:[~2024-09-03  9:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-14 12:52 [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector Ido Schimmel
2024-08-14 12:52 ` [PATCH net-next v2 1/3] ipv4: Mask upper DSCP bits and ECN bits in NETLINK_FIB_LOOKUP family Ido Schimmel
2024-08-14 12:52 ` [PATCH net-next v2 2/3] netfilter: nft_fib: Mask upper DSCP bits before FIB lookup Ido Schimmel
2024-08-14 12:52 ` [PATCH net-next v2 3/3] ipv4: Centralize TOS matching Ido Schimmel
2024-08-20 14:42   ` Guillaume Nault
2024-09-02 16:50   ` David Ahern
2024-09-02 19:09     ` Ido Schimmel
2024-09-03  9:02       ` Guillaume Nault [this message]
2024-08-14 13:36 ` [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector Guillaume Nault
2024-08-20 13:14 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZtbQqsYIwo12AhOm@debian \
    --to=gnault@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=idosch@nvidia.com \
    --cc=kadlec@netfilter.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.