From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bernard.iremonger@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id 1DF9F903
 for <dev@dpdk.org>; Mon,  9 Feb 2015 16:34:36 +0100 (CET)
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
 by orsmga103.jf.intel.com with ESMTP; 09 Feb 2015 07:29:39 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="452038155"
Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59])
 by FMSMGA003.fm.intel.com with ESMTP; 09 Feb 2015 07:20:03 -0800
Received: from irsmsx108.ger.corp.intel.com ([169.254.11.64]) by
 IRSMSX151.ger.corp.intel.com ([169.254.4.84]) with mapi id 14.03.0195.001;
 Mon, 9 Feb 2015 15:34:33 +0000
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [PATCH v7 07/14] ethdev: Add functions that will be used by
 port hotplug functions
Thread-Index: AQHQRELXT9i4q80PkUyVWEXWfroncpzocm0w
Date: Mon, 9 Feb 2015 15:34:32 +0000
Message-ID: <8CEF83825BEC744B83065625E567D7C2049DF12D@IRSMSX108.ger.corp.intel.com>
References: <1422763322-13742-4-git-send-email-mukawa@igel.co.jp>
 <1423470639-15744-1-git-send-email-mukawa@igel.co.jp>
 <1423470639-15744-8-git-send-email-mukawa@igel.co.jp>
In-Reply-To: <1423470639-15744-8-git-send-email-mukawa@igel.co.jp>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [163.33.239.180]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v7 07/14] ethdev: Add functions that will be
 used by port hotplug functions
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 09 Feb 2015 15:34:37 -0000



> -----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
>=20
> The patch adds following functions.
>=20
> - 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.
>=20
> 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.
>=20
> 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>
>=20
> 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(-)
>=20
> 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));  }
>=20
> -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);
>  }
>=20
> +void
> +rte_eth_dev_save(struct rte_eth_dev *devs) {
> +	if (devs =3D=3D NULL)
> +		return;
> +
Hi Tetsuya,

This function should probably have an input parameter for the size of the d=
evs 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 =3D=3D NULL) || (port_id =3D=3D NULL))
> +		return -EINVAL;
> +
> +	/* check which port was attached or detached */
> +	for (*port_id =3D 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) =3D=3D DEV_INVALID)
> +		return -EINVAL;
> +
> +	if (addr =3D=3D NULL) {
> +		PMD_DEBUG_TRACE("Null pointer is specified\n");
> +		return -EINVAL;
> +	}
> +
> +	*addr =3D 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 =3D=3D NULL) || (port_id =3D=3D NULL)) {
> +		PMD_DEBUG_TRACE("Null pointer is specified\n");
> +		return -EINVAL;
> +	}
> +
> +	for (*port_id =3D 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 =3D &rte_eth_devices[*port_id].pci_dev->addr;
> +		if (eal_compare_pci_addr(tmp, addr) =3D=3D 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) =3D=3D DEV_INVALID)
> +		return -EINVAL;
> +
> +	if (name =3D=3D 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 =3D 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 >=3D RTE_MAX_ETHPORTS) {
> +		PMD_DEBUG_TRACE("Invalid port_id=3D%d\n", port_id);
> +		return -EINVAL;
> +	}
> +
> +	if (rte_eth_devices[port_id].dev_type =3D=3D 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 =3D 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 91d=
9e86..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[];  exte=
rn uint8_t
> rte_eth_dev_count(void);
>=20
>  /**
> + * 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 identi=
fier.
> + * @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 po=
inter
> --
> 1.9.1