[PWE3] comments on draft-ietf-pwe3-frame-relay-05.txt

Please remember to address Carlos Pignataro's comments at

http://www.ietf.org/mail-archive/web/pwe3/current/msg07089.html

-------

      - Backward direction.

        In frame relay it is the direction opposite to the direction
        taken by a frame being forwarded (see also forward direction).

      - Forward direction.

        The forward direction is the direction taken by the frame being
        forwarded.

It would read better if you reversed the order of these definitions

------
    Although different layer 2 protocols
    require different information to be carried in this encapsulation, an
    attempt has been made to make the encapsulation as common as possible
    for all layer 2 protocols.  Other layer 2 protocols are described in
    separate documents. [ATM] [ETH] [PPP]

As before not sure of the value of this

------

    Two mapping modes can be defined between frame relay VCs and pseudo
    wires:  The first one is called "one-to-one" mapping, because there
    is a one-to-one correspondence between a frame-relay VC and one
    Pseudo Wire. The second mapping is called "many-to-one" mapping or
    "port mode" Because multiple frame-relay VCs assigned to a port are
s/"port mode" Because/"port mode", because/
one-to-one either all in "" or all not?

-------

           +-------------------------------+
           |                               |
           |    MPLS Transport header      |
           |       (As required)           |
           +-------------------------------+
           |   Pseudo Wire (PW) Header     |
           +-------------------------------+
           |        Control Word           |
           +-------------------------------+
           |          FR Service           |
           |           Payload             |
           +-------------------------------+

same comment as for ATM - you have PW header when you really mean
PW label

--------

6.5. PW packet processing

6.5.1. Generation of PW packets

I am not sure generation is the right word to use. How
about FR PW encapsulation

-------

      - The DE bit of the frame relay frame is copied into the D bit
        field. However if the D bit is not already set, it MAY be set as
        a result of ingress frame policing. If not already set by the
        copy operation, setting of this bit by a PE is OPTIONAL. The PE
        MUST NOT clear this bit (set it to 0 if it was received with the
        value of 1).

I think that we could express this in some simpler way.
How about something like

If the DE bit of the frame relay frame is set the D bit MUST be set.
The PE MAY set the D bit in response to ingress policing. Otherwise the
D bit MUST be clear. Setting the D in response to ingress policing is
OPTIONAL.

Same for the other bits.

----------

      - The sequence number field is processed if the PW uses sequence
        numbers.

Perhaps a forward reference

---------
6.6. Reception of PW packets

Perhaps decapsulation. Whatever word is used it should be the opposite
of the word used to title the encapo process.

----------

    When a PE receives a PW packet, it processes the different fields of
    the control word in order to generate a new frame relay frame for
    transmission to a CE on a frame relay UNI or NNI. The PE performs the
    following actions (not necessarily in the order shown):

      - It generates the following frame relay frame header fields from
        the corresponding fields of the PW packet.
      - The C/R bit is copied in the frame relay header.
      - The D bit is copied into the frame relay header DE bit.
      - The F bit is copied into the frame relay header FECN bit. If the
        F bit is set to zero, the FECN bit may be set to one, depending
        on the congestion state of the PE device in the forward
        direction. Changing the state of this bit by a PE is OPTIONAL.
      - The B bit is copied into the frame relay header BECN bit. If the
        B bit is set to zero, the BECN bit may be set to one, depending
        on the congestion state of the PE device in the backward
        direction. Changing the state of this bit by a PE is OPTIONAL.

I think that a few MUSTs would be appropriate above something like

- The C/R bit MUST be copied into the C/R bit in the frame relay header

- The D bit MUST be copied into the D bit in the frame relay header

- If the F bit is set, the FECN bit MUST be set in the frame relay
   header. If the PE detects congestion in the forward direction, it
   MAY set the set the FECN bit in the frame relay header. Otherwise
   the FECN bit MUST be clear. Setting the FECN bit in response to
   congestion is OPTIONAL.

- If the B bit is set, the BECN bit MUST be set in the frame relay
   header. If the PE detects congestion in the backwards direction, it
   MAY set the set the BECN bit in the frame relay header. Otherwise
   the BECN bit MUST be clear. Setting the BECN bit in response to
   congestion is OPTIONAL.

-------

     - It processes the length and sequence field, the details of which
        are in the subsequent sub-sections.

reference would be helpful to the reader

------

      - It generates the frame relay information field from the contents
        of the PW packet payload after removing any padding.

s/generates/copies/ ?

------

    Once the above fields of a FR frame have been generated, the FCS has
    to be computed,

s/computes/appended/ ?

    HDLC flags have to be added and any bit or byte
    stuffing has been performed (these final actions typically take place
    in a hardware framer).  The FR frame is queued for transmission on
    the selected frame relay UNI or NNI interface.

I think that there must be some simpler way of saying the above -
(FCS + bit stuffing + flags), but the words don't spring to mind
as a type this. Perhaps there are some key words in an HDLC or FR spec
that someone can point to.

----------

    If a packet passes the sequence number check, or is in order then, it

Do you mean "if there is no sequence number, or if the packet is in
order"

---------

    If a PE router negotiated not to use receive sequence number
    processing, and it received a non zero sequence number, then it
    SHOULD send a PW status message indicating a receive fault, and
    disable the PW.

Now I remember some discussion about this being a security hole.
Was this the final resolution?

---------

    If an egress PE receives an excessive number of out-of-sequence PW
    packets, it SHOULD inform the management plane responsible for PW
    setup/maintenance, and take the appropriate actions. The threshold
    for declaring that out-of-sequence PW packets are excessive is not
    defined in this document.

Hang on this is inconsistent with the above

-----------

6.7. MPLS Shim EXP Bit Values

    If it is desired to carry Quality of Service information, the Quality
    of Service information SHOULD be represented in the EXP field of the
    PW MPLS label. If more than one MPLS label is imposed by the ingress
    LSR, the EXP field of any labels higher in the stack SHOULD also
    carry the same value.

6.8. MPLS Shim S Bit Value

    The ingress LSR, PE1, MUST set the S bit of the PW label to a value
    of 1 to denote that the PW label is at the bottom of the stack.

6.7 and 6.8 should be moved earlier

-------------

6.9. Control Plane Details for Frame Relay Service

    When emulating a frame relay service, the frame relay PDUs are
    encapsulated according to the procedures defined herein.

    The PE MUST
    provide frame relay PVC status signaling to the frame relay network.
    If the PE detects a service-affecting condition for a particular
    DLCI, as defined in [ITUQ] Q.933 Annex A.5 sited in IA FRF1.1, the PE
    MUST communicate to the remote PE the status of the PW that
    corresponds to the frame relay DLCI status. The Egress PE SHOULD
    generate the corresponding errors and alarms as defined in [ITUQ] on
    the egress Frame relay PVC.

The above is out of place in this para. Should be a seperate para and
not in the middle of the encap stuff.

    There are two frame relay flags to
    control word bit mappings described below. The legacy bit ordering
    scheme will be used for a PW of type 0x0001 "Frame Relay DLCI
    (Martini Mode)", while the new bit ordering scheme will be used for a
    PW of type 0x0019 "Frame Relay DLCI". The IANA allocation registry of
    "Pseudowire Type" is defined in [IANA] along with initial allocated
    values.

-------

6.9.1. Frame-Relay Specific Interface Parameters

    A separate document [CONTROL], describes the PW control, and
    maintenance protocol in detail including generic interface
    parameters. The interface parameter information, when applicable, it

delete the trailing it

    MUST be used to validate that the PEs, and the ingress and egress
    ports at the edges of the circuit, have the necessary capabilities to
    interoperate with each other. The Interface parameter TLV is defined
    in [CONTROL], the IANA registry with initial values for interface
    parameter types is defined in [IANA], but the frame relay specific
    interface parameters are specified as follows:

      - 0x08 Frame-Relay DLCI Length.

        An optional 16 bit value indicating the length of the frame relay
        DLCI field. This OPTIONAL interface parameter can have value of 2

MUST have the value 2 or 4 ?

        , or 4, with the default being equal to 2. If this interface
        parameter is not present the default value of 2 is assumed.

You descibe default behaviour twice

---------

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