Subject: [Rserpool] draft-ietf-rserpool-common-param-03.txt

From: Harrie Hazewinkel <[email-masked]>
Date: Fri, 21 Mar 2003 11:20:33 +0100

Hi all,

Below some comments on draft-ietf-rserpool-common-param-03.txt.

Harrie

>  Aggregate Server Access Protocol (ASAP) and Endpoint Name  Resolution
>                Protocol (ENRP) common parameters document

'document' adds nothing to the title, I suggest to drop it.

While reading this document, I got the impression that not all
items defined are common for both protocols. Some are for ASAP
and some are for ENRP only. If so, I would suggest to reflect this
in the title, abstract and introduction.
A simple mean to achieve this is to
   do 'common parameters' -> 'parameters'

> Abstract

Abstract equals the Introduction, I suggest a new (following) abstract.

This document details the common parameters of the Aggregate Server
Access Protocol (ASAP) and Endpoint Name Resolution Protocol (ENRP)
protocols defined within the Reliable Server Pooling (RSERPOOL)
architecture.

> 2. Parameters in General
>
>    .. For fields with a length bigger than 4 octets, a
>    number in a pair of parentheses may follow the field name to 
> indicate
>    the length of the field in number of octets.

I do not understand this sentence.

> 3. ENRP-ASAP Common Parameters
>
>    Parameters are defined in the following Type-length-value (TLV)
                                                   ^      ^
NIT: capitals??

>    Parameter Type:  16 bits (unsigned integer)
>
>    The Type field is a 16 bit identifier of the type of parameter.  It
>    takes a value of 0 to 65534.
>
>    The value of 65535 is reserved for IETF-defined extensions.  Values
>    other than those defined in specific ENRP parameter description are
>    reserved for use by IETF.

How can enterprise specifc extensions be done??
Is there a potential future need for this??

>
>    Parameter Value: variable-length.
>
>    The Parameter Value field contains the actual information to be
>    transferred in the parameter.

IMHO, it would clarify if you mention that the

>
>    The total length of a parameter (including Type, Parameter Length 
> and
>    Value fields) MUST be a multiple of 4 bytes.  If the length of the
>    parameter is not a multiple of 4 bytes, the sender pads the 
> Parameter

'the sender pads' -> 'the sender MUST pad'
(although, the MUST is also defined just before)

>    at the end (i.e., after the Parameter Value field) with all zero
>    bytes.  The length of this padding is not included in the parameter
>    length field.  A sender SHOULD NOT pad with more than 3 bytes.  The
>    receiver MUST ignore the padding bytes.

I am not sure, but 'SHOULD NOT' is 'MUST NOT'.
Correct me if I am wrong. Since the padding is not included in the 
length
there is otherwise no way to find out where the new TLV starts.

>
>    (Editor's note: clarify further that any padding inside in the
>    parameter, such as the padding in sub-param is included in the total
>    length)

You could specify that one or more TLVs inside another TLV is just seen
as the parameter value. Then immplicit it is defined and some examples 
can
provide the clarity.

>    The Parameter Types are encoded such that the highest-order two bits
>    specify the action that must be taken if the processing endpoint 
> does
>    not recognize the Parameter Type.

The part above until section 3.1 is better to be moved to the
Parameter Type explaination before. In the document you are now
at Parameter Value and jump back.

> 3.3 SCTP Transport Parameter
>
>    Transport use: 16 bits (unsigned interger)
                                          ^
'interger' -> 'integer'
(Happens for more 'Transport ....' lines)

>    IPv4 or IPv6 Address #1 - #n:
>
>    Each indicates an IPv4 or IPv6 address parameter (as defined above 
> in
>    Section 3.1 and Section 3.2) assigned to this SCTP user transport.
>    At least one IP address parameter MUST be present in an SCTP
>    transport parameter.

Just to be sure, but are IPv4 and IPv6 addresses allowed to be
mixed?? Or is it only IPv4 addresses or only IPv6 addresses??

>
> 3.5 UDP Transport Parameter
>
>    Note: A UDP port MUST NOT be used for control information.  For this
>    reason, no Transport Use field is provided.  UDP MUST always be
>    treated as a "Data Only" type transport use.

Why not 'forcing' the value 'DATA ONLY'??

> 3.6 Pool Member Selection Policy Parameter
>
>    This parameter defines a pool member selection policy.  RSERPOOL
>    supports multiple pool member selection polices and also allows
                                                ^^^
'polices' -> 'policies'

>    definition of new selection polices in the future.
                                     ^^^
'polices' -> 'policies'

>     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
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |        Type = 0x6             |      Length = variable        |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    | Policy Type   |     Policy-specific Data....
>    +-+-+-+-+-+-+-+-+------------------------------------------------

Why would you suddenly do not align in 4 bytes??

>
>    Length: 16 bits (unsigned integer)
>
>    Indicates the entire length of the parameter in number of octets,
>    including the Type, Length, Policy Type fields, and the
>    Policy-specific Data.

Not sure, but in  the figure is a single 'Policy Type' specified and you
speak here of 'Policy Type fields'.
>
>    Note, the value in Length field will NOT cover any padding at the 
> end
>    of the parameter, if there is one.

'one' -> 'padding' (clarity)

>      Value     Policy
>      -----     ---------
>      0x0       (reserved by IETF)
>      0x1       Round Robin
>      0x2       Least Used
>      0x3       Least Used with Degradation (a.k.a. dog pile)
>      0x4       Weighted Round Robin
>      others    (reserved by IETF)

How are enterprise specific policies handled?? Or is that not foreseen
now and in the future??

>
>    Policy-specific Data:
>
>    The structure and fields for each presently defined policy types are

'types are' -> 'type is'
Not native speaker, but each makes it single, or not??

>    described in detail in the following subsections.  The enforcement
>    rules and handling procedures of these policies are defined in
>    Section xxxxx in ASAP [4].

If those policy types are ASAP specific, maybe the types should be 
described in
that document, since they are not common to ENRP and ASAP.

> 3.6.1 Round Robin Policy
> 3.6.2 Least Used Policy
> 3.6.3 Least Used with Degradation Policy
> 3.6.4 Weighted Round Robin Policy

These are examples of useages from the first format presented in 
section 3.6, right??
I believe it is better to make this more clear, since I was first 
confused if this
needed to go into the 'Policy-specifc Data' field.

>
>
> 3.8 Pool Element Parameter
>
>    This parameter is used in multiple ENRP messages to represent an 
> ASAP
>    endpoint (i.e., a PE in a pool) and the associated information, such
>    as its transport address(es), selection policy, and other 
> operational
>    or status information of the PE.
>
>
>     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
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |         Type = 0x8            |       Length=variable         |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                         PE Identifier                         |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                  Home ENRP Server Identifier                  |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                      Registration Life                        |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    :                    User Transport param #1                    :
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    :                    User Transport param #2                    :
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    :                                                               :
>    :                             .....                             :
>    :                                                               :
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    :                    User Transport param #k                    :
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    :                 Member Selection Policy param                 :
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>    Member Selection Policy:

I would suggest to move this parameter before the User Transport,
since there is not a specific count of 'User Transport' parameters.
OK, you can calculate it from the length of the complete parameter,
but I beleive it eases implementation.

>    Server Transport:
>
>    This is an SCTP Transport Parameter, as defined in Section 3.3 that
>    contains the network access address(es), SCTP port number, etc.  of
>    the ENRP server.

Is this only SCTP?? (Not sure)

>    Error causes are defined as variable-length parameters using the
>    format described in 3.2.1 of RFC2960 [3], i.e.:

  I suggest the start of new section 'Error Cause Parameter'.
Also why the reference to [3]. Since you copy the complete
definition it is redundant and let people believe this is SCTP.

>    Defines the type of error condition being reported.
>          Cause Code
>          Value           Cause Code
>          ---------      ----------------
>           1              Unrecognized Parameter
>           2              Unrecognized Message
>           3              Invalid Values
>           4              Non-unique PE Identifier
>           5              Pooling Policy Inconsistent
>           6              Lack of Resources
>          other values    reserved by IETF
>
>    Cause-specific Information: variable length

Assuming this is completely implementation dependant, I believe that
in this case also needs something as what caused it. For instance when
error cause '1' happens it must be indicated which parameter it was.
Although, this is maybe protocol specific semantics.

>    The following subsections (Section 3.10.1 - Section 3.10.2) define
>    specific error causes.

1) I believe that only specifying the first 2 error causes is not 
enough.
They all should be defined.

2) Are these really all errors that are possible??

3) How can one make new error causes and/or enterprise specific ones.
Maybe it is better to define this and forsee already how new error 
causes
can be added and how compatibility of implementations that do not know
those new error codes must be handled.
An idea is adding an Unrecognized Error to which it than can be mapped.
>
> 3.13 Communication Restrictions Parameter
>
>    This parameter defines a TLV that can be used by the sender to
>    restrict name server searches to a limited number of transport 
> types.
>
>
>     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
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |            Type = 0xd         |         Length=0x8            |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |        Control Type           |          Data Type            |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Previous the doc always specifies the item Length also.
Why not here?

>    Type       |    Value
>    -----------+-------------
>    ANY        |   0x0000
>    SCTP ONLY  |   0x0001
>    TCP ONLY   |   0x0002

Why not specifying other as well. Allows extension and backwards 
compatibility
for the future (maybe).
>
>
>    Data Type:  16 bits (unsigned integer)
>
>    This field requests the search be restricted to PE's that have a 
> data
>    channel type of the specified type.  One of the following values 
> MUST
>    be used:
>
>    Type       |    Value
>    -----------+-------------
>    ANY        |   0x0000
>    SCTP ONLY  |   0x0001
>    TCP ONLY   |   0x0002
>    UDP ONLY   |   0x0003

I neither understand why the values of 'Data Type' and 'Control Type'
are different. A 'Control Type' of UDP could be possible. Whether it
will work well is a different thing.

Making these values equal eases it for implementors I guess too.

>
> 4. Common Message Formats
>
>    10 reserved.
>
>    11 reserved.

Although reserved, but what is suggested for to do when these values
are received?? Handle it as '00'?

>    This value represents the size of the message in bytes including the
>    Message Type, Message Flags, Message Length, and Message Value
>    fields.  Therefore, if the Message Value field is zero-length, the
>    Length field will be set to 4.  The Message Length field does not
>    count any padding.
>
>    Note, the value in Message Length field will NOT cover any padding 
> at
>    the end of this message.

The 'Note,..' is equal in semantics to the previous sentence.
IMHO, drop it.

>
> 5. Security Considerations
>
>    Details on ENRP and ASAP security constraints are a addressed in 
> ENRP
                                                        ^

'are a addressed' -> 'are addressed'

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