> -----Original Message----- > From: Kamal Heib <kamalheib1@xxxxxxxxx> > Sent: Wednesday, October 10, 2018 7:17 AM > To: Parav Pandit <parav@xxxxxxxxxxxx> > Cc: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxx>; > linux-rdma@xxxxxxxxxxxxxxx > Subject: Re: [PATCH rdma-next 00/18] RDMA: Add support for ib_device_ops > > On Tue, Oct 09, 2018 at 07:06:56PM +0000, Parav Pandit wrote: > > > > > > > -----Original Message----- > > > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- > > > owner@xxxxxxxxxxxxxxx> On Behalf Of Kamal Heib > > > Sent: Tuesday, October 9, 2018 1:45 PM > > > To: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe > > > <jgg@xxxxxxxx> > > > Cc: linux-rdma@xxxxxxxxxxxxxxx; kamalheib1@xxxxxxxxx > > > Subject: [PATCH rdma-next 00/18] RDMA: Add support for ib_device_ops > > > > > > This patchset introduce a new structure that will contain all the > > > infiniband device operations, the structure will be used by the > > > providers to initialize their supported operations. This patchset > > > also includes the required changes in the core and ulps to start using it. > > > > > > Thanks, > > > Kamal > > > > > > Kamal Heib (18): > > > RDMA/core: Introduce ib_device_ops > > > RDMA/bnxt_re: Initialize ib_device_ops struct > > > RDMA/cxgb3: Initialize ib_device_ops struct > > > RDMA/cxgb4: Initialize ib_device_ops struct > > > RDMA/hfi1: Initialize ib_device_ops struct > > > RDMA/hns: Initialize ib_device_ops struct > > > RDMA/i40iw: Initialize ib_device_ops struct > > > RDMA/mlx4: Initialize ib_device_ops struct > > > RDMA/mlx5: Initialize ib_device_ops struct > > > RDMA/mthca: Initialize ib_device_ops struct > > > RDMA/nes: Initialize ib_device_ops struct > > > RDMA/ocrdma: Initialize ib_device_ops struct > > > RDMA/qedr: Initialize ib_device_ops struct > > > RDMA/qib: Initialize ib_device_ops struct > > > RDMA/usnic: Initialize ib_device_ops struct > > > RDMA/vmw_pvrdma: Initialize ib_device_ops struct > > > RDMA/rxe: Initialize ib_device_ops struct > > > RDMA: Start use ib_device_ops > > > > > > drivers/infiniband/core/cache.c | 12 +- > > > drivers/infiniband/core/core_priv.h | 12 +- > > > drivers/infiniband/core/cq.c | 6 +- > > > drivers/infiniband/core/device.c | 136 +++++++++++-- > > > drivers/infiniband/core/fmr_pool.c | 4 +- > > > drivers/infiniband/core/mad.c | 24 +-- > > > drivers/infiniband/core/nldev.c | 4 +- > > > drivers/infiniband/core/opa_smi.h | 4 +- > > > drivers/infiniband/core/rdma_core.c | 6 +- > > > drivers/infiniband/core/security.c | 8 +- > > > drivers/infiniband/core/smi.h | 4 +- > > > drivers/infiniband/core/sysfs.c | 26 +-- > > > drivers/infiniband/core/uverbs_cmd.c | 64 +++--- > > > drivers/infiniband/core/uverbs_main.c | 14 +- > > > drivers/infiniband/core/uverbs_std_types.c | 2 +- > > > .../infiniband/core/uverbs_std_types_counters.c | 10 +- > > > drivers/infiniband/core/uverbs_std_types_cq.c | 4 +- > > > drivers/infiniband/core/uverbs_std_types_dm.c | 6 +- > > > .../infiniband/core/uverbs_std_types_flow_action.c | 14 +- > > > drivers/infiniband/core/uverbs_std_types_mr.c | 4 +- > > > drivers/infiniband/core/verbs.c | 149 +++++++------- > > > drivers/infiniband/hw/bnxt_re/main.c | 97 +++++---- > > > drivers/infiniband/hw/cxgb3/iwch_provider.c | 64 +++--- > > > drivers/infiniband/hw/cxgb4/provider.c | 74 +++---- > > > drivers/infiniband/hw/hfi1/verbs.c | 19 +- > > > drivers/infiniband/hw/hns/hns_roce_device.h | 1 + > > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 11 ++ > > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 11 ++ > > > drivers/infiniband/hw/hns/hns_roce_main.c | 91 ++++----- > > > drivers/infiniband/hw/i40iw/i40iw_cm.c | 2 +- > > > drivers/infiniband/hw/i40iw/i40iw_verbs.c | 66 ++++--- > > > drivers/infiniband/hw/mlx4/alias_GUID.c | 2 +- > > > drivers/infiniband/hw/mlx4/main.c | 166 +++++++++------- > > > drivers/infiniband/hw/mlx5/main.c | 220 ++++++++++++--------- > > > drivers/infiniband/hw/mthca/mthca_provider.c | 139 ++++++++----- > > > drivers/infiniband/hw/nes/nes_cm.c | 2 +- > > > drivers/infiniband/hw/nes/nes_verbs.c | 66 ++++--- > > > drivers/infiniband/hw/ocrdma/ocrdma_main.c | 92 ++++----- > > > drivers/infiniband/hw/qedr/main.c | 103 +++++----- > > > drivers/infiniband/hw/qib/qib_verbs.c | 8 +- > > > drivers/infiniband/hw/usnic/usnic_ib_main.c | 61 +++--- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 82 ++++---- > > > drivers/infiniband/sw/rdmavt/vt.c | 90 ++++----- > > > drivers/infiniband/sw/rxe/rxe_verbs.c | 90 +++++---- > > > drivers/infiniband/ulp/ipoib/ipoib_main.c | 12 +- > > > drivers/infiniband/ulp/iser/iser_memory.c | 4 +- > > > drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c | 8 +- > > > drivers/infiniband/ulp/srp/ib_srp.c | 6 +- > > > include/rdma/ib_verbs.h | 212 ++++++++------------ > > > net/sunrpc/xprtrdma/fmr_ops.c | 2 +- > > > 50 files changed, 1257 insertions(+), 1057 deletions(-) > > > > > > -- > > > 2.14.4 > > > > I am not sure if this change is needed. > > The idea behind this change is to make some order in the IB device structure > by moving all the IB device operations to one place under the new > ib_device_ops structure. > > > But if an ops structure is defined, there should be logical useful group > should exist for the ops. > > > > Such as > > 1. datapath_ops (post_send, post_recv, poll_cq, req_notify_cq), which can > fall in same cacheline. > > 2. resource_ops(create, destroy, query, modify of qp, cq, mr, ah, srq) > > 3. gid_ops(query, add, del) 4. vf_ops(config_get, config_set, stats) > > 5. stats_ops(alloc, free, get) This aligns to other part of kernel, > > one example is netdev ops (rtnl, ndo, dcbnl, vf). > > > > > > Basiclly, I'm fine with the idea of creating different operations groups based > on common logic, but I'm not sure if the netdevice operations is a good > example because it includes operations for VF, stats, tunnel, fcoe, fdb and > etc. And similarly we have operations for rdma networking devices too on VFs, stats, resources. At minimum 5 broad logical groups I posted above.
