[ovs-dev,v2,1/7] dpif-netdev: Make dpcls optimization interval more generic.

So far the interval was only used for dpcls optimization.
Soon, we will use it for storing rxq cycles so make the
names more generic. Also, set the interval regardless
of whether dpcls optimization has occurred, as the
optimization interval will need to be consistent across
pmds.

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 lib/dpif-netdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

> So far the interval was only used for dpcls optimization.
> Soon, we will use it for storing rxq cycles so make the names more
> generic. Also, set the interval regardless of whether dpcls optimization
> has occurred, as the optimization interval will need to be consistent
> across pmds.
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
>  lib/dpif-netdev.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 47a9fa0..aa8c05d
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -178,6 +178,6 @@ struct emc_cache {
>  /* Simple non-wildcarding single-priority classifier. */
> 
> -/* Time in ms between successive optimizations of the dpcls subtable
> vector */ -#define DPCLS_OPTIMIZATION_INTERVAL 1000
> +/* Time in ms between successive optimizations of the pmd */ #define
> +PMD_OPTIMIZATION_INTERVAL 1000
> 
>  struct dpcls {
> @@ -4208,5 +4208,5 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread
> *pmd, struct dp_netdev *dp,
>      cmap_init(&pmd->flow_table);
>      cmap_init(&pmd->classifiers);
> -    pmd->next_optimization = time_msec() + DPCLS_OPTIMIZATION_INTERVAL;
> +    pmd->next_optimization = time_msec() + PMD_OPTIMIZATION_INTERVAL;

I'm not sure if we should be removing the dpcls optimization interval and combining it with the pmd optimization for the purpose rxq balancing.
Is there a situation when you will want the dpcls optimized at a different interval from the rxq rebalance?

Maybe you have thought of this already in this solution, but for instance, in the case of bursty non uniform traffic, would it be an idea to allow the pmd_optmization period to be configurable by the user so that enough time passes that the rxq rebalance occurs on statistic's that have been gathered over a longer period to negate the bursty aspect of the traffic? And in that case you may still want the dpcls optimization to occur at the usual period of 1000.

>      hmap_init(&pmd->poll_list);
>      hmap_init(&pmd->tx_ports);
> @@ -5656,7 +5656,7 @@ dp_netdev_pmd_try_optimize(struct
> dp_netdev_pmd_thread *pmd)
>              }
>              ovs_mutex_unlock(&pmd->flow_mutex);
> -            /* Start new measuring interval */
> -            pmd->next_optimization = now + DPCLS_OPTIMIZATION_INTERVAL;
>          }
> +        /* Start new measuring interval */
> +        pmd->next_optimization = now + PMD_OPTIMIZATION_INTERVAL;
>      }
>  }
> --
> 1.8.3.1

On 07/22/2017 03:50 PM, Stokes, Ian wrote:
>> So far the interval was only used for dpcls optimization.
>> Soon, we will use it for storing rxq cycles so make the names more
>> generic. Also, set the interval regardless of whether dpcls optimization
>> has occurred, as the optimization interval will need to be consistent
>> across pmds.
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>> ---
>>  lib/dpif-netdev.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 47a9fa0..aa8c05d
>> 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -178,6 +178,6 @@ struct emc_cache {
>>  /* Simple non-wildcarding single-priority classifier. */
>>
>> -/* Time in ms between successive optimizations of the dpcls subtable
>> vector */ -#define DPCLS_OPTIMIZATION_INTERVAL 1000
>> +/* Time in ms between successive optimizations of the pmd */ #define
>> +PMD_OPTIMIZATION_INTERVAL 1000
>>
>>  struct dpcls {
>> @@ -4208,5 +4208,5 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread
>> *pmd, struct dp_netdev *dp,
>>      cmap_init(&pmd->flow_table);
>>      cmap_init(&pmd->classifiers);
>> -    pmd->next_optimization = time_msec() + DPCLS_OPTIMIZATION_INTERVAL;
>> +    pmd->next_optimization = time_msec() + PMD_OPTIMIZATION_INTERVAL;
> 
> I'm not sure if we should be removing the dpcls optimization interval and combining it with the pmd optimization for the purpose rxq balancing.
> Is there a situation when you will want the dpcls optimized at a different interval from the rxq rebalance?
> 
> Maybe you have thought of this already in this solution, but for instance, in the case of bursty non uniform traffic, would it be an idea to allow the pmd_optmization period to be configurable by the user so that enough time passes that the rxq rebalance occurs on statistic's that have been gathered over a longer period to negate the bursty aspect of the traffic? And in that case you may still want the dpcls optimization to occur at the usual period of 1000.
> 

It's a good question. On the other hand a longer interval can mean using
older data. I'm reluctant to add another user config unless people
*really* think it's needed.

For the time being I've made the intervals independent. That way it
avoids a slight change in the dpcls optimization behaviour (when it
couldn't get the mutex), they can use different intervals and a user
config can easily be added in the future if needed.

As I don't need to make the dpcls interval generic anymore, I've dropped
this patch from v3.

>>      hmap_init(&pmd->poll_list);
>>      hmap_init(&pmd->tx_ports);
>> @@ -5656,7 +5656,7 @@ dp_netdev_pmd_try_optimize(struct
>> dp_netdev_pmd_thread *pmd)
>>              }
>>              ovs_mutex_unlock(&pmd->flow_mutex);
>> -            /* Start new measuring interval */
>> -            pmd->next_optimization = now + DPCLS_OPTIMIZATION_INTERVAL;
>>          }
>> +        /* Start new measuring interval */
>> +        pmd->next_optimization = now + PMD_OPTIMIZATION_INTERVAL;
>>      }
>>  }
>> --
>> 1.8.3.1
>