On Fri, 17 Nov 2017 14:36:22 +0100
Paolo Abeni <[email protected]> wrote:
> Currently, when an offloaded DP flow is deleted, the related stats
> are unconditionally cleared. As a result the counters for the
> originating open flow are corrupted.
> 
> This change addresses the issue updating the DP stats with the current
> values provided by the flower APIs before deleting the tc filter, as
> currently done by others DP providers.
> 
> Tested vs different OVS H/W offload devices, verifying that the open flow
> stats are correct after the expiration of the related, H/W offloaded DP
> flow.
> 
> Fixes: 30b6b047260b ("netdev-tc-offloads: Implement netdev flow del using tc 
> interface")
> CC: Paul Blakey <[email protected]>
> Reported-by: Kumar Sanghvi <[email protected]>
> Signed-off-by: Paolo Abeni <[email protected]>
> ---
> v1 -> v2:
>  - ensure to always clear stats if tc_get_flower() fails for some reasons
> ---
>  lib/netdev-tc-offloads.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index f86f3e046..781d849e4 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -1101,6 +1101,7 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
>                     const ovs_u128 *ufid,
>                     struct dpif_flow_stats *stats)
>  {
> +    struct tc_flower flower;
>      struct netdev *dev;
>      int prio = 0;
>      int ifindex;
> @@ -1120,14 +1121,20 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
>          return -ifindex;
>      }
>  
> +    if (stats) {
> +        memset(stats, 0, sizeof *stats);
> +        if (!tc_get_flower(ifindex, prio, handle, &flower)) {
> +            stats->n_packets = get_32aligned_u64(&flower.stats.n_packets);
> +            stats->n_bytes = get_32aligned_u64(&flower.stats.n_bytes);
> +            stats->used = flower.lastused;
> +        }
> +    }
> +
>      error = tc_del_filter(ifindex, prio, handle);
>      del_ufid_tc_mapping(ufid);
>  
>      netdev_close(dev);
>  
> -    if (stats) {
> -        memset(stats, 0, sizeof *stats);
> -    }
>      return error;
>  }
>  

Thanks for the V2 :-)
Acked-by: Flavio Leitner <[email protected]>

-- 
Flavio

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

Reply via email to