'ctx->was_mpls' is used to flag when an MPLS packet has been popped to
a non-MPLS packet, but it was not set when the MPLS POP is implicit
due to the 'ctx->xin->flow' being restored after a patch port
traversal to group bucket execution.

If MPLS push on a non-MPLS packet is followed by an MPLS POP in the
same actions list, a recirculation after the MPLS POP would not be
necessary, as the parsed L3/4 fields are still available.  Even so,
the current action validation and execution code in the Linux kernel
datapath implementation would need to be enhanced to support this
case.  For now we trigger recirculation instead.

Reported-by: Thomas Morin <thomas.mo...@orange.com>
Suggested-by: Takashi YAMAMOTO <yamam...@ovn.org>
Fixes: 742c0ac3c0ab ("mpls: Fix MPLS restoration after patch port and group 
bucket.")
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 371ced4..27b64d9 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3053,9 +3053,15 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
          * metadata table. */
         ctx->wc->masks.tunnel = old_flow_tnl_wc;
 
-        /* The peer bridge popping MPLS should have no effect on the original
-         * bridge. */
-        ctx->was_mpls = old_was_mpls;
+        /* An MPLS packet can be implicitly popped back to a non-MPLS packet
+         * when the flow key is restored, if the patch port peer pushed MPLS to
+         * a non-MPLS packet.  In this case the now restored flow key is of
+         * non-MPLS type, while the base flow is of an MPLS type.  Set the
+         * 'was_mpls' flag also in that case. */
+        ctx->was_mpls = old_was_mpls ||
+            (eth_type_mpls(ctx->base_flow.dl_type)
+             && !eth_type_mpls(ctx->xin->flow.dl_type)
+             && ctx->xbridge->support.odp.recirc);
 
         /* The peer bridge's conntrack execution should have no effect on the
          * original bridge. */
@@ -3379,9 +3385,15 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct 
ofputil_bucket *bucket)
      * group buckets. */
     ctx->xin->flow = old_flow;
 
-    /* The group bucket popping MPLS should have no effect after bucket
-     * execution. */
-    ctx->was_mpls = old_was_mpls;
+    /* An MPLS packet can be implicitly popped back to a non-MPLS packet
+     * when the flow key is restored, if the group bucket pushed MPLS to
+     * a non-MPLS packet.  In this case the now restored flow key is of
+     * non-MPLS type, while the base flow is of an MPLS type.  Set the
+     * 'was_mpls' flag also in that case. */
+    ctx->was_mpls = old_was_mpls ||
+        (eth_type_mpls(ctx->base_flow.dl_type)
+         && !eth_type_mpls(ctx->xin->flow.dl_type)
+         && ctx->xbridge->support.odp.recirc);
 
     /* The fact that the group bucket exits (for any reason) does not mean that
      * the translation after the group action should exit.  Specifically, if
-- 
2.1.4

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to