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

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(a)redhat.com&gt; > --- > 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 :-)