On 2/14/25 7:08 AM, [email protected] wrote: > On Fri, 2025-02-14 at 00:46 +0100, Dumitru Ceara wrote: >> <snip> > >>> >>> For the tests in ovn-northd.at, I think we should: >>> - remove unneeded DB table dumps >>> - reindent ovn-nbctl commands >>> - fix comments >>> - use fetch_column/check_column/check_row_count instead of bare >>> AT_CHECKs. >>> >>> I was trying to cover the review comments (and taking care of the >>> test >>> nits) here: >>> https://github.com/dceara/ovn/commit/0785375 >>> >>> If it looks OK to you, feel free to use it in v7. >>> That commit doesn't add a test for the distributed NAT scenario >>> though. >>> >> >> Actually, I couldn't help it and wrote the distributed NAT test and >> it >> uncovered a bug in my incremental commit above. The distributed NAT >> test and the fix for the bug are available here: >> >> https://github.com/dceara/ovn/commit/119d849 >> >> The actual dev branch is: >> https://github.com/dceara/ovn/commits/refs/heads/tmp-bgp-lb-nat-advertise-v6/ >> >> The last two commits there can be squashed in patch 4/4 if they look >> OK >> to you too. >> > Haha, I wish I noticed your mail sooner, would've saved me some time > :D. Anyway, I wanted to do testing in system tests, and in the meantime > I discovered one more bug. See my additional commit on top of yours: > https://github.com/mkalcok/ovn/commit/47d0c4bd6a80c6beb39e1cd5be730c2af7afb74c >
Nice! However, I think we need to adjust the fix a bit. You're checking nat->is_valid and if it's not still process the nat entry. We probably need to skip all invalid nat entries early instead. > Branch: > https://github.com/mkalcok/ovn/tree/bgp-route-leak-with-sb-v11 > > While I was stumbling around trying to hit the right combination of > options for distributed nat (I always forget all the conditions), I > found out that we need to check for nat record's validity as well. In > the tests that I added, I'm adding 2nd DGW port to R2, which causes all > the previous rules to be invalid (yet they somehow gain the > is_distributed=true). > > I'll squash all these and post next version. > Ack, I'll have a look soon. Thanks! > Thanks for the help, > Martin. > >> Regards, >> Dumitru >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
