Re: [ippm] Lars Eggert's Discuss on draft-ietf-ippm-stamp-srpm-17: (with DISCUSS and COMMENT)

[Lars Eggert <[e-mail]>]

Hi,

thanks; I cleared based on -18.

Lars

> On Aug 4, 2023, at 16:53, Rakesh Gandhi <[e-mail]> wrote:
> 
> Thank you Lars for the review.
> 
> We have posted a revised draft that addresses your comments:
> 
> URL:            https://www.ietf.org/archive/id/draft-ietf-ippm-stamp-srpm-18.txt
> Html:           https://www.ietf.org/archive/id/draft-ietf-ippm-stamp-srpm-18.html
> Diff:           https://author-tools.ietf.org/iddiff?url2=draft-ietf-ippm-stamp-srpm-18
> 
> Please let us know if you have further review comments.
> 
> Thanks,
> Rakesh
> 
> 
> 
> 
> On Fri, Aug 4, 2023 at 6:28 AM Lars Eggert via Datatracker <[e-mail]> wrote:
> Lars Eggert has entered the following ballot position for
> draft-ietf-ippm-stamp-srpm-17: Discuss
> 
> 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-ippm-stamp-srpm/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> # GEN AD review of draft-ietf-ippm-stamp-srpm-17
> 
> CC @larseggert
> 
> Thanks to Joel Halpern for the General Area Review Team (Gen-ART) review
> (https://mailarchive.ietf.org/arch/msg/gen-art/crZp5rrOYaDNMcoM95b5pFQBReo).
> 
> ## Discuss
> 
> Two issues that I think will be quick to fix:
> 
> ### Section 4, paragraph 12
> ```
>      other Return Path TLVs if present.  A Session-Reflector that supports
>      this TLV MUST reply using the Return Path received in the Session-
>      Sender test packet, if possible.
> ```
> "MUST ... if possible" is an odd construction. Please rephrase and
>  clarify the requirements level.
> 
> ### Section 4.1.3, paragraph 16
> ```
>      The SRv6 Segment List contains a list of 128-bit IPv6 addresses
>      representing the SRv6 SIDs.  Length of the Sub-TLV modulo MUST be 0.
> ```
> Modulo *what*?
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> ## Nits
> 
> All comments below are about very minor potential issues that you may choose to
> address in some way - or ignore - as you see fit. Some were flagged by
> automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
> will likely be some false positives. There is no need to let me know what you
> did with these suggestions.
> 
> ### Outdated references
> 
> Document references `draft-ietf-ippm-stamp-on-lag-01`, but `-03` is the latest
> available revision.
> 
> ### Grammar/style
> 
> #### Section 3, paragraph 4
> ```
> s field in octets. The length is 4 octet for IPv4 address and 16 octet for I
>                                    ^^^^^
> ```
> Possible agreement error. The noun "octet" seems to be countable.
> 
> #### Section 3, paragraph 4
> ```
>  is 4 octet for IPv4 address and 16 octet for IPv6 address. The Destination
>                                     ^^^^^
> ```
> Possible agreement error. The noun "octet" seems to be countable.
> 
> #### Section 4.1, paragraph 4
> ```
> bit): Reply Request Flag at bit 31 (least significant bit) is defined as fol
>                                     ^^^^^
> ```
> A determiner may be missing.
> 
> #### Section 4.1.2, paragraph 3
> ```
> s field in octets. The length is 4 octet for IPv4 address and 16 octet for I
>                                    ^^^^^
> ```
> Possible agreement error. The noun "octet" seems to be countable.
> 
> #### Section 4.1.2, paragraph 3
> ```
> h is 4 octet for IPv4 address and 16 octet for IPv6 address. 4.1.3. Return Se
>                                      ^^^^^
> ```
> Possible agreement error. The noun "octet" seems to be countable.
> 
> #### Section 4.1.3, paragraph 14
> ```
> re two possible combinations for such a interoperability use case: - STAMP S
>                                       ^
> ```
> Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
> "an article", "an hour".
> 
> #### Section 6, paragraph 4
> ```
> e allocated according to the "First Come First Served" procedure as specifie
>                                     ^^^^
> ```
> It seems that a comma is missing. (Also elsewhere.)
> 
> #### Section 8.1, paragraph 3
> ```
>  flow-label, etc. from the packet. Hence for IPv4, for example, different va
>                                    ^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "Hence".
> 
> ## Notes
> 
> This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
> [`ietf-comments` tool][ICT] to automatically convert this review into
> individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].
> 
> [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
> [ICT]: https://github.com/mnot/ietf-comments
> [IRT]: https://github.com/larseggert/ietf-reviewtool
> 
> 
> 
> _______________________________________________
> ippm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ippm