Re: [RTG-DIR] RtgDir review: draft-ietf-pim-mtid-08.txt

[Yiqun Cai <[e-mail]>]

Thomas,

Thanks for the comment.  We will review it and get back to you some time
next week  or the week after (next week is a short week here in the US).

Thanks.


On 8/30/11 12:47 AM, "Thomas Heide Clausen" <[e-mail]>
wrote:

> Dear Yiqun,
> 
> Thank you for your detailed reply to my review. As a reviewer, I appreciate
> when the authors engage  such as you have done, and I am pleased to continue
> in these iterations with you. Do accept my apologies for the delay in getting
> back to you - life had a way of interfering in an unpredictable manner these
> past two weeks, but I should be back now.
> 
> Executive summary for ADs:
> 
> o Some suggestions for readability made to the authors
> 
> o Question as to if Experimental is a more appropriate status than Proposed
> Standard, as
> this document apparently is a first-attempt to do multicast-multi-topology
> 
> o Continued issues regarding IANA registry creation / assignments discussed.
> 
> o Continued discussion as to clarity of (in-)validation of received packets
> 
> I am looking forward to reviewing an updated version of this document.
> 
> A couple of comments in-line below, after having re-read the I-D and your
> email.
> 
> On Aug 9, 2011, at 23:12 , Yiqun Cai wrote:
> 
>> Thomas,
>> 
>> Thank you very much for the review.  Please see comments inline.
>> 
>> 
>> On 6/28/11 9:47 AM, "Thomas Heide Clausen" <[e-mail]> wrote:
>> 
>>> Hello,
>>> 
>>> I have been selected as the Routing Directorate reviewer for this draft. The
>>> Routing Directorate seeks to review all routing or routing-related drafts as
>>> they pass through IETF last call and IESG review, and sometimes on special
>>> request. The purpose of the review is to provide assistance to the Routing
>>> ADs. For more information about the Routing Directorate, please see
>>> http://www.ietf.org/iesg/directorate/routing.html
>>> 
>>> Although these comments are primarily for the use of the Routing ADs, it
>>> would
>>> be helpful if you could consider them along with any other IETF Last Call
>>> comments that you receive, and strive to resolve them through discussion or
>>> by
>>> updating the draft.
>>> 
>>> Document:    draft-ietf-pim-mtid-08.txt
>>> Reviewer:    Thomas Heide Clausen
>>> Review Date:   2011-06-27
>>> IETF LC End Date: 2011-06-27
>>> Intended Status:   Proposed Standard
>>> 
>>> Summary:
>>> 
>>> I have some minor concerns about this document that I think should be
>>> resolved
>>> before publication.
>>> 
>>> Comments:
>>> 
>>> This is the first PIM document I have reviewed, so I went back and read some
>>> of the previous
>>> RFCs. It may mean that my review is not sufficiently in-depth.
>>> 
>>> The document is relatively difficult to read. Specifically the introduction
>>> reads more like a
>>> problem statement (e.g. "if PIM was able to access .... it would be ...")
>>> than
>>> it does a specification.
>>> I would much prefer to have perhaps a very short motivational subsection to
>>> the introduction,
>>> then introduce that which this document actually introduces (and how).
>>> Currently, that is difficult
>>> to extract.
>> 
>> This was actually described in the 4th paragraph in the "Introduction".
>> 
>> "This document introduces a new type of PIM Join Attribute ...".
>> 
>> As Marshall pointed out in another review comment,  there are potentially
>> quite a few use cases that can benefit from this functionality.  We hoped to
>> capture at least one of them.
> 
> Marshall is quite right, and I think that you do well in capturing one in the
> text. 
> 
> My comment is one of clarity as to what the document provides:  "if PIM was
> able to Š it would beŠ.." and "This capability would  facilitateŠ" etc. reads
> as a problem statement. I would suggest that it would be clearer to the reader
> what this document provides by phrasing it akin to "Using the PIM Join
> Attribute, specified in this document, PIM can Š."  and "This capability
> enablesŠ."
> 
> I remember from my initial read-through of the document that I thought "Well,
> true, if PIM had this capability it would be nice, someone should go specify
> that". As this is the document "specifying that", I suggest affirming what
> this document provides, rather than suggesting what PIM lacks in the
> introduction.
> 
> 
> 
>>> I feel that RFC2119 language is not used consistently, and that terminology
>>> in
>>> general is 
>>> used somewhat inconsistently. Is "MT-ID" and "PIM MT-ID" the same or
>>> different
>>> things, for example?
>>> I would suggest that this is in part due to the absence of the by now
>>> traditional "Terminology" section,
>>> which (in my opinion) has as its primary role to ensure that spec-authors
>>> are
>>> conscious and consistent
>>> in their choice of words inside a spec.
>> 
>> We will add a section to clarify that.
> 
> Great, thanks! That will help. I note from the below that you intend to go
> through the document for 2119-terminology tightening, which I appreciate and
> which I think will help the document. Will you add a proper terminology
> section? As a relative newcomer to PIM, I know that such would have helped me
> understand the spec. A suggestion here would also be that this terminology
> section points to relevant RFCs from which terminology is imported (ala: "This
> document furthermore uses the terminology defined in RFCxxxx, RFCyyyy and
> RFCzzzz, specifically the terms foo, bar and foobar"). This, for readability
> purposes.
> 
>>> I find that there are some internal inconsistencies in the document, i.e.
>>> where it is stating different
>>> things in different sections (e.g. that one MUST NOT include a MT-ID Join
>>> Attribute with value 0 --
>>> yet that when performing a validity check, a Join packet with an MT-ID=0 is
>>> still considered valid).
>> 
>> Indeed.
>> 
>> However it was done on practical purpose,
>> 
>> 1.  we want to specify a valid range
>> 2.  we also want to specify what an implementation should behave if another
>> implementation includes a value that is outside of that range.
>> 
>> What we learned from our experience is that if we don't specify how to
>> handle error cases, we will consistently run into interop issues that can't
>> be solved easily.
> 
> Quite so, I agree that it is good to specify error cases.
> 
> My issue is, that you state that:
> 
> o  an implementation MUST NOT include a MT-ID Join Attribute with value 0
> o yet, when performing a validity check, join packet with MT-ID=0 is
> considered valid.
> 
> If you really mean "MUST NOT", then you must also consider a packet received
> that violates that "MUST NOT" as to be invalid.
> 
> Otherwise, the specification appears incoherent.
> 
> 
>>> One concern that I do have with the document is, that there are no
>>> references
>>> to any kind of
>>> previous work (typically, I would expect  this to have been well studied in
>>> academia) suggesting
>>> the usefulness and justifying the validity of the proposed approach. Note: I
>>> am not saying that
>>> the approach isn't useful and valid - I am saying that I do not know, and
>>> would feel comforted by
>>> some supporting references to this effect.
>> 
>> Actually,  this is the first attempt to integrate multi-topology routing
>> with multicast.
> 
> OhŠ..this raises a red flag with me, then, even more than before. My worry
> when I read through the document was, that I had no idea if, or how well, it
> would work in an operational setting, so I looked to the references for some
> evidence - be that a whitepaper documenting an operational deployment, or
> academic studies, or the like. Finding nothing of the kind did leave me
> concerned as to if we understood the validity of the proposal, which was why I
> raised this issue in my review.
> 
> You tell me that this is a first attempt at integrating multicast and
> multi-topology routing - and that is, indeed, both interesting and
> commendable. 
> 
> Alas, if you are right in the above (and I have no reason to doubt you), then
> I would suggest that this document is a candidate to be published as
> "Experimental", rather than as "Proposed Standard", until we have studied the
> matter and obtained a better understanding of behavior and performance
> characteristics? 
> 
> Were there any WG discussions regarding Exp-vs-PS that I should go look at?
> 
>>> Major Issues:
>>> 
>>> o I have actually not found any single "major issues", however I feel that
>>> the
>>> document is
>>> hard to read and that the collection of the "minor issues" below would
>>> constitute a single
>>> "major issue".
>>> 
>>> Minor Issues:
>>> 
>>> o The RFC2119 language is typically seen in a terminology section; something
>>> which
>>> I feel that this document should have, if for no reason other than to help
>>> new
>>> readers
>>> to PIM family of documents. It could be as simple as importing (pointing to)
>>> relevant 
>>> terminology from RFC5496/5384/4601
>> 
>> We will fix as suggested.
>> 
> 
> Thanks.
> 
>>> 
>>> o Also, the RFC2119 paragraph does not reflect the errata ("NOT RECOMMENDED"
>>> is missing)
>> 
>> Will fix.
>> 
> 
> Thanks.
> 
>>> 
>>> o Penultimate paragraph of section 2 "It might further need to perform the
>>> function
>>> of splitting and merging. But the detailed processing is beyond the scope of
>>> the 
>>> document". It would be beneficial with elaboration of under which conditions
>>> "splitting and merging" is required, as well as some guidance - for example,
>>> in form
>>> of a pointer to where these conditions are detailed.
>>> 
>>> o Ultimate paragraph of section 2: "the MT-ID Join Attribute may be simply
>>> referred to as MT-ID"
>>> -- suggest "the MT-ID Join Attribute will be referred to as MT-ID
>>> 
>> 
>> Ok
> 
> Thanks.
> 
>> 
>>> o This may be a matter of individual style, but I do not like empty section
>>> introductions, 
>>> such as 3. 
>>> 
>>> o 3rd paragraph in 3.1, suggest removing "please"
>>> 
>>> o Suggest that the figure in 3.1 be given a proper figure environment and
>>> number, and
>>> that references (especially later in the document) be given to that figure
>>> number.
>>> 
>> 
>> Ok.
> 
> Thanks.
> 
>>> o As a relatively newcomer to PIM, examples are appreciated. Alas, the
>>> example
>>> in 
>>> section 3.1 actually didn't help my understanding much. Also, that example
>>> is
>>> filled with details, whose importance I am not sure I capture: is the choice
>>> of 
>>> "OSPF 1000" and "OSPF 2000" (etc.) important, or are they simply
>>> illustrative.
>>> 
>> 
>> The text was added based on the comment from WG LC.  Some people do like to
>> see how MT-IDs are assigned numerically.
> 
> Alright, then. Would it be possible to simply add a sentence clarifying that
> these numbers are mere examples, without any other significance?
> 
>>> o First paragraph on page 5 states "The above example shows that the naming
>>> spaces of 
>>> MT-ID are not required to be the same between PIM and IGPs". That appears
>>> reasonable 
>>> -- however the last bullet point in section 3.2 on the same page goes on to
>>> say that 
>>> "...the PIM MT-ID is RECOMMENDED to be assigned using the same value as the
>>> IGP 
>>> topology identifier".
>> 
>> Yes.  The text describes two different cases and provides two different
>> recommendations depending on how the topologies are built.
> 
> RECOMMENDED is a SHOULD - which I read to mean "If you do NOT follow this
> recommendation, you should know that there may be consequences - and these
> consequences are Š.." See below.
> 
>>> Citing RFC2119:
>>> 
>>> 3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
>>>  may exist valid reasons in particular circumstances to ignore a
>>>  particular item, but the full implications must be understood and
>>>  carefully weighed before choosing a different course.
>>> 
>>> It appears to me that if this RFC2119 meaning is intended, then it would be
>>> important
>>> to (i) explain why and (ii) give guidance as to understand what the
>>> implications are of
>>> not following the recommendation.
> 
> My issue with this text is that you say "RECOMMENDED" - but that I do not see
> any consequences of following (or not) the recommendation. In other words, the
> 2119-meaning of "RECOMMENDED" appears to be too strong. Maybe writing
> "suggested" (lower-case, non-2119) would suffice?
> 
> In either case, I would like to understand why you recommend or suggest this.
> Could you add an explanatory sentence/paragraph?
> 
>>> o Also, the document contains many uses of "must", "may" and "should" in
>>> lowercase - I am
>>> wondering if some (all?) of these should be capitalized to RFC2119 terms. I
>>> am
>>> trying to
>>> point out those I catch, but I would recommend that the authors review the
>>> document to
>>> ensure that they capitalize those where they mean RFC2119 terminology.
>>> 
>> 
>> We will go through the document one more time and fix them if appropriate.
> 
> Thanks.
> 
>>> o On the same first paragraph on page 5, when I read " .... shows that
>>> ....",
>>> then I actually
>>> expect a bit more rigor - tainted by having spent too much time in academia,
>>> perhaps,
>>> but I read "...shows that ..." to entail (close to) a formal proof. Maybe
>>> "....illustrates that..."
>>> is a better choice of words?
>>> 
>> 
>> Sure.
> 
> Thanks.
> 
>>> Same holds for the last bullet point on page 5.
>>> 
>>> o Same paragraph, what exactly is meant by "...the prefix covering S...."? I
>>> assume that
>>> it has something to do with that routing state for S is not maintained by
>>> the
>>> two routing
>>> topologies?
>>> 
>> 
>> In multicast jargons,  S usually refers to a /32 host address (assuming
>> IPv4).  But the routing table usually contains only a prefix that covers or
>> includes S, instead of S.
> 
> OK, let's ascribe that to me not knowing multicast jargon ;)
> 
> I suggest inserting a rephrasing of this paragraph into the terminology
> section, then?
> 
>>> o Generally to section 3.1, it is somewhat unclear what this specification
>>> does, vs. what
>>> PIM already does, which caused me to chase off reading PIM RFCs.
>>> 
>>> Reading through the specification, would it be correct to say that:
>>> 
>>> - This I-D introduces a new name-space, the "PIM MT-ID"
>>> - A given topology is provisioned with that "PIM MT-ID" by way of a
>>> "MT-ID PIM Join Attribute", defined by this specification?
>>> 
>>> If yes, then I would suggest calling that out explicitly.
>> 
>> No it is not a new name-space.
>> 
>> This document introduces a new type of Join Attribute which was introduced
>> by RFC5384.
> 
> Then I strongly suggest stating that, almost as you phrased it above, in
> section 3.
> 
>> 
>>> 
>>> o Section 3.2, second bullet-point talks about "...a value is not required
>>> to
>>> be the same as 
>>> the MT-ID used by the unicast routing protocol ...." However the ultimate
>>> paragraph in
>>> section 2 defines MT-ID to be a "Join Attribute", which is a PIM-entity
>>> (RFC5384). Do
>>> you mean to say that this MT-ID should be used also in an unicast routing
>>> protocol, or
>>> is it a terminology issue?
>> 
>> MT-ID is used by unicast routing protocols (such as OSPF and IS-IS).  This
>> document introduces its use to PIM via PIM Join Attribute.
>> 
> 
> I _think_ that this will be addressed by the previous comment on RECOMMENDED,
> so I will await that.
> 
>>> 
>>> o Page 6, second bullet "This value must be unique and consistent within the
>>> network domain...."
>>> 
>>> - must or MUST?
>>> - "network domain" is a wonderfully vague term. A quick google leads to the
>>> first many results relating to either "LAN-style domain controllers" or DNS.
>>> Suggest that his might be a good candidate term for replacement, or for
>>> definition in a terminology section.
>> 
>> It is probably better that we remove "domain".
> 
> Thanks.
> 
>>> o Section 4.2.1, "...it may chose" - may or MAY? Also, can guideline be
>>> given
>>> as to when
>>> a PIM router would chose to, or not, encode the PIM MT-ID? Finally, "PIM
>>> MT-ID", is that
>>> the same as what the ultimate paragraph of section 2 calls "MT-ID".
>> 
>> In this document, yes they are the same.
> 
> If they are the same, then I suggest picking one, defining in a terminology
> section - and sticking to that choice through the document. If both terms are
> used in literature, pick the most used term, and have the terminology section
> state "MT-ID - is <blah blah>. In some literature, this is also occasionally
> called PIM MT-ID"
> 
>> 
>>> 
>>> o Section 4.2.1: missing colon before bullet points
>>> 
>>> o Section 4.2.1 first bullet point: why MUST NOT? What are the consequences
>>> of
>>> including a Join Attribute when the MT-ID=0?
>>> Specifically, in 4.2.3, the ultimate bullet, the validation rules simply
>>> state
>>> that if
>>> the value is 0, then the Join Attribute "...is ignored" (there is a SHOULD
>>> or
>>> MUST 
>>> needed here?) and that the rest of the message is processed as if that Join
>>> Attribute was not included.
>> 
>> In common practice (as with OSPF and IS-IS too),  0 is reserved for
>> "default" topology.  Including 0 will cause unnecessary interop issues that
>> brings no additional gain.  Thus we prevent the inclusion of it.
> 
> This actually links in with the IANA section of this document, which I find
> somewhat hard to read/parse. You mention that you will rework that, so I look
> forward to seeing an updated version.
> 
> Still, does this document create an IANA registry for MT-IDs? Generally, I
> would expect an I-D to state something of the form:
> 
> "IANA is requested to create a registry for Š.. with initial assignments and
> allocation policies according to table 42"
> 
> (which the RFC editor tends to reword to something like "IANA has created a
> registry for Š.")
> 
> This makes it clear if the registry, its policies, etc., are to be found in
> this document, and are therefore completely documented in this document.
> 
> If this document does not create said IANA registry, I would expect the IANA
> section to say something like "IANA is requested to allocate a Š. from the
> XXXX registry, defined in RFCxxxx" - that way I would know where to go look
> for assignments, policies, Š.
> 
> Now, if this document creates an IANA registry for MT-IDs, then setting aside
> 0 as a non-allocatable, non-usable value simply removes a value from that
> space - there's nothing "magic" as such about the number 0. If there is a
> common practice that a specific value such as 0 has a commonly understood
> meaning, and that therefore that value should be avoided, it would actually be
> immensely useful to have that explanation with the IANA registry creation -
> effectively this would be guidance to IANA as to how to assign values from
> that registry.
> 
> If this document doesn't create that IANA registry, then I would expect this
> discussion to be in the document which does?
> 
>>> o Section 4.2.1 2nd and 3rd bullet points, why "SHOULD NOT"? Can any
>>> guidance
>>> be provided with respect to the consequences of ignoring such recommendation
>>> (or, if there are no negative consequences, then the benefits of following
>>> them)?
>>> 
>> 
>> Because these packets do not require the receiving router to perform any RPF
>> action.  And when a router doesn't perform RPF action,  MT-ID is not needed.
> 
> Suggest saying that in the document. Is it a SHOULD or a MUST? It sounds a bit
> like a MUST to me?
> 
>>> o Section 4.2.2 first bullet - prefer section references (if using xml2rfc
>>> <xref targe="...."/> tags)
>>> rather than "next section" or "in section Conflict Resolution". This is a
>>> general comment,
>>> but this seemed like a good point to mention this.
>>> 
>> Ok
>> 
> 
> Thanks.
> 
>>> o Section 4.2.3, first bullet: as t his is about "Validating a PIM MT-ID
>>> Join
>>> Attribute", I find it
>>> curious that it suggests to check that there is at most 1 PIM MT-ID Join
>>> Attribute included -
>>> then goes on to say that it's OK if there are more, just ignore all but the
>>> last. I would suggest
>>> that either this is a validation check and so if more than 1 PIM MT-ID Join
>>> Attribute then
>>> the packet is malformed and should be discarded - or, that this be specified
>>> somewhere
>>> other than a section specifying how to determine if an attribute is valid or
>>> not.
>> 
>> That is exactly why we wanted to include this document how to handle error
>> case.  We can choose to discard, or ignore the attributes but continue with
>> the next update.  The second is more appropriate for PIM because
>> "discarding" would require a rewinding of the operation already done on
>> previous PDUs which is hard to do.
> 
> That's fine, but then it is not "Validating" - a packet with one or more PIM
> MT-ID Join Attributes are all valid.
> 
> You state that an implementation must validate "There is at most 1 PIM MT-ID
> attribute encoded". Therefore, if I receive a packet with 2, it can't be
> valid. Yet, in the next sentence you say that it's ok if there are more than
> one, just use the last.
> 
> Either it is "at most 1" or it is "there may be more, use the last". It can't
> be both.
> 
> By the way, you use the terms "encoded" and "included" for PIM MT-ID
> attributes in this bullet, both. Are there any differences? If yes, what are
> they? If no, why different words?
> 
>>> o Same section & bullet: the title is "Validating a PIM MT-ID Join
>>> Attribute",
>>> i.e. validating _a_ single
>>> such. Yet, the bullet talks about having possibly multiple PIM MT-ID Join
>>> Attributes encoded.
>>> Encoded where? Should this section be retitled "Validating a Join Packet
>>> with
>>> a PIM MT-ID 
>>> Join Attribute" instead?
>> 
>> The paragraph does attempt to describe how to validate the particular type
>> of Join Attribute that is PIM MT-ID.  It doesn't specify how to validate
>> Join Attributes or the Join message in general.
> 
> Then it is even more confusing. Would "Validating PIM MT-ID Join Attributes in
> a received Join Packet" be a more correct title (we can fuss over if it is too
> long or not, but semantically it seems more correct)?
> 
>> 
>>> 
>>> o Section 5: Suggest that encoding, byte-order, ..., be called out (NBO,
>>> unsigned integer, ....) for
>>> newly defined fields.
>>> 
>> 
>> Ok.
> 
> Thanks.
> 
>> 
>>> o Section 5: Suggest calling out that reserved bits SHOULD be cleared on
>>> transmission and
>>> MUST  be ignored on reception.
>>> 
>> 
>> Yes it s ithere.
>> 
> 
> Sorry, no, I do not find that. Can you point me to where you say that, please?
> 
>>> o Section 5: Suggest citing the spec defining the format used (RFC5384?),
>>> least it appears
>>> odd to have an 8 bit "Length" field, and yet defining its value to always be
>>> 2.
>>> 
>> 
>> That would be RFC4601.
>> 
>>> o Section 6, IANA considerations.
>>> 
>>> - Suggest that "The IANA" be replaced by simply "IANA"
>>> 
>> 
>> Ok.
>> 
>>> - 2nd paragraph, should it be "IANA has assigned the PIM HELLO Option Type
>>> ...."
>>> 
>> 
>> Not yet but IANA will.
> 
> See comment above. The IANA section contains instructions to what we as
> protocol authors request (kindly) that the good folks at IANA do for us. To
> make their life easier (and the document more readable), suggest rephrasing as
> instructions (as also discussed above with registry creation).
> 
>>> - I do not understand what the IANA action for the ultimate paragraph in 6.1
>>> is about?
>>> 
>> 
>> The headings are wrong. We will fix them.
>> 
> 
> Thanks.
> 
> Respectfully Yours,
> 
> Thomas
> 
>>> Respectfully Yours,
>>> 
>>> Thomas
>> 
>> Thanks again.
>> 
>> 
>> -- 
>> Yiqun
>> 
> 


-- 
Yiqun