Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:

include/odp/api/spec/ipsec.h
line 8
@@ -1346,9 +1346,7 @@ int odp_ipsec_in(const odp_packet_t pkt_in[], int num_in,
  * and content of packet data before the IP header is undefined. Use outbound
  * operation parameters to specify the amount of TFC padding appended to
  * the packet during IPSEC transformation. Options can be used also to create
- * TFC dummy packets. Packet data content is ignored in tunnel mode TFC dummy
- * packet creation as tfc_pad_len option defines solely the packet length.
- * In all other cases, payload length for the IPSEC transformation is specified
+ * TFC dummy packets. Payload length for the IPSEC transformation is specified
  * by odp_packet_len() minus odp_packet_l3_offset() plus tfc_pad_len option.


Comment:
A dummy packet is defined only by the fact that it is a dummy and the requested 
length. No further metadata should be needed or required, so I'd be happy to 
see that requirement dropped. I view this as a building block for a TBD 
higher-level configuration service that would simply permit IPsec to be 
configured to provide a requested level of traffic masking and leave it up to 
the implementation to decide how best and most efficiently to achieve that via 
a combination of TFC dummy packets and padding.

> Dmitry Eremin-Solenikov(lumag) wrote:
> Well, it's just a magic number. Same as 0 would be.


>> Dmitry Eremin-Solenikov(lumag) wrote:
>> Hmm. We've spent several minutes on this, but nobody reminded of API 
>> convention. Should we change the spec here? @psavol what is your opinion?


>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>> We require that an application sets `l3_offset` for all packet it pushes to 
>>> IPsec. For TFC dummy packets it resulted in `l3_offset` being set but 
>>> ignored. Thus I proposed this change. Other solution might be to stop 
>>> requiring `l3_offset` for TFC dummy packets. 


>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> Should `0xa5` be a `#define` rather than a "magic number"?


>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> Nit: could use `odp_unlikely()` here.


>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> This change requires an API change as the spec says relevant offsets 
>>>>>> must be in the range `0..odp_packet_len(pkt) - 1` .  Same comment for 
>>>>>> the L3 and L4 changes in this patch.
>>>>>> 
>>>>>> In theory the validation tests should test these bounds, but as with 
>>>>>> most parts of the API violations simply result in undefined behavior, so 
>>>>>> this is an "honor system". Still, we can't violate the spec here without 
>>>>>> changing the spec.


>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> Unless it has been parsed, `odp_packet_l3_offset()` is initialized to 
>>>>>>> `ODP_PACKET_OFFSET_INVALID`, so this seems an undue burden. The 
>>>>>>> original wording seems cleaner from an application perspective.


https://github.com/Linaro/odp/pull/494#discussion_r170133591
updated_at 2018-02-23 00:13:09

Reply via email to