Re: [tsvwg] Reviewing draft-ietf-tsvwg-rfc6040update-shim

[Bob Briscoe <[e-mail]>]

Gorry,

Thank you for your review comments. I've amalgamated both your lists 
into one email below. I've adopted all the changes you propose verbatim 
except where noted with "[BB]".

I would also like to propose to make the following change:
Sorry, for the last-minute proposal post-WGLC, but the world has change 
in the x years that have passed while waiting for the dependent 
reference to move forward. In particular, RFC8311 ended the equivalence 
of ECT(0) and ECT(1).

Reasoning should be self-explanatory - TLDR; removes unnecessary 
uncertainty.

BEFORE:
    *  If there is mix of ECT(0) and ECT(1) fragments, then the
       reassembled packet MUST be set toeither ECT(0) or ECT(1). In
       this case, reassembly SHOULD take into account that the RFC series
       has so far ensured that ECT(0) and ECT(1) can either be considered
       equivalent, or they can provide 2 levels of congestion severity,
       where the ranking of severity from highest to lowest is CE,
       ECT(1), ECT(0) [RFC6040].

PROPOSED:
    *  If there is mix of ECT(0) and ECT(1) outer fragments, then the
       reassembled packet MUST be set to ECT(1).

***Reasoning: Originally [RFC3168] defined ECT(0) and ECT(1) as
       equivalent, but RFC 3168 has been updated by [RFC8311] to make
       ECT(1) available for congestion marking differences. ***The rule is
       independent of***the current use of ECT(1) for L4S [RFC9331]. The**
       rule is compatible with PCN [RFC6660], which uses ***2 levels of
       congestion severity, with the ranking of severity from highest to
       lowest being CE, ECT(1), ECT(0))*. **The decapsulation rules in
       [RFC6040] take a similar approach.*

See [BB] below...

On 15/09/2023 07:51, Gorry Fairhurst wrote:
> Bob,
>
> I have been reviewing draft-ietf-tsvwg-rfc6040update-shim, to prepare 
> a publication request as the new assigned document shepherd. Thanks 
> for the new revision updating the references to cite the latest rev.
>
> I think this draft is nearly ready to proceed - but I have review 
> comments that I think needs a new revision.
>
> Best wishem
>
> Gorry Fairhurst
> (as Document Shepherd)
>
> ===
> 1. Since the ID on intarea tunnels remains an item of pending work, it 
> would be wiser to explain the term outer fragments before first used - 
> citing intarea tunnels as a reference. The cited document is not the 
> easiest to parse to find this definition
>
> "Section 5.3 of [RFC3168] specifies ECN requirements for reassembly of
> sets of outer fragments."
>
> I suggest something like:
>
> Section 5.3 of [RFC3168] specifies ECN requirements for reassembly of
> sets of outer fragments [I-D.ietf-intarea-tunnels] into packets. (In 
> outer fragmentation, an outer header in each fragment indicates the 
> fragmentation and the inner transit header occurs only in the first 
> fragment, with the  following inner (transit) data broken across 
> multiple packets.)
> ===
> 2.
> In Section 6.1.1.1., can you define LCCE in the first sentence before 
> the quote so that the quote becomes readable?
> OLD:
> In particular, for an LCCE
> NEW:
> In particular, L2TP Control Connection Endpoint (LCCE)
> ===
> 3.
> Please could you add a VERY SHORT definition of X and E in the caption 
> of figure 1: Value Field for the LCCE Capability Attribute ...
> e.g.: (E is the ECN Capability flag, the value of X is reserved).
> ====
> 4. Please rephrase this:
> "It is believed that current implementations do not support"
> ... I'd suggest it does not say something about the belief - when 
> published by the IETF, the RFC-Ed will not hold these beliefs, please 
> choose another word.
> OLD:
> It is believed that
> NEW:
> At the time of publication, it is thought that

[BB] I've used "Those implementations known to the authors at the time 
of writing"

> ===
> 5.
> Section 9 ought to be removed at this stage in the process: "9. 
> Comments Solicited"

[BB] I've tagged it with the XML attribute removeInRFC="true" for the 
RFC Editor
And I've moved it to after the refs, along with the Acks, which I now 
understand is preferred RFC style.

> ====
> 6. Please consider citing RFC 6169 as an informativen ref alongside 
> the first citation of RFC 4380, as a note to indicate the IETFs 
> position on current development of Teredo.

[BB] I'd rather not cite RFC6169 because there is nothing in it that is 
at all relevant to the present draft. Despite the title "Security 
Concerns with IP Tunneling", it only covers security concerns to do with 
routing and addressing, which has nothing to do with ECN propagation.

See [BB] in your initial list below, with duplicates snipped
Continued...


On 13/09/2023 10:32, Gorry Fairhurst wrote:
> Bob,
> I have been reviewing draft-ietf-tsvwg-rfc6040update-shim, to prepare 
> a publication request as the new assigned document shepherd.
>
> I think this draft is nearly ready to proceed - but it now needs a new 
> revision to update the references, and to address some concerns.
>
>
> Best wishes,
>
> Gorry Fairhurst
> (In-Coming Document Shepherd)
>
> ===
> 1. Please update all references to cite the latest rev.

[BB] As you saw, now done in draft 17 (between your two emails).

> ===
[snip 2]

> 3. The document provides a list that it states is confined to 
> standards track or widely deployed protocols. Some work appears to 
> have stalled/failed to progress. Please be careful not to imply a 
> projected status for this work-in-progress.
>
> "GUE (Generic UDP Encapsulation) [I-D.ietf-intarea-gue];" - This list 
> includes GUE as work-in-progress. I am unsure we can confirm 
> deployment or document status. Is it wiser to remove the ice, from the 
> list, and simply discuss this alongside Geneva in the text. I 
> understand anyway that no change is requested to this specification.
>
> ietf-nvo3-vxlan-gpe - Unsure if this ref is needed, and did not appear 
> active in the WG, and it seems no specific recommendation included?

[BB] I thought it best to leave these in so that intarea review could 
say whether it wants any removed.

>
> ietf-sfc-nsh-ecn-support - appears active, current text appears OK, 
> please update rev.
>
> ===
> 4. The test says:
>    The requirements in Section 4 update RFC 6040, and hence
>    implicitly update the UDP usage guidelines in RFC 8085 to add the
>    important but previously unstated requirement that, if the UDP tunnel
>    egress does not, or might not, support ECN propagation, a UDP tunnel
>    ingress has to clear the outer IP ECN field to 0b00, e.g. by
>    configuration.
> - I have thought, and I think I am OK with this choice of action, but 
> "implicitly update" is likely to attract attention from an IESG 
> review. Could we avoid "update" explicitly say something, for example 
> I suggest:
>
> NEW:
>    The requirements in Section 4 update RFC 6040, and hence
>    add to the UDP usage guidelines in RFC 8085 by addition of the
>    important, but previously unstated requirement that, if the UDP tunnel
>    egress does not, or might not, support ECN propagation, a UDP tunnel
>    ingress has to clear the outer IP ECN field to 0b00, e.g. by
>    configuration.
> ===

[BB] I had independently updated this section myself and posted draft 17 
the day after your first email (even though I missed the list it 
contained). And I notice you had removed this item from the list you 
sent the next day, so I assume that implied agreement.

[snip 5-9]

> 9. Please confirm there is no known IPR relating to this ID, and that 
> you are content with the Editor's name being presented when published.
> ====

[BB] Confirmed. I know of no related IPR. And happy for my name being on it.

[snip 10]


Bob

-- 
________________________________________________________________
Bob Briscoehttp://bobbriscoe.net/