RE: [rohc] Comments on RoHC v2 Draft

Kristofer,

Pls find some comments below.

-----Original Message-----
From: Kristofer Sandlund [mailto:kristofer.sandlund@ericsson.com]
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,

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

/Kristofer

Kapoor, Rohit <mailto:rkapoor@qualcomm.com> wrote on den 18 oktober 2007 19:35 :

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

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.

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

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

> 
> Replace with: "This section provides an overview of some of the most 
> 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?

<Rohit> OK. I am fine with this.

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

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

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

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?

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

> 
> 
> 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?
> 

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

<Rohit> Sounds OK.

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

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

<Rohit> OK. Sounds good.

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

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.

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

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

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

<Rohit> OK.

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

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

<Rohit> OK.

> 
> 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?
> 

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.

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

Thanks,
Rohit

> 
> 