[PATCH net] psp: use sk->sk_hash in psp_write_headers()

[PATCH net] psp: use sk->sk_hash in psp_write_headers()

[Eric Dumazet]

udp_flow_src_port() is indirectly using sk->sk_txhash as a base,
because __tcp_transmit_skb() uses skb_set_hash_from_sk().

This is problematic because this field can change over the
lifetime of a TCP flow, thanks to calls to sk_rethink_txhash().

Problem is that some NIC might (ab)use the PSP UDP source port in their
RSS computation, and PSP packets for a given flow could jump
from one queue to another.

In order to avoid surprises, it is safer to let Protective Load
Balancing (PLB) get its entropy from the IPv6 flowlabel,
and change psp_write_headers() to use sk->sk_hash which
does not change for the duration of the flow.

We might add a sysctl to select the behavior, if there
is a need for it.

Fixes: fc724515741a ("psp: provide encapsulation helper for drivers")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Cc: Raed Salem <raeds@nvidia.com>
Cc: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Cc: Cosmin Ratiu <cratiu@nvidia.com>
Cc: Daniel Zahka <daniel.zahka@gmail.com>
Cc: Willem de Bruijn <willemb@google.com>
---
 net/psp/psp_main.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index a8534124f62669cf8b3611d5352e3827982d871d..066222eb56c4af187791445f30a70443aef8a6d8 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -166,9 +166,46 @@ static void psp_write_headers(struct net *net, struct sk_buff *skb, __be32 spi,
 {
 	struct udphdr *uh = udp_hdr(skb);
 	struct psphdr *psph = (struct psphdr *)(uh + 1);
+	const struct sock *sk = skb->sk;
 
 	uh->dest = htons(PSP_DEFAULT_UDP_PORT);
-	uh->source = udp_flow_src_port(net, skb, 0, 0, false);
+
+	/* A bit of theory: Selection of the source port.
+	 *
+	 * We need some entropy, so that multiple flows use different
+	 * source ports for better RSS spreading at the receiver.
+	 *
+	 * We also need that all packets belonging to one TCP flow
+	 * use the same source port through their duration,
+	 * so that all these packets land in the same receive queue.
+	 *
+	 * udp_flow_src_port() is using sk_txhash, inherited from
+	 * skb_set_hash_from_sk() call in __tcp_transmit_skb().
+	 * This field is subject to reshuffling, thanks to
+	 * sk_rethink_txhash() calls in various TCP functions.
+	 *
+	 * Instead, use sk->sk_hash which is constant through
+	 * the whole flow duration.
+	 */
+	if (likely(sk)) {
+		u32 hash = sk->sk_hash;
+		int min, max;
+
+		/* These operations are cheap, no need to cache the result
+		 * in another socket field.
+		 */
+		inet_get_local_port_range(net, &min, &max);
+		/* Since this is being sent on the wire obfuscate hash a bit
+		 * to minimize possibility that any useful information to an
+		 * attacker is leaked. Only upper 16 bits are relevant in the
+		 * computation for 16 bit port value because we use a
+		 * reciprocal divide.
+		 */
+		hash ^= hash << 16;
+		uh->source = htons((((u64)hash * (max - min)) >> 32) + min);
+	} else {
+		uh->source = udp_flow_src_port(net, skb, 0, 0, false);
+	}
 	uh->check = 0;
 	uh->len = htons(udp_len);
 
-- 
2.53.0.310.g728cabbaf7-goog

Re: [PATCH net] psp: use sk->sk_hash in psp_write_headers()

[Daniel Zahka]

On 2/18/26 9:13 AM, Eric Dumazet wrote:
> udp_flow_src_port() is indirectly using sk->sk_txhash as a base,
> because __tcp_transmit_skb() uses skb_set_hash_from_sk().
>
> This is problematic because this field can change over the
> lifetime of a TCP flow, thanks to calls to sk_rethink_txhash().
>
> Problem is that some NIC might (ab)use the PSP UDP source port in their
> RSS computation, and PSP packets for a given flow could jump
> from one queue to another.
>
> In order to avoid surprises, it is safer to let Protective Load
> Balancing (PLB) get its entropy from the IPv6 flowlabel,
> and change psp_write_headers() to use sk->sk_hash which
> does not change for the duration of the flow.
>
> We might add a sysctl to select the behavior, if there
> is a need for it.
>
> Fixes: fc724515741a ("psp: provide encapsulation helper for drivers")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> Cc: Raed Salem <raeds@nvidia.com>
> Cc: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Cc: Cosmin Ratiu <cratiu@nvidia.com>
> Cc: Daniel Zahka <daniel.zahka@gmail.com>
> Cc: Willem de Bruijn <willemb@google.com>
> ---
>   net/psp/psp_main.c | 39 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index a8534124f62669cf8b3611d5352e3827982d871d..066222eb56c4af187791445f30a70443aef8a6d8 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c
> @@ -166,9 +166,46 @@ static void psp_write_headers(struct net *net, struct sk_buff *skb, __be32 spi,
>   {
>   	struct udphdr *uh = udp_hdr(skb);
>   	struct psphdr *psph = (struct psphdr *)(uh + 1);
> +	const struct sock *sk = skb->sk;
>   
>   	uh->dest = htons(PSP_DEFAULT_UDP_PORT);
> -	uh->source = udp_flow_src_port(net, skb, 0, 0, false);
> +
> +	/* A bit of theory: Selection of the source port.
> +	 *
> +	 * We need some entropy, so that multiple flows use different
> +	 * source ports for better RSS spreading at the receiver.
> +	 *
> +	 * We also need that all packets belonging to one TCP flow
> +	 * use the same source port through their duration,
> +	 * so that all these packets land in the same receive queue.
> +	 *
> +	 * udp_flow_src_port() is using sk_txhash, inherited from
> +	 * skb_set_hash_from_sk() call in __tcp_transmit_skb().
> +	 * This field is subject to reshuffling, thanks to
> +	 * sk_rethink_txhash() calls in various TCP functions.
> +	 *
> +	 * Instead, use sk->sk_hash which is constant through
> +	 * the whole flow duration.
> +	 */
> +	if (likely(sk)) {
> +		u32 hash = sk->sk_hash;
> +		int min, max;
> +
> +		/* These operations are cheap, no need to cache the result
> +		 * in another socket field.
> +		 */
> +		inet_get_local_port_range(net, &min, &max);
> +		/* Since this is being sent on the wire obfuscate hash a bit
> +		 * to minimize possibility that any useful information to an
> +		 * attacker is leaked. Only upper 16 bits are relevant in the
> +		 * computation for 16 bit port value because we use a
> +		 * reciprocal divide.
> +		 */
> +		hash ^= hash << 16;
> +		uh->source = htons((((u64)hash * (max - min)) >> 32) + min);
> +	} else {
> +		uh->source = udp_flow_src_port(net, skb, 0, 0, false);
> +	}
>   	uh->check = 0;
>   	uh->len = htons(udp_len);
>   

Thanks. The PSP spec suggests in multiple places the that udp sport be a 
hash derived from "inner header fields", so sk->sk_hash fits that 
description better than sk->sk_txhash regardless of RSS use/abuse 
categorization.

It occurs to me now that fc724515741a ("psp: provide encapsulation 
helper for drivers") also has a bug of unused udp sport parameter. I can 
send a fix for that later, unless you want to remove that as well.

Reviewed-By: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net] psp: use sk->sk_hash in psp_write_headers()

[Eric Dumazet]

On Wed, Feb 18, 2026 at 6:55 PM Daniel Zahka <daniel.zahka@gmail.com> wrote:
>
>

> Thanks. The PSP spec suggests in multiple places the that udp sport be a
> hash derived from "inner header fields", so sk->sk_hash fits that
> description better than sk->sk_txhash regardless of RSS use/abuse
> categorization.
>
> It occurs to me now that fc724515741a ("psp: provide encapsulation
> helper for drivers") also has a bug of unused udp sport parameter. I can
> send a fix for that later, unless you want to remove that as well.

I saw this sport parameter, and thought this was there for a future
patch you had in mind.

No big deal.

>
> Reviewed-By: Daniel Zahka <daniel.zahka@gmail.com>

Thanks !

Re: [PATCH net] psp: use sk->sk_hash in psp_write_headers()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 18 Feb 2026 14:13:37 +0000 you wrote:
> udp_flow_src_port() is indirectly using sk->sk_txhash as a base,
> because __tcp_transmit_skb() uses skb_set_hash_from_sk().
> 
> This is problematic because this field can change over the
> lifetime of a TCP flow, thanks to calls to sk_rethink_txhash().
> 
> Problem is that some NIC might (ab)use the PSP UDP source port in their
> RSS computation, and PSP packets for a given flow could jump
> from one queue to another.
> 
> [...]

Here is the summary with links:
  - [net] psp: use sk->sk_hash in psp_write_headers()
    https://git.kernel.org/netdev/net/c/f891007ab1c7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html