From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58816 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753802AbeCVLwK (ORCPT ); Thu, 22 Mar 2018 07:52:10 -0400 Date: Thu, 22 Mar 2018 13:52:08 +0200 From: "Michael S. Tsirkin" To: Jay Vosburgh Cc: netdev@vger.kernel.org, Jason Wang , David Miller , Ben Hutchings Subject: Re: [PATCH net] virtio-net: Fix operstate for virtio when no VIRTIO_NET_F_STATUS Message-ID: <20180322134901-mutt-send-email-mst@kernel.org> References: <12441.1521709552@nyx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <12441.1521709552@nyx> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Mar 22, 2018 at 09:05:52AM +0000, Jay Vosburgh wrote: > The operstate update logic will leave an interface in the > default UNKNOWN operstate if the interface carrier state never changes > from the default carrier up state set at creation. This includes the > case of an explicit call to netif_carrier_on, as the carrier on to on > transition has no effect on operstate. > > This affects virtio-net for the case that the virtio peer does > not support VIRTIO_NET_F_STATUS (the feature that provides carrier state > updates). Without this feature, the virtio specification states that > "the link should be assumed active," so, logically, the operstate should > be UP instead of UNKNOWN. This has impact on user space applications > that use the operstate to make availability decisions for the interface. > > Resolve this by changing the virtio probe logic slightly to call > netif_carrier_off for both the "with" and "without" VIRTIO_NET_F_STATUS > cases, and then the existing call to netif_carrier_on for the "without" > case will cause an operstate transition. > > Cc: "Michael S. Tsirkin" > Cc: Jason Wang > Cc: Ben Hutchings > Fixes: 167c25e4c550 ("virtio-net: init link state correctly") I'd say that's an abuse of this notation. openstate was UNKNOWN even before that fix. > Signed-off-by: Jay Vosburgh Acked-by: Michael S. Tsirkin > --- > > I considered resolving this by changing linkwatch_init_dev to > unconditionally call rfc2863_policy, as that would always set operstate > for all interfaces. > > This would not have any impact on most cases (as most drivers > call netif_carrier_off during probe), except for the loopback device, > which currently has an operstate of UNKNOWN (because it never does any > carrier state transitions). This change would add a round trip on the > dev_base_lock for every loopback device creation, which could have a > negative impact when creating many loopback devices, e.g., when > concurrently creating large numbers of containers. > > > drivers/net/virtio_net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 23374603e4d9..7b187ec7411e 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -2857,8 +2857,8 @@ static int virtnet_probe(struct virtio_device *vdev) > > /* Assume link up if device can't report link status, > otherwise get link status from config. */ > + netif_carrier_off(dev); > if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) { > - netif_carrier_off(dev); > schedule_work(&vi->config_work); > } else { > vi->status = VIRTIO_NET_S_LINK_UP; > -- > 2.14.1