[libvirt] [PATCH] virNetDevGetLinkInfo: Don't report link speed if NIC's not up

[Laine Stump]

On 06/13/2014 12:34 PM, Michal Privoznik wrote:
> The kernel's more broken than one would think. Various drivers report
> various (usually spurious) values if the interface is in other state
> than 'up' . While on some we experience -EINVAL when read()-ing the
> speed sysfs file, with other drivers we might get anything from 0 to
> UINT_MAX. If that's the case it's better to not report link speed.
> Well, the interface is not up anyway.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/util/virnetdev.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 6f3a202..a551f98 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1875,6 +1875,16 @@ virNetDevGetLinkInfo(const char *ifname,
>  
>      lnk->state = tmp_state;
>  
> +    /* Shortcut to avoid some kernel issues. If link is not up several drivers
> +     * report several misleading values. While igb reports 65535, realtek goes
> +     * with 10. To avoid muddying XML with insane values, don't report link
> +     * speed if that's the case. */
> +    if (lnk->state != VIR_INTERFACE_STATE_UP) {
> +        lnk->speed = 0;
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
>      VIR_FREE(path);
>      VIR_FREE(buf);
>  
> @@ -1901,11 +1911,7 @@ virNetDevGetLinkInfo(const char *ifname,
>          goto cleanup;
>      }
>  
> -    /* 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. */
> -    lnk->speed = (int) tmp_speed == -1 ? 0 : tmp_speed;
> +    lnk->speed = tmp_speed;
>  
>      ret = 0;
>   cleanup:

ACK from me. Might want to give a short time for differing opinions (or
just push this and change as required up until the next release :-)