dpif: Hide the contents of struct dpif.
[openvswitch] / lib / dpif.c
index bcb4e7b44374a142bf61c504e01f015fe43c4b9b..8e8cfcd922c3fdf6a8d84a40128a2b22abf7e027 100644 (file)
 #include "vlog.h"
 #define THIS_MODULE VLM_dpif
 
+/* A datapath interface. */
+struct dpif {
+    char *name;
+    unsigned int minor;
+    int fd;
+};
+
 /* Rate limit for individual messages going to or from the datapath, output at
  * DBG level.  This is very high because, if these are enabled, it is because
  * we really need to see them. */
@@ -60,24 +67,26 @@ static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(9999, 5);
 static int get_minor_from_name(const char *name, unsigned int *minor);
 static int name_to_minor(const char *name, unsigned int *minor);
 static int lookup_minor(const char *name, unsigned int *minor);
-static int open_by_minor(unsigned int minor, struct dpif *);
+static int open_by_minor(unsigned int minor, struct dpif **dpifp);
 static int make_openvswitch_device(unsigned int minor, char **fnp);
 static void check_rw_odp_flow(struct odp_flow *);
 
 int
-dpif_open(const char *name, struct dpif *dpif)
+dpif_open(const char *name, struct dpif **dpifp)
 {
+    struct dpif *dpif;
+    unsigned int minor;
     int listen_mask;
     int error;
 
-    dpif->fd = -1;
+    *dpifp = NULL;
 
-    error = name_to_minor(name, &dpif->minor);
+    error = name_to_minor(name, &minor);
     if (error) {
         return error;
     }
 
-    error = open_by_minor(dpif->minor, dpif);
+    error = open_by_minor(minor, &dpif);
     if (error) {
         return error;
     }
@@ -89,12 +98,13 @@ dpif_open(const char *name, struct dpif *dpif)
     if (ioctl(dpif->fd, ODP_GET_LISTEN_MASK, &listen_mask)) {
         error = errno;
         if (error != ENODEV) {
-            VLOG_WARN("dp%u: probe returned unexpected error: %s",
-                      dpif->minor, strerror(error));
+            VLOG_WARN("%s: probe returned unexpected error: %s",
+                      dpif_name(dpif), strerror(error));
         }
         dpif_close(dpif);
         return error;
     }
+    *dpifp = dpif;
     return 0;
 }
 
@@ -102,8 +112,9 @@ void
 dpif_close(struct dpif *dpif)
 {
     if (dpif) {
+        free(dpif->name);
         close(dpif->fd);
-        dpif->fd = -1;
+        free(dpif);
     }
 }
 
@@ -114,25 +125,28 @@ do_ioctl(const struct dpif *dpif, int cmd, const char *cmd_name,
     int error = ioctl(dpif->fd, cmd, arg) ? errno : 0;
     if (cmd_name) {
         if (error) {
-            VLOG_WARN_RL(&error_rl, "dp%u: ioctl(%s) failed (%s)",
-                         dpif->minor, cmd_name, strerror(error));
+            VLOG_WARN_RL(&error_rl, "%s: ioctl(%s) failed (%s)",
+                         dpif_name(dpif), cmd_name, strerror(error));
         } else {
-            VLOG_DBG_RL(&dpmsg_rl, "dp%u: ioctl(%s): success",
-                        dpif->minor, cmd_name);
+            VLOG_DBG_RL(&dpmsg_rl, "%s: ioctl(%s): success",
+                        dpif_name(dpif), cmd_name);
         }
     }
     return error;
 }
 
 int
-dpif_create(const char *name, struct dpif *dpif)
+dpif_create(const char *name, struct dpif **dpifp)
 {
     unsigned int minor;
     int error;
 
+    *dpifp = NULL;
     if (!get_minor_from_name(name, &minor)) {
         /* Minor was specified in 'name', go ahead and create it. */
-        error = open_by_minor(minor, dpif);
+        struct dpif *dpif;
+
+        error = open_by_minor(minor, &dpif);
         if (error) {
             return error;
         }
@@ -144,19 +158,24 @@ dpif_create(const char *name, struct dpif *dpif)
         } else {
             error = ioctl(dpif->fd, ODP_DP_CREATE, name) < 0 ? errno : 0;
         }
-        if (error) {
+        if (!error) {
+            *dpifp = dpif;
+        } else {
             dpif_close(dpif);
         }
         return error;
     } else {
         for (minor = 0; minor < ODP_MAX; minor++) {
-            error = open_by_minor(minor, dpif);
+            struct dpif *dpif;
+
+            error = open_by_minor(minor, &dpif);
             if (error) {
                 return error;
             }
 
             error = ioctl(dpif->fd, ODP_DP_CREATE, name) < 0 ? errno : 0;
             if (!error) {
+                *dpifp = dpif;
                 return 0;
             }
             dpif_close(dpif);
@@ -168,6 +187,12 @@ dpif_create(const char *name, struct dpif *dpif)
     }
 }
 
+const char *
+dpif_name(const struct dpif *dpif)
+{
+    return dpif->name;
+}
+
 int
 dpif_delete(struct dpif *dpif)
 {
@@ -253,13 +278,12 @@ dpif_port_add(struct dpif *dpif, const char *devname, uint16_t port_no,
     port.port = port_no;
     port.flags = flags;
     if (!ioctl(dpif->fd, ODP_PORT_ADD, &port)) {
-        VLOG_DBG_RL(&dpmsg_rl, "dp%u: added %s as port %"PRIu16,
-                    dpif->minor, devname, port_no);
+        VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu16,
+                    dpif_name(dpif), devname, port_no);
         return 0;
     } else {
-        VLOG_WARN_RL(&error_rl, "dp%u: failed to add %s as port "
-                     "%"PRIu16": %s", dpif->minor, devname, port_no,
-                     strerror(errno));
+        VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port %"PRIu16": %s",
+                     dpif_name(dpif), devname, port_no, strerror(errno));
         return errno;
     }
 }
@@ -279,12 +303,12 @@ dpif_port_query_by_number(const struct dpif *dpif, uint16_t port_no,
     memset(port, 0, sizeof *port);
     port->port = port_no;
     if (!ioctl(dpif->fd, ODP_PORT_QUERY, port)) {
-        VLOG_DBG_RL(&dpmsg_rl, "dp%u: port %"PRIu16" is device %s",
-                    dpif->minor, port_no, port->devname);
+        VLOG_DBG_RL(&dpmsg_rl, "%s: port %"PRIu16" is device %s",
+                    dpif_name(dpif), port_no, port->devname);
         return 0;
     } else {
-        VLOG_WARN_RL(&error_rl, "dp%u: failed to query port %"PRIu16": %s",
-                     dpif->minor, port_no, strerror(errno));
+        VLOG_WARN_RL(&error_rl, "%s: failed to query port %"PRIu16": %s",
+                     dpif_name(dpif), port_no, strerror(errno));
         return errno;
     }
 }
@@ -296,12 +320,15 @@ dpif_port_query_by_name(const struct dpif *dpif, const char *devname,
     memset(port, 0, sizeof *port);
     strncpy(port->devname, devname, sizeof port->devname);
     if (!ioctl(dpif->fd, ODP_PORT_QUERY, port)) {
-        VLOG_DBG_RL(&dpmsg_rl, "dp%u: device %s is on port %"PRIu16,
-                    dpif->minor, devname, port->port);
+        VLOG_DBG_RL(&dpmsg_rl, "%s: device %s is on port %"PRIu16,
+                    dpif_name(dpif), devname, port->port);
         return 0;
     } else {
-        VLOG_WARN_RL(&error_rl, "dp%u: failed to query port %s: %s",
-                     dpif->minor, devname, strerror(errno));
+        /* Log level is DBG here because all the current callers are interested
+         * in whether 'dpif' actually has a port 'devname', so that it's not an
+         * issue worth logging if it doesn't. */
+        VLOG_DBG_RL(&error_rl, "%s: failed to query port %s: %s",
+                    dpif_name(dpif), devname, strerror(errno));
         return errno;
     }
 }
@@ -415,7 +442,7 @@ log_flow_message(const struct dpif *dpif, int error,
                  const union odp_action *actions, size_t n_actions)
 {
     struct ds ds = DS_EMPTY_INITIALIZER;
-    ds_put_format(&ds, "dp%u: ", dpif->minor);
+    ds_put_format(&ds, "%s: ", dpif_name(dpif));
     if (error) {
         ds_put_cstr(&ds, "failed to ");
     }
@@ -538,12 +565,13 @@ dpif_flow_list(const struct dpif *dpif, struct odp_flow flows[], size_t n,
     error = do_ioctl(dpif, ODP_FLOW_LIST, NULL, &fv);
     if (error) {
         *n_out = 0;
-        VLOG_WARN_RL(&error_rl, "dp%u: flow list failed (%s)",
-                     dpif->minor, strerror(error));
+        VLOG_WARN_RL(&error_rl, "%s: flow list failed (%s)",
+                     dpif_name(dpif), strerror(error));
     } else {
         COVERAGE_ADD(dpif_flow_query_list_n, fv.n_flows);
         *n_out = fv.n_flows;
-        VLOG_DBG_RL(&dpmsg_rl, "dp%u: listed %zu flows", dpif->minor, *n_out);
+        VLOG_DBG_RL(&dpmsg_rl, "%s: listed %zu flows",
+                    dpif_name(dpif), *n_out);
     }
     return error;
 }
@@ -573,9 +601,9 @@ dpif_flow_list_all(const struct dpif *dpif,
     }
 
     if (stats.n_flows != n_flows) {
-        VLOG_WARN_RL(&error_rl, "dp%u: datapath stats reported %"PRIu32" "
+        VLOG_WARN_RL(&error_rl, "%s: datapath stats reported %"PRIu32" "
                      "flows but flow listing reported %zu",
-                     dpif->minor, stats.n_flows, n_flows);
+                     dpif_name(dpif), stats.n_flows, n_flows);
     }
     *flowsp = flows;
     *np = n_flows;
@@ -606,7 +634,7 @@ dpif_execute(struct dpif *dpif, uint16_t in_port,
     if (!(error ? VLOG_DROP_WARN(&error_rl) : VLOG_DROP_DBG(&dpmsg_rl))) {
         struct ds ds = DS_EMPTY_INITIALIZER;
         char *packet = ofp_packet_to_string(buf->data, buf->size, buf->size);
-        ds_put_format(&ds, "dp%u: execute ", dpif->minor);
+        ds_put_format(&ds, "%s: execute ", dpif_name(dpif));
         format_odp_actions(&ds, actions, n_actions);
         if (error) {
             ds_put_format(&ds, " failed (%s)", strerror(error));
@@ -631,8 +659,8 @@ dpif_recv(struct dpif *dpif, struct ofpbuf **bufp)
     if (retval < 0) {
         error = errno;
         if (error != EAGAIN) {
-            VLOG_WARN_RL(&error_rl, "dp%u: read failed: %s",
-                         dpif->minor, strerror(error));
+            VLOG_WARN_RL(&error_rl, "%s: read failed: %s",
+                         dpif_name(dpif), strerror(error));
         }
     } else if (retval >= sizeof(struct odp_msg)) {
         struct odp_msg *msg = buf->data;
@@ -642,8 +670,8 @@ dpif_recv(struct dpif *dpif, struct ofpbuf **bufp)
                 void *payload = msg + 1;
                 size_t length = buf->size - sizeof *msg;
                 char *s = ofp_packet_to_string(payload, length, length);
-                VLOG_DBG_RL(&dpmsg_rl, "dp%u: received %s message of length "
-                            "%zu on port %"PRIu16": %s", dpif->minor,
+                VLOG_DBG_RL(&dpmsg_rl, "%s: received %s message of length "
+                            "%zu on port %"PRIu16": %s", dpif_name(dpif),
                             (msg->type == _ODPL_MISS_NR ? "miss"
                              : msg->type == _ODPL_ACTION_NR ? "action"
                              : "<unknown>"),
@@ -655,18 +683,18 @@ dpif_recv(struct dpif *dpif, struct ofpbuf **bufp)
             COVERAGE_INC(dpif_recv);
             return 0;
         } else {
-            VLOG_WARN_RL(&error_rl, "dp%u: discarding message truncated "
+            VLOG_WARN_RL(&error_rl, "%s: discarding message truncated "
                          "from %zu bytes to %d",
-                         dpif->minor, msg->length, retval);
+                         dpif_name(dpif), msg->length, retval);
             error = ERANGE;
         }
     } else if (!retval) {
-        VLOG_WARN_RL(&error_rl, "dp%u: unexpected end of file", dpif->minor);
+        VLOG_WARN_RL(&error_rl, "%s: unexpected end of file", dpif_name(dpif));
         error = EPROTO;
     } else {
         VLOG_WARN_RL(&error_rl,
-                     "dp%u: discarding too-short message (%d bytes)",
-                     dpif->minor, retval);
+                     "%s: discarding too-short message (%d bytes)",
+                     dpif_name(dpif), retval);
         error = ERANGE;
     }
 
@@ -689,7 +717,7 @@ dpif_get_netflow_ids(const struct dpif *dpif,
 }
 \f
 struct dpifmon {
-    struct dpif dpif;
+    struct dpif *dpif;
     struct nl_sock *sock;
     int local_ifindex;
 };
@@ -707,7 +735,7 @@ dpifmon_create(const char *datapath_name, struct dpifmon **monp)
     if (error) {
         goto error;
     }
-    error = dpif_port_get_name(&mon->dpif, ODPP_LOCAL,
+    error = dpif_port_get_name(mon->dpif, ODPP_LOCAL,
                                local_name, sizeof local_name);
     if (error) {
         goto error_close_dpif;
@@ -730,7 +758,7 @@ dpifmon_create(const char *datapath_name, struct dpifmon **monp)
     return 0;
 
 error_close_dpif:
-    dpif_close(&mon->dpif);
+    dpif_close(mon->dpif);
 error:
     free(mon);
     *monp = NULL;
@@ -741,7 +769,7 @@ void
 dpifmon_destroy(struct dpifmon *mon)
 {
     if (mon) {
-        dpif_close(&mon->dpif);
+        dpif_close(mon->dpif);
         nl_sock_destroy(mon->sock);
     }
 }
@@ -776,13 +804,9 @@ again:
                 uint32_t master_ifindex = nl_attr_get_u32(attrs[IFLA_MASTER]);
                 for_us = master_ifindex == mon->local_ifindex;
             } else {
-                /* It's for us if that device is one of our ports.  This is
-                 * open-coded instead of using dpif_port_query_by_name() to
-                 * avoid logging a warning on failure. */
+                /* It's for us if that device is one of our ports. */
                 struct odp_port port;
-                memset(&port, 0, sizeof port);
-                strncpy(port.devname, devname, sizeof port.devname);
-                for_us = !ioctl(mon->dpif.fd, ODP_PORT_QUERY, &port);
+                for_us = !dpif_port_query_by_name(mon->dpif, devname, &port);
             }
 
             if (!for_us) {
@@ -1018,14 +1042,14 @@ get_minor_from_name(const char *name, unsigned int *minor)
 }
 
 static int
-open_by_minor(unsigned int minor, struct dpif *dpif)
+open_by_minor(unsigned int minor, struct dpif **dpifp)
 {
+    struct dpif *dpif;
     int error;
     char *fn;
     int fd;
 
-    dpif->minor = -1;
-    dpif->fd = -1;
+    *dpifp = NULL;
     error = make_openvswitch_device(minor, &fn);
     if (error) {
         return error;
@@ -1038,10 +1062,13 @@ open_by_minor(unsigned int minor, struct dpif *dpif)
         free(fn);
         return error;
     }
-
     free(fn);
+
+    dpif = xmalloc(sizeof *dpif);
+    dpif->name = xasprintf("dp%u", dpif->minor);
     dpif->minor = minor;
     dpif->fd = fd;
+    *dpifp = dpif;
     return 0;
 }
 \f