connmgr: Fix packet-in reason for OpenFlow1.3 table-miss flow entries.

As per spec, make packet-in reason for OpenFlow1.3 table-miss flow
entries no_match rather than action.

Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>

9 files changed, 55 insertions, 7 deletions

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index 22be72cd..bd6ced6b 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -130,9 +130,6 @@ didn't compare the specs carefully yet.)
       - Change the default table-miss action (in the absense of table-miss
         entry) from packet_in to drop for OF1.3+.  Decide what to do if
         a switch is configured to support multiple OF versions.
-      - Distinguish table-miss flow entry and make its packet_in reason
-        OFPR_NO_MATCH.  (OFPR_TABLE_MISS for OF1.4)
-        Also, make it use the appropriate pin scheduler.
       [required for OF1.3+]
 
     * IPv6 extension header handling support.  Fully implementing this
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index f5bef59d..d42ab4f9 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1435,7 +1435,8 @@ ofconn_send(const struct ofconn *ofconn, struct ofpbuf *msg,
 
 /* Sending asynchronous messages. */
 
-static void schedule_packet_in(struct ofconn *, struct ofproto_packet_in);
+static void schedule_packet_in(struct ofconn *, struct ofproto_packet_in,
+                               enum ofp_packet_in_reason wire_reason);
 
 /* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate
  * controllers managed by 'mgr'. */
@@ -1482,6 +1483,25 @@ connmgr_send_flow_removed(struct connmgr *mgr,
     }
 }
 
+/* Normally a send-to-controller action uses reason OFPR_ACTION.  However, in
+ * OpenFlow 1.3 and later, packet_ins generated by a send-to-controller action
+ * in a "table-miss" flow (one with priority 0 and completely wildcarded) are
+ * sent as OFPR_NO_MATCH.  This function returns the reason that should
+ * actually be sent on 'ofconn' for 'pin'. */
+static enum ofp_packet_in_reason
+wire_reason(struct ofconn *ofconn, const struct ofproto_packet_in *pin)
+{
+    if (pin->generated_by_table_miss && pin->up.reason == OFPR_ACTION) {
+        enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+        enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
+
+        if (version >= OFP13_VERSION) {
+            return OFPR_NO_MATCH;
+        }
+    }
+    return pin->up.reason;
+}
+
 /* Given 'pin', sends an OFPT_PACKET_IN message to each OpenFlow controller as
  * necessary according to their individual configurations.
  *
@@ -1493,9 +1513,11 @@ connmgr_send_packet_in(struct connmgr *mgr,
     struct ofconn *ofconn;
 
     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
-        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason)
+        enum ofp_packet_in_reason reason = wire_reason(ofconn, pin);
+
+        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason)
             && ofconn->controller_id == pin->controller_id) {
-            schedule_packet_in(ofconn, *pin);
+            schedule_packet_in(ofconn, *pin, reason);
         }
     }
 }
@@ -1513,13 +1535,15 @@ do_send_packet_in(struct ofpbuf *ofp_packet_in, void *ofconn_)
 /* Takes 'pin', composes an OpenFlow packet-in message from it, and passes it
  * to 'ofconn''s packet scheduler for sending. */
 static void
-schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin)
+schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin,
+                   enum ofp_packet_in_reason wire_reason)
 {
     struct connmgr *mgr = ofconn->connmgr;
     uint16_t controller_max_len;
 
     pin.up.total_len = pin.up.packet_len;
 
+    pin.up.reason = wire_reason;
     if (pin.up.reason == OFPR_ACTION) {
         controller_max_len = pin.send_len;  /* max_len */
     } else {
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index e9ffbbc2..10caaf4e 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -68,6 +68,11 @@ struct ofproto_packet_in {
     struct list list_node;      /* For queuing. */
     uint16_t controller_id;     /* Controller ID to send to. */
     int send_len;               /* Length that the action requested sending. */
+
+    /* True if the packet_in was generated directly by a table-miss flow, that
+     * is, a flow with priority 0 that wildcards all fields.  (Our
+     * interpretation of "directly" is "not via groups".) */
+    bool generated_by_table_miss;
 };
 
 /* Basics. */
diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
index b0caf8d6..bae9dcaf 100644
--- a/ofproto/fail-open.c
+++ b/ofproto/fail-open.c
@@ -130,6 +130,7 @@ send_bogus_packet_ins(struct fail_open *fo)
     pin.up.reason = OFPR_NO_MATCH;
     pin.up.fmd.in_port = OFPP_LOCAL;
     pin.send_len = b.size;
+    pin.generated_by_table_miss = false;
     connmgr_send_packet_in(fo->connmgr, &pin);
 
     ofpbuf_uninit(&b);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 491f11e5..24a57296 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -849,6 +849,7 @@ handle_upcalls(struct udpif *udpif, struct list *upcalls)
             pin->up.cookie = OVS_BE64_MAX;
             flow_get_metadata(&miss->flow, &pin->up.fmd);
             pin->send_len = 0; /* Not used for flow table misses. */
+            pin->generated_by_table_miss = false;
             ofproto_dpif_send_packet_in(miss->ofproto, pin);
         }
     }
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ed9ca7cf..1eb09532 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1856,6 +1856,8 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 
     pin->controller_id = controller_id;
     pin->send_len = len;
+    pin->generated_by_table_miss = (ctx->rule
+                                    && rule_dpif_is_table_miss(ctx->rule));
     ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin);
     ofpbuf_delete(packet);
 }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 67c21f54..be7f807d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4308,6 +4308,11 @@ rule_dpif_is_fail_open(const struct rule_dpif *rule)
 {
     return is_fail_open_rule(&rule->up);
 }
+
+bool
+rule_dpif_is_table_miss(const struct rule_dpif *rule)
+{
+    return rule_is_table_miss(&rule->up);
 }
 
 ovs_be64
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index ab5dfbbd..c93d37c2 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -75,6 +75,7 @@ void rule_dpif_credit_stats(struct rule_dpif *rule ,
                             const struct dpif_flow_stats *);
 
 bool rule_dpif_is_fail_open(const struct rule_dpif *);
+bool rule_dpif_is_table_miss(const struct rule_dpif *);
 
 struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index de566e3b..07bb266f 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -398,6 +398,18 @@ struct rule_actions *rule_get_actions(const struct rule *rule)
 struct rule_actions *rule_get_actions__(const struct rule *rule)
     OVS_REQUIRES(rule->mutex);
 
+/* Returns true if 'rule' is an OpenFlow 1.3 "table-miss" rule, false
+ * otherwise.
+ *
+ * ("Table-miss" rules are special because a packet_in generated through one
+ * uses OFPR_NO_MATCH as its reason, whereas packet_ins generated by any other
+ * rule use OFPR_ACTION.) */
+static inline bool
+rule_is_table_miss(const struct rule *rule)
+{
+    return rule->cr.priority == 0 && cls_rule_is_catchall(&rule->cr);
+}
+
 /* A set of actions within a "struct rule".
  *
  *