ipv4: Remove RTO_ONLINK from udp, ping and raw sockets.

PATCH Applied Guillaume Nault 3 participants 8 messages May 22, 2023 – May 24, 2023

Subsystems (4)

Networking Network drivers IPv4/IPv6 UDP

lore

Share & tools

lore

Guillaume Nault

author

2023-05-22 14:37 UTC #0 lore raw

cover ipv4: Remove RTO_ONLINK from udp, ping and raw sockets.

udp_sendmsg(), ping_v4_sendmsg() and raw_sendmsg() use similar patterns for restricting their route lookup to on-link hosts. Although they use slightly different code, they all use RTO_ONLINK to override the least significant bit of their tos value.

RTO_ONLINK is used to restrict the route scope even when the scope is set to RT_SCOPE_UNIVERSE. Therefore it isn't necessary: we can properly set the scope to RT_SCOPE_LINK instead.

Removing RTO_ONLINK will allow to convert .flowi4_tos to dscp_t in the future, thus allowing to properly separate the DSCP from the ECN bits in the networking stack.

This patch series defines a common helper to figure out what's the scope of the route lookup. This unifies the way udp, ping and raw sockets get their routing scope and removes their dependency on RTO_ONLINK.

Guillaume Nault (3):
ping: Stop using RTO_ONLINK.

  raw: Stop using RTO_ONLINK.
  udp: Stop using RTO_ONLINK.

 include/net/ip.h | 16 ++++++++++++----
 net/ipv4/ping.c  | 15 +++++----------
 net/ipv4/raw.c   | 10 ++++------
 net/ipv4/udp.c   | 17 ++++++-----------
 4 files changed, 27 insertions(+), 31 deletions(-)

Signature · 1 line

2.39.2

Guillaume Nault

author

2023-05-22 14:38 UTC #1 lore raw

PATCH 1/3 ping: Stop using RTO_ONLINK.

Define a new helper to figure out the correct route scope to use on TX, depending on socket configuration, ancillary data and send flags.

Use this new helper to properly initialise the scope in flowi4_init_output(), instead of overriding tos with the RTO_ONLINK flag.

The objective is to eventually remove RTO_ONLINK, which will allow converting .flowi4_tos to dscp_t.

Signed-off-by Guillaume Nault <[email protected]>

---
 include/net/ip.h | 13 +++++++++++++
 net/ipv4/ping.c  | 15 +++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

include/net/ip.h +13 -0

index c3fffaa92d6e..6e262efa0d55 100644
@@ -242,6 +242,19 @@ static inline struct sk_buff *ip_finish_skb(struct sock *sk, struct flowi4 *fl4)
 	return __ip_make_skb(sk, fl4, &sk->sk_write_queue, &inet_sk(sk)->cork.base);
 }
 
+/* Get the route scope that should be used when sending a packet. */
+static inline u8 ip_sendmsg_scope(const struct inet_sock *inet,
+				  const struct ipcm_cookie *ipc,
+				  const struct msghdr *msg)
+{
+	if (sock_flag(&inet->sk, SOCK_LOCALROUTE) ||
+	    msg->msg_flags & MSG_DONTROUTE ||
+	    (ipc->opt && ipc->opt->opt.is_strictroute))
+		return RT_SCOPE_LINK;
+
+	return RT_SCOPE_UNIVERSE;
+}
+
 static inline __u8 get_rttos(struct ipcm_cookie* ipc, struct inet_sock *inet)
 {
 	return (ipc->tos != -1) ? RT_TOS(ipc->tos) : RT_TOS(inet->tos);

net/ipv4/ping.c +5 ~~-10~~

index 3793c81bda8a..25dd78cee179 100644
@@ -705,7 +705,7 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	struct ip_options_data opt_copy;
 	int free = 0;
 	__be32 saddr, daddr, faddr;
-	u8  tos;
+	u8 tos, scope;
 	int err;
 
 	pr_debug("ping_v4_sendmsg(sk=%p,sk->num=%u)\n", inet, inet->inet_num);

@@ -769,11 +769,7 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		faddr = ipc.opt->opt.faddr;
 	}
 	tos = get_rttos(&ipc, inet);
-	if (sock_flag(sk, SOCK_LOCALROUTE) ||
-	    (msg->msg_flags & MSG_DONTROUTE) ||
-	    (ipc.opt && ipc.opt->opt.is_strictroute)) {
-		tos |= RTO_ONLINK;
-	}
+	scope = ip_sendmsg_scope(inet, &ipc, msg);
 
 	if (ipv4_is_multicast(daddr)) {
 		if (!ipc.oif || netif_index_is_l3_master(sock_net(sk), ipc.oif))

@@ -783,10 +779,9 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	} else if (!ipc.oif)
 		ipc.oif = inet->uc_index;
 
-	flowi4_init_output(&fl4, ipc.oif, ipc.sockc.mark, tos,
-			   RT_SCOPE_UNIVERSE, sk->sk_protocol,
-			   inet_sk_flowi_flags(sk), faddr, saddr, 0, 0,
-			   sk->sk_uid);
+	flowi4_init_output(&fl4, ipc.oif, ipc.sockc.mark, tos, scope,
+			   sk->sk_protocol, inet_sk_flowi_flags(sk), faddr,
+			   saddr, 0, 0, sk->sk_uid);
 
 	fl4.fl4_icmp_type = user_icmph.type;
 	fl4.fl4_icmp_code = user_icmph.code;

Signature · 1 line

2.39.2

David Ahern

maintainer Reviewed-by on v1 1/3

2023-05-23 15:09 UTC #2 lore raw

Re: [PATCH net-next 1/3] ping: Stop using RTO_ONLINK.

Quoted 7 lines · Guillaume Nault

Define a new helper to figure out the correct route scope to use on TX, depending on socket configuration, ancillary data and send flags.
Use this new helper to properly initialise the scope in flowi4_init_output(), instead of overriding tos with the RTO_ONLINK flag.

Quoted 9 lines

The objective is to eventually remove RTO_ONLINK, which will allow converting .flowi4_tos to dscp_t.
Signed-off-by Guillaume Nault <[email protected]>
---
 include/net/ip.h | 13 +++++++++++++
 net/ipv4/ping.c  | 15 +++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

Reviewed-by David Ahern <[email protected]>

Guillaume Nault

author

2023-05-22 14:38 UTC #3 lore raw

PATCH 2/3 raw: Stop using RTO_ONLINK.

In reply to Guillaume Nault

Use ip_sendmsg_scope() to properly initialise the scope in flowi4_init_output(), instead of overriding tos with the RTO_ONLINK flag. The objective is to eventually remove RTO_ONLINK, which will allow converting .flowi4_tos to dscp_t.

The MSG_DONTROUTE and SOCK_LOCALROUTE cases were already handled by raw_sendmsg() (SOCK_LOCALROUTE was handled by the RT_CONN_FLAGS*() macros called by get_rtconn_flags()). However, opt.is_strictroute wasn't taken into account. Therefore, a side effect of this patch is to now honour opt.is_strictroute, and thus align raw_sendmsg() with ping_v4_sendmsg() and udp_sendmsg().

Since raw_sendmsg() was the only user of get_rtconn_flags(), we can now remove this function.

Signed-off-by Guillaume Nault <[email protected]>

---
 include/net/ip.h |  5 -----
 net/ipv4/raw.c   | 10 ++++------
 2 files changed, 4 insertions(+), 11 deletions(-)

include/net/ip.h +0 -5

index 6e262efa0d55..fab910be252c 100644
@@ -260,11 +260,6 @@ static inline __u8 get_rttos(struct ipcm_cookie* ipc, struct inet_sock *inet)
 	return (ipc->tos != -1) ? RT_TOS(ipc->tos) : RT_TOS(inet->tos);
 }
 
-static inline __u8 get_rtconn_flags(struct ipcm_cookie* ipc, struct sock* sk)
-{
-	return (ipc->tos != -1) ? RT_CONN_FLAGS_TOS(sk, ipc->tos) : RT_CONN_FLAGS(sk);
-}
-
 /* datagram.c */
 int __ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len);
 int ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len);

net/ipv4/raw.c +4 -6

index ff712bf2a98d..8b7b5c842bdd 100644
@@ -476,10 +476,10 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	struct ipcm_cookie ipc;
 	struct rtable *rt = NULL;
 	struct flowi4 fl4;
+	u8 tos, scope;
 	int free = 0;
 	__be32 daddr;
 	__be32 saddr;
-	u8  tos;
 	int err;
 	struct ip_options_data opt_copy;
 	struct raw_frag_vec rfv;

@@ -572,9 +572,8 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 			daddr = ipc.opt->opt.faddr;
 		}
 	}
-	tos = get_rtconn_flags(&ipc, sk);
-	if (msg->msg_flags & MSG_DONTROUTE)
-		tos |= RTO_ONLINK;
+	tos = get_rttos(&ipc, inet);
+	scope = ip_sendmsg_scope(inet, &ipc, msg);
 
 	if (ipv4_is_multicast(daddr)) {
 		if (!ipc.oif || netif_index_is_l3_master(sock_net(sk), ipc.oif))

@@ -597,8 +596,7 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		}
 	}
 
-	flowi4_init_output(&fl4, ipc.oif, ipc.sockc.mark, tos,
-			   RT_SCOPE_UNIVERSE,
+	flowi4_init_output(&fl4, ipc.oif, ipc.sockc.mark, tos, scope,
 			   hdrincl ? IPPROTO_RAW : sk->sk_protocol,
 			   inet_sk_flowi_flags(sk) |
 			    (hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),

Signature · 1 line

2.39.2

David Ahern

maintainer Reviewed-by on v1 2/3

2023-05-23 15:13 UTC #4 lore raw

Re: [PATCH net-next 2/3] raw: Stop using RTO_ONLINK.

Quoted 12 lines · Guillaume Nault

Use ip_sendmsg_scope() to properly initialise the scope in flowi4_init_output(), instead of overriding tos with the RTO_ONLINK flag. The objective is to eventually remove RTO_ONLINK, which will allow converting .flowi4_tos to dscp_t.
The MSG_DONTROUTE and SOCK_LOCALROUTE cases were already handled by raw_sendmsg() (SOCK_LOCALROUTE was handled by the RT_CONN_FLAGS*() macros called by get_rtconn_flags()). However, opt.is_strictroute wasn't taken into account. Therefore, a side effect of this patch is to now honour opt.is_strictroute, and thus align raw_sendmsg() with ping_v4_sendmsg() and udp_sendmsg().

Quoted 9 lines

Since raw_sendmsg() was the only user of get_rtconn_flags(), we can now remove this function.
Signed-off-by Guillaume Nault <[email protected]>
---
 include/net/ip.h |  5 -----
 net/ipv4/raw.c   | 10 ++++------
 2 files changed, 4 insertions(+), 11 deletions(-)

Reviewed-by David Ahern <[email protected]>

Guillaume Nault

author

2023-05-22 14:38 UTC #5 lore raw

PATCH 3/3 udp: Stop using RTO_ONLINK.

In reply to Guillaume Nault

Now that the scope is determined by ip_sendmsg_scope(), we need to check its result to set the 'connected' variable.

Signed-off-by Guillaume Nault <[email protected]>

---
 net/ipv4/udp.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

net/ipv4/udp.c +6 ~~-11~~

index aa32afd871ee..64750baa55d6 100644
@@ -1062,8 +1062,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int free = 0;
 	int connected = 0;
 	__be32 daddr, faddr, saddr;
+	u8 tos, scope;
 	__be16 dport;
-	u8  tos;
 	int err, is_udplite = IS_UDPLITE(sk);
 	int corkreq = READ_ONCE(up->corkflag) || msg->msg_flags&MSG_MORE;
 	int (*getfrag)(void *, char *, int, int, int, struct sk_buff *);

@@ -1183,12 +1183,9 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		connected = 0;
 	}
 	tos = get_rttos(&ipc, inet);
-	if (sock_flag(sk, SOCK_LOCALROUTE) ||
-	    (msg->msg_flags & MSG_DONTROUTE) ||
-	    (ipc.opt && ipc.opt->opt.is_strictroute)) {
-		tos |= RTO_ONLINK;
+	scope = ip_sendmsg_scope(inet, &ipc, msg);
+	if (scope == RT_SCOPE_LINK)
 		connected = 0;
-	}
 
 	if (ipv4_is_multicast(daddr)) {
 		if (!ipc.oif || netif_index_is_l3_master(sock_net(sk), ipc.oif))

@@ -1221,11 +1218,9 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 		fl4 = &fl4_stack;
 
-		flowi4_init_output(fl4, ipc.oif, ipc.sockc.mark, tos,
-				   RT_SCOPE_UNIVERSE, sk->sk_protocol,
-				   flow_flags,
-				   faddr, saddr, dport, inet->inet_sport,
-				   sk->sk_uid);
+		flowi4_init_output(fl4, ipc.oif, ipc.sockc.mark, tos, scope,
+				   sk->sk_protocol, flow_flags, faddr, saddr,
+				   dport, inet->inet_sport, sk->sk_uid);
 
 		security_sk_classify_flow(sk, flowi4_to_flowi_common(fl4));
 		rt = ip_route_output_flow(net, fl4, sk);

Signature · 1 line

2.39.2

David Ahern

maintainer Reviewed-by on v1 3/3

2023-05-23 15:14 UTC #6 lore raw

Re: [PATCH net-next 3/3] udp: Stop using RTO_ONLINK.

Quoted 5 lines · Guillaume Nault

Use ip_sendmsg_scope() to properly initialise the scope in flowi4_init_output(), instead of overriding tos with the RTO_ONLINK flag. The objective is to eventually remove RTO_ONLINK, which will allow converting .flowi4_tos to dscp_t.

Quoted 8 lines

Now that the scope is determined by ip_sendmsg_scope(), we need to check its result to set the 'connected' variable.
Signed-off-by Guillaume Nault <[email protected]>
---
 net/ipv4/udp.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Reviewed-by David Ahern <[email protected]>

[email protected]

CI Applied

2023-05-24 07:30 UTC #7 lore raw

In reply to Guillaume Nault

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Quoted 10 lines · On Mon, 22 May 2023 16:37:50 +0200 you wrote:

udp_sendmsg(), ping_v4_sendmsg() and raw_sendmsg() use similar patterns for restricting their route lookup to on-link hosts. Although they use slightly different code, they all use RTO_ONLINK to override the least significant bit of their tos value.
RTO_ONLINK is used to restrict the route scope even when the scope is set to RT_SCOPE_UNIVERSE. Therefore it isn't necessary: we can properly set the scope to RT_SCOPE_LINK instead.
[...]

Here is the summary with links:
- [net-next,1/3] ping: Stop using RTO_ONLINK.

    https://git.kernel.org/netdev/net-next/c/726de790f660
  - [net-next,2/3] raw: Stop using RTO_ONLINK.
    https://git.kernel.org/netdev/net-next/c/c85be08fc4fa
  - [net-next,3/3] udp: Stop using RTO_ONLINK.
    https://git.kernel.org/netdev/net-next/c/0e26371db548

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html