[PATCH iproute2-next] arpd: create /var/lib/arpd on first use

[PATCH iproute2-next] arpd: create /var/lib/arpd on first use

[Max Gautier]

The motivation is to build distributions packages without /var to go
towards stateless systems, see link below (TL;DR: provisionning anything
outside of /usr on boot).

We only try do create the database directory when it's in the default
location, and assume its parent (/var/lib in the usual case) exists.

Links: https://0pointer.net/blog/projects/stateless.html
---
Instead of modifying the default location, I opted to create it at
runtime, but only for the default location and assuming that /var/lib
exists. My thinking is that not changing defaults is somewhat better,
plus using /var/tmp directly might cause security concerns (I don't know
that it does, but at least someone could create a db file which the root
user would then open by default. Not sure what that could cause, so I'd
rather avoid it).

 Makefile    |  2 +-
 misc/arpd.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 8024d45e..2b2c3dec 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,7 @@ 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)\" \
+         -DARPDDIR=\"$(ARPDDIR)\" \
          -DCONF_COLOR=$(CONF_COLOR)
 
 #options for AX.25
@@ -104,7 +105,6 @@ config.mk:
 install: all
 	install -m 0755 -d $(DESTDIR)$(SBINDIR)
 	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_USR_DIR)
diff --git a/misc/arpd.c b/misc/arpd.c
index 1ef837c6..a133226c 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -19,6 +19,7 @@
 #include <fcntl.h>
 #include <sys/uio.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/time.h>
 #include <time.h>
 #include <signal.h>
@@ -35,7 +36,8 @@
 #include "rt_names.h"
 
 DB	*dbase;
-char	*dbname = "/var/lib/arpd/arpd.db";
+char const * const	default_dbname = ARPDDIR "/arpd.db";
+char const	*dbname = default_dbname;
 
 int	ifnum;
 int	*ifvec;
@@ -668,6 +670,14 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (default_dbname == dbname
+			&& mkdir(ARPDDIR, 0755) != 0
+			&& errno != EEXIST
+			) {
+		perror("create_db_dir");
+		exit(-1);
+	}
+
 	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
 	if (dbase == NULL) {
 		perror("db_open");
-- 
2.44.0

Re: [PATCH iproute2-next] arpd: create /var/lib/arpd on first use

[Stephen Hemminger]

On Sat, 16 Mar 2024 10:06:44 +0100
Max Gautier <mg@max.gautier.name> wrote:

> The motivation is to build distributions packages without /var to go
> towards stateless systems, see link below (TL;DR: provisionning anything
> outside of /usr on boot).
> 
> We only try do create the database directory when it's in the default
> location, and assume its parent (/var/lib in the usual case) exists.
> 
> Links: https://0pointer.net/blog/projects/stateless.html
> ---
> Instead of modifying the default location, I opted to create it at
> runtime, but only for the default location and assuming that /var/lib
> exists. My thinking is that not changing defaults is somewhat better,
> plus using /var/tmp directly might cause security concerns (I don't know
> that it does, but at least someone could create a db file which the root
> user would then open by default. Not sure what that could cause, so I'd
> rather avoid it).
> 
>  Makefile    |  2 +-
>  misc/arpd.c | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 8024d45e..2b2c3dec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -42,6 +42,7 @@ 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)\" \
> +         -DARPDDIR=\"$(ARPDDIR)\" \
>           -DCONF_COLOR=$(CONF_COLOR)
>  
>  #options for AX.25
> @@ -104,7 +105,6 @@ config.mk:
>  install: all
>  	install -m 0755 -d $(DESTDIR)$(SBINDIR)
>  	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_USR_DIR)
> diff --git a/misc/arpd.c b/misc/arpd.c
> index 1ef837c6..a133226c 100644
> --- a/misc/arpd.c
> +++ b/misc/arpd.c
> @@ -19,6 +19,7 @@
>  #include <fcntl.h>
>  #include <sys/uio.h>
>  #include <sys/socket.h>
> +#include <sys/stat.h>
>  #include <sys/time.h>
>  #include <time.h>
>  #include <signal.h>
> @@ -35,7 +36,8 @@
>  #include "rt_names.h"
>  
>  DB	*dbase;
> -char	*dbname = "/var/lib/arpd/arpd.db";
> +char const * const	default_dbname = ARPDDIR "/arpd.db";

Make this an array.
const char *default_dbname[] = ARPDDIR "/arpd.db";


> +char const	*dbname = default_dbname;
>  
>  int	ifnum;
>  int	*ifvec;
> @@ -668,6 +670,14 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> +	if (default_dbname == dbname
> +			&& mkdir(ARPDDIR, 0755) != 0
> +			&& errno != EEXIST
> +			) {
> +		perror("create_db_dir");
> +		exit(-1);
> +	}
> +
>  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
>  	if (dbase == NULL) {
>  		perror("db_open");

Missing signed-off-by

Re: [PATCH iproute2-next] arpd: create /var/lib/arpd on first use

[Jay Vosburgh]

Stephen Hemminger <stephen@networkplumber.org> wrote:

>On Sat, 16 Mar 2024 10:06:44 +0100
>Max Gautier <mg@max.gautier.name> wrote:
>
>> The motivation is to build distributions packages without /var to go
>> towards stateless systems, see link below (TL;DR: provisionning anything
>> outside of /usr on boot).
>> 
>> We only try do create the database directory when it's in the default
>> location, and assume its parent (/var/lib in the usual case) exists.
>> 
>> Links: https://0pointer.net/blog/projects/stateless.html
>> ---
>> Instead of modifying the default location, I opted to create it at
>> runtime, but only for the default location and assuming that /var/lib
>> exists. My thinking is that not changing defaults is somewhat better,
>> plus using /var/tmp directly might cause security concerns (I don't know
>> that it does, but at least someone could create a db file which the root
>> user would then open by default. Not sure what that could cause, so I'd
>> rather avoid it).
>> 
>>  Makefile    |  2 +-
>>  misc/arpd.c | 12 +++++++++++-
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index 8024d45e..2b2c3dec 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -42,6 +42,7 @@ 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)\" \
>> +         -DARPDDIR=\"$(ARPDDIR)\" \
>>           -DCONF_COLOR=$(CONF_COLOR)
>>  
>>  #options for AX.25
>> @@ -104,7 +105,6 @@ config.mk:
>>  install: all
>>  	install -m 0755 -d $(DESTDIR)$(SBINDIR)
>>  	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_USR_DIR)
>> diff --git a/misc/arpd.c b/misc/arpd.c
>> index 1ef837c6..a133226c 100644
>> --- a/misc/arpd.c
>> +++ b/misc/arpd.c
>> @@ -19,6 +19,7 @@
>>  #include <fcntl.h>
>>  #include <sys/uio.h>
>>  #include <sys/socket.h>
>> +#include <sys/stat.h>
>>  #include <sys/time.h>
>>  #include <time.h>
>>  #include <signal.h>
>> @@ -35,7 +36,8 @@
>>  #include "rt_names.h"
>>  
>>  DB	*dbase;
>> -char	*dbname = "/var/lib/arpd/arpd.db";
>> +char const * const	default_dbname = ARPDDIR "/arpd.db";
>
>Make this an array.
>const char *default_dbname[] = ARPDDIR "/arpd.db";

	I suspect this should be

const char default_dbname[] = ARPDDIR "/arpd.db";

	i.e., no "*" before "default_dbname", to match the type of
dbname (below).

>> +char const	*dbname = default_dbname;
>>  
>>  int	ifnum;
>>  int	*ifvec;
>> @@ -668,6 +670,14 @@ int main(int argc, char **argv)
>>  		}
>>  	}
>>  
>> +	if (default_dbname == dbname
>> +			&& mkdir(ARPDDIR, 0755) != 0
>> +			&& errno != EEXIST
>> +			) {
>> +		perror("create_db_dir");
>> +		exit(-1);
>> +	}
>> +

	Should this be a string comparison?  I don't think the pointer
comparison "default_dbname == dbname" will do what you expect if a user
specifies "-b" with the default value of ARPDIR "/arpd.db" as its
argument (i.e., the pointers won't match, but the actual text is the
same).

	-J

>>  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
>>  	if (dbase == NULL) {
>>  		perror("db_open");
>
>Missing signed-off-by
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

Re: [PATCH iproute2-next] arpd: create /var/lib/arpd on first use

[Max Gautier]

Le 16 mars 2024 20:56:37 GMT+01:00, Jay Vosburgh <jay.vosburgh@canonical.com> a écrit :
>Stephen Hemminger <stephen@networkplumber.org> wrote:
>
>>On Sat, 16 Mar 2024 10:06:44 +0100
>>Max Gautier <mg@max.gautier.name> wrote:
>>
>>> The motivation is to build distributions packages without /var to go
>>> towards stateless systems, see link below (TL;DR: provisionning anything
>>> outside of /usr on boot).
>>> 
>>> We only try do create the database directory when it's in the default
>>> location, and assume its parent (/var/lib in the usual case) exists.
>>> 
>>> Links: https://0pointer.net/blog/projects/stateless.html
>>> ---
>>> Instead of modifying the default location, I opted to create it at
>>> runtime, but only for the default location and assuming that /var/lib
>>> exists. My thinking is that not changing defaults is somewhat better,
>>> plus using /var/tmp directly might cause security concerns (I don't know
>>> that it does, but at least someone could create a db file which the root
>>> user would then open by default. Not sure what that could cause, so I'd
>>> rather avoid it).
>>> 
>>>  Makefile    |  2 +-
>>>  misc/arpd.c | 12 +++++++++++-
>>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/Makefile b/Makefile
>>> index 8024d45e..2b2c3dec 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -42,6 +42,7 @@ 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)\" \
>>> +         -DARPDDIR=\"$(ARPDDIR)\" \
>>>           -DCONF_COLOR=$(CONF_COLOR)
>>>  
>>>  #options for AX.25
>>> @@ -104,7 +105,6 @@ config.mk:
>>>  install: all
>>>  	install -m 0755 -d $(DESTDIR)$(SBINDIR)
>>>  	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_USR_DIR)
>>> diff --git a/misc/arpd.c b/misc/arpd.c
>>> index 1ef837c6..a133226c 100644
>>> --- a/misc/arpd.c
>>> +++ b/misc/arpd.c
>>> @@ -19,6 +19,7 @@
>>>  #include <fcntl.h>
>>>  #include <sys/uio.h>
>>>  #include <sys/socket.h>
>>> +#include <sys/stat.h>
>>>  #include <sys/time.h>
>>>  #include <time.h>
>>>  #include <signal.h>
>>> @@ -35,7 +36,8 @@
>>>  #include "rt_names.h"
>>>  
>>>  DB	*dbase;
>>> -char	*dbname = "/var/lib/arpd/arpd.db";
>>> +char const * const	default_dbname = ARPDDIR "/arpd.db";
>>
>>Make this an array.
>>const char *default_dbname[] = ARPDDIR "/arpd.db";
>
>	I suspect this should be
>
>const char default_dbname[] = ARPDDIR "/arpd.db";
>
>	i.e., no "*" before "default_dbname", to match the type of
>dbname (below).

Yeah, does not compile otherwise, that was my guess as well.

>
>>> +char const	*dbname = default_dbname;
>>>  
>>>  int	ifnum;
>>>  int	*ifvec;
>>> @@ -668,6 +670,14 @@ int main(int argc, char **argv)
>>>  		}
>>>  	}
>>>  
>>> +	if (default_dbname == dbname
>>> +			&& mkdir(ARPDDIR, 0755) != 0
>>> +			&& errno != EEXIST
>>> +			) {
>>> +		perror("create_db_dir");
>>> +		exit(-1);
>>> +	}
>>> +
>
>	Should this be a string comparison?  I don't think the pointer
>comparison "default_dbname == dbname" will do what you expect if a user
>specifies "-b" with the default value of ARPDIR "/arpd.db" as its
>argument (i.e., the pointers won't match, but the actual text is the
>same).
>
>	-J

I did consider that, the reasoning was mostly something like "if you specify a custom location, you're on your own".
But a string compare is probably best, it preserve the observable behavior of arpd and is overall less surprising. 

I'll send a v2 shortly.

>
>>>  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
>>>  	if (dbase == NULL) {
>>>  		perror("db_open");
>>
>>Missing signed-off-by
>>
>
>---
>	-Jay Vosburgh, jay.vosburgh@canonical.com

[PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Max Gautier]

The motivation is to build distributions packages without /var to go
towards stateless systems, see link below (TL;DR: provisionning anything
outside of /usr on boot).

We only try do create the database directory when it's in the default
location, and assume its parent (/var/lib in the usual case) exists.

Links: https://0pointer.net/blog/projects/stateless.html
Signed-off-by: Max Gautier <mg@max.gautier.name>
---
 Makefile    |  2 +-
 misc/arpd.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 8024d45e..2b2c3dec 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,7 @@ 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)\" \
+         -DARPDDIR=\"$(ARPDDIR)\" \
          -DCONF_COLOR=$(CONF_COLOR)
 
 #options for AX.25
@@ -104,7 +105,6 @@ config.mk:
 install: all
 	install -m 0755 -d $(DESTDIR)$(SBINDIR)
 	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_USR_DIR)
diff --git a/misc/arpd.c b/misc/arpd.c
index 1ef837c6..a64888aa 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -19,6 +19,7 @@
 #include <fcntl.h>
 #include <sys/uio.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/time.h>
 #include <time.h>
 #include <signal.h>
@@ -35,7 +36,8 @@
 #include "rt_names.h"
 
 DB	*dbase;
-char	*dbname = "/var/lib/arpd/arpd.db";
+char const	default_dbname[] = ARPDDIR "/arpd.db";
+char const	*dbname = default_dbname;
 
 int	ifnum;
 int	*ifvec;
@@ -668,6 +670,14 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (strcmp(default_dbname, dbname) == 0
+			&& mkdir(ARPDDIR, 0755) != 0
+			&& errno != EEXIST
+			) {
+		perror("create_db_dir");
+		exit(-1);
+	}
+
 	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
 	if (dbase == NULL) {
 		perror("db_open");
-- 
2.44.0

Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Stephen Hemminger]

On Sun, 17 Mar 2024 10:01:24 +0100
Max Gautier <mg@max.gautier.name> wrote:

> +	if (strcmp(default_dbname, dbname) == 0
> +			&& mkdir(ARPDDIR, 0755) != 0
> +			&& errno != EEXIST
> +			) {
> +		perror("create_db_dir");
> +		exit(-1);
> +	}

Please put closing paren on same line after EEXIST

Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Max Gautier]

Le 17 mars 2024 17:03:32 GMT+01:00, Meiyong Yu <meiyong.yu@126.com> a écrit :
>
>
>> On Mar 17, 2024, at 17:04, Max Gautier <mg@max.gautier.name> wrote:
>> 
>> The motivation is to build distributions packages without /var to go
>> towards stateless systems, see link below (TL;DR: provisionning anything
>> outside of /usr on boot).
>> 
>> We only try do create the database directory when it's in the default
>> location, and assume its parent (/var/lib in the usual case) exists.
>> 
>> Links: https://0pointer.net/blog/projects/stateless.html
>> Signed-off-by: Max Gautier <mg@max.gautier.name>
>> ---
>> Makefile    |  2 +-
>> misc/arpd.c | 12 +++++++++++-
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index 8024d45e..2b2c3dec 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -42,6 +42,7 @@ 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)\" \
>> +         -DARPDDIR=\"$(ARPDDIR)\" \
>>          -DCONF_COLOR=$(CONF_COLOR)
>> 
>> #options for AX.25
>> @@ -104,7 +105,6 @@ config.mk:
>> install: all
>>    install -m 0755 -d $(DESTDIR)$(SBINDIR)
>>    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_USR_DIR)
>> diff --git a/misc/arpd.c b/misc/arpd.c
>> index 1ef837c6..a64888aa 100644
>> --- a/misc/arpd.c
>> +++ b/misc/arpd.c
>> @@ -19,6 +19,7 @@
>> #include <fcntl.h>
>> #include <sys/uio.h>
>> #include <sys/socket.h>
>> +#include <sys/stat.h>
>> #include <sys/time.h>
>> #include <time.h>
>> #include <signal.h>
>> @@ -35,7 +36,8 @@
>> #include "rt_names.h"
>> 
>> DB    *dbase;
>> -char    *dbname = "/var/lib/arpd/arpd.db";
>> +char const    default_dbname[] = ARPDDIR "/arpd.db";
>> +char const    *dbname = default_dbname;
>> 
>> int    ifnum;
>> int    *ifvec;
>> @@ -668,6 +670,14 @@ int main(int argc, char **argv)
>>        }
>>    }
>> 
>> +    if (strcmp(default_dbname, dbname) == 0
>> +            && mkdir(ARPDDIR, 0755) != 0
>> +            && errno != EEXIST
>> +            ) {
>> +        perror("create_db_dir");
>> +        exit(-1);
>> +    }
>> +
>>    dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
>>    if (dbase == NULL) {
>>        perror("db_open");
>> --
>> 2.44.0
>> 
>
>if (strcmp(default_dbname, dbname) == 0
>
>Do you consider when the input dbname is relative path, for example: ../../var/lib/arpd/arpd.db, anther example: //var//lib//arpd//arpd.db

I don't. IMO this is a corner case which isn't worth doing path normalization.
Symlinks are not considered either.

Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Ratheesh Kannoth]

On 2024-03-17 at 14:31:24, Max Gautier (mg@max.gautier.name) wrote:
> The motivation is to build distributions packages without /var to go
> towards stateless systems, see link below (TL;DR: provisionning anything
> outside of /usr on boot).
>
> We only try do create the database directory when it's in the default
> location, and assume its parent (/var/lib in the usual case) exists.
>
> Links: https://0pointer.net/blog/projects/stateless.html
> Signed-off-by: Max Gautier <mg@max.gautier.name>
> ---
>  Makefile    |  2 +-
>  misc/arpd.c | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 8024d45e..2b2c3dec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -42,6 +42,7 @@ 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)\" \
> +         -DARPDDIR=\"$(ARPDDIR)\" \
>           -DCONF_COLOR=$(CONF_COLOR)
>
>  #options for AX.25
> @@ -104,7 +105,6 @@ config.mk:
>  install: all
>  	install -m 0755 -d $(DESTDIR)$(SBINDIR)
>  	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_USR_DIR)
> diff --git a/misc/arpd.c b/misc/arpd.c
> index 1ef837c6..a64888aa 100644
> --- a/misc/arpd.c
> +++ b/misc/arpd.c
> @@ -19,6 +19,7 @@
>  #include <fcntl.h>
>  #include <sys/uio.h>
>  #include <sys/socket.h>
> +#include <sys/stat.h>
>  #include <sys/time.h>
>  #include <time.h>
>  #include <signal.h>
> @@ -35,7 +36,8 @@
>  #include "rt_names.h"
>
>  DB	*dbase;
> -char	*dbname = "/var/lib/arpd/arpd.db";
> +char const	default_dbname[] = ARPDDIR "/arpd.db";
> +char const	*dbname = default_dbname;
>
>  int	ifnum;
>  int	*ifvec;
> @@ -668,6 +670,14 @@ int main(int argc, char **argv)
>  		}
>  	}
>
> +	if (strcmp(default_dbname, dbname) == 0
> +			&& mkdir(ARPDDIR, 0755) != 0
> +			&& errno != EEXIST
why do you need errno != EEXIST case ? mkdir() will return error in this case as well.
> +			) {
> +		perror("create_db_dir");
> +		exit(-1);
> +	}
> +
>  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
>  	if (dbase == NULL) {
>  		perror("db_open");
> --
> 2.44.0
>

Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Max Gautier]

On Mon, Mar 18, 2024 at 08:26:13AM +0530, Ratheesh Kannoth wrote:
> On 2024-03-17 at 14:31:24, Max Gautier (mg@max.gautier.name) wrote:
> > The motivation is to build distributions packages without /var to go
> > towards stateless systems, see link below (TL;DR: provisionning anything
> > outside of /usr on boot).
> >
> > We only try do create the database directory when it's in the default
> > location, and assume its parent (/var/lib in the usual case) exists.
> >
> > Links: https://0pointer.net/blog/projects/stateless.html
> > Signed-off-by: Max Gautier <mg@max.gautier.name>
> > ---
> >  Makefile    |  2 +-
> >  misc/arpd.c | 12 +++++++++++-
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 8024d45e..2b2c3dec 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -42,6 +42,7 @@ 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)\" \
> > +         -DARPDDIR=\"$(ARPDDIR)\" \
> >           -DCONF_COLOR=$(CONF_COLOR)
> >
> >  #options for AX.25
> > @@ -104,7 +105,6 @@ config.mk:
> >  install: all
> >  	install -m 0755 -d $(DESTDIR)$(SBINDIR)
> >  	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_USR_DIR)
> > diff --git a/misc/arpd.c b/misc/arpd.c
> > index 1ef837c6..a64888aa 100644
> > --- a/misc/arpd.c
> > +++ b/misc/arpd.c
> > @@ -19,6 +19,7 @@
> >  #include <fcntl.h>
> >  #include <sys/uio.h>
> >  #include <sys/socket.h>
> > +#include <sys/stat.h>
> >  #include <sys/time.h>
> >  #include <time.h>
> >  #include <signal.h>
> > @@ -35,7 +36,8 @@
> >  #include "rt_names.h"
> >
> >  DB	*dbase;
> > -char	*dbname = "/var/lib/arpd/arpd.db";
> > +char const	default_dbname[] = ARPDDIR "/arpd.db";
> > +char const	*dbname = default_dbname;
> >
> >  int	ifnum;
> >  int	*ifvec;
> > @@ -668,6 +670,14 @@ int main(int argc, char **argv)
> >  		}
> >  	}
> >
> > +	if (strcmp(default_dbname, dbname) == 0
> > +			&& mkdir(ARPDDIR, 0755) != 0
> > +			&& errno != EEXIST
> why do you need errno != EEXIST case ? mkdir() will return error in this case as well.

EEXIST is not an error in this case: if the default location already
exist, all is good. mkdir would still return -1 in this case, so we need
to exclude it manually.

> > +			) {
> > +		perror("create_db_dir");
> > +		exit(-1);
> > +	}
> > +
> >  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
> >  	if (dbase == NULL) {
> >  		perror("db_open");
> > --
> > 2.44.0
> >

-- 
Max Gautier

RE: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Ratheesh Kannoth]

> From: Max Gautier <mg@max.gautier.name>
> Sent: Monday, March 18, 2024 2:07 PM
> To: Ratheesh Kannoth <rkannoth@marvell.com>
> Cc: netdev@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd
> on first use

> > > +	if (strcmp(default_dbname, dbname) == 0
> > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > +			&& errno != EEXIST
> > why do you need errno != EEXIST case ? mkdir() will return error in this case
> as well.
> 
> EEXIST is not an error in this case: if the default location already exist, all is
> good. mkdir would still return -1 in this case, so we need to exclude it
> manually.

ACK. IMO, it would make a more readable code if you consider splitting the "if" loop. 


  
> 
> > > +			) {
> > > +		perror("create_db_dir");
> > > +		exit(-1);
> > > +	}
> > > +
> > >  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH,
> NULL);
> > >  	if (dbase == NULL) {
> > >  		perror("db_open");
> > > --
> > > 2.44.0
> > >
> 
> --
> Max Gautier

Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Max Gautier]

On Mon, Mar 18, 2024 at 08:51:40AM +0000, Ratheesh Kannoth wrote:
> > From: Max Gautier <mg@max.gautier.name>
> > Sent: Monday, March 18, 2024 2:07 PM
> > To: Ratheesh Kannoth <rkannoth@marvell.com>
> > Cc: netdev@vger.kernel.org
> > Subject: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd
> > on first use
> 
> > > > +	if (strcmp(default_dbname, dbname) == 0
> > > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > > +			&& errno != EEXIST
> > > why do you need errno != EEXIST case ? mkdir() will return error in this case
> > as well.
> > 
> > EEXIST is not an error in this case: if the default location already exist, all is
> > good. mkdir would still return -1 in this case, so we need to exclude it
> > manually.
> 
> ACK. IMO, it would make a more readable code if you consider splitting the "if" loop. 

Something like this ? I tend to pack conditions unless branching is
necessary, but no problem if this form is preferred.

if (strcmp(default_dbname, dbname) == 0) {
    if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
   ...
   }
}

> 
> 
>   
> > 
> > > > +			) {
> > > > +		perror("create_db_dir");
> > > > +		exit(-1);
> > > > +	}
> > > > +
> > > >  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH,
> > NULL);
> > > >  	if (dbase == NULL) {
> > > >  		perror("db_open");
> > > > --
> > > > 2.44.0
> > > >
> > 
> > --
> > Max Gautier

-- 
Max Gautier

RE: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Ratheesh Kannoth]

> From: Max Gautier <mg@max.gautier.name>
> Sent: Monday, March 18, 2024 2:29 PM
> To: Ratheesh Kannoth <rkannoth@marvell.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create
> /var/lib/arpd on first use
> 
> On Mon, Mar 18, 2024 at 08:51:40AM +0000, Ratheesh Kannoth wrote:
> > > From: Max Gautier <mg@max.gautier.name>
> > > Sent: Monday, March 18, 2024 2:07 PM
> > > To: Ratheesh Kannoth <rkannoth@marvell.com>
> > > Cc: netdev@vger.kernel.org
> > > Subject: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create
> > > /var/lib/arpd on first use
> >
> > > > > +	if (strcmp(default_dbname, dbname) == 0
> > > > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > > > +			&& errno != EEXIST
> > > > why do you need errno != EEXIST case ? mkdir() will return error
> > > > in this case
> > > as well.
> > >
> > > EEXIST is not an error in this case: if the default location already
> > > exist, all is good. mkdir would still return -1 in this case, so we
> > > need to exclude it manually.
> >
> > ACK. IMO, it would make a more readable code if you consider splitting the
> "if" loop.
> 
> Something like this ? I tend to pack conditions unless branching is necessary,
> but no problem if this form is preferred.
> 
> if (strcmp(default_dbname, dbname) == 0) {
>     if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
>    ...
>    }
> }
ACK.   
instead of errno != EXIST ,  you may consider stat() before mkdir() call. Just my way thinking(please ignore it, if you don't like). 
My thinking is --> you need to execute mkdir () only first time, second time onwards, stat() call will return 0.

Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Max Gautier]

On Mon, Mar 18, 2024 at 09:18:59AM +0000, Ratheesh Kannoth wrote:
> > > > > > +	if (strcmp(default_dbname, dbname) == 0
> > > > > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > > > > +			&& errno != EEXIST
> > > > > why do you need errno != EEXIST case ? mkdir() will return error
> > > > > in this case
> > > > as well.
> > > >
> > > > EEXIST is not an error in this case: if the default location already
> > > > exist, all is good. mkdir would still return -1 in this case, so we
> > > > need to exclude it manually.
> > >
> > > ACK. IMO, it would make a more readable code if you consider splitting the
> > "if" loop.
> > 
> > Something like this ? I tend to pack conditions unless branching is necessary,
> > but no problem if this form is preferred.
> > 
> > if (strcmp(default_dbname, dbname) == 0) {
> >     if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
> >    ...
> >    }
> > }
> ACK.   
> instead of errno != EXIST ,  you may consider stat() before mkdir() call. Just my way thinking(please ignore it, if you don't like). 
> My thinking is --> you need to execute mkdir () only first time, second time onwards, stat() call will return 0.

That's racy: we can stat and have a non existing folder, then have
another arpd instance (or anything else, really) create the directory,
and we would hit EEXIST anyway when we call mkdir.
Also, that needs two syscalls instead of one.

-- 
Max Gautier

Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Denis Kirjanov]

On 3/18/24 12:26, Max Gautier wrote:
> On Mon, Mar 18, 2024 at 09:18:59AM +0000, Ratheesh Kannoth wrote:
>>>>>>> +	if (strcmp(default_dbname, dbname) == 0
>>>>>>> +			&& mkdir(ARPDDIR, 0755) != 0
>>>>>>> +			&& errno != EEXIST
>>>>>> why do you need errno != EEXIST case ? mkdir() will return error
>>>>>> in this case
>>>>> as well.
>>>>>
>>>>> EEXIST is not an error in this case: if the default location already
>>>>> exist, all is good. mkdir would still return -1 in this case, so we
>>>>> need to exclude it manually.
>>>>
>>>> ACK. IMO, it would make a more readable code if you consider splitting the
>>> "if" loop.
>>>
>>> Something like this ? I tend to pack conditions unless branching is necessary,
>>> but no problem if this form is preferred.
>>>
>>> if (strcmp(default_dbname, dbname) == 0) {
>>>     if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
>>>    ...
>>>    }
>>> }
>> ACK.   
>> instead of errno != EXIST ,  you may consider stat() before mkdir() call. Just my way thinking(please ignore it, if you don't like). 
>> My thinking is --> you need to execute mkdir () only first time, second time onwards, stat() call will return 0.
> 
> That's racy: we can stat and have a non existing folder, then have
> another arpd instance (or anything else, really) create the directory,

Agreed ^^

> and we would hit EEXIST anyway when we call mkdir.
> Also, that needs two syscalls instead of one.
> 

RE: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Ratheesh Kannoth]

> From: Max Gautier <mg@max.gautier.name>
> Sent: Monday, March 18, 2024 2:57 PM
> To: Ratheesh Kannoth <rkannoth@marvell.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create
> /var/lib/arpd on first use
> 
> On Mon, Mar 18, 2024 at 09:18:59AM +0000, Ratheesh Kannoth wrote:
> > > > > > > +	if (strcmp(default_dbname, dbname) == 0
> > > > > > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > > > > > +			&& errno != EEXIST
> > > > > > why do you need errno != EEXIST case ? mkdir() will return
> > > > > > error in this case
> > > > > as well.
> > > > >
> > > > > EEXIST is not an error in this case: if the default location
> > > > > already exist, all is good. mkdir would still return -1 in this
> > > > > case, so we need to exclude it manually.
> > > >
> > > > ACK. IMO, it would make a more readable code if you consider
> > > > splitting the
> > > "if" loop.
> > >
> > > Something like this ? I tend to pack conditions unless branching is
> > > necessary, but no problem if this form is preferred.
> > >
> > > if (strcmp(default_dbname, dbname) == 0) {
> > >     if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
> > >    ...
> > >    }
> > > }
> > ACK.
> > instead of errno != EXIST ,  you may consider stat() before mkdir() call. Just
> my way thinking(please ignore it, if you don't like).
> > My thinking is --> you need to execute mkdir () only first time, second time
> onwards, stat() call will return 0.
> 
> That's racy: we can stat and have a non existing folder, then have another arpd
> instance (or anything else, really) create the directory, and we would hit EEXIST
> anyway when we call mkdir.
ACK. 

> Also, that needs two syscalls instead of one.
> 
> --
> Max Gautier

[PATCH iproute2-next v3] arpd: create /var/lib/arpd on first use

[Max Gautier]

The motivation is to build distributions packages without /var to go
towards stateless systems, see link below (TL;DR: provisionning anything
outside of /usr on boot).

We only try do create the database directory when it's in the default
location, and assume its parent (/var/lib in the usual case) exists.

Links: https://0pointer.net/blog/projects/stateless.html
Signed-off-by: Max Gautier <mg@max.gautier.name>
---
Some slight syntax changes from list feedback.
 Makefile    |  2 +-
 misc/arpd.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 8024d45e..2b2c3dec 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,7 @@ 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)\" \
+         -DARPDDIR=\"$(ARPDDIR)\" \
          -DCONF_COLOR=$(CONF_COLOR)
 
 #options for AX.25
@@ -104,7 +105,6 @@ config.mk:
 install: all
 	install -m 0755 -d $(DESTDIR)$(SBINDIR)
 	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_USR_DIR)
diff --git a/misc/arpd.c b/misc/arpd.c
index 1ef837c6..65ac6a38 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -19,6 +19,7 @@
 #include <fcntl.h>
 #include <sys/uio.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/time.h>
 #include <time.h>
 #include <signal.h>
@@ -35,7 +36,8 @@
 #include "rt_names.h"
 
 DB	*dbase;
-char	*dbname = "/var/lib/arpd/arpd.db";
+char const	default_dbname[] = ARPDDIR "/arpd.db";
+char const	*dbname = default_dbname;
 
 int	ifnum;
 int	*ifvec;
@@ -668,6 +670,13 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (strcmp(default_dbname, dbname) == 0) {
+		if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
+			perror("create_db_dir");
+			exit(-1);
+		}
+	}
+
 	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH, NULL);
 	if (dbase == NULL) {
 		perror("db_open");
-- 
2.44.0

Re: [PATCH iproute2-next v3] arpd: create /var/lib/arpd on first use

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Mon, 18 Mar 2024 16:49:13 +0100 you wrote:
> The motivation is to build distributions packages without /var to go
> towards stateless systems, see link below (TL;DR: provisionning anything
> outside of /usr on boot).
> 
> We only try do create the database directory when it's in the default
> location, and assume its parent (/var/lib in the usual case) exists.
> 
> [...]

Here is the summary with links:
  - [iproute2-next,v3] arpd: create /var/lib/arpd on first use
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=f740f5a165eb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html