linux-kernel - [PATCH 2/6] hv_utils: drop .getcrosststamp() support from PTP driver

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:   Thu, 18 May 2017 10:46:03 -0700
From:   kys@...hange.microsoft.com
To:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        devel@...uxdriverproject.org, olaf@...fle.de, apw@...onical.com,
        vkuznets@...hat.com, jasowang@...hat.com,
        leann.ogasawara@...onical.com, marcelo.cerri@...onical.com,
        sthemmin@...rosoft.com
Cc:     "K. Y. Srinivasan" <kys@...rosoft.com>
Subject: [PATCH 2/6] hv_utils: drop .getcrosststamp() support from PTP driver

From: Vitaly Kuznetsov <vkuznets@...hat.com>

Turns out that our implementation of .getcrosststamp() never actually
worked. Hyper-V is sending time samples every 5 seconds and this is
too much for get_device_system_crosststamp() as it's interpolation
algorithm (which nobody is currently using in kernel, btw) accounts
for a 'slow' device but we're not slow in Hyper-V, our time reference
is too far away.

.getcrosststamp() is not currently used, get_device_system_crosststamp()
almost always returns -EINVAL and client falls back to using PTP_SYS_OFFSET
so this patch doesn't change much. I also tried doing interpolation
manually (e.g. the same way hv_ptp_gettime() works and it turns out that
we're getting even lower quality:

PTP_SYS_OFFSET_PRECISE with manual interpolation:
* PHC0                     0   3    37     4  -3974ns[-5338ns] +/-  977ns
* PHC0                     0   3    77     7  +2227ns[+3184ns] +/-  576ns
* PHC0                     0   3   177    10  +3060ns[+5220ns] +/-  548ns
* PHC0                     0   3   377    12  +3937ns[+4371ns] +/- 1414ns
* PHC0                     0   3   377     6   +764ns[+1240ns] +/- 1047ns
* PHC0                     0   3   377     7  -1210ns[-3731ns] +/-  479ns
* PHC0                     0   3   377     9   +153ns[-1019ns] +/-  406ns
* PHC0                     0   3   377    12   -872ns[-1793ns] +/-  443ns
* PHC0                     0   3   377     5   +701ns[+3599ns] +/-  426ns
* PHC0                     0   3   377     5   -923ns[ -375ns] +/- 1062ns

PTP_SYS_OFFSET:
* PHC0                     0   3     7     5    +72ns[+8020ns] +/-  251ns
* PHC0                     0   3    17     5   -885ns[-3661ns] +/-  254ns
* PHC0                     0   3    37     6   -454ns[-5732ns] +/-  258ns
* PHC0                     0   3    77    10  +1183ns[+3754ns] +/-  164ns
* PHC0                     0   3   377     5   +579ns[+1137ns] +/-  110ns
* PHC0                     0   3   377     7   +501ns[+1064ns] +/-   96ns
* PHC0                     0   3   377     9  +1641ns[+3342ns] +/-  106ns
* PHC0                     0   3   377     8    -47ns[  +77ns] +/-  160ns
* PHC0                     0   3   377     5    +54ns[ +107ns] +/-  102ns
* PHC0                     0   3   377     8   -354ns[ -617ns] +/-   89ns

This fact wasn't noticed during the initial testing of the PTP device
somehow but got revealed now. Let's just drop .getcrosststamp()
implementation for now as it doesn't seem to be suitable for us.

Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
Signed-off-by: K. Y. Srinivasan <kys@...rosoft.com>
---
 drivers/hv/hv_util.c |   36 ------------------------------------
 1 files changed, 0 insertions(+), 36 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 186b100..2849143 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -248,7 +248,6 @@ static void hv_set_host_time(struct work_struct *work)
 static struct {
 	u64				host_time;
 	u64				ref_time;
-	struct system_time_snapshot	snap;
 	spinlock_t			lock;
 } host_ts;
 
@@ -281,7 +280,6 @@ static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 		cur_reftime = hyperv_cs->read(hyperv_cs);
 		host_ts.host_time = hosttime;
 		host_ts.ref_time = cur_reftime;
-		ktime_get_snapshot(&host_ts.snap);
 
 		/*
 		 * TimeSync v4 messages contain reference time (guest's Hyper-V
@@ -538,46 +536,12 @@ static int hv_ptp_gettime(struct ptp_clock_info *info, struct timespec64 *ts)
 	return 0;
 }
 
-static int hv_ptp_get_syncdevicetime(ktime_t *device,
-				     struct system_counterval_t *system,
-				     void *ctx)
-{
-	system->cs = hyperv_cs;
-	system->cycles = host_ts.ref_time;
-	*device = ns_to_ktime((host_ts.host_time - WLTIMEDELTA) * 100);
-
-	return 0;
-}
-
-static int hv_ptp_getcrosststamp(struct ptp_clock_info *ptp,
-				 struct system_device_crosststamp *xtstamp)
-{
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&host_ts.lock, flags);
-
-	/*
-	 * host_ts contains the last time sample from the host and the snapshot
-	 * of system time. We don't need to calculate the time delta between
-	 * the reception and now as get_device_system_crosststamp() does the
-	 * required interpolation.
-	 */
-	ret = get_device_system_crosststamp(hv_ptp_get_syncdevicetime,
-					    NULL, &host_ts.snap, xtstamp);
-
-	spin_unlock_irqrestore(&host_ts.lock, flags);
-
-	return ret;
-}
-
 static struct ptp_clock_info ptp_hyperv_info = {
 	.name		= "hyperv",
 	.enable         = hv_ptp_enable,
 	.adjtime        = hv_ptp_adjtime,
 	.adjfreq        = hv_ptp_adjfreq,
 	.gettime64      = hv_ptp_gettime,
-	.getcrosststamp = hv_ptp_getcrosststamp,
 	.settime64      = hv_ptp_settime,
 	.owner		= THIS_MODULE,
 };
-- 
1.7.1