[PATCH] tcp: Reset tcp connections in SYN-SENT state

[PATCH] tcp: Reset tcp connections in SYN-SENT state

[Manoj Basapathi]

Userspace sends tcp connection (sock) destroy on network switch
i.e switching the default network of the device between multiple
networks(Cellular/Wifi/Ethernet).

Kernel though doesn't send reset for the connections in SYN-SENT state
and these connections continue to remain.
Even as per RFC 793, there is no hard rule to not send RST on ABORT in
this state.

Modify tcp_abort and tcp_disconnect behavior to send RST for connections
in syn-sent state to avoid lingering connections on network switch.

Signed-off-by: Manoj Basapathi <manojbm@codeaurora.org>
Signed-off-by: Sauvik Saha <ssaha@codeaurora.org>
---
 net/ipv4/tcp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e14fd0c50c10..627a472161fb 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2888,7 +2888,7 @@ static inline bool tcp_need_reset(int state)
 {
 	return (1 << state) &
 	       (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
-		TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
+		TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
 }
 
 static void tcp_rtx_queue_purge(struct sock *sk)
@@ -2954,8 +2954,7 @@ int tcp_disconnect(struct sock *sk, int flags)
 		 */
 		tcp_send_active_reset(sk, gfp_any());
 		sk->sk_err = ECONNRESET;
-	} else if (old_state == TCP_SYN_SENT)
-		sk->sk_err = ECONNRESET;
+	}
 
 	tcp_clear_xmit_timers(sk);
 	__skb_queue_purge(&sk->sk_receive_queue);
-- 
2.29.0

Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state

[Eric Dumazet]

On 4/5/21 7:02 PM, Manoj Basapathi wrote:
> Userspace sends tcp connection (sock) destroy on network switch
> i.e switching the default network of the device between multiple
> networks(Cellular/Wifi/Ethernet).
> 
> Kernel though doesn't send reset for the connections in SYN-SENT state
> and these connections continue to remain.
> Even as per RFC 793, there is no hard rule to not send RST on ABORT in
> this state.
> 
> Modify tcp_abort and tcp_disconnect behavior to send RST for connections
> in syn-sent state to avoid lingering connections on network switch.
> 
> Signed-off-by: Manoj Basapathi <manojbm@codeaurora.org>
> Signed-off-by: Sauvik Saha <ssaha@codeaurora.org>
> ---
>  net/ipv4/tcp.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index e14fd0c50c10..627a472161fb 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2888,7 +2888,7 @@ static inline bool tcp_need_reset(int state)
>  {
>  	return (1 << state) &
>  	       (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
> -		TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
> +		TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
>  }
>  
>  static void tcp_rtx_queue_purge(struct sock *sk)
> @@ -2954,8 +2954,7 @@ int tcp_disconnect(struct sock *sk, int flags)
>  		 */
>  		tcp_send_active_reset(sk, gfp_any());
>  		sk->sk_err = ECONNRESET;
> -	} else if (old_state == TCP_SYN_SENT)
> -		sk->sk_err = ECONNRESET;
> +	}
>  
>  	tcp_clear_xmit_timers(sk);
>  	__skb_queue_purge(&sk->sk_receive_queue);
> 

This is a completely buggy patch.

This has been sent to many people but _not_ to TCP maintainers ????

I will send a revert.

Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state

[Eric Dumazet]

On Thu, Feb 21, 2019 at 3:01 AM Devi Sandeep Endluri V V
<dendluri@codeaurora.org> wrote:
>
> On Wed, Feb 20, 2019 at 07:47:59AM -0800, Eric Dumazet wrote:
> > On Wed, Feb 20, 2019 at 6:28 AM Devi Sandeep Endluri V V
> > <dendluri@codeaurora.org> wrote:
> > >
> > > Userspace sends tcp connection (sock) destroy on network permission
> > > change. Kernel though doesn't send reset for the connections in
> > > SYN-SENT state and these connections continue to remain. Even as
> > > per RFC 793, there is no hard rule to not send RST on ABORT in
> > > this state. Change to make sure RST are send for connections in
> > > syn-sent state to avoid lingering connections on network switch.
> > >
> > > References from RFC 793
> > >
> > > ABORT Call
> > >
> > >         SYN-SENT STATE
> > >
> > >         All queued SENDs and RECEIVEs should be given "connection reset"
> > >         notification, delete the TCB, enter CLOSED state, and return.
> > >
> > > SEGMENT ARRIVES
> > >
> > >         If the state is SYN-SENT then
> > >         If the RST bit is set
> > >
> > >           If the ACK was acceptable then signal the user "error:
> > >           connection reset", drop the segment, enter CLOSED state,
> > >           delete TCB, and return.  Otherwise (no ACK) drop the segment
> > >           and return.
> > >
> > > Signed-off-by: Devi Sandeep Endluri V V <dendluri@codeaurora.org>
> > > ---
> > >  net/ipv4/tcp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git net/ipv4/tcp.c net/ipv4/tcp.c
> > > index cf3c509..8569dc5e 100644
> > > --- net/ipv4/tcp.c
> > > +++ net/ipv4/tcp.c
> > > @@ -2495,7 +2495,7 @@ static inline bool tcp_need_reset(int state)
> > >  {
> > >         return (1 << state) &
> > >                (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
> > > -               TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
> > > +               TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
> > >  }
> > >
> > >  static void tcp_rtx_queue_purge(struct sock *sk)
> >
> > Hi
> >
> > 1) I do not believe this patch is complete :
> >    You have not changed tcp_disconnect() which seems to have dead code
> > if we apply your patch.
> >
> > 2) I am not sure we want to send yet another packet if the prior SYN
> > packets elect no answer.
> >   (It is not clear if your patch applies to this case)
> >
> > 3) If we do not RESET, the other side has not seen the 3rd packet and
> > should eventually exit from SYN_RECV state and die.
> >
> > 4) A RESET can be lost on the network, and nothing will retransmit it.
> >
> > Can you describe the use case more precisely, because it seems in
> > contradiction of a feature that we plan to upstream.
> > (TCP_SILENT_CLOSE : do not send flood of RST/FIN when a gigantic
> > server process prematurely dies)
> >
> > Thanks.
>
> The algorithm for our network switch needs to have all TCP sessions
> on the previous network cleaned up. So, we are trying to reset the
> connections in SYN-SENT state as well. Is there any other way to
> forcefully reset these connections immediately rather than waiting
> for the other side to eventually exit and die?

There is no way we can make sure all TCP sessions are cleaned up.
For example, we can not make sure no machine eventually crashes or is
unexpectedly powered down.

What is the "algorithm for your network switch" exactly.

Sorry, I am not convinced we want to add more RST packets on the network,
most SRE closely look at RST packets as some proxy of 'badness'.

Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state

[Devi Sandeep Endluri V V]

On Wed, Feb 20, 2019 at 07:47:59AM -0800, Eric Dumazet wrote:
> On Wed, Feb 20, 2019 at 6:28 AM Devi Sandeep Endluri V V
> <dendluri@codeaurora.org> wrote:
> >
> > Userspace sends tcp connection (sock) destroy on network permission
> > change. Kernel though doesn't send reset for the connections in
> > SYN-SENT state and these connections continue to remain. Even as
> > per RFC 793, there is no hard rule to not send RST on ABORT in
> > this state. Change to make sure RST are send for connections in
> > syn-sent state to avoid lingering connections on network switch.
> >
> > References from RFC 793
> >
> > ABORT Call
> >
> >         SYN-SENT STATE
> >
> >         All queued SENDs and RECEIVEs should be given "connection reset"
> >         notification, delete the TCB, enter CLOSED state, and return.
> >
> > SEGMENT ARRIVES
> >
> >         If the state is SYN-SENT then
> >         If the RST bit is set
> >
> >           If the ACK was acceptable then signal the user "error:
> >           connection reset", drop the segment, enter CLOSED state,
> >           delete TCB, and return.  Otherwise (no ACK) drop the segment
> >           and return.
> >
> > Signed-off-by: Devi Sandeep Endluri V V <dendluri@codeaurora.org>
> > ---
> >  net/ipv4/tcp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git net/ipv4/tcp.c net/ipv4/tcp.c
> > index cf3c509..8569dc5e 100644
> > --- net/ipv4/tcp.c
> > +++ net/ipv4/tcp.c
> > @@ -2495,7 +2495,7 @@ static inline bool tcp_need_reset(int state)
> >  {
> >         return (1 << state) &
> >                (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
> > -               TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
> > +               TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
> >  }
> >
> >  static void tcp_rtx_queue_purge(struct sock *sk)
> 
> Hi
> 
> 1) I do not believe this patch is complete :
>    You have not changed tcp_disconnect() which seems to have dead code
> if we apply your patch.
> 
> 2) I am not sure we want to send yet another packet if the prior SYN
> packets elect no answer.
>   (It is not clear if your patch applies to this case)
> 
> 3) If we do not RESET, the other side has not seen the 3rd packet and
> should eventually exit from SYN_RECV state and die.
> 
> 4) A RESET can be lost on the network, and nothing will retransmit it.
> 
> Can you describe the use case more precisely, because it seems in
> contradiction of a feature that we plan to upstream.
> (TCP_SILENT_CLOSE : do not send flood of RST/FIN when a gigantic
> server process prematurely dies)
> 
> Thanks.

The algorithm for our network switch needs to have all TCP sessions
on the previous network cleaned up. So, we are trying to reset the
connections in SYN-SENT state as well. Is there any other way to
forcefully reset these connections immediately rather than waiting
for the other side to eventually exit and die?

Re: [PATCH] tcp: Reset tcp connections in SYN-SENT state

[Eric Dumazet]

On Wed, Feb 20, 2019 at 6:28 AM Devi Sandeep Endluri V V
<dendluri@codeaurora.org> wrote:
>
> Userspace sends tcp connection (sock) destroy on network permission
> change. Kernel though doesn't send reset for the connections in
> SYN-SENT state and these connections continue to remain. Even as
> per RFC 793, there is no hard rule to not send RST on ABORT in
> this state. Change to make sure RST are send for connections in
> syn-sent state to avoid lingering connections on network switch.
>
> References from RFC 793
>
> ABORT Call
>
>         SYN-SENT STATE
>
>         All queued SENDs and RECEIVEs should be given "connection reset"
>         notification, delete the TCB, enter CLOSED state, and return.
>
> SEGMENT ARRIVES
>
>         If the state is SYN-SENT then
>         If the RST bit is set
>
>           If the ACK was acceptable then signal the user "error:
>           connection reset", drop the segment, enter CLOSED state,
>           delete TCB, and return.  Otherwise (no ACK) drop the segment
>           and return.
>
> Signed-off-by: Devi Sandeep Endluri V V <dendluri@codeaurora.org>
> ---
>  net/ipv4/tcp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git net/ipv4/tcp.c net/ipv4/tcp.c
> index cf3c509..8569dc5e 100644
> --- net/ipv4/tcp.c
> +++ net/ipv4/tcp.c
> @@ -2495,7 +2495,7 @@ static inline bool tcp_need_reset(int state)
>  {
>         return (1 << state) &
>                (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT | TCPF_FIN_WAIT1 |
> -               TCPF_FIN_WAIT2 | TCPF_SYN_RECV);
> +               TCPF_FIN_WAIT2 | TCPF_SYN_RECV | TCPF_SYN_SENT);
>  }
>
>  static void tcp_rtx_queue_purge(struct sock *sk)

Hi

1) I do not believe this patch is complete :
   You have not changed tcp_disconnect() which seems to have dead code
if we apply your patch.

2) I am not sure we want to send yet another packet if the prior SYN
packets elect no answer.
  (It is not clear if your patch applies to this case)

3) If we do not RESET, the other side has not seen the 3rd packet and
should eventually exit from SYN_RECV state and die.

4) A RESET can be lost on the network, and nothing will retransmit it.

Can you describe the use case more precisely, because it seems in
contradiction of a feature that we plan to upstream.
(TCP_SILENT_CLOSE : do not send flood of RST/FIN when a gigantic
server process prematurely dies)

Thanks.