virtio-net: Fix operstate for virtio when no VIRTIO_NET_F_STATUS
Subsystems (3)
PATCH v2 1/1 virtio-net: Fix operstate for virtio when no VIRTIO_NET_F_STATUS
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 callnetif_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.
---
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(-)
drivers/net/virtio_net.c +1 -1
@@ -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 ;
Signature · 1 line
2.14.1
Quoted 18 lines · Jay Vosburgh
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 tonetif_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 supportVIRTIO_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_offfor both the "with" and "without"VIRTIO_NET_F_STATUScases, and then the existing call tonetif_carrier_onfor the "without" case will cause an operstate transition.
Quoted 4 lines
Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>Cc Ben Hutchings <[email protected]>Signed-off-by Jay Vosburgh <[email protected]>
Arguably userspace apps should learn to treat UNKNOWN as UP. But on the balance this seems more likely to fix apps than to break any, so
Quoted 34 lines
---
I considered resolving this by changing
linkwatch_init_devto
unconditionally callrfc2863_policy, as that would always set operstate for all interfaces.This would not have any impact on most cases (as most drivers
callnetif_carrier_offduring 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 thedev_base_lockfor 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(-)drivers/net/virtio_net.c +1
-1@@ -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 ;Signature · 1 line
2.14.1
From: Jay Vosburgh <[email protected]>
Date: Thu, 22 Mar 2018 14:42:41 +0000
Quoted 18 lines
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 tonetif_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 supportVIRTIO_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_offfor both the "with" and "without"VIRTIO_NET_F_STATUScases, and then the existing call tonetif_carrier_onfor the "without" case will cause an operstate transition.
Quoted 4 lines
Cc "Michael S. Tsirkin" <[email protected]>Cc Jason Wang <[email protected]>Cc Ben Hutchings <[email protected]>Signed-off-by Jay Vosburgh <[email protected]>
Applied, thank you.