netdev - Re: [PATCH net-next 0/9] net-timestamp: bpf extension to equip applications transparently

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] [day] [month] [year] [list]

Message-ID: <CAL+tcoAPH=KG9eTYY2KeQh7udeBUyEazG_miTbiwe0Ph0-ssdw@mail.gmail.com>
Date: Wed, 9 Oct 2024 23:20:36 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, dsahern@...nel.org, 
	willemb@...gle.com, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, 
	martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org, 
	sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, bpf@...r.kernel.org, 
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next 0/9] net-timestamp: bpf extension to equip
 applications transparently

> It's more like this:
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index c58ca8dd561b..93f931dcc4cc 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -234,6 +234,14 @@ struct sock_common {
>   struct bpf_local_storage;
>   struct sk_filter;
>
> +enum {
> +       SOCKETOPT_TS_REQUESTOR = 0,
> +       CMSG_TS_REQUESTOR,
> +       BPFPROG_TS_REQUESTOR,
> +
> +       __MAX_TS_REQUESTOR,
> +};
> +
>   /**
>     *    struct sock - network layer representation of sockets
>     *    @__sk_common: shared layout with inet_timewait_sock
> @@ -444,7 +452,7 @@ struct sock {
>          socket_lock_t           sk_lock;
>          u32                     sk_reserved_mem;
>          int                     sk_forward_alloc;
> -       u32                     sk_tsflags;
> +       u32                     sk_tsflags[__MAX_TS_REQUESTOR];
>          __cacheline_group_end(sock_write_rxtx);
>
>          __cacheline_group_begin(sock_write_tx);
>
>
> And use existing SOF_TIMESTAMPING_* for each element in the array. Not
> sure that struct sock is the best place though, as some timestamping
> requests may be on per-packet basis for protocols other than TCP.
>
> Again, I'm just thinking out loud, kinda wild idea.

Thanks. I see.

Requestor or requester? I don't know.

For now, __MAX_TS_REQUESTOR can be two, one is used for the old
implementation, the other one is used for BPF extension.

One irrelevant question is if we need CMSG_TS_REQUESTOR to split the
old tsflags into two because the cmsg relies on sk->sk_tsflags which
works well.

The whole idea is very interesting and inspiring to me! It could be a
good way to go. But as you said, the memory can be a blocker. And
where exactly we should add in struct sock is another problem because
the size of this array could be different if we add more requestors in
the future.

I think I can write in the next version based on this idea
(sk_tsflags[MAX] array has two requestor members at the current stage)
and then seek for other experts' opinions.

+Willem, I'd like to know if you are for or against this idea?

Thanks,
Jason