diff mbox series

[bpf-next,v2,3/3] selftests/bpf: add rto max for bpf_setsockopt test

Message ID 20250217034245.11063-4-kerneljasonxing@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf: support setting max RTO for bpf_setsockopt | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/apply fail Patch does not apply to bpf-next-0
bpf/vmtest-bpf-net-PR success PR summary
bpf/vmtest-bpf-net-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-net-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-net-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-net-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-net-VM_Test-4 success Logs for aarch64-gcc / GCC BPF
bpf/vmtest-bpf-net-VM_Test-5 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-6 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-net-VM_Test-7 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-10 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-11 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-12 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-net-VM_Test-8 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-9 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-net-VM_Test-13 success Logs for s390x-gcc / GCC BPF
bpf/vmtest-bpf-net-VM_Test-14 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-net-VM_Test-15 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-net-VM_Test-18 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-net-VM_Test-19 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-20 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-net-VM_Test-21 success Logs for set-matrix
bpf/vmtest-bpf-net-VM_Test-23 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-24 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-net-VM_Test-34 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-35 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-net-VM_Test-40 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-43 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-41 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-net-VM_Test-44 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-net-VM_Test-50 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-net-VM_Test-51 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-net-VM_Test-16 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-net-VM_Test-17 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-net-VM_Test-22 success Logs for x86_64-gcc / GCC BPF / GCC BPF
bpf/vmtest-bpf-net-VM_Test-25 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-26 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-28 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-27 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-29 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-30 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-net-VM_Test-31 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-net-VM_Test-32 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-net-VM_Test-33 success Logs for x86_64-llvm-17 / GCC BPF / GCC BPF
bpf/vmtest-bpf-net-VM_Test-36 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-37 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-38 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-39 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-net-VM_Test-42 success Logs for x86_64-llvm-18 / GCC BPF / GCC BPF
bpf/vmtest-bpf-net-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-47 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-48 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-net-VM_Test-49 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18

Commit Message

Jason Xing Feb. 17, 2025, 3:42 a.m. UTC 
Add TCP_RTO_MAX_MS selftests for active and passive flows
in various bpf callbacks. Even though the TCP_RTO_MAX_MS
can be used in established phase, we highly discourage
to do so because it may trigger unexpected behaviour.
On the contrary, it's highly recommended that the maximum
value of RTO is set before first time of transmission, such
as BPF_SOCK_OPS_{PASSIVE|ACTIVE}_ESTABLISHED_CB,

Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
---
 tools/include/uapi/linux/tcp.h                      | 1 +
 tools/testing/selftests/bpf/progs/bpf_tracing_net.h | 1 +
 tools/testing/selftests/bpf/progs/setget_sockopt.c  | 1 +
 3 files changed, 3 insertions(+)

Comments

Martin KaFai Lau Feb. 19, 2025, 2:01 a.m. UTC | #1 
On 2/16/25 7:42 PM, Jason Xing wrote:
> Add TCP_RTO_MAX_MS selftests for active and passive flows
> in various bpf callbacks. Even though the TCP_RTO_MAX_MS
> can be used in established phase, we highly discourage
> to do so because it may trigger unexpected behaviour.
> On the contrary, it's highly recommended that the maximum
> value of RTO is set before first time of transmission, such
> as BPF_SOCK_OPS_{PASSIVE|ACTIVE}_ESTABLISHED_CB,

s/,/./

What unexpected behavior when setting in BPF after the established state?

Setting it after the established state or not is not specific to BPF. syscall 
can choose to do it after the connection established also. The above makes it 
unclear what unexpected behavior that the BPF prog will cause if TCP_RTO_MAX_MS 
is used in BPF instead of syscall.

If there is subtle difference between calling TCP_RTO_MAX_MS from bpf and from 
syscall, please write it clearly what are the unexpected behaviors when calling 
in BPF after the established states.

Otherwise, the commit message can be just this:

Test the TCP_RTO_MAX_MS optname in the existing setget_sockopt test.

> 
> Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
> ---
>   tools/include/uapi/linux/tcp.h                      | 1 +
>   tools/testing/selftests/bpf/progs/bpf_tracing_net.h | 1 +
>   tools/testing/selftests/bpf/progs/setget_sockopt.c  | 1 +
>   3 files changed, 3 insertions(+)
> 
> diff --git a/tools/include/uapi/linux/tcp.h b/tools/include/uapi/linux/tcp.h
> index 13ceeb395eb8..7989e3f34a58 100644
> --- a/tools/include/uapi/linux/tcp.h
> +++ b/tools/include/uapi/linux/tcp.h
> @@ -128,6 +128,7 @@ enum {
>   #define TCP_CM_INQ		TCP_INQ
>   
>   #define TCP_TX_DELAY		37	/* delay outgoing packets by XX usec */
> +#define TCP_RTO_MAX_MS		44	/* max rto time in ms */

Have you checked if this change is really needed?

>   
>   
>   #define TCP_REPAIR_ON		1
> diff --git a/tools/testing/selftests/bpf/progs/bpf_tracing_net.h b/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
> index 59843b430f76..eb6ed1b7b2ef 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
> +++ b/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
> @@ -49,6 +49,7 @@
>   #define TCP_SAVED_SYN		28
>   #define TCP_CA_NAME_MAX		16
>   #define TCP_NAGLE_OFF		1
> +#define TCP_RTO_MAX_MS		44
>   
>   #define TCP_ECN_OK              1
>   #define TCP_ECN_QUEUE_CWR       2
> diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
> index 6dd4318debbf..106fe430f41b 100644
> --- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
> +++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
> @@ -61,6 +61,7 @@ static const struct sockopt_test sol_tcp_tests[] = {
>   	{ .opt = TCP_NOTSENT_LOWAT, .new = 1314, .expected = 1314, },
>   	{ .opt = TCP_BPF_SOCK_OPS_CB_FLAGS, .new = BPF_SOCK_OPS_ALL_CB_FLAGS,
>   	  .expected = BPF_SOCK_OPS_ALL_CB_FLAGS, },
> +	{ .opt = TCP_RTO_MAX_MS, .new = 2000, .expected = 2000, },
>   	{ .opt = 0, },
>   };
>
Jason Xing Feb. 19, 2025, 2:17 a.m. UTC | #2 
On Wed, Feb 19, 2025 at 10:01 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 2/16/25 7:42 PM, Jason Xing wrote:
> > Add TCP_RTO_MAX_MS selftests for active and passive flows
> > in various bpf callbacks. Even though the TCP_RTO_MAX_MS
> > can be used in established phase, we highly discourage
> > to do so because it may trigger unexpected behaviour.
> > On the contrary, it's highly recommended that the maximum
> > value of RTO is set before first time of transmission, such
> > as BPF_SOCK_OPS_{PASSIVE|ACTIVE}_ESTABLISHED_CB,
>
> s/,/./
>
> What unexpected behavior when setting in BPF after the established state?
>
> Setting it after the established state or not is not specific to BPF. syscall
> can choose to do it after the connection established also. The above makes it
> unclear what unexpected behavior that the BPF prog will cause if TCP_RTO_MAX_MS
> is used in BPF instead of syscall.
>
> If there is subtle difference between calling TCP_RTO_MAX_MS from bpf and from
> syscall, please write it clearly what are the unexpected behaviors when calling
> in BPF after the established states.

I don't think there is any difference between them. For both of them,
It would be better to set before transmission.

>
> Otherwise, the commit message can be just this:
>
> Test the TCP_RTO_MAX_MS optname in the existing setget_sockopt test.

Got it. Will use this instead.

>
> >
> > Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
> > ---
> >   tools/include/uapi/linux/tcp.h                      | 1 +
> >   tools/testing/selftests/bpf/progs/bpf_tracing_net.h | 1 +
> >   tools/testing/selftests/bpf/progs/setget_sockopt.c  | 1 +
> >   3 files changed, 3 insertions(+)
> >
> > diff --git a/tools/include/uapi/linux/tcp.h b/tools/include/uapi/linux/tcp.h
> > index 13ceeb395eb8..7989e3f34a58 100644
> > --- a/tools/include/uapi/linux/tcp.h
> > +++ b/tools/include/uapi/linux/tcp.h
> > @@ -128,6 +128,7 @@ enum {
> >   #define TCP_CM_INQ          TCP_INQ
> >
> >   #define TCP_TX_DELAY                37      /* delay outgoing packets by XX usec */
> > +#define TCP_RTO_MAX_MS               44      /* max rto time in ms */
>
> Have you checked if this change is really needed?

I thought we needed to sync it from include/uapi/linux/tcp.h. Will test it then.

Thanks,
Jason

>
> >
> >
> >   #define TCP_REPAIR_ON               1
> > diff --git a/tools/testing/selftests/bpf/progs/bpf_tracing_net.h b/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
> > index 59843b430f76..eb6ed1b7b2ef 100644
> > --- a/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
> > +++ b/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
> > @@ -49,6 +49,7 @@
> >   #define TCP_SAVED_SYN               28
> >   #define TCP_CA_NAME_MAX             16
> >   #define TCP_NAGLE_OFF               1
> > +#define TCP_RTO_MAX_MS               44
> >
> >   #define TCP_ECN_OK              1
> >   #define TCP_ECN_QUEUE_CWR       2
> > diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
> > index 6dd4318debbf..106fe430f41b 100644
> > --- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
> > +++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
> > @@ -61,6 +61,7 @@ static const struct sockopt_test sol_tcp_tests[] = {
> >       { .opt = TCP_NOTSENT_LOWAT, .new = 1314, .expected = 1314, },
> >       { .opt = TCP_BPF_SOCK_OPS_CB_FLAGS, .new = BPF_SOCK_OPS_ALL_CB_FLAGS,
> >         .expected = BPF_SOCK_OPS_ALL_CB_FLAGS, },
> > +     { .opt = TCP_RTO_MAX_MS, .new = 2000, .expected = 2000, },
> >       { .opt = 0, },
> >   };
> >
>
diff mbox series

Patch

diff --git a/tools/include/uapi/linux/tcp.h b/tools/include/uapi/linux/tcp.h
index 13ceeb395eb8..7989e3f34a58 100644
--- a/tools/include/uapi/linux/tcp.h
+++ b/tools/include/uapi/linux/tcp.h
@@ -128,6 +128,7 @@  enum {
 #define TCP_CM_INQ		TCP_INQ
 
 #define TCP_TX_DELAY		37	/* delay outgoing packets by XX usec */
+#define TCP_RTO_MAX_MS		44	/* max rto time in ms */
 
 
 #define TCP_REPAIR_ON		1
diff --git a/tools/testing/selftests/bpf/progs/bpf_tracing_net.h b/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
index 59843b430f76..eb6ed1b7b2ef 100644
--- a/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
+++ b/tools/testing/selftests/bpf/progs/bpf_tracing_net.h
@@ -49,6 +49,7 @@ 
 #define TCP_SAVED_SYN		28
 #define TCP_CA_NAME_MAX		16
 #define TCP_NAGLE_OFF		1
+#define TCP_RTO_MAX_MS		44
 
 #define TCP_ECN_OK              1
 #define TCP_ECN_QUEUE_CWR       2
diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
index 6dd4318debbf..106fe430f41b 100644
--- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
+++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
@@ -61,6 +61,7 @@  static const struct sockopt_test sol_tcp_tests[] = {
 	{ .opt = TCP_NOTSENT_LOWAT, .new = 1314, .expected = 1314, },
 	{ .opt = TCP_BPF_SOCK_OPS_CB_FLAGS, .new = BPF_SOCK_OPS_ALL_CB_FLAGS,
 	  .expected = BPF_SOCK_OPS_ALL_CB_FLAGS, },
+	{ .opt = TCP_RTO_MAX_MS, .new = 2000, .expected = 2000, },
 	{ .opt = 0, },
 };