From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonatan Schlag To: development@lists.ipfire.org Subject: Re: [Patch RFC 04/15] network initscripts: check if the zone in the current config exists Date: Fri, 18 Aug 2023 14:55:08 +0200 Message-ID: In-Reply-To: <72FA5C9E-CD86-4090-A23A-FC35CC28C3A0@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7970007175999806940==" List-Id: --===============7970007175999806940== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi list, Am Mittwoch, dem 24.05.2023 um 10:00 +0100 schrieb Michael Tremer: > Hello, >=20 > If you want to make this script better, why not give the zone name as > a parameter? >=20 > 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: >=20 > =C2=A0 ZONE=3D"$(basename $0)=E2=80=9D >=20 I did not want to make such a big change, as people might rely on these files. But I am happy to create a script and delete the other scripts/links. I suggest as naming: /etc/init.d/networking/local Calling the script could look like: For one Zone /etc/init.d/networking/local start ${ZONE} /etc/init.d/networking/local stop ${ZONE} For all Zones /etc/init.d/networking/local start /etc/init.d/networking/local stop Look this fine to you? > Otherwise I agree with introducing unified functions that check > whether we have 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. Ok, fine by me. >=20 > I would prefer to give those function a =E2=80=9Cnetwork_=E2=80=9D prefix s= o 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_BLUE() = and > network_have_ORANGE(). >=20 > Can we do that instead? Yes, we can do that. I have only comments on the naming of the functions. I would like to reuse the naming style of the old ipfire-3.x networking code. So I would propose: network_zone_exists(ZONE) network_zones_get_all() network_zones_get_local() network_zones_get_nonlocal() As we all agreed on this naming some time ago, I think it faster to use these names instead of inventing new ones. Greetings Jonatan >=20 > -Michael >=20 > > On 23 May 2023, at 18:23, Jonatan Schlag > > wrote: > >=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 least a > > litte bit prettier. > >=20 > > Signed-off-by: Jonatan Schlag > > --- > > src/initscripts/networking/any=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 27 > > +++++++++++++++++--- > > src/initscripts/networking/functions.network | 12 +++++++++ > > src/initscripts/system/network=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 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 zone." ${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 zone." ${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 >=20 --===============7970007175999806940==--