From: Alex Wang <al...@nicira.com>
Since the use of single datapath, all bridges belonging to the same type of
datapath will use the same (single) datapath. This causes confusion in the
current 'ofproto/trace' command. Especially, when given the unrelated
'bridge' and 'in_port' combination, the current implementation will still
be able to process and give misleading output. Thusly, this patch changes
the 'ofproto/trace' command syntax to formats shown as follow.
ofproto/trace [datapath] priority tun_id in_port mark packet
ofproto/trace [datapath] dp_flow [-generate]
ofproto/trace bridge br_flow [-generate]
Therein, the bridge name is replaced by datapath name in the first format,
since the mapping between in_port and the corresponding datapath is unique.
The second format requires datapath name since it uses 'ovs-dpctl dump-flows'
output. And the thrid format requires bridge name since it uses 'ovs-ofctl
add-flow' input like flow.
Signed-off-by: Alex Wang <al...@nicira.com>
---
ofproto/ofproto-dpif.c | 176 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 133 insertions(+), 43 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6ec1c23..6550af3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6073,7 +6073,7 @@ fix_sflow_action(struct action_xlate_ctx *ctx)
}
cookie = ofpbuf_at(ctx->odp_actions, ctx->user_cookie_offset,
- sizeof cookie->sflow);
+ sizeof(*cookie));
ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW);
compose_sflow_cookie(ctx->ofproto, base->vlan_tci,
@@ -8138,7 +8138,9 @@ static void
ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
void *aux OVS_UNUSED)
{
- const char *dpname = argv[1];
+ const char **adjusted_argv;
+ const char *dpname;
+ const struct dpif_backer *backer;
struct ofproto_dpif *ofproto;
struct ofpbuf odp_key;
struct ofpbuf *packet;
@@ -8146,59 +8148,108 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int
argc, const char *argv[],
struct ds result;
struct flow flow;
char *s;
+ bool is_dp;
+ bool has_name;
+ is_dp = false;
+ has_name = false;
+ dpname = NULL;
packet = NULL;
+ backer = NULL;
ofpbuf_init(&odp_key, 0);
ds_init(&result);
- ofproto = ofproto_dpif_lookup(dpname);
- if (!ofproto) {
- unixctl_command_reply_error(conn, "Unknown ofproto (use ofproto/list "
- "for help)");
- goto exit;
+ /* Check the existence of datapath name in the argv. */
+ if (argc == 2 || (argc == 3 && !strcmp(argv[2], "-generate"))
+ || argc == 6) {
+ /* Check the number of datapaths. */
+ if (shash_count(&all_dpif_backers) == 1) {
+ is_dp = true;
+ has_name = false;
+ } else {
+ unixctl_command_reply_error(conn, "Must specify datapath name, "
+ "there is more than one type of datapath");
+ goto exit;
+ }
+ } else {
+ /* Else, the argv[1] is the datapath or bridge name.
+ * And since there are only three types of datapath (ovs-system,
+ * ovs-netdev and ovs-dummy), exact match is used to distingush
+ * datapath name and bridge name. */
+ if (!strcmp(argv[1], "ovs-system") || !strcmp(argv[1], "ovs-netdev")
+ || !strcmp(argv[1], "ovs-dummy")) {
+ is_dp = true;
+ /* extract the type of datapath */
+ dpname = argv[1] + 4;
+ } else {
+ is_dp = false;
+ dpname = argv[1];
+ }
+ has_name = true;
}
- if (argc == 3 || (argc == 4 && !strcmp(argv[3], "-generate"))) {
- /* ofproto/trace dpname flow [-generate] */
- const char *flow_s = argv[2];
- const char *generate_s = argv[3];
- /* Allow 'flow_s' to be either a datapath flow or an OpenFlow-like
- * flow. We guess which type it is based on whether 'flow_s' contains
- * an '(', since a datapath flow always contains '(') but an
- * OpenFlow-like flow should not (in fact it's allowed but I believe
- * that's not documented anywhere).
- *
- * An alternative would be to try to parse 'flow_s' both ways, but then
- * it would be tricky giving a sensible error message. After all, do
- * you just say "syntax error" or do you present both error messages?
- * Both choices seem lousy. */
- if (strchr(flow_s, '(')) {
- int error;
+ /* If datapath is specified, extract the corresponding backer object. */
+ if (is_dp) {
+ if (has_name) {
+ backer = shash_find_data(&all_dpif_backers, dpname);
+ } else {
+ struct shash_node *node;
+ adjusted_argv = argv;
+ node = shash_first(&all_dpif_backers);
+ backer = node->data;
+ }
+ if (!backer) {
+ unixctl_command_reply_error(conn, "Cannot find datapath "
+ "of this name");
+ goto exit;
+ }
+ }
+
+ /* Adjust the argv based on the existence of datapath or bridge name. */
+ if (has_name) {
+ adjusted_argv = argv + 2;
+ } else {
+ adjusted_argv = argv + 1;
+ }
+
+ if (argc > 1 && argc < 5) {
+ /* ofproto/trace [dpname] flow [-generate] */
+ const char *flow_s = adjusted_argv[0];
+ const char *generate_s = adjusted_argv[1];
+ if (is_dp) {
/* Convert string to datapath key. */
- ofpbuf_init(&odp_key, 0);
- error = odp_flow_key_from_string(flow_s, NULL, &odp_key);
- if (error) {
- unixctl_command_reply_error(conn, "Bad flow syntax");
+ if (odp_flow_key_from_string(flow_s, NULL, &odp_key)) {
+ unixctl_command_reply_error(conn, "Bad datapath flow syntax");
goto exit;
}
- /* The user might have specified the wrong ofproto but within the
- * same backer. That's OK, ofproto_receive() can find the right
- * one for us. */
- if (ofproto_receive(ofproto->backer, NULL, odp_key.data,
+ /* Extract the ofproto_dpif object from the ofproto_receive()
+ * function. */
+ if (ofproto_receive(backer, NULL, odp_key.data,
odp_key.size, &flow, NULL, &ofproto, NULL,
&initial_vals)) {
- unixctl_command_reply_error(conn, "Invalid flow");
+ unixctl_command_reply_error(conn, "Invalid datapath flow");
goto exit;
}
ds_put_format(&result, "Bridge: %s\n", ofproto->up.name);
} else {
+ /* ofproto/trace brname flow [-generate] */
char *error_s;
+ /* Extract the ofproto_dpif object from the
+ * ofproto_dpif_lookup() function. */
+ ofproto = ofproto_dpif_lookup(dpname);
+ if (!ofproto) {
+ unixctl_command_reply_error(conn, "Unknown bridge or "
+ "datapath name");
+ goto exit;
+ }
+
error_s = parse_ofp_exact_flow(&flow, argv[2]);
if (error_s) {
- unixctl_command_reply_error(conn, error_s);
+ unixctl_command_reply_error(conn, "Cannot parse the "
+ "openflow flow");
free(error_s);
goto exit;
}
@@ -8206,24 +8257,61 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int
argc, const char *argv[],
initial_vals.vlan_tci = flow.vlan_tci;
initial_vals.tunnel_ip_tos = flow.tunnel.ip_tos;
}
-
/* Generate a packet, if requested. */
if (generate_s) {
packet = ofpbuf_new(0);
flow_compose(packet, &flow);
}
- } else if (argc == 7) {
- /* ofproto/trace dpname priority tun_id in_port mark packet */
- const char *priority_s = argv[2];
- const char *tun_id_s = argv[3];
- const char *in_port_s = argv[4];
- const char *mark_s = argv[5];
- const char *packet_s = argv[6];
+ } else if (argc == 6 || argc == 7) {
+ /* ofproto/trace [dpname] priority tun_id in_port mark packet */
+ const char *priority_s = adjusted_argv[0];
+ const char *tun_id_s = adjusted_argv[1];
+ const char *in_port_s = adjusted_argv[2];
+ const char *mark_s = adjusted_argv[3];
+ const char *packet_s = adjusted_argv[4];
uint32_t in_port = atoi(in_port_s);
ovs_be64 tun_id = htonll(strtoull(tun_id_s, NULL, 0));
uint32_t priority = atoi(priority_s);
uint32_t mark = atoi(mark_s);
const char *msg;
+ const struct ofport_dpif *ofport;
+
+ /* Check the incorrect case:
+ * "ofproto/trace <bridge name> <metadata...> <packet>" */
+ if (has_name && !is_dp && argc == 7) {
+ unixctl_command_reply_error(conn, "Incorrect datapath name");
+ goto exit;
+ }
+
+ /* Extract the ofport_dpif object. */
+ ofport = odp_port_to_ofport(backer, in_port);
+
+ if (!ofport) {
+ unixctl_command_reply_error(conn, "Cannot find ofport, "
+ "from the given in_port");
+ goto exit;
+ }
+ if (!(ofport->bundle)) {
+ unixctl_command_reply_error(conn, "Cannot find the ofbundle "
+ "for the ofport");
+ goto exit;
+ }
+ /* Extract the ofproto_dpif object. */
+ ofproto = ofport->bundle->ofproto;
+ /* Convert the odp port to ofp port. */
+ in_port = ofport->up.ofp_port;
+
+ if (!ofproto) {
+ unixctl_command_reply_error(conn, "Unknown in_port number, "
+ "cannot find matched ofproto");
+ goto exit;
+ }
+
+ if (in_port == OFPP_NONE) {
+ unixctl_command_reply_error(conn, "Cannot convert datapath "
+ "port to openflow port.");
+ goto exit;
+ }
msg = eth_from_hex(packet_s, &packet);
if (msg) {
@@ -8670,8 +8758,10 @@ ofproto_dpif_unixctl_init(void)
unixctl_command_register(
"ofproto/trace",
- "bridge {priority tun_id in_port mark packet | odp_flow [-generate]}",
- 2, 6, ofproto_unixctl_trace, NULL);
+ "[datapath] {priority tun_id in_port mark packet | "
+ "bridge of_flow [-generate]} | "
+ "[datapath] dp_flow [-generate]}",
+ 1, 6, ofproto_unixctl_trace, NULL);
unixctl_command_register("fdb/flush", "[bridge]", 0, 1,
ofproto_unixctl_fdb_flush, NULL);
unixctl_command_register("fdb/show", "bridge", 1, 1,
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
