RE: [PATCH 5/6] event/cnxk: support to set runtime queue attributes

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----Original Message-----
> From: Shijith Thotton <sthotton@marvell.com>
> Sent: Tuesday, March 29, 2022 2:11 PM
> To: dev@dpdk.org; jerinj@marvell.com
> Cc: Shijith Thotton <sthotton@marvell.com>; pbhagavatula@marvell.com
> Subject: [PATCH 5/6] event/cnxk: support to set runtime queue attributes

<snip>

> +int
> +cnxk_sso_queue_attribute_get(struct rte_eventdev *event_dev, uint8_t
> queue_id,
> +			     uint32_t attr_id, uint32_t *attr_value)
> +{
> +	struct cnxk_sso_evdev *dev = cnxk_sso_pmd_priv(event_dev);
> +
> +	*attr_value = attr_id == RTE_EVENT_QUEUE_ATTR_WEIGHT ?
> +			      dev->mlt_prio[queue_id].weight :
> +			      dev->mlt_prio[queue_id].affinity;

This is future-bug prone, as adding a new Eventdev attr will return .affinity silently,
instead of the attr that is being requested.

Prefer a switch(attr_id), and explicitly handle each attr_id, with a default case
to return -1, showing the PMD refusing to handle the attr requested to the caller.

On reviewing the below, the set() below does this perfectly... except the return?

> +
> +	return 0;
> +}
> +
> +int
> +cnxk_sso_queue_attribute_set(struct rte_eventdev *event_dev, uint8_t
> queue_id,
> +			     uint32_t attr_id, uint32_t attr_value)
> +{
> +	struct cnxk_sso_evdev *dev = cnxk_sso_pmd_priv(event_dev);
> +	uint8_t priority, weight, affinity;
> +	struct rte_event_queue_conf *conf;
> +
> +	conf = &event_dev->data->queues_cfg[queue_id];
> +
> +	switch (attr_id) {
> +	case RTE_EVENT_QUEUE_ATTR_PRIORITY:
> +		conf->priority = attr_value;
> +		break;
> +	case RTE_EVENT_QUEUE_ATTR_WEIGHT:
> +		dev->mlt_prio[queue_id].weight = attr_value;
> +		break;
> +	case RTE_EVENT_QUEUE_ATTR_AFFINITY:
> +		dev->mlt_prio[queue_id].affinity = attr_value;
> +		break;
> +	default:
> +		plt_sso_dbg("Ignored setting attribute id %u", attr_id);
> +		return 0;
> +	}

Why return 0 here? This is a failure, the PMD did *not* set the attribute ID.
Make the user aware of that fact, return -1; or -EINVAL or something.

Document the explicit return values at Eventdev header level, so all PMDs can
align on the return values, providing consistency to the application.

<snip>