Hi Russell,
Really appreciate your comments. Will update soon. Regards, Juno Zhu IBM China Development Labs (CDL) Cloud IaaS Lab Email: na...@cn.ibm.com 5F, Building 10, 399 Keyuan Road, Zhangjiang Hi-Tech Park, Pudong New District, Shanghai, China (201203) From: Russell Bryant <russ...@ovn.org> To: Na Zhu/China/IBM@IBMCN Cc: ovs dev <dev@openvswitch.org> Date: 2016/04/05 20:35 Subject: Re: [ovs-dev] [PATCH 1/1] ovn: Add column enabled to table Logical_Router 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
