On Fri, Jun 16, 2017 at 7:15 PM, Bill Fischofer
<bill.fischo...@linaro.org> wrote:
> On Fri, Jun 16, 2017 at 8:33 AM, shally verma
> <shallyvermacav...@gmail.com> wrote:
>> On Fri, Jun 16, 2017 at 6:42 PM, Bill Fischofer
>> <bill.fischo...@linaro.org> wrote:
>>> On Fri, Jun 16, 2017 at 8:04 AM, Savolainen, Petri (Nokia - FI/Espoo)
>>> <petri.savolai...@nokia.com> wrote:
>>>>> > diff --git a/include/odp/api/spec/event.h b/include/odp/api/spec/event.h
>>>>> > index f22efce5..2ad3ce84 100644
>>>>> > --- a/include/odp/api/spec/event.h
>>>>> > +++ b/include/odp/api/spec/event.h
>>>>> > @@ -37,21 +37,91 @@ extern "C" {
>>>>> >
>>>>> > /**
>>>>> > * @typedef odp_event_type_t
>>>>> > - * ODP event types:
>>>>> > - * ODP_EVENT_BUFFER, ODP_EVENT_PACKET, ODP_EVENT_TIMEOUT,
>>>>> > - * ODP_EVENT_CRYPTO_COMPL, ODP_EVENT_IPSEC_RESULT,
>>>>> ODP_EVENT_IPSEC_STATUS
>>>>> > + * Event type
>>>>> > + *
>>>>> > + * Event type specifies purpose and general format of an event. It can
>>>>> be
>>>>> > + * checked with odp_event_type() or odp_event_types(). Each event type
>>>>> has
>>>>> > + * functions (e.g. odp_buffer_from_event()) to convert between the
>>>>> generic event
>>>>> > + * handle (odp_event_t) and the type specific handle (e.g.
>>>>> odp_buffer_t).
>>>>> > + * Results are undefined, if conversion function of a wrong event type
>>>>> is used.
>>>>> > + * Application cannot change event type by chaining conversion
>>>>> functions.
>>>>> > + *
>>>>> > + * List of event types:
>>>>> > + * - ODP_EVENT_BUFFER
>>>>> > + * - Buffer event (odp_buffer_t) for simple data storage and
>>>>> message passing
>>>>> > + * - ODP_EVENT_PACKET
>>>>> > + * - Packet event (odp_packet_t) containing packet data and plenty
>>>>> of
>>>>>
>>>>> Why "plenty of"? "containing packet data and associated metadata"
>>>>> seems sufficient.
>>>>
>>>> It highlights that packets should be used only for packets, not as random
>>>> buffer space (buffers are simple vs. packets are complex).
>>>
>>> Ok, it just looked a bit odd. Not a biggie.
>>>
>>>>
>>>> BTW. It would have been nice to get all review comments in one go. I did
>>>> sent v3 as there were no more comments. I cannot do v4 before our lab is
>>>> back online from a maintenance break... Hopefully v4 is not necessary.
>>>
>>> Still working on it. This series needs Bala and Nikhil's reviews as a
>>> check against their HW. But so far this looks very reasonable.
>>>
>>>>
>>>> -Petri
>>
>> So, how does this affect event notification mechanism now?
>> Currently we had ODP_EVENT_COMP_COMPL set on a packet to mark
>> compression/decompression complete status on requested input. So that
>> still remain valid Or we need to introduce it like:
>>
>> event ODP_EVENT_PACKET
>> subtype ODP_EVENT_PACKET_COMP_COMPL?
>
> For consistency with the subtype pattern defined here I'd guess
> ODP_EVENT_PACKET_COMP would be the name to use.
>
>>
Ok. but it still leave question open for me. Should existing
Compression/Crypto completion event is going to be there or do we need
to change as per current proposal? Or current proposal is for newer
event notifications. When an compression/encryption event should be
considered as sub-event to packet? What event should be if user send a
buffer for compression/decompression than packet? May be I am missing
broader perspective to this proposal.
Also, I have bit of confusion with event type enums placement.
ODO_EVENT_PACKET/ODP_EVENT_CRYPTO_COMPL .. are these standard event
types and public? Because I see their definition inside
platform/linux-generic/include/odp/api/plat/event_types.h instead of
standard spec file at include/odp/api/spec/event.h giving me an
impression they are platform specific enum defines.
So far I was under impression because they are platform specific so we
are introducing conversion APIs like
odp_xxx_event_from_event() so that user don't need to know specific
event enumeration. As example (referring to crypto as base design) ,
we have:
odp_comp_compl_t odp_comp_compl_from_event(odp_event_t);
odp_comp_compl_result(odp_comp_compl_t comp_handle,
odp_comp_op_result_t *result);
Am I missing any base assumption here?
>> Thanks
>> Shally
