Dmitry Eremin-Solenikov(lumag) replied on github web page: platform/linux-generic/odp_packet.c line 5 @@ -1286,7 +1286,7 @@ int odp_packet_l2_offset_set(odp_packet_t pkt, uint32_t offset) { odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt); - if (offset >= pkt_hdr->frame_len) + if (offset > pkt_hdr->frame_len)
Comment: 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_r169886720 updated_at 2018-02-22 08:46:35