ofproto-dpif-trace: add --name option for ofproto/trace.

Most of commands in ovs-ofctl and ovs-appctl can display port names
instead of port numbers by using --names option. This change adds
similar functionality to ofproto/trace.

For backward compatibility, the default behavior is the same as
before.

Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Nobuhiro MIKI <nmiki@yahoo-corp.jp>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

5 files changed, 94 insertions, 22 deletions

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 109940ad2..527e2f17e 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -213,7 +213,7 @@ parse_flow_and_packet(int argc, const char *argv[],
                       struct ofproto_dpif **ofprotop, struct flow *flow,
                       struct dp_packet **packetp,
                       struct ovs_list *next_ct_states,
-                      bool *consistent)
+                      bool *consistent, bool *names)
 {
     const struct dpif_backer *backer = NULL;
     char *error = NULL;
@@ -233,6 +233,9 @@ parse_flow_and_packet(int argc, const char *argv[],
     if (consistent) {
         *consistent = false;
     }
+    if (names) {
+        *names = false;
+    }
     for (int i = 1; i < argc; i++) {
         const char *arg = argv[i];
         if (!strcmp(arg, "-generate") || !strcmp(arg, "--generate")) {
@@ -270,6 +273,10 @@ parse_flow_and_packet(int argc, const char *argv[],
                    && (!strcmp(arg, "-consistent") ||
                        !strcmp(arg, "--consistent"))) {
             *consistent = true;
+        } else if (names
+                   && (!strcmp(arg, "-names") ||
+                       !strcmp(arg, "--names"))) {
+            *names = true;
         } else if (!strcmp(arg, "--ct-next")) {
             if (i + 1 >= argc) {
                 error = xasprintf("Missing argument for option %s", arg);
@@ -418,11 +425,7 @@ parse_flow_and_packet(int argc, const char *argv[],
         }
 
         struct ofputil_port_map map = OFPUTIL_PORT_MAP_INITIALIZER(&map);
-        const struct ofport *ofport;
-        HMAP_FOR_EACH (ofport, hmap_node, &(*ofprotop)->up.ports) {
-            ofputil_port_map_put(&map, ofport->ofp_port,
-                                 netdev_get_name(ofport->netdev));
-        }
+        ofproto_append_ports_to_map(&map, (*ofprotop)->up.ports);
         char *err = parse_ofp_exact_flow(flow, NULL,
                                          ofproto_get_tun_tab(&(*ofprotop)->up),
                                          args[n_args - 1], &map);
@@ -474,16 +477,17 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
     struct dp_packet *packet;
     char *error;
     struct flow flow;
+    bool names;
     struct ovs_list next_ct_states = OVS_LIST_INITIALIZER(&next_ct_states);
 
     error = parse_flow_and_packet(argc, argv, &ofproto, &flow, &packet,
-                                  &next_ct_states, NULL);
+                                  &next_ct_states, NULL, &names);
     if (!error) {
         struct ds result;
 
         ds_init(&result);
         ofproto_trace(ofproto, &flow, packet, NULL, 0, &next_ct_states,
-                      &result);
+                      &result, names);
         unixctl_command_reply(conn, ds_cstr(&result));
         ds_destroy(&result);
         dp_packet_delete(packet);
@@ -501,6 +505,7 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc,
     enum ofputil_protocol usable_protocols;
     struct ofproto_dpif *ofproto;
     bool enforce_consistency;
+    bool names;
     struct ofpbuf ofpacts;
     struct dp_packet *packet;
     struct ds result;
@@ -530,7 +535,8 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc,
     }
 
     error = parse_flow_and_packet(argc, argv, &ofproto, &match.flow, &packet,
-                                  &next_ct_states, &enforce_consistency);
+                                  &next_ct_states, &enforce_consistency,
+                                  &names);
     if (error) {
         unixctl_command_reply_error(conn, error);
         free(error);
@@ -578,7 +584,8 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc,
     }
 
     ofproto_trace(ofproto, &match.flow, packet,
-                  ofpacts.data, ofpacts.size, &next_ct_states, &result);
+                  ofpacts.data, ofpacts.size, &next_ct_states, &result,
+                  names);
     unixctl_command_reply(conn, ds_cstr(&result));
 
 exit:
@@ -729,7 +736,7 @@ static void
 ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
                 const struct dp_packet *packet, struct ovs_list *recirc_queue,
                 const struct ofpact ofpacts[], size_t ofpacts_len,
-                struct ds *output)
+                struct ds *output, bool names)
 {
     struct ofpbuf odp_actions;
     ofpbuf_init(&odp_actions, 0);
@@ -737,6 +744,8 @@ ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
     struct xlate_in xin;
     struct flow_wildcards wc;
     struct ovs_list trace = OVS_LIST_INITIALIZER(&trace);
+    struct ofputil_port_map map = OFPUTIL_PORT_MAP_INITIALIZER(&map);
+    struct hmap *portno_names = NULL;
     xlate_in_init(&xin, ofproto,
                   ofproto_dpif_get_tables_version(ofproto), flow,
                   flow->in_port.ofp_port, NULL, ntohs(flow->tcp_flags),
@@ -745,12 +754,27 @@ ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
     xin.ofpacts_len = ofpacts_len;
     xin.trace = &trace;
     xin.recirc_queue = recirc_queue;
+    xin.names = names;
+
+    if (names) {
+        ofproto_append_ports_to_map(&map, ofproto->up.ports);
+
+        portno_names = xmalloc(sizeof *portno_names);
+        hmap_init(portno_names);
+
+        struct dpif_port dpif_port;
+        struct dpif_port_dump port_dump;
+        DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, ofproto->backer->dpif) {
+            odp_portno_names_set(portno_names, dpif_port.port_no,
+                                 dpif_port.name);
+        }
+    }
 
     /* Copy initial flow out of xin.flow.  It differs from '*flow' because
      * xlate_in_init() initializes actset_output to OFPP_UNSET. */
     struct flow initial_flow = xin.flow;
     ds_put_cstr(output, "Flow: ");
-    flow_format(output, &initial_flow, NULL);
+    flow_format(output, &initial_flow, &map);
     ds_put_char(output, '\n');
 
     struct xlate_out xout;
@@ -762,18 +786,19 @@ ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
     if (flow_equal(&initial_flow, &xin.flow)) {
         ds_put_cstr(output, "unchanged");
     } else {
-        flow_format(output, &xin.flow, NULL);
+        flow_format(output, &xin.flow, &map);
     }
     ds_put_char(output, '\n');
 
     ds_put_cstr(output, "Megaflow: ");
     struct match match;
     match_init(&match, flow, &wc);
-    match_format(&match, NULL, output, OFP_DEFAULT_PRIORITY);
+    match_format(&match, &map, output, OFP_DEFAULT_PRIORITY);
     ds_put_char(output, '\n');
 
     ds_put_cstr(output, "Datapath actions: ");
-    format_odp_actions(output, odp_actions.data, odp_actions.size, NULL);
+    format_odp_actions(output, odp_actions.data, odp_actions.size,
+                       portno_names);
 
     if (error != XLATE_OK) {
         ds_put_format(output,
@@ -790,6 +815,13 @@ ofproto_trace__(struct ofproto_dpif *ofproto, const struct flow *flow,
         }
     }
 
+    if (names) {
+        ofputil_port_map_destroy(&map);
+
+        odp_portno_names_destroy(portno_names);
+        hmap_destroy(portno_names);
+        free(portno_names);
+    }
 
     xlate_out_uninit(&xout);
     ofpbuf_uninit(&odp_actions);
@@ -809,17 +841,19 @@ void
 ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
               const struct dp_packet *packet,
               const struct ofpact ofpacts[], size_t ofpacts_len,
-              struct ovs_list *next_ct_states, struct ds *output)
+              struct ovs_list *next_ct_states, struct ds *output,
+              bool names)
 {
     struct ovs_list recirc_queue = OVS_LIST_INITIALIZER(&recirc_queue);
     ofproto_trace__(ofproto, flow, packet, &recirc_queue,
-                    ofpacts, ofpacts_len, output);
+                    ofpacts, ofpacts_len, output, names);
 
     struct oftrace_recirc_node *recirc_node;
     LIST_FOR_EACH_POP (recirc_node, node, &recirc_queue) {
         ofproto_trace_recirc_node(recirc_node, next_ct_states, output);
         ofproto_trace__(ofproto, &recirc_node->flow, recirc_node->packet,
-                        &recirc_queue, ofpacts, ofpacts_len, output);
+                        &recirc_queue, ofpacts, ofpacts_len, output,
+                        names);
         oftrace_recirc_node_destroy(recirc_node);
     }
 }
@@ -843,3 +877,13 @@ ofproto_dpif_trace_init(void)
         "[-generate|packet] actions",
         2, INT_MAX, ofproto_unixctl_trace_actions, NULL);
 }
+
+void
+ofproto_append_ports_to_map(struct ofputil_port_map *map, struct hmap ports) {
+    struct ofport *ofport;
+
+    HMAP_FOR_EACH (ofport, hmap_node, &ports) {
+        ofputil_port_map_put(map, ofport->ofp_port,
+                             netdev_get_name(ofport->netdev));
+    }
+}
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index 4b04f1756..f579a5ca4 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -86,7 +86,8 @@ void ofproto_dpif_trace_init(void);
 void ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
               const struct dp_packet *packet,
               const struct ofpact *, size_t ofpacts_len,
-              struct ovs_list *next_ct_states, struct ds *output);
+              struct ovs_list *next_ct_states, struct ds *output,
+              bool names);
 
 struct oftrace_node *oftrace_report(struct ovs_list *, enum oftrace_node_type,
                                     const char *text);
@@ -96,4 +97,6 @@ bool oftrace_add_recirc_node(struct ovs_list *recirc_queue,
                              const struct dp_packet *, uint32_t recirc_id,
                              const uint16_t zone);
 
+void ofproto_append_ports_to_map(struct ofputil_port_map *, struct hmap ports);
+
 #endif /* ofproto-dpif-trace.h */
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 57f94df54..88ae46f14 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1246,7 +1246,8 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
         if (!VLOG_DROP_WARN(&rll)) {
             ds_init(&output);
             ofproto_trace(upcall->ofproto, upcall->flow,
-                          upcall->packet, NULL, 0, NULL, &output);
+                          upcall->packet, NULL, 0, NULL, &output,
+                          false);
             VLOG_WARN("%s", ds_cstr(&output));
             ds_destroy(&output);
         }
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b695baba9..2b1ed9cfe 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -813,9 +813,20 @@ xlate_report_table(const struct xlate_ctx *ctx, struct rule_dpif *rule,
         ds_put_cstr(&s, "Packets are IP fragments and "
                     "the fragment handling mode is \"drop\".");
     } else {
+        struct ofputil_port_map map = OFPUTIL_PORT_MAP_INITIALIZER(&map);
+
+        if (ctx->xin->names) {
+            struct ofproto_dpif *ofprotop;
+            ofprotop = ofproto_dpif_lookup_by_name(ctx->xbridge->name);
+            ofproto_append_ports_to_map(&map, ofprotop->up.ports);
+        }
+
         minimatch_format(&rule->up.cr.match,
                          ofproto_get_tun_tab(&ctx->xin->ofproto->up),
-                         NULL, &s, OFP_DEFAULT_PRIORITY);
+                         &map, &s, OFP_DEFAULT_PRIORITY);
+
+        ofputil_port_map_destroy(&map);
+
         if (ds_last(&s) != ' ') {
             ds_put_cstr(&s, ", ");
         }
@@ -6985,11 +6996,20 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         }
 
         if (OVS_UNLIKELY(ctx->xin->trace)) {
+            struct ofputil_port_map map = OFPUTIL_PORT_MAP_INITIALIZER(&map);
+
+            if (ctx->xin->names) {
+                struct ofproto_dpif *ofprotop;
+                ofprotop = ofproto_dpif_lookup_by_name(ctx->xbridge->name);
+                ofproto_append_ports_to_map(&map, ofprotop->up.ports);
+            }
+
             struct ds s = DS_EMPTY_INITIALIZER;
-            struct ofpact_format_params fp = { .s = &s };
+            struct ofpact_format_params fp = { .s = &s, .port_map = &map };
             ofpacts_format(a, OFPACT_ALIGN(a->len), &fp);
             xlate_report(ctx, OFT_ACTION, "%s", ds_cstr(&s));
             ds_destroy(&s);
+            ofputil_port_map_destroy(&map);
         }
 
         switch (a->type) {
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 2ba90e999..c1af477c4 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -166,6 +166,10 @@ struct xlate_in {
 
     /* UUID of first non-patch port packet was received on.*/
     struct uuid xport_uuid;
+
+    /* If true, port names are displayed instead of port numbers in
+     * tracing translation. */
+    bool names;
 };
 
 void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,