[PATCH net-next 1/2] net: un-export init_dummy_netdev()

[PATCH net-next 1/2] net: un-export init_dummy_netdev()

[Jakub Kicinski]

There are no in-tree module callers of init_dummy_netdev(), AFAICT.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/dev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1a90ed8cc6cc..23e7f6a3925b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10782,7 +10782,6 @@ void init_dummy_netdev(struct net_device *dev)
 	memset(dev, 0, sizeof(struct net_device));
 	init_dummy_netdev_core(dev);
 }
-EXPORT_SYMBOL_GPL(init_dummy_netdev);
 
 /**
  *	register_netdev	- register a network device
-- 
2.47.1

[PATCH net-next 2/2] net: initialize netdev->lock on dummy devices

[Jakub Kicinski]

Make sure netdev->lock is always valid, even on dummy netdevs.

Apparently it's legal to call mutex_destroy() on an uninitialized
mutex (and we do that in free_netdev()), but it doesn't seem right.
Plus we'll soon want to take netdev->lock on more paths which dummy
netdevs may reach.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 23e7f6a3925b..00552197d601 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10756,6 +10756,8 @@ static void init_dummy_netdev_core(struct net_device *dev)
 	/* napi_busy_loop stats accounting wants this */
 	dev_net_set(dev, &init_net);
 
+	mutex_init(&dev->lock);
+
 	/* Note : We dont allocate pcpu_refcnt for dummy devices,
 	 * because users of this 'device' dont need to change
 	 * its refcount.
-- 
2.47.1

Re: [PATCH net-next 2/2] net: initialize netdev->lock on dummy devices

[Andrew Lunn]

On Fri, Jan 10, 2025 at 10:59:55PM -0800, Jakub Kicinski wrote:
> Make sure netdev->lock is always valid, even on dummy netdevs.
> 
> Apparently it's legal to call mutex_destroy() on an uninitialized
> mutex (and we do that in free_netdev()), but it doesn't seem right.
> Plus we'll soon want to take netdev->lock on more paths which dummy
> netdevs may reach.

I assume here that dummy does not call alloc_netdev_mqs() or one of it
wrappers? That is how the lock seems to get initialised for real MAC
drivers. Are there other bits of initialisation in that function which
dummy is missing? Should we really be refactoring alloc_netdev_mqs()
to expose an initialisation helper for everything which is not related
queues?

	Andrew

Re: [PATCH net-next 2/2] net: initialize netdev->lock on dummy devices

[Jakub Kicinski]

On Sat, 11 Jan 2025 19:58:40 +0100 Andrew Lunn wrote:
> On Fri, Jan 10, 2025 at 10:59:55PM -0800, Jakub Kicinski wrote:
> > Make sure netdev->lock is always valid, even on dummy netdevs.
> > 
> > Apparently it's legal to call mutex_destroy() on an uninitialized
> > mutex (and we do that in free_netdev()), but it doesn't seem right.
> > Plus we'll soon want to take netdev->lock on more paths which dummy
> > netdevs may reach.  
> 
> I assume here that dummy does not call alloc_netdev_mqs() or one of it
> wrappers? 

Yes, we have both dummies which go thru alloc and static ones.

> That is how the lock seems to get initialised for real MAC
> drivers. Are there other bits of initialisation in that function which
> dummy is missing? Should we really be refactoring alloc_netdev_mqs()
> to expose an initialisation helper for everything which is not related
> queues?

You make a good point. Let me do the opposite, we only have two callers
of init_dummy_netdev(). Instead of unexporting it let me delete it
completely and make the two callers allocate the netdevs with
alloc_netdev_mqs().
-- 
pw-bot: cr

Re: [PATCH net-next 1/2] net: un-export init_dummy_netdev()

[Kalesh Anakkur Purayil]

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

On Sat, Jan 11, 2025 at 12:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> There are no in-tree module callers of init_dummy_netdev(), AFAICT.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/dev.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1a90ed8cc6cc..23e7f6a3925b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10782,7 +10782,6 @@ void init_dummy_netdev(struct net_device *dev)
>         memset(dev, 0, sizeof(struct net_device));
>         init_dummy_netdev_core(dev);
>  }
> -EXPORT_SYMBOL_GPL(init_dummy_netdev);
>
>  /**
>   *     register_netdev - register a network device
> --
> 2.47.1
>
>
I can see that "net/xfrm/xfrm_input.c" and "net/mptcp/protocol.c" are
invoking init_dummy_netdev() in the init routines.

-- 
Regards,
Kalesh AP

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

Re: [PATCH net-next 1/2] net: un-export init_dummy_netdev()

[Kalesh Anakkur Purayil]

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

My bad, missed the V2 of the series. Please ignore my comment.

On Mon, Jan 13, 2025 at 10:44 AM Kalesh Anakkur Purayil
<kalesh-anakkur.purayil@broadcom.com> wrote:
>
> On Sat, Jan 11, 2025 at 12:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > There are no in-tree module callers of init_dummy_netdev(), AFAICT.
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  net/core/dev.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 1a90ed8cc6cc..23e7f6a3925b 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -10782,7 +10782,6 @@ void init_dummy_netdev(struct net_device *dev)
> >         memset(dev, 0, sizeof(struct net_device));
> >         init_dummy_netdev_core(dev);
> >  }
> > -EXPORT_SYMBOL_GPL(init_dummy_netdev);
> >
> >  /**
> >   *     register_netdev - register a network device
> > --
> > 2.47.1
> >
> >
> I can see that "net/xfrm/xfrm_input.c" and "net/mptcp/protocol.c" are
> invoking init_dummy_netdev() in the init routines.
>
> --
> Regards,
> Kalesh AP



-- 
Regards,
Kalesh AP

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

Re: [PATCH net-next 1/2] net: un-export init_dummy_netdev()

[Andrew Lunn]

On Mon, Jan 13, 2025 at 10:44:37AM +0530, Kalesh Anakkur Purayil wrote:
> On Sat, Jan 11, 2025 at 12:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > There are no in-tree module callers of init_dummy_netdev(), AFAICT.
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  net/core/dev.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 1a90ed8cc6cc..23e7f6a3925b 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -10782,7 +10782,6 @@ void init_dummy_netdev(struct net_device *dev)
> >         memset(dev, 0, sizeof(struct net_device));
> >         init_dummy_netdev_core(dev);
> >  }
> > -EXPORT_SYMBOL_GPL(init_dummy_netdev);
> >
> >  /**
> >   *     register_netdev - register a network device
> > --
> > 2.47.1
> >
> >
> I can see that "net/xfrm/xfrm_input.c" and "net/mptcp/protocol.c" are
> invoking init_dummy_netdev() in the init routines.

I thought that initially. And then i checked that they can only be
built in. You only need exports for modules.

      Andrew

Re: [PATCH net-next 1/2] net: un-export init_dummy_netdev()

[Kalesh Anakkur Purayil]

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

On Mon, Jan 13, 2025 at 10:22 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Jan 13, 2025 at 10:44:37AM +0530, Kalesh Anakkur Purayil wrote:
> > On Sat, Jan 11, 2025 at 12:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > There are no in-tree module callers of init_dummy_netdev(), AFAICT.
> > >
> > > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > > ---
> > >  net/core/dev.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 1a90ed8cc6cc..23e7f6a3925b 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -10782,7 +10782,6 @@ void init_dummy_netdev(struct net_device *dev)
> > >         memset(dev, 0, sizeof(struct net_device));
> > >         init_dummy_netdev_core(dev);
> > >  }
> > > -EXPORT_SYMBOL_GPL(init_dummy_netdev);
> > >
> > >  /**
> > >   *     register_netdev - register a network device
> > > --
> > > 2.47.1
> > >
> > >
> > I can see that "net/xfrm/xfrm_input.c" and "net/mptcp/protocol.c" are
> > invoking init_dummy_netdev() in the init routines.
>
> I thought that initially. And then i checked that they can only be
> built in. You only need exports for modules.

Got it, thank you Andrew for the clarification.
>
>       Andrew



-- 
Regards,
Kalesh AP

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]