check_orig_tuple() looks a connection up by the ct_orig_tuple metadata, which records the pre-NAT (forward) tuple. Such a lookup can match an existing connection on its forward key even when the current packet is a new forward flow that merely carries stale ct_orig_tuple metadata from a previous NAT flow. The packet then aliases the wrong connection instead of creating its own.
A lookup by the orig tuple is only a valid match if the current packet is actually the reply to that connection, that is the wire tuple equals the connection's reverse key. Verify this after the lookup and reject the match otherwise, so a new connection is created for the current packet. Signed-off-by: Eli Britstein <[email protected]> --- NEWS | 3 + lib/conntrack.c | 14 ++++ tests/system-kmod-macros.at | 45 ++++++++++++ tests/system-traffic.at | 6 ++ tests/system-userspace-macros.at | 9 +++ tests/test-conntrack.c | 116 +++++++++++++++++++++++++++++++ 6 files changed, 193 insertions(+) diff --git a/NEWS b/NEWS index 23c1eb3eb..bc02152a5 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,9 @@ Post-v4.0.0 * A bare ct(commit,nat) action (no src/dst direction and no address or port range) now commits the connection without a NAT binding instead of installing an invalid reverse tuple, matching the Linux kernel datapath. + * A new packet carrying stale ct_orig_tuple metadata from a previous NAT + flow is no longer matched to an existing connection by its original + tuple; it now creates its own connection. v4.0.0 - 17 Aug 2026 diff --git a/lib/conntrack.c b/lib/conntrack.c index f7eecd24f..0f56f003f 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1297,6 +1297,20 @@ check_orig_tuple(struct conntrack *ct, struct dp_packet *pkt, key.dl_type = ctx_in->key.dl_type; key.zone = pkt->md.ct_zone; conn_lookup(ct, &key, now, conn, NULL); + + /* The ct_orig_tuple metadata records the pre-NAT (forward) tuple, so a + * lookup by it can match an existing connection on its forward key. That + * is only a valid match if the current packet is actually the reply to + * that connection, that is the wire tuple equals the connection's reverse + * key. Otherwise the metadata is stale (e.g. carried over from a prior + * NAT flow) and must not alias this connection; reject it so a new + * connection is created for the current packet. */ + if (*conn + && conn_key_cmp(&(*conn)->key_node[CT_DIR_REV].key, &ctx_in->key)) { + *conn = NULL; + return false; + } + return *conn ? true : false; } diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at index 75ef7642d..1756db3b3 100644 --- a/tests/system-kmod-macros.at +++ b/tests/system-kmod-macros.at @@ -279,6 +279,51 @@ m4_define([CHECK_NO_DPDK_OFFLOAD]) # The kernel module tests do not use TC offload. m4_define([CHECK_NO_TC_OFFLOAD]) +# OVS_CONNTRACK_ORIG_TUPLE_STALE_FORWARD() +# +# Kernel datapath smoke test: two distinct UDP flows in zone 1 must create +# two conntrack entries through the OpenFlow NAT commit path. +m4_define([OVS_CONNTRACK_ORIG_TUPLE_STALE_FORWARD], +[ +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") + +AT_DATA([flows.txt], [dnl +table=0,priority=100,in_port=1,udp,actions=ct(zone=1,table=1,nat) +table=1,cookie=0x1,priority=200,udp,tp_src=45112,ct_state=+new+trk,actions=ct(commit,zone=1,nat(src),table=2) +table=1,cookie=0x2,priority=200,udp,tp_src=37319,ct_state=+new+trk,actions=ct(commit,zone=1,nat(src),table=2) +table=1,priority=0,actions=drop +table=2,priority=0,actions=drop +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +flow_l3="eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,nw_src=10.1.1.1,nw_dst=8.8.0.10,nw_proto=17,nw_ttl=64,nw_frag=no" + +AT_CHECK([first_pkt=$(ovs-ofctl compose-packet --bare "$flow_l3, udp_src=45112,udp_dst=53"); dnl + ovs-ofctl -O OpenFlow13 packet-out br0 dnl + "in_port=1,packet=${first_pkt},actions=resubmit(,0)"]) + +OVS_WAIT_UNTIL([ovs-appctl dpctl/dump-conntrack zone=1 | grep -q "sport=45112,dport=53"]) + +AT_CHECK([second_pkt=$(ovs-ofctl compose-packet --bare "$flow_l3, udp_src=37319,udp_dst=53"); dnl + ovs-ofctl -O OpenFlow13 packet-out br0 dnl + "in_port=1,packet=${second_pkt},actions=resubmit(,0)"]) + +OVS_WAIT_UNTIL([ovs-appctl dpctl/dump-conntrack zone=1 | grep -q "sport=37319,dport=53"]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep "sport=37319,dport=53"], [0], [dnl +udp,orig=(src=10.1.1.1,dst=8.8.0.10,sport=37319,dport=53),reply=(src=8.8.0.10,dst=10.1.1.1,sport=53,dport=37319),zone=1 +]) + +AT_CHECK([sh -c 'test $(ovs-appctl dpctl/dump-conntrack zone=1 | grep -c "orig=.src=10\.1\.1\.1,dst=8\.8\.0\.10,") -eq 2']) + +OVS_TRAFFIC_VSWITCHD_STOP +]) + # OVS_CHECK_BAREUDP() # # The feature needs to be enabled in the kernel configuration (CONFIG_BAREUDP) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 254332afb..04a8500b9 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -4849,6 +4849,12 @@ OVS_TRAFFIC_VSWITCHD_STOP(["dnl /execute ct.*Invalid argument/d"]) AT_CLEANUP +AT_SETUP([conntrack - orig tuple rejects stale forward query]) +CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() +OVS_CONNTRACK_ORIG_TUPLE_STALE_FORWARD() +AT_CLEANUP + AT_SETUP([conntrack - generic IP protocol]) CHECK_CONNTRACK() OVS_TRAFFIC_VSWITCHD_START() diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at index f0d9121e3..e9ef86e82 100644 --- a/tests/system-userspace-macros.at +++ b/tests/system-userspace-macros.at @@ -384,6 +384,15 @@ m4_define([CHECK_EXTERNAL_CT], AT_SKIP_IF([:]) ]) +# OVS_CONNTRACK_ORIG_TUPLE_STALE_FORWARD() +# +# Userspace regression for stale ct_orig_tuple on a forward query. The +# kernel datapath uses a separate smoke test in system-kmod-macros.at. +m4_define([OVS_CONNTRACK_ORIG_TUPLE_STALE_FORWARD], +[ + AT_CHECK([ovstest test-conntrack orig-tuple-rejects-stale-forward]) +]) + # ADD_EXTERNAL_CT() # # The userspace datapath does not support external ct. diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c index 2babe989c..b2f650134 100644 --- a/tests/test-conntrack.c +++ b/tests/test-conntrack.c @@ -15,12 +15,17 @@ */ #include <config.h> + #include "conntrack.h" +#include <arpa/inet.h> + +#include "ct-dpif.h" #include "dp-packet.h" #include "fatal-signal.h" #include "flow.h" #include "netdev.h" +#include "packets.h" #include "ovs-thread.h" #include "ovstest.h" #include "pcap-file.h" @@ -148,6 +153,72 @@ build_tcp_packet(struct dp_packet *pkt, uint16_t tcp_src, uint16_t tcp_dst, return pkt; } +static struct dp_packet * +build_udp_packet(struct dp_packet *pkt, uint16_t udp_src, uint16_t udp_dst, + const char *udp_payload, size_t payload_len) +{ + struct udp_header *udph; + struct ip_header *iph; + uint16_t ip_tot_len; + uint32_t udp_csum; + struct flow flow; + + ovs_assert(pkt); + udph = dp_packet_l4(pkt); + ovs_assert(udph); + + udph->udp_src = htons(udp_src); + udph->udp_dst = htons(udp_dst); + udph->udp_len = htons(UDP_HEADER_LEN + payload_len); + udph->udp_csum = 0; + + if (udp_payload && payload_len > 0) { + memcpy((char *) udph + UDP_HEADER_LEN, udp_payload, payload_len); + } + + iph = dp_packet_l3(pkt); + ip_tot_len = IP_HEADER_LEN + UDP_HEADER_LEN + payload_len; + iph->ip_tot_len = htons(ip_tot_len); + iph->ip_csum = 0; + iph->ip_csum = csum(iph, IP_HEADER_LEN); + + udp_csum = packet_csum_pseudoheader(iph); + udph->udp_csum = csum_finish( + csum_continue(udp_csum, udph, UDP_HEADER_LEN + payload_len)); + + flow_extract(pkt, &flow); + return pkt; +} + +static void +set_ct_orig_tuple_ipv4(struct dp_packet *pkt, ovs_be32 src, ovs_be32 dst, + uint16_t src_port, uint16_t dst_port, uint8_t proto) +{ + pkt->md.ct_orig_tuple_ipv6 = false; + pkt->md.ct_orig_tuple.ipv4 = (struct ovs_key_ct_tuple_ipv4) { + src, dst, htons(src_port), htons(dst_port), proto, + }; +} + +static unsigned int +ct_zone_conn_count(struct conntrack *tracker, uint16_t zone) +{ + struct conntrack_dump dump; + struct ct_dpif_entry entry; + unsigned int count = 0; + int tot_bkts; + + conntrack_dump_start(tracker, &dump, &zone, &tot_bkts); + + while (conntrack_dump_next(&dump, &entry) != EOF) { + count++; + ct_dpif_entry_uninit(&entry); + } + + conntrack_dump_done(&dump); + return count; +} + static struct dp_packet_batch * prepare_packets(size_t n, bool change, unsigned tid, ovs_be16 *dl_type) { @@ -576,6 +647,49 @@ test_ftp_alg_large_payload(struct ovs_cmdl_context *ctx OVS_UNUSED) conntrack_destroy(ct); } +static void +test_orig_tuple_rejects_stale_forward(struct ovs_cmdl_context *ctx OVS_UNUSED) +{ + struct eth_addr eth_src = ETH_ADDR_C(50, 54, 00, 00, 00, 09); + struct eth_addr eth_dst = ETH_ADDR_C(50, 54, 00, 00, 00, 0a); + ovs_be32 ip_src = inet_addr("12.12.12.11"); + ovs_be32 ip_dst = inet_addr("8.8.0.10"); + struct nat_action_info_t src_only; + struct dp_packet_batch batch; + long long now = time_msec(); + struct dp_packet *pkt; + + ct = conntrack_init(); + + memset(&src_only, 0, sizeof src_only); + src_only.nat_action = NAT_ACTION_SRC; + + pkt = build_eth_ip_packet(NULL, eth_src, eth_dst, ip_src, ip_dst, + IPPROTO_UDP, 0); + build_udp_packet(pkt, 45112, 53, NULL, 0); + + dp_packet_batch_init_packet(&batch, pkt); + conntrack_execute(ct, &batch, htons(ETH_TYPE_IP), false, true, 1, + NULL, NULL, NULL, &src_only, now, 0); + dp_packet_delete_batch(&batch, false); + + ovs_assert(ct_zone_conn_count(ct, 1) == 1); + + build_udp_packet(pkt, 37319, 53, NULL, 0); + pkt->md.ct_state = CS_SRC_NAT; + pkt->md.ct_zone = 1; + set_ct_orig_tuple_ipv4(pkt, ip_src, ip_dst, 45112, 53, IPPROTO_UDP); + + dp_packet_batch_init_packet(&batch, pkt); + conntrack_execute(ct, &batch, htons(ETH_TYPE_IP), false, true, 1, + NULL, NULL, NULL, &src_only, now, 0); + ovs_assert(pkt->md.ct_state & CS_NEW); + ovs_assert(!(pkt->md.ct_state & CS_REPLY_DIR)); + dp_packet_delete_batch(&batch, true); + + ovs_assert(ct_zone_conn_count(ct, 1) == 2); + conntrack_destroy(ct); +} static const struct ovs_cmdl_command commands[] = { /* Connection tracker tests. */ @@ -601,6 +715,8 @@ static const struct ovs_cmdl_command commands[] = { * is rewritten to the SNAT target rather than causing a crash. */ {"ftp-alg-large-payload", "", 0, 0, test_ftp_alg_large_payload, OVS_RO}, + {"orig-tuple-rejects-stale-forward", "", 0, 0, + test_orig_tuple_rejects_stale_forward, OVS_RO}, {NULL, NULL, 0, 0, NULL, OVS_RO}, }; -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
