Make datapath interface library more flexible.

A single netlink socket can address any kernel datapath, but the datapath
interface library was artificially restricting a dpif to address a single
datapath that was selected at dpif creation time.  This commit removes
that restriction.

This is useful for vswitchd, which can create and destroy any number of
datapaths and add and remove ports on them.  It's more convenient to use
a single dpif for that instead of many.

diff --git a/lib/dpif.c b/lib/dpif.c
index 6407490def8ebf7f33f7f5876f5cb7030da0908b..8d16a22ab5de66e6c102cc1a158ca820d7fa6934 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -34,6 +34,7 @@
 #include <config.h>
 #include "dpif.h"
 
+#include <assert.h>
 #include <ctype.h>
 #include <errno.h>
 #include <inttypes.h>
@@ -60,15 +61,15 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 60);
 static int openflow_family;
 
 static int lookup_openflow_multicast_group(int dp_idx, int *multicast_group);
-static int send_mgmt_command(struct dpif *, int command,
+static int send_mgmt_command(struct dpif *, int dp_idx, int command,
                              const char *netdev);
 
-/* Opens the local datapath numbered 'dp_idx', initializing 'dp'.  If
- * 'subscribe' is true, listens for asynchronous messages (packet-in, etc.)
- * from the datapath; otherwise, 'dp' will receive only replies to explicitly
- * initiated requests. */
+/* Opens a socket for a local datapath, initializing 'dp'.  If
+ * 'subscribe_dp_idx' is nonnegative, listens for asynchronous messages
+ * (packet-in, etc.) from the datapath with that number; otherwise, 'dp' will
+ * receive only replies to explicitly initiated requests. */
 int
-dpif_open(int dp_idx, bool subscribe, struct dpif *dp)
+dpif_open(int subscribe_dp_idx, struct dpif *dp)
 {
     struct nl_sock *sock;
     int multicast_group = 0;
@@ -79,8 +80,9 @@ dpif_open(int dp_idx, bool subscribe, struct dpif *dp)
         return retval;
     }
 
-    if (subscribe) {
-        retval = lookup_openflow_multicast_group(dp_idx, &multicast_group);
+    if (subscribe_dp_idx >= 0) {
+        retval = lookup_openflow_multicast_group(subscribe_dp_idx,
+                                                 &multicast_group);
         if (retval) {
             return retval;
         }
@@ -94,7 +96,6 @@ dpif_open(int dp_idx, bool subscribe, struct dpif *dp)
         return retval;
     }
 
-    dp->dp_idx = dp_idx;
     dp->sock = sock;
     return 0;
 }
@@ -127,8 +128,7 @@ 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, struct ofpbuf **bufferp, bool wait)
 {
     struct nlattr *attrs[ARRAY_SIZE(openflow_policy)];
     struct ofpbuf *buffer;
@@ -165,12 +165,6 @@ dpif_recv_openflow(struct dpif *dp, struct ofpbuf **bufferp,
                          ARRAY_SIZE(openflow_policy))) {
         goto error;
     }
-    if (nl_attr_get_u32(attrs[DP_GENL_A_DP_IDX]) != dp->dp_idx) {
-        VLOG_WARN_RL(&rl, "received dp_idx (%"PRIu32") differs from expected "
-                     "(%d)", nl_attr_get_u32(attrs[DP_GENL_A_DP_IDX]),
-                     dp->dp_idx);
-        goto error;
-    }
 
     oh = buffer->data = (void *) nl_attr_get(attrs[DP_GENL_A_OPENFLOW]);
     buffer->size = nl_attr_get_size(attrs[DP_GENL_A_OPENFLOW]);
@@ -190,7 +184,8 @@ error:
 }
 
 /* Encapsulates 'msg', which must contain an OpenFlow message, in a Netlink
- * message, and sends it to the OpenFlow kernel module via 'sock'.
+ * message, and sends it to the OpenFlow local datapath numbered 'dp_idx' via
+ * 'sock'.
  *
  * Returns 0 if successful, otherwise a positive errno value.  If
  * 'wait' is true, then the send will wait until buffer space is ready;
@@ -201,7 +196,8 @@ error:
  * for details). 
  */
 int
-dpif_send_openflow(struct dpif *dp, struct ofpbuf *buffer, bool wait) 
+dpif_send_openflow(struct dpif *dp, int dp_idx, struct ofpbuf *buffer,
+                   bool wait) 
 {
     struct ofp_header *oh;
     unsigned int dump_flag;
@@ -221,7 +217,7 @@ dpif_send_openflow(struct dpif *dp, struct ofpbuf *buffer, bool wait)
     ofpbuf_use(&hdr, fixed_buffer, sizeof fixed_buffer);
     nl_msg_put_genlmsghdr(&hdr, dp->sock, 32, openflow_family,
                           NLM_F_REQUEST | dump_flag, DP_GENL_C_OPENFLOW, 1);
-    nl_msg_put_u32(&hdr, DP_GENL_A_DP_IDX, dp->dp_idx);
+    nl_msg_put_u32(&hdr, DP_GENL_A_DP_IDX, dp_idx);
     nla = ofpbuf_put_uninit(&hdr, sizeof *nla);
     nla->nla_len = sizeof *nla + buffer->size;
     nla->nla_type = DP_GENL_A_OPENFLOW;
@@ -245,36 +241,36 @@ dpif_send_openflow(struct dpif *dp, struct ofpbuf *buffer, bool wait)
     return retval;
 }
 
-/* Creates the datapath represented by 'dp'.  Returns 0 if successful,
+/* Creates local datapath numbered 'dp_idx'.  Returns 0 if successful,
  * otherwise a positive errno value. */
 int
-dpif_add_dp(struct dpif *dp)
+dpif_add_dp(struct dpif *dp, int dp_idx)
 {
-    return send_mgmt_command(dp, DP_GENL_C_ADD_DP, NULL);
+    return send_mgmt_command(dp, dp_idx, DP_GENL_C_ADD_DP, NULL);
 }
 
-/* Destroys the datapath represented by 'dp'.  Returns 0 if successful,
+/* Destroys local datapath numbered 'dp_idx'.  Returns 0 if successful,
  * otherwise a positive errno value. */
 int
-dpif_del_dp(struct dpif *dp) 
+dpif_del_dp(struct dpif *dp, int dp_idx)
 {
-    return send_mgmt_command(dp, DP_GENL_C_DEL_DP, NULL);
+    return send_mgmt_command(dp, dp_idx, DP_GENL_C_DEL_DP, NULL);
 }
 
-/* Adds the Ethernet device named 'netdev' to this datapath.  Returns 0 if
- * successful, otherwise a positive errno value. */
+/* Adds the Ethernet device named 'netdev' to the local datapath numbered
+ * 'dp_idx'.  Returns 0 if successful, otherwise a positive errno value. */
 int
-dpif_add_port(struct dpif *dp, const char *netdev)
+dpif_add_port(struct dpif *dp, int dp_idx, const char *netdev)
 {
-    return send_mgmt_command(dp, DP_GENL_C_ADD_PORT, netdev);
+    return send_mgmt_command(dp, dp_idx, DP_GENL_C_ADD_PORT, netdev);
 }
 
-/* Removes the Ethernet device named 'netdev' from this datapath.  Returns 0
- * if successful, otherwise a positive errno value. */
+/* Removes the Ethernet device named 'netdev' from the local datapath numbered
+ * 'dp_idx'.  Returns 0 if successful, otherwise a positive errno value. */
 int
-dpif_del_port(struct dpif *dp, const char *netdev)
+dpif_del_port(struct dpif *dp, int dp_idx, const char *netdev)
 {
-    return send_mgmt_command(dp, DP_GENL_C_DEL_PORT, netdev);
+    return send_mgmt_command(dp, dp_idx, DP_GENL_C_DEL_PORT, netdev);
 }
 \f
 static const struct nl_policy openflow_multicast_policy[] = {
@@ -320,11 +316,13 @@ lookup_openflow_multicast_group(int dp_idx, int *multicast_group)
     return 0;
 }
 
-/* Sends the given 'command' to datapath 'dp'.  If 'netdev' is nonnull, adds it
- * to the command as the port name attribute.  Returns 0 if successful,
- * otherwise a positive errno value. */
+/* Sends the given 'command' to datapath 'dp', related to the local datapath
+ * numbered 'dp_idx'.  If 'netdev' is nonnull, adds it to the command as the
+ * port name attribute.  Returns 0 if successful, otherwise a positive errno
+ * value. */
 static int
-send_mgmt_command(struct dpif *dp, int command, const char *netdev) 
+send_mgmt_command(struct dpif *dp, int dp_idx,
+                  int command, const char *netdev)
 {
     struct ofpbuf request, *reply;
     int retval;
@@ -332,7 +330,7 @@ send_mgmt_command(struct dpif *dp, int command, const char *netdev)
     ofpbuf_init(&request, 0);
     nl_msg_put_genlmsghdr(&request, dp->sock, 32, openflow_family,
                           NLM_F_REQUEST | NLM_F_ACK, command, 1);
-    nl_msg_put_u32(&request, DP_GENL_A_DP_IDX, dp->dp_idx);
+    nl_msg_put_u32(&request, DP_GENL_A_DP_IDX, dp_idx);
     if (netdev) {
         nl_msg_put_string(&request, DP_GENL_A_PORTNAME, netdev);
     }

diff --git a/lib/dpif.h b/lib/dpif.h
index 0316e13b4d8ee00e5555b39826b5854fd3ee4632..7a0cd08121afe54e829d07d23a01aa73e1a630b9 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -48,17 +48,20 @@ struct ofp_match;
 /* A datapath interface.  Opaque. */
 struct dpif
 {
-    int dp_idx;
     struct nl_sock *sock;
 };
 
-int dpif_open(int dp_idx, bool subscribe, struct dpif *);
+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_send_openflow(struct dpif *, struct ofpbuf *, bool wait);
-int dpif_add_dp(struct dpif *);
-int dpif_del_dp(struct dpif *);
-int dpif_add_port(struct dpif *, const char *netdev);
-int dpif_del_port(struct dpif *, const char *netdev);
+int dpif_send_openflow(struct dpif *, int dp_idx, struct ofpbuf *, bool wait);
+
+/* Management functions. */
+int dpif_add_dp(struct dpif *, int dp_idx);
+int dpif_del_dp(struct dpif *, int dp_idx);
+int dpif_add_port(struct dpif *, int dp_idx, const char *netdev);
+int dpif_del_port(struct dpif *, int dp_idx, const char *netdev);
 
 #endif /* dpif.h */

diff --git a/lib/vconn-netlink.c b/lib/vconn-netlink.c
index a0fcc8d1264d0e6e9973f381443f270a3661a09a..f7b86777e5d008b7899dd4d7784356aa0b0323bc 100644 (file)
--- a/lib/vconn-netlink.c
+++ b/lib/vconn-netlink.c
@@ -60,6 +60,7 @@ struct netlink_vconn
 {
     struct vconn vconn;
     struct dpif dp;
+    int dp_idx;
 };
 
 static struct netlink_vconn *
@@ -73,8 +74,8 @@ static int
 netlink_open(const char *name, char *suffix, struct vconn **vconnp)
 {
     struct netlink_vconn *netlink;
-    int dp_idx;
     int subscribe;
+    int dp_idx;
     int retval;
 
     subscribe = 1;
@@ -85,7 +86,8 @@ netlink_open(const char *name, char *suffix, struct vconn **vconnp)
 
     netlink = xmalloc(sizeof *netlink);
     vconn_init(&netlink->vconn, &netlink_vconn_class, 0, 0, name);
-    retval = dpif_open(dp_idx, subscribe, &netlink->dp);
+    retval = dpif_open(subscribe ? dp_idx : -1, &netlink->dp);
+    netlink->dp_idx = dp_idx;
     if (retval) {
         free(netlink);
         *vconnp = NULL;
@@ -114,7 +116,8 @@ static int
 netlink_send(struct vconn *vconn, struct ofpbuf *buffer) 
 {
     struct netlink_vconn *netlink = netlink_vconn_cast(vconn);
-    int retval = dpif_send_openflow(&netlink->dp, buffer, false);
+    int retval = dpif_send_openflow(&netlink->dp, netlink->dp_idx,
+                                    buffer, false);
     if (!retval) {
         ofpbuf_delete(buffer);
     }

diff --git a/utilities/dpctl.c b/utilities/dpctl.c
index 8fa0da850b75f55b975a3c64f6b148cb3f408940..1c23a24b89b67e02308532c3b95e37d4220e66a7 100644 (file)
--- a/utilities/dpctl.c
+++ b/utilities/dpctl.c
@@ -287,59 +287,65 @@ static int if_up(const char *netdev_name)
     return retval;
 }
 
-static void open_nl_vconn(const char *name, bool subscribe, struct dpif *dpif)
+static int
+get_dp_idx(const char *name)
 {
     if (strncmp(name, "nl:", 3)
         || strlen(name) < 4
         || name[strspn(name + 3, "0123456789") + 3]) {
         ofp_fatal(0, "%s: argument is not of the form \"nl:DP_ID\"", name);
     }
-    run(dpif_open(atoi(name + 3), subscribe, dpif), "opening datapath");
+    return atoi(name + 3);
 }
 
-static void do_add_dp(const struct settings *s, int argc UNUSED, char *argv[])
+static void
+do_add_dp(const struct settings *s, int argc UNUSED, char *argv[])
 {
-    struct dpif dp;
-    open_nl_vconn(argv[1], false, &dp);
-    run(dpif_add_dp(&dp), "add_dp");
-    dpif_close(&dp);
+    struct dpif dpif;
+    run(dpif_open(-1, &dpif), "opening management socket");
+    run(dpif_add_dp(&dpif, get_dp_idx(argv[1])), "add_dp");
+    dpif_close(&dpif);
 }
 
-static void do_del_dp(const struct settings *s, int argc UNUSED, char *argv[])
+static void
+do_del_dp(const struct settings *s, int argc UNUSED, char *argv[])
 {
-    struct dpif dp;
-    open_nl_vconn(argv[1], false, &dp);
-    run(dpif_del_dp(&dp), "del_dp");
-    dpif_close(&dp);
+    struct dpif dpif;
+    run(dpif_open(-1, &dpif), "opening management socket");
+    run(dpif_del_dp(&dpif, get_dp_idx(argv[1])), "del_dp");
+    dpif_close(&dpif);
 }
 
 static void add_del_ports(int argc UNUSED, char *argv[],
-                          int (*function)(struct dpif *, const char *netdev),
+                          int (*function)(struct dpif *, int dp_idx,
+                                          const char *netdev),
                           const char *operation, const char *preposition)
 {
-    struct dpif dp;
     bool failure = false;
+    struct dpif dpif;
+    int dp_idx;
     int i;
 
-    open_nl_vconn(argv[1], false, &dp);
+    run(dpif_open(-1, &dpif), "opening management socket");
+    dp_idx = get_dp_idx(argv[1]);
     for (i = 2; i < argc; i++) {
-        int retval = function(&dp, argv[i]);
+        int retval = function(&dpif, dp_idx, argv[i]);
         if (retval) {
             ofp_error(retval, "failed to %s %s %s %s",
                       operation, argv[i], preposition, argv[1]);
             failure = true;
         }
     }
-    dpif_close(&dp);
+    dpif_close(&dpif);
     if (failure) {
         exit(EXIT_FAILURE);
     }
 }
 
-static int ifup_and_add_port(struct dpif *dpif, const char *netdev)
+static int ifup_and_add_port(struct dpif *dpif, int dp_idx, const char *netdev)
 {
     int retval = if_up(netdev);
-    return retval ? retval : dpif_add_port(dpif, netdev);
+    return retval ? retval : dpif_add_port(dpif, dp_idx, netdev);
 }
 
 static void do_add_port(const struct settings *s, int argc UNUSED,