Re: [PATCH bpf-next v6 3/6] selftests/bpf: netns_new() and netns_free() helpers.

Martin KaFai Lau
2024-08-08T20:28:02Z

On 8/7/24 11:31 AM, Kui-Feng Lee wrote:
> +struct netns_obj *netns_new(const char *nsname, bool open)
> +{
> + struct netns_obj *netns_obj = malloc(sizeof(*netns_obj));
> + const char *test_name, *subtest_name;
> + int r;
> +
> + if (!netns_obj)
> + return NULL;
> + memset(netns_obj, 0, sizeof(*netns_obj));
> +
> + netns_obj->nsname = strdup(nsname);
> + if (!netns_obj->nsname)
> + goto fail;
> +
> + /* Create the network namespace */
> + r = make_netns(nsname);
> + if (r)
> + goto fail;
> +
> + /* Set the network namespace of the current process */
> + if (open) {
> + netns_obj->nstoken = open_netns(nsname);
> + if (!netns_obj->nstoken)
> + goto fail;
> + }
> +
> + /* Start traffic monitor */
> + if (env.test->should_tmon ||
> + (env.subtest_state && env.subtest_state->should_tmon)) {
> + test_name = env.test->test_name;
> + subtest_name = env.subtest_state ? env.subtest_state->name : NULL;
> + netns_obj->tmon = traffic_monitor_start(nsname, test_name, subtest_name);

The traffic_monitor_start() does open/close_netns(). close_netns() will restore
to the previous netns. Is it better to do traffic_monitor_start() before the
above open_netns() such that we don't have to worry about the stacking
open_netns and which netns the close_netns will restore?

> + if (!netns_obj->tmon)
> + fprintf(stderr, "Failed to start traffic monitor for %s\n", nsname);
> + } else {
> + netns_obj->tmon = NULL;
> + }
> +
> + system("ip link set lo up");

The "bool open" could be false here. This command could be acted on the
init_netns and the intention is to set lo up at the newly created netns.

> +
> + return netns_obj;
> +fail:
> + close_netns(netns_obj->nstoken);
> + remove_netns(nsname);
> + free(netns_obj->nsname);
> + free(netns_obj);
> + return NULL;
> +}
> +
> +/* Delete the network namespace.
> + *
> + * This function should be paired with netns_new() to delete the namespace
> + * created by netns_new().
> + */
> +void netns_free(struct netns_obj *netns_obj)
> +{
> + if (!netns_obj)
> + return;
> + if (netns_obj->tmon)
> + traffic_monitor_stop(netns_obj->tmon);
> + close_netns(netns_obj->nstoken);
> + remove_netns(netns_obj->nsname);
> + free(netns_obj->nsname);
> + free(netns_obj);
> +}
> +
> /* extern declarations for test funcs */
> #define DEFINE_TEST(name) \
> extern void test_##name(void) __weak; \
> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> index 966011eb7ec8..3ad131de14c6 100644
> --- a/tools/testing/selftests/bpf/test_progs.h
> +++ b/tools/testing/selftests/bpf/test_progs.h
> @@ -430,6 +430,10 @@ int write_sysctl(const char *sysctl, const char *value);
> int get_bpf_max_tramp_links_from(struct btf *btf);
> int get_bpf_max_tramp_links(void);
>
> +struct netns_obj;
> +struct netns_obj *netns_new(const char *name, bool open);
> +void netns_free(struct netns_obj *netns);
> +
> #ifdef __x86_64__
> #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep"
> #elif defined(__s390x__)