ofproto: Log changes made by flow normalization.

Open vSwitch has always "normalized" flows, that is, zeroed out fields that
are wildcarded or that otherwise cannot affect whether a packet actually
matches the flow.  But until now it has done so silently, which prevents
the authors of controllers from learning what is happening and makes it
less likely that they will update code on their end.  This commit makes
OVS log when normalization changes a flow.

Suggested by partner.

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 94682eb..16a3eee 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -754,3 +754,40 @@ normalize_match(struct ofp_match *m)
     m->wildcards = htonl(wc);
 }
 
+/* Returns a string that describes 'match' in a very literal way, without
+ * interpreting its contents except in a very basic fashion.  The returned
+ * string is intended to be fixed-length, so that it is easy to see differences
+ * between two such strings if one is put above another.  This is useful for
+ * describing changes made by normalize_match().
+ *
+ * The caller must free the returned string (with free()). */
+char *
+ofp_match_to_literal_string(const struct ofp_match *match)
+{
+    return xasprintf("wildcards=%#10"PRIx32" "
+                     " in_port=%5"PRId16" "
+                     " dl_src="ETH_ADDR_FMT" "
+                     " dl_dst="ETH_ADDR_FMT" "
+                     " dl_vlan=%5"PRId16" "
+                     " dl_vlan_pcp=%3"PRId8" "
+                     " dl_type=%#6"PRIx16" "
+                     " nw_tos=%#4"PRIx8" "
+                     " nw_proto=%#4"PRIx16" "
+                     " nw_src=%#10"PRIx32" "
+                     " nw_dst=%#10"PRIx32" "
+                     " tp_src=%5"PRId16" "
+                     " tp_dst=%5"PRId16,
+                     ntohl(match->wildcards),
+                     ntohs(match->in_port),
+                     ETH_ADDR_ARGS(match->dl_src),
+                     ETH_ADDR_ARGS(match->dl_dst),
+                     ntohs(match->dl_vlan),
+                     match->dl_vlan_pcp,
+                     ntohs(match->dl_type),
+                     match->nw_tos,
+                     match->nw_proto,
+                     ntohl(match->nw_src),
+                     ntohl(match->nw_dst),
+                     ntohs(match->tp_src),
+                     ntohs(match->tp_dst));
+}

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 2c6b2f9..b4af179 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -81,6 +81,7 @@ int validate_actions(const union ofp_action *, size_t n_actions,
 bool action_outputs_to_port(const union ofp_action *, uint16_t port);
 
 void normalize_match(struct ofp_match *);
+char *ofp_match_to_literal_string(const struct ofp_match *match);
 
 static inline int
 ofp_mkerr(uint16_t type, uint16_t code)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 481d50b..eb62fe8 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3639,6 +3639,7 @@ static int
 handle_flow_mod(struct ofproto *p, struct ofconn *ofconn,
                 struct ofp_flow_mod *ofm)
 {
+    struct ofp_match orig_match;
     size_t n_actions;
     int error;
 
@@ -3660,7 +3661,25 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn,
         return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL);
     }
 
+    /* Normalize ofp->match.  If normalization actually changes anything, then
+     * log the differences. */
+    ofm->match.pad1[0] = ofm->match.pad2[0] = 0;
+    orig_match = ofm->match;
     normalize_match(&ofm->match);
+    if (memcmp(&ofm->match, &orig_match, sizeof orig_match)) {
+        static struct vlog_rate_limit normal_rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        if (!VLOG_DROP_INFO(&normal_rl)) {
+            char *old = ofp_match_to_literal_string(&orig_match);
+            char *new = ofp_match_to_literal_string(&ofm->match);
+            VLOG_INFO("%s: normalization changed ofp_match, details:",
+                      rconn_get_name(ofconn->rconn));
+            VLOG_INFO(" pre: %s", old);
+            VLOG_INFO("post: %s", new);
+            free(old);
+            free(new);
+        }
+    }
+
     if (!ofm->match.wildcards) {
         ofm->priority = htons(UINT16_MAX);
     }