Re: [PATCH 8/9] tty: n_gsm: add DLCI specific rx/tx statistics

[Ilpo Järvinen]

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

> From: Daniel Starke <daniel.starke@xxxxxxxxxxx>
> 
> Add counters for the number of data bytes received/transmitted per DLCI in
> for preparation for an upcoming patch which will expose these values to the
> user.
> 
> Signed-off-by: Daniel Starke <daniel.starke@xxxxxxxxxxx>
> ---
>  drivers/tty/n_gsm.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 49cb2dbfa233..61f9825fde3c 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -185,6 +185,9 @@ struct gsm_dlci {
>  	void (*data)(struct gsm_dlci *dlci, const u8 *data, int len);
>  	void (*prev_data)(struct gsm_dlci *dlci, const u8 *data, int len);
>  	struct net_device *net; /* network interface, if created */
> +	/* Statistics (not currently exposed) */
> +	u64 tx;			/* Data bytes sent on this DLCI */
> +	u64 rx;			/* Data bytes received on this DLCI */
>  };
>  
>  /*
> @@ -1215,6 +1218,7 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci)
>  	tty_port_tty_wakeup(&dlci->port);
>  
>  	__gsm_data_queue(dlci, msg);
> +	dlci->tx += len;
>  	/* Bytes of data we used up */
>  	return size;
>  }

Reading the function comments and your changelog, I'm left to wonder why 
gsm_dlci_data_output() is supposed to increment ->tx but 
gsm_dlci_data_output_framed() is not?

-- 
 i.

> @@ -1459,6 +1463,7 @@ static int gsm_control_command(struct gsm_mux *gsm, int cmd, const u8 *data,
>  	msg->data[1] = (dlen << 1) | EA;
>  	memcpy(msg->data + 2, data, dlen);
>  	gsm_data_queue(gsm->dlci[0], msg);
> +	gsm->dlci[0]->tx += dlen;
>  
>  	return 0;
>  }
> @@ -1485,6 +1490,7 @@ static void gsm_control_reply(struct gsm_mux *gsm, int cmd, const u8 *data,
>  	msg->data[1] = (dlen << 1) | EA;
>  	memcpy(msg->data + 2, data, dlen);
>  	gsm_data_queue(gsm->dlci[0], msg);
> +	gsm->dlci[0]->tx += dlen;
>  }
>  
>  /**
> @@ -1849,10 +1855,13 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command,
>  						const u8 *data, int clen)
>  {
>  	u8 buf[1];
> +	struct gsm_dlci *dlci = gsm->dlci[0];
> +
> +	if (dlci)
> +		dlci->rx += clen;
>  
>  	switch (command) {
>  	case CMD_CLD: {
> -		struct gsm_dlci *dlci = gsm->dlci[0];
>  		/* Modem wishes to close down */
>  		if (dlci) {
>  			dlci->dead = true;
> @@ -1931,6 +1940,8 @@ static void gsm_control_response(struct gsm_mux *gsm, unsigned int command,
>  
>  	ctrl = gsm->pending_cmd;
>  	dlci = gsm->dlci[0];
> +	if (dlci)
> +		dlci->rx += clen;
>  	command |= 1;
>  	/* Does the reply match our command */
>  	if (ctrl != NULL && (command == ctrl->cmd || command == CMD_NSC)) {
> @@ -2295,6 +2306,9 @@ static void gsm_dlci_begin_open(struct gsm_dlci *dlci)
>  			need_pn = true;
>  	}
>  
> +	dlci->tx = 0;
> +	dlci->rx = 0;
> +
>  	switch (dlci->state) {
>  	case DLCI_CLOSED:
>  	case DLCI_WAITING_CONFIG:
> @@ -2327,6 +2341,9 @@ static void gsm_dlci_begin_open(struct gsm_dlci *dlci)
>   */
>  static void gsm_dlci_set_opening(struct gsm_dlci *dlci)
>  {
> +	dlci->tx = 0;
> +	dlci->rx = 0;
> +
>  	switch (dlci->state) {
>  	case DLCI_CLOSED:
>  	case DLCI_WAITING_CONFIG:
> @@ -2346,6 +2363,9 @@ static void gsm_dlci_set_opening(struct gsm_dlci *dlci)
>   */
>  static void gsm_dlci_set_wait_config(struct gsm_dlci *dlci)
>  {
> +	dlci->tx = 0;
> +	dlci->rx = 0;
> +
>  	switch (dlci->state) {
>  	case DLCI_CLOSED:
>  	case DLCI_CLOSING:
> @@ -2422,6 +2442,7 @@ static void gsm_dlci_data(struct gsm_dlci *dlci, const u8 *data, int clen)
>  		fallthrough;
>  	case 1:		/* Line state will go via DLCI 0 controls only */
>  	default:
> +		dlci->rx += clen;
>  		tty_insert_flip_string(port, data, clen);
>  		tty_flip_buffer_push(port);
>  	}
> @@ -2782,6 +2803,7 @@ static void gsm_queue(struct gsm_mux *gsm)
>  			gsm->open_error++;
>  			return;
>  		}
> +		dlci->rx += gsm->len;
>  		if (dlci->dead)
>  			gsm_response(gsm, address, DM|PF);
>  		else {
>