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