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

[Martin KaFai Lau <martin.lau@linux.dev>]

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__)