linux-kernel - [RFC PATCH net-next] net: ppp: convert to IFF_NO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241029103656.2151-1-dqfext@gmail.com>
Date: Tue, 29 Oct 2024 18:36:56 +0800
From: Qingfang Deng <dqfext@...il.com>
To: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-ppp@...r.kernel.org
Subject: [RFC PATCH net-next] net: ppp: convert to IFF_NO_QUEUE

When testing the parallel TX performance of a single PPPoE interface
over a 2.5GbE link with multiple hardware queues, the throughput could
not exceed 1.9Gbps, even with low CPU usage.

This issue arises because the PPP interface is registered with a single
queue and a tx_queue_len of 3. This default behavior dates back to Linux
2.3.13, which was suitable for slower serial ports. However, in modern
devices with multiple processors and hardware queues, this configuration
can lead to congestion.

For PPPoE/PPTP, the lower interface should handle qdisc, so we need to
set IFF_NO_QUEUE. For PPP over a serial port, we don't benefit from a
qdisc with such a short TX queue, so handling TX queueing in the driver
and setting IFF_NO_QUEUE is more effective.

With this change, PPPoE interfaces can now fully saturate a 2.5GbE link.

Signed-off-by: Qingfang Deng <dqfext@...il.com>
---
 drivers/net/ppp/ppp_generic.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 4b2971e2bf48..5470e0fe1f9b 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -236,8 +236,8 @@ struct ppp_net {
 /* Get the PPP protocol number from a skb */
 #define PPP_PROTO(skb)	get_unaligned_be16((skb)->data)
 
-/* We limit the length of ppp->file.rq to this (arbitrary) value */
-#define PPP_MAX_RQLEN	32
+/* We limit the length of ppp->file.rq/xq to this (arbitrary) value */
+#define PPP_MAX_QLEN	32
 
 /*
  * Maximum number of multilink fragments queued up.
@@ -920,8 +920,6 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 				break;
 		} else {
 			ppp->npmode[i] = npi.mode;
-			/* we may be able to transmit more packets now (??) */
-			netif_wake_queue(ppp->dev);
 		}
 		err = 0;
 		break;
@@ -1639,6 +1637,7 @@ static void ppp_setup(struct net_device *dev)
 	dev->tx_queue_len = 3;
 	dev->type = ARPHRD_PPP;
 	dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+	dev->priv_flags |= IFF_NO_QUEUE;
 	dev->priv_destructor = ppp_dev_priv_destructor;
 	netif_keep_dst(dev);
 }
@@ -1654,17 +1653,15 @@ static void __ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
 	if (!ppp->closing) {
 		ppp_push(ppp);
 
-		if (skb)
-			skb_queue_tail(&ppp->file.xq, skb);
+		if (skb) {
+			if (ppp->file.xq.qlen > PPP_MAX_QLEN)
+				kfree_skb(skb);
+			else
+				skb_queue_tail(&ppp->file.xq, skb);
+		}
 		while (!ppp->xmit_pending &&
 		       (skb = skb_dequeue(&ppp->file.xq)))
 			ppp_send_frame(ppp, skb);
-		/* If there's no work left to do, tell the core net
-		   code that we can accept some more. */
-		if (!ppp->xmit_pending && !skb_peek(&ppp->file.xq))
-			netif_wake_queue(ppp->dev);
-		else
-			netif_stop_queue(ppp->dev);
 	} else {
 		kfree_skb(skb);
 	}
@@ -1850,7 +1847,7 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 	 * queue it up for pppd to receive.
 	 */
 	if (ppp->flags & SC_LOOP_TRAFFIC) {
-		if (ppp->file.rq.qlen > PPP_MAX_RQLEN)
+		if (ppp->file.rq.qlen > PPP_MAX_QLEN)
 			goto drop;
 		skb_queue_tail(&ppp->file.rq, skb);
 		wake_up_interruptible(&ppp->file.rwait);
@@ -2319,7 +2316,7 @@ ppp_input(struct ppp_channel *chan, struct sk_buff *skb)
 		/* put it on the channel queue */
 		skb_queue_tail(&pch->file.rq, skb);
 		/* drop old frames if queue too long */
-		while (pch->file.rq.qlen > PPP_MAX_RQLEN &&
+		while (pch->file.rq.qlen > PPP_MAX_QLEN &&
 		       (skb = skb_dequeue(&pch->file.rq)))
 			kfree_skb(skb);
 		wake_up_interruptible(&pch->file.rwait);
@@ -2472,7 +2469,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
 		/* control or unknown frame - pass it to pppd */
 		skb_queue_tail(&ppp->file.rq, skb);
 		/* limit queue length by dropping old frames */
-		while (ppp->file.rq.qlen > PPP_MAX_RQLEN &&
+		while (ppp->file.rq.qlen > PPP_MAX_QLEN &&
 		       (skb = skb_dequeue(&ppp->file.rq)))
 			kfree_skb(skb);
 		/* wake up any process polling or blocking on read */
-- 
2.34.1