include/odp/api/config.h is really improperly used right now. What it should be as the definition of odp_config_xxx() API calls that allow applications to retrieve implementation-specific configuration limits. So I wouldn't make any changes to it at this point until we can move from static #defines to dynamic APIs.
On Mon, Jul 20, 2015 at 12:24 PM, Bala Manoharan <[email protected]> wrote: > Hi, > > Few comments inline > > On 20 July 2015 at 22:38, Nicolas Morey-Chaisemartin <[email protected]> > wrote: > >> Replace current segmentation with an explicit define. >> This mainly means two things: >> - All code can now test and check the max segmentation which will prove >> useful for tests and open the way for many code optimizations. >> - The minimum segment length and the maximum buffer len can now be >> decorrelated. >> This means that pool with very small footprints can be allocated for >> small packets, >> while pool for jumbo frame will still work as long as seg_len * >> ODP_CONFIG_PACKET_MAX_SEG >= packet_len >> >> Signed-off-by: Nicolas Morey-Chaisemartin <[email protected]> >> --- >> include/odp/api/config.h | 10 +++++++++- >> platform/linux-generic/include/odp_buffer_internal.h | 9 +++------ >> platform/linux-generic/odp_pool.c | 4 ++-- >> test/validation/packet/packet.c | 3 ++- >> 4 files changed, 16 insertions(+), 10 deletions(-) >> >> diff --git a/include/odp/api/config.h b/include/odp/api/config.h >> index b5c8fdd..1f44db6 100644 >> --- a/include/odp/api/config.h >> +++ b/include/odp/api/config.h >> @@ -108,6 +108,13 @@ extern "C" { >> #define ODP_CONFIG_PACKET_SEG_LEN_MAX (64*1024) >> >> /** >> + * Maximum number of segments in a packet >> + * >> + * This defines the maximum number of segment buffers in a packet >> + */ >> +#define ODP_CONFIG_PACKET_MAX_SEG 6 >> > > What is the use-case of the above define? Does it mean that the packet > should not be stored in a pool if the max number of segment gets reached? > If this is something used in the linux-generic we can define it in the > internal header file. > > The reason is that the #defines in config.h file has to be defined by all > the platforms. > > Regards, > Bala > > + >> +/** >> * Maximum packet buffer length >> * >> * This defines the maximum number of bytes that can be stored into a >> packet >> @@ -119,7 +126,8 @@ extern "C" { >> * - The value MUST be an integral number of segments >> * - The value SHOULD be large enough to accommodate jumbo packets (9K) >> */ >> -#define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) >> +#define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN * \ >> + ODP_CONFIG_PACKET_MAX_SEG) >> >> /** Maximum number of shared memory blocks. >> * >> diff --git a/platform/linux-generic/include/odp_buffer_internal.h >> b/platform/linux-generic/include/odp_buffer_internal.h >> index ae799dd..5d1199a 100644 >> --- a/platform/linux-generic/include/odp_buffer_internal.h >> +++ b/platform/linux-generic/include/odp_buffer_internal.h >> @@ -57,14 +57,11 @@ _ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MAX % >> ODP_CONFIG_PACKET_SEG_LEN_MIN) == 0, >> "Packet max size must be a multiple of segment size"); >> >> -#define ODP_BUFFER_MAX_SEG \ >> - (ODP_CONFIG_PACKET_BUF_LEN_MAX / ODP_CONFIG_PACKET_SEG_LEN_MIN) >> - >> /* We can optimize storage of small raw buffers within metadata area */ >> -#define ODP_MAX_INLINE_BUF ((sizeof(void *)) * (ODP_BUFFER_MAX_SEG - >> 1)) >> +#define ODP_MAX_INLINE_BUF ((sizeof(void *)) * >> (ODP_CONFIG_PACKET_MAX_SEG - 1)) >> >> #define ODP_BUFFER_POOL_BITS ODP_BITSIZE(ODP_CONFIG_POOLS) >> -#define ODP_BUFFER_SEG_BITS ODP_BITSIZE(ODP_BUFFER_MAX_SEG) >> +#define ODP_BUFFER_SEG_BITS ODP_BITSIZE(ODP_CONFIG_PACKET_MAX_SEG) >> #define ODP_BUFFER_INDEX_BITS (32 - ODP_BUFFER_POOL_BITS - >> ODP_BUFFER_SEG_BITS) >> #define ODP_BUFFER_PREFIX_BITS (ODP_BUFFER_POOL_BITS + >> ODP_BUFFER_INDEX_BITS) >> #define ODP_BUFFER_MAX_POOLS (1 << ODP_BUFFER_POOL_BITS) >> @@ -130,7 +127,7 @@ typedef struct odp_buffer_hdr_t { >> uint32_t uarea_size; /* size of user area */ >> uint32_t segcount; /* segment count */ >> uint32_t segsize; /* segment size */ >> - void *addr[ODP_BUFFER_MAX_SEG]; /* block addrs >> */ >> + void *addr[ODP_CONFIG_PACKET_MAX_SEG]; /* >> block addrs */ >> } odp_buffer_hdr_t; >> >> /** @internal Compile time assert that the >> diff --git a/platform/linux-generic/odp_pool.c >> b/platform/linux-generic/odp_pool.c >> index 0b8921c..ff2a89c 100644 >> --- a/platform/linux-generic/odp_pool.c >> +++ b/platform/linux-generic/odp_pool.c >> @@ -217,7 +217,7 @@ odp_pool_t odp_pool_create(const char *name, >> odp_pool_param_t *params) >> ODP_ALIGN_ROUNDUP(params->pkt.len, seg_len); >> >> /* Reject create if pkt.len needs too many segments */ >> - if (blk_size / seg_len > ODP_BUFFER_MAX_SEG) >> + if (blk_size / seg_len > ODP_CONFIG_PACKET_MAX_SEG) >> return ODP_POOL_INVALID; >> >> p_udata_size = params->pkt.uarea_size; >> @@ -481,7 +481,7 @@ odp_buffer_t buffer_alloc(odp_pool_t pool_hdl, size_t >> size) >> /* Reject oversized allocation requests */ >> if ((pool->s.flags.unsegmented && totsize > pool->s.seg_size) || >> (!pool->s.flags.unsegmented && >> - totsize > pool->s.seg_size * ODP_BUFFER_MAX_SEG)) >> + totsize > pool->s.seg_size * ODP_CONFIG_PACKET_MAX_SEG)) >> return ODP_BUFFER_INVALID; >> >> /* Try to satisfy request from the local cache */ >> diff --git a/test/validation/packet/packet.c >> b/test/validation/packet/packet.c >> index e6fb18a..05f0532 100644 >> --- a/test/validation/packet/packet.c >> +++ b/test/validation/packet/packet.c >> @@ -23,7 +23,8 @@ static const uint32_t packet_len = PACKET_BUF_LEN - >> ODP_CONFIG_PACKET_TAILROOM - >> PACKET_TAILROOM_RESERVE; >> >> -static const uint32_t segmented_packet_len = PACKET_BUF_LEN * 5 - >> +static const uint32_t segmented_packet_len = >> + PACKET_BUF_LEN * ODP_CONFIG_PACKET_MAX_SEG - >> ODP_CONFIG_PACKET_HEADROOM - ODP_CONFIG_PACKET_TAILROOM - >> PACKET_TAILROOM_RESERVE; >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > > > _______________________________________________ > lng-odp mailing list > [email protected] > https://lists.linaro.org/mailman/listinfo/lng-odp > >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
