From d9ccfdd4b3b675750518e035549f5c778d4027f2 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 14 Jan 2013 21:23:17 +0000 Subject: [PATCH] sfc: Fix replacement detection in efx_filter_insert_filter() efx_filter_insert_filter() uses the first table entry in the hash chain that either has the same match values or is empty. This means that replacement doesn't always work correctly: 1. Insert filter F1 with match values M1, hashing to H1, at first possible entry E1. 2. Insert filter F2 with match values M2, hashing to H1, at second possible entry E2. 3. Remove filter F1. 4. Insert filter F3 with match values M2, hashing to H1, at first possible entry E1. F3 should have either replaced F2 or been rejected (depending on priority and the replace_equal parameter). Instead, search for both a matching filter that the inserted filter would replace, and an available insertion point, up to the applicable maximum search depths. If we insert at lower depth than a replaced filter, clear the replaced filter. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/filter.c | 89 +++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 23 deletions(-) diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c index 769560a2cb34..0de8daf01c01 100644 --- a/drivers/net/ethernet/sfc/filter.c +++ b/drivers/net/ethernet/sfc/filter.c @@ -66,6 +66,10 @@ struct efx_filter_state { #endif }; +static void efx_filter_table_clear_entry(struct efx_nic *efx, + struct efx_filter_table *table, + unsigned int filter_idx); + /* The filter hash function is LFSR polynomial x^16 + x^3 + 1 of a 32-bit * key derived from the n-tuple. The initial LFSR state is 0xffff. */ static u16 efx_filter_hash(u32 key) @@ -626,9 +630,9 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec, { struct efx_filter_state *state = efx->filter_state; struct efx_filter_table *table = efx_filter_spec_table(state, spec); - struct efx_filter_spec *saved_spec; efx_oword_t filter; - unsigned int filter_idx, depth = 0; + int rep_index, ins_index; + unsigned int depth = 0; int rc; if (!table || table->size == 0) @@ -643,44 +647,74 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec, BUILD_BUG_ON(EFX_FILTER_INDEX_UC_DEF != 0); BUILD_BUG_ON(EFX_FILTER_INDEX_MC_DEF != EFX_FILTER_MC_DEF - EFX_FILTER_UC_DEF); - filter_idx = spec->type - EFX_FILTER_INDEX_UC_DEF; + rep_index = spec->type - EFX_FILTER_INDEX_UC_DEF; + ins_index = rep_index; spin_lock_bh(&state->lock); } else { + /* Search concurrently for + * (1) a filter to be replaced (rep_index): any filter + * with the same match values, up to the current + * search depth for this type, and + * (2) the insertion point (ins_index): (1) or any + * free slot before it or up to the maximum search + * depth for this priority + * We fail if we cannot find (2). + * + * We can stop once either + * (a) we find (1), in which case we have definitely + * found (2) as well; or + * (b) we have searched exhaustively for (1), and have + * either found (2) or searched exhaustively for it + */ u32 key = efx_filter_build(&filter, spec); unsigned int hash = efx_filter_hash(key); unsigned int incr = efx_filter_increment(key); - unsigned int depth_max = + unsigned int max_rep_depth = table->search_depth[spec->type]; + unsigned int max_ins_depth = spec->priority <= EFX_FILTER_PRI_HINT ? FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX; + unsigned int i = hash & (table->size - 1); - filter_idx = hash & (table->size - 1); + ins_index = -1; depth = 1; spin_lock_bh(&state->lock); for (;;) { - if (!test_bit(filter_idx, table->used_bitmap) || - efx_filter_equal(spec, &table->spec[filter_idx])) + if (!test_bit(i, table->used_bitmap)) { + if (ins_index < 0) + ins_index = i; + } else if (efx_filter_equal(spec, &table->spec[i])) { + /* Case (a) */ + if (ins_index < 0) + ins_index = i; + rep_index = i; break; + } - /* Return failure if we reached the maximum search - * depth - */ - if (depth == depth_max) { - rc = -EBUSY; - goto out; + if (depth >= max_rep_depth && + (ins_index >= 0 || depth >= max_ins_depth)) { + /* Case (b) */ + if (ins_index < 0) { + rc = -EBUSY; + goto out; + } + rep_index = -1; + break; } - filter_idx = (filter_idx + incr) & (table->size - 1); + i = (i + incr) & (table->size - 1); ++depth; } } - saved_spec = &table->spec[filter_idx]; + /* If we found a filter to be replaced, check whether we + * should do so + */ + if (rep_index >= 0) { + struct efx_filter_spec *saved_spec = &table->spec[rep_index]; - if (test_bit(filter_idx, table->used_bitmap)) { - /* Should we replace the existing filter? */ if (spec->priority == saved_spec->priority && !replace_equal) { rc = -EEXIST; goto out; @@ -689,11 +723,14 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec, rc = -EPERM; goto out; } - } else { - __set_bit(filter_idx, table->used_bitmap); + } + + /* Insert the filter */ + if (ins_index != rep_index) { + __set_bit(ins_index, table->used_bitmap); ++table->used; } - *saved_spec = *spec; + table->spec[ins_index] = *spec; if (table->id == EFX_FILTER_TABLE_RX_DEF) { efx_filter_push_rx_config(efx); @@ -707,13 +744,19 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec, } efx_writeo(efx, &filter, - table->offset + table->step * filter_idx); + table->offset + table->step * ins_index); + + /* If we were able to replace a filter by inserting + * at a lower depth, clear the replaced filter + */ + if (ins_index != rep_index && rep_index >= 0) + efx_filter_table_clear_entry(efx, table, rep_index); } netif_vdbg(efx, hw, efx->net_dev, "%s: filter type %d index %d rxq %u set", - __func__, spec->type, filter_idx, spec->dmaq_id); - rc = efx_filter_make_id(spec, filter_idx); + __func__, spec->type, ins_index, spec->dmaq_id); + rc = efx_filter_make_id(spec, ins_index); out: spin_unlock_bh(&state->lock); -- 2.20.1