Skip to content

Commit 551d37f

Browse files
committed
type safety: make Socket aware of namespace
Using a PhantomData and zero sized struct we can attach an additional generic data for either the host or container namespace to the struct. Because it is zero sized and not used it is optimized away and only the type checker sees it to enforce the right types are used. With that we basically create two different netlink socket types Socket<HostNS> and Socket<ContainerNS> so they must be used in all type signatures from now on. To keep the changes smaller I have set HostNS as default generic for the struct so we don't need to change most function signatures. For al call sides where we pass sockets around the compiler now enforces that we use the right ones avoid any possible mix ups. Signed-off-by: Paul Holzinger <[email protected]>
1 parent 6b44b7e commit 551d37f

File tree

10 files changed

+82
-34
lines changed

10 files changed

+82
-34
lines changed

src/commands/setup.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,11 @@ impl Setup {
160160
}
161161
}
162162

163-
fn teardown_drivers<'a, I>(drivers: I, host: &mut netlink::Socket, netns: &mut netlink::Socket)
164-
where
163+
fn teardown_drivers<'a, I>(
164+
drivers: I,
165+
host: &mut netlink::Socket,
166+
netns: &mut netlink::Socket<netlink::ContainerNS>,
167+
) where
165168
I: Iterator<Item = &'a Box<dyn NetworkDriver + 'a>>,
166169
{
167170
for driver in drivers {

src/dhcp_proxy/ip.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ trait Address<T> {
3434
fn new(l: &Lease, interface: &str) -> Result<Self, ProxyError>
3535
where
3636
Self: Sized;
37-
fn add_ip(&self, nls: &mut Socket) -> Result<(), ProxyError>;
38-
fn add_gws(&self, nls: &mut Socket) -> Result<(), ProxyError>;
37+
fn add_ip(&self, nls: &mut Socket<netlink::ContainerNS>) -> Result<(), ProxyError>;
38+
fn add_gws(&self, nls: &mut Socket<netlink::ContainerNS>) -> Result<(), ProxyError>;
3939
}
4040

4141
fn handle_gws(g: Vec<String>, netmask: &str) -> Result<Vec<IpNet>, ProxyError> {
@@ -112,7 +112,7 @@ impl Address<Ipv4Addr> for MacVLAN {
112112
}
113113

114114
// add the ip address to the container namespace
115-
fn add_ip(&self, nls: &mut Socket) -> Result<(), ProxyError> {
115+
fn add_ip(&self, nls: &mut netlink::Socket<netlink::ContainerNS>) -> Result<(), ProxyError> {
116116
debug!("adding network information for {}", self.interface);
117117
let ip = IpNet::new(self.address, self.prefix_length)?;
118118
let dev = nls.get_link(netlink::LinkID::Name(self.interface.clone()))?;
@@ -123,7 +123,7 @@ impl Address<Ipv4Addr> for MacVLAN {
123123
}
124124

125125
// add one or more routes to the container namespace
126-
fn add_gws(&self, nls: &mut Socket) -> Result<(), ProxyError> {
126+
fn add_gws(&self, nls: &mut Socket<netlink::ContainerNS>) -> Result<(), ProxyError> {
127127
debug!("adding gateways to {}", self.interface);
128128
match core_utils::add_default_routes(nls, &self.gateways, None) {
129129
Ok(_) => Ok(()),

src/network/bridge.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ impl driver::NetworkDriver for Bridge<'_> {
147147

148148
fn setup(
149149
&self,
150-
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
150+
netlink_sockets: (
151+
&mut netlink::Socket,
152+
&mut netlink::Socket<netlink::ContainerNS>,
153+
),
151154
) -> NetavarkResult<(StatusBlock, Option<AardvarkEntry>)> {
152155
let data = match &self.data {
153156
Some(d) => d,
@@ -302,7 +305,10 @@ impl driver::NetworkDriver for Bridge<'_> {
302305

303306
fn teardown(
304307
&self,
305-
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
308+
netlink_sockets: (
309+
&mut netlink::Socket,
310+
&mut netlink::Socket<netlink::ContainerNS>,
311+
),
306312
) -> NetavarkResult<()> {
307313
let mode: Option<String> = parse_option(&self.info.network.options, OPTION_MODE)?;
308314
let mode = get_bridge_mode_from_string(mode.as_deref())?;
@@ -547,7 +553,7 @@ const IPV6_FORWARD: &str = "net/ipv6/conf/all/forwarding";
547553
/// returns the container veth mac address
548554
fn create_interfaces(
549555
host: &mut netlink::Socket,
550-
netns: &mut netlink::Socket,
556+
netns: &mut netlink::Socket<netlink::ContainerNS>,
551557
data: &InternalData,
552558
internal: bool,
553559
rootless: bool,
@@ -745,7 +751,7 @@ fn create_interfaces(
745751
#[allow(clippy::too_many_arguments)]
746752
fn create_veth_pair<'fd>(
747753
host: &mut netlink::Socket,
748-
netns: &mut netlink::Socket,
754+
netns: &mut netlink::Socket<netlink::ContainerNS>,
749755
data: &InternalData,
750756
primary_index: u32,
751757
bridge_mac: Option<Vec<u8>>,
@@ -992,7 +998,7 @@ fn check_link_is_vrf(msg: LinkMessage, vrf_name: &str) -> NetavarkResult<LinkMes
992998

993999
fn remove_link(
9941000
host: &mut netlink::Socket,
995-
netns: &mut netlink::Socket,
1001+
netns: &mut netlink::Socket<netlink::ContainerNS>,
9961002
mode: BridgeMode,
9971003
br_name: &str,
9981004
container_veth_name: &str,

src/network/core_utils.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -270,24 +270,27 @@ macro_rules! exec_netns {
270270
}};
271271
}
272272

273-
pub struct NamespaceOptions {
273+
pub struct NamespaceOptions<N: netlink::Namespace> {
274274
/// Note we have to return the File object since the fd is only valid
275275
/// as long as the File object is valid
276276
pub file: File,
277-
pub netlink: netlink::Socket,
277+
pub netlink: netlink::Socket<N>,
278278
}
279279

280280
pub fn open_netlink_sockets(
281281
netns_path: &str,
282-
) -> NetavarkResult<(NamespaceOptions, NamespaceOptions)> {
282+
) -> NetavarkResult<(
283+
NamespaceOptions<netlink::HostNS>,
284+
NamespaceOptions<netlink::ContainerNS>,
285+
)> {
283286
let netns = open_netlink_socket(netns_path).wrap("open container netns")?;
284287
let hostns = open_netlink_socket("/proc/self/ns/net").wrap("open host netns")?;
285288

286-
let host_socket = netlink::Socket::new().wrap("host netlink socket")?;
289+
let host_socket = netlink::Socket::<netlink::HostNS>::new().wrap("host netlink socket")?;
287290
let netns_sock = exec_netns!(
288291
hostns.as_fd(),
289292
netns.as_fd(),
290-
netlink::Socket::new().wrap("netns netlink socket")
293+
netlink::Socket::<netlink::ContainerNS>::new().wrap("netns netlink socket")
291294
)?;
292295

293296
Ok((
@@ -307,7 +310,7 @@ fn open_netlink_socket(netns_path: &str) -> NetavarkResult<File> {
307310
}
308311

309312
pub fn add_default_routes(
310-
sock: &mut netlink::Socket,
313+
sock: &mut netlink::Socket<netlink::ContainerNS>,
311314
gws: &[ipnet::IpNet],
312315
metric: Option<u32>,
313316
) -> NetavarkResult<()> {

src/network/dhcp.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ pub fn release_dhcp_lease(
159159
Ok(())
160160
}
161161

162-
pub fn dhcp_teardown(info: &DriverInfo, sock: &mut netlink::Socket) -> NetavarkResult<()> {
162+
pub fn dhcp_teardown(
163+
info: &DriverInfo,
164+
sock: &mut netlink::Socket<netlink::ContainerNS>,
165+
) -> NetavarkResult<()> {
163166
let ipam = core_utils::get_ipam_addresses(info.per_network_opts, info.network)?;
164167
let if_name = info.per_network_opts.interface_name.clone();
165168

src/network/driver.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,18 @@ pub trait NetworkDriver {
3838
/// setup the network interfaces/firewall rules for this driver
3939
fn setup(
4040
&self,
41-
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
41+
netlink_sockets: (
42+
&mut netlink::Socket,
43+
&mut netlink::Socket<netlink::ContainerNS>,
44+
),
4245
) -> NetavarkResult<(StatusBlock, Option<AardvarkEntry>)>;
4346
/// teardown the network interfaces/firewall rules for this driver
4447
fn teardown(
4548
&self,
46-
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
49+
netlink_sockets: (
50+
&mut netlink::Socket,
51+
&mut netlink::Socket<netlink::ContainerNS>,
52+
),
4753
) -> NetavarkResult<()>;
4854

4955
/// return the network name

src/network/netlink.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,22 @@ use netlink_packet_route::{
2424
};
2525
use netlink_sys::{protocols::NETLINK_ROUTE, SocketAddr};
2626

27-
pub struct Socket {
27+
/// Marker trait for the Socket so we know it refers to either HostNS or ContainerNS.
28+
pub trait Namespace {}
29+
pub struct HostNS;
30+
pub struct ContainerNS;
31+
impl Namespace for HostNS {}
32+
impl Namespace for ContainerNS {}
33+
34+
pub struct Socket<N: Namespace = HostNS> {
2835
socket: netlink_sys::Socket,
2936
sequence_number: u32,
3037
/// buffer size for reading netlink messages, see NLMSG_GOODSIZE in the kernel
3138
buffer: [u8; 8192],
39+
40+
/// Marker for host or container namespace, allows functions to specify which netns
41+
/// socket they need which then gets enforced at compile time without any runtime overhead.
42+
_marker: std::marker::PhantomData<N>,
3243
}
3344

3445
#[derive(Clone)]
@@ -110,8 +121,8 @@ macro_rules! function {
110121
}};
111122
}
112123

113-
impl Socket {
114-
pub fn new() -> NetavarkResult<Socket> {
124+
impl<N: Namespace> Socket<N> {
125+
pub fn new() -> NetavarkResult<Socket<N>> {
115126
let mut socket = wrap!(netlink_sys::Socket::new(NETLINK_ROUTE), "open")?;
116127
let addr = &SocketAddr::new(0, 0);
117128
wrap!(socket.bind(addr), "bind")?;
@@ -121,6 +132,7 @@ impl Socket {
121132
socket,
122133
sequence_number: 0,
123134
buffer: [0; 8192],
135+
_marker: std::marker::PhantomData,
124136
})
125137
}
126138

src/network/plugin.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ impl NetworkDriver for PluginDriver<'_> {
3636

3737
fn setup(
3838
&self,
39-
_netlink_sockets: (&mut super::netlink::Socket, &mut super::netlink::Socket),
39+
_netlink_sockets: (
40+
&mut super::netlink::Socket,
41+
&mut super::netlink::Socket<super::netlink::ContainerNS>,
42+
),
4043
) -> NetavarkResult<(types::StatusBlock, Option<AardvarkEntry>)> {
4144
let result = self.exec_plugin(true, self.info.netns_path).wrap(format!(
4245
"plugin {:?} failed",
@@ -49,7 +52,10 @@ impl NetworkDriver for PluginDriver<'_> {
4952

5053
fn teardown(
5154
&self,
52-
_netlink_sockets: (&mut super::netlink::Socket, &mut super::netlink::Socket),
55+
_netlink_sockets: (
56+
&mut super::netlink::Socket,
57+
&mut super::netlink::Socket<super::netlink::ContainerNS>,
58+
),
5359
) -> NetavarkResult<()> {
5460
self.exec_plugin(false, self.info.netns_path).wrap(format!(
5561
"plugin {:?} failed",

src/network/vlan.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,10 @@ impl driver::NetworkDriver for Vlan<'_> {
145145

146146
fn setup(
147147
&self,
148-
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
148+
netlink_sockets: (
149+
&mut netlink::Socket,
150+
&mut netlink::Socket<netlink::ContainerNS>,
151+
),
149152
) -> Result<(StatusBlock, Option<AardvarkEntry>), NetavarkError> {
150153
let data = match &self.data {
151154
Some(d) => d,
@@ -218,7 +221,10 @@ impl driver::NetworkDriver for Vlan<'_> {
218221

219222
fn teardown(
220223
&self,
221-
netlink_sockets: (&mut netlink::Socket, &mut netlink::Socket),
224+
netlink_sockets: (
225+
&mut netlink::Socket,
226+
&mut netlink::Socket<netlink::ContainerNS>,
227+
),
222228
) -> NetavarkResult<()> {
223229
dhcp_teardown(&self.info, netlink_sockets.1)?;
224230

@@ -236,7 +242,7 @@ impl driver::NetworkDriver for Vlan<'_> {
236242

237243
fn setup(
238244
host: &mut netlink::Socket,
239-
netns: &mut netlink::Socket,
245+
netns: &mut netlink::Socket<netlink::ContainerNS>,
240246
if_name: &str,
241247
data: &InternalData,
242248
hostns_fd: BorrowedFd<'_>,

src/test/netlink.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,16 @@ mod tests {
2828
#[test]
2929
fn test_socket_new() {
3030
test_setup!();
31-
assert!(Socket::new().is_ok(), "Netlink Socket::new() should work");
31+
assert!(
32+
Socket::<HostNS>::new().is_ok(),
33+
"Netlink Socket::<HostNS>::new() should work"
34+
);
3235
}
3336

3437
#[test]
3538
fn test_add_link() {
3639
test_setup!();
37-
let mut sock = Socket::new().expect("Socket::new()");
40+
let mut sock = Socket::<HostNS>::new().expect("Socket::<HostNS>::new()");
3841

3942
let name = String::from("test1");
4043
sock.create_link(CreateLinkOptions::new(name.clone(), InfoKind::Dummy))
@@ -49,7 +52,7 @@ mod tests {
4952
#[test]
5053
fn test_add_addr() {
5154
test_setup!();
52-
let mut sock = Socket::new().expect("Socket::new()");
55+
let mut sock = Socket::<HostNS>::new().expect("Socket::<HostNS>::new()");
5356

5457
let out = run_command!("ip", "link", "add", "test1", "type", "dummy");
5558
eprintln!("{}", String::from_utf8(out.stderr).unwrap());
@@ -72,7 +75,7 @@ mod tests {
7275
#[test]
7376
fn test_del_addr() {
7477
test_setup!();
75-
let mut sock = Socket::new().expect("Socket::new()");
78+
let mut sock = Socket::<HostNS>::new().expect("Socket::<HostNS>::new()");
7679

7780
let out = run_command!("ip", "link", "add", "test1", "type", "dummy");
7881
eprintln!("{}", String::from_utf8(out.stderr).unwrap());
@@ -110,7 +113,7 @@ mod tests {
110113
#[ignore]
111114
fn test_del_route() {
112115
test_setup!();
113-
let mut sock = Socket::new().expect("Socket::new()");
116+
let mut sock = Socket::<HostNS>::new().expect("Socket::<HostNS>::new()");
114117

115118
let out = run_command!("ip", "link", "add", "test1", "type", "dummy");
116119
eprintln!("{}", String::from_utf8(out.stderr).unwrap());
@@ -159,7 +162,7 @@ mod tests {
159162
#[test]
160163
fn test_dump_addr() {
161164
test_setup!();
162-
let mut sock = Socket::new().expect("Socket::new()");
165+
let mut sock = Socket::<HostNS>::new().expect("Socket::<HostNS>::new()");
163166

164167
let out = run_command!("ip", "link", "add", "test1", "type", "dummy");
165168
eprintln!("{}", String::from_utf8(out.stderr).unwrap());

0 commit comments

Comments
 (0)