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

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

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@xxxxxxxx>
> ---
>   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.

> +	__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 {