Repository navigation
UCT: outstanding purge implementation #11668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
b50c30f
24f2678
bbf1a4d
e92b34c
14f31b8
a08eb39
0e80e0a
776c898
586b72a
7d20a98
372af2b
572dbd4
1a0e2e6
0574b70
a2188bb
fdb0541
7d36f0c
3974080
fa25afd
0980da9
e4fb91a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,6 +178,44 @@ uct_ib_mlx5_ext_ep_put_sgl_zcopy(uct_ep_h ep, void * const *buffers, | |
| return UCS_ERR_UNSUPPORTED; | ||
| } | ||
|
|
||
| static ucs_status_t uct_ib_mlx5_ext_ep_outstanding_purge_check_param( | ||
| const uct_ep_outstanding_purge_params_t *params) | ||
| { | ||
| if (params == NULL) { | ||
|
gleon99 marked this conversation as resolved.
Resolved
|
||
| return UCS_ERR_INVALID_PARAM; | ||
| } | ||
|
|
||
| if (!(params->field_mask & UCT_EP_OUTSTANDING_FIELD_RX_TOKEN) || | ||
| !(params->field_mask & UCT_EP_OUTSTANDING_FIELD_CB) || | ||
| (params->rx_token == NULL) || (params->cb == NULL)) { | ||
| return UCS_ERR_INVALID_PARAM; | ||
| } | ||
|
|
||
| return UCS_OK; | ||
| } | ||
|
|
||
| ucs_status_t uct_ib_mlx5_ext_ep_outstanding_purge( | ||
| uct_ep_h ep, const uct_ep_outstanding_purge_params_t *params) | ||
| { | ||
| uct_ib_mlx5_ext_plugin_t *plugin; | ||
|
|
||
| if (ucs_unlikely(uct_ib_mlx5_ext_ep_outstanding_purge_check_param(params) != | ||
| UCS_OK)) { | ||
| return UCS_ERR_INVALID_PARAM; | ||
| } | ||
|
gleon99 marked this conversation as resolved.
Resolved
|
||
|
|
||
| ucs_list_for_each(plugin, &uct_ib_mlx5_ext_plugins, list) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is an open question, see my comment #11668 (comment) |
||
| if (ucs_unlikely(uct_ib_mlx5_ext_is_unsupported_op( | ||
| (const void*)plugin->ops.ep_outstanding_purge))) { | ||
| continue; | ||
| } | ||
|
|
||
| return plugin->ops.ep_outstanding_purge(ep, params); | ||
| } | ||
|
evgeny-leksikov marked this conversation as resolved.
Resolved
|
||
|
|
||
| return UCS_ERR_UNSUPPORTED; | ||
| } | ||
|
|
||
| void uct_ib_mlx5_ext_cleanup(void) | ||
| { | ||
| uct_ib_mlx5_ext_plugin_t *plugin, *tmp; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| #endif | ||
|
|
||
| #include <uct/ib/mlx5/ib_mlx5_log.h> | ||
| #include <uct/ib/mlx5/ib_mlx5_ext.h> | ||
| #include <ucs/vfs/base/vfs_cb.h> | ||
| #include <ucs/vfs/base/vfs_obj.h> | ||
| #include <ucs/arch/cpu.h> | ||
|
|
@@ -24,6 +25,33 @@ | |
| #include "rc_mlx5.inl" | ||
|
|
||
|
|
||
| ucs_status_t uct_rc_mlx5_base_ep_query(uct_ep_h tl_ep, uct_ep_attr_t *ep_attr) | ||
| { | ||
| uct_ib_mlx5_ext_ep_query_attr_t attr = {}; | ||
| const uint64_t ext_mask = UCT_EP_ATTR_FIELD_TX_TOKEN; | ||
| ucs_status_t status; | ||
|
|
||
| if (ep_attr->field_mask & (UCT_EP_ATTR_FIELD_LOCAL_SOCKADDR | | ||
| UCT_EP_ATTR_FIELD_REMOTE_SOCKADDR)) { | ||
| return UCS_ERR_UNSUPPORTED; | ||
| } | ||
|
|
||
| if (ep_attr->field_mask & ext_mask) { | ||
| if (ep_attr->field_mask & UCT_EP_ATTR_FIELD_TX_TOKEN) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why to check twice? it seems like leftover |
||
| attr.field_mask |= UCT_IB_MLX5_EXT_EP_QUERY_ATTR_FIELD_TX_TOKEN; | ||
| attr.tx_token = ep_attr->tx_token; | ||
| } | ||
|
|
||
| status = uct_ib_mlx5_ext_ep_query(tl_ep, &attr); | ||
| if (status != UCS_OK) { | ||
| return status; | ||
| } | ||
| } | ||
|
|
||
| return UCS_OK; | ||
| } | ||
|
|
||
|
|
||
| static ucs_status_t UCS_F_ALWAYS_INLINE uct_rc_mlx5_base_ep_put_short_inline( | ||
| uct_ep_h tl_ep, const void *buffer, unsigned length, | ||
| uint64_t remote_addr, uct_rkey_t rkey) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1015,14 +1015,72 @@ static ucs_status_t | |
| uct_rc_mlx5_iface_query_v2(uct_iface_h UCS_V_UNUSED iface, | ||
|
gleon99 marked this conversation as resolved.
Resolved
|
||
| uct_iface_attr_v2_t *iface_attr) | ||
| { | ||
| size_t max_sgl = uct_ib_mlx5_ext_max_put_sgl_zcopy_count(); | ||
| uct_ib_mlx5_ext_iface_query_attr_t ext_attr = {}; | ||
| const uint64_t sgl_mask = UCT_IFACE_ATTR_FIELD_CAP_FLAGS | | ||
| UCT_IFACE_ATTR_FIELD_MAX_PUT_SGL_ZCOPY_COUNT; | ||
| size_t max_sgl; | ||
| ucs_status_t status; | ||
|
|
||
| status = uct_iface_base_query_v2(iface, iface_attr); | ||
| if (status != UCS_OK) { | ||
| return status; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & sgl_mask) { | ||
| max_sgl = uct_ib_mlx5_ext_max_put_sgl_zcopy_count(); | ||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_CAP_FLAGS) { | ||
| iface_attr->cap.flags |= (max_sgl > 0) ? | ||
| UCT_IFACE_FLAG_V2_PUT_SGL_ZCOPY : | ||
| 0; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & | ||
| UCT_IFACE_ATTR_FIELD_MAX_PUT_SGL_ZCOPY_COUNT) { | ||
| iface_attr->max_put_sgl_zcopy_count = max_sgl; | ||
| } | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_CAP_FLAGS) { | ||
| iface_attr->cap.flags = (max_sgl > 0) ? UCT_IFACE_FLAG_V2_PUT_SGL_ZCOPY : 0; | ||
| ext_attr.field_mask |= UCT_IB_MLX5_EXT_IFACE_QUERY_ATTR_FIELD_CAP_FLAGS; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_TX_TOKEN_LENGTH) { | ||
| ext_attr.field_mask |= | ||
| UCT_IB_MLX5_EXT_IFACE_QUERY_ATTR_FIELD_TX_TOKEN_LEN; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_RX_TOKEN_LENGTH) { | ||
| ext_attr.field_mask |= | ||
| UCT_IB_MLX5_EXT_IFACE_QUERY_ATTR_FIELD_RX_TOKEN_LEN; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_MAX_PUT_SGL_ZCOPY_COUNT) { | ||
| iface_attr->max_put_sgl_zcopy_count = max_sgl; | ||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_TX_TOKEN) { | ||
| ext_attr.field_mask |= UCT_IB_MLX5_EXT_IFACE_QUERY_ATTR_FIELD_TX_TOKEN; | ||
| ext_attr.tx_token = (void*)iface_attr->tx_token; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This casts away the const from the public TX-token input. Can we make the plugin attribute const void * instead?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jeynmann unresolved? |
||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_RX_TOKEN) { | ||
| ext_attr.field_mask |= UCT_IB_MLX5_EXT_IFACE_QUERY_ATTR_FIELD_RX_TOKEN; | ||
| ext_attr.rx_token = iface_attr->rx_token; | ||
| } | ||
|
|
||
| if (ext_attr.field_mask != 0) { | ||
| status = uct_ib_mlx5_ext_iface_query(iface, &ext_attr); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without a plugin, requesting token lengths now fails the entire query. Other transports return success with zero lengths and no capability bit..
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jeynmann unresolved?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed |
||
| if (status != UCS_OK) { | ||
| return status; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_CAP_FLAGS) { | ||
| iface_attr->cap.flags |= ext_attr.cap.flags; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_TX_TOKEN_LENGTH) { | ||
| iface_attr->tx_token_length = ext_attr.tx_token_len; | ||
| } | ||
|
|
||
| if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_RX_TOKEN_LENGTH) { | ||
| iface_attr->rx_token_length = ext_attr.rx_token_len; | ||
| } | ||
| } | ||
|
|
||
| return UCS_OK; | ||
|
|
@@ -1034,13 +1092,14 @@ static uct_rc_iface_ops_t uct_rc_mlx5_iface_ops = { | |
| .iface_query_v2 = uct_rc_mlx5_iface_query_v2, | ||
| .iface_estimate_perf = uct_rc_iface_estimate_perf, | ||
| .iface_vfs_refresh = uct_rc_iface_vfs_refresh, | ||
| .ep_query = (uct_ep_query_func_t)ucs_empty_function_return_unsupported, | ||
| .ep_query = uct_rc_mlx5_base_ep_query, | ||
| .ep_invalidate = uct_rc_mlx5_base_ep_invalidate, | ||
| .ep_connect_to_ep_v2 = uct_rc_mlx5_ep_connect_to_ep_v2, | ||
| .iface_is_reachable_v2 = uct_rc_mlx5_iface_is_reachable_v2, | ||
| .ep_is_connected = uct_rc_mlx5_base_ep_is_connected, | ||
| .ep_get_device_ep = (uct_ep_get_device_ep_func_t)ucs_empty_function_return_unsupported, | ||
| .ep_put_sgl_zcopy = uct_ib_mlx5_ext_ep_put_sgl_zcopy | ||
| .ep_put_sgl_zcopy = uct_ib_mlx5_ext_ep_put_sgl_zcopy, | ||
| .ep_outstanding_purge = uct_ib_mlx5_ext_ep_outstanding_purge | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does the caller discover purge support? QUERY_TOKEN can be advertised even when ep_query or purge is missing. Should we add a capability for purge, or require the complete query/derive/purge set?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jeynmann unresolved?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, we only need 1 cap for all token related queries and functions. |
||
| }, | ||
| .create_cq = uct_rc_mlx5_iface_common_create_cq, | ||
| .destroy_cq = uct_rc_mlx5_iface_common_destroy_cq, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.