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 94682eb479433e0c058ad177bf353ca7d17908e1..16a3eeeeb20ecee9b5136f4031fc047b0e9caae2 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 2c6b2f91928ec1e09a3728ddf5236910ae562b51..b4af179d6d68a6f5e0c948347afa3529c81934e3 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 481d50b845768941c0611bd552d6c2ba669f3151..eb62fe8c0bc34064976ddec06cb9d63f83768fd7 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);
     }