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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx wrote:
From: Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxx>
---
  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]