[libvirt] [PATCH v2]: set and restore MAC address of a NIC when using PASSTHROUGH mode

[Stefan Berger]

On 06/16/2011 02:16 PM, Daniel P. Berrange wrote:
> On Thu, Jun 16, 2011 at 02:23:07PM +0200, Gerhard Stenzel wrote:
>
>> Index: libvirt/src/util/macvtap.c
>> ===================================================================
>> --- libvirt.orig/src/util/macvtap.c
>> +++ libvirt/src/util/macvtap.c
>> @@ -87,6 +87,7 @@
>>
>>   # define LLDPAD_PID_FILE  "/var/run/lldpad.pid"
>>
>> +#define MACADDRSIZE 6
>>
>>   enum virVirtualPortOp {
>>       ASSOCIATE = 0x1,
>> @@ -191,6 +192,149 @@ err_exit:
>>
>>   # if WITH_MACVTAP
>>
>> +/**
>> + * getMacaddr:
>> + * Get the MAC address of a network device
>> + *
>> + * @macaddress: Pointer where the MAC address will be stored
>> + * @srcdev: The interface name of the NIC to get the MAC from
>> + *
>> + * Returns zero in case of success,
>> + * negative value otherwise with error reported.
>> + *
>> + */
>> +int
>> +getMacaddr(const unsigned char *macaddress, const char *srcdev )
>> +{
>> +    int sockfd;
>> +    int io;
>> +    struct ifreq ifr;
>> +
>> +    strcpy(ifr.ifr_name, srcdev);
>> +
>> +    sockfd = socket(AF_INET, SOCK_STREAM, 0);
>> +    if(sockfd<  0){
>> +        return -1;
>> +    }
>> +
>> +    io = ioctl(sockfd, SIOCGIFHWADDR, (char *)&ifr);
>> +    if(io<  0){
>> +        return -1;
>> +    }
>> +
>> +    memcpy(macaddress, ifr.ifr_ifru.ifru_hwaddr.sa_data, MACADDRSIZE);
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>> + * setMacaddr:
>> + * Set the MAC address of a network device
>> + *
>> + * @macaddress: MAC address to assign to the NIC
>> + * @srcdev: The interface name of the NIC
>> + *
>> + * Returns zero in case of success,
>> + * negative value otherwise with error reported.
>> + *
>> + */
>> +int
>> +setMacaddr(const unsigned char *macaddress, const char *srcdev )
>> +{
>> +    int rc = 0;
>> +    struct nlmsghdr *resp;
>> +    struct nlmsgerr *err;
>> +    struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
>> +    int ifindex;
>> +    unsigned char *recvbuf = NULL;
>> +    unsigned int recvbuflen;
>> +    struct nl_msg *nl_msg;
>> +
>> +    if (ifaceGetIndex(true, srcdev,&ifindex) != 0)
>> +        return -1;
>> +
>> +    nl_msg = nlmsg_alloc_simple(RTM_SETLINK, 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 (nla_put_u32(nl_msg, IFLA_LINK, ifindex)<  0)
>> +        goto buffer_too_small;
>> +
>> +    if (nla_put(nl_msg, IFLA_ADDRESS, MACADDRSIZE, macaddress)<  0)
>> +        goto buffer_too_small;
>> +
>> +    if (srcdev&&
>> +        nla_put(nl_msg, IFLA_IFNAME, strlen(srcdev)+1, srcdev)<  0)
>> +        goto buffer_too_small;
>> +
>> +    if (nlComm(nl_msg,&recvbuf,&recvbuflen, 0)<  0) {
>> +        rc = -1;
>> +        goto err_exit;
>> +    }
>> +
>> +    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;
>> +
>> +        switch (err->error) {
>> +
>> +        case 0:
>> +            break;
>> +
>> +        case -EEXIST:
>> +            rc = -1;
>> +            break;
>> +
>> +        default:
>> +            macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                         _("error setting device mac address"));
>> +            rc = -1;
>> +        }
>> +        break;
>> +
>> +    case NLMSG_DONE:
>> +        break;
>> +
>> +    default:
>> +        goto malformed_resp;
>> +    }
>> +
>> +err_exit:
>> +    nlmsg_free(nl_msg);
>> +
>> +    VIR_FREE(recvbuf);
>> +
>> +    return rc;
>> +
>> +malformed_resp:
>> +    nlmsg_free(nl_msg);
>> +
>> +    macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                 _("malformed netlink response message"));
>> +    VIR_FREE(recvbuf);
>> +    return -1;
>> +
>> +buffer_too_small:
>> +    nlmsg_free(nl_msg);
>> +
>> +    macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                 _("allocated netlink buffer is too small"));
>> +    return -1;
>> +}
> There are already (static) functions in src/util/bridge.c, which are
> able to get and set the MAC address on network devices. So I think we
> should just make them non-static and call them instead of duplicating
> it here.
The problem with bridge.c is the brControl structure that's private.

I think the above 2 functions should be moved to util/interface.c since 
they can become generic interface functions like the other ones already 
found there. There they should then be prefixed with 'iface'. If there 
is duplicate functionality in bridge.c, maybe bridge.c should be built 
on top of interface.c and then can keep its brControl structure private.

    Stefan