Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe
@ 2026-10-13 19:19 James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children James Hilliard
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: James Hilliard @ 2026-10-13 19:19 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Richard Cochran, Andrew Lunn,
	Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel, James Hilliard

Fix virtual-clock sampling errors, registration rollback and parent
teardown in separate patches. Keep failed PHC reads out of the timecounter
and serialize extended samples with virtual-clock adjustments. Also
preserve the zero sentinel for packets without hardware timestamps.

Unlike v2, this revision retains the existing cyclecounter mask, scaling
and conversion windows. It does not assume full-width nanosecond samples
or promise recovery of elapsed time after an unobserved counter wrap.

Changes in v3:
- Split the independent fixes into separate patches, as requested by Paolo.
- Retain the existing timecounter arithmetic and conversion windows; drop
  the full-width rewrite and extended-read anchor updates from v2.
- Propagate registration errors through sysfs, including a failed initial
  PHC sample, and initialize the counter before publishing the clock.
- Preserve absent hardware timestamps instead of converting zero.
- Correct the sysfs teardown rationale and its Fixes tag.
- Rebase onto current net.
- Link to v2: https://patch.msgid.link/20261005-ptp-vclock-sampling-v2-0-8ed12d4d10af@gmail.com

Changes in v2:
- Drain virtual clock sysfs changes before parent unregister walks children.
- Use full-width samples and overflow-safe scaling across long sampling gaps.
- Update the anchor for extended and cross-timestamp reads under the clock lock.
- Link to v1: https://patch.msgid.link/20260930-ptp-vclock-sampling-v1-1-c11bc16691ae@gmail.com

Assisted-by: Codex:gpt-6-astra
---
James Hilliard (5):
      ptp: drain virtual clock sysfs operations before unregistering children
      ptp: unwind partially completed virtual clock creation
      ptp: vclock: propagate physical clock sampling errors
      ptp: vclock: serialize extended samples with counter updates
      ptp: vclock: preserve missing hardware timestamps

 drivers/ptp/ptp_clock.c   |   5 +++
 drivers/ptp/ptp_private.h |   4 +-
 drivers/ptp/ptp_sysfs.c   |  18 ++++++++-
 drivers/ptp/ptp_vclock.c  | 100 +++++++++++++++++++++++++++++-----------------
 4 files changed, 87 insertions(+), 40 deletions(-)
---
base-commit: 513444cc62e231a11c9b15ea88de0890da9b2b4f
change-id: 20260930-ptp-vclock-sampling-902f98408c93

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children
  2026-10-13 19:19 [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe James Hilliard
@ 2026-10-13 19:19 ` James Hilliard
  2026-10-13 19:46   ` Vadim Fedorenko
  2026-10-13 19:19 ` [PATCH net v3 2/5] ptp: unwind partially completed virtual clock creation James Hilliard
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2026-10-13 19:19 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Richard Cochran, Andrew Lunn,
	Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel, James Hilliard

Parent clock removal walks its virtual clocks before removing the
n_vclocks sysfs attribute. The mutex taken by ptp_vclock_in_use() is
released before that walk, so a concurrent sysfs deletion can pick the
same child and unregister and free its ptp_vclock a second time. A
reference held by the device iterator does not protect that separately
allocated virtual clock.

Remove n_vclocks before walking the children. Removing the attribute
prevents new stores and drains stores already running, without holding
n_vclocks_mux across a callback that needs that mutex. Skip virtual
clocks, which do not have this attribute.

This race was found by code inspection of virtual-clock registration
failure cleanup and parent removal.

Fixes: bfcbb76b0f59 ("ptp: unregister virtual clocks when unregistering physical clock.")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/ptp/ptp_clock.c   | 5 +++++
 drivers/ptp/ptp_private.h | 1 +
 drivers/ptp/ptp_sysfs.c   | 6 ++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 4111342d64f0..47ffc773065a 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -508,6 +508,11 @@ static int unregister_vclock(struct device *dev, void *data)
 
 int ptp_clock_unregister(struct ptp_clock *ptp)
 {
+	/*
+	 * Stop and drain virtual-clock creation and deletion before walking the
+	 * children. Do not hold n_vclocks_mux while waiting for sysfs callbacks.
+	 */
+	ptp_vclock_remove_sysfs(ptp);
 	if (ptp_vclock_in_use(ptp)) {
 		device_for_each_child(&ptp->dev, NULL, unregister_vclock);
 	}
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index db4039d642b4..ec8633126d6b 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -169,6 +169,7 @@ extern const struct attribute_group *ptp_groups[];
 
 int ptp_populate_pin_groups(struct ptp_clock *ptp);
 void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
+void ptp_vclock_remove_sysfs(struct ptp_clock *ptp);
 
 struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
 void ptp_vclock_unregister(struct ptp_vclock *vclock);
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index dc398c6b7528..53388b123198 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -263,6 +263,12 @@ static ssize_t n_vclocks_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(n_vclocks);
 
+void ptp_vclock_remove_sysfs(struct ptp_clock *ptp)
+{
+	if (!ptp->is_virtual_clock)
+		device_remove_file(&ptp->dev, &dev_attr_n_vclocks);
+}
+
 static ssize_t max_vclocks_show(struct device *dev,
 				struct device_attribute *attr, char *page)
 {

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net v3 2/5] ptp: unwind partially completed virtual clock creation
  2026-10-13 19:19 [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children James Hilliard
@ 2026-10-13 19:19 ` James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 3/5] ptp: vclock: propagate physical clock sampling errors James Hilliard
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2026-10-13 19:19 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Richard Cochran, Andrew Lunn,
	Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel, James Hilliard

n_vclocks_store() publishes the new clock count only after every child
has been registered. If a later registration fails, it returns without
unregistering the children already created by that request. Those clocks
are left out of n_vclocks, and a subsequent request can overwrite their
index entries without removing them.

Unregister the newly created children in reverse order and clear their
index entries on failure. Leave the previously installed clocks and
their count unchanged. The n_vclocks mutex serializes this rollback with
other sysfs changes, and parent removal drains the store before walking
the children.

This was found by code inspection while adding initial PHC sample error
handling to virtual clock registration.

Fixes: 73f37068d540 ("ptp: support ptp physical/virtual clocks conversion")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/ptp/ptp_sysfs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 53388b123198..fabc1c9b1752 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -225,7 +225,7 @@ static ssize_t n_vclocks_store(struct device *dev,
 		for (i = 0; i < num - ptp->n_vclocks; i++) {
 			vclock = ptp_vclock_register(ptp);
 			if (!vclock)
-				goto out;
+				goto err_register;
 
 			*(ptp->vclock_index + ptp->n_vclocks + i) =
 				vclock->clock->index;
@@ -257,6 +257,12 @@ static ssize_t n_vclocks_store(struct device *dev,
 	mutex_unlock(&ptp->n_vclocks_mux);
 
 	return count;
+err_register:
+	num = i;
+	if (num)
+		device_for_each_child_reverse(dev, &num, unregister_vclock);
+	for (num = 0; num < i; num++)
+		ptp->vclock_index[ptp->n_vclocks + num] = -1;
 out:
 	mutex_unlock(&ptp->n_vclocks_mux);
 	return err;

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net v3 3/5] ptp: vclock: propagate physical clock sampling errors
  2026-10-13 19:19 [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 2/5] ptp: unwind partially completed virtual clock creation James Hilliard
@ 2026-10-13 19:19 ` James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 4/5] ptp: vclock: serialize extended samples with counter updates James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps James Hilliard
  4 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2026-10-13 19:19 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Richard Cochran, Andrew Lunn,
	Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel, James Hilliard

The cyclecounter read callback cannot report errors. ptp_vclock_read()
ignores the return from the physical clock and consumes its output even
if the read failed. A zero sample followed by the real counter can add
an extra 32-bit wrap to virtual time. This was found by code inspection
of reset-time PHC access and the virtual clock callers.

Sample the parent before calling timecounter_read() or timecounter_init()
and have the cyclecounter callback return the cached successful sample.
On failure, leave the counter and frequency adjustment unchanged and
return the PHC error to the caller. The refresh worker can retry later
without consuming an invalid sample.

Take the initial sample and initialize the counter before publishing the
clock. Return an error pointer from registration and propagate its errno
through n_vclocks, including when the initial sample fails.

Keep the existing cyclecounter mask, scaling and refresh interval. This
does not recover elapsed time if sampling is unavailable long enough to
miss a counter wrap, nor does it assume that getcycles64() exposes a
full-width nanosecond counter.

Fixes: 5d43f951b1ac ("ptp: add ptp virtual clock driver framework")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/ptp/ptp_private.h |  3 ++-
 drivers/ptp/ptp_sysfs.c   |  4 ++-
 drivers/ptp/ptp_vclock.c  | 66 +++++++++++++++++++++++++++++++++--------------
 3 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index ec8633126d6b..e389e0653afd 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -81,7 +81,8 @@ struct ptp_vclock {
 	struct hlist_node vclock_hash_node;
 	struct cyclecounter cc;
 	struct timecounter tc;
-	struct mutex lock;	/* protects tc/cc */
+	u64 cycles;
+	struct mutex lock;	/* protects tc/cc and the cached sample */
 };
 
 /*
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index fabc1c9b1752..026220b112c2 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -224,8 +224,10 @@ static ssize_t n_vclocks_store(struct device *dev,
 	if (num > ptp->n_vclocks) {
 		for (i = 0; i < num - ptp->n_vclocks; i++) {
 			vclock = ptp_vclock_register(ptp);
-			if (!vclock)
+			if (IS_ERR(vclock)) {
+				err = PTR_ERR(vclock);
 				goto err_register;
+			}
 
 			*(ptp->vclock_index + ptp->n_vclocks + i) =
 				vclock->clock->index;
diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index 84cb527f59cc..eea207e5a889 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -42,21 +42,39 @@ static void ptp_vclock_hash_del(struct ptp_vclock *vclock)
 	synchronize_srcu(&vclock_srcu);
 }
 
+/* The caller holds vclock->lock, or has not published the clock yet. */
+static int ptp_vclock_sample(struct ptp_vclock *vclock)
+{
+	struct ptp_clock *ptp = vclock->pclock;
+	struct timespec64 ts;
+	int err;
+
+	err = ptp->info->getcycles64(ptp->info, &ts);
+	if (!err)
+		vclock->cycles = timespec64_to_ns(&ts);
+
+	return err;
+}
+
 static int ptp_vclock_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct ptp_vclock *vclock = info_to_vclock(ptp);
 	s64 adj;
+	int err;
 
 	adj = (s64)scaled_ppm << PTP_VCLOCK_FADJ_SHIFT;
 	adj = div_s64(adj, PTP_VCLOCK_FADJ_DENOMINATOR);
 
 	if (mutex_lock_interruptible(&vclock->lock))
 		return -EINTR;
-	timecounter_read(&vclock->tc);
-	vclock->cc.mult = PTP_VCLOCK_CC_MULT + adj;
+	err = ptp_vclock_sample(vclock);
+	if (!err) {
+		timecounter_read(&vclock->tc);
+		vclock->cc.mult = PTP_VCLOCK_CC_MULT + adj;
+	}
 	mutex_unlock(&vclock->lock);
 
-	return 0;
+	return err;
 }
 
 static int ptp_vclock_adjtime(struct ptp_clock_info *ptp, s64 delta)
@@ -75,15 +93,16 @@ static int ptp_vclock_gettime(struct ptp_clock_info *ptp,
 			      struct timespec64 *ts)
 {
 	struct ptp_vclock *vclock = info_to_vclock(ptp);
-	u64 ns;
+	int err;
 
 	if (mutex_lock_interruptible(&vclock->lock))
 		return -EINTR;
-	ns = timecounter_read(&vclock->tc);
+	err = ptp_vclock_sample(vclock);
+	if (!err)
+		*ts = ns_to_timespec64(timecounter_read(&vclock->tc));
 	mutex_unlock(&vclock->lock);
-	*ts = ns_to_timespec64(ns);
 
-	return 0;
+	return err;
 }
 
 static int ptp_vclock_gettimex(struct ptp_clock_info *ptp,
@@ -115,13 +134,16 @@ static int ptp_vclock_settime(struct ptp_clock_info *ptp,
 {
 	struct ptp_vclock *vclock = info_to_vclock(ptp);
 	u64 ns = timespec64_to_ns(ts);
+	int err;
 
 	if (mutex_lock_interruptible(&vclock->lock))
 		return -EINTR;
-	timecounter_init(&vclock->tc, &vclock->cc, ns);
+	err = ptp_vclock_sample(vclock);
+	if (!err)
+		timecounter_init(&vclock->tc, &vclock->cc, ns);
 	mutex_unlock(&vclock->lock);
 
-	return 0;
+	return err;
 }
 
 static int ptp_vclock_getcrosststamp(struct ptp_clock_info *ptp,
@@ -174,12 +196,9 @@ static const struct ptp_clock_info ptp_vclock_info = {
 static u64 ptp_vclock_read(struct cyclecounter *cc)
 {
 	struct ptp_vclock *vclock = cc_to_vclock(cc);
-	struct ptp_clock *ptp = vclock->pclock;
-	struct timespec64 ts = {};
-
-	ptp->info->getcycles64(ptp->info, &ts);
 
-	return timespec64_to_ns(&ts);
+	/* The cyclecounter callback cannot report a failed PHC read. */
+	return vclock->cycles;
 }
 
 static const struct cyclecounter ptp_vclock_cc = {
@@ -192,10 +211,11 @@ static const struct cyclecounter ptp_vclock_cc = {
 struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock)
 {
 	struct ptp_vclock *vclock;
+	int err;
 
 	vclock = kzalloc_obj(*vclock);
 	if (!vclock)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	vclock->pclock = pclock;
 	vclock->info = ptp_vclock_info;
@@ -214,20 +234,28 @@ struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock)
 
 	mutex_init(&vclock->lock);
 
+	err = ptp_vclock_sample(vclock);
+	if (err)
+		goto free_vclock;
+	timecounter_init(&vclock->tc, &vclock->cc, 0);
+
 	vclock->clock = ptp_clock_register(&vclock->info, &pclock->dev);
-	if (IS_ERR_OR_NULL(vclock->clock)) {
-		kfree(vclock);
-		return NULL;
+	if (IS_ERR(vclock->clock)) {
+		err = PTR_ERR(vclock->clock);
+		goto free_vclock;
 	}
 
 	ptp_vclock_set_subclass(vclock->clock);
 
-	timecounter_init(&vclock->tc, &vclock->cc, 0);
 	ptp_schedule_worker(vclock->clock, PTP_VCLOCK_REFRESH_INTERVAL);
 
 	ptp_vclock_hash_add(vclock);
 
 	return vclock;
+
+free_vclock:
+	kfree(vclock);
+	return ERR_PTR(err);
 }
 
 void ptp_vclock_unregister(struct ptp_vclock *vclock)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net v3 4/5] ptp: vclock: serialize extended samples with counter updates
  2026-10-13 19:19 [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe James Hilliard
                   ` (2 preceding siblings ...)
  2026-10-13 19:19 ` [PATCH net v3 3/5] ptp: vclock: propagate physical clock sampling errors James Hilliard
@ 2026-10-13 19:19 ` James Hilliard
  2026-10-13 19:19 ` [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps James Hilliard
  4 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2026-10-13 19:19 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Richard Cochran, Andrew Lunn,
	Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel, James Hilliard

The extended and cross-timestamp callbacks sample the physical clock
before taking the virtual clock mutex. A concurrent frequency adjustment
can advance the timecounter and change its multiplier in between, so the
older sample is converted using a different frequency than the one in
effect when it was taken.

Take the mutex before sampling and keep it held through the conversion.
Retain timecounter_cyc2time() and its existing conversion window. A PHC
sample need not advance the counter: for example, e1000e explicitly
allows non-monotonic SYSTIM readings. Treating a slightly older sample
as a forward delta would instead advance virtual time by nearly a full
counter wrap.

This race was found by code inspection of the virtual clock sampling
paths.

Fixes: f0067ebfc42b ("ptp: add gettimex64() to virtual clocks.")
Fixes: 21fad63084c2 ("ptp: add getcrosststamp() to virtual clocks.")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/ptp/ptp_vclock.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index eea207e5a889..6675466bc307 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -113,20 +113,18 @@ static int ptp_vclock_gettimex(struct ptp_clock_info *ptp,
 	struct ptp_clock *pptp = vclock->pclock;
 	struct timespec64 pts;
 	int err;
-	u64 ns;
-
-	err = pptp->info->getcyclesx64(pptp->info, &pts, sts);
-	if (err)
-		return err;
 
 	if (mutex_lock_interruptible(&vclock->lock))
 		return -EINTR;
-	ns = timecounter_cyc2time(&vclock->tc, timespec64_to_ns(&pts));
-	mutex_unlock(&vclock->lock);
+	err = pptp->info->getcyclesx64(pptp->info, &pts, sts);
+	if (!err) {
+		u64 ns = timecounter_cyc2time(&vclock->tc, timespec64_to_ns(&pts));
 
-	*ts = ns_to_timespec64(ns);
+		*ts = ns_to_timespec64(ns);
+	}
+	mutex_unlock(&vclock->lock);
 
-	return 0;
+	return err;
 }
 
 static int ptp_vclock_settime(struct ptp_clock_info *ptp,
@@ -152,20 +150,18 @@ static int ptp_vclock_getcrosststamp(struct ptp_clock_info *ptp,
 	struct ptp_vclock *vclock = info_to_vclock(ptp);
 	struct ptp_clock *pptp = vclock->pclock;
 	int err;
-	u64 ns;
-
-	err = pptp->info->getcrosscycles(pptp->info, xtstamp);
-	if (err)
-		return err;
 
 	if (mutex_lock_interruptible(&vclock->lock))
 		return -EINTR;
-	ns = timecounter_cyc2time(&vclock->tc, ktime_to_ns(xtstamp->device));
-	mutex_unlock(&vclock->lock);
+	err = pptp->info->getcrosscycles(pptp->info, xtstamp);
+	if (!err) {
+		u64 ns = timecounter_cyc2time(&vclock->tc, ktime_to_ns(xtstamp->device));
 
-	xtstamp->device = ns_to_ktime(ns);
+		xtstamp->device = ns_to_ktime(ns);
+	}
+	mutex_unlock(&vclock->lock);
 
-	return 0;
+	return err;
 }
 
 static long ptp_vclock_refresh(struct ptp_clock_info *ptp)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps
  2026-10-13 19:19 [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe James Hilliard
                   ` (3 preceding siblings ...)
  2026-10-13 19:19 ` [PATCH net v3 4/5] ptp: vclock: serialize extended samples with counter updates James Hilliard
@ 2026-10-13 19:19 ` James Hilliard
  2026-10-13 19:52   ` Vadim Fedorenko
  4 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2026-10-13 19:19 UTC (permalink / raw)
  To: netdev, Paolo Abeni, Jakub Kicinski, Richard Cochran, Andrew Lunn,
	Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel, James Hilliard

Socket timestamp delivery converts hardware timestamps to a bound
virtual clock before checking whether a hardware timestamp is present.
A zero timestamp means that the packet has no hardware timestamp, but
timecounter_cyc2time() can turn that zero into a nonzero virtual time.
The socket then reports a hardware timestamp which was never supplied.

Return zero without conversion when the input timestamp is absent.
This also covers skb_get_tx_timestamp(), which checks for an absent
timestamp after the same conversion.

This was identified during review of virtual clock sampling changes.

Fixes: d7c088265588 ("net: socket: support hardware timestamp conversion to PHC bound")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/ptp/ptp_vclock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index 6675466bc307..4a62047f0ce0 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -307,6 +307,8 @@ ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp, int vclock_index)
 	u64 ns;
 
 	ns = ktime_to_ns(*hwtstamp);
+	if (!ns)
+		return 0;
 
 	srcu_idx = srcu_read_lock(&vclock_srcu);
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children
  2026-10-13 19:19 ` [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children James Hilliard
@ 2026-10-13 19:46   ` Vadim Fedorenko
  0 siblings, 0 replies; 8+ messages in thread
From: Vadim Fedorenko @ 2026-10-13 19:46 UTC (permalink / raw)
  To: James Hilliard, netdev, Paolo Abeni, Jakub Kicinski,
	Richard Cochran, Andrew Lunn, Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel

On 13/10/2026 20:19, James Hilliard wrote:
> Parent clock removal walks its virtual clocks before removing the
> n_vclocks sysfs attribute. The mutex taken by ptp_vclock_in_use() is
> released before that walk, so a concurrent sysfs deletion can pick the
> same child and unregister and free its ptp_vclock a second time. A
> reference held by the device iterator does not protect that separately
> allocated virtual clock.
> 
> Remove n_vclocks before walking the children. Removing the attribute
> prevents new stores and drains stores already running, without holding
> n_vclocks_mux across a callback that needs that mutex. Skip virtual
> clocks, which do not have this attribute.
> 
> This race was found by code inspection of virtual-clock registration
> failure cleanup and parent removal.
> 
> Fixes: bfcbb76b0f59 ("ptp: unregister virtual clocks when unregistering physical clock.")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   drivers/ptp/ptp_clock.c   | 5 +++++
>   drivers/ptp/ptp_private.h | 1 +
>   drivers/ptp/ptp_sysfs.c   | 6 ++++++
>   3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index 4111342d64f0..47ffc773065a 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -508,6 +508,11 @@ static int unregister_vclock(struct device *dev, void *data)
>   
>   int ptp_clock_unregister(struct ptp_clock *ptp)
>   {
> +	/*
> +	 * Stop and drain virtual-clock creation and deletion before walking the
> +	 * children. Do not hold n_vclocks_mux while waiting for sysfs callbacks.
> +	 */
> +	ptp_vclock_remove_sysfs(ptp);
>   	if (ptp_vclock_in_use(ptp)) {
>   		device_for_each_child(&ptp->dev, NULL, unregister_vclock);
>   	}
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> index db4039d642b4..ec8633126d6b 100644
> --- a/drivers/ptp/ptp_private.h
> +++ b/drivers/ptp/ptp_private.h
> @@ -169,6 +169,7 @@ extern const struct attribute_group *ptp_groups[];
>   
>   int ptp_populate_pin_groups(struct ptp_clock *ptp);
>   void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
> +void ptp_vclock_remove_sysfs(struct ptp_clock *ptp);
>   
>   struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
>   void ptp_vclock_unregister(struct ptp_vclock *vclock);
> diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
> index dc398c6b7528..53388b123198 100644
> --- a/drivers/ptp/ptp_sysfs.c
> +++ b/drivers/ptp/ptp_sysfs.c
> @@ -263,6 +263,12 @@ static ssize_t n_vclocks_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(n_vclocks);
>   
> +void ptp_vclock_remove_sysfs(struct ptp_clock *ptp)
> +{
> +	if (!ptp->is_virtual_clock)
> +		device_remove_file(&ptp->dev, &dev_attr_n_vclocks);
> +}
> +
>   static ssize_t max_vclocks_show(struct device *dev,
>   				struct device_attribute *attr, char *page)
>   {
> 

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps
  2026-10-13 19:19 ` [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps James Hilliard
@ 2026-10-13 19:52   ` Vadim Fedorenko
  0 siblings, 0 replies; 8+ messages in thread
From: Vadim Fedorenko @ 2026-10-13 19:52 UTC (permalink / raw)
  To: James Hilliard, netdev, Paolo Abeni, Jakub Kicinski,
	Richard Cochran, Andrew Lunn, Yangbo Lu, Miroslav Lichvar
  Cc: Eric Dumazet, David S. Miller, linux-kernel

On 13/10/2026 20:19, James Hilliard wrote:
> Socket timestamp delivery converts hardware timestamps to a bound
> virtual clock before checking whether a hardware timestamp is present.
> A zero timestamp means that the packet has no hardware timestamp, but
> timecounter_cyc2time() can turn that zero into a nonzero virtual time.
> The socket then reports a hardware timestamp which was never supplied.
> 
> Return zero without conversion when the input timestamp is absent.
> This also covers skb_get_tx_timestamp(), which checks for an absent
> timestamp after the same conversion.
> 
> This was identified during review of virtual clock sampling changes.
> 
> Fixes: d7c088265588 ("net: socket: support hardware timestamp conversion to PHC bound")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   drivers/ptp/ptp_vclock.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
> index 6675466bc307..4a62047f0ce0 100644
> --- a/drivers/ptp/ptp_vclock.c
> +++ b/drivers/ptp/ptp_vclock.c
> @@ -307,6 +307,8 @@ ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp, int vclock_index)
>   	u64 ns;
>   
>   	ns = ktime_to_ns(*hwtstamp);
> +	if (!ns)
> +		return 0;
>   
>   	srcu_idx = srcu_read_lock(&vclock_srcu);
>   
> 

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-13 19:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 19:19 [PATCH net v3 0/5] ptp: make virtual clock sampling and teardown failure-safe James Hilliard
2026-10-13 19:19 ` [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children James Hilliard
2026-10-13 19:46   ` Vadim Fedorenko
2026-10-13 19:19 ` [PATCH net v3 2/5] ptp: unwind partially completed virtual clock creation James Hilliard
2026-10-13 19:19 ` [PATCH net v3 3/5] ptp: vclock: propagate physical clock sampling errors James Hilliard
2026-10-13 19:19 ` [PATCH net v3 4/5] ptp: vclock: serialize extended samples with counter updates James Hilliard
2026-10-13 19:19 ` [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps James Hilliard
2026-10-13 19:52   ` Vadim Fedorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox