From 5b225fe7ac882c8696b9759251c519c785f5407c Mon Sep 17 00:00:00 2001

Message-Id: <5b225fe7ac882c8696b9759251c519c785f5407c@dist-git>

From: Laine Stump <laine@laine.org>

Date: Tue, 6 Mar 2018 16:19:31 -0500

Subject: [PATCH] conf: move 'generated' member from virMacAddr to

 virDomainNetDef

Commit 7e62c4cd26d (first appearing in libvirt-3.9.0 as a resolution

to rhbz #1343919) added a "generated" attribute to virMacAddr that was

set whenever a mac address was auto-generated by libvirt. This

knowledge was used in a single place - when trying to match a NetDef

from the Domain to Delete with user-provided XML. Since the XML parser

always auto-generates a MAC address for NetDefs when none is provided,

it was previously impossible to make a search where the MAC address

isn't significant, but the addition of the "generated" attribute made

it possible for the search function to ignore auto-generated MACs.

This implementation had a problem though - it was adding a field to a

"low level" struct - virMacAddr - which is used in other places with

the assumption that it contains exactly a 6 byte MAC address and

nothing else. In particular, virNWFilterSnoopEthHdr uses virMacAddr as

part of the definition of an ethernet packet header, whose layout must

of course match an actual ethernet packet. Adding the extra bools into

virNWFilterSnoopEthHdr caused the nwfilter driver's "IP discovery via

DHCP packet snooping" functionality to mysteriously stop working.

In order to fix that behavior, and prevent potential future similar

odd behavior, this patch moves the "generated" member out of

virMacAddr (so that it is again really is just a MAC address) into

virDomainNetDef, and sets it only when virDomainNetGenerateMAC() is

called from virDomainNetDefParseXML() (which is the only time we care

about it).

Resolves: https://bugzilla.redhat.com/1529338

(It should also be applied to any maintenance branch that applies

commit 7e62c4cd26 and friends to resolve

https://bugzilla.redhat.com/1343919)

Signed-off-by: Laine Stump <laine@laine.org>

(cherry picked from commit e62cb4a9b78c7f4499a206635fb4f06e6ac627e5)

---

 src/conf/domain_conf.c    | 3 ++-

 src/conf/domain_conf.h    | 1 +

 src/util/virmacaddr.c     | 5 -----

 src/util/virmacaddr.h     | 9 +++++++--

 tests/bhyveargv2xmlmock.c | 1 -

 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 0af3dd5ab6..1a3db4a7be 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -10989,6 +10989,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,

         }

     } else {

         virDomainNetGenerateMAC(xmlopt, &def->mac);

+        def->mac_generated = true;

     }

     if (devaddr) {

@@ -16205,7 +16206,7 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net)

     size_t i;

     int matchidx = -1;

     char mac[VIR_MAC_STRING_BUFLEN];

-    bool MACAddrSpecified = !net->mac.generated;

+    bool MACAddrSpecified = !net->mac_generated;

     bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&net->info,

                                                           VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

index ed7b587bed..3817887322 100644

--- a/src/conf/domain_conf.h

+++ b/src/conf/domain_conf.h

@@ -962,6 +962,7 @@ struct _virDomainActualNetDef {

 struct _virDomainNetDef {

     virDomainNetType type;

     virMacAddr mac;

+    bool mac_generated; /* true if mac was *just now* auto-generated by libvirt */

     char *model;

     union {

         struct {

diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c

index 409fdc34d5..7afe032b9c 100644

--- a/src/util/virmacaddr.c

+++ b/src/util/virmacaddr.c

@@ -107,7 +107,6 @@ void

 virMacAddrSet(virMacAddrPtr dst, const virMacAddr *src)

 {

     memcpy(dst, src, sizeof(*src));

-    dst->generated = false;

 }

 /**

@@ -121,7 +120,6 @@ void

 virMacAddrSetRaw(virMacAddrPtr dst, const unsigned char src[VIR_MAC_BUFLEN])

 {

     memcpy(dst->addr, src, VIR_MAC_BUFLEN);

-    dst->generated = false;

 }

 /**

@@ -151,7 +149,6 @@ virMacAddrParse(const char* str, virMacAddrPtr addr)

 {

     size_t i;

-    addr->generated = false;

     errno = 0;

     for (i = 0; i < VIR_MAC_BUFLEN; i++) {

         char *end_ptr;

@@ -220,7 +217,6 @@ virMacAddrParseHex(const char *str, virMacAddrPtr addr)

         str[VIR_MAC_HEXLEN])

         return -1;

-    addr->generated = false;

     for (i = 0; i < VIR_MAC_BUFLEN; i++)

         addr->addr[i] = (virHexToBin(str[2 * i]) << 4 |

                          virHexToBin(str[2 * i + 1]));

@@ -236,7 +232,6 @@ void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN],

     addr->addr[3] = virRandomBits(8);

     addr->addr[4] = virRandomBits(8);

     addr->addr[5] = virRandomBits(8);

-    addr->generated = true;

 }

 /* The low order bit of the first byte is the "multicast" bit. */

diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h

index ef4285d639..d0dd4a45c6 100644

--- a/src/util/virmacaddr.h

+++ b/src/util/virmacaddr.h

@@ -36,9 +36,14 @@ typedef virMacAddr *virMacAddrPtr;

 struct _virMacAddr {

     unsigned char addr[VIR_MAC_BUFLEN];

-    bool generated; /* True if MAC address was autogenerated,

-                       false otherwise. */

 };

+/* This struct is used as a part of a larger struct that is

+ * overlaid on an ethernet packet captured with libpcap, so it

+ * must not have any extra members added - it must remain exactly

+ * 6 bytes in length.

+ */

+verify(sizeof(struct _virMacAddr) == 6);

+

 int virMacAddrCompare(const char *mac1, const char *mac2);

 int virMacAddrCmp(const virMacAddr *mac1, const virMacAddr *mac2);

diff --git a/tests/bhyveargv2xmlmock.c b/tests/bhyveargv2xmlmock.c

index dd25f4e13a..1f08bebb7b 100644

--- a/tests/bhyveargv2xmlmock.c

+++ b/tests/bhyveargv2xmlmock.c

@@ -16,7 +16,6 @@ virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN],

     addr->addr[3] = 0;

     addr->addr[4] = 0;

     addr->addr[5] = 0;

-    addr->generated = true;

 }

 int

-- 

2.16.2