[ovs-dev] [PATCH v3] Remove mpls_depth field from flow
Simon Horman
horms at verge.net.au
Thu Sep 26 21:55:19 UTC 2013
Rather than tracking the MPLS depth as a field in the
flow, which is an entirely poor place for it, just track
the delta to the MPLS depth during translation.
This logic was developed while implementing recirculation
and intended to be used to detect when recirculation should
occur. This variant of the patch uses the logic to determine
if processing of actions should stop due to an MPLS
action which cannot be translated (without recirculation).
A side-effect of this patch is that it resolves a bug
whereby ovs-vswitchd will abort due to to an assertion
on eth_type_mpls(ctx->xin->flow.dl_type) in compose_mpls_pop_action(()
if the actions of a flow include pop_mpls twice without
a push_mpls in between.
Signed-off-by: Simon Horman <horms at verge.net.au>
---
v3
* As suggested by Ben Pfaff
- Remove empty zeros field from struct flow
v2
* As suggested by Ben Pfaff
- Move pre_push_mpls_lse fron a local variable of do_xlate_actions()
to a field of struct xlate_ctx so that it is accessible across resubmit
and goto table actions.
- Move removal of eth_mpls_depth() into a separate patch.
It is not strictly related to this change.
- Correct spelling errors
---
lib/flow.c | 7 +--
lib/flow.h | 8 ++-
lib/match.c | 2 +-
lib/nx-match.c | 2 +-
lib/odp-util.c | 48 ++++++++----------
lib/odp-util.h | 4 +-
lib/ofp-util.c | 3 +-
ofproto/ofproto-dpif-xlate.c | 114 ++++++++++++++++++++++++++++++++++---------
tests/ofproto-dpif.at | 22 +++++++++
tests/test-bundle.c | 2 -
tests/test-multipath.c | 2 -
utilities/ovs-dpctl.c | 2 +-
utilities/ovs-ofctl.8.in | 16 +++---
13 files changed, 152 insertions(+), 80 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 9ab1961..0ce694d 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -103,9 +103,11 @@ static void
parse_mpls(struct ofpbuf *b, struct flow *flow)
{
struct mpls_hdr *mh;
+ bool top = true;
while ((mh = ofpbuf_try_pull(b, sizeof *mh))) {
- if (flow->mpls_depth++ == 0) {
+ if (top) {
+ top = false;
flow->mpls_lse = mh->mpls_lse;
}
if (mh->mpls_lse & htonl(MPLS_BOS_MASK)) {
@@ -514,7 +516,7 @@ flow_zero_wildcards(struct flow *flow, const struct flow_wildcards *wildcards)
void
flow_get_metadata(const struct flow *flow, struct flow_metadata *fmd)
{
- BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20);
+ BUILD_ASSERT_DECL(FLOW_WC_SEQ == 21);
fmd->tun_id = flow->tunnel.tun_id;
fmd->tun_src = flow->tunnel.ip_src;
@@ -609,7 +611,6 @@ void
flow_wildcards_init_exact(struct flow_wildcards *wc)
{
memset(&wc->masks, 0xff, sizeof wc->masks);
- memset(wc->masks.zeros, 0, sizeof wc->masks.zeros);
}
/* Returns true if 'wc' matches every packet, false if 'wc' fixes any bits or
diff --git a/lib/flow.h b/lib/flow.h
index 75d95e8..8995b5f 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -36,7 +36,7 @@ struct ofpbuf;
/* This sequence number should be incremented whenever anything involving flows
* or the wildcarding of flows changes. This will cause build assertion
* failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 20
+#define FLOW_WC_SEQ 21
#define FLOW_N_REGS 8
BUILD_ASSERT_DECL(FLOW_N_REGS <= NXM_NX_MAX_REGS);
@@ -98,7 +98,6 @@ struct flow {
union flow_in_port in_port; /* Input port.*/
uint32_t pkt_mark; /* Packet mark. */
ovs_be32 mpls_lse; /* MPLS label stack entry. */
- uint16_t mpls_depth; /* Depth of MPLS stack. */
ovs_be16 vlan_tci; /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */
ovs_be16 dl_type; /* Ethernet frame type. */
ovs_be16 tp_src; /* TCP/UDP/SCTP source port. */
@@ -111,15 +110,14 @@ struct flow {
uint8_t arp_tha[6]; /* ARP/ND target hardware address. */
uint8_t nw_ttl; /* IP TTL/Hop Limit. */
uint8_t nw_frag; /* FLOW_FRAG_* flags. */
- uint8_t zeros[6];
};
BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
#define FLOW_U32S (sizeof(struct flow) / 4)
/* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
-BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 160 &&
- FLOW_WC_SEQ == 20);
+BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 152 &&
+ FLOW_WC_SEQ == 21);
/* Represents the metadata fields of struct flow. */
struct flow_metadata {
diff --git a/lib/match.c b/lib/match.c
index 03413fa..18fa596 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -835,7 +835,7 @@ match_format(const struct match *match, struct ds *s, unsigned int priority)
int i;
- BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20);
+ BUILD_ASSERT_DECL(FLOW_WC_SEQ == 21);
if (priority != OFP_DEFAULT_PRIORITY) {
ds_put_format(s, "priority=%u,", priority);
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 2d7ee34..8444ab7 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -570,7 +570,7 @@ nx_put_raw(struct ofpbuf *b, bool oxm, const struct match *match,
int match_len;
int i;
- BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20);
+ BUILD_ASSERT_DECL(FLOW_WC_SEQ == 21);
/* Metadata. */
if (match->wc.masks.in_port.ofp_port) {
diff --git a/lib/odp-util.c b/lib/odp-util.c
index aec4196..85256b7 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2503,9 +2503,7 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data,
arp_key->arp_op = htons(data->nw_proto);
memcpy(arp_key->arp_sha, data->arp_sha, ETH_ADDR_LEN);
memcpy(arp_key->arp_tha, data->arp_tha, ETH_ADDR_LEN);
- }
-
- if (flow->mpls_depth) {
+ } else if (eth_type_mpls(flow->dl_type)) {
struct ovs_key_mpls *mpls_key;
mpls_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_MPLS,
@@ -2798,7 +2796,6 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
return ODP_FIT_TOO_LITTLE;
}
flow->mpls_lse = nl_attr_get_be32(attrs[OVS_KEY_ATTR_MPLS]);
- flow->mpls_depth++;
} else if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS)) {
flow->mpls_lse = nl_attr_get_be32(attrs[OVS_KEY_ATTR_MPLS]);
@@ -2806,10 +2803,6 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
return ODP_FIT_ERROR;
}
expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_MPLS);
- if (flow->mpls_lse) {
- /* XXX Is this needed? */
- flow->mpls_depth = 0xffff;
- }
}
goto done;
} else if (src_flow->dl_type == htons(ETH_TYPE_IP)) {
@@ -3351,48 +3344,44 @@ commit_vlan_action(const struct flow *flow, struct flow *base,
static void
commit_mpls_action(const struct flow *flow, struct flow *base,
- struct ofpbuf *odp_actions, struct flow_wildcards *wc)
+ struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+ int *mpls_depth_delta)
{
- if (flow->mpls_lse == base->mpls_lse &&
- flow->mpls_depth == base->mpls_depth) {
+ if (flow->mpls_lse == base->mpls_lse && !*mpls_depth_delta) {
return;
}
memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
- if (flow->mpls_depth < base->mpls_depth) {
- if (base->mpls_depth - flow->mpls_depth > 1) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
- VLOG_WARN_RL(&rl, "Multiple mpls_pop actions reduced to "
- " a single mpls_pop action");
- }
-
+ switch (*mpls_depth_delta) {
+ case -1:
nl_msg_put_be16(odp_actions, OVS_ACTION_ATTR_POP_MPLS, flow->dl_type);
- } else if (flow->mpls_depth > base->mpls_depth) {
+ break;
+ case 1: {
struct ovs_action_push_mpls *mpls;
- if (flow->mpls_depth - base->mpls_depth > 1) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
- VLOG_WARN_RL(&rl, "Multiple mpls_push actions reduced to "
- " a single mpls_push action");
- }
-
mpls = nl_msg_put_unspec_uninit(odp_actions, OVS_ACTION_ATTR_PUSH_MPLS,
sizeof *mpls);
memset(mpls, 0, sizeof *mpls);
mpls->mpls_ethertype = flow->dl_type;
mpls->mpls_lse = flow->mpls_lse;
- } else {
+ break;
+ }
+ case 0: {
struct ovs_key_mpls mpls_key;
mpls_key.mpls_lse = flow->mpls_lse;
commit_set_action(odp_actions, OVS_KEY_ATTR_MPLS,
&mpls_key, sizeof(mpls_key));
+ break;
+ }
+ default:
+ NOT_REACHED();
}
base->dl_type = flow->dl_type;
base->mpls_lse = flow->mpls_lse;
- base->mpls_depth = flow->mpls_depth;
+ *mpls_depth_delta = 0;
}
static void
@@ -3563,7 +3552,8 @@ commit_set_pkt_mark_action(const struct flow *flow, struct flow *base,
* used as part of the action. */
void
commit_odp_actions(const struct flow *flow, struct flow *base,
- struct ofpbuf *odp_actions, struct flow_wildcards *wc)
+ struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+ int *mpls_depth_delta)
{
commit_set_ether_addr_action(flow, base, odp_actions, wc);
commit_vlan_action(flow, base, odp_actions, wc);
@@ -3573,7 +3563,7 @@ commit_odp_actions(const struct flow *flow, struct flow *base,
* actions. This is because committing MPLS actions may alter a packet so
* that it is no longer IP and thus nw and port actions are no longer valid.
*/
- commit_mpls_action(flow, base, odp_actions, wc);
+ commit_mpls_action(flow, base, odp_actions, wc, mpls_depth_delta);
commit_set_priority_action(flow, base, odp_actions, wc);
commit_set_pkt_mark_action(flow, base, odp_actions, wc);
}
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 192cfa0..4abf543 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -130,8 +130,8 @@ const char *odp_key_fitness_to_string(enum odp_key_fitness);
void commit_odp_tunnel_action(const struct flow *, struct flow *base,
struct ofpbuf *odp_actions);
void commit_odp_actions(const struct flow *, struct flow *base,
- struct ofpbuf *odp_actions,
- struct flow_wildcards *wc);
+ struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+ int *mpls_depth_delta);
/* ofproto-dpif interface.
*
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6a2bf5b..173b534 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -84,7 +84,7 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
void
ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
{
- BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20);
+ BUILD_ASSERT_DECL(FLOW_WC_SEQ == 21);
/* Initialize most of wc. */
flow_wildcards_init_catchall(wc);
@@ -4905,7 +4905,6 @@ ofputil_normalize_match__(struct match *match, bool may_log)
}
if (!(may_match & MAY_MPLS)) {
wc.masks.mpls_lse = htonl(0);
- wc.masks.mpls_depth = 0;
}
/* Log any changes. */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a5b6814..5482323 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -158,6 +158,13 @@ struct xlate_ctx {
/* The rule that we are currently translating, or NULL. */
struct rule_dpif *rule;
+ int mpls_depth_delta; /* Delta of the mpls stack depth since
+ * actions were last committed.
+ * Must be between -1 and 1 inclusive. */
+ ovs_be32 pre_push_mpls_lse; /* Used to record the top-most MPLS LSE
+ * prior to an mpls_push so that it may be
+ * used for a subsequent mpls_pop. */
+
int recurse; /* Recursion level, via xlate_table_action. */
uint32_t orig_skb_priority; /* Priority when packet arrived. */
uint8_t table_id; /* OpenFlow table ID where flow was found. */
@@ -1534,7 +1541,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
/* If 'struct flow' gets additional metadata, we'll need to zero it out
* before traversing a patch port. */
- BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20);
+ BUILD_ASSERT_DECL(FLOW_WC_SEQ == 21);
if (!xport) {
xlate_report(ctx, "Nonexistent output port");
@@ -1645,7 +1652,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
if (out_port != ODPP_NONE) {
commit_odp_actions(flow, &ctx->base_flow,
- &ctx->xout->odp_actions, &ctx->xout->wc);
+ &ctx->xout->odp_actions, &ctx->xout->wc,
+ &ctx->mpls_depth_delta);
nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
out_port);
@@ -1800,7 +1808,8 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
memset(&key.tunnel, 0, sizeof key.tunnel);
commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
- &ctx->xout->odp_actions, &ctx->xout->wc);
+ &ctx->xout->odp_actions, &ctx->xout->wc,
+ &ctx->mpls_depth_delta);
odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data,
ctx->xout->odp_actions.size, NULL, NULL);
@@ -1820,7 +1829,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, ovs_be16 eth_type)
{
struct flow_wildcards *wc = &ctx->xout->wc;
@@ -1828,12 +1837,35 @@ compose_mpls_push_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
ovs_assert(eth_type_mpls(eth_type));
+ /* If mpls_depth_delta is negative then an MPLS POP action has been
+ * composed and the resulting MPLS label stack is unknown. This means
+ * an MPLS PUSH action can't be composed as it needs to know either the
+ * top-most MPLS LSE to use as a template for the new MPLS LSE, or that
+ * there is no MPLS label stack present. Thus, stop processing.
+ *
+ * If mpls_depth_delta is positive then an MPLS PUSH action has been
+ * composed and no further MPLS PUSH action may be performed without
+ * losing MPLS LSE and ether type information held in xtx->xin->flow.
+ * Thus, stop processing.
+ *
+ * If the MPLS LSE of the flow and base_flow differ then the MPLS LSE
+ * has been updated. Performing a MPLS PUSH action may be would result in
+ * losing MPLS LSE and ether type information held in xtx->xin->flow.
+ * Thus, stop processing.
+ *
+ * It is planned that in the future this case will be handled
+ * by recirculation */
+ if (ctx->mpls_depth_delta ||
+ ctx->xin->flow.mpls_lse != ctx->base_flow.mpls_lse) {
+ return true;
+ }
+
memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
- memset(&wc->masks.mpls_depth, 0xff, sizeof wc->masks.mpls_depth);
- if (flow->mpls_depth) {
+ ctx->pre_push_mpls_lse = ctx->xin->flow.mpls_lse;
+
+ if (eth_type_mpls(ctx->xin->flow.dl_type)) {
flow->mpls_lse &= ~htonl(MPLS_BOS_MASK);
- flow->mpls_depth++;
} else {
ovs_be32 label;
uint8_t tc, ttl;
@@ -1848,30 +1880,48 @@ compose_mpls_push_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
tc = (flow->nw_tos & IP_DSCP_MASK) >> 2;
ttl = flow->nw_ttl ? flow->nw_ttl : 0x40;
flow->mpls_lse = set_mpls_lse_values(ttl, tc, 1, label);
- flow->mpls_depth = 1;
}
flow->dl_type = eth_type;
+ ctx->mpls_depth_delta++;
+
+ 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;
- ovs_assert(eth_type_mpls(ctx->xin->flow.dl_type));
- ovs_assert(!eth_type_mpls(eth_type));
+ if (!eth_type_mpls(ctx->xin->flow.dl_type)) {
+ return true;
+ }
+
+ /* If mpls_depth_delta is negative then an MPLS POP action has been
+ * composed. Performing another MPLS POP action
+ * would result in losing ether type that results from
+ * the already composed MPLS POP. Thus, stop processing.
+ *
+ * It is planned that in the future this case will be handled
+ * by recirculation */
+ if (ctx->mpls_depth_delta < 0) {
+ return true;
+ }
memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
- memset(&wc->masks.mpls_depth, 0xff, sizeof wc->masks.mpls_depth);
- if (flow->mpls_depth) {
- flow->mpls_depth--;
- flow->mpls_lse = htonl(0);
- if (!flow->mpls_depth) {
- flow->dl_type = eth_type;
- }
+ /* If mpls_depth_delta is positive then an MPLS PUSH action has been
+ * executed and the previous MPLS LSE saved in ctx->pre_push_mpls_lse. The
+ * flow's MPLS LSE should be restored to that value to allow any
+ * subsequent actions that update of the LSE to be executed correctly.
+ */
+ if (ctx->mpls_depth_delta > 0) {
+ ctx->xin->flow.mpls_lse = ctx->pre_push_mpls_lse;
}
+
+ ctx->xin->flow.dl_type = eth_type;
+ ctx->mpls_depth_delta--;
+
+ return false;
}
static bool
@@ -1907,6 +1957,18 @@ compose_set_mpls_ttl_action(struct xlate_ctx *ctx, uint8_t ttl)
return true;
}
+ /* If mpls_depth_delta is negative then an MPLS POP action has been
+ * executed and the resulting MPLS label stack is unknown. This means
+ * a SET MPLS TTL push action can't be executed as it needs to manipulate
+ * the top-most MPLS LSE. Thus, stop processing.
+ *
+ * It is planned that in the future this case will be handled
+ * by recirculation.
+ */
+ if (ctx->mpls_depth_delta < 0) {
+ return true;
+ }
+
ctx->xout->wc.masks.mpls_lse |= htonl(MPLS_TTL_MASK);
set_mpls_lse_ttl(&ctx->xin->flow.mpls_lse, ttl);
return false;
@@ -2134,7 +2196,8 @@ xlate_sample_action(struct xlate_ctx *ctx,
uint32_t probability = (os->probability << 16) | os->probability;
commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
- &ctx->xout->odp_actions, &ctx->xout->wc);
+ &ctx->xout->odp_actions, &ctx->xout->wc,
+ &ctx->mpls_depth_delta);
compose_flow_sample_cookie(os->probability, os->collector_set_id,
os->obs_domain_id, os->obs_point_id, &cookie);
@@ -2309,11 +2372,17 @@ 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)->ethertype);
+ if (compose_mpls_push_action(ctx,
+ ofpact_get_PUSH_MPLS(a)->ethertype)) {
+ 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_TTL:
@@ -2598,6 +2667,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
ctx.orig_skb_priority = flow->skb_priority;
ctx.table_id = 0;
ctx.exit = false;
+ ctx.mpls_depth_delta = 0;
if (xin->ofpacts) {
ofpacts = xin->ofpacts;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index f67c3ab..652304e 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -268,6 +268,7 @@ cookie=0x8 table=6 in_port=85 actions=mod_tp_src:85,controller,resubmit(86,7)
cookie=0x9 table=7 in_port=86 actions=mod_tp_dst:86,controller,controller
cookie=0xa dl_src=40:44:44:44:44:41 actions=mod_vlan_vid:99,mod_vlan_pcp:1,controller
cookie=0xa dl_src=40:44:44:44:44:42 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],controller
+cookie=0xa dl_src=41:44:44:44:44:42 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],pop_mpls:0x0800,controller
cookie=0xa dl_src=40:44:44:44:44:43 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],controller
cookie=0xa dl_src=40:44:44:44:44:44 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],controller
cookie=0xa dl_src=40:44:44:44:44:45 actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[[]],load:3->OXM_OF_MPLS_TC[[]],dec_mpls_ttl,controller
@@ -383,6 +384,26 @@ mpls,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=40:44:44:44:44:42,dl_dst=50:54:
dnl Modified MPLS controller action.
AT_CHECK([ovs-ofctl monitor br0 65534 -P nxm --detach --pidfile 2> ofctl_monitor.log])
+for i in 1 2 3; do
+ ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=41:44:44:44:44:42,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=60 in_port=1 (via action) data_len=60 (unbuffered)
+tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=41:44:44:44:44:42,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64 tcp_csum:0
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=60 in_port=1 (via action) data_len=60 (unbuffered)
+tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=41:44:44:44:44:42,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64 tcp_csum:0
+dnl
+NXT_PACKET_IN (xid=0x0): cookie=0xa total_len=60 in_port=1 (via action) data_len=60 (unbuffered)
+tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=41:44:44:44:44:42,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64 tcp_csum:0
+])
+
+dnl Modified MPLS controller action.
+AT_CHECK([ovs-ofctl monitor br0 65534 -P nxm --detach --pidfile 2> ofctl_monitor.log])
+
dnl in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x8847),mpls(label=100,tc=3,ttl=64,bos=1)
for i in 1 2 3; do
@@ -703,6 +724,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:46 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),CONTROLLER:65535
cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:47 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],dec_mpls_ttl,set_mpls_ttl(10),CONTROLLER:65535
cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:48 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),dec_mpls_ttl,CONTROLLER:65535
+ cookie=0xa, n_packets=3, n_bytes=180, dl_src=41:44:44:44:44:42 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],pop_mpls:0x0800,CONTROLLER:65535
cookie=0xb, n_packets=3, n_bytes=180, mpls,dl_src=50:55:55:55:55:55 actions=load:0x3e8->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535
cookie=0xc, n_packets=3, n_bytes=180, dl_src=70:77:77:77:77:77 actions=push_mpls:0x8848,load:0x3e8->OXM_OF_MPLS_LABEL[[]],load:0x7->OXM_OF_MPLS_TC[[]],CONTROLLER:65535
cookie=0xd, n_packets=3, n_bytes=186, dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,CONTROLLER:65535
diff --git a/tests/test-bundle.c b/tests/test-bundle.c
index 41e2e38..1bb2b0b 100644
--- a/tests/test-bundle.c
+++ b/tests/test-bundle.c
@@ -141,8 +141,6 @@ main(int argc, char *argv[])
flows = xmalloc(N_FLOWS * sizeof *flows);
for (i = 0; i < N_FLOWS; i++) {
random_bytes(&flows[i], sizeof flows[i]);
- memset(flows[i].zeros, 0, sizeof flows[i].zeros);
- flows[i].mpls_depth = 0;
flows[i].regs[0] = ofp_to_u16(OFPP_NONE);
}
diff --git a/tests/test-multipath.c b/tests/test-multipath.c
index f1b12e2..4ba3692 100644
--- a/tests/test-multipath.c
+++ b/tests/test-multipath.c
@@ -66,8 +66,6 @@ main(int argc, char *argv[])
struct flow flow;
random_bytes(&flow, sizeof flow);
- memset(flow.zeros, 0, sizeof flow.zeros);
- flow.mpls_depth = 0;
mp.max_link = n - 1;
multipath_execute(&mp, &flow, &wc);
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 98b47b8..9c98cda 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -1112,7 +1112,7 @@ dpctl_normalize_actions(int argc, char *argv[])
printf("no vlan: ");
}
- if (af->flow.mpls_depth) {
+ if (eth_type_mpls(af->flow.dl_type)) {
printf("mpls(label=%"PRIu32",tc=%d,ttl=%d): ",
mpls_lse_to_label(af->flow.mpls_lse),
mpls_lse_to_tc(af->flow.mpls_lse),
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 526e12c..dcd3a36 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1068,21 +1068,17 @@ from the IP TTL (64 if the packet is not IP).
If the packet does already contain an MPLS label, pushes a new
outermost label as a copy of the existing outermost label.
.IP
-There are some limitations in the implementation. \fBpush_mpls\fR
-followed by another \fBpush_mpls\fR will result in the first
-\fBpush_mpls\fR being discarded.
+A limitation of the implementation is that processing of actions will stop
+if \fBpush_mpls\fR follows another \fBpush_mpls\fR unless there is a
+\fBpop_mpls\fR in between.
.
.IP \fBpop_mpls\fR:\fIethertype\fR
Strips the outermost MPLS label stack entry.
Currently the implementation restricts \fIethertype\fR to a non-MPLS Ethertype
and thus \fBpop_mpls\fR should only be applied to packets with
-an MPLS label stack depth of one.
-.
-.IP
-There are some limitations in the implementation. \fBpop_mpls\fR
-followed by another \fBpush_mpls\fR without an intermediate
-\fBpush_mpls\fR will result in the first \fBpush_mpls\fR being
-discarded.
+an MPLS label stack depth of one. A further limitation is that processing of
+actions will stop if \fBpop_mpls\fR follows another \fBpop_mpls\fR unless
+there is a \fBpush_mpls\fR in between.
.
.IP \fBmod_dl_src\fB:\fImac\fR
Sets the source Ethernet address to \fImac\fR.
--
1.8.4
More information about the dev
mailing list