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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to