ddlog for ACL log meters. Implement fair meters for acl logs in ovn_northd.dl. Enable fair Meter test to also exercise ovn-northd-ddlog.
This is a small variation of blp's implementation of ddlog for ACL log meters, now using the northbound schema changes [1]. The changes address overall design issues mentioned in that link. [1]: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Co-authored-by: Ben Pfaff <[email protected]> Signed-off-by: Flavio Fernandes <[email protected]> --- northd/ovn_northd.dl | 281 ++++++++++++++++++++++++------------------- tests/ovn-northd.at | 2 + 2 files changed, 161 insertions(+), 122 deletions(-) diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 3fbe67b31..46489e4cb 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -27,6 +27,23 @@ output relation Warning[string] index Logical_Flow_Index() on sb::Out_Logical_Flow() +/* Single-row relation that contains the set of meters marked as fair. */ +relation AclFairLogMeters[Set<string>] +AclFairLogMeters[meters] :- AclFairLogMeters0[meters]. +AclFairLogMeters[set_empty()] :- + Unit(), + not AclFairLogMeters0[_]. + +relation AclFairLogMeters0[Set<string>] +AclFairLogMeters0[meters] :- + meter in nb::Meter(.fair = Some{fair}), + fair, + var meters = { + var meters = set_empty(); + set_insert(meters, meter.name); + meters + }. + /* Meter_Band table */ for (mb in nb::Meter_Band) { sb::Out_Meter_Band(._uuid = mb._uuid, @@ -42,6 +59,14 @@ for (meter in nb::Meter) { .unit = meter.unit, .bands = meter.bands) } +sb::Out_Meter(._uuid = hash128(meter_uuid ++ acl_uuid), + .name = "${name}__${uuid2str(acl_uuid)}", + .unit = unit, + .bands = bands) :- + AclFairLogMeters[names], + var name = FlatMap(names), + nb::Meter(._uuid = meter_uuid, .name = name, .unit = unit, .bands = bands), + nb::ACL(._uuid = acl_uuid, .meter = Some{name}). /* Proxy table for Out_Datapath_Binding: contains all Datapath_Binding fields, * except tunnel id, which is allocated separately (see TunKeyAllocation). */ @@ -2017,7 +2042,7 @@ for (&Switch(.ls = ls)) { .external_ids = map_empty()) } -function build_acl_log(acl: nb::ACL): string = +function build_acl_log(acl: nb::ACL, fair_meters: Set<string>): string = { if (not acl.log) { "" @@ -2049,7 +2074,14 @@ function build_acl_log(acl: nb::ACL): string = }; match (acl.meter) { None -> (), - Some{meter} -> vec_push(strs, "meter=${json_string_escape(meter)}") + Some{meter} -> { + var s = if (fair_meters.contains(meter)) { + "${meter}__${uuid2str(acl._uuid)}" + } else { + meter + }; + vec_push(strs, "meter=${json_string_escape(s)}") + } }; "log(${string_join(strs, \", \")}); " } @@ -2067,31 +2099,33 @@ function oVN_ACL_PRI_OFFSET(): integer = 1000 relation Reject(lsuuid: uuid, pipeline: string, stage: Stage, acl: nb::ACL, extra_match: string, extra_actions: string) /* build_reject_acl_rules() */ -for (Reject(lsuuid, pipeline, stage, acl, extra_match_, extra_actions_)) { - var extra_match = match (extra_match_) { - "" -> "", - s -> "(${s}) && " - } in - var extra_actions = match (extra_actions_) { - "" -> "", - s -> "${s} " - } in - var next = match (pipeline == "ingress") { - true -> "next(pipeline=egress,table=${stage_id(switch_stage(OUT, QOS_MARK)).0})", - false -> "next(pipeline=ingress,table=${stage_id(switch_stage(IN, L2_LKUP)).0})" - } in - var acl_log = build_acl_log(acl) in { - var __match = extra_match ++ acl.__match in - var actions = acl_log ++ extra_actions ++ "reg0 = 0; " - "reject { " - "/* eth.dst <-> eth.src; ip.dst <-> ip.src; is implicit. */ " - "outport <-> inport; ${next}; };" in - Flow(.logical_datapath = lsuuid, - .stage = stage, - .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = __match, - .actions = actions, - .external_ids = stage_hint(acl._uuid)) +for (AclFairLogMeters[fair_meters]) { + for (Reject(lsuuid, pipeline, stage, acl, extra_match_, extra_actions_)) { + var extra_match = match (extra_match_) { + "" -> "", + s -> "(${s}) && " + } in + var extra_actions = match (extra_actions_) { + "" -> "", + s -> "${s} " + } in + var next = match (pipeline == "ingress") { + true -> "next(pipeline=egress,table=${stage_id(switch_stage(OUT, QOS_MARK)).0})", + false -> "next(pipeline=ingress,table=${stage_id(switch_stage(IN, L2_LKUP)).0})" + } in + var acl_log = build_acl_log(acl, fair_meters) in { + var __match = extra_match ++ acl.__match in + var actions = acl_log ++ extra_actions ++ "reg0 = 0; " + "reject { " + "/* eth.dst <-> eth.src; ip.dst <-> ip.src; is implicit. */ " + "outport <-> inport; ${next}; };" in + Flow(.logical_datapath = lsuuid, + .stage = stage, + .priority = acl.priority + oVN_ACL_PRI_OFFSET(), + .__match = __match, + .actions = actions, + .external_ids = stage_hint(acl._uuid)) + } } } @@ -2347,114 +2381,117 @@ for (&Switch(.ls = ls)) { } /* Ingress or Egress ACL Table (Various priorities). */ -for (&SwitchACL(.sw = &Switch{.ls = ls, .has_stateful_acl = has_stateful}, .acl = &acl)) { - /* consider_acl */ - var ingress = acl.direction == "from-lport" in - var stage = if (ingress) { switch_stage(IN, ACL) } else { switch_stage(OUT, ACL) } in - var pipeline = if ingress "ingress" else "egress" in - var stage_hint = stage_hint(acl._uuid) in - if (acl.action == "allow" or acl.action == "allow-related") { - /* If there are any stateful flows, we must even commit "allow" - * actions. This is because, while the initiater's - * direction may not have any stateful rules, the server's - * may and then its return traffic would not have an - * associated conntrack entry and would return "+invalid". */ - if (not has_stateful) { - Flow(.logical_datapath = ls._uuid, - .stage = stage, - .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = acl.__match, - .actions = "${build_acl_log(acl)}next;", - .external_ids = stage_hint) - } else { - /* Commit the connection tracking entry if it's a new - * connection that matches this ACL. After this commit, - * the reply traffic is allowed by a flow we create at - * priority 65535, defined earlier. - * - * It's also possible that a known connection was marked for - * deletion after a policy was deleted, but the policy was - * re-added while that connection is still known. We catch - * that case here and un-set ct_label.blocked (which will be done - * by ct_commit in the "stateful" stage) to indicate that the - * connection should be allowed to resume. - */ - Flow(.logical_datapath = ls._uuid, - .stage = stage, - .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = "${rEGBIT_ACL_HINT_ALLOW_NEW()} == 1 && (${acl.__match})", - .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${build_acl_log(acl)}next;", - .external_ids = stage_hint); - - /* Match on traffic in the request direction for an established - * connection tracking entry that has not been marked for - * deletion. There is no need to commit here, so we can just - * proceed to the next table. We use this to ensure that this - * connection is still allowed by the currently defined - * policy. Match untracked packets too. */ - Flow(.logical_datapath = ls._uuid, - .stage = stage, - .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = "${rEGBIT_ACL_HINT_ALLOW()} == 1 && (${acl.__match})", - .actions = "${build_acl_log(acl)}next;", - .external_ids = stage_hint) - } - } else if (acl.action == "drop" or acl.action == "reject") { - /* The implementation of "drop" differs if stateful ACLs are in - * use for this datapath. In that case, the actions differ - * depending on whether the connection was previously committed - * to the connection tracker with ct_commit. */ - if (has_stateful) { - /* If the packet is not tracked or not part of an established - * connection, then we can simply reject/drop it. */ - var __match = "${rEGBIT_ACL_HINT_DROP()} == 1" in - if (acl.action == "reject") { - Reject(ls._uuid, pipeline, stage, acl, __match, "") - } else { +for (AclFairLogMeters[fair_meters]) { + for (&SwitchACL(.sw = &Switch{.ls = ls, .has_stateful_acl = has_stateful}, .acl = &acl)) { + /* consider_acl */ + var ingress = acl.direction == "from-lport" in + var stage = if (ingress) { switch_stage(IN, ACL) } else { switch_stage(OUT, ACL) } in + var pipeline = if ingress "ingress" else "egress" in + var stage_hint = stage_hint(acl._uuid) in + var log_acl = build_acl_log(acl, fair_meters) in + if (acl.action == "allow" or acl.action == "allow-related") { + /* If there are any stateful flows, we must even commit "allow" + * actions. This is because, while the initiater's + * direction may not have any stateful rules, the server's + * may and then its return traffic would not have an + * associated conntrack entry and would return "+invalid". */ + if (not has_stateful) { Flow(.logical_datapath = ls._uuid, .stage = stage, .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = __match ++ " && (${acl.__match})", - .actions = "${build_acl_log(acl)}/* drop */", + .__match = acl.__match, + .actions = "${log_acl}next;", .external_ids = stage_hint) - }; - /* For an existing connection without ct_label set, we've - * encountered a policy change. ACLs previously allowed - * this connection and we committed the connection tracking - * entry. Current policy says that we should drop this - * connection. First, we set bit 0 of ct_label to indicate - * that this connection is set for deletion. By not - * specifying "next;", we implicitly drop the packet after - * updating conntrack state. We would normally defer - * ct_commit() to the "stateful" stage, but since we're - * rejecting/dropping the packet, we go ahead and do it here. - */ - var __match = "${rEGBIT_ACL_HINT_BLOCK()} == 1" in - var actions = "ct_commit { ct_label.blocked = 1; }; " in - if (acl.action == "reject") { - Reject(ls._uuid, pipeline, stage, acl, __match, actions) } else { + /* Commit the connection tracking entry if it's a new + * connection that matches this ACL. After this commit, + * the reply traffic is allowed by a flow we create at + * priority 65535, defined earlier. + * + * It's also possible that a known connection was marked for + * deletion after a policy was deleted, but the policy was + * re-added while that connection is still known. We catch + * that case here and un-set ct_label.blocked (which will be done + * by ct_commit in the "stateful" stage) to indicate that the + * connection should be allowed to resume. + */ Flow(.logical_datapath = ls._uuid, .stage = stage, .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = __match ++ " && (${acl.__match})", - .actions = "${actions}${build_acl_log(acl)}/* drop */", - .external_ids = stage_hint) - } - } else { - /* There are no stateful ACLs in use on this datapath, - * so a "reject/drop" ACL is simply the "reject/drop" - * logical flow action in all cases. */ - if (acl.action == "reject") { - Reject(ls._uuid, pipeline, stage, acl, "", "") - } else { + .__match = "${rEGBIT_ACL_HINT_ALLOW_NEW()} == 1 && (${acl.__match})", + .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${log_acl}next;", + .external_ids = stage_hint); + + /* Match on traffic in the request direction for an established + * connection tracking entry that has not been marked for + * deletion. There is no need to commit here, so we can just + * proceed to the next table. We use this to ensure that this + * connection is still allowed by the currently defined + * policy. Match untracked packets too. */ Flow(.logical_datapath = ls._uuid, .stage = stage, .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = acl.__match, - .actions = "${build_acl_log(acl)}/* drop */", + .__match = "${rEGBIT_ACL_HINT_ALLOW()} == 1 && (${acl.__match})", + .actions = "${log_acl}next;", .external_ids = stage_hint) } + } else if (acl.action == "drop" or acl.action == "reject") { + /* The implementation of "drop" differs if stateful ACLs are in + * use for this datapath. In that case, the actions differ + * depending on whether the connection was previously committed + * to the connection tracker with ct_commit. */ + if (has_stateful) { + /* If the packet is not tracked or not part of an established + * connection, then we can simply reject/drop it. */ + var __match = "${rEGBIT_ACL_HINT_DROP()} == 1" in + if (acl.action == "reject") { + Reject(ls._uuid, pipeline, stage, acl, __match, "") + } else { + Flow(.logical_datapath = ls._uuid, + .stage = stage, + .priority = acl.priority + oVN_ACL_PRI_OFFSET(), + .__match = __match ++ " && (${acl.__match})", + .actions = "${log_acl}/* drop */", + .external_ids = stage_hint) + }; + /* For an existing connection without ct_label set, we've + * encountered a policy change. ACLs previously allowed + * this connection and we committed the connection tracking + * entry. Current policy says that we should drop this + * connection. First, we set bit 0 of ct_label to indicate + * that this connection is set for deletion. By not + * specifying "next;", we implicitly drop the packet after + * updating conntrack state. We would normally defer + * ct_commit() to the "stateful" stage, but since we're + * rejecting/dropping the packet, we go ahead and do it here. + */ + var __match = "${rEGBIT_ACL_HINT_BLOCK()} == 1" in + var actions = "ct_commit { ct_label.blocked = 1; }; " in + if (acl.action == "reject") { + Reject(ls._uuid, pipeline, stage, acl, __match, actions) + } else { + Flow(.logical_datapath = ls._uuid, + .stage = stage, + .priority = acl.priority + oVN_ACL_PRI_OFFSET(), + .__match = __match ++ " && (${acl.__match})", + .actions = "${actions}${log_acl}/* drop */", + .external_ids = stage_hint) + } + } else { + /* There are no stateful ACLs in use on this datapath, + * so a "reject/drop" ACL is simply the "reject/drop" + * logical flow action in all cases. */ + if (acl.action == "reject") { + Reject(ls._uuid, pipeline, stage, acl, "", "") + } else { + Flow(.logical_datapath = ls._uuid, + .stage = stage, + .priority = acl.priority + oVN_ACL_PRI_OFFSET(), + .__match = acl.__match, + .actions = "${log_acl}/* drop */", + .external_ids = stage_hint) + } + } } } } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 21ce20c87..ee17b13e2 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1920,6 +1920,7 @@ sw1flows3: table=5 (ls_out_acl ), priority=2003 , match=((reg0[[9]] == AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn-northd -- ACL fair Meters]) AT_KEYWORDS([acl log meter fair]) ovn_start @@ -2016,6 +2017,7 @@ check_meter_by_name meter_me check_meter_by_name NOT meter_me__${acl1} meter_me__${acl2} AT_CLEANUP +]) OVN_FOR_EACH_NORTHD([ AT_SETUP([datapath requested-tnl-key]) -- 2.18.4 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
