The compare-odp-actions.pl utility isn't fully general, even for its
intended purpose of allowing sets of ODP actions to be compared
ignoring unimportant differences in ordering of output actions and
VLAN set actions. I decided that the proper way to do it was to have
a utility that can actually parse the actions, instead of just
doing textual transformations on them. So, this commit replaces
compare-odp-actions.pl by "ovs-dpctl normalize-actions", which is
sufficiently general for the intended purpose.
The new ovs-dpctl functionality can be easily extended to handle
differences in fields other than VLAN, but only VLAN is needed so
far.
This will be needed in an upcoming commit that in some cases
introduces redundant "set vlan" actions into the ODP actions, which
compare-odp-actions.pl doesn't tolerate.
---
tests/automake.mk | 1 -
tests/compare-odp-actions.pl | 66 --------------
tests/ofproto-dpif.at | 6 +-
utilities/ovs-dpctl.c | 206 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 210 insertions(+), 69 deletions(-)
delete mode 100644 tests/compare-odp-actions.pl
diff --git a/tests/automake.mk b/tests/automake.mk
index f11e291..09de521 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -58,7 +58,6 @@ TESTSUITE_AT = \
tests/vlog.at
TESTSUITE = $(srcdir)/tests/testsuite
DISTCLEANFILES += tests/atconfig tests/atlocal
-EXTRA_DIST += tests/compare-odp-actions.pl
AUTOTEST_PATH = utilities:vswitchd:ovsdb:tests
diff --git a/tests/compare-odp-actions.pl b/tests/compare-odp-actions.pl
deleted file mode 100644
index b18a593..0000000
--- a/tests/compare-odp-actions.pl
+++ /dev/null
@@ -1,66 +0,0 @@
-# -*- perl -*-
-
-use strict;
-use warnings;
-
-if (@ARGV < 2) {
- print <<EOF;
-$0: to check ODP sets of actions for equivalence
-usage: $0 ACTIONS1 ACTIONS2 [NAME=NUMBER]...
-where ACTIONS1 and ACTIONS2 are sets of ODP actions as output by, e.g.
- "ovs-dpctl dump-flows" and each NAME=NUMBER pair identifies an ODP
- port's name-to-number mapping.
-
-Exits with status 0 if ACTIONS1 and ACTIONS2 are equivalent, with
-status 1 if they differ.
-EOF
- exit 1;
-}
-
-# Construct mappings between port numbers and names.
-our (%name_to_number);
-our (%number_to_name);
-for (@ARGV[2..$#ARGV]) {
- my ($name, $number) = /([^=]+)=([0-9]+)/
- or die "$_: bad syntax (use --help for help)\n";
- $number_to_name{$number} = $name;
- $name_to_number{$name} = $number;
-}
-
-my $n1 = normalize_odp_actions($ARGV[0]);
-my $n2 = normalize_odp_actions($ARGV[1]);
-print "Normalized action set 1: $n1\n";
-print "Normalized action set 2: $n2\n";
-exit($n1 ne $n2);
-
-sub normalize_odp_actions {
- my ($actions) = @_;
-
- # Transliterate all commas inside parentheses into semicolons.
- undef while $actions =~ s/(\([^),]*),([^)]*\))/$1;$2/g;
-
- # Split on commas.
- my (@actions) = split(',', $actions);
-
- # Map port numbers into port names.
- foreach my $s (@actions) {
- $s = $number_to_name{$s} if exists($number_to_name{$s});
- }
-
- # Sort sequential groups of port names into alphabetical order.
- for (my $i = 0; $i <= $#actions; ) {
- my $j = $i + 1;
- if (exists($name_to_number{$actions[$i]})) {
- for (; $j <= $#actions; $j++) {
- last if !exists($name_to_number{$actions[$j]});
- }
- }
- @actions[$i..($j - 1)] = sort(@actions[$i..($j - 1)]);
- $i = $j;
- }
-
- # Now compose a string again and transliterate semicolons back to commas.
- $actions = join(',', @actions);
- $actions =~ tr/;/,/;
- return $actions;
-}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ec5c238..312ec1f 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -220,8 +220,10 @@ do
AT_CHECK([ovs-appctl ofproto/trace br0 "$flow"], [0], [stdout])
actual=`tail -1 stdout | sed 's/Datapath actions: //'`
- AT_CHECK([echo "in_port=$in_port vlan=$vlan"
- $PERL $srcdir/compare-odp-actions.pl "$expected" "$actual"
br0=$br0 p1=$p1 p2=$p2 p3=$p3 p4=$p4 p5=$p5 p6=$p6 p7=$p7 p8=$p8], [0],
[ignore])
+ echo "in_port=$in_port vlan=$vlan"
+ AT_CHECK([ovs-dpctl normalize-actions "$flow" "$expected" br0=$br0 p1=$p1
p2=$p2 p3=$p3 p4=$p4 p5=$p5 p6=$p6 p7=$p7 p8=$p8], [0], [stdout])
+ mv stdout expout
+ AT_CHECK([ovs-dpctl normalize-actions "$flow" "$actual" br0=$br0 p1=$p1
p2=$p2 p3=$p3 p4=$p4 p5=$p5 p6=$p6 p7=$p7 p8=$p8], [0], [expout])
done
OVS_VSWITCHD_STOP
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 69a66b6..30e06b4 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -16,6 +16,7 @@
#include <config.h>
#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
#include <getopt.h>
#include <inttypes.h>
@@ -35,9 +36,12 @@
#include "dirs.h"
#include "dpif.h"
#include "dynamic-string.h"
+#include "flow.h"
#include "netdev.h"
+#include "netlink.h"
#include "odp-util.h"
#include "ofpbuf.h"
+#include "packets.h"
#include "shash.h"
#include "sset.h"
#include "timeval.h"
@@ -49,6 +53,12 @@ VLOG_DEFINE_THIS_MODULE(dpctl);
/* -s, --statistics: Print port statistics? */
static bool print_statistics;
+/* -m, --more: Output verbosity.
+ *
+ * So far only undocumented commands honor this option, so we don't document
+ * the option itself. */
+static int verbosity;
+
static const struct command all_commands[];
static void usage(void) NO_RETURN;
@@ -73,6 +83,7 @@ parse_options(int argc, char *argv[])
};
static struct option long_options[] = {
{"statistics", no_argument, NULL, 's'},
+ {"more", no_argument, NULL, 'm'},
{"timeout", required_argument, NULL, 't'},
{"help", no_argument, NULL, 'h'},
{"version", no_argument, NULL, 'V'},
@@ -95,6 +106,10 @@ parse_options(int argc, char *argv[])
print_statistics = true;
break;
+ case 'm':
+ verbosity++;
+ break;
+
case 't':
timeout = strtoul(optarg, NULL, 10);
if (timeout <= 0) {
@@ -718,6 +733,196 @@ do_parse_actions(int argc, char *argv[])
}
}
+struct actions_for_flow {
+ struct hmap_node hmap_node;
+ struct flow flow;
+ struct ofpbuf actions;
+};
+
+static struct actions_for_flow *
+get_actions_for_flow(struct hmap *actions_per_flow, const struct flow *flow)
+{
+ uint32_t hash = flow_hash(flow, 0);
+ struct actions_for_flow *af;
+
+ HMAP_FOR_EACH_WITH_HASH (af, hmap_node, hash, actions_per_flow) {
+ if (flow_equal(&af->flow, flow)) {
+ return af;
+ }
+ }
+
+ af = xmalloc(sizeof *af);
+ af->flow = *flow;
+ ofpbuf_init(&af->actions, 0);
+ hmap_insert(actions_per_flow, &af->hmap_node, hash);
+ return af;
+}
+
+static int
+compare_actions_for_flow(const void *a_, const void *b_)
+{
+ struct actions_for_flow *const *a = a_;
+ struct actions_for_flow *const *b = b_;
+
+ return flow_compare_3way(&(*a)->flow, &(*b)->flow);
+}
+
+static int
+compare_output_actions(const void *a_, const void *b_)
+{
+ const struct nlattr *a = a_;
+ const struct nlattr *b = b_;
+ uint32_t a_port = nl_attr_get_u32(a);
+ uint32_t b_port = nl_attr_get_u32(b);
+
+ return a_port < b_port ? -1 : a_port > b_port;
+}
+
+static void
+sort_output_actions__(struct nlattr *first, struct nlattr *end)
+{
+ size_t bytes = (uint8_t *) end - (uint8_t *) first;
+ size_t n = bytes / NL_A_U32_SIZE;
+
+ assert(bytes % NL_A_U32_SIZE == 0);
+ qsort(first, n, NL_A_U32_SIZE, compare_output_actions);
+}
+
+static void
+sort_output_actions(struct nlattr *actions, size_t length)
+{
+ struct nlattr *first_output = NULL;
+ struct nlattr *a;
+ int left;
+
+ NL_ATTR_FOR_EACH (a, left, actions, length) {
+ if (nl_attr_type(a) == OVS_ACTION_ATTR_OUTPUT) {
+ if (!first_output) {
+ first_output = a;
+ }
+ } else {
+ if (first_output) {
+ sort_output_actions__(first_output, a);
+ first_output = NULL;
+ }
+ }
+ }
+ if (first_output) {
+ uint8_t *end = (uint8_t *) actions + length;
+ sort_output_actions__(first_output, (struct nlattr *) end);
+ }
+}
+
+static void
+do_normalize_actions(int argc, char *argv[])
+{
+ struct shash port_names;
+ struct ofpbuf keybuf;
+ struct flow flow;
+ struct ofpbuf odp_actions;
+ struct hmap actions_per_flow;
+ struct actions_for_flow **afs;
+ struct actions_for_flow *af;
+ struct nlattr *a;
+ size_t n_afs;
+ struct ds s;
+ int left;
+ int i;
+
+ ds_init(&s);
+
+ shash_init(&port_names);
+ for (i = 3; i < argc; i++) {
+ char name[16];
+ int number;
+ int n = -1;
+
+ if (sscanf(argv[i], "%15[^=]=%d%n", name, &number, &n) > 0 && n > 0) {
+ shash_add(&port_names, name, (void *) number);
+ } else {
+ ovs_fatal(0, "%s: expected NAME=NUMBER", argv[i]);
+ }
+ }
+
+ /* Parse flow key. */
+ ofpbuf_init(&keybuf, 0);
+ run(odp_flow_key_from_string(argv[1], &port_names, &keybuf),
+ "odp_flow_key_from_string");
+
+ ds_clear(&s);
+ odp_flow_key_format(keybuf.data, keybuf.size, &s);
+ printf("input flow: %s\n", ds_cstr(&s));
+
+ run(odp_flow_key_to_flow(keybuf.data, keybuf.size, &flow),
+ "odp_flow_key_to_flow");
+ ofpbuf_uninit(&keybuf);
+
+ /* Parse actions. */
+ ofpbuf_init(&odp_actions, 0);
+ run(odp_actions_from_string(argv[2], &port_names, &odp_actions),
+ "odp_actions_from_string");
+
+ if (verbosity) {
+ ds_clear(&s);
+ format_odp_actions(&s, odp_actions.data, odp_actions.size);
+ printf("input actions: %s\n", ds_cstr(&s));
+ }
+
+ hmap_init(&actions_per_flow);
+ NL_ATTR_FOR_EACH (a, left, odp_actions.data, odp_actions.size) {
+ if (nl_attr_type(a) == OVS_ACTION_ATTR_POP
+ && nl_attr_get_u16(a) == OVS_KEY_ATTR_8021Q) {
+ flow.vlan_tci = htons(0);
+ continue;
+ }
+
+ if (nl_attr_type(a) == OVS_ACTION_ATTR_PUSH) {
+ const struct nlattr *nested = nl_attr_get(a);
+
+ if (nl_attr_type(nested) == OVS_KEY_ATTR_8021Q) {
+ const struct ovs_key_8021q *q_key;
+
+ q_key = nl_attr_get_unspec(nested, sizeof(*q_key));
+ flow.vlan_tci = q_key->q_tci | htons(VLAN_CFI);
+ continue;
+ }
+ }
+
+ af = get_actions_for_flow(&actions_per_flow, &flow);
+ nl_msg_put_unspec(&af->actions, nl_attr_type(a),
+ nl_attr_get(a), nl_attr_get_size(a));
+ }
+
+ n_afs = hmap_count(&actions_per_flow);
+ afs = xmalloc(n_afs * sizeof *afs);
+ i = 0;
+ HMAP_FOR_EACH (af, hmap_node, &actions_per_flow) {
+ afs[i++] = af;
+ }
+ assert(i == n_afs);
+
+ qsort(afs, n_afs, sizeof *afs, compare_actions_for_flow);
+
+ for (i = 0; i < n_afs; i++) {
+ const struct actions_for_flow *af = afs[i];
+
+ sort_output_actions(af->actions.data, af->actions.size);
+
+ if (af->flow.vlan_tci != htons(0)) {
+ printf("vlan(vid=%"PRIu16",pcp=%d): ",
+ vlan_tci_to_vid(af->flow.vlan_tci),
+ vlan_tci_to_pcp(af->flow.vlan_tci));
+ } else {
+ printf("no vlan: ");
+ }
+
+ ds_clear(&s);
+ format_odp_actions(&s, af->actions.data, af->actions.size);
+ puts(ds_cstr(&s));
+ }
+ ds_destroy(&s);
+}
+
static const struct command all_commands[] = {
{ "add-dp", 1, INT_MAX, do_add_dp },
{ "del-dp", 1, 1, do_del_dp },
@@ -732,6 +937,7 @@ static const struct command all_commands[] = {
/* Undocumented commands for testing. */
{ "parse-actions", 1, INT_MAX, do_parse_actions },
+ { "normalize-actions", 2, INT_MAX, do_normalize_actions },
{ NULL, 0, 0, NULL },
};
--
1.7.2.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
