Skip to content

Commit

Permalink
Merge pull request containers#474 from mheon/metric
Browse files Browse the repository at this point in the history
Add support for route metrics
  • Loading branch information
flouthoc authored Nov 3, 2022
2 parents 787a9ce + 0789f44 commit 9cb258f
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 15 deletions.
8 changes: 6 additions & 2 deletions src/network/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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<u32>,
// TODO: add vlan
}

Expand Down Expand Up @@ -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)?),
Expand All @@ -82,6 +85,7 @@ impl driver::NetworkDriver for Bridge<'_> {
ipam,
mtu,
isolate,
metric: Some(metric),
});
Ok(())
}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/network/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
8 changes: 7 additions & 1 deletion src/network/core_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32>,
) -> NetavarkResult<()> {
let mut ipv4 = false;
let mut ipv6 = false;
for addr in gws {
Expand All @@ -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) => {
Expand All @@ -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,
}
}
};
Expand Down
8 changes: 6 additions & 2 deletions src/network/macvlan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -31,6 +31,8 @@ struct InternalData {
mtu: u32,
/// macvlan mode
macvlan_mode: u32,
/// Route metric for default routes added to the network
metric: Option<u32>,
// TODO: add vlan
}

Expand Down Expand Up @@ -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)?),
Expand All @@ -83,6 +86,7 @@ impl driver::NetworkDriver for MacVlan<'_> {
ipam,
macvlan_mode,
mtu,
metric: Some(metric),
});
Ok(())
}
Expand Down Expand Up @@ -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 {
Expand Down
43 changes: 33 additions & 10 deletions src/network/netlink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<u32>,
},
Ipv6 {
dest: ipnet::Ipv6Net,
gw: Ipv6Addr,
metric: Option<u32>,
},
}

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)
}
}

Expand Down Expand Up @@ -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),
)
}
};
Expand All @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions src/test/netlink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
28 changes: 28 additions & 0 deletions test/100-bridge-iptables.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
37 changes: 37 additions & 0 deletions test/testfiles/metric.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}

0 comments on commit 9cb258f

Please sign in to comment.