Re: [PATCH v2] batman-adv: add dynamic, bridged-in TT VID detection support

["Nicolas Escande" <nico.escande@gmail.com>]

On Thu Nov 14, 2024 at 2:58 PM CET, Sven Eckelmann wrote:
> On Thursday, 14 November 2024 14:48:52 CET Nicolas Escande wrote:
> > Hi there,
> > 
> > We've been running this for a few time and it's very usefull. So is there any
> > news on merging this into the kernel ? Or is the BLA thing blocking ?
>
> I am not in favour of changing the behavior of batman-adv. Now everyone can 
> increase the number of managed VLANs without any control by the node admin.
Well ok but but it makes configuration so much easier in a setup where we have
many vlan that I still find this usefull.
>
> And as Linus wrote, it also shows odd behaviors. And Antonio also didn't write 
> his opinion here. I have therefore downgraded it from PATCH to RFC (instead of 
> simply rejecting it).
This is about the BLA behaviour ? On a setup that doesn't use BLA like mine it
makes things so much easier to configure that I still find this patch usefull :)
>
> [...]
> > Or maybe I missed something ?
> >  
> > ---
> >  net/batman-adv/soft-interface.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
> > index b61f35918b5d..d7de54734725 100644
> > --- a/net/batman-adv/soft-interface.c
> > +++ b/net/batman-adv/soft-interface.c
> > @@ -599,7 +599,6 @@ batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
> >  
> >  	atomic_set(&vlan->ap_isolation, 0);
> >  
> > -	kref_get(&vlan->refcount);
> >  	hlist_add_head_rcu(&vlan->list, &bat_priv->softif_vlan_list);
> >  	spin_unlock_bh(&bat_priv->softif_vlan_list_lock);
> >  
> > 
>
> This ref is for the VLAN list entry (just one line below the kref_get).
> This patch is therefore definitely wrong.

That's the part I have trouble understanding, we keep 1 ref for the list, 1 ref
per TT entry using this vlan. And on interface deletion, we clear the TT tables
(so we go back to a refcount of 1 for the global list), but I don't see where we
clear the list itself ?
>
> Kind regards,
> 	Sven