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
_______________________________________________
Mailing list: https://launchpad.net/~rohc
Post to : rohc@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~rohc
More help : https://help.launchpad.net/ListHelp
