Scott Feldman <scofeldm@cisco.com> wrote on 05/20/2010 03:22:12 PM:

>
> Next steps for V3, etc:
>
> 1) merge with Stefan's latest patch, as much as possible

Yes. Try to make it a patch on top of my V5 patch...

> 2) assign VM device mac addr to eth and set macvtap mac addr to random

Hm, why is this necessary? I have tested the macvtap device and always set it to the address that libvirt assigned to it and that happened to be the same as the one inside the VM? Is this something specific to your underlying driver?

> 3) figure out where host_uuid lives

I posted a message a while ago. It's not anywhere right now and should probably be provided via libvirt.conf overriding dmidecode, but dmidecode being used if its output is valid and resorting to a temporary (between libvirt restart) host uuid if everything fails...


> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 1f8dd29..7803bc8 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -85,14 +85,14 @@ static void nlClose(int fd)
>   * buffer will be returned.
>   */
>  static
> -int nlComm(struct nlmsghdr *nlmsg,
> +int nlComm(struct nlmsghdr *nlmsg, int nlgroups,
>             char **respbuf, int *respbuflen)
>  {
>      int rc = 0;
>      struct sockaddr_nl nladdr = {
>              .nl_family = AF_NETLINK,
>              .nl_pid    = 0,
> -            .nl_groups = 0,
> +            .nl_groups = nlgroups,

I think if only you use this function, nlgroups can be 0 here. I would use different code to use multicast for talking to lldpad.

>      };
>      int rcvChunkSize = 1024; // expecting less than that
>      int rcvoffset = 0;
> @@ -192,6 +192,27 @@ nlAppend(struct nlmsghdr *nlm, int totlen,
> const void *data, int datalen)
>      return pos;
>  }
>  
> +#define NLMSG_TAIL(nmsg) \
> +  ((struct rtattr *) (((char *) (nmsg)) + NLMSG_ALIGN((nmsg)->nlmsg_len)))
> +
> +static struct rtattr *nlNest(struct nlmsghdr *nlm, int totlen, int type)
> +{
> +    struct rtattr *nest = NLMSG_TAIL(nlm);
> +
> +    if (nlm->nlmsg_len + NLMSG_ALIGN(sizeof(*nest)) > totlen)
> +   return NULL;
> +    nest->rta_type = type;
> +    nest->rta_len = RTA_LENGTH(0);
> +    nlm->nlmsg_len += sizeof(*nest);
> +    nlAlign(nlm);
> +    return nest;
> +}
> +
> +static int nlNestEnd(struct nlmsghdr *nlm, struct rtattr *nest)
> +{
> +    nest->rta_len = (char *)NLMSG_TAIL(nlm) - (char *)nest;
> +    return nlm->nlmsg_len;
> +}
>  

At some point in the future someone may want to change all this netlink stuff to use libnl -- except for maybe in the case of the multicast message that I am sending in one of my patches and using select with a timeout to wait for a response from (future) lldpad where I am not sure whether an equivalent function exists in libnl...

> +#if 0
> +static int
> +get_host_uuid(char *host_uuid, int len)

this is not active code ... skipping it

> +
> +static int
> +portProfileIdMcast(const char *linkdev,
> +                   const char request,
> +                   const char *profileid,
> +                   const unsigned short *instance_uuid,
> +                   const unsigned short *host_uuid)
> +{
> +    struct rtattr *port_self;
> +    char nlmsgbuf[512];
> +    struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
> +    struct ifinfomsg infomsg = { .ifi_family = AF_UNSPEC };
> +    char rtattbuf[256];
> +    struct rtattr *rta;
> +    char *recvbuf = NULL;
> +    int recvbuflen;
> +    struct nlmsgerr *err;
> +    int rc = 0;
> +
> +    memset(&nlmsgbuf, 0, sizeof(nlmsgbuf));
> +    nlInit(nlm, NLM_F_REQUEST, RTM_SETLINK);
> +
> +    if (!nlAppend(nlm, sizeof(nlmsgbuf), &infomsg, sizeof(infomsg)))
> +        goto buffer_too_small;
> +
> +    rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME,
> +                       linkdev, strlen(linkdev) + 1);
> +    if (!rta)
> +        goto buffer_too_small;
> +
> +    if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> +        goto buffer_too_small;
> +
> +    port_self = nlNest(nlm, sizeof(nlmsgbuf), IFLA_PORT_SELF);
> +    if (!port_self)
> +        return -1;
> +
> +    switch (request) {
> +    case PORT_REQUEST_ASSOCIATE:
> +        if (profileid && strlen(profileid)) {
> +            rta = rtattrCreate(rtattbuf, sizeof(rtattbuf),
> +                               IFLA_PORT_PROFILE,
> +                               profileid, strlen(profileid) + 1);
> +            if (!rta)
> +                goto buffer_too_small;
> +
> +            if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> +                goto buffer_too_small;
> +        }
> +        if (instance_uuid) {
> +            rta = rtattrCreate(rtattbuf, sizeof(rtattbuf),
> +                               IFLA_PORT_INSTANCE_UUID,
> +                               instance_uuid, PORT_UUID_MAX);
> +            if (!rta)
> +                goto buffer_too_small;
> +
> +            if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> +                goto buffer_too_small;
> +        }
> +        if (host_uuid) {
> +            rta = rtattrCreate(rtattbuf, sizeof(rtattbuf),
> +                               IFLA_PORT_HOST_UUID,
> +                               host_uuid, PORT_UUID_MAX);
> +            if (!rta)
> +                goto buffer_too_small;
> +
> +            if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> +                goto buffer_too_small;
> +        }
> +        break;
> +    case PORT_REQUEST_DISASSOCIATE:
> +        break;
> +    }
> +
> +    rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_REQUEST,
> +                       &request, 1);
> +    if (!rta)
> +        goto buffer_too_small;
> +
> +    if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> +        goto buffer_too_small;
> +
> +    nlNestEnd(nlm, port_self);
> +
> +    if (nlComm(nlm, RTNLGRP_LINK, &recvbuf, &recvbuflen) < 0)
> +        return -1;
> +

I know that it doesn't work like this in the netlink multicast case where libvirt will receive an error from the kernel and hopefully some time later a response from lldpad. Still it would be nice if we could use common code here for both technologies, but the unicast vs. multicast receiver code makes it quite different. Have a look at my patches.

    Stefan