Re: [PATCH v3 3/7] ethdev: add port ownership

[Thomas Monjalon <thomas@monjalon.net>]

18/01/2018 17:35, Matan Azrad:
> The ownership of a port is implicit in DPDK.
> Making it explicit is better from the next reasons:
> 1. It will define well who is in charge of the port usage synchronization.
> 2. A library could work on top of a port.
> 3. A port can work on top of another port.
> 
> Also in the fail-safe case, an issue has been met in testpmd.
> We need to check that the application is not trying to use a port which
> is already managed by fail-safe.
> 
> A port owner is built from owner id(number) and owner name(string) while
> the owner id must be unique to distinguish between two identical entity
> instances and the owner name can be any name.
> The name helps to logically recognize the owner by different DPDK
> entities and allows easy debug.
> Each DPDK entity can allocate an owner unique identifier and can use it
> and its preferred name to owns valid ethdev ports.
> Each DPDK entity can get any port owner status to decide if it can
> manage the port or not.
> 
> The mechanism is synchronized for both the primary process threads and
> the secondary processes threads to allow secondary process entity to be
> a port owner.
> 
> Add a sinchronized ownership mechanism to DPDK Ethernet devices to

s/sinchronized/synchronized/

> avoid multiple management of a device by different DPDK entities.
> 
> The current ethdev internal port management is not affected by this
> feature.
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>

I think it is a good compromise between application and
ethdev responsibilities.
The application is still responsible of thread safety per port,
and it is consistent with the checkless Rx/Tx design (for performance).

Except the wording (see below),
Acked-by: Thomas Monjalon <thomas@monjalon.net>

> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> +/* Shared memory between primary and secondary processes. */
> +static struct {
> +	uint64_t next_owner_id;
> +	rte_spinlock_t ownership_lock;
> +	struct rte_eth_dev_data data[RTE_MAX_ETHPORTS];
> +} *rte_eth_dev_share_data;

Should be rte_eth_dev_shared_data.

> -rte_eth_dev_data_alloc(void)
> +rte_eth_dev_share_data_alloc(void)

rte_eth_dev_shared_data_alloc