| Message ID | 200903131017.11572.rusty@rustcorp.com.au |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 13 Mar 2009 10:17:11 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote: > On Friday 13 March 2009 08:09:49 Rusty Russell wrote: > > On Thursday 12 March 2009 23:22:35 David Miller wrote: > > > If the link is always on, you should make that explicit by providing > > > a link state handler, and making sure it always returns true. > > > > "If". We've discussed adding a virtio_net feature to indicate link status, > > which implies that it's *not* always on. > > Actually, I've changed my mind. > > Unlike a device which *has* a carrier which we can't detect, there's no > virtio_net "device" which can turn off link (not kvm/qemu, not lguest) without > the pending VIRTIO_NET_S_LINK feature. > Yes, need that feature, it is really useful for testing. It is about the only reason I hold onto using VMware. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 13, 2009 at 1:47 AM, Rusty Russell <rusty@rustcorp.com.au> wrote: > On Friday 13 March 2009 08:09:49 Rusty Russell wrote: >> On Thursday 12 March 2009 23:22:35 David Miller wrote: >> > If the link is always on, you should make that explicit by providing >> > a link state handler, and making sure it always returns true. >> >> "If". We've discussed adding a virtio_net feature to indicate link status, >> which implies that it's *not* always on. > > Actually, I've changed my mind. > > Unlike a device which *has* a carrier which we can't detect, there's no > virtio_net "device" which can turn off link (not kvm/qemu, not lguest) without > the pending VIRTIO_NET_S_LINK feature. Exactly :) > > Here's the patch for Dave's tree; the q. is do we want to put Pantelis' patch > in 2.6.29 and stable? It would be cool to put my patch in 2.6.29 since it is not intrusive (just a quickfix) and then drop it in favor of the proper carrier detection / control + your patch. Thanks, Pantelis -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Rusty Russell <rusty@rustcorp.com.au> Date: Fri, 13 Mar 2009 10:17:11 +1030 > Subject: virtio_net: set carrier on by default. > > Impact: fix carrier detection, older NetworkManager > > This is actually two fixes: > 1) If the virtio_net device doesn't support carrier, the answer is > "yes". This is because before the status feature there was no way > of turning the link off in any host implementation, and it also helps > (older) NetworkManager versions to see the device. > > 2) We should start with carrier on: virtnet_update_status() does nothing > if the status hasn't changed (ie. doesn't call netif_carrier_on()). > > Reported-by: Pantelis Koukousoulas <pktoss@gmail.com> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> You can set netif_carrier_on() until you are blue in the face, but until you hook up the ethtool link indication operation NetworkManager won't see it. I don't understand what all of this hob-knobbing is about, Pantelis's patch was perfect, appropriate, and should have gone straight in to net-2.6 and probably -stable too. Is this some kind of control issue Rusty? It's the only explanation I can come up with :-)) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -795,7 +795,10 @@ static int virtnet_probe(struct virtio_d goto unregister; } + /* If we have no carrier info, the answer is "always on". */ vi->status = VIRTIO_NET_S_LINK_UP; + netif_carrier_on(vi->dev); + virtnet_update_status(vi); pr_debug("virtnet: registered device %s\n", dev->name);