Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
Hi Sean,
I still have a problem. Many enums have both IB specific and RDMA generic
defines. Eg :
enum ibv_event_type {
IBV_EVENT_CQ_ERR,
IBV_EVENT_QP_FATAL,
IBV_EVENT_QP_REQ_ERR,
IBV_EVENT_QP_ACCESS_ERR,
IBV_EVENT_COMM_EST,
IBV_EVENT_SQ_DRAINED,
IBV_EVENT_PATH_MIG,
IBV_EVENT_PATH_MIG_ERR,
IBV_EVENT_DEVICE_FATAL,
IBV_EVENT_PORT_ACTIVE,
IBV_EVENT_PORT_ERR,
IBV_EVENT_LID_CHANGE,
IBV_EVENT_PKEY_CHANGE,
IBV_EVENT_SM_CHANGE,
IBV_EVENT_SRQ_ERR,
IBV_EVENT_SRQ_LIMIT_REACHED,
IBV_EVENT_QP_LAST_WQE_REACHED,
IBV_EVENT_CLIENT_REREGISTER
};
It looks weird to keep some elements in the above enum as IBV_ and others
as RDMA_. A lot of other enums have this same problem, eg
ibv_qp_attr_mask,
infact this is true of all enums/structures/defines that have some
elements that
are IB specific.
Thanks,
- KK
Sean Hefty <[EMAIL PROTECTED]> wrote on 07/31/2006 10:47:20 PM:
> > - "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 ?
>
> If an application is looking at a path record, GID, PKey, etc. they they
_are_
> caring about the underlying transport type and the fact that it is IB.
An
> application that wants to be transport neutral would just need to limit
itself
> to using rdma_* structures and APIs.
>
> If we take a larger view, I don't think we want transport neutral names
for the
> IB CM and IB MAD userspace APIs and structures. Things like path
records, GIDs,
> etc. are also used by those libraries.
>
> - Sean
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
> - "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 ? If an application is looking at a path record, GID, PKey, etc. they they _are_ caring about the underlying transport type and the fact that it is IB. An application that wants to be transport neutral would just need to limit itself to using rdma_* structures and APIs. If we take a larger view, I don't think we want transport neutral names for the IB CM and IB MAD userspace APIs and structures. Things like path records, GIDs, etc. are also used by those libraries. - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
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 -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/ver
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
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 -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_q
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
you're right...its friday... ;-)
On Fri, 2006-07-28 at 13:21 -0700, Roland Dreier wrote:
> Steve> Uh, won't that break binary compatibility? Or is that not
> Steve> a requirement? IE: If you make the ibv_* names #defines,
> Steve> then all apps will have to recompile to use the new lib.
> Steve> If you leave them as the real names, then apps can still
> Steve> link to the new lib and run ok...i think...
>
> I don't think the names of structures matters for ABI compatibility --
> if I have a header with
>
> struct foo {
> int a;
> int b;
> };
>
> void blah(struct foo *z);
>
> then a binary compiled against that will still work even if I later
> change the name of struct foo so I have:
>
> stuct bar {
> int c;
> int d;
> };
>
> void blah(struct bar *z);
>
> after all the layout of struct foo and struct bar are the same.
>
> - R.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
Steve> Uh, won't that break binary compatibility? Or is that not
Steve> a requirement? IE: If you make the ibv_* names #defines,
Steve> then all apps will have to recompile to use the new lib.
Steve> If you leave them as the real names, then apps can still
Steve> link to the new lib and run ok...i think...
I don't think the names of structures matters for ABI compatibility --
if I have a header with
struct foo {
int a;
int b;
};
void blah(struct foo *z);
then a binary compiled against that will still work even if I later
change the name of struct foo so I have:
stuct bar {
int c;
int d;
};
void blah(struct bar *z);
after all the layout of struct foo and struct bar are the same.
- R.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
On Fri, 2006-07-28 at 12:27 -0700, Roland Dreier wrote: > > 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. > > - R. Uh, won't that break binary compatibility? Or is that not a requirement? IE: If you make the ibv_* names #defines, then all apps will have to recompile to use the new lib. If you leave them as the real names, then apps can still link to the new lib and run ok...i think... ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
> I realize why you did this, but adding the 'v' to these two calls makes them > inconsistent with all the other calls in the API. I don't know what the > right > approach is here, but it may be better to have the RDMA CM use different > names. Maybe the best approach is to use rdmav_ to replace ibv_. - R. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
> 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. - R. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
Krishna Kumar wrote: > +/** > + * rdmav_create_qp - Create a queue pair. > + */ > +struct rdma_qp *rdmav_create_qp(struct rdma_pd *pd, > +struct rdma_qp_init_attr *qp_init_attr); > + > +/** > + * rdmav_destroy_qp - Destroy a queue pair. > + */ > +int rdmav_destroy_qp(struct rdma_qp *qp); I realize why you did this, but adding the 'v' to these two calls makes them inconsistent with all the other calls in the API. I don't know what the right approach is here, but it may be better to have the RDMA CM use different names. - Sean ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] libibverbs include files changes.
Krishna Kumar wrote:
> +struct rdma_kern_async_event {
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.
- Sean
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
