* [PATCH bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper
@ 2018-08-11 5:35 Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 1/4] bpf: Introduce " Andrey Ignatov
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-11 5:35 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, kernel-team
This patch set adds new BPF helper bpf_skb_ancestor_cgroup_id that returns
id of cgroup v2 that is ancestor of cgroup associated with the skb at the
ancestor_level.
The helper is useful to implement policies in TC based on cgroups that are
upper in hierarchy than immediate cgroup associated with skb.
Patch 0001 provides more details and describes use-cases.
Patch 0002 syncs UAPI changes to tools/.
Patch 0003 adds skb*cgroup_id helpers to bpf_helper.h header.
Patch 0004 adds selftest for the new helper and is an example of usage.
Andrey Ignatov (4):
bpf: Introduce bpf_skb_ancestor_cgroup_id helper
bpf: Sync bpf.h to tools/
selftests/bpf: Add cgroup id helpers to bpf_helpers.h
selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
include/linux/cgroup.h | 30 +++
include/uapi/linux/bpf.h | 21 +-
net/core/filter.c | 28 +++
tools/include/uapi/linux/bpf.h | 21 +-
tools/testing/selftests/bpf/Makefile | 9 +-
tools/testing/selftests/bpf/bpf_helpers.h | 4 +
.../selftests/bpf/test_skb_cgroup_id.sh | 61 ++++++
.../selftests/bpf/test_skb_cgroup_id_kern.c | 47 +++++
.../selftests/bpf/test_skb_cgroup_id_user.c | 187 ++++++++++++++++++
9 files changed, 403 insertions(+), 5 deletions(-)
create mode 100755 tools/testing/selftests/bpf/test_skb_cgroup_id.sh
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
2018-08-11 5:35 [PATCH bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper Andrey Ignatov
@ 2018-08-11 5:35 ` Andrey Ignatov
2018-08-13 14:46 ` Tejun Heo
2018-08-11 5:35 ` [PATCH bpf-next 2/4] bpf: Sync bpf.h to tools/ Andrey Ignatov
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-11 5:35 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, kernel-team
== Problem description ==
It's useful to be able to identify cgroup associated with skb in TC so
that a policy can be applied to this skb, and existing bpf_skb_cgroup_id
helper can help with this.
Though in real life cgroup hierarchy and hierarchy to apply a policy to
don't map 1:1.
It's often the case that there is a container and corresponding cgroup,
but there are many more sub-cgroups inside container, e.g. because it's
delegated to containerized application to control resources for its
subsystems, or to separate application inside container from infra that
belongs to containerization system (e.g. sshd).
At the same time it may be useful to apply a policy to container as a
whole.
If multiple containers like this are run on a host (what is often the
case) and many of them have sub-cgroups, it may not be possible to apply
per-container policy in TC with existing helpers such as
bpf_skb_under_cgroup or bpf_skb_cgroup_id:
* bpf_skb_cgroup_id will return id of immediate cgroup associated with
skb, i.e. if it's a sub-cgroup inside container, it can't be used to
identify container's cgroup;
* bpf_skb_under_cgroup can work only with one cgroup and doesn't scale,
i.e. if there are N containers on a host and a policy has to be
applied to M of them (0 <= M <= N), it'd require M calls to
bpf_skb_under_cgroup, and, if M changes, it'd require to rebuild &
load new BPF program.
== Solution ==
The patch introduces new helper bpf_skb_ancestor_cgroup_id that can be
used to get id of cgroup v2 that is an ancestor of cgroup associated
with skb at specified level of cgroup hierarchy.
That way admin can place all containers on one level of cgroup hierarchy
(what is a good practice in general and already used in many
configurations) and identify specific cgroup on this level no matter
what sub-cgroup skb is associated with.
E.g. if there is a cgroup hierarchy:
root/
root/container1/
root/container1/app11/
root/container1/app11/sub-app-a/
root/container1/app12/
root/container2/
root/container2/app21/
root/container2/app22/
root/container2/app22/sub-app-b/
, then having skb associated with root/container1/app11/sub-app-a/ it's
possible to get ancestor at level 1, what is container1 and apply policy
for this container, or apply another policy if it's container2.
Policies can be kept e.g. in a hash map where key is a container cgroup
id and value is an action.
Levels where container cgroups are created are usually known in advance
whether cgroup hierarchy inside container may be hard to predict
especially in case when its creation is delegated to containerized
application.
== Implementation details ==
The helper gets ancestor by walking parents up to specified level.
Another option would be to get different kind of "id" from
cgroup->ancestor_ids[level] and use it with idr_find() to get struct
cgroup for ancestor. But that would require radix lookup what doesn't
seem to be better (at least it's not obviously better).
Format of return value of the new helper is same as that of
bpf_skb_cgroup_id.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
include/linux/cgroup.h | 30 ++++++++++++++++++++++++++++++
include/uapi/linux/bpf.h | 21 ++++++++++++++++++++-
net/core/filter.c | 28 ++++++++++++++++++++++++++++
3 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index c9fdf6f57913..32c553556bbd 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -553,6 +553,36 @@ static inline bool cgroup_is_descendant(struct cgroup *cgrp,
return cgrp->ancestor_ids[ancestor->level] == ancestor->id;
}
+/**
+ * cgroup_ancestor - find ancestor of cgroup
+ * @cgrp: cgroup to find ancestor of
+ * @ancestor_level: level of ancestor to find starting from root
+ *
+ * Find ancestor of cgroup at specified level starting from root if it exists
+ * and return pointer to it. Return NULL if @cgrp doesn't have ancestor at
+ * @ancestor_level.
+ *
+ * This function is safe to call as long as @cgrp is accessible.
+ */
+static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
+ int ancestor_level)
+{
+ struct cgroup *ptr;
+
+ if (cgrp->level < ancestor_level)
+ return NULL;
+
+ for (ptr = cgrp;
+ ptr && ptr->level > ancestor_level;
+ ptr = cgroup_parent(ptr))
+ ;
+
+ if (ptr && ptr->level == ancestor_level)
+ return ptr;
+
+ return NULL;
+}
+
/**
* task_under_cgroup_hierarchy - test task's membership of cgroup ancestry
* @task: the task to be tested
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3102a2a23c31..66917a4eba27 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2093,6 +2093,24 @@ union bpf_attr {
* Return
* The id is returned or 0 in case the id could not be retrieved.
*
+ * u64 bpf_skb_ancestor_cgroup_id(struct sk_buff *skb, int ancestor_level)
+ * Description
+ * Return id of cgroup v2 that is ancestor of cgroup associated
+ * with the *skb* at the *ancestor_level*. The root cgroup is at
+ * *ancestor_level* zero and each step down the hierarchy
+ * increments the level. If *ancestor_level* == level of cgroup
+ * associated with *skb*, then return value will be same as that
+ * of **bpf_skb_cgroup_id**\ ().
+ *
+ * The helper is useful to implement policies based on cgroups
+ * that are upper in hierarchy than immediate cgroup associated
+ * with *skb*.
+ *
+ * The format of returned id and helper limitations are same as in
+ * **bpf_skb_cgroup_id**\ ().
+ * Return
+ * The id is returned or 0 in case the id could not be retrieved.
+ *
* u64 bpf_get_current_cgroup_id(void)
* Return
* A 64-bit integer containing the current cgroup id based
@@ -2207,7 +2225,8 @@ union bpf_attr {
FN(skb_cgroup_id), \
FN(get_current_cgroup_id), \
FN(get_local_storage), \
- FN(sk_select_reuseport),
+ FN(sk_select_reuseport), \
+ FN(skb_ancestor_cgroup_id),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 22906b31d43f..15b9d2df92ca 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3778,6 +3778,32 @@ static const struct bpf_func_proto bpf_skb_cgroup_id_proto = {
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
};
+
+BPF_CALL_2(bpf_skb_ancestor_cgroup_id, const struct sk_buff *, skb, int,
+ ancestor_level)
+{
+ struct sock *sk = skb_to_full_sk(skb);
+ struct cgroup *ancestor;
+ struct cgroup *cgrp;
+
+ if (!sk || !sk_fullsock(sk))
+ return 0;
+
+ cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
+ ancestor = cgroup_ancestor(cgrp, ancestor_level);
+ if (!ancestor)
+ return 0;
+
+ return ancestor->kn->id.id;
+}
+
+static const struct bpf_func_proto bpf_skb_ancestor_cgroup_id_proto = {
+ .func = bpf_skb_ancestor_cgroup_id,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+};
#endif
static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
@@ -4966,6 +4992,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
#ifdef CONFIG_SOCK_CGROUP_DATA
case BPF_FUNC_skb_cgroup_id:
return &bpf_skb_cgroup_id_proto;
+ case BPF_FUNC_skb_ancestor_cgroup_id:
+ return &bpf_skb_ancestor_cgroup_id_proto;
#endif
default:
return bpf_base_func_proto(func_id);
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 2/4] bpf: Sync bpf.h to tools/
2018-08-11 5:35 [PATCH bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 1/4] bpf: Introduce " Andrey Ignatov
@ 2018-08-11 5:35 ` Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 3/4] selftests/bpf: Add cgroup id helpers to bpf_helpers.h Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id Andrey Ignatov
3 siblings, 0 replies; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-11 5:35 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, kernel-team
Sync skb_ancestor_cgroup_id() related bpf UAPI changes to tools/.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/include/uapi/linux/bpf.h | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 3102a2a23c31..66917a4eba27 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2093,6 +2093,24 @@ union bpf_attr {
* Return
* The id is returned or 0 in case the id could not be retrieved.
*
+ * u64 bpf_skb_ancestor_cgroup_id(struct sk_buff *skb, int ancestor_level)
+ * Description
+ * Return id of cgroup v2 that is ancestor of cgroup associated
+ * with the *skb* at the *ancestor_level*. The root cgroup is at
+ * *ancestor_level* zero and each step down the hierarchy
+ * increments the level. If *ancestor_level* == level of cgroup
+ * associated with *skb*, then return value will be same as that
+ * of **bpf_skb_cgroup_id**\ ().
+ *
+ * The helper is useful to implement policies based on cgroups
+ * that are upper in hierarchy than immediate cgroup associated
+ * with *skb*.
+ *
+ * The format of returned id and helper limitations are same as in
+ * **bpf_skb_cgroup_id**\ ().
+ * Return
+ * The id is returned or 0 in case the id could not be retrieved.
+ *
* u64 bpf_get_current_cgroup_id(void)
* Return
* A 64-bit integer containing the current cgroup id based
@@ -2207,7 +2225,8 @@ union bpf_attr {
FN(skb_cgroup_id), \
FN(get_current_cgroup_id), \
FN(get_local_storage), \
- FN(sk_select_reuseport),
+ FN(sk_select_reuseport), \
+ FN(skb_ancestor_cgroup_id),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 3/4] selftests/bpf: Add cgroup id helpers to bpf_helpers.h
2018-08-11 5:35 [PATCH bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 1/4] bpf: Introduce " Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 2/4] bpf: Sync bpf.h to tools/ Andrey Ignatov
@ 2018-08-11 5:35 ` Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id Andrey Ignatov
3 siblings, 0 replies; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-11 5:35 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, kernel-team
Add bpf_skb_cgroup_id and bpf_skb_ancestor_cgroup_id helpers to
bpf_helpers.h to use them in tests and samples.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5c32266c2c38..e4be7730222d 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -139,6 +139,10 @@ static unsigned long long (*bpf_get_current_cgroup_id)(void) =
(void *) BPF_FUNC_get_current_cgroup_id;
static void *(*bpf_get_local_storage)(void *map, unsigned long long flags) =
(void *) BPF_FUNC_get_local_storage;
+static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
+ (void *) BPF_FUNC_skb_cgroup_id;
+static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
+ (void *) BPF_FUNC_skb_ancestor_cgroup_id;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
2018-08-11 5:35 [PATCH bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper Andrey Ignatov
` (2 preceding siblings ...)
2018-08-11 5:35 ` [PATCH bpf-next 3/4] selftests/bpf: Add cgroup id helpers to bpf_helpers.h Andrey Ignatov
@ 2018-08-11 5:35 ` Andrey Ignatov
2018-08-12 6:58 ` Yonghong Song
3 siblings, 1 reply; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-11 5:35 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, kernel-team
Add selftests for bpf_skb_ancestor_cgroup_id helper.
test_skb_cgroup_id.sh prepares testing interface and adds tc qdisc and
filter for it using BPF object compiled from test_skb_cgroup_id_kern.c
program.
BPF program in test_skb_cgroup_id_kern.c gets ancestor cgroup id using
the new helper at different levels of cgroup hierarchy that skb belongs
to, including root level and non-existing level, and saves it to the map
where the key is the level of corresponding cgroup and the value is its
id.
To trigger BPF program, user space program test_skb_cgroup_id_user is
run. It adds itself into testing cgroup and sends UDP datagram to
link-local multicast address of testing interface. Then it reads cgroup
ids saved in kernel for different levels from the BPF map and compares
them with those in user space. They must be equal for every level of
ancestry.
Example of run:
# ./test_skb_cgroup_id.sh
Wait for testing link-local IP to become available ... OK
Note: 8 bytes struct bpf_elf_map fixup performed due to size mismatch!
[PASS]
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/Makefile | 9 +-
.../selftests/bpf/test_skb_cgroup_id.sh | 61 ++++++
.../selftests/bpf/test_skb_cgroup_id_kern.c | 47 +++++
.../selftests/bpf/test_skb_cgroup_id_user.c | 187 ++++++++++++++++++
4 files changed, 301 insertions(+), 3 deletions(-)
create mode 100755 tools/testing/selftests/bpf/test_skb_cgroup_id.sh
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index daed162043c2..fff7fb1285fc 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -34,7 +34,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
- get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o
+ get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
+ test_skb_cgroup_id_kern.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
@@ -45,10 +46,11 @@ TEST_PROGS := test_kmod.sh \
test_sock_addr.sh \
test_tunnel.sh \
test_lwt_seg6local.sh \
- test_lirc_mode2.sh
+ test_lirc_mode2.sh \
+ test_skb_cgroup_id.sh
# Compile but not part of 'make run_tests'
-TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr
+TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user
include ../lib.mk
@@ -59,6 +61,7 @@ $(TEST_GEN_PROGS): $(BPFOBJ)
$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a
$(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
+$(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c
$(OUTPUT)/test_sock: cgroup_helpers.c
$(OUTPUT)/test_sock_addr: cgroup_helpers.c
$(OUTPUT)/test_socket_cookie: cgroup_helpers.c
diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id.sh b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
new file mode 100755
index 000000000000..b75e9b52f06f
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
@@ -0,0 +1,61 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Facebook
+
+set -eu
+
+wait_for_ip()
+{
+ local _i
+ echo -n "Wait for testing link-local IP to become available "
+ for _i in $(seq ${MAX_PING_TRIES}); do
+ echo -n "."
+ if ping -6 -q -c 1 -W 1 ff02::1%${TEST_IF} >/dev/null 2>&1; then
+ echo " OK"
+ return
+ fi
+ done
+ echo 1>&2 "ERROR: Timeout waiting for test IP to become available."
+ exit 1
+}
+
+setup()
+{
+ # Create testing interfaces not to interfere with current environment.
+ ip link add dev ${TEST_IF} type veth peer name ${TEST_IF_PEER}
+ ip link set ${TEST_IF} up
+ ip link set ${TEST_IF_PEER} up
+
+ wait_for_ip
+
+ tc qdisc add dev ${TEST_IF} clsact
+ tc filter add dev ${TEST_IF} egress bpf obj ${BPF_PROG_OBJ} \
+ sec ${BPF_PROG_SECTION} da
+
+ BPF_PROG_ID=$(tc filter show dev ${TEST_IF} egress | \
+ awk '/ id / {sub(/.* id /, "", $0); print($1)}')
+}
+
+cleanup()
+{
+ ip link del ${TEST_IF} 2>/dev/null || :
+ ip link del ${TEST_IF_PEER} 2>/dev/null || :
+}
+
+main()
+{
+ trap cleanup EXIT 2 3 6 15
+ setup
+ ${PROG} ${TEST_IF} ${BPF_PROG_ID}
+}
+
+DIR=$(dirname $0)
+TEST_IF="test_cgid_1"
+TEST_IF_PEER="test_cgid_2"
+MAX_PING_TRIES=5
+BPF_PROG_OBJ="${DIR}/test_skb_cgroup_id_kern.o"
+BPF_PROG_SECTION="cgroup_id_logger"
+BPF_PROG_ID=0
+PROG="${DIR}/test_skb_cgroup_id_user"
+
+main
diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
new file mode 100644
index 000000000000..68cf9829f5a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+
+#include <string.h>
+
+#include "bpf_helpers.h"
+
+#define NUM_CGROUP_LEVELS 4
+
+struct bpf_map_def SEC("maps") cgroup_ids = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(__u64),
+ .max_entries = NUM_CGROUP_LEVELS,
+};
+
+static __always_inline void log_nth_level(struct __sk_buff *skb, __u32 level)
+{
+ __u64 id;
+
+ /* [1] &level passed to external function that may change it, it's
+ * incompatible with loop unroll.
+ */
+ id = bpf_skb_ancestor_cgroup_id(skb, level);
+ bpf_map_update_elem(&cgroup_ids, &level, &id, 0);
+}
+
+SEC("cgroup_id_logger")
+int log_cgroup_id(struct __sk_buff *skb)
+{
+ /* Loop unroll can't be used here due to [1]. Unrolling manually.
+ * Number of calls should be in sync with NUM_CGROUP_LEVELS.
+ */
+ log_nth_level(skb, 0);
+ log_nth_level(skb, 1);
+ log_nth_level(skb, 2);
+ log_nth_level(skb, 3);
+
+ return TC_ACT_OK;
+}
+
+int _version SEC("version") = 1;
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
new file mode 100644
index 000000000000..c121cc59f314
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "cgroup_helpers.h"
+
+#define CGROUP_PATH "/skb_cgroup_test"
+#define NUM_CGROUP_LEVELS 4
+
+/* RFC 4291, Section 2.7.1 */
+#define LINKLOCAL_MULTICAST "ff02::1"
+
+static int mk_dst_addr(const char *ip, const char *iface,
+ struct sockaddr_in6 *dst)
+{
+ memset(dst, 0, sizeof(*dst));
+
+ dst->sin6_family = AF_INET6;
+ dst->sin6_port = htons(1025);
+
+ if (inet_pton(AF_INET6, ip, &dst->sin6_addr) != 1) {
+ log_err("Invalid IPv6: %s", ip);
+ return -1;
+ }
+
+ dst->sin6_scope_id = if_nametoindex(iface);
+ if (!dst->sin6_scope_id) {
+ log_err("Failed to get index of iface: %s", iface);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int send_packet(const char *iface)
+{
+ struct sockaddr_in6 dst;
+ char msg[] = "msg";
+ int err = 0;
+ int fd = -1;
+
+ if (mk_dst_addr(LINKLOCAL_MULTICAST, iface, &dst))
+ goto err;
+
+ fd = socket(AF_INET6, SOCK_DGRAM, 0);
+ if (fd == -1) {
+ log_err("Failed to create UDP socket");
+ goto err;
+ }
+
+ if (sendto(fd, &msg, sizeof(msg), 0, (const struct sockaddr *)&dst,
+ sizeof(dst)) == -1) {
+ log_err("Failed to send datagram");
+ goto err;
+ }
+
+ goto out;
+err:
+ err = -1;
+out:
+ if (fd >= 0)
+ close(fd);
+ return err;
+}
+
+int get_map_fd_by_prog_id(int prog_id)
+{
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ __u32 map_ids[1];
+ int prog_fd = -1;
+ int map_fd = -1;
+
+ prog_fd = bpf_prog_get_fd_by_id(prog_id);
+ if (prog_fd < 0) {
+ log_err("Failed to get fd by prog id %d", prog_id);
+ goto err;
+ }
+
+ info.nr_map_ids = 1;
+ info.map_ids = (__u64) (unsigned long) map_ids;
+
+ if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
+ log_err("Failed to get info by prog fd %d", prog_fd);
+ goto err;
+ }
+
+ if (!info.nr_map_ids) {
+ log_err("No maps found for prog fd %d", prog_fd);
+ goto err;
+ }
+
+ map_fd = bpf_map_get_fd_by_id(map_ids[0]);
+ if (map_fd < 0)
+ log_err("Failed to get fd by map id %d", map_ids[0]);
+err:
+ if (prog_fd >= 0)
+ close(prog_fd);
+ return map_fd;
+}
+
+int check_ancestor_cgroup_ids(int prog_id)
+{
+ __u64 actual_ids[NUM_CGROUP_LEVELS], expected_ids[NUM_CGROUP_LEVELS];
+ __u32 level;
+ int err = 0;
+ int map_fd;
+
+ expected_ids[0] = 0x100000001; /* root cgroup */
+ expected_ids[1] = get_cgroup_id("");
+ expected_ids[2] = get_cgroup_id(CGROUP_PATH);
+ expected_ids[3] = 0; /* non-existent cgroup */
+
+ map_fd = get_map_fd_by_prog_id(prog_id);
+ if (map_fd < 0)
+ goto err;
+
+ for (level = 0; level < NUM_CGROUP_LEVELS; ++level) {
+ if (bpf_map_lookup_elem(map_fd, &level, &actual_ids[level])) {
+ log_err("Failed to lookup key %d", level);
+ goto err;
+ }
+ if (actual_ids[level] != expected_ids[level]) {
+ log_err("%llx (actual) != %llx (expected), level: %u\n",
+ actual_ids[level], expected_ids[level], level);
+ goto err;
+ }
+ }
+
+ goto out;
+err:
+ err = -1;
+out:
+ if (map_fd >= 0)
+ close(map_fd);
+ return err;
+}
+
+int main(int argc, char **argv)
+{
+ int cgfd = -1;
+ int err = 0;
+
+ if (argc < 3) {
+ fprintf(stderr, "Usage: %s iface prog_id\n", argv[0]);
+ exit(EXIT_FAILURE);
+ }
+
+ if (setup_cgroup_environment())
+ goto err;
+
+ cgfd = create_and_get_cgroup(CGROUP_PATH);
+ if (!cgfd)
+ goto err;
+
+ if (join_cgroup(CGROUP_PATH))
+ goto err;
+
+ if (send_packet(argv[1]))
+ goto err;
+
+ if (check_ancestor_cgroup_ids(atoi(argv[2])))
+ goto err;
+
+ goto out;
+err:
+ err = -1;
+out:
+ close(cgfd);
+ cleanup_cgroup_environment();
+ printf("[%s]\n", err ? "FAIL" : "PASS");
+ return err;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
2018-08-11 5:35 ` [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id Andrey Ignatov
@ 2018-08-12 6:58 ` Yonghong Song
2018-08-12 17:41 ` Andrey Ignatov
0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2018-08-12 6:58 UTC (permalink / raw)
To: Andrey Ignatov, netdev; +Cc: ast, daniel, tj, guro, kernel-team
On 8/10/18 10:35 PM, Andrey Ignatov wrote:
> Add selftests for bpf_skb_ancestor_cgroup_id helper.
>
> test_skb_cgroup_id.sh prepares testing interface and adds tc qdisc and
> filter for it using BPF object compiled from test_skb_cgroup_id_kern.c
> program.
>
> BPF program in test_skb_cgroup_id_kern.c gets ancestor cgroup id using
> the new helper at different levels of cgroup hierarchy that skb belongs
> to, including root level and non-existing level, and saves it to the map
> where the key is the level of corresponding cgroup and the value is its
> id.
>
> To trigger BPF program, user space program test_skb_cgroup_id_user is
> run. It adds itself into testing cgroup and sends UDP datagram to
> link-local multicast address of testing interface. Then it reads cgroup
> ids saved in kernel for different levels from the BPF map and compares
> them with those in user space. They must be equal for every level of
> ancestry.
>
> Example of run:
> # ./test_skb_cgroup_id.sh
> Wait for testing link-local IP to become available ... OK
> Note: 8 bytes struct bpf_elf_map fixup performed due to size mismatch!
> [PASS]
I am not able to run the test on my FC27 based VM with the latest
bpf-next and the patch set.
[yhs@localhost bpf]$ sudo ./test_skb_cgroup_id.sh
Wait for testing link-local IP to become available .....ERROR: Timeout
waiting for test IP to become available.
[yhs@localhost bpf]$
I am able to run test_sock_addr.sh successfully.
$ sudo ./test_sock_addr.sh
Wait for testing IPv4/IPv6 to become available .
.. OK
Test case: bind4: load prog with wrong expected attach type .. [PASS]
Test case: bind4: attach prog with wrong attach type .. [PASS]
...
Test case: sendmsg6: deny call .. [PASS]
Summary: 27 PASSED, 0 FAILED
Maybe some issues in this addr ff02::1%${TEST_IF}?
>
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> ---
> tools/testing/selftests/bpf/Makefile | 9 +-
> .../selftests/bpf/test_skb_cgroup_id.sh | 61 ++++++
> .../selftests/bpf/test_skb_cgroup_id_kern.c | 47 +++++
> .../selftests/bpf/test_skb_cgroup_id_user.c | 187 ++++++++++++++++++
> 4 files changed, 301 insertions(+), 3 deletions(-)
> create mode 100755 tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
> create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index daed162043c2..fff7fb1285fc 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -34,7 +34,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
> test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
> test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
> test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
> - get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o
> + get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
> + test_skb_cgroup_id_kern.o
>
> # Order correspond to 'make run_tests' order
> TEST_PROGS := test_kmod.sh \
> @@ -45,10 +46,11 @@ TEST_PROGS := test_kmod.sh \
> test_sock_addr.sh \
> test_tunnel.sh \
> test_lwt_seg6local.sh \
> - test_lirc_mode2.sh
> + test_lirc_mode2.sh \
> + test_skb_cgroup_id.sh
>
> # Compile but not part of 'make run_tests'
> -TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr
> +TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user
>
> include ../lib.mk
>
> @@ -59,6 +61,7 @@ $(TEST_GEN_PROGS): $(BPFOBJ)
> $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a
>
> $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
> +$(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c
> $(OUTPUT)/test_sock: cgroup_helpers.c
> $(OUTPUT)/test_sock_addr: cgroup_helpers.c
> $(OUTPUT)/test_socket_cookie: cgroup_helpers.c
> diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id.sh b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> new file mode 100755
> index 000000000000..b75e9b52f06f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> @@ -0,0 +1,61 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Facebook
> +
> +set -eu
> +
> +wait_for_ip()
> +{
> + local _i
> + echo -n "Wait for testing link-local IP to become available "
> + for _i in $(seq ${MAX_PING_TRIES}); do
> + echo -n "."
> + if ping -6 -q -c 1 -W 1 ff02::1%${TEST_IF} >/dev/null 2>&1; then
> + echo " OK"
> + return
> + fi
> + done
> + echo 1>&2 "ERROR: Timeout waiting for test IP to become available."
> + exit 1
> +}
> +
> +setup()
> +{
> + # Create testing interfaces not to interfere with current environment.
> + ip link add dev ${TEST_IF} type veth peer name ${TEST_IF_PEER}
> + ip link set ${TEST_IF} up
> + ip link set ${TEST_IF_PEER} up
> +
> + wait_for_ip
> +
> + tc qdisc add dev ${TEST_IF} clsact
> + tc filter add dev ${TEST_IF} egress bpf obj ${BPF_PROG_OBJ} \
> + sec ${BPF_PROG_SECTION} da
> +
> + BPF_PROG_ID=$(tc filter show dev ${TEST_IF} egress | \
> + awk '/ id / {sub(/.* id /, "", $0); print($1)}')
> +}
> +
> +cleanup()
> +{
> + ip link del ${TEST_IF} 2>/dev/null || :
> + ip link del ${TEST_IF_PEER} 2>/dev/null || :
> +}
> +
> +main()
> +{
> + trap cleanup EXIT 2 3 6 15
> + setup
> + ${PROG} ${TEST_IF} ${BPF_PROG_ID}
> +}
> +
> +DIR=$(dirname $0)
> +TEST_IF="test_cgid_1"
> +TEST_IF_PEER="test_cgid_2"
> +MAX_PING_TRIES=5
> +BPF_PROG_OBJ="${DIR}/test_skb_cgroup_id_kern.o"
> +BPF_PROG_SECTION="cgroup_id_logger"
> +BPF_PROG_ID=0
> +PROG="${DIR}/test_skb_cgroup_id_user"
> +
> +main
> diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
> new file mode 100644
> index 000000000000..68cf9829f5a7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Facebook
> +
> +#include <linux/bpf.h>
> +#include <linux/pkt_cls.h>
> +
> +#include <string.h>
> +
> +#include "bpf_helpers.h"
> +
> +#define NUM_CGROUP_LEVELS 4
> +
> +struct bpf_map_def SEC("maps") cgroup_ids = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(__u32),
> + .value_size = sizeof(__u64),
> + .max_entries = NUM_CGROUP_LEVELS,
> +};
> +
> +static __always_inline void log_nth_level(struct __sk_buff *skb, __u32 level)
> +{
> + __u64 id;
> +
> + /* [1] &level passed to external function that may change it, it's
> + * incompatible with loop unroll.
> + */
> + id = bpf_skb_ancestor_cgroup_id(skb, level);
> + bpf_map_update_elem(&cgroup_ids, &level, &id, 0);
> +}
> +
> +SEC("cgroup_id_logger")
> +int log_cgroup_id(struct __sk_buff *skb)
> +{
> + /* Loop unroll can't be used here due to [1]. Unrolling manually.
> + * Number of calls should be in sync with NUM_CGROUP_LEVELS.
> + */
> + log_nth_level(skb, 0);
> + log_nth_level(skb, 1);
> + log_nth_level(skb, 2);
> + log_nth_level(skb, 3);
> +
> + return TC_ACT_OK;
> +}
> +
> +int _version SEC("version") = 1;
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
> new file mode 100644
> index 000000000000..c121cc59f314
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Facebook
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <arpa/inet.h>
> +#include <net/if.h>
> +#include <netinet/in.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +
> +
> +#include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
> +
> +#include "bpf_rlimit.h"
> +#include "cgroup_helpers.h"
> +
> +#define CGROUP_PATH "/skb_cgroup_test"
> +#define NUM_CGROUP_LEVELS 4
> +
> +/* RFC 4291, Section 2.7.1 */
> +#define LINKLOCAL_MULTICAST "ff02::1"
> +
> +static int mk_dst_addr(const char *ip, const char *iface,
> + struct sockaddr_in6 *dst)
> +{
> + memset(dst, 0, sizeof(*dst));
> +
> + dst->sin6_family = AF_INET6;
> + dst->sin6_port = htons(1025);
> +
> + if (inet_pton(AF_INET6, ip, &dst->sin6_addr) != 1) {
> + log_err("Invalid IPv6: %s", ip);
> + return -1;
> + }
> +
> + dst->sin6_scope_id = if_nametoindex(iface);
> + if (!dst->sin6_scope_id) {
> + log_err("Failed to get index of iface: %s", iface);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int send_packet(const char *iface)
> +{
> + struct sockaddr_in6 dst;
> + char msg[] = "msg";
> + int err = 0;
> + int fd = -1;
> +
> + if (mk_dst_addr(LINKLOCAL_MULTICAST, iface, &dst))
> + goto err;
> +
> + fd = socket(AF_INET6, SOCK_DGRAM, 0);
> + if (fd == -1) {
> + log_err("Failed to create UDP socket");
> + goto err;
> + }
> +
> + if (sendto(fd, &msg, sizeof(msg), 0, (const struct sockaddr *)&dst,
> + sizeof(dst)) == -1) {
> + log_err("Failed to send datagram");
> + goto err;
> + }
> +
> + goto out;
> +err:
> + err = -1;
> +out:
> + if (fd >= 0)
> + close(fd);
> + return err;
> +}
> +
> +int get_map_fd_by_prog_id(int prog_id)
> +{
> + struct bpf_prog_info info = {};
> + __u32 info_len = sizeof(info);
> + __u32 map_ids[1];
> + int prog_fd = -1;
> + int map_fd = -1;
> +
> + prog_fd = bpf_prog_get_fd_by_id(prog_id);
> + if (prog_fd < 0) {
> + log_err("Failed to get fd by prog id %d", prog_id);
> + goto err;
> + }
> +
> + info.nr_map_ids = 1;
> + info.map_ids = (__u64) (unsigned long) map_ids;
> +
> + if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
> + log_err("Failed to get info by prog fd %d", prog_fd);
> + goto err;
> + }
> +
> + if (!info.nr_map_ids) {
> + log_err("No maps found for prog fd %d", prog_fd);
> + goto err;
> + }
> +
> + map_fd = bpf_map_get_fd_by_id(map_ids[0]);
> + if (map_fd < 0)
> + log_err("Failed to get fd by map id %d", map_ids[0]);
> +err:
> + if (prog_fd >= 0)
> + close(prog_fd);
> + return map_fd;
> +}
> +
> +int check_ancestor_cgroup_ids(int prog_id)
> +{
> + __u64 actual_ids[NUM_CGROUP_LEVELS], expected_ids[NUM_CGROUP_LEVELS];
> + __u32 level;
> + int err = 0;
> + int map_fd;
> +
> + expected_ids[0] = 0x100000001; /* root cgroup */
> + expected_ids[1] = get_cgroup_id("");
> + expected_ids[2] = get_cgroup_id(CGROUP_PATH);
> + expected_ids[3] = 0; /* non-existent cgroup */
> +
> + map_fd = get_map_fd_by_prog_id(prog_id);
> + if (map_fd < 0)
> + goto err;
> +
> + for (level = 0; level < NUM_CGROUP_LEVELS; ++level) {
> + if (bpf_map_lookup_elem(map_fd, &level, &actual_ids[level])) {
> + log_err("Failed to lookup key %d", level);
> + goto err;
> + }
> + if (actual_ids[level] != expected_ids[level]) {
> + log_err("%llx (actual) != %llx (expected), level: %u\n",
> + actual_ids[level], expected_ids[level], level);
> + goto err;
> + }
> + }
> +
> + goto out;
> +err:
> + err = -1;
> +out:
> + if (map_fd >= 0)
> + close(map_fd);
> + return err;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int cgfd = -1;
> + int err = 0;
> +
> + if (argc < 3) {
> + fprintf(stderr, "Usage: %s iface prog_id\n", argv[0]);
> + exit(EXIT_FAILURE);
> + }
> +
> + if (setup_cgroup_environment())
> + goto err;
> +
> + cgfd = create_and_get_cgroup(CGROUP_PATH);
> + if (!cgfd)
> + goto err;
> +
> + if (join_cgroup(CGROUP_PATH))
> + goto err;
> +
> + if (send_packet(argv[1]))
> + goto err;
> +
> + if (check_ancestor_cgroup_ids(atoi(argv[2])))
> + goto err;
> +
> + goto out;
> +err:
> + err = -1;
> +out:
> + close(cgfd);
> + cleanup_cgroup_environment();
> + printf("[%s]\n", err ? "FAIL" : "PASS");
> + return err;
> +}
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
2018-08-12 6:58 ` Yonghong Song
@ 2018-08-12 17:41 ` Andrey Ignatov
0 siblings, 0 replies; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-12 17:41 UTC (permalink / raw)
To: Yonghong Song; +Cc: netdev, ast, daniel, tj, guro, kernel-team
Yonghong Song <yhs@fb.com> [Sat, 2018-08-11 23:59 -0700]:
>
>
> On 8/10/18 10:35 PM, Andrey Ignatov wrote:
> > Add selftests for bpf_skb_ancestor_cgroup_id helper.
> >
> > test_skb_cgroup_id.sh prepares testing interface and adds tc qdisc and
> > filter for it using BPF object compiled from test_skb_cgroup_id_kern.c
> > program.
> >
> > BPF program in test_skb_cgroup_id_kern.c gets ancestor cgroup id using
> > the new helper at different levels of cgroup hierarchy that skb belongs
> > to, including root level and non-existing level, and saves it to the map
> > where the key is the level of corresponding cgroup and the value is its
> > id.
> >
> > To trigger BPF program, user space program test_skb_cgroup_id_user is
> > run. It adds itself into testing cgroup and sends UDP datagram to
> > link-local multicast address of testing interface. Then it reads cgroup
> > ids saved in kernel for different levels from the BPF map and compares
> > them with those in user space. They must be equal for every level of
> > ancestry.
> >
> > Example of run:
> > # ./test_skb_cgroup_id.sh
> > Wait for testing link-local IP to become available ... OK
> > Note: 8 bytes struct bpf_elf_map fixup performed due to size mismatch!
> > [PASS]
>
> I am not able to run the test on my FC27 based VM with the latest bpf-next
> and the patch set.
>
> [yhs@localhost bpf]$ sudo ./test_skb_cgroup_id.sh
> Wait for testing link-local IP to become available .....ERROR: Timeout
> waiting for test IP to become available.
> [yhs@localhost bpf]$
>
> I am able to run test_sock_addr.sh successfully.
> $ sudo ./test_sock_addr.sh
> Wait for testing IPv4/IPv6 to become available .
> .. OK
> Test case: bind4: load prog with wrong expected attach type .. [PASS]
> Test case: bind4: attach prog with wrong attach type .. [PASS]
> ...
> Test case: sendmsg6: deny call .. [PASS]
> Summary: 27 PASSED, 0 FAILED
>
> Maybe some issues in this addr ff02::1%${TEST_IF}?
Thank you for checking it Yonghong!
I was able to repro it on a host different from where I tested
initially.
The problem is ping fails immediately due to link-local IPv6 being
tentative and all MAX_PING_TRIES happen very fast, w/o a chance for the
IPv6 to pass DAD and become ready.
On my original VM IPv6 was becoming ready much faster so even those 5
tries w/o a sleep between them were enough.
The fix is very simple: add `sleep 1` between iterations so there there
is enough time for IPv6 to pass DAD.
I'll send v2 with the fix.
> > Signed-off-by: Andrey Ignatov <rdna@fb.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 9 +-
> > .../selftests/bpf/test_skb_cgroup_id.sh | 61 ++++++
> > .../selftests/bpf/test_skb_cgroup_id_kern.c | 47 +++++
> > .../selftests/bpf/test_skb_cgroup_id_user.c | 187 ++++++++++++++++++
> > 4 files changed, 301 insertions(+), 3 deletions(-)
> > create mode 100755 tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> > create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
> > create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index daed162043c2..fff7fb1285fc 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -34,7 +34,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
> > test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
> > test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
> > test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
> > - get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o
> > + get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
> > + test_skb_cgroup_id_kern.o
> > # Order correspond to 'make run_tests' order
> > TEST_PROGS := test_kmod.sh \
> > @@ -45,10 +46,11 @@ TEST_PROGS := test_kmod.sh \
> > test_sock_addr.sh \
> > test_tunnel.sh \
> > test_lwt_seg6local.sh \
> > - test_lirc_mode2.sh
> > + test_lirc_mode2.sh \
> > + test_skb_cgroup_id.sh
> > # Compile but not part of 'make run_tests'
> > -TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr
> > +TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user
> > include ../lib.mk
> > @@ -59,6 +61,7 @@ $(TEST_GEN_PROGS): $(BPFOBJ)
> > $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a
> > $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
> > +$(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c
> > $(OUTPUT)/test_sock: cgroup_helpers.c
> > $(OUTPUT)/test_sock_addr: cgroup_helpers.c
> > $(OUTPUT)/test_socket_cookie: cgroup_helpers.c
> > diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id.sh b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> > new file mode 100755
> > index 000000000000..b75e9b52f06f
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> > @@ -0,0 +1,61 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2018 Facebook
> > +
> > +set -eu
> > +
> > +wait_for_ip()
> > +{
> > + local _i
> > + echo -n "Wait for testing link-local IP to become available "
> > + for _i in $(seq ${MAX_PING_TRIES}); do
> > + echo -n "."
> > + if ping -6 -q -c 1 -W 1 ff02::1%${TEST_IF} >/dev/null 2>&1; then
> > + echo " OK"
> > + return
> > + fi
> > + done
> > + echo 1>&2 "ERROR: Timeout waiting for test IP to become available."
> > + exit 1
> > +}
> > +
> > +setup()
> > +{
> > + # Create testing interfaces not to interfere with current environment.
> > + ip link add dev ${TEST_IF} type veth peer name ${TEST_IF_PEER}
> > + ip link set ${TEST_IF} up
> > + ip link set ${TEST_IF_PEER} up
> > +
> > + wait_for_ip
> > +
> > + tc qdisc add dev ${TEST_IF} clsact
> > + tc filter add dev ${TEST_IF} egress bpf obj ${BPF_PROG_OBJ} \
> > + sec ${BPF_PROG_SECTION} da
> > +
> > + BPF_PROG_ID=$(tc filter show dev ${TEST_IF} egress | \
> > + awk '/ id / {sub(/.* id /, "", $0); print($1)}')
> > +}
> > +
> > +cleanup()
> > +{
> > + ip link del ${TEST_IF} 2>/dev/null || :
> > + ip link del ${TEST_IF_PEER} 2>/dev/null || :
> > +}
> > +
> > +main()
> > +{
> > + trap cleanup EXIT 2 3 6 15
> > + setup
> > + ${PROG} ${TEST_IF} ${BPF_PROG_ID}
> > +}
> > +
> > +DIR=$(dirname $0)
> > +TEST_IF="test_cgid_1"
> > +TEST_IF_PEER="test_cgid_2"
> > +MAX_PING_TRIES=5
> > +BPF_PROG_OBJ="${DIR}/test_skb_cgroup_id_kern.o"
> > +BPF_PROG_SECTION="cgroup_id_logger"
> > +BPF_PROG_ID=0
> > +PROG="${DIR}/test_skb_cgroup_id_user"
> > +
> > +main
> > diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
> > new file mode 100644
> > index 000000000000..68cf9829f5a7
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 Facebook
> > +
> > +#include <linux/bpf.h>
> > +#include <linux/pkt_cls.h>
> > +
> > +#include <string.h>
> > +
> > +#include "bpf_helpers.h"
> > +
> > +#define NUM_CGROUP_LEVELS 4
> > +
> > +struct bpf_map_def SEC("maps") cgroup_ids = {
> > + .type = BPF_MAP_TYPE_ARRAY,
> > + .key_size = sizeof(__u32),
> > + .value_size = sizeof(__u64),
> > + .max_entries = NUM_CGROUP_LEVELS,
> > +};
> > +
> > +static __always_inline void log_nth_level(struct __sk_buff *skb, __u32 level)
> > +{
> > + __u64 id;
> > +
> > + /* [1] &level passed to external function that may change it, it's
> > + * incompatible with loop unroll.
> > + */
> > + id = bpf_skb_ancestor_cgroup_id(skb, level);
> > + bpf_map_update_elem(&cgroup_ids, &level, &id, 0);
> > +}
> > +
> > +SEC("cgroup_id_logger")
> > +int log_cgroup_id(struct __sk_buff *skb)
> > +{
> > + /* Loop unroll can't be used here due to [1]. Unrolling manually.
> > + * Number of calls should be in sync with NUM_CGROUP_LEVELS.
> > + */
> > + log_nth_level(skb, 0);
> > + log_nth_level(skb, 1);
> > + log_nth_level(skb, 2);
> > + log_nth_level(skb, 3);
> > +
> > + return TC_ACT_OK;
> > +}
> > +
> > +int _version SEC("version") = 1;
> > +
> > +char _license[] SEC("license") = "GPL";
> > diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
> > new file mode 100644
> > index 000000000000..c121cc59f314
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
> > @@ -0,0 +1,187 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 Facebook
> > +
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +
> > +#include <arpa/inet.h>
> > +#include <net/if.h>
> > +#include <netinet/in.h>
> > +#include <sys/socket.h>
> > +#include <sys/types.h>
> > +
> > +
> > +#include <bpf/bpf.h>
> > +#include <bpf/libbpf.h>
> > +
> > +#include "bpf_rlimit.h"
> > +#include "cgroup_helpers.h"
> > +
> > +#define CGROUP_PATH "/skb_cgroup_test"
> > +#define NUM_CGROUP_LEVELS 4
> > +
> > +/* RFC 4291, Section 2.7.1 */
> > +#define LINKLOCAL_MULTICAST "ff02::1"
> > +
> > +static int mk_dst_addr(const char *ip, const char *iface,
> > + struct sockaddr_in6 *dst)
> > +{
> > + memset(dst, 0, sizeof(*dst));
> > +
> > + dst->sin6_family = AF_INET6;
> > + dst->sin6_port = htons(1025);
> > +
> > + if (inet_pton(AF_INET6, ip, &dst->sin6_addr) != 1) {
> > + log_err("Invalid IPv6: %s", ip);
> > + return -1;
> > + }
> > +
> > + dst->sin6_scope_id = if_nametoindex(iface);
> > + if (!dst->sin6_scope_id) {
> > + log_err("Failed to get index of iface: %s", iface);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int send_packet(const char *iface)
> > +{
> > + struct sockaddr_in6 dst;
> > + char msg[] = "msg";
> > + int err = 0;
> > + int fd = -1;
> > +
> > + if (mk_dst_addr(LINKLOCAL_MULTICAST, iface, &dst))
> > + goto err;
> > +
> > + fd = socket(AF_INET6, SOCK_DGRAM, 0);
> > + if (fd == -1) {
> > + log_err("Failed to create UDP socket");
> > + goto err;
> > + }
> > +
> > + if (sendto(fd, &msg, sizeof(msg), 0, (const struct sockaddr *)&dst,
> > + sizeof(dst)) == -1) {
> > + log_err("Failed to send datagram");
> > + goto err;
> > + }
> > +
> > + goto out;
> > +err:
> > + err = -1;
> > +out:
> > + if (fd >= 0)
> > + close(fd);
> > + return err;
> > +}
> > +
> > +int get_map_fd_by_prog_id(int prog_id)
> > +{
> > + struct bpf_prog_info info = {};
> > + __u32 info_len = sizeof(info);
> > + __u32 map_ids[1];
> > + int prog_fd = -1;
> > + int map_fd = -1;
> > +
> > + prog_fd = bpf_prog_get_fd_by_id(prog_id);
> > + if (prog_fd < 0) {
> > + log_err("Failed to get fd by prog id %d", prog_id);
> > + goto err;
> > + }
> > +
> > + info.nr_map_ids = 1;
> > + info.map_ids = (__u64) (unsigned long) map_ids;
> > +
> > + if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
> > + log_err("Failed to get info by prog fd %d", prog_fd);
> > + goto err;
> > + }
> > +
> > + if (!info.nr_map_ids) {
> > + log_err("No maps found for prog fd %d", prog_fd);
> > + goto err;
> > + }
> > +
> > + map_fd = bpf_map_get_fd_by_id(map_ids[0]);
> > + if (map_fd < 0)
> > + log_err("Failed to get fd by map id %d", map_ids[0]);
> > +err:
> > + if (prog_fd >= 0)
> > + close(prog_fd);
> > + return map_fd;
> > +}
> > +
> > +int check_ancestor_cgroup_ids(int prog_id)
> > +{
> > + __u64 actual_ids[NUM_CGROUP_LEVELS], expected_ids[NUM_CGROUP_LEVELS];
> > + __u32 level;
> > + int err = 0;
> > + int map_fd;
> > +
> > + expected_ids[0] = 0x100000001; /* root cgroup */
> > + expected_ids[1] = get_cgroup_id("");
> > + expected_ids[2] = get_cgroup_id(CGROUP_PATH);
> > + expected_ids[3] = 0; /* non-existent cgroup */
> > +
> > + map_fd = get_map_fd_by_prog_id(prog_id);
> > + if (map_fd < 0)
> > + goto err;
> > +
> > + for (level = 0; level < NUM_CGROUP_LEVELS; ++level) {
> > + if (bpf_map_lookup_elem(map_fd, &level, &actual_ids[level])) {
> > + log_err("Failed to lookup key %d", level);
> > + goto err;
> > + }
> > + if (actual_ids[level] != expected_ids[level]) {
> > + log_err("%llx (actual) != %llx (expected), level: %u\n",
> > + actual_ids[level], expected_ids[level], level);
> > + goto err;
> > + }
> > + }
> > +
> > + goto out;
> > +err:
> > + err = -1;
> > +out:
> > + if (map_fd >= 0)
> > + close(map_fd);
> > + return err;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > + int cgfd = -1;
> > + int err = 0;
> > +
> > + if (argc < 3) {
> > + fprintf(stderr, "Usage: %s iface prog_id\n", argv[0]);
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + if (setup_cgroup_environment())
> > + goto err;
> > +
> > + cgfd = create_and_get_cgroup(CGROUP_PATH);
> > + if (!cgfd)
> > + goto err;
> > +
> > + if (join_cgroup(CGROUP_PATH))
> > + goto err;
> > +
> > + if (send_packet(argv[1]))
> > + goto err;
> > +
> > + if (check_ancestor_cgroup_ids(atoi(argv[2])))
> > + goto err;
> > +
> > + goto out;
> > +err:
> > + err = -1;
> > +out:
> > + close(cgfd);
> > + cleanup_cgroup_environment();
> > + printf("[%s]\n", err ? "FAIL" : "PASS");
> > + return err;
> > +}
> >
--
Andrey Ignatov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
2018-08-11 5:35 ` [PATCH bpf-next 1/4] bpf: Introduce " Andrey Ignatov
@ 2018-08-13 14:46 ` Tejun Heo
2018-08-13 15:39 ` Andrey Ignatov
0 siblings, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2018-08-13 14:46 UTC (permalink / raw)
To: Andrey Ignatov; +Cc: netdev, ast, daniel, guro, kernel-team
Hello, Andrey.
On Fri, Aug 10, 2018 at 10:35:23PM -0700, Andrey Ignatov wrote:
> +static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> + int ancestor_level)
> +{
> + struct cgroup *ptr;
> +
> + if (cgrp->level < ancestor_level)
> + return NULL;
> +
> + for (ptr = cgrp;
> + ptr && ptr->level > ancestor_level;
> + ptr = cgroup_parent(ptr))
> + ;
> +
> + if (ptr && ptr->level == ancestor_level)
> + return ptr;
> +
> + return NULL;
> +}
I don't have any objections functionanlity-wise but can we do sth like
the following instead?
static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
int ancestor_level)
{
if (cgrp->level < ancestor_level)
return NULL;
while (cgrp->level > ancestor_level)
cgrp = cgroup_parent(cgrp);
return cgrp;
}
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
2018-08-13 14:46 ` Tejun Heo
@ 2018-08-13 15:39 ` Andrey Ignatov
0 siblings, 0 replies; 9+ messages in thread
From: Andrey Ignatov @ 2018-08-13 15:39 UTC (permalink / raw)
To: Tejun Heo; +Cc: netdev, ast, daniel, guro, kernel-team
Tejun Heo <tj@kernel.org> [Mon, 2018-08-13 07:47 -0700]:
> Hello, Andrey.
Hey Tejun!
> On Fri, Aug 10, 2018 at 10:35:23PM -0700, Andrey Ignatov wrote:
> > +static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> > + int ancestor_level)
> > +{
> > + struct cgroup *ptr;
> > +
> > + if (cgrp->level < ancestor_level)
> > + return NULL;
> > +
> > + for (ptr = cgrp;
> > + ptr && ptr->level > ancestor_level;
> > + ptr = cgroup_parent(ptr))
> > + ;
> > +
> > + if (ptr && ptr->level == ancestor_level)
> > + return ptr;
> > +
> > + return NULL;
> > +}
>
> I don't have any objections functionanlity-wise but can we do sth like
> the following instead?
>
> static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> int ancestor_level)
> {
> if (cgrp->level < ancestor_level)
> return NULL;
>
> while (cgrp->level > ancestor_level)
> cgrp = cgroup_parent(cgrp);
> return cgrp;
> }
Sure, no problem. I'll send follow-up patch to simplify this part when
bpf-next is open again.
>
> Thanks.
>
> --
> tejun
--
Andrey Ignatov
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-08-13 18:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-11 5:35 [PATCH bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 1/4] bpf: Introduce " Andrey Ignatov
2018-08-13 14:46 ` Tejun Heo
2018-08-13 15:39 ` Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 2/4] bpf: Sync bpf.h to tools/ Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 3/4] selftests/bpf: Add cgroup id helpers to bpf_helpers.h Andrey Ignatov
2018-08-11 5:35 ` [PATCH bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id Andrey Ignatov
2018-08-12 6:58 ` Yonghong Song
2018-08-12 17:41 ` Andrey Ignatov
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).