Subject: [P2PSIP] draft-ietf-p2psip-base-19: fragment bits issue

From: Roland Bless <[email-masked]>
Date: Wed, 21 Dec 2011 18:32:24 +0100

Hi,

a student (André Becker) at our institute discovered
some inconsistencies with the proposed fragmentation
while implementing RELOAD, which he has started to work
on recently.

Issue: 5.3.2 and 5.7 are inconsistent and confusing
with respect to the fragment bit definition.

Sec. 5.3.2:
   fragment:  This field is used to handle fragmentation.  The high
      order two bits are used to indicate the fragmentation status:  If
      the high bit (0x80000000) is set, it indicates that the message is
      a fragment.  If the next bit (0x40000000) is set, it indicates
      that this is the last fragment.  The next six bits (0x20000000 to
      0x01000000) are reserved and SHOULD be set to zero.  The remainder
      of the field is used to indicate the fragment offset; see
      Section 5.7

I read this as:
- if the message is a fragment then the high bit is set.
- if the message is the last fragment, then the next bit is set.

(BTW: It is not clear to me why there is only a "SHOULD be set to zero"
for the Reserved bits instead of a MUST.)

And Section 5.7 states:
   If the message is not fragmented, then both the
   first and last fragment bits are set to 1 and the offset is 0
   resulting in a fragment value of 0xC0000000.  Note that this means
   that the first fragment bit is always 1, so isn't actually that
   useful.

Issues:
- there are no official names defined in 5.3.2 that are called
  "first fragment" and "last fragment" bits. The high bit of
  5.3.2 indicates that the message is a fragment, not only the first
  fragment.
- according to 5.3.2, it is perfectly ok to set both bits
  for the last fragment, since the high bit should be set because
  it's a fragment and the next bit should be set to indicate that
  it is the last fragment.
- the last sentence is a somewhat weird statement

Proposal:
- change 5.3.2 to a clearer definition of the bits
- fix the description in 5.7
as follows:

sec 5.3.2:
   fragment:  This field is used to handle fragmentation.  The high
      order two bits are used to indicate the fragmentation status:
      If the message is a fragment the high bit ("fragment bit")
     (0x80000000) MUST be set, as it indicates that the message is a
     fragment. If the message is the last fragment the next bit ("last
     fragment") (0x40000000) MUST be set in addition to the high bit,
     otherwise it MUST be set to zero. The next six bits (0x20000000 to
     0x01000000) are reserved and MUST be set to zero, their contents
     MUST be ignored on reception. The remaining 24 bits of the field
     are used to indicate the fragment offset; see Section 5.7

So if the message is a fragment, the high bit will always be set,
and the last bit will be additionally set to indicate "last fragment".
The combination high bit zero and last fragment is not allowed
(protocol error). Is there a need to report an error message back in
this case?

Proposed changed text for Section 5.7:
Old:
   The first fragment therefore has
   an offset of 0.  The first and last bit indicators MUST be
   appropriately set.  If the message is not fragmented, then both the
   first and last fragment bits are set to 1 and the offset is 0
   resulting in a fragment value of 0xC0000000.  Note that this means
   that the first fragment bit is always 1, so isn't actually that
   useful.

New:
   The first fragment therefore has an offset of zero.
   The fragment and last fragment bit indicators MUST be appropriately
   set. If the message is not fragmented, then both the first and last
   fragment bits are set to zero and the offset MUST be set to zero.

Regards,
 Roland

---

From: Marc Petit-Huguenin <[email-masked]>
Date: Wed, 21 Dec 2011 09:54:06 -0800

Hi Roland,

You may consider subscribing to the RELOAD implementers mailing-list:

http://implementers.org/mailman/listinfo/reload

One comment below.

> - the last sentence is a somewhat weird statement

For reference, here's the email exchange for this text (search for "A.20"):

http://www.ietf.org/mail-archive/web/p2psip/current/msg05801.html

>    fragment bits are set to zero and the offset MUST be set to zero.

- -- 
Marc Petit-Huguenin
Personal email: [email-masked]
Professional email: [email-masked]
Blog: http://blog.marc.petit-huguenin.org

---

From: Roland Bless <[email-masked]>
Date: Wed, 21 Dec 2011 22:20:43 +0100

On 21.12.2011 18:54, Marc Petit-Huguenin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi Roland,
>

> http://www.ietf.org/mail-archive/web/p2psip/current/msg05801.html

Thanks for the pointer, but its doesn't make it more reasonable.
IMHO it just provides a hint that something is broken: if it is
always set to 1, it doesn't provide any useful information, so
you could simply leave it out. Not sure how it was actually meant
to be used, but if it was indicating "first fragment" then it is
set for the first fragment and for non-fragmented packets together
with the last fragment bit. For all intermediate fragments it
probably had to be set to zero, so then it would have changed.
I think that my proposed definition is less confusing to
implementers and protocol debugging people.

Regards,
  Roland

---

From: Marc Petit-Huguenin <[email-masked]>
Date: Wed, 21 Dec 2011 13:45:31 -0800

> implementers and protocol debugging people.

Well, I agree with you, but for all intent and purpose, the protocol is
currently frozen.  The authors rejected all suggestions of modifications that
change the wire protocol for the last year or so.  They seems to not realize
that the longer they wait to publish it, the more likely it will be that people
will find things to fix in it and so will delay the publication even further.
The last WG meeting in Taipei was very disappointing - to say the least - with
very little progress made and I expect that it will be the same in Paris.

- -- 
Marc Petit-Huguenin