lib: fix cisco access list wildcard usage

Don't attempt to compress the wildcard information to fit a `/M`, but
use its own full 4 byte field.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

2 files changed, 61 insertions, 73 deletions

diff --git a/lib/filter_cli.c b/lib/filter_cli.c
index 09fc3289c..a8230f3a9 100644
--- a/lib/filter_cli.c
+++ b/lib/filter_cli.c
@@ -115,21 +115,6 @@ static int64_t acl_zebra_get_seq(struct access_list *acl, const char *action,
 }
 
 /*
- * Helper function to concatenate address with mask in Cisco style.
- */
-static void concat_addr_mask_v4(const char *addr, const char *mask, char *dst,
-				size_t dstlen)
-{
-	struct in_addr ia;
-	int plen;
-
-	assert(inet_pton(AF_INET, mask, &ia) == 1);
-	ia.s_addr = ~ia.s_addr;
-	plen = ip_masklen(ia);
-	snprintf(dst, dstlen, "%s/%d", addr, plen);
-}
-
-/*
  * Helper function to generate a sequence number for legacy commands.
  */
 static int acl_get_seq_cb(const struct lyd_node *dnode, void *arg)
@@ -177,7 +162,6 @@ DEFPY_YANG(
 	"Wildcard bits\n")
 {
 	int64_t sseq;
-	char ipmask[64];
 	char xpath[XPATH_MAXLEN];
 	char xpath_entry[XPATH_MAXLEN + 128];
 
@@ -203,8 +187,10 @@ DEFPY_YANG(
 	if (host_str != NULL && mask_str == NULL) {
 		nb_cli_enqueue_change(vty, "./host", NB_OP_MODIFY, host_str);
 	} else if (host_str != NULL && mask_str != NULL) {
-		concat_addr_mask_v4(host_str, mask_str, ipmask, sizeof(ipmask));
-		nb_cli_enqueue_change(vty, "./network", NB_OP_MODIFY, ipmask);
+		nb_cli_enqueue_change(vty, "./network/address", NB_OP_MODIFY,
+				      host_str);
+		nb_cli_enqueue_change(vty, "./network/mask", NB_OP_MODIFY,
+				      mask_str);
 	} else {
 		nb_cli_enqueue_change(vty, "./source-any", NB_OP_CREATE, NULL);
 	}
@@ -285,7 +271,6 @@ DEFPY_YANG(
 	"Any destination host\n")
 {
 	int64_t sseq;
-	char ipmask[64], ipmask_dst[64];
 	char xpath[XPATH_MAXLEN];
 	char xpath_entry[XPATH_MAXLEN + 128];
 
@@ -311,9 +296,10 @@ DEFPY_YANG(
 	if (src_str != NULL && src_mask_str == NULL) {
 		nb_cli_enqueue_change(vty, "./host", NB_OP_MODIFY, src_str);
 	} else if (src_str != NULL && src_mask_str != NULL) {
-		concat_addr_mask_v4(src_str, src_mask_str, ipmask,
-				    sizeof(ipmask));
-		nb_cli_enqueue_change(vty, "./network", NB_OP_MODIFY, ipmask);
+		nb_cli_enqueue_change(vty, "./network/address", NB_OP_MODIFY,
+				      src_str);
+		nb_cli_enqueue_change(vty, "./network/mask", NB_OP_MODIFY,
+				      src_mask_str);
 	} else {
 		nb_cli_enqueue_change(vty, "./source-any", NB_OP_CREATE, NULL);
 	}
@@ -322,10 +308,10 @@ DEFPY_YANG(
 		nb_cli_enqueue_change(vty, "./destination-host", NB_OP_MODIFY,
 				      dst_str);
 	} else if (dst_str != NULL && dst_mask_str != NULL) {
-		concat_addr_mask_v4(dst_str, dst_mask_str, ipmask_dst,
-				    sizeof(ipmask_dst));
-		nb_cli_enqueue_change(vty, "./destination-network",
-				      NB_OP_MODIFY, ipmask_dst);
+		nb_cli_enqueue_change(vty, "./destination-network/address",
+				      NB_OP_MODIFY, dst_str);
+		nb_cli_enqueue_change(vty, "./destination-network/mask",
+				      NB_OP_MODIFY, dst_mask_str);
 	} else {
 		nb_cli_enqueue_change(vty, "./destination-any", NB_OP_CREATE,
 				      NULL);
@@ -947,7 +933,7 @@ void access_list_show(struct vty *vty, struct lyd_node *dnode,
 	bool is_exact = false;
 	bool cisco_style = false;
 	bool cisco_extended = false;
-	struct in_addr mask;
+	struct in_addr addr, mask;
 	char macstr[PREFIX2STR_BUFFER];
 
 	is_any = yang_dnode_exists(dnode, "./any");
@@ -957,11 +943,12 @@ void access_list_show(struct vty *vty, struct lyd_node *dnode,
 			break;
 
 		if (yang_dnode_exists(dnode, "./host")
-		    || yang_dnode_exists(dnode, "./network")
+		    || yang_dnode_exists(dnode, "./network/address")
 		    || yang_dnode_exists(dnode, "./source-any")) {
 			cisco_style = true;
 			if (yang_dnode_exists(dnode, "./destination-host")
-			    || yang_dnode_exists(dnode, "./destination-network")
+			    || yang_dnode_exists(
+				    dnode, "./destination-network/address")
 			    || yang_dnode_exists(dnode, "./destination-any"))
 				cisco_extended = true;
 		} else {
@@ -998,9 +985,9 @@ void access_list_show(struct vty *vty, struct lyd_node *dnode,
 			vty_out(vty, " ip");
 
 		if (yang_dnode_exists(dnode, "./network")) {
-			yang_dnode_get_prefix(&p, dnode, "./network");
-			masklen2ip(p.prefixlen, &mask);
-			vty_out(vty, " %pI4 %pI4", &p.u.prefix4, &mask);
+			yang_dnode_get_ipv4(&addr, dnode, "./network/address");
+			yang_dnode_get_ipv4(&mask, dnode, "./network/mask");
+			vty_out(vty, " %pI4 %pI4", &addr, &mask);
 		} else if (yang_dnode_exists(dnode, "./host")) {
 			if (cisco_extended)
 				vty_out(vty, " host");
@@ -1018,10 +1005,11 @@ void access_list_show(struct vty *vty, struct lyd_node *dnode,
 
 		/* Handle destination address. */
 		if (yang_dnode_exists(dnode, "./destination-network")) {
-			yang_dnode_get_prefix(&p, dnode,
-					      "./destination-network");
-			masklen2ip(p.prefixlen, &mask);
-			vty_out(vty, " %pI4 %pI4", &p.u.prefix4, &mask);
+			yang_dnode_get_ipv4(&addr, dnode,
+					    "./destination-network/address");
+			yang_dnode_get_ipv4(&mask, dnode,
+					    "./destination-network/mask");
+			vty_out(vty, " %pI4 %pI4", &addr, &mask);
 		} else if (yang_dnode_exists(dnode, "./destination-host"))
 			vty_out(vty, " host %s",
 				yang_dnode_get_string(dnode,
diff --git a/lib/filter_nb.c b/lib/filter_nb.c
index 8838a48ab..b253743db 100644
--- a/lib/filter_nb.c
+++ b/lib/filter_nb.c
@@ -32,15 +32,6 @@
 #include "lib/routemap.h"
 
 /* Helper function. */
-static in_addr_t
-ipv4_network_addr(in_addr_t hostaddr, int masklen)
-{
-	struct in_addr mask;
-
-	masklen2ip(masklen, &mask);
-	return hostaddr & mask.s_addr;
-}
-
 static void acl_notify_route_map(struct access_list *acl, int route_map_event)
 {
 	switch (route_map_event) {
@@ -411,14 +402,13 @@ lib_access_list_entry_host_destroy(struct nb_cb_destroy_args *args)
 }
 
 /*
- * XPath: /frr-filter:lib/access-list/entry/network
+ * XPath: /frr-filter:lib/access-list/entry/network/address
  */
 static int
-lib_access_list_entry_network_modify(struct nb_cb_modify_args *args)
+lib_access_list_entry_network_address_modify(struct nb_cb_modify_args *args)
 {
 	struct filter_cisco *fc;
 	struct filter *f;
-	struct prefix p;
 
 	if (args->event != NB_EV_APPLY)
 		return NB_OK;
@@ -426,18 +416,18 @@ lib_access_list_entry_network_modify(struct nb_cb_modify_args *args)
 	f = nb_running_get_entry(args->dnode, NULL, true);
 	f->cisco = 1;
 	fc = &f->u.cfilter;
-	yang_dnode_get_prefix(&p, args->dnode, NULL);
-	fc->addr.s_addr = ipv4_network_addr(p.u.prefix4.s_addr, p.prefixlen);
-	masklen2ip(p.prefixlen, &fc->addr_mask);
-	fc->addr_mask.s_addr = ~fc->addr_mask.s_addr;
+	yang_dnode_get_ipv4(&fc->addr, args->dnode, NULL);
 
 	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
 
 	return NB_OK;
 }
 
+/*
+ * XPath: /frr-filter:lib/access-list/entry/network/mask
+ */
 static int
-lib_access_list_entry_network_destroy(struct nb_cb_destroy_args *args)
+lib_access_list_entry_network_mask_modify(struct nb_cb_modify_args *args)
 {
 	struct filter_cisco *fc;
 	struct filter *f;
@@ -446,10 +436,11 @@ lib_access_list_entry_network_destroy(struct nb_cb_destroy_args *args)
 		return NB_OK;
 
 	f = nb_running_get_entry(args->dnode, NULL, true);
+	f->cisco = 1;
 	fc = &f->u.cfilter;
-	cisco_unset_addr_mask(&fc->addr, &fc->addr_mask);
+	yang_dnode_get_ipv4(&fc->addr_mask, args->dnode, NULL);
 
-	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
+	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
 
 	return NB_OK;
 }
@@ -538,14 +529,13 @@ static int lib_access_list_entry_destination_host_destroy(
 }
 
 /*
- * XPath: /frr-filter:lib/access-list/entry/destination-network
+ * XPath: /frr-filter:lib/access-list/entry/destination-network/address
  */
-static int lib_access_list_entry_destination_network_modify(
+static int lib_access_list_entry_destination_network_address_modify(
 	struct nb_cb_modify_args *args)
 {
 	struct filter_cisco *fc;
 	struct filter *f;
-	struct prefix p;
 
 	if (args->event != NB_EV_APPLY)
 		return NB_OK;
@@ -553,18 +543,18 @@ static int lib_access_list_entry_destination_network_modify(
 	f = nb_running_get_entry(args->dnode, NULL, true);
 	fc = &f->u.cfilter;
 	fc->extended = 1;
-	yang_dnode_get_prefix(&p, args->dnode, NULL);
-	fc->mask.s_addr = ipv4_network_addr(p.u.prefix4.s_addr, p.prefixlen);
-	masklen2ip(p.prefixlen, &fc->mask_mask);
-	fc->mask_mask.s_addr = ~fc->mask_mask.s_addr;
+	yang_dnode_get_ipv4(&fc->mask, args->dnode, NULL);
 
 	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
 
 	return NB_OK;
 }
 
-static int lib_access_list_entry_destination_network_destroy(
-	struct nb_cb_destroy_args *args)
+/*
+ * XPath: /frr-filter:lib/access-list/entry/destination-network/mask
+ */
+static int lib_access_list_entry_destination_network_mask_modify(
+	struct nb_cb_modify_args *args)
 {
 	struct filter_cisco *fc;
 	struct filter *f;
@@ -574,10 +564,10 @@ static int lib_access_list_entry_destination_network_destroy(
 
 	f = nb_running_get_entry(args->dnode, NULL, true);
 	fc = &f->u.cfilter;
-	fc->extended = 0;
-	cisco_unset_addr_mask(&fc->mask, &fc->mask_mask);
+	fc->extended = 1;
+	yang_dnode_get_ipv4(&fc->mask_mask, args->dnode, NULL);
 
-	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
+	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_ADDED);
 
 	return NB_OK;
 }
@@ -1100,10 +1090,15 @@ const struct frr_yang_module_info frr_filter_info = {
 			}
 		},
 		{
-			.xpath = "/frr-filter:lib/access-list/entry/network",
+			.xpath = "/frr-filter:lib/access-list/entry/network/address",
 			.cbs = {
-				.modify = lib_access_list_entry_network_modify,
-				.destroy = lib_access_list_entry_network_destroy,
+				.modify = lib_access_list_entry_network_address_modify,
+			}
+		},
+		{
+			.xpath = "/frr-filter:lib/access-list/entry/network/mask",
+			.cbs = {
+				.modify = lib_access_list_entry_network_mask_modify,
 			}
 		},
 		{
@@ -1121,10 +1116,15 @@ const struct frr_yang_module_info frr_filter_info = {
 			}
 		},
 		{
-			.xpath = "/frr-filter:lib/access-list/entry/destination-network",
+			.xpath = "/frr-filter:lib/access-list/entry/destination-network/address",
+			.cbs = {
+				.modify = lib_access_list_entry_destination_network_address_modify,
+			}
+		},
+		{
+			.xpath = "/frr-filter:lib/access-list/entry/destination-network/mask",
 			.cbs = {
-				.modify = lib_access_list_entry_destination_network_modify,
-				.destroy = lib_access_list_entry_destination_network_destroy,
+				.modify = lib_access_list_entry_destination_network_mask_modify,
 			}
 		},
 		{