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> 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
