Merge pull request #16361 from opensourcerouting/ospfd-state-loc-snafu

ospfd: fix state location mixup

lib/libfrr.h

@@ -222,10 +222,39 @@ extern void frr_fini(void);
 
 extern char config_default[512];
 extern char frr_zclientpath[512];
+
+/* refer to lib/config_paths.h (generated during ./configure) for build config
+ * values of the following:
+ */
+
+/* sysconfdir is generally /etc/frr/, some BSDs may use /usr/local/etc/frr/.
+ * Will NOT include "pathspace" (namespace) suffix from -N. (libfrr.c handles
+ * pathspace'ing config files.)  Has a slash at the end for "historical"
+ * reasons.
+ */
 extern const char frr_sysconfdir[];
+
+/* runstatedir is *ephemeral* across reboots.  It may either be a ramdisk,
+ * or be wiped during boot.  Use only for pid files, sockets, and the like,
+ * not state.  Commonly /run/frr or /var/run/frr.
+ * Will include "pathspace" (namespace) suffix from -N.
+ */
 extern char frr_runstatedir[256];
+
+/* libstatedir is *persistent*.  It's the place to put state like sequence
+ * numbers or databases.  Commonly /var/lib/frr.
+ * Will include "pathspace" (namespace) suffix from -N.
+ */
 extern char frr_libstatedir[256];
+
+/* moduledir is something along the lines of /usr/lib/frr/modules or
+ * /usr/lib/x86_64-linux-gnu/frr/modules.  It is not guaranteed to be a
+ * subdirectory of the directory that the daemon binaries reside in.  (e.g.
+ * the "x86_64-linux-gnu" component will be absent from daemon paths.)
+ */
 extern const char frr_moduledir[];
+
+/* scriptdir is for Lua scripts, generally ${frr_sysconfdir}/scripts */
 extern const char frr_scriptdir[];
 
 extern char frr_protoname[];

ospfd/ospf_main.c

@@ -48,14 +48,20 @@
 #include "ospfd/ospf_apiserver.h"
 
 #define OSPFD_STATE_NAME	 "%s/ospfd.json", frr_libstatedir
-#define OSPFD_INST_STATE_NAME(i) "%s/ospfd-%d.json", frr_runstatedir, i
+#define OSPFD_INST_STATE_NAME(i) "%s/ospfd-%d.json", frr_libstatedir, i
 
 /* this one includes the path... because the instance number was in the path
  * before :( ... which totally didn't have a mkdir anywhere.
+ *
+ * ... and libstatedir & runstatedir got switched around while changing this;
+ * for non-instance it read the wrong path, for instance it wrote the wrong
+ * path.  (There is no COMPAT2 for non-instance because it was writing to the
+ * right place, i.e. no extra path to check exists from reading a wrong path.)
  */
-#define OSPFD_COMPAT_STATE_NAME "%s/ospfd-gr.json", frr_libstatedir
+#define OSPFD_COMPAT_STATE_NAME "%s/ospfd-gr.json", frr_runstatedir
-#define OSPFD_COMPAT_INST_STATE_NAME(i)                                        \
+#define OSPFD_COMPAT1_INST_STATE_NAME(i)                                       \
 	"%s-%d/ospfd-gr.json", frr_runstatedir, i
+#define OSPFD_COMPAT2_INST_STATE_NAME(i) "%s/ospfd-%d.json", frr_runstatedir, i
 
 /* ospfd privileges */
 zebra_capabilities_t _caps_p[] = {ZCAP_NET_RAW, ZCAP_BIND, ZCAP_NET_ADMIN,
@@ -139,10 +145,12 @@ static const struct frr_yang_module_info *const ospfd_yang_modules[] = {
 
 /* actual paths filled in main() */
 static char state_path[512];
-static char state_compat_path[512];
+static char state_compat1_path[512];
+static char state_compat2_path[512];
 static char *state_paths[] = {
 	state_path,
-	state_compat_path,
+	state_compat1_path,
+	state_compat2_path, /* NULLed out if not needed */
 	NULL,
 };
 
@@ -242,12 +250,18 @@ int main(int argc, char **argv)
 	if (ospf_instance) {
 		snprintf(state_path, sizeof(state_path),
 			 OSPFD_INST_STATE_NAME(ospf_instance));
-		snprintf(state_compat_path, sizeof(state_compat_path),
+		snprintf(state_compat1_path, sizeof(state_compat1_path),
-			 OSPFD_COMPAT_INST_STATE_NAME(ospf_instance));
+			 OSPFD_COMPAT1_INST_STATE_NAME(ospf_instance));
+		snprintf(state_compat2_path, sizeof(state_compat2_path),
+			 OSPFD_COMPAT2_INST_STATE_NAME(ospf_instance));
 	} else {
 		snprintf(state_path, sizeof(state_path), OSPFD_STATE_NAME);
-		snprintf(state_compat_path, sizeof(state_compat_path),
+		snprintf(state_compat1_path, sizeof(state_compat1_path),
 			 OSPFD_COMPAT_STATE_NAME);
+		/* no COMPAT2 here since it was reading that was broken,
+		 * there is no additional path that would've been written
+		 */
+		state_paths[2] = NULL;
 	}
 
 	/* OSPF master init. */