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....@gmail.com> --- NEWS | 2 + ofproto/ofproto-dpif-trace.c | 124 +++++++++++++++++++++++++++++++++++++------ ofproto/ofproto-dpif-trace.h | 6 +++ ofproto/ofproto-unixctl.man | 54 +++++++++++++++++-- tests/completion.at | 8 +-- tests/ofproto-dpif.at | 33 ++++++++---- 6 files changed, 195 insertions(+), 32 deletions(-) diff --git a/NEWS b/NEWS index 4ea7e628e1fc..ff8bc590e64a 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 ofproto/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 362f932a23a7..e102b3bf092e 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,28 @@ oftrace_recirc_node_destroy(struct oftrace_recirc_node *node) } static void +oftrace_push_ct_state(struct ovs_list *next_ct_states, uint32_t ct_state) +{ + struct oftrace_next_ct_state *next_ct_state = + xmalloc(sizeof *next_ct_state); + next_ct_state->state = ct_state; + ovs_list_push_back(next_ct_states, &next_ct_state->node); +} + +static uint32_t +oftrace_pop_ct_state(struct ovs_list *next_ct_states) +{ + struct ovs_list *list_node = ovs_list_pop_front(next_ct_states); + struct oftrace_next_ct_state *next_ct_state; + + ASSIGN_CONTAINER(next_ct_state, list_node, node); + uint32_t ct_state = next_ct_state->state; + free(next_ct_state); + + return ct_state; +} + +static void oftrace_node_print_details(struct ds *output, const struct ovs_list *nodes, int level) { @@ -159,18 +182,49 @@ 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) +{ + int k; + struct ds ds = DS_EMPTY_INITIALIZER; + + for (k = 0; k < argc; k++) { + if (!strncmp(argv[k], "--ct-next", 9)) { + if (k + 1 > argc) { + return xasprintf("Missing argument for option %s", argv[k]); + } + + uint32_t ct_state; + if (!parse_ct_state(argv[++k], 0, &ct_state, &ds)) { + return ds_steal_cstr(&ds); + } + if (!validate_ct_state(ct_state, &ds)) { + return ds_steal_cstr(&ds); + } + oftrace_push_ct_state(next_ct_states, ct_state); + } else { + return xasprintf("Invalid option %s", argv[k]); + } + } + + ds_destroy(&ds); + return NULL; +} + /* 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; @@ -179,6 +233,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); @@ -198,6 +253,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: invalid option"; + 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(), @@ -337,13 +411,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); @@ -365,6 +442,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; @@ -394,7 +472,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); @@ -442,7 +521,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: @@ -540,7 +619,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_RECIRC_INIT, flow, @@ -553,9 +632,21 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow, ASSIGN_CONTAINER(recirc_node, node, node); if (recirc_node->type == OFT_RECIRC_CONNTRACK) { - recirc_node->flow->ct_state = CS_TRACKED | CS_NEW; - ds_put_cstr(output, "\n\nResume conntrack prcessing with " - "default ct_state=trk|new.\n"); + if (ovs_list_is_empty(next_ct_states)) { + recirc_node->flow->ct_state = CS_TRACKED | CS_NEW; + ds_put_cstr(output, "\n\nResume conntrack prcessing with " + "default ct_state=trk|new (use --ct-next " + "to customize).\n"); + } else { + recirc_node->flow->ct_state = + oftrace_pop_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 +667,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 db3be290e477..8bcc7caf1a57 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_states 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..f511c392b548 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 than +ct actions, the \fIflags\fR default to trk,new. +. +.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 d28f135b4d60..7f79248c7340 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: ct(commit),2 ]) +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,tcp'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: ct(commit,zone=2),4 +]) + +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,tcp' --ct-next 'trk,est' --ct-next 'trk,est' ], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: 3 +]) + OVS_VSWITCHD_STOP AT_CLEANUP -- 2.7.4 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev