From: Stephen Worley Date: Wed, 18 Dec 2019 21:11:39 +0000 (-0500) Subject: pbrd: implement `set *` and `match *` config replacement X-Git-Tag: base_7.4~89^2~4 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=f143cffac2a3cfef96b9d6d796673ecc7c545f8f;p=mirror%2Ffrr.git pbrd: implement `set *` and `match *` config replacement Implement the ability to replace any existing `set *` or `match` with another one or adding more config without having to first delete the original config already there. Before, we needed to constantly execute a `no` command for everything to remove the rule before making changes to it. With this patch, you can replace configs on individual sequences much easier. Signed-off-by: Stephen Worley --- diff --git a/pbrd/pbr_map.c b/pbrd/pbr_map.c index 95f97af945..7928b8e2e7 100644 --- a/pbrd/pbr_map.c +++ b/pbrd/pbr_map.c @@ -145,7 +145,7 @@ static bool pbr_map_interface_is_valid(const struct pbr_map_interface *pmi) } static void pbr_map_pbrms_update_common(struct pbr_map_sequence *pbrms, - bool install) + bool install, bool changed) { struct pbr_map *pbrm; struct listnode *node; @@ -161,19 +161,19 @@ static void pbr_map_pbrms_update_common(struct pbr_map_sequence *pbrms, if (install && !pbr_map_interface_is_valid(pmi)) continue; - pbr_send_pbr_map(pbrms, pmi, install); + pbr_send_pbr_map(pbrms, pmi, install, changed); } } } -static void pbr_map_pbrms_install(struct pbr_map_sequence *pbrms) +static void pbr_map_pbrms_install(struct pbr_map_sequence *pbrms, bool changed) { - pbr_map_pbrms_update_common(pbrms, true); + pbr_map_pbrms_update_common(pbrms, true, changed); } static void pbr_map_pbrms_uninstall(struct pbr_map_sequence *pbrms) { - pbr_map_pbrms_update_common(pbrms, false); + pbr_map_pbrms_update_common(pbrms, false, false); } static const char *const pbr_map_reason_str[] = { @@ -292,7 +292,7 @@ void pbr_map_policy_interface_update(const struct interface *ifp, bool state_up) for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, node, pbrms)) for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi)) if (pmi->ifp == ifp && pbr_map_interface_is_valid(pmi)) - pbr_send_pbr_map(pbrms, pmi, state_up); + pbr_send_pbr_map(pbrms, pmi, state_up, false); } static void pbrms_vrf_update(struct pbr_map_sequence *pbrms, @@ -306,7 +306,7 @@ static void pbrms_vrf_update(struct pbr_map_sequence *pbrms, DEBUGD(&pbr_dbg_map, "\tSeq %u uses vrf %s (%u), updating map", pbrms->seqno, vrf_name, pbr_vrf_id(pbr_vrf)); - pbr_map_check(pbrms); + pbr_map_check(pbrms, false); } } @@ -360,7 +360,7 @@ extern void pbr_map_delete(struct pbr_map_sequence *pbrms) pbrm = pbrms->parent; for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi)) - pbr_send_pbr_map(pbrms, pmi, false); + pbr_send_pbr_map(pbrms, pmi, false, false); if (pbrms->nhg) pbr_nht_delete_individual_nexthop(pbrms); @@ -619,7 +619,7 @@ void pbr_map_schedule_policy_from_nhg(const char *nh_group) && (strcmp(nh_group, pbrms->nhgrp_name) == 0)) { pbrms->nhs_installed = true; - pbr_map_check(pbrms); + pbr_map_check(pbrms, false); } if (pbrms->nhg @@ -627,7 +627,7 @@ void pbr_map_schedule_policy_from_nhg(const char *nh_group) == 0)) { pbrms->nhs_installed = true; - pbr_map_check(pbrms); + pbr_map_check(pbrms, false); } } } @@ -656,7 +656,8 @@ void pbr_map_policy_install(const char *name) pbrms->seqno); for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi)) if (pbr_map_interface_is_valid(pmi)) - pbr_send_pbr_map(pbrms, pmi, true); + pbr_send_pbr_map(pbrms, pmi, true, + false); } } } @@ -668,7 +669,7 @@ void pbr_map_policy_delete(struct pbr_map *pbrm, struct pbr_map_interface *pmi) for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, node, pbrms)) - pbr_send_pbr_map(pbrms, pmi, false); + pbr_send_pbr_map(pbrms, pmi, false, false); pmi->delete = true; } @@ -710,13 +711,13 @@ void pbr_map_check_nh_group_change(const char *nh_group) pbrm->incoming, inode, pmi)) pbr_send_pbr_map(pbrms, pmi, - false); + false, false); } } } } -void pbr_map_check(struct pbr_map_sequence *pbrms) +void pbr_map_check(struct pbr_map_sequence *pbrms, bool changed) { struct pbr_map *pbrm; bool install; @@ -741,7 +742,7 @@ void pbr_map_check(struct pbr_map_sequence *pbrms) } if (install) - pbr_map_pbrms_install(pbrms); + pbr_map_pbrms_install(pbrms, changed); else pbr_map_pbrms_uninstall(pbrms); } @@ -755,7 +756,7 @@ void pbr_map_install(struct pbr_map *pbrm) return; for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, node, pbrms)) - pbr_map_pbrms_install(pbrms); + pbr_map_pbrms_install(pbrms, false); } void pbr_map_init(void) diff --git a/pbrd/pbr_map.h b/pbrd/pbr_map.h index 8bd22cbf2a..41f1703954 100644 --- a/pbrd/pbr_map.h +++ b/pbrd/pbr_map.h @@ -182,7 +182,15 @@ extern void pbr_map_init(void); extern bool pbr_map_check_valid(const char *name); -extern void pbr_map_check(struct pbr_map_sequence *pbrms); +/** + * Re-check the pbr map for validity. + * + * Install if valid, remove if not. + * + * If changed is set, the config on the on the map has changed somewhere + * and the rules need to be replaced if valid. + */ +extern void pbr_map_check(struct pbr_map_sequence *pbrms, bool changed); extern void pbr_map_check_nh_group_change(const char *nh_group); extern void pbr_map_reason_string(unsigned int reason, char *buf, int size); diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 8a0168924d..2f3591ac8d 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -510,12 +510,26 @@ char *pbr_nht_nexthop_make_name(char *name, size_t l, return buffer; } -void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms) +void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms, + const struct nexthop *nhop) { struct pbr_nexthop_group_cache *pnhgc; struct pbr_nexthop_group_cache find; struct pbr_nexthop_cache *pnhc; struct pbr_nexthop_cache lookup; + struct nexthop *nh; + char buf[PBR_NHC_NAMELEN]; + + pbrms->nhg = nexthop_group_new(); + pbrms->internal_nhg_name = XSTRDUP( + MTYPE_TMP, + pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_NHC_NAMELEN, + pbrms->seqno, buf)); + + nh = nexthop_new(); + memcpy(nh, nhop, sizeof(*nh)); + + nexthop_group_add_sorted(pbrms->nhg, nh); memset(&find, 0, sizeof(find)); pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_NHC_NAMELEN, @@ -539,7 +553,7 @@ void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms) pbr_nht_install_nexthop_group(pnhgc, *pbrms->nhg); } -void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) +static void pbr_nht_release_individual_nexthop(struct pbr_map_sequence *pbrms) { struct pbr_nexthop_group_cache *pnhgc; struct pbr_nexthop_group_cache find; @@ -548,8 +562,6 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) struct nexthop *nh; enum nexthop_types_t nh_type = 0; - pbr_map_delete_nexthops(pbrms); - memset(&find, 0, sizeof(find)); snprintf(find.name, sizeof(find.name), "%s", pbrms->internal_nhg_name); pnhgc = hash_lookup(pbr_nhg_hash, &find); @@ -570,6 +582,13 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) XFREE(MTYPE_TMP, pbrms->internal_nhg_name); } +void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms) +{ + pbr_map_delete_nexthops(pbrms); + + pbr_nht_release_individual_nexthop(pbrms); +} + struct pbr_nexthop_group_cache *pbr_nht_add_group(const char *name) { struct nexthop *nhop; diff --git a/pbrd/pbr_nht.h b/pbrd/pbr_nht.h index 4ef41cede7..2533942547 100644 --- a/pbrd/pbr_nht.h +++ b/pbrd/pbr_nht.h @@ -88,7 +88,8 @@ extern struct pbr_nexthop_group_cache *pbr_nht_add_group(const char *name); extern void pbr_nht_change_group(const char *name); extern void pbr_nht_delete_group(const char *name); -extern void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms); +extern void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms, + const struct nexthop *nhop); extern void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms); /* * Given the tableid of the installed default diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index dfc8bec1bc..7c1391df2e 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -142,18 +142,14 @@ DEFPY(pbr_map_match_src, pbr_map_match_src_cmd, if (pbrms->src) { if (prefix_same(pbrms->src, prefix)) return CMD_SUCCESS; + } else + pbrms->src = prefix_new(); - vty_out(vty, - "A `match src-ip XX` command already exists, please remove that first\n"); - return CMD_WARNING_CONFIG_FAILED; - } - - pbrms->src = prefix_new(); prefix_copy(pbrms->src, prefix); } else prefix_free(&pbrms->src); - pbr_map_check(pbrms); + pbr_map_check(pbrms, true); return CMD_SUCCESS; } @@ -174,18 +170,14 @@ DEFPY(pbr_map_match_dst, pbr_map_match_dst_cmd, if (pbrms->dst) { if (prefix_same(pbrms->dst, prefix)) return CMD_SUCCESS; + } else + pbrms->dst = prefix_new(); - vty_out(vty, - "A `match dst-ip XX` command already exists, please remove that first\n"); - return CMD_WARNING_CONFIG_FAILED; - } - - pbrms->dst = prefix_new(); prefix_copy(pbrms->dst, prefix); } else prefix_free(&pbrms->dst); - pbr_map_check(pbrms); + pbr_map_check(pbrms, true); return CMD_SUCCESS; } @@ -205,26 +197,49 @@ DEFPY(pbr_map_match_mark, pbr_map_match_mark_cmd, #endif if (!no) { - if (pbrms->mark) { + if (pbrms->mark) if (pbrms->mark == (uint32_t)mark) return CMD_SUCCESS; - vty_out(vty, - "A `match mark XX` command already exists, please remove that first\n"); - return CMD_WARNING_CONFIG_FAILED; - } - pbrms->mark = (uint32_t)mark; } else pbrms->mark = 0; - pbr_map_check(pbrms); + pbr_map_check(pbrms, true); return CMD_SUCCESS; } -#define SET_VRF_EXISTS_STR \ - "A `set vrf XX` command already exists, please remove that first\n" +static void pbrms_clear_set_vrf_config(struct pbr_map_sequence *pbrms) +{ + if (pbrms->vrf_lookup || pbrms->vrf_unchanged) { + pbr_map_delete_vrf(pbrms); + pbrms->vrf_name[0] = '\0'; + pbrms->vrf_lookup = false; + pbrms->vrf_unchanged = false; + } +} + +static void pbrms_clear_set_nhg_config(struct pbr_map_sequence *pbrms) +{ + if (pbrms->nhgrp_name) + pbr_map_delete_nexthops(pbrms); +} + +static void pbrms_clear_set_nexthop_config(struct pbr_map_sequence *pbrms) +{ + if (pbrms->nhg) + pbr_nht_delete_individual_nexthop(pbrms); +} + +static void pbrms_clear_set_config(struct pbr_map_sequence *pbrms) +{ + pbrms_clear_set_vrf_config(pbrms); + pbrms_clear_set_nhg_config(pbrms); + pbrms_clear_set_nexthop_config(pbrms); + + pbrms->nhs_installed = false; +} DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, "[no] set nexthop-group NHGNAME$name", @@ -236,17 +251,6 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence); struct nexthop_group_cmd *nhgc; - if (pbrms->nhg) { - vty_out(vty, - "A `set nexthop XX` command already exists, please remove that first\n"); - return CMD_WARNING_CONFIG_FAILED; - } - - if (pbrms->vrf_lookup || pbrms->vrf_unchanged) { - vty_out(vty, SET_VRF_EXISTS_STR); - return CMD_WARNING_CONFIG_FAILED; - } - nhgc = nhgc_find(name); if (!nhgc) { vty_out(vty, "Specified nexthop-group %s does not exist\n", @@ -255,29 +259,22 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, "PBR-MAP will not be applied until it is created\n"); } - if (no) { - if (pbrms->nhgrp_name && strcmp(name, pbrms->nhgrp_name) == 0) - pbr_map_delete_nexthops(pbrms); - else { - vty_out(vty, - "Nexthop Group specified: %s does not exist to remove\n", - name); - return CMD_WARNING_CONFIG_FAILED; - } - } else { - if (pbrms->nhgrp_name) { - if (strcmp(name, pbrms->nhgrp_name) != 0) { - vty_out(vty, - "Please delete current nexthop group before modifying current one\n"); - return CMD_WARNING_CONFIG_FAILED; - } - - return CMD_SUCCESS; - } - pbrms->nhgrp_name = XSTRDUP(MTYPE_TMP, name); - pbr_map_check(pbrms); + if (no && pbrms->nhgrp_name && strcmp(name, pbrms->nhgrp_name) != 0) { + vty_out(vty, + "Nexthop Group specified: %s does not exist to remove\n", + name); + return CMD_WARNING_CONFIG_FAILED; } + if (pbrms->nhgrp_name && strcmp(name, pbrms->nhgrp_name) == 0) + return CMD_SUCCESS; + + /* This is new/replacement config */ + pbrms_clear_set_config(pbrms); + + pbrms->nhgrp_name = XSTRDUP(MTYPE_TMP, name); + pbr_map_check(pbrms, true); + return CMD_SUCCESS; } @@ -301,18 +298,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence); struct vrf *vrf; struct nexthop nhop; - struct nexthop *nh; - - if (pbrms->nhgrp_name) { - vty_out(vty, - "Please unconfigure the nexthop group before adding an individual nexthop\n"); - return CMD_WARNING_CONFIG_FAILED; - } - - if (pbrms->vrf_lookup || pbrms->vrf_unchanged) { - vty_out(vty, SET_VRF_EXISTS_STR); - return CMD_WARNING_CONFIG_FAILED; - } + struct nexthop *nh = NULL; if (vrf_name) vrf = vrf_lookup_by_name(vrf_name); @@ -362,45 +348,23 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, if (pbrms->nhg) nh = nexthop_exists(pbrms->nhg, &nhop); - else { - char buf[PBR_NHC_NAMELEN]; - - if (no) { - vty_out(vty, "No nexthops to delete\n"); - return CMD_WARNING_CONFIG_FAILED; - } - pbrms->nhg = nexthop_group_new(); - pbrms->internal_nhg_name = - XSTRDUP(MTYPE_TMP, - pbr_nht_nexthop_make_name(pbrms->parent->name, - PBR_NHC_NAMELEN, - pbrms->seqno, - buf)); - nh = NULL; + if (no && !nh) { + vty_out(vty, "No nexthops to delete\n"); + return CMD_WARNING_CONFIG_FAILED; } - if (no) { - if (nh) - pbr_nht_delete_individual_nexthop(pbrms); - } else if (!nh) { - - if (pbrms->nhg->nexthop) { - vty_out(vty, - "If you would like more than one nexthop please use nexthop-groups\n"); - return CMD_WARNING_CONFIG_FAILED; - } + if (nh) /* Same config re-entered */ + goto done; - /* must be adding new nexthop since !no and !nexthop_exists */ - nh = nexthop_new(); + /* This is new/replacement config */ + pbrms_clear_set_config(pbrms); - memcpy(nh, &nhop, sizeof(nhop)); - _nexthop_add(&pbrms->nhg->nexthop, nh); + pbr_nht_add_individual_nexthop(pbrms, &nhop); - pbr_nht_add_individual_nexthop(pbrms); - pbr_map_check(pbrms); - } + pbr_map_check(pbrms, true); +done: if (nhop.type == NEXTHOP_TYPE_IFINDEX || (nhop.type == NEXTHOP_TYPE_IPV6_IFINDEX && IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6))) { @@ -425,54 +389,24 @@ DEFPY(pbr_map_vrf, pbr_map_vrf_cmd, struct pbr_map_sequence *pbrms = VTY_GET_CONTEXT(pbr_map_sequence); if (no) { - pbr_map_delete_vrf(pbrms); - /* Reset all data */ - pbrms->nhs_installed = false; - pbrms->vrf_name[0] = '\0'; - pbrms->vrf_lookup = false; - pbrms->vrf_unchanged = false; + pbrms_clear_set_config(pbrms); return CMD_SUCCESS; } - if (pbrms->nhgrp_name || pbrms->nhg) { - vty_out(vty, - "A `set nexthop/nexthop-group XX` command already exits, please remove that first\n"); - return CMD_WARNING_CONFIG_FAILED; - } - /* - * Determine if a set vrf * command already exists. - * - * If its equivalent, just return success. - * - * Else, return failure, we don't allow atomic swaps yet. + * If an equivalent set vrf * exists, just return success. */ - if (vrf_name && pbrms->vrf_lookup) { - /* New vrf specified and one already exists */ - - /* Is this vrf different from one already configured? */ - if (strncmp(pbrms->vrf_name, vrf_name, sizeof(pbrms->vrf_name)) - != 0) - goto vrf_exists; - + if (vrf_name && pbrms->vrf_lookup + && strncmp(pbrms->vrf_name, vrf_name, sizeof(pbrms->vrf_name)) == 0) return CMD_SUCCESS; - - } else if (!vrf_name && pbrms->vrf_unchanged) { - /* Unchanged specified and unchanged already exists */ + else if (!vrf_name && pbrms->vrf_unchanged) /* Unchanged already set */ return CMD_SUCCESS; - } else if (vrf_name && pbrms->vrf_unchanged) { - /* New vrf specified and unchanged is already set */ - goto vrf_exists; - - } else if (!vrf_name && pbrms->vrf_lookup) { - /* Unchanged specified and vrf to lookup already exists */ - goto vrf_exists; - } + /* This is new/replacement config */ + pbrms_clear_set_config(pbrms); - /* Create new lookup VRF or Unchanged */ if (vrf_name) { if (!pbr_vrf_lookup_by_name(vrf_name)) { vty_out(vty, "Specified: %s is non-existent\n", @@ -485,13 +419,9 @@ DEFPY(pbr_map_vrf, pbr_map_vrf_cmd, } else pbrms->vrf_unchanged = true; - pbr_map_check(pbrms); + pbr_map_check(pbrms, true); return CMD_SUCCESS; - -vrf_exists: - vty_out(vty, SET_VRF_EXISTS_STR); - return CMD_WARNING_CONFIG_FAILED; } DEFPY (pbr_policy, diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c index cb28015c18..4178ad5803 100644 --- a/pbrd/pbr_zebra.c +++ b/pbrd/pbr_zebra.c @@ -548,7 +548,7 @@ static void pbr_encode_pbr_map_sequence(struct stream *s, } void pbr_send_pbr_map(struct pbr_map_sequence *pbrms, - struct pbr_map_interface *pmi, bool install) + struct pbr_map_interface *pmi, bool install, bool changed) { struct pbr_map *pbrm = pbrms->parent; struct stream *s; @@ -560,11 +560,13 @@ void pbr_send_pbr_map(struct pbr_map_sequence *pbrms, pbrm->name, install, is_installed); /* - * If we are installed and asked to do so again - * just return. If we are not installed and asked + * If we are installed and asked to do so again and the config + * has not changed, just return. + * + * If we are not installed and asked * and asked to delete just return; */ - if (install && is_installed) + if (install && is_installed && !changed) return; if (!install && !is_installed) diff --git a/pbrd/pbr_zebra.h b/pbrd/pbr_zebra.h index d5d938021a..cc42e21abe 100644 --- a/pbrd/pbr_zebra.h +++ b/pbrd/pbr_zebra.h @@ -36,7 +36,8 @@ extern void route_delete(struct pbr_nexthop_group_cache *pnhgc, extern void pbr_send_rnh(struct nexthop *nhop, bool reg); extern void pbr_send_pbr_map(struct pbr_map_sequence *pbrms, - struct pbr_map_interface *pmi, bool install); + struct pbr_map_interface *pmi, bool install, + bool changed); extern struct pbr_interface *pbr_if_new(struct interface *ifp);