diff --git a/lib/zclient.h b/lib/zclient.h index 7adb294a31..99ed34138e 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -485,6 +485,29 @@ enum zapi_iptable_notify_owner { ZAPI_IPTABLE_FAIL_REMOVE, }; +static inline const char * +zapi_rule_notify_owner2str(enum zapi_rule_notify_owner note) +{ + const char *ret = "UNKNOWN"; + + switch (note) { + case ZAPI_RULE_FAIL_INSTALL: + ret = "ZAPI_RULE_FAIL_INSTALL"; + break; + case ZAPI_RULE_INSTALLED: + ret = "ZAPI_RULE_INSTALLED"; + break; + case ZAPI_RULE_FAIL_REMOVE: + ret = "ZAPI_RULE_FAIL_REMOVE"; + break; + case ZAPI_RULE_REMOVED: + ret = "ZAPI_RULE_REMOVED"; + break; + } + + return ret; +} + /* Zebra MAC types */ #define ZEBRA_MACIP_TYPE_STICKY 0x01 /* Sticky MAC*/ #define ZEBRA_MACIP_TYPE_GW 0x02 /* gateway (SVI) mac*/ diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c index bc4aa947a9..53248f5aaf 100644 --- a/pbrd/pbr_vty.c +++ b/pbrd/pbr_vty.c @@ -127,11 +127,16 @@ DEFPY(pbr_map_match_src, pbr_map_match_src_cmd, pbrms->family = prefix->family; if (!no) { - if (prefix_same(pbrms->src, prefix)) - return CMD_SUCCESS; + if (pbrms->src) { + if (prefix_same(pbrms->src, prefix)) + return CMD_SUCCESS; - if (!pbrms->src) - 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); @@ -145,7 +150,7 @@ DEFPY(pbr_map_match_dst, pbr_map_match_dst_cmd, "[no] match dst-ip $prefix", NO_STR "Match the rest of the command\n" - "Choose the src ip or ipv6 prefix to use\n" + "Choose the dst ip or ipv6 prefix to use\n" "v4 Prefix\n" "v6 Prefix\n") { @@ -154,11 +159,16 @@ DEFPY(pbr_map_match_dst, pbr_map_match_dst_cmd, pbrms->family = prefix->family; if (!no) { - if (prefix_same(pbrms->dst, prefix)) - return CMD_SUCCESS; + if (pbrms->dst) { + if (prefix_same(pbrms->dst, prefix)) + return CMD_SUCCESS; - if (!pbrms->dst) - 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); @@ -183,12 +193,18 @@ DEFPY(pbr_map_match_mark, pbr_map_match_mark_cmd, #endif if (!no) { - if (pbrms->mark == (uint32_t) mark) - return CMD_SUCCESS; - pbrms->mark = (uint32_t) mark; - } else { + 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); @@ -232,7 +248,7 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, pbr_map_delete_nexthops(pbrms); else { vty_out(vty, - "Nexthop Group specified: %s does not exist to remove", + "Nexthop Group specified: %s does not exist to remove\n", name); return CMD_WARNING_CONFIG_FAILED; } @@ -240,7 +256,7 @@ DEFPY(pbr_map_nexthop_group, pbr_map_nexthop_group_cmd, if (pbrms->nhgrp_name) { if (strcmp(name, pbrms->nhgrp_name) != 0) { vty_out(vty, - "Please delete current nexthop group before modifying current one"); + "Please delete current nexthop group before modifying current one\n"); return CMD_WARNING_CONFIG_FAILED; } @@ -277,7 +293,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, if (pbrms->nhgrp_name) { vty_out(vty, - "Please unconfigure the nexthop group before adding an individual nexthop"); + "Please unconfigure the nexthop group before adding an individual nexthop\n"); return CMD_WARNING_CONFIG_FAILED; } @@ -359,7 +375,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd, if (pbrms->nhg->nexthop) { vty_out(vty, - "If you would like more than one nexthop please use nexthop-groups"); + "If you would like more than one nexthop please use nexthop-groups\n"); return CMD_WARNING_CONFIG_FAILED; } diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c index b0a689a7e4..06ad0f40a4 100644 --- a/pbrd/pbr_zebra.c +++ b/pbrd/pbr_zebra.c @@ -234,23 +234,21 @@ static int rule_notify_owner(ZAPI_CALLBACK_ARGS) switch (note) { case ZAPI_RULE_FAIL_INSTALL: pbrms->installed &= ~installed; - DEBUGD(&pbr_dbg_zebra, - "%s: Received RULE_FAIL_INSTALL: %" PRIu64, - __PRETTY_FUNCTION__, pbrms->installed); break; case ZAPI_RULE_INSTALLED: pbrms->installed |= installed; - DEBUGD(&pbr_dbg_zebra, "%s: Received RULE_INSTALLED: %" PRIu64, - __PRETTY_FUNCTION__, pbrms->installed); break; case ZAPI_RULE_FAIL_REMOVE: + /* Don't change state on rule removal failure */ + break; case ZAPI_RULE_REMOVED: pbrms->installed &= ~installed; - DEBUGD(&pbr_dbg_zebra, "%s: Received RULE REMOVED: %" PRIu64, - __PRETTY_FUNCTION__, pbrms->installed); break; } + DEBUGD(&pbr_dbg_zebra, "%s: Received %s: %" PRIu64, __func__, + zapi_rule_notify_owner2str(note), pbrms->installed); + pbr_map_final_interface_deletion(pbrms->parent, pmi); return 0;