Hi Numan, Thanks for reviewing the patch.
> On 30 Mar 2022, at 16:55, Numan Siddique <[email protected]> wrote: > > > I think the "gateway_port" column can be a weak reference to > "Logical_Router_Port". > Otherwise CMS cannot delete the logical router port unless it clears > this column or deletes the NAT row. My bad. Will change this column to be a weak reference to LRP. > IMO, this column should be made optional. If CMS doesn''t specify the > gateway port to use for a NAT entry, > ovn-northd can figure out which router port is reachable for the > "external_ip" of the NAT. > ovn-northd already does this for the Logical_Router_Static_Route's > output_port column. > > Please check this out - > > I'd suggest making this column optional. What do you think ? > Got it. So, if we make this column optional, is this the behaviour that we want? If LR has a single DGP and “gateway_port” is not specified, we program the NAT rule with the DGP even if "external_ip" of the rule is not in the LRP networks. If LR has multiple DGPs and "gateway_port" is not specified, we try to determine which DGP is appropriate based on the LRP networks and "external_ip" of the rule. If no DGP matches, we do not program the NAT rule. > > There can be odd timing issues in CI. So I'd suggest adding --wait=sb > or --wait=hv in the ovn-nbctl command just before AT_CHECK is > called. > > There are many places in the test case you can do the same i..e use > --wait=sb/hv. > > Can you please check and add this in the relevant places ? Sure, will fix this. Thanks, Abhiram Sangana _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
