[ovs-dev] [PATCH v4 1/5] Sanitize remaining recirculation parameters.

Jarno Rajahalme jrajahalme at nicira.com
Fri Mar 20 01:03:25 UTC 2015


Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 ofproto/ofproto-dpif-xlate.c |   66 +++++++++++++++++++++++++-----------------
 ofproto/ofproto-dpif-xlate.h |    6 ----
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0e28c77..ece9670 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -208,14 +208,13 @@ struct xlate_ctx {
     uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
     bool exit;                  /* No further actions should be processed. */
 
-    bool use_recirc;            /* Should generate recirc? */
-    struct xlate_recirc recirc; /* Information used for generating
-                                 * recirculation actions */
-
     /* True if a packet was but is no longer MPLS (due to an MPLS pop action).
      * This is a trigger for recirculation in cases where translating an action
      * or looking up a flow requires access to the fields of the packet after
-     * the MPLS label stack that was originally present. */
+     * the MPLS label stack that was originally present.
+     *
+     * XXX: output to a table and patch port do not currently recirculate even
+     * if this is true. */
     bool was_mpls;
 
     /* OpenFlow 1.1+ action set.
@@ -353,7 +352,16 @@ static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
 static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
 static void output_normal(struct xlate_ctx *, const struct xbundle *,
                           uint16_t vlan);
-static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port);
+
+/* Optional bond recirculation parameter to compose_output_action(). */
+struct xlate_bond_recirc {
+    uint32_t recirc_id;  /* !0 Use recirculation instead of output. */
+    uint8_t  hash_alg;   /* !0 Compute hash for recirc before. */
+    uint32_t hash_basis;  /* Compute hash for recirc before. */
+};
+
+static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port,
+                                  const struct xlate_bond_recirc *xr);
 
 static struct xbridge *xbridge_lookup(struct xlate_cfg *,
                                       const struct ofproto_dpif *);
@@ -1670,28 +1678,28 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
     uint16_t vid;
     ovs_be16 tci, old_tci;
     struct xport *xport;
+    struct xlate_bond_recirc xr;
+    bool use_recirc = false;
 
     vid = output_vlan_to_vid(out_xbundle, vlan);
     if (list_is_empty(&out_xbundle->xports)) {
         /* Partially configured bundle with no slaves.  Drop the packet. */
         return;
     } else if (!out_xbundle->bond) {
-        ctx->use_recirc = false;
         xport = CONTAINER_OF(list_front(&out_xbundle->xports), struct xport,
                              bundle_node);
     } else {
         struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
         struct flow_wildcards *wc = &ctx->xout->wc;
-        struct xlate_recirc *xr = &ctx->recirc;
         struct ofport_dpif *ofport;
 
         if (ctx->xbridge->enable_recirc) {
-            ctx->use_recirc = bond_may_recirc(
-                out_xbundle->bond, &xr->recirc_id, &xr->hash_basis);
+            use_recirc = bond_may_recirc(
+                out_xbundle->bond, &xr.recirc_id, &xr.hash_basis);
 
-            if (ctx->use_recirc) {
+            if (use_recirc) {
                 /* Only TCP mode uses recirculation. */
-                xr->hash_alg = OVS_HASH_ALG_L4;
+                xr.hash_alg = OVS_HASH_ALG_L4;
                 bond_update_post_recirc_rules(out_xbundle->bond, false);
 
                 /* Recirculation does not require unmasking hash fields. */
@@ -1708,9 +1716,9 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
             return;
         }
 
-        /* If ctx->xout->use_recirc is set, the main thread will handle stats
+        /* If use_recirc is set, the main thread will handle stats
          * accounting for this bond. */
-        if (!ctx->use_recirc) {
+        if (!use_recirc) {
             if (ctx->xin->resubmit_stats) {
                 bond_account(out_xbundle->bond, &ctx->xin->flow, vid,
                              ctx->xin->resubmit_stats->n_bytes);
@@ -1738,7 +1746,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
     }
     *flow_tci = tci;
 
-    compose_output_action(ctx, xport->ofp_port);
+    compose_output_action(ctx, xport->ofp_port, use_recirc ? &xr : NULL);
     *flow_tci = old_tci;
 }
 
@@ -2673,7 +2681,7 @@ build_tunnel_send(const struct xlate_ctx *ctx, const struct xport *xport,
 
 static void
 compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
-                        bool check_stp)
+                        const struct xlate_bond_recirc *xr, bool check_stp)
 {
     const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
     struct flow_wildcards *wc = &ctx->xout->wc;
@@ -2731,6 +2739,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     if (xport->peer) {
         const struct xport *peer = xport->peer;
         struct flow old_flow = ctx->xin->flow;
+        bool old_was_mpls = ctx->was_mpls;
         enum slow_path_reason special;
         uint8_t table_id = rule_dpif_lookup_get_init_table_id(&ctx->xin->flow);
         struct ofpbuf old_stack = ctx->stack;
@@ -2781,6 +2790,10 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         ofpbuf_uninit(&ctx->stack);
         ctx->stack = old_stack;
 
+        /* The peer bridge popping MPLS should have no effect on the original
+         * bridge. */
+        ctx->was_mpls = old_was_mpls;
+
         /* The fact that the peer bridge exits (for any reason) does not mean
          * that the original bridge should exit.  Specifically, if the peer
          * bridge recirculates (which typically modifies the packet), the
@@ -2873,9 +2886,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
                                               wc,
                                               ctx->xbridge->masked_set_action);
 
-        if (ctx->use_recirc) {
+        if (xr) {
             struct ovs_action_hash *act_hash;
-            struct xlate_recirc *xr = &ctx->recirc;
 
             /* Hash action. */
             act_hash = nl_msg_put_unspec_uninit(ctx->xout->odp_actions,
@@ -2931,9 +2943,10 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
 }
 
 static void
-compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port)
+compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port,
+                      const struct xlate_bond_recirc *xr)
 {
-    compose_output_action__(ctx, ofp_port, true);
+    compose_output_action__(ctx, ofp_port, xr, true);
 }
 
 static void
@@ -3223,9 +3236,9 @@ flood_packets(struct xlate_ctx *ctx, bool all)
         }
 
         if (all) {
-            compose_output_action__(ctx, xport->ofp_port, false);
+            compose_output_action__(ctx, xport->ofp_port, NULL, false);
         } else if (!(xport->config & OFPUTIL_PC_NO_FLOOD)) {
-            compose_output_action(ctx, xport->ofp_port);
+            compose_output_action(ctx, xport->ofp_port, NULL);
         }
     }
 
@@ -3492,7 +3505,7 @@ xlate_output_action(struct xlate_ctx *ctx,
 
     switch (port) {
     case OFPP_IN_PORT:
-        compose_output_action(ctx, ctx->xin->flow.in_port.ofp_port);
+        compose_output_action(ctx, ctx->xin->flow.in_port.ofp_port, NULL);
         break;
     case OFPP_TABLE:
         xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
@@ -3519,7 +3532,7 @@ xlate_output_action(struct xlate_ctx *ctx,
     case OFPP_LOCAL:
     default:
         if (port != ctx->xin->flow.in_port.ofp_port) {
-            compose_output_action(ctx, port);
+            compose_output_action(ctx, port, NULL);
         } else {
             xlate_report(ctx, "skipping output to input port");
         }
@@ -3578,7 +3591,7 @@ xlate_enqueue_action(struct xlate_ctx *ctx,
     /* Add datapath actions. */
     flow_priority = ctx->xin->flow.skb_priority;
     ctx->xin->flow.skb_priority = priority;
-    compose_output_action(ctx, ofp_port);
+    compose_output_action(ctx, ofp_port, NULL);
     ctx->xin->flow.skb_priority = flow_priority;
 
     /* Update NetFlow output port. */
@@ -4494,7 +4507,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     ctx.table_id = 0;
     ctx.rule_cookie = OVS_BE64_MAX;
     ctx.exit = false;
-    ctx.use_recirc = false;
     ctx.was_mpls = false;
 
     if (!xin->ofpacts && !ctx.rule) {
@@ -4596,7 +4608,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         if (ctx.xbridge->has_in_band
             && in_band_must_output_to_local_port(flow)
             && !actions_output_to_local_port(&ctx)) {
-            compose_output_action(&ctx, OFPP_LOCAL);
+            compose_output_action(&ctx, OFPP_LOCAL, NULL);
         }
 
         fix_sflow_action(&ctx);
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index a53fa8e..3e596fb 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -36,12 +36,6 @@ struct mac_learning;
 struct mcast_snooping;
 struct xlate_cache;
 
-struct xlate_recirc {
-    uint32_t recirc_id;  /* !0 Use recirculation instead of output. */
-    uint8_t  hash_alg;   /* !0 Compute hash for recirc before. */
-    uint32_t hash_basis;  /* Compute hash for recirc before. */
-};
-
 struct xlate_out {
     /* Wildcards relevant in translation.  Any fields that were used to
      * calculate the action must be set for caching and kernel
-- 
1.7.10.4




More information about the dev mailing list