Re: [L2tpext] AD comments on draft-ietf-l2tpext-l2tp-base-07.txt

Thanks for the review, Thomas. Please see responses inline.

- Mark

Thomas Narten wrote:
> High level issues first, followed by editorial/detailed comments.
> 
> Running l2tp directly over IP:
>   - no checksum for l2tp fields. This seems a bad idea, especially for
>     the control connection.

Disagree for data connection, but understand that there could be some concern on 
the control connection. I think we could and address this via a new AVP which 
includes a checksum, CRC or hash of the control packet. Would that suffice?

>   - not clear that IPsec has been specified enough for running l2tp
>     directly over IP; this needs to get fixed before the IESG will
>     advance.

For UDP mode, L2TPv3 over IPsec should be identical to RFC3193. For IP mode, it 
is a simpler case. I could see room for a more precise definition, but would 
rather do that as an update to RFC3193.

> 
> The document should enable UDP checksum by default. It should require
> explicit action to disable them, rather than the other way around.

That's what RFC2661 required, and the result is that people always turn UDP 
checksums off anyway. Ultimately, this leaves only the L2TP header as 
unprotected by an additional checksum. In practice, this has not been a problem 
that I am aware of for L2TPv2. For L2TPv3, it is even less of a problem if 
cookies are enabled. e.g a bit error in the Session ID will result in a dropped 
packet due to a cookie mis-match. Same if there is a bit error in the Cookie 
itself. A bit error in the L2-Specific Sublayer (if present) could result in an 
out of sequence packet. As with L2TPv2, I believe ultimately folks would 
consider this an acceptable default behavior given the overhead of a UDP 
checksum on the entire packet.

> 
> Note: by staring sequence numbers at 0, the protocol is vulnerable to
> problems if control connections are retarted frequently, and one has
> delayed messages  in the network.

It hasn't been a problem with L2TPv2, which has the same mechanism. Packet 
duplicates are determined via a combination of the source of the message and the 
Assigned Control Connection ID (Assigned Tunnel ID in L2TPv2) present in the 
SCCRQ. If these are all the same, it is a duplicate message.

FWIW, I personally agree with you here. We could (and possibly should) have 
started sequence numbers at a random value in the SCCRQ back in '96. I even 
remember it coming up as a comment at one point, but it never happened for 
whatever reason. I don't see a huge advantage in changing it, so would rather 
stick with what we have from L2TPv2. We would have to play to this (and use a 
sequence number of 0 in the SCCRQ) for a seamless L2TPv2 backwards compatibility 
fallback mode anyway.

> 
> MUST use exponential backoff on retransmission (SHOULD is not
> sufficient). Same for congestion control/slow start.

OK

> 
> Killing a connection after only 5 retransmission seems aggressive. Is
> this the right default value to recommend??

It's what we had for v2, and at least some implementations have increased this 
value. So, I agree that it should be something less aggressive. If others do, I 
don't see why we can't change the recommendation here.

> 
> negotiating whether to use v2 or v3 could be better. There is text
> that suggests trying v3, and if no response, falling back to v2. But
> this is a bad idea because you'll get false fall backs if there are
> connectivity problems. In some parts of the document, it is suggested
> how to try and negotiate v3 but have it fall back to v2
> transparently. Shouldn't that be the preferred method in all cases?

Good point. Transparent mode should always be preferred.

> 
> Does this spec have adequate support for IPv6? Seems like there is at
> least one AVP that holds an IPv4 address, but not an IPv6 address.

One of those AVPs is a holdover from v2 (try another directed result code - 
actually defined in RFC3193), and you are correct that it doesn't consider IPv6 
properly. Since this is already implemented and interoperating, I would hate to 
change it at this point for IPv4. The other is a "Router ID" that may or may not 
be an IPv4 address. Yes, there is no specific treatment of IPv6 in this 
document. Past efforts to drum up L2TPv2 support over IPv6 has fallen on deaf 
ears, I suspect the same would be true for L2TPv3. If someone (that is clueful 
in IPv6) can be found to help with the text here, I would personally be happy to 
see better support for it incorporated into the spec. Another option would be a 
separate L2TPv3 over IPv6 spec, much as we saw L2TPv2 over ATM and FR.

> 
> IANA considerations could be made better my making it more clear what
> IANA needs to assign for this document (the document includes existing
> assignments plus some new ones for new AVPs defined in this document.)

Agreed.

> 
> My sense is that a new revision is appropriate before going to IETF
> Last Call.
> 
> Specific comments follow.
> 
> move acknowledgments to end?

Do we have to? There were so many people who contributed over the years, that I 
feel the need to acknowledge up front. If it is an RFC Editor requirement, we 
can move it.

> 
> Is there enough clarity with regards to what happens to v2 (historic?
> stays at proposed?)

Ideally, v2 goes historic. However, I think this is in conjuction with a PPP 
over L2TPv3 document rather than the base L2TPv3 document.

> 
> UTF-8 issues need a bit more work I think.

OK, I know who to ping for that.

> 
> The sequence number AVPs for resequencing traffic have fields that are
> too small. There will be problems with sequence number wrapping, I
> suspect.

It was really tough to find the right balance here. Some wanted 16 just like in 
L2TPv2, others 32 but you lose the flags. 24 was the compromise. If you need 
more, you can define and negotiate a bigger L2-Specific-Sublayer, so we at least 
have an escape hatch.

BTW, the default 24-bit sequence number is bigger than the PWE3 control word for 
MPLS that has only 16 bits :-)

> 
> 
>>1.  Introduction
> 
> 
> Could be better.
> 
> 
>>   The Layer Two Tunneling Protocol (L2TP) provides a dynamic tunneling
>>   mechanism for multiple Layer 2 (L2) circuits across a packet-oriented
>>   data network.
> 
> 
> Better:
> 
>     The Layer Two Tunneling Protocol (L2TP) provides a dynamic
>     mechanism for tunneling Layer 2 (L2) "circuits" across a
>     packet-oriented data network (e.g., over IP).

OK.

> 
> 
>> L2TP, as originally defined in RFC 2661, is a standard
>>   method for tunneling Point to Point Protocol (PPP) [RFC1661]
>>   sessions.  L2TP has since been adopted for tunneling a number of
>>   other L2 protocols.  In order to provide greater modularity, this
>>   document describes the base L2TP protocol, independent of the L2
>>   payload that is being tunneled.
>>
>>   The base L2TP protocol defined here consists of (1) the control
>>   protocol for dynamic creation, maintenance, and teardown of L2TP
>>   sessions, and (2) the L2TP data encapsulation to multiplex and
>>   demultiplex L2 data streams between two L2TP peers.
> 
> 
> Across IP, I would assume, so please say so.

OK.

> 
> 
>>Additional
>>   pseudowire-specific documents (e.g. for PPP, Ethernet, Frame Relay,
>>   etc.) are expected to be published for each pseudowire-type supported
>>   by L2TP, containing individual details outside the scope of this
>>   specification. Whenever possible, these documents will utilize the
>>   base constructs defined in this specification.
> 
> 
> Better:
> 
>  Additional documents are expected to describe the details of using
>  L2TP to tunnel L2 technologies other than PPP. Likewise, L2TP can be
>  carried across other network types besides IP (e.g., ATM); other
>  documents describe the details.

Agree on most of the wording, except that this base spec does not include PPP 
which this text seems to imply. PPP is a separate companion document, parallel 
with ATM, FR, etc.

> 
> 
> or something like that
> 
> 
>>   Attribute Value Pair (AVP)
>>
>>      The variable-length concatenation of a unique Attribute
>>      (represented by an integer) and a Value containing the actual
>>      value identified by the attribute.  Zero or more AVPs make up the
>>      body of control messages, which are used in the establishment,
>>      maintenance, and teardown of control connections.  This basic
>>      construct is sometimes referred to as a Type-Length-Value (TLV) in
>>      some specifications.  (See also: Control Connection, Control
>>      Message.)
> 
> 
> Why do you not mention the Length field in the definition of the AVP?
> Seems odd/incomplete.

I think just because "A = Attribute, V = Value, P = Pair" - No L here. Happy to 
change.

> 
> 
>>      point to point ethernet, or an L2TP session), or it may have
> 
> 
> point-to-point

OK

> 
> 
>>     two LCCEs over a PSN.  (See also: Control Connection, Data
> 
> 
> LCCE used before defined. ditto for PSN

OK

> 
> 
>>     The action of receiving a call (circuit up event) on an LAC.  The
> 
> 
> is it "an LAC" or "a LAC"? I would assume the latter, as the former
> isn't the way one would actually say it. (this occurs throughout)

During RFC2661 definition, I asked a technical editor at a well-respected book 
publishing firm this question. The reply was that the "a" or "an" is linked the 
first letter of the acronym. e.g. an "el" "ay" "cee" rather than a "lack"

> 
> 
> 
>>   L2TP Network Server (LNS)
>>
>>      An LCCE that logically terminates a tunneled circuit locally and
>>      that processes the tunneled traffic as though the circuit were
>>      physically connected to the device.  The LNS may tunnel to either
>>      an LAC or another LNS.  (See also: LCCE, LAC.)
> 
> 
> an LNS can tunnel to another LNS? as in tunnel switching?

You could think of it that way, but it doesn't have to be an incoming L2TP 
tunnel that triggers the next. e.g. you can have just two LNSs, connected by a 
single L2TP session. Each have virtual interfaces to route packets to/from. This 
looks a lot like a GRE tunnel, or a "Voluntary Tunnel" from RADIUS terminology.

> 
> 
>>   Outgoing Call
>>
>>      The action of placing a call on an LAC, typically in response to
> 
> 
> "on an LAC" or "by an LAC"?

Yes.

> 
> 
> 
>>   Outgoing Call Request
>>
>>      A request sent to an LAC to place an outgoing call.  The request
>>      contains specific information for the LAC in placing the call,
>>      information that is typically not known a priori by the LAC.
> 
> 
> Add "(e.g., the number to dial)" ??

OK, but perhaps "i.e.," as this is an example?

> 
> 
>> (See
>>      also: Call, Incoming Call, Outgoing Call.)
> 
> 
>>     L2TP control connection (i.e. the remote LCCE).  An LAC's peer may
> 
> 
> nit, it's i.e., (with a comma). e.g. should be handled the same
> way. Fix throughout?

Yes. Will do.

> 
> 
>>   Session
>>
>>      An L2TP session is created by a particular L2TP control connection
>>      between two LCCEs when a circuit is successfully established.  The
>>      circuit may either pass through (LAC) or terminate locally (LNS)
>>      on the LCCEs, which maintain state for the circuit.  There is a
>>      one-to-one relationship between established L2TP sessions and
>>      their associated circuits.  (See also: Circuit, LAC, LCCE, LNS.)
> 
> 
> the referencs to (LAC) and (LNS) is a bit cryptic and maybe not proper
> english. Indeed, does it make sense to define "tunnel switching"  to
> make more clear the difference between LAC and LNS as you describe
> above? Or add more text to teh LAC/LNS definition to explain how you
> can chain LACs/LNSs?

I'll try and work on the text myself or ask one of the other co-authors to.

> 
> 
> 
>>2.  Topology
>>
>>   L2TP operates between two L2TP Control Connection Endpoints (LCCEs),
>>   tunneling circuit traffic across a packet network.  An L2TP Network
>>   Server (LNS) is an LCCE that decapsulates tunneled traffic, removes
>>   layer 2 which may be present and processes the encapsulated layer 3
>>   payload without passing on to another piece of equipment (i.e.
>>   routing an IP packet within an HDLC frame tunneled by L2TP). An L2TP
> 
> 
> Weird wording about "equipment". Can't you just say and processes it
> using normal L3 stuff?

I'll try and avoid the word "stuff" as well, but I get your point ;-)

> 
>    tunneling circuit traffic across a packet network.  An L2TP Network
>    Server (LNS) is an LCCE that decapsulates tunneled traffic, removes
>    layer 2 which may be present and processes the encapsulated layer 3
>    payload without passing on to another piece of equipment (i.e.
>    routing an IP packet within an HDLC frame tunneled by L2TP). An L2TP
>    Access Concentrator (LAC) is an LCCE that processes tunneled traffic
>    based primarily on layer 2 information, either by directly switching
>    session traffic to an attached circuit or another L2TP session, or by
>    handing off to a virtual bridging function. A single piece of network
>    equipment may serve as both an LAC and LNS.
> 
> Can't you move the above sentences to the defintion of LAC/LNS?

Sure.

> 
>    (c) LNS-LNS Reference Model: This model has two LNSs as the LCCEs.  A
>    user-level, traffic-generated, or signaled event typically drives
>    session establishment from one side of the tunnel.
> 
> This model could be better explained/motivated. When does one use it?
> how is it different than LAC/LAC? Presumably you mention it in the
> document because there are features in the l2tp protocol to support this?

LNS - LNS is somewhat common in L2TPv2, even though it wasn't outlined 
specifically as such in RFC2661. It is sometimes referred to as a "voluntary 
tunnel" from a CPE device, or a tunnel initiated from a "LAC Client" on a host. 
In L2TPv3, we tried to make this at least sound a bit more peer-to-peer.

When would it be used? Imagine an LNS that accepts connections from a variety of 
sources. Some of those sources are from CE devices, some from PE devices. The 
PEs typically have an attachment circuit to a CE, sold as a leased-line service. 
The CEs have no need for this, so they virtualize the attachment circuit 
interface. The CE looks like an LNS here since it has no L2 attachment circuit 
for the L2TP session to cross-connect to. Instead, it is routing packets at L3.

> 
>    be negotiated by the L2TP control connection.  Similarly, the L2TPv3
>    data message format defined in this document may also be used without
>    the L2TP control channel, utilizing manual configuration to
>    statically "setup" each L2TP sessions. It is recommended that this be
>    limited to relatively small-scale deployments, or deployments in
>    which some other form of automatic control information distribution
>    is employed.
> 
> Does this even need to be mentioned? By saying so, one is blessing it
> as compliant with the spec. Is that necessary?

This simply allows the L2TPv3 encapsulation to be used without the L2TPv3 
control plane if desired. Somewhat like an L2TPv3 "PVC" vs. "SVC" mode, as well 
as the ability to use the L2TPv3 encapsulation with a different control plane 
("automatic control information distribution") if need be. In retrospect, I 
don't see why we need the parenthetical advice that may be what is objectionable 
in your reading. I'll flag this for editing.

> 
> 
>>   steps: (1) Establishing the control connection, if required, and (2)
> 
> 
> when is establishing the control connection NOT required?

As above, one *could* just use the encapsulation, and manually configure things 
like the Session ID, Cookie, etc. You lose the keepalive and ability to 
dynamically signal sessions, but the presumption is that you have some other 
mechanism doing that for you, or that you don't care "e.g. a 'PVC' type model"

> 
> 
>>   In general, an L2TP data message consists of a (1) Session Header,
>>   (2) an optional L2-Specific Sublayer, and (3) the Tunneled L2 Frame,
>>   as depicted below.
>>
>>                  Figure 3.2.2: L2TP Data Message Header
>>
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |                      L2TP Session Header                      |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |                      L2-Specific Sublayer                     |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 
> Picture only lists 2 of the three mentioned parts.

Got it. Thank you.

> 
> 
> 
>>   The L2 Specific Sublayer is an intermediary layer between the L2TP
>>   session header and the start of the tunneled frame.  It contains
>>   control fields that are used to facilitate the tunneling of each
>>   frame (e.g. sequence numbers or flags).  The default L2-Specific
>>   Sublayer for L2TPv3 is defined in Section 4.6.
> 
> 
> Could be better. Is the intent here to say something like:
> 
>    The L2 Specific Sublayer is an intermediary layer between the L2TP
>    session header and the start of the tunneled frame.  It contains
>    control fields that are used to facilitate the reconstruction of
>    the pseudo-wire stream that is begin carried in cases other than
>    when PPP is carried.  The default L2-Specific Sublayer for L2TPv3
>    is defined in Section 4.6.

Yes, except PPP does (will) use the L2-Specific Sublayer for sequencing.

> 
> 
>>4.  Protocol Operation
>>
>>   This section addresses defines the control and data protocol
>>   operation for L2TP.
> 
> 
> double word

Thanks.

> 
>    If necessary, L2TP may operate over a variety of Packet Switched
>    Networks.  The L2TP Session Header encapsulation MAY vary for a given
>    PSN, though the default methods for operation over IPv4 are defined
>    in this document.
>    
> s/though the default methods /though only the methods/

Agreed that they are the only methods. Default is the wrong word.

But, perhaps we can just say:

s/though the default methods /though the methods/

?

> 
> 
>>   While there are efficiencies gained by running L2TP directly over IP,
>>   there are possible side effects as well.  For instance, L2TP over IP
>>   is not as NAT-friendly as L2TP over UDP.  Also, control messages
>>   transmitted over IP are not protected by a network-layer checksum as
>>   they are with UDP.
> 
> 
> hmmm....
> 
> 
>>   When negotiating a control connection over UDP, control messages must
>>   be sent as UDP datagrams using the registered UDP port 1701
>>   [RFC1700].  The initiator of an L2TP control connection picks an
>>   available source UDP port (which may or may not be 1701), and sends
>>   to the desired destination address at port 1701.  The recipient picks
>>   a free port on its own system (which may or may not be 1701) and
>>   sends its reply to the initiator's UDP port and address, setting its
>>   own source port to the free port it found.
> 
> 
> Very un-NAT/firewall  friendly. I know I raised this with the original
> L2TP spec... Why allow the responder to change the source port? what
> value does this have? (I guess I have to ask...)

Deja-vu. In any case, I think we should leave this consistent between v2 and v3, 
no?

> 
> 
>>   choosing an arbitrary source port.  Any NAT device that can pass TFTP
>>   traffic should be able to pass L2TP UDP traffic since both protocols
>>   employ similar policies with regard to UDP port selection.
> 
> 
> I'd drop this, as the claim is somewhat dubious...

If I recall, this was the basis for the decision many years ago. I personally 
think it is useful information. I know at least for one popular NAT 
implementation where this is most definitely the case. I'll ask that the 
definitive nature of the statement be toned down if that is sufficient. e.g. "A 
NAT device that can pass TFTP traffic with variant UDP ports should be able to 
pass L2TP traffic as both protocols employ similar policies with regard to UDP 
port selection." Would this be sufficient?

> 
> 
>>4.1.2.3  UDP Checksum
>>
>>   UDP checksums MUST be enabled for control messages and MAY be enabled
>>   for data messages.  It should be noted that enabling checksums on
>>   data packets may significantly increase the data packet processing
>>   burden.
> 
> 
> UDP checksums should be enabled by default, with clueful people
> perhaps disabling them.

That's what we said in RFC2661. Result is that everyone turns them off, and the 
non-clueful people that leave them on are a major headache for others. (e.g. 
when you send them, you cause pain for your peer). So much so, that at least one 
implementation I know of has a knob that allows ignoring of the UDP checksum 
when sent from the peer. I can't imagine that reiterating the status-quo in the 
L2TPv3 document would change reality here, only serve to continue to foster 
confusion and workarounds.

> 
> 
>>   The message sequence number, Ns, begins at 0.  Each subsequent
>>   message is sent with the next increment of the sequence number.  The
>>   sequence number is thus a free-running counter represented modulo
>>   65536.  The sequence number in the header of a received message is
> 
> 
> Note: this algorithm is flawwed, in that the protocol becomes subject
> vulnerabilities from delayed or out-of-order packets. It would be
> better to let each end of the connection choose the starting sequence
> number OR require the cookie be used to diambiguate old from new
> packets.

As above.. Disambiguation is performed via the Assigned Control Connection ID in 
the control message setup.

And, again, I agree that starting with a non-zero sequence number would have 
been a Good Thing, but that the gain doesn't outweigh the existing deployment in 
RFC2661 that starts at zero.

> 
>    the Ns of the ZLB.  As a precaution, Nr should be sanity-checked
>    before flushing the retransmit queue.  For instance, if the Nr
>    received in a control message is greater than the last Ns sent plus 1
>    modulo 65536, the control message is clearly invalid.
> 
> the "as a precaution" wording is weird. Isn't this required from
> protocol correctness? (as opposed to being just a "precaution").

Agreed.

> 
> 
> 
>>   The reliable delivery mechanism at a receiving peer is responsible
>>   for making sure that control messages are delivered in order and
>>   without duplication to the upper level.  Messages arriving out of
>>   order may be queued for in-order delivery when the missing messages
>>   are received.  Alternatively, they may be discarded, thus requiring a
>>   retransmission by the peer.  When dropping out of order control
>>   packets, Nr MAY be updated before the packet is discarded.
> 
> 
> Don't follow last sentence. You can't update Nr if you are discarding
> packets... What would you update Nr to?

All control messages, even ones being resent which may be discarded as out of 
order by your peer (e.g. your peer had received them, you just didn't know it 
yet), should always carry the very latest Nr. So, before a control packet is 
sent or re-sent, the Nr gets bumped to the latest known value. This way, the 
peer gets acknowledgement updates as fast as possible. As such, even though the 
control message itself may be old, it may have new Nr information in it for 
acknowledging other control messages. Thinking about this again, I think that 
this MAY should be a SHOULD or MUST. I think we left it as a MAY only because it 
is an optimization that isn't _entirely_ necessary, but could help reduce 
retransmissions in some circumstances.

> 
> 
> 
>>   Each subsequent retransmission of a message MUST employ an
>>   exponential backoff interval.  Thus, if the first retransmission
>>   occurred after 1 second, the next retransmission should occur after 2
>>   seconds has elapsed, then 4 seconds, etc.  An implementation MAY
>>   place a cap upon the maximum interval between retransmissions.  This
>>   cap MUST be no less than 8 seconds per retransmission.  If no peer
>>   response is detected after several retransmissions (a recommended
>>   default is 5, but SHOULD be configurable), the control connection and
>>   all associated sessions MUST be cleared.
> 
> 
> Is the default of 5 reasonable in practice? Seems pretty aggressive to
> me.

Agreed.

> 
> 
>>   When a control connection is being shut down for reasons other than
>>   loss of connectivity, the state and reliable delivery mechanisms MUST
>>   be maintained and operated for the full retransmission interval after
>>   the final message exchange has occurred.
> 
> 
> And what is that time exactly?

1+2+4+8+8 by default now. More if we increase it.

> 
> 
>>   When retransmitting control messages, a slow start and congestion
>>   avoidance window adjustment procedure SHOULD be utilized.  A
>>   recommended procedure is described in Appendix A.
> 
> 
> MUST?

OK.

> 
> 
> 
>>   In addition, a peer MUST NOT withhold acknowledgment of messages in
>>   order to maintain state in the L2TP state machine.  Conversely, the
>>   L2TP state machine MUST be capable of maintaining state if a ZLB ACK
>>   is received in response to a control message.  However, determining
>>   when a state should no longer be maintained (e.g. how long to wait in
>>   wait-reply state for an ICRP from the peer) before destroying a
>>   session or control connection is an issue that is left to each
>>   implementation.
> 
> 
> Leaving this implementation dependent is inconsistent with previous
> text about MUST keep state around for the full retransmission
> interval.

No, it's supposed to be two different things (though this is obviously not 
entirely clear). Maintaining control connection state after a StopCCN is the 
1+2+4+8+8... period of time mentioned above. Maintaining state after an ICRQ is 
sent, acknowledged by a ZLB ack or other control message, and waiting for an 
ICRP to be received, was out of scope here.

I agree that this needs tightening up.

> 
> 
>>   The sending of Hello messages and the policy for sending them are
>>   left up to the implementation.  A peer MUST NOT expect Hello messages
> 
> 
> Makes it sound like Hellos are optional. Don't you want to (just) say
> that the policy for when to send them is implemenation dependent? But
> that they must be implemented?

The goal of this text was to clarify the common misperception that a Hello 
message is something that is replied to with another Hello message (this isn't 
the case, the simple reliable delivery of the Hello message is sufficient to 
indicate that the peer and path to the peer is alive). It does make it sound 
like Hellos are optional, which certainly isn't true.

> 
> 
>>   An L2TP implementation may first attempt to operate in L2TPv3 over IP
>>   mode, then fall back to L2TPv2 (over UDP) if L2TPv3 over IP is
>>   unavailable.  It does so by first sending an L2TPv3-formatted SCCRQ
>>   over IP to try to initiate an L2TPv3 control connection.  If the
>>   SCCRQ elicits no response, the implementation may fall back to L2TPv2
>>   operation, as defined in [RFC2661].  Fallback to L2TPv2 should be
>>   seamless and occur automatically.  (See Section 4.7.3 for further
>>   details.)
> 
> 
> suboptimal to have no response be used to determine lack of
> support. Temporary connectivity problems can cause the wrong type of
> connection to become established.

True. But, if L2TPv3 over IP doesn't exist on a peer or gets limited by a NAT or 
FW, what other choice is there?

The fallback between L2TPv3 or L2TPv2 over UDP is deterministic. But, between IP 
and UDP, I don't know what better choice there is.

In reality, I suspect participating nodes will know which to use in advance by 
config. But, allowing the fallback gives some flexibility. It is not a MUST by 
any stretch of the imagination.

> 
> 
>>   4.3).  Hidden values MUST NOT be unhidden until after LCCE
>>   authentication has completed successfully (perhaps requiring the
>>   hidden value to be stored until after receipt of additional setup
>>   messages).  To do otherwise runs the risk of AVP data being utilized
> 
> 
> What does this mean?

The AVP hiding uses the tunnel password. Tunnel authentication verifies that the 
tunnel password is correct. If you try to unhide an AVP with an incorrect 
password, you won't get a failure indication, you will simply have an incorrect 
value that could lead to unexpected results.

So, let's say you receive an SCCRQ with a hidden AVP value. You can hold onto 
that hidden value, but you are not allowed to unhide it until tunnel 
authentication is complete.

> 
> 
>>5.3  Hiding of AVP Attribute Values
> 
> 
> verify with security ADs that this is OK.

OK, but know that there is no change here at all from RFC2661.

> 
> 
> 
>>   The Attribute Value field for this AVP has the following format:
>>
>>    0                   1
>>    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   |         Message Type          |
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 
> This AVP is just a message type, with no Length or value? (Picture
> seems incomplete)

In general, the pictures in the AVPs only contain the Value, rather than 
repeating the Length and Attribute fields again and again throughout the text. 
The length is specified just as a number.

I think this was your idea back when we reformatted the drafts leading up to 
RFC2661 :-)

> 
> 
>>Random Vector (ICRQ, ICRP, ICCN, OCRQ, OCRP, OCCN, CDN, WEN, SLI,
>>StopCCN)
> 
> 
> Is this a subsection heading? Why doesn't it then have a subsection
> number? (this occurs on several...)

This is just an AVP definition. The AVPs do not have their own subheadings.

> 
> 
> 
>>   9 - Try another directed.  If an LAC or LNS is aware of other possible
>>       destinations, it should inform the initiator of the control
>>       connection or session.  The Error Message MUST contain a
>>       comma-separated list of addresses from which the initiator may
>>       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
>>       no servers for the LAC or LNS to suggest, then Error Code 7 should
>>       be used.  The delimiter between addresses MUST be precisely a
>>       single comma and a single space.
> 
> 
> Hmm. Does this spec support IPv6 at all? Shouldn't it?

Yes, I wish. See the general comments section at the beginning of this email.

> 
> 
>>Control Connection Tie Breaker (SCCRQ)
> 
> 
> I don't quite see the need for this. If running over IP, why not just
> use the IP addresses to break ties?

I suppose you could. However, this AVP does two things. (1) indicate that there 
is a desire to limit the number of tunnels between two peers by tie breaking, 
and (2) provide the value to break the tie with. So, you would need the AVP 
anyway. I suppose you could just use the IP address to say who wins. Both should 
work. Do you have a strong objection here? What about for IPv6?

> 
> 
>>Host Name (SCCRQ, SCCRP)
>>
>>   The Host Name AVP, Attribute Type 7, indicates the name of the
>>   issuing LAC or LNS.
>>
>>   The Attribute Value field for this AVP has the following format:
>>
>>    0                   1                   2                   3
>>    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   | Host Name ... (arbitrary number of octets)
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>
>>   The Host Name is of arbitrary length, but MUST be at least 1 octet.
>>
>>   This name should be as broadly unique as possible; for hosts
>>   participating in DNS [RFC1034], a host name with fully qualified
>>   domain would be appropriate.  The Host Name AVP and/or Router ID AVP
>>   MUST be used to identify an LCCE as described in Section 3.3.
> 
> 
> need to be more clear about the encoding of the DNS name. Just using
> strings can lead to interoperability issues.

Suggestions? Have a pointer to proper DNS formatting?

> 
> 
>>   The Vendor Name is the indicated number of octets representing the
>>   vendor string.  Human-readable text for this AVP MUST be provided in
>>   the UTF-8 charset using the Default Language [RFC2277].
> 
> 
> not sure this is the  right way to do this.

It's like this in 2661. What other way would you suggest? Are you objecting to 
having a vendor name at all, or the UTF-8 encoding?

Note that this is for informational purposes. Under no circumstances should an 
implementation parse this to change its behavior.

> 
> 
>>  The Router ID AVP, Attribute Type TBA, is an identifier used to
> 
> 
> clear enough to IANA?

I think so. L2TP AVP attribute space is already listed with IANA.

> 
> 
>>Router ID (SCCRQ, SCCRP)
> 
> 
> Is 4 bytes. OK for IPv4, but for IPv6???

The 4 bytes may or may not be an IP address. All that is required is that it be 
unique (section 8.1, RFC 2072). So, even with IPv6, a router could still have a 
32-bit router ID.

> 
> 
>>   The Assigned Control Connection ID AVP, Attribute Type TBA, encodes
>>   the ID being assigned to this control connection by the sender.
> 
> 
> s/encodes/contains/?

OK

> 
> 
>>Pseudowire Capabilities List (SCCRQ, SCCRP)
> 
> 
> 
> iana name space?

Yes.

> 
> 
>>Remote End ID (ICRQ, OCRQ)
> 
> 
> need to understand better.

Typically used to tie an L2TP session to the attachment circuit. Similar to the 
Martini VCID in this regard, but it is an opaque variable-length value.

> 
> 
>>Application ID (ICRQ, OCRQ)
> 
> 
> What is this for?

For providing a defined interpretation of the Remote End ID. For example, if 
this contained a value for sending to a directory server to realize what to do 
with this incoming session request.

> 
> 
>>Data Sequencing (ICRQ, ICRP, ICCN, OCRQ, OCRP, OCCN)
> 
> 
> The sequence number is only 2 bytes. THis seems too small for lots of
> uses (i.e., sequence wrap will occur very quickly..)

Should be 3 bytes.

> 
> 
>>   The following values are valid data sequencing levels:
>>
>>      0 - No incoming data packets require sequencing.
>>      1 - Only non-IP data packets require sequencing.
>>      2 - All incoming data packets require sequencing.
> 
> 
> is 2 well specified? What is PPP? Non-IP? What is IP exactly in this
> context?

IP in this context is an IP packet carried within an L2 frame (PPP, FR, etc). 
So, if you know this is IP, then you don't sequence the packet. Otherwise, you 
do. If you can't determine this for any reason, you still sequence the packet.

> 
> 
>>   Data sequencing may only be requested when there is a L2-Specific
>>   Sublayer that can provide sequence numbers present. If sequencing is
> 
> 
> doesn't parse well.

"that provides sequence numbers"

> 
> 
>>   The N (New) bit indicates whether the circuit status indication is
>>   for a new circuit (1) or an existing circuit (0).
> 
> 
> what is a new vs. old circuit? what is this distinction needed for?

It is needed for interworking with some link-types, for example, Frame Relay. A 
"new" circuit is one that has been provision and turned on for the first time.

> 
> 
>>   accept the call.  The peer may choose not to accept the call if, for
>>   instance, there are insufficient resources to handle an additional
>>   session.
> 
> 
> Section 4 could say more about how a peer refuses to complete a
> call. What messages get sent?

CDN. Will try and clarify.

> 
> 
> 
>>7.6  Termination of a Control Connection
>>
>>   The termination of a control connection consists of either peer
>>   issuing a StopCCN.  The sender of this message SHOULD wait a finite
>>   period of time for the acknowledgment of this message before
>>   releasing the control information associated with the control
>>   connection.  The recipient of this message should send an
>>   acknowledgment of the message to the peer, then release the
>>   associated control information.
> 
> 
> need to say what this time is...

OK.

> 
> 
>>   [RFC3193] defines the recommended method for securing L2TP as defined
>>   in [RFC2661].  L2TP as defined in this document should possess the
>>   same interface to IPsec as [RFC2661] when running on UDP/IP.  When
>>   operating over IP, some default security association parameters
>>   defined in [RFC3193] will have to change for IP protocol 115, vs. UDP
>>   and the associated ports for L2TPv2.
> 
> 
> details need to be specified...

In an update of RFC3193?

> 
> 
>>9.1  AVP Attributes
>>
>>   As defined in Section 5.1, AVPs contain Vendor ID, Attribute, and
>>   Value fields.  For a Vendor ID value of 0, IANA will maintain a
>>   registry of assigned Attributes and, in some cases, Values.
>>   Attributes 0-39 are assigned as defined in Section 5.4.  The
>>   remaining values are available for assignment upon Expert Review
>>   [RFC2434].
> 
> 
> mention that the name space was originally created via RFC
> 2661. (otherwise IANA may think this is a different registry).

Good point.

> 
> 
>>9.6 Pseudowire Types
>>
>>   The Pseudowire Type (PW Type) is used in the Pseudowire Type AVP and
>>   Pseudowire Capabilities List AVP defined in Section 5.4.3.  0 to
>>   32767 are assignable by Expert Review [RFC2434].  There are no
>>   specific pseudowire types assigned within this document, however each
>>   pseudowire-specific document MUST allocate its own PW types as
>>   necessary according to these guidelines.
> 
> 
> none are defined? But isn't the PW Type Required, e.g.,  in ICRQ?

L2TPv3 is the base specification, and there are follow-on documents for each PW 
type. e.g. draft-ietf-l2tpext-pwe3-fr-01.txt, 
draft-ietf-l2tpext-pwe3-ethernet-00.txt, etc.

> 
> Note: the iana considerations section needs to be more clear about
> which values IANA needs to fill in. I'd suggest including a table for
> each name space from which  values are needed, and fill them in with
> "TBDx" "Section XXX" type references. That makes it easy for IANA to
> fill things in. Also TBDx would have a different x for each distinct
> value needed, to be sure that the document gets updated correctly when
> IANA assigns the values.

Will do.

> 
> 
>>   The 24-bit field of the Default L2-Specific Sublayer contains a
>>   free-running counter, including zero. Each sequenced data packet that
>>   is sent must contain the sequence number, incremented by one, of the
>>   previous sequenced packet sent on a given L2TP session. Upon receipt,
>>   any packet with a sequence number equal to or greater than the
>>   current expected packet (the last received in-order packet plus one)
>>   should be considered "new" and accepted. All other packets are
>>   considered "old" and discarded.
> 
> 
> need to say something about sequence wrap here.

OK.

> 
> 
>>   To mitigate this additional packet loss, one MUST inspect the
>>   sequence numbers of packets dropped due to being classified as "old"
>>   and reset the expected sequence number accordingly. This may be
>>   accomplished by counting the number of "old" packets dropped that
>>   were in sequence among themselves and upon reaching a threshold,
>>   resetting the next expected sequence number to that seen in the
>>   arriving data packets. Packet timestamps may also be used as an
>>   indicator to reset the expected sequence number by detecting a period
>>   of time over which "old" packets have been received in-sequence. The
>>   ideal thresholds will vary depending on link speed, sequence number
> 
> 
> This is suspect...

Necessary to ensure that tons of packets are not dropped if more than 2^24/2 
packets are legitamately dropped. e.g. if you receive a ton of packets in order, 
then its probably because you lost sanity of what the sequence number should be, 
so go ahead and reset it and start accepting packets.

Thanks for the review,

- Mark

> 
> Thomas
> _______________________________________________
> L2tpext mailing list
> L2tpext@ietf.org
> https://www1.ietf.org/mailman/listinfo/l2tpext
> 

_______________________________________________
L2tpext mailing list
L2tpext@ietf.org
https://www1.ietf.org/mailman/listinfo/l2tpext