Subject: [Fwd: AD review of draft-ietf-pwe3-cw-01.txt]

From: Stewart Bryant <[email-masked]>
Date: Mon, 14 Feb 2005 10:43:33 +0000

The following AD review comments were recently received WRT
draft-ietf-pwe3-cw-01.txt

Stewart

====================================================================

Document could be tighter. Some of the stuff here might be more
appropriate in PW-specific documents. Not sure. Need to rereview in
conjunction with other documents.

expand abbreviations in abstract.

>    The standard MPLS encapsulations have no explicit protocol 
>    identifier. In order for a pseudo wire (PW) [ARCH] to operate 
>    correctly over an MPLS packet switched network (PSN) that performs 
>    deep packet inspection, a PW packet must not appear to the LSR as if 
>    it were an IP packet [BCP]. An example of an LSR that performs deep 

Is "deep packet inspection" defined somewhere? I don't see the word
"deep" in the cited document.

>    though the PSN. This may result in misordered packet deliver to the 
>    egress PE. The inability to ensure that all packets belonging to a 

s/deliver/being delivered/?

>    All IP packets [RFC791][RFC1883] start with a version number that is 
>    checked by LSRs performing deep packet inspection. To prevent the 
>    incorrect inspection of packets, PW packets carried over an MPLS PSN 
>    SHOULD NOT start with the value 4 (IPv4) or the value 6 (IPv6) in 
>    the first nibble [BCP]. 

Better (?):

    To prevent the incorrect processing of packets carried within a PW,
    PW packets carried over an MPLS PSN SHOULD NOT start with the value
    4 (IPv4) or the value 6 (IPv6) in the first nibble [BCP], as those
    are assumed to carry normal IP.

>       When a PW-CW is used, it SHOULD have the following preferred form: 

s/SHOULD/has/

This document should just say this is how it is done. If something
else is done, its not a PW-CW anymore.

>           These bits are available for per payload signalling.  Their 

s/per payload/per-payload/

>    FRG (bits 8 and 9): 
> 
>           These bits are used when fragmenting a PW payload. Their use 
>           is defined in [FRAG] which is currently work in progress. 

FRAG is listed as a normative reference. I suspect that is not what is
desired (nor is it required).

>           If the MPLS 
>           payload is between 46 bytes and 63 bytes the implementation 
>           MAY either set to the length of the MPLS payload, or it MAY 
>           set it to 0.  
> 
>           Note to the reader: In the definition above, both the MUSTs 
>           are needed to make the mechanism work, the MAY provides 
>           backwards compatibility with deployed systems. 

Is this really what is needed? Seems like the spec should be
saying. The sender MUST or SHOULD set it to (pick one or the other),
but (and this is what is critical) on receipt should be prepared to
handle _either_ value. That is what gives you interoperability.

Or, if you want new implementation to work with old, than say the
SHOULD (or MAY) also support sending a 0, but that this should be
configurable??? Or something. Kind of depends on what you are trying
to acheive. Is it interoperability with implementations that send a 0?
Or those that expect 0 on receipt and don't handle the "correct"
value?

>           If the sequence number is not used, it is set to zero by the 
>           sender and ignored by the receiver.  Otherwise it specifies 
>           the sequence number of a packet.  A circular list of sequence 
>           numbers is used.  A sequence number takes a value from 1 to 
>           65535 (2**16-1). The sequence number window size for packet 
>           acceptance is dependent on the parameters of the PSN, and 
>           SHOULD be configurable. The mechanism used by the 
>           decapsulating PE to (re)acquire the correct sequence number 
>           is implementation dependent. 

when you say "should be configurable" and "implementation dependent",
is it really the case that this is PW-specific, with the details
defined in those docs?

IANA considerations for all these documents need to be synced up, as
there are redundencies

> 6.   Security Considerations 
> 
>    An application using this mechanism to provide an OAM [VCCV] or 
>    other message channel MUST be aware that this can potentially be 

which mechanism does "this mechanism" refer to?

>    If a PW has been configured to operate without a CW, the PW 
>    Associated Channel Type mechanism described in the document MUST NOT 
>    be used. This is to prevent user payloads being fabricated in such a 
>    way that they mimic the PW Associated Channel header, and thereby 
>    provide a method of attacking the application that is using the 
>    Associated Channel. 

what does "MUST NOT be used" mean? Is there a way to _prevent_ it from
being used?

> 10.    Normative References 

I suspect many, if not most, of these references should be informative.