Re: [B.A.T.M.A.N.] [PATCHv5 1/2] batman-adv: add per VLAN interface attribute framework

[Marek Lindner <lindner_marek@yahoo.de>]

On Thursday, June 13, 2013 02:04:19 Antonio Quartulli wrote:
> --- a/hard-interface.c
> +++ b/hard-interface.c
> @@ -643,6 +643,8 @@ static int batadv_hard_if_event(struct notifier_block
> *this,
> 
>  	if (batadv_softif_is_valid(net_dev) && event == NETDEV_REGISTER) {
>  		batadv_sysfs_add_meshif(net_dev);
> +		bat_priv = netdev_priv(net_dev);
> +		batadv_softif_create_vlan(bat_priv, BATADV_NO_FLAGS);
>  		return NOTIFY_DONE;
>  	}

When is this entry ever removed ? Did you try to rmmod the module with these 
patches enabled ?


> +       /* explicitly remove the associated TT local entry because it is 
marked
> +        * with the NOPURGE flag
> +        */
> +       batadv_tt_local_remove(bat_priv, dev->dev_addr, vid,
> +                              "vlan interface destroyed", false);

Curious. In batadv_interface_add_vid() you force the BATADV_VLAN_HAS_TAG tag 
onto each batadv_tt_local_add() call but on removal we don't have the flag 
set. Are you certain that is correct ?


> @@ -613,6 +632,8 @@ struct batadv_priv {
>  	struct work_struct cleanup_work;
>  	struct batadv_hard_iface __rcu *primary_if;  /* rcu protected pointer */
>  	struct batadv_algo_ops *bat_algo_ops;
> +	struct list_head softif_vlan_list;
> +	spinlock_t softif_vlan_list_lock; /* protects softif_vlan_list */
>  #ifdef CONFIG_BATMAN_ADV_BLA
>  	struct batadv_priv_bla bla;
>  #endif

Why do we need a full blown list ? Isn't hlist sufficient ?

Cheers,
Marek