Bug 1358615
| Summary: | [RFE] Provide additional options for bridge configuration. | |||
|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Andreas Bleischwitz <ableisch> | |
| Component: | NetworkManager | Assignee: | Beniamino Galvani <bgalvani> | |
| Status: | CLOSED ERRATA | QA Contact: | Desktop QE <desktop-qa-list> | |
| Severity: | medium | Docs Contact: | ||
| Priority: | low | |||
| Version: | 7.2 | CC: | ableisch, atragler, bgalvani, lrintel, mleitner, rkhan, sfroemer, sukulkar, thaller, vbenes | |
| Target Milestone: | rc | Keywords: | FutureFeature | |
| Target Release: | 7.4 | |||
| Hardware: | All | |||
| OS: | Linux | |||
| Whiteboard: | ||||
| Fixed In Version: | Doc Type: | If docs needed, set a value | ||
| Doc Text: | Story Points: | --- | ||
| Clone Of: | ||||
| : | 1491322 (view as bug list) | Environment: | ||
| Last Closed: | 2018-04-10 13:19:11 UTC | Type: | Bug | |
| Regression: | --- | Mount Type: | --- | |
| Documentation: | --- | CRM: | ||
| Verified Versions: | Category: | --- | ||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | ||
| Cloudforms Team: | --- | Target Upstream Version: | ||
| Embargoed: | ||||
| Bug Depends On: | ||||
| Bug Blocks: | 1420851, 1470965, 1473733, 1491322 | |||
| Attachments: | ||||
|
Description
Andreas Bleischwitz
2016-07-21 07:21:38 UTC
> - How would the customer like to achieve this? (List the functional requirements)
> As this required option is just one of many, I would suggest to create
> a more generic interface which scans /sys/class/net/*/bridge/* for
> write-able files and provides a way to change the parameters found in
> that directory. That way this RFE would be future safe as new options
> would automatically be configurable. Integration into UI
> (nmtui.nm-connection-editor) does not yet need to be provided. nmcli
> should be able to set these options.
While providing a generic interface for setting options discovered at
runtime would help with new bridge options that are needed in the
future, unfortunately this doesn't fit very well the current
NetworkManager model.
First, NetworkManager must understand the meaning of the specific
options to perform syntax validation and checking of dependencies
between them. This is similar to what we do for bonds, where some
options must be applied in a specific order and some exclude
others. Applying the parameters in the wrong order means that some
will not have effect.
Also, reading /sys/class/net/*/bridge/* requires that a bridge is
already present in the system, which is not always the case when you
want to edit/create a new connection.
Integration in the UI (including nmcli and nmtui) would also be a
problem if NM doesn't understand the options, because the UI could
only provide a field where users can enter whatever they want without
any validation, which generally represents a bad user experience.
So, I believe it's better if we add a new property for each bridge
option that is needed. I understand that this bugzilla is about the
"group_fwd_mask" option; do you have any other ones in mind that could
be useful?
Thanks.
Thanks Beniamino for the explanation. Makes sense. From our current requirements "group_fwd_mask" would be sufficient. Reading https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-net, I could not imagine any other important option which is not already covered. Created attachment 1303510 [details]
[NM patch] bridge: introduce a bridge.group-forward-mask connection property
Created attachment 1303512 [details]
[nm-c-e patch] bridge: add support for group-forward-mask property
Both the NM & the editor patches look good to me. lgtm too Applied to master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=17ec3aef2fa4d31ef80f54923f89e6719f1ce83b https://git.gnome.org/browse/network-manager-applet/commit/?id=4a780cc25d69d2dce983a218f5eb435b696a86df (In reply to Beniamino Galvani from comment #11) > https://git.gnome.org/browse/network-manager-applet/commit/?id=4a780cc25d69d2dce983a218f5eb435b696a86df I reverted the patch in applet, because it breaks build: https://git.gnome.org/browse/network-manager-applet/commit/?id=3721371ee8b9762725ddd163204fdaa086812b33 Created attachment 1319895 [details]
[nm-c-e patch v2] bridge: add support for group-forward-mask property
nm-connection-editor v2 patch applied to master: https://git.gnome.org/browse/network-manager-applet/commit/?id=4f5a4d3a8b75f0d60f8124f59dcdcfe8e2888c05 Created attachment 1335308 [details]
[PATCH] tui: add group-forward-mask property to bridge page
Add missing support in nmtui.
(In reply to Beniamino Galvani from comment #17) > Created attachment 1335308 [details] > [PATCH] tui: add group-forward-mask property to bridge page > > Add missing support in nmtui. lgtm (In reply to Thomas Haller from comment #18) > (In reply to Beniamino Galvani from comment #17) > > Created attachment 1335308 [details] > > [PATCH] tui: add group-forward-mask property to bridge page > > > > Add missing support in nmtui. > > lgtm Applied to master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=802116db43381f67b873e7e0283485c4ff035fc9 3 tests added into CI test suite. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2018:0778 |