> -----Original Message-----
> From: ext Taras Kondratiuk [mailto:taras.kondrat...@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-boun...@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.fischo...@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
- 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

-Petri


_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to