On 9/1/26 2:11 AM, Han Zhou wrote:
> On Thu, Aug 27, 2026 at 1:58 PM Tim Rozet <[email protected]> wrote:
>>
>> 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.
>>
>> Reported-at: https://github.com/ovn-kubernetes/ovn-kubernetes/issues/6422
>> Assisted-by: GPT-5, OpenAI Codex
>> Signed-off-by: Tim Rozet <[email protected]>
>> ---
>> v2:
>> - Require a port-specific VIP for the new force-SNAT cases.
>> - Add coverage ensuring a port-less VIP does not bypass UNSNAT.
>> - Preserve the existing NAT external-IP behavior.
>
> Hi Tim,
>
> Thanks for v2. I merged and backported to 26.09 and 26.03.
>
Hi Tim, Han,
Thanks for the patch and review!
> For 25.09 and 25.03, since the code base is different and the
> lport_addresses_contains_ip()
> is missing, we have to implement the function first. Could you please
> review if the below code looks good for the 25.09 branch (25.03 would be
> the same):
Just a note, branch 25.03 is _unsupported_ according to our policy and
currently protected on GitHub (no pushes are allowed to it). So unless
we have a very compelling reason to backport this to 25.03 (I don't
think we do), I'd just backport it to 25.09.
> -------------------------------------
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12399,6 +12399,60 @@ build_gw_lrouter_nat_flows_for_lb(struct
> lrouter_nat_lb_flows_ctx *ctx,
> bitmap_free(dp_non_meter);
> }
>
> +static bool
> +lport_addresses_contains_ip(const struct lport_addresses *lsp_addrs,
> + size_t n_lsp_addrs, const char *ip_s)
> +{
> + struct in6_addr ip6;
> + ovs_be32 ip4;
> +
> + if (ip_parse(ip_s, &ip4)) {
> + for (size_t i = 0; i < n_lsp_addrs; i++) {
> + for (size_t j = 0; j < lsp_addrs[i].n_ipv4_addrs; j++) {
> + if (lsp_addrs[i].ipv4_addrs[j].addr == ip4) {
> + return true;
> + }
> + }
> + }
> + return false;
> + }
> +
> + if (ipv6_parse(ip_s, &ip6)) {
> + for (size_t i = 0; i < n_lsp_addrs; i++) {
> + for (size_t j = 0; j < lsp_addrs[i].n_ipv6_addrs; j++) {
> + if (IN6_ARE_ADDR_EQUAL(&lsp_addrs[i].ipv6_addrs[j].addr,
> + &ip6)) {
> + return true;
> + }
> + }
> + }
> + }
> +
> + return false;
> +}
> +
This part looks good to me.
> ------------------------------------------
>
> Thanks,
> Han
>
Regards,
Dumitru
>>
>> northd/northd.c | 37 +++++++++++++++++++++++++++++--------
>> tests/ovn-northd.at | 42 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 71 insertions(+), 8 deletions(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 88e3ece88..47f76b108 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -13761,6 +13761,28 @@ build_gw_lrouter_nat_flows_for_lb(struct
> lrouter_nat_lb_flows_ctx *ctx,
>> bitmap_free(dp_non_meter);
>> }
>>
>> +static bool
>> +lrouter_lb_vip_is_unsnat_ip(const struct ovn_datapath *od,
>> + const struct lr_nat_record *lrnat_rec,
>> + const struct ovn_lb_vip *lb_vip)
>> +{
>> + const char *vip = lb_vip->vip_str;
>> +
>> + if (sset_contains(&lrnat_rec->external_ips, vip)) {
>> + return true;
>> + }
>> +
>> + /* A port-less bypass would also match replies sent to a force-SNAT
>> + * address and prevent them from reaching the UNSNAT flow. */
>> + return lb_vip->port_str
>> + && (lport_addresses_contains_ip(
>> + &lrnat_rec->dnat_force_snat_addrs, 1, vip)
>> + || lport_addresses_contains_ip(
>> + &lrnat_rec->lb_force_snat_addrs, 1, vip)
>> + || (lrnat_rec->lb_force_snat_router_ip
>> + && sset_contains(&od->router_ips, vip)));
>> +}
>> +
>> static void
>> build_lrouter_nat_flows_for_lb(
>> struct ovn_lb_vip *lb_vip,
>> @@ -13910,16 +13932,15 @@ build_lrouter_nat_flows_for_lb(
>> bitmap_set1(aff_dp_bitmap[type], index);
>> }
>>
>> - if (sset_contains(&lrnat_rec->external_ips, lb_vip->vip_str)) {
>> - /* The load balancer vip is also present in the NAT entries.
>> - * So add a high priority lflow to advance the the packet
>> - * destined to the vip (and the vip port if defined)
>> - * in the S_ROUTER_IN_UNSNAT stage.
>> + if (lrouter_lb_vip_is_unsnat_ip(od, lrnat_rec, lb_vip)) {
>> + /* The load balancer VIP is also present in an UNSNAT flow.
>> + * Add a high priority lflow to advance packets destined to
> the
>> + * VIP (and the VIP port if defined) in S_ROUTER_IN_UNSNAT.
>> * There seems to be an issue with ovs-vswitchd. When the new
>> - * connection packet destined for the lb vip is received,
>> - * it is dnat'ed in the S_ROUTER_IN_DNAT stage in the dnat
>> + * connection packet destined for the LB VIP is received,
>> + * it is DNATed in the S_ROUTER_IN_DNAT stage in the DNAT
>> * conntrack zone. For the next packet, if it goes through
>> - * unsnat stage, the conntrack flags are not set properly,
> and
>> + * UNSNAT stage, the conntrack flags are not set properly,
> and
>> * it doesn't hit the established state flows in
>> * S_ROUTER_IN_DNAT stage. */
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 120,
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index 6d191c1a0..d19978a62 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -1882,6 +1882,48 @@ OVN_CLEANUP_NORTHD
>> AT_CLEANUP
>> ])
>>
>> +OVN_FOR_EACH_NORTHD_NO_HV([
>> +AT_SETUP([Load balancer VIP in force-SNAT addresses])
>> +ovn_start
>> +
>> +check ovn-nbctl ls-add public
>> +check ovn-nbctl lr-add lr0
>> +check ovn-nbctl set logical_router lr0 options:chassis=ch1
>> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:00:00:00:01 \
>> + 192.0.2.1/24
>> +check ovn-nbctl lsp-add-router-port public public-lr0 lr0-public
>> +
>> +check ovn-nbctl lb-add lb0 192.0.2.1:30663 198.51.100.10:5201
>> +check ovn-nbctl lb-add lb1 192.0.2.1 198.51.100.11
>> +check ovn-nbctl lr-lb-add lr0 lb0
>> +check ovn-nbctl lr-lb-add lr0 lb1
>> +check ovn-nbctl lr-nat-add lr0 snat 169.254.0.47 198.51.100.0/24
>> +check ovn-nbctl --wait=sb sync
>> +
>> +ovn-sbctl dump-flows lr0 > sbflows
>> +AT_CAPTURE_FILE([sbflows])
>> +AT_CHECK([grep "lr_in_unsnat.*priority=120" sbflows], [1])
>> +
>> +check ovn-nbctl --wait=sb set logical_router lr0 \
>> + options:lb_force_snat_ip=192.0.2.1
>> +
>> +AT_CHECK([ovn-sbctl dump-flows lr0 | \
>> + grep "lr_in_unsnat.*priority=120" | ovn_strip_lflows], [0], [dnl
>> + table=??(lr_in_unsnat ), priority=120 , match=(ip4 && ip4.dst
> == 192.0.2.1 && tcp && tcp.dst == 30663), action=(next;)
>> +])
>> +
>> +check ovn-nbctl --wait=sb set logical_router lr0 \
>> + options:lb_force_snat_ip=router_ip
>> +
>> +AT_CHECK([ovn-sbctl dump-flows lr0 | \
>> + grep "lr_in_unsnat.*priority=120" | ovn_strip_lflows], [0], [dnl
>> + table=??(lr_in_unsnat ), priority=120 , match=(ip4 && ip4.dst
> == 192.0.2.1 && tcp && tcp.dst == 30663), action=(next;)
>> +])
>> +
>> +OVN_CLEANUP_NORTHD
>> +AT_CLEANUP
>> +])
>> +
>> OVN_FOR_EACH_NORTHD_NO_HV([
>> AT_SETUP([LRP same IP as VIP or SNAT])
>> ovn_start
>> --
>> 2.55.0
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
