On Tue, Mar 05, 2013 at 10:14:44AM -0800, Ben Pfaff wrote:
> On Thu, Feb 28, 2013 at 06:15:07PM +0900, Simon Horman wrote:
> > This adds support for the OpenFlow 1.1+ dec_mpls_ttl action.
> > And also adds an NX dec_mpls_ttl action.
> >
> > The handling of the TTL modification is entirely handled in userspace.
> >
> > Reviewed-by: Isaku Yamahata <yamah...@valinux.co.jp>
> > Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> The only issue I see with this is that it seems uncertain about what is
> an invalid MPLS TTL. The code says that, pre-decrement, values 0 and 1
> are invalid:
>
> + if (ttl > 1) {
> + ttl--;
> + set_mpls_lse_ttl(&ctx->flow.mpls_lse, ttl);
> + return false;
> + } else {
> + execute_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0);
>
> The documentation says that, pre-decrement, value 0 is invalid:
>
> +.IP \fBdec_mpls_ttl\fR
> +Decrement TTL of the outer MPLS label stack entry of a packet. If the TTL
> +is initially zero, no decrement occurs. Instead, a ``packet-in'' message
>
> I don't know MPLS, so I don't know which definition is correct.
I am not sure and to be honest I had just followed dec_ttl implementation
when adding the code. Reading up I think that section 2.3 of RFC3443
implies this is the desired behaviour.
I'm not sure why the documentation conflicts with the code, most likely
it documents a previous incantation of the code. In any case I propose
updating it rather than the code.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
