From: Ben Pfaff Date: Tue, 16 Jun 2009 18:00:22 +0000 (-0700) Subject: dpif: Replace dpif_id() by dpif_name(). X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b29ba12809fc48d1b4e417c7444e53d984292a5f;p=openvswitch 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 bcb4e7b4..7cfe2a7c 100644 --- 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" : ""), @@ -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 52e8075b..e9b28ebb 100644 --- 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); diff --git a/secchan/ofproto.c b/secchan/ofproto.c index 8cde9bfe..e3c1fd61 100644 --- 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 c44291b6..da8c727f 100644 --- 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 47d67e37..1f446db0 100644 --- 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;