Subject: [P2PSIP] Reload Message System - Comments

From: Jaime Jimenez <[email-masked]>
Date: Thu, 26 Aug 2010 09:43:23 +0300

Hello,
I have done a J2ME implementation of the RELOAD message system and I would
like to ask about some parts that I might have misinterpreted.

1. RELOAD specifies a length value as the length of the destination_data
field. I think this might be redundant since in each of the cases (node,
resource or compressed) there is already a length field before the data.
Same thing happens in the Config_Update (page 70), config_data and kinds, do
also have a length.

2. In several parts of the code there are certain list fields, for instance
the destination_list, this field comes with a destination_list_length that
is used when parsing a destination_list.

The problem is that in many parts of the code, this kind of lists do not
have a length defined.

A solution would be to specify in Section 5.3.1.1. "Common Definitions",
that all lists should include a uint 16 length before the list.

Some of this cases are:
MessageContents

 struct {
           uint16                 message_code;
           opaque                 message_body<0..2^32-1>;
      *   **uint16                 extensions_length;***
           MessageExtensions      extensions<0..2^32-1>;
         } MessageContents;

SecurityBlock

   struct {
   *  **uint16                 certificates_length;***
      GenericCertificate certificates<0..2^16-1>;
      Signature          signature;
   } SecurityBlock;

3. There is no "model" specified in the diagram in page 92. The case
structure depends on a model field that is not there, it might be a typo.

***enum {single_value (1), array (2), dictionary (3), (255)} ModelType;***

    struct {
         KindId                  kind;
         uint64                  generation;
         uint16                  length;
       * **ModelType               model;***

          select (model) {
            case single_value: ;    /* Empty */

            case array:
                 ArrayRange       indices<0..2^16-1>;

            case dictionary:
                 DictionaryKey    keys<0..2^16-1>;

            /* This structure may be extended */

          } model_specifier;
        } StoredDataSpecifier;

4. In page 139 there is a leave_ans (code value 18) specified. Although it
probably is a simple ACK, may be it should be specified in RELOAD too?

Thanks for the time,
Jaime J.

---

From: Eric Rescorla <[email-masked]>
Date: Thu, 26 Aug 2010 05:53:07 -0700

On Wed, Aug 25, 2010 at 11:43 PM, Jaime Jimenez <[email-masked]> wrote:

> Same thing happens in the Config_Update (page 70), config_data and kinds,
> do also have a length.
>
>
Here's the relevant Pdu:

        enum {reserved(0), peer(1), resource(2), compressed(3), (255) }
              DestinationType;

         select (destination_type) {
           case peer:
              NodeId               node_id;

           case resource:
              ResourceId           resource_id;

           case compressed:
              opaque               compressed_id<0..2^8-1>;

           /* This structure may be extended with new types */

         } DestinationData;

         struct {
           DestinationType         type;
           uint8                   length;
           DestinationData         destination_data;
         } Destination;

1. NodeIds do not have lengths. They're fixed.
2. The DestinationData is extensible, so not all future ones might have
lengths. The length
is for unambiguous parsing if the receiver doesn't recognize the Destination
type.

The same logic applies to config update.

>            MessageExtensions      extensions<0..2^32-1>;

This isn't necessary. the <0..2^32-1> indicates that the value is preceded
by a length. The
only reason that things were done differently in the header was to
accomodate a fixed-length
prefix, thus separating the lengths from the values they specified.

>             /* This structure may be extended */

It's not a typo. It's just not carried on the wire.

> probably is a simple ACK, may be it should be specified in RELOAD too?

This looks like a bug. Thanks.

-Ekr

> Thanks for the time,
> Jaime J.
>
> _______________________________________________
> P2PSIP mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/p2psip
>
>