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.liljed...@linaro.org> wrote: > On 16 April 2015 at 15:01, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolai...@nokia.com> wrote: > >> >> >> > -----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 >> > 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
