#1553 PR closed: Add support for Bridge Interfaces

Labels: enhancement, won't fix / can't fix / obsolete

rmetrich opened issue at 2017-10-27 15:11:

Bridge interfaces are widely used. This patch enables configurations as
shown below:

  1. Bridge over simple Ethernet
  2. Bridge over Bond
  3. Bridger over Vlan interface

Bridges require the 'brctl' utility. This patch automatically includes
the utility upon need.
Usually, virtual interfaces are skipped, but for Bridges to work, we
consider Bridges as physical interfaces, because the Bridge interface
holds the IP address, not the physical interface attached to the Bridge.

Signed-off-by: Renaud M├ętrich rmetrich@redhat.com

gozora commented at 2017-10-27 15:25:

Hello @rmetrich
First and foremost thanks for this patch!

I have one question though, imagine that original system was setup without bridge-utils (containing brctl) and iproute2 was used instead (for whatever reason).
Do we want to force users to install additional packages despite they are not really needed for restore?

V.

rmetrich commented at 2017-10-30 08:58:

@gozora You are right, I've rewritten the code to use iproute2 only.
@gdha I don't understand your point, the bridge_handling function is used to create the bridge commands in the 60-network-devices.sh generated script.

gozora commented at 2017-10-30 09:44:

Hello @rmetrich,

Just one more remark, bridge configuration requires bridge.ko kernel module, which I'm afraid is not bundled into ReaR recovery system by default. So you might consider using MODULES (or similar) configuration option somewhere in your code.

V.

rmetrich commented at 2017-10-30 10:01:

@gozora thanks, good point. Was already included in my kernel but who knows ...

rmetrich commented at 2017-10-31 14:30:

@gdha hello, please advise regarding the bridge_handling function: I don't understand your point, the bridge_handling function is used to create the bridge commands in the 60-network-devices.sh generated script.

gdha commented at 2017-10-31 14:55:

@rmetrich What I meant with the bridge_handling function is to move the function code itself to a library script (e.g. the lib/network-functions.sh) instead of keeping it inside script 310_network_devices.sh (pure cosmetic and easier to read). Functions should never (well almost never) be part of a script. We have a special location for it under lib/ directory.
Hopefully I expressed myself better then before?

rmetrich commented at 2017-10-31 15:09:

@gdha hmm, here the function adds to generated script ($network_devices_setup_script), same as function vlan_setup and bond_setup, so should likely remain local.

gdha commented at 2017-10-31 15:47:

@rmetrich OK then - keep it together if you prefer that.

rmetrich commented at 2017-11-03 14:02:

@gdha Is there something special required from me? Such as squashing the commits?

jsmeix commented at 2017-11-06 11:46:

A side note regarding functions that are used only in one script:

I prefer to keep local stuff local so that I prefer to have
such functions defined only in the script where needed
and not globally defined.

In particular because such functions are often a bit
"quick and dirty" which is perfectly o.k. for a specific
script-local function that is not intended to be usable
in whatever generic ways by arbitrary scripts.

Because bash does not support

local function ...

such script-local function should be better 'unset -f'
when leaving the script (also care about 'return'), e.g. see
usr/share/rear/build/GNU/Linux/390_copy_binaries_libraries.sh
and
usr/share/rear/build/GNU/Linux/400_copy_modules.sh

rmetrich commented at 2017-11-06 12:35:

@jsmeix Sounds reasonable. Added the unset for all local functions in 310_network_devices.sh

rmetrich commented at 2017-11-08 10:25:

I'm not satisfied with my code, specially the bridges purely virtual (e.g. libvirt) get configured, which is useless.

jsmeix commented at 2017-11-08 15:35:

Superseded by https://github.com/rear/rear/pull/1570


[Export of Github issue for rear/rear.]