update sample scripts to use iproute #165
Conversation
Old `ioctl` based tools like `brctl` are deprecated and have been removed from the default package set of some distributions. Also drop usage of ebtables in favour of native bridge port isolation available in kernels 4.18 and newer. Signed-off-by: Felix Kaechele <[email protected]>
|
|
||
| # Turn on bridge port isolation | ||
| bridge link set dev $INTERFACE isolated on |
There was a problem hiding this comment.
This looks nicer than ebtables indeed, but OTOH there is a race condition here now, is there? Between the time this is added to the bridge, and when this bridge link set is executed, the host can communicate with all other hosts?
There was a problem hiding this comment.
Thanks for pointing this out. I think you might be right. Maybe we can up the link only after setting isolated on. I'll test that.
There was a problem hiding this comment.
Would a similar "isolated on" be needed in the mtu_changed script?
There was a problem hiding this comment.
I just checked, and yes, it would be necessary to set to isolated again when changing bridges with the mtu script. The port status can be seen in /sys/class/net/$INTERFACE/brport/isolated
Also, it's not necessary to use the bridge command. You can also simply echo 1 > /sys/class/net/$INTERFACE/brport/isolated
There was a problem hiding this comment.
Pushed a fix to my branch. Thanks for testing.
There was a problem hiding this comment.
I still recommend doing the port isolation with echo 1 > /sys/class/net/$INTERFACE/brport/isolated since it does not depend on bridge being installed.
This is most likely only important for embedded devices which try to minimize the amount of packages installed on the system.
Signed-off-by: Felix Kaechele <[email protected]>
| @@ -16,8 +16,6 @@ ensure_bridge() | |||
| ip addr add 10.254.0.2/16 dev $brname | |||
| # TODO Policy routing should probably not be hardcoded here? | |||
| ensure_policy from all iif $brname lookup mesh prio 1000 | |||
| # Disable forwarding between bridge ports | |||
| ebtables -A FORWARD --logical-in $brname -j DROP | |||
There was a problem hiding this comment.
Why didn't you put the isolation on here? That would avoid having to audit all places where ensure_bridge is used.
There was a problem hiding this comment.
You have to do the isolation per port, and isolating the bridge interface itself would mean that none of the attached bridge porst would be able to communicate with host.
There was a problem hiding this comment.
@pmelange is correct. isolation on is a function of the bridge port, not the bridge itself. As such it would functionally belong into the session handler scripts.
|
|
||
| # Bring the tunnel interface up only after port isolation is enabled | ||
| ip link set dev $INTERFACE up |
There was a problem hiding this comment.
I believe that just removing the interface from the old bridge does not bring the interface down. So, I suggest just after removing the interface from the old bridge, do an ip link set dev $INTERFACE down
There was a problem hiding this comment.
I will need to test this. Conversely, if what you say is true we could, things should continue to work even if we don't bring up the interface as we add it to the bridge for the new MTU value.
Old
ioctlbased tools likebrctlare deprecated and have beenremoved from the default package set of some distributions.
Also drop usage of ebtables in favour of native bridge port isolation
available in kernels 4.18 and newer.
Signed-off-by: Felix Kaechele [email protected]