Hi Dirk,
Your patch mail is line wrapped. I'd recommend resending your patch
using git send-email if possible, will at least increase the chances
that someone reviews/comments.
Thanks,
Cole
On 06/08/2011 08:10 AM, D. Herrendoerfer wrote:
> Hi to all,
>
> there is a problem present in libvirt that when a PASSTHROUGH mode
> DIRECT
> NIC connection is made the MAC address of the NIC is not automatically
> set
> and reset to the configured VM MAC and back again.
>
> The attached patch fixes this problem by setting and resetting the MAC
> while remembering the previous setting while the VM is running.
> This also works if libvirtd is restarted while the VM is running.
>
> The storing of the previous MAC address is done in a link ( currently
> in /tmp,
> but should probably go into /var/run somewhere ).
> Why a link ? It only uses one inode, no need to waste a whole data block
> for 16 bytes or less. It's also more readable by simply looking at it
> with 'ls -l'.
>
> Best regards
>
> Dirk
>
> Signed-off-by: Dirk Herrendoerfer <d.herrendoerfer at
> herrendoerfer.name>
>
> diff --git a/src/libvirt_macvtap.syms b/src/libvirt_macvtap.syms
> index b48565b..0520cb5 100644
> --- a/src/libvirt_macvtap.syms
> +++ b/src/libvirt_macvtap.syms
> @@ -6,5 +6,7 @@
> # macvtap.h
> delMacvtap;
> openMacvtapTap;
> +get_macaddr;
> +set_macaddr;
> vpAssociatePortProfileId;
> vpDisassociatePortProfileId;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ef2d002..4cddbba 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -118,6 +118,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
> int rc;
> #if WITH_MACVTAP
> char *res_ifname = NULL;
> + unsigned char old_macaddr[6];
> int vnet_hdr = 0;
> int err;
>
> @@ -125,6 +126,53 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
> net->model && STREQ(net->model, "virtio"))
> vnet_hdr = 1;
>
> + /** Note: When using PASSTHROUGH mode with MACVTAP devices the link
> + * devices's MAC address must be set to the VMs MAC address. In
> + * order to not confuse the first switch or bridge in line this MAC
> + * address must be reset when the VM is shut down.
> + * This is especially important when using SRIOV capable cards that
> + * emulate their switch in firmware.
> + */
> + if ( net->data.direct.mode ==
> VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU ) {
> + rc = get_macaddr(&old_macaddr, 6, net->data.direct.linkdev);
> + if (rc) {
> + virReportSystemError(rc,
> + _("Getting MAC address from '%s' "
> + "to '%02x:%02x:%02x:%02x:%02x:%02x'
> failed."),
> + net->data.direct.linkdev,
> +
> old_macaddr[0],old_macaddr[1],old_macaddr[2],
> +
> old_macaddr[3],old_macaddr[4],old_macaddr[5]);
> + } else {
> + char oldmacname[256], newmacname[256];
> +
> + sprintf(oldmacname,"%02x:%02x:%02x:%02x:%02x:%02x",
> + old_macaddr[0],old_macaddr[1],old_macaddr[2],
> + old_macaddr[3],old_macaddr[4],old_macaddr[5]);
> +
> + sprintf(newmacname,"/tmp/%s@%02x:%02x:%02x:%02x:%02x:%02x",
> + net->data.direct.linkdev,
> + net->mac[0],net->mac[1],net->mac[2],
> + net->mac[3],net->mac[4],net->mac[5]);
> +
> + rc = symlink (oldmacname, newmacname);
> + if (rc) {
> + virReportSystemError(rc,
> + _("MAC link file creation failed
> for %s."),
> + net->data.direct.linkdev);
> + }
> + }
> +
> + rc = set_macaddr(net->mac, 6, net->data.direct.linkdev);
> + if (rc) {
> + virReportSystemError(rc,
> + _("Setting MAC address on '%s' to "
> + "'%02x:%02x:%02x:%02x:%02x:%02x'
> failed."),
> + net->data.direct.linkdev,
> + net->mac[0],net->mac[1],net->mac[2],
> + net->mac[3],net->mac[4],net->mac[5]);
> + }
> + }
> +
> rc = openMacvtapTap(net->ifname, net->mac, net-
> >data.direct.linkdev,
> net->data.direct.mode, vnet_hdr, def->uuid,
> &net->data.direct.virtPortProfile,
> &res_ifname,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1efe024..127acb8 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2663,6 +2663,51 @@ void qemuProcessStop(struct qemud_driver *driver,
> if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> delMacvtap(net->ifname, net->mac, net-
> >data.direct.linkdev,
> &net->data.direct.virtPortProfile);
> +
> + if ( net->data.direct.mode ==
> VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU ) {
> + char newmacname[256], linkdata[64];
> + unsigned int old_macaddr[6];
> +
> + sprintf(newmacname,"/tmp/%s@%02x:%02x:%02x:%02x:%02x:
> %02x",
> + net->data.direct.linkdev,
> + net->mac[0],net->mac[1],net->mac[2],
> + net->mac[3],net->mac[4],net->mac[5]);
> +
> + ret = readlink (newmacname, linkdata, 64);
> + if ( ret == 17 ) {
> + linkdata[17] = 0;
> +
> + ret = sscanf(linkdata,"%x:%x:%x:%x:%x:%x",
> +
> &old_macaddr[0],&old_macaddr[1],&old_macaddr[2],
> +
> &old_macaddr[3],&old_macaddr[4],&old_macaddr[5]);
> + if ( ret == 6 ) {
> + unsigned char oldmac[6];
> + oldmac[0] = (unsigned char)(old_macaddr[0] & 0xFF) ;
> + oldmac[1] = (unsigned char)(old_macaddr[1] & 0xFF) ;
> + oldmac[2] = (unsigned char)(old_macaddr[2] & 0xFF) ;
> + oldmac[3] = (unsigned char)(old_macaddr[3] & 0xFF) ;
> + oldmac[4] = (unsigned char)(old_macaddr[4] & 0xFF) ;
> + oldmac[5] = (unsigned char)(old_macaddr[5] & 0xFF) ;
> +
> + VIR_WARN(("Resetting MAC address on '%s' "
> + "to '%02x:%02x:%02x:%02x:%02x:
> %02x'."),
> + net->data.direct.linkdev,
> + oldmac[0],oldmac[1],oldmac[2],
> + oldmac[3],oldmac[4],oldmac[5]);
> + /*reset mac and remove link file-ignore
> restults*/
> + ret = set_macaddr(oldmac, 6, net-
> >data.direct.linkdev);
> + ret = unlink(newmacname);
> + } else {
> + VIR_WARN(("Scanning MAC address from '%s' "
> + "failed ! Got %i values."),
> + linkdata, ret);
> + }
> + } else {
> + VIR_WARN(("Reading MAC address from '%s' failed ! "
> + "Got %i bytes."),
> + newmacname, ret);
> + }
> + }
> VIR_FREE(net->ifname);
> }
> }
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 068638e..1026a84 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -191,6 +191,155 @@ err_exit:
>
> # if WITH_MACVTAP
>
> +/**
> + * get_macaddr:
> + * Get the MAC address of a network device
> + *
> + * @macaddress: Pointer where the MAC address will be stored
> + * @macaddrsize: Size of the MAC address.
> + * @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
> +get_macaddr(const unsigned char *macaddress, int macaddrsize,
> + const char *srcdev )
> +{
> + int sockfd;
> + int io;
> + char buffer[1024];
> + 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;
> + }
> +
> + bcopy( ifr.ifr_ifru.ifru_hwaddr.sa_data, macaddress, macaddrsize);
> +
> + return 0;
> +}
> +
> +/**
> + * Set_macaddr:
> + * Set the MAC address of a network device
> + *
> + * @macaddress: MAC address to assign to the NIC
> + * @macaddrsize: Size of the MAC address.
> + * @srcdev: The interface name of the NIC
> + *
> + * Returns zero in case of success,
> + * negative value otherwise with error reported.
> + *
> + */
> +int
> +set_macaddr(const unsigned char *macaddress, int macaddrsize,
> + 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;
> + struct nlattr *linkinfo;
> +
> + 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;
> +}
> +
> static int
> link_add(const char *type,
> const unsigned char *macaddress, int macaddrsize,
> diff --git a/src/util/macvtap.h b/src/util/macvtap.h
> index a1383c4..d71546a 100644
> --- a/src/util/macvtap.h
> +++ b/src/util/macvtap.h
> @@ -75,6 +75,12 @@ enum virVMOperationType {
>
> # include "internal.h"
>
> +int get_macaddr(const unsigned char *macaddress, int macaddrsize,
> + const char *srcdev );
> +
> +int set_macaddr(const unsigned char *macaddress, int macaddrsize,
> + const char *srcdev );
> +
> int openMacvtapTap(const char *ifname,
> const unsigned char *macaddress,
> const char *linkdev,
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list