[dpdk-dev] [RFC PATCH 02/25] ethdev: Remove assumption that port will not be detached
Bruce Richardson
bruce.richardson at intel.com
Wed Oct 29 16:14:20 CET 2014
On Wed, Oct 29, 2014 at 05:49:13PM +0900, Tetsuya Mukawa wrote:
> To remove assumption, do like followings.
>
> - Add 'attached' member to rte_eth_dev structure.
> This member is used for indicating the port is attached, or not.
> - Delete nb_ports, and fix rte_eth_dev_count().
> The value was used for counting attached ports and also used for indicating
> maximum attached port number. But if some ports are detached, these 2 values
> may not be equal. So delete nb_ports, and fix rte_eth_dev_count() to count
> how many ports are attached actually.
> - Add rte_eth_dev_allocate_new_port().
> This function is used for allocating new port.
> - Add rte_eth_dev_validate_port().
> This function is used for check whether the port number is valid and the
> port is attached.
> - Replace port validation codes to rte_eth_dev_validate_port().
> nb_ports is deleted in this patch, so use rte_eth_dev_validate_port() to
> validate a port.
>
> Signed-off-by: Tetsuya Mukawa <mukawa at igel.co.jp>
> ---
> lib/librte_ether/rte_ethdev.c | 247 +++++++++++++++++++++++-------------------
> lib/librte_ether/rte_ethdev.h | 5 +
> 2 files changed, 143 insertions(+), 109 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index ff1c769..93d5a42 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -109,7 +109,6 @@
> static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
> struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
> static struct rte_eth_dev_data *rte_eth_dev_data = NULL;
> -static uint8_t nb_ports = 0;
>
> /* spinlock for eth device callbacks */
> static rte_spinlock_t rte_eth_dev_cb_lock = RTE_SPINLOCK_INITIALIZER;
> @@ -201,19 +200,33 @@ rte_eth_dev_allocated(const char *name)
> {
> unsigned i;
>
> - for (i = 0; i < nb_ports; i++) {
> - if (strcmp(rte_eth_devices[i].data->name, name) == 0)
> + for (i = 0; i < RTE_MAX_ETHPORTS; i++)
> + if (rte_eth_devices[i].attached && strcmp(
> + rte_eth_dev_data[i].name, name) == 0)
> return &rte_eth_devices[i];
> - }
> +
> return NULL;
> }
>
> +static uint8_t
> +rte_eth_dev_allocate_new_port(void)
> +{
> + unsigned i;
> +
> + for (i = 0; i < RTE_MAX_ETHPORTS; i++)
> + if (!rte_eth_devices[i].attached)
> + return i;
> + return RTE_MAX_ETHPORTS;
> +}
> +
> struct rte_eth_dev *
> rte_eth_dev_allocate(const char *name)
> {
> + uint8_t port_id;
> struct rte_eth_dev *eth_dev;
>
> - if (nb_ports == RTE_MAX_ETHPORTS) {
> + port_id = rte_eth_dev_allocate_new_port();
> + if (port_id >= RTE_MAX_ETHPORTS) {
> PMD_DEBUG_TRACE("Reached maximum number of Ethernet ports\n");
> return NULL;
> }
> @@ -226,10 +239,11 @@ rte_eth_dev_allocate(const char *name)
> return NULL;
> }
>
> - eth_dev = &rte_eth_devices[nb_ports];
> - eth_dev->data = &rte_eth_dev_data[nb_ports];
> + eth_dev = &rte_eth_devices[port_id];
> + eth_dev->data = &rte_eth_dev_data[port_id];
> snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
> - eth_dev->data->port_id = nb_ports++;
> + eth_dev->data->port_id = port_id;
> + eth_dev->attached = 1;
> return eth_dev;
> }
>
> @@ -283,7 +297,7 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
> (unsigned) pci_dev->id.device_id);
> if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> rte_free(eth_dev->data->dev_private);
> - nb_ports--;
> + eth_dev->attached = 0;
> return diag;
> }
>
> @@ -308,10 +322,19 @@ rte_eth_driver_register(struct eth_driver *eth_drv)
> rte_eal_pci_register(ð_drv->pci_drv);
> }
>
> +static int
> +rte_eth_dev_validate_port(uint8_t port_id)
> +{
> + if (port_id >= RTE_MAX_ETHPORTS)
> + return 1;
> +
> + return !rte_eth_devices[port_id].attached;
> +}
> +
> int
> rte_eth_dev_socket_id(uint8_t port_id)
> {
> - if (port_id >= nb_ports)
> + if (rte_eth_dev_validate_port(port_id))
> return -1;
> return rte_eth_devices[port_id].pci_dev->numa_node;
> }
> @@ -319,7 +342,13 @@ rte_eth_dev_socket_id(uint8_t port_id)
> uint8_t
> rte_eth_dev_count(void)
> {
> - return (nb_ports);
> + unsigned i;
> + uint8_t nb_ports = 0;
> +
> + for (i = 0; i < RTE_MAX_ETHPORTS; i++)
> + if (rte_eth_devices[i].attached)
> + nb_ports++;
> + return nb_ports;
> }
>
Given that apps may regularly use eth_dev_count, might it not be worthwhile to retain the nb_ports local variable to make this API call just be a variable read. The other APIs to enable/disable individual ports could easily just inc/dec this var as they do so.
/Bruce
More information about the dev
mailing list