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

Stanislav Fomichev
2025-06-30T23:19:25Z

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.