Hi Pau,
> If for whatever reason a network interface belonging to a modem is
> renamed after ofono startup, ofono then keeps advertising the old
> non-existing interface name instead of the new one.
> To catch the changes we use udev's "move" action together with
> DEVPATH_OLD variable, which points to the old devpath of the device.
>
> Unfortunately current debian9 systemd version contains systemd bug which
> prevents this feature to work fine if the iface is renamed more than
> once, see [1] for more information.
>
> [1] https://github.com/systemd/systemd/issues/9426
> ---
> plugins/udevng.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index c518553a..8338927b 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -56,6 +56,7 @@ struct modem_info {
> };
> struct ofono_modem *modem;
> const char *sysattr;
> + gboolean needs_update;
> };
>
> struct device_info {
> @@ -1630,6 +1631,7 @@ static void add_device(const char *syspath, const char
> *devname,
>
> modem->devices = g_slist_insert_sorted(modem->devices, info,
> compare_device);
> + modem->needs_update = TRUE;
> }
>
> static struct {
> @@ -1822,6 +1824,8 @@ static gboolean create_modem(gpointer key, gpointer
> value, gpointer user_data)
> const char *syspath = key;
> unsigned int i;
>
> + modem->needs_update = FALSE;
> +
> if (modem->modem != NULL)
> return FALSE;
>
> @@ -1909,6 +1913,81 @@ static gboolean check_modem_list(gpointer user_data)
> return FALSE;
> }
>
> +static void drop_old_device(gpointer key, gpointer value, gpointer user_data)
> +{
> + const char *syspath = key;
> + struct modem_info *modem = value;
> + struct udev_device *device = user_data;
> + const char *devpath_old;
> + GSList *list;
> +
> + devpath_old = udev_device_get_property_value(device, "DEVPATH_OLD");
> + if (!devpath_old)
> + return;
> +
> + for (list = modem->devices; list; list = list->next) {
> + struct device_info *info = list->data;
> + struct udev_device *dev;
> +
> + if (g_strcmp0(info->subsystem, "net") != 0)
> + continue;
> +
> + if (g_strcmp0(info->devpath, devpath_old) != 0)
> + continue;
> +
> + /* Drop no longer existing net iface device */
> + DBG("Dropping lost device %s", info->devpath);
I really prefer that DBG is surrounded by empty lines.
> + modem->devices = g_slist_remove_link(modem->devices, list);
> + g_slist_free_1 (list);
No space between 1 and ( here.
> + device_info_free(info);
> + udev_device_unref(dev);
> + modem->needs_update = TRUE;
> + }
> +}
> +
> +
> +static void update_modem(gpointer key, gpointer value, gpointer user_data)
> +{
> + const char *syspath = key;
> + struct modem_info *modem = value;
> + unsigned int i;
> +
> + if (!modem->needs_update)
> + return;
> +
> + DBG("Updating modem %s", syspath);
> +
> + for (i = 0; driver_list[i].name; i++) {
> + if (g_str_equal(driver_list[i].name, modem->driver) == FALSE)
> + continue;
> + if (driver_list[i].setup(modem) == FALSE)
> + continue;
> + }
> +}
> +static void update_device(struct udev_device *device)
> +{
> + const char *syspath;
> +
> + syspath = udev_device_get_syspath(device);
> + if (syspath == NULL)
> + return;
> +
> + DBG("%s", syspath);
> +
> + /* We only care about net iface renames for now */
> + if (g_strcmp0(udev_device_get_subsystem(device), "net") != 0)
> + return;
> +
> + /* Drop old device from modem */
> + g_hash_table_foreach(modem_list, drop_old_device, device);
> +
> + /* Add new device to modem */
> + check_device(device);
> +
> + /* Update modified modems */
> + g_hash_table_foreach(modem_list, update_modem, NULL);
> +}
> +
> static gboolean udev_event(GIOChannel *channel, GIOCondition cond,
> gpointer user_data)
> {
> @@ -1936,8 +2015,11 @@ static gboolean udev_event(GIOChannel *channel,
> GIOCondition cond,
> check_device(device);
>
> udev_delay = g_timeout_add_seconds(1, check_modem_list, NULL);
> - } else if (g_str_equal(action, "remove") == TRUE)
> + } else if (g_str_equal(action, "remove") == TRUE) {
> remove_device(device);
> + } else if (g_str_equal(action, "move") == TRUE) {
> + update_device(device);
> + }
>
> udev_device_unref(device);
Wouldn’t it generally be better for reading the name after oFono has brought up
the interface. In case we need this for the D-Bus API somehow. And for sake of
matching we start and handing this to the plugins and drivers we should start
using the ifindex.
Regards
Marcel
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
