[PATCH net v2] netfilter: nf_nat: recalculate TCP TS offset when snat change sport

[PATCH net v2] netfilter: nf_nat: recalculate TCP TS offset when snat change sport

[xietangxin]

Problem observed in Kubernetes environments where MASQUERADE target with
--random-fully is configured by default. after commit
165573e41f2f ("tcp: secure_seq: add back ports to TS offset") TCP short
connection QPS dropped from ~20000 to ~10000. This added source and
destination ports into TS offset calculation.

However, with MASQUERADE --random-fully, when multiple internal connections
(e.g sport 10000,20000) are mapped to the same external port (e.g 30000),
their TS offsets are calculated as ts_offset(10000) and ts_offset(20000).
If the server reuses the TIME_WAIT slot from the first connection, there is
a chance that ts_offset(20000) < ts_offset(10000), breaking TSval
monotonicity for the same 4-tuple and causing RST packets:
  Client -> Server 24870 -> 80 [SYN] TSval=2294041168
  Server -> Client 80 -> 24870 [ACK] TSecr=2846236456
  Client -> Server 24870 -> 80 [RST] Seq=855605690

After nf_nat_inet_fn() successfully assigns a new randomized
source port, recalculate the TS offset using the new port and
update the SYN packet's TSval accordingly.

Test results on 4U4G VM with
`./wrk -t8 -c200 -H "Connection: close" -d10s --latency http://5.5.5.5:80`
Before:
  random:10712 req/s, random-fully:10986 req/s
After:
  random:20530 req/s, random-fully:19511 req/s

Fixes: 165573e41f2f ("tcp: secure_seq: add back ports to TS offset")
Cc: stable@vger.kernel.org
Closes:https://lore.kernel.org/all/92935c00-e0be-4591-ac44-5978c7804d57@yeah.net/
Signed-off-by: xietangxin <xietangxin@h-partners.com>
---
v2:
  - Move the new helper in the IP_CT_NEW case of nf_nat_inet_fn().
  - Fix a compilation failure when CONFIG_IPV6 is disabled.

v1: https://lore.kernel.org/all/20260629093408.3927103-1-xietangxin@h-partners.com/
---
 net/netfilter/nf_nat_core.c | 103 +++++++++++++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 63ff6b4d5d21..9d0b316fa3c7 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -16,6 +16,8 @@
 #include <linux/siphash.h>
 #include <linux/rtnetlink.h>
 
+#include <net/tcp.h>
+#include <net/secure_seq.h>
 #include <net/netfilter/nf_conntrack_bpf.h>
 #include <net/netfilter/nf_conntrack_core.h>
 #include <net/netfilter/nf_conntrack_helper.h>
@@ -894,6 +896,99 @@ static bool in_vrf_postrouting(const struct nf_hook_state *state)
 	return false;
 }
 
+static __be32 *nf_nat_tcp_ts_option_ptr(const struct sk_buff *skb)
+{
+	struct tcphdr *th;
+	unsigned char *ptr;
+	unsigned char opcode;
+	unsigned char opsize;
+	unsigned int optlen, offset;
+
+	offset = 0;
+	th = tcp_hdr(skb);
+	optlen = (th->doff - 5) * 4;
+	ptr = (unsigned char *)(th + 1);
+
+	while (offset < optlen) {
+		opcode = ptr[offset];
+		if (opcode == TCPOPT_EOL)
+			break;
+
+		if (opcode == TCPOPT_NOP) {
+			offset++;
+			continue;
+		}
+
+		if (offset + 1 >= optlen)
+			break;
+
+		opsize = ptr[offset + 1];
+		if (opsize < 2 || offset + opsize > optlen)
+			break;
+
+		if (opcode == TCPOPT_TIMESTAMP && opsize == TCPOLEN_TIMESTAMP)
+			return (__be32 *)(ptr + offset + 2);
+
+		offset += opsize;
+	}
+
+	return NULL;
+}
+
+static void nf_nat_update_tcp_ts_offset(struct nf_conn *ct, struct sk_buff *skb)
+{
+	__be32 *tsptr;
+	struct net *net;
+	struct tcphdr *th;
+	struct tcp_sock *tp;
+	union tcp_seq_and_ts_off st;
+	struct nf_conntrack_tuple *orig_tuple;
+	struct nf_conntrack_tuple *reply_tuple;
+
+	orig_tuple = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
+	reply_tuple = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
+	if (orig_tuple->src.u.tcp.port == reply_tuple->dst.u.tcp.port)
+		return;
+
+	th = tcp_hdr(skb);
+	if (!th || !th->syn || th->ack)
+		return;
+
+	net = nf_ct_net(ct);
+	if (READ_ONCE(net->ipv4.sysctl_tcp_timestamps) != 1)
+		return;
+
+	if (!skb->sk)
+		return;
+
+	tsptr = nf_nat_tcp_ts_option_ptr(skb);
+	if (!tsptr)
+		return;
+
+	switch (nf_ct_l3num(ct)) {
+	case NFPROTO_IPV4:
+		st = secure_tcp_seq_and_ts_off(net, reply_tuple->dst.u3.ip,
+					       reply_tuple->src.u3.ip,
+					       reply_tuple->dst.u.tcp.port,
+					       reply_tuple->src.u.tcp.port);
+		break;
+#if IS_ENABLED(CONFIG_IPV6)
+	case NFPROTO_IPV6:
+		st = secure_tcpv6_seq_and_ts_off(net, reply_tuple->dst.u3.ip6,
+						 reply_tuple->src.u3.ip6,
+						 reply_tuple->dst.u.tcp.port,
+						 reply_tuple->src.u.tcp.port);
+		break;
+#endif
+	default:
+		return;
+	}
+
+	tp = tcp_sk(skb->sk);
+	*tsptr = htonl(tcp_skb_timestamp_ts(tp->tcp_usec_ts, skb) + st.ts_off);
+	WRITE_ONCE(tp->tsoffset, st.ts_off);
+}
+
 unsigned int
 nf_nat_inet_fn(void *priv, struct sk_buff *skb,
 	       const struct nf_hook_state *state)
@@ -937,8 +1032,14 @@ nf_nat_inet_fn(void *priv, struct sk_buff *skb,
 						       state);
 				if (ret != NF_ACCEPT)
 					return ret;
-				if (nf_nat_initialized(ct, maniptype))
+				if (nf_nat_initialized(ct, maniptype)) {
+					if (state->hook == NF_INET_POST_ROUTING &&
+					    nf_ct_protonum(ct) == IPPROTO_TCP &&
+					    (ct->status & IPS_SRC_NAT)) {
+						nf_nat_update_tcp_ts_offset(ct, skb);
+					}
 					goto do_nat;
+				}
 			}
 null_bind:
 			ret = nf_nat_alloc_null_binding(ct, state->hook);
-- 
2.43.0

Re: [PATCH net v2] netfilter: nf_nat: recalculate TCP TS offset when snat change sport

[Florian Westphal]

xietangxin <xietangxin@h-partners.com> wrote:
> v1: https://lore.kernel.org/all/20260629093408.3927103-1-xietangxin@h-partners.com/
> ---
>  net/netfilter/nf_nat_core.c | 103 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 63ff6b4d5d21..9d0b316fa3c7 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -16,6 +16,8 @@
>  #include <linux/siphash.h>
>  #include <linux/rtnetlink.h>
>  
> +#include <net/tcp.h>
> +#include <net/secure_seq.h>
>  #include <net/netfilter/nf_conntrack_bpf.h>
>  #include <net/netfilter/nf_conntrack_core.h>
>  #include <net/netfilter/nf_conntrack_helper.h>
> @@ -894,6 +896,99 @@ static bool in_vrf_postrouting(const struct nf_hook_state *state)
>  	return false;
>  }
>  
> +static __be32 *nf_nat_tcp_ts_option_ptr(const struct sk_buff *skb)
> +{
> +	struct tcphdr *th;
> +	unsigned char *ptr;
> +	unsigned char opcode;
> +	unsigned char opsize;
> +	unsigned int optlen, offset;
> +
> +	offset = 0;
> +	th = tcp_hdr(skb);
> +	optlen = (th->doff - 5) * 4;
> +	ptr = (unsigned char *)(th + 1);

Hmm, I don't think we should assume its in linear
area.  In future someone might re-use this for forwarded
packets too.

Given you munge the packet, I think you could just call
skb_ensure_writable() early to assert entire tcp header
including options is in linear area.

> +	while (offset < optlen) {
> +		opcode = ptr[offset];
> +		if (opcode == TCPOPT_EOL)
> +			break;
> +
> +		if (opcode == TCPOPT_NOP) {
> +			offset++;
> +			continue;
> +		}
> +
> +		if (offset + 1 >= optlen)
> +			break;
> +
> +		opsize = ptr[offset + 1];
> +		if (opsize < 2 || offset + opsize > optlen)
> +			break;
> +		if (opcode == TCPOPT_TIMESTAMP && opsize == TCPOLEN_TIMESTAMP)
> +			return (__be32 *)(ptr + offset + 2);

Maybe add a comment here that says that this is only for locally
generated packets and that linux tcp will always align the tsval.

Else, write needs put_unaligned_be32() instead of direct assign.

> +static void nf_nat_update_tcp_ts_offset(struct nf_conn *ct, struct sk_buff *skb)
> +{
> +	__be32 *tsptr;
> +	struct net *net;
> +	struct tcphdr *th;
> +	struct tcp_sock *tp;
> +	union tcp_seq_and_ts_off st;
> +	struct nf_conntrack_tuple *orig_tuple;
> +	struct nf_conntrack_tuple *reply_tuple;

Could reorder this for reverse x-mas tree (i.e. invert
the above order...).

> +	orig_tuple = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
> +	reply_tuple = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
> +	if (orig_tuple->src.u.tcp.port == reply_tuple->dst.u.tcp.port)
> +		return;

Maybe add a comment like /* no port rewrite? No need to update anything */

> +	th = tcp_hdr(skb);
> +	if (!th || !th->syn || th->ack)
> +		return;
> +
> +	net = nf_ct_net(ct);
> +	if (READ_ONCE(net->ipv4.sysctl_tcp_timestamps) != 1)
> +		return;
> +

Maybe add a comment that above check avoid bogus tsoff update for
non-randomized tcp timestamps?

> +	if (!skb->sk)
> +		return;
> +

I suggest to do this first, so that its obvious this function
is only for locally generated packets.

> +	tsptr = nf_nat_tcp_ts_option_ptr(skb);
> +	if (!tsptr)
> +		return;
> +
> +	switch (nf_ct_l3num(ct)) {
> +	case NFPROTO_IPV4:
> +		st = secure_tcp_seq_and_ts_off(net, reply_tuple->dst.u3.ip,
> +					       reply_tuple->src.u3.ip,
> +					       reply_tuple->dst.u.tcp.port,
> +					       reply_tuple->src.u.tcp.port);
> +		break;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case NFPROTO_IPV6:
> +		st = secure_tcpv6_seq_and_ts_off(net, reply_tuple->dst.u3.ip6,
> +						 reply_tuple->src.u3.ip6,
> +						 reply_tuple->dst.u.tcp.port,
> +						 reply_tuple->src.u.tcp.port);
> +		break;
> +#endif
> +	default:
> +		return;
> +	}
> +
> +	tp = tcp_sk(skb->sk);
> +	*tsptr = htonl(tcp_skb_timestamp_ts(tp->tcp_usec_ts, skb) + st.ts_off);
>
>  unsigned int
>  nf_nat_inet_fn(void *priv, struct sk_buff *skb,
>  	       const struct nf_hook_state *state)
> @@ -937,8 +1032,14 @@ nf_nat_inet_fn(void *priv, struct sk_buff *skb,
>  						       state);
>  				if (ret != NF_ACCEPT)
>  					return ret;
> -				if (nf_nat_initialized(ct, maniptype))
> +				if (nf_nat_initialized(ct, maniptype)) {
> +					if (state->hook == NF_INET_POST_ROUTING &&

I wonder if this should be LOCAL_OUT.  I do understand that many
people use SNAT/MASQUERADE from postrouting to deal with both local and
forwarded traffic at the same time, so more of a open question.