Sorry about the delay. I applied these to master and backported as far as 2.7.
On Fri, Oct 27, 2017 at 03:22:08PM +0200, Miguel Angel Ajo Pelayo wrote: > Can we move this patch forward and get a backport to 2.8 / 2.7 branches? > > Otherwise we get a failure on openstack's: > > neutron.tests.tempest.api.test_routers.RoutersTest.test_router_interface_status > > > Cheers & thanks. > > On Mon, Oct 2, 2017 at 6:25 PM, Miguel Angel Ajo Pelayo <[email protected] > > wrote: > > > Acked-by: Miguel Angel Ajo <[email protected]> > > > > On Mon, Oct 2, 2017 at 3:39 PM, Jakub Sitnicki <[email protected]> wrote: > > > >> Hi Ajo, > >> > >> On Mon, 2 Oct 2017 13:39:03 +0200 > >> Miguel Angel Ajo Pelayo <[email protected]> wrote: > >> > >> > On Fri, Sep 29, 2017 at 9:01 PM, Mark Michelson <[email protected]> > >> wrote: > >> > > >> > > LGTM > >> > > > >> > > On Fri, Sep 29, 2017 at 10:07 AM Jakub Sitnicki <[email protected]> > >> wrote: > >> > > > >> > > > Employ the simplest possible approach to determine the state of > >> logical > >> > > > ports that connect to logical routers by hardcoding it to always up. > >> > > > This is intended to be less surprising than the current approach > >> where > >> > > > router ports appear as being down (with the exception of ones > >> linking to > >> > > > gateway routers, which are bound). > >> > > > > >> > > > Reported-at: > >> > > > https://mail.openvswitch.org/pipermail/ovs-discuss/2017- > >> > > August/045202.html > >> > > > Signed-off-by: Jakub Sitnicki <[email protected]> > >> > > > > >> > > Acked-by: Mark Michelson <[email protected]> > >> > > > >> > > > --- > >> > > > ovn/northd/ovn-northd.c | 2 +- > >> > > > ovn/ovn-nb.xml | 26 +++++++++++++------ > >> > > > tests/ovn-northd.at | 69 > >> > > > +++++++++++++++++++++++++++++++++++++++++++++++++ > >> > > > 3 files changed, 88 insertions(+), 9 deletions(-) > >> > > > > >> > > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > >> > > > index 37651a0..791da1e 100644 > >> > > > --- a/ovn/northd/ovn-northd.c > >> > > > +++ b/ovn/northd/ovn-northd.c > >> > > > @@ -5980,7 +5980,7 @@ update_logical_port_status(struct > >> northd_context > >> > > > *ctx) > >> > > > continue; > >> > > > } > >> > > > > >> > > > - bool up = sb->chassis ? true : false; > >> > > > + bool up = (sb->chassis || !strcmp(nbsp->type, "router")); > >> > > > >> > > >> > doesn't this introduce a functional change for ports related to gateway > >> > routers (which are boundable?) Could we exclude those from the "always > >> up" > >> > and let it happen normally? > >> > > >> > We were counting on the up/down state of those ports to realize the > >> master > >> > of active/backup gateway chassis sets. > >> > >> Yes, there is a functional change for logical ports that link to a > >> gateway router (that is one in Logical_Router table that has > >> options:chassis set). The newly added "ovn -- check up state of router > >> LSP linked to a gateway LR" test aims to exercise this scenario. > >> > >> However, as I understand, gateway routers are a legacy way of providing > >> connectivity to an external network. They were superseded by LRPs > >> with as associated gateway chassis. (Corresponding test - "check up > >> state of router LSP linked to an LRP with set Gateway Chassis".) > >> > >> > > Thank you for the explanation, it makes sense. > > > > > >> For these, LSPs linked to one or more Gateway_Chassis, there is no > >> functionality loss. They currently appear as always down, and with this > >> patchset will appear as always up. > > > > > >> We could introduce an additional look-up in > >> update_logical_port_status() to locate the LRP that corresponds to a > >> LSP to take into account the existence Gateway_Chassis binding. > >> > >> I would probably do this on top of this patch in a separate series - as > >> a further improvement but just for logical ports associated with gateway > >> chassis this time. > >> > >> Would that be useful enough to justify an additional look-up each time > >> when we process a change in SB DB from ovn-northd's main loop? > >> > >> > > It's probably not worth it. I was thinking that we could look up on the > > SBDB for the chassisredirect port that we derive out of the lrp/lsp in this > > case. But what we finally get is a simple up/down state which is not very > > helpful. > > > > What would be helpful in neutron would be getting to know which chassis is > > active. But we can do that via the SBDB connection we also keep. > > > > > > I'm acking the patch based on the discussion. > > > > > >> Thanks, > >> Jakub > >> > >> > > >> > > >> > > >> > > if (!nbsp->up || *nbsp->up != up) { > >> > > > nbrec_logical_switch_port_set_up(nbsp, &up, 1); > >> > > > } > >> > > > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml > >> > > > index 9869d7e..5b02269 100644 > >> > > > --- a/ovn/ovn-nb.xml > >> > > > +++ b/ovn/ovn-nb.xml > >> > > > @@ -513,14 +513,24 @@ > >> > > > > >> > > > <group title="Port State"> > >> > > > <column name="up"> > >> > > > - This column is populated by <code>ovn-northd</code>, > >> rather than > >> > > > by the > >> > > > - CMS plugin as is most of this database. When a logical > >> port is > >> > > > bound > >> > > > - to a physical location in the OVN Southbound database <ref > >> > > > - db="OVN_Southbound" table="Binding"/> table, > >> > > > <code>ovn-northd</code> > >> > > > - sets this column to <code>true</code>; otherwise, or if > >> the port > >> > > > - becomes unbound later, it sets it to <code>false</code>. > >> This > >> > > > allows > >> > > > - the CMS to wait for a VM's (or container's) networking to > >> become > >> > > > active > >> > > > - before it allows the VM (or container) to start. > >> > > > + <p> > >> > > > + This column is populated by <code>ovn-northd</code>, > >> rather > >> > > > + than by the CMS plugin as is most of this database. > >> When a > >> > > > + logical port is bound to a physical location in the OVN > >> > > > + Southbound database <ref db="OVN_Southbound" > >> > > > + table="Binding"/> table, <code>ovn-northd</code> sets > >> this > >> > > > + column to <code>true</code>; otherwise, or if the port > >> > > > + becomes unbound later, it sets it to <code>false</code>. > >> > > > + This allows the CMS to wait for a VM's (or container's) > >> > > > + networking to become active before it allows the VM (or > >> > > > + container) to start. > >> > > > + </p> > >> > > > + > >> > > > + <p> > >> > > > + Logical ports of router type are an exception to this > >> rule. > >> > > > + They are considered to be always up, that is this column > >> is > >> > > > + always set to <code>true</code>. > >> > > > + </p> > >> > > > </column> > >> > > > > >> > > > <column name="enabled"> > >> > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > >> > > > index fc9eda8..954e259 100644 > >> > > > --- a/tests/ovn-northd.at > >> > > > +++ b/tests/ovn-northd.at > >> > > > @@ -83,3 +83,72 @@ ovn-nbctl --wait=sb remove Logical_Router_Port > >> bob > >> > > > options redirect-chassis > >> > > > AT_CHECK([ovn-sbctl find Gateway_Chassis name=bob_gw1], [0], []) > >> > > > > >> > > > AT_CLEANUP > >> > > > + > >> > > > +AT_SETUP([ovn -- check up state of VIF LSP]) > >> > > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > >> > > > +ovn_start > >> > > > + > >> > > > +ovn-nbctl ls-add S1 > >> > > > +ovn-nbctl lsp-add S1 S1-vm1 > >> > > > +AT_CHECK([test x`ovn-nbctl lsp-get-up S1-vm1` = xdown]) > >> > > > + > >> > > > +ovn-sbctl chassis-add hv1 geneve 127.0.0.1 > >> > > > +ovn-sbctl lsp-bind S1-vm1 hv1 > >> > > > +AT_CHECK([test x`ovn-nbctl lsp-get-up S1-vm1` = xup]) > >> > > > + > >> > > > +AT_CLEANUP > >> > > > + > >> > > > +AT_SETUP([ovn -- check up state of router LSP linked to a > >> distributed > >> > > LR]) > >> > > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > >> > > > +ovn_start > >> > > > + > >> > > > +ovn-nbctl lr-add R1 > >> > > > +ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24 > >> > > > + > >> > > > +ovn-nbctl ls-add S1 > >> > > > +ovn-nbctl lsp-add S1 S1-R1 > >> > > > +ovn-nbctl lsp-set-type S1-R1 router > >> > > > +ovn-nbctl lsp-set-addresses S1-R1 02:ac:10:01:00:01 > >> > > > +ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1 > >> > > > +AT_CHECK([test x`ovn-nbctl lsp-get-up S1-R1` = xup]) > >> > > > + > >> > > > +AT_CLEANUP > >> > > > + > >> > > > +AT_SETUP([ovn -- check up state of router LSP linked to a gateway > >> LR]) > >> > > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > >> > > > +ovn_start > >> > > > + > >> > > > +ovn-sbctl chassis-add gw1 geneve 127.0.0.1 > >> > > > + > >> > > > +ovn-nbctl create Logical_Router name=R1 options:chassis=gw1 > >> > > > +ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24 > >> > > > + > >> > > > +ovn-nbctl ls-add S1 > >> > > > +ovn-nbctl lsp-add S1 S1-R1 > >> > > > +ovn-nbctl lsp-set-type S1-R1 router > >> > > > +ovn-nbctl lsp-set-addresses S1-R1 02:ac:10:01:00:01 > >> > > > +ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1 > >> > > > + > >> > > > +ovn-sbctl lsp-bind S1-R1 gw1 > >> > > > +AT_CHECK([test x`ovn-nbctl lsp-get-up S1-R1` = xup]) > >> > > > + > >> > > > +AT_CLEANUP > >> > > > + > >> > > > +AT_SETUP([ovn -- check up state of router LSP linked to an LRP > >> with set > >> > > > Gateway Chassis]) > >> > > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > >> > > > +ovn_start > >> > > > + > >> > > > +ovn-sbctl chassis-add gw1 geneve 127.0.0.1 > >> > > > + > >> > > > +ovn-nbctl lr-add R1 > >> > > > +ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24 > >> > > > +ovn-nbctl lrp-set-gateway-chassis R1-S1 gw1 > >> > > > + > >> > > > +ovn-nbctl ls-add S1 > >> > > > +ovn-nbctl lsp-add S1 S1-R1 > >> > > > +ovn-nbctl lsp-set-type S1-R1 router > >> > > > +ovn-nbctl lsp-set-addresses S1-R1 router > >> > > > +ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1 > >> > > > +AT_CHECK([test x`ovn-nbctl lsp-get-up S1-R1` = xup]) > >> > > > + > >> > > > +AT_CLEANUP > >> > > > -- > >> > > > 2.9.5 > >> > > > > >> > > > _______________________________________________ > >> > > > 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 > >> > > > >> > >> > > > _______________________________________________ > 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
