Thanks for the quick response. Comments inline: On Oct 6, 2010, at 7:55 AM, Les Ginsberg (ginsberg) wrote: > Ben - > > Thanx for the review. > Inline. > >> -----Original Message----- >> From: Ben Campbell [mailto:ben@xxxxxxxxxxx] >> Sent: Tuesday, October 05, 2010 12:06 PM >> To: draft-ietf-isis-genapp.all@xxxxxxxxxxxxxx; General Area Review > Team >> Cc: The IETF >> Subject: Gen-ART LC/Telechat Review of draft-ietf-isis-genapp-03.txt >> >> I am the assigned Gen-ART reviewer for this draft. For background on >> Gen-ART, please see the FAQ at >> < http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. >> >> Please wait for direction from your document shepherd >> or AD before posting a new version of the draft. >> >> Document: draft-ietf-isis-genapp-03.txt >> Reviewer: Ben Campbell >> Review Date: 05 Oct 2010 >> IESG Telechat date: 07 Oct 2010 >> >> Summary: >> >> The draft is almost ready for publication as a proposed standard, but > I >> have some concerns that I think should be addressed first. >> >> >> Major issues: >> >> -- This draft creates an "expansion" code point in an IANA registry, >> where the expansion registration requirements are weaker than those of >> the parent registry. This always makes me nervous, as it opens the >> window for end-runs around the registration requirements of the > parent. >> >> In this particular instance, the parent registry policy is "expert >> review" while the proposed expansion registry policy is "specification >> required". This draft puts normative requirements on the content of > the >> required specifications, and makes additional non-normative statements >> about the intended use of the GENINFO code point. This implies to me >> that the review process needs to do more than determine that > sufficient >> specification exists. Rather, it needs to determine that the criteria >> in this draft are met by that specification. Therefore, I think that > it >> would be appropriate for the GENINFO registry to use the "expert >> review" policy. > > From RFC 5226: > > "Specification Required - Values and their meanings must be > documented in a permanent and readily available public > specification, in sufficient detail so that interoperability > between independent implementations is possible. When used, > Specification Required also implies use of a Designated > Expert, who will review the public specification..." > > We deliberately chose "Specification Required" because: > > a)It requires a public specification > b)It allows the public specification to be other than an RFC > c)It requires expert review Completing the sentence in your quote: "who will review the public specification and evaluate whether it is sufficiently clear to allow interoperable implementations." My understanding of "Specification Required" is that the expert review is merely to ensure that the documentation is sufficiently complete and readable to implement in an interoperable way. That review is not intended to ensure compliance to other criteria specified in the draft. However, the draft includes some specific criteria for GENINFO applications. If you want the reviewer to enforce those criteria, then I think you need at least "Expert Review". OTOH, in RFC5226, the "expert review" policy has less to say about the required level of documentation, so the draft might require some more meat in that area. I note that while RFC3563, which established the IS-IS TLV Codepoint registry, says "Expert Review", the review criteria is pretty much equivalent to "standards action". I'm guessing the only reason it was not "standards action" was to allow ISO/IEC JTC1/SC6 to submit specifications, which are to be held to the same standard as a standards-track RFC, but do not actually get published as such. So for practical purposes, the proposed policy for GENINFO is significantly weaker than for the parent registry--more so than one might think from just looking at the registry itself. > >> >> Minor issues: >> >> -- section 4.2, 2nd paragraph: "Where this is not possible, the two >> affected LSPs SHOULD be flooded as an atomic action" >> >> Any reason that this is not a MUST, since it seems like the safety-net >> behavior for when the aforementioned SHOULD is not possible to > follow? >> > > It is a recommended behavior. If an implementation does not do this it > does not create an interoperability issue - but it may create > sub-optimal behavior. Okay. > >> -- Section 4.3: "When information in the two GENINFO TLVs conflicts > i.e >> there are different settings for a given attribute, the procedure used >> to choose which copy shall be used is undefined." >> >> Should their be normative requirement not to create this undefined >> condition in the first place? > > If the revised version of a GENINFO TLV is sent in an LSP with a > different number from the previous version there can be transient > windows where other systems have two copies of information regarding the > same application. This may be unavoidable. For completeness we specify > that the choice of what to do in such transient situations is > implementation specific (undefined). This section does specify ways to > minimize the occurrence/duration of such transient periods. Does leaving this undefined cause interop issues? If not, then no problem. > >> >> >> >> -- Security Considerations: >> >> This seems too lightweight. Is it impossible for GENINFO applications >> to include sensitive information? Are there no security guidelines > that >> should apply to GENINFO application specifications? > > We have no way of knowing what type of information might be advertised > by a given application - and we are not limiting what may be advertised. > Clearly the public document which specifies the application would need > to address the security issues it introduces. We cannot do that here. Since the registration policy is not at least "RFC Required", there's no explicit requirement that the public document actually do this. If you wish to require them to do it, you will need to state something to that effect. (See previous comment about whether the registration policy actually enforces that sort of criteria.) > >> >> Even if the answer is that the underlying IS-IS protocol provides >> sufficient security for any reasonable use of the GENINFO code point, >> it would be worth saying that explicitly. >> >> Nits/editorial comments: >> >> -- section 2 >> >> Please expand IS-IS and PDU on first mention. > > OK > >> >> -- section 6, last paragraph: >> >> Expected/desired by whom? > > Well, at least by the authors. :-) Okay :-) I guess, I meant to say that, if this expectation was a matter of WG concensus, you could state it more strongly, as in "The IS-IS working group expected..." > >> >> -- Outdated reference for draft-ietf-isis-mi > > It was current at the time the draft was written. > > > Les > _______________________________________________ Ietf mailing list Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf