Dmitry Eremin-Solenikov(lumag) replied on github web page:

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

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.
updated_at 2018-02-22 08:47:03

Reply via email to