Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): <interface type='ethernet' name='eth0'> <start mode='none'/> <mac address='aa:bb:cc:dd:ee:ff'/> <link speed='1000' state='up'/> <mtu size='1492'/> ... </interface> Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: "unknown", "notpresent", "down", "lowerlayerdown","testing", "dormant", "up"). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/schemas/basictypes.rng | 25 ++++++++++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 +++++++++++++++++++++++++ src/conf/device_conf.h | 27 ++++++++++- src/conf/interface_conf.c | 11 ++++- src/conf/interface_conf.h | 2 + src/libvirt_private.syms | 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml | 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 34ef613..5fe3a97 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -397,4 +397,29 @@ </optional> </define> + <define name="link-speed-state"> + <optional> + <element name="link"> + <optional> + <attribute name="speed"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="state"> + <choice> + <value>unknown</value> + <value>notpresent</value> + <value>down</value> + <value>lowerlayerdown</value> + <value>testing</value> + <value>dormant</value> + <value>up</value> + </choice> + </attribute> + </optional> + </element> + </optional> + </define> + </grammar> diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 3984b63..8e2218d 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -41,6 +41,7 @@ <attribute name="address"><ref name="macAddr"/></attribute> </element> </optional> + <ref name="link-speed-state"/> <!-- FIXME: Allow (some) ethtool options --> </define> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 317fdf2..86157e6 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -38,6 +38,13 @@ VIR_ENUM_IMPL(virDeviceAddressPCIMulti, "on", "off") +VIR_ENUM_IMPL(virInterfaceState, + VIR_INTERFACE_STATE_LAST, + "" /* value of zero means no state */, + "unknown", "notpresent", + "down", "lowerlayerdown", + "testing", "dormant", "up") + int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr) { /* PCI bus has 32 slots and 8 functions per slot */ @@ -142,3 +149,58 @@ virDevicePCIAddressEqual(virDevicePCIAddress *addr1, } return false; } + +int +virInterfaceLinkParseXML(xmlNodePtr node, + virInterfaceLinkPtr lnk) +{ + int ret = -1; + char *stateStr, *speedStr; + int state; + + stateStr = virXMLPropString(node, "state"); + speedStr = virXMLPropString(node, "speed"); + + if (stateStr) { + if ((state = virInterfaceStateTypeFromString(stateStr)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown link state: %s"), + stateStr); + goto cleanup; + } + lnk->state = state; + } + + if (speedStr && + virStrToLong_ul(speedStr, NULL, 10, &lnk->speed) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unable to parse link speed: %s"), + speedStr); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(stateStr); + VIR_FREE(speedStr); + return ret; +} + +int +virInterfaceLinkFormat(virBufferPtr buf, + const virInterfaceLink *lnk) +{ + if (!lnk->speed && !lnk->state) { + /* If there's nothing to format, return early. */ + return 0; + } + + virBufferAddLit(buf, "<link"); + if (lnk->speed) + virBufferAsprintf(buf, " speed='%lu'", lnk->speed); + if (lnk->state) + virBufferAsprintf(buf, " state='%s'", + virInterfaceStateTypeToString(lnk->state)); + virBufferAddLit(buf, "/>\n"); + return 0; +} diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index d66afd2..16500db 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -40,6 +40,21 @@ typedef enum { VIR_DEVICE_ADDRESS_PCI_MULTI_LAST } virDeviceAddressPCIMulti; +VIR_ENUM_DECL(virDeviceAddressPCIMulti) + +typedef enum { + VIR_INTERFACE_STATE_UNKNOWN = 1, + VIR_INTERFACE_STATE_NOT_PRESENT, + VIR_INTERFACE_STATE_DOWN, + VIR_INTERFACE_STATE_LOWER_LAYER_DOWN, + VIR_INTERFACE_STATE_TESTING, + VIR_INTERFACE_STATE_DORMANT, + VIR_INTERFACE_STATE_UP, + VIR_INTERFACE_STATE_LAST +} virInterfaceState; + +VIR_ENUM_DECL(virInterfaceState) + typedef struct _virDevicePCIAddress virDevicePCIAddress; typedef virDevicePCIAddress *virDevicePCIAddressPtr; struct _virDevicePCIAddress { @@ -50,6 +65,13 @@ struct _virDevicePCIAddress { int multi; /* enum virDomainDeviceAddressPCIMulti */ }; +typedef struct _virInterfaceLink virInterfaceLink; +typedef virInterfaceLink *virInterfaceLinkPtr; +struct _virInterfaceLink { + virInterfaceState state; /* link state */ + unsigned long speed; /* link speed in Mb per second */ +}; + int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr); int virDevicePCIAddressParseXML(xmlNodePtr node, @@ -62,7 +84,10 @@ int virDevicePCIAddressFormat(virBufferPtr buf, bool virDevicePCIAddressEqual(virDevicePCIAddress *addr1, virDevicePCIAddress *addr2); +int virInterfaceLinkParseXML(xmlNodePtr node, + virInterfaceLinkPtr lnk); -VIR_ENUM_DECL(virDeviceAddressPCIMulti) +int virInterfaceLinkFormat(virBufferPtr buf, + const virInterfaceLink *lnk); #endif /* __DEVICE_CONF_H__ */ diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 1f67446..2b3f699 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -705,12 +705,19 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) } def->type = type; switch (type) { - case VIR_INTERFACE_TYPE_ETHERNET: + case VIR_INTERFACE_TYPE_ETHERNET: { + xmlNodePtr lnk; + if (virInterfaceDefParseName(def, ctxt) < 0) goto error; tmp = virXPathString("string(./mac/@address)", ctxt); if (tmp != NULL) def->mac = tmp; + + lnk = virXPathNode("./link", ctxt); + if (lnk && virInterfaceLinkParseXML(lnk, &def->lnk) < 0) + goto error; + if (parentIfType == VIR_INTERFACE_TYPE_LAST) { /* only recognize these in toplevel bond interfaces */ if (virInterfaceDefParseStartMode(def, ctxt) < 0) @@ -721,6 +728,7 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) goto error; } break; + } case VIR_INTERFACE_TYPE_BRIDGE: { xmlNodePtr bridge; @@ -1088,6 +1096,7 @@ virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def) virInterfaceStartmodeDefFormat(buf, def->startmode); if (def->mac != NULL) virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac); + virInterfaceLinkFormat(buf, &def->lnk); if (def->mtu != 0) virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); virInterfaceProtocolDefFormat(buf, def); diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index b3c92b2..94c18ef 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -31,6 +31,7 @@ # include "internal.h" # include "virutil.h" # include "virthread.h" +# include "device_conf.h" /* There is currently 3 types of interfaces */ @@ -146,6 +147,7 @@ struct _virInterfaceDef { char *name; /* interface name */ unsigned int mtu; /* maximum transmit size in byte */ char *mac; /* MAC address */ + virInterfaceLink lnk; /* interface link info */ virInterfaceStartMode startmode; /* how it is started */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4c1f61c..8c184c9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -83,6 +83,10 @@ virDevicePCIAddressEqual; virDevicePCIAddressFormat; virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; +virInterfaceLinkFormat; +virInterfaceLinkParseXML; +virInterfaceStateTypeFromString; +virInterfaceStateTypeToString; # conf/domain_addr.h diff --git a/tests/interfaceschemadata/bridge-no-address.xml b/tests/interfaceschemadata/bridge-no-address.xml index 7757534..68b8c94 100644 --- a/tests/interfaceschemadata/bridge-no-address.xml +++ b/tests/interfaceschemadata/bridge-no-address.xml @@ -4,6 +4,7 @@ <bridge stp='off'> <interface type='ethernet' name='eth0'> <mac address='ab:bb:cc:dd:ee:ff'/> + <link speed='1000' state='up'/> </interface> <interface type='ethernet' name='eth1'> </interface> diff --git a/tests/interfaceschemadata/bridge.xml b/tests/interfaceschemadata/bridge.xml index 2535edf..c865116 100644 --- a/tests/interfaceschemadata/bridge.xml +++ b/tests/interfaceschemadata/bridge.xml @@ -7,6 +7,7 @@ <bridge stp='off' delay='0.01'> <interface type='ethernet' name='eth0'> <mac address='ab:bb:cc:dd:ee:ff'/> + <link speed='10'/> </interface> <interface type='ethernet' name='eth1'> </interface> diff --git a/tests/interfaceschemadata/ethernet-dhcp.xml b/tests/interfaceschemadata/ethernet-dhcp.xml index fe969df..c124372 100644 --- a/tests/interfaceschemadata/ethernet-dhcp.xml +++ b/tests/interfaceschemadata/ethernet-dhcp.xml @@ -1,6 +1,7 @@ <interface type='ethernet' name='eth0'> <start mode='none'/> <mac address='aa:bb:cc:dd:ee:ff'/> + <link state='down'/> <mtu size='1492'/> <protocol family='ipv4'> <dhcp peerdns='no'/> -- 2.0.0

In previous commit the interface XML is prepared for exporting information on NIC link speed and state. This commit implement actual logic for getting such info and writing it into XML. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Notes: For the kernel issue I've posted patch here: http://patchwork.ozlabs.org/patch/354928/ But it wasn't accepted as developers are afraid of breaking backward compatibility. Which is weird as in 2.6.X the kernel was reporting -1 instead of unsigned -1. src/interface/interface_backend_udev.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index c5353ea..80ba93e 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -1036,6 +1036,8 @@ udevGetIfaceDef(struct udev *udev, const char *name) const char *mtu_str; char *vlan_parent_dev = NULL; const char *devtype; + const char *operstate; + const char *link_speed; /* Allocate our interface definition structure */ if (VIR_ALLOC(ifacedef) < 0) @@ -1059,6 +1061,31 @@ udevGetIfaceDef(struct udev *udev, const char *name) udev_device_get_sysattr_value(dev, "address")) < 0) goto error; + operstate = udev_device_get_sysattr_value(dev, "operstate"); + if ((ifacedef->lnk.state = virInterfaceStateTypeFromString(operstate)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse operstate: %s"), + operstate); + goto error; + } + + link_speed = udev_device_get_sysattr_value(dev, "speed"); + if (link_speed) { + if (virStrToLong_ul(link_speed, NULL, 10, &ifacedef->lnk.speed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse speed: %s"), + link_speed); + goto error; + } + + /* 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. */ + if ((int) ifacedef->lnk.speed == -1) + ifacedef->lnk.speed = 0; + } + /* MTU */ mtu_str = udev_device_get_sysattr_value(dev, "mtu"); if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { -- 2.0.0

While exposing the info under <interface/> in previous patches works, it may work only in cases where interface is configured on the host (especially if netcf backend is in charge). However, orchestrating application may want to know the link state and speed even in that case. That's why we ought to expose this in nodedev XML too: virsh # nodedev-dumpxml net_eth0_f0_de_f1_2b_1b_f3 <device> <name>net_eth0_f0_de_f1_2b_1b_f3</name> <path>/sys/devices/pci0000:00/0000:00:19.0/net/eth0</path> <parent>pci_0000_00_19_0</parent> <capability type='net'> <interface>eth0</interface> <address>f0:de:f1:2b:1b:f3</address> <link speed='1000' state='up'/> <capability type='80203'/> </capability> </device> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Notes: Unfortunately, udev doesn't register link changes, so ripping the cable out won't call our udev event callback. If it were so, things could be much more simpler... docs/formatnode.html.in | 6 ++++ docs/schemas/nodedev.rng | 1 + src/conf/node_device_conf.c | 7 +++- src/conf/node_device_conf.h | 2 ++ src/node_device/node_device_driver.c | 27 ++++++++++++--- src/node_device/node_device_hal.c | 9 +++++ src/node_device/node_device_hal.h | 1 + src/node_device/node_device_udev.c | 65 ++++++++++++++++++++++++++++++++++++ src/node_device/node_device_udev.h | 4 +++ 9 files changed, 116 insertions(+), 6 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index b424c96..b27b6bd 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -154,6 +154,12 @@ <dd>The interface name tied to this device.</dd> <dt><code>address</code></dt> <dd>If present, the MAC address of the device.</dd> + <dt><code>link</code></dt> + <dd>Optional to reflect the status of the link. It has + two optional attributes: <code>speed</code> in Mbit per + second and <code>state</code> to tell the state of the + link. + </dd> <dt><code>capability</code></dt> <dd>A network protocol exposed by the device, where the attribute <code>type</code> can be "80203" for IEEE diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 81ab4d4..8c70536 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -233,6 +233,7 @@ <ref name='mac'/> </element> </optional> + <ref name="link-speed-state"/> <zeroOrMore> <ref name='subcapnet'/> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index e65b5e4..8405a25 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -386,6 +386,7 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) if (data->net.address) virBufferEscapeString(&buf, "<address>%s</address>\n", data->net.address); + virInterfaceLinkFormat(&buf, &data->net.lnk); if (data->net.subtype != VIR_NODE_DEV_CAP_NET_LAST) { const char *subtyp = virNodeDevNetCapTypeToString(data->net.subtype); @@ -837,7 +838,7 @@ virNodeDevCapNetParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, union _virNodeDevCapData *data) { - xmlNodePtr orignode; + xmlNodePtr orignode, lnk; int ret = -1; char *tmp; @@ -869,6 +870,10 @@ virNodeDevCapNetParseXML(xmlXPathContextPtr ctxt, data->net.subtype = val; } + lnk = virXPathNode("./link", ctxt); + if (lnk && virInterfaceLinkParseXML(lnk, &data->net.lnk) < 0) + goto out; + ret = 0; out: ctxt->node = orignode; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 50e6805..462bfa4 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -29,6 +29,7 @@ # include "virutil.h" # include "virthread.h" # include "virpci.h" +# include "device_conf.h" # include <libxml/tree.h> @@ -135,6 +136,7 @@ struct _virNodeDevCapsDef { char *address; unsigned int address_len; char *ifname; + virInterfaceLink lnk; virNodeDevNetCapType subtype; /* LAST -> no subtype */ } net; struct { diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 6906463..f9f0db4 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -36,21 +36,37 @@ #include "virstring.h" #include "node_device_conf.h" #include "node_device_hal.h" +#include "node_device_udev.h" #include "node_device_driver.h" #include "virutil.h" #include "viraccessapicheck.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV +static int +update_link(virNodeDeviceDefPtr def) +{ +#ifdef WITH_UDEV + return udevUpdateLink(def); +#endif +#ifdef WITH_HAL + return halUpdateLink(def); +#endif +} + static int update_caps(virNodeDeviceObjPtr dev) { virNodeDevCapsDefPtr cap = dev->def->caps; while (cap) { - /* The only caps that currently need updating are FC related. */ - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { - detect_scsi_host_caps(&dev->def->caps->data); - } + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + detect_scsi_host_caps(&dev->def->caps->data) < 0) + return -1; + + if (cap->type == VIR_NODE_DEV_CAP_NET && + update_link(dev->def) < 0) + return -1; + cap = cap->next; } @@ -315,7 +331,8 @@ nodeDeviceGetXMLDesc(virNodeDevicePtr dev, goto cleanup; update_driver_name(obj); - update_caps(obj); + if (update_caps(obj) < 0) + goto cleanup; ret = virNodeDeviceDefFormat(obj->def); diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 8656b5d..e630bef 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -838,3 +838,12 @@ halNodeRegister(void) return -1; return virRegisterStateDriver(&halStateDriver); } + +int +halUpdateLink(virNodeDeviceDefPtr def ATTRIBUTE_UNUSED) +{ + /* Implement me */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s" + _("This function is not implemented yet")); + return -1; +} diff --git a/src/node_device/node_device_hal.h b/src/node_device/node_device_hal.h index 7226672..9fc92c0 100644 --- a/src/node_device/node_device_hal.h +++ b/src/node_device/node_device_hal.h @@ -22,4 +22,5 @@ #ifndef __VIR_NODE_DEVICE_HAL_H__ # define __VIR_NODE_DEVICE_HAL_H__ +int halUpdateLink(virNodeDeviceDefPtr def); #endif /* __VIR_NODE_DEVICE_HAL_H__ */ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 9a951d9..2a5b586 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -631,6 +631,44 @@ static int udevProcessUSBInterface(struct udev_device *device, } +static int +udevProcessNetworkInterfaceOperstate(struct udev_device *device, + virNodeDeviceDefPtr def) +{ + union _virNodeDevCapData *data = &def->caps->data; + const char *operstate; + const char *link_speed; + int ret = -1; + + operstate = udev_device_get_sysattr_value(device, "operstate"); + if ((data->net.lnk.state = virInterfaceStateTypeFromString(operstate)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse operstate: %s"), + operstate); + goto out; + } + + link_speed = udev_device_get_sysattr_value(device, "speed"); + if (link_speed) { + if (virStrToLong_ul(link_speed, NULL, 10, &data->net.lnk.speed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse speed: %s"), + link_speed); + goto out; + } + /* 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. */ + if ((int) data->net.lnk.speed == -1) + data->net.lnk.speed = 0; + } + + ret = 0; + out: + return ret; +} + static int udevProcessNetworkInterface(struct udev_device *device, virNodeDeviceDefPtr def) { @@ -667,6 +705,9 @@ static int udevProcessNetworkInterface(struct udev_device *device, goto out; } + if (udevProcessNetworkInterfaceOperstate(device, def) < 0) + goto out; + ret = 0; out: @@ -1837,3 +1878,27 @@ int udevNodeRegister(void) return virRegisterStateDriver(&udevStateDriver); } + +int +udevUpdateLink(virNodeDeviceDefPtr def) +{ + int ret = -1; + struct udev *udev = NULL; + struct udev_device *device = NULL; + + udev = udev_monitor_get_udev(DRV_STATE_UDEV_MONITOR(driverState)); + + device = udev_device_new_from_syspath(udev, def->sysfs_path); + if (!device) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to update link status on %s"), + def->sysfs_path); + goto cleanup; + } + + ret = udevProcessNetworkInterfaceOperstate(device, def); + cleanup: + if (device) + udev_device_unref(device); + return ret; +} diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_device_udev.h index 47eeb1f..5e50d66 100644 --- a/src/node_device/node_device_udev.h +++ b/src/node_device/node_device_udev.h @@ -23,6 +23,8 @@ #include <libudev.h> #include <stdint.h> +#include "node_device_conf.h" + typedef struct _udevPrivate udevPrivate; #define SYSFS_DATA_SIZE 4096 @@ -32,3 +34,5 @@ typedef struct _udevPrivate udevPrivate; #define PROPERTY_FOUND 0 #define PROPERTY_MISSING 1 #define PROPERTY_ERROR -1 + +int udevUpdateLink(virNodeDeviceDefPtr def); -- 2.0.0