| Message ID | 1496665637-15790-1-git-send-email-bala.manoharan@linaro.org |
|---|---|
| State | New |
| Headers | show |
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.manoharan@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? > } 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); > + > +/** > * @} > */ > >
On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <kda@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.manoharan@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. > >> } 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); >> + >> +/** >> * @} >> */ >> >>
Best Regards, Stanislaw Kardach On 06/05/2017 02:54 PM, Bill Fischofer wrote: > On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <kda@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.manoharan@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. > I'm not disputing the generalization, I've mentioned RED because it's right there in the comment. What I meant is whether this is a percentage value or absolute value (as in number of buffers), because commit message suggested former and this comments suggest latter with "When the number of packets in the pool is greater than this value". Additionally, shouldn't it be "when number of packets in this pool is _lower_ than this value"? Since otherwise we're saying that congestion algorithms are to be applied on a full pull. >> >>> } 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); >>> + >>> +/** >>> * @} >>> */ >>> >>>
Regards, Bala On 5 June 2017 at 18:02, Stanislaw Kardach <kda@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.manoharan@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? Yes. It is defined as a percentage. > >> } 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); >> + >> +/** >> * @} >> */ >> >>
Regards, Bala On 5 June 2017 at 18:42, Stanislaw Kardach <kda@semihalf.com> wrote: > > > Best Regards, > Stanislaw Kardach > > On 06/05/2017 02:54 PM, Bill Fischofer wrote: >> On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <kda@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.manoharan@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. >> > I'm not disputing the generalization, I've mentioned RED because it's > right there in the comment. > What I meant is whether this is a percentage value or absolute value (as > in number of buffers), because commit message suggested former and this > comments suggest latter with "When the number of packets in the pool is > greater than this value". So lets say the total size of the packet pool is 1024 segments and the threshold for starting/ stopping RED is 750 segments then the threshold limit is set as 75% rather than the absolute value of 750 segments. I will update the commit message to make it more clear. > > Additionally, shouldn't it be "when number of packets in this pool is > _lower_ than this value"? Since otherwise we're saying that congestion > algorithms are to be applied on a full pull. I will update the documentation to indicate when RED gets dropped in the system. The logic is RED is enabled when packets in pool is greater than threshold and RED is disabled when packets in pool is less than or equal to threshold limit. Regards, Bala >>> >>>> } 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); >>>> + >>>> +/** >>>> * @} >>>> */ >>>> >>>>
Regards, Bala On 5 June 2017 at 18:24, Bill Fischofer <bill.fischofer@linaro.org> wrote: > On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <kda@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.manoharan@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. > >> >>> } 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); >>> + >>> +/** >>> * @} >>> */ >>> >>>
On Mon, Jun 5, 2017 at 8:24 AM, Bala Manoharan <bala.manoharan@linaro.org> wrote: > Regards, > Bala > > > On 5 June 2017 at 18:24, Bill Fischofer <bill.fischofer@linaro.org> wrote: >> On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <kda@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.manoharan@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. > >> >>> >>>> } 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); >>>> + >>>> +/** >>>> * @} >>>> */ >>>> >>>>
Regards, Bala On 5 June 2017 at 21:52, Bill Fischofer <bill.fischofer@linaro.org> wrote: > On Mon, Jun 5, 2017 at 8:24 AM, Bala Manoharan > <bala.manoharan@linaro.org> wrote: >> Regards, >> Bala >> >> >> On 5 June 2017 at 18:24, Bill Fischofer <bill.fischofer@linaro.org> wrote: >>> On Mon, Jun 5, 2017 at 7:32 AM, Stanislaw Kardach <kda@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.manoharan@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); >>>>> + >>>>> +/** >>>>> * @} >>>>> */ >>>>> >>>>>
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; } 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); + +/** * @} */
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.manoharan@linaro.org> --- include/odp/api/spec/pool.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) -- 1.9.1