Messages in this thread Patch in this message |  | | From | Jakub Kicinski <> | | Subject | Re: [PATCH net-next 07/15] ixgbe: E610: add ACI dynamic debug | | Date | Sat, 2 May 2026 19:06:36 -0700 |
| |
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- ixgbe: E610: add ACI dynamic debug
This commit enables dynamic debugging of the Admin Command Interface (ACI) for the e610 adapter, allowing ACI commands and descriptor details to be printed into the kernel log.
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
[ ... ]
> @@ -106,8 +185,12 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw, > if (desc->flags & cpu_to_le16(LIBIE_AQ_FLAG_RD)) { > for (i = 0; i < buf_size / 4; i++) > IXGBE_WRITE_REG(hw, IXGBE_PF_HIBA(i), ((u32 *)buf)[i]); > - if (buf_tail_size) > + ixgbe_aci_debug(hw, desc, buf, buf_size); > + if (buf_tail_size) { > IXGBE_WRITE_REG(hw, IXGBE_PF_HIBA(i), buf_tail); > + ixgbe_aci_debug(hw, desc, &buf_tail, > + buf_tail_size); > + } > } > }
Does this result in redundant output being printed to the kernel log?
When buf_tail_size is non-zero, ixgbe_aci_debug() is called twice: once for the full buffer, and a second time for just the tail. Since ixgbe_aci_debug() prints the full CQ CMD descriptor details each time, this appears to print the descriptor headers twice and fragment the buffer output.
[ ... ]
> @@ -183,10 +278,13 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw, > if (valid_buf) { > for (i = 0; i < buf_size / 4; i++) > ((u32 *)buf)[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIBA(i)); > + ixgbe_aci_debug(hw, raw_desc, buf, buf_size); > if (buf_tail_size) { > buf_tail = IXGBE_READ_REG(hw, IXGBE_PF_HIBA(i)); > memcpy(buf + buf_size - buf_tail_size, &buf_tail, > buf_tail_size); > + ixgbe_aci_debug(hw, raw_desc, &buf_tail, > + buf_tail_size); > } > }
Can this read uninitialized memory from the buffer?
At the point ixgbe_aci_debug(..., buf, buf_size) is called, the loop has only populated the 4-byte aligned chunks of the buffer. The trailing bytes at the end of the buffer are not populated until the memcpy() occurs later in the block. Because ixgbe_aci_debug() consumes buf_size bytes, it looks like it will read and log uninitialized trailing memory (potentially from the kernel stack).
Also, similar to the transmit path, this calls ixgbe_aci_debug() a second time on &buf_tail, which will print the descriptor headers again.
Could we move a single ixgbe_aci_debug(..., buf, buf_size) call to the end of the block, after the entire buffer including the tail has been fully written?
|  |