netdev - Re: [PATCH rdma-next 03/10] RDMA/counters: Support to allocate per-port optional counter statistics

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: <20210823193020.GA1002624@nvidia.com>
Date:   Mon, 23 Aug 2021 16:30:20 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Mark Zhang <markzhang@...dia.com>
Cc:     dledford@...hat.com, saeedm@...dia.com, linux-rdma@...r.kernel.org,
        netdev@...r.kernel.org, aharonl@...dia.com, netao@...dia.com,
        leonro@...dia.com
Subject: Re: [PATCH rdma-next 03/10] RDMA/counters: Support to allocate
 per-port optional counter statistics

On Wed, Aug 18, 2021 at 02:24:21PM +0300, Mark Zhang wrote:
> From: Aharon Landau <aharonl@...dia.com>
> 
> Add an alloc_op_port_stats() API, as well as related structures, to support
> per-port op_stats allocation during counter module initialization.
> 
> Signed-off-by: Aharon Landau <aharonl@...dia.com>
> Signed-off-by: Neta Ostrovsky <netao@...dia.com>
> Signed-off-by: Mark Zhang <markzhang@...dia.com>
>  drivers/infiniband/core/counters.c | 18 ++++++++++++++++++
>  drivers/infiniband/core/device.c   |  1 +
>  include/rdma/ib_verbs.h            | 24 ++++++++++++++++++++++++
>  include/rdma/rdma_counter.h        |  1 +
>  4 files changed, 44 insertions(+)
> 
> diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
> index df9e6c5e4ddf..b8b6db98bfdf 100644
> +++ b/drivers/infiniband/core/counters.c
> @@ -611,6 +611,15 @@ void rdma_counter_init(struct ib_device *dev)
>  		port_counter->hstats = dev->ops.alloc_hw_port_stats(dev, port);
>  		if (!port_counter->hstats)
>  			goto fail;
> +
> +		if (dev->ops.alloc_op_port_stats) {
> +			port_counter->opstats =
> +				dev->ops.alloc_op_port_stats(dev, port);
> +			if (!port_counter->opstats)
> +				goto fail;

It would be nicer to change the normal stats to have more detailed
meta information instead of adding an entire parallel interface like
this.

struct rdma_hw_stats {
	struct mutex	lock;
	unsigned long	timestamp;
	unsigned long	lifespan;
	const char * const *names;

Change the names to a struct

 const struct rdma_hw_stat_desc *descs;

struct rdma_hw_stat_desc {
   const char *name;
   unsigned int flags;
   unsigned int private;
}

and then define a FLAG_OPTIONAL.

Then alot of this oddness goes away.

You might also need a small allocated bitmap to store the
enabled/disabled state

Then the series basically boils down to adding some 'modify counter'
driver op that flips the enable/disable flag

And the netlink patches to expose the additional information.

Jason