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

platform/linux-generic/odp_ipsec.c
line 245
@@ -1165,6 +1167,8 @@ static int ipsec_out_esp(odp_packet_t *pkt,
                                 ipsec_offset + _ODP_ESPHDR_LEN,
                                 ipsec_sa->esp_iv_len,
                                 state->iv + ipsec_sa->salt_length);
+       _odp_packet_set_data(*pkt, esptrl_offset - esptrl.pad_len - tfc_len,
+                            0xa5, tfc_len);


Comment:
True, but non-zero numbers tend to stand out when used like this. Why pick this 
number vs. some other? Is there something wrong with using zeros?

> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> 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_r170133989
updated_at 2018-02-23 00:16:05

Reply via email to