mbox series

[bpf-next,0/2] add hwtstamp to __sk_buff

Message ID 20210902221551.15566-1-vfedorenko@novek.ru
Headers show
Series add hwtstamp to __sk_buff | expand

Message

Vadim Fedorenko Sept. 2, 2021, 10:15 p.m. UTC 
This patch set adds hardware timestamps to __sk_buff. The first patch
implements feature, the second one adds a selftest.

Vadim Fedorenko (2):
  bpf: add hardware timestamp field to __sk_buff
  selftests/bpf: test new __sk_buff field hwtstamp

 include/uapi/linux/bpf.h                      |  2 +
 lib/test_bpf.c                                |  1 +
 net/bpf/test_run.c                            |  8 ++++
 net/core/filter.c                             | 11 +++++
 tools/include/uapi/linux/bpf.h                |  2 +
 .../selftests/bpf/prog_tests/skb_ctx.c        |  1 +
 .../selftests/bpf/progs/test_skb_ctx.c        |  2 +
 .../testing/selftests/bpf/verifier/ctx_skb.c  | 47 +++++++++++++++++++
 8 files changed, 74 insertions(+)

Comments

Daniel Borkmann Sept. 3, 2021, 8:22 a.m. UTC | #1 
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.

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

>
Vadim Fedorenko Sept. 3, 2021, 8:35 a.m. UTC | #2 
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 {

>>

>