Re: [PATCH net-next V2 1/5] net/mlx5: HWS, Print more details for bad completion
Tariq Toukan <[email protected]>
| Newsgroups | org.kernel.vger.linux-rdma,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 11/08/2026 18:25, Simon Horman wrote: > On Mon, Aug 10, 2026 at 12:26:26PM +0300, Tariq Toukan wrote: >> From: Yevgeny Kliteynik <[email protected]> >> >> When polling for completion returned completion with error, >> parse some more details: QP number and WQE count. >> Also, extract all the long value-to-string if conditions >> to a short value-to-string functions: do it for rule >> resize state, rule status, and syndrome. >> >> Signed-off-by: Yevgeny Kliteynik <[email protected]> >> Reviewed-by: Erez Shitrit <[email protected]> >> Signed-off-by: Tariq Toukan <[email protected]> > > Overall this series looks good to me. > But the AI-generated review of this patch, pasted below, does seem relevant. > Thanks. Yeah it seems relevant indeed. We'll handle. > ... > >> @@ -423,6 +445,15 @@ static void hws_send_engine_dump_error_cqe(struct mlx5hws_send_engine *queue, >> " rule 0x%08llx: |--- syndrome = 0x%x\n", >> HWS_PTR_TO_ID(rule), >> err_cqe->syndrome); >> + mlx5hws_err(ctx, >> + " rule 0x%08llx: |--- QPN = 0x%x\n", >> + HWS_PTR_TO_ID(rule), >> + be32_to_cpu(err_cqe->s_wqe_opcode_qpn) & >> + 0xffffff); > > AI-generated review text: > > Is this new QPN line printing something that the same function already > dumps a few lines later? > > At the tail of hws_send_engine_dump_error_cqe() there is already: > > mlx5hws_err(ctx, > " rule 0x%08llx: |------------ qpn = 0x%06x\n", > HWS_PTR_TO_ID(rule), > be32_to_cpu(cqe->sop_drop_qpn) & 0xffffff); > > err_cqe is just a cast of the same cqe pointer, and both fields land on > the same four bytes of the 64 byte CQE (include/linux/mlx5/device.h): > > struct mlx5_err_cqe { > u8 rsvd0[32]; /* 0..31 */ > __be32 srqn; /* 32..35 */ > u8 rsvd1[18]; /* 36..53 */ > u8 vendor_err_synd;/* 54 */ > u8 syndrome; /* 55 */ > __be32 s_wqe_opcode_qpn;/* 56..59 */ > ... > }; > struct mlx5_cqe64 { > ... > __be32 byte_cnt; /* 44..47 */ > __be32 timestamp_h; /* 48..51 */ > __be32 timestamp_l; /* 52..55 */ > __be32 sop_drop_qpn; /* 56..59 */ > ... > }; > > Same offset, same 0xffffff mask, so a single dump now reports the QP > number twice, once as "QPN = 0x%x" and once as "qpn = 0x%06x", with > different labels and widths. > > Would it make sense to drop the new QPN line, or at least align its label > and width with the existing one, given the one-shot > queue->error_cqe_printed guard is there to keep this dump compact? > > Source: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810092630.3137666-1-tariqt%40nvidia.com > >> + mlx5hws_err(ctx, >> + " rule 0x%08llx: |--- WQE_CNT = 0x%04x\n", >> + HWS_PTR_TO_ID(rule), >> + (u32)be16_to_cpu(err_cqe->wqe_counter)); >> } >> >> mlx5hws_err(ctx, > > ... >