Guidelines for Adding Congestion Notification to Protocols that Encapsulate IP
draft-ietf-tsvwg-ecn-encap-guidelines-22

Thank you for the work on this document.

Many thanks to Paul Kyzivat for his ART ART review: https://mailarchive.ietf.org/arch/msg/art/jJEK_HJPd3THXWsbRlEjPiP9lEA/.

Running the id-nits tool on this document (https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-tsvwg-ecn-encap-guidelines-21.txt), I was warned of the following:

  -- The document seems to contain a disclaimer for pre-RFC5378 work, and may
     have content which was first submitted before 10 November 2008.  The
     disclaimer is necessary when there are original authors that you have
     been unable to contact, or if some do not wish to grant the BCP78 rights
     to the IETF Trust.  If you are able to get all authors (current and
     original) to grant those rights, you can and should remove the
     disclaimer; otherwise, the disclaimer is needed and you can ignore this
     comment. (See the Legal Provisions document at
     https://trustee.ietf.org/license-info for more information.)

But I saw no hint of why that is in the shepherd write-up. Is it because this is an update to 3819? I would understand better if this was an "obsolete"... Is it common practice? (This is more a question to the responsible AD and the rest of the IESG than to the authors).

Thank you to Dan Harkins for the SECDIR review.

Thank you for this document, and to Tim Wicinski for the OpsDir review (https://datatracker.ietf.org/doc/review-ietf-tsvwg-ecn-encap-guidelines-20-opsdir-lc-wicinski-2023-10-31/).

I think that it would have been helpful for the Abstract to be more specific about the "Updates" - "This document is included in BCP 89 and updates the advice to subnetwork designers about ECN in RFC 3819." -- how does it update the advice? What has changed from RFC3819? The purpose of this is to help readers of the abstract understand what changed / where to focus their attention, and this seems to boil down to "read the whole document".

# Éric Vyncke, INT AD, comments for draft-ietf-tsvwg-ecn-encap-guidelines-21

Thank you for the work put into this document.

Please find below one nits.

Special thanks to Gorry Fairhurst for the shepherd's detailed write-up including the WG consensus *but it lacks* the justification of the intended status.

Other thanks to Brian Haberman, the Internet directorate reviewer (at my request):
https://datatracker.ietf.org/doc/review-ietf-tsvwg-ecn-encap-guidelines-21-intdir-telechat-haberman-2023-11-16/

I hope that this review helps to improve the document,

Regards,

-éric

# NITS (non-blocking / cosmetic)

## v4 v6

Please use "IPv6" rather than "v6" in written documents ;-)

# GEN AD review of draft-ietf-tsvwg-ecn-encap-guidelines-21

CC @larseggert

Thanks to Susan Hares for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/G7OyARFeLUNF3GnMjBSWmIjm2W4).

## Comments

### Boilerplate

This document uses the RFC2119 keywords "MUST NOT", "SHALL NOT", "OPTIONAL",
"SHALL", "SHOULD NOT", "REQUIRED", "RECOMMENDED", "MUST", "MAY", and "SHOULD",
but does not contain the recommended RFC8174 boilerplate. (It contains some
text with a similar beginning.)

Document has a TLP Section 6.c.iii "pre-5378" boilerplate. Is this really
needed?

### Inclusive language

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Terms `native` and `natively`; alternatives might be `built-in`,
   `fundamental`, `ingrained`, `intrinsic`, `original`

## 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-tsvwg-rfc6040update-shim-19`, but `-22` is the
latest available revision.

### URLs

These URLs in the document can probably be converted to HTTPS:

 * http://www.cisco.com/en/US/tech/tk39/tk51/technologies_tech_note09186a00800fbc76.shtml
 * http://ieeexplore.ieee.org/xpl/mostRecentIssue.jsp?punumber=5454061
 * http://bobbriscoe.net/

### Grammar/style

#### Section 1, paragraph 8
```
tination hosts were ECN-capable. Otherwise its congestion markings would nev
                                 ^^^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Otherwise".
(Also elsewhere in the document.)

#### Section 1, paragraph 10
```
y do not all fit a common pattern. Instead they have been divided into a few
                                   ^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Instead".
(Also elsewhere in the document.)

#### Section 1.1, paragraph 2
```
[RFC3168] and updated by [RFC8311]. Also the guidelines for AQM designers [RF
                                    ^^^^
```
A comma may be missing after the conjunctive/linking adverb "Also".

#### Section 1.2, paragraph 3
```
ned for alternative ECN semantics. However it reinforces the same point - tha
                                   ^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "However".

#### Section 1.2, paragraph 5
```
fic class indicated by the DSCP. Therefore correct propagation of congestion
                                 ^^^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Therefore".
(Also elsewhere in the document.)

#### Section 4.2, paragraph 7
```
deployment or deployment by the general public (e.g. Ethernet). For such 'pl
                                ^^^^^^^^^^^^^^
```
Consider using only "public" to avoid wordiness.

#### Section 4.2, paragraph 15
```
 arriving in incoming headers. For example it might copy any incoming congest
                                   ^^^^^^^
```
A comma is probably missing here.

#### Section 4.6, paragraph 26
```
the protocol header, care SHOULD be take to ensure that the field used is sp
                                    ^^^^
```
Did you mean "taken"?

#### Section 4.6, paragraph 31
```
hat encapsulate IP (v4 & v6) in a consistent way, so that IP continues to ful
                             ^^^^^^^^^^^^^^^^^^^
```
Consider replacing this phrase with the adverb "consistently" to avoid
wordiness.

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

Hi,

Thanks for this document, it is well written and an interesting read.

I have just one very minor comment for you to consider please:

(1) p 4, sec 1.  Introduction

   Therefore, the capitalized terms 'SHOULD' or 'SHOULD NOT' are often
   used in preference to 'MUST' or 'MUST NOT', because it is difficult
   to know the compromises that will be necessary in each protocol
   design.  If a particular protocol design chooses not to follow a
   'SHOULD (NOT)' given in the advice below, it MUST include a sound
   justification.

I would suggest "must include a sound justification", i.e., otherwise it is unclear how the justification affects protocol conformance.

Regards,
Rob