netdev - Re: [PATCH bpf-next v2 11/12] selftests/bpf: Convert test_flow

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: <87d06h3imy.fsf@cloudflare.com>
Date:   Tue, 02 Jun 2020 11:46:13 +0200
From:   Jakub Sitnicki <jakub@...udflare.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        kernel-team@...udflare.com
Subject: Re: [PATCH bpf-next v2 11/12] selftests/bpf: Convert test_flow_dissector to use BPF skeleton

On Tue, Jun 02, 2020 at 12:42 AM CEST, Andrii Nakryiko wrote:
> On Sun, May 31, 2020 at 1:29 AM Jakub Sitnicki <jakub@...udflare.com> wrote:
>>
>> Switch flow dissector test setup from custom BPF object loader to BPF
>> skeleton to save boilerplate and prepare for testing higher-level API for
>> attaching flow dissector with bpf_link.
>>
>> To avoid depending on program order in the BPF object when populating the
>> flow dissector PROG_ARRAY map, change the program section names to contain
>> the program index into the map. This follows the example set by tailcall
>> tests.
>>
>> Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
>> ---
>>  .../selftests/bpf/prog_tests/flow_dissector.c | 50 +++++++++++++++++--
>>  tools/testing/selftests/bpf/progs/bpf_flow.c  | 20 ++++----
>>  2 files changed, 55 insertions(+), 15 deletions(-)
>>
>
> [...]
>
>> diff --git a/tools/testing/selftests/bpf/progs/bpf_flow.c b/tools/testing/selftests/bpf/progs/bpf_flow.c
>> index 9941f0ba471e..de6de9221518 100644
>> --- a/tools/testing/selftests/bpf/progs/bpf_flow.c
>> +++ b/tools/testing/selftests/bpf/progs/bpf_flow.c
>> @@ -20,20 +20,20 @@
>>  #include <bpf/bpf_endian.h>
>>
>>  int _version SEC("version") = 1;
>> -#define PROG(F) SEC(#F) int bpf_func_##F
>> +#define PROG(F) PROG_(F, _##F)
>> +#define PROG_(NUM, NAME) SEC("flow_dissector/"#NUM) int bpf_func##NAME
>>
>>  /* These are the identifiers of the BPF programs that will be used in tail
>>   * calls. Name is limited to 16 characters, with the terminating character and
>>   * bpf_func_ above, we have only 6 to work with, anything after will be cropped.
>>   */
>> -enum {
>> -       IP,
>> -       IPV6,
>> -       IPV6OP, /* Destination/Hop-by-Hop Options IPv6 Extension header */
>> -       IPV6FR, /* Fragmentation IPv6 Extension Header */
>> -       MPLS,
>> -       VLAN,
>> -};
>
> not clear why? just add MAX_PROG after VLAN?

I wanted to change section names to:

  "flow_dissector/0"
  "flow_dissector/1"
  ...

while keeping the corresponding function names as:

  bpf_func_IP
  bpf_func_IPV6
  ...

For that I needed the preprocessor to know the value of the constant.

>
>> +#define IP             0
>> +#define IPV6           1
>> +#define IPV6OP         2 /* Destination/Hop-by-Hop Options IPv6 Ext. Header */
>> +#define IPV6FR         3 /* Fragmentation IPv6 Extension Header */
>> +#define MPLS           4
>> +#define VLAN           5
>> +#define MAX_PROG       6
>>
>>  #define IP_MF          0x2000
>>  #define IP_OFFSET      0x1FFF
>> @@ -59,7 +59,7 @@ struct frag_hdr {
>>
>>  struct {
>>         __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
>> -       __uint(max_entries, 8);
>> +       __uint(max_entries, MAX_PROG);
>>         __uint(key_size, sizeof(__u32));
>>         __uint(value_size, sizeof(__u32));
>>  } jmp_table SEC(".maps");
>> --
>> 2.25.4
>>