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 <cegg...@arri.de>
Reviewed-by: Willem de Bruijn <willemdebruijn.ker...@gmail.com>
Reviewed-by: Deepa Dinamani <deepa.ker...@gmail.com>
---
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:
--
Christian Eggers
Embedded software developer
Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer:
HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer:
HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger;
Markus Zeiler
