bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] add hwtstamp to __sk_buff
@ 2021-09-02 22:15 Vadim Fedorenko
  2021-09-02 22:15 ` [PATCH bpf-next 1/2] bpf: add hardware timestamp field " Vadim Fedorenko
  2021-09-02 22:15 ` [PATCH bpf-next 2/2] selftests/bpf: test new __sk_buff field hwtstamp Vadim Fedorenko
  0 siblings, 2 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2021-09-02 22:15 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Vadim Fedorenko, Alexei Starovoitov, Song Liu, Yonghong Song,
	John Fastabend, Jakub Kicinski, netdev, bpf

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(+)

-- 
2.18.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/2] bpf: add hardware timestamp field to __sk_buff
  2021-09-02 22:15 [PATCH bpf-next 0/2] add hwtstamp to __sk_buff Vadim Fedorenko
@ 2021-09-02 22:15 ` Vadim Fedorenko
  2021-09-03  8:22   ` Daniel Borkmann
  2021-09-02 22:15 ` [PATCH bpf-next 2/2] selftests/bpf: test new __sk_buff field hwtstamp Vadim Fedorenko
  1 sibling, 1 reply; 5+ messages in thread
From: Vadim Fedorenko @ 2021-09-02 22:15 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Vadim Fedorenko, Alexei Starovoitov, Song Liu, Yonghong Song,
	John Fastabend, Jakub Kicinski, netdev, bpf

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. */
+	__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 {
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: test new __sk_buff field hwtstamp
  2021-09-02 22:15 [PATCH bpf-next 0/2] add hwtstamp to __sk_buff Vadim Fedorenko
  2021-09-02 22:15 ` [PATCH bpf-next 1/2] bpf: add hardware timestamp field " Vadim Fedorenko
@ 2021-09-02 22:15 ` Vadim Fedorenko
  1 sibling, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2021-09-02 22:15 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Vadim Fedorenko, Alexei Starovoitov, Song Liu, Yonghong Song,
	John Fastabend, Jakub Kicinski, netdev, bpf

Analogous to the gso_segs selftests introduced in commit d9ff286a0f59
("bpf: allow BPF programs access skb_shared_info->gso_segs field")

Signed-off-by: Vadim Fedorenko <vfedorenko@novek.ru>
---
 lib/test_bpf.c                                |  1 +
 net/bpf/test_run.c                            |  8 ++++
 .../selftests/bpf/prog_tests/skb_ctx.c        |  1 +
 .../selftests/bpf/progs/test_skb_ctx.c        |  2 +
 .../testing/selftests/bpf/verifier/ctx_skb.c  | 47 +++++++++++++++++++
 5 files changed, 59 insertions(+)

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 830a18ecffc8..0018d51b93b0 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -8800,6 +8800,7 @@ static __init struct sk_buff *build_test_skb(void)
 	skb_shinfo(skb[0])->gso_type |= SKB_GSO_DODGY;
 	skb_shinfo(skb[0])->gso_segs = 0;
 	skb_shinfo(skb[0])->frag_list = skb[1];
+	skb_shinfo(skb[0])->hwtstamps.hwtstamp = 1000;
 
 	/* adjust skb[0]'s len */
 	skb[0]->len += skb[1]->len;
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 2eb0e55ef54d..1232dd839d7a 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -511,6 +511,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 	/* gso_size is allowed */
 
 	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, gso_size),
+			   offsetof(struct __sk_buff, hwtstamp)))
+		return -EINVAL;
+
+	/* hwtstamp is allowed */
+
+	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, hwtstamp),
 			   sizeof(struct __sk_buff)))
 		return -EINVAL;
 
@@ -532,6 +538,7 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 		return -EINVAL;
 	skb_shinfo(skb)->gso_segs = __skb->gso_segs;
 	skb_shinfo(skb)->gso_size = __skb->gso_size;
+	skb_shinfo(skb)->hwtstamps.hwtstamp = __skb->hwtstamp;
 
 	return 0;
 }
@@ -550,6 +557,7 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
 	memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
 	__skb->wire_len = cb->pkt_len;
 	__skb->gso_segs = skb_shinfo(skb)->gso_segs;
+	__skb->hwtstamp = skb_shinfo(skb)->hwtstamps.hwtstamp;
 }
 
 int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
diff --git a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
index fafeddaad6a9..87ca95e8442b 100644
--- a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
+++ b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
@@ -17,6 +17,7 @@ void test_skb_ctx(void)
 		.gso_segs = 8,
 		.mark = 9,
 		.gso_size = 10,
+		.hwtstamp = 11,
 	};
 	struct bpf_prog_test_run_attr tattr = {
 		.data_in = &pkt_v4,
diff --git a/tools/testing/selftests/bpf/progs/test_skb_ctx.c b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
index b02ea589ce7e..5cfa91cf0f08 100644
--- a/tools/testing/selftests/bpf/progs/test_skb_ctx.c
+++ b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
@@ -25,6 +25,8 @@ int process(struct __sk_buff *skb)
 		return 1;
 	if (skb->gso_size != 10)
 		return 1;
+	if (skb->hwtstamp != 11)
+		return 1;
 
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/verifier/ctx_skb.c b/tools/testing/selftests/bpf/verifier/ctx_skb.c
index 2022c0f2cd75..ab3df1aa691d 100644
--- a/tools/testing/selftests/bpf/verifier/ctx_skb.c
+++ b/tools/testing/selftests/bpf/verifier/ctx_skb.c
@@ -1057,6 +1057,53 @@
 	.result = ACCEPT,
 	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 },
+{
+	"read hwtstamp from CGROUP_SKB",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
+		    offsetof(struct __sk_buff, hwtstamp)),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+},
+{
+	"read hwtstamp from CGROUP_SKB",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1,
+		    offsetof(struct __sk_buff, hwtstamp)),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+},
+{
+	"write hwtstamp from CGROUP_SKB",
+	.insns = {
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+		    offsetof(struct __sk_buff, hwtstamp)),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.result = REJECT,
+	.result_unpriv = REJECT,
+	.errstr = "invalid bpf_context access off=184 size=8",
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+},
+{
+	"read hwtstamp from CLS",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
+		    offsetof(struct __sk_buff, hwtstamp)),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+},
 {
 	"check wire_len is not readable by sockets",
 	.insns = {
-- 
2.18.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: add hardware timestamp field to __sk_buff
  2021-09-02 22:15 ` [PATCH bpf-next 1/2] bpf: add hardware timestamp field " Vadim Fedorenko
@ 2021-09-03  8:22   ` Daniel Borkmann
  2021-09-03  8:35     ` Vadim Fedorenko
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2021-09-03  8:22 UTC (permalink / raw)
  To: Vadim Fedorenko, Martin KaFai Lau
  Cc: Alexei Starovoitov, Song Liu, Yonghong Song, John Fastabend,
	Jakub Kicinski, netdev, bpf, willemb

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: add hardware timestamp field to __sk_buff
  2021-09-03  8:22   ` Daniel Borkmann
@ 2021-09-03  8:35     ` Vadim Fedorenko
  0 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2021-09-03  8:35 UTC (permalink / raw)
  To: Daniel Borkmann, Martin KaFai Lau
  Cc: Alexei Starovoitov, Song Liu, Yonghong Song, John Fastabend,
	Jakub Kicinski, netdev, bpf, willemb

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-03  8:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-02 22:15 [PATCH bpf-next 0/2] add hwtstamp to __sk_buff Vadim Fedorenko
2021-09-02 22:15 ` [PATCH bpf-next 1/2] bpf: add hardware timestamp field " Vadim Fedorenko
2021-09-03  8:22   ` Daniel Borkmann
2021-09-03  8:35     ` Vadim Fedorenko
2021-09-02 22:15 ` [PATCH bpf-next 2/2] selftests/bpf: test new __sk_buff field hwtstamp Vadim Fedorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).