| 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
| ||
|
Message-ID: <CAL+tcoChfxje3wNxL2gpvro3y04DPY5YRpdUWBVYx7ixObakPw@mail.gmail.com>
Date: Mon, 2 Sep 2024 20:20:03 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
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 Mon, Sep 2, 2024 at 5:12 PM Vadim Fedorenko
<vadim.fedorenko@...ux.dev> wrote:
>
> 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.
Oh, I miss that.
>
> >> + 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!
Powered by blists - more mailing lists