On Fri, Nov 28, 2025 at 7:56 AM Dumitru Ceara via dev <[email protected]> wrote: > > On 11/28/25 1:16 PM, Rukomoinikova Aleksandra wrote: > > Hi! Thank you so much for the review. I'd be glad if you merged it after > > the squash. Thank you so much for the edits you made. There really are a > > lot of them. I'll take a look at them and keep them in mind for the future. > > > > Hi Alexandra, > > No worries! I squashed in the suggested minor changes and pushed the > patch to main. > > Regards, > Dumitru > > Hi Aleksandra,
Unfortunately this patch breaks the traffic from external to the dnat_and_snat distributed IP of a logical port if the logical router has only dnat_and_snat distributed entries (with NO snat entries). You can easily reproduce it using ovn-fake-multinode by deleting the snat entries from the logical router lr1 and then trying to reach 172.16.1.110 (dnat_and_snat ip) of sw01-port1 from ovnfake-ext1 namespace in the host. I tried to fix the issue but I was finding it hard to fix it in the handler of en-ls-arp engine node which this patch adds. Basically this commit is excluding all the NAT entries of a logical router if there are no snat entries. I'm not sure why the below check is added here https://github.com/ovn-org/ovn/blob/main/northd/en-ls-arp.c#L101 ---------------- for (size_t i = 0; i < lrnat_rec->n_nat_entries; i++) { const struct ovn_nat *nat_entry = &lrnat_rec->nat_entries[i]; if (is_centralized_nat_record(nat_entry)) { hmapx_add(&ls_arp_record->nat_records, (struct lrnat_rec *) lrnat_rec); } } ------------ In the above for loop, we iterate through all the nat entries of lrnat_rec and only add if it has atleast one centralized nat record. Why is this required ? Why can't the whole lrnat_rec be stored in the ls_arp_record ? Can you please take a look at it ? Thanks Numan > [...] > > >> The changes I'm suggesting are minor, here's the full diff (some more > >> style issues > >> are addressed in it): > >> > >> https://github.com/dceara/ovn/commit/a2307c9 > >> > >> If it looks good to you too I can squash it in and push the patch to main. > >> Please let me know what you think. > >> > >> Regards, > >> Dumitru > >> > > _______________________________________________ > 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
