[6/7] socket: Add recvmsg timestamp test
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
The test check if SO_TIMESTAMP and SO_TIMESTAMPNS is correctly
generated. The recvmsg() is called with different ancilliary data
buffers, to check if the conversion for 64-bit time on 32-bit
legacy ABIs are handled correctly.
For such archicutures, depending of the remmaning ancially data
space the timestamp can not be appended and MSG_TRUNC is returned
instead.
Checked on x86_64-linux and on i686-linux-gnu on both 5.11 and
4.15 kernel.
---
sysdeps/unix/sysv/linux/Makefile | 3 +
sysdeps/unix/sysv/linux/tst-recvmsg-time64.c | 1 +
sysdeps/unix/sysv/linux/tst-recvmsg.c | 171 +++++++++++++++++++
3 files changed, 175 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/tst-recvmsg-time64.c
create mode 100644 sysdeps/unix/sysv/linux/tst-recvmsg.c
Comments
* Adhemerval Zanella via Libc-alpha:
> The test check if SO_TIMESTAMP and SO_TIMESTAMPNS is correctly
> generated. The recvmsg() is called with different ancilliary data
> buffers, to check if the conversion for 64-bit time on 32-bit
> legacy ABIs are handled correctly.
>
> For such archicutures, depending of the remmaning ancially data
“architectures”, “remaining”
> space the timestamp can not be appended and MSG_TRUNC is returned
> instead.
“cannot”
> +static void
> +do_recvmsg (int s, bool support_64_timestamp, void *cmsg, size_t slack,
> + size_t tsize, int exp_msg)
> +{
> + /* A timestamp is expected if 32-bit timestamp are used (support in every
> + configuration) or if underlying kernel support 64-bit timestamps.
“if [the] underlying kernel support[s]”
> + Otherwise recvmsg will need extra space do add the 64-bit timestamp. */
“space [t]o add”
> + /* If there is not timestamp in the ancilliary data, recvmsg should set
“no[] timestamp”
> + the flag inidcating it. */
“indicating”
> + /* Setup the ancillary data buffer with an extra page with PROT_NONE to
> + check the possible timestamp conversion on some systems. */
> + pagesize = sysconf (_SC_PAGESIZE);
> + if (pagesize == -1)
> + FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
> + void *msgbuf = xmmap (0, 2 * pagesize, PROT_NONE,
> + MAP_ANONYMOUS | MAP_PRIVATE, -1);
> + xmprotect (msgbuf, pagesize, PROT_READ | PROT_WRITE);
Isn't this support_next_to_fault_allocate?
> +
> + do_test_send (&srv_addr, 1);
> + do_recvmsg (srv, false, NULL, 0, 0, 0);
> +
> + /* If underlying kernel does not support */
> + bool support_64_timestamp = support_socket_time64_timestamp (srv);
> +
> + /* Enable the timestamp using struct timeval precision. */
> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
Please use setsockopt here, nt the x wrapper, so that the ABI switches
with time64 support.
> + do_test_send (&srv_addr, array_length (slack));
> + for (int s = 0; s < array_length (slack); s++)
> + do_recvmsg (srv, support_64_timestamp, msgbuf, slack[s],
> + sizeof (struct timeval), s);
> +
> + /* Now enable timestamp using a higher precision, it overwrites the previous
> + precision. */
> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1}, sizeof (int));
Likewise here.
Thanks,
Florian
On 05/07/2021 16:32, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> The test check if SO_TIMESTAMP and SO_TIMESTAMPNS is correctly
>> generated. The recvmsg() is called with different ancilliary data
>> buffers, to check if the conversion for 64-bit time on 32-bit
>> legacy ABIs are handled correctly.
>>
>> For such archicutures, depending of the remmaning ancially data
>
> “architectures”, “remaining”
>
>> space the timestamp can not be appended and MSG_TRUNC is returned
>> instead.
>
> “cannot”
>
>> +static void
>> +do_recvmsg (int s, bool support_64_timestamp, void *cmsg, size_t slack,
>> + size_t tsize, int exp_msg)
>> +{
>> + /* A timestamp is expected if 32-bit timestamp are used (support in every
>> + configuration) or if underlying kernel support 64-bit timestamps.
>
> “if [the] underlying kernel support[s]”
>
>> + Otherwise recvmsg will need extra space do add the 64-bit timestamp. */
>
> “space [t]o add”
>
>> + /* If there is not timestamp in the ancilliary data, recvmsg should set
>
> “no[] timestamp”
>
>> + the flag inidcating it. */
>
> “indicating”
Ack for the typos and semantic fixes.
>
>> + /* Setup the ancillary data buffer with an extra page with PROT_NONE to
>> + check the possible timestamp conversion on some systems. */
>> + pagesize = sysconf (_SC_PAGESIZE);
>> + if (pagesize == -1)
>> + FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
>> + void *msgbuf = xmmap (0, 2 * pagesize, PROT_NONE,
>> + MAP_ANONYMOUS | MAP_PRIVATE, -1);
>> + xmprotect (msgbuf, pagesize, PROT_READ | PROT_WRITE);
>
> Isn't this support_next_to_fault_allocate?
It is, I will adjust.
>
>> +
>> + do_test_send (&srv_addr, 1);
>> + do_recvmsg (srv, false, NULL, 0, 0, 0);
>> +
>> + /* If underlying kernel does not support */
>> + bool support_64_timestamp = support_socket_time64_timestamp (srv);
>> +
>> + /* Enable the timestamp using struct timeval precision. */
>> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
>
> Please use setsockopt here, nt the x wrapper, so that the ABI switches
> with time64 support.
Currently it does not matter because SO_TIMESTAMP will be handled by
the kernel headers.
>
>> + do_test_send (&srv_addr, array_length (slack));
>> + for (int s = 0; s < array_length (slack); s++)
>> + do_recvmsg (srv, support_64_timestamp, msgbuf, slack[s],
>> + sizeof (struct timeval), s);
>> +
>> + /* Now enable timestamp using a higher precision, it overwrites the previous
>> + precision. */
>> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1}, sizeof (int));
>
> Likewise here.
>
> Thanks,
> Florian
>
* Adhemerval Zanella:
>>> + do_test_send (&srv_addr, 1);
>>> + do_recvmsg (srv, false, NULL, 0, 0, 0);
>>> +
>>> + /* If underlying kernel does not support */
>>> + bool support_64_timestamp = support_socket_time64_timestamp (srv);
>>> +
>>> + /* Enable the timestamp using struct timeval precision. */
>>> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
>>
>> Please use setsockopt here, nt the x wrapper, so that the ABI switches
>> with time64 support.
>
> Currently it does not matter because SO_TIMESTAMP will be handled by
> the kernel headers.
With my __setsockopt64 patch, the time64 version of the test will use a
different symbol. This is what I meant.
Thanks,
Florian
On 05/07/2021 17:02, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>>>> + do_test_send (&srv_addr, 1);
>>>> + do_recvmsg (srv, false, NULL, 0, 0, 0);
>>>> +
>>>> + /* If underlying kernel does not support */
>>>> + bool support_64_timestamp = support_socket_time64_timestamp (srv);
>>>> +
>>>> + /* Enable the timestamp using struct timeval precision. */
>>>> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
>>>
>>> Please use setsockopt here, nt the x wrapper, so that the ABI switches
>>> with time64 support.
>>
>> Currently it does not matter because SO_TIMESTAMP will be handled by
>> the kernel headers.
>
> With my __setsockopt64 patch, the time64 version of the test will use a
> different symbol. This is what I meant.
Yeah, I am not really convinced we do need such newer symbols that do
not correlate to 64-bit time_t syscall since I think it is *highly*
unlikely kernel will 64-bit time_t facilities for multiplexed syscall
(and I would consider this a bug) without any guards or checking
if the program is using 32-bit time_t.
Arnd, which would be kernel take in this regard? Will fcntl/ioctl/etc
exposes constants that might use 64-bit types if userland uses 32-bit
time_t? For socket constants I see that kernel does take
case of define different SO_* values, but these are for facilities
that are present for old 32-bit syscalls as well.
This would also require to use kernel types anyway, since the default
glibc provided ones will be 32-bit; and this won't be user-visible
(and not requiring any libc intervention).
Florian, in any case *currently* this patch does not need to use
xsetsockopt. I can adapt if your set get merged first.
* Adhemerval Zanella:
> On 05/07/2021 17:02, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>>>> + do_test_send (&srv_addr, 1);
>>>>> + do_recvmsg (srv, false, NULL, 0, 0, 0);
>>>>> +
>>>>> + /* If underlying kernel does not support */
>>>>> + bool support_64_timestamp = support_socket_time64_timestamp (srv);
>>>>> +
>>>>> + /* Enable the timestamp using struct timeval precision. */
>>>>> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
>>>>
>>>> Please use setsockopt here, nt the x wrapper, so that the ABI switches
>>>> with time64 support.
>>>
>>> Currently it does not matter because SO_TIMESTAMP will be handled by
>>> the kernel headers.
>>
>> With my __setsockopt64 patch, the time64 version of the test will use a
>> different symbol. This is what I meant.
>
> Yeah, I am not really convinced we do need such newer symbols that do
> not correlate to 64-bit time_t syscall since I think it is *highly*
> unlikely kernel will 64-bit time_t facilities for multiplexed syscall
> (and I would consider this a bug) without any guards or checking
> if the program is using 32-bit time_t.
What about SO_VM_SOCKETS_CONNECT_TIMEOUT?
> Florian, in any case *currently* this patch does not need to use
> xsetsockopt. I can adapt if your set get merged first.
Fine, go with xsetsockopt then.
Thanks,
Florian
On 06/07/2021 06:29, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 05/07/2021 17:02, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>>>> + do_test_send (&srv_addr, 1);
>>>>>> + do_recvmsg (srv, false, NULL, 0, 0, 0);
>>>>>> +
>>>>>> + /* If underlying kernel does not support */
>>>>>> + bool support_64_timestamp = support_socket_time64_timestamp (srv);
>>>>>> +
>>>>>> + /* Enable the timestamp using struct timeval precision. */
>>>>>> + xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
>>>>>
>>>>> Please use setsockopt here, nt the x wrapper, so that the ABI switches
>>>>> with time64 support.
>>>>
>>>> Currently it does not matter because SO_TIMESTAMP will be handled by
>>>> the kernel headers.
>>>
>>> With my __setsockopt64 patch, the time64 version of the test will use a
>>> different symbol. This is what I meant.
>>
>> Yeah, I am not really convinced we do need such newer symbols that do
>> not correlate to 64-bit time_t syscall since I think it is *highly*
>> unlikely kernel will 64-bit time_t facilities for multiplexed syscall
>> (and I would consider this a bug) without any guards or checking
>> if the program is using 32-bit time_t.
>
> What about SO_VM_SOCKETS_CONNECT_TIMEOUT?
Should we paper over all drivers or kernel features that do not have
proper 64-bit support? If so it would add a quite complex multiplex
handling on some syscall (ioctl). I would expect we will have quite
a few drivers out there which miss proper 64-bit support.
I added support for SO_TIMESTAMP{NS} because it is a features that
uses libc defined datatype and we include the kernel headers directly
(so user has no opt-out). I consider most driver as opt-in, where
users usually have to explicit include linux headers and they should
be aware that if we want to use _TIME_BITS=64 they would need care to
use libc-defined datatypes.
But this is also error prone (and I would consider that drivers using
libc-define datatypes are as well) and I see your point. I would
prefer if we avoid extra 64-bit symbols, but if you think we would
improve support in the future we can add as a conservative way.
>
>> Florian, in any case *currently* this patch does not need to use
>> xsetsockopt. I can adapt if your set get merged first.
>
> Fine, go with xsetsockopt then.
>
> Thanks,
> Florian
>
* Adhemerval Zanella:
> But this is also error prone (and I would consider that drivers using
> libc-define datatypes are as well) and I see your point. I would
> prefer if we avoid extra 64-bit symbols, but if you think we would
> improve support in the future we can add as a conservative way.
getsockopt/setsockopt is special because there is real length
information. We don't have that for fcntl, and only sort-of for ioctl.
So the kernel at least has a chance of doing the right thing for
getsockopt/setsockopt. And we don't need the time32-vs-time64
information in glibc for the present code because it's all encoded in
the option name. So getsockopt/setsockopt is definitely a corner case.
For recvmsg, sendmsg, sendmmsg, I think we need the time64 alias so that
we can disable the rewriting if it turns out to be too disruptive.
Thanks,
Florian
@@ -237,6 +237,9 @@ sysdep_routines += cmsg_nxthdr
CFLAGS-recvmmsg.c = -fexceptions -fasynchronous-unwind-tables
CFLAGS-sendmmsg.c = -fexceptions -fasynchronous-unwind-tables
+tests += tst-recvmsg
+tests-time64 += tst-recvmsg-time64
+
tests-special += $(objpfx)tst-socket-consts.out
$(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
PYTHONPATH=../scripts \
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-recvmsg.c"
new file mode 100644
@@ -0,0 +1,171 @@
+/* Check recvmsg timestamp support.
+ Copyright (C) 2021 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <array_length.h>
+#include <arpa/inet.h>
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <support/xsocket.h>
+#include <sys/mman.h>
+
+/* Some extra space added for ancillary data, it might be used to convert
+ 32-bit timestamp to 64-bit for _TIME_BITS=64 on older kernels. */
+static const int slack[] = { 0, 4, 8, 16, 32, 64 };
+
+static size_t pagesize;
+
+static int
+do_test_send (const struct sockaddr_in *addr, int nmsgs)
+{
+ int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+ xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
+
+ for (int i = 0; i < nmsgs; i++)
+ xsendto (s, &i, sizeof (i), 0, (const struct sockaddr *) addr,
+ sizeof (*addr));
+
+ return 0;
+}
+
+static void
+do_recvmsg (int s, bool support_64_timestamp, void *cmsg, size_t slack,
+ size_t tsize, int exp_msg)
+{
+ /* A timestamp is expected if 32-bit timestamp are used (support in every
+ configuration) or if underlying kernel support 64-bit timestamps.
+ Otherwise recvmsg will need extra space do add the 64-bit timestamp. */
+ bool exp_timestamp;
+ if (sizeof (time_t) == 4 || support_64_timestamp)
+ exp_timestamp = true;
+ else
+ exp_timestamp = slack >= CMSG_SPACE (tsize);
+
+ int msg;
+ struct iovec iov =
+ {
+ .iov_base = &msg,
+ .iov_len = sizeof (msg)
+ };
+ size_t msgs = CMSG_SPACE (tsize) + slack;
+ struct msghdr msghdr =
+ {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_iov = &iov,
+ .msg_iovlen = 1,
+ .msg_controllen = msgs,
+ .msg_control = cmsg != NULL ? cmsg + pagesize - msgs : NULL,
+ };
+ TEST_COMPARE (recvmsg (s, &msghdr, 0), sizeof (int));
+ TEST_COMPARE (msg, exp_msg);
+
+ if (cmsg == NULL)
+ return;
+
+ int timestamp = false;
+ for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&msghdr);
+ cmsg != NULL;
+ cmsg = CMSG_NXTHDR (&msghdr, cmsg))
+ {
+ if (cmsg->cmsg_level != SOL_SOCKET)
+ continue;
+ if (cmsg->cmsg_type == SCM_TIMESTAMP
+ && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timeval)))
+ {
+ struct timeval tv;
+ memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
+ printf ("SCM_TIMESTAMP: {%jd, %jd}\n", (intmax_t)tv.tv_sec,
+ (intmax_t)tv.tv_usec);
+ timestamp = true;
+ }
+ else if (cmsg->cmsg_type == SCM_TIMESTAMPNS
+ && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timespec)))
+ {
+ struct timespec ts;
+ memcpy (&ts, CMSG_DATA (cmsg), sizeof (ts));
+ printf ("SCM_TIMESTAMPNS: {%jd, %jd}\n", (intmax_t)ts.tv_sec,
+ (intmax_t)ts.tv_nsec);
+ timestamp = true;
+ }
+ }
+
+ /* If there is not timestamp in the ancilliary data, recvmsg should set
+ the flag inidcating it. */
+ if (exp_timestamp && !timestamp)
+ TEST_VERIFY (msghdr.msg_flags & MSG_TRUNC);
+ else
+ TEST_COMPARE (exp_timestamp, timestamp);
+}
+
+static int
+do_test (void)
+{
+ int srv = xsocket (AF_INET, SOCK_DGRAM, 0);
+ struct sockaddr_in srv_addr =
+ {
+ .sin_family = AF_INET,
+ .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) },
+ };
+ xbind (srv, (struct sockaddr *) &srv_addr, sizeof (srv_addr));
+ {
+ socklen_t sa_len = sizeof (srv_addr);
+ xgetsockname (srv, (struct sockaddr *) &srv_addr, &sa_len);
+ TEST_VERIFY (sa_len == sizeof (srv_addr));
+ }
+
+ TEST_COMPARE (recvmsg (-1, NULL, 0), -1);
+ TEST_COMPARE (errno, EBADF);
+
+ /* Setup the ancillary data buffer with an extra page with PROT_NONE to
+ check the possible timestamp conversion on some systems. */
+ pagesize = sysconf (_SC_PAGESIZE);
+ if (pagesize == -1)
+ FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
+ void *msgbuf = xmmap (0, 2 * pagesize, PROT_NONE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1);
+ xmprotect (msgbuf, pagesize, PROT_READ | PROT_WRITE);
+
+ do_test_send (&srv_addr, 1);
+ do_recvmsg (srv, false, NULL, 0, 0, 0);
+
+ /* If underlying kernel does not support */
+ bool support_64_timestamp = support_socket_time64_timestamp (srv);
+
+ /* Enable the timestamp using struct timeval precision. */
+ xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1}, sizeof (int));
+ do_test_send (&srv_addr, array_length (slack));
+ for (int s = 0; s < array_length (slack); s++)
+ do_recvmsg (srv, support_64_timestamp, msgbuf, slack[s],
+ sizeof (struct timeval), s);
+
+ /* Now enable timestamp using a higher precision, it overwrites the previous
+ precision. */
+ xsetsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1}, sizeof (int));
+ do_test_send (&srv_addr, array_length (slack));
+ for (int s = 0; s < array_length (slack); s++)
+ do_recvmsg (srv, support_64_timestamp, msgbuf, slack[s],
+ sizeof (struct timespec), s);
+
+ return 0;
+}
+
+#include <support/test-driver.c>