Re: [B.A.T.M.A.N.] [PATCH 2/7] batman-adv: split tq information in neigh_node struct

[Antonio Quartulli <antonio@meshcoding.com>]

[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]

On Wed, Oct 09, 2013 at 03:05:33PM +0200, Simon Wunderlich wrote:
> From: Simon Wunderlich <simon@open-mesh.com>
> 
> For the network wide multi interface optimization it is required to save
> metrics per outgoing interface in one neighbor. Therefore a new type is
> introduced to keep interface-specific information. This also requires
> some changes in access and list management.
> 
> Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
> ---

[...]

>  void batadv_gw_check_election(struct batadv_priv *bat_priv,
>  			      struct batadv_orig_node *orig_node)
>  {
> +	struct batadv_neigh_node_ifinfo *router_orig_tq = NULL;
> +	struct batadv_neigh_node_ifinfo *router_gw_tq = NULL;
>  	struct batadv_orig_node *curr_gw_orig;
>  	struct batadv_neigh_node *router_gw = NULL, *router_orig = NULL;
>  	uint8_t gw_tq_avg, orig_tq_avg;
>  
> +	rcu_read_lock();
>  	curr_gw_orig = batadv_gw_get_selected_orig(bat_priv);
>  	if (!curr_gw_orig)
>  		goto deselect;
> @@ -297,6 +316,10 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,
>  	if (!router_gw)
>  		goto deselect;
>  
> +	router_gw_tq = batadv_neigh_node_get_ifinfo(router_gw, NULL);
> +	if (!router_gw_tq)
> +		goto deselect;
> +
>  	/* this node already is the gateway */
>  	if (curr_gw_orig == orig_node)
>  		goto out;
> @@ -305,8 +328,15 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,
>  	if (!router_orig)
>  		goto out;
>  
> -	gw_tq_avg = router_gw->bat_iv.tq_avg;
> -	orig_tq_avg = router_orig->bat_iv.tq_avg;
> +	router_orig_tq = batadv_neigh_node_get_ifinfo(router_orig, NULL);
> +	if (!router_orig_tq) {
> +		batadv_gw_deselect(bat_priv);
> +		goto out;
> +	}
> +
> +
> +	gw_tq_avg = router_gw_tq->bat_iv.tq_avg;
> +	orig_tq_avg = router_orig_tq->bat_iv.tq_avg;
>  
>  	/* the TQ value has to be better */
>  	if (orig_tq_avg < gw_tq_avg)
> @@ -324,6 +354,7 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,
>  		   gw_tq_avg, orig_tq_avg);
>  
>  deselect:
> +	rcu_read_unlock();
>  	batadv_gw_deselect(bat_priv);
>  out:

there are several "goto out;" after having acquired the rcu_read_lock.....
maybe they should be converted to "goto deselect;"? Otherwise this
rcu_read_unlock() has been misplaced.

Next time use C=2 when compiling your code: sparse would have told you about
this mistake :-) (this is how I found it).


Cheers,

-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]