Re: [libvirt] [PATCH 3/4 v3] virnetdevvportprofile: Changes to support portprofiles for hostdevs

From: Roopa Prabhu <roprabhu(a)cisco.com&gt; This patch includes the following changes - removes some netlink functions which are now available in virnetdev.c - Adds a vf argument to all port profile functions For 802.1Qbh devices, the port profile calls can use a vf argument if passed by the caller. If the vf argument is -1 it will try to derive the vf if the device passed is a virtual function. For 802.1Qbg devices, This patch introduces a null check for the device argument because during port profile assignment on a hostdev, this argument can be null. Stefan CC'ed for comments

Signed-off-by: Roopa Prabhu <roprabhu(a)cisco.com&gt; --- src/qemu/qemu_migration.c | 2 src/util/virnetdevmacvlan.c | 8 + src/util/virnetdevvportprofile.c | 221 ++++++++------------------------------ src/util/virnetdevvportprofile.h | 8 + 4 files changed, 59 insertions(+), 180 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5c4297c..77d40c0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2650,6 +2650,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { virDomainNetGetActualVirtPortProfile(net), net->mac, virDomainNetGetActualDirectDev(net), + -1, def->uuid, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) < 0) goto err_exit; @@ -2667,6 +2668,7 @@ err_exit: virDomainNetGetActualVirtPortProfile(net), net->mac, virDomainNetGetActualDirectDev(net), + -1, VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)); } } diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index f38a98c..647679f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -452,6 +452,7 @@ struct virNetlinkCallbackData { virNetDevVPortProfilePtr virtPortProfile; unsigned char *macaddress; char *linkdev; + int vf; unsigned char *vmuuid; enum virNetDevVPortProfileOp vmOp; unsigned int linkState; @@ -719,6 +720,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, calld->virtPortProfile, calld->macaddress, calld->linkdev, + calld->vf, calld->vmuuid, calld->vmOp, true)); *handled = true; @@ -810,6 +812,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, const char *cr_ifname; virNetlinkCallbackDataPtr calld = NULL; int ret; + int vf = -1; macvtapMode = modeMap[mode]; @@ -871,6 +874,7 @@ create_name: virtPortProfile, macaddress, linkdev, + vf, vmuuid, vmOp, false) < 0) { rc = -1; goto link_del_exit; @@ -948,6 +952,7 @@ disassociate_exit: virtPortProfile, macaddress, linkdev, + vf, vmOp)); link_del_exit: @@ -975,6 +980,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, char *stateDir) { int ret = 0; + int vf = -1; + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir)); } @@ -984,6 +991,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, virtPortProfile, macaddr, linkdev, + vf, VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0) ret = -1; if (virNetDevMacVLanDelete(ifname) < 0) diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index f6db292..bd356d8 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -126,11 +126,6 @@ static struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = { [IFLA_PORT_RESPONSE] = { .type = NLA_U16 }, }; -static struct nla_policy ifla_policy[IFLA_MAX + 1] = -{ - [IFLA_VF_PORTS] = { .type = NLA_NESTED }, -}; - static uint32_t virNetDevVPortProfileGetLldpadPid(void) { @@ -164,126 +159,6 @@ virNetDevVPortProfileGetLldpadPid(void) { return pid; } - -/** - * virNetDevVPortProfileLinkDump: - * - * @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. - * - * Get information about an interface given its name or index. - * - * Returns 0 on success, -1 on fatal error. - */ -static int -virNetDevVPortProfileLinkDump(const char *ifname, int ifindex, bool nltarget_kernel, - struct nlattr **tb, unsigned char **recvbuf, - uint32_t (*getPidFunc)(void)) -{ - int rc = 0; - struct nlmsghdr *resp; - struct nlmsgerr *err; - struct ifinfomsg ifinfo = { - .ifi_family = AF_UNSPEC, - .ifi_index = ifindex - }; - unsigned int recvbuflen; - uint32_t pid = 0; - struct nl_msg *nl_msg; - - *recvbuf = NULL; - - nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST); - if (!nl_msg) { - virReportOOMError(); - return -1; - } - - if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0) - goto buffer_too_small; - - if (ifindex < 0 && ifname) { - if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) - goto buffer_too_small; - } - - if (!nltarget_kernel) { - pid = getPidFunc(); - if (pid == 0) { - rc = -1; - goto cleanup; - } - } - - if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) { - rc = -1; - goto cleanup; - } - - if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) - goto malformed_resp; - - resp = (struct nlmsghdr *)*recvbuf; - - switch (resp->nlmsg_type) { - case NLMSG_ERROR: - err = (struct nlmsgerr *)NLMSG_DATA(resp); - if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) - goto malformed_resp; - - if (err->error) { - virReportSystemError(-err->error, - _("error dumping %s (%d) interface"), - ifname, ifindex); - rc = -1; - } - break; - - case GENL_ID_CTRL: - case NLMSG_DONE: - if (nlmsg_parse(resp, sizeof(struct ifinfomsg), - tb, IFLA_MAX, ifla_policy)) { - goto malformed_resp; - } - break; - - default: - goto malformed_resp; - } - - if (rc != 0) - VIR_FREE(*recvbuf); - -cleanup: - nlmsg_free(nl_msg); - - return rc; - -malformed_resp: - nlmsg_free(nl_msg); - - virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed netlink response message")); - VIR_FREE(*recvbuf); - return -1; - -buffer_too_small: - nlmsg_free(nl_msg); - - virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", - _("allocated netlink buffer is too small")); - return -1; -} - /** * virNetDevVPortProfileGetStatus: * @@ -607,7 +482,7 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int return -1; while (!end && i <= nthParent) { - rc = virNetDevVPortProfileLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); + rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL); if (rc < 0) break; @@ -680,8 +555,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, return 0; while (--repeats >= 0) { - rc = virNetDevVPortProfileLinkDump(NULL, ifindex, nltarget_kernel, tb, - &recvbuf, virNetDevVPortProfileGetLldpadPid); + rc = virNetDevLinkDump(NULL, ifindex, nltarget_kernel, tb, + &recvbuf, virNetDevVPortProfileGetLldpadPid); if (rc < 0) goto err_exit; @@ -754,6 +629,7 @@ virNetDevVPortProfileGetPhysdevAndVlan(const char *ifname, int *root_ifindex, ch static int virNetDevVPortProfileOp8021Qbg(const char *ifname, const unsigned char *macaddr, + int vf, const virNetDevVPortProfilePtr virtPort, enum virNetDevVPortProfileLinkOp virtPortOp, bool setlink_only) @@ -768,7 +644,11 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname, int vlanid; int physdev_ifindex = 0; char physdev_ifname[IFNAMSIZ] = { 0, }; - int vf = PORT_SELF_VF; + + if (!ifname) + return -1; + + vf = PORT_SELF_VF; if (virNetDevVPortProfileGetPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname, &vlanid) < 0) { @@ -817,46 +697,11 @@ err_exit: return rc; } - -static int -virNetDevVPortProfileGetPhysfnDev(const char *linkdev, - int32_t *vf, - char **physfndev) -{ - int rc = -1; - - if (virNetDevIsVirtualFunction(linkdev) == 1) { - /* if linkdev is SR-IOV VF, then set vf = VF index */ - /* and set linkdev = PF device */ - - rc = virNetDevGetPhysicalFunction(linkdev, physfndev); - if (!rc) - rc = virNetDevGetVirtualFunctionIndex(*physfndev, linkdev, vf); - } else { - - /* Not SR-IOV VF: physfndev is linkdev and VF index - * refers to linkdev self - */ - - *vf = PORT_SELF_VF; - *physfndev = strdup(linkdev); - if (!*physfndev) { - virReportOOMError(); - goto err_exit; - } - rc = 0; - } - -err_exit: - - return rc; -} - - /* Returns 0 on success, -1 on general failure, and -2 on timeout */ static int virNetDevVPortProfileOp8021Qbh(const char *ifname, const unsigned char *macaddr, + int32_t vf, const virNetDevVPortProfilePtr virtPort, const unsigned char *vm_uuid, enum virNetDevVPortProfileLinkOp virtPortOp) @@ -864,18 +709,36 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, int rc = 0; char *physfndev = NULL; unsigned char hostuuid[VIR_UUID_BUFLEN]; - int32_t vf; bool nltarget_kernel = true; int ifindex; int vlanid = -1; + bool is_vf = false; - rc = virNetDevVPortProfileGetPhysfnDev(ifname, &vf, &physfndev); - if (rc < 0) - goto err_exit; + if (vf == -1) { + int isvf_ret = virNetDevIsVirtualFunction(ifname); + + if (isvf_ret == -1) + goto cleanup; + is_vf = !!isvf_ret; + } + + if (is_vf) { + if (virNetDevGetVirtualFunctionInfo(ifname, &physfndev, &vf) < 0) { + rc = -1; + goto cleanup; + } + } else { + physfndev = strdup(ifname); + if (!physfndev) { + virReportOOMError(); + rc = -1; + goto cleanup; + } + } rc = virNetDevGetIndex(physfndev, &ifindex); if (rc < 0) - goto err_exit; + goto cleanup; switch (virtPortOp) { case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR: @@ -883,7 +746,7 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, errno = virGetHostUUID(hostuuid); if (errno) { rc = -1; - goto err_exit; + goto cleanup; } rc = virNetDevVPortProfileOpCommon(NULL, ifindex, @@ -935,7 +798,7 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, rc = -1; } -err_exit: +cleanup: VIR_FREE(physfndev); return rc; } @@ -963,6 +826,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macvtap_macaddr, const char *linkdev, + int vf, const unsigned char *vmuuid, enum virNetDevVPortProfileOp vmOp, bool setlink_only) @@ -970,7 +834,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, int rc = 0; VIR_DEBUG("Associating port profile '%p' on link device '%s'", - virtPort, macvtap_ifname); + virtPort, (macvtap_ifname ? macvtap_ifname : linkdev)); VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__, virNetDevVPortProfileOpTypeToString(vmOp)); @@ -985,7 +849,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, case VIR_NETDEV_VPORT_PROFILE_8021QBG: rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, - virtPort, + vf, virtPort, (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE : VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE, @@ -993,7 +857,7 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname, break; case VIR_NETDEV_VPORT_PROFILE_8021QBH: - rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, + rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, vf, virtPort, vmuuid, (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR @@ -1026,6 +890,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macvtap_macaddr, const char *linkdev, + int vf, enum virNetDevVPortProfileOp vmOp) { int rc = 0; @@ -1045,7 +910,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, break; case VIR_NETDEV_VPORT_PROFILE_8021QBG: - rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, + rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, vf, virtPort, VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE, false); break; @@ -1055,7 +920,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname, if (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) break; ignore_value(virNetDevSetOnline(linkdev, false)); - rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, + rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, vf, virtPort, NULL, VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE); break; @@ -1069,6 +934,7 @@ int virNetDevVPortProfileAssociate(const char *macvtap_ifname ATTRIBUTE_UNUSED, const virNetDevVPortProfilePtr virtPort ATTRIBUTE_UNUSED, const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, + int vf ATTRIBUTE_UNUSED, const unsigned char *vmuuid ATTRIBUTE_UNUSED, enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED, bool setlink_only ATTRIBUTE_UNUSED) @@ -1082,6 +948,7 @@ int virNetDevVPortProfileDisassociate(const char *macvtap_ifname ATTRIBUTE_UNUSE const virNetDevVPortProfilePtr virtPort ATTRIBUTE_UNUSED, const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED, const char *linkdev ATTRIBUTE_UNUSED, + int vf ATTRIBUTE_UNUSED, enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index eb9f2b1..dff5943 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -85,18 +85,20 @@ int virNetDevVPortProfileAssociate(const char *ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macaddr, const char *linkdev, + int vf, const unsigned char *vmuuid, enum virNetDevVPortProfileOp vmOp, bool setlink_only) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) - ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6) + ATTRIBUTE_RETURN_CHECK; int virNetDevVPortProfileDisassociate(const char *ifname, const virNetDevVPortProfilePtr virtPort, const unsigned char *macaddr, const char *linkdev, + int vf, enum virNetDevVPortProfileOp vmOp) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;