Lars Eggert has entered the following ballot position for draft-ietf-pce-lsp-extended-flags-07: Discuss
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/about/groups/iesg/statements/handling-ballot-positions/ for more information about how to handle DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-pce-lsp-extended-flags/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- # GEN AD review of draft-ietf-pce-lsp-extended-flags-07 CC @larseggert Thanks to Roni Even for the General Area Review Team (Gen-ART) review (https://mailarchive.ietf.org/arch/msg/gen-art/J2jEQIJoyAsZEbtU5IK14E3yROA). ## Discuss ### 2119 terms This document has some ambiguities that would be clarified by using RFC2119 terms in a few more places: #### Section 3.2, paragraph 2 ``` - Note that PCEP peers MAY encounter varying lengths of the LSP- - ^^ -------- + Note that PCEP peers MUST handle varying lengths of the LSP- + ^^^ +++++ ``` #### Section 3.2, paragraph 3 ``` - than it currently supports or understands, it will simply ignore the - ^^^^^^^^^^^ + than it currently supports or understands, it MUST ignore the + ^^^^ ``` #### Section 3.2, paragraph 4 ``` - than the one supported by the implementation, it will consider the - ^^^^ ^^^^^^ ^ + than the one supported by the implementation, it MUST treat the + ^^^^ ^^ ^^ ``` #### Section 5, paragraph 2 ``` - not understood by an implementation would be ignored. It is expected - ^^^^^ + not understood by an implementation MUST be ignored. It is expected + ^^^^ ``` ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- ## Comments ### Section 3.1, paragraph 7 ``` LSP Extended Flags: this contains an array of units of 32-bit flags numbered from the most significant as bit zero, where each bit represents one LSP flag (for operation, feature, or state). Currently no bits are assigned. Unassigned bits MUST be set to zero on transmission and MUST be ignored on receipt. ``` Should this document give some encoding guidance, e.g., "the LSP Extended Flags field SHOULD (MUST?) use the minimal amount of space needed to encode the flag bits"? Otherwise one would be free to have trailing 32-bit values with all flags zero. ### Section 3.2, paragraph 2 ``` The LSP Extended Flags field is an array of units of 32 flags, to be allocated starting from the most significant bit. The bits of the LSP Extended Flags field will be assigned by future documents. This document does not define any flags. Unassigned flags MUST be set to zero on transmission and MUST be ignored on receipt. Implementations that do not understand any particular flag MUST ignore the flag. ``` What "unassigned flags" are will change as assignments are made, so this text would require implementations to (closely) track IANA assignments. Did you maybe mean "flags that an implementation is not supporting" instead? ### Section 4, paragraph 2 ``` Following the model provided in [RFC8786] Section 3.1, we provide the following advice for new specifications that define additional flags. Each such specification is expected to describe the interaction between these new flags and any existing flags. In particular, new specifications are expected to explain how to handle the cases when both new and pre-existing flags are set. They are also expected to discuss any security implications of the additional flags (if any) and their interactions with existing flags. ``` I think you mean "describe the interaction between these new flags and *all existing assigned* flags". That still leaves the issue of what to do when two documents simultaneously define new flags; that would need to be caught during standardization. ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### Grammar/style #### Section 3.1, paragraph 2 ``` or operation, feature, or state). Currently no bits are assigned. Unassigned ^^^^^^^^^ ``` A comma may be missing after the conjunctive/linking adverb "Currently". ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool _______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce