UCT/PLUGIN: ib plugin for token query#11277
UCT/PLUGIN: ib plugin for token query#11277jeynmann wants to merge 10 commits intoopenucx:masterfrom
Conversation
| } | ||
|
|
||
| ucs_status_t __attribute__((weak)) | ||
| uct_ib_plugin_ep_query(uct_ep_h ep, uct_ep_attr_t *ep_attr) |
There was a problem hiding this comment.
Will it be possible to change this to:
uct_ib_plugin_query_qp()
?
src/uct/ib/mlx5/Makefile.am
Outdated
| dv/ib_mlx5_dv.c \ | ||
| dv/ib_mlx5dv_md.c | ||
| dv/ib_mlx5dv_md.c \ | ||
| ../plugin/uct_ib_plugin_stubs.c |
There was a problem hiding this comment.
Add a Makefile.am in /plugin and the stub there
(we should also add the .h file to the noinst_HEADERS list)
src/uct/ib/Makefile.am
Outdated
| pluginincludedir = $(includedir)/uct/ib/plugin | ||
| plugininclude_HEADERS = plugin/uct_ib_plugin.h |
There was a problem hiding this comment.
should be added in plugin/Makefile.am
| uct_ib_plugin_qp_query(const uct_ib_plugin_qp_query_params_t *params, | ||
| uct_ib_plugin_qp_query_attr_t *attr) | ||
| { |
There was a problem hiding this comment.
Where is it going to be called from?
There was a problem hiding this comment.
It will be invoked by uct_iface_query_v2 and uct_ep_query.
The UCT implementation will be submitted in a separate PR.
src/uct/ib/plugin/uct_ib_plugin.h
Outdated
| * @brief QP query input parameters. | ||
| */ | ||
| typedef struct uct_ib_plugin_qp_query_params { | ||
| uint64_t field_mask; |
There was a problem hiding this comment.
should also define an enum for the field_mask
src/uct/api/uct.h
Outdated
| /* IB plugin capability */ | ||
| #define UCT_IFACE_FLAG_IB_PLUGIN_QUERY_TOKEN UCS_BIT(47) /**< IB plugin for token query is available */ |
There was a problem hiding this comment.
need to getrid of "IB" and "plugin" in UCT API, just "UCT_IFACE_FLAG_QUERY_TOKEN", not implemented/supported => no capability
| /** | ||
| * @brief Field mask bits for @ref uct_ib_plugin_qp_query_attr_t. | ||
| * Selects which output fields to populate. | ||
| */ | ||
| enum uct_ib_plugin_qp_query_attr_field { | ||
| UCT_IB_PLUGIN_QP_QUERY_ATTR_FIELD_TX_TOKEN_LEN = UCS_BIT(0), | ||
| UCT_IB_PLUGIN_QP_QUERY_ATTR_FIELD_RX_TOKEN_LEN = UCS_BIT(1), | ||
| UCT_IB_PLUGIN_QP_QUERY_ATTR_FIELD_TX_TOKEN = UCS_BIT(2), | ||
| UCT_IB_PLUGIN_QP_QUERY_ATTR_FIELD_RX_TOKEN = UCS_BIT(3) | ||
| }; |
There was a problem hiding this comment.
I think we'll have similar bits for uct_ep_query, so why dont pass the same semantic to the "uct_ib_plugin_ep_query", cast to correct ep_type and access QP.
Then uct_ib_plugin_qp_query_params_t becomes redundant.
| /** | ||
| * @brief Return plugin-contributed iface capability flags. | ||
| * | ||
| * @return Bitmask of UCT_IFACE_FLAG_* contributed by the plugin. | ||
| */ | ||
| uint64_t uct_ib_plugin_iface_flags(void); |
There was a problem hiding this comment.
👍 for plugin contribution to defined UCT API
| * @return UCS_OK on success, error code otherwise. | ||
| */ | ||
| ucs_status_t | ||
| uct_ib_plugin_qp_query(const uct_ib_plugin_qp_query_params_t *params, |
There was a problem hiding this comment.
uct_ib_plugin_ep_query?
There was a problem hiding this comment.
@roiedanino suggests renaming it to uct_ib_plugin_query_qp. We now rely on mlx5dv_devx_obj and ibv_qp rather than the internal UCT structure, which might provide a cleaner and simpler approach.
Will it be possible to change this to:
uct_ib_plugin_query_qp()?
There was a problem hiding this comment.
IMO it adds inconsistency and overcomplication. Why does iface_flags return native UCT API level flags, but ep_query requires conversion of ep -> qp and qp_attr -> ep_attr converting in base (non-plugin) UCT/IB code?
Signed-off-by: Roie Danino <rdanino@nvidia.com>
| UCT_IB_PLUGIN_QP_QUERY_ATTR_FIELD_TX_TOKEN_LEN = UCS_BIT(0), | ||
| UCT_IB_PLUGIN_QP_QUERY_ATTR_FIELD_RX_TOKEN_LEN = UCS_BIT(1), |
There was a problem hiding this comment.
in #11234 these are defined as iface attributes
4 participants
What?
Add plugin support for token query(capability, rx token, tx token, tx token lengh, rx token length).
This is the plugin part of PR#11234
How?
Add weak symbols in uct.
The implementation will be provided by plugin lib.