Re: [PATCH net-next V2 1/5] net/mlx5: HWS, Print more details for bad completion
Simon Horman <[email protected]>
| Newsgroups | org.kernel.vger.linux-rdma,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
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. ... > @@ -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, ...