| Message ID | 20240703033508.6321-1-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4b74726c01b7a0b5e1029e1e9247fd81590da726 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1,net] tcp: Don't flag tcp_sk(sk)->rx_opt.saw_unknown for TCP AO. | expand |
On Wed, Jul 3, 2024 at 5:35 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > When we process segments with TCP AO, we don't check it in > tcp_parse_options(). Thus, opt_rx->saw_unknown is set to 1, > which unconditionally triggers the BPF TCP option parser. > > Let's avoid the unnecessary BPF invocation. > > Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments") > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> Reviewed-by: Eric Dumazet <edumazet@google.com>
On Wed, 3 Jul 2024 at 04:35, Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > When we process segments with TCP AO, we don't check it in > tcp_parse_options(). Thus, opt_rx->saw_unknown is set to 1, > which unconditionally triggers the BPF TCP option parser. > > Let's avoid the unnecessary BPF invocation. > > Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments") > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> LGTM, thanks! Acked-by: Dmitry Safonov <0x7f454c46@gmail.com>
On Tue, 2024-07-02 at 20:35 -0700, Kuniyuki Iwashima wrote: > When we process segments with TCP AO, we don't check it in > tcp_parse_options(). Thus, opt_rx->saw_unknown is set to 1, > which unconditionally triggers the BPF TCP option parser. > > Let's avoid the unnecessary BPF invocation. > > Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments") > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> > --- > net/ipv4/tcp_input.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index e67cbeeeb95b..77294fd5fd3e 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -4223,6 +4223,13 @@ void tcp_parse_options(const struct net *net, > * checked (see tcp_v{4,6}_rcv()). > */ > break; > +#endif > +#ifdef CONFIG_TCP_AO > + case TCPOPT_AO: > + /* TCP AO has already been checked > + * (see tcp_inbound_ao_hash()). > + */ > + break; > #endif > case TCPOPT_FASTOPEN: > tcp_parse_fastopen_option( [not strictly related to this patch] possibly even MPTCP could benefit from a similar change, but I'm unsure if we want to add even more cases to this statement. Cheers, Paolo
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 2 Jul 2024 20:35:08 -0700 you wrote: > When we process segments with TCP AO, we don't check it in > tcp_parse_options(). Thus, opt_rx->saw_unknown is set to 1, > which unconditionally triggers the BPF TCP option parser. > > Let's avoid the unnecessary BPF invocation. > > Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments") > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> > > [...] Here is the summary with links: - [v1,net] tcp: Don't flag tcp_sk(sk)->rx_opt.saw_unknown for TCP AO. https://git.kernel.org/netdev/net/c/4b74726c01b7 You are awesome, thank you!
From: Paolo Abeni <pabeni@redhat.com> Date: Thu, 04 Jul 2024 12:05:42 +0200 > On Tue, 2024-07-02 at 20:35 -0700, Kuniyuki Iwashima wrote: > > When we process segments with TCP AO, we don't check it in > > tcp_parse_options(). Thus, opt_rx->saw_unknown is set to 1, > > which unconditionally triggers the BPF TCP option parser. > > > > Let's avoid the unnecessary BPF invocation. > > > > Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments") > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> > > --- > > net/ipv4/tcp_input.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > > index e67cbeeeb95b..77294fd5fd3e 100644 > > --- a/net/ipv4/tcp_input.c > > +++ b/net/ipv4/tcp_input.c > > @@ -4223,6 +4223,13 @@ void tcp_parse_options(const struct net *net, > > * checked (see tcp_v{4,6}_rcv()). > > */ > > break; > > +#endif > > +#ifdef CONFIG_TCP_AO > > + case TCPOPT_AO: > > + /* TCP AO has already been checked > > + * (see tcp_inbound_ao_hash()). > > + */ > > + break; > > #endif > > case TCPOPT_FASTOPEN: > > tcp_parse_fastopen_option( > > [not strictly related to this patch] possibly even MPTCP could benefit > from a similar change, but I'm unsure if we want to add even more cases > to this statement. Exactly, it seem no one has tried to inject/parse a new option with MPTCP.
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index e67cbeeeb95b..77294fd5fd3e 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -4223,6 +4223,13 @@ void tcp_parse_options(const struct net *net, * checked (see tcp_v{4,6}_rcv()). */ break; +#endif +#ifdef CONFIG_TCP_AO + case TCPOPT_AO: + /* TCP AO has already been checked + * (see tcp_inbound_ao_hash()). + */ + break; #endif case TCPOPT_FASTOPEN: tcp_parse_fastopen_option(
When we process segments with TCP AO, we don't check it in tcp_parse_options(). Thus, opt_rx->saw_unknown is set to 1, which unconditionally triggers the BPF TCP option parser. Let's avoid the unnecessary BPF invocation. Fixes: 0a3a809089eb ("net/tcp: Verify inbound TCP-AO signed segments") Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> --- net/ipv4/tcp_input.c | 7 +++++++ 1 file changed, 7 insertions(+)