On Tue, Apr 5, 2016 at 6:54 AM, Na Zhu <na...@cn.ibm.com> wrote: > This patch add column "enabled" to table Logical_Router for setting router > administrative state. > > The type of "enabled" is bool. > > If the administrative state is false, delete all the flows relevant to the > > logical router from table Logical_Flow. >
You have an extra blank line in the commit message above. > > Signed-off-by: Na Zhu <na...@cn.ibm.com> > Reported-by: Na Zhu <na...@cn.ibm.com> > Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1563175 > --- > ovn/northd/ovn-northd.8.xml | 4 ++++ > ovn/northd/ovn-northd.c | 46 > +++++++++++++++++++++++++++++++++++++++++++++ > ovn/ovn-nb.ovsschema | 3 ++- > ovn/ovn-nb.xml | 7 +++++++ > 4 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml > index da776e1..fed996c 100644 > --- a/ovn/northd/ovn-northd.8.xml > +++ b/ovn/northd/ovn-northd.8.xml > @@ -397,6 +397,10 @@ output; > > <h2>Logical Router Datapaths</h2> > > + <p> > + This is only enabled logical router. > + </p> > + > I suggest this alternative wording: "Logical router datapaths will only exist for <ref table="Logical_Router" db="OVN_Northbound"/> rows in the <ref db="OVN_Northbound"/> database that do not have <ref column="enabled" table="Logical_Router" db="OVN_Northbound"/> set to <code>false</code>." > <h3>Ingress Table 0: L2 Admission Control</h3> > > <p> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 4b1d611..ec1c6af 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -1312,6 +1312,12 @@ lport_is_up(const struct nbrec_logical_port *lport) > } > > static bool > +lrouter_is_enabled(const struct nbrec_logical_router *lrouter) > +{ > + return !lrouter->enabled || *lrouter->enabled; > +} > + > +static bool > has_stateful_acl(struct ovn_datapath *od) > { > for (size_t i = 0; i < od->nbs->n_acls; i++) { > @@ -1793,6 +1799,10 @@ build_lrouter_flows(struct hmap *datapaths, struct > hmap *ports, > continue; > } > > + if (!lrouter_is_enabled(od->nbr)) { > + continue; > + } > + > This patch has to check this in several places. I think it could be simplified to only check it in one place. ovn-northd only iterates the Logical_Router table in OVN_Northbound in a single place. Could you just ignore the Logical_Router row in that one place instead? diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema > index 40a7a97..e878ac8 100644 > --- a/ovn/ovn-nb.ovsschema > +++ b/ovn/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > "version": "2.0.2", > - "cksum": "4289495412 4436", > + "cksum": "1227843805 4513", > "tables": { > "Logical_Switch": { > "columns": { > You should update the schema version, as well. I would make it "2.1.0". -- Russell Bryant _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
