Hi Julien Many thanks for the speedy review! Please see a few answers below, marked with [Jon]. (All other comments are accepted.) I will hold the document mark-ups until WGLC ends.
Cheers Jon -----Original Message----- From: Julien Meuric [mailto:julien.meu...@orange.com] Sent: 21 November 2017 17:08 To: draft-ietf-pce-lsp-setup-t...@ietf.org Cc: pce@ietf.org Subject: Shepherd's Review of draft-ietf-pce-lsp-setup-type-06 Hi, Thank you for this new version of the I-D, it has greatly improved and clarifies former loose zones. Please find my review below. ------ Abstract --- - s/traffic engineering paths (TE paths)/Traffic Engineering paths (TE paths)/ - I wonder about the expansion of "TE path" above: why not "Engineered" instead of "Engineering"? (This is global to the I-D, and beyond... RFC Editor's list includes both.) - s/label switched paths (LSPs)/Label Switched Paths (LSPs)/ ------ Status --- - "https://"; was introduced in -05, but has now disappeared. ------ 1. Introduction --- - s/Path Computation Element Protocol/Path Computation Element communication Protocol/ - OLD path setup type needs to be either explicitly indicated or implied in the appropriate PCEP messages (when necessary)... NEW path setup type needs to be explicitly indicated in the appropriate PCEP messages, unless RSVP-TE type is meant (omission implying this type)... ------ 3. Path Setup Type Capability TLV --- - s/Initialization phase/initialization phase/ - I though the discussion on the list was about using a bitmap to identify supported PSTs: any reason why it is now a list of raw octets? [Jon] We did discuss a bit field on the list. The PST field is an 8-bit value, so a naive implementation of a bit field would use 32 bytes. This seems wasteful given we only have two values defined so far (possibly 3 with the up-coming IPv6-SR draft). I then looked at some schemes to shorten the bit field, e.g. by truncating it, but the result seemed more complicated to encode than just listing the path setup types. (I also didn't much like having a PST known both by a value and by a bit field position.) So I opted to list the PSTs. IMO it's not a problem given the low number of PSTs we expect (could be a case of famous last words!) What do you think of this tradeoff? - The definition of length and padding mixes the words "octet" and "bytes", depending on the field (probably to due to text coming from RFC 5440). Consistency would be welcome (the comment appears to be applicable beyond this section). [Jon] Let's standardize on bytes, per RFC 5440. ------ 4. Path Setup Type TLV --- - Figure 2 explicitly includes the codepoint for the "Type" (28), the "Length" field should be treated similarly (4). - The last sentence of section 4 puzzles me. If the PATH-SETUP-TYPE TLV is not supported, the PCEP peer cares little if it was "recognized" or not. If both sub-cases are commonly handled by ignoring, an implementation always including the RSVP-TE PST will be able to interwork with an implementation knowing about the TLV without actually supporting it; the current text turns this situation into an error. (Note also that RFC 5440 does not distinguish unrecognized and unsupported in TLV processing rules.) [Jon] I think you are right. The same also applies to the final sentence of section 3, I believe. - In case my previous comment does not fly, 3 more nits: * s/recognizes the TLV but does not support the TLV/recognizes the TLV but does not support its use/ * s/send PCErr/send a PCErr message/ * Error-Type is 2, would not 4 fit better? ------ 5. Operation --- - s/Initialization phase/initialization phase/ - s/MUST infer/MUST consider/ [explicit => nothing to infer] - The text says multiple times "unless the intended PST is RSVP-TE, in which case it MAY omit the PATH-SETUP-TYPE TLV". This is inconsistent with section 4: "It is RECOMMENDED that a PCEP speaker omits the TLV if the PST is RSVP-TE." Please choose between MAY and SHOULD, and align. [Jon] I think our intent is MAY, so we can reword the text in section 4 to "A PCEP speaker MAY omit the TLV if the PST is RSVP-TE." ------ Cheers, Julien _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce
