sachin-saxena replied on github web page: include/odp/api/spec/pool.h line 5 @@ -290,10 +290,16 @@ odp_pool_t odp_pool_lookup(const char *name); /** * Pool information struct * Used to get information about a pool. + * @note The difference between end_addr & start_addr + * will result in buffers address range belong to this pool.
Comment: OK. agreed > Bill Fischofer(Bill-Fischofer-Linaro) wrote: > The only other ambiguity is whether the address that applications see for a > packet is constant. VPP clearly assumes this but this is not implied by the > ODP spec. The "lifetime" of packet visibility is from the time a call like > `odp_packet_data()` is made until the thread releases the `odp_packet_t` that > was used to obtain this address. If some other thread receives that handle > and calls `odp_packet_data()` there's no requirement or guarantee that it > will get the same address as the previous owner. >> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >> I'd change these to "visible address" rather than "address". The point is >> that the spec says nothing about how packets are represented within an >> implementation. It only states what applications may see by using other ODP >> APIs that manipulate odp_packet_t object. >>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>> I'd delete this note as it's not necessary and not complete for the ODP API >>> spec. >>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>> As we discussed earlier today. We should only need an extension to >>>> `odp_pool_info()` to return the `min_addr` and `max_addr` of any packet >>>> contained in the pool. This can simply be added to the end of the >>>> `odp_pool_info_t` struct. The application (in this case VPP) can then >>>> verify that the range is usable by it (_i.e.,_ is containable in 32 bits) >>>> and can store the info it needs to do its indexing from this info. >>>>> Petri Savolainen(psavol) wrote: >>>>> How sparse a "linear pool" may be? All implementations can claim linear >>>>> pool support by returning info.first_addr = 0, info.last_addr = >>>>> (uintptr_t) -1, where address size may be 64bits. >>>>> >>>>> Addresses could be used for debugging, but not much more than that. What >>>>> VPP is going to do with this information ? >>>>> >>>>> >>>>> >>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>> @muvarov To your points: >>>>>> >>>>>> 1. No, VPP only deals with packets so there's no need to generalize this >>>>>> since it is an accommodation for VPP, not something we want to encourage >>>>>> other applications to use. >>>>>> >>>>>> 2. We abandoned that approach because it required the application to >>>>>> know how much memory the ODP implementation needed for its internal use, >>>>>> which is not something it can reasonably know. So `odp_pool_create()` is >>>>>> responsible for allocating any shm used by the pool based on input >>>>>> provided by the caller. >>>>>> >>>>>> 3. Agree, the union seems strange here. Since having an output parameter >>>>>> in the `odp_pool_param_t` is not something we want this will have to be >>>>>> reworked anyway. >>>>>>> muvarov wrote >>>>>>> 1) is it needed for tmo and bufs also? why it's on only inside packets? >>>>>>> 2) How memory for *start_addr is allocated in VPP? In previous version >>>>>>> of odp_pool_create() was odp_shm_t parameter which said to reuse >>>>>>> already created shm. Maybe that needs to be reconsidered. 3) why union >>>>>>> is needed? why it's not with uintptr_t ? >>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote: >>>>>>>> @Bill-Fischofer-Linaro yes, this looks like a good approach. >>>>>>>> @sachin-saxena could you please reimplement your PR following >>>>>>>> @Bill-Fischofer-Linaro 's suggestion? >>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote: >>>>>>>>> Got confused between single-segment packet and linear space for >>>>>>>>> packets, sorry. >>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>> Yes the existing `seg_len` parameter is set to the minimum size >>>>>>>>>> segment that the application requires. This can be set to be equal >>>>>>>>>> to `max_len` to require single-segment packets. >>>>>>>>>> >>>>>>>>>> However @sachin-saxena mentioned at SFO17 that VPP can deal with >>>>>>>>>> multi-segment packets, though I'm not sure how that works. >>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote: >>>>>>>>>>> VPP will not run on such platforms without modification. If the >>>>>>>>>>> goal is to not modify VPP then an `odp_pool_capability()` output >>>>>>>>>>> that says whether pools can present a linear interface. During >>>>>>>>>>> Connect @psavol also mentioned that a separate API to get this info >>>>>>>>>>> would be more appropriate than bending the input to >>>>>>>>>>> `odp_pool_create()` to give output parameters. >>>>>>>>>>> >>>>>>>>>>> My personal preference would be something like: >>>>>>>>>>> >>>>>>>>>>> * `odp_pool_capability()` indicates whether pools can be created >>>>>>>>>>> that provide linear addressing. >>>>>>>>>>> >>>>>>>>>>> * An option on `odp_pool_create()` to request a linear pool. >>>>>>>>>>> >>>>>>>>>>> * A new API (or an extension to the existing `odp_pool_info()` API) >>>>>>>>>>> to get the mapping info that VPP needs. >>>>>>>>>>> >>>>>>>>>>> That would seem to be more consistent with overall ODP structure. >>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote: >>>>>>>>>>>> What will happen for platforms where `odp_packet_t` is not mapped >>>>>>>>>>>> to the virtual memory? >>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote: >>>>>>>>>>>>> Isn't setting `seg_len` enough? Maybe we should provide an >>>>>>>>>>>>> updated definition for `seg_len` (e.g. in case `seg_len > >>>>>>>>>>>>> max_len`, always use single segment). https://github.com/Linaro/odp/pull/200#discussion_r143958436 updated_at 2017-10-11 09:34:01
