netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "David S. Miller" <davem@redhat.com>
To: Stephen Hemminger <shemminger@osdl.org>
Cc: netdev@oss.sgi.com, chas@cmf.nrl.navy.mil
Subject: Re: [PATCH] (1/5) replay netdev notifier events on registration
Date: Thu, 15 Jan 2004 00:42:55 -0800	[thread overview]
Message-ID: <20040115004255.62dc8b95.davem@redhat.com> (raw)
In-Reply-To: <20040114164416.753a62fc.shemminger@osdl.org>

On Wed, 14 Jan 2004 16:44:16 -0800
Stephen Hemminger <shemminger@osdl.org> wrote:

> Possible problems:
> qeth: s390 driver -- bug, code is narcissistic and thinks it only gets
> 	notified about it's own devices.
> 
> atm/mpc: only looks for "lec" devices, don't know if they could exist
> 	before it starts.

These are both hard errors and potential bogus pointer derefences, they both
assume the type of dev->priv.  The atm/mpc case has a netdev->name==NULL
test which is a funny relic :-)

Both these cases ought to be fixed.  However, the atm/mpc case poses an issue,
how to identify "my" devices?  We've established that the textual name is
basically arbitrary and not a reliable key.  Currently I see only two
possible reliable solutions, but I like neither of them:

1) Device driver doing this needs to keep own list of net devices it
   has created.  Then it's notifiier code does something like:

	if (!find_in_mpoa_devlist(dev))
		return NOTIFY_DONE;

2) Add a type cookie or similar to the generic netdev, only devices
   which need to identify themselves in these generic kind of cases
   add identifier values there, so currently that would be MPOA and
   QETH, then the code goes:

	if (dev->type_cookie == NETDEV_TYPE_MPOA)
		return NOTIFY_DONE;

But as stated I think both of these ideas absolutely stink.

... wait...

Ok, I have an idea, consider this.  We add a netdev->notifier()
method.  We create a new routine to net/core/dev.c:

static void run_netdev_notifiers(int event, struct net_device *dev)
{
	notifier_call_chain(&netdev_chain, event, dev);

	if (dev->notifier)
		dev->notifier(dev, event);
}

Then replace all the notifier_call_chain(&netdev_chain, ...) calls
in net/core/dev.c with invocations of run_netdev_notifiers().

I believe we can (and thus should) add an ASSERT_RTNL() to this new
run_netdev_notifiers() functions, although I'm not %100 sure.

What do you think Stephen?

> Unrelated problems:
> ddp: registers for notifier before it is initialized

Just moving it down to the end of atalk_init() should fix this?
Actually, I don't really see any potential problem here.

> ipmr: no locking for add/delete

Not a problem, RTNL semaphore is held.

> ipfwadm: no module owner on /proc interface

Please elaborate.  I don't see the ipfwadm netdev event notifier
messing with procfs stuff, or is this happen at a level or two of
indirection somewhere else?

> The following are okay:

Thanks for the audit.

  reply	other threads:[~2004-01-15  8:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-01-13 18:58 [PATCH] (1/5) replay netdev notifier events on registration Stephen Hemminger
2004-01-14  0:36 ` David S. Miller
2004-01-15  0:40   ` [PATCH] decnet initialization race Stephen Hemminger
2004-01-15  8:45     ` David S. Miller
2004-01-15  0:43   ` [PATH] atm/clip device discovery on init not needed Stephen Hemminger
2004-01-15  8:44     ` David S. Miller
2004-01-15 22:59     ` Stephen Hemminger
2004-01-15 23:00       ` David S. Miller
2004-01-15  0:44   ` [PATCH] (1/5) replay netdev notifier events on registration Stephen Hemminger
2004-01-15  8:42     ` David S. Miller [this message]
2004-01-15 18:16       ` Stephen Hemminger
2004-01-15 19:51         ` David S. Miller
2004-01-16  3:19       ` chas williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040115004255.62dc8b95.davem@redhat.com \
    --to=davem@redhat.com \
    --cc=chas@cmf.nrl.navy.mil \
    --cc=netdev@oss.sgi.com \
    --cc=shemminger@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).