Hi Mat, > -----Original Message----- > From: Mat Martineau <mathew.j.martineau@xxxxxxxxxxxxxxx> > Sent: 2021年6月16日 9:01 > To: Y.b. Lu <yangbo.lu@xxxxxxx> > Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > linux-kselftest@xxxxxxxxxxxxxxx; mptcp@xxxxxxxxxxxxxxx; Richard Cochran > <richardcochran@xxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; > Jakub Kicinski <kuba@xxxxxxxxxx>; Matthieu Baerts > <matthieu.baerts@xxxxxxxxxxxx>; Shuah Khan <shuah@xxxxxxxxxx>; Michal > Kubecek <mkubecek@xxxxxxx>; Florian Fainelli <f.fainelli@xxxxxxxxx>; > Andrew Lunn <andrew@xxxxxxx>; Rui Sousa <rui.sousa@xxxxxxx>; Sebastien > Laveze <sebastien.laveze@xxxxxxx>; Florian Westphal <fw@xxxxxxxxx> > 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@xxxxxxx> > > --- > > 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
