netdev - Re: [PATCH net-next] net: lan743x: Use NSEC_PER

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: <5cfbde6c-0e6e-6c1f-c872-23fd00494b77@huawei.com>
Date: Wed, 4 Sep 2024 14:33:23 +0800
From: Jinjie Ruan <ruanjinjie@...wei.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <bryan.whitehead@...rochip.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<richardcochran@...il.com>, <UNGLinuxDriver@...rochip.com>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: lan743x: Use NSEC_PER_SEC macro



On 2024/9/3 0:26, Andrew Lunn wrote:
> On Mon, Sep 02, 2024 at 03:18:41PM +0800, Jinjie Ruan wrote:
>> 1000000000L is number of ns per second, use NSEC_PER_SEC macro to replace
>> it to make it more readable.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>
>> ---
>>  drivers/net/ethernet/microchip/lan743x_ptp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
>> index dcea6652d56d..9c2ec293c163 100644
>> --- a/drivers/net/ethernet/microchip/lan743x_ptp.c
>> +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
>> @@ -409,7 +409,7 @@ static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci,
>>  				   ts->tv_sec);
>>  			return -ERANGE;
>>  		}
>> -		if (ts->tv_nsec >= 1000000000L ||
>> +		if (ts->tv_nsec >= NSEC_PER_SEC ||
>>  		    ts->tv_nsec < 0) {
>>  			netif_warn(adapter, drv, adapter->netdev,
>>  				   "ts->tv_nsec out of range, %ld\n",
> 
> https://elixir.bootlin.com/linux/v6.10.7/source/include/linux/time64.h#L92
> 
> /*
>  * Returns true if the timespec64 is norm, false if denorm:
>  */
> static inline bool timespec64_valid(const struct timespec64 *ts)
> {
>         /* Dates before 1970 are bogus */
>         if (ts->tv_sec < 0)
>                 return false;
>         /* Can't have more nanoseconds then a second */
>         if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)
>                 return false;
>         return true;
> }
> 
> And the next question is, why is the driver checking this? It would
> make more sense that the PTP core checked this before calling
> ptp->info->settime64()

There are 2 places call ptp->info->settime64(), it may make more sense
to check timespec64_valid() here and remove these check internal like
lan743x_ptp.c?

drivers/net/phy/micrel.c:4721:          ptp->settime64(ptp, &ts);
drivers/ptp/ptp_clock.c:103:    return  ptp->info->settime64(ptp->info, tp);

> 
> 	Andrew