patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 7 Packages Projects Releases Wiki Activity

net: ppp: Avoid wrapping each byte in muxing headers

Browse source 
When PPP is muxed, using uart_poll_out resulted in each byte getting
wrapped in a muxing header. This led to UART bombardment which
can quickly cause some modems to hang and panic. This was observed
regularly using a SIMCOM7600E modem.

A perfect fix would involve rewriting ppp.c, uart_mux.c and
modem_iface_uart.c to all use another UART API, but that would be more
invasive by several orders of magnitude than this one, which utilizes
the fact that the uart_mux implementation of uart_fifo_fill does NOT
require ISR context. Since the Zephyr UART API states that the behavior
of uart_fifo_fill outside of ISR context is implementation defined, this
should be kosher.

Signed-off-by: Benjamin Lindqvist <benjamin.lindqvist@endian.se>
This commit is contained in:
Benjamin Lindqvist 2020-10-15 14:10:08 +02:00 committed by Carles Cufí
parent 54cb2bcd41
commit 449fc7bb1c
3 changed files with 45 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
drivers/console/uart_mux.c
View file

@ -516,6 +516,15 @@ static int uart_mux_fifo_fill(const struct device *dev,
return -ENOENT;
}
/* If we're not in ISR context, do the xfer synchronously. This
* effectively let's applications use this implementation of fifo_fill
* as a multi-byte poll_out which prevents each byte getting wrapped by
* mux headers.
*/
if (!k_is_in_isr() && dev_data->dlci) {
return gsm_dlci_send(dev_data->dlci, tx_data, len);
}
LOG_DBG("dev_data %p len %d tx_ringbuf space %u",
dev_data, len, ring_buf_space_get(dev_data->tx_ringbuf));

27
drivers/modem/modem_iface_uart.c
View file

@ -125,6 +125,18 @@ static int modem_iface_uart_read(struct modem_iface *iface,
return 0;
}
static bool mux_is_active(struct modem_iface *iface)
{
bool active = false;
#if defined(CONFIG_UART_MUX_DEVICE_NAME)
const char *mux_name = CONFIG_UART_MUX_DEVICE_NAME;
active = (mux_name == iface->dev->name);
#endif /* CONFIG_UART_MUX_DEVICE_NAME */
return active;
}
static int modem_iface_uart_write(struct modem_iface *iface,
const uint8_t *buf, size_t size)
{
@ -136,9 +148,18 @@ static int modem_iface_uart_write(struct modem_iface *iface,
return 0;
}
do {
uart_poll_out(iface->dev, *buf++);
} while (--size);
/* If we're using gsm_mux, We don't want to use poll_out because sending
* one byte at a time causes each byte to get wrapped in muxing headers.
* But we can safely call uart_fifo_fill outside of ISR context when
* muxing because uart_mux implements it in software.
*/
if (mux_is_active(iface)) {
uart_fifo_fill(iface->dev, buf, size);
} else {
do {
uart_poll_out(iface->dev, *buf++);
} while (--size);
}
return 0;
}

14
drivers/net/ppp.c
View file

@ -194,9 +194,19 @@ static void ppp_change_state(struct ppp_driver_context *ctx,
static int ppp_send_flush(struct ppp_driver_context *ppp, int off)
{
if (!IS_ENABLED(CONFIG_NET_TEST)) {
uint8_t *buf = ppp->send_buf;
if (IS_ENABLED(CONFIG_NET_TEST)) {
return 0;
}
uint8_t *buf = ppp->send_buf;
/* If we're using gsm_mux, We don't want to use poll_out because sending
* one byte at a time causes each byte to get wrapped in muxing headers.
* But we can safely call uart_fifo_fill outside of ISR context when
* muxing because uart_mux implements it in software.
*/
if (IS_ENABLED(CONFIG_GSM_MUX)) {
(void)uart_fifo_fill(ppp->dev, buf, off);
} else {
while (off--) {
uart_poll_out(ppp->dev, *buf++);
}