> -----Original Message-----
> From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org]
> Sent: Tuesday, February 03, 2015 11:05 PM
> To: Petri Savolainen
> Cc: LNG ODP Mailman List
> Subject: Re: [lng-odp] [PATCH v2 1/3] api: pool: Added packet pool
> parameters
>
> My alternative approach that should achieve that same goals as Petri
> but give more freedom to implementations. You don't have to approve of
> it, I just want to show that given a defined and understood problem,
> many potential solutions exist and the first alternative might not be
> the best. Let's work together.
I think your proposal is not substantially different what I have suggested
already (including conf calls). Main difference is that 1st segment vs other
segments configuration would be introduced already in v1.0. I was thinking to
nail v1.0 with minimum support (keep it close to what we have already done) and
extend from there (with concept of 1st/other segments).
>
> * init_seg_len:
> On input: user's required (desired) minimal length of initial
> segment (including headroom)
> On output: implementation's best effort to match user's request
> Purpose: ensure that those packet headers the application
> normally will process are stored in a consecutive memory area.
> Applications do not have to check any configuration in order
> to initialize a configuration which the implementation anyway has to
> check if it can support.
> Applications should check output values to see if its desired
> values were matched. The application decides whether a failure to
> match is a fatal error or the application can handle the situation
> anyway (e.g. with degraded performance because it has to do some
> workarounds in SW).
This would be .first_seg_len. Application is not interested on the actual len,
either implementation can support the minimum requirement or not. That's why
output param is not necessary, but min/max limits are needed. Limits can be
provided as macros or function calls.
I'd postpone splitting .seg_len from current proposal to .first_seg_len and
.other_seg_len some time after v1.0 when we continue packet API segmentation
API development anyway.
>
> * seg_len:
> On input: user's desired length of other segments
> On output: implementation's best effort to match user's request
> Purpose: a hint from the user how to partition to pool into
> segments for best trade-off between memory utilization and SW
> processing performance.
> Note: I know some HW can support multiple segment sizes so I
> think it is useful for the API to allow for this. Targets which only
> support one segment size (per packet pool) could use e.g.
> max(int_seg_len, seg_len). Some targets may not allow user-defined
> segment sizes at all, the ODP implementation will just return the
> actual values and the application can check whether those are
> acceptable.
This would be .other_seg_len. This can be hint vs 1st seg len is strict
requirement.
>
> * init_seg_num:
> On input: Number of initial segments.
> On output: Updated with actual number of segments if a shared
> memory region was specified?
This is also max number of packets (== seg_num in my proposal). Again I
considered to postpone the split of first_seg_num/packet_num and other_seg_num
after v1.0.
Application may want (usually does) to specify max number of packet as hard
limit, since that's the limit for maximum number of in-flight packet inside
your app / SoC. The limit is needed for QoS, per packet context memory
allocation, etc.
It depends on application if actual number is interesting or only
success/failure.
> * seg_num:
> On input: Number of other segments.
> On output: Updated with actual number of segments if a shared
> memory region was specified?
>
> I dislike the defines because they will make a future portable ABI
> (binary interface) definition impossible. We will need a portable ABI
> to support e.g. shared library implementations. So all ODP_CONFIG's
> should only be used internally (by the implementation in question) and
> not be accessible as compile time constants to applications, i.e. they
> should not be part of the ODP API. Are there any other instances where
> the application is supposed to use these constants?
This a new requirement, and is probably a valid one also. Obviously, any config
interface development need to be postponed after v1.0.
Since v1.0 has ODP_CONFIG_XXX (config.h) in the API, it's possible to use that
mechanism also for these limits. Functions could be used also, but it would not
solve the config issue as have used ODP_CONFIG_XXX already for other things
like buffers. Minimizing changes to current practices to reach v1.0 sooner than
later...
-Petri
>
> -- Ola
>
> On 3 February 2015 at 14:31, Ola Liljedahl <ola.liljed...@linaro.org>
> wrote:
> > On 3 February 2015 at 13:59, Petri Savolainen
> > <petri.savolai...@linaro.org> wrote:
> >> Completed odp_pool_param_t definition with packet pool parameters.
> >> Parameter definition is close to what we are using already.
> >>
> >> * seg_len: Defines minimum segment buffer length.
> >> With this parameter user can:
> >> * trade-off between pool memory usage and SW performance
> (linear memory access)
> >> * avoid segmentation in packet head (e.g. if legacy app
> cannot handle
> >> segmentation in the middle of the packet headers)
> > We already had defined a minimum segment size for conforming ODP
> > implementations. Isn't that enough?
> >
> > I can see value in specifying the minimum size of the first segment of
> > a packet (which would contain all headers the application is going to
> > process). But this proposal goes much further than that.
> >
> >
> >> * seg_len < ODP_CONFIG_PACKET_SEG_LEN_MIN is rounded up to
> ODP_CONFIG_PACKET_SEG_LEN_MIN
> >> * seg_len > ODP_CONFIG_PACKET_SEG_LEN_MAX is not valid
> >>
> >> * seg_align: Defines minimum segment buffer alignment. With this
> parameter,
> >> user can force buffer alignment to match e.g. aligment
> requirements
> >> of data structures stored in or algorithms accessing the
> packet
> > Can you give a practical example of when this configuration is useful?
> > To my knowledge, most data structures have quite small alignment
> > requirements, e.g. based on alignment requirements of individual
> > fields. But here I assume that we would specify alignment in multiples
> > of cache lines here (because the minimum segment alignment would be
> > the cache line size).
> >
> >> headroom. When user don't have specific alignment
> requirement 0
> >> should be used for default.
> >>
> >> * seg_num: Number of segments. This is also the maximum number of
> packets.
> > I think these configurations could be hints but not strict
> > requirements. They do not change the *functionality* so an application
> > should not fail if these configurations can not be obeyed (except for
> > that legacy situation you describe above). The hints enable more
> > optimal utilization of e.g. packet memory and may decrease SW overhead
> > during packet processing but do not change the functionality.
> >
> > To enable different hardware implementations, ODP apps should not
> > enforce unnecessary (non-functional) requirements on the ODP
> > implementations and limit the number of targets ODP can be implemented
> > on. ODP is not DPDK.
> >
> > Applications should also not have to first check the limits of the
> > specific ODP implementation (as you suggested yesterday), adapts its
> > configuration to that and then send back those requirements to the ODP
> > implementation (which still has to check the parameters to verify that
> > they are valid). This is too complicated and will likely lead to code
> > that cheats and thus is not portable. Better for applications just to
> > specify its requested configuration to ODP and then get back the
> > results (i.e. actual values that will be used). The application can
> > then if necessary check that the configuration was honored. This
> > follows the normal programming flow.
> >
> >>
> >> Signed-off-by: Petri Savolainen <petri.savolai...@linaro.org>
> >> ---
> >> include/odp/api/pool.h | 26 +++++++++++++++++++++-----
> >> 1 file changed, 21 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h
> >> index d09d92e..a1d7494 100644
> >> --- a/include/odp/api/pool.h
> >> +++ b/include/odp/api/pool.h
> >> @@ -61,13 +61,29 @@ typedef struct odp_pool_param_t {
> >> of 8. */
> >> uint32_t num; /**< Number of buffers in the
> pool */
> >> } buf;
> >> -/* Reserved for packet and timeout specific params
> >> struct {
> >> - uint32_t seg_size;
> >> - uint32_t seg_align;
> >> - uint32_t num;
> >> + uint32_t seg_len; /**< Minimum packet segment
> buffer
> >> + length in bytes. It
> includes
> >> + possible head-
> /tailroom bytes.
> >> + The maximum value is
> defined by
> >> +
> ODP_CONFIG_PACKET_SEG_LEN_MAX.
> >> + Use 0 for default
> length. */
> >> + uint32_t seg_align; /**< Minimum packet segment
> buffer
> >> + alignment in bytes.
> Valid
> >> + values are powers of
> two. The
> >> + maximum value is
> defined by
> >> +
> ODP_CONFIG_PACKET_SEG_ALIGN_MAX
> >> + . Use 0 for default
> alignment.
> >> + Default will always be
> a
> >> + multiple of 8.
> >> + */
> >> + uint32_t seg_num; /**< Number of packet
> segments in
> >> + the pool. This is also
> the
> >> + maximum number of
> packets,
> >> + since each packet
> consist of
> >> + at least one segment.
> > What if both seg_num and a shared memory region is specified in the
> > odp_pool_create call? Which takes precedence?
> >
> >> + */
> >> } pkt;
> >> -*/
> >> struct {
> >> uint32_t __res1; /* Keep struct identical to
> buf, */
> >> uint32_t __res2; /* until pool implementation
> is fixed*/
> >> --
> >> 2.2.2
> >>
> >>
> >> _______________________________________________
> >> 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
