netdev - Re: [PATCH mlx5-next v2 11/13] IB/mlx5: Add flow counters binding support

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] [day] [month] [year] [list]

Message-ID: <20180530153521.GD30754@ziepe.ca>
Date:   Wed, 30 May 2018 09:35:21 -0600
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Yishai Hadas <yishaih@....mellanox.co.il>
Cc:     Leon Romanovsky <leon@...nel.org>,
        Doug Ledford <dledford@...hat.com>,
        Leon Romanovsky <leonro@...lanox.com>,
        RDMA mailing list <linux-rdma@...r.kernel.org>,
        Boris Pismenny <borisp@...lanox.com>,
        Matan Barak <matanb@...lanox.com>,
        Raed Salem <raeds@...lanox.com>,
        Yishai Hadas <yishaih@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        linux-netdev <netdev@...r.kernel.org>,
        Alex Rosenbaum <alexr@...lanox.com>
Subject: Re: [PATCH mlx5-next v2 11/13] IB/mlx5: Add flow counters binding
 support

On Wed, May 30, 2018 at 06:24:00PM +0300, Yishai Hadas wrote:
> On 5/30/2018 6:06 PM, Jason Gunthorpe wrote:
> >On Wed, May 30, 2018 at 03:31:34PM +0300, Yishai Hadas wrote:
> >>On 5/29/2018 10:56 PM, Jason Gunthorpe wrote:
> >>>On Tue, May 29, 2018 at 04:09:15PM +0300, Leon Romanovsky wrote:
> >>>>diff --git a/include/uapi/rdma/mlx5-abi.h b/include/uapi/rdma/mlx5-abi.h
> >>>>index 508ea8c82da7..ef3f430a7050 100644
> >>>>+++ b/include/uapi/rdma/mlx5-abi.h
> >>>>@@ -443,4 +443,18 @@ enum {
> >>>>  enum {
> >>>>  	MLX5_IB_CLOCK_INFO_V1              = 0,
> >>>>  };
> >>>>+
> >>>>+struct mlx5_ib_flow_counters_data {
> >>>>+	__aligned_u64   counters_data;
> >>>>+	__u32   ncounters;
> >>>>+	__u32   reserved;
> >>>>+};
> >>>>+
> >>>>+struct mlx5_ib_create_flow {
> >>>>+	__u32   ncounters_data;
> >>>>+	__u32   reserved;
> >>>>+	/* Following are counters data based on ncounters_data */
> >>>>+	struct mlx5_ib_flow_counters_data data[];
> >>>>+};
> >>>>+
> >>>>  #endif /* MLX5_ABI_USER_H */
> >>>
> >>>This uapi thing still needs to be fixed as I pointed out before.
> >>
> >>In V3 we can go with below, no change in memory layout but it can clarify
> >>the code/usage.
> >>
> >>struct mlx5_ib_flow_counters_desc {
> >>         __u32   description;
> >>         __u32   index;
> >>};
> >>
> >>struct mlx5_ib_flow_counters_data {
> >>         RDMA_UAPI_PTR(struct mlx5_ib_flow_counters_desc *, counters_data);
> >>         __u32   ncounters;
> >>         __u32   reserved;
> >>};
> >
> >OK, this is what I asked for originally..
> >
> >>struct mlx5_ib_create_flow {
> >>         __u32   ncounters_data;
> >>         __u32   reserved;
> >>         /* Following are counters data based on ncounters_data */
> >>         struct mlx5_ib_flow_counters_data data[];
> >>
> >>
> >>>I still can't figure out why this should be a 2d array.
> >>
> >>This comes to support the future case of multiple counters objects/specs
> >>passed with the same flow. There is a need to differentiate mapping data for
> >>each counters object and that is done via the 'ncounters_data' field and the
> >>2d array.
> >
> >This still doesn't make any sense to me. How are these multiple
> >counters objects/specs going to be identified?
> >
> >Basically, what does the array index for data[] mean? Should it match
> >the spec index from the main verb or something?
> >
> 
> Each entry in the data[] should match a corresponding counter object that
> was pointed by a counters spec upon the flow creation.

What if there are a mixture of specs, some with counters and some
without?

The index is just matching the index of the spec? That makes sense.

> >This is a good place for a comment, since the intention is completely
> >opaque here.
> 
> Sure, we'll add comment to clarify the above.

Sure, can leave the flex array then too

Jason