forked from Mirror/frr
lib: fix wrong returned value for filter
When setting rule for access-list ( and prefix-list ) without sequence, it will automatically get a sequence by `acl_get_seq()`, and return `CMD_SUCCESS` for command even this sequence value is wrong. In this scene, `CMD_WARNING_CONFIG_FAILED` should be returned with a warning. So, add the check in `acl_get_seq()` and move `nb_cli_enqueue_change()` after the check of wrong sequence. Both `plist_remove_if_empty()` and `acl_remove_if_empty()` should ignore this check, there is no change on them. Before: ``` anlan(config)# access-list aa seq 4294967295 deny 6.6.6.6/32 anlan(config)# access-list aa deny 6.6.6.7/32 <- Return CMD_SUCCESS YANG error(s): Value "4294967300" is out of uint32's min/max bounds. Value "4294967300" is out of uint32's min/max bounds. Value "4294967300" is out of uint32's min/max bounds. Value "4294967300" is out of uint32's min/max bounds. Value "4294967300" is out of uint32's min/max bounds. YANG path: Schema location /frr-filter:lib/prefix-list/entry/sequence. % Failed to edit configuration. ``` After: ``` anlan(config)# access-list aa seq 4294967295 deny 6.6.6.6/32 anlan(config)# access-list aa deny 6.6.6.7/32 <- Return CMD_WARNING_CONFIG_FAILED % Malformed sequence value ``` Additionally, fixed the overflow issue on `acl_get_seq()` on **32bit** platforms. Just change the returned type of `acl_get_seq()` from `long` to `int64_t`. Before: ``` anlan(config)# access-list bb seq 4294967295 deny 6.6.6.6/32 anlan(config)# access-list bb deny 6.6.6.7/32 anlan(config)# do show run ... access-list bb seq 4294967295 deny 6.6.6.6/32 access-list bb seq 4 deny 6.6.6.7/32 <- Overflow ``` After: ``` anlan(config)# access-list bb seq 4294967295 deny 6.6.6.6/32 anlan(config)# access-list bb deny 6.6.6.7/32 % Malformed sequence value ``` Signed-off-by: anlan_cs <vic.lan@pica8.com>
This commit is contained in:
parent
a8adf1b3cb
commit
efa354a978
|
@ -66,16 +66,21 @@ static int acl_get_seq_cb(const struct lyd_node *dnode, void *arg)
|
||||||
*
|
*
|
||||||
* \param[in] vty shell context with the candidate configuration.
|
* \param[in] vty shell context with the candidate configuration.
|
||||||
* \param[in] xpath the XPath to look for the sequence leaf.
|
* \param[in] xpath the XPath to look for the sequence leaf.
|
||||||
* \returns next unused sequence number.
|
* \returns next unused sequence number, -1 if out of range when adding.
|
||||||
*/
|
*/
|
||||||
static long acl_get_seq(struct vty *vty, const char *xpath)
|
static int64_t acl_get_seq(struct vty *vty, const char *xpath, bool is_remove)
|
||||||
{
|
{
|
||||||
int64_t seq = 0;
|
int64_t seq = 0;
|
||||||
|
|
||||||
yang_dnode_iterate(acl_get_seq_cb, &seq, vty->candidate_config->dnode,
|
yang_dnode_iterate(acl_get_seq_cb, &seq, vty->candidate_config->dnode,
|
||||||
"%s/entry", xpath);
|
"%s/entry", xpath);
|
||||||
|
|
||||||
return seq + 5;
|
seq += 5;
|
||||||
|
if (!is_remove && seq > UINT32_MAX) {
|
||||||
|
vty_out(vty, "%% Malformed sequence value\n");
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
return seq;
|
||||||
}
|
}
|
||||||
|
|
||||||
static int acl_remove_if_empty(struct vty *vty, const char *iptype,
|
static int acl_remove_if_empty(struct vty *vty, const char *iptype,
|
||||||
|
@ -98,7 +103,7 @@ static int acl_remove_if_empty(struct vty *vty, const char *iptype,
|
||||||
* NOTE: if the list is empty it will return the first sequence
|
* NOTE: if the list is empty it will return the first sequence
|
||||||
* number: 5.
|
* number: 5.
|
||||||
*/
|
*/
|
||||||
if (acl_get_seq(vty, xpath) != 5)
|
if (acl_get_seq(vty, xpath, true) != 5)
|
||||||
return CMD_SUCCESS;
|
return CMD_SUCCESS;
|
||||||
|
|
||||||
/* Nobody is using this list, lets remove it. */
|
/* Nobody is using this list, lets remove it. */
|
||||||
|
@ -174,16 +179,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/access-list[type='ipv4'][name='%s']", name);
|
"/frr-filter:lib/access-list[type='ipv4'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
@ -321,16 +329,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/access-list[type='ipv4'][name='%s']", name);
|
"/frr-filter:lib/access-list[type='ipv4'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
@ -483,16 +494,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/access-list[type='ipv4'][name='%s']", name);
|
"/frr-filter:lib/access-list[type='ipv4'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
@ -670,16 +684,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/access-list[type='ipv6'][name='%s']", name);
|
"/frr-filter:lib/access-list[type='ipv6'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
@ -857,16 +874,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/access-list[type='mac'][name='%s']", name);
|
"/frr-filter:lib/access-list[type='mac'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
@ -1167,7 +1187,7 @@ static int plist_remove_if_empty(struct vty *vty, const char *iptype,
|
||||||
* NOTE: if the list is empty it will return the first sequence
|
* NOTE: if the list is empty it will return the first sequence
|
||||||
* number: 5.
|
* number: 5.
|
||||||
*/
|
*/
|
||||||
if (acl_get_seq(vty, xpath) != 5)
|
if (acl_get_seq(vty, xpath, true) != 5)
|
||||||
return CMD_SUCCESS;
|
return CMD_SUCCESS;
|
||||||
|
|
||||||
/* Nobody is using this list, lets remove it. */
|
/* Nobody is using this list, lets remove it. */
|
||||||
|
@ -1275,16 +1295,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/prefix-list[type='ipv4'][name='%s']", name);
|
"/frr-filter:lib/prefix-list[type='ipv4'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
@ -1476,16 +1499,19 @@ DEFPY_YANG(
|
||||||
*/
|
*/
|
||||||
snprintf(xpath, sizeof(xpath),
|
snprintf(xpath, sizeof(xpath),
|
||||||
"/frr-filter:lib/prefix-list[type='ipv6'][name='%s']", name);
|
"/frr-filter:lib/prefix-list[type='ipv6'][name='%s']", name);
|
||||||
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
|
||||||
if (seq_str == NULL) {
|
if (seq_str == NULL) {
|
||||||
/* Use XPath to find the next sequence number. */
|
/* Use XPath to find the next sequence number. */
|
||||||
sseq = acl_get_seq(vty, xpath);
|
sseq = acl_get_seq(vty, xpath, false);
|
||||||
|
if (sseq < 0)
|
||||||
|
return CMD_WARNING_CONFIG_FAILED;
|
||||||
|
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
"%s/entry[sequence='%" PRId64 "']", xpath, sseq);
|
||||||
} else
|
} else
|
||||||
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
snprintfrr(xpath_entry, sizeof(xpath_entry),
|
||||||
"%s/entry[sequence='%s']", xpath, seq_str);
|
"%s/entry[sequence='%s']", xpath, seq_str);
|
||||||
|
|
||||||
|
nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, NULL);
|
||||||
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
nb_cli_enqueue_change(vty, xpath_entry, NB_OP_CREATE, NULL);
|
||||||
|
|
||||||
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
nb_cli_enqueue_change(vty, "./action", NB_OP_MODIFY, action);
|
||||||
|
|
Loading…
Reference in a new issue