netdev - Re: [PATCH net-next v2 10/15] ice: disable Tx timestamps while link is down

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: <Y5El5C8EFQhU+Ukd@x130>
Date:   Wed, 7 Dec 2022 15:46:44 -0800
From:   Saeed Mahameed <saeed@...nel.org>
To:     Tony Nguyen <anthony.l.nguyen@...el.com>
Cc:     davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
        edumazet@...gle.com, Jacob Keller <jacob.e.keller@...el.com>,
        netdev@...r.kernel.org, richardcochran@...il.com, leon@...nel.org,
        Gurucharan G <gurucharanx.g@...el.com>
Subject: Re: [PATCH net-next v2 10/15] ice: disable Tx timestamps while link
 is down

On 07 Dec 13:09, Tony Nguyen wrote:
>From: Jacob Keller <jacob.e.keller@...el.com>
>
>Introduce a new link_down field for the Tx tracker which indicates whether
>the link is down for a given port.
>
>Use this bit to prevent any Tx timestamp requests from starting while link
Can tx timestamp requests be generated in a context other than the xmit ndo
? how ?

why not just use the same sync mechanisms we have for tx queues, 
carrier_down etc ..? 

Anyway I'm just curious.. 

>is down. This ensures that we do not try to start new timestamp requests
>until after link has been restored.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>Tested-by: Gurucharan G <gurucharanx.g@...el.com> (A Contingent worker at Intel)
>Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
>---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 11 ++++++++++-
> drivers/net/ethernet/intel/ice/ice_ptp.h |  2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
>index 481492d84e0e..dffcd50bac3f 100644
>--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
>+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
>@@ -613,7 +613,7 @@ ice_ptp_is_tx_tracker_up(struct ice_ptp_tx *tx)
> {
> 	lockdep_assert_held(&tx->lock);
>
>-	return tx->init && !tx->calibrating;
>+	return tx->init && !tx->calibrating && !tx->link_down;
> }
>
> /**
>@@ -806,6 +806,8 @@ ice_ptp_alloc_tx_tracker(struct ice_ptp_tx *tx)
> 	}
>
> 	tx->init = 1;
>+	tx->link_down = 0;
>+	tx->calibrating = 0;
>
> 	spin_lock_init(&tx->lock);
>
>@@ -1396,6 +1398,13 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
> 	/* Update cached link status for this port immediately */
> 	ptp_port->link_up = linkup;
>
>+	/* Set the link status of the Tx tracker. While link is down, all Tx
>+	 * timestamp requests will be ignored.
>+	 */
>+	spin_lock(&ptp_port->tx.lock);
>+	ptp_port->tx.link_down = !linkup;
>+	spin_unlock(&ptp_port->tx.lock);
>+
> 	/* E810 devices do not need to reconfigure the PHY */
> 	if (ice_is_e810(&pf->hw))
> 		return;
>diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
>index 0bfafaaab6c7..75dcab8e1124 100644
>--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
>+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
>@@ -119,6 +119,7 @@ struct ice_tx_tstamp {
>  * @init: if true, the tracker is initialized;
>  * @calibrating: if true, the PHY is calibrating the Tx offset. During this
>  *               window, timestamps are temporarily disabled.
>+ * @link_down: if true, the link is down and timestamp requests are disabled
>  * @verify_cached: if true, verify new timestamp differs from last read value
>  */
> struct ice_ptp_tx {
>@@ -130,6 +131,7 @@ struct ice_ptp_tx {
> 	u8 len;
> 	u8 init : 1;
> 	u8 calibrating : 1;
>+	u8 link_down : 1;
> 	u8 verify_cached : 1;
> };
>
>-- 
>2.35.1
>