|SUCCESS| [v2] net/cpfl: add LEM block support in CPFL PMD

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162965

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-09

Review of patch: net/cpfl: add LEM block support in CPFL PMD

Commit: (not provided in patch file)
Author: Sushmita Hoskeri <sushmita.hoskeri at intel.com>
Subject: net/cpfl: add LEM block support in CPFL PMD

This patch adds APIs to enable support for LEM (Large Exact Match) block
in CPFL PMD.

> diff --git a/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c b/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c
> index 361827cb10..f42156e28f 100644
> --- a/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c
> +++ b/drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c

> @@ -180,6 +181,15 @@ cpfl_fxp_parse_pattern(const struct cpfl_flow_pr_action *pr_action,
>  		memcpy(rinfo->sem.key, pr_action->sem.cpfl_flow_pr_fv, rinfo->sem.key_byte_len);
>  		rinfo->sem.pin_to_cache = CPFL_PIN_TO_CACHE_DEF;
>  		rinfo->sem.fixed_fetch = CPFL_FIXED_FETCH_DEF;
> +	} else if (pr_action->type == CPFL_JS_PR_ACTION_TYPE_LEM) {
> +		struct cpfl_rule_info *rinfo = &rim->rules[i];
> +
> +		rinfo->type = CPFL_RULE_TYPE_LEM;
> +		rinfo->lem.prof_id = pr_action->lem.prof;
> +		rinfo->lem.key_byte_len = pr_action->lem.keysize;
> +		memcpy(rinfo->lem.key, pr_action->lem.cpfl_flow_pr_fv, rinfo->lem.key_byte_len);
> +		rinfo->lem.pin_to_cache = CPFL_PIN_TO_CACHE_CUST;
> +		rinfo->lem.fixed_fetch = CPFL_FIXED_FETCH_DEF;

Does this code overflow rinfo->lem.key[]? The key_byte_len comes from
pr_action->lem.keysize without bounds checking. CPFL_MAX_KEY_LEN should
be verified here, similar to how the SEM case should also check bounds.

> diff --git a/drivers/net/intel/cpfl/cpfl_flow_parser.c b/drivers/net/intel/cpfl/cpfl_flow_parser.c
> index e7deb619ee..dfaddc9ec5 100644
> --- a/drivers/net/intel/cpfl/cpfl_flow_parser.c
> +++ b/drivers/net/intel/cpfl/cpfl_flow_parser.c

> @@ -421,6 +421,56 @@ cpfl_flow_js_pattern_act_fv(json_t *ob_fvs, struct cpfl_flow_js_pr_action *js_ac
>  	return -EINVAL;
>  }
>  
> +static int
> +cpfl_flow_js_pattern_act_fv_lem(json_t *cjson_fv, struct cpfl_flow_js_pr_action *js_act)
> +{
> +	int len, i, ret;
> +
> +	len = json_array_size(cjson_fv);
> +	js_act->lem.fv = rte_malloc(NULL, sizeof(struct cpfl_flow_js_fv) * len, 0);
> +	if (!js_act->lem.fv) {

The coding style guide requires explicit comparisons for pointer checks.
This should be written as "if (js_act->lem.fv == NULL)".

> +		PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> +		return -ENOMEM;
> +	}
> +	js_act->lem.fv_size = len;
> +	for (i = 0; i < len; i++) {
> +		struct cpfl_flow_js_fv *js_fv;
> +		json_t *object, *cjson_value;
> +		const char *type;
> +
> +		object = json_array_get(cjson_fv, i);
> +		js_fv = &js_act->lem.fv[i];
> +		ret = cpfl_json_t_to_uint16(object, "offset", &js_fv->offset);
> +		if (ret < 0) {
> +			PMD_DRV_LOG(ERR, "Can not parse 'offset'.");
> +			return -EINVAL;

Does this leak js_act->lem.fv? When an error occurs in the loop, the
function returns without freeing the allocated memory.

> +		}
> +		type = cpfl_json_t_to_string(object, "type");
> +		if (!type) {

Same pointer comparison style issue. Should be "if (type == NULL)".

> +			PMD_DRV_LOG(ERR, "Can not parse 'type'.");
> +			return -EINVAL;

Does this also leak js_act->lem.fv on this error path?

> +		}
> +		cjson_value = json_object_get(object, "value");
> +		if (strcmp(type, "immediate") == 0) {
> +			js_fv->type = CPFL_FV_TYPE_IMMEDIATE;
> +			js_fv->immediate = json_integer_value(cjson_value);
> +		} else if (strcmp(type, "metadata") == 0) {
> +			js_fv->type = CPFL_FV_TYPE_METADATA;
> +			cpfl_flow_js_pattern_act_fv_metadata(cjson_value, js_fv);
> +		} else if (strcmp(type, "protocol") == 0) {
> +			js_fv->type = CPFL_FV_TYPE_PROTOCOL;
> +			cpfl_flow_js_pattern_act_fv_proto(cjson_value, js_fv);
> +		} else {
> +			PMD_DRV_LOG(ERR, "Not support this type: %s.", type);
> +			goto err;
> +		}
> +	}
> +	return 0;
> +err:
> +	rte_free(js_act->lem.fv);
> +	return -EINVAL;
> +}

[ ... ]

> @@ -984,6 +1053,8 @@ cpfl_parser_free_pr_action(struct cpfl_flow_js_pr_action *pr_act)
>  {
>  	if (pr_act->type == CPFL_JS_PR_ACTION_TYPE_SEM)
>  		rte_free(pr_act->sem.fv);
> +	else if (pr_act->type == CPFL_JS_PR_ACTION_TYPE_LEM)
> +		rte_free(pr_act->lem.fv);

Per coding style, braces are required for multi-line if-else statements.
Since there are multiple else-if clauses, all branches should use braces.

>  }

[ ... ]

> diff --git a/drivers/net/intel/cpfl/cpfl_fxp_rule.h b/drivers/net/intel/cpfl/cpfl_fxp_rule.h
> index 94eab6808c..544f441025 100644
> --- a/drivers/net/intel/cpfl/cpfl_fxp_rule.h
> +++ b/drivers/net/intel/cpfl/cpfl_fxp_rule.h

> @@ -28,9 +28,18 @@ struct cpfl_mod_rule_info {
>  	uint8_t mod_obj_size;
>  };
>  
> +struct cpfl_lem_rule_info {
> +	uint16_t prof_id;
> +	uint8_t key[CPFL_MAX_KEY_LEN];
> +	uint8_t key_byte_len;
> +	uint8_t pin_to_cache;
> +	uint8_t fixed_fetch;
> +};

Does CPFL_MAX_KEY_LEN accommodate MEV_LEM_RULE_KEY_SIZE (128)? The
cpfl_lem_rule_cfg_pkt structure defines a 128-byte key. Is CPFL_MAX_KEY_LEN
sufficiently large?

> diff --git a/drivers/net/intel/cpfl/cpfl_rules.h b/drivers/net/intel/cpfl/cpfl_rules.h
> index 10569b1fdc..2b65c7ecc8 100644
> --- a/drivers/net/intel/cpfl/cpfl_rules.h
> +++ b/drivers/net/intel/cpfl/cpfl_rules.h

> @@ -46,6 +46,12 @@ enum cpfl_ctlq_rule_cfg_opc {
>  	cpfl_ctlq_sem_query_rule_hash_addr		= 0x1307,
>  	cpfl_ctlq_sem_query_del_rule_hash_addr		= 0x1308,
>  
> +	cpfl_ctlq_lem_add_update_rule			= 0x1343,
> +	cpfl_ctlq_lem_del_rule					= 0x1345,
> +	cpfl_ctlq_lem_query_rule				= 0x1346,
> +	cpfl_ctlq_lem_query_rule_hash_addr		= 0x1347,
> +	cpfl_ctlq_lem_query_del_rule_hash_addr	= 0x1348,

The alignment here appears to use tabs inconsistently with the surrounding
code. The SEM entries use tabs for alignment, but these LEM entries appear
to mix tabs and spaces differently. Can this alignment be made consistent
with the existing enum members?