[spring] Robert Wilton's No Objection on draft-ietf-spring-srv6-network-programming-19: (with COMMENT)

[Robert Wilton via Datatracker <[e-mail]>]

Robert Wilton has entered the following ballot position for
draft-ietf-spring-srv6-network-programming-19: No Objection

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/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-spring-srv6-network-programming/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Hi,

Thank for this document.  Not surprisingly given the large number of
contributors working on this document I only have a few minor comments:

    2.  Terminology

       NH: Next-header field of the IPv6 header [RFC8200].  NH=SRH means
       that the next-header of the IPv6 header is Routing Header for
       IPv6(43) with the Type field set to 4.

Suggest expanding "4" to SRH(4)

    10.1.  Ethernet Next Header Type

       This document requests IANA to allocate, in the "Protocol Numbers"
       registry (https://www.iana.org/assignments/protocol-numbers/protocol-
       numbers.xhtml), a new value for "Ethernet" with the following
       definition: The value 143 in the Next Header field of an IPv6 header
       or any extension header indicates that the payload is an Ethernet
       [IEEE.802.3_2012].

Should this point to the latest published version of 802.3 (2018), or does it
specifically need to reference an older version?

    3.1.  SID Format

       This document defines an SRv6 SID as consisting of LOC:FUNCT:ARG,
       where a locator (LOC) is encoded in the L most significant bits of
       the SID, followed by F bits of function (FUNCT) and A bits of
       arguments (ARG).  L, the locator length, is flexible, and an operator
       is free to use the locator length of their choice.  F and A may be
       any value as long as L+F+A <= 128.  When L+F+A is less than 128 then
       the remainder of the SID MUST be zero.

This was reasonably clear to me, but since the rest of the description refers
to bits, then I wonder whether the last sentence would be more precise as "the
remaining bits of the SID must be set to zero"?

    4.1.  End: Endpoint
       S09.   If ((Last Entry > max_LE) or (Segments Left > Last Entry+1)) {

It wasn't entirely apparent to me where 'Last Entry' comes from, although that
became more clear when I saw its usage later.  Possibly it would be helpful to
list "Segments Left" and "Last Entry" in the terminology as taken from RFC 8754?

    4.1.  End: Endpoint
       S12.   Decrement Hop Limit by 1

Would it be more clear to expand this to say "Decrement IPv6 Hop Limit by 1",
since it is referred to as the IPv6 Hop Limit previously.  If this is changed,
then there are a couple of other places in the document that should be checked
as well.

    4.2.  End.X: Layer-3 Cross-Connect

       Note that if N has an outgoing interface bundle I to a neighbor Q
       made of 10 member links, N may allocate up to 11 End.X local SIDs:
       one for the bundle(LAG) itself and then up to one for each Layer-2
       member link.

This behaviour seems slightly unusual to me.  I presume that the intent here is
for the SR program to control which member link the traffic travels over rather
than the normal behaviour of relying on the Bundle hashing.  Possibly adding a
bit more text to clarify the expected behaviour might be helpful.

    4.4.  End.DX6: Decapsulation and IPv6 Cross-Connect

       S05.  If the End.DX6 SID is bound to an array of L3 adjacencies, then
       one entry of the array is selected based on the hash of the packet's
       header Section 7.

The end of this sentence doesn't scan for me, probably it should be something
end something like: ", see Section 7"

    6.  Counters

       A node supporting this document SHOULD implement a combined traffic
       counter (packets and bytes) per local SID entry, for traffic that
       matched that SID and was processed correctly.  The retrieval of these
       counters via MIB, NETCONF/YANG or any other means is outside the
       scope of this document.

>From my reading, a combined counter implies a single counter that is
incremented by both the number of packets processed and number of bytes
processed.  Normally in the management models you would normally expect this to
be modeled as a pair of counters, and I presume that is what is desired here? 
If so I would suggest clarifying this text to indicate that it is a pair of
counters (one for packets, one for bytes) that are required here.

    Section 4, various places.

At various points the Upper-Layer Header type is compared to a number (e.g. 4,
41, 143).  It might be a bit more readable if the associated names were also
included in the description.  E.g., 4(IPv4), 41(IPv6), etc.

    4.10.  End.DX2V: Decapsulation and VLAN L2 Table Lookup

Is this lookup just on the outermost VLAN Id, or all VLAN tags in the exposed
Ethernet header?  Or is this implementation specific?

Regards,
Rob