From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: network@lists.ipfire.org
Subject: Re: [PATCH] dhcp: merge hooks ipv4-dhcp and ipv6-dhcp
Date: Tue, 11 Jul 2017 12:39:57 +0100
Message-ID: <A90C03E3-F5D0-4BCA-9BB9-494A0399B3EE@ipfire.org>
In-Reply-To: <1499687123-6711-1-git-send-email-jonatan.schlag@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============4551909663976661356=="
List-Id: <network.lists.ipfire.org>

--===============4551909663976661356==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

(Replying to this from my phone... let's see how this works...)

> On 10 Jul 2017, at 12:45 pm, Jonatan Schlag <jonatan.schlag(a)ipfire.org> w=
rote:
>=20
> We use on and off instead of true and false
> to keep the config files more humand readable.
>=20
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> Makefile.am                 |   3 +-
> src/hooks/configs/dhcp      | 223 +++++++++++++++++++++++++++++++++++++++++=
+++
> src/hooks/configs/ipv4-dhcp | 122 ------------------------
> src/hooks/configs/ipv6-dhcp | 100 --------------------
> 4 files changed, 224 insertions(+), 224 deletions(-)
> create mode 100644 src/hooks/configs/dhcp
> delete mode 100644 src/hooks/configs/ipv4-dhcp
> delete mode 100644 src/hooks/configs/ipv6-dhcp
>=20
> diff --git a/Makefile.am b/Makefile.am
> index 32e7166..2bbc537 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -196,10 +196,9 @@ INSTALL_EXEC_HOOKS +=3D bridge-stp-install-hook
> UNINSTALL_EXEC_HOOKS +=3D bridge-stp-uninstall-hook
>=20
> dist_hooks_configs_SCRIPTS =3D \
> -    src/hooks/configs/ipv4-dhcp \
> +    src/hooks/configs/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
>=20
> diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
> new file mode 100644
> index 0000000..4aca87f
> --- /dev/null
> +++ b/src/hooks/configs/dhcp
> @@ -0,0 +1,223 @@
> +#!/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=3D"HOOK DELAY ENABLE_IPV4 ENABLE_IPV6"
> +
> +# Default settings.
> +DELAY=3D0
> +ENABLE_IPV4=3D"on"
> +ENABLE_IPV6=3D"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() {
> +    # These variables are there to check if a user tries to disable
> +    # and enable ipv6 or ipv4 at the same time. I would not chrash
> +    # but things could happen the user did not expect and
> +    # thats why we check for this mistake
> +    local ipv6_passed
> +    local ipv4_passed
> +    ipv6_passed=3Dfalse
> +    ipv4_passed=3Dfalse
> +
> +    while [ $# -gt 0 ]; do
> +        case "${1}" in
> +            --delay=3D*)
> +                DELAY=3D"$(cli_get_val "${1}")"
> +                ;;
> +            --enable-ipv6)
> +                ENABLE_IPV6=3D"on"
> +                if ${ipv6_passed}; then
> +                    # --disable-ipv6 was passed earlier
> +                    log ERROR "You cannot disable and enable ipv6 at the s=
ame time"
> +                    return ${EXIT_ERROR}
> +                fi
> +                ipv6_passed=3Dtrue
> +                ;;
> +            --disable-ipv6)
> +                ENABLE_IPV6=3D"off"
> +                if ${ipv6_passed}; then
> +                    # --enable-ipv6 was passed earlier
> +                    log ERROR "You cannot disable and enable ipv6 at the s=
ame time"
> +                    return ${EXIT_ERROR}
> +                fi
> +                ipv6_passed=3Dtrue
> +                ;;
> +            --enable-ipv4)
> +                ENABLE_IPV4=3D"on"
> +                if ${ipv4_passed}; then
> +                    # --disable-ipv4 was passed earlier
> +                    log ERROR "You cannot disable and enable ipv4 at the s=
ame time"
> +                    return ${EXIT_ERROR}
> +                fi
> +                ipv4_passed=3Dtrue
> +                ;;
> +            --disable-ipv4)
> +                ENABLE_IPV4=3D"off"
> +                if ${ipv4_passed}; then
> +                    # --enable-ipv4 was passed earlier
> +                    log ERROR "You cannot disable and enable ipv4 at the s=
ame time"
> +                    return ${EXIT_ERROR}
> +                fi
> +                ipv4_passed=3Dtrue
> +                ;;
> +            *)
> +                warning "Ignoring unknown option '${1}'"
> +                ;;
> +        esac
> +        shift
> +    done
> +}

I think you are making this too complicated. It doesn't need to be that. Most=
 tools just go with the last argument passed and we should do that, too (conf=
igure for example).

So when you get an argument passed multiple times just let is run through the=
 loop every time and that will set the variable to what ever was passed last.

The rest is all fine.

-M
> +
> +hook_new() {
> +    local zone=3D"${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
> +
> +    if ! hook_parse_cmdline $@; then
> +        # Return an error if the parsing of the cmd line fails
> +        return ${EXIT_ERROR}
> +    fi
> +
> +    # Check if the user disabled ipv4 and ipv6
> +
> +    if ! enabled ENABLE_IPV6 && ! enabled ENABLE_IPV4; then
> +        log ERROR "You disabled IPv6 and IPv4. At least one must be enable=
d"
> +        return ${EXIT_ERROR}
> +    fi
> +
> +    zone_config_settings_write "${zone}" "${HOOK}"
> +
> +    exit ${EXIT_OK}
> +}
> +
> +hook_up() {
> +    local zone=3D${1}
> +    local config=3D${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 IPv6 on this zone if enabled.
> +    if enabled ENABLE_IPV6; then
> +        dhclient_start ${zone} ipv6
> +    fi
> +
> +    # Start dhclient for IPv4 on this zone if enabled.
> +    if enabled ENABLE_IPV4; then
> +        dhclient_start ${zone} ipv4
> +    fi
> +
> +    exit ${EXIT_OK}
> +}
> +
> +hook_down() {
> +    local zone=3D${1}
> +    local config=3D${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
> +
> +    # Stop dhclient for IPv4 on this zone.
> +    dhclient_stop ${zone} ipv4
> +
> +    exit ${EXIT_OK}
> +}
> +
> +hook_status() {
> +    local zone=3D${1}
> +    local config=3D${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=3D"${MSG_HOOK_UP}"
> +    else
> +        status=3D"${MSG_HOOK_DOWN}"
> +    fi
> +    cli_statusline 3 "${HOOK}" "${status}"
> +
> +    cli_space
> +
> +    local proto
> +    for proto in "IPv6" "IPv4"; do
> +        local _proto=3D${proto,,}
> +
> +        cli_print_fmt1 3 "${proto}"
> +
> +        if enabled ENABLE_${proto^^}; then
> +            cli_print_fmt1 4 "Status" "enabled"
> +
> +            local address=3D"$(db_get "${zone}/${_proto}/local-ip-address"=
)"
> +            if isset address; then
> +                cli_print_fmt1 4 "Address" "${address}"
> +            fi
> +
> +            local gateway=3D"$(db_get "${zone}/${_proto}/remote-ip-address=
")"
> +            if isset gateway; then
> +                cli_print_fmt1 4 "Gateway" "${gateway}"
> +            fi
> +
> +            local dns_servers=3D"$(db_get "${zone}/${_proto}/domain-name-s=
ervers")"
> +            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=3D"HOOK DELAY"
> -
> -# Default settings.
> -DELAY=3D0
> -
> -hook_check_config_settings() {
> -    assert isset DELAY
> -    assert isinteger DELAY
> -}
> -
> -hook_new() {
> -    local zone=3D"${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 zo=
ne"
> -        return ${EXIT_ERROR}
> -    fi
> -
> -    while [ $# -gt 0 ]; do
> -        case "${1}" in
> -            --delay=3D*)
> -                DELAY=3D"$(cli_get_val "${1}")"
> -                ;;
> -        esac
> -        shift
> -    done
> -
> -    zone_config_settings_write "${zone}" "${HOOK}"
> -
> -    exit ${EXIT_OK}
> -}
> -
> -hook_up() {
> -    local zone=3D${1}
> -    local config=3D${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=3D${1}
> -    local config=3D${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=3D${1}
> -    local config=3D${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=3D"${MSG_HOOK_UP}"
> -    else
> -        status=3D"${MSG_HOOK_DOWN}"
> -    fi
> -    cli_statusline 3 "${HOOK}" "${status}"
> -
> -    cli_print_fmt1 3 "IPv4 address" "$(db_get "${zone}/ipv4/local-ip-addre=
ss")"
> -    local gateway=3D"$(db_get "${zone}/ipv4/remote-ip-address")"
> -    if isset gateway; then
> -        cli_print_fmt1 3 "Gateway" "${gateway}"
> -        cli_space
> -    fi
> -    local dns_servers=3D"$(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=3D"HOOK"
> -
> -hook_new() {
> -    local zone=3D"${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 zo=
ne"
> -        return ${EXIT_ERROR}
> -    fi
> -
> -    zone_config_settings_write "${zone}" "${HOOK}"
> -
> -    exit ${EXIT_OK}
> -}
> -
> -hook_up() {
> -    local zone=3D"${1}"
> -    local config=3D"${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=3D"${1}"
> -    local config=3D"${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=3D"${1}"
> -    local config=3D"${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=3D"${MSG_HOOK_UP}"
> -    else
> -        status=3D"${MSG_HOOK_DOWN}"
> -    fi
> -    cli_statusline 3 "${HOOK}" "${status}"
> -
> -    cli_print_fmt1 3 "IPv6 address" "$(db_get "${zone}/ipv6/local-ip-addre=
ss")"
> -    local gateway=3D"$(db_get "${zone}/ipv6/remote-ip-address")"
> -    if isset gateway; then
> -        cli_print_fmt1 3 "Gateway" "${gateway}"
> -    fi
> -    cli_space
> -
> -    exit ${EXIT_OK}
> -}
> --=20
> 2.6.3
>=20


--===============4551909663976661356==--