dhcpcd-discuss

Comments on the patch.

On 2017-01-24 15:43, Philipp Gesang wrote:
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -760,7 +760,7 @@ make_message(struct bootp **bootpm, const struct
> interface *ifp, uint8_t type)
>  	if ((mtu = if_getmtu(ifp)) == -1)
>  		logger(ifp->ctx, LOG_ERR,
>  		    "%s: if_getmtu: %m", ifp->name);
> -	else if (mtu < MTU_MIN) {
> +	else if (mtu < MTU_MIN && ifo->options & DHCPCD_CONFIGURE_LINK) {
>  		if (if_setmtu(ifp, MTU_MIN) == -1)
>  			logger(ifp->ctx, LOG_ERR,
>  			    "%s: if_setmtu: %m", ifp->name);

So if interface MTU < MTU_MIN we can't actually send or recieve a DHCP 
message.
Is this the behaviour you want?

> diff --git a/if-linux.c b/if-linux.c
> index 26f4d3c..1957c6a 100644
> --- a/if-linux.c
> +++ b/if-linux.c
> @@ -226,6 +226,12 @@ if_init(struct interface *ifp)
>  		return errno == ENOENT ? 0 : -1;
>  	if (n == 1)
>  		return 0;
> +	if (!(ifp->ctx->options & DHCPCD_CONFIGURE_LINK)) {
> +		logger(ifp->ctx, LOG_DEBUG,
> +		       "%s: leaving IPv4 sysctl promote_secondaries unset",
> +		       ifp->name);
> +		return 0;
> +	}
>  	return write_path(path, "1") == -1 ? -1 : 0;
>  }
>
> diff --git a/if-options.c b/if-options.c
> index 7dd6a43..02b69db 100644
> --- a/if-options.c
> +++ b/if-options.c
> @@ -104,6 +104,7 @@
>  #define O_LASTLEASE_EXTEND	O_BASE + 46
>  #define O_INACTIVE		O_BASE + 47
>  #define	O_MUDURL		O_BASE + 48
> +#define	O_NOCONFIGURE		O_BASE + 49
>
>  const struct option cf_options[] = {
>  	{"background",      no_argument,       NULL, 'b'},
> @@ -205,6 +206,7 @@ const struct option cf_options[] = {
>  	{"lastleaseextend", no_argument,       NULL, O_LASTLEASE_EXTEND},
>  	{"inactive",        no_argument,       NULL, O_INACTIVE},
>  	{"mudurl",          required_argument, NULL, O_MUDURL},
> +	{"noconfigure",     no_argument,       NULL, O_NOCONFIGURE},
>  	{NULL,              0,                 NULL, '\0'}
>  };
>
> @@ -2146,6 +2148,10 @@ err_sla:
>  		}
>  		*ifo->mudurl = (uint8_t)s;
>  		break;
> +	case O_NOCONFIGURE:
> +		logger(ctx, LOG_DEBUG, "all: suppressing link configuration");

"all" isn't strictly true.
This could be a per interface option and ifname is passed to the 
function.
 +		logger(ctx, LOG_DEBUG, "%s: suppressing link configuration", 
ifname);

> +		ifo->options &= ~DHCPCD_CONFIGURE_LINK;
> +		break;
>  	default:
>  		return 0;
>  	}
> @@ -2251,7 +2257,8 @@ default_config(struct dhcpcd_ctx *ctx)
>  		logger(ctx, LOG_ERR, "%s: %m", __func__);
>  		return NULL;
>  	}
> -	ifo->options |= DHCPCD_IF_UP | DHCPCD_LINK | DHCPCD_INITIAL_DELAY;
> +	ifo->options |= DHCPCD_IF_UP | DHCPCD_LINK | DHCPCD_INITIAL_DELAY
> +		| DHCPCD_CONFIGURE_LINK;
>  	ifo->timeout = DEFAULT_TIMEOUT;
>  	ifo->reboot = DEFAULT_REBOOT;
>  	ifo->metric = -1;
> diff --git a/if-options.h b/if-options.h
> index 3f2eb04..93216f6 100644
> --- a/if-options.h
> +++ b/if-options.h
> @@ -118,6 +118,7 @@
>  #define DHCPCD_PRINT_PIDFILE		(1ULL << 59)
>  #define DHCPCD_ONESHOT			(1ULL << 60)
>  #define DHCPCD_INACTIVE			(1ULL << 61)
> +#define DHCPCD_CONFIGURE_LINK		(1ULL << 62)
>
>  #define DHCPCD_NODROP	(DHCPCD_EXITING | DHCPCD_PERSISTENT)
>
> diff --git a/ipv4.c b/ipv4.c
> index 57a19df..33efcea 100644
> --- a/ipv4.c
> +++ b/ipv4.c
> @@ -513,6 +513,7 @@ delete_address(struct interface *ifp)
>  	 * by a 3rd party. */
>  	if (state->addr == NULL ||
>  	    ifo->options & DHCPCD_INFORM ||
> +	    !(ifo->options & DHCPCD_CONFIGURE_LINK) ||
>  	    (ifo->options & DHCPCD_STATIC && ifo->req_addr.s_addr == 0))
>  		return 0;
>  	r = ipv4_deladdr(state->addr, 0);
> @@ -638,15 +639,20 @@ ipv4_addaddr(struct interface *ifp, const struct
> in_addr *addr,
>  		    ia->alias, ia->saddr);
>  #endif
>
> -	logger(ifp->ctx, LOG_DEBUG, "%s: adding IP address %s broadcast %s",
> -	    ifp->name, ia->saddr, inet_ntoa(*bcast));
> -	if (if_address(RTM_NEWADDR, ia) == -1) {
> -		if (errno != EEXIST)
> -			logger(ifp->ctx, LOG_ERR, "%s: if_addaddress: %m",
> -			    __func__);
> -		free(ia);
> -		return NULL;
> -	}
> +	if (ifp->options->options & DHCPCD_CONFIGURE_LINK) {
> +		logger(ifp->ctx, LOG_DEBUG, "%s: adding IP address %s broadcast %s",
> +		       ifp->name, ia->saddr, inet_ntoa(*bcast));
> +		if (if_address(RTM_NEWADDR, ia) == -1) {
> +			if (errno != EEXIST)
> +				logger(ifp->ctx, LOG_ERR, "%s: if_addaddress: %m",
> +				       __func__);
> +			free(ia);
> +			return NULL;
> +		}
> +	} else
> +		logger(ifp->ctx, LOG_DEBUG, "%s: skipping link config with lease "
> +		       "data IP address %s broadcast %s",
> +		       ifp->name, ia->saddr, inet_ntoa(*bcast));
>
>  #ifdef ALIAS_ADDR
>  	if (replaced) {
> diff --git a/route.c b/route.c
> index 1e6bd84..3da6cde 100644
> --- a/route.c
> +++ b/route.c
> @@ -290,6 +290,8 @@ rt_add(struct rt *nrt, struct rt *ort)
>  	    sa_is_unspecified(&nrt->rt_dest) &&
>  	    sa_is_unspecified(&nrt->rt_netmask))
>  		return false;
> +	if (!(options & DHCPCD_CONFIGURE_LINK))
> +		return false;
>
>  	rt_desc(ort == NULL ? "adding" : "changing", nrt);
>
> @@ -394,6 +396,8 @@ rt_delete(struct rt *rt)
>  {
>  	int retval;
>
> +	if (!(rt->rt_ifp->options->options &  DHCPCD_CONFIGURE_LINK))
> +		return false;
>  	rt_desc("deleting", rt);
>  	retval = if_route(RTM_DELETE, rt) == -1 ? false : true;
>  	if (!retval && errno != ENOENT && errno != ESRCH)

Otherwise, looks ok.

Thanks

Roy