netdev - Re: [PATCH v6 net-next 08/12] flow_offload: add process to update action stats from hardware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c03a786e-6d21-1d93-2b97-9bf9a13250ef@mojatatu.com>
Date:   Sat, 11 Dec 2021 14:52:07 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Simon Horman <simon.horman@...igine.com>, netdev@...r.kernel.org
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Ido Schimmel <idosch@...dia.com>,
        Jiri Pirko <jiri@...nulli.us>, Oz Shlomo <ozsh@...dia.com>,
        Roi Dayan <roid@...dia.com>, Vlad Buslov <vladbu@...dia.com>,
        Baowen Zheng <baowen.zheng@...igine.com>,
        Louis Peens <louis.peens@...igine.com>,
        oss-drivers@...igine.com
Subject: Re: [PATCH v6 net-next 08/12] flow_offload: add process to update
 action stats from hardware

On 2021-12-09 04:28, Simon Horman wrote:
> include/net/act_api.h |  1 +
>   include/net/pkt_cls.h | 18 ++++++++++--------
>   net/sched/act_api.c   | 34 ++++++++++++++++++++++++++++++++++
>   3 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 7e4e79b50216..ce094e79f722 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -253,6 +253,7 @@ void tcf_action_update_stats(struct tc_action *a, u64 bytes, u64 packets,
>   			     u64 drops, bool hw);
>   int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
>   
> +int tcf_action_update_hw_stats(struct tc_action *action);
>   int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
>   			     struct tcf_chain **handle,
>   			     struct netlink_ext_ack *newchain);
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 13f0e4a3a136..1942fe72b3e3 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -269,18 +269,20 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
>   #ifdef CONFIG_NET_CLS_ACT
>   	int i;
>   
> -	preempt_disable();
> -
>   	for (i = 0; i < exts->nr_actions; i++) {
>   		struct tc_action *a = exts->actions[i];
>   
> -		tcf_action_stats_update(a, bytes, packets, drops,
> -					lastuse, true);
> -		a->used_hw_stats = used_hw_stats;
> -		a->used_hw_stats_valid = used_hw_stats_valid;
> -	}
> +		/* if stats from hw, just skip */
> +		if (tcf_action_update_hw_stats(a)) {
> +			preempt_disable();
> +			tcf_action_stats_update(a, bytes, packets, drops,
> +						lastuse, true);
> +			preempt_enable();
>   
> -	preempt_enable();
> +			a->used_hw_stats = used_hw_stats;
> +			a->used_hw_stats_valid = used_hw_stats_valid;
> +		}
> +	}
>   #endif
>   }

Sorry - didnt quiet follow this one even after reading to the end.
I may have missed the obvious in the equivalence:
In the old code we did the preempt first then collect. The changed
version only does it if tcf_action_update_hw_stats() is true.

cheers,
jamal