On Wed, Mar 30, 2022 at 1:41 PM Abhiram Sangana <[email protected]> wrote: > > Hi Numan, > > Thanks for reviewing the patch > > On 30 Mar 2022, at 16:55, Numan Siddique > <[email protected]<mailto:[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.
No worries. I guess it would make sense to have a strong reference if we go with the approach to make it mandatory for CMS to specify gateway_port. But since we can determine the gateway_port ourselves, this can be optional and weak reference. > > > 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 - > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_ovn-2Dorg_ovn_blob_main_northd_northd.c-23L9327&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=RotwPXnAckhnfEzdWgnpPR0nnZ46Y0RYo-uUGaS4vXY&m=JI0ykaKcvbB2_KQ9SVReBzWtqdpVkyfUf4yA4lHeJW7p7AGEj0-UcEijs3jMkRpS&s=htB40Gb52UEOPhtBUlXqtMHZq12vpII3d-eHclbBqlM&e= > > 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. > > Is the current behaviour of ovn-nbctl ok? Yes. But I'd suggest ovn-northd determine the appropriate DGP and not ovn-nbctl if CMS doesn't specify the gateway_port. Numan > > > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
