rohc team mailing list archive

[Didier Barvaux <didier@xxxxxxxxxxx>]

Hello FWX,

I finally added the ESP profile to the trunk [1] [2].

Thank you very much for your patch! And your patience for me reading,
analysing and committing it :-)

About your last email (see below): I handled the ESP SN > 16 bits by
modifying the 'generic' code to handle 32-bit SN. It works fine with
only small changes.

If you want to test:
 $ bzr branch lp:rohc rohc-trunk
 $ cd rohc-trunk
 $ ./autogen.sh
 $ make clean all
 $ make check

Any feedback is welcome!
 
Regards,
Didier

[1] https://bugs.launchpad.net/rohc/trunk/+bug/952134
[2] http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/501


Le Fri, 01 Jun 2012 11:09:45 +0200,
RoHC Team <rohc_team@xxxxxxxxxx> a écrit :
>
>   Hello Didier,
> 
>   I download the current branch and patch the code with your file 
> without problem.
> 
>   Your integration of my previous patch seems ok, and I agree with
> you for SN <=> sequence_number. I don't think it is a problem to use
> only 16 lowest bits of sequence_number field of ESP for SN.
> 
>   Perhaps I don't understand exactly your questions in the previous 
> e-mail ...
> 
>   Thank you for this good job.
> 
>   Best regards,
> 
>               FWX.
> 
> 
> Le 31/05/2012 21:18, Didier Barvaux a écrit :
> > Hello FWX,
> >
> >
> > Please keep the discussion on the ML, so that anyone can follow it,
> > and maybe participate :)
> >
> > My answers below.
> >
> >
> >
> > Le Wed, 30 May 2012 11:09:05 +0200,
> > RoHC Team<rohc_team@xxxxxxxxxx>  a écrit :
> >
> >>    Hello Didier,
> >>
> >>    Remeber that the support of the ESP (profile 0x0003) was the
> >> beginnig of my work with the library. So, I don't know
> >> everything ...
> >
> > No problem with that. This is no judgement, there are just questions
> > about the understanding of the RFC.
> >
> >
> >>    You'll find my answers bellow, in the text :
> >>
> >> Le 29/05/2012 22:20, Didier Barvaux a écrit :
> >>> FWX,
> >>>
> >>>>     To complete the RoHC library, I add code to support profile
> >>>> 0x0003 (ESP: Encapsulating Security Payload) in the current
> >>>> trunk.
> >>>>
> >>>>     The special case when the NULL encryption algorithm is used,
> >>>> is not supported in this version.
> >>>
> >>> I got enough time to read your patch (thanks again for it!).
> >>> Please find hereafter some questions/notes.
> >>>
> >>> Please do not make any change to you patch for the moment. I
> >>> already rebased your patch to the current trunk tree, and it
> >>> would be a shame to do the work twice :)
> >>>
> >>>
> >>> 1/ SN not required at the end of base header?
> >>>
> >>>     The 16-bit SN field at the end of the base header seems to be
> >>> only required for IP-only, UDP and UDP-Lite profiles, not the RTP
> >>> and ESP ones.
> >>>
> >>>     Section 5.12.1 of RFC 3095 states:
> >>>       In contrast to ROHC UDP, no extra sequence number is added
> >>> to the dynamic part of the ESP header: the ESP sequence number is
> >>> the only element.
> >>>
> >>>     So, on compression side, part 8 of the code_IR_packet()
> >>> function should test for the ESP profile in addition to the RTP
> >>> profile. So does part 7 of the code_IR_DYN_packet() function.
> >>>
> >>>     On decompression side, esp_decode_dynamic_esp() should call
> >>> the ip_decode_dynamic_ip() because it decodes an extra 16-bit SN
> >>> field.
> >>>
> >>
> >> ==>  In the ESP base header, there is a 32 bits field, called
> >> sequence_number. See RFC4303, page 7 to understand the frame
> >> format. It is not the SN of other profile. So, your suggestions
> >> are not ok, here.
> >
> > My understanding of ESP profile in the the RFC 3095 is that the ESP
> > sequence_number field shall be used as the ROHC main sequence
> > number (MSN).
> >
> > I found this sentence in section 5.12 of RFC 3095:
> >
> >     This profile is very similar to the ROHC UDP profile.  It uses
> > the ESP sequence number as the basis for compression instead of a
> >     generated number, but is otherwise very similar to ROHC UDP.
> >
> > The RTP header uses the RTP SN as MSN. The UDP profile uses a
> > generated number as MSN as the UDP header misses a SN. So is the
> > IP-only profile. According to me, the ESP profile shall use the ESP
> > SN field as MSN.
> >
> > The fact that the ESP SN is 32-bit long while the library currently
> > uses 16-bit MSN is an implementation problem, not a constraint.
> >
> >
> > The UDP and IP-only profiles requires a SN field at the end of the
> > base header. As the ROHC MSN is one field of the uncompressed ESP
> > header, it is transmitted in the ESP part of the DYNAMIC chain. So,
> > there is - to my opinion - no need for a 2nd SN field in the very
> > end of the base header. The RTP profile behaves that way too.
> >   - RFC 3095, section 5.11.1 about the UDP profile:
> >       For ROHC UDP, the dynamic part of a UDP packet is different
> > from section 5.7.7.5: a two-octet field containing the UDP SN is
> > added after the Checksum field.  This affects the format of dynamic
> >       chains in IR and IR-DYN packets.
> >   - RFC 3095, section 5.12.1 about ESP profile:
> >       In contrast to ROHC UDP, no extra sequence number is added to
> >       the dynamic part of the ESP header: the ESP sequence number
> > is the only element.
> >
> >
> > To confirm my understanding, I modified the ESP profile to act as
> > described above. The patch (to current trunk, rev 375) is attached.
> >
> >
> > What do you think of my searches in RFCs (and, of my patch)?
> >
> >
> >
> >>> 2/ LSB shift value?
> >>>
> >>>     The LSB shift "p" value for the ESP profile seems to be the
> >>> same as for the RTP profile, not the one of the IP-only, non-RTP
> >>> UDP and non-RTP UDP-Lite profiles.
> >>>
> >>>     Section 5.12 of RFC 3095 states:
> >>>       The interpretation interval (value of p) for the ESP-based
> >>> SN is as with ROHC RTP (profile 0x0001).
> >>>
> >>>     So, in c_generic_create(), the switch seems wrong. On
> >>> decompression side this is a problem too. Also, the
> >>> esp_detect_ir_size() seems to return 2 bytes more than expected.
> >>>
> >>
> >> ==>  Because of the 32 bits specific ESP field sequence_number, not
> >> the 16 bits field SN.
> >
> > See my previous answer about the MSN.
> >
> >
> >>> 3/ ESP-specific context required?
> >>>
> >>>     You defined an ESP-specific context that stores the SN of the
> >>> ESP header. The ESP SN is the main SN (MSN). The MSN is stored in
> >>> the generic context. I think you should use the g_context->sn. It
> >>> would avoid duplication, and remove the need for an ESP-specific
> >>> context. What do you think of?
> >>>
> >>
> >> ==>  See previous response : ESP sequence_number is not the SN!
> >> Sorry ...
> >
> > See my previous answer about the MSN.
> >
> >
> >> ==>  Best regards too,
> >>
> >>              FWX.
> >
> > Regards,
> > Didier
> >
> >
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~rohc
> > Post to     : rohc@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~rohc
> > More help   : https://help.launchpad.net/ListHelp

Attachment: signature.asc
Description: PGP signature