From patchwork Mon Nov 16 15:49:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Flaviof X-Patchwork-Id: 1401001 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=flaviof.com Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CZYSV6WJ3z9sRK for ; Tue, 17 Nov 2020 02:50:34 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id B776D86794; Mon, 16 Nov 2020 15:50:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kBn-2ApZQmIu; Mon, 16 Nov 2020 15:50:17 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 2DBD28672B; Mon, 16 Nov 2020 15:50:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id F3984C1833; Mon, 16 Nov 2020 15:50:16 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 85417C0891 for ; Mon, 16 Nov 2020 15:50:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 72220867F0 for ; Mon, 16 Nov 2020 15:50:15 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1geSKm9nYm8h for ; Mon, 16 Nov 2020 15:50:11 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-qt1-f173.google.com (mail-qt1-f173.google.com [209.85.160.173]) by whitealder.osuosl.org (Postfix) with ESMTPS id A28E2866AE for ; Mon, 16 Nov 2020 15:50:11 +0000 (UTC) Received: by mail-qt1-f173.google.com with SMTP id b16so12925141qtb.6 for ; Mon, 16 Nov 2020 07:50:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=ONtdo3ybWOmVSEpD7UW/A8DIEsC6n4E1WUY2nTszMqI=; b=AIZcFRv1SRTTsdcFGTSA9sRkYoqy3Oi3qdTkRZJaYKUGqd2C+u9xMa1qzkgeqD6SJa sJ0lJ2TBJ3SLUuSlDJufjasMVl49eSpxNMG3efryAHO1JxPlcisyxAlbktKga1xw6D9J qc/WJ2ooj8PgtgXGTyCIgKZo80F/MkywsZ/NIRPL2/FRqzOwY1kvewO4fGc9N7kWBpTu CnMWoArcu8gKG32TO5HKGsx93RYFl9xPxO7GqXIAs21h8/5b0HvzIcdefXQb1BGL8S/x CAoDq8HCh/3TbjgW2+/rTsjy3lMTi8MpUbtQimc3u3Fnm9KBenxMLBbkyZ8C9gk+B3mo /QVQ== X-Gm-Message-State: AOAM532fRJtPxPqtw1gQtDcQZQ83uhNx0hlgnqYOT2YcPr4mHxx1BrFN VyX3NJ9P2DSjGlkkoRBRvC/N+7TX26xIyw== X-Google-Smtp-Source: ABdhPJwLYeg7NObbC9o8pFlCIxsOiTWDYd5BLMVppmc7p0E+YZY74Q1NuNemSHSn8aW5C58rbUzorA== X-Received: by 2002:ac8:65d3:: with SMTP id t19mr5046682qto.28.1605541809956; Mon, 16 Nov 2020 07:50:09 -0800 (PST) Received: from c8vm.redhat.com (pool-173-76-170-96.bstnma.fios.verizon.net. [173.76.170.96]) by smtp.gmail.com with ESMTPSA id y4sm11960196qtd.24.2020.11.16.07.50.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Nov 2020 07:50:09 -0800 (PST) From: Flavio Fernandes To: dev@openvswitch.org Date: Mon, 16 Nov 2020 15:49:57 +0000 Message-Id: <20201116154957.2443-3-flavio@flaviof.com> X-Mailer: git-send-email 2.18.4 In-Reply-To: <20201116154957.2443-1-flavio@flaviof.com> References: <20201116154957.2443-1-flavio@flaviof.com> Subject: [ovs-dev] [PATCH v5 ovn 2/2] northd: Fair ACL log meters. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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/20201107213913.GC2753733@ovn.org/ Co-authored-by: Ben Pfaff Signed-off-by: Flavio Fernandes --- 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] +AclFairLogMeters[meters] :- AclFairLogMeters0[meters]. +AclFairLogMeters[set_empty()] :- + Unit(), + not AclFairLogMeters0[_]. + +relation AclFairLogMeters0[Set] +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 = { 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])