Fix bug during call tcp_done.

[EvgeniiMekhanik]

When Tempesta FW closes socket from ss_do_close
function, Tempesta FW sends TCP FIN using tcp_send_fin or TCP RST using tcp_send_active_reset. Both this functions use tcp_write_xmit to push FIN/RST to network. If error occurs in tcp_write_xmit->tcp_tfw_sk_write_xmit we call tcp_tfw_handle_error->tcp_done. In this case ss_do_close called recursively from tcp_done function, that leads to extra inet_csk_destroy_sock call and extra socket put. This patch fixes this behaviour - Tempesta FW set new special flag at the beginning of ss_do_close and if this flag is set we don't call sk_state_change / inet_csk_destroy_sock from tcp_done.

[const-t]

I have proposal for this patch.

First of all I can't see cases how tcp_write_xmit() could be called from tcp_send_active_reset(), the last avoids using of write_queue and calls tcp_transmit_skb() directly. Therefore tcp_tfw_handle_error() can't be called from tcp_send_active_reset().

The second. Looking into ss_do_close() I see single call of tcp_send_fin() here, and only inside this function ss_do_close() might be called recursively. Let's breakdown this call chain:

(#1) ss_do_close()
          \/ 
tcp_send_fin() #Before the call set state to TCP_FIN_WAIT1
          \/
tcp_write_xmit() #Try to send skb with FIN.
          \/
handle_error() #Error occurred during sending FIN. 
          \/
tcp_done() #SET state to TCP_CLOSE.
          \/
(#2) ss_do_close() -> goto adjudge_to_death -> set socket as DEAD -> call inet_csk_destroy_sock() the first time.
          \/
return from tcp_done() to ss_do_close()
          \/
(#1) ss_do_close() -> fall to adjudge_to_death -> set socket as DEAD again -> call inet_csk_destroy_sock() second time, because state is TCP_CLOSE.

As we can see, we should not call tcp_done() in case of error during sending FIN, maybe instead of adding new flag we can rely on socket's state and do something like this:

tcp_tfw_handle_error(struct sock *sk, int error)
{
	tcp_send_active_reset(sk, GFP_ATOMIC);
	sk->sk_err = error;
	sk->sk_error_report(sk);
	tcp_write_queue_purge(sk);
	if (sk->sk_state == TCP_FIN_WAIT1)
		tcp_set_state(sk, TCP_CLOSE);
	else
		tcp_done(sk);
}

[EvgeniiMekhanik]

it seems that we can't rely on the state here. For example when we call tcp_shutdown->tcp_close_state->tcp_send_fin and if tcp_send_fin fails we should call tcp_done

[const-t]

Maybe we can check the state in tcp_shutdown as well?

static inline void
ss_do_shutdown(struct sock *sk)
{
	tcp_shutdown(sk, SEND_SHUTDOWN);
	SS_CONN_TYPE(sk) |= Conn_Shutdown;
       if (sk->state == TCP_CLOSE)
           ss_do_close(sk, 0);
}

[krizhanovsky]

Yeah, it'd be wonderful if @const-t 's proposal can be implemented: it's not so beautiful to introduce a new flag in the kernel patch just for one condition.