[PATCH] udev: add tags also on bind action

Sun May 27 16:16:25 UTC 2018

Never heard of "bind" events before.  Googling them led me straight to
https://github.com/systemd/systemd/issues/8221
which tells me that that I haven't been paying much attention for the
last 5 kernel releases...

Except for that, this looks like a systemd bug.  The Debian bug is
reported agains v237, so your v236 will be affected.

I will test your patch, but adding "bind" to every existing rule for
every device and every application cannot be right?  This should be
handled by systemd-udev.

Bjørn

Aleksander Morgado <aleksander at aleksander.es> writes:

> When a new USB device is hotplugged, e.g. a USB<->RS232 converter that
> exposes a single ttyUSB0, these udev events happen:
>
>   add  /devices/pci0000:00/0000:00:14.0/usb2/2-1 (usb/usb-device)
>   add  /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0 (usb/usb-interface)
>   add  /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0/ttyUSB0 (usb-serial)
>   add  /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0/ttyUSB0/tty/ttyUSB0 (tty)
>   bind /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0/ttyUSB0 (usb-serial)
>   bind /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0 (usb/usb-interface)
>   bind /devices/pci0000:00/0000:00:14.0/usb2/2-1 (usb/usb-device)
>
> Our udev rules in MM only added tags in the 'add' events, and it looks
> like the only ones 'persistent' after this sequence are those of the
> last event happening on the specific path.
>
> This meant that all TTY subsystem rules (e.g. ID_MM_CANDIDATE) would
> be stored for later check (e.g. if ModemManager is started after these
> rules have been applied), which was ok. "udevadm info -p ..." would
> show these tags correctly always.
>
> But this also meant that the 'bind' udev event happening for the USB
> device didn't get any of our device-specific tags, and so we would be
> missing them (e.g. ID_MM_DEVICE_MANUAL_SCAN_ONLY) if MM is started
> after the last event has happened. "udevadm info -p ..." would
> not show these tags.
>
> Modify all our rules to also run at the 'bind' events.
> ---
>
> Hey Bjørn, Dan, Ben and all,
>
> I have no idea if this is something that has recently changed in systemd/udev (I'm using systemd 236.81-1 in Arch Linux), or if it's some other issue triggered by my setup, so I would like to ask you all a favor and review if this makes sense or if you can reproduce it...
>
> The thing is, with our current set of rules, I could see how the port-specific udev tags (those applied in the ttyUSBx device for example) would be persistent and browsable with "udevadm info -p /sys...". But the device-specific tags (those applied to the "usb-device") wouldn't be persistent.
>
> The outcome of this is that if ModemManager gets the events while it's running, everything works perfectly, because for the 'udev-device' we would be notified of both 'add' and 'bind' events. But if MM starts after the events have happened, ModemManager only gets the last event ('bind' in this case) and so we would lose the device-specific tags because we were not adding them in the bind event. Does this make sense?
>
> There's no need to reboot laptop to test this ;) you can just: stop ModemManager, plug in e.g. a USB<->serial adapter (which shouldn't be probed automatically), and then start ModemManager. If the port is probed automatically, the tag wasn't applied. Running "udevadm monitor -e" during the hotplug event also helps to see the contents of each of the events.
>
> What do you all think?
>
> ---
>  plugins/cinterion/77-mm-cinterion-port-types.rules | 2 +-
>  plugins/dell/77-mm-dell-port-types.rules           | 2 +-
>  plugins/haier/77-mm-haier-port-types.rules         | 2 +-
>  plugins/huawei/77-mm-huawei-net-port-types.rules   | 2 +-
>  plugins/longcheer/77-mm-longcheer-port-types.rules | 2 +-
>  plugins/mbm/77-mm-ericsson-mbm.rules               | 2 +-
>  plugins/mtk/77-mm-mtk-port-types.rules             | 2 +-
>  plugins/nokia/77-mm-nokia-port-types.rules         | 2 +-
>  plugins/sierra/77-mm-sierra.rules                  | 2 +-
>  plugins/simtech/77-mm-simtech-port-types.rules     | 2 +-
>  plugins/telit/77-mm-telit-port-types.rules         | 2 +-
>  plugins/ublox/77-mm-ublox-port-types.rules         | 2 +-
>  plugins/x22x/77-mm-x22x-port-types.rules           | 2 +-
>  plugins/zte/77-mm-zte-port-types.rules             | 2 +-
>  src/77-mm-pcmcia-device-blacklist.rules            | 2 +-
>  src/77-mm-usb-device-blacklist.rules               | 2 +-
>  src/77-mm-usb-serial-adapters-greylist.rules       | 2 +-
>  src/80-mm-candidate.rules                          | 2 +-
>  18 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/plugins/cinterion/77-mm-cinterion-port-types.rules b/plugins/cinterion/77-mm-cinterion-port-types.rules
> index 7c63a1aa..7ef80719 100644
> --- a/plugins/cinterion/77-mm-cinterion-port-types.rules
> +++ b/plugins/cinterion/77-mm-cinterion-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_cinterion_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_cinterion_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1e2d", GOTO="mm_cinterion_port_types"
>  GOTO="mm_cinterion_port_types_end"
>
> diff --git a/plugins/dell/77-mm-dell-port-types.rules b/plugins/dell/77-mm-dell-port-types.rules
> index 206fb6e1..eb271fc7 100644
> --- a/plugins/dell/77-mm-dell-port-types.rules
> +++ b/plugins/dell/77-mm-dell-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_dell_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_dell_port_types_end"
>
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="413c", GOTO="mm_dell_vendorcheck"
>  GOTO="mm_dell_port_types_end"
> diff --git a/plugins/haier/77-mm-haier-port-types.rules b/plugins/haier/77-mm-haier-port-types.rules
> index d095847f..35c2166f 100644
> --- a/plugins/haier/77-mm-haier-port-types.rules
> +++ b/plugins/haier/77-mm-haier-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_haier_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_haier_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="201e", GOTO="mm_haier_port_types"
>  GOTO="mm_haier_port_types_end"
>
> diff --git a/plugins/huawei/77-mm-huawei-net-port-types.rules b/plugins/huawei/77-mm-huawei-net-port-types.rules
> index 25747081..5bf99cf1 100644
> --- a/plugins/huawei/77-mm-huawei-net-port-types.rules
> +++ b/plugins/huawei/77-mm-huawei-net-port-types.rules
> @@ -1,5 +1,5 @@
>  # do not edit this file, it will be overwritten on update
> -ACTION!="add|change|move", GOTO="mm_huawei_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_huawei_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="12d1", GOTO="mm_huawei_port_types"
>  GOTO="mm_huawei_port_types_end"
>
> diff --git a/plugins/longcheer/77-mm-longcheer-port-types.rules b/plugins/longcheer/77-mm-longcheer-port-types.rules
> index 512c3ded..9d29453f 100644
> --- a/plugins/longcheer/77-mm-longcheer-port-types.rules
> +++ b/plugins/longcheer/77-mm-longcheer-port-types.rules
> @@ -12,7 +12,7 @@
>  # cmser.inf lists the aux ports that may be either AT-capable or not but
>  # cannot be used for PPP.
>
> -ACTION!="add|change|move", GOTO="mm_longcheer_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_longcheer_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1c9e", GOTO="mm_longcheer_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1bbb", GOTO="mm_tamobile_vendorcheck"
>  GOTO="mm_longcheer_port_types_end"
> diff --git a/plugins/mbm/77-mm-ericsson-mbm.rules b/plugins/mbm/77-mm-ericsson-mbm.rules
> index 2065e0d5..13a40c48 100644
> --- a/plugins/mbm/77-mm-ericsson-mbm.rules
> +++ b/plugins/mbm/77-mm-ericsson-mbm.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_mbm_end"
> +ACTION!="add|change|move|bind", GOTO="mm_mbm_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="0bdb", GOTO="mm_mbm_ericsson_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="0fce", GOTO="mm_mbm_sony_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="413c", GOTO="mm_mbm_dell_vendorcheck"
> diff --git a/plugins/mtk/77-mm-mtk-port-types.rules b/plugins/mtk/77-mm-mtk-port-types.rules
> index dd8a067a..8b226b29 100644
> --- a/plugins/mtk/77-mm-mtk-port-types.rules
> +++ b/plugins/mtk/77-mm-mtk-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_mtk_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_mtk_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="0e8d", GOTO="mm_mtk_port_types_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="2001", GOTO="mm_dlink_port_types_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="07d1", GOTO="mm_dlink_port_types_vendorcheck"
> diff --git a/plugins/nokia/77-mm-nokia-port-types.rules b/plugins/nokia/77-mm-nokia-port-types.rules
> index a3e9fdb8..853443bb 100644
> --- a/plugins/nokia/77-mm-nokia-port-types.rules
> +++ b/plugins/nokia/77-mm-nokia-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_nokia_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_nokia_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="0421", GOTO="mm_nokia_port_types"
>  GOTO="mm_nokia_port_types_end"
>
> diff --git a/plugins/sierra/77-mm-sierra.rules b/plugins/sierra/77-mm-sierra.rules
> index cea83bdb..d3da313b 100644
> --- a/plugins/sierra/77-mm-sierra.rules
> +++ b/plugins/sierra/77-mm-sierra.rules
> @@ -1,6 +1,6 @@
>
>  # do not edit this file, it will be overwritten on update
> -ACTION!="add|change|move", GOTO="mm_sierra_end"
> +ACTION!="add|change|move|bind", GOTO="mm_sierra_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1199", GOTO="mm_sierra"
>  GOTO="mm_sierra_end"
>
> diff --git a/plugins/simtech/77-mm-simtech-port-types.rules b/plugins/simtech/77-mm-simtech-port-types.rules
> index d0a9a6fc..27ec6d93 100644
> --- a/plugins/simtech/77-mm-simtech-port-types.rules
> +++ b/plugins/simtech/77-mm-simtech-port-types.rules
> @@ -10,7 +10,7 @@
>  # *ser.inf lists the aux ports that may be used for PPP.
>
>
> -ACTION!="add|change|move", GOTO="mm_simtech_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_simtech_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1e0e", GOTO="mm_simtech_port_types"
>  GOTO="mm_simtech_port_types_end"
>
> diff --git a/plugins/telit/77-mm-telit-port-types.rules b/plugins/telit/77-mm-telit-port-types.rules
> index b052962f..1a77b3c0 100644
> --- a/plugins/telit/77-mm-telit-port-types.rules
> +++ b/plugins/telit/77-mm-telit-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_telit_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_telit_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1bc7", GOTO="mm_telit_port_types"
>  GOTO="mm_telit_port_types_end"
>
> diff --git a/plugins/ublox/77-mm-ublox-port-types.rules b/plugins/ublox/77-mm-ublox-port-types.rules
> index 168d1571..ab4ec4ad 100644
> --- a/plugins/ublox/77-mm-ublox-port-types.rules
> +++ b/plugins/ublox/77-mm-ublox-port-types.rules
> @@ -1,5 +1,5 @@
>  # do not edit this file, it will be overwritten on update
> -ACTION!="add|change|move", GOTO="mm_ublox_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_ublox_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1546", GOTO="mm_ublox_port_types"
>  GOTO="mm_ublox_port_types_end"
>
> diff --git a/plugins/x22x/77-mm-x22x-port-types.rules b/plugins/x22x/77-mm-x22x-port-types.rules
> index 52a7cbcc..bbdae91b 100644
> --- a/plugins/x22x/77-mm-x22x-port-types.rules
> +++ b/plugins/x22x/77-mm-x22x-port-types.rules
> @@ -8,7 +8,7 @@
>  # aux ports that may be either AT-capable or not but cannot be used for PPP.
>
>
> -ACTION!="add|change|move", GOTO="mm_x22x_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_x22x_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="1bbb", GOTO="mm_x22x_generic_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="0b3c", GOTO="mm_x22x_olivetti_vendorcheck"
>  GOTO="mm_x22x_port_types_end"
> diff --git a/plugins/zte/77-mm-zte-port-types.rules b/plugins/zte/77-mm-zte-port-types.rules
> index de6642fd..bc2e0ac1 100644
> --- a/plugins/zte/77-mm-zte-port-types.rules
> +++ b/plugins/zte/77-mm-zte-port-types.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_zte_port_types_end"
> +ACTION!="add|change|move|bind", GOTO="mm_zte_port_types_end"
>  SUBSYSTEMS=="usb", ATTRS{idVendor}=="19d2", GOTO="mm_zte_port_types"
>  GOTO="mm_zte_port_types_end"
>
> diff --git a/src/77-mm-pcmcia-device-blacklist.rules b/src/77-mm-pcmcia-device-blacklist.rules
> index f0f8474f..b496a8e5 100644
> --- a/src/77-mm-pcmcia-device-blacklist.rules
> +++ b/src/77-mm-pcmcia-device-blacklist.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_pcmcia_device_blacklist_end"
> +ACTION!="add|change|move|bind", GOTO="mm_pcmcia_device_blacklist_end"
>  SUBSYSTEM!="pcmcia", GOTO="mm_pcmcia_device_blacklist_end"
>
>  # Gemplus Serial Port smartcard adapter
> diff --git a/src/77-mm-usb-device-blacklist.rules b/src/77-mm-usb-device-blacklist.rules
> index e06643ad..854739a6 100644
> --- a/src/77-mm-usb-device-blacklist.rules
> +++ b/src/77-mm-usb-device-blacklist.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_usb_device_blacklist_end"
> +ACTION!="add|change|move|bind", GOTO="mm_usb_device_blacklist_end"
>  SUBSYSTEM!="usb", GOTO="mm_usb_device_blacklist_end"
>
>  # APC UPS devices
> diff --git a/src/77-mm-usb-serial-adapters-greylist.rules b/src/77-mm-usb-serial-adapters-greylist.rules
> index f4f588bf..2dc6f7db 100644
> --- a/src/77-mm-usb-serial-adapters-greylist.rules
> +++ b/src/77-mm-usb-serial-adapters-greylist.rules
> @@ -1,6 +1,6 @@
>  # do not edit this file, it will be overwritten on update
>
> -ACTION!="add|change|move", GOTO="mm_usb_serial_adapters_greylist_end"
> +ACTION!="add|change|move|bind", GOTO="mm_usb_serial_adapters_greylist_end"
>  SUBSYSTEM!="usb", GOTO="mm_usb_serial_adapters_greylist_end"
>
>  # Belkin F5U183 Serial Adapter
> diff --git a/src/80-mm-candidate.rules b/src/80-mm-candidate.rules
> index 2f95e7d2..3d9cd6e7 100644
> --- a/src/80-mm-candidate.rules
> +++ b/src/80-mm-candidate.rules
> @@ -7,7 +7,7 @@
>  # that don't have this tag.  MM will still get the udev 'add' event for the
>  # device a short while later and then process it as normal.
>
> -ACTION!="add|change|move", GOTO="mm_candidate_end"
> +ACTION!="add|change|move|bind", GOTO="mm_candidate_end"
>
>  # Opening bound but disconnected Bluetooth RFCOMM ttys would initiate the
>  # connection. Don't do that.
> --
> 2.16.1