Petri Savolainen(psavol) replied on github web page:
include/odp/api/spec/pool.h
line 13
@@ -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.
*/
typedef struct odp_pool_info_t {
const char *name; /**< pool name */
odp_pool_param_t params; /**< pool parameters */
+ /** Minimum address of any packet contained in this pool */
+ uintptr_t start_addr;
+ /** Maximum address of any packet contained in this pool */
+ uintptr_t end_addr;
Comment:
This spec should apply for all pool types (also other than packet). 0 address
may be used when there's no data in an event (e.g. timeout).
/** Minimum data address.
* This is the minimum address that application accessible data of any
* object (event) allocated from the pool may locate. When there's no
* application accessible data (e.g. ODP_POOL_TIMEOUT pools), the
* value maybe zero. */
uintptr_t min_data_addr;
/** Maximum data address.
* This is the maximum address that application accessible data of any
* object (event) allocated from the pool may locate. When there's no
* application accessible data (e.g. ODP_POOL_TIMEOUT pools), the
* value maybe zero. */
uintptr_t max_data_addr;
> sachin-saxena wrote
> I will wait for other's comments, if any, before sending V3 patchset
>> sachin-saxena wrote
>> 1. Agreed for first suggestion.
>> 2. For Second, As per my understanding, the VPP only needs to know the
>> range of addresses in order to calculate OFFSET within. VPP doesn't
>> assume/store packet address beyond its lifetime (Rx to Tx)
>>> sachin-saxena wrote
>>> 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_r143978589
updated_at 2017-10-11 11:09:52
