* [PATCH 0/2] add tx timestamp tests
@ 2025-06-30 16:09 Pavel Begunkov
2025-06-30 16:09 ` [PATCH 1/2] Sync io_uring.h with tx timestamp api Pavel Begunkov
2025-06-30 16:09 ` [PATCH 2/2] tests: timestamp example Pavel Begunkov
0 siblings, 2 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-06-30 16:09 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
Reworked into a liburing test selftest/.../txtimestamp.c
Pavel Begunkov (2):
Sync io_uring.h with tx timestamp api
tests: timestamp example
src/include/liburing/io_uring.h | 16 ++
test/Makefile | 1 +
test/timestamp.c | 376 ++++++++++++++++++++++++++++++++
3 files changed, 393 insertions(+)
create mode 100644 test/timestamp.c
--
2.49.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] Sync io_uring.h with tx timestamp api
2025-06-30 16:09 [PATCH 0/2] add tx timestamp tests Pavel Begunkov
@ 2025-06-30 16:09 ` Pavel Begunkov
2025-06-30 16:09 ` [PATCH 2/2] tests: timestamp example Pavel Begunkov
1 sibling, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-06-30 16:09 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
src/include/liburing/io_uring.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/include/liburing/io_uring.h b/src/include/liburing/io_uring.h
index 73d29976..94de038d 100644
--- a/src/include/liburing/io_uring.h
+++ b/src/include/liburing/io_uring.h
@@ -915,6 +915,22 @@ enum io_uring_socket_op {
SOCKET_URING_OP_SIOCOUTQ,
SOCKET_URING_OP_GETSOCKOPT,
SOCKET_URING_OP_SETSOCKOPT,
+ SOCKET_URING_OP_TX_TIMESTAMP,
+};
+
+/*
+ * SOCKET_URING_OP_TX_TIMESTAMP definitions
+ */
+
+#define IORING_TIMESTAMP_HW_SHIFT 16
+/* The cqe->flags bit from which the timestamp type is stored */
+#define IORING_TIMESTAMP_TYPE_SHIFT (IORING_TIMESTAMP_HW_SHIFT + 1)
+/* The cqe->flags flag signifying whether it's a hardware timestamp */
+#define IORING_CQE_F_TSTAMP_HW ((__u32)1 << IORING_TIMESTAMP_HW_SHIFT);
+
+struct io_timespec {
+ __u64 tv_sec;
+ __u64 tv_nsec;
};
/* Zero copy receive refill queue entry */
--
2.49.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] tests: timestamp example
2025-06-30 16:09 [PATCH 0/2] add tx timestamp tests Pavel Begunkov
2025-06-30 16:09 ` [PATCH 1/2] Sync io_uring.h with tx timestamp api Pavel Begunkov
@ 2025-06-30 16:09 ` Pavel Begunkov
2025-06-30 16:20 ` Jens Axboe
1 sibling, 1 reply; 9+ messages in thread
From: Pavel Begunkov @ 2025-06-30 16:09 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
test/Makefile | 1 +
test/timestamp.c | 376 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 377 insertions(+)
create mode 100644 test/timestamp.c
diff --git a/test/Makefile b/test/Makefile
index ee09f5a8..99b272d7 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -249,6 +249,7 @@ test_srcs := \
xattr.c \
zcrx.c \
vec-regbuf.c \
+ timestamp.c \
# EOL
# Please keep this list sorted alphabetically.
diff --git a/test/timestamp.c b/test/timestamp.c
new file mode 100644
index 00000000..47c82d9a
--- /dev/null
+++ b/test/timestamp.c
@@ -0,0 +1,376 @@
+#include <arpa/inet.h>
+#include <error.h>
+#include <errno.h>
+#include <inttypes.h>
+#include <linux/errqueue.h>
+#include <linux/ipv6.h>
+#include <linux/net_tstamp.h>
+#include <netinet/in.h>
+#include <netinet/ip.h>
+#include <netinet/udp.h>
+#include <netinet/tcp.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <time.h>
+#include <unistd.h>
+#include <assert.h>
+
+#include "liburing.h"
+#include "helpers.h"
+
+#ifndef SCM_TS_OPT_ID
+#define SCM_TS_OPT_ID 0
+#endif
+
+static const int cfg_payload_len = 10;
+static uint16_t dest_port = 9000;
+static uint32_t ts_opt_id = 81;
+static bool cfg_use_cmsg_opt_id = false;
+static char buffer[128];
+static const bool verbose = false;
+
+static struct sockaddr_in6 daddr6;
+
+static int saved_tskey = -1;
+static int saved_tskey_type = -1;
+
+struct ctx {
+ int family;
+ int proto;
+ int report_opt;
+ int num_pkts;
+};
+
+static int validate_key(int tskey, int tstype, struct ctx *ctx)
+{
+ int stepsize;
+
+ /* compare key for each subsequent request
+ * must only test for one type, the first one requested
+ */
+ if (saved_tskey == -1 || cfg_use_cmsg_opt_id)
+ saved_tskey_type = tstype;
+ else if (saved_tskey_type != tstype)
+ return 0;
+
+ stepsize = ctx->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);
+ return -EINVAL;
+ }
+
+ saved_tskey = tskey;
+ return 0;
+}
+
+static int test_prep_sock(int family, int proto, unsigned report_opt)
+{
+ int ipproto = proto == SOCK_STREAM ? IPPROTO_TCP : IPPROTO_UDP;
+ unsigned int sock_opt;
+ int fd, val = 1;
+
+ fd = socket(family, proto, ipproto);
+ if (fd < 0)
+ error(1, errno, "socket");
+
+ if (proto == SOCK_STREAM) {
+ if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
+ (char*) &val, sizeof(val)))
+ error(1, 0, "setsockopt no nagle");
+
+ if (connect(fd, (void *) &daddr6, sizeof(daddr6)))
+ error(1, errno, "connect ipv6");
+ }
+
+ sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+ SOF_TIMESTAMPING_OPT_CMSG |
+ SOF_TIMESTAMPING_OPT_ID;
+ sock_opt |= report_opt;
+ sock_opt |= SOF_TIMESTAMPING_OPT_TSONLY;
+
+ if (setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING,
+ (char *) &sock_opt, sizeof(sock_opt)))
+ error(1, 0, "setsockopt timestamping");
+
+ return fd;
+}
+
+#define MAX_PACKETS 32
+
+struct send_req {
+ struct msghdr msg;
+ struct iovec iov;
+ char control[CMSG_SPACE(sizeof(uint32_t))];
+};
+
+static void queue_ts_cmd(struct io_uring *ring, int fd)
+{
+ struct io_uring_sqe *sqe;
+
+ sqe = io_uring_get_sqe(ring);
+ io_uring_prep_rw(IORING_OP_URING_CMD, sqe, fd, NULL, 0, 0);
+ sqe->cmd_op = SOCKET_URING_OP_TX_TIMESTAMP;
+ sqe->user_data = 43;
+}
+
+static void queue_send(struct io_uring *ring, int fd, void *buf, struct send_req *r,
+ int proto)
+{
+ struct io_uring_sqe *sqe;
+
+ r->iov.iov_base = buf;
+ r->iov.iov_len = cfg_payload_len;
+
+ memset(&r->msg, 0, sizeof(r->msg));
+ r->msg.msg_iov = &r->iov;
+ r->msg.msg_iovlen = 1;
+ if (proto == SOCK_STREAM) {
+ r->msg.msg_name = (void *)&daddr6;
+ r->msg.msg_namelen = sizeof(daddr6);
+ }
+
+ if (cfg_use_cmsg_opt_id) {
+ struct cmsghdr *cmsg;
+
+ memset(r->control, 0, sizeof(r->control));
+ r->msg.msg_control = r->control;
+ r->msg.msg_controllen = CMSG_SPACE(sizeof(uint32_t));
+
+ cmsg = CMSG_FIRSTHDR(&r->msg);
+ 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;
+ }
+
+ sqe = io_uring_get_sqe(ring);
+ io_uring_prep_sendmsg(sqe, fd, &r->msg, 0);
+ sqe->user_data = 0;
+}
+
+static const char *get_tstype_name(int tstype)
+{
+ if (tstype == SCM_TSTAMP_SCHED)
+ return "ENQ";
+ if (tstype == SCM_TSTAMP_SND)
+ return "SND";
+ if (tstype == SCM_TSTAMP_ACK)
+ return "ACK";
+ return "unknown";
+}
+
+static int do_test(struct ctx *ctx)
+{
+ struct send_req reqs[MAX_PACKETS];
+ struct io_uring_cqe *cqe;
+ struct io_uring ring;
+ unsigned long head;
+ int cqes_seen = 0;
+ int i, fd, ret;
+ int ts_expected = 0, ts_got = 0;
+
+ ts_expected += !!(ctx->report_opt & SOF_TIMESTAMPING_TX_SCHED);
+ ts_expected += !!(ctx->report_opt & SOF_TIMESTAMPING_TX_SOFTWARE);
+ ts_expected += !!(ctx->report_opt & SOF_TIMESTAMPING_TX_ACK);
+
+ ret = t_create_ring(32, &ring, IORING_SETUP_CQE32);
+ if (ret == T_SETUP_SKIP)
+ return T_EXIT_SKIP;
+ else if (ret)
+ t_error(1, ret, "queue init\n");
+
+ assert(ctx->num_pkts <= MAX_PACKETS);
+
+ fd = test_prep_sock(ctx->family, ctx->proto, ctx->report_opt);
+ if (fd < 0)
+ t_error(1, fd, "can't create socket\n");
+
+ memset(buffer, 'a', cfg_payload_len);
+ saved_tskey = -1;
+
+ if (cfg_use_cmsg_opt_id)
+ saved_tskey = ts_opt_id;
+
+ for (i = 0; i < ctx->num_pkts; i++) {
+ queue_send(&ring, fd, buffer, &reqs[i], ctx->proto);
+ ret = io_uring_submit(&ring);
+ if (ret != 1)
+ t_error(1, ret, "submit failed");
+
+ ret = io_uring_wait_cqe(&ring, &cqe);
+ if (ret || cqe->res != cfg_payload_len) {
+ fprintf(stderr, "wait send cqe, %d %d, expected %d\n",
+ ret, cqe->res, cfg_payload_len);
+ return T_EXIT_FAIL;
+ }
+ io_uring_cqe_seen(&ring, cqe);
+ }
+
+ usleep(200000);
+
+ queue_ts_cmd(&ring, fd);
+ ret = io_uring_submit(&ring);
+ if (ret != 1)
+ t_error(1, ret, "submit failed");
+
+ ret = io_uring_wait_cqe(&ring, &cqe);
+ if (ret) {
+ fprintf(stderr, "wait_cqe failed %d\n", ret);
+ return T_EXIT_FAIL;
+ }
+
+ io_uring_for_each_cqe(&ring, head, cqe) {
+ struct io_timespec *ts;
+ int tskey, tstype;
+ bool hwts;
+
+ cqes_seen++;
+
+ if (!(cqe->flags & IORING_CQE_F_MORE)) {
+ if (cqe->res == -EINVAL || cqe->res == -EOPNOTSUPP)
+ return T_EXIT_SKIP;
+ if (cqe->res)
+ t_error(1, 0, "failed cqe %i", cqe->res);
+ break;
+ }
+
+ ts = (void *)(cqe + 1);
+ tstype = cqe->flags >> IORING_TIMESTAMP_TYPE_SHIFT;
+ tskey = cqe->res;
+ hwts = cqe->flags & IORING_CQE_F_TSTAMP_HW;
+
+ ts_got++;
+ if (verbose)
+ fprintf(stderr, "ts: key %x, type %i (%s), is hw %i, sec %lu, nsec %lu\n",
+ tskey, tstype, get_tstype_name(tstype), hwts,
+ (unsigned long)ts->tv_sec,
+ (unsigned long)ts->tv_nsec);
+
+ ret = validate_key(tskey, tstype, ctx);
+ if (ret)
+ return T_EXIT_FAIL;
+ }
+
+ if (ts_got != ts_expected) {
+ fprintf(stderr, "expected %i timestamps, got %i\n",
+ ts_expected, ts_got);
+ return -EINVAL;
+ }
+
+ close(fd);
+ io_uring_cq_advance(&ring, cqes_seen);
+ io_uring_queue_exit(&ring);
+ return T_EXIT_PASS;
+}
+
+static void resolve_hostname(const char *name, int port)
+{
+ memset(&daddr6, 0, sizeof(daddr6));
+ daddr6.sin6_family = AF_INET6;
+ daddr6.sin6_port = htons(port);
+ if (inet_pton(AF_INET6, name, &daddr6.sin6_addr) != 1)
+ t_error(1, 0, "ipv6 parse error: %s", name);
+}
+
+static void do_listen(int family, int type, void *addr, int alen)
+{
+ int fd;
+
+ fd = socket(family, type, 0);
+ if (fd == -1)
+ error(1, errno, "socket rx");
+
+ if (bind(fd, addr, alen))
+ error(1, errno, "bind rx");
+
+ if (type == SOCK_STREAM && listen(fd, 10))
+ error(1, errno, "listen rx");
+
+ /* leave fd open, will be closed on process exit.
+ * this enables connect() to succeed and avoids icmp replies
+ */
+}
+
+static int do_main(int family, int proto)
+{
+ struct ctx ctx;
+ int ret;
+
+ ctx.num_pkts = 1;
+ ctx.family = family;
+ ctx.proto = proto;
+
+ if (verbose)
+ fprintf(stderr, "test SND\n");
+ ctx.report_opt = SOF_TIMESTAMPING_TX_SOFTWARE;
+ ret = do_test(&ctx);
+ if (ret) {
+ if (ret == T_EXIT_SKIP)
+ fprintf(stderr, "no timestamp cmd, skip\n");
+ return ret;
+ }
+
+ if (verbose)
+ fprintf(stderr, "test ENQ\n");
+ ctx.report_opt = SOF_TIMESTAMPING_TX_SCHED;
+ ret = do_test(&ctx);
+ if (ret)
+ return T_EXIT_FAIL;
+
+ if (verbose)
+ fprintf(stderr, "test ENQ + SND\n");
+ ctx.report_opt = SOF_TIMESTAMPING_TX_SCHED | SOF_TIMESTAMPING_TX_SOFTWARE;
+ ret = do_test(&ctx);
+ if (ret)
+ return T_EXIT_FAIL;
+
+ if (proto == SOCK_STREAM) {
+ if (verbose)
+ fprintf(stderr, "test ACK\n");
+ ctx.report_opt = SOF_TIMESTAMPING_TX_ACK;
+ ret = do_test(&ctx);
+ if (ret)
+ return T_EXIT_FAIL;
+
+ if (verbose)
+ fprintf(stderr, "test SND + ACK\n");
+ ctx.report_opt = SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_TX_ACK;
+ ret = do_test(&ctx);
+ if (ret)
+ return T_EXIT_FAIL;
+
+ if (verbose)
+ fprintf(stderr, "test ENQ + SND + ACK\n");
+ ctx.report_opt = SOF_TIMESTAMPING_TX_SCHED |
+ SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_TX_ACK;
+ ret = do_test(&ctx);
+ if (ret)
+ return T_EXIT_FAIL;
+ }
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ const char *hostname;
+
+ if (SCM_TS_OPT_ID == 0) {
+ fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
+ return T_EXIT_SKIP;
+ }
+
+ hostname = "::1";
+ resolve_hostname(hostname, dest_port);
+ do_listen(PF_INET6, SOCK_STREAM, &daddr6, sizeof(daddr6));
+ return do_main(PF_INET6, SOCK_STREAM);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests: timestamp example
2025-06-30 16:09 ` [PATCH 2/2] tests: timestamp example Pavel Begunkov
@ 2025-06-30 16:20 ` Jens Axboe
2025-06-30 16:45 ` Pavel Begunkov
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2025-06-30 16:20 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 6/30/25 10:09 AM, Pavel Begunkov wrote:
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
A bit of commit message might be nice? Ditto the other patch.
I know they are pretty straight forward, but doesn't hurt to
spell out a bit why the change is being made.
> +#ifndef SCM_TS_OPT_ID
> +#define SCM_TS_OPT_ID 0
> +#endif
This one had me a bit puzzled, particularly with:
> + if (SCM_TS_OPT_ID == 0) {
> + fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
> + return T_EXIT_SKIP;
> + }
as that'll just make the test skip on even my debian unstable/testing
base as it's still not defined there. But I guess it's because it's arch
specific? FWIW, looks like anything but sparc/parisc define it as 81,
hence in terms of coverage might be better to simply define it for
anything but those and actually have the test run?
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests: timestamp example
2025-06-30 16:20 ` Jens Axboe
@ 2025-06-30 16:45 ` Pavel Begunkov
2025-06-30 16:47 ` Jens Axboe
2025-06-30 16:50 ` Pavel Begunkov
0 siblings, 2 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-06-30 16:45 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 6/30/25 17:20, Jens Axboe wrote:
> On 6/30/25 10:09 AM, Pavel Begunkov wrote:
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>
> A bit of commit message might be nice? Ditto the other patch.
> I know they are pretty straight forward, but doesn't hurt to
> spell out a bit why the change is being made.
It's not like there is much to describe. The only bit
I can add is the reference to the selftest as per the CV
>> +#ifndef SCM_TS_OPT_ID
>> +#define SCM_TS_OPT_ID 0
>> +#endif
Otherwise it needs to be
#ifdef SCM_TS_OPT_ID
All tests using SCM_TS_OPT_ID
#else
int main() {
return skip;
}
#endif
which is even uglier
> This one had me a bit puzzled, particularly with:
>
>> + if (SCM_TS_OPT_ID == 0) {
>> + fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
>> + return T_EXIT_SKIP;
>> + }
>
> as that'll just make the test skip on even my debian unstable/testing
> base as it's still not defined there. But I guess it's because it's arch
> specific? FWIW, looks like anything but sparc/parisc define it as 81,
> hence in terms of coverage might be better to simply define it for
> anything but those and actually have the test run?
That only works until someone runs it on those arches and complain,
i.e. delaying the problem. And I honesty don't want to parse the
current architecture and figuring the value just for a test.
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests: timestamp example
2025-06-30 16:45 ` Pavel Begunkov
@ 2025-06-30 16:47 ` Jens Axboe
2025-06-30 16:50 ` Pavel Begunkov
1 sibling, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2025-06-30 16:47 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 6/30/25 10:45 AM, Pavel Begunkov wrote:
> On 6/30/25 17:20, Jens Axboe wrote:
>> On 6/30/25 10:09 AM, Pavel Begunkov wrote:
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>
>> A bit of commit message might be nice? Ditto the other patch.
>> I know they are pretty straight forward, but doesn't hurt to
>> spell out a bit why the change is being made.
>
> It's not like there is much to describe. The only bit
> I can add is the reference to the selftest as per the CV
Agree, but even a link to the cover letter for the feature would be
nice, then people can at least look it up rather than need to search for
what it is.
>>> +#ifndef SCM_TS_OPT_ID
>>> +#define SCM_TS_OPT_ID 0
>>> +#endif
>
> Otherwise it needs to be
>
> #ifdef SCM_TS_OPT_ID
>
> All tests using SCM_TS_OPT_ID
>
> #else
> int main() {
> return skip;
> }
> #endif
>
> which is even uglier
That's not what I meant, as per below. It was about defining it to
something valid, rather than gate the entire test on the define being
available.
>> This one had me a bit puzzled, particularly with:
>>
>>> + if (SCM_TS_OPT_ID == 0) {
>>> + fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
>>> + return T_EXIT_SKIP;
>>> + }
>>
>> as that'll just make the test skip on even my debian unstable/testing
>> base as it's still not defined there. But I guess it's because it's arch
>> specific? FWIW, looks like anything but sparc/parisc define it as 81,
>> hence in terms of coverage might be better to simply define it for
>> anything but those and actually have the test run?
>
> That only works until someone runs it on those arches and complain,
> i.e. delaying the problem. And I honesty don't want to parse the
> current architecture and figuring the value just for a test.
Since it's just those two obscure archs that nobody uses, I can just add
it after the fact. I'd rather deal with that than not have the test run
just because some arch relics use private defines. Not a big deal.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests: timestamp example
2025-06-30 16:45 ` Pavel Begunkov
2025-06-30 16:47 ` Jens Axboe
@ 2025-06-30 16:50 ` Pavel Begunkov
2025-06-30 16:54 ` Jens Axboe
2025-06-30 16:55 ` Pavel Begunkov
1 sibling, 2 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-06-30 16:50 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 6/30/25 17:45, Pavel Begunkov wrote:
> On 6/30/25 17:20, Jens Axboe wrote:
>> On 6/30/25 10:09 AM, Pavel Begunkov wrote:
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>
>> A bit of commit message might be nice? Ditto the other patch.
>> I know they are pretty straight forward, but doesn't hurt to
>> spell out a bit why the change is being made.
>
> It's not like there is much to describe. The only bit
> I can add is the reference to the selftest as per the CV
>
>>> +#ifndef SCM_TS_OPT_ID
>>> +#define SCM_TS_OPT_ID 0
>>> +#endif
>
> Otherwise it needs to be
>
> #ifdef SCM_TS_OPT_ID
>
> All tests using SCM_TS_OPT_ID
>
> #else
> int main() {
> return skip;
> }
> #endif
>
> which is even uglier
>
>> This one had me a bit puzzled, particularly with:
>>
>>> + if (SCM_TS_OPT_ID == 0) {
>>> + fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
>>> + return T_EXIT_SKIP;
>>> + }
>>
>> as that'll just make the test skip on even my debian unstable/testing
>> base as it's still not defined there. But I guess it's because it's arch
>> specific? FWIW, looks like anything but sparc/parisc define it as 81,
>> hence in terms of coverage might be better to simply define it for
>> anything but those and actually have the test run?
>
> That only works until someone runs it on those arches and complain,
> i.e. delaying the problem. And I honesty don't want to parse the
> current architecture and figuring the value just for a test.
#include <asm/socket.h>
Is that even legit?
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests: timestamp example
2025-06-30 16:50 ` Pavel Begunkov
@ 2025-06-30 16:54 ` Jens Axboe
2025-06-30 16:55 ` Pavel Begunkov
1 sibling, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2025-06-30 16:54 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 6/30/25 10:50 AM, Pavel Begunkov wrote:
> On 6/30/25 17:45, Pavel Begunkov wrote:
>> On 6/30/25 17:20, Jens Axboe wrote:
>>> On 6/30/25 10:09 AM, Pavel Begunkov wrote:
>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>
>>> A bit of commit message might be nice? Ditto the other patch.
>>> I know they are pretty straight forward, but doesn't hurt to
>>> spell out a bit why the change is being made.
>>
>> It's not like there is much to describe. The only bit
>> I can add is the reference to the selftest as per the CV
>>
>>>> +#ifndef SCM_TS_OPT_ID
>>>> +#define SCM_TS_OPT_ID 0
>>>> +#endif
>>
>> Otherwise it needs to be
>>
>> #ifdef SCM_TS_OPT_ID
>>
>> All tests using SCM_TS_OPT_ID
>>
>> #else
>> int main() {
>> return skip;
>> }
>> #endif
>>
>> which is even uglier
>>
>>> This one had me a bit puzzled, particularly with:
>>>
>>>> + if (SCM_TS_OPT_ID == 0) {
>>>> + fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
>>>> + return T_EXIT_SKIP;
>>>> + }
>>>
>>> as that'll just make the test skip on even my debian unstable/testing
>>> base as it's still not defined there. But I guess it's because it's arch
>>> specific? FWIW, looks like anything but sparc/parisc define it as 81,
>>> hence in terms of coverage might be better to simply define it for
>>> anything but those and actually have the test run?
>>
>> That only works until someone runs it on those arches and complain,
>> i.e. delaying the problem. And I honesty don't want to parse the
>> current architecture and figuring the value just for a test.
>
> #include <asm/socket.h>
>
> Is that even legit?
Good question... We can always try. Looking at debian, their newest
package for testing/unstable is 6.12.33 which is why asm doesn't even
have it. There is an 6.15 based package too, but still listed as
experimental.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests: timestamp example
2025-06-30 16:50 ` Pavel Begunkov
2025-06-30 16:54 ` Jens Axboe
@ 2025-06-30 16:55 ` Pavel Begunkov
1 sibling, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-06-30 16:55 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 6/30/25 17:50, Pavel Begunkov wrote:
> On 6/30/25 17:45, Pavel Begunkov wrote:
>> On 6/30/25 17:20, Jens Axboe wrote:
>>> On 6/30/25 10:09 AM, Pavel Begunkov wrote:
>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>
>>> A bit of commit message might be nice? Ditto the other patch.
>>> I know they are pretty straight forward, but doesn't hurt to
>>> spell out a bit why the change is being made.
>>
>> It's not like there is much to describe. The only bit
>> I can add is the reference to the selftest as per the CV
>>
>>>> +#ifndef SCM_TS_OPT_ID
>>>> +#define SCM_TS_OPT_ID 0
>>>> +#endif
>>
>> Otherwise it needs to be
>>
>> #ifdef SCM_TS_OPT_ID
>>
>> All tests using SCM_TS_OPT_ID
>>
>> #else
>> int main() {
>> return skip;
>> }
>> #endif
>>
>> which is even uglier
>>
>>> This one had me a bit puzzled, particularly with:
>>>
>>>> + if (SCM_TS_OPT_ID == 0) {
>>>> + fprintf(stderr, "no SCM_TS_OPT_ID, skip\n");
>>>> + return T_EXIT_SKIP;
>>>> + }
>>>
>>> as that'll just make the test skip on even my debian unstable/testing
>>> base as it's still not defined there. But I guess it's because it's arch
>>> specific? FWIW, looks like anything but sparc/parisc define it as 81,
>>> hence in terms of coverage might be better to simply define it for
>>> anything but those and actually have the test run?
>>
>> That only works until someone runs it on those arches and complain,
>> i.e. delaying the problem. And I honesty don't want to parse the
>> current architecture and figuring the value just for a test.
>
> #include <asm/socket.h>
>
> Is that even legit?
Except that it doesn't work, nevermind
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-06-30 16:54 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-30 16:09 [PATCH 0/2] add tx timestamp tests Pavel Begunkov
2025-06-30 16:09 ` [PATCH 1/2] Sync io_uring.h with tx timestamp api Pavel Begunkov
2025-06-30 16:09 ` [PATCH 2/2] tests: timestamp example Pavel Begunkov
2025-06-30 16:20 ` Jens Axboe
2025-06-30 16:45 ` Pavel Begunkov
2025-06-30 16:47 ` Jens Axboe
2025-06-30 16:50 ` Pavel Begunkov
2025-06-30 16:54 ` Jens Axboe
2025-06-30 16:55 ` Pavel Begunkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox