[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 03/05/2012 08:12 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roprabhu cisco com>
>
> 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

ACK. (I'm shortening the summary line so that it's < 72 columns :-)

I'll push this along with the others after I review 4/4.
>
> Signed-off-by: Roopa Prabhu <roprabhu cisco com>
> ---
>  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;
>  
>  
>
>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]