[libvirt] [PATCH 2/2] qemu: change macvtap multicast list in response to NIC_RX_FILTER_CHANGED

Tony Krowiak akrowiak at linux.vnet.ibm.com
Wed Oct 8 20:08:57 UTC 2014


On 10/08/2014 03:15 PM, Laine Stump wrote:
> On 10/06/2014 05:37 PM, akrowiak at linux.vnet.ibm.com wrote:
>> From: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
>>
>> This patch adds functionality to processNicRxFilterChangedEvent().
>> The old and new multicast lists are compared and the filters in
>> the macvtap are programmed to match the guest's filters.
>>
>> Signed-off-by: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
>> ---
>>   src/qemu/qemu_driver.c |  138 +++++++++++++++++++++++++++++++++++++++--------
>>   1 files changed, 114 insertions(+), 24 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 4e2b356..7ff9c38 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4147,6 +4147,106 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
>>   
>>   
>>   static void
>> +syncNicRxFilterMacAddr(char *ifname, virNetDevRxFilterPtr guestFilter,
>> +                       virNetDevRxFilterPtr hostFilter)
>> +{
>> +    char newMacStr[VIR_MAC_STRING_BUFLEN];
>> +
>> +    if (virMacAddrCmp(&hostFilter->mac, &guestFilter->mac)) {
>> +        virMacAddrFormat(&guestFilter->mac, newMacStr);
>> +
>> +        /* set new MAC address from guest to associated macvtap device */
>> +        if (virNetDevSetMAC(ifname, &guestFilter->mac)) {
>> +            VIR_WARN("Couldn't set new MAC address %s to device %s "
>> +                     "while responding to NIC_RX_FILTER_CHANGED",
>> +                     newMacStr, ifname);
>> +        } else {
>> +            VIR_DEBUG("device %s MAC address set to %s", ifname, newMacStr);
>> +        }
>> +    }
>> +}
>> +
>> +
>> +static void
>> +syncNicRxFilterGuestMulticast(char *ifname, virNetDevRxFilterPtr guestFilter,
>> +                              virNetDevRxFilterPtr hostFilter)
>> +{
>> +    size_t i, j;
>> +    bool found;
>> +    char macstr[VIR_MAC_STRING_BUFLEN];
>> +
>> +    for (i = 0; i < guestFilter->multicast.nTable; i++) {
>> +        found = false;
>> +
>> +        for (j = 0; j < hostFilter->multicast.nTable; j++) {
>> +            if (virMacAddrCmp(&guestFilter->multicast.table[i],
>> +                              &hostFilter->multicast.table[j]) == 0) {
>> +                found = true;
>> +                break;
>> +            }
>> +        }
>> +
>> +        if (!found) {
>> +            virMacAddrFormat(&guestFilter->multicast.table[i], macstr);
>> +
>> +            if (virNetDevAddMulti(ifname, &guestFilter->multicast.table[i])) {
>> +                VIR_WARN("Couldn't add new multicast MAC address %s to "
>> +                         "device %s while responding to NIC_RX_FILTER_CHANGED",
>> +                         macstr, ifname);
>> +            } else {
>> +                VIR_DEBUG("Added multicast MAC %s to %s interface",
>> +                          macstr, ifname);
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +
>> +static void
>> +syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilterPtr guestFilter,
>> +                             virNetDevRxFilterPtr hostFilter)
>> +{
>> +    size_t i, j;
>> +    bool found;
>> +    char macstr[VIR_MAC_STRING_BUFLEN];
>> +
>> +    for (i = 0; i < hostFilter->multicast.nTable; i++) {
>> +        found = false;
>> +
>> +        for (j = 0; j < guestFilter->multicast.nTable; j++) {
>> +            if (virMacAddrCmp(&hostFilter->multicast.table[i],
>> +                              &guestFilter->multicast.table[j]) == 0) {
>> +                found = true;
>> +                break;
>> +            }
>> +        }
>> +
>> +        if (!found) {
>> +            virMacAddrFormat(&hostFilter->multicast.table[i], macstr);
>> +
>> +            if (virNetDevDelMulti(ifname, &hostFilter->multicast.table[i])) {
>> +                VIR_WARN("Couldn't delete multicast MAC address %s from "
>> +                         "device %s while responding to NIC_RX_FILTER_CHANGED",
>> +                         macstr, ifname);
>> +            } else {
>> +                VIR_DEBUG("Deleted multicast MAC %s from %s interface",
>> +                          macstr, ifname);
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +
>> +static void
>> +syncNicRxFilterMulticast(char *ifname,
>> +                         virNetDevRxFilterPtr guestFilter,
>> +                         virNetDevRxFilterPtr hostFilter)
>> +{
>> +    syncNicRxFilterGuestMulticast(ifname, guestFilter, hostFilter);
>> +    syncNicRxFilterHostMulticast(ifname, guestFilter, hostFilter);
> Interesting method. I had planned to sort both lists, then do a single
> loop that went through the two in parallel, deleting/adding from the
> host interface filter at each step as necessary. Yours works, too,
> though. :-)
>
> ACK to this patch. I'll push it along with 1/2 once a new eversion of
> that patch has gotten ACK.
>
> BTW, once we're programming the multicast filter, do we need to set any
> other mode of the macvtap device in order for this to have a positive
> impact on performance? Or does multicast simply not work properly
> without this patch (I don't use multicast for anything, and don't have
> any tests setup to use multicast).
To be honest, the only testing I did was to make sure that if I added or 
deleted a multicast MAC on the guest, that it showed up in the macvtap 
list on the host.  I am not too networking literate and rely on our 
system test team to flesh out those issues.  The next thing on my todo 
list is to synchronize changes to the guest network device flags - such 
as the PROMISCUOUS, MULTICAST, ALLMULTI and BROADCAST flags - to the 
macvtap device.  In fact, I've already written the virnetdev functions 
to do that, but the ioctl's I used don't always do what they seem to 
imply.  I do have a question related to my (mis)understanding of the 
data returned from the query_rx_filter call vis a vis these device 
flags.  These are my assumptions regarding to what network device flags 
the following query-rx-filter data elements equate:

  * promiscuous : true         => ip link set macvtap0 promisc on
  * promiscuous : false        => ip link set macvtap0 promisc off
  * multicast   : normal       => ip link set macvtap0 multicast on
  * multicast   : all          => ip link set macvtap0 allmulticast on
  * multicast   : none         => ip link set macvtap0 multicast off, ip
    link set macvtap0 allmulticast off
  * broadcast-allowed : true   => ????????


I used the SIOCSIFFLAGS and SIOCGIFFLAGS ioctl's to set and get the 
network device flags respectively, as follows:

  * IFF_PROMISC to get/set promiscuous flag
  * IFF_MULTICAST to get/set multicast flag
  * IFF_ALLMULTI to get/set allmulticast flag
  * IFF_BROADCAST to get/set the broadcast flag

The thing is, I can set both the MULTICAST and ALLMULTI flags using the 
*ip link* command, but the normal, all and none values for multicast do 
not lend themselves to setting these flags independently.  Maybe thats 
because I am misunderstanding what they mean?  Also, using the ioctl to 
set the BROADCAST flag does not seem to work.  Any light you can shed on 
this would be helpful.  Thanks.
>
>> +}
>> +
>> +static void
>>   processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>                                  virDomainObjPtr vm,
>>                                  char *devAlias)
>> @@ -4155,9 +4255,8 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>       qemuDomainObjPrivatePtr priv = vm->privateData;
>>       virDomainDeviceDef dev;
>>       virDomainNetDefPtr def;
>> -    virNetDevRxFilterPtr filter = NULL;
>> -    virMacAddr oldMAC;
>> -    char newMacStr[VIR_MAC_STRING_BUFLEN];
>> +    virNetDevRxFilterPtr guestFilter = NULL;
>> +    virNetDevRxFilterPtr hostFilter = NULL;
>>       int ret;
>>   
>>       VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s "
>> @@ -4202,37 +4301,27 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>                 "device %s in domain %s", def->info.alias, vm->def->name);
>>   
>>       qemuDomainObjEnterMonitor(driver, vm);
>> -    ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &filter);
>> +    ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &guestFilter);
>>       qemuDomainObjExitMonitor(driver, vm);
>>       if (ret < 0)
>>           goto endjob;
>>   
>> -    virMacAddrFormat(&filter->mac, newMacStr);
>> -
>>       if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) {
>>   
>> -        /* For macvtap connections, set the macvtap device's MAC
>> -         * address to match that of the guest device.
>> -         */
>> -
>> -        if (virNetDevGetMAC(def->ifname, &oldMAC) < 0) {
>> -            VIR_WARN("Couldn't get current MAC address of device %s "
>> +        if (virNetDevGetRxFilter(def->ifname, &hostFilter)) {
>> +            VIR_WARN("Couldn't get current RX filter for device %s "
>>                        "while responding to NIC_RX_FILTER_CHANGED",
>>                        def->ifname);
>>               goto endjob;
>>           }
>>   
>> -        if (virMacAddrCmp(&oldMAC, &filter->mac)) {
>> -            /* set new MAC address from guest to associated macvtap device */
>> -            if (virNetDevSetMAC(def->ifname, &filter->mac) < 0) {
>> -                VIR_WARN("Couldn't set new MAC address %s to device %s "
>> -                         "while responding to NIC_RX_FILTER_CHANGED",
>> -                         newMacStr, def->ifname);
>> -            } else {
>> -                VIR_DEBUG("device %s MAC address set to %s",
>> -                          def->ifname, newMacStr);
>> -            }
>> -        }
>> +        /* For macvtap connections, set the following macvtap network device
>> +         * attributes to match those of the guest network device:
>> +         * - MAC address
>> +         * - Multicast MAC address table
>> +         */
>> +        syncNicRxFilterMacAddr(def->ifname, guestFilter, hostFilter);
>> +        syncNicRxFilterMulticast(def->ifname, guestFilter, hostFilter);
>>       }
>>   
>>    endjob:
>> @@ -4242,7 +4331,8 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>       ignore_value(qemuDomainObjEndJob(driver, vm));
>>   
>>    cleanup:
>> -    virNetDevRxFilterFree(filter);
>> +    virNetDevRxFilterFree(hostFilter);
>> +    virNetDevRxFilterFree(guestFilter);
>>       VIR_FREE(devAlias);
>>       virObjectUnref(cfg);
>>   }
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141008/76663c3c/attachment-0001.htm>


More information about the libvir-list mailing list