Re: [PATCH v2 07/17] net: tcp: allow tcp_timewait_sock to validate skbs before handing to device

Willem de Bruijn
2025-06-26T00:02:09Z

Daniel Zahka wrote:
> Provide a callback to validate skb's originating from tcp timewait
> socks before passing to the device layer. Full socks have a
> sk_validate_xmit_skb member for checking that a device is capable of
> performing offloads required for transmitting an skb. With psp, tcp
> timewait socks will inherit the crypto state from their corresponding
> full socks. Any ACKs or RSTs that originate from a tcp timewait sock
> carrying psp state should be psp encapsulated.
>
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
> ---
>
> Notes:
> v2:
> - patch introduced in v2
>
> include/net/inet_timewait_sock.h | 5 +++++
> net/core/dev.c | 14 ++++++++++++--
> net/ipv4/inet_timewait_sock.c | 3 +++
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
> index c1295246216c..3a31c74c9e15 100644
> --- a/include/net/inet_timewait_sock.h
> +++ b/include/net/inet_timewait_sock.h
> @@ -84,6 +84,11 @@ struct inet_timewait_sock {
> #if IS_ENABLED(CONFIG_INET_PSP)
> struct psp_assoc __rcu *psp_assoc;
> #endif
> +#ifdef CONFIG_SOCK_VALIDATE_XMIT
> + struct sk_buff* (*tw_validate_xmit_skb)(struct sock *sk,
> + struct net_device *dev,
> + struct sk_buff *skb);
> +#endif
> };
> #define tw_tclass tw_tos
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b825b3f5b7db..bf013436a57b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3904,10 +3904,20 @@ static struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
> struct net_device *dev)
> {
> #ifdef CONFIG_SOCK_VALIDATE_XMIT
> + struct sk_buff *(*sk_validate)(struct sock *sk, struct net_device *dev,
> + struct sk_buff *skb);
> struct sock *sk = skb->sk;
>
> - if (sk && sk_fullsock(sk) && sk->sk_validate_xmit_skb) {
> - skb = sk->sk_validate_xmit_skb(sk, dev, skb);
> + sk_validate = NULL;
> + if (sk) {
> + if (sk_fullsock(sk))
> + sk_validate = sk->sk_validate_xmit_skb;
> + else if (sk->sk_state == TCP_TIME_WAIT)

Can this shadow a different constant for other protocols?

To be on the safe side: sk_is_tcp

> + sk_validate = inet_twsk(sk)->tw_validate_xmit_skb;
> + }
> +
> + if (sk_validate) {
> + skb = sk_validate(sk, dev, skb);

If this callback is in the hot path, a candidate for an INDIRECT_CALL wrapper.
There are only two users, TLS and PSP.

> } else if (unlikely(skb_is_decrypted(skb))) {
> pr_warn_ratelimited("unencrypted skb with no associated socket - dropping\n");
> kfree_skb(skb);
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index dfde7895d8f2..859c03e07466 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -210,6 +210,9 @@ struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk,
> atomic64_set(&tw->tw_cookie, atomic64_read(&sk->sk_cookie));
> twsk_net_set(tw, sock_net(sk));
> timer_setup(&tw->tw_timer, tw_timer_handler, 0);
> +#ifdef CONFIG_SOCK_VALIDATE_XMIT
> + tw->tw_validate_xmit_skb = NULL;
> +#endif
> /*
> * Because we use RCU lookups, we should not set tw_refcnt
> * to a non null value before everything is setup for this
> --
> 2.47.1
>