Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200115184308.162644-1-brianvv@google.com> <0ed19302-a43c-c04e-110e-eb1f0a72146f@fb.com>
In-Reply-To: <0ed19302-a43c-c04e-110e-eb1f0a72146f@fb.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Wed, 15 Jan 2020 14:09:54 -0800
Message-ID: <CAADnVQK5sBx7PFwqMGZi_6+6xFZQoE9deTujL4ewNBQzOi1q6Q@mail.gmail.com>
Subject: Re: [PATCH v5 bpf-next 0/9] add bpf batch ops to process more than 1 elem
To:     Yonghong Song <yhs@fb.com>
Cc:     Brian Vazquez <brianvv@google.com>,
        Brian Vazquez <brianvv.kernel@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        "David S . Miller" <davem@davemloft.net>,
        Andrii Nakryiko <andrii.nakryiko@gmail.com>,
        Stanislav Fomichev <sdf@google.com>,
        Petar Penkov <ppenkov@google.com>,
        Willem de Bruijn <willemb@google.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jan 15, 2020 at 12:13 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 1/15/20 10:42 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 v4:
> >   - Remove unnecessary checks from libbpf API (Andrii Nakryiko)
> >   - Move DECLARE_LIBBPF_OPTS with all var declarations (Andrii Nakryiko)
> >   - Change bucket internal buffer size to 5 entries (Yonghong Song)
> >   - Fix some minor bugs in hashtab batch ops implementation (Yonghong Song)
> >
> > 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
> >
> > Changelog sinve v2:
> >   - Add generic batch support for lpm_trie and test it (Yonghong Song)
> >   - Use define MAP_LOOKUP_RETRIES for retries (John Fastabend)
> >   - Return errors directly and remove labels (Yonghong Song)
> >   - Insert new API functions into libbpf alphabetically (Yonghong Song)
> >   - Change hlist_nulls_for_each_entry_rcu to
> >     hlist_nulls_for_each_entry_safe in htab batch ops (Yonghong Song)
> >
> > Changelog since v1:
> >   - Fix SOB ordering and remove Co-authored-by tag (Alexei Starovoitov)
> >
> > Changelog since RFC:
> >   - Change batch to in_batch and out_batch to support more flexible opaque
> >     values to iterate the bpf maps.
> >   - Remove update/delete specific batch ops for htab and use the generic
> >     implementations instead.
> >
> > Brian Vazquez (5):
> >    bpf: add bpf_map_{value_size,update_value,map_copy_value} functions
> >    bpf: add generic support for lookup batch op
> >    bpf: add generic support for update and delete batch ops
> >    bpf: add lookup and update batch ops to arraymap
> >    selftests/bpf: add batch ops testing to array bpf map
> >
> > Yonghong Song (4):
> >    bpf: add batch ops to all htab bpf map
> >    tools/bpf: sync uapi header bpf.h
> >    libbpf: add libbpf support to batch ops
> >    selftests/bpf: add batch ops testing for htab and htab_percpu map
> >
> >   include/linux/bpf.h                           |  18 +
> >   include/uapi/linux/bpf.h                      |  21 +
> >   kernel/bpf/arraymap.c                         |   2 +
> >   kernel/bpf/hashtab.c                          | 264 +++++++++
> >   kernel/bpf/syscall.c                          | 554 ++++++++++++++----
> >   tools/include/uapi/linux/bpf.h                |  21 +
> >   tools/lib/bpf/bpf.c                           |  58 ++
> >   tools/lib/bpf/bpf.h                           |  22 +
> >   tools/lib/bpf/libbpf.map                      |   4 +
> >   .../bpf/map_tests/array_map_batch_ops.c       | 129 ++++
> >   .../bpf/map_tests/htab_map_batch_ops.c        | 283 +++++++++
> >   11 files changed, 1248 insertions(+), 128 deletions(-)
> >   create mode 100644 tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
> >   create mode 100644 tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
>
> Thanks for the work! LGTM. Ack for the whole series.
>
> Acked-by: Yonghong Song <yhs@fb.com>

Applied. Thanks!