From a0145975e3aff120df7edbd3cd76cebadbe71940 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 6 Apr 2021 21:09:50 +0300 Subject: [PATCH] lib: fix usage of operational data in CLI CLI must never use operational data, because this won't work in transactional mode. Rework search for prefix-list/access-list entries using only candidate config. Signed-off-by: Igor Ryzhov --- lib/filter.c | 53 ------- lib/filter.h | 10 +- lib/filter_cli.c | 391 ++++++++++++++++++++++------------------------- lib/filter_nb.c | 2 + 4 files changed, 188 insertions(+), 268 deletions(-) diff --git a/lib/filter.c b/lib/filter.c index 83423ba321..ab62e95fb6 100644 --- a/lib/filter.c +++ b/lib/filter.c @@ -469,59 +469,6 @@ void access_list_filter_add(struct access_list *access, host A single host address */ -struct filter *filter_lookup_cisco(struct access_list *access, - struct filter *mnew) -{ - struct filter *mfilter; - struct filter_cisco *filter; - struct filter_cisco *new; - - new = &mnew->u.cfilter; - - for (mfilter = access->head; mfilter; mfilter = mfilter->next) { - filter = &mfilter->u.cfilter; - - if (filter->extended) { - if (mfilter->type == mnew->type - && filter->addr.s_addr == new->addr.s_addr - && filter->addr_mask.s_addr == new->addr_mask.s_addr - && filter->mask.s_addr == new->mask.s_addr - && filter->mask_mask.s_addr - == new->mask_mask.s_addr) - return mfilter; - } else { - if (mfilter->type == mnew->type - && filter->addr.s_addr == new->addr.s_addr - && filter->addr_mask.s_addr - == new->addr_mask.s_addr) - return mfilter; - } - } - - return NULL; -} - -struct filter *filter_lookup_zebra(struct access_list *access, - struct filter *mnew) -{ - struct filter *mfilter; - struct filter_zebra *filter; - struct filter_zebra *new; - - new = &mnew->u.zfilter; - - for (mfilter = access->head; mfilter; mfilter = mfilter->next) { - filter = &mfilter->u.zfilter; - - if (filter->exact == new->exact - && mfilter->type == mnew->type) { - if (prefix_same(&filter->prefix, &new->prefix)) - return mfilter; - } - } - return NULL; -} - static void config_write_access_zebra(struct vty *, struct filter *); static void config_write_access_cisco(struct vty *, struct filter *); diff --git a/lib/filter.h b/lib/filter.h index 28f5202022..ade68a4567 100644 --- a/lib/filter.h +++ b/lib/filter.h @@ -151,10 +151,6 @@ void access_list_filter_add(struct access_list *access, void access_list_filter_delete(struct access_list *access, struct filter *filter); int64_t filter_new_seq_get(struct access_list *access); -struct filter *filter_lookup_cisco(struct access_list *access, - struct filter *mnew); -struct filter *filter_lookup_zebra(struct access_list *access, - struct filter *mnew); extern const struct frr_yang_module_info frr_filter_info; @@ -194,6 +190,9 @@ struct acl_dup_args { /** Duplicated entry found in list? */ bool ada_found; + /** Sequence number of the found entry */ + int64_t ada_seq; + /** (Optional) Already existing `dnode`. */ const struct lyd_node *ada_entry_dnode; }; @@ -224,6 +223,9 @@ struct plist_dup_args { /** Duplicated entry found in list? */ bool pda_found; + /** Sequence number of the found entry */ + int64_t pda_seq; + /** (Optional) Already existing `dnode`. */ const struct lyd_node *pda_entry_dnode; }; diff --git a/lib/filter_cli.c b/lib/filter_cli.c index a92b0d05e9..02dfd7f8b2 100644 --- a/lib/filter_cli.c +++ b/lib/filter_cli.c @@ -51,69 +51,6 @@ #define PREFIX_LIST_NAME_STR "Prefix list entry name\n" -/* - * Helper function to locate filter data structures for Cisco-style ACLs. - */ -static int64_t acl_cisco_get_seq(struct access_list *acl, const char *action, - const char *src, const char *src_mask, - const char *dst, const char *dst_mask) -{ - struct filter_cisco *fc; - struct filter f, *fn; - - memset(&f, 0, sizeof(f)); - f.cisco = 1; - if (strcmp(action, "permit") == 0) - f.type = FILTER_PERMIT; - else - f.type = FILTER_DENY; - - fc = &f.u.cfilter; - inet_pton(AF_INET, src, &fc->addr); - inet_pton(AF_INET, src_mask, &fc->addr_mask); - fc->addr.s_addr &= ~fc->addr_mask.s_addr; - if (dst != NULL) { - fc->extended = 1; - inet_pton(AF_INET, dst, &fc->mask); - inet_pton(AF_INET, dst_mask, &fc->mask_mask); - fc->mask.s_addr &= ~fc->mask_mask.s_addr; - } - - fn = filter_lookup_cisco(acl, &f); - if (fn == NULL) - return -1; - - return fn->seq; -} - -/* - * Helper function to locate filter data structures for zebra-style ACLs. - */ -static int64_t acl_zebra_get_seq(struct access_list *acl, const char *action, - const struct prefix *p, bool exact) -{ - struct filter_zebra *fz; - struct filter f, *fn; - - memset(&f, 0, sizeof(f)); - memset(&fz, 0, sizeof(fz)); - if (strcmp(action, "permit") == 0) - f.type = FILTER_PERMIT; - else - f.type = FILTER_DENY; - - fz = &f.u.zfilter; - if (p->family) - prefix_copy(&fz->prefix, p); - fz->exact = exact; - - fn = filter_lookup_zebra(acl, &f); - if (fn == NULL) - return -1; - - return fn->seq; -} - /* * Helper function to generate a sequence number for legacy commands. */ @@ -238,9 +175,8 @@ DEFPY_YANG( "Address to match\n" "Wildcard bits\n") { - struct access_list *acl; - struct lyd_node *dnode; int64_t sseq; + struct acl_dup_args ada = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; @@ -255,24 +191,30 @@ DEFPY_YANG( } /* Otherwise, to keep compatibility, we need to figure it out. */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='ipv4'][name='%s']", name); - - /* Access-list must exist before entries. */ - if (yang_dnode_exists(running_config->dnode, xpath) == false) - return CMD_WARNING_CONFIG_FAILED; + ada.ada_type = "ipv4"; + ada.ada_name = name; + ada.ada_action = action; + if (host_str && mask_str == NULL) { + ada.ada_xpath[0] = "./host"; + ada.ada_value[0] = host_str; + } else if (host_str && mask_str) { + ada.ada_xpath[0] = "./network/address"; + ada.ada_value[0] = host_str; + ada.ada_xpath[1] = "./network/mask"; + ada.ada_value[1] = mask_str; + } else { + ada.ada_xpath[0] = "./source-any"; + ada.ada_value[0] = ""; + } - /* Use access-list data structure to fetch sequence. */ - dnode = yang_dnode_get(running_config->dnode, xpath); - acl = nb_running_get_entry(dnode, NULL, true); - sseq = acl_cisco_get_seq(acl, action, host_str, - mask_str ? mask_str : CISCO_HOST_WILDCARD_MASK, - NULL, NULL); - if (sseq == -1) + if (acl_is_dup(vty->candidate_config->dnode, &ada)) + sseq = ada.ada_seq; + else return CMD_WARNING_CONFIG_FAILED; snprintfrr(xpath_entry, sizeof(xpath_entry), - "%s/entry[sequence='%" PRId64 "']", xpath, sseq); + "/frr-filter:lib/access-list[type='ipv4'][name='%s']/entry[sequence='%" PRId64 "']", + name, sseq); nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); @@ -416,9 +358,9 @@ DEFPY_YANG( "Destination address to match\n" "Any destination host\n") { - struct access_list *acl; - struct lyd_node *dnode; + int idx = 0; int64_t sseq; + struct acl_dup_args ada = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; @@ -433,48 +375,51 @@ DEFPY_YANG( } /* Otherwise, to keep compatibility, we need to figure it out. */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='ipv4'][name='%s']", name); - - /* Access-list must exist before entries. */ - if (yang_dnode_exists(running_config->dnode, xpath) == false) - return CMD_WARNING_CONFIG_FAILED; + ada.ada_type = "ipv4"; + ada.ada_name = name; + ada.ada_action = action; + if (src_str && src_mask_str == NULL) { + ada.ada_xpath[idx] = "./host"; + ada.ada_value[idx] = src_str; + idx++; + } else if (src_str && src_mask_str) { + ada.ada_xpath[idx] = "./network/address"; + ada.ada_value[idx] = src_str; + idx++; + ada.ada_xpath[idx] = "./network/mask"; + ada.ada_value[idx] = src_mask_str; + idx++; + } else { + ada.ada_xpath[idx] = "./source-any"; + ada.ada_value[idx] = ""; + idx++; + } - /* Use access-list data structure to fetch sequence. */ - dnode = yang_dnode_get(running_config->dnode, xpath); - acl = nb_running_get_entry(dnode, NULL, true); - if (src_str != NULL) { - if (dst_str != NULL) - sseq = acl_cisco_get_seq( - acl, action, src_str, - src_mask_str ? src_mask_str - : CISCO_HOST_WILDCARD_MASK, - dst_str, - dst_mask_str ? dst_mask_str - : CISCO_HOST_WILDCARD_MASK); - else - sseq = acl_cisco_get_seq( - acl, action, src_str, - src_mask_str ? src_mask_str - : CISCO_HOST_WILDCARD_MASK, - "0.0.0.0", CISCO_ANY_WILDCARD_MASK); + if (dst_str && dst_mask_str == NULL) { + ada.ada_xpath[idx] = "./destination-host"; + ada.ada_value[idx] = dst_str; + idx++; + } else if (dst_str && dst_mask_str) { + ada.ada_xpath[idx] = "./destination-network/address"; + ada.ada_value[idx] = dst_str; + idx++; + ada.ada_xpath[idx] = "./destination-network/mask"; + ada.ada_value[idx] = dst_mask_str; + idx++; } else { - if (dst_str != NULL) - sseq = acl_cisco_get_seq( - acl, action, "0.0.0.0", CISCO_ANY_WILDCARD_MASK, - dst_str, - dst_mask_str ? dst_mask_str - : CISCO_HOST_WILDCARD_MASK); - else - sseq = acl_cisco_get_seq( - acl, action, "0.0.0.0", CISCO_ANY_WILDCARD_MASK, - "0.0.0.0", CISCO_ANY_WILDCARD_MASK); + ada.ada_xpath[idx] = "./destination-any"; + ada.ada_value[idx] = ""; + idx++; } - if (sseq == -1) + + if (acl_is_dup(vty->candidate_config->dnode, &ada)) + sseq = ada.ada_seq; + else return CMD_WARNING_CONFIG_FAILED; snprintfrr(xpath_entry, sizeof(xpath_entry), - "%s/entry[sequence='%" PRId64 "']", xpath, sseq); + "/frr-filter:lib/access-list[type='ipv4'][name='%s']/entry[sequence='%" PRId64 "']", + name, sseq); nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); @@ -568,10 +513,8 @@ DEFPY_YANG( "Exact match of the prefixes\n" "Match any IPv4\n") { - struct access_list *acl; - struct lyd_node *dnode; int64_t sseq; - struct prefix pany; + struct acl_dup_args ada = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; @@ -586,28 +529,30 @@ DEFPY_YANG( } /* Otherwise, to keep compatibility, we need to figure it out. */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='ipv4'][name='%s']", name); - - /* Access-list must exist before entries. */ - if (yang_dnode_exists(running_config->dnode, xpath) == false) - return CMD_WARNING_CONFIG_FAILED; + ada.ada_type = "ipv4"; + ada.ada_name = name; + ada.ada_action = action; + + if (prefix_str) { + ada.ada_xpath[0] = "./ipv4-prefix"; + ada.ada_value[0] = prefix_str; + if (exact) { + ada.ada_xpath[1] = "./ipv4-exact-match"; + ada.ada_value[1] = "true"; + } + } else { + ada.ada_xpath[0] = "./any"; + ada.ada_value[0] = ""; + } - /* Use access-list data structure to fetch sequence. */ - dnode = yang_dnode_get(running_config->dnode, xpath); - acl = nb_running_get_entry(dnode, NULL, true); - if (prefix_str == NULL) { - memset(&pany, 0, sizeof(pany)); - pany.family = AF_INET; - sseq = acl_zebra_get_seq(acl, action, &pany, exact); - } else - sseq = acl_zebra_get_seq(acl, action, (struct prefix *)prefix, - exact); - if (sseq == -1) + if (acl_is_dup(vty->candidate_config->dnode, &ada)) + sseq = ada.ada_seq; + else return CMD_WARNING_CONFIG_FAILED; snprintfrr(xpath_entry, sizeof(xpath_entry), - "%s/entry[sequence='%" PRId64 "']", xpath, sseq); + "/frr-filter:lib/access-list[type='ipv4'][name='%s']/entry[sequence='%" PRId64 "']", + name, sseq); nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); @@ -767,10 +712,8 @@ DEFPY_YANG( "Exact match of the prefixes\n" "Match any IPv6\n") { - struct access_list *acl; - struct lyd_node *dnode; int64_t sseq; - struct prefix pany; + struct acl_dup_args ada = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; @@ -785,28 +728,30 @@ DEFPY_YANG( } /* Otherwise, to keep compatibility, we need to figure it out. */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='ipv6'][name='%s']", name); - - /* Access-list must exist before entries. */ - if (yang_dnode_exists(running_config->dnode, xpath) == false) - return CMD_WARNING_CONFIG_FAILED; + ada.ada_type = "ipv6"; + ada.ada_name = name; + ada.ada_action = action; + + if (prefix_str) { + ada.ada_xpath[0] = "./ipv6-prefix"; + ada.ada_value[0] = prefix_str; + if (exact) { + ada.ada_xpath[1] = "./ipv6-exact-match"; + ada.ada_value[1] = "true"; + } + } else { + ada.ada_xpath[0] = "./any"; + ada.ada_value[0] = ""; + } - /* Use access-list data structure to fetch sequence. */ - dnode = yang_dnode_get(running_config->dnode, xpath); - acl = nb_running_get_entry(dnode, NULL, true); - if (prefix == NULL) { - memset(&pany, 0, sizeof(pany)); - pany.family = AF_INET6; - sseq = acl_zebra_get_seq(acl, action, &pany, exact); - } else - sseq = acl_zebra_get_seq(acl, action, (struct prefix *)prefix, - exact); - if (sseq == -1) + if (acl_is_dup(vty->candidate_config->dnode, &ada)) + sseq = ada.ada_seq; + else return CMD_WARNING_CONFIG_FAILED; snprintfrr(xpath_entry, sizeof(xpath_entry), - "%s/entry[sequence='%" PRId64 "']", xpath, sseq); + "/frr-filter:lib/access-list[type='ipv6'][name='%s']/entry[sequence='%" PRId64 "']", + name, sseq); nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); @@ -951,7 +896,7 @@ DEFPY_YANG( DEFPY_YANG( no_mac_access_list, no_mac_access_list_cmd, - "no mac access-list WORD$name [seq (1-4294967295)$seq] $action ", + "no mac access-list WORD$name [seq (1-4294967295)$seq] $action ", NO_STR MAC_STR ACCESS_LIST_STR @@ -961,10 +906,8 @@ DEFPY_YANG( "MAC address\n" "Match any MAC address\n") { - struct access_list *acl; - struct lyd_node *dnode; int64_t sseq; - struct prefix pany; + struct acl_dup_args ada = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; @@ -979,28 +922,26 @@ DEFPY_YANG( } /* Otherwise, to keep compatibility, we need to figure it out. */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/access-list[type='mac'][name='%s']", name); + ada.ada_type = "mac"; + ada.ada_name = name; + ada.ada_action = action; - /* Access-list must exist before entries. */ - if (yang_dnode_exists(running_config->dnode, xpath) == false) - return CMD_WARNING_CONFIG_FAILED; + if (mac_str) { + ada.ada_xpath[0] = "./mac"; + ada.ada_value[0] = mac_str; + } else { + ada.ada_xpath[0] = "./any"; + ada.ada_value[0] = ""; + } - /* Use access-list data structure to fetch sequence. */ - dnode = yang_dnode_get(running_config->dnode, xpath); - acl = nb_running_get_entry(dnode, NULL, true); - if (prefix == NULL) { - memset(&pany, 0, sizeof(pany)); - pany.family = AF_ETHERNET; - sseq = acl_zebra_get_seq(acl, action, &pany, false); - } else - sseq = acl_zebra_get_seq(acl, action, (struct prefix *)prefix, - false); - if (sseq == -1) + if (acl_is_dup(vty->candidate_config->dnode, &ada)) + sseq = ada.ada_seq; + else return CMD_WARNING_CONFIG_FAILED; snprintfrr(xpath_entry, sizeof(xpath_entry), - "%s/entry[sequence='%" PRId64 "']", xpath, sseq); + "/frr-filter:lib/access-list[type='mac'][name='%s']/entry[sequence='%" PRId64 "']", + name, sseq); nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); return nb_cli_apply_changes(vty, NULL); @@ -1257,13 +1198,13 @@ static int plist_remove_if_empty(struct vty *vty, const char *iptype, } static int plist_remove(struct vty *vty, const char *iptype, const char *name, - const char *seq, const char *action, struct prefix *p, - long ge, long le) + const char *seq, const char *action, + const char *prefix_str, const char *ge_str, + const char *le_str) { - struct prefix_list_entry *pentry; - enum prefix_list_type plt; - struct prefix_list *pl; - struct lyd_node *dnode; + int64_t sseq; + int arg_idx = 0; + struct plist_dup_args pda = {}; char xpath[XPATH_MAXLEN]; char xpath_entry[XPATH_MAXLEN + 32]; int rv; @@ -1284,29 +1225,57 @@ static int plist_remove(struct vty *vty, const char *iptype, const char *name, } /* Otherwise, to keep compatibility, we need to figure it out. */ - snprintf(xpath, sizeof(xpath), - "/frr-filter:lib/prefix-list[type='%s'][name='%s']", iptype, - name); - - /* Access-list must exist before entries. */ - if (yang_dnode_exists(running_config->dnode, xpath) == false) - return CMD_WARNING_CONFIG_FAILED; + pda.pda_type = iptype; + pda.pda_name = name; + pda.pda_action = action; + if (prefix_str) { + if (strmatch(iptype, "ipv4")) { + pda.pda_xpath[arg_idx] = "./ipv4-prefix"; + pda.pda_value[arg_idx] = prefix_str; + arg_idx++; + if (ge_str) { + pda.pda_xpath[arg_idx] = + "./ipv4-prefix-length-greater-or-equal"; + pda.pda_value[arg_idx] = ge_str; + arg_idx++; + } + if (le_str) { + pda.pda_xpath[arg_idx] = + "./ipv4-prefix-length-lesser-or-equal"; + pda.pda_value[arg_idx] = le_str; + arg_idx++; + } + } else { + pda.pda_xpath[arg_idx] = "./ipv6-prefix"; + pda.pda_value[arg_idx] = prefix_str; + arg_idx++; + if (ge_str) { + pda.pda_xpath[arg_idx] = + "./ipv6-prefix-length-greater-or-equal"; + pda.pda_value[arg_idx] = ge_str; + arg_idx++; + } + if (le_str) { + pda.pda_xpath[arg_idx] = + "./ipv6-prefix-length-lesser-or-equal"; + pda.pda_value[arg_idx] = le_str; + arg_idx++; + } + } + } else { + pda.pda_xpath[0] = "./any"; + pda.pda_value[0] = ""; + } - /* Use access-list data structure to fetch sequence. */ - assert(action != NULL); - if (strcmp(action, "permit") == 0) - plt = PREFIX_PERMIT; + if (plist_is_dup(vty->candidate_config->dnode, &pda)) + sseq = pda.pda_seq; else - plt = PREFIX_DENY; - - dnode = yang_dnode_get(running_config->dnode, xpath); - pl = nb_running_get_entry(dnode, NULL, true); - pentry = prefix_list_entry_lookup(pl, p, plt, -1, le, ge); - if (pentry == NULL) return CMD_WARNING_CONFIG_FAILED; - snprintfrr(xpath_entry, sizeof(xpath_entry), - "%s/entry[sequence='%" PRId64 "']", xpath, pentry->seq); + snprintfrr( + xpath_entry, sizeof(xpath_entry), + "/frr-filter:lib/prefix-list[type='%s'][name='%s']/entry[sequence='%" PRId64 "']", + iptype, name, sseq); nb_cli_enqueue_change(vty, xpath_entry, NB_OP_DESTROY, NULL); rv = nb_cli_apply_changes(vty, NULL); @@ -1425,8 +1394,8 @@ DEFPY_YANG( "Maximum prefix length to be matched\n" "Maximum prefix length\n") { - return plist_remove(vty, "ipv4", name, seq_str, action, - (struct prefix *)prefix, ge, le); + return plist_remove(vty, "ipv4", name, seq_str, action, prefix_str, + ge_str, le_str); } DEFPY_YANG( @@ -1438,7 +1407,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_SEQ_STR) { - return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, 0, 0); + return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, NULL, NULL); } DEFPY_YANG( @@ -1621,8 +1590,8 @@ DEFPY_YANG( "Minimum prefix length to be matched\n" "Minimum prefix length\n") { - return plist_remove(vty, "ipv6", name, seq_str, action, - (struct prefix *)prefix, ge, le); + return plist_remove(vty, "ipv6", name, seq_str, action, prefix_str, + ge_str, le_str); } DEFPY_YANG( @@ -1634,7 +1603,7 @@ DEFPY_YANG( PREFIX_LIST_NAME_STR ACCESS_LIST_SEQ_STR) { - return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, 0, 0); + return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, NULL, NULL); } DEFPY_YANG( diff --git a/lib/filter_nb.c b/lib/filter_nb.c index 3aa362ad63..08c29789b9 100644 --- a/lib/filter_nb.c +++ b/lib/filter_nb.c @@ -279,6 +279,7 @@ static int _acl_is_dup(const struct lyd_node *dnode, void *arg) } ada->ada_found = true; + ada->ada_seq = yang_dnode_get_uint32(dnode, "sequence"); return YANG_ITER_STOP; } @@ -416,6 +417,7 @@ static int _plist_is_dup(const struct lyd_node *dnode, void *arg) } pda->pda_found = true; + pda->pda_seq = yang_dnode_get_uint32(dnode, "sequence"); return YANG_ITER_STOP; } -- 2.39.5