Re: [Idr] Document shepherd review for draft-ietf-idr-rfc7752bis

["Ketan Talaulikar (ketant)" <[e-mail]>]

Hi Jeff,

Given the cut-off date today, I've posted the update as per the proposals in this thread to give you and the WG an opportunity to review/comment.

https://datatracker.ietf.org/doc/html/draft-ietf-idr-rfc7752bis-09

Will look forward to your inputs.

Thanks,
Ketan

-----Original Message-----
From: Ketan Talaulikar (ketant) 
Sent: 21 October 2021 22:18
To: Jeffrey Haas <[e-mail]>; [e-mail]; [e-mail]
Subject: RE: Document shepherd review for draft-ietf-idr-rfc7752bis

Hi Jeff,

Thanks for your detailed review and apologies again for the delay in response.

Please check inline below.

-----Original Message-----
From: Jeffrey Haas <[e-mail]> 
Sent: 28 August 2021 01:31
To: [e-mail]; [e-mail]
Subject: Document shepherd review for draft-ietf-idr-rfc7752bis

Ketan and Working Group,

Here's my document shepherd review for RFC7752-bis.  

General assessment: Generally good shape, some concerns on error handling and consistency on NLRI keying.

-- Jeff


---

NLRI keying consistency:
There are a number of TLVs that are optional.  Some of these optional fields may be present in the NLRI.  These include:
- Autonomous System
- BGP-LS Identifier (deprecated)
- Multi-Topology ID

Additional optional TLVs may be distributed in the protocol in the future.

Consider two BGP-LS Producers.  Producer 1 emits no optional fields for a given piece of link state.  Producer 2 emits all of the optional fields.

A BGP-LS Consumer may receive two distinct pieces of information for the same topology data.  

There is no discussion in the document as to how such inconsistencies could be handled by the Consumer.
[KT] I agree with your point. How about we add the follow text towards the end of Sec 4.1 for both this one and the next comment? We want to cover this aspect but not really try to specify the behavior of a BGP-LS Consumer as that is outside the scope of this document.

<NEW>
When there are multiple BGP-LS Producers originating the same link-state information, implementation variations of BGP-LS Producers may result in the generation of different and inconsistent BGP-LS updates for the same link-state object based on the inclusion or exclusion of optional TLVs. An inconsistency between BGP-LS Producers with regards to the inclusion of optional TLVs in the NLRI results in multiple NLRIs being generated for the same link-state object. A BGP-LS Consumer would need the ability to merge such duplicate updates to handle such situations. An inconsistency between BGP-LS Producers with regards to the inclusion of optional TLVs in the BGP-LS Attribute results in one of them being delivered to a BGP-LS Consumer as part the BGP propagation and best-path selection procedures in most typical deployments. This can result in a BGP-LS Consumer missing out on some of the information in a potentially unpredictable manner. The use of BGP-LS Producers that have a consistent support for the origination of optional TLVs between them can help mitigate such situations for the BGP-LS Consumers.
</NEW> 

---

BGP-LS Attribute consistency:

Consider two BGP-LS Producers.  Producer 1 emits no optional fields for a given piece of link state.  Producer 2 emits all of the optional fields.
(This is a simplified example.  The critical detail is that the Path Attributes are not identical.)

In any case where the BGP-LS NLRI is identical, an implementation will use BGP route selection procedures to pick one of the possible paths and select it to forward to its peers.  In the absence of overriding policy features such as LOCAL_PREF or MED, this will often devolve to selecting the IGP distance to the BGP NEXT_HOP.  (Choose closest point of Truth.)

There's no discussion about how inconsistent information may be lost by having multiple producers that do not consistently produce their data.
[KT] Please check if the proposed text for the previous comment addresses this one as well.

---

The remainder of my comments have to do with Error Handling.  My intent is not to re-litigate prior decisions, it's to note that the consequences of those prior decisions are not adequately discussed in the document.

Section 7.2.2 does an admirable job in noting the usual things a BGP developer should look for in a specification. But mostly it says, no matter how bad the contents, move along:

 :    Each TLV MAY indicate the valid and permissible values and their
 :    semantics that can to be used only by a BGP-LS Consumer for its
 :    semantic validation.  However, the handling of any errors may be
 :    specific to the particular application and outside the scope of this
 :    document.  A BGP-LS Consumer should ignore unrecognized and
 :    unexpected TLV types in both the NLRI and BGP-LS Attribute portions
 :    and not consider their presence as an error.

While I must echo a common pithy comment from one of my protocol elders, "BGP is not a dump truck", in this case it is exactly that.  The expectation is that if an implementation has a series of well-formed and nested TLVs that follow their ordering rules, that implementation is to pass things on.

A significant portion of RFC 7606, BGP's Error Handling extensions, was intended to deal with what was called at the time "Optional-Transitive Nonsense".  Much of the motivation for the RFC was preventing a downstream BGP peer from resetting its session because an ignorant upstream router was propagating bad data.  Another portion of the motivation is that once you realize you have bad data, you shouldn't pass it along.

The strict TLV structuring of BGP-LS makes the most types of syntactic errors impossible.  However, BGP-LS describes a number of semantic validations using RFC 2119/8174 keywords without adequately describing what to do when those behaviors are violated at the BGP-LS Consumer.
[KT] The specification of behavior and handling at a BGP-LS Consumer was considered to be out of scope of this draft.

Simple examples of such things are:
- TLVs of fixed length that are not of the proper length.
[KT] This is covered in Section 7.2.2 for BGP-LS Speakers. It would be malformed and then handled differently based on whether that TLV is part of the NLRI or the BGP-LS Attribute.

- TLVs that are mandatory that are absent.
- TLVs that are not appropriate for a given NLRI type present in the attributes;
  e.g. Node properties for a Link NLRI.
- Metrics that are of inappropriate size for their underlying protocol
  semantics.
[KT] These all fall under the category of "semantic" verification and hence outside the scope of this draft - it is left to the specific BGP-LS Consumer application.

While there may be an urge to not address such issues as "this is the Consumer's problem", this misses a common bit of learned BGP wisdom: Such things are the nightmare of interoperability.  If you have two applications that are BGP-LS consumers that are intended to cooperate for implementing an application enabled by BGP-LS, and they have different error handling semantics, you may have incorrect operational behavior.
[KT] You are obviously correct. Given the nature of the protocol, building a system using different implementations that are cooperating in a sort of distributed manner is fraught with challenges. Unless of course, there are specifications written for those systems (i.e. specific BGP-LS Consumer apps) on how they should handle such issues. Therefore out of scope of this document. Let me take an example, a semantic error in a BGP-LS consumer app that is just showing the network visualization may not care as much about a wrong IGP metric while a BGP-LS consumer app such as SR-PCE would have problems with it. While the visualization app may just use whatever is reported or not even care, the SR-PCE implementation might just not consider that link to be "valid" and consider it unavailable. That is why, IMO, it seems to be a correct decision to not get into the BGP-LS Consumer specification. That said, I fully agree with the challenges here.

"Make the same mistakes everywhere" is not a great recipe.

I strongly recommend that this issue be addressed either in section 7.2.2 or in a new section.
[KT] Give, the previous comment, I am open to inputs on what can be incorporated into the document in this respect.

---------------------------------------------------------------------------------

Some comments on the draft text are noted below.  This is from the idnits output.

407	4.1.  TLV Format
[...]
435	   type MUST be in ascending order based on the value field.  Comparison
436	   of the value fields is performed by treating the entire field as an
437	   opaque hexadecimal string.  Standard string comparison rules apply.

I suspect what is intended here is opaque binary data rather than representing it in hexadecimal format.  "Standard string comparison" might make sense if you were doing strcmp() on an actual hexadecimal string.  The intent here is probably lexicographically ordered since "standard string comparison" isn't really defined here.
[KT] Yes, I believe so. How about:

<OLD>
Comparison of the value fields is performed by treating the entire field as an opaque hexadecimal string.  Standard string comparison rules apply. 
</OLD>
<NEW>
Comparison of the value fields is performed by treating the entire field as opaque binary data and ordered lexicographically.
</NEW>

447	   The TLVs within the BGP-LS Attribute MAY be ordered in ascending
448	   order by TLV type.  BGP-LS Attribute with unordered TLVs MUST NOT be
449	   considered malformed.

Probably "SHOULD be ordered"?  If it's MAY be ordered and MUST NOT be malformed, why mention it at all?
[KT] I am happy to drop both the sentences. The current text is more to clarify given the ambiguity in this regards in RFC7752.

451	4.2.  The Link-State NLRI

453	   The MP_REACH_NLRI and MP_UNREACH_NLRI attributes are BGP's containers
454	   for carrying opaque information.  This specification defines three
455	   Link-State NLRI types that describe either a node, a link, and a
456	   prefix.

"either a node, a link, or a prefix"
[KT] Ack
[...]

495	      0                   1                   2                   3
496	      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
497	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
498	     |            NLRI Type          |     Total NLRI Length         |
499	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
500	     |                                                               |
501	     +                       Route Distinguisher                     +
502	     |                                                               |
503	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
504	     |                                                               |
505	     //                  Link-State NLRI (variable)                 //
506	     |                                                               |
507	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

509	         Figure 6: Link-State VPN AFI 16388 / SAFI 72 NLRI Format

I'd suggest mentioning the length of the route distinguisher in the diagram rather than letting it be solely inferred by the text representation.
[KT] Ack

516	                   +------+---------------------------+
517	                   | Type | NLRI Type                 |
518	                   +------+---------------------------+
519	                   |  1   | Node NLRI                 |
520	                   |  2   | Link NLRI                 |
521	                   |  3   | IPv4 Topology Prefix NLRI |
522	                   |  4   | IPv6 Topology Prefix NLRI |
523	                   +------+---------------------------+

525	                            Table 1: NLRI Types

Note that the contents of this table do not match the IANA Considerations section.
[KT] I think you are referring to 0 and the Private Use? Do they need to match since the IANA is more about allocation and this is about listing what is being introduced in the document.


529	   The Node NLRI (NLRI Type = 1) is shown in the following figure.

531	      0                   1                   2                   3
532	      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
533	     +-+-+-+-+-+-+-+-+
534	     |  Protocol-ID  |
535	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
536	     |                           Identifier                          |
537	     |                            (64 bits)                          |
538	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
539	     //                Local Node Descriptors (variable)            //
540	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

542	                      Figure 7: The Node NLRI Format

The ASCII diagram covering Identifier isn't consistent with the multi-row format.  Compare vs. Figure 6's Route Distinguisher.
[KT] Fixed

This issue occurs multiple places further in the document.
[KT] Fixed

The syntax of the Identifier isn't clearly discussed in the document.  Is the intent to be a network-byte ordered uint64 number?
[KT] Yes. Fixed as : s/ The 64-bit Identifier field carries a BGP-LS Instance Identifier (Instance-ID)  / The Identifier field carries a 64-bit BGP-LS Instance Identifier (Instance-ID) number

[...]

753	   Autonomous System:  Opaque value (32-bit AS Number).  This is an
754	      optional TLV.  The value SHOULD be set to the AS Number associated
755	      with the BGP process originating the link-state information.  An
756	      implementation MAY provide a configuration option on the BGP-LS
757	      Producer to use a different value.

Some text discussing private AS numbers would be helpful.  The item of concern is when BGP-LS is used between cooperating ASes not under a single administrative control.  In such cases, avoiding private AS collisions is an edge case.
[KT] I am not sure if I've understood your point correctly, but how about add the following at the end of the current text. Please clarify or suggest text if you meant something different.

E.g., to avoid collisions when using private AS numbers.

[...]

795	      There can be at most one instance of each sub-TLV type present in
796	      any Node Descriptor.  The sub-TLVs within a Node Descriptor MUST
797	      be arranged in ascending order by sub-TLV type.  This needs to be
798	      done to compare NLRIs, even when an implementation encounters an
799	      unknown sub-TLV.  Using stable sorting, an implementation can do a
800	      binary comparison of NLRIs and hence allow incremental deployment
801	      of new key sub-TLVs.

I believe this section is intended to be outdented one level?
[KT] Yes. Fixed.

[...]

895	   The TLVs/sub-TLVs corresponding to the interface addresses and/or the
896	   local/remote identifiers may not always be signaled in the IGPs
897	   unless their advertisement is enabled specifically.  In such cases,
898	   it is valid to advertise a BGP-LS Link NLRI without any of these
899	   identifiers.

If there are multiple BGP-LS producers, can this happen if one producer is configured with this option and another is not?

If so, the same Link Descriptor may be generated with different NLRI representation.
[KT] The "advertisement is enabled" is referring to the IGP configuration. Once received, all BGP-LS producers will work the same with whatever they are able to get from the underlying IGPs. To provide more context, IGPs do not always advertise some of these identifiers unless features like TE or SR are enabled.

901	4.2.2.1.  Multi-Topology ID

903	   The Multi-Topology ID (MT-ID) TLV carries one or more IS-IS or OSPF
904	   Multi-Topology IDs for a link, node, or prefix.

906	   The semantics of the IS-IS MT-ID are defined in Section 7.1 and 7.2
907	   of RFC 5120 [RFC5120].  The semantics of the OSPF MT-ID are defined
908	   in Section 3.7 of RFC 4915 [RFC4915].  If the value in the MT-ID TLV
909	   is derived from OSPF, then the upper 5 bits of the MT-ID field MUST
910	   be set to 0.

912	   The format of the MT-ID TLV is shown in the following figure.

914	      0                   1                   2                   3
915	      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
916	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
917	     |              Type             |          Length=2*n           |
918	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
919	     |R R R R|  Multi-Topology ID 1  |             ....             //
920	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
921	     //             ....             |R R R R|  Multi-Topology ID n  |
922	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

924	                  Figure 12: Multi-Topology ID TLV Format

926	   where Type is 263, Length is 2*n, and n is the number of MT-IDs
927	   carried in the TLV.

929	   The MT-ID TLV MAY be present in a Link Descriptor, a Prefix
930	   Descriptor, or the BGP-LS attribute of a Node NLRI.  In a Link or
931	   Prefix Descriptor, only a single MT-ID TLV containing the MT-ID of
932	   the topology where the link or the prefix is reachable is allowed.
933	   In case one wants to advertise multiple topologies for a given Link
934	   Descriptor or Prefix Descriptor, multiple NLRIs MUST be generated
935	   where each NLRI contains a single unique MT-ID.  When used in the
936	   Link or Prefix Descriptor TLV for IS-IS, the Bits R are reserved and
937	   MUST be set to 0 (as per Section 7.2 of RFC 5120 [RFC5120]) when
938	   originated and ignored on receipt.

940	   In the BGP-LS attribute of a Node NLRI, one MT-ID TLV containing the
941	   array of MT-IDs of all topologies where the node is reachable is
942	   allowed.  When used in the Node Attribute TLV for IS-IS, the Bits R
943	   are set as per Section 7.1 of RFC 5120 [RFC5120].

The text above for OSPF MT-ID values (RFC 4915, §3.7) could be somewhat clearer.

The diagram above is structured for IS-IS.  The OSPF intent is BOTH that R is set to zero (the IS-IS text has cases where it may be zero, or potentially
non-zero) and also that the next 5 bits are also zero.  

Some clarity might be achieved by noting the MT-ID field for OSPF is 0..127.
[KT] Ack. Fixed.

988	   where the Type and Length fields of the TLV are defined in Table 5.
989	   The OSPF Route Type field values are defined in the OSPF protocol and
990	   can be one of the following:

992	   o  Intra-Area (0x1)
993	   o  Inter-Area (0x2)

995	   o  External 1 (0x3)

997	   o  External 2 (0x4)

999	   o  NSSA 1 (0x5)

1001	   o  NSSA 2 (0x6)

The semantics of these are from OSPF, but what's the matching RFC section reference for the code points?
[KT] These are actually BGP-LS code points. We don't anticipate new route types in OSPF (been so for decades) and that is why I skipped creating an IANA registry for them. But we can if that is the right thing to do.

1003	4.2.3.2.  IP Reachability Information

1005	   The IP Reachability Information TLV is a mandatory TLV for IPv4 &
1006	   IPv6 Prefix NLRI types.  The TLV contains one IP address prefix (IPv4
1007	   or IPv6) originally advertised in the IGP topology.  Its purpose is
1008	   to glue a particular BGP service NLRI, using its BGP next-hop, to a
1009	   given node in the LSDB.  A router SHOULD advertise an IP Prefix NLRI

"BGP service NLRI" isn't a well-defined thing.  I suggest defining it.
[KT] I will rather just delete that sentence since that is obviously not the main or even the sole purpose of the Prefix NLRI.

1013	      0                   1                   2                   3
1014	      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
1015	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1016	     |              Type             |             Length            |
1017	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1018	     | Prefix Length | IP Prefix (variable)                         //
1019	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

1021	             Figure 14: IP Reachability Information TLV Format

1023	   The Type and Length fields of the TLV are defined in Table 5.  The
1024	   following two fields determine the reachability information of the
1025	   address family.  The Prefix Length field contains the length of the
1026	   prefix in bits.  The IP Prefix field contains the most significant
1027	   octets of the prefix, i.e., 1 octet for prefix length 1 up to 8, 2
1028	   octets for prefix length 9 to 16, 3 octets for prefix length 17 up to
1029	   24, 4 octets for prefix length 25 up to 32, etc.

We have a possible need for additional pedantry here regarding "trailing bits".
RFC 4271, §4.3 defines the format for IPv4 NLRI.  It says:

 :      b) Prefix:
 :
 :         The Prefix field contains an IP address prefix, followed by
 :         the minimum number of trailing bits needed to make the end
 :         of the field fall on an octet boundary.  Note that the value
 :         of trailing bits is irrelevant.

A common fault in BGP implementations is failing to ignore the trailing bits for purposes of NLRI keying.  This particular bug tends to be less of a problem for AFI/SAFIs where multiple NLRI may be packed in the same UPDATE message.
Individual BGP Routes are unpacked as tuples of Path Attributes and their attached NLRI.

In BGP-LS, the IP Reachability Information TLV is a component that is not surrounded solely by other prefixes, it's embedded.  The procedures for BGP-LS push implementors toward simply doing binary comparison of various TLV contents.
Even more so, as discussed under my notes on error handling, the implementor is pushed to ignore most semantic validation errors.

The implication is that most developers are basically doing memcmp() for their comparison work.  Since "trailing bits" are irrelevant, but matter for comparison purposes, I strongly suggest text be added that trailing bits are zeroed.
[KT] Ack - agree.

1031	4.3.  The BGP-LS Attribute

1033	   The BGP-LS Attribute is an optional, non-transitive BGP attribute
1034	   that is used to carry link, node, and prefix parameters and
1035	   attributes.  It is defined as a set of Type/Length/Value (TLV)

It'd be useful to note that IANA has assigned code point 29 here.  It is noted in the IANA Considerations.
[KT] Ack


1044	   The size of the BGP-LS Attribute may potentially grow large depending
1045	   on the amount of link-state information associated with a single
1046	   Link-State NLRI.  The BGP specification [RFC4271] mandates a maximum
1047	   BGP message size of 4096 octets.  It is RECOMMENDED that an
1048	   implementation support [RFC8654] to accommodate a larger size of
1049	   information within the BGP-LS Attribute.  BGP-LS Producers MUST
1050	   ensure that they limit the TLVs included in the BGP-LS Attribute to
1051	   ensure that a BGP update message for a single Link-State NLRI does
1052	   not cross the maximum limit for a BGP message.  The determination of
1053	   the types of TLVs to be included MAY be made by the BGP-LS Producer
1054	   based on the BGP-LS Consumer applications requirement and is outside
1055	   the scope of this document.  When a BGP-LS Propagator finds that it
1056	   is exceeding the maximum BGP message size due to addition or update
1057	   of some other BGP Attribute (e.g.  AS_PATH), it MUST consider the
1058	   BGP-LS Attribute to be malformed and handle the propagation as
1059	   described in Section 7.2.2.

While discussing RFC 8654 is appropriate, a related deployment consideration should be mentioned:  If the BGP route propagation path is such that a large message (> 4096 octets) is created, it can only be sent along a contiguous path of BGP sessions that all support RFC 8654.

Since loss of messages can result in incomplete link state information, this may result in a broken LSDB.
[KT] Ack

1174	4.3.1.5.  Opaque Node Attribute TLV

1176	   The Opaque Node Attribute TLV is an envelope that transparently
1177	   carries optional Node Attribute TLVs advertised by a router.  An
1178	   originating router shall use this TLV for encoding information
1179	   specific to the protocol advertised in the NLRI header Protocol-ID
1180	   field or new protocol extensions to the protocol as advertised in the
1181	   NLRI header Protocol-ID field for which there is no protocol-neutral
1182	   representation in the BGP Link-State NLRI.  The primary use of the
1183	   Opaque Node Attribute TLV is to bridge the document lag between,
1184	   e.g., a new IGP link-state attribute being defined and the protocol-
1185	   neutral BGP-LS extensions being published.

This, and the related Opaque sections could use some discussion about what happens when that bridge has been crossed.

Consider a feature that can move from a new link-state attribute carried in the Opaque information and has a code point later added to carry in BGP-LS:  There's possibly an incremental deployment motivation to carry the information in both spots.  This means keeping the values consistent in each place becomes important.
[KT] Ack. How about adding the following?

<NEW>
Once the protocol-neutral BGP-LS extensions are defined, the BGP-LS implementations would still need to advertise the information both within the Opaque Attribute TLV and the new TLV definition for incremental deployment and transition.
</NEW>

1273	      0                   1                   2                   3
1274	      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
1275	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1276	     |              Type             |             Length            |
1277	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1278	     |L|R|  Reserved |
1279	     +-+-+-+-+-+-+-+-+

1281	                     Figure 19: MPLS Protocol Mask TLV

Here, and later in the document, "Reserved" likely should be "MUST be set to zero and SHOULD be ignored on receipt".
[KT] Ack


1451	      0                   1                   2                   3
1452	      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
1453	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1454	     |              Type             |             Length            |
1455	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
1456	     |D|N|L|P|       |
1457	     +-+-+-+-+-+-+-+-+

1459	                      Figure 25: IGP Flag TLV Format

Suggest marking the empty space as Reserved.  See prior comment about Reserved.
[KT] Ack


2032	6.1.4.  BGP-LS Node Flags Registry

2034	   The "BGP-LS Node Flags" registry is requested to be created for the
2035	   one-octet sized flags field of the Node Flag Bits TLV (1024) and
2036	   populated with the initial values shown below:

2038	              Bit   Description             Reference
2039	              -----------------------------------------------
2040	               0    Overload Bit (O-bit)    [This document]
2041	               1    Attached Bit (A-bit)    [This document]
2042	               2    External Bit (E-bit)    [This document]
2043	               3    ABR Bit (B-bit)         [This document]
2044	               4    Router Bit (R-bit)      [This document]
2045	               5    V6 Bit (V-bit)          [This document]
2046	               6-7  Unassigned

2048	   Allocations within the registry under the "RFC Required" policy (see
2049	   [RFC8126]).

This registry is absent from the IANA Considerations.  Similarly, IANA does not have a registry in the expected place:
https://www.iana.org/assignments/bgp-ls-parameters/bgp-ls-parameters.xhtml
[KT] Yes, this was missed by RFC7752 and we are fixing this now by creating it.

2051	6.1.5.  BGP-LS MPLS Protocol Mask Registry

2053	   The "BGP-LS MPLS Protocol Mask" registry is requested to be created
2054	   for the one-octet sized flags field of the MPLS Protocol Mask TLV
2055	   (1094) and populated with the initial values shown below:

2057	    Bit   Description                                Reference
2058	    ------------------------------------------------------------------
2059	     0    Label Distribution Protocol (L-bit)        [This document]
2060	     1    Extension to RSVP for LSP Tunnels (R-bit)  [This document]
2061	     2-7  Unassigned

2063	   Allocations within the registry under the "RFC Required" policy (see
2064	   [RFC8126]).

See prior comment about missing IANA Considerations and registry at IANA.
[KT] Same as previous.

2066	6.1.6.  BGP-LS IGP Prefix Flags Registry

2068	   The "BGP-LS IGP Prefix Flags" registry is requested to be created for
2069	   the 1 octet sized flags field of the IGP Flags TLV (1152) and
2070	   populated with the initial values shown below:

2072	        Bit   Description                        Reference
2073	        ----------------------------------------------------------
2074	         0    IS-IS Up/Down Bit (D-bit)          [This document]
2075	         1    OSPF "no unicast" Bit (N-bit)      [This document]
2076	         2    OSPF "local address" Bit (L-bit)   [This document]
2077	         3    OSPF "propagate NSSA" Bit (P-bit)  [This document]
2078	         4-7  Unassigned

2080	   Allocations within the registry under the "RFC Required" policy (see
2081	   [RFC8126]).

See prior comment about missing IANA Considerations and registry at IANA.
[KT] Same as previous.

Thanks,
Ketan