Re: [RFC net-next 07/15] net: psp: update the TCP MSS to reflect PSP packet overhead

Willem de Bruijn
2024-05-13T01:47:17Z

Jakub Kicinski wrote:
> PSP eats 32B of header space. Adjust MSS appropriately.
>
> We can either modify tcp_mtu_to_mss() / tcp_mss_to_mtu()
> or reuse icsk_ext_hdr_len. The former option is more TCP
> specific and has runtime overhead. The latter is a bit
> of a hack as PSP is not an ext_hdr. If one squints hard
> enough, UDP encap is just a more practical version of
> IPv6 exthdr, so go with the latter. Happy to change.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

> +static inline unsigned int psp_sk_overhead(const struct sock *sk)
> +{
> + bool has_psp = rcu_access_pointer(sk->psp_assoc);
> +
> + return has_psp ? PSP_HDR_SIZE + PSP_TRL_SIZE : 0;
> +}

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6991464511c3..c67700fc49a1 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -299,10 +299,10 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
> sk->sk_gso_type = SKB_GSO_TCPV6;
> ip6_dst_store(sk, dst, NULL, NULL);
>
> - icsk->icsk_ext_hdr_len = 0;
> + icsk->icsk_ext_hdr_len = psp_sk_overhead(sk);
> if (opt)
> - icsk->icsk_ext_hdr_len = opt->opt_flen +
> - opt->opt_nflen;
> + icsk->icsk_ext_hdr_len += opt->opt_flen +
> + opt->opt_nflen;
>
> tp->rx_opt.mss_clamp = IPV6_MIN_MTU - sizeof(struct tcphdr) - sizeof(struct ipv6hdr);
>
> @@ -1500,10 +1500,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
> opt = ipv6_dup_options(newsk, opt);
> RCU_INIT_POINTER(newnp->opt, opt);
> }
> - inet_csk(newsk)->icsk_ext_hdr_len = 0;
> + inet_csk(newsk)->icsk_ext_hdr_len = psp_sk_overhead(sk);
> if (opt)
> - inet_csk(newsk)->icsk_ext_hdr_len = opt->opt_nflen +
> - opt->opt_flen;
> + inet_csk(newsk)->icsk_ext_hdr_len += opt->opt_nflen +
> + opt->opt_flen;
>
> tcp_ca_openreq_child(newsk, dst);

The below code adjusts ext_hdr_len and recalculates mss when
setting the tx association.

Why already include it at connect and syn_recv, above?

My assumption was that the upgrade to PSP only happens during
TCP_ESTABLISHED. But perhaps I'm wrong.

Is it allowed to set rx and tx association even from as early as the
initial socket(), when still in TCP_CLOSE, client-side?

Server-side, there is no connection fd to pass to netlink commands
before TCP_ESTABLISHED.

> diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
> index 42b881e681b9..bcef042cb8a5 100644
> --- a/net/psp/psp_sock.c
> +++ b/net/psp/psp_sock.c
> @@ -170,6 +170,7 @@ int psp_sock_assoc_set_tx(struct sock *sk, struct psp_dev *psd,
> u32 version, struct psp_key_parsed *key,
> struct netlink_ext_ack *extack)
> {
> + struct inet_connection_sock *icsk;
> struct psp_assoc *pas, *dummy;
> int err;
>
> @@ -220,6 +221,10 @@ int psp_sock_assoc_set_tx(struct sock *sk, struct psp_dev *psd,
>
> WRITE_ONCE(sk->sk_validate_xmit_skb, psp_validate_xmit);
>
> + icsk = inet_csk(sk);
> + icsk->icsk_ext_hdr_len += psp_sk_overhead(sk);
> + icsk->icsk_sync_mss(sk, icsk->icsk_pmtu_cookie);
> +
> exit_free_dummy:
> kfree(dummy);
> exit_clear_rx:
> --
> 2.45.0
>