Bill Fischofer(Bill-Fischofer-Linaro) 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:
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_r143715810
updated_at 2017-10-10 12:47:07
