[ovs-dev] [PATCH 1/1] ovn: Add column enabled to table Logical_Router
Na Zhu
nazhu at cn.ibm.com
Tue Apr 5 13:07:14 UTC 2016
Hi Russell,
Really appreciate your comments. Will update soon.
Regards,
Juno Zhu
IBM China Development Labs (CDL) Cloud IaaS Lab
Email: nazhu at cn.ibm.com
5F, Building 10, 399 Keyuan Road, Zhangjiang Hi-Tech Park, Pudong New
District, Shanghai, China (201203)
From: Russell Bryant <russell at ovn.org>
To: Na Zhu/China/IBM at IBMCN
Cc: ovs dev <dev at 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 <nazhu at 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 <nazhu at cn.ibm.com>
Reported-by: Na Zhu <nazhu at 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
More information about the dev
mailing list