From e00c81b1536cdfb72fa4776a7b0768b573c58009 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 25 Jul 2024 09:49:18 +0200 Subject: [PATCH] bridge: change the signature for nm_platform_link_set_bridge_vlans() Currently, nm_platform_link_set_bridge_vlans() accepts an array of pointers to vlan objects; to avoid multiple allocations, setting_vlans_to_platform() creates the array by piggybacking the actual data after the pointers array. In the next commits, the array will need to be manipulated and extended, which is difficult with the current structure. Instead, pass separately an array of objects and its size. --- src/core/devices/nm-device-bridge.c | 64 +++++++++++++------------- src/libnm-platform/nm-linux-platform.c | 19 ++++---- src/libnm-platform/nm-platform.c | 32 ++++++------- src/libnm-platform/nm-platform.h | 18 ++++---- 4 files changed, 70 insertions(+), 63 deletions(-) diff --git a/src/core/devices/nm-device-bridge.c b/src/core/devices/nm-device-bridge.c index b256056d43..cde94a4264 100644 --- a/src/core/devices/nm-device-bridge.c +++ b/src/core/devices/nm-device-bridge.c @@ -419,20 +419,18 @@ static const Option controller_options[] = { 0, }}; -static const NMPlatformBridgeVlan ** -setting_vlans_to_platform(GPtrArray *array) +static NMPlatformBridgeVlan * +setting_vlans_to_platform(GPtrArray *array, guint *out_len) { - NMPlatformBridgeVlan **arr; - NMPlatformBridgeVlan *p_data; - guint i; + NMPlatformBridgeVlan *arr; + guint i; - if (!array || !array->len) + if (!array || !array->len) { + *out_len = 0; return NULL; + } - G_STATIC_ASSERT_EXPR(_nm_alignof(NMPlatformBridgeVlan *) >= _nm_alignof(NMPlatformBridgeVlan)); - arr = g_malloc((sizeof(NMPlatformBridgeVlan *) * (array->len + 1)) - + (sizeof(NMPlatformBridgeVlan) * (array->len))); - p_data = (NMPlatformBridgeVlan *) &arr[array->len + 1]; + arr = g_new(NMPlatformBridgeVlan, array->len); for (i = 0; i < array->len; i++) { NMBridgeVlan *vlan = array->pdata[i]; @@ -440,16 +438,16 @@ setting_vlans_to_platform(GPtrArray *array) nm_bridge_vlan_get_vid_range(vlan, &vid_start, &vid_end); - p_data[i] = (NMPlatformBridgeVlan){ + arr[i] = (NMPlatformBridgeVlan){ .vid_start = vid_start, .vid_end = vid_end, .pvid = nm_bridge_vlan_is_pvid(vlan), .untagged = nm_bridge_vlan_is_untagged(vlan), }; - arr[i] = &p_data[i]; } - arr[i] = NULL; - return (const NMPlatformBridgeVlan **) arr; + + *out_len = array->len; + return arr; } static void @@ -639,15 +637,16 @@ is_bridge_pvid_changed(NMDevice *device, NMSettingBridge *s_bridge) static gboolean bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge, gboolean is_reapply) { - NMDeviceBridge *self = NM_DEVICE_BRIDGE(device); - gconstpointer hwaddr; - size_t length; - gboolean enabled; - guint16 pvid; - NMPlatform *plat; - int ifindex; - gs_unref_ptrarray GPtrArray *vlans = NULL; - gs_free const NMPlatformBridgeVlan **plat_vlans = NULL; + NMDeviceBridge *self = NM_DEVICE_BRIDGE(device); + gconstpointer hwaddr; + size_t length; + gboolean enabled; + guint16 pvid; + NMPlatform *plat; + int ifindex; + gs_unref_ptrarray GPtrArray *vlans = NULL; + gs_free NMPlatformBridgeVlan *plat_vlans = NULL; + guint num_vlans; if (self->vlan_configured) return TRUE; @@ -664,7 +663,7 @@ bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge, gboolean is .vlan_filtering_val = FALSE, .vlan_default_pvid_has = TRUE, .vlan_default_pvid_val = 1})); - nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, NULL); + nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, NULL, 0); return TRUE; } @@ -696,7 +695,7 @@ bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge, gboolean is .vlan_default_pvid_val = 0})); /* Clear all existing VLANs */ - if (!nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, NULL)) + if (!nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, NULL, 0)) return FALSE; /* Now set the default PVID. After this point the kernel creates @@ -714,8 +713,9 @@ bridge_set_vlan_options(NMDevice *device, NMSettingBridge *s_bridge, gboolean is /* Create VLANs only after setting the default PVID, so that * any PVID VLAN overrides the bridge's default PVID. */ g_object_get(s_bridge, NM_SETTING_BRIDGE_VLANS, &vlans, NULL); - plat_vlans = setting_vlans_to_platform(vlans); - if (plat_vlans && !nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, plat_vlans)) + plat_vlans = setting_vlans_to_platform(vlans, &num_vlans); + if (plat_vlans + && !nm_platform_link_set_bridge_vlans(plat, ifindex, FALSE, plat_vlans, num_vlans)) return FALSE; nm_platform_link_set_bridge_info(plat, @@ -937,13 +937,14 @@ attach_port(NMDevice *device, bridge_set_vlan_options(device, s_bridge, FALSE); if (nm_setting_bridge_get_vlan_filtering(s_bridge)) { - gs_free const NMPlatformBridgeVlan **plat_vlans = NULL; - gs_unref_ptrarray GPtrArray *vlans = NULL; + gs_free NMPlatformBridgeVlan *plat_vlans = NULL; + gs_unref_ptrarray GPtrArray *vlans = NULL; + guint num_vlans; if (s_port) g_object_get(s_port, NM_SETTING_BRIDGE_PORT_VLANS, &vlans, NULL); - plat_vlans = setting_vlans_to_platform(vlans); + plat_vlans = setting_vlans_to_platform(vlans, &num_vlans); /* Since the link was just enportd, there are no existing VLANs * (except for the default one) and so there's no need to flush. */ @@ -952,7 +953,8 @@ attach_port(NMDevice *device, && !nm_platform_link_set_bridge_vlans(nm_device_get_platform(port), nm_device_get_ifindex(port), TRUE, - plat_vlans)) + plat_vlans, + num_vlans)) return FALSE; } diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index 65f42d8a27..9a1920555d 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -9505,17 +9505,20 @@ nla_put_failure: } static gboolean -link_set_bridge_vlans(NMPlatform *platform, - int ifindex, - gboolean on_controller, - const NMPlatformBridgeVlan *const *vlans) +link_set_bridge_vlans(NMPlatform *platform, + int ifindex, + gboolean on_controller, + const NMPlatformBridgeVlan *vlans, + guint num_vlans) { nm_auto_nlmsg struct nl_msg *nlmsg = NULL; struct nlattr *list; struct bridge_vlan_info vinfo = {}; guint i; - nlmsg = _nl_msg_new_link_full(vlans ? RTM_SETLINK : RTM_DELLINK, + nm_assert(num_vlans == 0 || vlans); + + nlmsg = _nl_msg_new_link_full(num_vlans > 0 ? RTM_SETLINK : RTM_DELLINK, 0, ifindex, NULL, @@ -9533,10 +9536,10 @@ link_set_bridge_vlans(NMPlatform *platform, IFLA_BRIDGE_FLAGS, on_controller ? BRIDGE_FLAGS_CONTROLLER : BRIDGE_FLAGS_SELF); - if (vlans) { + if (num_vlans > 0) { /* Add VLANs */ - for (i = 0; vlans[i]; i++) { - const NMPlatformBridgeVlan *vlan = vlans[i]; + for (i = 0; i < num_vlans; i++) { + const NMPlatformBridgeVlan *vlan = &vlans[i]; gboolean is_range = vlan->vid_start != vlan->vid_end; vinfo.vid = vlan->vid_start; diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index f82de7f9d7..670f56bb2b 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -2070,10 +2070,11 @@ nm_platform_link_set_sriov_vfs(NMPlatform *self, int ifindex, const NMPlatformVF } gboolean -nm_platform_link_set_bridge_vlans(NMPlatform *self, - int ifindex, - gboolean on_controller, - const NMPlatformBridgeVlan *const *vlans) +nm_platform_link_set_bridge_vlans(NMPlatform *self, + int ifindex, + gboolean on_controller, + const NMPlatformBridgeVlan *vlans, + guint num_vlans) { guint i; _CHECK_SELF(self, klass, FALSE); @@ -2085,9 +2086,9 @@ nm_platform_link_set_bridge_vlans(NMPlatform *self, vlans ? "setting" : "clearing", on_controller ? "controller" : "self"); if (vlans) { - for (i = 0; vlans[i]; i++) { + for (i = 0; i < num_vlans; i++) { char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; - const NMPlatformBridgeVlan *vlan = vlans[i]; + const NMPlatformBridgeVlan *vlan = &vlans[i]; _LOG3D("link: bridge VLAN %s", nm_platform_bridge_vlan_to_string(vlan, sbuf, sizeof(sbuf))); @@ -2095,7 +2096,7 @@ nm_platform_link_set_bridge_vlans(NMPlatform *self, } } - return klass->link_set_bridge_vlans(self, ifindex, on_controller, vlans); + return klass->link_set_bridge_vlans(self, ifindex, on_controller, vlans, num_vlans); } gboolean @@ -6148,9 +6149,9 @@ nm_platform_link_to_string(const NMPlatformLink *link, char *buf, gsize len) link->initialized ? " init" : " not-init", link->inet6_addr_gen_mode_inv ? " addrgenmode " : "", link->inet6_addr_gen_mode_inv ? nm_platform_link_inet6_addrgenmode2str( - _nm_platform_uint8_inv(link->inet6_addr_gen_mode_inv), - str_addrmode, - sizeof(str_addrmode)) + _nm_platform_uint8_inv(link->inet6_addr_gen_mode_inv), + str_addrmode, + sizeof(str_addrmode)) : "", str_address[0] ? " addr " : "", str_address[0] ? str_address : "", @@ -7386,12 +7387,11 @@ nm_platform_ip6_route_to_string(const NMPlatformIP6Route *route, char *buf, gsiz route->lock_mtu ? "lock " : "", route->mtu) : "", - route->rt_pref - ? nm_sprintf_buf( - str_pref, - " pref %s", - nm_icmpv6_router_pref_to_string(route->rt_pref, str_pref2, sizeof(str_pref2))) - : ""); + route->rt_pref ? nm_sprintf_buf( + str_pref, + " pref %s", + nm_icmpv6_router_pref_to_string(route->rt_pref, str_pref2, sizeof(str_pref2))) + : ""); return buf; } diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index b05b1297a5..7754fba953 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -1185,10 +1185,11 @@ typedef struct { gpointer callback_data, GCancellable *cancellable); gboolean (*link_set_sriov_vfs)(NMPlatform *self, int ifindex, const NMPlatformVF *const *vfs); - gboolean (*link_set_bridge_vlans)(NMPlatform *self, - int ifindex, - gboolean on_controller, - const NMPlatformBridgeVlan *const *vlans); + gboolean (*link_set_bridge_vlans)(NMPlatform *self, + int ifindex, + gboolean on_controller, + const NMPlatformBridgeVlan *vlans, + guint num_vlans); gboolean (*link_set_bridge_info)(NMPlatform *self, int ifindex, const NMPlatformLinkSetBridgeInfoData *bridge_info); @@ -2049,10 +2050,11 @@ void nm_platform_link_set_sriov_params_async(NMPlatform *self, gboolean nm_platform_link_set_sriov_vfs(NMPlatform *self, int ifindex, const NMPlatformVF *const *vfs); -gboolean nm_platform_link_set_bridge_vlans(NMPlatform *self, - int ifindex, - gboolean on_controller, - const NMPlatformBridgeVlan *const *vlans); +gboolean nm_platform_link_set_bridge_vlans(NMPlatform *self, + int ifindex, + gboolean on_controller, + const NMPlatformBridgeVlan *vlans, + guint num_vlans); gboolean nm_platform_link_set_bridge_info(NMPlatform *self, int ifindex, const NMPlatformLinkSetBridgeInfoData *bridge_info);