ipv6: fix flowlabel issue for reset packet

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel,

include/net/ipv6.h +1 -3

-                                       struct flowi6 *fl6)
+                                       struct flowi6 *fl6, u32 hash)
 {
-       u32 hash;
-
        /* @flowlabel may include more than a flow label, eg, the traffic class.
         * Here we want only the flow label value.
         */

Quoted 11 lines · Shaohua Li

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel,

include/net/ipv6.h +1 -3

-                                       struct flowi6 *fl6)
+                                       struct flowi6 *fl6, u32 hash)
 {
-       u32 hash;
-
        /* @flowlabel may include more than a flow label, eg, the traffic class.
         * Here we want only the flow label value.
         */

include/net/ipv6.h static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb, · line 790

Quoted 6 lines

include/net/ipv6.h +2 -1

@@ -788,7 +786,8 @@ static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
             net->ipv6.sysctl.auto_flowlabels != IP6_AUTO_FLOW_LABEL_FORCED))
                return flowlabel;
 
-       hash = skb_get_hash_flowi6(skb, fl6);
+       if (skb)
+               hash = skb_get_hash_flowi6(skb, fl6);

Why not just move skb_get_hash_flowi6() to its caller? This check is not necessary. If you don't want to touch existing callers, you can just introduce a wrapper:

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel, struct flowi6 *fl6)
{
u32 hash = skb_get_hash_flowi6(skb, fl6);
return __ip6_make_flowlabel(net, flowlabel, autolabel, hash);
}

And your code can just call:

__ip6_make_flowlabel(net, flowlabel, autolabel, sk->sk_txhash);

Quoted 25 lines · Cong Wang

Quoted 11 lines · Shaohua Li

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel,

(quoted diff) +1 -3

-                                       struct flowi6 *fl6)
+                                       struct flowi6 *fl6, u32 hash)
 {
-       u32 hash;
-
        /* @flowlabel may include more than a flow label, eg, the traffic class.
         * Here we want only the flow label value.
         */

Review static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb, · line 790

Quoted 6 lines

(quoted diff) +2 -1

@@ -788,7 +786,8 @@ static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
             net->ipv6.sysctl.auto_flowlabels != IP6_AUTO_FLOW_LABEL_FORCED))
                return flowlabel;
 
-       hash = skb_get_hash_flowi6(skb, fl6);
+       if (skb)
+               hash = skb_get_hash_flowi6(skb, fl6);

Why not just move skb_get_hash_flowi6() to its caller? This check is not necessary. If you don't want to touch existing callers, you can just introduce a wrapper:

Quoted 7 lines

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel, struct flowi6 *fl6)
{
u32 hash = skb_get_hash_flowi6(skb, fl6);
return __ip6_make_flowlabel(net, flowlabel, autolabel, hash);
}

this will always call skb_get_hash_flowi6 for the fast path even auto flowlabel is disabled. I thought we should avoid this.

Quoted 34 lines · Shaohua Li

Quoted 32 lines · Cong Wang

Quoted 11 lines · Shaohua Li

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel,

(quoted diff) +1 -3

-                                       struct flowi6 *fl6)
+                                       struct flowi6 *fl6, u32 hash)
 {
-       u32 hash;
-
        /* @flowlabel may include more than a flow label, eg, the traffic class.
         * Here we want only the flow label value.
         */

Review static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb, · line 790

Quoted 6 lines

(quoted diff) +2 -1

@@ -788,7 +786,8 @@ static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
             net->ipv6.sysctl.auto_flowlabels != IP6_AUTO_FLOW_LABEL_FORCED))
                return flowlabel;
 
-       hash = skb_get_hash_flowi6(skb, fl6);
+       if (skb)
+               hash = skb_get_hash_flowi6(skb, fl6);

Why not just move skb_get_hash_flowi6() to its caller? This check is not necessary. If you don't want to touch existing callers, you can just introduce a wrapper:

static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
__be32 flowlabel, bool autolabel, struct flowi6 *fl6)
{
u32 hash = skb_get_hash_flowi6(skb, fl6);
return __ip6_make_flowlabel(net, flowlabel, autolabel, hash);
}

Quoted 2 lines

this will always call skb_get_hash_flowi6 for the fast path even auto flowlabel is disabled. I thought we should avoid this.

Yeah, but you can move the check out too,
something like:

include/net/ipv6.h +7 -16

index 6eac5cf8f1e6..18ffa824c00a 100644
@@ -771,31 +771,22 @@ static inline void
 iph_to_flow_copy_v6addrs(struct flow_keys *flow,
 
 #define IP6_DEFAULT_AUTO_FLOW_LABELS   IP6_AUTO_FLOW_LABEL_OPTOUT
 
-static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
-                                       __be32 flowlabel, bool autolabel,
-                                       struct flowi6 *fl6)
+static inline bool ip6_need_flowlabel(struct net *net, __be32
 flowlabel, bool autolabel)
 {
-       u32 hash;
-
        /* @flowlabel may include more than a flow label, eg, the traffic class.
         * Here we want only the flow label value.
         */
-       flowlabel &= IPV6_FLOWLABEL_MASK;
-
-       if (flowlabel ||
+       if ((flowlabel & IPV6_FLOWLABEL_MASK) ||
            net->ipv6.sysctl.auto_flowlabels == IP6_AUTO_FLOW_LABEL_OFF ||
            (!autolabel &&
             net->ipv6.sysctl.auto_flowlabels != IP6_AUTO_FLOW_LABEL_FORCED))
-               return flowlabel;
-
-       hash = skb_get_hash_flowi6(skb, fl6);
+               return false;
 
-       /* Since this is being sent on the wire obfuscate hash a bit
-        * to minimize possbility that any useful information to an
-        * attacker is leaked. Only lower 20 bits are relevant.
-        */
-       rol32(hash, 16);
+       return true;
+}
 
+static inline __be32 __ip6_make_flowlabel(struct net *net, __be32
 flowlabel, u32 hash)
+{

        flowlabel = (__force __be32)hash & IPV6_FLOWLABEL_MASK;

        if (net->ipv6.sysctl.flowlabel_state_ranges)

include/net/ipv6.h +13 -0

@@ -804,6 +795,19 @@ static inline __be32 ip6_make_flowlabel(struct
 net *net, struct sk_buff *skb,
        return flowlabel;
 }
 
+static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
+                                       __be32 flowlabel, bool autolabel,
+                                       struct flowi6 *fl6)
+{
+       u32 hash;
+
+       if (!ip6_need_flowlabel(net, flowlabel, autolabel))
+               return flowlabel & IPV6_FLOWLABEL_MASK;
+
+       hash = skb_get_hash_flowi6(skb, fl6);
+       return __ip6_make_flowlabel(net, flowlabel, hash);
+}
+
 static inline int ip6_default_np_autolabel(struct net *net)
 {

        switch (net->ipv6.sysctl.auto_flowlabels) {

Is this really better? I don't see any point. I'd use my original patch other than this one. that said, there are just several lines of code, brutally 'abstract' them into a function doesn't make the code better.

From: Shaohua Li <[email protected]>

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 flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Thanks,
Shaohua

Shaohua Li (2):
net: remove unnecessary rotation
net: fix tcp reset packet flowlabel for ipv6

 include/net/ipv6.h       | 15 ++++-----------
 net/ipv4/tcp_minisocks.c |  8 +++++++-
 net/ipv6/ip6_gre.c       |  2 +-
 net/ipv6/ip6_output.c    |  4 ++--
 net/ipv6/ip6_tunnel.c    |  2 +-
 net/ipv6/tcp_ipv6.c      | 18 +++++++++++++++++-
 6 files changed, 32 insertions(+), 17 deletions(-)

Signature · 1 line

2.9.3

Could you please look at the patches?

From: Shaohua Li <[email protected]>

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 flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Thanks,
Shaohua

Shaohua Li (2):
net: remove unnecessary rotation
net: fix tcp reset packet flowlabel for ipv6

 include/net/ipv6.h       | 15 ++++-----------
 net/ipv4/tcp_minisocks.c |  8 +++++++-
 net/ipv6/ip6_gre.c       |  2 +-
 net/ipv6/ip6_output.c    |  4 ++--
 net/ipv6/ip6_tunnel.c    |  2 +-
 net/ipv6/tcp_ipv6.c      | 18 +++++++++++++++++-
 6 files changed, 32 insertions(+), 17 deletions(-)

--
2.9.3

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Thanks,
Shaohua

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

Thanks,
Shaohua

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Quoted 78 lines · Tom Herbert

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Quoted 2 lines

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Hmm, I still hesitate to add a new field in skc_common. Fixing current problem looks propriate in current stage. I'd defer fixing the generic issue till it's necessary.

Thanks,
Shaohua

Quoted 79 lines · Shaohua Li

Quoted 78 lines · Tom Herbert

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Quoted 7 lines

Quoted 2 lines

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Hmm, I still hesitate to add a new field in skc_common. Fixing current problem looks propriate in current stage. I'd defer fixing the generic issue till it's necessary.

Shaohua,

An alternative would be to not initialize sk_txhash, but instead defer hash computation to use flow dissector in the TX path when the hash is needed (to get flow label, src port for UDP encap, route for multipath, etc.). At the first hash computation in TX path the result in sk_txhash. In TW state there is no socket so flow dissector is always used but that should yield the same hash. No extra fields would be needed and additional cost is negligible.

Quoted 89 lines · Tom Herbert

Quoted 79 lines · Shaohua Li

Quoted 78 lines · Tom Herbert

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Quoted 7 lines

Quoted 2 lines

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Hmm, I still hesitate to add a new field in skc_common. Fixing current problem looks propriate in current stage. I'd defer fixing the generic issue till it's necessary.

Shaohua,

Quoted 7 lines

An alternative would be to not initialize sk_txhash, but instead defer hash computation to use flow dissector in the TX path when the hash is needed (to get flow label, src port for UDP encap, route for multipath, etc.). At the first hash computation in TX path the result in sk_txhash. TW state there is no socket so flow dissector is always used but that should yield the same hash. No extra fields would be needed and additional cost is negligible.

Hi Tom,

Did you mean revert 877d1f6291f8(net: Set sk_txhash from a random number)? This could fix the issue. So in normal case we calculate the sk_txhash using flow dissector but in negative routing case we use the random hash, is this what you want?

There seems to have other bug in this side. From my understanding, commit 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to select a different route. But the multipath selection code (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit doesn't change anything.

What's the 'src port for UDP encap'? I can't find the code setting skb->hash to sk_txhash in UDP side.

Thanks,
Shaohua

Quoted 98 lines · Shaohua Li

Quoted 89 lines · Tom Herbert

Quoted 79 lines · Shaohua Li

Quoted 78 lines · Tom Herbert

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Quoted 7 lines

Quoted 2 lines

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Hmm, I still hesitate to add a new field in skc_common. Fixing current problem looks propriate in current stage. I'd defer fixing the generic issue till it's necessary.

Shaohua,

Quoted 7 lines

An alternative would be to not initialize sk_txhash, but instead defer hash computation to use flow dissector in the TX path when the hash is needed (to get flow label, src port for UDP encap, route for multipath, etc.). At the first hash computation in TX path the result in sk_txhash. TW state there is no socket so flow dissector is always used but that should yield the same hash. No extra fields would be needed and additional cost is negligible.

Quoted 7 lines

Hi Tom,

Did you mean revert 877d1f6291f8(net: Set sk_txhash from a random number)? This could fix the issue. So in normal case we calculate the sk_txhash using flow dissector but in negative routing case we use the random hash, is this what you want?

No. What you'd want is something like a sysctl that sets an alternate mode for sk_txhash processing. sk_txhash is derived from flow dissector for the first TX packet and then it's never allowed to change. Maybe this should be called persistent-hash mode.

Quoted 7 lines

There seems to have other bug in this side. From my understanding, commit 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to select a different route. But the multipath selection code (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit doesn't change anything.

The routing functions typically don't use sock of skbuff, but use flow structs instead. It may be reasonable to add a hash to those.

Quoted 106 lines · Tom Herbert

Quoted 98 lines · Shaohua Li

Quoted 89 lines · Tom Herbert

Quoted 79 lines · Shaohua Li

Quoted 78 lines · Tom Herbert

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Quoted 7 lines

Quoted 2 lines

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Hmm, I still hesitate to add a new field in skc_common. Fixing current problem looks propriate in current stage. I'd defer fixing the generic issue till it's necessary.

Shaohua,

Quoted 7 lines

An alternative would be to not initialize sk_txhash, but instead defer hash computation to use flow dissector in the TX path when the hash is needed (to get flow label, src port for UDP encap, route for multipath, etc.). At the first hash computation in TX path the result in sk_txhash. TW state there is no socket so flow dissector is always used but that should yield the same hash. No extra fields would be needed and additional cost is negligible.

Quoted 7 lines

Hi Tom,

Did you mean revert 877d1f6291f8(net: Set sk_txhash from a random number)? This could fix the issue. So in normal case we calculate the sk_txhash using flow dissector but in negative routing case we use the random hash, is this what you want?

Quoted 4 lines

No. What you'd want is something like a sysctl that sets an alternate mode for sk_txhash processing. sk_txhash is derived from flow dissector for the first TX packet and then it's never allowed to change. Maybe this should be called persistent-hash mode.

persistent-hash will almostly mean disabling auto flowlabel. We'd prefer disabling auto flowlabel rather than the persistent-hash. But I think we do want to use the auto flowlabel for load balancing, so the persistent-hash isn't a go for us.

Let's take the other way. Your main concern is the multipath selection. I think this can be fixed. We can still use the tw->tw_flowlabel to store the auto created flowlabel like what my patch does, but we use an extra bit in tw_pad of inet_timewait_sock to indicate this flowlabel is auto created. In the reset packet send path, we don't use the auto created flowlabel for route slection, but just for packet data. This will keep current multipath selection behavior and fix the reset packet flowlabel issue.

Quoted 7 lines

Quoted 7 lines

There seems to have other bug in this side. From my understanding, commit 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to select a different route. But the multipath selection code (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit doesn't change anything.

Quoted 2 lines

The routing functions typically don't use sock of skbuff, but use flow structs instead. It may be reasonable to add a hash to those.

I'll look at this issue later. It's not related to current flowlabel problem.

Quoted 98 lines · Shaohua Li

Quoted 89 lines · Tom Herbert

Quoted 79 lines · Shaohua Li

Quoted 78 lines · Tom Herbert

Quoted 58 lines · Shaohua Li

Quoted 47 lines · Tom Herbert

Quoted 40 lines · Shaohua Li

Quoted 22 lines · Tom Herbert

Quoted 15 lines · Shaohua Li

From: Shaohua Li <[email protected]>

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

Quoted 4 lines

The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet (0xd827f) are different. This causes our router doesn't correctly close tcp connection. The patches try to fix the issue.

Shaohua,

Quoted 9 lines

Can you give some more detail about what the router doesn't close the TCP connection means? I'm guessing the problem is either: 1) the router is maintaining connection state that includes the flow label in a connection tuple. 2) some router in the path is maintaining connection state, but when the flow label changes the flow's packet are routed through a different router that doesn't have a state for the flow it drops the packet. #1 should be easily fix in the router, flow labels cannot be used as state. #2 is the known problem that stateful firewalls have killed our ability to use multihoming.

The #2 is exactly the problem we saw.

Quoted 4 lines

Another consideration is that sk_txhash is also used in routing decisions by the local host (flow label is normally derived from txhash). If you want to ensure that connections are routed consistently for timewait state you might need sk_txhash saved also.

Quoted 6 lines

As far as I understood, we don't use sk_txhash for routing selection. The code does routing selection with flowlabel user configured, at that time we don't derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always does routing selection first and then uses ip6_make_flowlabel to build packet data where we derive flowlabel from skb->hash.

Quoted 9 lines

That is assuming one particular use case. Generally, if you want to ensure all packets for a flow take the same path you'll need tx_hash and make it persistent (disable flow bender). For instance, if you were doing UDP encapsulation like in VXLAN the UDP source port selection is unaffected by saved flow label for the lifetime of the flow. So we would still hit #2 in that case and the stateful device doesn't see whole flow. It might be just as easy to move tx_hash in skc_common so that it's available in TW state for this purpose. Then when moving to TW state just copy the tx_hash.

Quoted 6 lines

Hi Tom,

My original implementation is to add a tx_hash in tw sock, we then copy sock's tx_hash to the tw tx_hash. This does makes things simplier. One concern from Eric is this will increase the size of tw sock. If we move tx_hash to skc_common, all sock size will increase, is this acceptable?

I think that can only be measured by how critical it is to persistently route all packets the same exact way for every connection. Page one of the IP book clearly states that IP packets can be dropped, duplicated, or received out of order. Received OOO implies that packet for the same flow are allowed to take different paths. The requirement that packets for the same flow must always take the same path through the network was created by stateful middleboxes-- it's not inherent in the architecture of IP networking. Unfortunately, we're seeing this become more and more of a problem as more devices are multi-homed (like smart phones) and these network requirement cripple our ability to take advantage of features like that.

Quoted 7 lines

Quoted 2 lines

Personally, I wish the middleboxes fix the problem they created, but I suppose we need to be pragmatic at least in the short term.

Hmm, I still hesitate to add a new field in skc_common. Fixing current problem looks propriate in current stage. I'd defer fixing the generic issue till it's necessary.

Shaohua,

Quoted 7 lines

An alternative would be to not initialize sk_txhash, but instead defer hash computation to use flow dissector in the TX path when the hash is needed (to get flow label, src port for UDP encap, route for multipath, etc.). At the first hash computation in TX path the result in sk_txhash. TW state there is no socket so flow dissector is always used but that should yield the same hash. No extra fields would be needed and additional cost is negligible.

Quoted 7 lines

Hi Tom,

Did you mean revert 877d1f6291f8(net: Set sk_txhash from a random number)? This could fix the issue. So in normal case we calculate the sk_txhash using flow dissector but in negative routing case we use the random hash, is this what you want?

No. What you'd want is something like a sysctl that sets an alternate mode for sk_txhash processing. sk_txhash is derived from flow dissector for the first TX packet and then it's never allowed to change. Maybe this should be called persistent-hash mode.

Quoted 8 lines

There seems to have other bug in this side. From my understanding, commit 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to select a different route. But the multipath selection code (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit doesn't change anything.

The routing functions typically don't use sock of skbuff, but use flow structs instead. It may be reasonable to add a hash to those.

Quoted 4 lines

What's the 'src port for UDP encap'? I can't find the code setting skb->hash to sk_txhash in UDP side.

udp_flow_src_port is function call by UDP encaps to set source port. This is call skb_get_hash. sk_set_txhash is function to set txhash right now to random value. skb_set_hash_from_sk set skb->hash when skbuff is owned by socket (skb_set_owner_w).

Thanks,
Tom

Quoted 2 lines

Thanks,
Shaohua

We had been using the auto_flowlabels=1 (i.e. essentially enable flowlabel) mainly because we want to take the benefit of dst_negative_advice() when tcp_write_timeout() happens.

During our test, our system handles quite well with changing flowlabel. The only exception we have hit is the TCP_RST sent from an inet_timewait_sock.

If we keep the flowlabel consistent (or persistent sk_txhash), there is no practical usage for us to turn on flowlabel and the problem also goes away. We have it off for now.

Quoted 10 lines

Quoted 8 lines

There seems to have other bug in this side. From my understanding, commit 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to select a different route. But the multipath selection code (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit doesn't change anything.

The routing functions typically don't use sock of skbuff, but use flow structs instead. It may be reasonable to add a hash to those.

The localhost's mutlipath selection is another existing issue. AFAICT, it does not take the sk_txhash (or skb->hash) into account and the following dst_negative_advice() will also have no effect in the route selction. It is another issue to be fixed and to be figured out how to pass the sk_txhash down. (1)

Shaohua is proposing to record the 20 bits of the sk_txhash in the tw_flowlabel of the 'struct inet_timewait_sock'. The tw_flowlabel could potentially be used to do the multipath selection once we figured out how to tackle (1).

Thanks,
Martin

Quoted 15 lines

Quoted 4 lines

What's the 'src port for UDP encap'? I can't find the code setting skb->hash to sk_txhash in UDP side.

udp_flow_src_port is function call by UDP encaps to set source port. This is call skb_get_hash. sk_set_txhash is function to set txhash right now to random value. skb_set_hash_from_sk set skb->hash when skbuff is owned by socket (skb_set_owner_w).

Thanks,
Tom

Quoted 2 lines

Thanks,
Shaohua

Quoted 7 lines

We had been using the auto_flowlabels=1 (i.e. essentially enable flowlabel) mainly because we want to take the benefit of dst_negative_advice() when tcp_write_timeout() happens.

During our test, our system handles quite well with changing flowlabel. The only exception we have hit is the TCP_RST sent from an inet_timewait_sock.

Martin,

That is interesting data. Have you determined why the middlebox has a problem with flow label change in TW state but not other states?

Tom

Quoted 44 lines

If we keep the flowlabel consistent (or persistent sk_txhash), there is no practical usage for us to turn on flowlabel and the problem also goes away. We have it off for now.

Quoted 10 lines

Quoted 8 lines

There seems to have other bug in this side. From my understanding, commit 265f94ff54d6(net: Recompute sk_txhash on negative routing advice) tries to select a different route. But the multipath selection code (rt6_multipath_select) doesn't use sk_txhash or skb->hash, it does use fl6.flowlabel, but that is the flowlabel user sets. So looks like the commit doesn't change anything.

The routing functions typically don't use sock of skbuff, but use flow structs instead. It may be reasonable to add a hash to those.

The localhost's mutlipath selection is another existing issue. AFAICT, it does not take the sk_txhash (or skb->hash) into account and the following dst_negative_advice() will also have no effect in the route selction. It is another issue to be fixed and to be figured out how to pass the sk_txhash down. (1)

Shaohua is proposing to record the 20 bits of the sk_txhash in the tw_flowlabel of the 'struct inet_timewait_sock'. The tw_flowlabel could potentially be used to do the multipath selection once we figured out how to tackle (1).

Thanks,
Martin

Quoted 15 lines

Quoted 4 lines

What's the 'src port for UDP encap'? I can't find the code setting skb->hash to sk_txhash in UDP side.

udp_flow_src_port is function call by UDP encaps to set source port. This is call skb_get_hash. sk_set_txhash is function to set txhash right now to random value. skb_set_hash_from_sk set skb->hash when skbuff is owned by socket (skb_set_owner_w).

Thanks,
Tom

Quoted 2 lines

Thanks,
Shaohua

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

From: Martin KaFai Lau <[email protected]>
Date: Fri, 18 Aug 2017 13:51:36 -0700

Quoted 4 lines

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

This really is illegal behavior. The flow label is not a flow _KEY_ by any definition whatsoever.

Flow labels are an optimization, not a determinant for flow matching particularly for proper TCP state processing.

I'd rather you invest all of this energy getting that vendor to fix their kit.

Quoted 8 lines · David Miller

From: Martin KaFai Lau <[email protected]>
Date: Fri, 18 Aug 2017 13:51:36 -0700

Quoted 4 lines

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

Quoted 9 lines

This really is illegal behavior. The flow label is not a flow _KEY_ by any definition whatsoever.

Flow labels are an optimization, not a determinant for flow matching particularly for proper TCP state processing.

I'd rather you invest all of this energy getting that vendor to fix their kit.

We're now seeing several router vendors recommending people to not use flow labels for ECMP hashing. This is precisely because when a flow label changes, network devices that maintain state (firewalls, NAT, load balancers) can't deal with packets being rerouted so connections are dropped. Unfortunately, the need for packets of a flow to always follow the same path has become an implicit requirement that I think we need follow at least as the default behavior.

Martin: is there any change you could resurrect these patches? In order to solve the general problem of making routing consistent, I believe we want to keep sk_tx_hash consistent for the connection from which a consistent flow label can be derived. To avoid the overhead of a hash field in sk_common, maybe we could initially set a connection hash to a five-tuple hash for a flow instead of a random value? So in TW state the consistent hash can be computed on the fly.

Tom

Quoted 9 lines · Tom Herbert

Quoted 1 line

We're now seeing several router vendors recommending people to not use flow labels for ECMP hashing. This is precisely because when a flow label changes, network devices that maintain state (firewalls, NAT, load balancers) can't deal with packets being rerouted so connections are dropped. Unfortunately, the need for packets of a flow to always follow the same path has become an implicit requirement that I think we need follow at least as the default behavior.

Quoted 8 lines

Martin: is there any change you could resurrect these patches? In order to solve the general problem of making routing consistent, I believe we want to keep sk_tx_hash consistent for the connection from which a consistent flow label can be derived. To avoid the overhead of a hash field in sk_common, maybe we could initially set a connection hash to a five-tuple hash for a flow instead of a random value? So in TW state the consistent hash can be computed on the fly.

Sorry, I failed to give credit to Shaohua for submitting the initial patch. Please take look!

Quoted 8 lines · David Miller

From: Martin KaFai Lau <[email protected]>
Date: Fri, 18 Aug 2017 13:51:36 -0700

Quoted 4 lines

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

Quoted 9 lines

This really is illegal behavior. The flow label is not a flow _KEY_ by any definition whatsoever.

Flow labels are an optimization, not a determinant for flow matching particularly for proper TCP state processing.

I'd rather you invest all of this energy getting that vendor to fix their kit.

We're now seeing several router vendors recommending people to not use flow labels for ECMP hashing. This is precisely because when a flow label changes, network devices that maintain state (firewalls, NAT, load balancers) can't deal with packets being rerouted so connections are dropped. Unfortunately, the need for packets of a flow to always follow the same path has become an implicit requirement that I think we need follow at least as the default behavior.

Martin: is there any change you could resurrect these patches? In order to solve the general problem of making routing consistent, I believe we want to keep sk_tx_hash consistent for the connection from which a consistent flow label can be derived. To avoid the overhead of a hash field in sk_common, maybe we could initially set a connection hash to a five-tuple hash for a flow instead of a random value? So in TW state the consistent hash can be computed on the fly.

Quoted 26 lines · Tom Herbert

Quoted 8 lines · David Miller

From: Martin KaFai Lau <[email protected]>
Date: Fri, 18 Aug 2017 13:51:36 -0700

Quoted 4 lines

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

Quoted 9 lines

This really is illegal behavior. The flow label is not a flow _KEY_ by any definition whatsoever.

Flow labels are an optimization, not a determinant for flow matching particularly for proper TCP state processing.

I'd rather you invest all of this energy getting that vendor to fix their kit.

We're now seeing several router vendors recommending people to not use flow labels for ECMP hashing. This is precisely because when a flow label changes, network devices that maintain state (firewalls, NAT, load balancers) can't deal with packets being rerouted so connections are dropped. Unfortunately, the need for packets of a flow to always follow the same path has become an implicit requirement that I think we need follow at least as the default behavior.

Quoted 7 lines

Martin: is there any change you could resurrect these patches? In order to solve the general problem of making routing consistent, I believe we want to keep sk_tx_hash consistent for the connection from which a consistent flow label can be derived. To avoid the overhead of a hash field in sk_common, maybe we could initially set a connection hash to a five-tuple hash for a flow instead of a random value? So in TW state the consistent hash can be computed on the fly.

Hi Tom,
Do we really need to use the five-tupe hash? There are several places using current random hash, which looks more lightweight. To fix issue, we only need to make sure reset packet include the correct flowlabel. Like what my previous patch did, we can set tw->tw_flowlabel in tcp_time_wait based on txhash and use it reset packet. In this way we can use the random hash and not add extra field in sock.

Thanks,
Shaohua

Quoted 35 lines · Shaohua Li

Quoted 26 lines · Tom Herbert

Quoted 8 lines · David Miller

From: Martin KaFai Lau <[email protected]>
Date: Fri, 18 Aug 2017 13:51:36 -0700

Quoted 4 lines

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

Quoted 9 lines

This really is illegal behavior. The flow label is not a flow _KEY_ by any definition whatsoever.

Flow labels are an optimization, not a determinant for flow matching particularly for proper TCP state processing.

I'd rather you invest all of this energy getting that vendor to fix their kit.

We're now seeing several router vendors recommending people to not use flow labels for ECMP hashing. This is precisely because when a flow label changes, network devices that maintain state (firewalls, NAT, load balancers) can't deal with packets being rerouted so connections are dropped. Unfortunately, the need for packets of a flow to always follow the same path has become an implicit requirement that I think we need follow at least as the default behavior.

Quoted 7 lines

Martin: is there any change you could resurrect these patches? In order to solve the general problem of making routing consistent, I believe we want to keep sk_tx_hash consistent for the connection from which a consistent flow label can be derived. To avoid the overhead of a hash field in sk_common, maybe we could initially set a connection hash to a five-tuple hash for a flow instead of a random value? So in TW state the consistent hash can be computed on the fly.

Quoted 8 lines

Hi Tom,
Do we really need to use the five-tupe hash? There are several places using current random hash, which looks more lightweight. To fix issue, we only need to make sure reset packet include the correct flowlabel. Like what my previous patch did, we can set tw->tw_flowlabel in tcp_time_wait based on txhash and use it reset packet. In this way we can use the random hash and not add extra field in sock.

Shaohua,

But that patch discards the full txhash in TW. So it's not just a problem with the flow label. sk_tx_hash can also be used for route selection in ECMP, port selection we're doing tunneling, etc. The general solution should maintains tx_hash or be able to reconstruct it in any state, flow label fix is a point solution.

Quoted 44 lines · Tom Herbert

Quoted 35 lines · Shaohua Li

Quoted 26 lines · Tom Herbert

Quoted 8 lines · David Miller

From: Martin KaFai Lau <[email protected]>
Date: Fri, 18 Aug 2017 13:51:36 -0700

Quoted 4 lines

It seems like that middle box specifically drops TCP_RST if it does not know anything about this flow. Since the flowlabel of the TCP_RST (sent in TW state) is always different, it always lands to a different middle box. All of these TCP_RST cannot be delivered.

Quoted 9 lines

This really is illegal behavior. The flow label is not a flow _KEY_ by any definition whatsoever.

Flow labels are an optimization, not a determinant for flow matching particularly for proper TCP state processing.

I'd rather you invest all of this energy getting that vendor to fix their kit.

We're now seeing several router vendors recommending people to not use flow labels for ECMP hashing. This is precisely because when a flow label changes, network devices that maintain state (firewalls, NAT, load balancers) can't deal with packets being rerouted so connections are dropped. Unfortunately, the need for packets of a flow to always follow the same path has become an implicit requirement that I think we need follow at least as the default behavior.

Quoted 7 lines

Martin: is there any change you could resurrect these patches? In order to solve the general problem of making routing consistent, I believe we want to keep sk_tx_hash consistent for the connection from which a consistent flow label can be derived. To avoid the overhead of a hash field in sk_common, maybe we could initially set a connection hash to a five-tuple hash for a flow instead of a random value? So in TW state the consistent hash can be computed on the fly.

Quoted 8 lines

Hi Tom,
Do we really need to use the five-tupe hash? There are several places using current random hash, which looks more lightweight. To fix issue, we only need to make sure reset packet include the correct flowlabel. Like what my previous patch did, we can set tw->tw_flowlabel in tcp_time_wait based on txhash and use it reset packet. In this way we can use the random hash and not add extra field in sock.

Quoted 7 lines

Shaohua,

But that patch discards the full txhash in TW. So it's not just a problem with the flow label. sk_tx_hash can also be used for route selection in ECMP, port selection we're doing tunneling, etc. The general solution should maintains tx_hash or be able to reconstruct it in any state, flow label fix is a point solution.

Hi Tom,

do you want to keep sk_rethink_txhash() then? If we changed the hash to random number, we can't reconstruct it for sure.

Thanks,
Shaohua