Skip to content

Commit

Permalink
Merge branch 'mlxsw-acl-fixes'
Browse files Browse the repository at this point in the history
Petr Machata says:

====================
mlxsw: ACL fixes

Ido Schimmel writes:

Patches #1-#3 fix various spelling mistakes I noticed while working on
the code base.

Patch #4 fixes a general protection fault by bailing out when the error
occurs and warning.

Patch #5 fixes the warning.

Patch #6 fixes ACL scale regression and firmware errors.

See the commit messages for more info.
====================

Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
davem330 committed Jun 10, 2024
2 parents 28f961f + 75d8d7a commit 8d466c8
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 53 deletions.
20 changes: 9 additions & 11 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,15 +391,16 @@ mlxsw_sp_acl_atcam_region_entry_insert(struct mlxsw_sp *mlxsw_sp,
if (err)
return err;

lkey_id = aregion->ops->lkey_id_get(aregion, aentry->enc_key, erp_id);
lkey_id = aregion->ops->lkey_id_get(aregion, aentry->ht_key.enc_key,
erp_id);
if (IS_ERR(lkey_id))
return PTR_ERR(lkey_id);
aentry->lkey_id = lkey_id;

kvdl_index = mlxsw_afa_block_first_kvdl_index(rulei->act_block);
mlxsw_reg_ptce3_pack(ptce3_pl, true, MLXSW_REG_PTCE3_OP_WRITE_WRITE,
priority, region->tcam_region_info,
aentry->enc_key, erp_id,
aentry->ht_key.enc_key, erp_id,
aentry->delta_info.start,
aentry->delta_info.mask,
aentry->delta_info.value,
Expand Down Expand Up @@ -428,7 +429,7 @@ mlxsw_sp_acl_atcam_region_entry_remove(struct mlxsw_sp *mlxsw_sp,

mlxsw_reg_ptce3_pack(ptce3_pl, false, MLXSW_REG_PTCE3_OP_WRITE_WRITE, 0,
region->tcam_region_info,
aentry->enc_key, erp_id,
aentry->ht_key.enc_key, erp_id,
aentry->delta_info.start,
aentry->delta_info.mask,
aentry->delta_info.value,
Expand Down Expand Up @@ -457,7 +458,7 @@ mlxsw_sp_acl_atcam_region_entry_action_replace(struct mlxsw_sp *mlxsw_sp,
kvdl_index = mlxsw_afa_block_first_kvdl_index(rulei->act_block);
mlxsw_reg_ptce3_pack(ptce3_pl, true, MLXSW_REG_PTCE3_OP_WRITE_UPDATE,
priority, region->tcam_region_info,
aentry->enc_key, erp_id,
aentry->ht_key.enc_key, erp_id,
aentry->delta_info.start,
aentry->delta_info.mask,
aentry->delta_info.value,
Expand All @@ -480,26 +481,23 @@ __mlxsw_sp_acl_atcam_entry_add(struct mlxsw_sp *mlxsw_sp,
int err;

mlxsw_afk_encode(afk, region->key_info, &rulei->values,
aentry->ht_key.full_enc_key, mask);
aentry->ht_key.enc_key, mask);

erp_mask = mlxsw_sp_acl_erp_mask_get(aregion, mask, false);
if (IS_ERR(erp_mask))
return PTR_ERR(erp_mask);
aentry->erp_mask = erp_mask;
aentry->ht_key.erp_id = mlxsw_sp_acl_erp_mask_erp_id(erp_mask);
memcpy(aentry->enc_key, aentry->ht_key.full_enc_key,
sizeof(aentry->enc_key));

/* Compute all needed delta information and clear the delta bits
* from the encrypted key.
* from the encoded key.
*/
delta = mlxsw_sp_acl_erp_delta(aentry->erp_mask);
aentry->delta_info.start = mlxsw_sp_acl_erp_delta_start(delta);
aentry->delta_info.mask = mlxsw_sp_acl_erp_delta_mask(delta);
aentry->delta_info.value =
mlxsw_sp_acl_erp_delta_value(delta,
aentry->ht_key.full_enc_key);
mlxsw_sp_acl_erp_delta_clear(delta, aentry->enc_key);
mlxsw_sp_acl_erp_delta_value(delta, aentry->ht_key.enc_key);
mlxsw_sp_acl_erp_delta_clear(delta, aentry->ht_key.enc_key);

/* Add rule to the list of A-TCAM rules, assuming this
* rule is intended to A-TCAM. In case this rule does
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
memcpy(chunk + pad_bytes, &erp_region_id,
sizeof(erp_region_id));
memcpy(chunk + key_offset,
&aentry->enc_key[chunk_key_offsets[chunk_index]],
&aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
chunk_key_len);
chunk += chunk_len;
}
Expand Down
13 changes: 0 additions & 13 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1217,18 +1217,6 @@ static bool mlxsw_sp_acl_erp_delta_check(void *priv, const void *parent_obj,
return err ? false : true;
}

static int mlxsw_sp_acl_erp_hints_obj_cmp(const void *obj1, const void *obj2)
{
const struct mlxsw_sp_acl_erp_key *key1 = obj1;
const struct mlxsw_sp_acl_erp_key *key2 = obj2;

/* For hints purposes, two objects are considered equal
* in case the masks are the same. Does not matter what
* the "ctcam" value is.
*/
return memcmp(key1->mask, key2->mask, sizeof(key1->mask));
}

static void *mlxsw_sp_acl_erp_delta_create(void *priv, void *parent_obj,
void *obj)
{
Expand Down Expand Up @@ -1308,7 +1296,6 @@ static void mlxsw_sp_acl_erp_root_destroy(void *priv, void *root_priv)
static const struct objagg_ops mlxsw_sp_acl_erp_objagg_ops = {
.obj_size = sizeof(struct mlxsw_sp_acl_erp_key),
.delta_check = mlxsw_sp_acl_erp_delta_check,
.hints_obj_cmp = mlxsw_sp_acl_erp_hints_obj_cmp,
.delta_create = mlxsw_sp_acl_erp_delta_create,
.delta_destroy = mlxsw_sp_acl_erp_delta_destroy,
.root_create = mlxsw_sp_acl_erp_root_create,
Expand Down
9 changes: 3 additions & 6 deletions drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ struct mlxsw_sp_acl_atcam_region {
};

struct mlxsw_sp_acl_atcam_entry_ht_key {
char full_enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded
* key.
*/
char enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded key, minus
* delta bits.
*/
u8 erp_id;
};

Expand All @@ -181,9 +181,6 @@ struct mlxsw_sp_acl_atcam_entry {
struct rhash_head ht_node;
struct list_head list; /* Member in entries_list */
struct mlxsw_sp_acl_atcam_entry_ht_key ht_key;
char enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded key,
* minus delta bits.
*/
struct {
u16 start;
u8 mask;
Expand Down
1 change: 0 additions & 1 deletion include/linux/objagg.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ struct objagg_ops {
size_t obj_size;
bool (*delta_check)(void *priv, const void *parent_obj,
const void *obj);
int (*hints_obj_cmp)(const void *obj1, const void *obj2);
void * (*delta_create)(void *priv, void *parent_obj, void *obj);
void (*delta_destroy)(void *priv, void *delta_priv);
void * (*root_create)(void *priv, void *obj, unsigned int root_id);
Expand Down
20 changes: 4 additions & 16 deletions lib/objagg.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ static int objagg_obj_parent_assign(struct objagg *objagg,
{
void *delta_priv;

if (WARN_ON(!objagg_obj_is_root(parent)))
return -EINVAL;

delta_priv = objagg->ops->delta_create(objagg->priv, parent->obj,
objagg_obj->obj);
if (IS_ERR(delta_priv))
Expand Down Expand Up @@ -421,7 +424,7 @@ static struct objagg_obj *__objagg_obj_get(struct objagg *objagg, void *obj)
*
* There are 3 main options this function wraps:
* 1) The object according to "obj" already exist. In that case
* the reference counter is incrementes and the object is returned.
* the reference counter is incremented and the object is returned.
* 2) The object does not exist, but it can be aggregated within
* another object. In that case, user ops->delta_create() is called
* to obtain delta data and a new object is created with returned
Expand Down Expand Up @@ -903,20 +906,6 @@ static const struct objagg_opt_algo *objagg_opt_algos[] = {
[OBJAGG_OPT_ALGO_SIMPLE_GREEDY] = &objagg_opt_simple_greedy,
};

static int objagg_hints_obj_cmp(struct rhashtable_compare_arg *arg,
const void *obj)
{
struct rhashtable *ht = arg->ht;
struct objagg_hints *objagg_hints =
container_of(ht, struct objagg_hints, node_ht);
const struct objagg_ops *ops = objagg_hints->ops;
const char *ptr = obj;

ptr += ht->p.key_offset;
return ops->hints_obj_cmp ? ops->hints_obj_cmp(ptr, arg->key) :
memcmp(ptr, arg->key, ht->p.key_len);
}

/**
* objagg_hints_get - obtains hints instance
* @objagg: objagg instance
Expand Down Expand Up @@ -955,7 +944,6 @@ struct objagg_hints *objagg_hints_get(struct objagg *objagg,
offsetof(struct objagg_hints_node, obj);
objagg_hints->ht_params.head_offset =
offsetof(struct objagg_hints_node, ht_node);
objagg_hints->ht_params.obj_cmpfn = objagg_hints_obj_cmp;

err = rhashtable_init(&objagg_hints->node_ht, &objagg_hints->ht_params);
if (err)
Expand Down
2 changes: 1 addition & 1 deletion lib/test_objagg.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static struct objagg_obj *world_obj_get(struct world *world,
if (!world->key_refs[key_id_index(key_id)]) {
world->objagg_objs[key_id_index(key_id)] = objagg_obj;
} else if (world->objagg_objs[key_id_index(key_id)] != objagg_obj) {
pr_err("Key %u: God another object for the same key.\n",
pr_err("Key %u: Got another object for the same key.\n",
key_id);
err = -EINVAL;
goto err_key_id_check;
Expand Down
55 changes: 51 additions & 4 deletions tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ ALL_TESTS="single_mask_test identical_filters_test two_masks_test \
multiple_masks_test ctcam_edge_cases_test delta_simple_test \
delta_two_masks_one_key_test delta_simple_rehash_test \
bloom_simple_test bloom_complex_test bloom_delta_test \
max_erp_entries_test max_group_size_test"
max_erp_entries_test max_group_size_test collision_test"
NUM_NETIFS=2
source $lib_dir/lib.sh
source $lib_dir/tc_common.sh
Expand Down Expand Up @@ -457,7 +457,7 @@ delta_two_masks_one_key_test()
{
# If 2 keys are the same and only differ in mask in a way that
# they belong under the same ERP (second is delta of the first),
# there should be no C-TCAM spill.
# there should be C-TCAM spill.

RET=0

Expand All @@ -474,8 +474,8 @@ delta_two_masks_one_key_test()
tp_record "mlxsw:*" "tc filter add dev $h2 ingress protocol ip \
pref 2 handle 102 flower $tcflags dst_ip 192.0.2.2 \
action drop"
tp_check_hits "mlxsw:mlxsw_sp_acl_atcam_entry_add_ctcam_spill" 0
check_err $? "incorrect C-TCAM spill while inserting the second rule"
tp_check_hits "mlxsw:mlxsw_sp_acl_atcam_entry_add_ctcam_spill" 1
check_err $? "C-TCAM spill did not happen while inserting the second rule"

$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac -A 192.0.2.1 -B 192.0.2.2 \
-t ip -q
Expand Down Expand Up @@ -1087,6 +1087,53 @@ max_group_size_test()
log_test "max ACL group size test ($tcflags). max size $max_size"
}

collision_test()
{
# Filters cannot share an eRP if in the common unmasked part (i.e.,
# without the delta bits) they have the same values. If the driver does
# not prevent such configuration (by spilling into the C-TCAM), then
# multiple entries will be present in the device with the same key,
# leading to collisions and a reduced scale.
#
# Create such a scenario and make sure all the filters are successfully
# added.

RET=0

local ret

if [[ "$tcflags" != "skip_sw" ]]; then
return 0;
fi

# Add a single dst_ip/24 filter and multiple dst_ip/32 filters that all
# have the same values in the common unmasked part (dst_ip/24).

tc filter add dev $h2 ingress pref 1 proto ipv4 handle 101 \
flower $tcflags dst_ip 198.51.100.0/24 \
action drop

for i in {0..255}; do
tc filter add dev $h2 ingress pref 2 proto ipv4 \
handle $((102 + i)) \
flower $tcflags dst_ip 198.51.100.${i}/32 \
action drop
ret=$?
[[ $ret -ne 0 ]] && break
done

check_err $ret "failed to add all the filters"

for i in {255..0}; do
tc filter del dev $h2 ingress pref 2 proto ipv4 \
handle $((102 + i)) flower
done

tc filter del dev $h2 ingress pref 1 proto ipv4 handle 101 flower

log_test "collision test ($tcflags)"
}

setup_prepare()
{
h1=${NETIFS[p1]}
Expand Down

0 comments on commit 8d466c8

Please sign in to comment.