On Mon, Apr 01, 2019 at 04:36:42PM +0300, Gal Pressman wrote:
> On 01-Apr-19 16:26, Leon Romanovsky wrote:
> > On Mon, Apr 01, 2019 at 03:04:55PM +0300, Gal Pressman wrote:
> >> On 01-Apr-19 11:47, Leon Romanovsky wrote:
> >>> From: Mark Zhang <markz@xxxxxxxxxxxx>
> >>>
> >>> Add an API to support set/clear per-port auto mode.
> >>>
> >>> Signed-off-by: Mark Zhang <markz@xxxxxxxxxxxx>
> >>> Reviewed-by: Majd Dibbiny <majd@xxxxxxxxxxxx>
> >>> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >>> ---
> >>> +void rdma_counter_init(struct ib_device *dev)
> >>> +{
> >>> + struct rdma_port_counter *port_counter;
> >>> + u32 port;
> >>> +
> >>> + if (!dev->ops.alloc_hw_stats)
> >>
> >> Why is alloc_hw_stats needed?
> >
> > We are using mlx5_ib Q-counters to bind/unbind and they are allocated through
> > alloc_hw_stat interface. This interface is not available in all flavours of mlx4.
>
> What about other drivers which are not mlx4/5?
> From an API standpoint, I don't see a reason to require alloc_hw_stats.
>
> This kinda relates to my question of whether these new counters are related to
> the existing hw_counters. This check suggests that they are, but I don't
> understand why.
I see "rdma statistics ..." as an ultimate tool to present everything
related to device statistics. It includes already exposed HW counters.
This is an extra reason why we want ops.alloc_hw_stats check.
>
> >
> >>
> >>> + return 0;
> >>> +
> >>> + rdma_for_each_port(dev, port) {
> >>> + port_counter = &dev->port_data[port].port_counter;
> >>> + port_counter->mode.mode = RDMA_COUNTER_MODE_NONE;
> >>> + mutex_init(&port_counter->lock);
> >>> + }
> >>> +}
> >>> +
> >>> +void rdma_counter_cleanup(struct ib_device *dev)
> >>> +{
> >>> +}
> >>> /* rdma netdev type - specifies protocol type */
> >>> diff --git a/include/rdma/rdma_counter.h b/include/rdma/rdma_counter.h
> >>> index 283ac1a0cdb7..a8a7c1627800 100644
> >>> --- a/include/rdma/rdma_counter.h
> >>> +++ b/include/rdma/rdma_counter.h
> >>> @@ -6,8 +6,26 @@
> >>> #ifndef _RDMA_COUNTER_H_
> >>> #define _RDMA_COUNTER_H_
> >>>
> >>> +#include <linux/mutex.h>
> >>> +
> >>> #include <rdma/ib_verbs.h>
> >>> #include <rdma/restrack.h>
> >>> +#include <rdma/rdma_netlink.h>
> >>> +
> >>> +struct auto_mode_param {
> >>> + int qp_type;
> >>
> >> enum ib_qp_type?
> >
> > Maybe, it is not exposed now, but can expose if it is needed.
> >
> > At least it needs to be _u8 and not int.
> > See include/uapi/rdma/ib_user_verbs.h:struct ib_uverbs_create_qp
>
> I think the reason it's not used in ib_uverbs_create_qp is it's kabi,
> rdma_counter.h isn't.
No problem.
>
> >
> >>
> >>> +};
> >>> +
> >>> +struct rdma_counter_mode {
> >>> + enum rdma_nl_counter_mode mode;
> >>> + enum rdma_nl_counter_mask mask;
> >>> + struct auto_mode_param param;
> >>> +};
> >>> +
> >>> +struct rdma_port_counter {
> >>> + struct rdma_counter_mode mode;
> >>> + struct mutex lock;
> >>> +};
> >>>
Attachment:
signature.asc
Description: PGP signature
