Patch "mlxsw: spectrum_acl_erp: Fix object nesting warning" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    mlxsw: spectrum_acl_erp: Fix object nesting warning

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mlxsw-spectrum_acl_erp-fix-object-nesting-warning.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 57e1cae563b86bda3c98204e10b003cebdce656b
Author: Ido Schimmel <idosch@xxxxxxxxxx>
Date:   Thu Jun 6 16:49:42 2024 +0200

    mlxsw: spectrum_acl_erp: Fix object nesting warning
    
    [ Upstream commit 97d833ceb27dc19f8777d63f90be4a27b5daeedf ]
    
    ACLs in Spectrum-2 and newer ASICs can reside in the algorithmic TCAM
    (A-TCAM) or in the ordinary circuit TCAM (C-TCAM). The former can
    contain more ACLs (i.e., tc filters), but the number of masks in each
    region (i.e., tc chain) is limited.
    
    In order to mitigate the effects of the above limitation, the device
    allows filters to share a single mask if their masks only differ in up
    to 8 consecutive bits. For example, dst_ip/25 can be represented using
    dst_ip/24 with a delta of 1 bit. The C-TCAM does not have a limit on the
    number of masks being used (and therefore does not support mask
    aggregation), but can contain a limited number of filters.
    
    The driver uses the "objagg" library to perform the mask aggregation by
    passing it objects that consist of the filter's mask and whether the
    filter is to be inserted into the A-TCAM or the C-TCAM since filters in
    different TCAMs cannot share a mask.
    
    The set of created objects is dependent on the insertion order of the
    filters and is not necessarily optimal. Therefore, the driver will
    periodically ask the library to compute a more optimal set ("hints") by
    looking at all the existing objects.
    
    When the library asks the driver whether two objects can be aggregated
    the driver only compares the provided masks and ignores the A-TCAM /
    C-TCAM indication. This is the right thing to do since the goal is to
    move as many filters as possible to the A-TCAM. The driver also forbids
    two identical masks from being aggregated since this can only happen if
    one was intentionally put in the C-TCAM to avoid a conflict in the
    A-TCAM.
    
    The above can result in the following set of hints:
    
    H1: {mask X, A-TCAM} -> H2: {mask Y, A-TCAM} // X is Y + delta
    H3: {mask Y, C-TCAM} -> H4: {mask Z, A-TCAM} // Y is Z + delta
    
    After getting the hints from the library the driver will start migrating
    filters from one region to another while consulting the computed hints
    and instructing the device to perform a lookup in both regions during
    the transition.
    
    Assuming a filter with mask X is being migrated into the A-TCAM in the
    new region, the hints lookup will return H1. Since H2 is the parent of
    H1, the library will try to find the object associated with it and
    create it if necessary in which case another hints lookup (recursive)
    will be performed. This hints lookup for {mask Y, A-TCAM} will either
    return H2 or H3 since the driver passes the library an object comparison
    function that ignores the A-TCAM / C-TCAM indication.
    
    This can eventually lead to nested objects which are not supported by
    the library [1].
    
    Fix by removing the object comparison function from both the driver and
    the library as the driver was the only user. That way the lookup will
    only return exact matches.
    
    I do not have a reliable reproducer that can reproduce the issue in a
    timely manner, but before the fix the issue would reproduce in several
    minutes and with the fix it does not reproduce in over an hour.
    
    Note that the current usefulness of the hints is limited because they
    include the C-TCAM indication and represent aggregation that cannot
    actually happen. This will be addressed in net-next.
    
    [1]
    WARNING: CPU: 0 PID: 153 at lib/objagg.c:170 objagg_obj_parent_assign+0xb5/0xd0
    Modules linked in:
    CPU: 0 PID: 153 Comm: kworker/0:18 Not tainted 6.9.0-rc6-custom-g70fbc2c1c38b #42
    Hardware name: Mellanox Technologies Ltd. MSN3700C/VMOD0008, BIOS 5.11 10/10/2018
    Workqueue: mlxsw_core mlxsw_sp_acl_tcam_vregion_rehash_work
    RIP: 0010:objagg_obj_parent_assign+0xb5/0xd0
    [...]
    Call Trace:
     <TASK>
     __objagg_obj_get+0x2bb/0x580
     objagg_obj_get+0xe/0x80
     mlxsw_sp_acl_erp_mask_get+0xb5/0xf0
     mlxsw_sp_acl_atcam_entry_add+0xe8/0x3c0
     mlxsw_sp_acl_tcam_entry_create+0x5e/0xa0
     mlxsw_sp_acl_tcam_vchunk_migrate_one+0x16b/0x270
     mlxsw_sp_acl_tcam_vregion_rehash_work+0xbe/0x510
     process_one_work+0x151/0x370
    
    Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
    Signed-off-by: Ido Schimmel <idosch@xxxxxxxxxx>
    Reviewed-by: Amit Cohen <amcohen@xxxxxxxxxx>
    Tested-by: Alexander Zubkov <green@xxxxxxxxxx>
    Signed-off-by: Petr Machata <petrm@xxxxxxxxxx>
    Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
index d231f4d2888be..9eee229303cce 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_erp.c
@@ -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)
 {
@@ -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,
diff --git a/include/linux/objagg.h b/include/linux/objagg.h
index 78021777df462..6df5b887dc547 100644
--- a/include/linux/objagg.h
+++ b/include/linux/objagg.h
@@ -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);
diff --git a/lib/objagg.c b/lib/objagg.c
index 90f3aa68c30a0..1608895b009c8 100644
--- a/lib/objagg.c
+++ b/lib/objagg.c
@@ -906,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
@@ -958,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)




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux