From: Kui-Feng Lee <sinquersw@gmail.com>
To: Geliang Tang <geliang@kernel.org>,
Kui-Feng Lee <thinker.li@gmail.com>,
bpf@vger.kernel.org, ast@kernel.org, martin.lau@linux.dev,
song@kernel.org, kernel-team@meta.com, andrii@kernel.org,
sdf@fomichev.me
Cc: kuifeng@meta.com
Subject: Re: [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect.
Date: Tue, 30 Jul 2024 09:33:13 -0700 [thread overview]
Message-ID: <ccd4c20a-84f4-4677-b4e5-c0028e6e6f5a@gmail.com> (raw)
In-Reply-To: <7da1f071b59bebf6583844b79c72271cf4ab958d.camel@kernel.org>
On 7/30/24 02:43, Geliang Tang wrote:
> On Mon, 2024-07-29 at 17:27 -0700, Kui-Feng Lee wrote:
>> Enable traffic monitoring for the test case tc_redirect.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>> .../selftests/bpf/prog_tests/tc_redirect.c | 48 ++++++++++++++---
>> --
>> 1 file changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> index 327d51f59142..46d397c5c79a 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> @@ -68,6 +68,7 @@
>> __FILE__, __LINE__, strerror(errno), ##__VA_ARGS__)
>>
>> static const char * const namespaces[] = {NS_SRC, NS_FWD, NS_DST,
>> NULL};
>> +static struct netns_obj *netns_objs[3];
>>
>> static int write_file(const char *path, const char *newval)
>> {
>> @@ -85,29 +86,52 @@ static int write_file(const char *path, const
>> char *newval)
>> return 0;
>> }
>>
>> -static int netns_setup_namespaces(const char *verb)
>> +enum NETNS_VERB {
>> + NETNS_ADD,
>> + NETNS_DEL,
>> +};
>> +
>> +static int netns_setup_namespaces(enum NETNS_VERB verb)
>> {
>> const char * const *ns = namespaces;
>> - char cmd[128];
>> + struct netns_obj **ns_obj = netns_objs;
>>
>> while (*ns) {
>> - snprintf(cmd, sizeof(cmd), "ip netns %s %s", verb,
>> *ns);
>> - if (!ASSERT_OK(system(cmd), cmd))
>> - return -1;
>> + if (verb == NETNS_ADD) {
>
> Maybe better to keep "verb" parameter as "char *", and use
>
> if (!strcmp(verb, "add"))
>
> here instead?
I have no strong opinion here.
May I know why you think string is better here?
>
>> + *ns_obj = netns_new(*ns, false);
>> + if (!*ns_obj) {
>> + log_err("netns_new failed");
>> + return -1;
>> + }
>> + } else {
>> + if (!*ns_obj) {
>> + log_err("netns_obj is NULL");
>> + return -1;
>> + }
>> + netns_free(*ns_obj);
>> + *ns_obj = NULL;
>> + }
>> ns++;
>> + ns_obj++;
>> }
>> return 0;
>> }
>>
>> -static void netns_setup_namespaces_nofail(const char *verb)
>> +static void netns_setup_namespaces_nofail(enum NETNS_VERB verb)
>> {
>> const char * const *ns = namespaces;
>> - char cmd[128];
>> + struct netns_obj **ns_obj = netns_objs;
>>
>> while (*ns) {
>> - snprintf(cmd, sizeof(cmd), "ip netns %s %s >
>> /dev/null 2>&1", verb, *ns);
>> - system(cmd);
>> + if (verb == NETNS_ADD) {
>> + *ns_obj = netns_new(*ns, false);
>> + } else {
>> + if (*ns_obj)
>> + netns_free(*ns_obj);
>> + *ns_obj = NULL;
>> + }
>> ns++;
>> + ns_obj++;
>> }
>> }
>>
>> @@ -1250,17 +1274,17 @@ static void test_tc_redirect_peer_l3(struct
>> netns_setup_result *setup_result)
>> ({
>> \
>> struct netns_setup_result setup_result = { .dev_mode
>> = mode, }; \
>> if
>> (test__start_subtest(#name))
>> \
>> - if (ASSERT_OK(netns_setup_namespaces("add"),
>> "setup namespaces")) { \
>> + if
>> (ASSERT_OK(netns_setup_namespaces(NETNS_ADD), "setup namespaces")) {
>> \
>> if
>> (ASSERT_OK(netns_setup_links_and_routes(&setup_result), \
>> "setup links and
>> routes")) \
>> test_ ##
>> name(&setup_result); \
>> -
>> netns_setup_namespaces("delete"); \
>> + netns_setup_namespaces(NETNS_DEL);
>> \
>> }
>> \
>> })
>>
>> static void *test_tc_redirect_run_tests(void *arg)
>> {
>> - netns_setup_namespaces_nofail("delete");
>> + netns_setup_namespaces_nofail(NETNS_DEL);
>>
>> RUN_TEST(tc_redirect_peer, MODE_VETH);
>> RUN_TEST(tc_redirect_peer, MODE_NETKIT);
>
next prev parent reply other threads:[~2024-07-30 16:33 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-30 0:27 [PATCH bpf-next v3 0/6] monitor network traffic for flaky test cases Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 1/6] selftests/bpf: Add traffic monitor functions Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 2/6] selftests/bpf: Add the traffic monitor option to test_progs Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 3/6] selftests/bpf: netns_new() and netns_free() helpers Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 4/6] selftests/bpf: Monitor traffic for tc_redirect Kui-Feng Lee
2024-07-30 9:43 ` Geliang Tang
2024-07-30 16:33 ` Kui-Feng Lee [this message]
2024-07-31 1:25 ` Geliang Tang
2024-07-30 0:27 ` [PATCH bpf-next v3 5/6] selftests/bpf: Monitor traffic for sockmap_listen Kui-Feng Lee
2024-07-30 0:27 ` [PATCH bpf-next v3 6/6] selftests/bpf: Monitor traffic for select_reuseport Kui-Feng Lee
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=ccd4c20a-84f4-4677-b4e5-c0028e6e6f5a@gmail.com \
--to=sinquersw@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=geliang@kernel.org \
--cc=kernel-team@meta.com \
--cc=kuifeng@meta.com \
--cc=martin.lau@linux.dev \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=thinker.li@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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