From 4dae6d0b669206686c991ae12892a17608f46bde Mon Sep 17 00:00:00 2001 From: Michael Zimmermann Date: Wed, 13 Nov 2024 21:31:21 +0100 Subject: [PATCH] network: bridge: add support for unmanaged mode While Linux doesn't support modes on bridges, we use this concept to let the user tell us if they want podman/netavark to own the bridge or not. Managed behaves the same way as before this commit. Unmanaged requires the bridge to exist already, will not setup any sysctls or firewall rules on the host and will not delete the bridge once all containers left. Fixes #1090 Signed-off-by: Michael Zimmermann --- src/network/bridge.rs | 165 ++++++++++++++++----------- test/620-bridge-mode.bats | 42 +++++++ test/testfiles/bridge-managed.json | 32 ++++++ test/testfiles/bridge-unmanaged.json | 26 +++++ 4 files changed, 200 insertions(+), 65 deletions(-) create mode 100644 test/620-bridge-mode.bats create mode 100644 test/testfiles/bridge-managed.json create mode 100644 test/testfiles/bridge-unmanaged.json diff --git a/src/network/bridge.rs b/src/network/bridge.rs index ce89427a..7e843c25 100644 --- a/src/network/bridge.rs +++ b/src/network/bridge.rs @@ -21,7 +21,7 @@ use super::{ constants::{ ISOLATE_OPTION_FALSE, ISOLATE_OPTION_STRICT, ISOLATE_OPTION_TRUE, NO_CONTAINER_INTERFACE_ERROR, OPTION_HOST_INTERFACE_NAME, OPTION_ISOLATE, OPTION_METRIC, - OPTION_MTU, OPTION_NO_DEFAULT_ROUTE, OPTION_VRF, + OPTION_MODE, OPTION_MTU, OPTION_NO_DEFAULT_ROUTE, OPTION_VRF, }, core_utils::{self, get_ipam_addresses, join_netns, parse_option, CoreUtils}, driver::{self, DriverInfo}, @@ -35,6 +35,14 @@ use super::{ const NO_BRIDGE_NAME_ERROR: &str = "no bridge interface name given"; +#[derive(Clone, Copy, PartialEq)] +enum BridgeMode { + /// The bridge is managed by netavark. + Managed, + /// The bridge was created externally and we only add/remove veths. + Unmanaged, +} + struct InternalData { /// interface name of the veth pair inside the container netns container_interface_name: String, @@ -52,6 +60,8 @@ struct InternalData { isolate: IsolateOption, /// Route metric for any default routes added for the network metric: Option, + /// Management mode of the bridge. + mode: BridgeMode, /// if set, no default gateway will be added no_default_route: bool, /// sef vrf for bridge @@ -82,6 +92,7 @@ impl driver::NetworkDriver for Bridge<'_> { } let ipam = get_ipam_addresses(self.info.per_network_opts, self.info.network)?; + let mode: Option = parse_option(&self.info.network.options, OPTION_MODE)?; let mtu: u32 = parse_option(&self.info.network.options, OPTION_MTU)?.unwrap_or(0); let isolate: IsolateOption = get_isolate_option(&self.info.network.options)?; let metric: u32 = parse_option(&self.info.network.options, OPTION_METRIC)?.unwrap_or(100); @@ -108,6 +119,7 @@ impl driver::NetworkDriver for Bridge<'_> { mtu, isolate, metric: Some(metric), + mode: get_bridge_mode_from_string(mode.as_deref())?, no_default_route, vrf, }); @@ -133,9 +145,11 @@ impl driver::NetworkDriver for Bridge<'_> { data.bridge_interface_name, data.ipam.gateway_addresses ); - setup_ipv4_fw_sysctl()?; - if data.ipam.ipv6_enabled { - setup_ipv6_fw_sysctl()?; + if let BridgeMode::Managed = data.mode { + setup_ipv4_fw_sysctl()?; + if data.ipam.ipv6_enabled { + setup_ipv6_fw_sysctl()?; + } } let (host_sock, netns_sock) = netlink_sockets; @@ -224,30 +238,30 @@ impl driver::NetworkDriver for Bridge<'_> { None }; - // if the network is internal block routing and do not setup firewall rules - if self.info.network.internal { - CoreUtils::apply_sysctl_value( - format!( - "/proc/sys/net/ipv4/conf/{}/forwarding", - data.bridge_interface_name - ), - "0", - )?; - if data.ipam.ipv6_enabled { + if let BridgeMode::Managed = data.mode { + // if the network is internal block routing and do not setup firewall rules + if self.info.network.internal { CoreUtils::apply_sysctl_value( format!( - "/proc/sys/net/ipv6/conf/{}/forwarding", + "/proc/sys/net/ipv4/conf/{}/forwarding", data.bridge_interface_name ), "0", )?; + if data.ipam.ipv6_enabled { + CoreUtils::apply_sysctl_value( + format!( + "/proc/sys/net/ipv6/conf/{}/forwarding", + data.bridge_interface_name + ), + "0", + )?; + } + } else { + self.setup_firewall(data)? } - // return here to skip setting up firewall rules - return Ok((response, aardvark_entry)); } - self.setup_firewall(data)?; - Ok((response, aardvark_entry)) } @@ -255,6 +269,8 @@ impl driver::NetworkDriver for Bridge<'_> { &self, netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket), ) -> NetavarkResult<()> { + let mode: Option = parse_option(&self.info.network.options, OPTION_MODE)?; + let mode = get_bridge_mode_from_string(mode.as_deref())?; let (host_sock, netns_sock) = netlink_sockets; let mut error_list = NetavarkErrorList::new(); @@ -271,6 +287,7 @@ impl driver::NetworkDriver for Bridge<'_> { let complete_teardown = match remove_link( host_sock, netns_sock, + mode, &bridge_name, &self.info.per_network_opts.interface_name, ) { @@ -281,20 +298,15 @@ impl driver::NetworkDriver for Bridge<'_> { } }; - if self.info.network.internal { - if !error_list.is_empty() { - return Err(NetavarkError::List(error_list)); + if !self.info.network.internal && mode == BridgeMode::Managed { + match self.teardown_firewall(complete_teardown, bridge_name) { + Ok(_) => {} + Err(err) => { + error_list.push(err); + } } - return Ok(()); } - match self.teardown_firewall(complete_teardown, bridge_name) { - Ok(_) => {} - Err(err) => { - error_list.push(err); - } - }; - if !error_list.is_empty() { return Err(NetavarkError::List(error_list)); } @@ -550,6 +562,12 @@ fn create_interfaces( // for all other errors we want to return the error return Err(err).wrap("get bridge interface"); } + + if let BridgeMode::Unmanaged = data.mode { + return Err(err) + .wrap("in unmanaged mode, the bridge must already exist on the host"); + } + let mut create_link_opts = netlink::CreateLinkOptions::new( data.bridge_interface_name.to_string(), InfoKind::Bridge, @@ -703,41 +721,44 @@ fn create_veth_pair<'fd>( )); } - exec_netns!(hostns_fd, netns_fd, res, { - disable_ipv6_autoconf(&data.container_interface_name)?; - if data.ipam.ipv6_enabled { - // Disable dad inside the container too - let disable_dad_in_container = format!( - "/proc/sys/net/ipv6/conf/{}/accept_dad", + if let BridgeMode::Managed = data.mode { + exec_netns!(hostns_fd, netns_fd, res, { + disable_ipv6_autoconf(&data.container_interface_name)?; + if data.ipam.ipv6_enabled { + // Disable dad inside the container too + let disable_dad_in_container = format!( + "/proc/sys/net/ipv6/conf/{}/accept_dad", + &data.container_interface_name + ); + core_utils::CoreUtils::apply_sysctl_value(disable_dad_in_container, "0")?; + } + let enable_arp_notify = format!( + "/proc/sys/net/ipv4/conf/{}/arp_notify", &data.container_interface_name ); - core_utils::CoreUtils::apply_sysctl_value(disable_dad_in_container, "0")?; - } - let enable_arp_notify = format!( - "/proc/sys/net/ipv4/conf/{}/arp_notify", - &data.container_interface_name - ); - core_utils::CoreUtils::apply_sysctl_value(enable_arp_notify, "1")?; - - // disable strict reverse path search validation - let rp_filter = format!( - "/proc/sys/net/ipv4/conf/{}/rp_filter", - &data.container_interface_name - ); - CoreUtils::apply_sysctl_value(rp_filter, "2")?; - Ok::<(), NetavarkError>(()) - }); - // check the result and return error - res?; + core_utils::CoreUtils::apply_sysctl_value(enable_arp_notify, "1")?; - if data.ipam.ipv6_enabled { - let host_veth = host.get_link(netlink::LinkID::ID(host_link))?; + // disable strict reverse path search validation + let rp_filter = format!( + "/proc/sys/net/ipv4/conf/{}/rp_filter", + &data.container_interface_name + ); + CoreUtils::apply_sysctl_value(rp_filter, "2")?; + Ok::<(), NetavarkError>(()) + }); + // check the result and return error + res?; - for nla in host_veth.attributes.into_iter() { - if let LinkAttribute::IfName(name) = nla { - // Disable dad inside on the host too - let disable_dad_in_container = format!("/proc/sys/net/ipv6/conf/{name}/accept_dad"); - core_utils::CoreUtils::apply_sysctl_value(disable_dad_in_container, "0")?; + if data.ipam.ipv6_enabled { + let host_veth = host.get_link(netlink::LinkID::ID(host_link))?; + + for nla in host_veth.attributes.into_iter() { + if let LinkAttribute::IfName(name) = nla { + // Disable dad inside on the host too + let disable_dad_in_container = + format!("/proc/sys/net/ipv6/conf/{name}/accept_dad"); + core_utils::CoreUtils::apply_sysctl_value(disable_dad_in_container, "0")?; + } } } } @@ -830,6 +851,7 @@ fn check_link_is_vrf(msg: LinkMessage, vrf_name: &str) -> NetavarkResult NetavarkResult { @@ -848,10 +870,12 @@ fn remove_link( .wrap("failed to get connected bridge interfaces")?; // no connected interfaces on that bridge we can remove it if links.is_empty() { - log::info!("removing bridge {}", br_name); - host.del_link(netlink::LinkID::ID(br.header.index)) - .wrap(format!("failed to delete bridge {container_veth_name}"))?; - return Ok(true); + if let BridgeMode::Managed = mode { + log::info!("removing bridge {}", br_name); + host.del_link(netlink::LinkID::ID(br.header.index)) + .wrap(format!("failed to delete bridge {container_veth_name}"))?; + return Ok(true); + } } Ok(false) } @@ -866,3 +890,14 @@ fn get_isolate_option(opts: &Option>) -> NetavarkResult< _ => IsolateOption::Never, }) } + +fn get_bridge_mode_from_string(mode: Option<&str>) -> NetavarkResult { + match mode { + // default to l3 when unset + None | Some("") | Some("managed") => Ok(BridgeMode::Managed), + Some("unmanaged") => Ok(BridgeMode::Unmanaged), + Some(name) => Err(NetavarkError::msg(format!( + "invalid bridge mode \"{name}\"" + ))), + } +} diff --git a/test/620-bridge-mode.bats b/test/620-bridge-mode.bats new file mode 100644 index 00000000..83c215a6 --- /dev/null +++ b/test/620-bridge-mode.bats @@ -0,0 +1,42 @@ +#!/usr/bin/env bats -*- bats -*- +# +# bridge driver tests with explicit modes +# + +load helpers + +@test bridge - managed mode { + run_netavark --file ${TESTSDIR}/testfiles/bridge-managed.json setup $(get_container_netns_path) + + run_in_host_netns ip -j --details link show podman0 + link_info="$output" + assert_json "$link_info" '.[].flags[] | select(.=="UP")' == "UP" "Host bridge interface is up" + + run_netavark --file ${TESTSDIR}/testfiles/bridge-managed.json teardown $(get_container_netns_path) + + # make sure, that the bridge was removed + expected_rc=1 run_in_host_netns ip -j --details link show podman0 + assert "$output" "==" 'Device "podman0" does not exist.' +} + +@test bridge - unmanaged mode { + expected_rc=1 run_netavark --file ${TESTSDIR}/testfiles/bridge-unmanaged.json setup $(get_container_netns_path) + assert_json ".error" "in unmanaged mode, the bridge must already exist on the host: Netlink error: No such device (os error 19)" + + run_in_host_netns ip link add brtest0 type bridge + run_in_host_netns ip link set brtest0 up + + run_netavark --file ${TESTSDIR}/testfiles/bridge-unmanaged.json setup $(get_container_netns_path) + + run_in_host_netns ip -j --details link show brtest0 + link_info="$output" + assert_json "$link_info" '.[].flags[] | select(.=="UP")' == "UP" "Host bridge interface is up" + + run_netavark --file ${TESTSDIR}/testfiles/bridge-unmanaged.json teardown $(get_container_netns_path) + + # make sure, that the bridge was NOT removed + run_in_host_netns ip -j --details link show brtest0 + link_info="$output" + assert_json "$link_info" '.[].flags[] | select(.=="UP")' == "UP" "Host bridge interface is up" +} + diff --git a/test/testfiles/bridge-managed.json b/test/testfiles/bridge-managed.json new file mode 100644 index 00000000..3269ece0 --- /dev/null +++ b/test/testfiles/bridge-managed.json @@ -0,0 +1,32 @@ +{ + "container_id": "6ce776ea58b5", + "container_name": "testcontainer", + "networks": { + "podman": { + "interface_name": "eth0", + "static_ips": [ + "10.88.0.2" + ] + } + }, + "network_info": { + "podman": { + "dns_enabled": false, + "driver": "bridge", + "id": "53ce4390f2adb1681eb1a90ec8b48c49c015e0a8d336c197637e7f65e365fa9e", + "internal": false, + "ipv6_enabled": false, + "name": "podman", + "network_interface": "podman0", + "subnets": [ + { + "gateway": "10.88.0.1", + "subnet": "10.88.0.0/16" + } + ], + "options": { + "mode": "managed" + } + } + } +} diff --git a/test/testfiles/bridge-unmanaged.json b/test/testfiles/bridge-unmanaged.json new file mode 100644 index 00000000..018c52c7 --- /dev/null +++ b/test/testfiles/bridge-unmanaged.json @@ -0,0 +1,26 @@ +{ + "container_id": "6ce776ea58b5", + "container_name": "testcontainer", + "networks": { + "podman": { + "interface_name": "eth0", + "static_ips": [ + "10.88.0.2" + ] + } + }, + "network_info": { + "podman": { + "dns_enabled": false, + "driver": "bridge", + "id": "53ce4390f2adb1681eb1a90ec8b48c49c015e0a8d336c197637e7f65e365fa9e", + "internal": false, + "ipv6_enabled": false, + "name": "podman", + "network_interface": "brtest0", + "options": { + "mode": "unmanaged" + } + } + } +}