* [net-next, v3, 00/10] ptp: support virtual clocks and timestamping
@ 2021-06-15 9:45 Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework Yangbo Lu
` (9 more replies)
0 siblings, 10 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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_op_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.
Yangbo Lu (10):
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()
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 | 13 ++
MAINTAINERS | 7 +
drivers/ptp/Makefile | 2 +-
drivers/ptp/ptp_clock.c | 27 ++-
drivers/ptp/ptp_private.h | 34 ++++
drivers/ptp/ptp_sysfs.c | 95 +++++++++
drivers/ptp/ptp_vclock.c | 212 +++++++++++++++++++++
include/linux/ethtool.h | 2 +
include/linux/ptp_clock_kernel.h | 29 ++-
include/net/sock.h | 8 +-
include/uapi/linux/ethtool.h | 14 ++
include/uapi/linux/ethtool_netlink.h | 15 ++
include/uapi/linux/net_tstamp.h | 17 +-
include/uapi/linux/ptp_clock.h | 5 +
net/core/sock.c | 65 ++++++-
net/ethtool/Makefile | 2 +-
net/ethtool/common.c | 24 +++
net/ethtool/common.h | 2 +
net/ethtool/ioctl.c | 27 +++
net/ethtool/netlink.c | 10 +
net/ethtool/netlink.h | 2 +
net/ethtool/phc_vclocks.c | 86 +++++++++
net/mptcp/sockopt.c | 10 +-
net/socket.c | 19 +-
tools/testing/selftests/net/timestamping.c | 62 ++++--
25 files changed, 750 insertions(+), 39 deletions(-)
create mode 100644 drivers/ptp/ptp_vclock.c
create mode 100644 net/ethtool/phc_vclocks.c
base-commit: 89212e160b81e778f829b89743570665810e3b13
--
2.25.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-17 17:32 ` Richard Cochran
2021-06-15 9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
` (8 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
This patch is to add ptp virtual clock driver framework
utilizing timecounter/cyclecounter.
The patch just exports two essential APIs for PTP driver.
- ptp_vclock_register()
- ptp_vclock_unregister()
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
- Split from v1 patch #1.
- Fixed build warning.
- Updated copyright.
Changes for v3:
- Supported PTP virtual clock in default in PTP driver.
---
drivers/ptp/Makefile | 2 +-
drivers/ptp/ptp_private.h | 16 ++++
drivers/ptp/ptp_vclock.c | 154 +++++++++++++++++++++++++++++++
include/linux/ptp_clock_kernel.h | 4 +-
4 files changed, 174 insertions(+), 2 deletions(-)
create mode 100644 drivers/ptp/ptp_vclock.c
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 8673d1743faa..3c6a905760e2 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -3,7 +3,7 @@
# Makefile for PTP 1588 clock support.
#
-ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
+ptp-y := ptp_clock.o ptp_vclock.o ptp_chardev.o ptp_sysfs.o
ptp_kvm-$(CONFIG_X86) := ptp_kvm_x86.o ptp_kvm_common.o
ptp_kvm-$(CONFIG_HAVE_ARM_SMCCC) := ptp_kvm_arm.o ptp_kvm_common.o
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 6b97155148f1..3f388d63904c 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -48,6 +48,20 @@ struct ptp_clock {
struct kthread_delayed_work aux_work;
};
+#define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
+#define cc_to_vclock(d) container_of((d), struct ptp_vclock, cc)
+#define dw_to_vclock(d) container_of((d), struct ptp_vclock, refresh_work)
+
+struct ptp_vclock {
+ struct ptp_clock *pclock;
+ struct ptp_clock_info info;
+ struct ptp_clock *clock;
+ struct cyclecounter cc;
+ struct timecounter tc;
+ spinlock_t lock; /* protects tc/cc */
+ struct delayed_work refresh_work;
+};
+
/*
* The function queue_cnt() is safe for readers to call without
* holding q->lock. Readers use this function to verify that the queue
@@ -89,4 +103,6 @@ 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);
+struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
+void ptp_vclock_unregister(struct ptp_vclock *vclock);
#endif
diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
new file mode 100644
index 000000000000..b8f500677314
--- /dev/null
+++ b/drivers/ptp/ptp_vclock.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * PTP virtual clock driver
+ *
+ * Copyright 2021 NXP
+ */
+#include <linux/slab.h>
+#include "ptp_private.h"
+
+#define PTP_VCLOCK_CC_MULT (1 << 31)
+#define PTP_VCLOCK_CC_SHIFT 31
+#define PTP_VCLOCK_CC_MULT_NUM (1 << 9)
+#define PTP_VCLOCK_CC_MULT_DEM 15625ULL
+#define PTP_VCLOCK_CC_REFRESH_INTERVAL (HZ * 2)
+
+static int ptp_vclock_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+ struct ptp_vclock *vclock = info_to_vclock(ptp);
+ unsigned long flags;
+ s64 adj;
+
+ adj = (s64)scaled_ppm * PTP_VCLOCK_CC_MULT_NUM;
+ adj = div_s64(adj, PTP_VCLOCK_CC_MULT_DEM);
+
+ spin_lock_irqsave(&vclock->lock, flags);
+ timecounter_read(&vclock->tc);
+ vclock->cc.mult = PTP_VCLOCK_CC_MULT + adj;
+ spin_unlock_irqrestore(&vclock->lock, flags);
+
+ return 0;
+}
+
+static int ptp_vclock_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+ struct ptp_vclock *vclock = info_to_vclock(ptp);
+ unsigned long flags;
+
+ spin_lock_irqsave(&vclock->lock, flags);
+ timecounter_adjtime(&vclock->tc, delta);
+ spin_unlock_irqrestore(&vclock->lock, flags);
+
+ return 0;
+}
+
+static int ptp_vclock_gettime(struct ptp_clock_info *ptp,
+ struct timespec64 *ts)
+{
+ struct ptp_vclock *vclock = info_to_vclock(ptp);
+ unsigned long flags;
+ u64 ns;
+
+ spin_lock_irqsave(&vclock->lock, flags);
+ ns = timecounter_read(&vclock->tc);
+ spin_unlock_irqrestore(&vclock->lock, flags);
+ *ts = ns_to_timespec64(ns);
+
+ return 0;
+}
+
+static int ptp_vclock_settime(struct ptp_clock_info *ptp,
+ const struct timespec64 *ts)
+{
+ struct ptp_vclock *vclock = info_to_vclock(ptp);
+ u64 ns = timespec64_to_ns(ts);
+ unsigned long flags;
+
+ spin_lock_irqsave(&vclock->lock, flags);
+ timecounter_init(&vclock->tc, &vclock->cc, ns);
+ spin_unlock_irqrestore(&vclock->lock, flags);
+
+ return 0;
+}
+
+static const struct ptp_clock_info ptp_vclock_info = {
+ .owner = THIS_MODULE,
+ .name = "ptp virtual clock",
+ /* The maximum ppb value that long scaled_ppm can support */
+ .max_adj = 32767999,
+ .adjfine = ptp_vclock_adjfine,
+ .adjtime = ptp_vclock_adjtime,
+ .gettime64 = ptp_vclock_gettime,
+ .settime64 = ptp_vclock_settime,
+};
+
+static void ptp_vclock_refresh(struct work_struct *work)
+{
+ struct delayed_work *dw = to_delayed_work(work);
+ struct ptp_vclock *vclock = dw_to_vclock(dw);
+ struct timespec64 ts;
+
+ ptp_vclock_gettime(&vclock->info, &ts);
+ schedule_delayed_work(&vclock->refresh_work,
+ PTP_VCLOCK_CC_REFRESH_INTERVAL);
+}
+
+static u64 ptp_vclock_read(const struct cyclecounter *cc)
+{
+ struct ptp_vclock *vclock = cc_to_vclock(cc);
+ struct ptp_clock *ptp = vclock->pclock;
+ struct timespec64 ts = {};
+
+ if (ptp->info->gettimex64)
+ ptp->info->gettimex64(ptp->info, &ts, NULL);
+ else
+ ptp->info->gettime64(ptp->info, &ts);
+
+ return timespec64_to_ns(&ts);
+}
+
+static const struct cyclecounter ptp_vclock_cc = {
+ .read = ptp_vclock_read,
+ .mask = CYCLECOUNTER_MASK(32),
+ .mult = PTP_VCLOCK_CC_MULT,
+ .shift = PTP_VCLOCK_CC_SHIFT,
+};
+
+struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock)
+{
+ struct ptp_vclock *vclock;
+
+ vclock = kzalloc(sizeof(*vclock), GFP_KERNEL);
+ if (!vclock)
+ return NULL;
+
+ vclock->pclock = pclock;
+ vclock->info = ptp_vclock_info;
+ vclock->cc = ptp_vclock_cc;
+
+ snprintf(vclock->info.name, PTP_CLOCK_NAME_LEN, "ptp%d_virt",
+ pclock->index);
+
+ spin_lock_init(&vclock->lock);
+
+ vclock->clock = ptp_clock_register(&vclock->info, &pclock->dev);
+ if (IS_ERR_OR_NULL(vclock->clock)) {
+ kfree(vclock);
+ return NULL;
+ }
+
+ timecounter_init(&vclock->tc, &vclock->cc, 0);
+
+ INIT_DELAYED_WORK(&vclock->refresh_work, ptp_vclock_refresh);
+ schedule_delayed_work(&vclock->refresh_work,
+ PTP_VCLOCK_CC_REFRESH_INTERVAL);
+
+ return vclock;
+}
+
+void ptp_vclock_unregister(struct ptp_vclock *vclock)
+{
+ cancel_delayed_work_sync(&vclock->refresh_work);
+ ptp_clock_unregister(vclock->clock);
+ kfree(vclock);
+}
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index a311bddd9e85..af12cc1e76d7 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -11,7 +11,9 @@
#include <linux/device.h>
#include <linux/pps_kernel.h>
#include <linux/ptp_clock.h>
+#include <linux/timecounter.h>
+#define PTP_CLOCK_NAME_LEN 32
/**
* struct ptp_clock_request - request PTP clock event
*
@@ -134,7 +136,7 @@ struct ptp_system_timestamp {
struct ptp_clock_info {
struct module *owner;
- char name[16];
+ char name[PTP_CLOCK_NAME_LEN];
s32 max_adj;
int n_alarm;
int n_ext_ts;
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-17 17:42 ` Richard Cochran
` (2 more replies)
2021-06-15 9:45 ` [net-next, v3, 03/10] ptp: track available ptp vclocks information Yangbo Lu
` (7 subsequent siblings)
9 siblings, 3 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
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.
---
Documentation/ABI/testing/sysfs-ptp | 13 +++++
drivers/ptp/ptp_clock.c | 22 +++++++
drivers/ptp/ptp_private.h | 15 +++++
drivers/ptp/ptp_sysfs.c | 89 +++++++++++++++++++++++++++++
include/uapi/linux/ptp_clock.h | 5 ++
5 files changed, 144 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index 2363ad810ddb..2ef11b775f47 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -61,6 +61,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 <yangbo.lu@nxp.com>
+Description:
+ This file contains the ptp virtual clocks number in use,
+ based on current ptp physical clock. 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 ptp physical clock is
+ guaranteed to stay free running. Setting the value back
+ to 0 can delete ptp virtual clocks and back use ptp
+ physical clock again.
+
What: /sys/class/ptp/ptpN/pins
Date: March 2014
Contact: Richard Cochran <richardcochran@gmail.com>
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index a780435331c8..78414b3e16dd 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_guaranteed_pclock(ptp)) {
+ pr_err("ptp: virtual clock in use, guarantee physical clock free running\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_guaranteed_pclock(ptp)) {
+ pr_err("ptp: virtual clock in use, guarantee physical clock free running\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) {
@@ -220,6 +232,10 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
}
}
+ /* PTP virtual clock is being registered under physical clock */
+ if (parent->class && strcmp(parent->class->name, "ptp") == 0)
+ ptp->vclock_flag = true;
+
err = ptp_populate_pin_groups(ptp);
if (err)
goto no_pin_groups;
@@ -269,6 +285,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);
@@ -279,6 +296,11 @@ EXPORT_SYMBOL(ptp_clock_register);
int ptp_clock_unregister(struct ptp_clock *ptp)
{
+ if (ptp_guaranteed_pclock(ptp)) {
+ pr_err("ptp: virtual clock in use, guarantee physical clock free running\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 3f388d63904c..6949afc9d733 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -46,6 +46,9 @@ struct ptp_clock {
const struct attribute_group *pin_attr_groups[2];
struct kthread_worker *kworker;
struct kthread_delayed_work aux_work;
+ u8 n_vclocks;
+ struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
+ bool vclock_flag;
};
#define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
@@ -75,6 +78,18 @@ static inline int queue_cnt(struct timestamp_event_queue *q)
return cnt < 0 ? PTP_MAX_TIMESTAMPS + cnt : cnt;
}
+/*
+ * Guarantee physical clock to stay free running, if ptp virtual clocks
+ * on it are in use.
+ */
+static inline bool ptp_guaranteed_pclock(struct ptp_clock *ptp)
+{
+ if (!ptp->vclock_flag && ptp->n_vclocks)
+ return true;
+
+ return false;
+}
+
/*
* see ptp_chardev.c
*/
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index be076a91e20e..600edd7a90af 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,90 @@ 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);
+
+ return snprintf(page, PAGE_SIZE-1, "%d\n", ptp->n_vclocks);
+}
+
+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;
+ u8 num, i;
+
+ if (kstrtou8(buf, 0, &num))
+ goto out;
+
+ if (num > PTP_MAX_VCLOCKS) {
+ dev_err(dev, "max value is %d\n", PTP_MAX_VCLOCKS);
+ goto out;
+ }
+
+ if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
+ return -ERESTARTSYS;
+
+ /* 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) {
+ mutex_unlock(&ptp->n_vclocks_mux);
+ 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:
+ return err;
+}
+static DEVICE_ATTR_RW(n_vclocks);
+
static struct attribute *ptp_attrs[] = {
&dev_attr_clock_name.attr,
@@ -162,6 +247,7 @@ static struct attribute *ptp_attrs[] = {
&dev_attr_fifo.attr,
&dev_attr_period.attr,
&dev_attr_pps_enable.attr,
+ &dev_attr_n_vclocks.attr,
NULL
};
@@ -183,6 +269,9 @@ 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) {
+ if (ptp->vclock_flag)
+ mode = 0;
}
return mode;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 1d108d597f66..4b933dc1b81b 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -69,6 +69,11 @@
*/
#define PTP_PEROUT_V1_VALID_FLAGS (0)
+/*
+ * Max number of PTP virtual clocks per PTP physical clock
+ */
+#define PTP_MAX_VCLOCKS 20
+
/*
* struct ptp_clock_time - represents a time value
*
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 03/10] ptp: track available ptp vclocks information
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 04/10] ptp: add kernel API ptp_get_vclocks_index() Yangbo Lu
` (6 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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 <yangbo.lu@nxp.com>
---
Change for v3:
- Added this patch.
---
drivers/ptp/ptp_clock.c | 2 ++
drivers/ptp/ptp_private.h | 1 +
drivers/ptp/ptp_sysfs.c | 6 ++++++
3 files changed, 9 insertions(+)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 78414b3e16dd..38842a76acf8 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -236,6 +236,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (parent->class && strcmp(parent->class->name, "ptp") == 0)
ptp->vclock_flag = true;
+ memset(ptp->vclock_index, -1, sizeof(ptp->vclock_index));
+
err = ptp_populate_pin_groups(ptp);
if (err)
goto no_pin_groups;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 6949afc9d733..5671710ca0fa 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -47,6 +47,7 @@ struct ptp_clock {
struct kthread_worker *kworker;
struct kthread_delayed_work aux_work;
u8 n_vclocks;
+ int vclock_index[PTP_MAX_VCLOCKS];
struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
bool vclock_flag;
};
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index 600edd7a90af..d9534935c1e6 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -207,6 +207,9 @@ static ssize_t n_vclocks_store(struct device *dev,
goto out;
}
+ ptp->vclock_index[ptp->n_vclocks + i] =
+ vclock->clock->index;
+
dev_info(dev, "new virtual clock ptp%d\n",
vclock->clock->index);
}
@@ -217,6 +220,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)
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 04/10] ptp: add kernel API ptp_get_vclocks_index()
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (2 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 03/10] ptp: track available ptp vclocks information Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks Yangbo Lu
` (5 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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 <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
drivers/ptp/ptp_clock.c | 3 ++-
drivers/ptp/ptp_private.h | 2 ++
drivers/ptp/ptp_vclock.c | 24 ++++++++++++++++++++++++
include/linux/ptp_clock_kernel.h | 12 ++++++++++++
4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 38842a76acf8..88e43451b062 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 5671710ca0fa..16b7ba583ddd 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -91,6 +91,8 @@ static inline bool ptp_guaranteed_pclock(struct ptp_clock *ptp)
return false;
}
+extern struct class *ptp_class;
+
/*
* see ptp_chardev.c
*/
diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index b8f500677314..8944b4fe32d8 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -152,3 +152,27 @@ 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);
+ num = ptp->n_vclocks;
+ memcpy(vclock_index, ptp->vclock_index, sizeof(int) * ptp->n_vclocks);
+ 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 af12cc1e76d7..37f49d3e761e 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -306,6 +306,16 @@ 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
+ *
+ * @pclock_index: phc index of ptp pclock.
+ * @vclock_index: pointer to phc index of ptp vclocks.
+ *
+ * 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 +335,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 int ptp_get_vclocks_index(int pclock_index, int *vclock_index)
+{ return 0; }
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (3 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 04/10] ptp: add kernel API ptp_get_vclocks_index() Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-15 19:49 ` Jakub Kicinski
2021-06-15 23:24 ` Michal Kubecek
2021-06-15 9:45 ` [net-next, v3, 06/10] ptp: add kernel API ptp_convert_timestamp() Yangbo Lu
` (4 subsequent siblings)
9 siblings, 2 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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 <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
include/linux/ethtool.h | 2 +
include/uapi/linux/ethtool.h | 14 +++++
include/uapi/linux/ethtool_netlink.h | 15 +++++
net/ethtool/Makefile | 2 +-
net/ethtool/common.c | 23 ++++++++
net/ethtool/common.h | 2 +
net/ethtool/ioctl.c | 27 +++++++++
net/ethtool/netlink.c | 10 ++++
net/ethtool/netlink.h | 2 +
net/ethtool/phc_vclocks.c | 86 ++++++++++++++++++++++++++++
10 files changed, 182 insertions(+), 1 deletion(-)
create mode 100644 net/ethtool/phc_vclocks.c
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index e030f7510cd3..dccbe1829ea5 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -86,6 +86,8 @@ struct netlink_ext_ack;
/* Some generic methods drivers may use in their ethtool_ops */
u32 ethtool_op_get_link(struct net_device *dev);
int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *eti);
+int ethtool_op_get_phc_vclocks(struct net_device *dev,
+ struct ethtool_phc_vclocks *phc_vclocks);
/* Link extended state and substate. */
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index cfef6b08169a..0fb04f945767 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -17,6 +17,7 @@
#include <linux/const.h>
#include <linux/types.h>
#include <linux/if_ether.h>
+#include <linux/ptp_clock.h>
#ifndef __KERNEL__
#include <limits.h> /* for INT_MAX */
@@ -1341,6 +1342,18 @@ struct ethtool_ts_info {
__u32 rx_reserved[3];
};
+/**
+ * struct ethtool_phc_vclocks - holds a device's PTP virtual clocks
+ * @cmd: command number = %ETHTOOL_GET_PHC_VCLOCKS
+ * @num: number of PTP vclocks
+ * @index: all index values of PTP vclocks
+ */
+struct ethtool_phc_vclocks {
+ __u32 cmd;
+ __u8 num;
+ __s32 index[PTP_MAX_VCLOCKS];
+};
+
/*
* %ETHTOOL_SFEATURES changes features present in features[].valid to the
* values of corresponding bits in features[].requested. Bits in .requested
@@ -1552,6 +1565,7 @@ enum ethtool_fec_config_bits {
#define ETHTOOL_PHY_STUNABLE 0x0000004f /* Set PHY tunable configuration */
#define ETHTOOL_GFECPARAM 0x00000050 /* Get FEC settings */
#define ETHTOOL_SFECPARAM 0x00000051 /* Set FEC settings */
+#define ETHTOOL_GET_PHC_VCLOCKS 0x00000052 /* Get PHC virtual clocks info */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 825cfda1c5d5..f8fa688f8351 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, /* u8 */
+ ETHTOOL_A_PHC_VCLOCKS_INDEX, /* 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..14035f2dc6d4 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,28 @@ 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,
+ struct ethtool_phc_vclocks *phc_vclocks)
+{
+ struct ethtool_ts_info info = { };
+ int index[PTP_MAX_VCLOCKS];
+ int num = 0;
+
+ phc_vclocks->cmd = ETHTOOL_GET_PHC_VCLOCKS;
+ phc_vclocks->num = 0;
+ memset(phc_vclocks->index, -1, sizeof(phc_vclocks->index));
+
+ if (!__ethtool_get_ts_info(dev, &info))
+ num = ptp_get_vclocks_index(info.phc_index, index);
+
+ if (num > 0) {
+ phc_vclocks->num = num;
+ memcpy(phc_vclocks->index, index, sizeof(int) * num);
+ }
+
+ return 0;
+}
+
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/common.h b/net/ethtool/common.h
index 2dc2b80aea5f..e5296bfc21a4 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -44,6 +44,8 @@ bool convert_legacy_settings_to_link_ksettings(
const struct ethtool_cmd *legacy_settings);
int ethtool_get_max_rxfh_channel(struct net_device *dev, u32 *max);
int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info);
+int __ethtool_get_phc_vclocks(struct net_device *dev,
+ struct ethtool_phc_vclocks *phc_vclocks);
extern const struct ethtool_phy_ops *ethtool_phy_ops;
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 3fa7a394eabf..c199e5785197 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2188,6 +2188,29 @@ static int ethtool_get_ts_info(struct net_device *dev, void __user *useraddr)
return 0;
}
+static int ethtool_get_phc_vclocks(struct net_device *dev,
+ void __user *useraddr)
+{
+ struct ethtool_phc_vclocks phc_vclocks;
+ int err;
+
+ err = __ethtool_get_phc_vclocks(dev, &phc_vclocks);
+ if (err)
+ return err;
+
+ if (copy_to_user(useraddr, &phc_vclocks, sizeof(phc_vclocks)))
+ return -EFAULT;
+
+ return 0;
+}
+
+int ethtool_op_get_phc_vclocks(struct net_device *dev,
+ struct ethtool_phc_vclocks *phc_vclocks)
+{
+ return __ethtool_get_phc_vclocks(dev, phc_vclocks);
+}
+EXPORT_SYMBOL(ethtool_op_get_phc_vclocks);
+
int ethtool_get_module_info_call(struct net_device *dev,
struct ethtool_modinfo *modinfo)
{
@@ -2634,6 +2657,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_GFEATURES:
case ETHTOOL_GCHANNELS:
case ETHTOOL_GET_TS_INFO:
+ case ETHTOOL_GET_PHC_VCLOCKS:
case ETHTOOL_GEEE:
case ETHTOOL_GTUNABLE:
case ETHTOOL_PHY_GTUNABLE:
@@ -2858,6 +2882,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_SFECPARAM:
rc = ethtool_set_fecparam(dev, useraddr);
break;
+ case ETHTOOL_GET_PHC_VCLOCKS:
+ rc = ethtool_get_phc_vclocks(dev, useraddr);
+ break;
default:
rc = -EOPNOTSUPP;
}
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 88d8a0243f35..2436232d0ecc 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] = ðnl_tsinfo_request_ops,
[ETHTOOL_MSG_MODULE_EEPROM_GET] = ðnl_module_eeprom_request_ops,
[ETHTOOL_MSG_STATS_GET] = ðnl_stats_request_ops,
+ [ETHTOOL_MSG_PHC_VCLOCKS_GET] = ðnl_phc_vclocks_request_ops,
};
static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -953,6 +954,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 90b10966b16b..c424f243392b 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_DATA + 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..5423e74ef9af
--- /dev/null
+++ b/net/ethtool/phc_vclocks.c
@@ -0,0 +1,86 @@
+// 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;
+ struct ethtool_phc_vclocks phc_vclocks;
+};
+
+#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;
+ ret = __ethtool_get_phc_vclocks(dev, &data->phc_vclocks);
+ 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);
+ const struct ethtool_phc_vclocks *phc_vclocks = &data->phc_vclocks;
+ int len = 0;
+
+ if (phc_vclocks->num > 0) {
+ len += nla_total_size(sizeof(u32));
+ len += nla_total_size(sizeof(data->phc_vclocks.index));
+ }
+
+ 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);
+ const struct ethtool_phc_vclocks *phc_vclocks = &data->phc_vclocks;
+
+ if (phc_vclocks->num <= 0)
+ return 0;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PHC_VCLOCKS_NUM, phc_vclocks->num) ||
+ nla_put(skb, ETHTOOL_A_PHC_VCLOCKS_INDEX,
+ sizeof(phc_vclocks->index), phc_vclocks->index))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+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,
+};
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 06/10] ptp: add kernel API ptp_convert_timestamp()
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (4 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
` (3 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
Add kernel API ptp_convert_timestamp() to convert raw hardware timestamp
to a specified ptp vclock time.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
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.
---
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 8944b4fe32d8..65ca31916691 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -176,3 +176,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->vclock_flag) {
+ 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 37f49d3e761e..b9414fc19249 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
/**
@@ -316,6 +317,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)
@@ -337,6 +347,9 @@ static inline void ptp_cancel_worker_sync(struct ptp_clock *ptp)
{ }
static int ptp_get_vclocks_index(int pclock_index, int *vclock_index)
{ return 0; }
+static void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
+ int vclock_index)
+{ }
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (5 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 06/10] ptp: add kernel API ptp_convert_timestamp() Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-16 1:00 ` Mat Martineau
` (2 more replies)
2021-06-15 9:45 ` [net-next, v3, 08/10] net: socket: support hardware timestamp conversion to PHC bound Yangbo Lu
` (2 subsequent siblings)
9 siblings, 3 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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 <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
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 | 10 +++--
5 files changed, 91 insertions(+), 10 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 9b341c2c924f..adb6c0edc867 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;
@@ -2744,7 +2747,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, int val,
+ sockptr_t optval, unsigned int optlen);
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..9d4cde4ef7e6 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 ddfa88082a2b..70d2c2322429 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);
@@ -794,8 +796,53 @@ 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 ethtool_phc_vclocks phc_vclocks = { };
+ struct net *net = sock_net(sk);
+ struct net_device *dev = NULL;
+ bool match = false;
+ int i;
+
+ 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;
+ }
+
+ ethtool_op_get_phc_vclocks(dev, &phc_vclocks);
+
+ for (i = 0; i < phc_vclocks.num; i++) {
+ if (phc_vclocks.index[i] == phc_index) {
+ match = true;
+ break;
+ }
+ }
+
+ if (!match)
+ return -EINVAL;
+
+ sk->sk_bind_phc = phc_index;
+
+ return 0;
+}
+
+int sock_set_timestamping(struct sock *sk, int optname, int val,
+ sockptr_t optval, unsigned int optlen)
+{
+ struct so_timestamping timestamping;
+ int ret;
+
+ if (optlen == sizeof(timestamping)) {
+ if (copy_from_sockptr(×tamping, optval,
+ sizeof(timestamping)))
+ return -EFAULT;
+
+ val = timestamping.flags;
+ }
+
if (val & ~SOF_TIMESTAMPING_MASK)
return -EINVAL;
@@ -816,6 +863,15 @@ int sock_set_timestamping(struct sock *sk, int optname, int val)
!(val & SOF_TIMESTAMPING_OPT_TSONLY))
return -EINVAL;
+ if (optlen == sizeof(timestamping) &&
+ val & SOF_TIMESTAMPING_BIND_PHC) {
+ ret = sock_timestamping_bind_phc(sk, timestamping.bind_phc);
+ if (ret)
+ return ret;
+ } else {
+ sk->sk_bind_phc = -1;
+ }
+
sk->sk_tsflags = val;
sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);
@@ -1057,7 +1113,7 @@ 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);
+ ret = sock_set_timestamping(sk, optname, val, optval, optlen);
break;
case SO_RCVLOWAT:
@@ -1332,6 +1388,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);
@@ -1435,7 +1492,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 14035f2dc6d4..4bf1bd3a3db6 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 092d1f635d27..2ca90b230827 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -140,7 +140,10 @@ static void mptcp_so_incoming_cpu(struct mptcp_sock *msk, int val)
mptcp_sol_socket_sync_intval(msk, SO_INCOMING_CPU, val);
}
-static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optname, int val)
+static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
+ int optname, int val,
+ sockptr_t optval,
+ unsigned int optlen)
{
sockptr_t optval = KERNEL_SOCKPTR(&val);
struct mptcp_subflow_context *subflow;
@@ -166,7 +169,7 @@ static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
break;
case SO_TIMESTAMPING_NEW:
case SO_TIMESTAMPING_OLD:
- sock_set_timestamping(sk, optname, val);
+ sock_set_timestamping(sk, optname, val, optval, optlen);
break;
}
@@ -207,7 +210,8 @@ static int mptcp_setsockopt_sol_socket_int(struct mptcp_sock *msk, int optname,
case SO_TIMESTAMPNS_NEW:
case SO_TIMESTAMPING_OLD:
case SO_TIMESTAMPING_NEW:
- return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val);
+ return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val,
+ optval, optlen);
}
return -ENOPROTOOPT;
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 08/10] net: socket: support hardware timestamp conversion to PHC bound
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (6 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 09/10] selftests/net: timestamping: support binding PHC Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 10/10] MAINTAINERS: add entry for PTP virtual clock driver Yangbo Lu
9 siblings, 0 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
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 <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
net/socket.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 27e3e7d53f8e..ac07df4feb29 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;
@@ -825,12 +826,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 09/10] selftests/net: timestamping: support binding PHC
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (7 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 08/10] net: socket: support hardware timestamp conversion to PHC bound Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 10/10] MAINTAINERS: add entry for PTP virtual clock driver Yangbo Lu
9 siblings, 0 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
Support binding PHC of PTP vclock for timestamping.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
tools/testing/selftests/net/timestamping.c | 62 +++++++++++++++-------
1 file changed, 43 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/net/timestamping.c b/tools/testing/selftests/net/timestamping.c
index 21091be70688..abcbd7271187 100644
--- a/tools/testing/selftests/net/timestamping.c
+++ b/tools/testing/selftests/net/timestamping.c
@@ -43,11 +43,19 @@
# define SO_TIMESTAMPNS 35
#endif
+#ifndef SOF_TIMESTAMPING_BIND_PHC
+#define SOF_TIMESTAMPING_BIND_PHC (1<<15)
+struct so_timestamping {
+ int flags;
+ int bind_phc;
+};
+#endif
+
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 +66,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 +320,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 +333,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 +352,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 +371,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 +402,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 +430,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 +464,10 @@ int main(int argc, char **argv)
&enabled, sizeof(enabled)) < 0)
bail("setsockopt SO_TIMESTAMPNS");
- if (so_timestamping_flags &&
+ if (so_timestamping.flags &&
setsockopt(sock, SOL_SOCKET, SO_TIMESTAMPING,
- &so_timestamping_flags,
- sizeof(so_timestamping_flags)) < 0)
+ &so_timestamping,
+ sizeof(so_timestamping)) < 0)
bail("setsockopt SO_TIMESTAMPING");
/* request IP_PKTINFO for debugging purposes */
@@ -468,14 +488,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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [net-next, v3, 10/10] MAINTAINERS: add entry for PTP virtual clock driver
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
` (8 preceding siblings ...)
2021-06-15 9:45 ` [net-next, v3, 09/10] selftests/net: timestamping: support binding PHC Yangbo Lu
@ 2021-06-15 9:45 ` Yangbo Lu
9 siblings, 0 replies; 31+ messages in thread
From: Yangbo Lu @ 2021-06-15 9:45 UTC (permalink / raw)
To: netdev
Cc: Yangbo Lu, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Michal Kubecek, Florian Fainelli, Andrew Lunn,
Rui Sousa, Sebastien Laveze
Add entry for PTP virtual clock driver.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v3:
- Added this patch.
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 183cc61e2dc0..537f9f19dfa8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14841,6 +14841,13 @@ F: drivers/net/phy/dp83640*
F: drivers/ptp/*
F: include/linux/ptp_cl*
+PTP VIRTUAL CLOCK SUPPORT
+M: Yangbo Lu <yangbo.lu@nxp.com>
+L: netdev@vger.kernel.org
+S: Maintained
+F: drivers/ptp/ptp_vclock.c
+F: net/ethtool/phc_vclocks.c
+
PTRACE SUPPORT
M: Oleg Nesterov <oleg@redhat.com>
S: Maintained
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks
2021-06-15 9:45 ` [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks Yangbo Lu
@ 2021-06-15 19:49 ` Jakub Kicinski
2021-06-22 10:10 ` Y.b. Lu
2021-06-15 23:24 ` Michal Kubecek
1 sibling, 1 reply; 31+ messages in thread
From: Jakub Kicinski @ 2021-06-15 19:49 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
On Tue, 15 Jun 2021 17:45:12 +0800 Yangbo Lu wrote:
> 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 <yangbo.lu@nxp.com>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index cfef6b08169a..0fb04f945767 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -17,6 +17,7 @@
> #include <linux/const.h>
> #include <linux/types.h>
> #include <linux/if_ether.h>
> +#include <linux/ptp_clock.h>
>
> #ifndef __KERNEL__
> #include <limits.h> /* for INT_MAX */
> @@ -1341,6 +1342,18 @@ struct ethtool_ts_info {
> __u32 rx_reserved[3];
> };
>
> +/**
> + * struct ethtool_phc_vclocks - holds a device's PTP virtual clocks
> + * @cmd: command number = %ETHTOOL_GET_PHC_VCLOCKS
> + * @num: number of PTP vclocks
> + * @index: all index values of PTP vclocks
> + */
> +struct ethtool_phc_vclocks {
> + __u32 cmd;
> + __u8 num;
> + __s32 index[PTP_MAX_VCLOCKS];
> +};
> +
> /*
> * %ETHTOOL_SFEATURES changes features present in features[].valid to the
> * values of corresponding bits in features[].requested. Bits in .requested
> @@ -1552,6 +1565,7 @@ enum ethtool_fec_config_bits {
> #define ETHTOOL_PHY_STUNABLE 0x0000004f /* Set PHY tunable configuration */
> #define ETHTOOL_GFECPARAM 0x00000050 /* Get FEC settings */
> #define ETHTOOL_SFECPARAM 0x00000051 /* Set FEC settings */
> +#define ETHTOOL_GET_PHC_VCLOCKS 0x00000052 /* Get PHC virtual clocks info */
We don't add new IOCTL commands, only netlink API is going to be
extended. Please remove the IOCTL interface & uAPI.
> /* compatibility with older code */
> #define SPARC_ETH_GSET ETHTOOL_GSET
> +/* PHC VCLOCKS */
> +
> +enum {
> + ETHTOOL_A_PHC_VCLOCKS_UNSPEC,
> + ETHTOOL_A_PHC_VCLOCKS_HEADER, /* nest - _A_HEADER_* */
> + ETHTOOL_A_PHC_VCLOCKS_NUM, /* u8 */
u32, no need to limit yourself, the netlink attribute is rounded up to
4B anyway.
> + ETHTOOL_A_PHC_VCLOCKS_INDEX, /* s32 */
This is an array, AFAICT, not a single 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 {
> +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);
> + const struct ethtool_phc_vclocks *phc_vclocks = &data->phc_vclocks;
> +
> + if (phc_vclocks->num <= 0)
> + return 0;
> +
> + if (nla_put_u32(skb, ETHTOOL_A_PHC_VCLOCKS_NUM, phc_vclocks->num) ||
> + nla_put(skb, ETHTOOL_A_PHC_VCLOCKS_INDEX,
> + sizeof(phc_vclocks->index), phc_vclocks->index))
Looks like you'll report the whole array, why not just num?
> + return -EMSGSIZE;
> +
> + return 0;
> +}
> +
> +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,
> +};
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks
2021-06-15 9:45 ` [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks Yangbo Lu
2021-06-15 19:49 ` Jakub Kicinski
@ 2021-06-15 23:24 ` Michal Kubecek
2021-06-22 10:10 ` Y.b. Lu
1 sibling, 1 reply; 31+ messages in thread
From: Michal Kubecek @ 2021-06-15 23:24 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
On Tue, Jun 15, 2021 at 05:45:12PM +0800, Yangbo Lu wrote:
> 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 <yangbo.lu@nxp.com>
> ---
> Changes for v3:
> - Added this patch.
> ---
> include/linux/ethtool.h | 2 +
> include/uapi/linux/ethtool.h | 14 +++++
> include/uapi/linux/ethtool_netlink.h | 15 +++++
> net/ethtool/Makefile | 2 +-
> net/ethtool/common.c | 23 ++++++++
> net/ethtool/common.h | 2 +
> net/ethtool/ioctl.c | 27 +++++++++
> net/ethtool/netlink.c | 10 ++++
> net/ethtool/netlink.h | 2 +
> net/ethtool/phc_vclocks.c | 86 ++++++++++++++++++++++++++++
> 10 files changed, 182 insertions(+), 1 deletion(-)
> create mode 100644 net/ethtool/phc_vclocks.c
When updating the ethtool netlink API, please update also its
documentation in Documentation/networking/ethtool-netlink.rst
Michal
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
2021-06-15 9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
@ 2021-06-16 1:00 ` Mat Martineau
2021-06-16 10:27 ` kernel test robot
2021-06-16 10:39 ` kernel test robot
2 siblings, 0 replies; 31+ messages in thread
From: Mat Martineau @ 2021-06-16 1:00 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze, Florian Westphal
On Tue, 15 Jun 2021, 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.
>
> 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 <yangbo.lu@nxp.com>
> ---
> Changes for v3:
> - Added this patch.
> ---
> 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 | 10 +++--
> 5 files changed, 91 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 9b341c2c924f..adb6c0edc867 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;
> @@ -2744,7 +2747,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, int val,
> + sockptr_t optval, unsigned int optlen);
>
> 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..9d4cde4ef7e6 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 ddfa88082a2b..70d2c2322429 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);
>
> @@ -794,8 +796,53 @@ 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 ethtool_phc_vclocks phc_vclocks = { };
> + struct net *net = sock_net(sk);
> + struct net_device *dev = NULL;
> + bool match = false;
> + int i;
> +
> + 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;
> + }
> +
> + ethtool_op_get_phc_vclocks(dev, &phc_vclocks);
> +
> + for (i = 0; i < phc_vclocks.num; i++) {
> + if (phc_vclocks.index[i] == phc_index) {
> + match = true;
> + break;
> + }
> + }
> +
> + if (!match)
> + return -EINVAL;
> +
> + sk->sk_bind_phc = phc_index;
> +
> + return 0;
> +}
> +
> +int sock_set_timestamping(struct sock *sk, int optname, int val,
> + sockptr_t optval, unsigned int optlen)
> +{
> + struct so_timestamping timestamping;
> + int ret;
> +
> + if (optlen == sizeof(timestamping)) {
> + if (copy_from_sockptr(×tamping, optval,
> + sizeof(timestamping)))
> + return -EFAULT;
> +
> + val = timestamping.flags;
> + }
> +
> if (val & ~SOF_TIMESTAMPING_MASK)
> return -EINVAL;
>
> @@ -816,6 +863,15 @@ int sock_set_timestamping(struct sock *sk, int optname, int val)
> !(val & SOF_TIMESTAMPING_OPT_TSONLY))
> return -EINVAL;
>
> + if (optlen == sizeof(timestamping) &&
> + val & SOF_TIMESTAMPING_BIND_PHC) {
> + ret = sock_timestamping_bind_phc(sk, timestamping.bind_phc);
> + if (ret)
> + return ret;
> + } else {
> + sk->sk_bind_phc = -1;
> + }
> +
> sk->sk_tsflags = val;
> sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);
>
> @@ -1057,7 +1113,7 @@ 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);
> + ret = sock_set_timestamping(sk, optname, val, optval, optlen);
> break;
>
> case SO_RCVLOWAT:
> @@ -1332,6 +1388,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);
> @@ -1435,7 +1492,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 14035f2dc6d4..4bf1bd3a3db6 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 092d1f635d27..2ca90b230827 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -140,7 +140,10 @@ static void mptcp_so_incoming_cpu(struct mptcp_sock *msk, int val)
> mptcp_sol_socket_sync_intval(msk, SO_INCOMING_CPU, val);
> }
>
> -static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optname, int val)
> +static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
> + int optname, int val,
> + sockptr_t optval,
> + unsigned int optlen)
> {
> sockptr_t optval = KERNEL_SOCKPTR(&val);
> struct mptcp_subflow_context *subflow;
> @@ -166,7 +169,7 @@ static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
> break;
> case SO_TIMESTAMPING_NEW:
> case SO_TIMESTAMPING_OLD:
> - sock_set_timestamping(sk, optname, val);
> + sock_set_timestamping(sk, optname, val, optval, optlen);
This is inside a loop, so in cases where optlen == sizeof(struct
so_timestamping) this will end up re-copying the structure from userspace
one extra time for each MPTCP subflow: once for the MPTCP socket, plus one
time for each of the TCP subflows that are grouped under this MPTCP
connection.
Given that the extra copies only happen when using the extended bind_phc
option, it's not a huge cost. But sock_set_timestamping() was written to
avoid the extra copies for 'int' sized options, and if that was worth the
effort then the larger so_timestamping structure could be copied (once)
before the sock_set_timestamping() call and passed in.
> break;
> }
>
> @@ -207,7 +210,8 @@ static int mptcp_setsockopt_sol_socket_int(struct mptcp_sock *msk, int optname,
> case SO_TIMESTAMPNS_NEW:
> case SO_TIMESTAMPING_OLD:
> case SO_TIMESTAMPING_NEW:
> - return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val);
> + return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val,
> + optval, optlen);
Rather than modifying mptcp_setsockopt_sol_socket_int(), I suggest adding
a mptcp_setsockopt_sol_socket_timestamping() helper function that can
handle the special copying for so_timestamping.
> }
>
> return -ENOPROTOOPT;
> --
> 2.25.1
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
@ 2021-06-16 1:00 ` Mat Martineau
0 siblings, 0 replies; 31+ messages in thread
From: Mat Martineau @ 2021-06-16 1:00 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze, Florian Westphal
On Tue, 15 Jun 2021, 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.
>
> 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 <yangbo.lu@nxp.com>
> ---
> Changes for v3:
> - Added this patch.
> ---
> 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 | 10 +++--
> 5 files changed, 91 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 9b341c2c924f..adb6c0edc867 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;
> @@ -2744,7 +2747,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, int val,
> + sockptr_t optval, unsigned int optlen);
>
> 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..9d4cde4ef7e6 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 ddfa88082a2b..70d2c2322429 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);
>
> @@ -794,8 +796,53 @@ 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 ethtool_phc_vclocks phc_vclocks = { };
> + struct net *net = sock_net(sk);
> + struct net_device *dev = NULL;
> + bool match = false;
> + int i;
> +
> + 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;
> + }
> +
> + ethtool_op_get_phc_vclocks(dev, &phc_vclocks);
> +
> + for (i = 0; i < phc_vclocks.num; i++) {
> + if (phc_vclocks.index[i] == phc_index) {
> + match = true;
> + break;
> + }
> + }
> +
> + if (!match)
> + return -EINVAL;
> +
> + sk->sk_bind_phc = phc_index;
> +
> + return 0;
> +}
> +
> +int sock_set_timestamping(struct sock *sk, int optname, int val,
> + sockptr_t optval, unsigned int optlen)
> +{
> + struct so_timestamping timestamping;
> + int ret;
> +
> + if (optlen == sizeof(timestamping)) {
> + if (copy_from_sockptr(×tamping, optval,
> + sizeof(timestamping)))
> + return -EFAULT;
> +
> + val = timestamping.flags;
> + }
> +
> if (val & ~SOF_TIMESTAMPING_MASK)
> return -EINVAL;
>
> @@ -816,6 +863,15 @@ int sock_set_timestamping(struct sock *sk, int optname, int val)
> !(val & SOF_TIMESTAMPING_OPT_TSONLY))
> return -EINVAL;
>
> + if (optlen == sizeof(timestamping) &&
> + val & SOF_TIMESTAMPING_BIND_PHC) {
> + ret = sock_timestamping_bind_phc(sk, timestamping.bind_phc);
> + if (ret)
> + return ret;
> + } else {
> + sk->sk_bind_phc = -1;
> + }
> +
> sk->sk_tsflags = val;
> sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);
>
> @@ -1057,7 +1113,7 @@ 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);
> + ret = sock_set_timestamping(sk, optname, val, optval, optlen);
> break;
>
> case SO_RCVLOWAT:
> @@ -1332,6 +1388,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);
> @@ -1435,7 +1492,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 14035f2dc6d4..4bf1bd3a3db6 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 092d1f635d27..2ca90b230827 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -140,7 +140,10 @@ static void mptcp_so_incoming_cpu(struct mptcp_sock *msk, int val)
> mptcp_sol_socket_sync_intval(msk, SO_INCOMING_CPU, val);
> }
>
> -static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optname, int val)
> +static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
> + int optname, int val,
> + sockptr_t optval,
> + unsigned int optlen)
> {
> sockptr_t optval = KERNEL_SOCKPTR(&val);
> struct mptcp_subflow_context *subflow;
> @@ -166,7 +169,7 @@ static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
> break;
> case SO_TIMESTAMPING_NEW:
> case SO_TIMESTAMPING_OLD:
> - sock_set_timestamping(sk, optname, val);
> + sock_set_timestamping(sk, optname, val, optval, optlen);
This is inside a loop, so in cases where optlen == sizeof(struct
so_timestamping) this will end up re-copying the structure from userspace
one extra time for each MPTCP subflow: once for the MPTCP socket, plus one
time for each of the TCP subflows that are grouped under this MPTCP
connection.
Given that the extra copies only happen when using the extended bind_phc
option, it's not a huge cost. But sock_set_timestamping() was written to
avoid the extra copies for 'int' sized options, and if that was worth the
effort then the larger so_timestamping structure could be copied (once)
before the sock_set_timestamping() call and passed in.
> break;
> }
>
> @@ -207,7 +210,8 @@ static int mptcp_setsockopt_sol_socket_int(struct mptcp_sock *msk, int optname,
> case SO_TIMESTAMPNS_NEW:
> case SO_TIMESTAMPING_OLD:
> case SO_TIMESTAMPING_NEW:
> - return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val);
> + return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val,
> + optval, optlen);
Rather than modifying mptcp_setsockopt_sol_socket_int(), I suggest adding
a mptcp_setsockopt_sol_socket_timestamping() helper function that can
handle the special copying for so_timestamping.
> }
>
> return -ENOPROTOOPT;
> --
> 2.25.1
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
2021-06-15 9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
@ 2021-06-16 10:27 ` kernel test robot
2021-06-16 10:27 ` kernel test robot
2021-06-16 10:39 ` kernel test robot
2 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2021-06-16 10:27 UTC (permalink / raw)
To: Yangbo Lu, netdev
Cc: kbuild-all, Yangbo Lu, linux-kernel, linux-kselftest, mptcp,
Richard Cochran, David S . Miller, Jakub Kicinski, Mat Martineau,
Matthieu Baerts
[-- Attachment #1: Type: text/plain, Size: 4497 bytes --]
Hi Yangbo,
I love your patch! Yet something to improve:
[auto build test ERROR on 89212e160b81e778f829b89743570665810e3b13]
url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
base: 89212e160b81e778f829b89743570665810e3b13
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/f03864a45f4fe97414824545398c837eead55409
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
git checkout f03864a45f4fe97414824545398c837eead55409
# save the attached .config to linux build tree
make W=1 ARCH=um SUBARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/mptcp/sockopt.c: In function 'mptcp_setsockopt_sol_socket_tstamp':
>> net/mptcp/sockopt.c:148:12: error: 'optval' redeclared as different kind of symbol
148 | sockptr_t optval = KERNEL_SOCKPTR(&val);
| ^~~~~~
net/mptcp/sockopt.c:145:22: note: previous definition of 'optval' was here
145 | sockptr_t optval,
| ~~~~~~~~~~^~~~~~
vim +/optval +148 net/mptcp/sockopt.c
6f0d7198084c40 Florian Westphal 2021-04-15 142
f03864a45f4fe9 Yangbo Lu 2021-06-15 143 static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
f03864a45f4fe9 Yangbo Lu 2021-06-15 144 int optname, int val,
f03864a45f4fe9 Yangbo Lu 2021-06-15 145 sockptr_t optval,
f03864a45f4fe9 Yangbo Lu 2021-06-15 146 unsigned int optlen)
9061f24bf82ec2 Florian Westphal 2021-06-03 147 {
9061f24bf82ec2 Florian Westphal 2021-06-03 @148 sockptr_t optval = KERNEL_SOCKPTR(&val);
9061f24bf82ec2 Florian Westphal 2021-06-03 149 struct mptcp_subflow_context *subflow;
9061f24bf82ec2 Florian Westphal 2021-06-03 150 struct sock *sk = (struct sock *)msk;
9061f24bf82ec2 Florian Westphal 2021-06-03 151 int ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 152
9061f24bf82ec2 Florian Westphal 2021-06-03 153 ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
9061f24bf82ec2 Florian Westphal 2021-06-03 154 optval, sizeof(val));
9061f24bf82ec2 Florian Westphal 2021-06-03 155 if (ret)
9061f24bf82ec2 Florian Westphal 2021-06-03 156 return ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 157
9061f24bf82ec2 Florian Westphal 2021-06-03 158 lock_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 159 mptcp_for_each_subflow(msk, subflow) {
9061f24bf82ec2 Florian Westphal 2021-06-03 160 struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
9061f24bf82ec2 Florian Westphal 2021-06-03 161 bool slow = lock_sock_fast(ssk);
9061f24bf82ec2 Florian Westphal 2021-06-03 162
9061f24bf82ec2 Florian Westphal 2021-06-03 163 switch (optname) {
9061f24bf82ec2 Florian Westphal 2021-06-03 164 case SO_TIMESTAMP_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 165 case SO_TIMESTAMP_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 166 case SO_TIMESTAMPNS_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 167 case SO_TIMESTAMPNS_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 168 sock_set_timestamp(sk, optname, !!val);
9061f24bf82ec2 Florian Westphal 2021-06-03 169 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 170 case SO_TIMESTAMPING_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 171 case SO_TIMESTAMPING_OLD:
f03864a45f4fe9 Yangbo Lu 2021-06-15 172 sock_set_timestamping(sk, optname, val, optval, optlen);
9061f24bf82ec2 Florian Westphal 2021-06-03 173 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 174 }
9061f24bf82ec2 Florian Westphal 2021-06-03 175
9061f24bf82ec2 Florian Westphal 2021-06-03 176 unlock_sock_fast(ssk, slow);
9061f24bf82ec2 Florian Westphal 2021-06-03 177 }
9061f24bf82ec2 Florian Westphal 2021-06-03 178
9061f24bf82ec2 Florian Westphal 2021-06-03 179 release_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 180 return 0;
9061f24bf82ec2 Florian Westphal 2021-06-03 181 }
9061f24bf82ec2 Florian Westphal 2021-06-03 182
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8673 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
@ 2021-06-16 10:27 ` kernel test robot
0 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2021-06-16 10:27 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4578 bytes --]
Hi Yangbo,
I love your patch! Yet something to improve:
[auto build test ERROR on 89212e160b81e778f829b89743570665810e3b13]
url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
base: 89212e160b81e778f829b89743570665810e3b13
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/f03864a45f4fe97414824545398c837eead55409
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
git checkout f03864a45f4fe97414824545398c837eead55409
# save the attached .config to linux build tree
make W=1 ARCH=um SUBARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/mptcp/sockopt.c: In function 'mptcp_setsockopt_sol_socket_tstamp':
>> net/mptcp/sockopt.c:148:12: error: 'optval' redeclared as different kind of symbol
148 | sockptr_t optval = KERNEL_SOCKPTR(&val);
| ^~~~~~
net/mptcp/sockopt.c:145:22: note: previous definition of 'optval' was here
145 | sockptr_t optval,
| ~~~~~~~~~~^~~~~~
vim +/optval +148 net/mptcp/sockopt.c
6f0d7198084c40 Florian Westphal 2021-04-15 142
f03864a45f4fe9 Yangbo Lu 2021-06-15 143 static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
f03864a45f4fe9 Yangbo Lu 2021-06-15 144 int optname, int val,
f03864a45f4fe9 Yangbo Lu 2021-06-15 145 sockptr_t optval,
f03864a45f4fe9 Yangbo Lu 2021-06-15 146 unsigned int optlen)
9061f24bf82ec2 Florian Westphal 2021-06-03 147 {
9061f24bf82ec2 Florian Westphal 2021-06-03 @148 sockptr_t optval = KERNEL_SOCKPTR(&val);
9061f24bf82ec2 Florian Westphal 2021-06-03 149 struct mptcp_subflow_context *subflow;
9061f24bf82ec2 Florian Westphal 2021-06-03 150 struct sock *sk = (struct sock *)msk;
9061f24bf82ec2 Florian Westphal 2021-06-03 151 int ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 152
9061f24bf82ec2 Florian Westphal 2021-06-03 153 ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
9061f24bf82ec2 Florian Westphal 2021-06-03 154 optval, sizeof(val));
9061f24bf82ec2 Florian Westphal 2021-06-03 155 if (ret)
9061f24bf82ec2 Florian Westphal 2021-06-03 156 return ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 157
9061f24bf82ec2 Florian Westphal 2021-06-03 158 lock_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 159 mptcp_for_each_subflow(msk, subflow) {
9061f24bf82ec2 Florian Westphal 2021-06-03 160 struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
9061f24bf82ec2 Florian Westphal 2021-06-03 161 bool slow = lock_sock_fast(ssk);
9061f24bf82ec2 Florian Westphal 2021-06-03 162
9061f24bf82ec2 Florian Westphal 2021-06-03 163 switch (optname) {
9061f24bf82ec2 Florian Westphal 2021-06-03 164 case SO_TIMESTAMP_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 165 case SO_TIMESTAMP_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 166 case SO_TIMESTAMPNS_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 167 case SO_TIMESTAMPNS_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 168 sock_set_timestamp(sk, optname, !!val);
9061f24bf82ec2 Florian Westphal 2021-06-03 169 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 170 case SO_TIMESTAMPING_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 171 case SO_TIMESTAMPING_OLD:
f03864a45f4fe9 Yangbo Lu 2021-06-15 172 sock_set_timestamping(sk, optname, val, optval, optlen);
9061f24bf82ec2 Florian Westphal 2021-06-03 173 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 174 }
9061f24bf82ec2 Florian Westphal 2021-06-03 175
9061f24bf82ec2 Florian Westphal 2021-06-03 176 unlock_sock_fast(ssk, slow);
9061f24bf82ec2 Florian Westphal 2021-06-03 177 }
9061f24bf82ec2 Florian Westphal 2021-06-03 178
9061f24bf82ec2 Florian Westphal 2021-06-03 179 release_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 180 return 0;
9061f24bf82ec2 Florian Westphal 2021-06-03 181 }
9061f24bf82ec2 Florian Westphal 2021-06-03 182
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 8673 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
2021-06-15 9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
@ 2021-06-16 10:39 ` kernel test robot
2021-06-16 10:27 ` kernel test robot
2021-06-16 10:39 ` kernel test robot
2 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2021-06-16 10:39 UTC (permalink / raw)
To: Yangbo Lu, netdev
Cc: kbuild-all, Yangbo Lu, linux-kernel, linux-kselftest, mptcp,
Richard Cochran, David S . Miller, Jakub Kicinski, Mat Martineau,
Matthieu Baerts
[-- Attachment #1: Type: text/plain, Size: 4691 bytes --]
Hi Yangbo,
I love your patch! Yet something to improve:
[auto build test ERROR on 89212e160b81e778f829b89743570665810e3b13]
url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
base: 89212e160b81e778f829b89743570665810e3b13
config: arm-randconfig-r023-20210615 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f03864a45f4fe97414824545398c837eead55409
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
git checkout f03864a45f4fe97414824545398c837eead55409
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/mptcp/sockopt.c: In function 'mptcp_setsockopt_sol_socket_tstamp':
>> net/mptcp/sockopt.c:148:12: error: 'optval' redeclared as different kind of symbol
148 | sockptr_t optval = KERNEL_SOCKPTR(&val);
| ^~~~~~
net/mptcp/sockopt.c:145:22: note: previous definition of 'optval' was here
145 | sockptr_t optval,
| ~~~~~~~~~~^~~~~~
vim +/optval +148 net/mptcp/sockopt.c
6f0d7198084c40 Florian Westphal 2021-04-15 142
f03864a45f4fe9 Yangbo Lu 2021-06-15 143 static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
f03864a45f4fe9 Yangbo Lu 2021-06-15 144 int optname, int val,
f03864a45f4fe9 Yangbo Lu 2021-06-15 145 sockptr_t optval,
f03864a45f4fe9 Yangbo Lu 2021-06-15 146 unsigned int optlen)
9061f24bf82ec2 Florian Westphal 2021-06-03 147 {
9061f24bf82ec2 Florian Westphal 2021-06-03 @148 sockptr_t optval = KERNEL_SOCKPTR(&val);
9061f24bf82ec2 Florian Westphal 2021-06-03 149 struct mptcp_subflow_context *subflow;
9061f24bf82ec2 Florian Westphal 2021-06-03 150 struct sock *sk = (struct sock *)msk;
9061f24bf82ec2 Florian Westphal 2021-06-03 151 int ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 152
9061f24bf82ec2 Florian Westphal 2021-06-03 153 ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
9061f24bf82ec2 Florian Westphal 2021-06-03 154 optval, sizeof(val));
9061f24bf82ec2 Florian Westphal 2021-06-03 155 if (ret)
9061f24bf82ec2 Florian Westphal 2021-06-03 156 return ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 157
9061f24bf82ec2 Florian Westphal 2021-06-03 158 lock_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 159 mptcp_for_each_subflow(msk, subflow) {
9061f24bf82ec2 Florian Westphal 2021-06-03 160 struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
9061f24bf82ec2 Florian Westphal 2021-06-03 161 bool slow = lock_sock_fast(ssk);
9061f24bf82ec2 Florian Westphal 2021-06-03 162
9061f24bf82ec2 Florian Westphal 2021-06-03 163 switch (optname) {
9061f24bf82ec2 Florian Westphal 2021-06-03 164 case SO_TIMESTAMP_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 165 case SO_TIMESTAMP_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 166 case SO_TIMESTAMPNS_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 167 case SO_TIMESTAMPNS_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 168 sock_set_timestamp(sk, optname, !!val);
9061f24bf82ec2 Florian Westphal 2021-06-03 169 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 170 case SO_TIMESTAMPING_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 171 case SO_TIMESTAMPING_OLD:
f03864a45f4fe9 Yangbo Lu 2021-06-15 172 sock_set_timestamping(sk, optname, val, optval, optlen);
9061f24bf82ec2 Florian Westphal 2021-06-03 173 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 174 }
9061f24bf82ec2 Florian Westphal 2021-06-03 175
9061f24bf82ec2 Florian Westphal 2021-06-03 176 unlock_sock_fast(ssk, slow);
9061f24bf82ec2 Florian Westphal 2021-06-03 177 }
9061f24bf82ec2 Florian Westphal 2021-06-03 178
9061f24bf82ec2 Florian Westphal 2021-06-03 179 release_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 180 return 0;
9061f24bf82ec2 Florian Westphal 2021-06-03 181 }
9061f24bf82ec2 Florian Westphal 2021-06-03 182
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33280 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
@ 2021-06-16 10:39 ` kernel test robot
0 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2021-06-16 10:39 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4774 bytes --]
Hi Yangbo,
I love your patch! Yet something to improve:
[auto build test ERROR on 89212e160b81e778f829b89743570665810e3b13]
url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
base: 89212e160b81e778f829b89743570665810e3b13
config: arm-randconfig-r023-20210615 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f03864a45f4fe97414824545398c837eead55409
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210616-141518
git checkout f03864a45f4fe97414824545398c837eead55409
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/mptcp/sockopt.c: In function 'mptcp_setsockopt_sol_socket_tstamp':
>> net/mptcp/sockopt.c:148:12: error: 'optval' redeclared as different kind of symbol
148 | sockptr_t optval = KERNEL_SOCKPTR(&val);
| ^~~~~~
net/mptcp/sockopt.c:145:22: note: previous definition of 'optval' was here
145 | sockptr_t optval,
| ~~~~~~~~~~^~~~~~
vim +/optval +148 net/mptcp/sockopt.c
6f0d7198084c40 Florian Westphal 2021-04-15 142
f03864a45f4fe9 Yangbo Lu 2021-06-15 143 static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk,
f03864a45f4fe9 Yangbo Lu 2021-06-15 144 int optname, int val,
f03864a45f4fe9 Yangbo Lu 2021-06-15 145 sockptr_t optval,
f03864a45f4fe9 Yangbo Lu 2021-06-15 146 unsigned int optlen)
9061f24bf82ec2 Florian Westphal 2021-06-03 147 {
9061f24bf82ec2 Florian Westphal 2021-06-03 @148 sockptr_t optval = KERNEL_SOCKPTR(&val);
9061f24bf82ec2 Florian Westphal 2021-06-03 149 struct mptcp_subflow_context *subflow;
9061f24bf82ec2 Florian Westphal 2021-06-03 150 struct sock *sk = (struct sock *)msk;
9061f24bf82ec2 Florian Westphal 2021-06-03 151 int ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 152
9061f24bf82ec2 Florian Westphal 2021-06-03 153 ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
9061f24bf82ec2 Florian Westphal 2021-06-03 154 optval, sizeof(val));
9061f24bf82ec2 Florian Westphal 2021-06-03 155 if (ret)
9061f24bf82ec2 Florian Westphal 2021-06-03 156 return ret;
9061f24bf82ec2 Florian Westphal 2021-06-03 157
9061f24bf82ec2 Florian Westphal 2021-06-03 158 lock_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 159 mptcp_for_each_subflow(msk, subflow) {
9061f24bf82ec2 Florian Westphal 2021-06-03 160 struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
9061f24bf82ec2 Florian Westphal 2021-06-03 161 bool slow = lock_sock_fast(ssk);
9061f24bf82ec2 Florian Westphal 2021-06-03 162
9061f24bf82ec2 Florian Westphal 2021-06-03 163 switch (optname) {
9061f24bf82ec2 Florian Westphal 2021-06-03 164 case SO_TIMESTAMP_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 165 case SO_TIMESTAMP_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 166 case SO_TIMESTAMPNS_OLD:
9061f24bf82ec2 Florian Westphal 2021-06-03 167 case SO_TIMESTAMPNS_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 168 sock_set_timestamp(sk, optname, !!val);
9061f24bf82ec2 Florian Westphal 2021-06-03 169 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 170 case SO_TIMESTAMPING_NEW:
9061f24bf82ec2 Florian Westphal 2021-06-03 171 case SO_TIMESTAMPING_OLD:
f03864a45f4fe9 Yangbo Lu 2021-06-15 172 sock_set_timestamping(sk, optname, val, optval, optlen);
9061f24bf82ec2 Florian Westphal 2021-06-03 173 break;
9061f24bf82ec2 Florian Westphal 2021-06-03 174 }
9061f24bf82ec2 Florian Westphal 2021-06-03 175
9061f24bf82ec2 Florian Westphal 2021-06-03 176 unlock_sock_fast(ssk, slow);
9061f24bf82ec2 Florian Westphal 2021-06-03 177 }
9061f24bf82ec2 Florian Westphal 2021-06-03 178
9061f24bf82ec2 Florian Westphal 2021-06-03 179 release_sock(sk);
9061f24bf82ec2 Florian Westphal 2021-06-03 180 return 0;
9061f24bf82ec2 Florian Westphal 2021-06-03 181 }
9061f24bf82ec2 Florian Westphal 2021-06-03 182
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33280 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework
2021-06-15 9:45 ` [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework Yangbo Lu
@ 2021-06-17 17:32 ` Richard Cochran
2021-06-22 10:17 ` Y.b. Lu
0 siblings, 1 reply; 31+ messages in thread
From: Richard Cochran @ 2021-06-17 17:32 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
On Tue, Jun 15, 2021 at 05:45:08PM +0800, Yangbo Lu wrote:
> diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> index 8673d1743faa..3c6a905760e2 100644
> --- a/drivers/ptp/Makefile
> +++ b/drivers/ptp/Makefile
> @@ -3,7 +3,7 @@
> # Makefile for PTP 1588 clock support.
> #
>
> -ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
> +ptp-y := ptp_clock.o ptp_vclock.o ptp_chardev.o ptp_sysfs.o
Nit: Please place ptp_vclock.o at the end of the list.
> ptp_kvm-$(CONFIG_X86) := ptp_kvm_x86.o ptp_kvm_common.o
> ptp_kvm-$(CONFIG_HAVE_ARM_SMCCC) := ptp_kvm_arm.o ptp_kvm_common.o
> obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> index 6b97155148f1..3f388d63904c 100644
> --- a/drivers/ptp/ptp_private.h
> +++ b/drivers/ptp/ptp_private.h
> @@ -48,6 +48,20 @@ struct ptp_clock {
> struct kthread_delayed_work aux_work;
> };
>
> +#define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
> +#define cc_to_vclock(d) container_of((d), struct ptp_vclock, cc)
> +#define dw_to_vclock(d) container_of((d), struct ptp_vclock, refresh_work)
> +
> +struct ptp_vclock {
> + struct ptp_clock *pclock;
> + struct ptp_clock_info info;
> + struct ptp_clock *clock;
> + struct cyclecounter cc;
> + struct timecounter tc;
> + spinlock_t lock; /* protects tc/cc */
> + struct delayed_work refresh_work;
Can we please have a kthread worker here instead of work?
Experience shows that plain work can be delayed for a long, long time
on busy systems.
> +};
> +
> /*
> * The function queue_cnt() is safe for readers to call without
> * holding q->lock. Readers use this function to verify that the queue
> @@ -89,4 +103,6 @@ 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);
>
> +struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
> +void ptp_vclock_unregister(struct ptp_vclock *vclock);
> #endif
> diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
> new file mode 100644
> index 000000000000..b8f500677314
> --- /dev/null
> +++ b/drivers/ptp/ptp_vclock.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * PTP virtual clock driver
> + *
> + * Copyright 2021 NXP
> + */
> +#include <linux/slab.h>
> +#include "ptp_private.h"
> +
> +#define PTP_VCLOCK_CC_MULT (1 << 31)
> +#define PTP_VCLOCK_CC_SHIFT 31
These two are okay, but ...
> +#define PTP_VCLOCK_CC_MULT_NUM (1 << 9)
> +#define PTP_VCLOCK_CC_MULT_DEM 15625ULL
the similarity of naming is confusing for these two. They are only
used in the .adjfine method. How about this?
PTP_VCLOCK_FADJ_NUMERATOR, or even PTP_VCLOCK_FADJ_SHIFT (see below)
PTP_VCLOCK_FADJ_DENOMINATOR
> +#define PTP_VCLOCK_CC_REFRESH_INTERVAL (HZ * 2)
Consider dropping CC from the name.
PTP_VCLOCK_REFRESH_INTERVAL sounds good to me.
> +static int ptp_vclock_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> + struct ptp_vclock *vclock = info_to_vclock(ptp);
> + unsigned long flags;
> + s64 adj;
> +
> + adj = (s64)scaled_ppm * PTP_VCLOCK_CC_MULT_NUM;
Rather than
scaled_ppm * (1 << 9)
I suggest
scaled_ppm << 9
instead. I suppose a good compiler would replace the multiplication
with a bit shift, but it never hurts to spell it out.
> + adj = div_s64(adj, PTP_VCLOCK_CC_MULT_DEM);
> +
> + spin_lock_irqsave(&vclock->lock, flags);
> + timecounter_read(&vclock->tc);
> + vclock->cc.mult = PTP_VCLOCK_CC_MULT + adj;
> + spin_unlock_irqrestore(&vclock->lock, flags);
> +
> + return 0;
> +}
Thanks,
Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-15 9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
@ 2021-06-17 17:42 ` Richard Cochran
2021-06-22 10:18 ` Y.b. Lu
2021-06-17 18:27 ` Richard Cochran
2021-06-19 4:06 ` Richard Cochran
2 siblings, 1 reply; 31+ messages in thread
From: Richard Cochran @ 2021-06-17 17:42 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
> diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
> index 2363ad810ddb..2ef11b775f47 100644
> --- a/Documentation/ABI/testing/sysfs-ptp
> +++ b/Documentation/ABI/testing/sysfs-ptp
> @@ -61,6 +61,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 <yangbo.lu@nxp.com>
> +Description:
> + This file contains the ptp virtual clocks number in use,
> + based on current ptp physical clock. 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 ptp physical clock is
> + guaranteed to stay free running. Setting the value back
> + to 0 can delete ptp virtual clocks and back use ptp
> + physical clock again.
The native speaker in me suggests:
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.
Thanks,
Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-15 9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
2021-06-17 17:42 ` Richard Cochran
@ 2021-06-17 18:27 ` Richard Cochran
2021-06-22 10:35 ` Y.b. Lu
2021-06-19 4:06 ` Richard Cochran
2 siblings, 1 reply; 31+ messages in thread
From: Richard Cochran @ 2021-06-17 18:27 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> index 3f388d63904c..6949afc9d733 100644
> --- a/drivers/ptp/ptp_private.h
> +++ b/drivers/ptp/ptp_private.h
> @@ -46,6 +46,9 @@ struct ptp_clock {
> const struct attribute_group *pin_attr_groups[2];
> struct kthread_worker *kworker;
> struct kthread_delayed_work aux_work;
> + u8 n_vclocks;
Hm, type is u8, but ...
> + struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
> + bool vclock_flag;
> };
>
> #define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
> diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
> index 1d108d597f66..4b933dc1b81b 100644
> --- a/include/uapi/linux/ptp_clock.h
> +++ b/include/uapi/linux/ptp_clock.h
> @@ -69,6 +69,11 @@
> */
> #define PTP_PEROUT_V1_VALID_FLAGS (0)
>
> +/*
> + * Max number of PTP virtual clocks per PTP physical clock
> + */
> +#define PTP_MAX_VCLOCKS 20
Only 20 clocks are allowed? Why?
Thanks,
Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-15 9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
2021-06-17 17:42 ` Richard Cochran
2021-06-17 18:27 ` Richard Cochran
@ 2021-06-19 4:06 ` Richard Cochran
2021-06-22 10:39 ` Y.b. Lu
2 siblings, 1 reply; 31+ messages in thread
From: Richard Cochran @ 2021-06-19 4:06 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index a780435331c8..78414b3e16dd 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_guaranteed_pclock(ptp)) {
Can we please invent a more descriptive name for this method?
The word "guaranteed" suggests much more.
> + pr_err("ptp: virtual clock in use, guarantee physical clock free running\n");
This is good: ^^^^^^^^^^^^^^^^^^^^^^^^^
You can drop this part: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
So, please rename ptp_guaranteed_pclock() to ptp_vclock_in_use();
> + return -EBUSY;
> + }
> +
> return ptp->info->settime64(ptp->info, tp);
> }
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> index 3f388d63904c..6949afc9d733 100644
> --- a/drivers/ptp/ptp_private.h
> +++ b/drivers/ptp/ptp_private.h
> @@ -46,6 +46,9 @@ struct ptp_clock {
> const struct attribute_group *pin_attr_groups[2];
> struct kthread_worker *kworker;
> struct kthread_delayed_work aux_work;
> + u8 n_vclocks;
Why not use "unsigned int" type? I don't see a need to set an artificial limit.
> + struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
> + bool vclock_flag;
"flag" is vague. How about "is_virtual_clock" instead?
> };
>
> #define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
> @@ -75,6 +78,18 @@ static inline int queue_cnt(struct timestamp_event_queue *q)
> return cnt < 0 ? PTP_MAX_TIMESTAMPS + cnt : cnt;
> }
>
> +/*
> + * Guarantee physical clock to stay free running, if ptp virtual clocks
> + * on it are in use.
> + */
> +static inline bool ptp_guaranteed_pclock(struct ptp_clock *ptp)
> +{
> + if (!ptp->vclock_flag && ptp->n_vclocks)
Need to take mutex for n_vclocks to prevent load tearing.
> + return true;
> +
> + return false;
> +}
> +
> /*
> * see ptp_chardev.c
> */
> @@ -148,6 +149,90 @@ 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);
> +
> + return snprintf(page, PAGE_SIZE-1, "%d\n", ptp->n_vclocks);
Take mutex.
> +}
> +
> +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;
> + u8 num, i;
> +
> + if (kstrtou8(buf, 0, &num))
> + goto out;
> +
> + if (num > PTP_MAX_VCLOCKS) {
> + dev_err(dev, "max value is %d\n", PTP_MAX_VCLOCKS);
> + goto out;
> + }
> +
> + if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
> + return -ERESTARTSYS;
> +
> + /* 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) {
> + mutex_unlock(&ptp->n_vclocks_mux);
> + 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:
> + return err;
> +}
> +static DEVICE_ATTR_RW(n_vclocks);
> +
> static struct attribute *ptp_attrs[] = {
> &dev_attr_clock_name.attr,
>
> diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
> index 1d108d597f66..4b933dc1b81b 100644
> --- a/include/uapi/linux/ptp_clock.h
> +++ b/include/uapi/linux/ptp_clock.h
> @@ -69,6 +69,11 @@
> */
> #define PTP_PEROUT_V1_VALID_FLAGS (0)
>
> +/*
> + * Max number of PTP virtual clocks per PTP physical clock
> + */
> +#define PTP_MAX_VCLOCKS 20
Why limit this to twenty clocks?
Thanks,
Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks
2021-06-15 19:49 ` Jakub Kicinski
@ 2021-06-22 10:10 ` Y.b. Lu
0 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Jakub,
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: 2021年6月16日 3:49
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; Richard Cochran
> <richardcochran@gmail.com>; David S . Miller <davem@davemloft.net>; Mat
> Martineau <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 05/10] ethtool: add a new command for getting PHC
> virtual clocks
>
> On Tue, 15 Jun 2021 17:45:12 +0800 Yangbo Lu wrote:
> > 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 <yangbo.lu@nxp.com>
>
> > diff --git a/include/uapi/linux/ethtool.h
> > b/include/uapi/linux/ethtool.h index cfef6b08169a..0fb04f945767 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -17,6 +17,7 @@
> > #include <linux/const.h>
> > #include <linux/types.h>
> > #include <linux/if_ether.h>
> > +#include <linux/ptp_clock.h>
> >
> > #ifndef __KERNEL__
> > #include <limits.h> /* for INT_MAX */ @@ -1341,6 +1342,18 @@ struct
> > ethtool_ts_info {
> > __u32 rx_reserved[3];
> > };
> >
> > +/**
> > + * struct ethtool_phc_vclocks - holds a device's PTP virtual clocks
> > + * @cmd: command number = %ETHTOOL_GET_PHC_VCLOCKS
> > + * @num: number of PTP vclocks
> > + * @index: all index values of PTP vclocks */ struct
> > +ethtool_phc_vclocks {
> > + __u32 cmd;
> > + __u8 num;
> > + __s32 index[PTP_MAX_VCLOCKS];
> > +};
> > +
> > /*
> > * %ETHTOOL_SFEATURES changes features present in features[].valid to
> the
> > * values of corresponding bits in features[].requested. Bits in
> > .requested @@ -1552,6 +1565,7 @@ enum ethtool_fec_config_bits {
> > #define ETHTOOL_PHY_STUNABLE 0x0000004f /* Set PHY tunable
> configuration */
> > #define ETHTOOL_GFECPARAM 0x00000050 /* Get FEC settings */
> > #define ETHTOOL_SFECPARAM 0x00000051 /* Set FEC settings */
> > +#define ETHTOOL_GET_PHC_VCLOCKS 0x00000052 /* Get PHC virtual
> clocks info */
>
> We don't add new IOCTL commands, only netlink API is going to be extended.
> Please remove the IOCTL interface & uAPI.
Will remove. Thanks.
>
> > /* compatibility with older code */
> > #define SPARC_ETH_GSET ETHTOOL_GSET
>
> > +/* PHC VCLOCKS */
> > +
> > +enum {
> > + ETHTOOL_A_PHC_VCLOCKS_UNSPEC,
> > + ETHTOOL_A_PHC_VCLOCKS_HEADER, /* nest - _A_HEADER_*
> */
> > + ETHTOOL_A_PHC_VCLOCKS_NUM, /* u8 */
>
> u32, no need to limit yourself, the netlink attribute is rounded up to 4B
> anyway.
Get it. Will use u32.
>
> > + ETHTOOL_A_PHC_VCLOCKS_INDEX, /* s32 */
>
> This is an array, AFAICT, not a single s32.
Will fix. Thanks.
>
> > +
> > + /* add new constants above here */
> > + __ETHTOOL_A_PHC_VCLOCKS_CNT,
> > + ETHTOOL_A_PHC_VCLOCKS_MAX = (__ETHTOOL_A_PHC_VCLOCKS_CNT -
> 1) };
> > +
> > /* CABLE TEST */
> >
> > enum {
>
> > +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);
> > + const struct ethtool_phc_vclocks *phc_vclocks = &data->phc_vclocks;
> > +
> > + if (phc_vclocks->num <= 0)
> > + return 0;
> > +
> > + if (nla_put_u32(skb, ETHTOOL_A_PHC_VCLOCKS_NUM, phc_vclocks->num)
> ||
> > + nla_put(skb, ETHTOOL_A_PHC_VCLOCKS_INDEX,
> > + sizeof(phc_vclocks->index), phc_vclocks->index))
>
> Looks like you'll report the whole array, why not just num?
Will report just num. Thanks.
>
> > + return -EMSGSIZE;
> > +
> > + return 0;
> > +}
> > +
> > +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,
> > +};
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks
2021-06-15 23:24 ` Michal Kubecek
@ 2021-06-22 10:10 ` Y.b. Lu
0 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:10 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Mat Martineau, Matthieu Baerts,
Shuah Khan, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Michal,
> -----Original Message-----
> From: Michal Kubecek <mkubecek@suse.cz>
> Sent: 2021年6月16日 7:25
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; Richard Cochran
> <richardcochran@gmail.com>; David S . Miller <davem@davemloft.net>;
> Jakub Kicinski <kuba@kernel.org>; Mat Martineau
> <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Florian
> Fainelli <f.fainelli@gmail.com>; Andrew Lunn <andrew@lunn.ch>; Rui Sousa
> <rui.sousa@nxp.com>; Sebastien Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 05/10] ethtool: add a new command for getting PHC
> virtual clocks
>
> On Tue, Jun 15, 2021 at 05:45:12PM +0800, Yangbo Lu wrote:
> > 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 <yangbo.lu@nxp.com>
> > ---
> > Changes for v3:
> > - Added this patch.
> > ---
> > include/linux/ethtool.h | 2 +
> > include/uapi/linux/ethtool.h | 14 +++++
> > include/uapi/linux/ethtool_netlink.h | 15 +++++
> > net/ethtool/Makefile | 2 +-
> > net/ethtool/common.c | 23 ++++++++
> > net/ethtool/common.h | 2 +
> > net/ethtool/ioctl.c | 27 +++++++++
> > net/ethtool/netlink.c | 10 ++++
> > net/ethtool/netlink.h | 2 +
> > net/ethtool/phc_vclocks.c | 86
> ++++++++++++++++++++++++++++
> > 10 files changed, 182 insertions(+), 1 deletion(-) create mode
> > 100644 net/ethtool/phc_vclocks.c
>
> When updating the ethtool netlink API, please update also its documentation
> in Documentation/networking/ethtool-netlink.rst
Will update doc. Thank you.
>
> Michal
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
2021-06-16 1:00 ` Mat Martineau
(?)
@ 2021-06-22 10:14 ` Y.b. Lu
-1 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:14 UTC (permalink / raw)
To: Mat Martineau
Cc: netdev, linux-kernel, linux-kselftest, mptcp, Richard Cochran,
David S . Miller, Jakub Kicinski, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze, Florian Westphal
Hi Mat,
> -----Original Message-----
> From: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Sent: 2021年6月16日 9:01
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; Richard Cochran
> <richardcochran@gmail.com>; David S . Miller <davem@davemloft.net>;
> Jakub Kicinski <kuba@kernel.org>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>; Florian Westphal <fw@strlen.de>
> Subject: Re: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for
> PHC binding
>
> On Tue, 15 Jun 2021, 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.
> >
> > 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 <yangbo.lu@nxp.com>
> > ---
> > Changes for v3:
> > - Added this patch.
> > ---
> > 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 | 10 +++--
> > 5 files changed, 91 insertions(+), 10 deletions(-)
> >
[...]
> > @@ -166,7 +169,7 @@ static int
> mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
> > break;
> > case SO_TIMESTAMPING_NEW:
> > case SO_TIMESTAMPING_OLD:
> > - sock_set_timestamping(sk, optname, val);
> > + sock_set_timestamping(sk, optname, val, optval, optlen);
>
> This is inside a loop, so in cases where optlen == sizeof(struct
> so_timestamping) this will end up re-copying the structure from userspace
> one extra time for each MPTCP subflow: once for the MPTCP socket, plus one
> time for each of the TCP subflows that are grouped under this MPTCP
> connection.
>
> Given that the extra copies only happen when using the extended bind_phc
> option, it's not a huge cost. But sock_set_timestamping() was written to
> avoid the extra copies for 'int' sized options, and if that was worth the
> effort then the larger so_timestamping structure could be copied (once)
> before the sock_set_timestamping() call and passed in.
I see now...
Let me pass so_timestamping structure in to avoid re-copying from userspace.
>
> > break;
> > }
> >
> > @@ -207,7 +210,8 @@ static int mptcp_setsockopt_sol_socket_int(struct
> mptcp_sock *msk, int optname,
> > case SO_TIMESTAMPNS_NEW:
> > case SO_TIMESTAMPING_OLD:
> > case SO_TIMESTAMPING_NEW:
> > - return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val);
> > + return mptcp_setsockopt_sol_socket_tstamp(msk, optname, val,
> > + optval, optlen);
>
> Rather than modifying mptcp_setsockopt_sol_socket_int(), I suggest adding
> a mptcp_setsockopt_sol_socket_timestamping() helper function that can
> handle the special copying for so_timestamping.
Thank you. Let me do this in next version.
>
> > }
> >
> > return -ENOPROTOOPT;
> > --
> > 2.25.1
> >
> >
>
> --
> Mat Martineau
> Intel
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework
2021-06-17 17:32 ` Richard Cochran
@ 2021-06-22 10:17 ` Y.b. Lu
0 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:17 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Richard,
> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年6月18日 1:33
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; David S . Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mat Martineau
> <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework
>
> On Tue, Jun 15, 2021 at 05:45:08PM +0800, Yangbo Lu wrote:
> > diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile index
> > 8673d1743faa..3c6a905760e2 100644
> > --- a/drivers/ptp/Makefile
> > +++ b/drivers/ptp/Makefile
> > @@ -3,7 +3,7 @@
> > # Makefile for PTP 1588 clock support.
> > #
> >
> > -ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
> > +ptp-y := ptp_clock.o ptp_vclock.o ptp_chardev.o
> ptp_sysfs.o
>
> Nit: Please place ptp_vclock.o at the end of the list.
Ok.
>
> > ptp_kvm-$(CONFIG_X86) := ptp_kvm_x86.o ptp_kvm_common.o
> > ptp_kvm-$(CONFIG_HAVE_ARM_SMCCC) := ptp_kvm_arm.o
> ptp_kvm_common.o
> > obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>
> > diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> > index 6b97155148f1..3f388d63904c 100644
> > --- a/drivers/ptp/ptp_private.h
> > +++ b/drivers/ptp/ptp_private.h
> > @@ -48,6 +48,20 @@ struct ptp_clock {
> > struct kthread_delayed_work aux_work; };
> >
> > +#define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
> > +#define cc_to_vclock(d) container_of((d), struct ptp_vclock, cc)
> > +#define dw_to_vclock(d) container_of((d), struct ptp_vclock,
> > +refresh_work)
> > +
> > +struct ptp_vclock {
> > + struct ptp_clock *pclock;
> > + struct ptp_clock_info info;
> > + struct ptp_clock *clock;
> > + struct cyclecounter cc;
> > + struct timecounter tc;
> > + spinlock_t lock; /* protects tc/cc */
> > + struct delayed_work refresh_work;
>
> Can we please have a kthread worker here instead of work?
>
> Experience shows that plain work can be delayed for a long, long time on busy
> systems.
>
I think do_aux_work callback could be utilized for ptp virtual clock, right?
> > +};
> > +
> > /*
> > * The function queue_cnt() is safe for readers to call without
> > * holding q->lock. Readers use this function to verify that the
> > queue @@ -89,4 +103,6 @@ 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);
> >
> > +struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
> > +void ptp_vclock_unregister(struct ptp_vclock *vclock);
> > #endif
>
> > diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c new
> > file mode 100644 index 000000000000..b8f500677314
> > --- /dev/null
> > +++ b/drivers/ptp/ptp_vclock.c
> > @@ -0,0 +1,154 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * PTP virtual clock driver
> > + *
> > + * Copyright 2021 NXP
> > + */
> > +#include <linux/slab.h>
> > +#include "ptp_private.h"
> > +
> > +#define PTP_VCLOCK_CC_MULT (1 << 31)
> > +#define PTP_VCLOCK_CC_SHIFT 31
>
> These two are okay, but ...
>
> > +#define PTP_VCLOCK_CC_MULT_NUM (1 << 9)
> > +#define PTP_VCLOCK_CC_MULT_DEM 15625ULL
>
> the similarity of naming is confusing for these two. They are only used in
> the .adjfine method. How about this?
>
> PTP_VCLOCK_FADJ_NUMERATOR, or even PTP_VCLOCK_FADJ_SHIFT (see
> below)
> PTP_VCLOCK_FADJ_DENOMINATOR
>
> > +#define PTP_VCLOCK_CC_REFRESH_INTERVAL (HZ * 2)
>
> Consider dropping CC from the name.
> PTP_VCLOCK_REFRESH_INTERVAL sounds good to me.
>
Thanks. Will rename the MACROs per your suggestion.
> > +static int ptp_vclock_adjfine(struct ptp_clock_info *ptp, long
> > +scaled_ppm) {
> > + struct ptp_vclock *vclock = info_to_vclock(ptp);
> > + unsigned long flags;
> > + s64 adj;
> > +
> > + adj = (s64)scaled_ppm * PTP_VCLOCK_CC_MULT_NUM;
>
> Rather than
>
> scaled_ppm * (1 << 9)
>
> I suggest
>
> scaled_ppm << 9
>
> instead. I suppose a good compiler would replace the multiplication with a
> bit shift, but it never hurts to spell it out.
Ok.
>
> > + adj = div_s64(adj, PTP_VCLOCK_CC_MULT_DEM);
> > +
> > + spin_lock_irqsave(&vclock->lock, flags);
> > + timecounter_read(&vclock->tc);
> > + vclock->cc.mult = PTP_VCLOCK_CC_MULT + adj;
> > + spin_unlock_irqrestore(&vclock->lock, flags);
> > +
> > + return 0;
> > +}
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-17 17:42 ` Richard Cochran
@ 2021-06-22 10:18 ` Y.b. Lu
0 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:18 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Richard,
> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年6月18日 1:43
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; David S . Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mat Martineau
> <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks
> conversion
>
> On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
>
> > diff --git a/Documentation/ABI/testing/sysfs-ptp
> b/Documentation/ABI/testing/sysfs-ptp
> > index 2363ad810ddb..2ef11b775f47 100644
> > --- a/Documentation/ABI/testing/sysfs-ptp
> > +++ b/Documentation/ABI/testing/sysfs-ptp
> > @@ -61,6 +61,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 <yangbo.lu@nxp.com>
> > +Description:
> > + This file contains the ptp virtual clocks number in use,
> > + based on current ptp physical clock. 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 ptp physical clock is
> > + guaranteed to stay free running. Setting the value back
> > + to 0 can delete ptp virtual clocks and back use ptp
> > + physical clock again.
>
> The native speaker in me suggests:
>
> 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.
Thank you! Will update. That's better than mine.
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-17 18:27 ` Richard Cochran
@ 2021-06-22 10:35 ` Y.b. Lu
2021-06-25 11:09 ` Y.b. Lu
0 siblings, 1 reply; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:35 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Richard,
> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年6月18日 2:28
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; David S . Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mat Martineau
> <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks
> conversion
>
> On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
>
> > diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> > index 3f388d63904c..6949afc9d733 100644
> > --- a/drivers/ptp/ptp_private.h
> > +++ b/drivers/ptp/ptp_private.h
> > @@ -46,6 +46,9 @@ struct ptp_clock {
> > const struct attribute_group *pin_attr_groups[2];
> > struct kthread_worker *kworker;
> > struct kthread_delayed_work aux_work;
> > + u8 n_vclocks;
>
> Hm, type is u8, but ...
>
> > + struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
> > + bool vclock_flag;
> > };
> >
>
> > #define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
> > diff --git a/include/uapi/linux/ptp_clock.h
> > b/include/uapi/linux/ptp_clock.h index 1d108d597f66..4b933dc1b81b
> > 100644
> > --- a/include/uapi/linux/ptp_clock.h
> > +++ b/include/uapi/linux/ptp_clock.h
> > @@ -69,6 +69,11 @@
> > */
> > #define PTP_PEROUT_V1_VALID_FLAGS (0)
> >
> > +/*
> > + * Max number of PTP virtual clocks per PTP physical clock */
> > +#define PTP_MAX_VCLOCKS 20
>
> Only 20 clocks are allowed? Why?
Initially I think vclock can be used for ptp multiple domains synchronization. Since the PTP domainValue is u8, u8 vclock number is large enough.
This is not a good idea to hard-code a PTP_MAX_VCLOCKS value. But it looks a little crazy to create numbers of vclocks via one command (echo n > /sys/class/ptp/ptp0/n_vclocks).
Maybe a typo creates hundreds of vclocks we don’t need.
Do you think we should be care about setting a limitation of vclock number? Any suggestion for implementation?
Thanks.
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-19 4:06 ` Richard Cochran
@ 2021-06-22 10:39 ` Y.b. Lu
0 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-22 10:39 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Richard,
> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年6月19日 12:06
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; David S . Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mat Martineau
> <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks
> conversion
>
> On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
>
> > diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c index
> > a780435331c8..78414b3e16dd 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_guaranteed_pclock(ptp)) {
>
> Can we please invent a more descriptive name for this method?
> The word "guaranteed" suggests much more.
>
> > + pr_err("ptp: virtual clock in use, guarantee physical clock free
> > +running\n");
>
> This is good: ^^^^^^^^^^^^^^^^^^^^^^^^^
> You can drop this part:
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> So, please rename ptp_guaranteed_pclock() to ptp_vclock_in_use();
>
Thank you. Will convert to that.
> > + return -EBUSY;
> > + }
> > +
> > return ptp->info->settime64(ptp->info, tp); }
>
>
> > diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> > index 3f388d63904c..6949afc9d733 100644
> > --- a/drivers/ptp/ptp_private.h
> > +++ b/drivers/ptp/ptp_private.h
> > @@ -46,6 +46,9 @@ struct ptp_clock {
> > const struct attribute_group *pin_attr_groups[2];
> > struct kthread_worker *kworker;
> > struct kthread_delayed_work aux_work;
> > + u8 n_vclocks;
>
> Why not use "unsigned int" type? I don't see a need to set an artificial limit.
Please see my explain in another email thread. Thanks.
>
> > + struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
> > + bool vclock_flag;
>
> "flag" is vague. How about "is_virtual_clock" instead?
That's better. Will use it. Thank you.
>
> > };
> >
> > #define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
> > @@ -75,6 +78,18 @@ static inline int queue_cnt(struct
> timestamp_event_queue *q)
> > return cnt < 0 ? PTP_MAX_TIMESTAMPS + cnt : cnt; }
> >
> > +/*
> > + * Guarantee physical clock to stay free running, if ptp virtual
> > +clocks
> > + * on it are in use.
> > + */
> > +static inline bool ptp_guaranteed_pclock(struct ptp_clock *ptp) {
> > + if (!ptp->vclock_flag && ptp->n_vclocks)
>
> Need to take mutex for n_vclocks to prevent load tearing.
>
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > /*
> > * see ptp_chardev.c
> > */
>
> > @@ -148,6 +149,90 @@ 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);
> > +
> > + return snprintf(page, PAGE_SIZE-1, "%d\n", ptp->n_vclocks);
>
> Take mutex.
Will take mutex everywhere to access it.
>
> > +}
> > +
> > +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;
> > + u8 num, i;
> > +
> > + if (kstrtou8(buf, 0, &num))
> > + goto out;
> > +
> > + if (num > PTP_MAX_VCLOCKS) {
> > + dev_err(dev, "max value is %d\n", PTP_MAX_VCLOCKS);
> > + goto out;
> > + }
> > +
> > + if (mutex_lock_interruptible(&ptp->n_vclocks_mux))
> > + return -ERESTARTSYS;
> > +
> > + /* 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) {
> > + mutex_unlock(&ptp->n_vclocks_mux);
> > + 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:
> > + return err;
> > +}
> > +static DEVICE_ATTR_RW(n_vclocks);
> > +
> > static struct attribute *ptp_attrs[] = {
> > &dev_attr_clock_name.attr,
> >
>
>
> > diff --git a/include/uapi/linux/ptp_clock.h
> > b/include/uapi/linux/ptp_clock.h index 1d108d597f66..4b933dc1b81b
> > 100644
> > --- a/include/uapi/linux/ptp_clock.h
> > +++ b/include/uapi/linux/ptp_clock.h
> > @@ -69,6 +69,11 @@
> > */
> > #define PTP_PEROUT_V1_VALID_FLAGS (0)
> >
> > +/*
> > + * Max number of PTP virtual clocks per PTP physical clock */
> > +#define PTP_MAX_VCLOCKS 20
>
> Why limit this to twenty clocks?
Please see my explain in another email thread. Thanks.
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion
2021-06-22 10:35 ` Y.b. Lu
@ 2021-06-25 11:09 ` Y.b. Lu
0 siblings, 0 replies; 31+ messages in thread
From: Y.b. Lu @ 2021-06-25 11:09 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-kselftest, mptcp, David S . Miller,
Jakub Kicinski, Mat Martineau, Matthieu Baerts, Shuah Khan,
Michal Kubecek, Florian Fainelli, Andrew Lunn, Rui Sousa,
Sebastien Laveze
Hi Richard,
> -----Original Message-----
> From: Y.b. Lu
> Sent: 2021年6月22日 18:35
> To: Richard Cochran <richardcochran@gmail.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; David S . Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mat Martineau
> <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>; Sebastien
> Laveze <sebastien.laveze@nxp.com>
> Subject: RE: [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks
> conversion
>
> Hi Richard,
>
> > -----Original Message-----
> > From: Richard Cochran <richardcochran@gmail.com>
> > Sent: 2021年6月18日 2:28
> > To: Y.b. Lu <yangbo.lu@nxp.com>
> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-kselftest@vger.kernel.org; mptcp@lists.linux.dev; David S .
> > Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mat
> > Martineau <mathew.j.martineau@linux.intel.com>; Matthieu Baerts
> > <matthieu.baerts@tessares.net>; Shuah Khan <shuah@kernel.org>; Michal
> > Kubecek <mkubecek@suse.cz>; Florian Fainelli <f.fainelli@gmail.com>;
> > Andrew Lunn <andrew@lunn.ch>; Rui Sousa <rui.sousa@nxp.com>;
> Sebastien
> > Laveze <sebastien.laveze@nxp.com>
> > Subject: Re: [net-next, v3, 02/10] ptp: support ptp physical/virtual
> > clocks conversion
> >
> > On Tue, Jun 15, 2021 at 05:45:09PM +0800, Yangbo Lu wrote:
> >
> > > diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> > > index 3f388d63904c..6949afc9d733 100644
> > > --- a/drivers/ptp/ptp_private.h
> > > +++ b/drivers/ptp/ptp_private.h
> > > @@ -46,6 +46,9 @@ struct ptp_clock {
> > > const struct attribute_group *pin_attr_groups[2];
> > > struct kthread_worker *kworker;
> > > struct kthread_delayed_work aux_work;
> > > + u8 n_vclocks;
> >
> > Hm, type is u8, but ...
> >
> > > + struct mutex n_vclocks_mux; /* protect concurrent n_vclocks access */
> > > + bool vclock_flag;
> > > };
> > >
> >
> > > #define info_to_vclock(d) container_of((d), struct ptp_vclock,
> > > info) diff --git a/include/uapi/linux/ptp_clock.h
> > > b/include/uapi/linux/ptp_clock.h index 1d108d597f66..4b933dc1b81b
> > > 100644
> > > --- a/include/uapi/linux/ptp_clock.h
> > > +++ b/include/uapi/linux/ptp_clock.h
> > > @@ -69,6 +69,11 @@
> > > */
> > > #define PTP_PEROUT_V1_VALID_FLAGS (0)
> > >
> > > +/*
> > > + * Max number of PTP virtual clocks per PTP physical clock */
> > > +#define PTP_MAX_VCLOCKS 20
> >
> > Only 20 clocks are allowed? Why?
>
> Initially I think vclock can be used for ptp multiple domains synchronization.
> Since the PTP domainValue is u8, u8 vclock number is large enough.
> This is not a good idea to hard-code a PTP_MAX_VCLOCKS value. But it looks a
> little crazy to create numbers of vclocks via one command (echo n >
> /sys/class/ptp/ptp0/n_vclocks).
> Maybe a typo creates hundreds of vclocks we don’t need.
> Do you think we should be care about setting a limitation of vclock number?
> Any suggestion for implementation?
> Thanks.
I sent v4. I removed the u8 limitation for vclocks number, using unsigned int instead.
I introduced max_vclocks attribute which could be re-configured.
Please help to review.
Thanks.
>
> >
> > Thanks,
> > Richard
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2021-06-25 11:09 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 9:45 [net-next, v3, 00/10] ptp: support virtual clocks and timestamping Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 01/10] ptp: add ptp virtual clock driver framework Yangbo Lu
2021-06-17 17:32 ` Richard Cochran
2021-06-22 10:17 ` Y.b. Lu
2021-06-15 9:45 ` [net-next, v3, 02/10] ptp: support ptp physical/virtual clocks conversion Yangbo Lu
2021-06-17 17:42 ` Richard Cochran
2021-06-22 10:18 ` Y.b. Lu
2021-06-17 18:27 ` Richard Cochran
2021-06-22 10:35 ` Y.b. Lu
2021-06-25 11:09 ` Y.b. Lu
2021-06-19 4:06 ` Richard Cochran
2021-06-22 10:39 ` Y.b. Lu
2021-06-15 9:45 ` [net-next, v3, 03/10] ptp: track available ptp vclocks information Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 04/10] ptp: add kernel API ptp_get_vclocks_index() Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 05/10] ethtool: add a new command for getting PHC virtual clocks Yangbo Lu
2021-06-15 19:49 ` Jakub Kicinski
2021-06-22 10:10 ` Y.b. Lu
2021-06-15 23:24 ` Michal Kubecek
2021-06-22 10:10 ` Y.b. Lu
2021-06-15 9:45 ` [net-next, v3, 06/10] ptp: add kernel API ptp_convert_timestamp() Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding Yangbo Lu
2021-06-16 1:00 ` Mat Martineau
2021-06-16 1:00 ` Mat Martineau
2021-06-22 10:14 ` Y.b. Lu
2021-06-16 10:27 ` kernel test robot
2021-06-16 10:27 ` kernel test robot
2021-06-16 10:39 ` kernel test robot
2021-06-16 10:39 ` kernel test robot
2021-06-15 9:45 ` [net-next, v3, 08/10] net: socket: support hardware timestamp conversion to PHC bound Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 09/10] selftests/net: timestamping: support binding PHC Yangbo Lu
2021-06-15 9:45 ` [net-next, v3, 10/10] MAINTAINERS: add entry for PTP virtual clock driver Yangbo Lu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.