[Pce] Lars Eggert's Discuss on draft-ietf-pce-lsp-extended-flags-07: (with DISCUSS and COMMENT)

[Lars Eggert via Datatracker <[e-mail]>]

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