* [PATCH net] macvlan: Return error on register_netdevice_notifier() failure
@ 2024-07-24 13:56 Shigeru Yoshida
2024-07-24 18:40 ` Simon Horman
2024-07-25 9:44 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Shigeru Yoshida @ 2024-07-24 13:56 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni; +Cc: netdev, linux-kernel, Shigeru Yoshida
register_netdevice_notifier() may fail, but macvlan_init_module() does
not handle the failure. Handle the failure by returning an error.
Fixes: b863ceb7ddce ("[NET]: Add macvlan driver")
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
drivers/net/macvlan.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 24298a33e0e9..ae2f1a8325a5 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1849,7 +1849,9 @@ static int __init macvlan_init_module(void)
{
int err;
- register_netdevice_notifier(&macvlan_notifier_block);
+ err = register_netdevice_notifier(&macvlan_notifier_block);
+ if (err < 0)
+ return err;
err = macvlan_link_register(&macvlan_link_ops);
if (err < 0)
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] macvlan: Return error on register_netdevice_notifier() failure 2024-07-24 13:56 [PATCH net] macvlan: Return error on register_netdevice_notifier() failure Shigeru Yoshida @ 2024-07-24 18:40 ` Simon Horman 2024-07-25 9:44 ` Eric Dumazet 1 sibling, 0 replies; 6+ messages in thread From: Simon Horman @ 2024-07-24 18:40 UTC (permalink / raw) To: Shigeru Yoshida; +Cc: davem, edumazet, kuba, pabeni, netdev, linux-kernel On Wed, Jul 24, 2024 at 10:56:22PM +0900, Shigeru Yoshida wrote: > register_netdevice_notifier() may fail, but macvlan_init_module() does > not handle the failure. Handle the failure by returning an error. > > Fixes: b863ceb7ddce ("[NET]: Add macvlan driver") > Signed-off-by: Shigeru Yoshida <syoshida@redhat.com> Reviewed-by: Simon Horman <horms@kernel.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] macvlan: Return error on register_netdevice_notifier() failure 2024-07-24 13:56 [PATCH net] macvlan: Return error on register_netdevice_notifier() failure Shigeru Yoshida 2024-07-24 18:40 ` Simon Horman @ 2024-07-25 9:44 ` Eric Dumazet 2024-07-25 10:13 ` Paolo Abeni 1 sibling, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2024-07-25 9:44 UTC (permalink / raw) To: Shigeru Yoshida; +Cc: davem, kuba, pabeni, netdev, linux-kernel On Wed, Jul 24, 2024 at 3:56 PM Shigeru Yoshida <syoshida@redhat.com> wrote: > > register_netdevice_notifier() may fail, but macvlan_init_module() does > not handle the failure. Handle the failure by returning an error. How could this fail exactly ? Please provide details, because I do not think it can. > > Fixes: b863ceb7ddce ("[NET]: Add macvlan driver") > Signed-off-by: Shigeru Yoshida <syoshida@redhat.com> > --- > drivers/net/macvlan.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 24298a33e0e9..ae2f1a8325a5 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -1849,7 +1849,9 @@ static int __init macvlan_init_module(void) > { > int err; > > - register_netdevice_notifier(&macvlan_notifier_block); > + err = register_netdevice_notifier(&macvlan_notifier_block); > + if (err < 0) > + return err; > > err = macvlan_link_register(&macvlan_link_ops); > if (err < 0) > -- > 2.45.2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] macvlan: Return error on register_netdevice_notifier() failure 2024-07-25 9:44 ` Eric Dumazet @ 2024-07-25 10:13 ` Paolo Abeni 2024-07-25 11:53 ` Eric Dumazet 0 siblings, 1 reply; 6+ messages in thread From: Paolo Abeni @ 2024-07-25 10:13 UTC (permalink / raw) To: Eric Dumazet, Shigeru Yoshida; +Cc: davem, kuba, netdev, linux-kernel On 7/25/24 11:44, Eric Dumazet wrote: > On Wed, Jul 24, 2024 at 3:56 PM Shigeru Yoshida <syoshida@redhat.com> wrote: >> >> register_netdevice_notifier() may fail, but macvlan_init_module() does >> not handle the failure. Handle the failure by returning an error. > > How could this fail exactly ? Please provide details, because I do not > think it can. Yup, it looks like the registration can't fail for macvlan. It's better to avoid adding unneeded checks, to reduce noise on the tree, keep stable backport easy, etc. Thanks, Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] macvlan: Return error on register_netdevice_notifier() failure 2024-07-25 10:13 ` Paolo Abeni @ 2024-07-25 11:53 ` Eric Dumazet 2024-07-25 14:05 ` Shigeru Yoshida 0 siblings, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2024-07-25 11:53 UTC (permalink / raw) To: Paolo Abeni; +Cc: Shigeru Yoshida, davem, kuba, netdev, linux-kernel On Thu, Jul 25, 2024 at 12:13 PM Paolo Abeni <pabeni@redhat.com> wrote: > > > > On 7/25/24 11:44, Eric Dumazet wrote: > > On Wed, Jul 24, 2024 at 3:56 PM Shigeru Yoshida <syoshida@redhat.com> wrote: > >> > >> register_netdevice_notifier() may fail, but macvlan_init_module() does > >> not handle the failure. Handle the failure by returning an error. > > > > How could this fail exactly ? Please provide details, because I do not > > think it can. > > Yup, it looks like the registration can't fail for macvlan. > > It's better to avoid adding unneeded checks, to reduce noise on the > tree, keep stable backport easy, etc. Shigeru, you could send a debug patch when net-next reopens next week, so that we do not get another attempt on fixing a non-existent bug. diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 24298a33e0e94851ebf9c704c723f25ac7bf5eec..0803fcf8df4c56ede10597c862288c7aa887160e 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -1849,7 +1849,8 @@ static int __init macvlan_init_module(void) { int err; - register_netdevice_notifier(&macvlan_notifier_block); + err = register_netdevice_notifier(&macvlan_notifier_block); + DEBUG_NET_WARN_ON_ONCE(err < 0); err = macvlan_link_register(&macvlan_link_ops); if (err < 0) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] macvlan: Return error on register_netdevice_notifier() failure 2024-07-25 11:53 ` Eric Dumazet @ 2024-07-25 14:05 ` Shigeru Yoshida 0 siblings, 0 replies; 6+ messages in thread From: Shigeru Yoshida @ 2024-07-25 14:05 UTC (permalink / raw) To: pabeni, edumazet; +Cc: davem, kuba, netdev, linux-kernel Hi Eric, Paolo, On Thu, 25 Jul 2024 13:53:27 +0200, Eric Dumazet wrote: > On Thu, Jul 25, 2024 at 12:13 PM Paolo Abeni <pabeni@redhat.com> wrote: >> >> >> >> On 7/25/24 11:44, Eric Dumazet wrote: >> > On Wed, Jul 24, 2024 at 3:56 PM Shigeru Yoshida <syoshida@redhat.com> wrote: >> >> >> >> register_netdevice_notifier() may fail, but macvlan_init_module() does >> >> not handle the failure. Handle the failure by returning an error. >> > >> > How could this fail exactly ? Please provide details, because I do not >> > think it can. >> >> Yup, it looks like the registration can't fail for macvlan. >> >> It's better to avoid adding unneeded checks, to reduce noise on the >> tree, keep stable backport easy, etc. Thank you for your comments, and it's my bad, sorry. I happened to look at macvlan's code and found this. I compared this with others like macvtap and I assumed that registering for notifier needs error handling without checking carefully. > Shigeru, you could send a debug patch when net-next reopens next week, > so that we do not get another attempt > on fixing a non-existent bug. I think we don't need this DEBUG_NET_WARN_ON_ONCE() because it makes the source code a little messy. Next time I find something, I'll be more careful. Thanks, Shigeru > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 24298a33e0e94851ebf9c704c723f25ac7bf5eec..0803fcf8df4c56ede10597c862288c7aa887160e > 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -1849,7 +1849,8 @@ static int __init macvlan_init_module(void) > { > int err; > > - register_netdevice_notifier(&macvlan_notifier_block); > + err = register_netdevice_notifier(&macvlan_notifier_block); > + DEBUG_NET_WARN_ON_ONCE(err < 0); > > err = macvlan_link_register(&macvlan_link_ops); > if (err < 0) > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-25 14:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-24 13:56 [PATCH net] macvlan: Return error on register_netdevice_notifier() failure Shigeru Yoshida 2024-07-24 18:40 ` Simon Horman 2024-07-25 9:44 ` Eric Dumazet 2024-07-25 10:13 ` Paolo Abeni 2024-07-25 11:53 ` Eric Dumazet 2024-07-25 14:05 ` Shigeru Yoshida
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).