| Message ID | 1427244794-9964-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 25, 2015 at 2:53 AM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > > 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. > > include/odp/api/buffer.h | 11 +++++++++++ > include/odp/api/packet.h | 11 +++++++++++ > include/odp/api/pool.h | 15 ++++++++++++++- > 3 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h > index 9ad08ea..98a66ee 100644 > --- a/include/odp/api/buffer.h > +++ b/include/odp/api/buffer.h > @@ -145,6 +145,17 @@ void odp_buffer_print(odp_buffer_t buf); > uint64_t odp_buffer_to_u64(odp_buffer_t hdl); > > /** > + * Get address and size of user metadata associated with a buffer > + * > + * @param buf Buffer handle > + * @param udata_size[out] Number of bytes of user metadata available > + * at the returned address > + * @return Address of the user metadata for this buffer > + * or NULL if the buffer has no user metadata. > + */ > +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); > + > +/** > * @} > */ > > diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h > index a31c54d..eb392e4 100644 > --- a/include/odp/api/packet.h > +++ b/include/odp/api/packet.h > @@ -193,6 +193,17 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); > void *odp_packet_data(odp_packet_t pkt); > > /** > + * Get address and size of user metadata associated with a packet > + * > + * @param pkt Packet handle > + * @param udata_size[out] Number of bytes of user metadata available > + * at the returned address > + * @return Address of the usermeta data for this buffer > + * or NULL if the buffer has no user metadata. > + */ > +void *odp_packet_udata(odp_packet_t pkt, uint32_t *udata_size); Would it be better to call it odp_packet_get_udata ? > + > +/** > * Packet segment data length > * > * Returns number of data bytes following the current data pointer > diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h > index 241b98a..57f026c 100644 > --- a/include/odp/api/pool.h > +++ b/include/odp/api/pool.h > @@ -85,6 +85,14 @@ typedef struct odp_pool_param_t { > int type; /**< Pool type */ > } odp_pool_param_t; > > +/** > + * Pool options > + * Used to communicate pool initialization options. > + */ > +typedef struct odp_pool_init_t { > + uint32_t udata_size; /**< Size of user metadata in bytes */ > +} odp_pool_init_t; > + > /** Packet pool*/ > #define ODP_POOL_PACKET ODP_EVENT_PACKET > /** Buffer pool */ > @@ -109,13 +117,17 @@ typedef struct odp_pool_param_t { > * > * @param params Pool parameters. > * > + * @param init_params Pool initialization options. May be specified as NULL > + * to indicate default options are to be used. > + * > * @return Handle of the created pool > * @retval ODP_POOL_INVALID Pool could not be created > */ > > odp_pool_t odp_pool_create(const char *name, > odp_shm_t shm, > - odp_pool_param_t *params); > + odp_pool_param_t *params, > + odp_pool_init_t *init_params); > > /** > * Destroy a pool previously created by odp_pool_create() > @@ -161,6 +173,7 @@ typedef struct odp_pool_info_t { > ODP_SHM_INVALID if this pool is > managed by ODP */ > odp_pool_param_t params; /**< pool parameters */ > + odp_pool_init_t init_params; /**< pool initialization parameters */ > } odp_pool_info_t; > > /** > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 25 March 2015 at 09:38, Ciprian Barbu <ciprian.barbu@linaro.org> wrote: > On Wed, Mar 25, 2015 at 2:53 AM, Bill Fischofer > <bill.fischofer@linaro.org> wrote: > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > > --- > > > > 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. > > > > include/odp/api/buffer.h | 11 +++++++++++ > > include/odp/api/packet.h | 11 +++++++++++ > > include/odp/api/pool.h | 15 ++++++++++++++- > > 3 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h > > index 9ad08ea..98a66ee 100644 > > --- a/include/odp/api/buffer.h > > +++ b/include/odp/api/buffer.h > > @@ -145,6 +145,17 @@ void odp_buffer_print(odp_buffer_t buf); > > uint64_t odp_buffer_to_u64(odp_buffer_t hdl); > > > > /** > > + * Get address and size of user metadata associated with a buffer > > + * > > + * @param buf Buffer handle > > + * @param udata_size[out] Number of bytes of user metadata available > > + * at the returned address > > + * @return Address of the user metadata for this buffer > > + * or NULL if the buffer has no user metadata. > > + */ > > +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); > > + > > +/** > > * @} > > */ > > > > diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h > > index a31c54d..eb392e4 100644 > > --- a/include/odp/api/packet.h > > +++ b/include/odp/api/packet.h > > @@ -193,6 +193,17 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); > > void *odp_packet_data(odp_packet_t pkt); > > > > /** > > + * Get address and size of user metadata associated with a packet > > + * > > + * @param pkt Packet handle > > + * @param udata_size[out] Number of bytes of user metadata available > > + * at the returned address > > + * @return Address of the usermeta data for this buffer > > + * or NULL if the buffer has no user metadata. > > + */ > > +void *odp_packet_udata(odp_packet_t pkt, uint32_t *udata_size); > > Would it be better to call it odp_packet_get_udata ? > No, as a rule "get" accessors don't only the word get while "set" accessors end with "_set". odp_queue_get_context() and odp_queue_set_context() seems to be the exception to this rule, they should be renamed to odp_queue_context() and odp_queue_context_set(). > > + > > +/** > > * Packet segment data length > > * > > * Returns number of data bytes following the current data pointer > > diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h > > index 241b98a..57f026c 100644 > > --- a/include/odp/api/pool.h > > +++ b/include/odp/api/pool.h > > @@ -85,6 +85,14 @@ typedef struct odp_pool_param_t { > > int type; /**< Pool type */ > > } odp_pool_param_t; > > > > +/** > > + * Pool options > > + * Used to communicate pool initialization options. > > + */ > > +typedef struct odp_pool_init_t { > > + uint32_t udata_size; /**< Size of user metadata in > bytes */ > > +} odp_pool_init_t; > > + > > /** Packet pool*/ > > #define ODP_POOL_PACKET ODP_EVENT_PACKET > > /** Buffer pool */ > > @@ -109,13 +117,17 @@ typedef struct odp_pool_param_t { > > * > > * @param params Pool parameters. > > * > > + * @param init_params Pool initialization options. May be specified > as NULL > > + * to indicate default options are to be used. > > + * > > * @return Handle of the created pool > > * @retval ODP_POOL_INVALID Pool could not be created > > */ > > > > odp_pool_t odp_pool_create(const char *name, > > odp_shm_t shm, > > - odp_pool_param_t *params); > > + odp_pool_param_t *params, > > + odp_pool_init_t *init_params); > > > > /** > > * Destroy a pool previously created by odp_pool_create() > > @@ -161,6 +173,7 @@ typedef struct odp_pool_info_t { > > ODP_SHM_INVALID if this pool is > > managed by ODP */ > > odp_pool_param_t params; /**< pool parameters */ > > + odp_pool_init_t init_params; /**< pool initialization parameters > */ > > } odp_pool_info_t; > > > > /** > > -- > > 2.1.0 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 25 March 2015 at 01:53, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > > 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. > I approve of this RFC. Me want it now! > > include/odp/api/buffer.h | 11 +++++++++++ > include/odp/api/packet.h | 11 +++++++++++ include/odp/api/pool.h | 15 ++++++++++++++- > 3 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h > index 9ad08ea..98a66ee 100644 > --- a/include/odp/api/buffer.h > +++ b/include/odp/api/buffer.h > @@ -145,6 +145,17 @@ void odp_buffer_print(odp_buffer_t buf); > uint64_t odp_buffer_to_u64(odp_buffer_t hdl); > > /** > + * Get address and size of user metadata associated with a buffer > + * > + * @param buf Buffer handle > + * @param udata_size[out] Number of bytes of user metadata available > + * at the returned address > + * @return Address of the user metadata for this buffer > + * or NULL if the buffer has no user metadata. > + */ > +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); > + > +/** > * @} > */ > > diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h > index a31c54d..eb392e4 100644 > --- a/include/odp/api/packet.h > +++ b/include/odp/api/packet.h > @@ -193,6 +193,17 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); > void *odp_packet_data(odp_packet_t pkt); > > /** > + * Get address and size of user metadata associated with a packet > + * > + * @param pkt Packet handle > + * @param udata_size[out] Number of bytes of user metadata available > + * at the returned address > + * @return Address of the usermeta data for this buffer > + * or NULL if the buffer has no user metadata. > + */ > +void *odp_packet_udata(odp_packet_t pkt, uint32_t *udata_size); > + > +/** > * Packet segment data length > * > * Returns number of data bytes following the current data pointer > diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h > index 241b98a..57f026c 100644 > --- a/include/odp/api/pool.h > +++ b/include/odp/api/pool.h > @@ -85,6 +85,14 @@ typedef struct odp_pool_param_t { > int type; /**< Pool type */ > } odp_pool_param_t; > > +/** > + * Pool options > + * Used to communicate pool initialization options. > + */ > +typedef struct odp_pool_init_t { > + uint32_t udata_size; /**< Size of user metadata in > bytes */ > What alignment of the user metadata will be honored? If it is "suitable aligned for any kind of variable", then I am fine. > +} odp_pool_init_t; > + > /** Packet pool*/ > #define ODP_POOL_PACKET ODP_EVENT_PACKET > /** Buffer pool */ > @@ -109,13 +117,17 @@ typedef struct odp_pool_param_t { > * > * @param params Pool parameters. > * > + * @param init_params Pool initialization options. May be specified as > NULL > + * to indicate default options are to be used. > + * > * @return Handle of the created pool > * @retval ODP_POOL_INVALID Pool could not be created > */ > > odp_pool_t odp_pool_create(const char *name, > odp_shm_t shm, > - odp_pool_param_t *params); > + odp_pool_param_t *params, > + odp_pool_init_t *init_params); > > /** > * Destroy a pool previously created by odp_pool_create() > @@ -161,6 +173,7 @@ typedef struct odp_pool_info_t { > ODP_SHM_INVALID if this pool is > managed by ODP */ > odp_pool_param_t params; /**< pool parameters */ > + odp_pool_init_t init_params; /**< pool initialization parameters */ > } odp_pool_info_t; > > /** > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
I think we should specify that the user metadata area is 8-byte aligned. Do we need anything more? We could add an align option to the odp_init_param_t if needed, but I'm not sure if all platforms would be comfortable with that. Or else add an ODP_CONFIG_UDATA_ALIGN_MAX that they can specify? Seems overly complicated. On Wed, Mar 25, 2015 at 4:02 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 25 March 2015 at 01:53, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> >> 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. >> > > I approve of this RFC. Me want it now! > > >> >> include/odp/api/buffer.h | 11 +++++++++++ >> include/odp/api/packet.h | 11 +++++++++++ > > include/odp/api/pool.h | 15 ++++++++++++++- >> 3 files changed, 36 insertions(+), 1 deletion(-) >> >> diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h >> index 9ad08ea..98a66ee 100644 >> --- a/include/odp/api/buffer.h >> +++ b/include/odp/api/buffer.h >> @@ -145,6 +145,17 @@ void odp_buffer_print(odp_buffer_t buf); >> uint64_t odp_buffer_to_u64(odp_buffer_t hdl); >> >> /** >> + * Get address and size of user metadata associated with a buffer >> + * >> + * @param buf Buffer handle >> + * @param udata_size[out] Number of bytes of user metadata available >> + * at the returned address >> + * @return Address of the user metadata for this buffer >> + * or NULL if the buffer has no user metadata. >> + */ >> +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); >> + >> +/** >> * @} >> */ >> >> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h >> index a31c54d..eb392e4 100644 >> --- a/include/odp/api/packet.h >> +++ b/include/odp/api/packet.h >> @@ -193,6 +193,17 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); >> void *odp_packet_data(odp_packet_t pkt); >> >> /** >> + * Get address and size of user metadata associated with a packet >> + * >> + * @param pkt Packet handle >> + * @param udata_size[out] Number of bytes of user metadata available >> + * at the returned address >> + * @return Address of the usermeta data for this buffer >> + * or NULL if the buffer has no user metadata. >> + */ >> +void *odp_packet_udata(odp_packet_t pkt, uint32_t *udata_size); >> + >> +/** >> * Packet segment data length >> * >> * Returns number of data bytes following the current data pointer >> diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h >> index 241b98a..57f026c 100644 >> --- a/include/odp/api/pool.h >> +++ b/include/odp/api/pool.h >> @@ -85,6 +85,14 @@ typedef struct odp_pool_param_t { >> int type; /**< Pool type */ >> } odp_pool_param_t; >> >> +/** >> + * Pool options >> + * Used to communicate pool initialization options. >> + */ >> +typedef struct odp_pool_init_t { >> + uint32_t udata_size; /**< Size of user metadata in >> bytes */ >> > What alignment of the user metadata will be honored? If it is "suitable > aligned for any kind of variable", then I am fine. > > > >> +} odp_pool_init_t; >> + >> /** Packet pool*/ >> #define ODP_POOL_PACKET ODP_EVENT_PACKET >> /** Buffer pool */ >> @@ -109,13 +117,17 @@ typedef struct odp_pool_param_t { >> * >> * @param params Pool parameters. >> * >> + * @param init_params Pool initialization options. May be specified as >> NULL >> + * to indicate default options are to be used. >> + * >> * @return Handle of the created pool >> * @retval ODP_POOL_INVALID Pool could not be created >> */ >> >> odp_pool_t odp_pool_create(const char *name, >> odp_shm_t shm, >> - odp_pool_param_t *params); >> + odp_pool_param_t *params, >> + odp_pool_init_t *init_params); >> >> /** >> * Destroy a pool previously created by odp_pool_create() >> @@ -161,6 +173,7 @@ typedef struct odp_pool_info_t { >> ODP_SHM_INVALID if this pool is >> managed by ODP */ >> odp_pool_param_t params; /**< pool parameters */ >> + odp_pool_init_t init_params; /**< pool initialization parameters >> */ >> } odp_pool_info_t; >> >> /** >> -- >> 2.1.0 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >
On 03/25/15 03:53, Bill Fischofer wrote: > /** > + * Get address and size of user metadata associated with a buffer > + * > + * @param buf Buffer handle > + * @param udata_size[out] Number of bytes of user metadata available > + * at the returned address > + * @return Address of the user metadata for this buffer > + * or NULL if the buffer has no user metadata. > + */ > +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); > + > +/** > * @} > */ We should use the same style for all function. For example for mac we return size and set pointer. Negative size might be coded return code. It will be great to revisit all other functions and update odp coding style document for that sort of things. regards, Maxim.
On 25 March 2015 at 18:14, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/25/15 03:53, Bill Fischofer wrote: > >> /** >> + * Get address and size of user metadata associated with a buffer >> + * >> + * @param buf Buffer handle >> + * @param udata_size[out] Number of bytes of user metadata available >> + * at the returned address >> + * @return Address of the user metadata for this buffer >> + * or NULL if the buffer has no user metadata. >> + */ >> +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); >> + >> +/** >> * @} >> */ >> > We should use the same style for all function. There are different classes of functions. One class contains functions that returns a pointer to some data, e.g. odp_packet_head() and odp_packet_data(). odp_buffer_udata() seems similar. For example for mac we return size and set pointer. Which function is this? odp_pktio_mac_addr()? That one copies the MAC address, it does not return a pointer to some hidden buffer where the MAC address is stored. Different class of function so different prototype. Negative size might be coded return code. It will be great to revisit all > other functions and update odp coding style document for that sort of > things. > > regards, > Maxim. > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h index 9ad08ea..98a66ee 100644 --- a/include/odp/api/buffer.h +++ b/include/odp/api/buffer.h @@ -145,6 +145,17 @@ void odp_buffer_print(odp_buffer_t buf); uint64_t odp_buffer_to_u64(odp_buffer_t hdl); /** + * Get address and size of user metadata associated with a buffer + * + * @param buf Buffer handle + * @param udata_size[out] Number of bytes of user metadata available + * at the returned address + * @return Address of the user metadata for this buffer + * or NULL if the buffer has no user metadata. + */ +void *odp_buffer_udata(odp_buffer_t buf, uint32_t *udata_size); + +/** * @} */ diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h index a31c54d..eb392e4 100644 --- a/include/odp/api/packet.h +++ b/include/odp/api/packet.h @@ -193,6 +193,17 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt); void *odp_packet_data(odp_packet_t pkt); /** + * Get address and size of user metadata associated with a packet + * + * @param pkt Packet handle + * @param udata_size[out] Number of bytes of user metadata available + * at the returned address + * @return Address of the usermeta data for this buffer + * or NULL if the buffer has no user metadata. + */ +void *odp_packet_udata(odp_packet_t pkt, uint32_t *udata_size); + +/** * Packet segment data length * * Returns number of data bytes following the current data pointer diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h index 241b98a..57f026c 100644 --- a/include/odp/api/pool.h +++ b/include/odp/api/pool.h @@ -85,6 +85,14 @@ typedef struct odp_pool_param_t { int type; /**< Pool type */ } odp_pool_param_t; +/** + * Pool options + * Used to communicate pool initialization options. + */ +typedef struct odp_pool_init_t { + uint32_t udata_size; /**< Size of user metadata in bytes */ +} odp_pool_init_t; + /** Packet pool*/ #define ODP_POOL_PACKET ODP_EVENT_PACKET /** Buffer pool */ @@ -109,13 +117,17 @@ typedef struct odp_pool_param_t { * * @param params Pool parameters. * + * @param init_params Pool initialization options. May be specified as NULL + * to indicate default options are to be used. + * * @return Handle of the created pool * @retval ODP_POOL_INVALID Pool could not be created */ odp_pool_t odp_pool_create(const char *name, odp_shm_t shm, - odp_pool_param_t *params); + odp_pool_param_t *params, + odp_pool_init_t *init_params); /** * Destroy a pool previously created by odp_pool_create() @@ -161,6 +173,7 @@ typedef struct odp_pool_info_t { ODP_SHM_INVALID if this pool is managed by ODP */ odp_pool_param_t params; /**< pool parameters */ + odp_pool_init_t init_params; /**< pool initialization parameters */ } odp_pool_info_t; /**
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- 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. include/odp/api/buffer.h | 11 +++++++++++ include/odp/api/packet.h | 11 +++++++++++ include/odp/api/pool.h | 15 ++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-)