On Fri, Jun 16, 2017 at 5:21 AM, Dmitry Eremin-Solenikov
<dmitry.ereminsoleni...@linaro.org> wrote:
> On 16.06.2017 12:37, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>>>> switch (odp_event_type(event)) {
>>>>> diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-
>>>>> generic/odp_packet.c
>>>>> index eb66af2d3b9c..3789feca45f9 100644
>>>>> --- a/platform/linux-generic/odp_packet.c
>>>>> +++ b/platform/linux-generic/odp_packet.c
>>>>> @@ -268,6 +268,7 @@ static inline void packet_init(odp_packet_hdr_t
>>>>> *pkt_hdr, uint32_t len)
>>>>> CONFIG_PACKET_TAILROOM;
>>>>>
>>>>> pkt_hdr->input = ODP_PKTIO_INVALID;
>>>>> + pkt_hdr->buf_hdr.event_subtype = ODP_EVENT_PACKET_BASIC;
>>>>
>>>>
>>>> This is not needed if you update crypto.c with
>>> _odp_buffer_event_subtype_set() calls, where _odp_buffer_event_type_set()
>>> is done already -right? Packet_init() is done for every alloc and should
>>> avoid setting constant data.
>>>
>>> I gave this idea a thought. I will update crypto.c (thanks for the
>>> point!), but I still insist that packet_init should set subtype.
>>> Otherwise subtype resetting should go into packet free code (which is
>>> uglier) in my opinion. Consider ODP application receiving IPsec packets
>>> from queue then freeing them for some reason before doing
>>> odp_ipsec_result() call. Packet will be freed, but event_subtype will be
>>> left as PACKET_IPSEC.
>>
>> OK. We will optimize it later if needed: either set subtype to basic only
>> when it's not already basic, or add subtype as packet_init() argument (to
>> avoid first setting it to basic and then to ipsec).
>
> Agreed.
packet_init() is a critical-path routine that should not be added to
lightly. Subtypes can be added at zero cost by simply taking a couple
of bits from the parser input flags _odp_packet_input_flags_t in
include/odp/api/plat/packet_types.h and using them to encode packet
subtype. Since these flags are set to zeros anyway by packet_init()
there's no additional cost and zeros will be the value for
ODP_PACKET_BASIC. Note that this will also automatically reset the
subtype on odp_packet_reset() calls, which is something else that's
needed.
odp_event_subtype() will return ODP_EVENT_SUBTYPE_NONE for anything
other than packets anyway, so there's no need to have a subtype
encoding in anything other than packets for now.
>
> --
> With best wishes
> Dmitry
