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

Bruce Davie bdavie at nicira.com
Wed Mar 6 17:26:19 UTC 2013


The processing for MPLS TTL is somewhat complex compared to IP, because there are a few different options for what is acceptable when pushing and popping labels. However, one thing that DOES carry forward straightforwardly from IP is that if the TTL reaches zero, the packet MUST be discarded. This is documented in RFC 1812, Section 5.3.

If you want to see all the gorey details about MPLS TTL processing, you need to read RFC 3443.

Bruce



On Mar 6, 2013, at 8:54 AM, Ben Pfaff wrote:

> On Wed, Mar 06, 2013 at 08:40:38AM -0800, Ben Pfaff wrote:
>> On Wed, Mar 06, 2013 at 08:29:13AM -0800, Jesse Gross wrote:
>>> On Wed, Mar 6, 2013 at 8:21 AM, Ben Pfaff <blp at nicira.com> wrote:
>>>> On Wed, Mar 06, 2013 at 07:46:16AM -0800, Jesse Gross wrote:
>>>>> On Tue, Mar 5, 2013 at 11:08 PM, Simon Horman <horms at verge.net.au> wrote:
>>>>>> [ Cc Pravin B Shelar ]
>>>>>> 
>>>>>> On Tue, Mar 05, 2013 at 06:11:27PM -0800, Ben Pfaff wrote:
>>>>>>> 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.
>>>>>> 
>>>>>> It seems to me that dec_ttl has the same problem.
>>>>>> 
>>>>>> But it also seems to me that it is the code rather than the documentation
>>>>>> that should be updated.
>>>>>> 
>>>>>> The commit log for the addition of the TTL decrement action states:
>>>>>> 
>>>>>>    commit f0fd1a1772665ea57662281d9cccadb0f0146196
>>>>>>    Author: Pravin B Shelar <pshelar at nicira.com>
>>>>>>    Date:   Fri Jan 13 17:54:04 2012 -0800
>>>>>> 
>>>>>>    ofproto: New action TTL decrement.
>>>>>> 
>>>>>>    Following patch implements dec_ttl as vendor action with similar
>>>>>>    semantics as OpenFlow 1.2. If TTL reaches zero while procession
>>>>>>    actions in current table, the remaining actions in previous tables
>>>>>>    are processed. A configuration parameter is added to make TTL
>>>>>>    decrement to zero generate packet in.
>>>>>> 
>>>>>>    Feature #8758
>>>>>>    Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>>>>>> 
>>>>>> But the code features "if (ttl > 1)".
>>>>>> 
>>>>>> That now seems like a logic bug to me (though perhaps the afternoon
>>>>>> air is affecting my brain adversely).
>>>>>> 
>>>>>> 
>>>>>> I have revised the dec_mpls_ttl patch to use "(ttl > 0)".
>>>>>> Should I provide a patch to update dec_ttl?
>>>>> 
>>>>> I think you were right the first time - if a decrement causes the TTL
>>>>> to hit zero then it should go to the exception case (in theory we
>>>>> should never receive a packet with TTL zero here, so it's always an
>>>>> exception).  I believe this applies equally to IP and MPLS.
>>>> 
>>>> My understanding is that, in IP, a host accepts packets with TTL=0, but
>>>> a router discards them.  If that is correct, then decrementing a TTL to
>>>> 0 should not discard the packet, only decrementing a TTL that is
>>>> initially zero.
>>> 
>>> From the IPv6 RFC (since the meaning has somewhat changed over time):
>>> 
>>>   Hop Limit            8-bit unsigned integer.  Decremented by 1 by
>>>                        each node that forwards the packet. The packet
>>>                        is discarded if Hop Limit is decremented to
>>>                        zero.
>> 
>> The requirements are a little confusing.  For example, RFC 1122 states:
>> 
>>            A host MUST NOT discard a datagram just because it was
>>            received with TTL less than 2.
>> 
>> However, RFC 1009 is very clear:
>> 
>>      If the TTL is reduced to zero, the datagram must be discarded, and
>>      the gateway may send an ICMP Time Exceeded message to the source.
>>      A datagram should never be received with a TTL of zero.
>> 
>> so I agree with you.
> 
> Bruce, are the TTL requirements for MPLS the same as stated for IP in
> the text quoted directly above?  If not, what are the TTL requirements
> for MPLS?
> 
> Thanks,
> 
> Ben.




More information about the dev mailing list