Skip to content

Commit

Permalink
ovn: Reduce range of ACL priorities.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
Justin Pettit committed Oct 19, 2015
1 parent 5f8c05a commit 6bb4a18
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 12 deletions.
18 changes: 14 additions & 4 deletions ovn/northd/ovn-northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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;");
}
}
Expand Down
6 changes: 3 additions & 3 deletions ovn/ovn-nb.ovsschema
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
"version": "2.0.0",
"cksum": "4186002454 4601",
"cksum": "3039293926 4601",
"tables": {
"Logical_Switch": {
"columns": {
Expand Down Expand Up @@ -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"},
Expand Down
2 changes: 1 addition & 1 deletion ovn/ovn-nb.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
8 changes: 4 additions & 4 deletions ovn/utilities/ovn-nbctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 6bb4a18

Please sign in to comment.