From patchwork Mon Oct 12 09:35:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Eggers X-Patchwork-Id: 1380766 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=arri.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4C8tq43DMgz9sT6 for ; Mon, 12 Oct 2020 20:36:32 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729405AbgJLJg3 (ORCPT ); Mon, 12 Oct 2020 05:36:29 -0400 Received: from mailout11.rmx.de ([94.199.88.76]:49377 "EHLO mailout11.rmx.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726104AbgJLJg3 (ORCPT ); Mon, 12 Oct 2020 05:36:29 -0400 Received: from kdin02.retarus.com (kdin02.dmz1.retloc [172.19.17.49]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mailout11.rmx.de (Postfix) with ESMTPS id 4C8tpx1mWpz3ynH; Mon, 12 Oct 2020 11:36:25 +0200 (CEST) Received: from mta.arri.de (unknown [217.111.95.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by kdin02.retarus.com (Postfix) with ESMTPS id 4C8tpQ3g2wz2TTM9; Mon, 12 Oct 2020 11:35:58 +0200 (CEST) Received: from N95HX1G2.wgnetz.xx (192.168.54.142) by mta.arri.de (192.168.100.104) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 12 Oct 2020 11:35:58 +0200 From: Christian Eggers To: "David S . Miller" , Jakub Kicinski , Deepa Dinamani , Willem de Bruijn CC: Christoph Hellwig , , , Christian Eggers , "Willem de Bruijn" Subject: [PATCH net v2 1/2] socket: fix option SO_TIMESTAMPING_NEW Date: Mon, 12 Oct 2020 11:35:41 +0200 Message-ID: <20201012093542.15504-1-ceggers@arri.de> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Originating-IP: [192.168.54.142] X-RMX-ID: 20201012-113558-4C8tpQ3g2wz2TTM9-0@kdin02 X-RMX-SOURCE: 217.111.95.66 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The comparison of optname with SO_TIMESTAMPING_NEW is wrong way around, so SOCK_TSTAMP_NEW will first be set and than reset again. Additionally move it out of the test for SOF_TIMESTAMPING_RX_SOFTWARE as this seems unrelated. This problem happens on 32 bit platforms were the libc has already switched to struct timespec64 (from SO_TIMExxx_OLD to SO_TIMExxx_NEW socket options). ptp4l complains with "missing timestamp on transmitted peer delay request" because the wrong format is received (and discarded). Fixes: 9718475e6908 ("socket: Add SO_TIMESTAMPING_NEW") Signed-off-by: Christian Eggers Reviewed-by: Willem de Bruijn Reviewed-by: Deepa Dinamani Acked-by: Willem de Bruijn Acked-by: Deepa Dinamani --- v2: ----- - integrated proposal from Willem de Bruijn - added Reviewed-by: from Willem and Deepa On Saturday, 10 October 2020, 02:23:10 CEST, Willem de Bruijn wrote: > This suggested fix still sets and clears the flag if calling > SO_TIMESTAMPING_NEW to disable timestamping. where is it cleared? > Instead, how about > > case SO_TIMESTAMPING_NEW: > - sock_set_flag(sk, SOCK_TSTAMP_NEW); > fallthrough; > case SO_TIMESTAMPING_OLD: > [..] > + sock_valbool_flag(sk, SOCK_TSTAMP_NEW, > + optname == SO_TIMESTAMPING_NEW); > + using you version looks clearer > if (val & SOF_TIMESTAMPING_OPT_ID && > I would like to keep this below the "ret = -FOO; break" statements. IMHO the setsockopt() call should either completely fail or succeed. net/core/sock.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/net/core/sock.c b/net/core/sock.c index 34a8d12e38d7..669cf9b8bb44 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -994,8 +994,6 @@ int sock_setsockopt(struct socket *sock, int level, int optname, __sock_set_timestamps(sk, valbool, true, true); break; case SO_TIMESTAMPING_NEW: - sock_set_flag(sk, SOCK_TSTAMP_NEW); - fallthrough; case SO_TIMESTAMPING_OLD: if (val & ~SOF_TIMESTAMPING_MASK) { ret = -EINVAL; @@ -1024,16 +1022,14 @@ int sock_setsockopt(struct socket *sock, int level, int optname, } sk->sk_tsflags = val; + sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW); + if (val & SOF_TIMESTAMPING_RX_SOFTWARE) sock_enable_timestamp(sk, SOCK_TIMESTAMPING_RX_SOFTWARE); - else { - if (optname == SO_TIMESTAMPING_NEW) - sock_reset_flag(sk, SOCK_TSTAMP_NEW); - + else sock_disable_timestamp(sk, (1UL << SOCK_TIMESTAMPING_RX_SOFTWARE)); - } break; case SO_RCVLOWAT: From patchwork Mon Oct 12 09:35:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Eggers X-Patchwork-Id: 1380767 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=arri.de Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4C8tqm04Ftz9sT6 for ; Mon, 12 Oct 2020 20:37:07 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729476AbgJLJhG (ORCPT ); Mon, 12 Oct 2020 05:37:06 -0400 Received: from mailout07.rmx.de ([94.199.90.95]:44842 "EHLO mailout07.rmx.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726104AbgJLJhF (ORCPT ); Mon, 12 Oct 2020 05:37:05 -0400 Received: from kdin02.retarus.com (kdin02.dmz1.retloc [172.19.17.49]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mailout07.rmx.de (Postfix) with ESMTPS id 4C8tqc4S9WzBwTl; Mon, 12 Oct 2020 11:37:00 +0200 (CEST) Received: from mta.arri.de (unknown [217.111.95.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by kdin02.retarus.com (Postfix) with ESMTPS id 4C8tpx6QPkz2TTJR; Mon, 12 Oct 2020 11:36:25 +0200 (CEST) Received: from N95HX1G2.wgnetz.xx (192.168.54.142) by mta.arri.de (192.168.100.104) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 12 Oct 2020 11:36:25 +0200 From: Christian Eggers To: "David S . Miller" , Jakub Kicinski , Deepa Dinamani , Willem de Bruijn CC: Christoph Hellwig , , , Christian Eggers Subject: [PATCH net v2 2/2] socket: don't clear SOCK_TSTAMP_NEW when SO_TIMESTAMPNS is disabled Date: Mon, 12 Oct 2020 11:35:42 +0200 Message-ID: <20201012093542.15504-2-ceggers@arri.de> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201012093542.15504-1-ceggers@arri.de> References: <20201012093542.15504-1-ceggers@arri.de> MIME-Version: 1.0 X-Originating-IP: [192.168.54.142] X-RMX-ID: 20201012-113625-4C8tpx6QPkz2TTJR-0@kdin02 X-RMX-SOURCE: 217.111.95.66 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org SOCK_TSTAMP_NEW (timespec64 instead of timespec) is also used for hardware time stamps (configured via SO_TIMESTAMPING_NEW). User space (ptp4l) first configures hardware time stamping via SO_TIMESTAMPING_NEW which sets SOCK_TSTAMP_NEW. In the next step, ptp4l disables SO_TIMESTAMPNS(_NEW) (software time stamps), but this must not switch hardware time stamps back to "32 bit mode". This problem happens on 32 bit platforms were the libc has already switched to struct timespec64 (from SO_TIMExxx_OLD to SO_TIMExxx_NEW socket options). ptp4l complains with "missing timestamp on transmitted peer delay request" because the wrong format is received (and discarded). Fixes: 887feae36aee ("socket: Add SO_TIMESTAMP[NS]_NEW") Fixes: 783da70e8396 ("net: add sock_enable_timestamps") Signed-off-by: Christian Eggers Acked-by: Willem de Bruijn Acked-by: Deepa Dinamani --- v2: ----- - Added ACKs from Willem and Deepa On Saturday, 10 October 2020, 05:38:56 CEST, Deepa Dinamani wrote: > On Fri, Oct 9, 2020 at 5:35 PM Willem de Bruijn wrote: > > As this commit message shows, with SO_TIMESTAMP(NS) and > > SO_TIMESTAMPING that can be independently turned on and off, disabling > > one can incorrectly switch modes while the other is still active. > > This will not help the case when a child process that inherits the fd > and then sets SO_TIMESTAMP*_OLD/NEW on it, while the parent uses the > other version. > One of the two processes might still be surprised. But, child and > parent actively using the same socket fd might be expecting trouble > already. Usually the decision between SO_TIMESTAMP*_OLD/NEW will be done at compile time (trough the system C library headers). For a typical "distribution" it should be quite unlikely that two programs will be compiled using different settings. net/core/sock.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/core/sock.c b/net/core/sock.c index 669cf9b8bb44..f4236c7512b5 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -757,7 +757,6 @@ static void __sock_set_timestamps(struct sock *sk, bool val, bool new, bool ns) } else { sock_reset_flag(sk, SOCK_RCVTSTAMP); sock_reset_flag(sk, SOCK_RCVTSTAMPNS); - sock_reset_flag(sk, SOCK_TSTAMP_NEW); } }