From 269dc90a0a090f2b40e55b4572ac5edeb1684b7d Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 4 Feb 2014 12:39:37 -0800 Subject: [PATCH] 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 Acked-by: Simon Horman --- include/linux/openvswitch.h | 8 ++++---- lib/odp-util.c | 4 ---- lib/ofp-actions.c | 15 ++++----------- lib/ofp-actions.h | 16 +--------------- ofproto/ofproto-dpif-xlate.c | 7 ------- 5 files changed, 9 insertions(+), 41 deletions(-) diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h index 5137c2f59..d1ff5ec7b 100644 --- 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 b81d0d215..e20564f29 100644 --- 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 df7aebd09..781c3a1ca 100644 --- 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 00cba6ad1..0f6bf7095 100644 --- 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 ad4458240..a880376fb 100644 --- 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 -- 2.43.0