netdev - Re: [PATCH net-next v2] net: mvpp2: tai: warn once if we fail to update our timestamp

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: <07eeb151-008b-4b92-8db9-31cd51ec3e77@intel.com>
Date: Wed, 11 Dec 2024 12:48:21 +0100
From: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
To: Russell King <rmk+kernel@...linux.org.uk>, Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>
CC: Marcin Wojtas <marcin.s.wojtas@...il.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2] net: mvpp2: tai: warn once if we fail to
 update our timestamp



On 12/11/2024 11:55 AM, Russell King wrote:
> The hardware timestamps for packets contain a truncated seconds field,
> only containing two bits of seconds. In order to provide the full
> number of seconds, we need to keep track of the full hardware clock by
> reading it every two seconds.
> 
> However, if we fail to read the clock, we silently ignore the error.
> Print a warning indicating that the PP2 TAI clock timestamps have
> become unreliable.
> 
> Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
> --
> v2: correct dev_warn_once() indentation
> ---
>   drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c
> index 95862aff49f1..6b60beb1f3ed 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c
> @@ -54,6 +54,7 @@
>   #define TCSR_CAPTURE_0_VALID		BIT(0)
>   
>   struct mvpp2_tai {
> +	struct device *dev;
>   	struct ptp_clock_info caps;
>   	struct ptp_clock *ptp_clock;
>   	void __iomem *base;
> @@ -303,7 +304,8 @@ static long mvpp22_tai_aux_work(struct ptp_clock_info *ptp)
>   {
>   	struct mvpp2_tai *tai = ptp_to_tai(ptp);
>   
> -	mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL);
> +	if (mvpp22_tai_gettimex64(ptp, &tai->stamp, NULL) < 0)
> +		dev_warn_once(tai->dev, "PTP timestamps are unreliable");

Only small nitpick/question - shouldn't text end with '\n'? I see in the
code that most of calls of dev_warn_once has '\n' at the end.

>   
>   	return msecs_to_jiffies(2000);
>   }
> @@ -401,6 +403,7 @@ int mvpp22_tai_probe(struct device *dev, struct mvpp2 *priv)
>   
>   	spin_lock_init(&tai->lock);
>   
> +	tai->dev = dev;
>   	tai->base = priv->iface_base;
>   
>   	/* The step size consists of three registers - a 16-bit nanosecond step

Nevertheless the code itself looks good to me, nice improvement.

Reviewed-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>