linux-kernel - Re: [PATCH v4 bpf-next 0/9] add bpf batch ops to process more than 1 elem

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d554cbd3-3225-6238-3b78-552f0e813ce9@fb.com>
Date:   Tue, 14 Jan 2020 23:12:22 +0000
From:   Yonghong Song <yhs@...com>
To:     Brian Vazquez <brianvv@...gle.com>,
        Brian Vazquez <brianvv.kernel@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        "Daniel Borkmann" <daniel@...earbox.net>,
        "David S . Miller" <davem@...emloft.net>
CC:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Petar Penkov <ppenkov@...gle.com>,
        Willem de Bruijn <willemb@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [PATCH v4 bpf-next 0/9] add bpf batch ops to process more than 1
 elem



On 1/14/20 8:46 AM, Brian Vazquez wrote:
> This patch series introduce batch ops that can be added to bpf maps to
> lookup/lookup_and_delete/update/delete more than 1 element at the time,
> this is specially useful when syscall overhead is a problem and in case
> of hmap it will provide a reliable way of traversing them.
> 
> The implementation inclues a generic approach that could potentially be
> used by any bpf map and adds it to arraymap, it also includes the specific
> implementation of hashmaps which are traversed using buckets instead
> of keys.
> 
> The bpf syscall subcommands introduced are:
> 
>    BPF_MAP_LOOKUP_BATCH
>    BPF_MAP_LOOKUP_AND_DELETE_BATCH
>    BPF_MAP_UPDATE_BATCH
>    BPF_MAP_DELETE_BATCH
> 
> The UAPI attribute is:
> 
>    struct { /* struct used by BPF_MAP_*_BATCH commands */
>           __aligned_u64   in_batch;       /* start batch,
>                                            * NULL to start from beginning
>                                            */
>           __aligned_u64   out_batch;      /* output: next start batch */
>           __aligned_u64   keys;
>           __aligned_u64   values;
>           __u32           count;          /* input/output:
>                                            * input: # of key/value
>                                            * elements
>                                            * output: # of filled elements
>                                            */
>           __u32           map_fd;
>           __u64           elem_flags;
>           __u64           flags;
>    } batch;
> 
> 
> in_batch and out_batch are only used for lookup and lookup_and_delete since
> those are the only two operations that attempt to traverse the map.
> 
> update/delete batch ops should provide the keys/values that user wants
> to modify.
> 
> Here are the previous discussions on the batch processing:
>   - https://lore.kernel.org/bpf/20190724165803.87470-1-brianvv@google.com/
>   - https://lore.kernel.org/bpf/20190829064502.2750303-1-yhs@fb.com/
>   - https://lore.kernel.org/bpf/20190906225434.3635421-1-yhs@fb.com/
> 
> Changelog sinve v3:
>   - Do not use copy_to_user inside atomic region (Yonghong Song)
>   - Use _opts approach on libbpf APIs (Andrii Nakryiko)
>   - Drop generic_map_lookup_and_delete_batch support
>   - Free malloc-ed memory in tests (Yonghong Song)
>   - Reverse christmas tree (Yonghong Song)
>   - Add acked labels

Thanks for the new revision! Overall looks good. Only have a few minor
comments. Also tested in my environment and everything works as expected.