<br><br><div class="gmail_quote">On Fri, Feb 17, 2012 at 10:55 AM, Laine Stump <span dir="ltr"><<a href="mailto:laine@laine.org">laine@laine.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 02/16/2012 06:49 PM, Ansis Atteka wrote:<br>
> Currently libvirt sets the attached-mac to altered MAC address that has<br>
> first byte set to FE. This patch will change that behavior by using the<br>
> original (unaltered) MAC address from the domain XML configuration file.<br>
<br>
</div>Maybe I didn't read thoroughly enough, but I don't see where it changes<br>
the behavior - in the cases where previously the first byte was set to<br>
0xFE, now you send discourage=true, and in the cases where it didn't,<br>
now you send discourage=false.<br></blockquote><div>"discourage" means whether bridge should be discouraged to use the newly added<br>TAP device's MAC address. Libvirt does that by setting the first MAC address byte<br>
high enough.<br><br>And here is how this patch works:<br><ol><li>Now in virNetDevTapCreateInBridgePort() function we always pass exactly the same MAC address that was defined in XML.<br></li><li>If "discourage" flag was set to true, then we create a copy of MAC address and set its first byte to 0xFE<br>
</li><li>virNetDevSetMAC() function would use the MAC address that was product of #2</li><li>while virNetDevOpenvswitchAddPort() function would use the original MAC address that was passed in #1 (this code did not need to be changed so most likely that was the reason why you did not notice behavior changes)<br>
</li></ol><p></p><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Is this a precursor to something else? Does openvswitch maybe not need<br>
this extremely high MAC address?<br></blockquote><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5"><br>
<br>
<br>
> ---<br>
>  src/network/bridge_driver.c |    2 +-<br>
>  src/qemu/qemu_command.c     |    5 +----<br>
>  src/uml/uml_conf.c          |    5 +----<br>
>  src/util/virnetdevtap.c     |   11 ++++++++++-<br>
>  src/util/virnetdevtap.h     |    1 +<br>
>  5 files changed, 14 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c<br>
> index 8575d3e..3e1e031 100644<br>
> --- a/src/network/bridge_driver.c<br>
> +++ b/src/network/bridge_driver.c<br>
> @@ -1766,7 +1766,7 @@ networkStartNetworkVirtual(struct network_driver *driver,<br>
>          }<br>
>          if (virNetDevTapCreateInBridgePort(network->def->bridge,<br>
>                                             &macTapIfName, network->def->mac,<br>
> -                                           0, false, NULL, NULL) < 0) {<br>
> +                                           false, 0, false, NULL, NULL) < 0) {<br>
>              VIR_FREE(macTapIfName);<br>
>              goto err0;<br>
>          }<br>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c<br>
> index 5a34504..671054c 100644<br>
> --- a/src/qemu/qemu_command.c<br>
> +++ b/src/qemu/qemu_command.c<br>
> @@ -180,7 +180,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,<br>
>      int tapfd = -1;<br>
>      int vnet_hdr = 0;<br>
>      bool template_ifname = false;<br>
> -    unsigned char tapmac[VIR_MAC_BUFLEN];<br>
>      int actualType = virDomainNetGetActualType(net);<br>
><br>
>      if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {<br>
> @@ -244,9 +243,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,<br>
>          net->model && STREQ(net->model, "virtio"))<br>
>          vnet_hdr = 1;<br>
><br>
> -    memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);<br>
> -    tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */<br>
> -    err = virNetDevTapCreateInBridgePort(brname, &net->ifname, tapmac,<br>
> +    err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, true,<br>
>                                           vnet_hdr, true, &tapfd,<br>
>                                           virDomainNetGetActualVirtPortProfile(net));<br>
>      virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);<br>
> diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c<br>
> index dbbbfda..c7b29a0 100644<br>
> --- a/src/uml/uml_conf.c<br>
> +++ b/src/uml/uml_conf.c<br>
> @@ -127,7 +127,6 @@ umlConnectTapDevice(virConnectPtr conn,<br>
>                      const char *bridge)<br>
>  {<br>
>      bool template_ifname = false;<br>
> -    unsigned char tapmac[VIR_MAC_BUFLEN];<br>
><br>
>      if (!net->ifname ||<br>
>          STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||<br>
> @@ -139,9 +138,7 @@ umlConnectTapDevice(virConnectPtr conn,<br>
>          template_ifname = true;<br>
>      }<br>
><br>
> -    memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);<br>
> -    tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */<br>
> -    if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, tapmac,<br>
> +    if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, true,<br>
>                                         0, true, NULL,<br>
>                                         virDomainNetGetActualVirtPortProfile(net)) < 0) {<br>
>          if (template_ifname)<br>
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c<br>
> index 0fce08d..868ba57 100644<br>
> --- a/src/util/virnetdevtap.c<br>
> +++ b/src/util/virnetdevtap.c<br>
> @@ -22,6 +22,7 @@<br>
><br>
>  #include <config.h><br>
><br>
> +#include "virmacaddr.h"<br>
>  #include "virnetdevtap.h"<br>
>  #include "virnetdev.h"<br>
>  #include "virnetdevbridge.h"<br>
> @@ -248,6 +249,7 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)<br>
>   * @brname: the bridge name<br>
>   * @ifname: the interface name (or name template)<br>
>   * @macaddr: desired MAC address (VIR_MAC_BUFLEN long)<br>
> + * @discourage: whether bridge should be discouraged from using macaddr<br>
>   * @vnet_hdr: whether to try enabling IFF_VNET_HDR<br>
>   * @tapfd: file descriptor return value for the new tap device<br>
>   * @virtPortProfile: bridge/port specific configuration<br>
> @@ -265,11 +267,14 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)<br>
>  int virNetDevTapCreateInBridgePort(const char *brname,<br>
>                                     char **ifname,<br>
>                                     const unsigned char *macaddr,<br>
> +                                   bool discourage,<br>
>                                     int vnet_hdr,<br>
>                                     bool up,<br>
>                                     int *tapfd,<br>
>                                     virNetDevVPortProfilePtr virtPortProfile)<br>
>  {<br>
> +    unsigned char tapmac[VIR_MAC_BUFLEN];<br>
> +<br>
>      if (virNetDevTapCreate(ifname, vnet_hdr, tapfd) < 0)<br>
>          return -1;<br>
><br>
> @@ -279,7 +284,11 @@ int virNetDevTapCreateInBridgePort(const char *brname,<br>
>       * seeing the kernel allocate random MAC for the TAP<br>
>       * device before we set our static MAC.<br>
>       */<br>
> -    if (virNetDevSetMAC(*ifname, macaddr) < 0)<br>
> +    memcpy(tapmac, macaddr, VIR_MAC_BUFLEN);<br>
> +    if (discourage)<br>
> +        tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */<br>
> +<br>
> +    if (virNetDevSetMAC(*ifname, tapmac) < 0)<br>
>          goto error;<br>
><br>
>      /* We need to set the interface MTU before adding it<br>
> diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h<br>
> index 918f3dc..fc50e22 100644<br>
> --- a/src/util/virnetdevtap.h<br>
> +++ b/src/util/virnetdevtap.h<br>
> @@ -37,6 +37,7 @@ int virNetDevTapDelete(const char *ifname)<br>
>  int virNetDevTapCreateInBridgePort(const char *brname,<br>
>                                     char **ifname,<br>
>                                     const unsigned char *macaddr,<br>
> +                                   bool discourage,<br>
>                                     int vnet_hdr,<br>
>                                     bool up,<br>
>                                     int *tapfd,<br>
<br>
--<br>
</div></div>libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br>
</blockquote></div><br>