Subject: [Pce] Alvaro Retana's No Objection on draft-ietf-pce-lsp-setup-type-09: (with COMMENT)

From: Alvaro Retana <[email-masked]>
Date: Mon, 02 Apr 2018 11:18:58 -0700

Alvaro Retana has entered the following ballot position for
draft-ietf-pce-lsp-setup-type-09: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-pce-lsp-setup-type/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

(1) The Length field in S3 has no units.  I'm sure people can guess it is in
bytes, from the rest of the description, but it should be explicit.

(2) The Reserved fields "MUST be set to zero".  What happens if they're not? 
Typically they are also ignored by the receiver...

(3) S3: "Each sub-TLV MUST obey the rules for TLV formatting defined in
([RFC5440]).  That is, each sub-TLV MUST be padded to a four byte alignment,
and the length field of each sub-TLV MUST NOT include the padding bytes."  The
first sentence is ok.  The second one tries to paraphrase what rfc5440 says --
but rfc5440 doesn't say that, it doesn't even use Normative language!  This is
the text from rfc5440:

   The Length field defines the length of the value portion in bytes.
   The TLV is padded to 4-bytes alignment; padding is not included in
   the Length field (so a 3-byte value would have a length of 3, but the
   total size of the TLV would be 8 bytes).

(3a) The text in this document shouldn't use Normative language to describe
what rfc5440 says and specifies.

(3b) Note that the text from rfc5440 (specifically the part about "padding is
not included in the Length field") is not aligned with the description of the
Length field in this document: "The TLV Length field MUST be equal to the size
of the appended sub-TLVs plus the size of the PST list (rounded up to the
nearest multiple of four) plus four bytes."  Rounding up includes the padding.

(4) S6: "Each document that introduces a new path setup type to PCEP must
include a manageability section."  Why is a Normative "MUST" not used?

(5) rfc6123 is a Historic document.  Maybe a reference to rfc5706 is more
appropriate (even in addition to rfc6123).

---

From: Jonathan Hardwick <[email-masked]>
Date: Mon, 16 Apr 2018 14:38:39 +0000

Thanks for the comments, Alvaro.  Please see [Jon] below.

Cheers
Jon

-----Original Message-----
From: Alvaro Retana [mailto:[email-masked]] 
Sent: 02 April 2018 19:19
To: The IESG <[email-masked]>
Cc: [email-masked]; Julien Meuric <[email-masked]>; [email-masked]; [email-masked]; [email-masked]
Subject: Alvaro Retana's No Objection on draft-ietf-pce-lsp-setup-type-09: (with COMMENT)

<snip>

(1) The Length field in S3 has no units.  I'm sure people can guess it is in bytes, from the rest of the description, but it should be explicit.

[Jon] OK, fixed.

[Jon] New text: "Its reserved field MUST be set to zero by the sender and MUST be ignored by the receiver."

(3) S3: "Each sub-TLV MUST obey the rules for TLV formatting defined in ([RFC5440]).  That is, each sub-TLV MUST be padded to a four byte alignment, and the length field of each sub-TLV MUST NOT include the padding bytes."  The first sentence is ok.  The second one tries to paraphrase what rfc5440 says -- but rfc5440 doesn't say that, it doesn't even use Normative language!  This is the text from rfc5440:

(3a) The text in this document shouldn't use Normative language to describe what rfc5440 says and specifies.

(3b) Note that the text from rfc5440 (specifically the part about "padding is not included in the Length field") is not aligned with the description of the Length field in this document: "The TLV Length field MUST be equal to the size of the appended sub-TLVs plus the size of the PST list (rounded up to the nearest multiple of four) plus four bytes."  Rounding up includes the padding.

[Jon] Yes, this area is a bit awkward, and you are correct to point out that this wording is flawed.  I agree that any trailing padding bytes ought not to be included in the length field, for consistency with the rules in RFC 5440.  However, we do have to include any intermediate padding bytes in the length.  For example, if the TLV looked like this:

[TLV] [padding1] [Sub-TLV A] [padding2] [Sub-TLV B] [padding3]

[padding1] pads the PST list to a multiple of 4 bytes.
[padding2] pads sub-TLV A to a multiple of 4 bytes.
[padding3] pads sub-TLV B to a multiple of 4 bytes.

The overall TLV length needs to include padding1 and padding2, but not padding 3.  The reason: an implementation not recognising this TLV needs to be able to skip over it and ignore it.  If the overall length did not include padding1 or padding2 then that implementation would skip right into the middle of the sub-TLV list and get lost.

I propose the following text, which hopefully addresses your comment.

OLD
  That is, each sub-TLV MUST be padded to a four byte alignment, and the length field of each sub-TLV MUST NOT include the padding bytes.
  [...]
  The TLV Length field MUST be equal to the size of the appended sub-TLVs plus the size of the PST list (rounded up to the nearest multiple of four) plus four bytes.
NEW
  That is, each sub-TLV is padded to a four byte alignment, and the length field of each sub-TLV does not include the padding bytes.
  [...]
  If there are no sub-TLVs, then the TLV length field MUST be equal to four bytes plus the size of the PST list, excluding any padding bytes.  If there are sub-TLVs then the TLV Length field MUST be equal to four bytes plus the size of the PST list (rounded up to the nearest multiple of four) plus the size of the appended sub-TLVs excluding any padding bytes in the final sub-TLV.
END

(4) S6: "Each document that introduces a new path setup type to PCEP must include a manageability section."  Why is a Normative "MUST" not used?

[Jon] This is a requirement on document writers, not on implementations, so I thought a normative MUST would not be appropriate here.

(5) rfc6123 is a Historic document.  Maybe a reference to rfc5706 is more appropriate (even in addition to rfc6123).

[Jon] Yes, I agree. Fixed.