From e0a65e3c5e3b7b11ec9320524b8fcc210f2026e9 Mon Sep 17 00:00:00 2001 From: Edward Cree Date: Tue, 27 Mar 2018 17:44:36 +0100 Subject: [PATCH] sfc: protect list of RSS contexts under a mutex Otherwise races are possible between ethtool ops and efx_ef10_rx_restore_rss_contexts(). Also, don't try to perform the restore on every reset, only after an MC reboot, otherwise we'll leak RSS contexts on the NIC. Fixes: 42356d9a137b ("sfc: support RSS spreading of ethtool ntuple filters") Signed-off-by: Edward Cree Signed-off-by: David S. Miller --- drivers/net/ethernet/sfc/ef10.c | 32 +++++++++++-- drivers/net/ethernet/sfc/efx.c | 14 +++++- drivers/net/ethernet/sfc/efx.h | 4 +- drivers/net/ethernet/sfc/ethtool.c | 66 +++++++++++++++++---------- drivers/net/ethernet/sfc/net_driver.h | 2 + drivers/net/ethernet/sfc/nic.h | 3 ++ 6 files changed, 89 insertions(+), 32 deletions(-) diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c index 9a139c390037..c4c45c94da77 100644 --- a/drivers/net/ethernet/sfc/ef10.c +++ b/drivers/net/ethernet/sfc/ef10.c @@ -1499,6 +1499,7 @@ static void efx_ef10_reset_mc_allocations(struct efx_nic *efx) /* All our allocations have been reset */ nic_data->must_realloc_vis = true; + nic_data->must_restore_rss_contexts = true; nic_data->must_restore_filters = true; nic_data->must_restore_piobufs = true; efx_ef10_forget_old_piobufs(efx); @@ -2899,6 +2900,8 @@ static int efx_ef10_rx_push_rss_context_config(struct efx_nic *efx, { int rc; + WARN_ON(!mutex_is_locked(&efx->rss_lock)); + if (ctx->context_id == EFX_EF10_RSS_CONTEXT_INVALID) { rc = efx_ef10_alloc_rss_context(efx, true, ctx, NULL); if (rc) @@ -2929,6 +2932,8 @@ static int efx_ef10_rx_pull_rss_context_config(struct efx_nic *efx, size_t outlen; int rc, i; + WARN_ON(!mutex_is_locked(&efx->rss_lock)); + BUILD_BUG_ON(MC_CMD_RSS_CONTEXT_GET_TABLE_IN_LEN != MC_CMD_RSS_CONTEXT_GET_KEY_IN_LEN); @@ -2972,14 +2977,25 @@ static int efx_ef10_rx_pull_rss_context_config(struct efx_nic *efx, static int efx_ef10_rx_pull_rss_config(struct efx_nic *efx) { - return efx_ef10_rx_pull_rss_context_config(efx, &efx->rss_context); + int rc; + + mutex_lock(&efx->rss_lock); + rc = efx_ef10_rx_pull_rss_context_config(efx, &efx->rss_context); + mutex_unlock(&efx->rss_lock); + return rc; } static void efx_ef10_rx_restore_rss_contexts(struct efx_nic *efx) { + struct efx_ef10_nic_data *nic_data = efx->nic_data; struct efx_rss_context *ctx; int rc; + WARN_ON(!mutex_is_locked(&efx->rss_lock)); + + if (!nic_data->must_restore_rss_contexts) + return; + list_for_each_entry(ctx, &efx->rss_context.list, list) { /* previous NIC RSS context is gone */ ctx->context_id = EFX_EF10_RSS_CONTEXT_INVALID; @@ -2993,6 +3009,7 @@ static void efx_ef10_rx_restore_rss_contexts(struct efx_nic *efx) "; RSS filters may fail to be applied\n", ctx->user_id, rc); } + nic_data->must_restore_rss_contexts = false; } static int efx_ef10_pf_rx_push_rss_config(struct efx_nic *efx, bool user, @@ -4307,6 +4324,7 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx, struct efx_rss_context *ctx = NULL; unsigned int match_pri, hash; unsigned int priv_flags; + bool rss_locked = false; bool replacing = false; unsigned int depth, i; int ins_index = -1; @@ -4336,9 +4354,10 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx, bitmap_zero(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT); if (spec->flags & EFX_FILTER_FLAG_RX_RSS) { + mutex_lock(&efx->rss_lock); + rss_locked = true; if (spec->rss_context) - ctx = efx_find_rss_context_entry(spec->rss_context, - &efx->rss_context.list); + ctx = efx_find_rss_context_entry(efx, spec->rss_context); else ctx = &efx->rss_context; if (!ctx) { @@ -4501,6 +4520,8 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx, rc = efx_ef10_make_filter_id(match_pri, ins_index); out_unlock: + if (rss_locked) + mutex_unlock(&efx->rss_lock); up_write(&table->lock); up_read(&efx->filter_sem); return rc; @@ -5008,6 +5029,7 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx) return; down_write(&table->lock); + mutex_lock(&efx->rss_lock); for (filter_idx = 0; filter_idx < HUNT_FILTER_TBL_ROWS; filter_idx++) { spec = efx_ef10_filter_entry_spec(table, filter_idx); @@ -5024,8 +5046,7 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx) goto not_restored; } if (spec->rss_context) - ctx = efx_find_rss_context_entry(spec->rss_context, - &efx->rss_context.list); + ctx = efx_find_rss_context_entry(efx, spec->rss_context); else ctx = &efx->rss_context; if (spec->flags & EFX_FILTER_FLAG_RX_RSS) { @@ -5064,6 +5085,7 @@ not_restored: } } + mutex_unlock(&efx->rss_lock); up_write(&table->lock); /* This can happen validly if the MC's capabilities have changed, so diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c index cb9273016916..692dd729ee2a 100644 --- a/drivers/net/ethernet/sfc/efx.c +++ b/drivers/net/ethernet/sfc/efx.c @@ -2657,6 +2657,7 @@ void efx_reset_down(struct efx_nic *efx, enum reset_type method) efx_disable_interrupts(efx); mutex_lock(&efx->mac_lock); + mutex_lock(&efx->rss_lock); if (efx->port_initialized && method != RESET_TYPE_INVISIBLE && method != RESET_TYPE_DATAPATH) efx->phy_op->fini(efx); @@ -2712,6 +2713,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok) if (efx->type->rx_restore_rss_contexts) efx->type->rx_restore_rss_contexts(efx); + mutex_unlock(&efx->rss_lock); down_read(&efx->filter_sem); efx_restore_filters(efx); up_read(&efx->filter_sem); @@ -2730,6 +2732,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok) fail: efx->port_initialized = false; + mutex_unlock(&efx->rss_lock); mutex_unlock(&efx->mac_lock); return rc; @@ -3016,6 +3019,7 @@ static int efx_init_struct(struct efx_nic *efx, efx->rx_packet_ts_offset = efx->type->rx_ts_offset - efx->type->rx_prefix_size; INIT_LIST_HEAD(&efx->rss_context.list); + mutex_init(&efx->rss_lock); spin_lock_init(&efx->stats_lock); efx->vi_stride = EFX_DEFAULT_VI_STRIDE; efx->num_mac_stats = MC_CMD_MAC_NSTATS; @@ -3091,11 +3095,14 @@ void efx_update_sw_stats(struct efx_nic *efx, u64 *stats) /* RSS contexts. We're using linked lists and crappy O(n) algorithms, because * (a) this is an infrequent control-plane operation and (b) n is small (max 64) */ -struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *head) +struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx) { + struct list_head *head = &efx->rss_context.list; struct efx_rss_context *ctx, *new; u32 id = 1; /* Don't use zero, that refers to the master RSS context */ + WARN_ON(!mutex_is_locked(&efx->rss_lock)); + /* Search for first gap in the numbering */ list_for_each_entry(ctx, head, list) { if (ctx->user_id != id) @@ -3121,10 +3128,13 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *head) return new; } -struct efx_rss_context *efx_find_rss_context_entry(u32 id, struct list_head *head) +struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id) { + struct list_head *head = &efx->rss_context.list; struct efx_rss_context *ctx; + WARN_ON(!mutex_is_locked(&efx->rss_lock)); + list_for_each_entry(ctx, head, list) if (ctx->user_id == id) return ctx; diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h index 545c2ea1622e..a3140e16fcef 100644 --- a/drivers/net/ethernet/sfc/efx.h +++ b/drivers/net/ethernet/sfc/efx.h @@ -187,8 +187,8 @@ static inline void efx_filter_rfs_expire(struct work_struct *data) {} bool efx_filter_is_mc_recipient(const struct efx_filter_spec *spec); /* RSS contexts */ -struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *list); -struct efx_rss_context *efx_find_rss_context_entry(u32 id, struct list_head *list); +struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx); +struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id); void efx_free_rss_context_entry(struct efx_rss_context *ctx); static inline bool efx_rss_active(struct efx_rss_context *ctx) { diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c index bb1c80d48d12..e07c8bc8f4bd 100644 --- a/drivers/net/ethernet/sfc/ethtool.c +++ b/drivers/net/ethernet/sfc/ethtool.c @@ -979,7 +979,7 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev, { struct efx_nic *efx = netdev_priv(net_dev); u32 rss_context = 0; - s32 rc; + s32 rc = 0; switch (info->cmd) { case ETHTOOL_GRXRINGS: @@ -989,15 +989,17 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev, case ETHTOOL_GRXFH: { struct efx_rss_context *ctx = &efx->rss_context; + mutex_lock(&efx->rss_lock); if (info->flow_type & FLOW_RSS && info->rss_context) { - ctx = efx_find_rss_context_entry(info->rss_context, - &efx->rss_context.list); - if (!ctx) - return -ENOENT; + ctx = efx_find_rss_context_entry(efx, info->rss_context); + if (!ctx) { + rc = -ENOENT; + goto out_unlock; + } } info->data = 0; if (!efx_rss_active(ctx)) /* No RSS */ - return 0; + goto out_unlock; switch (info->flow_type & ~FLOW_RSS) { case UDP_V4_FLOW: if (ctx->rx_hash_udp_4tuple) @@ -1024,7 +1026,9 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev, default: break; } - return 0; +out_unlock: + mutex_unlock(&efx->rss_lock); + return rc; } case ETHTOOL_GRXCLSRLCNT: @@ -1366,16 +1370,20 @@ static int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir, { struct efx_nic *efx = netdev_priv(net_dev); struct efx_rss_context *ctx; - int rc; + int rc = 0; if (!efx->type->rx_pull_rss_context_config) return -EOPNOTSUPP; - ctx = efx_find_rss_context_entry(rss_context, &efx->rss_context.list); - if (!ctx) - return -ENOENT; + + mutex_lock(&efx->rss_lock); + ctx = efx_find_rss_context_entry(efx, rss_context); + if (!ctx) { + rc = -ENOENT; + goto out_unlock; + } rc = efx->type->rx_pull_rss_context_config(efx, ctx); if (rc) - return rc; + goto out_unlock; if (hfunc) *hfunc = ETH_RSS_HASH_TOP; @@ -1383,7 +1391,9 @@ static int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir, memcpy(indir, ctx->rx_indir_table, sizeof(ctx->rx_indir_table)); if (key) memcpy(key, ctx->rx_hash_key, efx->type->rx_hash_key_size); - return 0; +out_unlock: + mutex_unlock(&efx->rss_lock); + return rc; } static int efx_ethtool_set_rxfh_context(struct net_device *net_dev, @@ -1401,23 +1411,31 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev, /* Hash function is Toeplitz, cannot be changed */ if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP) return -EOPNOTSUPP; + + mutex_lock(&efx->rss_lock); + if (*rss_context == ETH_RXFH_CONTEXT_ALLOC) { - if (delete) + if (delete) { /* alloc + delete == Nothing to do */ - return -EINVAL; - ctx = efx_alloc_rss_context_entry(&efx->rss_context.list); - if (!ctx) - return -ENOMEM; + rc = -EINVAL; + goto out_unlock; + } + ctx = efx_alloc_rss_context_entry(efx); + if (!ctx) { + rc = -ENOMEM; + goto out_unlock; + } ctx->context_id = EFX_EF10_RSS_CONTEXT_INVALID; /* Initialise indir table and key to defaults */ efx_set_default_rx_indir_table(efx, ctx); netdev_rss_key_fill(ctx->rx_hash_key, sizeof(ctx->rx_hash_key)); allocated = true; } else { - ctx = efx_find_rss_context_entry(*rss_context, - &efx->rss_context.list); - if (!ctx) - return -ENOENT; + ctx = efx_find_rss_context_entry(efx, *rss_context); + if (!ctx) { + rc = -ENOENT; + goto out_unlock; + } } if (delete) { @@ -1425,7 +1443,7 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev, rc = efx->type->rx_push_rss_context_config(efx, ctx, NULL, NULL); if (!rc) efx_free_rss_context_entry(ctx); - return rc; + goto out_unlock; } if (!key) @@ -1438,6 +1456,8 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev, efx_free_rss_context_entry(ctx); else *rss_context = ctx->user_id; +out_unlock: + mutex_unlock(&efx->rss_lock); return rc; } diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h index 92ee55d84b7d..5e379a83c729 100644 --- a/drivers/net/ethernet/sfc/net_driver.h +++ b/drivers/net/ethernet/sfc/net_driver.h @@ -796,6 +796,7 @@ struct efx_rss_context { * @rx_scatter: Scatter mode enabled for receives * @rss_context: Main RSS context. Its @list member is the head of the list of * RSS contexts created by user requests + * @rss_lock: Protects custom RSS context software state in @rss_context.list * @int_error_count: Number of internal errors seen recently * @int_error_expire: Time at which error count will be expired * @irq_soft_enabled: Are IRQs soft-enabled? If not, IRQ handler will @@ -940,6 +941,7 @@ struct efx_nic { int rx_packet_ts_offset; bool rx_scatter; struct efx_rss_context rss_context; + struct mutex rss_lock; unsigned int_error_count; unsigned long int_error_expire; diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h index ac59afad6e5c..5640034bda10 100644 --- a/drivers/net/ethernet/sfc/nic.h +++ b/drivers/net/ethernet/sfc/nic.h @@ -365,6 +365,8 @@ enum { * @vi_base: Absolute index of first VI in this function * @n_allocated_vis: Number of VIs allocated to this function * @must_realloc_vis: Flag: VIs have yet to be reallocated after MC reboot + * @must_restore_rss_contexts: Flag: RSS contexts have yet to be restored after + * MC reboot * @must_restore_filters: Flag: filters have yet to be restored after MC reboot * @n_piobufs: Number of PIO buffers allocated to this function * @wc_membase: Base address of write-combining mapping of the memory BAR @@ -407,6 +409,7 @@ struct efx_ef10_nic_data { unsigned int vi_base; unsigned int n_allocated_vis; bool must_realloc_vis; + bool must_restore_rss_contexts; bool must_restore_filters; unsigned int n_piobufs; void __iomem *wc_membase, *pio_write_base; -- 2.30.2