linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order
@ 2022-08-25 21:39 Hao Luo
  2022-08-25 21:39 ` [PATCH bpf-next 1/2] bpf: Add CGROUP to cgroup_iter order Hao Luo
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Hao Luo @ 2022-08-25 21:39 UTC (permalink / raw)
  To: linux-kernel, bpf, netdev
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	John Fastabend, Jiri Olsa, Stanislav Fomichev, Yosry Ahmed,
	Hao Luo

As suggested by Andrii, add 'CGROUP' to cgroup_iter_order. This fix is
divided into two patches. Patch 1/2 fixes the commit that introduced
cgroup_iter. Patch 2/2 fixes the selftest that uses the
cgroup_iter_order. This is because the selftest was introduced in a
different commit. I tested this patchset via the following command:

  test_progs -t cgroup,iter,btf_dump

Hao Luo (2):
  bpf: Add CGROUP to cgroup_iter order
  selftests/bpf: Fix test that uses cgroup_iter order

 include/uapi/linux/bpf.h                      | 10 +++---
 kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
 tools/include/uapi/linux/bpf.h                | 10 +++---
 .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
 .../prog_tests/cgroup_hierarchical_stats.c    |  2 +-
 .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
 6 files changed, 33 insertions(+), 33 deletions(-)

-- 
2.37.2.672.g94769d06f0-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH bpf-next 1/2] bpf: Add CGROUP to cgroup_iter order
  2022-08-25 21:39 [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Hao Luo
@ 2022-08-25 21:39 ` Hao Luo
  2022-08-25 21:39 ` [PATCH bpf-next 2/2] selftests/bpf: Fix test that uses " Hao Luo
  2022-08-25 21:56 ` [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Andrii Nakryiko
  2 siblings, 0 replies; 8+ messages in thread
From: Hao Luo @ 2022-08-25 21:39 UTC (permalink / raw)
  To: linux-kernel, bpf, netdev
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	John Fastabend, Jiri Olsa, Stanislav Fomichev, Yosry Ahmed,
	Hao Luo

bpf_cgroup_iter_order is global visible but the entries do not have
CGROUP prefix. As requested by Andrii, put a CGROUP in the names.
This patch changes API, the following patch changes the selftest
added in a later commit that uses the API.

Fixes: d4ccaf58a847 ("bpf: Introduce cgroup iter")
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/uapi/linux/bpf.h                      | 10 +++---
 kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
 tools/include/uapi/linux/bpf.h                | 10 +++---
 .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
 .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
 5 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0f61f09f467a..bdf4bc6d8d6b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -88,11 +88,11 @@ struct bpf_cgroup_storage_key {
 };
 
 enum bpf_cgroup_iter_order {
-	BPF_ITER_ORDER_UNSPEC = 0,
-	BPF_ITER_SELF_ONLY,		/* process only a single object. */
-	BPF_ITER_DESCENDANTS_PRE,	/* walk descendants in pre-order. */
-	BPF_ITER_DESCENDANTS_POST,	/* walk descendants in post-order. */
-	BPF_ITER_ANCESTORS_UP,		/* walk ancestors upward. */
+	BPF_CGROUP_ITER_ORDER_UNSPEC = 0,
+	BPF_CGROUP_ITER_SELF_ONLY,		/* process only a single object. */
+	BPF_CGROUP_ITER_DESCENDANTS_PRE,	/* walk descendants in pre-order. */
+	BPF_CGROUP_ITER_DESCENDANTS_POST,	/* walk descendants in post-order. */
+	BPF_CGROUP_ITER_ANCESTORS_UP,		/* walk ancestors upward. */
 };
 
 union bpf_iter_link_info {
diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
index cf6d763a57d5..c69bce2f4403 100644
--- a/kernel/bpf/cgroup_iter.c
+++ b/kernel/bpf/cgroup_iter.c
@@ -74,13 +74,13 @@ static void *cgroup_iter_seq_start(struct seq_file *seq, loff_t *pos)
 	++*pos;
 	p->terminate = false;
 	p->visited_all = false;
-	if (p->order == BPF_ITER_DESCENDANTS_PRE)
+	if (p->order == BPF_CGROUP_ITER_DESCENDANTS_PRE)
 		return css_next_descendant_pre(NULL, p->start_css);
-	else if (p->order == BPF_ITER_DESCENDANTS_POST)
+	else if (p->order == BPF_CGROUP_ITER_DESCENDANTS_POST)
 		return css_next_descendant_post(NULL, p->start_css);
-	else if (p->order == BPF_ITER_ANCESTORS_UP)
+	else if (p->order == BPF_CGROUP_ITER_ANCESTORS_UP)
 		return p->start_css;
-	else /* BPF_ITER_SELF_ONLY */
+	else /* BPF_CGROUP_ITER_SELF_ONLY */
 		return p->start_css;
 }
 
@@ -109,13 +109,13 @@ static void *cgroup_iter_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 	if (p->terminate)
 		return NULL;
 
-	if (p->order == BPF_ITER_DESCENDANTS_PRE)
+	if (p->order == BPF_CGROUP_ITER_DESCENDANTS_PRE)
 		return css_next_descendant_pre(curr, p->start_css);
-	else if (p->order == BPF_ITER_DESCENDANTS_POST)
+	else if (p->order == BPF_CGROUP_ITER_DESCENDANTS_POST)
 		return css_next_descendant_post(curr, p->start_css);
-	else if (p->order == BPF_ITER_ANCESTORS_UP)
+	else if (p->order == BPF_CGROUP_ITER_ANCESTORS_UP)
 		return curr->parent;
-	else  /* BPF_ITER_SELF_ONLY */
+	else  /* BPF_CGROUP_ITER_SELF_ONLY */
 		return NULL;
 }
 
@@ -188,10 +188,10 @@ static int bpf_iter_attach_cgroup(struct bpf_prog *prog,
 	int order = linfo->cgroup.order;
 	struct cgroup *cgrp;
 
-	if (order != BPF_ITER_DESCENDANTS_PRE &&
-	    order != BPF_ITER_DESCENDANTS_POST &&
-	    order != BPF_ITER_ANCESTORS_UP &&
-	    order != BPF_ITER_SELF_ONLY)
+	if (order != BPF_CGROUP_ITER_DESCENDANTS_PRE &&
+	    order != BPF_CGROUP_ITER_DESCENDANTS_POST &&
+	    order != BPF_CGROUP_ITER_ANCESTORS_UP &&
+	    order != BPF_CGROUP_ITER_SELF_ONLY)
 		return -EINVAL;
 
 	if (fd && id)
@@ -239,13 +239,13 @@ static void bpf_iter_cgroup_show_fdinfo(const struct bpf_iter_aux_info *aux,
 	kfree(buf);
 
 show_order:
-	if (aux->cgroup.order == BPF_ITER_DESCENDANTS_PRE)
+	if (aux->cgroup.order == BPF_CGROUP_ITER_DESCENDANTS_PRE)
 		seq_puts(seq, "order: descendants_pre\n");
-	else if (aux->cgroup.order == BPF_ITER_DESCENDANTS_POST)
+	else if (aux->cgroup.order == BPF_CGROUP_ITER_DESCENDANTS_POST)
 		seq_puts(seq, "order: descendants_post\n");
-	else if (aux->cgroup.order == BPF_ITER_ANCESTORS_UP)
+	else if (aux->cgroup.order == BPF_CGROUP_ITER_ANCESTORS_UP)
 		seq_puts(seq, "order: ancestors_up\n");
-	else /* BPF_ITER_SELF_ONLY */
+	else /* BPF_CGROUP_ITER_SELF_ONLY */
 		seq_puts(seq, "order: self_only\n");
 }
 
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 5056cef2112f..92f7387e378a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -88,11 +88,11 @@ struct bpf_cgroup_storage_key {
 };
 
 enum bpf_cgroup_iter_order {
-	BPF_ITER_ORDER_UNSPEC = 0,
-	BPF_ITER_SELF_ONLY,		/* process only a single object. */
-	BPF_ITER_DESCENDANTS_PRE,	/* walk descendants in pre-order. */
-	BPF_ITER_DESCENDANTS_POST,	/* walk descendants in post-order. */
-	BPF_ITER_ANCESTORS_UP,		/* walk ancestors upward. */
+	BPF_CGROUP_ITER_ORDER_UNSPEC = 0,
+	BPF_CGROUP_ITER_SELF_ONLY,		/* process only a single object. */
+	BPF_CGROUP_ITER_DESCENDANTS_PRE,	/* walk descendants in pre-order. */
+	BPF_CGROUP_ITER_DESCENDANTS_POST,	/* walk descendants in post-order. */
+	BPF_CGROUP_ITER_ANCESTORS_UP,		/* walk ancestors upward. */
 };
 
 union bpf_iter_link_info {
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index a1bae92be1fc..7b5bbe21b549 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -764,7 +764,7 @@ static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d,
 
 	/* union with nested struct */
 	TEST_BTF_DUMP_DATA(btf, d, "union", str, union bpf_iter_link_info, BTF_F_COMPACT,
-			   "(union bpf_iter_link_info){.map = (struct){.map_fd = (__u32)1,},.cgroup = (struct){.order = (enum bpf_cgroup_iter_order)BPF_ITER_SELF_ONLY,.cgroup_fd = (__u32)1,},}",
+			   "(union bpf_iter_link_info){.map = (struct){.map_fd = (__u32)1,},.cgroup = (struct){.order = (enum bpf_cgroup_iter_order)BPF_CGROUP_ITER_SELF_ONLY,.cgroup_fd = (__u32)1,},}",
 			   { .cgroup = { .order = 1, .cgroup_fd = 1, }});
 
 	/* struct skb with nested structs/unions; because type output is so
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c
index 38958c37b9ce..c4a2adb38da1 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter.c
@@ -134,7 +134,7 @@ static void test_walk_preorder(struct cgroup_iter *skel)
 		 cg_id[PARENT], cg_id[CHILD1], cg_id[CHILD2]);
 
 	read_from_cgroup_iter(skel->progs.cgroup_id_printer, cg_fd[PARENT],
-			      BPF_ITER_DESCENDANTS_PRE, "preorder");
+			      BPF_CGROUP_ITER_DESCENDANTS_PRE, "preorder");
 }
 
 /* Postorder walk prints child and parent in order. */
@@ -145,7 +145,7 @@ static void test_walk_postorder(struct cgroup_iter *skel)
 		 cg_id[CHILD1], cg_id[CHILD2], cg_id[PARENT]);
 
 	read_from_cgroup_iter(skel->progs.cgroup_id_printer, cg_fd[PARENT],
-			      BPF_ITER_DESCENDANTS_POST, "postorder");
+			      BPF_CGROUP_ITER_DESCENDANTS_POST, "postorder");
 }
 
 /* Walking parents prints parent and then root. */
@@ -159,7 +159,7 @@ static void test_walk_ancestors_up(struct cgroup_iter *skel)
 		 cg_id[PARENT], cg_id[ROOT]);
 
 	read_from_cgroup_iter(skel->progs.cgroup_id_printer, cg_fd[PARENT],
-			      BPF_ITER_ANCESTORS_UP, "ancestors_up");
+			      BPF_CGROUP_ITER_ANCESTORS_UP, "ancestors_up");
 
 	skel->bss->terminal_cgroup = 0;
 }
@@ -174,7 +174,7 @@ static void test_early_termination(struct cgroup_iter *skel)
 		 PROLOGUE "%8llu\n" EPILOGUE, cg_id[PARENT]);
 
 	read_from_cgroup_iter(skel->progs.cgroup_id_printer, cg_fd[PARENT],
-			      BPF_ITER_DESCENDANTS_PRE, "early_termination");
+			      BPF_CGROUP_ITER_DESCENDANTS_PRE, "early_termination");
 
 	skel->bss->terminate_early = 0;
 }
@@ -186,7 +186,7 @@ static void test_walk_self_only(struct cgroup_iter *skel)
 		 PROLOGUE "%8llu\n" EPILOGUE, cg_id[PARENT]);
 
 	read_from_cgroup_iter(skel->progs.cgroup_id_printer, cg_fd[PARENT],
-			      BPF_ITER_SELF_ONLY, "self_only");
+			      BPF_CGROUP_ITER_SELF_ONLY, "self_only");
 }
 
 void test_cgroup_iter(void)
-- 
2.37.2.672.g94769d06f0-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: Fix test that uses cgroup_iter order
  2022-08-25 21:39 [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Hao Luo
  2022-08-25 21:39 ` [PATCH bpf-next 1/2] bpf: Add CGROUP to cgroup_iter order Hao Luo
@ 2022-08-25 21:39 ` Hao Luo
  2022-08-25 21:56 ` [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Andrii Nakryiko
  2 siblings, 0 replies; 8+ messages in thread
From: Hao Luo @ 2022-08-25 21:39 UTC (permalink / raw)
  To: linux-kernel, bpf, netdev
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	John Fastabend, Jiri Olsa, Stanislav Fomichev, Yosry Ahmed,
	Hao Luo

Previous commit added a CGROUP prefix to the bpf_cgroup_iter_order.
It fixes the commit that introduced cgroup iter. This commit fixes
the selftest that uses the cgroup_iter_order. Because they fix two
different commits, so put them in separate patches.

Fixes: 88886309d2e8 ("selftests/bpf: add a selftest for cgroup hierarchical stats collection")
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Hao Luo <haoluo@google.com>
---
 .../selftests/bpf/prog_tests/cgroup_hierarchical_stats.c        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
index 101a6d70b863..bed1661596f7 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
@@ -275,7 +275,7 @@ static int setup_cgroup_iter(struct cgroup_hierarchical_stats *obj,
 	 * traverse one cgroup, so set the traversal order to "self".
 	 */
 	linfo.cgroup.cgroup_fd = cgroup_fd;
-	linfo.cgroup.order = BPF_ITER_SELF_ONLY;
+	linfo.cgroup.order = BPF_CGROUP_ITER_SELF_ONLY;
 	opts.link_info = &linfo;
 	opts.link_info_len = sizeof(linfo);
 	link = bpf_program__attach_iter(obj->progs.dump_vmscan, &opts);
-- 
2.37.2.672.g94769d06f0-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order
  2022-08-25 21:39 [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Hao Luo
  2022-08-25 21:39 ` [PATCH bpf-next 1/2] bpf: Add CGROUP to cgroup_iter order Hao Luo
  2022-08-25 21:39 ` [PATCH bpf-next 2/2] selftests/bpf: Fix test that uses " Hao Luo
@ 2022-08-25 21:56 ` Andrii Nakryiko
  2022-08-25 22:00   ` Hao Luo
  2022-08-25 23:19   ` Yosry Ahmed
  2 siblings, 2 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2022-08-25 21:56 UTC (permalink / raw)
  To: Hao Luo
  Cc: linux-kernel, bpf, netdev, Alexei Starovoitov, Andrii Nakryiko,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	KP Singh, John Fastabend, Jiri Olsa, Stanislav Fomichev,
	Yosry Ahmed

On Thu, Aug 25, 2022 at 2:39 PM Hao Luo <haoluo@google.com> wrote:
>
> As suggested by Andrii, add 'CGROUP' to cgroup_iter_order. This fix is
> divided into two patches. Patch 1/2 fixes the commit that introduced
> cgroup_iter. Patch 2/2 fixes the selftest that uses the
> cgroup_iter_order. This is because the selftest was introduced in a

but if you split rename into two patches, you break selftests build
and thus potentially bisectability of selftests regressions. So I
think you have to keep both in the same patch.

With that:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> different commit. I tested this patchset via the following command:
>
>   test_progs -t cgroup,iter,btf_dump
>
> Hao Luo (2):
>   bpf: Add CGROUP to cgroup_iter order
>   selftests/bpf: Fix test that uses cgroup_iter order
>
>  include/uapi/linux/bpf.h                      | 10 +++---
>  kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
>  tools/include/uapi/linux/bpf.h                | 10 +++---
>  .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
>  .../prog_tests/cgroup_hierarchical_stats.c    |  2 +-
>  .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
>  6 files changed, 33 insertions(+), 33 deletions(-)
>
> --
> 2.37.2.672.g94769d06f0-goog
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order
  2022-08-25 21:56 ` [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Andrii Nakryiko
@ 2022-08-25 22:00   ` Hao Luo
  2022-08-25 23:19   ` Yosry Ahmed
  1 sibling, 0 replies; 8+ messages in thread
From: Hao Luo @ 2022-08-25 22:00 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: linux-kernel, bpf, netdev, Alexei Starovoitov, Andrii Nakryiko,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	KP Singh, John Fastabend, Jiri Olsa, Stanislav Fomichev,
	Yosry Ahmed

On Thu, Aug 25, 2022 at 2:56 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Aug 25, 2022 at 2:39 PM Hao Luo <haoluo@google.com> wrote:
> >
> > As suggested by Andrii, add 'CGROUP' to cgroup_iter_order. This fix is
> > divided into two patches. Patch 1/2 fixes the commit that introduced
> > cgroup_iter. Patch 2/2 fixes the selftest that uses the
> > cgroup_iter_order. This is because the selftest was introduced in a
>
> but if you split rename into two patches, you break selftests build
> and thus potentially bisectability of selftests regressions. So I
> think you have to keep both in the same patch.
>
> With that:
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>

Yeah. I wasn't sure what to do. Then we need bundle this fix with
those two commits together. Will squash the commits into one and send
a v2.

> > different commit. I tested this patchset via the following command:
> >
> >   test_progs -t cgroup,iter,btf_dump
> >
> > Hao Luo (2):
> >   bpf: Add CGROUP to cgroup_iter order
> >   selftests/bpf: Fix test that uses cgroup_iter order
> >
> >  include/uapi/linux/bpf.h                      | 10 +++---
> >  kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
> >  tools/include/uapi/linux/bpf.h                | 10 +++---
> >  .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
> >  .../prog_tests/cgroup_hierarchical_stats.c    |  2 +-
> >  .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
> >  6 files changed, 33 insertions(+), 33 deletions(-)
> >
> > --
> > 2.37.2.672.g94769d06f0-goog
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order
  2022-08-25 21:56 ` [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Andrii Nakryiko
  2022-08-25 22:00   ` Hao Luo
@ 2022-08-25 23:19   ` Yosry Ahmed
  2022-08-26  2:03     ` Andrii Nakryiko
  1 sibling, 1 reply; 8+ messages in thread
From: Yosry Ahmed @ 2022-08-25 23:19 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Hao Luo, Linux Kernel Mailing List, bpf, Networking,
	Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	John Fastabend, Jiri Olsa, Stanislav Fomichev

On Thu, Aug 25, 2022 at 2:56 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Aug 25, 2022 at 2:39 PM Hao Luo <haoluo@google.com> wrote:
> >
> > As suggested by Andrii, add 'CGROUP' to cgroup_iter_order. This fix is
> > divided into two patches. Patch 1/2 fixes the commit that introduced
> > cgroup_iter. Patch 2/2 fixes the selftest that uses the
> > cgroup_iter_order. This is because the selftest was introduced in a
>
> but if you split rename into two patches, you break selftests build
> and thus potentially bisectability of selftests regressions. So I
> think you have to keep both in the same patch.

I thought fixes to commits still in bpf-next would get squashed. Would
you mind elaborating why we don't do this?

>
> With that:
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > different commit. I tested this patchset via the following command:
> >
> >   test_progs -t cgroup,iter,btf_dump
> >
> > Hao Luo (2):
> >   bpf: Add CGROUP to cgroup_iter order
> >   selftests/bpf: Fix test that uses cgroup_iter order
> >
> >  include/uapi/linux/bpf.h                      | 10 +++---
> >  kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
> >  tools/include/uapi/linux/bpf.h                | 10 +++---
> >  .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
> >  .../prog_tests/cgroup_hierarchical_stats.c    |  2 +-
> >  .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
> >  6 files changed, 33 insertions(+), 33 deletions(-)
> >
> > --
> > 2.37.2.672.g94769d06f0-goog
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order
  2022-08-25 23:19   ` Yosry Ahmed
@ 2022-08-26  2:03     ` Andrii Nakryiko
  2022-08-26  2:04       ` Yosry Ahmed
  0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2022-08-26  2:03 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Hao Luo, Linux Kernel Mailing List, bpf, Networking,
	Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	John Fastabend, Jiri Olsa, Stanislav Fomichev

On Thu, Aug 25, 2022 at 4:20 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Thu, Aug 25, 2022 at 2:56 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Aug 25, 2022 at 2:39 PM Hao Luo <haoluo@google.com> wrote:
> > >
> > > As suggested by Andrii, add 'CGROUP' to cgroup_iter_order. This fix is
> > > divided into two patches. Patch 1/2 fixes the commit that introduced
> > > cgroup_iter. Patch 2/2 fixes the selftest that uses the
> > > cgroup_iter_order. This is because the selftest was introduced in a
> >
> > but if you split rename into two patches, you break selftests build
> > and thus potentially bisectability of selftests regressions. So I
> > think you have to keep both in the same patch.
>
> I thought fixes to commits still in bpf-next would get squashed. Would
> you mind elaborating why we don't do this?
>

We don't amend follow up fixes into original commits and preserve history.

> >
> > With that:
> >
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > > different commit. I tested this patchset via the following command:
> > >
> > >   test_progs -t cgroup,iter,btf_dump
> > >
> > > Hao Luo (2):
> > >   bpf: Add CGROUP to cgroup_iter order
> > >   selftests/bpf: Fix test that uses cgroup_iter order
> > >
> > >  include/uapi/linux/bpf.h                      | 10 +++---
> > >  kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
> > >  tools/include/uapi/linux/bpf.h                | 10 +++---
> > >  .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
> > >  .../prog_tests/cgroup_hierarchical_stats.c    |  2 +-
> > >  .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
> > >  6 files changed, 33 insertions(+), 33 deletions(-)
> > >
> > > --
> > > 2.37.2.672.g94769d06f0-goog
> > >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order
  2022-08-26  2:03     ` Andrii Nakryiko
@ 2022-08-26  2:04       ` Yosry Ahmed
  0 siblings, 0 replies; 8+ messages in thread
From: Yosry Ahmed @ 2022-08-26  2:04 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Hao Luo, Linux Kernel Mailing List, bpf, Networking,
	Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	John Fastabend, Jiri Olsa, Stanislav Fomichev

On Thu, Aug 25, 2022 at 7:03 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Aug 25, 2022 at 4:20 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> >
> > On Thu, Aug 25, 2022 at 2:56 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, Aug 25, 2022 at 2:39 PM Hao Luo <haoluo@google.com> wrote:
> > > >
> > > > As suggested by Andrii, add 'CGROUP' to cgroup_iter_order. This fix is
> > > > divided into two patches. Patch 1/2 fixes the commit that introduced
> > > > cgroup_iter. Patch 2/2 fixes the selftest that uses the
> > > > cgroup_iter_order. This is because the selftest was introduced in a
> > >
> > > but if you split rename into two patches, you break selftests build
> > > and thus potentially bisectability of selftests regressions. So I
> > > think you have to keep both in the same patch.
> >
> > I thought fixes to commits still in bpf-next would get squashed. Would
> > you mind elaborating why we don't do this?
> >
>
> We don't amend follow up fixes into original commits and preserve history.

Got it. Thanks Andrii.

>
> > >
> > > With that:
> > >
> > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > >
> > > > different commit. I tested this patchset via the following command:
> > > >
> > > >   test_progs -t cgroup,iter,btf_dump
> > > >
> > > > Hao Luo (2):
> > > >   bpf: Add CGROUP to cgroup_iter order
> > > >   selftests/bpf: Fix test that uses cgroup_iter order
> > > >
> > > >  include/uapi/linux/bpf.h                      | 10 +++---
> > > >  kernel/bpf/cgroup_iter.c                      | 32 +++++++++----------
> > > >  tools/include/uapi/linux/bpf.h                | 10 +++---
> > > >  .../selftests/bpf/prog_tests/btf_dump.c       |  2 +-
> > > >  .../prog_tests/cgroup_hierarchical_stats.c    |  2 +-
> > > >  .../selftests/bpf/prog_tests/cgroup_iter.c    | 10 +++---
> > > >  6 files changed, 33 insertions(+), 33 deletions(-)
> > > >
> > > > --
> > > > 2.37.2.672.g94769d06f0-goog
> > > >

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-08-26  2:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25 21:39 [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Hao Luo
2022-08-25 21:39 ` [PATCH bpf-next 1/2] bpf: Add CGROUP to cgroup_iter order Hao Luo
2022-08-25 21:39 ` [PATCH bpf-next 2/2] selftests/bpf: Fix test that uses " Hao Luo
2022-08-25 21:56 ` [PATCH bpf-next 0/2] Add CGROUP prefix to cgroup_iter_order Andrii Nakryiko
2022-08-25 22:00   ` Hao Luo
2022-08-25 23:19   ` Yosry Ahmed
2022-08-26  2:03     ` Andrii Nakryiko
2022-08-26  2:04       ` Yosry Ahmed

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).