Thanks,
-Tim
------------------------------------------------------------------------------
From: Aaron Conole <[email protected]>
Sent: Monday, September 14, 2026 11:45 AM
To: Eli Britstein <[email protected]>
Cc: [email protected] <[email protected]>; Tim Rozet <[email protected]>;
Alin
Serdean <[email protected]>; Maor Dickman <[email protected]>
Subject: Re: [PATCH v1 1/3] conntrack: Match kernel NAT handling for new
packets.
External email: Use caution opening links or attachments
Eli Britstein <[email protected]> writes:
From: Tim Rozet <[email protected]>
The userspace datapath applies a connection's existing NAT mapping to
a packet in the new state when a bare ct(nat) action is executed. This
differs from the Linux datapath, which leaves a new packet untranslated
unless the current action explicitly requests source or destination
NAT.
The kernel's nf_ct_nat() infers the NAT direction from conntrack status
only when the connection state is not IP_CT_NEW. For IP_CT_NEW, the
action must provide the manipulation direction:
https://github.com/torvalds/linux/blob/master/net/netfilter/nf_nat_ovs.c
Pass the current NAT action into handle_nat() and use its source or
destination flags to decide whether NAT may run for a new packet. This
also applies the same behavior to the userspace fast path.
Add a system test that sends the same UDP packet twice without a reply.
It verifies that bare NAT leaves the second new packet untranslated so
it reaches an explicit DNAT action. Run the test against both kernel
and userspace datapaths.
Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
I wouldn't call this a fix. Each CT implementation is allowed to
implement CT in whichever way it chooses. For example, there will be CT
offload paths, and they may not behave similarly.
I am planning on reviewing the series this week, but just noting that
even if the series is applied as-is, I wouldn't consider this a 'fix'
and would strip that label unless there was compelling reason not to do
so.
Assisted-by: GPT-5, Codex
Co-authored-by: Tim Rozet <[email protected]>
Signed-off-by: Tim Rozet <[email protected]>
Signed-off-by: Eli Britstein <[email protected]>
---
lib/conntrack.c | 16 +++++++++---
tests/system-traffic.at | 56 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f84cdd216..168954c35 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1195,9 +1195,17 @@ conn_update_state(struct conntrack *ct, struct dp_packet
*pkt,
static void
handle_nat(struct dp_packet *pkt, struct conn *conn,
- uint16_t zone, bool reply, bool related)
+ uint16_t zone, bool reply, bool related,
+ const struct nat_action_info_t *nat_action_info)
{
+ bool nat_config = nat_action_info->nat_action &
+ (NAT_ACTION_SRC | NAT_ACTION_DST);
+
+ /* Like the kernel datapath, do not infer an existing NAT mapping for a
+ * packet in the new state. Applying NAT in this state requires an
+ * explicit source or destination NAT action. */
if (conn->nat_action &&
+ (!(pkt->md.ct_state & CS_NEW) || nat_config) &&
(!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT) &&
zone != pkt->md.ct_zone))) {
@@ -1320,7 +1328,8 @@ process_one_fast(uint16_t zone, const uint32_t *setmark,
struct conn *conn, struct dp_packet *pkt)
{
if (nat_action_info) {
- handle_nat(pkt, conn, zone, pkt->md.reply, pkt->md.icmp_related);
+ handle_nat(pkt, conn, zone, pkt->md.reply, pkt->md.icmp_related,
+ nat_action_info);
pkt->md.conn = NULL;
}
@@ -1410,7 +1419,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
create_new_conn = conn_update_state(ct, pkt, ctx, conn, now);
}
if (nat_action_info && !create_new_conn) {
- handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related);
+ handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related,
+ nat_action_info);
}
} else if (check_orig_tuple(ct, pkt, ctx, now, &conn, nat_action_info)) {
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index ffb80d1e2..4ad51223d 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -4663,6 +4663,62 @@ NXST_FLOW reply:
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - bare NAT on repeated new UDP connection])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+OVS_TRAFFIC_VSWITCHD_START()
+
+AT_CHECK([ovs-vsctl -- add-port br0 p0 -- set Interface p0 type=internal dnl
+ ofport_request=1])
+
+AT_DATA([flows.txt], [dnl
+table=0,priority=100,in_port=1,udp,actions=ct(table=1,zone=42,nat)
+table=1,cookie=0x1,priority=200,udp,nw_dst=10.1.1.64,ct_state=+new+trk-dnat,ct_mark=0,actions=ct(commit,table=2,zone=42,nat(dst=10.1.1.2:53),exec(set_field:0x1->ct_mark))
+table=1,cookie=0x2,priority=200,udp,nw_dst=10.1.1.64,ct_state=+new+trk-dnat,ct_mark=0x1,actions=ct(commit,table=2,zone=42,nat(dst=10.1.1.2:53))
+table=1,cookie=0x3,priority=0,actions=drop
+table=2,cookie=0x4,priority=100,udp,nw_dst=10.1.1.2,ct_state=+new+trk+dnat,ct_mark=0x1,actions=drop
+table=2,cookie=0x5,priority=0,actions=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl 10.1.1.1:40000 -> 10.1.1.64:53, with the UDP checksum disabled.
+packet=50540000000a50540000000908004500001c000000004011648f0a0101010a0101409c40003500080000
+
+dnl The first packet creates the DNAT entry and stores the mark.
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 dnl
+ "in_port=1,packet=${packet},actions=resubmit(,0)"])
+OVS_WAIT_UNTIL([ovs-appctl dpctl/dump-conntrack zone=42 | grep -q "mark=1"])
+
+dnl With no reply seen, the second packet remains new. A bare ct(nat) must
+dnl leave it untranslated so that the explicit DNAT action is reached again.
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 dnl
+ "in_port=1,packet=${packet},actions=resubmit(,0)"])
+
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x1/-1 | dnl
+ grep -o "n_packets=[[0-9]]*"], [0], [dnl
+n_packets=1
+])
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x2/-1 | dnl
+ grep -o "n_packets=[[0-9]]*"], [0], [dnl
+n_packets=1
+])
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x3/-1 | dnl
+ grep -o "n_packets=[[0-9]]*"], [0], [dnl
+n_packets=0
+])
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x4/-1 | dnl
+ grep -o "n_packets=[[0-9]]*"], [0], [dnl
+n_packets=2
+])
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x5/-1 | dnl
+ grep -o "n_packets=[[0-9]]*"], [0], [dnl
+n_packets=0
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([conntrack - generic IP protocol])
CHECK_CONNTRACK()
OVS_TRAFFIC_VSWITCHD_START()
