[ovs-dev] [PATCH 2/3] ofproto-dpif: Refactor feature support structure.

Joe Stringer joestringer at nicira.com
Tue Apr 14 00:56:16 UTC 2015


Place all of the detected datapath features into a separate structure,
initialized when the dpif_backer is opened and shared with xbridges.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
 ofproto/ofproto-dpif-xlate.c |   80 +++++++++++++++---------------------------
 ofproto/ofproto-dpif-xlate.h |    7 ++--
 ofproto/ofproto-dpif.c       |   64 +++++++++++++--------------------
 ofproto/ofproto-dpif.h       |   18 ++++++++++
 4 files changed, 73 insertions(+), 96 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 55ae683..e02c8fa 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -96,21 +96,8 @@ struct xbridge {
     bool has_in_band;             /* Bridge has in band control? */
     bool forward_bpdu;            /* Bridge forwards STP BPDUs? */
 
-    /* True if the datapath supports recirculation. */
-    bool enable_recirc;
-
-    /* True if the datapath supports variable-length
-     * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
-     * False if the datapath supports only 8-byte (or shorter) userdata. */
-    bool variable_length_userdata;
-
-    /* Number of MPLS label stack entries that the datapath supports
-     * in matches. */
-    size_t max_mpls_depth;
-
-    /* True if the datapath supports masked data in OVS_ACTION_ATTR_SET
-     * actions. */
-    bool masked_set_action;
+    /* Datapath feature support. */
+    struct dpif_backer_support support;
 };
 
 struct xbundle {
@@ -492,10 +479,7 @@ static void xlate_xbridge_set(struct xbridge *, struct dpif *,
                               const struct dpif_ipfix *,
                               const struct netflow *,
                               bool forward_bpdu, bool has_in_band,
-                              bool enable_recirc,
-                              bool variable_length_userdata,
-                              size_t max_mpls_depth,
-                              bool masked_set_action);
+                              const struct dpif_backer_support *);
 static void xlate_xbundle_set(struct xbundle *xbundle,
                               enum port_vlan_mode vlan_mode, int vlan,
                               unsigned long *trunks, bool use_priority_tags,
@@ -563,10 +547,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
                   const struct dpif_ipfix *ipfix,
                   const struct netflow *netflow,
                   bool forward_bpdu, bool has_in_band,
-                  bool enable_recirc,
-                  bool variable_length_userdata,
-                  size_t max_mpls_depth,
-                  bool masked_set_action)
+                  const struct dpif_backer_support *support)
 {
     if (xbridge->ml != ml) {
         mac_learning_unref(xbridge->ml);
@@ -611,10 +592,7 @@ xlate_xbridge_set(struct xbridge *xbridge,
     xbridge->dpif = dpif;
     xbridge->forward_bpdu = forward_bpdu;
     xbridge->has_in_band = has_in_band;
-    xbridge->enable_recirc = enable_recirc;
-    xbridge->variable_length_userdata = variable_length_userdata;
-    xbridge->max_mpls_depth = max_mpls_depth;
-    xbridge->masked_set_action = masked_set_action;
+    xbridge->support = *support;
 }
 
 static void
@@ -698,10 +676,8 @@ xlate_xbridge_copy(struct xbridge *xbridge)
                       xbridge->dpif, xbridge->ml, xbridge->stp,
                       xbridge->rstp, xbridge->ms, xbridge->mbridge,
                       xbridge->sflow, xbridge->ipfix, xbridge->netflow,
-                      xbridge->forward_bpdu,
-                      xbridge->has_in_band, xbridge->enable_recirc,
-                      xbridge->variable_length_userdata,
-                      xbridge->max_mpls_depth, xbridge->masked_set_action);
+                      xbridge->forward_bpdu, xbridge->has_in_band,
+                      &xbridge->support);
     LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) {
         xlate_xbundle_copy(new_xbridge, xbundle);
     }
@@ -852,9 +828,8 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
                   const struct dpif_sflow *sflow,
                   const struct dpif_ipfix *ipfix,
                   const struct netflow *netflow,
-                  bool forward_bpdu, bool has_in_band, bool enable_recirc,
-                  bool variable_length_userdata, size_t max_mpls_depth,
-                  bool masked_set_action)
+                  bool forward_bpdu, bool has_in_band,
+                  const struct dpif_backer_support *support)
 {
     struct xbridge *xbridge;
 
@@ -872,9 +847,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
     xbridge->name = xstrdup(name);
 
     xlate_xbridge_set(xbridge, dpif, ml, stp, rstp, ms, mbridge, sflow, ipfix,
-                      netflow, forward_bpdu, has_in_band, enable_recirc,
-                      variable_length_userdata, max_mpls_depth,
-                      masked_set_action);
+                      netflow, forward_bpdu, has_in_band, support);
 }
 
 static void
@@ -1754,7 +1727,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
         struct flow_wildcards *wc = &ctx->xout->wc;
         struct ofport_dpif *ofport;
 
-        if (ctx->xbridge->enable_recirc) {
+        if (ctx->xbridge->support.recirc) {
             use_recirc = bond_may_recirc(
                 out_xbundle->bond, &xr.recirc_id, &xr.hash_basis);
 
@@ -2953,10 +2926,11 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     }
 
     if (out_port != ODPP_NONE) {
+        bool use_masked = ctx->xbridge->support.masked_set_action;
+
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
                                               ctx->xout->odp_actions,
-                                              wc,
-                                              ctx->xbridge->masked_set_action);
+                                              wc, use_masked);
 
         if (xr) {
             struct ovs_action_hash *act_hash;
@@ -3426,6 +3400,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 {
     struct ofproto_packet_in *pin;
     struct dp_packet *packet;
+    bool use_masked;
 
     ctx->xout->slow |= SLOW_CONTROLLER;
     if (!ctx->xin->packet) {
@@ -3434,10 +3409,10 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 
     packet = dp_packet_clone(ctx->xin->packet);
 
+    use_masked = ctx->xbridge->support.masked_set_action;
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                           ctx->xout->odp_actions,
-                                          &ctx->xout->wc,
-                                          ctx->xbridge->masked_set_action);
+                                          &ctx->xout->wc, use_masked);
 
     odp_execute_actions(NULL, &packet, 1, false,
                         ctx->xout->odp_actions->data,
@@ -3479,12 +3454,13 @@ static void
 compose_recirculate_action(struct xlate_ctx *ctx)
 {
     struct recirc_metadata md;
+    bool use_masked;
     uint32_t id;
 
+    use_masked = ctx->xbridge->support.masked_set_action;
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                           ctx->xout->odp_actions,
-                                          &ctx->xout->wc,
-                                          ctx->xbridge->masked_set_action);
+                                          &ctx->xout->wc, use_masked);
 
     recirc_metadata_from_flow(&md, &ctx->xin->flow);
 
@@ -3534,10 +3510,11 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
 
     n = flow_count_mpls_labels(flow, wc);
     if (!n) {
+        bool use_masked = ctx->xbridge->support.masked_set_action;
+
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
                                               ctx->xout->odp_actions,
-                                              &ctx->xout->wc,
-                                              ctx->xbridge->masked_set_action);
+                                              &ctx->xout->wc, use_masked);
     } else if (n >= FLOW_MAX_MPLS_LABELS) {
         if (ctx->xin->packet != NULL) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -3561,7 +3538,7 @@ compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
     int n = flow_count_mpls_labels(flow, wc);
 
     if (flow_pop_mpls(flow, n, eth_type, wc)) {
-        if (ctx->xbridge->enable_recirc) {
+        if (ctx->xbridge->support.recirc) {
             ctx->was_mpls = true;
         }
     } else if (n >= FLOW_MAX_MPLS_LABELS) {
@@ -3885,8 +3862,9 @@ xlate_sample_action(struct xlate_ctx *ctx,
     /* Scale the probability from 16-bit to 32-bit while representing
      * the same percentage. */
     uint32_t probability = (os->probability << 16) | os->probability;
+    bool use_masked;
 
-    if (!ctx->xbridge->variable_length_userdata) {
+    if (!ctx->xbridge->support.variable_length_userdata) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 
         VLOG_ERR_RL(&rl, "ignoring NXAST_SAMPLE action because datapath "
@@ -3895,10 +3873,10 @@ xlate_sample_action(struct xlate_ctx *ctx,
         return;
     }
 
+    use_masked = ctx->xbridge->support.masked_set_action;
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                           ctx->xout->odp_actions,
-                                          &ctx->xout->wc,
-                                          ctx->xbridge->masked_set_action);
+                                          &ctx->xout->wc, use_masked);
 
     compose_flow_sample_cookie(os->probability, os->collector_set_id,
                                os->obs_domain_id, os->obs_point_id, &cookie);
@@ -4743,7 +4721,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         if (is_ip_any(flow)) {
             wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
         }
-        if (xbridge->enable_recirc) {
+        if (xbridge->support.recirc) {
             /* Always exactly match recirc_id when datapath supports
              * recirculation.  */
             wc->masks.recirc_id = UINT32_MAX;
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 6c8ade3..e39847b 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -210,11 +210,8 @@ void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,
                        struct rstp *, const struct mcast_snooping *,
                        const struct mbridge *, const struct dpif_sflow *,
                        const struct dpif_ipfix *, const struct netflow *,
-                       bool forward_bpdu,
-                       bool has_in_band, bool enable_recirc,
-                       bool variable_length_userdata,
-                       size_t mpls_label_stack_length,
-                       bool masked_set_action);
+                       bool forward_bpdu, bool has_in_band,
+                       const struct dpif_backer_support *support);
 void xlate_remove_ofproto(struct ofproto_dpif *);
 
 void xlate_bundle_set(struct ofproto_dpif *, struct ofbundle *,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 01d99c5..8f1df74 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -271,30 +271,11 @@ struct dpif_backer {
 
     bool recv_set_enable; /* Enables or disables receiving packets. */
 
-    /* Recirculation. */
-    bool enable_recirc;   /* True if the datapath supports recirculation */
-
-    /* True if the datapath supports unique flow identifiers */
-    bool enable_ufid;
-
-    /* True if the datapath supports variable-length
-     * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
-     * False if the datapath supports only 8-byte (or shorter) userdata. */
-    bool variable_length_userdata;
-
-    /* True if the datapath supports masked data in OVS_ACTION_ATTR_SET
-     * actions. */
-    bool masked_set_action;
-
-    /* Maximum number of MPLS label stack entries that the datapath supports
-     * in a match */
-    size_t max_mpls_depth;
-
     /* Version string of the datapath stored in OVSDB. */
     char *dp_version_string;
 
-    /* True if the datapath supports tnl_push and pop actions. */
-    bool enable_tnl_push_pop;
+    /* Datapath feature support. */
+    struct dpif_backer_support support;
     struct atomic_count tnl_count;
 };
 
@@ -370,19 +351,19 @@ ofproto_dpif_cast(const struct ofproto *ofproto)
 size_t
 ofproto_dpif_get_max_mpls_depth(const struct ofproto_dpif *ofproto)
 {
-    return ofproto->backer->max_mpls_depth;
+    return ofproto->backer->support.max_mpls_depth;
 }
 
 bool
 ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *ofproto)
 {
-    return ofproto->backer->enable_recirc;
+    return ofproto->backer->support.recirc;
 }
 
 bool
 ofproto_dpif_get_enable_ufid(struct dpif_backer *backer)
 {
-    return backer->enable_ufid;
+    return backer->support.ufid;
 }
 
 static void ofproto_trace(struct ofproto_dpif *, struct flow *,
@@ -646,10 +627,7 @@ type_run(const char *type)
                               ofproto->netflow,
                               ofproto->up.forward_bpdu,
                               connmgr_has_in_band(ofproto->up.connmgr),
-                              ofproto->backer->enable_recirc,
-                              ofproto->backer->variable_length_userdata,
-                              ofproto->backer->max_mpls_depth,
-                              ofproto->backer->masked_set_action);
+                              &ofproto->backer->support);
 
             HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
                 xlate_bundle_set(ofproto, bundle, bundle->name,
@@ -873,10 +851,7 @@ struct odp_garbage {
 };
 
 static bool check_variable_length_userdata(struct dpif_backer *backer);
-static size_t check_max_mpls_depth(struct dpif_backer *backer);
-static bool check_recirc(struct dpif_backer *backer);
-static bool check_ufid(struct dpif_backer *backer);
-static bool check_masked_set_action(struct dpif_backer *backer);
+static void check_support(struct dpif_backer *backer);
 
 static int
 open_dpif_backer(const char *type, struct dpif_backer **backerp)
@@ -971,12 +946,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
 
     shash_add(&all_dpif_backers, type, backer);
 
-    backer->enable_recirc = check_recirc(backer);
-    backer->max_mpls_depth = check_max_mpls_depth(backer);
-    backer->masked_set_action = check_masked_set_action(backer);
-    backer->enable_ufid = check_ufid(backer);
-
-    backer->enable_tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif);
+    check_support(backer);
     atomic_count_init(&backer->tnl_count, 0);
 
     error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
@@ -994,7 +964,8 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     /* This check fails if performed before udpif threads have been set,
      * as the kernel module checks that the 'pid' in userspace action
      * is non-zero. */
-    backer->variable_length_userdata = check_variable_length_userdata(backer);
+    backer->support.variable_length_userdata
+        = check_variable_length_userdata(backer);
     backer->dp_version_string = dpif_get_dp_version(backer->dpif);
 
     return error;
@@ -1003,7 +974,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
 bool
 ovs_native_tunneling_is_on(struct ofproto_dpif *ofproto)
 {
-    return ofproto_use_tnl_push_pop && ofproto->backer->enable_tnl_push_pop &&
+    return ofproto_use_tnl_push_pop && ofproto->backer->support.tnl_push_pop &&
            atomic_count_get(&ofproto->backer->tnl_count);
 }
 
@@ -1228,6 +1199,19 @@ check_masked_set_action(struct dpif_backer *backer)
     return !error;
 }
 
+static void
+check_support(struct dpif_backer *backer)
+{
+    /* This feature needs to be tested after udpif threads are set. */
+    backer->support.variable_length_userdata = false;
+
+    backer->support.recirc = check_recirc(backer);
+    backer->support.max_mpls_depth = check_max_mpls_depth(backer);
+    backer->support.masked_set_action = check_masked_set_action(backer);
+    backer->support.ufid = check_ufid(backer);
+    backer->support.tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif);
+}
+
 static int
 construct(struct ofproto *ofproto_)
 {
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 9566924..f9f62ab 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -73,6 +73,24 @@ BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
  *   Ofproto-dpif-xlate is responsible for translating OpenFlow actions into
  *   datapath actions. */
 
+/* Stores the various features which the corresponding backer supports. */
+struct dpif_backer_support {
+    /* True if the datapath supports variable-length
+     * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
+     * False if the datapath supports only 8-byte (or shorter) userdata. */
+    bool variable_length_userdata;
+
+    /* Maximum number of MPLS label stack entries that the datapath supports
+     * in a match */
+    size_t max_mpls_depth;
+
+    /* True if the datapath supports the corresponding feature. */
+    bool masked_set_action;
+    bool recirc;
+    bool tnl_push_pop;
+    bool ufid;
+};
+
 size_t ofproto_dpif_get_max_mpls_depth(const struct ofproto_dpif *);
 bool ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *);
 bool ofproto_dpif_get_enable_ufid(struct dpif_backer *backer);
-- 
1.7.10.4




More information about the dev mailing list