* [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode
@ 2019-12-19 7:44 Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach Andrey Ignatov
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
v3->v4:
- use OPTS_VALID and OPTS_GET to handle bpf_prog_attach_opts.
v2->v3:
- rely on DECLARE_LIBBPF_OPTS from libbpf_common.h;
- separate "required" and "optional" arguments in bpf_prog_attach_xattr;
- convert test_cgroup_attach to prog_tests;
- move the new selftest to prog_tests/cgroup_attach_multi.
v1->v2:
- move DECLARE_LIBBPF_OPTS from libbpf.h to bpf.h (patch 4);
- switch new libbpf API to OPTS framework;
- switch selftest to libbpf OPTS framework.
This patch set adds support for replacing cgroup-bpf programs attached with
BPF_F_ALLOW_MULTI flag so that any program in a list can be updated to a new
version without service interruption and order of programs can be preserved.
Please see patch 3 for details on the use-case and API changes.
Other patches:
Patch 1 is preliminary refactoring of __cgroup_bpf_attach to simplify it.
Patch 2 is minor cleanup of hierarchy_allows_attach.
Patch 4 extends libbpf API to support new set of attach attributes.
Patch 5 converts test_cgroup_attach to prog_tests.
Patch 6 adds selftest coverage for the new API.
Andrey Ignatov (6):
bpf: Simplify __cgroup_bpf_attach
bpf: Remove unused new_flags in hierarchy_allows_attach()
bpf: Support replacing cgroup-bpf program in MULTI mode
libbpf: Introduce bpf_prog_attach_xattr
selftests/bpf: Convert test_cgroup_attach to prog_tests
selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi
include/linux/bpf-cgroup.h | 4 +-
include/uapi/linux/bpf.h | 10 +
kernel/bpf/cgroup.c | 97 +--
kernel/bpf/syscall.c | 4 +-
kernel/cgroup/cgroup.c | 5 +-
tools/include/uapi/linux/bpf.h | 10 +
tools/lib/bpf/bpf.c | 17 +-
tools/lib/bpf/bpf.h | 11 +
tools/lib/bpf/libbpf.map | 1 +
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 3 +-
.../bpf/prog_tests/cgroup_attach_autodetach.c | 111 ++++
.../bpf/prog_tests/cgroup_attach_multi.c | 285 +++++++++
.../bpf/prog_tests/cgroup_attach_override.c | 148 +++++
.../selftests/bpf/test_cgroup_attach.c | 571 ------------------
15 files changed, 652 insertions(+), 626 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
delete mode 100644 tools/testing/selftests/bpf/test_cgroup_attach.c
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
@ 2019-12-19 7:44 ` Andrey Ignatov
2019-12-19 22:31 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 2/6] bpf: Remove unused new_flags in hierarchy_allows_attach() Andrey Ignatov
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
__cgroup_bpf_attach has a lot of identical code to handle two scenarios:
BPF_F_ALLOW_MULTI is set and unset.
Simplify it by splitting the two main steps:
* First, the decision is made whether a new bpf_prog_list entry should
be allocated or existing entry should be reused for the new program.
This decision is saved in replace_pl pointer;
* Next, replace_pl pointer is used to handle both possible states of
BPF_F_ALLOW_MULTI flag (set / unset) instead of doing similar work for
them separately.
This splitting, in turn, allows to make further simplifications:
* The check for attaching same program twice in BPF_F_ALLOW_MULTI mode
can be done before allocating cgroup storage, so that if user tries to
attach same program twice no alloc/free happens as it was before;
* pl_was_allocated becomes redundant so it's removed.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/cgroup.c | 62 +++++++++++++++++----------------------------
1 file changed, 23 insertions(+), 39 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 9f90d3c92bda..e8cbdd1be687 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -295,9 +295,8 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
struct bpf_prog *old_prog = NULL;
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE],
*old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {NULL};
+ struct bpf_prog_list *pl, *replace_pl = NULL;
enum bpf_cgroup_storage_type stype;
- struct bpf_prog_list *pl;
- bool pl_was_allocated;
int err;
if ((flags & BPF_F_ALLOW_OVERRIDE) && (flags & BPF_F_ALLOW_MULTI))
@@ -317,6 +316,16 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
if (prog_list_length(progs) >= BPF_CGROUP_MAX_PROGS)
return -E2BIG;
+ if (flags & BPF_F_ALLOW_MULTI) {
+ list_for_each_entry(pl, progs, node) {
+ if (pl->prog == prog)
+ /* disallow attaching the same prog twice */
+ return -EINVAL;
+ }
+ } else if (!list_empty(progs)) {
+ replace_pl = list_first_entry(progs, typeof(*pl), node);
+ }
+
for_each_cgroup_storage_type(stype) {
storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
if (IS_ERR(storage[stype])) {
@@ -327,52 +336,27 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
}
}
- if (flags & BPF_F_ALLOW_MULTI) {
- list_for_each_entry(pl, progs, node) {
- if (pl->prog == prog) {
- /* disallow attaching the same prog twice */
- for_each_cgroup_storage_type(stype)
- bpf_cgroup_storage_free(storage[stype]);
- return -EINVAL;
- }
+ if (replace_pl) {
+ pl = replace_pl;
+ old_prog = pl->prog;
+ for_each_cgroup_storage_type(stype) {
+ old_storage[stype] = pl->storage[stype];
+ bpf_cgroup_storage_unlink(old_storage[stype]);
}
-
+ } else {
pl = kmalloc(sizeof(*pl), GFP_KERNEL);
if (!pl) {
for_each_cgroup_storage_type(stype)
bpf_cgroup_storage_free(storage[stype]);
return -ENOMEM;
}
-
- pl_was_allocated = true;
- pl->prog = prog;
- for_each_cgroup_storage_type(stype)
- pl->storage[stype] = storage[stype];
list_add_tail(&pl->node, progs);
- } else {
- if (list_empty(progs)) {
- pl = kmalloc(sizeof(*pl), GFP_KERNEL);
- if (!pl) {
- for_each_cgroup_storage_type(stype)
- bpf_cgroup_storage_free(storage[stype]);
- return -ENOMEM;
- }
- pl_was_allocated = true;
- list_add_tail(&pl->node, progs);
- } else {
- pl = list_first_entry(progs, typeof(*pl), node);
- old_prog = pl->prog;
- for_each_cgroup_storage_type(stype) {
- old_storage[stype] = pl->storage[stype];
- bpf_cgroup_storage_unlink(old_storage[stype]);
- }
- pl_was_allocated = false;
- }
- pl->prog = prog;
- for_each_cgroup_storage_type(stype)
- pl->storage[stype] = storage[stype];
}
+ pl->prog = prog;
+ for_each_cgroup_storage_type(stype)
+ pl->storage[stype] = storage[stype];
+
cgrp->bpf.flags[type] = flags;
err = update_effective_progs(cgrp, type);
@@ -401,7 +385,7 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
pl->storage[stype] = old_storage[stype];
bpf_cgroup_storage_link(old_storage[stype], cgrp, type);
}
- if (pl_was_allocated) {
+ if (!replace_pl) {
list_del(&pl->node);
kfree(pl);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 2/6] bpf: Remove unused new_flags in hierarchy_allows_attach()
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach Andrey Ignatov
@ 2019-12-19 7:44 ` Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 3/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
new_flags is unused, remove it.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/cgroup.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index e8cbdd1be687..283efe3ce052 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -103,8 +103,7 @@ static u32 prog_list_length(struct list_head *head)
* if parent has overridable or multi-prog, allow attaching
*/
static bool hierarchy_allows_attach(struct cgroup *cgrp,
- enum bpf_attach_type type,
- u32 new_flags)
+ enum bpf_attach_type type)
{
struct cgroup *p;
@@ -303,7 +302,7 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
/* invalid combination */
return -EINVAL;
- if (!hierarchy_allows_attach(cgrp, type, flags))
+ if (!hierarchy_allows_attach(cgrp, type))
return -EPERM;
if (!list_empty(progs) && cgrp->bpf.flags[type] != flags)
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 3/6] bpf: Support replacing cgroup-bpf program in MULTI mode
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 2/6] bpf: Remove unused new_flags in hierarchy_allows_attach() Andrey Ignatov
@ 2019-12-19 7:44 ` Andrey Ignatov
2019-12-19 22:41 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 4/6] libbpf: Introduce bpf_prog_attach_xattr Andrey Ignatov
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
The common use-case in production is to have multiple cgroup-bpf
programs per attach type that cover multiple use-cases. Such programs
are attached with BPF_F_ALLOW_MULTI and can be maintained by different
people.
Order of programs usually matters, for example imagine two egress
programs: the first one drops packets and the second one counts packets.
If they're swapped the result of counting program will be different.
It brings operational challenges with updating cgroup-bpf program(s)
attached with BPF_F_ALLOW_MULTI since there is no way to replace a
program:
* One way to update is to detach all programs first and then attach the
new version(s) again in the right order. This introduces an
interruption in the work a program is doing and may not be acceptable
(e.g. if it's egress firewall);
* Another way is attach the new version of a program first and only then
detach the old version. This introduces the time interval when two
versions of same program are working, what may not be acceptable if a
program is not idempotent. It also imposes additional burden on
program developers to make sure that two versions of their program can
co-exist.
Solve the problem by introducing a "replace" mode in BPF_PROG_ATTACH
command for cgroup-bpf programs being attached with BPF_F_ALLOW_MULTI
flag. This mode is enabled by newly introduced BPF_F_REPLACE attach flag
and bpf_attr.replace_bpf_fd attribute to pass fd of the old program to
replace
That way user can replace any program among those attached with
BPF_F_ALLOW_MULTI flag without the problems described above.
Details of the new API:
* If BPF_F_REPLACE is set but replace_bpf_fd doesn't have valid
descriptor of BPF program, BPF_PROG_ATTACH will return corresponding
error (EINVAL or EBADF).
* If replace_bpf_fd has valid descriptor of BPF program but such a
program is not attached to specified cgroup, BPF_PROG_ATTACH will
return ENOENT.
BPF_F_REPLACE is introduced to make the user intent clear, since
replace_bpf_fd alone can't be used for this (its default value, 0, is a
valid fd). BPF_F_REPLACE also makes it possible to extend the API in the
future (e.g. add BPF_F_BEFORE and BPF_F_AFTER if needed).
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
include/linux/bpf-cgroup.h | 4 +++-
include/uapi/linux/bpf.h | 10 ++++++++++
kernel/bpf/cgroup.c | 30 ++++++++++++++++++++++++++----
kernel/bpf/syscall.c | 4 ++--
kernel/cgroup/cgroup.c | 5 +++--
tools/include/uapi/linux/bpf.h | 10 ++++++++++
6 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 169fd25f6bc2..18f6a6da7c3c 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -85,6 +85,7 @@ int cgroup_bpf_inherit(struct cgroup *cgrp);
void cgroup_bpf_offline(struct cgroup *cgrp);
int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
+ struct bpf_prog *replace_prog,
enum bpf_attach_type type, u32 flags);
int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
enum bpf_attach_type type);
@@ -93,7 +94,8 @@ int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
/* Wrapper for __cgroup_bpf_*() protected by cgroup_mutex */
int cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
- enum bpf_attach_type type, u32 flags);
+ struct bpf_prog *replace_prog, enum bpf_attach_type type,
+ u32 flags);
int cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
enum bpf_attach_type type, u32 flags);
int cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index dbbcf0b02970..7df436da542d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -231,6 +231,11 @@ enum bpf_attach_type {
* When children program makes decision (like picking TCP CA or sock bind)
* parent program has a chance to override it.
*
+ * With BPF_F_ALLOW_MULTI a new program is added to the end of the list of
+ * programs for a cgroup. Though it's possible to replace an old program at
+ * any position by also specifying BPF_F_REPLACE flag and position itself in
+ * replace_bpf_fd attribute. Old program at this position will be released.
+ *
* A cgroup with MULTI or OVERRIDE flag allows any attach flags in sub-cgroups.
* A cgroup with NONE doesn't allow any programs in sub-cgroups.
* Ex1:
@@ -249,6 +254,7 @@ enum bpf_attach_type {
*/
#define BPF_F_ALLOW_OVERRIDE (1U << 0)
#define BPF_F_ALLOW_MULTI (1U << 1)
+#define BPF_F_REPLACE (1U << 2)
/* If BPF_F_STRICT_ALIGNMENT is used in BPF_PROG_LOAD command, the
* verifier will perform strict alignment checking as if the kernel
@@ -442,6 +448,10 @@ union bpf_attr {
__u32 attach_bpf_fd; /* eBPF program to attach */
__u32 attach_type;
__u32 attach_flags;
+ __u32 replace_bpf_fd; /* previously attached eBPF
+ * program to replace if
+ * BPF_F_REPLACE is used
+ */
};
struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 283efe3ce052..45346c79613a 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -282,14 +282,17 @@ static int update_effective_progs(struct cgroup *cgrp,
* propagate the change to descendants
* @cgrp: The cgroup which descendants to traverse
* @prog: A program to attach
+ * @replace_prog: Previously attached program to replace if BPF_F_REPLACE is set
* @type: Type of attach operation
* @flags: Option flags
*
* Must be called with cgroup_mutex held.
*/
int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
+ struct bpf_prog *replace_prog,
enum bpf_attach_type type, u32 flags)
{
+ u32 saved_flags = (flags & (BPF_F_ALLOW_OVERRIDE | BPF_F_ALLOW_MULTI));
struct list_head *progs = &cgrp->bpf.progs[type];
struct bpf_prog *old_prog = NULL;
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE],
@@ -298,14 +301,15 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
enum bpf_cgroup_storage_type stype;
int err;
- if ((flags & BPF_F_ALLOW_OVERRIDE) && (flags & BPF_F_ALLOW_MULTI))
+ if (((flags & BPF_F_ALLOW_OVERRIDE) && (flags & BPF_F_ALLOW_MULTI)) ||
+ ((flags & BPF_F_REPLACE) && !(flags & BPF_F_ALLOW_MULTI)))
/* invalid combination */
return -EINVAL;
if (!hierarchy_allows_attach(cgrp, type))
return -EPERM;
- if (!list_empty(progs) && cgrp->bpf.flags[type] != flags)
+ if (!list_empty(progs) && cgrp->bpf.flags[type] != saved_flags)
/* Disallow attaching non-overridable on top
* of existing overridable in this cgroup.
* Disallow attaching multi-prog if overridable or none
@@ -320,7 +324,12 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
if (pl->prog == prog)
/* disallow attaching the same prog twice */
return -EINVAL;
+ if (pl->prog == replace_prog)
+ replace_pl = pl;
}
+ if ((flags & BPF_F_REPLACE) && !replace_pl)
+ /* prog to replace not found for cgroup */
+ return -ENOENT;
} else if (!list_empty(progs)) {
replace_pl = list_first_entry(progs, typeof(*pl), node);
}
@@ -356,7 +365,7 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
for_each_cgroup_storage_type(stype)
pl->storage[stype] = storage[stype];
- cgrp->bpf.flags[type] = flags;
+ cgrp->bpf.flags[type] = saved_flags;
err = update_effective_progs(cgrp, type);
if (err)
@@ -522,6 +531,7 @@ int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
int cgroup_bpf_prog_attach(const union bpf_attr *attr,
enum bpf_prog_type ptype, struct bpf_prog *prog)
{
+ struct bpf_prog *replace_prog = NULL;
struct cgroup *cgrp;
int ret;
@@ -529,8 +539,20 @@ int cgroup_bpf_prog_attach(const union bpf_attr *attr,
if (IS_ERR(cgrp))
return PTR_ERR(cgrp);
- ret = cgroup_bpf_attach(cgrp, prog, attr->attach_type,
+ if ((attr->attach_flags & BPF_F_ALLOW_MULTI) &&
+ (attr->attach_flags & BPF_F_REPLACE)) {
+ replace_prog = bpf_prog_get_type(attr->replace_bpf_fd, ptype);
+ if (IS_ERR(replace_prog)) {
+ cgroup_put(cgrp);
+ return PTR_ERR(replace_prog);
+ }
+ }
+
+ ret = cgroup_bpf_attach(cgrp, prog, replace_prog, attr->attach_type,
attr->attach_flags);
+
+ if (replace_prog)
+ bpf_prog_put(replace_prog);
cgroup_put(cgrp);
return ret;
}
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b08c362f4e02..81ee8595dfee 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2073,10 +2073,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
}
}
-#define BPF_PROG_ATTACH_LAST_FIELD attach_flags
+#define BPF_PROG_ATTACH_LAST_FIELD replace_bpf_fd
#define BPF_F_ATTACH_MASK \
- (BPF_F_ALLOW_OVERRIDE | BPF_F_ALLOW_MULTI)
+ (BPF_F_ALLOW_OVERRIDE | BPF_F_ALLOW_MULTI | BPF_F_REPLACE)
static int bpf_prog_attach(const union bpf_attr *attr)
{
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 735af8f15f95..725365df066d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6288,12 +6288,13 @@ void cgroup_sk_free(struct sock_cgroup_data *skcd)
#ifdef CONFIG_CGROUP_BPF
int cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
- enum bpf_attach_type type, u32 flags)
+ struct bpf_prog *replace_prog, enum bpf_attach_type type,
+ u32 flags)
{
int ret;
mutex_lock(&cgroup_mutex);
- ret = __cgroup_bpf_attach(cgrp, prog, type, flags);
+ ret = __cgroup_bpf_attach(cgrp, prog, replace_prog, type, flags);
mutex_unlock(&cgroup_mutex);
return ret;
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index dbbcf0b02970..7df436da542d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -231,6 +231,11 @@ enum bpf_attach_type {
* When children program makes decision (like picking TCP CA or sock bind)
* parent program has a chance to override it.
*
+ * With BPF_F_ALLOW_MULTI a new program is added to the end of the list of
+ * programs for a cgroup. Though it's possible to replace an old program at
+ * any position by also specifying BPF_F_REPLACE flag and position itself in
+ * replace_bpf_fd attribute. Old program at this position will be released.
+ *
* A cgroup with MULTI or OVERRIDE flag allows any attach flags in sub-cgroups.
* A cgroup with NONE doesn't allow any programs in sub-cgroups.
* Ex1:
@@ -249,6 +254,7 @@ enum bpf_attach_type {
*/
#define BPF_F_ALLOW_OVERRIDE (1U << 0)
#define BPF_F_ALLOW_MULTI (1U << 1)
+#define BPF_F_REPLACE (1U << 2)
/* If BPF_F_STRICT_ALIGNMENT is used in BPF_PROG_LOAD command, the
* verifier will perform strict alignment checking as if the kernel
@@ -442,6 +448,10 @@ union bpf_attr {
__u32 attach_bpf_fd; /* eBPF program to attach */
__u32 attach_type;
__u32 attach_flags;
+ __u32 replace_bpf_fd; /* previously attached eBPF
+ * program to replace if
+ * BPF_F_REPLACE is used
+ */
};
struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 4/6] libbpf: Introduce bpf_prog_attach_xattr
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
` (2 preceding siblings ...)
2019-12-19 7:44 ` [PATCH v4 bpf-next 3/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
@ 2019-12-19 7:44 ` Andrey Ignatov
2019-12-19 22:42 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 5/6] selftests/bpf: Convert test_cgroup_attach to prog_tests Andrey Ignatov
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
Introduce a new bpf_prog_attach_xattr function that, in addition to
program fd, target fd and attach type, accepts an extendable struct
bpf_prog_attach_opts.
bpf_prog_attach_opts relies on DECLARE_LIBBPF_OPTS macro to maintain
backward and forward compatibility and has the following "optional"
attach attributes:
* existing attach_flags, since it's not required when attaching in NONE
mode. Even though it's quite often used in MULTI and OVERRIDE mode it
seems to be a good idea to reduce number of arguments to
bpf_prog_attach_xattr;
* newly introduced attribute of BPF_PROG_ATTACH command: replace_prog_fd
that is fd of previously attached cgroup-bpf program to replace if
BPF_F_REPLACE flag is used.
The new function is named to be consistent with other xattr-functions
(bpf_prog_test_run_xattr, bpf_create_map_xattr, bpf_load_program_xattr).
The struct bpf_prog_attach_opts is supposed to be used with
DECLARE_LIBBPF_OPTS macro.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/lib/bpf/bpf.c | 17 ++++++++++++++++-
tools/lib/bpf/bpf.h | 11 +++++++++++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 98596e15390f..a787d53699c8 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -466,14 +466,29 @@ int bpf_obj_get(const char *pathname)
int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type,
unsigned int flags)
+{
+ DECLARE_LIBBPF_OPTS(bpf_prog_attach_opts, opts,
+ .flags = flags,
+ );
+
+ return bpf_prog_attach_xattr(prog_fd, target_fd, type, &opts);
+}
+
+int bpf_prog_attach_xattr(int prog_fd, int target_fd,
+ enum bpf_attach_type type,
+ const struct bpf_prog_attach_opts *opts)
{
union bpf_attr attr;
+ if (!OPTS_VALID(opts, bpf_prog_attach_opts))
+ return -EINVAL;
+
memset(&attr, 0, sizeof(attr));
attr.target_fd = target_fd;
attr.attach_bpf_fd = prog_fd;
attr.attach_type = type;
- attr.attach_flags = flags;
+ attr.attach_flags = OPTS_GET(opts, flags, 0);
+ attr.replace_bpf_fd = OPTS_GET(opts, replace_prog_fd, 0);
return sys_bpf(BPF_PROG_ATTACH, &attr, sizeof(attr));
}
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 269807ce9ef5..f0ab8519986e 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -126,8 +126,19 @@ LIBBPF_API int bpf_map_get_next_key(int fd, const void *key, void *next_key);
LIBBPF_API int bpf_map_freeze(int fd);
LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
LIBBPF_API int bpf_obj_get(const char *pathname);
+
+struct bpf_prog_attach_opts {
+ size_t sz; /* size of this struct for forward/backward compatibility */
+ unsigned int flags;
+ int replace_prog_fd;
+};
+#define bpf_prog_attach_opts__last_field replace_prog_fd
+
LIBBPF_API int bpf_prog_attach(int prog_fd, int attachable_fd,
enum bpf_attach_type type, unsigned int flags);
+LIBBPF_API int bpf_prog_attach_xattr(int prog_fd, int attachable_fd,
+ enum bpf_attach_type type,
+ const struct bpf_prog_attach_opts *opts);
LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
enum bpf_attach_type type);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index e3a471f38a71..e9713a574243 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -219,6 +219,7 @@ LIBBPF_0.0.7 {
bpf_object__detach_skeleton;
bpf_object__load_skeleton;
bpf_object__open_skeleton;
+ bpf_prog_attach_xattr;
bpf_program__attach;
bpf_program__name;
btf__align_of;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 5/6] selftests/bpf: Convert test_cgroup_attach to prog_tests
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
` (3 preceding siblings ...)
2019-12-19 7:44 ` [PATCH v4 bpf-next 4/6] libbpf: Introduce bpf_prog_attach_xattr Andrey Ignatov
@ 2019-12-19 7:44 ` Andrey Ignatov
2019-12-19 22:46 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 6/6] selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi Andrey Ignatov
2019-12-20 5:33 ` [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Alexei Starovoitov
6 siblings, 1 reply; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
Convert test_cgroup_attach to prog_tests.
This change does a lot of things but in many cases it's pretty expensive
to separate them, so they go in one commit. Nevertheless the logic is
ketp as is and changes made are just moving things around, simplifying
them (w/o changing the meaning of the tests) and making prog_tests
compatible:
* split the 3 tests in the file into 3 separate files in prog_tests/;
* rename the test functions to test_<file_base_name>;
* remove unused includes, constants, variables and functions from every
test;
* replace `if`-s with or `if (CHECK())` where additional context should
be logged and with `if (CHECK_FAIL())` where line number is enough;
* switch from `log_err()` to logging via `CHECK()`;
* replace `assert`-s with `CHECK_FAIL()` to avoid crashing the whole
test_progs if one assertion fails;
* replace cgroup_helpers with test__join_cgroup() in
cgroup_attach_override only, other tests need more fine-grained
control for cgroup creation/deletion so cgroup_helpers are still used
there;
* simplify cgroup_attach_autodetach by switching to easiest possible
program since this test doesn't really need such a complicated program
as cgroup_attach_multi does;
* remove test_cgroup_attach.c itself.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/.gitignore | 1 -
tools/testing/selftests/bpf/Makefile | 3 +-
.../bpf/prog_tests/cgroup_attach_autodetach.c | 111 ++++
.../bpf/prog_tests/cgroup_attach_multi.c | 238 ++++++++
.../bpf/prog_tests/cgroup_attach_override.c | 148 +++++
.../selftests/bpf/test_cgroup_attach.c | 571 ------------------
6 files changed, 498 insertions(+), 574 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
delete mode 100644 tools/testing/selftests/bpf/test_cgroup_attach.c
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index ce5af95ede42..b139b3d75ebb 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -21,7 +21,6 @@ test_lirc_mode2_user
get_cgroup_id_user
test_skb_cgroup_id_user
test_socket_cookie
-test_cgroup_attach
test_cgroup_storage
test_select_reuseport
test_flow_dissector
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c652bd84ef0e..866fc1cadd7c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -32,7 +32,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_cgroup_attach test_progs-no_alu32
+ test_progs-no_alu32
# Also test bpf-gcc, if present
ifneq ($(BPF_GCC),)
@@ -136,7 +136,6 @@ $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c
$(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
-$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
new file mode 100644
index 000000000000..5b13f2c6c402
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#include "cgroup_helpers.h"
+
+#define PING_CMD "ping -q -c1 -w1 127.0.0.1 > /dev/null"
+
+char bpf_log_buf[BPF_LOG_BUF_SIZE];
+
+static int prog_load(void)
+{
+ struct bpf_insn prog[] = {
+ BPF_MOV64_IMM(BPF_REG_0, 1), /* r0 = 1 */
+ BPF_EXIT_INSN(),
+ };
+ size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+
+ return bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
+ prog, insns_cnt, "GPL", 0,
+ bpf_log_buf, BPF_LOG_BUF_SIZE);
+}
+
+void test_cgroup_attach_autodetach(void)
+{
+ __u32 duration = 0, prog_cnt = 4, attach_flags;
+ int allow_prog[2] = {-1};
+ __u32 prog_ids[2] = {0};
+ void *ptr = NULL;
+ int cg = 0, i;
+ int attempts;
+
+ for (i = 0; i < ARRAY_SIZE(allow_prog); i++) {
+ allow_prog[i] = prog_load();
+ if (CHECK(allow_prog[i] < 0, "prog_load",
+ "verifier output:\n%s\n-------\n", bpf_log_buf))
+ goto err;
+ }
+
+ if (CHECK_FAIL(setup_cgroup_environment()))
+ goto err;
+
+ /* create a cgroup, attach two programs and remember their ids */
+ cg = create_and_get_cgroup("/cg_autodetach");
+ if (CHECK_FAIL(cg < 0))
+ goto err;
+
+ if (CHECK_FAIL(join_cgroup("/cg_autodetach")))
+ goto err;
+
+ for (i = 0; i < ARRAY_SIZE(allow_prog); i++)
+ if (CHECK(bpf_prog_attach(allow_prog[i], cg,
+ BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_MULTI),
+ "prog_attach", "prog[%d], errno=%d\n", i, errno))
+ goto err;
+
+ /* make sure that programs are attached and run some traffic */
+ if (CHECK(bpf_prog_query(cg, BPF_CGROUP_INET_EGRESS, 0, &attach_flags,
+ prog_ids, &prog_cnt),
+ "prog_query", "errno=%d\n", errno))
+ goto err;
+ if (CHECK_FAIL(system(PING_CMD)))
+ goto err;
+
+ /* allocate some memory (4Mb) to pin the original cgroup */
+ ptr = malloc(4 * (1 << 20));
+ if (CHECK_FAIL(!ptr))
+ goto err;
+
+ /* close programs and cgroup fd */
+ for (i = 0; i < ARRAY_SIZE(allow_prog); i++) {
+ close(allow_prog[i]);
+ allow_prog[i] = -1;
+ }
+
+ close(cg);
+ cg = 0;
+
+ /* leave the cgroup and remove it. don't detach programs */
+ cleanup_cgroup_environment();
+
+ /* wait for the asynchronous auto-detachment.
+ * wait for no more than 5 sec and give up.
+ */
+ for (i = 0; i < ARRAY_SIZE(prog_ids); i++) {
+ for (attempts = 5; attempts >= 0; attempts--) {
+ int fd = bpf_prog_get_fd_by_id(prog_ids[i]);
+
+ if (fd < 0)
+ break;
+
+ /* don't leave the fd open */
+ close(fd);
+
+ if (CHECK_FAIL(!attempts))
+ goto err;
+
+ sleep(1);
+ }
+ }
+
+err:
+ for (i = 0; i < ARRAY_SIZE(allow_prog); i++)
+ if (allow_prog[i] >= 0)
+ close(allow_prog[i]);
+ if (cg)
+ close(cg);
+ free(ptr);
+ cleanup_cgroup_environment();
+}
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
new file mode 100644
index 000000000000..4eaab7435044
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#include "cgroup_helpers.h"
+
+#define PING_CMD "ping -q -c1 -w1 127.0.0.1 > /dev/null"
+
+char bpf_log_buf[BPF_LOG_BUF_SIZE];
+
+static int map_fd = -1;
+
+static int prog_load_cnt(int verdict, int val)
+{
+ int cgroup_storage_fd, percpu_cgroup_storage_fd;
+
+ if (map_fd < 0)
+ map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, 4, 8, 1, 0);
+ if (map_fd < 0) {
+ printf("failed to create map '%s'\n", strerror(errno));
+ return -1;
+ }
+
+ cgroup_storage_fd = bpf_create_map(BPF_MAP_TYPE_CGROUP_STORAGE,
+ sizeof(struct bpf_cgroup_storage_key), 8, 0, 0);
+ if (cgroup_storage_fd < 0) {
+ printf("failed to create map '%s'\n", strerror(errno));
+ return -1;
+ }
+
+ percpu_cgroup_storage_fd = bpf_create_map(
+ BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
+ sizeof(struct bpf_cgroup_storage_key), 8, 0, 0);
+ if (percpu_cgroup_storage_fd < 0) {
+ printf("failed to create map '%s'\n", strerror(errno));
+ return -1;
+ }
+
+ struct bpf_insn prog[] = {
+ BPF_MOV32_IMM(BPF_REG_0, 0),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = fp - 4 */
+ BPF_LD_MAP_FD(BPF_REG_1, map_fd),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_1, val), /* r1 = 1 */
+ BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
+
+ BPF_LD_MAP_FD(BPF_REG_1, cgroup_storage_fd),
+ BPF_MOV64_IMM(BPF_REG_2, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
+ BPF_MOV64_IMM(BPF_REG_1, val),
+ BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_W, BPF_REG_0, BPF_REG_1, 0, 0),
+
+ BPF_LD_MAP_FD(BPF_REG_1, percpu_cgroup_storage_fd),
+ BPF_MOV64_IMM(BPF_REG_2, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 0x1),
+ BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_3, 0),
+
+ BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
+ BPF_EXIT_INSN(),
+ };
+ size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+ int ret;
+
+ ret = bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
+ prog, insns_cnt, "GPL", 0,
+ bpf_log_buf, BPF_LOG_BUF_SIZE);
+
+ close(cgroup_storage_fd);
+ return ret;
+}
+
+void test_cgroup_attach_multi(void)
+{
+ __u32 prog_ids[4], prog_cnt = 0, attach_flags, saved_prog_id;
+ int cg1 = 0, cg2 = 0, cg3 = 0, cg4 = 0, cg5 = 0, key = 0;
+ int allow_prog[6] = {-1};
+ unsigned long long value;
+ __u32 duration = 0;
+ int i = 0;
+
+ for (i = 0; i < ARRAY_SIZE(allow_prog); i++) {
+ allow_prog[i] = prog_load_cnt(1, 1 << i);
+ if (CHECK(allow_prog[i] < 0, "prog_load",
+ "verifier output:\n%s\n-------\n", bpf_log_buf))
+ goto err;
+ }
+
+ if (CHECK_FAIL(setup_cgroup_environment()))
+ goto err;
+
+ cg1 = create_and_get_cgroup("/cg1");
+ if (CHECK_FAIL(cg1 < 0))
+ goto err;
+ cg2 = create_and_get_cgroup("/cg1/cg2");
+ if (CHECK_FAIL(cg2 < 0))
+ goto err;
+ cg3 = create_and_get_cgroup("/cg1/cg2/cg3");
+ if (CHECK_FAIL(cg3 < 0))
+ goto err;
+ cg4 = create_and_get_cgroup("/cg1/cg2/cg3/cg4");
+ if (CHECK_FAIL(cg4 < 0))
+ goto err;
+ cg5 = create_and_get_cgroup("/cg1/cg2/cg3/cg4/cg5");
+ if (CHECK_FAIL(cg5 < 0))
+ goto err;
+
+ if (CHECK_FAIL(join_cgroup("/cg1/cg2/cg3/cg4/cg5")))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog[0], cg1, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_MULTI),
+ "prog0_attach_to_cg1_multi", "errno=%d\n", errno))
+ goto err;
+
+ if (CHECK(!bpf_prog_attach(allow_prog[0], cg1, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_MULTI),
+ "fail_same_prog_attach_to_cg1", "unexpected success\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog[1], cg1, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_MULTI),
+ "prog1_attach_to_cg1_multi", "errno=%d\n", errno))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog[2], cg2, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "prog2_attach_to_cg2_override", "errno=%d\n", errno))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog[3], cg3, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_MULTI),
+ "prog3_attach_to_cg3_multi", "errno=%d\n", errno))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog[4], cg4, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "prog4_attach_to_cg4_override", "errno=%d\n", errno))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog[5], cg5, BPF_CGROUP_INET_EGRESS, 0),
+ "prog5_attach_to_cg5_none", "errno=%d\n", errno))
+ goto err;
+
+ CHECK_FAIL(system(PING_CMD));
+ CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
+ CHECK_FAIL(value != 1 + 2 + 8 + 32);
+
+ /* query the number of effective progs in cg5 */
+ CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS,
+ BPF_F_QUERY_EFFECTIVE, NULL, NULL, &prog_cnt));
+ CHECK_FAIL(prog_cnt != 4);
+ /* retrieve prog_ids of effective progs in cg5 */
+ CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS,
+ BPF_F_QUERY_EFFECTIVE, &attach_flags,
+ prog_ids, &prog_cnt));
+ CHECK_FAIL(prog_cnt != 4);
+ CHECK_FAIL(attach_flags != 0);
+ saved_prog_id = prog_ids[0];
+ /* check enospc handling */
+ prog_ids[0] = 0;
+ prog_cnt = 2;
+ CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS,
+ BPF_F_QUERY_EFFECTIVE, &attach_flags,
+ prog_ids, &prog_cnt) != -1);
+ CHECK_FAIL(errno != ENOSPC);
+ CHECK_FAIL(prog_cnt != 4);
+ /* check that prog_ids are returned even when buffer is too small */
+ CHECK_FAIL(prog_ids[0] != saved_prog_id);
+ /* retrieve prog_id of single attached prog in cg5 */
+ prog_ids[0] = 0;
+ CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, 0, NULL,
+ prog_ids, &prog_cnt));
+ CHECK_FAIL(prog_cnt != 1);
+ CHECK_FAIL(prog_ids[0] != saved_prog_id);
+
+ /* detach bottom program and ping again */
+ if (CHECK(bpf_prog_detach2(-1, cg5, BPF_CGROUP_INET_EGRESS),
+ "prog_detach_from_cg5", "errno=%d\n", errno))
+ goto err;
+
+ value = 0;
+ CHECK_FAIL(bpf_map_update_elem(map_fd, &key, &value, 0));
+ CHECK_FAIL(system(PING_CMD));
+ CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
+ CHECK_FAIL(value != 1 + 2 + 8 + 16);
+
+ /* detach 3rd from bottom program and ping again */
+ if (CHECK(!bpf_prog_detach2(0, cg3, BPF_CGROUP_INET_EGRESS),
+ "fail_prog_detach_from_cg3", "unexpected success\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_detach2(allow_prog[3], cg3, BPF_CGROUP_INET_EGRESS),
+ "prog3_detach_from_cg3", "errno=%d\n", errno))
+ goto err;
+
+ value = 0;
+ CHECK_FAIL(bpf_map_update_elem(map_fd, &key, &value, 0));
+ CHECK_FAIL(system(PING_CMD));
+ CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
+ CHECK_FAIL(value != 1 + 2 + 16);
+
+ /* detach 2nd from bottom program and ping again */
+ if (CHECK(bpf_prog_detach2(-1, cg4, BPF_CGROUP_INET_EGRESS),
+ "prog_detach_from_cg4", "errno=%d\n", errno))
+ goto err;
+
+ value = 0;
+ CHECK_FAIL(bpf_map_update_elem(map_fd, &key, &value, 0));
+ CHECK_FAIL(system(PING_CMD));
+ CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
+ CHECK_FAIL(value != 1 + 2 + 4);
+
+ prog_cnt = 4;
+ CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS,
+ BPF_F_QUERY_EFFECTIVE, &attach_flags,
+ prog_ids, &prog_cnt));
+ CHECK_FAIL(prog_cnt != 3);
+ CHECK_FAIL(attach_flags != 0);
+ CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, 0, NULL,
+ prog_ids, &prog_cnt));
+ CHECK_FAIL(prog_cnt != 0);
+
+err:
+ for (i = 0; i < ARRAY_SIZE(allow_prog); i++)
+ if (allow_prog[i] >= 0)
+ close(allow_prog[i]);
+ close(cg1);
+ close(cg2);
+ close(cg3);
+ close(cg4);
+ close(cg5);
+ cleanup_cgroup_environment();
+}
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
new file mode 100644
index 000000000000..9d8cb48b99de
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#include "cgroup_helpers.h"
+
+#define FOO "/foo"
+#define BAR "/foo/bar/"
+#define PING_CMD "ping -q -c1 -w1 127.0.0.1 > /dev/null"
+
+char bpf_log_buf[BPF_LOG_BUF_SIZE];
+
+static int prog_load(int verdict)
+{
+ struct bpf_insn prog[] = {
+ BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
+ BPF_EXIT_INSN(),
+ };
+ size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+
+ return bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
+ prog, insns_cnt, "GPL", 0,
+ bpf_log_buf, BPF_LOG_BUF_SIZE);
+}
+
+void test_cgroup_attach_override(void)
+{
+ int drop_prog = -1, allow_prog = -1, foo = -1, bar = -1;
+ __u32 duration = 0;
+
+ allow_prog = prog_load(1);
+ if (CHECK(allow_prog < 0, "prog_load_allow",
+ "verifier output:\n%s\n-------\n", bpf_log_buf))
+ goto err;
+
+ drop_prog = prog_load(0);
+ if (CHECK(drop_prog < 0, "prog_load_drop",
+ "verifier output:\n%s\n-------\n", bpf_log_buf))
+ goto err;
+
+ foo = test__join_cgroup(FOO);
+ if (CHECK(foo < 0, "cgroup_join_foo", "cgroup setup failed\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(drop_prog, foo, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "prog_attach_drop_foo_override",
+ "attach prog to %s failed, errno=%d\n", FOO, errno))
+ goto err;
+
+ if (CHECK(!system(PING_CMD), "ping_fail",
+ "ping unexpectedly succeeded\n"))
+ goto err;
+
+ bar = test__join_cgroup(BAR);
+ if (CHECK(bar < 0, "cgroup_join_bar", "cgroup setup failed\n"))
+ goto err;
+
+ if (CHECK(!system(PING_CMD), "ping_fail",
+ "ping unexpectedly succeeded\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "prog_attach_allow_bar_override",
+ "attach prog to %s failed, errno=%d\n", BAR, errno))
+ goto err;
+
+ if (CHECK(system(PING_CMD), "ping_ok", "ping failed\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_detach(bar, BPF_CGROUP_INET_EGRESS),
+ "prog_detach_bar",
+ "detach prog from %s failed, errno=%d\n", BAR, errno))
+ goto err;
+
+ if (CHECK(!system(PING_CMD), "ping_fail",
+ "ping unexpectedly succeeded\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "prog_attach_allow_bar_override",
+ "attach prog to %s failed, errno=%d\n", BAR, errno))
+ goto err;
+
+ if (CHECK(bpf_prog_detach(foo, BPF_CGROUP_INET_EGRESS),
+ "prog_detach_foo",
+ "detach prog from %s failed, errno=%d\n", FOO, errno))
+ goto err;
+
+ if (CHECK(system(PING_CMD), "ping_ok", "ping failed\n"))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "prog_attach_allow_bar_override",
+ "attach prog to %s failed, errno=%d\n", BAR, errno))
+ goto err;
+
+ if (CHECK(!bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS, 0),
+ "fail_prog_attach_allow_bar_none",
+ "attach prog to %s unexpectedly succeeded\n", BAR))
+ goto err;
+
+ if (CHECK(bpf_prog_detach(bar, BPF_CGROUP_INET_EGRESS),
+ "prog_detach_bar",
+ "detach prog from %s failed, errno=%d\n", BAR, errno))
+ goto err;
+
+ if (CHECK(!bpf_prog_detach(foo, BPF_CGROUP_INET_EGRESS),
+ "fail_prog_detach_foo",
+ "double detach from %s unexpectedly succeeded\n", FOO))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(allow_prog, foo, BPF_CGROUP_INET_EGRESS, 0),
+ "prog_attach_allow_foo_none",
+ "attach prog to %s failed, errno=%d\n", FOO, errno))
+ goto err;
+
+ if (CHECK(!bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS, 0),
+ "fail_prog_attach_allow_bar_none",
+ "attach prog to %s unexpectedly succeeded\n", BAR))
+ goto err;
+
+ if (CHECK(!bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "fail_prog_attach_allow_bar_override",
+ "attach prog to %s unexpectedly succeeded\n", BAR))
+ goto err;
+
+ if (CHECK(!bpf_prog_attach(allow_prog, foo, BPF_CGROUP_INET_EGRESS,
+ BPF_F_ALLOW_OVERRIDE),
+ "fail_prog_attach_allow_foo_override",
+ "attach prog to %s unexpectedly succeeded\n", FOO))
+ goto err;
+
+ if (CHECK(bpf_prog_attach(drop_prog, foo, BPF_CGROUP_INET_EGRESS, 0),
+ "prog_attach_drop_foo_none",
+ "attach prog to %s failed, errno=%d\n", FOO, errno))
+ goto err;
+
+err:
+ close(foo);
+ close(bar);
+ close(allow_prog);
+ close(drop_prog);
+}
diff --git a/tools/testing/selftests/bpf/test_cgroup_attach.c b/tools/testing/selftests/bpf/test_cgroup_attach.c
deleted file mode 100644
index 7671909ee1cb..000000000000
--- a/tools/testing/selftests/bpf/test_cgroup_attach.c
+++ /dev/null
@@ -1,571 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-/* eBPF example program:
- *
- * - Creates arraymap in kernel with 4 bytes keys and 8 byte values
- *
- * - Loads eBPF program
- *
- * The eBPF program accesses the map passed in to store two pieces of
- * information. The number of invocations of the program, which maps
- * to the number of packets received, is stored to key 0. Key 1 is
- * incremented on each iteration by the number of bytes stored in
- * the skb. The program also stores the number of received bytes
- * in the cgroup storage.
- *
- * - Attaches the new program to a cgroup using BPF_PROG_ATTACH
- *
- * - Every second, reads map[0] and map[1] to see how many bytes and
- * packets were seen on any socket of tasks in the given cgroup.
- */
-
-#define _GNU_SOURCE
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <assert.h>
-#include <sys/resource.h>
-#include <sys/time.h>
-#include <unistd.h>
-#include <linux/filter.h>
-
-#include <linux/bpf.h>
-#include <bpf/bpf.h>
-
-#include "bpf_util.h"
-#include "bpf_rlimit.h"
-#include "cgroup_helpers.h"
-
-#define FOO "/foo"
-#define BAR "/foo/bar/"
-#define PING_CMD "ping -q -c1 -w1 127.0.0.1 > /dev/null"
-
-char bpf_log_buf[BPF_LOG_BUF_SIZE];
-
-#ifdef DEBUG
-#define debug(args...) printf(args)
-#else
-#define debug(args...)
-#endif
-
-static int prog_load(int verdict)
-{
- int ret;
- struct bpf_insn prog[] = {
- BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
- BPF_EXIT_INSN(),
- };
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
-
- ret = bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
- prog, insns_cnt, "GPL", 0,
- bpf_log_buf, BPF_LOG_BUF_SIZE);
-
- if (ret < 0) {
- log_err("Loading program");
- printf("Output from verifier:\n%s\n-------\n", bpf_log_buf);
- return 0;
- }
- return ret;
-}
-
-static int test_foo_bar(void)
-{
- int drop_prog, allow_prog, foo = 0, bar = 0, rc = 0;
-
- allow_prog = prog_load(1);
- if (!allow_prog)
- goto err;
-
- drop_prog = prog_load(0);
- if (!drop_prog)
- goto err;
-
- if (setup_cgroup_environment())
- goto err;
-
- /* Create cgroup /foo, get fd, and join it */
- foo = create_and_get_cgroup(FOO);
- if (foo < 0)
- goto err;
-
- if (join_cgroup(FOO))
- goto err;
-
- if (bpf_prog_attach(drop_prog, foo, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- log_err("Attaching prog to /foo");
- goto err;
- }
-
- debug("Attached DROP prog. This ping in cgroup /foo should fail...\n");
- assert(system(PING_CMD) != 0);
-
- /* Create cgroup /foo/bar, get fd, and join it */
- bar = create_and_get_cgroup(BAR);
- if (bar < 0)
- goto err;
-
- if (join_cgroup(BAR))
- goto err;
-
- debug("Attached DROP prog. This ping in cgroup /foo/bar should fail...\n");
- assert(system(PING_CMD) != 0);
-
- if (bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- log_err("Attaching prog to /foo/bar");
- goto err;
- }
-
- debug("Attached PASS prog. This ping in cgroup /foo/bar should pass...\n");
- assert(system(PING_CMD) == 0);
-
- if (bpf_prog_detach(bar, BPF_CGROUP_INET_EGRESS)) {
- log_err("Detaching program from /foo/bar");
- goto err;
- }
-
- debug("Detached PASS from /foo/bar while DROP is attached to /foo.\n"
- "This ping in cgroup /foo/bar should fail...\n");
- assert(system(PING_CMD) != 0);
-
- if (bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- log_err("Attaching prog to /foo/bar");
- goto err;
- }
-
- if (bpf_prog_detach(foo, BPF_CGROUP_INET_EGRESS)) {
- log_err("Detaching program from /foo");
- goto err;
- }
-
- debug("Attached PASS from /foo/bar and detached DROP from /foo.\n"
- "This ping in cgroup /foo/bar should pass...\n");
- assert(system(PING_CMD) == 0);
-
- if (bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- log_err("Attaching prog to /foo/bar");
- goto err;
- }
-
- if (!bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS, 0)) {
- errno = 0;
- log_err("Unexpected success attaching prog to /foo/bar");
- goto err;
- }
-
- if (bpf_prog_detach(bar, BPF_CGROUP_INET_EGRESS)) {
- log_err("Detaching program from /foo/bar");
- goto err;
- }
-
- if (!bpf_prog_detach(foo, BPF_CGROUP_INET_EGRESS)) {
- errno = 0;
- log_err("Unexpected success in double detach from /foo");
- goto err;
- }
-
- if (bpf_prog_attach(allow_prog, foo, BPF_CGROUP_INET_EGRESS, 0)) {
- log_err("Attaching non-overridable prog to /foo");
- goto err;
- }
-
- if (!bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS, 0)) {
- errno = 0;
- log_err("Unexpected success attaching non-overridable prog to /foo/bar");
- goto err;
- }
-
- if (!bpf_prog_attach(allow_prog, bar, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- errno = 0;
- log_err("Unexpected success attaching overridable prog to /foo/bar");
- goto err;
- }
-
- if (!bpf_prog_attach(allow_prog, foo, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- errno = 0;
- log_err("Unexpected success attaching overridable prog to /foo");
- goto err;
- }
-
- if (bpf_prog_attach(drop_prog, foo, BPF_CGROUP_INET_EGRESS, 0)) {
- log_err("Attaching different non-overridable prog to /foo");
- goto err;
- }
-
- goto out;
-
-err:
- rc = 1;
-
-out:
- close(foo);
- close(bar);
- cleanup_cgroup_environment();
- if (!rc)
- printf("#override:PASS\n");
- else
- printf("#override:FAIL\n");
- return rc;
-}
-
-static int map_fd = -1;
-
-static int prog_load_cnt(int verdict, int val)
-{
- int cgroup_storage_fd, percpu_cgroup_storage_fd;
-
- if (map_fd < 0)
- map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, 4, 8, 1, 0);
- if (map_fd < 0) {
- printf("failed to create map '%s'\n", strerror(errno));
- return -1;
- }
-
- cgroup_storage_fd = bpf_create_map(BPF_MAP_TYPE_CGROUP_STORAGE,
- sizeof(struct bpf_cgroup_storage_key), 8, 0, 0);
- if (cgroup_storage_fd < 0) {
- printf("failed to create map '%s'\n", strerror(errno));
- return -1;
- }
-
- percpu_cgroup_storage_fd = bpf_create_map(
- BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
- sizeof(struct bpf_cgroup_storage_key), 8, 0, 0);
- if (percpu_cgroup_storage_fd < 0) {
- printf("failed to create map '%s'\n", strerror(errno));
- return -1;
- }
-
- struct bpf_insn prog[] = {
- BPF_MOV32_IMM(BPF_REG_0, 0),
- BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
- BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = fp - 4 */
- BPF_LD_MAP_FD(BPF_REG_1, map_fd),
- BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
- BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
- BPF_MOV64_IMM(BPF_REG_1, val), /* r1 = 1 */
- BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
-
- BPF_LD_MAP_FD(BPF_REG_1, cgroup_storage_fd),
- BPF_MOV64_IMM(BPF_REG_2, 0),
- BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
- BPF_MOV64_IMM(BPF_REG_1, val),
- BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_W, BPF_REG_0, BPF_REG_1, 0, 0),
-
- BPF_LD_MAP_FD(BPF_REG_1, percpu_cgroup_storage_fd),
- BPF_MOV64_IMM(BPF_REG_2, 0),
- BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
- BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 0x1),
- BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_3, 0),
-
- BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
- BPF_EXIT_INSN(),
- };
- size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
- int ret;
-
- ret = bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
- prog, insns_cnt, "GPL", 0,
- bpf_log_buf, BPF_LOG_BUF_SIZE);
-
- if (ret < 0) {
- log_err("Loading program");
- printf("Output from verifier:\n%s\n-------\n", bpf_log_buf);
- return 0;
- }
- close(cgroup_storage_fd);
- return ret;
-}
-
-
-static int test_multiprog(void)
-{
- __u32 prog_ids[4], prog_cnt = 0, attach_flags, saved_prog_id;
- int cg1 = 0, cg2 = 0, cg3 = 0, cg4 = 0, cg5 = 0, key = 0;
- int drop_prog, allow_prog[6] = {}, rc = 0;
- unsigned long long value;
- int i = 0;
-
- for (i = 0; i < 6; i++) {
- allow_prog[i] = prog_load_cnt(1, 1 << i);
- if (!allow_prog[i])
- goto err;
- }
- drop_prog = prog_load_cnt(0, 1);
- if (!drop_prog)
- goto err;
-
- if (setup_cgroup_environment())
- goto err;
-
- cg1 = create_and_get_cgroup("/cg1");
- if (cg1 < 0)
- goto err;
- cg2 = create_and_get_cgroup("/cg1/cg2");
- if (cg2 < 0)
- goto err;
- cg3 = create_and_get_cgroup("/cg1/cg2/cg3");
- if (cg3 < 0)
- goto err;
- cg4 = create_and_get_cgroup("/cg1/cg2/cg3/cg4");
- if (cg4 < 0)
- goto err;
- cg5 = create_and_get_cgroup("/cg1/cg2/cg3/cg4/cg5");
- if (cg5 < 0)
- goto err;
-
- if (join_cgroup("/cg1/cg2/cg3/cg4/cg5"))
- goto err;
-
- if (bpf_prog_attach(allow_prog[0], cg1, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_MULTI)) {
- log_err("Attaching prog to cg1");
- goto err;
- }
- if (!bpf_prog_attach(allow_prog[0], cg1, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_MULTI)) {
- log_err("Unexpected success attaching the same prog to cg1");
- goto err;
- }
- if (bpf_prog_attach(allow_prog[1], cg1, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_MULTI)) {
- log_err("Attaching prog2 to cg1");
- goto err;
- }
- if (bpf_prog_attach(allow_prog[2], cg2, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- log_err("Attaching prog to cg2");
- goto err;
- }
- if (bpf_prog_attach(allow_prog[3], cg3, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_MULTI)) {
- log_err("Attaching prog to cg3");
- goto err;
- }
- if (bpf_prog_attach(allow_prog[4], cg4, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_OVERRIDE)) {
- log_err("Attaching prog to cg4");
- goto err;
- }
- if (bpf_prog_attach(allow_prog[5], cg5, BPF_CGROUP_INET_EGRESS, 0)) {
- log_err("Attaching prog to cg5");
- goto err;
- }
- assert(system(PING_CMD) == 0);
- assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
- assert(value == 1 + 2 + 8 + 32);
-
- /* query the number of effective progs in cg5 */
- assert(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, BPF_F_QUERY_EFFECTIVE,
- NULL, NULL, &prog_cnt) == 0);
- assert(prog_cnt == 4);
- /* retrieve prog_ids of effective progs in cg5 */
- assert(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, BPF_F_QUERY_EFFECTIVE,
- &attach_flags, prog_ids, &prog_cnt) == 0);
- assert(prog_cnt == 4);
- assert(attach_flags == 0);
- saved_prog_id = prog_ids[0];
- /* check enospc handling */
- prog_ids[0] = 0;
- prog_cnt = 2;
- assert(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, BPF_F_QUERY_EFFECTIVE,
- &attach_flags, prog_ids, &prog_cnt) == -1 &&
- errno == ENOSPC);
- assert(prog_cnt == 4);
- /* check that prog_ids are returned even when buffer is too small */
- assert(prog_ids[0] == saved_prog_id);
- /* retrieve prog_id of single attached prog in cg5 */
- prog_ids[0] = 0;
- assert(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, 0,
- NULL, prog_ids, &prog_cnt) == 0);
- assert(prog_cnt == 1);
- assert(prog_ids[0] == saved_prog_id);
-
- /* detach bottom program and ping again */
- if (bpf_prog_detach2(-1, cg5, BPF_CGROUP_INET_EGRESS)) {
- log_err("Detaching prog from cg5");
- goto err;
- }
- value = 0;
- assert(bpf_map_update_elem(map_fd, &key, &value, 0) == 0);
- assert(system(PING_CMD) == 0);
- assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
- assert(value == 1 + 2 + 8 + 16);
-
- /* detach 3rd from bottom program and ping again */
- errno = 0;
- if (!bpf_prog_detach2(0, cg3, BPF_CGROUP_INET_EGRESS)) {
- log_err("Unexpected success on detach from cg3");
- goto err;
- }
- if (bpf_prog_detach2(allow_prog[3], cg3, BPF_CGROUP_INET_EGRESS)) {
- log_err("Detaching from cg3");
- goto err;
- }
- value = 0;
- assert(bpf_map_update_elem(map_fd, &key, &value, 0) == 0);
- assert(system(PING_CMD) == 0);
- assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
- assert(value == 1 + 2 + 16);
-
- /* detach 2nd from bottom program and ping again */
- if (bpf_prog_detach2(-1, cg4, BPF_CGROUP_INET_EGRESS)) {
- log_err("Detaching prog from cg4");
- goto err;
- }
- value = 0;
- assert(bpf_map_update_elem(map_fd, &key, &value, 0) == 0);
- assert(system(PING_CMD) == 0);
- assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
- assert(value == 1 + 2 + 4);
-
- prog_cnt = 4;
- assert(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, BPF_F_QUERY_EFFECTIVE,
- &attach_flags, prog_ids, &prog_cnt) == 0);
- assert(prog_cnt == 3);
- assert(attach_flags == 0);
- assert(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS, 0,
- NULL, prog_ids, &prog_cnt) == 0);
- assert(prog_cnt == 0);
- goto out;
-err:
- rc = 1;
-
-out:
- for (i = 0; i < 6; i++)
- if (allow_prog[i] > 0)
- close(allow_prog[i]);
- close(cg1);
- close(cg2);
- close(cg3);
- close(cg4);
- close(cg5);
- cleanup_cgroup_environment();
- if (!rc)
- printf("#multi:PASS\n");
- else
- printf("#multi:FAIL\n");
- return rc;
-}
-
-static int test_autodetach(void)
-{
- __u32 prog_cnt = 4, attach_flags;
- int allow_prog[2] = {0};
- __u32 prog_ids[2] = {0};
- int cg = 0, i, rc = -1;
- void *ptr = NULL;
- int attempts;
-
- for (i = 0; i < ARRAY_SIZE(allow_prog); i++) {
- allow_prog[i] = prog_load_cnt(1, 1 << i);
- if (!allow_prog[i])
- goto err;
- }
-
- if (setup_cgroup_environment())
- goto err;
-
- /* create a cgroup, attach two programs and remember their ids */
- cg = create_and_get_cgroup("/cg_autodetach");
- if (cg < 0)
- goto err;
-
- if (join_cgroup("/cg_autodetach"))
- goto err;
-
- for (i = 0; i < ARRAY_SIZE(allow_prog); i++) {
- if (bpf_prog_attach(allow_prog[i], cg, BPF_CGROUP_INET_EGRESS,
- BPF_F_ALLOW_MULTI)) {
- log_err("Attaching prog[%d] to cg:egress", i);
- goto err;
- }
- }
-
- /* make sure that programs are attached and run some traffic */
- assert(bpf_prog_query(cg, BPF_CGROUP_INET_EGRESS, 0, &attach_flags,
- prog_ids, &prog_cnt) == 0);
- assert(system(PING_CMD) == 0);
-
- /* allocate some memory (4Mb) to pin the original cgroup */
- ptr = malloc(4 * (1 << 20));
- if (!ptr)
- goto err;
-
- /* close programs and cgroup fd */
- for (i = 0; i < ARRAY_SIZE(allow_prog); i++) {
- close(allow_prog[i]);
- allow_prog[i] = 0;
- }
-
- close(cg);
- cg = 0;
-
- /* leave the cgroup and remove it. don't detach programs */
- cleanup_cgroup_environment();
-
- /* wait for the asynchronous auto-detachment.
- * wait for no more than 5 sec and give up.
- */
- for (i = 0; i < ARRAY_SIZE(prog_ids); i++) {
- for (attempts = 5; attempts >= 0; attempts--) {
- int fd = bpf_prog_get_fd_by_id(prog_ids[i]);
-
- if (fd < 0)
- break;
-
- /* don't leave the fd open */
- close(fd);
-
- if (!attempts)
- goto err;
-
- sleep(1);
- }
- }
-
- rc = 0;
-err:
- for (i = 0; i < ARRAY_SIZE(allow_prog); i++)
- if (allow_prog[i] > 0)
- close(allow_prog[i]);
- if (cg)
- close(cg);
- free(ptr);
- cleanup_cgroup_environment();
- if (!rc)
- printf("#autodetach:PASS\n");
- else
- printf("#autodetach:FAIL\n");
- return rc;
-}
-
-int main(void)
-{
- int (*tests[])(void) = {
- test_foo_bar,
- test_multiprog,
- test_autodetach,
- };
- int errors = 0;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(tests); i++)
- if (tests[i]())
- errors++;
-
- if (errors)
- printf("test_cgroup_attach:FAIL\n");
- else
- printf("test_cgroup_attach:PASS\n");
-
- return errors ? EXIT_FAILURE : EXIT_SUCCESS;
-}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 bpf-next 6/6] selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
` (4 preceding siblings ...)
2019-12-19 7:44 ` [PATCH v4 bpf-next 5/6] selftests/bpf: Convert test_cgroup_attach to prog_tests Andrey Ignatov
@ 2019-12-19 7:44 ` Andrey Ignatov
2019-12-19 22:48 ` Andrii Nakryiko
2019-12-20 5:33 ` [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Alexei Starovoitov
6 siblings, 1 reply; 13+ messages in thread
From: Andrey Ignatov @ 2019-12-19 7:44 UTC (permalink / raw)
To: bpf; +Cc: Andrey Ignatov, ast, daniel, kafai, andriin, kernel-team
Test replacing a cgroup-bpf program attached with BPF_F_ALLOW_MULTI and
possible failure modes: invalid combination of flags, invalid
replace_bpf_fd, replacing a non-attachd to specified cgroup program.
Example of program replacing:
# gdb -q --args ./test_progs --name=cgroup_attach_multi
...
Breakpoint 1, test_cgroup_attach_multi () at cgroup_attach_multi.c:227
(gdb)
[1]+ Stopped gdb -q --args ./test_progs --name=cgroup_attach_multi
# bpftool c s /mnt/cgroup2/cgroup-test-work-dir/cg1
ID AttachType AttachFlags Name
2133 egress multi
2134 egress multi
# fg
gdb -q --args ./test_progs --name=cgroup_attach_multi
(gdb) c
Continuing.
Breakpoint 2, test_cgroup_attach_multi () at cgroup_attach_multi.c:233
(gdb)
[1]+ Stopped gdb -q --args ./test_progs --name=cgroup_attach_multi
# bpftool c s /mnt/cgroup2/cgroup-test-work-dir/cg1
ID AttachType AttachFlags Name
2139 egress multi
2134 egress multi
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
.../bpf/prog_tests/cgroup_attach_multi.c | 53 +++++++++++++++++--
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
index 4eaab7435044..2ff21dbce179 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
@@ -78,7 +78,8 @@ void test_cgroup_attach_multi(void)
{
__u32 prog_ids[4], prog_cnt = 0, attach_flags, saved_prog_id;
int cg1 = 0, cg2 = 0, cg3 = 0, cg4 = 0, cg5 = 0, key = 0;
- int allow_prog[6] = {-1};
+ DECLARE_LIBBPF_OPTS(bpf_prog_attach_opts, attach_opts);
+ int allow_prog[7] = {-1};
unsigned long long value;
__u32 duration = 0;
int i = 0;
@@ -189,6 +190,52 @@ void test_cgroup_attach_multi(void)
CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
CHECK_FAIL(value != 1 + 2 + 8 + 16);
+ /* test replace */
+
+ attach_opts.flags = BPF_F_ALLOW_OVERRIDE | BPF_F_REPLACE;
+ attach_opts.replace_prog_fd = allow_prog[0];
+ if (CHECK(!bpf_prog_attach_xattr(allow_prog[6], cg1,
+ BPF_CGROUP_INET_EGRESS, &attach_opts),
+ "fail_prog_replace_override", "unexpected success\n"))
+ goto err;
+ CHECK_FAIL(errno != EINVAL);
+
+ attach_opts.flags = BPF_F_REPLACE;
+ if (CHECK(!bpf_prog_attach_xattr(allow_prog[6], cg1,
+ BPF_CGROUP_INET_EGRESS, &attach_opts),
+ "fail_prog_replace_no_multi", "unexpected success\n"))
+ goto err;
+ CHECK_FAIL(errno != EINVAL);
+
+ attach_opts.flags = BPF_F_ALLOW_MULTI | BPF_F_REPLACE;
+ attach_opts.replace_prog_fd = -1;
+ if (CHECK(!bpf_prog_attach_xattr(allow_prog[6], cg1,
+ BPF_CGROUP_INET_EGRESS, &attach_opts),
+ "fail_prog_replace_bad_fd", "unexpected success\n"))
+ goto err;
+ CHECK_FAIL(errno != EBADF);
+
+ /* replacing a program that is not attached to cgroup should fail */
+ attach_opts.replace_prog_fd = allow_prog[3];
+ if (CHECK(!bpf_prog_attach_xattr(allow_prog[6], cg1,
+ BPF_CGROUP_INET_EGRESS, &attach_opts),
+ "fail_prog_replace_no_ent", "unexpected success\n"))
+ goto err;
+ CHECK_FAIL(errno != ENOENT);
+
+ /* replace 1st from the top program */
+ attach_opts.replace_prog_fd = allow_prog[0];
+ if (CHECK(bpf_prog_attach_xattr(allow_prog[6], cg1,
+ BPF_CGROUP_INET_EGRESS, &attach_opts),
+ "prog_replace", "errno=%d\n", errno))
+ goto err;
+
+ value = 0;
+ CHECK_FAIL(bpf_map_update_elem(map_fd, &key, &value, 0));
+ CHECK_FAIL(system(PING_CMD));
+ CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
+ CHECK_FAIL(value != 64 + 2 + 8 + 16);
+
/* detach 3rd from bottom program and ping again */
if (CHECK(!bpf_prog_detach2(0, cg3, BPF_CGROUP_INET_EGRESS),
"fail_prog_detach_from_cg3", "unexpected success\n"))
@@ -202,7 +249,7 @@ void test_cgroup_attach_multi(void)
CHECK_FAIL(bpf_map_update_elem(map_fd, &key, &value, 0));
CHECK_FAIL(system(PING_CMD));
CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
- CHECK_FAIL(value != 1 + 2 + 16);
+ CHECK_FAIL(value != 64 + 2 + 16);
/* detach 2nd from bottom program and ping again */
if (CHECK(bpf_prog_detach2(-1, cg4, BPF_CGROUP_INET_EGRESS),
@@ -213,7 +260,7 @@ void test_cgroup_attach_multi(void)
CHECK_FAIL(bpf_map_update_elem(map_fd, &key, &value, 0));
CHECK_FAIL(system(PING_CMD));
CHECK_FAIL(bpf_map_lookup_elem(map_fd, &key, &value));
- CHECK_FAIL(value != 1 + 2 + 4);
+ CHECK_FAIL(value != 64 + 2 + 4);
prog_cnt = 4;
CHECK_FAIL(bpf_prog_query(cg5, BPF_CGROUP_INET_EGRESS,
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach
2019-12-19 7:44 ` [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach Andrey Ignatov
@ 2019-12-19 22:31 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-12-19 22:31 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Andrii Nakryiko, Kernel Team
On Wed, Dec 18, 2019 at 11:45 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> __cgroup_bpf_attach has a lot of identical code to handle two scenarios:
> BPF_F_ALLOW_MULTI is set and unset.
>
> Simplify it by splitting the two main steps:
>
> * First, the decision is made whether a new bpf_prog_list entry should
> be allocated or existing entry should be reused for the new program.
> This decision is saved in replace_pl pointer;
>
> * Next, replace_pl pointer is used to handle both possible states of
> BPF_F_ALLOW_MULTI flag (set / unset) instead of doing similar work for
> them separately.
>
> This splitting, in turn, allows to make further simplifications:
>
> * The check for attaching same program twice in BPF_F_ALLOW_MULTI mode
> can be done before allocating cgroup storage, so that if user tries to
> attach same program twice no alloc/free happens as it was before;
>
> * pl_was_allocated becomes redundant so it's removed.
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
lgtm
Acked-by: Andrii Nakryiko <andriin@fb.com>
> kernel/bpf/cgroup.c | 62 +++++++++++++++++----------------------------
> 1 file changed, 23 insertions(+), 39 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 3/6] bpf: Support replacing cgroup-bpf program in MULTI mode
2019-12-19 7:44 ` [PATCH v4 bpf-next 3/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
@ 2019-12-19 22:41 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-12-19 22:41 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Andrii Nakryiko, Kernel Team
On Wed, Dec 18, 2019 at 11:45 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> The common use-case in production is to have multiple cgroup-bpf
> programs per attach type that cover multiple use-cases. Such programs
> are attached with BPF_F_ALLOW_MULTI and can be maintained by different
> people.
>
> Order of programs usually matters, for example imagine two egress
> programs: the first one drops packets and the second one counts packets.
> If they're swapped the result of counting program will be different.
>
> It brings operational challenges with updating cgroup-bpf program(s)
> attached with BPF_F_ALLOW_MULTI since there is no way to replace a
> program:
>
> * One way to update is to detach all programs first and then attach the
> new version(s) again in the right order. This introduces an
> interruption in the work a program is doing and may not be acceptable
> (e.g. if it's egress firewall);
>
> * Another way is attach the new version of a program first and only then
> detach the old version. This introduces the time interval when two
> versions of same program are working, what may not be acceptable if a
> program is not idempotent. It also imposes additional burden on
> program developers to make sure that two versions of their program can
> co-exist.
>
> Solve the problem by introducing a "replace" mode in BPF_PROG_ATTACH
> command for cgroup-bpf programs being attached with BPF_F_ALLOW_MULTI
> flag. This mode is enabled by newly introduced BPF_F_REPLACE attach flag
> and bpf_attr.replace_bpf_fd attribute to pass fd of the old program to
> replace
>
> That way user can replace any program among those attached with
> BPF_F_ALLOW_MULTI flag without the problems described above.
>
> Details of the new API:
>
> * If BPF_F_REPLACE is set but replace_bpf_fd doesn't have valid
> descriptor of BPF program, BPF_PROG_ATTACH will return corresponding
> error (EINVAL or EBADF).
>
> * If replace_bpf_fd has valid descriptor of BPF program but such a
> program is not attached to specified cgroup, BPF_PROG_ATTACH will
> return ENOENT.
>
> BPF_F_REPLACE is introduced to make the user intent clear, since
> replace_bpf_fd alone can't be used for this (its default value, 0, is a
> valid fd). BPF_F_REPLACE also makes it possible to extend the API in the
> future (e.g. add BPF_F_BEFORE and BPF_F_AFTER if needed).
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
Looks good.
Acked-by: Andrii Narkyiko <andriin@fb.com>
> include/linux/bpf-cgroup.h | 4 +++-
> include/uapi/linux/bpf.h | 10 ++++++++++
> kernel/bpf/cgroup.c | 30 ++++++++++++++++++++++++++----
> kernel/bpf/syscall.c | 4 ++--
> kernel/cgroup/cgroup.c | 5 +++--
> tools/include/uapi/linux/bpf.h | 10 ++++++++++
> 6 files changed, 54 insertions(+), 9 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 4/6] libbpf: Introduce bpf_prog_attach_xattr
2019-12-19 7:44 ` [PATCH v4 bpf-next 4/6] libbpf: Introduce bpf_prog_attach_xattr Andrey Ignatov
@ 2019-12-19 22:42 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-12-19 22:42 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Andrii Nakryiko, Kernel Team
On Wed, Dec 18, 2019 at 11:45 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> Introduce a new bpf_prog_attach_xattr function that, in addition to
> program fd, target fd and attach type, accepts an extendable struct
> bpf_prog_attach_opts.
>
> bpf_prog_attach_opts relies on DECLARE_LIBBPF_OPTS macro to maintain
> backward and forward compatibility and has the following "optional"
> attach attributes:
>
> * existing attach_flags, since it's not required when attaching in NONE
> mode. Even though it's quite often used in MULTI and OVERRIDE mode it
> seems to be a good idea to reduce number of arguments to
> bpf_prog_attach_xattr;
>
> * newly introduced attribute of BPF_PROG_ATTACH command: replace_prog_fd
> that is fd of previously attached cgroup-bpf program to replace if
> BPF_F_REPLACE flag is used.
>
> The new function is named to be consistent with other xattr-functions
> (bpf_prog_test_run_xattr, bpf_create_map_xattr, bpf_load_program_xattr).
>
> The struct bpf_prog_attach_opts is supposed to be used with
> DECLARE_LIBBPF_OPTS macro.
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> ---
Looks great, thanks!
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/lib/bpf/bpf.c | 17 ++++++++++++++++-
> tools/lib/bpf/bpf.h | 11 +++++++++++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 28 insertions(+), 1 deletion(-)
>
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 5/6] selftests/bpf: Convert test_cgroup_attach to prog_tests
2019-12-19 7:44 ` [PATCH v4 bpf-next 5/6] selftests/bpf: Convert test_cgroup_attach to prog_tests Andrey Ignatov
@ 2019-12-19 22:46 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-12-19 22:46 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Andrii Nakryiko, Kernel Team
On Wed, Dec 18, 2019 at 11:45 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> Convert test_cgroup_attach to prog_tests.
>
> This change does a lot of things but in many cases it's pretty expensive
> to separate them, so they go in one commit. Nevertheless the logic is
> ketp as is and changes made are just moving things around, simplifying
> them (w/o changing the meaning of the tests) and making prog_tests
> compatible:
>
> * split the 3 tests in the file into 3 separate files in prog_tests/;
>
> * rename the test functions to test_<file_base_name>;
>
> * remove unused includes, constants, variables and functions from every
> test;
>
> * replace `if`-s with or `if (CHECK())` where additional context should
> be logged and with `if (CHECK_FAIL())` where line number is enough;
>
> * switch from `log_err()` to logging via `CHECK()`;
>
> * replace `assert`-s with `CHECK_FAIL()` to avoid crashing the whole
> test_progs if one assertion fails;
>
> * replace cgroup_helpers with test__join_cgroup() in
> cgroup_attach_override only, other tests need more fine-grained
> control for cgroup creation/deletion so cgroup_helpers are still used
> there;
>
> * simplify cgroup_attach_autodetach by switching to easiest possible
> program since this test doesn't really need such a complicated program
> as cgroup_attach_multi does;
>
> * remove test_cgroup_attach.c itself.
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/testing/selftests/bpf/.gitignore | 1 -
> tools/testing/selftests/bpf/Makefile | 3 +-
> .../bpf/prog_tests/cgroup_attach_autodetach.c | 111 ++++
> .../bpf/prog_tests/cgroup_attach_multi.c | 238 ++++++++
> .../bpf/prog_tests/cgroup_attach_override.c | 148 +++++
> .../selftests/bpf/test_cgroup_attach.c | 571 ------------------
> 6 files changed, 498 insertions(+), 574 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
> create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
> create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> delete mode 100644 tools/testing/selftests/bpf/test_cgroup_attach.c
>
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 6/6] selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi
2019-12-19 7:44 ` [PATCH v4 bpf-next 6/6] selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi Andrey Ignatov
@ 2019-12-19 22:48 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2019-12-19 22:48 UTC (permalink / raw)
To: Andrey Ignatov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Andrii Nakryiko, Kernel Team
On Wed, Dec 18, 2019 at 11:45 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> Test replacing a cgroup-bpf program attached with BPF_F_ALLOW_MULTI and
> possible failure modes: invalid combination of flags, invalid
> replace_bpf_fd, replacing a non-attachd to specified cgroup program.
>
> Example of program replacing:
>
> # gdb -q --args ./test_progs --name=cgroup_attach_multi
> ...
> Breakpoint 1, test_cgroup_attach_multi () at cgroup_attach_multi.c:227
> (gdb)
> [1]+ Stopped gdb -q --args ./test_progs --name=cgroup_attach_multi
> # bpftool c s /mnt/cgroup2/cgroup-test-work-dir/cg1
> ID AttachType AttachFlags Name
> 2133 egress multi
> 2134 egress multi
> # fg
> gdb -q --args ./test_progs --name=cgroup_attach_multi
> (gdb) c
> Continuing.
>
> Breakpoint 2, test_cgroup_attach_multi () at cgroup_attach_multi.c:233
> (gdb)
> [1]+ Stopped gdb -q --args ./test_progs --name=cgroup_attach_multi
> # bpftool c s /mnt/cgroup2/cgroup-test-work-dir/cg1
> ID AttachType AttachFlags Name
> 2139 egress multi
> 2134 egress multi
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> ---
LGTM.
Acked-by: Andrii Nakryiko <andriin@fb.com>
> .../bpf/prog_tests/cgroup_attach_multi.c | 53 +++++++++++++++++--
> 1 file changed, 50 insertions(+), 3 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
` (5 preceding siblings ...)
2019-12-19 7:44 ` [PATCH v4 bpf-next 6/6] selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi Andrey Ignatov
@ 2019-12-20 5:33 ` Alexei Starovoitov
6 siblings, 0 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2019-12-20 5:33 UTC (permalink / raw)
To: Andrey Ignatov; +Cc: bpf, ast, daniel, kafai, andriin, kernel-team
On Wed, Dec 18, 2019 at 11:44:32PM -0800, Andrey Ignatov wrote:
> v3->v4:
> - use OPTS_VALID and OPTS_GET to handle bpf_prog_attach_opts.
>
> v2->v3:
> - rely on DECLARE_LIBBPF_OPTS from libbpf_common.h;
> - separate "required" and "optional" arguments in bpf_prog_attach_xattr;
> - convert test_cgroup_attach to prog_tests;
> - move the new selftest to prog_tests/cgroup_attach_multi.
>
> v1->v2:
> - move DECLARE_LIBBPF_OPTS from libbpf.h to bpf.h (patch 4);
> - switch new libbpf API to OPTS framework;
> - switch selftest to libbpf OPTS framework.
>
> This patch set adds support for replacing cgroup-bpf programs attached with
> BPF_F_ALLOW_MULTI flag so that any program in a list can be updated to a new
> version without service interruption and order of programs can be preserved.
>
> Please see patch 3 for details on the use-case and API changes.
>
> Other patches:
> Patch 1 is preliminary refactoring of __cgroup_bpf_attach to simplify it.
> Patch 2 is minor cleanup of hierarchy_allows_attach.
> Patch 4 extends libbpf API to support new set of attach attributes.
> Patch 5 converts test_cgroup_attach to prog_tests.
> Patch 6 adds selftest coverage for the new API.
Applied, Thanks
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-12-20 5:33 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 7:44 [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 1/6] bpf: Simplify __cgroup_bpf_attach Andrey Ignatov
2019-12-19 22:31 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 2/6] bpf: Remove unused new_flags in hierarchy_allows_attach() Andrey Ignatov
2019-12-19 7:44 ` [PATCH v4 bpf-next 3/6] bpf: Support replacing cgroup-bpf program in MULTI mode Andrey Ignatov
2019-12-19 22:41 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 4/6] libbpf: Introduce bpf_prog_attach_xattr Andrey Ignatov
2019-12-19 22:42 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 5/6] selftests/bpf: Convert test_cgroup_attach to prog_tests Andrey Ignatov
2019-12-19 22:46 ` Andrii Nakryiko
2019-12-19 7:44 ` [PATCH v4 bpf-next 6/6] selftests/bpf: Test BPF_F_REPLACE in cgroup_attach_multi Andrey Ignatov
2019-12-19 22:48 ` Andrii Nakryiko
2019-12-20 5:33 ` [PATCH v4 bpf-next 0/6] bpf: Support replacing cgroup-bpf program in MULTI mode Alexei Starovoitov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.