[PATCH net] tcp: refactor tcp_retransmit_timer()

[PATCH net] tcp: refactor tcp_retransmit_timer()

[Eric Dumazet]

It appears linux-4.14 stable needs a backport of commit
88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")

Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()

I will provide to stable teams the squashed patches.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/ipv4/tcp_timer.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index dd5a6317a8018a45ad609f832ced6df2937ad453..1097b438befe14ae3f375f3dbcc1f2d375a93879 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -434,6 +434,7 @@ void tcp_retransmit_timer(struct sock *sk)
 	struct net *net = sock_net(sk);
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct request_sock *req;
+	struct sk_buff *skb;
 
 	req = rcu_dereference_protected(tp->fastopen_rsk,
 					lockdep_sock_is_held(sk));
@@ -446,7 +447,12 @@ void tcp_retransmit_timer(struct sock *sk)
 		 */
 		return;
 	}
-	if (!tp->packets_out || WARN_ON_ONCE(tcp_rtx_queue_empty(sk)))
+
+	if (!tp->packets_out)
+		return;
+
+	skb = tcp_rtx_queue_head(sk);
+	if (WARN_ON_ONCE(!skb))
 		return;
 
 	tp->tlp_high_seq = 0;
@@ -480,7 +486,7 @@ void tcp_retransmit_timer(struct sock *sk)
 			goto out;
 		}
 		tcp_enter_loss(sk);
-		tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1);
+		tcp_retransmit_skb(sk, skb, 1);
 		__sk_dst_reset(sk);
 		goto out_reset_timer;
 	}
-- 
2.24.0.393.g34dc348eaf-goog

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Soheil Hassas Yeganeh]

On Tue, Dec 3, 2019 at 11:05 AM Eric Dumazet <edumazet@google.com> wrote:
>
> It appears linux-4.14 stable needs a backport of commit
> 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
>
> Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
>
> I will provide to stable teams the squashed patches.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Thanks, Eric!

> ---
>  net/ipv4/tcp_timer.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index dd5a6317a8018a45ad609f832ced6df2937ad453..1097b438befe14ae3f375f3dbcc1f2d375a93879 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -434,6 +434,7 @@ void tcp_retransmit_timer(struct sock *sk)
>         struct net *net = sock_net(sk);
>         struct inet_connection_sock *icsk = inet_csk(sk);
>         struct request_sock *req;
> +       struct sk_buff *skb;
>
>         req = rcu_dereference_protected(tp->fastopen_rsk,
>                                         lockdep_sock_is_held(sk));
> @@ -446,7 +447,12 @@ void tcp_retransmit_timer(struct sock *sk)
>                  */
>                 return;
>         }
> -       if (!tp->packets_out || WARN_ON_ONCE(tcp_rtx_queue_empty(sk)))
> +
> +       if (!tp->packets_out)
> +               return;
> +
> +       skb = tcp_rtx_queue_head(sk);
> +       if (WARN_ON_ONCE(!skb))
>                 return;
>
>         tp->tlp_high_seq = 0;
> @@ -480,7 +486,7 @@ void tcp_retransmit_timer(struct sock *sk)
>                         goto out;
>                 }
>                 tcp_enter_loss(sk);
> -               tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1);
> +               tcp_retransmit_skb(sk, skb, 1);
>                 __sk_dst_reset(sk);
>                 goto out_reset_timer;
>         }
> --
> 2.24.0.393.g34dc348eaf-goog
>

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Greg Kroah-Hartman]

On Tue, Dec 03, 2019 at 08:05:52AM -0800, Eric Dumazet wrote:
> It appears linux-4.14 stable needs a backport of commit
> 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> 
> Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()

So does that mean that 4.19.y should get 88f8598d0a30 ("tcp: exit if
nothing to retransmit on RTO timeout") as-is?

> I will provide to stable teams the squashed patches.

This is that squashed patch, right?

thanks,

greg k-h

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Eric Dumazet]

On Tue, Dec 3, 2019 at 11:13 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Dec 03, 2019 at 08:05:52AM -0800, Eric Dumazet wrote:
> > It appears linux-4.14 stable needs a backport of commit
> > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> >
> > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
>
> So does that mean that 4.19.y should get 88f8598d0a30 ("tcp: exit if
> nothing to retransmit on RTO timeout") as-is?
>

Yes indeed. All stable versions need it.

> > I will provide to stable teams the squashed patches.
>
> This is that squashed patch, right?

Yes, both patches squashed to avoid breaking future bisections.

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Greg Kroah-Hartman]

On Tue, Dec 03, 2019 at 11:15:31AM -0800, Eric Dumazet wrote:
> On Tue, Dec 3, 2019 at 11:13 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Dec 03, 2019 at 08:05:52AM -0800, Eric Dumazet wrote:
> > > It appears linux-4.14 stable needs a backport of commit
> > > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> > >
> > > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> > > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
> >
> > So does that mean that 4.19.y should get 88f8598d0a30 ("tcp: exit if
> > nothing to retransmit on RTO timeout") as-is?
> >
> 
> Yes indeed. All stable versions need it.

So 4.4.y and 4.9.y as well?

thanks,

greg k-h

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Eric Dumazet]

On Tue, Dec 3, 2019 at 11:22 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Dec 03, 2019 at 11:15:31AM -0800, Eric Dumazet wrote:
> > On Tue, Dec 3, 2019 at 11:13 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Dec 03, 2019 at 08:05:52AM -0800, Eric Dumazet wrote:
> > > > It appears linux-4.14 stable needs a backport of commit
> > > > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> > > >
> > > > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> > > > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
> > >
> > > So does that mean that 4.19.y should get 88f8598d0a30 ("tcp: exit if
> > > nothing to retransmit on RTO timeout") as-is?
> > >
> >
> > Yes indeed. All stable versions need it.
>
> So 4.4.y and 4.9.y as well?

They should be fine, since they do not have yet :

ba2ddb43f270e6492ccce4fc42fc32c611de8f68 tcp: Don't dequeue
SYN/FIN-segments from write-queue
f1dcc5ed4bea3f2d63b74ad86617ec12b1e5e9d4 tcp: Reset send_head when
removing skb from write-queue
8e6521f6404e704fac313ab2479923be1f741f73 tcp: remove empty skb from
write queue in error cases

I would leave them untouched, to limit further problems :/

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Greg Kroah-Hartman]

On Tue, Dec 03, 2019 at 11:30:29AM -0800, Eric Dumazet wrote:
> On Tue, Dec 3, 2019 at 11:22 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Dec 03, 2019 at 11:15:31AM -0800, Eric Dumazet wrote:
> > > On Tue, Dec 3, 2019 at 11:13 AM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, Dec 03, 2019 at 08:05:52AM -0800, Eric Dumazet wrote:
> > > > > It appears linux-4.14 stable needs a backport of commit
> > > > > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> > > > >
> > > > > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> > > > > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
> > > >
> > > > So does that mean that 4.19.y should get 88f8598d0a30 ("tcp: exit if
> > > > nothing to retransmit on RTO timeout") as-is?
> > > >
> > >
> > > Yes indeed. All stable versions need it.
> >
> > So 4.4.y and 4.9.y as well?
> 
> They should be fine, since they do not have yet :
> 
> ba2ddb43f270e6492ccce4fc42fc32c611de8f68 tcp: Don't dequeue
> SYN/FIN-segments from write-queue
> f1dcc5ed4bea3f2d63b74ad86617ec12b1e5e9d4 tcp: Reset send_head when
> removing skb from write-queue
> 8e6521f6404e704fac313ab2479923be1f741f73 tcp: remove empty skb from
> write queue in error cases
> 
> I would leave them untouched, to limit further problems :/

Great, thanks for letting me know.

greg k-h

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Tue,  3 Dec 2019 08:05:52 -0800

> It appears linux-4.14 stable needs a backport of commit
> 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> 
> Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
> 
> I will provide to stable teams the squashed patches.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Greg KH]

On Tue, Dec 03, 2019 at 11:53:11AM -0800, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Tue,  3 Dec 2019 08:05:52 -0800
> 
> > It appears linux-4.14 stable needs a backport of commit
> > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> > 
> > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
> > 
> > I will provide to stable teams the squashed patches.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Applied, thanks Eric.

Applied where, do you have a 4.14-stable queue too?  :)

I can just take this directly to my 4.14.y tree now if you don't object.

thanks,

greg k-h

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[Greg Kroah-Hartman]

On Tue, Dec 03, 2019 at 12:51:18PM -0800, Eric Dumazet wrote:
> On Tue, Dec 3, 2019, 12:24 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Tue, Dec 03, 2019 at 11:53:11AM -0800, David Miller wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > > Date: Tue,  3 Dec 2019 08:05:52 -0800
> > >
> > > > It appears linux-4.14 stable needs a backport of commit
> > > > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
> > > >
> > > > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
> > > > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
> > > >
> > > > I will provide to stable teams the squashed patches.
> > > >
> > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > >
> > > Applied, thanks Eric.
> >
> > Applied where, do you have a 4.14-stable queue too?  :)
> >
> > I can just take this directly to my 4.14.y tree now if you don't object.
> >
> 
> Sure, please do, I will double-check it ;)

Ah, nevermind, I see now that your patch does apply to Dave's tree.  I
thought this was a 4.14-only thing here.

sorry for the confusion, I'll just ignore this for now then.

greg k-h

Re: [PATCH net] tcp: refactor tcp_retransmit_timer()

[David Miller]

From: Greg KH <gregkh@linuxfoundation.org>
Date: Tue, 3 Dec 2019 21:23:58 +0100

> On Tue, Dec 03, 2019 at 11:53:11AM -0800, David Miller wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Tue,  3 Dec 2019 08:05:52 -0800
>> 
>> > It appears linux-4.14 stable needs a backport of commit
>> > 88f8598d0a30 ("tcp: exit if nothing to retransmit on RTO timeout")
>> > 
>> > Since tcp_rtx_queue_empty() is not in pre 4.15 kernels,
>> > let's refactor tcp_retransmit_timer() to only use tcp_rtx_queue_head()
>> > 
>> > I will provide to stable teams the squashed patches.
>> > 
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> 
>> Applied, thanks Eric.
> 
> Applied where, do you have a 4.14-stable queue too?  :)

Applied to my net GIT tree, I do not have a 4.14 -stable queue :-)

> I can just take this directly to my 4.14.y tree now if you don't object.

Please integrate all of the -stable parts, yes.

Thanks.