From 0789f447433375bee5b79666f29012527c2f36ac Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 1 Nov 2022 15:12:40 -0400 Subject: [PATCH] Add support for route metrics Metrics allow us to specify which default route will be preferred when a container connects to multiple networks. By default, all routes will get a metric of 100. If two routes go to the same location (e.g. multiple default routes, one for each network, as Netavark does by default), the kernel by default round-robins traffic between all of them, but if the metrics for the routes differ, the route(s) with the lowest metric are chosen. For confusing reasons, metric is actually called preference in Netlink - and, doubly confusing, there is also a "metrics" option for routes in Netlink as well. Which, naturally, is completely unrelated to route metric. Sigh. Signed-off-by: Matthew Heon --- src/network/bridge.rs | 8 +++++-- src/network/constants.rs | 4 ++++ src/network/core_utils.rs | 8 ++++++- src/network/macvlan.rs | 8 +++++-- src/network/netlink.rs | 43 +++++++++++++++++++++++++++-------- src/test/netlink.rs | 1 + test/100-bridge-iptables.bats | 28 +++++++++++++++++++++++ test/testfiles/metric.json | 37 ++++++++++++++++++++++++++++++ 8 files changed, 122 insertions(+), 15 deletions(-) create mode 100644 test/testfiles/metric.json diff --git a/src/network/bridge.rs b/src/network/bridge.rs index ab00af401..55501c086 100644 --- a/src/network/bridge.rs +++ b/src/network/bridge.rs @@ -16,7 +16,7 @@ use crate::{ }; use super::{ - constants::{NO_CONTAINER_INTERFACE_ERROR, OPTION_ISOLATE, OPTION_MTU}, + constants::{NO_CONTAINER_INTERFACE_ERROR, OPTION_ISOLATE, OPTION_METRIC, OPTION_MTU}, core_utils::{self, get_ipam_addresses, join_netns, parse_option, CoreUtils}, driver::{self, DriverInfo}, internal_types::{ @@ -41,6 +41,8 @@ struct InternalData { mtu: u32, /// if this network should be isolated from others isolate: bool, + /// Route metric for any default routes added for the network + metric: Option, // TODO: add vlan } @@ -69,6 +71,7 @@ impl driver::NetworkDriver for Bridge<'_> { let mtu: u32 = parse_option(&self.info.network.options, OPTION_MTU, 0)?; let isolate: bool = parse_option(&self.info.network.options, OPTION_ISOLATE, false)?; + let metric: u32 = parse_option(&self.info.network.options, OPTION_METRIC, 100)?; let static_mac = match &self.info.per_network_opts.static_mac { Some(mac) => Some(CoreUtils::decode_address_from_hex(mac)?), @@ -82,6 +85,7 @@ impl driver::NetworkDriver for Bridge<'_> { ipam, mtu, isolate, + metric: Some(metric), }); Ok(()) } @@ -620,7 +624,7 @@ fn create_veth_pair( .wrap("set container veth up")?; if !internal { - core_utils::add_default_routes(netns, &data.ipam.gateway_addresses)?; + core_utils::add_default_routes(netns, &data.ipam.gateway_addresses, data.metric)?; } Ok(mac) diff --git a/src/network/constants.rs b/src/network/constants.rs index ff943c826..30a7394be 100644 --- a/src/network/constants.rs +++ b/src/network/constants.rs @@ -22,5 +22,9 @@ pub const DRIVER_MACVLAN: &str = "macvlan"; pub const OPTION_ISOLATE: &str = "isolate"; pub const OPTION_MTU: &str = "mtu"; pub const OPTION_MODE: &str = "mode"; +pub const OPTION_METRIC: &str = "metric"; + +// 100 is the default metric for most Linux networking tools. +pub const DEFAULT_METRIC: u32 = 100; pub const NO_CONTAINER_INTERFACE_ERROR: &str = "no container interface name given"; diff --git a/src/network/core_utils.rs b/src/network/core_utils.rs index ba8527002..8ca39d3fb 100644 --- a/src/network/core_utils.rs +++ b/src/network/core_utils.rs @@ -309,7 +309,11 @@ fn open_netlink_socket(netns_path: &str) -> NetavarkResult<(File, RawFd)> { Ok((ns, ns_fd)) } -pub fn add_default_routes(sock: &mut netlink::Socket, gws: &[ipnet::IpNet]) -> NetavarkResult<()> { +pub fn add_default_routes( + sock: &mut netlink::Socket, + gws: &[ipnet::IpNet], + metric: Option, +) -> NetavarkResult<()> { let mut ipv4 = false; let mut ipv6 = false; for addr in gws { @@ -323,6 +327,7 @@ pub fn add_default_routes(sock: &mut netlink::Socket, gws: &[ipnet::IpNet]) -> N netlink::Route::Ipv4 { dest: ipnet::Ipv4Net::new(Ipv4Addr::new(0, 0, 0, 0), 0)?, gw: v4.addr(), + metric, } } ipnet::IpNet::V6(v6) => { @@ -334,6 +339,7 @@ pub fn add_default_routes(sock: &mut netlink::Socket, gws: &[ipnet::IpNet]) -> N netlink::Route::Ipv6 { dest: ipnet::Ipv6Net::new(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0), 0)?, gw: v6.addr(), + metric, } } }; diff --git a/src/network/macvlan.rs b/src/network/macvlan.rs index 2d2dd96a2..0e9b3f2db 100644 --- a/src/network/macvlan.rs +++ b/src/network/macvlan.rs @@ -12,7 +12,7 @@ use crate::{ }; use super::{ - constants::{NO_CONTAINER_INTERFACE_ERROR, OPTION_MODE, OPTION_MTU}, + constants::{NO_CONTAINER_INTERFACE_ERROR, OPTION_METRIC, OPTION_MODE, OPTION_MTU}, core_utils::{self, get_ipam_addresses, parse_option, CoreUtils}, driver::{self, DriverInfo}, internal_types::IPAMAddresses, @@ -31,6 +31,8 @@ struct InternalData { mtu: u32, /// macvlan mode macvlan_mode: u32, + /// Route metric for default routes added to the network + metric: Option, // TODO: add vlan } @@ -61,6 +63,7 @@ impl driver::NetworkDriver for MacVlan<'_> { let mut ipam = get_ipam_addresses(self.info.per_network_opts, self.info.network)?; let mtu = parse_option(&self.info.network.options, OPTION_MTU, 0)?; + let metric = parse_option(&self.info.network.options, OPTION_METRIC, 100)?; let static_mac = match &self.info.per_network_opts.static_mac { Some(mac) => Some(CoreUtils::decode_address_from_hex(mac)?), @@ -83,6 +86,7 @@ impl driver::NetworkDriver for MacVlan<'_> { ipam, macvlan_mode, mtu, + metric: Some(metric), }); Ok(()) } @@ -226,7 +230,7 @@ fn setup( .set_up(netlink::LinkID::ID(dev.header.index)) .wrap("set macvlan up")?; - core_utils::add_default_routes(netns, &data.ipam.gateway_addresses)?; + core_utils::add_default_routes(netns, &data.ipam.gateway_addresses, data.metric)?; for nla in dev.nlas.into_iter() { if let Nla::Address(ref addr) = nla { diff --git a/src/network/netlink.rs b/src/network/netlink.rs index 8f2951537..742a12c58 100644 --- a/src/network/netlink.rs +++ b/src/network/netlink.rs @@ -5,9 +5,10 @@ use std::{ use crate::{ error::{ErrorWrap, NetavarkError, NetavarkResult}, + network::constants, wrap, }; -use log::trace; +use log::{info, trace}; use netlink_packet_route::{ nlas::link::{Info, InfoData, InfoKind, Nla}, AddressMessage, LinkMessage, NetlinkHeader, NetlinkMessage, NetlinkPayload, RouteMessage, @@ -41,17 +42,33 @@ pub enum LinkID { } pub enum Route { - Ipv4 { dest: ipnet::Ipv4Net, gw: Ipv4Addr }, - Ipv6 { dest: ipnet::Ipv6Net, gw: Ipv6Addr }, + Ipv4 { + dest: ipnet::Ipv4Net, + gw: Ipv4Addr, + metric: Option, + }, + Ipv6 { + dest: ipnet::Ipv6Net, + gw: Ipv6Addr, + metric: Option, + }, } impl std::fmt::Display for Route { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let (dest, gw) = match self { - Route::Ipv4 { dest, gw } => (dest.to_string(), gw.to_string()), - Route::Ipv6 { dest, gw } => (dest.to_string(), gw.to_string()), + let (dest, gw, metric) = match self { + Route::Ipv4 { dest, gw, metric } => ( + dest.to_string(), + gw.to_string(), + metric.unwrap_or(constants::DEFAULT_METRIC), + ), + Route::Ipv6 { dest, gw, metric } => ( + dest.to_string(), + gw.to_string(), + metric.unwrap_or(constants::DEFAULT_METRIC), + ), }; - write!(f, "(dest: {} ,gw: {})", dest, gw) + write!(f, "(dest: {} ,gw: {}, metric {})", dest, gw, metric) } } @@ -220,21 +237,25 @@ impl Socket { msg.header.scope = RT_SCOPE_UNIVERSE; msg.header.kind = RTN_UNICAST; - let (dest_vec, dest_prefix, gateway_vec) = match route { - Route::Ipv4 { dest, gw } => { + info!("Adding route {}", route); + + let (dest_vec, dest_prefix, gateway_vec, final_metric) = match route { + Route::Ipv4 { dest, gw, metric } => { msg.header.address_family = AF_INET as u8; ( dest.addr().octets().to_vec(), dest.prefix_len(), gw.octets().to_vec(), + metric.unwrap_or(constants::DEFAULT_METRIC), ) } - Route::Ipv6 { dest, gw } => { + Route::Ipv6 { dest, gw, metric } => { msg.header.address_family = AF_INET6 as u8; ( dest.addr().octets().to_vec(), dest.prefix_len(), gw.octets().to_vec(), + metric.unwrap_or(constants::DEFAULT_METRIC), ) } }; @@ -244,6 +265,8 @@ impl Socket { .push(netlink_packet_route::route::Nla::Destination(dest_vec)); msg.nlas .push(netlink_packet_route::route::Nla::Gateway(gateway_vec)); + msg.nlas + .push(netlink_packet_route::route::Nla::Priority(final_metric)); msg } diff --git a/src/test/netlink.rs b/src/test/netlink.rs index a75ee076c..033d23039 100644 --- a/src/test/netlink.rs +++ b/src/test/netlink.rs @@ -142,6 +142,7 @@ mod tests { sock.del_route(&Route::Ipv4 { dest: net.parse().unwrap(), gw: gw.parse().unwrap(), + metric: None, }) .expect("del_route failed"); diff --git a/test/100-bridge-iptables.bats b/test/100-bridge-iptables.bats index cdffe3b46..4911d231c 100644 --- a/test/100-bridge-iptables.bats +++ b/test/100-bridge-iptables.bats @@ -666,3 +666,31 @@ EOF expected_rc=1 run_netavark --file ${TESTSDIR}/testfiles/ipv6-bridge.json setup $(get_container_netns_path) assert '{"error":"add ip addr to bridge: failed to add ipv6 address, is ipv6 enabled in the kernel?: Netlink error: Permission denied (os error 13)"}' "error message" } + +@test "$fw_driver - route metric from config" { + run_netavark --file ${TESTSDIR}/testfiles/metric.json setup $(get_container_netns_path) + + run_in_container_netns ip -j route list match 0.0.0.0 + default_route="$output" + assert_json "$default_route" '.[0].dst' == "default" "Default route was selected" + assert_json "$default_route" '.[0].metric' == "200" "Route metric set from config" + + run_in_container_netns ip -j -6 route list match ::0 + default_route_v6="$output" + assert_json "$default_route_v6" '.[0].dst' == "default" "Default route was selected" + assert_json "$default_route_v6" '.[0].metric' == "200" "v6 route metric matches v4" +} + +@test "$fw_drive - default route metric" { + run_netavark --file ${TESTSDIR}/testfiles/dualstack-bridge.json setup $(get_container_netns_path) + + run_in_container_netns ip -j route list match 0.0.0.0 + default_route="$output" + assert_json "$default_route" '.[0].dst' == "default" "Default route was selected" + assert_json "$default_route" '.[0].metric' == "100" "Default metric was chosen" + + run_in_container_netns ip -j -6 route list match ::0 + default_route_v6="$output" + assert_json "$default_route_v6" '.[0].dst' == "default" "Default route was selected" + assert_json "$default_route_v6" '.[0].metric' == "100" "v6 route metric matches v4" +} diff --git a/test/testfiles/metric.json b/test/testfiles/metric.json new file mode 100644 index 000000000..eee417c8d --- /dev/null +++ b/test/testfiles/metric.json @@ -0,0 +1,37 @@ +{ + "container_id": "bc14fe7cd3633e7be338522002bb0c3ccb18150da7a6c733735ffdf8ff7e85d1", + "container_name": "metrictest", + "networks": { + "metric": { + "interface_name": "eth1", + "static_ips": [ + "10.89.0.2", + "fde0::2" + ] + } + }, + "network_info": { + "metric": { + "dns_enabled": false, + "driver": "bridge", + "id": "7ba44a9a709f8093621eae1a1db2ccafc2471bae19cdf9dd2ea7cf3773b9211c", + "internal": false, + "ipv6_enabled": true, + "name": "metric", + "network_interface": "metric", + "subnets": [ + { + "gateway": "10.89.0.1", + "subnet": "10.89.0.0/24" + }, + { + "subnet": "fde0::/64", + "gateway": "fde0::1" + } + ], + "options": { + "metric": "200" + } + } + } +}