Re: [Teas] Gen-art LC review of draft-ietf-teas-rsvp-te-srlg-collect-02

[Elwyn Davies <[e-mail]>]

Hi, Matt.

Thanks for the update... As they say, it's your document, so the delay 
wasn't a problem for me!

Anyway, I have had a read through it, and the sum total of my comments 
is 'nice job'!

AFAICS the new version has addressed all my comments, and now reads very 
well.  Perhaps, one might say 'worth the wait'.  I'll do a formal review 
as and when it goes to the IESG (or if it has to go to last call again).

Once again, thabks.
Elwyn

On 17/03/16 16:15, Matt Hartley (mhartley) wrote:
>
> Elwyn,
>
> Apologies for taking so long over this (again). We’ve now published 
> v-04 of this, which hopefully addresses your comments.
>
> Cheers
>
> Matt
>
> *From:*Elwyn Davies [mailto:[e-mail]]
> *Sent:* Tuesday, October 06, 2015 1:04 PM
> *To:* Matt Hartley (mhartley) <[e-mail]>; General area 
> reviewing team <[e-mail]>; 
> [e-mail]; TEAS WG 
> ([e-mail]) <[e-mail]>
> *Subject:* Re: Gen-art LC review of 
> draft-ietf-teas-rsvp-te-srlg-collect-02
>
> Hi, Matt.
>
> Thanks for the responses.
>
> Some comments in line.
> Regards,
> Elwyn
>
> On 02/10/2015 20:25, Matt Hartley (mhartley) wrote:
>
>     Elwyn,
>
>     Thanks for the review, and apologies for the delay in replying. Responses to your concerns inline.
>
>         I am the assigned Gen-ART reviewer for this draft. The General Area
>
>         Review Team (Gen-ART) reviews all IETF documents being processed by
>
>         the IESG for the IETF Chair.  Please treat these comments just like
>
>         any other last call comments.
>
>         For more information, please see the FAQ at
>
>         <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
>         <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
>         Document: draft-ietf-teas-rsvp-te-srlg-collect-02.txt
>
>         Reviewer: Elwyn Davies
>
>         Review Date: 2015/09/13
>
>         IETF LC End Date: 2015/09/24
>
>         IESG Telechat date: (if known) -
>
>         Summary:
>
>         Not ready for publication.  The main problem appears to be that the
>
>         use of SRLG ID RRO sub-objects in Resv messages seems to be an
>
>         afterthought.  It needs to be covered in the introduction. It is also
>
>         not clear how nodes are informed that SRLG information needs to be added to Resv messages.
>
>     OK. More on this later.
>
> Comments are later as well!
>
>         I
>
>         am also concerned that the ordering constraints imposed on RRO
>
>         sub-objects by various different standards (at least this one and RFC
>
>         7571 - there may be others that I am not aware of) may become overly
>
>         complex and potentially mutually incompatible.  When I reviewed the
>
>         draft that subsequently became RFC 7571 earlier this year there were
>
>         already internal issues with sub-object ordering that had to be sorted
>
>         out - checking that ordering constraints do not result in a deadly
>
>         embrace could get quite complicated if more specifications add to the RRO stack.
>
>     I take your point... but do you consider this something that has to be addressed by this document? This seems to me to be a more general problem with RSVP-TE that the WG may wish to address separately.
>
> More relevant to this point was RFC 7570 - apologies.  To some extent 
> this RFC covers the ordering problem in Sections 2.2 and  2.3 and, in 
> conjunction with RFC 4490, Section 6.1 recommends that an RFC defining 
> a new ERO/RRO subobject  should specify ordering rules to go with the 
> object.
>
> I suspect that the SRLG information objects probably don't have any 
> ordering rules other than the upstream/downstream story between 
> themselves (and that could be handled by a bit flag as suggested 
> elsewhere in this email).  However, I am not an RSVP expert and there 
> might be some other ERO/RRO subobjects that I don't know about with 
> which it might interact.  Explicitly stating the ordering constraints 
> (if any)  relative to other subobjects would address the issue.
>
>         Major issues:
>
>         What is an SRLG ID?  OK, it is a 4 octet (opaque) data item. However,
>
>         the specification says nothing about how it might be used to convey
>
>         the SRLG information.
>
>     No, we don't. Do you think that an explanation over and above that provided by RFC 4202 is necessary?
>
>         I *guess* that this may be because this is just a handy facility that
>
>         can be used in whatever way an implementation chooses - but if so it
>
>         would good to say this.  Maybe an example of how the authors envisage
>
>         it might be used, maybe in the context of the use case in s1.1 would
>
>         be helpful.
>
>     We can add an example if you think it'd be helpful, but I'm reluctant to (re)define what SRLGs are or mean in this document.
>
> Probably just adding the statement (filched from RFC 4874):
>
>     The identifier of an SRLG (SRLG ID) is defined as a 32-bit quantity in
>
>         [RFC4202 <https://tools.ietf.org/html/rfc4202>].
>
> would help.
>
>
>         s4.1:
>
>                  The SRLG Collection flag is meaningful on a Path message.  If the
>
>                  SRLG Collection flag is set to 1, it means that the SRLG information
>
>                  SHOULD be reported to the ingress and egress node along the setup of
>
>                  the LSP.
>
>         I am unclear how the Path message is going to impart SRLG information
>
>         to both ends of the LSP as my understanding of RSVP is that the Path
>
>         message travels only in one direction along the path of the LSP.  ...
>
>         Ah, when I read down towards the end of s5.1, it appears that the SRLG
>
>         info may be in Resv messages as well, and might be collected during
>
>         the processing of the Resv message along the (reverse) path. According
>
>         to RFC 3209, the presence of an RRO in the Path message will trigger
>
>         the addition of an RRO to the Resv message, but this does not tell the
>
>         nodes that SRLG information ought to be added. Presumably the SRLG
>
>         Collection flag should be set somewhere in a suitable place in the
>
>         Resv message also.  But where?  The text above says the flag is only
>
>         meaningful on Path messages!!  Maybe the RRO Attributes sub-object
>
>         might be relevant [RFC5420].
>
>     The idea is that the processing node knows that SRLG information is required for the LSP from its processing of the Path message, and will apply this to the Resv too when it arrives. But, yeah... this is never explicitly stated, and it should be.
>
> Fair enough. Given that RFC 3209 implies you have to store the whole 
> RRO stack anyway, this isn't exactly much of a burden.
>
>         The draft needs to talk about both directions and uni-/bidirectional
>
>         LSPs from an earlier point in the document.
>
>     So perhaps add a short overview of how the collection works at, say, the beginning of s3? I think that would make the document easier to understand at first reading.
>
> That would be an excellent idea.
>
>         Minor Issues:
>
>         s5: RRO Sub-object ordering constraints.  In s4.2, a number of
>
>         ordering constraints are specified indicating where the SRLG info
>
>         objects should be placed in the stack of RRO sub-objects.  Section 5
>
>         does not discuss what should happen if the receiving node detects that
>
>         the sub-objects don't match the specified ordering constraints.
>
>     Yes, we should address this. Given the principle of "be strict in what you send, and liberal in what you accept", I'll add some text to say that the processing node MAY reject the Path or Resv and generate a PathErr/ResvErr if the ordering is wrong. Sound reasonable?
>
> Yep.. If you took the flag option below this would be a NO-OP.
>
>         A more general issue is
>
>         whether there are interactions with ordering constraints from other
>
>         specifications that use RRO sub-objects - for example, RFC 7571 has
>
>         some quite complex ordering constraints.
>
>     Yes. But as I said earlier, I think this is an issue that would probably be best addressed separately.
>
> See previous response.
>
>         There is also the following text in
>
>         s5.1:
>
>                  o  For Path and Resv messages for a bidirectional LSP, a node SHOULD
>
>                     include SRLG sub-objects in the RRO for both the upstream data
>
>                     link and the downstream data link from the local node.  In this
>
>                     case, the node MUST include the information in the same order for
>
>                     both Path messages and Resv messages.  That is, the SRLG sub-
>
>                     object for the upstream link is added to the RRO before the SRLG
>
>                     sub-object for the downstream link.
>
>         It strikes me that using one of the reserved bits in the SRLG
>
>         sub-object to explicitly identify whether a sub-object applies to the
>
>         upstream or downstream direction would make things less error prone
>
>         and reduce the ordering constraint which might get quite complicated
>
>         as time goes on if new RRO sub-objects continue to be added.
>
>     Yes, this seems reasonable.
>
> I'll look out for your choice in this.
>
>         Nits/editorial comments:
>
>         General: Just checking: does this specification apply to basic MPLS or
>
>         only to the extended Generalized MPLS?  It would be good to be clear
>
>         about the scope up front.
>
>     By "basic MPLS", do you mean packet-TE? If so, that's a sub-set of GMPLS, and so this specification applies there too.
>
> Ok.  I guess what I ought to be saying is that it would be good to be 
> explicit that this is an additional capability for RSVP-TE - and hence 
> is relevant for both (basic) MPLS with TE and GMPLS.
>
>         General: Bringing out the IANA temporary allocations at every point
>
>         where they apply in Sections 4.1, 4.2, 5.1  and 8 is undesirable as it
>
>         will have to be edited out by the RFC Editor increasing the scope for
>
>         error.  OK this is not a big risk but it would simplify things if the
>
>         body text had the expected final form and the temporary allocation
>
>         text notes were confined to a special section that was marked for
>
>         removal by the RFC Editor.
>
>     OK
>
>         General: s/byte/octet/g
>
>         Abstract: Probably ought to mention (G)MPLS explicitly and expand TE.
>
>         s1, para 1:  It would be good to expand TE explicitly (as Generalized
>
>         MPLS Traffic Engineering) again.
>
>     Looking at the RFC Editor's list athttps://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt, GMPLS seems to be considered well enough known to not require expansion. TE, apparently, isn't :)
>
>         s3: Using RFC 2119 language here is inappropriate - they are
>
>         design/usage issues not testable protocol features.
>
>     OK - will re-work this
>
>         s4.1, para 1: s/indicate nodes/indicate to nodes/
>
>         s4.1 and s4.2, last para in each case: s/The rules of the/The rules
>
>         for the/ (2 places)
>
>         s5.1, paras 2 and 3: s/and the SRLG Collection Flag set/with the SRLG
>
>         Collection Flag set/ (2 places)
>
>         s5.1, last para: Need to expand FA acronym.
>
>     OK on these four
>
>         s6.1: It would be useful to repeat the policy configurations mentioned
>
>         in s5 in this section.
>
>     I'm not quite sure which bits of s5 you're getting at here?
>
> Looking again at ss5 and 6, it looks as if there are three pieces of 
> policy to manage in a node:
> - Whether the node is allowed to participate in SRLG info collection 
> (s5.1)
> - Whether the node SHOULD explicitly notify changes in the SRLG info 
> as it becomes aware of them  (5.2)
> - Whether the border node SHOULD summarize etc collected SRLG info 
> regarding its domain for messages passing out of the domain.
>
> Arguably the processing documented in the bullet in s6.1 should be in 
> an additional section in s5 as it describes processing of the RRO.   
> s6.1 would then just be a list similar to the one just above here 
> documenting the three policy settings.
>
>
>
>         s6.2, para 1: s/SRLG ids/SRLG IDs/  [please check that there aren't
>
>         any other cases].
>
>     Yep.
>
>     One other thing (from Deborah) for completeness and so I don't forget it - we need to update the IANA section to reflect the recent update to the temporary allocation.
>
> Missed this one on the previous pass: As it stands, the IANA 
> specification in s8.1 for the attributes flag:
>
>              Bit No       Name        Attribute   Attribute   RRO  Reference
>
>                                       Flags Path  Flags Resv
>
>              -----------  ----------  ----------  ----------- ---  ---------
>
>              12 (tempo-   SRLG        Yes         Yes         Yes  This I-D
>
>              rary expires collection
>
>              2015-09-11)  Flag
>
> indicates that the collection flag could appear in Resv messages.  
> This is not what s4.1 says at present.
>
>
> /Elwyn
>
>     Cheers
>
>     Matt
>