This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
octeontx2-pf: cn20k: Add TC rules support

This patch adds dynamic MCAM entry allocation for TC rules on CN20K,
with X2/X4 key width support and entry shifting based on priorities.

[ ... ]

+int cn20k_tc_update_mcam_table_add_req(struct otx2_nic *nic,
+				       struct otx2_flow_config *flow_cfg,
+				       struct otx2_tc_flow *node)
+{
+	struct otx2_tc_flow *tmp;
+	u16 cntr_val = 0;
+	int list_idx, i;
+	int entry, prev;
+
+	list_idx = otx2_tc_add_to_flow_list(flow_cfg, node);
+	entry = node->entry;
+	if (!cn20k_tc_check_entry_shiftable(nic, flow_cfg, node,
+					    list_idx, true)) {
+		/* Due to mix of X2 and X4, entries cannot be shifted.
+		 * In this case free the entry allocated for this rule.
+		 */
+		if (cn20k_tc_free_mcam_entry(nic, entry))
+			netdev_err(nic->netdev,
+				   "Freeing entry %d to AF failed\n", entry);
+		return -EINVAL;

                ^^^^^^^^^^^^^^

When this returns -EINVAL, the caller otx2_tc_add_flow() will then jump
to free_leaf and call cn20k_tc_free_mcam_entry() again with the same
entry value (stored in new_node->entry). Is this a double-free of the
MCAM entry?

+	}

[ ... ]

[ ... ]

+	if (is_cn20k(nic->pdev)) {
+		rc = cn20k_tc_alloc_entry(nic, tc_flow_cmd, new_node, &dummy);
+		if (rc) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "MCAM rule allocation failed");
+			kfree_rcu(new_node, rcu);
+			return rc;
+		}
+	}
+
+	entry = otx2_tc_update_mcam_table(nic, flow_cfg, new_node, true);
+	if (entry < 0) {
+		NL_SET_ERR_MSG_MOD(extack, "Adding rule failed");
+		rc = entry;
+		goto free_leaf;
+	}
+
 	mutex_lock(&nic->mbox.lock);
 	req = otx2_mbox_alloc_msg_npc_install_flow(&nic->mbox);
 	if (!req) {
 		mutex_unlock(&nic->mbox.lock);
 		rc = -ENOMEM;
 		goto free_leaf;
 	}

[ ... ]

 free_leaf:
+	cn20k_tc_free_mcam_entry(nic, new_node->entry);

        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This function is called unconditionally in the error path, but on
non-CN20K devices new_node->entry was never initialized (it would be 0
from kzalloc). Would this send a mbox message to free MCAM entry 0 on
older silicon? Should this call be guarded with is_cn20k()?

Additionally, when cn20k_tc_update_mcam_table_add_req() fails due to
X2/X4 mix, it already frees the entry before returning -EINVAL. Reaching
this point would then attempt to free the same entry again.