Regards,
Bala
On 5 June 2017 at 21:52, Bill Fischofer <bill.fischo...@linaro.org> wrote:
> On Mon, Jun 5, 2017 at 8:24 AM, Bala Manoharan
> <bala.manoha...@linaro.org> wrote:
>> Regards,
>> Bala
>>
>>
>> On 5 June 2017 at 18:24, Bill Fischofer <bill.fischo...@linaro.org> wrote:
>>> On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <k...@semihalf.com> wrote:
>>>>
>>>>
>>>> Best Regards,
>>>> Stanislaw Kardach
>>>>
>>>> On 06/05/2017 02:27 PM, Balasubramanian Manoharan wrote:
>>>>> Adds threshold limit of the pool to the pool parameters.
>>>>> This threshold limit is a percentage of total size of the pool.
>>>>>
>>>>> Signed-off-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>>>>> ---
>>>>> include/odp/api/spec/pool.h | 32 ++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
>>>>> index 6fc5b6b..1c1ebe4 100644
>>>>> --- a/include/odp/api/spec/pool.h
>>>>> +++ b/include/odp/api/spec/pool.h
>>>>> @@ -20,6 +20,7 @@ extern "C" {
>>>>> #endif
>>>>>
>>>>> #include <odp/api/std_types.h>
>>>>> +#include <odp/api/support.h>
>>>>>
>>>>> /** @defgroup odp_pool ODP POOL
>>>>> * Operations on a pool.
>>>>> @@ -127,6 +128,9 @@ typedef struct odp_pool_capability_t {
>>>>> * The value of zero means that limited only by the
>>>>> available
>>>>> * memory size for the pool. */
>>>>> uint32_t max_uarea_size;
>>>>> +
>>>>> + /** Pool Threshold limit support */
>>>>> + odp_support_t pool_threshold_limit;
>>>>> } pkt;
>>>>>
>>>>> /** Timeout pool capabilities */
>>>>> @@ -214,6 +218,17 @@ typedef struct odp_pool_param_t {
>>>>> defined by pool capability pkt.max_uarea_size.
>>>>> Specify as 0 if no user area is needed. */
>>>>> uint32_t uarea_size;
>>>>> +
>>>>> + /** Pool threshold limit in percentage
>>>>> + *
>>>>> + * This value denotes the threshold limit of the
>>>>> pool in
>>>>> + * percentage of the total size of the pool and is
>>>>> used
>>>>> + * to configure the Random Early Discard (RED).
>>>>> + * When the number of packets in the pool is greater
>>>>> + * than this value RED is initiated in the pool and
>>>>> + * further incoming packets to the pool are dropped
>>>>> in
>>>>> + * a random sequence. */
>>>>> + uint8_t threshold_limit;
>>>> Is threshold limit "a percentage of total size of the pool" (hence I
>>>> guess 0-100) or rather an absolute value that triggers RED?
>>>
>>> Do we want to tie this specifically to RED or try to make things more
>>> general? There are many types of flow/congestion control algorithms
>>> that can be used so perhaps that should be better parameterized? Also,
>>> when specifying thresholds it's customary to set high and low
>>> watermarks to provide hysteresis. Some algorithms also need more than
>>> one data point to control the curves, so again this suggests that we
>>> need additional parameterization for this.
>>
>> The idea is to have a configuration value for enabling or disabling
>> Random Early Discard in the packet ingress which could be configured
>> either in the queue or in pool depending on the implementation. I
>> actually thought about having a higher /lower watermark but went
>> against this since the logic I have followed in this proposal is to
>> have a mechanism to start or stop the RED, IMO a single threshold
>> value is sufficient and RED gets initiated when packet is greater than
>> the threshold and is disabled when the packet limit on the pool is
>> lesser than this threshold.
>
> A single value may work for basic RED, but if you want to support
> other options like PFC then you need more than a single value or else
> you wind up "stuttering" around the single value when utilization is
> very close to it. That's because PFC doesn't just do something with a
> packet--it actively transmits pause frames.
I have only targeted basic RED in this proposal, which is common
across all platforms and it would be great to incorporate it to the
ODP ASAP.
If some other platforms comes with support for PFC at the hardware
level then we can add those additional features.
IMO we could keep this proposal simple so it is easier for upstreaming.
>
>>
>>>
>>>>
>>>>> } pkt;
>>>>>
>>>>> /** Parameters for timeout pools */
>>>>> @@ -329,6 +344,23 @@ uint64_t odp_pool_to_u64(odp_pool_t hdl);
>>>>> void odp_pool_param_init(odp_pool_param_t *param);
>>>>>
>>>>> /**
>>>>> + * Set threshold limit of the pool
>>>>> + *
>>>>> + * Set the threshold limit of the pool as a percentage of the total
>>>>> + * size of the pool. This value is used to configure Random Early
>>>>> Discard(RED)
>>>>> + * parameters of the pool and any incoming packets to the pool after
>>>>> reaching
>>>>> + * this threshold is dropped in a random sequence.
>>>>> + *
>>>>> + * @param pool Pool handle
>>>>> + * @param threshold_limit Threshold limit of the pool.
>>>>> + *
>>>>> + * @retval 0 on success
>>>>> + * @retval -1 on failure
>>>>> + */
>>>>> +
>>>>> +int odp_pool_threshold_limit(odp_pool_t pool, uint8_t threshold_limit);
>>>>> +
>>>>> +/**
>>>>> * @}
>>>>> */
>>>>>
>>>>>
