[PATCH 1/1] applet: fix tracking of active access-point
Caught the following valgrind error on network-manager-applet-1.2.0-0.3.beta1.fc24: == Invalid read of size 8 ==at 0x822471D: g_type_check_instance (gtype.c:4137) ==by 0x8218B63: g_signal_handlers_disconnect_matched (gsignal.c:2925) ==by 0x129B3D: update_active_ap (applet-device-wifi.c:1195) ==by 0x129C92: wifi_device_state_changed (applet-device-wifi.c:1219) ==by 0x11C96E: foo_device_state_changed_cb (applet.c:2308) ==by 0xF2FCC57: ffi_call_unix64 (in /usr/lib64/libffi.so.6.0.2) ==by 0xF2FC6B9: ffi_call (in /usr/lib64/libffi.so.6.0.2) ==by 0x81FF279: g_cclosure_marshal_generic_va (gclosure.c:1604) ==by 0x81FE7A6: _g_closure_invoke_va (gclosure.c:867) ==by 0x821A1D7: g_signal_emit_valist (gsignal.c:3294) ==by 0x821A82E: g_signal_emit (gsignal.c:3441) ==by 0x7ED59DC: g_simple_async_result_complete (gsimpleasyncresult.c:801) This happens, because we hookup the access-point at the device, without taking any strong references or otherwise ensuring proper lifetime handling. Fix that, by registering a weak-ref to the access-point, so that we notice when the access-point gets destroyed. Note that we don't want to take strong references, because neither device, access-point nor applet should keep each other alive only because of an active access-point. Also, instead of registering the access-point at the device, register it at the applet. In principle there could be multiple applet instances and it is wrong that they all try to register the access-point on the same device. --- src/applet-device-wifi.c | 186 +++ 1 file changed, 155 insertions(+), 31 deletions(-) diff --git a/src/applet-device-wifi.c b/src/applet-device-wifi.c index d248566..b7bdd2c 100644 --- a/src/applet-device-wifi.c +++ b/src/applet-device-wifi.c @@ -46,6 +46,154 @@ static void _do_new_auto_connection (NMApplet *applet, AppletNewAutoConnectionCallback callback, gpointer callback_data); +/*/ + +typedef struct { + NMApplet *applet; + NMDevice *device; + NMAccessPoint *ap; + gulong signal_id; +} ActiveAPData; + +static void _active_ap_set (NMApplet *applet, NMDevice *device, NMAccessPoint *ap); +static void _active_ap_set_weakref (gpointer data, GObject *where_the_object_was); + +static void +_active_ap_set_notify (NMAccessPoint *ap, GParamSpec *pspec, gpointer user_data) +{ + ActiveAPData *d = user_data; + + g_return_if_fail (NM_IS_ACCESS_POINT (ap)); + g_return_if_fail (d); + g_return_if_fail (NM_IS_APPLET (d->applet)); + g_return_if_fail (NM_IS_DEVICE (d->device)); + g_return_if_fail (d->ap == ap); + g_return_if_fail (d->signal_id); + + applet_schedule_update_icon (d->applet); +} + +static void +_active_ap_data_free (ActiveAPData *d) +{ + if (d->device) + g_object_weak_unref ((GObject *) d->device, _active_ap_set_weakref, d); + if (d->ap) { + g_object_weak_unref ((GObject *) d->ap, _active_ap_set_weakref, d); + g_signal_handler_disconnect (d->ap, d->signal_id); + } + g_slice_free (ActiveAPData, d); +} + +static NMAccessPoint * +_active_ap_get (NMApplet *applet, NMDevice *device) +{ + GSList *list, *iter; + + g_return_val_if_fail (NM_IS_APPLET (applet), NULL); + g_return_val_if_fail (NM_IS_DEVICE (device), NULL); + + list = g_object_get_data ((GObject *) applet, ACTIVE_AP_TAG); + for (iter = list; iter; iter = iter->next) { + ActiveAPData *d = iter->data; + + if (device == d->device && d->ap) + return d->ap; + } + return NULL; +} + +static void +_active_ap_set_destroy (gpointer data) +{ + g_slist_free_full (data, (GDestroyNotify) _active_ap_data_free); +} + +static void +_active_ap_set_weakref (gpointer data, GObject *where_the_object_was) +{ + ActiveAPData *d = data; + + if ((GObject *) d->ap == where_the_object_was) + d->ap = NULL; + else if ((GObject *) d->device == where_the_object_was) + d->device = NULL; + else + g_return_if_reached (); + _active_ap_set (d->applet, NULL, NULL); + + applet_schedule_update_icon (d->applet); +} + +static void +_active_ap_set (NMApplet *applet, NMDevice *device, NMAccessPoint *ap) +{ + GSList *list, *iter, *list0, *pcurrent; + ActiveAPData *d; + + g_return_if_fail (NM_IS_APPLET (applet)); + g_return_if_fail (!device || NM_IS_DEVICE (device)); + g_return_if_fail (!ap || NM_IS_ACCESS_POINT (ap)); + + list0 = g_object_get_data ((GObject *) applet, ACTIVE_AP_TAG); + list = list0; + +remove_empty: + pcurrent = NULL; + for (iter = list; iter; iter = iter->next) { +
Re: Bonding wifi and wired
On Mon, 2016-03-07 at 23:34 -0500, Nikolay Martynov wrote: > Hi. > > I just wanted to reach out to ask what's NetworkManager team's > position on allowing bonding of wifi and wired interface? Are there > any plans to support this? If not - it is due to no one being > interested implementing it or because this feature would go against > some large plans/ideology? > > I was able to make it work with NM on client side and OpenWRT on > router side. Router side is easy. On NM side there is some manual > config editing (which is fine) and some kinds that make day-to-day > life less enjoyable than it should (like inability to manually > connect > to bonded wifi). SO I was curious - would patches fixing those > smaller > kinks be acceptable at all or is it something that NM team is just > not > interested in? > I'm not familiar with the quirks of bonding of Wi-Fi and ethernet. Anyway, we are definitely interested in supporting all kind of use- cases and patches are always welcome :) Thomas signature.asc Description: This is a digitally signed message part ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] manager: fix assumption of child connections with autoconnect=no
During startup, when a link is detected (enp0s25 in the example below) we try to create also virtual devices (ipip1) on it through system_create_virtual_device(), however this realizes only devices for connections which can autoactivate. To support the assumption of child devices with autoconnect=no, we should take in consideration in retry_connections_for_parent_device() only connections for which the link does not exist, and let existing links be handled by platform_link_added(), which also realizes them. Reproducer: $ nmcli c add type ip-tunnel ifname ipip1 con-name ipip1+ autoconnect no \ mode ipip remote 172.25.16.1 dev enp0s25 ip4 1.2.3.4/31 $ nmcli c up ipip1+ $ systemctl restart NetworkManager Result: * before: ipip1+ is not assumed, ipip1 is not present in 'nmcli d' output * after: ipip1+ is assumed, ipip1 detected --- src/nm-manager.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index e7eb7c6..625a235 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1127,23 +1127,31 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) static void retry_connections_for_parent_device (NMManager *self, NMDevice *device) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GSList *connections, *iter; + gs_free_error GError *error = NULL; + gs_free char *ifname = NULL; g_return_if_fail (device); connections = nm_settings_get_connections (priv->settings); for (iter = connections; iter; iter = g_slist_next (iter)) { NMConnection *candidate = iter->data; NMDevice *parent; parent = find_parent_device_for_connection (self, candidate, NULL); - if (parent == device) - connection_changed (priv->settings, candidate, self); + if (parent == device) { + /* Only try to activate devices that don't already exist */ + ifname = nm_manager_get_connection_iface (self, candidate, &parent, &error); + if (ifname) { + if (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, ifname)) + connection_changed (priv->settings, candidate, self); + } + } } g_slist_free (connections); } static void -- 2.5.0 ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] manager: fix assumption of child connections with autoconnect=no
On Tue, 2016-03-08 at 12:02 +0100, Beniamino Galvani wrote: > During startup, when a link is detected (enp0s25 in the example > below) > we try to create also virtual devices (ipip1) on it through > system_create_virtual_device(), however this realizes only devices > for > connections which can autoactivate. > > To support the assumption of child devices with autoconnect=no, we > should take in consideration in retry_connections_for_parent_device() > only connections for which the link does not exist, and let existing > links be handled by platform_link_added(), which also realizes them. > > Reproducer: > $ nmcli c add type ip-tunnel ifname ipip1 con-name ipip1+ > autoconnect no \ > mode ipip remote 172.25.16.1 dev enp0s25 ip4 > 1.2.3.4/31 > $ nmcli c up ipip1+ > $ systemctl restart NetworkManager > > Result: > * before: ipip1+ is not assumed, ipip1 is not present in 'nmcli d' > output > * after: ipip1+ is assumed, ipip1 detected > --- > src/nm-manager.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/nm-manager.c b/src/nm-manager.c > index e7eb7c6..625a235 100644 > --- a/src/nm-manager.c > +++ b/src/nm-manager.c > @@ -1127,23 +1127,31 @@ system_create_virtual_device (NMManager > *self, NMConnection *connection) > > static void > retry_connections_for_parent_device (NMManager *self, NMDevice > *device) > { > NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); > GSList *connections, *iter; > + gs_free_error GError *error = NULL; > + gs_free char *ifname = NULL; should be declared inside the loop, otherwise as you iterate they get reset and leak (or fail assertion g_return_if_fail (!error || !*error)). > > g_return_if_fail (device); > > connections = nm_settings_get_connections (priv->settings); > for (iter = connections; iter; iter = g_slist_next (iter)) { > NMConnection *candidate = iter->data; > NMDevice *parent; > > parent = find_parent_device_for_connection (self, > candidate, NULL); > - if (parent == device) > - connection_changed (priv->settings, > candidate, self); > + if (parent == device) { > + /* Only try to activate devices that don't > already exist */ > + ifname = nm_manager_get_connection_iface > (self, candidate, &parent, &error); > + if (ifname) { > + if (!nm_platform_link_get_by_ifname > (NM_PLATFORM_GET, ifname)) > + connection_changed (priv- > >settings, candidate, self); > + } > + } > } > > g_slist_free (connections); > } > > static void otherwise, lgtm. Thomas signature.asc Description: This is a digitally signed message part ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH 1/1] applet: fix tracking of active access-point
On Tue, Mar 08, 2016 at 10:00:51AM +0100, Thomas Haller wrote: > Fix that, by registering a weak-ref to the access-point, so that we > notice when the access-point gets destroyed. Note that we don't want > to take strong references, because neither device, access-point nor > applet should keep each other alive only because of an active > access-point. Patch looks good to me. Beniamino signature.asc Description: PGP signature ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH 1/1] applet: fix tracking of active access-point
On Tue, 2016-03-08 at 14:07 +0100, Beniamino Galvani wrote: > On Tue, Mar 08, 2016 at 10:00:51AM +0100, Thomas Haller wrote: > > > > Fix that, by registering a weak-ref to the access-point, so that we > > notice when the access-point gets destroyed. Note that we don't > > want > > to take strong references, because neither device, access-point nor > > applet should keep each other alive only because of an active > > access-point. > Patch looks good to me. > Thanks. Merged as https://git.gnome.org/browse/network-manager-applet/commit/?id=0ba3b6537fc0c9c7c43ae6ae92ea2ec5c358ebad Thomas signature.asc Description: This is a digitally signed message part ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: Bonding wifi and wired
On Tue, 2016-03-08 at 10:50 +0100, Thomas Haller wrote: > On Mon, 2016-03-07 at 23:34 -0500, Nikolay Martynov wrote: > > > > Hi. > > > > I just wanted to reach out to ask what's NetworkManager team's > > position on allowing bonding of wifi and wired interface? Are there > > any plans to support this? If not - it is due to no one being > > interested implementing it or because this feature would go against > > some large plans/ideology? > > > > I was able to make it work with NM on client side and OpenWRT on > > router side. Router side is easy. On NM side there is some manual > > config editing (which is fine) and some kinds that make day-to-day > > life less enjoyable than it should (like inability to manually > > connect > > to bonded wifi). SO I was curious - would patches fixing those > > smaller > > kinks be acceptable at all or is it something that NM team is just > > not > > interested in? > > > I'm not familiar with the quirks of bonding of Wi-Fi and ethernet. > > Anyway, we are definitely interested in supporting all kind of use- > cases and patches are always welcome :) I know it's been done before, but one caveat is that the wifi driver and the supplicant have to correctly support setting the WiFi device's MAC address since bonding requires they have the same one. That's been a problem in the past and required fixes to the supplicant and drivers. But yeah, I'm sure we'd take patches fixing anything on the NM side. Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: Bonding wifi and wired
2016-03-08 14:30 GMT-03:00 Dan Williams : > > I know it's been done before, but one caveat is that the wifi driver > and the supplicant have to correctly support setting the WiFi device's > MAC address since bonding requires they have the same one. That's been > a problem in the past and required fixes to the supplicant and drivers. > But yeah, I'm sure we'd take patches fixing anything on the NM side. > > Hi Dan, And what about if NM assume this to be done based on the WiFi's MAC? This way we could use this feature despite the drive in use... ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
NetworkManager 1.1.90 : nm-exported-object.c:293:nm_exported_object_class_add_interface: assertion failed: (object_property != NULL)
Hi guys I installed the development version of NM 1.1.90 and I was trying to add a connection but NM crapped out with the error: NetworkManager:ERROR:nm-exported-object.c:293:nm_exported_object_class_add_interface: assertion failed: (object_property != NULL) Any ideas why? root@beaglebone:/etc/NetworkManager/system-connections# NetworkManager --debug & [2] 2751 root@beaglebone:/etc/NetworkManager/system-connections# NetworkManager: /lib/arm-linux-gnueabihf/libnl-3.so.200: no version information available (required by NetworkManager) NetworkManager-Message: No config file found or given; using /etc/NetworkManager/NetworkManager.conf (NetworkManager:2751): NetworkManager-WARNING **: glib-version: cannot handle SIGUSR1 and SIGUSR2 signals. Consider upgrading glib to 2.36.0 or newer NetworkManager[2751]: NetworkManager (version 1.1.90) is starting... NetworkManager[2751]: Read config: /etc/NetworkManager/NetworkManager.conf NetworkManager[2751]: monitoring kernel firmware directory '/lib/firmware'. NetworkManager[2751]: dns-mgr[0x1f4c20]: set resolv-conf-mode: default NetworkManager[2751]: dns-mgr[0x1f4c20]: using resolv.conf manager 'none' NetworkManager[2751]: init! NetworkManager[2751]: interface-parser: parsing file /etc/network/interfaces NetworkManager[2751]: interface-parser: finished parsing file /etc/network/interfaces NetworkManager[2751]: guessed connection type (eth0) = 802-3-ethernet NetworkManager[2751]: update_connection_setting_from_if_block: name:eth0, type:802-3-ethernet, id:Ifupdown (eth0), uuid: 681b428f-beaf-8932-dce4-687ed5bae28e NetworkManager[2751]: adding eth0 to connections NetworkManager[2751]: adding iface eth0 to eni_ifaces NetworkManager[2751]: guessed connection type (usb0) = 802-3-ethernet NetworkManager[2751]: update_connection_setting_from_if_block: name:usb0, type:802-3-ethernet, id:Ifupdown (usb0), uuid: 3232978a-bef5-2ef2-3aa8-fdd650bb306d NetworkManager[2751]: addresses count: 1 NetworkManager[2751]: No dns-nameserver configured in /etc/network/interfaces NetworkManager[2751]: adding usb0 to connections NetworkManager[2751]: adding iface usb0 to eni_ifaces NetworkManager[2751]: autoconnect NetworkManager[2751]: management mode: unmanaged NetworkManager[2751]: devices added (path: /sys/devices/ocp.2/481cc000.d_can/net/can0, iface: can0) NetworkManager[2751]: device added (path: /sys/devices/ocp.2/481cc000.d_can/net/can0, iface: can0): no ifupdown configuration found. NetworkManager[2751]: devices added (path: /sys/devices/ocp.2/481d.d_can/net/can1, iface: can1) NetworkManager[2751]: device added (path: /sys/devices/ocp.2/481d.d_can/net/can1, iface: can1): no ifupdown configuration found. NetworkManager[2751]: devices added (path: /sys/devices/ocp.2/4a10.ethernet/net/eth0, iface: eth0) NetworkManager[2751]: locking wired connection setting NetworkManager[2751]: devices added (path: /sys/devices/virtual/net/lo, iface: lo) NetworkManager[2751]: device added (path: /sys/devices/virtual/net/lo, iface: lo): no ifupdown configuration found. NetworkManager[2751]: end _init. NetworkManager[2751]: Loaded settings plugin ifupdown: (C) 2008 Canonical Ltd. To report bugs please use the NetworkManager mailing list. (/usr/lib/NetworkManager/libnm-settings-plugin-ifupdown.so) NetworkManager[2751]: Loaded settings plugin iBFT: (c) 2014 Red Hat, Inc. To report bugs please use the NetworkManager mailing list. (/usr/lib/NetworkManager/libnm-settings-plugin-ibft.so) NetworkManager[2751]: Loaded settings plugin keyfile: (c) 2007 - 2015 Red Hat, Inc. To report bugs please use the NetworkManager mailing list. NetworkManager[2751]: (2088960) ... get_connections. NetworkManager[2751]: (2088960) ... get_connections (managed=false): return empty list. NetworkManager[2751]: get unmanaged devices count: 1 NetworkManager[2751]: hostname: couldn't get property from hostnamed NetworkManager[2751]: WiFi enabled by radio killswitch; enabled by state file NetworkManager[2751]: WWAN enabled by radio killswitch; enabled by state file NetworkManager[2751]: Networking is enabled by state file NetworkManager[2751]: Loaded device plugin: NMVxlanFactory (internal) NetworkManager[2751]: Loaded device plugin: NMVlanFactory (internal) NetworkManager[2751]: Loaded device plugin: NMVethFactory (internal) NetworkManager[2751]: Loaded device plugin: NMTunFactory (internal) NetworkManager[2751]: Loaded device plugin: NMMacvlanFactory (internal) NetworkManager[2751]: Loaded device plugin: NMIPTunnelFactory (internal) NetworkManager[2751]: Loaded device plugin: NMInfinibandFactory (internal) NetworkManager[2751]: Loaded device plugin: NMEthernetFactory (internal) NetworkManager[2751]: Loaded device plugin: NMBridgeFactory (internal) NetworkManager[2751]: Loaded device plugin: NMBondFactory (internal) NetworkManager[2751]: Loaded device plugin: NMWifiFactory (/usr/lib/NetworkManager/libnm-de
Re: Bonding wifi and wired
Hi. 2016-03-08 15:55 GMT-05:00 José Queiroz : > > > 2016-03-08 14:30 GMT-03:00 Dan Williams : >> >> >> I know it's been done before, but one caveat is that the wifi driver >> and the supplicant have to correctly support setting the WiFi device's >> MAC address since bonding requires they have the same one. That's been >> a problem in the past and required fixes to the supplicant and drivers. >> But yeah, I'm sure we'd take patches fixing anything on the NM side. >> > > Hi Dan, > > And what about if NM assume this to be done based on the WiFi's MAC? This > way we could use this feature despite the drive in use... Yeah, that's exactly what I do currently. Just to clarify: this whole idea works on 'stock' ubuntu's NM since at least 14.04, and probably before that. I've setup my eth interface to clone MAC from my wifi. This way bond interface gets MAC that is equal to wifi's MAC. Without this wifi (at least one that I have) refuses properly authenticate on AP. On router side I had to enable wifi clients isolation and LAN bridge hairpin so two bonded clients can properly talk to each other and other AP clients. Things work in a sense that I can pull the plug out of laptop during hangout call and not notice anything, and I can put it back in and see traffic flowing on wire. But NM's clearly gets confused by this setu. But since it sounds like NM's team would welcome patches I hope I will be able to address some of those problems. -- Martynov Nikolay. Email: mar.ko...@gmail.com ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list