[ovs-dev] [PATCH 1/3] Add support for dec_mpls_ttl action

Ben Pfaff blp at nicira.com
Wed Mar 6 02:11:27 UTC 2013


On Wed, Mar 06, 2013 at 10:42:02AM +0900, Simon Horman wrote:
> 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 <yamahata at valinux.co.jp>
> > > Signed-off-by: Simon Horman <horms at 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.

OK, that makes sense, thank you.



More information about the dev mailing list