Merge pull request #13228 from LabNConsulting/chopps/json-via-yang

Update ripd YANG operational state (ECMP routes)

ripd/rip_nb.c

@@ -337,6 +337,66 @@ const struct frr_yang_module_info frr_ripd_info = {
 				.get_elem = ripd_instance_state_routes_route_interface_get_elem,
 			},
 		},
+		{
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop",
+                        .cbs = {
+                                .get_next = ripd_instance_state_routes_route_nexthops_nexthop_get_next,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/nh-type",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_nh_type_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/protocol",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_protocol_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/rip-type",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_rip_type_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/gateway",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_gateway_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/interface",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_interface_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/from",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_from_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/tag",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_tag_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/external-metric",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_external_metric_get_elem,
+                        }
+                },
+                {
+                        .xpath = "/frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/expire-time",
+                        .cbs = {
+                                .get_elem = ripd_instance_state_routes_route_nexthops_nexthop_expire_time_get_elem,
+                        }
+                },
 		{
 			.xpath = "/frr-ripd:ripd/instance/state/routes/route/metric",
 			.cbs = {

ripd/rip_nb.h

@@ -89,6 +89,37 @@ struct yang_data *ripd_instance_state_routes_route_interface_get_elem(
 	struct nb_cb_get_elem_args *args);
 struct yang_data *ripd_instance_state_routes_route_metric_get_elem(
 	struct nb_cb_get_elem_args *args);
+const void *ripd_instance_state_routes_route_nexthops_nexthop_get_next(
+	struct nb_cb_get_next_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_nh_type_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_protocol_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_rip_type_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_gateway_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_interface_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_from_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_tag_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_external_metric_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_expire_time_get_elem(
+	struct nb_cb_get_elem_args *args);
+struct yang_data *ripd_instance_state_routes_route_metric_get_elem(
+	struct nb_cb_get_elem_args *args);
 int clear_rip_route_rpc(struct nb_cb_rpc_args *args);
 int lib_interface_rip_split_horizon_modify(struct nb_cb_modify_args *args);
 int lib_interface_rip_v2_broadcast_modify(struct nb_cb_modify_args *args);

ripd/rip_nb_state.c

@@ -207,9 +207,170 @@ struct yang_data *ripd_instance_state_routes_route_prefix_get_elem(
 	const struct route_node *rn = args->list_entry;
 	const struct rip_info *rinfo = listnode_head(rn->info);
 
+	assert(rinfo);
 	return yang_data_new_ipv4p(args->xpath, &rinfo->rp->p);
 }
 
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop
+ */
+const void *ripd_instance_state_routes_route_nexthops_nexthop_get_next(
+	struct nb_cb_get_next_args *args)
+{
+	const struct route_node *rn = args->parent_list_entry;
+	const struct listnode *node = args->list_entry;
+
+	assert(rn);
+	if (node)
+		return listnextnode(node);
+	assert(rn->info);
+	return listhead((struct list *)rn->info);
+}
+
+static inline const struct rip_info *get_rip_info(const void *info)
+{
+	return (const struct rip_info *)listgetdata(
+		(const struct listnode *)info);
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/nh-type
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_nh_type_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	assert(rinfo);
+	return yang_data_new_enum(args->xpath, rinfo->nh.type);
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/protocol
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_protocol_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	assert(rinfo);
+	return yang_data_new_enum(args->xpath, rinfo->type);
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/rip-type
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_rip_type_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	assert(rinfo);
+	return yang_data_new_enum(args->xpath, rinfo->sub_type);
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/gateway
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_gateway_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	if (rinfo->nh.type != NEXTHOP_TYPE_IPV4 &&
+	    rinfo->nh.type != NEXTHOP_TYPE_IPV4_IFINDEX)
+		return NULL;
+
+	return yang_data_new_ipv4(args->xpath, &rinfo->nh.gate.ipv4);
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/interface
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_interface_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+	const struct rip *rip = rip_info_get_instance(rinfo);
+
+	if (rinfo->nh.type != NEXTHOP_TYPE_IFINDEX &&
+	    rinfo->nh.type != NEXTHOP_TYPE_IPV4_IFINDEX)
+		return NULL;
+
+	return yang_data_new_string(
+		args->xpath,
+		ifindex2ifname(rinfo->nh.ifindex, rip->vrf->vrf_id));
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/from
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_from_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	if (rinfo->type != ZEBRA_ROUTE_RIP || rinfo->sub_type != RIP_ROUTE_RTE)
+		return NULL;
+
+	return yang_data_new_ipv4(args->xpath, &rinfo->from);
+}
+
+/*
+ * XPath: /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/tag
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_tag_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	return yang_data_new_uint32(args->xpath, rinfo->tag);
+}
+
+/*
+ * XPath:
+ * /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/external-metric
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_external_metric_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+
+	if ((rinfo->type == ZEBRA_ROUTE_RIP &&
+	     rinfo->sub_type == RIP_ROUTE_RTE) ||
+	    rinfo->metric == RIP_METRIC_INFINITY || rinfo->external_metric == 0)
+		return NULL;
+	return yang_data_new_uint32(args->xpath, rinfo->external_metric);
+}
+
+/*
+ * XPath:
+ * /frr-ripd:ripd/instance/state/routes/route/nexthops/nexthop/expire-time
+ */
+struct yang_data *
+ripd_instance_state_routes_route_nexthops_nexthop_expire_time_get_elem(
+	struct nb_cb_get_elem_args *args)
+{
+	const struct rip_info *rinfo = get_rip_info(args->list_entry);
+	struct event *event;
+
+	if ((event = rinfo->t_timeout) == NULL)
+		event = rinfo->t_garbage_collect;
+	if (!event)
+		return NULL;
+
+	return yang_data_new_uint32(args->xpath,
+				    event_timer_remain_second(event));
+}
+
 /*
  * XPath: /frr-ripd:ripd/instance/state/routes/route/next-hop
  */

tests/topotests/rip_allow_ecmp/r1/frr.conf

@@ -0,0 +1,9 @@
+!
+int r1-eth0
+ ip address 192.168.1.1/24
+!
+router rip
+ allow-ecmp
+ network 192.168.1.0/24
+ timers basic 5 15 10
+exit

tests/topotests/rip_allow_ecmp/r2/frr.conf

@@ -0,0 +1,13 @@
+!
+int lo
+ ip address 10.10.10.1/32
+!
+int r2-eth0
+ ip address 192.168.1.2/24
+!
+router rip
+ network 192.168.1.0/24
+ network 10.10.10.1/32
+ timers basic 5 15 10
+exit
+

tests/topotests/rip_allow_ecmp/r3/frr.conf

@@ -0,0 +1,13 @@
+!
+int lo
+ ip address 10.10.10.1/32
+!
+int r3-eth0
+ ip address 192.168.1.3/24
+!
+router rip
+ network 192.168.1.0/24
+ network 10.10.10.1/32
+ timers basic 5 15 10
+exit
+

tests/topotests/rip_allow_ecmp/test_rip_allow_ecmp.py

@@ -0,0 +1,123 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: ISC
+
+# Copyright (c) 2023 by
+# Donatas Abraitis <donatas@opensourcerouting.org>
+#
+
+"""
+Test if RIP `allow-ecmp` command works correctly.
+"""
+
+import os
+import sys
+import json
+import pytest
+import functools
+
+CWD = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(CWD, "../"))
+
+# pylint: disable=C0413
+from lib import topotest
+from lib.topogen import Topogen, TopoRouter, get_topogen
+
+pytestmark = [pytest.mark.ripd]
+
+
+def setup_module(mod):
+    topodef = {"s1": ("r1", "r2", "r3")}
+    tgen = Topogen(topodef, mod.__name__)
+    tgen.start_topology()
+
+    router_list = tgen.routers()
+
+    for _, (rname, router) in enumerate(router_list.items(), 1):
+        router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname)))
+
+    tgen.start_router()
+
+
+def teardown_module(mod):
+    tgen = get_topogen()
+    tgen.stop_topology()
+
+
+def test_rip_allow_ecmp():
+    tgen = get_topogen()
+
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r1 = tgen.gears["r1"]
+
+    def _show_rip_routes():
+        output = json.loads(r1.vtysh_cmd("show yang operational-data /frr-ripd:ripd ripd"))
+        try:
+            output = output["frr-ripd:ripd"]["instance"][0]["state"]["routes"]
+        except KeyError:
+            return False
+
+        expected = {
+            "route": [
+                {
+                    "prefix": "10.10.10.1/32",
+                    "nexthops": {
+                        "nexthop": [
+                            {
+                                "nh-type": "ip4",
+                                "protocol": "rip",
+                                "rip-type": "normal",
+                                "gateway": "192.168.1.2",
+                                "from": "192.168.1.2",
+                                "tag": 0,
+                            },
+                            {
+                                "nh-type": "ip4",
+                                "protocol": "rip",
+                                "rip-type": "normal",
+                                "gateway": "192.168.1.3",
+                                "from": "192.168.1.3",
+                                "tag": 0,
+                            },
+                        ]
+                    },
+                    "metric": 2,
+                    "next-hop": "192.168.1.2"
+                },
+            ]
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_show_rip_routes)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
+    assert result is None, "Can't see 10.10.10.1/32 as multipath in `show ip rip`"
+
+    def _show_routes():
+        output = json.loads(r1.vtysh_cmd("show ip route json"))
+        expected = {
+            "10.10.10.1/32": [
+                {
+                    "nexthops": [
+                        {
+                            "ip": "192.168.1.2",
+                            "active": True,
+                        },
+                        {
+                            "ip": "192.168.1.3",
+                            "active": True,
+                        },
+                    ]
+                }
+            ]
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_show_routes)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
+    assert result is None, "Can't see 10.10.10.1/32 as multipath in `show ip route`"
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    sys.exit(pytest.main(args))

yang/frr-ripd.yang

@@ -13,6 +13,9 @@ module frr-ripd {
   import frr-interface {
     prefix frr-interface;
   }
+  import frr-nexthop {
+    prefix frr-nexthop;
+  }
   import frr-vrf {
     prefix frr-vrf;
   }
@@ -60,6 +63,7 @@ module frr-ripd {
     description
       "Changed interface references to use
       frr-interface:interface-ref typedef";
+    reference "FRRouting";
   }
   revision 2017-12-06 {
     description
@@ -69,10 +73,35 @@ module frr-ripd {
        RFC 2453: RIP Version 2.";
   }
 
+  typedef rip-route-type {
+    type enumeration {
+      enum normal {
+        value 0;
+        description "Normal RIP route type.";
+      }
+      enum static {
+        value 1;
+        description "Static RIP route type.";
+      }
+      enum default {
+        value 2;
+        description "Default RIP route type.";
+      }
+      enum redistribute {
+        value 3;
+        description "Redistribute RIP route type.";
+      }
+      enum interface {
+        value 4;
+        description "Interface RIP route type.";
+      }
+    }
+    description
+      "Types of RIP routes.";
+  }
+
   container ripd {
-    /*
+    description "rip routing instance data";
-     * Routing instance configuration.
-     */
     list instance {
       key "vrf";
       description
@@ -229,9 +258,9 @@ module frr-ripd {
           "Redistributes routes learned from other routing protocols.";
         leaf protocol {
           type frr-route-types:frr-route-types-v4;
+          must '. != "rip"';
           description
             "Routing protocol.";
-          must '. != "rip"';
         }
         leaf route-map {
           type frr-route-map:route-map-ref;
@@ -291,11 +320,8 @@ module frr-ripd {
         }
       }
       container version {
+        description "version of rip";
         leaf receive {
-          must
-            '(. = "1" and ../send = "1") or ' +
-            '(. = "2" and ../send = "2") or ' +
-            '(. = "1-2" and ../send = "2")';
           type enumeration {
             enum "1" {
               value 1;
@@ -313,15 +339,15 @@ module frr-ripd {
                 "Accept both RIPv1 and RIPv2 updates.";
             }
           }
+          must
+            '(. = "1" and ../send = "1") or ' +
+            '(. = "2" and ../send = "2") or ' +
+            '(. = "1-2" and ../send = "2")';
           default "1-2";
           description
             "Advertisement reception - Version control.";
         }
         leaf send {
-          must
-            '(../receive = "1" and . = "1") or ' +
-            '(../receive = "2" and . = "2") or ' +
-            '(../receive = "1-2" and . = "2")';
           type enumeration {
             enum "1" {
               value 1;
@@ -334,6 +360,10 @@ module frr-ripd {
                 "Send RIPv2 updates only.";
             }
           }
+          must
+            '(../receive = "1" and . = "1") or ' +
+            '(../receive = "2" and . = "2") or ' +
+            '(../receive = "1-2" and . = "2")';
           default "2";
           description
             "Advertisement transmission - Version control.";
@@ -399,15 +429,58 @@ module frr-ripd {
                  separated by the slash (/) character. The range of
                  values for the prefix-length is 0 to 32.";
             }
-            leaf next-hop {
+            container nexthops {
-              type inet:ipv4-address;
+              description "container of nexthops";
-              description
+              list nexthop {
-                "Next hop IPv4 address.";
+                description "A list of nexthop objects.";
-            }
+                leaf nh-type {
-            leaf interface {
+                  type frr-nexthop:nexthop-type;
-              type frr-interface:interface-ref;
+                  mandatory true;
-              description
+                  description
-                "The interface that the route uses.";
+                    "The nexthop type.";
+                }
+                leaf protocol {
+                  type frr-route-types:frr-route-types-v4;
+                  description
+                    "The protocol originating this route.";
+                }
+                leaf rip-type {
+                  type rip-route-type;
+                  description
+                    "The RIP type of route.";
+                }
+                leaf gateway {
+                  type inet:ipv4-address;
+                  description
+                    "The nexthop gateway address.";
+                }
+                leaf interface {
+                  type frr-interface:interface-ref;
+                  description
+                    "The nexthop egress interface.";
+                }
+                leaf from {
+                  type inet:ipv4-address;
+                  description
+                    "The nexthop gateway address.";
+                }
+                leaf tag {
+                  type uint32;
+                  default "0";
+                  description
+                    "Route tag";
+                }
+                leaf external-metric {
+                  type uint32;
+                  description
+                    "External metric if learned from external protocol.";
+                }
+                leaf expire-time {
+                  type uint32;
+                  description
+                    "Seconds before route expires.";
+                }
+              }
             }
             leaf metric {
               type uint8 {
@@ -416,6 +489,21 @@ module frr-ripd {
               description
                 "Route metric.";
             }
+            /*
+             * Replaced by container `nexthops` above.
+             */
+            leaf next-hop {
+              type inet:ipv4-address;
+              status deprecated;
+              description
+                "Next hop IPv4 address.";
+            }
+            leaf interface {
+              type frr-interface:interface-ref;
+              status deprecated;
+              description
+                "The interface that the route uses.";
+            }
           }
         }
       }
@@ -426,6 +514,7 @@ module frr-ripd {
    * Per-interface configuration data
    */
   augment "/frr-interface:lib/frr-interface:interface" {
+    description "rip interface data";
     container rip {
       description
         "RIP interface parameters.";