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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoD9qZvbo53QsUcC27Dp=tJshBFdjoM9RCHxHEsYjwaXWg@mail.gmail.com>
Date: Fri, 7 Feb 2025 20:07:40 +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:18 AM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> 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.
>
> I see. I'm grateful that you point out the right path. I'm still
> investigating to find a good existing example in selftests and how to
> support kfunc.

Martin, sorry to revive this thread.

It's a little bit hard for me to find a proper example to follow. I
tried to call __bpf_kfunc in the BPF_SOCK_OPS_TS_SND_CB callback and
then failed because kfunc is not supported in the sock_ops case.
Later, I tried to kprobe to hook a function, say,
tcp_tx_timestamp_bpf(), passed the skb parameter to the kfunc and then
got an error.

Here is code snippet:
1) net/ipv4/tcp.c
+__bpf_kfunc static void tcp_init_tx_timestamp(struct sk_buff *skb)
+{
+       struct skb_shared_info *shinfo = skb_shinfo(skb);
+       struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);
+
+       printk(KERN_ERR "jason: %d, %d\n\n", tcb->txstamp_ack,
shinfo->tx_flags);
+       /*
+       tcb->txstamp_ack = 2;
+       shinfo->tx_flags |= SKBTX_BPF;
+       shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
+       */
+}
Note: I skipped copying some codes like BTF_ID_FLAGS...

2) bpf prog
SEC("kprobe/tcp_tx_timestamp_bpf") // I wrote a new function/wrapper to hook
int BPF_KPROBE(kprobe__tcp_tx_timestamp_bpf, struct sock *sk, struct
sk_buff *skb)
{
        tcp_init_tx_timestamp(skb);
        return 0;
}

Then running the bpf prog, I got the following message:
; tcp_init_tx_timestamp(skb); @ so_timestamping.c:281
1: (85) call tcp_init_tx_timestamp#120682
arg#0 pointer type STRUCT sk_buff must point to scalar, or struct with scalar
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
-- END PROG LOAD LOG --
libbpf: prog 'kprobe__tcp_tx_timestamp_bpf': failed to load: -22
libbpf: failed to load object 'so_timestamping'
libbpf: failed to load BPF skeleton 'so_timestamping': -22
test_so_timestamping:FAIL:open and load skel unexpected error: -22

If I don't pass any parameter in the kfunc, it can work.

Should we support the sock_ops for __bpf_kfunc?

Please enlighten me more about this. Thanks in advance!

Thanks,
Jason