2021-06-30 08:02:43

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 00/11] ptp: support virtual clocks and timestamping

Current PTP driver exposes one PTP device to user which binds network
interface/interfaces to provide timestamping. Actually we have a way
utilizing timecounter/cyclecounter to virtualize any number of PTP
clocks based on a same free running physical clock for using.
The purpose of having multiple PTP virtual clocks is for user space
to directly/easily use them for multiple domains synchronization.

user
space: ^ ^
| SO_TIMESTAMPING new flag: | Packets with
| SOF_TIMESTAMPING_BIND_PHC | TX/RX HW timestamps
v v
+--------------------------------------------+
sock: | sock (new member sk_bind_phc) |
+--------------------------------------------+
^ ^
| ethtool_get_phc_vclocks | Convert HW timestamps
| | to sk_bind_phc
v v
+--------------+--------------+--------------+
vclock: | ptp1 | ptp2 | ptpN |
+--------------+--------------+--------------+
pclock: | ptp0 free running |
+--------------------------------------------+

The block diagram may explain how it works. Besides the PTP virtual
clocks, the packet HW timestamp converting to the bound PHC is also
done in sock driver. For user space, PTP virtual clocks can be
created via sysfs, and extended SO_TIMESTAMPING API (new flag
SOF_TIMESTAMPING_BIND_PHC) can be used to bind one PTP virtual clock
for timestamping.

The test tool timestamping.c (together with linuxptp phc_ctl tool) can
be used to verify:

# echo 4 > /sys/class/ptp/ptp0/n_vclocks
[ 129.399472] ptp ptp0: new virtual clock ptp2
[ 129.404234] ptp ptp0: new virtual clock ptp3
[ 129.409532] ptp ptp0: new virtual clock ptp4
[ 129.413942] ptp ptp0: new virtual clock ptp5
[ 129.418257] ptp ptp0: guarantee physical clock free running
#
# phc_ctl /dev/ptp2 set 10000
# phc_ctl /dev/ptp3 set 20000
#
# timestamping eno0 2 SOF_TIMESTAMPING_TX_HARDWARE SOF_TIMESTAMPING_RAW_HARDWARE SOF_TIMESTAMPING_BIND_PHC
# timestamping eno0 2 SOF_TIMESTAMPING_RX_HARDWARE SOF_TIMESTAMPING_RAW_HARDWARE SOF_TIMESTAMPING_BIND_PHC
# timestamping eno0 3 SOF_TIMESTAMPING_TX_HARDWARE SOF_TIMESTAMPING_RAW_HARDWARE SOF_TIMESTAMPING_BIND_PHC
# timestamping eno0 3 SOF_TIMESTAMPING_RX_HARDWARE SOF_TIMESTAMPING_RAW_HARDWARE SOF_TIMESTAMPING_BIND_PHC

Changes for v2:
- Converted to num_vclocks for creating virtual clocks.
- Guranteed physical clock free running when using virtual
clocks.
- Fixed build warning.
- Updated copyright.
Changes for v3:
- Supported PTP virtual clock in default in PTP driver.
- Protected concurrency of ptp->num_vclocks accessing.
- Supported PHC vclocks query via ethtool.
- Extended SO_TIMESTAMPING API for PHC binding.
- Converted HW timestamps to PHC bound, instead of previous
binding domain value to PHC idea.
- Other minor fixes.
Changes for v4:
- Used do_aux_work callback for vclock refreshing instead.
- Used unsigned int for vclocks number, and max_vclocks
for limitiation.
- Fixed mutex locking.
- Dynamically allocated memory for vclock index storage.
- Removed ethtool ioctl command for vclocks getting.
- Updated doc for ethtool phc vclocks get.
- Converted to mptcp_setsockopt_sol_socket_timestamping().
- Passed so_timestamping for sock_set_timestamping.
- Fixed checkpatch/build.
- Other minor fixed.
Changes for v5:
- Fixed checkpatch/build/bug reported by test robot.

Yangbo Lu (11):
ptp: add ptp virtual clock driver framework
ptp: support ptp physical/virtual clocks conversion
ptp: track available ptp vclocks information
ptp: add kernel API ptp_get_vclocks_index()
ethtool: add a new command for getting PHC virtual clocks
ptp: add kernel API ptp_convert_timestamp()
mptcp: setsockopt: convert to
mptcp_setsockopt_sol_socket_timestamping()
net: sock: extend SO_TIMESTAMPING for PHC binding
net: socket: support hardware timestamp conversion to PHC bound
selftests/net: timestamping: support binding PHC
MAINTAINERS: add entry for PTP virtual clock driver

Documentation/ABI/testing/sysfs-ptp | 20 ++
Documentation/networking/ethtool-netlink.rst | 22 ++
MAINTAINERS | 7 +
drivers/ptp/Makefile | 2 +-
drivers/ptp/ptp_clock.c | 42 +++-
drivers/ptp/ptp_private.h | 39 ++++
drivers/ptp/ptp_sysfs.c | 160 ++++++++++++++
drivers/ptp/ptp_vclock.c | 219 +++++++++++++++++++
include/linux/ethtool.h | 10 +
include/linux/ptp_clock_kernel.h | 31 ++-
include/net/sock.h | 8 +-
include/uapi/linux/ethtool_netlink.h | 15 ++
include/uapi/linux/net_tstamp.h | 17 +-
net/core/sock.c | 65 +++++-
net/ethtool/Makefile | 2 +-
net/ethtool/common.c | 14 ++
net/ethtool/netlink.c | 10 +
net/ethtool/netlink.h | 2 +
net/ethtool/phc_vclocks.c | 94 ++++++++
net/mptcp/sockopt.c | 68 ++++--
net/socket.c | 19 +-
tools/testing/selftests/net/timestamping.c | 55 +++--
22 files changed, 867 insertions(+), 54 deletions(-)
create mode 100644 drivers/ptp/ptp_vclock.c
create mode 100644 net/ethtool/phc_vclocks.c


base-commit: b6df00789e2831fff7a2c65aa7164b2a4dcbe599
--
2.25.1


2021-06-30 08:02:54

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 02/11] ptp: support ptp physical/virtual clocks conversion

Support ptp physical/virtual clocks conversion via sysfs.
There will be a new attribute n_vclocks under ptp physical
clock sysfs.

- In default, the value is 0 meaning only ptp physical clock
is in use.
- Setting the value can create corresponding number of ptp
virtual clocks to use. But current physical clock is guaranteed
to stay free running.
- Setting the value back to 0 can delete virtual clocks and back
use physical clock again.

Another new attribute max_vclocks control the maximum number of
ptp vclocks.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v2:
- Split from v1 patch #2.
- Converted to num_vclocks for creating virtual clocks.
- Guranteed physical clock free running when using virtual
clocks.
- Fixed build warning.
- Updated copyright.
Changes for v3:
- Protected concurrency of ptp->num_vclocks accessing.
Changes for v4:
- Rephrased description in doc.
- Used unsigned int for vclocks number, and max_vclocks
for limitiation.
- Fixed mutex locking.
- Other minor fixes.
Changes for v5:
- Fixed checkpatch.
- Checked pointer parent->class->name.
---
Documentation/ABI/testing/sysfs-ptp | 20 ++++
drivers/ptp/ptp_clock.c | 26 ++++++
drivers/ptp/ptp_private.h | 21 +++++
drivers/ptp/ptp_sysfs.c | 138 ++++++++++++++++++++++++++++
4 files changed, 205 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index 2363ad810ddb..d378f57c1b73 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -33,6 +33,13 @@ Description:
frequency adjustment value (a positive integer) in
parts per billion.

+What: /sys/class/ptp/ptpN/max_vclocks
+Date: May 2021
+Contact: Yangbo Lu <[email protected]>
+Description:
+ This file contains the maximum number of ptp vclocks.
+ Write integer to re-configure it.
+
What: /sys/class/ptp/ptpN/n_alarms
Date: September 2010
Contact: Richard Cochran <[email protected]>
@@ -61,6 +68,19 @@ Description:
This file contains the number of programmable pins
offered by the PTP hardware clock.

+What: /sys/class/ptp/ptpN/n_vclocks
+Date: May 2021
+Contact: Yangbo Lu <[email protected]>
+Description:
+ This file contains the number of virtual PTP clocks in
+ use. By default, the value is 0 meaning that only the
+ physical clock is in use. Setting the value creates
+ the corresponding number of virtual clocks and causes
+ the physical clock to become free running. Setting the
+ value back to 0 deletes the virtual clocks and
+ switches the physical clock back to normal, adjustable
+ operation.
+
What: /sys/class/ptp/ptpN/pins
Date: March 2014
Contact: Richard Cochran <[email protected]>
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index a23a37a4d5dc..7334f478dde7 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -76,6 +76,11 @@ static int ptp_clock_settime(struct posix_clock *pc, const struct timespec64 *tp
{
struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);

+ if (ptp_vclock_in_use(ptp)) {
+ pr_err("ptp: virtual clock in use\n");
+ return -EBUSY;
+ }
+
return ptp->info->settime64(ptp->info, tp);
}

@@ -97,6 +102,11 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
struct ptp_clock_info *ops;
int err = -EOPNOTSUPP;

+ if (ptp_vclock_in_use(ptp)) {
+ pr_err("ptp: virtual clock in use\n");
+ return -EBUSY;
+ }
+
ops = ptp->info;

if (tx->modes & ADJ_SETOFFSET) {
@@ -161,6 +171,7 @@ static void ptp_clock_release(struct device *dev)
ptp_cleanup_pin_groups(ptp);
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
+ mutex_destroy(&ptp->n_vclocks_mux);
ida_simple_remove(&ptp_clocks_map, ptp->index);
kfree(ptp);
}
@@ -208,6 +219,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
spin_lock_init(&ptp->tsevq.lock);
mutex_init(&ptp->tsevq_mux);
mutex_init(&ptp->pincfg_mux);
+ mutex_init(&ptp->n_vclocks_mux);
init_waitqueue_head(&ptp->tsev_wq);

if (ptp->info->do_aux_work) {
@@ -221,6 +233,14 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
ptp->pps_source->lookup_cookie = ptp;
}

+ /* PTP virtual clock is being registered under physical clock */
+ if (parent->class && parent->class->name &&
+ strcmp(parent->class->name, "ptp") == 0)
+ ptp->is_virtual_clock = true;
+
+ if (!ptp->is_virtual_clock)
+ ptp->max_vclocks = PTP_DEFAULT_MAX_VCLOCKS;
+
err = ptp_populate_pin_groups(ptp);
if (err)
goto no_pin_groups;
@@ -270,6 +290,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
kworker_err:
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
+ mutex_destroy(&ptp->n_vclocks_mux);
ida_simple_remove(&ptp_clocks_map, index);
no_slot:
kfree(ptp);
@@ -280,6 +301,11 @@ EXPORT_SYMBOL(ptp_clock_register);

int ptp_clock_unregister(struct ptp_clock *ptp)
{
+ if (ptp_vclock_in_use(ptp)) {
+ pr_err("ptp: virtual clock in use\n");
+ return -EBUSY;
+ }
+
ptp->defunct = 1;
wake_up_interruptible(&ptp->tsev_wq);

diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 853b79b6b30e..87cb55953b69 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -18,6 +18,7 @@

#define PTP_MAX_TIMESTAMPS 128
#define PTP_BUF_TIMESTAMPS 30
+#define PTP_DEFAULT_MAX_VCLOCKS 20

struct timestamp_event_queue {
struct ptp_extts_event buf[PTP_MAX_TIMESTAMPS];
@@ -46,6 +47,10 @@ struct ptp_clock {
const struct attribute_group *pin_attr_groups[2];
struct kthread_worker *kworker;
struct kthread_delayed_work aux_work;
+ unsigned int max_vclocks;
+ unsigned int n_vclocks;
+ struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
+ bool is_virtual_clock;
};

#define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
@@ -74,6 +79,22 @@ static inline int queue_cnt(struct timestamp_event_queue *q)
return cnt < 0 ? PTP_MAX_TIMESTAMPS + cnt : cnt;
}

+/* Check if ptp virtual clock is in use */
+static inline bool ptp_vclock_in_use(struct ptp_clock *ptp)
+{
+ bool in_use = false;
+
+ if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
+ return true;
+
+ if (!ptp->is_virtual_clock && ptp->n_vclocks)
+ in_use = true;
+
+ mutex_unlock(&ptp->n_vclocks_mux);
+
+ return in_use;
+}
+
/*
* see ptp_chardev.c
*/
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index be076a91e20e..0b05041783a5 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -3,6 +3,7 @@
* PTP 1588 clock support - sysfs interface.
*
* Copyright (C) 2010 OMICRON electronics GmbH
+ * Copyright 2021 NXP
*/
#include <linux/capability.h>
#include <linux/slab.h>
@@ -148,6 +149,137 @@ static ssize_t pps_enable_store(struct device *dev,
}
static DEVICE_ATTR(pps_enable, 0220, NULL, pps_enable_store);

+static int unregister_vclock(struct device *dev, void *data)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ struct ptp_clock_info *info = ptp->info;
+ struct ptp_vclock *vclock;
+ u8 *num = data;
+
+ vclock = info_to_vclock(info);
+ dev_info(dev->parent, "delete virtual clock ptp%d\n",
+ vclock->clock->index);
+
+ ptp_vclock_unregister(vclock);
+ (*num)--;
+
+ /* For break. Not error. */
+ if (*num == 0)
+ return -EINVAL;
+
+ return 0;
+}
+
+static ssize_t n_vclocks_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ ssize_t size;
+
+ if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
+ return -ERESTARTSYS;
+
+ size = snprintf(page, PAGE_SIZE - 1, "%d\n", ptp->n_vclocks);
+
+ mutex_unlock(&ptp->n_vclocks_mux);
+
+ return size;
+}
+
+static ssize_t n_vclocks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ struct ptp_vclock *vclock;
+ int err = -EINVAL;
+ u32 num, i;
+
+ if (kstrtou32(buf, 0, &num))
+ return err;
+
+ if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
+ return -ERESTARTSYS;
+
+ if (num > ptp->max_vclocks) {
+ dev_err(dev, "max value is %d\n", ptp->max_vclocks);
+ goto out;
+ }
+
+ /* Need to create more vclocks */
+ if (num > ptp->n_vclocks) {
+ for (i = 0; i < num - ptp->n_vclocks; i++) {
+ vclock = ptp_vclock_register(ptp);
+ if (!vclock)
+ goto out;
+
+ dev_info(dev, "new virtual clock ptp%d\n",
+ vclock->clock->index);
+ }
+ }
+
+ /* Need to delete vclocks */
+ if (num < ptp->n_vclocks) {
+ i = ptp->n_vclocks - num;
+ device_for_each_child_reverse(dev, &i,
+ unregister_vclock);
+ }
+
+ if (num == 0)
+ dev_info(dev, "only physical clock in use now\n");
+ else
+ dev_info(dev, "guarantee physical clock free running\n");
+
+ ptp->n_vclocks = num;
+ mutex_unlock(&ptp->n_vclocks_mux);
+
+ return count;
+out:
+ mutex_unlock(&ptp->n_vclocks_mux);
+ return err;
+}
+static DEVICE_ATTR_RW(n_vclocks);
+
+static ssize_t max_vclocks_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ ssize_t size;
+
+ size = snprintf(page, PAGE_SIZE - 1, "%d\n", ptp->max_vclocks);
+
+ return size;
+}
+
+static ssize_t max_vclocks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ptp_clock *ptp = dev_get_drvdata(dev);
+ u32 max;
+
+ if (kstrtou32(buf, 0, &max) || max == 0)
+ return -EINVAL;
+
+ if (max == ptp->max_vclocks)
+ return count;
+
+ if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
+ return -ERESTARTSYS;
+
+ if (max < ptp->n_vclocks) {
+ mutex_unlock(&ptp->n_vclocks_mux);
+ return -EINVAL;
+ }
+
+ ptp->max_vclocks = max;
+
+ mutex_unlock(&ptp->n_vclocks_mux);
+
+ return count;
+}
+static DEVICE_ATTR_RW(max_vclocks);
+
static struct attribute *ptp_attrs[] = {
&dev_attr_clock_name.attr,

@@ -162,6 +294,8 @@ static struct attribute *ptp_attrs[] = {
&dev_attr_fifo.attr,
&dev_attr_period.attr,
&dev_attr_pps_enable.attr,
+ &dev_attr_n_vclocks.attr,
+ &dev_attr_max_vclocks.attr,
NULL
};

@@ -183,6 +317,10 @@ static umode_t ptp_is_attribute_visible(struct kobject *kobj,
} else if (attr == &dev_attr_pps_enable.attr) {
if (!info->pps)
mode = 0;
+ } else if (attr == &dev_attr_n_vclocks.attr ||
+ attr == &dev_attr_max_vclocks.attr) {
+ if (ptp->is_virtual_clock)
+ mode = 0;
}

return mode;
--
2.25.1

2021-06-30 08:03:03

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 03/11] ptp: track available ptp vclocks information

Track available ptp vclocks information. Record index values
of available ptp vclocks during registering and unregistering.

This is preparation for supporting ptp vclocks info query
through ethtool.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Dynamically allocated memory for vclock index storage.
Changes for v5:
- None.
---
drivers/ptp/ptp_clock.c | 15 ++++++++++++++-
drivers/ptp/ptp_private.h | 1 +
drivers/ptp/ptp_sysfs.c | 28 +++++++++++++++++++++++++---
3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 7334f478dde7..9205a9362a9d 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -196,6 +196,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
{
struct ptp_clock *ptp;
int err = 0, index, major = MAJOR(ptp_devt);
+ size_t size;

if (info->n_alarm > PTP_MAX_ALARMS)
return ERR_PTR(-EINVAL);
@@ -238,9 +239,17 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
strcmp(parent->class->name, "ptp") == 0)
ptp->is_virtual_clock = true;

- if (!ptp->is_virtual_clock)
+ if (!ptp->is_virtual_clock) {
ptp->max_vclocks = PTP_DEFAULT_MAX_VCLOCKS;

+ size = sizeof(int) * ptp->max_vclocks;
+ ptp->vclock_index = kzalloc(size, GFP_KERNEL);
+ if (!ptp->vclock_index) {
+ err = -ENOMEM;
+ goto no_mem_for_vclocks;
+ }
+ }
+
err = ptp_populate_pin_groups(ptp);
if (err)
goto no_pin_groups;
@@ -285,6 +294,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
no_pps:
ptp_cleanup_pin_groups(ptp);
no_pin_groups:
+ kfree(ptp->vclock_index);
+no_mem_for_vclocks:
if (ptp->kworker)
kthread_destroy_worker(ptp->kworker);
kworker_err:
@@ -309,6 +320,8 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
ptp->defunct = 1;
wake_up_interruptible(&ptp->tsev_wq);

+ kfree(ptp->vclock_index);
+
if (ptp->kworker) {
kthread_cancel_delayed_work_sync(&ptp->aux_work);
kthread_destroy_worker(ptp->kworker);
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 87cb55953b69..f75fadd9b244 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -49,6 +49,7 @@ struct ptp_clock {
struct kthread_delayed_work aux_work;
unsigned int max_vclocks;
unsigned int n_vclocks;
+ int *vclock_index;
struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
bool is_virtual_clock;
};
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 0b05041783a5..6a36590ca77a 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -213,6 +213,9 @@ static ssize_t n_vclocks_store(struct device *dev,
if (!vclock)
goto out;

+ *(ptp->vclock_index + ptp->n_vclocks + i) =
+ vclock->clock->index;
+
dev_info(dev, "new virtual clock ptp%d\n",
vclock->clock->index);
}
@@ -223,6 +226,9 @@ static ssize_t n_vclocks_store(struct device *dev,
i = ptp->n_vclocks - num;
device_for_each_child_reverse(dev, &i,
unregister_vclock);
+
+ for (i = 1; i <= ptp->n_vclocks - num; i++)
+ *(ptp->vclock_index + ptp->n_vclocks - i) = -1;
}

if (num == 0)
@@ -256,6 +262,9 @@ static ssize_t max_vclocks_store(struct device *dev,
const char *buf, size_t count)
{
struct ptp_clock *ptp = dev_get_drvdata(dev);
+ unsigned int *vclock_index;
+ int err = -EINVAL;
+ size_t size;
u32 max;

if (kstrtou32(buf, 0, &max) || max == 0)
@@ -267,16 +276,29 @@ static ssize_t max_vclocks_store(struct device *dev,
if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
return -ERESTARTSYS;

- if (max < ptp->n_vclocks) {
- mutex_unlock(&ptp->n_vclocks_mux);
- return -EINVAL;
+ if (max < ptp->n_vclocks)
+ goto out;
+
+ size = sizeof(int) * max;
+ vclock_index = kzalloc(size, GFP_KERNEL);
+ if (!vclock_index) {
+ err = -ENOMEM;
+ goto out;
}

+ size = sizeof(int) * ptp->n_vclocks;
+ memcpy(vclock_index, ptp->vclock_index, size);
+
+ kfree(ptp->vclock_index);
+ ptp->vclock_index = vclock_index;
ptp->max_vclocks = max;

mutex_unlock(&ptp->n_vclocks_mux);

return count;
+out:
+ mutex_unlock(&ptp->n_vclocks_mux);
+ return err;
}
static DEVICE_ATTR_RW(max_vclocks);

--
2.25.1

2021-06-30 08:03:22

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 05/11] ethtool: add a new command for getting PHC virtual clocks

Add an interface for getting PHC (PTP Hardware Clock)
virtual clocks, which are based on PHC physical clock
providing hardware timestamp to network packets.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Updated doc.
- Removed ioctl command.
- Replied only the number of vclock index.
Changes for v5:
- None.
---
Documentation/networking/ethtool-netlink.rst | 22 +++++
include/linux/ethtool.h | 10 +++
include/uapi/linux/ethtool_netlink.h | 15 ++++
net/ethtool/Makefile | 2 +-
net/ethtool/common.c | 13 +++
net/ethtool/netlink.c | 10 +++
net/ethtool/netlink.h | 2 +
net/ethtool/phc_vclocks.c | 94 ++++++++++++++++++++
8 files changed, 167 insertions(+), 1 deletion(-)
create mode 100644 net/ethtool/phc_vclocks.c

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 6ea91e41593f..c86628e6a235 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -212,6 +212,7 @@ Userspace to kernel:
``ETHTOOL_MSG_FEC_SET`` set FEC settings
``ETHTOOL_MSG_MODULE_EEPROM_GET`` read SFP module EEPROM
``ETHTOOL_MSG_STATS_GET`` get standard statistics
+ ``ETHTOOL_MSG_PHC_VCLOCKS_GET`` get PHC virtual clocks info
===================================== ================================

Kernel to userspace:
@@ -250,6 +251,7 @@ Kernel to userspace:
``ETHTOOL_MSG_FEC_NTF`` FEC settings
``ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY`` read SFP module EEPROM
``ETHTOOL_MSG_STATS_GET_REPLY`` standard statistics
+ ``ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY`` PHC virtual clocks info
======================================== =================================

``GET`` requests are sent by userspace applications to retrieve device
@@ -1477,6 +1479,25 @@ Low and high bounds are inclusive, for example:
etherStatsPkts512to1023Octets 512 1023
============================= ==== ====

+PHC_VCLOCKS_GET
+===============
+
+Query device PHC virtual clocks information.
+
+Request contents:
+
+ ==================================== ====== ==========================
+ ``ETHTOOL_A_PHC_VCLOCKS_HEADER`` nested request header
+ ==================================== ====== ==========================
+
+Kernel response contents:
+
+ ==================================== ====== ==========================
+ ``ETHTOOL_A_PHC_VCLOCKS_HEADER`` nested reply header
+ ``ETHTOOL_A_PHC_VCLOCKS_NUM`` u32 PHC virtual clocks number
+ ``ETHTOOL_A_PHC_VCLOCKS_INDEX`` s32 PHC index array
+ ==================================== ====== ==========================
+
Request translation
===================

@@ -1575,4 +1596,5 @@ are netlink only.
n/a ``ETHTOOL_MSG_CABLE_TEST_ACT``
n/a ``ETHTOOL_MSG_CABLE_TEST_TDR_ACT``
n/a ``ETHTOOL_MSG_TUNNEL_INFO_GET``
+ n/a ``ETHTOOL_MSG_PHC_VCLOCKS_GET``
=================================== =====================================
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 29dbb603bc91..232daaec56e4 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -757,6 +757,16 @@ void
ethtool_params_from_link_mode(struct ethtool_link_ksettings *link_ksettings,
enum ethtool_link_mode_bit_indices link_mode);

+/**
+ * ethtool_get_phc_vclocks - Derive phc vclocks information, and caller
+ * is responsible to free memory of vclock_index
+ * @dev: pointer to net_device structure
+ * @vclock_index: pointer to pointer of vclock index
+ *
+ * Return number of phc vclocks
+ */
+int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index);
+
/**
* ethtool_sprintf - Write formatted string to ethtool string data
* @data: Pointer to start of string to update
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index c7135c9c37a5..b3b93710eff7 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -46,6 +46,7 @@ enum {
ETHTOOL_MSG_FEC_SET,
ETHTOOL_MSG_MODULE_EEPROM_GET,
ETHTOOL_MSG_STATS_GET,
+ ETHTOOL_MSG_PHC_VCLOCKS_GET,

/* add new constants above here */
__ETHTOOL_MSG_USER_CNT,
@@ -88,6 +89,7 @@ enum {
ETHTOOL_MSG_FEC_NTF,
ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY,
ETHTOOL_MSG_STATS_GET_REPLY,
+ ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY,

/* add new constants above here */
__ETHTOOL_MSG_KERNEL_CNT,
@@ -440,6 +442,19 @@ enum {
ETHTOOL_A_TSINFO_MAX = (__ETHTOOL_A_TSINFO_CNT - 1)
};

+/* PHC VCLOCKS */
+
+enum {
+ ETHTOOL_A_PHC_VCLOCKS_UNSPEC,
+ ETHTOOL_A_PHC_VCLOCKS_HEADER, /* nest - _A_HEADER_* */
+ ETHTOOL_A_PHC_VCLOCKS_NUM, /* u32 */
+ ETHTOOL_A_PHC_VCLOCKS_INDEX, /* array, s32 */
+
+ /* add new constants above here */
+ __ETHTOOL_A_PHC_VCLOCKS_CNT,
+ ETHTOOL_A_PHC_VCLOCKS_MAX = (__ETHTOOL_A_PHC_VCLOCKS_CNT - 1)
+};
+
/* CABLE TEST */

enum {
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 723c9a8a8cdf..0a19470efbfb 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -7,4 +7,4 @@ obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o
ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o \
linkstate.o debug.o wol.o features.o privflags.o rings.o \
channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
- tunnels.o fec.o eeprom.o stats.o
+ tunnels.o fec.o eeprom.o stats.o phc_vclocks.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index f9dcbad84788..798231b07676 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -4,6 +4,7 @@
#include <linux/net_tstamp.h>
#include <linux/phy.h>
#include <linux/rtnetlink.h>
+#include <linux/ptp_clock_kernel.h>

#include "common.h"

@@ -554,6 +555,18 @@ int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
return 0;
}

+int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index)
+{
+ struct ethtool_ts_info info = { };
+ int num = 0;
+
+ if (!__ethtool_get_ts_info(dev, &info))
+ num = ptp_get_vclocks_index(info.phc_index, vclock_index);
+
+ return num;
+}
+EXPORT_SYMBOL(ethtool_get_phc_vclocks);
+
const struct ethtool_phy_ops *ethtool_phy_ops;

void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index a7346346114f..73e0f5b626bf 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -248,6 +248,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
[ETHTOOL_MSG_TSINFO_GET] = &ethnl_tsinfo_request_ops,
[ETHTOOL_MSG_MODULE_EEPROM_GET] = &ethnl_module_eeprom_request_ops,
[ETHTOOL_MSG_STATS_GET] = &ethnl_stats_request_ops,
+ [ETHTOOL_MSG_PHC_VCLOCKS_GET] = &ethnl_phc_vclocks_request_ops,
};

static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -958,6 +959,15 @@ static const struct genl_ops ethtool_genl_ops[] = {
.policy = ethnl_stats_get_policy,
.maxattr = ARRAY_SIZE(ethnl_stats_get_policy) - 1,
},
+ {
+ .cmd = ETHTOOL_MSG_PHC_VCLOCKS_GET,
+ .doit = ethnl_default_doit,
+ .start = ethnl_default_start,
+ .dumpit = ethnl_default_dumpit,
+ .done = ethnl_default_done,
+ .policy = ethnl_phc_vclocks_get_policy,
+ .maxattr = ARRAY_SIZE(ethnl_phc_vclocks_get_policy) - 1,
+ },
};

static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 3e25a47fd482..3fc395c86702 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -347,6 +347,7 @@ extern const struct ethnl_request_ops ethnl_tsinfo_request_ops;
extern const struct ethnl_request_ops ethnl_fec_request_ops;
extern const struct ethnl_request_ops ethnl_module_eeprom_request_ops;
extern const struct ethnl_request_ops ethnl_stats_request_ops;
+extern const struct ethnl_request_ops ethnl_phc_vclocks_request_ops;

extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
@@ -382,6 +383,7 @@ extern const struct nla_policy ethnl_fec_get_policy[ETHTOOL_A_FEC_HEADER + 1];
extern const struct nla_policy ethnl_fec_set_policy[ETHTOOL_A_FEC_AUTO + 1];
extern const struct nla_policy ethnl_module_eeprom_get_policy[ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS + 1];
extern const struct nla_policy ethnl_stats_get_policy[ETHTOOL_A_STATS_GROUPS + 1];
+extern const struct nla_policy ethnl_phc_vclocks_get_policy[ETHTOOL_A_PHC_VCLOCKS_HEADER + 1];

int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/ethtool/phc_vclocks.c b/net/ethtool/phc_vclocks.c
new file mode 100644
index 000000000000..637b2f5297d5
--- /dev/null
+++ b/net/ethtool/phc_vclocks.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2021 NXP
+ */
+#include "netlink.h"
+#include "common.h"
+
+struct phc_vclocks_req_info {
+ struct ethnl_req_info base;
+};
+
+struct phc_vclocks_reply_data {
+ struct ethnl_reply_data base;
+ int num;
+ int *index;
+};
+
+#define PHC_VCLOCKS_REPDATA(__reply_base) \
+ container_of(__reply_base, struct phc_vclocks_reply_data, base)
+
+const struct nla_policy ethnl_phc_vclocks_get_policy[] = {
+ [ETHTOOL_A_PHC_VCLOCKS_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+static int phc_vclocks_prepare_data(const struct ethnl_req_info *req_base,
+ struct ethnl_reply_data *reply_base,
+ struct genl_info *info)
+{
+ struct phc_vclocks_reply_data *data = PHC_VCLOCKS_REPDATA(reply_base);
+ struct net_device *dev = reply_base->dev;
+ int ret;
+
+ ret = ethnl_ops_begin(dev);
+ if (ret < 0)
+ return ret;
+ data->num = ethtool_get_phc_vclocks(dev, &data->index);
+ ethnl_ops_complete(dev);
+
+ return ret;
+}
+
+static int phc_vclocks_reply_size(const struct ethnl_req_info *req_base,
+ const struct ethnl_reply_data *reply_base)
+{
+ const struct phc_vclocks_reply_data *data =
+ PHC_VCLOCKS_REPDATA(reply_base);
+ int len = 0;
+
+ if (data->num > 0) {
+ len += nla_total_size(sizeof(u32));
+ len += nla_total_size(sizeof(s32) * data->num);
+ }
+
+ return len;
+}
+
+static int phc_vclocks_fill_reply(struct sk_buff *skb,
+ const struct ethnl_req_info *req_base,
+ const struct ethnl_reply_data *reply_base)
+{
+ const struct phc_vclocks_reply_data *data =
+ PHC_VCLOCKS_REPDATA(reply_base);
+
+ if (data->num <= 0)
+ return 0;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PHC_VCLOCKS_NUM, data->num) ||
+ nla_put(skb, ETHTOOL_A_PHC_VCLOCKS_INDEX,
+ sizeof(s32) * data->num, data->index))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+static void phc_vclocks_cleanup_data(struct ethnl_reply_data *reply_base)
+{
+ const struct phc_vclocks_reply_data *data =
+ PHC_VCLOCKS_REPDATA(reply_base);
+
+ kfree(data->index);
+}
+
+const struct ethnl_request_ops ethnl_phc_vclocks_request_ops = {
+ .request_cmd = ETHTOOL_MSG_PHC_VCLOCKS_GET,
+ .reply_cmd = ETHTOOL_MSG_PHC_VCLOCKS_GET_REPLY,
+ .hdr_attr = ETHTOOL_A_PHC_VCLOCKS_HEADER,
+ .req_info_size = sizeof(struct phc_vclocks_req_info),
+ .reply_data_size = sizeof(struct phc_vclocks_reply_data),
+
+ .prepare_data = phc_vclocks_prepare_data,
+ .reply_size = phc_vclocks_reply_size,
+ .fill_reply = phc_vclocks_fill_reply,
+ .cleanup_data = phc_vclocks_cleanup_data,
+};
--
2.25.1

2021-06-30 08:04:13

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 06/11] ptp: add kernel API ptp_convert_timestamp()

Add kernel API ptp_convert_timestamp() to convert raw hardware timestamp
to a specified ptp vclock time.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v2:
- Split from v1 patch #1 and #2.
- Fixed build warning.
Changes for v3:
- Converted HW timestamps to PHC bound, instead of previous
binding domain value to PHC idea.
Changes for v4:
- Fixed build issue.
Changes for v5:
- None.
---
drivers/ptp/ptp_vclock.c | 34 ++++++++++++++++++++++++++++++++
include/linux/ptp_clock_kernel.h | 13 ++++++++++++
2 files changed, 47 insertions(+)

diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index cefab29a0592..e0f87c57749a 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -183,3 +183,37 @@ int ptp_get_vclocks_index(int pclock_index, int **vclock_index)
return num;
}
EXPORT_SYMBOL(ptp_get_vclocks_index);
+
+void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
+ int vclock_index)
+{
+ char name[PTP_CLOCK_NAME_LEN] = "";
+ struct ptp_vclock *vclock;
+ struct ptp_clock *ptp;
+ unsigned long flags;
+ struct device *dev;
+ u64 ns;
+
+ snprintf(name, PTP_CLOCK_NAME_LEN, "ptp%d", vclock_index);
+ dev = class_find_device_by_name(ptp_class, name);
+ if (!dev)
+ return;
+
+ ptp = dev_get_drvdata(dev);
+ if (!ptp->is_virtual_clock) {
+ put_device(dev);
+ return;
+ }
+
+ vclock = info_to_vclock(ptp->info);
+
+ ns = ktime_to_ns(hwtstamps->hwtstamp);
+
+ spin_lock_irqsave(&vclock->lock, flags);
+ ns = timecounter_cyc2time(&vclock->tc, ns);
+ spin_unlock_irqrestore(&vclock->lock, flags);
+
+ put_device(dev);
+ hwtstamps->hwtstamp = ns_to_ktime(ns);
+}
+EXPORT_SYMBOL(ptp_convert_timestamp);
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 300a984fec87..71fac9237725 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -12,6 +12,7 @@
#include <linux/pps_kernel.h>
#include <linux/ptp_clock.h>
#include <linux/timecounter.h>
+#include <linux/skbuff.h>

#define PTP_CLOCK_NAME_LEN 32
/**
@@ -318,6 +319,15 @@ void ptp_cancel_worker_sync(struct ptp_clock *ptp);
*/
int ptp_get_vclocks_index(int pclock_index, int **vclock_index);

+/**
+ * ptp_convert_timestamp() - convert timestamp to a ptp vclock time
+ *
+ * @hwtstamps: skb_shared_hwtstamps structure pointer
+ * @vclock_index: phc index of ptp vclock.
+ */
+void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
+ int vclock_index);
+
#else
static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
struct device *parent)
@@ -339,6 +349,9 @@ static inline void ptp_cancel_worker_sync(struct ptp_clock *ptp)
{ }
static inline int ptp_get_vclocks_index(int pclock_index, int **vclock_index)
{ return 0; }
+static inline void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
+ int vclock_index)
+{ }

#endif

--
2.25.1

2021-06-30 08:04:28

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 07/11] mptcp: setsockopt: convert to mptcp_setsockopt_sol_socket_timestamping()

Split timestamping handling into a new function
mptcp_setsockopt_sol_socket_timestamping().
This is preparation for extending SO_TIMESTAMPING
for PHC binding, since optval will no longer be
integer.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v4:
- Added this patch.
Changes for v5:
- None.
---
net/mptcp/sockopt.c | 57 +++++++++++++++++++++++++++++++--------------
1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 092d1f635d27..ea38cbcd2ad4 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -157,19 +157,7 @@ static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
bool slow = lock_sock_fast(ssk);

- switch (optname) {
- case SO_TIMESTAMP_OLD:
- case SO_TIMESTAMP_NEW:
- case SO_TIMESTAMPNS_OLD:
- case SO_TIMESTAMPNS_NEW:
- sock_set_timestamp(sk, optname, !!val);
- break;
- case SO_TIMESTAMPING_NEW:
- case SO_TIMESTAMPING_OLD:
- sock_set_timestamping(sk, optname, val);
- break;
- }
-
+ sock_set_timestamp(sk, optname, !!val);
unlock_sock_fast(ssk, slow);
}

@@ -178,7 +166,8 @@ static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
}

static int mptcp_setsockopt_sol_socket_int(struct mptcp_sock *msk, int optname,
- sockptr_t optval, unsigned int optlen)
+ sockptr_t optval,
+ unsigned int optlen)
{
int val, ret;

@@ -205,14 +194,45 @@ static int mptcp_setsockopt_sol_socket_int(struct mptcp_sock *msk, int optname,
case SO_TIMESTAMP_NEW:
case SO_TIMESTAMPNS_OLD:
case SO_TIMESTAMPNS_NEW:
- case SO_TIMESTAMPING_OLD:
- case SO_TIMESTAMPING_NEW:
return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val);
}

return -ENOPROTOOPT;
}

+static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
+ int optname,
+ sockptr_t optval,
+ unsigned int optlen)
+{
+ struct mptcp_subflow_context *subflow;
+ struct sock *sk = (struct sock *)msk;
+ int val, ret;
+
+ ret = mptcp_get_int_option(msk, optval, optlen, &val);
+ if (ret)
+ return ret;
+
+ ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
+ KERNEL_SOCKPTR(&val), sizeof(val));
+ if (ret)
+ return ret;
+
+ lock_sock(sk);
+
+ mptcp_for_each_subflow(msk, subflow) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+ bool slow = lock_sock_fast(ssk);
+
+ sock_set_timestamping(sk, optname, val);
+ unlock_sock_fast(ssk, slow);
+ }
+
+ release_sock(sk);
+
+ return 0;
+}
+
static int mptcp_setsockopt_sol_socket_linger(struct mptcp_sock *msk, sockptr_t optval,
unsigned int optlen)
{
@@ -299,9 +319,12 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
case SO_TIMESTAMP_NEW:
case SO_TIMESTAMPNS_OLD:
case SO_TIMESTAMPNS_NEW:
+ return mptcp_setsockopt_sol_socket_int(msk, optname, optval,
+ optlen);
case SO_TIMESTAMPING_OLD:
case SO_TIMESTAMPING_NEW:
- return mptcp_setsockopt_sol_socket_int(msk, optname, optval, optlen);
+ return mptcp_setsockopt_sol_socket_timestamping(msk, optname,
+ optval, optlen);
case SO_LINGER:
return mptcp_setsockopt_sol_socket_linger(msk, optval, optlen);
case SO_RCVLOWAT:
--
2.25.1

2021-06-30 08:04:30

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 08/11] net: sock: extend SO_TIMESTAMPING for PHC binding

Since PTP virtual clock support is added, there can be
several PTP virtual clocks based on one PTP physical
clock for timestamping.

This patch is to extend SO_TIMESTAMPING API to support
PHC (PTP Hardware Clock) binding by adding a new flag
SOF_TIMESTAMPING_BIND_PHC. When PTP virtual clocks are
in use, user space can configure to bind one for
timestamping, but PTP physical clock is not supported
and not needed to bind.

This patch is preparation for timestamp conversion from
raw timestamp to a specific PTP virtual clock time in
core net.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Passed so_timestamping for sock_set_timestamping.
Changes for v5:
- Fixed build.
---
include/net/sock.h | 8 +++-
include/uapi/linux/net_tstamp.h | 17 ++++++++-
net/core/sock.c | 65 +++++++++++++++++++++++++++++++--
net/ethtool/common.c | 1 +
net/mptcp/sockopt.c | 23 +++++++++---
5 files changed, 101 insertions(+), 13 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 8bdd80027ffb..f23cb259b0e2 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -316,7 +316,9 @@ struct bpf_local_storage;
* @sk_timer: sock cleanup timer
* @sk_stamp: time stamp of last packet received
* @sk_stamp_seq: lock for accessing sk_stamp on 32 bit architectures only
- * @sk_tsflags: SO_TIMESTAMPING socket options
+ * @sk_tsflags: SO_TIMESTAMPING flags
+ * @sk_bind_phc: SO_TIMESTAMPING bind PHC index of PTP virtual clock
+ * for timestamping
* @sk_tskey: counter to disambiguate concurrent tstamp requests
* @sk_zckey: counter to order MSG_ZEROCOPY notifications
* @sk_socket: Identd and reporting IO signals
@@ -493,6 +495,7 @@ struct sock {
seqlock_t sk_stamp_seq;
#endif
u16 sk_tsflags;
+ int sk_bind_phc;
u8 sk_shutdown;
u32 sk_tskey;
atomic_t sk_zckey;
@@ -2755,7 +2758,8 @@ void sock_def_readable(struct sock *sk);

int sock_bindtoindex(struct sock *sk, int ifindex, bool lock_sk);
void sock_set_timestamp(struct sock *sk, int optname, bool valbool);
-int sock_set_timestamping(struct sock *sk, int optname, int val);
+int sock_set_timestamping(struct sock *sk, int optname,
+ struct so_timestamping timestamping);

void sock_enable_timestamps(struct sock *sk);
void sock_no_linger(struct sock *sk);
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index 7ed0b3d1c00a..fcc61c73a666 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -13,7 +13,7 @@
#include <linux/types.h>
#include <linux/socket.h> /* for SO_TIMESTAMPING */

-/* SO_TIMESTAMPING gets an integer bit field comprised of these values */
+/* SO_TIMESTAMPING flags */
enum {
SOF_TIMESTAMPING_TX_HARDWARE = (1<<0),
SOF_TIMESTAMPING_TX_SOFTWARE = (1<<1),
@@ -30,8 +30,9 @@ enum {
SOF_TIMESTAMPING_OPT_STATS = (1<<12),
SOF_TIMESTAMPING_OPT_PKTINFO = (1<<13),
SOF_TIMESTAMPING_OPT_TX_SWHW = (1<<14),
+ SOF_TIMESTAMPING_BIND_PHC = (1 << 15),

- SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_TX_SWHW,
+ SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_BIND_PHC,
SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
SOF_TIMESTAMPING_LAST
};
@@ -46,6 +47,18 @@ enum {
SOF_TIMESTAMPING_TX_SCHED | \
SOF_TIMESTAMPING_TX_ACK)

+/**
+ * struct so_timestamping - SO_TIMESTAMPING parameter
+ *
+ * @flags: SO_TIMESTAMPING flags
+ * @bind_phc: Index of PTP virtual clock bound to sock. This is available
+ * if flag SOF_TIMESTAMPING_BIND_PHC is set.
+ */
+struct so_timestamping {
+ int flags;
+ int bind_phc;
+};
+
/**
* struct hwtstamp_config - %SIOCGHWTSTAMP and %SIOCSHWTSTAMP parameter
*
diff --git a/net/core/sock.c b/net/core/sock.c
index ba1c0f75cd45..06bba0e3c207 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -139,6 +139,8 @@
#include <net/tcp.h>
#include <net/busy_poll.h>

+#include <linux/ethtool.h>
+
static DEFINE_MUTEX(proto_list_mutex);
static LIST_HEAD(proto_list);

@@ -810,8 +812,47 @@ void sock_set_timestamp(struct sock *sk, int optname, bool valbool)
}
}

-int sock_set_timestamping(struct sock *sk, int optname, int val)
+static int sock_timestamping_bind_phc(struct sock *sk, int phc_index)
{
+ struct net *net = sock_net(sk);
+ struct net_device *dev = NULL;
+ bool match = false;
+ int *vclock_index;
+ int i, num;
+
+ if (sk->sk_bound_dev_if)
+ dev = dev_get_by_index(net, sk->sk_bound_dev_if);
+
+ if (!dev) {
+ pr_err("%s: sock not bind to device\n", __func__);
+ return -EOPNOTSUPP;
+ }
+
+ num = ethtool_get_phc_vclocks(dev, &vclock_index);
+ for (i = 0; i < num; i++) {
+ if (*(vclock_index + i) == phc_index) {
+ match = true;
+ break;
+ }
+ }
+
+ if (num > 0)
+ kfree(vclock_index);
+
+ if (!match)
+ return -EINVAL;
+
+ sk->sk_bind_phc = phc_index;
+
+ return 0;
+}
+
+int sock_set_timestamping(struct sock *sk, int optname,
+ struct so_timestamping timestamping)
+{
+ int val = timestamping.flags;
+ int ret;
+
if (val & ~SOF_TIMESTAMPING_MASK)
return -EINVAL;

@@ -832,6 +873,12 @@ int sock_set_timestamping(struct sock *sk, int optname, int val)
!(val & SOF_TIMESTAMPING_OPT_TSONLY))
return -EINVAL;

+ if (val & SOF_TIMESTAMPING_BIND_PHC) {
+ ret = sock_timestamping_bind_phc(sk, timestamping.bind_phc);
+ if (ret)
+ return ret;
+ }
+
sk->sk_tsflags = val;
sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);

@@ -907,6 +954,7 @@ EXPORT_SYMBOL(sock_set_mark);
int sock_setsockopt(struct socket *sock, int level, int optname,
sockptr_t optval, unsigned int optlen)
{
+ struct so_timestamping timestamping;
struct sock_txtime sk_txtime;
struct sock *sk = sock->sk;
int val;
@@ -1073,7 +1121,15 @@ int sock_setsockopt(struct socket *sock, int level, int optname,

case SO_TIMESTAMPING_NEW:
case SO_TIMESTAMPING_OLD:
- ret = sock_set_timestamping(sk, optname, val);
+ if (optlen == sizeof(timestamping)) {
+ if (copy_from_sockptr(&timestamping, optval,
+ sizeof(timestamping)))
+ return -EFAULT;
+ } else {
+ memset(&timestamping, 0, sizeof(timestamping));
+ timestamping.flags = val;
+ }
+ ret = sock_set_timestamping(sk, optname, timestamping);
break;

case SO_RCVLOWAT:
@@ -1348,6 +1404,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
struct __kernel_old_timeval tm;
struct __kernel_sock_timeval stm;
struct sock_txtime txtime;
+ struct so_timestamping timestamping;
} v;

int lv = sizeof(int);
@@ -1451,7 +1508,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
break;

case SO_TIMESTAMPING_OLD:
- v.val = sk->sk_tsflags;
+ lv = sizeof(v.timestamping);
+ v.timestamping.flags = sk->sk_tsflags;
+ v.timestamping.bind_phc = sk->sk_bind_phc;
break;

case SO_RCVTIMEO_OLD:
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 798231b07676..c63e0739dc6a 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -398,6 +398,7 @@ const char sof_timestamping_names[][ETH_GSTRING_LEN] = {
[const_ilog2(SOF_TIMESTAMPING_OPT_STATS)] = "option-stats",
[const_ilog2(SOF_TIMESTAMPING_OPT_PKTINFO)] = "option-pktinfo",
[const_ilog2(SOF_TIMESTAMPING_OPT_TX_SWHW)] = "option-tx-swhw",
+ [const_ilog2(SOF_TIMESTAMPING_BIND_PHC)] = "bind-phc",
};
static_assert(ARRAY_SIZE(sof_timestamping_names) == __SOF_TIMESTAMPING_CNT);

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index ea38cbcd2ad4..8c03afac5ca0 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -207,14 +207,25 @@ static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
{
struct mptcp_subflow_context *subflow;
struct sock *sk = (struct sock *)msk;
- int val, ret;
+ struct so_timestamping timestamping;
+ int ret;

- ret = mptcp_get_int_option(msk, optval, optlen, &val);
- if (ret)
- return ret;
+ if (optlen == sizeof(timestamping)) {
+ if (copy_from_sockptr(&timestamping, optval,
+ sizeof(timestamping)))
+ return -EFAULT;
+ } else if (optlen == sizeof(int)) {
+ memset(&timestamping, 0, sizeof(timestamping));
+
+ if (copy_from_sockptr(&timestamping.flags, optval, sizeof(int)))
+ return -EFAULT;
+ } else {
+ return -EINVAL;
+ }

ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
- KERNEL_SOCKPTR(&val), sizeof(val));
+ KERNEL_SOCKPTR(&timestamping),
+ sizeof(timestamping));
if (ret)
return ret;

@@ -224,7 +235,7 @@ static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
bool slow = lock_sock_fast(ssk);

- sock_set_timestamping(sk, optname, val);
+ sock_set_timestamping(sk, optname, timestamping);
unlock_sock_fast(ssk, slow);
}

--
2.25.1

2021-06-30 08:04:47

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 10/11] selftests/net: timestamping: support binding PHC

Support binding PHC of PTP vclock for timestamping.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Fixed checkpatch.
Changes for v5:
- Dropped wrong conditional compile.
---
tools/testing/selftests/net/timestamping.c | 55 ++++++++++++++--------
1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/net/timestamping.c b/tools/testing/selftests/net/timestamping.c
index 21091be70688..aee631c5284e 100644
--- a/tools/testing/selftests/net/timestamping.c
+++ b/tools/testing/selftests/net/timestamping.c
@@ -47,7 +47,7 @@ static void usage(const char *error)
{
if (error)
printf("invalid option: %s\n", error);
- printf("timestamping interface option*\n\n"
+ printf("timestamping <interface> [bind_phc_index] [option]*\n\n"
"Options:\n"
" IP_MULTICAST_LOOP - looping outgoing multicasts\n"
" SO_TIMESTAMP - normal software time stamping, ms resolution\n"
@@ -58,6 +58,7 @@ static void usage(const char *error)
" SOF_TIMESTAMPING_RX_SOFTWARE - software fallback for incoming packets\n"
" SOF_TIMESTAMPING_SOFTWARE - request reporting of software time stamps\n"
" SOF_TIMESTAMPING_RAW_HARDWARE - request reporting of raw HW time stamps\n"
+ " SOF_TIMESTAMPING_BIND_PHC - request to bind a PHC of PTP vclock\n"
" SIOCGSTAMP - check last socket time stamp\n"
" SIOCGSTAMPNS - more accurate socket time stamp\n"
" PTPV2 - use PTPv2 messages\n");
@@ -311,7 +312,6 @@ static void recvpacket(int sock, int recvmsg_flags,

int main(int argc, char **argv)
{
- int so_timestamping_flags = 0;
int so_timestamp = 0;
int so_timestampns = 0;
int siocgstamp = 0;
@@ -325,6 +325,8 @@ int main(int argc, char **argv)
struct ifreq device;
struct ifreq hwtstamp;
struct hwtstamp_config hwconfig, hwconfig_requested;
+ struct so_timestamping so_timestamping_get = { 0, -1 };
+ struct so_timestamping so_timestamping = { 0, -1 };
struct sockaddr_in addr;
struct ip_mreq imr;
struct in_addr iaddr;
@@ -342,7 +344,12 @@ int main(int argc, char **argv)
exit(1);
}

- for (i = 2; i < argc; i++) {
+ if (argc >= 3 && sscanf(argv[2], "%d", &so_timestamping.bind_phc) == 1)
+ val = 3;
+ else
+ val = 2;
+
+ for (i = val; i < argc; i++) {
if (!strcasecmp(argv[i], "SO_TIMESTAMP"))
so_timestamp = 1;
else if (!strcasecmp(argv[i], "SO_TIMESTAMPNS"))
@@ -356,17 +363,19 @@ int main(int argc, char **argv)
else if (!strcasecmp(argv[i], "PTPV2"))
ptpv2 = 1;
else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_TX_HARDWARE"))
- so_timestamping_flags |= SOF_TIMESTAMPING_TX_HARDWARE;
+ so_timestamping.flags |= SOF_TIMESTAMPING_TX_HARDWARE;
else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_TX_SOFTWARE"))
- so_timestamping_flags |= SOF_TIMESTAMPING_TX_SOFTWARE;
+ so_timestamping.flags |= SOF_TIMESTAMPING_TX_SOFTWARE;
else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_RX_HARDWARE"))
- so_timestamping_flags |= SOF_TIMESTAMPING_RX_HARDWARE;
+ so_timestamping.flags |= SOF_TIMESTAMPING_RX_HARDWARE;
else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_RX_SOFTWARE"))
- so_timestamping_flags |= SOF_TIMESTAMPING_RX_SOFTWARE;
+ so_timestamping.flags |= SOF_TIMESTAMPING_RX_SOFTWARE;
else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_SOFTWARE"))
- so_timestamping_flags |= SOF_TIMESTAMPING_SOFTWARE;
+ so_timestamping.flags |= SOF_TIMESTAMPING_SOFTWARE;
else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_RAW_HARDWARE"))
- so_timestamping_flags |= SOF_TIMESTAMPING_RAW_HARDWARE;
+ so_timestamping.flags |= SOF_TIMESTAMPING_RAW_HARDWARE;
+ else if (!strcasecmp(argv[i], "SOF_TIMESTAMPING_BIND_PHC"))
+ so_timestamping.flags |= SOF_TIMESTAMPING_BIND_PHC;
else
usage(argv[i]);
}
@@ -385,10 +394,10 @@ int main(int argc, char **argv)
hwtstamp.ifr_data = (void *)&hwconfig;
memset(&hwconfig, 0, sizeof(hwconfig));
hwconfig.tx_type =
- (so_timestamping_flags & SOF_TIMESTAMPING_TX_HARDWARE) ?
+ (so_timestamping.flags & SOF_TIMESTAMPING_TX_HARDWARE) ?
HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
hwconfig.rx_filter =
- (so_timestamping_flags & SOF_TIMESTAMPING_RX_HARDWARE) ?
+ (so_timestamping.flags & SOF_TIMESTAMPING_RX_HARDWARE) ?
ptpv2 ? HWTSTAMP_FILTER_PTP_V2_L4_SYNC :
HWTSTAMP_FILTER_PTP_V1_L4_SYNC : HWTSTAMP_FILTER_NONE;
hwconfig_requested = hwconfig;
@@ -413,6 +422,9 @@ int main(int argc, char **argv)
sizeof(struct sockaddr_in)) < 0)
bail("bind");

+ if (setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE, interface, if_len))
+ bail("bind device");
+
/* set multicast group for outgoing packets */
inet_aton("224.0.1.130", &iaddr); /* alternate PTP domain 1 */
addr.sin_addr = iaddr;
@@ -444,10 +456,9 @@ int main(int argc, char **argv)
&enabled, sizeof(enabled)) < 0)
bail("setsockopt SO_TIMESTAMPNS");

- if (so_timestamping_flags &&
- setsockopt(sock, SOL_SOCKET, SO_TIMESTAMPING,
- &so_timestamping_flags,
- sizeof(so_timestamping_flags)) < 0)
+ if (so_timestamping.flags &&
+ setsockopt(sock, SOL_SOCKET, SO_TIMESTAMPING, &so_timestamping,
+ sizeof(so_timestamping)) < 0)
bail("setsockopt SO_TIMESTAMPING");

/* request IP_PKTINFO for debugging purposes */
@@ -468,14 +479,18 @@ int main(int argc, char **argv)
else
printf("SO_TIMESTAMPNS %d\n", val);

- if (getsockopt(sock, SOL_SOCKET, SO_TIMESTAMPING, &val, &len) < 0) {
+ len = sizeof(so_timestamping_get);
+ if (getsockopt(sock, SOL_SOCKET, SO_TIMESTAMPING, &so_timestamping_get,
+ &len) < 0) {
printf("%s: %s\n", "getsockopt SO_TIMESTAMPING",
strerror(errno));
} else {
- printf("SO_TIMESTAMPING %d\n", val);
- if (val != so_timestamping_flags)
- printf(" not the expected value %d\n",
- so_timestamping_flags);
+ printf("SO_TIMESTAMPING flags %d, bind phc %d\n",
+ so_timestamping_get.flags, so_timestamping_get.bind_phc);
+ if (so_timestamping_get.flags != so_timestamping.flags ||
+ so_timestamping_get.bind_phc != so_timestamping.bind_phc)
+ printf(" not expected, flags %d, bind phc %d\n",
+ so_timestamping.flags, so_timestamping.bind_phc);
}

/* send packets forever every five seconds */
--
2.25.1

2021-06-30 08:05:29

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 04/11] ptp: add kernel API ptp_get_vclocks_index()

Add kernel API ptp_get_vclocks_index() to get all ptp
vclocks index on pclock.

This is preparation for supporting ptp vclocks info query
through ethtool.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Dynamically allocated memory for vclock index getting.
Changes for v5:
- None.
---
drivers/ptp/ptp_clock.c | 3 ++-
drivers/ptp/ptp_private.h | 2 ++
drivers/ptp/ptp_vclock.c | 35 ++++++++++++++++++++++++++++++++
include/linux/ptp_clock_kernel.h | 14 +++++++++++++
4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 9205a9362a9d..f012fa581cf4 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -24,10 +24,11 @@
#define PTP_PPS_EVENT PPS_CAPTUREASSERT
#define PTP_PPS_MODE (PTP_PPS_DEFAULTS | PPS_CANWAIT | PPS_TSFMT_TSPEC)

+struct class *ptp_class;
+
/* private globals */

static dev_t ptp_devt;
-static struct class *ptp_class;

static DEFINE_IDA(ptp_clocks_map);

diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index f75fadd9b244..dba6be477067 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -96,6 +96,8 @@ static inline bool ptp_vclock_in_use(struct ptp_clock *ptp)
return in_use;
}

+extern struct class *ptp_class;
+
/*
* see ptp_chardev.c
*/
diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index fc9205cc504d..cefab29a0592 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -148,3 +148,38 @@ void ptp_vclock_unregister(struct ptp_vclock *vclock)
ptp_clock_unregister(vclock->clock);
kfree(vclock);
}
+
+int ptp_get_vclocks_index(int pclock_index, int **vclock_index)
+{
+ char name[PTP_CLOCK_NAME_LEN] = "";
+ struct ptp_clock *ptp;
+ struct device *dev;
+ int num = 0;
+
+ if (pclock_index < 0)
+ return num;
+
+ snprintf(name, PTP_CLOCK_NAME_LEN, "ptp%d", pclock_index);
+ dev = class_find_device_by_name(ptp_class, name);
+ if (!dev)
+ return num;
+
+ ptp = dev_get_drvdata(dev);
+
+ if (mutex_lock_interruptible(&ptp->n_vclocks_mux)) {
+ put_device(dev);
+ return num;
+ }
+
+ *vclock_index = kzalloc(sizeof(int) * ptp->n_vclocks, GFP_KERNEL);
+ if (!(*vclock_index))
+ goto out;
+
+ memcpy(*vclock_index, ptp->vclock_index, sizeof(int) * ptp->n_vclocks);
+ num = ptp->n_vclocks;
+out:
+ mutex_unlock(&ptp->n_vclocks_mux);
+ put_device(dev);
+ return num;
+}
+EXPORT_SYMBOL(ptp_get_vclocks_index);
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index b6fb771ee524..300a984fec87 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -306,6 +306,18 @@ int ptp_schedule_worker(struct ptp_clock *ptp, unsigned long delay);
*/
void ptp_cancel_worker_sync(struct ptp_clock *ptp);

+/**
+ * ptp_get_vclocks_index() - get all vclocks index on pclock, and
+ * caller is responsible to free memory
+ * of vclock_index
+ *
+ * @pclock_index: phc index of ptp pclock.
+ * @vclock_index: pointer to pointer of vclock index.
+ *
+ * return number of vclocks.
+ */
+int ptp_get_vclocks_index(int pclock_index, int **vclock_index);
+
#else
static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
struct device *parent)
@@ -325,6 +337,8 @@ static inline int ptp_schedule_worker(struct ptp_clock *ptp,
{ return -EOPNOTSUPP; }
static inline void ptp_cancel_worker_sync(struct ptp_clock *ptp)
{ }
+static inline int ptp_get_vclocks_index(int pclock_index, int **vclock_index)
+{ return 0; }

#endif

--
2.25.1

2021-06-30 08:05:53

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 11/11] MAINTAINERS: add entry for PTP virtual clock driver

Add entry for PTP virtual clock driver.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- None.
Changes for v5:
- None.
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 25956727ff24..7b174dbaa81b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14848,6 +14848,13 @@ F: drivers/net/phy/dp83640*
F: drivers/ptp/*
F: include/linux/ptp_cl*

+PTP VIRTUAL CLOCK SUPPORT
+M: Yangbo Lu <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/ptp/ptp_vclock.c
+F: net/ethtool/phc_vclocks.c
+
PTRACE SUPPORT
M: Oleg Nesterov <[email protected]>
S: Maintained
--
2.25.1

2021-06-30 08:06:37

by Yangbo Lu

[permalink] [raw]
Subject: [net-next, v5, 09/11] net: socket: support hardware timestamp conversion to PHC bound

This patch is to support hardware timestamp conversion to
PHC bound. This applies to both RX and TX since their skb
handling (for TX, it's skb clone in error queue) all goes
through __sock_recv_timestamp.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- None.
Changes for v5:
- None.
---
net/socket.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index bd9233da2497..0b2dad3bdf7f 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -104,6 +104,7 @@
#include <linux/sockios.h>
#include <net/busy_poll.h>
#include <linux/errqueue.h>
+#include <linux/ptp_clock_kernel.h>

#ifdef CONFIG_NET_RX_BUSY_POLL
unsigned int sysctl_net_busy_read __read_mostly;
@@ -873,12 +874,18 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
empty = 0;
if (shhwtstamps &&
(sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
- !skb_is_swtx_tstamp(skb, false_tstamp) &&
- ktime_to_timespec64_cond(shhwtstamps->hwtstamp, tss.ts + 2)) {
- empty = 0;
- if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_PKTINFO) &&
- !skb_is_err_queue(skb))
- put_ts_pktinfo(msg, skb);
+ !skb_is_swtx_tstamp(skb, false_tstamp)) {
+ if (sk->sk_tsflags & SOF_TIMESTAMPING_BIND_PHC)
+ ptp_convert_timestamp(shhwtstamps, sk->sk_bind_phc);
+
+ if (ktime_to_timespec64_cond(shhwtstamps->hwtstamp,
+ tss.ts + 2)) {
+ empty = 0;
+
+ if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_PKTINFO) &&
+ !skb_is_err_queue(skb))
+ put_ts_pktinfo(msg, skb);
+ }
}
if (!empty) {
if (sock_flag(sk, SOCK_TSTAMP_NEW))
--
2.25.1

2021-07-04 10:07:45

by kernel test robot

[permalink] [raw]
Subject: [ptp] becdd56786: BUG:kernel_NULL_pointer_dereference,address



Greeting,

FYI, we noticed the following commit (built with gcc-9):

commit: becdd56786002a908afd8a62f68976ed78572413 ("[net-next, v5, 02/11] ptp: support ptp physical/virtual clocks conversion")
url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210630-160348


in testcase: trinity
version: trinity-i386
with following parameters:

number: 99999
group: group-03

test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/


on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):



If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


[ 139.958903] BUG: kernel NULL pointer dereference, address: 00000304
[ 139.960977] #PF: supervisor read access in kernel mode
[ 139.962097] #PF: error_code(0x0000) - not-present page
[ 139.962097] *pde = 00000000
[ 139.962097] Oops: 0000 [#1] SMP
[ 139.962097] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G S 5.13.0-rc6-02622-gbecdd5678600 #1
[ 139.962097] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 139.962097] EIP: ptp_clock_register (drivers/ptp/ptp_clock.c:237)
[ 139.962097] Code: 6a 00 e8 1f 1d 83 fc 89 83 44 15 00 00 83 c4 14 3d 00 f0 ff ff 0f 87 03 4f 9b 01 8b 83 f4 03 00 00 89 98 e0 00 00 00 8b 45 9c <8b> 80 04 03 00 00 85 c0 74 18 8b 00 85 c0 74 12 ba 7a e3 19 da e8
All code
========
0: 6a 00 pushq $0x0
2: e8 1f 1d 83 fc callq 0xfffffffffc831d26
7: 89 83 44 15 00 00 mov %eax,0x1544(%rbx)
d: 83 c4 14 add $0x14,%esp
10: 3d 00 f0 ff ff cmp $0xfffff000,%eax
15: 0f 87 03 4f 9b 01 ja 0x19b4f1e
1b: 8b 83 f4 03 00 00 mov 0x3f4(%rbx),%eax
21: 89 98 e0 00 00 00 mov %ebx,0xe0(%rax)
27: 8b 45 9c mov -0x64(%rbp),%eax
2a:* 8b 80 04 03 00 00 mov 0x304(%rax),%eax <-- trapping instruction
30: 85 c0 test %eax,%eax
32: 74 18 je 0x4c
34: 8b 00 mov (%rax),%eax
36: 85 c0 test %eax,%eax
38: 74 12 je 0x4c
3a: ba 7a e3 19 da mov $0xda19e37a,%edx
3f: e8 .byte 0xe8

Code starting with the faulting instruction
===========================================
0: 8b 80 04 03 00 00 mov 0x304(%rax),%eax
6: 85 c0 test %eax,%eax
8: 74 18 je 0x22
a: 8b 00 mov (%rax),%eax
c: 85 c0 test %eax,%eax
e: 74 12 je 0x22
10: ba 7a e3 19 da mov $0xda19e37a,%edx
15: e8 .byte 0xe8
[ 139.962097] EAX: 00000000 EBX: c98ba000 ECX: 00000002 EDX: da436e01
[ 139.962097] ESI: dc3727a4 EDI: 00000000 EBP: c1c71f14 ESP: c1c71ea0
[ 139.962097] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246
[ 139.962097] CR0: 80050033 CR2: 00000304 CR3: 1b9ef000 CR4: 000406d0
[ 139.962097] Call Trace:
[ 139.962097] ? kobject_uevent_env (lib/kobject_uevent.c:628)
[ 139.962097] ? ptp_pch_init (drivers/ptp/ptp_kvm_common.c:136)
[ 139.962097] ? slow_virt_to_phys (arch/x86/mm/pat/set_memory.c:704)
[ 139.962097] ptp_kvm_init (include/linux/err.h:31 include/linux/err.h:60 drivers/ptp/ptp_kvm_common.c:150)
[ 139.962097] ? ptp_pch_init (drivers/ptp/ptp_kvm_common.c:136)
[ 139.962097] do_one_initcall (init/main.c:1249)
[ 139.962097] ? kernel_init_freeable (include/linux/compiler.h:234 include/linux/init.h:124 init/main.c:1322 init/main.c:1338 init/main.c:1358 init/main.c:1560)
[ 139.962097] kernel_init_freeable (init/main.c:1321 init/main.c:1338 init/main.c:1358 init/main.c:1560)
[ 139.962097] ? rest_init (init/main.c:1444)
[ 140.005239] kernel_init (init/main.c:1449)
[ 140.005239] ret_from_fork (arch/x86/entry/entry_32.S:775)
[ 140.005239] Modules linked in:
[ 140.005239] CR2: 0000000000000304
[ 140.005239] _warn_unseeded_randomness: 9 callbacks suppressed
[ 140.005239] random: get_random_bytes called from init_oops_id+0x42/0x60 with crng_init=0
[ 140.005239] ---[ end trace 739df3099651fd35 ]---
[ 140.005239] EIP: ptp_clock_register (drivers/ptp/ptp_clock.c:237)
[ 140.005239] Code: 6a 00 e8 1f 1d 83 fc 89 83 44 15 00 00 83 c4 14 3d 00 f0 ff ff 0f 87 03 4f 9b 01 8b 83 f4 03 00 00 89 98 e0 00 00 00 8b 45 9c <8b> 80 04 03 00 00 85 c0 74 18 8b 00 85 c0 74 12 ba 7a e3 19 da e8
All code
========
0: 6a 00 pushq $0x0
2: e8 1f 1d 83 fc callq 0xfffffffffc831d26
7: 89 83 44 15 00 00 mov %eax,0x1544(%rbx)
d: 83 c4 14 add $0x14,%esp
10: 3d 00 f0 ff ff cmp $0xfffff000,%eax
15: 0f 87 03 4f 9b 01 ja 0x19b4f1e
1b: 8b 83 f4 03 00 00 mov 0x3f4(%rbx),%eax
21: 89 98 e0 00 00 00 mov %ebx,0xe0(%rax)
27: 8b 45 9c mov -0x64(%rbp),%eax
2a:* 8b 80 04 03 00 00 mov 0x304(%rax),%eax <-- trapping instruction
30: 85 c0 test %eax,%eax
32: 74 18 je 0x4c
34: 8b 00 mov (%rax),%eax
36: 85 c0 test %eax,%eax
38: 74 12 je 0x4c
3a: ba 7a e3 19 da mov $0xda19e37a,%edx
3f: e8 .byte 0xe8

Code starting with the faulting instruction
===========================================
0: 8b 80 04 03 00 00 mov 0x304(%rax),%eax
6: 85 c0 test %eax,%eax
8: 74 18 je 0x22
a: 8b 00 mov (%rax),%eax
c: 85 c0 test %eax,%eax
e: 74 12 je 0x22
10: ba 7a e3 19 da mov $0xda19e37a,%edx
15: e8 .byte 0xe8


To reproduce:

# build kernel
cd linux
cp config-5.13.0-rc6-02622-gbecdd5678600 .config
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email



---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Thanks,
Oliver Sang


Attachments:
(No filename) (6.40 kB)
config-5.13.0-rc6-02622-gbecdd5678600 (276.16 kB)
job-script (4.11 kB)
dmesg.xz (35.55 kB)
Download all attachments

2021-07-04 13:35:53

by Richard Cochran

[permalink] [raw]
Subject: Re: [net-next, v5, 08/11] net: sock: extend SO_TIMESTAMPING for PHC binding

On Wed, Jun 30, 2021 at 04:11:59PM +0800, Yangbo Lu wrote:
> Since PTP virtual clock support is added, there can be
> several PTP virtual clocks based on one PTP physical
> clock for timestamping.
>
> This patch is to extend SO_TIMESTAMPING API to support
> PHC (PTP Hardware Clock) binding by adding a new flag
> SOF_TIMESTAMPING_BIND_PHC. When PTP virtual clocks are
> in use, user space can configure to bind one for
> timestamping, but PTP physical clock is not supported
> and not needed to bind.

Would it not be better to simply bind automatically?

Like this pseudo code:

if (hw_timestamping_requested() && interface_is_vclock()) {
bind_vclock();
}

It would be great to avoid forcing user space to use a new option.

Especially because NOT setting the option makes no sense. Or maybe
there is a use case for omitting the option?


Thoughts?

Thanks,
Richard

2021-07-05 08:21:57

by Yangbo Lu

[permalink] [raw]
Subject: RE: [net-next, v5, 08/11] net: sock: extend SO_TIMESTAMPING for PHC binding

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <[email protected]>
> Sent: 2021??7??4?? 21:34
> To: Y.b. Lu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; David S . Miller
> <[email protected]>; Jakub Kicinski <[email protected]>; Mat Martineau
> <[email protected]>; Matthieu Baerts
> <[email protected]>; Shuah Khan <[email protected]>; Michal
> Kubecek <[email protected]>; Florian Fainelli <[email protected]>;
> Andrew Lunn <[email protected]>; Rui Sousa <[email protected]>; Sebastien
> Laveze <[email protected]>
> Subject: Re: [net-next, v5, 08/11] net: sock: extend SO_TIMESTAMPING for
> PHC binding
>
> On Wed, Jun 30, 2021 at 04:11:59PM +0800, Yangbo Lu wrote:
> > Since PTP virtual clock support is added, there can be several PTP
> > virtual clocks based on one PTP physical clock for timestamping.
> >
> > This patch is to extend SO_TIMESTAMPING API to support PHC (PTP
> > Hardware Clock) binding by adding a new flag
> > SOF_TIMESTAMPING_BIND_PHC. When PTP virtual clocks are in use, user
> > space can configure to bind one for timestamping, but PTP physical
> > clock is not supported and not needed to bind.
>
> Would it not be better to simply bind automatically?
>
> Like this pseudo code:
>
> if (hw_timestamping_requested() && interface_is_vclock()) {
> bind_vclock();
> }
>
> It would be great to avoid forcing user space to use a new option.
>
> Especially because NOT setting the option makes no sense. Or maybe there is
> a use case for omitting the option?
>
>
> Thoughts?

When several ptp virtual clocks are created, the ptp physical clock is guaranteed for free running.

What I think is, for timestamping, if no flag SOF_TIMESTAMPING_BIND_PHC, the timestamping keeps using ptp physical clock.
If application wants to bind one ptp virtual clock for timestamping, the flag SOF_TIMESTAMPING_BIND_PHC should be set and clock index should be provided.
After all, several ptp virtual clocks created are likely for different timescale/use case. There should be a method for any of applications to select the right one to use.

Does it make sense?
Thank you.

>
> Thanks,
> Richard

2021-07-05 18:47:19

by Richard Cochran

[permalink] [raw]
Subject: Re: [net-next, v5, 08/11] net: sock: extend SO_TIMESTAMPING for PHC binding

On Mon, Jul 05, 2021 at 08:19:30AM +0000, Y.b. Lu wrote:
> When several ptp virtual clocks are created, the ptp physical clock is guaranteed for free running.
>
> What I think is, for timestamping, if no flag SOF_TIMESTAMPING_BIND_PHC, the timestamping keeps using ptp physical clock.
> If application wants to bind one ptp virtual clock for timestamping, the flag SOF_TIMESTAMPING_BIND_PHC should be set and clock index should be provided.
> After all, several ptp virtual clocks created are likely for different timescale/use case. There should be a method for any of applications to select the right one to use.
>
> Does it make sense?

Yes, indeed. I totally forgot that the user space PTP stack has
network sockets bond to interfaces, and they are completely separate
from the PHC clockid_t. Need more sleep...

Thanks,
Richard

2021-08-07 01:16:26

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [net-next, v5, 02/11] ptp: support ptp physical/virtual clocks conversion

Yangbo Lu <[email protected]> writes:

> Support ptp physical/virtual clocks conversion via sysfs.
> There will be a new attribute n_vclocks under ptp physical
> clock sysfs.
>
> - In default, the value is 0 meaning only ptp physical clock
> is in use.
> - Setting the value can create corresponding number of ptp
> virtual clocks to use. But current physical clock is guaranteed
> to stay free running.
> - Setting the value back to 0 can delete virtual clocks and back
> use physical clock again.
>
> Another new attribute max_vclocks control the maximum number of
> ptp vclocks.
>
> Signed-off-by: Yangbo Lu <[email protected]>
> ---
> Changes for v2:
> - Split from v1 patch #2.
> - Converted to num_vclocks for creating virtual clocks.
> - Guranteed physical clock free running when using virtual
> clocks.
> - Fixed build warning.
> - Updated copyright.
> Changes for v3:
> - Protected concurrency of ptp->num_vclocks accessing.
> Changes for v4:
> - Rephrased description in doc.
> - Used unsigned int for vclocks number, and max_vclocks
> for limitiation.
> - Fixed mutex locking.
> - Other minor fixes.
> Changes for v5:
> - Fixed checkpatch.
> - Checked pointer parent->class->name.
> ---
> Documentation/ABI/testing/sysfs-ptp | 20 ++++
> drivers/ptp/ptp_clock.c | 26 ++++++
> drivers/ptp/ptp_private.h | 21 +++++
> drivers/ptp/ptp_sysfs.c | 138 ++++++++++++++++++++++++++++
> 4 files changed, 205 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
> index 2363ad810ddb..d378f57c1b73 100644
> --- a/Documentation/ABI/testing/sysfs-ptp
> +++ b/Documentation/ABI/testing/sysfs-ptp
> @@ -33,6 +33,13 @@ Description:
> frequency adjustment value (a positive integer) in
> parts per billion.
>
> +What: /sys/class/ptp/ptpN/max_vclocks
> +Date: May 2021
> +Contact: Yangbo Lu <[email protected]>
> +Description:
> + This file contains the maximum number of ptp vclocks.
> + Write integer to re-configure it.
> +
> What: /sys/class/ptp/ptpN/n_alarms
> Date: September 2010
> Contact: Richard Cochran <[email protected]>
> @@ -61,6 +68,19 @@ Description:
> This file contains the number of programmable pins
> offered by the PTP hardware clock.
>
> +What: /sys/class/ptp/ptpN/n_vclocks
> +Date: May 2021
> +Contact: Yangbo Lu <[email protected]>
> +Description:
> + This file contains the number of virtual PTP clocks in
> + use. By default, the value is 0 meaning that only the
> + physical clock is in use. Setting the value creates
> + the corresponding number of virtual clocks and causes
> + the physical clock to become free running. Setting the
> + value back to 0 deletes the virtual clocks and
> + switches the physical clock back to normal, adjustable
> + operation.
> +
> What: /sys/class/ptp/ptpN/pins
> Date: March 2014
> Contact: Richard Cochran <[email protected]>
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index a23a37a4d5dc..7334f478dde7 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -76,6 +76,11 @@ static int ptp_clock_settime(struct posix_clock *pc, const struct timespec64 *tp
> {
> struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
>
> + if (ptp_vclock_in_use(ptp)) {
> + pr_err("ptp: virtual clock in use\n");
> + return -EBUSY;
> + }
> +
> return ptp->info->settime64(ptp->info, tp);
> }
>
> @@ -97,6 +102,11 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
> struct ptp_clock_info *ops;
> int err = -EOPNOTSUPP;
>
> + if (ptp_vclock_in_use(ptp)) {
> + pr_err("ptp: virtual clock in use\n");
> + return -EBUSY;
> + }
> +
> ops = ptp->info;
>
> if (tx->modes & ADJ_SETOFFSET) {
> @@ -161,6 +171,7 @@ static void ptp_clock_release(struct device *dev)
> ptp_cleanup_pin_groups(ptp);
> mutex_destroy(&ptp->tsevq_mux);
> mutex_destroy(&ptp->pincfg_mux);
> + mutex_destroy(&ptp->n_vclocks_mux);
> ida_simple_remove(&ptp_clocks_map, ptp->index);
> kfree(ptp);
> }
> @@ -208,6 +219,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> spin_lock_init(&ptp->tsevq.lock);
> mutex_init(&ptp->tsevq_mux);
> mutex_init(&ptp->pincfg_mux);
> + mutex_init(&ptp->n_vclocks_mux);
> init_waitqueue_head(&ptp->tsev_wq);
>
> if (ptp->info->do_aux_work) {
> @@ -221,6 +233,14 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> ptp->pps_source->lookup_cookie = ptp;
> }
>
> + /* PTP virtual clock is being registered under physical clock */
> + if (parent->class && parent->class->name &&
> + strcmp(parent->class->name, "ptp") == 0)
> + ptp->is_virtual_clock = true;
> +
> + if (!ptp->is_virtual_clock)
> + ptp->max_vclocks = PTP_DEFAULT_MAX_VCLOCKS;
> +
> err = ptp_populate_pin_groups(ptp);
> if (err)
> goto no_pin_groups;
> @@ -270,6 +290,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> kworker_err:
> mutex_destroy(&ptp->tsevq_mux);
> mutex_destroy(&ptp->pincfg_mux);
> + mutex_destroy(&ptp->n_vclocks_mux);
> ida_simple_remove(&ptp_clocks_map, index);
> no_slot:
> kfree(ptp);
> @@ -280,6 +301,11 @@ EXPORT_SYMBOL(ptp_clock_register);
>
> int ptp_clock_unregister(struct ptp_clock *ptp)
> {
> + if (ptp_vclock_in_use(ptp)) {
> + pr_err("ptp: virtual clock in use\n");
> + return -EBUSY;
> + }
> +

None of the drivers (that I looked) expect ptp_clock_unregister() to
return an error.

So, what should we do?
1. Fix all the drivers to return an error on module unloading (that's
usually the path ptp_clock_unregister() is called)?
2. Remove all the PTP virtual clocks when the physical clock is
unregistered?

(And as always, I could be missing something obvious here)

Sorry for being (extremely) late about this.


Cheers,
--
Vinicius

2021-08-07 14:24:00

by Richard Cochran

[permalink] [raw]
Subject: Re: [net-next, v5, 02/11] ptp: support ptp physical/virtual clocks conversion

On Fri, Aug 06, 2021 at 06:15:29PM -0700, Vinicius Costa Gomes wrote:

> > int ptp_clock_unregister(struct ptp_clock *ptp)
> > {
> > + if (ptp_vclock_in_use(ptp)) {
> > + pr_err("ptp: virtual clock in use\n");
> > + return -EBUSY;
> > + }
> > +
>
> None of the drivers (that I looked) expect ptp_clock_unregister() to
> return an error.
>
> So, what should we do?
> 1. Fix all the drivers to return an error on module unloading (that's
> usually the path ptp_clock_unregister() is called)?
> 2. Remove all the PTP virtual clocks when the physical clock is
> unregistered?

This:

3. Let the vclocks hold a reference to the underlying posix dynamic clock.


Thanks,
Richard

2021-08-07 14:47:40

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [net-next, v5, 02/11] ptp: support ptp physical/virtual clocks conversion

On Sat, Aug 07, 2021 at 07:22:59AM -0700, Richard Cochran wrote:
> On Fri, Aug 06, 2021 at 06:15:29PM -0700, Vinicius Costa Gomes wrote:
>
> > > int ptp_clock_unregister(struct ptp_clock *ptp)
> > > {
> > > + if (ptp_vclock_in_use(ptp)) {
> > > + pr_err("ptp: virtual clock in use\n");
> > > + return -EBUSY;
> > > + }
> > > +
> >
> > None of the drivers (that I looked) expect ptp_clock_unregister() to
> > return an error.
> >
> > So, what should we do?
> > 1. Fix all the drivers to return an error on module unloading (that's
> > usually the path ptp_clock_unregister() is called)?
> > 2. Remove all the PTP virtual clocks when the physical clock is
> > unregistered?
>
> This:
>
> 3. Let the vclocks hold a reference to the underlying posix dynamic clock.

So even if the vclock holds a reference to the underlying POSIX clock,
that won't prevent the hardware driver from unbinding, and further
gettime() calls on the vclock from faulting, will it?

What about:

4. Create a device link with the vclock being a consumer and the parent
clock being a supplier? This way, ptp_vclock_unregister() is
automatically called whenever (and before) ptp_clock_unregister() is.

https://www.kernel.org/doc/html/latest/driver-api/device_link.html

2021-08-07 21:01:40

by Richard Cochran

[permalink] [raw]
Subject: Re: [net-next, v5, 02/11] ptp: support ptp physical/virtual clocks conversion

On Sat, Aug 07, 2021 at 05:43:32PM +0300, Vladimir Oltean wrote:
> > 3. Let the vclocks hold a reference to the underlying posix dynamic clock.
>
> So even if the vclock holds a reference to the underlying POSIX clock,
> that won't prevent the hardware driver from unbinding, and further
> gettime() calls on the vclock from faulting, will it?

Oh, your are right. The vclocks call the real PHC clock's methods
directly, not through the posix dynamic clock layer.

> What about:
>
> 4. Create a device link with the vclock being a consumer and the parent
> clock being a supplier? This way, ptp_vclock_unregister() is
> automatically called whenever (and before) ptp_clock_unregister() is.
>
> https://www.kernel.org/doc/html/latest/driver-api/device_link.html

Sounds promising.

Thanks,
Richard