netdev - Re: [ovs-dev] [RFC: add openvswitch actions using BPF 1/2] BPF: add a new BPF program type BPF_PROG_TYPE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACzMAJ+AhoDs487TN02bawS9fWCQ26FmGwOvw1C79PW5iX=QwA@mail.gmail.com>
Date:	Thu, 5 Feb 2015 11:47:15 -0800
From:	Andy Zhou <azhou@...ira.com>
To:	Thomas Graf <tgraf@...ronetworks.com>
Cc:	"dev@...nvswitch.com" <dev@...nvswitch.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [ovs-dev] [RFC: add openvswitch actions using BPF 1/2] BPF: add a
 new BPF program type BPF_PROG_TYPE_OPENVSWITCH

On Thu, Feb 5, 2015 at 6:48 AM, Thomas Graf <tgraf@...ronetworks.com> wrote:
> On 02/04/15 at 02:48pm, Andy Zhou wrote:
>>  struct bpf_verifier_ops {
>>       /* return eBPF function prototype for verification */
>> -     const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
>> +     const struct bpf_func_proto *(*get_func_proto)(int func_id);
>
> This change should maybe go in a separate commit.
Agreed.  Ideally, each program type should have its own func_id space. Current
registration does not allow this. Should this be added?

In the interest of reusing common helper functions, should we also consider
partition the func_id space into common ones and program type specific ones?
For example, the prink like function can be specified the common func_id space.

>
>> +static const struct bpf_func_proto *ovs_func_proto(int func_id)
>> +{
>> +     switch (func_id) {
>> +     case OVS_BPF_FUNC_output:
>> +             return &bpf_helper_output_proto;
>> +     default:
>> +             return NULL;
>> +     }
>> +}
>
> You'd still want to use the map helpers so it seems like we should
> change the bpf verified to verify against both a global and type
> specific list unless we want to add all the map helpers to
> ovs_func_proto as well.

It is not clear what OVS can benefit from the map helpers.  Map provides
a fixed sized array.  OVS data structure, such as flow, are more
dynamic and non-contiguous in memory.

Is extending BPF to allow for passing those dynamic objects a reasonable
direction?

>
>> +static bool test_is_valid_access(int off, int size, enum bpf_access_type type)
>> +{
>> +     const struct bpf_context_access *access;
>> +
>> +     if (off < 0 || off >= ARRAY_SIZE(bpf_ctx_access))
>> +             return false;
>> +
>> +     access = &bpf_ctx_access[off];
>> +     if (access->size == size && (access->type & type))
>> +             return true;
>> +
>> +     return false;
>> +}
>
> OK. I see why you kept ctxt simple at first ;-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html