ofproto-dpif: Avoid dereferencing possibly null or wild pointer.
[sliver-openvswitch.git] / ofproto / ofproto-dpif.c
index dc15c15..ac1a963 100644 (file)
@@ -66,7 +66,7 @@ COVERAGE_DEFINE(facet_suppress);
 
 /* Maximum depth of flow table recursion (due to resubmit actions) in a
  * flow translation. */
-#define MAX_RESUBMIT_RECURSION 32
+#define MAX_RESUBMIT_RECURSION 64
 
 /* Number of implemented OpenFlow tables. */
 enum { N_TABLES = 255 };
@@ -243,6 +243,11 @@ struct action_xlate_ctx {
      * calling action_xlate_ctx_init(). */
     void (*resubmit_hook)(struct action_xlate_ctx *, struct rule_dpif *rule);
 
+    /* If nonnull, flow translation calls this function to report some
+     * significant decision, e.g. to explain why OFPP_NORMAL translation
+     * dropped a packet. */
+    void (*report_hook)(struct action_xlate_ctx *, const char *s);
+
     /* If nonnull, flow translation credits the specified statistics to each
      * rule reached through a resubmit or OFPP_TABLE action.
      *
@@ -299,6 +304,8 @@ static void compose_slow_path(const struct ofproto_dpif *, const struct flow *,
                               const struct nlattr **actionsp,
                               size_t *actions_lenp);
 
+static void xlate_report(struct action_xlate_ctx *ctx, const char *s);
+
 /* A subfacet (see "struct subfacet" below) has three possible installation
  * states:
  *
@@ -1146,7 +1153,7 @@ get_features(struct ofproto *ofproto_ OVS_UNUSED,
 }
 
 static void
-get_tables(struct ofproto *ofproto_, struct ofp_table_stats *ots)
+get_tables(struct ofproto *ofproto_, struct ofp10_table_stats *ots)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct dpif_dp_stats s;
@@ -1308,6 +1315,14 @@ get_cfm_fault(const struct ofport *ofport_)
     return ofport->cfm ? cfm_get_fault(ofport->cfm) : -1;
 }
 
+static int
+get_cfm_opup(const struct ofport *ofport_)
+{
+    struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
+
+    return ofport->cfm ? cfm_get_opup(ofport->cfm) : -1;
+}
+
 static int
 get_cfm_remote_mpids(const struct ofport *ofport_, const uint64_t **rmps,
                      size_t *n_rmps)
@@ -1957,7 +1972,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
         break;
 
     case PORT_VLAN_TRUNK:
-        trunks = (unsigned long *) s->trunks;
+        trunks = CONST_CAST(unsigned long *, s->trunks);
         break;
 
     case PORT_VLAN_NATIVE_UNTAGGED:
@@ -1974,7 +1989,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
             bitmap_set1(trunks, vlan);
             bitmap_set0(trunks, 0);
         } else {
-            trunks = (unsigned long *) s->trunks;
+            trunks = CONST_CAST(unsigned long *, s->trunks);
         }
         break;
 
@@ -2457,9 +2472,14 @@ port_run(struct ofport_dpif *ofport)
 
     port_run_fast(ofport);
     if (ofport->cfm) {
+        int cfm_opup = cfm_get_opup(ofport->cfm);
+
         cfm_run(ofport->cfm);
-        enable = enable && !cfm_get_fault(ofport->cfm)
-            && cfm_get_opup(ofport->cfm);
+        enable = enable && !cfm_get_fault(ofport->cfm);
+
+        if (cfm_opup >= 0) {
+            enable = enable && cfm_opup;
+        }
     }
 
     if (ofport->bundle) {
@@ -2507,7 +2527,7 @@ static int
 port_add(struct ofproto *ofproto_, struct netdev *netdev, uint16_t *ofp_portp)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
-    uint16_t odp_port;
+    uint16_t odp_port = UINT16_MAX;
     int error;
 
     error = dpif_port_add(ofproto->dpif, netdev, &odp_port);
@@ -2563,7 +2583,7 @@ port_get_stats(const struct ofport *ofport_, struct netdev_stats *stats)
 
         /* ofproto->stats.rx_packets represents packets that were received on
          * some port and we processed internally and dropped (e.g. STP).
-         * Account fro them as if they had been forwarded to OFPP_LOCAL. */
+         * Account for them as if they had been forwarded to OFPP_LOCAL. */
 
         if (stats->tx_packets != UINT64_MAX) {
             stats->tx_packets += ofproto->stats.rx_packets;
@@ -3774,7 +3794,8 @@ facet_is_controller_flow(struct facet *facet)
         const struct ofpact *ofpacts = rule->ofpacts;
         size_t ofpacts_len = rule->ofpacts_len;
 
-        if (ofpacts->type == OFPACT_CONTROLLER &&
+        if (ofpacts_len > 0 &&
+            ofpacts->type == OFPACT_CONTROLLER &&
             ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len)) {
             return true;
         }
@@ -4235,20 +4256,33 @@ subfacet_create(struct facet *facet, enum odp_key_fitness key_fitness,
     uint32_t key_hash = odp_flow_key_hash(key, key_len);
     struct subfacet *subfacet;
 
-    subfacet = subfacet_find__(ofproto, key, key_len, key_hash, &facet->flow);
-    if (subfacet) {
-        if (subfacet->facet == facet) {
-            return subfacet;
+    if (list_is_empty(&facet->subfacets)) {
+        subfacet = &facet->one_subfacet;
+
+        /* This subfacet should conceptually be created, and have its first
+         * packet pass through, at the same time that its facet was created.
+         * If we called time_msec() here, then the subfacet could look
+         * (occasionally) as though it was used some time after the facet was
+         * used.  That can make a one-packet flow look like it has a nonzero
+         * duration, which looks odd in e.g. NetFlow statistics. */
+        subfacet->used = facet->used;
+    } else {
+        subfacet = subfacet_find__(ofproto, key, key_len, key_hash,
+                                   &facet->flow);
+        if (subfacet) {
+            if (subfacet->facet == facet) {
+                return subfacet;
+            }
+
+            /* This shouldn't happen. */
+            VLOG_ERR_RL(&rl, "subfacet with wrong facet");
+            subfacet_destroy(subfacet);
         }
 
-        /* This shouldn't happen. */
-        VLOG_ERR_RL(&rl, "subfacet with wrong facet");
-        subfacet_destroy(subfacet);
+        subfacet = xmalloc(sizeof *subfacet);
+        subfacet->used = time_msec();
     }
 
-    subfacet = (list_is_empty(&facet->subfacets)
-                ? &facet->one_subfacet
-                : xmalloc(sizeof *subfacet));
     hmap_insert(&ofproto->subfacets, &subfacet->hmap_node, key_hash);
     list_push_back(&facet->subfacets, &subfacet->list_node);
     subfacet->facet = facet;
@@ -4260,7 +4294,6 @@ subfacet_create(struct facet *facet, enum odp_key_fitness key_fitness,
         subfacet->key = NULL;
         subfacet->key_len = 0;
     }
-    subfacet->used = time_msec();
     subfacet->dp_packet_count = 0;
     subfacet->dp_byte_count = 0;
     subfacet->actions_len = 0;
@@ -4725,7 +4758,7 @@ send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     struct flow flow;
     int error;
 
-    flow_extract((struct ofpbuf *) packet, 0, 0, 0, &flow);
+    flow_extract(packet, 0, 0, 0, &flow);
     odp_port = vsp_realdev_to_vlandev(ofproto, ofport->odp_port,
                                       flow.vlan_tci);
     if (odp_port != ofport->odp_port) {
@@ -5127,7 +5160,7 @@ execute_controller_action(struct action_xlate_ctx *ctx, int len,
 }
 
 static bool
-compose_dec_ttl(struct action_xlate_ctx *ctx)
+compose_dec_ttl(struct action_xlate_ctx *ctx, struct ofpact_cnt_ids *ids)
 {
     if (ctx->flow.dl_type != htons(ETH_TYPE_IP) &&
         ctx->flow.dl_type != htons(ETH_TYPE_IPV6)) {
@@ -5138,7 +5171,12 @@ compose_dec_ttl(struct action_xlate_ctx *ctx)
         ctx->flow.nw_ttl--;
         return false;
     } else {
-        execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
+        size_t i;
+
+        for (i = 0; i < ids->n_controllers; i++) {
+            execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL,
+                                      ids->cnt_ids[i]);
+        }
 
         /* Stop processing for current table. */
         return true;
@@ -5498,7 +5536,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_DEC_TTL:
-            if (compose_dec_ttl(ctx)) {
+            if (compose_dec_ttl(ctx, ofpact_get_DEC_TTL(a))) {
                 goto out;
             }
             break;
@@ -5570,6 +5608,7 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx,
     ctx->may_learn = packet != NULL;
     ctx->tcp_flags = tcp_flags;
     ctx->resubmit_hook = NULL;
+    ctx->report_hook = NULL;
     ctx->resubmit_stats = NULL;
 }
 
@@ -5696,6 +5735,14 @@ xlate_actions_for_side_effects(struct action_xlate_ctx *ctx,
     xlate_actions(ctx, ofpacts, ofpacts_len, &odp_actions);
     ofpbuf_uninit(&odp_actions);
 }
+
+static void
+xlate_report(struct action_xlate_ctx *ctx, const char *s)
+{
+    if (ctx->report_hook) {
+        ctx->report_hook(ctx, s);
+    }
+}
 \f
 /* OFPP_NORMAL implementation. */
 
@@ -6107,14 +6154,17 @@ lookup_input_bundle(const struct ofproto_dpif *ofproto, uint16_t in_port,
  * so in one special case.
  */
 static bool
-is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
-              struct ofport_dpif *in_port, uint16_t vlan, tag_type *tags)
+is_admissible(struct action_xlate_ctx *ctx, struct ofport_dpif *in_port,
+              uint16_t vlan)
 {
+    struct ofproto_dpif *ofproto = ctx->ofproto;
+    struct flow *flow = &ctx->flow;
     struct ofbundle *in_bundle = in_port->bundle;
 
     /* Drop frames for reserved multicast addresses
      * only if forward_bpdu option is absent. */
     if (!ofproto->up.forward_bpdu && eth_addr_is_reserved(flow->dl_dst)) {
+        xlate_report(ctx, "packet has reserved destination MAC, dropping");
         return false;
     }
 
@@ -6122,11 +6172,12 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
         struct mac_entry *mac;
 
         switch (bond_check_admissibility(in_bundle->bond, in_port,
-                                         flow->dl_dst, tags)) {
+                                         flow->dl_dst, &ctx->tags)) {
         case BV_ACCEPT:
             break;
 
         case BV_DROP:
+            xlate_report(ctx, "bonding refused admissibility, dropping");
             return false;
 
         case BV_DROP_IF_MOVED:
@@ -6134,6 +6185,8 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
             if (mac && mac->port.p != in_bundle &&
                 (!is_gratuitous_arp(flow)
                  || mac_entry_is_grat_arp_locked(mac))) {
+                xlate_report(ctx, "SLB bond thinks this packet looped back, "
+                            "dropping");
                 return false;
             }
             break;
@@ -6157,6 +6210,7 @@ xlate_normal(struct action_xlate_ctx *ctx)
     in_bundle = lookup_input_bundle(ctx->ofproto, ctx->flow.in_port,
                                     ctx->packet != NULL, &in_port);
     if (!in_bundle) {
+        xlate_report(ctx, "no input bundle, dropping");
         return;
     }
 
@@ -6169,6 +6223,7 @@ xlate_normal(struct action_xlate_ctx *ctx)
                          "VLAN tag received on port %s",
                          ctx->ofproto->up.name, in_bundle->name);
         }
+        xlate_report(ctx, "partial VLAN tag, dropping");
         return;
     }
 
@@ -6180,19 +6235,20 @@ xlate_normal(struct action_xlate_ctx *ctx)
                          "%s, which is reserved exclusively for mirroring",
                          ctx->ofproto->up.name, in_bundle->name);
         }
+        xlate_report(ctx, "input port is mirror output port, dropping");
         return;
     }
 
     /* Check VLAN. */
     vid = vlan_tci_to_vid(ctx->flow.vlan_tci);
     if (!input_vid_is_valid(vid, in_bundle, ctx->packet != NULL)) {
+        xlate_report(ctx, "disallowed VLAN VID for this input port, dropping");
         return;
     }
     vlan = input_vid_to_vlan(in_bundle, vid);
 
     /* Check other admissibility requirements. */
-    if (in_port &&
-         !is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan, &ctx->tags)) {
+    if (in_port && !is_admissible(ctx, in_port, vlan)) {
         return;
     }
 
@@ -6206,11 +6262,15 @@ xlate_normal(struct action_xlate_ctx *ctx)
                               &ctx->tags);
     if (mac) {
         if (mac->port.p != in_bundle) {
+            xlate_report(ctx, "forwarding to learned port");
             output_normal(ctx, mac->port.p, vlan);
+        } else {
+            xlate_report(ctx, "learned port is input port, dropping");
         }
     } else {
         struct ofbundle *bundle;
 
+        xlate_report(ctx, "no learned MAC for destination, flooding");
         HMAP_FOR_EACH (bundle, hmap_node, &ctx->ofproto->bundles) {
             if (bundle != in_bundle
                 && ofbundle_includes_vlan(bundle, vlan)
@@ -6600,6 +6660,17 @@ trace_resubmit(struct action_xlate_ctx *ctx, struct rule_dpif *rule)
     trace_format_rule(result, ctx->table_id, ctx->recurse + 1, rule);
 }
 
+static void
+trace_report(struct action_xlate_ctx *ctx, const char *s)
+{
+    struct trace_ctx *trace = CONTAINER_OF(ctx, struct trace_ctx, ctx);
+    struct ds *result = trace->result;
+
+    ds_put_char_multiple(result, '\t', ctx->recurse);
+    ds_put_cstr(result, s);
+    ds_put_char(result, '\n');
+}
+
 static void
 ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
                       void *aux OVS_UNUSED)
@@ -6750,6 +6821,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
         action_xlate_ctx_init(&trace.ctx, ofproto, flow, initial_tci,
                               rule, tcp_flags, packet);
         trace.ctx.resubmit_hook = trace_resubmit;
+        trace.ctx.report_hook = trace_report;
         xlate_actions(&trace.ctx, rule->up.ofpacts, rule->up.ofpacts_len,
                       &odp_actions);
 
@@ -7127,6 +7199,7 @@ const struct ofproto_class ofproto_dpif_class = {
     set_sflow,
     set_cfm,
     get_cfm_fault,
+    get_cfm_opup,
     get_cfm_remote_mpids,
     get_cfm_health,
     set_stp,