Merge pull request #12688 from dorDiogo/isis_hello_padding_sometimes

isisd: Add support for IS-IS hello padding during-adjacency-formation

12 files changed, 200 insertions, 32 deletions

diff --git a/doc/user/isisd.rst b/doc/user/isisd.rst
index 055841d23..90c13d4f9 100644
--- a/doc/user/isisd.rst
+++ b/doc/user/isisd.rst
@@ -212,6 +212,10 @@ ISIS interface
 
    Add padding to IS-IS hello packets.
 
+.. clicmd:: isis hello padding during-adjacency-formation
+
+   Add padding to IS-IS hello packets during adjacency formation only.
+
 .. clicmd:: isis hello-interval (1-600) [level-1 | level-2]
 
    Set Hello interval in seconds globally, for an area (level-1) or a domain
diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c
index 8644da2f0..1ee7f4451 100644
--- a/isisd/isis_circuit.c
+++ b/isisd/isis_circuit.c
@@ -107,7 +107,7 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag)
 		"/frr-interface:lib/interface/frr-isisd:isis/circuit-type");
 	circuit->flags = 0;
 
-	circuit->pad_hellos = yang_get_default_bool(
+	circuit->pad_hellos = yang_get_default_enum(
 		"/frr-interface:lib/interface/frr-isisd:isis/hello/padding");
 	circuit->hello_interval[0] = yang_get_default_uint32(
 		"/frr-interface:lib/interface/frr-isisd:isis/hello/interval/level-1");
@@ -145,7 +145,7 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag)
 #else
 	circuit->is_type_config = IS_LEVEL_1_AND_2;
 	circuit->flags = 0;
-	circuit->pad_hellos = 1;
+	circuit->pad_hellos = ISIS_HELLO_PADDING_ALWAYS;
 	for (i = 0; i < 2; i++) {
 		circuit->hello_interval[i] = DEFAULT_HELLO_INTERVAL;
 		circuit->hello_multiplier[i] = DEFAULT_HELLO_MULTIPLIER;
@@ -1029,7 +1029,8 @@ void isis_circuit_print_json(struct isis_circuit *circuit,
 					circuit->hello_multiplier[0]);
 				json_object_string_add(
 					hold_json, "pad",
-					(circuit->pad_hellos ? "yes" : "no"));
+					isis_hello_padding2string(
+						circuit->pad_hellos));
 				json_object_int_add(level_json, "cnsp-interval",
 						    circuit->csnp_interval[0]);
 				json_object_int_add(level_json, "psnp-interval",
@@ -1137,11 +1138,11 @@ void isis_circuit_print_vty(struct isis_circuit *circuit, struct vty *vty,
 				vty_out(vty, ", Active neighbors: %u\n",
 					circuit->upadjcount[0]);
 				vty_out(vty,
-					"      Hello interval: %u, Holddown count: %u %s\n",
+					"      Hello interval: %u, Holddown count: %u, Padding: %s\n",
 					circuit->hello_interval[0],
 					circuit->hello_multiplier[0],
-					(circuit->pad_hellos ? "(pad)"
-							     : "(no-pad)"));
+					isis_hello_padding2string(
+						circuit->pad_hellos));
 				vty_out(vty,
 					"      CNSP interval: %u, PSNP interval: %u\n",
 					circuit->csnp_interval[0],
@@ -1169,11 +1170,11 @@ void isis_circuit_print_vty(struct isis_circuit *circuit, struct vty *vty,
 				vty_out(vty, ", Active neighbors: %u\n",
 					circuit->upadjcount[1]);
 				vty_out(vty,
-					"      Hello interval: %u, Holddown count: %u %s\n",
+					"      Hello interval: %u, Holddown count: %u, Padding: %s\n",
 					circuit->hello_interval[1],
 					circuit->hello_multiplier[1],
-					(circuit->pad_hellos ? "(pad)"
-							     : "(no-pad)"));
+					isis_hello_padding2string(
+						circuit->pad_hellos));
 				vty_out(vty,
 					"      CNSP interval: %u, PSNP interval: %u\n",
 					circuit->csnp_interval[1],
@@ -1319,11 +1320,20 @@ static int isis_interface_config_write(struct vty *vty)
 				}
 			}
 
-			/* ISIS - Hello padding - Defaults to true so only
-			 * display if false */
-			if (circuit->pad_hellos == 0) {
+			/* ISIS - Hello padding - Defaults to always so only
+			 * display if not always */
+			switch (circuit->pad_hellos) {
+			case ISIS_HELLO_PADDING_DISABLED:
 				vty_out(vty, " no " PROTO_NAME " hello padding\n");
 				write++;
+				break;
+			case ISIS_HELLO_PADDING_DURING_ADJACENCY_FORMATION:
+				vty_out(vty, PROTO_NAME
+					" hello padding during-adjacency-formation\n");
+				write++;
+				break;
+			case ISIS_HELLO_PADDING_ALWAYS:
+				break;
 			}
 
 			if (circuit->disable_threeway_adj) {
diff --git a/isisd/isis_circuit.h b/isisd/isis_circuit.h
index 494e96b69..df977893e 100644
--- a/isisd/isis_circuit.h
+++ b/isisd/isis_circuit.h
@@ -63,6 +63,15 @@ struct isis_circuit_arg {
 	struct isis_circuit *circuit;
 };
 
+/*
+ * Hello padding types
+ */
+enum isis_hello_padding {
+	ISIS_HELLO_PADDING_ALWAYS,
+	ISIS_HELLO_PADDING_DISABLED,
+	ISIS_HELLO_PADDING_DURING_ADJACENCY_FORMATION
+};
+
 struct isis_circuit {
 	enum isis_circuit_state state;
 	uint8_t circuit_id;	  /* l1/l2 bcast CircuitID */
@@ -100,7 +109,7 @@ struct isis_circuit {
 		struct isis_p2p_info p2p;
 	} u;
 	uint8_t priority[ISIS_LEVELS]; /* l1/2 IS configured priority */
-	int pad_hellos;     /* add padding to Hello PDUs ? */
+	enum isis_hello_padding pad_hellos; /* type of Hello PDUs padding */
 	char ext_domain;    /* externalDomain   (boolean) */
 	int lsp_regenerate_pending[ISIS_LEVELS];
 	uint64_t lsp_error_counter;
diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c
index 4a598aa8c..7e1bb9255 100644
--- a/isisd/isis_cli.c
+++ b/isisd/isis_cli.c
@@ -2262,15 +2262,22 @@ void cli_show_ip_isis_threeway_shake(struct vty *vty,
 /*
  * XPath: /frr-interface:lib/interface/frr-isisd:isis/hello/padding
  */
-DEFPY_YANG(isis_hello_padding, isis_hello_padding_cmd, "[no] isis hello padding",
-      NO_STR
-      "IS-IS routing protocol\n"
-      "Add padding to IS-IS hello packets\n"
-      "Pad hello packets\n")
+DEFPY_YANG(isis_hello_padding, isis_hello_padding_cmd,
+	   "[no] isis hello padding [during-adjacency-formation]$padding_type",
+	   NO_STR
+	   "IS-IS routing protocol\n"
+	   "Type of padding for IS-IS hello packets\n"
+	   "Pad hello packets\n"
+	   "Add padding to hello packets during adjacency formation only.\n")
 {
-	nb_cli_enqueue_change(vty, "./frr-isisd:isis/hello/padding",
-			      NB_OP_MODIFY, no ? "false" : "true");
-
+	if (no) {
+		nb_cli_enqueue_change(vty, "./frr-isisd:isis/hello/padding",
+				      NB_OP_MODIFY, "disabled");
+	} else {
+		nb_cli_enqueue_change(vty, "./frr-isisd:isis/hello/padding",
+				      NB_OP_MODIFY,
+				      padding_type ? padding_type : "always");
+	}
 	return nb_cli_apply_changes(vty, NULL);
 }
 
@@ -2278,10 +2285,13 @@ void cli_show_ip_isis_hello_padding(struct vty *vty,
 				    const struct lyd_node *dnode,
 				    bool show_defaults)
 {
-	if (!yang_dnode_get_bool(dnode, NULL))
+	int hello_padding_type = yang_dnode_get_enum(dnode, NULL);
+	if (hello_padding_type == ISIS_HELLO_PADDING_DISABLED)
 		vty_out(vty, " no");
-
-	vty_out(vty, " isis hello padding\n");
+	vty_out(vty, " isis hello padding");
+	if (hello_padding_type == ISIS_HELLO_PADDING_DURING_ADJACENCY_FORMATION)
+		vty_out(vty, " during-adjacency-formation");
+	vty_out(vty, "\n");
 }
 
 /*
diff --git a/isisd/isis_misc.c b/isisd/isis_misc.c
index 4a490ab5a..95d24036e 100644
--- a/isisd/isis_misc.c
+++ b/isisd/isis_misc.c
@@ -294,6 +294,19 @@ const char *syst2string(int type)
 	return NULL; /* not reached */
 }
 
+const char *isis_hello_padding2string(int hello_padding_type)
+{
+	switch (hello_padding_type) {
+	case ISIS_HELLO_PADDING_DISABLED:
+		return "no";
+	case ISIS_HELLO_PADDING_DURING_ADJACENCY_FORMATION:
+		return "during-adjacency-formation";
+	case ISIS_HELLO_PADDING_ALWAYS:
+		return "yes";
+	}
+	return NULL; /* not reached */
+}
+
 /*
  * Print functions - we print to static vars
  */
diff --git a/isisd/isis_misc.h b/isisd/isis_misc.h
index b8b4ebd28..01d9abe86 100644
--- a/isisd/isis_misc.h
+++ b/isisd/isis_misc.h
@@ -16,6 +16,7 @@ const char *circuit_t2string(int);
 const char *circuit_state2string(int state);
 const char *circuit_type2string(int type);
 const char *syst2string(int);
+const char *isis_hello_padding2string(int hello_padding_type);
 struct in_addr newprefix2inaddr(uint8_t *prefix_start, uint8_t prefix_masklen);
 /*
  * Converting input to memory stored format
diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c
index 2b3355bc9..3817465a6 100644
--- a/isisd/isis_nb_config.c
+++ b/isisd/isis_nb_config.c
@@ -2783,7 +2783,7 @@ int lib_interface_isis_hello_padding_modify(struct nb_cb_modify_args *args)
 		return NB_OK;
 
 	circuit = nb_running_get_entry(args->dnode, NULL, true);
-	circuit->pad_hellos = yang_dnode_get_bool(args->dnode, NULL);
+	circuit->pad_hellos = yang_dnode_get_enum(args->dnode, NULL);
 
 	return NB_OK;
 }
diff --git a/isisd/isis_pdu.c b/isisd/isis_pdu.c
index 70ec2426d..f659f3abc 100644
--- a/isisd/isis_pdu.c
+++ b/isisd/isis_pdu.c
@@ -1964,8 +1964,14 @@ int send_hello(struct isis_circuit *circuit, int level)
 		isis_tlvs_add_global_ipv6_addresses(tlvs,
 						    circuit->ipv6_non_link);
 
+	bool should_pad_hello =
+		circuit->pad_hellos == ISIS_HELLO_PADDING_ALWAYS ||
+		(circuit->pad_hellos ==
+			 ISIS_HELLO_PADDING_DURING_ADJACENCY_FORMATION &&
+		 circuit->upadjcount[0] + circuit->upadjcount[1] == 0);
+
 	if (isis_pack_tlvs(tlvs, circuit->snd_stream, len_pointer,
-			   circuit->pad_hellos, false)) {
+			   should_pad_hello, false)) {
 		isis_free_tlvs(tlvs);
 		return ISIS_WARNING; /* XXX: Maybe Log TLV structure? */
 	}
diff --git a/isisd/isis_snmp.c b/isisd/isis_snmp.c
index ae570a086..e0bcb180b 100644
--- a/isisd/isis_snmp.c
+++ b/isisd/isis_snmp.c
@@ -2161,7 +2161,10 @@ static uint8_t *isis_snmp_find_circ(struct variable *v, oid *name,
 		/*
 		 * return false if lan hellos must be padded
 		 */
-		if (circuit->pad_hellos)
+		if (circuit->pad_hellos == ISIS_HELLO_PADDING_ALWAYS ||
+		    (circuit->pad_hellos ==
+			     ISIS_HELLO_PADDING_DURING_ADJACENCY_FORMATION &&
+		     circuit->upadjcount[0] + circuit->upadjcount[1] == 0))
 			return SNMP_INTEGER(ISIS_SNMP_TRUTH_VALUE_FALSE);
 
 		return SNMP_INTEGER(ISIS_SNMP_TRUTH_VALUE_TRUE);
diff --git a/tests/topotests/all_protocol_startup/r1/show_isis_interface_detail.ref b/tests/topotests/all_protocol_startup/r1/show_isis_interface_detail.ref
index 0534b64d3..865970065 100644
--- a/tests/topotests/all_protocol_startup/r1/show_isis_interface_detail.ref
+++ b/tests/topotests/all_protocol_startup/r1/show_isis_interface_detail.ref
@@ -3,7 +3,7 @@ Area test:
     Type: lan, Level: L1, SNPA: XXXX.XXXX.XXXX
     Level-1 Information:
       Metric: 10, Active neighbors: 0
-      Hello interval: 3, Holddown count: 10 (pad)
+      Hello interval: 3, Holddown count: 10, Padding: yes
       CNSP interval: 10, PSNP interval: 2
       LAN Priority: 64, is not DIS
     IP Prefix(es):
@@ -17,7 +17,7 @@ Area test:
     Type: lan, Level: L2, SNPA: XXXX.XXXX.XXXX
     Level-2 Information:
       Metric: 10, Active neighbors: 0
-      Hello interval: 3, Holddown count: 10 (pad)
+      Hello interval: 3, Holddown count: 10, Padding: yes
       CNSP interval: 10, PSNP interval: 2
       LAN Priority: 64, is not DIS
     IP Prefix(es):
diff --git a/tests/topotests/isis_topo1/test_isis_topo1.py b/tests/topotests/isis_topo1/test_isis_topo1.py
index baacba613..48e9d5336 100644
--- a/tests/topotests/isis_topo1/test_isis_topo1.py
+++ b/tests/topotests/isis_topo1/test_isis_topo1.py
@@ -572,6 +572,98 @@ def test_isis_advertise_passive_only():
     assert result is True, result
 
 
+def test_isis_hello_padding_during_adjacency_formation():
+    """Check that IIH packets is only padded when adjacency is still being formed
+    when isis hello padding during-adjacency-formation is configured
+    """
+    tgen = get_topogen()
+    net = get_topogen().net
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    logger.info("Testing isis hello padding during-adjacency-formation behavior")
+    r3 = tgen.gears["r3"]
+
+    # Reduce hello-multiplier to make the adjacency go down faster.
+    r3.vtysh_cmd(
+        """
+        configure
+        interface r3-eth0
+            isis hello-multiplier 2
+        """
+    )
+
+    r1 = tgen.gears["r1"]
+    cmd_output = r1.vtysh_cmd(
+        """
+        configure
+        interface r1-eth0
+            isis hello padding during-adjacency-formation
+        end
+        debug isis adj-packets
+        """
+    )
+    result = check_last_iih_packet_for_padding(r1, expect_padding=False)
+    assert result is True, result
+
+    r3.vtysh_cmd(
+        """
+        configure
+        interface r3-eth0
+            shutdown
+        """
+    )
+    result = check_last_iih_packet_for_padding(r1, expect_padding=True)
+    assert result is True, result
+
+    r3 = tgen.gears["r3"]
+    r3.vtysh_cmd(
+        """
+        configure
+        interface r3-eth0
+            no shutdown
+        """
+    )
+    result = check_last_iih_packet_for_padding(r1, expect_padding=False)
+    assert result is True, result
+
+
+@retry(retry_timeout=5)
+def check_last_iih_packet_for_padding(router, expect_padding):
+    logfilename = "{}/{}".format(router.gearlogdir, "isisd.log")
+    last_hello_packet_line = None
+    with open(logfilename, "r") as f:
+        lines = f.readlines()
+        for line in lines:
+            if re.search("Sending .+? IIH", line):
+                last_hello_packet_line = line
+
+    if last_hello_packet_line is None:
+        return "Expected IIH packet in {}, but no packet found".format(logfilename)
+
+    interface_name, packet_length = re.search(
+        r"Sending .+ IIH on (.+), length (\d+)", last_hello_packet_line
+    ).group(1, 2)
+    packet_length = int(packet_length)
+    interface_output = router.vtysh_cmd("show interface {} json".format(interface_name))
+    interface_json = json.loads(interface_output)
+    padded_packet_length = interface_json[interface_name]["mtu"] - 3
+    if expect_padding:
+        if packet_length == padded_packet_length:
+            return True
+        return (
+            "Expected padded packet with length {}, got packet with length {}".format(
+                padded_packet_length, packet_length
+            )
+        )
+    if packet_length < padded_packet_length:
+        return True
+    return "Expected unpadded packet with length less than {}, got packet with length {}".format(
+        padded_packet_length, packet_length
+    )
+
+
 @retry(retry_timeout=5)
 def check_advertised_prefixes(router, lsp_id, expected_prefixes):
     output = router.vtysh_cmd("show isis database detail {}".format(lsp_id))
diff --git a/yang/frr-isisd.yang b/yang/frr-isisd.yang
index 0c2cf232f..66ec6a410 100644
--- a/yang/frr-isisd.yang
+++ b/yang/frr-isisd.yang
@@ -116,6 +116,26 @@ module frr-isisd {
        associated with an interface.";
   }
 
+  typedef hello-padding-type {
+    type enumeration {
+      enum "always" {
+        value 0;
+        description
+          "Add padding to all hello packets.";
+      }
+      enum "disabled" {
+        value 1;
+        description
+          "Do not add any padding to hello packets.";
+      } 
+      enum "during-adjacency-formation" {
+        value 2;
+        description
+          "Add padding to hello packets during adjacency formation only.";
+      }
+    }
+  }
+
   typedef network-type {
     type enumeration {
       enum "unknown" {
@@ -605,10 +625,10 @@ module frr-isisd {
       description
         "Parameters related to IS-IS hello PDUs.";
       leaf padding {
-        type boolean;
-        default "true";
+        type hello-padding-type;
+        default "always";
         description
-          "Add padding to IS-IS hello PDUs.";
+          "Type of padding for IS-IS hello packets.";
       }
 
       container interval {