netdev - Re: [PATCH net] ice: avoid executing commands on other ports when driving sync

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20230824135139.GH3523530@kernel.org>
Date: Thu, 24 Aug 2023 15:51:39 +0200
From: Simon Horman <horms@...nel.org>
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,
	Jacob Keller <jacob.e.keller@...el.com>, richardcochran@...il.com,
	Siddaraju DH <siddaraju.dh@...el.com>,
	Sunitha Mekala <sunithax.d.mekala@...el.com>
Subject: Re: [PATCH net] ice: avoid executing commands on other ports when
 driving sync

On Wed, Aug 23, 2023 at 08:18:14AM -0700, Tony Nguyen wrote:
> From: Jacob Keller <jacob.e.keller@...el.com>
> 
> The ice hardware has a synchronization mechanism used to drive the
> simultaneous application of commands on both PHY ports and the source timer
> in the MAC.
> 
> When issuing a sync via ice_ptp_exec_tmr_cmd(), the hardware will
> simultaneously apply the commands programmed for the main timer and each
> PHY port. Neither the main timer command register, nor the PHY port command
> registers auto clear on command execution.
> 
> During the execution of a timer command intended for a single port on E822
> devices, such as those used to configure a PHY during link up, the driver
> is not correctly clearing the previous commands.
> 
> This results in unintentionally executing the last programmed command on
> the main timer and other PHY ports whenever performing reconfiguration on
> E822 ports after link up. This results in unintended side effects on other
> timers, depending on what command was previously programmed.
> 
> To fix this, the driver must ensure that the main timer and all other PHY
> ports are properly initialized to perform no action.
> 
> The enumeration for timer commands does not include an enumeration value
> for doing nothing. Introduce ICE_PTP_NOP for this purpose. When writing a
> timer command to hardware, leave the command bits set to zero which
> indicates that no operation should be performed on that port.
> 
> Modify ice_ptp_one_port_cmd() to always initialize all ports. For all ports
> other than the one being configured, write their timer command register to
> ICE_PTP_NOP. This ensures that no side effect happens on the timer command.
> 
> To fix this for the PHY ports, modify ice_ptp_one_port_cmd() to always
> initialize all other ports to ICE_PTP_NOP. This ensures that no side
> effects happen on the other ports.
> 
> Call ice_ptp_src_cmd() with a command value if ICE_PTP_NOP in
> ice_sync_phy_timer_e822() and ice_start_phy_timer_e822().
> 
> With both of these changes, the driver should no longer execute a stale
> command on the main timer or another PHY port when reconfiguring one of the
> PHY ports after link up.
> 
> Fixes: 3a7496234d17 ("ice: implement basic E822 PTP support")
> Signed-off-by: Siddaraju DH <siddaraju.dh@...el.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Tested-by: Sunitha Mekala <sunithax.d.mekala@...el.com> (A Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>

Reviewed-by: Simon Horman <horms@...nel.org>