Re: [tipc-discussion] [PATCH net-next v2 05/16] tipc: feed tipc sock pointer to tipc_sk_timeout routine

[Jon M. <jon...@er...>]

> -----Original Message-----
> From: Ying Xue [mailto:yin...@wi...]
> Sent: December-30-14 10:02 PM
> To: ma...@do...; Erik Hugne; Pau...@wi...
> Cc: Jon Maloy; Richard Alpe; Ter...@co...;
> yin...@wi...; tip...@li...
> Subject: [PATCH net-next v2 05/16] tipc: feed tipc sock pointer to
> tipc_sk_timeout routine
> 
> In order to make tipc socket table aware of namespace, a networking
> namespace instance must be passed to tipc_sk_lookup(), allowing it to look
> up tipc socket instance with a given port ID from a concrete socket table.
> However, as now tipc_sk_timeout() only has one port ID parameter and

/s little knows the information of namespace,/is not namespace aware,/

///jon

> it's unable to obtain a correct socket
> instance through tipc_sk_lookup() just with a port ID, especially after
> namespace is completely supported.
> 
> If port ID is replaced with socket instance as tipc_sk_timeout()'s parameter,
> it's unnecessary to look up socket table. But as the timer handler -
> tipc_sk_timeout() is run asynchronously, socket reference must be held
> before its timer is launched, and must be carefully checked to identify
> whether the socket reference needs to be put or not when its timer is
> terminated.
> 
> Signed-off-by: Ying Xue <yin...@wi...>
> Tested-by: Tero Aho <Ter...@co...>
> ---
>  net/tipc/socket.c |   33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 3090fc1..cc529ae
> 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -110,7 +110,7 @@ static void tipc_write_space(struct sock *sk);  static int
> tipc_release(struct socket *sock);  static int tipc_accept(struct socket *sock,
> struct socket *new_sock, int flags);  static int tipc_wait_for_sndmsg(struct
> socket *sock, long *timeo_p); -static void tipc_sk_timeout(unsigned long
> portid);
> +static void tipc_sk_timeout(unsigned long data);
>  static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
>  			   struct tipc_name_seq const *seq);  static int
> tipc_sk_withdraw(struct tipc_sock *tsk, uint scope, @@ -369,7 +369,7 @@
> static int tipc_sk_create(struct net *net, struct socket *sock,
>  		return -EINVAL;
>  	}
>  	msg_set_origport(msg, tsk->portid);
> -	setup_timer(&tsk->timer, tipc_sk_timeout, tsk->portid);
> +	setup_timer(&tsk->timer, tipc_sk_timeout, (unsigned long)tsk);
>  	sk->sk_backlog_rcv = tipc_backlog_rcv;
>  	sk->sk_rcvbuf = sysctl_tipc_rmem[1];
>  	sk->sk_data_ready = tipc_data_ready;
> @@ -483,7 +483,7 @@ static int tipc_release(struct socket *sock)
>  	struct sock *sk = sock->sk;
>  	struct tipc_sock *tsk;
>  	struct sk_buff *skb;
> -	u32 dnode;
> +	u32 dnode, probing_state;
> 
>  	/*
>  	 * Exit if socket isn't fully initialized (occurs when a failed accept() @@
> -519,7 +519,9 @@ static int tipc_release(struct socket *sock)
>  	}
> 
>  	tipc_sk_withdraw(tsk, 0, NULL);
> -	del_timer_sync(&tsk->timer);
> +	probing_state = tsk->probing_state;
> +	if (del_timer_sync(&tsk->timer) && probing_state !=
> TIPC_CONN_PROBING)
> +		sock_put(sk);
>  	tipc_sk_remove(tsk);
>  	if (tsk->connected) {
>  		skb = tipc_msg_create(TIPC_CRITICAL_IMPORTANCE,
> TIPC_CONN_MSG, @@ -1149,7 +1151,8 @@ static void
> tipc_sk_finish_conn(struct tipc_sock *tsk, u32 peer_port,
>  	tsk->probing_intv = CONN_PROBING_INTERVAL;
>  	tsk->probing_state = TIPC_CONN_OK;
>  	tsk->connected = 1;
> -	mod_timer(&tsk->timer, jiffies + tsk->probing_intv);
> +	if (!mod_timer(&tsk->timer, jiffies + tsk->probing_intv))
> +		sock_hold(&tsk->sk);
>  	tipc_node_add_conn(peer_node, tsk->portid, peer_port);
>  	tsk->max_pkt = tipc_node_get_mtu(peer_node, tsk->portid);  } @@
> -2104,18 +2107,13 @@ restart:
>  	return res;
>  }
> 
> -static void tipc_sk_timeout(unsigned long portid)
> +static void tipc_sk_timeout(unsigned long data)
>  {
> -	struct tipc_sock *tsk;
> -	struct sock *sk;
> +	struct tipc_sock *tsk = (struct tipc_sock *)data;
> +	struct sock *sk = &tsk->sk;
>  	struct sk_buff *skb = NULL;
>  	u32 peer_port, peer_node;
> 
> -	tsk = tipc_sk_lookup(portid);
> -	if (!tsk)
> -		return;
> -
> -	sk = &tsk->sk;
>  	bh_lock_sock(sk);
>  	if (!tsk->connected) {
>  		bh_unlock_sock(sk);
> @@ -2128,18 +2126,19 @@ static void tipc_sk_timeout(unsigned long portid)
>  		/* Previous probe not answered -> self abort */
>  		skb = tipc_msg_create(TIPC_CRITICAL_IMPORTANCE,
> TIPC_CONN_MSG,
>  				      SHORT_H_SIZE, 0, tipc_own_addr,
> -				      peer_node, portid, peer_port,
> +				      peer_node, tsk->portid, peer_port,
>  				      TIPC_ERR_NO_PORT);
>  	} else {
>  		skb = tipc_msg_create(CONN_MANAGER, CONN_PROBE,
> INT_H_SIZE,
>  				      0, peer_node, tipc_own_addr,
> -				      peer_port, portid, TIPC_OK);
> +				      peer_port, tsk->portid, TIPC_OK);
>  		tsk->probing_state = TIPC_CONN_PROBING;
> -		mod_timer(&tsk->timer, jiffies + tsk->probing_intv);
> +		if (!mod_timer(&tsk->timer, jiffies + tsk->probing_intv))
> +			sock_hold(sk);
>  	}
>  	bh_unlock_sock(sk);
>  	if (skb)
> -		tipc_link_xmit_skb(skb, peer_node, portid);
> +		tipc_link_xmit_skb(skb, peer_node, tsk->portid);
>  exit:
>  	sock_put(sk);
>  }
> --
> 1.7.9.5