From 31a19d69db3f8058257c47f986ec908571b263e7 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 4 May 2012 10:05:44 -0700 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff --- AUTHORS | 1 + ofproto/ofproto-dpif.c | 56 +++++++++++++++++++++++++++---------- ofproto/ofproto-unixctl.man | 20 +++++++------ tests/ofproto-dpif.at | 2 +- 4 files changed, 55 insertions(+), 24 deletions(-) diff --git a/AUTHORS b/AUTHORS index b4ec9c73..e207f3ca 100644 --- 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 8a732090..11bde38f 100644 --- 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 ad9e02da..9d0c8e32 100644 --- 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 2c5df96c..17b064d5 100644 --- 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 ]) -- 2.30.2