ofproto: Issue OpenFlow error for bad table IDs.

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index db8134cc8cf7146c0f98fe3f68a5c2e36659461d..1441ee583f7725a2996c4adea0e550d7da375595 100644 (file)
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -99,7 +99,14 @@ enum nx_bad_request_code {
     NXBRC_NXM_BAD_PREREQ = 0x104,
 
     /* A given nxm_type was specified more than once. */
-    NXBRC_NXM_DUP_TYPE = 0x105
+    NXBRC_NXM_DUP_TYPE = 0x105,
+
+/* Other errors. */
+
+    /* A request specified a nonexistent table ID.  (But NXFMFC_BAD_TABLE_ID is
+     * used instead, when it is appropriate, because that is such a special
+     * case.) */
+    NXBRC_BAD_TABLE_ID = 0x200,
 };
 
 /* Additional "code" values for OFPET_FLOW_MOD_FAILED. */

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 2d41704fff85f6751b2eff5ecac5729a42a5ef34..869b16cd64fb2369cce510218e9c365436fdda25 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1925,6 +1925,17 @@ calc_flow_duration__(long long int start, uint32_t *sec, uint32_t *nsec)
     *nsec = (msecs % 1000) * (1000 * 1000);
 }
 
+/* Checks whether 'table_id' is 0xff or a valid table ID in 'ofproto'.  Returns
+ * 0 if 'table_id' is OK, otherwise an OpenFlow error code.  */
+static int
+check_table_id(const struct ofproto *ofproto, uint8_t table_id)
+{
+    return (table_id == 0xff || table_id < ofproto->n_tables
+            ? 0
+            : ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_TABLE_ID));
+
+}
+
 static struct classifier *
 first_matching_table(struct ofproto *ofproto, uint8_t table_id)
 {
@@ -1933,11 +1944,6 @@ first_matching_table(struct ofproto *ofproto, uint8_t table_id)
     } else if (table_id < ofproto->n_tables) {
         return &ofproto->tables[table_id];
     } else {
-        /* It would probably be better to reply with an error but there doesn't
-         * seem to be any appropriate value, so that might just be
-         * confusing. */
-        VLOG_WARN_RL(&rl, "controller asked for invalid table %"PRIu8,
-                     table_id);
         return NULL;
     }
 }
@@ -1960,8 +1966,9 @@ next_matching_table(struct ofproto *ofproto,
  *   - If TABLE_ID is the number of a table in OFPROTO, then the loop iterates
  *     only once, for that table.
  *
- *   - Otherwise, TABLE_ID isn't valid for OFPROTO, so ofproto logs a warning
- *     and does not enter the loop at all.
+ *   - Otherwise, TABLE_ID isn't valid for OFPROTO, so the loop won't be
+ *     entered at all.  (Perhaps you should have validated TABLE_ID with
+ *     check_table_id().)
  *
  * All parameters are evaluated multiple times.
  */
@@ -1987,6 +1994,12 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t table_id,
                     struct list *rules)
 {
     struct classifier *cls;
+    int error;
+
+    error = check_table_id(ofproto, table_id);
+    if (error) {
+        return error;
+    }
 
     list_init(rules);
     FOR_EACH_MATCHING_TABLE (cls, table_id, ofproto) {
@@ -2023,6 +2036,12 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t table_id,
                      struct list *rules)
 {
     struct classifier *cls;
+    int error;
+
+    error = check_table_id(ofproto, table_id);
+    if (error) {
+        return error;
+    }
 
     list_init(rules);
     FOR_EACH_MATCHING_TABLE (cls, table_id, ofproto) {
@@ -2350,6 +2369,11 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     struct rule *rule;
     int error;
 
+    error = check_table_id(ofproto, fm->table_id);
+    if (error) {
+        return error;
+    }
+
     /* Pick table. */
     if (fm->table_id == 0xff) {
         uint8_t table_id;