On Fri, Feb 7, 2025 at 10:18 AM Jason Xing <kerneljasonxing@xxxxxxxxx> wrote:
>
> On Fri, Feb 7, 2025 at 10:07 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> 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
