Skip to main content

Early Review of draft-ietf-idr-segment-routing-te-policy-18
review-ietf-idr-segment-routing-te-policy-18-rtgdir-early-boucadair-2022-07-08-00

Request Review of draft-ietf-idr-segment-routing-te-policy
Requested revision No specific revision (document currently at 20)
Type Early Review
Team Routing Area Directorate (rtgdir)
Deadline 2022-07-31
Requested 2022-06-21
Requested by Susan Hares
Authors Stefano Previdi , Clarence Filsfils , Ketan Talaulikar , Paul Mattes , Dhanendra Jain , Steven Lin
Draft last updated 2022-07-08
Completed reviews Secdir Early review of -18 by Vincent Roca (diff)
Intdir Early review of -18 by Brian Haberman (diff)
Rtgdir Early review of -18 by Mohamed Boucadair (diff)
Assignment Reviewer Mohamed Boucadair
State Completed
Review review-ietf-idr-segment-routing-te-policy-18-rtgdir-early-boucadair-2022-07-08
Posted at https://mailarchive.ietf.org/arch/msg/rtg-dir/sZbZ36ja8FaIh_s0yZwierbG28Q
Reviewed revision 18 (document currently at 20)
Result Has Issues
Completed 2022-07-08
review-ietf-idr-segment-routing-te-policy-18-rtgdir-early-boucadair-2022-07-08-00
Document: draft-ietf-idr-segment-routing-te-policy-18
Reviewer: Mohamed Boucadair
Review Date: 08/07/2022
IETF LC End Date: N/A
Intended Status: Standards Track

I appreciate the effort that was spent to progress this draft since more than 6
years!

Before reviewing the document, I started first by re-reading RFC8024/RFC9012
and reading draft-ietf-spring-segment-routing-policy for establishing the
context. Overall, the approach documented in
draft-ietf-idr-segment-routing-te-policy is sound and straightforward.

I didn’t find major concerns from a routing standpoint other than the need to
motivate some few claims (see the detailed review file about RRs, for example)
and the lack of considerations related to the handling of the various sub-TLVs
by intermediate routers (if any).

However, there are a number of generic issues that I would recommend to
consider (see the detailed review for the full list). All these are easy-to-fix
issues.

# General Comments (in no specific order)

## Consistency

### Single or multiple paths

There is an apparent inconsistency in the document about the handling of
multiple paths. For example, Section 1 says :"Selection of the best candidate
path for an SR Policy" while the same section says also “this will result in
one or more candidate paths being installed into ..”.

If multipath is supported, then please add an explicit statement and make sure
the overall text is consistent.

### Value 0 is marked as reserved for some registries, while that value is
associated with a meaning for other registries.

Is there any reason why a consistent approach isn’t followed here? what is the
issues if value 0 is open for assignment?

## Modifications to the format of the Color Extended Community

The text says that you are modifying the format the Color Extended Community,
while this is not true. What this draft does is just associating a meaning with
some bits. I would update the text accordingly.

## Normative language

The use of the normative language should be double-checked. The most apparent
concern is related the statement related to the handling of the reserved bits
(SHOULD) while this RFC9012 uses MUST (which is correct, IMO).

I tagged many others in the full review, fwiw.

## Lack of description

Many fields are provided without acceptable description (e.g., “Local IPv4
Address: a 4-octet IPv4 address.” or “Preference: a 4-octet value” !!).

Also, some fields are provided with a structure but the text says also that
these are reserved (e.g., 2.4.2 says “TC, S and TTL (Total of 12 bits) are
RESERVED”).

I wonder whether you can add a statement to say that multiple flags can be set
simultaneously unless this is precluded by future flag assignments.

Last, the document does not include the expected behavior of intermediate
routers (e.g., whether it is allowed or not to alter some fields). I guess,
they must not touch the content of the attributes but it is better if this is
explicitly mentioned in the text.

## Reserved vs. Unassigned

Almost all the “reserved” bits in the spec can be assigned in the future. I
would use “Unassigned” as per RFC8126.

FWIW, 8126 says the following:

      Unassigned:  Not currently assigned, and available for assignment
            via documented procedures.

      Reserved:  Not assigned and not available for assignment.
            Reserved values are held for special uses, such as to extend
            the namespace when it becomes exhausted.

## Deprecated values

The document includes notes about some “deprecated” codepoints. I’m not sure
there is a value in having such notes in the final RFC.

## IANA considerations

### The document uses a mix of TBD statements (e.g., Section 2.4.3) and
hard-coded values (early assignments). Not sure what’s was the rationale
especially that code 20 was assigned but not listed as such.

### The IANA actions should be more explicit and ask IANA to update existing
entries. For example, the current registry for code 73 points to
[draft-previdi-idr-segment-routing-te-policy]. Need to update that entry and
similar ones.

### The document lists (under IANA section) some values that are deprecated.
The document should be clear whether these codes are available for future
assignment or not.

### Many sub-TLVs have flag bits but not all of them have a registry to track
future flag bit assignments.

## Manageability considerations

No such considerations are included in the document.

# Detailed review

FWIW, you can find my full review at:

* pdf:
https://github.com/boucadair/IETF-Drafts-Reviews/raw/master/draft-ietf-idr-segment-routing-te-policy-18-rev%20Med.pdf
* doc:
https://github.com/boucadair/IETF-Drafts-Reviews/raw/master/draft-ietf-idr-segment-routing-te-policy-18-rev%20Med.doc