* [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail()
@ 2024-11-29 1:22 Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 1/4] bpf: Check negative offsets in __bpf_skb_min_len() Cong Wang
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Cong Wang @ 2024-11-29 1:22 UTC (permalink / raw)
To: netdev; +Cc: bpf, Cong Wang
From: Cong Wang <cong.wang@bytedance.com>
This patchset fixes a bug in bpf_skb_change_tail() helper and adds test
cases for it, as requested by Daniel and John.
---
v2: added a test case for TC where offsets are positive
fixed a typo in 1/4 patch description
reduced buffer size in the sockmap test case
Cong Wang (4):
bpf: Check negative offsets in __bpf_skb_min_len()
selftests/bpf: Add a BPF selftest for bpf_skb_change_tail()
selftests/bpf: Introduce socket_helpers.h for TC tests
selftests/bpf: Test bpf_skb_change_tail() in TC ingress
net/core/filter.c | 21 +-
.../selftests/bpf/prog_tests/socket_helpers.h | 394 ++++++++++++++++++
.../selftests/bpf/prog_tests/sockmap_basic.c | 51 +++
.../bpf/prog_tests/sockmap_helpers.h | 385 +----------------
.../selftests/bpf/prog_tests/tc_change_tail.c | 78 ++++
.../bpf/progs/test_sockmap_change_tail.c | 40 ++
.../selftests/bpf/progs/test_tc_change_tail.c | 114 +++++
7 files changed, 693 insertions(+), 390 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/socket_helpers.h
create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_change_tail.c
create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
create mode 100644 tools/testing/selftests/bpf/progs/test_tc_change_tail.c
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Patch bpf v2 1/4] bpf: Check negative offsets in __bpf_skb_min_len()
2024-11-29 1:22 [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Cong Wang
@ 2024-11-29 1:22 ` Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 2/4] selftests/bpf: Add a BPF selftest for bpf_skb_change_tail() Cong Wang
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Cong Wang @ 2024-11-29 1:22 UTC (permalink / raw)
To: netdev; +Cc: bpf, Cong Wang, John Fastabend, Daniel Borkmann
From: Cong Wang <cong.wang@bytedance.com>
skb_network_offset() and skb_transport_offset() can be negative when
they are called after we pull the transport header, for example, when
we use eBPF sockmap at the point of ->sk_data_ready().
__bpf_skb_min_len() uses an unsigned int to get these offsets, this
leads to a very large number which then causes bpf_skb_change_tail()
failed unexpectedly.
Fix this by using a signed int to get these offsets and ensure the
minimum is at least zero.
Fixes: 5293efe62df8 ("bpf: add bpf_skb_change_tail helper")
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
net/core/filter.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 6625b3f563a4..c1982fd04b25 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3734,13 +3734,22 @@ static const struct bpf_func_proto bpf_skb_adjust_room_proto = {
static u32 __bpf_skb_min_len(const struct sk_buff *skb)
{
- u32 min_len = skb_network_offset(skb);
+ int offset = skb_network_offset(skb);
+ u32 min_len = 0;
- if (skb_transport_header_was_set(skb))
- min_len = skb_transport_offset(skb);
- if (skb->ip_summed == CHECKSUM_PARTIAL)
- min_len = skb_checksum_start_offset(skb) +
- skb->csum_offset + sizeof(__sum16);
+ if (offset > 0)
+ min_len = offset;
+ if (skb_transport_header_was_set(skb)) {
+ offset = skb_transport_offset(skb);
+ if (offset > 0)
+ min_len = offset;
+ }
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ offset = skb_checksum_start_offset(skb) +
+ skb->csum_offset + sizeof(__sum16);
+ if (offset > 0)
+ min_len = offset;
+ }
return min_len;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Patch bpf v2 2/4] selftests/bpf: Add a BPF selftest for bpf_skb_change_tail()
2024-11-29 1:22 [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 1/4] bpf: Check negative offsets in __bpf_skb_min_len() Cong Wang
@ 2024-11-29 1:22 ` Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 3/4] selftests/bpf: Introduce socket_helpers.h for TC tests Cong Wang
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Cong Wang @ 2024-11-29 1:22 UTC (permalink / raw)
To: netdev; +Cc: bpf, Cong Wang, John Fastabend, Daniel Borkmann, Zijian Zhang
From: Cong Wang <cong.wang@bytedance.com>
As requested by Daniel, we need to add a selftest to cover
bpf_skb_change_tail() cases in skb_verdict. Here we test trimming,
growing and error cases, and validate its expected return values and the
expected sizes of the payload.
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Zijian Zhang <zijianzhang@bytedance.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 51 +++++++++++++++++++
.../bpf/progs/test_sockmap_change_tail.c | 40 +++++++++++++++
2 files changed, 91 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index fdff0652d7ef..574db6471fb5 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -12,6 +12,7 @@
#include "test_sockmap_progs_query.skel.h"
#include "test_sockmap_pass_prog.skel.h"
#include "test_sockmap_drop_prog.skel.h"
+#include "test_sockmap_change_tail.skel.h"
#include "bpf_iter_sockmap.skel.h"
#include "sockmap_helpers.h"
@@ -643,6 +644,54 @@ static void test_sockmap_skb_verdict_fionread(bool pass_prog)
test_sockmap_drop_prog__destroy(drop);
}
+static void test_sockmap_skb_verdict_change_tail(void)
+{
+ struct test_sockmap_change_tail *skel;
+ int err, map, verdict;
+ int c1, p1, sent, recvd;
+ int zero = 0;
+ char buf[2];
+
+ skel = test_sockmap_change_tail__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+ verdict = bpf_program__fd(skel->progs.prog_skb_verdict);
+ map = bpf_map__fd(skel->maps.sock_map_rx);
+
+ err = bpf_prog_attach(verdict, map, BPF_SK_SKB_STREAM_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+ err = create_pair(AF_INET, SOCK_STREAM, &c1, &p1);
+ if (!ASSERT_OK(err, "create_pair()"))
+ goto out;
+ err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
+ if (!ASSERT_OK(err, "bpf_map_update_elem(c1)"))
+ goto out_close;
+ sent = xsend(p1, "Tr", 2, 0);
+ ASSERT_EQ(sent, 2, "xsend(p1)");
+ recvd = recv(c1, buf, 2, 0);
+ ASSERT_EQ(recvd, 1, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, 0, "change_tail_ret");
+
+ sent = xsend(p1, "G", 1, 0);
+ ASSERT_EQ(sent, 1, "xsend(p1)");
+ recvd = recv(c1, buf, 2, 0);
+ ASSERT_EQ(recvd, 2, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, 0, "change_tail_ret");
+
+ sent = xsend(p1, "E", 1, 0);
+ ASSERT_EQ(sent, 1, "xsend(p1)");
+ recvd = recv(c1, buf, 1, 0);
+ ASSERT_EQ(recvd, 1, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, -EINVAL, "change_tail_ret");
+
+out_close:
+ close(c1);
+ close(p1);
+out:
+ test_sockmap_change_tail__destroy(skel);
+}
+
static void test_sockmap_skb_verdict_peek_helper(int map)
{
int err, c1, p1, zero = 0, sent, recvd, avail;
@@ -1056,6 +1105,8 @@ void test_sockmap_basic(void)
test_sockmap_skb_verdict_fionread(true);
if (test__start_subtest("sockmap skb_verdict fionread on drop"))
test_sockmap_skb_verdict_fionread(false);
+ if (test__start_subtest("sockmap skb_verdict change tail"))
+ test_sockmap_skb_verdict_change_tail();
if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
test_sockmap_skb_verdict_peek();
if (test__start_subtest("sockmap skb_verdict msg_f_peek with link"))
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
new file mode 100644
index 000000000000..2796dd8545eb
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 ByteDance */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_SOCKMAP);
+ __uint(max_entries, 1);
+ __type(key, int);
+ __type(value, int);
+} sock_map_rx SEC(".maps");
+
+long change_tail_ret = 1;
+
+SEC("sk_skb")
+int prog_skb_verdict(struct __sk_buff *skb)
+{
+ char *data, *data_end;
+
+ bpf_skb_pull_data(skb, 1);
+ data = (char *)(unsigned long)skb->data;
+ data_end = (char *)(unsigned long)skb->data_end;
+
+ if (data + 1 > data_end)
+ return SK_PASS;
+
+ if (data[0] == 'T') { /* Trim the packet */
+ change_tail_ret = bpf_skb_change_tail(skb, skb->len - 1, 0);
+ return SK_PASS;
+ } else if (data[0] == 'G') { /* Grow the packet */
+ change_tail_ret = bpf_skb_change_tail(skb, skb->len + 1, 0);
+ return SK_PASS;
+ } else if (data[0] == 'E') { /* Error */
+ change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
+ return SK_PASS;
+ }
+ return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Patch bpf v2 3/4] selftests/bpf: Introduce socket_helpers.h for TC tests
2024-11-29 1:22 [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 1/4] bpf: Check negative offsets in __bpf_skb_min_len() Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 2/4] selftests/bpf: Add a BPF selftest for bpf_skb_change_tail() Cong Wang
@ 2024-11-29 1:22 ` Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 4/4] selftests/bpf: Test bpf_skb_change_tail() in TC ingress Cong Wang
2024-12-06 21:35 ` [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Daniel Borkmann
4 siblings, 0 replies; 7+ messages in thread
From: Cong Wang @ 2024-11-29 1:22 UTC (permalink / raw)
To: netdev; +Cc: bpf, Cong Wang, John Fastabend, Daniel Borkmann, Zijian Zhang
From: Cong Wang <cong.wang@bytedance.com>
Pull socket helpers out of sockmap_helpers.h so that they can be reused
for TC tests as well. This prepares for the next patch.
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Zijian Zhang <zijianzhang@bytedance.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
.../selftests/bpf/prog_tests/socket_helpers.h | 394 ++++++++++++++++++
.../bpf/prog_tests/sockmap_helpers.h | 385 +----------------
2 files changed, 395 insertions(+), 384 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/socket_helpers.h
diff --git a/tools/testing/selftests/bpf/prog_tests/socket_helpers.h b/tools/testing/selftests/bpf/prog_tests/socket_helpers.h
new file mode 100644
index 000000000000..1bdfb79ef009
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/socket_helpers.h
@@ -0,0 +1,394 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __SOCKET_HELPERS__
+#define __SOCKET_HELPERS__
+
+#include <linux/vm_sockets.h>
+
+/* include/linux/net.h */
+#define SOCK_TYPE_MASK 0xf
+
+#define IO_TIMEOUT_SEC 30
+#define MAX_STRERR_LEN 256
+
+/* workaround for older vm_sockets.h */
+#ifndef VMADDR_CID_LOCAL
+#define VMADDR_CID_LOCAL 1
+#endif
+
+/* include/linux/cleanup.h */
+#define __get_and_null(p, nullvalue) \
+ ({ \
+ __auto_type __ptr = &(p); \
+ __auto_type __val = *__ptr; \
+ *__ptr = nullvalue; \
+ __val; \
+ })
+
+#define take_fd(fd) __get_and_null(fd, -EBADF)
+
+/* Wrappers that fail the test on error and report it. */
+
+#define _FAIL(errnum, fmt...) \
+ ({ \
+ error_at_line(0, (errnum), __func__, __LINE__, fmt); \
+ CHECK_FAIL(true); \
+ })
+#define FAIL(fmt...) _FAIL(0, fmt)
+#define FAIL_ERRNO(fmt...) _FAIL(errno, fmt)
+#define FAIL_LIBBPF(err, msg) \
+ ({ \
+ char __buf[MAX_STRERR_LEN]; \
+ libbpf_strerror((err), __buf, sizeof(__buf)); \
+ FAIL("%s: %s", (msg), __buf); \
+ })
+
+
+#define xaccept_nonblock(fd, addr, len) \
+ ({ \
+ int __ret = \
+ accept_timeout((fd), (addr), (len), IO_TIMEOUT_SEC); \
+ if (__ret == -1) \
+ FAIL_ERRNO("accept"); \
+ __ret; \
+ })
+
+#define xbind(fd, addr, len) \
+ ({ \
+ int __ret = bind((fd), (addr), (len)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("bind"); \
+ __ret; \
+ })
+
+#define xclose(fd) \
+ ({ \
+ int __ret = close((fd)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("close"); \
+ __ret; \
+ })
+
+#define xconnect(fd, addr, len) \
+ ({ \
+ int __ret = connect((fd), (addr), (len)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("connect"); \
+ __ret; \
+ })
+
+#define xgetsockname(fd, addr, len) \
+ ({ \
+ int __ret = getsockname((fd), (addr), (len)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("getsockname"); \
+ __ret; \
+ })
+
+#define xgetsockopt(fd, level, name, val, len) \
+ ({ \
+ int __ret = getsockopt((fd), (level), (name), (val), (len)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("getsockopt(" #name ")"); \
+ __ret; \
+ })
+
+#define xlisten(fd, backlog) \
+ ({ \
+ int __ret = listen((fd), (backlog)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("listen"); \
+ __ret; \
+ })
+
+#define xsetsockopt(fd, level, name, val, len) \
+ ({ \
+ int __ret = setsockopt((fd), (level), (name), (val), (len)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("setsockopt(" #name ")"); \
+ __ret; \
+ })
+
+#define xsend(fd, buf, len, flags) \
+ ({ \
+ ssize_t __ret = send((fd), (buf), (len), (flags)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("send"); \
+ __ret; \
+ })
+
+#define xrecv_nonblock(fd, buf, len, flags) \
+ ({ \
+ ssize_t __ret = recv_timeout((fd), (buf), (len), (flags), \
+ IO_TIMEOUT_SEC); \
+ if (__ret == -1) \
+ FAIL_ERRNO("recv"); \
+ __ret; \
+ })
+
+#define xsocket(family, sotype, flags) \
+ ({ \
+ int __ret = socket(family, sotype, flags); \
+ if (__ret == -1) \
+ FAIL_ERRNO("socket"); \
+ __ret; \
+ })
+
+static inline void close_fd(int *fd)
+{
+ if (*fd >= 0)
+ xclose(*fd);
+}
+
+#define __close_fd __attribute__((cleanup(close_fd)))
+
+static inline struct sockaddr *sockaddr(struct sockaddr_storage *ss)
+{
+ return (struct sockaddr *)ss;
+}
+
+static inline void init_addr_loopback4(struct sockaddr_storage *ss,
+ socklen_t *len)
+{
+ struct sockaddr_in *addr4 = memset(ss, 0, sizeof(*ss));
+
+ addr4->sin_family = AF_INET;
+ addr4->sin_port = 0;
+ addr4->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+ *len = sizeof(*addr4);
+}
+
+static inline void init_addr_loopback6(struct sockaddr_storage *ss,
+ socklen_t *len)
+{
+ struct sockaddr_in6 *addr6 = memset(ss, 0, sizeof(*ss));
+
+ addr6->sin6_family = AF_INET6;
+ addr6->sin6_port = 0;
+ addr6->sin6_addr = in6addr_loopback;
+ *len = sizeof(*addr6);
+}
+
+static inline void init_addr_loopback_vsock(struct sockaddr_storage *ss,
+ socklen_t *len)
+{
+ struct sockaddr_vm *addr = memset(ss, 0, sizeof(*ss));
+
+ addr->svm_family = AF_VSOCK;
+ addr->svm_port = VMADDR_PORT_ANY;
+ addr->svm_cid = VMADDR_CID_LOCAL;
+ *len = sizeof(*addr);
+}
+
+static inline void init_addr_loopback(int family, struct sockaddr_storage *ss,
+ socklen_t *len)
+{
+ switch (family) {
+ case AF_INET:
+ init_addr_loopback4(ss, len);
+ return;
+ case AF_INET6:
+ init_addr_loopback6(ss, len);
+ return;
+ case AF_VSOCK:
+ init_addr_loopback_vsock(ss, len);
+ return;
+ default:
+ FAIL("unsupported address family %d", family);
+ }
+}
+
+static inline int enable_reuseport(int s, int progfd)
+{
+ int err, one = 1;
+
+ err = xsetsockopt(s, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one));
+ if (err)
+ return -1;
+ err = xsetsockopt(s, SOL_SOCKET, SO_ATTACH_REUSEPORT_EBPF, &progfd,
+ sizeof(progfd));
+ if (err)
+ return -1;
+
+ return 0;
+}
+
+static inline int socket_loopback_reuseport(int family, int sotype, int progfd)
+{
+ struct sockaddr_storage addr;
+ socklen_t len = 0;
+ int err, s;
+
+ init_addr_loopback(family, &addr, &len);
+
+ s = xsocket(family, sotype, 0);
+ if (s == -1)
+ return -1;
+
+ if (progfd >= 0)
+ enable_reuseport(s, progfd);
+
+ err = xbind(s, sockaddr(&addr), len);
+ if (err)
+ goto close;
+
+ if (sotype & SOCK_DGRAM)
+ return s;
+
+ err = xlisten(s, SOMAXCONN);
+ if (err)
+ goto close;
+
+ return s;
+close:
+ xclose(s);
+ return -1;
+}
+
+static inline int socket_loopback(int family, int sotype)
+{
+ return socket_loopback_reuseport(family, sotype, -1);
+}
+
+static inline int poll_connect(int fd, unsigned int timeout_sec)
+{
+ struct timeval timeout = { .tv_sec = timeout_sec };
+ fd_set wfds;
+ int r, eval;
+ socklen_t esize = sizeof(eval);
+
+ FD_ZERO(&wfds);
+ FD_SET(fd, &wfds);
+
+ r = select(fd + 1, NULL, &wfds, NULL, &timeout);
+ if (r == 0)
+ errno = ETIME;
+ if (r != 1)
+ return -1;
+
+ if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &eval, &esize) < 0)
+ return -1;
+ if (eval != 0) {
+ errno = eval;
+ return -1;
+ }
+
+ return 0;
+}
+
+static inline int poll_read(int fd, unsigned int timeout_sec)
+{
+ struct timeval timeout = { .tv_sec = timeout_sec };
+ fd_set rfds;
+ int r;
+
+ FD_ZERO(&rfds);
+ FD_SET(fd, &rfds);
+
+ r = select(fd + 1, &rfds, NULL, NULL, &timeout);
+ if (r == 0)
+ errno = ETIME;
+
+ return r == 1 ? 0 : -1;
+}
+
+static inline int accept_timeout(int fd, struct sockaddr *addr, socklen_t *len,
+ unsigned int timeout_sec)
+{
+ if (poll_read(fd, timeout_sec))
+ return -1;
+
+ return accept(fd, addr, len);
+}
+
+static inline int recv_timeout(int fd, void *buf, size_t len, int flags,
+ unsigned int timeout_sec)
+{
+ if (poll_read(fd, timeout_sec))
+ return -1;
+
+ return recv(fd, buf, len, flags);
+}
+
+
+static inline int create_pair(int family, int sotype, int *p0, int *p1)
+{
+ __close_fd int s, c = -1, p = -1;
+ struct sockaddr_storage addr;
+ socklen_t len = sizeof(addr);
+ int err;
+
+ s = socket_loopback(family, sotype);
+ if (s < 0)
+ return s;
+
+ err = xgetsockname(s, sockaddr(&addr), &len);
+ if (err)
+ return err;
+
+ c = xsocket(family, sotype, 0);
+ if (c < 0)
+ return c;
+
+ err = connect(c, sockaddr(&addr), len);
+ if (err) {
+ if (errno != EINPROGRESS) {
+ FAIL_ERRNO("connect");
+ return err;
+ }
+
+ err = poll_connect(c, IO_TIMEOUT_SEC);
+ if (err) {
+ FAIL_ERRNO("poll_connect");
+ return err;
+ }
+ }
+
+ switch (sotype & SOCK_TYPE_MASK) {
+ case SOCK_DGRAM:
+ err = xgetsockname(c, sockaddr(&addr), &len);
+ if (err)
+ return err;
+
+ err = xconnect(s, sockaddr(&addr), len);
+ if (err)
+ return err;
+
+ *p0 = take_fd(s);
+ break;
+ case SOCK_STREAM:
+ case SOCK_SEQPACKET:
+ p = xaccept_nonblock(s, NULL, NULL);
+ if (p < 0)
+ return p;
+
+ *p0 = take_fd(p);
+ break;
+ default:
+ FAIL("Unsupported socket type %#x", sotype);
+ return -EOPNOTSUPP;
+ }
+
+ *p1 = take_fd(c);
+ return 0;
+}
+
+static inline int create_socket_pairs(int family, int sotype, int *c0, int *c1,
+ int *p0, int *p1)
+{
+ int err;
+
+ err = create_pair(family, sotype, c0, p0);
+ if (err)
+ return err;
+
+ err = create_pair(family, sotype, c1, p1);
+ if (err) {
+ close(*c0);
+ close(*p0);
+ }
+
+ return err;
+}
+
+#endif // __SOCKET_HELPERS__
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
index 38e35c72bdaa..3e5571dd578d 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
@@ -1,139 +1,12 @@
#ifndef __SOCKMAP_HELPERS__
#define __SOCKMAP_HELPERS__
-#include <linux/vm_sockets.h>
+#include "socket_helpers.h"
-/* include/linux/net.h */
-#define SOCK_TYPE_MASK 0xf
-
-#define IO_TIMEOUT_SEC 30
-#define MAX_STRERR_LEN 256
#define MAX_TEST_NAME 80
-/* workaround for older vm_sockets.h */
-#ifndef VMADDR_CID_LOCAL
-#define VMADDR_CID_LOCAL 1
-#endif
-
#define __always_unused __attribute__((__unused__))
-/* include/linux/cleanup.h */
-#define __get_and_null(p, nullvalue) \
- ({ \
- __auto_type __ptr = &(p); \
- __auto_type __val = *__ptr; \
- *__ptr = nullvalue; \
- __val; \
- })
-
-#define take_fd(fd) __get_and_null(fd, -EBADF)
-
-#define _FAIL(errnum, fmt...) \
- ({ \
- error_at_line(0, (errnum), __func__, __LINE__, fmt); \
- CHECK_FAIL(true); \
- })
-#define FAIL(fmt...) _FAIL(0, fmt)
-#define FAIL_ERRNO(fmt...) _FAIL(errno, fmt)
-#define FAIL_LIBBPF(err, msg) \
- ({ \
- char __buf[MAX_STRERR_LEN]; \
- libbpf_strerror((err), __buf, sizeof(__buf)); \
- FAIL("%s: %s", (msg), __buf); \
- })
-
-/* Wrappers that fail the test on error and report it. */
-
-#define xaccept_nonblock(fd, addr, len) \
- ({ \
- int __ret = \
- accept_timeout((fd), (addr), (len), IO_TIMEOUT_SEC); \
- if (__ret == -1) \
- FAIL_ERRNO("accept"); \
- __ret; \
- })
-
-#define xbind(fd, addr, len) \
- ({ \
- int __ret = bind((fd), (addr), (len)); \
- if (__ret == -1) \
- FAIL_ERRNO("bind"); \
- __ret; \
- })
-
-#define xclose(fd) \
- ({ \
- int __ret = close((fd)); \
- if (__ret == -1) \
- FAIL_ERRNO("close"); \
- __ret; \
- })
-
-#define xconnect(fd, addr, len) \
- ({ \
- int __ret = connect((fd), (addr), (len)); \
- if (__ret == -1) \
- FAIL_ERRNO("connect"); \
- __ret; \
- })
-
-#define xgetsockname(fd, addr, len) \
- ({ \
- int __ret = getsockname((fd), (addr), (len)); \
- if (__ret == -1) \
- FAIL_ERRNO("getsockname"); \
- __ret; \
- })
-
-#define xgetsockopt(fd, level, name, val, len) \
- ({ \
- int __ret = getsockopt((fd), (level), (name), (val), (len)); \
- if (__ret == -1) \
- FAIL_ERRNO("getsockopt(" #name ")"); \
- __ret; \
- })
-
-#define xlisten(fd, backlog) \
- ({ \
- int __ret = listen((fd), (backlog)); \
- if (__ret == -1) \
- FAIL_ERRNO("listen"); \
- __ret; \
- })
-
-#define xsetsockopt(fd, level, name, val, len) \
- ({ \
- int __ret = setsockopt((fd), (level), (name), (val), (len)); \
- if (__ret == -1) \
- FAIL_ERRNO("setsockopt(" #name ")"); \
- __ret; \
- })
-
-#define xsend(fd, buf, len, flags) \
- ({ \
- ssize_t __ret = send((fd), (buf), (len), (flags)); \
- if (__ret == -1) \
- FAIL_ERRNO("send"); \
- __ret; \
- })
-
-#define xrecv_nonblock(fd, buf, len, flags) \
- ({ \
- ssize_t __ret = recv_timeout((fd), (buf), (len), (flags), \
- IO_TIMEOUT_SEC); \
- if (__ret == -1) \
- FAIL_ERRNO("recv"); \
- __ret; \
- })
-
-#define xsocket(family, sotype, flags) \
- ({ \
- int __ret = socket(family, sotype, flags); \
- if (__ret == -1) \
- FAIL_ERRNO("socket"); \
- __ret; \
- })
-
#define xbpf_map_delete_elem(fd, key) \
({ \
int __ret = bpf_map_delete_elem((fd), (key)); \
@@ -193,130 +66,6 @@
__ret; \
})
-static inline void close_fd(int *fd)
-{
- if (*fd >= 0)
- xclose(*fd);
-}
-
-#define __close_fd __attribute__((cleanup(close_fd)))
-
-static inline int poll_connect(int fd, unsigned int timeout_sec)
-{
- struct timeval timeout = { .tv_sec = timeout_sec };
- fd_set wfds;
- int r, eval;
- socklen_t esize = sizeof(eval);
-
- FD_ZERO(&wfds);
- FD_SET(fd, &wfds);
-
- r = select(fd + 1, NULL, &wfds, NULL, &timeout);
- if (r == 0)
- errno = ETIME;
- if (r != 1)
- return -1;
-
- if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &eval, &esize) < 0)
- return -1;
- if (eval != 0) {
- errno = eval;
- return -1;
- }
-
- return 0;
-}
-
-static inline int poll_read(int fd, unsigned int timeout_sec)
-{
- struct timeval timeout = { .tv_sec = timeout_sec };
- fd_set rfds;
- int r;
-
- FD_ZERO(&rfds);
- FD_SET(fd, &rfds);
-
- r = select(fd + 1, &rfds, NULL, NULL, &timeout);
- if (r == 0)
- errno = ETIME;
-
- return r == 1 ? 0 : -1;
-}
-
-static inline int accept_timeout(int fd, struct sockaddr *addr, socklen_t *len,
- unsigned int timeout_sec)
-{
- if (poll_read(fd, timeout_sec))
- return -1;
-
- return accept(fd, addr, len);
-}
-
-static inline int recv_timeout(int fd, void *buf, size_t len, int flags,
- unsigned int timeout_sec)
-{
- if (poll_read(fd, timeout_sec))
- return -1;
-
- return recv(fd, buf, len, flags);
-}
-
-static inline void init_addr_loopback4(struct sockaddr_storage *ss,
- socklen_t *len)
-{
- struct sockaddr_in *addr4 = memset(ss, 0, sizeof(*ss));
-
- addr4->sin_family = AF_INET;
- addr4->sin_port = 0;
- addr4->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
- *len = sizeof(*addr4);
-}
-
-static inline void init_addr_loopback6(struct sockaddr_storage *ss,
- socklen_t *len)
-{
- struct sockaddr_in6 *addr6 = memset(ss, 0, sizeof(*ss));
-
- addr6->sin6_family = AF_INET6;
- addr6->sin6_port = 0;
- addr6->sin6_addr = in6addr_loopback;
- *len = sizeof(*addr6);
-}
-
-static inline void init_addr_loopback_vsock(struct sockaddr_storage *ss,
- socklen_t *len)
-{
- struct sockaddr_vm *addr = memset(ss, 0, sizeof(*ss));
-
- addr->svm_family = AF_VSOCK;
- addr->svm_port = VMADDR_PORT_ANY;
- addr->svm_cid = VMADDR_CID_LOCAL;
- *len = sizeof(*addr);
-}
-
-static inline void init_addr_loopback(int family, struct sockaddr_storage *ss,
- socklen_t *len)
-{
- switch (family) {
- case AF_INET:
- init_addr_loopback4(ss, len);
- return;
- case AF_INET6:
- init_addr_loopback6(ss, len);
- return;
- case AF_VSOCK:
- init_addr_loopback_vsock(ss, len);
- return;
- default:
- FAIL("unsupported address family %d", family);
- }
-}
-
-static inline struct sockaddr *sockaddr(struct sockaddr_storage *ss)
-{
- return (struct sockaddr *)ss;
-}
-
static inline int add_to_sockmap(int sock_mapfd, int fd1, int fd2)
{
u64 value;
@@ -334,136 +83,4 @@ static inline int add_to_sockmap(int sock_mapfd, int fd1, int fd2)
return xbpf_map_update_elem(sock_mapfd, &key, &value, BPF_NOEXIST);
}
-static inline int enable_reuseport(int s, int progfd)
-{
- int err, one = 1;
-
- err = xsetsockopt(s, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one));
- if (err)
- return -1;
- err = xsetsockopt(s, SOL_SOCKET, SO_ATTACH_REUSEPORT_EBPF, &progfd,
- sizeof(progfd));
- if (err)
- return -1;
-
- return 0;
-}
-
-static inline int socket_loopback_reuseport(int family, int sotype, int progfd)
-{
- struct sockaddr_storage addr;
- socklen_t len = 0;
- int err, s;
-
- init_addr_loopback(family, &addr, &len);
-
- s = xsocket(family, sotype, 0);
- if (s == -1)
- return -1;
-
- if (progfd >= 0)
- enable_reuseport(s, progfd);
-
- err = xbind(s, sockaddr(&addr), len);
- if (err)
- goto close;
-
- if (sotype & SOCK_DGRAM)
- return s;
-
- err = xlisten(s, SOMAXCONN);
- if (err)
- goto close;
-
- return s;
-close:
- xclose(s);
- return -1;
-}
-
-static inline int socket_loopback(int family, int sotype)
-{
- return socket_loopback_reuseport(family, sotype, -1);
-}
-
-static inline int create_pair(int family, int sotype, int *p0, int *p1)
-{
- __close_fd int s, c = -1, p = -1;
- struct sockaddr_storage addr;
- socklen_t len = sizeof(addr);
- int err;
-
- s = socket_loopback(family, sotype);
- if (s < 0)
- return s;
-
- err = xgetsockname(s, sockaddr(&addr), &len);
- if (err)
- return err;
-
- c = xsocket(family, sotype, 0);
- if (c < 0)
- return c;
-
- err = connect(c, sockaddr(&addr), len);
- if (err) {
- if (errno != EINPROGRESS) {
- FAIL_ERRNO("connect");
- return err;
- }
-
- err = poll_connect(c, IO_TIMEOUT_SEC);
- if (err) {
- FAIL_ERRNO("poll_connect");
- return err;
- }
- }
-
- switch (sotype & SOCK_TYPE_MASK) {
- case SOCK_DGRAM:
- err = xgetsockname(c, sockaddr(&addr), &len);
- if (err)
- return err;
-
- err = xconnect(s, sockaddr(&addr), len);
- if (err)
- return err;
-
- *p0 = take_fd(s);
- break;
- case SOCK_STREAM:
- case SOCK_SEQPACKET:
- p = xaccept_nonblock(s, NULL, NULL);
- if (p < 0)
- return p;
-
- *p0 = take_fd(p);
- break;
- default:
- FAIL("Unsupported socket type %#x", sotype);
- return -EOPNOTSUPP;
- }
-
- *p1 = take_fd(c);
- return 0;
-}
-
-static inline int create_socket_pairs(int family, int sotype, int *c0, int *c1,
- int *p0, int *p1)
-{
- int err;
-
- err = create_pair(family, sotype, c0, p0);
- if (err)
- return err;
-
- err = create_pair(family, sotype, c1, p1);
- if (err) {
- close(*c0);
- close(*p0);
- }
-
- return err;
-}
-
#endif // __SOCKMAP_HELPERS__
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Patch bpf v2 4/4] selftests/bpf: Test bpf_skb_change_tail() in TC ingress
2024-11-29 1:22 [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Cong Wang
` (2 preceding siblings ...)
2024-11-29 1:22 ` [Patch bpf v2 3/4] selftests/bpf: Introduce socket_helpers.h for TC tests Cong Wang
@ 2024-11-29 1:22 ` Cong Wang
2024-12-06 21:35 ` [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Daniel Borkmann
4 siblings, 0 replies; 7+ messages in thread
From: Cong Wang @ 2024-11-29 1:22 UTC (permalink / raw)
To: netdev; +Cc: bpf, Cong Wang, John Fastabend, Daniel Borkmann, Zijian Zhang
From: Cong Wang <cong.wang@bytedance.com>
Similarly to the previous test, we also need a test case to cover
positive offsets as well, TC is an excellent hook for this.
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Zijian Zhang <zijianzhang@bytedance.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
.../selftests/bpf/prog_tests/tc_change_tail.c | 78 ++++++++++++
.../selftests/bpf/progs/test_tc_change_tail.c | 114 ++++++++++++++++++
2 files changed, 192 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_change_tail.c
create mode 100644 tools/testing/selftests/bpf/progs/test_tc_change_tail.c
diff --git a/tools/testing/selftests/bpf/prog_tests/tc_change_tail.c b/tools/testing/selftests/bpf/prog_tests/tc_change_tail.c
new file mode 100644
index 000000000000..110f54a71a35
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/tc_change_tail.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <error.h>
+#include <test_progs.h>
+#include <linux/pkt_cls.h>
+
+#include "test_tc_change_tail.skel.h"
+#include "socket_helpers.h"
+
+#define LO_IFINDEX 1
+
+void test_tc_change_tail(void)
+{
+ DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX,
+ .attach_point = BPF_TC_INGRESS);
+ DECLARE_LIBBPF_OPTS(bpf_tc_opts, opts);
+ struct test_tc_change_tail *skel = NULL;
+ bool hook_created = false;
+ int ret, fd;
+ int c1, p1;
+ char buf[2];
+
+ skel = test_tc_change_tail__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_tc_change_tail__open_and_load"))
+ return;
+ ret = bpf_tc_hook_create(&hook);
+ if (ret == 0)
+ hook_created = true;
+ ret = ret == -EEXIST ? 0 : ret;
+ if (!ASSERT_OK(ret, "bpf_tc_hook_create"))
+ goto destroy;
+ fd = bpf_program__fd(skel->progs.change_tail);
+ opts.prog_fd = fd;
+ opts.handle = 1;
+ opts.priority = 1;
+ opts.flags = BPF_TC_F_REPLACE;
+ ret = bpf_tc_attach(&hook, &opts);
+ if (!ASSERT_OK(ret, "bpf_tc_attach"))
+ goto hook_destroy;
+
+ ret = create_pair(AF_INET, SOCK_STREAM, &c1, &p1);
+ if (!ASSERT_OK(ret, "create_pair"))
+ goto detach;
+
+ ret = xsend(p1, "Tr", 2, 0);
+ ASSERT_EQ(ret, 2, "xsend(p1)");
+ ret = recv(c1, buf, 2, 0);
+ ASSERT_EQ(ret, 2, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, 0, "change_tail_ret");
+
+ ret = xsend(p1, "G", 1, 0);
+ ASSERT_EQ(ret, 1, "xsend(p1)");
+ ret = recv(c1, buf, 1, 0);
+ ASSERT_EQ(ret, 1, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, 0, "change_tail_ret");
+
+ ret = xsend(p1, "E", 1, 0);
+ ASSERT_EQ(ret, 1, "xsend(p1)");
+ ret = recv(c1, buf, 1, 0);
+ ASSERT_EQ(ret, 1, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, -EINVAL, "change_tail_ret");
+
+ ret = xsend(p1, "Z", 1, 0);
+ ASSERT_EQ(ret, 1, "xsend(p1)");
+ ret = recv(c1, buf, 1, 0);
+ ASSERT_EQ(ret, 1, "recv(c1)");
+ ASSERT_EQ(skel->data->change_tail_ret, -EINVAL, "change_tail_ret");
+
+ close(c1);
+ close(p1);
+detach:
+ bpf_tc_detach(&hook, &opts);
+hook_destroy:
+ if (hook_created)
+ bpf_tc_hook_destroy(&hook);
+destroy:
+ test_tc_change_tail__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
new file mode 100644
index 000000000000..735c7325a2ab
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <linux/pkt_cls.h>
+
+long change_tail_ret = 1;
+
+static __always_inline struct iphdr *parse_ip_header(struct __sk_buff *skb, int *ip_proto)
+{
+ void *data_end = (void *)(long)skb->data_end;
+ void *data = (void *)(long)skb->data;
+ struct ethhdr *eth = data;
+ struct iphdr *iph;
+
+ /* Verify Ethernet header */
+ if ((void *)(data + sizeof(*eth)) > data_end)
+ return NULL;
+
+ /* Skip Ethernet header to get to IP header */
+ iph = (void *)(data + sizeof(struct ethhdr));
+
+ /* Verify IP header */
+ if ((void *)(data + sizeof(struct ethhdr) + sizeof(*iph)) > data_end)
+ return NULL;
+
+ /* Basic IP header validation */
+ if (iph->version != 4) /* Only support IPv4 */
+ return NULL;
+
+ if (iph->ihl < 5) /* Minimum IP header length */
+ return NULL;
+
+ *ip_proto = iph->protocol;
+ return iph;
+}
+
+static __always_inline struct tcphdr *parse_tcp_header(struct __sk_buff *skb, struct iphdr *iph)
+{
+ void *data_end = (void *)(long)skb->data_end;
+ void *hdr = (void *)iph;
+ struct tcphdr *tcp;
+
+ /* Calculate TCP header position */
+ tcp = hdr + (iph->ihl * 4);
+ hdr = (void *)tcp;
+
+ /* Verify TCP header bounds */
+ if ((void *)(hdr + sizeof(*tcp)) > data_end)
+ return NULL;
+
+ /* Basic TCP validation */
+ if (tcp->doff < 5) /* Minimum TCP header length */
+ return NULL;
+
+ /* Success */
+ return tcp;
+}
+
+SEC("tc")
+int change_tail(struct __sk_buff *skb)
+{
+ int len = skb->len;
+ struct tcphdr *tcp;
+ struct iphdr *iph;
+ void *data_end;
+ char *payload;
+ int ip_proto;
+
+ bpf_skb_pull_data(skb, len);
+
+ data_end = (void *)(long)skb->data_end;
+ iph = parse_ip_header(skb, &ip_proto);
+ if (!iph)
+ return TC_ACT_OK;
+
+ if (ip_proto != IPPROTO_TCP) /* Only support TCP packets */
+ return TC_ACT_OK;
+
+ tcp = parse_tcp_header(skb, iph);
+ if (!tcp)
+ return TC_ACT_OK;
+
+ payload = (char *)tcp + (tcp->doff * 4);
+ if (payload + 1 > (char *)data_end)
+ return TC_ACT_OK;
+
+ if (payload[0] == 'T') {
+ change_tail_ret = bpf_skb_change_tail(skb, len - 1, 0);
+ /* Change it back to make TCP happy */
+ if (change_tail_ret == 0)
+ bpf_skb_change_tail(skb, len, 0);
+ return TC_ACT_OK;
+ } else if (payload[0] == 'G') {
+ change_tail_ret = bpf_skb_change_tail(skb, len + 1, 0);
+ /* Change it back to make TCP happy */
+ if (change_tail_ret == 0)
+ bpf_skb_change_tail(skb, len, 0);
+ return TC_ACT_OK;
+ } else if (payload[0] == 'E') {
+ change_tail_ret = bpf_skb_change_tail(skb, 65535, 0); /* Should fail */
+ return TC_ACT_OK;
+ } else if (payload[0] == 'Z') {
+ change_tail_ret = bpf_skb_change_tail(skb, 0, 0); /* Should fail */
+ return TC_ACT_OK;
+ }
+ return TC_ACT_SHOT;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail()
2024-11-29 1:22 [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Cong Wang
` (3 preceding siblings ...)
2024-11-29 1:22 ` [Patch bpf v2 4/4] selftests/bpf: Test bpf_skb_change_tail() in TC ingress Cong Wang
@ 2024-12-06 21:35 ` Daniel Borkmann
2024-12-10 5:36 ` Cong Wang
4 siblings, 1 reply; 7+ messages in thread
From: Daniel Borkmann @ 2024-12-06 21:35 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: bpf, Cong Wang
Hi Cong,
On 11/29/24 2:22 AM, Cong Wang wrote:
> From: Cong Wang <cong.wang@bytedance.com>
>
> This patchset fixes a bug in bpf_skb_change_tail() helper and adds test
> cases for it, as requested by Daniel and John.
>
> ---
> v2: added a test case for TC where offsets are positive
> fixed a typo in 1/4 patch description
> reduced buffer size in the sockmap test case
I ran the selftest several times but it's repeatedly failing whereas
without the series bpf tree CI seems fine. The CI fails on tc tests,
so potentially patch 4 is causing this.
Switching over to tcx APIs from libbpf might automatically address
this given the failures seem to be in 'revision unexpected' which is
likely due to legacy libbpf tc APIs detaching but not deleting the
underlying qdisc.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail()
2024-12-06 21:35 ` [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Daniel Borkmann
@ 2024-12-10 5:36 ` Cong Wang
0 siblings, 0 replies; 7+ messages in thread
From: Cong Wang @ 2024-12-10 5:36 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, bpf, Cong Wang
On Fri, Dec 06, 2024 at 10:35:28PM +0100, Daniel Borkmann wrote:
> Hi Cong,
>
> On 11/29/24 2:22 AM, Cong Wang wrote:
> > From: Cong Wang <cong.wang@bytedance.com>
> >
> > This patchset fixes a bug in bpf_skb_change_tail() helper and adds test
> > cases for it, as requested by Daniel and John.
> >
> > ---
> > v2: added a test case for TC where offsets are positive
> > fixed a typo in 1/4 patch description
> > reduced buffer size in the sockmap test case
>
> I ran the selftest several times but it's repeatedly failing whereas
> without the series bpf tree CI seems fine. The CI fails on tc tests,
> so potentially patch 4 is causing this.
Ah, thanks for catching it.
Previously, the CI job failed due to flaky tests, which are tests that
inconsistently pass or fail. However, this time the failure indicates
a genuine issue.
>
> Switching over to tcx APIs from libbpf might automatically address
> this given the failures seem to be in 'revision unexpected' which is
> likely due to legacy libbpf tc APIs detaching but not deleting the
> underlying qdisc.
Sure, thanks for the hint. I will update this patchset.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-12-10 5:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-29 1:22 [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 1/4] bpf: Check negative offsets in __bpf_skb_min_len() Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 2/4] selftests/bpf: Add a BPF selftest for bpf_skb_change_tail() Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 3/4] selftests/bpf: Introduce socket_helpers.h for TC tests Cong Wang
2024-11-29 1:22 ` [Patch bpf v2 4/4] selftests/bpf: Test bpf_skb_change_tail() in TC ingress Cong Wang
2024-12-06 21:35 ` [Patch bpf v2 0/4] bpf: a bug fix and test cases for bpf_skb_change_tail() Daniel Borkmann
2024-12-10 5:36 ` Cong Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox