netdev - Re: [PATCH bpf-next v8 10/12] bpf: make TCP tx timestamp bpf extension work

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: <CAL+tcoCEw7ppu7OvgOhcb=oeJLi4ZwhVdCHuHVSkhP8gEcpVDg@mail.gmail.com>
Date: Fri, 7 Feb 2025 21:34:33 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, Jakub Kicinski <kuba@...nel.org>, 
	davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, 
	dsahern@...nel.org, willemb@...gle.com, ast@...nel.org, daniel@...earbox.net, 
	andrii@...nel.org, 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, horms@...nel.org, 
	bpf@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v8 10/12] bpf: make TCP tx timestamp bpf
 extension work

On Fri, Feb 7, 2025 at 10:07 AM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
> On 2/5/25 10:56 PM, Jason Xing wrote:
> >>> I have to rephrase a bit in case Martin visits here soon: I will
> >>> compare two approaches 1) reply value, 2) bpf kfunc and then see which
> >>> way is better.
> >>
> >> I have already explained in details why the 1) reply value from the bpf prog
> >> won't work. Please go back to that reply which has the context.
> >
> > Yes, of course I saw this, but I said I need to implement and dig more
> > into this on my own. One of my replies includes a little code snippet
> > regarding reply value approach. I didn't expect you to misunderstand
> > that I would choose reply value, so I rephrase it like above :)
>
> I did see the code snippet which is incomplete, so I have to guess. afaik, it is
> not going to work. I was hoping to save some time without detouring to the
> reply-value path in case my earlier message was missed. I will stay quiet and
> wait for v9 first then to avoid extending this long thread further.

FYI, the code I adjusted works, a little bit ugly though.

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ad4f056aff22..44b4f8655668 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -498,10 +498,13 @@ static void tcp_tx_timestamp(struct sock *sk,
struct sockcm_cookie *sockc)
                struct skb_shared_info *shinfo = skb_shinfo(skb);
                struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);

-               tcb->txstamp_ack = 2;
-               shinfo->tx_flags |= SKBTX_BPF;
                shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
-               bpf_skops_tx_timestamping(sk, skb, BPF_SOCK_OPS_TS_SND_CB);
+               if (bpf_skops_tx_timestamping(sk, skb,
BPF_SOCK_OPS_TS_SND_CB)) {
+                       tcb->txstamp_ack = 2;
+                       shinfo->tx_flags |= SKBTX_BPF;
+               } else {
+                       shinfo->tskey = 0;
+               }
        }
 }

I'm not sure if it meets your requirement? The reason why I resorted
to this method is because I failed to attempt to use kfunc and
struggled to read many btf codes :(

So please provide more hints so that I can start again. Thanks.

Thanks,
Jason