netdev - Re: [PATCH net-next 4/8] ice: stop intermixing AQ commands/responses debug dumps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zs18kiU9gxJRgA3v@mev-dev.igk.intel.com>
Date: Tue, 27 Aug 2024 09:13:22 +0200
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
	edumazet@...gle.com, netdev@...r.kernel.org,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	Jacek Wierzbicki <jacek.wierzbicki@...el.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net-next 4/8] ice: stop intermixing AQ commands/responses
 debug dumps

On Mon, Aug 26, 2024 at 03:46:44PM -0700, Tony Nguyen wrote:
> From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> 
> The ice_debug_cq() function is called to generate a debug log of control
> queue messages both sent and received. It currently does this over a
> potential total of 6 different printk invocations.
> 
> The main logic prints over 4 calls to ice_debug():
> 
>  1. The metadata including opcode, flags, datalength and return value.
>  2. The cookie in the descriptor.
>  3. The parameter values.
>  4. The address for the databuffer.
> 
> In addition, if the descriptor has a data buffer, it can be logged with two
> additional prints:
> 
>  5. A message indicating the start of the data buffer.
>  6. The actual data buffer, printed using print_hex_dump_debug.
> 
> This can lead to trouble in the event that two different PFs are logging
> messages. The messages become intermixed and it may not be possible to
> determine which part of the output belongs to which control queue message.
> 
> To fix this, it needs to be possible to unambiguously determine which
> messages belong together. This is trivial for the messages that comprise
> the main printing. Combine them together into a single invocation of
> ice_debug().
> 
> The message containing a hex-dump of the data buffer is a bit more
> complicated. This is printed separately as part of print_hex_dump_debug.
> This function takes a prefix, which is currently always set to
> KBUILD_MODNAME. Extend this prefix to include the buffer address for the
> databuffer, which is printed as part of the main print, and which is
> guaranteed to be unique for each buffer.
> 
> Refactor the ice_debug_array(), introducing an ice_debug_array_w_prefix().
> Build the prefix by combining KBUILD_MODNAME with the databuffer address
> using snprintf().
> 
> These changes make it possible to unambiguously determine what data belongs
> to what control queue message.
> 
> Reported-by: Jacek Wierzbicki <jacek.wierzbicki@...el.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com> (A Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_controlq.c | 23 +++++++++---------
>  drivers/net/ethernet/intel/ice/ice_osdep.h    | 24 +++++++++++--------
>  2 files changed, 26 insertions(+), 21 deletions(-)
> 
[...]

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>

Thanks