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

[Thomas Heide Clausen <[e-mail]>]

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
>