This patch reuses the framework already in place for sk_lookup, allowing
it now to send a reply from the server fd directly, instead of having to
create a socket bound to the original destination address and reply from
there. It does this by passing the source address and port from where to
reply from in a IP_ORIGDSTADDR ancilliary message passed in sendmsg.
Signed-off-by: Tiago Lam <tiagolam@cloudflare.com>
---
tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 70 +++++++++++++++-------
1 file changed, 48 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
index ae87c00867ba..b99aff2e3976 100644
--- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
+++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
@@ -75,6 +75,7 @@ struct test {
struct inet_addr listen_at;
enum server accept_on;
bool reuseport_has_conns; /* Add a connected socket to reuseport group */
+ bool dont_bind_reply; /* Don't bind, send direct from server fd. */
};
struct cb_opts {
@@ -363,7 +364,7 @@ static void v4_to_v6(struct sockaddr_storage *ss)
memset(&v6->sin6_addr.s6_addr[0], 0, 10);
}
-static int udp_recv_send(int server_fd)
+static int udp_recv_send(int server_fd, bool dont_bind_reply)
{
char cmsg_buf[CMSG_SPACE(sizeof(struct sockaddr_storage))];
struct sockaddr_storage _src_addr = { 0 };
@@ -415,26 +416,41 @@ static int udp_recv_send(int server_fd)
v4_to_v6(dst_addr);
}
- /* Reply from original destination address. */
- fd = start_server_addr(SOCK_DGRAM, dst_addr, sizeof(*dst_addr), NULL);
- if (!ASSERT_OK_FD(fd, "start_server_addr")) {
- log_err("failed to create tx socket");
- return -1;
- }
+ if (dont_bind_reply) {
+ /* Reply directly from server fd, specifying the source address and/or
+ * port in struct msghdr.
+ */
+ n = sendmsg(server_fd, &msg, 0);
+ if (CHECK(n <= 0, "sendmsg", "failed\n")) {
+ log_err("failed to send echo reply");
+ return -1;
+ }
+ } else {
+ /* Reply from original destination address. */
+ fd = socket(dst_addr->ss_family, SOCK_DGRAM, 0);
+ if (CHECK(fd < 0, "socket", "failed\n")) {
+ log_err("failed to create tx socket");
+ return -1;
+ }
- msg.msg_control = NULL;
- msg.msg_controllen = 0;
- n = sendmsg(fd, &msg, 0);
- if (CHECK(n <= 0, "sendmsg", "failed\n")) {
- log_err("failed to send echo reply");
- ret = -1;
- goto out;
- }
+ ret = bind(fd, (struct sockaddr *)dst_addr, sizeof(*dst_addr));
+ if (CHECK(ret, "bind", "failed\n")) {
+ log_err("failed to bind tx socket");
+ close(fd);
+ return ret;
+ }
- ret = 0;
-out:
- close(fd);
- return ret;
+ msg.msg_control = NULL;
+ msg.msg_controllen = 0;
+ n = sendmsg(fd, &msg, 0);
+ if (CHECK(n <= 0, "sendmsg", "failed\n")) {
+ log_err("failed to send echo reply");
+ close(fd);
+ return -1;
+ }
+
+ close(fd);
+ }
}
static int tcp_echo_test(int client_fd, int server_fd)
@@ -454,14 +470,14 @@ static int tcp_echo_test(int client_fd, int server_fd)
return 0;
}
-static int udp_echo_test(int client_fd, int server_fd)
+static int udp_echo_test(int client_fd, int server_fd, bool dont_bind_reply)
{
int err;
err = send_byte(client_fd);
if (err)
return -1;
- err = udp_recv_send(server_fd);
+ err = udp_recv_send(server_fd, dont_bind_reply);
if (err)
return -1;
err = recv_byte(client_fd);
@@ -653,7 +669,7 @@ static void run_lookup_prog(const struct test *t)
if (t->sotype == SOCK_STREAM)
tcp_echo_test(client_fd, server_fds[t->accept_on]);
else
- udp_echo_test(client_fd, server_fds[t->accept_on]);
+ udp_echo_test(client_fd, server_fds[t->accept_on], t->dont_bind_reply);
close(client_fd);
close:
@@ -775,6 +791,16 @@ static void test_redirect_lookup(struct test_sk_lookup *skel)
.listen_at = { INT_IP4, INT_PORT },
.accept_on = SERVER_B,
},
+ {
+ .desc = "UDP IPv4 redir different ports",
+ .lookup_prog = skel->progs.select_sock_a_no_reuseport,
+ .sock_map = skel->maps.redir_map,
+ .sotype = SOCK_DGRAM,
+ .connect_to = { EXT_IP4, EXT_PORT },
+ .listen_at = { INT_IP4, INT_PORT },
+ .accept_on = SERVER_A,
+ .dont_bind_reply = true,
+ },
{
.desc = "UDP IPv4 redir and reuseport with conns",
.lookup_prog = skel->progs.select_sock_a,
--
2.34.1Hi Tiago,
On 2024/9/13 17:39, Tiago Lam wrote:
> This patch reuses the framework already in place for sk_lookup, allowing
> it now to send a reply from the server fd directly, instead of having to
> create a socket bound to the original destination address and reply from
> there. It does this by passing the source address and port from where to
> reply from in a IP_ORIGDSTADDR ancilliary message passed in sendmsg.
>
> Signed-off-by: Tiago Lam <tiagolam@cloudflare.com>
> ---
> tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 70 +++++++++++++++-------
> 1 file changed, 48 insertions(+), 22 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> index ae87c00867ba..b99aff2e3976 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> @@ -75,6 +75,7 @@ struct test {
> struct inet_addr listen_at;
> enum server accept_on;
> bool reuseport_has_conns; /* Add a connected socket to reuseport group */
> + bool dont_bind_reply; /* Don't bind, send direct from server fd. */
> };
>
> struct cb_opts {
> @@ -363,7 +364,7 @@ static void v4_to_v6(struct sockaddr_storage *ss)
> memset(&v6->sin6_addr.s6_addr[0], 0, 10);
> }
>
> -static int udp_recv_send(int server_fd)
> +static int udp_recv_send(int server_fd, bool dont_bind_reply)
> {
> char cmsg_buf[CMSG_SPACE(sizeof(struct sockaddr_storage))];
> struct sockaddr_storage _src_addr = { 0 };
> @@ -415,26 +416,41 @@ static int udp_recv_send(int server_fd)
> v4_to_v6(dst_addr);
> }
>
> - /* Reply from original destination address. */
> - fd = start_server_addr(SOCK_DGRAM, dst_addr, sizeof(*dst_addr), NULL);
> - if (!ASSERT_OK_FD(fd, "start_server_addr")) {
> - log_err("failed to create tx socket");
> - return -1;
> - }
> + if (dont_bind_reply) {
> + /* Reply directly from server fd, specifying the source address and/or
> + * port in struct msghdr.
> + */
> + n = sendmsg(server_fd, &msg, 0);
> + if (CHECK(n <= 0, "sendmsg", "failed\n")) {
> + log_err("failed to send echo reply");
> + return -1;
> + }
> + } else {
> + /* Reply from original destination address. */
> + fd = socket(dst_addr->ss_family, SOCK_DGRAM, 0);
> + if (CHECK(fd < 0, "socket", "failed\n")) {
> + log_err("failed to create tx socket");
> + return -1;
> + }
>
Just curious, why not use start_server_addr() here like before?
> - msg.msg_control = NULL;
> - msg.msg_controllen = 0;
> - n = sendmsg(fd, &msg, 0);
> - if (CHECK(n <= 0, "sendmsg", "failed\n")) {
> - log_err("failed to send echo reply");
> - ret = -1;
> - goto out;
> - }
> + ret = bind(fd, (struct sockaddr *)dst_addr, sizeof(*dst_addr));
> + if (CHECK(ret, "bind", "failed\n")) {
> + log_err("failed to bind tx socket");
> + close(fd);
> + return ret;
> + }
>
> - ret = 0;
> -out:
> - close(fd);
> - return ret;
> + msg.msg_control = NULL;
> + msg.msg_controllen = 0;
> + n = sendmsg(fd, &msg, 0);
> + if (CHECK(n <= 0, "sendmsg", "failed\n")) {
> + log_err("failed to send echo reply");
> + close(fd);
> + return -1;
> + }
> +
> + close(fd);
> + }
nit: "return 0" missed.
> }
>
--
Philo
On Fri, Sep 13, 2024 at 08:10:24PM +0800, Philo Lu wrote:
> Hi Tiago,
>
> On 2024/9/13 17:39, Tiago Lam wrote:
> > This patch reuses the framework already in place for sk_lookup, allowing
> > it now to send a reply from the server fd directly, instead of having to
> > create a socket bound to the original destination address and reply from
> > there. It does this by passing the source address and port from where to
> > reply from in a IP_ORIGDSTADDR ancilliary message passed in sendmsg.
> >
> > Signed-off-by: Tiago Lam <tiagolam@cloudflare.com>
> > ---
> > tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 70 +++++++++++++++-------
> > 1 file changed, 48 insertions(+), 22 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> > index ae87c00867ba..b99aff2e3976 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> > @@ -75,6 +75,7 @@ struct test {
> > struct inet_addr listen_at;
> > enum server accept_on;
> > bool reuseport_has_conns; /* Add a connected socket to reuseport group */
> > + bool dont_bind_reply; /* Don't bind, send direct from server fd. */
> > };
> > struct cb_opts {
> > @@ -363,7 +364,7 @@ static void v4_to_v6(struct sockaddr_storage *ss)
> > memset(&v6->sin6_addr.s6_addr[0], 0, 10);
> > }
> > -static int udp_recv_send(int server_fd)
> > +static int udp_recv_send(int server_fd, bool dont_bind_reply)
> > {
> > char cmsg_buf[CMSG_SPACE(sizeof(struct sockaddr_storage))];
> > struct sockaddr_storage _src_addr = { 0 };
> > @@ -415,26 +416,41 @@ static int udp_recv_send(int server_fd)
> > v4_to_v6(dst_addr);
> > }
> > - /* Reply from original destination address. */
> > - fd = start_server_addr(SOCK_DGRAM, dst_addr, sizeof(*dst_addr), NULL);
> > - if (!ASSERT_OK_FD(fd, "start_server_addr")) {
> > - log_err("failed to create tx socket");
> > - return -1;
> > - }
> > + if (dont_bind_reply) {
> > + /* Reply directly from server fd, specifying the source address and/or
> > + * port in struct msghdr.
> > + */
> > + n = sendmsg(server_fd, &msg, 0);
> > + if (CHECK(n <= 0, "sendmsg", "failed\n")) {
> > + log_err("failed to send echo reply");
> > + return -1;
> > + }
> > + } else {
> > + /* Reply from original destination address. */
> > + fd = socket(dst_addr->ss_family, SOCK_DGRAM, 0);
> > + if (CHECK(fd < 0, "socket", "failed\n")) {
> > + log_err("failed to create tx socket");
> > + return -1;
> > + }
> Just curious, why not use start_server_addr() here like before?
>
Thanks, you're right. Initially I rebased this on commit
e3d332aaf898ed755b29c8cdf59be2cfba1cac4b (v6.6.31), which didn't have
start_server_addr(), and used bind() instead. I must have messed up the
rebased.
I've fixed this and included your other suggestion in the patch below
and will fold it into the next revision.
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
index b99aff2e3976..0628a67681c5 100644
--- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
+++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
@@ -374,7 +374,7 @@ static int udp_recv_send(int server_fd, bool dont_bind_reply)
struct iovec iov = { 0 };
struct cmsghdr *cm;
char buf[1];
- int ret, fd;
+ int fd;
ssize_t n;
iov.iov_base = buf;
@@ -427,19 +427,12 @@ static int udp_recv_send(int server_fd, bool dont_bind_reply)
}
} else {
/* Reply from original destination address. */
- fd = socket(dst_addr->ss_family, SOCK_DGRAM, 0);
- if (CHECK(fd < 0, "socket", "failed\n")) {
+ fd = start_server_addr(SOCK_DGRAM, dst_addr, sizeof(*dst_addr), NULL);
+ if (!ASSERT_OK_FD(fd, "start_server_addr")) {
log_err("failed to create tx socket");
return -1;
}
- ret = bind(fd, (struct sockaddr *)dst_addr, sizeof(*dst_addr));
- if (CHECK(ret, "bind", "failed\n")) {
- log_err("failed to bind tx socket");
- close(fd);
- return ret;
- }
-
msg.msg_control = NULL;
msg.msg_controllen = 0;
n = sendmsg(fd, &msg, 0);
@@ -451,6 +444,8 @@ static int udp_recv_send(int server_fd, bool dont_bind_reply)
close(fd);
}
+
+ return 0;
}
static int tcp_echo_test(int client_fd, int server_fd)
© 2016 - 2026 Red Hat, Inc.