From 94baabf754dde243e4cf5b7d0bfd9ca8def114ab Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Fri, 15 Nov 2013 18:10:18 +0900 Subject: [PATCH] ofp-actions: Consider L4 actions after mpls_push as inconsistent After an mpls_push action the resulting packet is MPLS and the MPLS payload is opaque. Thus nothing can be assumed about the packets network protocol and it is inconsistent to apply L4 actions. With regards to actions that affect the packet at other layers of the protocol stack: * L3: The consistency of L3 actions should already be handled correctly by virtue of the dl_type of the flow being temporarily altered during consistency checking by both push_mpls and pop_mpls actions. * MPLS: The consistency checking of MPLS actions appear to already be handled correctly. * VLAN: At this time Open vSwitch on mpls_push an MPLS LSE is always added after any VLAN tags that follow the ethernet header. That is the tag ordering defined prior to OpenFlow1.3. As such VLAN actions should sill be equally valid before and after mpls_push and mpls_pop actions. * L2 actions are equally valid before and after mpls_push and mpls_pop actions. Acked-by: Jarno Rajahalme Signed-off-by: Simon Horman Signed-off-by: Ben Pfaff --- lib/ofp-actions.c | 9 ++++++++- tests/ofp-actions.at | 12 ++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 91cee6141..45586690b 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -1867,7 +1867,8 @@ ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports) } } -/* May modify flow->dl_type and flow->vlan_tci, caller must restore them. +/* May modify flow->dl_type, flow->nw_proto and flow->vlan_tci, + * caller must restore them. * * Modifies some actions, filling in fields that could not be properly set * without context. */ @@ -2054,6 +2055,10 @@ ofpact_check__(struct ofpact *a, struct flow *flow, case OFPACT_PUSH_MPLS: flow->dl_type = ofpact_get_PUSH_MPLS(a)->ethertype; + /* The packet is now MPLS and the MPLS payload is opaque. + * Thus nothing can be assumed about the network protocol. + * Temporarily mark that we have no nw_proto. */ + flow->nw_proto = 0; return 0; case OFPACT_POP_MPLS: @@ -2125,6 +2130,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len, struct ofpact *a; ovs_be16 dl_type = flow->dl_type; ovs_be16 vlan_tci = flow->vlan_tci; + uint8_t nw_proto = flow->nw_proto; enum ofperr error = 0; OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { @@ -2137,6 +2143,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len, /* Restore fields that may have been modified. */ flow->dl_type = dl_type; flow->vlan_tci = vlan_tci; + flow->nw_proto = nw_proto; return error; } diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index cdca8ca1f..08ebccf51 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -477,3 +477,15 @@ AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-ofp11-instructions < input.txt], [0], [expout], [experr]) AT_CLEANUP + +AT_SETUP([ofp-actions - inconsistent MPLS actions]) +OVS_VSWITCHD_START +dnl OK: Use fin_timeout action on TCP flow +AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)']) +dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS +AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'], + [1], [], [dnl +ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT) +]) +OVS_VSWITCHD_STOP +AT_CLEANUP -- 2.20.1