[openib-general] [PATCH 1/6] libibverbs include files changes.

[Krishna Kumar2]

OTOH, I think #A below is possible (will provide feedback on this tmrw).
Please let me know comments on other items though.

Thanks,

- KK

Krishna Kumar2/India/IBM wrote on 07/31/2006 02:44:46 PM:

> Hi Sean & Roland,
> 
> Thanks for your review comments. I will respond to all comments in
> this mail to make it easy :)
> 
> A.
> -------
> >  > Why not just do:
> >  > 
> >  > #define rdma_structure_name ibv_structure_name
> >  > 
> >  > Then we simply remove the #define and replace ibv_structure_name 
with 
> >  > rdma_structure_name when ready.  This better shows the changes, 
prevents 
> >  > duplicating every structure, and should avoid any compile warnings.
> > 
> > I agree, except let's make rdma_structure_name the real name, and do
> > the define the opposite way.  That way it's easier to remove the
> > compatibility stuff later.

> I had tried this some time back (with amso driver actually) and found it 
had some
> issues. Eg, some data structures have the same name as API routines. I 
did :
> 
> "cscope -1<fn_names> -L" on all function names and found 27 functions 
that
> also clashed with structure names. The full list is :
> 
> ********* Problem with ibv_query_device **********
> include/infiniband/kern-abi.h ibv_query_device 146 struct 
ibv_query_device {
> src/verbs.c ibv_query_device 81 int ibv_query_device(struct ibv_context 
*context
> ********* Problem with ibv_query_port **********
> include/infiniband/kern-abi.h ibv_query_port 198 struct ibv_query_port {
> src/verbs.c ibv_query_port 87 int ibv_query_port(struct ibv_context 
*context
> ********* Problem with ibv_alloc_pd **********
> include/infiniband/kern-abi.h ibv_alloc_pd 231 struct ibv_alloc_pd {
> src/verbs.c ibv_alloc_pd 137 struct ibv_pd *ibv_alloc_pd(struct 
ibv_context *
> ********* Problem with ibv_dealloc_pd **********
> include/infiniband/kern-abi.h ibv_dealloc_pd 243 struct ibv_dealloc_pd {
> src/verbs.c ibv_dealloc_pd 148 int ibv_dealloc_pd(struct ibv_pd *pd)
> ********* Problem with ibv_reg_mr **********
> include/infiniband/kern-abi.h ibv_reg_mr 250 struct ibv_reg_mr {
> src/verbs.c ibv_reg_mr 153 struct ibv_mr *ibv_reg_mr(struct ibv_pd *pd,
> ********* Problem with ibv_dereg_mr **********
> include/infiniband/kern-abi.h ibv_dereg_mr 269 struct ibv_dereg_mr {
> src/verbs.c ibv_dereg_mr 167 int ibv_dereg_mr(struct ibv_mr *mr)
> ********* Problem with ibv_create_comp_channel **********
> include/infiniband/kern-abi.h ibv_create_comp_channel 276 struct 
> ibv_create_comp_channel {
> src/verbs.c ibv_create_comp_channel 190 struct ibv_comp_channel 
> *ibv_create_comp_channel(struct ibv_context *context)
> ********* Problem with ibv_create_cq **********
> include/infiniband/kern-abi.h ibv_create_cq 287 struct ibv_create_cq {
> src/verbs.c ibv_create_cq 232 struct ibv_cq *ibv_create_cq(struct 
ibv_context 
> *context, int cqe, void *cq_context,
> ********* Problem with ibv_resize_cq **********
> include/infiniband/kern-abi.h ibv_resize_cq 346 struct ibv_resize_cq {
> src/verbs.c ibv_resize_cq 250 int ibv_resize_cq(struct ibv_cq *cq, int 
cqe)
> ********* Problem with ibv_destroy_cq **********
> include/infiniband/kern-abi.h ibv_destroy_cq 360 struct ibv_destroy_cq {
> src/verbs.c ibv_destroy_cq 258 int ibv_destroy_cq(struct ibv_cq *cq)
> ********* Problem with ibv_poll_cq **********
> include/infiniband/kern-abi.h ibv_poll_cq 323 struct ibv_poll_cq {
> include/infiniband/verbs.h ibv_poll_cq 831 static inline int 
> ibv_poll_cq(struct ibv_cq *cq, int num_entries, struct ibv_wc *wc)
> ********* Problem with ibv_req_notify_cq **********
> include/infiniband/kern-abi.h ibv_req_notify_cq 338 struct 
ibv_req_notify_cq {
> include/infiniband/verbs.h ibv_req_notify_cq 845 static inline int 
> ibv_req_notify_cq(struct ibv_cq *cq, int solicited_only)
> ********* Problem with ibv_create_srq **********
> include/infiniband/kern-abi.h ibv_create_srq 696 struct ibv_create_srq {
> src/verbs.c ibv_create_srq 289 struct ibv_srq *ibv_create_srq(struct 
ibv_pd *pd,
> ********* Problem with ibv_modify_srq **********
> include/infiniband/kern-abi.h ibv_modify_srq 716 struct ibv_modify_srq {
> src/verbs.c ibv_modify_srq 310 int ibv_modify_srq(struct ibv_srq *srq,
> ********* Problem with ibv_query_srq **********
> include/infiniband/kern-abi.h ibv_query_srq 727 struct ibv_query_srq {
> src/verbs.c ibv_query_srq 317 int ibv_query_srq(struct ibv_srq *srq, 
struct 
> ibv_srq_attr *srq_attr)
> ********* Problem with ibv_destroy_srq **********
> include/infiniband/kern-abi.h ibv_destroy_srq 744 struct ibv_destroy_srq 
{
> src/verbs.c ibv_destroy_srq 322 int ibv_destroy_srq(struct ibv_srq *srq)
> ********* Problem with ibv_post_srq_recv **********
> include/infiniband/kern-abi.h ibv_post_srq_recv 636 struct 
ibv_post_srq_recv {
> include/infiniband/verbs.h ibv_post_srq_recv 901 static inline int 
> ibv_post_srq_recv(struct ibv_srq *srq,
> ********* Problem with ibv_create_qp **********
> include/infiniband/kern-abi.h ibv_create_qp 432 struct ibv_create_qp {
> src/verbs.c ibv_create_qp 327 struct ibv_qp *ibv_create_qp(struct ibv_pd 
*pd,
> ********* Problem with ibv_modify_qp **********
> include/infiniband/kern-abi.h ibv_modify_qp 524 struct ibv_modify_qp {
> src/verbs.c ibv_modify_qp 364 int ibv_modify_qp(struct ibv_qp *qp, 
struct 
> ibv_qp_attr *attr,
> ********* Problem with ibv_query_qp **********
> include/infiniband/kern-abi.h ibv_query_qp 480 struct ibv_query_qp {
> src/verbs.c ibv_query_qp 348 int ibv_query_qp(struct ibv_qp *qp, struct 
> ibv_qp_attr *attr,
> ********* Problem with ibv_destroy_qp **********
> include/infiniband/kern-abi.h ibv_destroy_qp 557 struct ibv_destroy_qp {
> src/verbs.c ibv_destroy_qp 379 int ibv_destroy_qp(struct ibv_qp *qp)
> ********* Problem with ibv_post_send **********
> include/infiniband/kern-abi.h ibv_post_send 598 struct ibv_post_send {
> include/infiniband/verbs.h ibv_post_send 943 static inline int 
> ibv_post_send(struct ibv_qp *qp, struct ibv_send_wr *wr,
> ********* Problem with ibv_post_recv **********
> include/infiniband/kern-abi.h ibv_post_recv 620 struct ibv_post_recv {
> include/infiniband/verbs.h ibv_post_recv 952 static inline int 
> ibv_post_recv(struct ibv_qp *qp, struct ibv_recv_wr *wr,
> ********* Problem with ibv_create_ah **********
> include/infiniband/kern-abi.h ibv_create_ah 652 struct ibv_create_ah {
> src/verbs.c ibv_create_ah 384 struct ibv_ah *ibv_create_ah(struct ibv_pd 
*pd, 
> struct ibv_ah_attr *attr)
> ********* Problem with ibv_destroy_ah **********
> include/infiniband/kern-abi.h ibv_destroy_ah 667 struct ibv_destroy_ah {
> src/verbs.c ibv_destroy_ah 452 int ibv_destroy_ah(struct ibv_ah *ah)
> ********* Problem with ibv_attach_mcast **********
> include/infiniband/kern-abi.h ibv_attach_mcast 674 struct 
ibv_attach_mcast {
> src/verbs.c ibv_attach_mcast 457 int ibv_attach_mcast(struct ibv_qp *qp, 
union
> ibv_gid *gid, uint16_t lid)
> ********* Problem with ibv_detach_mcast **********
> include/infiniband/kern-abi.h ibv_detach_mcast 685 struct 
ibv_detach_mcast {
> src/verbs.c ibv_detach_mcast 462 int ibv_detach_mcast(struct ibv_qp *qp, 
union
> ibv_gid *gid, uint16_t lid)
> 
> That was the reason I wrote separate routines and data structures so 
that
> #defines don't do wrong things. Does that sound reasonable ?
> 
> B.
> -----
> 
> - "Maybe the best approach is to use rdmav_ to replace ibv_" :
> 
> OK, that sounds consistent. Is "rdv_" better than the longer "rdmav_" ?
> 
> C.
> --------
> 
> - "Path records are IB specific.  Not sure we need to rename them" and 
"These
>    changes look fine.  We just need to decide if we want to change 
everything 
>    that's ibv_* to rdma_*, or keep IB specific names (path records, 
GIDs, PKeys, 
>    etc.) the same."
> 
> I had indicated this in my "Information notes" in the [PATCH 0/6] :
>  "IB specific routines are also converted to use RDMA generic API's for 
sake
>  of uniformness (knowing that transport dependent names will be removed
>    once all apps are converted)."
> 
> The issue is between deciding to have either rd(ma)_v or ibv_ for IB 
specific
> structures. Currently there is no other transport other than IB that has 
these
> specific structures, but if that changes it might be better to keep the 
name
> transport agnostic. Another reason that I see at this time is to have 
uniform
> names which means that this library exports names using one prefix - 
this means
> that I do not have to care about the underlying transport type and I 
also do not
> have to remember that ibv_ is for [a, b, c, d] operations and rdma_ is 
for [e, f, g, h]
> operations. What do you feel ?
> 
> D.
> --------
> > > We named ourselves OpenFabrics instead of OpenRDMA for a reason

> > Wasn't OpenRDMA already taken?
> 
> Correct. Also "RDMA" name is a generic enough name and has nothing to do 
with
> openRDMA. Roland had rightly pointed out in my first attempt that usage 
of "#ifdef
> OPENRDMA_xxx" was inappropirate, so I corrected that in this patchset (I 
had
> changed OPENIB to OPENRDMA blindly :)).
> 
> Thanks,
> 
> - KK