diff mbox

[API-NEXT,PATCHv4,1/6] api: packet: add user metadata APIs

Message ID 1428681138-24954-2-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer April 10, 2015, 3:52 p.m. UTC
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 include/odp/api/packet.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Taras Kondratiuk April 16, 2015, 9:05 a.m. UTC | #1
On 04/10/2015 06:52 PM, Bill Fischofer wrote:
> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>   include/odp/api/packet.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
> index a31c54d..840e152 100644
> --- a/include/odp/api/packet.h
> +++ b/include/odp/api/packet.h
> @@ -467,6 +467,26 @@ uint64_t odp_packet_user_u64(odp_packet_t pkt);
>   void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
>
>   /**
> + * Get address of user metadata associated with a packet
> + *
> + * @param pkt             Packet handle
> + *
> + * @retval addr           Address of the user metadata associated with pkt
> + * @retval NULL           The packet has no user metadata.
> + */
> +void *odp_packet_user_data(odp_packet_t pkt);
> +
> +/**
> + * Get size of user metadata associated with a packet
> + *
> + * @param pkt             Packet handle
> + *
> + * @return                Number of bytes of user metadata associated
> + *                        with pkt.
> + */
> +uint32_t odp_packet_user_data_size(odp_packet_t pkt);
> +
> +/**
>    * Layer 2 start pointer
>    *
>    * Returns pointer to the start of the layer 2 header. Optionally, outputs
>

I assume usage of user_data, user_ptr and user_u64 are all mutually
exclusive. I mean the same memory location can be used to store all of
them. It should be noted somewhere.
Ola Liljedahl April 16, 2015, 11:58 a.m. UTC | #2
On 16 April 2015 at 13:40, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia.com> wrote:

>
>
> > -----Original Message-----
> > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext
> > Taras Kondratiuk
> > Sent: Thursday, April 16, 2015 12:05 PM
> > To: Bill Fischofer; lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
> > metadata APIs
> >
> > On 04/10/2015 06:52 PM, Bill Fischofer wrote:
> > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> > > ---
> > >   include/odp/api/packet.h | 20 ++++++++++++++++++++
> > >   1 file changed, 20 insertions(+)
> > >
> > > diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
> > > index a31c54d..840e152 100644
> > > --- a/include/odp/api/packet.h
> > > +++ b/include/odp/api/packet.h
> > > @@ -467,6 +467,26 @@ uint64_t odp_packet_user_u64(odp_packet_t pkt);
> > >   void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
> > >
> > >   /**
> > > + * Get address of user metadata associated with a packet
> > > + *
> > > + * @param pkt             Packet handle
> > > + *
> > > + * @retval addr           Address of the user metadata associated with
> > pkt
> > > + * @retval NULL           The packet has no user metadata.
> > > + */
> > > +void *odp_packet_user_data(odp_packet_t pkt);
> > > +
> > > +/**
> > > + * Get size of user metadata associated with a packet
> > > + *
> > > + * @param pkt             Packet handle
> > > + *
> > > + * @return                Number of bytes of user metadata associated
> > > + *                        with pkt.
> > > + */
> > > +uint32_t odp_packet_user_data_size(odp_packet_t pkt);
> > > +
> > > +/**
> > >    * Layer 2 start pointer
> > >    *
> > >    * Returns pointer to the start of the layer 2 header. Optionally,
> > outputs
> > >
> >
> > I assume usage of user_data, user_ptr and user_u64 are all mutually
> > exclusive. I mean the same memory location can be used to store all of
> > them. It should be noted somewhere.
>
> I was thinking this as a separate area, but maybe it's clearer that we
> have only one user metadata area, which is always at least 8 bytes
> (user_ptr / user_64).

This could be the public concept (as defined by the ODP API).


> An additional area is allocated, when user requests pool_param.udata_size
> >8 bytes. Implementation can always reserve 8 bytes in the descriptor and
> use the same bytes for the pointer to a larger udata area.
>
This is a potential implementation. I don't think this should be part of
the public API. Maybe documented in the implementation notes for those
platforms that use this implementation.


>
> -Petri
>
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk April 16, 2015, 12:13 p.m. UTC | #3
On 04/16/2015 02:40 PM, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
>
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of ext
>> Taras Kondratiuk
>> Sent: Thursday, April 16, 2015 12:05 PM
>> To: Bill Fischofer; lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
>> metadata APIs
>>
>> On 04/10/2015 06:52 PM, Bill Fischofer wrote:
>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>>> ---
>>>    include/odp/api/packet.h | 20 ++++++++++++++++++++
>>>    1 file changed, 20 insertions(+)
>>>
>>> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
>>> index a31c54d..840e152 100644
>>> --- a/include/odp/api/packet.h
>>> +++ b/include/odp/api/packet.h
>>> @@ -467,6 +467,26 @@ uint64_t odp_packet_user_u64(odp_packet_t pkt);
>>>    void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
>>>
>>>    /**
>>> + * Get address of user metadata associated with a packet
>>> + *
>>> + * @param pkt             Packet handle
>>> + *
>>> + * @retval addr           Address of the user metadata associated with
>> pkt
>>> + * @retval NULL           The packet has no user metadata.
>>> + */
>>> +void *odp_packet_user_data(odp_packet_t pkt);
>>> +
>>> +/**
>>> + * Get size of user metadata associated with a packet
>>> + *
>>> + * @param pkt             Packet handle
>>> + *
>>> + * @return                Number of bytes of user metadata associated
>>> + *                        with pkt.
>>> + */
>>> +uint32_t odp_packet_user_data_size(odp_packet_t pkt);
>>> +
>>> +/**
>>>     * Layer 2 start pointer
>>>     *
>>>     * Returns pointer to the start of the layer 2 header. Optionally,
>> outputs
>>>
>>
>> I assume usage of user_data, user_ptr and user_u64 are all mutually
>> exclusive. I mean the same memory location can be used to store all of
>> them. It should be noted somewhere.
>
> I was thinking this as a separate area, but maybe it's clearer that we have only one user metadata area, which is always at least 8 bytes (user_ptr / user_64). An additional area is allocated, when user requests pool_param.udata_size >8 bytes. Implementation can always reserve 8 bytes in the descriptor and use the same bytes for the pointer to a larger udata area.

Actually having three ways (and six! API functions) to access packets'
user data is too much. And it is confusing. Now we have to explain in
comments how all these three ways affect each other.

IMO only one user_data is enough. This is exactly what we had in the
initial Buffer/Packet design document. Then for some reason we decided
to switch to just a single pointer. Then we have added u64. And now we
are bringing configurable udata back. I haven't quite followed the
logic.
Ola Liljedahl April 20, 2015, 1:39 p.m. UTC | #4
On 16 April 2015 at 15:01, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia.com> wrote:

>
>
> > -----Original Message-----
> > From: ext Taras Kondratiuk [mailto:taras.kondratiuk@linaro.org]
> > Sent: Thursday, April 16, 2015 3:13 PM
> > To: Savolainen, Petri (Nokia - FI/Espoo); Bill Fischofer
> > Cc: lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
> > metadata APIs
> >
> > On 04/16/2015 02:40 PM, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of
> > ext
> > >> Taras Kondratiuk
> > >> Sent: Thursday, April 16, 2015 12:05 PM
> > >> To: Bill Fischofer; lng-odp@lists.linaro.org
> > >> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
> > >> metadata APIs
> > >>
> > >> On 04/10/2015 06:52 PM, Bill Fischofer wrote:
> > >>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> > >>> ---
> > >>>    include/odp/api/packet.h | 20 ++++++++++++++++++++
> > >>>    1 file changed, 20 insertions(+)
> > >>>
> > >>> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
> > >>> index a31c54d..840e152 100644
> > >>> --- a/include/odp/api/packet.h
> > >>> +++ b/include/odp/api/packet.h
> > >>> @@ -467,6 +467,26 @@ uint64_t odp_packet_user_u64(odp_packet_t pkt);
> > >>>    void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
> > >>>
> > >>>    /**
> > >>> + * Get address of user metadata associated with a packet
> > >>> + *
> > >>> + * @param pkt             Packet handle
> > >>> + *
> > >>> + * @retval addr           Address of the user metadata associated
> > with
> > >> pkt
> > >>> + * @retval NULL           The packet has no user metadata.
> > >>> + */
> > >>> +void *odp_packet_user_data(odp_packet_t pkt);
> > >>> +
> > >>> +/**
> > >>> + * Get size of user metadata associated with a packet
> > >>> + *
> > >>> + * @param pkt             Packet handle
> > >>> + *
> > >>> + * @return                Number of bytes of user metadata
> associated
> > >>> + *                        with pkt.
> > >>> + */
> > >>> +uint32_t odp_packet_user_data_size(odp_packet_t pkt);
> > >>> +
> > >>> +/**
> > >>>     * Layer 2 start pointer
> > >>>     *
> > >>>     * Returns pointer to the start of the layer 2 header. Optionally,
> > >> outputs
> > >>>
> > >>
> > >> I assume usage of user_data, user_ptr and user_u64 are all mutually
> > >> exclusive. I mean the same memory location can be used to store all of
> > >> them. It should be noted somewhere.
> > >
> > > I was thinking this as a separate area, but maybe it's clearer that we
> > have only one user metadata area, which is always at least 8 bytes
> > (user_ptr / user_64). An additional area is allocated, when user requests
> > pool_param.udata_size >8 bytes. Implementation can always reserve 8 bytes
> > in the descriptor and use the same bytes for the pointer to a larger
> udata
> > area.
> >
> > Actually having three ways (and six! API functions) to access packets'
> > user data is too much. And it is confusing. Now we have to explain in
> > comments how all these three ways affect each other.
> >
> > IMO only one user_data is enough. This is exactly what we had in the
> > initial Buffer/Packet design document. Then for some reason we decided
> > to switch to just a single pointer. Then we have added u64. And now we
> > are bringing configurable udata back. I haven't quite followed the
> > logic.
>
> Simple set/get ptr is easier to pack into the packet descriptor itself
> (those bytes do not have to aligned or even linear in the memory). And
> usually one pointer is enough for the application. U64 is there to avoid
> ptr vs. uint cast problems.
>
> The variable size user data area has to be contiguous and in a certain
> alignment (8 bytes ?) and thus more difficult to fit into the descriptor
> itself (=> lower  performance due to extra pointer reference and potential
> cache miss).
>
> So, to get benefit from these we should specify that:
>
> - If pool_param.udata_size != 0, udata is supported but user_ptr/user_u64
> is not
>    * packet descriptor needs to have space only for one pointer
>
System-provided userdata area.

- If pool_param.udata_size == 0, user_ptr/user_u64 are supported but udata
> is not
>    * the pointer space in the descriptor does not have to be aligned and
> contiguous
>
User-provided userdata area (if the pointer is used, the corresponding
64-bit userdata area *is* provided by the system by reusing the memory for
the pointer).

Or call it "application-provided" userdata if the use of "user" in multiple
places is confusing?



>
> -Petri
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Bill Fischofer April 20, 2015, 7:56 p.m. UTC | #5
User metadata is orthogonal to the existing user_ptr/u64 because we didn't
change the latter when we added the former.  If we want to deprecate the
latter in favor of the former, that's a separate API change.

We have to be very careful to not make unnecessary changes to existing APIs
at this point. Introducing new APIs is fine and deprecating old ones is
fine (with an appropriate transition period), but yanking APIs as part of
introducing new ones is a no-no.  That's the sort of gratuitous change that
applications really don't like.

On Mon, Apr 20, 2015 at 6:39 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> On 16 April 2015 at 15:01, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolainen@nokia.com> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: ext Taras Kondratiuk [mailto:taras.kondratiuk@linaro.org]
>> > Sent: Thursday, April 16, 2015 3:13 PM
>> > To: Savolainen, Petri (Nokia - FI/Espoo); Bill Fischofer
>> > Cc: lng-odp@lists.linaro.org
>> > Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
>> > metadata APIs
>> >
>> > On 04/16/2015 02:40 PM, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>> > >
>> > >
>> > >> -----Original Message-----
>> > >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of
>> > ext
>> > >> Taras Kondratiuk
>> > >> Sent: Thursday, April 16, 2015 12:05 PM
>> > >> To: Bill Fischofer; lng-odp@lists.linaro.org
>> > >> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
>> > >> metadata APIs
>> > >>
>> > >> On 04/10/2015 06:52 PM, Bill Fischofer wrote:
>> > >>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>> > >>> ---
>> > >>>    include/odp/api/packet.h | 20 ++++++++++++++++++++
>> > >>>    1 file changed, 20 insertions(+)
>> > >>>
>> > >>> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
>> > >>> index a31c54d..840e152 100644
>> > >>> --- a/include/odp/api/packet.h
>> > >>> +++ b/include/odp/api/packet.h
>> > >>> @@ -467,6 +467,26 @@ uint64_t odp_packet_user_u64(odp_packet_t pkt);
>> > >>>    void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
>> > >>>
>> > >>>    /**
>> > >>> + * Get address of user metadata associated with a packet
>> > >>> + *
>> > >>> + * @param pkt             Packet handle
>> > >>> + *
>> > >>> + * @retval addr           Address of the user metadata associated
>> > with
>> > >> pkt
>> > >>> + * @retval NULL           The packet has no user metadata.
>> > >>> + */
>> > >>> +void *odp_packet_user_data(odp_packet_t pkt);
>> > >>> +
>> > >>> +/**
>> > >>> + * Get size of user metadata associated with a packet
>> > >>> + *
>> > >>> + * @param pkt             Packet handle
>> > >>> + *
>> > >>> + * @return                Number of bytes of user metadata
>> associated
>> > >>> + *                        with pkt.
>> > >>> + */
>> > >>> +uint32_t odp_packet_user_data_size(odp_packet_t pkt);
>> > >>> +
>> > >>> +/**
>> > >>>     * Layer 2 start pointer
>> > >>>     *
>> > >>>     * Returns pointer to the start of the layer 2 header.
>> Optionally,
>> > >> outputs
>> > >>>
>> > >>
>> > >> I assume usage of user_data, user_ptr and user_u64 are all mutually
>> > >> exclusive. I mean the same memory location can be used to store all
>> of
>> > >> them. It should be noted somewhere.
>> > >
>> > > I was thinking this as a separate area, but maybe it's clearer that we
>> > have only one user metadata area, which is always at least 8 bytes
>> > (user_ptr / user_64). An additional area is allocated, when user
>> requests
>> > pool_param.udata_size >8 bytes. Implementation can always reserve 8
>> bytes
>> > in the descriptor and use the same bytes for the pointer to a larger
>> udata
>> > area.
>> >
>> > Actually having three ways (and six! API functions) to access packets'
>> > user data is too much. And it is confusing. Now we have to explain in
>> > comments how all these three ways affect each other.
>> >
>> > IMO only one user_data is enough. This is exactly what we had in the
>> > initial Buffer/Packet design document. Then for some reason we decided
>> > to switch to just a single pointer. Then we have added u64. And now we
>> > are bringing configurable udata back. I haven't quite followed the
>> > logic.
>>
>> Simple set/get ptr is easier to pack into the packet descriptor itself
>> (those bytes do not have to aligned or even linear in the memory). And
>> usually one pointer is enough for the application. U64 is there to avoid
>> ptr vs. uint cast problems.
>>
>> The variable size user data area has to be contiguous and in a certain
>> alignment (8 bytes ?) and thus more difficult to fit into the descriptor
>> itself (=> lower  performance due to extra pointer reference and potential
>> cache miss).
>>
>> So, to get benefit from these we should specify that:
>>
>> - If pool_param.udata_size != 0, udata is supported but user_ptr/user_u64
>> is not
>>    * packet descriptor needs to have space only for one pointer
>>
> System-provided userdata area.
>
> - If pool_param.udata_size == 0, user_ptr/user_u64 are supported but udata
>> is not
>>    * the pointer space in the descriptor does not have to be aligned and
>> contiguous
>>
> User-provided userdata area (if the pointer is used, the corresponding
> 64-bit userdata area *is* provided by the system by reusing the memory for
> the pointer).
>
> Or call it "application-provided" userdata if the use of "user" in
> multiple places is confusing?
>
>
>
>>
>> -Petri
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
Mike Holmes April 20, 2015, 8:14 p.m. UTC | #6
On 20 April 2015 at 15:56, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> User metadata is orthogonal to the existing user_ptr/u64 because we didn't
> change the latter when we added the former.  If we want to deprecate the
> latter in favor of the former, that's a separate API change.
>
> We have to be very careful to not make unnecessary changes to existing
> APIs at this point. Introducing new APIs is fine and deprecating old ones
> is fine (with an appropriate transition period), but yanking APIs as part
> of introducing new ones is a no-no.  That's the sort of gratuitous change
> that applications really don't like.
>

I think you have a good point - we have a marker in the API already

#define ODP_DEPRECATED
<http://docs.opendataplane.org/linux-generic-doxygen-html/group__odp__compiler__optim.html#ga76d645458c64fb14622e94b5bceae186>
   __attribute__((__deprecated__)) Indicate deprecated variables, functions
or types.

But we have never before had to depreciate anything, our guide lines
<http://docs.opendataplane.org/linux-generic-doxygen-html/api_guide_lines.html>
should
be updated to state our deprecation policy, I will try gather some words.

I propose we can leave it in the public API for the next 2 major releases
perhaps. Of course then a vendor should possibly be free to not implement
deprecated APIs, their customers may not be happy. This ripples on becasue
then those deprecated APIs need to be come part of the optional tests in
test/validation.

On Mon, Apr 20, 2015 at 6:39 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
> wrote:
>
>> On 16 April 2015 at 15:01, Savolainen, Petri (Nokia - FI/Espoo) <
>> petri.savolainen@nokia.com> wrote:
>>
>>>
>>>
>>> > -----Original Message-----
>>> > From: ext Taras Kondratiuk [mailto:taras.kondratiuk@linaro.org]
>>> > Sent: Thursday, April 16, 2015 3:13 PM
>>> > To: Savolainen, Petri (Nokia - FI/Espoo); Bill Fischofer
>>> > Cc: lng-odp@lists.linaro.org
>>> > Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
>>> > metadata APIs
>>> >
>>> > On 04/16/2015 02:40 PM, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>> > >
>>> > >
>>> > >> -----Original Message-----
>>> > >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf
>>> Of
>>> > ext
>>> > >> Taras Kondratiuk
>>> > >> Sent: Thursday, April 16, 2015 12:05 PM
>>> > >> To: Bill Fischofer; lng-odp@lists.linaro.org
>>> > >> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/6] api: packet: add user
>>> > >> metadata APIs
>>> > >>
>>> > >> On 04/10/2015 06:52 PM, Bill Fischofer wrote:
>>> > >>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>>> > >>> ---
>>> > >>>    include/odp/api/packet.h | 20 ++++++++++++++++++++
>>> > >>>    1 file changed, 20 insertions(+)
>>> > >>>
>>> > >>> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
>>> > >>> index a31c54d..840e152 100644
>>> > >>> --- a/include/odp/api/packet.h
>>> > >>> +++ b/include/odp/api/packet.h
>>> > >>> @@ -467,6 +467,26 @@ uint64_t odp_packet_user_u64(odp_packet_t
>>> pkt);
>>> > >>>    void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
>>> > >>>
>>> > >>>    /**
>>> > >>> + * Get address of user metadata associated with a packet
>>> > >>> + *
>>> > >>> + * @param pkt             Packet handle
>>> > >>> + *
>>> > >>> + * @retval addr           Address of the user metadata associated
>>> > with
>>> > >> pkt
>>> > >>> + * @retval NULL           The packet has no user metadata.
>>> > >>> + */
>>> > >>> +void *odp_packet_user_data(odp_packet_t pkt);
>>> > >>> +
>>> > >>> +/**
>>> > >>> + * Get size of user metadata associated with a packet
>>> > >>> + *
>>> > >>> + * @param pkt             Packet handle
>>> > >>> + *
>>> > >>> + * @return                Number of bytes of user metadata
>>> associated
>>> > >>> + *                        with pkt.
>>> > >>> + */
>>> > >>> +uint32_t odp_packet_user_data_size(odp_packet_t pkt);
>>> > >>> +
>>> > >>> +/**
>>> > >>>     * Layer 2 start pointer
>>> > >>>     *
>>> > >>>     * Returns pointer to the start of the layer 2 header.
>>> Optionally,
>>> > >> outputs
>>> > >>>
>>> > >>
>>> > >> I assume usage of user_data, user_ptr and user_u64 are all mutually
>>> > >> exclusive. I mean the same memory location can be used to store all
>>> of
>>> > >> them. It should be noted somewhere.
>>> > >
>>> > > I was thinking this as a separate area, but maybe it's clearer that
>>> we
>>> > have only one user metadata area, which is always at least 8 bytes
>>> > (user_ptr / user_64). An additional area is allocated, when user
>>> requests
>>> > pool_param.udata_size >8 bytes. Implementation can always reserve 8
>>> bytes
>>> > in the descriptor and use the same bytes for the pointer to a larger
>>> udata
>>> > area.
>>> >
>>> > Actually having three ways (and six! API functions) to access packets'
>>> > user data is too much. And it is confusing. Now we have to explain in
>>> > comments how all these three ways affect each other.
>>> >
>>> > IMO only one user_data is enough. This is exactly what we had in the
>>> > initial Buffer/Packet design document. Then for some reason we decided
>>> > to switch to just a single pointer. Then we have added u64. And now we
>>> > are bringing configurable udata back. I haven't quite followed the
>>> > logic.
>>>
>>> Simple set/get ptr is easier to pack into the packet descriptor itself
>>> (those bytes do not have to aligned or even linear in the memory). And
>>> usually one pointer is enough for the application. U64 is there to avoid
>>> ptr vs. uint cast problems.
>>>
>>> The variable size user data area has to be contiguous and in a certain
>>> alignment (8 bytes ?) and thus more difficult to fit into the descriptor
>>> itself (=> lower  performance due to extra pointer reference and potential
>>> cache miss).
>>>
>>> So, to get benefit from these we should specify that:
>>>
>>> - If pool_param.udata_size != 0, udata is supported but
>>> user_ptr/user_u64 is not
>>>    * packet descriptor needs to have space only for one pointer
>>>
>> System-provided userdata area.
>>
>> - If pool_param.udata_size == 0, user_ptr/user_u64 are supported but
>>> udata is not
>>>    * the pointer space in the descriptor does not have to be aligned and
>>> contiguous
>>>
>> User-provided userdata area (if the pointer is used, the corresponding
>> 64-bit userdata area *is* provided by the system by reusing the memory for
>> the pointer).
>>
>> Or call it "application-provided" userdata if the use of "user" in
>> multiple places is confusing?
>>
>>
>>
>>>
>>> -Petri
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
diff mbox

Patch

diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
index a31c54d..840e152 100644
--- a/include/odp/api/packet.h
+++ b/include/odp/api/packet.h
@@ -467,6 +467,26 @@  uint64_t odp_packet_user_u64(odp_packet_t pkt);
 void odp_packet_user_u64_set(odp_packet_t pkt, uint64_t ctx);
 
 /**
+ * Get address of user metadata associated with a packet
+ *
+ * @param pkt             Packet handle
+ *
+ * @retval addr           Address of the user metadata associated with pkt
+ * @retval NULL           The packet has no user metadata.
+ */
+void *odp_packet_user_data(odp_packet_t pkt);
+
+/**
+ * Get size of user metadata associated with a packet
+ *
+ * @param pkt             Packet handle
+ *
+ * @return                Number of bytes of user metadata associated
+ *                        with pkt.
+ */
+uint32_t odp_packet_user_data_size(odp_packet_t pkt);
+
+/**
  * Layer 2 start pointer
  *
  * Returns pointer to the start of the layer 2 header. Optionally, outputs