modemmanager: fix device unplug handling
authorAleksander Morgado <aleksander@aleksander.es>
Wed, 20 Nov 2019 11:49:11 +0000 (12:49 +0100)
committerAleksander Morgado <aleksander@aleksander.es>
Wed, 27 Nov 2019 08:28:10 +0000 (09:28 +0100)
When a USB modem device is unplugged, we had to do two different
things: first, cleanup the sysfs cache; and second, set interface as
unavailable.

Those two things were never being done properly due to several
different issues:

 * The parent sysfs path retrieval logic relies on checking for which
 sysfs path has the vid/pid files. This logic obviously only works
 when the device is available, and cannot be used on e.g. removal
 events.

 * The command to cleanup the modem wait status from the sysfs cache
 was not removing the previous state properly, because the sysfs path
 variable wasn't escaped properly for the sed command.

This patch handles those issues in order to have a proper device
removal handling, by making sure the sysfs path is properly escaped in
the sed command, and by introducing a new hotplug script that runs
when the full USB device is removed.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
net/modemmanager/Makefile
net/modemmanager/files/25-modemmanager-usb [new file with mode: 0644]
net/modemmanager/files/modemmanager.common

index 952f7c3e7f8468d64cc5434ef9fccad9738a8460..0c73a8880be9f46354b25a180758707efd9ef544 100644 (file)
@@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=modemmanager
 PKG_VERSION:=1.12.0
-PKG_RELEASE:=5
+PKG_RELEASE:=6
 
 PKG_SOURCE:=ModemManager-$(PKG_VERSION).tar.xz
 PKG_SOURCE_URL:=https://www.freedesktop.org/software/ModemManager
@@ -123,6 +123,9 @@ define Package/modemmanager/install
        $(INSTALL_DIR) $(1)/etc/init.d
        $(INSTALL_BIN) ./files/modemmanager.init $(1)/etc/init.d/modemmanager
 
+       $(INSTALL_DIR) $(1)/etc/hotplug.d/usb
+       $(INSTALL_DATA) ./files/25-modemmanager-usb $(1)/etc/hotplug.d/usb
+
        $(INSTALL_DIR) $(1)/etc/hotplug.d/net
        $(INSTALL_DATA) ./files/25-modemmanager-net $(1)/etc/hotplug.d/net
 
diff --git a/net/modemmanager/files/25-modemmanager-usb b/net/modemmanager/files/25-modemmanager-usb
new file mode 100644 (file)
index 0000000..8ca8ba1
--- /dev/null
@@ -0,0 +1,13 @@
+#!/bin/sh
+# Copyright (C) 2019 Aleksander Morgado <aleksander@aleksander.es>
+
+# We need to process only full USB device removal events, we don't
+# want to process specific interface removal events.
+[ "$ACTION" = remove ] || exit
+[ -z "${INTERFACE}" ] || exit
+
+# Load common utilities
+. /etc/modemmanager/modemmanager.common
+
+mm_clear_modem_wait_status "/sys${DEVPATH}"
+mm_cleanup_interface_by_sysfspath "/sys${DEVPATH}"
index 2f882e496f0f7ac009ec0321b5c8ce0d9d8de52e..532d90ac006348e845191d7d1f1cab4f8421dc9d 100644 (file)
@@ -26,6 +26,9 @@ mm_log() {
 ################################################################################
 # Receives as input argument the full sysfs path of the device
 # Returns the physical device sysfs path
+#
+# NOTE: this method only works when the device exists, i.e. it cannot be used
+# on removal hotplug events
 
 mm_find_physdev_sysfs_path() {
        local tmp_path="$1"
@@ -96,14 +99,26 @@ mm_get_modem_wait_status() {
        awk -v sysfspath="${sysfspath}" '!/^#/ && $0 ~ sysfspath { print $2 }' "${MODEMMANAGER_SYSFS_CACHE}"
 }
 
+# Clear the modem wait status from the cache, if any
+mm_clear_modem_wait_status() {
+       local sysfspath="$1"
+
+       local escaped_sysfspath
+
+       [ -f "${MODEMMANAGER_SYSFS_CACHE}" ] && {
+               # escape '/', '\' and '&' for sed...
+               escaped_sysfspath=$(echo "$sysfspath" | sed -e 's/[\/&]/\\&/g')
+               sed -i "/${escaped_sysfspath}/d" "${MODEMMANAGER_SYSFS_CACHE}"
+       }
+}
+
 # Sets the modem wait status in the cache
 mm_set_modem_wait_status() {
        local sysfspath="$1"
        local status="$2"
 
        # Remove sysfs line before adding the new one with the new state
-       [ -f "${MODEMMANAGER_SYSFS_CACHE}" ] &&
-               sed -i "/${sysfspath}/d" "${MODEMMANAGER_SYSFS_CACHE}"
+       mm_clear_modem_wait_status "${sysfspath}"
 
        # Add the new status
        echo "${sysfspath} ${status}" >> "${MODEMMANAGER_SYSFS_CACHE}"
@@ -171,8 +186,7 @@ mm_wait_for_modem() {
 }
 
 mm_report_modem_wait() {
-       local action=$1
-       local sysfspath=$2
+       local sysfspath=$1
 
        local parent_sysfspath status
 
@@ -183,53 +197,31 @@ mm_report_modem_wait() {
        }
 
        status=$(mm_get_modem_wait_status "${parent_sysfspath}")
-
-       [ "$action" = "add" ] && {
-               case "${status}" in
-                       "")
-                               local cfg
-
-                               cfg=$(mm_get_modem_config "${parent_sysfspath}")
-                               if [ -n "${cfg}" ]; then
-                                       mm_log "interface '${cfg}' is set to configure device '${parent_sysfspath}'"
-                                       mm_log "now waiting for modem at sysfs path ${parent_sysfspath}"
-                                       mm_set_modem_wait_status "${parent_sysfspath}" "processed"
-                                       # Launch subshell for the explicit wait
-                                       ( mm_wait_for_modem "${cfg}" "${parent_sysfspath}" ) > /dev/null 2>&1 &
-                               else
-                                       mm_log "no need to wait for modem at sysfs path ${parent_sysfspath}"
-                                       mm_set_modem_wait_status "${parent_sysfspath}" "ignored"
-                               fi
-                               ;;
-                       "processed")
-                               mm_log "already waiting for modem at sysfs path ${parent_sysfspath}"
-                               ;;
-                       "ignored")
-                               ;;
-                       *)
-                               mm_log "error: unknown status read for device at sysfs path ${parent_sysfspath}"
-                               ;;
-               esac
-               return
-       }
-
-       [ "$action" = "remove" ] && {
-               local cfg
-
-               [ -n "$status" ] && {
+       case "${status}" in
+               "")
                        local cfg
 
-                       mm_log "cleanup wait for modem at sysfs path ${parent_sysfspath}"
-                       mm_set_modem_wait_status "${parent_sysfspath}" ""
-
                        cfg=$(mm_get_modem_config "${parent_sysfspath}")
-                       [ -n "${cfg}" ] && {
-                               mm_log "setting interface '$cfg' as unavailable"
-                               proto_set_available "${cfg}" 0
-                       }
-               }
-               return
-       }
+                       if [ -n "${cfg}" ]; then
+                               mm_log "interface '${cfg}' is set to configure device '${parent_sysfspath}'"
+                               mm_log "now waiting for modem at sysfs path ${parent_sysfspath}"
+                               mm_set_modem_wait_status "${parent_sysfspath}" "processed"
+                               # Launch subshell for the explicit wait
+                               ( mm_wait_for_modem "${cfg}" "${parent_sysfspath}" ) > /dev/null 2>&1 &
+                       else
+                               mm_log "no need to wait for modem at sysfs path ${parent_sysfspath}"
+                               mm_set_modem_wait_status "${parent_sysfspath}" "ignored"
+                       fi
+                       ;;
+               "processed")
+                       mm_log "already waiting for modem at sysfs path ${parent_sysfspath}"
+                       ;;
+               "ignored")
+                       ;;
+               *)
+                       mm_log "error: unknown status read for device at sysfs path ${parent_sysfspath}"
+                       ;;
+       esac
 }
 
 ################################################################################
@@ -250,6 +242,17 @@ mm_cleanup_interfaces() {
        config_foreach mm_cleanup_interface_cb interface
 }
 
+mm_cleanup_interface_by_sysfspath() {
+       local dev="$1"
+
+       local cfg
+       cfg=$(mm_get_modem_config "$dev")
+       [ -n "${cfg}" ] || return
+
+       mm_log "setting interface '$cfg' as unavailable"
+       proto_set_available "${cfg}" 0
+}
+
 ################################################################################
 # Event reporting
 
@@ -277,8 +280,8 @@ mm_report_event() {
        mm_log "event reported: action=${action}, name=${name}, subsystem=${subsystem}"
        mmcli --report-kernel-event="action=${action},name=${name},subsystem=${subsystem}" 1>/dev/null 2>&1 &
 
-       # Wait for modem if a sysfspath is given
-       [ -n "${sysfspath}" ] && mm_report_modem_wait "${action}" "${sysfspath}"
+       # Wait for added modem if a sysfspath is given
+       [ -n "${sysfspath}" ] && [ "$action" = "add" ] && mm_report_modem_wait "${sysfspath}"
 }
 
 mm_report_event_from_cache_line() {