Re: [PATCH net-next v2 7/7] net: nexthop: Expose nexthop group HW stats to user space

[David Ahern <dsahern@kernel.org>]

On 2/29/24 11:16 AM, Petr Machata wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> Add netlink support for reading NH group hardware stats.
> 
> Stats collection is done through a new notifier,
> NEXTHOP_EVENT_HW_STATS_REPORT_DELTA. Drivers that implement HW counters for
> a given NH group are thereby asked to collect the stats and report back to
> core by calling nh_grp_hw_stats_report_delta(). This is similar to what
> netdevice L3 stats do.
> 
> Besides exposing number of packets that passed in the HW datapath, also
> include information on whether any driver actually realizes the counters.
> The core can tell based on whether it got any _report_delta() reports from
> the drivers. This allows enabling the statistics at the group at any time,
> with drivers opting into supporting them. This is also in line with what
> netdevice L3 stats are doing.
> 
> So as not to waste time and space, tie the collection and reporting of HW
> stats with a new op flag, NHA_OP_FLAG_DUMP_HW_STATS.
> 
> Co-developed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Kees Cook <keescook@chromium.org> # For the __counted_by bits
> ---
> 
> Notes:
>     v2:
>     - Use uint to encode NHA_GROUP_STATS_ENTRY_PACKETS_HW
>     - Do not cancel outside of nesting in nla_put_nh_group_stats()
> 
>  include/net/nexthop.h        |  18 +++++
>  include/uapi/linux/nexthop.h |   9 +++
>  net/ipv4/nexthop.c           | 133 ++++++++++++++++++++++++++++++++---
>  3 files changed, 151 insertions(+), 9 deletions(-)
> 


> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 15f108c440ae..169a003cc855 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -43,7 +43,8 @@ static const struct nla_policy rtm_nh_policy_new[] = {
>  static const struct nla_policy rtm_nh_policy_get[] = {
>  	[NHA_ID]		= { .type = NLA_U32 },
>  	[NHA_OP_FLAGS]		= NLA_POLICY_MASK(NLA_U32,
> -						  NHA_OP_FLAG_DUMP_STATS),
> +						  NHA_OP_FLAG_DUMP_STATS |
> +						  NHA_OP_FLAG_DUMP_HW_STATS),
>  };
>  
>  static const struct nla_policy rtm_nh_policy_del[] = {
> @@ -56,7 +57,8 @@ static const struct nla_policy rtm_nh_policy_dump[] = {
>  	[NHA_MASTER]		= { .type = NLA_U32 },
>  	[NHA_FDB]		= { .type = NLA_FLAG },
>  	[NHA_OP_FLAGS]		= NLA_POLICY_MASK(NLA_U32,
> -						  NHA_OP_FLAG_DUMP_STATS),
> +						  NHA_OP_FLAG_DUMP_STATS |
> +						  NHA_OP_FLAG_DUMP_HW_STATS),

2 instances of the same mask; worth giving it a name.

Reviewed-by: David Ahern <dsahern@kernel.org>