Re: [PATCH nf,v2] netfilter: flowtable: fetch stats only if flow is still alive

[wenxu <wenxu@xxxxxxxxx>]

Acked-by: wenxu <wenxu@xxxxxxxxx>

On 1/6/2020 5:59 AM, Pablo Neira Ayuso wrote:
> Do not fetch statistics if flow has expired since it might not in
> hardware anymore. After this update, remove the FLOW_OFFLOAD_HW_DYING
> check from nf_flow_offload_stats() since this flag is never set on.
>
> Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> v2: remove dead code after this update in nf_flow_offload_stats().
>
>  net/netfilter/nf_flow_table_core.c    | 5 ++---
>  net/netfilter/nf_flow_table_offload.c | 3 +--
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index e33a73cb1f42..9e6de2bbeccb 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -348,9 +348,6 @@ static void nf_flow_offload_gc_step(struct flow_offload *flow, void *data)
>  {
>  	struct nf_flowtable *flow_table = data;
>  
> -	if (flow->flags & FLOW_OFFLOAD_HW)
> -		nf_flow_offload_stats(flow_table, flow);
> -
>  	if (nf_flow_has_expired(flow) || nf_ct_is_dying(flow->ct) ||
>  	    (flow->flags & (FLOW_OFFLOAD_DYING | FLOW_OFFLOAD_TEARDOWN))) {
>  		if (flow->flags & FLOW_OFFLOAD_HW) {
> @@ -361,6 +358,8 @@ static void nf_flow_offload_gc_step(struct flow_offload *flow, void *data)
>  		} else {
>  			flow_offload_del(flow_table, flow);
>  		}
> +	} else if (flow->flags & FLOW_OFFLOAD_HW) {
> +		nf_flow_offload_stats(flow_table, flow);
>  	}
>  }
>  
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index d06969af1085..4d1e81e2880f 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -784,8 +784,7 @@ void nf_flow_offload_stats(struct nf_flowtable *flowtable,
>  	__s32 delta;
>  
>  	delta = nf_flow_timeout_delta(flow->timeout);
> -	if ((delta >= (9 * NF_FLOW_TIMEOUT) / 10) ||
> -	    flow->flags & FLOW_OFFLOAD_HW_DYING)
> +	if ((delta >= (9 * NF_FLOW_TIMEOUT) / 10))
>  		return;
>  
>  	offload = kzalloc(sizeof(struct flow_offload_work), GFP_ATOMIC);