Advertising Segment Routing Policies in BGP
draft-ietf-idr-sr-policy-safi-13

I'd recommend either adding an acronym/terminology section, or pointing to one in another rfc.
   - Terms like NLRI should be expanded upon first use.  
   - Definitions of 'color extended community' would be lovely.

# Internet AD comments for draft-ietf-idr-sr-policy-safi-10
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

### S2.1, S3

* Policy Color: might be worth a reminder that RFC 9256 says that SR Policy
  color values must be non-zero.

# Gunter Van de Velde, RTG AD, comments for draft-ietf-idr-sr-policy-safi-11

# Many thanks for to Zhaohui 'Jeffrey' Zhang and Mohamed 'Med' Boucadair for the RTGDIR reviews

# Many thanks to the authors to fix the blocking DISCUSS and resolve/responded to the non-blocking observations

Thanks for adding a separate manageability section to outline how the feature is going to be managed. And thanks to Roman for catching the fact that the referenced YANG module draft had expired.

Looking at draft-ietf-spring-sr-policy-yang and granted it is still a draft, I could not help noticing the paucity of statistics being maintained for the feature supported by this draft. Granted, this comment is more for the other draft, but I wonder if enough thought has been given to what would be required to manage the feature in the network. For example, these are the only counters associated with policies that I could find.

         description
           "Counters containing stats related to forwarding";

         leaf pkts {
           type yang:counter64;
           description "Number of packets forwarded";
         }
         leaf octets {
           type yang:counter64;
           units "byte";
           description "Number of bytes forwarded";
         }

Description or lack of it aside, what it appears to be counting is the number of packets or octets forwarded when the policy is successfully installed. How do these stats help with debugging if the policy was learned by the headend in the first place or the number of candidate paths it learned?

# Éric Vyncke, INT AD, comments for draft-ietf-idr-sr-policy-safi-12
CC @evyncke

Thank you for the work put into this document.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and some nits.

Special thanks to Susan Hares for the shepherd's very terse write-up including the WG consensus and some justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS (non-blocking)

### Section 1

Unsure whether the meaning of "headend node" is well understood, it appears in RFC 8402 without any explanation though.

Who is the "we" in `While for simplicity we may write` ? The authors ? The WG ? The IETF ? The operator ? Let's be accurate in defined the subject of this sentence.

Bear with my lack of expertise about BGP... about `One or more IPv4 address-specific format route target extended community`, and knowing that section 3.2 of RFC 4360 only supports IPv4, does this mean that this specification supports only IPv4 "headends" or is this simply a 32-bit identifier and not an IPv4 locator?

### Section 2.2

In order to be consistent with "SRv6 Binding SID" suggest s/Binding SID/SR-MPLS Binding SID/ this will also remove any ambiguity.

### Section 2.4

Unsure whether the paragraph starting with `An early version of this document included` is really useful once this I-D is published. Consider removing it.

### Section 2.4.3

This may explain why the term "Binding SID" is used (cfr my comment about section 2.2)... as the "binding SID" can either be for SR-MPLS or for SRv6. But, it is unclear to me how to make a choice between the 2 TLV when sending a SRv6 BSID without the optional "Endpoint Behavior and SID Structure" beyond the 'RECOMMENDED' of section 2.4.2.

### Section 2.4.5

s/into an SR policy/into an SR-MPLS policy/ as this sub-TLV applies only to SR-MPLS ?

## NITS (non-blocking / cosmetic)

### Use of SVG graphics

To make a much nicer HTML rendering, suggest using the aasvg too to generate SVG graphics. It is worth a try ;-)

The IANA questions in the shepherd writeup were not completed (they're marked "[NA]").

Why is the SHOULD in Section 2.4.5 there?  What guidance do we have to implementers around deciding whether to do what it says?