2021-06-25 09:26:02

by Yangbo Lu

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

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

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

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

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

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

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

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

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


base-commit: 19938bafa7ae8fc0a4a2c1c1430abb1a04668da1
--
2.25.1


2021-06-25 09:26:28

by Yangbo Lu

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

Add entry for PTP virtual clock driver.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index cc375fda89d0..8afd96d4d194 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14847,6 +14847,13 @@ F: drivers/net/phy/dp83640*
F: drivers/ptp/*
F: include/linux/ptp_cl*

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

2021-06-25 09:26:43

by Yangbo Lu

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

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

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

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

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

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

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

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

return -ENOPROTOOPT;
}

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

2021-06-25 09:26:56

by Yangbo Lu

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

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

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

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

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

2021-06-25 09:27:12

by Yangbo Lu

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

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

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

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

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Passed so_timestamping for sock_set_timestamping.
---
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 | 22 ++++++++---
5 files changed, 101 insertions(+), 12 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index ced2fc965ec7..49a7858a8a03 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;
@@ -2753,7 +2756,8 @@ void sock_def_readable(struct sock *sk);

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

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

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

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

+/**
+ * struct so_timestamping - SO_TIMESTAMPING parameter
+ *
+ * @flags: SO_TIMESTAMPING flags
+ * @bind_phc: Index of PTP virtual clock bound to sock. This is available
+ * if flag SOF_TIMESTAMPING_BIND_PHC is set.
+ */
+struct so_timestamping {
+ int flags;
+ int bind_phc;
+};
+
/**
* struct hwtstamp_config - %SIOCGHWTSTAMP and %SIOCSHWTSTAMP parameter
*
diff --git a/net/core/sock.c b/net/core/sock.c
index a2337b37eba6..c59b8a20dc00 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,47 @@ void sock_set_timestamp(struct sock *sk, int optname, bool valbool)
}
}

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

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

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

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

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

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

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

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

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

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

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

--
2.25.1

2021-06-25 09:27:27

by Yangbo Lu

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

Support binding PHC of PTP vclock for timestamping.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v3:
- Added this patch.
Changes for v4:
- Fixed checkpatch.
---
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..be8032632073 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

2021-06-25 14:21:35

by kernel test robot

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

Hi Yangbo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 19938bafa7ae8fc0a4a2c1c1430abb1a04668da1]

url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210625-172554
base: 19938bafa7ae8fc0a4a2c1c1430abb1a04668da1
config: x86_64-randconfig-r013-20210625 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9ca0171a9ffdef5fdb1511d197a3fd72490362de)
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
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/e6720a23e3833ed72016804e74875c63c8f2c414
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/20210625-172554
git checkout e6720a23e3833ed72016804e74875c63c8f2c414
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

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

All warnings (new ones prefixed by >>):

net/mptcp/sockopt.c:218:45: error: indirection requires pointer operand ('int' invalid)
if (copy_from_sockptr(val, optval, sizeof(*val)))
^~~~
>> net/mptcp/sockopt.c:218:25: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'void *' [-Wint-conversion]
if (copy_from_sockptr(val, optval, sizeof(*val)))
^~~
include/linux/sockptr.h:53:43: note: passing argument to parameter 'dst' here
static inline int copy_from_sockptr(void *dst, sockptr_t src, size_t size)
^
1 warning and 1 error generated.


vim +218 net/mptcp/sockopt.c

202
203 static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
204 int optname,
205 sockptr_t optval,
206 unsigned int optlen)
207 {
208 struct mptcp_subflow_context *subflow;
209 struct sock *sk = (struct sock *)msk;
210 struct so_timestamping timestamping;
211 int val, ret;
212
213 if (optlen == sizeof(timestamping)) {
214 if (copy_from_sockptr(&timestamping, optval,
215 sizeof(timestamping)))
216 return -EFAULT;
217 } else if (optlen == sizeof(int)) {
> 218 if (copy_from_sockptr(val, optval, sizeof(*val)))
219 return -EFAULT;
220
221 memset(&timestamping, 0, sizeof(timestamping));
222 timestamping.flags = val;
223 } else {
224 return -EINVAL;
225 }
226
227 ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
228 KERNEL_SOCKPTR(&timestamping),
229 sizeof(timestamping));
230 if (ret)
231 return ret;
232
233 lock_sock(sk);
234
235 mptcp_for_each_subflow(msk, subflow) {
236 struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
237 bool slow = lock_sock_fast(ssk);
238
239 sock_set_timestamping(sk, optname, timestamping);
240 unlock_sock_fast(ssk, slow);
241 }
242
243 release_sock(sk);
244
245 return 0;
246 }
247

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.66 kB)
.config.gz (43.21 kB)
Download all attachments

2021-06-26 00:28:14

by kernel test robot

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 19938bafa7ae8fc0a4a2c1c1430abb1a04668da1]

url: https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-and-timestamping/20210625-172554
base: 19938bafa7ae8fc0a4a2c1c1430abb1a04668da1
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/57d8340e44c72b7f0a11cfbcd6f9f4ce81b40c18
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/20210625-172554
git checkout 57d8340e44c72b7f0a11cfbcd6f9f4ce81b40c18
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=x86_64

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

All errors (new ones prefixed by >>):

>> timestamping.c:48:8: error: redefinition of 'struct so_timestamping'
48 | struct so_timestamping {
| ^~~~~~~~~~~~~~~
In file included from timestamping.c:33:
usr/include/linux/net_tstamp.h:57:8: note: originally defined here
57 | struct so_timestamping {
| ^~~~~~~~~~~~~~~

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (1.55 kB)
.config.gz (40.98 kB)
Download all attachments

2021-06-26 01:02:02

by Mat Martineau

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

On Fri, 25 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 <[email protected]>
> ---
> Changes for v3:
> - Added this patch.
> Changes for v4:
> - Passed so_timestamping for sock_set_timestamping.
> ---
> 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 | 22 ++++++++---
> 5 files changed, 101 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index ced2fc965ec7..49a7858a8a03 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;
> @@ -2753,7 +2756,8 @@ void sock_def_readable(struct sock *sk);
>
> int sock_bindtoindex(struct sock *sk, int ifindex, bool lock_sk);
> void sock_set_timestamp(struct sock *sk, int optname, bool valbool);
> -int sock_set_timestamping(struct sock *sk, int optname, int val);
> +int sock_set_timestamping(struct sock *sk, int optname,
> + struct so_timestamping timestamping);
>
> void sock_enable_timestamps(struct sock *sk);
> void sock_no_linger(struct sock *sk);
> diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
> index 7ed0b3d1c00a..fcc61c73a666 100644
> --- a/include/uapi/linux/net_tstamp.h
> +++ b/include/uapi/linux/net_tstamp.h
> @@ -13,7 +13,7 @@
> #include <linux/types.h>
> #include <linux/socket.h> /* for SO_TIMESTAMPING */
>
> -/* SO_TIMESTAMPING gets an integer bit field comprised of these values */
> +/* SO_TIMESTAMPING flags */
> enum {
> SOF_TIMESTAMPING_TX_HARDWARE = (1<<0),
> SOF_TIMESTAMPING_TX_SOFTWARE = (1<<1),
> @@ -30,8 +30,9 @@ enum {
> SOF_TIMESTAMPING_OPT_STATS = (1<<12),
> SOF_TIMESTAMPING_OPT_PKTINFO = (1<<13),
> SOF_TIMESTAMPING_OPT_TX_SWHW = (1<<14),
> + SOF_TIMESTAMPING_BIND_PHC = (1 << 15),
>
> - SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_TX_SWHW,
> + SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_BIND_PHC,
> SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
> SOF_TIMESTAMPING_LAST
> };
> @@ -46,6 +47,18 @@ enum {
> SOF_TIMESTAMPING_TX_SCHED | \
> SOF_TIMESTAMPING_TX_ACK)
>
> +/**
> + * struct so_timestamping - SO_TIMESTAMPING parameter
> + *
> + * @flags: SO_TIMESTAMPING flags
> + * @bind_phc: Index of PTP virtual clock bound to sock. This is available
> + * if flag SOF_TIMESTAMPING_BIND_PHC is set.
> + */
> +struct so_timestamping {
> + int flags;
> + int bind_phc;
> +};
> +
> /**
> * struct hwtstamp_config - %SIOCGHWTSTAMP and %SIOCSHWTSTAMP parameter
> *
> diff --git a/net/core/sock.c b/net/core/sock.c
> index a2337b37eba6..c59b8a20dc00 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,47 @@ void sock_set_timestamp(struct sock *sk, int optname, bool valbool)
> }
> }
>
> -int sock_set_timestamping(struct sock *sk, int optname, int val)
> +static int sock_timestamping_bind_phc(struct sock *sk, int phc_index)
> {
> + struct net *net = sock_net(sk);
> + struct net_device *dev = NULL;
> + bool match = false;
> + int *vclock_index;
> + int i, num;
> +
> + if (sk->sk_bound_dev_if)
> + dev = dev_get_by_index(net, sk->sk_bound_dev_if);
> +
> + if (!dev) {
> + pr_err("%s: sock not bind to device\n", __func__);
> + return -EOPNOTSUPP;
> + }
> +
> + num = ethtool_get_phc_vclocks(dev, &vclock_index);
> + for (i = 0; i < num; i++) {
> + if (*(vclock_index + i) == phc_index) {
> + match = true;
> + break;
> + }
> + }
> +
> + if (num > 0)
> + kfree(vclock_index);
> +
> + if (!match)
> + return -EINVAL;
> +
> + sk->sk_bind_phc = phc_index;
> +
> + return 0;
> +}
> +
> +int sock_set_timestamping(struct sock *sk, int optname,
> + struct so_timestamping timestamping)
> +{
> + int val = timestamping.flags;
> + int ret;
> +
> if (val & ~SOF_TIMESTAMPING_MASK)
> return -EINVAL;
>
> @@ -816,6 +857,12 @@ int sock_set_timestamping(struct sock *sk, int optname, int val)
> !(val & SOF_TIMESTAMPING_OPT_TSONLY))
> return -EINVAL;
>
> + if (val & SOF_TIMESTAMPING_BIND_PHC) {
> + ret = sock_timestamping_bind_phc(sk, timestamping.bind_phc);
> + if (ret)
> + return ret;
> + }
> +
> sk->sk_tsflags = val;
> sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);
>
> @@ -891,6 +938,7 @@ EXPORT_SYMBOL(sock_set_mark);
> int sock_setsockopt(struct socket *sock, int level, int optname,
> sockptr_t optval, unsigned int optlen)
> {
> + struct so_timestamping timestamping;
> struct sock_txtime sk_txtime;
> struct sock *sk = sock->sk;
> int val;
> @@ -1057,7 +1105,15 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>
> case SO_TIMESTAMPING_NEW:
> case SO_TIMESTAMPING_OLD:
> - ret = sock_set_timestamping(sk, optname, val);
> + if (optlen == sizeof(timestamping)) {
> + if (copy_from_sockptr(&timestamping, optval,
> + sizeof(timestamping)))
> + return -EFAULT;
> + } else {
> + memset(&timestamping, 0, sizeof(timestamping));
> + timestamping.flags = val;
> + }
> + ret = sock_set_timestamping(sk, optname, timestamping);
> break;
>
> case SO_RCVLOWAT:
> @@ -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 798231b07676..c63e0739dc6a 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -398,6 +398,7 @@ const char sof_timestamping_names[][ETH_GSTRING_LEN] = {
> [const_ilog2(SOF_TIMESTAMPING_OPT_STATS)] = "option-stats",
> [const_ilog2(SOF_TIMESTAMPING_OPT_PKTINFO)] = "option-pktinfo",
> [const_ilog2(SOF_TIMESTAMPING_OPT_TX_SWHW)] = "option-tx-swhw",
> + [const_ilog2(SOF_TIMESTAMPING_BIND_PHC)] = "bind-phc",
> };
> static_assert(ARRAY_SIZE(sof_timestamping_names) == __SOF_TIMESTAMPING_CNT);
>
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index ea38cbcd2ad4..e20aefc20d75 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -207,14 +207,26 @@ static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
> {
> struct mptcp_subflow_context *subflow;
> struct sock *sk = (struct sock *)msk;
> + struct so_timestamping timestamping;
> int val, ret;
>
> - ret = mptcp_get_int_option(msk, optval, optlen, &val);
> - if (ret)
> - return ret;
> + if (optlen == sizeof(timestamping)) {
> + if (copy_from_sockptr(&timestamping, optval,
> + sizeof(timestamping)))
> + return -EFAULT;
> + } else if (optlen == sizeof(int)) {
> + if (copy_from_sockptr(val, optval, sizeof(*val)))
^^^

As the kbuild bot noted, this needs to be a pointer. You could pass in
&timestamping.flags and you wouldn't need the 'val' variable at all.


-Mat


> + return -EFAULT;
> +
> + memset(&timestamping, 0, sizeof(timestamping));
> + timestamping.flags = val;
> + } else {
> + return -EINVAL;
> + }
>
> ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
> - KERNEL_SOCKPTR(&val), sizeof(val));
> + KERNEL_SOCKPTR(&timestamping),
> + sizeof(timestamping));
> if (ret)
> return ret;
>
> @@ -224,7 +236,7 @@ static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
> struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> bool slow = lock_sock_fast(ssk);
>
> - sock_set_timestamping(sk, optname, val);
> + sock_set_timestamping(sk, optname, timestamping);
> unlock_sock_fast(ssk, slow);
> }
>
> --
> 2.25.1
>
>

--
Mat Martineau
Intel

2021-06-30 08:31:02

by Yangbo Lu

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

Hi Mat,

> -----Original Message-----
> From: Mat Martineau <[email protected]>
> Sent: 2021??6??26?? 9:00
> To: Y.b. Lu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Richard Cochran
> <[email protected]>; David S . Miller <[email protected]>;
> Jakub Kicinski <[email protected]>; Matthieu Baerts
> <[email protected]>; Shuah Khan <[email protected]>; Michal
> Kubecek <[email protected]>; Florian Fainelli <[email protected]>;
> Andrew Lunn <[email protected]>; Rui Sousa <[email protected]>; Sebastien
> Laveze <[email protected]>
> Subject: Re: [net-next, v4, 08/11] net: sock: extend SO_TIMESTAMPING for
> PHC binding
>
> On Fri, 25 Jun 2021, Yangbo Lu wrote:
>
[...]
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index ea38cbcd2ad4..e20aefc20d75 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -207,14 +207,26 @@ static int
> mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
> > {
> > struct mptcp_subflow_context *subflow;
> > struct sock *sk = (struct sock *)msk;
> > + struct so_timestamping timestamping;
> > int val, ret;
> >
> > - ret = mptcp_get_int_option(msk, optval, optlen, &val);
> > - if (ret)
> > - return ret;
> > + if (optlen == sizeof(timestamping)) {
> > + if (copy_from_sockptr(&timestamping, optval,
> > + sizeof(timestamping)))
> > + return -EFAULT;
> > + } else if (optlen == sizeof(int)) {
> > + if (copy_from_sockptr(val, optval, sizeof(*val)))
> ^^^
>
> As the kbuild bot noted, this needs to be a pointer. You could pass in
> &timestamping.flags and you wouldn't need the 'val' variable at all.
>
>

Ok, I sent v5 to fix.
Sorry for the trouble. It seemed my test config missed to enable MPTCP.

Thanks.

> -Mat
>
>
> > + return -EFAULT;
> > +
> > + memset(&timestamping, 0, sizeof(timestamping));
> > + timestamping.flags = val;
> > + } else {
> > + return -EINVAL;
> > + }
> >
> > ret = sock_setsockopt(sk->sk_socket, SOL_SOCKET, optname,
> > - KERNEL_SOCKPTR(&val), sizeof(val));
> > + KERNEL_SOCKPTR(&timestamping),
> > + sizeof(timestamping));
> > if (ret)
> > return ret;
> >
> > @@ -224,7 +236,7 @@ static int
> mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
> > struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> > bool slow = lock_sock_fast(ssk);
> >
> > - sock_set_timestamping(sk, optname, val);
> > + sock_set_timestamping(sk, optname, timestamping);
> > unlock_sock_fast(ssk, slow);
> > }
> >
> > --
> > 2.25.1
> >
> >
>
> --
> Mat Martineau
> Intel