Re: [PATCH] Core 152: the script "network-hotplug-bridges" now reads the variable ${ZONE}_STP from /var/ipfire/ethernet/settings so that STP can be turned on and off for each bridge

[hofmann@leo-andres.de]

[-- Attachment #1: Type: text/plain, Size: 11544 bytes --]

Hi Michael,

thanks for the explanation! Now we can really close the topic.

Have a nice weekend
Leo

11. Dezember 2020 09:51, "Michael Tremer" <michael.tremer(a)ipfire.org> schrieb:

> Hi,
> 
>> On 10 Dec 2020, at 18:59, Leo Hofmann <hofmann(a)leo-andres.de> wrote:
>> 
>> Hi,
>> 
>> agreed, I'll do option 1 then and separate the STP options a bit clearer. We can close this long
>> topic now :)
>> 
>> @Michael I'm not really sure how to add new lines (e.g. "STP enable") to the language files. But
>> I'll look into it and get back to you if I can't figure it out.
> 
> You can simply edit langs/en/cgi-bin/en.pl and add new lines. English is always required because
> the web UI falls back to English in case there is no string in the user’s language. As you are a
> native German speaker, you can of course add those strings to the German version too. On you test
> system, copy the *.pl files to /var/ipfire/lang and run “update-language-cache” and your CGI script
> will be able to use the new strings.
> 
> Before committing, please run “./make.sh lang” in the build environment. This will sort the
> strings, add a note about missing translations to other languages and so on.
> 
> That is all.
> 
> Best,
> -Michael
> 
>> Regards
>> Leo
>> 
>> Am 10.12.2020 um 14:34 schrieb Michael Tremer:
>>> Hello,
>>> 
>>> I would say we can close this then in favour of option one.
>>> 
>>> Leo, do you need to know anything else or do you have everything to start hacking?
>>> 
>>> Best,
>>> -Michael
>> 
>> On 10 Dec 2020, at 12:13, Daniel Weismüller <daniel.weismueller(a)ipfire.org> wrote:
>> 
>> H Leo,
>> 
>> I vote for "One big table" but with a few change requests.
>> 
>> - please change the order of the nics to red, green, orange, blue
>> 
>> - please change the way you implement the STP-line. I would discard the STP-field and change
>> "enable" to "enable STP" and maybe the border between the options and the assignment a bit bigger.
>> At the moment you get the impression that you can switch the zone on and off and not only STP.
>> 
>> -
>> 
>> Daniel
>> 
>> Am 10.12.20 um 09:24 schrieb hofmann(a)leo-andres.de:
>> Hi Michael,
>> 
>> I can start working on this next week, so I suggest we keep voting until 13.12.! Votes so far:
>> - One big table: II
>> - Two tables: II
>> 
>> Regards
>> Leo
>> 
>> Am 01.12.2020 um 17:27 schrieb Michael Tremer:
>> Hello Leo, Thanks for putting all this leg work in. I would as well vote for option two: one big
>> table. I do not expect that we will add too much more, and splitting this into two tables pulls two
>> things that belong together apart. Until when do we want to keep the voting open? Best, -Michael
>> On 28 Nov 2020, at 13:24, Adolf Belka <ahb.ipfire(a)gmail.com> wrote: Hi Leo, I prefer the one big
>> table. There are just a couple of extra rows and boxes in each of the zone headings, so I don't
>> think it is overly busy and easier to scan if you are making changes. At the end of the day, if the
>> two table option was preferred by more people I could also live with that. My 2p worth. Regards,
>> Adolf. On 28/11/2020 13:06, Leo Hofmann wrote:
>> Hi, once again, thanks for your feedback! I spent some time and created two more detailed UI
>> drafts. I hope that I have incorporated all your ideas: 1: Two tables, zone options on the top, NIC
>> assign matrix (without any unrelated options) on the bottom 2: One big table, STP options inside
>> NIC selection Your thoughts? Regards Leo Am 27.11.2020 um 11:59 schrieb Michael Tremer:
>> Hello,
>> On 27 Nov 2020, at 08:34, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote: Hi, Sorry for jumping
>> late into this conversation, time is rare in these days .... I will try to bring in my thoughts,
>> but maybe they are not well structured.
>> Am 26.11.2020 um 15:47 schrieb Daniel Weismüller <daniel.weismueller(a)ipfire.org>: Hi, there is one
>> thing that we didn't talked about... STP and priority must only be activatable if the zone is in
>> bridge mode otherwise it must be grayed out.
>> Shouldn’t we just not display these fields, if they do not matter?
>> - Daniel
>> Am 25.11.20 um 21:57 schrieb Michael Tremer: Hello,
>> On 25 Nov 2020, at 17:00, Leo Hofmann <hofmann(a)leo-andres.de> wrote:
>> Hi Daniel, thank you very much for the draft & the explanation! Do you happen to know if there are
>> any other zone-related options that might be added in the future? If this is a possibility, I think
>> we should add a second table. So we don't clutter the NIC assignment with unrelated options.
>> Did I mention that it is very nice, when you wrote what I think so I don’t have to write it again
>> 😉. +1 For a second structure
>> Good question. On one hand it is good to have things that go together in one place. On the other
>> hand, this whole page is becoming longer and longer and that simply makes it complicated.
>> Definitely to complicated. Already right now.
>> The only thing I can think of is MTU. We currently have no UI to set that, but it has never been
>> asked for. We set it automatically on some of the cloud providers, but that is it.
>> I took up your and Michael's suggestions and created a quick HTML demo.
>> Looks good :)
>> Here I don’t think so, because more zones (4) will make it impossible to display this on very small
>> displays. Why not creating a table for every zone and putting them among each other.
>> I think this definitely has some upsides, but it also has some downsides: It would be good to have
>> more space for each zone and put all settings for one zone together. The biggest problem that I see
>> is that it is no longer obvious which ports are now available and configured to other zones and
>> that makes this part a little bit more complicated. People would have to scroll up and down or hit
>> Save and see an error message that tells you that you did something wrong. What do we think about
>> this? It is a bit of extra work - and should be considered a step two after STP has been
>> implemented - but I do not think that someone will spend a week on implementing this.
>> This new table could be placed below the NIC assignment table. What do you think?
>> Call the checkboxes “Enable”, because that is what they do. I would also suggest to have the labels
>> (e.g. “Priority”) on the left so that it is only wasting space once. With plenty of zones the table
>> just becomes unnecessarily wide then.
>> @Michael: I would like to base this new feature on my recently patched zoneconf.cgi. Is this
>> somehow a bad idea?
>> Well, good question. I have no idea why I didn’t merge it yet. I didn’t realise it was ready. I
>> will check if there is enough testing feedback already. Best, -Michael
>> Regards Leo Am 23.11.2020 um 16:13 schrieb Daniel Weismüller:
>> Hi Leo, that pleases me to hear and I gladly accept your offer. ;-) I quickly made a draft and
>> attached it. As I said it is only a draft so there is still plenty of room for improvement. The
>> checkbox switches the variable named ${ZONE}_STP to 0 or 1. The input field fills the variable
>> named ${ZONE}_STP_PRIORITY. Here must a number between 1 and 65535 inserted. - Daniel Am 21.11.20
>> um 17:39 schrieb Leo Hofmann:
>> Hi Daniel, a few days ago I finally submitted my patches for zoneconf.cgi and I would now have time
>> to work on this as well.
>> Thank you for submitting these patches. It is enjoyable to read good code.
>> (By the way, I almost forgot, thanks @Michael for reviewing my patches!) If you want me to take
>> this on, it would be very helpful if you could summarize how this should work. For example, which
>> config parameters need to be modified. Perhaps you could even paint a simple GUI mock-up like you
>> did last time? Regards, Leo Am 20.11.2020 um 19:31 schrieb Daniel Weismüller:
>> OK. ;-) The first step will be the introduction of the possibility to enable STP. The next step
>> will be the implementation in the webif. I hope I find someone who can do that. - Daniel Am
>> 20.11.2020 um 16:18 schrieb Kienker, Fred:
>> I'm with Michael on this one. If it deserves to be in IPFire, it deserves to be on the web
>> interface. Don't created exceptions which are only available from a command line. Best regards,
>> Fred -----Original Message----- From: Michael Tremer <michael.tremer(a)ipfire.org> Sent: Friday,
>> November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller(a)ipfire.org> Cc:
>> development(a)lists.ipfire.org Subject: Re: [PATCH] Core 152: the script "network-hotplug-bridges"
>> now reads the variable ${ZONE}_STP from /var/ipfire/ethernet/settings so that STP can be turned on
>> and off for each bridge Hi,
>> On 20 Nov 2020, at 06:58, Daniel Weismüller
>> <daniel.weismueller(a)ipfire.org> wrote:
>> Hello, In my opinion it is sufficient to be able to set these parameters via
>> command line. Why is that? IPFire is a distribution that is supposed to be managed entirely by the
>> web user interface.
>> And here I was wondering a lot. A lot of options are only available via command line. The setup
>> command is entirely based on the command line. So where do we draw a border what should be
>> available from the webinterface? These and some other questions I have belonging to the
>> webinterface, are some how fundamental, so that I like to discuss these in the telco ...
>> The setup is a CLI tool because the web UI is not set up yet, when it is being launched. We must
>> have everything that is possible on the web UI and only what is necessary on the CLI.
>> Best Jonatan
>> It should only be made sure that the settings are persitend and not
>> overwritten by a reboot or the webif. They wont be as they are in /var/ipfire/ethernet/settings.
>> Best, -Michael
>> - Daniel Am 19.11.2020 um 15:56 schrieb Michael Tremer:
>> Hello Daniel, This patch looks good to me. I had assumed that we automatically enabled STP on all
>> bridges, but
>> apparently we do not.
>> How do we process with this? I suppose it is not the most user-friendly way to ask the user to
>> edit the configuration file. This either must be documented somewhere or the zoneconfig.cgi script
>> needs to be extended to allow enabling STP.
>> Does anyone want to be able to change any STP parameters like
>> priority or cost of the ports?
>> Best, -Michael
>> On 19 Nov 2020, at 13:18, Daniel Weismüller
>> <daniel.weismueller(a)ipfire.org> wrote:
>> Signed-off-by: Daniel Weismüller <daniel.weismueller(a)ipfire.org> ---
>> config/udev/network-hotplug-bridges | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff
>> --git a/config/udev/network-hotplug-bridges
>> b/config/udev/network-hotplug-bridges
>> index 33d6d65ba..7431377bb 100644 --- a/config/udev/network-hotplug-bridges +++
>> b/config/udev/network-hotplug-bridges @@ -81,6 +81,7 @@ MODE="$(get_value "${ZONE}_MODE")" # The
>> name of the virtual bridge BRIDGE="$(get_value "${ZONE}_DEV")" +STP="$(get_value "${ZONE}_STP")"
>> case "${MODE}" in bridge) @@ -89,7 +90,8 @@ case "${MODE}" in # We need to create the bridge if it
>> doesn't exist, yet if [ ! -d "/sys/class/net/${BRIDGE}" ]; then - ip link add "${BRIDGE}" address
>> "${ADDRESS}" type
>> bridge
>> + ip link add "${BRIDGE}" address "${ADDRESS}" type
>> bridge \
>> + $([ "${STP}" = "on" ] && echo "stp_state 1") #ip link set "${BRIDGE}" up fi -- 2.28.0
>> <zoneconf-stp.png>