Subject: [rohc] Comments on RoHC v2 Draft

From: "Kapoor, Rohit" <[email-masked]>
Date: Thu, 18 Oct 2007 10:35:15 -0700

Ghyslain, Kristofer,
 
Good job on the new draft. It looks in fairly good shape now. Pls find some comments below.

1) Section 2: "How the      decompressor detects a sequentially late packet is outside the
      scope of this specification, but it can for example use the MSN to
      this purpose."
 
Not sure what this sentence is trying to convey. Isn't MSN used anyway in detecting a sequentially late packet? So, what exactly is this trying to convey? Somewhat confusing text. I would recommend removing it.
 
2) Section 4.2:
 
"This section provides an overview of some of the most
   significant improvements:"
 
Not all the mentioned points are advantages, as an example "IP Extension Header", "Feedback" etc; so we should replace sentence with below.
 
Replace with: "This section provides an overview of some of the most
   significant differences:"

"Profiles defined in RFC3095 require that the channel between
      compressor and decompressor provide in-order delivery between
      compression endpoints."
 
This does not capture the entire picture of RFC 3095.
 
Replace with: "Profiles defined in RFC3095 require that the channel between
      compressor and decompressor provide in-order delivery between
      compression endpoints, however they can handle a limited amount of reordering before the compression point."

      "ROHCv2 profiles define a CRC in the format of the FEEDBACK-2, and
      different feedback options are available."
 
RoHC v1 can also do CRC, though its use is "SHOULD" for some FEEDBACK options. We should say this clearly when comparing RoHC v2 and RFC 3095. Also, different feedback options are also available for RFC 3095: should mention this also. Recommend adding the following sentence:
 
"RFC 3095 profiles also allow the FEEDBACK to carry a CRC for some of the feedback options; however, the use of CRC is not mandatory. Also, different feedback options are available with RFC 3095 profiles."

3) Section 5.2: "If the received packet is older
   than the current reference packet based on the MSN in the compressed
   header, the decompressor may refrain from using this packet as the
   new reference packet, even if the correctness of its header was
   successfully verified."
 
Why leave this as MAY? Why not define what the decompressor SHALL do? Any advantage from leaving this as MAY?

4) Section 5.2.2: "A packet
   for which the CRC succeeds updates the reference values of all header
   fields, either explicitly (from the information about a field carried
   within the compressed header) or implicitly (fields that are inferred
   from other fields)."
 
This conflicts with text above in point 3, where the decompressor "may refrain from using this packet as the new reference packet".
 
5) Section 5.2.2: "   Validity of the context rather relates to the detection of a problem
   with the context.  The decompressor first assume that the type of
   information that most likely caused the failure(s) is the state that
   normally changes for each packet, i.e. context damage of the dynamic
   part of the context.  Upon repeated decompression failures and
   unsuccessful repairs, the decompressor then assumes that the entire
   context, including the static part, needs to be repaired, i.e. static
   context damage.  Failure to validate the 3-bit CRC that protects
   control fields should be treated as a decompression failure when the
   decompressor asserts the validity of its context."
 
Though this is in an informative section, this text sounds a bit normative. Can we make it sound a little more informative? How about something on these lines (some minor changes):
 
"   Validity of the context rather relates to the detection of a problem
   with the context.  The decompressor ++may++ first assume that the type of
   information that most likely caused the failure(s) is the state that
   normally changes for each packet, i.e. context damage of the dynamic
   part of the context.  Upon repeated decompression failures and
   unsuccessful repairs, the decompressor ++may++ then -assume-- that the entire
   context, including the static part, needs to be repaired, i.e. static
   context damage.  Failure to validate the 3-bit CRC that protects
   control fields should be treated as a decompression failure when the
   decompressor asserts the validity of its context."

6) In RoHC v2, the negative half of the interval for timer-based is ¼. In RFC 3095, this was ½, which is a better setting since jitter can be both negative and positive with respect to the reference.

   COMPRESSED timerbased {
     ENFORCE(time_stride_value != 0);
     timestamp =:= timer_based_lsb(time_stride_value, k,
                                   ((2^k) / 4) - 1);
   }
 
Can we change the interval to (2^k)/2 - 1.
 
7) Section 6.3.1: "For profiles for which the MSN is created by the compressor,"

I would suggest adding the names of these profiles here for better readability.

8) Section 6.6.13.5: "   A compressed list that is part of the dynamic chain (i.e. in IR
   packets) must have all its list items present, i.e. all X-bits in the
   XI list MUST be set.  All items previously established in the item
   table that are not present in the list decompressed from this packet
   MUST also be retained in the decompressor context."

Not sure I understand the second line. If all list items are present in the dynamic chain, then why do previous established items in the item table need to be retained?

Small typos:

1) Page 12: "         where (2^k-1) - p is the maximum positive delta, corresponding
         to the maximum number of consecutive losses for which the lsb
         encoding can recover the original value of the field"
 
This is not correct. (2^k-1) - p is the maximum increase, while (2^k-2) - p is the maximum consecutive losses.
 
2) Section 6.6.7: "Using    this encoding method, the decompressor infers the value of this field
   by counting in octets the length of the entire packet after
   decompression."

Replace "length of the entire packet" with "length of the IPv6 payload".

3) Section 6.6.10: ""reference_unscaled_value" is the value of the
   field being scaled in the context"
 
This will read better: ""reference_unscaled_value" is the ++reference++ value of the
   field being scaled in the context".
 
4) Section 6.6.12: ""previous_IP_ID_value" is the value of the IP-ID in
   the context."
 
Should be: ""reference_IP_ID_value" is the value of the IP-ID in
   the context."
 
5) Section 6.7: "This flag may only be set to 1 either for the
         "co_common" packet format in the different profiles."
 
Should be: "This flag may only be set to 1 for the
         "co_common" packet format in the different profiles."

Thanks,
Rohit

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

---

From: "Kristofer Sandlund" <[email-masked]>
Date: Mon, 22 Oct 2007 15:53:22 +0200

Hi Rohit,

thanks for a very detailed review. My replies inline below.

/Kristofer

Kapoor, Rohit <mailto:[email-masked]> wrote on den 18 oktober 2007 19:35 :

> Not sure what this sentence is trying to convey. Isn't MSN
> used anyway in detecting a sequentially late packet? So, what
> exactly is this trying to convey? Somewhat confusing text. I
> would recommend removing it.

The point is that yes, you would usually use the MSN, but if someone
has a better way (for example lower-layer info on some specific link
technology, timestamps or whatever), they may use such information
instead of the MSN. And we also want to point out that this *how* to do
this is up to implementation to avoid people having to
ask how it is done (which I think would be an FAQ without this text).
So I prefer to keep the text as-is.

> 
> 2) Section 4.2:
> 
> "This section provides an overview of some of the most    significant
> improvements:" 
> 
> Not all the mentioned points are advantages, as an example
> "IP Extension Header", "Feedback" etc; so we should replace
> sentence with below.

I would say that both these _are_ improvements. The ext-hdr
scheme is a _huge_ improvement for anyone unfortunate enough to have
to suffer through implementation of the terrible ext-hdr scheme in
3095 (the round hole, square peg analogy always comes to mind when
I think of list compression of extension headers in 3095). 

> significant differences:" 

I still think that all the things listed actually are improvements 
over 3095 so, I think the text is correct right now.
OTOH, if you think this is important, I can probably be convinced to
change it. What does the rest of the list think?

>       compression endpoints, however they can handle a
> limited amount of reordering before the compression point."
> 

Sounds like a reasonable change, since prelink is mentioned for v2
but not for v1. Will update.

> RoHC v1 can also do CRC, though its use is "SHOULD" for some
> FEEDBACK options. We should say this clearly when comparing
> RoHC v2 and RFC 3095. Also, different feedback options are
> also available for RFC 3095: should mention this also.
> Recommend adding the following sentence:
> 
> "RFC 3095 profiles also allow the FEEDBACK to carry a CRC for
> some of the feedback options; however, the use of CRC is not
> mandatory. Also, different feedback options are available
> with RFC 3095 profiles."

I can agree that the text you point to is a bit confusing, but I don't
think I agree with the proposed change. The intent of the text is to
say that:
1) v2 mandates a CRC in feedback-2 
2) that "different feedback options compared to 3095" are available.

So I prefer to change the quoted text to the following instead:

       "ROHCv2 profiles mandate a CRC for all FEEDBACK-2, and
       a different set of feedback options is available compared
       to RFC 3095 profiles."

Is that ok with you?

>    than the current reference packet based on the MSN in the
>    compressed header, the decompressor may refrain from using this
>    packet as the new reference packet, even if the correctness of its
> header was    successfully verified." 
> 
> Why leave this as MAY? Why not define what the decompressor
> SHALL do? Any advantage from leaving this as MAY?
> 

It is "may" since you can't really make any normative rule about this,
since it is often a guess by the decompressor (especially with the
RTP profiles). And if the decompressor has some reason for doing an
update in this case, it doesn't really break interoperability, so I
don't think we should have stronger language here (also, non-normative
section).

> This conflicts with text above in point 3, where the
> decompressor "may refrain from using this packet as the new
> reference packet".

Agreed, maybe "usually updates" or "normally updates" instead is
sufficient to capture this? 

> Though this is in an informative section, this text sounds a
> bit normative. Can we make it sound a little more
> informative? How about something on these lines (some minor changes):
> 

I think the current text is ok, but I can also agree to your
suggestion. Unless my my co-author objects, I'll change it.

> 
> 
> 6) In RoHC v2, the negative half of the interval for
> timer-based is ¼. In RFC 3095, this was ½, which is a better
> setting since jitter can be both negative and positive with
> respect to the reference.
> 
> 
> 
>    COMPRESSED timerbased {
>                                    ((2^k) / 4) - 1);    }

I don't have an opinion either way.
So if no one objects, I can change this as per your suggestion.

> I would suggest adding the names of these profiles here for

Ok, I can add the profile numbers in parenthesis. Ok with you?

> Not sure I understand the second line. If all list items are
> present in the dynamic chain, then why do previous
> established items in the item table need to be retained?
> 

These would be items that have been established before, but are
not present in the current list. These may re-appear at a later time.
This logic is kind of inherited from 3095 (and 4815), so it is
nothing new, really. I prefer not to change the logic since there
might be some strange concequences, esp. w.r.t. reordering.

To me, the current text is good enough and doesn't need motivation,
but if you have an improvement suggestion that would clarify it for
you, let me know. 

If you would like to have the logic modified so that an IR flushes
the table, we need to have a longer discussion on any concequences,
they might be small, but I can't remember off the top of my head.

> This is not correct. (2^k-1) - p is the maximum increase,
> while (2^k-2) - p is the maximum consecutive losses.

Sounds right, so the text should probably be updated.

> 
> 2) Section 6.6.7: "Using    this encoding method, the
> decompressor infers the value of this field

ok, will change

> This will read better: ""reference_unscaled_value" is the
>    ++reference++ value of the field being scaled in the context".

ok, will change

> Should be: ""reference_IP_ID_value" is the value of the IP-ID in   

ok, will change

>          "co_common" packet format in the different profiles."

ok, will change

---

From: "Kapoor, Rohit" <[email-masked]>
Date: Sun, 28 Oct 2007 20:54:15 -0700

Kristofer,

Pls find some comments below.

-----Original Message-----
From: Kristofer Sandlund [mailto:[email-masked]]
Sent: Mon 10/22/2007 6:53 AM
To: rohc; Kapoor, Rohit
Cc: Ghyslain Pelletier; Kristofer Sandlund
Subject: RE: [rohc] Comments on RoHC v2 Draft
 
Hi Rohit,

/Kristofer

<Rohit> OK. I am OK with keeping the text.

<Rohit> OK. I am fine with this.

Is that ok with you?

<Rohit> This sentence is mostly OK, as long as we also add that RFC 3095
 can also do CRC for FEEDBACK, just that it is SHOULD and not mandatory.

<Rohit> Sounds OK.

<Rohit> OK. Sounds good.

<Rohit> OK. I don't feel strongly about changing it, but I think it would help readability in terms of the intent of the section.

<Rohit> OK.

<Rohit> OK.

<Rohit> OK. I am OK with the current text.

Thanks,
Rohit