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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aGMbe0hxH78xQvD8@mini-arch>
Date: Mon, 30 Jun 2025 16:19:23 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Stanislav Fomichev <sdf@...ichev.me>, netdev@...r.kernel.org,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com
Subject: Re: [PATCH net-next v2 3/8] net:
 s/dev_get_mac_address/netif_get_mac_address/

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@...ichev.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.