The patch "Implement OpenFlow support for MPLS, for up to 3 labels"
allows ovs-vswtichd to fully represent an MPLS label stack containing
up to three labels in a flow.
MPLS push and pop actions are not permitted by that patch in the case where
there are more than three labels present or more than three labels would be
present as the result of an MPLS push action as it is not possible to
accurately describe changes to the MPLS label stack using flows.
This patch alters the behaviour in such situations to drop the packet
rather than skipping to the next action. This avoids the danger of
subsequent actions making invalid assumptions about the packet as a result
of the skipped actions.
As suggested by Jesse Gross.
Cc: Jesse Gross <je...@nicira.com>
Signed-off-by: Simon Horman <ho...@verge.net.au>
---
v2
* Set ctx->exit
* Clear odp_actions in compose_mpls_pop_action()
to be consistent with compose_mpls_push_action()
This is an incremental patch for "Implement OpenFlow support for MPLS, for
up to 3 labels" intended to further discussion of an issue raised by Jesse
Gross. I am happy for it to be squashed into "Implement OpenFlow support
for MPLS, for up to 3 labels" if appropriate.
---
ofproto/ofproto-dpif-xlate.c | 43 ++++++++++++++++++++++++++++++++++------
tests/ofproto-dpif.at | 47 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b56a746..352b443 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2098,7 +2098,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
ofpbuf_delete(packet);
}
-static void
+static bool
compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls)
{
struct flow_wildcards *wc = &ctx->xout->wc;
@@ -2119,20 +2119,46 @@ compose_mpls_push_action(struct xlate_ctx *ctx, struct
ofpact_push_mpls *mpls)
&ctx->xout->odp_actions,
&ctx->xout->wc);
} else if (n >= ARRAY_SIZE(flow->mpls_lse)) {
- return;
+ if (ctx->xin->packet != NULL) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_WARN_RL(&rl, "bridge %s: dropping packet which after an "
+ "MPLS push action will have more MPLS LSEs than "
+ "the %"PRIuSIZE" that can be handled.",
+ ctx->xbridge->name, ARRAY_SIZE(flow->mpls_lse) - 1);
+ }
+ ctx->exit = true;
+ ofpbuf_clear(&ctx->xout->odp_actions);
+ return true;
}
flow_push_mpls(flow, mpls->ethertype, wc);
flow->vlan_tci = vlan_tci;
+
+ return false;
}
-static void
+static bool
compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
{
struct flow_wildcards *wc = &ctx->xout->wc;
struct flow *flow = &ctx->xin->flow;
- flow_pop_mpls(flow, eth_type, wc);
+ if (!flow_pop_mpls(flow, eth_type, wc) &&
+ flow_count_mpls_labels(flow) >= ARRAY_SIZE(flow->mpls_lse)) {
+ if (ctx->xin->packet != NULL) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_WARN_RL(&rl, "bridge %s: dropping packet on which an "
+ "MPLS pop action can't be performed as it has "
+ "have more MPLS LSEs than the %"PRIuSIZE" "
+ "that can be handled.",
+ ctx->xbridge->name, ARRAY_SIZE(flow->mpls_lse) - 1);
+ }
+ ctx->exit = true;
+ ofpbuf_clear(&ctx->xout->odp_actions);
+ return true;
+ }
+
+ return false;
}
static bool
@@ -2644,11 +2670,16 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
ofpacts_len,
break;
case OFPACT_PUSH_MPLS:
- compose_mpls_push_action(ctx, ofpact_get_PUSH_MPLS(a));
+ if (compose_mpls_push_action(ctx, ofpact_get_PUSH_MPLS(a))) {
+ return;
+ }
break;
case OFPACT_POP_MPLS:
- compose_mpls_pop_action(ctx, ofpact_get_POP_MPLS(a)->ethertype);
+ if (compose_mpls_pop_action(ctx,
+ ofpact_get_POP_MPLS(a)->ethertype)) {
+ return;
+ }
break;
case OFPACT_SET_MPLS_LABEL:
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index de6397a..9eaaaa3 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2380,6 +2380,53 @@
skb_priority(0),skb_mark(0/0),in_port(p3),eth(src=50:54:00:00:00:09/00:00:00:00:
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - ovs-appctl dpif/dump-flows - MPLS actions that result
in a drop])
+OVS_VSWITCHD_START([dnl
+ add-port br0 p1 -- set Interface p1 type=dummy
+])
+ON_EXIT([kill `cat ovs-ofctl.pid`])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_DATA([flows.txt], [dnl
+dl_src=60:66:66:66:66:00 actions=push_mpls:0x8847,controller
+dl_src=60:66:66:66:66:01 actions=pop_mpls:0x8849,controller
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+dnl Packet is dropped because an MPLS POP action is applied to a packet with
+dnl 4 MPLS LSEs but ovs-vswtichd can only handle up to 3 MPLS LSEs and thus
+dnl can't determine the resulting MPLS label after an MPLS POP action.
+dnl
+dnl The input is a frame with two MPLS headers which tcpdump -vve shows as:
+dnl 60:66:66:66:66:00 > 50:54:00:00:00:07, ethertype MPLS unicast (0x8847),
length 74: MPLS (label 20, exp 0, ttl 32)
+dnl (label 20, exp 0, ttl 32)
+dnl (label 20, exp 0, ttl 32)
+dnl (label 20, exp 0, [S], ttl 32)
+dnl (tos 0x0, ttl 64, id 0, offset 0, flags [none], proto TCP (6),
length 44, bad cksum 3b78 (->f978)!)
+dnl 192.168.0.1.80 > 192.168.0.2.0: Flags [none], cksum 0x7744 (correct),
seq 42:46, win 10000, length 4
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 66
00 88 47 00 01 40 20 00 01 40 20 00 01 40 20 00 01 41 20 45 00 00 2c 00 00 00
00 40 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50
00 27 10 77 44 00 00 48 4f 47 45'])
+
+dnl Packet is dropped because an MPLS PUSH action is applied to a packet with
+dnl 4 MPLS LSEs but ovs-vswtichd can only handle up to 3 MPLS LSEs and thus
+dnl can't determine the resulting MPLS label after an MPLS PUSH action.
+dnl
+dnl The input is a frame with two MPLS headers which tcpdump -vve shows as:
+dnl 60:66:66:66:66:01 > 50:54:00:00:00:07, ethertype MPLS unicast (0x8847),
length 74: MPLS (label 20, exp 0, ttl 32)
+dnl (label 20, exp 0, ttl 32)
+dnl (label 20, exp 0, ttl 32)
+dnl (label 20, exp 0, [S], ttl 32)
+dnl (tos 0x0, ttl 64, id 0, offset 0, flags [none], proto TCP (6),
length 44, bad cksum 3b78 (->f978)!)
+dnl 192.168.0.1.80 > 192.168.0.2.0: Flags [none], cksum 0x7744 (correct),
seq 42:46, win 10000, length 4
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 66
01 88 47 00 01 40 20 00 01 40 20 00 01 40 20 00 01 41 20 45 00 00 2c 00 00 00
00 40 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50
00 27 10 77 44 00 00 48 4f 47 45'])
+
+AT_CHECK([ovs-appctl dpif/dump-flows br0 | sort | STRIP_USED], [0], [dnl
+skb_priority(0),in_port(1),eth(src=60:66:66:66:66:00/ff:ff:ff:ff:ff:ff,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x8847),mpls(lse0=0x140200,lse1=0x140200,lse2=0x140200),
packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),in_port(1),eth(src=60:66:66:66:66:01/ff:ff:ff:ff:ff:ff,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x8847),mpls(lse0=0x140200,lse1=0x140200,lse2=0x140200),
packets:0, bytes:0, used:never, actions:drop
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - patch ports])
OVS_VSWITCHD_START([add-br br1 \
-- set bridge br1 datapath-type=dummy fail-mode=secure \
--
1.8.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
