Speaking as document shepherd: Hi Med,
See inline. > On Mar 25, 2025, at 4:28 AM, Mohamed Boucadair via Datatracker > <nore...@ietf.org> wrote: > > Mohamed Boucadair has entered the following ballot position for > draft-ietf-lsr-ospf-prefix-extended-flags-06: Yes > > 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-lsr-ospf-prefix-extended-flags/ > > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Hi Ran, Detao, Peter, Ketan, and Liyan, > > Thank you for the effort put into this specification. This is an important > piece of work. > > I definitely support this. I have some few comments that I’d like to be > addressed, especially the first three comments and the comment in Section 3. > > # General > > ## Should we have a recommendation whether the remaining flags are assigned > first vs. use of the sub-TLV? I think we need a guidance (not rigid, though). I don't even know what you mean by this question - I doubt the authors do. > > ## Not sure if this is assumed, but is it allowed that a group of bits (e.g., > 2 > bits) may be allocated for one single purpose? The current description seems > to > assume that flags will be allocated individually. This is not limited by the specification so if one were to define a Trid (i.e., 3 states), it could take 2 bits. However, the draft that defines this should deal with that. It need not be here. > > ## Should we have configuration parameters to control the use of the flags > (e.g., rfc8362#appendix-A)? No. Configuration will be described in the future documents that define the bits. Thanks, Acee > > ## (nit) Please use terms to be consistent with the use in RFC8362 (sub-TLV, > etc.). > > ## (nit) Use explicit references (with sections) to help readers to find where > to look. > > ## (nit) Be consistent with the IANA registry names > > # Abstract: Please indicate that the sub-TLV applies for both versions > > OLD: > Each OSPF prefix can be advertised with an 8-bit field to indicate > specific properties of that prefix. However, all the OSPFv3 Prefix > Options bits have already been assigned and only a few bits remain > unassigned in the flags field of the OSPFv2 Extended Prefix TLV. > > This document solves the problem of insufficient prefix options bits > by defining variable-length Prefix Attribute Flags Sub-TLV for OSPF. > > NEW: > Each OSPF prefix can be advertised with an 8-bit field to indicate > specific properties of that prefix. However, all the OSPFv3 Prefix > Options bits have already been assigned and only a few bits remain > unassigned in the flags field of the OSPFv2 Extended Prefix TLV. > > This document solves this problem by defining variable-length Prefix > Attribute Flags sub-TLV for OSPF. This sub-TLV is applicable to OSPFv2 > and OSPFv3. > > # Section 2: > > ## (nits) Fix several nits > > OLD: > This document defines variable-Length Prefix Attribute Flags Sub-TLVs > for OSPFv2 and OSPFv3. These Sub-TLVs specify the variable-flag > fields to advertise additional attributes associated with OSPF > prefixs i.e., the advertisement and processing of the existing fixed- > size prefix attribute flags remains unchanged. > > NEW: > This document defines variable-Length Prefix Attribute Flags sub-TLV > for OSPFv2 and OSPFv3. Such Sub-TLV specifies the variable-flag > fields to advertise additional attributes associated with OSPF > prefixes. The advertisement and processing of the existing fixed- > size prefix attribute flags remain unchanged. > > ## (nit) Add caption and call them in the text. For example, > > OLD: > The format of OSPFv2/OSPFv3 Prefix Attribute Flags Sub-TLVs is: > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Type | Length | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | | > // Prefix Attribute Flags (Variable) // > | | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > NEW: > The format of OSPFv2/OSPFv3 Prefix Attribute Flags sub-TLV is shown in > Figure 1. > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Type | Length | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | | > // Prefix Attribute Flags (Variable) // > | | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > Figure 1: Format of OSPFv2/OSPFv3 Prefix Attribute Flags Sub-TLV > > ## I don’t parse what is meant by “defined prefix flags” in the following: > > CURRENT: > Defined prefix flags that are not > transmitted due to being beyond the transmitted length MUST be > treated as being set to 0. > > # Should the event covered in this part be logged? > > CURRENT: > When multiple instances of an OSPFv2/OSPFv3 Prefix Attribute Flags > Sub-TLVs are received within the same TLV, an implementation MUST use > only the first occurrence of the Sub-TLV and MUST ignore all > subsequent instances of the Sub-TLV. > > (nit) Maybe better to reword as follows: > > NEW: > When multiple instances of the OSPFv2/OSPFv3 Prefix Attribute Flags > sub-TLV are received within the same TLV, an implementation MUST use > only the first occurrence of the sub-TLV and MUST ignore all > subsequent instances of the sub-TLV. > > # Section 3: MUST seems redundant with “Unrecognized TLVs and sub-TLVs are > ignored” stated in rfc8362#section-6 > > CURRENT: > The Prefix Attribute Flags Sub-TLVs defined in this document does not > introduce any backward compatibility issues. An implementation that > does not recognize the OSPFv2/OSPFv3 Prefix Attribute Flags Sub-TLV > MUST ignore the Sub-TLV. > > (nit) We may consider: s/The Prefix Attribute Flags Sub-TLVs defined in this > document/The Prefix Attribute Flags sub-TLV does not > > # Section 5: We may add subsections for each OSPF version to better organize > the actions (and also avoid orphan subsections): > > OLD: > 5.1. OSPFv2 Prefix Attribute Flags Sub-TLV Registry > 5.1.1. OSPFv2 Prefix Extended Flags Field Registry > 5.2. OSPFv3 Prefix Attribute Flags Sub-TLV Registry > 5.2.1. OSPFv3 Prefix Extended Flags Field Registry > > NEW > 5.1. OSPFv2 > 5.1.1. OSPFv2 Prefix Attribute Flags Sub-TLV Registry > 5.1.2. OSPFv2 Prefix Extended Flags Field Registry > 5.2. OSPFv3 > 5.2.1. OSPFv3 Prefix Attribute Flags Sub-TLV Registry > 5.2.2. OSPFv3 Prefix Extended Flags Field Registry > > # FWIW, the full review with other minor edits/nits can be found at: > > * pdf: > https://github.com/boucadair/IETF-Drafts-Reviews/blob/master/2025/draft-ietf-lsr-ospf-prefix-extended-flags-06-rev%20Med.pdf > * doc: > https://github.com/boucadair/IETF-Drafts-Reviews/blob/master/2025/draft-ietf-lsr-ospf-prefix-extended-flags-06-rev%20Med.doc > > Feel free to grab whatever useful there. > > Cheers, > Med > > > _______________________________________________ Lsr mailing list -- lsr@ietf.org To unsubscribe send an email to lsr-le...@ietf.org
