Hi,

On Thu, 2017-07-06 at 15:15 +0200, Jonatan Schlag wrote:
> Fixes: #11404
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  Makefile.am                 |   5 +-
>  src/hooks/configs/dhcp      | 188
> ++++++++++++++++++++++++++++++++++++++++++++
>  src/hooks/configs/ipv4-dhcp | 122 ----------------------------
>  src/hooks/configs/ipv6-dhcp | 100 -----------------------
>  4 files changed, 190 insertions(+), 225 deletions(-)
>  create mode 100644 src/hooks/configs/dhcp
>  delete mode 100644 src/hooks/configs/ipv4-dhcp
>  delete mode 100644 src/hooks/configs/ipv6-dhcp
> 
> diff --git a/Makefile.am b/Makefile.am
> index 32e7166..3a42a03 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -196,12 +196,11 @@ INSTALL_EXEC_HOOKS += bridge-stp-install-hook
>  UNINSTALL_EXEC_HOOKS += bridge-stp-uninstall-hook
>  
>  dist_hooks_configs_SCRIPTS = \
> -	src/hooks/configs/ipv4-dhcp \
>  	src/hooks/configs/ipv4-static \
>  	src/hooks/configs/ipv6-auto \
> -	src/hooks/configs/ipv6-dhcp \
>  	src/hooks/configs/ipv6-static \
> -	src/hooks/configs/pppoe-server
> +	src/hooks/configs/pppoe-server \
> +	src/hooks/configs/dhcp

Can we have this in alphabetical order to avoid merge conflicts?

>  
>  dist_hooks_ports_SCRIPTS = \
>  	src/hooks/ports/batman-adv \
> diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
> new file mode 100644
> index 0000000..8f8b2f6
> --- /dev/null
> +++ b/src/hooks/configs/dhcp
> @@ -0,0 +1,188 @@
> +#!/bin/bash
> +####################################################################
> ###########
> +#                                                                   
>           #
> +# IPFire.org - A linux based
> firewall                                         #
> +# Copyright (C) 2010  Michael Tremer & Christian
> Schmidt                      #
> +#                                                                   
>           #
> +# This program is free software: you can redistribute it and/or
> modify        #
> +# it under the terms of the GNU General Public License as published
> by        #
> +# the Free Software Foundation, either version 3 of the License,
> or           #
> +# (at your option) any later
> version.                                         #
> +#                                                                   
>           #
> +# This program is distributed in the hope that it will be
> useful,             #
> +# but WITHOUT ANY WARRANTY; without even the implied warranty
> of              #
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> the               #
> +# GNU General Public License for more
> details.                                #
> +#                                                                   
>           #
> +# You should have received a copy of the GNU General Public
> License           #
> +# along with this program.  If not, see <http://www.gnu.org/licenses
> />.       #
> +#                                                                   
>           #
> +####################################################################
> ###########
> +
> +. /usr/lib/network/header-config
> +
> +HOOK_CONFIG_SETTINGS="HOOK DELAY ENABLE_IPV4 ENABLE_IPV6"
> +
> +# Default settings.
> +DELAY=0
> +ENABLE_IPV4="on"
> +ENABLE_IPV6="on"
> +
> +hook_check_config_settings() {
> +	assert isset DELAY
> +	assert isinteger DELAY
> +	assert isset ENABLE_IPV4
> +	assert isbool ENABLE_IPV4
> +	assert isset ENABLE_IPV6
> +	assert isbool ENABLE_IPV6
> +}
> +
> +hook_parse_cmdline() {
> +	while [ $# -gt 0 ]; do
> +		case "${1}" in
> +			--delay=*)
> +				DELAY="$(cli_get_val "${1}")"
> +				;;
> +			--enable-ipv4=*)
> +				ENABLE_IPV4="$(cli_get_val "${1}")"
> +				if ! isbool ${ENABLE_IPV4}; then
> +					log ERROR "--enable-ipv4
> accept only on of the following values: on off 1 0 no yes"
> +					return ${EXIT_ERROR}
> +				fi
> +				;;
> +			--enable-ipv6=*)
> +				ENABLE_IPV6="$(cli_get_val "${1}")"
> +				if ! isbool ${ENABLE_IPV6}; then
> +					log ERROR "--enable-ipv6
> accept only on of the following values: on off 1 0 no yes"
> +					return ${EXIT_ERROR}
> +				fi
> +				;;
> +			*)
> +				warning "Ignoring unknown option
> '${1}'"
> +				;;
> +		esac
> +		shift
> +	done
> +}

Did you consider --disable-ipv6 and --enable-ipv6 instead of passing a
value that is always a little bit hard to interpret?

And don't give the user options for bools. Just suggest one.

> +
> +hook_new() {
> +	local zone="${1}"
> +	shift
> +
> +	if zone_config_hook_is_configured ${zone} "dhcp"; then
> +		log ERROR "You can configure the dhcp hook only once
> for a zone"
> +		return ${EXIT_ERROR}
> +	fi
> +
> +	hook_parse_cmdline $@
> +
> +	# Check if the user disabled ipv4 and ipv6
> +
> +	if ! enabled ENABLE_IPV6 && ! enabled ENABLE_IPV4; then
> +		log ERROR "You disabled IPv4 and IPv6. At least one
> must be enabled"
> +	fi

IPv6 should always be first.

> +
> +	zone_config_settings_write "${zone}" "${HOOK}"
> +
> +	exit ${EXIT_OK}
> +}
> +
> +hook_up() {
> +	local zone=${1}
> +	local config=${2}
> +	shift 2
> +
> +	if ! device_exists ${zone}; then
> +		error "Zone '${zone}' doesn't exist."
> +		exit ${EXIT_ERROR}
> +	fi
> +
> +	zone_config_settings_read "${zone}" "${config}"
> +
> +	# Start dhclient for IPv4 on this zone if enabled.
> +	if enabled ENABLE_IPV4; then
> +		dhclient_start ${zone} ipv4
> +	fi
> +
> +	# Start dhclient for IPv6 on this zone if enabled.
> +	if enabled ENABLE_IPV6; then
> +		dhclient_start ${zone} ipv6
> +	fi

Same here.

> +
> +	exit ${EXIT_OK}
> +}
> +
> +hook_down() {
> +	local zone=${1}
> +	local config=${2}
> +	shift 2
> +
> +	if ! device_exists ${zone}; then
> +		error "Zone '${zone}' doesn't exist."
> +		exit ${EXIT_ERROR}
> +	fi
> +
> +	# Stop dhclient for IPv4 on this zone.
> +	dhclient_stop ${zone} ipv4
> +
> +	# Stop dhclient for IPv6 on this zone.
> +	dhclient_stop ${zone} ipv6

Same.

> +
> +	exit ${EXIT_OK}
> +}
> +
> +hook_status() {
> +	local zone=${1}
> +	local config=${2}
> +	shift 2
> +
> +	if ! device_exists ${zone}; then
> +		error "Zone '${zone}' doesn't exist."
> +		exit ${EXIT_ERROR}
> +	fi
> +
> +	zone_config_settings_read "${zone}" "${config}"
> +
> +	local status
> +	if dhclient_status ${zone} ipv4 || dhclient_status ${zone}
> ipv6; then
> +		status="${MSG_HOOK_UP}"
> +	else
> +		status="${MSG_HOOK_DOWN}"
> +	fi
> +	cli_statusline 3 "${HOOK}" "${status}"
> +
> +	cli_space
> +
> +	local proto
> +	for proto in "IPv6" "IPv4"; do
> +		local _proto=${proto,,}
> +
> +		cli_print_fmt1 3 "${proto}"
> +
> +		if enabled ENABLE_${proto^^}; then
> +			cli_print_fmt1 4 "Status" "enabled"
> +
> +			local address="$(db_get
> "${zone}/${_proto}/local-ip-address")"
> +			if isset address; then
> +				cli_print_fmt1 4 "Address"
> "${address}"
> +			fi
> +
> +			local gateway="$(db_get
> "${zone}/${_proto}/remote-ip-address")"
> +			if isset gateway; then
> +				cli_print_fmt1 4 "Gateway"
> "${gateway}"
> +			fi
> +		
> +			local dns_servers="$(db_get
> "${zone}/${_proto}/domain-name-servers")"
> +			if isset dns_servers; then
> +				cli_print_fmt1 4 "DNS Servers"
> "${dns_servers}"
> +			fi
> +		else
> +			cli_print_fmt1 4 "Status" "disabled"
> +		fi
> +
> +		cli_space
> +
> +	done
> +
> +	exit ${EXIT_OK}
> +}
> diff --git a/src/hooks/configs/ipv4-dhcp b/src/hooks/configs/ipv4-
> dhcp
> deleted file mode 100644
> index 39e0312..0000000
> --- a/src/hooks/configs/ipv4-dhcp
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -#!/bin/bash
> -####################################################################
> ###########
> -#                                                                   
>           #
> -# IPFire.org - A linux based
> firewall                                         #
> -# Copyright (C) 2010  Michael Tremer & Christian
> Schmidt                      #
> -#                                                                   
>           #
> -# This program is free software: you can redistribute it and/or
> modify        #
> -# it under the terms of the GNU General Public License as published
> by        #
> -# the Free Software Foundation, either version 3 of the License,
> or           #
> -# (at your option) any later
> version.                                         #
> -#                                                                   
>           #
> -# This program is distributed in the hope that it will be
> useful,             #
> -# but WITHOUT ANY WARRANTY; without even the implied warranty
> of              #
> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> the               #
> -# GNU General Public License for more
> details.                                #
> -#                                                                   
>           #
> -# You should have received a copy of the GNU General Public
> License           #
> -# along with this program.  If not, see <http://www.gnu.org/licenses
> />.       #
> -#                                                                   
>           #
> -####################################################################
> ###########
> -
> -. /usr/lib/network/header-config
> -
> -HOOK_CONFIG_SETTINGS="HOOK DELAY"
> -
> -# Default settings.
> -DELAY=0
> -
> -hook_check_config_settings() {
> -	assert isset DELAY
> -	assert isinteger DELAY
> -}
> -
> -hook_new() {
> -	local zone="${1}"
> -	shift
> -
> -	if zone_config_hook_is_configured ${zone} "ipv4-dhcp"; then
> -		log ERROR "You can configure the ipv4-dhcp hook only
> once for a zone"
> -		return ${EXIT_ERROR}
> -	fi
> -
> -	while [ $# -gt 0 ]; do
> -		case "${1}" in
> -			--delay=*)
> -				DELAY="$(cli_get_val "${1}")"
> -				;;
> -		esac
> -		shift
> -	done
> -
> -	zone_config_settings_write "${zone}" "${HOOK}"
> -
> -	exit ${EXIT_OK}
> -}
> -
> -hook_up() {
> -	local zone=${1}
> -	local config=${2}
> -	shift 2
> -
> -	if ! device_exists ${zone}; then
> -		error "Zone '${zone}' doesn't exist."
> -		exit ${EXIT_ERROR}
> -	fi
> -
> -	# Start dhclient for IPv4 on this zone.
> -	dhclient_start ${zone} ipv4
> -
> -	exit ${EXIT_OK}
> -}
> -
> -hook_down() {
> -	local zone=${1}
> -	local config=${2}
> -	shift 2
> -
> -	if ! device_exists ${zone}; then
> -		error "Zone '${zone}' doesn't exist."
> -		exit ${EXIT_ERROR}
> -	fi
> -
> -	# Stop dhclient for IPv4 on this zone.
> -	dhclient_stop ${zone} ipv4
> -
> -	exit ${EXIT_OK}
> -}
> -
> -hook_status() {
> -	local zone=${1}
> -	local config=${2}
> -	shift 2
> -
> -	if ! device_exists ${zone}; then
> -		error "Zone '${zone}' doesn't exist."
> -		exit ${EXIT_ERROR}
> -	fi
> -
> -	zone_config_settings_read "${zone}" "${config}"
> -
> -	local status
> -	if dhclient_status ${zone} ipv4; then
> -		status="${MSG_HOOK_UP}"
> -	else
> -		status="${MSG_HOOK_DOWN}"
> -	fi
> -	cli_statusline 3 "${HOOK}" "${status}"
> -
> -	cli_print_fmt1 3 "IPv4 address" "$(db_get
> "${zone}/ipv4/local-ip-address")"
> -	local gateway="$(db_get "${zone}/ipv4/remote-ip-address")"
> -	if isset gateway; then
> -		cli_print_fmt1 3 "Gateway" "${gateway}"
> -		cli_space
> -	fi
> -	local dns_servers="$(db_get "${zone}/ipv4/domain-name-
> servers")"
> -	if isset dns_servers; then
> -		cli_print_fmt1 3 "DNS-Servers" "${dns_servers}"
> -		cli_space
> -	fi
> -
> -	exit ${EXIT_OK}
> -}
> diff --git a/src/hooks/configs/ipv6-dhcp b/src/hooks/configs/ipv6-
> dhcp
> deleted file mode 100644
> index 74ec765..0000000
> --- a/src/hooks/configs/ipv6-dhcp
> +++ /dev/null
> @@ -1,100 +0,0 @@
> -#!/bin/bash
> -####################################################################
> ###########
> -#                                                                   
>           #
> -# IPFire.org - A linux based
> firewall                                         #
> -# Copyright (C) 2010  Michael Tremer & Christian
> Schmidt                      #
> -#                                                                   
>           #
> -# This program is free software: you can redistribute it and/or
> modify        #
> -# it under the terms of the GNU General Public License as published
> by        #
> -# the Free Software Foundation, either version 3 of the License,
> or           #
> -# (at your option) any later
> version.                                         #
> -#                                                                   
>           #
> -# This program is distributed in the hope that it will be
> useful,             #
> -# but WITHOUT ANY WARRANTY; without even the implied warranty
> of              #
> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> the               #
> -# GNU General Public License for more
> details.                                #
> -#                                                                   
>           #
> -# You should have received a copy of the GNU General Public
> License           #
> -# along with this program.  If not, see <http://www.gnu.org/licenses
> />.       #
> -#                                                                   
>           #
> -####################################################################
> ###########
> -
> -. /usr/lib/network/header-config
> -
> -HOOK_CONFIG_SETTINGS="HOOK"
> -
> -hook_new() {
> -	local zone="${1}"
> -	shift
> -
> -	if zone_config_hook_is_configured ${zone} "ipv6-dhcp"; then
> -		log ERROR "You can configure the ipv6-dhcp hook only
> once for a zone"
> -		return ${EXIT_ERROR}
> -	fi
> -
> -	zone_config_settings_write "${zone}" "${HOOK}"
> -
> -	exit ${EXIT_OK}
> -}
> -
> -hook_up() {
> -	local zone="${1}"
> -	local config="${2}"
> -	shift 2
> -
> -	if ! device_exists "${zone}"; then
> -		error "Zone '${zone}' doesn't exist."
> -		exit ${EXIT_ERROR}
> -	fi
> -
> -	# Start dhclient for IPv6 on this zone.
> -	dhclient_start "${zone}" "ipv6"
> -
> -	exit ${EXIT_OK}
> -}
> -
> -hook_down() {
> -	local zone="${1}"
> -	local config="${2}"
> -	shift 2
> -
> -	if ! device_exists "${zone}"; then
> -		error "Zone '${zone}' doesn't exist."
> -		exit ${EXIT_ERROR}
> -	fi
> -
> -	# Stop dhclient for IPv6 on this zone.
> -	dhclient_stop "${zone}" "ipv6"
> -
> -	exit ${EXIT_OK}
> -}
> -
> -hook_status() {
> -	local zone="${1}"
> -	local config="${2}"
> -	shift 2
> -
> -	if ! device_exists "${zone}"; then
> -		error "Zone '${zone}' doesn't exist."
> -		exit ${EXIT_ERROR}
> -	fi
> -
> -	zone_config_settings_read "${zone}" "${config}"
> -
> -	local status
> -	if dhclient_status "${zone}" "ipv6"; then
> -		status="${MSG_HOOK_UP}"
> -	else
> -		status="${MSG_HOOK_DOWN}"
> -	fi
> -	cli_statusline 3 "${HOOK}" "${status}"
> -
> -	cli_print_fmt1 3 "IPv6 address" "$(db_get
> "${zone}/ipv6/local-ip-address")"
> -	local gateway="$(db_get "${zone}/ipv6/remote-ip-address")"
> -	if isset gateway; then
> -		cli_print_fmt1 3 "Gateway" "${gateway}"
> -	fi
> -	cli_space
> -
> -	exit ${EXIT_OK}
> -}