lib: Introducing a 3rd state for route_map_apply: RMAP_OKAY

Introducing a 3rd state for route_map_apply library function: RMAP_OKAY

Traditionally route map rule apis  were designed to return
a binary response, consisting of either RMAP_MATCH or RMAP_NOMATCH.
Depending on this response, the following statemachine decided the
course of action:

State1: Receveived RMAP_MATCH
THEN: If Routemap type is PERMIT, execute other rules if applicable,
otherwise we PERMIT!
Else: If Routemap type is DENY, we DENYMATCH right away

State2: Received RMAP_NOMATCH, continue on to next rule, otherwise,
return DENYMATCH by default if nothing matched.

With reference to PR 4078 (FRRouting#4078),
we require a 3rd state because of the following situation:

The issue - what if, the rule api needs to abort or ignore a rule?:
"match evpn vni xx" route-map filter can be applied to incoming routes
regardless of whether the tunnel type is vxlan or mpls.
This rule should be N/A for mpls based evpn route, but applicable to only
vxlan based evpn route.

Today, the filter produces either a match or nomatch response regardless of
whether it is mpls/vxlan, resulting in either permitting or denying the
route.. So an mpls evpn route may get filtered out incorrectly.
Eg: "route-map RM1 permit 10 ; match evpn vni 20" or
"route-map RM2 deny 20 ; match vni 20"

With the introduction of the 3rd state, we can abort this rule check safely.
How? The rules api can now return RMAP_OKAY (or another enum) to indicate
that it encountered an invalid check, and needs to abort just that rule,
but continue with other rules.

Question: Do we repurpose an existing enum RMAP_OKAY or RMAP_ERROR
as the 3rd state (or create a new enum like RMAP_IGNORE)?

We chose to go with RMAP_OKAY (but open to ideas),
as a way to bypass the rmap filter

As a result we have a 3rd state:
State3: Received RMAP_OKAY
Then, proceed to other rules, otherwise return RMAP_OKAY by default.

Signed-off-by:Lakshman Krishnamoorthy <lkrishnamoor@vmware.com>

bgpd/bgp_rpki.c

@@ -129,8 +129,10 @@ static void print_record(const struct pfx_record *record, struct vty *vty);
 static int is_synchronized(void);
 static int is_running(void);
 static void route_match_free(void *rule);
-static route_map_result_t route_match(void *rule, const struct prefix *prefix,
-				      route_map_object_t type, void *object);
+static route_map_match_result_t route_match(void *rule,
+					    const struct prefix *prefix,
+					    route_map_object_t type,
+					    void *object);
 static void *route_match_compile(const char *arg);
 static void revalidate_bgp_node(struct bgp_node *bgp_node, afi_t afi,
 				safi_t safi);
@@ -213,8 +215,10 @@ static void ipv6_addr_to_host_byte_order(const uint32_t *src, uint32_t *dest)
 		dest[i] = ntohl(src[i]);
 }
 
-static route_map_result_t route_match(void *rule, const struct prefix *prefix,
-				      route_map_object_t type, void *object)
+static route_map_match_result_t route_match(void *rule,
+					    const struct prefix *prefix,
+					    route_map_object_t type,
+					    void *object)
 {
 	int *rpki_status = rule;
 	struct bgp_path_info *path;

eigrpd/eigrp_routemap.c

@@ -251,9 +251,9 @@ void eigrp_route_map_update(const char *notused)
 
 /* `match metric METRIC' */
 /* Match function return 1 if match is success else return zero. */
-static route_map_result_t route_match_metric(void *rule, struct prefix *prefix,
-					     route_map_object_t type,
-					     void *object)
+static route_map_match_result_t
+route_match_metric(void *rule, struct prefix *prefix, route_map_object_t type,
+		   void *object)
 {
 	//  uint32_t *metric;
 	//  uint32_t  check;
@@ -311,10 +311,9 @@ struct route_map_rule_cmd route_match_metric_cmd = {
 
 /* `match interface IFNAME' */
 /* Match function return 1 if match is success else return zero. */
-static route_map_result_t route_match_interface(void *rule,
-						struct prefix *prefix,
-						route_map_object_t type,
-						void *object)
+static route_map_match_result_t
+route_match_interface(void *rule, struct prefix *prefix,
+		      route_map_object_t type, void *object)
 {
 	//  struct rip_info *rinfo;
 	//  struct interface *ifp;
@@ -360,10 +359,10 @@ struct route_map_rule_cmd route_match_interface_cmd = {
 /* `match ip next-hop IP_ACCESS_LIST' */
 
 /* Match function return 1 if match is success else return zero. */
-static route_map_result_t route_match_ip_next_hop(void *rule,
-						  struct prefix *prefix,
-						  route_map_object_t type,
-						  void *object)
+static route_map_match_result_t route_match_ip_next_hop(void *rule,
+							struct prefix *prefix,
+							route_map_object_t type,
+							void *object)
 {
 	//  struct access_list *alist;
 	//  struct rip_info *rinfo;
@@ -407,7 +406,7 @@ static struct route_map_rule_cmd route_match_ip_next_hop_cmd = {
 
 /* `match ip next-hop prefix-list PREFIX_LIST' */
 
-static route_map_result_t
+static route_map_match_result_t
 route_match_ip_next_hop_prefix_list(void *rule, struct prefix *prefix,
 				    route_map_object_t type, void *object)
 {
@@ -452,10 +451,9 @@ static struct route_map_rule_cmd route_match_ip_next_hop_prefix_list_cmd = {
 
 /* Match function should return 1 if match is success else return
    zero. */
-static route_map_result_t route_match_ip_address(void *rule,
-						 struct prefix *prefix,
-						 route_map_object_t type,
-						 void *object)
+static route_map_match_result_t
+route_match_ip_address(void *rule, struct prefix *prefix,
+		       route_map_object_t type, void *object)
 {
 	struct access_list *alist;
 
@@ -491,7 +489,7 @@ static struct route_map_rule_cmd route_match_ip_address_cmd = {
 
 /* `match ip address prefix-list PREFIX_LIST' */
 
-static route_map_result_t
+static route_map_match_result_t
 route_match_ip_address_prefix_list(void *rule, struct prefix *prefix,
 				   route_map_object_t type, void *object)
 {
@@ -526,8 +524,9 @@ static struct route_map_rule_cmd route_match_ip_address_prefix_list_cmd = {
 
 /* `match tag TAG' */
 /* Match function return 1 if match is success else return zero. */
-static route_map_result_t route_match_tag(void *rule, struct prefix *prefix,
-					  route_map_object_t type, void *object)
+static route_map_match_result_t
+route_match_tag(void *rule, struct prefix *prefix, route_map_object_t type,
+		void *object)
 {
 	//  unsigned short *tag;
 	//  struct rip_info *rinfo;
@@ -568,9 +567,9 @@ struct route_map_rule_cmd route_match_tag_cmd = {
 	"tag", route_match_tag, route_match_tag_compile, route_match_tag_free};
 
 /* Set metric to attribute. */
-static route_map_result_t route_set_metric(void *rule, struct prefix *prefix,
-					   route_map_object_t type,
-					   void *object)
+static route_map_match_result_t
+route_set_metric(void *rule, struct prefix *prefix,
+		 route_map_object_t type, void *object)
 {
 	//  if (type == RMAP_RIP)
 	//    {
@@ -662,7 +661,7 @@ static struct route_map_rule_cmd route_set_metric_cmd = {
 /* `set ip next-hop IP_ADDRESS' */
 
 /* Set nexthop to object.  ojbect must be pointer to struct attr. */
-static route_map_result_t route_set_ip_nexthop(void *rule,
+static route_map_match_result_t route_set_ip_nexthop(void *rule,
 					       struct prefix *prefix,
 					       route_map_object_t type,
 					       void *object)
@@ -718,8 +717,9 @@ static struct route_map_rule_cmd route_set_ip_nexthop_cmd = {
 /* `set tag TAG' */
 
 /* Set tag to object.  ojbect must be pointer to struct attr. */
-static route_map_result_t route_set_tag(void *rule, struct prefix *prefix,
-					route_map_object_t type, void *object)
+static route_map_match_result_t
+route_set_tag(void *rule, struct prefix *prefix,
+	      route_map_object_t type, void *object)
 {
 	//  unsigned short *tag;
 	//  struct rip_info *rinfo;

isisd/isis_routemap.c

@@ -48,10 +48,9 @@
 #include "isis_zebra.h"
 #include "isis_routemap.h"
 
-static route_map_result_t route_match_ip_address(void *rule,
-						 const struct prefix *prefix,
-						 route_map_object_t type,
-						 void *object)
+static route_map_match_result_t
+route_match_ip_address(void *rule, const struct prefix *prefix,
+		       route_map_object_t type, void *object)
 {
 	struct access_list *alist;
 
@@ -81,7 +80,7 @@ static struct route_map_rule_cmd route_match_ip_address_cmd = {
 
 /* ------------------------------------------------------------*/
 
-static route_map_result_t
+static route_map_match_result_t
 route_match_ip_address_prefix_list(void *rule, const struct prefix *prefix,
 				   route_map_object_t type, void *object)
 {
@@ -114,10 +113,9 @@ struct route_map_rule_cmd route_match_ip_address_prefix_list_cmd = {
 
 /* ------------------------------------------------------------*/
 
-static route_map_result_t route_match_ipv6_address(void *rule,
-						   const struct prefix *prefix,
-						   route_map_object_t type,
-						   void *object)
+static route_map_match_result_t
+route_match_ipv6_address(void *rule, const struct prefix *prefix,
+			 route_map_object_t type, void *object)
 {
 	struct access_list *alist;
 
@@ -147,7 +145,7 @@ static struct route_map_rule_cmd route_match_ipv6_address_cmd = {
 
 /* ------------------------------------------------------------*/
 
-static route_map_result_t
+static route_map_match_result_t
 route_match_ipv6_address_prefix_list(void *rule, const struct prefix *prefix,
 				     route_map_object_t type, void *object)
 {
@@ -180,10 +178,9 @@ struct route_map_rule_cmd route_match_ipv6_address_prefix_list_cmd = {
 
 /* ------------------------------------------------------------*/
 
-static route_map_result_t route_set_metric(void *rule,
-					   const struct prefix *prefix,
-					   route_map_object_t type,
-					   void *object)
+static route_map_match_result_t
+route_set_metric(void *rule, const struct prefix *prefix,
+		 route_map_object_t type, void *object)
 {
 	uint32_t *metric;
 	struct isis_ext_info *info;

lib/routemap.c

@@ -1471,14 +1471,14 @@ int route_map_delete_set(struct route_map_index *index, const char *set_name,
    (note, this includes the description for the "NEXT"
    and "GOTO" frobs now
 
-	      Match   |   No Match
-		      |
-    permit    action  |     cont
-		      |
-    ------------------+---------------
-		      |
-    deny      deny    |     cont
-		      |
+	   |   Match   |   No Match   | No op
+	   |-----------|--------------|-------
+    permit |   action  |     cont     | cont.
+	   |           | default:deny | default:permit
+    -------------------+-----------------------
+	   |   deny    |     cont     | cont.
+    deny   |           | default:deny | default:permit
+	   |-----------|--------------|--------
 
    action)
       -Apply Set statements, accept route
@@ -1512,12 +1512,12 @@ int route_map_delete_set(struct route_map_index *index, const char *set_name,
    We need to make sure our route-map processing matches the above
 */
 
-static route_map_result_t
+static route_map_match_result_t
 route_map_apply_match(struct route_map_rule_list *match_list,
 		      const struct prefix *prefix, route_map_object_t type,
 		      void *object)
 {
-	route_map_result_t ret = RMAP_NOMATCH;
+	route_map_match_result_t ret = RMAP_NOMATCH;
 	struct route_map_rule *match;
 
 
@@ -1549,7 +1549,8 @@ route_map_result_t route_map_apply(struct route_map *map,
 				   route_map_object_t type, void *object)
 {
 	static int recursion = 0;
-	int ret = 0;
+	route_map_match_result_t match_ret = RMAP_NOMATCH;
+	route_map_result_t ret = 0;
 	struct route_map_index *index;
 	struct route_map_rule *set;
 
@@ -1569,24 +1570,24 @@ route_map_result_t route_map_apply(struct route_map *map,
 	for (index = map->head; index; index = index->next) {
 		/* Apply this index. */
 		index->applied++;
-		ret = route_map_apply_match(&index->match_list, prefix, type,
-					    object);
+		match_ret = route_map_apply_match(&index->match_list, prefix, type,
+						  object);
 
 		/* Now we apply the matrix from above */
-		if (ret == RMAP_NOMATCH)
+		if (match_ret == RMAP_NOMATCH || match_ret == RMAP_NOOP)
 			/* 'cont' from matrix - continue to next route-map
 			 * sequence */
 			continue;
-		else if (ret == RMAP_MATCH) {
+		else if (match_ret == RMAP_MATCH) {
 			if (index->type == RMAP_PERMIT)
 			/* 'action' */
 			{
 				/* permit+match must execute sets */
 				for (set = index->set_list.head; set;
 				     set = set->next)
-					ret = (*set->cmd->func_apply)(
-						set->value, prefix, type,
-						object);
+					match_ret = (*set->cmd->func_apply)(
+						     set->value, prefix, type,
+						     object);
 
 				/* Call another route-map if available */
 				if (index->nextrm) {
@@ -1637,6 +1638,10 @@ route_map_result_t route_map_apply(struct route_map *map,
 			}
 		}
 	}
+
+	if (match_ret == RMAP_NOOP)
+		return RMAP_OKAY;
+
 	/* Finally route-map does not match at all. */
 	return RMAP_DENYMATCH;
 }

lib/routemap.h

@@ -34,13 +34,18 @@ DECLARE_MTYPE(ROUTE_MAP_NAME)
 DECLARE_MTYPE(ROUTE_MAP_RULE)
 DECLARE_MTYPE(ROUTE_MAP_COMPILED)
 
+/* Route-map match result "Eg: match evpn vni xx"  */
+typedef enum {
+	RMAP_MATCH,
+	RMAP_NOMATCH,
+	RMAP_NOOP
+} route_map_match_result_t;
+
 /* Route map's type. */
 enum route_map_type { RMAP_PERMIT, RMAP_DENY, RMAP_ANY };
 
 typedef enum {
-	RMAP_MATCH,
 	RMAP_DENYMATCH,
-	RMAP_NOMATCH,
 	RMAP_ERROR,
 	RMAP_OKAY
 } route_map_result_t;
@@ -91,10 +96,10 @@ struct route_map_rule_cmd {
 	const char *str;
 
 	/* Function for value set or match. */
-	route_map_result_t (*func_apply)(void *rule,
-					 const struct prefix *prefix,
-					 route_map_object_t type,
-					 void *object);
+	route_map_match_result_t (*func_apply)(void *rule,
+					       const struct prefix *prefix,
+					       route_map_object_t type,
+					       void *object);
 
 	/* Compile argument and return result as void *. */
 	void *(*func_compile)(const char *);