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

[Michal Privoznik]

On 12.06.2014 15:13, Laine Stump wrote:
> 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"

Wow, so much analysis.

>
> 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".

[I was going to start discussion here, but meanwhile it started on the 
patch I've sent "virNetDevGetLinkInfo: Don't report link speed if NIC's 
down" so I'll reply there]

Michal