[got a bit distracted while writing this so Simon got to the process stuff before me, but I'll leave it in:]
BTW, a few details about process: since this is a new feature, the subject prefix should be [PATCH net-next v4 n/m] (new stuff targets the net-next tree), and the patches should be based on the net-next tree [1] (I'm not sure what you based this on, git am complained on both net and net-next for this patch). More info about this in the docs [2].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/ [2] https://docs.kernel.org/process/maintainer-netdev.html (in case you're not aware: also note the bits about "merge window" which will quite likely become relevant in a few days)
2025-09-23, 15:32:07 +1000, Wilfred Mallawa wrote:
From: Wilfred Mallawa wilfred.mallawa@wdc.com
Test that outgoing plaintext records respect the tls record_size_limit set using setsockopt(). The record size limit is set to be 128, thus, in all received records, the plaintext must not exceed this amount.
Also test that setting a new record size limit whilst a pending open record exists is handled correctly by discarding the request.
Suggested-by: Sabrina Dubroca sd@queasysnail.net Signed-off-by: Wilfred Mallawa wilfred.mallawa@wdc.com
Thanks for adding this patch. (and for the tag :))
tools/testing/selftests/net/tls.c | 149 ++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+)
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c index 0f5640d8dc7f..c5bd431d5af3 100644 --- a/tools/testing/selftests/net/tls.c +++ b/tools/testing/selftests/net/tls.c @@ -24,6 +24,7 @@ #include "../kselftest_harness.h" #define TLS_PAYLOAD_MAX_LEN 16384 +#define TLS_TX_RECORD_SIZE_LIM 5
nit: That should not be needed if you run `make headers_install` before compiling the selftest:
make -s headers_install ; make -C tools/testing/selftests/net tls make: Entering directory '/home/sab/linux/net/tools/testing/selftests/net' gcc -Wall -Wl,--no-as-needed -O2 -g -I../../../../usr/include/ -isystem /home/sab/linux/net/tools/testing/selftests/../../../usr/include -I../ -D_GNU_SOURCE= tls.c -o tls
and that will find the new constant defined in the previous patch using the headers from the current kernel tree, instead of those in the system.
[...]
+TEST(tx_record_size) +{
- struct tls_crypto_info_keys tls12;
- int cfd, ret, fd, rx_len, overhead;
- size_t total_plaintext_rx = 0;
- __u8 tx[1024], rx[2000];
- __u8 *rec;
- __u16 limit = 128;
- __u16 opt = 0;
- __u8 rec_header_len = 5;
gcc complains about unused variables, I guess leftovers from extracting parse_tls_records:
tls.c: In function ‘tx_record_size’: tls.c:2840:14: warning: unused variable ‘rec_header_len’ [-Wunused-variable] 2840 | __u8 rec_header_len = 5; | ^~~~~~~~~~~~~~ tls.c:2837:15: warning: unused variable ‘rec’ [-Wunused-variable] 2837 | __u8 *rec; | ^~~ tls.c: In function ‘tx_record_size_open_rec’: tls.c:2893:14: warning: unused variable ‘rec_header_len’ [-Wunused-variable] 2893 | __u8 rec_header_len = 5; | ^~~~~~~~~~~~~~ tls.c:2891:15: warning: unused variable ‘rec’ [-Wunused-variable] 2891 | __u8 *rec; | ^~~
- unsigned int optlen = sizeof(opt);
- bool notls;
- tls_crypto_info_init(TLS_1_2_VERSION, TLS_CIPHER_AES_CCM_128,
&tls12, 0);- ulp_sock_pair(_metadata, &fd, &cfd, ¬ls);
- if (notls)
exit(KSFT_SKIP);- /* Don't install keys on fd, we'll parse raw records */
- ret = setsockopt(cfd, SOL_TLS, TLS_TX, &tls12, tls12.len);
- ASSERT_EQ(ret, 0);
- ret = setsockopt(cfd, SOL_TLS, TLS_TX_RECORD_SIZE_LIM, &limit, sizeof(limit));
- ASSERT_EQ(ret, 0);
- ret = getsockopt(cfd, SOL_TLS, TLS_TX_RECORD_SIZE_LIM, &opt, &optlen);
- ASSERT_EQ(ret, 0);
- ASSERT_EQ(limit, opt);
- ASSERT_EQ(optlen, sizeof(limit));
nit: Maybe a few of those should be EXPECT_EQ? (ASSERT_* stops the test, EXPECT_* will run the rest of the test)
Getting the wrong value back from this getsockopt is worth noting but there's value in running the traffic through anyway?
- memset(tx, 0, sizeof(tx));
- EXPECT_EQ(send(cfd, tx, sizeof(tx), 0), sizeof(tx));
But this one should maybe be an ASSERT because trying to parse records from whatever data we managed to send (if any) may not make much sense?
(just some thoughts, this is not a "strict requirement" to change anything in the patch)
- close(cfd);
- ret = recv(fd, rx, sizeof(rx), 0);
- memcpy(&rx_len, rx + 3, 2);
- rx_len = htons(rx_len);
nit: set but not used (also in tx_record_size_open_rec)