Messages in this thread Patch in this message | | | From | Jeff Moyer <> | Date | Wed, 27 Oct 2004 14:58:04 -0400 | Subject | Re: netpoll_setup questions |
| |
==> Regarding Re: netpoll_setup questions; Matt Mackall <mpm@selenic.com> adds:
mpm> On Wed, Oct 27, 2004 at 11:50:05AM -0400, Jeff Moyer wrote: >> Hi, Matt, >> >> The section of code in the body of this if statement: >> >> if (!(ndev->flags & IFF_UP)) { >> >> is a bit broken. First, upon discussion with jgarzik, it seems we >> should not check for IFF_UP, but instead do netif_running.
mpm> Well I cribbed it from the nfsroot code, which is perhaps not up on mpm> fashion.
Hmm, I can't seem to find the relevant code. Where is it?
>> However, I'm wondering why we try to force the interface up in the first >> place?
mpm> Uh, so we can send packets before userspace has configured the mpm> network?
Heh. right.
>> Just because we force it up doesn't mean that it will get an IP address.
mpm> I don't expect it does. Usually we have our own IP address from the mpm> command line, but if we don't, we will check if there's an in_dev mpm> connected to the device and if so, look up the device's IP. This is mpm> useful for the modular case where the network is up before we start mpm> and we have a handy default for local IP.
Right, but the case in question is the one where we don't have a local IP specified:
if (!np->local_ip) { rcu_read_lock(); in_dev = __in_dev_get(ndev);
if (!in_dev) { rcu_read_unlock(); printk(KERN_ERR "%s: no IP address for %s, aborting\n", np->name, np->dev_name); goto release; }
np->local_ip = ntohl(in_dev->ifa_list->ifa_local); rcu_read_unlock(); printk(KERN_INFO "%s: local IP %d.%d.%d.%d\n", np->name, HIPQUAD(np->local_ip)); }
>> And, in the case where it doesn't, you will get an oops further on when >> dereferencing the ifa_list.
mpm> Does this actually happen? I'm checking in_dev for null already, but mpm> perhaps I need to check ifa_list as well.
Yes, that should do it.
>> So, why does this section of code exist at all? If it has a good >> purpose, can we replace it with a call to ndev->open?
mpm> Maybe. We should probably revisit the nfsroot code as well.
Right, just point me at it.
Here is a patch which should work. I'll test today.
-Jeff
--- linux-2.6.9/net/core/netpoll.c~ 2004-10-21 17:49:10.000000000 -0400 +++ linux-2.6.9/net/core/netpoll.c 2004-10-27 14:53:57.492149880 -0400 @@ -571,7 +571,7 @@ int netpoll_setup(struct netpoll *np) goto release; } - if (!(ndev->flags & IFF_UP)) { + if (!netif_running(ndev)) { unsigned short oflags; unsigned long atmost, atleast; @@ -617,7 +617,7 @@ int netpoll_setup(struct netpoll *np) rcu_read_lock(); in_dev = __in_dev_get(ndev); - if (!in_dev) { + if (!in_dev || !in_dev->ifa_list) { rcu_read_unlock(); printk(KERN_ERR "%s: no IP address for %s, aborting\n", np->name, np->dev_name); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |