Merge pull request #1161 from Luap99/vlan

bridge: add vlan support

src/network/bridge.rs

@@ -3,7 +3,8 @@ use std::{collections::HashMap, net::IpAddr, os::fd::BorrowedFd, sync::Once};
 use ipnet::IpNet;
 use log::{debug, error};
 use netlink_packet_route::link::{
-    InfoData, InfoKind, InfoVeth, LinkAttribute, LinkInfo, LinkMessage,
+    BridgeVlanInfoFlags, InfoBridge, InfoData, InfoKind, InfoVeth, LinkAttribute, LinkInfo,
+    LinkMessage,
 };
 
 use crate::{
@@ -21,7 +22,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_MODE, OPTION_MTU, OPTION_NO_DEFAULT_ROUTE, OPTION_VRF,
+        OPTION_MODE, OPTION_MTU, OPTION_NO_DEFAULT_ROUTE, OPTION_VLAN, OPTION_VRF,
     },
     core_utils::{self, get_ipam_addresses, join_netns, parse_option, CoreUtils},
     driver::{self, DriverInfo},
@@ -66,7 +67,8 @@ struct InternalData {
     no_default_route: bool,
     /// sef vrf for bridge
     vrf: Option<String>,
-    // TODO: add vlan
+    /// vlan id of the interface attached to the bridge
+    vlan: Option<u16>,
 }
 
 pub struct Bridge<'a> {
@@ -99,6 +101,7 @@ impl driver::NetworkDriver for Bridge<'_> {
         let no_default_route: bool =
             parse_option(&self.info.network.options, OPTION_NO_DEFAULT_ROUTE)?.unwrap_or(false);
         let vrf: Option<String> = parse_option(&self.info.network.options, OPTION_VRF)?;
+        let vlan: Option<u16> = parse_option(&self.info.network.options, OPTION_VLAN)?;
         let host_interface_name = parse_option(
             &self.info.per_network_opts.options,
             OPTION_HOST_INTERFACE_NAME,
@@ -122,6 +125,7 @@ impl driver::NetworkDriver for Bridge<'_> {
             mode: get_bridge_mode_from_string(mode.as_deref())?,
             no_default_route,
             vrf,
+            vlan,
         });
         Ok(())
     }
@@ -549,9 +553,12 @@ fn create_interfaces(
         data.bridge_interface_name.to_string(),
     )) {
         Ok(bridge) => (
-            check_link_is_bridge(bridge, &data.bridge_interface_name)?
-                .header
-                .index,
+            validate_bridge_link(
+                bridge,
+                data.vlan.is_some(),
+                host,
+                &data.bridge_interface_name,
+            )?,
             None,
         ),
         Err(err) => match err.unwrap() {
@@ -573,6 +580,11 @@ fn create_interfaces(
                 );
                 create_link_opts.mtu = data.mtu;
 
+                if data.vlan.is_some() {
+                    create_link_opts.info_data =
+                        Some(InfoData::Bridge(vec![InfoBridge::VlanFiltering(true)]));
+                }
+
                 if let Some(vrf_name) = &data.vrf {
                     let vrf = match host.get_link(netlink::LinkID::Name(vrf_name.to_string())) {
                         Ok(vrf) => check_link_is_vrf(vrf, vrf_name)?,
@@ -720,6 +732,14 @@ fn create_veth_pair<'fd>(
         ));
     }
 
+    if let Some(vid) = data.vlan {
+        host.set_vlan_id(
+            host_link,
+            vid,
+            BridgeVlanInfoFlags::Pvid | BridgeVlanInfoFlags::Untagged,
+        )?;
+    }
+
     if let BridgeMode::Managed = data.mode {
         exec_netns!(hostns_fd, netns_fd, res, {
             disable_ipv6_autoconf(&data.container_interface_name)?;
@@ -801,14 +821,54 @@ fn create_veth_pair<'fd>(
     Ok(mac)
 }
 
-/// make sure the LinkMessage has the kind bridge
-fn check_link_is_bridge(msg: LinkMessage, br_name: &str) -> NetavarkResult<LinkMessage> {
+/// Make sure the LinkMessage is of type bridge and if vlan is set also checks
+/// that the bridge has vlan_filtering enabled and if not enables it. Returns
+/// the link id or errors when the link is not a bridge.
+fn validate_bridge_link(
+    msg: LinkMessage,
+    vlan: bool,
+    netlink: &mut netlink::Socket,
+    br_name: &str,
+) -> NetavarkResult<u32> {
     for nla in msg.attributes.iter() {
         if let LinkAttribute::LinkInfo(info) = nla {
+            // when vlan is requested also check the VlanFiltering attribute
+            if vlan {
+                for inf in info.iter() {
+                    if let LinkInfo::Data(data) = inf {
+                        match data {
+                            InfoData::Bridge(vec) => {
+                                // set the return value here based on the VlanFiltering state
+                                let vlan_enabled = vec
+                                    .iter()
+                                    .find_map(|a| {
+                                        if let InfoBridge::VlanFiltering(on) = a {
+                                            Some(*on)
+                                        } else {
+                                            None
+                                        }
+                                    })
+                                    .unwrap_or(false);
+                                if !vlan_enabled {
+                                    // vlan filtering not enabled, enable it now
+                                    netlink.set_vlan_filtering(msg.header.index, true)?;
+                                }
+                            }
+                            _ => {
+                                return Err(NetavarkError::Message(format!(
+                                    "bridge interface {br_name} doesn't contain any bridge data",
+                                )))
+                            }
+                        }
+                        break;
+                    }
+                }
+            }
+
             for inf in info.iter() {
                 if let LinkInfo::Kind(kind) = inf {
                     if *kind == InfoKind::Bridge {
-                        return Ok(msg);
+                        return Ok(msg.header.index);
                     } else {
                         return Err(NetavarkError::Message(format!(
                             "bridge interface {br_name} already exists but is a {kind:?} interface"

src/network/constants.rs

@@ -22,6 +22,7 @@ pub const OPTION_METRIC: &str = "metric";
 pub const OPTION_NO_DEFAULT_ROUTE: &str = "no_default_route";
 pub const OPTION_BCLIM: &str = "bclim";
 pub const OPTION_VRF: &str = "vrf";
+pub const OPTION_VLAN: &str = "vlan";
 pub const OPTION_HOST_INTERFACE_NAME: &str = "host_interface_name";
 
 /// 100 is the default metric for most Linux networking tools.

src/network/netlink.rs

@@ -15,7 +15,10 @@ use netlink_packet_core::{
 };
 use netlink_packet_route::{
     address::AddressMessage,
-    link::{InfoData, InfoKind, LinkAttribute, LinkFlags, LinkInfo, LinkMessage},
+    link::{
+        AfSpecBridge, BridgeVlanInfo, BridgeVlanInfoFlags, InfoBridge, InfoData, InfoKind,
+        LinkAttribute, LinkFlags, LinkInfo, LinkMessage,
+    },
     route::{RouteAddress, RouteMessage, RouteProtocol, RouteScope, RouteType},
     AddressFamily, RouteNetlinkMessage,
 };
@@ -186,6 +189,48 @@ impl Socket {
         Ok(())
     }
 
+    /// set the vlan_filtering attribute on a bridge
+    pub fn set_vlan_filtering(&mut self, link_id: u32, vlan_filtering: bool) -> NetavarkResult<()> {
+        let mut msg = LinkMessage::default();
+        msg.header.index = link_id;
+        msg.attributes.push(LinkAttribute::LinkInfo(vec![
+            LinkInfo::Kind(InfoKind::Bridge),
+            LinkInfo::Data(InfoData::Bridge(vec![InfoBridge::VlanFiltering(
+                vlan_filtering,
+            )])),
+        ]));
+
+        // Now idea why this must use NewLink not SetLink, I strace'd ip route
+        // and they use newlink and which setlink here it does not error but also does not set the setting.
+        let result = self.make_netlink_request(RouteNetlinkMessage::NewLink(msg), NLM_F_ACK)?;
+        expect_netlink_result!(result, 0);
+        Ok(())
+    }
+
+    /// set the vlan id for an interface which is attached to the bridge with vlan_filtering
+    /// Performs the equivalent of "bridge vlan add dev test vid <num> [flags]"
+    pub fn set_vlan_id(
+        &mut self,
+        link_id: u32,
+        // vlan id
+        vid: u16,
+        // flags for the vlan config
+        flags: BridgeVlanInfoFlags,
+    ) -> NetavarkResult<()> {
+        let mut msg = LinkMessage::default();
+        msg.header.interface_family = AddressFamily::Bridge;
+        // msg.header.link_layer_type = LinkLayerType::Netrom;
+        msg.header.index = link_id;
+        msg.attributes
+            .push(LinkAttribute::AfSpecBridge(vec![AfSpecBridge::VlanInfo(
+                BridgeVlanInfo { flags, vid },
+            )]));
+
+        let result = self.make_netlink_request(RouteNetlinkMessage::SetLink(msg), NLM_F_ACK)?;
+        expect_netlink_result!(result, 0);
+        Ok(())
+    }
+
     fn create_addr_msg(link_id: u32, addr: &ipnet::IpNet) -> AddressMessage {
         let mut msg = AddressMessage::default();
         msg.header.index = link_id;

test/630-bridge-vlan.bats

@@ -0,0 +1,142 @@
+#!/usr/bin/env bats   -*- bats -*-
+#
+# bridge driver tests with explicit modes
+#
+
+load helpers
+
+# accepts the mode (managed/unmanaged) as first arg and
+# as second arg the vlan id (int)
+function createVlanConfig() {
+    local mode=$1
+    local vlan=$2
+
+    read -r -d '\0' config <<EOF
+{
+  "container_id": "6ce776ea58b5",
+  "container_name": "testcontainer",
+  "networks": {
+    "podman1": {
+      "static_ips": [
+        "10.88.0.2"
+      ],
+      "interface_name": "eth0"
+    }
+  },
+  "network_info": {
+    "podman1": {
+      "name": "podman0",
+      "id": "ed82e3a703682a9c09629d3cf45c1f1e7da5b32aeff3faf82837ef4d005356e6",
+      "driver": "bridge",
+      "network_interface": "podman0",
+      "subnets": [
+        {
+          "gateway": "10.88.0.1",
+          "subnet": "10.88.0.0/16"
+        }
+      ],
+      "ipv6_enabled": true,
+      "internal": false,
+      "dns_enabled": false,
+      "ipam_options": {
+        "driver": "host-local"
+      },
+      "options": {
+        "mode": "$mode",
+        "vlan": "$vlan"
+      }
+    }
+  }
+}\0
+EOF
+
+echo "$config"
+}
+
+@test "bridge - vlan create bridge" {
+    local vlan=20
+    local config=$(createVlanConfig managed $vlan)
+
+    run_netavark setup $(get_container_netns_path) <<<"$config"
+
+    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"
+    assert_json "$link_info" '.[].linkinfo.info_data.vlan_filtering' == "1" "vlan_filtering enabled on the bridge"
+
+    run_in_host_netns bridge -j vlan show
+    vlan_info="$output"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[0].vlan' == "1" "default vlan 1 connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].vlan' == "$vlan" "vlan connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].flags' == '[
+  "PVID",
+  "Egress Untagged"
+]' "vlan flags"
+
+    run_netavark teardown $(get_container_netns_path) <<<"$config"
+}
+
+@test "bridge - vlan with existing bridge" {
+    local vlan=99
+    local config=$(createVlanConfig unmanaged $vlan)
+
+    # pre create bridge in managed mode to ensure to code still enabled vlan_filtering
+    run_in_host_netns ip link add podman0 type bridge
+    run_in_host_netns ip link set up podman0
+
+    run_netavark setup $(get_container_netns_path) <<<"$config"
+
+    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"
+    assert_json "$link_info" '.[].linkinfo.info_data.vlan_filtering' == "1" "vlan_filtering enabled on the bridge"
+
+    run_in_host_netns bridge -j vlan show
+    vlan_info="$output"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[0].vlan' == "1" "default vlan 1 connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].vlan' == "$vlan" "vlan connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].flags' == '[
+  "PVID",
+  "Egress Untagged"
+]' "vlan flags"
+
+    run_netavark teardown $(get_container_netns_path) <<<"$config"
+}
+
+@test "bridge - two vlan's on same bridge" {
+    local vlan1=10
+    local config1=$(createVlanConfig managed $vlan1)
+
+    run_netavark setup $(get_container_netns_path) <<<"$config1"
+
+    run_in_host_netns bridge -j vlan show
+    vlan_info="$output"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[0].vlan' == "1" "default vlan 1 connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].vlan' == "$vlan1" "vlan1 connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].flags' == '[
+  "PVID",
+  "Egress Untagged"
+]' "vlan flags"
+
+
+    local vlan2=10
+    local config2=$(createVlanConfig unmanaged $vlan2)
+    create_container_ns
+
+    run_netavark setup $(get_container_netns_path 1) <<<"$config2"
+
+    # the second setup should have used veth1 as name
+
+    run_in_host_netns bridge -j vlan show
+    vlan_info="$output"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth0") | .vlans[1].vlan' == "$vlan1" "vlan1 connected"
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth1") | .vlans[1].vlan' == "$vlan2" "vlan2 connected"
+
+    assert_json "$vlan_info" '.[] | select(.ifname=="veth1") | .vlans[1].flags' == '[
+  "PVID",
+  "Egress Untagged"
+]' "vlan flags"
+
+    run_netavark teardown $(get_container_netns_path) <<<"$config1"
+    run_netavark teardown $(get_container_netns_path 1) <<<"$config2"
+}