On Tue, Sep 1, 2026 at 2:10 AM Dumitru Ceara <[email protected]> wrote:
>
> 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.
Thanks Dumitru. I applied it to 25.09.
I think I was mislead by the texts in backporting-patches.rst:
---
To make things easy, maintainers should simply backport all bugfixes to the
previous four branches before main.
---
Best,
Han
>
> > -------------------------------------
> > --- 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
