Re: [dpdk-dev] [PATCH v7 07/14] ethdev: Add functions that will be used by port hotplug functions

["Iremonger, Bernard" <bernard.iremonger@intel.com>]

> -----Original Message-----
> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
> Sent: Monday, February 9, 2015 8:31 AM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard; Qiu, Michael; Tetsuya Mukawa
> Subject: [PATCH v7 07/14] ethdev: Add functions that will be used by port hotplug functions
> 
> The patch adds following functions.
> 
> - rte_eth_dev_save()
>   The function is used for saving current rte_eth_dev structures.
> - rte_eth_dev_get_changed_port()
>   The function receives the rte_eth_dev structures, then compare
>   these with current values to know which port is actually
>   attached or detached.
> - rte_eth_dev_get_addr_by_port()
>   The function returns a pci address of an ethdev specified by port
>   identifier.
> - rte_eth_dev_get_port_by_addr()
>   The function returns a port identifier of an ethdev specified by
>   pci address.
> - rte_eth_dev_get_name_by_port()
>   The function returns a unique identifier name of an ethdev
>   specified by port identifier.
> - Add rte_eth_dev_check_detachable()
>   The function returns whether a PMD supports detach function.
> 
> Also, the patch changes scope of rte_eth_dev_allocated() to global.
> This function will be called by virtual PMDs to support port hotplug.
> So change scope of the function to global.
> 
> v7:
> - Add pt_driver checking to rte_eth_dev_check_detachable().
>   (Thanks to Qiu, Michael)
> v5:
> - Fix return value of below functions.
>   rte_eth_dev_get_changed_port().
>   rte_eth_dev_get_port_by_addr().
> v4:
> - Add parameter checking.
> v3:
> - Fix if-condition bug while comparing pci addresses.
> - Add error checking codes.
> Reported-by: Mark Enright <menrigh@brocade.com>
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_ether/rte_ethdev.c | 109 +++++++++++++++++++++++++++++++++++++++++-
>  lib/librte_ether/rte_ethdev.h |  80 +++++++++++++++++++++++++++++++
>  2 files changed, 188 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 7bed901..14a040a
> 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -206,7 +206,7 @@ rte_eth_dev_data_alloc(void)
>  				RTE_MAX_ETHPORTS * sizeof(*rte_eth_dev_data));  }
> 
> -static struct rte_eth_dev *
> +struct rte_eth_dev *
>  rte_eth_dev_allocated(const char *name)  {
>  	unsigned i;
> @@ -426,6 +426,113 @@ rte_eth_dev_count(void)
>  	return (nb_ports);
>  }
> 
> +void
> +rte_eth_dev_save(struct rte_eth_dev *devs) {
> +	if (devs == NULL)
> +		return;
> +
Hi Tetsuya,

This function should probably have an input parameter for the size of the devs buffer.
This input parameter should be used with the memcpy function to ensure that nothing is overwritten.

Regards,

Bernard.

> +	/* save current rte_eth_devices */
> +	memcpy(devs, rte_eth_devices,
> +			sizeof(struct rte_eth_dev) * RTE_MAX_ETHPORTS); }
> +
> +int
> +rte_eth_dev_get_changed_port(struct rte_eth_dev *devs, uint8_t
> +*port_id) {
> +	if ((devs == NULL) || (port_id == NULL))
> +		return -EINVAL;
> +
> +	/* check which port was attached or detached */
> +	for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++, devs++) {
> +		if (rte_eth_devices[*port_id].attached ^ devs->attached)
> +			return 0;
> +	}
> +	return -ENODEV;
> +}
> +
> +int
> +rte_eth_dev_get_addr_by_port(uint8_t port_id, struct rte_pci_addr
> +*addr) {
> +	if (rte_eth_dev_validate_port(port_id, TRACE) == DEV_INVALID)
> +		return -EINVAL;
> +
> +	if (addr == NULL) {
> +		PMD_DEBUG_TRACE("Null pointer is specified\n");
> +		return -EINVAL;
> +	}
> +
> +	*addr = rte_eth_devices[port_id].pci_dev->addr;
> +	return 0;
> +}
> +
> +int
> +rte_eth_dev_get_port_by_addr(struct rte_pci_addr *addr, uint8_t
> +*port_id) {
> +	struct rte_pci_addr *tmp;
> +
> +	if ((addr == NULL) || (port_id == NULL)) {
> +		PMD_DEBUG_TRACE("Null pointer is specified\n");
> +		return -EINVAL;
> +	}
> +
> +	for (*port_id = 0; *port_id < RTE_MAX_ETHPORTS; (*port_id)++) {
> +		if (!rte_eth_devices[*port_id].attached)
> +			continue;
> +		if (!rte_eth_devices[*port_id].pci_dev)
> +			continue;
> +		tmp = &rte_eth_devices[*port_id].pci_dev->addr;
> +		if (eal_compare_pci_addr(tmp, addr) == 0)
> +			return 0;
> +	}
> +	return -ENODEV;
> +}
> +
> +int
> +rte_eth_dev_get_name_by_port(uint8_t port_id, char *name) {
> +	char *tmp;
> +
> +	if (rte_eth_dev_validate_port(port_id, TRACE) == DEV_INVALID)
> +		return -EINVAL;
> +
> +	if (name == NULL) {
> +		PMD_DEBUG_TRACE("Null pointer is specified\n");
> +		return -EINVAL;
> +	}
> +
> +	/* shouldn't check 'rte_eth_devices[i].data',
> +	 * because it might be overwritten by VDEV PMD */
> +	tmp = rte_eth_dev_data[port_id].name;
> +	strncpy(name, tmp, strlen(tmp) + 1);
> +	return 0;
> +}
> +
> +int
> +rte_eth_dev_check_detachable(uint8_t port_id) {
> +	uint32_t drv_flags;
> +
> +	if (port_id >= RTE_MAX_ETHPORTS) {
> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> +		return -EINVAL;
> +	}
> +
> +	if (rte_eth_devices[port_id].dev_type == RTE_ETH_DEV_PHYSICAL) {
> +		switch (rte_eth_devices[port_id].pci_dev->pt_driver) {
> +		case RTE_PT_IGB_UIO:
> +		case RTE_PT_UIO_GENERIC:
> +			break;
> +		case RTE_PT_VFIO:
> +		default:
> +			return -ENOTSUP;
> +		}
> +	}
> +
> +	drv_flags = rte_eth_devices[port_id].driver->pci_drv.drv_flags;
> +	return !(drv_flags & RTE_PCI_DRV_DETACHABLE); }
> +
>  static int
>  rte_eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues)  { diff --git
> a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 91d9e86..9919968 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1616,6 +1616,86 @@ extern struct rte_eth_dev rte_eth_devices[];  extern uint8_t
> rte_eth_dev_count(void);
> 
>  /**
> + * Function for internal use by port hotplug functions.
> + * Copies current ethdev structures to the specified pointer.
> + *
> + * @param	devs	The pointer to the ethdev structures
> + */
> +extern void rte_eth_dev_save(struct rte_eth_dev *devs);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Compare the specified ethdev structures with currents. Then
> + * if there is a port which status is changed, fill the specified
> +pointer
> + * with the port id of that port.
> + * @param	devs	The pointer to the ethdev structures
> + * @param	port_id	The pointer to the port id
> + * @return
> + *   - 0 on success, negative on error
> + */
> +extern int rte_eth_dev_get_changed_port(
> +		struct rte_eth_dev *devs, uint8_t *port_id);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Returns a pci address of a ethdev specified by port identifier.
> + * @param	port_id
> + *   The port identifier of the Ethernet device
> + * @param	addr
> + *   The pointer to the pci address
> + * @return
> + *   - 0 on success, negative on error
> + */
> +extern int rte_eth_dev_get_addr_by_port(
> +		uint8_t port_id, struct rte_pci_addr *addr);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Returns a port identifier of a ethdev specified by pci address.
> + * @param	addr
> + *   The pointer to the pci address of the Ethernet device.
> + * @param	port_id
> + *   The pointer to the port identifier
> + * @return
> + *   - 0 on success, negative on error
> + */
> +extern int rte_eth_dev_get_port_by_addr(
> +		struct rte_pci_addr *addr, uint8_t *port_id);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Returns a unique identifier name of a ethdev specified by port identifier.
> + * @param	port_id
> + *   The port identifier.
> + * @param	name
> + *  The pointer to the Unique identifier name for each Ethernet device
> + * @return
> + *   - 0 on success, negative on error
> + */
> +extern int rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Check whether or not, a PMD that is handling the ethdev specified by
> +port
> + * identifier can support detach function.
> + * @param	port_id
> + *   The port identifier
> + * @return
> + *   - 0 on supporting detach function, negative on not supporting
> + */
> +extern int rte_eth_dev_check_detachable(uint8_t port_id);
> +
> +/**
> + * Function for internal use by port hotplug functions.
> + * Returns a ethdev slot specified by the unique identifier name.
> + * @param	name
> + *  The pointer to the Unique identifier name for each Ethernet device
> + * @return
> + *   - The pointer to the ethdev slot, on success. NULL on error
> + */
> +extern struct rte_eth_dev *rte_eth_dev_allocated(const char *name);
> +
> +/**
>   * Function for internal use by dummy drivers primarily, e.g. ring-based
>   * driver.
>   * Allocates a new ethdev slot for an ethernet device and returns the pointer
> --
> 1.9.1