@@ -26,7 +26,7 @@ Debugging the DDlog version of ovn-northd
=========================================
This document gives some tips for debugging correctness issues in the
-DDlog implementation of ``ovn-northd``. To keep things conrete, we
+DDlog implementation of ``ovn-northd``. To keep things concrete, we
assume here that a failure occurred in one of the test cases in
``ovn-e2e.at``, but the same methodology applies in any other
environment. If none of these methods helps, ask for assistance or
@@ -22,8 +22,7 @@ bin_PROGRAMS += northd/ovn-northd-ddlog
northd_ovn_northd_ddlog_SOURCES = \
northd/ovn-northd-ddlog.c \
northd/ovn-northd-ddlog-sb.inc \
- northd/ovn-northd-ddlog-nb.inc \
- northd/ovn_northd_ddlog/ddlog.h
+ northd/ovn-northd-ddlog-nb.inc
northd_ovn_northd_ddlog_LDADD = \
northd/ovn_northd_ddlog/target/release/libovn_northd_ddlog.la \
lib/libovn.la \
@@ -46,6 +45,7 @@ BUILT_SOURCES += \
northd/ovn-northd-ddlog-sb.inc \
northd/ovn-northd-ddlog-nb.inc
+northd/ovn-northd-ddlog.$(OBJEXT): northd/ovn_northd_ddlog/ddlog.h
northd/ovn_northd_ddlog/ddlog.h: northd/ddlog.stamp
CARGO_VERBOSE = $(cargo_verbose_$(V))
@@ -101,18 +101,16 @@ LogicalSwitchHasStatefulACL(ls, false) :-
nb::Logical_Switch(._uuid = ls),
not LogicalSwitchStatefulACL(ls, _).
-relation LogicalSwitchLocalnetPort0(ls_uuid: uuid, lsp_name: string)
-LogicalSwitchLocalnetPort0(ls_uuid, lsp_name) :-
+relation LogicalSwitchLocalnetPort0(ls_uuid: uuid, lsp: nb::Logical_Switch_Port)
+LogicalSwitchLocalnetPort0(ls_uuid, lsp) :-
ls in nb::Logical_Switch(._uuid = ls_uuid),
var lsp_uuid = FlatMap(ls.ports),
- lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
- lsp.__type == "localnet",
- var lsp_name = lsp.name.
+ lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid, .__type = "localnet").
-relation LogicalSwitchLocalnetPorts(ls_uuid: uuid, localnet_port_names: Vec<string>)
-LogicalSwitchLocalnetPorts(ls_uuid, localnet_port_names) :-
+relation LogicalSwitchLocalnetPorts(ls_uuid: uuid, localnet_ports: Vec<nb::Logical_Switch_Port>)
+LogicalSwitchLocalnetPorts(ls_uuid, localnet_ports) :-
LogicalSwitchLocalnetPort0(ls_uuid, lsp_name),
- var localnet_port_names = lsp_name.group_by(ls_uuid).to_vec().
+ var localnet_ports = lsp_name.group_by(ls_uuid).to_vec().
LogicalSwitchLocalnetPorts(ls_uuid, vec_empty()) :-
ls in nb::Logical_Switch(),
var ls_uuid = ls._uuid,
@@ -163,7 +161,7 @@ relation &Switch(
has_stateful_acl: bool,
has_lb_vip: bool,
has_dns_records: bool,
- localnet_port_names: Vec<string>,
+ localnet_ports: Vec<nb::Logical_Switch_Port>,
subnet: Option<(in_addr/*subnet*/, in_addr/*mask*/, bit<32>/*start_ipv4*/, bit<32>/*total_ipv4s*/)>,
ipv6_prefix: Option<in6_addr>,
mcast_cfg: Ref<McastSwitchCfg>,
@@ -187,7 +185,7 @@ function ipv6_parse_prefix(s: string): Option<in6_addr> {
.has_stateful_acl = has_stateful_acl,
.has_lb_vip = has_lb_vip,
.has_dns_records = has_dns_records,
- .localnet_port_names = localnet_port_names,
+ .localnet_ports = localnet_ports,
.subnet = subnet,
.ipv6_prefix = ipv6_prefix,
.mcast_cfg = mcast_cfg,
@@ -196,7 +194,7 @@ function ipv6_parse_prefix(s: string): Option<in6_addr> {
LogicalSwitchHasStatefulACL(ls._uuid, has_stateful_acl),
LogicalSwitchHasLBVIP(ls._uuid, has_lb_vip),
LogicalSwitchHasDNSRecords(ls._uuid, has_dns_records),
- LogicalSwitchLocalnetPorts(ls._uuid, localnet_port_names),
+ LogicalSwitchLocalnetPorts(ls._uuid, localnet_ports),
LogicalSwitchHasNonRouterPort(ls._uuid, has_non_router_port),
mcast_cfg in &McastSwitchCfg(.datapath = ls._uuid),
var subnet =
@@ -208,6 +208,10 @@ IgmpSwitchGroupPort(address, switch, lsp_uuid) :-
.lsp = nb::Logical_Switch_Port{._uuid = lsp_uuid, .name = lsp_name},
.sw = switch).
+IgmpSwitchGroupPort(address, switch, localnet_port._uuid) :-
+ IgmpSwitchGroupPort(address, switch, _),
+ var localnet_port = FlatMap(switch.localnet_ports).
+
/* Aggregated IGMP group: merges all IgmpSwitchGroupPort for a given
* address-switch tuple from all chassis.
*/
@@ -5356,8 +5356,53 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows)
}
}
+static bool
+is_a_shared_meter(const struct smap *nb_options, const char *meter_name)
+{
+ bool is_shared_meter = false;
+ const char *meters = smap_get(nb_options, "acl_shared_log_meters");
+ if (meters && meters[0]) {
+ char *cur, *next, *start;
+ next = start = xstrdup(meters);
+ while ((cur = strsep(&next, ",")) && *cur) {
+ if (!strcmp(meter_name, cur)) {
+ is_shared_meter = true;
+ break;
+ }
+ }
+ free(start);
+ }
+ return is_shared_meter;
+}
+
+static char*
+alloc_acl_log_unique_meter_name(const struct nbrec_acl *acl)
+{
+ return xasprintf("%s__" UUID_FMT,
+ acl->meter, UUID_ARGS(&acl->header_.uuid));
+}
+
+static void
+build_acl_log_meter(struct ds *actions, const struct nbrec_acl *acl,
+ const struct smap *nb_options)
+{
+ if (!acl->meter) {
+ return;
+ }
+
+ /* If ACL log meter uses a shared meter, use unique Meter name. */
+ if (is_a_shared_meter(nb_options, acl->meter)) {
+ char *meter_name = alloc_acl_log_unique_meter_name(acl);
+ ds_put_format(actions, "meter=\"%s\", ", meter_name);
+ free(meter_name);
+ } else {
+ ds_put_format(actions, "meter=\"%s\", ", acl->meter);
+ }
+}
+
static void
-build_acl_log(struct ds *actions, const struct nbrec_acl *acl)
+build_acl_log(struct ds *actions, const struct nbrec_acl *acl,
+ const struct smap *nb_options)
{
if (!acl->log) {
return;
@@ -5385,9 +5430,7 @@ build_acl_log(struct ds *actions, const struct nbrec_acl *acl)
ds_put_cstr(actions, "verdict=allow, ");
}
- if (acl->meter) {
- ds_put_format(actions, "meter=\"%s\", ", acl->meter);
- }
+ build_acl_log_meter(actions, acl, nb_options);
ds_chomp(actions, ' ');
ds_chomp(actions, ',');
@@ -5398,7 +5441,8 @@ static void
build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
enum ovn_stage stage, struct nbrec_acl *acl,
struct ds *extra_match, struct ds *extra_actions,
- const struct ovsdb_idl_row *stage_hint)
+ const struct ovsdb_idl_row *stage_hint,
+ const struct smap *nb_options)
{
struct ds match = DS_EMPTY_INITIALIZER;
struct ds actions = DS_EMPTY_INITIALIZER;
@@ -5410,7 +5454,7 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
ingress ? ovn_stage_get_table(S_SWITCH_OUT_QOS_MARK)
: ovn_stage_get_table(S_SWITCH_IN_L2_LKUP));
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
if (extra_match->length > 0) {
ds_put_format(&match, "(%s) && ", extra_match->string);
}
@@ -5435,7 +5479,8 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
static void
consider_acl(struct hmap *lflows, struct ovn_datapath *od,
- struct nbrec_acl *acl, bool has_stateful)
+ struct nbrec_acl *acl, bool has_stateful,
+ const struct smap *nb_options)
{
bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL : S_SWITCH_OUT_ACL;
@@ -5449,7 +5494,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
* associated conntrack entry and would return "+invalid". */
if (!has_stateful) {
struct ds actions = DS_EMPTY_INITIALIZER;
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
ds_put_cstr(&actions, "next;");
ovn_lflow_add_with_hint(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
@@ -5475,7 +5520,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
ds_put_format(&match, REGBIT_ACL_HINT_ALLOW_NEW " == 1 && (%s)",
acl->match);
ds_put_cstr(&actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
ds_put_cstr(&actions, "next;");
ovn_lflow_add_with_hint(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
@@ -5494,7 +5539,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
ds_put_format(&match, REGBIT_ACL_HINT_ALLOW " == 1 && (%s)",
acl->match);
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
ds_put_cstr(&actions, "next;");
ovn_lflow_add_with_hint(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
@@ -5519,10 +5564,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
ds_put_cstr(&match, REGBIT_ACL_HINT_DROP " == 1");
if (!strcmp(acl->action, "reject")) {
build_reject_acl_rules(od, lflows, stage, acl, &match,
- &actions, &acl->header_);
+ &actions, &acl->header_, nb_options);
} else {
ds_put_format(&match, " && (%s)", acl->match);
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
ds_put_cstr(&actions, "/* drop */");
ovn_lflow_add_with_hint(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
@@ -5546,10 +5591,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
ds_put_cstr(&actions, "ct_commit { ct_label.blocked = 1; }; ");
if (!strcmp(acl->action, "reject")) {
build_reject_acl_rules(od, lflows, stage, acl, &match,
- &actions, &acl->header_);
+ &actions, &acl->header_, nb_options);
} else {
ds_put_format(&match, " && (%s)", acl->match);
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
ds_put_cstr(&actions, "/* drop */");
ovn_lflow_add_with_hint(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
@@ -5562,9 +5607,9 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
* logical flow action in all cases. */
if (!strcmp(acl->action, "reject")) {
build_reject_acl_rules(od, lflows, stage, acl, &match,
- &actions, &acl->header_);
+ &actions, &acl->header_, nb_options);
} else {
- build_acl_log(&actions, acl);
+ build_acl_log(&actions, acl, nb_options);
ds_put_cstr(&actions, "/* drop */");
ovn_lflow_add_with_hint(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
@@ -5644,7 +5689,7 @@ build_port_group_lswitches(struct northd_context *ctx, struct hmap *pgs,
static void
build_acls(struct ovn_datapath *od, struct hmap *lflows,
- struct hmap *port_groups)
+ struct hmap *port_groups, const struct smap *nb_options)
{
bool has_stateful = (has_stateful_acl(od) || has_lb_vip(od));
@@ -5747,13 +5792,14 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
/* Ingress or Egress ACL Table (Various priorities). */
for (size_t i = 0; i < od->nbs->n_acls; i++) {
struct nbrec_acl *acl = od->nbs->acls[i];
- consider_acl(lflows, od, acl, has_stateful);
+ consider_acl(lflows, od, acl, has_stateful, nb_options);
}
struct ovn_port_group *pg;
HMAP_FOR_EACH (pg, key_node, port_groups) {
if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
- consider_acl(lflows, od, pg->nb_pg->acls[i], has_stateful);
+ consider_acl(lflows, od, pg->nb_pg->acls[i], has_stateful,
+ nb_options);
}
}
}
@@ -6776,7 +6822,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
struct hmap *port_groups, struct hmap *lflows,
struct hmap *mcgroups, struct hmap *igmp_groups,
struct shash *meter_groups,
- struct hmap *lbs)
+ struct hmap *lbs, const struct smap *nb_options)
{
/* This flow table structure is documented in ovn-northd(8), so please
* update ovn-northd.8.xml if you change anything. */
@@ -6796,7 +6842,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
build_pre_lb(od, lflows, meter_groups, lbs);
build_pre_stateful(od, lflows);
build_acl_hints(od, lflows);
- build_acls(od, lflows, port_groups);
+ build_acls(od, lflows, port_groups, nb_options);
build_qos(od, lflows);
build_lb(od, lflows);
build_stateful(od, lflows, lbs);
@@ -11379,8 +11425,12 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
{
struct hmap lflows = HMAP_INITIALIZER(&lflows);
+ const struct nbrec_nb_global *nb_global =
+ nbrec_nb_global_first(ctx->ovnnb_idl);
+ ovs_assert(nb_global);
+
build_lswitch_flows(datapaths, ports, port_groups, &lflows, mcgroups,
- igmp_groups, meter_groups, lbs);
+ igmp_groups, meter_groups, lbs, &nb_global->options);
build_lrouter_flows(datapaths, ports, &lflows, meter_groups, lbs);
/* Push changes to the Logical_Flow table to database. */
@@ -11706,15 +11756,85 @@ done:
return need_update;
}
+static void
+sync_meters_iterate_nb_meter(struct northd_context *ctx,
+ const char *meter_name,
+ const struct nbrec_meter *nb_meter,
+ struct shash *sb_meters)
+{
+ bool new_sb_meter = false;
+
+ const struct sbrec_meter *sb_meter = shash_find_and_delete(sb_meters,
+ meter_name);
+ if (!sb_meter) {
+ sb_meter = sbrec_meter_insert(ctx->ovnsb_txn);
+ sbrec_meter_set_name(sb_meter, meter_name);
+ new_sb_meter = true;
+ }
+
+ if (new_sb_meter || bands_need_update(nb_meter, sb_meter)) {
+ struct sbrec_meter_band **sb_bands;
+ sb_bands = xcalloc(nb_meter->n_bands, sizeof *sb_bands);
+ for (size_t i = 0; i < nb_meter->n_bands; i++) {
+ const struct nbrec_meter_band *nb_band = nb_meter->bands[i];
+
+ sb_bands[i] = sbrec_meter_band_insert(ctx->ovnsb_txn);
+
+ sbrec_meter_band_set_action(sb_bands[i], nb_band->action);
+ sbrec_meter_band_set_rate(sb_bands[i], nb_band->rate);
+ sbrec_meter_band_set_burst_size(sb_bands[i],
+ nb_band->burst_size);
+ }
+ sbrec_meter_set_bands(sb_meter, sb_bands, nb_meter->n_bands);
+ free(sb_bands);
+ }
+
+ sbrec_meter_set_unit(sb_meter, nb_meter->unit);
+}
+
+static void
+sync_acl_shared_meters(struct northd_context *ctx,
+ struct hmap *datapaths,
+ const struct smap *nb_options,
+ const struct nbrec_meter *nb_meter,
+ struct shash *sb_meters)
+{
+ if (!is_a_shared_meter(nb_options, nb_meter->name)) {
+ return;
+ }
+
+ struct ovn_datapath *od;
+ HMAP_FOR_EACH (od, key_node, datapaths) {
+ if (!od->nbs) {
+ continue;
+ }
+ for (size_t i = 0; i < od->nbs->n_acls; i++) {
+ struct nbrec_acl *acl = od->nbs->acls[i];
+ if (!acl->meter || strcmp(nb_meter->name, acl->meter)) {
+ continue;
+ }
+
+ char *meter_name = alloc_acl_log_unique_meter_name(acl);
+ sync_meters_iterate_nb_meter(ctx, meter_name, nb_meter, sb_meters);
+ free(meter_name);
+ }
+ }
+}
+
/* Each entry in the Meter and Meter_Band tables in OVN_Northbound have
* a corresponding entries in the Meter and Meter_Band tables in
- * OVN_Southbound.
+ * OVN_Southbound. Additionally, ACL logs that use shared meters have
+ * a private copy of its meter in the SB table.
*/
static void
-sync_meters(struct northd_context *ctx)
+sync_meters(struct northd_context *ctx, struct hmap *datapaths)
{
struct shash sb_meters = SHASH_INITIALIZER(&sb_meters);
+ const struct nbrec_nb_global *nb_global =
+ nbrec_nb_global_first(ctx->ovnnb_idl);
+ ovs_assert(nb_global);
+
const struct sbrec_meter *sb_meter;
SBREC_METER_FOR_EACH (sb_meter, ctx->ovnsb_idl) {
shash_add(&sb_meters, sb_meter->name, sb_meter);
@@ -11722,33 +11842,10 @@ sync_meters(struct northd_context *ctx)
const struct nbrec_meter *nb_meter;
NBREC_METER_FOR_EACH (nb_meter, ctx->ovnnb_idl) {
- bool new_sb_meter = false;
-
- sb_meter = shash_find_and_delete(&sb_meters, nb_meter->name);
- if (!sb_meter) {
- sb_meter = sbrec_meter_insert(ctx->ovnsb_txn);
- sbrec_meter_set_name(sb_meter, nb_meter->name);
- new_sb_meter = true;
- }
-
- if (new_sb_meter || bands_need_update(nb_meter, sb_meter)) {
- struct sbrec_meter_band **sb_bands;
- sb_bands = xcalloc(nb_meter->n_bands, sizeof *sb_bands);
- for (size_t i = 0; i < nb_meter->n_bands; i++) {
- const struct nbrec_meter_band *nb_band = nb_meter->bands[i];
-
- sb_bands[i] = sbrec_meter_band_insert(ctx->ovnsb_txn);
-
- sbrec_meter_band_set_action(sb_bands[i], nb_band->action);
- sbrec_meter_band_set_rate(sb_bands[i], nb_band->rate);
- sbrec_meter_band_set_burst_size(sb_bands[i],
- nb_band->burst_size);
- }
- sbrec_meter_set_bands(sb_meter, sb_bands, nb_meter->n_bands);
- free(sb_bands);
- }
-
- sbrec_meter_set_unit(sb_meter, nb_meter->unit);
+ sync_meters_iterate_nb_meter(ctx, nb_meter->name, nb_meter,
+ &sb_meters);
+ sync_acl_shared_meters(ctx, datapaths, &nb_global->options, nb_meter,
+ &sb_meters);
}
struct shash_node *node, *next;
@@ -12241,7 +12338,7 @@ ovnnb_db_run(struct northd_context *ctx,
sync_address_sets(ctx);
sync_port_groups(ctx, &port_groups);
- sync_meters(ctx);
+ sync_meters(ctx, datapaths);
sync_dns_entries(ctx, datapaths);
destroy_ovn_lbs(&lbs);
hmap_destroy(&lbs);
@@ -27,6 +27,19 @@ output relation Warning[string]
index Logical_Flow_Index() on sb::Out_Logical_Flow()
+/* Single-row relation that contains the set of shared meters. */
+relation AclSharedLogMeters[Set<string>]
+AclSharedLogMeters[meters] :- AclSharedLogMeters0[meters].
+AclSharedLogMeters[set_empty()] :-
+ Unit(),
+ not AclSharedLogMeters0[_].
+
+relation AclSharedLogMeters0[Set<string>]
+AclSharedLogMeters0[meters] :-
+ nb in nb::NB_Global(),
+ Some{var s} = nb.options.get("acl_shared_log_meters"),
+ var meters = s.split(",").to_set().
+
/* Meter_Band table */
for (mb in nb::Meter_Band) {
sb::Out_Meter_Band(._uuid = mb._uuid,
@@ -42,6 +55,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) :-
+ AclSharedLogMeters[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). */
@@ -222,7 +243,7 @@ OutProxy_Port_Binding(._uuid = lsp._uuid,
and is_some(l3dgw_port)) or
Some{rport.lrp} == l3dgw_port or
(is_some(map_get(rport.router.lr.options, "chassis")) and
- not sw.localnet_port_names.is_empty())) {
+ not sw.localnet_ports.is_empty())) {
false -> set_empty(),
true -> set_singleton(get_garp_nat_addresses(deref(rport)))
},
@@ -2008,7 +2029,7 @@ for (&Switch(.ls = ls)) {
.external_ids = map_empty())
}
-function build_acl_log(acl: nb::ACL): string =
+function build_acl_log(acl: nb::ACL, shared_meters: Set<string>): string =
{
if (not acl.log) {
""
@@ -2040,7 +2061,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 (shared_meters.contains(meter)) {
+ "${meter}__${uuid2str(acl._uuid)}"
+ } else {
+ meter
+ };
+ vec_push(strs, "meter=${json_string_escape(s)}")
+ }
};
"log(${string_join(strs, \", \")}); "
}
@@ -2058,31 +2086,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 (AclSharedLogMeters[shared_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, shared_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))
+ }
}
}
@@ -2338,114 +2368,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 (AclSharedLogMeters[shared_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, shared_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)
+ }
+ }
}
}
}
@@ -3432,8 +3465,12 @@ function json_string_escape_vec(names: Vec<string>): string
*/
function match_dhcp_input(lsp: Ref<SwitchPort>): (string, string) =
{
- if (lsp.lsp.__type == "external" and not lsp.sw.localnet_port_names.is_empty()) {
- ("inport == " ++ json_string_escape_vec(lsp.sw.localnet_port_names) ++ " && ",
+ if (lsp.lsp.__type == "external" and not lsp.sw.localnet_ports.is_empty()) {
+ var localnet_port_names = vec_with_capacity(lsp.sw.localnet_ports.len());
+ for (localnet_port in lsp.sw.localnet_ports) {
+ localnet_port_names.push(localnet_port.name)
+ };
+ ("inport == " ++ json_string_escape_vec(localnet_port_names) ++ " && ",
" && is_chassis_resident(${lsp.json_name})")
} else {
("inport == ${lsp.json_name} && ", "")
@@ -3449,7 +3486,7 @@ for (lsp in &SwitchPort
/* If it's an external port and there is no localnet port
* and if it doesn't belong to an HA chassis group ignore it. */
and (lsp.lsp.__type != "external"
- or (not lsp.sw.localnet_port_names.is_empty()
+ or (not lsp.sw.localnet_ports.is_empty()
and is_some(lsp.lsp.ha_chassis_group))))
{
for (lps in LogicalSwitchPort(.lport = lsp.lsp._uuid, .lswitch = lsuuid)) {
@@ -3805,7 +3842,7 @@ for (IgmpSwitchMulticastGroup(.address = address, .switch = &sw)) {
Flow(.logical_datapath = sp.sw.ls._uuid,
.stage = switch_stage(IN, EXTERNAL_PORT),
.priority = 100,
- .__match = ("inport == ${json_string_escape(localnet_port_name)} && "
+ .__match = ("inport == ${json_string_escape(localnet_port.name)} && "
"eth.src == ${lp_addr.ea} && "
"!is_chassis_resident(${sp.json_name}) && "
"arp.tpa == ${rp_addr.addr} && arp.op == 1"),
@@ -3813,7 +3850,7 @@ Flow(.logical_datapath = sp.sw.ls._uuid,
.external_ids = stage_hint(sp.lsp._uuid)) :-
sp in &SwitchPort(),
sp.lsp.__type == "external",
- var localnet_port_name = FlatMap(sp.sw.localnet_port_names),
+ var localnet_port = FlatMap(sp.sw.localnet_ports),
var lp_addr = FlatMap(sp.static_addresses),
rp in &SwitchPort(.sw = sp.sw),
rp.lsp.__type == "router",
@@ -3821,7 +3858,7 @@ Flow(.logical_datapath = sp.sw.ls._uuid,
Flow(.logical_datapath = sp.sw.ls._uuid,
.stage = switch_stage(IN, EXTERNAL_PORT),
.priority = 100,
- .__match = ("inport == ${json_string_escape(localnet_port_name)} && "
+ .__match = ("inport == ${json_string_escape(localnet_port.name)} && "
"eth.src == ${lp_addr.ea} && "
"!is_chassis_resident(${sp.json_name}) && "
"nd_ns && ip6.dst == {${rp_addr.addr}, ${ipv6_netaddr_solicited_node(rp_addr)}} && "
@@ -3830,7 +3867,7 @@ Flow(.logical_datapath = sp.sw.ls._uuid,
.external_ids = stage_hint(sp.lsp._uuid)) :-
sp in &SwitchPort(),
sp.lsp.__type == "external",
- var localnet_port_name = FlatMap(sp.sw.localnet_port_names),
+ var localnet_port = FlatMap(sp.sw.localnet_ports),
var lp_addr = FlatMap(sp.static_addresses),
rp in &SwitchPort(.sw = sp.sw),
rp.lsp.__type == "router",
@@ -3838,7 +3875,7 @@ Flow(.logical_datapath = sp.sw.ls._uuid,
Flow(.logical_datapath = sp.sw.ls._uuid,
.stage = switch_stage(IN, EXTERNAL_PORT),
.priority = 100,
- .__match = ("inport == ${json_string_escape(localnet_port_name)} && "
+ .__match = ("inport == ${json_string_escape(localnet_port.name)} && "
"eth.src == ${lp_addr.ea} && "
"eth.dst == ${ea} && "
"!is_chassis_resident(${sp.json_name})"),
@@ -3846,7 +3883,7 @@ Flow(.logical_datapath = sp.sw.ls._uuid,
.external_ids = stage_hint(sp.lsp._uuid)) :-
sp in &SwitchPort(),
sp.lsp.__type == "external",
- var localnet_port_name = FlatMap(sp.sw.localnet_port_names),
+ var localnet_port = FlatMap(sp.sw.localnet_ports),
var lp_addr = FlatMap(sp.static_addresses),
rp in &SwitchPort(.sw = sp.sw),
rp.lsp.__type == "router",
@@ -4057,7 +4094,7 @@ for (&SwitchPort(.lsp = lsp,
{
Some{var mac} = scan_eth_addr(lrp.mac) in {
var add_chassis_resident_check =
- not sw.localnet_port_names.is_empty() and
+ not sw.localnet_ports.is_empty() and
(/* The peer of this port represents a distributed
* gateway port. The destination lookup flow for the
* router's distributed gateway port MAC address should
@@ -4574,7 +4611,7 @@ AddChassisResidentCheck_(lrp._uuid, res) :-
&SwitchPort(.peer = Some{&RouterPort{.lrp = lrp, .router = &router, .is_redirect = is_redirect}},
.sw = sw),
is_some(router.l3dgw_port),
- not sw.localnet_port_names.is_empty(),
+ not sw.localnet_ports.is_empty(),
var res = if (is_redirect) {
/* Traffic with eth.src = l3dgw_port->lrp_networks.ea
* should only be sent from the "redirect-chassis", so that
@@ -206,6 +206,20 @@
</p>
</column>
+ <column name="options" key="acl_shared_log_meters">
+ <p>
+ A string value that contains a list of meter names delimited by ",".
+ The <ref column="meter" table="ACL"/> column is used by ACL to rate
+ limit log events. As multiple ACL rows may opt to use the same meter
+ name, a potentially adverse effect is that a "noisy" ACL log could
+ consume all the allowable events for the shared
+ <ref column="name" table="meter"/>. This global option can be used to
+ eliminate that behavior, by listing the meters that are meant to
+ provide its "complete" rate to each one of the <ref table="ACL"/>
+ logs that use it.
+ </p>
+ </column>
+
<group title="Options for configuring interconnection route advertisement">
<p>
These options control how routes are advertised between OVN
@@ -1920,6 +1920,105 @@ sw1flows3: table=5 (ls_out_acl ), priority=2003 , match=((reg0[[9]] ==
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-northd -- ACL shared Meter])
+AT_KEYWORDS([acl meter])
+ovn_start
+
+ovn-nbctl ls-add sw0
+ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 "50:54:00:00:00:01 10.0.0.11"
+ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 "50:54:00:00:00:02 10.0.0.12"
+ovn-nbctl lsp-add sw0 sw0-p3 -- lsp-set-addresses sw0-p3 "50:54:00:00:00:03 10.0.0.13"
+
+ovn-nbctl remove nb_global . options acl_shared_log_meters
+ovn-nbctl meter-add meter_me drop 1 pktps
+
+ovn-nbctl acl-add sw0 to-lport 1002 'outport == "sw0-p1" && ip4.src == 10.0.0.12' allow
+ovn-nbctl acl-add sw0 to-lport 1002 'outport == "sw0-p1" && ip4.src == 10.0.0.13' allow
+
+acl1=$(ovn-nbctl --bare --column _uuid,match find acl | grep -B1 '10.0.0.12' | head -1)
+acl2=$(ovn-nbctl --bare --column _uuid,match find acl | grep -B1 '10.0.0.13' | head -1)
+ovn-nbctl set acl $acl1 log=true severity=alert meter=meter_me name=acl_one
+ovn-nbctl set acl $acl2 log=true severity=info meter=meter_me name=acl_two
+ovn-nbctl --wait=sb sync
+
+check_row_count nb:meter 1
+check_column meter_me nb:meter name
+
+check_acl_lflow() {
+ acl_log_name=$1
+ meter_name=$2
+ # echo checking that logical flow for acl log $acl_log_name has $meter_name
+ AT_CHECK([ovn-sbctl lflow-list | grep ls_out_acl | \
+ grep "\"${acl_log_name}\"" | \
+ grep -c "meter=\"${meter_name}\""], [0], [1
+])
+}
+
+check_meter_by_name() {
+ [test "$1" = "NOT"] && { expected_count=0; shift; } || expected_count=1
+ for meter in $* ; do
+ # echo checking for $expected_count $meter in sb meter table
+ check_row_count meter $expected_count name=$meter
+ done
+}
+
+# With acl_shared_log_meters unset, make sure no unique meters are in SB
+check_meter_by_name meter_me
+check_meter_by_name NOT meter_me__${acl1} meter_me__${acl2}
+for other_meters in 'm1' 'm1,m2,m3' 'meter_Xe'; do
+ ovn-nbctl --wait=sb set nb_global . options:acl_shared_log_meters="$other_meters"
+ check_meter_by_name meter_me
+ check_meter_by_name NOT meter_me__${acl1} meter_me__${acl2}
+done
+
+# Check variations where unique meters are in SB
+for other_meters in 'm1,meter_me' 'm1,m2,meter_me,m3' 'meter_me,m1' 'meter_me'; do
+ ovn-nbctl --wait=sb set nb_global . options:acl_shared_log_meters="$other_meters"
+ check_meter_by_name meter_me meter_me__${acl1} meter_me__${acl2}
+done
+
+# Change template meter and make sure that is reflected on acl meters as well
+template_band=$(ovn-nbctl --bare --column=bands find meter name=meter_me)
+ovn-nbctl --wait=sb set meter_band $template_band rate=123
+# Make sure that every Meter_Band has the right rate. (ovn-northd
+# creates 3 identical Meter_Band rows, all identical; ovn-northd-ddlog
+# creates just 1. It doesn't matter, they work just as well.)
+n_meter_bands=$(count_rows meter_band)
+AT_FAIL_IF([test "$n_meter_bands" != 1 && test "$n_meter_bands" != 3])
+check_row_count meter_band $n_meter_bands rate=123
+
+# Check meter in logical flows for acl logs
+check_acl_lflow acl_one meter_me__${acl1}
+check_acl_lflow acl_two meter_me__${acl2}
+
+# Stop using meter for acl1
+ovn-nbctl --wait=sb clear acl $acl1 meter
+check_meter_by_name meter_me meter_me__${acl2}
+check_meter_by_name NOT meter_me__${acl1}
+check_acl_lflow acl_two meter_me__${acl2}
+
+# Remove template Meter should remove all others as well
+ovn-nbctl --wait=sb meter-del meter_me
+check_row_count meter 0
+# Check that logical flow remains. It refers to a named row that does not exist.
+check_acl_lflow acl_two meter_me__${acl2}
+
+# Re-add template meter and make sure acl2's meter is back in sb
+ovn-nbctl --wait=sb meter-add meter_me drop 1 pktps
+check_meter_by_name meter_me meter_me__${acl2}
+check_meter_by_name NOT meter_me__${acl1}
+check_acl_lflow acl_two meter_me__${acl2}
+
+# Remove acl2
+sw0=$(ovn-nbctl --bare --column=_uuid find logical_switch name=sw0)
+ovn-nbctl --wait=sb remove logical_switch $sw0 acls $acl2
+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])
AT_KEYWORDS([requested tnl tunnel key keys])
@@ -16662,9 +16662,6 @@ AT_CLEANUP
OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn -- IGMP snoop/querier/relay])
-dnl This test has problems with ovn-northd-ddlog.
-AT_SKIP_IF([test NORTHD_TYPE = ovn-northd-ddlog && test "$RUN_ANYWAY" != yes])
-
ovn_start
# Logical network:
@@ -17332,9 +17329,6 @@ AT_CLEANUP
OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn -- MLD snoop/querier/relay])
-dnl This test has problems with ovn-northd-ddlog.
-AT_SKIP_IF([test NORTHD_TYPE = ovn-northd-ddlog && test "$RUN_ANYWAY" != yes])
-
ovn_start
# Logical network: