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: <5d8b523d-ca30-40d7-bc08-f7959de47e4b@linux.dev>
Date: Mon, 2 Sep 2024 10:12:01 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: Willem de Bruijn <willemb@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>,
 netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/2] selftests: txtimestamp: add SCM_TS_OPT_ID
 test

On 02/09/2024 02:29, Jason Xing wrote:
> Hello Vadim,
> 
> On Fri, Aug 30, 2024 at 4:55 AM Vadim Fedorenko <vadfed@...a.com> wrote:
>>
>> Extend txtimestamp udp test to run with fixed tskey using
>> SCM_TS_OPT_ID control message.
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>
>> ---
>>   tools/include/uapi/asm-generic/socket.h    |  2 +
>>   tools/testing/selftests/net/txtimestamp.c  | 48 +++++++++++++++++-----
>>   tools/testing/selftests/net/txtimestamp.sh |  1 +
>>   3 files changed, 41 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/include/uapi/asm-generic/socket.h b/tools/include/uapi/asm-generic/socket.h
>> index 54d9c8bf7c55..281df9139d2b 100644
>> --- a/tools/include/uapi/asm-generic/socket.h
>> +++ b/tools/include/uapi/asm-generic/socket.h
>> @@ -124,6 +124,8 @@
>>   #define SO_PASSPIDFD           76
>>   #define SO_PEERPIDFD           77
>>
>> +#define SCM_TS_OPT_ID          78
>> +
>>   #if !defined(__KERNEL__)
>>
>>   #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
>> diff --git a/tools/testing/selftests/net/txtimestamp.c b/tools/testing/selftests/net/txtimestamp.c
>> index ec60a16c9307..3a8f716e72ae 100644
>> --- a/tools/testing/selftests/net/txtimestamp.c
>> +++ b/tools/testing/selftests/net/txtimestamp.c
>> @@ -54,6 +54,10 @@
>>   #define USEC_PER_SEC   1000000L
>>   #define NSEC_PER_SEC   1000000000LL
>>
>> +#ifndef SCM_TS_OPT_ID
>> +# define SCM_TS_OPT_ID 78
>> +#endif
>> +
>>   /* command line parameters */
>>   static int cfg_proto = SOCK_STREAM;
>>   static int cfg_ipproto = IPPROTO_TCP;
>> @@ -77,6 +81,8 @@ static bool cfg_epollet;
>>   static bool cfg_do_listen;
>>   static uint16_t dest_port = 9000;
>>   static bool cfg_print_nsec;
>> +static uint32_t ts_opt_id;
>> +static bool cfg_use_cmsg_opt_id;
>>
>>   static struct sockaddr_in daddr;
>>   static struct sockaddr_in6 daddr6;
>> @@ -136,12 +142,13 @@ static void validate_key(int tskey, int tstype)
>>          /* compare key for each subsequent request
>>           * must only test for one type, the first one requested
>>           */
>> -       if (saved_tskey == -1)
>> +       if (saved_tskey == -1 || cfg_use_cmsg_opt_id)
>>                  saved_tskey_type = tstype;
>>          else if (saved_tskey_type != tstype)
>>                  return;
>>
>>          stepsize = cfg_proto == SOCK_STREAM ? cfg_payload_len : 1;
>> +       stepsize = cfg_use_cmsg_opt_id ? 0 : stepsize;
>>          if (tskey != saved_tskey + stepsize) {
>>                  fprintf(stderr, "ERROR: key %d, expected %d\n",
>>                                  tskey, saved_tskey + stepsize);
>> @@ -480,7 +487,7 @@ static void fill_header_udp(void *p, bool is_ipv4)
>>
>>   static void do_test(int family, unsigned int report_opt)
>>   {
>> -       char control[CMSG_SPACE(sizeof(uint32_t))];
>> +       char control[2 * CMSG_SPACE(sizeof(uint32_t))];
>>          struct sockaddr_ll laddr;
>>          unsigned int sock_opt;
>>          struct cmsghdr *cmsg;
>> @@ -620,18 +627,32 @@ static void do_test(int family, unsigned int report_opt)
>>                  msg.msg_iov = &iov;
>>                  msg.msg_iovlen = 1;
>>
>> -               if (cfg_use_cmsg) {
>> +               if (cfg_use_cmsg || cfg_use_cmsg_opt_id) {
>>                          memset(control, 0, sizeof(control));
>>
>>                          msg.msg_control = control;
>> -                       msg.msg_controllen = sizeof(control);
>> +                       msg.msg_controllen = cfg_use_cmsg * CMSG_SPACE(sizeof(uint32_t));
>> +                       msg.msg_controllen += cfg_use_cmsg_opt_id * CMSG_SPACE(sizeof(uint32_t));
>>
>> -                       cmsg = CMSG_FIRSTHDR(&msg);
>> -                       cmsg->cmsg_level = SOL_SOCKET;
>> -                       cmsg->cmsg_type = SO_TIMESTAMPING;
>> -                       cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
>> +                       cmsg = NULL;
> 
> nit: we don't need to initialize it with NULL since it will be init
> one way or another in the following code.

NULL initialization is needed here because I use cmsg as a flag to
choose between CMSG_FIRSTHDR or CMSG_NXTHDR.

>> +                       if (cfg_use_cmsg) {
>> +                               cmsg = CMSG_FIRSTHDR(&msg);
>> +                               cmsg->cmsg_level = SOL_SOCKET;
>> +                               cmsg->cmsg_type = SO_TIMESTAMPING;
>> +                               cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
>> +
>> +                               *((uint32_t *)CMSG_DATA(cmsg)) = report_opt;
>> +                       }
>> +                       if (cfg_use_cmsg_opt_id) {
>> +                               cmsg = cmsg ? CMSG_NXTHDR(&msg, cmsg) : CMSG_FIRSTHDR(&msg);

This line has the check.

>> +                               cmsg->cmsg_level = SOL_SOCKET;
>> +                               cmsg->cmsg_type = SCM_TS_OPT_ID;
>> +                               cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
>> +
>> +                               *((uint32_t *)CMSG_DATA(cmsg)) = ts_opt_id;
>> +                               saved_tskey = ts_opt_id;
>> +                       }
>>
>> -                       *((uint32_t *) CMSG_DATA(cmsg)) = report_opt;
>>                  }
>>
>>                  val = sendmsg(fd, &msg, 0);
>> @@ -681,6 +702,7 @@ static void __attribute__((noreturn)) usage(const char *filepath)
>>                          "  -L    listen on hostname and port\n"
>>                          "  -n:   set no-payload option\n"
>>                          "  -N:   print timestamps and durations in nsec (instead of usec)\n"
>> +                       "  -o N: use SCM_TS_OPT_ID control message to provide N as tskey\n"
>>                          "  -p N: connect to port N\n"
>>                          "  -P:   use PF_PACKET\n"
>>                          "  -r:   use raw\n"
>> @@ -701,7 +723,7 @@ static void parse_opt(int argc, char **argv)
>>          int c;
>>
>>          while ((c = getopt(argc, argv,
>> -                               "46bc:CeEFhIl:LnNp:PrRS:t:uv:V:x")) != -1) {
>> +                               "46bc:CeEFhIl:LnNo:p:PrRS:t:uv:V:x")) != -1) {
>>                  switch (c) {
>>                  case '4':
>>                          do_ipv6 = 0;
>> @@ -742,6 +764,10 @@ static void parse_opt(int argc, char **argv)
>>                  case 'N':
>>                          cfg_print_nsec = true;
>>                          break;
>> +               case 'o':
>> +                       ts_opt_id = strtoul(optarg, NULL, 10);
>> +                       cfg_use_cmsg_opt_id = true;
>> +                       break;
>>                  case 'p':
>>                          dest_port = strtoul(optarg, NULL, 10);
>>                          break;
>> @@ -799,6 +825,8 @@ static void parse_opt(int argc, char **argv)
>>                  error(1, 0, "cannot ask for pktinfo over pf_packet");
>>          if (cfg_busy_poll && cfg_use_epoll)
>>                  error(1, 0, "pass epoll or busy_poll, not both");
>> +       if (cfg_proto != SOCK_DGRAM && cfg_use_cmsg_opt_id)
>> +               error(1, 0, "control message TS_OPT_ID can only be used with udp socket");
>>
>>          if (optind != argc - 1)
>>                  error(1, 0, "missing required hostname argument");
>> diff --git a/tools/testing/selftests/net/txtimestamp.sh b/tools/testing/selftests/net/txtimestamp.sh
>> index 25baca4b148e..7894628a9355 100755
>> --- a/tools/testing/selftests/net/txtimestamp.sh
>> +++ b/tools/testing/selftests/net/txtimestamp.sh
>> @@ -39,6 +39,7 @@ run_test_tcpudpraw() {
>>
>>          run_test_v4v6 ${args}           # tcp
>>          run_test_v4v6 ${args} -u        # udp
>> +       run_test_v4v6 ${args} -u -o 5   # udp with fixed tskey
> 
> Could we also add another test with "-C" based on the above command?
> It can test when both ID related flags are set, which will be helpful
> in the future :)

yep, sure, I'll add it in the next iteration.

> Thanks,
> Jason
> 
>>          run_test_v4v6 ${args} -r        # raw
>>          run_test_v4v6 ${args} -R        # raw (IPPROTO_RAW)
>>          run_test_v4v6 ${args} -P        # pf_packet
>> --
>> 2.43.5
>>
>>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ