Re: [PATCH net-next v2 3/8] net: s/dev_get_mac_address/netif_get_mac_address/

[Stanislav Fomichev <stfomichev@gmail.com>]

On 06/30, Willem de Bruijn wrote:
> Stanislav Fomichev wrote:
> > Commit cc34acd577f1 ("docs: net: document new locking reality")
> > introduced netif_ vs dev_ function semantics: the former expects locked
> > netdev, the latter takes care of the locking. We don't strictly
> > follow this semantics on either side, but there are more dev_xxx handlers
> > now that don't fit. Rename them to netif_xxx where appropriate.
> > 
> > netif_get_mac_address is used only by tun/tap, so move it into
> > NETDEV_INTERNAL namespace.
> > 
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > ---
> >  drivers/net/tap.c         | 6 ++++--
> >  drivers/net/tun.c         | 4 +++-
> >  include/linux/netdevice.h | 2 +-
> >  net/core/dev.c            | 4 ++--
> >  net/core/dev_ioctl.c      | 3 ++-
> >  net/core/net-sysfs.c      | 2 +-
> >  6 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> > index bdf0788d8e66..4c85770c809b 100644
> > --- a/drivers/net/tap.c
> > +++ b/drivers/net/tap.c
> > @@ -28,6 +28,8 @@
> >  
> >  #include "tun_vnet.h"
> >  
> > +MODULE_IMPORT_NS("NETDEV_INTERNAL");
> > +
> >  #define TAP_IFFEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE)
> >  
> >  static struct proto tap_proto = {
> > @@ -1000,8 +1002,8 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
> >  			return -ENOLINK;
> >  		}
> >  		ret = 0;
> > -		dev_get_mac_address((struct sockaddr *)&ss, dev_net(tap->dev),
> > -				    tap->dev->name);
> > +		netif_get_mac_address((struct sockaddr *)&ss, dev_net(tap->dev),
> > +				      tap->dev->name);
> >  		if (copy_to_user(&ifr->ifr_name, tap->dev->name, IFNAMSIZ) ||
> >  		    copy_to_user(&ifr->ifr_hwaddr, &ss, sizeof(ifr->ifr_hwaddr)))
> >  			ret = -EFAULT;
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index f8c5e2fd04df..4509ae68decf 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -85,6 +85,8 @@
> >  
> >  #include "tun_vnet.h"
> >  
> > +MODULE_IMPORT_NS("NETDEV_INTERNAL");
> > +
> 
> Thanks for giving this a go. Now that you've implemented it, does the
> risk (of overlooking callers, mainly) indeed seem acceptable?
> 
> Documentation/core-api/symbol-namespaces.rst says
> 
>   It is advisable to add the MODULE_IMPORT_NS() statement close to other module
>   metadata definitions like MODULE_AUTHOR() or MODULE_LICENSE().
> 
> No need to respin just for this from me. Something to consider,
> especially if anything else comes up.

I put it at the top because it was at the top in bnxt. But it is
at the top in bnxt is because the MODULE_LICENSE is there :-(
Thanks for pointing it out, I'll definitely address that to be
consistent.

> Just curious, did you use the modpost and make nsdeps, or was it
> sufficient to find the callers with tools like cscope and grep?

Only grep. I'm hoping the build bots will tell me if missed something.