* [PATCH net] tcp: add proper TS val into RST packets
@ 2015-09-23 20:19 Eric Dumazet
2015-09-23 20:30 ` Neal Cardwell
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2015-09-23 20:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng
From: Eric Dumazet <edumazet@google.com>
RST packets sent on behalf of TCP connections with TS option (RFC 7323
TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100 ecr 0], length 0
B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss 1460,nop,nop,TS val 7264344 ecr 100], length 0
A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr 7264344], length 0
B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0 ecr 110], length 0
We need to call skb_mstamp_get() to get proper TS val,
derived from skb->skb_mstamp
Note that RFC 1323 was advocating to not send TS option in RST segment,
but RFC 7323 recommends the opposite :
Once TSopt has been successfully negotiated, that is both <SYN> and
<SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
segment for the duration of the connection, and SHOULD be sent in an
<RST> segment (see Section 5.2 for details)
Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f9a8a12b62ee..1100ffe4a722 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2897,6 +2897,7 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority)
skb_reserve(skb, MAX_TCP_HEADER);
tcp_init_nondata_skb(skb, tcp_acceptable_seq(sk),
TCPHDR_ACK | TCPHDR_RST);
+ skb_mstamp_get(&skb->skb_mstamp);
/* Send it off. */
if (tcp_transmit_skb(sk, skb, 0, priority))
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] tcp: add proper TS val into RST packets
2015-09-23 20:19 [PATCH net] tcp: add proper TS val into RST packets Eric Dumazet
@ 2015-09-23 20:30 ` Neal Cardwell
2015-09-23 20:36 ` Yuchung Cheng
0 siblings, 1 reply; 7+ messages in thread
From: Neal Cardwell @ 2015-09-23 20:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Yuchung Cheng
On Wed, Sep 23, 2015 at 4:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> RST packets sent on behalf of TCP connections with TS option (RFC 7323
> TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
>
> A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100 ecr 0], length 0
> B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss 1460,nop,nop,TS val 7264344 ecr 100], length 0
> A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr 7264344], length 0
>
> B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0 ecr 110], length 0
>
> We need to call skb_mstamp_get() to get proper TS val,
> derived from skb->skb_mstamp
>
> Note that RFC 1323 was advocating to not send TS option in RST segment,
> but RFC 7323 recommends the opposite :
>
> Once TSopt has been successfully negotiated, that is both <SYN> and
> <SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
> segment for the duration of the connection, and SHOULD be sent in an
> <RST> segment (see Section 5.2 for details)
>
> Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] tcp: add proper TS val into RST packets
2015-09-23 20:30 ` Neal Cardwell
@ 2015-09-23 20:36 ` Yuchung Cheng
2015-09-23 20:52 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Yuchung Cheng @ 2015-09-23 20:36 UTC (permalink / raw)
To: Neal Cardwell; +Cc: Eric Dumazet, David Miller, netdev
On Wed, Sep 23, 2015 at 1:30 PM, Neal Cardwell <ncardwell@google.com> wrote:
>
> On Wed, Sep 23, 2015 at 4:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > RST packets sent on behalf of TCP connections with TS option (RFC 7323
> > TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
> >
> > A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100 ecr 0], length 0
> > B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss 1460,nop,nop,TS val 7264344 ecr 100], length 0
> > A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr 7264344], length 0
> >
> > B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0 ecr 110], length 0
> >
> > We need to call skb_mstamp_get() to get proper TS val,
> > derived from skb->skb_mstamp
> >
> > Note that RFC 1323 was advocating to not send TS option in RST segment,
> > but RFC 7323 recommends the opposite :
> >
> > Once TSopt has been successfully negotiated, that is both <SYN> and
> > <SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
> > segment for the duration of the connection, and SHOULD be sent in an
> > <RST> segment (see Section 5.2 for details)
Nice comment. I think it's worth adding why we choose not to follow
RFC7323 sec 5.2 recommendation on setting TSval to 0?
>
> >
> > Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
>
> Thanks, Eric!
>
> neal
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] tcp: add proper TS val into RST packets
2015-09-23 20:36 ` Yuchung Cheng
@ 2015-09-23 20:52 ` Eric Dumazet
2015-09-23 21:00 ` [PATCH v2 " Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2015-09-23 20:52 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: Neal Cardwell, David Miller, netdev
On Wed, 2015-09-23 at 13:36 -0700, Yuchung Cheng wrote:
> On Wed, Sep 23, 2015 at 1:30 PM, Neal Cardwell <ncardwell@google.com> wrote:
> >
> > On Wed, Sep 23, 2015 at 4:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > RST packets sent on behalf of TCP connections with TS option (RFC 7323
> > > TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
> > >
> > > A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100 ecr 0], length 0
> > > B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss 1460,nop,nop,TS val 7264344 ecr 100], length 0
> > > A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr 7264344], length 0
> > >
> > > B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0 ecr 110], length 0
> > >
> > > We need to call skb_mstamp_get() to get proper TS val,
> > > derived from skb->skb_mstamp
> > >
> > > Note that RFC 1323 was advocating to not send TS option in RST segment,
> > > but RFC 7323 recommends the opposite :
> > >
> > > Once TSopt has been successfully negotiated, that is both <SYN> and
> > > <SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
> > > segment for the duration of the connection, and SHOULD be sent in an
> > > <RST> segment (see Section 5.2 for details)
> Nice comment. I think it's worth adding why we choose not to follow
> RFC7323 sec 5.2 recommendation on setting TSval to 0?
We revert to prior behavior. This is a bug fix, as it clearly was not
even considered in 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
In 5 years, if/when all TCP stack are RFC 7323 ready, we might consider
to decide to send TS val = 0
Problem with RFC is they suppose both senders and receivers are ready
and support new recommendations.
Linux always sent TSopt in its RST packets, and apparently it works
well.
It is premature to send TS val = 0 we believe.
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 net] tcp: add proper TS val into RST packets
2015-09-23 20:52 ` Eric Dumazet
@ 2015-09-23 21:00 ` Eric Dumazet
2015-09-23 21:13 ` Yuchung Cheng
2015-09-23 21:25 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Eric Dumazet @ 2015-09-23 21:00 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: Neal Cardwell, David Miller, netdev
From: Eric Dumazet <edumazet@google.com>
RST packets sent on behalf of TCP connections with TS option (RFC 7323
TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100
ecr 0], length 0
B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss
1460,nop,nop,TS val 7264344 ecr 100], length 0
A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr
7264344], length 0
B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0
ecr 110], length 0
We need to call skb_mstamp_get() to get proper TS val,
derived from skb->skb_mstamp
Note that RFC 1323 was advocating to not send TS option in RST segment,
but RFC 7323 recommends the opposite :
Once TSopt has been successfully negotiated, that is both <SYN> and
<SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
segment for the duration of the connection, and SHOULD be sent in an
<RST> segment (see Section 5.2 for details)
Note this RFC recommends to send TS val = 0, but we believe it is
premature : We do not know if all TCP stacks are properly
handling the receive side :
When an <RST> segment is
received, it MUST NOT be subjected to the PAWS check by verifying an
acceptable value in SEG.TSval, and information from the Timestamps
option MUST NOT be used to update connection state information.
SEG.TSecr MAY be used to provide stricter <RST> acceptance checks.
In 5 years, if/when all TCP stack are RFC 7323 ready, we might consider
to decide to send TS val = 0, if it buys something.
Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f9a8a12b62ee..1100ffe4a722 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2897,6 +2897,7 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority)
skb_reserve(skb, MAX_TCP_HEADER);
tcp_init_nondata_skb(skb, tcp_acceptable_seq(sk),
TCPHDR_ACK | TCPHDR_RST);
+ skb_mstamp_get(&skb->skb_mstamp);
/* Send it off. */
if (tcp_transmit_skb(sk, skb, 0, priority))
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 net] tcp: add proper TS val into RST packets
2015-09-23 21:00 ` [PATCH v2 " Eric Dumazet
@ 2015-09-23 21:13 ` Yuchung Cheng
2015-09-23 21:25 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: Yuchung Cheng @ 2015-09-23 21:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Neal Cardwell, David Miller, netdev
On Wed, Sep 23, 2015 at 2:00 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> RST packets sent on behalf of TCP connections with TS option (RFC 7323
> TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
>
> A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100
> ecr 0], length 0
> B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss
> 1460,nop,nop,TS val 7264344 ecr 100], length 0
> A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr
> 7264344], length 0
>
> B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0
> ecr 110], length 0
>
> We need to call skb_mstamp_get() to get proper TS val,
> derived from skb->skb_mstamp
>
> Note that RFC 1323 was advocating to not send TS option in RST segment,
> but RFC 7323 recommends the opposite :
>
> Once TSopt has been successfully negotiated, that is both <SYN> and
> <SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
> segment for the duration of the connection, and SHOULD be sent in an
> <RST> segment (see Section 5.2 for details)
>
> Note this RFC recommends to send TS val = 0, but we believe it is
> premature : We do not know if all TCP stacks are properly
> handling the receive side :
>
> When an <RST> segment is
> received, it MUST NOT be subjected to the PAWS check by verifying an
> acceptable value in SEG.TSval, and information from the Timestamps
> option MUST NOT be used to update connection state information.
> SEG.TSecr MAY be used to provide stricter <RST> acceptance checks.
>
>
> In 5 years, if/when all TCP stack are RFC 7323 ready, we might consider
> to decide to send TS val = 0, if it buys something.
>
> Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Nice fix (and comment)!
> ---
> net/ipv4/tcp_output.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f9a8a12b62ee..1100ffe4a722 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2897,6 +2897,7 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority)
> skb_reserve(skb, MAX_TCP_HEADER);
> tcp_init_nondata_skb(skb, tcp_acceptable_seq(sk),
> TCPHDR_ACK | TCPHDR_RST);
> + skb_mstamp_get(&skb->skb_mstamp);
> /* Send it off. */
> if (tcp_transmit_skb(sk, skb, 0, priority))
> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTFAILED);
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 net] tcp: add proper TS val into RST packets
2015-09-23 21:00 ` [PATCH v2 " Eric Dumazet
2015-09-23 21:13 ` Yuchung Cheng
@ 2015-09-23 21:25 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2015-09-23 21:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: ycheng, ncardwell, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 23 Sep 2015 14:00:21 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> RST packets sent on behalf of TCP connections with TS option (RFC 7323
> TCP timestamps) have incorrect TS val (set to 0), but correct TS ecr.
>
> A > B: Flags [S], seq 0, win 65535, options [mss 1000,nop,nop,TS val 100
> ecr 0], length 0
> B > A: Flags [S.], seq 2444755794, ack 1, win 28960, options [mss
> 1460,nop,nop,TS val 7264344 ecr 100], length 0
> A > B: Flags [.], ack 1, win 65535, options [nop,nop,TS val 110 ecr
> 7264344], length 0
>
> B > A: Flags [R.], seq 1, ack 1, win 28960, options [nop,nop,TS val 0
> ecr 110], length 0
>
> We need to call skb_mstamp_get() to get proper TS val,
> derived from skb->skb_mstamp
>
> Note that RFC 1323 was advocating to not send TS option in RST segment,
> but RFC 7323 recommends the opposite :
>
> Once TSopt has been successfully negotiated, that is both <SYN> and
> <SYN,ACK> contain TSopt, the TSopt MUST be sent in every non-<RST>
> segment for the duration of the connection, and SHOULD be sent in an
> <RST> segment (see Section 5.2 for details)
>
> Note this RFC recommends to send TS val = 0, but we believe it is
> premature : We do not know if all TCP stacks are properly
> handling the receive side :
>
> When an <RST> segment is
> received, it MUST NOT be subjected to the PAWS check by verifying an
> acceptable value in SEG.TSval, and information from the Timestamps
> option MUST NOT be used to update connection state information.
> SEG.TSecr MAY be used to provide stricter <RST> acceptance checks.
>
>
> In 5 years, if/when all TCP stack are RFC 7323 ready, we might consider
> to decide to send TS val = 0, if it buys something.
>
> Fixes: 7faee5c0d514 ("tcp: remove TCP_SKB_CB(skb)->when")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-09-23 21:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-23 20:19 [PATCH net] tcp: add proper TS val into RST packets Eric Dumazet
2015-09-23 20:30 ` Neal Cardwell
2015-09-23 20:36 ` Yuchung Cheng
2015-09-23 20:52 ` Eric Dumazet
2015-09-23 21:00 ` [PATCH v2 " Eric Dumazet
2015-09-23 21:13 ` Yuchung Cheng
2015-09-23 21:25 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).