Using the ACL IDs created in the previous commit, northd adds this ID to the conntrack entry for allow-established ACLs.
ovn-controller keeps track of the southbound ACL IDs. If an ACL ID is removed from the southbound database, then ovn-controller flushes the conntrack entry whose label contains the deleted ACL ID. With this change, it means that deleting an allow-established ACL, or changing its action type to something else, will result in the conntrack entry being flushed. This will cause the traffic to no longer automatically be allowed. Reported-at: https://issues.redhat.com/browse/FDP-815 Signed-off-by: Mark Michelson <mmich...@redhat.com> --- controller/acl_ids.c | 279 ++++++++++++++++++++++++++++++++++++ controller/acl_ids.h | 30 ++++ controller/automake.mk | 2 + controller/ovn-controller.c | 12 +- lib/logical-fields.c | 2 + northd/en-acl-ids.c | 13 ++ northd/en-acl-ids.h | 3 + northd/en-lflow.c | 1 + northd/inc-proc-northd.c | 1 + northd/northd.c | 54 +++++-- northd/northd.h | 1 + tests/ovn-northd.at | 20 ++- tests/system-ovn.at | 20 +++ 13 files changed, 417 insertions(+), 21 deletions(-) create mode 100644 controller/acl_ids.c create mode 100644 controller/acl_ids.h diff --git a/controller/acl_ids.c b/controller/acl_ids.c new file mode 100644 index 000000000..6267f3399 --- /dev/null +++ b/controller/acl_ids.c @@ -0,0 +1,279 @@ +/* Copyright (c) 2024, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <config.h> + +#include "openvswitch/hmap.h" +#include "openvswitch/rconn.h" +#include "openvswitch/ofp-ct.h" +#include "openvswitch/ofp-util.h" +#include "openvswitch/ofp-msgs.h" +#include "openvswitch/vlog.h" +#include "lib/socket-util.h" + +#include "lib/ovn-sb-idl.h" +#include "acl_ids.h" + +VLOG_DEFINE_THIS_MODULE(acl_ids); + +enum acl_id_state { + /* The ID exists in the SB DB. */ + ACTIVE, + /* The ID has been removed from the DB and needs to have its conntrack + * entries flushed. + */ + SB_DELETED, + /* We have sent the conntrack flush request to OVS for this ACL ID. */ + FLUSHING, + /* We have either successfully flushed the ID, or we have failed enough + * times that we have given up. + */ + TO_DELETE, +}; + +struct acl_id { + int64_t id; + enum acl_id_state state; + struct hmap_node hmap_node; + ovs_be32 xid; + int flush_count; +}; + +struct tracked_acl_ids { + struct hmap ids; +}; + +static struct acl_id * +find_tracked_acl_id(struct tracked_acl_ids *tracked_ids, int64_t id) +{ + uint32_t hash = hash_uint64(id); + struct acl_id *acl_id; + HMAP_FOR_EACH_WITH_HASH (acl_id, hmap_node, hash, &tracked_ids->ids) { + if (acl_id->id == id) { + return acl_id; + } + } + return NULL; +} + +static void +acl_id_destroy(struct acl_id *acl_id) +{ + free(acl_id); +} + +void * +en_acl_id_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct tracked_acl_ids *ids = xzalloc(sizeof *ids); + hmap_init(&ids->ids); + return ids; +} + +void +en_acl_id_run(struct engine_node *node, void *data) +{ + const struct sbrec_acl_id_table *sb_acl_id_table = + EN_OVSDB_GET(engine_get_input("SB_acl_id", node)); + const struct sbrec_acl_id *sb_id; + + struct tracked_acl_ids *ids = data; + struct acl_id *id; + + /* Pre-mark each active ID as SB_DELETED. */ + HMAP_FOR_EACH (id, hmap_node, &ids->ids) { + if (id->state == ACTIVE) { + id->state = SB_DELETED; + } + } + + SBREC_ACL_ID_TABLE_FOR_EACH (sb_id, sb_acl_id_table) { + id = find_tracked_acl_id(ids, sb_id->id); + if (!id) { + id = xzalloc(sizeof *id); + id->id = sb_id->id; + hmap_insert(&ids->ids, &id->hmap_node, hash_uint64(sb_id->id)); + } + id->state = ACTIVE; + } + + engine_set_node_state(node, EN_UPDATED); +} + +void +en_acl_id_cleanup(void *data) +{ + struct tracked_acl_ids *tracked_ids = data; + struct acl_id *id; + HMAP_FOR_EACH_POP (id, hmap_node, &tracked_ids->ids) { + acl_id_destroy(id); + } + hmap_destroy(&tracked_ids->ids); +} + +static struct rconn *swconn; +static ovs_be32 barrier_xid; + +void +acl_ids_update_swconn(const char *target, int probe_interval) +{ + if (!swconn) { + swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << OFP15_VERSION); + } + ovn_update_swconn_at(swconn, target, probe_interval, "acl_ids"); +} + +#define MAX_FLUSHES 3 + +static void +acl_ids_handle_rconn_msg(struct ofpbuf *msg, struct tracked_acl_ids *acl_ids) +{ + const struct ofp_header *oh = msg->data; + + enum ofptype type; + ofptype_decode(&type, oh); + + if (type == OFPTYPE_ECHO_REQUEST) { + rconn_send(swconn, ofputil_encode_echo_reply(oh), NULL); + return; + } + + struct acl_id *acl_id; + if (oh->xid != barrier_xid) { + if (type != OFPTYPE_ERROR) { + return; + } + /* Uh oh! It looks like one of the flushes failed :( + * Let's find this particular one and move its state + * back to SB_DELETED so we can retry the flush. Of + * course, if this is a naughty little ID and has + * been flushed unsuccessfully too many times, we'll + * set it to TO_DELETE so it doesn't cause any more + * trouble. + */ + HMAP_FOR_EACH (acl_id, hmap_node, &acl_ids->ids) { + if (acl_id->xid != oh->xid) { + continue; + } + + acl_id->xid = 0; + acl_id->flush_count++; + if (acl_id->flush_count >= MAX_FLUSHES) { + acl_id->state = TO_DELETE; + } else { + acl_id->state = SB_DELETED; + } + + break; + } + } else { + HMAP_FOR_EACH (acl_id, hmap_node, &acl_ids->ids) { + if (acl_id->state != FLUSHING) { + continue; + } + acl_id->state = TO_DELETE; + } + barrier_xid = 0; + } +} + +static void +flush_expired_ids(struct tracked_acl_ids *acl_ids) +{ + if (barrier_xid != 0) { + /* We haven't received the previous barrier's reply, so + * hold off on sending new flushes until we get the + * reply. + */ + return; + } + + ovs_u128 mask = { + /* ct_labels.label BITS[80-95] */ + .u64.hi = 0xffff0000, + }; + struct acl_id *acl_id; + bool send_barrier = false; + HMAP_FOR_EACH (acl_id, hmap_node, &acl_ids->ids) { + if (acl_id->state != SB_DELETED) { + continue; + } + ovs_u128 ct_id = { + .u64.hi = acl_id->id << 16, + }; + VLOG_DBG("Flushing conntrack entry for ACL id %"PRId64, acl_id->id); + struct ofp_ct_match match = { + .labels = ct_id, + .labels_mask = mask, + }; + struct ofpbuf *msg = ofp_ct_match_encode(&match, NULL, + rconn_get_version(swconn)); + const struct ofp_header *oh = msg->data; + acl_id->xid = oh->xid; + acl_id->state = FLUSHING; + rconn_send(swconn, msg, NULL); + send_barrier = true; + } + + if (!send_barrier) { + return; + } + + struct ofpbuf *barrier = ofputil_encode_barrier_request(OFP15_VERSION); + const struct ofp_header *oh = barrier->data; + barrier_xid = oh->xid; + rconn_send(swconn, barrier, NULL); +} + +static void +clear_flushed_ids(struct tracked_acl_ids *acl_ids) +{ + struct acl_id *acl_id; + HMAP_FOR_EACH_SAFE (acl_id, hmap_node, &acl_ids->ids) { + if (acl_id->state != TO_DELETE) { + continue; + } + hmap_remove(&acl_ids->ids, &acl_id->hmap_node); + acl_id_destroy(acl_id); + } +} + +#define MAX_RECV_MSGS 50 + +void +acl_ids_run(struct tracked_acl_ids *acl_ids) +{ + rconn_run(swconn); + if (!rconn_is_connected(swconn)) { + rconn_run_wait(swconn); + rconn_recv_wait(swconn); + return; + } + + for (int i = 0; i < MAX_RECV_MSGS; i++) { + struct ofpbuf *msg = rconn_recv(swconn); + if (!msg) { + break; + } + acl_ids_handle_rconn_msg(msg, acl_ids); + ofpbuf_delete(msg); + } + flush_expired_ids(acl_ids); + clear_flushed_ids(acl_ids); + + rconn_run_wait(swconn); + rconn_recv_wait(swconn); +} diff --git a/controller/acl_ids.h b/controller/acl_ids.h new file mode 100644 index 000000000..e3556c37e --- /dev/null +++ b/controller/acl_ids.h @@ -0,0 +1,30 @@ +/* Copyright (c) 2024 Red Hat, INc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef OVN_ACL_IDS_H +#define OVN_ACL_IDS_H + +#include <config.h> +#include "lib/inc-proc-eng.h" + +void *en_acl_id_init(struct engine_node *, struct engine_arg *); +void en_acl_id_run(struct engine_node *, void *); +void en_acl_id_cleanup(void *); + +struct tracked_acl_ids; +void acl_ids_update_swconn(const char *target, int probe_interval); +void acl_ids_run(struct tracked_acl_ids *); + +#endif /* OVN_ACL_IDS_H */ diff --git a/controller/automake.mk b/controller/automake.mk index bb0bf2d33..c19ae27be 100644 --- a/controller/automake.mk +++ b/controller/automake.mk @@ -1,5 +1,7 @@ bin_PROGRAMS += controller/ovn-controller controller_ovn_controller_SOURCES = \ + controller/acl_ids.c \ + controller/acl_ids.h \ controller/bfd.c \ controller/bfd.h \ controller/binding.c \ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 157def2a3..f19c291d6 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -88,6 +88,7 @@ #include "lib/dns-resolve.h" #include "ct-zone.h" #include "ovn-dns.h" +#include "acl_ids.h" VLOG_DEFINE_THIS_MODULE(main); @@ -864,7 +865,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) SB_NODE(fdb, "fdb") \ SB_NODE(meter, "meter") \ SB_NODE(static_mac_binding, "static_mac_binding") \ - SB_NODE(chassis_template_var, "chassis_template_var") + SB_NODE(chassis_template_var, "chassis_template_var") \ + SB_NODE(acl_id, "acl_id") enum sb_engine_node { #define SB_NODE(NAME, NAME_STR) SB_##NAME, @@ -5095,6 +5097,7 @@ main(int argc, char *argv[]) ENGINE_NODE(mac_cache, "mac_cache"); ENGINE_NODE(bfd_chassis, "bfd_chassis"); ENGINE_NODE(dns_cache, "dns_cache"); + ENGINE_NODE(acl_id, "acl_id"); #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); SB_NODES @@ -5303,6 +5306,9 @@ main(int argc, char *argv[]) engine_add_input(&en_controller_output, &en_bfd_chassis, controller_output_bfd_chassis_handler); + engine_add_input(&en_acl_id, &en_sb_acl_id, NULL); + engine_add_input(&en_controller_output, &en_acl_id, NULL); + struct engine_arg engine_arg = { .sb_idl = ovnsb_idl_loop.idl, .ovs_idl = ovs_idl_loop.idl, @@ -5538,6 +5544,8 @@ main(int argc, char *argv[]) br_int_remote.probe_interval); pinctrl_update_swconn(br_int_remote.target, br_int_remote.probe_interval); + acl_ids_update_swconn(br_int_remote.target, + br_int_remote.probe_interval); /* Enable ACL matching for double tagged traffic. */ if (ovs_idl_txn && cfg) { @@ -5842,6 +5850,8 @@ main(int argc, char *argv[]) !ovnsb_idl_txn, !ovs_idl_txn); stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME, time_msec()); + + acl_ids_run(engine_get_data(&en_acl_id)); } } diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 5b578b5c2..89431b939 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -192,6 +192,8 @@ ovn_init_symtab(struct shash *symtab) "ct_label[96..127]", WR_CT_COMMIT); expr_symtab_add_subfield_scoped(symtab, "ct_label.obs_unused", NULL, "ct_label[0..95]", WR_CT_COMMIT); + expr_symtab_add_subfield_scoped(symtab, "ct_label.acl_id", NULL, + "ct_label[80..95]", WR_CT_COMMIT); expr_symtab_add_field(symtab, "ct_state", MFF_CT_STATE, NULL, false); diff --git a/northd/en-acl-ids.c b/northd/en-acl-ids.c index 897cc2da1..cb220dd80 100644 --- a/northd/en-acl-ids.c +++ b/northd/en-acl-ids.c @@ -204,3 +204,16 @@ void sync_acl_ids(const struct acl_id_data *id_data, } } } + +int64_t +get_acl_id(const struct acl_id_data *acl_id_data, const struct nbrec_acl *acl) +{ + struct acl_id *acl_id; + uint32_t hash = uuid_hash(&acl->header_.uuid); + HMAP_FOR_EACH_WITH_HASH (acl_id, node, hash, &acl_id_data->ids) { + if (uuid_equals(&acl_id->nb_acl_uuid, &acl->header_.uuid)) { + return acl_id->id; + } + } + return 0; +} diff --git a/northd/en-acl-ids.h b/northd/en-acl-ids.h index 8b60b3c7c..8bea2e078 100644 --- a/northd/en-acl-ids.h +++ b/northd/en-acl-ids.h @@ -14,4 +14,7 @@ void en_acl_id_cleanup(void *data); struct acl_id_data; void sync_acl_ids(const struct acl_id_data *, struct ovsdb_idl_txn *, struct ovsdb_idl_index *sbrec_acl_id_by_id); + +struct nbrec_acl; +int64_t get_acl_id(const struct acl_id_data *, const struct nbrec_acl *); #endif diff --git a/northd/en-lflow.c b/northd/en-lflow.c index fa1f0236d..3d57a469b 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -96,6 +96,7 @@ lflow_get_input_data(struct engine_node *node, struct ed_type_sampling_app_data *sampling_app_data = engine_get_input_data("sampling_app", node); lflow_input->sampling_apps = &sampling_app_data->apps; + lflow_input->acl_id_data = engine_get_input_data("acl_id", node); } void en_lflow_run(struct engine_node *node, void *data) diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 43abf042d..04c208c97 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -293,6 +293,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler); engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler); engine_add_input(&en_lflow, &en_ls_stateful, lflow_ls_stateful_handler); + engine_add_input(&en_lflow, &en_acl_id, NULL); engine_add_input(&en_sync_to_sb_addr_set, &en_northd, NULL); engine_add_input(&en_sync_to_sb_addr_set, &en_lr_stateful, NULL); diff --git a/northd/northd.c b/northd/northd.c index 8dab88f62..081305b09 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -206,6 +206,9 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2)); #define REG_OBS_COLLECTOR_ID_NEW "reg8[0..7]" #define REG_OBS_COLLECTOR_ID_EST "reg8[8..15]" +/* Register used for storing persistent ACL IDs */ +#define REG_ACL_ID "reg7[0..15]" + /* Register used for temporarily store ECMP eth.src to avoid masked ct_label * access. It doesn't really occupy registers because the content of the * register is saved to stack and then restored in the same flow. @@ -7063,7 +7066,8 @@ consider_acl(struct lflow_table *lflows, const struct ovn_datapath *od, const struct nbrec_acl *acl, bool has_stateful, const struct shash *meter_groups, uint64_t max_acl_tier, struct ds *match, struct ds *actions, - struct lflow_ref *lflow_ref) + struct lflow_ref *lflow_ref, + const struct acl_id_data *acl_id_data) { bool ingress = !strcmp(acl->direction, "from-lport") ? true :false; enum ovn_stage stage; @@ -7151,8 +7155,17 @@ consider_acl(struct lflow_table *lflows, const struct ovn_datapath *od, ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; "); if (!strcmp(acl->action, "allow-established")) { - ds_put_format(actions, - REGBIT_ACL_PERSIST_ID " = 1; "); + int64_t id = get_acl_id(acl_id_data, acl); + if (id) { + ds_put_format(actions, + REG_ACL_ID " = %"PRId64 "; " + REGBIT_ACL_PERSIST_ID " = 1; ", + id); + } else { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "No ID found for ACL "UUID_FMT" (%s)", + UUID_ARGS(&acl->header_.uuid), acl->match); + } } /* For stateful ACLs sample "new" and "established" packets. */ @@ -7476,7 +7489,8 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, const struct shash *meter_groups, const struct sampling_app_table *sampling_apps, const struct chassis_features *features, - struct lflow_ref *lflow_ref) + struct lflow_ref *lflow_ref, + const struct acl_id_data *acl_id_data) { const char *default_acl_action = default_acl_drop ? debug_implicit_drop_action() @@ -7686,7 +7700,8 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, lflow_ref); consider_acl(lflows, od, acl, has_stateful, meter_groups, ls_stateful_rec->max_acl_tier, - &match, &actions, lflow_ref); + &match, &actions, lflow_ref, + acl_id_data); build_acl_sample_flows(ls_stateful_rec, od, lflows, acl, &match, &actions, sampling_apps, features, lflow_ref); @@ -7705,7 +7720,8 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, lflow_ref); consider_acl(lflows, od, acl, has_stateful, meter_groups, ls_stateful_rec->max_acl_tier, - &match, &actions, lflow_ref); + &match, &actions, lflow_ref, + acl_id_data); build_acl_sample_flows(ls_stateful_rec, od, lflows, acl, &match, &actions, sampling_apps, features, lflow_ref); @@ -8394,6 +8410,7 @@ build_stateful(struct ovn_datapath *od, struct lflow_table *lflows, "ct_mark.obs_stage = " REGBIT_ACL_OBS_STAGE "; " "ct_mark.obs_collector_id = " REG_OBS_COLLECTOR_ID_EST "; " "ct_label.obs_point_id = " REG_OBS_POINT_ID_EST "; " + "ct_label.acl_id = " REG_ACL_ID "; " "}; next;"); ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100, REGBIT_CONNTRACK_COMMIT" == 1 && " @@ -8415,6 +8432,7 @@ build_stateful(struct ovn_datapath *od, struct lflow_table *lflows, "ct_commit { " "ct_mark.blocked = 0; " "ct_mark.allow_established = " REGBIT_ACL_PERSIST_ID "; " + "ct_label.acl_id = " REG_ACL_ID "; " "}; next;"); ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100, REGBIT_CONNTRACK_COMMIT" == 1 && " @@ -17216,7 +17234,8 @@ build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec, const struct shash *meter_groups, const struct sampling_app_table *sampling_apps, const struct chassis_features *features, - struct lflow_table *lflows) + struct lflow_table *lflows, + const struct acl_id_data *acl_id_data) { build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows, ls_stateful_rec->lflow_ref); @@ -17225,7 +17244,8 @@ build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec, build_acl_hints(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref); build_acls(ls_stateful_rec, od, lflows, ls_pgs, meter_groups, - sampling_apps, features, ls_stateful_rec->lflow_ref); + sampling_apps, features, ls_stateful_rec->lflow_ref, + acl_id_data); build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref); } @@ -17253,6 +17273,7 @@ struct lswitch_flow_build_info { struct hmap *parsed_routes; struct hmap *route_policies; struct simap *route_tables; + const struct acl_id_data *acl_id_data; }; /* Helper function to combine all lflow generation which is iterated by @@ -17552,7 +17573,8 @@ build_lflows_thread(void *arg) lsi->meter_groups, lsi->sampling_apps, lsi->features, - lsi->lflows); + lsi->lflows, + lsi->acl_id_data); } } @@ -17629,7 +17651,8 @@ build_lswitch_and_lrouter_flows( const struct sampling_app_table *sampling_apps, struct hmap *parsed_routes, struct hmap *route_policies, - struct simap *route_tables) + struct simap *route_tables, + const struct acl_id_data *acl_id_data) { char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); @@ -17667,6 +17690,7 @@ build_lswitch_and_lrouter_flows( lsiv[index].parsed_routes = parsed_routes; lsiv[index].route_tables = route_tables; lsiv[index].route_policies = route_policies; + lsiv[index].acl_id_data = acl_id_data; ds_init(&lsiv[index].match); ds_init(&lsiv[index].actions); @@ -17713,6 +17737,7 @@ build_lswitch_and_lrouter_flows( .route_policies = route_policies, .match = DS_EMPTY_INITIALIZER, .actions = DS_EMPTY_INITIALIZER, + .acl_id_data = acl_id_data, }; /* Combined build - all lflow generation from lswitch and lrouter @@ -17786,7 +17811,8 @@ build_lswitch_and_lrouter_flows( lsi.meter_groups, lsi.sampling_apps, lsi.features, - lsi.lflows); + lsi.lflows, + lsi.acl_id_data); } stopwatch_stop(LFLOWS_LS_STATEFUL_STOPWATCH_NAME, time_msec()); stopwatch_start(LFLOWS_IGMP_STOPWATCH_NAME, time_msec()); @@ -17879,7 +17905,8 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, input_data->sampling_apps, input_data->parsed_routes, input_data->route_policies, - input_data->route_tables); + input_data->route_tables, + input_data->acl_id_data); if (parallelization_state == STATE_INIT_HASH_SIZES) { parallelization_state = STATE_USE_PARALLELIZATION; @@ -18306,7 +18333,8 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn, lflow_input->meter_groups, lflow_input->sampling_apps, lflow_input->features, - lflows); + lflows, + lflow_input->acl_id_data); /* Sync the new flows to SB. */ bool handled = lflow_ref_sync_lflows( diff --git a/northd/northd.h b/northd/northd.h index bccb1c5d8..175c318af 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -232,6 +232,7 @@ struct lflow_input { struct hmap *parsed_routes; struct hmap *route_policies; struct simap *route_tables; + const struct acl_id_data *acl_id_data; }; extern int parallelization_state; diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 1d90622e1..b5b94330b 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -14297,21 +14297,27 @@ after_lb_uuid=$(fetch_column nb:ACL _uuid priority=1003) check ovn-nbctl set acl $ingress_uuid action=allow-established check ovn-nbctl set acl $egress_uuid action=allow-established -check ovn-nbctl set acl $after_lb_uuid action=allow-established +check ovn-nbctl --wait=sb set acl $after_lb_uuid action=allow-established + +dnl Retrieve the IDs for the ACLs so we can check them properly. + +ingress_id=$(fetch_column ACL_ID id nb_acl=$ingress_uuid) +egress_id=$(fetch_column ACL_ID id nb_acl=$egress_uuid) +after_lb_id=$(fetch_column ACL_ID id nb_acl=$after_lb_uuid) dnl Now we should see the registers being set to the appropriate values. -AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep priority=2001 | ovn_strip_lflows], [0], [dnl - table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[7]] == 1 && (tcp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[20]] = 1; next;) +AT_CHECK_UNQUOTED([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep priority=2001 | ovn_strip_lflows], [0], [dnl + table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[7]] == 1 && (tcp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg7[[0..15]] = $ingress_id; reg0[[20]] = 1; next;) table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[8]] == 1 && (tcp)), action=(reg8[[16]] = 1; next;) ]) -AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep priority=2003 | ovn_strip_lflows], [0], [dnl - table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[7]] == 1 && (udp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[20]] = 1; next;) +AT_CHECK_UNQUOTED([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep priority=2003 | ovn_strip_lflows], [0], [dnl + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[7]] == 1 && (udp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg7[[0..15]] = $after_lb_id; reg0[[20]] = 1; next;) table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[8]] == 1 && (udp)), action=(reg8[[16]] = 1; next;) ]) -AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep priority=2002 | ovn_strip_lflows], [0], [dnl - table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[7]] == 1 && (ip)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[20]] = 1; next;) +AT_CHECK_UNQUOTED([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep priority=2002 | ovn_strip_lflows], [0], [dnl + table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[7]] == 1 && (ip)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg7[[0..15]] = $egress_id; reg0[[20]] = 1; next;) table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[8]] == 1 && (ip)), action=(reg8[[16]] = 1; next;) ]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index a6ddda0f7..27e112a26 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -14251,6 +14251,18 @@ test : > output.txt +# Get the ID for this ACL +acl_id=$(fetch_column ACL_ID id nb_acl=$acl_uuid) +acl_id=$(printf %x $acl_id) + +OVS_WAIT_FOR_OUTPUT([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.1.2) | \ +grep "labels=0x"$acl_id"00000000000000000000" | \ +sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | \ +sed -e 's/mark=[[0-9]]*/mark=<cleared>/' | \ +sed -e 's/labels=0x[[0-9a-f]]*/labels=<cleared>/'], [0], [dnl +tcp,orig=(src=192.168.1.1,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.1.1,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=<cleared>,labels=<cleared>,protoinfo=(state=<cleared>) +]) + # Adjust the ACL so that it no longer matches check ovn-nbctl set ACL $acl_uuid match="\"ip4.dst == 192.168.1.3\"" @@ -14264,6 +14276,14 @@ test : > output.txt +# Now remove the ACL. This should remove the conntrack entry as well. +check ovn-nbctl --wait=hv acl-del sw from-lport 1000 'ip4.dst == 192.168.1.3' + +OVS_WAIT_FOR_OUTPUT([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.1.2) | \ +grep "labels=0x"$acl_id"00000000000000000000" | \ +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl +]) + OVS_APP_EXIT_AND_WAIT([ovn-controller]) as ovn-sb -- 2.45.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
