[libvirt] [PATCH 2/4 v3] virtnetdev: Add support functions for mac and portprofile associations on a hostdev

Laine Stump laine at laine.org
Tue Mar 6 09:48:32 UTC 2012


On 03/05/2012 08:12 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roprabhu at cisco.com>
>
> This patch adds the following:
> - functions to set and get vf configs
> - Functions to replace and store vf configs (Only mac address is handled today.
>   But the functions can be easily extended for vlans and other vf configs)
> - function to dump link dev info (This is moved from virnetdevvportprofile.c)

Looks like you addressed all my issues. (I need to add "cleanup return
paths of virnetdev" to my todo list to address the bits of ugliness the
code movement pointed out).

>
> Signed-off-by: Roopa Prabhu <roprabhu at cisco.com>
> ---
>  src/util/virnetdev.c |  535 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virnetdev.h |   19 ++
>  2 files changed, 553 insertions(+), 1 deletions(-)
>
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 4aa7639..9f93fda 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1127,8 +1127,47 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
>  
>      return ret;
>  }
> -#else /* !__linux__ */
>  
> +/**
> + * virNetDevGetVirtualFunctionInfo:
> + * @vfname: name of the virtual function interface
> + * @pfname: name of the physical function
> + * @vf: vf index
> + *
> + * Returns 0 on success, -errno on failure.
> + *
> + */
> +int
> +virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
> +                                int *vf)
> +{
> +    char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
> +    int ret = -1;
> +
> +    *pfname = NULL;
> +
> +    if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)
> +        return ret;
> +
> +    if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0)
> +        goto cleanup;
> +
> +    if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0)
> +        goto cleanup;
> +
> +    ret = pciGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf);
> +
> +cleanup:
> +    if (ret < 0)
> +        VIR_FREE(*pfname);
> +
> +    VIR_FREE(vf_sysfs_path);
> +    VIR_FREE(pf_sysfs_path);
> +
> +    return ret;
> +}

I didn't notice before that this new function wasn't being defined in
the case that HAVE_LIBNL isn't defined. I'm squashing in an empty
function just like all the others.

> +
> +#else /* !__linux__ */
>  int
>  virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED,
>                               char ***vfname ATTRIBUTE_UNUSED,
> @@ -1165,4 +1204,498 @@ virNetDevGetPhysicalFunction(const char *ifname ATTRIBUTE_UNUSED,
>                           _("Unable to get physical function status on this platform"));
>      return -1;
>  }
> +
>  #endif /* !__linux__ */
> +#if defined(__linux__) && defined(HAVE_LIBNL)
> +
> +static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
> +    [IFLA_VF_MAC]       = { .type = NLA_UNSPEC,
> +                            .maxlen = sizeof(struct ifla_vf_mac) },
> +    [IFLA_VF_VLAN]      = { .type = NLA_UNSPEC,
> +                            .maxlen = sizeof(struct ifla_vf_vlan) },
> +};
> +
> +/**
> + * virNetDevLinkDump:
> + *
> + * @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.
> + */
> +int
> +virNetDevLinkDump(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;
> +
> +    if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0)
> +        return -1;
> +
> +    ifinfo.ifi_index = ifindex;
> +
> +    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 (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:
> +        rc = nlmsg_parse(resp, sizeof(struct ifinfomsg),
> +                         tb, IFLA_MAX, NULL);
> +        if (rc < 0)
> +            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;
> +}
> +
> +static int
> +virNetDevSetVfConfig(const char *ifname, int ifindex, int vf,
> +                     bool nltarget_kernel, const unsigned char *macaddr,
> +                     int vlanid, uint32_t (*getPidFunc)(void))
> +{
> +    int rc = -1;
> +    struct nlmsghdr *resp;
> +    struct nlmsgerr *err;
> +    unsigned char *recvbuf = NULL;
> +    unsigned int recvbuflen = 0;
> +    uint32_t pid = 0;
> +    struct nl_msg *nl_msg;
> +    struct nlattr *vfinfolist, *vfinfo;
> +    struct ifinfomsg ifinfo = {
> +        .ifi_family = AF_UNSPEC,
> +        .ifi_index  = ifindex
> +    };
> +
> +    if (!macaddr && vlanid < 0)
> +        return -1;
> +
> +    nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST);
> +    if (!nl_msg) {
> +        virReportOOMError();
> +        return rc;
> +    }
> +
> +    if (nlmsg_append(nl_msg,  &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
> +        goto buffer_too_small;
> +
> +    if (ifname &&
> +        nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
> +        goto buffer_too_small;
> +
> +
> +    if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST)))
> +        goto buffer_too_small;
> +
> +    if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO)))
> +        goto buffer_too_small;
> +
> +    if (macaddr) {
> +        struct ifla_vf_mac ifla_vf_mac = {
> +             .vf = vf,
> +             .mac = { 0, },
> +        };
> +
> +        memcpy(ifla_vf_mac.mac, macaddr, VIR_MAC_BUFLEN);
> +
> +        if (nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
> +                    &ifla_vf_mac) < 0)
> +            goto buffer_too_small;
> +    }
> +
> +    if (vlanid >= 0) {
> +        struct ifla_vf_vlan ifla_vf_vlan = {
> +             .vf = vf,
> +             .vlan = vlanid,
> +             .qos = 0,
> +        };
> +
> +        if (nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan),
> +                    &ifla_vf_vlan) < 0)
> +            goto buffer_too_small;
> +    }
> +
> +    nla_nest_end(nl_msg, vfinfo);
> +    nla_nest_end(nl_msg, vfinfolist);
> +
> +    if (!nltarget_kernel) {
> +        pid = getPidFunc();
> +        if (pid == 0) {
> +            rc = -1;
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, pid) < 0)
> +        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 during set %s of ifindex %d"),
> +                (macaddr ? (vlanid >= 0 ? "mac/vlan" : "mac") : "vlanid"),
> +                ifindex);
> +            goto cleanup;
> +        }
> +        break;
> +
> +    case NLMSG_DONE:
> +        break;
> +
> +    default:
> +        goto malformed_resp;
> +    }
> +
> +    rc = 0;
> +
> +cleanup:
> +    nlmsg_free(nl_msg);
> +
> +    VIR_FREE(recvbuf);
> +
> +    return rc;
> +
> +malformed_resp:
> +    nlmsg_free(nl_msg);
> +
> +    virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                   _("malformed netlink response message"));
> +    VIR_FREE(recvbuf);
> +    return rc;
> +
> +buffer_too_small:
> +    nlmsg_free(nl_msg);
> +
> +    virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                   _("allocated netlink buffer is too small"));
> +    return rc;
> +}
> +
> +static int
> +virNetDevParseVfConfig(struct nlattr **tb, int32_t vf, unsigned char *mac,
> +                       int *vlanid)
> +{
> +    const char *msg = NULL;
> +    int rc = -1;
> +
> +    if (tb[IFLA_VFINFO_LIST]) {
> +        struct ifla_vf_mac *vf_mac;
> +        struct ifla_vf_vlan *vf_vlan;
> +        struct nlattr *tb_vf_info = {NULL, };
> +        struct nlattr *tb_vf[IFLA_VF_MAX+1];
> +        int found = 0;
> +        int rem;
> +
> +        nla_for_each_nested(tb_vf_info, tb[IFLA_VFINFO_LIST], rem) {
> +            if (nla_type(tb_vf_info) != IFLA_VF_INFO)
> +                continue;
> +
> +            if (nla_parse_nested(tb_vf, IFLA_VF_MAX, tb_vf_info,
> +                                 ifla_vf_policy)) {
> +                msg = _("error parsing IFLA_VF_INFO");
> +                goto cleanup;
> +            }
> +
> +            if (tb[IFLA_VF_MAC]) {
> +                vf_mac = RTA_DATA(tb_vf[IFLA_VF_MAC]);
> +                if (vf_mac && vf_mac->vf == vf)  {
> +                    memcpy(mac, vf_mac->mac, VIR_MAC_BUFLEN);
> +                    found = 1;
> +                }
> +            }
> +
> +            if (tb[IFLA_VF_VLAN]) {
> +                vf_vlan = RTA_DATA(tb_vf[IFLA_VF_VLAN]);
> +                if (vf_vlan && vf_vlan->vf == vf)  {
> +                    *vlanid = vf_vlan->vlan;
> +                    found = 1;
> +                }
> +            }
> +            if (found) {
> +                rc = 0;
> +                break;
> +            }
> +        }
> +    }
> +
> +cleanup:
> +    if (msg)
> +        virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s", msg);
> +
> +    return rc;
> +}
> +
> +static int
> +virNetDevGetVfConfig(const char *ifname, int vf, unsigned char *mac,
> +                     int *vlanid)
> +{
> +    int rc = -1;
> +    unsigned char *recvbuf = NULL;
> +    struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
> +    int ifindex = -1;
> +
> +    rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL);
> +    if (rc < 0)
> +        return rc;
> +
> +    rc = virNetDevParseVfConfig(tb, vf, mac, vlanid);
> +
> +    VIR_FREE(recvbuf);
> +
> +    return rc;
> +}
> +
> +static int
> +virNetDevReplaceVfConfig(const char *pflinkdev, int vf,
> +                         const unsigned char *macaddress,
> +                         int vlanid,
> +                         const char *stateDir)
> +{
> +    unsigned char oldmac[6];
> +    int oldvlanid = -1;
> +    char *path = NULL;
> +    char macstr[VIR_MAC_STRING_BUFLEN];
> +    int ifindex = -1;
> +
> +    if (virNetDevGetVfConfig(pflinkdev, vf, oldmac, &oldvlanid) < 0)
> +        return -1;
> +
> +    if (virAsprintf(&path, "%s/%s_vf%d",
> +                    stateDir, pflinkdev, vf) < 0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    virMacAddrFormat(oldmac, macstr);
> +    if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
> +        virReportSystemError(errno, _("Unable to preserve mac for pf = %s,"
> +                             " vf = %d"), pflinkdev, vf);
> +        VIR_FREE(path);
> +        return -1;
> +    }
> +
> +    VIR_FREE(path);
> +
> +    return virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
> +                                macaddress, vlanid, NULL);
> +}
> +
> +static int
> +virNetDevRestoreVfConfig(const char *pflinkdev, int vf,
> +                         const char *stateDir)
> +{
> +    int rc = -1;
> +    char *macstr = NULL;
> +    char *path = NULL;
> +    unsigned char oldmac[6];
> +    int vlanid = -1;
> +    int ifindex = -1;
> +
> +    if (virAsprintf(&path, "%s/%s_vf%d",
> +                    stateDir, pflinkdev, vf) < 0) {
> +        virReportOOMError();
> +        return rc;
> +    }
> +
> +    if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN, &macstr) < 0) {
> +        goto cleanup;
> +    }
> +
> +    if (virMacAddrParse(macstr, &oldmac[0]) != 0) {
> +        virNetDevError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Cannot parse MAC address from '%s'"),
> +                       macstr);
> +        goto cleanup;
> +    }
> +
> +    /*reset mac and remove file-ignore results*/
> +    rc = virNetDevSetVfConfig(pflinkdev, ifindex, vf, true,
> +                              oldmac, vlanid, NULL);
> +    ignore_value(unlink(path));
> +
> +cleanup:
> +    VIR_FREE(path);
> +    VIR_FREE(macstr);
> +
> +    return rc;
> +}
> +
> +/**
> + * virNetDevReplaceNetConfig:
> + * @linkdev: name of the interface
> + * @vf: vf index if linkdev is a pf
> + * @macaddress: new MAC address for interface
> + * @vlanid: new vlanid
> + * @stateDir: directory to store old net config
> + *
> + * Returns 0 on success, -1 on failure
> + *
> + */
> +int
> +virNetDevReplaceNetConfig(char *linkdev, int vf,
> +                          const unsigned char *macaddress, int vlanid,
> +                          char *stateDir)
> +{
> +    if (vf == -1)
> +        return virNetDevReplaceMacAddress(linkdev, macaddress, stateDir);
> +    else
> +        return virNetDevReplaceVfConfig(linkdev, vf, macaddress, vlanid,
> +                                        stateDir);
> +}
> +
> +/**
> + * virNetDevRestoreNetConfig:
> + * @linkdev: name of the interface
> + * @vf: vf index if linkdev is a pf
> + * @stateDir: directory containing old net config
> + *
> + * Returns 0 on success, -errno on failure.
> + *
> + */
> +int
> +virNetDevRestoreNetConfig(char *linkdev, int vf, char *stateDir)
> +{
> +    if (vf == -1)
> +        return virNetDevRestoreMacAddress(linkdev, stateDir);
> +    else
> +        return virNetDevRestoreVfConfig(linkdev, vf, stateDir);
> +}
> +
> +#else /* defined(__linux__) && defined(HAVE_LIBNL) */
> +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)

This function definition was missing an "int" return type at the
beginning. I'm squashing one in.

Also, this function uses struct nlattr, while is defined in
linux/netlink.h, but that isn't #included when HAVE_LIBNL isn't defined.
I added a "forward" declaration of struct nlattr in util/virnetlink.h
(there are already examples of declaring netlink/libnl structs there).
We may later decide we need to use something less platform-specific
there, but for now the function is only useful/used on systems with
libnl anyway, and this fix at least allows it to compile properly on
platforms without libnl.

> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Unable to dump link info on this platform"));
> +    return -1;
> +}
> +
> +int
> +virNetDevReplaceNetConfig(char *linkdev ATTRIBUTE_UNUSED,
> +                          int vf ATTRIBUTE_UNUSED,
> +                          const unsigned char *macaddress ATTRIBUTE_UNUSED,
> +                          int vlanid ATTRIBUTE_UNUSED,
> +                          char *stateDir ATTRIBUTE_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Unable to replace net config on this platform"));
> +    return -1;
> +
> +}
> +
> +int
> +virNetDevRestoreNetConfig(char *linkdev ATTRIBUTE_UNUSED,
> +                          int vf ATTRIBUTE_UNUSED,
> +                          char *stateDir ATTRIBUTE_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Unable to restore net config on this platform"));
> +    return -1;
> +}
> +
> +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */
> diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
> index 3456113..73f4c64 100644
> --- a/src/util/virnetdev.h
> +++ b/src/util/virnetdev.h
> @@ -24,6 +24,7 @@
>  # define __VIR_NETDEV_H__
>  
>  # include "virsocketaddr.h"
> +# include "virnetlink.h"
>  
>  int virNetDevExists(const char *brname)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> @@ -105,4 +106,22 @@ int virNetDevGetVirtualFunctions(const char *pfname,
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>      ATTRIBUTE_RETURN_CHECK;
>  
> +int virNetDevLinkDump(const char *ifname, int ifindex,
> +                      bool nltarget_kernel, struct nlattr **tb,
> +                      unsigned char **recvbuf,
> +                      uint32_t (*getPidFunc)(void))
> +    ATTRIBUTE_RETURN_CHECK;
> +
> +int virNetDevReplaceNetConfig(char *linkdev, int vf,
> +                              const unsigned char *macaddress, int vlanid,
> +                              char *stateDir)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
> +
> +int virNetDevRestoreNetConfig(char *linkdev, int vf, char *stateDir)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
> +
> +int virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
> +                                    int *vf)
> +    ATTRIBUTE_NONNULL(1);
> +
>  #endif /* __VIR_NETDEV_H__ */
>
>

Again, we forgot to add the new global functions to
src/libvirt_private.syms. I'm attaching what I'll squash in before I push.

ACK with the extra bits squashed in.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-squash-into-virtnetdev-Add-support-functions-for-mac.patch
Type: text/x-patch
Size: 2553 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120306/36caa7d0/attachment-0001.bin>


More information about the libvir-list mailing list