ofproto: Move 'max_ports' from ofproto-dpif.c to ofproto.c.

This allows port numbers in actions and elsewhere in OpenFlow messages to
be checked at a higher level.

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

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 78cb186a6a884961725ef41d7cabe9c538a1aea1..beb12f0ac2daa40bd240a53db692a705987973c9 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -598,7 +598,6 @@ struct ofproto_dpif {
     struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */
     struct ofproto up;
     struct dpif *dpif;
-    int max_ports;
 
     /* Special OpenFlow rules. */
     struct rule_dpif *miss_rule; /* Sends flow table misses to controller. */
@@ -744,6 +743,7 @@ construct(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     const char *name = ofproto->up.name;
+    int max_ports;
     int error;
     int i;
 
@@ -753,7 +753,9 @@ construct(struct ofproto *ofproto_)
         return error;
     }
 
-    ofproto->max_ports = dpif_get_max_ports(ofproto->dpif);
+    max_ports = dpif_get_max_ports(ofproto->dpif);
+    ofproto_init_max_ports(ofproto_, MIN(max_ports, OFPP_MAX));
+
     ofproto->n_matches = 0;
 
     dpif_flow_flush(ofproto->dpif);
@@ -4639,7 +4641,7 @@ rule_construct(struct rule *rule_)
     enum ofperr error;
 
     error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
-                          &rule->up.cr.flow, ofproto->max_ports);
+                          &rule->up.cr.flow, ofproto->up.max_ports);
     if (error) {
         return error;
     }
@@ -4749,7 +4751,7 @@ rule_modify_actions(struct rule *rule_)
     enum ofperr error;
 
     error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
-                          &rule->up.cr.flow, ofproto->max_ports);
+                          &rule->up.cr.flow, ofproto->up.max_ports);
     if (error) {
         ofoperation_complete(rule->up.pending, error);
         return;
@@ -6443,11 +6445,7 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     enum ofperr error;
 
-    if (flow->in_port >= ofproto->max_ports && flow->in_port < OFPP_MAX) {
-        return OFPERR_NXBRC_BAD_IN_PORT;
-    }
-
-    error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->max_ports);
+    error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->up.max_ports);
     if (!error) {
         struct odputil_keybuf keybuf;
         struct dpif_flow_stats stats;

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 15dc34721810f3a5e75c51a6f583a7670e79b231..72efb1410bcbbab5d62e5b47f273b322237ecf31 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -61,6 +61,7 @@ struct ofproto {
     /* Datapath. */
     struct hmap ports;          /* Contains "struct ofport"s. */
     struct shash port_by_name;
+    uint16_t max_ports;         /* Max possible OpenFlow port num, plus one. */
 
     /* Flow tables. */
     struct oftable *tables;
@@ -93,6 +94,7 @@ struct ofproto {
 };
 
 void ofproto_init_tables(struct ofproto *, int n_tables);
+void ofproto_init_max_ports(struct ofproto *, uint16_t max_ports);
 
 struct ofproto *ofproto_lookup(const char *name);
 struct ofport *ofproto_get_port(const struct ofproto *, uint16_t ofp_port);
@@ -365,6 +367,11 @@ struct ofproto_class {
      * ->construct() should delete flows from the underlying datapath, if
      * necessary, rather than populating the tables.
      *
+     * If the ofproto knows the maximum port number that the datapath can have,
+     * then it can call ofproto_init_max_ports().  If it does so, then the
+     * client will ensure that the actions it allows to be used through
+     * OpenFlow do not refer to ports above that maximum number.
+     *
      * Only one ofproto instance needs to be supported for any given datapath.
      * If a datapath is already open as part of one "ofproto", then another
      * attempt to "construct" the same datapath as part of another ofproto is
@@ -946,17 +953,18 @@ struct ofproto_class {
      *
      * flow->in_port comes from the OpenFlow OFPT_PACKET_OUT message.  The
      * implementation should reject invalid flow->in_port values by returning
-     * OFPERR_NXBRC_BAD_IN_PORT.  For consistency, the implementation should
-     * consider valid for flow->in_port any value that could possibly be seen
-     * in a packet that it passes to connmgr_send_packet_in().  Ideally, even
-     * an implementation that never generates packet-ins (e.g. due to hardware
-     * limitations) should still allow flow->in_port values for every possible
-     * physical port and OFPP_LOCAL.  The only virtual ports (those above
-     * OFPP_MAX) that the caller will ever pass in as flow->in_port, other than
-     * OFPP_LOCAL, are OFPP_NONE and OFPP_CONTROLLER.  The implementation
-     * should allow both of these, treating each of them as packets generated
-     * by the controller as opposed to packets originating from some switch
-     * port.
+     * OFPERR_NXBRC_BAD_IN_PORT.  (If the implementation called
+     * ofproto_init_max_ports(), then the client will reject these ports
+     * itself.)  For consistency, the implementation should consider valid for
+     * flow->in_port any value that could possibly be seen in a packet that it
+     * passes to connmgr_send_packet_in().  Ideally, even an implementation
+     * that never generates packet-ins (e.g. due to hardware limitations)
+     * should still allow flow->in_port values for every possible physical port
+     * and OFPP_LOCAL.  The only virtual ports (those above OFPP_MAX) that the
+     * caller will ever pass in as flow->in_port, other than OFPP_LOCAL, are
+     * OFPP_NONE and OFPP_CONTROLLER.  The implementation should allow both of
+     * these, treating each of them as packets generated by the controller as
+     * opposed to packets originating from some switch port.
      *
      * (Ordinarily the only effect of flow->in_port is on output actions that
      * involve the input port, such as actions that output to OFPP_IN_PORT,

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5c9ab9d31540779d617d6c8e7a55028dd325347f..ae24fd681723484303afd96895351314574db5fb 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -390,6 +390,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->frag_handling = OFPC_FRAG_NORMAL;
     hmap_init(&ofproto->ports);
     shash_init(&ofproto->port_by_name);
+    ofproto->max_ports = OFPP_MAX;
     ofproto->tables = NULL;
     ofproto->n_tables = 0;
     ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
@@ -422,6 +423,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     return 0;
 }
 
+/* Must be called (only) by an ofproto implementation in its constructor
+ * function.  See the large comment on 'construct' in struct ofproto_class for
+ * details. */
 void
 ofproto_init_tables(struct ofproto *ofproto, int n_tables)
 {
@@ -437,6 +441,24 @@ ofproto_init_tables(struct ofproto *ofproto, int n_tables)
     }
 }
 
+/* To be optionally called (only) by an ofproto implementation in its
+ * constructor function.  See the large comment on 'construct' in struct
+ * ofproto_class for details.
+ *
+ * Sets the maximum number of ports to 'max_ports'.  The ofproto generic layer
+ * will then ensure that actions passed into the ofproto implementation will
+ * not refer to OpenFlow ports numbered 'max_ports' or higher.  If this
+ * function is not called, there will be no such restriction.
+ *
+ * Reserved ports numbered OFPP_MAX and higher are special and not subject to
+ * the 'max_ports' restriction. */
+void
+ofproto_init_max_ports(struct ofproto *ofproto, uint16_t max_ports)
+{
+    assert(max_ports <= OFPP_MAX);
+    ofproto->max_ports = max_ports;
+}
+
 uint64_t
 ofproto_get_datapath_id(const struct ofproto *ofproto)
 {
@@ -2103,6 +2125,10 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
     if (error) {
         goto exit_free_ofpacts;
     }
+    if (po.in_port >= p->max_ports && po.in_port < OFPP_MAX) {
+        error = OFPERR_NXBRC_BAD_IN_PORT;
+        goto exit_free_ofpacts;
+    }
 
     /* Get payload. */
     if (po.buffer_id != UINT32_MAX) {