On Tue, Aug 27, 2019 at 4:07 PM Eric Dumazet <eduma...@google.com> wrote:
>
> On Tue, Aug 27, 2019 at 9:09 PM Willem de Bruijn
> <willemdebruijn.ker...@gmail.com> wrote:
> >
> > From: Willem de Bruijn <will...@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 <will...@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.

Reply via email to