[dpdk-dev] [PATCH v5] power: add fifo per core for JSON interface

[Burakov, Anatoly]

On 09-Jul-19 4:07 PM, David Hunt wrote:
> From: Marcin Hajkowski <marcinx.hajkowski at intel.com>
> 
> This patch implements a separate FIFO for each cpu core to improve the
> previous functionality where anyone with access to the FIFO could affect
> any core on the system. By using appropriate permissions, fifo interfaces
> can be configured to only affect the particular cores.
> 
> Because each FIFO is per core, the following fields have been removed
> from the command JSON format: core_list, resource_id, name.
> 
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak at intel.com>
> Signed-off-by: Lukasz Gosiewski <lukaszx.gosiewski at intel.com>
> Signed-off-by: Marcin Hajkowski <marcinx.hajkowski at intel.com>
> Tested-by: David Hunt <david.hunt at intel.com>
> 
> ---
> v2:
> * updated handling vm_name (use proper buff size)
> * rebase to master changes
> 
> v3:
> * improvement to coding style
> 
> v4:
> * rebase to tip of master
> 
> v5:
> * merged docs into same patch as the code, as per mailing list policy
> * made changes out of review by Anatoly.
> ---

<snip>

> -	if (setup_host_channel_info(&chan_info, 0) < 0) {
> -		rte_free(chan_info);
> -		return 0;
> +		ret = fifo_path(socket_path, sizeof(socket_path), i);
> +		if (ret < 0)
> +			goto error;
> +
> +		ret = mkfifo(socket_path, 0660);
> +		RTE_LOG(DEBUG, CHANNEL_MANAGER, "TRY CREATE fifo '%s'\n",
> +			socket_path);
> +		if ((errno != EEXIST) && (ret < 0)) {
> +			RTE_LOG(ERR, CHANNEL_MANAGER, "Cannot create fifo '%s' error: "
> +					"%s\n", socket_path, strerror(errno));
> +			goto error;
> +		}
> +		chan_info = rte_malloc(NULL, sizeof(*chan_info), 0);
> +		if (chan_info == NULL) {
> +			RTE_LOG(ERR, CHANNEL_MANAGER, "Error allocating memory for "
> +					"channel '%s'\n", socket_path);
> +			goto error;
> +		}
> +		chan_infos[i] = chan_info;
> +		rte_strlcpy(chan_info->channel_path, socket_path,
> +				sizeof(chan_info->channel_path));
> +
> +		if (setup_host_channel_info(&chan_info, i) < 0) {
> +			rte_free(chan_info);
> +			chan_infos[i] = NULL;
> +			goto error;
> +		}
> +		num_channels_enabled++;
>   	}
> -	num_channels_enabled++;
>   
>   	return num_channels_enabled;
> +error:
> +	/* Clean up the channels opened before we hit an error. */
> +	for (i = 0; i < RTE_MAX_LCORE; i++) {

You're going up to RTE_MAX_LCORE here, but up to ci->core_count in the 
original loop. Is that intentional?

Other than that,

Acked-by: Anatoly Burakov <anatoly.burakov at intel.com>

-- 
Thanks,
Anatoly