On Thu, Feb 6, 2025 at 5:57 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 2/4/25 5:57 PM, Jakub Kicinski wrote:
> > On Wed, 5 Feb 2025 02:30:22 +0800 Jason Xing wrote:
> >> + if (cgroup_bpf_enabled(CGROUP_SOCK_OPS) &&
> >> + SK_BPF_CB_FLAG_TEST(sk, SK_BPF_CB_TX_TIMESTAMPING) && skb) {
> >> + struct skb_shared_info *shinfo = skb_shinfo(skb);
> >> + struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);
> >> +
> >> + tcb->txstamp_ack_bpf = 1;
> >> + shinfo->tx_flags |= SKBTX_BPF;
> >> + shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
> >> + }
> >
> > If BPF program is attached we'll timestamp all skbs? Am I reading this
> > right?
>
> If the attached bpf program explicitly turns on the SK_BPF_CB_TX_TIMESTAMPING
> bit of a sock, then all skbs of this sock will be tx timestamp-ed.
Martin, I'm afraid it's not like what you expect. Only the last
portion of the sendmsg will enter the above function which means if
the size of sendmsg is large, only the last skb will be set SKBTX_BPF
and be timestamped.
>
> >
> > Wouldn't it be better to let BPF_SOCK_OPS_TS_SND_CB return whether it's
> > interested in tracing current packet all the way thru the stack?
>
> I like this idea. It can give the BPF prog a chance to do skb sampling on a
> particular socket.
>
> The return value of BPF_SOCK_OPS_TS_SND_CB (or any cgroup BPF prog return value)
> already has another usage, which its return value is currently enforced by the
> verifier. It is better not to convolute it further.
>
> I don't prefer to add more use cases to skops->reply either, which is an union
> of args[4], such that later progs (in the cgrp prog array) may lose the args value.
>
> Jason, instead of always setting SKBTX_BPF and txstamp_ack_bpf in the kernel, a
> new BPF kfunc can be added so that the BPF prog can call it to selectively set
> SKBTX_BPF and txstamp_ack_bpf in some skb.
Agreed because at netdev 0x19 I have an explicit plan to share the
experience from our company about how to trace all the skbs which were
completed through a kernel module. It's how we use in production
especially for debug or diagnose use.
I'm not knowledgeable enough about BPF, so I'd like to know if there
are some functions that I can take as good examples?
I think it's a standalone and good feature, can I handle it after this series?
Thanks,
Jason
