From 3f09c339f7e5e3d74bb06600b7f05c8efccbf132 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Thu, 24 Jun 2010 14:20:45 -0700
Subject: [PATCH] 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.
---
 lib/ofp-util.c    | 37 +++++++++++++++++++++++++++++++++++++
 lib/ofp-util.h    |  1 +
 ofproto/ofproto.c | 19 +++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 94682eb4..16a3eeee 100644
--- 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 2c6b2f91..b4af179d 100644
--- 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 481d50b8..eb62fe8c 100644
--- 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);
     }
-- 
2.30.2