[PATCH bpf-next v2 0/2] Cgroup skb add helper to get net_cls's classid

[PATCH bpf-next v2 0/2] Cgroup skb add helper to get net_cls's classid

[Feng zhou]

From: Feng Zhou <zhoufeng.zf@bytedance.com>

0001: Cgroup skb add bpf_skb_cgroup_classid_proto.
0002: Add a testcase for it.

Feng Zhou (2):
  bpf: cg_skb add get classid helper
  bpf, selftests: Add test case for cgroup skb to get net_cls classid
    helpers

Changelog:
v1->v2: Addressed comments from Martin KaFai Lau
- Just bpf_skb_cgroup_classid_proto.
- Add a testcase.
Details in here:
https://lore.kernel.org/lkml/20240814095038.64523-1-zhoufeng.zf@bytedance.com/T/

 net/core/filter.c                             |  4 +
 .../bpf/prog_tests/cg_skb_get_classid.c       | 87 +++++++++++++++++++
 .../selftests/bpf/progs/cg_skb_get_classid.c  | 19 ++++
 3 files changed, 110 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
 create mode 100644 tools/testing/selftests/bpf/progs/cg_skb_get_classid.c

-- 
2.30.2

[PATCH bpf-next v2 1/2] bpf: cg_skb add get classid helper

[Feng zhou]

From: Feng Zhou <zhoufeng.zf@bytedance.com>

At cg_skb hook point, can get classid for v1 or v2, allowing
users to do more functions such as acl.

Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
 net/core/filter.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index e4a4454df5f9..a4aa39b6dbba 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -8115,6 +8115,10 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_listener_sock_proto;
 	case BPF_FUNC_skb_ecn_set_ce:
 		return &bpf_skb_ecn_set_ce_proto;
+#endif
+#ifdef CONFIG_CGROUP_NET_CLASSID
+	case BPF_FUNC_skb_cgroup_classid:
+		return &bpf_skb_cgroup_classid_proto;
 #endif
 	default:
 		return sk_filter_func_proto(func_id, prog);
-- 
2.30.2

[PATCH bpf-next v2 2/2] bpf, selftests: Add test case for cgroup skb to get net_cls classid helpers

[Feng zhou]

From: Feng Zhou <zhoufeng.zf@bytedance.com>

This patch adds a test for cgroup skb to get classid.

Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
 .../bpf/prog_tests/cg_skb_get_classid.c       | 87 +++++++++++++++++++
 .../selftests/bpf/progs/cg_skb_get_classid.c  | 19 ++++
 2 files changed, 106 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
 create mode 100644 tools/testing/selftests/bpf/progs/cg_skb_get_classid.c

diff --git a/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c b/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
new file mode 100644
index 000000000000..13a5943c387d
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright 2024 Bytedance.
+ */
+
+#include <test_progs.h>
+
+#include "cg_skb_get_classid.skel.h"
+
+#include "cgroup_helpers.h"
+#include "network_helpers.h"
+
+static int run_test(int cgroup_fd, int server_fd)
+{
+	struct cg_skb_get_classid *skel;
+	int fd, err = 0;
+
+	skel = cg_skb_get_classid__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return -1;
+
+	skel->links.cg_skb_classid =
+		bpf_program__attach_cgroup(skel->progs.cg_skb_classid,
+					   cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links.cg_skb_classid, "prog_attach")) {
+		err = -1;
+		goto out;
+	}
+
+	if (!ASSERT_OK(join_classid(), "join_classid")) {
+		err = -1;
+		goto out;
+	}
+
+	errno = 0;
+	fd = connect_to_fd_opts(server_fd, NULL);
+	if (fd >= 0) {
+		if (skel->bss->classid != getpid()) {
+			log_err("Get unexpected classid");
+			err = -1;
+		}
+
+		close(fd);
+	} else {
+		log_err("Unexpected errno from connect to server");
+		err = -1;
+	}
+out:
+	cg_skb_get_classid__destroy(skel);
+	return err;
+}
+
+void test_cg_skb_get_classid(void)
+{
+	struct network_helper_opts opts = {};
+	int server_fd, client_fd, cgroup_fd;
+	static const int port = 60120;
+
+	/* Step 1: Check base connectivity works without any BPF. */
+	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
+	if (!ASSERT_GE(server_fd, 0, "server_fd"))
+		return;
+	client_fd = connect_to_fd_opts(server_fd, &opts);
+	if (!ASSERT_GE(client_fd, 0, "client_fd")) {
+		close(server_fd);
+		return;
+	}
+	close(client_fd);
+	close(server_fd);
+
+	/* Step 2: Check BPF prog attached to cgroups. */
+	cgroup_fd = test__join_cgroup("/cg_skb_get_classid");
+	if (!ASSERT_GE(cgroup_fd, 0, "cgroup_fd"))
+		return;
+	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
+	if (!ASSERT_GE(server_fd, 0, "server_fd")) {
+		close(cgroup_fd);
+		return;
+	}
+	setup_classid_environment();
+	set_classid();
+	ASSERT_OK(run_test(cgroup_fd, server_fd), "cg_skb_get_classid");
+	cleanup_classid_environment();
+	close(server_fd);
+	close(cgroup_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/cg_skb_get_classid.c b/tools/testing/selftests/bpf/progs/cg_skb_get_classid.c
new file mode 100644
index 000000000000..aef0265d24eb
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cg_skb_get_classid.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright 2024 Bytedance.
+ */
+
+#include <errno.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+__u32 classid = 0;
+
+SEC("cgroup_skb/egress")
+int cg_skb_classid(struct __sk_buff *ctx)
+{
+	classid = bpf_skb_cgroup_classid(ctx);
+
+	return 1;
+}
-- 
2.30.2

Re: [PATCH bpf-next v2 2/2] bpf, selftests: Add test case for cgroup skb to get net_cls classid helpers

[Martin KaFai Lau]

On 9/18/24 12:45 AM, Feng zhou wrote:
> From: Feng Zhou <zhoufeng.zf@bytedance.com>
> 
> This patch adds a test for cgroup skb to get classid.
> 
> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
> ---
>   .../bpf/prog_tests/cg_skb_get_classid.c       | 87 +++++++++++++++++++
>   .../selftests/bpf/progs/cg_skb_get_classid.c  | 19 ++++
>   2 files changed, 106 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
>   create mode 100644 tools/testing/selftests/bpf/progs/cg_skb_get_classid.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c b/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
> new file mode 100644
> index 000000000000..13a5943c387d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright 2024 Bytedance.
> + */
> +
> +#include <test_progs.h>
> +
> +#include "cg_skb_get_classid.skel.h"
> +
> +#include "cgroup_helpers.h"
> +#include "network_helpers.h"
> +
> +static int run_test(int cgroup_fd, int server_fd)
> +{
> +	struct cg_skb_get_classid *skel;
> +	int fd, err = 0;
> +
> +	skel = cg_skb_get_classid__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return -1;
> +
> +	skel->links.cg_skb_classid =
> +		bpf_program__attach_cgroup(skel->progs.cg_skb_classid,
> +					   cgroup_fd);
> +	if (!ASSERT_OK_PTR(skel->links.cg_skb_classid, "prog_attach")) {
> +		err = -1;
> +		goto out;
> +	}
> +
> +	if (!ASSERT_OK(join_classid(), "join_classid")) {
> +		err = -1;
> +		goto out;
> +	}
> +
> +	errno = 0;
> +	fd = connect_to_fd_opts(server_fd, NULL);
> +	if (fd >= 0) {
> +		if (skel->bss->classid != getpid()) {
> +			log_err("Get unexpected classid");
> +			err = -1;
> +		}
> +
> +		close(fd);
> +	} else {
> +		log_err("Unexpected errno from connect to server");
> +		err = -1;
> +	}
> +out:
> +	cg_skb_get_classid__destroy(skel);
> +	return err;
> +}
> +
> +void test_cg_skb_get_classid(void)
> +{
> +	struct network_helper_opts opts = {};
> +	int server_fd, client_fd, cgroup_fd;
> +	static const int port = 60120;

Running a test with a specific port without netns could fail when test_progs is 
run in parallel (-j). e.g. cgroup_v1v2 is using the same port.

> +
> +	/* Step 1: Check base connectivity works without any BPF. */
> +	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
> +	if (!ASSERT_GE(server_fd, 0, "server_fd"))
> +		return;
> +	client_fd = connect_to_fd_opts(server_fd, &opts);
> +	if (!ASSERT_GE(client_fd, 0, "client_fd")) {
> +		close(server_fd);
> +		return;
> +	}
> +	close(client_fd);
> +	close(server_fd);

imo, this connection pre-test is unnecessary. I would remove it.

> +
> +	/* Step 2: Check BPF prog attached to cgroups. */
> +	cgroup_fd = test__join_cgroup("/cg_skb_get_classid");
> +	if (!ASSERT_GE(cgroup_fd, 0, "cgroup_fd"))
> +		return;
> +	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
> +	if (!ASSERT_GE(server_fd, 0, "server_fd")) {
> +		close(cgroup_fd);
> +		return;
> +	}
> +	setup_classid_environment();
> +	set_classid();
> +	ASSERT_OK(run_test(cgroup_fd, server_fd), "cg_skb_get_classid");

Please run this test under a netns and without specifying a particular port. 
connect_to_fd_opts will figure out the port used in server_fd.

Patch 1 lgtm.

Please add a few words to the cover letter also.

pw-bot: cr

Re: [External] Re: [PATCH bpf-next v2 2/2] bpf, selftests: Add test case for cgroup skb to get net_cls classid helpers

[Feng Zhou]

在 2024/10/1 09:58, Martin KaFai Lau 写道:
> On 9/18/24 12:45 AM, Feng zhou wrote:
>> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>>
>> This patch adds a test for cgroup skb to get classid.
>>
>> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>> ---
>>   .../bpf/prog_tests/cg_skb_get_classid.c       | 87 +++++++++++++++++++
>>   .../selftests/bpf/progs/cg_skb_get_classid.c  | 19 ++++
>>   2 files changed, 106 insertions(+)
>>   create mode 100644 
>> tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
>>   create mode 100644 
>> tools/testing/selftests/bpf/progs/cg_skb_get_classid.c
>>
>> diff --git 
>> a/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c 
>> b/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
>> new file mode 100644
>> index 000000000000..13a5943c387d
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/cg_skb_get_classid.c
>> @@ -0,0 +1,87 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/*
>> + * Copyright 2024 Bytedance.
>> + */
>> +
>> +#include <test_progs.h>
>> +
>> +#include "cg_skb_get_classid.skel.h"
>> +
>> +#include "cgroup_helpers.h"
>> +#include "network_helpers.h"
>> +
>> +static int run_test(int cgroup_fd, int server_fd)
>> +{
>> +    struct cg_skb_get_classid *skel;
>> +    int fd, err = 0;
>> +
>> +    skel = cg_skb_get_classid__open_and_load();
>> +    if (!ASSERT_OK_PTR(skel, "skel_open"))
>> +        return -1;
>> +
>> +    skel->links.cg_skb_classid =
>> +        bpf_program__attach_cgroup(skel->progs.cg_skb_classid,
>> +                       cgroup_fd);
>> +    if (!ASSERT_OK_PTR(skel->links.cg_skb_classid, "prog_attach")) {
>> +        err = -1;
>> +        goto out;
>> +    }
>> +
>> +    if (!ASSERT_OK(join_classid(), "join_classid")) {
>> +        err = -1;
>> +        goto out;
>> +    }
>> +
>> +    errno = 0;
>> +    fd = connect_to_fd_opts(server_fd, NULL);
>> +    if (fd >= 0) {
>> +        if (skel->bss->classid != getpid()) {
>> +            log_err("Get unexpected classid");
>> +            err = -1;
>> +        }
>> +
>> +        close(fd);
>> +    } else {
>> +        log_err("Unexpected errno from connect to server");
>> +        err = -1;
>> +    }
>> +out:
>> +    cg_skb_get_classid__destroy(skel);
>> +    return err;
>> +}
>> +
>> +void test_cg_skb_get_classid(void)
>> +{
>> +    struct network_helper_opts opts = {};
>> +    int server_fd, client_fd, cgroup_fd;
>> +    static const int port = 60120;
> 
> Running a test with a specific port without netns could fail when 
> test_progs is run in parallel (-j). e.g. cgroup_v1v2 is using the same 
> port.
> 
>> +
>> +    /* Step 1: Check base connectivity works without any BPF. */
>> +    server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
>> +    if (!ASSERT_GE(server_fd, 0, "server_fd"))
>> +        return;
>> +    client_fd = connect_to_fd_opts(server_fd, &opts);
>> +    if (!ASSERT_GE(client_fd, 0, "client_fd")) {
>> +        close(server_fd);
>> +        return;
>> +    }
>> +    close(client_fd);
>> +    close(server_fd);
> 
> imo, this connection pre-test is unnecessary. I would remove it.
> 
>> +
>> +    /* Step 2: Check BPF prog attached to cgroups. */
>> +    cgroup_fd = test__join_cgroup("/cg_skb_get_classid");
>> +    if (!ASSERT_GE(cgroup_fd, 0, "cgroup_fd"))
>> +        return;
>> +    server_fd = start_server(AF_INET, SOCK_STREAM, NULL, port, 0);
>> +    if (!ASSERT_GE(server_fd, 0, "server_fd")) {
>> +        close(cgroup_fd);
>> +        return;
>> +    }
>> +    setup_classid_environment();
>> +    set_classid();
>> +    ASSERT_OK(run_test(cgroup_fd, server_fd), "cg_skb_get_classid");
> 
> Please run this test under a netns and without specifying a particular 
> port. connect_to_fd_opts will figure out the port used in server_fd.
> 
> Patch 1 lgtm.
> 
> Please add a few words to the cover letter also.
> 
> pw-bot: cr

Sorry for taking so long to reply.

Will do, thanks.