[Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Paolo Bonzini]

Linux uses the lowest enslaved MAC address as the MAC address of
the bridge.  Set MAC address to a high value so that it does not
affect the MAC address of the bridge.

Changing the MAC address of the bridge could cause a few seconds
of network downtime.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-bridge-helper.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 287bfd5..6a0974e 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -367,6 +367,24 @@ int main(int argc, char **argv)
         goto cleanup;
     }
 
+    /* Linux uses the lowest enslaved MAC address as the MAC address of
+     * the bridge.  Set MAC address to a high value so that it doesn't
+     * affect the MAC address of the bridge.
+     */
+    if (ioctl(ctlfd, SIOCGIFHWADDR, &ifr) < 0) {
+        fprintf(stderr, "failed to get MAC address of device `%s': %s\n",
+                iface, strerror(errno));
+        ret = EXIT_FAILURE;
+        goto cleanup;
+    }
+    ifr.ifr_hwaddr.sa_data[0] = 0xFE;
+    if (ioctl(ctlfd, SIOCSIFHWADDR, &ifr) < 0) {
+        fprintf(stderr, "failed to set MAC address of device `%s': %s\n",
+                iface, strerror(errno));
+        ret = EXIT_FAILURE;
+        goto cleanup;
+    }
+
     /* add the interface to the bridge */
     prep_ifreq(&ifr, bridge);
     ifindex = if_nametoindex(iface);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Corey Bryant]

On 03/22/2013 12:57 PM, Paolo Bonzini wrote:
> Linux uses the lowest enslaved MAC address as the MAC address of
> the bridge.  Set MAC address to a high value so that it does not
> affect the MAC address of the bridge.
>
> Changing the MAC address of the bridge could cause a few seconds
> of network downtime.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   qemu-bridge-helper.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index 287bfd5..6a0974e 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -367,6 +367,24 @@ int main(int argc, char **argv)
>           goto cleanup;
>       }
>
> +    /* Linux uses the lowest enslaved MAC address as the MAC address of
> +     * the bridge.  Set MAC address to a high value so that it doesn't
> +     * affect the MAC address of the bridge.
> +     */
> +    if (ioctl(ctlfd, SIOCGIFHWADDR, &ifr) < 0) {
> +        fprintf(stderr, "failed to get MAC address of device `%s': %s\n",
> +                iface, strerror(errno));
> +        ret = EXIT_FAILURE;
> +        goto cleanup;
> +    }
> +    ifr.ifr_hwaddr.sa_data[0] = 0xFE;
> +    if (ioctl(ctlfd, SIOCSIFHWADDR, &ifr) < 0) {
> +        fprintf(stderr, "failed to set MAC address of device `%s': %s\n",
> +                iface, strerror(errno));
> +        ret = EXIT_FAILURE;
> +        goto cleanup;
> +    }
> +
>       /* add the interface to the bridge */
>       prep_ifreq(&ifr, bridge);
>       ifindex = if_nametoindex(iface);
>

Is it desirable to change a mac address under the covers?  I know you 
mentioned that libvirt does something like this already.

Also it seems like this might be better if it was optional. 
qemu-bridge-helper can take command line options like this:

-net tap,helper="/usr/local/libexec/qemu-bridge-helper --br=br0"

Perhaps adding a --macaddr option is a better approach?

-- 
Regards,
Corey Bryant

Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Paolo Bonzini]

Il 22/03/2013 22:37, Corey Bryant ha scritto:
> Is it desirable to change a mac address under the covers?

This is the TAP mac address.  It is unrelated to the guest's MAC
address.  It is a random link-local address, all this patch does is make
it less random.

> Also it seems like this might be better if it was optional.
> qemu-bridge-helper can take command line options like this:
> 
> -net tap,helper="/usr/local/libexec/qemu-bridge-helper --br=br0"
> 
> Perhaps adding a --macaddr option is a better approach?

Adding a --macaddr=00:11:... option to force a _particular_ MAC address
could also be useful (if only to help associating guests' MAC addresses
with tap interfaces; the only difference would be the initial 0xFE
byte).  But it is not easily done in QEMU because MAC addresses are a
-device option, not a -netdev option.

This patch fixes the real issue that, when the bridge changes MAC
address, there is a small but significant network downtime.  See libvirt
commit 6ea90b843eff95be6bcbb49a327656fc6f6445ef for reference.

What would you make it conditional on?

Paolo

Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Corey Bryant]

On 03/22/2013 06:09 PM, Paolo Bonzini wrote:
> Il 22/03/2013 22:37, Corey Bryant ha scritto:
>> Is it desirable to change a mac address under the covers?
>
> This is the TAP mac address.  It is unrelated to the guest's MAC
> address.  It is a random link-local address, all this patch does is make
> it less random.
>

Ah, okay then my concern isn't a concern any longer.

-- 
Regards,
Corey Bryant

Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Corey Bryant]

On 03/22/2013 12:57 PM, Paolo Bonzini wrote:
> Linux uses the lowest enslaved MAC address as the MAC address of
> the bridge.  Set MAC address to a high value so that it does not
> affect the MAC address of the bridge.
>
> Changing the MAC address of the bridge could cause a few seconds
> of network downtime.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   qemu-bridge-helper.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index 287bfd5..6a0974e 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -367,6 +367,24 @@ int main(int argc, char **argv)
>           goto cleanup;
>       }
>
> +    /* Linux uses the lowest enslaved MAC address as the MAC address of
> +     * the bridge.  Set MAC address to a high value so that it doesn't
> +     * affect the MAC address of the bridge.
> +     */
> +    if (ioctl(ctlfd, SIOCGIFHWADDR, &ifr) < 0) {
> +        fprintf(stderr, "failed to get MAC address of device `%s': %s\n",
> +                iface, strerror(errno));
> +        ret = EXIT_FAILURE;
> +        goto cleanup;
> +    }
> +    ifr.ifr_hwaddr.sa_data[0] = 0xFE;
> +    if (ioctl(ctlfd, SIOCSIFHWADDR, &ifr) < 0) {
> +        fprintf(stderr, "failed to set MAC address of device `%s': %s\n",
> +                iface, strerror(errno));
> +        ret = EXIT_FAILURE;
> +        goto cleanup;
> +    }
> +
>       /* add the interface to the bridge */
>       prep_ifreq(&ifr, bridge);
>       ifindex = if_nametoindex(iface);
>

Reviewed-by: Corey Bryant <coreyb@linux.vnet.ibm.com>

-- 
Regards,
Corey Bryant

Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori