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 ctinfo 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.")
Assisted-by: GPT-5, OpenAI Codex
Signed-off-by: Tim Rozet <[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()
-- 
2.55.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to