* [PATCH bpf-next 0/2] Enable bpf_setsockopt() on ktls enabled sockets.
@ 2023-01-21 2:57 Kui-Feng Lee
2023-01-21 2:57 ` [PATCH bpf-next 1/2] bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt() Kui-Feng Lee
2023-01-21 2:57 ` [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket Kui-Feng Lee
0 siblings, 2 replies; 6+ messages in thread
From: Kui-Feng Lee @ 2023-01-21 2:57 UTC (permalink / raw)
To: bpf, ast, andrii, song, kernel-team; +Cc: Kui-Feng Lee
This patchset implements a change to bpf_setsockopt() which allows
ktls enabled sockets to be used with the SOL_TCP level. This is
necessary as when ktls is enabled, it changes the function pointer of
setsockopt of the socket, which bpf_setsockopt() checks in order to
make sure that the socket is a TCP socket. Checking sk_protocol
instead of the function pointer will ensure that bpf_setsockopt() with
the SOL_TCP level still works on sockets with ktls enabled.
Kui-Feng Lee (2):
bpf: Check the protocol of a sock to agree the calls to
bpf_setsockopt().
selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket.
net/core/filter.c | 2 +-
.../selftests/bpf/prog_tests/setget_sockopt.c | 71 +++++++++++++++++++
.../selftests/bpf/progs/setget_sockopt.c | 8 +++
3 files changed, 80 insertions(+), 1 deletion(-)
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 1/2] bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt().
2023-01-21 2:57 [PATCH bpf-next 0/2] Enable bpf_setsockopt() on ktls enabled sockets Kui-Feng Lee
@ 2023-01-21 2:57 ` Kui-Feng Lee
2023-01-24 0:55 ` Martin KaFai Lau
2023-01-21 2:57 ` [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket Kui-Feng Lee
1 sibling, 1 reply; 6+ messages in thread
From: Kui-Feng Lee @ 2023-01-21 2:57 UTC (permalink / raw)
To: bpf, ast, andrii, song, kernel-team; +Cc: Kui-Feng Lee
Resolve an issue when calling sol_tcp_sockopt() on a socket with ktls
enabled. Prior to this patch, sol_tcp_sockopt() would only allow calls
if the function pointer of setsockopt of the socket was set to
tcp_setsockopt(). However, any socket with ktls enabled would have its
function pointer set to tls_setsockopt(). To resolve this issue, the
patch adds a check of the protocol of the linux socket and allows
bpf_setsockopt() to be called if ktls is initialized on the linux
socket. This ensures that calls to sol_tcp_sockopt() will succeed on
sockets with ktls enabled.
Signed-off-by: Kui-Feng Lee <kuifeng@meta.com>
---
net/core/filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index b4547a2c02f4..890384cbdeb2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5204,7 +5204,7 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
char *optval, int *optlen,
bool getopt)
{
- if (sk->sk_prot->setsockopt != tcp_setsockopt)
+ if (sk->sk_protocol != IPPROTO_TCP)
return -EINVAL;
switch (optname) {
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket.
2023-01-21 2:57 [PATCH bpf-next 0/2] Enable bpf_setsockopt() on ktls enabled sockets Kui-Feng Lee
2023-01-21 2:57 ` [PATCH bpf-next 1/2] bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt() Kui-Feng Lee
@ 2023-01-21 2:57 ` Kui-Feng Lee
2023-01-24 0:52 ` Martin KaFai Lau
1 sibling, 1 reply; 6+ messages in thread
From: Kui-Feng Lee @ 2023-01-21 2:57 UTC (permalink / raw)
To: bpf, ast, andrii, song, kernel-team; +Cc: Kui-Feng Lee
Ensures that whenever bpf_setsockopt() is called with the SOL_TCP
option on a ktls enabled socket, the call will be accepted by the
system. The provided test makes sure of this by performing an
examination when the server side socket is in the CLOSE_WAIT state. At
this stage, ktls is still enabled on the server socket and can be used
to test if bpf_setsockopt() works correctly with linux.
Signed-off-by: Kui-Feng Lee <kuifeng@meta.com>
---
.../selftests/bpf/prog_tests/setget_sockopt.c | 71 +++++++++++++++++++
.../selftests/bpf/progs/setget_sockopt.c | 8 +++
2 files changed, 79 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c b/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c
index 018611e6b248..20507642c099 100644
--- a/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c
+++ b/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c
@@ -4,6 +4,7 @@
#define _GNU_SOURCE
#include <sched.h>
#include <linux/socket.h>
+#include <linux/tls.h>
#include <net/if.h>
#include "test_progs.h"
@@ -83,6 +84,75 @@ static void test_udp(int family)
ASSERT_EQ(bss->nr_binddev, 1, "nr_bind");
}
+static void test_ktls(void)
+{
+ struct tls12_crypto_info_aes_gcm_128 aes128;
+ struct setget_sockopt__bss *bss = skel->bss;
+ int cfd = -1, sfd = -1, fd = -1, ret;
+
+ memset(bss, 0, sizeof(*bss));
+
+ sfd = start_server(AF_INET, SOCK_STREAM, addr4_str, 0, 0);
+ if (!ASSERT_GE(sfd, 0, "start_server"))
+ return;
+ fd = connect_to_fd(sfd, 0);
+ if (!ASSERT_GE(fd, 0, "connect_to_fd"))
+ goto err_out;
+
+ cfd = accept(sfd, NULL, 0);
+ if (!ASSERT_GE(cfd, 0, "accept"))
+ goto err_out;
+
+ close(sfd);
+ sfd = -1;
+
+ /* Setup KTLS */
+ ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
+ if (ret != 0) {
+ ASSERT_EQ(errno, ENOENT, "setsockopt return ENOENT");
+ printf("Failure setting TCP_ULP, testing without tls\n");
+ goto err_out;
+ }
+ ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
+ if (!ASSERT_EQ(ret, 0, "setsockopt"))
+ goto err_out;
+
+ memset(&aes128, 0, sizeof(aes128));
+ aes128.info.version = TLS_1_2_VERSION;
+ aes128.info.cipher_type = TLS_CIPHER_AES_GCM_128;
+
+ ret = setsockopt(fd, SOL_TLS, TLS_TX, &aes128, sizeof(aes128));
+ if (!ASSERT_EQ(ret, 0, "setsockopt"))
+ goto err_out;
+
+ ret = setsockopt(cfd, SOL_TLS, TLS_RX, &aes128, sizeof(aes128));
+ if (!ASSERT_EQ(ret, 0, "setsockopt"))
+ goto err_out;
+
+ /* KTLS is enabled */
+
+ close(fd);
+ /* At this point, the cfd socket is at the CLOSE_WAIT state
+ * and still run TLS protocol. The test for
+ * BPF_TCP_CLOSE_WAIT should be run at this point.
+ */
+ close(cfd);
+
+ ASSERT_EQ(bss->nr_listen, 1, "nr_listen");
+ ASSERT_EQ(bss->nr_connect, 1, "nr_connect");
+ ASSERT_EQ(bss->nr_active, 1, "nr_active");
+ ASSERT_EQ(bss->nr_passive, 1, "nr_passive");
+ ASSERT_EQ(bss->nr_socket_post_create, 2, "nr_socket_post_create");
+ ASSERT_EQ(bss->nr_binddev, 2, "nr_bind");
+ ASSERT_EQ(bss->nr_fin_wait1, 1, "nr_fin_wait1");
+ return;
+
+err_out:
+ close(fd);
+ close(cfd);
+ close(sfd);
+}
+
void test_setget_sockopt(void)
{
cg_fd = test__join_cgroup(CG_NAME);
@@ -118,6 +188,7 @@ void test_setget_sockopt(void)
test_tcp(AF_INET);
test_udp(AF_INET6);
test_udp(AF_INET);
+ test_ktls();
done:
setget_sockopt__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
index 9523333b8905..027d95755f9f 100644
--- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
+++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
@@ -6,6 +6,8 @@
#include <bpf/bpf_core_read.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
+#define BPF_PROG_TEST_TCP_HDR_OPTIONS
+#include "test_tcp_hdr_options.h"
#ifndef ARRAY_SIZE
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
@@ -22,6 +24,7 @@ int nr_active;
int nr_connect;
int nr_binddev;
int nr_socket_post_create;
+int nr_fin_wait1;
struct sockopt_test {
int opt;
@@ -386,6 +389,11 @@ int skops_sockopt(struct bpf_sock_ops *skops)
nr_passive += !(bpf_test_sockopt(skops, sk) ||
test_tcp_maxseg(skops, sk) ||
test_tcp_saved_syn(skops, sk));
+ set_hdr_cb_flags(skops, BPF_SOCK_OPS_STATE_CB_FLAG);
+ break;
+ case BPF_SOCK_OPS_STATE_CB:
+ if (skops->args[1] == BPF_TCP_CLOSE_WAIT)
+ nr_fin_wait1 += !bpf_test_sockopt(skops, sk);
break;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket.
2023-01-21 2:57 ` [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket Kui-Feng Lee
@ 2023-01-24 0:52 ` Martin KaFai Lau
2023-01-24 17:11 ` Kui-Feng Lee
0 siblings, 1 reply; 6+ messages in thread
From: Martin KaFai Lau @ 2023-01-24 0:52 UTC (permalink / raw)
To: Kui-Feng Lee; +Cc: bpf, ast, andrii, song, kernel-team
On 1/20/23 6:57 PM, Kui-Feng Lee wrote:
> +static void test_ktls(void)
> +{
> + struct tls12_crypto_info_aes_gcm_128 aes128;
> + struct setget_sockopt__bss *bss = skel->bss;
> + int cfd = -1, sfd = -1, fd = -1, ret;
> +
> + memset(bss, 0, sizeof(*bss));
> +
> + sfd = start_server(AF_INET, SOCK_STREAM, addr4_str, 0, 0);
> + if (!ASSERT_GE(sfd, 0, "start_server"))
> + return;
> + fd = connect_to_fd(sfd, 0);
> + if (!ASSERT_GE(fd, 0, "connect_to_fd"))
> + goto err_out;
> +
> + cfd = accept(sfd, NULL, 0);
> + if (!ASSERT_GE(cfd, 0, "accept"))
> + goto err_out;
> +
> + close(sfd);
> + sfd = -1;
> +
> + /* Setup KTLS */
> + ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
> + if (ret != 0) {
nit. ASSERT_OK(ret, ...). It should print the errno also.
> + ASSERT_EQ(errno, ENOENT, "setsockopt return ENOENT");
> + printf("Failure setting TCP_ULP, testing without tls\n");
Then these two ASSERT_EQ and printf are not needed.
> + goto err_out;
> + }
> + ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
> + if (!ASSERT_EQ(ret, 0, "setsockopt"))
nit. ASSERT_OK.
> + goto err_out;
> +
> + memset(&aes128, 0, sizeof(aes128));
> + aes128.info.version = TLS_1_2_VERSION;
> + aes128.info.cipher_type = TLS_CIPHER_AES_GCM_128;
> +
> + ret = setsockopt(fd, SOL_TLS, TLS_TX, &aes128, sizeof(aes128));
> + if (!ASSERT_EQ(ret, 0, "setsockopt"))
> + goto err_out;
> +
> + ret = setsockopt(cfd, SOL_TLS, TLS_RX, &aes128, sizeof(aes128));
> + if (!ASSERT_EQ(ret, 0, "setsockopt"))
> + goto err_out;
> +
> + /* KTLS is enabled */
> +
> + close(fd);
> + /* At this point, the cfd socket is at the CLOSE_WAIT state
> + * and still run TLS protocol. The test for
> + * BPF_TCP_CLOSE_WAIT should be run at this point.
> + */
> + close(cfd);
> +
> + ASSERT_EQ(bss->nr_listen, 1, "nr_listen");
> + ASSERT_EQ(bss->nr_connect, 1, "nr_connect");
> + ASSERT_EQ(bss->nr_active, 1, "nr_active");
> + ASSERT_EQ(bss->nr_passive, 1, "nr_passive");
> + ASSERT_EQ(bss->nr_socket_post_create, 2, "nr_socket_post_create");
> + ASSERT_EQ(bss->nr_binddev, 2, "nr_bind");
> + ASSERT_EQ(bss->nr_fin_wait1, 1, "nr_fin_wait1");
> + return;
> +
> +err_out:
> + close(fd);
> + close(cfd);
> + close(sfd);
> +}
> +
> void test_setget_sockopt(void)
> {
> cg_fd = test__join_cgroup(CG_NAME);
> @@ -118,6 +188,7 @@ void test_setget_sockopt(void)
> test_tcp(AF_INET);
> test_udp(AF_INET6);
> test_udp(AF_INET);
> + test_ktls();
Although not related to the IPPROTO_IPV6 code path, it seems pretty cheap to
test AF_INET6 also like the above tests?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt().
2023-01-21 2:57 ` [PATCH bpf-next 1/2] bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt() Kui-Feng Lee
@ 2023-01-24 0:55 ` Martin KaFai Lau
0 siblings, 0 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2023-01-24 0:55 UTC (permalink / raw)
To: Kui-Feng Lee; +Cc: bpf, ast, andrii, song, kernel-team
On 1/20/23 6:57 PM, Kui-Feng Lee wrote:
> Resolve an issue when calling sol_tcp_sockopt() on a socket with ktls
> enabled. Prior to this patch, sol_tcp_sockopt() would only allow calls
> if the function pointer of setsockopt of the socket was set to
> tcp_setsockopt(). However, any socket with ktls enabled would have its
> function pointer set to tls_setsockopt(). To resolve this issue, the
> patch adds a check of the protocol of the linux socket and allows
> bpf_setsockopt() to be called if ktls is initialized on the linux
> socket. This ensures that calls to sol_tcp_sockopt() will succeed on
> sockets with ktls enabled.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@meta.com>
> ---
> net/core/filter.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b4547a2c02f4..890384cbdeb2 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5204,7 +5204,7 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
> char *optval, int *optlen,
> bool getopt)
> {
> - if (sk->sk_prot->setsockopt != tcp_setsockopt)
> + if (sk->sk_protocol != IPPROTO_TCP)
It is a pretty broad test but I don't see particular issue also. Let see how it
goes.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket.
2023-01-24 0:52 ` Martin KaFai Lau
@ 2023-01-24 17:11 ` Kui-Feng Lee
0 siblings, 0 replies; 6+ messages in thread
From: Kui-Feng Lee @ 2023-01-24 17:11 UTC (permalink / raw)
To: Martin KaFai Lau, Kui-Feng Lee; +Cc: bpf, ast, andrii, song, kernel-team
On 1/23/23 16:52, Martin KaFai Lau wrote:
> On 1/20/23 6:57 PM, Kui-Feng Lee wrote:
>> void test_setget_sockopt(void)
>> {
>> cg_fd = test__join_cgroup(CG_NAME);
>> @@ -118,6 +188,7 @@ void test_setget_sockopt(void)
>> test_tcp(AF_INET);
>> test_udp(AF_INET6);
>> test_udp(AF_INET);
>> + test_ktls();
>
> Although not related to the IPPROTO_IPV6 code path, it seems pretty
> cheap to test AF_INET6 also like the above tests?
>
Sure
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-24 17:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-21 2:57 [PATCH bpf-next 0/2] Enable bpf_setsockopt() on ktls enabled sockets Kui-Feng Lee
2023-01-21 2:57 ` [PATCH bpf-next 1/2] bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt() Kui-Feng Lee
2023-01-24 0:55 ` Martin KaFai Lau
2023-01-21 2:57 ` [PATCH bpf-next 2/2] selftests/bpf: Calls bpf_setsockopt() on a ktls enabled socket Kui-Feng Lee
2023-01-24 0:52 ` Martin KaFai Lau
2023-01-24 17:11 ` Kui-Feng Lee
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.