On Wed, Aug 02, 2023 at 09:15:43AM +0200, Michal Prívozník wrote:
On 7/26/23 16:45, Martin Kletzander wrote:On Wed, Jun 28, 2023 at 12:53:35PM +0200, Michal Privoznik wrote: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. NB, we should relay this event to clients, but that is covered in next commits. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/qemu/qemu_domain.c | 18 ++++++++++++++++++ src/qemu/qemu_driver.c | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 94587638c3..5e5789a28c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12482,6 +12482,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, @@ -12535,6 +12548,11 @@ qemuDomainSyncRxFilter(virDomainObj *vm, return -1; } + /* Reflect changed MAC address in the domain XML. */ + if (virMacAddrCmp(&def->mac, &guestFilter->mac)) { + virMacAddrSet(&def->mac, &guestFilter->mac); + } +If we go with the idea I suggested this needs to be done even when we're not updating the filters.You mean, even when this qemuDomainSyncRxFilter() function is not called, i.e. update MAC addr from processNicRxFilterChangedEvent()? We could do that, but this RX_FILTER_CHANGED event is a bit special. To avoid flooding libvirt with this event, there's a antispam mechanism implemented - after QEMU emits the event it awaits 'query-rx-filter' monitor cmd. No other RX_FILTER_CHANGED event is emitted until the monitor cmd is issued. IOW, if we want to refresh MAC address on each event, we must (unconditionally) issue the command and then (based on trustGuestRxFilters) sync RX filters. What I can't decide is whether it's better to reflect MAC change in domain XML iff trustGuestRxFilters is set, or regardless.
I meant this if we go with the idea of reporting both the HW MAC address and the guest's set MAC address in the live XML. That way we do not break existing use cases where they rely on the MAC address not changing and on top of that we allow for checking what the guest's MAC really is. I think this is a win-win, but feel free to disagree.
Michal
Attachment:
signature.asc
Description: PGP signature
