Skip to content

Commit 5d22d47

Browse files
davem330davem330
committed
Merge branch 'sfc-filter-locking'
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 <[email protected]>
2 parents c709002 + a8e8fbe commit 5d22d47

File tree

9 files changed

+400
-484
lines changed

9 files changed

+400
-484
lines changed

‎drivers/net/ethernet/sfc/ef10.c

Lines changed: 190 additions & 355 deletions
Large diffs are not rendered by default.

‎drivers/net/ethernet/sfc/efx.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,10 @@ static int efx_poll(struct napi_struct *napi, int budget)
340340
efx_update_irq_mod(efx, channel);
341341
}
342342

343-
efx_filter_rfs_expire(channel);
343+
#ifdef CONFIG_RFS_ACCEL
344+
/* Perhaps expire some ARFS filters */
345+
schedule_work(&channel->filter_work);
346+
#endif
344347

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

476+
#ifdef CONFIG_RFS_ACCEL
477+
INIT_WORK(&channel->filter_work, efx_filter_rfs_expire);
478+
#endif
479+
473480
rx_queue = &channel->rx_queue;
474481
rx_queue->efx = efx;
475482
timer_setup(&rx_queue->slow_fill, efx_rx_slow_fill, 0);
@@ -512,6 +519,9 @@ efx_copy_channel(const struct efx_channel *old_channel)
512519
rx_queue->buffer = NULL;
513520
memset(&rx_queue->rxd, 0, sizeof(rx_queue->rxd));
514521
timer_setup(&rx_queue->slow_fill, efx_rx_slow_fill, 0);
522+
#ifdef CONFIG_RFS_ACCEL
523+
INIT_WORK(&channel->filter_work, efx_filter_rfs_expire);
524+
#endif
515525

516526
return channel;
517527
}
@@ -1773,7 +1783,6 @@ static int efx_probe_filters(struct efx_nic *efx)
17731783
{
17741784
int rc;
17751785

1776-
spin_lock_init(&efx->filter_lock);
17771786
init_rwsem(&efx->filter_sem);
17781787
mutex_lock(&efx->mac_lock);
17791788
down_write(&efx->filter_sem);
@@ -2648,6 +2657,7 @@ void efx_reset_down(struct efx_nic *efx, enum reset_type method)
26482657
efx_disable_interrupts(efx);
26492658

26502659
mutex_lock(&efx->mac_lock);
2660+
mutex_lock(&efx->rss_lock);
26512661
if (efx->port_initialized && method != RESET_TYPE_INVISIBLE &&
26522662
method != RESET_TYPE_DATAPATH)
26532663
efx->phy_op->fini(efx);
@@ -2703,6 +2713,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
27032713

27042714
if (efx->type->rx_restore_rss_contexts)
27052715
efx->type->rx_restore_rss_contexts(efx);
2716+
mutex_unlock(&efx->rss_lock);
27062717
down_read(&efx->filter_sem);
27072718
efx_restore_filters(efx);
27082719
up_read(&efx->filter_sem);
@@ -2721,6 +2732,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
27212732
fail:
27222733
efx->port_initialized = false;
27232734

2735+
mutex_unlock(&efx->rss_lock);
27242736
mutex_unlock(&efx->mac_lock);
27252737

27262738
return rc;
@@ -3007,11 +3019,15 @@ static int efx_init_struct(struct efx_nic *efx,
30073019
efx->rx_packet_ts_offset =
30083020
efx->type->rx_ts_offset - efx->type->rx_prefix_size;
30093021
INIT_LIST_HEAD(&efx->rss_context.list);
3022+
mutex_init(&efx->rss_lock);
30103023
spin_lock_init(&efx->stats_lock);
30113024
efx->vi_stride = EFX_DEFAULT_VI_STRIDE;
30123025
efx->num_mac_stats = MC_CMD_MAC_NSTATS;
30133026
BUILD_BUG_ON(MC_CMD_MAC_NSTATS - 1 != MC_CMD_MAC_GENERATION_END);
30143027
mutex_init(&efx->mac_lock);
3028+
#ifdef CONFIG_RFS_ACCEL
3029+
mutex_init(&efx->rps_mutex);
3030+
#endif
30153031
efx->phy_op = &efx_dummy_phy_operations;
30163032
efx->mdio.dev = net_dev;
30173033
INIT_WORK(&efx->mac_work, efx_mac_work);
@@ -3079,11 +3095,14 @@ void efx_update_sw_stats(struct efx_nic *efx, u64 *stats)
30793095
/* RSS contexts. We're using linked lists and crappy O(n) algorithms, because
30803096
* (a) this is an infrequent control-plane operation and (b) n is small (max 64)
30813097
*/
3082-
struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *head)
3098+
struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx)
30833099
{
3100+
struct list_head *head = &efx->rss_context.list;
30843101
struct efx_rss_context *ctx, *new;
30853102
u32 id = 1; /* Don't use zero, that refers to the master RSS context */
30863103

3104+
WARN_ON(!mutex_is_locked(&efx->rss_lock));
3105+
30873106
/* Search for first gap in the numbering */
30883107
list_for_each_entry(ctx, head, list) {
30893108
if (ctx->user_id != id)
@@ -3109,10 +3128,13 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *head)
31093128
return new;
31103129
}
31113130

3112-
struct efx_rss_context *efx_find_rss_context_entry(u32 id, struct list_head *head)
3131+
struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id)
31133132
{
3133+
struct list_head *head = &efx->rss_context.list;
31143134
struct efx_rss_context *ctx;
31153135

3136+
WARN_ON(!mutex_is_locked(&efx->rss_lock));
3137+
31163138
list_for_each_entry(ctx, head, list)
31173139
if (ctx->user_id == id)
31183140
return ctx;

‎drivers/net/ethernet/sfc/efx.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,22 +170,25 @@ static inline s32 efx_filter_get_rx_ids(struct efx_nic *efx,
170170
int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
171171
u16 rxq_index, u32 flow_id);
172172
bool __efx_filter_rfs_expire(struct efx_nic *efx, unsigned quota);
173-
static inline void efx_filter_rfs_expire(struct efx_channel *channel)
173+
static inline void efx_filter_rfs_expire(struct work_struct *data)
174174
{
175+
struct efx_channel *channel = container_of(data, struct efx_channel,
176+
filter_work);
177+
175178
if (channel->rfs_filters_added >= 60 &&
176179
__efx_filter_rfs_expire(channel->efx, 100))
177180
channel->rfs_filters_added -= 60;
178181
}
179182
#define efx_filter_rfs_enabled() 1
180183
#else
181-
static inline void efx_filter_rfs_expire(struct efx_channel *channel) {}
184+
static inline void efx_filter_rfs_expire(struct work_struct *data) {}
182185
#define efx_filter_rfs_enabled() 0
183186
#endif
184187
bool efx_filter_is_mc_recipient(const struct efx_filter_spec *spec);
185188

186189
/* RSS contexts */
187-
struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *list);
188-
struct efx_rss_context *efx_find_rss_context_entry(u32 id, struct list_head *list);
190+
struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx);
191+
struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id);
189192
void efx_free_rss_context_entry(struct efx_rss_context *ctx);
190193
static inline bool efx_rss_active(struct efx_rss_context *ctx)
191194
{

‎drivers/net/ethernet/sfc/ethtool.c

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -979,7 +979,7 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
979979
{
980980
struct efx_nic *efx = netdev_priv(net_dev);
981981
u32 rss_context = 0;
982-
s32 rc;
982+
s32 rc = 0;
983983

984984
switch (info->cmd) {
985985
case ETHTOOL_GRXRINGS:
@@ -989,15 +989,17 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
989989
case ETHTOOL_GRXFH: {
990990
struct efx_rss_context *ctx = &efx->rss_context;
991991

992+
mutex_lock(&efx->rss_lock);
992993
if (info->flow_type & FLOW_RSS && info->rss_context) {
993-
ctx = efx_find_rss_context_entry(info->rss_context,
994-
&efx->rss_context.list);
995-
if (!ctx)
996-
return -ENOENT;
994+
ctx = efx_find_rss_context_entry(efx, info->rss_context);
995+
if (!ctx) {
996+
rc = -ENOENT;
997+
goto out_unlock;
998+
}
997999
}
9981000
info->data = 0;
9991001
if (!efx_rss_active(ctx)) /* No RSS */
1000-
return 0;
1002+
goto out_unlock;
10011003
switch (info->flow_type & ~FLOW_RSS) {
10021004
case UDP_V4_FLOW:
10031005
if (ctx->rx_hash_udp_4tuple)
@@ -1024,7 +1026,9 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
10241026
default:
10251027
break;
10261028
}
1027-
return 0;
1029+
out_unlock:
1030+
mutex_unlock(&efx->rss_lock);
1031+
return rc;
10281032
}
10291033

10301034
case ETHTOOL_GRXCLSRLCNT:
@@ -1084,6 +1088,7 @@ static int efx_ethtool_set_class_rule(struct efx_nic *efx,
10841088
struct ethtool_tcpip6_spec *ip6_mask = &rule->m_u.tcp_ip6_spec;
10851089
struct ethtool_usrip6_spec *uip6_entry = &rule->h_u.usr_ip6_spec;
10861090
struct ethtool_usrip6_spec *uip6_mask = &rule->m_u.usr_ip6_spec;
1091+
u32 flow_type = rule->flow_type & ~(FLOW_EXT | FLOW_RSS);
10871092
struct ethhdr *mac_entry = &rule->h_u.ether_spec;
10881093
struct ethhdr *mac_mask = &rule->m_u.ether_spec;
10891094
enum efx_filter_flags flags = 0;
@@ -1117,14 +1122,14 @@ static int efx_ethtool_set_class_rule(struct efx_nic *efx,
11171122
if (rule->flow_type & FLOW_RSS)
11181123
spec.rss_context = rss_context;
11191124

1120-
switch (rule->flow_type & ~(FLOW_EXT | FLOW_RSS)) {
1125+
switch (flow_type) {
11211126
case TCP_V4_FLOW:
11221127
case UDP_V4_FLOW:
11231128
spec.match_flags = (EFX_FILTER_MATCH_ETHER_TYPE |
11241129
EFX_FILTER_MATCH_IP_PROTO);
11251130
spec.ether_type = htons(ETH_P_IP);
1126-
spec.ip_proto = ((rule->flow_type & ~FLOW_EXT) == TCP_V4_FLOW ?
1127-
IPPROTO_TCP : IPPROTO_UDP);
1131+
spec.ip_proto = flow_type == TCP_V4_FLOW ? IPPROTO_TCP
1132+
: IPPROTO_UDP;
11281133
if (ip_mask->ip4dst) {
11291134
if (ip_mask->ip4dst != IP4_ADDR_FULL_MASK)
11301135
return -EINVAL;
@@ -1158,8 +1163,8 @@ static int efx_ethtool_set_class_rule(struct efx_nic *efx,
11581163
spec.match_flags = (EFX_FILTER_MATCH_ETHER_TYPE |
11591164
EFX_FILTER_MATCH_IP_PROTO);
11601165
spec.ether_type = htons(ETH_P_IPV6);
1161-
spec.ip_proto = ((rule->flow_type & ~FLOW_EXT) == TCP_V6_FLOW ?
1162-
IPPROTO_TCP : IPPROTO_UDP);
1166+
spec.ip_proto = flow_type == TCP_V6_FLOW ? IPPROTO_TCP
1167+
: IPPROTO_UDP;
11631168
if (!ip6_mask_is_empty(ip6_mask->ip6dst)) {
11641169
if (!ip6_mask_is_full(ip6_mask->ip6dst))
11651170
return -EINVAL;
@@ -1366,24 +1371,30 @@ static int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir,
13661371
{
13671372
struct efx_nic *efx = netdev_priv(net_dev);
13681373
struct efx_rss_context *ctx;
1369-
int rc;
1374+
int rc = 0;
13701375

13711376
if (!efx->type->rx_pull_rss_context_config)
13721377
return -EOPNOTSUPP;
1373-
ctx = efx_find_rss_context_entry(rss_context, &efx->rss_context.list);
1374-
if (!ctx)
1375-
return -ENOENT;
1378+
1379+
mutex_lock(&efx->rss_lock);
1380+
ctx = efx_find_rss_context_entry(efx, rss_context);
1381+
if (!ctx) {
1382+
rc = -ENOENT;
1383+
goto out_unlock;
1384+
}
13761385
rc = efx->type->rx_pull_rss_context_config(efx, ctx);
13771386
if (rc)
1378-
return rc;
1387+
goto out_unlock;
13791388

13801389
if (hfunc)
13811390
*hfunc = ETH_RSS_HASH_TOP;
13821391
if (indir)
13831392
memcpy(indir, ctx->rx_indir_table, sizeof(ctx->rx_indir_table));
13841393
if (key)
13851394
memcpy(key, ctx->rx_hash_key, efx->type->rx_hash_key_size);
1386-
return 0;
1395+
out_unlock:
1396+
mutex_unlock(&efx->rss_lock);
1397+
return rc;
13871398
}
13881399

13891400
static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
@@ -1401,31 +1412,39 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
14011412
/* Hash function is Toeplitz, cannot be changed */
14021413
if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
14031414
return -EOPNOTSUPP;
1415+
1416+
mutex_lock(&efx->rss_lock);
1417+
14041418
if (*rss_context == ETH_RXFH_CONTEXT_ALLOC) {
1405-
if (delete)
1419+
if (delete) {
14061420
/* alloc + delete == Nothing to do */
1407-
return -EINVAL;
1408-
ctx = efx_alloc_rss_context_entry(&efx->rss_context.list);
1409-
if (!ctx)
1410-
return -ENOMEM;
1421+
rc = -EINVAL;
1422+
goto out_unlock;
1423+
}
1424+
ctx = efx_alloc_rss_context_entry(efx);
1425+
if (!ctx) {
1426+
rc = -ENOMEM;
1427+
goto out_unlock;
1428+
}
14111429
ctx->context_id = EFX_EF10_RSS_CONTEXT_INVALID;
14121430
/* Initialise indir table and key to defaults */
14131431
efx_set_default_rx_indir_table(efx, ctx);
14141432
netdev_rss_key_fill(ctx->rx_hash_key, sizeof(ctx->rx_hash_key));
14151433
allocated = true;
14161434
} else {
1417-
ctx = efx_find_rss_context_entry(*rss_context,
1418-
&efx->rss_context.list);
1419-
if (!ctx)
1420-
return -ENOENT;
1435+
ctx = efx_find_rss_context_entry(efx, *rss_context);
1436+
if (!ctx) {
1437+
rc = -ENOENT;
1438+
goto out_unlock;
1439+
}
14211440
}
14221441

14231442
if (delete) {
14241443
/* delete this context */
14251444
rc = efx->type->rx_push_rss_context_config(efx, ctx, NULL, NULL);
14261445
if (!rc)
14271446
efx_free_rss_context_entry(ctx);
1428-
return rc;
1447+
goto out_unlock;
14291448
}
14301449

14311450
if (!key)
@@ -1438,6 +1457,8 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
14381457
efx_free_rss_context_entry(ctx);
14391458
else
14401459
*rss_context = ctx->user_id;
1460+
out_unlock:
1461+
mutex_unlock(&efx->rss_lock);
14411462
return rc;
14421463
}
14431464

0 commit comments

Comments
 (0)