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

[Luca Martini <[e-mail]>]

Stewart,

Replies below.
Thanks.
Luca

Stewart Bryant wrote:
> Please remember to address Carlos Pignataro's comments at
>
> http://www.ietf.org/mail-archive/web/pwe3/current/msg07089.html
>
Done.

> -------
>
>      - 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
>
Ok.
> ------
>    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
>
The point is to make sure the reader realizes that we strive to use the 
same CW, and that we have other documents for the other layer2 protocols...
I do not think this text is bad .... So I would like to keep it.
> ------
>
>    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?
>
> -------
>
Ok.

>           +-------------------------------+
>           |                               |
>           |    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
>
what does rfc3985 say ? This is aligned with it.
I added text to make it a little better. I identified an MPLS label as 
the PW label which is the PW header ....

> --------
>
>
> 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
>
Yes, this is not my terminology - I will fix it.

> -------
>
>      - 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.
>
it is unclear to me why this is simpler .... the point that was critical 
here is not to clear the D bit.
Some implementations actually cleared the D bit....


> ----------
>
>      - The sequence number field is processed if the PW uses sequence
>        numbers.
>
> Perhaps a forward reference
>
to CW ? yes . I'll add it.
I will also edit the sequence number processing section to point to CW.

> ---------
> 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.
>
Ok decacapsulation it is.
> ----------
>
>    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
>
Ok .
> - 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.
>
The original section is a bit clearer then what you suggest.
> - 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.
>
Ditto.
> -------
>
>     - It processes the length and sequence field, the details of which
>        are in the subsequent sub-sections.
>
> reference would be helpful to the reader
>
The sections are immediately following a few lines of text, I do not 
think that it is necessary to mention the section name and number given 
that they are a few lines below ...

> ------
>
>      - It generates the frame relay information field from the contents
>        of the PW packet payload after removing any padding.
>
> s/generates/copies/ ?
>
ok.
> ------
>
>    Once the above fields of a FR frame have been generated, the FCS has
>    to be computed,
>
> s/computes/appended/ ?
>
ok.
>    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.
>
I'm re-writing this as it is clearly not clear.

> ----------
>
>    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"
>
section removed.
> ---------
>
>    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?
>
Section removed ..
However this is no more a security hole then sending a packet with the 
wrong sequence number to make the PW loose many packets ....

> ---------
>
>
>    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
>
Removed.

> -----------
>
>
> 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.
>
Not sure i follow, I will remove the top paragraph as it is not useful, 
but this one looks fine ...

>    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
>
Ok.
>    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 ?
>
I do not see why I should use the MUST here ...
>        , 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
>
??? this does not parse.

> ---------
>
Thanks for all the comments !
Luca


_______________________________________________
pwe3 mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/pwe3