[libvirt] [PATCH 2/1] nwfilter: Fix IP address learning

John Ferlan jferlan at redhat.com
Sat May 26 12:27:47 UTC 2018


In a previous commit:

   commit d4bf8f415074759baf051644559e04fe78888f8b
   Author: Daniel P. Berrangé <berrange at redhat.com>
   Date:   Wed Feb 14 09:43:59 2018 +0000

     nwfilter: handle missing switch enum cases

     Ensure all enum cases are listed in switch statements, or cast away
     enum type in places where we don't wish to cover all cases.

     Reviewed-by: John Ferlan <jferlan at redhat.com>
     Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>

we changed a switch in the nwfilter learning thread so that it had
explict cases for all enum entries. Unfortunately the parameters in the
method had been declared with incorrect type. The "howDetect" parameter
does *not* accept "enum howDetect" values, rather it accepts a bitmask
of "enum howDetect" values, so it should have been an "int" type.

The caller always passes DETECT_STATIC|DETECT_DHCP, so essentially the
IP addressing learning was completely broken by the above change, as it
never matched any switch case, hitting the default leading to EINVAL.

Rather than treat the howDetect as a numerical enum, let's treat it
as a bitmask/flag since that's the way it's used. Thus, the switch
changes to a simple if bit is set keeping the original intention that
if @howDetect == DETECT_DHCP, then use applyDHCPOnlyRules; otherwise,
use applyBasicRules to determine the IP Address.

Proposed-by: Daniel P. Berrangé <berrange at redhat.com>
Signed-off-by: John Ferlan <jferlan at redhat.com>
---

 Here's to hoping I've used the correct magic incantation to get this
 to reply to Daniel's poll series as this should be pushed in
 conjunction with that (and thus would need review). This should be
 done before the 4.4.0 release.

 BTW: This also fixes a strange phenomena I was seeing in my testing
      environment where domain startups would periodically fail with:

          error: Failed to start domain f23
          error: Machine 'qemu-6-f23' already exists

      but could be started with enough retry attempts. What causes me
      to believe there's a relationship with this series is that if
      running libvirtd debug, I see the following as output:


          Detaching after fork from child process 22442.
          2018-05-25 20:37:24.966+0000: 25923: error :
              virSystemdCreateMachine:361 : Machine 'qemu-6-f23' already exists
          2018-05-25 20:37:24.988+0000: 22440: error :
              learnIPAddressThread:668   : encountered an error on interface
              vnet0 index 150: Invalid argument
          [Thread 0x7fffacdb7700 (LWP 22440) exited]

      With these two patches in place, no more errors.

 src/nwfilter/nwfilter_learnipaddr.c | 27 +++++++++++++--------------
 src/nwfilter/nwfilter_learnipaddr.h | 10 +++++-----
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 506b98fd07..ab2697274c 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -145,7 +145,7 @@ struct _virNWFilterIPAddrLearnReq {
     char *filtername;
     virHashTablePtr filterparams;
     virNWFilterDriverStatePtr driver;
-    enum howDetect howDetect;
+    virNWFilterLearnIPHowDetectFlags howDetect;
 
     int status;
     volatile bool terminate;
@@ -344,7 +344,7 @@ virNWFilterDeregisterLearnReq(int ifindex)
 static void
 procDHCPOpts(struct dhcp *dhcp, int dhcp_opts_len,
              uint32_t *vmaddr, uint32_t *bcastaddr,
-             enum howDetect *howDetected)
+             virNWFilterLearnIPHowDetectFlags *howDetected)
 {
     struct dhcp_option *dhcpopt = &dhcp->options[0];
 
@@ -413,7 +413,7 @@ learnIPAddressThread(void *arg)
     char *filter = NULL;
     uint16_t etherType;
     bool showError = true;
-    enum howDetect howDetected = 0;
+    virNWFilterLearnIPHowDetectFlags howDetected = 0;
     virNWFilterTechDriverPtr techdriver = req->techdriver;
     struct pollfd fds[1];
 
@@ -442,28 +442,27 @@ learnIPAddressThread(void *arg)
 
     virMacAddrFormat(&req->macaddr, macaddr);
 
-    switch (req->howDetect) {
-    case DETECT_DHCP:
+    if (req->howDetect == DETECT_DHCP) {
         if (techdriver->applyDHCPOnlyRules(req->ifname,
                                            &req->macaddr,
                                            NULL, false) < 0) {
+            VIR_DEBUG("Unable to apply DHCP only rules");
             req->status = EINVAL;
             goto done;
         }
         virBufferAddLit(&buf, "src port 67 and dst port 68");
-        break;
-    case DETECT_STATIC:
+    } else {
+        /* howDetect == DETECT_DHCP ||
+         * howDetect == (DETECT_DHCP | DETECT_STATIC)
+         */
         if (techdriver->applyBasicRules(req->ifname,
                                         &req->macaddr) < 0) {
+            VIR_DEBUG("Unable to apply basic rules");
             req->status = EINVAL;
             goto done;
         }
         virBufferAsprintf(&buf, "ether host %s or ether dst ff:ff:ff:ff:ff:ff",
                           macaddr);
-        break;
-    default:
-        req->status = EINVAL;
-        goto done;
     }
 
     if (virBufferError(&buf)) {
@@ -726,7 +725,7 @@ learnIPAddressThread(void *arg)
  *               once its IP address has been detected
  * @driver : the network filter driver
  * @howDetect : the method on how the thread is supposed to detect the
- *              IP address; must choose any of the available flags
+ *              IP address using virNWFilterLearnIPHowDetectFlags bitmask
  *
  * Instruct to learn the IP address being used on a given interface (ifname).
  * Unless there already is a thread attempting to learn the IP address
@@ -744,7 +743,7 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
                           const char *filtername,
                           virHashTablePtr filterparams,
                           virNWFilterDriverStatePtr driver,
-                          enum howDetect howDetect)
+                          virNWFilterLearnIPHowDetectFlags howDetect)
 {
     int rc;
     virThread thread;
@@ -833,7 +832,7 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
                           const char *filtername ATTRIBUTE_UNUSED,
                           virHashTablePtr filterparams ATTRIBUTE_UNUSED,
                           virNWFilterDriverStatePtr driver ATTRIBUTE_UNUSED,
-                          enum howDetect howDetect ATTRIBUTE_UNUSED)
+                          virNWFilterLearnIPHowDetectFlags howDetect ATTRIBUTE_UNUSED)
 {
     virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                    _("IP parameter must be given since libvirt "
diff --git a/src/nwfilter/nwfilter_learnipaddr.h b/src/nwfilter/nwfilter_learnipaddr.h
index 06fea5bff8..ab21603090 100644
--- a/src/nwfilter/nwfilter_learnipaddr.h
+++ b/src/nwfilter/nwfilter_learnipaddr.h
@@ -30,10 +30,10 @@
 # include "nwfilter_tech_driver.h"
 # include <net/if.h>
 
-enum howDetect {
-  DETECT_DHCP = 1,
-  DETECT_STATIC = 2,
-};
+typedef enum {
+  DETECT_DHCP = 1 << 0,
+  DETECT_STATIC = 1 << 1,
+} virNWFilterLearnIPHowDetectFlags;
 
 int virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
                               const char *ifname,
@@ -43,7 +43,7 @@ int virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
                               const char *filtername,
                               virHashTablePtr filterparams,
                               virNWFilterDriverStatePtr driver,
-                              enum howDetect howDetect);
+                              virNWFilterLearnIPHowDetectFlags howDetect);
 
 bool virNWFilterHasLearnReq(int ifindex);
 int virNWFilterTerminateLearnReq(const char *ifname);
-- 
2.14.3




More information about the libvir-list mailing list