From 6d95c406dbe3b92676cb0aeb5b98e755c7cfc06a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 6 Sep 2022 10:25:34 +0200 Subject: [PATCH 1/3] platform: don't fallback to IFLA_BOND_ACTIVE_SLAVE for the primary The IFLA_BOND_ACTIVE_SLAVE and IFLA_BOND_PRIMARY are not the same. If the primary is not set, then that's it. Don't fallback. Only NetworkManager API deprecated "active-slave" and uses it as alias for "primary". That does not mean, kernel/netlink does that. --- src/libnm-platform/nm-linux-platform.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index 472884d3d5..954b8a61e6 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -1589,11 +1589,8 @@ _parse_lnk_bond(const char *kind, struct nlattr *info_data) if (tb[IFLA_BOND_MODE]) props->mode = nla_get_u8(tb[IFLA_BOND_MODE]); - if (tb[IFLA_BOND_PRIMARY]) { + if (tb[IFLA_BOND_PRIMARY]) props->primary = nla_get_u32(tb[IFLA_BOND_PRIMARY]); - } else if (tb[IFLA_BOND_ACTIVE_SLAVE]) { - props->primary = nla_get_u32(tb[IFLA_BOND_ACTIVE_SLAVE]); - } if (tb[IFLA_BOND_MIIMON]) { props->miimon = nla_get_u32(tb[IFLA_BOND_MIIMON]); props->miimon_has = TRUE; From c28dd78c05074cab1ee4a08df5369b01bdd65769 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 6 Sep 2022 10:17:04 +0200 Subject: [PATCH 2/3] platform: use signed int for NMPlatformLnkBond.primary On netlink API, the attribute is indeed u32. However, this is an ifindex which in most other kernel APIs and in NetworkManager code is a signed integer. Note that of course kernel would only ever assign numbers that are valid ifindexes, thus in the suitable range. --- src/libnm-platform/nm-linux-platform.c | 4 ++-- src/libnm-platform/nm-platform.c | 2 +- src/libnm-platform/nm-platform.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index 954b8a61e6..3cbb6a685d 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -1590,7 +1590,7 @@ _parse_lnk_bond(const char *kind, struct nlattr *info_data) if (tb[IFLA_BOND_MODE]) props->mode = nla_get_u8(tb[IFLA_BOND_MODE]); if (tb[IFLA_BOND_PRIMARY]) - props->primary = nla_get_u32(tb[IFLA_BOND_PRIMARY]); + props->primary = NM_CLAMP((int) nla_get_u32(tb[IFLA_BOND_PRIMARY]), 0, G_MAXINT); if (tb[IFLA_BOND_MIIMON]) { props->miimon = nla_get_u32(tb[IFLA_BOND_MIIMON]); props->miimon_has = TRUE; @@ -4536,7 +4536,7 @@ _nl_msg_new_link_set_linkinfo(struct nl_msg *msg, NMLinkType link_type, gconstpo NLA_PUT_U32(msg, IFLA_BOND_PACKETS_PER_SLAVE, props->packets_per_port); if (props->peer_notif_delay_has) NLA_PUT_U32(msg, IFLA_BOND_PEER_NOTIF_DELAY, props->peer_notif_delay); - if (props->primary) + if (props->primary > 0) NLA_PUT_U32(msg, IFLA_BOND_PRIMARY, props->primary); if (props->resend_igmp_has) NLA_PUT_U32(msg, IFLA_BOND_RESEND_IGMP, props->resend_igmp); diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index 3dfe3404d4..01be185df0 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -6093,7 +6093,7 @@ nm_platform_lnk_bond_to_string(const NMPlatformLnkBond *lnk, char *buf, gsize le &len, "bond" " mode %u" - " primary %u" + " primary %d" "%s" /* miimon */ "%s" /* updelay */ "%s" /* downdelay */ diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index 9aaed5dd1f..cb08a3bff7 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -889,6 +889,7 @@ extern const NMPlatformLnkBridge nm_platform_lnk_bridge_default; #define NM_BOND_MAX_ARP_TARGETS 16 typedef struct { + int primary; in_addr_t arp_ip_target[NM_BOND_MAX_ARP_TARGETS]; guint32 arp_all_targets; guint32 arp_interval; @@ -899,7 +900,6 @@ typedef struct { guint32 min_links; guint32 packets_per_port; guint32 peer_notif_delay; - guint32 primary; guint32 resend_igmp; guint32 updelay; guint16 ad_actor_sys_prio; From 4fd90fb6cc098105dcf989ff772c28167874c40e Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Thu, 8 Sep 2022 12:00:45 +0200 Subject: [PATCH 3/3] bond: fix primary bond option when the link is not present Bond option netlink support requires primary property to be a ifindex instead of the interface name. This is a workaround for supporting specifying a primary that does not exist yet. ``` nmcli con add type bond ifname mybond0 bond.options "mode=active-backup,primary=veth1" Connection 'bond-mybond0' (38100ef9-11e2-4003-aff9-cb2d152ce34f) successfully added. nmcli con add type ethernet ifname veth1 master mybond0 cat /sys/class/net/mybond0/bonding/primary veth1 ``` https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1362 Fixes: e064eb9d1361 ('bond: use netlink to set bond options') --- src/core/devices/nm-device-bond.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/devices/nm-device-bond.c b/src/core/devices/nm-device-bond.c index 558f8f7598..b4e9a4e921 100644 --- a/src/core/devices/nm-device-bond.c +++ b/src/core/devices/nm-device-bond.c @@ -493,6 +493,10 @@ act_stage1_prepare(NMDevice *device, NMDeviceStateReason *out_failure_reason) if (!nm_device_hw_addr_set_cloned(device, nm_device_get_applied_connection(device), FALSE)) ret = NM_ACT_STAGE_RETURN_FAILURE; } + + /* This is a workaround because netlink do not support ifname as primary */ + set_bond_attr_or_default(device, s_bond, NM_SETTING_BOND_OPTION_PRIMARY); + nm_device_bring_up(device, TRUE, NULL); return ret;