Subject: RE: [rohc] TCP profile FN, ip_id_behavior_enc

From: "Finking, Robert" <[email-masked]>
Date: Wed, 6 Apr 2005 15:12:16 +0100

Hi Eilert,

Thanks for the reply. I agree with you about the alternative encoding that
uses a single format, but also remember the off list discussion now you
mention it! The reason I think there is a bug here is that the default
encoding will never be used. The default methods are only referred to for
fields which do not have any bindings specified by a packet format. All the
packet formats specify a binding for the ip_id_behavior field (albeit just
for one of its attributes), therefore the default encoding methods will not
be consulted for that field. If instead the irregular encoding was added to
the uc_format encodings list, it would always apply:

 ip_id_behavior_enc ===
 {
         uc_format = ip_id_behavior %[ 2 ]
         {
           ip_id_behavior ::= irregular(2);
         };
     :
     :

There are quite a few formats like this, where there is something in the
default encodings which should always apply, and should really be moved to
the uc_format field encodings list.

Does this make sense or have I missed something?

I am right behind you with using constants and getting rid of the other
formats though! If needs be we could just put a comment in to explain what
the format is doing (though without enumerating all the constants inside the
comment).

Cheers

Raffles

> From: Eilert Brinkmann [mailto:[email-masked]] 
> Sent: Wednesday, April 06, 2005 2:46 PM
> 
> "Finking, Robert" wrote:
> > The current tcp-pf.txt contains the following:
> >
> > ip_id_behavior_enc ===
> > {
> >         uc_format = ip_id_behavior; %[ 2 ]
> >
> >         default_methods =
> >         {
> >           ip_id_behavior ::= irregular(2);
> >         };
> >
> >         co_format_sequential = ip_id_behavior, %[ 2 ]
> >         {
> >           let (ip_id_behavior:uncomp_value == 0b00);
> >         };
> >
> >         co_format_sequential_swapped = ip_id_behavior, %[ 2 ]
> >         {
> >           let (ip_id_behavior:uncomp_value == 0b01);
> >         };
> >
> >         co_format_random = ip_id_behavior, %[ 2 ]
> >         {
> >           let (ip_id_behavior:uncomp_value == 0b10);
> >         };
> >
> >         co_format_zero = ip_id_behavior, %[ 2 ]
> >         {
> >           let (ip_id_behavior:uncomp_value == 0b11);
> >         };
> > };
> >
> > Can somebody explain to me what this is supposed to do
> 
> This structure is supposed to encode the value of the ip_id_behavior
> field where it is present in compressed packets. In fact, all four
> compressed formats of this structure encode the value in the same way
> (with irregular), although a specific compressed format is selected
> based on the uncomp_value (which is identical to comp_value)
> attribute.
> 
> In one of our off-list discussions I proposed to define this structure
> with only one compressed format which is independent of the
> value. There was an objection saying that the mapping between values
> and meanings (expressed by format names) would be lost that
> way. However, I still think the "right" solution to associate values
> with meanings is the use of constant definitions rather than different
> format names. A new proposal: The above structure could be replaced by
> the following definitions:
> 
> IP_ID_BEHAVIOR_SEQUENTIAL         = 0;
> IP_ID_BEHAVIOR_SEQUENTIAL_SWAPPED = 1;
> IP_ID_BEHAVIOR_RANDOM             = 2;
> IP_ID_BEHAVIOR_RANDOM_SWAPPED     = 3;
> 
> ip_id_behavior_enc ===
> {
>         uc_format = ip_id_behavior; %[ 2 ]
> 
>         co_format =
>         {
>           ip_id_behavior ::= irregular(2);
>         };
> };
> 
> Then all occurances of ip_id_behavior values in the spec could be
> changed to use the defined constants.
> 
> > - because I don't
> > think it does what it's supposed to (though I think the fix 
> is quite easy).
> 
> What do you think it does?
> 
> > As an aside, it is odd seeing the behaviour flags written 
> in binary here
> > when they are in decimal in the preceding format 
> (ip_id_enc_irreg). I think
> > it would be clearer if we stuck to using a single number base for a
> > particular attribute like this - or is there some 
> significance of the actual
> > bits here that we don't need to see elsewhere (in which 
> case a comment might
> > be useful)? Minor point, but I thought I'd mention it since 
> I noticed it.
> 
> You are right. In an earlier version this structure had encodings such
> as "ip_id_behavior ::= '00';" (i.e., the compressed_value encoding
> method) and so on its compressed formats rather than using
> irregular. This was wrong as the structure has to bind both the
> compressed and the uncompressed side to the same value. When changing
> the structure, I used binary numbers so that the bits remained visible
> as in the previous version, but there is no real reason to keep it
> that way. (With the use of constants this issue would 
> disappear anyway.)
> 
> Regards
> 
> Eilert
> 

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

---

From: "Lars-Erik Jonsson \(LU/EAB\)" <[email-masked]>
Date: Fri, 8 Apr 2005 13:35:21 +0200

Actually, the way we cover the zero case is by the more generic definition CONSTANT, right?

/L-E

> -----Original Message-----
> Sent: den 8 april 2005 09:25
> To: Lars-Erik Jonsson (LU/EAB)
> Cc: Finking, Robert; [email-masked]
> Subject: Re: [rohc] TCP profile FN, ip_id_behavior_enc
> 
> 
> "Lars-Erik Jonsson \(LU/EAB\)" wrote:
> > And what would "RANDOM_SWAPPED" mean? 
> 
> Argh, just an ugly mistake. Obviously, I mixed something up when
> composing the constant names.
> 
> > Do we cover IP_ID_ZERO? Then no field at all is sent, but anyway...
> 
> Yes. Of course, the fourth definition should be:
> 
> IP_ID_BEHAVIOR_ZERO = 3;
> 
> Eilert
>