On Thu, Apr 13, 2017 at 2:17 AM, Elo, Matias (Nokia - FI/Espoo) <matias....@nokia-bell-labs.com> wrote: > >> On 12 Apr 2017, at 22:52, Bill Fischofer <bill.fischo...@linaro.org> wrote: >> >> On Wed, Apr 12, 2017 at 7:58 AM, Matias Elo <matias....@nokia.com> wrote: >>> On some packet pool implementations the number of available packets may vary >>> depending on the packet length (examples below). E.g. a packet pool may >>> include >>> smaller sub-pools for different packet length ranges. >> >> I'm not sure what the motivation is for this proposed change, but the >> pkt.num specified on odp_pool_create() sets the maximum number of >> packets that can be allocated from the pool, not the minimum. This is >> combined with the pkt.len field to say that pkt.num packets of len >> pkt.len can be allocated. > > The need for this information originally came up when fixing an issue in > pool_create(). pool_create() didn't allocate enough packets if the requested > length (pkt.len) packets were segmented. As a result of this fix it is now > possible to allocate more than pkt.num packets from the pool, if the > requested packet length fits into a single segment (or less segments than > the original pkt.len).
This sounds like trying to fix an implementation bug with an API change. The implementation has all the info it needs with pkt.num, pkt.len, and pkt.max_len to realize the specified semantics. If there is segmentation involved in pkt.len it can take that into account in configuring the pool. > > The spec of pkt.num is changed to "The exact number of 'len' byte packets > that the pool must provide.". The current spec say this is "The number of packets that the pool must provide that are packet length 'len' bytes or smaller". That seems perfectly unambiguous. If a given implementation doesn't do that, that's a bug in that implementation and it should be fixed. The validation tests should be testing this, so if that's not being done then that's a bug in the validation test which should also be fixed. > >> If the application allocated packets larger >> than this size (up to pkt.max_len) then the actual number of packets >> that can be successfully allocated from the pool may be lower than >> pkt.num, but it will never be greater. > > Depending on the pool implementation this may not always be the case > (not likely) and the ascii pictures in the cover letter tried to visualise > this > possibility. For example, a pool may consists of smaller HW sub-pools for > different packet length ranges and these sub-pools may be different in size. How an implementation realizes the specified API semantics is not defined by ODP. It should never be possible to allocate more than pkt.num packets from a pool, and it should be possible to allocate exactly pkt.num packets of len pkt.len. That's what the configuration parameters control. > >> >> The reason for wanting the maximum number is to bound the number of >> "in flight" packets for QoS purposes. As a pool reaches this limit, >> RED and similar algorithms kick in to start dropping packets at RX. If >> there is no fixed maximum number of packets that makes QoS processing >> erratic. >> > > There is still a maximum and an application can query it with odp_pool_info() > (odp_pool_info_t.pkt.max_num). RED and similar work as previously as > there is no API for configuring them in ODP. True today, but we have discussed adding watermarks on pools that can be used to trigger such behavior. > >> We had previously floated the idea of allowing pool groups to be >> defined which would accommodate the practice where packets get >> assigned to different sub-pools based on their size, however this was >> seen to be overly complicated, especially on platforms that have >> hardware buffer managers. > > True, this gets complicated fast. The updated odp_pool_info_t includes new > maximum packet counts to provide more information to the application > without going to the specifics of the underlying pool implementation. > >> Do you have a specific use case that can't be handled by the current >> structures? > > Described above. It seems you've described problems with some implementations rather than application needs. The spec was designed to allow applications to specify their requirements on pool capacity, which it seems to do. > > > -Matias > > >
