Re: [tsvwg] David Black's WGLC review of draft-ietf-tsvwg-rfc6040update-shim-08

["Black, David" <[e-mail]>]

idnits found a couple of minor things:

  *   Current reference for IKEv2 is RFC 7296 instead of RFC 5996
  *   If any text was copied from pre-5378 RFCs, an additional boilerplate disclaimer is required.

Thanks, --David

From: Black, David
Sent: Wednesday, May 8, 2019 8:50 PM
To: [e-mail]
Subject: David Black's WGLC review of draft-ietf-tsvwg-rfc6040update-shim-08

This is a solidly written draft on updating tunnel protocols that use shim headers to propagate ECN according to RFC 6040.

Unfortunately, I found a major issue with the updates [1] along with a few minor issues (easily dealt with) and a bunch of editorial items.

The major issue [1] on updates creating non-compliant implementations looks like it will need WG discussion on what should be done, as the "right thing to do" appears to be in potential conflict with deployed "running code".

--- Major ---

[1] Compliance with RFC 6040 update

Section 3.1 says:

   However, an RFC has no jurisdiction over implementations that choose
   not to comply with it or cannot comply with it, including all those
   implementations that pre-dated the RFC.  Therefore it would have been
   unreasonable to add such a requirement to RFC 6040.

But then Section 4 goes on to do exactly that by updating RFC 6040 to
add this text:

      In order that the network operator can comply with the above
      safety rules, even if an implementation of a tunnel ingress does
      not claim to support RFC 6040, RFC 4301 or the full functionality
      mode of RFC 3168:

      *  it MUST make propagation of the ECN field between inner and
         outer IP headers independent of any configuration of Diffserv
         codepoint propagation;

      *  it SHOULD be able to be configured to zero the outer ECN field.

followed by this attempt to excuse the result:

   There might be concern that the above "MUST" makes compliant
   equipment non-compliant at a stroke.  However, any equipment that is
   still treating the former ToS octet (IPv4) or the former Traffic
   Class octet (IPv6) as a single 8-bit field is already non-compliant,
   and has been since 1998 when the upper 6 bits were separated off for
   the Diffserv field [RFC2474], [RFC3260].  For instance, copying the
   ECN field as a side-effect of copying the DSCP is a seriously unsafe
   bug that risks breaking the feedback loops that regulate load on a
   tunnel.

These three chunks of text need to be aligned.  I'm not sure what should
be done here, as the concern about updating RFC 6040 to cause non-compliance
is an important concern.

Analogous non-compliance concerns apply to the updates to other RFCs in
Section 5 to varying degrees.

--- Minor ---

[A] Updated RFCs.

Need to be explicit about what the updates are to each of the updated RFCs.
The updates are scattered throughout the draft - a possible approach is to
add a section that summarizes the update(s) to each RFC and provides pointers
to the section(s) that contain the update(s).

[B] Section 4

      *  if it is known that the tunnel egress does not support
         propagation of the ECN field (RFC 6040, RFC 4301 or the full
         functionality mode of RFC 3168)

The parenthetical listing of RFCs is unclear - I think these all do support
ECN field propagation, so edits are needed to make this clear.


[C] Section 4

   For instance, copying the
   ECN field as a side-effect of copying the DSCP is a seriously unsafe
   bug that risks breaking the feedback loops that regulate load on a
   tunnel.

This text comes immediately after the text in issue [1].  The problem with
this text is that it's implicitly assuming that the tunnel egress discards
the outer header including any congestion indication that it may contain.

That needs to be stated/explained.

[D] Section 5

   Section 3.1.11 of the UDP usage guidelines [RFC8085] already explains
   that a tunnel that encapsulates an IP header directly within a UDP/IP
   datagram needs to follow RFC 6040 when propagating the ECN field
   between inner and outer IP headers.  The requirements in Section 4
   update RFC 6040 so, by reference, they automatically update 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
   legacy UDP tunnel ingress has to clear the outer IP ECN field to
   0b00, e.g. by configuration.

That's too clever/subtle - this draft should explicitly update RFC 8085.

[E] Have the L2TP changes been reviewed by L2TP experts?

[F] Have the AMT changes been reviewed by AMT experts?


--- Editorial ---

Abstract

It surveys widely deployed IP tunnelling
   protocols separated by such shim header(s)

separated by -> that use

Section 4

   Permanently zeroing the outer ECN field is safe, but it is not
   sufficient to claim compliance with RFC 6040 because it does not meet
   the aim of introducing ECN support to tunnels (see Section 4.3 of
   [RFC6040]).

"Permanently" is not the right word.  Suggest rephrasing start of paragraph to

   Zeroing the ECN field in the outer header of all packets is safe, ...

IANA Considerations

The "[TO BE REMOVED: ..." text is not the "right thing to do" - do this instead:

OLD
   IANA is requested to assign the following L2TP Control Message
   Attribute Value Pair:
NEW
   IANA is requested to assign the following L2TP Control Message
   Attribute Value Pair in the Layer Two Tunneling Protocol "L2TP"
   registry (https://www.iana.org/assignments/l2tp-parameters/l2tp-
   parameters.xhtml):
END

IANA will edit this text after performing the assignment and can
remove the link if appropriate.

Thanks, --David
----------------------------------------------------------------
David L. Black, Senior Distinguished Engineer
Dell EMC, 176 South St., Hopkinton, MA  01748
[phone] New    Mobile: [phone]
[e-mail]<mailto:[e-mail]>
----------------------------------------------------------------