Always insert MPLS labels after VLAN tags.

OpenFlow 1.1 and 1.2 always inserted MPLS labels after VLAN tags.
OpenFlow 1.3 and 1.4 insert MPLS labels before VLAN tags.
OpenFlow 1.3.4 and 1.5, both in preparation, recognize that the change in
1.3 was an error and revert it.  This commit implements that reversion
in Open vSwitch.

EXT-457.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Simon Horman <horms@verge.net.au>

diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index 5137c2f..d1ff5ec 100644 (file)
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007-2013 Nicira, Inc.
+ * Copyright (c) 2007-2014 Nicira, Inc.
  *
  * This file is offered under your choice of two licenses: Apache 2.0 or GNU
  * GPL 2.0 or later.  The permission statements for each of these licenses is
@@ -545,13 +545,13 @@ struct ovs_action_push_vlan {
  * single nested %OVS_KEY_ATTR_* attribute specifies a header to modify and its
  * value.
  * @OVS_ACTION_ATTR_PUSH_MPLS: Push a new MPLS label stack entry onto the
- * top of the packets MPLS label stack. Set the ethertype of the
+ * top of the packets MPLS label stack.  Set the ethertype of the
  * encapsulating frame to either %ETH_P_MPLS_UC or %ETH_P_MPLS_MC to
  * indicate the new packet contents.
  * @OVS_ACTION_ATTR_POP_MPLS: Pop an MPLS label stack entry off of the
  * packet's MPLS label stack.  Set the encapsulating frame's ethertype to
- * indicate the new packet contents This could potentially still be
- * %ETH_P_MPLS_* if the resulting MPLS label stack is not empty.  If there
+ * indicate the new packet contents. This could potentially still be
+ * %ETH_P_MPLS if the resulting MPLS label stack is not empty.  If there
  * is no MPLS label stack, as determined by ethertype, no action is taken.
  *
  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all

diff --git a/lib/odp-util.c b/lib/odp-util.c
index b81d0d2..e20564f 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3548,10 +3548,6 @@ commit_mpls_action(const struct flow *flow, struct flow *base,
     while (base_n < flow_n) {
         struct ovs_action_push_mpls *mpls;
 
-        /* If there's a VLAN tag, pop it off so that our new MPLS label doesn't
-         * end up outside it. */
-        pop_vlan(base, odp_actions, wc);
-
         mpls = nl_msg_put_unspec_zero(odp_actions,
                                       OVS_ACTION_ATTR_PUSH_MPLS,
                                       sizeof *mpls);

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index df7aebd..781c3a1 100644 (file)
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -286,8 +286,7 @@ sample_from_openflow(const struct nx_action_sample *nas,
 }
 
 static enum ofperr
-push_mpls_from_openflow(ovs_be16 ethertype, enum ofpact_mpls_position position,
-                        struct ofpbuf *out)
+push_mpls_from_openflow(ovs_be16 ethertype, struct ofpbuf *out)
 {
     struct ofpact_push_mpls *oam;
 
@@ -296,7 +295,6 @@ push_mpls_from_openflow(ovs_be16 ethertype, enum ofpact_mpls_position position,
     }
     oam = ofpact_put_PUSH_MPLS(out);
     oam->ethertype = ethertype;
-    oam->position = position;
 
     return 0;
 }
@@ -473,8 +471,7 @@ ofpact_from_nxast(const union ofp_action *a, enum ofputil_action_code code,
         break;
 
     case OFPUTIL_NXAST_PUSH_MPLS:
-        error = push_mpls_from_openflow(a->push_mpls.ethertype,
-                                        OFPACT_MPLS_AFTER_VLAN, out);
+        error = push_mpls_from_openflow(a->push_mpls.ethertype, out);
         break;
 
     case OFPUTIL_NXAST_SET_MPLS_LABEL:
@@ -1255,11 +1252,7 @@ ofpact_from_openflow11(const union ofp_action *a, enum ofp_version version,
         break;
 
     case OFPUTIL_OFPAT11_PUSH_MPLS:
-        /* OpenFlow 1.3 has different semantics. */
-        error = push_mpls_from_openflow(a->push.ethertype,
-                                        version >= OFP13_VERSION ?
-                                        OFPACT_MPLS_BEFORE_VLAN :
-                                        OFPACT_MPLS_AFTER_VLAN, out);
+        error = push_mpls_from_openflow(a->push.ethertype, out);
         break;
 
     case OFPUTIL_OFPAT11_POP_MPLS:

diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 00cba6a..0f6bf70 100644 (file)
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -363,19 +363,6 @@ struct ofpact_reg_load {
     union mf_subvalue subvalue; /* Least-significant bits are used. */
 };
 
-/* The position in the packet at which to insert an MPLS header.
- *
- * Used NXAST_PUSH_MPLS, OFPAT11_PUSH_MPLS. */
-enum ofpact_mpls_position {
-    /* Add the MPLS LSE after the Ethernet header but before any VLAN tags.
-     * OpenFlow 1.3+ requires this behavior. */
-   OFPACT_MPLS_BEFORE_VLAN,
-
-   /* Add the MPLS LSE after the Ethernet header and any VLAN tags.
-    * OpenFlow 1.1 and 1.2 require this behavior. */
-   OFPACT_MPLS_AFTER_VLAN
-};
-
 /* OFPACT_SET_FIELD.
  *
  * Used for OFPAT12_SET_FIELD. */
@@ -392,7 +379,6 @@ struct ofpact_set_field {
 struct ofpact_push_mpls {
     struct ofpact ofpact;
     ovs_be16 ethertype;
-    enum ofpact_mpls_position position;
 };
 
 /* OFPACT_POP_MPLS

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ad44582..a880376 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2103,18 +2103,12 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
 {
     struct flow_wildcards *wc = &ctx->xout->wc;
     struct flow *flow = &ctx->xin->flow;
-    ovs_be16 vlan_tci = flow->vlan_tci;
     int n;
 
     ovs_assert(eth_type_mpls(mpls->ethertype));
 
     n = flow_count_mpls_labels(flow, wc);
     if (!n) {
-        if (mpls->position == OFPACT_MPLS_BEFORE_VLAN) {
-            vlan_tci = 0;
-        } else {
-            flow->vlan_tci = 0;
-        }
         ctx->xout->slow |= commit_odp_actions(flow, &ctx->base_flow,
                                               &ctx->xout->odp_actions,
                                               &ctx->xout->wc);
@@ -2134,7 +2128,6 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
     }
 
     flow_push_mpls(flow, n, mpls->ethertype, wc);
-    flow->vlan_tci = vlan_tci;
 }
 
 static void