[PATCH net-next 0/2] eBPF updates

[PATCH net-next 0/2] eBPF updates

[Daniel Borkmann]

Two small eBPF helper additions to better match up with ancillary
classic BPF functionality.

Daniel Borkmann (2):
  ebpf: add prandom helper for packet sampling
  ebpf: add helper for obtaining current processor id

 include/linux/bpf.h      |  3 +++
 include/uapi/linux/bpf.h |  2 ++
 kernel/bpf/core.c        |  3 +++
 kernel/bpf/helpers.c     | 25 +++++++++++++++++++++++++
 net/core/filter.c        |  4 ++++
 5 files changed, 37 insertions(+)

-- 
1.9.3

[PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

[Daniel Borkmann]

This work is similar to commit 4cd3675ebf74 ("filter: added BPF
random opcode") and adds a possibility for packet sampling in eBPF.

Currently, this is only possible in classic BPF and useful to
combine sampling with f.e. packet sockets, possible also with tc.

Given classic BPF operates in 32bit space only, we leave the
option to the eBPF application developer whether he needs a single
or double word wide random number since eBPF is in 64bit space.

Example function proto-type looks like:

  static u64 (*get_rnd)(bool dw) = (void *) BPF_FUNC_random;

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/bpf.h      |  2 ++
 include/uapi/linux/bpf.h |  1 +
 kernel/bpf/core.c        |  2 ++
 kernel/bpf/helpers.c     | 13 +++++++++++++
 net/core/filter.c        |  2 ++
 5 files changed, 20 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 80f2e0f..6da2145 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -154,4 +154,6 @@ extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
 extern const struct bpf_func_proto bpf_map_update_elem_proto;
 extern const struct bpf_func_proto bpf_map_delete_elem_proto;
 
+extern const struct bpf_func_proto bpf_random_proto;
+
 #endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3fa1af8..4efc41f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -165,6 +165,7 @@ enum bpf_func_id {
 	BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(&map, &key) */
 	BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
 	BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
+	BPF_FUNC_random,	  /* prandom() as u32 or u64 */
 	__BPF_FUNC_MAX_ID,
 };
 
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 50603ae..cc1f3f8 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -661,6 +661,8 @@ const struct bpf_func_proto bpf_map_lookup_elem_proto __weak;
 const struct bpf_func_proto bpf_map_update_elem_proto __weak;
 const struct bpf_func_proto bpf_map_delete_elem_proto __weak;
 
+const struct bpf_func_proto bpf_random_proto __weak;
+
 /* To execute LD_ABS/LD_IND instructions __bpf_prog_run() may call
  * skb_copy_bits(), so provide a weak definition of it for NET-less config.
  */
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a3c7701..9b0e6eb 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -11,6 +11,7 @@
  */
 #include <linux/bpf.h>
 #include <linux/rcupdate.h>
+#include <linux/random.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -87,3 +88,15 @@ const struct bpf_func_proto bpf_map_delete_elem_proto = {
 	.arg1_type = ARG_CONST_MAP_PTR,
 	.arg2_type = ARG_PTR_TO_MAP_KEY,
 };
+
+static u64 bpf_random(u64 dw, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	return prandom_u32() | (dw ? ((u64)prandom_u32() << 32) : 0);
+}
+
+const struct bpf_func_proto bpf_random_proto = {
+	.func		= bpf_random,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/net/core/filter.c b/net/core/filter.c
index 7a4eb70..c8f1efd 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1139,6 +1139,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
 		return &bpf_map_update_elem_proto;
 	case BPF_FUNC_map_delete_elem:
 		return &bpf_map_delete_elem_proto;
+	case BPF_FUNC_random:
+		return &bpf_random_proto;
 	default:
 		return NULL;
 	}
-- 
1.9.3

Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

[Alexei Starovoitov]

On 3/13/15 6:22 AM, Daniel Borkmann wrote:
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 3fa1af8..4efc41f 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -165,6 +165,7 @@ enum bpf_func_id {
>   	BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(&map, &key) */
>   	BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
>   	BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
> +	BPF_FUNC_random,	  /* prandom() as u32 or u64 */

can you rename it to BPF_FUNC_get_random_value ?
I'd like names to be as descriptive as possible.
User space can pick any name it likes anyway:
static u64 (*my_foo_func)() = (void *) BPF_FUNC_get_random_value;

Also it will make the following less obscure:
 > +extern const struct bpf_func_proto bpf_random_proto;

'bpf_get_random_value_proto' looks better then 'bpf_random_proto' ;)

> +static u64 bpf_random(u64 dw, u64 r2, u64 r3, u64 r4, u64 r5)
> +{
> +	return prandom_u32() | (dw ? ((u64)prandom_u32() << 32) : 0);

can you make first argument to be a flag instead of bool ?
0 to return u32, 1 to return u64 and >=2 to be reserved and
return -EINVAL?
This way we can extend it later. For example 2 might return
8 truly random bytes from get_random_bytes_arch()

> +const struct bpf_func_proto bpf_random_proto = {
> +	.func		= bpf_random,
> +	.gpl_only	= false,

non-gpl allowed?
well I guess prandom_u32() is non-gpl either.

Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

[Daniel Borkmann]

On 03/13/2015 05:58 PM, Alexei Starovoitov wrote:
> On 3/13/15 6:22 AM, Daniel Borkmann wrote:
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 3fa1af8..4efc41f 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -165,6 +165,7 @@ enum bpf_func_id {
>>       BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(&map, &key) */
>>       BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
>>       BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
>> +    BPF_FUNC_random,      /* prandom() as u32 or u64 */
>
> can you rename it to BPF_FUNC_get_random_value ?
> I'd like names to be as descriptive as possible.

Ok, sure.

> User space can pick any name it likes anyway:
> static u64 (*my_foo_func)() = (void *) BPF_FUNC_get_random_value;
>
> Also it will make the following less obscure:
>  > +extern const struct bpf_func_proto bpf_random_proto;
>
> 'bpf_get_random_value_proto' looks better then 'bpf_random_proto' ;)
>
>> +static u64 bpf_random(u64 dw, u64 r2, u64 r3, u64 r4, u64 r5)
>> +{
>> +    return prandom_u32() | (dw ? ((u64)prandom_u32() << 32) : 0);
>
> can you make first argument to be a flag instead of bool ?
> 0 to return u32, 1 to return u64 and >=2 to be reserved and
> return -EINVAL?
> This way we can extend it later. For example 2 might return
> 8 truly random bytes from get_random_bytes_arch()

Yes, that's better.

>> +const struct bpf_func_proto bpf_random_proto = {
>> +    .func        = bpf_random,
>> +    .gpl_only    = false,
>
> non-gpl allowed?
> well I guess prandom_u32() is non-gpl either.

It can be used by anyone, yes. If there are strong opinions to make
this GPL-only, I don't mind, but I think it might be exaggerated.

Will respin.

Thanks,
Daniel

Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

[Daniel Borkmann]

On 03/13/2015 06:03 PM, Daniel Borkmann wrote:
> On 03/13/2015 05:58 PM, Alexei Starovoitov wrote:
...
>>> +static u64 bpf_random(u64 dw, u64 r2, u64 r3, u64 r4, u64 r5)
>>> +{
>>> +    return prandom_u32() | (dw ? ((u64)prandom_u32() << 32) : 0);
>>
>> can you make first argument to be a flag instead of bool ?
>> 0 to return u32, 1 to return u64 and >=2 to be reserved and
>> return -EINVAL?
...
>> This way we can extend it later. For example 2 might return
>> 8 truly random bytes from get_random_bytes_arch()

Hmm, just got back to this thinking about it a bit more. I don't
like that we would return with a negative error here. An eBPF
application would then need to check for errors as well to make
sure, and the signature returns max u64. So _theoretically_,
-EINVAL could also come as a result from prandom() albeit unlikely.
That would make the interface a bit ugly, imho.

I think the interface should be as simple as possible for
obtaining a random number. I think that if we would want to
support something like get_random_bytes_arch(), it would be
better to do so in a new interface, where the name also more
clearly indicates where the random data comes from.

That said, I'd rather prefer to leave the code as is, but go
with the rename, maybe even BPF_FUNC_get_prandom_val so it
indicates it's pseudo-random as opposed to, say, something like
BPF_FUNC_get_srandom_val if we need that one day.

Cheers,
Daniel

Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

[Alexei Starovoitov]

On 3/13/15 3:44 PM, Daniel Borkmann wrote:
> On 03/13/2015 06:03 PM, Daniel Borkmann wrote:
>> On 03/13/2015 05:58 PM, Alexei Starovoitov wrote:
> ...
>>>> +static u64 bpf_random(u64 dw, u64 r2, u64 r3, u64 r4, u64 r5)
>>>> +{
>>>> +    return prandom_u32() | (dw ? ((u64)prandom_u32() << 32) : 0);
>>>
>>> can you make first argument to be a flag instead of bool ?
>>> 0 to return u32, 1 to return u64 and >=2 to be reserved and
>>> return -EINVAL?
> ...
>>> This way we can extend it later. For example 2 might return
>>> 8 truly random bytes from get_random_bytes_arch()
>
> Hmm, just got back to this thinking about it a bit more. I don't
> like that we would return with a negative error here. An eBPF
> application would then need to check for errors as well to make
> sure, and the signature returns max u64. So _theoretically_,
> -EINVAL could also come as a result from prandom() albeit unlikely.
> That would make the interface a bit ugly, imho.
>
> I think the interface should be as simple as possible for
> obtaining a random number. I think that if we would want to
> support something like get_random_bytes_arch(), it would be
> better to do so in a new interface, where the name also more
> clearly indicates where the random data comes from.
>
> That said, I'd rather prefer to leave the code as is, but go
> with the rename, maybe even BPF_FUNC_get_prandom_val so it
> indicates it's pseudo-random as opposed to, say, something like
> BPF_FUNC_get_srandom_val if we need that one day.

fair point.

I was also thinking about it more and I believe that 'bool' flag
is redundant.
If program wants to have 64-bit value it should just call this
helper twice. It actually will be faster, since such 64-bit
value will be without branches whereas 'dw' flag introduces
if condition in critical path that will be executed for every packet.
So my vote is for BPF_FUNC_get_prandom_u32 that returns
prandom_u32() and nothing else.

Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

[Daniel Borkmann]

On 03/13/2015 11:53 PM, Alexei Starovoitov wrote:
...
> So my vote is for BPF_FUNC_get_prandom_u32 that returns
> prandom_u32() and nothing else.

I'll go for that. Occam's razor. ;)

[PATCH net-next 2/2] ebpf: add helper for obtaining current processor id

[Daniel Borkmann]

This patch adds the possibility to obtain raw_smp_processor_id() in
eBPF. Currently, this is only possible in classic BPF where commit
da2033c28226 ("filter: add SKF_AD_RXHASH and SKF_AD_CPU") has added
facilities for this.

Single threaded packet socket applications (f.e. tcpdump, netsniff-ng)
e.g. writing to pcap files and making use of eBPF can use this helper
function to spread flows after RPS dispatch to several instances. It
would also allow to track per CPU statistics (or data) with eBPF maps.

Example function proto-type looks like:

  unsigned int (*get_cpu_id)(void) = (void *) BPF_FUNC_cpu_id;

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h |  1 +
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 12 ++++++++++++
 net/core/filter.c        |  2 ++
 5 files changed, 17 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 6da2145..28e427d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -155,5 +155,6 @@ extern const struct bpf_func_proto bpf_map_update_elem_proto;
 extern const struct bpf_func_proto bpf_map_delete_elem_proto;
 
 extern const struct bpf_func_proto bpf_random_proto;
+extern const struct bpf_func_proto bpf_cpu_id_proto;
 
 #endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4efc41f..9080cf5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -166,6 +166,7 @@ enum bpf_func_id {
 	BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
 	BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
 	BPF_FUNC_random,	  /* prandom() as u32 or u64 */
+	BPF_FUNC_cpu_id,	  /* raw_smp_processor_id() */
 	__BPF_FUNC_MAX_ID,
 };
 
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index cc1f3f8..b303cf3 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -662,6 +662,7 @@ const struct bpf_func_proto bpf_map_update_elem_proto __weak;
 const struct bpf_func_proto bpf_map_delete_elem_proto __weak;
 
 const struct bpf_func_proto bpf_random_proto __weak;
+const struct bpf_func_proto bpf_cpu_id_proto __weak;
 
 /* To execute LD_ABS/LD_IND instructions __bpf_prog_run() may call
  * skb_copy_bits(), so provide a weak definition of it for NET-less config.
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 9b0e6eb..ba7fab6 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -12,6 +12,7 @@
 #include <linux/bpf.h>
 #include <linux/rcupdate.h>
 #include <linux/random.h>
+#include <linux/smp.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -100,3 +101,14 @@ const struct bpf_func_proto bpf_random_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_ANYTHING,
 };
+
+static u64 bpf_cpu_id(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	return raw_smp_processor_id();
+}
+
+const struct bpf_func_proto bpf_cpu_id_proto = {
+	.func		= bpf_cpu_id,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+};
diff --git a/net/core/filter.c b/net/core/filter.c
index c8f1efd..1643521 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1141,6 +1141,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
 		return &bpf_map_delete_elem_proto;
 	case BPF_FUNC_random:
 		return &bpf_random_proto;
+	case BPF_FUNC_cpu_id:
+		return &bpf_cpu_id_proto;
 	default:
 		return NULL;
 	}
-- 
1.9.3

Re: [PATCH net-next 2/2] ebpf: add helper for obtaining current processor id

[Alexei Starovoitov]

On 3/13/15 6:22 AM, Daniel Borkmann wrote:
> This patch adds the possibility to obtain raw_smp_processor_id() in
> eBPF. Currently, this is only possible in classic BPF where commit
> da2033c28226 ("filter: add SKF_AD_RXHASH and SKF_AD_CPU") has added
> facilities for this.
>
> Single threaded packet socket applications (f.e. tcpdump, netsniff-ng)
> e.g. writing to pcap files and making use of eBPF can use this helper
> function to spread flows after RPS dispatch to several instances. It
> would also allow to track per CPU statistics (or data) with eBPF maps.

I'm not sure whether classic extension is still used by Eric as it was
envisioned in that old commit, but I do like addition of it for eBPF,
since it allows a poor men's per-cpu data structures via maps :)

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4efc41f..9080cf5 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -166,6 +166,7 @@ enum bpf_func_id {
>   	BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
>   	BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
>   	BPF_FUNC_random,	  /* prandom() as u32 or u64 */
> +	BPF_FUNC_cpu_id,	  /* raw_smp_processor_id() */

same comment as in other patch:
could you rename it to BPF_FUNC_get_smp_processor_id ?

Re: [PATCH net-next 2/2] ebpf: add helper for obtaining current processor id

[Daniel Borkmann]

On 03/13/2015 06:06 PM, Alexei Starovoitov wrote:
...
> same comment as in other patch:
> could you rename it to BPF_FUNC_get_smp_processor_id ?

Will do, yep.

Re: [PATCH net-next 0/2] eBPF updates

[David Miller]

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 13 Mar 2015 14:22:02 +0100

> Two small eBPF helper additions to better match up with ancillary
> classic BPF functionality.

Applied, thanks.