On Tue, Feb 21, 2012 at 11:53 AM, Ansis Atteka <aatteka(a)nicira.com> wrote:
On Mon, Feb 20, 2012 at 1:46 AM, Daniel P. Berrange <berrange(a)redhat.com>wrote:
> On Sat, Feb 18, 2012 at 10:07:45PM -0500, Laine Stump wrote:
> > On 02/17/2012 02:51 PM, Ansis Atteka wrote:
> > >
> > >
> > > On Fri, Feb 17, 2012 at 10:55 AM, Laine Stump <laine(a)laine.org
> > > <mailto:laine@laine.org>> wrote:
> > >
> > > On 02/16/2012 06:49 PM, Ansis Atteka wrote:
> > > > Currently libvirt sets the attached-mac to altered MAC address
> > > that has
> > > > first byte set to FE. This patch will change that behavior by
> > > using the
> > > > original (unaltered) MAC address from the domain XML
> > > configuration file.
> > >
> > > Maybe I didn't read thoroughly enough, but I don't see where
it
> > > changes
> > > the behavior - in the cases where previously the first byte was
> set to
> > > 0xFE, now you send discourage=true, and in the cases where it
> didn't,
> > > now you send discourage=false.
> > >
> > > "discourage" means whether bridge should be discouraged to use
the
> > > newly added
> > > TAP device's MAC address. Libvirt does that by setting the first MAC
> > > address byte
> > > high enough.
> > >
> > > And here is how this patch works:
> > >
> > > 1. Now in virNetDevTapCreateInBridgePort() function we always pass
> > > exactly the same MAC address that was defined in XML.
> > > 2. If "discourage" flag was set to true, then we create a copy
of MAC
> > > address and set its first byte to 0xFE
> > > 3. virNetDevSetMAC() function would use the MAC address that was
> > > product of #2
> > > 4. 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)
> > >
> >
> >
> > Right. That's what I missed - all I saw was every occurrence of creating
> > a temporary mac address with 0xFE in the first byte replaced with adding
> > "discourage=true" to the args. I didn't notice that
> > virNetDevOpenvswitchAddPort() takes the macaddr (while
> > virNetDevBridgeAddPort() doesn't).
> >
> > But that means that the tap device has been created with an
> > 0xFE-initiated MAC address, and then you attach to the bridge using the
> > unmodified address. Is the issue that the mac address used during the
> > attach needs to match the MAC address that will be in the traffic? Do
> > connections to an openvswitch bridge have an implied MAC filter on them,
> > such that only that MAC address gets through?
> >
> > (Also, the only time discourage is false is for libvirt's virtual
> > network bridges. I'm wondering if they could also use the modified MAC
> > address for the tap devices - if that was the case we could just always
> > create the temporary MAC address in virNetDevTapCreateInBridgePort()
> > (and always set the tap device's mac to that).)
> >
> > Let's let this sit over the weekend and see if anyone else has a
> > brilliant idea/insight.
>
> The reason for setting the top byte to 0xFE, was that the bridge
> device will initialize its own MAC address, to the numerically
> lowest MAC address of all interfaces that are attached. Every
> time the bridge MAC address changes, we got a network blackout
> of something like 30-60 seconds IIRC. Setting the guest MAC
> address to 0xFE ensured that the bridge acquired the MAC address
> of a physical NIC, and ignored the guest NICs.
>
This behavior applies to OVS as well. So we must set MAC address top
byte to 0xFE also for TAP devices that get attached to OVS bridges.
>
> So before we can go with this patch we need to determine what
> the OpenVSwitch behaviour is wrt bridge MAC addresses, otherwise
> this patch could be just reintroducing the problem we solved
> with this 0xFE byte setting.
>
Sorry for the confusion. I think I needed to to make it clear that this
"attached-mac" address has nothing to do with MAC address that
gets assigned to the TAP devices.
After applying this patch the TAP devices would still get the same
FE:XX:XX:XX:XX:XX MAC addresses (irrelevant whether Linux
Bridge or OVS bridge). The only thing that would change after
applying this patch would be that "attached-mac" (see where
we invoke ovs-vsctl command) would match the original MAC
address that was defined in Domain XML file.
By the way, the only consumer of this "attached-mac" is OpenFlow
Controller, which must see the MAC address that is being used by
the guest and not the one that gets assigned to the TAP device with
highest byte set to 0xFE.
Wanted to bump up this patch to see if there are still any concerns?
Not sure if I am missing something, but all this patch does is set the
external-ids:attached-mac field to the same MAC address that guest will
be using. It should not interfere with what MAC address gets assigned to
TAP devices or Bridges on Hypervisor.