net: fix flowlabel inconsistency in reset packet

PATCH In review Shaohua Li 3 participants 7 messages Dec 1, 2017 – Dec 5, 2017

Subsystems (5)

Networking Network drivers Documentation TCP +1 more subsystems

lore

Share & tools

lore

Shaohua Li

author

2017-12-01 23:32 UTC #0 lore raw

v3 cover net: fix flowlabel inconsistency in reset packet

From: Shaohua Li <[email protected]>

Hi,

Please see below tcpdump output:
21:00:48.109122 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 40) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [S], cksum 0x0529 (incorrect -> 0xf56c), seq 3282214508, win 43690, options [mss 65476,sackOK,TS val 2500903437 ecr 0,nop,wscale 7], length 0 21:00:48.109381 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 40) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [S.], cksum 0x0529 (incorrect -> 0x49ad), seq 1923801573, ack 3282214509, win 43690, options [mss 65476,sackOK,TS val 2500903437 ecr 2500903437,nop,wscale 7], length 0 21:00:48.109548 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1bdf), seq 1, ack 1, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 0 21:00:48.109823 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 62) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [P.], cksum 0x053f (incorrect -> 0xb8b1), seq 1:31, ack 1, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 30 21:00:48.109910 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [.], cksum 0x0521 (incorrect -> 0x1bc1), seq 1, ack 31, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 0 21:00:48.110043 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 56) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [P.], cksum 0x0539 (incorrect -> 0xb726), seq 1:25, ack 31, win 342, options [nop,nop,TS val 2500903438 ecr 2500903437], length 24 21:00:48.110173 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1ba7), seq 31, ack 25, win 342, options [nop,nop,TS val 2500903438 ecr 2500903438], length 0 21:00:48.110211 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [F.], cksum 0x0521 (incorrect -> 0x1ba7), seq 25, ack 31, win 342, options [nop,nop,TS val 2500903438 ecr 2500903437], length 0 21:00:48.151099 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1ba6), seq 31, ack 26, win 342, options [nop,nop,TS val 2500903438 ecr 2500903438], length 0 21:00:49.110524 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 56) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [P.], cksum 0x0539 (incorrect -> 0xb324), seq 31:55, ack 26, win 342, options [nop,nop,TS val 2500904438 ecr 2500903438], length 24 21:00:49.110637 IP6 (flowlabel 0xb34d5, hlim 64, next-header TCP (6) payload length: 20) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [R], cksum 0x0515 (incorrect -> 0x668c), seq 1923801599, win 0, length 0

The tcp reset packet has a different flowlabel, which causes our router doesn't correctly close tcp connection. We are using flowlabel to do load balance. Routers in the path maintain connection state. So if flow label changes, the packet is routed through a different router. In this case, the old router doesn't get the reset packet to close the tcp connection.

The reason is the normal packet gets the skb->hash from sk->sk_txhash, which is generated randomly. ip6_make_flowlabel then uses the hash to create a flowlabel. The reset packet doesn't get assigned a hash, so the flowlabel is calculated with flowi6.

The patches fix the issue.

Thanks,
Shaohua

V2->V3:
- Address Tom's comments
- Add a new sysctl suggested by Tom

Shaohua Li (3):
net-next: use five-tuple hash for sk_txhash
net-next: copy user configured flowlabel to reset packet
net: add a sysctl to make auto flowlabel consistent

 Documentation/networking/ip-sysctl.txt |  7 +++++++
 include/linux/tcp.h                    |  5 +++++
 include/net/netns/ipv6.h               |  1 +
 include/net/sock.h                     | 35 +++++++++++++++-----------------
 include/net/tcp.h                      |  2 +-
 net/ipv4/datagram.c                    |  2 +-
 net/ipv4/syncookies.c                  |  4 +++-
 net/ipv4/tcp_input.c                   |  1 -
 net/ipv4/tcp_ipv4.c                    | 18 ++++++++++++-----
 net/ipv4/tcp_output.c                  |  1 -
 net/ipv6/af_inet6.c                    |  1 +
 net/ipv6/datagram.c                    |  4 +++-
 net/ipv6/syncookies.c                  |  3 ++-
 net/ipv6/sysctl_net_ipv6.c             |  8 ++++++++
 net/ipv6/tcp_ipv6.c                    | 37 ++++++++++++++++++++++++++++------
 15 files changed, 92 insertions(+), 37 deletions(-)

Signature · 1 line

2.9.5

Shaohua Li

author

2017-12-01 23:32 UTC #1 lore raw

PATCH v3 1/3 net-next: use five-tuple hash for sk_txhash

From: Shaohua Li <[email protected]>

We are using sk_txhash to calculate flowlabel, but sk_txhash isn't always available, for example, in inet_timewait_sock. This causes problem for reset packet, which will have a different flowlabel. This causes our router doesn't correctly close tcp connection. We are using flowlabel to do load balance. Routers in the path maintain connection state. So if flow label changes, the packet is routed through a different router. In this case, the old router doesn't get the reset packet to close the tcp connection.

Per Tom's suggestion, we switch back to five-tuple hash, so we can reconstruct correct flowlabel for reset packet.

At most places, we already have the flowi info, so we directly use it build sk_txhash. For synack, we do this after route search. At that time, we have the flowi info ready, so don't need to create the flowi info again.

Cc Martin KaFai Lau <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Florent Fourcot <[email protected]>

Cc Cong Wang <[email protected]>

Cc Tom Herbert <[email protected]>

Signed-off-by Shaohua Li <[email protected]>

---
 include/linux/tcp.h   |  5 +++++
 include/net/sock.h    | 17 ++++++-----------
 include/net/tcp.h     |  2 +-
 net/ipv4/datagram.c   |  2 +-
 net/ipv4/syncookies.c |  4 +++-
 net/ipv4/tcp_input.c  |  1 -
 net/ipv4/tcp_ipv4.c   | 18 +++++++++++++-----
 net/ipv4/tcp_output.c |  1 -
 net/ipv6/datagram.c   |  4 +++-
 net/ipv6/syncookies.c |  3 ++-
 net/ipv6/tcp_ipv6.c   | 19 ++++++++++++++-----
 11 files changed, 48 insertions(+), 28 deletions(-)

include/linux/tcp.h +5 -0

index df5d97a..227e8b2 100644
@@ -139,6 +139,11 @@ struct tcp_request_sock {
 						  */
 };
 
+static inline void tcp_rsk_set_txhash(struct tcp_request_sock *rsk, u32 hash)
+{
+	rsk->txhash = hash;
+}
+
 static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req)
 {
 	return (struct tcp_request_sock *)req;

include/net/sock.h +6 ~~-11~~

index 79e1a2c..b9cb9d2 100644
@@ -1729,22 +1729,17 @@ static inline kuid_t sock_net_uid(const struct net *net, const struct sock *sk)
 	return sk ? sk->sk_uid : make_kuid(net->user_ns, 0);
 }
 
-static inline u32 net_tx_rndhash(void)
+static inline void sk_set_txhash(struct sock *sk, u32 hash)
 {
-	u32 v = prandom_u32();
-
-	return v ?: 1;
-}
-
-static inline void sk_set_txhash(struct sock *sk)
-{
-	sk->sk_txhash = net_tx_rndhash();
+	sk->sk_txhash = hash;
 }
 
 static inline void sk_rethink_txhash(struct sock *sk)
 {
-	if (sk->sk_txhash)
-		sk_set_txhash(sk);
+	if (sk->sk_txhash) {
+		u32 v = prandom_u32();
+		sk->sk_txhash = v ?: 1;
+	}
 }
 
 static inline struct dst_entry *

include/net/tcp.h +1 -1

index 4e09398..a5c28be 100644
@@ -1840,7 +1840,7 @@ struct tcp_request_sock_ops {
 				 __u16 *mss);
 #endif
 	struct dst_entry *(*route_req)(const struct sock *sk, struct flowi *fl,
-				       const struct request_sock *req);
+				       struct request_sock *req);
 	u32 (*init_seq)(const struct sk_buff *skb);
 	u32 (*init_ts_off)(const struct net *net, const struct sk_buff *skb);
 	int (*send_synack)(const struct sock *sk, struct dst_entry *dst,

net/ipv4/datagram.c +1 -1

index f915abf..1f2f9fc 100644
@@ -74,7 +74,7 @@ int __ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
 	inet->inet_daddr = fl4->daddr;
 	inet->inet_dport = usin->sin_port;
 	sk->sk_state = TCP_ESTABLISHED;
-	sk_set_txhash(sk);
+	sk_set_txhash(sk, get_hash_from_flowi4(fl4));
 	inet->inet_id = jiffies;
 
 	sk_dst_set(sk, &rt->dst);

net/ipv4/syncookies.c +3 -1

index fda37f2..ecf6e7a 100644
@@ -335,7 +335,6 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	treq->rcv_isn		= ntohl(th->seq) - 1;
 	treq->snt_isn		= cookie;
 	treq->ts_off		= 0;
-	treq->txhash		= net_tx_rndhash();
 	req->mss		= mss;
 	ireq->ir_num		= ntohs(th->dest);
 	ireq->ir_rmt_port	= th->source;

@@ -376,6 +375,9 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 			   opt->srr ? opt->faddr : ireq->ir_rmt_addr,
 			   ireq->ir_loc_addr, th->source, th->dest, sk->sk_uid);
 	security_req_classify_flow(req, flowi4_to_flowi(&fl4));
+
+	tcp_rsk_set_txhash(treq, get_hash_from_flowi4(&fl4));
+
 	rt = ip_route_output_key(sock_net(sk), &fl4);
 	if (IS_ERR(rt)) {
 		reqsk_free(req);

net/ipv4/tcp_input.c +0 -1

index 734cfc8..e886c28 100644
@@ -6288,7 +6288,6 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 	}
 
 	tcp_rsk(req)->snt_isn = isn;
-	tcp_rsk(req)->txhash = net_tx_rndhash();
 	tcp_openreq_init_rwin(req, sk, dst);
 	if (!want_cookie) {
 		tcp_reqsk_record_syn(sk, req, skb);

net/ipv4/tcp_ipv4.c +13 -5

index c6bc0c4..8c47870 100644
@@ -222,7 +222,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	if (err)
 		goto failure;
 
-	sk_set_txhash(sk);
+	sk_set_txhash(sk, get_hash_from_flowi4(fl4));
 
 	rt = ip_route_newports(fl4, rt, orig_sport, orig_dport,
 			       inet->inet_sport, inet->inet_dport, sk);

@@ -871,8 +871,12 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
 	struct sk_buff *skb;
 
 	/* First, grab a route. */
-	if (!dst && (dst = inet_csk_route_req(sk, &fl4, req)) == NULL)
-		return -1;
+	if (!dst) {
+		if ((dst = inet_csk_route_req(sk, &fl4, req)) == NULL)
+			return -1;
+
+		tcp_rsk_set_txhash(tcp_rsk(req), get_hash_from_flowi4(&fl4));
+	}
 
 	skb = tcp_make_synack(sk, dst, req, foc, synack_type);

@@ -1278,9 +1282,13 @@ static void tcp_v4_init_req(struct request_sock *req,
 
 static struct dst_entry *tcp_v4_route_req(const struct sock *sk,
 					  struct flowi *fl,
-					  const struct request_sock *req)
+					  struct request_sock *req)
 {
-	return inet_csk_route_req(sk, &fl->u.ip4, req);
+	struct dst_entry *dst = inet_csk_route_req(sk, &fl->u.ip4, req);
+	if (dst)
+		tcp_rsk_set_txhash(tcp_rsk(req),
+				   get_hash_from_flowi4(&fl->u.ip4));
+	return dst;
 }
 
 struct request_sock_ops tcp_request_sock_ops __read_mostly = {

net/ipv4/tcp_output.c +0 -1

index a4d214c..94e56cb 100644
@@ -3752,7 +3752,6 @@ int tcp_rtx_synack(const struct sock *sk, struct request_sock *req)
 	struct flowi fl;
 	int res;
 
-	tcp_rsk(req)->txhash = net_tx_rndhash();
 	res = af_ops->send_synack(sk, NULL, &fl, req, NULL, TCP_SYNACK_NORMAL);
 	if (!res) {
 		__TCP_INC_STATS(sock_net(sk), TCP_MIB_RETRANSSEGS);

net/ipv6/datagram.c +3 -1

index a1f9187..003af09 100644
@@ -150,6 +150,7 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
 	int			addr_type;
 	int			err;
 	__be32			fl6_flowlabel = 0;
+	struct flowi6		fl6;
 
 	if (usin->sin6_family == AF_INET) {
 		if (__ipv6_only_sock(sk))

@@ -260,7 +261,8 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
 	}
 
 	sk->sk_state = TCP_ESTABLISHED;
-	sk_set_txhash(sk);
+	ip6_datagram_flow_key_init(&fl6, sk);
+	sk_set_txhash(sk, get_hash_from_flowi6(&fl6));
 out:
 	return err;
 }

net/ipv6/syncookies.c +2 -1

index e7a3a6b..6897aca 100644
@@ -216,7 +216,6 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 	treq->rcv_isn = ntohl(th->seq) - 1;
 	treq->snt_isn = cookie;
 	treq->ts_off = 0;
-	treq->txhash = net_tx_rndhash();
 
 	/*
 	 * We need to lookup the dst_entry to get the correct window size.

@@ -238,6 +237,8 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 		fl6.flowi6_uid = sk->sk_uid;
 		security_req_classify_flow(req, flowi6_to_flowi(&fl6));
 
+		tcp_rsk_set_txhash(treq, get_hash_from_flowi6(&fl6));
+
 		dst = ip6_dst_lookup_flow(sk, &fl6, final_p);
 		if (IS_ERR(dst))
 			goto out_free;

net/ipv6/tcp_ipv6.c +14 -5

index 6bb98c9..1e4ce06 100644
@@ -286,7 +286,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 	if (err)
 		goto late_failure;
 
-	sk_set_txhash(sk);
+	sk_set_txhash(sk, get_hash_from_flowi6(&fl6));
 
 	if (likely(!tp->repair)) {
 		if (!tp->write_seq)

@@ -470,9 +470,13 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
 	int err = -ENOMEM;
 
 	/* First, grab a route. */
-	if (!dst && (dst = inet6_csk_route_req(sk, fl6, req,
+	if (!dst) {
+		if ((dst = inet6_csk_route_req(sk, fl6, req,
 					       IPPROTO_TCP)) == NULL)
-		goto done;
+			goto done;
+
+		tcp_rsk_set_txhash(tcp_rsk(req), get_hash_from_flowi6(fl6));
+	}
 
 	skb = tcp_make_synack(sk, dst, req, foc, synack_type);

@@ -743,9 +747,14 @@ static void tcp_v6_init_req(struct request_sock *req,
 
 static struct dst_entry *tcp_v6_route_req(const struct sock *sk,
 					  struct flowi *fl,
-					  const struct request_sock *req)
+					  struct request_sock *req)
 {
-	return inet6_csk_route_req(sk, &fl->u.ip6, req, IPPROTO_TCP);
+	struct dst_entry *dst;
+	dst = inet6_csk_route_req(sk, &fl->u.ip6, req, IPPROTO_TCP);
+	if (dst)
+		tcp_rsk_set_txhash(tcp_rsk(req),
+				   get_hash_from_flowi6(&fl->u.ip6));
+	return dst;
 }
 
 struct request_sock_ops tcp6_request_sock_ops __read_mostly = {

Signature · 1 line

2.9.5

Shaohua Li

author

2017-12-01 23:32 UTC #2 lore raw

PATCH v3 2/3 net-next: copy user configured flowlabel to reset packet

In reply to Shaohua Li

From: Shaohua Li <[email protected]>

Reset packet doesn't use user configured flowlabel, instead, it always uses 0. This will cause inconsistency for flowlabel. tw sock already records flowlabel info, so we can directly use it.

Cc Martin KaFai Lau <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Florent Fourcot <[email protected]>

Cc Cong Wang <[email protected]>

Cc Tom Herbert <[email protected]>

Signed-off-by Shaohua Li <[email protected]>

---
 net/ipv6/tcp_ipv6.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

net/ipv6/tcp_ipv6.c +17 -1

index 1e4ce06..b8383be 100644
@@ -902,6 +902,8 @@ static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 	struct sock *sk1 = NULL;
 #endif
 	int oif = 0;
+	u8 tclass = 0;
+	__be32 flowlabel = 0;
 
 	if (th->rst)
 		return;

@@ -955,7 +957,21 @@ static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 		trace_tcp_send_reset(sk, skb);
 	}
 
-	tcp_v6_send_response(sk, skb, seq, ack_seq, 0, 0, 0, oif, key, 1, 0, 0);
+	if (sk) {
+		if (sk_fullsock(sk)) {
+			struct ipv6_pinfo *np = inet6_sk(sk);
+
+			tclass = np->tclass;
+			flowlabel = np->flow_label & IPV6_FLOWLABEL_MASK;
+		} else {
+			struct inet_timewait_sock *tw = inet_twsk(sk);
+
+			tclass = tw->tw_tclass;
+			flowlabel = cpu_to_be32(tw->tw_flowlabel);
+		}
+	}
+	tcp_v6_send_response(sk, skb, seq, ack_seq, 0, 0, 0, oif, key, 1,
+		tclass, flowlabel);
 
 #ifdef CONFIG_TCP_MD5SIG
 out:

Signature · 1 line

2.9.5

Shaohua Li

author

2017-12-01 23:32 UTC #3 lore raw

PATCH v3 3/3 net: add a sysctl to make auto flowlabel consistent

In reply to Shaohua Li

From: Shaohua Li <[email protected]>

Currently if there is negative routing, we change sock's txhash, so the sock will have a different flowlabel and route to different path. According to Tom, we'd better to have option to enable this, because some routers require flowlabel consistent. By default, we maintain consistent flowlabel, eg, negative routing doesn't change flowlabel.

Signed-off-by Shaohua Li <[email protected]>

---
 Documentation/networking/ip-sysctl.txt |  7 +++++++
 include/net/netns/ipv6.h               |  1 +
 include/net/sock.h                     | 28 +++++++++++++++-------------
 net/ipv6/af_inet6.c                    |  1 +
 net/ipv6/sysctl_net_ipv6.c             |  8 ++++++++
 5 files changed, 32 insertions(+), 13 deletions(-)

Documentation/networking/ip-sysctl.txt +7 -0

index 46c7e10..14132a0 100644
@@ -1345,6 +1345,13 @@ auto_flowlabels - INTEGER
 	   be disabled by the socket option
 	Default: 1
 
+consistent_auto_flowlabel - BOOLEAN
+	When auto_flowlabels is enabled, this option makes socket flowlabel
+	consistent in the lifetime.
+	TRUE: enabled
+	FALSE: disabled
+	Default: TRUE
+
 flowlabel_state_ranges - BOOLEAN
 	Split the flow label number space into two ranges. 0-0x7FFFF is
 	reserved for the IPv6 flow manager facility, 0x80000-0xFFFFF

include/net/netns/ipv6.h +1 -0

index 987cc45..e55f851 100644
@@ -30,6 +30,7 @@ struct netns_sysctl_ipv6 {
 	int ip6_rt_min_advmss;
 	int flowlabel_consistency;
 	int auto_flowlabels;
+	int consistent_auto_flowlabel;
 	int icmpv6_time;
 	int anycast_src_echo_reply;
 	int ip_nonlocal_bind;

include/net/sock.h +15 ~~-13~~

index b9cb9d2..45e868f 100644
@@ -1729,6 +1729,18 @@ static inline kuid_t sock_net_uid(const struct net *net, const struct sock *sk)
 	return sk ? sk->sk_uid : make_kuid(net->user_ns, 0);
 }
 
+static inline
+struct net *sock_net(const struct sock *sk)
+{
+	return read_pnet(&sk->sk_net);
+}
+
+static inline
+void sock_net_set(struct sock *sk, struct net *net)
+{
+	write_pnet(&sk->sk_net, net);
+}
+
 static inline void sk_set_txhash(struct sock *sk, u32 hash)
 {
 	sk->sk_txhash = hash;

@@ -1736,7 +1748,9 @@ static inline void sk_set_txhash(struct sock *sk, u32 hash)
 
 static inline void sk_rethink_txhash(struct sock *sk)
 {
-	if (sk->sk_txhash) {
+	struct net *net = sock_net(sk);
+
+	if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
 		u32 v = prandom_u32();
 		sk->sk_txhash = v ?: 1;
 	}

@@ -2291,18 +2305,6 @@ static inline void sk_eat_skb(struct sock *sk, struct sk_buff *skb)
 	__kfree_skb(skb);
 }
 
-static inline
-struct net *sock_net(const struct sock *sk)
-{
-	return read_pnet(&sk->sk_net);
-}
-
-static inline
-void sock_net_set(struct sock *sk, struct net *net)
-{
-	write_pnet(&sk->sk_net, net);
-}
-
 static inline struct sock *skb_steal_sock(struct sk_buff *skb)
 {
 	if (skb->sk) {

net/ipv6/af_inet6.c +1 -0

index c26f712..fe9b312 100644
@@ -807,6 +807,7 @@ static int __net_init inet6_net_init(struct net *net)
 	net->ipv6.sysctl.icmpv6_time = 1*HZ;
 	net->ipv6.sysctl.flowlabel_consistency = 1;
 	net->ipv6.sysctl.auto_flowlabels = IP6_DEFAULT_AUTO_FLOW_LABELS;
+	net->ipv6.sysctl.consistent_auto_flowlabel = 1;
 	net->ipv6.sysctl.idgen_retries = 3;
 	net->ipv6.sysctl.idgen_delay = 1 * HZ;
 	net->ipv6.sysctl.flowlabel_state_ranges = 0;

net/ipv6/sysctl_net_ipv6.c +8 -0

index a789a8a..8908092 100644
@@ -126,6 +126,13 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "consistent_auto_flowlabel",
+		.data		= &init_net.ipv6.sysctl.consistent_auto_flowlabel,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
 	{ }
 };

@@ -190,6 +197,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
 	ipv6_table[11].data = &net->ipv6.sysctl.max_hbh_opts_cnt;
 	ipv6_table[12].data = &net->ipv6.sysctl.max_dst_opts_len;
 	ipv6_table[13].data = &net->ipv6.sysctl.max_hbh_opts_len;
+	ipv6_table[14].data = &net->ipv6.sysctl.consistent_auto_flowlabel;
 
 	ipv6_route_table = ipv6_route_sysctl_init(net);
 	if (!ipv6_route_table)

Signature · 1 line

2.9.5

Tom Herbert

2017-12-02 01:14 UTC #4 lore raw

Re: [PATCH net-next V3 3/3] net: add a sysctl to make auto flowlabel consistent

Quoted 23 lines · Shaohua Li

From: Shaohua Li <[email protected]>
Currently if there is negative routing, we change sock's txhash, so the sock will have a different flowlabel and route to different path. According to Tom, we'd better to have option to enable this, because some routers require flowlabel consistent. By default, we maintain consistent flowlabel, eg, negative routing doesn't change flowlabel.
Suggested-by Tom Herbert <[email protected]>
Signed-off-by Shaohua Li <[email protected]>
---
 Documentation/networking/ip-sysctl.txt |  7 +++++++
 include/net/netns/ipv6.h               |  1 +
 include/net/sock.h                     | 28 +++++++++++++++-------------
 net/ipv6/af_inet6.c                    |  1 +
 net/ipv6/sysctl_net_ipv6.c             |  8 ++++++++
 5 files changed, 32 insertions(+), 13 deletions(-)
Documentation/networking/ip-sysctl.txt +0 -0
index 46c7e10..14132a0 100644

Documentation/networking/ip-sysctl.txt auto_flowlabels - INTEGER · line 1348

Quoted 4 lines

Documentation/networking/ip-sysctl.txt +1 -0

@@ -1345,6 +1345,13 @@ auto_flowlabels - INTEGER
           be disabled by the socket option
        Default: 1
 
+consistent_auto_flowlabel - BOOLEAN

I think we should call it consistent_txhash since this isn't just about the flow label.

Quoted 115 lines

Documentation/networking/ip-sysctl.txt +6 -0

+       When auto_flowlabels is enabled, this option makes socket flowlabel
+       consistent in the lifetime.
+       TRUE: enabled
+       FALSE: disabled
+       Default: TRUE
+
 flowlabel_state_ranges - BOOLEAN
        Split the flow label number space into two ranges. 0-0x7FFFF is
        reserved for the IPv6 flow manager facility, 0x80000-0xFFFFF

include/net/netns/ipv6.h +1 -0

index 987cc45..e55f851 100644
@@ -30,6 +30,7 @@ struct netns_sysctl_ipv6 {
        int ip6_rt_min_advmss;
        int flowlabel_consistency;
        int auto_flowlabels;
+       int consistent_auto_flowlabel;
        int icmpv6_time;
        int anycast_src_echo_reply;
        int ip_nonlocal_bind;

include/net/sock.h +15 ~~-13~~

index b9cb9d2..45e868f 100644
@@ -1729,6 +1729,18 @@ static inline kuid_t sock_net_uid(const struct net *net, const struct sock *sk)
        return sk ? sk->sk_uid : make_kuid(net->user_ns, 0);
 }
 
+static inline
+struct net *sock_net(const struct sock *sk)
+{
+       return read_pnet(&sk->sk_net);
+}
+
+static inline
+void sock_net_set(struct sock *sk, struct net *net)
+{
+       write_pnet(&sk->sk_net, net);
+}
+
 static inline void sk_set_txhash(struct sock *sk, u32 hash)
 {
        sk->sk_txhash = hash;

@@ -1736,7 +1748,9 @@ static inline void sk_set_txhash(struct sock *sk, u32 hash)
 
 static inline void sk_rethink_txhash(struct sock *sk)
 {
-       if (sk->sk_txhash) {
+       struct net *net = sock_net(sk);
+
+       if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
                u32 v = prandom_u32();
                sk->sk_txhash = v ?: 1;
        }

@@ -2291,18 +2305,6 @@ static inline void sk_eat_skb(struct sock *sk, struct sk_buff *skb)
        __kfree_skb(skb);
 }
 
-static inline
-struct net *sock_net(const struct sock *sk)
-{
-       return read_pnet(&sk->sk_net);
-}
-
-static inline
-void sock_net_set(struct sock *sk, struct net *net)
-{
-       write_pnet(&sk->sk_net, net);
-}
-
 static inline struct sock *skb_steal_sock(struct sk_buff *skb)
 {
        if (skb->sk) {

net/ipv6/af_inet6.c +1 -0

index c26f712..fe9b312 100644
@@ -807,6 +807,7 @@ static int __net_init inet6_net_init(struct net *net)
        net->ipv6.sysctl.icmpv6_time = 1*HZ;
        net->ipv6.sysctl.flowlabel_consistency = 1;
        net->ipv6.sysctl.auto_flowlabels = IP6_DEFAULT_AUTO_FLOW_LABELS;
+       net->ipv6.sysctl.consistent_auto_flowlabel = 1;
        net->ipv6.sysctl.idgen_retries = 3;
        net->ipv6.sysctl.idgen_delay = 1 * HZ;
        net->ipv6.sysctl.flowlabel_state_ranges = 0;

net/ipv6/sysctl_net_ipv6.c +8 -1

index a789a8a..8908092 100644
@@ -126,6 +126,13 @@ static struct ctl_table ipv6_table_template[] = {
                .mode           = 0644,
                .proc_handler   = proc_dointvec
        },
+       {
+               .procname       = "consistent_auto_flowlabel",
+               .data           = &init_net.ipv6.sysctl.consistent_auto_flowlabel,
+               .maxlen         = sizeof(int),
+               .mode           = 0644,
+               .proc_handler   = proc_dointvec
+       },
        { }
 };

@@ -190,6 +197,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
        ipv6_table[11].data = &net->ipv6.sysctl.max_hbh_opts_cnt;
        ipv6_table[12].data = &net->ipv6.sysctl.max_dst_opts_len;
        ipv6_table[13].data = &net->ipv6.sysctl.max_hbh_opts_len;
+       ipv6_table[14].data = &net->ipv6.sysctl.consistent_auto_flowlabel;
 
        ipv6_route_table = ipv6_route_sysctl_init(net);
        if (!ipv6_route_table)
--

2.9.5

kbuild test robot

build bot

2017-12-05 02:05 UTC #5 lore raw

Re: [PATCH net-next V3 3/3] net: add a sysctl to make auto flowlabel consistent

In reply to Shaohua Li

I love your patch! Yet something to improve:

Hi Shaohua,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Shaohua-Li/net-fix-flowlabel-inconsistency-in-reset-packet/20171205-072626
config: x86_64-randconfig-x004-201749 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

In file included from include/net/cls_cgroup.h:19:0,

                    from net/socket.c:98:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:1753:29: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
                                ^~~~
                                ipv4

vim +1753 include/net/sock.h

1748
1749 static inline void sk_rethink_txhash(struct sock *sk)
1750 {
1751 struct net *net = sock_net(sk);
1752

> 1753		if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {

1754 u32 v = prandom_u32();
1755 sk->sk_txhash = v ?: 1;
1756 }
1757 }
1758

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

kbuild test robot

build bot

2017-12-05 02:11 UTC #6 lore raw

Re: [PATCH net-next V3 3/3] net: add a sysctl to make auto flowlabel consistent

In reply to Shaohua Li

I love your patch! Perhaps something to improve:

Hi Shaohua,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url: https://github.com/0day-ci/linux/commits/Shaohua-Li/net-fix-flowlabel-inconsistency-in-reset-packet/20171205-072626
config: i386-randconfig-x002-12041112 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386

All warnings (new ones prefixed by >>):

In file included from include/linux/kernel.h:10:0,

                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net//x25/af_x25.c:40:
   include/net/sock.h: In function 'sk_rethink_txhash':
   include/net/sock.h:1753:29: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
                                ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> include/net/sock.h:1753:2: note: in expansion of macro 'if'
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
     ^~
   include/net/sock.h:1753:29: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
                                ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> include/net/sock.h:1753:2: note: in expansion of macro 'if'
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
     ^~
   include/net/sock.h:1753:29: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
                                ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> include/net/sock.h:1753:2: note: in expansion of macro 'if'
     if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {
     ^~

vim +/if +1753 include/net/sock.h

1748
1749 static inline void sk_rethink_txhash(struct sock *sk)
1750 {
1751 struct net *net = sock_net(sk);
1752

> 1753		if (sk->sk_txhash && !net->ipv6.sysctl.consistent_auto_flowlabel) {

1754 u32 v = prandom_u32();
1755 sk->sk_txhash = v ?: 1;
1756 }
1757 }
1758

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation