[libvirt] [PATCH 6/6] qemu: change macvtap device MAC address in response to NIC_RX_FILTER_CHANGED

[Laine Stump]

On 09/24/2014 10:19 PM, John Ferlan wrote:
>
> On 09/24/2014 05:50 AM, Laine Stump wrote:
>> This patch fills in the functionality of
>> processNicRxFilterChangedEvent().  It now checks if it is appropriate
>> to respond to the NIC_RX_FILTER_CHANGED event (based on device type
>> and configuration) and takes appropriate action. Currently it checks
>> if the guest interface has been configured with
>> trustGuestRxFilters='yes', and if the host side device is macvtap. If
>> so, and the MAC address on the guest has changed, the MAC address of
>> the macvtap device is changed to match.
>>
>> The result of this is that networking from the guest will continue to
>> work if the mac address of a macvtap-connected network device is
>> changed from within the guest, as long as trustGuestRxFilters='yes'
>> (previously changing the MAC address in the guest would break
>> networking).
>> ---
>>  src/qemu/qemu_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 64f1d45..7801d91 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4146,8 +4146,13 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>                                 char *devAlias)
>>  {
>>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>>      virDomainDeviceDef dev;
>>      virDomainNetDefPtr def;
>> +    virNetDevRxFilterPtr filter = NULL;
>> +    virMacAddr oldMAC;
>> +    char newMacStr[VIR_MAC_STRING_BUFLEN];
>> +    int ret;
>>  
>>      VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s "
>>                "from domain %p %s",
>> @@ -4175,11 +4180,55 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>      }
>>      def = dev.data.net;
>>  
>> +    if (!virDomainNetGetActualTrustGuestRxFilters(def)) {
>> +        VIR_WARN("ignore NIC_RX_FILTER_CHANGED event for network "
>> +                  "device %s in domain %s",
>> +                  def->info.alias, vm->def->name);
> So how often could this be emitted? Do we need some sort of "filter" to
> only message once per life of the domain?

As often as we respond to the previous NIC_RX_FILTER_CHANGED event -
qemu won't send another event until we've issued a new query-rx-filter
command for that same interface. So for domains where we've set
trustGuestRxFilters='no', we will get exactly one event per interface.
On domains where we trust the guest enough to use its filter info, we
will never get a backlog of more than one per interface.

>
>> +        /* not sending "query-rx-filter" will also suppress any
>> +         * further NIC_RX_FILTER_CHANGED events for this device
>> +         */
>> +        goto endjob;
>> +    }
>> +
>>      /* handle the event - send query-rx-filter and respond to it. */
>>  
>>      VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network "
>>                "device %s in domain %s", def->info.alias, vm->def->name);
>>  
> There's no capabilities check necessary?  What if the command doesn't
> exist in the underlying qemu?

If NIC_RX_FILTER_CHANGED exists, then query-rx-filter exists. If
NIC_RX_FILTER_CHANGED doesn't exist, we will never get to this point.

>
>> +    qemuDomainObjEnterMonitor(driver, vm);
>> +    ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &filter);
>> +    qemuDomainObjExitMonitor(driver, vm);
>> +    if (ret < 0)
>> +        goto endjob;
> You filled in a lot of data from the qemuMonitorQueryRxFilter(), but
> you're only using filter->mac (e.g. main-mac).
>
> Or am I missing something?

No, you're not missing anything. All the other information will be used
in the future in a similar manner. This single patch was all that was
necessary to get regular unicast traffic working after a mac address
change, so it is useful on its own, but I didn't want to add in the
utility functions half-finished.