[PATCH v4 0/3] tty/serial: Process XON/XOFF robustly

Ilpo Järvinen posted 3 patches 4 years ago
drivers/tty/n_tty.c        | 107 ++++++++++++++++++++++++++++---------
drivers/tty/tty_buffer.c   |  59 ++++++++++++++++----
drivers/tty/tty_port.c     |  21 ++++++++
include/linux/tty_buffer.h |   1 +
include/linux/tty_ldisc.h  |  13 +++++
include/linux/tty_port.h   |   2 +
6 files changed, 170 insertions(+), 33 deletions(-)
[PATCH v4 0/3] tty/serial: Process XON/XOFF robustly
Posted by Ilpo Järvinen 4 years ago
For this v4, I dropped Gilles' Tested-by as I made rather major
modifications to the patch series so I don't want to claim he has
tested this version (earlier versions are known to fix his problem and
this one should too).

XON/XOFF are used for software flow-control on serial lines. XON and
XOFF appear as characters within the stream but should be processed as
soon as possible.

The characters received by the UART drivers are in intermediate buffers
until TTY receives them. In the case where the TTY is not read from,
the characters may get stuck into those intermediate buffers until
user-space reads from the TTY. Among the characters stuck in the
buffers, can also be those XON/XOFF flow control characters. A stuck
flow-control character is not very useful.

This patch series addresses the issue by checking if TTY is slow to
process characters, that is, eats less than the given amount. If TTY is
slow, a lookahead is invoked for the characters that remain in the
intermediate buffer(s).

Then at a later time, receive_buf needs to ensure the flow-control
actions are not retaken when those same characters get pushed to TTY.

This patch series fixes an issue but I'm not able to pinpoint to a
specific commit id to provide a Fixes tag. The last patch of the series
is not needed for minimal fix (and has a small chance of regression
too), thus that patch shouldn't be sent to stable.

v1 -> v2:
- Add flow ctrl char funcs in separate change & rework logic a bit.
  I believe it's now cleaner than it would have been with the
  suggestions during v1 review, please recheck.
- Renamed n_tty_lookahead_special to n_tty_lookahead_flow_ctrl
- Fixed logic for START_CHAR == STOP_CHAR case
- Use unsigned int for lookahead_count in receive_buf call chain
- Use consistent types in lookahead call chain
- Improved indentation & line splits for function params
- Corrected tty_ldisc.h comments documenting tty_ldisc_ops
- Tweaked comment format

v2 -> v3:
- Split preparatory patch moving/rearranging code to two
- Fix closing path giving change for ... || xx to execute
  instead of skipping the flow-control char
- Use the same flow-control char function on closing path
  (just a cleanup, non-fix as last patch, a small change of
  regression exists)

v3 -> v4:
- Rework lookahead_count, it is now kept internal to n_tty ldisc rather
  than passed around through the whole callchain
- Dropped Gilles' Tested-by due to major changes
- Improve comments & changelogs

Ilpo Järvinen (3):
  tty: Rework receive flow control char logic
  tty: Implement lookahead to process XON/XOFF timely
  tty: Use flow-control char function on closing path

 drivers/tty/n_tty.c        | 107 ++++++++++++++++++++++++++++---------
 drivers/tty/tty_buffer.c   |  59 ++++++++++++++++----
 drivers/tty/tty_port.c     |  21 ++++++++
 include/linux/tty_buffer.h |   1 +
 include/linux/tty_ldisc.h  |  13 +++++
 include/linux/tty_port.h   |   2 +
 6 files changed, 170 insertions(+), 33 deletions(-)

-- 
2.30.2

Re: [PATCH v4 0/3] tty/serial: Process XON/XOFF robustly
Posted by Greg KH 3 years, 11 months ago
On Tue, Apr 26, 2022 at 05:49:32PM +0300, Ilpo Järvinen wrote:
> For this v4, I dropped Gilles' Tested-by as I made rather major
> modifications to the patch series so I don't want to claim he has
> tested this version (earlier versions are known to fix his problem and
> this one should too).
> 
> XON/XOFF are used for software flow-control on serial lines. XON and
> XOFF appear as characters within the stream but should be processed as
> soon as possible.
> 
> The characters received by the UART drivers are in intermediate buffers
> until TTY receives them. In the case where the TTY is not read from,
> the characters may get stuck into those intermediate buffers until
> user-space reads from the TTY. Among the characters stuck in the
> buffers, can also be those XON/XOFF flow control characters. A stuck
> flow-control character is not very useful.
> 
> This patch series addresses the issue by checking if TTY is slow to
> process characters, that is, eats less than the given amount. If TTY is
> slow, a lookahead is invoked for the characters that remain in the
> intermediate buffer(s).
> 
> Then at a later time, receive_buf needs to ensure the flow-control
> actions are not retaken when those same characters get pushed to TTY.
> 
> This patch series fixes an issue but I'm not able to pinpoint to a
> specific commit id to provide a Fixes tag. The last patch of the series
> is not needed for minimal fix (and has a small chance of regression
> too), thus that patch shouldn't be sent to stable.
> 
> v1 -> v2:
> - Add flow ctrl char funcs in separate change & rework logic a bit.
>   I believe it's now cleaner than it would have been with the
>   suggestions during v1 review, please recheck.
> - Renamed n_tty_lookahead_special to n_tty_lookahead_flow_ctrl
> - Fixed logic for START_CHAR == STOP_CHAR case
> - Use unsigned int for lookahead_count in receive_buf call chain
> - Use consistent types in lookahead call chain
> - Improved indentation & line splits for function params
> - Corrected tty_ldisc.h comments documenting tty_ldisc_ops
> - Tweaked comment format
> 
> v2 -> v3:
> - Split preparatory patch moving/rearranging code to two
> - Fix closing path giving change for ... || xx to execute
>   instead of skipping the flow-control char
> - Use the same flow-control char function on closing path
>   (just a cleanup, non-fix as last patch, a small change of
>   regression exists)
> 
> v3 -> v4:
> - Rework lookahead_count, it is now kept internal to n_tty ldisc rather
>   than passed around through the whole callchain
> - Dropped Gilles' Tested-by due to major changes
> - Improve comments & changelogs
> 
> Ilpo Järvinen (3):
>   tty: Rework receive flow control char logic
>   tty: Implement lookahead to process XON/XOFF timely
>   tty: Use flow-control char function on closing path
> 
>  drivers/tty/n_tty.c        | 107 ++++++++++++++++++++++++++++---------
>  drivers/tty/tty_buffer.c   |  59 ++++++++++++++++----
>  drivers/tty/tty_port.c     |  21 ++++++++
>  include/linux/tty_buffer.h |   1 +
>  include/linux/tty_ldisc.h  |  13 +++++
>  include/linux/tty_port.h   |   2 +
>  6 files changed, 170 insertions(+), 33 deletions(-)
> 
> -- 
> 2.30.2
> 

I took patch 1/3 of this series as it is "obviously correct".  I'd like
others to review the rest before taking them, and I don't have the
bandwidth today to do so, so I'll wait for others before considering
them.

thanks,

greg k-h