From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [Patch RFC 04/15] network initscripts: check if the zone in the current config exists Date: Wed, 24 May 2023 10:00:02 +0100 Message-ID: <72FA5C9E-CD86-4090-A23A-FC35CC28C3A0@ipfire.org> In-Reply-To: <20230523172314.7826-5-jonatan.schlag@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3286303575912933591==" List-Id: --===============3286303575912933591== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, If you want to make this script better, why not give the zone name as a param= eter? First of all it should be renamed to something like =E2=80=9Clocal=E2=80=9D (= as in: not RED) and then you don=E2=80=99t have to use any symlinks and read = back the script name like this: ZONE=3D"$(basename $0)=E2=80=9D Otherwise I agree with introducing unified functions that check whether we ha= ve GREEN/BLUE/ORANGE and have them in one function. We should then go through= the entire code base and replace those calls first before we are introducing= any new changes. I would prefer to give those function a =E2=80=9Cnetwork_=E2=80=9D prefix so = that it becomes clear where this function is from and ideally shorten =E2=80= =9Cis used=E2=80=9D to just =E2=80=9Chave=E2=80=9D. Like so: network_have_BLU= E() and network_have_ORANGE(). Can we do that instead? -Michael > On 23 May 2023, at 18:23, Jonatan Schlag wrot= e: >=20 > We check in /etc/init.d/network if the current Configuration (RED+GREEN > or RED+GREEN+BLUE) contains the zone we want to start or stop. > We do this not in /etc/init.d/networking/green,blue,orange >=20 > As this checks make sense also there and as these scripts are called > form /etc/init.d/network I moved the check to these scripts. >=20 > As CONFIG_TYPE =3D=3D 2 is unreadable I wrote functions to make things at l= east a > litte bit prettier. >=20 > Signed-off-by: Jonatan Schlag > --- > src/initscripts/networking/any | 27 +++++++++++++++++--- > src/initscripts/networking/functions.network | 12 +++++++++ > src/initscripts/system/network | 16 ++++-------- > 3 files changed, 41 insertions(+), 14 deletions(-) >=20 > diff --git a/src/initscripts/networking/any b/src/initscripts/networking/any > index dc4796e91..6dba5bef9 100644 > --- a/src/initscripts/networking/any > +++ b/src/initscripts/networking/any > @@ -21,23 +21,44 @@ >=20 > . /etc/sysconfig/rc > . ${rc_functions} > +. /etc/init.d/networking/functions.network > + > eval $(/usr/local/bin/readhash /var/ipfire/ethernet/settings) >=20 > -if [ "$(basename $0)" =3D=3D "green" ]; then > +ZONE=3D"$(basename $0)" > + > + > +if [ "${ZONE}" =3D=3D "green" ]; then > + if ! is_green_used; then > + boot_mesg "Green zone is not configured. No action can be taken on this z= one." ${FAILURE} > + echo_failure > + exit 1 > + fi > + > DEVICE=3D"${GREEN_DEV}" > ADDRESS=3D"${GREEN_ADDRESS}" > NETADDRESS=3D"${GREEN_NETADDRESS}" > NETMASK=3D"${GREEN_NETMASK}" > DEVICE=3D"${GREEN_DEV}" > MTU=3D"${GREEN_MTU}" > -elif [ "$(basename $0)" =3D=3D "blue" ]; then > +elif [ "${ZONE}" =3D=3D "blue" ]; then > + if ! is_blue_used; then > + boot_mesg "Blue zone is not configured. No action can be taken on this zo= ne." ${FAILURE} > + echo_failure > + exit 1 > + fi > DEVICE=3D"${BLUE_DEV}" > ADDRESS=3D"${BLUE_ADDRESS}" > NETADDRESS=3D"${BLUE_NETADDRESS}" > NETMASK=3D"${BLUE_NETMASK}" > DEVICE=3D"${BLUE_DEV}" > MTU=3D"${BLUE_MTU}" > -elif [ "$(basename $0)" =3D=3D "orange" ]; then > +elif [ "${ZONE}" =3D=3D "orange" ]; then > + if ! is_orange_used; then > + boot_mesg "Orange zone is not configured. No action can be taken on this = zone." ${FAILURE} > + echo_failure > + exit 1 > + fi > DEVICE=3D"${ORANGE_DEV}" > ADDRESS=3D"${ORANGE_ADDRESS}" > NETADDRESS=3D"${ORANGE_NETADDRESS}" > diff --git a/src/initscripts/networking/functions.network b/src/initscripts= /networking/functions.network > index 4c7ad51d4..9cc4da24b 100644 > --- a/src/initscripts/networking/functions.network > +++ b/src/initscripts/networking/functions.network > @@ -285,3 +285,15 @@ qmi_assign_address() { > # Change the MAC address > ip link set "${intf}" address "${address}" > } > + > +is_blue_used() { > + [ "${CONFIG_TYPE}" =3D "3" ] || [ "${CONFIG_TYPE}" =3D "4" ] > +} > + > +is_green_used() { > + [ -n "${GREEN_DEV}" ] && [ -v "GREEN_DEV" ] > +} > + > +is_orange_used() { > + [ "${CONFIG_TYPE}" =3D "2" ] || [ "${CONFIG_TYPE}" =3D "4" ] > +} > diff --git a/src/initscripts/system/network b/src/initscripts/system/network > index 0d63b4e8b..fda16919d 100644 > --- a/src/initscripts/system/network > +++ b/src/initscripts/system/network > @@ -54,12 +54,10 @@ case "${DO}" in > [ "$green" =3D=3D "1" ] && /etc/rc.d/init.d/networking/green start >=20 > # BLUE > - [ "$blue" =3D=3D "1" ] && [ "$CONFIG_TYPE" =3D "3" -o "$CONFIG_TYPE" =3D = "4" ] && \ > - /etc/rc.d/init.d/networking/blue start > + [ "$blue" =3D=3D "1" ] && /etc/rc.d/init.d/networking/blue start >=20 > # ORANGE > - [ "$orange" =3D=3D "1" ] && [ "$CONFIG_TYPE" =3D "2" -o "$CONFIG_TYPE" = =3D "4" ] && \ > - /etc/rc.d/init.d/networking/orange start > + [ "$orange" =3D=3D "1" ] && /etc/rc.d/init.d/networking/orange start >=20 > # RED > if [ "$red" =3D=3D "1" ]; then > @@ -87,18 +85,14 @@ case "${DO}" in > [ "$green" =3D=3D "1" ] && /etc/rc.d/init.d/networking/green stop >=20 > # BLUE > - [ "$blue" =3D=3D "1" ] && [ "$CONFIG_TYPE" =3D "3" -o "$CONFIG_TYPE" =3D = "4" ] && \ > - /etc/rc.d/init.d/networking/blue stop > + [ "$blue" =3D=3D "1" ] && /etc/rc.d/init.d/networking/blue stop >=20 > # ORANGE > - [ "$orange" =3D=3D "1" ] && [ "$CONFIG_TYPE" =3D "2" -o "$CONFIG_TYPE" = =3D "4" ] && \ > - /etc/rc.d/init.d/networking/orange stop > + [ "$orange" =3D=3D "1" ] && /etc/rc.d/init.d/networking/orange stop >=20 > # RED > if [ "$red" =3D=3D "1" ]; then > - if [ "$CONFIG_TYPE" =3D "1" -o "$CONFIG_TYPE" =3D "2" -o "$CONFIG_TYPE" = =3D "3" -o "$CONFIG_TYPE" =3D "4" ]; then > - /etc/rc.d/init.d/networking/red stop > - fi > + /etc/rc.d/init.d/networking/red stop > fi >=20 > exit 0 > --=20 > 2.30.2 >=20 --===============3286303575912933591==--