UCT/IB/MLX5: enable striding message based receive work queue on CX8 #10455
Conversation
| @@ -52,7 +52,7 @@ ucs_config_field_t uct_dc_mlx5_iface_config_sub_table[] = { | |||
|
|
|||
| /* Since long timeout will block SRQ in case of network failure on single | |||
| * peer default SRQ to list topology. Incur performance degradation. */ | |||
| {"RC_", "SRQ_TOPO=list", NULL, | |||
| {"RC_", "SRQ_TOPO=striding_message_based,list", NULL, | |||
There was a problem hiding this comment.
the comment above needs to be updated now
There was a problem hiding this comment.
also maybe just msg_based? it is already strided rq topo
| @@ -398,6 +398,8 @@ ucs_status_t uct_dc_mlx5_iface_create_dci(uct_dc_mlx5_iface_t *iface, | |||
| attr.rdma_wr_disabled = (iface->flags & UCT_DC_MLX5_IFACE_FLAG_DISABLE_PUT) && | |||
| (md->flags & UCT_IB_MLX5_MD_FLAG_NO_RDMA_WR_OPTIMIZED); | |||
| attr.log_num_dci_stream_channels = ucs_ilog2(num_dci_channels); | |||
| attr.is_smbrwq_associated = uct_rc_mlx5_iface_is_srq_smbrwq( | |||
There was a problem hiding this comment.
imo smbrwq is hard to read. Maybe something like msg_srq?
| } | ||
|
|
||
| static int uct_ib_mlx5_devx_is_smbrwq_enabled(uct_ib_mlx5_md_t *md, | ||
| uct_ib_mlx5_qp_attr_t *attr) |
| (md->flags & UCT_IB_MLX5_MD_FLAG_QP_CTX_EXTENSION) && | ||
| (((md->smbrwq.supported_tls & UCT_IB_MLX5_SMBRWQ_SUPPORT_RC) && | ||
| (attr->super.qp_type == IBV_QPT_RC)) || | ||
| ((md->smbrwq.supported_tls & UCT_IB_MLX5_SMBRWQ_SUPPORT_DC) && | ||
| (attr->super.qp_type == UCT_IB_QPT_DCI))); |
There was a problem hiding this comment.
minor: i'd add some if/else to check qp_type and make itt easier to read
| uint8_t driver_version_before_init_hca[0x1]; | ||
| uint8_t adv_virtualization[0x1]; | ||
| uint8_t driver_metadata_ptr[0x1]; | ||
| uint8_t log_max_dct_connections[0x5]; |
There was a problem hiding this comment.
need to update the name of the field below reserved_at_...
| UCT_IB_MLX5_SMBRWQ_SUPPORT_UC = UCS_BIT(1), | ||
| UCT_IB_MLX5_SMBRWQ_SUPPORT_DC = UCS_BIT(2), | ||
| UCT_IB_MLX5_SMBRWQ_SUPPORT_UD = UCS_BIT(3), | ||
| UCT_IB_MLX5_SMBRWQ_SUPPORT_XRC = UCS_BIT(4), |
There was a problem hiding this comment.
do we really need to mention XRC in our code?
|
|
||
| uint8_t pas[0][0x40]; | ||
| uint8_t wq_umem_valid[0x1]; | ||
| uint8_t reserved_at_861[0x1f]; |
There was a problem hiding this comment.
seems need to update array size
| @@ -168,6 +168,8 @@ struct mlx5_grh_av { | |||
|
|
|||
| #define UCT_IB_MLX5_DEVX_ECE_TRIG_RESP 0x10000000 | |||
|
|
|||
| #define UCT_IB_MLX5_DEVX_SMBRWQ_MAX_SEND_RECEIVE_MESSAGE_SIZE 512 | |||
There was a problem hiding this comment.
is it byte, log or? can you pls reflect that in the name?
| @@ -226,7 +231,7 @@ enum { | |||
| UCT_IB_MLX5_SRQ_TOPO_LIST = 0x0, | |||
| UCT_IB_MLX5_SRQ_TOPO_CYCLIC = 0x1, | |||
| UCT_IB_MLX5_SRQ_TOPO_LIST_MP_RQ = 0x2, | |||
| UCT_IB_MLX5_SRQ_TOPO_CYCLIC_MP_RQ = 0x3 | |||
| UCT_IB_MLX5_SRQ_TOPO_CYCLIC_MP_RQ = 0x3, | |||
There was a problem hiding this comment.
btw, why don't we need to add new type here?
There was a problem hiding this comment.
Because from FW perspective they want us to pass 0x2 as wq_type (striding list)
| UCT_IB_MLX5_SMBRWQ_TLS_UC = UCS_BIT(1), | ||
| UCT_IB_MLX5_SMBRWQ_TLS_DC = UCS_BIT(2), | ||
| UCT_IB_MLX5_SMBRWQ_TLS_UD = UCS_BIT(3), | ||
| UCT_IB_MLX5_SMBRWQ_TLS_XRC = UCS_BIT(4), |
roiedanino
added
WIP-DNM
What?
Added a new SRQ topology - Striding Message-Based Receive Work Queue which is available on CX8
Why?
The new SRQ topology enables generating a CQE for each message (which can be multi-packet) instead of generating a CQE for each packet (which is less efficient).