netdev - Re: [PATCH bpf-next v2 0/6] xdp: Add devmap

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]
Date:   Sat, 6 Jul 2019 11:58:49 -0700
From:   Y Song <ys114321@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Björn Töpel <bjorn.topel@...il.com>
Subject: Re: [PATCH bpf-next v2 0/6] xdp: Add devmap_hash map type

On Sat, Jul 6, 2019 at 1:47 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> This series adds a new map type, devmap_hash, that works like the existing
> devmap type, but using a hash-based indexing scheme. This is useful for the use
> case where a devmap is indexed by ifindex (for instance for use with the routing
> table lookup helper). For this use case, the regular devmap needs to be sized
> after the maximum ifindex number, not the number of devices in it. A hash-based
> indexing scheme makes it possible to size the map after the number of devices it
> should contain instead.
>
> This was previously part of my patch series that also turned the regular
> bpf_redirect() helper into a map-based one; for this series I just pulled out
> the patches that introduced the new map type.
>
> Changelog:
>
> v2:
>
> - Split commit adding the new map type so uapi and tools changes are separate.
>
> Changes to these patches since the previous series:
>
> - Rebase on top of the other devmap changes (makes this one simpler!)
> - Don't enforce key==val, but allow arbitrary indexes.
> - Rename the type to devmap_hash to reflect the fact that it's just a hashmap now.
>
> ---
>
> Toke Høiland-Jørgensen (6):
>       include/bpf.h: Remove map_insert_ctx() stubs
>       xdp: Refactor devmap allocation code for reuse
>       uapi/bpf: Add new devmap_hash type
>       xdp: Add devmap_hash map type for looking up devices by hashed index
>       tools/libbpf_probes: Add new devmap_hash type
>       tools: Add definitions for devmap_hash map type

Thanks for re-organize the patch. I guess this can be tweaked a little more
to better suit for syncing between kernel and libbpf repo.

Let me provide a little bit background here. The below is
a sync done by Andrii from kernel/tools to libbpf repo.

=============
commit 39de6711795f6d1583ae96ed8d13892bc4475ac1
Author: Andrii Nakryiko <andriin@...com>
Date:   Tue Jun 11 09:56:11 2019 -0700

    sync: latest libbpf changes from kernel

    Syncing latest libbpf commits from kernel repository.
    Baseline commit:   e672db03ab0e43e41ab6f8b2156a10d6e40f243d
    Checkpoint commit: 5e2ac390fbd08b2a462db66cef2663e4db0d5191

    Andrii Nakryiko (9):
      libbpf: fix detection of corrupted BPF instructions section
      libbpf: preserve errno before calling into user callback
      libbpf: simplify endianness check
      libbpf: check map name retrieved from ELF
      libbpf: fix error code returned on corrupted ELF
      libbpf: use negative fd to specify missing BTF
      libbpf: simplify two pieces of logic
      libbpf: typo and formatting fixes
      libbpf: reduce unnecessary line wrapping

    Hechao Li (1):
      bpf: add a new API libbpf_num_possible_cpus()

    Jonathan Lemon (2):
      bpf/tools: sync bpf.h
      libbpf: remove qidconf and better support external bpf programs.

    Quentin Monnet (1):
      libbpf: prevent overwriting of log_level in bpf_object__load_progs()

     include/uapi/linux/bpf.h |   4 +
     src/libbpf.c             | 207 ++++++++++++++++++++++-----------------
     src/libbpf.h             |  16 +++
     src/libbpf.map           |   1 +
     src/xsk.c                | 103 ++++++-------------
     5 files changed, 167 insertions(+), 164 deletions(-)
==========

You can see the commits at tools/lib/bpf and
commits at tools/include/uapi/{linux/[bpf.h, btf.h], ...}
are sync'ed to libbpf repo.

So we would like kernel commits to be aligned that way for better
automatic syncing.

Therefore, your current patch set could be changed from
   >       include/bpf.h: Remove map_insert_ctx() stubs
   >       xdp: Refactor devmap allocation code for reuse
   >       uapi/bpf: Add new devmap_hash type
   >       xdp: Add devmap_hash map type for looking up devices by hashed index
   >       tools/libbpf_probes: Add new devmap_hash type
   >       tools: Add definitions for devmap_hash map type
to
      1. include/bpf.h: Remove map_insert_ctx() stubs
      2. xdp: Refactor devmap allocation code for reuse
      3. kernel non-tools changes (the above patch #3 and #4)
      4. tools/include/uapi change (part of the above patch #6)
      5. tools/libbpf_probes change
      6. other tools/ change (the above patch #6 - new patch #4).

Thanks!

Yonghong

>
>
>  include/linux/bpf.h                     |   11 -
>  include/linux/bpf_types.h               |    1
>  include/trace/events/xdp.h              |    3
>  include/uapi/linux/bpf.h                |    1
>  kernel/bpf/devmap.c                     |  325 ++++++++++++++++++++++++++-----
>  kernel/bpf/verifier.c                   |    2
>  net/core/filter.c                       |    9 +
>  tools/bpf/bpftool/map.c                 |    1
>  tools/include/uapi/linux/bpf.h          |    1
>  tools/lib/bpf/libbpf_probes.c           |    1
>  tools/testing/selftests/bpf/test_maps.c |   16 ++
>  11 files changed, 310 insertions(+), 61 deletions(-)
>