* [PATCH net] tcp: inherit timestamp on mtu probe
@ 2019-08-27 19:09 Willem de Bruijn
2019-08-27 20:07 ` Eric Dumazet
2019-08-28 22:57 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Willem de Bruijn @ 2019-08-27 19:09 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, jakub.kicinski, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
TCP associates tx timestamp requests with a byte in the bytestream.
If merging skbs in tcp_mtu_probe, migrate the tstamp request.
Similar to MSG_EOR, do not allow moving a timestamp from any segment
in the probe but the last. This to avoid merging multiple timestamps.
Tested with the packetdrill script at
https://github.com/wdebruij/packetdrill/commits/mtu_probe-1
Link: http://patchwork.ozlabs.org/patch/1143278/#2232897
Fixes: 4ed2d765dfac ("net-timestamp: TCP timestamping")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/tcp_output.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5c46bc4c7e8d..42abc9bd687a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2053,7 +2053,7 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len)
if (len <= skb->len)
break;
- if (unlikely(TCP_SKB_CB(skb)->eor))
+ if (unlikely(TCP_SKB_CB(skb)->eor) || tcp_has_tx_tstamp(skb))
return false;
len -= skb->len;
@@ -2170,6 +2170,7 @@ static int tcp_mtu_probe(struct sock *sk)
* we need to propagate it to the new skb.
*/
TCP_SKB_CB(nskb)->eor = TCP_SKB_CB(skb)->eor;
+ tcp_skb_collapse_tstamp(nskb, skb);
tcp_unlink_write_queue(skb, sk);
sk_wmem_free_skb(sk, skb);
} else {
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] tcp: inherit timestamp on mtu probe
2019-08-27 19:09 [PATCH net] tcp: inherit timestamp on mtu probe Willem de Bruijn
@ 2019-08-27 20:07 ` Eric Dumazet
2019-08-27 20:53 ` Willem de Bruijn
2019-08-28 22:57 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2019-08-27 20:07 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, David Miller, Jakub Kicinski, Willem de Bruijn
On Tue, Aug 27, 2019 at 9:09 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> TCP associates tx timestamp requests with a byte in the bytestream.
> If merging skbs in tcp_mtu_probe, migrate the tstamp request.
>
> Similar to MSG_EOR, do not allow moving a timestamp from any segment
> in the probe but the last. This to avoid merging multiple timestamps.
>
> Tested with the packetdrill script at
> https://github.com/wdebruij/packetdrill/commits/mtu_probe-1
>
> Link: http://patchwork.ozlabs.org/patch/1143278/#2232897
> Fixes: 4ed2d765dfac ("net-timestamp: TCP timestamping")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> net/ipv4/tcp_output.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 5c46bc4c7e8d..42abc9bd687a 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2053,7 +2053,7 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len)
> if (len <= skb->len)
> break;
>
> - if (unlikely(TCP_SKB_CB(skb)->eor))
> + if (unlikely(TCP_SKB_CB(skb)->eor) || tcp_has_tx_tstamp(skb))
> return false;
>
> len -= skb->len;
> @@ -2170,6 +2170,7 @@ static int tcp_mtu_probe(struct sock *sk)
> * we need to propagate it to the new skb.
> */
> TCP_SKB_CB(nskb)->eor = TCP_SKB_CB(skb)->eor;
> + tcp_skb_collapse_tstamp(nskb, skb);
nit: maybe rename tcp_skb_collapse_tstamp() to tcp_skb_tstamp_copy()
or something ?
Its name came from the fact that it was only used from
tcp_collapse_retrans(), but it will no
longer be the case after your fix.
> tcp_unlink_write_queue(skb, sk);
> sk_wmem_free_skb(sk, skb);
> } else {
> --
> 2.23.0.187.g17f5b7556c-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tcp: inherit timestamp on mtu probe
2019-08-27 20:07 ` Eric Dumazet
@ 2019-08-27 20:53 ` Willem de Bruijn
2019-08-27 20:58 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2019-08-27 20:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Willem de Bruijn, netdev, David Miller, Jakub Kicinski
On Tue, Aug 27, 2019 at 4:07 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Aug 27, 2019 at 9:09 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > From: Willem de Bruijn <willemb@google.com>
> >
> > TCP associates tx timestamp requests with a byte in the bytestream.
> > If merging skbs in tcp_mtu_probe, migrate the tstamp request.
> >
> > Similar to MSG_EOR, do not allow moving a timestamp from any segment
> > in the probe but the last. This to avoid merging multiple timestamps.
> >
> > Tested with the packetdrill script at
> > https://github.com/wdebruij/packetdrill/commits/mtu_probe-1
> >
> > Link: http://patchwork.ozlabs.org/patch/1143278/#2232897
> > Fixes: 4ed2d765dfac ("net-timestamp: TCP timestamping")
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
> > ---
> > net/ipv4/tcp_output.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index 5c46bc4c7e8d..42abc9bd687a 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -2053,7 +2053,7 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len)
> > if (len <= skb->len)
> > break;
> >
> > - if (unlikely(TCP_SKB_CB(skb)->eor))
> > + if (unlikely(TCP_SKB_CB(skb)->eor) || tcp_has_tx_tstamp(skb))
> > return false;
> >
> > len -= skb->len;
> > @@ -2170,6 +2170,7 @@ static int tcp_mtu_probe(struct sock *sk)
> > * we need to propagate it to the new skb.
> > */
> > TCP_SKB_CB(nskb)->eor = TCP_SKB_CB(skb)->eor;
> > + tcp_skb_collapse_tstamp(nskb, skb);
>
> nit: maybe rename tcp_skb_collapse_tstamp() to tcp_skb_tstamp_copy()
> or something ?
>
> Its name came from the fact that it was only used from
> tcp_collapse_retrans(), but it will no
> longer be the case after your fix.
Sure, that's more descriptive.
One caveat, the function is exposed in a header, so it's a
bit more churn. If you don't mind that, I'll send the v2.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tcp: inherit timestamp on mtu probe
2019-08-27 20:53 ` Willem de Bruijn
@ 2019-08-27 20:58 ` Eric Dumazet
2019-08-27 21:15 ` Willem de Bruijn
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2019-08-27 20:58 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, David Miller, Jakub Kicinski
On Tue, Aug 27, 2019 at 10:54 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> Sure, that's more descriptive.
>
> One caveat, the function is exposed in a header, so it's a
> bit more churn. If you don't mind that, I'll send the v2.
Oh right it is also used from tcp_shifted_skb() after Martin KaFai Lau fix ...
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tcp: inherit timestamp on mtu probe
2019-08-27 20:58 ` Eric Dumazet
@ 2019-08-27 21:15 ` Willem de Bruijn
2019-08-27 21:23 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2019-08-27 21:15 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Jakub Kicinski
On Tue, Aug 27, 2019 at 4:58 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Aug 27, 2019 at 10:54 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
>
> > Sure, that's more descriptive.
> >
> > One caveat, the function is exposed in a header, so it's a
> > bit more churn. If you don't mind that, I'll send the v2.
>
> Oh right it is also used from tcp_shifted_skb() after Martin KaFai Lau fix ...
Leave as is then?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tcp: inherit timestamp on mtu probe
2019-08-27 21:15 ` Willem de Bruijn
@ 2019-08-27 21:23 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2019-08-27 21:23 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, David Miller, Jakub Kicinski
On Tue, Aug 27, 2019 at 11:16 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Aug 27, 2019 at 4:58 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Tue, Aug 27, 2019 at 10:54 PM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> >
> > > Sure, that's more descriptive.
> > >
> > > One caveat, the function is exposed in a header, so it's a
> > > bit more churn. If you don't mind that, I'll send the v2.
> >
> > Oh right it is also used from tcp_shifted_skb() after Martin KaFai Lau fix ...
>
> Leave as is then?
Not a big deal really ;)
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tcp: inherit timestamp on mtu probe
2019-08-27 19:09 [PATCH net] tcp: inherit timestamp on mtu probe Willem de Bruijn
2019-08-27 20:07 ` Eric Dumazet
@ 2019-08-28 22:57 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2019-08-28 22:57 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, edumazet, jakub.kicinski, willemb
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Tue, 27 Aug 2019 15:09:33 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> TCP associates tx timestamp requests with a byte in the bytestream.
> If merging skbs in tcp_mtu_probe, migrate the tstamp request.
>
> Similar to MSG_EOR, do not allow moving a timestamp from any segment
> in the probe but the last. This to avoid merging multiple timestamps.
>
> Tested with the packetdrill script at
> https://github.com/wdebruij/packetdrill/commits/mtu_probe-1
>
> Link: http://patchwork.ozlabs.org/patch/1143278/#2232897
> Fixes: 4ed2d765dfac ("net-timestamp: TCP timestamping")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-28 22:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 19:09 [PATCH net] tcp: inherit timestamp on mtu probe Willem de Bruijn
2019-08-27 20:07 ` Eric Dumazet
2019-08-27 20:53 ` Willem de Bruijn
2019-08-27 20:58 ` Eric Dumazet
2019-08-27 21:15 ` Willem de Bruijn
2019-08-27 21:23 ` Eric Dumazet
2019-08-28 22:57 ` 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).