On Tue, Feb 14, 2023 at 02:39:25PM +0100, Eelco Chaudron wrote:
> Depending on the driver implementation, it can take from 0.2 seconds
> up to 2 seconds before offloaded flow statistics are updated. This is
> true for both TC and rte_flow-based offloading. This is causing a
> problem with min-revalidate-pps, as old statistic values are used
> during this period.
> 
> This fix will wait for at least 2 seconds, by default, before assuming no
> packets where received during this period.
> 
> Signed-off-by: Eelco Chaudron <[email protected]>

Some minor nits inline, but this looks good to me.

Reviewed-by: Simon Horman <[email protected]>

> Changes:
> - v2: Use existing ukey->offloaded so the revalidate_missed_dp_flow case is
>       also covered.
> - v3: Added OVS_REQUIRES(ukey->mutex) to should_revalidate() to keep
>       thread-safety-analysis happy.
> - v4: Add a configurable option.
>       After looking at multiple vendor implementation for both TC and
>       rte_flow I came to the conclusion that the delay is roughly between
>       0.2 and 2 seconds. Updated commit message.
> ---
>  ofproto/ofproto-dpif-upcall.c |   26 ++++++++++++++++----------

nit: there was some fuzz in ofproto-dpif-upcall. when applying this patch.

>  ofproto/ofproto-provider.h    |    4 ++++
>  ofproto/ofproto.c             |    9 +++++++++
>  ofproto/ofproto.h             |    2 ++
>  vswitchd/bridge.c             |    3 +++
>  vswitchd/vswitch.xml          |   12 ++++++++++++
>  6 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index db7570ee2..24ac6806c 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -2100,10 +2100,12 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey)
>  }
>  
>  static bool
> -should_revalidate(const struct udpif *udpif, uint64_t packets,
> -                  long long int used)
> +should_revalidate(const struct udpif *udpif, struct udpif_key *ukey,
> +                  uint64_t packets)

nit: can ukey be const?

> +    OVS_REQUIRES(ukey->mutex)
>  {
>      long long int metric, now, duration;
> +    long long int used = ukey->stats.used;
>  
>      if (!ofproto_min_revalidate_pps) {
>          return true;

...
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to