ofproto-dpif: Unhide structure contents.

Until now, ofproto-dpif.c has hidden the definitions of several structures,
such as struct ofproto_dpif and struct rule_dpif.  This kind of information
hiding is often beneficial, because it forces code outside the file with
the definition to use the documented interfaces.  However, in this case it
was starting to burden ofproto-dpif with an increasing number of trivial
helpers that were not improving or maintaining a useful abstraction and
that were making code harder to maintain and read.

Information hiding also made it hard to move blocks of code outside
ofproto-dpif.c itself, since any code moved out often needed new helpers if
it used anything that wasn't previously exposed.  In the present instance,
upcoming patches will move code for tracing outside ofproto-dpif, and this
would require adding several helpers that would just obscure the function
of the code otherwise needlessly.

In balance, it seems that there is more harm than good in the information
hiding here, so this commit moves the definitions of several structures
from ofproto-dpif.c into ofproto-dpif.h.  It also removes all of the
trivial helpers that had accumulated, instead changing their users to
directly access the members that they needed.  It also reorganizes
ofproto-dpif.h, grouping structure definitions and function prototypes in a
sensible way.

Signed-off-by: Ben Pfaff <[email protected]>
Acked-by: Lance Richardson <[email protected]>
Acked-by: Justin Pettit <[email protected]>

ofproto/bond.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -59,6 +59,9 @@ static struct hmap *const all_bonds OVS_GUARDED_BY(rwlock) = &all_bonds__;
 #define BOND_MASK 0xff
 #define BOND_BUCKETS (BOND_MASK + 1)
 
+/* Priority for internal rules created to handle recirculation */
+#define RECIRC_RULE_PRIORITY 20
+
 /* A hash bucket for mapping a flow to a slave.
  * "struct bond" has an array of BOND_BUCKETS of these. */
 struct bond_entry {
@@ -1131,7 +1134,7 @@ bond_rebalance(struct bond *bond)
     }
     bond->next_rebalance = time_msec() + bond->rebalance_interval;
 
-    use_recirc = ofproto_dpif_get_support(bond->ofproto)->odp.recirc &&
+    use_recirc = bond->ofproto->backer->support.odp.recirc &&
                  bond_may_recirc(bond, NULL, NULL);
 
     if (use_recirc) {

ofproto/ofproto-dpif-rid.c

@@ -285,7 +285,7 @@ recirc_alloc_id(struct ofproto_dpif *ofproto)
     tunnel.ipv6_dst = in6addr_any;
     struct frozen_state state = {
         .table_id = TBL_INTERNAL,
-        .ofproto_uuid = *ofproto_dpif_get_uuid(ofproto),
+        .ofproto_uuid = ofproto->uuid,
         .metadata = { .tunnel = &tunnel, .in_port = OFPP_NONE },
     };
     return recirc_alloc_id__(&state, frozen_state_hash(&state))->id;
@@ -338,9 +338,8 @@ recirc_free_ofproto(struct ofproto_dpif *ofproto, const char *ofproto_name)
 {
     struct recirc_id_node *n;
 
-    const struct uuid *ofproto_uuid = ofproto_dpif_get_uuid(ofproto);
     CMAP_FOR_EACH (n, metadata_node, &metadata_map) {
-        if (uuid_equals(&n->state.ofproto_uuid, ofproto_uuid)) {
+        if (uuid_equals(&n->state.ofproto_uuid, &ofproto->uuid)) {
             VLOG_ERR("recirc_id %"PRIu32
                      " left allocated when ofproto (%s)"
                      " is destructed", n->id, ofproto_name);

ofproto/ofproto-dpif-upcall.c

@@ -541,7 +541,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
                 "handler", udpif_upcall_handler, handler);
         }
 
-        enable_ufid = ofproto_dpif_get_enable_ufid(udpif->backer);
+        enable_ufid = udpif->backer->support.ufid;
         atomic_init(&udpif->enable_ufid, enable_ufid);
         dpif_enable_upcall(udpif->dpif);
 
@@ -567,7 +567,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
 static void
 udpif_pause_revalidators(struct udpif *udpif)
 {
-    if (ofproto_dpif_backer_enabled(udpif->backer)) {
+    if (udpif->backer->recv_set_enable) {
         latch_set(&udpif->pause_latch);
         ovs_barrier_block(&udpif->pause_barrier);
     }
@@ -578,7 +578,7 @@ udpif_pause_revalidators(struct udpif *udpif)
 static void
 udpif_resume_revalidators(struct udpif *udpif)
 {
-    if (ofproto_dpif_backer_enabled(udpif->backer)) {
+    if (udpif->backer->recv_set_enable) {
         latch_poll(&udpif->pause_latch);
         ovs_barrier_block(&udpif->pause_barrier);
     }
@@ -700,7 +700,7 @@ udpif_use_ufid(struct udpif *udpif)
     bool enable;
 
     atomic_read_relaxed(&enable_ufid, &enable);
-    return enable && ofproto_dpif_get_enable_ufid(udpif->backer);
+    return enable && udpif->backer->support.ufid;
 }
 
 
@@ -1507,7 +1507,7 @@ ukey_create_from_upcall(struct upcall *upcall, struct flow_wildcards *wc)
         .mask = wc ? &wc->masks : NULL,
     };
 
-    odp_parms.support = ofproto_dpif_get_support(upcall->ofproto)->odp;
+    odp_parms.support = upcall->ofproto->backer->support.odp;
     if (upcall->key_len) {
         ofpbuf_use_const(&keybuf, upcall->key, upcall->key_len);
     } else {

ofproto/ofproto-dpif-xlate-cache.c

@@ -139,8 +139,8 @@ xlate_push_stats_entry(struct xc_entry *entry,
         break;
     case XC_FIN_TIMEOUT:
         if (stats->tcp_flags & (TCP_FIN | TCP_RST)) {
-            rule_dpif_reduce_timeouts(entry->fin.rule, entry->fin.idle,
-                                      entry->fin.hard);
+            ofproto_rule_reduce_timeouts(&entry->fin.rule->up, entry->fin.idle,
+                                         entry->fin.hard);
         }
         break;
     case XC_GROUP:
@@ -208,7 +208,7 @@ xlate_cache_clear_entry(struct xc_entry *entry)
     case XC_TABLE:
         break;
     case XC_RULE:
-        rule_dpif_unref(entry->rule);
+        ofproto_rule_unref(&entry->rule->up);
         break;
     case XC_BOND:
         free(entry->bond.flow);
@@ -234,7 +234,7 @@ xlate_cache_clear_entry(struct xc_entry *entry)
          * has already released it's reference above. */
         break;
     case XC_GROUP:
-        group_dpif_unref(entry->group.group);
+        ofproto_group_unref(&entry->group.group->up);
         break;
     case XC_TNL_NEIGH:
         break;

ofproto/ofproto-dpif-xlate.c

@@ -1246,7 +1246,7 @@ xbridge_lookup_by_uuid(struct xlate_cfg *xcfg, const struct uuid *uuid)
     struct xbridge *xbridge;
 
     HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) {
-        if (uuid_equals(ofproto_dpif_get_uuid(xbridge->ofproto), uuid)) {
+        if (uuid_equals(&xbridge->ofproto->uuid, uuid)) {
             return xbridge;
         }
     }
@@ -1485,10 +1485,7 @@ group_first_live_bucket(const struct xlate_ctx *ctx,
                         const struct group_dpif *group, int depth)
 {
     struct ofputil_bucket *bucket;
-    const struct ovs_list *buckets;
-
-    buckets = group_dpif_get_buckets(group, NULL);
-    LIST_FOR_EACH (bucket, list_node, buckets) {
+    LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
         if (bucket_is_alive(ctx, bucket, depth)) {
             return bucket;
         }
@@ -1506,10 +1503,7 @@ group_best_live_bucket(const struct xlate_ctx *ctx,
     uint32_t best_score = 0;
 
     struct ofputil_bucket *bucket;
-    const struct ovs_list *buckets;
-
-    buckets = group_dpif_get_buckets(group, NULL);
-    LIST_FOR_EACH (bucket, list_node, buckets) {
+    LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
         if (bucket_is_alive(ctx, bucket, 0)) {
             uint32_t score =
                 (hash_int(bucket->bucket_id, basis) & 0xffff) * bucket->weight;
@@ -2979,7 +2973,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         flow->in_port.ofp_port = peer->ofp_port;
         flow->metadata = htonll(0);
         memset(&flow->tunnel, 0, sizeof flow->tunnel);
-        flow->tunnel.metadata.tab = ofproto_dpif_get_tun_tab(peer->xbridge->ofproto);
+        flow->tunnel.metadata.tab = ofproto_get_tun_tab(
+            &peer->xbridge->ofproto->up);
         ctx->wc->masks.tunnel.metadata.tab = flow->tunnel.metadata.tab;
         memset(flow->regs, 0, sizeof flow->regs);
         flow->actset_output = OFPP_UNSET;
@@ -3249,8 +3244,8 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule, bool deepens)
     ctx->indentation++;
     ctx->depth += deepens;
     ctx->rule = rule;
-    ctx->rule_cookie = rule_dpif_get_flow_cookie(rule);
-    actions = rule_dpif_get_actions(rule);
+    ctx->rule_cookie = rule->up.flow_cookie;
+    actions = rule_get_actions(&rule->up);
     do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);
     ctx->rule_cookie = old_cookie;
     ctx->rule = old_rule;
@@ -3317,7 +3312,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
 
                 entry = xlate_cache_add_entry(ctx->xin->xcache, XC_RULE);
                 entry->rule = rule;
-                rule_dpif_ref(rule);
+                ofproto_rule_ref(&rule->up);
             }
             xlate_recursively(ctx, rule, table_id <= old_table_id);
         }
@@ -3399,10 +3394,7 @@ static void
 xlate_all_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
     struct ofputil_bucket *bucket;
-    const struct ovs_list *buckets;
-
-    buckets = group_dpif_get_buckets(group, NULL);
-    LIST_FOR_EACH (bucket, list_node, buckets) {
+    LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
         xlate_group_bucket(ctx, bucket);
     }
     xlate_group_stats(ctx, group, NULL);
@@ -3418,7 +3410,7 @@ xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)
         xlate_group_bucket(ctx, bucket);
         xlate_group_stats(ctx, group, bucket);
     } else if (ctx->xin->xcache) {
-        group_dpif_unref(group);
+        ofproto_group_unref(&group->up);
     }
 }
 
@@ -3436,23 +3428,18 @@ xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
         xlate_group_bucket(ctx, bucket);
         xlate_group_stats(ctx, group, bucket);
     } else if (ctx->xin->xcache) {
-        group_dpif_unref(group);
+        ofproto_group_unref(&group->up);
     }
 }
 
 static void
 xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
-    const struct field_array *fields;
-    struct ofputil_bucket *bucket;
-    const uint8_t *mask_values;
-    uint32_t basis;
-    size_t i;
-
-    fields = group_dpif_get_fields(group);
-    mask_values = fields->values;
-    basis = hash_uint64(group_dpif_get_selection_method_param(group));
+    const struct field_array *fields = &group->up.props.fields;
+    const uint8_t *mask_values = fields->values;
+    uint32_t basis = hash_uint64(group->up.props.selection_method_param);
 
+    size_t i;
     BITMAP_FOR_EACH_1 (i, MFF_N_IDS, fields->used.bm) {
         const struct mf_field *mf = mf_from_id(i);
 
@@ -3482,12 +3469,12 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
         mf_mask_field_masked(mf, &mask, ctx->wc);
     }
 
-    bucket = group_best_live_bucket(ctx, group, basis);
+    struct ofputil_bucket *bucket = group_best_live_bucket(ctx, group, basis);
     if (bucket) {
         xlate_group_bucket(ctx, bucket);
         xlate_group_stats(ctx, group, bucket);
     } else if (ctx->xin->xcache) {
-        group_dpif_unref(group);
+        ofproto_group_unref(&group->up);
     }
 }
 
@@ -3501,13 +3488,11 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
      * compare to zero can be used to decide if the dp_hash value is valid
      * without masking the dp_hash field. */
     if (!ctx->xin->flow.dp_hash) {
-        uint64_t param = group_dpif_get_selection_method_param(group);
+        uint64_t param = group->up.props.selection_method_param;
 
         ctx_trigger_recirculate_with_hash(ctx, param >> 32, (uint32_t)param);
     } else {
-        uint32_t n_buckets;
-
-        group_dpif_get_buckets(group, &n_buckets);
+        uint32_t n_buckets = group->up.n_buckets;
         if (n_buckets) {
             /* Minimal mask to cover the number of buckets. */
             uint32_t mask = (1 << log_2_ceil(n_buckets)) - 1;
@@ -3528,7 +3513,7 @@ xlate_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 static void
 xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
 {
-    const char *selection_method = group_dpif_get_selection_method(group);
+    const char *selection_method = group->up.props.selection_method;
 
     /* Select groups may access flow keys beyond L2 in order to
      * select a bucket. Recirculate as appropriate to make this possible.
@@ -3555,7 +3540,7 @@ xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
     bool was_in_group = ctx->in_group;
     ctx->in_group = true;
 
-    switch (group_dpif_get_type(group)) {
+    switch (group->up.type) {
     case OFPGT11_ALL:
     case OFPGT11_INDIRECT:
         xlate_all_group(ctx, group);
@@ -3670,7 +3655,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
      * explicit table miss.  OpenFlow before 1.3 doesn't have that concept so
      * it will get translated back to OFPR_ACTION for those versions. */
     if (reason == OFPR_ACTION
-        && ctx->rule && rule_dpif_is_table_miss(ctx->rule)) {
+        && ctx->rule && rule_is_table_miss(&ctx->rule->up)) {
         reason = OFPR_EXPLICIT_MISS;
     }
 
@@ -3736,7 +3721,7 @@ emit_continuation(struct xlate_ctx *ctx, const struct frozen_state *state)
                     .packet_len = dp_packet_size(ctx->xin->packet),
                     .reason = ctx->pause->reason,
                 },
-                .bridge = *ofproto_dpif_get_uuid(ctx->xbridge->ofproto),
+                .bridge = ctx->xbridge->ofproto->uuid,
                 .stack = xmemdup(state->stack,
                                  state->n_stack * sizeof *state->stack),
                 .n_stack = state->n_stack,
@@ -3773,7 +3758,7 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
 
     struct frozen_state state = {
         .table_id = table,
-        .ofproto_uuid = *ofproto_dpif_get_uuid(ctx->xbridge->ofproto),
+        .ofproto_uuid = ctx->xbridge->ofproto->uuid,
         .stack = ctx->stack.data,
         .n_stack = ctx->stack.size / sizeof(union mf_subvalue),
         .mirrors = ctx->mirrors,
@@ -4215,7 +4200,7 @@ xlate_fin_timeout__(struct rule_dpif *rule, uint16_t tcp_flags,
                     uint16_t idle_timeout, uint16_t hard_timeout)
 {
     if (tcp_flags & (TCP_FIN | TCP_RST)) {
-        rule_dpif_reduce_timeouts(rule, idle_timeout, hard_timeout);
+        ofproto_rule_reduce_timeouts(&rule->up, idle_timeout, hard_timeout);
     }
 }
 
@@ -5478,8 +5463,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         }
 
         /* Set the bridge for post-recirculation processing if needed. */
-        if (!uuid_equals(ofproto_dpif_get_uuid(ctx.xbridge->ofproto),
-                         &state->ofproto_uuid)) {
+        if (!uuid_equals(&ctx.xbridge->ofproto->uuid, &state->ofproto_uuid)) {
             struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
             const struct xbridge *new_bridge
                 = xbridge_lookup_by_uuid(xcfg, &state->ofproto_uuid);
@@ -5549,7 +5533,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
     /* Tunnel metadata in udpif format must be normalized before translation. */
     if (flow->tunnel.flags & FLOW_TNL_F_UDPIF) {
-        const struct tun_table *tun_tab = ofproto_dpif_get_tun_tab(xin->ofproto);
+        const struct tun_table *tun_tab
+            = ofproto_get_tun_tab(&xin->ofproto->up);
         int err;
 
         err = tun_metadata_from_geneve_udpif(tun_tab, &xin->upcall_flow->tunnel,
@@ -5564,7 +5549,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         /* If the original flow did not come in on a tunnel, then it won't have
          * FLOW_TNL_F_UDPIF set. However, we still need to have a metadata
          * table in case we generate tunnel actions. */
-        flow->tunnel.metadata.tab = ofproto_dpif_get_tun_tab(xin->ofproto);
+        flow->tunnel.metadata.tab = ofproto_get_tun_tab(&xin->ofproto->up);
     }
     ctx.wc->masks.tunnel.metadata.tab = flow->tunnel.metadata.tab;
 
@@ -5581,7 +5566,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
             entry = xlate_cache_add_entry(ctx.xin->xcache, XC_RULE);
             entry->rule = ctx.rule;
-            rule_dpif_ref(ctx.rule);
+            ofproto_rule_ref(&ctx.rule->up);
         }
 
         if (OVS_UNLIKELY(ctx.xin->resubmit_hook)) {
@@ -5643,10 +5628,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
                 ofpacts_len = xin->ofpacts_len;
             } else if (ctx.rule) {
                 const struct rule_actions *actions
-                    = rule_dpif_get_actions(ctx.rule);
+                    = rule_get_actions(&ctx.rule->up);
                 ofpacts = actions->ofpacts;
                 ofpacts_len = actions->ofpacts_len;
-                ctx.rule_cookie = rule_dpif_get_flow_cookie(ctx.rule);
+                ctx.rule_cookie = ctx.rule->up.flow_cookie;
             } else {
                 OVS_NOT_REACHED();
             }