issue: 4203242 dump wqe to log in case of ecqe #273
Conversation
|
based on the following patch: |
src/core/dev/cq_mgr_tx.cpp
Outdated
| const auto wqe = static_cast<const uint8_t *>(m_hqtx_ptr->m_mlx5_qp.sq.buf) + 64 * index; | ||
|
|
||
| std::ostringstream oss; | ||
| for (uint8_t i = 0; i < m_hqtx_ptr->m_mlx5_qp.cap.max_inline_data; ++i) { |
There was a problem hiding this comment.
Using max_inline_data is not good here. Wqe may vary in size.
There was a problem hiding this comment.
you have the credits field inside sq_wqe_prop which should specify the number of weqbbs this wqe consumed. Comment there says 'usually' so need to verify.
|
TODO - squash |
tomerdbz
force-pushed
the
dump_wqe_4203242
branch
2 times, most recently
from
c3407af to
3b59cb9
Compare
src/core/dev/cq_mgr_tx.cpp
Outdated
| std::ostringstream oss; | ||
| // see `calculate_credits` - credits is give or take the amount of WQEBBs per WQE | ||
| for (uint32_t wqebb_i = 0; wqebb_i < credits; ++wqebb_i) { | ||
| const uint32_t wqebb_wrapped_index = wqebb_i & (m_hqtx_ptr->m_tx_num_wr - 1); |
There was a problem hiding this comment.
im not sure this will work
There was a problem hiding this comment.
i think we need to wrap current_wqebb_begin
There was a problem hiding this comment.
i would say wrap(wqe_begin + wqebb_i * WQEBB_SIZE)
There was a problem hiding this comment.
This will make the wrapping less readable imho.
The PRM says how to wrap WQEBB index - not actual byte index.
I think we should conform to the PRM reader and not do a complex logic.
Though if you think this doesn't look right, let me know and I'll change.
|
This will make the wrapping less readable imho.
The PRM says how to wrap WQEBB index - not actual byte index.
I think we should conform to the PRM reader and not do a complex logic.
Though if you think this doesn't look right, let me know and I'll change.
…On Tue, Dec 17, 2024, 15:05 Alexander Grissik ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/core/dev/cq_mgr_tx.cpp
<#273 (comment)>:
> @@ -204,6 +207,27 @@ cq_mgr_tx *cq_mgr_tx::get_cq_mgr_from_cq_event(struct ibv_comp_channel *p_cq_cha
return p_cq_mgr;
}
+std::string cq_mgr_tx::wqe_to_hexstring(uint16_t index, uint32_t credits) const
+{
+ const auto wqe_begin =
+ static_cast<const uint8_t *>(m_hqtx_ptr->m_mlx5_qp.sq.buf) + WQEBB_SIZE * index;
+
+ std::ostringstream oss;
+ // see `calculate_credits` - credits is give or take the amount of WQEBBs per WQE
+ for (uint32_t wqebb_i = 0; wqebb_i < credits; ++wqebb_i) {
+ const uint32_t wqebb_wrapped_index = wqebb_i & (m_hqtx_ptr->m_tx_num_wr - 1);
i would say wrap(wqe_begin + wqebb_i * WQEBB_SIZE)
—
Reply to this email directly, view it on GitHub
<#273 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC6BOZNF6NOH5WP7BSBQTGT2GAOSNAVCNFSM6AAAAABTIWSW5OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMBYHAZTGMRRGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
@AlexanderGrissik, can you approve? |
tomerdbz
force-pushed
the
dump_wqe_4203242
branch
from
3b59cb9 to
964e880
Compare
|
bot:retest |
dump wqe's content for further debug in case of ecqe. Signed-off-by: Tomer Cabouly <[email protected]>
tomerdbz
force-pushed
the
dump_wqe_4203242
branch
from
964e880 to
d06ce09
Compare
dump wqe's content for further debug in case of ecqe.
Description
Following https://redmine.mellanox.com/issues/4183221
We created a patch to dump the wqe that had the invalid state.
No reason to not have it in our vNext as well.
What
Subject: what this PR is doing in one line.
Why ?
Further debug ability.
Change type
What kind of change does this PR introduce?
Check list