[bpf-next,v4,3/6] selftests/bpf: netns_new() and netns_free() helpers.

Commit Message

Kui-Feng Lee July 31, 2024, 7:31 p.m. UTC

netns_new()/netns_free() create/delete network namespaces. They support the
option '-m' of test_progs to start/stop traffic monitor for the network
namespace being created for matched tests.

Signed-off-by: Kui-Feng Lee <[e-mail]>
---
 tools/testing/selftests/bpf/network_helpers.c | 26 ++++++
 tools/testing/selftests/bpf/network_helpers.h |  2 +
 tools/testing/selftests/bpf/test_progs.c      | 80 +++++++++++++++++++
 tools/testing/selftests/bpf/test_progs.h      |  4 +
 4 files changed, 112 insertions(+)

Comments

Stanislav Fomichev July 31, 2024, 9:02 p.m. UTC | #1

On 07/31, Kui-Feng Lee wrote:
> netns_new()/netns_free() create/delete network namespaces. They support the
> option '-m' of test_progs to start/stop traffic monitor for the network
> namespace being created for matched tests.
> 
> Signed-off-by: Kui-Feng Lee <[e-mail]>
> ---
>  tools/testing/selftests/bpf/network_helpers.c | 26 ++++++
>  tools/testing/selftests/bpf/network_helpers.h |  2 +
>  tools/testing/selftests/bpf/test_progs.c      | 80 +++++++++++++++++++
>  tools/testing/selftests/bpf/test_progs.h      |  4 +
>  4 files changed, 112 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index a3f0a49fb26f..f2cf43382a8e 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -432,6 +432,32 @@ char *ping_command(int family)
>  	return "ping";
>  }
>  
> +int make_netns(const char *name)
> +{

[..]

> +	char cmd[128];
> +	int r;
> +
> +	snprintf(cmd, sizeof(cmd), "ip netns add %s", name);
> +	r = system(cmd);

I doubt that we're gonna see any real problems with that in the tests,
but maybe easier to use apsrint and avoid dealing with fixed 128-byte
string?

> +	if (r > 0)
> +		/* exit code */
> +		return -r;
> +	return r;
> +}
> +
> +int remove_netns(const char *name)
> +{
> +	char cmd[128];
> +	int r;
> +
> +	snprintf(cmd, sizeof(cmd), "ip netns del %s >/dev/null 2>&1", name);
> +	r = system(cmd);
> +	if (r > 0)
> +		/* exit code */
> +		return -r;
> +	return r;
> +}
> +
>  struct nstoken {
>  	int orig_netns_fd;
>  };
> diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
> index cce56955371f..f8aa8680a640 100644
> --- a/tools/testing/selftests/bpf/network_helpers.h
> +++ b/tools/testing/selftests/bpf/network_helpers.h
> @@ -93,6 +93,8 @@ struct nstoken;
>  struct nstoken *open_netns(const char *name);
>  void close_netns(struct nstoken *token);
>  int send_recv_data(int lfd, int fd, uint32_t total_bytes);
> +int make_netns(const char *name);
> +int remove_netns(const char *name);
>  
>  static __u16 csum_fold(__u32 csum)
>  {
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 95643cd3119a..f86d47efe06e 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1074,6 +1074,86 @@ int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len)
>  	return err;
>  }
>  
> +struct netns_obj {
> +	char nsname[128];
> +	struct tmonitor_ctx *tmon;
> +	struct nstoken *nstoken;
> +};
> +
> +/* Create a new network namespace with the given name.
> + *
> + * Create a new network namespace and set the network namespace of the
> + * current process to the new network namespace if the argument "open" is
> + * true. This function should be paired with netns_free() to release the
> + * resource and delete the network namespace.
> + *
> + * It also implements the functionality of the option "-m" by starting
> + * traffic monitor on the background to capture the packets in this network
> + * namespace if the current test or subtest matching the pattern.
> + *
> + * name: the name of the network namespace to create.
> + * open: open the network namespace if true.
> + *
> + * Return: the network namespace object on success, NULL on failure.
> + */
> +struct netns_obj *netns_new(const char *name, bool open)
> +{
> +	struct netns_obj *netns_obj = malloc(sizeof(*netns_obj));
> +	int r;
> +
> +	if (!netns_obj)
> +		return NULL;
> +	memset(netns_obj, 0, sizeof(*netns_obj));
> +
> +	strncpy(netns_obj->nsname, name, sizeof(netns_obj->nsname));
> +	netns_obj->nsname[sizeof(netns_obj->nsname) - 1] = '\0';

Same here. Seems easier to have "char *nsname" and do
netns_obj->nsname = strdup(name) here. Trimming the name, in theory,
is problematic because do do remove_netns(netns_obj->nsname) later
on (with potentially trimmed name).

But, again, probably not a huge deal in the selftests. So up to you on
whether you want to address it or not.

Kui-Feng Lee Aug. 2, 2024, 4:42 a.m. UTC | #2

On 7/31/24 14:02, Stanislav Fomichev wrote:
> On 07/31, Kui-Feng Lee wrote:
>> netns_new()/netns_free() create/delete network namespaces. They support the
>> option '-m' of test_progs to start/stop traffic monitor for the network
>> namespace being created for matched tests.
>>
>> Signed-off-by: Kui-Feng Lee <[e-mail]>
>> ---
>>   tools/testing/selftests/bpf/network_helpers.c | 26 ++++++
>>   tools/testing/selftests/bpf/network_helpers.h |  2 +
>>   tools/testing/selftests/bpf/test_progs.c      | 80 +++++++++++++++++++
>>   tools/testing/selftests/bpf/test_progs.h      |  4 +
>>   4 files changed, 112 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
>> index a3f0a49fb26f..f2cf43382a8e 100644
>> --- a/tools/testing/selftests/bpf/network_helpers.c
>> +++ b/tools/testing/selftests/bpf/network_helpers.c
>> @@ -432,6 +432,32 @@ char *ping_command(int family)
>>   	return "ping";
>>   }
>>   
>> +int make_netns(const char *name)
>> +{
> 
> [..]
> 
>> +	char cmd[128];
>> +	int r;
>> +
>> +	snprintf(cmd, sizeof(cmd), "ip netns add %s", name);
>> +	r = system(cmd);
> 
> I doubt that we're gonna see any real problems with that in the tests,
> but maybe easier to use apsrint and avoid dealing with fixed 128-byte
> string?

asprintf? Sure!

> 
>> +	if (r > 0)
>> +		/* exit code */
>> +		return -r;
>> +	return r;
>> +}
>> +
>> +int remove_netns(const char *name)
>> +{
>> +	char cmd[128];
>> +	int r;
>> +
>> +	snprintf(cmd, sizeof(cmd), "ip netns del %s >/dev/null 2>&1", name);
>> +	r = system(cmd);
>> +	if (r > 0)
>> +		/* exit code */
>> +		return -r;
>> +	return r;
>> +}
>> +
>>   struct nstoken {
>>   	int orig_netns_fd;
>>   };
>> diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
>> index cce56955371f..f8aa8680a640 100644
>> --- a/tools/testing/selftests/bpf/network_helpers.h
>> +++ b/tools/testing/selftests/bpf/network_helpers.h
>> @@ -93,6 +93,8 @@ struct nstoken;
>>   struct nstoken *open_netns(const char *name);
>>   void close_netns(struct nstoken *token);
>>   int send_recv_data(int lfd, int fd, uint32_t total_bytes);
>> +int make_netns(const char *name);
>> +int remove_netns(const char *name);
>>   
>>   static __u16 csum_fold(__u32 csum)
>>   {
>> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
>> index 95643cd3119a..f86d47efe06e 100644
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
>> @@ -1074,6 +1074,86 @@ int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len)
>>   	return err;
>>   }
>>   
>> +struct netns_obj {
>> +	char nsname[128];
>> +	struct tmonitor_ctx *tmon;
>> +	struct nstoken *nstoken;
>> +};
>> +
>> +/* Create a new network namespace with the given name.
>> + *
>> + * Create a new network namespace and set the network namespace of the
>> + * current process to the new network namespace if the argument "open" is
>> + * true. This function should be paired with netns_free() to release the
>> + * resource and delete the network namespace.
>> + *
>> + * It also implements the functionality of the option "-m" by starting
>> + * traffic monitor on the background to capture the packets in this network
>> + * namespace if the current test or subtest matching the pattern.
>> + *
>> + * name: the name of the network namespace to create.
>> + * open: open the network namespace if true.
>> + *
>> + * Return: the network namespace object on success, NULL on failure.
>> + */
>> +struct netns_obj *netns_new(const char *name, bool open)
>> +{
>> +	struct netns_obj *netns_obj = malloc(sizeof(*netns_obj));
>> +	int r;
>> +
>> +	if (!netns_obj)
>> +		return NULL;
>> +	memset(netns_obj, 0, sizeof(*netns_obj));
>> +
>> +	strncpy(netns_obj->nsname, name, sizeof(netns_obj->nsname));
>> +	netns_obj->nsname[sizeof(netns_obj->nsname) - 1] = '\0';
> 
> Same here. Seems easier to have "char *nsname" and do
> netns_obj->nsname = strdup(name) here. Trimming the name, in theory,
> is problematic because do do remove_netns(netns_obj->nsname) later
> on (with potentially trimmed name).

You are right!

> 
> But, again, probably not a huge deal in the selftests. So up to you on
> whether you want to address it or not.

Patch

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index a3f0a49fb26f..f2cf43382a8e 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -432,6 +432,32 @@  char *ping_command(int family)
 	return "ping";
 }
 
+int make_netns(const char *name)
+{
+	char cmd[128];
+	int r;
+
+	snprintf(cmd, sizeof(cmd), "ip netns add %s", name);
+	r = system(cmd);
+	if (r > 0)
+		/* exit code */
+		return -r;
+	return r;
+}
+
+int remove_netns(const char *name)
+{
+	char cmd[128];
+	int r;
+
+	snprintf(cmd, sizeof(cmd), "ip netns del %s >/dev/null 2>&1", name);
+	r = system(cmd);
+	if (r > 0)
+		/* exit code */
+		return -r;
+	return r;
+}
+
 struct nstoken {
 	int orig_netns_fd;
 };
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index cce56955371f..f8aa8680a640 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -93,6 +93,8 @@  struct nstoken;
 struct nstoken *open_netns(const char *name);
 void close_netns(struct nstoken *token);
 int send_recv_data(int lfd, int fd, uint32_t total_bytes);
+int make_netns(const char *name);
+int remove_netns(const char *name);
 
 static __u16 csum_fold(__u32 csum)
 {
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 95643cd3119a..f86d47efe06e 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1074,6 +1074,86 @@  int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len)
 	return err;
 }
 
+struct netns_obj {
+	char nsname[128];
+	struct tmonitor_ctx *tmon;
+	struct nstoken *nstoken;
+};
+
+/* Create a new network namespace with the given name.
+ *
+ * Create a new network namespace and set the network namespace of the
+ * current process to the new network namespace if the argument "open" is
+ * true. This function should be paired with netns_free() to release the
+ * resource and delete the network namespace.
+ *
+ * It also implements the functionality of the option "-m" by starting
+ * traffic monitor on the background to capture the packets in this network
+ * namespace if the current test or subtest matching the pattern.
+ *
+ * name: the name of the network namespace to create.
+ * open: open the network namespace if true.
+ *
+ * Return: the network namespace object on success, NULL on failure.
+ */
+struct netns_obj *netns_new(const char *name, bool open)
+{
+	struct netns_obj *netns_obj = malloc(sizeof(*netns_obj));
+	int r;
+
+	if (!netns_obj)
+		return NULL;
+	memset(netns_obj, 0, sizeof(*netns_obj));
+
+	strncpy(netns_obj->nsname, name, sizeof(netns_obj->nsname));
+	netns_obj->nsname[sizeof(netns_obj->nsname) - 1] = '\0';
+
+	/* Create the network namespace */
+	r = make_netns(name);
+	if (r)
+		goto fail;
+
+	/* Set the network namespace of the current process */
+	if (open) {
+		netns_obj->nstoken = open_netns(name);
+		if (!netns_obj->nstoken)
+			goto fail;
+	}
+
+	/* Start traffic monitor */
+	if (env.test->should_tmon ||
+	    (env.subtest_state && env.subtest_state->should_tmon)) {
+		netns_obj->tmon = traffic_monitor_start(name);
+		if (!netns_obj->tmon)
+			goto fail;
+	} else {
+		netns_obj->tmon = NULL;
+	}
+
+	return netns_obj;
+fail:
+	close_netns(netns_obj->nstoken);
+	remove_netns(name);
+	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);
+}
+
 /* 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 ceda86a5a524..e025ac6f5a8d 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__)