Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
as discussed on meeting merged to api-next. Petri will do all required renames. Best regards, Maxim. On 04/10/15 18:52, Bill Fischofer wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any associated user metadata as part of that operation, but ODP will otherwise ignore these bytes. Bill Fischofer (6): api: packet: add user metadata APIs api: pool: add user metadata APIs linux-generic: buffer: restructure user mdatadata fields linux-generic: pool: add user metadata support linux-generic: packet: add user metadata support validation: packet: add user metadata tests include/odp/api/packet.h | 20 include/odp/api/pool.h | 4 ++ .../linux-generic/include/odp_buffer_internal.h| 4 +- platform/linux-generic/include/odp_pool_internal.h | 2 +- platform/linux-generic/odp_packet.c| 34 ++ platform/linux-generic/odp_pool.c | 34 +- test/validation/odp_packet.c | 54 ++ 7 files changed, 126 insertions(+), 26 deletions(-) ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org] Sent: Thursday, April 23, 2015 8:29 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: ext Bill Fischofer; lng-odp@lists.linaro.org Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support On 22 April 2015 at 15:01, Savolainen, Petri (Nokia - FI/Espoo) petri.savolai...@nokia.commailto:petri.savolai...@nokia.com wrote: There is queue level context (odp_queue_get_context() / odp_queue_set_context()), which is (close to) flow level (depending on flow to queue mapping). Shouldn't these calls also follow the get/set conventions previously defined and implemented? E.g. call the functions odp_queue_context() and odp_queue_context_set()? Yes. Those should be renamed. -Petri ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
There is queue level context (odp_queue_get_context() / odp_queue_set_context()), which is (close to) flow level (depending on flow to queue mapping). -Petri From: ext Bill Fischofer [mailto:bill.fischo...@linaro.org] Sent: Wednesday, April 22, 2015 3:50 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: ext Zoltan Kiss; lng-odp@lists.linaro.org Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support The question of prefetching is part of the larger issue of context management, which is something we discussed last year and then deferred. I agree that this is key to achieving best performance, but we should address that as a first-order design rather than trying to tack it onto some existing APIs. We need to have the ability to identify flow-level context that would be maintained with coherence across multiple packets associated with a flow. As you note, the per-packet metadata isn't flow-related but rather packet-related and so doesn't really cover this use case. On Wed, Apr 22, 2015 at 7:18 AM, Savolainen, Petri (Nokia - FI/Espoo) petri.savolai...@nokia.commailto:petri.savolai...@nokia.com wrote: I’m thinking different usage for these. The context pointer (odp_packet_userctx_ptr()) would point to some data structure in the memory, where as odp_packet_userarea() would be used as a per packet scratchpad memory. When implementation knows that the user stores a pointer, it can also prefetch the data behind the pointer. If the pointer is saved as part of the userarea, implementation cannot know where it is. Other advantage comes from get/set accessors vs random memory access – bits can be packed freely vs. must be contiguous with certain (8 byte) alignment. Enabling prefething is likely more important for performance, than bit packing. For shorter/cleaner API, we could drop _userctx_u64(). -Petri From: ext Bill Fischofer [mailto:bill.fischo...@linaro.orgmailto:bill.fischo...@linaro.org] Sent: Wednesday, April 22, 2015 2:46 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: ext Zoltan Kiss; lng-odp@lists.linaro.orgmailto:lng-odp@lists.linaro.org Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support I'm not sure why we wouldn't want to just deprecate the odp_packet_user_ptr/_u64() calls. They were stopgaps since we didn't have user metadata in v1.0. Now that we have that, what's the purpose of keeping the earlier calls? The latter is a more flexible and capable feature, and can be equally efficient for small user metadata areas (implementation choice). On Wed, Apr 22, 2015 at 6:37 AM, Savolainen, Petri (Nokia - FI/Espoo) petri.savolai...@nokia.commailto:petri.savolai...@nokia.com wrote: Hi, After thinking this again, it's cleaner for the API (and the user) if these two user metadata fields are kept separate (as they currently are). The user_ptr/_u64 is always present and user_data only when param.pkt.udata_size is non-zero. Implementations have different ways to store both of these into the packet descriptor (maybe pointer to user_data is not needed, but it can be found with a fixed offset, etc). However the naming of these two could be cleaner (== highlight the dependencies): - odp_packet_user_ptr/_u64()could be renamed to odp_packet_userctx_ptr/_u64() - odp_packet_user_data/_data_size() could be renamed to odp_packet_userarea/userarea_size() - we should avoid confusion with similar terms in DPDK - mbuf.userdata == current odp_packet_user_ptr() - mbuf.udata64 == current odp_packet_user_u64() Opinions? I can send a patch for renames and documentation improvements. -Petri -Original Message- From: ext Zoltan Kiss [mailto:zoltan.k...@linaro.orgmailto:zoltan.k...@linaro.org] Sent: Tuesday, April 21, 2015 4:08 PM To: Bill Fischofer; lng-odp@lists.linaro.orgmailto:lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support It seems to me all questions were answered, is there anything which prevents applying this into master branch? Petri? On 10/04/15 16:52, Bill Fischofer wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
The question of prefetching is part of the larger issue of context management, which is something we discussed last year and then deferred. I agree that this is key to achieving best performance, but we should address that as a first-order design rather than trying to tack it onto some existing APIs. We need to have the ability to identify flow-level context that would be maintained with coherence across multiple packets associated with a flow. As you note, the per-packet metadata isn't flow-related but rather packet-related and so doesn't really cover this use case. On Wed, Apr 22, 2015 at 7:18 AM, Savolainen, Petri (Nokia - FI/Espoo) petri.savolai...@nokia.com wrote: I’m thinking different usage for these. The context pointer (odp_packet_userctx_ptr()) would point to some data structure in the memory, where as odp_packet_userarea() would be used as a per packet scratchpad memory. When implementation knows that the user stores a pointer, it can also prefetch the data behind the pointer. If the pointer is saved as part of the userarea, implementation cannot know where it is. Other advantage comes from get/set accessors vs random memory access – bits can be packed freely vs. must be contiguous with certain (8 byte) alignment. Enabling prefething is likely more important for performance, than bit packing. For shorter/cleaner API, we could drop _userctx_u64(). -Petri *From:* ext Bill Fischofer [mailto:bill.fischo...@linaro.org] *Sent:* Wednesday, April 22, 2015 2:46 PM *To:* Savolainen, Petri (Nokia - FI/Espoo) *Cc:* ext Zoltan Kiss; lng-odp@lists.linaro.org *Subject:* Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support I'm not sure why we wouldn't want to just deprecate the odp_packet_user_ptr/_u64() calls. They were stopgaps since we didn't have user metadata in v1.0. Now that we have that, what's the purpose of keeping the earlier calls? The latter is a more flexible and capable feature, and can be equally efficient for small user metadata areas (implementation choice). On Wed, Apr 22, 2015 at 6:37 AM, Savolainen, Petri (Nokia - FI/Espoo) petri.savolai...@nokia.com wrote: Hi, After thinking this again, it's cleaner for the API (and the user) if these two user metadata fields are kept separate (as they currently are). The user_ptr/_u64 is always present and user_data only when param.pkt.udata_size is non-zero. Implementations have different ways to store both of these into the packet descriptor (maybe pointer to user_data is not needed, but it can be found with a fixed offset, etc). However the naming of these two could be cleaner (== highlight the dependencies): - odp_packet_user_ptr/_u64()could be renamed to odp_packet_userctx_ptr/_u64() - odp_packet_user_data/_data_size() could be renamed to odp_packet_userarea/userarea_size() - we should avoid confusion with similar terms in DPDK - mbuf.userdata == current odp_packet_user_ptr() - mbuf.udata64 == current odp_packet_user_u64() Opinions? I can send a patch for renames and documentation improvements. -Petri -Original Message- From: ext Zoltan Kiss [mailto:zoltan.k...@linaro.org] Sent: Tuesday, April 21, 2015 4:08 PM To: Bill Fischofer; lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support It seems to me all questions were answered, is there anything which prevents applying this into master branch? Petri? On 10/04/15 16:52, Bill Fischofer wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any associated user metadata as part of that operation, but ODP will otherwise ignore these bytes. Bill Fischofer (6): api: packet: add user metadata APIs api: pool: add user metadata APIs linux-generic: buffer: restructure user mdatadata fields linux-generic: pool: add user metadata support linux-generic: packet: add user metadata support validation: packet: add user metadata tests include/odp/api/packet.h | 20 include/odp/api/pool.h | 4
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
Hi, After thinking this again, it's cleaner for the API (and the user) if these two user metadata fields are kept separate (as they currently are). The user_ptr/_u64 is always present and user_data only when param.pkt.udata_size is non-zero. Implementations have different ways to store both of these into the packet descriptor (maybe pointer to user_data is not needed, but it can be found with a fixed offset, etc). However the naming of these two could be cleaner (== highlight the dependencies): - odp_packet_user_ptr/_u64()could be renamed to odp_packet_userctx_ptr/_u64() - odp_packet_user_data/_data_size() could be renamed to odp_packet_userarea/userarea_size() - we should avoid confusion with similar terms in DPDK - mbuf.userdata == current odp_packet_user_ptr() - mbuf.udata64 == current odp_packet_user_u64() Opinions? I can send a patch for renames and documentation improvements. -Petri -Original Message- From: ext Zoltan Kiss [mailto:zoltan.k...@linaro.org] Sent: Tuesday, April 21, 2015 4:08 PM To: Bill Fischofer; lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support It seems to me all questions were answered, is there anything which prevents applying this into master branch? Petri? On 10/04/15 16:52, Bill Fischofer wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any associated user metadata as part of that operation, but ODP will otherwise ignore these bytes. Bill Fischofer (6): api: packet: add user metadata APIs api: pool: add user metadata APIs linux-generic: buffer: restructure user mdatadata fields linux-generic: pool: add user metadata support linux-generic: packet: add user metadata support validation: packet: add user metadata tests include/odp/api/packet.h | 20 include/odp/api/pool.h | 4 ++ .../linux-generic/include/odp_buffer_internal.h| 4 +- platform/linux-generic/include/odp_pool_internal.h | 2 +- platform/linux-generic/odp_packet.c| 34 ++ platform/linux-generic/odp_pool.c | 34 +- test/validation/odp_packet.c | 54 ++ 7 files changed, 126 insertions(+), 26 deletions(-) ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
I’m thinking different usage for these. The context pointer (odp_packet_userctx_ptr()) would point to some data structure in the memory, where as odp_packet_userarea() would be used as a per packet scratchpad memory. When implementation knows that the user stores a pointer, it can also prefetch the data behind the pointer. If the pointer is saved as part of the userarea, implementation cannot know where it is. Other advantage comes from get/set accessors vs random memory access – bits can be packed freely vs. must be contiguous with certain (8 byte) alignment. Enabling prefething is likely more important for performance, than bit packing. For shorter/cleaner API, we could drop _userctx_u64(). -Petri From: ext Bill Fischofer [mailto:bill.fischo...@linaro.org] Sent: Wednesday, April 22, 2015 2:46 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: ext Zoltan Kiss; lng-odp@lists.linaro.org Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support I'm not sure why we wouldn't want to just deprecate the odp_packet_user_ptr/_u64() calls. They were stopgaps since we didn't have user metadata in v1.0. Now that we have that, what's the purpose of keeping the earlier calls? The latter is a more flexible and capable feature, and can be equally efficient for small user metadata areas (implementation choice). On Wed, Apr 22, 2015 at 6:37 AM, Savolainen, Petri (Nokia - FI/Espoo) petri.savolai...@nokia.commailto:petri.savolai...@nokia.com wrote: Hi, After thinking this again, it's cleaner for the API (and the user) if these two user metadata fields are kept separate (as they currently are). The user_ptr/_u64 is always present and user_data only when param.pkt.udata_size is non-zero. Implementations have different ways to store both of these into the packet descriptor (maybe pointer to user_data is not needed, but it can be found with a fixed offset, etc). However the naming of these two could be cleaner (== highlight the dependencies): - odp_packet_user_ptr/_u64()could be renamed to odp_packet_userctx_ptr/_u64() - odp_packet_user_data/_data_size() could be renamed to odp_packet_userarea/userarea_size() - we should avoid confusion with similar terms in DPDK - mbuf.userdata == current odp_packet_user_ptr() - mbuf.udata64 == current odp_packet_user_u64() Opinions? I can send a patch for renames and documentation improvements. -Petri -Original Message- From: ext Zoltan Kiss [mailto:zoltan.k...@linaro.orgmailto:zoltan.k...@linaro.org] Sent: Tuesday, April 21, 2015 4:08 PM To: Bill Fischofer; lng-odp@lists.linaro.orgmailto:lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) Subject: Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support It seems to me all questions were answered, is there anything which prevents applying this into master branch? Petri? On 10/04/15 16:52, Bill Fischofer wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any associated user metadata as part of that operation, but ODP will otherwise ignore these bytes. Bill Fischofer (6): api: packet: add user metadata APIs api: pool: add user metadata APIs linux-generic: buffer: restructure user mdatadata fields linux-generic: pool: add user metadata support linux-generic: packet: add user metadata support validation: packet: add user metadata tests include/odp/api/packet.h | 20 include/odp/api/pool.h | 4 ++ .../linux-generic/include/odp_buffer_internal.h| 4 +- platform/linux-generic/include/odp_pool_internal.h | 2 +- platform/linux-generic/odp_packet.c| 34 ++ platform/linux-generic/odp_pool.c | 34 +- test/validation/odp_packet.c | 54 ++ 7 files changed, 126 insertions(+), 26 deletions(-) ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
It seems to me all questions were answered, is there anything which prevents applying this into master branch? Petri? On 10/04/15 16:52, Bill Fischofer wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any associated user metadata as part of that operation, but ODP will otherwise ignore these bytes. Bill Fischofer (6): api: packet: add user metadata APIs api: pool: add user metadata APIs linux-generic: buffer: restructure user mdatadata fields linux-generic: pool: add user metadata support linux-generic: packet: add user metadata support validation: packet: add user metadata tests include/odp/api/packet.h | 20 include/odp/api/pool.h | 4 ++ .../linux-generic/include/odp_buffer_internal.h| 4 +- platform/linux-generic/include/odp_pool_internal.h | 2 +- platform/linux-generic/odp_packet.c| 34 ++ platform/linux-generic/odp_pool.c | 34 +- test/validation/odp_packet.c | 54 ++ 7 files changed, 126 insertions(+), 26 deletions(-) ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv4 0/6] Add packet user metadata support
Two minor aspect related nits, but this looks good to me. Reviewed-and-tested-by: Ciprian Barbu ciprian.ba...@linaro.org On Fri, Apr 10, 2015 at 6:52 PM, Bill Fischofer bill.fischo...@linaro.org wrote: v4 changes: - Removed RFC status, patch is now ready for API-NEXT - Added implementation of approved APIs - Added user metadata tests to packet validation test v3 changes: - Renamed odp_packet_user_metadata() to odp_packet_user_data() - Split addr/size return, adding odp_packet_user_data_size() - Moved udata_size to pkt structure within odp_pool_param_t v2 changes: - Moved udata_size to odp_pool_param_t - Renamed odp_packet_udata() to odp_packet_user_metadata() - Removed odp_buffer_udata(). User metadata is for packets only RFC for proposed minimal API set for user metadata support based on today's discussions. Note that all initialization and management of user metadata contents is the responsibility of the ODP application. ODP APIs that copy system metadata will also copy any associated user metadata as part of that operation, but ODP will otherwise ignore these bytes. Bill Fischofer (6): api: packet: add user metadata APIs api: pool: add user metadata APIs linux-generic: buffer: restructure user mdatadata fields linux-generic: pool: add user metadata support linux-generic: packet: add user metadata support validation: packet: add user metadata tests include/odp/api/packet.h | 20 include/odp/api/pool.h | 4 ++ .../linux-generic/include/odp_buffer_internal.h| 4 +- platform/linux-generic/include/odp_pool_internal.h | 2 +- platform/linux-generic/odp_packet.c| 34 ++ platform/linux-generic/odp_pool.c | 34 +- test/validation/odp_packet.c | 54 ++ 7 files changed, 126 insertions(+), 26 deletions(-) -- 2.1.0 ___ 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
