Re: [dpdk-dev] [PATCH v5 1/2] librte_ether: add protection against overwrite device data

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-09-30 16:00, Marcin Kerlin:
> Added protection against overwrite device data in array rte_eth_dev_data[]
> for the next secondary applications. Secondary process appends in the
> first free place rather than at the beginning. This behavior prevents
> overwriting devices data of primary process by secondary process.

It would be good to state what is a secondary process.
You are trying to extend its capabilities to be able to initialize devices.
So primary and secondary processes are almost equivalent?
What happens if we do not create any device in the primary?
Answer from code review: "Cannot allocate memzone for ethernet port data\n"

The secondary process is a hack to me.
But it is fine to have such hack for debug or monitoring purpose.
I would like to understand what are the other use cases?

By the way, the code managing the shared data of a device should
be at the EAL level in order to be used by other interfaces like crypto.

> @@ -631,6 +692,8 @@ int
>  rte_eth_dev_detach(uint8_t port_id, char *name)
>  {
>  	struct rte_pci_addr addr;
> +	struct rte_eth_dev_data *eth_dev_data = NULL;
> +	char device[RTE_ETH_NAME_MAX_LEN];
>  	int ret = -1;
>  
>  	if (name == NULL) {
> @@ -642,6 +705,15 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
>  	if (rte_eth_dev_is_detachable(port_id))
>  		goto err;
>  
> +	/* get device name by port id */
> +	if (rte_eth_dev_get_name_by_port(port_id, device))
> +		goto err;
> +
> +	/* look for an entry in the shared device data */
> +	eth_dev_data = rte_eth_dev_get_dev_data_by_name(device);
> +	if (eth_dev_data == NULL)
> +		goto err;

Why not getting eth_dev_data from rte_eth_devices[port_id].data ?

> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
>  /**
>   * @internal
> + * Release device data kept in shared memory for all processes.
> + *
> + * @param	port_id
> + *   The port identifier of the device to release device data.
> + * @return
> + *   - 0 on success, negative on error
> + */
> +int rte_eth_dev_release_dev_data(uint8_t port_id);

Why this function? It is not used.
You already have done the job in the detach function.