Just one minor comment below.
Acked-by: Jon Maloy <jon.ma...@ericsson.com>
> -----Original Message-----
> From: Tuong Lien <tuong.t.l...@dektech.com.au>
> Sent: 24-Dec-19 05:09
> To: tipc-discussion@lists.sourceforge.net; Jon Maloy
> <jon.ma...@ericsson.com>; ma...@donjonn.com;
> ying....@windriver.com
> Subject: [net] tipc: fix link overflow issue at socket shutdown
>
> When a socket is suddenly shutdown or released, it will reject all the
> unreceived messages in its receive queue. This applies to a connected
> socket too, whereas there is only one 'FIN' message required to be sent
> back to its peer in this case.
>
> In case there are many messages in the queue and/or some connections
> with such messages are shutdown at the same time, the link layer will
> easily get overflowed at the 'TIPC_SYSTEM_IMPORTANCE' backlog level
> because of the message rejections. As a result, the link will be taken
> down. Moreover, immediately when the link is re-established, the socket
> layer can continue to reject the messages and the same issue happens...
>
> The commit refactors the '__tipc_shutdown()' function to only send one
> 'FIN' in the situation mentioned above. For the connectionless case, it
> is unavoidable but usually there is no rejections for such socket
> messages because they are 'dest-droppable' by default.
>
> In addition, the new code makes the other socket states clear
> (e.g.'TIPC_LISTEN') and treats as a separate case to avoid misbehaving.
>
> --------------
> v2: completely refactor the function;
> cover the other socket states;
> fix a memleak issue (- reported by 'Hoang Huu Le').
> --------------
>
> Signed-off-by: Tuong Lien <tuong.t.l...@dektech.com.au>
> ---
> net/tipc/socket.c | 53 ++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 41688da233ab..aa0ffd0dba50 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -287,12 +287,12 @@ static void tipc_sk_respond(struct sock *sk, struct
> sk_buff *skb, int err)
> *
> * Caller must hold socket lock
> */
> -static void tsk_rej_rx_queue(struct sock *sk)
> +static void tsk_rej_rx_queue(struct sock *sk, int error)
> {
> struct sk_buff *skb;
>
> while ((skb = __skb_dequeue(&sk->sk_receive_queue)))
> - tipc_sk_respond(sk, skb, TIPC_ERR_NO_PORT);
> + tipc_sk_respond(sk, skb, error);
> }
>
> static bool tipc_sk_connected(struct sock *sk)
> @@ -545,34 +545,45 @@ static void __tipc_shutdown(struct socket *sock, int
> error)
> /* Remove pending SYN */
> __skb_queue_purge(&sk->sk_write_queue);
>
> - /* Reject all unreceived messages, except on an active connection
> - * (which disconnects locally & sends a 'FIN+' to peer).
> - */
> - while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
> - if (TIPC_SKB_CB(skb)->bytes_read) {
> - kfree_skb(skb);
> - continue;
> - }
> - if (!tipc_sk_type_connectionless(sk) &&
> - sk->sk_state != TIPC_DISCONNECTING) {
> - tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> - tipc_node_remove_conn(net, dnode, tsk->portid);
> - }
> - tipc_sk_respond(sk, skb, error);
> + /* Remove partial received buffer if any */
s/partial/partially/
> + skb = skb_peek(&sk->sk_receive_queue);
> + if (skb && TIPC_SKB_CB(skb)->bytes_read) {
> + __skb_unlink(skb, &sk->sk_receive_queue);
> + kfree_skb(skb);
> }
>
> - if (tipc_sk_type_connectionless(sk))
> + /* Reject all unreceived messages if connectionless */
> + if (tipc_sk_type_connectionless(sk)) {
> + tsk_rej_rx_queue(sk, error);
> return;
> + }
>
> - if (sk->sk_state != TIPC_DISCONNECTING) {
> + switch (sk->sk_state) {
> + case TIPC_CONNECTING:
> + case TIPC_ESTABLISHED:
> + tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> + tipc_node_remove_conn(net, dnode, tsk->portid);
> + /* Send a FIN+/- to its peer */
> + skb = __skb_dequeue(&sk->sk_receive_queue);
> + if (skb) {
> + __skb_queue_purge(&sk->sk_receive_queue);
> + tipc_sk_respond(sk, skb, error);
> + break;
> + }
> skb = tipc_msg_create(TIPC_CRITICAL_IMPORTANCE,
> TIPC_CONN_MSG, SHORT_H_SIZE, 0, dnode,
> tsk_own_node(tsk), tsk_peer_port(tsk),
> tsk->portid, error);
> if (skb)
> tipc_node_xmit_skb(net, skb, dnode, tsk->portid);
> - tipc_node_remove_conn(net, dnode, tsk->portid);
> - tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> + break;
> + case TIPC_LISTEN:
> + /* Reject all SYN messages */
> + tsk_rej_rx_queue(sk, error);
> + break;
> + default:
> + __skb_queue_purge(&sk->sk_receive_queue);
> + break;
> }
> }
>
> @@ -2639,7 +2650,7 @@ static int tipc_accept(struct socket *sock, struct
> socket *new_sock, int flags,
> * Reject any stray messages received by new socket
> * before the socket lock was taken (very, very unlikely)
> */
> - tsk_rej_rx_queue(new_sk);
> + tsk_rej_rx_queue(new_sk, TIPC_ERR_NO_PORT);
>
> /* Connect new socket to it's peer */
> tipc_sk_finish_conn(new_tsock, msg_origport(msg), msg_orignode(msg));
> --
> 2.13.7
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
