[dpdk-dev] [PATCH v8 1/2] ethdev: add pre-defined meter policy API
Singh, Jasvinder
jasvinder.singh at intel.com
Mon Apr 19 14:34:54 CEST 2021
<Snip>
> +/* MTR meter policy add */
> +static int
> +pmd_mtr_meter_policy_add(struct rte_eth_dev *dev,
> + uint32_t meter_policy_id,
> + struct rte_mtr_meter_policy_params *policy,
> + struct rte_mtr_error *error)
> +{
> + struct pmd_internals *p = dev->data->dev_private;
> + struct softnic_mtr_meter_policy_list *mpl = &p-
> >mtr.meter_policies;
> + struct softnic_mtr_meter_policy *mp;
> + const struct rte_flow_action *act;
> + const struct rte_flow_action_meter_color *recolor;
> + uint32_t i;
> + bool valid_act_found;
> +
> + if (policy == NULL)
> + return -rte_mtr_error_set(error,
> + EINVAL,
> + RTE_MTR_ERROR_TYPE_METER_POLICY,
> + NULL,
> + "Null meter policy invalid");
> +
> + /* Meter policy must not exist. */
> + mp = softnic_mtr_meter_policy_find(p, meter_policy_id);
> + if (mp != NULL)
> + return -rte_mtr_error_set(error,
> + EINVAL,
> + RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
> + NULL,
> + "Meter policy already exists");
> +
> + for (i = 0; i < RTE_COLORS; i++) {
> + if (policy->actions[i] == NULL)
> + return -rte_mtr_error_set(error,
> + EINVAL,
> + RTE_MTR_ERROR_TYPE_METER_POLICY,
> + NULL,
> + "Null action list");
> + for (act = policy->actions[i], valid_act_found = false;
> + act && act->type != RTE_FLOW_ACTION_TYPE_END;
> + act++) {
Hi Li,
No need to check "act" in for loop instruction as it is validated before the for loop. Also, would be good to skip all the steps inside this loop for the actions like RTE_FLOW_ACTION_TYPE_VOID. After for loop, will be good to check "valid_act_found" is true else return an error for that color action.
Rest seems good to me
With above fix for softnic-
Acked-by: Jasvinder Singh <jasvinder.singh at intel.com>
More information about the dev
mailing list