netdev - Re: [PATCH net-next 2/3] ptp: introduce a phase offset in the periodic output request

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: <8e487020-7879-e08b-f1fc-a0ebd368a300@intel.com>
Date:   Thu, 16 Jul 2020 14:40:22 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Vladimir Oltean <olteanv@...il.com>, kuba@...nel.org,
        davem@...emloft.net, netdev@...r.kernel.org
Cc:     richardcochran@...il.com, yangbo.lu@....com,
        xiaoliang.yang_1@....com, po.liu@....com,
        UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next 2/3] ptp: introduce a phase offset in the
 periodic output request



On 7/16/2020 2:20 PM, Vladimir Oltean wrote:
> Some PHCs like the ocelot/felix switch cannot emit generic periodic
> output, but just PPS (pulse per second) signals, which:
> - don't start from arbitrary absolute times, but are rather
>   phase-aligned to the beginning of [the closest next] second.
> - have an optional phase offset relative to that beginning of the
>   second.
> 
> For those, it was initially established that they should reject any
> other absolute time for the PTP_PEROUT_REQUEST than 0.000000000 [1].
> 
> But when it actually came to writing an application [2] that makes use
> of this functionality, we realized that we can't really deal generically
> with PHCs that support absolute start time, and with PHCs that don't,
> without an explicit interface. Namely, in an ideal world, PHC drivers
> would ensure that the "perout.start" value written to hardware will
> result in a functional output. This means that if the PTP time has
> become in the past of this PHC's current time, it should be
> automatically fast-forwarded by the driver into a close enough future
> time that is known to work (note: this is necessary only if the hardware
> doesn't do this fast-forward by itself). But we don't really know what
> is the status for PHC drivers in use today, so in the general sense,
> user space would be risking to have a non-functional periodic output if
> it simply asked for a start time of 0.000000000.
> 
> So let's introduce a flag for this type of reduced-functionality
> hardware, named PTP_PEROUT_PHASE. The start time is just "soon", the
> only thing we know for sure about this signal is that its rising edge
> events, Rn, occur at:
> 
> Rn = period.phase + n * perout.period
> 
> The "phase" in the periodic output structure is simply an alias to the
> "start" time, since both cannot logically be specified at the same time.
> Therefore, the binary layout of the structure is not affected.
> 
> [1]: https://patchwork.ozlabs.org/project/netdev/patch/20200320103726.32559-7-yangbo.lu@nxp.com/
> [2]: https://www.mail-archive.com/linuxptp-devel@lists.sourceforge.net/msg04142.html
> 
> Signed-off-by: Vladimir Oltean <olteanv@...il.com>
> ---
>  include/uapi/linux/ptp_clock.h | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
> index 1d2841155f7d..1d108d597f66 100644
> --- a/include/uapi/linux/ptp_clock.h
> +++ b/include/uapi/linux/ptp_clock.h
> @@ -55,12 +55,14 @@
>   */
>  #define PTP_PEROUT_ONE_SHOT		(1<<0)
>  #define PTP_PEROUT_DUTY_CYCLE		(1<<1)
> +#define PTP_PEROUT_PHASE		(1<<2)
>  
>  /*
>   * flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl.
>   */
>  #define PTP_PEROUT_VALID_FLAGS		(PTP_PEROUT_ONE_SHOT | \
> -					 PTP_PEROUT_DUTY_CYCLE)
> +					 PTP_PEROUT_DUTY_CYCLE | \
> +					 PTP_PEROUT_PHASE)
>  
>  /*
>   * No flags are valid for the original PTP_PEROUT_REQUEST ioctl
> @@ -103,7 +105,20 @@ struct ptp_extts_request {
>  };
>  
>  struct ptp_perout_request {
> -	struct ptp_clock_time start;  /* Absolute start time. */
> +	union {
> +		/*
> +		 * Absolute start time.
> +		 * Valid only if (flags & PTP_PEROUT_PHASE) is unset.
> +		 */
> +		struct ptp_clock_time start;
> +		/*
> +		 * Phase offset. The signal should start toggling at an
> +		 * unspecified integer multiple of the period, plus this value.
> +		 * The start time should be "as soon as possible".
> +		 * Valid only if (flags & PTP_PEROUT_PHASE) is set.
> +		 */
> +		struct ptp_clock_time phase;
> +	};

Ok. Since when using the PHASE mode the start time is "meaningless" we
can re-use it for this purpose without breaking the binary structure.
Makes sense.

>  	struct ptp_clock_time period; /* Desired period, zero means disable. */
>  	unsigned int index;           /* Which channel to configure. */
>  	unsigned int flags;
>