Re: [PATCH] USB: cdc-acm: expose serial close_delay and closing_wait in sysfs

[Simon Arlott]

On 24/08/2023 15:48, Greg Kroah-Hartman wrote:
> On Wed, Aug 23, 2023 at 09:37:45PM +0100, Simon Arlott wrote:
>> This can happen with the ESP32-H2/ESP32-C6 USB serial interface. Instead of
>> changing all userspace applications to flush (discard) their output in this
>> specific scenario it would be easier to adjust the closing_wait timeout
>> with udev rules but the only available interface is the TIOCGSERIAL ioctl.
> 
> Then why not use that?

It's not practical to use TIOCGSERIAL from a udev rule. Instead of a
sysfs attribute (that udev has built-in support for writing) it would
require a separate compiled process or other non-trivial dependencies
(e.g. Python) to modify the closing_wait value. There's no shell script
support for read-modify-write of a complex ioctl struct.

The ioctl can't be used without opening and closing the tty, which has
side effects. On open() it'll raise DTR/RTS and activate it. For cdc-acm
that will indicate to the device that the serial port has been opened
which will be visible to the software running on the USB device. On
close() it'll be delayed by the close_delay if any process is currently
doing a blocking open() and there's no carrier, then the closing_wait
time if there's been any incomplete transmitted data (by any process).

I want to be able to automatically set close_delay to 0 and closing_wait
to 65535 ("no waiting") on all USB serial devices without these kind of
side effects. I'm sure these have a purpose when connected to a real tty
but forcing a process to wait 30 seconds before it can close the port
(or exit) if the USB device isn't reading data properly is inconvenient.

Those two values require CAP_SYS_ADMIN to modify (which is separately
enforced by many of the tty drivers) so user applications can't change
them even if they're aware of them.

> If any apis are needed, let's make them for all tty devices, through the
> existing ioctl api, so they work for all devices and userspace doesn't
> have to try to figure out just exactly what type of tty/serial device it
> is talking to (as that will not scale and is exactly the opposite of
> what common apis are for.)

Are you ok with adding the same two attributes to sysfs for all ttys?

I'd use a different name (appending "_centisecs") because serial_core
already uses those names on the tty device and I think it's better to
define "infinite wait" and "no wait" as -1 and 0 instead of 0 and 65535.

-- 
Simon Arlott