ovs-ofctl: Report attempts to add (remove, etc.) non-normalized flows.

Commit 0b3f27253 (ovs-ofctl: Warn about flows not in normal form) made
ovs-ofctl warn about non-normalized flows, that is, flows some of whose
specified fields will be ignored by the switch.  This was convenient for
users, who are understandably confused by flow normalization.  However,
later commit 8050b31d6 (ofp-parse: Refactor flow parsing) accidentally
deleted the warning.  This commit restores it and adds a test to ensure
that it doesn't get deleted again later.

Reported-by: Reid Price <reid@nicira.com>
Bug #5029.

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 29137ac9d34675b78941e8e64a4bd8f334357d39..52fdbf2f6e81f3a15bf72ed5184c5b918c6778a2 100644 (file)
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -845,6 +845,7 @@ parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
 {
     bool is_del = command == OFPFC_DELETE || command == OFPFC_DELETE_STRICT;
     enum nx_flow_format min_format, next_format;
+    struct cls_rule rule_copy;
     struct ofpbuf actions;
     struct ofpbuf *ofm;
     struct flow_mod fm;
@@ -861,6 +862,12 @@ parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
         *cur_format = next_format;
     }
 
+    /* Normalize a copy of the rule.  This ensures that non-normalized flows
+     * get logged but doesn't affect what gets sent to the switch, so that the
+     * switch can do whatever it likes with the flow. */
+    rule_copy = fm.cr;
+    ofputil_normalize_rule(&rule_copy, next_format);
+
     if (fm.table_id != 0xff && !*flow_mod_table_id) {
         struct ofpbuf *sff = ofputil_make_flow_mod_table_id(true);
         list_push_back(packets, &sff->list_node);

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 3927550684e2d8e6e1377e3975c27fd86199b2c4..dc863e73f90b4bf632eabf914ffd3390a7c7caa1 100644 (file)
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -1,5 +1,6 @@
 m4_define([STRIP_XIDS], [[sed 's/ (xid=0x[0-9a-fA-F]*)//']])
 m4_define([STRIP_DURATION], [[sed 's/\bduration=[0-9.]*s/duration=?s/']])
+m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m'])
 
 m4_define([OFPROTO_START],
   [OVS_RUNDIR=$PWD; export OVS_RUNDIR

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 997cc306298d00b6e119158aa6d69de2e5d0fece..6d636c1de2939209e7bf076272ac1b6a67c25003 100644 (file)
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -491,3 +491,15 @@ AT_CHECK([ovs-ofctl -F openflow10 dump-flows unix:br0.mgmt reg0=0xabcdef], [1],
 ])
 OFPROTO_STOP
 AT_CLEANUP
+
+dnl Check that add-flow reports non-normalized flows (feature #5029).
+AT_SETUP([ovs-ofctl add-flow reports non-normalized flows])
+OFPROTO_START
+AT_CHECK([ovs-ofctl TESTABLE_LOG add-flow br0 nw_src=1.2.3.4,actions=5],
+  [0], [], [dnl
+ofp_util|INFO|normalization changed ofp_match, details:
+ofp_util|INFO| pre: nw_src=1.2.3.4
+ofp_util|INFO|post: @&t@
+])
+OFPROTO_STOP
+AT_CLEANUP