Re: [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: additional checks for virtual interfaces on top of WiFi

[Sven Eckelmann <sven@narfation.org>]

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

On Dienstag, 12. Juli 2016 14:33:09 CEST Sven Eckelmann wrote:
[....]
> > diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-
interface.c
> > index 478977b..6324474 100644
> > --- a/net/batman-adv/hard-interface.c
> > +++ b/net/batman-adv/hard-interface.c
> [...]
> > +struct net_device *batadv_get_real_netdev(struct net_device *net_device)
> > +{
> [...]
> > +	net = dev_net(hard_iface->soft_iface);
> > +	ifindex = dev_get_iflink(net_device);
> > +	real_netdev = dev_get_by_index(net, ifindex);
> 
> Andrew provided a patch [1] which gets the correct namespace of a link. You
> should consider to first accept his patch and then use batadv_getlink_net to
> get the correct namespace for the device instead of just assuming that it 
will
> be in the same netns as the batman-adv interface.
> 
> Something like this:
> 
>        net = dev_net(hard_iface->soft_iface);
>        ifindex = dev_get_iflink(net_device);
>        real_net = = batadv_getlink_net(net_device, net);
>        real_netdev = dev_get_by_index(real_net, ifindex);
> 
> Could it also be that this has to be done with the rtnl lock held? Otherwise
> some of the the information may change while we are going through all the
> steps to get the real interface.

Maybe you can add ASSERT_RTNL(); in this function. The caller of these
functions (see patch 3) have to make sure that they take the rtnl_lock().

batadv_is_cfg80211_netdev could get the rtnl_lock because it is used for the
elp code (which isn't inside the rtnl_lock, right?). But the call to
batadv_get_real_netdev in batadv_v_elp_get_throughput from Patch 3 still has
to be protected.

Or you could create an extra function which takes care of getting the real 
device for batadv_v_elp_get_throughput when it is a cfg80211 based one 
(otherwise returning NULL). This function can take care of getting the lock. 
Then you can also drop this
batadv_is_cfg80211_netdev -> _batadv_is_cfg80211_netdev change. Of course, you 
have to re-arrange many things in your patchset.

1. create batadv_is_cfg80211_netdev
2. introduce function that returns the device when it is a cfg80211
   * make use of it in batadv_v_elp_get_throughput and use it instead of
     batadv_is_cfg80211_netdev
3. introduce batadv_get_real_netdev
   * make use of it in your newly created function of patch 2
   * don't use batadv_get_real_netdev directly in batadv_v_elp_get_throughput

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]