xsk: proper AF_XDP socket teardown ordering
[ Upstream commit 541d7fdd7694560404c502f64298a90ffe017e6b ] The AF_XDP socket struct can exist in three different, implicit states: setup, bound and released. Setup is prior the socket has been bound to a device. Bound is when the socket is active for receive and send. Released is when the process/userspace side of the socket is released, but the sock object is still lingering, e.g. when there is a reference to the socket in an XSKMAP after process termination. The Rx fast-path code uses the "dev" member of struct xdp_sock to check whether a socket is bound or relased, and the Tx code uses the struct xdp_umem "xsk_list" member in conjunction with "dev" to determine the state of a socket. However, the transition from bound to released did not tear the socket down in correct order. On the Rx side "dev" was cleared after synchronize_net() making the synchronization useless. On the Tx side, the internal queues were destroyed prior removing them from the "xsk_list". This commit corrects the cleanup order, and by doing so xdp_del_sk_umem() can be simplified and one synchronize_net() can be removed. Fixes:965a990984
("xsk: add support for bind for Rx") Fixes:ac98d8aab6
("xsk: wire upp Tx zero-copy functions") Reported-by: Jesper Dangaard Brouer <brouer@redhat.com> Signed-off-by: Björn Töpel <bjorn.topel@intel.com> Acked-by: Song Liu <songliubraving@fb.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
29fda86178
commit
b17ddbdc60
2 changed files with 11 additions and 13 deletions
|
@ -32,14 +32,9 @@ void xdp_del_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs)
|
||||||
{
|
{
|
||||||
unsigned long flags;
|
unsigned long flags;
|
||||||
|
|
||||||
if (xs->dev) {
|
spin_lock_irqsave(&umem->xsk_list_lock, flags);
|
||||||
spin_lock_irqsave(&umem->xsk_list_lock, flags);
|
list_del_rcu(&xs->list);
|
||||||
list_del_rcu(&xs->list);
|
spin_unlock_irqrestore(&umem->xsk_list_lock, flags);
|
||||||
spin_unlock_irqrestore(&umem->xsk_list_lock, flags);
|
|
||||||
|
|
||||||
if (umem->zc)
|
|
||||||
synchronize_net();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
int xdp_umem_query(struct net_device *dev, u16 queue_id)
|
int xdp_umem_query(struct net_device *dev, u16 queue_id)
|
||||||
|
|
|
@ -343,12 +343,18 @@ static int xsk_release(struct socket *sock)
|
||||||
local_bh_enable();
|
local_bh_enable();
|
||||||
|
|
||||||
if (xs->dev) {
|
if (xs->dev) {
|
||||||
|
struct net_device *dev = xs->dev;
|
||||||
|
|
||||||
/* Wait for driver to stop using the xdp socket. */
|
/* Wait for driver to stop using the xdp socket. */
|
||||||
synchronize_net();
|
xdp_del_sk_umem(xs->umem, xs);
|
||||||
dev_put(xs->dev);
|
|
||||||
xs->dev = NULL;
|
xs->dev = NULL;
|
||||||
|
synchronize_net();
|
||||||
|
dev_put(dev);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
xskq_destroy(xs->rx);
|
||||||
|
xskq_destroy(xs->tx);
|
||||||
|
|
||||||
sock_orphan(sk);
|
sock_orphan(sk);
|
||||||
sock->sk = NULL;
|
sock->sk = NULL;
|
||||||
|
|
||||||
|
@ -707,9 +713,6 @@ static void xsk_destruct(struct sock *sk)
|
||||||
if (!sock_flag(sk, SOCK_DEAD))
|
if (!sock_flag(sk, SOCK_DEAD))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
xskq_destroy(xs->rx);
|
|
||||||
xskq_destroy(xs->tx);
|
|
||||||
xdp_del_sk_umem(xs->umem, xs);
|
|
||||||
xdp_put_umem(xs->umem);
|
xdp_put_umem(xs->umem);
|
||||||
|
|
||||||
sk_refcnt_debug_dec(sk);
|
sk_refcnt_debug_dec(sk);
|
||||||
|
|
Loading…
Reference in a new issue