From: Shaohua Li <shli@kernel.org> Date: 2017-07-13 17:57:02
From: Shaohua Li <redacted>
Currently tcp_v6_send_reset ignores user defined flowlabel, so the reset
packet doesn't include the flowlabel info.
Signed-off-by: Shaohua Li <redacted>
---
net/ipv6/tcp_ipv6.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
From: Eric Dumazet <hidden> Date: 2017-07-17 08:51:54
On Thu, 2017-07-13 at 10:56 -0700, Shaohua Li wrote:
From: Shaohua Li <redacted>
Please see below tcpdump output:
The tcp reset packet has a different flowlabel, which causes our router
doesn't correctly close tcp connection.
This looks a bug in your router, because (IPv6 only) flowlabel is not
part of the tuple identifying a TCP flow.
The reason is the normal packet
gets the skb->hash from sk->sk_txhash, which is generated randomly.
ip6_make_flowlabel then uses the hash to create a flowlabel. The reset
packet doesn't get assigned a hash, so the flowlabel is calculated with
flowi6.
The solution is to save the hash value for timeout sock and use it for
reset packet.
I am a bit unsure why we need to add yet another field in TCP timewait
structure, since :
1) flowlabel can vary during a TCP flow lifetime.
2) flowlabel is different unde synflood (each syncookie gets a random
flowlabel), and if 3rd packet comes back from the client to finish 3WHS,
the flowlabel will again be different from the one that SYNACK used.
I am a bit unsure why we need to add yet another field in TCP timewait
structure, since :
1) flowlabel can vary during a TCP flow lifetime.
2) flowlabel is different unde synflood (each syncookie gets a random
flowlabel), and if 3rd packet comes back from the client to finish 3WHS,
the flowlabel will again be different from the one that SYNACK used.
Actually, there is already a field for flowlabel in TCP timewait
structure (see commit 1d13a96c74). At least for "stateful" flow labels
(set with setsockopt(), and not randomly generated by kernel), it should
work (if not, this is a regression).
The solution is perhaps to store the random generated value in the same
place than standard flowlabels, instead to store the hash.
From: Shaohua Li <shli@kernel.org> Date: 2017-07-17 21:53:25
On Mon, Jul 17, 2017 at 01:51:51AM -0700, Eric Dumazet wrote:
On Thu, 2017-07-13 at 10:56 -0700, Shaohua Li wrote:
quoted
From: Shaohua Li <redacted>
Please see below tcpdump output:
quoted
The tcp reset packet has a different flowlabel, which causes our router
doesn't correctly close tcp connection.
This looks a bug in your router, because (IPv6 only) flowlabel is not
part of the tuple identifying a TCP flow.
Actually it's for load balance between several routers.
quoted
The reason is the normal packet
gets the skb->hash from sk->sk_txhash, which is generated randomly.
ip6_make_flowlabel then uses the hash to create a flowlabel. The reset
packet doesn't get assigned a hash, so the flowlabel is calculated with
flowi6.
The solution is to save the hash value for timeout sock and use it for
reset packet.
I am a bit unsure why we need to add yet another field in TCP timewait
structure, since :
1) flowlabel can vary during a TCP flow lifetime.
2) flowlabel is different unde synflood (each syncookie gets a random
flowlabel), and if 3rd packet comes back from the client to finish 3WHS,
the flowlabel will again be different from the one that SYNACK used.
Is it acceptable we reuse tw_flowlabel as Florent Fourcot suggested? It makes
no sense to change flowlabel for no reason.
Thanks,
Shaohua
From: Eric Dumazet <hidden> Date: 2017-07-18 04:03:01
On Mon, 2017-07-17 at 14:53 -0700, Shaohua Li wrote:
On Mon, Jul 17, 2017 at 01:51:51AM -0700, Eric Dumazet wrote:
quoted
On Thu, 2017-07-13 at 10:56 -0700, Shaohua Li wrote:
quoted
From: Shaohua Li <redacted>
Please see below tcpdump output:
quoted
The tcp reset packet has a different flowlabel, which causes our router
doesn't correctly close tcp connection.
This looks a bug in your router, because (IPv6 only) flowlabel is not
part of the tuple identifying a TCP flow.
Actually it's for load balance between several routers.
What happens then when flowlabel changes as I described ?
See commit 3acf3ec3f4b0 ("tcp: Change txhash on every SYN and RTO
retransmit")
quoted
quoted
The reason is the normal packet
gets the skb->hash from sk->sk_txhash, which is generated randomly.
ip6_make_flowlabel then uses the hash to create a flowlabel. The reset
packet doesn't get assigned a hash, so the flowlabel is calculated with
flowi6.
The solution is to save the hash value for timeout sock and use it for
reset packet.
I am a bit unsure why we need to add yet another field in TCP timewait
structure, since :
1) flowlabel can vary during a TCP flow lifetime.
2) flowlabel is different unde synflood (each syncookie gets a random
flowlabel), and if 3rd packet comes back from the client to finish 3WHS,
the flowlabel will again be different from the one that SYNACK used.
Is it acceptable we reuse tw_flowlabel as Florent Fourcot suggested? It makes
no sense to change flowlabel for no reason.
Sure, if you can find a way to keep storage as small as possible.
Current size is dangerously approaching 256 bytes, so we might soon use
one additional cache line (64 bytes)
From: Shaohua Li <shli@kernel.org> Date: 2017-07-18 18:59:08
On Mon, Jul 17, 2017 at 09:02:57PM -0700, Eric Dumazet wrote:
On Mon, 2017-07-17 at 14:53 -0700, Shaohua Li wrote:
quoted
On Mon, Jul 17, 2017 at 01:51:51AM -0700, Eric Dumazet wrote:
quoted
On Thu, 2017-07-13 at 10:56 -0700, Shaohua Li wrote:
quoted
From: Shaohua Li <redacted>
Please see below tcpdump output:
quoted
The tcp reset packet has a different flowlabel, which causes our router
doesn't correctly close tcp connection.
This looks a bug in your router, because (IPv6 only) flowlabel is not
part of the tuple identifying a TCP flow.
Actually it's for load balance between several routers.
What happens then when flowlabel changes as I described ?
See commit 3acf3ec3f4b0 ("tcp: Change txhash on every SYN and RTO
retransmit")
Frankly I have no idea. People in the team do think this is a problem in some
corner cases. Didn't get any report yet though.
quoted
quoted
quoted
The reason is the normal packet
gets the skb->hash from sk->sk_txhash, which is generated randomly.
ip6_make_flowlabel then uses the hash to create a flowlabel. The reset
packet doesn't get assigned a hash, so the flowlabel is calculated with
flowi6.
The solution is to save the hash value for timeout sock and use it for
reset packet.
I am a bit unsure why we need to add yet another field in TCP timewait
structure, since :
1) flowlabel can vary during a TCP flow lifetime.
2) flowlabel is different unde synflood (each syncookie gets a random
flowlabel), and if 3rd packet comes back from the client to finish 3WHS,
the flowlabel will again be different from the one that SYNACK used.
Is it acceptable we reuse tw_flowlabel as Florent Fourcot suggested? It makes
no sense to change flowlabel for no reason.
Sure, if you can find a way to keep storage as small as possible.
Current size is dangerously approaching 256 bytes, so we might soon use
one additional cache line (64 bytes)