[prev in list] [next in list] [prev in thread] [next in thread]
List: ipfire-development
Subject: Re: [PATCH 2/5] functions.network: Add network_zone_exists
From: Jonatan Schlag <jonatan.schlag () ipfire ! org>
Date: 2024-03-28 18:50:06
Message-ID: 095E6B21-B755-4EA4-9D0B-AD5D6C1E516B () ipfire ! org
[Download RAW message or body]
[Attachment #2 (multipart/related)]
[Attachment #4 (text/html)]
<html><head><meta http-equiv="content-type" content="text/html; \
charset=utf-8"></head><body dir="auto"><div dir="ltr"></div><div \
dir="ltr">Hi,</div><div dir="ltr"><br><blockquote type="cite">Am 25.03.2024 um 11:55 \
schrieb Michael Tremer \
<michael.tremer@ipfire.org>:<br><br></blockquote></div><blockquote \
type="cite"><div dir="ltr"><span>Hello,</span><br><span></span><br><blockquote \
type="cite"><span>On 25 Mar 2024, at 07:46, Jonatan Schlag \
<jonatan.schlag@ipfire.org> wrote:</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Am 2024-03-18 \
17:27, schrieb Michael Tremer:</span><br></blockquote><blockquote \
type="cite"><blockquote \
type="cite"><span>Hello,</span><br></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>On 16 Mar 2024, at \
09:35, Jonatan Schlag <jonatan.schlag@ipfire.org> \
wrote:</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>Hi \
Michael,</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>Am 2024-03-12 \
11:32, schrieb Michael \
Tremer:</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>Hello \
Jonatan,</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>On 8 Mar 2024, at 11:14, Jonatan Schlag \
<jonatan.schlag@ipfire.org> \
wrote:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span>Hi,</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span>Tl;dr:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>It is not sensible to duplicate code in \
bash and the setup command.</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>There should be one source for checking a \
network config. The minimal</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>should be done in Bash. If C is the right \
language to go is another</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span>question.</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>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.</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>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.</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>Would you still be \
OK, if I write some port for examples as loadable for bash? Or do you want everything \
in Bash?</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span>Which parts would that be? I cannot think \
of anything that would be better implemented in something \
else.</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span>Let me know what you have in mind and we will \
discuss.</span><br></blockquote></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Reusing the \
function here https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/libsmooth/varval.c;hb=08b7500b267a54aa634fb34b67b4dfc0934ae2be#l46 \
and creating a bash loadable so that I can do</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>readkeyvalues \
FILE ARRAY_NAME</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>and \
subsequently, access the data like</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>echo \
${ARRY_NAME["GREEN_DEV"]}</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>So we stay \
bug compatible and reusing code which we use already in several other \
places.</span><br></blockquote><span></span><br><span>This is what we use in C \
because we cannot create new variables just like that. C is not a scripting \
language.</span><br><span></span><br><span>In shell scripts we use \
"readhash".</span><br><span></span><br><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>After thought about this whole situation a little bit more, I came \
to</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>the conclusion that we need a better \
solution here. So what are \
the</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>problems with this patch and the current \
situation:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>Better solution to what? I am not entirely sure where you are going \
with all of this.</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>For reading and \
checking our network \
configuration.</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span>Reading it is not a problem as we have a \
script for that.</span><br></blockquote></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>We currently \
do an eval on the file. I would not call this a script. Or am I missing \
something.</span><br></blockquote><span></span><br><span>Call it what you want. There \
is something that imports a KEY=VALUE configuration file into a shell \
script.</span><br><span></span><br><blockquote type="cite"><blockquote \
type="cite"><span>Checking? I think you need to be a little bit more \
clear.</span><br></blockquote></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Check for \
correct IP addresses and so on. I would like to avoid that the setup utility lets you \
save an IP address with the bash script fails on. So, the best way to do this: using \
the same function to check the network config.</span><br></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>* We need to check for a correct network configuration before we \
start</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>it and when a user edits it. The editing is \
done via /usr/sbin/setup</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>which is a C program from 2001. The \
startup is done via a \
shell</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>script. It is a bad idea, as we learned \
from ipfire-3.x, to have \
a</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>duplicated code in two languages. So it is \
a bad idea two write \
shell</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>functions to check for a valid network \
config and C functions. \
There</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>needs to be one way to check for a valid \
network config.</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>setup is probably slightly older than 2001 \
:)</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>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.</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>*Nearly everything can be programmed in \
Bash, but maybe not everything</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>should. It may be the better approach to do \
only the network startup</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>with as minimal code as possible in Bash \
(calling ip from something</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>else with something like system() is a bad \
idea.) Checking and parsing</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>a config file is perhaps better to be done \
in other languages. After \
we</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>parsed a config file and checked the \
content for validity, we \
could,</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>for example, export the necessary \
information as environment \
variables.</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>Basically, we do the same thing here, as \
eval "parses" the config \
and</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote type="cite"><span>export \
variables:</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><blockquote \
type="cite"><span>https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/initscripts/ne \
tworking/any;h=dc4796e914536dae03b70390e9447b3bb2bf127b;hb=refs/heads/next#l24</span><br></blockquote></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>Apart from copying variables back and forth this should not be the \
worst bit of code we have \
here.</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>No, it is not. My \
point was, more, that I need to redefine which CONFIG_TYPE is what. I would like to \
avoid that.</span><br></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><span>I think there is no way to avoid \
that.</span><br></blockquote></blockquote><blockquote type="cite"><blockquote \
type="cite"><span>I believe that there would be a huge benefit from checking the \
entire code base for this. We would be able to open a couple of extra doors for the \
future this way.</span><br></blockquote></blockquote><blockquote \
type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Checking the \
entire code base for what?</span><br></blockquote><span></span><br><span>For how we \
interpret CONFIG_TYPE or if there are other ways used to determine whether there is a \
BLUE or ORANGE network.</span><br><span></span><br><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote \
type="cite"><span>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?</span><br></blockquote></blockquote></blockquote></blockquote><blockquote \
type="cite"><blockquote type="cite"><blockquote type="cite"><span>Not a full rewrite. \
But If I can program a function in C or in Bash and maybe reuse some Code of the \
installer in C, I would like to use \
C.</span><br></blockquote></blockquote></blockquote><blockquote \
["favicon.ico;" (image/vnd.microsoft.icon)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic