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" + } + } + } +}