ospf6d: fix GR & auth seqno state location

Unfortunately, `ospf6d` is much worse than `ospfd` and `isisd` regarding its state saving, due to the existence of the auth trailer code. Again, this belongs in `/var/lib`, not `/var/run`. Merge both state files into one, and add reconciliation code for the auth seqno. I'm gonna save my comment on the fact that `ospf6_auth_seqno_nvm_delete` is not in fact used anywhere. Which is now a warning because it's `static`. Well. It probably should be used somewhere, so leave it in. Signed-off-by: David Lamparter <[email protected]>
FRRouting · Jan 27, 2024 · 567f570 · 567f570
1 parent 110945b
commit 567f570
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 76 deletions.
diff --git a/configure.ac b/configure.ac
@@ -2741,8 +2741,6 @@ AC_DEFINE_UNQUOTED([FRR_LIBSTATE_PATH], ["$CFG_LIBSTATE"], [/var/lib/frr equival
 AC_DEFINE_UNQUOTED([YANG_MODELS_PATH], ["$CFG_YANGMODELS"], [path to YANG data models])
 AC_DEFINE_UNQUOTED([WATCHFRR_SH_PATH], ["${CFG_SBIN%/}/watchfrr.sh"], [path to watchfrr.sh])
 
-AC_DEFINE_UNQUOTED([OSPF6D_GR_STATE], ["$CFG_STATE/ospf6d-gr.json"], [ospf6d GR state information])
-AC_DEFINE_UNQUOTED([OSPF6_AUTH_SEQ_NUM_FILE], ["$CFG_STATE/ospf6d-at-seq-no.dat"], [ospf6d AT Sequence number information])
 AC_DEFINE_UNQUOTED([DAEMON_DB_DIR], ["$CFG_STATE"], [daemon database directory])
 
 dnl various features

diff --git a/ospf6d/ospf6_auth_trailer.c b/ospf6d/ospf6_auth_trailer.c
@@ -30,9 +30,13 @@
 #include "ospf6_zebra.h"
 #include "lib/keychain.h"
 
+#define OSPF6D_COMPAT_AUTHSEQ_NAME "%s/ospf6d-at-seq-no.dat", frr_runstatedir
+
 unsigned char conf_debug_ospf6_auth[2];
 DEFINE_MTYPE_STATIC(OSPF6D, OSPF6_AUTH_HASH_XOR, "OSPF6 auth hash xor");
 
+static void ospf6_auth_seqno_nvm_update(struct ospf6 *ospf6);
+
 /*Apad is the hexadecimal value 0x878FE1F3. */
 const uint8_t ospf6_hash_apad_max[KEYCHAIN_MAX_HASH_SIZE] = {
 	0x87, 0x8f, 0xe1, 0xf3, 0x87, 0x8f, 0xe1, 0xf3, 0x87, 0x8f, 0xe1,
@@ -862,23 +866,11 @@ void install_element_ospf6_clear_intf_auth(void)
 	install_element(ENABLE_NODE, &clear_ipv6_ospf6_intf_auth_cmd);
 }
 
-enum ospf6_auth_err ospf6_auth_nvm_file_exist(void)
-{
-	struct stat buffer;
-	int exist;
-
-	exist = stat(OSPF6_AUTH_SEQ_NUM_FILE, &buffer);
-	if (exist == 0)
-		return OSPF6_AUTH_FILE_EXIST;
-	else
-		return OSPF6_AUTH_FILE_DO_NOT_EXIST;
-}
-
 /*
  * Record in non-volatile memory the given ospf6 process,
  * authentication trailer higher order sequence number.
  */
-void ospf6_auth_seqno_nvm_update(struct ospf6 *ospf6)
+static void ospf6_auth_seqno_nvm_update(struct ospf6 *ospf6)
 {
 	const char *inst_name;
 	json_object *json;
@@ -890,9 +882,7 @@ void ospf6_auth_seqno_nvm_update(struct ospf6 *ospf6)
 
 	inst_name = ospf6->name ? ospf6->name : VRF_DEFAULT_NAME;
 
-	json = json_object_from_file((char *)OSPF6_AUTH_SEQ_NUM_FILE);
-	if (json == NULL)
-		json = json_object_new_object();
+	json = frr_daemon_state_load();
 
 	json_object_object_get_ex(json, "instances", &json_instances);
 	if (!json_instances) {
@@ -912,49 +902,82 @@ void ospf6_auth_seqno_nvm_update(struct ospf6 *ospf6)
 	 */
 	json_object_int_add(json_instance, "sequence_number", ospf6->seqnum_h);
 
-	json_object_to_file_ext((char *)OSPF6_AUTH_SEQ_NUM_FILE, json,
-				JSON_C_TO_STRING_PRETTY);
-	json_object_free(json);
+	frr_daemon_state_save(&json);
 }
 
 /*
  * Delete authentication sequence number for a given OSPF6 process
  * from non-volatile memory.
  */
-void ospf6_auth_seqno_nvm_delete(struct ospf6 *ospf6)
+__attribute__((unused)) static void
+ospf6_auth_seqno_nvm_delete(struct ospf6 *ospf6)
 {
 	const char *inst_name;
 	json_object *json;
 	json_object *json_instances;
+	json_object *json_instance;
 
 	zlog_err("Higher order sequence number delete for %s process",
 		 ospf6->name);
 
 	inst_name = ospf6->name ? ospf6->name : VRF_DEFAULT_NAME;
 
-	json = json_object_from_file((char *)OSPF6_AUTH_SEQ_NUM_FILE);
-	if (json == NULL)
-		json = json_object_new_object();
+	json = frr_daemon_state_load();
 
 	json_object_object_get_ex(json, "instances", &json_instances);
 	if (!json_instances) {
-		json_instances = json_object_new_object();
-		json_object_object_add(json, "instances", json_instances);
+		json_object_put(json);
+		return;
+	}
+
+	json_object_object_get_ex(json_instances, inst_name, &json_instance);
+	if (json_instance) {
+		json_object_put(json);
+		return;
 	}
 
-	json_object_object_del(json_instances, inst_name);
+	json_object_object_del(json_instance, "sequence_number");
 
-	json_object_to_file_ext((char *)OSPF6_AUTH_SEQ_NUM_FILE, json,
-				JSON_C_TO_STRING_PRETTY);
-	json_object_free(json);
+	frr_daemon_state_save(&json);
 }
 
 
+static struct json_object *ospf6_auth_seqno_compat_read(const char *inst_name)
+{
+	/* try legacy location */
+	char compat_path[512];
+	json_object *json;
+	json_object *json_instances = NULL;
+	json_object *json_instance = NULL;
+	json_object *json_seqnum = NULL;
+
+	snprintf(compat_path, sizeof(compat_path), OSPF6D_COMPAT_AUTHSEQ_NAME);
+	json = json_object_from_file(compat_path);
+
+	if (json)
+		json_object_object_get_ex(json, "instances", &json_instances);
+	if (json_instances)
+		json_object_object_get_ex(json_instances, inst_name,
+					  &json_instance);
+	if (json_instance)
+		json_object_object_get_ex(json_instance, "sequence_number",
+					  &json_seqnum);
+	if (json_seqnum)
+		/* => free the file-level object and still return this */
+		json_seqnum = json_object_get(json_seqnum);
+
+	if (json) {
+		json_object_free(json);
+		unlink(compat_path);
+	}
+	return json_seqnum;
+}
+
 /*
  * Fetch from non-volatile memory the stored ospf6 process
  * authentication sequence number.
  */
-void ospf6_auth_seqno_nvm_read(struct ospf6 *ospf6)
+static void ospf6_auth_seqno_nvm_read(struct ospf6 *ospf6)
 {
 	const char *inst_name;
 	json_object *json;
@@ -964,9 +987,7 @@ void ospf6_auth_seqno_nvm_read(struct ospf6 *ospf6)
 
 	inst_name = ospf6->name ? ospf6->name : VRF_DEFAULT_NAME;
 
-	json = json_object_from_file((char *)OSPF6_AUTH_SEQ_NUM_FILE);
-	if (json == NULL)
-		json = json_object_new_object();
+	json = frr_daemon_state_load();
 
 	json_object_object_get_ex(json, "instances", &json_instances);
 	if (!json_instances) {
@@ -983,13 +1004,34 @@ void ospf6_auth_seqno_nvm_read(struct ospf6 *ospf6)
 
 	json_object_object_get_ex(json_instance, "sequence_number",
 				  &json_seqnum);
-	ospf6->seqnum_h = json_object_get_int(json_seqnum);
+
+	if (json_seqnum)
+		/* cf. reference taken in compat_read above */
+		json_seqnum = json_object_get(json_seqnum);
+	else
+		json_seqnum = ospf6_auth_seqno_compat_read(inst_name);
+
+	ospf6->seqnum_l = 0;
+	if (json_seqnum) {
+		ospf6->seqnum_h = json_object_get_int(json_seqnum);
+		ospf6->seqnum_h += 1;
+	} else {
+		ospf6->seqnum_h = 0;
+	}
+
+	if (json_seqnum)
+		json_object_put(json_seqnum);
 
 	zlog_err("Higher order sequence number %d read for %s process %s",
 		 ospf6->seqnum_h, ospf6->name, strerror(errno));
 
-	json_object_object_del(json_instances, inst_name);
-	json_object_to_file_ext((char *)OSPF6_AUTH_SEQ_NUM_FILE, json,
-				JSON_C_TO_STRING_PRETTY);
-	json_object_free(json);
+	json_object_object_del(json_instance, "sequence_number");
+
+	frr_daemon_state_save(&json);
+}
+
+void ospf6_auth_init(struct ospf6 *o)
+{
+	ospf6_auth_seqno_nvm_read(o);
+	ospf6_auth_seqno_nvm_update(o);
 }
diff --git a/ospf6d/ospf6_auth_trailer.h b/ospf6d/ospf6_auth_trailer.h
@@ -48,10 +48,10 @@ enum ospf6_auth_err {
 	OSPF6_AUTH_VALIDATE_SUCCESS = 0,
 	OSPF6_AUTH_VALIDATE_FAILURE,
 	OSPF6_AUTH_PROCESS_NORMAL,
-	OSPF6_AUTH_FILE_EXIST,
-	OSPF6_AUTH_FILE_DO_NOT_EXIST
 };
 
+void ospf6_auth_init(struct ospf6 *o);
+
 void ospf6_auth_hdr_dump_send(struct ospf6_header *ospfh, uint16_t length);
 void ospf6_auth_hdr_dump_recv(struct ospf6_header *ospfh, uint16_t length,
 			      unsigned int lls_len);
@@ -73,8 +73,5 @@ void ospf6_auth_digest_send(struct in6_addr *src, struct ospf6_interface *oi,
 void install_element_ospf6_debug_auth(void);
 int config_write_ospf6_debug_auth(struct vty *vty);
 void install_element_ospf6_clear_intf_auth(void);
-enum ospf6_auth_err ospf6_auth_nvm_file_exist(void);
-void ospf6_auth_seqno_nvm_update(struct ospf6 *ospf6);
-void ospf6_auth_seqno_nvm_delete(struct ospf6 *ospf6);
-void ospf6_auth_seqno_nvm_read(struct ospf6 *ospf6);
+
 #endif /* __OSPF6_AUTH_TRAILER_H__ */
diff --git a/ospf6d/ospf6_gr.c b/ospf6d/ospf6_gr.c
@@ -561,9 +561,7 @@ static void ospf6_gr_nvm_update(struct ospf6 *ospf6, bool prepare)
 
 	inst_name = ospf6->name ? ospf6->name : VRF_DEFAULT_NAME;
 
-	json = json_object_from_file((char *)OSPF6D_GR_STATE);
-	if (json == NULL)
-		json = json_object_new_object();
+	json = frr_daemon_state_load();
 
 	json_object_object_get_ex(json, "instances", &json_instances);
 	if (!json_instances) {
@@ -591,9 +589,7 @@ static void ospf6_gr_nvm_update(struct ospf6 *ospf6, bool prepare)
 		json_object_int_add(json_instance, "timestamp",
 				    time(NULL) + ospf6->gr_info.grace_period);
 
-	json_object_to_file_ext((char *)OSPF6D_GR_STATE, json,
-				JSON_C_TO_STRING_PRETTY);
-	json_object_free(json);
+	frr_daemon_state_save(&json);
 }
 
 /*
@@ -608,9 +604,7 @@ void ospf6_gr_nvm_delete(struct ospf6 *ospf6)
 
 	inst_name = ospf6->name ? ospf6->name : VRF_DEFAULT_NAME;
 
-	json = json_object_from_file((char *)OSPF6D_GR_STATE);
-	if (json == NULL)
-		json = json_object_new_object();
+	json = frr_daemon_state_load();
 
 	json_object_object_get_ex(json, "instances", &json_instances);
 	if (!json_instances) {
@@ -620,9 +614,7 @@ void ospf6_gr_nvm_delete(struct ospf6 *ospf6)
 
 	json_object_object_del(json_instances, inst_name);
 
-	json_object_to_file_ext((char *)OSPF6D_GR_STATE, json,
-				JSON_C_TO_STRING_PRETTY);
-	json_object_free(json);
+	frr_daemon_state_save(&json);
 }
 
 /*
@@ -641,9 +633,7 @@ void ospf6_gr_nvm_read(struct ospf6 *ospf6)
 
 	inst_name = ospf6->name ? ospf6->name : VRF_DEFAULT_NAME;
 
-	json = json_object_from_file((char *)OSPF6D_GR_STATE);
-	if (json == NULL)
-		json = json_object_new_object();
+	json = frr_daemon_state_load();
 
 	json_object_object_get_ex(json, "instances", &json_instances);
 	if (!json_instances) {
@@ -687,11 +677,10 @@ void ospf6_gr_nvm_read(struct ospf6 *ospf6)
 					       ospf6->gr_info.grace_period);
 	}
 
-	json_object_object_del(json_instances, inst_name);
+	json_object_object_del(json_instance, "gracePeriod");
+	json_object_object_del(json_instance, "timestamp");
 
-	json_object_to_file_ext((char *)OSPF6D_GR_STATE, json,
-				JSON_C_TO_STRING_PRETTY);
-	json_object_free(json);
+	frr_daemon_state_save(&json);
 }
 
 void ospf6_gr_unplanned_start_interface(struct ospf6_interface *oi)

diff --git a/ospf6d/ospf6_main.c b/ospf6d/ospf6_main.c
@@ -38,6 +38,14 @@
 /* Default configuration file name for ospf6d. */
 #define OSPF6_DEFAULT_CONFIG       "ospf6d.conf"
 
+/* GR and auth trailer persistent state */
+#define OSPF6D_STATE_NAME	 "%s/ospf6d.json", frr_libstatedir
+#define OSPF6D_COMPAT_STATE_NAME "%s/ospf6d-gr.json", frr_runstatedir
+/* for extra confusion, "ospf6d-at-seq-no.dat" is handled directly in
+ * ospf6_auth_trailer.c;  the alternative would be somehow merging JSON which
+ * is excessive for just supporting a legacy compatibility file location
+ */
+
 /* Default port values. */
 #define OSPF6_VTY_PORT             2606
 
@@ -166,6 +174,15 @@ static const struct frr_yang_module_info *const ospf6d_yang_modules[] = {
 	&frr_ospf6_route_map_info,
 };
 
+/* actual paths filled in main() */
+static char state_path[512];
+static char state_compat_path[512];
+static char *state_paths[] = {
+	state_path,
+	state_compat_path,
+	NULL,
+};
+
 /* clang-format off */
 FRR_DAEMON_INFO(ospf6d, OSPF6,
 	.vty_port = OSPF6_VTY_PORT,
@@ -178,6 +195,8 @@ FRR_DAEMON_INFO(ospf6d, OSPF6,
 
 	.yang_modules = ospf6d_yang_modules,
 	.n_yang_modules = array_size(ospf6d_yang_modules),
+
+	.state_paths = state_paths,
 );
 /* clang-format on */
 
@@ -237,6 +256,10 @@ int main(int argc, char *argv[], char *envp[])
 		exit(1);
 	}
 
+	snprintf(state_path, sizeof(state_path), OSPF6D_STATE_NAME);
+	snprintf(state_compat_path, sizeof(state_compat_path),
+		 OSPF6D_COMPAT_STATE_NAME);
+
 	/* OSPF6 master init. */
 	ospf6_master_init(frr_init());
 

diff --git a/ospf6d/ospf6_top.c b/ospf6d/ospf6_top.c
@@ -430,17 +430,7 @@ static struct ospf6 *ospf6_create(const char *name)
 	/* Make ospf protocol socket. */
 	ospf6_serv_sock(o);
 
-	/* If sequence number is stored in persistent storage, read it.
-	 */
-	if (ospf6_auth_nvm_file_exist() == OSPF6_AUTH_FILE_EXIST) {
-		ospf6_auth_seqno_nvm_read(o);
-		o->seqnum_h = o->seqnum_h + 1;
-		ospf6_auth_seqno_nvm_update(o);
-	} else {
-		o->seqnum_l = o->seqnum_h = 0;
-		ospf6_auth_seqno_nvm_update(o);
-	}
-
+	ospf6_auth_init(o);
 	return o;
 }