Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf, cgroup: Add BPF support for cgroup1 hierarchy #631

Closed
wants to merge 11 commits into from
Closed

bpf, cgroup: Add BPF support for cgroup1 hierarchy #631

wants to merge 11 commits into from

Conversation

danielocfb
Copy link
Owner

Pull request for series with
subject: bpf, cgroup: Add BPF support for cgroup1 hierarchy
version: 3
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378

@danielocfb
Copy link
Owner Author

Upstream branch: f2fbb90
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb
Copy link
Owner Author

Upstream branch: ff269e2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 3c06420 to 3c501ae Compare November 2, 2023 05:38
@danielocfb
Copy link
Owner Author

Upstream branch: cb3c6a5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 3c501ae to 53f8b8f Compare November 2, 2023 05:54
@danielocfb
Copy link
Owner Author

Upstream branch: b4e59c1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb added needs-ack and removed new labels Nov 9, 2023
@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 53f8b8f to 863d91e Compare November 9, 2023 01:12
@danielocfb danielocfb force-pushed the bpf-next_base branch 3 times, most recently from 902178a to bc1b833 Compare November 9, 2023 18:41
@danielocfb
Copy link
Owner Author

Upstream branch: 3815f89
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 863d91e to 8a4fe5e Compare November 9, 2023 18:46
@danielocfb danielocfb force-pushed the bpf-next_base branch 2 times, most recently from 0fbe9f3 to b0d20d2 Compare November 9, 2023 19:02
@danielocfb
Copy link
Owner Author

Upstream branch: 6f101db
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 8a4fe5e to b672ec2 Compare November 9, 2023 19:07
@danielocfb danielocfb force-pushed the bpf-next_base branch 2 times, most recently from 1e02037 to a98cbca Compare November 9, 2023 19:19
@danielocfb
Copy link
Owner Author

Upstream branch: e80742d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb
Copy link
Owner Author

Upstream branch: e80742d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 3f818ef to f9181ca Compare November 9, 2023 20:41
@danielocfb
Copy link
Owner Author

Upstream branch: e80742d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from f9181ca to 5b414cf Compare November 9, 2023 21:08
laoar added 8 commits November 9, 2023 13:17
@laoar @d-e-s-o
cgroup: Remove unnecessary list_empty()
7a03d97 
The root hasn't been removed from the root_list, so the list can't be NULL.
However, if it had been removed, attempting to destroy it once more is not
possible. Let's replace this with WARN_ON_ONCE() for clarity.

Signed-off-by: Yafang Shao <[email protected]>
Acked-by: Tejun Heo <[email protected]>
@laoar @d-e-s-o
cgroup: Make operations on the cgroup root_list RCU safe
4bf4eaa 
At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.

Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
cgroup: Eliminate the need for cgroup_mutex in proc_cgroup_show()
6134b7c 
The cgroup root_list is already RCU-safe. Therefore, we can replace the
cgroup_mutex with the RCU read lock in some particular paths. This change
will be particularly beneficial for frequent operations, such as
`cat /proc/self/cgroup`, in a cgroup1-based container environment.

I did stress tests with this change, as outlined below
(with CONFIG_PROVE_RCU_LIST enabled):

- Continuously mounting and unmounting named cgroups in some tasks,
  for example:

  cgrp_name=$1
  while true
  do
      mount -t cgroup -o none,name=$cgrp_name none /$cgrp_name
      umount /$cgrp_name
  done

- Continuously triggering proc_cgroup_show() in some tasks concurrently,
  for example:
  while true; do cat /proc/self/cgroup > /dev/null; done

They can ran successfully after implementing this change, with no RCU
warnings in dmesg.

Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
cgroup: Add annotation for holding namespace_sem in current_cgns_cgro…
0d40ede 
…up_from_root()

When I initially examined the function current_cgns_cgroup_from_root(), I
was perplexed by its lack of holding cgroup_mutex. However, after Michal
explained the reason[0] to me, I realized that it already holds the
namespace_sem. I believe this intricacy could also confuse others, so it
would be advisable to include an annotation for clarification.

After we replace the cgroup_mutex with RCU read lock, if current doesn't
hold the namespace_sem, the root cgroup will be NULL. So let's add a
WARN_ON_ONCE() for it.

[0]. https://lore.kernel.org/bpf/afdnpo3jz2ic2ampud7swd6so5carkilts2mkygcaw67vbw6yh@5b5mncf7qyet

Signed-off-by: Yafang Shao <[email protected]>
Cc: Michal Koutný <[email protected]>
@laoar @d-e-s-o
cgroup: Add a new helper for cgroup1 hierarchy
96ddadc 
A new helper is added for cgroup1 hierarchy:

- task_get_cgroup1
  Acquires the associated cgroup of a task within a specific cgroup1
  hierarchy. The cgroup1 hierarchy is identified by its hierarchy ID.

This helper function is added to facilitate the tracing of tasks within
a particular container or cgroup dir in BPF programs. It's important to
note that this helper is designed specifically for cgroup1 only.

Suggested-by: Tejun Heo <[email protected]>
Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
bpf: Add a new kfunc for cgroup1 hierarchy
4bb8501 
A new kfunc is added to acquire cgroup1 of a task:

- bpf_task_get_cgroup1
  Acquires the associated cgroup of a task whithin a specific cgroup1
  hierarchy. The cgroup1 hierarchy is identified by its hierarchy ID.

This new kfunc enables the tracing of tasks within a designated
container or cgroup directory in BPF programs.

Suggested-by: Tejun Heo <[email protected]>
Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
selftests/bpf: Fix issues in setup_classid_environment()
0d0d0c9 
If the net_cls subsystem is already mounted, attempting to mount it again
in setup_classid_environment() will result in a failure with the error code
EBUSY. Despite this, tmpfs will have been successfully mounted at
/sys/fs/cgroup/net_cls. Consequently, the /sys/fs/cgroup/net_cls directory
will be empty, causing subsequent setup operations to fail.

Here's an error log excerpt illustrating the issue when net_cls has already
been mounted at /sys/fs/cgroup/net_cls prior to running
setup_classid_environment():

- Before that change

  $ tools/testing/selftests/bpf/test_progs --name=cgroup_v1v2
  test_cgroup_v1v2:PASS:server_fd 0 nsec
  test_cgroup_v1v2:PASS:client_fd 0 nsec
  test_cgroup_v1v2:PASS:cgroup_fd 0 nsec
  test_cgroup_v1v2:PASS:server_fd 0 nsec
  run_test:PASS:skel_open 0 nsec
  run_test:PASS:prog_attach 0 nsec
  test_cgroup_v1v2:PASS:cgroup-v2-only 0 nsec
  (cgroup_helpers.c:248: errno: No such file or directory) Opening Cgroup Procs: /sys/fs/cgroup/net_cls/cgroup.procs
  (cgroup_helpers.c:540: errno: No such file or directory) Opening cgroup classid: /sys/fs/cgroup/net_cls/cgroup-test-work-dir/net_cls.classid
  run_test:PASS:skel_open 0 nsec
  run_test:PASS:prog_attach 0 nsec
  (cgroup_helpers.c:248: errno: No such file or directory) Opening Cgroup Procs: /sys/fs/cgroup/net_cls/cgroup-test-work-dir/cgroup.procs
  run_test:FAIL:join_classid unexpected error: 1 (errno 2)
  test_cgroup_v1v2:FAIL:cgroup-v1v2 unexpected error: -1 (errno 2)
  (cgroup_helpers.c:248: errno: No such file or directory) Opening Cgroup Procs: /sys/fs/cgroup/net_cls/cgroup.procs
  #44      cgroup_v1v2:FAIL
  Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

- After that change
  $ tools/testing/selftests/bpf/test_progs --name=cgroup_v1v2
  #44      cgroup_v1v2:OK
  Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
selftests/bpf: Add parallel support for classid
d9fa44b 
Include the current pid in the classid cgroup path. This way, different
testers relying on classid-based configurations will have distinct classid
cgroup directories, enabling them to run concurrently. Additionally, we
leverage the current pid as the classid, ensuring unique identification.

Signed-off-by: Yafang Shao <[email protected]>
@danielocfb
Copy link
Owner Author

Upstream branch: e80742d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=797378
version: 3

laoar added 3 commits November 9, 2023 13:17
@laoar @d-e-s-o
selftests/bpf: Add a new cgroup helper get_classid_cgroup_id()
567f6aa 
Introduce a new helper function to retrieve the cgroup ID from a net_cls
cgroup directory.

Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
selftests/bpf: Add a new cgroup helper get_cgroup_hierarchy_id()
b1d13ff 
A new cgroup helper function, get_cgroup1_hierarchy_id(), has been
introduced to obtain the ID of a cgroup1 hierarchy based on the provided
cgroup name. This cgroup name can be obtained from the /proc/self/cgroup
file.

Signed-off-by: Yafang Shao <[email protected]>
@laoar @d-e-s-o
selftests/bpf: Add selftests for cgroup1 hierarchy
cfd8dba 
Add selftests for cgroup1 hierarchy.
The result as follows,

  $ tools/testing/selftests/bpf/test_progs --name=cgroup1_hierarchy
  #36/1    cgroup1_hierarchy/test_cgroup1_hierarchy:OK
  #36/2    cgroup1_hierarchy/test_root_cgid:OK
  #36/3    cgroup1_hierarchy/test_invalid_level:OK
  #36/4    cgroup1_hierarchy/test_invalid_cgid:OK
  #36/5    cgroup1_hierarchy/test_invalid_hid:OK
  #36/6    cgroup1_hierarchy/test_invalid_cgrp_name:OK
  #36/7    cgroup1_hierarchy/test_invalid_cgrp_name2:OK
  #36/8    cgroup1_hierarchy/test_sleepable_prog:OK
  #36      cgroup1_hierarchy:OK
  Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED

Besides, I also did some stress test similar to the patch #2 in this
series, as follows (with CONFIG_PROVE_RCU_LIST enabled):

- Continuously mounting and unmounting named cgroups in some tasks,
  for example:

  cgrp_name=$1
  while true
  do
      mount -t cgroup -o none,name=$cgrp_name none /$cgrp_name
      umount /$cgrp_name
  done

- Continuously run this selftest concurrently,
  while true; do ./test_progs --name=cgroup1_hierarchy; done

They can ran successfully without any RCU warnings in dmesg.

Signed-off-by: Yafang Shao <[email protected]>
@danielocfb danielocfb force-pushed the series/790938=>bpf-next branch from 5b414cf to cfd8dba Compare November 9, 2023 21:17
@danielocfb
Copy link
Owner Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=797378 expired. Closing PR.

@danielocfb danielocfb closed this Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next changes-requested V3-ci-fail V3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danielocfb @laoar