[7/9] tty: n_gsm: increase gsm_mux unsupported counted where appropriate

Message ID 20230405054730.3850-7-daniel.starke@siemens.com
State New
Headers
Series [1/9] tty: n_gsm: fix redundant assignment of gsm->encoding |

Commit Message

D. Starke April 5, 2023, 5:47 a.m. UTC
  From: Daniel Starke <daniel.starke@siemens.com>

The structure gsm_mux contains the 'unsupported' field. However, there is
currently no place in the code which increases this counter.

Increase the 'unsupported' statistics counter in the following case:
- an unsupported frame type has been requested by the peer via parameter
  negotiation
- a control frame with an unsupported but known command has been received

Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
---
 drivers/tty/n_gsm.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Ilpo Järvinen April 5, 2023, 9 a.m. UTC | #1
On Wed, 5 Apr 2023, D. Starke wrote:

> From: Daniel Starke <daniel.starke@siemens.com>
> 
> The structure gsm_mux contains the 'unsupported' field. However, there is
> currently no place in the code which increases this counter.
> 
> Increase the 'unsupported' statistics counter in the following case:
> - an unsupported frame type has been requested by the peer via parameter
>   negotiation
> - a control frame with an unsupported but known command has been received

So inconsistent/unsupported adaptation doesn't fall under the second 
bullet?

(Please excuse my ignorance, I'm trying to review your patches with 
somewhat limited knowledge about how things work).
  
D. Starke April 6, 2023, 5:57 a.m. UTC | #2
> > From: Daniel Starke <daniel.starke@siemens.com>
> > 
> > The structure gsm_mux contains the 'unsupported' field. However, there is
> > currently no place in the code which increases this counter.
> > 
> > Increase the 'unsupported' statistics counter in the following case:
> > - an unsupported frame type has been requested by the peer via parameter
> >   negotiation
> > - a control frame with an unsupported but known command has been received
> 
> So inconsistent/unsupported adaptation doesn't fall under the second 
> bullet?
> 
> (Please excuse my ignorance, I'm trying to review your patches with 
> somewhat limited knowledge about how things work).

A wrong adaption is nothing we can detect with sufficient accuracy as this
changes the structure of the UI/UIH frames. E.g. a one byte header is added
in case of convergence layer type 2 instead of 1 and contains the modem
signal octet with the state of the signal lines. There is no checksum or
other value which indicates of this field is correct or should be present.
Therefore, we can only assume protocol correctness here.
See also function 'gsm_dlci_data' where this is handled.

Best regards,
Daniel Starke
  

Patch

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 317aa67ed169..49cb2dbfa233 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1589,6 +1589,7 @@  static int gsm_process_negotiation(struct gsm_mux *gsm, unsigned int addr,
 		if (debug & DBG_ERRORS)
 			pr_info("%s unsupported I frame request in PN\n",
 				__func__);
+		gsm->unsupported++;
 		return -EINVAL;
 	default:
 		if (debug & DBG_ERRORS)
@@ -1896,6 +1897,8 @@  static void gsm_control_message(struct gsm_mux *gsm, unsigned int command,
 		/* Optional unsupported commands */
 	case CMD_RPN:	/* Remote port negotiation */
 	case CMD_SNC:	/* Service negotiation command */
+		gsm->unsupported++;
+		fallthrough;
 	default:
 		/* Reply to bad commands with an NSC */
 		buf[0] = command;