[PATCH net] drivers/net/wan/x25_asy: Keep the ldisc running even when netif is down

* [PATCH net] drivers/net/wan/x25_asy: Keep the ldisc running even when netif is down
@ 2020-09-26 20:56 Xie He
  2020-09-28 22:58 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Xie He @ 2020-09-26 20:56 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev, linux-kernel, Martin Schiller
  Cc: Xie He

I believe it is necessary to keep the N_X25 line discipline running even
when the upper network interface is down, because otherwise, the last
frame transmitted before the interface going down may be incompletely
transmitted, and the first frame received after the interface going up
may be incompletely received.

By allowing the line discipline to continue doing R/W on the TTY, we can
ensure that the last frame before the interface goes down is completely
transmitted, and the first frame after the interface goes up is completely
received.

To achieve this, I did these changes:

1. Postpone the netif_running check in x25_asy_write_wakeup until the
transmission of data is complete.

2. Postpone the netif_running check in x25_asy_receive_buf until a
complete frame is received (in x25_asy_bump).

3. Move x25_asy_close from x25_asy_close_dev to x25_asy_close_tty,
so that when closing the interface, TTY transmission will not stop and
the line discipline's read buffer and write buffer will not be cleared.
(Do these only when the line discipline is closing.)

(Also add FIXME comments because the netif_running checks may race with
the closing of the netif. Currently there's no locking to prevent this.
This needs to be fixed.)

Cc: Martin Schiller <ms@dev.tdt.de>
Signed-off-by: Xie He <xie.he.0141@gmail.com>
---
 drivers/net/wan/x25_asy.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
index c418767a890a..22fcc0dd4b57 100644
--- a/drivers/net/wan/x25_asy.c
+++ b/drivers/net/wan/x25_asy.c
@@ -192,6 +192,10 @@ static void x25_asy_bump(struct x25_asy *sl)
 	int count;
 	int err;
 
+	/* FIXME: The netif may go down after netif_running returns true */
+	if (!netif_running(dev))
+		return;
+
 	count = sl->rcount;
 	dev->stats.rx_bytes += count;
 
@@ -257,7 +261,7 @@ static void x25_asy_write_wakeup(struct tty_struct *tty)
 	struct x25_asy *sl = tty->disc_data;
 
 	/* First make sure we're connected. */
-	if (!sl || sl->magic != X25_ASY_MAGIC || !netif_running(sl->dev))
+	if (!sl || sl->magic != X25_ASY_MAGIC)
 		return;
 
 	if (sl->xleft <= 0) {
@@ -265,7 +269,9 @@ static void x25_asy_write_wakeup(struct tty_struct *tty)
 		 * transmission of another packet */
 		sl->dev->stats.tx_packets++;
 		clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
-		x25_asy_unlock(sl);
+		/* FIXME: The netif may go down after netif_running returns */
+		if (netif_running(sl->dev))
+			x25_asy_unlock(sl);
 		return;
 	}
 
@@ -529,7 +535,7 @@ static void x25_asy_receive_buf(struct tty_struct *tty,
 {
 	struct x25_asy *sl = tty->disc_data;
 
-	if (!sl || sl->magic != X25_ASY_MAGIC || !netif_running(sl->dev))
+	if (!sl || sl->magic != X25_ASY_MAGIC)
 		return;
 
 
@@ -605,6 +611,7 @@ static void x25_asy_close_tty(struct tty_struct *tty)
 		dev_close(sl->dev);
 	rtnl_unlock();
 
+	x25_asy_close(sl->dev);
 	tty->disc_data = NULL;
 	sl->tty = NULL;
 	x25_asy_free(sl);
@@ -732,8 +739,6 @@ static int x25_asy_close_dev(struct net_device *dev)
 		pr_err("%s: lapb_unregister error: %d\n",
 		       __func__, err);
 
-	x25_asy_close(dev);
-
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread