Bonjour François,

Thanks for your prompt reply and revised I-D.

All changes in the diff are fine for me. You have a fair comment on the use of 
documentation prefix vs. the SRv6 prefix. I think the document is better now 
with updated section 7.2 and 9.5.

Regards

-éric

From: Francois Clad <fclad.i...@gmail.com>
Date: Tuesday, 4 February 2025 at 22:47
To: Eric Vyncke (evyncke) <evyn...@cisco.com>
Cc: draft-ietf-spring-srv6-srh-compress...@ietf.org 
<draft-ietf-spring-srv6-srh-compress...@ietf.org>, spring-cha...@ietf.org 
<spring-cha...@ietf.org>, spring@ietf.org <spring@ietf.org>, Pablo Camarillo 
(pcamaril) <pcama...@cisco.com>, benson_mu...@emailplus.org 
<benson_mu...@emailplus.org>, The IESG <i...@ietf.org>
Subject: Re: Éric Vyncke's No Objection on 
draft-ietf-spring-srv6-srh-compression-19: (with COMMENT)
Dear Eric,

Many thanks for reviewing this document.

As noted in my reply to Erik, I am talking with my co-authors about your 
comment on Section 4.2 and will get back to you soon.

We also made several changes in revision -20 to address the remaining points of 
your review. These changes are listed below.

Thanks,
Francois

On 3 Feb 2025 at 18:44:45, Éric Vyncke via Datatracker 
<nore...@ietf.org<mailto:nore...@ietf.org>> wrote:
Éric Vyncke has entered the following ballot position for
draft-ietf-spring-srv6-srh-compression-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/about/groups/iesg/statements/handling-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.


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



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


# Éric Vyncke, INT AD, comments for draft-ietf-spring-srv6-srh-compression-19
CC @evyncke

Thank you for the work put into this document. While the content is rather
complex, it is also easy to read.

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

Special thanks to Pablo Camarillo for the shepherd's *very detailed* write-up
including the WG consensus *and* the justification of the intended status.

Please note that Benson Muite is the Internet directorate reviewer (at my
request) and you may want to consider this int-dir review as well when it will
be available (no need to wait for it though):
https://datatracker.ietf.org/doc/draft-ietf-spring-srv6-srh-compression/reviewrequest/21238/

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS (non-blocking)

### Erik Kline's comment about section 4.2

I support Erik Kline's DISCUSS about some CSID containers not being valid IPv6
addresses, i.e., not compliant to RFC 8754 (requiring that this document
updates RFC 8754).

### Title

Suggest adding the acronym CSID in the title (e.g., like in RFC 7599, 9631, and
others) this will help search engines.

We added "(CSID)" at the end of the document title in revision -20.


### Section 1

Please expand CSID on first use.

We rephrased the sentence in Section 1 to avoid using the term CSID (or its 
expanded version) before it is defined.

| This document also specifies new
| SRv6 endpoint behaviors to preserve the compression efficiency in
| multi-domain environments.

The first occurrence of CSID is now in the terminology section and properly 
expanded.


### Section 2

Suggest adding the extension of SID at first use.

We fixed this in revision -20.


I wonder whether `If the Locator-Node length ... its CSID encoding is zero.`
belongs to terminology or should rather be in the specification part of this
document to ease the implementer task.

This is merely a very long way to say that either constituent of the CSID can 
be empty. We attempted to clarify this in revision -20 as follows.

| *  Compressed-SID (CSID): A compressed encoding of a SID.  The CSID
|    includes the Locator-Node and Function bits of the SID being
|    compressed.  If either constituent of the SID is empty (zero
|    length), then the same applies to its CSID encoding.

Unsure how to do it, but should LNFL be expanded ?

We expanded LNFL on first use in revision -20.

| In addition, the Locator-Node and Function length (LNFL) is the sum
| of the Locator-Node length and the Function length of the SID.

### Section 4.1

Should this be an uppercase MAY in `An implementation MUST support a 32-bit
Locator-Block length (LBL) and a 16-bit CSID length (LNFL) for NEXT-CSID flavor
SIDs, and may support any other Locator-Block and CSID length.` ? (to be
consistent with the uppercase MUST)

We changed this to an uppercase "MAY" in revision -20.

| An implementation MUST support a 32-bit Locator-Block length (LBL)
| and a 16-bit CSID length (LNFL) for NEXT-CSID flavor SIDs, and MAY
| support any additional Locator-Block and CSID length.

### Section 4.1.6 (and possibly other places)

s/Destination Option header/Destination Option*s* header/

We fixed this in revision -20.


### Section 5.3

Like Erik Kline, please use the RFC 9602 SRv6 block for the examples rather
than 2001:db8::/32

As mentioned in my reply to Erik Kline, it may be best to stick to the usual 
documentation prefix until one gets formally assigned for SRv6 SIDs 
documentation.


### Section 7.1

I was about to DISCUSS this point but, as I may have misread this section, I am
just commenting. Should there be clearly specified that the prefixes being
swapped are of the same length ?

The old and new Locator-Blocks can have different lengths as long as the 
information written in the new DA fits within 128 bits. We clarified this in 
revision -20.

| The original and target Locator-Blocks can
| have different prefix lengths as long as the new Destination Address
| formed by combining the target Locator-Block with the Locator-Node,
| Function, and Argument as described in the pseudocodes of
| Section 7.1.1 and Section 7.1.2 is a valid IPv6 address.

It is also a little weird that the title and the PS acronym use "prefix" while
it is all about locator block...

We renamed the "Endpoint with SRv6 Prefix Swap" (End.PS) and the "Endpoint with 
L3 cross-connect and SRv6 Locator-Block Swap" (End.XPS) to "Endpoint with SRv6 
Locator-Block Swap" (End.LBS) and "Endpoint with L3 cross-connect and SRv6 
Locator-Block Swap" (End.XLBS), respectively, in revision -20.

We also fixed a small glitch in the pseudocodes of sections 7.1.2 and 7.2.2. 
Line R20.2 incorrectly referred to the Destination Address of the IPv6 header 
instead of the new address A, and the value of DA.Arg.Index was not properly 
copied to the new address.

| R20.1. Initialize an IPv6 address A equal to B2.
| R20.2. Write Segment List[Segments Left][DA.Arg.Index] into the bits
|          [m..m+LNFL-1] of A.
| R20.3. Write DA.Arg.Index into the bits
|          [(128-ceiling(log_2(128/LNFL)))..127] of A.
| R20.4. Copy A to the Destination Address of the IPv6 header.

### Section 9.5

Below comments are to be consistent with my DISCUSS point about section 7 of
RFC 9631 "The IPv6 Compact Routing Header (CRH)" that was easily resolved in
the 6MAN WG IETF draft. See
https://mailarchive.ietf.org/arch/msg/ipv6/a_RdmiI3iYk6Sk6mamrppoLTvnc/

The last two paragraphs are useless as they state the obvious and, therefore,
could be confusing to the reader. I strongly suggest removing them (I was even
about to DISCUSS these points).

`Such SR source nodes leveraging TCP/UDP offload engines may require
enhancements to convey the ultimate destination address.` it is obvious to me
that current HW / SW do not always support the latest RFC, e.g., when RFC 2460
(IPv6) was published not all routers supported IPv6, this issue did not prevent
publication of RFC 2460 nor the deployment of IPv6 (even if I would prefer to
have a broader deployment than now ;-) ).

`It was reported that some network node implementations ... may attempt to
verify the upper layer checksum of transit IPv6 packets.` These implementations
clearly violates the end to end architecture of the Internet, so, why would a
standard track document would care about them? And if these box are used for
RFC 7258 pervasive monitoring then it is even a benefit of this specification
;-)

That's a good point. We removed the entire section 9.5 in revision -20 
consistently with what is done for RFC 9631.


### SVG

Thanks for using the nice SVG graphics ;-)


_______________________________________________
spring mailing list -- spring@ietf.org
To unsubscribe send an email to spring-le...@ietf.org

Reply via email to