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
