Discard OpenFlow messages not for us in dpif_recv_openflow().

Currently there is only a single Netlink multicast group that is shared
by all OpenFlow datapaths.  This is undesirable and should be fixed, but
a solution in every case is impossible due to Linux kernel limitations.
This commit works around the problem by dropping OpenFlow messages that
are related to a datapath that we are not interested in.

This should fix vswitchd behavior when more than one bridge is configured.

Thanks to Keith and Justin for diagnosing the problem.

diff --git a/lib/dpif.c b/lib/dpif.c
index 90856b5f0bd2c277ff95261d54f9224284e0c377..a116bc08a61dc0d1b21046497246b96b1aa6299e 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -110,13 +110,15 @@ dpif_close(struct dpif *dp)
 }
 
 static const struct nl_policy openflow_policy[] = {
-    [DP_GENL_A_DP_IDX] = { .type = NL_A_U32 },
+    [DP_GENL_A_DP_IDX] = { .type = NL_A_U32,
+                           .optional = false },
     [DP_GENL_A_OPENFLOW] = { .type = NL_A_UNSPEC,
-                              .min_len = sizeof(struct ofp_header),
-                              .max_len = 65535 },
+                             .min_len = sizeof(struct ofp_header),
+                             .max_len = 65535,
+                             .optional = false },
 };
 
-/* Tries to receive an openflow message from the kernel on 'sock'.  If
+/* Tries to receive an openflow message from datapath 'dp_idx' on 'sock'.  If
  * successful, stores the received message into '*msgp' and returns 0.  The
  * caller is responsible for destroying the message with ofpbuf_delete().  On
  * failure, returns a positive errno value and stores a null pointer into
@@ -128,43 +130,48 @@ static const struct nl_policy openflow_policy[] = {
  * If 'wait' is true, dpif_recv_openflow waits for a message to be ready;
  * otherwise, returns EAGAIN if the 'sock' receive buffer is empty. */
 int
-dpif_recv_openflow(struct dpif *dp, struct ofpbuf **bufferp, bool wait)
+dpif_recv_openflow(struct dpif *dp, int dp_idx, struct ofpbuf **bufferp,
+                   bool wait)
 {
     struct nlattr *attrs[ARRAY_SIZE(openflow_policy)];
     struct ofpbuf *buffer;
     struct ofp_header *oh;
     uint16_t ofp_len;
-    int retval;
 
     buffer = *bufferp = NULL;
     do {
-        ofpbuf_delete(buffer);
-        retval = nl_sock_recv(dp->sock, &buffer, wait);
-    } while (retval == ENOBUFS
-             || (!retval
-                 && (nl_msg_nlmsgerr(buffer, NULL)
-                     || nl_msg_nlmsghdr(buffer)->nlmsg_type == NLMSG_DONE)));
-    if (retval) {
-        if (retval != EAGAIN) {
-            VLOG_WARN_RL(&rl, "dpif_recv_openflow: %s", strerror(retval)); 
+        int retval;
+
+        do {
+            ofpbuf_delete(buffer);
+            retval = nl_sock_recv(dp->sock, &buffer, wait);
+        } while (retval == ENOBUFS
+                 || (!retval
+                     && (nl_msg_nlmsghdr(buffer)->nlmsg_type == NLMSG_DONE
+                         || nl_msg_nlmsgerr(buffer, NULL))));
+        if (retval) {
+            if (retval != EAGAIN) {
+                VLOG_WARN_RL(&rl, "dpif_recv_openflow: %s", strerror(retval));
+            }
+            return retval;
         }
-        return retval;
-    }
 
-    if (nl_msg_genlmsghdr(buffer) == NULL) {
-        VLOG_DBG_RL(&rl, "received packet too short for Generic Netlink");
-        goto error;
-    }
-    if (nl_msg_nlmsghdr(buffer)->nlmsg_type != openflow_family) {
-        VLOG_DBG_RL(&rl, "received type (%"PRIu16") != openflow family (%d)",
-                    nl_msg_nlmsghdr(buffer)->nlmsg_type, openflow_family);
-        goto error;
-    }
+        if (nl_msg_genlmsghdr(buffer) == NULL) {
+            VLOG_DBG_RL(&rl, "received packet too short for Generic Netlink");
+            goto error;
+        }
+        if (nl_msg_nlmsghdr(buffer)->nlmsg_type != openflow_family) {
+            VLOG_DBG_RL(&rl,
+                        "received type (%"PRIu16") != openflow family (%d)",
+                        nl_msg_nlmsghdr(buffer)->nlmsg_type, openflow_family);
+            goto error;
+        }
 
-    if (!nl_policy_parse(buffer, openflow_policy, attrs,
-                         ARRAY_SIZE(openflow_policy))) {
-        goto error;
-    }
+        if (!nl_policy_parse(buffer, openflow_policy, attrs,
+                             ARRAY_SIZE(openflow_policy))) {
+            goto error;
+        }
+    } while (nl_attr_get_u32(attrs[DP_GENL_A_DP_IDX]) != dp_idx);
 
     oh = buffer->data = (void *) nl_attr_get(attrs[DP_GENL_A_OPENFLOW]);
     buffer->size = nl_attr_get_size(attrs[DP_GENL_A_OPENFLOW]);

diff --git a/lib/dpif.h b/lib/dpif.h
index 313621b79873f6455b744917ae3c809fde8a868e..794b4df7532d28b68bfba4634dde2a206a60e895 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -55,7 +55,7 @@ int dpif_open(int subscribe_dp_idx, struct dpif *);
 void dpif_close(struct dpif *);
 
 /* OpenFlow. */
-int dpif_recv_openflow(struct dpif *, struct ofpbuf **, bool wait);
+int dpif_recv_openflow(struct dpif *, int dp_idx, struct ofpbuf **, bool wait);
 int dpif_send_openflow(struct dpif *, int dp_idx, struct ofpbuf *, bool wait);
 
 /* Management functions. */

diff --git a/lib/vconn-netlink.c b/lib/vconn-netlink.c
index f4aa65a841f353779623f88adb60b2aac4e37217..4fc0e9b07c03a7bac1081cda84b873172f00fcdc 100644 (file)
--- a/lib/vconn-netlink.c
+++ b/lib/vconn-netlink.c
@@ -109,7 +109,7 @@ static int
 netlink_recv(struct vconn *vconn, struct ofpbuf **bufferp)
 {
     struct netlink_vconn *netlink = netlink_vconn_cast(vconn);
-    return dpif_recv_openflow(&netlink->dp, bufferp, false);
+    return dpif_recv_openflow(&netlink->dp, netlink->dp_idx, bufferp, false);
 }
 
 static int