Skip to content

Commit

Permalink
ofproto-dpif: Make ofproto/trace output easier to read.
Browse files Browse the repository at this point in the history
"ovs-appctl ofproto/trace" is invaluable for debugging, but as the users of
Open vSwitch have evolved it has failed to keep up with the times.  It's
pretty easy to design OpenFlow tables and pipelines that resubmit dozens of
times.  Each resubmit causes an additional tab of indentation, so the
output wraps around, sometimes again and again, and makes the output close
to unreadable.

ovn-trace pioneered better formatting for tracing in OVN logical datapaths,
mostly by not increasing indentation for tail recursion, which in practice
gets rid of almost all indentation.

This commit experiments with redoing ofproto/trace the same way.  Try
looking at, for example, the testsuite output for test 2282 "ovn -- 3 HVs,
3 LRs connected via LS, source IP based routes".  Without this commit, it
indents 61 levels (488 spaces!).  With this commit, it indents 1 level
(4 spaces) and it's possible to actually understand what's going on almost
at a glance.

To see this for yourself, try the following command either with or without
this commit (but be sure to keep the change to ovn.at that adds an
ofproto/trace to the test):
make check TESTSUITEFLAGS='-d 2282' && less tests/testsuite.dir/2282/testsuite.log

Signed-off-by: Ben Pfaff <[email protected]>
Acked-by: Lance Richardson <[email protected]>
Acked-by: Justin Pettit <[email protected]>
  • Loading branch information
blp committed Jan 12, 2017
1 parent 9fff138 commit 2d9b49d
Show file tree
Hide file tree
Showing 13 changed files with 924 additions and 628 deletions.
268 changes: 168 additions & 100 deletions Documentation/tutorials/ovs-advanced.rst

Large diffs are not rendered by default.

15 changes: 5 additions & 10 deletions lib/nx-match.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
* Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -1768,15 +1768,13 @@ nxm_execute_stack_push(const struct ofpact_stack *push,
nx_stack_push(stack, &dst_value.u8[sizeof dst_value - bytes], bytes);
}

void
bool
nxm_execute_stack_pop(const struct ofpact_stack *pop,
struct flow *flow, struct flow_wildcards *wc,
struct ofpbuf *stack)
{
uint8_t src_bytes;
const void *src = nx_stack_pop(stack, &src_bytes);

/* Only pop if stack is not empty. Otherwise, give warning. */
if (src) {
union mf_subvalue src_value;
uint8_t dst_bytes = DIV_ROUND_UP(pop->subfield.n_bits, 8);
Expand All @@ -1790,13 +1788,10 @@ nxm_execute_stack_pop(const struct ofpact_stack *pop,
(union mf_subvalue *)&exact_match_mask,
&wc->masks);
mf_write_subfield_flow(&pop->subfield, &src_value, flow);
return true;
} else {
if (!VLOG_DROP_WARN(&rl)) {
char *flow_str = flow_to_string(flow);
VLOG_WARN_RL(&rl, "Failed to pop from an empty stack. On flow\n"
" %s", flow_str);
free(flow_str);
}
/* Attempted to pop from an empty stack. */
return false;
}
}

Expand Down
6 changes: 3 additions & 3 deletions lib/nx-match.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ void *nx_stack_pop(struct ofpbuf *stack, uint8_t *bytes);
void nxm_execute_stack_push(const struct ofpact_stack *,
const struct flow *, struct flow_wildcards *,
struct ofpbuf *);
void nxm_execute_stack_pop(const struct ofpact_stack *,
struct flow *, struct flow_wildcards *,
struct ofpbuf *);
bool nxm_execute_stack_pop(const struct ofpact_stack *,
struct flow *, struct flow_wildcards *,
struct ofpbuf *);

ovs_be64 oxm_bitmap_from_mf_bitmap(const struct mf_bitmap *, enum ofp_version);
struct mf_bitmap oxm_bitmap_to_mf_bitmap(ovs_be64 oxm_bitmap,
Expand Down
Loading

0 comments on commit 2d9b49d

Please sign in to comment.