On Fri, Feb 6, 2015 at 3:15 AM, Savolainen, Petri (NSN - FI/Espoo) < petri.savolai...@nsn.com> wrote:
> > > > -----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. > I agree, which is why there should be a num_pkts parameter specified here just as there is a num_bufs parameter for buffer pool creation. The number of packets is independent of the number of segments (one or more) they may occupy. Given num_pkts and the expected average length of those packets the implementation can determine how much storage it needs to reserve for the pool, which is what the current linux-generic code is doing. > > 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 >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
