netdev - Re: [PATCH net-next v2 1/5] ptp: Add cycles support for virtual clocks

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]

Date:   Sat, 9 Apr 2022 23:26:21 -0700
From:   Richard Cochran <richardcochran@...il.com>
To:     Gerhard Engleder <gerhard@...leder-embedded.com>
Cc:     vinicius.gomes@...el.com, yangbo.lu@....com, davem@...emloft.net,
        kuba@...nel.org, mlichvar@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/5] ptp: Add cycles support for virtual
 clocks

On Sun, Apr 03, 2022 at 07:55:40PM +0200, Gerhard Engleder wrote:

> @@ -225,6 +233,21 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
>  	mutex_init(&ptp->n_vclocks_mux);
>  	init_waitqueue_head(&ptp->tsev_wq);
>  
> +	if (!ptp->info->getcycles64 && !ptp->info->getcyclesx64) {

Please swap blocks, using non-negated logical test:

	if (ptp->info->getcycles64 || ptp->info->getcyclesx64)

> +		/* Free running cycle counter not supported, use time. */
> +		ptp->info->getcycles64 = ptp_getcycles64;
> +
> +		if (ptp->info->gettimex64)
> +			ptp->info->getcyclesx64 = ptp->info->gettimex64;
> +
> +		if (ptp->info->getcrosststamp)
> +			ptp->info->getcrosscycles = ptp->info->getcrosststamp;
> +	} else {
> +		ptp->has_cycles = true;
> +		if (!ptp->info->getcycles64 && ptp->info->getcyclesx64)
> +			ptp->info->getcycles64 = ptp_getcycles64;
> +	}
> +
>  	if (ptp->info->do_aux_work) {
>  		kthread_init_delayed_work(&ptp->aux_work, ptp_aux_kworker);
>  		ptp->kworker = kthread_create_worker(0, "ptp%d", ptp->index);


> @@ -231,10 +231,12 @@ static ssize_t n_vclocks_store(struct device *dev,
>  			*(ptp->vclock_index + ptp->n_vclocks - i) = -1;
>  	}
>  
> -	if (num == 0)
> -		dev_info(dev, "only physical clock in use now\n");
> -	else
> -		dev_info(dev, "guarantee physical clock free running\n");
> +	if (!ptp->has_cycles) {

Not sure what this test means ...

> +		if (num == 0)
> +			dev_info(dev, "only physical clock in use now\n");

Shouldn't this one print even if has_cycles == false?

> +		else
> +			dev_info(dev, "guarantee physical clock free running\n");
> +	}
>  
>  	ptp->n_vclocks = num;
>  	mutex_unlock(&ptp->n_vclocks_mux);

Thanks,
Richard