netdev - Re: [PATCH net-next 02/12] bnxt_en: Retry PTP TX timestamp from FW for 1 second

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: <00936e03-e4a0-4d93-a576-58a7cd61fd78@linux.dev>
Date: Wed, 27 Mar 2024 23:02:24 -0700
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Michael Chan <michael.chan@...adcom.com>, davem@...emloft.net,
 kuba@...nel.org
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
 pavan.chebbi@...adcom.com, andrew.gospodarek@...adcom.com,
 Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH net-next 02/12] bnxt_en: Retry PTP TX timestamp from FW
 for 1 second

On 25/03/2024 22:28, Michael Chan wrote:
> From: Pavan Chebbi <pavan.chebbi@...adcom.com>
> 
> Use a new default 1 second timeout value instead of the existing
> 1 msec value.  The driver will keep track of the remaining time
> before timeout and will pass this value to bnxt_hwrm_port_ts_query().
> The firmware supports timeout values up to 65535 usecs.  If the
> timeout value passed to bnxt_hwrm_port_ts_query() is less than the
> FW max value, we will use that value to precisely control the
> specified timeout.  If it is larger than the FW max value, we will
> use the FW max value and any additional retry to reach the desired
> timeout will be done in the context of bnxt_ptp_ts_aux_eork().
> 
> Link: https://lore.kernel.org/netdev/20240229070202.107488-2-michael.chan@broadcom.com/
> Cc: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> Cc: Richard Cochran <richardcochran@...il.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@...adcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> ---
> v2: Don't use the devlink parameter.
>      Pass the timeout parameter to bnxt_hwrm_port_ts_query() to precisely
>      control the timeout.
> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 16 +++++++++++++++-
>   drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h |  4 ++++
>   2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index dbfd1b36774c..345aac4484ee 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -678,11 +678,17 @@ static void bnxt_stamp_tx_skb(struct bnxt *bp, struct sk_buff *skb)
>   {
>   	struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
>   	struct skb_shared_hwtstamps timestamp;
> +	unsigned long now = jiffies;
>   	u64 ts = 0, ns = 0;
> +	u32 tmo = 0;
>   	int rc;
>   
> +	if (!ptp->txts_pending)
> +		ptp->abs_txts_tmo = now + msecs_to_jiffies(ptp->txts_tmo);
> +	if (!time_after_eq(now, ptp->abs_txts_tmo))
> +		tmo = jiffies_to_msecs(ptp->abs_txts_tmo - now);
>   	rc = bnxt_hwrm_port_ts_query(bp, PORT_TS_QUERY_REQ_FLAGS_PATH_TX, &ts,
> -				     0);
> +				     tmo);
>   	if (!rc) {
>   		memset(&timestamp, 0, sizeof(timestamp));
>   		spin_lock_bh(&ptp->ptp_lock);
> @@ -691,6 +697,10 @@ static void bnxt_stamp_tx_skb(struct bnxt *bp, struct sk_buff *skb)
>   		timestamp.hwtstamp = ns_to_ktime(ns);
>   		skb_tstamp_tx(ptp->tx_skb, &timestamp);
>   	} else {
> +		if (!time_after_eq(jiffies, ptp->abs_txts_tmo)) {
> +			ptp->txts_pending = true;
> +			return;
> +		}
>   		netdev_warn_once(bp->dev,
>   				 "TS query for TX timer failed rc = %x\n", rc);
>   	}
> @@ -698,6 +708,7 @@ static void bnxt_stamp_tx_skb(struct bnxt *bp, struct sk_buff *skb)
>   	dev_kfree_skb_any(ptp->tx_skb);
>   	ptp->tx_skb = NULL;
>   	atomic_inc(&ptp->tx_avail);
> +	ptp->txts_pending = false;
>   }
>   
>   static long bnxt_ptp_ts_aux_work(struct ptp_clock_info *ptp_info)
> @@ -721,6 +732,8 @@ static long bnxt_ptp_ts_aux_work(struct ptp_clock_info *ptp_info)
>   		spin_unlock_bh(&ptp->ptp_lock);
>   		ptp->next_overflow_check = now + BNXT_PHC_OVERFLOW_PERIOD;
>   	}
> +	if (ptp->txts_pending)
> +		return 0;
>   	return HZ;
>   }
>   
> @@ -973,6 +986,7 @@ int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg)
>   		spin_unlock_bh(&ptp->ptp_lock);
>   		ptp_schedule_worker(ptp->ptp_clock, 0);
>   	}
> +	ptp->txts_tmo = BNXT_PTP_DFLT_TX_TMO;
>   	return 0;
>   
>   out:
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
> index 04886d5f22ad..6a2bba3f9e2d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
> @@ -22,6 +22,7 @@
>   #define BNXT_LO_TIMER_MASK	0x0000ffffffffUL
>   #define BNXT_HI_TIMER_MASK	0xffff00000000UL
>   
> +#define BNXT_PTP_DFLT_TX_TMO	1000 /* ms */
>   #define BNXT_PTP_QTS_TIMEOUT	1000
>   #define BNXT_PTP_QTS_MAX_TMO_US	65535
>   #define BNXT_PTP_QTS_TX_ENABLES	(PORT_TS_QUERY_REQ_ENABLES_PTP_SEQ_ID |	\
> @@ -116,11 +117,14 @@ struct bnxt_ptp_cfg {
>   					 BNXT_PTP_MSG_PDELAY_REQ |	\
>   					 BNXT_PTP_MSG_PDELAY_RESP)
>   	u8			tx_tstamp_en:1;
> +	u8			txts_pending:1;
>   	int			rx_filter;
>   	u32			tstamp_filters;
>   
>   	u32			refclk_regs[2];
>   	u32			refclk_mapped_regs[2];
> +	u32			txts_tmo;
> +	unsigned long		abs_txts_tmo;
>   };
>   
>   #if BITS_PER_LONG == 32

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>