Re: [mpls] AD review of draft-ietf-mpls-in-udp

[Xuxiaohu <[e-mail]>]

Hi Adrian,

Thanks a lot for your careful AD review. Please see my response inline.

> -----邮件原件-----
> 发件人: Adrian Farrel [mailto:[e-mail]]
> 发送时间: 2013年10月13日 4:29
> 收件人: [e-mail]
> 抄送: [e-mail]; [e-mail]
> 主题: AD review of draft-ietf-mpls-in-udp
> 
> Hi authors of draft-ietf-mpls-in-udp,
> 
> I have performed my usual AD review of your document following the
> publication request from the working group. The purpose of this
> review is to catch and fix any issues as early in the process as
> possible.
> 
> There are a number of concerns listed below. Some are minor editorial
> points, and a good number of the rest can be handled by the simple
> addition of text to describe things that I believe need extra words.
> a few of the issues are a little more significant and need direct
> discussion or modification of the draft.
> 
> I am not requiring changes, but we do need to talk about any of the
> points that you don't think need to be addressed by updates to the
> document.
> 
> Thanks for your work,
> Adrian
> 
> ---
> 
> It seems to me that the issue raised by Sri on the mailing list during
> WG last call was not addressed. Maybe it was not of concern to this
> working group because you are only interested in encapsulating MPLS.
> 
> The question asked, however, seems to be very valid for the people
> charged with looking after UDP port allocation and the future use of
> UDP. It can be summarised as:
> 
>   What would happen if every protocol asked for a port number to
>   encapsulate it? Why don't you ask for a single port number to indicate
>   "there is a payload protocol" and insert a shim to identify and demux
>   the payload?
> 
> I don't have a view on this, but I don't see that the WG either
> discussed the issue or asked for input from the TSV area. I have asked
> the TSV ADs to solicit input from the TSV directorate and the Port
> Doctors. You should not wait for this input (which may come any time
> between now and the end of IESG evaluation, but you should consider the
> issue and be prepared to discuss with the reviewers.

Thanks for pointing this out.

> ---
> 
> Abstract
> 
>    This document specifies an additional IP-based encapsulation for
>    MPLS, referred to as MPLS-in-UDP (User Datagram Protocol), which is
>    applicable in some circumstances. This document only describes the
>    MPLS-in-UDP encapsulation.
> 
> "Additional" to what?

Will remove it.

> Why "referred to as"? Isn't that exactly what it is?

Will remove it.

> The second sentence seems redundant since the first sentence say exactly
> that.

Will remove it.

> "...applicable in some circumstances" says nothing, of course. Either
> don't say this, or explicitly state the circumstance. Since (see below)
> the applicability is very specific and there is a clear use case that is
> the target of your work, I strongly suggest that you call out this case
> in the Abstract.

Is it ok for you if the above sentence is changed to"... applicable in some circumstances where IP-based encapsulation for MPLS is required and fine-grained load-balancing of IP tunneled MPLS packets is required as well..."?  Or can you suggest any text on this point?

> Introduction
> 
> Many of the same comment apply here as I noted for the Abstract, but in
> addition:
> 
>    This document specifies an additional IP-based encapsulation for
>    MPLS, referred to as MPLS-in-UDP (User Datagram Protocol). It also
>    describes the applicability of this encapsulation in presence of
>    other IP-based encapsulations for MPLS.
> 
> s/presence/the presence/

Will fix it.
> 
>    This encapsulation allows for two Label Switching Routers (LSR) to
>    be adjacent on a Label Switched Path (LSP), while separated by an
>    IP network.
> 
> This makes it sound like a new feature, but (of course) MPLS-in-IP and
> MPLS-in-GRE allows this as well.

How about adding "Like other IP-based encapsulation methods (e.g., MPLS-in-GRE)..." ahead of this sentence? 

>    In order to support this encapsulation, each LSR needs
>    to know the capability to decapsulate MPLS-in-UDP by the remote
>    LSRs. This specification defines only the data plane encapsulation
>    and does not concern itself with how the knowledge to use this
>    encapsulation is conveyed.
> 
> While this a fundamentally reasonable thing to say, it also makes the
> encapsulation entirely unusable. Since implementations already exist,
> perhaps you could tell us how this works. I suspect that in some
> environments the ability exist de facto (in other words, all devices are
> capable) while in other environments it is configured. You could say
> this and then suggest that distribution of this capability between
> participating nodes is for future study.

Will fix it.

>    An applicability statement will compare situations in which using
>    the MPLS-in-UDP encapsulation might be advantageous over other IP-
>    based encapsulations for MPLS. One of the key considerations in
>    this respect is how to achieve efficient load-balance of traffic
>    over Equal Cost Multi-Path (ECMP) and/or Link Aggregation Group
>    (LAG).
> 
> I don't understand this paragraph. Are you saying that a future document
> might be written? That isn't very helpful!

No, there is no future document. 

> It appears that you are trying to say "This encapsulation is better than
> other encapsulations" but without actually demonstrating it. Either
> delete this paragraph or actually do the work by adding a section to
> this document.

OK, I will delete this paragraph.

> ---
> 
> Section 1
> 
> There is quite a lot missing from this section. I don't mind whether the
> information goes in the main part of Section 1 or in Section 1.2, but it
> needs to show up.
> 
> - What environment / use case is this work targeted at?
> - Forward reference to the Security section and a note about the non-
>   availability of security. This is key since the applicability of this
>   is significantly restricted by this feature.
> - Brief overview of the mechanism (which is very simple, so doesn't need
>   more than a sentence mentioning "direct encapsulation", "use of dest
>   port" and a high-level observation about using the source port for
>   entropy and why.

Will fix it.

> 
> Section 1.1
> 
>    Currently, there are a number of IP-based encapsulations for MPLS.
>    These include MPLS-in-IP, MPLS-in- GRE (Generic Routing
>    Encapsulation) [RFC4023], and MPLS-in-L2TPv3 (Layer Two Tunneling
>    Protocol - Version 3)[RFC4817]. In all these methods, the IP
>    addresses can be varied to achieve load-balancing.
> 
> "These include..." implies you know of others. What are they and why
> haven't you mentioned them?

Will fix it.

> I think the statement about varying the IP addresses could be clearer.
> Probably by saying how this is done (since simply picking another
> destination address may balance the load by sending it somewhere else
> :-)

Will fix it.

> ---
> 
> Section 1.1
> 
>    In terms of MPLS-based encapsulations, load-balancing is achieved
>    with the introduction of the Entropy Label [RFC6790].
> 
> I think you need to delete the word "encapsulations".

Will fix it.

> Section 1.2
> 
> s/ECMP paths/ECMPs/

Will fix it.

> Section 1.2
> 
> As noted above, the explanation in this section is very sparse and could
> benefit from additional material.

Will fix it.

> Section 3 Source Port of UDP
> 
> I think this description needs to describe how a source behaves when the
> flow does not need entropy.

Will fix it.

> Furthermore, the text is not clear about the objective of "providing
> entropy". What type of algorithm should the source use and what must it
> not do?
> 
>                 For example, the
>                 entropy value can be generated by performing hash
>                 calculation on certain fields in the customer packets
>                 (e.g., the five tuple of UDP/TCP packets).
> 
> I find this to be a poor example because the source has in hand an MPLS
> packet with an arbitrary label stack and an unknown payload. Indeed,
> your Section 5 notes that the MPLS payload might not be TCP or UDP.

Our intention is whatever algorithm is actually used by the source to generate an entropy should be irrelevant to the data encapsulation and therefore it should be outside the scope of this doc. The entropy could be obtained from a hash of certain fields, or directly obtained from the IPv6 flow label in the case of IPv6 packet, or obtained from an entropy label in the case of a MPLS packet. The above just gives a simple example. How about adding explicitly " whatever algorithm is actually used by the source to generate an entropy is outside the scope of this doc " to the above sentence?

> Section 3 Destination Port of UDP
> 
> The text about upstream/downstream label assignment is perfectly fine,
> but doesn't belong in the description of this field.

Ok, will move it to section 4.

> 
> Section 3 UDP Checksum
> 
> I note that RFC 6935 says that using a zero UDP checksum for a UDP
> tunnel is appropriate when the payload protocol header includes its own
> protection. MPLS headers do not contain this form of protection. How do
> you justify the zero checksum in this case?

It said in this doc, "The usage of this field is in accordance with the current UDP specification. To simplify the operation on egress PE routers, this field is RECOMMENDED to be set to zero in IPv4 UDP encapsulation case, and also in IPv6 UDP encapsulation case if appropriate [RFC6935] [RFC6936]". That's to say, in case of IPv4 UDP tunnel, it's recommended to set to zero since the IPv4 header have the similar checksum field. In case of IPv6 UDP tunnel, it's recommend to set to zero "if appropriate". In other words, if inappropriate, this field is not set to zero in accordance with the current UDP specification [RFC768]. Whether or not it's appropriate is judged according to the specifications defined in [RFC6935] [RFC6936] ,e.g., 

  "3.   A transported protocol that encapsulates Internet Protocol (IPv4
        or IPv6) packets MAY rely on the inner packet integrity checks,
        provided that the tunnel protocol will not significantly
        increase the rate of corruption of the inner IP packet.  If a
        significantly increased corruption rate can occur, the tunnel
        protocol MUST provide an additional integrity verification
        mechanism.  Early detection is desirable to avoid wasting
        unnecessary computation, transmission capacity, or storage for
        packets that will subsequently be discarded". (quoted from http://tools.ietf.org/html/rfc6936#page-21) 
and 
   "5.   A transported protocol with a non-tunnel payload or one that
        encapsulates non-IP packets MUST have a CRC or other mechanism
        for checking packet integrity, unless the non-IP packet is
        specifically designed for transmission over a lower layer that
        does not provide a packet integrity guarantee". (quoted from http://tools.ietf.org/html/rfc6936#page-21)"

> I also don't think that proper attention has been paid to Section 5 of
> RFC 6936. You need to examine the requirements and describe additional
> behavior within your document.

Did you mean that we should add more detailed descriptions like: 

       "if the MPLS payload is Internet Protocol (IPv4
        or IPv6) packets. it MAY rely on the inner packet integrity checks... "
   and 
        "If the MPLS payload is non-IP packets, the UDP checksum MUST NOT be set to zero
        for checking packet integrity, unless the non-IP packet is
        specifically designed for transmission over a lower layer that
        does not provide a packet integrity guarantee..."

> Section 4
> 
>    As for other common processing procedures associated with tunneling
>    encapsulation technologies including but not limited to Maximum
>    Transmission Unit (MTU) and preventing fragmentation and reassembly,
>    Time to Live (TTL) and differentiated services, the corresponding
>    "Common Procedures" defined in [RFC4023] which are applicable for
>    MPLS-in-IP and MPLS-in-GRE encapsulation formats SHOULD be followed.
> 
> I think it is probably important to consider PMTU in the presence of
> ECMP (probably not necessary in the case of LAG). How does the source
> know the PMTU for each different value of the source port that it might
> apply?

IMHO, it is a common issue for any load-balancing mechanism. Would it be better to write a separate doc to address this common issue?

> ---
> 
> As far as I can see Section 5 is not ECN-friendly and says that when the
> payload protocol of the MPLS packet is not "TCP-friendly" the
> application generating the packets must use magic to avoid swamping the
> network.
> 
> We will see what the TSV area congestion experts have to say, but I
> think we will find that the approach here is simplistic unless the
> network across which the UDP tunnel runs is used for no other traffic
> except UDP tunnels carrying MPLS packets.

We originally just intented to mention this issue to the same extent as MPLS over L2TPv3 [rfc4817]. BTW, it seems that MPLS in IP or GRE [RFC4023] doesn't mention this point at all. We would like to see what the TSV area congestion experts have to say as well on this point. 

> Section 6 seems to indicate a major draw-back of this scheme. You have
> to note that MPLS networks are able to get away with having very little
> security because it is very hard to inject MPLS packets into a network.
> But MPLS-in-(foo-in-)IP encapsulation provides a way to inject packets
> just like any packet can be injected into an IP network.

Sure. It's the same issue with any other IP-based encapsulations for MPLS.

> Security (such as IPsec) provides a way to ensure that rogue packets do
> not have their headers stripped and their payload MPLS packets added to
> an LSP.

> You are making a clear statement that using IPsec means that there is no
> point in doing MPLS-in-UDP encapsulation. You need to follow up on this!
> 
> The first thing to do is to enhance the applicability text in Section 1
> to say where you would deploy this such that security is not an issue.

> The second thing is to talk about the security mechanisms that can be
> applied at the edges of the network to reduce the likelihood of such
> attacks being possible.
> 
> Lastly (or probably firstly!) you need to describe the attack vector and
> the implications of such an attack so that the implementer/deployer is
> clear what the risks are.

Will fix it.

> 
> 10.1
> 
> RFC 4023 needs to be a normative reference since you rely on it to
> describe how to set TTL and avoid fragmentation.

Will fix it.

Thanks again for your conscientious reviews:)

Best regards,
Xiaohu