diff mbox series

[bpf] Revert "xsk: simplify AF_XDP socket teardown"

Message ID 20190221120738.24608-1-bjorn.topel@gmail.com
State Accepted
Delegated to: BPF Maintainers
Headers show
Series [bpf] Revert "xsk: simplify AF_XDP socket teardown" | expand

Commit Message

Björn Töpel Feb. 21, 2019, 12:07 p.m. UTC 
From: Björn Töpel <bjorn.topel@intel.com>

This reverts commit e2ce3674883ecba2605370404208c9d4a07ae1c3.

It turns out that the sock destructor xsk_destruct was needed after
all. The cleanup simplification broke the skb transmit cleanup path,
due to that the umem was prematurely destroyed.

The umem cannot be destroyed until all outstanding skbs are freed,
which means that we cannot remove the umem until the sk_destruct has
been called.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 net/xdp/xsk.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Daniel Borkmann Feb. 21, 2019, 3:36 p.m. UTC | #1 
On 02/21/2019 01:07 PM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> This reverts commit e2ce3674883ecba2605370404208c9d4a07ae1c3.
> 
> It turns out that the sock destructor xsk_destruct was needed after
> all. The cleanup simplification broke the skb transmit cleanup path,
> due to that the umem was prematurely destroyed.
> 
> The umem cannot be destroyed until all outstanding skbs are freed,
> which means that we cannot remove the umem until the sk_destruct has
> been called.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

Applied, thanks!
diff mbox series

Patch

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 45f3b528dc09..85e4fe4f18cc 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -366,7 +366,6 @@  static int xsk_release(struct socket *sock)
 
 	xskq_destroy(xs->rx);
 	xskq_destroy(xs->tx);
-	xdp_put_umem(xs->umem);
 
 	sock_orphan(sk);
 	sock->sk = NULL;
@@ -718,6 +717,18 @@  static const struct proto_ops xsk_proto_ops = {
 	.sendpage	= sock_no_sendpage,
 };
 
+static void xsk_destruct(struct sock *sk)
+{
+	struct xdp_sock *xs = xdp_sk(sk);
+
+	if (!sock_flag(sk, SOCK_DEAD))
+		return;
+
+	xdp_put_umem(xs->umem);
+
+	sk_refcnt_debug_dec(sk);
+}
+
 static int xsk_create(struct net *net, struct socket *sock, int protocol,
 		      int kern)
 {
@@ -744,6 +755,9 @@  static int xsk_create(struct net *net, struct socket *sock, int protocol,
 
 	sk->sk_family = PF_XDP;
 
+	sk->sk_destruct = xsk_destruct;
+	sk_refcnt_debug_inc(sk);
+
 	sock_set_flag(sk, SOCK_RCU_FREE);
 
 	xs = xdp_sk(sk);