netdev - Re: PPPoE performance regression

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 13 Jun 2012 14:50:01 +0100
From:	David Woodhouse <dwmw2@...radead.org>
To:	Nathan Williams <nathan@...verse.com.au>
Cc:	Karl Hiramoto <karl@...amoto.org>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	Paul Mackerras <paulus@...ba.org>,
	John Crispin <blogic@...nwrt.org>
Subject: Re: PPPoE performance regression

On Wed, 2012-06-13 at 10:57 +0100, David Woodhouse wrote:
> This doesn't look *so* evil... if the basic concept of using
> skb_orphan() and then setting our own destructor is OK, then I'll work
> out the rest of the details and do it for l2tp too.

Stupid dwmw2. With patch this time...

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index cbf7047..ddaf156 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -689,6 +689,8 @@ static int pppoe_connect(struct socket *sock, struct sockaddr *uservaddr,
 		sk->sk_state = PPPOX_CONNECTED;
 	}
 
+	atomic_set(&po->inflight, -2);
+
 	po->num = sp->sa_addr.pppoe.sid;
 
 end:
@@ -952,9 +954,34 @@ abort:
  * sends PPP frame over PPPoE socket
  *
  ***********************************************************************/
+static void pppoe_skb_destructor(struct sk_buff *skb)
+{
+	struct sock *sk = skb->sk;
+	struct pppox_sock *po = pppox_sk(sk);
+
+	atomic_dec(&po->inflight);
+	/* Schedule a call to ppp_output_wakeup(chan), if it was already blocked.
+	   Mind for race conditions with another CPU which is in pppoe_xmit() 
+	   right now. See commit 9d02daf7 in pppoatm. */
+	sock_put(sk);
+}
+
 static int pppoe_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 {
 	struct sock *sk = (struct sock *)chan->private;
+	struct pppox_sock *po = pppox_sk(sk);
+
+	if (!atomic_inc_not_zero(&po->inflight))
+		return 0;
+
+	/* mine! all mine! */
+	skb_orphan(skb);
+	skb->destructor = pppoe_skb_destructor;
+	/* XXX: Are there other implications of setting this? Should we use ->cb? */
+	skb->sk = sk;
+
+	sock_hold(sk);
+
 	return __pppoe_xmit(sk, skb);
 }
 
diff --git a/include/linux/if_pppox.h b/include/linux/if_pppox.h
index 09c474c..339c75d 100644
--- a/include/linux/if_pppox.h
+++ b/include/linux/if_pppox.h
@@ -186,6 +186,7 @@ struct pppox_sock {
 	/* struct sock must be the first member of pppox_sock */
 	struct sock sk;
 	struct ppp_channel chan;
+	atomic_t inflight;
 	struct pppox_sock	*next;	  /* for hash table */
 	union {
 		struct pppoe_opt pppoe;

-- 
dwmw2

Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (6171 bytes)