Until now, the nl_pid of the source address of every message sent by virNetlinkCommand has been set to the value of getpid(). Most of the time this doesn't matter, and in the one case where it does (communication with lldpad), it previously was the proper thing to do, because the netlink event service (which listens on a netlink socket for unsolicited messages from lldpad) coincidentally always happened to bind with a local nl_pid == getpid(). With the fix for: https://bugzilla.redhat.com/show_bug.cgi?id=816465 that particular nl_pid is now effectively a reserved value, so the netlink event service will always bind to something else (coincidentally "getpid() + (1 << 22)", but it really could be anything). The result is that communication between lldpad and libvirtd is broken (lldpad gets a "disconnected" error when it tries to send a directed message). The solution to this problem cause by a solution, is to query the netlink event service's nlhandle for its "local_port", and send that as the source nl_pid (but only when sending to lldpad, of course - in other cases we maintain the old behavior of sending getpid()). There are two cases where a message is being directed at lldpad - one in virNetDevLinkDump, and one in virNetDevVPortProfileOpSetLink. The case of virNetDevVPortProfileOpSetLink is simplest to explain - only if !nltarget_kernel, i.e. the message isn't targetted for the kernel, is the dst_pid set (by calling virNetDevVPortProfileGetLldpadPid()), so only in that case do we call virNetlinkEvenServiceLocalPid() to set src_pid. For virNetDevLinkDump, it's a bit more complicated. The call to virNetDevVPortProfileGetLldpadPid() was effectively up one level (in virNetDevVPortProfileOpCommon), although obscured by an unnecessary passing of a function pointer. This patch removes the function pointer, and calls virNetDevVPortProfileGetLldpadPid() directly in virNetDevVPortProfileOpCommon - if it's doing this, it knows that it should also call virNetlinkEventServiceLocalPid() to set src_pid too; then it just passes src_pid and dst_pid down to virNetDevLinkDump. Since (src_pid == 0 && dst_pid == 0) implies that the kernel is the destination, there is no longer any need to send nltarget_kernel as an arg to virNetDevLinkDump, so it's been removed. The disparity between src_pid being int and dst_pid being uint32_t may be a bit disconcerting to some, but I didn't want to complicate virNetlinkEventServiceLocalPid() by having status returned separately from the value. --- src/util/virnetdev.c | 30 +++++++++++------------------- src/util/virnetdev.h | 4 ++-- src/util/virnetdevvportprofile.c | 24 +++++++++++++++++------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 72af3cd..d53352f 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1229,15 +1229,15 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { * * @ifname: The name of the interface; only use if ifindex < 0 * @ifindex: The interface index; may be < 0 if ifname is given - * @nltarget_kernel: whether to send the message to the kernel or another - * process * @nlattr: pointer to a pointer of netlink attributes that will contain * the results * @recvbuf: Pointer to the buffer holding the returned netlink response * message; free it, once not needed anymore - * @getPidFunc: Pointer to a function that will be invoked if the kernel - * is not the target of the netlink message but it is to be - * sent to another process. + * @src_pid: pid used for nl_pid of the local end of the netlink message + * (0 == "use getpid()") + * @dst_pid: pid of destination nl_pid if the kernel + * is not the target of the netlink message but it is to be + * sent to another process (0 if sending to the kernel) * * Get information about an interface given its name or index. * @@ -1245,9 +1245,9 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { */ int virNetDevLinkDump(const char *ifname, int ifindex, - bool nltarget_kernel, struct nlattr **tb, + struct nlattr **tb, unsigned char **recvbuf, - uint32_t (*getPidFunc)(void)) + uint32_t src_pid, uint32_t dst_pid) { int rc = -1; struct nlmsghdr *resp; @@ -1257,7 +1257,6 @@ virNetDevLinkDump(const char *ifname, int ifindex, .ifi_index = ifindex }; unsigned int recvbuflen; - uint32_t pid = 0; struct nl_msg *nl_msg; *recvbuf = NULL; @@ -1281,14 +1280,7 @@ virNetDevLinkDump(const char *ifname, int ifindex, goto buffer_too_small; } - if (!nltarget_kernel) { - pid = getPidFunc(); - if (pid == 0) { - goto cleanup; - } - } - - if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, 0, pid) < 0) + if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, src_pid, dst_pid) < 0) goto cleanup; if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) @@ -1526,7 +1518,7 @@ virNetDevGetVfConfig(const char *ifname, int vf, unsigned char *mac, struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; int ifindex = -1; - rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); + rc = virNetDevLinkDump(ifname, ifindex, tb, &recvbuf, 0, 0); if (rc < 0) return rc; @@ -1658,10 +1650,10 @@ virNetDevRestoreNetConfig(char *linkdev, int vf, char *stateDir) int virNetDevLinkDump(const char *ifname ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED, - bool nltarget_kernel ATTRIBUTE_UNUSED, struct nlattr **tb ATTRIBUTE_UNUSED, unsigned char **recvbuf ATTRIBUTE_UNUSED, - uint32_t (*getPidFunc)(void) ATTRIBUTE_UNUSED) + uint32_t src_pid ATTRIBUTE_UNUSED, + uint32_t dst_pid ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Unable to dump link info on this platform")); diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 73f4c64..660d2db 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -107,9 +107,9 @@ int virNetDevGetVirtualFunctions(const char *pfname, ATTRIBUTE_RETURN_CHECK; int virNetDevLinkDump(const char *ifname, int ifindex, - bool nltarget_kernel, struct nlattr **tb, + struct nlattr **tb, unsigned char **recvbuf, - uint32_t (*getPidFunc)(void)) + uint32_t src_pid, uint32_t dst_pid) ATTRIBUTE_RETURN_CHECK; int virNetDevReplaceNetConfig(char *linkdev, int vf, diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index def5c62..38261d1 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -282,7 +282,8 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, }; unsigned char *recvbuf = NULL; unsigned int recvbuflen = 0; - uint32_t pid = 0; + int src_pid = 0; + uint32_t dst_pid = 0; struct nl_msg *nl_msg; struct nlattr *vfports = NULL, *vfport; @@ -390,12 +391,13 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, } if (!nltarget_kernel) { - pid = virNetDevVPortProfileGetLldpadPid(); - if (pid == 0) + if ((src_pid = virNetlinkEventServiceLocalPid()) < 0) + goto cleanup; + if ((dst_pid = virNetDevVPortProfileGetLldpadPid()) == 0) goto cleanup; } - if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, 0, pid) < 0) + if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, src_pid, dst_pid) < 0) goto cleanup; if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) @@ -477,7 +479,7 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int return -1; while (!end && i <= nthParent) { - rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); + rc = virNetDevLinkDump(ifname, ifindex, tb, &recvbuf, 0, 0); if (rc < 0) break; @@ -524,6 +526,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, bool setlink_only) { int rc; + int src_pid = 0; + uint32_t dst_pid = 0; unsigned char *recvbuf = NULL; struct nlattr *tb[IFLA_MAX + 1] = { NULL , }; int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC; @@ -549,9 +553,15 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, if (setlink_only) /*for re-associations on existing links*/ return 0; + if (!nltarget_kernel && + (((src_pid = virNetlinkEventServiceLocalPid()) < 0) || + ((dst_pid = virNetDevVPortProfileGetLldpadPid()) == 0))) { + rc = -1; + goto cleanup; + } + while (--repeats >= 0) { - rc = virNetDevLinkDump(NULL, ifindex, nltarget_kernel, tb, - &recvbuf, virNetDevVPortProfileGetLldpadPid); + rc = virNetDevLinkDump(NULL, ifindex, tb, &recvbuf, src_pid, dst_pid); if (rc < 0) goto cleanup; -- 1.7.10 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list