rohc team mailing list archive

[Sean Kelly <seandtkelly@xxxxxxxxx>]

Hi Didier,

Thanks for the patch to fix the return values, it appears to work after
some testing.

> Does the assertion looks like the below?
>   rfc4996.c:438: d_ip_id_lsb: Assertion `ip_id_offset == value' failed.

> If yes, the assertion seems wrong. I've a work-in-progress patch to
> better handle IP-ID compression/decompression in the TCP profile, but
> it was not ready in time for the 1.7.0 release.

Yes that was the assertion I was receiving. Will the patch be ready for
1.7.1?

As a part of my testing I also tried the kernel module tests which appear
to be failing with or without the patch - all of the non regression tests
are failing. I suspected it has something to do with the new feedback
system and it appears that there is no code to insert feedback in the test
kernel module. Is feedback supposed to be prepended in rohc-lib to support
the old API or does the test kernel module need some alterations?

Regards,

Sean


On Wed, Jun 25, 2014 at 7:19 PM, Didier Barvaux <didier@xxxxxxxxxxx> wrote:

> Hi Sean,
>
> > I have discovered a problem with the new positive values for
> > rohc_status_t. In the d_tcp_decode function(in d_tcp.c) the following
> > code operates incorrectly due to the positive return value if there
> > is an error.
> >
> > if(ret >= 0)
> > {
> > uncomp_packet->len += ret;
> > ret = ROHC_STATUS_OK;
> > }
> > rohc_decomp_debug(context, "return %d", ret);
> >
> > The new return values defined in rohc_status_t are all positive so
> > the code above assumes that the length of the decompression was
> > returned. I assume changing the values to negative in the definition
> > of rohc_status_t will fix this? or are the situations were the type
> > of the return value is unsigned?
>
> You're right! Something is wrong there. I forgot to change the called
> functions and the return code check to handle the new positive return
> codes. Good catch, thanks!
>
> I use positive return code because they are now grouped in one enum
> named rohc_status_t. Enum and negative values seems to cause problems
> of portability, so I defined new positive return codes. For backward
> compatibility, the old API functions still use the old return codes,
> but internally the library shall use the new codes.
>
> I have just written a small fix for the problem (see attachement). Tell
> me if it fix the problem for you. It did with a small test case for me.
> If it works, I'll commit it with the small test case. I'll also
> proof-read the whole TCP profile to catch potential other similar
> mistakes.
>
>
> > I found this problem when implementing a fix to an assertion error in
> > the decompressor when tcp compressed packets are lost. My quick fix
> > to this is to return an error when the MSN increments more than 1,
> > which is in the code below (from d_tcp.c).
> >
> > // Check the MSN received with MSN required
> > if( ( (tcp_context->msn + 1) & 0x000F ) != msn)
> > {
> > rohc_decomp_debug(context, "last_msn = 0x%04x, waiting for msn =
> > 0x%x, " "received 0x%x!", tcp_context->msn,
> >                   (tcp_context->msn + 1) & 0x000F, msn);
> > // To be completed !!!
> > // Store and rework packets
> >    //Fixes missing packet assert problems.
> >    rohc_decomp_debug(context, "Error packet missing - assuming context
> > damaged\n");
> >    goto error;
> >
> > }
>
> Does the assertion looks like the below?
>   rfc4996.c:438: d_ip_id_lsb: Assertion `ip_id_offset == value' failed.
>
> If yes, the assertion seems wrong. I've a work-in-progress patch to
> better handle IP-ID compression/decompression in the TCP profile, but
> it was not ready in time for the 1.7.0 release.
>
>
> > From the comments in the code it appears that the implementation of a
> > recovery mechanism is planned when packets are lost. Is an
> > implementation being worked on?
>
> It is implemented in 1.7.0 but "only" for the IP-only, IP/UDP,
> IP/UDP-Lite, IP/ESP and IP/UDP/RTP profiles. The TCP profile doesn't
> handle it yet. For an example of use, see option --repair of the test
> tool test/robustness/lost_packet/test_lost_packet.c
>
> Regards,
> Didier
>
> _______________________________________________
> Mailing list: https://launchpad.net/~rohc
> Post to     : rohc@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~rohc
> More help   : https://help.launchpad.net/ListHelp
>
>