Why do we need function calls for that? Defines for these kind of values can be quite useful for compile time optimization/configuration (ie static array size)
Nicolas On 07/21/2015 12:58 AM, Bill Fischofer wrote: > 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] > <mailto:[email protected]>> wrote: > > Hi, > > Few comments inline > > On 20 July 2015 at 22:38, Nicolas Morey-Chaisemartin <[email protected] > <mailto:[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] > <mailto:[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] <mailto:[email protected]> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > [email protected] <mailto:[email protected]> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
