On 09/25/2018 07:34 AM, Jon Maloy wrote:
> We refactor the function tipc_sk_filter_connect(), both to make it
> more readable and as a preparation for the next commit.
>
> Signed-off-by: Jon Maloy <[email protected]>
> ---
> net/tipc/socket.c | 101
> +++++++++++++++++++++++-------------------------------
> 1 file changed, 43 insertions(+), 58 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 4c50cff..14e6471 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1959,91 +1959,76 @@ static void tipc_sk_proto_rcv(struct sock *sk,
> }
>
> /**
> - * tipc_filter_connect - Handle incoming message for a connection-based
> socket
> + * tipc_sk_filter_connect - check incoming message for a connection-based
> socket
> * @tsk: TIPC socket
> - * @skb: pointer to message buffer. Set to NULL if buffer is consumed
> - *
> - * Returns true if everything ok, false otherwise
> + * @skb: pointer to message buffer.
> + * Returns true if message should be added to receive queue, false otherwise
> */
> static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff
> *skb)
> {
> struct sock *sk = &tsk->sk;
> struct net *net = sock_net(sk);
> struct tipc_msg *hdr = buf_msg(skb);
> - u32 pport = msg_origport(hdr);
> - u32 pnode = msg_orignode(hdr);
> + bool con_msg = msg_connected(hdr);
> + u32 pport = tsk_peer_port(tsk);
> + u32 pnode = tsk_peer_node(tsk);
> + u32 oport = msg_origport(hdr);
> + u32 onode = msg_orignode(hdr);
> + int err = msg_errcode(hdr);
>
> if (unlikely(msg_mcast(hdr)))
> return false;
>
> switch (sk->sk_state) {
> case TIPC_CONNECTING:
> - /* Accept only ACK or NACK message */
> - if (unlikely(!msg_connected(hdr))) {
> - if (pport != tsk_peer_port(tsk) ||
> - pnode != tsk_peer_node(tsk))
> - return false;
> -
> - tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> - sk->sk_err = ECONNREFUSED;
> - sk->sk_state_change(sk);
> - return true;
> - }
> -
> - if (unlikely(msg_errcode(hdr))) {
> - tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> - sk->sk_err = ECONNREFUSED;
> - sk->sk_state_change(sk);
> - return true;
> - }
> -
> - if (unlikely(!msg_isdata(hdr))) {
> - tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> - sk->sk_err = EINVAL;
> - sk->sk_state_change(sk);
> - return true;
> + /* Setup ACK */
> + if (likely(con_msg)) {
> + if (err)
> + break;
> + tipc_sk_finish_conn(tsk, oport, onode);
> + msg_set_importance(&tsk->phdr, msg_importance(hdr));
> + /* ACK+ message with data is added to receive queue */
> + if (msg_data_sz(hdr))
> + return true;
> + /* Empty ACK-, - wake up sleeping connect() and drop */
> + sk->sk_data_ready(sk);
> + msg_set_dest_droppable(hdr, 1);
> + return false;
> }
> + /* Ignore connectionless message if not from listening socket */
> + if (oport != pport || onode != pnode)
> + return false;
>
> - tipc_sk_finish_conn(tsk, msg_origport(hdr), msg_orignode(hdr));
> - msg_set_importance(&tsk->phdr, msg_importance(hdr));
> -
> - /* If 'ACK+' message, add to socket receive queue */
> - if (msg_data_sz(hdr))
> - return true;
> -
> - /* If empty 'ACK-' message, wake up sleeping connect() */
> - sk->sk_data_ready(sk);
> -
> - /* 'ACK-' message is neither accepted nor rejected: */
> - msg_set_dest_droppable(hdr, 1);
> - return false;
> -
> + /* Rejected SYN - abort */
> + break;
> case TIPC_OPEN:
> case TIPC_DISCONNECTING:
> - break;
> + return false;
> case TIPC_LISTEN:
> /* Accept only SYN message */
> - if (!msg_connected(hdr) && !(msg_errcode(hdr)))
> + if (!con_msg && !err)
> return true;
> - break;
> + return false;
> case TIPC_ESTABLISHED:
> /* Accept only connection-based messages sent by peer */
> - if (unlikely(!tsk_peer_msg(tsk, hdr)))
> + if (likely(con_msg && !err && pport == oport && pnode == onode))
> + return true;
> + if (!tsk_peer_msg(tsk, hdr))
> return false;
> -
> - if (unlikely(msg_errcode(hdr))) {
> - tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> - /* Let timer expire on it's own */
> - tipc_node_remove_conn(net, tsk_peer_node(tsk),
> - tsk->portid);
> - sk->sk_state_change(sk);
> - }
> + if (!err)
> + return true;
> + tipc_set_sk_state(sk, TIPC_DISCONNECTING);
It's better to set a error code to sk->sk_err.
> + tipc_node_remove_conn(net, pnode, tsk->portid);
> + sk->sk_state_change(sk);
> return true;
> default:
> pr_err("Unknown sk_state %u\n", sk->sk_state);
> }
> -
> - return false;
> + /* Abort connection setup attempt */
> + tipc_set_sk_state(sk, TIPC_DISCONNECTING);
> + sk->sk_err = ECONNREFUSED;
> + sk->sk_state_change(sk);
> + return true;
> }
>
> /**
>
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
