ofproto: Fix use of uninitialized local variable.
[cascardo/ovs.git] / ofproto / ofproto.c
index 3c4184f..c9bb877 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  * Copyright (c) 2010 Jean Tourrilhes - HP-Labs.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -379,7 +379,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
                 hash_string(ofproto->name, 0));
     ofproto->datapath_id = 0;
     ofproto_set_flow_eviction_threshold(ofproto,
-                                        OFPROTO_FLOW_EVICTON_THRESHOLD_DEFAULT);
+                                        OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT);
     ofproto->forward_bpdu = false;
     ofproto->fallback_dpid = pick_fallback_dpid();
     ofproto->mfr_desc = xstrdup(DEFAULT_MFR_DESC);
@@ -1970,7 +1970,7 @@ rule_execute(struct rule *rule, uint16_t in_port, struct ofpbuf *packet)
 
     assert(ofpbuf_headroom(packet) >= sizeof(struct ofp_packet_in));
 
-    flow_extract(packet, 0, 0, in_port, &flow);
+    flow_extract(packet, 0, 0, NULL, in_port, &flow);
     return rule->ofproto->ofproto_class->rule_execute(rule, &flow, packet);
 }
 
@@ -2147,7 +2147,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
     }
 
     /* Verify actions against packet, then send packet if successful. */
-    flow_extract(payload, 0, 0, po.in_port, &flow);
+    flow_extract(payload, 0, 0, NULL, po.in_port, &flow);
     error = ofpacts_check(po.ofpacts, po.ofpacts_len, &flow, p->max_ports);
     if (!error) {
         error = p->ofproto_class->packet_out(p, payload, &flow,
@@ -2242,16 +2242,30 @@ handle_table_stats_request(struct ofconn *ofconn,
                            const struct ofp_header *request)
 {
     struct ofproto *p = ofconn_get_ofproto(ofconn);
-    struct ofp10_table_stats *ots;
+    struct ofp12_table_stats *ots;
     struct ofpbuf *msg;
     size_t i;
 
-    msg = ofpraw_alloc_stats_reply(request, sizeof *ots * p->n_tables);
-    ots = ofpbuf_put_zeros(msg, sizeof *ots * p->n_tables);
+    /* Set up default values.
+     *
+     * ofp12_table_stats is used as a generic structure as
+     * it is able to hold all the fields for ofp10_table_stats
+     * and ofp11_table_stats (and of course itself).
+     */
+    ots = xcalloc(p->n_tables, sizeof *ots);
     for (i = 0; i < p->n_tables; i++) {
         ots[i].table_id = i;
         sprintf(ots[i].name, "table%zu", i);
-        ots[i].wildcards = htonl(OFPFW10_ALL);
+        ots[i].match = htonll(OFPXMT12_MASK);
+        ots[i].wildcards = htonll(OFPXMT12_MASK);
+        ots[i].write_actions = htonl(OFPAT11_OUTPUT);
+        ots[i].apply_actions = htonl(OFPAT11_OUTPUT);
+        ots[i].write_setfields = htonll(OFPXMT12_MASK);
+        ots[i].apply_setfields = htonll(OFPXMT12_MASK);
+        ots[i].metadata_match = htonll(UINT64_MAX);
+        ots[i].metadata_write = htonll(UINT64_MAX);
+        ots[i].instructions = htonl(OFPIT11_ALL);
+        ots[i].config = htonl(OFPTC11_TABLE_MISS_MASK);
         ots[i].max_entries = htonl(1000000); /* An arbitrary big number. */
         ots[i].active_count = htonl(classifier_count(&p->tables[i].cls));
     }
@@ -2270,36 +2284,25 @@ handle_table_stats_request(struct ofconn *ofconn,
         }
     }
 
+    msg = ofputil_encode_table_stats_reply(ots, p->n_tables, request);
     ofconn_send_reply(ofconn, msg);
+
+    free(ots);
+
     return 0;
 }
 
 static void
 append_port_stat(struct ofport *port, struct list *replies)
 {
-    struct netdev_stats stats;
-    struct ofp10_port_stats *ops;
+    struct ofputil_port_stats ops = { .port_no = port->pp.port_no };
 
     /* Intentionally ignore return value, since errors will set
      * 'stats' to all-1s, which is correct for OpenFlow, and
      * netdev_get_stats() will log errors. */
-    ofproto_port_get_stats(port, &stats);
-
-    ops = ofpmp_append(replies, sizeof *ops);
-    ops->port_no = htons(port->pp.port_no);
-    memset(ops->pad, 0, sizeof ops->pad);
-    put_32aligned_be64(&ops->rx_packets, htonll(stats.rx_packets));
-    put_32aligned_be64(&ops->tx_packets, htonll(stats.tx_packets));
-    put_32aligned_be64(&ops->rx_bytes, htonll(stats.rx_bytes));
-    put_32aligned_be64(&ops->tx_bytes, htonll(stats.tx_bytes));
-    put_32aligned_be64(&ops->rx_dropped, htonll(stats.rx_dropped));
-    put_32aligned_be64(&ops->tx_dropped, htonll(stats.tx_dropped));
-    put_32aligned_be64(&ops->rx_errors, htonll(stats.rx_errors));
-    put_32aligned_be64(&ops->tx_errors, htonll(stats.tx_errors));
-    put_32aligned_be64(&ops->rx_frame_err, htonll(stats.rx_frame_errors));
-    put_32aligned_be64(&ops->rx_over_err, htonll(stats.rx_over_errors));
-    put_32aligned_be64(&ops->rx_crc_err, htonll(stats.rx_crc_errors));
-    put_32aligned_be64(&ops->collisions, htonll(stats.collisions));
+    ofproto_port_get_stats(port, &ops.stats);
+
+    ofputil_append_port_stat(replies, &ops);
 }
 
 static enum ofperr
@@ -2307,13 +2310,19 @@ handle_port_stats_request(struct ofconn *ofconn,
                           const struct ofp_header *request)
 {
     struct ofproto *p = ofconn_get_ofproto(ofconn);
-    const struct ofp10_port_stats_request *psr = ofpmsg_body(request);
     struct ofport *port;
     struct list replies;
+    uint16_t port_no;
+    enum ofperr error;
+
+    error = ofputil_decode_port_stats_request(request, &port_no);
+    if (error) {
+        return error;
+    }
 
     ofpmp_init(&replies, request);
-    if (psr->port_no != htons(OFPP_NONE)) {
-        port = ofproto_get_port(p, ntohs(psr->port_no));
+    if (port_no != OFPP_NONE) {
+        port = ofproto_get_port(p, port_no);
         if (port) {
             append_port_stat(port, &replies);
         }
@@ -2363,7 +2372,7 @@ check_table_id(const struct ofproto *ofproto, uint8_t table_id)
 {
     return (table_id == 0xff || table_id < ofproto->n_tables
             ? 0
-            : OFPERR_NXBRC_BAD_TABLE_ID);
+            : OFPERR_OFPBRC_BAD_TABLE_ID);
 
 }
 
@@ -2766,15 +2775,13 @@ static void
 put_queue_stats(struct queue_stats_cbdata *cbdata, uint32_t queue_id,
                 const struct netdev_queue_stats *stats)
 {
-    struct ofp10_queue_stats *reply;
 
-    reply = ofpmp_append(&cbdata->replies, sizeof *reply);
-    reply->port_no = htons(cbdata->ofport->pp.port_no);
-    memset(reply->pad, 0, sizeof reply->pad);
-    reply->queue_id = htonl(queue_id);
-    put_32aligned_be64(&reply->tx_bytes, htonll(stats->tx_bytes));
-    put_32aligned_be64(&reply->tx_packets, htonll(stats->tx_packets));
-    put_32aligned_be64(&reply->tx_errors, htonll(stats->tx_errors));
+    struct ofputil_queue_stats oqs = {
+        .port_no = cbdata->ofport->pp.port_no,
+        .queue_id = queue_id,
+        .stats = *stats,
+    };
+    ofputil_append_queue_stat(&cbdata->replies, &oqs);
 }
 
 static void
@@ -2812,30 +2819,31 @@ handle_queue_stats_request(struct ofconn *ofconn,
                            const struct ofp_header *rq)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    const struct ofp10_queue_stats_request *qsr = ofpmsg_body(rq);
     struct queue_stats_cbdata cbdata;
-    unsigned int port_no;
     struct ofport *port;
-    uint32_t queue_id;
     enum ofperr error;
+    struct ofputil_queue_stats_request oqsr;
 
     COVERAGE_INC(ofproto_queue_req);
 
     ofpmp_init(&cbdata.replies, rq);
 
-    port_no = ntohs(qsr->port_no);
-    queue_id = ntohl(qsr->queue_id);
-    if (port_no == OFPP_ALL) {
+    error = ofputil_decode_queue_stats_request(rq, &oqsr);
+    if (error) {
+        return error;
+    }
+
+    if (oqsr.port_no == OFPP_ALL) {
         error = OFPERR_OFPQOFC_BAD_QUEUE;
         HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
-            if (!handle_queue_stats_for_port(port, queue_id, &cbdata)) {
+            if (!handle_queue_stats_for_port(port, oqsr.queue_id, &cbdata)) {
                 error = 0;
             }
         }
     } else {
-        port = ofproto_get_port(ofproto, port_no);
+        port = ofproto_get_port(ofproto, oqsr.port_no);
         error = (port
-                 ? handle_queue_stats_for_port(port, queue_id, &cbdata)
+                 ? handle_queue_stats_for_port(port, oqsr.queue_id, &cbdata)
                  : OFPERR_OFPQOFC_BAD_PORT);
     }
     if (!error) {
@@ -2887,8 +2895,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     struct oftable *table;
     struct ofopgroup *group;
     struct rule *victim;
-    struct cls_rule cr;
     struct rule *rule;
+    uint8_t table_id;
     int error;
 
     error = check_table_id(ofproto, fm->table_id);
@@ -2898,7 +2906,6 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
 
     /* Pick table. */
     if (fm->table_id == 0xff) {
-        uint8_t table_id;
         if (ofproto->ofproto_class->rule_choose_table) {
             error = ofproto->ofproto_class->rule_choose_table(ofproto,
                                                               &fm->match,
@@ -2907,16 +2914,17 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
                 return error;
             }
             assert(table_id < ofproto->n_tables);
-            table = &ofproto->tables[table_id];
         } else {
-            table = &ofproto->tables[0];
+            table_id = 0;
         }
     } else if (fm->table_id < ofproto->n_tables) {
-        table = &ofproto->tables[fm->table_id];
+        table_id = fm->table_id;
     } else {
-        return OFPERR_NXFMFC_BAD_TABLE_ID;
+        return OFPERR_OFPBRC_BAD_TABLE_ID;
     }
 
+    table = &ofproto->tables[table_id];
+
     if (table->flags & OFTABLE_READONLY) {
         return OFPERR_OFPBRC_EPERM;
     }
@@ -2931,7 +2939,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     cls_rule_init(&rule->cr, &fm->match, fm->priority);
 
     /* Serialize against pending deletion. */
-    if (is_flow_deletion_pending(ofproto, &cr, table - ofproto->tables)) {
+    if (is_flow_deletion_pending(ofproto, &rule->cr, table_id)) {
         cls_rule_destroy(&rule->cr);
         ofproto->ofproto_class->rule_dealloc(rule);
         return OFPROTO_POSTPONE;
@@ -2980,7 +2988,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
             rule->evictable = was_evictable;
 
             if (!evict) {
-                error = OFPERR_OFPFMFC_ALL_TABLES_FULL;
+                error = OFPERR_OFPFMFC_TABLE_FULL;
                 goto exit;
             } else if (evict->pending) {
                 error = OFPROTO_POSTPONE;
@@ -3050,10 +3058,6 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
         new_cookie = (fm->new_cookie != htonll(UINT64_MAX)
                       ? fm->new_cookie
                       : rule->flow_cookie);
-        if (!actions_changed && new_cookie == rule->flow_cookie) {
-            /* No change at all. */
-            continue;
-        }
 
         op = ofoperation_create(group, rule, OFOPERATION_MODIFY, 0);
         rule->flow_cookie = new_cookie;
@@ -3072,6 +3076,17 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
     return error;
 }
 
+static enum ofperr
+modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
+                 const struct ofputil_flow_mod *fm,
+                 const struct ofp_header *request)
+{
+    if (fm->cookie_mask != htonll(0) || fm->new_cookie == htonll(UINT64_MAX)) {
+        return 0;
+    }
+    return add_flow(ofproto, ofconn, fm, request);
+}
+
 /* Implements OFPFC_MODIFY.  Returns 0 on success or an OpenFlow error code on
  * failure.
  *
@@ -3091,7 +3106,7 @@ modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
-        return fm->cookie_mask ? 0 : add_flow(ofproto, ofconn, fm, request);
+        return modify_flows_add(ofproto, ofconn, fm, request);
     } else {
         return modify_flows__(ofproto, ofconn, fm, request, &rules);
     }
@@ -3117,7 +3132,7 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
-        return fm->cookie_mask ? 0 : add_flow(ofproto, ofconn, fm, request);
+        return modify_flows_add(ofproto, ofconn, fm, request);
     } else {
         return list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn,
                                                           fm, request, &rules)
@@ -3207,6 +3222,7 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
     fr.priority = rule->cr.priority;
     fr.cookie = rule->flow_cookie;
     fr.reason = reason;
+    fr.table_id = rule->table_id;
     calc_flow_duration__(rule->created, time_msec(),
                          &fr.duration_sec, &fr.duration_nsec);
     fr.idle_timeout = rule->idle_timeout;
@@ -3280,12 +3296,11 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
     }
 
     if (fm.flags & OFPFF10_EMERG) {
-    /* We do not support the OpenFlow 1.0 emergency flow cache, which is not
-     * required in OpenFlow 1.0.1 and removed from OpenFlow 1.1. */
-        /* We do not support the emergency flow cache.  It will hopefully get
-         * dropped from OpenFlow in the near future.  There is no good error
-         * code, so just state that the flow table is full. */
-        error = OFPERR_OFPFMFC_ALL_TABLES_FULL;
+        /* We do not support the OpenFlow 1.0 emergency flow cache, which
+         * is not required in OpenFlow 1.0.1 and removed from OpenFlow 1.1.
+         * There is no good error code, so just state that the flow table
+         * is full. */
+        error = OFPERR_OFPFMFC_TABLE_FULL;
     }
     if (!error) {
         error = ofpacts_check(fm.ofpacts, fm.ofpacts_len,
@@ -3532,6 +3547,7 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
     fu.cookie = rule->flow_cookie;
     minimatch_expand(&rule->cr.match, &match);
     fu.match = &match;
+    fu.priority = rule->cr.priority;
     if (!(flags & NXFMF_ACTIONS)) {
         fu.ofpacts = NULL;
         fu.ofpacts_len = 0;
@@ -4002,7 +4018,19 @@ ofopgroup_complete(struct ofopgroup *group)
     LIST_FOR_EACH_SAFE (op, next_op, group_node, &group->ops) {
         struct rule *rule = op->rule;
 
-        if (!op->error && !ofproto_rule_is_hidden(rule)) {
+        /* We generally want to report the change to active OpenFlow flow
+           monitors (e.g. NXST_FLOW_MONITOR).  There are three exceptions:
+
+              - The operation failed.
+
+              - The affected rule is not visible to controllers.
+
+              - The operation's only effect was to update rule->modified. */
+        if (!(op->error
+              || ofproto_rule_is_hidden(rule)
+              || (op->type == OFOPERATION_MODIFY
+                  && op->ofpacts
+                  && rule->flow_cookie == op->flow_cookie))) {
             /* Check that we can just cast from ofoperation_type to
              * nx_flow_update_event. */
             BUILD_ASSERT_DECL((enum nx_flow_update_event) OFOPERATION_ADD