Skip to content

Commit

Permalink
Merge branch 'sfc-filter-locking'
Browse files Browse the repository at this point in the history 
Edward Cree says:

====================
sfc: rework locking around filter management

The use of a spinlock to protect filter state combined with the need for a
 sleeping operation (MCDI) to apply that state to the NIC (on EF10) led to
 unfixable race conditions, around the handling of filter restoration after
 an MC reboot.
So, this patch series removes the requirement to be able to modify the SW
 filter table from atomic context, by using a workqueue to request
 asynchronous filter operations (which are needed for ARFS).  Then, the
 filter table locks are changed to mutexes, replacing the dance of spinlocks
 and 'busy' flags.  Also, a mutex is added to protect the RSS context state,
 since otherwise a similar race is possible around restoring that after an
 MC reboot.  While we're at it, fix a couple of other related bugs.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Mar 27, 2018
2 parents c709002 + a8e8fbe commit 5d22d47
Show file tree
Hide file tree
Showing 9 changed files with 400 additions and 484 deletions.
545 changes: 190 additions & 355 deletions drivers/net/ethernet/sfc/ef10.c
View file

Large diffs are not rendered by default.

30 changes: 26 additions & 4 deletions drivers/net/ethernet/sfc/efx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,10 @@ static int efx_poll(struct napi_struct *napi, int budget)
efx_update_irq_mod(efx, channel);
}

efx_filter_rfs_expire(channel);
#ifdef CONFIG_RFS_ACCEL
/* Perhaps expire some ARFS filters */
schedule_work(&channel->filter_work);
#endif

/* There is no race here; although napi_disable() will
* only wait for napi_complete(), this isn't a problem
Expand Down Expand Up @@ -470,6 +473,10 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
tx_queue->channel = channel;
}

#ifdef CONFIG_RFS_ACCEL
INIT_WORK(&channel->filter_work, efx_filter_rfs_expire);
#endif

rx_queue = &channel->rx_queue;
rx_queue->efx = efx;
timer_setup(&rx_queue->slow_fill, efx_rx_slow_fill, 0);
Expand Down Expand Up @@ -512,6 +519,9 @@ efx_copy_channel(const struct efx_channel *old_channel)
rx_queue->buffer = NULL;
memset(&rx_queue->rxd, 0, sizeof(rx_queue->rxd));
timer_setup(&rx_queue->slow_fill, efx_rx_slow_fill, 0);
#ifdef CONFIG_RFS_ACCEL
INIT_WORK(&channel->filter_work, efx_filter_rfs_expire);
#endif

return channel;
}
Expand Down Expand Up @@ -1773,7 +1783,6 @@ static int efx_probe_filters(struct efx_nic *efx)
{
int rc;

spin_lock_init(&efx->filter_lock);
init_rwsem(&efx->filter_sem);
mutex_lock(&efx->mac_lock);
down_write(&efx->filter_sem);
Expand Down Expand Up @@ -2648,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);
Expand Down Expand Up @@ -2703,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);
Expand All @@ -2721,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;
Expand Down Expand Up @@ -3007,11 +3019,15 @@ 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;
BUILD_BUG_ON(MC_CMD_MAC_NSTATS - 1 != MC_CMD_MAC_GENERATION_END);
mutex_init(&efx->mac_lock);
#ifdef CONFIG_RFS_ACCEL
mutex_init(&efx->rps_mutex);
#endif
efx->phy_op = &efx_dummy_phy_operations;
efx->mdio.dev = net_dev;
INIT_WORK(&efx->mac_work, efx_mac_work);
Expand Down Expand Up @@ -3079,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)
Expand All @@ -3109,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;
Expand Down
11 changes: 7 additions & 4 deletions drivers/net/ethernet/sfc/efx.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,22 +170,25 @@ static inline s32 efx_filter_get_rx_ids(struct efx_nic *efx,
int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
u16 rxq_index, u32 flow_id);
bool __efx_filter_rfs_expire(struct efx_nic *efx, unsigned quota);
static inline void efx_filter_rfs_expire(struct efx_channel *channel)
static inline void efx_filter_rfs_expire(struct work_struct *data)
{
struct efx_channel *channel = container_of(data, struct efx_channel,
filter_work);

if (channel->rfs_filters_added >= 60 &&
__efx_filter_rfs_expire(channel->efx, 100))
channel->rfs_filters_added -= 60;
}
#define efx_filter_rfs_enabled() 1
#else
static inline void efx_filter_rfs_expire(struct efx_channel *channel) {}
static inline void efx_filter_rfs_expire(struct work_struct *data) {}
#define efx_filter_rfs_enabled() 0
#endif
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)
{
Expand Down
77 changes: 49 additions & 28 deletions drivers/net/ethernet/sfc/ethtool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand All @@ -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:
Expand Down Expand Up @@ -1084,6 +1088,7 @@ static int efx_ethtool_set_class_rule(struct efx_nic *efx,
struct ethtool_tcpip6_spec *ip6_mask = &rule->m_u.tcp_ip6_spec;
struct ethtool_usrip6_spec *uip6_entry = &rule->h_u.usr_ip6_spec;
struct ethtool_usrip6_spec *uip6_mask = &rule->m_u.usr_ip6_spec;
u32 flow_type = rule->flow_type & ~(FLOW_EXT | FLOW_RSS);
struct ethhdr *mac_entry = &rule->h_u.ether_spec;
struct ethhdr *mac_mask = &rule->m_u.ether_spec;
enum efx_filter_flags flags = 0;
Expand Down Expand Up @@ -1117,14 +1122,14 @@ static int efx_ethtool_set_class_rule(struct efx_nic *efx,
if (rule->flow_type & FLOW_RSS)
spec.rss_context = rss_context;

switch (rule->flow_type & ~(FLOW_EXT | FLOW_RSS)) {
switch (flow_type) {
case TCP_V4_FLOW:
case UDP_V4_FLOW:
spec.match_flags = (EFX_FILTER_MATCH_ETHER_TYPE |
EFX_FILTER_MATCH_IP_PROTO);
spec.ether_type = htons(ETH_P_IP);
spec.ip_proto = ((rule->flow_type & ~FLOW_EXT) == TCP_V4_FLOW ?
IPPROTO_TCP : IPPROTO_UDP);
spec.ip_proto = flow_type == TCP_V4_FLOW ? IPPROTO_TCP
: IPPROTO_UDP;
if (ip_mask->ip4dst) {
if (ip_mask->ip4dst != IP4_ADDR_FULL_MASK)
return -EINVAL;
Expand Down Expand Up @@ -1158,8 +1163,8 @@ static int efx_ethtool_set_class_rule(struct efx_nic *efx,
spec.match_flags = (EFX_FILTER_MATCH_ETHER_TYPE |
EFX_FILTER_MATCH_IP_PROTO);
spec.ether_type = htons(ETH_P_IPV6);
spec.ip_proto = ((rule->flow_type & ~FLOW_EXT) == TCP_V6_FLOW ?
IPPROTO_TCP : IPPROTO_UDP);
spec.ip_proto = flow_type == TCP_V6_FLOW ? IPPROTO_TCP
: IPPROTO_UDP;
if (!ip6_mask_is_empty(ip6_mask->ip6dst)) {
if (!ip6_mask_is_full(ip6_mask->ip6dst))
return -EINVAL;
Expand Down Expand Up @@ -1366,24 +1371,30 @@ 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;
if (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,
Expand All @@ -1401,31 +1412,39 @@ 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) {
/* delete this context */
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)
Expand All @@ -1438,6 +1457,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;
}

Expand Down
Loading

0 comments on commit 5d22d47

Please sign in to comment.