Prepare to use (struct bpf_dynptr)->offset to distinguish between an skb dynptr for the payload vs the metadata area. ptr->offset is always set to zero by bpf_dynptr_from_skb(). We don't need to account for it on access. Signed-off-by: Jakub Sitnicki --- kernel/bpf/helpers.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index f48fa3fe8dec..40c18b37ab05 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1776,7 +1776,7 @@ static int __bpf_dynptr_read(void *dst, u32 len, const struct bpf_dynptr_kern *s memmove(dst, src->data + src->offset + offset, len); return 0; case BPF_DYNPTR_TYPE_SKB: - return __bpf_skb_load_bytes(src->data, src->offset + offset, dst, len); + return __bpf_skb_load_bytes(src->data, offset, dst, len); case BPF_DYNPTR_TYPE_XDP: return __bpf_xdp_load_bytes(src->data, src->offset + offset, dst, len); default: @@ -1829,8 +1829,7 @@ int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset, void *src, memmove(dst->data + dst->offset + offset, src, len); return 0; case BPF_DYNPTR_TYPE_SKB: - return __bpf_skb_store_bytes(dst->data, dst->offset + offset, src, len, - flags); + return __bpf_skb_store_bytes(dst->data, offset, src, len, flags); case BPF_DYNPTR_TYPE_XDP: if (flags) return -EINVAL; @@ -2695,9 +2694,9 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr *p, u32 offset, return ptr->data + ptr->offset + offset; case BPF_DYNPTR_TYPE_SKB: if (buffer__opt) - return skb_header_pointer(ptr->data, ptr->offset + offset, len, buffer__opt); + return skb_header_pointer(ptr->data, offset, len, buffer__opt); else - return skb_pointer_if_linear(ptr->data, ptr->offset + offset, len); + return skb_pointer_if_linear(ptr->data, offset, len); case BPF_DYNPTR_TYPE_XDP: { void *xdp_ptr = bpf_xdp_pointer(ptr->data, ptr->offset + offset, len); -- 2.43.0 Prepare to handle skb dynptrs for accessing the metadata area. Code move. No observable changes. Signed-off-by: Jakub Sitnicki --- include/linux/filter.h | 25 ++++++++++++++++++------- kernel/bpf/helpers.c | 9 +++------ net/core/filter.c | 38 +++++++++++++++++++++++++++----------- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index eca229752cbe..468d83604241 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -1764,27 +1764,38 @@ static __always_inline long __bpf_xdp_redirect_map(struct bpf_map *map, u64 inde } #ifdef CONFIG_NET -int __bpf_skb_load_bytes(const struct sk_buff *skb, u32 offset, void *to, u32 len); -int __bpf_skb_store_bytes(struct sk_buff *skb, u32 offset, const void *from, - u32 len, u64 flags); +int bpf_dynptr_skb_read(const struct bpf_dynptr_kern *src, u32 offset, + void *dst, u32 len); +int bpf_dynptr_skb_write(const struct bpf_dynptr_kern *dst, u32 offset, + const void *src, u32 len, u64 flags); +void *bpf_dynptr_skb_slice(const struct bpf_dynptr_kern *ptr, u32 offset, + void *buf, u32 len); int __bpf_xdp_load_bytes(struct xdp_buff *xdp, u32 offset, void *buf, u32 len); int __bpf_xdp_store_bytes(struct xdp_buff *xdp, u32 offset, void *buf, u32 len); void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len); void bpf_xdp_copy_buf(struct xdp_buff *xdp, unsigned long off, void *buf, unsigned long len, bool flush); #else /* CONFIG_NET */ -static inline int __bpf_skb_load_bytes(const struct sk_buff *skb, u32 offset, - void *to, u32 len) +static inline int bpf_dynptr_skb_read(const struct bpf_dynptr_kern *src, + u32 offset, void *dst, u32 len) { return -EOPNOTSUPP; } -static inline int __bpf_skb_store_bytes(struct sk_buff *skb, u32 offset, - const void *from, u32 len, u64 flags) +static inline int bpf_dynptr_skb_write(const struct bpf_dynptr_kern *dst, + u32 offset, const void *src, u32 len, + u64 flags); { return -EOPNOTSUPP; } +static inline void *bpf_dynptr_skb_slice(const struct bpf_dynptr_kern *ptr, + u32 offset, void *buf, u32 len); + +{ + return NULL; +} + static inline int __bpf_xdp_load_bytes(struct xdp_buff *xdp, u32 offset, void *buf, u32 len) { diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 40c18b37ab05..da9c6ccd7cd7 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1776,7 +1776,7 @@ static int __bpf_dynptr_read(void *dst, u32 len, const struct bpf_dynptr_kern *s memmove(dst, src->data + src->offset + offset, len); return 0; case BPF_DYNPTR_TYPE_SKB: - return __bpf_skb_load_bytes(src->data, offset, dst, len); + return bpf_dynptr_skb_read(src, offset, dst, len); case BPF_DYNPTR_TYPE_XDP: return __bpf_xdp_load_bytes(src->data, src->offset + offset, dst, len); default: @@ -1829,7 +1829,7 @@ int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset, void *src, memmove(dst->data + dst->offset + offset, src, len); return 0; case BPF_DYNPTR_TYPE_SKB: - return __bpf_skb_store_bytes(dst->data, offset, src, len, flags); + return bpf_dynptr_skb_write(dst, offset, src, len, flags); case BPF_DYNPTR_TYPE_XDP: if (flags) return -EINVAL; @@ -2693,10 +2693,7 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr *p, u32 offset, case BPF_DYNPTR_TYPE_RINGBUF: return ptr->data + ptr->offset + offset; case BPF_DYNPTR_TYPE_SKB: - if (buffer__opt) - return skb_header_pointer(ptr->data, offset, len, buffer__opt); - else - return skb_pointer_if_linear(ptr->data, offset, len); + return bpf_dynptr_skb_slice(ptr, offset, buffer__opt, len); case BPF_DYNPTR_TYPE_XDP: { void *xdp_ptr = bpf_xdp_pointer(ptr->data, ptr->offset + offset, len); diff --git a/net/core/filter.c b/net/core/filter.c index 7a72f766aacf..1fee51b72220 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1736,12 +1736,6 @@ static const struct bpf_func_proto bpf_skb_store_bytes_proto = { .arg5_type = ARG_ANYTHING, }; -int __bpf_skb_store_bytes(struct sk_buff *skb, u32 offset, const void *from, - u32 len, u64 flags) -{ - return ____bpf_skb_store_bytes(skb, offset, from, len, flags); -} - BPF_CALL_4(bpf_skb_load_bytes, const struct sk_buff *, skb, u32, offset, void *, to, u32, len) { @@ -1772,11 +1766,6 @@ static const struct bpf_func_proto bpf_skb_load_bytes_proto = { .arg4_type = ARG_CONST_SIZE, }; -int __bpf_skb_load_bytes(const struct sk_buff *skb, u32 offset, void *to, u32 len) -{ - return ____bpf_skb_load_bytes(skb, offset, to, len); -} - BPF_CALL_4(bpf_flow_dissector_load_bytes, const struct bpf_flow_dissector *, ctx, u32, offset, void *, to, u32, len) @@ -11978,6 +11967,33 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return func; } +int bpf_dynptr_skb_read(const struct bpf_dynptr_kern *src, u32 offset, + void *dst, u32 len) +{ + const struct sk_buff *skb = src->data; + + return ____bpf_skb_load_bytes(skb, offset, dst, len); +} + +int bpf_dynptr_skb_write(const struct bpf_dynptr_kern *dst, u32 offset, + const void *src, u32 len, u64 flags) +{ + struct sk_buff *skb = dst->data; + + return ____bpf_skb_store_bytes(skb, offset, src, len, flags); +} + +void *bpf_dynptr_skb_slice(const struct bpf_dynptr_kern *ptr, u32 offset, + void *buf, u32 len) +{ + const struct sk_buff *skb = ptr->data; + + if (buf) + return skb_header_pointer(skb, offset, len, buf); + else + return skb_pointer_if_linear(skb, offset, len); +} + __bpf_kfunc_start_defs(); __bpf_kfunc int bpf_dynptr_from_skb(struct __sk_buff *s, u64 flags, struct bpf_dynptr *ptr__uninit) -- 2.43.0 Add a new flag for the bpf_dynptr_from_skb helper to let users to create dynptrs to skb metadata area. Access paths are stubbed out. Implemented by the following changes. Signed-off-by: Jakub Sitnicki --- include/uapi/linux/bpf.h | 9 ++++++++ net/core/filter.c | 60 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 719ba230032f..ab5730d2fb29 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -7591,4 +7591,13 @@ enum bpf_kfunc_flags { BPF_F_PAD_ZEROS = (1ULL << 0), }; +/** + * enum bpf_dynptr_from_skb_flags - Flags for bpf_dynptr_from_skb() + * + * @BPF_DYNPTR_F_SKB_METADATA: Create dynptr to the SKB metadata area + */ +enum bpf_dynptr_from_skb_flags { + BPF_DYNPTR_F_SKB_METADATA = (1ULL << 0), +}; + #endif /* _UAPI__LINUX_BPF_H__ */ diff --git a/net/core/filter.c b/net/core/filter.c index 1fee51b72220..3c2948517838 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -11967,12 +11967,27 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return func; } +enum skb_dynptr_offset { + SKB_DYNPTR_METADATA = -1, + SKB_DYNPTR_PAYLOAD = 0, +}; + int bpf_dynptr_skb_read(const struct bpf_dynptr_kern *src, u32 offset, void *dst, u32 len) { const struct sk_buff *skb = src->data; - return ____bpf_skb_load_bytes(skb, offset, dst, len); + switch (src->offset) { + case SKB_DYNPTR_PAYLOAD: + return ____bpf_skb_load_bytes(skb, offset, dst, len); + + case SKB_DYNPTR_METADATA: + return -EOPNOTSUPP; /* not implemented */ + + default: + WARN_ONCE(true, "%s: unknown skb dynptr offset %d\n", __func__, src->offset); + return -EFAULT; + } } int bpf_dynptr_skb_write(const struct bpf_dynptr_kern *dst, u32 offset, @@ -11980,7 +11995,17 @@ int bpf_dynptr_skb_write(const struct bpf_dynptr_kern *dst, u32 offset, { struct sk_buff *skb = dst->data; - return ____bpf_skb_store_bytes(skb, offset, src, len, flags); + switch (dst->offset) { + case SKB_DYNPTR_PAYLOAD: + return ____bpf_skb_store_bytes(skb, offset, src, len, flags); + + case SKB_DYNPTR_METADATA: + return -EOPNOTSUPP; /* not implemented */ + + default: + WARN_ONCE(true, "%s: unknown skb dynptr offset %d\n", __func__, dst->offset); + return -EFAULT; + } } void *bpf_dynptr_skb_slice(const struct bpf_dynptr_kern *ptr, u32 offset, @@ -11988,10 +12013,20 @@ void *bpf_dynptr_skb_slice(const struct bpf_dynptr_kern *ptr, u32 offset, { const struct sk_buff *skb = ptr->data; - if (buf) - return skb_header_pointer(skb, offset, len, buf); - else - return skb_pointer_if_linear(skb, offset, len); + switch (ptr->offset) { + case SKB_DYNPTR_PAYLOAD: + if (buf) + return skb_header_pointer(skb, offset, len, buf); + else + return skb_pointer_if_linear(skb, offset, len); + + case SKB_DYNPTR_METADATA: + return NULL; /* not implemented */ + + default: + WARN_ONCE(true, "%s: unknown skb dynptr offset %d\n", __func__, ptr->offset); + return NULL; + } } __bpf_kfunc_start_defs(); @@ -12000,13 +12035,22 @@ __bpf_kfunc int bpf_dynptr_from_skb(struct __sk_buff *s, u64 flags, { struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit; struct sk_buff *skb = (struct sk_buff *)s; + u32 offset, size; - if (flags) { + if (unlikely(flags & ~BPF_DYNPTR_F_SKB_METADATA)) { bpf_dynptr_set_null(ptr); return -EINVAL; } - bpf_dynptr_init(ptr, skb, BPF_DYNPTR_TYPE_SKB, 0, skb->len); + if (flags & BPF_DYNPTR_F_SKB_METADATA) { + offset = SKB_DYNPTR_METADATA; + size = skb_metadata_len(skb); + } else { + offset = SKB_DYNPTR_PAYLOAD; + size = skb->len; + } + + bpf_dynptr_init(ptr, skb, BPF_DYNPTR_TYPE_SKB, offset, size); return 0; } -- 2.43.0 Make it possible to read from skb metadata area using the bpf_dynptr_read() BPF helper. This prepares ground for access to skb metadata from all BPF hooks which operate on __sk_buff context. Signed-off-by: Jakub Sitnicki --- net/core/filter.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/net/core/filter.c b/net/core/filter.c index 3c2948517838..f71b4b6b09fb 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -11981,9 +11981,15 @@ int bpf_dynptr_skb_read(const struct bpf_dynptr_kern *src, u32 offset, case SKB_DYNPTR_PAYLOAD: return ____bpf_skb_load_bytes(skb, offset, dst, len); - case SKB_DYNPTR_METADATA: - return -EOPNOTSUPP; /* not implemented */ + case SKB_DYNPTR_METADATA: { + u32 meta_len = skb_metadata_len(skb); + + if (len > meta_len || offset > meta_len - len) + return -E2BIG; /* out of bounds */ + memmove(dst, skb_metadata_end(skb) - meta_len + offset, len); + return 0; + } default: WARN_ONCE(true, "%s: unknown skb dynptr offset %d\n", __func__, src->offset); return -EFAULT; -- 2.43.0 Make it possible to write to skb metadata area using the bpf_dynptr_write() BPF helper. This prepares ground for access to skb metadata from all BPF hooks which operate on __sk_buff context. Signed-off-by: Jakub Sitnicki --- net/core/filter.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/net/core/filter.c b/net/core/filter.c index f71b4b6b09fb..ab6599f42bb7 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -12005,8 +12005,15 @@ int bpf_dynptr_skb_write(const struct bpf_dynptr_kern *dst, u32 offset, case SKB_DYNPTR_PAYLOAD: return ____bpf_skb_store_bytes(skb, offset, src, len, flags); - case SKB_DYNPTR_METADATA: - return -EOPNOTSUPP; /* not implemented */ + case SKB_DYNPTR_METADATA: { + u32 meta_len = skb_metadata_len(skb); + + if (len > meta_len || offset > meta_len - len) + return -E2BIG; /* out of bounds */ + + memmove(skb_metadata_end(skb) - meta_len + offset, src, len); + return 0; + } default: WARN_ONCE(true, "%s: unknown skb dynptr offset %d\n", __func__, dst->offset); -- 2.43.0 Make it possible to read from or write to skb metadata area using the dynptr slices creates with bpf_dynptr_slice() or bpf_dynptr_slice_rdwr(). This prepares ground for access to skb metadata from all BPF hooks which operate on __sk_buff context. Signed-off-by: Jakub Sitnicki --- net/core/filter.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/net/core/filter.c b/net/core/filter.c index ab6599f42bb7..020da46f93a7 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -12033,9 +12033,14 @@ void *bpf_dynptr_skb_slice(const struct bpf_dynptr_kern *ptr, u32 offset, else return skb_pointer_if_linear(skb, offset, len); - case SKB_DYNPTR_METADATA: - return NULL; /* not implemented */ + case SKB_DYNPTR_METADATA: { + u32 meta_len = skb_metadata_len(skb); + if (len > meta_len || offset > meta_len - len) + return NULL; /* out of bounds */ + + return skb_metadata_end(skb) - meta_len + offset; + } default: WARN_ONCE(true, "%s: unknown skb dynptr offset %d\n", __func__, ptr->offset); return NULL; -- 2.43.0 With the extension of bpf_dynptr_from_skb(BPF_DYNPTR_F_SKB_METADATA), all BPF programs authorized to call this kfunc now have access to the skb metadata area. These programs can read up to skb_shinfo(skb)->meta_len bytes located just before skb_mac_header(skb), regardless of what data is currently there. However, as the network stack processes the skb, headers may be added or removed. Hence, we cannot assume that skb_mac_header() always marks the end of the metadata area. To avoid potential pitfalls, reset the skb metadata length to zero before passing the skb to the protocol layers. This is a temporary measure until we can make metadata persist through protocol processing. Signed-off-by: Jakub Sitnicki --- net/core/dev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/core/dev.c b/net/core/dev.c index be97c440ecd5..4a2389997535 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5839,6 +5839,7 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc, } #endif skb_reset_redirect(skb); + skb_metadata_clear(skb); skip_classify: if (pfmemalloc && !skb_pfmemalloc_protocol(skb)) goto drop; -- 2.43.0 Prepare for parametrizing the xdp_context tests. The assert_test_result helper doesn't need the whole skeleton. Pass just what it needs. Signed-off-by: Jakub Sitnicki --- tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index b9d9f0a502ce..0134651d94ab 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -156,15 +156,14 @@ static int send_test_packet(int ifindex) return -1; } -static void assert_test_result(struct test_xdp_meta *skel) +static void assert_test_result(const struct bpf_map *result_map) { int err; __u32 map_key = 0; __u8 map_value[TEST_PAYLOAD_LEN]; - err = bpf_map__lookup_elem(skel->maps.test_result, &map_key, - sizeof(map_key), &map_value, - TEST_PAYLOAD_LEN, BPF_ANY); + err = bpf_map__lookup_elem(result_map, &map_key, sizeof(map_key), + &map_value, TEST_PAYLOAD_LEN, BPF_ANY); if (!ASSERT_OK(err, "lookup test_result")) return; @@ -248,7 +247,7 @@ void test_xdp_context_veth(void) if (!ASSERT_OK(ret, "send_test_packet")) goto close; - assert_test_result(skel); + assert_test_result(skel->maps.test_result); close: close_netns(nstoken); @@ -313,7 +312,7 @@ void test_xdp_context_tuntap(void) if (!ASSERT_EQ(ret, sizeof(packet), "write packet")) goto close; - assert_test_result(skel); + assert_test_result(skel->maps.test_result); close: if (tap_fd >= 0) -- 2.43.0 We want to add more test cases to cover different ways to access the metadata area. Prepare for it. Pull up the skeleton management. Signed-off-by: Jakub Sitnicki --- .../bpf/prog_tests/xdp_context_test_run.c | 31 +++++++++++++++------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index 0134651d94ab..6c66e27e5bc7 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -256,12 +256,13 @@ void test_xdp_context_veth(void) netns_free(tx_ns); } -void test_xdp_context_tuntap(void) +static void test_tuntap(struct bpf_program *xdp_prog, + struct bpf_program *tc_prog, + struct bpf_map *result_map) { LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1); struct netns_obj *ns = NULL; - struct test_xdp_meta *skel = NULL; __u8 packet[sizeof(struct ethhdr) + TEST_PAYLOAD_LEN]; int tap_fd = -1; int tap_ifindex; @@ -277,10 +278,6 @@ void test_xdp_context_tuntap(void) SYS(close, "ip link set dev " TAP_NAME " up"); - skel = test_xdp_meta__open_and_load(); - if (!ASSERT_OK_PTR(skel, "open and load skeleton")) - goto close; - tap_ifindex = if_nametoindex(TAP_NAME); if (!ASSERT_GE(tap_ifindex, 0, "if_nametoindex")) goto close; @@ -290,12 +287,12 @@ void test_xdp_context_tuntap(void) if (!ASSERT_OK(ret, "bpf_tc_hook_create")) goto close; - tc_opts.prog_fd = bpf_program__fd(skel->progs.ing_cls); + tc_opts.prog_fd = bpf_program__fd(tc_prog); ret = bpf_tc_attach(&tc_hook, &tc_opts); if (!ASSERT_OK(ret, "bpf_tc_attach")) goto close; - ret = bpf_xdp_attach(tap_ifindex, bpf_program__fd(skel->progs.ing_xdp), + ret = bpf_xdp_attach(tap_ifindex, bpf_program__fd(xdp_prog), 0, NULL); if (!ASSERT_GE(ret, 0, "bpf_xdp_attach")) goto close; @@ -312,11 +309,25 @@ void test_xdp_context_tuntap(void) if (!ASSERT_EQ(ret, sizeof(packet), "write packet")) goto close; - assert_test_result(skel->maps.test_result); + assert_test_result(result_map); close: if (tap_fd >= 0) close(tap_fd); - test_xdp_meta__destroy(skel); netns_free(ns); } + +void test_xdp_context_tuntap(void) +{ + struct test_xdp_meta *skel = NULL; + + skel = test_xdp_meta__open_and_load(); + if (!ASSERT_OK_PTR(skel, "open and load skeleton")) + return; + + if (test__start_subtest("data_meta")) + test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls, + skel->maps.test_result); + + test_xdp_meta__destroy(skel); +} -- 2.43.0 Exercise reading from SKB metadata area in two new ways: 1. indirectly, with bpf_dynptr_read(), and 2. directly, with bpf_dynptr_slice(). Signed-off-by: Jakub Sitnicki --- tools/include/uapi/linux/bpf.h | 9 +++++ .../bpf/prog_tests/xdp_context_test_run.c | 21 +++++++++++ tools/testing/selftests/bpf/progs/test_xdp_meta.c | 42 ++++++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 719ba230032f..ab5730d2fb29 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -7591,4 +7591,13 @@ enum bpf_kfunc_flags { BPF_F_PAD_ZEROS = (1ULL << 0), }; +/** + * enum bpf_dynptr_from_skb_flags - Flags for bpf_dynptr_from_skb() + * + * @BPF_DYNPTR_F_SKB_METADATA: Create dynptr to the SKB metadata area + */ +enum bpf_dynptr_from_skb_flags { + BPF_DYNPTR_F_SKB_METADATA = (1ULL << 0), +}; + #endif /* _UAPI__LINUX_BPF_H__ */ diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index 6c66e27e5bc7..7e4526461a4c 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -171,6 +171,18 @@ static void assert_test_result(const struct bpf_map *result_map) "test_result map contains test payload"); } +static bool clear_test_result(struct bpf_map *result_map) +{ + const __u8 v[sizeof(test_payload)] = {}; + const __u32 k = 0; + int err; + + err = bpf_map__update_elem(result_map, &k, sizeof(k), v, sizeof(v), BPF_ANY); + ASSERT_OK(err, "update test_result"); + + return err == 0; +} + void test_xdp_context_veth(void) { LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); @@ -268,6 +280,9 @@ static void test_tuntap(struct bpf_program *xdp_prog, int tap_ifindex; int ret; + if (!clear_test_result(result_map)) + return; + ns = netns_new(TAP_NETNS, true); if (!ASSERT_OK_PTR(ns, "create and open ns")) return; @@ -328,6 +343,12 @@ void test_xdp_context_tuntap(void) if (test__start_subtest("data_meta")) test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls, skel->maps.test_result); + if (test__start_subtest("dynptr_read")) + test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_read, + skel->maps.test_result); + if (test__start_subtest("dynptr_slice")) + test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_slice, + skel->maps.test_result); test_xdp_meta__destroy(skel); } diff --git a/tools/testing/selftests/bpf/progs/test_xdp_meta.c b/tools/testing/selftests/bpf/progs/test_xdp_meta.c index fcf6ca14f2ea..6b11134520be 100644 --- a/tools/testing/selftests/bpf/progs/test_xdp_meta.c +++ b/tools/testing/selftests/bpf/progs/test_xdp_meta.c @@ -1,8 +1,10 @@ +#include #include #include #include #include +#include "bpf_kfuncs.h" #define META_SIZE 32 @@ -40,6 +42,46 @@ int ing_cls(struct __sk_buff *ctx) return TC_ACT_SHOT; } +/* Read from metadata using bpf_dynptr_read helper */ +SEC("tc") +int ing_cls_dynptr_read(struct __sk_buff *ctx) +{ + struct bpf_dynptr meta; + const __u32 zero = 0; + __u8 *dst; + + dst = bpf_map_lookup_elem(&test_result, &zero); + if (!dst) + return TC_ACT_SHOT; + + bpf_dynptr_from_skb(ctx, BPF_DYNPTR_F_SKB_METADATA, &meta); + bpf_dynptr_read(dst, META_SIZE, &meta, 0, 0); + + return TC_ACT_SHOT; +} + +/* Read from metadata using read-only dynptr slice */ +SEC("tc") +int ing_cls_dynptr_slice(struct __sk_buff *ctx) +{ + struct bpf_dynptr meta; + const __u32 zero = 0; + __u8 *dst, *src; + + dst = bpf_map_lookup_elem(&test_result, &zero); + if (!dst) + return TC_ACT_SHOT; + + bpf_dynptr_from_skb(ctx, BPF_DYNPTR_F_SKB_METADATA, &meta); + src = bpf_dynptr_slice(&meta, 0, NULL, META_SIZE); + if (!src) + return TC_ACT_SHOT; + + __builtin_memcpy(dst, src, META_SIZE); + + return TC_ACT_SHOT; +} + SEC("xdp") int ing_xdp(struct xdp_md *ctx) { -- 2.43.0 Add tests what exercise writes to skb metadata in two ways: 1. indirectly, using bpf_dynptr_write helper, 2. directly, using a read-write dynptr slice. Signed-off-by: Jakub Sitnicki --- .../bpf/prog_tests/xdp_context_test_run.c | 36 ++++++++++-- tools/testing/selftests/bpf/progs/test_xdp_meta.c | 67 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index 7e4526461a4c..79c4c58276e6 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -269,7 +269,8 @@ void test_xdp_context_veth(void) } static void test_tuntap(struct bpf_program *xdp_prog, - struct bpf_program *tc_prog, + struct bpf_program *tc_prio_1_prog, + struct bpf_program *tc_prio_2_prog, struct bpf_map *result_map) { LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); @@ -302,11 +303,20 @@ static void test_tuntap(struct bpf_program *xdp_prog, if (!ASSERT_OK(ret, "bpf_tc_hook_create")) goto close; - tc_opts.prog_fd = bpf_program__fd(tc_prog); + tc_opts.prog_fd = bpf_program__fd(tc_prio_1_prog); ret = bpf_tc_attach(&tc_hook, &tc_opts); if (!ASSERT_OK(ret, "bpf_tc_attach")) goto close; + if (tc_prio_2_prog) { + LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 2, + .prog_fd = bpf_program__fd(tc_prio_2_prog)); + + ret = bpf_tc_attach(&tc_hook, &tc_opts); + if (!ASSERT_OK(ret, "bpf_tc_attach")) + goto close; + } + ret = bpf_xdp_attach(tap_ifindex, bpf_program__fd(xdp_prog), 0, NULL); if (!ASSERT_GE(ret, 0, "bpf_xdp_attach")) @@ -341,13 +351,29 @@ void test_xdp_context_tuntap(void) return; if (test__start_subtest("data_meta")) - test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls, + test_tuntap(skel->progs.ing_xdp, + skel->progs.ing_cls, + NULL, /* tc prio 2 */ skel->maps.test_result); if (test__start_subtest("dynptr_read")) - test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_read, + test_tuntap(skel->progs.ing_xdp, + skel->progs.ing_cls_dynptr_read, + NULL, /* tc prio 2 */ skel->maps.test_result); if (test__start_subtest("dynptr_slice")) - test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_slice, + test_tuntap(skel->progs.ing_xdp, + skel->progs.ing_cls_dynptr_slice, + NULL, /* tc prio 2 */ + skel->maps.test_result); + if (test__start_subtest("dynptr_write")) + test_tuntap(skel->progs.ing_xdp_zalloc_meta, + skel->progs.ing_cls_dynptr_write, + skel->progs.ing_cls_dynptr_read, + skel->maps.test_result); + if (test__start_subtest("dynptr_slice_rdwr")) + test_tuntap(skel->progs.ing_xdp_zalloc_meta, + skel->progs.ing_cls_dynptr_slice_rdwr, + skel->progs.ing_cls_dynptr_slice, skel->maps.test_result); test_xdp_meta__destroy(skel); diff --git a/tools/testing/selftests/bpf/progs/test_xdp_meta.c b/tools/testing/selftests/bpf/progs/test_xdp_meta.c index 6b11134520be..b6fed72b1005 100644 --- a/tools/testing/selftests/bpf/progs/test_xdp_meta.c +++ b/tools/testing/selftests/bpf/progs/test_xdp_meta.c @@ -60,6 +60,24 @@ int ing_cls_dynptr_read(struct __sk_buff *ctx) return TC_ACT_SHOT; } +/* Write to metadata using bpf_dynptr_write helper */ +SEC("tc") +int ing_cls_dynptr_write(struct __sk_buff *ctx) +{ + struct bpf_dynptr data, meta; + __u8 *src; + + bpf_dynptr_from_skb(ctx, 0, &data); + src = bpf_dynptr_slice(&data, sizeof(struct ethhdr), NULL, META_SIZE); + if (!src) + return TC_ACT_SHOT; + + bpf_dynptr_from_skb(ctx, BPF_DYNPTR_F_SKB_METADATA, &meta); + bpf_dynptr_write(&meta, 0, src, META_SIZE, 0); + + return TC_ACT_UNSPEC; /* pass */ +} + /* Read from metadata using read-only dynptr slice */ SEC("tc") int ing_cls_dynptr_slice(struct __sk_buff *ctx) @@ -82,6 +100,55 @@ int ing_cls_dynptr_slice(struct __sk_buff *ctx) return TC_ACT_SHOT; } +/* Write to metadata using writeable dynptr slice */ +SEC("tc") +int ing_cls_dynptr_slice_rdwr(struct __sk_buff *ctx) +{ + struct bpf_dynptr data, meta; + __u8 *src, *dst; + + bpf_dynptr_from_skb(ctx, 0, &data); + src = bpf_dynptr_slice(&data, sizeof(struct ethhdr), NULL, META_SIZE); + if (!src) + return TC_ACT_SHOT; + + bpf_dynptr_from_skb(ctx, BPF_DYNPTR_F_SKB_METADATA, &meta); + dst = bpf_dynptr_slice_rdwr(&meta, 0, NULL, META_SIZE); + if (!dst) + return TC_ACT_SHOT; + + __builtin_memcpy(dst, src, META_SIZE); + + return TC_ACT_UNSPEC; /* pass */ +} + +/* Reserve and clear space for metadata but don't populate it */ +SEC("xdp") +int ing_xdp_zalloc_meta(struct xdp_md *ctx) +{ + struct ethhdr *eth = ctx_ptr(ctx, data); + __u8 *meta; + int ret; + + /* Drop any non-test packets */ + if (eth + 1 > ctx_ptr(ctx, data_end)) + return XDP_DROP; + if (eth->h_proto != 0) + return XDP_DROP; + + ret = bpf_xdp_adjust_meta(ctx, -META_SIZE); + if (ret < 0) + return XDP_DROP; + + meta = ctx_ptr(ctx, data_meta); + if (meta + META_SIZE > ctx_ptr(ctx, data)) + return XDP_DROP; + + __builtin_memset(meta, 0, META_SIZE); + + return XDP_PASS; +} + SEC("xdp") int ing_xdp(struct xdp_md *ctx) { -- 2.43.0 Currently we don't expect skb metadata to persist beyond the device hooks. Extend the test run BPF program from the Netfilter pre-routing hook to verify this behavior. Note, that the added test has no observable side-effect yet. This will be addressed by the next change. Signed-off-by: Jakub Sitnicki --- .../bpf/prog_tests/xdp_context_test_run.c | 94 ++++++++++++++++------ tools/testing/selftests/bpf/progs/test_xdp_meta.c | 62 +++++++++----- tools/testing/selftests/bpf/test_progs.h | 1 + 3 files changed, 115 insertions(+), 42 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index 79c4c58276e6..4cf8e009a054 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -19,6 +19,9 @@ static const __u8 test_payload[TEST_PAYLOAD_LEN] = { 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, }; +#define PACKET_LEN \ + (sizeof(struct ethhdr) + sizeof(struct iphdr) + TEST_PAYLOAD_LEN) + void test_xdp_context_error(int prog_fd, struct bpf_test_run_opts opts, __u32 data_meta, __u32 data, __u32 data_end, __u32 ingress_ifindex, __u32 rx_queue_index, @@ -120,18 +123,38 @@ void test_xdp_context_test_run(void) test_xdp_context_test_run__destroy(skel); } +static void init_test_packet(__u8 *pkt) +{ + struct ethhdr *eth = &(struct ethhdr){ + .h_dest = { 0x02, 0x00, 0x00, 0x00, 0x00, 0x01 }, + .h_source = { 0x02, 0x00, 0x00, 0x00, 0x00, 0x02 }, + .h_proto = htons(ETH_P_IP), + }; + struct iphdr *iph = &(struct iphdr){ + .ihl = 5, + .version = IPVERSION, + .ttl = IPDEFTTL, + .protocol = 61, /* host internal protocol */ + .saddr = inet_addr("10.0.0.2"), + .daddr = inet_addr("10.0.0.1"), + }; + + eth = memcpy(pkt, eth, sizeof(*eth)); + pkt += sizeof(*eth); + iph = memcpy(pkt, iph, sizeof(*iph)); + pkt += sizeof(*iph); + memcpy(pkt, test_payload, sizeof(test_payload)); + + iph->tot_len = htons(sizeof(*iph) + sizeof(test_payload)); + iph->check = build_ip_csum(iph); +} + static int send_test_packet(int ifindex) { + __u8 packet[PACKET_LEN]; int n, sock = -1; - __u8 packet[sizeof(struct ethhdr) + TEST_PAYLOAD_LEN]; - - /* The ethernet header is not relevant for this test and doesn't need to - * be meaningful. - */ - struct ethhdr eth = { 0 }; - memcpy(packet, ð, sizeof(eth)); - memcpy(packet + sizeof(eth), test_payload, TEST_PAYLOAD_LEN); + init_test_packet(packet); sock = socket(AF_PACKET, SOCK_RAW, IPPROTO_RAW); if (!ASSERT_GE(sock, 0, "socket")) @@ -271,17 +294,18 @@ void test_xdp_context_veth(void) static void test_tuntap(struct bpf_program *xdp_prog, struct bpf_program *tc_prio_1_prog, struct bpf_program *tc_prio_2_prog, + struct bpf_program *nf_prog, struct bpf_map *result_map) { LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); - LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1); + struct bpf_link *nf_link = NULL; struct netns_obj *ns = NULL; - __u8 packet[sizeof(struct ethhdr) + TEST_PAYLOAD_LEN]; + __u8 packet[PACKET_LEN]; int tap_fd = -1; int tap_ifindex; int ret; - if (!clear_test_result(result_map)) + if (result_map && !clear_test_result(result_map)) return; ns = netns_new(TAP_NETNS, true); @@ -292,6 +316,8 @@ static void test_tuntap(struct bpf_program *xdp_prog, if (!ASSERT_GE(tap_fd, 0, "open_tuntap")) goto close; + SYS(close, "ip link set dev " TAP_NAME " addr 02:00:00:00:00:01"); + SYS(close, "ip addr add dev " TAP_NAME " 10.0.0.1/24"); SYS(close, "ip link set dev " TAP_NAME " up"); tap_ifindex = if_nametoindex(TAP_NAME); @@ -303,10 +329,14 @@ static void test_tuntap(struct bpf_program *xdp_prog, if (!ASSERT_OK(ret, "bpf_tc_hook_create")) goto close; - tc_opts.prog_fd = bpf_program__fd(tc_prio_1_prog); - ret = bpf_tc_attach(&tc_hook, &tc_opts); - if (!ASSERT_OK(ret, "bpf_tc_attach")) - goto close; + if (tc_prio_1_prog) { + LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1, + .prog_fd = bpf_program__fd(tc_prio_1_prog)); + + ret = bpf_tc_attach(&tc_hook, &tc_opts); + if (!ASSERT_OK(ret, "bpf_tc_attach")) + goto close; + } if (tc_prio_2_prog) { LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 2, @@ -317,28 +347,33 @@ static void test_tuntap(struct bpf_program *xdp_prog, goto close; } + if (nf_prog) { + LIBBPF_OPTS(bpf_netfilter_opts, nf_opts, + .pf = NFPROTO_IPV4, .hooknum = NF_INET_PRE_ROUTING); + + nf_link = bpf_program__attach_netfilter(nf_prog, &nf_opts); + if (!ASSERT_OK_PTR(nf_link, "attach_netfilter")) + goto close; + } + ret = bpf_xdp_attach(tap_ifindex, bpf_program__fd(xdp_prog), 0, NULL); if (!ASSERT_GE(ret, 0, "bpf_xdp_attach")) goto close; - /* The ethernet header is not relevant for this test and doesn't need to - * be meaningful. - */ - struct ethhdr eth = { 0 }; - - memcpy(packet, ð, sizeof(eth)); - memcpy(packet + sizeof(eth), test_payload, TEST_PAYLOAD_LEN); - + init_test_packet(packet); ret = write(tap_fd, packet, sizeof(packet)); if (!ASSERT_EQ(ret, sizeof(packet), "write packet")) goto close; - assert_test_result(result_map); + if (result_map) + assert_test_result(result_map); close: if (tap_fd >= 0) close(tap_fd); + if (nf_link) + bpf_link__destroy(nf_link); netns_free(ns); } @@ -354,27 +389,38 @@ void test_xdp_context_tuntap(void) test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls, NULL, /* tc prio 2 */ + NULL, /* netfilter */ skel->maps.test_result); if (test__start_subtest("dynptr_read")) test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_read, NULL, /* tc prio 2 */ + NULL, /* netfilter */ skel->maps.test_result); if (test__start_subtest("dynptr_slice")) test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_slice, NULL, /* tc prio 2 */ + NULL, /* netfilter */ skel->maps.test_result); if (test__start_subtest("dynptr_write")) test_tuntap(skel->progs.ing_xdp_zalloc_meta, skel->progs.ing_cls_dynptr_write, skel->progs.ing_cls_dynptr_read, + NULL, /* netfilter */ skel->maps.test_result); if (test__start_subtest("dynptr_slice_rdwr")) test_tuntap(skel->progs.ing_xdp_zalloc_meta, skel->progs.ing_cls_dynptr_slice_rdwr, skel->progs.ing_cls_dynptr_slice, + NULL, /* netfilter */ skel->maps.test_result); + if (test__start_subtest("dynptr_nf_hook")) + test_tuntap(skel->progs.ing_xdp, + NULL, /* tc prio 1 */ + NULL, /* tc prio 2 */ + skel->progs.ing_nf, + NULL /* ignore result for now */); test_xdp_meta__destroy(skel); } diff --git a/tools/testing/selftests/bpf/progs/test_xdp_meta.c b/tools/testing/selftests/bpf/progs/test_xdp_meta.c index b6fed72b1005..41411d164190 100644 --- a/tools/testing/selftests/bpf/progs/test_xdp_meta.c +++ b/tools/testing/selftests/bpf/progs/test_xdp_meta.c @@ -1,15 +1,25 @@ #include #include #include +#include #include +#include #include #include "bpf_kfuncs.h" +#define META_OFFSET (sizeof(struct ethhdr) + sizeof(struct iphdr)) #define META_SIZE 32 +#define NF_DROP 0 +#define NF_ACCEPT 1 + #define ctx_ptr(ctx, mem) (void *)(unsigned long)ctx->mem +struct bpf_nf_ctx { + struct sk_buff *skb; +} __attribute__((preserve_access_index)); + /* Demonstrates how metadata can be passed from an XDP program to a TC program * using bpf_xdp_adjust_meta. * For the sake of testing the metadata support in drivers, the XDP program uses @@ -60,6 +70,20 @@ int ing_cls_dynptr_read(struct __sk_buff *ctx) return TC_ACT_SHOT; } +/* Check that we can't get a dynptr slice to skb metadata yet */ +SEC("netfilter") +int ing_nf(struct bpf_nf_ctx *ctx) +{ + struct __sk_buff *skb = (struct __sk_buff *)ctx->skb; + struct bpf_dynptr meta; + + bpf_dynptr_from_skb(skb, BPF_DYNPTR_F_SKB_METADATA, &meta); + if (bpf_dynptr_size(&meta) != 0) + return NF_DROP; + + return NF_ACCEPT; +} + /* Write to metadata using bpf_dynptr_write helper */ SEC("tc") int ing_cls_dynptr_write(struct __sk_buff *ctx) @@ -68,7 +92,7 @@ int ing_cls_dynptr_write(struct __sk_buff *ctx) __u8 *src; bpf_dynptr_from_skb(ctx, 0, &data); - src = bpf_dynptr_slice(&data, sizeof(struct ethhdr), NULL, META_SIZE); + src = bpf_dynptr_slice(&data, META_OFFSET, NULL, META_SIZE); if (!src) return TC_ACT_SHOT; @@ -108,7 +132,7 @@ int ing_cls_dynptr_slice_rdwr(struct __sk_buff *ctx) __u8 *src, *dst; bpf_dynptr_from_skb(ctx, 0, &data); - src = bpf_dynptr_slice(&data, sizeof(struct ethhdr), NULL, META_SIZE); + src = bpf_dynptr_slice(&data, META_OFFSET, NULL, META_SIZE); if (!src) return TC_ACT_SHOT; @@ -126,14 +150,18 @@ int ing_cls_dynptr_slice_rdwr(struct __sk_buff *ctx) SEC("xdp") int ing_xdp_zalloc_meta(struct xdp_md *ctx) { - struct ethhdr *eth = ctx_ptr(ctx, data); + const void *data_end = ctx_ptr(ctx, data_end); + const struct ethhdr *eth; + const struct iphdr *iph; __u8 *meta; int ret; - /* Drop any non-test packets */ - if (eth + 1 > ctx_ptr(ctx, data_end)) + /* Expect Eth | IPv4 (proto=61) | ... */ + eth = ctx_ptr(ctx, data); + if (eth + 1 > data_end || eth->h_proto != bpf_htons(ETH_P_IP)) return XDP_DROP; - if (eth->h_proto != 0) + iph = (void *)(eth + 1); + if (iph + 1 > data_end || iph->protocol != 61) return XDP_DROP; ret = bpf_xdp_adjust_meta(ctx, -META_SIZE); @@ -153,7 +181,8 @@ SEC("xdp") int ing_xdp(struct xdp_md *ctx) { __u8 *data, *data_meta, *data_end, *payload; - struct ethhdr *eth; + const struct ethhdr *eth; + const struct iphdr *iph; int ret; ret = bpf_xdp_adjust_meta(ctx, -META_SIZE); @@ -164,18 +193,15 @@ int ing_xdp(struct xdp_md *ctx) data_end = ctx_ptr(ctx, data_end); data = ctx_ptr(ctx, data); - eth = (struct ethhdr *)data; - payload = data + sizeof(struct ethhdr); - - if (payload + META_SIZE > data_end || - data_meta + META_SIZE > data) + /* Expect Eth | IPv4 (proto=61) | meta blob */ + eth = (void *)data; + if (eth + 1 > data_end || eth->h_proto != bpf_htons(ETH_P_IP)) return XDP_DROP; - - /* The Linux networking stack may send other packets on the test - * interface that interfere with the test. Just drop them. - * The test packets can be recognized by their ethertype of zero. - */ - if (eth->h_proto != 0) + iph = (void *)(eth + 1); + if (iph + 1 > data_end || iph->protocol != 61) + return XDP_DROP; + payload = (void *)(iph + 1); + if (payload + META_SIZE > data_end || data_meta + META_SIZE > data) return XDP_DROP; __builtin_memcpy(data_meta, payload, META_SIZE); diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h index df2222a1806f..204f54cdaab1 100644 --- a/tools/testing/selftests/bpf/test_progs.h +++ b/tools/testing/selftests/bpf/test_progs.h @@ -20,6 +20,7 @@ typedef __u16 __sum16; #include #include #include +#include #include #include #include -- 2.43.0 The skb metadata tests for BPF programs which don't have metadata access yet have no observable side-effects. Hence, we can't detect breakage. Count each successful BPF program pass, when taking the expected path, as a side-effect to test for. Signed-off-by: Jakub Sitnicki --- .../bpf/prog_tests/xdp_context_test_run.c | 19 ++++++++++++++++- tools/testing/selftests/bpf/progs/test_xdp_meta.c | 24 ++++++++++++++-------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index 4cf8e009a054..b9c9f874f1b4 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -294,13 +294,15 @@ void test_xdp_context_veth(void) static void test_tuntap(struct bpf_program *xdp_prog, struct bpf_program *tc_prio_1_prog, struct bpf_program *tc_prio_2_prog, - struct bpf_program *nf_prog, + struct bpf_program *nf_prog, __u32 *prog_run_cnt, struct bpf_map *result_map) { LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); struct bpf_link *nf_link = NULL; struct netns_obj *ns = NULL; __u8 packet[PACKET_LEN]; + __u32 want_run_cnt = 0; + __u32 have_run_cnt; int tap_fd = -1; int tap_ifindex; int ret; @@ -336,6 +338,7 @@ static void test_tuntap(struct bpf_program *xdp_prog, ret = bpf_tc_attach(&tc_hook, &tc_opts); if (!ASSERT_OK(ret, "bpf_tc_attach")) goto close; + want_run_cnt++; } if (tc_prio_2_prog) { @@ -345,6 +348,7 @@ static void test_tuntap(struct bpf_program *xdp_prog, ret = bpf_tc_attach(&tc_hook, &tc_opts); if (!ASSERT_OK(ret, "bpf_tc_attach")) goto close; + want_run_cnt++; } if (nf_prog) { @@ -354,18 +358,25 @@ static void test_tuntap(struct bpf_program *xdp_prog, nf_link = bpf_program__attach_netfilter(nf_prog, &nf_opts); if (!ASSERT_OK_PTR(nf_link, "attach_netfilter")) goto close; + want_run_cnt++; } ret = bpf_xdp_attach(tap_ifindex, bpf_program__fd(xdp_prog), 0, NULL); if (!ASSERT_GE(ret, 0, "bpf_xdp_attach")) goto close; + want_run_cnt++; init_test_packet(packet); ret = write(tap_fd, packet, sizeof(packet)); if (!ASSERT_EQ(ret, sizeof(packet), "write packet")) goto close; + have_run_cnt = __atomic_exchange_n(prog_run_cnt, 0, __ATOMIC_SEQ_CST); + if (!ASSERT_EQ(have_run_cnt, want_run_cnt, + "unexpected bpf prog runs")) + goto close; + if (result_map) assert_test_result(result_map); @@ -390,36 +401,42 @@ void test_xdp_context_tuntap(void) skel->progs.ing_cls, NULL, /* tc prio 2 */ NULL, /* netfilter */ + &skel->bss->prog_run_cnt, skel->maps.test_result); if (test__start_subtest("dynptr_read")) test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_read, NULL, /* tc prio 2 */ NULL, /* netfilter */ + &skel->bss->prog_run_cnt, skel->maps.test_result); if (test__start_subtest("dynptr_slice")) test_tuntap(skel->progs.ing_xdp, skel->progs.ing_cls_dynptr_slice, NULL, /* tc prio 2 */ NULL, /* netfilter */ + &skel->bss->prog_run_cnt, skel->maps.test_result); if (test__start_subtest("dynptr_write")) test_tuntap(skel->progs.ing_xdp_zalloc_meta, skel->progs.ing_cls_dynptr_write, skel->progs.ing_cls_dynptr_read, NULL, /* netfilter */ + &skel->bss->prog_run_cnt, skel->maps.test_result); if (test__start_subtest("dynptr_slice_rdwr")) test_tuntap(skel->progs.ing_xdp_zalloc_meta, skel->progs.ing_cls_dynptr_slice_rdwr, skel->progs.ing_cls_dynptr_slice, NULL, /* netfilter */ + &skel->bss->prog_run_cnt, skel->maps.test_result); if (test__start_subtest("dynptr_nf_hook")) test_tuntap(skel->progs.ing_xdp, NULL, /* tc prio 1 */ NULL, /* tc prio 2 */ skel->progs.ing_nf, + &skel->bss->prog_run_cnt, NULL /* ignore result for now */); test_xdp_meta__destroy(skel); diff --git a/tools/testing/selftests/bpf/progs/test_xdp_meta.c b/tools/testing/selftests/bpf/progs/test_xdp_meta.c index 41411d164190..ae149e45cf0c 100644 --- a/tools/testing/selftests/bpf/progs/test_xdp_meta.c +++ b/tools/testing/selftests/bpf/progs/test_xdp_meta.c @@ -35,6 +35,14 @@ struct { __uint(value_size, META_SIZE); } test_result SEC(".maps"); +__u32 prog_run_cnt = 0; + +static __always_inline int run_ok(int retval) +{ + __sync_fetch_and_add(&prog_run_cnt, 1); + return retval; +} + SEC("tc") int ing_cls(struct __sk_buff *ctx) { @@ -49,7 +57,7 @@ int ing_cls(struct __sk_buff *ctx) bpf_map_update_elem(&test_result, &key, data_meta, BPF_ANY); - return TC_ACT_SHOT; + return run_ok(TC_ACT_SHOT); } /* Read from metadata using bpf_dynptr_read helper */ @@ -67,7 +75,7 @@ int ing_cls_dynptr_read(struct __sk_buff *ctx) bpf_dynptr_from_skb(ctx, BPF_DYNPTR_F_SKB_METADATA, &meta); bpf_dynptr_read(dst, META_SIZE, &meta, 0, 0); - return TC_ACT_SHOT; + return run_ok(TC_ACT_SHOT); } /* Check that we can't get a dynptr slice to skb metadata yet */ @@ -81,7 +89,7 @@ int ing_nf(struct bpf_nf_ctx *ctx) if (bpf_dynptr_size(&meta) != 0) return NF_DROP; - return NF_ACCEPT; + return run_ok(NF_ACCEPT); } /* Write to metadata using bpf_dynptr_write helper */ @@ -99,7 +107,7 @@ int ing_cls_dynptr_write(struct __sk_buff *ctx) bpf_dynptr_from_skb(ctx, BPF_DYNPTR_F_SKB_METADATA, &meta); bpf_dynptr_write(&meta, 0, src, META_SIZE, 0); - return TC_ACT_UNSPEC; /* pass */ + return run_ok(TC_ACT_UNSPEC); /* pass */ } /* Read from metadata using read-only dynptr slice */ @@ -121,7 +129,7 @@ int ing_cls_dynptr_slice(struct __sk_buff *ctx) __builtin_memcpy(dst, src, META_SIZE); - return TC_ACT_SHOT; + return run_ok(TC_ACT_SHOT); } /* Write to metadata using writeable dynptr slice */ @@ -143,7 +151,7 @@ int ing_cls_dynptr_slice_rdwr(struct __sk_buff *ctx) __builtin_memcpy(dst, src, META_SIZE); - return TC_ACT_UNSPEC; /* pass */ + return run_ok(TC_ACT_UNSPEC); /* pass */ } /* Reserve and clear space for metadata but don't populate it */ @@ -174,7 +182,7 @@ int ing_xdp_zalloc_meta(struct xdp_md *ctx) __builtin_memset(meta, 0, META_SIZE); - return XDP_PASS; + return run_ok(XDP_PASS); } SEC("xdp") @@ -205,7 +213,7 @@ int ing_xdp(struct xdp_md *ctx) return XDP_DROP; __builtin_memcpy(data_meta, payload, META_SIZE); - return XDP_PASS; + return run_ok(XDP_PASS); } char _license[] SEC("license") = "GPL"; -- 2.43.0