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 <[email protected]> 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<[email protected]> 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 : [email protected] > > Unsubscribe : https://launchpad.net/~rohc > > More help : https://help.launchpad.net/ListHelp
signature.asc
Description: PGP signature
_______________________________________________ Mailing list: https://launchpad.net/~rohc Post to : [email protected] Unsubscribe : https://launchpad.net/~rohc More help : https://help.launchpad.net/ListHelp
