[libvirt] [PATCH 2/2] qemu: change macvtap device options in response to NIC_RX_FILTER_CHANGED

[Tony Krowiak]

On 01/21/2015 01:44 PM, Laine Stump wrote:
> (I didn't try make syntax-check, but am assuming you have and that it
> passes)
>
>
> On 01/19/2015 11:18 AM, akrowiak at linux.vnet.ibm.com wrote:
>> From: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
>>
>> This patch supplies the funtionality of synchronizing the host macvtap
>> device options with the guest device's in response to the
>> NIC_RX_FILTER_CHANGED event.
> "supplies the functionality of" sounds too busy and doesn't add
> anything. Instead maybe say "This patch enables synchronization of the
> host macvtap options ...."
Okie dokie.
>
>> The following device options will be synchronized:
>> * PROMISC
>> * MULTICAST
>> * ALLMULTI
>>
>> Signed-off-by: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
>> ---
>>   src/qemu/qemu_driver.c |   92 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 92 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 5994558..141f91a 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4168,6 +4168,93 @@ syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilterPtr guestFilter,
>>   
>>   
>>   static void
>> +syncNicRxFilterPromiscMode(char *ifname, virNetDevRxFilterPtr guestFilter,
>> +                               virNetDevRxFilterPtr hostFilter)
>> +{
>> +    bool promisc;
>> +    bool setpromisc = false;
>> +
>> +    /* Set macvtap promisc mode to true if the guest has vlans defined */
>> +    /* or synchronize the macvtap promisc mode if different from guest */
>> +    if (guestFilter->vlan.nTable > 0) {
>> +        if (!hostFilter->promiscuous) {
>> +            setpromisc = true;
>> +            promisc = true;
>> +        }
>> +    } else if (hostFilter->promiscuous != guestFilter->promiscuous) {
>> +        setpromisc = true;
>> +        promisc = guestFilter->promiscuous;
>> +    }
>> +
>> +    if (setpromisc) {
>> +        if (virNetDevSetPromiscuous(ifname, promisc) < 0) {
>> +            VIR_WARN("Couldn't set PROMISC flag to %s for device %s "
>> +                     "while responding to NIC_RX_FILTER_CHANGED",
>> +                     promisc ? "true" : "false", ifname);
>> +        }
>> +    }
>> +}
>> +
>> +
>> +static void
>> +syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter,
>> +                         virNetDevRxFilterPtr hostFilter)
>> +{
>> +    if (hostFilter->multicast.mode != guestFilter->multicast.mode) {
>> +        switch (guestFilter->multicast.mode) {
> If you typecast the above to virNetDevRxFilterMode, then replace the
> "default" below with VIR_NETDEV_RX_FILTER_MODE_NONE, we will get a nice
> reminder to add a new case if a new value is ever added to the enum.
Okay, will do.
>
>> +            case VIR_NETDEV_RX_FILTER_MODE_ALL:
>> +                if (virNetDevSetRcvAllMulti(ifname, true)) {
>> +
>> +                    VIR_WARN("Couldn't set allmulticast flag to 'on' for "
>> +                             "device %s while responding to "
>> +                             "NIC_RX_FILTER_CHANGED", ifname);
>> +                }
>> +                break;
>> +
>> +            case VIR_NETDEV_RX_FILTER_MODE_NORMAL:
>> +                if (virNetDevSetRcvMulti(ifname, true)) {
>> +
>> +                    VIR_WARN("Couldn't set multicast flag to 'on' for "
>> +                             "device %s while responding to "
>> +                             "NIC_RX_FILTER_CHANGED", ifname);
>> +                }
>> +
>> +                if (virNetDevSetRcvAllMulti(ifname, false)) {
>> +                    VIR_WARN("Couldn't set allmulticast flag to 'off' for "
>> +                             "device %s while responding to "
>> +                             "NIC_RX_FILTER_CHANGED", ifname);
>> +                }
>> +                break;
>> +
>> +            default:
> I think this should use VIR_NETDEV_RX_FILTER_MODE_NONE instead of
> "default". See above.
>
>> +                if (virNetDevSetRcvAllMulti(ifname, false)) {
>> +                    VIR_WARN("Couldn't set allmulticast flag to 'off' for "
>> +                             "device %s while responding to "
>> +                             "NIC_RX_FILTER_CHANGED", ifname);
>> +                }
>> +
>> +                if (virNetDevSetRcvMulti(ifname, false)) {
>> +                    VIR_WARN("Couldn't set multicast flag to 'off' for "
>> +                             "device %s while responding to "
>> +                             "NIC_RX_FILTER_CHANGED",
>> +                             ifname);
>> +                }
>> +                break;
>> +        }
>> +    }
>> +}
>> +
>> +
>> +static void
>> +syncNicRxFilterDeviceOptions(char *ifname, virNetDevRxFilterPtr guestFilter,
>> +                           virNetDevRxFilterPtr hostFilter)
>> +{
>> +    syncNicRxFilterPromiscMode(ifname, guestFilter, hostFilter);
>> +    syncNicRxFilterMultiMode(ifname, guestFilter, hostFilter);
>> +}
>> +
>> +
>> +static void
>>   syncNicRxFilterMulticast(char *ifname,
>>                            virNetDevRxFilterPtr guestFilter,
>>                            virNetDevRxFilterPtr hostFilter)
>> @@ -4250,9 +4337,14 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
>>            * attributes to match those of the guest network device:
>>            * - MAC address
>>            * - Multicast MAC address table
>> +         * - Device options:
>> +         *   - PROMISC
>> +         *   - MULTICAST
>> +         *   - ALLMULTI
>>            */
>>           syncNicRxFilterMacAddr(def->ifname, guestFilter, hostFilter);
>>           syncNicRxFilterMulticast(def->ifname, guestFilter, hostFilter);
>> +        syncNicRxFilterDeviceOptions(def->ifname, guestFilter, hostFilter);
>>       }
>>   
>>    endjob:
> I think this is otherwise okay.
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>