Skip to content

Add mtu for network connections #1101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
imobachgs merged 3 commits into from
Jun 11, 2024
Merged

Add mtu for network connections #1101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7e11e7
Add mtu to network model
jcronenberg Mar 15, 2024
83cf368
Add mtu network setting
jcronenberg May 16, 2024
26fc60d
Update changes
jcronenberg Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions rust/agama-lib/share/profile.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@
"description": "Custom mac-address (can also be 'preserve', 'permanent', 'random' or 'stable')",
"type": "string"
},
"mtu": {
"description": "Connection MTU",
"type": "integer",
"minimum": 0
},
"method4": {
"description": "IPv4 configuration method (e.g., 'auto')",
"type": "string",
Expand Down
4 changes: 4 additions & 0 deletions rust/agama-lib/src/network/proxies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ trait Connection {
fn mac_address(&self) -> zbus::Result<String>;
#[dbus_proxy(property)]
fn set_mac_address(&self, mac_address: &str) -> zbus::Result<()>;
#[dbus_proxy(property)]
fn mtu(&self) -> zbus::Result<u32>;
#[dbus_proxy(property)]
fn set_mtu(&self, mtu: u32) -> zbus::Result<()>;
}

#[dbus_proxy(
Expand Down
6 changes: 6 additions & 0 deletions rust/agama-lib/src/network/settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ pub struct NetworkConnection {
pub mac_address: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub status: Option<Status>,
#[serde(skip_serializing_if = "is_zero", default)]
pub mtu: u32,
}

fn is_zero(u: &u32) -> bool {
*u == 0
}

impl NetworkConnection {
Expand Down
5 changes: 5 additions & 0 deletions rust/agama-server/src/network/model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ pub struct Connection {
#[serde_as(as = "DisplayFromStr")]
pub mac_address: MacAddress,
pub firewall_zone: Option<String>,
pub mtu: u32,
pub ip_config: IpConfig,
pub status: Status,
pub interface: Option<String>,
Expand Down Expand Up @@ -551,6 +552,7 @@ impl Default for Connection {
uuid: Uuid::new_v4(),
mac_address: Default::default(),
firewall_zone: Default::default(),
mtu: Default::default(),
ip_config: Default::default(),
status: Default::default(),
interface: Default::default(),
Expand Down Expand Up @@ -598,6 +600,7 @@ impl TryFrom<NetworkConnection> for Connection {
connection.ip_config.gateway4 = conn.gateway4;
connection.ip_config.gateway6 = conn.gateway6;
connection.interface = conn.interface;
connection.mtu = conn.mtu;

Ok(connection)
}
Expand All @@ -618,6 +621,7 @@ impl TryFrom<Connection> for NetworkConnection {
let gateway6 = conn.ip_config.gateway6;
let interface = conn.interface;
let status = Some(conn.status);
let mtu = conn.mtu;

let mut connection = NetworkConnection {
id,
Expand All @@ -630,6 +634,7 @@ impl TryFrom<Connection> for NetworkConnection {
mac_address,
interface,
addresses,
mtu,
..Default::default()
};

Expand Down
64 changes: 50 additions & 14 deletions rust/agama-server/src/network/nm/dbus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,30 @@ pub fn connection_to_dbus<'a>(
result.insert("match", match_config_to_dbus(&conn.match_config));

if conn.is_ethernet() {
let ethernet_config = HashMap::from([(
"assigned-mac-address",
Value::new(conn.mac_address.to_string()),
)]);
let ethernet_config = HashMap::from([
(
"assigned-mac-address",
Copy link
Copy Markdown
Contributor

@teclator teclator Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we could do this assignation generally and not based in the specific connection type taking into account that in case of empty it could be removed later when the connection is cleaned up just to avoid the code duplication...

Copy link
Copy Markdown
Contributor Author

@jcronenberg jcronenberg Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like this?

    match &conn.config {
        ConnectionConfig::Wireless(wireless) => {
            connection_dbus.insert("type", WIRELESS_KEY.into());
            result.extend(wireless_config_to_dbus(wireless));
        }
        // ...
    }

    let conn_config = HashMap::from([
        (
            "assigned-mac-address",
            Value::new(conn.mac_address.to_string()),
        ),
        ("mtu", Value::new(conn.mtu)),
    ]);
    if conn.is_ethernet() {
        result.insert(ETHERNET_KEY, conn_config);
    } else if let ConnectionConfig::Wireless(_) = conn.config {
        result.get_mut(WIRELESS_KEY).unwrap().extend(conn_config);
    }

or like this?

    match &conn.config {
        ConnectionConfig::Wireless(wireless) => {
            connection_dbus.insert("type", WIRELESS_KEY.into());
            result.extend(wireless_config_to_dbus(wireless));
        }
        // ...
    }

    add_base_conn_values(conn, &mut result);
    // ...
}

fn add_base_conn_values(conn: &Connection, conn_result: &mut NestedHash) {
    let conn_type = if conn.is_ethernet() {
        ETHERNET_KEY
    } else if let ConnectionConfig::Wireless(_) = conn.config {
        WIRELESS_KEY
    } else {
        return;
    };
    let conn_config = HashMap::from([
        (
            "assigned-mac-address",
            Value::new(conn.mac_address.to_string()),
        ),
        ("mtu", Value::new(conn.mtu)),
    ]);
    if let Some(conn_entry) = conn_result.get_mut(conn_type) {
        conn_entry.extend(conn_config);
    } else {
        conn_result.insert(conn_type, conn_config);
    }
}

Generally without identifying connection type I don't think is possible, because it needs to be inserted at the *_KEY key.
The above solutions at least avoid the duplicate HashMap generation code. I personally don't really have a preference 🙂

Value::new(conn.mac_address.to_string()),
),
("mtu", Value::new(conn.mtu)),
]);
result.insert(ETHERNET_KEY, ethernet_config);
}

match &conn.config {
ConnectionConfig::Wireless(wireless) => {
connection_dbus.insert("type", WIRELESS_KEY.into());
let wireless_dbus = wireless_config_to_dbus(wireless, &conn.mac_address);
let mut wireless_dbus = wireless_config_to_dbus(wireless);
if let Some(wireless_dbus_key) = wireless_dbus.get_mut(WIRELESS_KEY) {
wireless_dbus_key.extend(HashMap::from([
("mtu", Value::new(conn.mtu)),
(
"assigned-mac-address",
Value::new(conn.mac_address.to_string()),
),
]));
}

result.extend(wireless_dbus);
}
ConnectionConfig::Bond(bond) => {
Expand Down Expand Up @@ -336,14 +349,10 @@ fn ip_config_to_ipv6_dbus(ip_config: &IpConfig) -> HashMap<&str, zvariant::Value
ipv6_dbus
}

fn wireless_config_to_dbus<'a>(
config: &'a WirelessConfig,
mac_address: &MacAddress,
) -> NestedHash<'a> {
fn wireless_config_to_dbus<'a>(config: &'a WirelessConfig) -> NestedHash<'a> {
let mut wireless: HashMap<&str, zvariant::Value> = HashMap::from([
("mode", Value::new(config.mode.to_string())),
("ssid", Value::new(config.ssid.to_vec())),
("assigned-mac-address", Value::new(mac_address.to_string())),
("hidden", Value::new(config.hidden)),
]);

Expand Down Expand Up @@ -568,8 +577,10 @@ fn base_connection_from_dbus(conn: &OwnedNestedHash) -> Option<Connection> {

if let Some(ethernet_config) = conn.get(ETHERNET_KEY) {
base_connection.mac_address = mac_address_from_dbus(ethernet_config)?;
base_connection.mtu = mtu_from_dbus(ethernet_config);
} else if let Some(wireless_config) = conn.get(WIRELESS_KEY) {
base_connection.mac_address = mac_address_from_dbus(wireless_config)?;
base_connection.mtu = mtu_from_dbus(wireless_config);
}

base_connection.ip_config = ip_config_from_dbus(conn)?;
Expand All @@ -591,6 +602,14 @@ fn mac_address_from_dbus(config: &HashMap<String, OwnedValue>) -> Option<MacAddr
}
}

fn mtu_from_dbus(config: &HashMap<String, OwnedValue>) -> u32 {
if let Some(mtu) = config.get("mtu") {
*mtu.downcast_ref::<u32>().unwrap_or(&0)
} else {
0
}
}

fn match_config_from_dbus(
match_config: &HashMap<String, zvariant::OwnedValue>,
) -> Option<MatchConfig> {
Expand Down Expand Up @@ -995,6 +1014,8 @@ mod test {

assert_eq!(connection.mac_address.to_string(), "12:34:56:78:9A:BC");

assert_eq!(connection.mtu, 9000_u32);

assert_eq!(
ip_config.addresses,
vec![
Expand Down Expand Up @@ -1379,10 +1400,13 @@ mod test {
Value::new("eth0".to_string()).to_owned(),
),
]);
let ethernet = HashMap::from([(
"assigned-mac-address".to_string(),
Value::new("12:34:56:78:9A:BC".to_string()).to_owned(),
)]);
let ethernet = HashMap::from([
(
"assigned-mac-address".to_string(),
Value::new("12:34:56:78:9A:BC".to_string()).to_owned(),
),
("mtu".to_string(), Value::new(9000).to_owned()),
]);
original.insert("connection".to_string(), connection);
original.insert(ETHERNET_KEY.to_string(), ethernet);

Expand All @@ -1398,6 +1422,7 @@ mod test {
assert_eq!(connection.get("interface-name"), None);
let ethernet = merged.get(ETHERNET_KEY).unwrap();
assert_eq!(ethernet.get("assigned-mac-address"), Some(&Value::from("")));
assert_eq!(ethernet.get("mtu"), Some(&Value::from(0_u32)));
}

fn build_ethernet_section_from_dbus() -> HashMap<String, OwnedValue> {
Expand All @@ -1407,6 +1432,7 @@ mod test {
"assigned-mac-address".to_string(),
Value::new("12:34:56:78:9A:BC").to_owned(),
),
("mtu".to_string(), Value::new(9000_u32).to_owned()),
])
}

Expand Down Expand Up @@ -1436,6 +1462,7 @@ mod test {
id: "agama".to_string(),
ip_config,
mac_address,
mtu: 1500_u32,
..Default::default()
}
}
Expand All @@ -1453,6 +1480,15 @@ mod test {
.unwrap();
assert_eq!(mac_address, "FD:CB:A9:87:65:43");

assert_eq!(
*ethernet_connection
.get("mtu")
.unwrap()
.downcast_ref::<u32>()
.unwrap(),
1500_u32
);

let ipv4_dbus = conn_dbus.get("ipv4").unwrap();
let gateway4: &str = ipv4_dbus.get("gateway").unwrap().downcast_ref().unwrap();
assert_eq!(gateway4, "192.168.0.1");
Expand Down
6 changes: 6 additions & 0 deletions rust/package/agama.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Mon Jun 10 14:24:33 UTC 2024 - Jorik Cronenberg <jorik.cronenberg@suse.com>

- Add mtu property for network connections
(gh#openSUSE/agama#1101).

-------------------------------------------------------------------
Fri Jun 7 05:58:48 UTC 2024 - Michal Filka <mfilka@suse.com>

Expand Down