public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/1] tty: n_gsm: improve standard compliance and feature completeness
@ 2022-02-25  8:07 Daniel Starke
  2022-02-25  8:07 ` [RFC 1/1] " Daniel Starke
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Starke @ 2022-02-25  8:07 UTC (permalink / raw)
  To: linux-serial, gregkh, jirislaby; +Cc: linux-kernel, Daniel Starke

We as Siemens Mobility GmbH are using the n_gsm module as specified by
3GPP 27.010 and required by UNISIG SUBSET-037 chapter 6 of the European
train control system standard to establish multiple parallel connections to
GSM-R radio mobiles as used in our communication system.
This is needed to handle CS (circuit switched) and PS (packet switched)
services and functions in parallel. It requires at least four multiplexed
data link connections (DLCs), including the control channel. It also
requires the use of convergence layer type 2 in advanced option mode.

We have improved support for this use case and the overall stability of the
driver to achieve this. The modified driver was extensively tested and is
already included in our productive system. We have integrated it with
mobiles from Funkwerk Systems GmbH and Triorail Bahnfunk GmbH.
A rough summary of changes made can be found below. The patch presented
here shall provide a first impression of the planned upcoming commits and
allow others to test and comment on these at an early stage. We plan to
separate these changes and commit smaller change sets of this. But this
is still work in progress.

Our motivation of this commitment to the community lies in a smoother
migration to upcoming kernel versions, a broader use for a better test
coverage and an overall better quality of the driver. In a first step a set
of bug fixes unrelated to the new functions has already been committed.

[CR] Changes included:
[1] added: proper n_gsm Kernel module information fields and module version
[2] added: optional start-of-frame flag skipping in advanced option mode
[3] added: UI (unnumbered information) frame support
[4] added: PN (parameter negotiation) message handling and function support
[5] added: optional keep-alive control link supervision via Test messages
[6] added: proc fs files to provide meta data for the n_gsm instances
[7] added: various parameter gated kernel debug messages
[8] added: option for strict mode to reject non-standard compliant behavior
[9] added: TIOCM_OUT1 and TIOCM_OUT2 to allow responder to operate as modem
[10] added: TIOCMIWAIT support on virtual ttys
[11] added: additional ioctls and parameters to configure the new functions
[12] added: more function comments and aligned all to the coding guidelines
[13] changed: overall locking mechanism to avoid data race conditions
[14] changed: outgoing data flow to decouple physical from virtual tty
     handling for better performance and to avoid dead-locks
[15] fixed: advanced option mode implementation
[16] fixed: convergence layer type 2 implementation
[17] fixed: handling of CLD (multiplexer close down) messages
[18] fixed: broken muxer close down procedure

[NO] Points that remain open and are out of scope here:
[1] DLCI priority handling according to the configured values. The priority
    is currently only handled according to the DLCI number.
[2] correct flow control handling (the standard is ambiguous, ch. 5.4.8.1)
[3] combined opening and closing flag handling in basic option mode
    (ch. 5.2.6.1; the data can be ambiguous)
[4] power control services (ch. 5.1.5 and 5.4.7)
[5] error recovery mode option (ch. 6)
[6] convergence layer type 3 and 4 (ch. 5.5.3 and 5.5.4)
[7] remote port negotiation (ch. 5.1.8.2.1 and 5.4.6.3.9)
[8] service negotiation (ch. 5.1.8.1.2 and 5.4.6.3.11)

The referenced chapters can be found in 3GPP TS 27.010. See link below.

[X] Furthermore, the following commits conflict with this implementation:
[1] commit 509067bbd264 ("tty: n_gsm: Delete gsm_disconnect when config requester")
[2] commit 5b87686e3203 ("tty: n_gsm: Modify gsmtty driver register method when config requester")
[3] commit 0b91b5332368 ("tty: n_gsm: Save dlci address open status when config requester")
[4] commit 46292622ad73 ("tty: n_gsm: clean up indenting in gsm_queue()")

[X1] conflicts with the new implementation for parameter negotiation.
[X2], [X3] and [X4] implement dynamic virtual tty registration to allow
uevents on the responder side. However, on responder side an application
may first want to configure the link before it is established. That is why
the virtual ttys need to be available directly after the control channel
establishment. The responder still gets informed about a line open by
waiting for DTR (data terminal ready) to be set. Just like on a physical
serial interface.

Please provide comments and hints for the integration of these changes into
the upcoming Linux kernel. Feel free to refer to specific points by the
given identifier (e.g. CR11 for TIOCMIWAIT support).

With best regards,
Daniel Starke

Link: https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.aspx?specificationId=1516
Link: https://www.era.europa.eu/sites/default/files/filesystem/ertms/ccs_tsi_annex_a_-_mandatory_specifications/set_of_specifications_3_etcs_b3_r2_gsm-r_b1/index010_-_subset-037_v320.pdf

Daniel Starke (1):
  tty: n_gsm: improve standard compliance and feature completeness

 drivers/tty/n_gsm.c         | 4523 +++++++++++++++++++++++++----------
 include/uapi/linux/gsmmux.h |   70 +-
 2 files changed, 3364 insertions(+), 1229 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [RFC 1/1] tty: n_gsm: improve standard compliance and feature completeness
@ 2022-03-02 14:47 Starke, Daniel
  2022-03-02 14:59 ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Starke, Daniel @ 2022-03-02 14:47 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-serial@vger.kernel.org, jirislaby@kernel.org,
	linux-kernel@vger.kernel.org

Thank you for this quick response.

> I'm sorry, but there is nothing we can do with such a large patch here at
> all.
> 
> Please break this up into "one logical change per patch" and we will be
> glad to review it.

You are absolutely right but this is still work in progress. The attached
patch was only for reference for all those who would like to look at the
changes in detail or try out the proposed work. This was not a request for
code review yet. Also, it seems that the patch was too large for the
linux-serial mailing list so I have uploaded it here for reference:
https://github.com/siemens/linux/tree/dstarke-siemens/n_gsm_rfc

> Put your "fixes" at the beginning of the patch series, so they can be
> properly backported to older kernels as needed. Don't mix up new features
> with fixes as that means that the fixes can never be backported.

All fixes that have no dependency on the upcoming features have been
submitted by me already. I will double check the remaining fixes but I
could not find a way to separate those from the new features so far.

> One note, please drop the useless MODULE_VERSION() that never belongs in
> an in-kernel driver as it makes no sense (the version is the kernel
> version).

This helped us during our internal development and test procedure. But I
will remove it before submitting the actual patches.

> Same for the license "boiler-plate" text, that should not be added back,
> we removed it for a reason.

Will be removed.

> Also, no module parameters should be added. Especially pointless ones
> like debugging flags, use the built-in kernel debugging options that the
> whole rest of the kernel uses. There is no need for a special case for
> just one tiny line discipline module :)

The module parameter "debug" is not new. It is included in all kernel
version. And I did not plan to remove it. Especially, since this protocol
is complex and hard to debug with the provided kernel debugging options
only.
My addition was the parameter "strict" to verify standard conformance of
connected devices with the underlying protocol. Otherwise, the protocol
gracefully handles invalid values. Would you suggest to make this parameter
an ioctl instead?

With best regards,
Daniel Starke

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-02 15:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-25  8:07 [RFC 0/1] tty: n_gsm: improve standard compliance and feature completeness Daniel Starke
2022-02-25  8:07 ` [RFC 1/1] " Daniel Starke
2022-02-25  8:29   ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2022-03-02 14:47 Starke, Daniel
2022-03-02 14:59 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox