[Idr] Shepherd report for draft-ietf-idr-sr-te-policy-attr-01

[Susan Hares <[e-mail]>]

Summary: Needs review
Reviewer: Susan Hares

Draft: draft-ietf-idr-sr-te-policy-attr-01
Type:  Proposed Standard
status: WG Draft
adopted:  8/12/2024 (adoption call: 7/12/2024 - 7/30/2024)
current version:
**Early Allocation**: yes, need early allocation based on [draft-ietf-idr-sr-policy-safi]
**implementations:** H3C and ZTE (2 implementations)
**bgp-ls draft:** none (would need to augment draft-ietf-idr-bgp-ls-sr-policy)

Wiki pointer: https://wiki.ietf.org/e/en/group/idr/Shepherd-SR-BGP-LS/BGP-SR/draft-ietf-idr-sr-policy-te-policy-attr


Detailed review
----------
Technical issues:
Issue-1: Section 1, paragraph 2, description of BGP attribute

You need to make clear which NLRI and BGP attributes passed include the Tunnel Encapsulation Attribute.

Old text:/
In UPDATE messages of that address family, the NLRI identifies an SR Policy Candidate Path,
and the attributes encode the segment lists and other details of that SR Policy Candidate Path./

New text:/
BGP UPDATE message send the NLRI identifies an SR Policy Candidate Path,
and the attributes encode the segment lists and other details of that SR Policy Candidate Path.
These BGP attributes include the Tunnel Encapsulation attribute [RFC9012] which encodes
SR Policy tunnel information in an SR Policy Tunnel  TLV (per [I-D.ietf-idr-sr-policy-safi]).

Issue-2:  Sub-TLV template checklist check:

Please provide this information in your draft:

1.  Can this Sub-TLV go in any other tunnel TLVs in the Tunnel-Encapsulation Attribute?

My understanding is that this draft (like draft-ietf-idr-sr-policy-safi) specifies
that segment lists with these types are only defined for the Segment list Sub-TLV
are only defined to be used by the SR Policy Tunnel TLV.  All other usages are outside
the scope of this document.

Some statement like this needs to go in your document.

2.  How do these segment types impact validation of the BGP route (NLRI + Attribute pair)?  Can the segment types repeat?

Just like [draft-ietf-idr-sr-policy-safi], the new segment types are checked syntactically, but
not semantically.   The SRPM in the head-end does the semantic checking to determine the
values make sense for deployments.  If so, I suggest the following change

Section 3 text:
old text: / The processing procedures for SID with algorithm specified in
[RFC9256] and [I-D.ietf-idr-bgp-sr-segtypes-ext] are still applicable for the new segment types./

New text: /  The processing procedures for SID with algorithm specified in
[RFC9256] and [I-D.ietf-idr-bgp-sr-segtypes-ext] are still applicable for the new segment types.
Just as in [drafat-ietf-idr-sr-policy-safi], the segment list sub-TLV specified in this document
(sections 3.1, 3.2, 3.3, and 3.4) may repeat multiple times within the segment-list Sub-TLV.

BGP only checks the syntax of the fields, but the semantic meaning is check by the consumer. /

Issue-3: Need update to security section

The details in the security section need to be expanded.  [draft-ietf-idr-sr-policy-safi]
specifies information on the critical information that it passes.  You need to extend this
to the information included in the new segment types.

Issue-4: Need to have a management section

Would it be helpful for the BGP-LS or a Yang model help manage the creation of segments by the
headend? For example, does draft-ietf-bgp-ls-sr-policy in section 5.7.1.1 need to be
extended to include the segment list types in this document.

Do the authors recommend the creation of Yang module for managing the creation
of segment types?



Editorial NITS
============
NIT-1, introduction, paragraph 4, add space before (

old text:/
As specified in [RFC9256], the SR algorithm can be optionally specified for Segment Types C(IPv4 Node and SID), D(IPv6 Node and SID for SR-MPLS), I(IPv6 Node and SID for SRv6), J(IPv6 Node, index for remote and local pair, and SID for SRv6), and K(IPv6 Local/Remote addresses and SID for SRv6)./

New text:/
As specified in [RFC9256], the SR algorithm can be optionally specified for Segment Types C (IPv4 Node and SID), D (IPv6 Node and SID for SR-MPLS), I (IPv6 Node and SID for SRv6), J (IPv6 Node, index for remote and local pair, and SID for SRv6), and K (IPv6 Local/Remote addresses and SID for SRv6)./


NIT-2, section 3.1, paragraph 1, add

Old text:/
This type allows for identification of an Adjacency SID or BGP Peer Adjacency SID
(as defined in [RFC8402] ) SR-MPLS label for point-to-point links including IP unnumbered links./

Change /(as defined in [RFC8402] )/ to /(as defined in [RFC8402])/