Netdev List
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Rishikesh Jethwani <rjethwani@purestorage.com>
Cc: netdev@vger.kernel.org, saeedm@nvidia.com, tariqt@nvidia.com,
	mbloch@nvidia.com, borisp@nvidia.com, john.fastabend@gmail.com,
	kuba@kernel.org, davem@davemloft.net, pabeni@redhat.com,
	edumazet@google.com, leon@kernel.org
Subject: Re: [PATCH v13 6/6] selftests: net: add TLS hardware offload test
Date: Wed, 6 May 2026 19:07:23 +0200	[thread overview]
Message-ID: <aft1S8uBtoyiDdmm@krikkit> (raw)
In-Reply-To: <20260429181016.3164935-7-rjethwani@purestorage.com>

2026-04-29, 12:10:16 -0600, Rishikesh Jethwani wrote:
[...]
> +static int do_client(void)
> +{

Both this and do_server could be refactored into a few nice helpers
(prep the socket up to connect+ulp/accept+ulp, echo handling, rekey,
payload verification on RX).

[...]
> +	for (i = 1; i <= num_iterations; i++) {
> +		int this_size;
> +
> +		if (random_size_max > 0)
> +			this_size = (rand() % random_size_max) + 1;
> +		else
> +			this_size = send_size;
> +
> +		/* In burst mode, use a per-iteration fill pattern so the
> +		 * receiver can detect any plaintext corruption without a
> +		 * round-trip echo.
> +		 */
> +		if (burst_mode) {
> +			memset(buf, i & 0xFF, this_size);
> +		} else {
> +			for (j = 0; j < this_size; j++)
> +				buf[j] = rand() & 0xFF;
> +		}
> +
> +		n = send(csk, buf, this_size, 0);
> +		if (n != this_size) {
> +			printf("FAIL: send failed: %s\n", strerror(errno));
> +			goto out;
> +		}
> +		/* Throttle per-iteration progress lines on long burst runs so
> +		 * stdout over ssh doesn't become the bottleneck.

I'm not sure those lines have enough benefit in burst mode to be worth
printing. (same on the server side)


> +static int do_server(void)
> +{
[...]
> +	while (1) {
[...]
> +		/* Burst mode: verify payload matches the client's fill
> +		 * pattern. TLS record boundaries may differ from send()
> +		 * boundaries, so walk the received buffer in chunks that
> +		 * fit within the current iteration's remaining bytes.
> +		 * Catches decrypt-succeeded-but-plaintext-corrupt bugs
> +		 * that AEAD counters alone would miss.
> +		 */
> +		if (burst_mode) {
> +			int off = 0;
> +
> +			while (off < n) {

This would be a good deal simpler if you passed MSG_WAITALL to the
recvmsg call in burst mode. Then you'd get the full chunk of data for
that iteration.

[...]
> +static void print_usage(const char *prog)
> +{
> +	printf("TLS Hardware Offload Two-Node Test\n\n");
> +	printf("Usage:\n");
> +	printf("  %s server [OPTIONS]\n", prog);
> +	printf("  %s client -s <ip> [OPTIONS]\n", prog);
> +	printf("\nOptions:\n");
> +	printf("  -s <ip>       Server IPv4 address (client, required)\n");
> +	printf("  -p <port>     Server port (default: 4433)\n");
> +	printf("  -b <size>     Send buffer size in bytes (default: 16384)\n");
> +	printf("  -r <max>      Use random send buffer sizes (1..<max>)\n");
> +	printf("  -v <version>  TLS version: 1.2 or 1.3 (default: 1.3)\n");
> +	printf("  -c <cipher>   Cipher: 128 or 256 (default: 128)\n");
> +	printf("  -n <N>        Number of send/echo iterations (default: 100)\n");
> +	printf("  -k <N>        Perform N rekeys (client only, TLS 1.3; N < iterations)\n");
> +	printf("  -B            Burst mode: client sends continuously without echo;\n");
> +	printf("                server drains and handles KeyUpdate without responding.\n");
> +	printf("  -Z            Enable zero-copy RX (TLS_RX_EXPECT_NO_PAD);\n");

This is misleading, since zero-copy RX will be enabled by default for 1.2.



> diff --git a/tools/testing/selftests/drivers/net/hw/tls_hw_offload.py b/tools/testing/selftests/drivers/net/hw/tls_hw_offload.py
> new file mode 100755
> index 000000000000..f12da0e66afd
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/tls_hw_offload.py
[...]
> +def verify_tls_counters(stats_before, stats_after, expected_rekeys,
> +                        tls_role, is_dut, burst=False):
> +    """Verify TLS counters on one side of the connection.

Even with the introduction of the check_* helpers, this function still
has a lot of c/p'd code just depending on role and test mode.

> +    tls_role: 'client' or 'server' (TLS role this side played).
> +    is_dut: True for the local DUT; requires HW offload counters.
> +    burst: burst mode - only the TLS client rotates its TX key; the TLS
> +           server only follows with an RX rotation on KeyUpdate receipt.
> +    """
> +    errors = 0
> +    role = 'DUT' if is_dut else 'Peer'
> +
> +    # In burst mode only one direction carries TLS traffic per side
> +    # (TLS client sends, TLS server receives). Check HW offload only on
> +    # the active direction(s); require HW on the DUT's active direction.
> +    if burst:
> +        if tls_role == 'client':
> +            errors += check_path(stats_before, stats_after, 'Tx', role,
> +                                 require_hw=is_dut)
> +        else:
> +            errors += check_path(stats_before, stats_after, 'Rx', role,
> +                                 require_hw=is_dut)
> +    else:
> +        errors += check_path(stats_before, stats_after, 'Tx', role,
> +                             require_hw=is_dut)
> +        errors += check_path(stats_before, stats_after, 'Rx', role,
> +                             require_hw=is_dut)

    # in burst mode, client does TX and server only does RX
    # otherwise, both sides do both TX and RX
    with_tx = not burst or tls_role == 'client':
    with_rx = not burst or tls_role != 'client':

    if with_tx:
        check_path(Tx...)
    if with_rx:
        check_path(Rx...)

> +    if expected_rekeys > 0:
> +        if burst:
> +            if tls_role == 'client':
> +                errors += check_min(stats_before, stats_after,
> +                                    'TlsTxRekeyOk', expected_rekeys, role)
> +                errors += check_zero(stats_before, stats_after,
> +                                     'TlsTxRekeyError', role)

and same for those

> +            else:
> +                errors += check_min(stats_before, stats_after,
> +                                    'TlsRxRekeyOk', expected_rekeys, role)
> +                errors += check_min(stats_before, stats_after,
> +                                    'TlsRxRekeyReceived', expected_rekeys,
> +                                    role)
> +                errors += check_zero(stats_before, stats_after,
> +                                     'TlsRxRekeyError', role)
> +        else:
> +            errors += check_min(stats_before, stats_after,
> +                                'TlsTxRekeyOk', expected_rekeys, role)
> +            errors += check_min(stats_before, stats_after,
> +                                'TlsRxRekeyOk', expected_rekeys, role)
> +            if tls_role == 'server':
> +                errors += check_min(stats_before, stats_after,
> +                                    'TlsRxRekeyReceived', expected_rekeys,
> +                                    role)

Why are you restricting this to the server? The client should get as
many rekeys as it sends.

> +            errors += check_zero(stats_before, stats_after,
> +                                 'TlsTxRekeyError', role)
> +            errors += check_zero(stats_before, stats_after,
> +                                 'TlsRxRekeyError', role)
> +
> +    errors += check_zero(stats_before, stats_after, 'TlsDecryptError', role)
> +
> +    return errors == 0

-- 
Sabrina

      reply	other threads:[~2026-05-06 17:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-29 18:10 [PATCH net-next v13 0/6] tls: Add TLS 1.3 hardware offload support Rishikesh Jethwani
2026-04-29 18:10 ` [PATCH v13 1/6] net: tls: reject TLS 1.3 offload in chcr_ktls and nfp drivers Rishikesh Jethwani
2026-04-29 18:10 ` [PATCH v13 2/6] net/mlx5e: add TLS 1.3 hardware offload support Rishikesh Jethwani
2026-04-29 18:10 ` [PATCH v13 3/6] tls: " Rishikesh Jethwani
2026-04-29 18:10 ` [PATCH v13 4/6] tls: split tls_set_sw_offload into init and finalize stages Rishikesh Jethwani
2026-04-29 18:10 ` [PATCH v13 5/6] tls: add hardware offload key update support Rishikesh Jethwani
2026-05-05  8:40   ` Paolo Abeni
2026-05-05  8:41     ` Paolo Abeni
2026-05-06 10:37     ` Sabrina Dubroca
2026-05-06 11:30   ` Sabrina Dubroca
2026-04-29 18:10 ` [PATCH v13 6/6] selftests: net: add TLS hardware offload test Rishikesh Jethwani
2026-05-06 17:07   ` Sabrina Dubroca [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aft1S8uBtoyiDdmm@krikkit \
    --to=sd@queasysnail.net \
    --cc=borisp@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=mbloch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rjethwani@purestorage.com \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox