Subject: [rohc] Re: More comments on ROHCv2 profiles draft

From: Carl Knutsson <[email-masked]>
Date: Mon, 10 Sep 2007 11:00:09 +0200

Hi Kristofer,

Sorry for the my delayed reply.

more comments inline...

Kristofer Sandlund (LU/EAB) wrote:
> Hi Calle,
> 
> sorry for the delayed reply. I agree to the cut&paste issue about the
> CRC option, so that will go away.
> 

Good!

> Regarding the other issue, me and Ghyslain have discussed this and we 
> agree that the feedback parts of the draft lack some information and 
> can also use some re-structuring. So, we propose the following changes
> to v2:
> 
> - The MSN field inside all feedback is renamed "Acknowledgment Number"
> which makes it a lot easier to describe what to put in there and also 
> makes it easier to see that NACKs also contain ACK numbers.
> Concequently,
> the MSN-NOT-VALID option is now ACKNUMBER-NOT-VALID. We also add this to
> terminology with the following text:
>    Acknowledgment Number
>       The Acknowledgment Number in ROHCv2 profiles is carried in all
>       feedback elements and is used for identifying what packet is being
>       acknowledged.  The value of this field is usually set to the MSN
>       value of the latest successfully decompressed packet in the flow.
> 
> - Text in 6.9.1 is updated as follows:
>    The Acknowledgment Number field of the feedback formats contains the
>    least significant bits of the MSN (see Section 6.2.1) that
>    corresponds to the reference header that is being acknowledged.  A
>    reference header is a header that has been successfully CRC-8
>    validated or CRC verified.  If there is no reference header
>    available, the feedback MUST carry an ACKNUMBER-NOT-VALID option.
> 

This addition will improve the draft. I like the addition of the "Acknowledgment
Number".

> - The Ack Number field is just "LSBs" of the MSN and not "LSB encoded 
> MSN" (since we're only having one reference, these are actually 
> equivalent, but it makes it easier to understand that if we avoid 
> the word "LSB encoding" for the field in the feedback element
> 

Yes, I agree that it should be "lsb the MSN" (or maybe Acknowledge number,
whichever suits you best). The Compressor may have more references depending on
its updating strategy. How the compressor updates the context and the information
it keeps for every packet is out-of-scope of this draft, so it should be left out.
I wasn't expecting an exact description like FN, so I agree with this wording...

> - We feel that since we have 14 bits of Ack number (MSN) inside 
> FEEDBACK-2 and no MSN is longer than 16 bits, the "MSN option" feedback
> seems a bit silly. Is there really any case where two more bits would
> make any sense (unless we want v2 to support interplanetary ROHC)?
> So, we propose to remove this feedback option. The text in 3095 of why
> more bits of SN would be needed in feedback really requires the 
> decompressor to know about how many packets are sent in each link RTT,
> so we do not think this option is useful in any case.
> 

No interplanetary solution!! Throwing away an old ROHC tradition just like that :)

14 bits will probably be sufficient. Let's remove the MSN option..

> I think the above changes will make it clear how to choose, encode, 
> decode and use the sequence number of feedback at both compressor and
> decompressor.
> 

I agree.

/Calle

> Unless anyone objects, these changes go into the next revision of 
> the draft
> 
> /Kristofer
> 
> Carl Knutsson <mailto:[email-masked]> wrote on den 20 juni
> 2007 08:50 :
> 
>> Hi Kristofer, Ghyslain,
>>
>> I found a small issue in the feedback section.
>> copy-and-paste? I believe a more
>> detailed description of the msn decompression in the feedback packet
>> would improve the draft.
>>
>> Section 6.9.2.1:
>> 	"carrying a REJECT option MUST also carry a CRC option."
>>
>> There are no CRC options in ROCHv2 and crc8 is always
>> included in feedback-2 packets.
>>
>> In Section 6.9.1, it would be nice with a more detailed description
>> how the compressor should decompress the msn in feedback packets.
>> There are some more places in the draft that describes the msn in
>> feedback, but it does not cover the details on how to
>> decompress/compress the field other than that it is lsb-encoded. 
>>
>> Best regards,
>> Carl Knutsson

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

---

From: Carl Knutsson <[email-masked]>
Date: Fri, 14 Sep 2007 09:03:41 +0200

Hi Kristofer, Ghyslain,

I have found a typo in FN for draft-ietf-rohc-rfc3095bis-rohcv2-profiles-01.txt.

There is one occurrence of field "outer_flag" in compressed format
ipv4_outer_repair_irregular. I believe it should be ip_outer_flag.

COMPRESSED ipv4_outer_repair_irregular {
    ENFORCE(repair_flag == 1);
    ENFORCE(is_innermost == 0);

    df : ip_id_behavior =:=
==> variable_ipv4_fields( >>>>outer_flag<<<< ) [ 0, 8 ];

    ip_id =:= ip_id_enc_irreg(ip_id_behavior.UVALUE) [ 0, 16 ];
    tos_tc   =:= static_or_irreg(ip_outer_flag, 8) [ 0, 8 ];
    ttl_hopl =:= static_or_irreg(ip_outer_flag, 8) [ 0, 8 ];
  }

/Calle

---

From: "Kristofer Sandlund" <[email-masked]>
Date: Fri, 14 Sep 2007 09:16:04 +0200

Hi Calle,

you're right. I just fixed it in our local CVS, so it will go into
the next revision. Thanks for catching the typo.

/Kristofer

Carl Knutsson <mailto:[email-masked]> wrote on den 14
september 2007 09:04 :

> Hi Kristofer, Ghyslain,
> 
> I have found a typo in FN for
> draft-ietf-rohc-rfc3095bis-rohcv2-profiles-01.txt.
> 
>     ttl_hopl =:= static_or_irreg(ip_outer_flag, 8) [ 0, 8 ];   }
> 
> /Calle

---

From: Carl Knutsson <[email-masked]>
Date: Wed, 19 Sep 2007 15:01:49 +0200

Kristofer, Ghyslain,

There may be a problem in the FN of draft-ietf-rohc-rfc3095bis-rohcv2-
profiles-01.txt for ip-id with ip_id_beh <2 (sequential & swapped sequential).

There is no relationship between ip_id field and ip_id_offset, when using
compressed format 'long' in encoding optional_ip_id_lsb used by co_common and
enc_ipid_dyn used in dynamic chain. How do ip_id_offset get an initial value (if
not set by long) when format nbo/non_nbo applies a lsb to the field?

I am looking at this from the decompressor's point of view. The FN only describes
it for the compressor. Shouldn't it be both ways...

The first time nbo or non_nbo is used, ip_id_offset will be undefined. Applying a
lsb to an undefined field will make the format fail?

As far as I can see, the ipid.ULENGTH attribute is undefined for nbo and non_nbo
in ip_id_lsb used by optional_ip_id (co_common)?

Am I getting this right or have I missed something? Please have a look at this.

See FN below for encoding methods used in co_common...

ip_id_lsb(behavior, k)
{
  UNCOMPRESSED {
    ip_id [ 16 ];
  }

  CONTROL {
    ip_id_offset [ 16 ];
    ip_id_nbo    [ 16 ];
  }

  COMPRESSED nbo {
    ENFORCE(behavior == IP_ID_BEHAVIOR_SEQUENTIAL);
    ENFORCE(ip_id_offset.UVALUE == ip_id.UVALUE - msn.UVALUE);
    ip_id_offset =:= lsb(k, ((2^k) / 4) - 1) [ k ];
  }

  COMPRESSED non_nbo {
    ENFORCE(behavior == IP_ID_BEHAVIOR_SEQUENTIAL_SWAPPED);
    ENFORCE(ip_id_nbo.UVALUE ==
            (ip_id.UVALUE / 256) + (ip_id.UVALUE % 256) * 256);
    ENFORCE(ip_id_nbo.ULENGTH == 16);
    ENFORCE(ip_id_offset.UVALUE == ip_id_nbo.UVALUE - msn.UVALUE);
    ip_id_offset =:= lsb(k, ((2^k) / 4) - 1) [ k ];
  }
}

optional_ip_id_lsb(behavior, indicator)
{
  UNCOMPRESSED {
    ip_id [ 16 ];
  }

  COMPRESSED short {
    ENFORCE((behavior == IP_ID_BEHAVIOR_SEQUENTIAL) ||
            (behavior == IP_ID_BEHAVIOR_SEQUENTIAL_SWAPPED));
    ENFORCE(indicator == 0);
    ip_id =:= ip_id_lsb(behavior, 8) [ 8 ];
  }

  COMPRESSED long {
    ENFORCE((behavior == IP_ID_BEHAVIOR_SEQUENTIAL) ||
            (behavior == IP_ID_BEHAVIOR_SEQUENTIAL_SWAPPED));
    ENFORCE(indicator == 1);
    ip_id =:= irregular(16)  [ 16 ];

  }

  COMPRESSED not_present {
    ENFORCE((behavior == IP_ID_BEHAVIOR_RANDOM) ||
            (behavior == IP_ID_BEHAVIOR_ZERO));
  }
}

/Calle

---

From: Carl Knutsson <[email-masked]>
Date: Fri, 21 Sep 2007 11:33:59 +0200

Hi Kristofer and Ghyslain,

I have some suggestions for the compressed formats co_common and co_repair for all
profiles. I would like to have fields that differ between profiles last in the
packet formats as much as possible, because of implementation reasons. I would
also like to see depending field located after the field they depend on the
compressed formats as much as possible.

I suggest that ipid field is moved after the 'msn' field in co_common and
co_repair for all profiles. ('s/msn/sequencenumber/' for esp profile). ipid is in
many cases dependent of the msn.

I would also like to see a two changes in the encoding method of the sequence
number in the ESP profile:

First, I would like the 'sequencenumber' field to be encoded using the
REORDER_RATIO in co_common. The reson for this is to make packet selection easier
and make the calculation of p the same for all k.

My second suggestion concerning the ESP sequence number is to remove the last 32
bit option in the sdvl-encoding(co_common). 32 bit can be sent co_repair (with
fewer bytes) and there is no need for redundancy here:)

This would mean a new encoding function.

To help for you guys out.. here is the FN (that is if the working group agrees to
my changes).

svdl_esp_sn {
  UNCOMPRESSED {
    sn [ field_width ];
  }

  COMPRESSED lsb7 {
    discriminator =:= '0'               [ 1 ];
    sn =:= msn_lsb(7)                   [ 7 ];
  }

  COMPRESSED lsb14 {
    discriminator =:= '10'              [  2 ];
    sn =:= msn_lsb(14)                  [ 14 ];
  }

  COMPRESSED lsb21 {
    discriminator =:= '110'             [  3 ];
    sn=:= msn_lsb(21)                   [ 21 ];
  }

  COMPRESSED lsb29 {
    discriminator =:= '111'             [  3 ];
    sn =:= msn_lsb(29)                  [ 29 ];
  }
}

/Calle

---

From: "Kristofer Sandlund" <[email-masked]>
Date: Mon, 24 Sep 2007 15:00:00 +0200

Hi,

I think the changes you propose look fine to me, so I will add these
to the next revision of the draft.

/Kristofer

Carl Knutsson <mailto:[email-masked]> wrote on den 21
september 2007 11:34 :

> packet formats as much as possible, because of implementation
> reasons. I would also like to see depending field located after the
> field they 
> depend on the
> co_repair for all profiles. ('s/msn/sequencenumber/' for esp
> profile). ipid is in many cases dependent of the msn.
>