Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
On 1/2/2012 7:34 PM, Hefty, Sean wrote: this field is a -- union -- how would that work if more than one extension is to be applied for a structure? The fields at the end of the structure should only be accessed if the structure is of the correct type. In this case, ext.xrc_recv is only available if the qp type is xrc recv. Maybe I wasn't clear enough, I was thinking on a case where one extends a structure and later on a 2nd extension is applied to the same structure, but the functionality / list of use cased related to the 1st extension isn't disjoint with that of the 2nd, e.g the second extends the first... maybe generally we can have a union named ext and on rare cases added ext2, etc? 2. indeed, reality wise, new features, much of the time will also interact with existing data structures... so what happens if we extend a structure but the the extended strucutre is actually a field within another existing structure e.g suppose we want to extend ibv_ah_attr which is within ibv_qp_attr e.g to be used for the RAW Ethernet QPs. I don't see how we can be backward compatible with apps linked against libibverbs with the internal structure size being smaller, correct? so extended fields must be on the END always - in the actual structure they are added and if this structure is a field of another structure then we can't just extend it and need to come up with new structure which is in turn used as the field? New features want to interact with existing structures and functions, which is what makes providing a clean separation difficult. We can extend the structures using the above method as long as we have some sort type field available. Where one is not available, we need to add one. See the proposed struct ibv_srq for an example. The extended SRQ type is only available by calling ibv_create_xsrq(), since ibv_create_srq() cannot know whether the user supports the extended ibv_srq_init_attr or not. Yes, I understand that, in 2nd thought, for the case of extending a structure which sits within another structure, e.g ibv_ah_attr within ibv_qp_attr, maybe we don't have much choice and rather add ib_ah_ext_attr and place it in the end of ibv_qp_attr --- and here's a case for double extensions... as we need the ext union of ibv_qp_attr to contain struct ibv_ah_ext_attr prim , struct ibv_ah_ext_attr alt and possibly also struct qp_attr_ext qp_ext... ibv_wc and ibv_send_wr are allocated by the caller, so those are more difficult to deal with. I agree that the size of those structures cannot change. It may be possible that some of the features you mentioned could be set as part of the qp attributes (ibv_modify_qp), and for the others, I'm not sure. Run time checks shouldn't be a big deal, since we already have to check things like ibv_wr_opcode and ibv_send_flags anyway. But it could be that we require a new function, similar to ibv_create_xsrq. I'd like to better understand the allocated by the caller ... are more difficult to deal with part of your response - for ibv_send_wr - if the caller have set a new IBV_WR_NEW_FEATURE value for the wr type, they surely aware to the new fields and actually the size of the structure can change as of structs allocated by the library. As for ibv_wc, yep, looks like we can't change the size unless we want to write a copatility layer that also comes into play in fast path calls, specifically ibv_poll_cq and translates from the new ibv_wc to the old ibv_wc structure. Or. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
I'd like to better understand the allocated by the caller ... are more difficult to deal with part of your response - for ibv_send_wr - if the caller have set a new IBV_WR_NEW_FEATURE value for the wr type, they surely aware to the new fields and actually the size of the structure can change as of structs allocated by the library. As for ibv_wc, yep, looks like we can't change the size unless we want to write a copatility layer that also comes into play in fast path calls, specifically ibv_poll_cq and translates from the new ibv_wc to the old ibv_wc structure. You're right. I was thinking more of ibv_wc, which has issues, since it may be used as an array. ibv_send_wr is probably okay, since we walk a list using pointers. - Sean -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
1. for libibverbs some structures extended field is added at their end
saying Following fields only available if device supports extensionse.g
@@ -590,6 +634,13 @@ struct ibv_qp {
pthread_mutex_t mutex;
pthread_cond_t cond;
uint32_tevents_completed;
+
+ /* Following fields only available if device supports
extensions */
+ union {
+ struct {
+ struct ibv_xrcd *xrcd;
+ } xrc_recv;
+ } ext;
};
but this field is a -- union -- how would that work if more than one
extension is to be applied for a structure?
The fields at the end of the structure should only be accessed if the structure
is of the correct type. In this case, ext.xrc_recv is only available if the qp
type is xrc recv.
2. indeed, reality wise, new features, much of the time will also
interact with existing data structures... so what happens if we extend a
structure but the the extended strucutre is actually a field within
another existing structure e.g suppose we want to extend ibv_ah_attr
which is within ibv_qp_attr e.g to be used for the RAW Ethernet QPs. I
don't see how we can be backward compatible with apps linked against
libibverbs with the internal structure size being smaller, correct? so
extended fields must be on the END always - in the actual structure they
are added and if this structure is a field of another structure then we
can't just extend it and need to come up with new structure which is in
turn used as the field?
New features want to interact with existing structures and functions, which is
what makes providing a clean separation difficult. We can extend the
structures using the above method as long as we have some sort type field
available. Where one is not available, we need to add one. See the proposed
struct ibv_srq for an example. The extended SRQ type is only available by
calling ibv_create_xsrq(), since ibv_create_srq() cannot know whether the user
supports the extended ibv_srq_init_attr or not.
3. The usage of #ifdef IBV_NEW_FEATURE_OPS (IBV_XRC_OPS) in libmlx4
will become tireding to follow maybe and could be replaced by placing
dependency on a version of libibverbs that supports IBV_NEW_FEATURE_OPS?
Yes, you could make that replacement, which will work in this case.
4. can we somehow come up with a method to avoid IBV_NEW_FEATURE_OPS for
--every-- new feature?
or call an existing function with some new enum value
This probably depends on the feature.
5. what happens if we just want to enhance an -- existing -- function -
suppose we want to enhance ibv_post_send / ibv_poll_cq to support
features like LSO, checksum offload, masked atomic operations, fast
memory remote invalidate, etc so we add IBV_WR_NEW_FEATURE /
IB_WC_NEW_FEATURE enum values, this step is simple, again if we go the
way of the applicayion ensuring through dependencies that they are
loaded against libibverbs that supports IB_{WR,WC}_NEW_FEATURE.
Now we come up with a need to extend struct ibv_send_wr and struct
ibv_wc - where we should be very careful - walking on glasses for the
backward compatibility thing - so only adding at the end on as union
field which doesn't change the size of the union, correct? for
ibv_post_send we can rely on the provider library to return error if
they don't support NEW_FEATURE, but this would happen in run time... is
there a way to avoid that, should we better go and add
ib_post_send_new_feature? this would be very tedious doing for each
new_feature and not applicable to ibv_poll_cq - any idea what should be
done here?
ibv_wc and ibv_send_wr are allocated by the caller, so those are more difficult
to deal with. I agree that the size of those structures cannot change. It may
be possible that some of the features you mentioned could be set as part of the
qp attributes (ibv_modify_qp), and for the others, I'm not sure. Run time
checks shouldn't be a big deal, since we already have to check things like
ibv_wr_opcode and ibv_send_flags anyway. But it could be that we require a new
function, similar to ibv_create_xsrq.
- Sean
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
On Thu, Dec 29, 2011 at 3:43 PM, Or Gerlitz ogerl...@mellanox.com wrote:
5. what happens if we just want to enhance an -- existing -- function -
suppose we want to enhance ibv_post_send / ibv_poll_cq to support features
like LSO, checksum offload, masked atomic operations, fast memory remote
invalidate, etc so we add IBV_WR_NEW_FEATURE / IB_WC_NEW_FEATURE enum
values, this step is simple, again if we go the way of the applicayion
ensuring through dependencies that they are loaded against libibverbs that
supports IB_{WR,WC}_NEW_FEATURE.
Not only for the ABI between the Linux kernel and Linux user space but
also for Linux shared libraries it is required that the ABI of a new
version is backwards compatible with previous versions. So any
functionality that can't be added in a backwards compatible way to an
existing function should be added as a new system call, sysfs file or
shared library function - whatever is appropriate.
Bart.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
or call an existing function with some new enum value
5. what happens if we just want to enhance an -- existing -- function
- suppose we want to enhance ibv_post_send / ibv_poll_cq to support
features like LSO, checksum offload, masked atomic operations, fast
memory remote invalidate, etc so we add IBV_WR_NEW_FEATURE /
IB_WC_NEW_FEATURE enum values, this step is simple, again if we go the
way of the applicayion ensuring through dependencies that they are
loaded against libibverbs that supports IB_{WR,WC}_NEW_FEATURE.
Another element to take into account / use here, is the device
capabilities, in the kernel RDMA stack this is the way to go for
supporting extended features - those who are not common to all devices,
e.g the way ipoib queries the device and concludes if checksum/lso
offloads can be exposed to the network stack, or the rpc/rdma layer
decides what memory management model to use for rdma conditioned if
memory extensions etc are supported. But the kernel is one piece, so if
a cap is defined and there's a driver down there advertizing it, all is
in place software wise, where in user space, there are some more layers
to address, as this thread attempts to do.
Or.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
On Tuesday 02 August 2011 08:38, Jason Gunthorpe wrote: The hope is once the infrastructure is in libibverbs there will not be as much need to change libibverbs, just the apps and the drivers. If I understand correctly, the various additions which would normally be made to libibverbs would then be made by third-party libraries which extend libibverbs to support their additions. These additions would include the new ibv_cmd_xxx functions (the core functions reside in src/cmd.c), and new, additional, enum values of the form IBV_USER_VERBS_CMD_, of which the core enum is in file include/infiniband/kern_abi.h. The modified apps would then include the header files of the 3rd party additions after the libibverbs headers when compiling. Each new third-party package would need such a library. While this will lead to a multiplicity of new libraries (one per addition), the core libibverbs package would remain as is. Am I correct? If so, shouldn't the current XRC userspace implementation do the same (and take the XRC-specific additions out of libibverbs and put them into a separate library)? Note that coordination between third parties would still be required to insure that there is no collision of enum values between the various packages. -Jack -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
If I understand correctly, the various additions which would normally be made
to libibverbs
would then be made by third-party libraries which extend libibverbs to support
their additions.
It may help to read about extensions in opengl:
http://www.opengl.org/registry/doc/rules.html
Additions can be made to both. Obviously Roland has the final say on any
changes to ibverbs, but what I envision is:
If a feature is based on an industry standard and all necessary kernel changes
are upstream, then the feature should be integrated into ibverbs. An
application would simply call ibv_new_feature() to make use of the feature (or
call an existing function with some new enum value). Internally, ibverbs may
need to obtain a new interface from the provider library.
If there is no published specification for a feature (maybe it's still under
development), kernel patches are needed, and there are customers who want to
use the feature immediately, then a vendor can define an extension. In this
case, the application may call vendor_ops = ibv_get_ext_ops(), followed by
vendor_ops-new_feature(). Or the app may call ibv_some_existing_function()
using a vendor specific enum value.
These additions would include the new ibv_cmd_xxx functions
(the core functions reside in src/cmd.c), and new, additional, enum values of
the form IBV_USER_VERBS_CMD_, of which the core enum is in file
include/infiniband/kern_abi.h.
If an app uses an extension, then there are no changes to ibverbs. The
provider library either needs to use an existing ibv_cmd_* call or call into
the kernel itself. If the provider needs a new command, it could declare it as:
enum {
MLX4_USER_VERBS_CMD_BASE = IBV_EXTENSION_VENDOR
IBV_EXTENSION_BASE_SHIFT,
MLX4_USER_VERBS_CMD_NEW_FEATURE ...
};
This requires a kernel patch to uverbs maintained by the vendor. Note that
this means that the vendor can continue to support their version of a feature
(with continued kernel patches) even once the feature is merged into ibverbs.
Each new third-party package would need such a library.
While this will lead to a multiplicity of new libraries (one per addition),
the core libibverbs package would remain as is.
I didn't follow this. The new feature could be integrated directly into the
provider library (e.g. mlx4).
Am I correct? If so, shouldn't the current XRC userspace implementation do
the same (and
take the XRC-specific additions out of libibverbs and put them into a separate
library)?
XRC could be added as an extension, but since it's based on a published
specification, IMO it makes more sense being integrated directly into ibverbs
with the necessary kernel changes pushed upstream. Extensions are more
difficult for apps to use than an integrated feature.
Note that coordination between third parties would still be required to insure
that there is
no collision of enum values between the various packages.
The use of enum ibv_extension_type should prevent collisions. It may be that
different vendors use the same value for different objects, but that doesn't
result in a collision. The scope of a vendor specific value is per device.
- Sean
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
On Tue, Aug 02, 2011 at 10:53:05AM +0300, Jack Morgenstein wrote: These additions would include the new ibv_cmd_xxx functions (the core functions reside in src/cmd.c), and new, additional, enum values of the form IBV_USER_VERBS_CMD_, of which the core enum is in file include/infiniband/kern_abi.h. I believe the only case where using an extension would make sense if it if can be implemented entirely within the low level driver. So you can't add new ibv_cmd calls to ibverbs, must duplicate them in your driver, etc. I'm a little unclear on how the application is going to get the access enums and other structure definitions, though.. I suppose the low level driver can install a .h file as well. Each new third-party package would need such a library. While this will lead to a multiplicity of new libraries (one per addition), the core libibverbs package would remain as is. I definitely don't want to see this.. Jason -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
On Thursday 16 June 2011 22:27, Hefty, Sean wrote: Basically, I wasn't referring to such new verbs as vendor extensions, but rather as new verbs we want to add at this and/or future points of time which didn't exist at the time the IB stack and specifically its kernel/user ABIs/APIs were written (couple of years ago...). To be clear, there are 2 sides to ibverbs - the app side, and the provider library. On the app side, new functionality would be added directly to libibverbs. I would reuse what's there if possible, and provide direct API calls where needed. For example, the xrc patch adds: ibv_create_xsrq() ibv_open_xrcd() ibv_close_xrcd() as new APIs. On the provider side, the necessary calls are obtained by ibverbs calling get_ext_ops(). I haven't come up with another way of extended verbs that would be as easy for an application to use, given that most of the calls and data structures are reusable. - Sean Hi Sean, I'm not sure about what this mechanism saves us over bumping the ABI numbers. Actually, I think I do see a problem here, under the following situation: - All libraries (app, libibverbs, and libmlx4) support extensions. - A new verb (say extension ib_new_verb was added as an extension to the app, to libibverbs, and to libmlx4 and everything was compiled. - The APP is built on the full configuration, but is run on a configuration which has the verb extension added to libmlx4, but NOT to libibverbs (new libibverbs was installed originally, so that libmlx4 would succeed in the install, then somehow libibverbs was rolled back to before ib_new_verb was added). When the APP tries to run, it calls: ibv_get_device_ext_ops(struct ibv_device *device, ib_new_verb); This call will succeed (in the current implementation). However, the verb helper function (ibv_cmd_new_verb) is not present in libibverbs, so things will crash (if, indeed, libmlx4 can be loaded at all - In fact, I'm not sure if it will fail loading because of unresolved references). Indeed, I am not sure that the app can run at all due to unresolved references. The problem here is that the new additions need support in libibverbs in order to work (this is not simply a pass-through by libibverbs to the lower layer). I may be wrong about all this -- userspace is not really my expertise. If I am not wrong, what, then, is the advantage of this methodology over simply bumping the ABI numbers? -Jack -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
In fact, I'm not sure if it will fail loading because of unresolved references). Indeed, I am not sure that the app can run at all due to unresolved references. I'm not sure what exactly we can do in the situation you described. I would expect the app to see an unresolved reference. I may be wrong about all this -- userspace is not really my expertise. If I am not wrong, what, then, is the advantage of this methodology over simply bumping the ABI numbers? I'm not sure I fully understand your question. In an ideal case, any new feature would be added to the upstream ibverbs. The extensions would be used to support some new feature that ibverbs is not aware of. For example, a vendor could expose APIs that offload MPI collectives, which may still be under development. - Sean
Re: [PATCH 1/2] libibverbs: Allow 3rd party extensions to verb routines
In fact, I'm not sure if it will fail loading because of unresolved references). Indeed, I am not sure that the app can run at all due to unresolved references. The dlopen of mlx4 should fail due to unresolved references, so the net effect will be that no apps cannot open the RDMA device in this situation - so it is an invalid system configuration that is properly detected by the runtime. The problem here is that the new additions need support in libibverbs in order to work (this is not simply a pass-through by libibverbs to the lower layer). The hope is once the infrastructure is in libibverbs there will not be as much need to change libibverbs, just the apps and the drivers. Jason -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
