From: Alexei Starovoitov <ast@plumgrid•com>
To: Daniel Borkmann <daniel@iogearbox•net>, davem@davemloft•net
Cc: netdev@vger•kernel.org
Subject: Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling
Date: Fri, 13 Mar 2015 15:53:49 -0700 [thread overview]
Message-ID: <55036A7D.7050402@plumgrid.com> (raw)
In-Reply-To: <55036856.1040309@iogearbox.net>
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.
next prev parent reply other threads:[~2015-03-13 22:53 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-13 13:22 [PATCH net-next 0/2] eBPF updates Daniel Borkmann
2015-03-13 13:22 ` [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling Daniel Borkmann
2015-03-13 16:58 ` Alexei Starovoitov
2015-03-13 17:03 ` Daniel Borkmann
2015-03-13 22:44 ` Daniel Borkmann
2015-03-13 22:53 ` Alexei Starovoitov [this message]
2015-03-14 0:11 ` Daniel Borkmann
2015-03-13 13:22 ` [PATCH net-next 2/2] ebpf: add helper for obtaining current processor id Daniel Borkmann
2015-03-13 17:06 ` Alexei Starovoitov
2015-03-13 17:08 ` Daniel Borkmann
2015-03-16 1:57 ` [PATCH net-next 0/2] eBPF updates David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55036A7D.7050402@plumgrid.com \
--to=ast@plumgrid$(echo .)com \
--cc=daniel@iogearbox$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox