ofproto-dpif: Accept OpenFlow-like flows in "ofproto/trace".

Until now it has not been possible to directly trace flows that include
register values and other concepts that are not in datapath flows, because
"ofproto/trace" requires a flow in the format output by "ovs-dpctl
dump-flows", which doesn't know anything about registers.  This commit
makes it possible to instead specify an OpenFlow-like flow.

Feature #10084.
Requested-by: Igor Ganichev <iganichev@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index b4ec9c736b0ea386ed54669e22dfb20afc2dc3e8..e207f3ca826d3a47fa0c6ec6a7d298fafe79e465 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -101,6 +101,7 @@ Hassan Khan             hassan.khan@seecs.edu.pk
 Hector Oron             hector.oron@gmail.com
 Henrik Amren            henrik@nicira.com
 Hiroshi Tanaka          htanaka@nicira.com
+Igor Ganichev           iganichev@nicira.com
 Jacob Cherkas           jcherkas@nicira.com
 Jad Naous               jnaous@gmail.com
 Jamal Hadi Salim        hadi@cyberus.ca

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8a7320903fde131114ee0fa58571d6efd5844e37..11bde38fd05bb4eb9ac3b47726eadc7f53c3db2f 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -42,6 +42,7 @@
 #include "odp-util.h"
 #include "ofp-util.h"
 #include "ofpbuf.h"
+#include "ofp-parse.h"
 #include "ofp-print.h"
 #include "ofproto-dpif-governor.h"
 #include "ofproto-dpif-sflow.h"
@@ -6387,23 +6388,48 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
         /* ofproto/trace dpname flow [-generate] */
         const char *flow_s = argv[2];
         const char *generate_s = argv[3];
-        int error;
 
-        /* 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");
-            goto exit;
-        }
+        /* 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;
+
+            /* 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");
+                goto exit;
+            }
 
-        /* Convert odp_key to flow. */
-        error = ofproto_dpif_extract_flow_key(ofproto, odp_key.data,
-                                              odp_key.size, &flow,
-                                              &initial_tci, NULL);
-        if (error == ODP_FIT_ERROR) {
-            unixctl_command_reply_error(conn, "Invalid flow");
-            goto exit;
+            /* Convert odp_key to flow. */
+            error = ofproto_dpif_extract_flow_key(ofproto, odp_key.data,
+                                                  odp_key.size, &flow,
+                                                  &initial_tci, NULL);
+            if (error == ODP_FIT_ERROR) {
+                unixctl_command_reply_error(conn, "Invalid flow");
+                goto exit;
+            }
+        } else {
+            char *error_s;
+
+            error_s = parse_ofp_exact_flow(&flow, argv[2]);
+            if (error_s) {
+                unixctl_command_reply_error(conn, error_s);
+                free(error_s);
+                goto exit;
+            }
+
+            initial_tci = flow.vlan_tci;
+            vsp_adjust_flow(ofproto, &flow);
         }
 
         /* Generate a packet, if requested. */

diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man
index ad9e02da9c521b67175a79cf4d23a3cdc875757d..9d0c8e32783e0ee1dccd7ef7197d7abb56683e55 100644 (file)
--- a/ofproto/ofproto-unixctl.man
+++ b/ofproto/ofproto-unixctl.man
@@ -7,7 +7,7 @@ Lists the names of the running ofproto instances.  These are the names
 that may be used on \fBofproto/trace\fR.
 .
 .IP "\fBofproto/trace \fIswitch priority tun_id in_port packet\fR"
-.IQ "\fBofproto/trace \fIswitch odp_flow \fB\-generate\fR"
+.IQ "\fBofproto/trace \fIswitch flow \fB\-generate\fR"
 Traces the path of an imaginary packet through \fIswitch\fR.  Both
 forms require \fIswitch\fR, the switch on which the packet arrived
 (one of those listed by \fBofproto/list\fR).  The first form specifies
@@ -31,32 +31,36 @@ utilities provide easier ways.
 .IP
 The second form specifies the packet's contents implicitly:
 .RS
-.IP "\fIodp_flow\fR"
-A flow in the form printed by \fBovs\-dpctl\fR(8)'s \fBdump\-flows\fR
+.IP "\flow\fR"
+A flow in one of two forms: either the form printed by
+\fBovs\-dpctl\fR(8)'s \fBdump\-flows\fR command, or in a format
+similar to that accepted by \Bovs\-ofctl\fR(8)'s \fBadd\-flow\fR
 command.  This is not an OpenFlow flow: besides other differences, it
 never contains wildcards.  \fB\*(PN\fR generates an arbitrary packet
-that has the specified \fIodp_flow\fR.
+that has the specified \flow\fR.
 .RE
 .IP
 \fB\*(PN\fR will respond with extensive information on how the packet
 would be handled if it were to be received.  The packet will not
 actually be sent, but side effects such as MAC learning will occur.
 .
-.IP "\fBofproto/trace \fIswitch odp_flow\fR"
+.IP "\fBofproto/trace \fIswitch flow\fR"
 Traces the path of a packet in an imaginary flow through
 \fIswitch\fR.  The arguments are:
 .RS
 .IP "\fIswitch\fR"
 The switch on which the packet arrived (one of those listed by
 \fBofproto/list\fR).
-.IP "\fIodp_flow\fR"
-A flow in the form printed by \fBovs\-dpctl\fR(8)'s \fBdump\-flows\fR
+.IP "\flow\fR"
+A flow in one of two forms: either the form printed by
+\fBovs\-dpctl\fR(8)'s \fBdump\-flows\fR command, or in a format
+similar to that accepted by \Bovs\-ofctl\fR(8)'s \fBadd\-flow\fR
 command.  This is not an OpenFlow flow: besides other differences, it
 never contains wildcards.
 .RE
 .IP
 \fB\*(PN\fR will respond with extensive information on how a packet
-in \fIodp_flow\fR would be handled if it were received by
+in \flow\fR would be handled if it were received by
 \fIswitch\fR.  No packet will actually be sent.  Some side effects may
 occur, but MAC learning in particular will not.
 .IP

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 2c5df96c5bbe80764f0c717ced9dc998de68db7c..17b064d5ab7c005a9c40b6562d56341911208d9a 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -11,7 +11,7 @@ table=1 in_port=2 priority=1500 icmp actions=output(17),resubmit(,2)
 table=1 in_port=3 priority=1500 icmp actions=output(14),resubmit(,2)
 ])
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
-AT_CHECK([ovs-appctl ofproto/trace br0 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
   [Datapath actions: 10,11,12,13,14,15,16,17,18,19,20,21
 ])