mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	Fix the warning below by calling rhashtable_lookup_fast.
Also, make some code movements for better quality and human
readability.
[  342.450870] WARNING: suspicious RCU usage
[  342.455856] 4.18.0-rc2+ #17 Tainted: G           O
[  342.462210] -----------------------------
[  342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
[  342.476568]
[  342.476568] other info that might help us debug this:
[  342.476568]
[  342.486978]
[  342.486978] rcu_scheduler_active = 2, debug_locks = 1
[  342.495211] 4 locks held by modprobe/3934:
[  342.500265]  #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
mlx5_unregister_interface+0x18/0x90 [mlx5_core]
[  342.511953]  #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
[  342.521109]  #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
[mlx5_core]
[  342.531642]  #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
[  342.541206]
[  342.541206] stack backtrace:
[  342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G           O      4.18.0-rc2+ #17
[  342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
[  342.565606] Call Trace:
[  342.568861]  dump_stack+0x78/0xb3
[  342.573086]  xdp_rxq_info_unreg+0x3f5/0x6b0
[  342.578285]  ? __call_rcu+0x220/0x300
[  342.582911]  mlx5e_free_rq+0x38/0xc0 [mlx5_core]
[  342.588602]  mlx5e_close_channel+0x20/0x120 [mlx5_core]
[  342.594976]  mlx5e_close_channels+0x26/0x40 [mlx5_core]
[  342.601345]  mlx5e_close_locked+0x44/0x50 [mlx5_core]
[  342.607519]  mlx5e_close+0x42/0x60 [mlx5_core]
[  342.613005]  __dev_close_many+0xb1/0x120
[  342.617911]  dev_close_many+0xa2/0x170
[  342.622622]  rollback_registered_many+0x148/0x460
[  342.628401]  ? __lock_acquire+0x48d/0x11b0
[  342.633498]  ? unregister_netdev+0xe/0x20
[  342.638495]  rollback_registered+0x56/0x90
[  342.643588]  unregister_netdevice_queue+0x7e/0x100
[  342.649461]  unregister_netdev+0x18/0x20
[  342.654362]  mlx5e_remove+0x2a/0x50 [mlx5_core]
[  342.659944]  mlx5_remove_device+0xe5/0x110 [mlx5_core]
[  342.666208]  mlx5_unregister_interface+0x39/0x90 [mlx5_core]
[  342.673038]  cleanup+0x5/0xbfc [mlx5_core]
[  342.678094]  __x64_sys_delete_module+0x16b/0x240
[  342.683725]  ? do_syscall_64+0x1c/0x210
[  342.688476]  do_syscall_64+0x5a/0x210
[  342.693025]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: 8d5d885275 ("xdp: rhashtable with allocator ID to pointer mapping")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
		
	
			
		
			
				
	
	
		
			400 lines
		
	
	
	
		
			9.3 KiB
		
	
	
	
		
			C
		
	
	
	
	
	
			
		
		
	
	
			400 lines
		
	
	
	
		
			9.3 KiB
		
	
	
	
		
			C
		
	
	
	
	
	
/* net/core/xdp.c
 | 
						|
 *
 | 
						|
 * Copyright (c) 2017 Jesper Dangaard Brouer, Red Hat Inc.
 | 
						|
 * Released under terms in GPL version 2.  See COPYING.
 | 
						|
 */
 | 
						|
#include <linux/bpf.h>
 | 
						|
#include <linux/filter.h>
 | 
						|
#include <linux/types.h>
 | 
						|
#include <linux/mm.h>
 | 
						|
#include <linux/netdevice.h>
 | 
						|
#include <linux/slab.h>
 | 
						|
#include <linux/idr.h>
 | 
						|
#include <linux/rhashtable.h>
 | 
						|
#include <net/page_pool.h>
 | 
						|
 | 
						|
#include <net/xdp.h>
 | 
						|
 | 
						|
#define REG_STATE_NEW		0x0
 | 
						|
#define REG_STATE_REGISTERED	0x1
 | 
						|
#define REG_STATE_UNREGISTERED	0x2
 | 
						|
#define REG_STATE_UNUSED	0x3
 | 
						|
 | 
						|
static DEFINE_IDA(mem_id_pool);
 | 
						|
static DEFINE_MUTEX(mem_id_lock);
 | 
						|
#define MEM_ID_MAX 0xFFFE
 | 
						|
#define MEM_ID_MIN 1
 | 
						|
static int mem_id_next = MEM_ID_MIN;
 | 
						|
 | 
						|
static bool mem_id_init; /* false */
 | 
						|
static struct rhashtable *mem_id_ht;
 | 
						|
 | 
						|
struct xdp_mem_allocator {
 | 
						|
	struct xdp_mem_info mem;
 | 
						|
	union {
 | 
						|
		void *allocator;
 | 
						|
		struct page_pool *page_pool;
 | 
						|
		struct zero_copy_allocator *zc_alloc;
 | 
						|
	};
 | 
						|
	struct rhash_head node;
 | 
						|
	struct rcu_head rcu;
 | 
						|
};
 | 
						|
 | 
						|
static u32 xdp_mem_id_hashfn(const void *data, u32 len, u32 seed)
 | 
						|
{
 | 
						|
	const u32 *k = data;
 | 
						|
	const u32 key = *k;
 | 
						|
 | 
						|
	BUILD_BUG_ON(FIELD_SIZEOF(struct xdp_mem_allocator, mem.id)
 | 
						|
		     != sizeof(u32));
 | 
						|
 | 
						|
	/* Use cyclic increasing ID as direct hash key */
 | 
						|
	return key;
 | 
						|
}
 | 
						|
 | 
						|
static int xdp_mem_id_cmp(struct rhashtable_compare_arg *arg,
 | 
						|
			  const void *ptr)
 | 
						|
{
 | 
						|
	const struct xdp_mem_allocator *xa = ptr;
 | 
						|
	u32 mem_id = *(u32 *)arg->key;
 | 
						|
 | 
						|
	return xa->mem.id != mem_id;
 | 
						|
}
 | 
						|
 | 
						|
static const struct rhashtable_params mem_id_rht_params = {
 | 
						|
	.nelem_hint = 64,
 | 
						|
	.head_offset = offsetof(struct xdp_mem_allocator, node),
 | 
						|
	.key_offset  = offsetof(struct xdp_mem_allocator, mem.id),
 | 
						|
	.key_len = FIELD_SIZEOF(struct xdp_mem_allocator, mem.id),
 | 
						|
	.max_size = MEM_ID_MAX,
 | 
						|
	.min_size = 8,
 | 
						|
	.automatic_shrinking = true,
 | 
						|
	.hashfn    = xdp_mem_id_hashfn,
 | 
						|
	.obj_cmpfn = xdp_mem_id_cmp,
 | 
						|
};
 | 
						|
 | 
						|
static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
 | 
						|
{
 | 
						|
	struct xdp_mem_allocator *xa;
 | 
						|
 | 
						|
	xa = container_of(rcu, struct xdp_mem_allocator, rcu);
 | 
						|
 | 
						|
	/* Allow this ID to be reused */
 | 
						|
	ida_simple_remove(&mem_id_pool, xa->mem.id);
 | 
						|
 | 
						|
	/* Notice, driver is expected to free the *allocator,
 | 
						|
	 * e.g. page_pool, and MUST also use RCU free.
 | 
						|
	 */
 | 
						|
 | 
						|
	/* Poison memory */
 | 
						|
	xa->mem.id = 0xFFFF;
 | 
						|
	xa->mem.type = 0xF0F0;
 | 
						|
	xa->allocator = (void *)0xDEAD9001;
 | 
						|
 | 
						|
	kfree(xa);
 | 
						|
}
 | 
						|
 | 
						|
static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
 | 
						|
{
 | 
						|
	struct xdp_mem_allocator *xa;
 | 
						|
	int id = xdp_rxq->mem.id;
 | 
						|
 | 
						|
	if (id == 0)
 | 
						|
		return;
 | 
						|
 | 
						|
	mutex_lock(&mem_id_lock);
 | 
						|
 | 
						|
	xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
 | 
						|
	if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
 | 
						|
		call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
 | 
						|
 | 
						|
	mutex_unlock(&mem_id_lock);
 | 
						|
}
 | 
						|
 | 
						|
void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)
 | 
						|
{
 | 
						|
	/* Simplify driver cleanup code paths, allow unreg "unused" */
 | 
						|
	if (xdp_rxq->reg_state == REG_STATE_UNUSED)
 | 
						|
		return;
 | 
						|
 | 
						|
	WARN(!(xdp_rxq->reg_state == REG_STATE_REGISTERED), "Driver BUG");
 | 
						|
 | 
						|
	__xdp_rxq_info_unreg_mem_model(xdp_rxq);
 | 
						|
 | 
						|
	xdp_rxq->reg_state = REG_STATE_UNREGISTERED;
 | 
						|
	xdp_rxq->dev = NULL;
 | 
						|
 | 
						|
	/* Reset mem info to defaults */
 | 
						|
	xdp_rxq->mem.id = 0;
 | 
						|
	xdp_rxq->mem.type = 0;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_rxq_info_unreg);
 | 
						|
 | 
						|
static void xdp_rxq_info_init(struct xdp_rxq_info *xdp_rxq)
 | 
						|
{
 | 
						|
	memset(xdp_rxq, 0, sizeof(*xdp_rxq));
 | 
						|
}
 | 
						|
 | 
						|
/* Returns 0 on success, negative on failure */
 | 
						|
int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq,
 | 
						|
		     struct net_device *dev, u32 queue_index)
 | 
						|
{
 | 
						|
	if (xdp_rxq->reg_state == REG_STATE_UNUSED) {
 | 
						|
		WARN(1, "Driver promised not to register this");
 | 
						|
		return -EINVAL;
 | 
						|
	}
 | 
						|
 | 
						|
	if (xdp_rxq->reg_state == REG_STATE_REGISTERED) {
 | 
						|
		WARN(1, "Missing unregister, handled but fix driver");
 | 
						|
		xdp_rxq_info_unreg(xdp_rxq);
 | 
						|
	}
 | 
						|
 | 
						|
	if (!dev) {
 | 
						|
		WARN(1, "Missing net_device from driver");
 | 
						|
		return -ENODEV;
 | 
						|
	}
 | 
						|
 | 
						|
	/* State either UNREGISTERED or NEW */
 | 
						|
	xdp_rxq_info_init(xdp_rxq);
 | 
						|
	xdp_rxq->dev = dev;
 | 
						|
	xdp_rxq->queue_index = queue_index;
 | 
						|
 | 
						|
	xdp_rxq->reg_state = REG_STATE_REGISTERED;
 | 
						|
	return 0;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_rxq_info_reg);
 | 
						|
 | 
						|
void xdp_rxq_info_unused(struct xdp_rxq_info *xdp_rxq)
 | 
						|
{
 | 
						|
	xdp_rxq->reg_state = REG_STATE_UNUSED;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_rxq_info_unused);
 | 
						|
 | 
						|
bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq)
 | 
						|
{
 | 
						|
	return (xdp_rxq->reg_state == REG_STATE_REGISTERED);
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_rxq_info_is_reg);
 | 
						|
 | 
						|
static int __mem_id_init_hash_table(void)
 | 
						|
{
 | 
						|
	struct rhashtable *rht;
 | 
						|
	int ret;
 | 
						|
 | 
						|
	if (unlikely(mem_id_init))
 | 
						|
		return 0;
 | 
						|
 | 
						|
	rht = kzalloc(sizeof(*rht), GFP_KERNEL);
 | 
						|
	if (!rht)
 | 
						|
		return -ENOMEM;
 | 
						|
 | 
						|
	ret = rhashtable_init(rht, &mem_id_rht_params);
 | 
						|
	if (ret < 0) {
 | 
						|
		kfree(rht);
 | 
						|
		return ret;
 | 
						|
	}
 | 
						|
	mem_id_ht = rht;
 | 
						|
	smp_mb(); /* mutex lock should provide enough pairing */
 | 
						|
	mem_id_init = true;
 | 
						|
 | 
						|
	return 0;
 | 
						|
}
 | 
						|
 | 
						|
/* Allocate a cyclic ID that maps to allocator pointer.
 | 
						|
 * See: https://www.kernel.org/doc/html/latest/core-api/idr.html
 | 
						|
 *
 | 
						|
 * Caller must lock mem_id_lock.
 | 
						|
 */
 | 
						|
static int __mem_id_cyclic_get(gfp_t gfp)
 | 
						|
{
 | 
						|
	int retries = 1;
 | 
						|
	int id;
 | 
						|
 | 
						|
again:
 | 
						|
	id = ida_simple_get(&mem_id_pool, mem_id_next, MEM_ID_MAX, gfp);
 | 
						|
	if (id < 0) {
 | 
						|
		if (id == -ENOSPC) {
 | 
						|
			/* Cyclic allocator, reset next id */
 | 
						|
			if (retries--) {
 | 
						|
				mem_id_next = MEM_ID_MIN;
 | 
						|
				goto again;
 | 
						|
			}
 | 
						|
		}
 | 
						|
		return id; /* errno */
 | 
						|
	}
 | 
						|
	mem_id_next = id + 1;
 | 
						|
 | 
						|
	return id;
 | 
						|
}
 | 
						|
 | 
						|
static bool __is_supported_mem_type(enum xdp_mem_type type)
 | 
						|
{
 | 
						|
	if (type == MEM_TYPE_PAGE_POOL)
 | 
						|
		return is_page_pool_compiled_in();
 | 
						|
 | 
						|
	if (type >= MEM_TYPE_MAX)
 | 
						|
		return false;
 | 
						|
 | 
						|
	return true;
 | 
						|
}
 | 
						|
 | 
						|
int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
 | 
						|
			       enum xdp_mem_type type, void *allocator)
 | 
						|
{
 | 
						|
	struct xdp_mem_allocator *xdp_alloc;
 | 
						|
	gfp_t gfp = GFP_KERNEL;
 | 
						|
	int id, errno, ret;
 | 
						|
	void *ptr;
 | 
						|
 | 
						|
	if (xdp_rxq->reg_state != REG_STATE_REGISTERED) {
 | 
						|
		WARN(1, "Missing register, driver bug");
 | 
						|
		return -EFAULT;
 | 
						|
	}
 | 
						|
 | 
						|
	if (!__is_supported_mem_type(type))
 | 
						|
		return -EOPNOTSUPP;
 | 
						|
 | 
						|
	xdp_rxq->mem.type = type;
 | 
						|
 | 
						|
	if (!allocator) {
 | 
						|
		if (type == MEM_TYPE_PAGE_POOL || type == MEM_TYPE_ZERO_COPY)
 | 
						|
			return -EINVAL; /* Setup time check page_pool req */
 | 
						|
		return 0;
 | 
						|
	}
 | 
						|
 | 
						|
	/* Delay init of rhashtable to save memory if feature isn't used */
 | 
						|
	if (!mem_id_init) {
 | 
						|
		mutex_lock(&mem_id_lock);
 | 
						|
		ret = __mem_id_init_hash_table();
 | 
						|
		mutex_unlock(&mem_id_lock);
 | 
						|
		if (ret < 0) {
 | 
						|
			WARN_ON(1);
 | 
						|
			return ret;
 | 
						|
		}
 | 
						|
	}
 | 
						|
 | 
						|
	xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
 | 
						|
	if (!xdp_alloc)
 | 
						|
		return -ENOMEM;
 | 
						|
 | 
						|
	mutex_lock(&mem_id_lock);
 | 
						|
	id = __mem_id_cyclic_get(gfp);
 | 
						|
	if (id < 0) {
 | 
						|
		errno = id;
 | 
						|
		goto err;
 | 
						|
	}
 | 
						|
	xdp_rxq->mem.id = id;
 | 
						|
	xdp_alloc->mem  = xdp_rxq->mem;
 | 
						|
	xdp_alloc->allocator = allocator;
 | 
						|
 | 
						|
	/* Insert allocator into ID lookup table */
 | 
						|
	ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
 | 
						|
	if (IS_ERR(ptr)) {
 | 
						|
		errno = PTR_ERR(ptr);
 | 
						|
		goto err;
 | 
						|
	}
 | 
						|
 | 
						|
	mutex_unlock(&mem_id_lock);
 | 
						|
 | 
						|
	return 0;
 | 
						|
err:
 | 
						|
	mutex_unlock(&mem_id_lock);
 | 
						|
	kfree(xdp_alloc);
 | 
						|
	return errno;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
 | 
						|
 | 
						|
/* XDP RX runs under NAPI protection, and in different delivery error
 | 
						|
 * scenarios (e.g. queue full), it is possible to return the xdp_frame
 | 
						|
 * while still leveraging this protection.  The @napi_direct boolian
 | 
						|
 * is used for those calls sites.  Thus, allowing for faster recycling
 | 
						|
 * of xdp_frames/pages in those cases.
 | 
						|
 */
 | 
						|
static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
 | 
						|
			 unsigned long handle)
 | 
						|
{
 | 
						|
	struct xdp_mem_allocator *xa;
 | 
						|
	struct page *page;
 | 
						|
 | 
						|
	switch (mem->type) {
 | 
						|
	case MEM_TYPE_PAGE_POOL:
 | 
						|
		rcu_read_lock();
 | 
						|
		/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
 | 
						|
		xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
 | 
						|
		page = virt_to_head_page(data);
 | 
						|
		if (xa) {
 | 
						|
			napi_direct &= !xdp_return_frame_no_direct();
 | 
						|
			page_pool_put_page(xa->page_pool, page, napi_direct);
 | 
						|
		} else {
 | 
						|
			put_page(page);
 | 
						|
		}
 | 
						|
		rcu_read_unlock();
 | 
						|
		break;
 | 
						|
	case MEM_TYPE_PAGE_SHARED:
 | 
						|
		page_frag_free(data);
 | 
						|
		break;
 | 
						|
	case MEM_TYPE_PAGE_ORDER0:
 | 
						|
		page = virt_to_page(data); /* Assumes order0 page*/
 | 
						|
		put_page(page);
 | 
						|
		break;
 | 
						|
	case MEM_TYPE_ZERO_COPY:
 | 
						|
		/* NB! Only valid from an xdp_buff! */
 | 
						|
		rcu_read_lock();
 | 
						|
		/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
 | 
						|
		xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
 | 
						|
		xa->zc_alloc->free(xa->zc_alloc, handle);
 | 
						|
		rcu_read_unlock();
 | 
						|
	default:
 | 
						|
		/* Not possible, checked in xdp_rxq_info_reg_mem_model() */
 | 
						|
		break;
 | 
						|
	}
 | 
						|
}
 | 
						|
 | 
						|
void xdp_return_frame(struct xdp_frame *xdpf)
 | 
						|
{
 | 
						|
	__xdp_return(xdpf->data, &xdpf->mem, false, 0);
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_return_frame);
 | 
						|
 | 
						|
void xdp_return_frame_rx_napi(struct xdp_frame *xdpf)
 | 
						|
{
 | 
						|
	__xdp_return(xdpf->data, &xdpf->mem, true, 0);
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_return_frame_rx_napi);
 | 
						|
 | 
						|
void xdp_return_buff(struct xdp_buff *xdp)
 | 
						|
{
 | 
						|
	__xdp_return(xdp->data, &xdp->rxq->mem, true, xdp->handle);
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_return_buff);
 | 
						|
 | 
						|
int xdp_attachment_query(struct xdp_attachment_info *info,
 | 
						|
			 struct netdev_bpf *bpf)
 | 
						|
{
 | 
						|
	bpf->prog_id = info->prog ? info->prog->aux->id : 0;
 | 
						|
	bpf->prog_flags = info->prog ? info->flags : 0;
 | 
						|
	return 0;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_attachment_query);
 | 
						|
 | 
						|
bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
 | 
						|
			     struct netdev_bpf *bpf)
 | 
						|
{
 | 
						|
	if (info->prog && (bpf->flags ^ info->flags) & XDP_FLAGS_MODES) {
 | 
						|
		NL_SET_ERR_MSG(bpf->extack,
 | 
						|
			       "program loaded with different flags");
 | 
						|
		return false;
 | 
						|
	}
 | 
						|
	return true;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_attachment_flags_ok);
 | 
						|
 | 
						|
void xdp_attachment_setup(struct xdp_attachment_info *info,
 | 
						|
			  struct netdev_bpf *bpf)
 | 
						|
{
 | 
						|
	if (info->prog)
 | 
						|
		bpf_prog_put(info->prog);
 | 
						|
	info->prog = bpf->prog;
 | 
						|
	info->flags = bpf->flags;
 | 
						|
}
 | 
						|
EXPORT_SYMBOL_GPL(xdp_attachment_setup);
 |