lkml.org 
[lkml]   [2021]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC binding
Date
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
\
 
 \ /
  Last update: 2021-06-22 12:15    [W:0.289 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site