[PATCH RFC 0/3] ipv4: preparation work for making ipv4 routing ecn robust

[PATCH RFC 0/3] ipv4: preparation work for making ipv4 routing ecn robust

[Hannes Frederic Sowa]

Probably out of laziness we encoded the RTO_ONLINK flag into the
lowest ToS bit to easier pass it around in the flowi structure (we
need one less argument to do so).

This series cleans it up and uses flowi4_scope correctly from nearly
the top-level, thus removing the need to encode the RTO_ONLINK flag
into tos. This makes it later easier to verify that we correctly use
the ToS bit. Currently we are leaking one ECN bit into the routing
engine, which could make different decisions based on ECN signaling.

This is the first clean-up preparational work to later on fix the
RT_TOS macros.

My question is, if this is acceptable. Especially because it removes
the flowi4_init_output and flowi4_update_output functions and expands
them inline. I did so to easy code review with grepping and finally
not being forced to count the arguments.

Opinions before I go on? Thanks!

Hannes Frederic Sowa (3):
  ipv4: make flow functions more grepable by removing flowi4_init_output
  ipv4: make flow functions more grepable by removing
    flowi4_update_output
  ipv4: kill RTO_ONLINK, RT_CONN_FLAGS and RT_CONN_FLAGS_TOS

 drivers/net/bonding/bond_main.c |  3 +-
 drivers/net/gtp.c               |  3 +-
 drivers/net/ppp/pptp.c          |  2 +-
 drivers/net/vrf.c               |  7 ----
 include/net/flow.h              | 32 -------------------
 include/net/ip.h                |  5 ---
 include/net/route.h             | 71 +++++++++++++++++++++++++++++++----------
 net/dccp/ipv4.c                 |  5 +--
 net/ipv4/af_inet.c              |  4 +--
 net/ipv4/arp.c                  |  4 +--
 net/ipv4/datagram.c             |  5 +--
 net/ipv4/inet_connection_sock.c | 44 +++++++++++++++++--------
 net/ipv4/ip_output.c            | 23 ++++++++-----
 net/ipv4/ping.c                 | 20 +++++++++---
 net/ipv4/raw.c                  | 31 ++++++++++++------
 net/ipv4/route.c                | 50 +++++++++++++++++++----------
 net/ipv4/syncookies.c           | 19 ++++++++---
 net/ipv4/tcp_ipv4.c             |  2 +-
 net/ipv4/udp.c                  | 21 ++++++++----
 net/l2tp/l2tp_ip.c              |  3 +-
 net/netfilter/ipvs/ip_vs_xmit.c |  9 ++++--
 net/sctp/protocol.c             | 14 ++++----
 22 files changed, 230 insertions(+), 147 deletions(-)

-- 
2.5.5

[PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[Hannes Frederic Sowa]

No functional changes.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/flow.h              | 21 ---------------------
 include/net/route.h             | 33 +++++++++++++++++++++++++++------
 net/ipv4/inet_connection_sock.c | 41 +++++++++++++++++++++++++++++------------
 net/ipv4/ip_output.c            | 21 ++++++++++++++-------
 net/ipv4/ping.c                 | 16 +++++++++++++---
 net/ipv4/raw.c                  | 20 ++++++++++++++------
 net/ipv4/route.c                | 38 +++++++++++++++++++++++++++++---------
 net/ipv4/syncookies.c           | 19 ++++++++++++++-----
 net/ipv4/udp.c                  | 17 +++++++++++++----
 9 files changed, 153 insertions(+), 73 deletions(-)

diff --git a/include/net/flow.h b/include/net/flow.h
index d47ef4bb5423a3..2c8e95b987c98c 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -90,27 +90,6 @@ struct flowi4 {
 #define fl4_gre_key		uli.gre_key
 } __attribute__((__aligned__(BITS_PER_LONG/8)));
 
-static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
-				      __u32 mark, __u8 tos, __u8 scope,
-				      __u8 proto, __u8 flags,
-				      __be32 daddr, __be32 saddr,
-				      __be16 dport, __be16 sport)
-{
-	fl4->flowi4_oif = oif;
-	fl4->flowi4_iif = LOOPBACK_IFINDEX;
-	fl4->flowi4_mark = mark;
-	fl4->flowi4_tos = tos;
-	fl4->flowi4_scope = scope;
-	fl4->flowi4_proto = proto;
-	fl4->flowi4_flags = flags;
-	fl4->flowi4_secid = 0;
-	fl4->flowi4_tun_key.tun_id = 0;
-	fl4->daddr = daddr;
-	fl4->saddr = saddr;
-	fl4->fl4_dport = dport;
-	fl4->fl4_sport = sport;
-}
-
 /* Reset some input parameters after previous lookup */
 static inline void flowi4_update_output(struct flowi4 *fl4, int oif, __u8 tos,
 					__be32 daddr, __be32 saddr)
diff --git a/include/net/route.h b/include/net/route.h
index ad777d79af9458..61164593d41ecf 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -151,10 +151,20 @@ static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi
 						   __be16 dport, __be16 sport,
 						   __u8 proto, __u8 tos, int oif)
 {
-	flowi4_init_output(fl4, oif, sk ? sk->sk_mark : 0, tos,
-			   RT_SCOPE_UNIVERSE, proto,
-			   sk ? inet_sk_flowi_flags(sk) : 0,
-			   daddr, saddr, dport, sport);
+	fl4->flowi4_oif = oif;
+	fl4->flowi4_iif = LOOPBACK_IFINDEX;
+	fl4->flowi4_mark = sk ? sk->sk_mark : 0;
+	fl4->flowi4_tos = tos;
+	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_proto = proto;
+	fl4->flowi4_flags = sk ? inet_sk_flowi_flags(sk) : 0;
+	fl4->flowi4_secid = 0;
+	fl4->flowi4_tun_key.tun_id = 0;
+	fl4->daddr = daddr;
+	fl4->saddr = saddr;
+	fl4->fl4_dport = dport;
+	fl4->fl4_sport = sport;
+
 	if (sk)
 		security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
 	return ip_route_output_flow(net, fl4, sk);
@@ -269,8 +279,19 @@ static inline void ip_route_connect_init(struct flowi4 *fl4, __be32 dst, __be32
 	if (inet_sk(sk)->transparent)
 		flow_flags |= FLOWI_FLAG_ANYSRC;
 
-	flowi4_init_output(fl4, oif, sk->sk_mark, tos, RT_SCOPE_UNIVERSE,
-			   protocol, flow_flags, dst, src, dport, sport);
+	fl4->flowi4_oif = oif;
+	fl4->flowi4_iif = LOOPBACK_IFINDEX;
+	fl4->flowi4_mark = sk->sk_mark;
+	fl4->flowi4_tos = tos;
+	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_proto = protocol;
+	fl4->flowi4_flags = flow_flags;
+	fl4->flowi4_secid = 0;
+	fl4->flowi4_tun_key.tun_id = 0;
+	fl4->daddr = dst;
+	fl4->saddr = src;
+	fl4->fl4_dport = dport;
+	fl4->fl4_sport = sport;
 }
 
 static inline struct rtable *ip_route_connect(struct flowi4 *fl4,
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index fa8c39804bdbae..f84b851e050534 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -410,12 +410,20 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
 	struct ip_options_rcu *opt = ireq->opt;
 	struct rtable *rt;
 
-	flowi4_init_output(fl4, ireq->ir_iif, ireq->ir_mark,
-			   RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
-			   sk->sk_protocol, inet_sk_flowi_flags(sk),
-			   (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr,
-			   ireq->ir_loc_addr, ireq->ir_rmt_port,
-			   htons(ireq->ir_num));
+	fl4->flowi4_oif = ireq->ir_iif;
+	fl4->flowi4_iif = LOOPBACK_IFINDEX;
+	fl4->flowi4_mark = ireq->ir_mark;
+	fl4->flowi4_tos = RT_CONN_FLAGS(sk);
+	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_proto = sk->sk_protocol;
+	fl4->flowi4_flags = inet_sk_flowi_flags(sk);
+	fl4->flowi4_secid = 0;
+	fl4->flowi4_tun_key.tun_id = 0;
+	fl4->daddr = (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr;
+	fl4->saddr = ireq->ir_loc_addr;
+	fl4->fl4_dport = ireq->ir_rmt_port;
+	fl4->fl4_sport = htons(ireq->ir_num);
+
 	security_req_classify_flow(req, flowi4_to_flowi(fl4));
 	rt = ip_route_output_flow(net, fl4, sk);
 	if (IS_ERR(rt))
@@ -447,12 +455,21 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk,
 
 	rcu_read_lock();
 	opt = rcu_dereference(newinet->inet_opt);
-	flowi4_init_output(fl4, ireq->ir_iif, ireq->ir_mark,
-			   RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
-			   sk->sk_protocol, inet_sk_flowi_flags(sk),
-			   (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr,
-			   ireq->ir_loc_addr, ireq->ir_rmt_port,
-			   htons(ireq->ir_num));
+
+	fl4->flowi4_oif = ireq->ir_iif;
+	fl4->flowi4_iif = LOOPBACK_IFINDEX;
+	fl4->flowi4_mark = ireq->ir_mark;
+	fl4->flowi4_tos = RT_CONN_FLAGS(sk);
+	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_proto = sk->sk_protocol;
+	fl4->flowi4_flags = inet_sk_flowi_flags(sk);
+	fl4->flowi4_secid = 0;
+	fl4->flowi4_tun_key.tun_id = 0;
+	fl4->daddr = (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr;
+	fl4->saddr = ireq->ir_loc_addr;
+	fl4->fl4_dport = ireq->ir_rmt_port;
+	fl4->fl4_sport = htons(ireq->ir_num);
+
 	security_req_classify_flow(req, flowi4_to_flowi(fl4));
 	rt = ip_route_output_flow(net, fl4, sk);
 	if (IS_ERR(rt))
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index cbac493c913ac3..90076310888d7f 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1567,13 +1567,20 @@ void ip_send_unicast_reply(struct sock *sk, struct sk_buff *skb,
 	if (!oif && netif_index_is_l3_master(net, skb->skb_iif))
 		oif = skb->skb_iif;
 
-	flowi4_init_output(&fl4, oif,
-			   IP4_REPLY_MARK(net, skb->mark),
-			   RT_TOS(arg->tos),
-			   RT_SCOPE_UNIVERSE, ip_hdr(skb)->protocol,
-			   ip_reply_arg_flowi_flags(arg),
-			   daddr, saddr,
-			   tcp_hdr(skb)->source, tcp_hdr(skb)->dest);
+	fl4.flowi4_oif = oif;
+	fl4.flowi4_iif = LOOPBACK_IFINDEX;
+	fl4.flowi4_mark = IP4_REPLY_MARK(net, skb->mark);
+	fl4.flowi4_tos = RT_TOS(arg->tos);
+	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_proto = ip_hdr(skb)->protocol;
+	fl4.flowi4_flags = ip_reply_arg_flowi_flags(arg);
+	fl4.flowi4_secid = 0;
+	fl4.flowi4_tun_key.tun_id = 0;
+	fl4.daddr = daddr;
+	fl4.saddr = saddr;
+	fl4.fl4_dport = tcp_hdr(skb)->source;
+	fl4.fl4_sport = tcp_hdr(skb)->dest;
+
 	security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
 	rt = ip_route_output_key(net, &fl4);
 	if (IS_ERR(rt))
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 66ddcb60519a16..dc6a42d1136752 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -792,9 +792,19 @@ 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, sk->sk_mark, tos,
-			   RT_SCOPE_UNIVERSE, sk->sk_protocol,
-			   inet_sk_flowi_flags(sk), faddr, saddr, 0, 0);
+	fl4.flowi4_oif = ipc.oif;
+	fl4.flowi4_iif = LOOPBACK_IFINDEX;
+	fl4.flowi4_mark = sk->sk_mark;
+	fl4.flowi4_tos = tos;
+	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_proto = sk->sk_protocol;
+	fl4.flowi4_flags = inet_sk_flowi_flags(sk);
+	fl4.flowi4_secid = 0;
+	fl4.flowi4_tun_key.tun_id = 0;
+	fl4.daddr = faddr;
+	fl4.saddr = saddr;
+	fl4.fl4_dport = 0;
+	fl4.fl4_sport = 0;
 
 	security_sk_classify_flow(sk, flowi4_to_flowi(&fl4));
 	rt = ip_route_output_flow(net, &fl4, sk);
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 438f50c1a6765c..50057df31fc9da 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -599,12 +599,20 @@ static int raw_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, sk->sk_mark, tos,
-			   RT_SCOPE_UNIVERSE,
-			   inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
-			   inet_sk_flowi_flags(sk) |
-			    (inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0),
-			   daddr, saddr, 0, 0);
+	fl4.flowi4_oif = ipc.oif;
+	fl4.flowi4_iif = LOOPBACK_IFINDEX;
+	fl4.flowi4_mark = sk->sk_mark;
+	fl4.flowi4_tos = tos;
+	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_proto = inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol;
+	fl4.flowi4_flags = inet_sk_flowi_flags(sk) |
+			    inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0;
+	fl4.flowi4_secid = 0;
+	fl4.flowi4_tun_key.tun_id = 0;
+	fl4.daddr = daddr;
+	fl4.saddr = saddr;
+	fl4.fl4_dport = 0;
+	fl4.fl4_sport = 0;
 
 	if (!saddr && ipc.oif) {
 		err = l3mdev_get_saddr(net, ipc.oif, &fl4);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a1f2830d811025..e8f499d224ec2a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -514,10 +514,20 @@ static void __build_flow_key(struct flowi4 *fl4, const struct sock *sk,
 		tos = RT_CONN_FLAGS(sk);
 		prot = inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol;
 	}
-	flowi4_init_output(fl4, oif, mark, tos,
-			   RT_SCOPE_UNIVERSE, prot,
-			   flow_flags,
-			   iph->daddr, iph->saddr, 0, 0);
+
+	fl4->flowi4_oif = oif;
+	fl4->flowi4_iif = LOOPBACK_IFINDEX;
+	fl4->flowi4_mark = mark;
+	fl4->flowi4_tos = tos;
+	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_proto = prot;
+	fl4->flowi4_flags = flow_flags;
+	fl4->flowi4_secid = 0;
+	fl4->flowi4_tun_key.tun_id = 0;
+	fl4->daddr = iph->daddr;
+	fl4->saddr = iph->saddr;
+	fl4->fl4_dport = 0;
+	fl4->fl4_sport = 0;
 }
 
 static void build_skb_flow_key(struct flowi4 *fl4, const struct sk_buff *skb,
@@ -542,11 +552,21 @@ static void build_sk_flow_key(struct flowi4 *fl4, const struct sock *sk)
 	inet_opt = rcu_dereference(inet->inet_opt);
 	if (inet_opt && inet_opt->opt.srr)
 		daddr = inet_opt->opt.faddr;
-	flowi4_init_output(fl4, sk->sk_bound_dev_if, sk->sk_mark,
-			   RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
-			   inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
-			   inet_sk_flowi_flags(sk),
-			   daddr, inet->inet_saddr, 0, 0);
+
+	fl4->flowi4_oif = sk->sk_bound_dev_if;
+	fl4->flowi4_iif = LOOPBACK_IFINDEX;
+	fl4->flowi4_mark = sk->sk_mark;
+	fl4->flowi4_tos = RT_CONN_FLAGS(sk);
+	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_proto = inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol;
+	fl4->flowi4_flags = inet_sk_flowi_flags(sk);
+	fl4->flowi4_secid = 0;
+	fl4->flowi4_tun_key.tun_id = 0;
+	fl4->daddr = daddr;
+	fl4->saddr = inet->inet_saddr;
+	fl4->fl4_dport = 0;
+	fl4->fl4_sport = 0;
+
 	rcu_read_unlock();
 }
 
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index e3c4043c27de28..d5eedcae69f188 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -368,11 +368,20 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	 * hasn't changed since we received the original syn, but I see
 	 * no easy way to do this.
 	 */
-	flowi4_init_output(&fl4, ireq->ir_iif, ireq->ir_mark,
-			   RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE, IPPROTO_TCP,
-			   inet_sk_flowi_flags(sk),
-			   opt->srr ? opt->faddr : ireq->ir_rmt_addr,
-			   ireq->ir_loc_addr, th->source, th->dest);
+	fl4.flowi4_oif = ireq->ir_iif;
+	fl4.flowi4_iif = LOOPBACK_IFINDEX;
+	fl4.flowi4_mark = ireq->ir_mark;
+	fl4.flowi4_tos = RT_CONN_FLAGS(sk);
+	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_proto = IPPROTO_TCP;
+	fl4.flowi4_flags = inet_sk_flowi_flags(sk);
+	fl4.flowi4_secid = 0;
+	fl4.flowi4_tun_key.tun_id = 0;
+	fl4.daddr = opt->srr ? opt->faddr : ireq->ir_rmt_addr;
+	fl4.saddr = ireq->ir_loc_addr;
+	fl4.fl4_dport = th->source;
+	fl4.fl4_sport = th->dest;
+
 	security_req_classify_flow(req, flowi4_to_flowi(&fl4));
 	rt = ip_route_output_key(sock_net(sk), &fl4);
 	if (IS_ERR(rt)) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0ff31d97d48586..2d3de2565e13bf 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1056,10 +1056,19 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 		fl4 = &fl4_stack;
 
-		flowi4_init_output(fl4, ipc.oif, sk->sk_mark, tos,
-				   RT_SCOPE_UNIVERSE, sk->sk_protocol,
-				   flow_flags,
-				   faddr, saddr, dport, inet->inet_sport);
+		fl4->flowi4_oif = ipc.oif;
+		fl4->flowi4_iif = LOOPBACK_IFINDEX;
+		fl4->flowi4_mark = sk->sk_mark;
+		fl4->flowi4_tos = tos;
+		fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+		fl4->flowi4_proto = sk->sk_protocol;
+		fl4->flowi4_flags = flow_flags;
+		fl4->flowi4_secid = 0;
+		fl4->flowi4_tun_key.tun_id = 0;
+		fl4->daddr = faddr;
+		fl4->saddr = saddr;
+		fl4->fl4_dport = dport;
+		fl4->fl4_sport = inet->inet_sport;
 
 		if (!saddr && ipc.oif) {
 			err = l3mdev_get_saddr(net, ipc.oif, fl4);
-- 
2.5.5

[PATCH RFC 2/3] ipv4: make flow functions more grepable by removing flowi4_update_output

[Hannes Frederic Sowa]

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/flow.h              | 11 -----------
 include/net/route.h             |  8 ++++----
 net/netfilter/ipvs/ip_vs_xmit.c |  9 +++++++--
 net/sctp/protocol.c             | 10 ++++------
 4 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/include/net/flow.h b/include/net/flow.h
index 2c8e95b987c98c..6276a81f7b3d13 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -90,17 +90,6 @@ struct flowi4 {
 #define fl4_gre_key		uli.gre_key
 } __attribute__((__aligned__(BITS_PER_LONG/8)));
 
-/* Reset some input parameters after previous lookup */
-static inline void flowi4_update_output(struct flowi4 *fl4, int oif, __u8 tos,
-					__be32 daddr, __be32 saddr)
-{
-	fl4->flowi4_oif = oif;
-	fl4->flowi4_tos = tos;
-	fl4->daddr = daddr;
-	fl4->saddr = saddr;
-}
-				      
-
 struct flowi6 {
 	struct flowi_common	__fl_common;
 #define flowi6_oif		__fl_common.flowic_oif
diff --git a/include/net/route.h b/include/net/route.h
index 61164593d41ecf..8937e36fc9fd11 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -320,7 +320,8 @@ static inline struct rtable *ip_route_connect(struct flowi4 *fl4,
 		if (IS_ERR(rt))
 			return rt;
 		ip_rt_put(rt);
-		flowi4_update_output(fl4, oif, tos, fl4->daddr, fl4->saddr);
+		fl4->flowi4_oif = oif;
+		fl4->flowi4_tos = tos;
 	}
 	security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
 	return ip_route_output_flow(net, fl4, sk);
@@ -335,9 +336,8 @@ static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable
 		fl4->fl4_dport = dport;
 		fl4->fl4_sport = sport;
 		ip_rt_put(rt);
-		flowi4_update_output(fl4, sk->sk_bound_dev_if,
-				     RT_CONN_FLAGS(sk), fl4->daddr,
-				     fl4->saddr);
+		fl4->flowi4_oif = sk->sk_bound_dev_if;
+		fl4->flowi4_tos = RT_CONN_FLAGS(sk);
 		security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
 		return ip_route_output_flow(sock_net(sk), fl4, sk);
 	}
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 01d3d894de4630..89444ee244c930 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -140,7 +140,10 @@ retry:
 		if (PTR_ERR(rt) == -EINVAL && *saddr &&
 		    rt_mode & IP_VS_RT_MODE_CONNECT && !loop) {
 			*saddr = 0;
-			flowi4_update_output(&fl4, 0, 0, daddr, 0);
+			fl4.flowi4_oif = 0;
+			fl4.flowi4_tos = 0;
+			fl4.daddr = daddr;
+			fl4.saddr = 0;
 			goto retry;
 		}
 		IP_VS_DBG_RL("ip_route_output error, dest: %pI4\n", &daddr);
@@ -148,7 +151,9 @@ retry:
 	} else if (!*saddr && rt_mode & IP_VS_RT_MODE_CONNECT && fl4.saddr) {
 		ip_rt_put(rt);
 		*saddr = fl4.saddr;
-		flowi4_update_output(&fl4, 0, 0, daddr, fl4.saddr);
+		fl4.flowi4_oif = 0;
+		fl4.flowi4_tos = 0;
+		fl4.daddr = daddr;
 		loop++;
 		goto retry;
 	}
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 40022ee885d7e8..f85b37a73ee633 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -499,12 +499,10 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 			continue;
 
 		fl4->fl4_sport = laddr->a.v4.sin_port;
-		flowi4_update_output(fl4,
-				     asoc->base.sk->sk_bound_dev_if,
-				     RT_CONN_FLAGS(asoc->base.sk),
-				     daddr->v4.sin_addr.s_addr,
-				     laddr->a.v4.sin_addr.s_addr);
-
+		fl4->flowi4_oif = asoc->base.sk->sk_bound_dev_if;
+		fl4->flowi4_tos = RT_CONN_FLAGS(asoc->base.sk);
+		fl4->daddr = daddr->v4.sin_addr.s_addr;
+		fl4->saddr = laddr->a.v4.sin_addr.s_addr;
 		rt = ip_route_output_key(sock_net(sk), fl4);
 		if (IS_ERR(rt))
 			continue;
-- 
2.5.5

[PATCH RFC 3/3] ipv4: kill RTO_ONLINK, RT_CONN_FLAGS and RT_CONN_FLAGS_TOS

[Hannes Frederic Sowa]

Unfortunately because of bisectability I cannot split this patch. :(

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 drivers/net/bonding/bond_main.c |  3 +--
 drivers/net/gtp.c               |  3 ++-
 drivers/net/ppp/pptp.c          |  2 +-
 drivers/net/vrf.c               |  7 -------
 include/net/ip.h                |  5 -----
 include/net/route.h             | 38 ++++++++++++++++++++++++++++----------
 net/dccp/ipv4.c                 |  5 +++--
 net/ipv4/af_inet.c              |  4 ++--
 net/ipv4/arp.c                  |  4 ++--
 net/ipv4/datagram.c             |  5 +++--
 net/ipv4/inet_connection_sock.c | 11 ++++++-----
 net/ipv4/ip_output.c            |  2 +-
 net/ipv4/ping.c                 |  6 +++---
 net/ipv4/raw.c                  | 13 ++++++++-----
 net/ipv4/route.c                | 18 +++++++-----------
 net/ipv4/syncookies.c           |  4 ++--
 net/ipv4/tcp_ipv4.c             |  2 +-
 net/ipv4/udp.c                  |  6 +++---
 net/l2tp/l2tp_ip.c              |  3 ++-
 net/sctp/protocol.c             |  6 ++++--
 20 files changed, 79 insertions(+), 68 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 90157e20357e62..0074563be80201 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2389,8 +2389,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		tags = NULL;
 
 		/* Find out through which dev should the packet go */
-		rt = ip_route_output(dev_net(bond->dev), targets[i], 0,
-				     RTO_ONLINK, 0);
+		rt = ip_route_output_link(dev_net(bond->dev), targets[i]);
 		if (IS_ERR(rt)) {
 			/* there's no route to target - try to send arp
 			 * probe to generate any traffic (arp_validate=0)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 4e976a0d5a7644..a0437bac63f624 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -392,7 +392,8 @@ static struct rtable *ip4_route_output_gtp(struct net *net, struct flowi4 *fl4,
 	fl4->flowi4_oif		= sk->sk_bound_dev_if;
 	fl4->daddr		= daddr;
 	fl4->saddr		= inet_sk(sk)->inet_saddr;
-	fl4->flowi4_tos		= RT_CONN_FLAGS(sk);
+	fl4->flowi4_tos		= RT_TOS(inet_sk(sk)->tos);
+	fl4->flowi4_scope	= sk_rt_scope(sk);
 	fl4->flowi4_proto	= sk->sk_protocol;
 
 	return ip_route_output_key(net, fl4);
diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
index ae0905ed4a32b5..0696c5c24fda2f 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -491,7 +491,7 @@ static int pptp_connect(struct socket *sock, struct sockaddr *uservaddr,
 				   opt->dst_addr.sin_addr.s_addr,
 				   opt->src_addr.sin_addr.s_addr,
 				   0, 0,
-				   IPPROTO_GRE, RT_CONN_FLAGS(sk), 0);
+				   IPPROTO_GRE, RT_TOS(inet_sk(sk)->tos), 0);
 	if (IS_ERR(rt)) {
 		error = -EHOSTUNREACH;
 		goto end;
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index b4d746943bc570..961d7905599150 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -37,9 +37,6 @@
 #include <net/l3mdev.h>
 #include <net/fib_rules.h>
 
-#define RT_FL_TOS(oldflp4) \
-	((oldflp4)->flowi4_tos & (IPTOS_RT_MASK | RTO_ONLINK))
-
 #define DRV_NAME	"vrf"
 #define DRV_VERSION	"1.0"
 
@@ -721,7 +718,6 @@ static int vrf_get_saddr(struct net_device *dev, struct flowi4 *fl4)
 	u32 orig_tos = fl4->flowi4_tos;
 	u8 flags = fl4->flowi4_flags;
 	u8 scope = fl4->flowi4_scope;
-	u8 tos = RT_FL_TOS(fl4);
 	int rc;
 
 	if (unlikely(!fl4->daddr))
@@ -731,9 +727,6 @@ static int vrf_get_saddr(struct net_device *dev, struct flowi4 *fl4)
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	/* make sure oif is set to VRF device for lookup */
 	fl4->flowi4_oif = dev->ifindex;
-	fl4->flowi4_tos = tos & IPTOS_RT_MASK;
-	fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
-			     RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
 
 	rc = fib_lookup(net, fl4, &res, 0);
 	if (!rc) {
diff --git a/include/net/ip.h b/include/net/ip.h
index 37165fba3741ac..1b41763fb7900a 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -153,11 +153,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);
diff --git a/include/net/route.h b/include/net/route.h
index 8937e36fc9fd11..c374c217de7d7f 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -41,10 +41,13 @@
 /* IPv4 datagram length is stored into 16bit field (tot_len) */
 #define IP_MAX_MTU	0xFFFFU
 
-#define RTO_ONLINK	0x01
-
-#define RT_CONN_FLAGS(sk)   (RT_TOS(inet_sk(sk)->tos) | sock_flag(sk, SOCK_LOCALROUTE))
-#define RT_CONN_FLAGS_TOS(sk,tos)   (RT_TOS(tos) | sock_flag(sk, SOCK_LOCALROUTE))
+static inline u8 sk_rt_scope(const struct sock *sk)
+{
+	if (sock_flag(sk, SOCK_LOCALROUTE))
+		return RT_SCOPE_LINK;
+	else
+		return RT_SCOPE_UNIVERSE;
+}
 
 struct fib_nh;
 struct fib_info;
@@ -133,8 +136,10 @@ static inline struct rtable *ip_route_output_key(struct net *net, struct flowi4
 	return ip_route_output_flow(net, flp, NULL);
 }
 
-static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
-					     __be32 saddr, u8 tos, int oif)
+static inline struct rtable *ip_route_output_scope(struct net *net,
+						   __be32 daddr,
+						   __be32 saddr, u8 tos,
+						   u8 scope, int oif)
 {
 	struct flowi4 fl4 = {
 		.flowi4_oif = oif,
@@ -145,6 +150,18 @@ static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
 	return ip_route_output_key(net, &fl4);
 }
 
+static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
+					     __be32 saddr, u8 tos, int oif)
+{
+	return ip_route_output_scope(net, daddr, saddr, tos, RT_SCOPE_UNIVERSE,
+				     oif);
+}
+
+static inline struct rtable *ip_route_output_link(struct net *net, __be32 daddr)
+{
+	return ip_route_output_scope(net, daddr, 0, 0, RT_SCOPE_LINK, 0);
+}
+
 static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi4 *fl4,
 						   struct sock *sk,
 						   __be32 daddr, __be32 saddr,
@@ -155,7 +172,7 @@ static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	fl4->flowi4_mark = sk ? sk->sk_mark : 0;
 	fl4->flowi4_tos = tos;
-	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_scope = sk ? sk_rt_scope(sk) : RT_SCOPE_UNIVERSE;
 	fl4->flowi4_proto = proto;
 	fl4->flowi4_flags = sk ? inet_sk_flowi_flags(sk) : 0;
 	fl4->flowi4_secid = 0;
@@ -282,8 +299,8 @@ static inline void ip_route_connect_init(struct flowi4 *fl4, __be32 dst, __be32
 	fl4->flowi4_oif = oif;
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	fl4->flowi4_mark = sk->sk_mark;
-	fl4->flowi4_tos = tos;
-	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_tos = RT_TOS(tos);
+	fl4->flowi4_scope = sk_rt_scope(sk);
 	fl4->flowi4_proto = protocol;
 	fl4->flowi4_flags = flow_flags;
 	fl4->flowi4_secid = 0;
@@ -337,7 +354,8 @@ static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable
 		fl4->fl4_sport = sport;
 		ip_rt_put(rt);
 		fl4->flowi4_oif = sk->sk_bound_dev_if;
-		fl4->flowi4_tos = RT_CONN_FLAGS(sk);
+		fl4->flowi4_tos = RT_TOS(inet_sk(sk)->tos);
+		fl4->flowi4_scope = sk_rt_scope(sk);
 		security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
 		return ip_route_output_flow(sock_net(sk), fl4, sk);
 	}
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 5c7e413a3ae407..59df4a807d2991 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -73,7 +73,7 @@ int dccp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	orig_dport = usin->sin_port;
 	fl4 = &inet->cork.fl.u.ip4;
 	rt = ip_route_connect(fl4, nexthop, inet->inet_saddr,
-			      RT_CONN_FLAGS(sk), sk->sk_bound_dev_if,
+			      RT_TOS(inet_sk(sk)->tos), sk->sk_bound_dev_if,
 			      IPPROTO_DCCP,
 			      orig_sport, orig_dport, sk);
 	if (IS_ERR(rt))
@@ -453,7 +453,8 @@ static struct dst_entry* dccp_v4_route_skb(struct net *net, struct sock *sk,
 		.flowi4_oif = inet_iif(skb),
 		.daddr = iph->saddr,
 		.saddr = iph->daddr,
-		.flowi4_tos = RT_CONN_FLAGS(sk),
+		.flowi4_tos = RT_TOS(inet_sk(sk)->tos),
+		.flowi4_scope = sk_rt_scope(sk),
 		.flowi4_proto = sk->sk_protocol,
 		.fl4_sport = dccp_hdr(skb)->dccph_dport,
 		.fl4_dport = dccp_hdr(skb)->dccph_sport,
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d39e9e47a26e55..7a62bb5f14e7d5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1113,7 +1113,7 @@ static int inet_sk_reselect_saddr(struct sock *sk)
 
 	/* Query new route. */
 	fl4 = &inet->cork.fl.u.ip4;
-	rt = ip_route_connect(fl4, daddr, 0, RT_CONN_FLAGS(sk),
+	rt = ip_route_connect(fl4, daddr, 0, RT_TOS(inet_sk(sk)->tos),
 			      sk->sk_bound_dev_if, sk->sk_protocol,
 			      inet->inet_sport, inet->inet_dport, sk);
 	if (IS_ERR(rt))
@@ -1167,7 +1167,7 @@ int inet_sk_rebuild_header(struct sock *sk)
 	fl4 = &inet->cork.fl.u.ip4;
 	rt = ip_route_output_ports(sock_net(sk), fl4, sk, daddr, inet->inet_saddr,
 				   inet->inet_dport, inet->inet_sport,
-				   sk->sk_protocol, RT_CONN_FLAGS(sk),
+				   sk->sk_protocol, RT_TOS(inet_sk(sk)->tos),
 				   sk->sk_bound_dev_if);
 	if (!IS_ERR(rt)) {
 		err = 0;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 89a8cac4726a5e..d7e3a707bbd9ee 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -994,7 +994,7 @@ static int arp_req_set(struct net *net, struct arpreq *r,
 	if (r->arp_flags & ATF_PERM)
 		r->arp_flags |= ATF_COM;
 	if (!dev) {
-		struct rtable *rt = ip_route_output(net, ip, 0, RTO_ONLINK, 0);
+		struct rtable *rt = ip_route_output_link(net, ip);
 
 		if (IS_ERR(rt))
 			return PTR_ERR(rt);
@@ -1116,7 +1116,7 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
 
 	ip = ((struct sockaddr_in *)&r->arp_pa)->sin_addr.s_addr;
 	if (!dev) {
-		struct rtable *rt = ip_route_output(net, ip, 0, RTO_ONLINK, 0);
+		struct rtable *rt = ip_route_output_link(net, ip);
 		if (IS_ERR(rt))
 			return PTR_ERR(rt);
 		dev = rt->dst.dev;
diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index f915abff1350a8..3c4d0b25222c8e 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -49,7 +49,7 @@ int __ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
 	}
 	fl4 = &inet->cork.fl.u.ip4;
 	rt = ip_route_connect(fl4, usin->sin_addr.s_addr, saddr,
-			      RT_CONN_FLAGS(sk), oif,
+			      RT_TOS(inet_sk(sk)->tos), oif,
 			      sk->sk_protocol,
 			      inet->inet_sport, usin->sin_port, sk);
 	if (IS_ERR(rt)) {
@@ -121,7 +121,8 @@ void ip4_datagram_release_cb(struct sock *sk)
 	rt = ip_route_output_ports(sock_net(sk), &fl4, sk, daddr,
 				   inet->inet_saddr, inet->inet_dport,
 				   inet->inet_sport, sk->sk_protocol,
-				   RT_CONN_FLAGS(sk), sk->sk_bound_dev_if);
+				   RT_TOS(inet_sk(sk)->tos),
+				   sk->sk_bound_dev_if);
 
 	dst = !IS_ERR(rt) ? &rt->dst : NULL;
 	sk_dst_set(sk, dst);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f84b851e050534..9289aca0c98cfd 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -413,8 +413,8 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
 	fl4->flowi4_oif = ireq->ir_iif;
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	fl4->flowi4_mark = ireq->ir_mark;
-	fl4->flowi4_tos = RT_CONN_FLAGS(sk);
-	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_tos = RT_TOS(inet_sk(sk)->tos);
+	fl4->flowi4_scope = sk_rt_scope(sk);
 	fl4->flowi4_proto = sk->sk_protocol;
 	fl4->flowi4_flags = inet_sk_flowi_flags(sk);
 	fl4->flowi4_secid = 0;
@@ -459,8 +459,8 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk,
 	fl4->flowi4_oif = ireq->ir_iif;
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	fl4->flowi4_mark = ireq->ir_mark;
-	fl4->flowi4_tos = RT_CONN_FLAGS(sk);
-	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_tos = RT_TOS(inet_sk(sk)->tos);
+	fl4->flowi4_scope = sk_rt_scope(sk);
 	fl4->flowi4_proto = sk->sk_protocol;
 	fl4->flowi4_flags = inet_sk_flowi_flags(sk);
 	fl4->flowi4_secid = 0;
@@ -950,7 +950,8 @@ static struct dst_entry *inet_csk_rebuild_route(struct sock *sk, struct flowi *f
 	rt = ip_route_output_ports(sock_net(sk), fl4, sk, daddr,
 				   inet->inet_saddr, inet->inet_dport,
 				   inet->inet_sport, sk->sk_protocol,
-				   RT_CONN_FLAGS(sk), sk->sk_bound_dev_if);
+				   RT_TOS(inet_sk(sk)->tos),
+				   sk->sk_bound_dev_if);
 	if (IS_ERR(rt))
 		rt = NULL;
 	if (rt)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 90076310888d7f..b016679c988e3a 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -412,7 +412,7 @@ int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 					   inet->inet_dport,
 					   inet->inet_sport,
 					   sk->sk_protocol,
-					   RT_CONN_FLAGS(sk),
+					   RT_TOS(inet_sk(sk)->tos),
 					   sk->sk_bound_dev_if);
 		if (IS_ERR(rt))
 			goto no_route;
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index dc6a42d1136752..87a901ce43c62b 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -708,7 +708,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 = RT_SCOPE_UNIVERSE;
 	int err;
 
 	pr_debug("ping_v4_sendmsg(sk=%p,sk->num=%u)\n", inet, inet->inet_num);
@@ -781,7 +781,7 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (sock_flag(sk, SOCK_LOCALROUTE) ||
 	    (msg->msg_flags & MSG_DONTROUTE) ||
 	    (ipc.opt && ipc.opt->opt.is_strictroute)) {
-		tos |= RTO_ONLINK;
+		scope = RT_SCOPE_LINK;
 	}
 
 	if (ipv4_is_multicast(daddr)) {
@@ -796,7 +796,7 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	fl4.flowi4_iif = LOOPBACK_IFINDEX;
 	fl4.flowi4_mark = sk->sk_mark;
 	fl4.flowi4_tos = tos;
-	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_scope = scope;
 	fl4.flowi4_proto = sk->sk_protocol;
 	fl4.flowi4_flags = inet_sk_flowi_flags(sk);
 	fl4.flowi4_secid = 0;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 50057df31fc9da..d4b4dedb44a1eb 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -495,7 +495,7 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int free = 0;
 	__be32 daddr;
 	__be32 saddr;
-	u8  tos;
+	u8  tos, scope = RT_SCOPE_UNIVERSE;
 	int err;
 	struct ip_options_data opt_copy;
 	struct raw_frag_vec rfv;
@@ -587,9 +587,12 @@ 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_sk(sk));
+
+	if (msg->msg_flags & MSG_DONTROUTE ||
+	    sock_flag(sk, SOCK_LOCALROUTE))
+		scope = RT_SCOPE_LINK;
 
 	if (ipv4_is_multicast(daddr)) {
 		if (!ipc.oif)
@@ -603,7 +606,7 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	fl4.flowi4_iif = LOOPBACK_IFINDEX;
 	fl4.flowi4_mark = sk->sk_mark;
 	fl4.flowi4_tos = tos;
-	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_scope = scope;
 	fl4.flowi4_proto = inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol;
 	fl4.flowi4_flags = inet_sk_flowi_flags(sk) |
 			    inet->hdrincl ? FLOWI_FLAG_KNOWN_NH : 0;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e8f499d224ec2a..2446727ca6f980 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -114,9 +114,6 @@
 #include <net/ip_tunnels.h>
 #include <net/l3mdev.h>
 
-#define RT_FL_TOS(oldflp4) \
-	((oldflp4)->flowi4_tos & (IPTOS_RT_MASK | RTO_ONLINK))
-
 #define RT_GC_TIMEOUT (300*HZ)
 
 static int ip_rt_max_size;
@@ -506,20 +503,23 @@ static void __build_flow_key(struct flowi4 *fl4, const struct sock *sk,
 			     int oif, u8 tos,
 			     u8 prot, u32 mark, int flow_flags)
 {
+	u8 scope = RT_SCOPE_UNIVERSE;
+
 	if (sk) {
 		const struct inet_sock *inet = inet_sk(sk);
 
 		oif = sk->sk_bound_dev_if;
 		mark = sk->sk_mark;
-		tos = RT_CONN_FLAGS(sk);
+		tos = RT_TOS(inet_sk(sk)->tos);
 		prot = inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol;
+		scope = sk_rt_scope(sk);
 	}
 
 	fl4->flowi4_oif = oif;
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	fl4->flowi4_mark = mark;
 	fl4->flowi4_tos = tos;
-	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_scope = scope;
 	fl4->flowi4_proto = prot;
 	fl4->flowi4_flags = flow_flags;
 	fl4->flowi4_secid = 0;
@@ -556,8 +556,8 @@ static void build_sk_flow_key(struct flowi4 *fl4, const struct sock *sk)
 	fl4->flowi4_oif = sk->sk_bound_dev_if;
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
 	fl4->flowi4_mark = sk->sk_mark;
-	fl4->flowi4_tos = RT_CONN_FLAGS(sk);
-	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4->flowi4_tos = RT_TOS(inet_sk(sk)->tos);
+	fl4->flowi4_scope = sk_rt_scope(sk);
 	fl4->flowi4_proto = inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol;
 	fl4->flowi4_flags = inet_sk_flowi_flags(sk);
 	fl4->flowi4_secid = 0;
@@ -2162,7 +2162,6 @@ struct rtable *__ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
 					  int mp_hash)
 {
 	struct net_device *dev_out = NULL;
-	__u8 tos = RT_FL_TOS(fl4);
 	unsigned int flags = 0;
 	struct fib_result res;
 	struct rtable *rth;
@@ -2180,9 +2179,6 @@ struct rtable *__ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
 	if (master_idx)
 		fl4->flowi4_oif = master_idx;
 	fl4->flowi4_iif = LOOPBACK_IFINDEX;
-	fl4->flowi4_tos = tos & IPTOS_RT_MASK;
-	fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
-			 RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
 
 	rcu_read_lock();
 	if (fl4->saddr) {
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index d5eedcae69f188..bdc23e4fafd64f 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -371,8 +371,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	fl4.flowi4_oif = ireq->ir_iif;
 	fl4.flowi4_iif = LOOPBACK_IFINDEX;
 	fl4.flowi4_mark = ireq->ir_mark;
-	fl4.flowi4_tos = RT_CONN_FLAGS(sk);
-	fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+	fl4.flowi4_tos = RT_TOS(inet_sk(sk)->tos);
+	fl4.flowi4_scope = sk_rt_scope(sk);
 	fl4.flowi4_proto = IPPROTO_TCP;
 	fl4.flowi4_flags = inet_sk_flowi_flags(sk);
 	fl4.flowi4_secid = 0;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3708de2a66833c..a0d0f08090a39e 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -168,7 +168,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	orig_dport = usin->sin_port;
 	fl4 = &inet->cork.fl.u.ip4;
 	rt = ip_route_connect(fl4, nexthop, inet->inet_saddr,
-			      RT_CONN_FLAGS(sk), sk->sk_bound_dev_if,
+			      RT_TOS(inet_sk(sk)->tos), sk->sk_bound_dev_if,
 			      IPPROTO_TCP,
 			      orig_sport, orig_dport, sk);
 	if (IS_ERR(rt)) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 2d3de2565e13bf..5203c1f89f6bb3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -923,7 +923,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int connected = 0;
 	__be32 daddr, faddr, saddr;
 	__be16 dport;
-	u8  tos;
+	u8  tos, scope = RT_SCOPE_UNIVERSE;
 	int err, is_udplite = IS_UDPLITE(sk);
 	int corkreq = up->corkflag || msg->msg_flags&MSG_MORE;
 	int (*getfrag)(void *, char *, int, int, int, struct sk_buff *);
@@ -1034,7 +1034,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (sock_flag(sk, SOCK_LOCALROUTE) ||
 	    (msg->msg_flags & MSG_DONTROUTE) ||
 	    (ipc.opt && ipc.opt->opt.is_strictroute)) {
-		tos |= RTO_ONLINK;
+		scope = RT_SCOPE_LINK;
 		connected = 0;
 	}
 
@@ -1060,7 +1060,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		fl4->flowi4_iif = LOOPBACK_IFINDEX;
 		fl4->flowi4_mark = sk->sk_mark;
 		fl4->flowi4_tos = tos;
-		fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
+		fl4->flowi4_scope = scope;
 		fl4->flowi4_proto = sk->sk_protocol;
 		fl4->flowi4_flags = flow_flags;
 		fl4->flowi4_secid = 0;
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 42de4ccd159f6f..66b33bd8153d8b 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -470,7 +470,8 @@ static int l2tp_ip_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		rt = ip_route_output_ports(sock_net(sk), fl4, sk,
 					   daddr, inet->inet_saddr,
 					   inet->inet_dport, inet->inet_sport,
-					   sk->sk_protocol, RT_CONN_FLAGS(sk),
+					   sk->sk_protocol,
+					   RT_TOS(inet_sk(sk)->tos),
 					   sk->sk_bound_dev_if);
 		if (IS_ERR(rt))
 			goto no_route;
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index f85b37a73ee633..5e8eee6182bbec 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -438,7 +438,8 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 	fl4->fl4_dport = daddr->v4.sin_port;
 	fl4->flowi4_proto = IPPROTO_SCTP;
 	if (asoc) {
-		fl4->flowi4_tos = RT_CONN_FLAGS(asoc->base.sk);
+		fl4->flowi4_tos = RT_TOS(inet_sk(asoc->base.sk)->tos);
+		fl4->flowi4_scope = sk_rt_scope(asoc->base.sk);
 		fl4->flowi4_oif = asoc->base.sk->sk_bound_dev_if;
 		fl4->fl4_sport = htons(asoc->base.bind_addr.port);
 	}
@@ -500,7 +501,8 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 
 		fl4->fl4_sport = laddr->a.v4.sin_port;
 		fl4->flowi4_oif = asoc->base.sk->sk_bound_dev_if;
-		fl4->flowi4_tos = RT_CONN_FLAGS(asoc->base.sk);
+		fl4->flowi4_tos = RT_TOS(inet_sk(asoc->base.sk)->tos);
+		fl4->flowi4_scope = sk_rt_scope(asoc->base.sk);
 		fl4->daddr = daddr->v4.sin_addr.s_addr;
 		fl4->saddr = laddr->a.v4.sin_addr.s_addr;
 		rt = ip_route_output_key(sock_net(sk), fl4);
-- 
2.5.5

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[David Ahern]

On 6/13/16 2:47 PM, Hannes Frederic Sowa wrote:
> No functional changes.
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  include/net/flow.h              | 21 ---------------------
>  include/net/route.h             | 33 +++++++++++++++++++++++++++------
>  net/ipv4/inet_connection_sock.c | 41 +++++++++++++++++++++++++++++------------
>  net/ipv4/ip_output.c            | 21 ++++++++++++++-------
>  net/ipv4/ping.c                 | 16 +++++++++++++---
>  net/ipv4/raw.c                  | 20 ++++++++++++++------
>  net/ipv4/route.c                | 38 +++++++++++++++++++++++++++++---------
>  net/ipv4/syncookies.c           | 19 ++++++++++++++-----
>  net/ipv4/udp.c                  | 17 +++++++++++++----
>  9 files changed, 153 insertions(+), 73 deletions(-)
>
> diff --git a/include/net/flow.h b/include/net/flow.h
> index d47ef4bb5423a3..2c8e95b987c98c 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -90,27 +90,6 @@ struct flowi4 {
>  #define fl4_gre_key		uli.gre_key
>  } __attribute__((__aligned__(BITS_PER_LONG/8)));
>
> -static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
> -				      __u32 mark, __u8 tos, __u8 scope,
> -				      __u8 proto, __u8 flags,
> -				      __be32 daddr, __be32 saddr,
> -				      __be16 dport, __be16 sport)
> -{
> -	fl4->flowi4_oif = oif;
> -	fl4->flowi4_iif = LOOPBACK_IFINDEX;
> -	fl4->flowi4_mark = mark;
> -	fl4->flowi4_tos = tos;
> -	fl4->flowi4_scope = scope;
> -	fl4->flowi4_proto = proto;
> -	fl4->flowi4_flags = flags;
> -	fl4->flowi4_secid = 0;
> -	fl4->flowi4_tun_key.tun_id = 0;
> -	fl4->daddr = daddr;
> -	fl4->saddr = saddr;
> -	fl4->fl4_dport = dport;
> -	fl4->fl4_sport = sport;
> -}
> -

I found this helper to be unhelpful developing the VRF changes. Really 
obfuscates the initialization of the flow struct. Happy to see it go.

I would like to see more initialization at declaration time as well to 
fix the recurring problem of failing to add an init for new fields. 
Doable for a lot of flow use cases with just some minor refactorings.

Anyways, change LGTM.

Reviewed-by: David Ahern <dsa@cumulusnetworks.com>

Re: [PATCH RFC 2/3] ipv4: make flow functions more grepable by removing flowi4_update_output

[David Ahern]

On 6/13/16 2:47 PM, Hannes Frederic Sowa wrote:
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  include/net/flow.h              | 11 -----------
>  include/net/route.h             |  8 ++++----
>  net/netfilter/ipvs/ip_vs_xmit.c |  9 +++++++--
>  net/sctp/protocol.c             | 10 ++++------
>  4 files changed, 15 insertions(+), 23 deletions(-)
>
> diff --git a/include/net/flow.h b/include/net/flow.h
> index 2c8e95b987c98c..6276a81f7b3d13 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -90,17 +90,6 @@ struct flowi4 {
>  #define fl4_gre_key		uli.gre_key
>  } __attribute__((__aligned__(BITS_PER_LONG/8)));
>
> -/* Reset some input parameters after previous lookup */
> -static inline void flowi4_update_output(struct flowi4 *fl4, int oif, __u8 tos,
> -					__be32 daddr, __be32 saddr)
> -{
> -	fl4->flowi4_oif = oif;
> -	fl4->flowi4_tos = tos;
> -	fl4->daddr = daddr;
> -	fl4->saddr = saddr;
> -}
> -				
> -

Ditto with this helper. Glad to see it go.

Change LGTM. Reviewed-by: David Ahern <dsa@cumulusnetworks.com>

Re: [PATCH RFC 3/3] ipv4: kill RTO_ONLINK, RT_CONN_FLAGS and RT_CONN_FLAGS_TOS

[David Ahern]

On 6/13/16 2:47 PM, Hannes Frederic Sowa wrote:
> Unfortunately because of bisectability I cannot split this patch. :(
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  drivers/net/bonding/bond_main.c |  3 +--
>  drivers/net/gtp.c               |  3 ++-
>  drivers/net/ppp/pptp.c          |  2 +-
>  drivers/net/vrf.c               |  7 -------
>  include/net/ip.h                |  5 -----
>  include/net/route.h             | 38 ++++++++++++++++++++++++++++----------
>  net/dccp/ipv4.c                 |  5 +++--
>  net/ipv4/af_inet.c              |  4 ++--
>  net/ipv4/arp.c                  |  4 ++--
>  net/ipv4/datagram.c             |  5 +++--
>  net/ipv4/inet_connection_sock.c | 11 ++++++-----
>  net/ipv4/ip_output.c            |  2 +-
>  net/ipv4/ping.c                 |  6 +++---
>  net/ipv4/raw.c                  | 13 ++++++++-----
>  net/ipv4/route.c                | 18 +++++++-----------
>  net/ipv4/syncookies.c           |  4 ++--
>  net/ipv4/tcp_ipv4.c             |  2 +-
>  net/ipv4/udp.c                  |  6 +++---
>  net/l2tp/l2tp_ip.c              |  3 ++-
>  net/sctp/protocol.c             |  6 ++++--
>  20 files changed, 79 insertions(+), 68 deletions(-)

Reviewed-by: David Ahern <dsa@cumulusnetworks.com>

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[Julian Anastasov]

	Hello,

On Mon, 13 Jun 2016, Hannes Frederic Sowa wrote:

> -	flowi4_init_output(fl4, oif, sk->sk_mark, tos, RT_SCOPE_UNIVERSE,
> -			   protocol, flow_flags, dst, src, dport, sport);
> +	fl4->flowi4_oif = oif;
> +	fl4->flowi4_iif = LOOPBACK_IFINDEX;
> +	fl4->flowi4_mark = sk->sk_mark;
> +	fl4->flowi4_tos = tos;
> +	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
> +	fl4->flowi4_proto = protocol;
> +	fl4->flowi4_flags = flow_flags;
> +	fl4->flowi4_secid = 0;
> +	fl4->flowi4_tun_key.tun_id = 0;
> +	fl4->daddr = dst;
> +	fl4->saddr = src;
> +	fl4->fl4_dport = dport;
> +	fl4->fl4_sport = sport;

	The way you initialize the fields adds new risks
when new field is added, it is difficult to track all such
places. If they are missed, the lookup will use random
value for the new field. I'm not sure what will compile,
may be something likes this?:

	*fl4 = (struct flowi4) {
		.flowi4_oif = oif,
		...
	};

Regards

Re: [PATCH RFC 2/3] ipv4: make flow functions more grepable by removing flowi4_update_output

[Julian Anastasov]

	Hello,

On Mon, 13 Jun 2016, Hannes Frederic Sowa wrote:

> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  include/net/flow.h              | 11 -----------
>  include/net/route.h             |  8 ++++----
>  net/netfilter/ipvs/ip_vs_xmit.c |  9 +++++++--
>  net/sctp/protocol.c             | 10 ++++------
>  4 files changed, 15 insertions(+), 23 deletions(-)
> 
> diff --git a/include/net/flow.h b/include/net/flow.h
> index 2c8e95b987c98c..6276a81f7b3d13 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -90,17 +90,6 @@ struct flowi4 {
>  #define fl4_gre_key		uli.gre_key
>  } __attribute__((__aligned__(BITS_PER_LONG/8)));
>  
> -/* Reset some input parameters after previous lookup */
> -static inline void flowi4_update_output(struct flowi4 *fl4, int oif, __u8 tos,
> -					__be32 daddr, __be32 saddr)
> -{
> -	fl4->flowi4_oif = oif;
> -	fl4->flowi4_tos = tos;
> -	fl4->daddr = daddr;
> -	fl4->saddr = saddr;
> -}

	My preference is this function to stay, it helps
to know the few places that use the route result for new
lookup.

Regards

Re: [PATCH RFC 3/3] ipv4: kill RTO_ONLINK, RT_CONN_FLAGS and RT_CONN_FLAGS_TOS

[Julian Anastasov]

	Hello,

On Mon, 13 Jun 2016, Hannes Frederic Sowa wrote:

> Unfortunately because of bisectability I cannot split this patch. :(
> 
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index b4d746943bc570..961d7905599150 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -37,9 +37,6 @@
>  #include <net/l3mdev.h>
>  #include <net/fib_rules.h>
>  
> -#define RT_FL_TOS(oldflp4) \
> -	((oldflp4)->flowi4_tos & (IPTOS_RT_MASK | RTO_ONLINK))

	IPTOS_RT_MASK is 3 bits (2..4) while RT_TOS is
4 bits (1..4). Should we just redefine RT_TOS to 3 bits?
Otherwise, flowi4_tos still needs to be masked with IPTOS_RT_MASK
and as result, it should be present in flowi4_update_output.
But after removing the RTO_ONLINK flag from flowi4_tos,
the tos should not be updated by flowi4_update_output,
even if we mask it with IPTOS_RT_MASK.

> -
>  #define DRV_NAME	"vrf"
>  #define DRV_VERSION	"1.0"
>  
> @@ -721,7 +718,6 @@ static int vrf_get_saddr(struct net_device *dev, struct flowi4 *fl4)
>  	u32 orig_tos = fl4->flowi4_tos;
>  	u8 flags = fl4->flowi4_flags;
>  	u8 scope = fl4->flowi4_scope;
> -	u8 tos = RT_FL_TOS(fl4);
>  	int rc;
>  
>  	if (unlikely(!fl4->daddr))
> @@ -731,9 +727,6 @@ static int vrf_get_saddr(struct net_device *dev, struct flowi4 *fl4)
>  	fl4->flowi4_iif = LOOPBACK_IFINDEX;
>  	/* make sure oif is set to VRF device for lookup */
>  	fl4->flowi4_oif = dev->ifindex;
> -	fl4->flowi4_tos = tos & IPTOS_RT_MASK;

	Ops, now output route has the same bug as the
input route from 3.6: bit 1 reaches the rule matching,
we left all 4 bits from RT_TOS in flowi4_tos.

> -	fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
> -			     RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
>  
>  	rc = fib_lookup(net, fl4, &res, 0);
>  	if (!rc) {
> diff --git a/include/net/route.h b/include/net/route.h
> index 8937e36fc9fd11..c374c217de7d7f 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -133,8 +136,10 @@ static inline struct rtable *ip_route_output_key(struct net *net, struct flowi4
>  	return ip_route_output_flow(net, flp, NULL);
>  }
>  
> -static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
> -					     __be32 saddr, u8 tos, int oif)
> +static inline struct rtable *ip_route_output_scope(struct net *net,
> +						   __be32 daddr,
> +						   __be32 saddr, u8 tos,
> +						   u8 scope, int oif)

	scope added but ignored?

>  {
>  	struct flowi4 fl4 = {
>  		.flowi4_oif = oif,
> @@ -145,6 +150,18 @@ static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
>  	return ip_route_output_key(net, &fl4);
>  }
>  
> +static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
> +					     __be32 saddr, u8 tos, int oif)
> +{
> +	return ip_route_output_scope(net, daddr, saddr, tos, RT_SCOPE_UNIVERSE,
> +				     oif);
> +}
> +
> +static inline struct rtable *ip_route_output_link(struct net *net, __be32 daddr)
> +{
> +	return ip_route_output_scope(net, daddr, 0, 0, RT_SCOPE_LINK, 0);
> +}
> +
>  static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi4 *fl4,
>  						   struct sock *sk,
>  						   __be32 daddr, __be32 saddr,

> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index e8f499d224ec2a..2446727ca6f980 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2162,7 +2162,6 @@ struct rtable *__ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
>  					  int mp_hash)
>  {
>  	struct net_device *dev_out = NULL;
> -	__u8 tos = RT_FL_TOS(fl4);
>  	unsigned int flags = 0;
>  	struct fib_result res;
>  	struct rtable *rth;
> @@ -2180,9 +2179,6 @@ struct rtable *__ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
>  	if (master_idx)
>  		fl4->flowi4_oif = master_idx;
>  	fl4->flowi4_iif = LOOPBACK_IFINDEX;
> -	fl4->flowi4_tos = tos & IPTOS_RT_MASK;

	Now bit 1 reaches rule matching if not masked
with IPTOS_RT_MASK.

> -	fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
> -			 RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
>  
>  	rcu_read_lock();
>  	if (fl4->saddr) {

	Also, do you plan to include a patch that adds the
missing IPTOS_RT_MASK for input route or it is a material
for the net tree?

Regards

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[Hannes Frederic Sowa]

On 14.06.2016 08:02, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 13 Jun 2016, Hannes Frederic Sowa wrote:
> 
>> -	flowi4_init_output(fl4, oif, sk->sk_mark, tos, RT_SCOPE_UNIVERSE,
>> -			   protocol, flow_flags, dst, src, dport, sport);
>> +	fl4->flowi4_oif = oif;
>> +	fl4->flowi4_iif = LOOPBACK_IFINDEX;
>> +	fl4->flowi4_mark = sk->sk_mark;
>> +	fl4->flowi4_tos = tos;
>> +	fl4->flowi4_scope = RT_SCOPE_UNIVERSE;
>> +	fl4->flowi4_proto = protocol;
>> +	fl4->flowi4_flags = flow_flags;
>> +	fl4->flowi4_secid = 0;
>> +	fl4->flowi4_tun_key.tun_id = 0;
>> +	fl4->daddr = dst;
>> +	fl4->saddr = src;
>> +	fl4->fl4_dport = dport;
>> +	fl4->fl4_sport = sport;
> 
> 	The way you initialize the fields adds new risks
> when new field is added, it is difficult to track all such
> places. If they are missed, the lookup will use random
> value for the new field. I'm not sure what will compile,
> may be something likes this?:
> 
> 	*fl4 = (struct flowi4) {
> 		.flowi4_oif = oif,
> 		...
> 	};

Yes, this is probably the better way to go, I adapted my patch accordingly.

Thanks,

Re: [PATCH RFC 3/3] ipv4: kill RTO_ONLINK, RT_CONN_FLAGS and RT_CONN_FLAGS_TOS

[Hannes Frederic Sowa]

On 14.06.2016 08:29, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 13 Jun 2016, Hannes Frederic Sowa wrote:
> 
>> Unfortunately because of bisectability I cannot split this patch. :(
>>
>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> ---
>> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
>> index b4d746943bc570..961d7905599150 100644
>> --- a/drivers/net/vrf.c
>> +++ b/drivers/net/vrf.c
>> @@ -37,9 +37,6 @@
>>  #include <net/l3mdev.h>
>>  #include <net/fib_rules.h>
>>  
>> -#define RT_FL_TOS(oldflp4) \
>> -	((oldflp4)->flowi4_tos & (IPTOS_RT_MASK | RTO_ONLINK))
> 
> 	IPTOS_RT_MASK is 3 bits (2..4) while RT_TOS is
> 4 bits (1..4). Should we just redefine RT_TOS to 3 bits?
> Otherwise, flowi4_tos still needs to be masked with IPTOS_RT_MASK
> and as result, it should be present in flowi4_update_output.
> But after removing the RTO_ONLINK flag from flowi4_tos,
> the tos should not be updated by flowi4_update_output,
> even if we mask it with IPTOS_RT_MASK.

Yes, my plan is to start adding either sparse or a new type for tos
fields, so we can have the compiler check all the casts/masking.

I hope those problems will simply vanish then. ;)

My plan is in the end to remove IPTTOS_TOS_MASK, IPTOS_RT_MASK and
RT_TOS and come up with something common for all tos handling.

>> -
>>  #define DRV_NAME	"vrf"
>>  #define DRV_VERSION	"1.0"
>>  
>> @@ -721,7 +718,6 @@ static int vrf_get_saddr(struct net_device *dev, struct flowi4 *fl4)
>>  	u32 orig_tos = fl4->flowi4_tos;
>>  	u8 flags = fl4->flowi4_flags;
>>  	u8 scope = fl4->flowi4_scope;
>> -	u8 tos = RT_FL_TOS(fl4);
>>  	int rc;
>>  
>>  	if (unlikely(!fl4->daddr))
>> @@ -731,9 +727,6 @@ static int vrf_get_saddr(struct net_device *dev, struct flowi4 *fl4)
>>  	fl4->flowi4_iif = LOOPBACK_IFINDEX;
>>  	/* make sure oif is set to VRF device for lookup */
>>  	fl4->flowi4_oif = dev->ifindex;
>> -	fl4->flowi4_tos = tos & IPTOS_RT_MASK;
> 
> 	Ops, now output route has the same bug as the
> input route from 3.6: bit 1 reaches the rule matching,
> we left all 4 bits from RT_TOS in flowi4_tos.

Exactly, the next change in RT_TOS will fix this. We shouldn't be able
to store the lower bits in flowi4_tos then.

> 
>> -	fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
>> -			     RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
>>  
>>  	rc = fib_lookup(net, fl4, &res, 0);
>>  	if (!rc) {
>> diff --git a/include/net/route.h b/include/net/route.h
>> index 8937e36fc9fd11..c374c217de7d7f 100644
>> --- a/include/net/route.h
>> +++ b/include/net/route.h
>> @@ -133,8 +136,10 @@ static inline struct rtable *ip_route_output_key(struct net *net, struct flowi4
>>  	return ip_route_output_flow(net, flp, NULL);
>>  }
>>  
>> -static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
>> -					     __be32 saddr, u8 tos, int oif)
>> +static inline struct rtable *ip_route_output_scope(struct net *net,
>> +						   __be32 daddr,
>> +						   __be32 saddr, u8 tos,
>> +						   u8 scope, int oif)
> 
> 	scope added but ignored?

Ups, I fixed this locally. Thanks for spotting.

> 
>>  {
>>  	struct flowi4 fl4 = {
>>  		.flowi4_oif = oif,
>> @@ -145,6 +150,18 @@ static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
>>  	return ip_route_output_key(net, &fl4);
>>  }
>>  
>> +static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
>> +					     __be32 saddr, u8 tos, int oif)
>> +{
>> +	return ip_route_output_scope(net, daddr, saddr, tos, RT_SCOPE_UNIVERSE,
>> +				     oif);
>> +}
>> +
>> +static inline struct rtable *ip_route_output_link(struct net *net, __be32 daddr)
>> +{
>> +	return ip_route_output_scope(net, daddr, 0, 0, RT_SCOPE_LINK, 0);
>> +}
>> +
>>  static inline struct rtable *ip_route_output_ports(struct net *net, struct flowi4 *fl4,
>>  						   struct sock *sk,
>>  						   __be32 daddr, __be32 saddr,
> 
>> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
>> index e8f499d224ec2a..2446727ca6f980 100644
>> --- a/net/ipv4/route.c
>> +++ b/net/ipv4/route.c
>> @@ -2162,7 +2162,6 @@ struct rtable *__ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
>>  					  int mp_hash)
>>  {
>>  	struct net_device *dev_out = NULL;
>> -	__u8 tos = RT_FL_TOS(fl4);
>>  	unsigned int flags = 0;
>>  	struct fib_result res;
>>  	struct rtable *rth;
>> @@ -2180,9 +2179,6 @@ struct rtable *__ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
>>  	if (master_idx)
>>  		fl4->flowi4_oif = master_idx;
>>  	fl4->flowi4_iif = LOOPBACK_IFINDEX;
>> -	fl4->flowi4_tos = tos & IPTOS_RT_MASK;
> 
> 	Now bit 1 reaches rule matching if not masked
> with IPTOS_RT_MASK.
> 
>> -	fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
>> -			 RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
>>  
>>  	rcu_read_lock();
>>  	if (fl4->saddr) {
> 
> 	Also, do you plan to include a patch that adds the
> missing IPTOS_RT_MASK for input route or it is a material
> for the net tree?

I am not sure if I want to go for net tree. I would much rather focus on
fixing those things up upstream so it is easy to work with in future and
isn't as error-prone as it is now. This RFC was mostly about if the
re-factoring would be accepted upstream.

Bye,
Hannes

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Mon, 13 Jun 2016 16:02:53 -0600

> I found this helper to be unhelpful developing the VRF changes. Really
> obfuscates the initialization of the flow struct. Happy to see it go.

The whole point was so that it would actually be easier.

If flow structures only get initialized by 2 or 3 helpers, then when
adding new flow fields you only need to update the helpers.

Since you have to update the helper argument signatures in order to do
this, you would be forced to update all call sites and make sure they
passed in suitable new argument(s).

But if people don't use the helpers, and initialize flow structures
on their own, yeah that defeats the whole mechanism and things will
seem harder and "unhelpful".

I really think this is a step backwards.

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[David Ahern]

On 6/15/16 1:48 PM, David Miller wrote:
> But if people don't use the helpers, and initialize flow structures
> on their own, yeah that defeats the whole mechanism and things will
> seem harder and "unhelpful".

That's my point -- the flow struct does not have a consistent 
initializer. There have been a number of bug patches over the past year 
like 4cfc86f3dae6 to handle uninitialized elements. My suggestion here 
is the same as 4cfc86f3dae6 which is to initialize the flow when it is 
declared.

Consider the recent change from Hannes for 38b7097b55b6. fl6 can be 
initialized when it is declared with a bit of straightforward 
refactoring -- icmp6_send has an easy split into 2.

Seems like that is a more robust long term solution considering the 
various init use cases.

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Wed, 15 Jun 2016 14:29:28 -0600

> On 6/15/16 1:48 PM, David Miller wrote:
>> But if people don't use the helpers, and initialize flow structures
>> on their own, yeah that defeats the whole mechanism and things will
>> seem harder and "unhelpful".
> 
> That's my point -- the flow struct does not have a consistent
> initializer. There have been a number of bug patches over the past
> year like 4cfc86f3dae6 to handle uninitialized elements. My suggestion
> here is the same as 4cfc86f3dae6 which is to initialize the flow when
> it is declared.
> 
> Consider the recent change from Hannes for 38b7097b55b6. fl6 can be
> initialized when it is declared with a bit of straightforward
> refactoring -- icmp6_send has an easy split into 2.
> 
> Seems like that is a more robust long term solution considering the
> various init use cases.

The danger with initializers is it puts the burdon all over the tree.

So now there are many places that need to be audited when a new
element is added.

So from my perspective, the bug is that the code in question did
things by hand rather than using the helper function.

If everyone used the flow initializer helpers, the compiler would
tell us every place that needs to be fixed up.

By doing initializers inline, this process of discovery is more
difficult and error prone.

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[Hannes Frederic Sowa]

On 16.06.2016 23:18, David Miller wrote:
> From: David Ahern <dsa@cumulusnetworks.com>
> Date: Wed, 15 Jun 2016 14:29:28 -0600
> 
>> On 6/15/16 1:48 PM, David Miller wrote:
>>> But if people don't use the helpers, and initialize flow structures
>>> on their own, yeah that defeats the whole mechanism and things will
>>> seem harder and "unhelpful".
>>
>> That's my point -- the flow struct does not have a consistent
>> initializer. There have been a number of bug patches over the past
>> year like 4cfc86f3dae6 to handle uninitialized elements. My suggestion
>> here is the same as 4cfc86f3dae6 which is to initialize the flow when
>> it is declared.
>>
>> Consider the recent change from Hannes for 38b7097b55b6. fl6 can be
>> initialized when it is declared with a bit of straightforward
>> refactoring -- icmp6_send has an easy split into 2.
>>
>> Seems like that is a more robust long term solution considering the
>> various init use cases.
> 
> The danger with initializers is it puts the burdon all over the tree.
> 
> So now there are many places that need to be audited when a new
> element is added.
> 
> So from my perspective, the bug is that the code in question did
> things by hand rather than using the helper function.
> 
> If everyone used the flow initializer helpers, the compiler would
> tell us every place that needs to be fixed up.
> 
> By doing initializers inline, this process of discovery is more
> difficult and error prone.

I see both sides and acknowledge the fact that we don't generate compile
errors any more when we add fields. OTOH we already have a lot of
places, probably even more, which don't use the initialization helper
functions as they just need to feed in a subset of the flowi4
information (memset and fl4.daddr = <>; and some other attributes is a
common case. Most of the time a new attribute that gets added to flow4i
only decreases the scope of the actual lookup, thus a missing attribute
doesn't necessarily indicate a bug.

In my local branch I adapted the changes from Julian so far. I will
revisit the initialization of flowi4, thanks for all the input so far.

Currently I have difficulties keeping user space behavior the same while
improving the ECN situation, as ToS simply specifies the use of the
second bit. I can't improve the ECN situation in a transparent way.

Bye,
Hannes

Re: [PATCH RFC 1/3] ipv4: make flow functions more grepable by removing flowi4_init_output

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 16 Jun 2016 23:39:12 +0200

> Most of the time a new attribute that gets added to flow4i only
> decreases the scope of the actual lookup, thus a missing attribute
> doesn't necessarily indicate a bug.

That's true, and it works well so long as the value zero provides
the behavior that existed before the new flow key was added.