[ovs-dev] ovn: Reduce range of ACL priorities.
diff mbox

Message ID 1445294822-115676-1-git-send-email-jpettit@nicira.com
State Accepted
Headers show

Commit Message

Justin Pettit Oct. 19, 2015, 10:47 p.m. UTC
To implement stateful ACLs, we've needed to reserve multiple logical
flow priorities in the ACL table.  Rather than continue to have a
strange range of ACL priorities, we'll make ACL priority range 0 to
32767 and then offset them by 1000 when inserting them into the logical
flow table.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
---
 ovn/northd/ovn-northd.c   |   18 ++++++++++++++----
 ovn/ovn-nb.ovsschema      |    6 +++---
 ovn/ovn-nb.xml            |    2 +-
 ovn/utilities/ovn-nbctl.c |    8 ++++----
 4 files changed, 22 insertions(+), 12 deletions(-)

Comments

Ben Pfaff Oct. 19, 2015, 11:42 p.m. UTC | #1
On Mon, Oct 19, 2015 at 03:47:02PM -0700, Justin Pettit wrote:
> To implement stateful ACLs, we've needed to reserve multiple logical
> flow priorities in the ACL table.  Rather than continue to have a
> strange range of ACL priorities, we'll make ACL priority range 0 to
> 32767 and then offset them by 1000 when inserting them into the logical
> flow table.
> 
> Signed-off-by: Justin Pettit <jpettit@nicira.com>

Acked-by: Ben Pfaff <blp@nicira.com>
Justin Pettit Oct. 19, 2015, 11:45 p.m. UTC | #2
> On Oct 19, 2015, at 4:42 PM, Ben Pfaff <blp@nicira.com> wrote:
> 
> On Mon, Oct 19, 2015 at 03:47:02PM -0700, Justin Pettit wrote:
>> To implement stateful ACLs, we've needed to reserve multiple logical
>> flow priorities in the ACL table.  Rather than continue to have a
>> strange range of ACL priorities, we'll make ACL priority range 0 to
>> 32767 and then offset them by 1000 when inserting them into the logical
>> flow table.
>> 
>> Signed-off-by: Justin Pettit <jpettit@nicira.com>
> 
> Acked-by: Ben Pfaff <blp@nicira.com>

Thanks.  I pushed this to master.

--Justin

Patch
diff mbox

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index a1ad34c..e199937 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -111,6 +111,12 @@  enum ovn_stage {
 #undef PIPELINE_STAGE
 };
 
+/* Due to various hard-coded priorities need to implement ACLs, the
+ * northbound database supports a smaller range of ACL priorities than
+ * are available to logical flows.  This value is added to an ACL
+ * priority to determine the ACL's logical flow priority. */
+#define OVN_ACL_PRI_OFFSET 1000
+
 /* Returns an "enum ovn_stage" built from the arguments. */
 static enum ovn_stage
 ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline,
@@ -1056,7 +1062,8 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
              * may and then its return traffic would not have an
              * associated conntrack entry and would return "+invalid". */
             const char *actions = has_stateful ? "ct_commit; next;" : "next;";
-            ovn_lflow_add(lflows, od, stage, acl->priority,
+            ovn_lflow_add(lflows, od, stage,
+                          acl->priority + OVN_ACL_PRI_OFFSET,
                           acl->match, actions);
         } else if (!strcmp(acl->action, "allow-related")) {
             struct ds match = DS_EMPTY_INITIALIZER;
@@ -1065,17 +1072,20 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
              * other traffic related to this entry to flow due to the
              * 65535 priority flow defined earlier. */
             ds_put_format(&match, "ct.new && (%s)", acl->match);
-            ovn_lflow_add(lflows, od, stage, acl->priority,
+            ovn_lflow_add(lflows, od, stage,
+                          acl->priority + OVN_ACL_PRI_OFFSET,
                           ds_cstr(&match), "ct_commit; next;");
 
             ds_destroy(&match);
         } else if (!strcmp(acl->action, "drop")) {
-            ovn_lflow_add(lflows, od, stage, acl->priority,
+            ovn_lflow_add(lflows, od, stage,
+                          acl->priority + OVN_ACL_PRI_OFFSET,
                           acl->match, "drop;");
         } else if (!strcmp(acl->action, "reject")) {
             /* xxx Need to support "reject". */
             VLOG_INFO("reject is not a supported action");
-            ovn_lflow_add(lflows, od, stage, acl->priority,
+            ovn_lflow_add(lflows, od, stage,
+                          acl->priority + OVN_ACL_PRI_OFFSET,
                           acl->match, "drop;");
         }
     }
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index d45a682..3921e98 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Northbound",
     "version": "2.0.0",
-    "cksum": "4186002454 4601",
+    "cksum": "3039293926 4601",
     "tables": {
         "Logical_Switch": {
             "columns": {
@@ -51,8 +51,8 @@ 
         "ACL": {
             "columns": {
                 "priority": {"type": {"key": {"type": "integer",
-                                              "minInteger": 1,
-                                              "maxInteger": 65534}}},
+                                              "minInteger": 0,
+                                              "maxInteger": 32767}}},
                 "direction": {"type": {"key": {"type": "string",
                                             "enum": ["set", ["from-lport", "to-lport"]]}}},
                 "match": {"type": "string"},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 0bfb587..b6eef03 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -332,7 +332,7 @@ 
       column="action"/> column for the highest-<ref column="priority"/>
       matching row in this table determines a packet's treatment.  If no row
       matches, packets are allowed by default.  (Default-deny treatment is
-      possible: add a rule with <ref column="priority"/> 1, <code>1</code> as
+      possible: add a rule with <ref column="priority"/> 0, <code>0</code> as
       <ref column="match"/>, and <code>deny</code> as <ref column="action"/>.)
     </p>
 
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index aac4c27..947c58c 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -947,8 +947,8 @@  nbctl_acl_add(struct ctl_context *ctx)
     }
 
     /* Validate priority. */
-    if (!ovs_scan(ctx->argv[3], "%"SCNd64, &priority) || priority < 1
-        || priority > 65535) {
+    if (!ovs_scan(ctx->argv[3], "%"SCNd64, &priority) || priority < 0
+        || priority > 32767) {
         VLOG_WARN("Invalid priority '%s'", ctx->argv[3]);
         return;
     }
@@ -1035,8 +1035,8 @@  nbctl_acl_del(struct ctl_context *ctx)
     }
 
     /* Validate priority. */
-    if (!ovs_scan(ctx->argv[3], "%"SCNd64, &priority) || priority < 1
-        || priority > 65535) {
+    if (!ovs_scan(ctx->argv[3], "%"SCNd64, &priority) || priority < 0
+        || priority > 32767) {
         VLOG_WARN("Invalid priority '%s'", ctx->argv[3]);
         return;
     }