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

line 5
@@ -1286,7 +1286,7 @@ int odp_packet_l2_offset_set(odp_packet_t pkt, uint32_t 
        odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt);
-       if (offset >= pkt_hdr->frame_len)
+       if (offset > pkt_hdr->frame_len)

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:46:35

Reply via email to