On Tue, Mar 18, 2025 at 02:58:18PM +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <mprivozn@xxxxxxxxxx>
>
> If a guest changes MAC address on its vNIC, then QEMU emits
> NIC_RX_FILTER_CHANGED event (the event is emitted in other cases
> too, but that's not important right now). Now, domain XML allows
> users to chose whether to trust these events or not:
>
> <interface trustGuestRxFilters='yes|no'/>
>
> For the 'no' case no action is performed and the event is
> ignored. But for the 'yes' case, some host side features of
> corresponding vNIC (well tap/macvtap device) are tweaked to
> reflect changed MAC address. But what is missing is reflecting
> this new MAC address in domain XML.
>
> Basically, what happens is: the host sees traffic with new MAC
> address, all tools inside the guest see the new MAC address
> (including 'virsh domifaddr --source agent') which makes it
> harder to match device in the guest with the one in the domain
> XML.
>
> Therefore, report this new MAC address as another attribute of
> the <mac/> element:
>
> <mac address="52:54:00:a4:6f:91" guestAddress="00:11:22:33:44:55"/>
What happens when the guest OS reboots, or rather the
machine is reset ? Will the virtio-net device revert
back to its original configured MAC, or if the guest
MAC change persistent until QEMU is shut off.
If the former, we would need to be clearly guestAddress
at reset time.
I wonder a little whether 'address' has any purpose
at all if the guest MAC is changed ? ie should we
just be updating 'address' in-place, and letting
apps request "inactive" XML if they want the original
configured MAC ?
The 'address' is used by the NW filter code to apply
rules tied to guest MAC, which presumably need updating
if the guest changes its MAC
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> docs/formatdomain.rst | 5 +++++
> src/conf/domain_conf.c | 6 ++++++
> src/conf/domain_conf.h | 3 +++
> src/conf/schemas/domaincommon.rng | 5 +++++
> src/qemu/qemu_domain.c | 32 +++++++++++++++++++++++++++++++
> src/qemu/qemu_driver.c | 2 +-
> 6 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 4bc6a318f5..c82d158922 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -4966,6 +4966,11 @@ when it's in the reserved VMware range by adding a ``type="static"`` attribute
> to the ``<mac/>`` element. Note that this attribute is useless if the provided
> MAC address is outside of the reserved VMWare ranges.
>
> +:since:`Since 11.2.0`, the ``<mac/>`` element can optionally contain
> +``guestAddress`` attribute (output only), which contains new MAC address if the
> +guest changed it. This is currently implemented only for QEMU/KVM and requires
> +setting ``trustGuestRxFilters`` to ``yes``.
> +
> :since:`Since 7.3.0`, one can set the ACPI index against network interfaces.
> With some operating systems (eg Linux with systemd), the ACPI index is used
> to provide network interface device naming, that is stable across changes
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index dedcf76511..7c68fd6f2a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2840,6 +2840,7 @@ virDomainNetDefFree(virDomainNetDef *def)
> if (!def)
> return;
>
> + g_free(def->guestAddress);
> g_free(def->modelstr);
>
> switch (def->type) {
> @@ -24586,6 +24587,11 @@ virDomainNetDefFormat(virBuffer *buf,
> virBufferAsprintf(&macAttrBuf, " type='%s'", virDomainNetMacTypeTypeToString(def->mac_type));
> if (def->mac_check != VIR_TRISTATE_BOOL_ABSENT)
> virBufferAsprintf(&macAttrBuf, " check='%s'", virTristateBoolTypeToString(def->mac_check));
> + if (def->guestAddress &&
> + !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
> + virBufferAsprintf(&macAttrBuf, " guestAddress='%s'",
> + virMacAddrFormat(def->guestAddress, macstr));
> + }
> virXMLFormatElement(buf, "mac", &macAttrBuf, NULL);
>
> if (publicActual) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3a97fd866c..3368921d6d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1078,6 +1078,9 @@ struct _virDomainNetDef {
> bool mac_generated; /* true if mac was *just now* auto-generated by libvirt */
> virDomainNetMacType mac_type;
> virTristateBool mac_check;
> + virMacAddr *guestAddress; /* MAC address from query-rx-filter (as reported
> + by guest). Not parsed from domain XML. Output
> + only. */
> int model; /* virDomainNetModelType */
> char *modelstr;
> union {
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 3276569325..ad6b889a93 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -3837,6 +3837,11 @@
> <ref name="virYesNo"/>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="guestAddress">
> + <ref name="uniMacAddr"/>
> + </attribute>
> + </optional>
> <empty/>
> </element>
> </optional>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 29fac0034e..47ae59d408 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11002,6 +11002,19 @@ syncNicRxFilterMulticast(char *ifname,
> }
>
>
> +/**
> + * qemuDomainSyncRxFilter:
> + * @vm: domain object
> + * @def: domain interface definition
> + * @asyncJob: async job type
> + *
> + * Fetch new state of RX Filter and set host side of the interface
> + * accordingly (e.g. reflect MAC address change on macvtap).
> + *
> + * Reflect changed MAC address in the domain definition.
> + *
> + * Returns: 0 on success, -1 on error.
> + */
> int
> qemuDomainSyncRxFilter(virDomainObj *vm,
> virDomainNetDef *def,
> @@ -11010,6 +11023,7 @@ qemuDomainSyncRxFilter(virDomainObj *vm,
> qemuDomainObjPrivate *priv = vm->privateData;
> g_autoptr(virNetDevRxFilter) guestFilter = NULL;
> g_autoptr(virNetDevRxFilter) hostFilter = NULL;
> + virMacAddr *oldMac = NULL;
> int rc;
>
> if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
> @@ -11055,6 +11069,24 @@ qemuDomainSyncRxFilter(virDomainObj *vm,
> return -1;
> }
>
> + if (def->guestAddress)
> + oldMac = def->guestAddress;
> + else
> + oldMac = &def->mac;
> +
> + if (virMacAddrCmp(oldMac, &guestFilter->mac)) {
> + /* Reflect changed MAC address in the domain XML. */
> + if (virMacAddrCmp(&def->mac, &guestFilter->mac)) {
> + if (!def->guestAddress) {
> + def->guestAddress = g_new0(virMacAddr, 1);
> + }
> +
> + virMacAddrSet(def->guestAddress, &guestFilter->mac);
> + } else {
> + VIR_FREE(def->guestAddress);
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f0e9681161..a4866450fc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3677,7 +3677,7 @@ processNicRxFilterChangedEvent(virDomainObj *vm,
> "from domain %p %s",
> devAlias, vm, vm->def->name);
>
> - if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
> + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
> return;
>
> if (!virDomainObjIsActive(vm)) {
> --
> 2.48.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
