From 01b389b1109fc667c37c1ace3bded423d9f38637 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 10 May 2011 08:59:21 -0700 Subject: [PATCH] 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 Bug #5029. --- lib/ofp-parse.c | 7 +++++++ tests/ofproto-macros.at | 1 + tests/ovs-ofctl.at | 12 ++++++++++++ 3 files changed, 20 insertions(+) diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 29137ac9d..52fdbf2f6 100644 --- 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 392755068..dc863e73f 100644 --- 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 997cc3062..6d636c1de 100644 --- 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 -- 2.20.1