netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector
@ 2024-08-14 12:52 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
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Ido Schimmel @ 2024-08-14 12:52 UTC (permalink / raw)
  To: netdev, netfilter-devel
  Cc: davem, kuba, pabeni, edumazet, dsahern, gnault, pablo, kadlec, fw,
	Ido Schimmel

This patchset moves the masking of the upper DSCP bits in 'flowi4_tos'
to the core instead of relying on callers of the FIB lookup API to do
it.

This will allow us to start changing users of the API to initialize the
'flowi4_tos' field with all six bits of the DSCP field. In turn, this
will allow us to extend FIB rules with a new DSCP selector.

By masking the upper DSCP bits in the core we are able to maintain the
behavior of the TOS selector in FIB rules and routes to only match on
the lower DSCP bits.

While working on this I found two users of the API that do not mask the
upper DSCP bits before performing the lookup. The first is an ancient
netlink family that is unlikely to be used. It is adjusted in patch #1
to mask both the upper DSCP bits and the ECN bits before calling the
API.

The second user is a nftables module that differs in this regard from
its equivalent iptables module. It is adjusted in patch #2 to invoke the
API with the upper DSCP bits masked, like all other callers. The
relevant selftest passed, but in the unlikely case that regressions are
reported because of this change, we can restore the existing behavior
using a new flow information flag as discussed here [1].

The last patch moves the masking of the upper DSCP bits to the core,
making the first two patches redundant, but I wanted to post them
separately to call attention to the behavior change for these two users
of the FIB lookup API.

Future patchsets (around 3) will start unmasking the upper DSCP bits
throughout the networking stack before adding support for the new FIB
rule DSCP selector.

Changes from v1 [2]:

Patch #3: Include <linux/ip.h> in <linux/in_route.h> instead of
including it in net/ip_fib.h

[1] https://lore.kernel.org/netdev/ZpqpB8vJU%2FQ6LSqa@debian/
[2] https://lore.kernel.org/netdev/20240725131729.1729103-1-idosch@nvidia.com/

Ido Schimmel (3):
  ipv4: Mask upper DSCP bits and ECN bits in NETLINK_FIB_LOOKUP family
  netfilter: nft_fib: Mask upper DSCP bits before FIB lookup
  ipv4: Centralize TOS matching

 include/net/ip_fib.h              | 6 ++++++
 include/uapi/linux/in_route.h     | 2 ++
 net/ipv4/fib_frontend.c           | 2 +-
 net/ipv4/fib_rules.c              | 2 +-
 net/ipv4/fib_semantics.c          | 3 +--
 net/ipv4/fib_trie.c               | 3 +--
 net/ipv4/netfilter/nft_fib_ipv4.c | 4 +---
 7 files changed, 13 insertions(+), 9 deletions(-)

-- 
2.46.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next v2 1/3] ipv4: Mask upper DSCP bits and ECN bits in NETLINK_FIB_LOOKUP family
  2024-08-14 12:52 [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector Ido Schimmel
@ 2024-08-14 12:52 ` 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
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2024-08-14 12:52 UTC (permalink / raw)
  To: netdev, netfilter-devel
  Cc: davem, kuba, pabeni, edumazet, dsahern, gnault, pablo, kadlec, fw,
	Ido Schimmel

The NETLINK_FIB_LOOKUP netlink family can be used to perform a FIB
lookup according to user provided parameters and communicate the result
back to user space.

However, unlike other users of the FIB lookup API, the upper DSCP bits
and the ECN bits of the DS field are not masked, which can result in the
wrong result being returned.

Solve this by masking the upper DSCP bits and the ECN bits using
IPTOS_RT_MASK.

The structure that communicates the request and the response is not
exported to user space, so it is unlikely that this netlink family is
actually in use [1].

[1] https://lore.kernel.org/netdev/ZpqpB8vJU%2FQ6LSqa@debian/

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Guillaume Nault <gnault@redhat.com>
---
 net/ipv4/fib_frontend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 7ad2cafb9276..da540ddb7af6 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -1343,7 +1343,7 @@ static void nl_fib_lookup(struct net *net, struct fib_result_nl *frn)
 	struct flowi4           fl4 = {
 		.flowi4_mark = frn->fl_mark,
 		.daddr = frn->fl_addr,
-		.flowi4_tos = frn->fl_tos,
+		.flowi4_tos = frn->fl_tos & IPTOS_RT_MASK,
 		.flowi4_scope = frn->fl_scope,
 	};
 	struct fib_table *tb;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next v2 2/3] netfilter: nft_fib: Mask upper DSCP bits before FIB lookup
  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 ` Ido Schimmel
  2024-08-14 12:52 ` [PATCH net-next v2 3/3] ipv4: Centralize TOS matching Ido Schimmel
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2024-08-14 12:52 UTC (permalink / raw)
  To: netdev, netfilter-devel
  Cc: davem, kuba, pabeni, edumazet, dsahern, gnault, pablo, kadlec, fw,
	Ido Schimmel

As part of its functionality, the nftables FIB expression module
performs a FIB lookup, but unlike other users of the FIB lookup API, it
does so without masking the upper DSCP bits. In particular, this differs
from the equivalent iptables match ("rpfilter") that does mask the upper
DSCP bits before the FIB lookup.

Align the module to other users of the FIB lookup API and mask the upper
DSCP bits using IPTOS_RT_MASK before the lookup.

No regressions in nft_fib.sh:

 # ./nft_fib.sh
 PASS: fib expression did not cause unwanted packet drops
 PASS: fib expression did drop packets for 1.1.1.1
 PASS: fib expression did drop packets for 1c3::c01d
 PASS: fib expression forward check with policy based routing

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Guillaume Nault <gnault@redhat.com>
---
 net/ipv4/netfilter/nft_fib_ipv4.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
index 9eee535c64dd..df94bc28c3d7 100644
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -22,8 +22,6 @@ static __be32 get_saddr(__be32 addr)
 	return addr;
 }
 
-#define DSCP_BITS     0xfc
-
 void nft_fib4_eval_type(const struct nft_expr *expr, struct nft_regs *regs,
 			const struct nft_pktinfo *pkt)
 {
@@ -110,7 +108,7 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
 	if (priv->flags & NFTA_FIB_F_MARK)
 		fl4.flowi4_mark = pkt->skb->mark;
 
-	fl4.flowi4_tos = iph->tos & DSCP_BITS;
+	fl4.flowi4_tos = iph->tos & IPTOS_RT_MASK;
 
 	if (priv->flags & NFTA_FIB_F_DADDR) {
 		fl4.daddr = iph->daddr;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next v2 3/3] ipv4: Centralize TOS matching
  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 ` Ido Schimmel
  2024-08-20 14:42   ` Guillaume Nault
  2024-09-02 16:50   ` David Ahern
  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
  4 siblings, 2 replies; 10+ messages in thread
From: Ido Schimmel @ 2024-08-14 12:52 UTC (permalink / raw)
  To: netdev, netfilter-devel
  Cc: davem, kuba, pabeni, edumazet, dsahern, gnault, pablo, kadlec, fw,
	Ido Schimmel

The TOS field in the IPv4 flow information structure ('flowi4_tos') is
matched by the kernel against the TOS selector in IPv4 rules and routes.
The field is initialized differently by different call sites. Some treat
it as DSCP (RFC 2474) and initialize all six DSCP bits, some treat it as
RFC 1349 TOS and initialize it using RT_TOS() and some treat it as RFC
791 TOS and initialize it using IPTOS_RT_MASK.

What is common to all these call sites is that they all initialize the
lower three DSCP bits, which fits the TOS definition in the initial IPv4
specification (RFC 791).

Therefore, the kernel only allows configuring IPv4 FIB rules that match
on the lower three DSCP bits which are always guaranteed to be
initialized by all call sites:

 # ip -4 rule add tos 0x1c table 100
 # ip -4 rule add tos 0x3c table 100
 Error: Invalid tos.

While this works, it is unlikely to be very useful. RFC 791 that
initially defined the TOS and IP precedence fields was updated by RFC
2474 over twenty five years ago where these fields were replaced by a
single six bits DSCP field.

Extending FIB rules to match on DSCP can be done by adding a new DSCP
selector while maintaining the existing semantics of the TOS selector
for applications that rely on that.

A prerequisite for allowing FIB rules to match on DSCP is to adjust all
the call sites to initialize the high order DSCP bits and remove their
masking along the path to the core where the field is matched on.

However, making this change alone will result in a behavior change. For
example, a forwarded IPv4 packet with a DS field of 0xfc will no longer
match a FIB rule that was configured with 'tos 0x1c'.

This behavior change can be avoided by masking the upper three DSCP bits
in 'flowi4_tos' before comparing it against the TOS selectors in FIB
rules and routes.

Implement the above by adding a new function that checks whether a given
DSCP value matches the one specified in the IPv4 flow information
structure and invoke it from the three places that currently match on
'flowi4_tos'.

Use RT_TOS() for the masking of 'flowi4_tos' instead of IPTOS_RT_MASK
since the latter is not uAPI and we should be able to remove it at some
point.

Include <linux/ip.h> in <linux/in_route.h> since the former defines
IPTOS_TOS_MASK which is used in the definition of RT_TOS() in
<linux/in_route.h>.

No regressions in FIB tests:

 # ./fib_tests.sh
 [...]
 Tests passed: 218
 Tests failed:   0

And FIB rule tests:

 # ./fib_rule_tests.sh
 [...]
 Tests passed: 116
 Tests failed:   0

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
v2:

Include <linux/ip.h> in <linux/in_route.h> instead of including it in
net/ip_fib.h
---
 include/net/ip_fib.h          | 6 ++++++
 include/uapi/linux/in_route.h | 2 ++
 net/ipv4/fib_rules.c          | 2 +-
 net/ipv4/fib_semantics.c      | 3 +--
 net/ipv4/fib_trie.c           | 3 +--
 5 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 72af2f223e59..269ec10f63e4 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/in_route.h>
 
 struct fib_config {
 	u8			fc_dst_len;
@@ -434,6 +435,11 @@ static inline bool fib4_rules_early_flow_dissect(struct net *net,
 
 #endif /* CONFIG_IP_MULTIPLE_TABLES */
 
+static inline bool fib_dscp_masked_match(dscp_t dscp, const struct flowi4 *fl4)
+{
+	return dscp == inet_dsfield_to_dscp(RT_TOS(fl4->flowi4_tos));
+}
+
 /* Exported by fib_frontend.c */
 extern const struct nla_policy rtm_ipv4_policy[];
 void ip_fib_init(void);
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
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 5bdd1c016009..c26776b71e97 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -186,7 +186,7 @@ INDIRECT_CALLABLE_SCOPE int fib4_rule_match(struct fib_rule *rule,
 	    ((daddr ^ r->dst) & r->dstmask))
 		return 0;
 
-	if (r->dscp && r->dscp != inet_dsfield_to_dscp(fl4->flowi4_tos))
+	if (r->dscp && !fib_dscp_masked_match(r->dscp, fl4))
 		return 0;
 
 	if (rule->ip_proto && (rule->ip_proto != fl4->flowi4_proto))
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 2b57cd2b96e2..0f70341cb8b5 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -2066,8 +2066,7 @@ static void fib_select_default(const struct flowi4 *flp, struct fib_result *res)
 
 		if (fa->fa_slen != slen)
 			continue;
-		if (fa->fa_dscp &&
-		    fa->fa_dscp != inet_dsfield_to_dscp(flp->flowi4_tos))
+		if (fa->fa_dscp && !fib_dscp_masked_match(fa->fa_dscp, flp))
 			continue;
 		if (fa->tb_id != tb->tb_id)
 			continue;
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 8f30e3f00b7f..09e31757e96c 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1580,8 +1580,7 @@ int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
 			if (index >= (1ul << fa->fa_slen))
 				continue;
 		}
-		if (fa->fa_dscp &&
-		    inet_dscp_to_dsfield(fa->fa_dscp) != flp->flowi4_tos)
+		if (fa->fa_dscp && !fib_dscp_masked_match(fa->fa_dscp, flp))
 			continue;
 		/* Paired with WRITE_ONCE() in fib_release_info() */
 		if (READ_ONCE(fi->fib_dead))
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector
  2024-08-14 12:52 [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector Ido Schimmel
                   ` (2 preceding siblings ...)
  2024-08-14 12:52 ` [PATCH net-next v2 3/3] ipv4: Centralize TOS matching Ido Schimmel
@ 2024-08-14 13:36 ` Guillaume Nault
  2024-08-20 13:14 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 10+ messages in thread
From: Guillaume Nault @ 2024-08-14 13:36 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, netfilter-devel, davem, kuba, pabeni, edumazet, dsahern,
	pablo, kadlec, fw

On Wed, Aug 14, 2024 at 03:52:21PM +0300, Ido Schimmel wrote:
> This patchset moves the masking of the upper DSCP bits in 'flowi4_tos'
> to the core instead of relying on callers of the FIB lookup API to do
> it.

FWIW, I plan to review this patch set next week (I'm mostly offline
this week).


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector
  2024-08-14 12:52 [PATCH net-next v2 0/3] Preparations for FIB rule DSCP selector Ido Schimmel
                   ` (3 preceding siblings ...)
  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
  4 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-08-20 13:14 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, netfilter-devel, davem, kuba, pabeni, edumazet, dsahern,
	gnault, pablo, kadlec, fw

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 14 Aug 2024 15:52:21 +0300 you wrote:
> This patchset moves the masking of the upper DSCP bits in 'flowi4_tos'
> to the core instead of relying on callers of the FIB lookup API to do
> it.
> 
> This will allow us to start changing users of the API to initialize the
> 'flowi4_tos' field with all six bits of the DSCP field. In turn, this
> will allow us to extend FIB rules with a new DSCP selector.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] ipv4: Mask upper DSCP bits and ECN bits in NETLINK_FIB_LOOKUP family
    https://git.kernel.org/netdev/net-next/c/8fed54758cd2
  - [net-next,v2,2/3] netfilter: nft_fib: Mask upper DSCP bits before FIB lookup
    https://git.kernel.org/netdev/net-next/c/548a2029eb66
  - [net-next,v2,3/3] ipv4: Centralize TOS matching
    https://git.kernel.org/netdev/net-next/c/1fa3314c14c6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v2 3/3] ipv4: Centralize TOS matching
  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
  1 sibling, 0 replies; 10+ messages in thread
From: Guillaume Nault @ 2024-08-20 14:42 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, netfilter-devel, davem, kuba, pabeni, edumazet, dsahern,
	pablo, kadlec, fw

On Wed, Aug 14, 2024 at 03:52:24PM +0300, Ido Schimmel wrote:
> The TOS field in the IPv4 flow information structure ('flowi4_tos') is
> matched by the kernel against the TOS selector in IPv4 rules and routes.
> The field is initialized differently by different call sites. Some treat
> it as DSCP (RFC 2474) and initialize all six DSCP bits, some treat it as
> RFC 1349 TOS and initialize it using RT_TOS() and some treat it as RFC
> 791 TOS and initialize it using IPTOS_RT_MASK.
> 
> What is common to all these call sites is that they all initialize the
> lower three DSCP bits, which fits the TOS definition in the initial IPv4
> specification (RFC 791).
> 
> Therefore, the kernel only allows configuring IPv4 FIB rules that match
> on the lower three DSCP bits which are always guaranteed to be
> initialized by all call sites:
> 
>  # ip -4 rule add tos 0x1c table 100
>  # ip -4 rule add tos 0x3c table 100
>  Error: Invalid tos.
> 
> While this works, it is unlikely to be very useful. RFC 791 that
> initially defined the TOS and IP precedence fields was updated by RFC
> 2474 over twenty five years ago where these fields were replaced by a
> single six bits DSCP field.
> 
> Extending FIB rules to match on DSCP can be done by adding a new DSCP
> selector while maintaining the existing semantics of the TOS selector
> for applications that rely on that.
> 
> A prerequisite for allowing FIB rules to match on DSCP is to adjust all
> the call sites to initialize the high order DSCP bits and remove their
> masking along the path to the core where the field is matched on.
> 
> However, making this change alone will result in a behavior change. For
> example, a forwarded IPv4 packet with a DS field of 0xfc will no longer
> match a FIB rule that was configured with 'tos 0x1c'.
> 
> This behavior change can be avoided by masking the upper three DSCP bits
> in 'flowi4_tos' before comparing it against the TOS selectors in FIB
> rules and routes.
> 
> Implement the above by adding a new function that checks whether a given
> DSCP value matches the one specified in the IPv4 flow information
> structure and invoke it from the three places that currently match on
> 'flowi4_tos'.

A bit late for the review, but anyway...

Reviewed-by: Guillaume Nault <gnault@redhat.com>

Thanks Ido!


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v2 3/3] ipv4: Centralize TOS matching
  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
  1 sibling, 1 reply; 10+ messages in thread
From: David Ahern @ 2024-09-02 16:50 UTC (permalink / raw)
  To: Ido Schimmel, netdev, netfilter-devel
  Cc: davem, kuba, pabeni, edumazet, dsahern, gnault, pablo, kadlec, fw

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):

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
      |        ^~~~~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v2 3/3] ipv4: Centralize TOS matching
  2024-09-02 16:50   ` David Ahern
@ 2024-09-02 19:09     ` Ido Schimmel
  2024-09-03  9:02       ` Guillaume Nault
  0 siblings, 1 reply; 10+ messages in thread
From: Ido Schimmel @ 2024-09-02 19:09 UTC (permalink / raw)
  To: David Ahern, gnault
  Cc: netdev, netfilter-devel, davem, kuba, pabeni, edumazet, pablo,
	kadlec, fw

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]?

[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
>       |        ^~~~~

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next v2 3/3] ipv4: Centralize TOS matching
  2024-09-02 19:09     ` Ido Schimmel
@ 2024-09-03  9:02       ` Guillaume Nault
  0 siblings, 0 replies; 10+ messages in thread
From: Guillaume Nault @ 2024-09-03  9:02 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: David Ahern, netdev, netfilter-devel, davem, kuba, pabeni,
	edumazet, pablo, kadlec, fw

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
> >       |        ^~~~~
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-09-03  9:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).