2023-04-05 05:50:20

by D. Starke

[permalink] [raw]
Subject: [PATCH 2/9] tty: n_gsm: add restart parameter to DLC specific ioctl config

From: Daniel Starke <[email protected]>

'gsm_config_ext' already allows to force a link reset via 'restart'
parameter. An equivalent parameter for an forced channel reset is still
missing in 'gsm_dlci_config'. Therefore, the user has no means to perform
an automatic channel reset after parameter configuration for
non-conflicting changes. Conflicting changes automatically reset the
channel already in the current implementation.

Add the parameter 'restart' to 'gsm_dlci_config' to force a channel reset
after ioctl setting regardless of whether the changes made require this or
not.

Note that the parameter is limited to the values 0 and 1 to allow future
additions here.

Signed-off-by: Daniel Starke <[email protected]>
---
drivers/tty/n_gsm.c | 4 ++++
include/uapi/linux/gsmmux.h | 3 ++-
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c42c8b89fd46..eb21ca583642 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2531,6 +2531,8 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
return -EINVAL;
if (dc->k > 7)
return -EINVAL;
+ if (dc->restart > 1) /* allow future extensions */
+ return -EINVAL;

/*
* See what is needed for reconfiguration
@@ -2545,6 +2547,8 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
/* Requires care */
if (dc->priority != dlci->prio)
need_restart = true;
+ if (dc->restart)
+ need_restart = true;

if ((open && gsm->wait_config) || need_restart)
need_open = true;
diff --git a/include/uapi/linux/gsmmux.h b/include/uapi/linux/gsmmux.h
index eb67884e5f38..33ee7b857c52 100644
--- a/include/uapi/linux/gsmmux.h
+++ b/include/uapi/linux/gsmmux.h
@@ -58,7 +58,8 @@ struct gsm_dlci_config {
__u32 priority; /* Priority (0 for default value) */
__u32 i; /* Frame type (1 = UIH, 2 = UI) */
__u32 k; /* Window size (0 for default value) */
- __u32 reserved[8]; /* For future use, must be initialized to zero */
+ __u32 restart; /* Force DLCI channel reset? */
+ __u32 reserved[7]; /* For future use, must be initialized to zero */
};

#define GSMIOC_GETCONF_DLCI _IOWR('G', 7, struct gsm_dlci_config)
--
2.34.1


2023-04-05 08:31:09

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 2/9] tty: n_gsm: add restart parameter to DLC specific ioctl config

On Wed, 5 Apr 2023, D. Starke wrote:

> From: Daniel Starke <[email protected]>
>
> 'gsm_config_ext' already allows to force a link reset via 'restart'
> parameter.

Please be more precise, there a function and struct with this name.

HOWEVER, there is no such parameter in tty-next (am I looking from wrong
repo??):

static int gsm_config_ext(struct gsm_mux *gsm, struct gsm_config_ext *ce)

...nor here:

struct gsm_config_ext {
__u32 keep_alive; /* Control channel keep-alive in 1/100th of a
* second (0 to disable)
*/
__u32 wait_config; /* Wait for DLCI config before opening virtual link? */
__u32 reserved[6]; /* For future use, must be initialized to zero */
};

???


--
i.

> An equivalent parameter for an forced channel reset is still
> missing in 'gsm_dlci_config'. Therefore, the user has no means to perform
> an automatic channel reset after parameter configuration for
> non-conflicting changes. Conflicting changes automatically reset the
> channel already in the current implementation.
>
> Add the parameter 'restart' to 'gsm_dlci_config' to force a channel reset
> after ioctl setting regardless of whether the changes made require this or
> not.
>
> Note that the parameter is limited to the values 0 and 1 to allow future
> additions here.
>
> Signed-off-by: Daniel Starke <[email protected]>
> ---
> drivers/tty/n_gsm.c | 4 ++++
> include/uapi/linux/gsmmux.h | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c42c8b89fd46..eb21ca583642 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2531,6 +2531,8 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
> return -EINVAL;
> if (dc->k > 7)
> return -EINVAL;
> + if (dc->restart > 1) /* allow future extensions */
> + return -EINVAL;
>
> /*
> * See what is needed for reconfiguration
> @@ -2545,6 +2547,8 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
> /* Requires care */
> if (dc->priority != dlci->prio)
> need_restart = true;
> + if (dc->restart)
> + need_restart = true;
>
> if ((open && gsm->wait_config) || need_restart)
> need_open = true;
> diff --git a/include/uapi/linux/gsmmux.h b/include/uapi/linux/gsmmux.h
> index eb67884e5f38..33ee7b857c52 100644
> --- a/include/uapi/linux/gsmmux.h
> +++ b/include/uapi/linux/gsmmux.h
> @@ -58,7 +58,8 @@ struct gsm_dlci_config {
> __u32 priority; /* Priority (0 for default value) */
> __u32 i; /* Frame type (1 = UIH, 2 = UI) */
> __u32 k; /* Window size (0 for default value) */
> - __u32 reserved[8]; /* For future use, must be initialized to zero */
> + __u32 restart; /* Force DLCI channel reset? */
> + __u32 reserved[7]; /* For future use, must be initialized to zero */
> };
>
> #define GSMIOC_GETCONF_DLCI _IOWR('G', 7, struct gsm_dlci_config)
>

2023-04-06 05:19:16

by D. Starke

[permalink] [raw]
Subject: RE: [PATCH 2/9] tty: n_gsm: add restart parameter to DLC specific ioctl config

> > From: Daniel Starke <[email protected]>
> >
> > 'gsm_config_ext' already allows to force a link reset via 'restart'
> > parameter.
>
> Please be more precise, there a function and struct with this name.
>
> HOWEVER, there is no such parameter in tty-next (am I looking from wrong
> repo??):
>
> static int gsm_config_ext(struct gsm_mux *gsm, struct gsm_config_ext *ce)
>
> ...nor here:
>
> struct gsm_config_ext {
> __u32 keep_alive; /* Control channel keep-alive in 1/100th of a
> * second (0 to disable)
> */
> __u32 wait_config; /* Wait for DLCI config before opening virtual link? */
> __u32 reserved[6]; /* For future use, must be initialized to zero */
> };
>
> ???

You are right. Looks like I was looking at the wrong branch. I will reword
the commit message and deliver the patch for 'struct gsm_config_ext'
afterwards.

Best regards,
Daniel Starke