Hello Jonatan,
On 8 Mar 2024, at 11:14, Jonatan Schlag jonatan.schlag@ipfire.org wrote:
Hi,
Tl;dr:
It is not sensible to duplicate code in bash and the setup command. There should be one source for checking a network config. The minimal should be done in Bash. If C is the right language to go is another question.
I believe that you will have to duplicate a couple of things quite a bit. We use three different languages to implement the network configuration scripts, the management utility and the web UI. That means: Bash, C, and Perl.
If we were going really bug there could be some benefit in writing a single C library with the logic and exposing that to Perl and Bash, but that would be a lot more work that we are willing to invest into this project right now.
After thought about this whole situation a little bit more, I came to the conclusion that we need a better solution here. So what are the problems with this patch and the current situation:
Better solution to what? I am not entirely sure where you are going with all of this.
- We need to check for a correct network configuration before we start
it and when a user edits it. The editing is done via /usr/sbin/setup which is a C program from 2001. The startup is done via a shell script. It is a bad idea, as we learned from ipfire-3.x, to have a duplicated code in two languages. So it is a bad idea two write shell functions to check for a valid network config and C functions. There needs to be one way to check for a valid network config.
setup is probably slightly older than 2001 :)
As mentioned above, I don’t think this will hurt us too bad here because the complexity of the network stack in IPFire 2 is substantially smaller than IPFire 3. You will also only copy a very small subnet of the functionality which mainly is reading and writing the configuration. That is all. Depending on how you are splitting it, this could be less than a hand full of functions. That sounds very acceptable to me.
*Nearly everything can be programmed in Bash, but maybe not everything should. It may be the better approach to do only the network startup with as minimal code as possible in Bash (calling ip from something else with something like system() is a bad idea.) Checking and parsing a config file is perhaps better to be done in other languages. After we parsed a config file and checked the content for validity, we could, for example, export the necessary information as environment variables. Basically, we do the same thing here, as eval "parses" the config and export variables: https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/initscripts/networking...
Apart from copying variables back and forth this should not be the worst bit of code we have here.
Bash is a great language for these things. Can it be done better? Yes, but you are not seriously considering to do a full rewrite of everything in C, are you?
- Bash has somehow limited tooling, for example for testing. There are
other languages like python and pytest, so we have to write less code. It is pointless to write a testing framework for ourselves in Bash.
To test a bash function, you need a bash script. There is nothing else required.
*This patch duplicates code which is somehow also found here: https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/setup/networking.c;h=9...
This is not a duplicate. This is the only place where the network card assignment happens.
So what do I propose. This is a rough sketch, not a detailed plan: There should be a program which I can call from bash, which:
*script
- checks for the validity of the network config
- exports all variables which are valid.
The same config check should be used in the program which users use to edit the network settings and in the web interface to change the zone settings. The language for this program should not be Bash. What language should be used largely depends on preferences. I can program in C, but I try to avoid it when there is no reason, like performance. A program in an interpreter language with enough tests, which are easy to write when you only need to mock a config file, is equally stable. But before this discussion starts, I would like to gather some opinions on the general thoughts a wrote down, here.
You want to write shell scripts here. Nothing else, because you will run into new headaches:
* You have a lot of shell scripts that execute the boot process, run daemons and what not. You don’t want to touch those, so you will have to be compatible.
* You will have to rewrite all sorts of callback scripts for DHCP and PPP. You don’t want to do that.
* Shell can be written quickly.
* You already have some functionality there which is working perfectly fine. Why would you want to rewrite something that already works so well?
* I believe that the IPFire 3 code shows that you can write large projects in shell. They work well and shell would cover everything we need in IPFire 2.
In order to get to some sort of milestone, I believe that we need to work with what we have and incrementally improve it. If you want to work on a total rewrite, please look at IPFire 3 and not IPFire 2.
-Michael
Greetings Jonatan
Am Samstag, dem 02.03.2024 um 12:09 +0100 schrieb Jonatan Schlag:
As our Network is quite static a case does the trick
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/initscripts/networking/functions.network | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/src/initscripts/networking/functions.network b/src/initscripts/networking/functions.network index dedbb6f7f..ad0d3f36a 100644 --- a/src/initscripts/networking/functions.network +++ b/src/initscripts/networking/functions.network @@ -289,3 +289,25 @@ qmi_assign_address() { # Change the MAC address ip link set "${intf}" address "${address}" }
+network_zone_exists(){
local zone="${1}"case "${zone}" in"blue")[ "${CONFIG_TYPE}" = "3" ] || ["${CONFIG_TYPE}" = "4" ]
;;"green")[ -n "${GREEN_DEV}" ] && [ -v "GREEN_DEV" ];;"orange")[ "${CONFIG_TYPE}" = "2" ] || ["${CONFIG_TYPE}" = "4" ]
;;"red")return ${EXIT_TRUE};;*)return ${EXIT_FALSE};;esac+}