On Sun, 2018-05-27 at 17:03 +0200, Aleksander Morgado wrote:
> 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.

Unfortunate, but LGTM.

Dan

> ---
> 
> 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
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel

Reply via email to