Skip to content

Commit 1f0c776

Browse files
trozethzhou8
authored andcommitted
northd: Skip UNSNAT for force-SNAT load balancer VIPs.
Northd adds a priority-120 UNSNAT bypass when a load balancer VIP also appears as a NAT external IP. This prevents packets for the VIP from repeatedly entering an SNAT zone lookup that never commits the pre-DNAT tuple. Such packets remain ct.new and cannot be offloaded. Gateway routers using lb_force_snat_ip=router_ip may use a router port IP as the VIP while their NAT external IP is a different masquerade address. Northd then omits the bypass even though it creates an UNSNAT flow for the router port IP. Generate the bypass when a port-specific VIP also matches a DNAT or load balancer force-SNAT address, or a router port address selected by lb_force_snat_ip=router_ip. Do not extend the bypass to port-less VIPs in the new force-SNAT cases. Such a flow would match replies sent to the force-SNAT address and shadow the flow that reverses SNAT. Keep the existing NAT external-IP behavior unchanged. Add coverage using port-specific and port-less VIPs with a different masquerade SNAT address. A single-stream OVN-Kubernetes DPU test with the port-specific NodePort VIP improved from 8.8 Gbit/s to 19.8 Gbit/s. Backport note: lib/ovn-util.c on this branch has no lport_addresses_contains_ip(); upstream gained it in a feature commit that is not stable material. northd.c therefore carries a local static copy with identical semantics. The rest of the change is unmodified. Reported-at: ovn-kubernetes/ovn-kubernetes#6422 Assisted-by: GPT-5, OpenAI Codex Signed-off-by: Tim Rozet <trozet@nvidia.com> Signed-off-by: Han Zhou <hzhou@ovn.org> (cherry picked from commit 45831b7)
1 parent f7a907d commit 1f0c776

2 files changed

Lines changed: 103 additions & 8 deletions

File tree

northd/northd.c

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12403,6 +12403,60 @@ build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx,
1240312403
bitmap_free(dp_non_meter);
1240412404
}
1240512405

12406+
static bool
12407+
lport_addresses_contains_ip(const struct lport_addresses *lsp_addrs,
12408+
size_t n_lsp_addrs, const char *ip_s)
12409+
{
12410+
struct in6_addr ip6;
12411+
ovs_be32 ip4;
12412+
12413+
if (ip_parse(ip_s, &ip4)) {
12414+
for (size_t i = 0; i < n_lsp_addrs; i++) {
12415+
for (size_t j = 0; j < lsp_addrs[i].n_ipv4_addrs; j++) {
12416+
if (lsp_addrs[i].ipv4_addrs[j].addr == ip4) {
12417+
return true;
12418+
}
12419+
}
12420+
}
12421+
return false;
12422+
}
12423+
12424+
if (ipv6_parse(ip_s, &ip6)) {
12425+
for (size_t i = 0; i < n_lsp_addrs; i++) {
12426+
for (size_t j = 0; j < lsp_addrs[i].n_ipv6_addrs; j++) {
12427+
if (IN6_ARE_ADDR_EQUAL(&lsp_addrs[i].ipv6_addrs[j].addr,
12428+
&ip6)) {
12429+
return true;
12430+
}
12431+
}
12432+
}
12433+
}
12434+
12435+
return false;
12436+
}
12437+
12438+
static bool
12439+
lrouter_lb_vip_is_unsnat_ip(const struct ovn_datapath *od,
12440+
const struct lr_nat_record *lrnat_rec,
12441+
const struct ovn_lb_vip *lb_vip)
12442+
{
12443+
const char *vip = lb_vip->vip_str;
12444+
12445+
if (sset_contains(&lrnat_rec->external_ips, vip)) {
12446+
return true;
12447+
}
12448+
12449+
/* A port-less bypass would also match replies sent to a force-SNAT
12450+
* address and prevent them from reaching the UNSNAT flow. */
12451+
return lb_vip->port_str
12452+
&& (lport_addresses_contains_ip(
12453+
&lrnat_rec->dnat_force_snat_addrs, 1, vip)
12454+
|| lport_addresses_contains_ip(
12455+
&lrnat_rec->lb_force_snat_addrs, 1, vip)
12456+
|| (lrnat_rec->lb_force_snat_router_ip
12457+
&& sset_contains(&od->router_ips, vip)));
12458+
}
12459+
1240612460
static void
1240712461
build_lrouter_nat_flows_for_lb(
1240812462
struct ovn_lb_vip *lb_vip,
@@ -12552,16 +12606,15 @@ build_lrouter_nat_flows_for_lb(
1255212606
bitmap_set1(aff_dp_bitmap[type], index);
1255312607
}
1255412608

12555-
if (sset_contains(&lrnat_rec->external_ips, lb_vip->vip_str)) {
12556-
/* The load balancer vip is also present in the NAT entries.
12557-
* So add a high priority lflow to advance the the packet
12558-
* destined to the vip (and the vip port if defined)
12559-
* in the S_ROUTER_IN_UNSNAT stage.
12609+
if (lrouter_lb_vip_is_unsnat_ip(od, lrnat_rec, lb_vip)) {
12610+
/* The load balancer VIP is also present in an UNSNAT flow.
12611+
* Add a high priority lflow to advance packets destined to the
12612+
* VIP (and the VIP port if defined) in S_ROUTER_IN_UNSNAT.
1256012613
* There seems to be an issue with ovs-vswitchd. When the new
12561-
* connection packet destined for the lb vip is received,
12562-
* it is dnat'ed in the S_ROUTER_IN_DNAT stage in the dnat
12614+
* connection packet destined for the LB VIP is received,
12615+
* it is DNATed in the S_ROUTER_IN_DNAT stage in the DNAT
1256312616
* conntrack zone. For the next packet, if it goes through
12564-
* unsnat stage, the conntrack flags are not set properly, and
12617+
* UNSNAT stage, the conntrack flags are not set properly, and
1256512618
* it doesn't hit the established state flows in
1256612619
* S_ROUTER_IN_DNAT stage. */
1256712620
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_UNSNAT, 120,

tests/ovn-northd.at

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,6 +1733,48 @@ OVN_CLEANUP_NORTHD
17331733
AT_CLEANUP
17341734
])
17351735

1736+
OVN_FOR_EACH_NORTHD_NO_HV([
1737+
AT_SETUP([Load balancer VIP in force-SNAT addresses])
1738+
ovn_start
1739+
1740+
check ovn-nbctl ls-add public
1741+
check ovn-nbctl lr-add lr0
1742+
check ovn-nbctl set logical_router lr0 options:chassis=ch1
1743+
check ovn-nbctl lrp-add lr0 lr0-public 00:00:00:00:00:01 \
1744+
192.0.2.1/24
1745+
check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public
1746+
1747+
check ovn-nbctl lb-add lb0 192.0.2.1:30663 198.51.100.10:5201
1748+
check ovn-nbctl lb-add lb1 192.0.2.1 198.51.100.11
1749+
check ovn-nbctl lr-lb-add lr0 lb0
1750+
check ovn-nbctl lr-lb-add lr0 lb1
1751+
check ovn-nbctl lr-nat-add lr0 snat 169.254.0.47 198.51.100.0/24
1752+
check ovn-nbctl --wait=sb sync
1753+
1754+
ovn-sbctl dump-flows lr0 > sbflows
1755+
AT_CAPTURE_FILE([sbflows])
1756+
AT_CHECK([grep "lr_in_unsnat.*priority=120" sbflows], [1])
1757+
1758+
check ovn-nbctl --wait=sb set logical_router lr0 \
1759+
options:lb_force_snat_ip=192.0.2.1
1760+
1761+
AT_CHECK([ovn-sbctl dump-flows lr0 | \
1762+
grep "lr_in_unsnat.*priority=120" | ovn_strip_lflows], [0], [dnl
1763+
table=??(lr_in_unsnat ), priority=120 , match=(ip4 && ip4.dst == 192.0.2.1 && tcp && tcp.dst == 30663), action=(next;)
1764+
])
1765+
1766+
check ovn-nbctl --wait=sb set logical_router lr0 \
1767+
options:lb_force_snat_ip=router_ip
1768+
1769+
AT_CHECK([ovn-sbctl dump-flows lr0 | \
1770+
grep "lr_in_unsnat.*priority=120" | ovn_strip_lflows], [0], [dnl
1771+
table=??(lr_in_unsnat ), priority=120 , match=(ip4 && ip4.dst == 192.0.2.1 && tcp && tcp.dst == 30663), action=(next;)
1772+
])
1773+
1774+
OVN_CLEANUP_NORTHD
1775+
AT_CLEANUP
1776+
])
1777+
17361778
OVN_FOR_EACH_NORTHD_NO_HV([
17371779
AT_SETUP([LRP same IP as VIP or SNAT])
17381780
ovn_start

0 commit comments

Comments
 (0)