From: ext Bill Fischofer [mailto:[email protected]] Sent: Tuesday, December 23, 2014 2:58 PM To: Petri Savolainen Cc: lng-odp-forward Subject: Re: [lng-odp] [PATCH v2 1/2] api: buffer_pool: Correct buf_size pool param documentation
On Tue, Dec 23, 2014 at 5:29 AM, Petri Savolainen <[email protected]<mailto:[email protected]>> wrote: Buf_size parameter defines minimum buffer/segment length. Use 0 for default length. Signed-off-by: Petri Savolainen <[email protected]<mailto:[email protected]>> --- .../linux-generic/include/api/odp_buffer_pool.h | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h b/platform/linux-generic/include/api/odp_buffer_pool.h index 8380ac1..9329405 100644 --- a/platform/linux-generic/include/api/odp_buffer_pool.h +++ b/platform/linux-generic/include/api/odp_buffer_pool.h @@ -35,19 +35,25 @@ extern "C" { /** * Buffer pool parameters * Used to communicate buffer pool creation options. + * + * @see ODP_CONFIG_PACKET_BUF_LEN_MIN, ODP_CONFIG_BUFFER_ALIGN_MIN, + * ODP_CONFIG_BUFFER_ALIGN_MAX */ typedef struct odp_buffer_pool_param_t { - uint32_t buf_size; /**< Buffer size in bytes. The maximum - number of bytes application will - store in each buffer. For packets, this - is the maximum packet data length, and - configured headroom and tailroom will be - added to this number */ - uint32_t buf_align; /**< Minimum buffer alignment in bytes. - Valid values are powers of two. Use 0 - for default alignment. Default will - always be a multiple of 8. */ - uint32_t num_bufs; /**< Number of buffers in the pool */ + uint32_t buf_size; /**< Minimum buffer size in bytes. For packets, + this is the minimum segment buffer length, + which includes possible head-/tailroom bytes. + Use 0 for the default size of the buffer type + (e.g. for timeouts or min packet segment + length).*/ I continue to have difficulty with understanding how the implementation is expected to use this interpretation of buf_size to calculate how much storage should be reserved for the buffer pool. The implementation needs to know this and under the former definition that was straightforward since the application was specifying how large each buffer may be. Minimums do not do this. Application specifies the minimum size (e.g. 100 bytes). Implementation can round it up to a sensible value (e.g. 128 bytes). odp_buffer_size() and odp_packet_seg_buf_len() return the actual buffer length (e.g. 128 bytes). Pool size is num_bufs * (buf_size + round up + internal headers, per buffer overheads, etc). + uint32_t buf_align; /**< Minimum buffer alignment in bytes. Valid values + are powers of two. Use 0 for default + alignment. Default will always be a multiple + of 8. */ + uint32_t num_bufs; /**< Number of buffers in the pool. For packets, + this is the total number of segments and the + maximum number of packets (in case that all + packets have a single segment). */ This is not what the implementation needs to know. A buffer is not a segment since a buffer is what anchors the metadata associated with the object. Whether or not we introduce packet segment metadata, packets (single object) have metadata associated with them that are independent of the number of segments they occupy. Conflating these concepts doesn't simplify anything and just confuses the terminology and APIs. Implementation needs to know this. As it says above, in the worst case all packets in the pool have only single segment and thus the max number of packet metadata objects is ‘num_bufs’. Number of segments per packet is not a constant, it depends on received packet lengths (and fluctuates with those) It’s an implementation detail if buffer/packet metadata is stored separately from actual buffer space (or next to it). Typically it’s stored next to the buffer space so that the first cache line of the packet covers both the metadata and the first N bytes of packet data/headroom. When packet consists of multiple segments, the first segment holds per packet meta-data, other segments hold just pointers to the data and next segments. The previous linux-generic implementation was designed like that. -Petri int buf_type; /**< Buffer type */ } odp_buffer_pool_param_t; -- 2.2.1 _______________________________________________ lng-odp mailing list [email protected]<mailto:[email protected]> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
