[iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[Gioele Barabucci]

Dear iproute2 maintainers,

this patch series adds support for the so called "stateless" configuration
pattern, i.e. reading the default configuration from /usr while allowing
overriding it in /etc, giving system administrators a way to define local
configuration without changing any distro-provided files.

In practice this means that each configuration file FOO is loaded
from /usr/lib/iproute2/FOO unless /etc/iproute2/FOO exists.

Gioele Barabucci (22):
  Makefile: Rename CONFDIR to CONF_ETC_DIR
  Makefile: Add CONF_USR_DIR for system-installed configuration files
  include/utils.h: Use /usr/lib/iproute2 as default CONF_USR_DIR
  tc/tc_util: Read class names from provided path, /etc/, /usr
  tc/m_ematch: Read ematch from /etc and /usr
  lib/bpf_legacy: bpf_hash_init: Relay returned value
  lib/bpf_legacy: Read bpf_pinning from /etc and /usr
  lib/rt_names: rtnl_hash_initialize: Relay returned value
  lib/rt_names: rtnl_tab_initialize: Relay returned value
  lib/rt_names: Read rt_protos from /etc and /usr
  lib/rt_names: Read rt_scopes from /etc and /usr
  lib/rt_names: Read rt_names from /etc and /usr
  lib/rt_names: Read rt_tables from /etc and /usr
  lib/rt_names: Read rt_dsfield from /etc and /usr
  lib/rt_names: Read group from /etc and /usr
  lib/rt_names: Read nl_protos from /etc and /usr
  lib/rt_names: Read rt_protos.d/* from /etc and /usr
  lib/rt_names: Read rt_protos.d/* using rtnl_tab_initialize_dir
  lib/rt_names: Read protodown_reasons.d/* using rtnl_tab_initialize_dir
  lib/rt_names: Read rt_tables.d/* using rtnl_hash_initialize_dir
  man: Document lookup of configuration files in /etc and /usr
  Makefile: Install default configuration files in /usr

 Makefile                 |  10 +-
 include/utils.h          |   7 +-
 lib/bpf_legacy.c         |  12 ++-
 lib/rt_names.c           | 217 ++++++++++++++++++++++++---------------
 man/man8/Makefile        |   3 +-
 man/man8/ip-address.8.in |   5 +-
 man/man8/ip-link.8.in    |  12 ++-
 man/man8/ip-route.8.in   |  43 +++++---
 tc/m_ematch.c            |  17 ++-
 tc/tc_util.c             |  18 +++-
 10 files changed, 214 insertions(+), 130 deletions(-)

-- 
2.39.2

[iproute2 01/22] Makefile: Rename CONFDIR to CONF_ETC_DIR

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 Makefile                 |  8 ++++----
 include/utils.h          |  4 ++--
 lib/bpf_legacy.c         |  2 +-
 lib/rt_names.c           | 28 ++++++++++++++--------------
 man/man8/Makefile        |  2 +-
 man/man8/ip-address.8.in |  2 +-
 man/man8/ip-link.8.in    |  4 ++--
 man/man8/ip-route.8.in   | 18 +++++++++---------
 8 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/Makefile b/Makefile
index 8a17d614..54e5cde2 100644
--- a/Makefile
+++ b/Makefile
@@ -16,7 +16,7 @@ endif
 
 PREFIX?=/usr
 SBINDIR?=/sbin
-CONFDIR?=/etc/iproute2
+CONF_ETC_DIR?=/etc/iproute2
 NETNS_RUN_DIR?=/var/run/netns
 NETNS_ETC_DIR?=/etc/netns
 DATADIR?=$(PREFIX)/share
@@ -37,7 +37,7 @@ ifneq ($(SHARED_LIBS),y)
 DEFINES+= -DNO_SHARED_LIBS
 endif
 
-DEFINES+=-DCONFDIR=\"$(CONFDIR)\" \
+DEFINES+=-DCONF_ETC_DIR=\"$(CONF_ETC_DIR)\" \
          -DNETNS_RUN_DIR=\"$(NETNS_RUN_DIR)\" \
          -DNETNS_ETC_DIR=\"$(NETNS_ETC_DIR)\"
 
@@ -100,11 +100,11 @@ config.mk:
 
 install: all
 	install -m 0755 -d $(DESTDIR)$(SBINDIR)
-	install -m 0755 -d $(DESTDIR)$(CONFDIR)
+	install -m 0755 -d $(DESTDIR)$(CONF_ETC_DIR)
 	install -m 0755 -d $(DESTDIR)$(ARPDDIR)
 	install -m 0755 -d $(DESTDIR)$(HDRDIR)
 	@for i in $(SUBDIRS);  do $(MAKE) -C $$i install; done
-	install -m 0644 $(shell find etc/iproute2 -maxdepth 1 -type f) $(DESTDIR)$(CONFDIR)
+	install -m 0644 $(shell find etc/iproute2 -maxdepth 1 -type f) $(DESTDIR)$(CONF_ETC_DIR)
 	install -m 0755 -d $(DESTDIR)$(BASH_COMPDIR)
 	install -m 0644 bash-completion/tc $(DESTDIR)$(BASH_COMPDIR)
 	install -m 0644 bash-completion/devlink $(DESTDIR)$(BASH_COMPDIR)
diff --git a/include/utils.h b/include/utils.h
index 0b5d86a2..d3bf49bf 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -38,8 +38,8 @@ extern int numeric;
 extern bool do_all;
 extern int echo_request;
 
-#ifndef CONFDIR
-#define CONFDIR		"/etc/iproute2"
+#ifndef CONF_ETC_DIR
+#define CONF_ETC_DIR "/etc/iproute2"
 #endif
 
 #define SPRINT_BSIZE 64
diff --git a/lib/bpf_legacy.c b/lib/bpf_legacy.c
index 8ac64235..706820f5 100644
--- a/lib/bpf_legacy.c
+++ b/lib/bpf_legacy.c
@@ -2924,7 +2924,7 @@ static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname,
 	}
 
 	bpf_save_finfo(ctx);
-	bpf_hash_init(ctx, CONFDIR "/bpf_pinning");
+	bpf_hash_init(ctx, CONF_ETC_DIR "/bpf_pinning");
 
 	return 0;
 out_free:
diff --git a/lib/rt_names.c b/lib/rt_names.c
index 68db74e3..0407b361 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -153,10 +153,10 @@ static void rtnl_rtprot_initialize(void)
 	DIR *d;
 
 	rtnl_rtprot_init = 1;
-	rtnl_tab_initialize(CONFDIR "/rt_protos",
+	rtnl_tab_initialize(CONF_ETC_DIR "/rt_protos",
 			    rtnl_rtprot_tab, 256);
 
-	d = opendir(CONFDIR "/rt_protos.d");
+	d = opendir(CONF_ETC_DIR "/rt_protos.d");
 	if (!d)
 		return;
 
@@ -174,7 +174,7 @@ static void rtnl_rtprot_initialize(void)
 		if (strcmp(de->d_name + len - 5, ".conf"))
 			continue;
 
-		snprintf(path, sizeof(path), CONFDIR "/rt_protos.d/%s",
+		snprintf(path, sizeof(path), CONF_ETC_DIR "/rt_protos.d/%s",
 			 de->d_name);
 		rtnl_tab_initialize(path, rtnl_rtprot_tab, 256);
 	}
@@ -240,7 +240,7 @@ static bool rtnl_addrprot_tab_initialized;
 
 static void rtnl_addrprot_initialize(void)
 {
-	rtnl_tab_initialize(CONFDIR "/rt_addrprotos",
+	rtnl_tab_initialize(CONF_ETC_DIR "/rt_addrprotos",
 			    rtnl_addrprot_tab,
 			    ARRAY_SIZE(rtnl_addrprot_tab));
 	rtnl_addrprot_tab_initialized = true;
@@ -297,7 +297,7 @@ static int rtnl_rtscope_init;
 static void rtnl_rtscope_initialize(void)
 {
 	rtnl_rtscope_init = 1;
-	rtnl_tab_initialize(CONFDIR "/rt_scopes",
+	rtnl_tab_initialize(CONF_ETC_DIR "/rt_scopes",
 			    rtnl_rtscope_tab, 256);
 }
 
@@ -362,7 +362,7 @@ static int rtnl_rtrealm_init;
 static void rtnl_rtrealm_initialize(void)
 {
 	rtnl_rtrealm_init = 1;
-	rtnl_tab_initialize(CONFDIR "/rt_realms",
+	rtnl_tab_initialize(CONF_ETC_DIR "/rt_realms",
 			    rtnl_rtrealm_tab, 256);
 }
 
@@ -439,10 +439,10 @@ static void rtnl_rttable_initialize(void)
 		if (rtnl_rttable_hash[i])
 			rtnl_rttable_hash[i]->id = i;
 	}
-	rtnl_hash_initialize(CONFDIR "/rt_tables",
+	rtnl_hash_initialize(CONF_ETC_DIR "/rt_tables",
 			     rtnl_rttable_hash, 256);
 
-	d = opendir(CONFDIR "/rt_tables.d");
+	d = opendir(CONF_ETC_DIR "/rt_tables.d");
 	if (!d)
 		return;
 
@@ -461,7 +461,7 @@ static void rtnl_rttable_initialize(void)
 			continue;
 
 		snprintf(path, sizeof(path),
-			 CONFDIR "/rt_tables.d/%s", de->d_name);
+			 CONF_ETC_DIR "/rt_tables.d/%s", de->d_name);
 		rtnl_hash_initialize(path, rtnl_rttable_hash, 256);
 	}
 	closedir(d);
@@ -527,7 +527,7 @@ static int rtnl_rtdsfield_init;
 static void rtnl_rtdsfield_initialize(void)
 {
 	rtnl_rtdsfield_init = 1;
-	rtnl_tab_initialize(CONFDIR "/rt_dsfield",
+	rtnl_tab_initialize(CONF_ETC_DIR "/rt_dsfield",
 			    rtnl_rtdsfield_tab, 256);
 }
 
@@ -606,7 +606,7 @@ static int rtnl_group_init;
 static void rtnl_group_initialize(void)
 {
 	rtnl_group_init = 1;
-	rtnl_hash_initialize(CONFDIR "/group",
+	rtnl_hash_initialize(CONF_ETC_DIR "/group",
 			     rtnl_group_hash, 256);
 }
 
@@ -696,7 +696,7 @@ static int nl_proto_init;
 static void nl_proto_initialize(void)
 {
 	nl_proto_init = 1;
-	rtnl_tab_initialize(CONFDIR "/nl_protos",
+	rtnl_tab_initialize(CONF_ETC_DIR "/nl_protos",
 			    nl_proto_tab, 256);
 }
 
@@ -762,7 +762,7 @@ static void protodown_reason_initialize(void)
 
 	protodown_reason_init = 1;
 
-	d = opendir(CONFDIR "/protodown_reasons.d");
+	d = opendir(CONF_ETC_DIR "/protodown_reasons.d");
 	if (!d)
 		return;
 
@@ -780,7 +780,7 @@ static void protodown_reason_initialize(void)
 		if (strcmp(de->d_name + len - 5, ".conf"))
 			continue;
 
-		snprintf(path, sizeof(path), CONFDIR "/protodown_reasons.d/%s",
+		snprintf(path, sizeof(path), CONF_ETC_DIR "/protodown_reasons.d/%s",
 			 de->d_name);
 		rtnl_tab_initialize(path, protodown_reason_tab,
 				    PROTODOWN_REASON_NUM_BITS);
diff --git a/man/man8/Makefile b/man/man8/Makefile
index b1fd87bd..ae5e37a5 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -9,7 +9,7 @@ all: $(TARGETS)
 	sed \
 		-e "s|@NETNS_ETC_DIR@|$(NETNS_ETC_DIR)|g" \
 		-e "s|@NETNS_RUN_DIR@|$(NETNS_RUN_DIR)|g" \
-		-e "s|@SYSCONFDIR@|$(CONFDIR)|g" \
+		-e "s|@SYSCONF_ETC_DIR@|$(CONF_ETC_DIR)|g" \
 		$< > $@
 
 distclean: clean
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index abdd6a20..a2df22d4 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -209,7 +209,7 @@ The maximum allowed total length of label is 15 characters.
 .BI scope " SCOPE_VALUE"
 the scope of the area where this address is valid.
 The available scopes are listed in file
-.BR "@SYSCONFDIR@/rt_scopes" .
+.BR "@SYSCONF_ETC_DIR@/rt_scopes" .
 Predefined scope values are:
 
 .in +8
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 6a82ddc4..ac1a3b5f 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -2251,7 +2251,7 @@ give the device a symbolic name for easy reference.
 .BI group " GROUP"
 specify the group the device belongs to.
 The available groups are listed in file
-.BR "@SYSCONFDIR@/group" .
+.BR "@SYSCONF_ETC_DIR@/group" .
 
 .TP
 .BI vf " NUM"
@@ -2852,7 +2852,7 @@ specifies which help of link type to display.
 .SS
 .I GROUP
 may be a number or a string from the file
-.B @SYSCONFDIR@/group
+.B @SYSCONF_ETC_DIR@/group
 which can be manually filled.
 
 .SH "EXAMPLES"
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index c2b00833..9fc3f4a1 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -357,7 +357,7 @@ normal routing tables.
 .B Route tables:
 Linux-2.x can pack routes into several routing tables identified
 by a number in the range from 1 to 2^32-1 or by name from the file
-.B @SYSCONFDIR@/rt_tables
+.B @SYSCONF_ETC_DIR@/rt_tables
 By default all normal routes are inserted into the
 .B main
 table (ID 254) and the kernel only uses this table when calculating routes.
@@ -420,7 +420,7 @@ may still match a route with a zero TOS.
 .I TOS
 is either an 8 bit hexadecimal number or an identifier
 from
-.BR "@SYSCONFDIR@/rt_dsfield" .
+.BR "@SYSCONF_ETC_DIR@/rt_dsfield" .
 
 .TP
 .BI metric " NUMBER"
@@ -435,7 +435,7 @@ is an arbitrary 32bit number, where routes with lower values are preferred.
 the table to add this route to.
 .I TABLEID
 may be a number or a string from the file
-.BR "@SYSCONFDIR@/rt_tables" .
+.BR "@SYSCONF_ETC_DIR@/rt_tables" .
 If this parameter is omitted,
 .B ip
 assumes the
@@ -476,7 +476,7 @@ covered by the route prefix.
 the realm to which this route is assigned.
 .I REALMID
 may be a number or a string from the file
-.BR "@SYSCONFDIR@/rt_realms" .
+.BR "@SYSCONF_ETC_DIR@/rt_realms" .
 
 .TP
 .BI mtu " MTU"
@@ -627,7 +627,7 @@ command.
 the scope of the destinations covered by the route prefix.
 .I SCOPE_VAL
 may be a number or a string from the file
-.BR "@SYSCONFDIR@/rt_scopes" .
+.BR "@SYSCONF_ETC_DIR@/rt_scopes" .
 If this parameter is omitted,
 .B ip
 assumes scope
@@ -647,7 +647,7 @@ routes.
 the routing protocol identifier of this route.
 .I RTPROTO
 may be a number or a string from the file
-.BR "@SYSCONFDIR@/rt_protos" .
+.BR "@SYSCONF_ETC_DIR@/rt_protos" .
 If the routing protocol ID is not given,
 .B ip assumes protocol
 .B boot
@@ -880,7 +880,7 @@ matching packets are dropped.
 specified lookup table.
 .I TABLEID
 is either a number or a string from the file
-.BR "@SYSCONFDIR@/rt_tables" .
+.BR "@SYSCONF_ETC_DIR@/rt_tables" .
 If
 .B vrftable
 is used, the argument must be a VRF device associated with
@@ -896,7 +896,7 @@ and an inner IPv6 packet. Other matching packets are dropped.
 specified lookup table.
 .I TABLEID
 is either a number or a string from the file
-.BR "@SYSCONFDIR@/rt_tables" .
+.BR "@SYSCONF_ETC_DIR@/rt_tables" .
 The argument must be a VRF device associated with the table id.
 Moreover, the VRF table associated with the table id must be configured
 with the VRF strict mode turned on (net.vrf.strict_mode=1). This action
@@ -909,7 +909,7 @@ at all, and an inner IPv4 packet. Other matching packets are dropped.
 to the specified lookup table.
 .I TABLEID
 is either a number or a string from the file
-.BR "@SYSCONFDIR@/rt_tables" .
+.BR "@SYSCONF_ETC_DIR@/rt_tables" .
 The argument must be a VRF device associated with the table id.
 Moreover, the VRF table associated with the table id must be configured
 with the VRF strict mode turned on (net.vrf.strict_mode=1). This action
-- 
2.39.2

[iproute2 02/22] Makefile: Add CONF_USR_DIR for system-installed configuration files

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 54e5cde2..a2af4426 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,7 @@ endif
 PREFIX?=/usr
 SBINDIR?=/sbin
 CONF_ETC_DIR?=/etc/iproute2
+CONF_USR_DIR?=$(PREFIX)/lib/iproute2
 NETNS_RUN_DIR?=/var/run/netns
 NETNS_ETC_DIR?=/etc/netns
 DATADIR?=$(PREFIX)/share
@@ -37,7 +38,8 @@ ifneq ($(SHARED_LIBS),y)
 DEFINES+= -DNO_SHARED_LIBS
 endif
 
-DEFINES+=-DCONF_ETC_DIR=\"$(CONF_ETC_DIR)\" \
+DEFINES+=-DCONF_USR_DIR=\"$(CONF_USR_DIR)\" \
+         -DCONF_ETC_DIR=\"$(CONF_ETC_DIR)\" \
          -DNETNS_RUN_DIR=\"$(NETNS_RUN_DIR)\" \
          -DNETNS_ETC_DIR=\"$(NETNS_ETC_DIR)\"
 
-- 
2.39.2

[iproute2 03/22] include/utils.h: Use /usr/lib/iproute2 as default CONF_USR_DIR

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 include/utils.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/utils.h b/include/utils.h
index d3bf49bf..3159dbab 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -38,6 +38,9 @@ extern int numeric;
 extern bool do_all;
 extern int echo_request;
 
+#ifndef CONF_USR_DIR
+#define CONF_USR_DIR "/usr/lib/iproute2"
+#endif
 #ifndef CONF_ETC_DIR
 #define CONF_ETC_DIR "/etc/iproute2"
 #endif
-- 
2.39.2

[iproute2 04/22] tc/tc_util: Read class names from provided path, /etc/, /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 tc/tc_util.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index ed9efa70..e6235291 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -28,7 +28,8 @@
 
 static struct db_names *cls_names;
 
-#define NAMES_DB "/etc/iproute2/tc_cls"
+#define NAMES_DB_USR "/usr/lib/iproute2/tc_cls"
+#define NAMES_DB_ETC "/etc/iproute2/tc_cls"
 
 int cls_names_init(char *path)
 {
@@ -38,11 +39,18 @@ int cls_names_init(char *path)
 	if (!cls_names)
 		return -1;
 
-	ret = db_names_load(cls_names, path ?: NAMES_DB);
-	if (ret == -ENOENT && path) {
-		fprintf(stderr, "Can't open class names file: %s\n", path);
-		return -1;
+	if (path) {
+		ret = db_names_load(cls_names, path);
+		if (ret == -ENOENT) {
+			fprintf(stderr, "Can't open class names file: %s\n", path);
+			return -1;
+		}
 	}
+
+	ret = db_names_load(cls_names, NAMES_DB_ETC);
+	if (ret == -ENOENT)
+		ret = db_names_load(cls_names, NAMES_DB_USR);
+
 	if (ret) {
 		db_names_free(cls_names);
 		cls_names = NULL;
-- 
2.39.2

Re: [iproute2 04/22] tc/tc_util: Read class names from provided path, /etc/, /usr

[Petr Machata]

Gioele Barabucci <gioele@svario.it> writes:

> Signed-off-by: Gioele Barabucci <gioele@svario.it>
> ---
>  tc/tc_util.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index ed9efa70..e6235291 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -28,7 +28,8 @@
>  
>  static struct db_names *cls_names;
>  
> -#define NAMES_DB "/etc/iproute2/tc_cls"
> +#define NAMES_DB_USR "/usr/lib/iproute2/tc_cls"
> +#define NAMES_DB_ETC "/etc/iproute2/tc_cls"

Is there a reason that these don't use CONF_USR_DIR and CONF_ETC_DIR?
I thought maybe the caller uses those and this is just a hardcoded
fallback, but that's not the case.

>  
>  int cls_names_init(char *path)
>  {
> @@ -38,11 +39,18 @@ int cls_names_init(char *path)
>  	if (!cls_names)
>  		return -1;
>  
> -	ret = db_names_load(cls_names, path ?: NAMES_DB);
> -	if (ret == -ENOENT && path) {
> -		fprintf(stderr, "Can't open class names file: %s\n", path);
> -		return -1;
> +	if (path) {
> +		ret = db_names_load(cls_names, path);
> +		if (ret == -ENOENT) {
> +			fprintf(stderr, "Can't open class names file: %s\n", path);
> +			return -1;

The caller always calls cls_uninit(), even for failures, so this
technically does not leak. Not great, but let's keep it as is,
fixing this is clearly out of scope.

> +		}
>  	}
> +
> +	ret = db_names_load(cls_names, NAMES_DB_ETC);
> +	if (ret == -ENOENT)
> +		ret = db_names_load(cls_names, NAMES_DB_USR);
> +
>  	if (ret) {
>  		db_names_free(cls_names);
>  		cls_names = NULL;

Otherwise looking good. In fact IMHO clearer than the old code.

Re: [iproute2 04/22] tc/tc_util: Read class names from provided path, /etc/, /usr

[Gioele Barabucci]

On 20/07/23 12:10, Petr Machata wrote:
>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>> index ed9efa70..e6235291 100644
>> --- a/tc/tc_util.c
>> +++ b/tc/tc_util.c
>> @@ -28,7 +28,8 @@
>>   
>>   static struct db_names *cls_names;
>>   
>> -#define NAMES_DB "/etc/iproute2/tc_cls"
>> +#define NAMES_DB_USR "/usr/lib/iproute2/tc_cls"
>> +#define NAMES_DB_ETC "/etc/iproute2/tc_cls"
> 
> Is there a reason that these don't use CONF_USR_DIR and CONF_ETC_DIR?
> I thought maybe the caller uses those and this is just a hardcoded
> fallback, but that's not the case.

Thanks for the review Petr.

The reason why I did not use CONF_USR_DIR in these patches is because I 
wanted to minimize the number and amount of changes. But I asked myself 
the same question when I first looked at this and other similar occurrences.

Let me know if I should update the patches to use CONF_{USR,ETC}_DIR.

Regards,

-- 
Gioele Barabucci

Re: [iproute2 04/22] tc/tc_util: Read class names from provided path, /etc/, /usr

[Petr Machata]

Gioele Barabucci <gioele@svario.it> writes:

> On 20/07/23 12:10, Petr Machata wrote:
>>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>>> index ed9efa70..e6235291 100644
>>> --- a/tc/tc_util.c
>>> +++ b/tc/tc_util.c
>>> @@ -28,7 +28,8 @@
>>>     static struct db_names *cls_names;
>>>   -#define NAMES_DB "/etc/iproute2/tc_cls"
>>> +#define NAMES_DB_USR "/usr/lib/iproute2/tc_cls"
>>> +#define NAMES_DB_ETC "/etc/iproute2/tc_cls"
>> Is there a reason that these don't use CONF_USR_DIR and CONF_ETC_DIR?
>> I thought maybe the caller uses those and this is just a hardcoded
>> fallback, but that's not the case.
>
> Thanks for the review Petr.
>
> The reason why I did not use CONF_USR_DIR in these patches is because I wanted to minimize the
> number and amount of changes. But I asked myself the same question when I first looked at this and
> other similar occurrences.
>
> Let me know if I should update the patches to use CONF_{USR,ETC}_DIR.

The change would make sense to me, it looks like rest of the code tends
to use those defines. But it should probably be sent separately, yeah.

[iproute2 05/22] tc/m_ematch: Read ematch from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 tc/m_ematch.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tc/m_ematch.c b/tc/m_ematch.c
index e30ee205..1d0a208f 100644
--- a/tc/m_ematch.c
+++ b/tc/m_ematch.c
@@ -21,7 +21,8 @@
 #include "tc_util.h"
 #include "m_ematch.h"
 
-#define EMATCH_MAP "/etc/iproute2/ematch_map"
+#define EMATCH_MAP_USR	"/usr/lib/iproute2/ematch_map"
+#define EMATCH_MAP_ETC	"/etc/iproute2/ematch_map"
 
 static struct ematch_util *ematch_list;
 
@@ -39,11 +40,11 @@ static void bstr_print(FILE *fd, const struct bstr *b, int ascii);
 static inline void map_warning(int num, char *kind)
 {
 	fprintf(stderr,
-	    "Error: Unable to find ematch \"%s\" in %s\n" \
+	    "Error: Unable to find ematch \"%s\" in %s or %s\n" \
 	    "Please assign a unique ID to the ematch kind the suggested " \
 	    "entry is:\n" \
 	    "\t%d\t%s\n",
-	    kind, EMATCH_MAP, num, kind);
+	    kind, EMATCH_MAP_ETC, EMATCH_MAP_USR, num, kind);
 }
 
 static int lookup_map(__u16 num, char *dst, int len, const char *file)
@@ -160,8 +161,12 @@ static struct ematch_util *get_ematch_kind(char *kind)
 static struct ematch_util *get_ematch_kind_num(__u16 kind)
 {
 	char name[513];
+	int ret;
 
-	if (lookup_map(kind, name, sizeof(name), EMATCH_MAP) < 0)
+	ret = lookup_map(kind, name, sizeof(name), EMATCH_MAP_ETC);
+	if (ret == -ENOENT)
+		ret = lookup_map(kind, name, sizeof(name), EMATCH_MAP_USR);
+	if (ret < 0)
 		return NULL;
 
 	return get_ematch_kind(name);
@@ -227,7 +232,9 @@ static int parse_tree(struct nlmsghdr *n, struct ematch *tree)
 				return -1;
 			}
 
-			err = lookup_map_id(buf, &num, EMATCH_MAP);
+			err = lookup_map_id(buf, &num, EMATCH_MAP_ETC);
+			if (err == -ENOENT)
+				err = lookup_map_id(buf, &num, EMATCH_MAP_USR);
 			if (err < 0) {
 				if (err == -ENOENT)
 					map_warning(e->kind_num, buf);
-- 
2.39.2

Re: [iproute2 05/22] tc/m_ematch: Read ematch from /etc and /usr

[Petr Machata]

Gioele Barabucci <gioele@svario.it> writes:

> Signed-off-by: Gioele Barabucci <gioele@svario.it>
> ---
>  tc/m_ematch.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tc/m_ematch.c b/tc/m_ematch.c
> index e30ee205..1d0a208f 100644
> --- a/tc/m_ematch.c
> +++ b/tc/m_ematch.c
> @@ -21,7 +21,8 @@
>  #include "tc_util.h"
>  #include "m_ematch.h"
>  
> -#define EMATCH_MAP "/etc/iproute2/ematch_map"
> +#define EMATCH_MAP_USR	"/usr/lib/iproute2/ematch_map"
> +#define EMATCH_MAP_ETC	"/etc/iproute2/ematch_map"
>  
>  static struct ematch_util *ematch_list;
>  
> @@ -39,11 +40,11 @@ static void bstr_print(FILE *fd, const struct bstr *b, int ascii);
>  static inline void map_warning(int num, char *kind)
>  {
>  	fprintf(stderr,
> -	    "Error: Unable to find ematch \"%s\" in %s\n" \
> +	    "Error: Unable to find ematch \"%s\" in %s or %s\n" \
>  	    "Please assign a unique ID to the ematch kind the suggested " \
>  	    "entry is:\n" \
>  	    "\t%d\t%s\n",
> -	    kind, EMATCH_MAP, num, kind);
> +	    kind, EMATCH_MAP_ETC, EMATCH_MAP_USR, num, kind);
>  }
>  
>  static int lookup_map(__u16 num, char *dst, int len, const char *file)
> @@ -160,8 +161,12 @@ static struct ematch_util *get_ematch_kind(char *kind)
>  static struct ematch_util *get_ematch_kind_num(__u16 kind)
>  {
>  	char name[513];
> +	int ret;
>  
> -	if (lookup_map(kind, name, sizeof(name), EMATCH_MAP) < 0)
> +	ret = lookup_map(kind, name, sizeof(name), EMATCH_MAP_ETC);
> +	if (ret == -ENOENT)

OK, so this retains other errors, so e.g. -EACCES would be treated the
same as before, as a failure. I guess that makes sense.

> +		ret = lookup_map(kind, name, sizeof(name), EMATCH_MAP_USR);
> +	if (ret < 0)
>  		return NULL;
>  
>  	return get_ematch_kind(name);
> @@ -227,7 +232,9 @@ static int parse_tree(struct nlmsghdr *n, struct ematch *tree)
>  				return -1;
>  			}
>  
> -			err = lookup_map_id(buf, &num, EMATCH_MAP);
> +			err = lookup_map_id(buf, &num, EMATCH_MAP_ETC);
> +			if (err == -ENOENT)
> +				err = lookup_map_id(buf, &num, EMATCH_MAP_USR);
>  			if (err < 0) {
>  				if (err == -ENOENT)
>  					map_warning(e->kind_num, buf);

Reviewed-by: Petr Machata <me@pmachata.org>

[iproute2 06/22] lib/bpf_legacy: bpf_hash_init: Relay returned value

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/bpf_legacy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/bpf_legacy.c b/lib/bpf_legacy.c
index 706820f5..4bb3f7d8 100644
--- a/lib/bpf_legacy.c
+++ b/lib/bpf_legacy.c
@@ -2751,7 +2751,7 @@ static bool bpf_pinning_reserved(uint32_t pinning)
 	}
 }
 
-static void bpf_hash_init(struct bpf_elf_ctx *ctx, const char *db_file)
+static int bpf_hash_init(struct bpf_elf_ctx *ctx, const char *db_file)
 {
 	struct bpf_hash_entry *entry;
 	char subpath[PATH_MAX] = {};
@@ -2761,14 +2761,14 @@ static void bpf_hash_init(struct bpf_elf_ctx *ctx, const char *db_file)
 
 	fp = fopen(db_file, "r");
 	if (!fp)
-		return;
+		return -errno;
 
 	while ((ret = bpf_read_pin_mapping(fp, &pinning, subpath))) {
 		if (ret == -1) {
 			fprintf(stderr, "Database %s is corrupted at: %s\n",
 				db_file, subpath);
 			fclose(fp);
-			return;
+			return -1;
 		}
 
 		if (bpf_pinning_reserved(pinning)) {
@@ -2796,6 +2796,8 @@ static void bpf_hash_init(struct bpf_elf_ctx *ctx, const char *db_file)
 	}
 
 	fclose(fp);
+
+	return 0;
 }
 
 static void bpf_hash_destroy(struct bpf_elf_ctx *ctx)
-- 
2.39.2

[iproute2 07/22] lib/bpf_legacy: Read bpf_pinning from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/bpf_legacy.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/bpf_legacy.c b/lib/bpf_legacy.c
index 4bb3f7d8..52a951c6 100644
--- a/lib/bpf_legacy.c
+++ b/lib/bpf_legacy.c
@@ -2926,7 +2926,9 @@ static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname,
 	}
 
 	bpf_save_finfo(ctx);
-	bpf_hash_init(ctx, CONF_ETC_DIR "/bpf_pinning");
+	ret = bpf_hash_init(ctx, CONF_USR_DIR "/bpf_pinning");
+	if (ret == -ENOENT)
+		bpf_hash_init(ctx, CONF_ETC_DIR "/bpf_pinning");
 
 	return 0;
 out_free:
-- 
2.39.2

[iproute2 08/22] lib/rt_names: rtnl_hash_initialize: Relay returned value

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 0407b361..27aca3ec 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -14,6 +14,7 @@
 #include <sys/socket.h>
 #include <dirent.h>
 #include <limits.h>
+#include <errno.h>
 
 #include <asm/types.h>
 #include <linux/rtnetlink.h>
@@ -56,7 +57,7 @@ static int fread_id_name(FILE *fp, int *id, char *namebuf)
 	return 0;
 }
 
-static void
+static int
 rtnl_hash_initialize(const char *file, struct rtnl_hash_entry **hash, int size)
 {
 	struct rtnl_hash_entry *entry;
@@ -67,14 +68,14 @@ rtnl_hash_initialize(const char *file, struct rtnl_hash_entry **hash, int size)
 
 	fp = fopen(file, "r");
 	if (!fp)
-		return;
+		return -errno;
 
 	while ((ret = fread_id_name(fp, &id, &namebuf[0]))) {
 		if (ret == -1) {
 			fprintf(stderr, "Database %s is corrupted at %s\n",
 					file, namebuf);
 			fclose(fp);
-			return;
+			return -1;
 		}
 
 		if (id < 0)
@@ -91,6 +92,8 @@ rtnl_hash_initialize(const char *file, struct rtnl_hash_entry **hash, int size)
 		hash[id & (size - 1)] = entry;
 	}
 	fclose(fp);
+
+	return 0;
 }
 
 static void rtnl_tab_initialize(const char *file, char **tab, int size)
-- 
2.39.2

[iproute2 09/22] lib/rt_names: rtnl_tab_initialize: Relay returned value

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 27aca3ec..f64602b5 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -96,7 +96,7 @@ rtnl_hash_initialize(const char *file, struct rtnl_hash_entry **hash, int size)
 	return 0;
 }
 
-static void rtnl_tab_initialize(const char *file, char **tab, int size)
+static int rtnl_tab_initialize(const char *file, char **tab, int size)
 {
 	FILE *fp;
 	int id;
@@ -105,14 +105,14 @@ static void rtnl_tab_initialize(const char *file, char **tab, int size)
 
 	fp = fopen(file, "r");
 	if (!fp)
-		return;
+		return -errno;
 
 	while ((ret = fread_id_name(fp, &id, &namebuf[0]))) {
 		if (ret == -1) {
 			fprintf(stderr, "Database %s is corrupted at %s\n",
 					file, namebuf);
 			fclose(fp);
-			return;
+			return -1;
 		}
 		if (id < 0 || id > size)
 			continue;
@@ -120,6 +120,8 @@ static void rtnl_tab_initialize(const char *file, char **tab, int size)
 		tab[id] = strdup(namebuf);
 	}
 	fclose(fp);
+
+	return 0;
 }
 
 static char *rtnl_rtprot_tab[256] = {
-- 
2.39.2

[iproute2 10/22] lib/rt_names: Read rt_protos from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index f64602b5..142954cd 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -156,10 +156,14 @@ static void rtnl_rtprot_initialize(void)
 {
 	struct dirent *de;
 	DIR *d;
+	int ret;
 
 	rtnl_rtprot_init = 1;
-	rtnl_tab_initialize(CONF_ETC_DIR "/rt_protos",
-			    rtnl_rtprot_tab, 256);
+	ret = rtnl_tab_initialize(CONF_ETC_DIR "/rt_protos",
+	                          rtnl_rtprot_tab, 256);
+	if (ret == -ENOENT)
+		rtnl_tab_initialize(CONF_USR_DIR "/rt_protos",
+		                    rtnl_rtprot_tab, 256);
 
 	d = opendir(CONF_ETC_DIR "/rt_protos.d");
 	if (!d)
-- 
2.39.2

[iproute2 11/22] lib/rt_names: Read rt_scopes from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 142954cd..37f85ded 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -305,9 +305,14 @@ static int rtnl_rtscope_init;
 
 static void rtnl_rtscope_initialize(void)
 {
+	int ret;
+
 	rtnl_rtscope_init = 1;
-	rtnl_tab_initialize(CONF_ETC_DIR "/rt_scopes",
-			    rtnl_rtscope_tab, 256);
+	ret = rtnl_tab_initialize(CONF_ETC_DIR "/rt_scopes",
+			          rtnl_rtscope_tab, 256);
+	if (ret == -ENOENT)
+		rtnl_tab_initialize(CONF_USR_DIR "/rt_scopes",
+				    rtnl_rtscope_tab, 256);
 }
 
 const char *rtnl_rtscope_n2a(int id, char *buf, int len)
-- 
2.39.2

[iproute2 12/22] lib/rt_names: Read rt_names from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 37f85ded..5b911753 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -375,9 +375,14 @@ static int rtnl_rtrealm_init;
 
 static void rtnl_rtrealm_initialize(void)
 {
+	int ret;
+
 	rtnl_rtrealm_init = 1;
-	rtnl_tab_initialize(CONF_ETC_DIR "/rt_realms",
-			    rtnl_rtrealm_tab, 256);
+	ret = rtnl_tab_initialize(CONF_ETC_DIR "/rt_realms",
+	                          rtnl_rtrealm_tab, 256);
+	if (ret == -ENOENT)
+		rtnl_tab_initialize(CONF_USR_DIR "/rt_realms",
+		                    rtnl_rtrealm_tab, 256);
 }
 
 const char *rtnl_rtrealm_n2a(int id, char *buf, int len)
-- 
2.39.2

[iproute2 13/22] lib/rt_names: Read rt_tables from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 5b911753..e0659250 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -452,14 +452,18 @@ static void rtnl_rttable_initialize(void)
 	struct dirent *de;
 	DIR *d;
 	int i;
+	int ret;
 
 	rtnl_rttable_init = 1;
 	for (i = 0; i < 256; i++) {
 		if (rtnl_rttable_hash[i])
 			rtnl_rttable_hash[i]->id = i;
 	}
-	rtnl_hash_initialize(CONF_ETC_DIR "/rt_tables",
-			     rtnl_rttable_hash, 256);
+	ret = rtnl_hash_initialize(CONF_ETC_DIR "/rt_tables",
+	                           rtnl_rttable_hash, 256);
+	if (ret == -ENOENT)
+		rtnl_hash_initialize(CONF_USR_DIR "/rt_tables",
+		                     rtnl_rttable_hash, 256);
 
 	d = opendir(CONF_ETC_DIR "/rt_tables.d");
 	if (!d)
-- 
2.39.2

[iproute2 14/22] lib/rt_names: Read rt_dsfield from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index e0659250..a96ae540 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -549,9 +549,14 @@ static int rtnl_rtdsfield_init;
 
 static void rtnl_rtdsfield_initialize(void)
 {
+	int ret;
+
 	rtnl_rtdsfield_init = 1;
-	rtnl_tab_initialize(CONF_ETC_DIR "/rt_dsfield",
-			    rtnl_rtdsfield_tab, 256);
+	ret = rtnl_tab_initialize(CONF_ETC_DIR "/rt_dsfield",
+	                          rtnl_rtdsfield_tab, 256);
+	if (ret == -ENOENT)
+		rtnl_tab_initialize(CONF_USR_DIR "/rt_dsfield",
+		                    rtnl_rtdsfield_tab, 256);
 }
 
 const char *rtnl_dsfield_n2a(int id, char *buf, int len)
-- 
2.39.2

[iproute2 15/22] lib/rt_names: Read group from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index a96ae540..046a3614 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -633,9 +633,14 @@ static int rtnl_group_init;
 
 static void rtnl_group_initialize(void)
 {
+	int ret;
+
 	rtnl_group_init = 1;
-	rtnl_hash_initialize(CONF_ETC_DIR "/group",
-			     rtnl_group_hash, 256);
+	ret = rtnl_hash_initialize(CONF_ETC_DIR "/group",
+	                           rtnl_group_hash, 256);
+	if (ret == -ENOENT)
+		rtnl_hash_initialize(CONF_USR_DIR "/group",
+		                     rtnl_group_hash, 256);
 }
 
 int rtnl_group_a2n(int *id, const char *arg)
-- 
2.39.2

[iproute2 16/22] lib/rt_names: Read nl_protos from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 046a3614..4f38fcbe 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -728,9 +728,14 @@ static int nl_proto_init;
 
 static void nl_proto_initialize(void)
 {
+	int ret;
+
 	nl_proto_init = 1;
-	rtnl_tab_initialize(CONF_ETC_DIR "/nl_protos",
-			    nl_proto_tab, 256);
+	ret = rtnl_tab_initialize(CONF_ETC_DIR "/nl_protos",
+	                          nl_proto_tab, 256);
+	if (ret == -ENOENT)
+		rtnl_tab_initialize(CONF_USR_DIR "/nl_protos",
+		                    nl_proto_tab, 256);
 }
 
 const char *nl_proto_n2a(int id, char *buf, int len)
-- 
2.39.2

[iproute2 17/22] lib/rt_names: Read rt_protos.d/* from /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 44 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 4f38fcbe..d9c48813 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -12,6 +12,7 @@
 #include <string.h>
 #include <sys/time.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <dirent.h>
 #include <limits.h>
 #include <errno.h>
@@ -165,13 +166,12 @@ static void rtnl_rtprot_initialize(void)
 		rtnl_tab_initialize(CONF_USR_DIR "/rt_protos",
 		                    rtnl_rtprot_tab, 256);
 
-	d = opendir(CONF_ETC_DIR "/rt_protos.d");
-	if (!d)
-		return;
-
-	while ((de = readdir(d)) != NULL) {
+	/* load /usr/lib/iproute2/foo.d/X, unless /etc/iproute2/foo.d/X exists */
+	d = opendir(CONF_USR_DIR "/rt_protos.d");
+	while (d && (de = readdir(d)) != NULL) {
 		char path[PATH_MAX];
 		size_t len;
+		struct stat sb;
 
 		if (*de->d_name == '.')
 			continue;
@@ -183,11 +183,43 @@ static void rtnl_rtprot_initialize(void)
 		if (strcmp(de->d_name + len - 5, ".conf"))
 			continue;
 
+		/* only consider filenames not present in /etc */
+		snprintf(path, sizeof(path), CONF_USR_DIR "/rt_protos.d/%s",
+		         de->d_name);
+		if (lstat(path, &sb) == 0)
+			continue;
+
+		/* load the conf file in /usr */
 		snprintf(path, sizeof(path), CONF_ETC_DIR "/rt_protos.d/%s",
 			 de->d_name);
 		rtnl_tab_initialize(path, rtnl_rtprot_tab, 256);
 	}
-	closedir(d);
+	if (d)
+		closedir(d);
+
+	/* load /etc/iproute2/foo.d/X */
+	d = opendir(CONF_ETC_DIR "/rt_protos.d");
+	while (d && (de = readdir(d)) != NULL) {
+		char path[PATH_MAX];
+		size_t len;
+
+		if (*de->d_name == '.')
+			continue;
+
+		/* only consider filenames ending in '.conf' */
+		len = strlen(de->d_name);
+		if (len <= 5)
+			continue;
+		if (strcmp(de->d_name + len - 5, ".conf"))
+			continue;
+
+		/* load the conf file in /etc */
+		snprintf(path, sizeof(path), CONF_USR_DIR "/rt_protos.d/%s",
+			 de->d_name);
+		rtnl_tab_initialize(path, rtnl_rtprot_tab, 256);
+	}
+	if (d)
+		closedir(d);
 }
 
 const char *rtnl_rtprot_n2a(int id, char *buf, int len)
-- 
2.39.2

[iproute2 18/22] lib/rt_names: Read rt_protos.d/* using rtnl_tab_initialize_dir

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 52 +++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index d9c48813..3b69d05d 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -151,23 +151,18 @@ static char *rtnl_rtprot_tab[256] = {
 };
 
 
-static int rtnl_rtprot_init;
-
-static void rtnl_rtprot_initialize(void)
+static void
+rtnl_tab_initialize_dir(const char *ddir, char **tab, const int size)
 {
+	char dirpath_usr[PATH_MAX], dirpath_etc[PATH_MAX];
 	struct dirent *de;
 	DIR *d;
-	int ret;
 
-	rtnl_rtprot_init = 1;
-	ret = rtnl_tab_initialize(CONF_ETC_DIR "/rt_protos",
-	                          rtnl_rtprot_tab, 256);
-	if (ret == -ENOENT)
-		rtnl_tab_initialize(CONF_USR_DIR "/rt_protos",
-		                    rtnl_rtprot_tab, 256);
+	snprintf(dirpath_usr, sizeof(dirpath_usr), "%s/%s", CONF_USR_DIR, ddir);
+	snprintf(dirpath_etc, sizeof(dirpath_etc), "%s/%s", CONF_ETC_DIR, ddir);
 
-	/* load /usr/lib/iproute2/foo.d/X, unless /etc/iproute2/foo.d/X exists */
-	d = opendir(CONF_USR_DIR "/rt_protos.d");
+	/* load /usr/lib/iproute2/foo.d/X conf files, unless /etc/iproute2/foo.d/X exists */
+	d = opendir(dirpath_usr);
 	while (d && (de = readdir(d)) != NULL) {
 		char path[PATH_MAX];
 		size_t len;
@@ -184,21 +179,19 @@ static void rtnl_rtprot_initialize(void)
 			continue;
 
 		/* only consider filenames not present in /etc */
-		snprintf(path, sizeof(path), CONF_USR_DIR "/rt_protos.d/%s",
-		         de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", dirpath_etc, de->d_name);
 		if (lstat(path, &sb) == 0)
 			continue;
 
 		/* load the conf file in /usr */
-		snprintf(path, sizeof(path), CONF_ETC_DIR "/rt_protos.d/%s",
-			 de->d_name);
-		rtnl_tab_initialize(path, rtnl_rtprot_tab, 256);
+		snprintf(path, sizeof(path), "%s/%s", dirpath_usr, de->d_name);
+		rtnl_tab_initialize(path, tab, size);
 	}
 	if (d)
 		closedir(d);
 
-	/* load /etc/iproute2/foo.d/X */
-	d = opendir(CONF_ETC_DIR "/rt_protos.d");
+	/* load /etc/iproute2/foo.d/X conf files */
+	d = opendir(dirpath_etc);
 	while (d && (de = readdir(d)) != NULL) {
 		char path[PATH_MAX];
 		size_t len;
@@ -214,14 +207,29 @@ static void rtnl_rtprot_initialize(void)
 			continue;
 
 		/* load the conf file in /etc */
-		snprintf(path, sizeof(path), CONF_USR_DIR "/rt_protos.d/%s",
-			 de->d_name);
-		rtnl_tab_initialize(path, rtnl_rtprot_tab, 256);
+		snprintf(path, sizeof(path), "%s/%s", dirpath_etc, de->d_name);
+		rtnl_tab_initialize(path, tab, size);
 	}
 	if (d)
 		closedir(d);
 }
 
+static int rtnl_rtprot_init;
+
+static void rtnl_rtprot_initialize(void)
+{
+	int ret;
+
+	rtnl_rtprot_init = 1;
+	ret = rtnl_tab_initialize(CONF_ETC_DIR "/rt_protos",
+	                          rtnl_rtprot_tab, 256);
+	if (ret == -ENOENT)
+		rtnl_tab_initialize(CONF_USR_DIR "/rt_protos",
+		                    rtnl_rtprot_tab, 256);
+
+	rtnl_tab_initialize_dir("rt_protos.d", rtnl_rtprot_tab, 256);
+}
+
 const char *rtnl_rtprot_n2a(int id, char *buf, int len)
 {
 	if (id < 0 || id >= 256 || numeric) {
-- 
2.39.2

[iproute2 19/22] lib/rt_names: Read protodown_reasons.d/* using rtnl_tab_initialize_dir

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 3b69d05d..090fc883 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -835,35 +835,10 @@ static int protodown_reason_init;
 
 static void protodown_reason_initialize(void)
 {
-	struct dirent *de;
-	DIR *d;
-
 	protodown_reason_init = 1;
 
-	d = opendir(CONF_ETC_DIR "/protodown_reasons.d");
-	if (!d)
-		return;
-
-	while ((de = readdir(d)) != NULL) {
-		char path[PATH_MAX];
-		size_t len;
-
-		if (*de->d_name == '.')
-			continue;
-
-		/* only consider filenames ending in '.conf' */
-		len = strlen(de->d_name);
-		if (len <= 5)
-			continue;
-		if (strcmp(de->d_name + len - 5, ".conf"))
-			continue;
-
-		snprintf(path, sizeof(path), CONF_ETC_DIR "/protodown_reasons.d/%s",
-			 de->d_name);
-		rtnl_tab_initialize(path, protodown_reason_tab,
-				    PROTODOWN_REASON_NUM_BITS);
-	}
-	closedir(d);
+	rtnl_tab_initialize_dir("protodown_reasons.d", protodown_reason_tab,
+                                PROTODOWN_REASON_NUM_BITS);
 }
 
 int protodown_reason_n2a(int id, char *buf, int len)
-- 
2.39.2

[iproute2 20/22] lib/rt_names: Read rt_tables.d/* using rtnl_hash_initialize_dir

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 lib/rt_names.c | 50 ++++++++++++++++++++++----------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 090fc883..eca8b4fa 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -152,7 +152,8 @@ static char *rtnl_rtprot_tab[256] = {
 
 
 static void
-rtnl_tab_initialize_dir(const char *ddir, char **tab, const int size)
+rtnl_tabhash_initialize_dir(const char *ddir, void *tabhash, const int size,
+                            const bool is_tab)
 {
 	char dirpath_usr[PATH_MAX], dirpath_etc[PATH_MAX];
 	struct dirent *de;
@@ -185,7 +186,10 @@ rtnl_tab_initialize_dir(const char *ddir, char **tab, const int size)
 
 		/* load the conf file in /usr */
 		snprintf(path, sizeof(path), "%s/%s", dirpath_usr, de->d_name);
-		rtnl_tab_initialize(path, tab, size);
+		if (is_tab)
+			rtnl_tab_initialize(path, (char**)tabhash, size);
+		else
+			rtnl_hash_initialize(path, (struct rtnl_hash_entry**)tabhash, size);
 	}
 	if (d)
 		closedir(d);
@@ -208,12 +212,26 @@ rtnl_tab_initialize_dir(const char *ddir, char **tab, const int size)
 
 		/* load the conf file in /etc */
 		snprintf(path, sizeof(path), "%s/%s", dirpath_etc, de->d_name);
-		rtnl_tab_initialize(path, tab, size);
+		if (is_tab)
+			rtnl_tab_initialize(path, (char**)tabhash, size);
+		else
+			rtnl_hash_initialize(path, (struct rtnl_hash_entry**)tabhash, size);
 	}
 	if (d)
 		closedir(d);
 }
 
+static void
+rtnl_tab_initialize_dir(const char *ddir, char **tab, const int size) {
+	rtnl_tabhash_initialize_dir(ddir, (void*)tab, size, true);
+}
+
+static void
+rtnl_hash_initialize_dir(const char *ddir, struct rtnl_hash_entry **tab,
+                         const int size) {
+	rtnl_tabhash_initialize_dir(ddir, (void*)tab, size, false);
+}
+
 static int rtnl_rtprot_init;
 
 static void rtnl_rtprot_initialize(void)
@@ -489,8 +507,6 @@ static int rtnl_rttable_init;
 
 static void rtnl_rttable_initialize(void)
 {
-	struct dirent *de;
-	DIR *d;
 	int i;
 	int ret;
 
@@ -505,29 +521,7 @@ static void rtnl_rttable_initialize(void)
 		rtnl_hash_initialize(CONF_USR_DIR "/rt_tables",
 		                     rtnl_rttable_hash, 256);
 
-	d = opendir(CONF_ETC_DIR "/rt_tables.d");
-	if (!d)
-		return;
-
-	while ((de = readdir(d)) != NULL) {
-		char path[PATH_MAX];
-		size_t len;
-
-		if (*de->d_name == '.')
-			continue;
-
-		/* only consider filenames ending in '.conf' */
-		len = strlen(de->d_name);
-		if (len <= 5)
-			continue;
-		if (strcmp(de->d_name + len - 5, ".conf"))
-			continue;
-
-		snprintf(path, sizeof(path),
-			 CONF_ETC_DIR "/rt_tables.d/%s", de->d_name);
-		rtnl_hash_initialize(path, rtnl_rttable_hash, 256);
-	}
-	closedir(d);
+	rtnl_hash_initialize_dir("rt_tables.d", rtnl_rttable_hash, 256);
 }
 
 const char *rtnl_rttable_n2a(__u32 id, char *buf, int len)
-- 
2.39.2

[iproute2 21/22] man: Document lookup of configuration files in /etc and /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 man/man8/Makefile        |  1 +
 man/man8/ip-address.8.in |  5 +++--
 man/man8/ip-link.8.in    | 10 ++++++----
 man/man8/ip-route.8.in   | 43 ++++++++++++++++++++++++----------------
 4 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/man/man8/Makefile b/man/man8/Makefile
index ae5e37a5..6dab182f 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -10,6 +10,7 @@ all: $(TARGETS)
 		-e "s|@NETNS_ETC_DIR@|$(NETNS_ETC_DIR)|g" \
 		-e "s|@NETNS_RUN_DIR@|$(NETNS_RUN_DIR)|g" \
 		-e "s|@SYSCONF_ETC_DIR@|$(CONF_ETC_DIR)|g" \
+		-e "s|@SYSCONF_USR_DIR@|$(CONF_USR_DIR)|g" \
 		$< > $@
 
 distclean: clean
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index a2df22d4..b9a476a5 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -208,8 +208,9 @@ The maximum allowed total length of label is 15 characters.
 .TP
 .BI scope " SCOPE_VALUE"
 the scope of the area where this address is valid.
-The available scopes are listed in file
-.BR "@SYSCONF_ETC_DIR@/rt_scopes" .
+The available scopes are listed in
+.BR "@SYSCONF_USR_DIR@/rt_scopes" or
+.BR "@SYSCONF_ETC_DIR@/rt_scopes" (has precedence if exists).
 Predefined scope values are:
 
 .in +8
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index ac1a3b5f..8f07de9a 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -2250,8 +2250,9 @@ give the device a symbolic name for easy reference.
 .TP
 .BI group " GROUP"
 specify the group the device belongs to.
-The available groups are listed in file
-.BR "@SYSCONF_ETC_DIR@/group" .
+The available groups are listed in
+.BR "@SYSCONF_USR_DIR@/group" or
+.BR "@SYSCONF_ETC_DIR@/group" (has precedence if exists).
 
 .TP
 .BI vf " NUM"
@@ -2851,9 +2852,10 @@ specifies which help of link type to display.
 
 .SS
 .I GROUP
-may be a number or a string from the file
+may be a number or a string from
+.B @SYSCONF_USR_DIR@/group or
 .B @SYSCONF_ETC_DIR@/group
-which can be manually filled.
+which can be manually filled and has precedence if exists.
 
 .SH "EXAMPLES"
 .PP
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index 9fc3f4a1..76151689 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -356,8 +356,9 @@ normal routing tables.
 .P
 .B Route tables:
 Linux-2.x can pack routes into several routing tables identified
-by a number in the range from 1 to 2^32-1 or by name from the file
-.B @SYSCONF_ETC_DIR@/rt_tables
+by a number in the range from 1 to 2^32-1 or by name from
+.B @SYSCONF_USR_DIR@/rt_tables or
+.B @SYSCONF_ETC_DIR@/rt_tables (has precedence if exists).
 By default all normal routes are inserted into the
 .B main
 table (ID 254) and the kernel only uses this table when calculating routes.
@@ -420,7 +421,8 @@ may still match a route with a zero TOS.
 .I TOS
 is either an 8 bit hexadecimal number or an identifier
 from
-.BR "@SYSCONF_ETC_DIR@/rt_dsfield" .
+.BR "@SYSCONF_USR_DIR@/rt_dsfield" or
+.BR "@SYSCONF_ETC_DIR@/rt_dsfield" (has precedence if exists).
 
 .TP
 .BI metric " NUMBER"
@@ -434,8 +436,9 @@ is an arbitrary 32bit number, where routes with lower values are preferred.
 .BI table " TABLEID"
 the table to add this route to.
 .I TABLEID
-may be a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_tables" .
+may be a number or a string from
+.BR "@SYSCONF_USR_DIR@/rt_tables" or
+.BR "@SYSCONF_ETC_DIR@/rt_tables" (has precedence if exists).
 If this parameter is omitted,
 .B ip
 assumes the
@@ -475,8 +478,9 @@ covered by the route prefix.
 .BI realm " REALMID"
 the realm to which this route is assigned.
 .I REALMID
-may be a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_realms" .
+may be a number or a string from
+.BR "@SYSCONF_USR_DIR@/rt_realms" or
+.BR "@SYSCONF_ETC_DIR@/rt_realms" (has precedence if exists).
 
 .TP
 .BI mtu " MTU"
@@ -626,8 +630,9 @@ command.
 .BI scope " SCOPE_VAL"
 the scope of the destinations covered by the route prefix.
 .I SCOPE_VAL
-may be a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_scopes" .
+may be a number or a string from
+.BR "@SYSCONF_USR_DIR@/rt_scopes" or
+.BR "@SYSCONF_ETC_DIR@/rt_scopes" (has precedence if exists).
 If this parameter is omitted,
 .B ip
 assumes scope
@@ -646,8 +651,9 @@ routes.
 .BI protocol " RTPROTO"
 the routing protocol identifier of this route.
 .I RTPROTO
-may be a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_protos" .
+may be a number or a string from
+.BR "@SYSCONF_ETC_DIR@/rt_protos" or
+.BR "@SYSCONF_ETC_DIR@/rt_protos" (has precedence if exists).
 If the routing protocol ID is not given,
 .B ip assumes protocol
 .B boot
@@ -879,8 +885,9 @@ matching packets are dropped.
 - Decapsulate the inner IPv6 packet and forward it according to the
 specified lookup table.
 .I TABLEID
-is either a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_tables" .
+is either a number or a string from
+.BR "@SYSCONF_USR_DIR@/rt_tables" or
+.BR "@SYSCONF_ETC_DIR@/rt_tables" (has precedence if exists).
 If
 .B vrftable
 is used, the argument must be a VRF device associated with
@@ -895,8 +902,9 @@ and an inner IPv6 packet. Other matching packets are dropped.
 - Decapsulate the inner IPv4 packet and forward it according to the
 specified lookup table.
 .I TABLEID
-is either a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_tables" .
+is either a number or a string from
+.BR "@SYSCONF_USR_DIR@/rt_tables" or
+.BR "@SYSCONF_ETC_DIR@/rt_tables" (has precedence if exists).
 The argument must be a VRF device associated with the table id.
 Moreover, the VRF table associated with the table id must be configured
 with the VRF strict mode turned on (net.vrf.strict_mode=1). This action
@@ -908,8 +916,9 @@ at all, and an inner IPv4 packet. Other matching packets are dropped.
 - Decapsulate the inner IPv4 or IPv6 packet and forward it according
 to the specified lookup table.
 .I TABLEID
-is either a number or a string from the file
-.BR "@SYSCONF_ETC_DIR@/rt_tables" .
+is either a number or a string from
+.BR "@SYSCONF_USR_DIR@/rt_tables" or
+.BR "@SYSCONF_ETC_DIR@/rt_tables" (has precedence if exists).
 The argument must be a VRF device associated with the table id.
 Moreover, the VRF table associated with the table id must be configured
 with the VRF strict mode turned on (net.vrf.strict_mode=1). This action
-- 
2.39.2

[iproute2 22/22] Makefile: Install default configuration files in /usr

[Gioele Barabucci]

Signed-off-by: Gioele Barabucci <gioele@svario.it>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a2af4426..7d1819ce 100644
--- a/Makefile
+++ b/Makefile
@@ -102,11 +102,11 @@ config.mk:
 
 install: all
 	install -m 0755 -d $(DESTDIR)$(SBINDIR)
-	install -m 0755 -d $(DESTDIR)$(CONF_ETC_DIR)
+	install -m 0755 -d $(DESTDIR)$(CONF_USR_DIR)
 	install -m 0755 -d $(DESTDIR)$(ARPDDIR)
 	install -m 0755 -d $(DESTDIR)$(HDRDIR)
 	@for i in $(SUBDIRS);  do $(MAKE) -C $$i install; done
-	install -m 0644 $(shell find etc/iproute2 -maxdepth 1 -type f) $(DESTDIR)$(CONF_ETC_DIR)
+	install -m 0644 $(shell find etc/iproute2 -maxdepth 1 -type f) $(DESTDIR)$(CONF_USR_DIR)
 	install -m 0755 -d $(DESTDIR)$(BASH_COMPDIR)
 	install -m 0644 bash-completion/tc $(DESTDIR)$(BASH_COMPDIR)
 	install -m 0644 bash-completion/devlink $(DESTDIR)$(BASH_COMPDIR)
-- 
2.39.2

Re: [iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[Stephen Hemminger]

On Wed, 19 Jul 2023 20:50:44 +0200
Gioele Barabucci <gioele@svario.it> wrote:

> Dear iproute2 maintainers,
> 
> this patch series adds support for the so called "stateless" configuration
> pattern, i.e. reading the default configuration from /usr while allowing
> overriding it in /etc, giving system administrators a way to define local
> configuration without changing any distro-provided files.
> 
> In practice this means that each configuration file FOO is loaded
> from /usr/lib/iproute2/FOO unless /etc/iproute2/FOO exists.

I don't understand the motivation for the change.
Is /etc going away in some future version of systemd?

Perhaps just using an an environment variable instead of hard coding
/etc/iproute2 directory.

I do like the conslidation of the initialize_dir code though.

Re: [iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[Gioele Barabucci]

On 19/07/23 23:36, Stephen Hemminger wrote:
> On Wed, 19 Jul 2023 20:50:44 +0200
> Gioele Barabucci <gioele@svario.it> wrote:
> 
>> this patch series adds support for the so called "stateless" configuration
>> pattern, i.e. reading the default configuration from /usr while allowing
>> overriding it in /etc, giving system administrators a way to define local
>> configuration without changing any distro-provided files.
>>
>> In practice this means that each configuration file FOO is loaded
>> from /usr/lib/iproute2/FOO unless /etc/iproute2/FOO exists.
> 
> I don't understand the motivation for the change.

The main, but not the only, motivation for stateless systems is explained in

https://clearlinux.org/features/stateless
https://fedoraproject.org/wiki/StatelessLinux
https://summit.debconf.org/debconf15/meeting/276/stateless-cloud-friendly-debian/

In a nutshell: to better support factory resets, shared read-only base 
systems, containers & Co, all software should work even without /etc.

A nice side effect of adopting stateless-style configuration (read from 
/etc, fallback to /usr) is that it allows for distro-provided files to 
be strictly read-only, avoiding a bunch of common failures during 
updates and upgrades (Debian spends a huge amount of resources to 
correctly handle these so called "conffiles". The fewer, the better.)

> Is /etc going away in some future version of systemd?
This is unrelated to systemd, although systemd is probably the most well 
known software that uses this pattern.

> Perhaps just using an an environment variable instead of hard coding
> /etc/iproute2 directory.

Build-time or run-time env variable?

I'd say that run-time env variables (a là XDG Base Directory) are kind 
of hard to deal with in a command like `ip` that is often invoked via 
`sudo` (that filters and changes env in complex ways).

BTW, I strongly suggest to just go with this common pattern that is now 
known to all sysadmins instead of inventing an ad-hoc way to move the 
default configuration away from /etc.

> I do like the conslidation of the initialize_dir code though.

Thanks. :)

Regards,

-- 
Gioele Barabucci

Re: [iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[Petr Machata]

Gioele Barabucci <gioele@svario.it> writes:

> On 19/07/23 23:36, Stephen Hemminger wrote:
>> On Wed, 19 Jul 2023 20:50:44 +0200
>> Gioele Barabucci <gioele@svario.it> wrote:
>> 
>>> this patch series adds support for the so called "stateless" configuration
>>> pattern, i.e. reading the default configuration from /usr while allowing
>>> overriding it in /etc, giving system administrators a way to define local
>>> configuration without changing any distro-provided files.
>>>
>>> In practice this means that each configuration file FOO is loaded
>>> from /usr/lib/iproute2/FOO unless /etc/iproute2/FOO exists.
>> I don't understand the motivation for the change.
>
> The main, but not the only, motivation for stateless systems is explained in
>
> https://clearlinux.org/features/stateless
> https://fedoraproject.org/wiki/StatelessLinux
> https://summit.debconf.org/debconf15/meeting/276/stateless-cloud-friendly-debian/
>
> In a nutshell: to better support factory resets, shared read-only base systems, containers & Co, all
> software should work even without /etc.
>
> A nice side effect of adopting stateless-style configuration (read from /etc, fallback to /usr) is
> that it allows for distro-provided files to be strictly read-only, avoiding a bunch of common
> failures during updates and upgrades (Debian spends a huge amount of resources to correctly handle
> these so called "conffiles". The fewer, the better.)
>
>> Is /etc going away in some future version of systemd?
> This is unrelated to systemd, although systemd is probably the most well known software that uses
> this pattern.
>
>> Perhaps just using an an environment variable instead of hard coding
>> /etc/iproute2 directory.
>
> Build-time or run-time env variable?
>
> I'd say that run-time env variables (a là XDG Base Directory) are kind of hard to deal with in a
> command like `ip` that is often invoked via `sudo` (that filters and changes env in complex ways).
>
> BTW, I strongly suggest to just go with this common pattern that is now known to all sysadmins
> instead of inventing an ad-hoc way to move the default configuration away from /etc.
>
>> I do like the conslidation of the initialize_dir code though.
>
> Thanks. :)
>
> Regards,

OK, I looked at a couple bits, and the code looks reasonable overall.
I'll wait for resolution of this discussion before doing full formal
review, so as not to waste effort unnecessarily.

Re: [iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[Stephen Hemminger]

On Wed, 19 Jul 2023 20:50:44 +0200
Gioele Barabucci <gioele@svario.it> wrote:

> Dear iproute2 maintainers,
> 
> this patch series adds support for the so called "stateless" configuration
> pattern, i.e. reading the default configuration from /usr while allowing
> overriding it in /etc, giving system administrators a way to define local
> configuration without changing any distro-provided files.
> 
> In practice this means that each configuration file FOO is loaded
> from /usr/lib/iproute2/FOO unless /etc/iproute2/FOO exists.

These files are not something the typical user ever looks at or changes.
Please explain why all this churn is necessary and consoldate into
one patch. Trying to manage/review 22 little pieces doesn't make much
sense.

Re: [iproute2 00/22] Support for stateless configuration (read from /etc and /usr)

[Gioele Barabucci]

On 25/07/23 03:40, Stephen Hemminger wrote:
> On Wed, 19 Jul 2023 20:50:44 +0200
> Gioele Barabucci <gioele@svario.it> wrote:
> 
>> Dear iproute2 maintainers,
>>
>> this patch series adds support for the so called "stateless" configuration
>> pattern, i.e. reading the default configuration from /usr while allowing
>> overriding it in /etc, giving system administrators a way to define local
>> configuration without changing any distro-provided files.
>>
>> In practice this means that each configuration file FOO is loaded
>> from /usr/lib/iproute2/FOO unless /etc/iproute2/FOO exists.
> 
> These files are not something the typical user ever looks at or changes.
> Please explain why all this churn is necessary

Dear Stephen,

I fully agree that these files are rarely if ever modified.

However I assumed that you wished them to remain configurable given that:

1) these files are in /etc, suggesting that they are normal system-wide 
configuration files, and
2) the const is called CONFDIR,
3) the man pages refer to these files as modifiable files, for example, 
ip-link: «GROUP may be a number or a string from the file 
/etc/iproute2/group which can be manually filled.».
4) there are a few guides around the Web that suggest to add entries to 
these files.

If these files are to be configurable, then they should follow the 
stateless pattern (default provided by distro in /usr, local sysadmin 
override /etc).

If these files are not supposed to be configurable and are just 
convenience listings of settings to be considered hard-coded, then 
changing CONFDIR to /usr/lib/iproute2 is the simplest way to make 
iproute2 stateless (i.e. working in cases where /etc is not present).

Would you prefer, instead of this patch series, a patch that simply 
changes CONFDIR to /usr/lib/iproute2/?

Regards,

-- 
Gioele Barabucci