Skip to content

Commit

Permalink
tty: n_gsm: fix wrong queuing behavior in gsm_dlci_data_output()
Browse files Browse the repository at this point in the history
[ Upstream commit 556fc8a ]

1) The function drains the fifo for the given user tty/DLCI without
considering 'TX_THRESH_HI' and different to gsm_dlci_data_output_framed(),
which moves only one packet from the user side to the internal transmission
queue. We can only handle one packet at a time here if we want to allow
DLCI priority handling in gsm_dlci_data_sweep() to avoid link starvation.
2) Furthermore, the additional header octet from convergence layer type 2
is not counted against MTU. It is part of the UI/UIH frame message which
needs to be limited to MTU. Hence, it is wrong not to consider this octet.
3) Finally, the waiting user tty is not informed about freed space in its
send queue.

Take at most one packet worth of data out of the DLCI fifo to fix 1).
Limit the max user data size per packet to MTU - 1 in case of convergence
layer type 2 to leave space for the control signal octet which is added in
the later part of the function. This fixes 2).
Add tty_port_tty_wakeup() to wake up the user tty if new write space has
been made available to fix 3).

Fixes: 268e526 ("tty/n_gsm: avoid fifo overflow in gsm_dlci_data_output")
Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
Link: https://lore.kernel.org/r/20220701061652.39604-3-daniel.starke@siemens.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
dstarke-siemens authored and gregkh committed Aug 17, 2022
1 parent dfa9b6d commit 1018139
Showing 1 changed file with 42 additions and 32 deletions.
74 changes: 42 additions & 32 deletions drivers/tty/n_gsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -886,41 +886,51 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci)
{
struct gsm_msg *msg;
u8 *dp;
int len, total_size, size;
int h = dlci->adaption - 1;
int h, len, size;

total_size = 0;
while (1) {
len = kfifo_len(&dlci->fifo);
if (len == 0)
return total_size;

/* MTU/MRU count only the data bits */
if (len > gsm->mtu)
len = gsm->mtu;

size = len + h;

msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
/* FIXME: need a timer or something to kick this so it can't
get stuck with no work outstanding and no buffer free */
if (msg == NULL)
return -ENOMEM;
dp = msg->data;
switch (dlci->adaption) {
case 1: /* Unstructured */
break;
case 2: /* Unstructed with modem bits.
Always one byte as we never send inline break data */
*dp++ = (gsm_encode_modem(dlci) << 1) | EA;
break;
}
WARN_ON(kfifo_out_locked(&dlci->fifo, dp , len, &dlci->lock) != len);
__gsm_data_queue(dlci, msg);
total_size += size;
/* for modem bits without break data */
h = ((dlci->adaption == 1) ? 0 : 1);

len = kfifo_len(&dlci->fifo);
if (len == 0)
return 0;

/* MTU/MRU count only the data bits but watch adaption mode */
if ((len + h) > gsm->mtu)
len = gsm->mtu - h;

size = len + h;

msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
/* FIXME: need a timer or something to kick this so it can't
* get stuck with no work outstanding and no buffer free
*/
if (!msg)
return -ENOMEM;
dp = msg->data;
switch (dlci->adaption) {
case 1: /* Unstructured */
break;
case 2: /* Unstructured with modem bits.
* Always one byte as we never send inline break data
*/
*dp++ = (gsm_encode_modem(dlci) << 1) | EA;
break;
default:
pr_err("%s: unsupported adaption %d\n", __func__,
dlci->adaption);
break;
}

WARN_ON(len != kfifo_out_locked(&dlci->fifo, dp, len,
&dlci->lock));

/* Notify upper layer about available send space. */
tty_port_tty_wakeup(&dlci->port);

__gsm_data_queue(dlci, msg);
/* Bytes of data we used up */
return total_size;
return size;
}

/**
Expand Down

0 comments on commit 1018139

Please sign in to comment.