[ovs-dev] [PATCH branch-2.8 1/2] xlate: Correct handling of double encap() actions

Jan Scheurich jan.scheurich at ericsson.com
Mon Apr 16 12:32:12 UTC 2018


When the same encap() header was pushed twice onto a packet (e.g in the
case of NSH in NSH), the translation logic only generated a datapath push
action for the first encap() action. The second encap() did not emit a
push action because the packet type was unchanged.

commit_encap_decap_action() (renamed from commit_packet_type_change) must
solely rely on ctx->pending_encap to generate an datapath push action.

Similarly, the first decap() action on a double header packet does not
change the packet_type either. Add a corresponding ctx->pending_decap
flag and use that to trigger emitting a datapath pop action.

Fixes: f839892a2 ("OF support and translation of generic encap and decap")
Fixes: 1fc11c594 ("Generic encap and decap support for NSH")

Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>
---
 lib/odp-util.c               | 16 ++++++----------
 lib/odp-util.h               |  1 +
 ofproto/ofproto-dpif-xlate.c |  7 ++++++-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 7f42b98..78cc903 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6883,17 +6883,13 @@ odp_put_encap_nsh_action(struct ofpbuf *odp_actions,
 }
 
 static void
-commit_packet_type_change(const struct flow *flow,
+commit_encap_decap_action(const struct flow *flow,
                           struct flow *base_flow,
                           struct ofpbuf *odp_actions,
                           struct flow_wildcards *wc,
-                          bool pending_encap,
+                          bool pending_encap, bool pending_decap,
                           struct ofpbuf *encap_data)
 {
-    if (flow->packet_type == base_flow->packet_type) {
-        return;
-    }
-
     if (pending_encap) {
         switch (ntohl(flow->packet_type)) {
         case PT_ETH: {
@@ -6918,7 +6914,7 @@ commit_packet_type_change(const struct flow *flow,
              * The check is done at action translation. */
             OVS_NOT_REACHED();
         }
-    } else {
+    } else if (pending_decap || flow->packet_type != base_flow->packet_type) {
         /* This is an explicit or implicit decap case. */
         if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE &&
             base_flow->packet_type == htonl(PT_ETH)) {
@@ -6957,14 +6953,14 @@ commit_packet_type_change(const struct flow *flow,
 enum slow_path_reason
 commit_odp_actions(const struct flow *flow, struct flow *base,
                    struct ofpbuf *odp_actions, struct flow_wildcards *wc,
-                   bool use_masked, bool pending_encap,
+                   bool use_masked, bool pending_encap, bool pending_decap,
                    struct ofpbuf *encap_data)
 {
     enum slow_path_reason slow1, slow2;
     bool mpls_done = false;
 
-    commit_packet_type_change(flow, base, odp_actions, wc,
-                              pending_encap, encap_data);
+    commit_encap_decap_action(flow, base, odp_actions, wc,
+                              pending_encap, pending_decap, encap_data);
     commit_set_ether_action(flow, base, odp_actions, wc, use_masked);
     /* Make packet a non-MPLS packet before committing L3/4 actions,
      * which would otherwise do nothing. */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 27c2ab4..9d6cc45 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -278,6 +278,7 @@ enum slow_path_reason commit_odp_actions(const struct flow *,
                                          struct flow_wildcards *wc,
                                          bool use_masked,
                                          bool pending_encap,
+                                         bool pending_decap,
                                          struct ofpbuf *encap_data);
 
 /* ofproto-dpif interface.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 3890b2e..54fd06c 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -241,6 +241,8 @@ struct xlate_ctx {
                                  * true. */
     bool pending_encap;         /* True when waiting to commit a pending
                                  * encap action. */
+    bool pending_decap;         /* True when waiting to commit a pending
+                                 * decap action. */
     struct ofpbuf *encap_data;  /* May contain a pointer to an ofpbuf with
                                  * context for the datapath encap action.*/
 
@@ -3537,8 +3539,9 @@ xlate_commit_actions(struct xlate_ctx *ctx)
     ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
                                           ctx->odp_actions, ctx->wc,
                                           use_masked, ctx->pending_encap,
-                                          ctx->encap_data);
+                                          ctx->pending_decap, ctx->encap_data);
     ctx->pending_encap = false;
+    ctx->pending_decap = false;
     ofpbuf_delete(ctx->encap_data);
     ctx->encap_data = NULL;
 }
@@ -6067,6 +6070,7 @@ xlate_generic_decap_action(struct xlate_ctx *ctx,
                 break;
             }
             ctx->wc->masks.nsh.np = UINT8_MAX;
+            ctx->pending_decap = true;
             /* Trigger recirculation. */
             return true;
         default:
@@ -6922,6 +6926,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         .in_action_set = false,
         .in_packet_out = xin->in_packet_out,
         .pending_encap = false,
+        .pending_decap = false,
         .encap_data = NULL,
 
         .table_id = 0,
-- 
1.9.1



More information about the dev mailing list