Skip to content

Commit

Permalink
Merge pull request #1126 from M1cha/l2-bridge
Browse files Browse the repository at this point in the history 
network: bridge: add support for unmanaged mode
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Nov 22, 2024
2 parents d7184d6 + 2900e92 commit d5358e4
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 65 deletions.
167 changes: 102 additions & 65 deletions src/network/bridge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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,
Expand All @@ -52,6 +60,8 @@ struct InternalData {
isolate: IsolateOption,
/// Route metric for any default routes added for the network
metric: Option<u32>,
/// Management mode of the bridge.
mode: BridgeMode,
/// if set, no default gateway will be added
no_default_route: bool,
/// sef vrf for bridge
Expand Down Expand Up @@ -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<String> = 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);
Expand All @@ -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,
});
Expand All @@ -133,9 +145,13 @@ 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 {
if !self.info.network.internal {
setup_ipv4_fw_sysctl()?;
if data.ipam.ipv6_enabled {
setup_ipv6_fw_sysctl()?;
}
}
}

let (host_sock, netns_sock) = netlink_sockets;
Expand Down Expand Up @@ -221,37 +237,39 @@ 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))
}

fn teardown(
&self,
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
) -> NetavarkResult<()> {
let mode: Option<String> = 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();
Expand All @@ -268,6 +286,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,
) {
Expand All @@ -278,20 +297,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));
}
Expand Down Expand Up @@ -547,6 +561,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,
Expand Down Expand Up @@ -700,41 +720,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")?;
}
}
}
}
Expand Down Expand Up @@ -827,6 +850,7 @@ fn check_link_is_vrf(msg: LinkMessage, vrf_name: &str) -> NetavarkResult<LinkMes
fn remove_link(
host: &mut netlink::Socket,
netns: &mut netlink::Socket,
mode: BridgeMode,
br_name: &str,
container_veth_name: &str,
) -> NetavarkResult<bool> {
Expand All @@ -845,10 +869,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)
}
Expand All @@ -863,3 +889,14 @@ fn get_isolate_option(opts: &Option<HashMap<String, String>>) -> NetavarkResult<
_ => IsolateOption::Never,
})
}

fn get_bridge_mode_from_string(mode: Option<&str>) -> NetavarkResult<BridgeMode> {
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}\""
))),
}
}
42 changes: 42 additions & 0 deletions test/620-bridge-mode.bats
View file
Original file line number Diff line number Diff line change
@@ -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"
}

32 changes: 32 additions & 0 deletions test/testfiles/bridge-managed.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
26 changes: 26 additions & 0 deletions test/testfiles/bridge-unmanaged.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}

0 comments on commit d5358e4

Please sign in to comment.