On Fri, 10 Mar 2006 10:41:51 +0000
Baruch Even <[EMAIL PROTECTED]> wrote:
> Herbert Xu wrote:
> > Baruch Even <[EMAIL PROTECTED]> wrote:
> >
> >>+ case NETDEV_UNREGISTER:
> >> case NETDEV_GOING_DOWN:
> >> case NETDEV_DOWN:
> >> /* Find every socket on this device and kill it. */
> >
> >
> > This brings up the question as to why we need to flush it on
> > NETDEV_GOING_DOWN and NETDEV_DOWN as well. If it's possible
> > for things to get added after the flush then isn't it pointless
> > to flush there?
>
> It's the first time I've looked at this code and I'm not completely sure
> I understand the whole state machine for net devices, which is why I
> opted to do the simplest thing that might work. Someone more versed in
> this code can do better.
>
> I've taken the time to survey the other protocols/devices that handle
> these events and it does seem like there is a complete seperation
> between the actions of these events (which makes sense otherwise why
> have different events).
>
> It does look like it should be sufficient to remove all sockets only in
> the unregister case but I'm not sure why we should also keep those
> sockets when a device was taken down?
>
> Baruch
Yuck.. If pppoe was written correctly, it should only need to handle
NETDEV_DOWN.
We always bring the device down before unregistering it. And pppoe_connect
doesn't allow new connections.
For safety:
if (!(dev->flags & IFF_UP))
goto err_put;
should be replaced with
if (!netif_running(dev))
goto err_put;
or for more complete SMP safety, it needs to use rtnl_lock() and
call __dev_get_by_name()
This code would be better if it used standard hlist type from list.h and
RCU could easily replace the reader/writer locking.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
