On Wed, Jun 11, 2014 at 09:28:06AM +0900, Simon Horman wrote: > In some cases an pop MPLS action changes a packet to be a non-mpls packet. > In this case subsequent any L3+ actions require access to portions > of the packet which were not decoded as they were opaque when the > packet was MPLS. Allow such actions to be translated by > first recirculating the packet. > > Signed-off-by: Simon Horman <[email protected]>
Weaving the was_mpls checks into do_xlate_actions() makes the code harder to read. It also might be harder to maintain as we add new kinds of actions, since it adds something else that one must consider during translation, that is easy to forget about. Can we use a separate function to do these checks? Then it is nicely separated from the main translation code and the compiler will remind us when we add a new action. Here is my idea. Does it work? Will you review it? Thanks, Ben. --8<--------------------------cut here-------------------------->8-- From: Simon Horman <[email protected]> Date: Wed, 11 Jun 2014 09:28:06 +0900 Subject: [PATCH] ofproto-dpif: MPLS recirculation In some cases an pop MPLS action changes a packet to be a non-mpls packet. In this case subsequent any L3+ actions require access to portions of the packet which were not decoded as they were opaque when the packet was MPLS. Allow such actions to be translated by first recirculating the packet. Signed-off-by: Simon Horman <[email protected]> Co-authored-by: Ben Pfaff <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> --- ofproto/ofproto-dpif-xlate.c | 159 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 71eaad1..8f59085 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -61,6 +61,9 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate); #define MAX_INTERNAL_RESUBMITS 1 /* Max resbmits allowed using rules in internal table. */ +/* Timeout for internal rules created to handle recirculation */ +#define RECIRC_TIMEOUT 60 + /* Maximum number of resubmit actions in a flow translation, whether they are * recursive or not. */ #define MAX_RESUBMITS (MAX_RESUBMIT_RECURSION * MAX_RESUBMIT_RECURSION) @@ -194,6 +197,12 @@ struct xlate_ctx { struct xlate_recirc recirc; /* Information used for generating * recirculation actions */ + /* True if a packet was but is no longer MPLS (due to an MPLS pop action). + * This is a trigger for recirculation in cases where translating an action + * or looking up a flow requires access to the fields of the packet after + * the MPLS label stack that was originally present. */ + bool was_mpls; + /* OpenFlow 1.1+ action set. * * 'action_set' accumulates "struct ofpact"s added by OFPACT_WRITE_ACTIONS. @@ -2695,6 +2704,67 @@ execute_controller_action(struct xlate_ctx *ctx, int len, } static void +compose_recirculate_action(struct xlate_ctx *ctx, + const struct ofpact *ofpacts_base, + const struct ofpact *ofpact_current, + size_t ofpacts_base_len) +{ + uint32_t id; + int error; + unsigned ofpacts_len; + struct match match; + struct rule *rule; + struct ofpbuf ofpacts; + + ofpacts_len = ofpacts_base_len - + ((uint8_t *)ofpact_current - (uint8_t *)ofpacts_base); + + if (ctx->rule) { + id = rule_dpif_get_recirc_id(ctx->rule); + } else { + /* In the case where ctx has no rule then allocate a recirc id. + * The life-cycle of this recirc id is managed by associating it + * with the internal rule that is created to to handle + * recirculation below. + * + * The known use-case of this is packet_out which + * translates actions without a rule */ + id = ofproto_dpif_alloc_recirc_id(ctx->xbridge->ofproto); + } + if (!id) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_ERR_RL(&rl, "Failed to allocate recirculation id"); + ctx->exit = true; + return; + } + + match_init_catchall(&match); + match_set_recirc_id(&match, id); + ofpbuf_use_const(&ofpacts, ofpact_current, ofpacts_len); + error = ofproto_dpif_add_internal_flow(ctx->xbridge->ofproto, &match, + RECIRC_RULE_PRIORITY, + RECIRC_TIMEOUT, &ofpacts, &rule); + if (error) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_ERR_RL(&rl, "Failed to add post recirculation flow %s", + match_to_string(&match, 0)); + ctx->exit = true; + return; + } + /* If ctx has no rule then associate the recirc id, which + * was allocated above, with the internal rule. This allows + * the recirc id to be released when the internal rule times out. */ + if (!ctx->rule) { + rule_set_recirc_id(rule, id); + } + + ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow, + &ctx->xout->odp_actions, + &ctx->xout->wc); + nl_msg_put_u32(&ctx->xout->odp_actions, OVS_ACTION_ATTR_RECIRC, id); +} + +static void compose_mpls_push_action(struct xlate_ctx *ctx, struct ofpact_push_mpls *mpls) { struct flow_wildcards *wc = &ctx->xout->wc; @@ -2733,7 +2803,11 @@ compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type) struct flow *flow = &ctx->xin->flow; int n = flow_count_mpls_labels(flow, wc); - if (!flow_pop_mpls(flow, n, eth_type, wc) && n >= FLOW_MAX_MPLS_LABELS) { + if (flow_pop_mpls(flow, n, eth_type, wc)) { + if (ctx->xbridge->enable_recirc && !eth_type_mpls(eth_type)) { + ctx->was_mpls = true; + } + } else if (n >= FLOW_MAX_MPLS_LABELS) { 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 " @@ -3111,6 +3185,83 @@ xlate_action_set(struct xlate_ctx *ctx) ofpbuf_uninit(&action_list); } +static bool +ofpact_needs_recirculation_after_mpls(const struct xlate_ctx *ctx, + const struct ofpact *a) +{ + struct flow_wildcards *wc = &ctx->xout->wc; + struct flow *flow = &ctx->xin->flow; + + switch (a->type) { + case OFPACT_OUTPUT: + case OFPACT_GROUP: + case OFPACT_CONTROLLER: + case OFPACT_STRIP_VLAN: + case OFPACT_SET_VLAN_PCP: + case OFPACT_SET_VLAN_VID: + case OFPACT_ENQUEUE: + case OFPACT_PUSH_VLAN: + case OFPACT_SET_ETH_SRC: + case OFPACT_SET_ETH_DST: + case OFPACT_SET_TUNNEL: + case OFPACT_SET_QUEUE: + case OFPACT_POP_QUEUE: + case OFPACT_POP_MPLS: + case OFPACT_DEC_MPLS_TTL: + case OFPACT_SET_MPLS_TTL: + case OFPACT_SET_MPLS_TC: + case OFPACT_SET_MPLS_LABEL: + case OFPACT_NOTE: + case OFPACT_OUTPUT_REG: + case OFPACT_EXIT: + case OFPACT_METER: + case OFPACT_WRITE_METADATA: + case OFPACT_WRITE_ACTIONS: + case OFPACT_CLEAR_ACTIONS: + case OFPACT_SAMPLE: + return false; + + case OFPACT_SET_IPV4_SRC: + case OFPACT_SET_IPV4_DST: + case OFPACT_SET_IP_DSCP: + case OFPACT_SET_IP_ECN: + case OFPACT_SET_IP_TTL: + case OFPACT_SET_L4_SRC_PORT: + case OFPACT_SET_L4_DST_PORT: + case OFPACT_RESUBMIT: + case OFPACT_STACK_PUSH: + case OFPACT_STACK_POP: + case OFPACT_DEC_TTL: + case OFPACT_MULTIPATH: + case OFPACT_BUNDLE: + case OFPACT_LEARN: + case OFPACT_FIN_TIMEOUT: + case OFPACT_GOTO_TABLE: + return true; + + case OFPACT_REG_MOVE: + return (mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->dst.field) || + mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->src.field)); + + case OFPACT_REG_LOAD: + return mf_is_l3_or_higher(ofpact_get_REG_LOAD(a)->dst.field); + + case OFPACT_SET_FIELD: + return mf_is_l3_or_higher(ofpact_get_SET_FIELD(a)->field); + + case OFPACT_PUSH_MPLS: + /* Recirculate if it is an IP packet with a zero ttl. This may + * indicate that the packet was previously MPLS and an MPLS pop action + * converted it to IP. In this case recirculating should reveal the IP + * TTL which is used as the basis for a new MPLS LSE. */ + return (!flow_count_mpls_labels(flow, wc) + && flow->nw_ttl == 0 + && is_ip_any(flow)); + } + + OVS_NOT_REACHED(); +} + static void do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, struct xlate_ctx *ctx) @@ -3131,6 +3282,11 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; } + if (ctx->was_mpls && ofpact_needs_recirculation_after_mpls(ctx, a)) { + compose_recirculate_action(ctx, ofpacts, a, ofpacts_len); + return; + } + switch (a->type) { case OFPACT_OUTPUT: xlate_output_action(ctx, ofpact_get_OUTPUT(a)->port, @@ -3607,6 +3763,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) ctx.table_id = 0; ctx.exit = false; ctx.use_recirc = false; + ctx.was_mpls = false; if (!xin->ofpacts && !ctx.rule) { ctx.table_id = rule_dpif_lookup(ctx.xbridge->ofproto, flow, -- 1.7.10.4 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
