Subject: [Gen-art] review of draft-ietf-l2tpext-pwe3-ethernet-07.txt

From: Francis Dupont <[email-masked]>
Date: Tue, 27 Jun 2006 23:54:19 +0200

I am the assigned Gen-ART reviewer for this I-D.
For background on Gen-ART, please see the FAQ at
<http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html>.

Please resolve these comments along with any other
Last Call comments you may receive.

Summary: not ready

The document has a real problem with its style: it is very unpleasant
to read. Unfortunately this is not something the RFC editor can fix...

Detailed comments/suggestions:
 - Abstract: This document describes transport -> the transport?
 - 1.1: remove trailing "." of PE definition
 - 1.1: I suggest to introduce here the L2TPv3 messages as
   imported abbreviations in the logical (i.e., not alpha) order.
 - 1.2 (NSP function): Ethernet packets -> Ethernet frames
 - 1.2: as VLAN tags are in some further cases 802.1q tags,
   I propose to cite 802.1Q the first time and use "VLAN" everywhere
   at the exception of 802.1Q specific places.
 - 2.2 c): must -> MUST?
 - 2.3.2: to indicate the Ethernet interface -> plural without "the"?
 - 3.2: may -> MAY?
 - (technical) 3.3: I am not convinced at all by the very last part
   (fragmentation/reassemble recommendation) because it is a layer violation
   and the goal (manage the issue at only one place as explain in
   L2TPFRAG section 5.1) is not explained.
 - 4: please introduce the AC abbrev
 - 4: the 802.1Q tag -> a VLAN tag
 - 4: preamble or FCS -> preamble and FCS
 - 4: IPSec -> IPsec (cf RFC 4301 introduction)
 - 4: For Ethernet VLAN PW, VLAN tag rewrite ...: can't parse this statement
 - 4: there is no TOS octet or QoS field in the IP header (look at RFC 2474?)
 - 4: 802.1Q CS and ... -> too many "and" (note that here 802.1Q is the
   right term)
 5: should -> SHOULD ??
 9.1 L2TPFRAG: missing I-D name (draft-ietf-pwe3-fragmentation-10.txt?)

Regards

[email-masked]

_______________________________________________
Gen-art mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/gen-art

---

From: Mark Townsley <[email-masked]>
Date: Fri, 30 Jun 2006 11:18:21 -0400

Thanks for the review, Francis. I agree that the document is in need of 
another revision. Alice, do you have the source and can you take on 
addressing these comments?

Please see inline. I suggest some text for some of the comments, and 
have one or two questions as well.

Thanks,

- Mark

>  - 2.3.2: to indicate the Ethernet interface -> plural without "the"?
Agree with all of the above, including RFC2119 language suggestions.
>    L2TPFRAG section 5.1) is not explained.

I agree that this is confusing, and I believe it was text that was asked 
for during AD review ;-)

How about changing this text:

   As a result, the pseudowire endpoints SHOULD support
   Fragmentation and Reassembly procedures specified in Section 5 of
   [L2TPFRAG] or section 4.1.4 of [RFC3931] when the Ethernet payload is
   IPv4.  Only one of these methods SHOULD be used for a given Ethernet
   PW.

to:

   This is not specific only to Ethernet over L2TPv3, and the base L2TPv3 specification 
   [RFC3931] provides general recommendations with respect to fragmentation
   and reassembly in section 4.1.4. "PWE3 Fragmentation and Reassembly" [L2TPFRAG] 
   expounds on this topic further, including a fragmentation and reassembly 
   mechanism within L2TP itself in the event that no other option is available. 
   Implementations MUST follow these guidelines with respect to Fragmentation
   and reassembly.

(Note that I use "MUST" above, but [L2TPFRAG] doesn't mandate the L2TP-specific 
fragmentation and reassembly method. It allows for IP fragmentation and reassembly
as well)

>  - 4: For Ethernet VLAN PW, VLAN tag rewrite ...: can't parse this statement
How about:

"While architecturally [RFC3985] outside the scope of the L2TPv3 PW 
itself, if VLAN tags are present, the NSP may rewrite VLAN tags on 
ingress or egress from the PW (see section 3.1)."

>  - 4: there is no TOS octet or QoS field in the IP header (look at RFC 2474?)
No doubt. We need to at a minimum change this to "ToS bits"and strike 
"IP QoS field" - the paragraph should perhaps simply be rewritten to 
refer to DSCP only.
>  - 4: 802.1Q CS and ... -> too many "and" (note that here 802.1Q is the
I'm not entirely convinced that this is a SHOULD given that it isn't 
really a protocol mechanism that we are defining here, but a deployment 
recommendation.
>  9.1 L2TPFRAG: missing I-D name (draft-ietf-pwe3-fragmentation-10.txt?)
Yes, that's it.
> Regards
Many thanks for the careful review. You caught many mistakes.

- Mark