From 449fc7bb1c90c1393b63c959cc63b75140b2e195 Mon Sep 17 00:00:00 2001 From: Benjamin Lindqvist Date: Thu, 15 Oct 2020 14:10:08 +0200 Subject: [PATCH] net: ppp: Avoid wrapping each byte in muxing headers 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 --- drivers/console/uart_mux.c | 9 +++++++++ drivers/modem/modem_iface_uart.c | 27 ++++++++++++++++++++++++--- drivers/net/ppp.c | 14 ++++++++++++-- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/drivers/console/uart_mux.c b/drivers/console/uart_mux.c index d16a295e11..7107c7b0e4 100644 --- a/drivers/console/uart_mux.c +++ b/drivers/console/uart_mux.c @@ -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)); diff --git a/drivers/modem/modem_iface_uart.c b/drivers/modem/modem_iface_uart.c index d052fe9da4..1f676a9a50 100644 --- a/drivers/modem/modem_iface_uart.c +++ b/drivers/modem/modem_iface_uart.c @@ -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; } diff --git a/drivers/net/ppp.c b/drivers/net/ppp.c index 7e8d00c593..9927ad98b5 100644 --- a/drivers/net/ppp.c +++ b/drivers/net/ppp.c @@ -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++); }