From: "D. Herrendoerfer" <d.herrendoerfer(a)herrendoerfer.name&gt; This code adds a netlink event interface to libvirt. It is based upon the event_poll code and makes use of it. An event is generated for each netlink message sent to the libvirt pid. Signed-off-by: D. Herrendoerfer <d.herrendoerfer(a)herrendoerfer.name&gt; --- daemon/libvirtd.c | 8 + src/libvirt_private.syms | 6 + src/util/virnetlink.c | 438 +++++++++++++++++++++++++++++++++++++++++++++- src/util/virnetlink.h | 29 +++ 4 files changed, 476 insertions(+), 5 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b1b542b..ca8074d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -47,6 +47,7 @@ #include "conf.h" #include "memory.h" #include "conf.h" +#include "virnetlink.h" #include "virnetserver.h" #include "threads.h" #include "remote.h" @@ -1598,6 +1599,12 @@ int main(int argc, char **argv) { goto cleanup; } + /* Register the netlink event service */ + if (virNetlinkEventServiceStart() < 0) { + ret = VIR_DAEMON_ERR_NETWORK; + goto cleanup; + } + /* Run event loop. */ virNetServerRun(srv); @@ -1607,6 +1614,7 @@ int main(int argc, char **argv) { 0, "shutdown", NULL); cleanup: + virNetlinkEventServiceStop(); virNetServerProgramFree(remoteProgram); virNetServerProgramFree(qemuProgram); virNetServerClose(srv); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6ad36c..008470e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1258,6 +1258,12 @@ virNetDevVPortProfileOpTypeToString; #virnetlink.h virNetlinkCommand; +virNetlinkEventServiceIsRunning; +virNetlinkEventServiceStop; +virNetlinkEventServiceStart; +virNetlinkEventAddClient; +virNetlinkEventRemoveClient; + # virnetmessage.h diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index d03d171..ec4727e 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -35,7 +35,10 @@ #include <sys/types.h> #include "virnetlink.h" +#include "logging.h" #include "memory.h" +#include "threads.h" +#include "virmacaddr.h" #include "virterror_internal.h" #define VIR_FROM_THIS VIR_FROM_NET @@ -46,6 +49,50 @@ #define NETLINK_ACK_TIMEOUT_S 2 +#if defined(__linux__) && defined(HAVE_LIBNL) +/* State for a single netlink event handle */ +struct virNetlinkEventHandle { + int watch; + virNetlinkEventHandleCallback cb; + void *opaque; + unsigned char macaddr[VIR_MAC_BUFLEN]; + int deleted; +}; + +typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate; +typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr; +struct _virNetlinkEventSrvPrivate { + /*Server*/ + virMutex lock; + int eventwatch; + int netlinkfd; + struct nl_handle *netlinknh; + /*Events*/ + int handled; + size_t handlesCount; + size_t handlesAlloc; + struct virNetlinkEventHandle *handles; +}; + +enum virNetlinkDeleteMode { + VIR_NETLINK_HANDLE_VALID, + VIR_NETLINK_HANDLE_DELETED, +}; + +/* Only have one event loop */ +//static struct virNetlinkEventLoop eventLoop;

+ +/* Unique ID for the next netlink watch to be registered */ +static int nextWatch = 1; + +/* Allocate extra slots for virEventPollHandle/virEventPollTimeout + records in this multiple */ +#define NETLINK_EVENT_ALLOC_EXTENT 10 + +static virNetlinkEventSrvPrivatePtr server = NULL; + +/* Function definitions */ + /** * virNetlinkCommand: * @nlmsg: pointer to netlink message @@ -58,7 +105,6 @@ * Returns 0 on success, -1 on error. In case of error, no response * buffer will be returned. */ -#if defined(__linux__) && defined(HAVE_LIBNL) int virNetlinkCommand(struct nl_msg *nl_msg, unsigned char **respbuf, unsigned int *respbuflen, int nl_pid) @@ -89,7 +135,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, virReportSystemError(errno, "%s", _("cannot connect to netlink socket")); rc = -1; - goto err_exit; + goto error; } nlmsg_set_dst(nl_msg, &nladdr); @@ -101,7 +147,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, virReportSystemError(errno, "%s", _("cannot send to netlink socket")); rc = -1; - goto err_exit; + goto error; } fd = nl_socket_get_fd(nlhandle); @@ -118,7 +164,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, virReportSystemError(ETIMEDOUT, "%s", _("no valid netlink response was received")); rc = -1; - goto err_exit; + goto error; } *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL); @@ -127,7 +173,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, "%s", _("nl_recv failed")); rc = -1; } -err_exit: +error: if (rc == -1) { VIR_FREE(*respbuf); *respbuf = NULL; @@ -138,6 +184,323 @@ err_exit: return rc; } +static void +virNetlinkEventServerLock(virNetlinkEventSrvPrivatePtr driver) { + virMutexLock(&driver->lock); +} + +static void +virNetlinkEventServerUnlock(virNetlinkEventSrvPrivatePtr driver) { + virMutexUnlock(&driver->lock); +} + +static void +virNetlinkEventCallback(int watch, + int fd ATTRIBUTE_UNUSED, + int events ATTRIBUTE_UNUSED, + void *opaque) { + virNetlinkEventSrvPrivatePtr srv = opaque; + unsigned char *msg; + struct sockaddr_nl peer; + struct ucred *creds = NULL; + int i, length; + bool handled = false; + + length = nl_recv(srv->netlinknh, &peer, &msg, &creds); + + if (length == 0) + return; + if (length < 0) { + netlinkError(errno, + "%s", _("nl_recv returned with error")); + return; + } + + virNetlinkEventServerLock(srv); + + VIR_DEBUG("dispatching to max %d clients, called from event watch %d", + (int)srv->handlesCount, watch); + + for (i = 0; i < srv->handlesCount; i++) { + if (srv->handles[i].deleted != VIR_NETLINK_HANDLE_VALID) { + continue; + } + + VIR_DEBUG("dispatching client %d.",i); + + virNetlinkEventHandleCallback cb = srv->handles[i].cb; + void *cpopaque = srv->handles[i].opaque; + (cb)( msg, length, &peer, &handled, cpopaque); + } + + if (!handled) { + VIR_DEBUG("nobody cared.");

+ } + + VIR_FREE(msg); + + virNetlinkEventServerUnlock(srv); +} + +/** + * virNetlinkEventServiceStop: + * + * stop the monitor to receive netlink messages for libvirtd. + * This removes the netlink socket fd from the event handler. + * + * returns -1 if the monitor cannot be unregistered, 0 upon success + */ +int +virNetlinkEventServiceStop(void) { + virNetlinkEventSrvPrivatePtr srv = server; + + VIR_INFO("stopping netlink event service"); + + if (!server) { + return 0; + } + + virNetlinkEventServerLock(srv); + + nl_close(srv->netlinknh); + nl_handle_destroy(srv->netlinknh); + + virEventRemoveHandle(srv->eventwatch); + server=0; + + virNetlinkEventServerUnlock(srv); + + virMutexDestroy(&srv->lock); + + VIR_FREE(srv); + + return 0; +} + +/** + * virNetlinkEventServiceIsRunning: + * + * returns if the netlink event service is running. + * + * returns 'true' if the service is running, 'false' if stopped. + */ +bool +virNetlinkEventServiceIsRunning(void) { + return (server != NULL); +} + +/** + * virNetlinkEventServiceStart: + * + * start a monitor to receive netlink messages for libvirtd. + * This registers a netlink socket with the event interface. + * + * returns -1 if the monitor cannot be registered, 0 upon success + */ +int +virNetlinkEventServiceStart(void) { + virNetlinkEventSrvPrivatePtr srv; + int fd; + int ret = -1; + + if (server) { + return 0; + } + + VIR_INFO("starting netlink event service"); + + if (VIR_ALLOC(srv) < 0) { + virReportOOMError(); + goto error; + } + + /* Init the mutexes */

+ if ( virMutexInit(&srv->lock) < 0) + goto error; + + virNetlinkEventServerLock(srv); + + /* Allocate a new socket and get fd */ + srv->netlinknh = nl_handle_alloc(); + + if (!srv->netlinknh) { + netlinkError(errno, + "%s", _("cannot allocate nlhandle for virNetlinkEvent server")); + goto error_locked; + } + + if (nl_connect(srv->netlinknh, NETLINK_ROUTE) < 0) { + netlinkError(errno, + "%s", _("cannot connect to netlink socket")); + goto error_server; + } + + fd = nl_socket_get_fd(srv->netlinknh); + + if (fd < 0) { + netlinkError(errno, + "%s", _("cannot get netlink socket fd")); + goto error_server; + } + + if (nl_socket_set_nonblocking(srv->netlinknh)) { + netlinkError(errno, + "%s", _("cannot set netlink socket nonblocking")); + goto error_server; + } + + if ((srv->eventwatch = virEventAddHandle(fd, + VIR_EVENT_HANDLE_READABLE, + virNetlinkEventCallback, + srv, NULL)) < 0) { + netlinkError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to add netlink event handle watch")); + + goto error_server; + } + + srv->netlinkfd = fd; + VIR_DEBUG("netlink event listener on fd: %i running",fd); + + ret = 0; + server=srv; + +error_server: + if (ret == -1){ + nl_close(srv->netlinknh); + nl_handle_destroy(srv->netlinknh); + } +error_locked: + virNetlinkEventServerUnlock(srv); + if (ret == -1) { + virMutexDestroy(&srv->lock); + VIR_FREE(srv); + } +error: + return ret; +} + +/** + * virNetlinkEventAddClient: + * + * @cb: callback to invoke when an event occurs + * @opaque: user data to pass to callback + * @macaddr: macaddr to store with the data. Used to identify callers. May be null. + * + * register a callback for handling of netlink messages. The + * registered function receives the entire netlink message and + * may choose to act upon it. + * + * returns -1 if the file handle cannot be registered, number of monitor upon success + */ +int +virNetlinkEventAddClient(virNetlinkEventHandleCallback cb, + void *opaque, + const unsigned char *macaddr) { + int i,r, result; + virNetlinkEventSrvPrivatePtr srv = server; + + if ( cb == NULL ) + return -1; + + virNetlinkEventServerLock(srv); + + VIR_DEBUG("adding client: %d.",nextWatch); + + r = 0; + /* first try to re-use deleted free slots */ + for (i = 0; i < srv->handlesCount; i++) { + if (srv->handles[i].deleted == VIR_NETLINK_HANDLE_DELETED) { + r = i; + goto addentry; + } + } + /* Resize the eventLoop array if needed */ + if (srv->handlesCount == srv->handlesAlloc) { + VIR_DEBUG("Used %zu handle slots, adding at least %d more", + srv->handlesAlloc, NETLINK_EVENT_ALLOC_EXTENT); + if (VIR_RESIZE_N(srv->handles, srv->handlesAlloc, + srv->handlesCount, NETLINK_EVENT_ALLOC_EXTENT) < 0) { + result = -1; + goto error; + } + } + r = srv->handlesCount++; + +addentry: + srv->handles[r].watch = nextWatch; + srv->handles[r].cb = cb; + srv->handles[r].opaque = opaque; + srv->handles[r].deleted = VIR_NETLINK_HANDLE_VALID; + if (macaddr) + memcpy(srv->handles[r].macaddr, macaddr, VIR_MAC_BUFLEN); + + VIR_DEBUG("added client to loop slot: %d. with macaddr ptr=%p", r, macaddr); + + result = nextWatch++; + +error: + virNetlinkEventServerUnlock(srv); + + return result; +} + +/** + * virNetlinkEventRemoveClient: + * + * @watch: watch whose handle to remove + * @macaddr: macaddr whose handle to remove + * + * Unregister a callback from a netlink monitor. + * The handler function referenced will no longer receive netlink messages. + * Either watch or macaddr may be used, the other should be null. + * + * returns -1 if the file handle was not registered, 0 upon success + */ +int +virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr) { + int i; + int ret = -1; + virNetlinkEventSrvPrivatePtr srv = server; + + VIR_DEBUG("removing client watch=%d, mac=%p.", + watch, macaddr); + + if (watch <= 0 && !macaddr) { + VIR_WARN("Ignoring invalid netlink client id: %d", watch); + return -1; + } + + virNetlinkEventServerLock(srv); + + for (i = 0; i < srv->handlesCount; i++) { + if (srv->handles[i].deleted != VIR_NETLINK_HANDLE_VALID) + continue; + + if (watch && srv->handles[i].watch == watch) { + VIR_FREE(srv->handles[i].opaque); + srv->handles[i].deleted = VIR_NETLINK_HANDLE_DELETED; + VIR_DEBUG("removed client: %d by index.", + srv->handles[i].watch); + ret = 0; + goto error; + } + if (!watch && memcmp(macaddr, srv->handles[i].macaddr, VIR_MAC_BUFLEN) == 0) { + VIR_FREE(srv->handles[i].opaque); + srv->handles[i].deleted = VIR_NETLINK_HANDLE_DELETED; + VIR_DEBUG("removed client: %d by mac.", + srv->handles[i].watch); + ret = 0; + goto error; + } + } + VIR_DEBUG("client not found to remove."); + +error: + virNetlinkEventServerUnlock(srv); + return ret; +} + #else int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED, @@ -154,4 +517,69 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED, return -1; } +/** + * stopNetlinkEventServer: stop the monitor to receive netlink messages for libvirtd + */ +int virNetlinkEventServiceStop(void) { + netlinkError(VIR_ERR_INTERNAL_ERROR, + "%s", +# if defined(__linux__) && !defined(HAVE_LIBNL) + _("virNetlinkEventServiceStop is not supported since libnl was not available")); +# endif + return 0; +} + +/** + * startNetlinkEventServer: start a monitor to receive netlink messages for libvirtd + */ +int virNetlinkEventServiceStart(void) { +# if defined(__linux__) && !defined(HAVE_LIBNL) + netlinkError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("virNetlinkEventServiceStart is not supported since libnl was not available")); +# endif + return 0; +} + +/** + * virNetlinkEventServiceIsRunning: returns if the netlink event service is running. + */ +int virNetlinkEventServiceIsRunning(void) { +# if defined(__linux__) && !defined(HAVE_LIBNL) + netlinkError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("virNetlinkEventServiceIsRunning is not supported since libnl was not available")); +# endif + return 0; +} + +/** + * virNetlinkEventAddClient: register a callback for handling of netlink messages + */ +int virNetlinkEventAddClient(virNetlinkEventHandleCallback cb, void *opaque, + const unsigned char *macaddr) { + netlinkError(VIR_ERR_INTERNAL_ERROR, + "%s", +# if defined(__linux__) && !defined(HAVE_LIBNL) + _("virNetlinkEventServiceAddClient is not supported since libnl was not available")); +# else + _("virNetlinkEventServiceAddClient is not supported on non-linux platforms")); +# endif + return -1; +} + +/** + * virNetlinkEventRemoveClient: unregister a callback from a netlink monitor + */ +int virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr) { + netlinkError(VIR_ERR_INTERNAL_ERROR, + "%s", +# if defined(__linux__) && !defined(HAVE_LIBNL) + _("virNetlinkEventRemoveClient is not supported since libnl was not available")); +# else + _("virNetlinkEventRemoveClient is not supported on non-linux platforms")); +# endif + return -1; +} + #endif /* __linux__ */ diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index a70abb6..1365afa 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -21,6 +21,7 @@ # define __VIR_NETLINK_H__ # include "config.h" +# include "internal.h" # if defined(__linux__) && defined(HAVE_LIBNL) @@ -29,6 +30,7 @@ # else struct nl_msg; +struct sockaddr_nl; # endif /* __linux__ */ @@ -36,4 +38,31 @@ int virNetlinkCommand(struct nl_msg *nl_msg, unsigned char **respbuf, unsigned int *respbuflen, int nl_pid); +typedef void (*virNetlinkEventHandleCallback)( unsigned char *msg, int length, struct sockaddr_nl *peer, bool *handled, void *opaque); + +/** + * stopNetlinkEventServer: stop the monitor to receive netlink messages for libvirtd + */ +int virNetlinkEventServiceStop(void); + +/** + * startNetlinkEventServer: start a monitor to receive netlink messages for libvirtd + */ +int virNetlinkEventServiceStart(void); + +/** + * virNetlinkEventServiceIsRunning: returns if the netlink event service is running. + */ +bool virNetlinkEventServiceIsRunning(void); + +/** + * virNetlinkEventAddClient: register a callback for handling of netlink messages + */ +int virNetlinkEventAddClient(virNetlinkEventHandleCallback cb, void *opaque, const unsigned char *macaddr); + +/** + * virNetlinkEventRemoveClient: unregister a callback from a netlink monitor + */ +int virNetlinkEventRemoveClient(int watch, const unsigned char *macaddr); + #endif /* __VIR_NETLINK_H__ */

From: "D. Herrendoerfer" <d.herrendoerfer(a)herrendoerfer.name&gt; Add de-association handling for 802.1qbg (vepa) via lldpad netlink messages. Also adds the possibility to perform an association request without waiting for a confirmation.

Signed-off-by: D. Herrendoerfer <d.herrendoerfer(a)herrendoerfer.name&gt; --- src/qemu/qemu_migration.c | 2 +- src/util/virnetdevmacvlan.c | 315 +++++++++++++++++++++++++++++++++++++- src/util/virnetdevvportprofile.c | 33 +++-- src/util/virnetdevvportprofile.h | 3 +- 4 files changed, 339 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 12cfbde..31428f8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { net->mac, virDomainNetGetActualDirectDev(net), def->uuid, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) < 0) + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) < 0) goto err_exit; } last_good_net = i; diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 25d0846..b3e3325 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, "passthrough") #if WITH_MACVTAP -

# include <stdint.h> # include <stdio.h> # include <errno.h> @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include <linux/if.h> # include <linux/if_tun.h> +# include <c-ctype.h> + /* Older kernels lacked this enum value. */ # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU # define MACVLAN_MODE_PASSTHRU 8 @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include "virfile.h" # include "virnetlink.h" # include "virnetdev.h" +# include "virpidfile.h" + # define MACVTAP_NAME_PREFIX "macvtap" # define MACVTAP_NAME_PATTERN "macvtap%d" @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # define MACVLAN_NAME_PREFIX "macvlan" # define MACVLAN_NAME_PATTERN "macvlan%d" +

/** * virNetDevMacVLanCreate: * @@ -445,6 +449,293 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU, }; +/* Struct to hold the state and configuration of a 802.1qbg port */ +struct virNetlinkCallbackData { + char cr_ifname[64]; + virNetDevVPortProfilePtr virtPortProfile; + const unsigned char *macaddress; + const char *linkdev; + const unsigned char *vmuuid; + enum virNetDevVPortProfileOp vmOp; + unsigned int linkState; +}; + +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; + +#define INSTANCE_STRLEN 36 + +static int instance2str(const unsigned char *p, char *dst, size_t size) +{ + if (dst && size > INSTANCE_STRLEN) { + snprintf(dst, size, "%02x%02x%02x%02x-%02x%02x-%02x%02x-" + "%02x%02x-%02x%02x%02x%02x%02x%02x", + p[0], p[1], p[2], p[3], + p[4], p[5], p[6], p[7], + p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); + return 0; + } + return -1; +} + +# define LLDPAD_PID_FILE "/var/run/lldpad.pid" +# define VIRIP_PID_FILE "/var/run/virip.pid" + +/** + * virNetDevMacVLanVPortProfileCallback: + * + * @msg: The buffer containing the received netlink message + * @length: The length of the received netlink message. + * @peer: The netling sockaddr containing the peer information + * @handeled: Contains information if the message has been replied to yet + * @opaque: Contains vital information regarding the associated vm an interface + * + * This function is called when a netlink message is received. The function + * reads the message and responds if it is pertinent to the running VMs + * network interface. + */ + +static void +virNetDevMacVLanVPortProfileCallback( unsigned char *msg, + int length, + struct sockaddr_nl *peer, + bool *handled, + void *opaque) +{ + struct nla_policy ifla_vf_policy[IFLA_VF_MAX + 1] = { + [IFLA_VF_MAC] = {.minlen = sizeof(struct ifla_vf_mac), + .maxlen = sizeof(struct ifla_vf_mac)}, + [IFLA_VF_VLAN] = {.minlen = sizeof(struct ifla_vf_vlan), + .maxlen = sizeof(struct ifla_vf_vlan)},

+ }; + + struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = { + [IFLA_PORT_RESPONSE] = {.type = NLA_U16}, + }; + + struct nlattr *tb[IFLA_MAX + 1], + *tb3[IFLA_PORT_MAX + 1], + *tb_vfinfo[IFLA_VF_MAX + 1], *tb_vfinfo_list; + + struct ifinfomsg ifinfo; + struct nlmsghdr *hdr; + void *data; + int rem; + char *ifname; + bool indicate = false;

+ virNetlinkCallbackDataPtr calld = opaque; + pid_t lldpad_pid = 0; + pid_t virip_pid = 0; + + hdr = (struct nlmsghdr *) msg;

+ data = nlmsg_data(hdr); + + /* Quickly decide if we want this or not */ + + if (virPidFileReadPath(LLDPAD_PID_FILE, &lldpad_pid) < 0) + return; + + ignore_value(virPidFileReadPath(VIRIP_PID_FILE, &virip_pid)); + + if (hdr->nlmsg_pid != lldpad_pid && hdr->nlmsg_pid != virip_pid) + return; // we only care for lldpad and virip messages + if (hdr->nlmsg_type != RTM_SETLINK) + return; // we only care for RTM_SETLINK + if (*handled) + return; // if it has been handeled - dont handle again

+ + /* DEBUG start */ + VIR_INFO("netlink message nl_sockaddr: %p len: %d", peer, length); + VIR_DEBUG("nlmsg_type = 0x%02x",hdr->nlmsg_type); + VIR_DEBUG("nlmsg_len = 0x%04x",hdr->nlmsg_len ); + VIR_DEBUG("nlmsg_pid = %d",hdr->nlmsg_pid ); + VIR_DEBUG("nlmsg_seq = 0x%08x",hdr->nlmsg_seq ); + VIR_DEBUG("nlmsg_flags = 0x%04x",hdr->nlmsg_flags ); + + VIR_DEBUG("lldpad pid = %d",lldpad_pid); + + switch (hdr->nlmsg_type) { + case RTM_NEWLINK: + case RTM_DELLINK: + case RTM_SETLINK: + case RTM_GETLINK: + VIR_DEBUG(" IFINFOMSG\n"); + VIR_DEBUG(" ifi_family = 0x%02x\n", + ((struct ifinfomsg *)data)->ifi_family); + VIR_DEBUG(" ifi_type = 0x%x\n", + ((struct ifinfomsg *)data)->ifi_type); + VIR_DEBUG(" ifi_index = %i\n", + ((struct ifinfomsg *)data)->ifi_index); + VIR_DEBUG(" ifi_flags = 0x%04x\n", + ((struct ifinfomsg *)data)->ifi_flags); + VIR_DEBUG(" ifi_change = 0x%04x\n", + ((struct ifinfomsg *)data)->ifi_change); + } + /* DEBUG end */ + + /* Parse netlink message assume a setlink with vfports */ + memcpy(&ifinfo, NLMSG_DATA(hdr), sizeof ifinfo); + VIR_DEBUG("family:%#x type:%#x index:%d flags:%#x change:%#x", + ifinfo.ifi_family, ifinfo.ifi_type, ifinfo.ifi_index, + ifinfo.ifi_flags, ifinfo.ifi_change); + if (nlmsg_parse(hdr, sizeof ifinfo, + (struct nlattr **)&tb, IFLA_MAX, NULL)) { + VIR_DEBUG("error parsing request..."); + return; + } + + if (tb[IFLA_VFINFO_LIST]) { + VIR_DEBUG("FOUND IFLA_VFINFO_LIST!"); + + nla_for_each_nested(tb_vfinfo_list, tb[IFLA_VFINFO_LIST], rem) { + if (nla_type(tb_vfinfo_list) != IFLA_VF_INFO) { + VIR_DEBUG("nested parsing of" + "IFLA_VFINFO_LIST failed."); + return; + } + if (nla_parse_nested(tb_vfinfo, IFLA_VF_MAX, + tb_vfinfo_list, ifla_vf_policy)) { + VIR_DEBUG("nested parsing of " + "IFLA_VF_INFO failed."); + return; + } + } + + if (tb_vfinfo[IFLA_VF_MAC]) { + struct ifla_vf_mac *mac = + RTA_DATA(tb_vfinfo[IFLA_VF_MAC]); + unsigned char *m = mac->mac; + + VIR_DEBUG("IFLA_VF_MAC=%2x:%2x:%2x:%2x:%2x:%2x", + m[0], m[1], m[2], m[3], m[4], m[5]); + + if (memcmp(calld->macaddress, m, VIR_MAC_BUFLEN)) + { + /* Repeat the same check for a broadcast mac */ + unsigned char broadcastmac[VIR_MAC_BUFLEN]; + int i; + + for (i=0;i<VIR_MAC_BUFLEN;i++) + broadcastmac[i]=0xFF; + + if (memcmp(calld->macaddress, broadcastmac, VIR_MAC_BUFLEN)) { + VIR_DEBUG("MAC address match failed."); + return;

+ } + } + } + + if (tb_vfinfo[IFLA_VF_VLAN]) { + struct ifla_vf_vlan *vlan = + RTA_DATA(tb_vfinfo[IFLA_VF_VLAN]); + + VIR_DEBUG("IFLA_VF_VLAN=%d", vlan->vlan); + } + } + + if (tb[IFLA_IFNAME]) { + ifname = (char *)RTA_DATA(tb[IFLA_IFNAME]); + VIR_DEBUG("IFLA_IFNAME=%s\n", ifname); + } + + if (tb[IFLA_OPERSTATE]) { + rem = *(unsigned short *)RTA_DATA(tb[IFLA_OPERSTATE]); + VIR_DEBUG("IFLA_OPERSTATE=%d\n", rem); + } + + if (tb[IFLA_VF_PORTS]) { + struct nlattr *tb_vf_ports; + + VIR_DEBUG("found IFLA_VF_PORTS\n"); + nla_for_each_nested(tb_vf_ports, tb[IFLA_VF_PORTS], rem) { + + VIR_DEBUG("iterating\n"); + if (nla_type(tb_vf_ports) != IFLA_VF_PORT) { + VIR_DEBUG("not a IFLA_VF_PORT. skipping\n"); + continue; + } + if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb_vf_ports, + ifla_port_policy)) { + VIR_DEBUG("nested parsing on level 2" + " failed."); + } + if (tb3[IFLA_PORT_VF]) { + VIR_DEBUG("IFLA_PORT_VF=%d", + *(uint32_t + *) (RTA_DATA(tb3[IFLA_PORT_VF]))); + } + if (tb3[IFLA_PORT_PROFILE]) { + VIR_DEBUG("IFLA_PORT_PROFILE=%s", (char *) + RTA_DATA(tb3[IFLA_PORT_PROFILE])); + } + + if (tb3[IFLA_PORT_VSI_TYPE]) { + struct ifla_port_vsi *pvsi; + int tid = 0; + + pvsi = (struct ifla_port_vsi *) + RTA_DATA(tb3[IFLA_PORT_VSI_TYPE]); + tid = pvsi->vsi_type_id[2] << 16 | + pvsi->vsi_type_id[1] << 8 | + pvsi->vsi_type_id[0]; + + VIR_DEBUG("mgr_id: %d", pvsi->vsi_mgr_id); + VIR_DEBUG("type_id: %d", tid); + VIR_DEBUG("type_version: %d", + pvsi->vsi_type_version); + } + + if (tb3[IFLA_PORT_INSTANCE_UUID]) { + char instance[INSTANCE_STRLEN + 2]; + unsigned char *uuid; + + uuid = (unsigned char *) + RTA_DATA(tb3[IFLA_PORT_INSTANCE_UUID]); + instance2str(uuid, instance, sizeof(instance)); + VIR_DEBUG("IFLA_PORT_INSTANCE_UUID=%s\n", + instance); + } + + if (tb3[IFLA_PORT_REQUEST]) { + uint8_t req = *(uint8_t *) RTA_DATA(tb3[IFLA_PORT_REQUEST]); + VIR_DEBUG("IFLA_PORT_REQUEST=%d", req); + + if ( req == PORT_REQUEST_DISASSOCIATE ) { + VIR_DEBUG("Set dissaccociated."); + indicate=true; + } + } + + if (tb3[IFLA_PORT_RESPONSE]) { + VIR_DEBUG("IFLA_PORT_RESPONSE=%d\n", *(uint16_t *) + RTA_DATA(tb3[IFLA_PORT_RESPONSE])); + } + } + } + + if (!indicate) { + return; + } + + VIR_INFO("Re-send 802.1qbg associate request:"); + VIR_INFO(" if: %s",calld->cr_ifname ); + VIR_INFO(" lf: %s",calld->linkdev ); + VIR_INFO(" mac: %02x:%02x:%02x:%02x:%02x:%02x",calld->macaddress[0], + calld->macaddress[1], + calld->macaddress[2], + calld->macaddress[3], + calld->macaddress[4], + calld->macaddress[5] );

+ + ignore_value(virNetDevVPortProfileAssociate(calld->cr_ifname, + calld->virtPortProfile, + calld->macaddress, + calld->linkdev, + calld->vmuuid, calld->vmOp, true)); + + *handled = true; + return; +} + /** * virNetDevMacVLanCreateWithVPortProfile: * Create an instance of a macvtap device and open its tap character @@ -547,7 +838,7 @@ create_name: virtPortProfile, macaddress, linkdev, - vmuuid, vmOp) < 0) { + vmuuid, vmOp, false) < 0) { rc = -1; goto link_del_exit; } @@ -589,6 +880,23 @@ create_name: goto disassociate_exit; } + if (virNetlinkEventServiceIsRunning()) { + virNetlinkCallbackDataPtr calld; + + if (VIR_ALLOC(calld) < 0) { + virReportOOMError(); + goto disassociate_exit; + } + + strncpy(calld->cr_ifname,cr_ifname,64); + calld->virtPortProfile=virtPortProfile; + calld->macaddress=macaddress; + calld->linkdev=linkdev; + calld->vmuuid=vmuuid; + calld->vmOp=vmOp;

+ + virNetlinkEventAddClient(virNetDevMacVLanVPortProfileCallback, calld, macaddress);

+ } return rc; @@ -638,6 +946,9 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, if (virNetDevMacVLanDelete(ifname) < 0) ret = -1; } + + virNetlinkEventRemoveClient(0,macaddr); + return ret; } diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index faadc3a..554f128 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -650,7 +650,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, const unsigned char *instanceId, const unsigned char *hostUUID, int32_t vf, - uint8_t op) + uint8_t op, + bool setlink_only) { int rc; unsigned char *recvbuf = NULL; @@ -675,6 +676,9 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, return rc; } + if (setlink_only) /*for re-associations on existing links*/ + return 0; + while (--repeats >= 0) { rc = virNetDevVPortProfileLinkDump(NULL, ifindex, nltarget_kernel, tb, &recvbuf, virNetDevVPortProfileGetLldpadPid); @@ -751,7 +755,8 @@ static int virNetDevVPortProfileOp8021Qbg(const char *ifname, const unsigned char *macaddr, const virNetDevVPortProfilePtr virtPort, - enum virNetDevVPortProfileLinkOp virtPortOp) + enum virNetDevVPortProfileLinkOp virtPortOp, + bool setlink_only) { int rc = 0; int op = PORT_REQUEST_ASSOCIATE; @@ -804,7 +809,8 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname, virtPort->u.virtPort8021Qbg.instanceID, NULL, vf, - op); + op, + setlink_only); err_exit: @@ -892,7 +898,8 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, (virtPortOp == VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR) ? PORT_REQUEST_PREASSOCIATE_RR - : PORT_REQUEST_ASSOCIATE); + : PORT_REQUEST_ASSOCIATE, + false); if (rc == -2) /* Association timed out, disassociate */ virNetDevVPortProfileOpCommon(NULL, ifindex, @@ -904,7 +911,8 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, NULL, NULL, vf, - PORT_REQUEST_DISASSOCIATE); + PORT_REQUEST_DISASSOCIATE, + false); break; case VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE: @@ -917,7 +925,8 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, NULL, NULL, vf, - PORT_REQUEST_DISASSOCIATE); + PORT_REQUEST_DISASSOCIATE, + false); break; default: @@ -938,6 +947,7 @@ err_exit: * @virtPort: pointer to the object holding port profile parameters * @vmuuid : the UUID of the virtual machine * @vmOp : The VM operation (i.e., create, no-op) + * @setlink_only : Only set the link - dont wait for the link to come up * * Associate a port on a swtich with a profile. This function * may notify a kernel driver or an external daemon to run @@ -954,7 +964,8 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, const unsigned char *macvtap_macaddr, const char *linkdev, const unsigned char *vmuuid, - enum virNetDevVPortProfileOp vmOp) + enum virNetDevVPortProfileOp vmOp, + bool setlink_only) { int rc = 0; @@ -976,7 +987,8 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, virtPort, (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE - : VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE); + : VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE, + setlink_only); break; case VIR_NETDEV_VPORT_PROFILE_8021QBH: @@ -1033,7 +1045,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, case VIR_NETDEV_VPORT_PROFILE_8021QBG: rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, virtPort, - VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE); + VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE, false); break; case VIR_NETDEV_VPORT_PROFILE_8021QBH: @@ -1056,7 +1068,8 @@ int virNetDevVPortProfileAssociate(const char *macvtap_ifname ATTRIBUTE_UNUSED, const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, const unsigned char *vmuuid ATTRIBUTE_UNUSED, - enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED) + enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED, + bool setlink_only) { virReportSystemError(ENOSYS, "%s", _("Virtual port profile association not supported on this platform")); diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index 7a8d81f..fe80086 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -81,7 +81,8 @@ int virNetDevVPortProfileAssociate(const char *ifname, const unsigned char *macaddr, const char *linkdev, const unsigned char *vmuuid, - enum virNetDevVPortProfileOp vmOp) + enum virNetDevVPortProfileOp vmOp, + bool setlink_only) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;

On Feb 23, 2012, at 11:16 PM, Laine Stump wrote: > On 02/22/2012 08:17 AM, D. Herrendoerfer wrote: >> From: "D. Herrendoerfer" <d.herrendoerfer(a)herrendoerfer.name&gt; >> >> Add de-association handling for 802.1qbg (vepa) via lldpad >> netlink messages. Also adds the possibility to perform an >> association request without waiting for a confirmation. > > The main issue I see with this patch is with whitespace, but that can > easily be fixed prior to pushing it, so ACK. Is the message format > used > by this patch, the absolute final version? (i.e. can we safely push > it > an know that it will be correct?) The patch to libvirt was picked this week. So yes, we can assume that the message will not be changed. But as always : Never say never !

The callback mechanism is not re-armed when libvirt is restarted now. The reason is: lldpad remembers who sent the associate by pid - since in theory there could be multiple agents performing associations. So if the libvirt pid changes, there is little we can do now. And thanks ! Dirk H > >> >> Signed-off-by: D. Herrendoerfer <d.herrendoerfer(a)herrendoerfer.name&gt; >> --- >> src/qemu/qemu_migration.c | 2 +- >> src/util/virnetdevmacvlan.c | 315 +++++++++++++++++++++++++++ >> ++++++++++- >> src/util/virnetdevvportprofile.c | 33 +++-- >> src/util/virnetdevvportprofile.h | 3 +- >> 4 files changed, 339 insertions(+), 14 deletions(-) >> >> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c >> index 12cfbde..31428f8 100644 >> --- a/src/qemu/qemu_migration.c >> +++ b/src/qemu/qemu_migration.c >> @@ -2567,7 +2567,7 @@ >> qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { >> net->mac, >> >> virDomainNetGetActualDirectDev(net), >> def->uuid, >> - >> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) < 0) >> + >> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) < 0) >> goto err_exit; >> } >> last_good_net = i; >> diff --git a/src/util/virnetdevmacvlan.c b/src/util/ >> virnetdevmacvlan.c >> index 25d0846..b3e3325 100644 >> --- a/src/util/virnetdevmacvlan.c >> +++ b/src/util/virnetdevmacvlan.c >> @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, >> VIR_NETDEV_MACVLAN_MODE_LAST, >> "passthrough") >> >> #if WITH_MACVTAP >> - > > spurious whitespace change. > >> # include <stdint.h> >> # include <stdio.h> >> # include <errno.h> >> @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, >> VIR_NETDEV_MACVLAN_MODE_LAST, >> # include <linux/if.h> >> # include <linux/if_tun.h> >> >> +# include <c-ctype.h> >> + >> /* Older kernels lacked this enum value. */ >> # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU >> # define MACVLAN_MODE_PASSTHRU 8 >> @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, >> VIR_NETDEV_MACVLAN_MODE_LAST, >> # include "virfile.h" >> # include "virnetlink.h" >> # include "virnetdev.h" >> +# include "virpidfile.h" >> + >> >> # define MACVTAP_NAME_PREFIX "macvtap" >> # define MACVTAP_NAME_PATTERN "macvtap%d" >> @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, >> VIR_NETDEV_MACVLAN_MODE_LAST, >> # define MACVLAN_NAME_PREFIX "macvlan" >> # define MACVLAN_NAME_PATTERN "macvlan%d" >> >> + > > Another spurious whitespace change. > >> /** >> * virNetDevMacVLanCreate: >> * >> @@ -445,6 +449,293 @@ static const uint32_t >> modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { >> [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU, >> }; >> >> +/* Struct to hold the state and configuration of a 802.1qbg port */ >> +struct virNetlinkCallbackData { >> + char cr_ifname[64]; >> + virNetDevVPortProfilePtr virtPortProfile; >> + const unsigned char *macaddress; >> + const char *linkdev; >> + const unsigned char *vmuuid; >> + enum virNetDevVPortProfileOp vmOp; >> + unsigned int linkState; >> +}; >> + >> +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; >> + >> +#define INSTANCE_STRLEN 36 >> + >> +static int instance2str(const unsigned char *p, char *dst, size_t >> size) >> +{ >> + if (dst && size > INSTANCE_STRLEN) { >> + snprintf(dst, size, "%02x%02x%02x%02x-%02x%02x-%02x%02x-" >> + "%02x%02x-%02x%02x%02x%02x%02x%02x", >> + p[0], p[1], p[2], p[3], >> + p[4], p[5], p[6], p[7], >> + p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); >> + return 0; >> + } >> + return -1; >> +} >> + >> +# define LLDPAD_PID_FILE "/var/run/lldpad.pid" >> +# define VIRIP_PID_FILE "/var/run/virip.pid" >> + >> +/** >> + * virNetDevMacVLanVPortProfileCallback: >> + * >> + * @msg: The buffer containing the received netlink message >> + * @length: The length of the received netlink message. >> + * @peer: The netling sockaddr containing the peer information >> + * @handeled: Contains information if the message has been >> replied to yet >> + * @opaque: Contains vital information regarding the associated >> vm an interface >> + * >> + * This function is called when a netlink message is received. >> The function >> + * reads the message and responds if it is pertinent to the >> running VMs >> + * network interface. >> + */ >> + >> +static void >> +virNetDevMacVLanVPortProfileCallback( unsigned char *msg, >> + int length, >> + struct sockaddr_nl *peer, >> + bool *handled, >> + void *opaque) >> +{ >> + struct nla_policy ifla_vf_policy[IFLA_VF_MAX + 1] = { >> + [IFLA_VF_MAC] = {.minlen = sizeof(struct ifla_vf_mac), >> + .maxlen = sizeof(struct ifla_vf_mac)}, >> + [IFLA_VF_VLAN] = {.minlen = sizeof(struct ifla_vf_vlan), >> + .maxlen = sizeof(struct ifla_vf_vlan)}, > > The indentation looks off here. Don't know if it's the actual patch > or > my mail client... > >> + }; >> + >> + struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = { >> + [IFLA_PORT_RESPONSE] = {.type = NLA_U16}, >> + }; >> + >> + struct nlattr *tb[IFLA_MAX + 1], >> + *tb3[IFLA_PORT_MAX + 1], >> + *tb_vfinfo[IFLA_VF_MAX + 1], *tb_vfinfo_list; >> + >> + struct ifinfomsg ifinfo; >> + struct nlmsghdr *hdr; >> + void *data; >> + int rem; >> + char *ifname; >> + bool indicate = false; > > The previous 4 lines all have tab characters instead of spaces. > libvirt > likes spaces only. > >> + virNetlinkCallbackDataPtr calld = opaque; >> + pid_t lldpad_pid = 0; >> + pid_t virip_pid = 0; >> + >> + hdr = (struct nlmsghdr *) msg; > > Another tab. I'll stop pointing these out, and just mention that > "make > syntax-check" will show these to you - you should try to run make > syntax > check before sending patches. > >> + data = nlmsg_data(hdr); >> + >> + /* Quickly decide if we want this or not */ >> + >> + if (virPidFileReadPath(LLDPAD_PID_FILE, &lldpad_pid) < 0) >> + return; >> + >> + ignore_value(virPidFileReadPath(VIRIP_PID_FILE, &virip_pid)); >> + >> + if (hdr->nlmsg_pid != lldpad_pid && hdr->nlmsg_pid != >> virip_pid) >> + return; // we only care for lldpad and virip messages >> + if (hdr->nlmsg_type != RTM_SETLINK) >> + return; // we only care for RTM_SETLINK >> + if (*handled) >> + return; // if it has been handeled - dont handle again > > misspelled "handled" (Could this if be further up in the function?) > >> + >> + /* DEBUG start */ >> + VIR_INFO("netlink message nl_sockaddr: %p len: %d", peer, >> length); >> + VIR_DEBUG("nlmsg_type = 0x%02x",hdr->nlmsg_type); >> + VIR_DEBUG("nlmsg_len = 0x%04x",hdr->nlmsg_len ); >> + VIR_DEBUG("nlmsg_pid = %d",hdr->nlmsg_pid ); >> + VIR_DEBUG("nlmsg_seq = 0x%08x",hdr->nlmsg_seq ); >> + VIR_DEBUG("nlmsg_flags = 0x%04x",hdr->nlmsg_flags ); >> + >> + VIR_DEBUG("lldpad pid = %d",lldpad_pid); >> + >> + switch (hdr->nlmsg_type) { >> + case RTM_NEWLINK: >> + case RTM_DELLINK: >> + case RTM_SETLINK: >> + case RTM_GETLINK: >> + VIR_DEBUG(" IFINFOMSG\n"); >> + VIR_DEBUG(" ifi_family = 0x%02x\n", >> + ((struct ifinfomsg *)data)->ifi_family); >> + VIR_DEBUG(" ifi_type = 0x%x\n", >> + ((struct ifinfomsg *)data)->ifi_type); >> + VIR_DEBUG(" ifi_index = %i\n", >> + ((struct ifinfomsg *)data)->ifi_index); >> + VIR_DEBUG(" ifi_flags = 0x%04x\n", >> + ((struct ifinfomsg *)data)->ifi_flags); >> + VIR_DEBUG(" ifi_change = 0x%04x\n", >> + ((struct ifinfomsg *)data)->ifi_change); >> + } >> + /* DEBUG end */ >> + >> + /* Parse netlink message assume a setlink with vfports */ >> + memcpy(&ifinfo, NLMSG_DATA(hdr), sizeof ifinfo); >> + VIR_DEBUG("family:%#x type:%#x index:%d flags:%#x change:%#x", >> + ifinfo.ifi_family, ifinfo.ifi_type, ifinfo.ifi_index, >> + ifinfo.ifi_flags, ifinfo.ifi_change); >> + if (nlmsg_parse(hdr, sizeof ifinfo, >> + (struct nlattr **)&tb, IFLA_MAX, NULL)) { >> + VIR_DEBUG("error parsing request..."); >> + return; >> + } >> + >> + if (tb[IFLA_VFINFO_LIST]) { >> + VIR_DEBUG("FOUND IFLA_VFINFO_LIST!"); >> + >> + nla_for_each_nested(tb_vfinfo_list, tb[IFLA_VFINFO_LIST], >> rem) { >> + if (nla_type(tb_vfinfo_list) != IFLA_VF_INFO) { >> + VIR_DEBUG("nested parsing of" >> + "IFLA_VFINFO_LIST failed."); >> + return; >> + } >> + if (nla_parse_nested(tb_vfinfo, IFLA_VF_MAX, >> + tb_vfinfo_list, ifla_vf_policy)) { >> + VIR_DEBUG("nested parsing of " >> + "IFLA_VF_INFO failed."); >> + return; >> + } >> + } >> + >> + if (tb_vfinfo[IFLA_VF_MAC]) { >> + struct ifla_vf_mac *mac = >> + RTA_DATA(tb_vfinfo[IFLA_VF_MAC]); >> + unsigned char *m = mac->mac; >> + >> + VIR_DEBUG("IFLA_VF_MAC=%2x:%2x:%2x:%2x:%2x:%2x", >> + m[0], m[1], m[2], m[3], m[4], m[5]); >> + >> + if (memcmp(calld->macaddress, m, VIR_MAC_BUFLEN)) >> + { >> + /* Repeat the same check for a broadcast mac */ >> + unsigned char broadcastmac[VIR_MAC_BUFLEN]; >> + int i; >> + >> + for (i=0;i<VIR_MAC_BUFLEN;i++) >> + broadcastmac[i]=0xFF; >> + >> + if (memcmp(calld->macaddress, broadcastmac, >> VIR_MAC_BUFLEN)) { >> + VIR_DEBUG("MAC address match failed."); >> + return; > > > Why not just compare calld->macaddress[i] to 0xFF in the for loop, > returning immediately when a non-FF byte is found? > > >> + } >> + } >> + } >> + >> + if (tb_vfinfo[IFLA_VF_VLAN]) { >> + struct ifla_vf_vlan *vlan = >> + RTA_DATA(tb_vfinfo[IFLA_VF_VLAN]); >> + >> + VIR_DEBUG("IFLA_VF_VLAN=%d", vlan->vlan); >> + } >> + } >> + >> + if (tb[IFLA_IFNAME]) { >> + ifname = (char *)RTA_DATA(tb[IFLA_IFNAME]); >> + VIR_DEBUG("IFLA_IFNAME=%s\n", ifname); >> + } >> + >> + if (tb[IFLA_OPERSTATE]) { >> + rem = *(unsigned short *)RTA_DATA(tb[IFLA_OPERSTATE]); >> + VIR_DEBUG("IFLA_OPERSTATE=%d\n", rem); >> + } >> + >> + if (tb[IFLA_VF_PORTS]) { >> + struct nlattr *tb_vf_ports; >> + >> + VIR_DEBUG("found IFLA_VF_PORTS\n"); >> + nla_for_each_nested(tb_vf_ports, tb[IFLA_VF_PORTS], rem) { >> + >> + VIR_DEBUG("iterating\n"); >> + if (nla_type(tb_vf_ports) != IFLA_VF_PORT) { >> + VIR_DEBUG("not a IFLA_VF_PORT. skipping\n"); >> + continue; >> + } >> + if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb_vf_ports, >> + ifla_port_policy)) { >> + VIR_DEBUG("nested parsing on level 2" >> + " failed."); >> + } >> + if (tb3[IFLA_PORT_VF]) { >> + VIR_DEBUG("IFLA_PORT_VF=%d", >> + *(uint32_t >> + *) (RTA_DATA(tb3[IFLA_PORT_VF]))); >> + } >> + if (tb3[IFLA_PORT_PROFILE]) { >> + VIR_DEBUG("IFLA_PORT_PROFILE=%s", (char *) >> + RTA_DATA(tb3[IFLA_PORT_PROFILE])); >> + } >> + >> + if (tb3[IFLA_PORT_VSI_TYPE]) { >> + struct ifla_port_vsi *pvsi; >> + int tid = 0; >> + >> + pvsi = (struct ifla_port_vsi *) >> + RTA_DATA(tb3[IFLA_PORT_VSI_TYPE]); >> + tid = pvsi->vsi_type_id[2] << 16 | >> + pvsi->vsi_type_id[1] << 8 | >> + pvsi->vsi_type_id[0]; >> + >> + VIR_DEBUG("mgr_id: %d", pvsi->vsi_mgr_id); >> + VIR_DEBUG("type_id: %d", tid); >> + VIR_DEBUG("type_version: %d", >> + pvsi->vsi_type_version); >> + } >> + >> + if (tb3[IFLA_PORT_INSTANCE_UUID]) { >> + char instance[INSTANCE_STRLEN + 2]; >> + unsigned char *uuid; >> + >> + uuid = (unsigned char *) >> + RTA_DATA(tb3[IFLA_PORT_INSTANCE_UUID]); >> + instance2str(uuid, instance, sizeof(instance)); >> + VIR_DEBUG("IFLA_PORT_INSTANCE_UUID=%s\n", >> + instance); >> + } >> + >> + if (tb3[IFLA_PORT_REQUEST]) { >> + uint8_t req = *(uint8_t *) >> RTA_DATA(tb3[IFLA_PORT_REQUEST]); >> + VIR_DEBUG("IFLA_PORT_REQUEST=%d", req); >> + >> + if ( req == PORT_REQUEST_DISASSOCIATE ) { >> + VIR_DEBUG("Set dissaccociated."); >> + indicate=true; >> + } >> + } >> + >> + if (tb3[IFLA_PORT_RESPONSE]) { >> + VIR_DEBUG("IFLA_PORT_RESPONSE=%d\n", *(uint16_t *) >> + RTA_DATA(tb3[IFLA_PORT_RESPONSE])); >> + } >> + } >> + } >> + >> + if (!indicate) { >> + return; >> + } >> + >> + VIR_INFO("Re-send 802.1qbg associate request:"); >> + VIR_INFO(" if: %s",calld->cr_ifname ); >> + VIR_INFO(" lf: %s",calld->linkdev ); >> + VIR_INFO(" mac: %02x:%02x:%02x:%02x:%02x:%02x",calld- >> >macaddress[0], >> + calld- >> >macaddress[1], >> + calld- >> >macaddress[2], >> + calld- >> >macaddress[3], >> + calld- >> >macaddress[4], >> + calld- >> >macaddress[5] ); > > Bad indentation. > >> + >> + ignore_value(virNetDevVPortProfileAssociate(calld->cr_ifname, >> + calld->virtPortProfile, >> + calld->macaddress, >> + calld->linkdev, >> + calld->vmuuid, calld- >> >vmOp, true)); >> + >> + *handled = true; >> + return; >> +} >> + >> /** >> * virNetDevMacVLanCreateWithVPortProfile: >> * Create an instance of a macvtap device and open its tap character >> @@ -547,7 +838,7 @@ create_name: >> virtPortProfile, >> macaddress, >> linkdev, >> - vmuuid, vmOp) < 0) { >> + vmuuid, vmOp, false) < 0) { >> rc = -1; >> goto link_del_exit; >> } >> @@ -589,6 +880,23 @@ create_name: >> goto disassociate_exit; >> } >> >> + if (virNetlinkEventServiceIsRunning()) { >> + virNetlinkCallbackDataPtr calld; >> + >> + if (VIR_ALLOC(calld) < 0) { >> + virReportOOMError(); >> + goto disassociate_exit; >> + } >> + >> + strncpy(calld->cr_ifname,cr_ifname,64); >> + calld->virtPortProfile=virtPortProfile; >> + calld->macaddress=macaddress; >> + calld->linkdev=linkdev; >> + calld->vmuuid=vmuuid; >> + calld->vmOp=vmOp; > > Put spaces around "=" signs. > >> + >> + >> virNetlinkEventAddClient(virNetDevMacVLanVPortProfileCallback, >> calld, macaddress); > > Maybe we should check for a return value here. > > >> + } >> >> return rc; >> >> @@ -638,6 +946,9 @@ int >> virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, >> if (virNetDevMacVLanDelete(ifname) < 0) >> ret = -1; >> } >> + >> + virNetlinkEventRemoveClient(0,macaddr); >> + >> return ret; >> } >> >> diff --git a/src/util/virnetdevvportprofile.c b/src/util/ >> virnetdevvportprofile.c >> index faadc3a..554f128 100644 >> --- a/src/util/virnetdevvportprofile.c >> +++ b/src/util/virnetdevvportprofile.c >> @@ -650,7 +650,8 @@ virNetDevVPortProfileOpCommon(const char >> *ifname, int ifindex, >> const unsigned char *instanceId, >> const unsigned char *hostUUID, >> int32_t vf, >> - uint8_t op) >> + uint8_t op, >> + bool setlink_only) >> { >> int rc; >> unsigned char *recvbuf = NULL; >> @@ -675,6 +676,9 @@ virNetDevVPortProfileOpCommon(const char >> *ifname, int ifindex, >> return rc; >> } >> >> + if (setlink_only) /*for re-associations on existing links*/ >> + return 0; >> + >> while (--repeats >= 0) { >> rc = virNetDevVPortProfileLinkDump(NULL, ifindex, >> nltarget_kernel, tb, >> &recvbuf, >> virNetDevVPortProfileGetLldpadPid); >> @@ -751,7 +755,8 @@ static int >> virNetDevVPortProfileOp8021Qbg(const char *ifname, >> const unsigned char *macaddr, >> const virNetDevVPortProfilePtr >> virtPort, >> - enum virNetDevVPortProfileLinkOp >> virtPortOp) >> + enum virNetDevVPortProfileLinkOp >> virtPortOp, >> + bool setlink_only) >> { >> int rc = 0; >> int op = PORT_REQUEST_ASSOCIATE; >> @@ -804,7 +809,8 @@ virNetDevVPortProfileOp8021Qbg(const char >> *ifname, >> virtPort- >> >u.virtPort8021Qbg.instanceID, >> NULL, >> vf, >> - op); >> + op, >> + setlink_only); >> >> err_exit: >> >> @@ -892,7 +898,8 @@ virNetDevVPortProfileOp8021Qbh(const char >> *ifname, >> (virtPortOp == >> >> VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR) ? >> >> PORT_REQUEST_PREASSOCIATE_RR >> - : >> PORT_REQUEST_ASSOCIATE); >> + : >> PORT_REQUEST_ASSOCIATE, >> + false); >> if (rc == -2) >> /* Association timed out, disassociate */ >> virNetDevVPortProfileOpCommon(NULL, ifindex, >> @@ -904,7 +911,8 @@ virNetDevVPortProfileOp8021Qbh(const char >> *ifname, >> NULL, >> NULL, >> vf, >> - >> PORT_REQUEST_DISASSOCIATE); >> + >> PORT_REQUEST_DISASSOCIATE, >> + false); >> break; >> >> case VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE: >> @@ -917,7 +925,8 @@ virNetDevVPortProfileOp8021Qbh(const char >> *ifname, >> NULL, >> NULL, >> vf, >> - >> PORT_REQUEST_DISASSOCIATE); >> + >> PORT_REQUEST_DISASSOCIATE, >> + false); >> break; >> >> default: >> @@ -938,6 +947,7 @@ err_exit: >> * @virtPort: pointer to the object holding port profile parameters >> * @vmuuid : the UUID of the virtual machine >> * @vmOp : The VM operation (i.e., create, no-op) >> + * @setlink_only : Only set the link - dont wait for the link to >> come up >> * >> * Associate a port on a swtich with a profile. This function >> * may notify a kernel driver or an external daemon to run >> @@ -954,7 +964,8 @@ virNetDevVPortProfileAssociate(const char >> *macvtap_ifname, >> const unsigned char *macvtap_macaddr, >> const char *linkdev, >> const unsigned char *vmuuid, >> - enum virNetDevVPortProfileOp vmOp) >> + enum virNetDevVPortProfileOp vmOp, >> + bool setlink_only) >> { >> int rc = 0; >> >> @@ -976,7 +987,8 @@ virNetDevVPortProfileAssociate(const char >> *macvtap_ifname, >> virtPort, >> (vmOp == >> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) >> ? >> VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE >> - : >> VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE); >> + : >> VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE, >> + setlink_only); >> break; >> >> case VIR_NETDEV_VPORT_PROFILE_8021QBH: >> @@ -1033,7 +1045,7 @@ virNetDevVPortProfileDisassociate(const char >> *macvtap_ifname, >> case VIR_NETDEV_VPORT_PROFILE_8021QBG: >> rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, >> macvtap_macaddr, >> virtPort, >> - >> VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE); >> + >> VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE, false); >> break; >> >> case VIR_NETDEV_VPORT_PROFILE_8021QBH: >> @@ -1056,7 +1068,8 @@ int virNetDevVPortProfileAssociate(const >> char *macvtap_ifname ATTRIBUTE_UNUSED, >> const unsigned char *macvtap_macaddr >> ATTRIBUTE_UNUSED, >> const char *linkdev ATTRIBUTE_UNUSED, >> const unsigned char *vmuuid >> ATTRIBUTE_UNUSED, >> - enum virNetDevVPortProfileOp vmOp >> ATTRIBUTE_UNUSED) >> + enum virNetDevVPortProfileOp vmOp >> ATTRIBUTE_UNUSED, >> + bool setlink_only) >> { >> virReportSystemError(ENOSYS, "%s", >> _("Virtual port profile association not >> supported on this platform")); >> diff --git a/src/util/virnetdevvportprofile.h b/src/util/ >> virnetdevvportprofile.h >> index 7a8d81f..fe80086 100644 >> --- a/src/util/virnetdevvportprofile.h >> +++ b/src/util/virnetdevvportprofile.h >> @@ -81,7 +81,8 @@ int virNetDevVPortProfileAssociate(const char >> *ifname, >> const unsigned char *macaddr, >> const char *linkdev, >> const unsigned char *vmuuid, >> - enum virNetDevVPortProfileOp >> vmOp) >> + enum virNetDevVPortProfileOp >> vmOp, >> + bool setlink_only) >> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) >> ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK; >> > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Feb 29, 2012, at 9:51 AM, Laine Stump wrote: > Since I found a couple other problems, but have made you suffer through > enough back and forth already, I've made some final suggested changes > myself, and am sending a diff patch as a response to this message > (differences from your latest version to mine), as well as doing a > repost of the original two patches with your latest changes and mine > squashed in. > > Please 1) take a look at the changes in my diff patches , then 2) test > the updated versions of the full patches (I'm unable to test beyond > compiling), and send your ACK if they're okay. Once I have your ACK > back, I'll push (I promise, I won't find any new issues *this* time :-) > ACK. The code tests out fine. One nit I found in the logs was that the removal was indicated by the wrong method: diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fd6f751..614f8da 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -509,7 +509,7 @@ virNetlinkEventRemoveClient(int watch, const unsigned char * virNetlinkEventRemoveClientPrimitive(i); VIR_DEBUG("removed client: %d by %s.", srv->handles[i].watch, - srv->handles[i].watch ? "index" : "mac"); + srv->handles[i].watch ? "mac" : "index");

ret = 0; goto cleanup; } -- Thanks for the effort ! Dirk H