dpif: Replace dpif_id() by dpif_name().

dpif_id() is often used in error messages, e.g. "dp%u: screwed up".  But
soon we will be generalizing the concept of a datapath, so it is better
to have a function that returns a full name, e.g. "%s: screwed up".
Accordingly, this commit replaces dpif_id() by a new function dpif_name()
that does so.

diff --git a/lib/dpif.c b/lib/dpif.c
index bcb4e7b..7cfe2a7 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -89,8 +89,8 @@ 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;
@@ -102,6 +102,8 @@ void
 dpif_close(struct dpif *dpif)
 {
     if (dpif) {
+        free(dpif->name);
+        dpif->name = NULL;
         close(dpif->fd);
         dpif->fd = -1;
     }
@@ -114,11 +116,11 @@ 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;
@@ -168,6 +170,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 +261,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 +286,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 +303,12 @@ 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));
+        VLOG_WARN_RL(&error_rl, "%s: failed to query port %s: %s",
+                     dpif_name(dpif), devname, strerror(errno));
         return errno;
     }
 }
@@ -415,7 +422,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 +545,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 +581,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 +614,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 +639,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 +650,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 +663,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;
     }
 
@@ -1040,6 +1048,7 @@ open_by_minor(unsigned int minor, struct dpif *dpif)
     }
 
     free(fn);
+    dpif->name = xasprintf("dp%u", dpif->minor);
     dpif->minor = minor;
     dpif->fd = fd;
     return 0;

diff --git a/lib/dpif.h b/lib/dpif.h
index 52e8075..e9b28eb 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -31,7 +31,8 @@ struct ofpbuf;
 
 /* A datapath interface.  Opaque. */
 struct dpif {
-    unsigned int minor;         /* For use in error messages. */
+    char *name;
+    unsigned int minor;
     int fd;
 };
 
@@ -39,7 +40,7 @@ int dpif_open(const char *name, struct dpif *);
 int dpif_create(const char *name, struct dpif *);
 void dpif_close(struct dpif *);
 
-static inline unsigned int dpif_id(const struct dpif *dpif);
+const char *dpif_name(const struct dpif *);
 
 int dpif_delete(struct dpif *);
 
@@ -84,11 +85,6 @@ int dpif_execute(struct dpif *, uint16_t in_port,
 int dpif_recv(struct dpif *, struct ofpbuf **);
 void dpif_recv_wait(struct dpif *);
 
-static inline unsigned int
-dpif_id(const struct dpif *dpif)
-{
-    return dpif->minor;
-}
 void dpif_get_netflow_ids(const struct dpif *,
                           uint8_t *engine_type, uint8_t *engine_id);
 \f

diff --git a/secchan/ofproto.c b/secchan/ofproto.c
index 8cde9bf..e3c1fd6 100644 (file)
--- a/secchan/ofproto.c
+++ b/secchan/ofproto.c
@@ -767,8 +767,8 @@ ofproto_run1(struct ofproto *p)
                  * better destroy us and give up, because we're just going to
                  * spin from here on out. */
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-                VLOG_ERR_RL(&rl, "dp%u: datapath was destroyed externally",
-                            dpif_id(&p->dpif));
+                VLOG_ERR_RL(&rl, "%s: datapath was destroyed externally",
+                            dpif_name(&p->dpif));
                 return ENODEV;
             }
             break;

diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index c44291b..da8c727 100644 (file)
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -398,7 +398,7 @@ show_dpif(struct dpif *dpif)
     size_t n_ports;
     size_t i;
 
-    printf("dp%u:\n", dpif_id(dpif));
+    printf("%s:\n", dpif_name(dpif));
     if (!dpif_get_dp_stats(dpif, &stats)) {
         printf("\tflows: cur:%"PRIu32", soft-max:%"PRIu32", "
                "hard-max:%"PRIu32"\n",

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 47d67e3..1f446db 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -259,8 +259,8 @@ bridge_get_ifaces(struct svec *svec)
             for (j = 0; j < port->n_ifaces; j++) {
                 struct iface *iface = port->ifaces[j];
                 if (iface->dp_ifidx < 0) {
-                    VLOG_ERR("%s interface not in dp%u, ignoring",
-                             iface->name, dpif_id(&br->dpif));
+                    VLOG_ERR("%s interface not in datapath %s, ignoring",
+                             iface->name, dpif_name(&br->dpif));
                 } else {
                     if (iface->dp_ifidx != ODPP_LOCAL) {
                         svec_add(svec, iface->name);
@@ -426,8 +426,9 @@ bridge_reconfigure(void)
                 && strcmp(p->devname, br->name)) {
                 int retval = dpif_port_del(&br->dpif, p->port);
                 if (retval) {
-                    VLOG_ERR("failed to remove %s interface from dp%u: %s",
-                             p->devname, dpif_id(&br->dpif), strerror(retval));
+                    VLOG_ERR("failed to remove %s interface from %s: %s",
+                             p->devname, dpif_name(&br->dpif),
+                             strerror(retval));
                 }
             }
         }
@@ -459,13 +460,14 @@ bridge_reconfigure(void)
                                           internal ? ODP_PORT_INTERNAL : 0);
                 if (error != EEXIST) {
                     if (next_port_no >= 256) {
-                        VLOG_ERR("ran out of valid port numbers on dp%u",
-                                 dpif_id(&br->dpif));
+                        VLOG_ERR("ran out of valid port numbers on %s",
+                                 dpif_name(&br->dpif));
                         goto out;
                     }
                     if (error) {
-                        VLOG_ERR("failed to add %s interface to dp%u: %s",
-                                 if_name, dpif_id(&br->dpif), strerror(error));
+                        VLOG_ERR("failed to add %s interface to %s: %s",
+                                 if_name, dpif_name(&br->dpif),
+                                 strerror(error));
                     }
                     break;
                 }
@@ -492,15 +494,16 @@ bridge_reconfigure(void)
             for (j = 0; j < port->n_ifaces; ) {
                 struct iface *iface = port->ifaces[j];
                 if (iface->dp_ifidx < 0) {
-                    VLOG_ERR("%s interface not in dp%u, dropping",
-                             iface->name, dpif_id(&br->dpif));
+                    VLOG_ERR("%s interface not in %s, dropping",
+                             iface->name, dpif_name(&br->dpif));
                     iface_destroy(iface);
                 } else {
                     if (iface->dp_ifidx == ODPP_LOCAL) {
                         local_iface = iface;
                     }
-                    VLOG_DBG("dp%u has interface %s on port %d",
-                             dpif_id(&br->dpif), iface->name, iface->dp_ifidx);
+                    VLOG_DBG("%s has interface %s on port %d",
+                             dpif_name(&br->dpif),
+                             iface->name, iface->dp_ifidx);
                     j++;
                 }
             }
@@ -837,7 +840,7 @@ bridge_create(const char *name)
 
     list_push_back(&all_bridges, &br->node);
 
-    VLOG_INFO("created bridge %s on dp%u", br->name, dpif_id(&br->dpif));
+    VLOG_INFO("created bridge %s on %s", br->name, dpif_name(&br->dpif));
 
     return br;
 }
@@ -854,8 +857,8 @@ bridge_destroy(struct bridge *br)
         list_remove(&br->node);
         error = dpif_delete(&br->dpif);
         if (error && error != ENOENT) {
-            VLOG_ERR("failed to delete dp%u: %s",
-                     dpif_id(&br->dpif), strerror(error));
+            VLOG_ERR("failed to delete %s: %s",
+                     dpif_name(&br->dpif), strerror(error));
         }
         dpif_close(&br->dpif);
         ofproto_destroy(br->ofproto);
@@ -1251,11 +1254,11 @@ bridge_fetch_dp_ifaces(struct bridge *br)
         struct iface *iface = iface_lookup(br, p->devname);
         if (iface) {
             if (iface->dp_ifidx >= 0) {
-                VLOG_WARN("dp%u reported interface %s twice",
-                          dpif_id(&br->dpif), p->devname);
+                VLOG_WARN("%s reported interface %s twice",
+                          dpif_name(&br->dpif), p->devname);
             } else if (iface_from_dp_ifidx(br, p->port)) {
-                VLOG_WARN("dp%u reported interface %"PRIu16" twice",
-                          dpif_id(&br->dpif), p->port);
+                VLOG_WARN("%s reported interface %"PRIu16" twice",
+                          dpif_name(&br->dpif), p->port);
             } else {
                 port_array_set(&br->ifaces, p->port, iface);
                 iface->dp_ifidx = p->port;