Layer Two Tunneling Protocol - Version 3 (L2TPv3)
draft-ietf-l2tpext-l2tp-base-15

[Ballot discuss]
1.Page 30 states:

    vendor's extensions or future IETF extensions.  Note that there are
    16 bits allocated for the Vendor ID, thus limiting this feature to
    the first 65,535 enterprises.

  Currently we already have 20.418 values assigned. New assignments occur
  at a rate of 275-300 a month, so say over 3000 a year. Most other
  protocols use a 32bit (or at least 24 bit) field for the Vendor-Id.

2.Bottom of page 39:

    with the condition.  Human-readable text in all error messages MUST
    be provided in the UTF-8 charset using the Default Language [RFC2277].

  I think the doc needs a normative reference to RFC3629
  because of this UTF-8 encoding on the wire.

3. Page 41:
      choose.  If the L2TP data channel runs over IPv4, then this would
      be a comma-separated list of IP addresses in the canonical
      dotted-decimal format (e.g. "10.0.0.1, 10.0.0.2, 10.0.0.3") in the
      UTF-8 charset using the Default Language [RFC2277].  If there are
  (again the "UTF-8 charset" seems wrong).
  But also the IP addresses used in the example should be from the
  192.0.2.0/24 range (as per RFC3330) I think.

4. Pages 42 and 43... talk about a hostname and a vendor-name as being
  ASCII. Is that acceptable? Sect 9 explains why. But I want to hear
  if that is indeed OK.

[Ballot comment]
Did the PWE3 folks review the PWE3 capabilities additions?

I reviewed the control message reliability and congestion control:  it would be
even better if it was not optional, but it's an acceptable spec.

[Ballot discuss]
5.3: State that a node MUST NOT reuse a Random Value for protecting (via Hidden) two or more instances of the Attribute Number unless the values in the two AVPs identical.

5.4.4: For the connect speed field, 32 bits is too small; there are faster links in use today.

[Ballot comment]
Comments from the ops directorate (Pekka Savola)

Tx/Rx Connect speed is in bit/s and is a 32 bit value.
While is this sufficient at the moment, and immediate future,
this could become a problem later on. Is there a reason for
using a 32 bit value ? Otherwise, you might want to
define the field as kb/s or make it a 64bit value.

Section 4.1.4 discusses fragmentation. You might want to split
this section in a part that discusses fragmentation for
ipv4 and one for ipv6 due to differences in fragmentation
handling in ipv4 and ipv6.

Nits:

- has normative references to two Informational documents RFC1958,
  RFC2072; this is not allowed.  Move to informational references.

- there are some cases of bogus use of upper-case keywords for
  non-implementation purposes, for example in:

  Protecting the L2TP packet stream with IPsec does, in turn, also
  protect the data within the tunneled session packets while
  transported from one LCCE to the other.  Such protection MUST NOT be
  considered a substitution for end-to-end security between
  communicating hosts or applications.

(This should obviously be in lower-case -- please review the use of
the keywords.)

[Ballot discuss]
1.Page 30 states:

    vendor's extensions or future IETF extensions.  Note that there are
    16 bits allocated for the Vendor ID, thus limiting this feature to
    the first 65,535 enterprises.

  Currently we already have 20.418 values assigned. New assignments occur
  at a rate of 275-300 a month, so say over 3000 a year. Most other
  protocols use a 32bit (or at least 24 bit) field for the Vendor-Id.

2.Bottom of page 39:

    with the condition.  Human-readable text in all error messages MUST
    be provided in the UTF-8 charset using the Default Language [RFC2277].

  Mmm... I thought UTF-8 is an encoding. The charset that is meant is
  probably UCS or ISO/IEC 10646-1. But then... RFC2277 talks about the
  UTF-8 charset too, so I am not sure. Maybe it is OK.
  In any event, I think the doc needs a normative reference to RFC3629
  because of this UTF-8 encoding on the wire.

3. Page 41:
      choose.  If the L2TP data channel runs over IPv4, then this would
      be a comma-separated list of IP addresses in the canonical
      dotted-decimal format (e.g. "10.0.0.1, 10.0.0.2, 10.0.0.3") in the
      UTF-8 charset using the Default Language [RFC2277].  If there are
  (again the "UTF-8 charset" seems wrong).
  But also the IP addresses used in the example should be from the
  192.0.2.0/24 range (as per RFC3330) I think.

4. Pages 42 and 43... talk about a hostname and a vendor-name as being
  ASCII. Is that acceptable? Sect 9 explains why. But I want to hear
  if that is indeed OK.

[Ballot comment]
$ idnits-v1.27 --nitcount <drafts/draft-ietf-l2tpext-l2tp-base-13.txt
idnits 1.27, (09 May 2004)

  Checking conformance with RFC 2026 boilerplate...
  There are 14 instances of too long lines in the document,
  -- the longest one being 4 characters in excess of 72.

  Warnings:
    The document seems to lack an RFC 2026 Section 10.4(B) IPR Disclosure Invitation
    The document has an RFC 2026 Section 10.4(D) IPR Notice
    There are 10 instances of lines with hyphenated line breaks in the document.

    Line 3169 has weird spacing: '...eptable  conn...'
    Line 3175 has weird spacing: '...breaker    los...'
    Line 3180 has weird spacing: '...ol-conn    est...'
    Line 3190 has weird spacing: '...ol-conn    est...'
    Line 3260 has weird spacing: '...e local    wai...'
      ...

[Ballot discuss]
The application code AVP concerns me.  The current text says:

  The Application Code is a 2 octet value used to identify a specific
  application for an L2TP session, perhaps causing certain values
  within AVPs defined in this document to be interpreted or acted upon
  in a different manner dictated by the Application Code. For example,
  a given Application Code could instruct an LCCE to perform a specific
  directory lookup on the Hostname and/or Router ID AVP information
  associated with this session (perhaps even encoding the destination
  address of the given directory server).

This seems to allow the code to change the meanings of AVPs which
are mandatory in this spec.  Is this much rope necessary?  If this is
required, I agree with the IANA consideration that IETF Consensus would
be required to register a Application Code, and I would expect it to
be pretty hard to get that consensus.  If this were more limited
(by, for example, using it to imply the presence of specific AVPs and/or
values), a less restrictive IANA consideration might be appropriate.

Note that the draft defines Application Code, but appears to use "application ID"
in some references; if these are the same, they need to be collapsed to
a single name.  If they aren't, then a definition of application id is needed.

The draft also refers to language tags, but does not have a reference to
RFC 3066 or the update, draft-phillips-langtags-xx.txt.  It should probably
reference one or the other.

[Ballot comment]
In section 4.1, the draft says:

      The optional Cookie field contains a variable length (maximum 64
      bits) value used to check the association of a received data
      message with the session identified by the Session ID.  The Cookie
      MUST be set to the configured or signaled random value for this
      session utilizing all bits in the field.

If this is a variable length value, what does "utilizing all bits in the field" mean
here--that there is padding to the maximum 64 bits?  If so, how is it padded?
If it means something else, can this be clarified.

In section 4.1.2.2, the draft says:


  A NAT device that can pass TFTP
  traffic with variant UDP ports should be able to pass L2TP UDP
  traffic since both protocols employ similar policies with regard to
  UDP port selection.

RFC 3617's applicability statement discourages its use, so the selection
of a different example protocol is probably in order.

[Ballot comment]
Why does the new version permit directly layering on top of IP? That seems like a step backwards.

Where does the cookie's length come from?  It's not explicit in the packet.  Is the receiver supposed to know what it should be, based on what it assigned?

4.1.3: State that the proposed IPsec solution takes away the ability to verify ports numbers based on cryptographic identity.

5.1: It's usually better to ignore reserved bits, rather than verify that they're zero.

7.1: The text implies that a node SHOULD ignore malformed AVPs, but it's stated as MUST.  As the text notes, it's not always possible to ignore such an error.

[Ballot discuss]
4.1.2.3 discourages UDP checksums.  This is a bad idea.

5.3: State that a node MUST NOT reuse a Random Value with the same Attribute Number unless the value is identical.

5.4.1: A replay of the SCCRQ message is possible.  Is that a a problem?

5.4.4: For the connect speed field, 32 bits is too small; there are faster links in use today.