[PATCH v2 6/6] dhcpcd: Add hooks

Sebastian Huber sebastian.huber at embedded-brains.de
Mon May 7 07:04:59 UTC 2018


On 04/05/18 02:42, Chris Johns wrote:
> On 03/05/2018 16:49, Sebastian Huber wrote:
>> ---
>>   dhcpcd/dhcpcd.c                 |  9 +++++++
>>   dhcpcd/namespace.h              |  1 +
>>   dhcpcd/script.c                 | 56 +++++++++++++++++++++++++++++++++++++++++
>>   dhcpcd/script.h                 |  6 +----
>>   libbsd.py                       |  1 +
>>   rtemsbsd/include/rtems/dhcpcd.h | 22 ++++++++++++++++
>>   testsuite/dhcpcd01/test_main.c  | 21 +++++++++++++++-
>>   7 files changed, 110 insertions(+), 6 deletions(-)
>>
>> diff --git a/dhcpcd/dhcpcd.c b/dhcpcd/dhcpcd.c
>> index 59562d635..c75aafa5e 100644
>> --- a/dhcpcd/dhcpcd.c
>> +++ b/dhcpcd/dhcpcd.c
>> @@ -1599,3 +1599,12 @@ main(int argc, char **argv)
>>   	eloop_start(&dhcpcd_sigset);
>>   	exit(EXIT_SUCCESS);
>>   }
>> +#ifdef __rtems__
>> +int
>> +dhcpcd_script_runreason_do_nothing(const struct interface *ifp,
>> +    const char *reason)
>> +{
>> +	return 0;
>> +}
>> +__weak_reference(dhcpcd_script_runreason_do_nothing, script_runreason);
> What is the use case to override this call?

The goal was to include the script.c stuff only if it is used. 
Apparently this didn't work as expected. I will fix this in v3.

>
>> +#endif /* __rtems__ */
>> diff --git a/dhcpcd/namespace.h b/dhcpcd/namespace.h
>> index efff909ad..c9f5fb1fa 100644
>> --- a/dhcpcd/namespace.h
>> +++ b/dhcpcd/namespace.h
>> @@ -156,6 +156,7 @@ extern rtems_recursive_mutex dhcpcd_mutex;
>>   #define print_string dhcpcd_print_string
>>   #define read_config dhcpcd_read_config
>>   #define read_lease dhcpcd_read_lease
>> +#define script_runreason dhcpcd_script_runreason
>>   #define select_profile dhcpcd_select_profile
>>   #define set_cloexec dhcpcd_set_cloexec
>>   #define set_nonblock dhcpcd_set_nonblock
>> diff --git a/dhcpcd/script.c b/dhcpcd/script.c
>> index 2de1e6347..2c184c248 100644
>> --- a/dhcpcd/script.c
>> +++ b/dhcpcd/script.c
>> @@ -51,7 +51,30 @@
>>   #include "ipv6nd.h"
>>   #include "net.h"
>>   #include "script.h"
>> +#ifdef __rtems__
>> +#include <rtems/dhcpcd.h>
>>   
>> +static SLIST_HEAD(, rtems_dhcpcd_hook) dhcpcd_hooks =
>> +  SLIST_HEAD_INITIALIZER(dhcpcd_hooks);
>> +
>> +void
>> +rtems_dhcpcd_add_hook(rtems_dhcpcd_hook *hook)
>> +{
>> +	rtems_recursive_mutex_lock(&dhcpcd_mutex);
>> +	SLIST_INSERT_HEAD(&dhcpcd_hooks, hook, node);
>> +	rtems_recursive_mutex_unlock(&dhcpcd_mutex);
>> +}
>> +
>> +void
>> +rtems_dhcpcd_remove_hook(rtems_dhcpcd_hook *hook)
>> +{
>> +	rtems_recursive_mutex_lock(&dhcpcd_mutex);
>> +	SLIST_REMOVE(&dhcpcd_hooks, hook, rtems_dhcpcd_hook, node);
>> +	rtems_recursive_mutex_unlock(&dhcpcd_mutex);
>> +}
>> +#endif /* __rtems__ */
> This seems like a common issue we will face with FreeBSD, I am sure callouts
> which normally exec's something would be needed else where. I am thinking of wifi?
>
> Could this be made a common base type struct and used here?

I am not sure if the handler signature will be the same.

>
>> +
>> +#ifndef __rtems__
>>   #define DEFAULT_PATH	"PATH=/usr/bin:/usr/sbin:/bin:/sbin"
>>   
>>   static const char * const if_params[] = {
>> @@ -75,10 +98,12 @@ if_printoptions(void)
>>   	for (p = if_params; *p; p++)
>>   		printf(" -  %s\n", *p);
>>   }
>> +#endif /* __rtems__ */
>>   
>>   static int
>>   exec_script(char *const *argv, char *const *env)
>>   {
>> +#ifndef __rtems__
>>   	pid_t pid;
>>   	posix_spawnattr_t attr;
>>   	short flags;
>> @@ -103,6 +128,20 @@ exec_script(char *const *argv, char *const *env)
>>   		return -1;
>>   	}
>>   	return pid;
>> +#else /* __rtems__ */
>> +	rtems_dhcpcd_hook *hook;
>> +	rtems_dhcpcd_hook *hook2;
>> +
>> +	rtems_recursive_mutex_lock(&dhcpcd_mutex);
>> +
>> +	SLIST_FOREACH_SAFE(hook, &dhcpcd_hooks, node, hook2) {
>> +		(*hook->handler)(env);
>> +	}
>> +
>> +	rtems_recursive_mutex_unlock(&dhcpcd_mutex);
>> +
>> +	return 0;
>> +#endif /* __rtems__ */
>>   }
>>   
>>   #ifdef INET
>> @@ -168,6 +207,7 @@ append_config(char ***env, ssize_t *len,
>>   }
>>   #endif
>>   
>> +#ifndef __rtems__
>>   static size_t
>>   arraytostr(const char *const *argv, char **s)
>>   {
>> @@ -191,6 +231,7 @@ arraytostr(const char *const *argv, char **s)
>>   	}
>>   	return len;
>>   }
>> +#endif /* __rtems__ */
>>   
>>   static ssize_t
>>   make_env(const struct interface *ifp, const char *reason, char ***argv)
>> @@ -435,6 +476,7 @@ eexit:
>>   	return -1;
>>   }
>>   
>> +#ifndef __rtems__
>>   static int
>>   send_interface1(int fd, const struct interface *iface, const char *reason)
>>   {
>> @@ -507,29 +549,37 @@ send_interface(int fd, const struct interface *iface)
>>   	}
>>   	return retval;
>>   }
>> +#endif /* __rtems__ */
>>   
>>   int
>>   script_runreason(const struct interface *ifp, const char *reason)
>>   {
>>   	char *const argv[2] = { UNCONST(ifp->options->script), NULL };
>>   	char **env = NULL, **ep;
>> +#ifndef __rtems__
>>   	char *path, *bigenv;
>> +#endif /* __rtems__ */
>>   	ssize_t e, elen = 0;
>>   	pid_t pid;
>>   	int status = 0;
>> +#ifndef __rtems__
>>   	const struct fd_list *fd;
>>   	struct iovec iov[2];
>> +#endif /* __rtems__ */
>>   
>>   	if (ifp->options->script == NULL ||
>>   	    ifp->options->script[0] == '\0' ||
>>   	    strcmp(ifp->options->script, "/dev/null") == 0)
>>   		return 0;
>>   
>> +#ifndef __rtems__
>>   	syslog(LOG_DEBUG, "%s: executing `%s' %s",
>>   	    ifp->name, argv[0], reason);
>> +#endif /* __rtems__ */
> Is there any ability to get some indication a hooked handler is being called?

You can do whatever you want in your hook handler.

>
> If this due to the exec being removed?

We don't execute a script here.

>
>>   
>>   	/* Make our env */
>>   	elen = make_env(ifp, reason, &env);
>> +#ifndef __rtems__
>>   	ep = realloc(env, sizeof(char *) * (elen + 2));
>>   	if (ep == NULL) {
>>   		elen = -1;
>> @@ -554,8 +604,10 @@ script_runreason(const struct interface *ifp, const char *reason)
>>   		}
>>   	}
>>   	env[++elen] = NULL;
>> +#endif /* __rtems__ */
>>   
>>   	pid = exec_script(argv, env);
>> +#ifndef __rtems__
>>   	if (pid == -1)
>>   		syslog(LOG_ERR, "%s: %s: %m", __func__, argv[0]);
>>   	else if (pid != 0) {
> Are you expecting no errors? Why not leave this in and use the result in the way
> it is intended?

The pid is normally returned by posix_spawn() which we don't have in RTEMS.

>
>> @@ -596,6 +648,10 @@ script_runreason(const struct interface *ifp, const char *reason)
>>   	free(bigenv);
>>   
>>   out:
>> +#else /* __rtems__ */
>> +	(void)e;
>> +	(void)pid;
>> +#endif /* __rtems__ */
>>   	/* Cleanup */
>>   	ep = env;
>>   	while (*ep)
>> diff --git a/dhcpcd/script.h b/dhcpcd/script.h
>> index dfd4a1726..9c5fd4d4e 100644
>> --- a/dhcpcd/script.h
>> +++ b/dhcpcd/script.h
>> @@ -33,16 +33,12 @@
>>   void if_printoptions(void);
>>   #ifndef __rtems__
>>   int send_interface(int, const struct interface *);
>> -int script_runreason(const struct interface *, const char *);
>>   #else /* __rtems__ */
>>   static inline int send_interface(int fd, const struct interface *iface)
>>   {
>>   	return 0;
>>   }
>> -static inline int script_runreason(const struct interface *ifp, const char *reason)
>> -{
>> -	return 0;
>> -}
>>   #endif /* __rtems__ */
>> +int script_runreason(const struct interface *, const char *);
>>   
>>   #endif
>> diff --git a/libbsd.py b/libbsd.py
>> index a62581faa..c842534b5 100644
>> --- a/libbsd.py
>> +++ b/libbsd.py
>> @@ -4582,6 +4582,7 @@ class dhcpcd(builder.Module):
>>                   'dhcpcd/ipv6nd.c',
>>                   'dhcpcd/net.c',
>>                   'dhcpcd/platform-bsd.c',
>> +                'dhcpcd/script.c',
>>                   'dhcpcd/compat/pselect.c',
>>                   'dhcpcd/crypt/hmac_md5.c',
>>               ],
>> diff --git a/rtemsbsd/include/rtems/dhcpcd.h b/rtemsbsd/include/rtems/dhcpcd.h
>> index 324c8a90b..c5c5cb54f 100644
>> --- a/rtemsbsd/include/rtems/dhcpcd.h
>> +++ b/rtemsbsd/include/rtems/dhcpcd.h
>> @@ -40,6 +40,9 @@
>>   #ifndef _RTEMS_DHCPCD_H_
>>   #define _RTEMS_DHCPCD_H_
>>   
>> +#include <sys/cdefs.h>
>> +#include <sys/queue.h>
>> +
>>   #include <rtems.h>
>>   
>>   #ifdef __cplusplus
>> @@ -75,6 +78,25 @@ typedef struct rtems_dhcpcd_config {
>>    */
>>   rtems_status_code rtems_dhcpcd_start(const rtems_dhcpcd_config *config);
>>   
>> +typedef struct rtems_dhcpcd_hook {
>> +	void (*handler)(char *const *env);
>> +	SLIST_ENTRY(rtems_dhcpcd_hook) node;
>> +} rtems_dhcpcd_hook;
> This looks to me like it would be useful elsewhere.

The handler signature looks quite special to me.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.




More information about the devel mailing list