learning-switch: Don't use exact-match on every field by default.

OVS has all kinds of odd fields, e.g. registers, and it doesn't make sense
to try to match on all of them.  This commit changes learning-switch to
only try to match on the fields defined by OpenFlow 1.0.  That's still not
minimal, but it's more reasonable.

This commit should not have an immediately visible effect since
ovs-controller always sends OF1.0 format flows to the switch, and OF1.0
format flows don't have these extra fields.  But in the future when we
add support for new protocols and flow formats to ovs-controller, it
will make a difference.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index f52d3c9d2d69b074c5141609f2e9004344b39ab6..e4287c4e31f605a690bf3ff0cc4865d2ba5433af 100644 (file)
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -112,6 +112,7 @@ struct lswitch *
 lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
 {
     struct lswitch *sw;
+    uint32_t ofpfw;
 
     sw = xzalloc(sizeof *sw);
     sw->rconn = rconn;
@@ -123,24 +124,26 @@ lswitch_create(struct rconn *rconn, const struct lswitch_config *cfg)
               : NULL);
     sw->action_normal = cfg->mode == LSW_NORMAL;
 
-    flow_wildcards_init_exact(&sw->wc);
-    if (cfg->wildcards) {
-        uint32_t ofpfw;
-
-        if (cfg->wildcards == UINT32_MAX) {
-            /* Try to wildcard as many fields as possible, but we cannot
-             * wildcard all fields.  We need in_port to detect moves.  We need
-             * Ethernet source and dest and VLAN VID to do L2 learning. */
-            ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP
-                     | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL
-                     | OFPFW10_NW_TOS | OFPFW10_NW_PROTO
-                     | OFPFW10_TP_SRC | OFPFW10_TP_DST);
-        } else {
-            ofpfw = cfg->wildcards;
-        }
+    switch (cfg->wildcards) {
+    case 0:
+        ofpfw = 0;
+        break;
 
-        ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc);
+    case UINT32_MAX:
+        /* Try to wildcard as many fields as possible, but we cannot
+         * wildcard all fields.  We need in_port to detect moves.  We need
+         * Ethernet source and dest and VLAN VID to do L2 learning. */
+        ofpfw = (OFPFW10_DL_TYPE | OFPFW10_DL_VLAN_PCP
+                 | OFPFW10_NW_SRC_ALL | OFPFW10_NW_DST_ALL
+                 | OFPFW10_NW_TOS | OFPFW10_NW_PROTO
+                 | OFPFW10_TP_SRC | OFPFW10_TP_DST);
+        break;
+
+    default:
+        ofpfw = cfg->wildcards;
+        break;
     }
+    ofputil_wildcard_from_ofpfw10(ofpfw, &sw->wc);
 
     sw->default_queue = cfg->default_queue;
     hmap_init(&sw->queue_numbers);

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1fe30b469582e00178545e3abdb88f204a12e2c5..030c8129ca141b75f8020f38853139655fbe86db 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3319,23 +3319,8 @@ ofputil_put_action(enum ofputil_action_code code, struct ofpbuf *buf)
     }
 #include "ofp-util.def"
 
-/* "Normalizes" the wildcards in 'rule'.  That means:
- *
- *    1. If the type of level N is known, then only the valid fields for that
- *       level may be specified.  For example, ARP does not have a TOS field,
- *       so nw_tos must be wildcarded if 'rule' specifies an ARP flow.
- *       Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and
- *       ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an
- *       IPv4 flow.
- *
- *    2. If the type of level N is not known (or not understood by Open
- *       vSwitch), then no fields at all for that level may be specified.  For
- *       example, Open vSwitch does not understand SCTP, an L4 protocol, so the
- *       L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies an
- *       SCTP flow.
- */
-void
-ofputil_normalize_rule(struct cls_rule *rule)
+static void
+ofputil_normalize_rule__(struct cls_rule *rule, bool may_log)
 {
     enum {
         MAY_NW_ADDR     = 1 << 0, /* nw_src, nw_dst */
@@ -3409,7 +3394,7 @@ ofputil_normalize_rule(struct cls_rule *rule)
 
     /* Log any changes. */
     if (!flow_wildcards_equal(&wc, &rule->wc)) {
-        bool log = !VLOG_DROP_INFO(&bad_ofmsg_rl);
+        bool log = may_log && !VLOG_DROP_INFO(&bad_ofmsg_rl);
         char *pre = log ? cls_rule_to_string(rule) : NULL;
 
         rule->wc = wc;
@@ -3426,6 +3411,39 @@ ofputil_normalize_rule(struct cls_rule *rule)
     }
 }
 
+/* "Normalizes" the wildcards in 'rule'.  That means:
+ *
+ *    1. If the type of level N is known, then only the valid fields for that
+ *       level may be specified.  For example, ARP does not have a TOS field,
+ *       so nw_tos must be wildcarded if 'rule' specifies an ARP flow.
+ *       Similarly, IPv4 does not have any IPv6 addresses, so ipv6_src and
+ *       ipv6_dst (and other fields) must be wildcarded if 'rule' specifies an
+ *       IPv4 flow.
+ *
+ *    2. If the type of level N is not known (or not understood by Open
+ *       vSwitch), then no fields at all for that level may be specified.  For
+ *       example, Open vSwitch does not understand SCTP, an L4 protocol, so the
+ *       L4 fields tp_src and tp_dst must be wildcarded if 'rule' specifies an
+ *       SCTP flow.
+ *
+ * If this function changes 'rule', it logs a rate-limited informational
+ * message. */
+void
+ofputil_normalize_rule(struct cls_rule *rule)
+{
+    ofputil_normalize_rule__(rule, true);
+}
+
+/* Same as ofputil_normalize_rule() without the logging.  Thus, this function
+ * is suitable for a program's internal use, whereas ofputil_normalize_rule()
+ * sense for use on flows received from elsewhere (so that a bug in the program
+ * that sent them can be reported and corrected). */
+void
+ofputil_normalize_rule_quiet(struct cls_rule *rule)
+{
+    ofputil_normalize_rule__(rule, false);
+}
+
 /* Parses a key or a key-value pair from '*stringp'.
  *
  * On success: Stores the key into '*keyp'.  Stores the value, if present, into

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 64f6c39c3d71351d1758f9aed2dd15c01c968edf..7b30e8786298a371817e3fff0ccc9b428fb766bb 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -114,6 +114,7 @@ void ofputil_cls_rule_from_ofp10_match(const struct ofp10_match *,
                                        unsigned int priority,
                                        struct cls_rule *);
 void ofputil_normalize_rule(struct cls_rule *);
+void ofputil_normalize_rule_quiet(struct cls_rule *);
 void ofputil_cls_rule_to_ofp10_match(const struct cls_rule *,
                                      struct ofp10_match *);