RE: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
There are some other input structure changes such as ibv_qp_init_attr, if the
qp_type is not IBV_QPT_XRC,
the field xrc_domain is not touched, right ?
Similar thing for struct ibv_send_wr xrc_remote_srq_num field.
--CQ Tang
-Original Message-
From: Jack Morgenstein [mailto:[EMAIL PROTECTED]
Sent: Wednesday, December 05, 2007 12:34 PM
To: ewg@lists.openfabrics.org
Cc: Roland Dreier; Tang, Changqing;
[EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is
available
On Wednesday 05 December 2007 07:24, Roland Dreier wrote:
I think the only alternative we have to preserve backwards
compatibility is to leave struct ibv_context_ops alone and
change the
structure to:
struct ibv_context {
struct ibv_device *device;
struct ibv_context_ops ops;
int cmd_fd;
int async_fd;
int num_comp_vectors;
pthread_mutex_t mutex;
void *abi_compat;
struct ibv_xrc_op *xrc_ops;
};
with xrc_ops added at the end. It's my fault for not
making the ops
member a pointer I guess.
We don't need to have this as a pointer, really (I'd like to
save the extra malloc and associated bookkeeping). If we have
the ibv_xrc_op struct at the end of ibv_context, this is
sufficient for backwards binary
compatibility(libmlx4 itself allocates the ibv_context
structure for libibverbs. If the actual structure is a bit
bigger, who cares -- we just need to preserve the current
offsets of the structure fields for binary compatibility).
If you want to be a bit more generic, we could do this as an
extra_ops
structure and add new ops as needed.
(If future changes are messier than just adding a new op, we
can then increment the API version):
struct ibv_context_extra_ops {
struct ibv_srq *(*create_xrc_srq)(struct ibv_pd *pd,
struct
ibv_xrc_domain *xrc_domain,
struct
ibv_cq *xrc_cq,
struct
ibv_srq_init_attr *srq_init_attr);
struct ibv_xrc_domain * (*open_xrc_domain)(struct
ibv_context *context,
int fd, int oflag);
int (*close_xrc_domain)(struct
ibv_xrc_domain *d);
};
struct ibv_context {
struct ibv_device *device;
struct ibv_context_ops ops;
int cmd_fd;
int async_fd;
int num_comp_vectors;
pthread_mutex_t mutex;
void *abi_compat;
struct ibv_context_extra_ops extra_ops; };
___
ewg mailing list
ewg@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
On Wednesday 05 December 2007 07:24, Roland Dreier wrote:
I think the only alternative we have to preserve backwards
compatibility is to leave struct ibv_context_ops alone and change the
structure to:
struct ibv_context {
struct ibv_device *device;
struct ibv_context_ops ops;
int cmd_fd;
int async_fd;
int num_comp_vectors;
pthread_mutex_t mutex;
void *abi_compat;
struct ibv_xrc_op *xrc_ops;
};
with xrc_ops added at the end. It's my fault for not making the ops
member a pointer I guess.
We don't need to have this as a pointer, really (I'd like to save the
extra malloc and associated bookkeeping). If we have the ibv_xrc_op struct
at the end of ibv_context, this is sufficient for backwards binary
compatibility(libmlx4 itself allocates the ibv_context structure for
libibverbs. If the actual structure is a bit bigger, who cares --
we just need to preserve the current offsets of the structure
fields for binary compatibility).
If you want to be a bit more generic, we could do this as an extra_ops
structure and add new ops as needed.
(If future changes are messier than just adding a new op, we can then
increment the API version):
struct ibv_context_extra_ops {
struct ibv_srq *(*create_xrc_srq)(struct ibv_pd *pd,
struct ibv_xrc_domain
*xrc_domain,
struct ibv_cq *xrc_cq,
struct ibv_srq_init_attr
*srq_init_attr);
struct ibv_xrc_domain * (*open_xrc_domain)(struct ibv_context *context,
int fd, int oflag);
int (*close_xrc_domain)(struct ibv_xrc_domain *d);
};
struct ibv_context {
struct ibv_device *device;
struct ibv_context_ops ops;
int cmd_fd;
int async_fd;
int num_comp_vectors;
pthread_mutex_t mutex;
void *abi_compat;
struct ibv_context_extra_ops extra_ops;
};
___
ewg mailing list
ewg@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
struct ibv_context {
struct ibv_device *device;
struct ibv_context_ops ops;
int cmd_fd;
int async_fd;
int num_comp_vectors;
pthread_mutex_t mutex;
void *abi_compat;
struct ibv_xrc_op *xrc_ops;
};
We don't need to have this as a pointer, really (I'd like to save the
extra malloc and associated bookkeeping).
I think we could actually have libmlx4 have one copy of xrc_ops and
set the pointer to point at its copy. And then the tests in each of
the xrc operations become just 'if (!context-xrc_ops) return ENOSYS;
But it's not a big deal really.
If we have the ibv_xrc_op struct at the end of ibv_context, this is
sufficient for backwards binary compatibility(libmlx4 itself
allocates the ibv_context structure for libibverbs. If the actual
structure is a bit bigger, who cares -- we just need to preserve
the current offsets of the structure fields for binary
compatibility).
Yes, that's true. I don't have much objection to adding a struct
ibv_xrc_ops inside the structure (rather than the pointer as I
suggested).
If you want to be a bit more generic, we could do this as an extra_ops
structure and add new ops as needed.
Actually I'd prefer to add xrc_ops and then if we need to extend
further with more new ops, add another structure afterw it. That way
we avoid having to put any define in libibverbs to tell drivers like
libmlx4 that xrc support is present; libmlx4 et al can just use
AC_CHECK_MEMBER(struct ibv_context.xrc_ops) to test with autoconf.
- R.
___
ewg mailing list
ewg@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
On Wednesday 05 December 2007 02:40, Roland Dreier wrote: BTW, sifting through the OFED 1.3 libibverbs tree, I do see that the commit to add max_xrc_domains to struct ibv_device_attr did break things by adding the member in the middle of the structure (so that an app compiled against the old header will see bogus values for local_ca_ack_delay and phys_port_count. Actually looking at the commit again, it's worse than that... anything compiled against the old header that calls ibv_query_device() may get memory corrupted, because the new ibv_query_device() writes to a bigger structure than the app passes in. The perils of not reviewing properly I guess... This commit was subsequently reversed for exactly that reason. Unfortunately, though, when I reviewed things regarding backwards binary compatibility at the time I reversed the above commit, I also missed the problem of the ibv_context_ops structure. - Jack ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
On Wednesday 05 December 2007 21:59, Tang, Changqing wrote: There are some other input structure changes such as ibv_qp_init_attr, if the qp_type is not IBV_QPT_XRC, the field xrc_domain is not touched, right ? Right. Similar thing for struct ibv_send_wr xrc_remote_srq_num field. Same thing -- the fields are not touched unless the qp type is IBV_QPT_XRC. - Jack ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
BTW, sifting through the OFED 1.3 libibverbs tree, I do see that the commit to add max_xrc_domains to struct ibv_device_attr did break things by adding the member in the middle of the structure (so that an app compiled against the old header will see bogus values for local_ca_ack_delay and phys_port_count. Actually looking at the commit again, it's worse than that... anything compiled against the old header that calls ibv_query_device() may get memory corrupted, because the new ibv_query_device() writes to a bigger structure than the app passes in. The perils of not reviewing properly I guess... - R. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] Re: [ofa-general] OFED 1.3 Beta release is available
oops, sorry... I see that the very next OFED 1.3 commit reverted that change, so things aren't as bad as I thought. Never mind. - R. ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
Re: [ewg] RE: [ofa-general] OFED 1.3 Beta release is available
Moshe Kazir wrote: Are backport patches for SLES9 SP4 and RHEL 5.1 included in this version ? As you can see here: RHEL5.1 - yes SLES9 (any SP) - No OS support: Novell: - SLES10 - SLES10 SP1 and up1 Redhat: - Redhat EL4 up4 and up5 - Redhat EL5 and up1 kernel.org: - 2.6.23 and 2.6.24-rc2 Systems: * x86_64 * x86 * ia64 * ppc64* ___ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
