netdev - RE: [net-next, v3, 07/10] net: sock: extend SO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB7PR04MB5017165F5C590671CCCA137EF8099@DB7PR04MB5017.eurprd04.prod.outlook.com>
Date:   Tue, 22 Jun 2021 10:14:33 +0000
From:   "Y.b. Lu" <yangbo.lu@....com>
To:     Mat Martineau <mathew.j.martineau@...ux.intel.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "mptcp@...ts.linux.dev" <mptcp@...ts.linux.dev>,
        Richard Cochran <richardcochran@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Matthieu Baerts <matthieu.baerts@...sares.net>,
        Shuah Khan <shuah@...nel.org>,
        Michal Kubecek <mkubecek@...e.cz>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>, Rui Sousa <rui.sousa@....com>,
        Sebastien Laveze <sebastien.laveze@....com>,
        Florian Westphal <fw@...len.de>
Subject: RE: [net-next, v3, 07/10] net: sock: extend SO_TIMESTAMPING for PHC
 binding

Hi Mat,

> -----Original Message-----
> From: Mat Martineau <mathew.j.martineau@...ux.intel.com>
> Sent: 2021年6月16日 9:01
> To: Y.b. Lu <yangbo.lu@....com>
> Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
> linux-kselftest@...r.kernel.org; mptcp@...ts.linux.dev; Richard Cochran
> <richardcochran@...il.com>; David S . Miller <davem@...emloft.net>;
> Jakub Kicinski <kuba@...nel.org>; Matthieu Baerts
> <matthieu.baerts@...sares.net>; Shuah Khan <shuah@...nel.org>; Michal
> Kubecek <mkubecek@...e.cz>; Florian Fainelli <f.fainelli@...il.com>;
> Andrew Lunn <andrew@...n.ch>; Rui Sousa <rui.sousa@....com>; Sebastien
> Laveze <sebastien.laveze@....com>; Florian Westphal <fw@...len.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@....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