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:
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_r143714568
updated_at 2017-10-10 12:41:52
