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
| Message ID | 20201119131848.596-1-daniel.weismueller@ipfire.org |
|---|---|
| State | Accepted |
| Commit | f8bf19c92e8b99b097754847f85cc17509135731 |
| Headers |
Return-Path: <development-bounces@lists.ipfire.org>
Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature ECDSA (P-384)
client-signature ECDSA (P-384))
(Client CN "mail01.haj.ipfire.org",
Issuer "Let's Encrypt Authority X3" (verified OK))
by web04.haj.ipfire.org (Postfix) with ESMTPS id 4CcL0k1GC2z3wg9
for <patchwork@web04.haj.ipfire.org>; Thu, 19 Nov 2020 13:21:10 +0000 (UTC)
Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [172.28.1.201])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature ECDSA (P-384)
client-signature ECDSA (P-384))
(Client CN "mail02.haj.ipfire.org",
Issuer "Let's Encrypt Authority X3" (verified OK))
by mail01.ipfire.org (Postfix) with ESMTPS id 4CcL0j0DdqzWX;
Thu, 19 Nov 2020 13:21:09 +0000 (UTC)
Received: from mail02.haj.ipfire.org (localhost [127.0.0.1])
by mail02.haj.ipfire.org (Postfix) with ESMTP id 4CcL0h3gdBz2yLG;
Thu, 19 Nov 2020 13:21:08 +0000 (UTC)
Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature ECDSA (P-384)
client-signature ECDSA (P-384)) (Client CN "mail01.haj.ipfire.org",
Issuer "Let's Encrypt Authority X3" (verified OK))
by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4CcL0g6PgLz2xjj
for <development@lists.ipfire.org>; Thu, 19 Nov 2020 13:21:07 +0000 (UTC)
Received: from [127.0.0.1] (localhost [127.0.0.1])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384)
(No client certificate requested)
by mail01.ipfire.org (Postfix) with ESMTPSA id 4CcL0g1BYXzWX;
Thu, 19 Nov 2020 13:21:07 +0000 (UTC)
DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org;
s=202003ed25519; t=1605792067;
h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
to:to:cc:cc:mime-version:mime-version:content-type:content-type:
content-transfer-encoding:content-transfer-encoding;
bh=SHl3v/Jg5i0KieS3wWxrS8PrKY6caOypPSCBEaYp7gQ=;
b=lNpBc3y/IdQ5gOqj4xl3QSlQDEr8TlMyXRUOqL4O4nOq9LPMjSItJBAYe6Bfo1hMQJfG2m
hiSUEBQOoPNQIFCw==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org;
s=202003rsa;
t=1605792067;
h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
to:to:cc:cc:mime-version:mime-version:content-type:content-type:
content-transfer-encoding:content-transfer-encoding;
bh=SHl3v/Jg5i0KieS3wWxrS8PrKY6caOypPSCBEaYp7gQ=;
b=Ck/VvaF8/ccvehOC7P/Dkg2kaTye2JQiBU6jHcu1lLVMohmhCCRBwZCVltFOEOytNBj7pz
qBOcKHDhuNVqJNhrlmsUr8h79pVN0SD5CcoIYSxZc/9ONEXaA16y6j2t05N+cvwSZ+n2hE
LXIQu2tCsE42bftmi9PBlkTh6K91hS+SMWey4lTIz6lSarNolkOYbhwq89hVcgb6k0HAHr
j/ONZ4PJhm/WO8XwRi8VtzjqxMxCCzl4VK04up7vMUuN8hiElhXOoBldlGWQI2l9wwUXcX
hlnM+X6xxEup0L5MMLRMijswNB04BLgJ5R1UpuJT3bBRY43vZDXiQLeLX3JyCg==
From: =?utf-8?q?Daniel_Weism=C3=BCller?= <daniel.weismueller@ipfire.org>
To: development@lists.ipfire.org
Subject: [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
Date: Thu, 19 Nov 2020 14:18:49 +0100
Message-Id: <20201119131848.596-1-daniel.weismueller@ipfire.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: development@lists.ipfire.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: IPFire development talk <development.lists.ipfire.org>
List-Unsubscribe: <https://lists.ipfire.org/mailman/options/development>,
<mailto:development-request@lists.ipfire.org?subject=unsubscribe>
List-Archive: <http://lists.ipfire.org/pipermail/development/>
List-Post: <mailto:development@lists.ipfire.org>
List-Help: <mailto:development-request@lists.ipfire.org?subject=help>
List-Subscribe: <https://lists.ipfire.org/mailman/listinfo/development>,
<mailto:development-request@lists.ipfire.org?subject=subscribe>
Errors-To: development-bounces@lists.ipfire.org
Sender: "Development" <development-bounces@lists.ipfire.org>
|
| Series |
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
|
|
Commit Message
Daniel Weismueller
Nov. 19, 2020, 1:18 p.m. UTC
Signed-off-by: Daniel Weismüller <daniel.weismueller@ipfire.org>
---
config/udev/network-hotplug-bridges | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Comments
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@ipfire.org> wrote: > > Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >
Hello, In my opinion it is sufficient to be able to set these parameters via command line. It should only be made sure that the settings are persitend and not overwritten by a reboot or the webif. - 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@ipfire.org> wrote: >> >> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>
Of course I mean the additional settings. ;-) The possibility to activate STP via webif should be given in any case. Am 20.11.2020 um 07:58 schrieb Daniel Weismüller: > Hello, > > In my opinion it is sufficient to be able to set these parameters via > command line. > > It should only be made sure that the settings are persitend and not > overwritten by a reboot or the webif. > > - > 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@ipfire.org> wrote: >>> >>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>
Hi, > On 20 Nov 2020, at 06:58, Daniel Weismüller <daniel.weismueller@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. > It should only be made sure that the settings are persitend and not overwritten by a reboot or the webif. They won’t 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@ipfire.org> wrote: >>> >>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>
Am 20.11.2020 um 11:55 schrieb Michael Tremer: > Hi, > >> On 20 Nov 2020, at 06:58, Daniel Weismüller <daniel.weismueller@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. Ok I'll revise the script. >> It should only be made sure that the settings are persitend and not overwritten by a reboot or the webif. > They won’t 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@ipfire.org> wrote: >>>> >>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>>
This patch is fine and does not need to be changed. I will merge it for now and you will then send a second patch at a later time that extends the web UI to configure STP. Best, -Michael > On 20 Nov 2020, at 11:36, Daniel Weismüller <daniel.weismueller@ipfire.org> wrote: > > Am 20.11.2020 um 11:55 schrieb Michael Tremer: >> Hi, >> >>> On 20 Nov 2020, at 06:58, Daniel Weismüller <daniel.weismueller@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. > > Ok I'll revise the script. > > >>> It should only be made sure that the settings are persitend and not overwritten by a reboot or the webif. >> They won’t 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@ipfire.org> wrote: >>>>> >>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>>>
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@ipfire.org> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller@ipfire.org> Cc: development@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@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. > 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@ipfire.org> wrote: >>> >>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>
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@ipfire.org> > Sent: Friday, November 20, 2020 5:55 AM > To: Daniel Weismüller <daniel.weismueller@ipfire.org> > Cc: development@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@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. > >> 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@ipfire.org> wrote: >>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>> > >
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. (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@ipfire.org> >> Sent: Friday, November 20, 2020 5:55 AM >> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >> Cc: development@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@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. >> >>> 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@ipfire.org> wrote: >>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>>> >> >> >
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.
>
> (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@ipfire.org>
>>> Sent: Friday, November 20, 2020 5:55 AM
>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org>
>>> Cc: development@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@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.
>>>
>>>> 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@ipfire.org> wrote:
>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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
>>>>>>
>>>
>>>
>>
Hi, > On 23 Nov 2020, at 15:13, Daniel Weismüller <daniel.weismueller@ipfire.org> wrote: > > 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. Great drawing skills. I would propose to add a second row for the priority because you might not fit it all into one cell. The “priority” label in that box probably isn’t a good idea. > 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. I would like the default to be enabled on new systems only. Can we make that happen? -Michael > > - > > 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. >> >> (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@ipfire.org> >>>> Sent: Friday, November 20, 2020 5:55 AM >>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>> Cc: development@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@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. >>>> >>>>> 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@ipfire.org> wrote: >>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>>>>> >>>> >>>> >>> > <STP.png>
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. I took up your and Michael's suggestions and created a quick HTML demo. This new table could be placed below the NIC assignment table. What do you think? @Michael: I would like to base this new feature on my recently patched zoneconf.cgi. Is this somehow a bad idea? 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. >> >> (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@ipfire.org> >>>> Sent: Friday, November 20, 2020 5:55 AM >>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>> Cc: development@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@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. >>>> >>>>> 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@ipfire.org> wrote: >>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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 >>>>>>> >>>> >>>> >>>
Hello, > On 25 Nov 2020, at 17:00, Leo Hofmann <hofmann@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. 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. 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 :) > 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. >>> >>> (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@ipfire.org> >>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>> Cc: development@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@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. >>>>> >>>>>> 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@ipfire.org> wrote: >>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
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. - Daniel Am 25.11.20 um 21:57 schrieb Michael Tremer: > Hello, > >> On 25 Nov 2020, at 17:00, Leo Hofmann <hofmann@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. > 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. > > 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 :) > >> 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. >>>> >>>> (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@ipfire.org> >>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>> Cc: development@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@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. >>>>>> >>>>>>> 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@ipfire.org> wrote: >>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
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@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@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. >>> 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@ipfire.org> >>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>> Cc: development@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@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 ... 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@ipfire.org> wrote: >>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
Hello, > On 27 Nov 2020, at 08:34, Jonatan Schlag <jonatan.schlag@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@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@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@ipfire.org> >>>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>>> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
Hi All, On 27/11/2020 11:59, Michael Tremer wrote: > Hello, > >> On 27 Nov 2020, at 08:34, Jonatan Schlag <jonatan.schlag@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@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@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? As a user just of the native or vlan options in the zone table, I find it very good to be able to see the whole table in one go, with all 4 zones. When I look at the current situation, I have all four zones present on my system and can see them in one go (desktop system). Above it was written that having 4 zones would make it impossible to display on very small displays but that would be the current status because the demo page did not have any additional columns compared to the current approach, only more rows. It just only showed 3 zones instead of 4. The priority number might be harder to read because the size of the numbers is smaller than the rest but if the priority box height is made similar to the other boxes then the font size should also be similar. > > 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@ipfire.org> >>>>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>>>> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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> >
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@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@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@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@ipfire.org> >>>>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>>>> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
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@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@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@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@ipfire.org> >>>>>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>>>>> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
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@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@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@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@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@ipfire.org> >>>>>>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>>>>>> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
Hi, I vote for option one: two separate tables. There will be definitely more options to come, even if we might not see them now. I also see no direct connection between STP (or any other option) and which ports are connected to a zone. Another option would be like in the network of ipfire 3.x a table per zone. (like network zone net0 status). Greetings Jonatan Am Dienstag, den 01.12.2020, 16:27 +0000 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@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@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@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@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 secon > > > > > d 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@ipfire.org > > > > > > > > > > > > > > > > > > > > > > > > > Sent: Friday, November 20, 2020 5:55 AM > > > > > > > > > > > > To: Daniel Weismüller < > > > > > > > > > > > > daniel.weismueller@ipfire.org> > > > > > > > > > > > > Cc: development@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@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@ipfire.org> wrote: > > > > > > > > > > > > > > > Signed-off-by: Daniel Weismüller < > > > > > > > > > > > > > > > daniel.weismueller@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>
I would vote for option 1 as well - two separate tables. Option 2 just increases visual clutter and increases the time required to find the settings. Jonatan is correct, there will be more options added here as time goes on. Why start with so much crammed onto the table when it will likely have to be split out later anyway? Best regards, Fred Please note: Although we may sometimes respond to email, text and phone calls instantly at all hours of the day, our regular business hours are 9:00 AM - 6:00 PM ET, Monday thru Friday. -----Original Message----- From: Leo Hofmann <hofmann@leo-andres.de> Sent: Saturday, November 28, 2020 7:06 AM To: Michael Tremer <michael.tremer@ipfire.org>; Jonatan Schlag <jonatan.schlag@ipfire.org> Cc: development <development@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, 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@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@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@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@ipfire.org> >>>>>>>>> Sent: Friday, November 20, 2020 5:55 AM >>>>>>>>> To: Daniel Weismüller <daniel.weismueller@ipfire.org> >>>>>>>>> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>> Signed-off-by: Daniel Weismüller >>>>>>>>>>>> <daniel.weismueller@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>
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@gmail.com> (mailto:ahb.ipfire@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@ipfire.org> (mailto:jonatan.schlag@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@ipfire.org> (mailto:daniel.weismueller@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@leo-andres.de> (mailto:hofmann@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@ipfire.org> (mailto:michael.tremer@ipfire.org) Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller@ipfire.org> (mailto:daniel.weismueller@ipfire.org) Cc: development@lists.ipfire.org (mailto:development@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@ipfire.org> (mailto:daniel.weismueller@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@ipfire.org> (mailto:daniel.weismueller@ipfire.org) wrote:
Signed-off-by: Daniel Weismüller <daniel.weismueller@ipfire.org> (mailto:daniel.weismueller@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>
<!DOCTYPE html><html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8" /></head><body><div data-html-editor-font-wrapper="true" style="font-family: arial, sans-serif; font-size: 13px;"><div><div> <p>Hi Michael,</p> <p>I can start working on this next week, so I suggest we keep voting until 13.12.! Votes so far:<br>- One big table: II<br>- Two tables: II</p> <p>Regards<br>Leo</p> <div>Am 01.12.2020 um 17:27 schrieb Michael Tremer:</div> <blockquote type="cite" cite="mid:277BB02F-8213-487D-B630-EAEAF644BFF7@ipfire.org"> <pre wrap="">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</pre> <blockquote type="cite"> <pre wrap="">On 28 Nov 2020, at 13:24, Adolf Belka <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:ahb.ipfire@gmail.com"><ahb.ipfire@gmail.com></a> 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:</pre> <blockquote type="cite"> <pre wrap="">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:</pre> <blockquote type="cite"> <pre wrap="">Hello,</pre> <blockquote type="cite"> <pre wrap="">On 27 Nov 2020, at 08:34, Jonatan Schlag <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:jonatan.schlag@ipfire.org"><jonatan.schlag@ipfire.org></a> 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.</pre> <blockquote type="cite"><pre wrap="">Am 26.11.2020 um 15:47 schrieb Daniel Weismüller <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org"><daniel.weismueller@ipfire.org></a>: 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.</pre></blockquote> <pre wrap="">Shouldn’t we just not display these fields, if they do not matter?</pre> <blockquote type="cite"> <pre wrap="">- Daniel</pre> <blockquote type="cite"> <pre wrap="">Am 25.11.20 um 21:57 schrieb Michael Tremer: Hello,</pre> <blockquote type="cite"> <blockquote type="cite"><pre wrap="">On 25 Nov 2020, at 17:00, Leo Hofmann <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:hofmann@leo-andres.de"><hofmann@leo-andres.de></a> wrote:</pre></blockquote> <pre wrap="">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.</pre> </blockquote> </blockquote> </blockquote> <pre wrap="">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</pre> <blockquote type="cite"><blockquote type="cite"><pre wrap="">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.</pre></blockquote></blockquote> <pre wrap="">Definitely to complicated. Already right now.</pre> <blockquote type="cite"><blockquote type="cite"> <pre wrap="">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.</pre> <blockquote type="cite"><pre wrap="">I took up your and Michael's suggestions and created a quick HTML demo.</pre></blockquote> <pre wrap="">Looks good :)</pre> </blockquote></blockquote> <pre wrap="">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.</pre> </blockquote> <pre wrap="">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.</pre> <blockquote type="cite"> <blockquote type="cite"><blockquote type="cite"> <blockquote type="cite"><pre wrap="">This new table could be placed below the NIC assignment table. What do you think?</pre></blockquote> <pre wrap="">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.</pre> <blockquote type="cite"><pre wrap="">@Michael: I would like to base this new feature on my recently patched zoneconf.cgi. Is this somehow a bad idea?</pre></blockquote> <pre wrap="">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</pre> <blockquote type="cite"> <pre wrap="">Regards Leo Am 23.11.2020 um 16:13 schrieb Daniel Weismüller:</pre> <blockquote type="cite"> <pre wrap="">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:</pre> <blockquote type="cite"><pre wrap="">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.</pre></blockquote> </blockquote> </blockquote> </blockquote></blockquote> <pre wrap="">Thank you for submitting these patches. It is enjoyable to read good code.</pre> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> <pre wrap="">(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:</pre> <blockquote type="cite"> <pre wrap="">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:</pre> <blockquote type="cite"> <pre wrap="">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 <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:michael.tremer@ipfire.org"><michael.tremer@ipfire.org></a> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org"><daniel.weismueller@ipfire.org></a> Cc: <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:development@lists.ipfire.org">development@lists.ipfire.org</a> 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,</pre> <blockquote type="cite"><pre wrap="">On 20 Nov 2020, at 06:58, Daniel Weismüller</pre></blockquote> <pre wrap=""><a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org"><daniel.weismueller@ipfire.org></a> wrote:</pre> <blockquote type="cite"><pre wrap="">Hello, In my opinion it is sufficient to be able to set these parameters via</pre></blockquote> <pre wrap="">command line. Why is that? IPFire is a distribution that is supposed to be managed entirely by the web user interface.</pre> </blockquote> </blockquote> </blockquote></blockquote></blockquote></blockquote></blockquote> <pre wrap="">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 ...</pre> </blockquote> <pre wrap="">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.</pre> <blockquote type="cite"> <pre wrap="">Best Jonatan</pre> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> <blockquote type="cite"><pre wrap="">It should only be made sure that the settings are persitend and not</pre></blockquote> <pre wrap="">overwritten by a reboot or the webif. They wont be as they are in /var/ipfire/ethernet/settings. Best, -Michael</pre> <blockquote type="cite"> <pre wrap="">- Daniel Am 19.11.2020 um 15:56 schrieb Michael Tremer:</pre> <blockquote type="cite"><pre wrap="">Hello Daniel, This patch looks good to me. I had assumed that we automatically enabled STP on all bridges, but</pre></blockquote> </blockquote> <pre wrap="">apparently we do not.</pre> <blockquote type="cite"><blockquote type="cite"><pre wrap="">How do we process with this? I suppose it is not the most user-friendly way to ask the user to</pre></blockquote></blockquote> <pre wrap="">edit the configuration file. This either must be documented somewhere or the zoneconfig.cgi script needs to be extended to allow enabling STP.</pre> <blockquote type="cite"><blockquote type="cite"><pre wrap="">Does anyone want to be able to change any STP parameters like</pre></blockquote></blockquote> <pre wrap="">priority or cost of the ports?</pre> <blockquote type="cite"><blockquote type="cite"> <pre wrap="">Best, -Michael</pre> <blockquote type="cite"><pre wrap="">On 19 Nov 2020, at 13:18, Daniel Weismüller</pre></blockquote> </blockquote></blockquote> <pre wrap=""><a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org"><daniel.weismueller@ipfire.org></a> wrote:</pre> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><pre wrap="">Signed-off-by: Daniel Weismüller <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org"><daniel.weismueller@ipfire.org></a> --- config/udev/network-hotplug-bridges | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/udev/network-hotplug-bridges</pre></blockquote></blockquote></blockquote> <pre wrap="">b/config/udev/network-hotplug-bridges</pre> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><pre wrap="">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</pre></blockquote></blockquote></blockquote> <pre wrap="">bridge</pre> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><pre wrap="">+ ip link add "${BRIDGE}" address "${ADDRESS}" type</pre></blockquote></blockquote></blockquote> <pre wrap="">bridge \</pre> <blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><pre wrap="">+ $([ "${STP}" = "on" ] && echo "stp_state 1") #ip link set "${BRIDGE}" up fi -- 2.28.0</pre></blockquote></blockquote></blockquote> </blockquote></blockquote></blockquote></blockquote> <pre wrap=""><zoneconf-stp.png></pre> </blockquote></blockquote></blockquote> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap=""> </pre> </blockquote> </div></div></div></body></html>
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@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@gmail.com> <mailto:ahb.ipfire@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@ipfire.org> <mailto:jonatan.schlag@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@ipfire.org> <mailto:daniel.weismueller@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@leo-andres.de> <mailto:hofmann@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@ipfire.org> <mailto:michael.tremer@ipfire.org> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller<daniel.weismueller@ipfire.org> <mailto:daniel.weismueller@ipfire.org> Cc:development@lists.ipfire.org <mailto:development@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@ipfire.org> <mailto:daniel.weismueller@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@ipfire.org> <mailto:daniel.weismueller@ipfire.org> wrote: >>>>>>>>>>>>>>>> Signed-off-by: Daniel Weismüller<daniel.weismueller@ipfire.org> <mailto:daniel.weismueller@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> >> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> <p>H Leo,</p> <p>I vote for "One big table" but with a few change requests.</p> <p>- please change the order of the nics to red, green, orange, blue</p> <p>- 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.<br> </p> <p>-</p> <p>Daniel<br> </p> <p> <br> </p> <div class="moz-cite-prefix">Am 10.12.20 um 09:24 schrieb <a class="moz-txt-link-abbreviated" href="mailto:hofmann@leo-andres.de">hofmann@leo-andres.de</a>:<br> </div> <blockquote type="cite" cite="mid:1ef3916892a947e6b777fdf640eee26e@leo-andres.de"> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <div data-html-editor-font-wrapper="true" style="font-family: arial, sans-serif; font-size: 13px;"> <div> <div> <p>Hi Michael,</p> <p>I can start working on this next week, so I suggest we keep voting until 13.12.! Votes so far:<br> - One big table: II<br> - Two tables: II</p> <p>Regards<br> Leo</p> <div>Am 01.12.2020 um 17:27 schrieb Michael Tremer:</div> <blockquote type="cite" cite="mid:277BB02F-8213-487D-B630-EAEAF644BFF7@ipfire.org"> <pre wrap="">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</pre> <blockquote type="cite"> <pre wrap="">On 28 Nov 2020, at 13:24, Adolf Belka <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:ahb.ipfire@gmail.com" moz-do-not-send="true"><ahb.ipfire@gmail.com></a> 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:</pre> <blockquote type="cite"> <pre wrap="">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:</pre> <blockquote type="cite"> <pre wrap="">Hello,</pre> <blockquote type="cite"> <pre wrap="">On 27 Nov 2020, at 08:34, Jonatan Schlag <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:jonatan.schlag@ipfire.org" moz-do-not-send="true"><jonatan.schlag@ipfire.org></a> 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.</pre> <blockquote type="cite"> <pre wrap="">Am 26.11.2020 um 15:47 schrieb Daniel Weismüller <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org" moz-do-not-send="true"><daniel.weismueller@ipfire.org></a>: 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.</pre> </blockquote> <pre wrap="">Shouldn’t we just not display these fields, if they do not matter?</pre> <blockquote type="cite"> <pre wrap="">- Daniel</pre> <blockquote type="cite"> <pre wrap="">Am 25.11.20 um 21:57 schrieb Michael Tremer: Hello,</pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">On 25 Nov 2020, at 17:00, Leo Hofmann <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:hofmann@leo-andres.de" moz-do-not-send="true"><hofmann@leo-andres.de></a> wrote:</pre> </blockquote> <pre wrap="">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.</pre> </blockquote> </blockquote> </blockquote> <pre wrap="">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</pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">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.</pre> </blockquote> </blockquote> <pre wrap="">Definitely to complicated. Already right now.</pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">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.</pre> <blockquote type="cite"> <pre wrap="">I took up your and Michael's suggestions and created a quick HTML demo.</pre> </blockquote> <pre wrap="">Looks good :)</pre> </blockquote> </blockquote> <pre wrap="">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.</pre> </blockquote> <pre wrap="">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.</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">This new table could be placed below the NIC assignment table. What do you think?</pre> </blockquote> <pre wrap="">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.</pre> <blockquote type="cite"> <pre wrap="">@Michael: I would like to base this new feature on my recently patched zoneconf.cgi. Is this somehow a bad idea?</pre> </blockquote> <pre wrap="">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</pre> <blockquote type="cite"> <pre wrap="">Regards Leo Am 23.11.2020 um 16:13 schrieb Daniel Weismüller:</pre> <blockquote type="cite"> <pre wrap="">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:</pre> <blockquote type="cite"> <pre wrap="">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.</pre> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap="">Thank you for submitting these patches. It is enjoyable to read good code.</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">(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:</pre> <blockquote type="cite"> <pre wrap="">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:</pre> <blockquote type="cite"> <pre wrap="">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 <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:michael.tremer@ipfire.org" moz-do-not-send="true"><michael.tremer@ipfire.org></a> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org" moz-do-not-send="true"><daniel.weismueller@ipfire.org></a> Cc: <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:development@lists.ipfire.org" moz-do-not-send="true">development@lists.ipfire.org</a> 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,</pre> <blockquote type="cite"> <pre wrap="">On 20 Nov 2020, at 06:58, Daniel Weismüller</pre> </blockquote> <pre wrap=""><a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org" moz-do-not-send="true"><daniel.weismueller@ipfire.org></a> wrote:</pre> <blockquote type="cite"> <pre wrap="">Hello, In my opinion it is sufficient to be able to set these parameters via</pre> </blockquote> <pre wrap="">command line. Why is that? IPFire is a distribution that is supposed to be managed entirely by the web user interface.</pre> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap="">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 ...</pre> </blockquote> <pre wrap="">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.</pre> <blockquote type="cite"> <pre wrap="">Best Jonatan</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">It should only be made sure that the settings are persitend and not</pre> </blockquote> <pre wrap="">overwritten by a reboot or the webif. They wont be as they are in /var/ipfire/ethernet/settings. Best, -Michael</pre> <blockquote type="cite"> <pre wrap="">- Daniel Am 19.11.2020 um 15:56 schrieb Michael Tremer:</pre> <blockquote type="cite"> <pre wrap="">Hello Daniel, This patch looks good to me. I had assumed that we automatically enabled STP on all bridges, but</pre> </blockquote> </blockquote> <pre wrap="">apparently we do not.</pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">How do we process with this? I suppose it is not the most user-friendly way to ask the user to</pre> </blockquote> </blockquote> <pre wrap="">edit the configuration file. This either must be documented somewhere or the zoneconfig.cgi script needs to be extended to allow enabling STP.</pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">Does anyone want to be able to change any STP parameters like</pre> </blockquote> </blockquote> <pre wrap="">priority or cost of the ports?</pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">Best, -Michael</pre> <blockquote type="cite"> <pre wrap="">On 19 Nov 2020, at 13:18, Daniel Weismüller</pre> </blockquote> </blockquote> </blockquote> <pre wrap=""><a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org" moz-do-not-send="true"><daniel.weismueller@ipfire.org></a> wrote:</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">Signed-off-by: Daniel Weismüller <a rel="external nofollow noopener noreferrer" target="_blank" tabindex="-1" href="mailto:daniel.weismueller@ipfire.org" moz-do-not-send="true"><daniel.weismueller@ipfire.org></a> --- config/udev/network-hotplug-bridges | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/udev/network-hotplug-bridges</pre> </blockquote> </blockquote> </blockquote> <pre wrap="">b/config/udev/network-hotplug-bridges</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">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</pre> </blockquote> </blockquote> </blockquote> <pre wrap="">bridge</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">+ ip link add "${BRIDGE}" address "${ADDRESS}" type</pre> </blockquote> </blockquote> </blockquote> <pre wrap="">bridge \</pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">+ $([ "${STP}" = "on" ] && echo "stp_state 1") #ip link set "${BRIDGE}" up fi -- 2.28.0</pre> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap=""><zoneconf-stp.png></pre> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap=""> </pre> </blockquote> </div> </div> </div> </blockquote> </body> </html>
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@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@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@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@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@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@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@ipfire.org> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller@ipfire.org> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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> >>>
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. 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@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@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@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@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@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@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@ipfire.org> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller@ipfire.org> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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> >>>>
Hi, > On 10 Dec 2020, at 18:59, Leo Hofmann <hofmann@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@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@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@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@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@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@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@ipfire.org> Sent: Friday, November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller@ipfire.org> Cc: development@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@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@ipfire.org> wrote: >>>>>>>>>>>>>>>>>>> Signed-off-by: Daniel Weismüller <daniel.weismueller@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> >>>>>
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@ipfire.org> schrieb: > Hi, > >> On 10 Dec 2020, at 18:59, Leo Hofmann <hofmann@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@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@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@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@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@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@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@ipfire.org> Sent: Friday, >> November 20, 2020 5:55 AM To: Daniel Weismüller <daniel.weismueller@ipfire.org> Cc: >> development@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@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@ipfire.org> wrote: >> Signed-off-by: Daniel Weismüller <daniel.weismueller@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>
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