From: "Toke Høiland-Jørgensen" <toke@redhat•com>
To: Andrii Nakryiko <andrii.nakryiko@gmail•com>
Cc: Alexei Starovoitov <ast@kernel•org>,
Daniel Borkmann <daniel@iogearbox•net>,
Andrii Nakryiko <andrii@kernel•org>,
Martin KaFai Lau <kafai@fb•com>, Song Liu <songliubraving@fb•com>,
Yonghong Song <yhs@fb•com>,
John Fastabend <john.fastabend@gmail•com>,
KP Singh <kpsingh@kernel•org>,
Stanislav Fomichev <sdf@google•com>,
"David S. Miller" <davem@davemloft•net>,
Jesper Dangaard Brouer <brouer@redhat•com>,
Andrea Arcangeli <aarcange@redhat•com>,
Clark Williams <williams@redhat•com>, bpf <bpf@vger•kernel.org>,
Networking <netdev@vger•kernel.org>
Subject: Re: [PATCH bpf v2 2/2] bpf/selftests: test that kernel rejects a TCP CC with an invalid license
Date: Fri, 26 Mar 2021 10:43:34 +0100 [thread overview]
Message-ID: <87lfaacks9.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzaxmrWFBJ1mzzWzu0yb_iFX528cAFVbXrncPEaJBXrd2A@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail•com> writes:
> On Thu, Mar 25, 2021 at 2:11 PM Toke Høiland-Jørgensen <toke@redhat•com> wrote:
>>
>> This adds a selftest to check that the verifier rejects a TCP CC struct_ops
>> with a non-GPL license.
>>
>> v2:
>> - Use a minimal struct_ops BPF program instead of rewriting bpf_dctcp's
>> license in memory.
>> - Check for the verifier reject message instead of just the return code.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat•com>
>> ---
>> .../selftests/bpf/prog_tests/bpf_tcp_ca.c | 44 +++++++++++++++++++
>> .../selftests/bpf/progs/bpf_nogpltcp.c | 19 ++++++++
>> 2 files changed, 63 insertions(+)
>> create mode 100644 tools/testing/selftests/bpf/progs/bpf_nogpltcp.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
>> index 37c5494a0381..a09c716528e1 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c
>> @@ -6,6 +6,7 @@
>> #include <test_progs.h>
>> #include "bpf_dctcp.skel.h"
>> #include "bpf_cubic.skel.h"
>> +#include "bpf_nogpltcp.skel.h"
>
> total nit, but my eyes can't read "nogpltcp"... wouldn't
> "bpf_tcp_nogpl" be a bit easier?
Haha, yeah, good point - my eyes also just lump it into a blob...
>>
>> #define min(a, b) ((a) < (b) ? (a) : (b))
>>
>> @@ -227,10 +228,53 @@ static void test_dctcp(void)
>> bpf_dctcp__destroy(dctcp_skel);
>> }
>>
>> +static char *err_str = NULL;
>> +static bool found = false;
>> +
>> +static int libbpf_debug_print(enum libbpf_print_level level,
>> + const char *format, va_list args)
>> +{
>> + char *log_buf;
>> +
>> + if (level != LIBBPF_WARN ||
>> + strcmp(format, "libbpf: \n%s\n")) {
>> + vprintf(format, args);
>> + return 0;
>> + }
>> +
>> + log_buf = va_arg(args, char *);
>> + if (!log_buf)
>> + goto out;
>> + if (err_str && strstr(log_buf, err_str) != NULL)
>> + found = true;
>> +out:
>> + printf(format, log_buf);
>> + return 0;
>> +}
>> +
>> +static void test_invalid_license(void)
>> +{
>> + libbpf_print_fn_t old_print_fn = NULL;
>> + struct bpf_nogpltcp *skel;
>> +
>> + err_str = "struct ops programs must have a GPL compatible license";
>> + old_print_fn = libbpf_set_print(libbpf_debug_print);
>> +
>> + skel = bpf_nogpltcp__open_and_load();
>> + if (CHECK(skel, "bpf_nogplgtcp__open_and_load()", "didn't fail\n"))
>
> ASSERT_OK_PTR()
>
>> + bpf_nogpltcp__destroy(skel);
>
> you should destroy unconditionally
>
>> +
>> + CHECK(!found, "errmsg check", "expected string '%s'", err_str);
>
> ASSERT_EQ(found, true, "expected_err_msg");
>
> I can never be sure which way CHECK() is checking
Ah, thanks! I always get confused about CHECK() as well! Maybe it should
be renamed to ASSERT()? But that would require flipping all the if()
statements around them as well :/
-Toke
next prev parent reply other threads:[~2021-03-26 9:44 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-25 21:11 [PATCH bpf v2 1/2] bpf: enforce that struct_ops programs be GPL-only Toke Høiland-Jørgensen
2021-03-25 21:11 ` [PATCH bpf v2 2/2] bpf/selftests: test that kernel rejects a TCP CC with an invalid license Toke Høiland-Jørgensen
2021-03-25 22:58 ` Martin KaFai Lau
2021-03-26 4:04 ` Andrii Nakryiko
2021-03-26 9:43 ` Toke Høiland-Jørgensen [this message]
2021-03-26 18:14 ` Andrii Nakryiko
2021-03-26 18:25 ` Toke Høiland-Jørgensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87lfaacks9.fsf@toke.dk \
--to=toke@redhat$(echo .)com \
--cc=aarcange@redhat$(echo .)com \
--cc=andrii.nakryiko@gmail$(echo .)com \
--cc=andrii@kernel$(echo .)org \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=brouer@redhat$(echo .)com \
--cc=daniel@iogearbox$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=john.fastabend@gmail$(echo .)com \
--cc=kafai@fb$(echo .)com \
--cc=kpsingh@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sdf@google$(echo .)com \
--cc=songliubraving@fb$(echo .)com \
--cc=williams@redhat$(echo .)com \
--cc=yhs@fb$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox