| Message ID | 1423798368-16597-3-git-send-email-petri.savolainen@linaro.org |
|---|---|
| State | Accepted |
| Commit | 1460be05c8da565893463cacf916b94bb10e29d2 |
| Headers | show |
On 13 February 2015 at 04:32, Petri Savolainen <petri.savolainen@linaro.org> wrote: > Renamed ODP_CONFIG_PACKET_BUF_LEN_MIN to ODP_CONFIG_PACKET_SEG_LEN_MIN. > The packet pool parameter pkt.seg_len is rounded up into this value. > > Added ODP_CONFIG_PACKET_SEG_LEN_MAX to define the maximum segment > length supported by the implementation. This is the maximum valid > value for pkt.seg_len. > > If ODP_CONFIG_PACKET_SEG_LEN_MIN equals _SEG_LEN_MAX, the > implementation supports only one segment length value. As I have voiced before, I think config.h is a weird beast. Is it an output from the ODP implementation? Is it an input to the ODP implementation should it be recompiled? Is it both? Which config items can be changed by the user? Why are there references to linux-generic in the current config.h? When all of these questions arise, something is fishy. Let's look ahead. ODP is installed as a development library (e.g. deb package libodp-dev), possibly with a well-defined ABI. Applications might be dynamically linked to some target-specific ODP implementation. How does the application get to know about internal limitations in the actual ODP implementation (e.g. max number of shared memory segments or maximum packet size)? By calling an API at run-time which returns the requested value. What happens if the application requests something (e.g. too large segment or packet size) which cannot be supported by the implementation? The relevant API (e.g. create_pool) returns a well-defined error. No need for elaborate computations in the application in order to satisfy implementation specific limitations. -- Ola > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > include/odp/api/config.h | 20 +++++++++++++++----- > platform/linux-generic/include/odp_buffer_internal.h | 12 ++++++------ > platform/linux-generic/odp_buffer_pool.c | 4 ++-- > test/validation/buffer/odp_packet_test.c | 2 +- > 4 files changed, 24 insertions(+), 14 deletions(-) > > diff --git a/include/odp/api/config.h b/include/odp/api/config.h > index bd69b98..3ac9e2c 100644 > --- a/include/odp/api/config.h > +++ b/include/odp/api/config.h > @@ -92,9 +92,9 @@ extern "C" { > /** > * Minimum packet segment length > * > - * This defines the minimum packet segment length in bytes. The user defined > - * buffer size (in odp_buffer_pool_param_t) in buffer pool creation will be > - * rounded up into this value. > + * This defines the minimum packet segment buffer length in bytes. The user > + * defined segment length (seg_len in odp_pool_param_t) will be rounded up into > + * this value. > * > * @internal In linux-generic implementation: > * - The value MUST be a multiple of 8. > @@ -103,7 +103,17 @@ extern "C" { > * with the default headroom shown above and is a multiple of both 64 and 128, > * which are the most common cache line sizes. > */ > -#define ODP_CONFIG_PACKET_BUF_LEN_MIN (1664) > +#define ODP_CONFIG_PACKET_SEG_LEN_MIN (1664) > + > +/** > + * Maximum packet segment length > + * > + * This defines the maximum packet segment buffer length in bytes. The user > + * defined segment length (seg_len in odp_pool_param_t) must not be larger than > + * this. > + * > + */ > +#define ODP_CONFIG_PACKET_SEG_LEN_MAX ODP_CONFIG_PACKET_SEG_LEN_MIN > > /** > * Maximum packet buffer length > @@ -117,7 +127,7 @@ 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_BUF_LEN_MIN*6) > +#define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) > > /** > * @} > diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h > index f199e2e..c532e35 100644 > --- a/platform/linux-generic/include/odp_buffer_internal.h > +++ b/platform/linux-generic/include/odp_buffer_internal.h > @@ -50,18 +50,18 @@ extern "C" { > ((x) <= 65536 ? 16 : \ > (0/0))))))))))))))))) > > -_ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_BUF_LEN_MIN >= 256, > - "ODP Segment size must be a minimum of 256 bytes"); > +_ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256, > + "ODP Segment size must be a minimum of 256 bytes"); > > -_ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MIN % ODP_CACHE_LINE_SIZE) == 0, > - "ODP Segment size must be a multiple of cache line size"); > +_ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_SEG_LEN_MIN % ODP_CACHE_LINE_SIZE) == 0, > + "ODP Segment size must be a multiple of cache line size"); > > _ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MAX % > - ODP_CONFIG_PACKET_BUF_LEN_MIN) == 0, > + 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_BUF_LEN_MIN) > + (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)) > diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c > index 18ad358..f8e35b4 100644 > --- a/platform/linux-generic/odp_buffer_pool.c > +++ b/platform/linux-generic/odp_buffer_pool.c > @@ -182,7 +182,7 @@ odp_pool_t odp_pool_create(const char *name, > else > blk_size = ODP_ALIGN_ROUNDUP( > headroom + params->pkt.seg_len + tailroom, > - ODP_CONFIG_PACKET_BUF_LEN_MIN); > + ODP_CONFIG_PACKET_SEG_LEN_MIN); > > buf_stride = params->type == ODP_POOL_PACKET ? > sizeof(odp_packet_hdr_stride) : > @@ -284,7 +284,7 @@ odp_pool_t odp_pool_create(const char *name, > pool->s.flags.unsegmented = unseg; > pool->s.flags.zeroized = zeroized; > pool->s.seg_size = unseg ? > - blk_size : ODP_CONFIG_PACKET_BUF_LEN_MIN; > + blk_size : ODP_CONFIG_PACKET_SEG_LEN_MIN; > > > uint8_t *block_base_addr = pool->s.pool_base_addr; > diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c > index f5830cc..2b87be9 100644 > --- a/test/validation/buffer/odp_packet_test.c > +++ b/test/validation/buffer/odp_packet_test.c > @@ -7,7 +7,7 @@ > #include "odp_buffer_tests.h" > #include <stdlib.h> > > -#define PACKET_BUF_LEN ODP_CONFIG_PACKET_BUF_LEN_MIN > +#define PACKET_BUF_LEN ODP_CONFIG_PACKET_SEG_LEN_MIN > /* Reserve some tailroom for tests */ > #define PACKET_TAILROOM_RESERVE 4 > > -- > 2.3.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 02/16/2015 11:28 AM, Ola Liljedahl wrote: > On 13 February 2015 at 04:32, Petri Savolainen > <petri.savolainen@linaro.org> wrote: >> Renamed ODP_CONFIG_PACKET_BUF_LEN_MIN to ODP_CONFIG_PACKET_SEG_LEN_MIN. >> The packet pool parameter pkt.seg_len is rounded up into this value. >> >> Added ODP_CONFIG_PACKET_SEG_LEN_MAX to define the maximum segment >> length supported by the implementation. This is the maximum valid >> value for pkt.seg_len. >> >> If ODP_CONFIG_PACKET_SEG_LEN_MIN equals _SEG_LEN_MAX, the >> implementation supports only one segment length value. > As I have voiced before, I think config.h is a weird beast. Is it an > output from the ODP implementation? Is it an input to the ODP > implementation should it be recompiled? Is it both? Which config items > can be changed by the user? Why are there references to linux-generic > in the current config.h? > > When all of these questions arise, something is fishy. > > Let's look ahead. ODP is installed as a development library (e.g. deb > package libodp-dev), possibly with a well-defined ABI. Applications > might be dynamically linked to some target-specific ODP > implementation. How does the application get to know about internal > limitations in the actual ODP implementation (e.g. max number of > shared memory segments or maximum packet size)? By calling an API at > run-time which returns the requested value. What happens if the > application requests something (e.g. too large segment or packet size) > which cannot be supported by the implementation? The relevant API > (e.g. create_pool) returns a well-defined error. > > No need for elaborate computations in the application in order to > satisfy implementation specific limitations. > > -- Ola Historically, ODP dev came with some static variables defined for number of pools, shm and etc. It will be good to switch that to dynamic allocation. But to do it right now I think it will require big efforts for rewriting and testing. That can be good subject for post 1.0 work. Best regards, Maxim. > >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> >> --- >> include/odp/api/config.h | 20 +++++++++++++++----- >> platform/linux-generic/include/odp_buffer_internal.h | 12 ++++++------ >> platform/linux-generic/odp_buffer_pool.c | 4 ++-- >> test/validation/buffer/odp_packet_test.c | 2 +- >> 4 files changed, 24 insertions(+), 14 deletions(-) >> >> diff --git a/include/odp/api/config.h b/include/odp/api/config.h >> index bd69b98..3ac9e2c 100644 >> --- a/include/odp/api/config.h >> +++ b/include/odp/api/config.h >> @@ -92,9 +92,9 @@ extern "C" { >> /** >> * Minimum packet segment length >> * >> - * This defines the minimum packet segment length in bytes. The user defined >> - * buffer size (in odp_buffer_pool_param_t) in buffer pool creation will be >> - * rounded up into this value. >> + * This defines the minimum packet segment buffer length in bytes. The user >> + * defined segment length (seg_len in odp_pool_param_t) will be rounded up into >> + * this value. >> * >> * @internal In linux-generic implementation: >> * - The value MUST be a multiple of 8. >> @@ -103,7 +103,17 @@ extern "C" { >> * with the default headroom shown above and is a multiple of both 64 and 128, >> * which are the most common cache line sizes. >> */ >> -#define ODP_CONFIG_PACKET_BUF_LEN_MIN (1664) >> +#define ODP_CONFIG_PACKET_SEG_LEN_MIN (1664) >> + >> +/** >> + * Maximum packet segment length >> + * >> + * This defines the maximum packet segment buffer length in bytes. The user >> + * defined segment length (seg_len in odp_pool_param_t) must not be larger than >> + * this. >> + * >> + */ >> +#define ODP_CONFIG_PACKET_SEG_LEN_MAX ODP_CONFIG_PACKET_SEG_LEN_MIN >> >> /** >> * Maximum packet buffer length >> @@ -117,7 +127,7 @@ 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_BUF_LEN_MIN*6) >> +#define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) >> >> /** >> * @} >> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h >> index f199e2e..c532e35 100644 >> --- a/platform/linux-generic/include/odp_buffer_internal.h >> +++ b/platform/linux-generic/include/odp_buffer_internal.h >> @@ -50,18 +50,18 @@ extern "C" { >> ((x) <= 65536 ? 16 : \ >> (0/0))))))))))))))))) >> >> -_ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_BUF_LEN_MIN >= 256, >> - "ODP Segment size must be a minimum of 256 bytes"); >> +_ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256, >> + "ODP Segment size must be a minimum of 256 bytes"); >> >> -_ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MIN % ODP_CACHE_LINE_SIZE) == 0, >> - "ODP Segment size must be a multiple of cache line size"); >> +_ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_SEG_LEN_MIN % ODP_CACHE_LINE_SIZE) == 0, >> + "ODP Segment size must be a multiple of cache line size"); >> >> _ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MAX % >> - ODP_CONFIG_PACKET_BUF_LEN_MIN) == 0, >> + 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_BUF_LEN_MIN) >> + (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)) >> diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c >> index 18ad358..f8e35b4 100644 >> --- a/platform/linux-generic/odp_buffer_pool.c >> +++ b/platform/linux-generic/odp_buffer_pool.c >> @@ -182,7 +182,7 @@ odp_pool_t odp_pool_create(const char *name, >> else >> blk_size = ODP_ALIGN_ROUNDUP( >> headroom + params->pkt.seg_len + tailroom, >> - ODP_CONFIG_PACKET_BUF_LEN_MIN); >> + ODP_CONFIG_PACKET_SEG_LEN_MIN); >> >> buf_stride = params->type == ODP_POOL_PACKET ? >> sizeof(odp_packet_hdr_stride) : >> @@ -284,7 +284,7 @@ odp_pool_t odp_pool_create(const char *name, >> pool->s.flags.unsegmented = unseg; >> pool->s.flags.zeroized = zeroized; >> pool->s.seg_size = unseg ? >> - blk_size : ODP_CONFIG_PACKET_BUF_LEN_MIN; >> + blk_size : ODP_CONFIG_PACKET_SEG_LEN_MIN; >> >> >> uint8_t *block_base_addr = pool->s.pool_base_addr; >> diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c >> index f5830cc..2b87be9 100644 >> --- a/test/validation/buffer/odp_packet_test.c >> +++ b/test/validation/buffer/odp_packet_test.c >> @@ -7,7 +7,7 @@ >> #include "odp_buffer_tests.h" >> #include <stdlib.h> >> >> -#define PACKET_BUF_LEN ODP_CONFIG_PACKET_BUF_LEN_MIN >> +#define PACKET_BUF_LEN ODP_CONFIG_PACKET_SEG_LEN_MIN >> /* Reserve some tailroom for tests */ >> #define PACKET_TAILROOM_RESERVE 4 >> >> -- >> 2.3.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
diff --git a/include/odp/api/config.h b/include/odp/api/config.h index bd69b98..3ac9e2c 100644 --- a/include/odp/api/config.h +++ b/include/odp/api/config.h @@ -92,9 +92,9 @@ extern "C" { /** * Minimum packet segment length * - * This defines the minimum packet segment length in bytes. The user defined - * buffer size (in odp_buffer_pool_param_t) in buffer pool creation will be - * rounded up into this value. + * This defines the minimum packet segment buffer length in bytes. The user + * defined segment length (seg_len in odp_pool_param_t) will be rounded up into + * this value. * * @internal In linux-generic implementation: * - The value MUST be a multiple of 8. @@ -103,7 +103,17 @@ extern "C" { * with the default headroom shown above and is a multiple of both 64 and 128, * which are the most common cache line sizes. */ -#define ODP_CONFIG_PACKET_BUF_LEN_MIN (1664) +#define ODP_CONFIG_PACKET_SEG_LEN_MIN (1664) + +/** + * Maximum packet segment length + * + * This defines the maximum packet segment buffer length in bytes. The user + * defined segment length (seg_len in odp_pool_param_t) must not be larger than + * this. + * + */ +#define ODP_CONFIG_PACKET_SEG_LEN_MAX ODP_CONFIG_PACKET_SEG_LEN_MIN /** * Maximum packet buffer length @@ -117,7 +127,7 @@ 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_BUF_LEN_MIN*6) +#define ODP_CONFIG_PACKET_BUF_LEN_MAX (ODP_CONFIG_PACKET_SEG_LEN_MIN*6) /** * @} diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h index f199e2e..c532e35 100644 --- a/platform/linux-generic/include/odp_buffer_internal.h +++ b/platform/linux-generic/include/odp_buffer_internal.h @@ -50,18 +50,18 @@ extern "C" { ((x) <= 65536 ? 16 : \ (0/0))))))))))))))))) -_ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_BUF_LEN_MIN >= 256, - "ODP Segment size must be a minimum of 256 bytes"); +_ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256, + "ODP Segment size must be a minimum of 256 bytes"); -_ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MIN % ODP_CACHE_LINE_SIZE) == 0, - "ODP Segment size must be a multiple of cache line size"); +_ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_SEG_LEN_MIN % ODP_CACHE_LINE_SIZE) == 0, + "ODP Segment size must be a multiple of cache line size"); _ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MAX % - ODP_CONFIG_PACKET_BUF_LEN_MIN) == 0, + 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_BUF_LEN_MIN) + (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)) diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c index 18ad358..f8e35b4 100644 --- a/platform/linux-generic/odp_buffer_pool.c +++ b/platform/linux-generic/odp_buffer_pool.c @@ -182,7 +182,7 @@ odp_pool_t odp_pool_create(const char *name, else blk_size = ODP_ALIGN_ROUNDUP( headroom + params->pkt.seg_len + tailroom, - ODP_CONFIG_PACKET_BUF_LEN_MIN); + ODP_CONFIG_PACKET_SEG_LEN_MIN); buf_stride = params->type == ODP_POOL_PACKET ? sizeof(odp_packet_hdr_stride) : @@ -284,7 +284,7 @@ odp_pool_t odp_pool_create(const char *name, pool->s.flags.unsegmented = unseg; pool->s.flags.zeroized = zeroized; pool->s.seg_size = unseg ? - blk_size : ODP_CONFIG_PACKET_BUF_LEN_MIN; + blk_size : ODP_CONFIG_PACKET_SEG_LEN_MIN; uint8_t *block_base_addr = pool->s.pool_base_addr; diff --git a/test/validation/buffer/odp_packet_test.c b/test/validation/buffer/odp_packet_test.c index f5830cc..2b87be9 100644 --- a/test/validation/buffer/odp_packet_test.c +++ b/test/validation/buffer/odp_packet_test.c @@ -7,7 +7,7 @@ #include "odp_buffer_tests.h" #include <stdlib.h> -#define PACKET_BUF_LEN ODP_CONFIG_PACKET_BUF_LEN_MIN +#define PACKET_BUF_LEN ODP_CONFIG_PACKET_SEG_LEN_MIN /* Reserve some tailroom for tests */ #define PACKET_TAILROOM_RESERVE 4
Renamed ODP_CONFIG_PACKET_BUF_LEN_MIN to ODP_CONFIG_PACKET_SEG_LEN_MIN. The packet pool parameter pkt.seg_len is rounded up into this value. Added ODP_CONFIG_PACKET_SEG_LEN_MAX to define the maximum segment length supported by the implementation. This is the maximum valid value for pkt.seg_len. If ODP_CONFIG_PACKET_SEG_LEN_MIN equals _SEG_LEN_MAX, the implementation supports only one segment length value. Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> --- include/odp/api/config.h | 20 +++++++++++++++----- platform/linux-generic/include/odp_buffer_internal.h | 12 ++++++------ platform/linux-generic/odp_buffer_pool.c | 4 ++-- test/validation/buffer/odp_packet_test.c | 2 +- 4 files changed, 24 insertions(+), 14 deletions(-)