On Wed, Jan 15, 2014 at 04:13:24PM +0900, Simon Horman wrote:
> This is an proposed enhancement to
> "Implement OpenFlow support for MPLS, for up to 3 labels."
> 
> Signed-off-by: Simon Horman <ho...@verge.net.au>
> ---
>  ofproto/ofproto-dpif-xlate.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index f14f308..57e8978 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2155,18 +2155,22 @@ 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;
>  
> -    if (!flow_pop_mpls(flow, eth_type, wc) &&
> -        flow_count_mpls_labels(flow, wc) >= 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));
> +    if (!flow_pop_mpls(flow, eth_type, wc)) {
> +        int n = flow_count_mpls_labels(flow, wc);
> +        size_t max_stack = MIN(ARRAY_SIZE(flow->mpls_lse),
> +                               ctx->xbridge->max_mpls_depth);
> +        if (n >= max_stack) {
> +            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,
> +                             max_stack);
> +            }
> +            ctx->exit = true;
> +            ofpbuf_clear(&ctx->xout->odp_actions);

I think that this can be simplified to just
    if (!flow_pop_mpls(flow, eth_type, wc)
        && flow_count_mpls_labels(flow, wc) >= 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 "
                         "more MPLS LSEs than the %"PRIuSIZE" "
                         "that can be handled.", ctx->xbridge->name,
                         max_stack);
        }
        ctx->exit = true;
        ofpbuf_clear(&ctx->xout->odp_actions);
    }
because flow_pop_mpls() is not going to complain based on
max_mpls_depth but only on ARRAY_SIZE(flow->mpls_lse), and I think
that's OK because we should flag anything with more MPLS labels than
max_mpls_depth but fewer than ARRAY_SIZE(flow->mpls_lse) as
ODP_FIT_TOO_LITTLE so that we get to look at every packet.  (Actually,
we don't seem to do that, so I'll propose a patch myself.)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to