Re: [PATCH bpf-next 1/2] bpf: add hardware timestamp field to __sk_buff

[Vadim Fedorenko <vfedorenko@novek.ru>]

On 03.09.2021 09:22, Daniel Borkmann wrote:
> On 9/3/21 12:15 AM, Vadim Fedorenko wrote:
>> BPF programs may want to know hardware timestamps if NIC supports
>> such timestamping.
>>
>> Expose this data as hwtstamp field of __sk_buff the same way as
>> gso_segs/gso_size.
>>
>> Also update BPF_PROG_TEST_RUN tests of the feature.
>>
>> Signed-off-by: Vadim Fedorenko <vfedorenko@novek.ru>
>> ---
>>   include/uapi/linux/bpf.h       |  2 ++
>>   net/core/filter.c              | 11 +++++++++++
>>   tools/include/uapi/linux/bpf.h |  2 ++
>>   3 files changed, 15 insertions(+)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 791f31dd0abe..c7d05b49f557 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -5284,6 +5284,8 @@ struct __sk_buff {
>>       __u32 gso_segs;
>>       __bpf_md_ptr(struct bpf_sock *, sk);
>>       __u32 gso_size;
>> +    __u32 padding;        /* Padding, future use. */
> 
> nit, instead of explicit padding field, just use: __u32 :32;
> 
> Also please add test_verifier coverage for this in BPF selftests, meaning,
> the expectation would be in case someone tries to access the padding field
> with this patch that we get a 'bpf verifier is misconfigured' error given
> it would have no bpf_convert_ctx_access() translation. But it would be overall
> better to add this to bpf_skb_is_valid_access(), so we can reject access to
> the padding area right there instead.

Thanks Daniel, I will update it in v2

>> +    __u64 hwtstamp;
>>   };
>>   struct bpf_tunnel_key {
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 2e32cee2c469..1d8f8494d325 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -8884,6 +8884,17 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type 
>> type,
>>                         si->dst_reg, si->src_reg,
>>                         offsetof(struct sk_buff, sk));
>>           break;
>> +    case offsetof(struct __sk_buff, hwtstamp):
>> +        BUILD_BUG_ON(sizeof_field(struct skb_shared_hwtstamps, hwtstamp) != 8);
>> +        BUILD_BUG_ON(offsetof(struct skb_shared_hwtstamps, hwtstamp) != 0);
>> +
>> +        insn = bpf_convert_shinfo_access(si, insn);
>> +        *insn++ = BPF_LDX_MEM(BPF_DW,
>> +                      si->dst_reg, si->dst_reg,
>> +                      bpf_target_off(struct skb_shared_info,
>> +                             hwtstamps, 8,
>> +                             target_size));
>> +        break;
>>       }
>>       return insn - insn_buf;
>> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>> index 791f31dd0abe..c7d05b49f557 100644
>> --- a/tools/include/uapi/linux/bpf.h
>> +++ b/tools/include/uapi/linux/bpf.h
>> @@ -5284,6 +5284,8 @@ struct __sk_buff {
>>       __u32 gso_segs;
>>       __bpf_md_ptr(struct bpf_sock *, sk);
>>       __u32 gso_size;
>> +    __u32 padding;        /* Padding, future use. */
>> +    __u64 hwtstamp;
>>   };
>>   struct bpf_tunnel_key {
>>
>