[ovs-dev] [PATCH 3/3] ofproto/trace: Add --ct-next option to ofproto/trace
Yi-Hung Wei
yihung.wei at gmail.com
Wed Jun 21 18:06:52 UTC 2017
Previous patch enables ofproto/trace to automatically trace a flow
that involves multiple recirculation on conntrack. However, it always
sets the ct_state to trk|est when it processes recirculated conntrack flows.
With this patch, users can customize the expected next ct_state in the
aforementioned use case.
Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
---
NEWS | 2 +
ofproto/ofproto-dpif-trace.c | 111 ++++++++++++++++++++++++++++++++++++-------
ofproto/ofproto-dpif-trace.h | 6 +++
ofproto/ofproto-unixctl.man | 54 +++++++++++++++++++--
tests/completion.at | 8 ++--
tests/ofproto-dpif.at | 33 +++++++++----
6 files changed, 182 insertions(+), 32 deletions(-)
diff --git a/NEWS b/NEWS
index 4ea7e628e1fc..1824ec9de744 100644
--- a/NEWS
+++ b/NEWS
@@ -58,6 +58,8 @@ Post-v2.7.0
- Fedora Packaging:
* OVN services are no longer restarted automatically after upgrade.
- Add --cleanup option to command 'ovs-appctl exit' (see ovs-vswitchd(8)).
+ - Add --ct-next option to command 'ovs-appctl ofprot/trace' (see
+ ovs-vswitchd(8)).
- L3 tunneling:
* Add "layer3" options for tunnel ports that support non-Ethernet (L3)
payload (GRE, VXLAN-GPE).
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index d8cdd92493cf..e75c62d464eb 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -18,6 +18,7 @@
#include "ofproto-dpif-trace.h"
+#include "conntrack.h"
#include "dpif.h"
#include "ofproto-dpif-xlate.h"
#include "openvswitch/ofp-parse.h"
@@ -26,7 +27,7 @@
static void ofproto_trace(struct ofproto_dpif *, struct flow *,
const struct dp_packet *packet,
const struct ofpact[], size_t ofpacts_len,
- struct ds *);
+ struct ds *, struct ovs_list *next_ct_states);
static void oftrace_node_destroy(struct oftrace_node *);
/* Creates a new oftrace_node, populates it with the given 'type' and a copy of
@@ -119,6 +120,17 @@ oftrace_recirc_node_destroy(struct oftrace_recirc_node *node)
free(node);
}
+static uint32_t
+oftrace_next_ct_state(struct ovs_list *next_ct_states)
+{
+ struct oftrace_next_ct_state *next_ct_state;
+ ASSIGN_CONTAINER(next_ct_state, ovs_list_pop_front(next_ct_states), node);
+ uint32_t state = next_ct_state->state;
+ free(next_ct_state);
+
+ return state;
+}
+
static void
oftrace_node_print_details(struct ds *output,
const struct ovs_list *nodes, int level)
@@ -160,18 +172,47 @@ oftrace_node_print_details(struct ds *output,
}
}
+static char * OVS_WARN_UNUSED_RESULT
+parse_oftrace_options(int argc, const char *argv[],
+ struct ovs_list *next_ct_states)
+{
+ char *error = NULL;
+ int k;
+
+ for (k = 0; k < argc; k++) {
+ if (!strncmp(argv[k], "--ct-next", 9)) {
+ if (k + 1 > argc) {
+ error = xasprintf("Missing argument for option %s", argv[k]);
+ return error;
+ }
+
+ uint32_t ct_state = parse_ct_state(argv[++k], 0);
+ struct oftrace_next_ct_state *next_state =
+ xmalloc(sizeof *next_state);
+ next_state->state = ct_state;
+ ovs_list_push_back(next_ct_states, &next_state->node);
+ } else {
+ error = xasprintf("Invalid option %s", argv[k]);
+ return error;
+ }
+ }
+
+ return error;
+}
+
/* Parses the 'argc' elements of 'argv', ignoring argv[0]. The following
* forms are supported:
*
- * - [dpname] odp_flow [-generate | packet]
- * - bridge br_flow [-generate | packet]
+ * - [dpname] odp_flow [OPTIONS] [-generate | packet]
+ * - bridge br_flow [OPTIONS] [-generate | packet]
*
* On success, initializes '*ofprotop' and 'flow' and returns NULL. On failure
* returns a nonnull malloced error message. */
static char * OVS_WARN_UNUSED_RESULT
parse_flow_and_packet(int argc, const char *argv[],
struct ofproto_dpif **ofprotop, struct flow *flow,
- struct dp_packet **packetp)
+ struct dp_packet **packetp,
+ struct ovs_list *next_ct_states)
{
const struct dpif_backer *backer = NULL;
const char *error = NULL;
@@ -180,6 +221,7 @@ parse_flow_and_packet(int argc, const char *argv[],
struct dp_packet *packet;
struct ofpbuf odp_key;
struct ofpbuf odp_mask;
+ int first_option;
ofpbuf_init(&odp_key, 0);
ofpbuf_init(&odp_mask, 0);
@@ -199,6 +241,25 @@ parse_flow_and_packet(int argc, const char *argv[],
error = NULL;
}
+ /* Parse options. */
+ if (argc >= 4) {
+ if (!strncmp(argv[2], "--", 2)) {
+ first_option = 2;
+ } else if (!strncmp(argv[3], "--", 2)) {
+ first_option = 3;
+ } else {
+ error = "Syntax error";
+ goto exit;
+ }
+
+ m_err = parse_oftrace_options(argc - first_option, argv + first_option,
+ next_ct_states);
+ if (m_err) {
+ goto exit;
+ }
+ argc = first_option;
+ }
+
/* odp_flow can have its in_port specified as a name instead of port no.
* We do not yet know whether a given flow is a odp_flow or a br_flow.
* But, to know whether a flow is odp_flow through odp_flow_from_string(),
@@ -338,13 +399,16 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
struct dp_packet *packet;
char *error;
struct flow flow;
+ struct ovs_list next_ct_states = OVS_LIST_INITIALIZER(&next_ct_states);
- error = parse_flow_and_packet(argc, argv, &ofproto, &flow, &packet);
+ error = parse_flow_and_packet(argc, argv, &ofproto, &flow, &packet,
+ &next_ct_states);
if (!error) {
struct ds result;
ds_init(&result);
- ofproto_trace(ofproto, &flow, packet, NULL, 0, &result);
+ ofproto_trace(ofproto, &flow, packet, NULL, 0, &result,
+ &next_ct_states);
unixctl_command_reply(conn, ds_cstr(&result));
ds_destroy(&result);
dp_packet_delete(packet);
@@ -366,6 +430,7 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc,
struct ds result;
struct match match;
uint16_t in_port;
+ struct ovs_list next_ct_states = OVS_LIST_INITIALIZER(&next_ct_states);
/* Three kinds of error return values! */
enum ofperr retval;
@@ -395,7 +460,8 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc,
enforce_consistency = false;
}
- error = parse_flow_and_packet(argc, argv, &ofproto, &match.flow, &packet);
+ error = parse_flow_and_packet(argc, argv, &ofproto, &match.flow, &packet,
+ &next_ct_states);
if (error) {
unixctl_command_reply_error(conn, error);
free(error);
@@ -443,7 +509,7 @@ ofproto_unixctl_trace_actions(struct unixctl_conn *conn, int argc,
}
ofproto_trace(ofproto, &match.flow, packet,
- ofpacts.data, ofpacts.size, &result);
+ ofpacts.data, ofpacts.size, &result, &next_ct_states);
unixctl_command_reply(conn, ds_cstr(&result));
exit:
@@ -541,7 +607,7 @@ static void
ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
const struct dp_packet *packet,
const struct ofpact ofpacts[], size_t ofpacts_len,
- struct ds *output)
+ struct ds *output, struct ovs_list *next_ct_states)
{
struct ovs_list recirc_queue = OVS_LIST_INITIALIZER(&recirc_queue);
oftrace_add_recirc_node(&recirc_queue, OFT_INIT, flow, flow->recirc_id);
@@ -553,9 +619,21 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
ASSIGN_CONTAINER(recirc_node, node, node);
if (recirc_node->type == OFT_CONNTRACK) {
- recirc_node->flow->ct_state = CS_ESTABLISHED | CS_TRACKED;
- ds_put_cstr(output, "\n\nResume conntrack prcessing with "
- "default ct_state=trk|est.\n");
+ if (ovs_list_is_empty(next_ct_states)) {
+ recirc_node->flow->ct_state = CS_ESTABLISHED | CS_TRACKED;
+ ds_put_cstr(output, "\n\nResume conntrack prcessing with "
+ "default ct_state=trk|est. Use --ct-next "
+ "to customize\n");
+ } else {
+ recirc_node->flow->ct_state =
+ oftrace_next_ct_state(next_ct_states);
+ struct ds s = DS_EMPTY_INITIALIZER;
+ format_flags(&s, ct_state_to_string,
+ recirc_node->flow->ct_state, '|');
+ ds_put_format(output, "\n\nResume conntrack prcessing with "
+ "ct_state=%s\n", ds_cstr(&s));
+ ds_destroy(&s);
+ }
ds_put_char_multiple(output, '-', 79);
ds_put_char(output, '\n');
}
@@ -576,10 +654,11 @@ ofproto_dpif_trace_init(void)
unixctl_command_register(
"ofproto/trace",
- "{[dp_name] odp_flow | bridge br_flow} [-generate|packet]",
- 1, 3, ofproto_unixctl_trace, NULL);
+ "{[dp_name] odp_flow | bridge br_flow} [OPTIONS...] "
+ "[-generate|packet]", 1, INT_MAX, ofproto_unixctl_trace, NULL);
unixctl_command_register(
"ofproto/trace-packet-out",
- "[-consistent] {[dp_name] odp_flow | bridge br_flow} [-generate|packet] actions",
- 2, 6, ofproto_unixctl_trace_actions, NULL);
+ "[-consistent] {[dp_name] odp_flow | bridge br_flow} [OPTIONS...] "
+ "[-generate|packet] actions",
+ 2, INT_MAX, ofproto_unixctl_trace_actions, NULL);
}
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index f7299fd42848..9bb080534a7a 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -73,6 +73,12 @@ struct oftrace_recirc_node {
struct flow *flow;
};
+/* A node within a next_ct_state list. */
+struct oftrace_next_ct_state {
+ struct ovs_list node; /* In next_ct_states. */
+ uint32_t state;
+};
+
void ofproto_dpif_trace_init(void);
struct oftrace_node *oftrace_report(struct ovs_list *, enum oftrace_node_type,
diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man
index 423837351910..e6c37894e980 100644
--- a/ofproto/ofproto-unixctl.man
+++ b/ofproto/ofproto-unixctl.man
@@ -6,10 +6,10 @@ These commands manage the core OpenFlow switch implementation (called
Lists the names of the running ofproto instances. These are the names
that may be used on \fBofproto/trace\fR.
.
-.IP "\fBofproto/trace\fR [\fIdpname\fR] \fIodp_flow\fR [\fB\-generate \fR| \fIpacket\fR]"
-.IQ "\fBofproto/trace\fR \fIbridge\fR \fIbr_flow\fR [\fB\-generate \fR| \fIpacket\fR]"
-.IQ "\fBofproto/trace\-packet\-out\fR [\fB\-consistent\fR] [\fIdpname\fR] \fIodp_flow\fR [\fB\-generate \fR| \fIpacket\fR] \fIactions\fR"
-.IQ "\fBofproto/trace\-packet\-out\fR [\fB\-consistent\fR] \fIbridge\fR \fIbr_flow\fR [\fB\-generate \fR| \fIpacket\fR] \fIactions\fR"
+.IP "\fBofproto/trace\fR [\fIdpname\fR] \fIodp_flow\fR [\fIOPTIONS\fR] [\fB\-generate \fR| \fIpacket\fR]"
+.IQ "\fBofproto/trace\fR \fIbridge\fR \fIbr_flow\fR [\fIOPTIONS\fR] [\fB\-generate \fR| \fIpacket\fR]"
+.IQ "\fBofproto/trace\-packet\-out\fR [\fB\-consistent\fR] [\fIdpname\fR] \fIodp_flow\fR [\fIOPTIONS\fR] [\fB\-generate \fR| \fIpacket\fR] \fIactions\fR"
+.IQ "\fBofproto/trace\-packet\-out\fR [\fB\-consistent\fR] \fIbridge\fR \fIbr_flow\fR [\fIOPTIONS\fR] [\fB\-generate \fR| \fIpacket\fR] \fIactions\fR"
Traces the path of an imaginary packet through \fIswitch\fR and
reports the path that it took. The initial treatment of the packet
varies based on the command:
@@ -48,6 +48,52 @@ wildcards.) \fIbridge\fR names of the bridge through which
.RE
.
.IP
+.RS
+\fBofproto/trace\fR supports the following options:
+.
+.IP "--ct-next \fIflags\fR"
+When the traced flow triggers conntrack actions, \fBofproto/trace\fR
+will automatically trace the forked packet processing pipeline with
+user specified ct_state. This option sets the ct_state flags that the
+conntrack module will report. The \fIflags\fR must be a comma- or
+space-separated list of the following connection tracking flags:
+.
+.RS
+.IP \(bu
+\fBtrk\fR: Include to indicate connection tracking has taken place.
+.
+.IP \(bu
+\fBnew\fR: Include to indicate a new flow.
+.
+.IP \(bu
+\fBest\fR: Include to indicate an established flow.
+.
+.IP \(bu
+\fBrel\fR: Include to indicate a related flow.
+.
+.IP \(bu
+\fBrpl\fR: Include to indicate a reply flow.
+.
+.IP \(bu
+\fBinv\fR: Include to indicate a connection entry in a bad state.
+.
+.IP \(bu
+\fBdnat\fR: Include to indicate a packet whose destination IP address has been
+changed.
+.
+.IP \(bu
+\fBsnat\fR: Include to indicate a packet whose source IP address has been
+changed.
+.
+.RE
+.
+.IP
+When --ct-next is unspecified, or when there are fewer --ct-next options that
+ct actions, the \fIflags\fR default to trk,est.
+.
+.RE
+.
+.IP
Most commonly, one specifies only a flow, using one of the forms
above, but sometimes one might need to specify an actual packet
instead of just a flow:
diff --git a/tests/completion.at b/tests/completion.at
index e28c75239227..00e3a46b8bfa 100644
--- a/tests/completion.at
+++ b/tests/completion.at
@@ -171,7 +171,7 @@ AT_CLEANUP
# complex completion check - ofproto/trace
-# ofproto/trace {[dp_name] odp_flow | bridge br_flow} [-generate|packet]
+# ofproto/trace {[dp_name] odp_flow | bridge br_flow} [OPTIONS] [-generate|packet]
# test expansion on 'dp|dp_name' and 'bridge'
AT_SETUP([appctl-bashcomp - complex completion check 3])
AT_SKIP_IF([test -z ${BASH_VERSION+x}])
@@ -209,7 +209,8 @@ AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})], [0])
# set odp_flow to some random string, should go to the next level.
INPUT="$(bash ovs-appctl-bashcomp.bash debug ovs-appctl ofproto/trace ovs-dummy "in_port(123),mac(),ip,tcp" TAB 2>&1)"
MATCH="$(GET_COMP_STR([-generate], [-generate])
-GET_COMP_STR([packet], []))"
+GET_COMP_STR([packet], [])
+GET_COMP_STR([OPTIONS...], []))"
AT_CHECK_UNQUOTED([GET_EXPAN(${INPUT})],
[0], [dnl
${MATCH}
@@ -242,7 +243,8 @@ AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})], [0])
# set argument to some random string, should go to the odp_flow path.
INPUT="$(bash ovs-appctl-bashcomp.bash debug ovs-appctl ofproto/trace "in_port(123),mac(),ip,tcp" TAB 2>&1)"
MATCH="$(GET_COMP_STR([-generate], [-generate])
-GET_COMP_STR([packet], []))"
+GET_COMP_STR([packet], [])
+GET_COMP_STR([OPTIONS...], []))"
AT_CHECK_UNQUOTED([GET_EXPAN(${INPUT})],
[0], [dnl
${MATCH}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index c51c689b9f46..142322afd777 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5208,14 +5208,6 @@ Trailing garbage in packet data
ovs-appctl: ovs-vswitchd: server returned an error
])
-# Test incorrect command: ofproto/trace with 4 arguments
-AT_CHECK([ovs-appctl ofproto/trace \
- arg1, arg2, arg3, arg4], [2], [stdout],[stderr])
-AT_CHECK([tail -2 stderr], [0], [dnl
-"ofproto/trace" command takes at most 3 arguments
-ovs-appctl: ovs-vswitchd: server returned an error
-])
-
# Test incorrect command: ofproto/trace with 0 argument
AT_CHECK([ovs-appctl ofproto/trace ], [2], [stdout],[stderr])
AT_CHECK([tail -2 stderr], [0], [dnl
@@ -9713,13 +9705,14 @@ AT_CLEANUP
AT_SETUP([ofproto-dpif - conntrack - ofproto/trace])
OVS_VSWITCHD_START
-add_of_ports br0 1 2
+add_of_ports br0 1 2 3 4
AT_DATA([flows.txt], [dnl
dnl Table 0
dnl
table=0,priority=100,arp,action=normal
table=0,priority=10,udp,action=ct(table=1,zone=0)
+table=0,priority=10,tcp,action=ct(table=2,zone=1)
table=0,priority=1,action=drop
dnl
dnl Table 1
@@ -9728,6 +9721,18 @@ table=1,priority=10,in_port=1,ct_state=+trk+new,udp,action=ct(commit,zone=0),2
table=1,priority=10,in_port=1,ct_state=+trk+est,udp,action=2
table=1,priority=10,in_port=2,ct_state=+trk+est,udp,action=1
table=1,priority=1,action=drop
+dnl
+dnl Table 2
+dnl
+table=2,priority=10,in_port=1,tcp,ct_state=+trk+new,tcp,action=ct(commit,zone=1),ct(table=3,zone=2)
+table=2,priority=10,in_port=1,tcp,ct_state=+trk+est,tcp,action=ct(table=3,zone=2)
+table=2,priority=1,action=drop
+dnl
+dnl Table 3
+dnl
+table=3,priority=10,in_port=1,tcp,ct_state=+trk+new,tcp,action=ct(commit,zone=2),4
+table=3,priority=10,in_port=1,tcp,ct_state=+trk+est,tcp,action=3
+table=2,priority=1,action=drop
])
AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
@@ -9742,6 +9747,16 @@ AT_CHECK([tail -1 stdout], [0],
[Datapath actions: 2
])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,tcp'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+ [Datapath actions: 3
+])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,tcp' --ct-next 'trk,new' --ct-next 'trk,new' ], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+ [Datapath actions: ct(commit,zone=2),4
+])
+
OVS_VSWITCHD_STOP
AT_CLEANUP
--
2.7.4
More information about the dev
mailing list