Re: [PATCH bpf-next v12 01/12] bpf: add networking timestamping support to bpf_get/setsockopt()

[Martin KaFai Lau <martin.lau@linux.dev>]

On 2/18/25 6:22 AM, Willem de Bruijn wrote:
> Jason Xing wrote:
>> The new SK_BPF_CB_FLAGS and new SK_BPF_CB_TX_TIMESTAMPING are
>> added to bpf_get/setsockopt. The later patches will implement the
>> BPF networking timestamping. The BPF program will use
>> bpf_setsockopt(SK_BPF_CB_FLAGS, SK_BPF_CB_TX_TIMESTAMPING) to
>> enable the BPF networking timestamping on a socket.
>>
>> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
>> ---
>>   include/net/sock.h             |  3 +++
>>   include/uapi/linux/bpf.h       |  8 ++++++++
>>   net/core/filter.c              | 23 +++++++++++++++++++++++
>>   tools/include/uapi/linux/bpf.h |  1 +
>>   4 files changed, 35 insertions(+)
>>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 8036b3b79cd8..7916982343c6 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -303,6 +303,7 @@ struct sk_filter;
>>     *	@sk_stamp: time stamp of last packet received
>>     *	@sk_stamp_seq: lock for accessing sk_stamp on 32 bit architectures only
>>     *	@sk_tsflags: SO_TIMESTAMPING flags
>> +  *	@sk_bpf_cb_flags: used in bpf_setsockopt()
>>     *	@sk_use_task_frag: allow sk_page_frag() to use current->task_frag.
>>     *			   Sockets that can be used under memory reclaim should
>>     *			   set this to false.
>> @@ -445,6 +446,8 @@ struct sock {
>>   	u32			sk_reserved_mem;
>>   	int			sk_forward_alloc;
>>   	u32			sk_tsflags;
>> +#define SK_BPF_CB_FLAG_TEST(SK, FLAG) ((SK)->sk_bpf_cb_flags & (FLAG))
>> +	u32			sk_bpf_cb_flags;
>>   	__cacheline_group_end(sock_write_rxtx);
> 
> So far only one bit is defined. Does this have to be a 32-bit field in
> every socket?

iirc, I think there were multiple callback (cb) flags/bits in the earlier 
revisions, but it had been simplified to one bit in the later revisions.

It's an internal implementation detail. We can reuse some free bits from another 
variable for now. Probably get a bit from sk_tsflags? SOCKCM_FLAG_TS_OPT_ID uses 
BIT(31). Maybe a new SK_TS_FLAG_BPF_TX that uses BIT(30)? I don't have a strong 
preference on the name.

When the BPF program calls bpf_setsockopt(SK_BPF_CB_FLAGS, 
SK_BPF_CB_TX_TIMESTAMPING), the kernel will set/test the BIT(30) of sk_tsflags.

We can wait until there are more socket-level cb flags in the future (e.g., more 
SK_BPF_CB_XXX will be needed) before adding a dedicated int field in the sock.