Erik - Thanx for the detailed review. I have published V24 of the draft which addresses all of your comments (and a few pending AD review comments from Alvaro). Some exceptions noted below. > -----Original Message----- > From: Erik Kline via Datatracker <noreply@xxxxxxxx> > Sent: Wednesday, April 17, 2019 7:20 PM > To: gen-art@xxxxxxxx > Cc: lsr@xxxxxxxx; ietf@xxxxxxxx; draft-ietf-isis-segment-routing- > extensions.all@xxxxxxxx > Subject: Genart last call review of draft-ietf-isis-segment-routing-extensions- > 23 > > Reviewer: Erik Kline > Review result: Ready with Nits > > 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 > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-isis-segment-routing-extensions-?? > Reviewer: Erik Kline > Review Date: 2019-04-17 > IETF LC End Date: 2019-04-17 > IESG Telechat date: Not scheduled for a telechat > > Summary: > > For what little I know of IS-IS and segment routing, this all seems to make > general sense. I simply had some language/style nits (below). > > Major issues: > > Minor issues: > > Nits/editorial comments: > > # Section 1 > > * "SR's control-plane can be applied ..., and do not require...". It looks > like the subject of the sentence is "control-plane" and so perhaps "do not" > should be "does not". > > * s/draft/document/g > > # Section 2.1 > > * "Algorithms identifiers" -> "Algorithm identifiers" > > # Section 2.2.2 > > * Length: variable > > Should this say "11-12" (1 + 1 + 6 + 3-4)? > [Les:] No. System ID may be a value from 1-8 octets in length (though in practice only the value 6 is used). I have clarified the text to mention that this field is of "ID Length" (as per ISO 10589). > * "set of Adj-SID each router" -> "set of Adj-SIDs each router", perhaps. > > # Section 2.3 > > s/valu eis/value is/ > > # Section 2.4 > > Silly, naive question: does the length include the sum of the octets > representing the sub-TLVs? > [Les:] Yes. TLV length includes all of the data contained in the TLV - including sub-TLVs. Les > # Section 2.4.6 > > In example 3, I would recommend s/0xD/0x0D/ & s/0x0/0x00/ & s/0x1/0x01/ > , > but perhaps that's just a personal readability thing. > > # Section 3.3 > > * "by other components than" -> "by components other than", perhaps. > > * "to know what are the local SIDs" -> "to know what the local SIDs are", > perhaps. > > * "The SRLB sub-TLV is used for this purpose...", (instead of "that purpose") > maybe. > > * "which mechanisms are outside" -> "which are outside", maybe. > > * "the SRLB TLV" -> "the SRLB sub-TLV", I think. >
