Re: [libvirt] [PATCH v2 2/4] virnetdev: Introduce virNetDevGetLinkInfo

On 06/05/2014 06:39 PM, Michal Privoznik wrote: > + > + /* Workaround broken kernel API. If the link is unplugged then > + * depending on the NIC driver, link speed can be reported as -1. > + * However, the value is printed out as unsigned integer instead of > + * signed one. Terrifying but true. */ > + *speed = (int) tmp_speed == -1 ? 0 : tmp_speed; It appears the problem isn't quite so simple. On my system (Fedora 20, with a built-in realtek NIC, an Intel 82576-based SRIOV NIC, two bonds, a bridge device, a macvtap device, and several taps, when I look at /sys/class/net/$ifname/speed, I get the following results: (yes, I realized later on that the support you put into libvirt only reports link state for ethernets. Still it may make sense to report one or both items for some other types). * ANY device when ifconfiged down - attempts to read $ifname/speed return EINVAL. In addition, when *UP*: * lo/speed - EINVAL (note that lo self-identifies as type='ethernet' in netcf for some reason) * bridge - EINVAL (operstate seems to always be "down" for bridges, even if the bridge ifflags show UP|RUNNING.) * tap device - "10" * macvtap (physdev is Realtek) - physdev connected - "1000" - physdev disconnected - "10" (same thing reported by physdev) (about operstate - for a macvtap device, if the physdev is disconnected, the macvtap operstate is "lowerlayerdown", *not* "down") * bond - "4294967295" (if the physdevs (2 VFs) are disconnected) * bond - "1000" (2 physdevs connected) (interesting note - when I looked at the speed of bond0 (tying together two sriov VFs each with speed = 1000 using mode="802.3ad"), it showed "1000". At least one time when I ifconfiged both of the VFs down and back up, bond0/speed became "2000". I later could not reproduce this behavior.) (operstate: bonds with mode="activebackup" always show status as "down" and speed as "4294967295", but a bond with mode="802.3ad" properly reflects the physdev state) * RealTek ethernet - connected - "1000" - disconnected - "10" (operstate does show "down") * sriov-PF (Intel 82576) - up/disconnected - "65535" - up/connected - "1000" * sriov-VF (Intel 82576) - up/disconnected - "4294967295" - up/connected - "1000"

So: 1) we need to allow for an EINVAL error, and report the speed as "0" in that case rather than logging an error. 2) just checking for ((unsigned int)-1) isn't good enough - notice that at least one driver (igb) returns 65535 when disconnected, and another returns "10" when disconnected. I think if the operstate is "down", then speed should unconditionally be 0. 3) we should probably report what we can for other interface types. In particular, the status for bond interfaces is useful in at least some modes of operation, and macvtap interfaces seem to reliably reflect the link state of their physdev. And maybe we should even detect bridges and report "unknown" or "indeterminate" or something as the state rather than "down".