DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC PATCH 2/6] eal: introduce bus-device-driver structure
From: Jan Blunck @ 2016-11-17 11:19 UTC (permalink / raw)
  To: Shreyansh Jain; +Cc: David Marchand, dev
In-Reply-To: <1479360605-20558-3-git-send-email-shreyansh.jain@nxp.com>

On Thu, Nov 17, 2016 at 6:30 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> A device is connected to a bus and services by a driver associated with
> the bus. It is responsibility of the bus to identify the devices (scan)
> and then assign each device to a matching driver.
>
> A PMD would allocate a rte_xxx_driver and rte_xxx_device.
> rte_xxx_driver has rte_driver and rte_bus embedded. Similarly, rte_xxx_device
> has rte_device and rte_bus embedded.

I don't think so: the rte_xxx_device embeds the generic rte_device and
references a the rte_bus
that it is attached to.

> When a ethernet or crypto device (rte_eth_dev, rte_cryptodev) is allocated,
> it contains a reference of rte_device and rte_driver.
> Each ethernet device implementation would use container_of for finding the
> enclosing structure of rte_xxx_*.
>
>                             +-------------------+
>  +--------------+           |rte_pci_device     |
>  |rte_eth_dev   |           |+-----------------+|
>  |+------------+|   .-------->rte_device       ||
>  ||rte_device*-----'        |+-----------------+|
>  |+------------+|           ||rte_bus          ||
>  |              |           |+-----------------+|
>  /              /           +-------------------+
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  lib/librte_eal/common/include/rte_bus.h | 243 ++++++++++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_dev.h |  36 ++---
>  2 files changed, 261 insertions(+), 18 deletions(-)
>  create mode 100644 lib/librte_eal/common/include/rte_bus.h
>
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> new file mode 100644
> index 0000000..dc3aeb8
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -0,0 +1,243 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 NXP
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of NXP nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTE_BUS_H_
> +#define _RTE_BUS_H_
> +
> +/**
> + * @file
> + *
> + * RTE PMD Bus Abstraction interfaces
> + *
> + * This file exposes APIs and Interfaces for Bus Abstraction over the devices
> + * drivers in EAL.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdio.h>
> +#include <sys/queue.h>
> +
> +#include <rte_log.h>
> +#include <rte_dev.h>
> +
> +
> +/** Double linked list of buses */
> +TAILQ_HEAD(rte_bus_list, rte_bus);
> +
> +/**
> + * Bus specific scan for devices attached on the bus.
> + * For each bus object, the scan would be reponsible for finding devices and
> + * adding them to its private device list.
> + *
> + * Successful detection of a device results in rte_device object which is
> + * embedded within the respective device type (rte_pci_device, for example).
> + * Thereafter, PCI specific bus would need to perform
> + * container_of(rte_pci_device) to obtain PCI device object.
> + *
> + * Scan failure of a bus is not treated as exit criteria for application. Scan
> + * for all other buses would still continue.
> + *
> + * @param void
> + * @return
> + *     0 for successful scan
> + *     !0 (<0) for unsuccessful scan with error value
> + */
> +typedef int (* bus_scan_t)(void);
> +
> +/**
> + * Bus specific match for devices and drivers which can service them.
> + * For each scanned device, during probe the match would link the devices with
> + * drivers which can service the device.
> + *
> + * It is the work of each bus handler to obtain the specific device object
> + * using container_of (or typecasting, as a less preferred way).
> + *
> + * @param drv
> + *     Driver object attached to the bus
> + * @param dev
> + *     Device object which is being probed.
> + * @return
> + *     0 for successful match
> + *     !0 for unsuccessful match
> + */
> +typedef int (* bus_match_t)(struct rte_driver *drv, struct rte_device *dev);

Do you think this should do match & probe?

I believe it is better to separate this into two functions to match()
and probe(). The
result of matching tells if the driver would want to claim the device
in general. But
probe()ing the device should only happen if the device isn't claimed
by a driver yet and
is not blacklisted.

> +
> +/**
> + * Dump the devices on the bus.
> + * Each bus type can define its own definition of information to dump.
> + *
> + * @param bus
> + *     Handle for bus, device from which are to be dumped.
> + * @param f
> + *     Handle to output device or file.
> + * @return void
> + */
> +typedef void (* bus_dump_t)(struct rte_bus *bus, FILE *f);
> +
> +/**
> + * Search for a specific device in device list of the bus
> + * This would rely on the bus specific addressing. Each implementation would
> + * extract its specific device type and perform address compare.
> + *
> + * @param dev
> + *     device handle to search for.
> + * @return
> + *     rte_device handle for matched device, or NULL
> + */
> +typedef struct rte_device * (* bus_device_get_t)(struct rte_device *dev);

>From my experience it is better to delegate this to a helper:

int bus_for_each_dev(struct rte_bus *bus, struct rte_device *start,
int (*match)(struct rte_device *dev, void *data), void *data);

Based on that you can easily implement all kinds of functions like
bus_find_dev(), bus_find_dev_by_name(), bus_dump(), ...

> +
> +struct rte_bus {
> +       TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
> +       struct rte_driver_list driver_list; /**< List of all drivers of bus */

TAILQ_HEAD?

> +       struct rte_device_list device_list; /**< List of all devices on bus */

TAILQ_HEAD?

> +       const char *name;            /**< Name of the bus */
> +       /* Mandatory hooks */
> +       bus_scan_t *scan;            /**< Hook for scanning for devices */
> +       bus_match_t *match;          /**< Hook for matching device & driver */
> +       /* Optional hooks */
> +       bus_dump_t *dump_dev;        /**< Hook for dumping devices on bus */
> +       bus_device_get_t *find_dev;  /**< Search for a device on bus */
> +};
> +
> +/** @internal
> + * Add a device to a bus.
> + *
> + * @param bus
> + *     Bus on which device is to be added
> + * @param dev
> + *     Device handle
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev);

Why do we need this? From my understanding the rte_bus->scan() is
adding the devices to the rte_bus->device_list.

> +/** @internal
> + * Remove a device from its bus.
> + *
> + * @param dev
> + *     Device handle to remove
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_remove_device(struct rte_device *dev);
> +
> +/** @internal
> + * Associate a driver with a bus.
> + *
> + * @param bus
> + *     Bus on which driver is to be added
> + * @param dev
> + *     Driver handle
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv);
> +

What happens if a driver is added at runtime to a bus? Does that immediately
trigger a match & probe of unclaimed devices?

> +/** @internal
> + * Disassociate a driver from its bus.
> + *
> + * @param dev
> + *     Driver handle to remove
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_remove_driver(struct rte_driver *drv);
> +
> +/**
> + * Register a Bus handler.
> + *
> + * @param driver
> + *   A pointer to a rte_bus structure describing the bus
> + *   to be registered.
> + */
> +void rte_eal_bus_register(struct rte_bus *bus);
> +
> +/**
> + * Unregister a Bus handler.
> + *
> + * @param driver
> + *   A pointer to a rte_bus structure describing the bus
> + *   to be unregistered.
> + */
> +void rte_eal_bus_unregister(struct rte_bus *bus);
> +
> +/**
> + * Obtain handle for bus given its name.
> + *
> + * @param bus_name
> + *     Name of the bus handle to search
> + * @return
> + *     Pointer to Bus object if name matches any registered bus object
> + *     NULL, if no matching bus found
> + */
> +struct rte_bus * rte_eal_get_bus(const char *bus_name);
> +
> +/**
> + * Register a device driver.
> + *
> + * @param driver
> + *   A pointer to a rte_dev structure describing the driver
> + *   to be registered.
> + */
> +void rte_eal_driver_register(struct rte_driver *driver);
> +
> +/**
> + * Unregister a device driver.
> + *
> + * @param driver
> + *   A pointer to a rte_dev structure describing the driver
> + *   to be unregistered.
> + */
> +void rte_eal_driver_unregister(struct rte_driver *driver);
> +
> +/** Helper for Bus registration */
> +#define RTE_PMD_REGISTER_BUS(nm, bus) \
> +RTE_INIT(businitfn_ ##nm); \
> +static void businitfn_ ##nm(void) \
> +{\
> +       (bus).name = RTE_STR(nm);\
> +       rte_eal_bus_register(&bus); \
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_BUS_H */
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index 8840380..b08bab5 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -116,12 +116,14 @@ TAILQ_HEAD(rte_device_list, rte_device);
>
>  /* Forward declaration */
>  struct rte_driver;
> +struct rte_bus;
>
>  /**
>   * A structure describing a generic device.
>   */
>  struct rte_device {
>         TAILQ_ENTRY(rte_device) next; /**< Next device */
> +       struct rte_bus *bus;          /**< Bus on which device is placed */
>         struct rte_driver *driver;    /**< Associated driver */
>         int numa_node;                /**< NUMA node connection */
>         struct rte_devargs *devargs;  /**< Device user arguments */
> @@ -144,31 +146,29 @@ void rte_eal_device_insert(struct rte_device *dev);
>  void rte_eal_device_remove(struct rte_device *dev);
>
>  /**
> - * A structure describing a device driver.
> + * @internal
> + * TODO
>   */
> -struct rte_driver {
> -       TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> -       const char *name;                   /**< Driver name. */
> -       const char *alias;              /**< Driver alias. */
> -};
> +typedef int (*driver_init_t)(struct rte_device *eth_dev);
>
>  /**
> - * Register a device driver.
> - *
> - * @param driver
> - *   A pointer to a rte_dev structure describing the driver
> - *   to be registered.
> + * @internal
> + * TODO
>   */
> -void rte_eal_driver_register(struct rte_driver *driver);
> +typedef int (*driver_uninit_t)(struct rte_device *eth_dev);
>
>  /**
> - * Unregister a device driver.
> - *
> - * @param driver
> - *   A pointer to a rte_dev structure describing the driver
> - *   to be unregistered.
> + * A structure describing a device driver.
>   */
> -void rte_eal_driver_unregister(struct rte_driver *driver);
> +struct rte_driver {
> +       TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> +       struct rte_bus *bus;           /**< Bus which drivers services */

I think this should be TAILQ_ENTRY instead.

> +       const char *name;              /**< Driver name. */
> +       const char *alias;             /**< Driver alias. */
> +       driver_init_t *init;           /**< Driver initialization */
> +       driver_uninit_t *uninit;       /**< Driver uninitialization */

Shouldn't this be probe() and remove()?

> +       unsigned int dev_private_size; /**< Size of device private data ??*/

I don't think that dev_private_size is really required at this level.
First of all this is related to the rte_eth_dev structure and
therefore it really depends on the driver_init_t (aka probe()) if it
is actually allocating an rte_eth_dev or not. Anyway that is up to the
drivers probe() function.

> +};
>
>  /**
>   * Initalize all the registered drivers in this process
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH 3/3] net/mlx5: do not invalidate title CQE
From: Adrien Mazarguil @ 2016-11-17 10:38 UTC (permalink / raw)
  To: Nelio Laranjeiro; +Cc: dev
In-Reply-To: <47daa16c78c14e3e11904308b39e086d495ca1d0.1479376117.git.nelio.laranjeiro@6wind.com>

On Thu, Nov 17, 2016 at 10:49:56AM +0100, Nelio Laranjeiro wrote:
> We can leave the title completion queue entry untouched since its contents
> are not modified.
> 
> Reported-by: Liming Sun <lsun@mellanox.com>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  drivers/net/mlx5/mlx5_rxtx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index 04860bb..ffd09ac 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1162,7 +1162,7 @@ mlx5_rx_poll_len(struct rxq *rxq, volatile struct mlx5_cqe *cqe,
>  			zip->na += 8;
>  		}
>  		if (unlikely(rxq->zip.ai == rxq->zip.cqe_cnt)) {
> -			uint16_t idx = rxq->cq_ci;
> +			uint16_t idx = rxq->cq_ci + 1;
>  			uint16_t end = zip->cq_ci;
>  
>  			while (idx != end) {
> -- 
> 2.1.4

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH 2/3] net/mlx5: fix wrong htons
From: Adrien Mazarguil @ 2016-11-17 10:38 UTC (permalink / raw)
  To: Nelio Laranjeiro; +Cc: dev, stable
In-Reply-To: <92fae1c8490a9dfa782bbd62bac21076832d4cee.1479376117.git.nelio.laranjeiro@6wind.com>

On Thu, Nov 17, 2016 at 10:49:55AM +0100, Nelio Laranjeiro wrote:
> Completion queue entry data uses network endian, to access them we should use
> ntoh*().
> 
> Fixes: c305090bbaf8 ("net/mlx5: replace countdown with threshold for Tx completions")
> 
> CC: stable@dpdk.org
> Reported-by: Liming Sun <lsun@mellanox.com>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  drivers/net/mlx5/mlx5_rxtx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index 9bd4d80..04860bb 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -201,7 +201,7 @@ txq_complete(struct txq *txq)
>  	} while (1);
>  	if (unlikely(cqe == NULL))
>  		return;
> -	wqe = &(*txq->wqes)[htons(cqe->wqe_counter) &
> +	wqe = &(*txq->wqes)[ntohs(cqe->wqe_counter) &
>  			    ((1 << txq->wqe_n) - 1)].hdr;
>  	elts_tail = wqe->ctrl[3];
>  	assert(elts_tail < (1 << txq->wqe_n));
> -- 
> 2.1.4

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH 1/3] net/mlx5: fix leak when starvation occurs
From: Adrien Mazarguil @ 2016-11-17 10:37 UTC (permalink / raw)
  To: Nelio Laranjeiro; +Cc: dev, stable
In-Reply-To: <e636af2f3f462bc75bc461eb8f8a2f429270e458.1479376117.git.nelio.laranjeiro@6wind.com>

On Thu, Nov 17, 2016 at 10:49:54AM +0100, Nelio Laranjeiro wrote:
> The list of segments to free was wrongly manipulated ending by only freeing
> the first segment instead of freeing all of them.  The last one still
> belongs to the NIC and thus should not be freed.
> 
> Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx")
> 
> CC: stable@dpdk.org
> Reported-by: Liming Sun <lsun@mellanox.com>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  drivers/net/mlx5/mlx5_rxtx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index beff580..9bd4d80 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1312,10 +1312,10 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  			}
>  			while (pkt != seg) {
>  				assert(pkt != (*rxq->elts)[idx]);
> -				seg = NEXT(pkt);
> +				rep = NEXT(pkt);
>  				rte_mbuf_refcnt_set(pkt, 0);
>  				__rte_mbuf_raw_free(pkt);
> -				pkt = seg;
> +				pkt = rep;
>  			}
>  			break;
>  		}
> -- 
> 2.1.4

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [RFC PATCH 0/7] RFC: EventDev Software PMD
From: Bruce Richardson @ 2016-11-17 10:05 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: Harry van Haaren, dev
In-Reply-To: <20161116201924.GA32292@svelivela-lt.caveonetworks.com>

On Thu, Nov 17, 2016 at 01:49:25AM +0530, Jerin Jacob wrote:
> On Wed, Nov 16, 2016 at 06:00:00PM +0000, Harry van Haaren wrote:
> > This series of RFC patches implements the libeventdev API and a software
> > eventdev PMD.
> > 
> > The implementation here is intended to enable the community to use the
> > eventdev API, specifically to test if the API serves the purpose that it is
> > designed to. It should be noted this is an RFC implementation, and hence
> > there should be no performance expectations.
> > 
> > An RFC for the eventdev was sent in August[1] by Jerin Jacob of Cavium,
> > which introduced the core concepts of the eventdev to the community. Since
> > then there has been extensive discussion[2] on the mailing list, which had
> > led to various modifications to the initial proposed API.
> > 
> > The API as presented in the first patch contains a number of changes that
> > have not yet been discussed. These changes were noticed during the
> > implementation of the software eventdev PMD, and were added to the API to
> > enable completion of the PMD. These modifications include a statistics API
> > and a dump API. For more details, please refer to the commit message of the
> > patch itself.
> > 
> > The functionality provided by each of the patches is as follows:
> >   1: Add eventdev API and library infrastructure
> >   2: Enable compilation of library
> >   3: Add software eventdev PMD
> >   4: Enable compilation of PMD
> >   5: Add test code
> >   6: Enable test code compilation
> >   7: Sample application demonstrating basic usage
> > 
> > This breakdown of the patchset hopefully enables the community to experiment
> > with the eventdev API, and allows us all to gain first-hand experience in
> > using the eventdev API.  Note also that this patchset has not passed
> > checkpatch testing just yet - will fix for v2 :)
> > 
> > As next steps I see value in discussing the proposed changes included in
> > this version of the header file, while welcoming feedback from the community
> > on the API in general too.
> 
> Thanks. Harry.
> 
> Even I was writing the similar stuff.I took a bit different approach on
> the common code side, where I was trying to have fat common code(
> lib/librte_eventdev/rte_eventdev.c) with start/stop support for the
> slow-path code. I will post the implementation in few days and then we
> can work on a converged solution.

Looking forward to seeing this. Hopefully some of our code can be reused
on your side too, maybe the registration and args parsing bits, perhaps.

> 
> Following sections of code does not have any overlap at all.
> test/eventdev: unit and functional tests
> event/sw: software eventdev implementation
> examples/eventdev_pipeline: adding example
> 
> Some questions and initial feedback
> 1) I thought RTE_EVENT_OP_DROP and rte_event_release() are same ? No ?

They should be largely equivalent, just that the DROP op can be done as
part of a burst. If they are not, it could be a bug in our
implementation, as it's still an early draft using this eventdev API.

> 2) device stats API can be based on capability, HW implementations may not
> support all the stats

Yes, this is something we were thinking about. It would be nice if we
could at least come up with a common set of stats - maybe even ones
tracked at an eventdev API level, e.g. nb enqueues/dequeues. As well as
that, we think the idea of an xstats API, like in ethdev, might work
well. For our software implementation, having visibility into the
scheduler behaviour can be important, so we'd like a way to report out
things like internal queue depths etc.

> 3) From the HW implementation perspective, eventdev_pipeline application
> needs to have a lot of changes.I will post the comments in coming days
> and we can work together on the converged solution.

Yes, please do. I expect we'll need a good set of guidelines in order to
allow people to write truly portable apps using this API.

Thanks for the feedback.

/Bruce

> 
> Jerin
> 
> 

^ permalink raw reply

* [PATCH 3/3] net/mlx5: do not invalidate title CQE
From: Nelio Laranjeiro @ 2016-11-17  9:49 UTC (permalink / raw)
  To: dev; +Cc: Adrien Mazarguil
In-Reply-To: <e636af2f3f462bc75bc461eb8f8a2f429270e458.1479376117.git.nelio.laranjeiro@6wind.com>

We can leave the title completion queue entry untouched since its contents
are not modified.

Reported-by: Liming Sun <lsun@mellanox.com>
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 04860bb..ffd09ac 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1162,7 +1162,7 @@ mlx5_rx_poll_len(struct rxq *rxq, volatile struct mlx5_cqe *cqe,
 			zip->na += 8;
 		}
 		if (unlikely(rxq->zip.ai == rxq->zip.cqe_cnt)) {
-			uint16_t idx = rxq->cq_ci;
+			uint16_t idx = rxq->cq_ci + 1;
 			uint16_t end = zip->cq_ci;
 
 			while (idx != end) {
-- 
2.1.4

^ permalink raw reply related

* [PATCH 2/3] net/mlx5: fix wrong htons
From: Nelio Laranjeiro @ 2016-11-17  9:49 UTC (permalink / raw)
  To: dev; +Cc: Adrien Mazarguil, stable
In-Reply-To: <e636af2f3f462bc75bc461eb8f8a2f429270e458.1479376117.git.nelio.laranjeiro@6wind.com>

Completion queue entry data uses network endian, to access them we should use
ntoh*().

Fixes: c305090bbaf8 ("net/mlx5: replace countdown with threshold for Tx completions")

CC: stable@dpdk.org
Reported-by: Liming Sun <lsun@mellanox.com>
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 9bd4d80..04860bb 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -201,7 +201,7 @@ txq_complete(struct txq *txq)
 	} while (1);
 	if (unlikely(cqe == NULL))
 		return;
-	wqe = &(*txq->wqes)[htons(cqe->wqe_counter) &
+	wqe = &(*txq->wqes)[ntohs(cqe->wqe_counter) &
 			    ((1 << txq->wqe_n) - 1)].hdr;
 	elts_tail = wqe->ctrl[3];
 	assert(elts_tail < (1 << txq->wqe_n));
-- 
2.1.4

^ permalink raw reply related

* [PATCH 1/3] net/mlx5: fix leak when starvation occurs
From: Nelio Laranjeiro @ 2016-11-17  9:49 UTC (permalink / raw)
  To: dev; +Cc: Adrien Mazarguil, stable

The list of segments to free was wrongly manipulated ending by only freeing
the first segment instead of freeing all of them.  The last one still
belongs to the NIC and thus should not be freed.

Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx")

CC: stable@dpdk.org
Reported-by: Liming Sun <lsun@mellanox.com>
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 drivers/net/mlx5/mlx5_rxtx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index beff580..9bd4d80 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1312,10 +1312,10 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			}
 			while (pkt != seg) {
 				assert(pkt != (*rxq->elts)[idx]);
-				seg = NEXT(pkt);
+				rep = NEXT(pkt);
 				rte_mbuf_refcnt_set(pkt, 0);
 				__rte_mbuf_raw_free(pkt);
-				pkt = seg;
+				pkt = rep;
 			}
 			break;
 		}
-- 
2.1.4

^ permalink raw reply related

* Re: dpdk/vpp and cross-version migration for vhost
From: Yuanhan Liu @ 2016-11-17  9:49 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: Michael S. Tsirkin, dev, Stephen Hemminger, qemu-devel,
	libvir-list, vpp-dev, Marc-André Lureau
In-Reply-To: <b9e55320-f53d-d7d3-978f-ec696f3c1d93@redhat.com>

On Thu, Nov 17, 2016 at 09:47:09AM +0100, Maxime Coquelin wrote:
> 
> 
> On 11/17/2016 09:29 AM, Yuanhan Liu wrote:
> >As usaual, sorry for late response :/
> >
> >On Thu, Oct 13, 2016 at 08:50:52PM +0300, Michael S. Tsirkin wrote:
> >>Hi!
> >>So it looks like we face a problem with cross-version
> >>migration when using vhost. It's not new but became more
> >>acute with the advent of vhost user.
> >>
> >>For users to be able to migrate between different versions
> >>of the hypervisor the interface exposed to guests
> >>by hypervisor must stay unchanged.
> >>
> >>The problem is that a qemu device is connected
> >>to a backend in another process, so the interface
> >>exposed to guests depends on the capabilities of that
> >>process.
> >>
> >>Specifically, for vhost user interface based on virtio, this includes
> >>the "host features" bitmap that defines the interface, as well as more
> >>host values such as the max ring size.  Adding new features/changing
> >>values to this interface is required to make progress, but on the other
> >>hand we need ability to get the old host features to be compatible.
> >
> >It looks like to the same issue of vhost-user reconnect to me. For example,
> >
> >- start dpdk 16.07 & qemu 2.5
> >- kill dpdk
> >- start dpdk 16.11
> >
> >Though DPDK 16.11 has more features comparing to dpdk 16.07 (say, indirect),
> >above should work. Because qemu saves the negotiated features before the
> >disconnect and stores it back after the reconnection.
> >
> >    commit a463215b087c41d7ca94e51aa347cde523831873
> >    Author: Marc-André Lureau <marcandre.lureau@redhat.com>
> >    Date:   Mon Jun 6 18:45:05 2016 +0200
> >
> >        vhost-net: save & restore vhost-user acked features
> >
> >        The initial vhost-user connection sets the features to be negotiated
> >        with the driver. Renegotiation isn't possible without device reset.
> >
> >        To handle reconnection of vhost-user backend, ensure the same set of
> >        features are provided, and reuse already acked features.
> >
> >        Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> >
> >So we could do similar to vhost-user? I mean, save the acked features
> >before migration and store it back after it. This should be able to
> >keep the compatibility. If user downgrades DPDK version, it also could
> >be easily detected, and then exit with an error to user: migration
> >failed due to un-compatible vhost features.
> >
> >Just some rough thoughts. Makes tiny sense?
> 
> My understanding is that the management tool has to know whether
> versions are compatible before initiating the migration:

Makes sense. How about getting and restoring the acked features through
qemu command lines then, say, through the monitor interface?

With that, it would be something like:

- start vhost-user backend (DPDK, VPP, or whatever) & qemu in the src host

- read the acked features (through monitor interface)

- start vhost-user backend in the dst host

- start qemu in the dst host with the just queried acked features

  QEMU then is expected to use this feature set for the later vhost-user
  feature negotitation. Exit if features compatibility is broken.

Thoughts?

	--yliu

>  1. The downtime could be unpredictable if a VM has to move from hosts
>     to hosts multiple times, which is problematic, especially for NFV.
>  2. If migration is not possible, maybe the management tool would
>     prefer not to interrupt the VM on current host.
> 
> I have little experience with migration though, so I could be mistaken.
> 
> Thanks,
> Maxime
> 
> >
> >	--yliu
> >>
> >>To solve this problem within qemu, qemu has a versioning system based on
> >>a machine type concept which fundamentally is a version string, by
> >>specifying that string one can get hardware compatible with a previous
> >>qemu version. QEMU also reports the latest version and list of versions
> >>supported so libvirt records the version at VM creation and then is
> >>careful to use this machine version whenever it migrates a VM.
> >>
> >>One might wonder how is this solved with a kernel vhost backend. The
> >>answer is that it mostly isn't - instead an assumption is made, that
> >>qemu versions are deployed together with the kernel - this is generally
> >>true for downstreams.  Thus whenever qemu gains a new feature, it is
> >>already supported by the kernel as well.  However, if one attempts
> >>migration with a new qemu from a system with a new to old kernel, one
> >>would get a failure.
> >>
> >>In the world where we have multiple userspace backends, with some of
> >>these supplied by ISVs, this seems non-realistic.
> >>
> >>IMO we need to support vhost backend versioning, ideally
> >>in a way that will also work for vhost kernel backends.
> >>
> >>So I'd like to get some input from both backend and management
> >>developers on what a good solution would look like.
> >>
> >>If we want to emulate the qemu solution, this involves adding the
> >>concept of interface versions to dpdk.  For example, dpdk could supply a
> >>file (or utility printing?) with list of versions: latest and versions
> >>supported. libvirt could read that and
> >>- store latest version at vm creation
> >>- pass it around with the vm
> >>- pass it to qemu
> >>
> >>>From here, qemu could pass this over the vhost-user channel,
> >>thus making sure it's initialized with the correct
> >>compatible interface.
> >>
> >>As version here is an opaque string for libvirt and qemu,
> >>anything can be used - but I suggest either a list
> >>of values defining the interface, e.g.
> >>any_layout=on,max_ring=256
> >>or a version including the name and vendor of the backend,
> >>e.g. "org.dpdk.v4.5.6".
> >>
> >>Note that typically the list of supported versions can only be
> >>extended, not shrunk. Also, if the host/guest interface
> >>does not change, don't change the current version as
> >>this just creates work for everyone.
> >>
> >>Thoughts? Would this work well for management? dpdk? vpp?
> >>
> >>Thanks!
> >>
> >>--
> >>MST

^ permalink raw reply

* [PATCH 5/5] ethtool: dispaly bus info and firmware version
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

This patch enhances the ethtool-API to support to show the
bus information and firmware version as same as kernel
version ethtool displayed.

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 examples/ethtool/ethtool-app/ethapp.c | 2 ++
 examples/ethtool/lib/rte_ethtool.c    | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/examples/ethtool/ethtool-app/ethapp.c b/examples/ethtool/ethtool-app/ethapp.c
index 38e466c..9b77385 100644
--- a/examples/ethtool/ethtool-app/ethapp.c
+++ b/examples/ethtool/ethtool-app/ethapp.c
@@ -184,6 +184,8 @@ pcmd_drvinfo_callback(__rte_unused void *ptr_params,
 		printf("Port %i driver: %s (ver: %s)\n",
 			id_port, info.driver, info.version
 		      );
+		printf("bus-info: %s\n", info.bus_info);
+		printf("firmware-version: %s\n", info.fw_version);
 	}
 }
 
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index a1f91d4..542179c 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -54,6 +54,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
+	rte_eth_dev_fwver_get(port_id, drvinfo->fw_version,
+			      sizeof(drvinfo->fw_version));
+
 	memset(&dev_info, 0, sizeof(dev_info));
 	rte_eth_dev_info_get(port_id, &dev_info);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 4/5] i40e: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..f79bc5e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -324,6 +324,8 @@ static int i40e_dev_queue_stats_mapping_set(struct rte_eth_dev *dev,
 					    uint16_t queue_id,
 					    uint8_t stat_idx,
 					    uint8_t is_rx);
+static void i40e_fw_version_get(struct rte_eth_dev *dev,
+				char *fw_version, int fw_length);
 static void i40e_dev_info_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static int i40e_vlan_filter_set(struct rte_eth_dev *dev,
@@ -503,6 +505,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.stats_reset                  = i40e_dev_stats_reset,
 	.xstats_reset                 = i40e_dev_stats_reset,
 	.queue_stats_mapping_set      = i40e_dev_queue_stats_mapping_set,
+	.fw_version_get               = i40e_fw_version_get,
 	.dev_infos_get                = i40e_dev_info_get,
 	.dev_supported_ptypes_get     = i40e_dev_supported_ptypes_get,
 	.vlan_filter_set              = i40e_vlan_filter_set,
@@ -2577,6 +2580,18 @@ i40e_dev_queue_stats_mapping_set(__rte_unused struct rte_eth_dev *dev,
 }
 
 static void
+i40e_fw_version_get(struct rte_eth_dev *dev, char *fw_version, int fw_length)
+{
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	snprintf(fw_version, fw_length,
+		 "%d.%d%d 0x%04x",
+		 ((hw->nvm.version >> 12) & 0xf),
+		 ((hw->nvm.version >> 4) & 0xff),
+		 (hw->nvm.version & 0xf), hw->nvm.eetrack);
+}
+
+static void
 i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/5] ixgbe: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..da9aa31 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -193,6 +193,8 @@ static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
 					     uint8_t is_rx);
+static void ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
+				 int fw_length);
 static void ixgbe_dev_info_get(struct rte_eth_dev *dev,
 			       struct rte_eth_dev_info *dev_info);
 static const uint32_t *ixgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev);
@@ -537,6 +539,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.xstats_reset         = ixgbe_dev_xstats_reset,
 	.xstats_get_names     = ixgbe_dev_xstats_get_names,
 	.queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
+	.fw_version_get       = ixgbe_fw_version_get,
 	.dev_infos_get        = ixgbe_dev_info_get,
 	.dev_supported_ptypes_get = ixgbe_dev_supported_ptypes_get,
 	.mtu_set              = ixgbe_dev_mtu_set,
@@ -3029,6 +3032,21 @@ ixgbevf_dev_stats_reset(struct rte_eth_dev *dev)
 }
 
 static void
+ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, int fw_length)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	u16 eeprom_verh, eeprom_verl;
+	u32 etrack_id;
+
+	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
+	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
+
+	etrack_id = (eeprom_verh << 16) | eeprom_verl;
+	snprintf(fw_version, fw_length,
+		 "0x%08x", etrack_id);
+}
+
+static void
 ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/5] e1000: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/e1000/igb_ethdev.c | 46 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..c47f1c8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -120,6 +120,8 @@ static int eth_igb_xstats_get_names(struct rte_eth_dev *dev,
 				    unsigned limit);
 static void eth_igb_stats_reset(struct rte_eth_dev *dev);
 static void eth_igb_xstats_reset(struct rte_eth_dev *dev);
+static void eth_igb_fw_version_get(struct rte_eth_dev *dev,
+				   char *fw_version, int fw_length);
 static void eth_igb_infos_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static const uint32_t *eth_igb_supported_ptypes_get(struct rte_eth_dev *dev);
@@ -388,6 +390,7 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.xstats_get_names     = eth_igb_xstats_get_names,
 	.stats_reset          = eth_igb_stats_reset,
 	.xstats_reset         = eth_igb_xstats_reset,
+	.fw_version_get       = eth_igb_fw_version_get,
 	.dev_infos_get        = eth_igb_infos_get,
 	.dev_supported_ptypes_get = eth_igb_supported_ptypes_get,
 	.mtu_set              = eth_igb_mtu_set,
@@ -1977,6 +1980,49 @@ eth_igbvf_stats_reset(struct rte_eth_dev *dev)
 }
 
 static void
+eth_igb_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
+		       int fw_length)
+{
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_fw_version fw;
+
+	e1000_get_fw_version(hw, &fw);
+
+	switch (hw->mac.type) {
+	case e1000_i210:
+	case e1000_i211:
+		if (!(e1000_get_flash_presence_i210(hw))) {
+			snprintf(fw_version, fw_length,
+				 "%2d.%2d-%d",
+				 fw.invm_major, fw.invm_minor,
+				 fw.invm_img_type);
+			break;
+		}
+		/* fall through */
+	default:
+		/* if option rom is valid, display its version too*/
+		if (fw.or_valid) {
+			snprintf(fw_version, fw_length,
+				 "%d.%d, 0x%08x, %d.%d.%d",
+				 fw.eep_major, fw.eep_minor, fw.etrack_id,
+				 fw.or_major, fw.or_build, fw.or_patch);
+		/* no option rom */
+		} else {
+			if (fw.etrack_id != 0X0000) {
+			snprintf(fw_version, fw_length,
+				 "%d.%d, 0x%08x",
+				 fw.eep_major, fw.eep_minor, fw.etrack_id);
+			} else {
+			snprintf(fw_version, fw_length,
+				 "%d.%d.%d",
+				 fw.eep_major, fw.eep_minor, fw.eep_build);
+			}
+		}
+		break;
+	}
+}
+
+static void
 eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ixgbe: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..da9aa31 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -193,6 +193,8 @@ static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
 					     uint8_t is_rx);
+static void ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
+				 int fw_length);
 static void ixgbe_dev_info_get(struct rte_eth_dev *dev,
 			       struct rte_eth_dev_info *dev_info);
 static const uint32_t *ixgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev);
@@ -537,6 +539,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.xstats_reset         = ixgbe_dev_xstats_reset,
 	.xstats_get_names     = ixgbe_dev_xstats_get_names,
 	.queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
+	.fw_version_get       = ixgbe_fw_version_get,
 	.dev_infos_get        = ixgbe_dev_info_get,
 	.dev_supported_ptypes_get = ixgbe_dev_supported_ptypes_get,
 	.mtu_set              = ixgbe_dev_mtu_set,
@@ -3029,6 +3032,21 @@ ixgbevf_dev_stats_reset(struct rte_eth_dev *dev)
 }
 
 static void
+ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, int fw_length)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	u16 eeprom_verh, eeprom_verl;
+	u32 etrack_id;
+
+	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
+	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
+
+	etrack_id = (eeprom_verh << 16) | eeprom_verl;
+	snprintf(fw_version, fw_length,
+		 "0x%08x", etrack_id);
+}
+
+static void
 ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] i40e: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..f79bc5e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -324,6 +324,8 @@ static int i40e_dev_queue_stats_mapping_set(struct rte_eth_dev *dev,
 					    uint16_t queue_id,
 					    uint8_t stat_idx,
 					    uint8_t is_rx);
+static void i40e_fw_version_get(struct rte_eth_dev *dev,
+				char *fw_version, int fw_length);
 static void i40e_dev_info_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static int i40e_vlan_filter_set(struct rte_eth_dev *dev,
@@ -503,6 +505,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.stats_reset                  = i40e_dev_stats_reset,
 	.xstats_reset                 = i40e_dev_stats_reset,
 	.queue_stats_mapping_set      = i40e_dev_queue_stats_mapping_set,
+	.fw_version_get               = i40e_fw_version_get,
 	.dev_infos_get                = i40e_dev_info_get,
 	.dev_supported_ptypes_get     = i40e_dev_supported_ptypes_get,
 	.vlan_filter_set              = i40e_vlan_filter_set,
@@ -2577,6 +2580,18 @@ i40e_dev_queue_stats_mapping_set(__rte_unused struct rte_eth_dev *dev,
 }
 
 static void
+i40e_fw_version_get(struct rte_eth_dev *dev, char *fw_version, int fw_length)
+{
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	snprintf(fw_version, fw_length,
+		 "%d.%d%d 0x%04x",
+		 ((hw->nvm.version >> 12) & 0xf),
+		 ((hw->nvm.version >> 4) & 0xff),
+		 (hw->nvm.version & 0xf), hw->nvm.eetrack);
+}
+
+static void
 i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ethtool: dispaly bus info and firmware version
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

This patch enhances the ethtool-API to support to show the
bus information and firmware version as same as kernel
version ethtool displayed.

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 examples/ethtool/ethtool-app/ethapp.c | 2 ++
 examples/ethtool/lib/rte_ethtool.c    | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/examples/ethtool/ethtool-app/ethapp.c b/examples/ethtool/ethtool-app/ethapp.c
index 38e466c..9b77385 100644
--- a/examples/ethtool/ethtool-app/ethapp.c
+++ b/examples/ethtool/ethtool-app/ethapp.c
@@ -184,6 +184,8 @@ pcmd_drvinfo_callback(__rte_unused void *ptr_params,
 		printf("Port %i driver: %s (ver: %s)\n",
 			id_port, info.driver, info.version
 		      );
+		printf("bus-info: %s\n", info.bus_info);
+		printf("firmware-version: %s\n", info.fw_version);
 	}
 }
 
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index a1f91d4..542179c 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -54,6 +54,9 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
+	rte_eth_dev_fwver_get(port_id, drvinfo->fw_version,
+			      sizeof(drvinfo->fw_version));
+
 	memset(&dev_info, 0, sizeof(dev_info));
 	rte_eth_dev_info_get(port_id, &dev_info);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/5] ethdev: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang
In-Reply-To: <1479375779-46629-1-git-send-email-qiming.yang@intel.com>

This patch added API for 'rte_eth_dev_fwver_get'

void rte_eth_dev_fwver_get(uint8_t port_id,
char *fw_version, int fw_length);

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 12 ++++++++++++
 lib/librte_ether/rte_ethdev.h          | 18 ++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  7 +++++++
 3 files changed, 37 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..793e50f 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1538,6 +1538,18 @@ rte_eth_dev_set_rx_queue_stats_mapping(uint8_t port_id, uint16_t rx_queue_id,
 }
 
 void
+rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_RET(port_id);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->fw_version_get);
+	(*dev->dev_ops->fw_version_get)(dev, fw_version, fw_length);
+}
+
+void
 rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
 {
 	struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..cf54f1b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1150,6 +1150,10 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
 /**< @internal Check DD bit of specific RX descriptor */
 
+typedef void (*eth_fw_version_get_t)(struct rte_eth_dev *dev,
+				    char *fw_version, int fw_length);
+/**< @internal Get firmware information of an Ethernet device. */
+
 typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
 	uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
 
@@ -1444,6 +1448,7 @@ struct eth_dev_ops {
 	/**< Get names of extended statistics. */
 	eth_queue_stats_mapping_set_t queue_stats_mapping_set;
 	/**< Configure per queue stat counter mapping. */
+	eth_fw_version_get_t       fw_version_get; /**< Get firmware version. */
 	eth_dev_infos_get_t        dev_infos_get; /**< Get device info. */
 	eth_dev_supported_ptypes_get_t dev_supported_ptypes_get;
 	/**< Get packet types supported and identified by device*/
@@ -2385,6 +2390,19 @@ void rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr);
 void rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info);
 
 /**
+ * Retrieve the firmware version of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param fw_version
+ *   A pointer the firmware version of an Ethernet device
+ * @param fw_length
+ *   The size of the firmware version, which should be large enough to store
+ *   the firmware version of the device.
+ */
+void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int fw_length);
+
+/**
  * Retrieve the supported packet types of an Ethernet device.
  *
  * When a packet type is announced as supported, it *must* be recognized by
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 72be66d..5e6387f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -147,3 +147,10 @@ DPDK_16.11 {
 	rte_eth_dev_pci_remove;
 
 } DPDK_16.07;
+
+DPDK_17.02 {
+	global:
+
+	rte_eth_dev_fwver_get;
+
+} DPDK_16.11;
-- 
2.7.4

^ permalink raw reply related

* [PATCH] e1000: add firmware version get
From: Qiming Yang @ 2016-11-17  9:42 UTC (permalink / raw)
  To: dev; +Cc: remy.horton, jingjing.wu, jing.d.chen, Qiming Yang

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/e1000/igb_ethdev.c | 46 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..c47f1c8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -120,6 +120,8 @@ static int eth_igb_xstats_get_names(struct rte_eth_dev *dev,
 				    unsigned limit);
 static void eth_igb_stats_reset(struct rte_eth_dev *dev);
 static void eth_igb_xstats_reset(struct rte_eth_dev *dev);
+static void eth_igb_fw_version_get(struct rte_eth_dev *dev,
+				   char *fw_version, int fw_length);
 static void eth_igb_infos_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static const uint32_t *eth_igb_supported_ptypes_get(struct rte_eth_dev *dev);
@@ -388,6 +390,7 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.xstats_get_names     = eth_igb_xstats_get_names,
 	.stats_reset          = eth_igb_stats_reset,
 	.xstats_reset         = eth_igb_xstats_reset,
+	.fw_version_get       = eth_igb_fw_version_get,
 	.dev_infos_get        = eth_igb_infos_get,
 	.dev_supported_ptypes_get = eth_igb_supported_ptypes_get,
 	.mtu_set              = eth_igb_mtu_set,
@@ -1977,6 +1980,49 @@ eth_igbvf_stats_reset(struct rte_eth_dev *dev)
 }
 
 static void
+eth_igb_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
+		       int fw_length)
+{
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_fw_version fw;
+
+	e1000_get_fw_version(hw, &fw);
+
+	switch (hw->mac.type) {
+	case e1000_i210:
+	case e1000_i211:
+		if (!(e1000_get_flash_presence_i210(hw))) {
+			snprintf(fw_version, fw_length,
+				 "%2d.%2d-%d",
+				 fw.invm_major, fw.invm_minor,
+				 fw.invm_img_type);
+			break;
+		}
+		/* fall through */
+	default:
+		/* if option rom is valid, display its version too*/
+		if (fw.or_valid) {
+			snprintf(fw_version, fw_length,
+				 "%d.%d, 0x%08x, %d.%d.%d",
+				 fw.eep_major, fw.eep_minor, fw.etrack_id,
+				 fw.or_major, fw.or_build, fw.or_patch);
+		/* no option rom */
+		} else {
+			if (fw.etrack_id != 0X0000) {
+			snprintf(fw_version, fw_length,
+				 "%d.%d, 0x%08x",
+				 fw.eep_major, fw.eep_minor, fw.etrack_id);
+			} else {
+			snprintf(fw_version, fw_length,
+				 "%d.%d.%d",
+				 fw.eep_major, fw.eep_minor, fw.eep_build);
+			}
+		}
+		break;
+	}
+}
+
+static void
 eth_igb_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-- 
2.7.4

^ permalink raw reply related

* Proposal for a new Committer model
From: Mcnamara, John @ 2016-11-17  9:20 UTC (permalink / raw)
  To: dev@dpdk.org, moving@dpdk.org

Repost from the moving@dpdk.org mailing list to get a wider audience.
Original thread: http://dpdk.org/ml/archives/moving/2016-November/000059.html


Hi,

I'd like to propose a change to the DPDK committer model. Currently we have one committer for the master branch of the DPDK project. 

One committer to master represents a single point of failure and at times can be inefficient. There is also no agreed cover for times when the committer is unavailable such as vacation, public holidays, etc. I propose that we change to a multi-committer model for the DPDK project. We should have three committers for each release that can commit changes to the master branch.
 
There are a number of benefits:
 
1. Greater capacity to commit patches.
2. No single points of failure - a committer should always be available if we have three.
3. A more timely committing of patches. More committers should equal a faster turnaround - ideally, maintainers should also provide feedback on patches submitted within a 2-3 day period, as much as possible, to facilitate this. 
4. It follows best practice in creating a successful multi-vendor community - to achieve this we must ensure there is a level playing field for all participants, no single person should be required to make all of the decisions on patches to be included in the release.  

Having multiple committers will require some degree of co-ordination but there are a number of other communities successfully following this model such as Apache, OVS, FD.io, OpenStack etc. so the approach is workable.

John

^ permalink raw reply

* Re: dpdk/vpp and cross-version migration for vhost
From: Maxime Coquelin @ 2016-11-17  8:47 UTC (permalink / raw)
  To: Yuanhan Liu, Michael S. Tsirkin
  Cc: dev, Stephen Hemminger, qemu-devel, libvir-list, vpp-dev,
	Marc-André Lureau
In-Reply-To: <20161117082902.GM5048@yliu-dev.sh.intel.com>



On 11/17/2016 09:29 AM, Yuanhan Liu wrote:
> As usaual, sorry for late response :/
>
> On Thu, Oct 13, 2016 at 08:50:52PM +0300, Michael S. Tsirkin wrote:
>> Hi!
>> So it looks like we face a problem with cross-version
>> migration when using vhost. It's not new but became more
>> acute with the advent of vhost user.
>>
>> For users to be able to migrate between different versions
>> of the hypervisor the interface exposed to guests
>> by hypervisor must stay unchanged.
>>
>> The problem is that a qemu device is connected
>> to a backend in another process, so the interface
>> exposed to guests depends on the capabilities of that
>> process.
>>
>> Specifically, for vhost user interface based on virtio, this includes
>> the "host features" bitmap that defines the interface, as well as more
>> host values such as the max ring size.  Adding new features/changing
>> values to this interface is required to make progress, but on the other
>> hand we need ability to get the old host features to be compatible.
>
> It looks like to the same issue of vhost-user reconnect to me. For example,
>
> - start dpdk 16.07 & qemu 2.5
> - kill dpdk
> - start dpdk 16.11
>
> Though DPDK 16.11 has more features comparing to dpdk 16.07 (say, indirect),
> above should work. Because qemu saves the negotiated features before the
> disconnect and stores it back after the reconnection.
>
>     commit a463215b087c41d7ca94e51aa347cde523831873
>     Author: Marc-André Lureau <marcandre.lureau@redhat.com>
>     Date:   Mon Jun 6 18:45:05 2016 +0200
>
>         vhost-net: save & restore vhost-user acked features
>
>         The initial vhost-user connection sets the features to be negotiated
>         with the driver. Renegotiation isn't possible without device reset.
>
>         To handle reconnection of vhost-user backend, ensure the same set of
>         features are provided, and reuse already acked features.
>
>         Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>
> So we could do similar to vhost-user? I mean, save the acked features
> before migration and store it back after it. This should be able to
> keep the compatibility. If user downgrades DPDK version, it also could
> be easily detected, and then exit with an error to user: migration
> failed due to un-compatible vhost features.
>
> Just some rough thoughts. Makes tiny sense?

My understanding is that the management tool has to know whether
versions are compatible before initiating the migration:
  1. The downtime could be unpredictable if a VM has to move from hosts
     to hosts multiple times, which is problematic, especially for NFV.
  2. If migration is not possible, maybe the management tool would
     prefer not to interrupt the VM on current host.

I have little experience with migration though, so I could be mistaken.

Thanks,
Maxime

>
> 	--yliu
>>
>> To solve this problem within qemu, qemu has a versioning system based on
>> a machine type concept which fundamentally is a version string, by
>> specifying that string one can get hardware compatible with a previous
>> qemu version. QEMU also reports the latest version and list of versions
>> supported so libvirt records the version at VM creation and then is
>> careful to use this machine version whenever it migrates a VM.
>>
>> One might wonder how is this solved with a kernel vhost backend. The
>> answer is that it mostly isn't - instead an assumption is made, that
>> qemu versions are deployed together with the kernel - this is generally
>> true for downstreams.  Thus whenever qemu gains a new feature, it is
>> already supported by the kernel as well.  However, if one attempts
>> migration with a new qemu from a system with a new to old kernel, one
>> would get a failure.
>>
>> In the world where we have multiple userspace backends, with some of
>> these supplied by ISVs, this seems non-realistic.
>>
>> IMO we need to support vhost backend versioning, ideally
>> in a way that will also work for vhost kernel backends.
>>
>> So I'd like to get some input from both backend and management
>> developers on what a good solution would look like.
>>
>> If we want to emulate the qemu solution, this involves adding the
>> concept of interface versions to dpdk.  For example, dpdk could supply a
>> file (or utility printing?) with list of versions: latest and versions
>> supported. libvirt could read that and
>> - store latest version at vm creation
>> - pass it around with the vm
>> - pass it to qemu
>>
>> >From here, qemu could pass this over the vhost-user channel,
>> thus making sure it's initialized with the correct
>> compatible interface.
>>
>> As version here is an opaque string for libvirt and qemu,
>> anything can be used - but I suggest either a list
>> of values defining the interface, e.g.
>> any_layout=on,max_ring=256
>> or a version including the name and vendor of the backend,
>> e.g. "org.dpdk.v4.5.6".
>>
>> Note that typically the list of supported versions can only be
>> extended, not shrunk. Also, if the host/guest interface
>> does not change, don't change the current version as
>> this just creates work for everyone.
>>
>> Thoughts? Would this work well for management? dpdk? vpp?
>>
>> Thanks!
>>
>> --
>> MST

^ permalink raw reply

* Re: dpdk/vpp and cross-version migration for vhost
From: Yuanhan Liu @ 2016-11-17  8:29 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: dev, Stephen Hemminger, Maxime Coquelin, qemu-devel, libvir-list,
	vpp-dev, Marc-André Lureau
In-Reply-To: <20161011173526-mutt-send-email-mst@kernel.org>

As usaual, sorry for late response :/

On Thu, Oct 13, 2016 at 08:50:52PM +0300, Michael S. Tsirkin wrote:
> Hi!
> So it looks like we face a problem with cross-version
> migration when using vhost. It's not new but became more
> acute with the advent of vhost user.
> 
> For users to be able to migrate between different versions
> of the hypervisor the interface exposed to guests
> by hypervisor must stay unchanged.
> 
> The problem is that a qemu device is connected
> to a backend in another process, so the interface
> exposed to guests depends on the capabilities of that
> process.
> 
> Specifically, for vhost user interface based on virtio, this includes
> the "host features" bitmap that defines the interface, as well as more
> host values such as the max ring size.  Adding new features/changing
> values to this interface is required to make progress, but on the other
> hand we need ability to get the old host features to be compatible.

It looks like to the same issue of vhost-user reconnect to me. For example,

- start dpdk 16.07 & qemu 2.5
- kill dpdk
- start dpdk 16.11

Though DPDK 16.11 has more features comparing to dpdk 16.07 (say, indirect),
above should work. Because qemu saves the negotiated features before the
disconnect and stores it back after the reconnection.

    commit a463215b087c41d7ca94e51aa347cde523831873
    Author: Marc-André Lureau <marcandre.lureau@redhat.com>
    Date:   Mon Jun 6 18:45:05 2016 +0200
    
        vhost-net: save & restore vhost-user acked features
    
        The initial vhost-user connection sets the features to be negotiated
        with the driver. Renegotiation isn't possible without device reset.
    
        To handle reconnection of vhost-user backend, ensure the same set of
        features are provided, and reuse already acked features.
    
        Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>


So we could do similar to vhost-user? I mean, save the acked features
before migration and store it back after it. This should be able to
keep the compatibility. If user downgrades DPDK version, it also could
be easily detected, and then exit with an error to user: migration
failed due to un-compatible vhost features.

Just some rough thoughts. Makes tiny sense?

	--yliu
> 
> To solve this problem within qemu, qemu has a versioning system based on
> a machine type concept which fundamentally is a version string, by
> specifying that string one can get hardware compatible with a previous
> qemu version. QEMU also reports the latest version and list of versions
> supported so libvirt records the version at VM creation and then is
> careful to use this machine version whenever it migrates a VM.
> 
> One might wonder how is this solved with a kernel vhost backend. The
> answer is that it mostly isn't - instead an assumption is made, that
> qemu versions are deployed together with the kernel - this is generally
> true for downstreams.  Thus whenever qemu gains a new feature, it is
> already supported by the kernel as well.  However, if one attempts
> migration with a new qemu from a system with a new to old kernel, one
> would get a failure.
> 
> In the world where we have multiple userspace backends, with some of
> these supplied by ISVs, this seems non-realistic.
> 
> IMO we need to support vhost backend versioning, ideally
> in a way that will also work for vhost kernel backends.
> 
> So I'd like to get some input from both backend and management
> developers on what a good solution would look like.
> 
> If we want to emulate the qemu solution, this involves adding the
> concept of interface versions to dpdk.  For example, dpdk could supply a
> file (or utility printing?) with list of versions: latest and versions
> supported. libvirt could read that and
> - store latest version at vm creation
> - pass it around with the vm
> - pass it to qemu
> 
> >From here, qemu could pass this over the vhost-user channel,
> thus making sure it's initialized with the correct
> compatible interface.
> 
> As version here is an opaque string for libvirt and qemu,
> anything can be used - but I suggest either a list
> of values defining the interface, e.g.
> any_layout=on,max_ring=256
> or a version including the name and vendor of the backend,
> e.g. "org.dpdk.v4.5.6".
> 
> Note that typically the list of supported versions can only be
> extended, not shrunk. Also, if the host/guest interface
> does not change, don't change the current version as
> this just creates work for everyone.
> 
> Thoughts? Would this work well for management? dpdk? vpp?
> 
> Thanks!
> 
> -- 
> MST

^ permalink raw reply

* [PATCH v3] eal/linuxapp: fix return value check of mknod()
From: Wei Dai @ 2016-11-17  6:47 UTC (permalink / raw)
  To: david.marchand, olivier.matz; +Cc: dev, Wei Dai
In-Reply-To: <1479264047-67966-1-git-send-email-wei.dai@intel.com>

In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
The return value of mknod() is ret, not f got by fopen().
So the value of ret should be checked for mknod().

Fixes: f7f97c16048e ("pci: add option --create-uio-dev to run without hotplug")

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
v3:
* correct Fixes: line in git commit message body

v2:
* fix my local git setting and send same patch again to remove
  "From: Wei Dai <wei.dai@intel.com>" in git commit message body
  and make merging easier

 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 1786b75..3e4ffb5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path, unsigned uio_num)
 	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
 	dev = makedev(major, minor);
 	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
-	if (f == NULL) {
+	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "%s(): mknod() failed %s\n",
 			__func__, strerror(errno));
 		return -1;
-- 
2.5.5

^ permalink raw reply related

* [RFC PATCH 6/6] eal: removing eth_driver
From: Shreyansh Jain @ 2016-11-17  5:30 UTC (permalink / raw)
  To: david.marchand; +Cc: dev, Shreyansh Jain
In-Reply-To: <1479360605-20558-1-git-send-email-shreyansh.jain@nxp.com>

This patch demonstrates how eth_driver can be replaced with appropriate
changes for rte_xxx_driver from the PMD itself. It uses ixgbe_ethernet as
an example.

A large set of changes exists in the rte_ethdev.c - primarily because too
much PCI centric code (names, assumption of rte_pci_device) still exists
in it. Most, except symbol naming, has been changed in this patch.

This proposes that:
 - PMD would declare the rte_xxx_driver. In case of ixgbe, it would be
   rte_pci_driver.
 - Probe and remove continue to exists in rte_pci_driver. But, the
   rte_driver has new hooks for init and uninit. The rationale is that
   once a ethernet or cryto device is created, the rte_driver->init would
   be responsible for initializing the device.
   -- Eth_dev -> rte_driver -> rte_pci_driver
                          |    `-> probe/remove
                          `--> init/uninit
 - necessary changes in the rte_eth_dev have also been done so that it
   refers to the rte_device and rte_driver rather than rte_xxx_*. This
   would imply, ethernet device is 'linked' to a rte_device/rte_driver
   which in turn is a rte_xxx_device/rte_xxx_driver type.
   - for all operations related to extraction relvant xxx type,
     container_of would have to be used.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 49 +++++++++++++++++++++-------------------
 lib/librte_ether/rte_ethdev.c    | 36 +++++++++++++++++------------
 lib/librte_ether/rte_ethdev.h    |  6 ++---
 3 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..acead31 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1419,7 +1419,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
-	pci_dev = eth_dev->pci_dev;
+	pci_dev = container_of(eth_dev->device, struct rte_pci_device, device);
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
@@ -1532,7 +1532,9 @@ static int
 eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct ixgbe_hw *hw;
-	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
+	struct rte_pci_device *pci_dev;
+
+	pci_dev = container_of(eth_dev->device, struct rte_pci_device, device);
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1562,32 +1564,33 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
-static struct eth_driver rte_ixgbe_pmd = {
-	.pci_drv = {
-		.id_table = pci_id_ixgbe_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
+static struct rte_pci_driver rte_ixgbe_pci_driver = {
+	.id_table = pci_id_ixgbe_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+		     RTE_PCI_DRV_DETACHABLE,
+	.probe = rte_eth_dev_pci_probe,
+	.remove = rte_eth_dev_pci_remove,
+	.driver = {
+		.driver_init_t= eth_ixgbe_dev_init,
+		.driver_uninit_t= eth_ixgbe_dev_uninit,
+		.dev_private_size = sizeof(struct ixgbe_adapter),
 	},
-	.eth_dev_init = eth_ixgbe_dev_init,
-	.eth_dev_uninit = eth_ixgbe_dev_uninit,
-	.dev_private_size = sizeof(struct ixgbe_adapter),
 };
 
 /*
  * virtual function driver struct
  */
-static struct eth_driver rte_ixgbevf_pmd = {
-	.pci_drv = {
-		.id_table = pci_id_ixgbevf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
+static struct rte_pci_driver rte_ixgbevf_pci_driver = {
+	.id_table = pci_id_ixgbevf_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+	.probe = rte_eth_dev_pci_probe,
+	.remove = rte_eth_dev_pci_remove,
+	.driver = {
+		/* rte_driver hooks */
+		.init = eth_ixgbevf_dev_init,
+		.uninit = eth_ixgbevf_dev_uninit,
+		.dev_private_size = sizeof(struct ixgbe_adapter),
 	},
-	.eth_dev_init = eth_ixgbevf_dev_init,
-	.eth_dev_uninit = eth_ixgbevf_dev_uninit,
-	.dev_private_size = sizeof(struct ixgbe_adapter),
 };
 
 static int
@@ -7592,7 +7595,7 @@ ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 	ixgbevf_dev_interrupt_action(dev);
 }
 
-RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pci_driver);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
-RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pci_driver);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..3535ff4 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -235,13 +235,13 @@ int
 rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 		      struct rte_pci_device *pci_dev)
 {
-	struct eth_driver    *eth_drv;
+	struct rte_driver *drv;
 	struct rte_eth_dev *eth_dev;
 	char ethdev_name[RTE_ETH_NAME_MAX_LEN];
 
 	int diag;
 
-	eth_drv = (struct eth_driver *)pci_drv;
+	drv = pci_drv->driver;
 
 	rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
 			sizeof(ethdev_name));
@@ -252,13 +252,13 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
-				  eth_drv->dev_private_size,
+				  drv->dev_private_size,
 				  RTE_CACHE_LINE_SIZE);
 		if (eth_dev->data->dev_private == NULL)
 			rte_panic("Cannot allocate memzone for private port data\n");
 	}
-	eth_dev->pci_dev = pci_dev;
-	eth_dev->driver = eth_drv;
+	eth_dev->device = pci_dev->device;
+	eth_dev->driver = drv;
 	eth_dev->data->rx_mbuf_alloc_failed = 0;
 
 	/* init user callbacks */
@@ -270,7 +270,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 	eth_dev->data->mtu = ETHER_MTU;
 
 	/* Invoke PMD device initialization function */
-	diag = (*eth_drv->eth_dev_init)(eth_dev);
+	diag = (*drv->init)(eth_dev);
 	if (diag == 0)
 		return 0;
 
@@ -287,7 +287,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 int
 rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
 {
-	const struct eth_driver *eth_drv;
+	const struct rte_driver *drv;
 	struct rte_eth_dev *eth_dev;
 	char ethdev_name[RTE_ETH_NAME_MAX_LEN];
 	int ret;
@@ -302,11 +302,11 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
 	if (eth_dev == NULL)
 		return -ENODEV;
 
-	eth_drv = (const struct eth_driver *)pci_dev->driver;
+	drv = pci_dev->driver;
 
 	/* Invoke PMD device uninit function */
-	if (*eth_drv->eth_dev_uninit) {
-		ret = (*eth_drv->eth_dev_uninit)(eth_dev);
+	if (*drv->uninit) {
+		ret = (*drv->uninit)(eth_dev);
 		if (ret)
 			return ret;
 	}
@@ -317,7 +317,7 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_free(eth_dev->data->dev_private);
 
-	eth_dev->pci_dev = NULL;
+	eth_dev->device = NULL;
 	eth_dev->driver = NULL;
 	eth_dev->data = NULL;
 
@@ -1556,7 +1556,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
 
 	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
 	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
-	dev_info->pci_dev = dev->pci_dev;
+	dev_info->device = dev->device;
 	dev_info->driver_name = dev->data->drv_name;
 	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
 	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
@@ -2537,6 +2537,7 @@ rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
 {
 	uint32_t vec;
 	struct rte_eth_dev *dev;
+	struct rte_pci_device *pci_dev;
 	struct rte_intr_handle *intr_handle;
 	uint16_t qid;
 	int rc;
@@ -2544,6 +2545,10 @@ rte_eth_dev_rx_intr_ctl(uint8_t port_id, int epfd, int op, void *data)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
 	dev = &rte_eth_devices[port_id];
+	/* TODO intr_handle is currently in rte_pci_device;
+	 * Below is incorrect until that time
+	 */
+	pci_dev = container_of(dev->device, struct rte_pci_device, device);
 	intr_handle = &dev->pci_dev->intr_handle;
 	if (!intr_handle->intr_vec) {
 		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
@@ -2572,7 +2577,7 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 	const struct rte_memzone *mz;
 
 	snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
-		 dev->driver->pci_drv.driver.name, ring_name,
+		 dev->driver->name, ring_name,
 		 dev->data->port_id, queue_id);
 
 	mz = rte_memzone_lookup(z_name);
@@ -2593,6 +2598,7 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
 {
 	uint32_t vec;
 	struct rte_eth_dev *dev;
+	struct rte_pci_device *pci_dev;
 	struct rte_intr_handle *intr_handle;
 	int rc;
 
@@ -2604,7 +2610,9 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t queue_id,
 		return -EINVAL;
 	}
 
-	intr_handle = &dev->pci_dev->intr_handle;
+	/* TODO; Until intr_handle is available in rte_device, below is incorrect */
+	pci_dev = container_of(dev->device, struct rte_pci_device, device);
+	intr_handle = &pci_dev->intr_handle;
 	if (!intr_handle->intr_vec) {
 		RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
 		return -EPERM;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 38641e8..2b1d826 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -876,7 +876,7 @@ struct rte_eth_conf {
  * Ethernet device information
  */
 struct rte_eth_dev_info {
-	struct rte_pci_device *pci_dev; /**< Device PCI information. */
+	struct rte_device *device; /**< Device PCI information. */
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if none.
 		Use if_indextoname() to translate into an interface name. */
@@ -1623,9 +1623,9 @@ struct rte_eth_dev {
 	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
 	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
 	struct rte_eth_dev_data *data;  /**< Pointer to device data */
-	const struct eth_driver *driver;/**< Driver for this device */
+	const struct rte_driver *driver;/**< Driver for this device */
 	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
-	struct rte_pci_device *pci_dev; /**< PCI info. supplied by probing */
+	struct rte_device *device; /**< Device instance */
 	/** User application callbacks for NIC interrupts */
 	struct rte_eth_dev_cb_list link_intr_cbs;
 	/**
-- 
2.7.4

^ permalink raw reply related

* [RFC PATCH 5/6] eal: supporting bus model in init process
From: Shreyansh Jain @ 2016-11-17  5:30 UTC (permalink / raw)
  To: david.marchand; +Cc: dev, Shreyansh Jain
In-Reply-To: <1479360605-20558-1-git-send-email-shreyansh.jain@nxp.com>

This patch makes necessary changes to the EAL for handling bus scan and
probe. Most of the scan function has been moved to bus/* implementation,
(currently only for pci, linuxapp). There are still some functions which
exists in the EAL (bind, unbind module, mmap etc).

Missing/Grey area;
 - Should all the operations for a PCI device, whether binding to a driver
   or mmap, be moved to PCI bus code base? All of that is not relevant for
   a bus, but then having multiple implementation areas for a common
   sub-system (PCI, here) is also not a nice thing.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/linuxapp/eal/Makefile  |   1 +
 lib/librte_eal/linuxapp/eal/eal.c     |  51 +++++-
 lib/librte_eal/linuxapp/eal/eal_pci.c | 298 ----------------------------------
 3 files changed, 44 insertions(+), 306 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 4e206f0..124dceb 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -77,6 +77,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_timer.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_memzone.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_log.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_launch.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_vdev.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_pci.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_pci_uio.c
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 2075282..332d1f4 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -739,6 +739,44 @@ static int rte_eal_vfio_setup(void)
 }
 #endif
 
+static int
+rte_eal_scan(void)
+{
+	int ret = 0;
+
+	/* For now, as vdev replacement is not complete, continue with a call
+	 * rte_bus_list scan
+	 */
+	rte_eal_bus_scan();
+
+	ret = rte_eal_dev_init();
+	if (ret) {
+		RTE_LOG(ERR, EAL, "Cannot init pmd devices\n");
+	}
+
+out:
+	return ret;
+}
+
+static int
+rte_eal_probe(void)
+{
+	int ret = 0;
+
+	/* Probe & Initialize PCI devices */
+	ret = rte_eal_pci_probe();
+	if (ret) {
+		RTE_LOG(ERR, EAL, "Cannot probe PCI\n");
+		goto out;
+	}
+
+	/* ToDo: vdev scan already does the probe - it should be moved here */
+
+	/* Last successful bus probe would set ret = 0 */
+out:
+	return ret;
+}
+
 /* Launch threads, called at application init(). */
 int
 rte_eal_init(int argc, char **argv)
@@ -802,9 +840,6 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0)
 		rte_panic("Cannot init logs\n");
 
-	if (rte_eal_pci_init() < 0)
-		rte_panic("Cannot init PCI\n");
-
 #ifdef VFIO_PRESENT
 	if (rte_eal_vfio_setup() < 0)
 		rte_panic("Cannot init VFIO\n");
@@ -828,6 +863,9 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_timer_init() < 0)
 		rte_panic("Cannot init HPET or TSC timers\n");
 
+	if (rte_eal_scan() < 0)
+		rte_panic("Cannot scan for devices\n");
+
 	eal_check_mem_on_local_socket();
 
 	if (eal_plugins_init() < 0)
@@ -841,9 +879,6 @@ rte_eal_init(int argc, char **argv)
 		rte_config.master_lcore, (int)thread_id, cpuset,
 		ret == 0 ? "" : "...");
 
-	if (rte_eal_dev_init() < 0)
-		rte_panic("Cannot init pmd devices\n");
-
 	if (rte_eal_intr_init() < 0)
 		rte_panic("Cannot init interrupt-handling thread\n");
 
@@ -884,8 +919,8 @@ rte_eal_init(int argc, char **argv)
 	rte_eal_mp_wait_lcore();
 
 	/* Probe & Initialize PCI devices */
-	if (rte_eal_pci_probe())
-		rte_panic("Cannot probe PCI\n");
+	if (rte_eal_probe())
+		rte_panic("Cannot complete probe\n");
 
 	rte_eal_mcfg_complete();
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 876ba38..e3916ab 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -224,202 +224,6 @@ pci_parse_one_sysfs_resource(char *line, size_t len, uint64_t *phys_addr,
 	return 0;
 }
 
-/* parse the "resource" sysfs file */
-static int
-pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
-{
-	FILE *f;
-	char buf[BUFSIZ];
-	int i;
-	uint64_t phys_addr, end_addr, flags;
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot open sysfs resource\n");
-		return -1;
-	}
-
-	for (i = 0; i<PCI_MAX_RESOURCE; i++) {
-
-		if (fgets(buf, sizeof(buf), f) == NULL) {
-			RTE_LOG(ERR, EAL,
-				"%s(): cannot read resource\n", __func__);
-			goto error;
-		}
-		if (pci_parse_one_sysfs_resource(buf, sizeof(buf), &phys_addr,
-				&end_addr, &flags) < 0)
-			goto error;
-
-		if (flags & IORESOURCE_MEM) {
-			dev->mem_resource[i].phys_addr = phys_addr;
-			dev->mem_resource[i].len = end_addr - phys_addr + 1;
-			/* not mapped for now */
-			dev->mem_resource[i].addr = NULL;
-		}
-	}
-	fclose(f);
-	return 0;
-
-error:
-	fclose(f);
-	return -1;
-}
-
-/* Scan one pci sysfs entry, and fill the devices list from it. */
-static int
-pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
-	     uint8_t devid, uint8_t function)
-{
-	char filename[PATH_MAX];
-	unsigned long tmp;
-	struct rte_pci_device *dev;
-	char driver[PATH_MAX];
-	int ret;
-
-	dev = malloc(sizeof(*dev));
-	if (dev == NULL)
-		return -1;
-
-	memset(dev, 0, sizeof(*dev));
-	dev->addr.domain = domain;
-	dev->addr.bus = bus;
-	dev->addr.devid = devid;
-	dev->addr.function = function;
-
-	/* get vendor id */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
-		free(dev);
-		return -1;
-	}
-	dev->id.vendor_id = (uint16_t)tmp;
-
-	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
-		free(dev);
-		return -1;
-	}
-	dev->id.device_id = (uint16_t)tmp;
-
-	/* get subsystem_vendor id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_vendor",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
-		free(dev);
-		return -1;
-	}
-	dev->id.subsystem_vendor_id = (uint16_t)tmp;
-
-	/* get subsystem_device id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_device",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
-		free(dev);
-		return -1;
-	}
-	dev->id.subsystem_device_id = (uint16_t)tmp;
-
-	/* get class_id */
-	snprintf(filename, sizeof(filename), "%s/class",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
-		free(dev);
-		return -1;
-	}
-	/* the least 24 bits are valid: class, subclass, program interface */
-	dev->id.class_id = (uint32_t)tmp & RTE_CLASS_ANY_ID;
-
-	/* get max_vfs */
-	dev->max_vfs = 0;
-	snprintf(filename, sizeof(filename), "%s/max_vfs", dirname);
-	if (!access(filename, F_OK) &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
-		dev->max_vfs = (uint16_t)tmp;
-	else {
-		/* for non igb_uio driver, need kernel version >= 3.8 */
-		snprintf(filename, sizeof(filename),
-			 "%s/sriov_numvfs", dirname);
-		if (!access(filename, F_OK) &&
-		    eal_parse_sysfs_value(filename, &tmp) == 0)
-			dev->max_vfs = (uint16_t)tmp;
-	}
-
-	/* get numa node */
-	snprintf(filename, sizeof(filename), "%s/numa_node",
-		 dirname);
-	if (access(filename, R_OK) != 0) {
-		/* if no NUMA support, set default to 0 */
-		dev->device.numa_node = 0;
-	} else {
-		if (eal_parse_sysfs_value(filename, &tmp) < 0) {
-			free(dev);
-			return -1;
-		}
-		dev->device.numa_node = tmp;
-	}
-
-	/* parse resources */
-	snprintf(filename, sizeof(filename), "%s/resource", dirname);
-	if (pci_parse_sysfs_resource(filename, dev) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse resource\n", __func__);
-		free(dev);
-		return -1;
-	}
-
-	/* parse driver */
-	snprintf(filename, sizeof(filename), "%s/driver", dirname);
-	ret = pci_get_kernel_driver_by_path(filename, driver);
-	if (ret < 0) {
-		RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
-		free(dev);
-		return -1;
-	}
-
-	if (!ret) {
-		if (!strcmp(driver, "vfio-pci"))
-			dev->kdrv = RTE_KDRV_VFIO;
-		else if (!strcmp(driver, "igb_uio"))
-			dev->kdrv = RTE_KDRV_IGB_UIO;
-		else if (!strcmp(driver, "uio_pci_generic"))
-			dev->kdrv = RTE_KDRV_UIO_GENERIC;
-		else
-			dev->kdrv = RTE_KDRV_UNKNOWN;
-	} else
-		dev->kdrv = RTE_KDRV_NONE;
-
-	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		rte_eal_device_insert(&dev->device);
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	} else {
-		struct rte_pci_device *dev2;
-		int ret;
-
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
-			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
-			if (ret > 0)
-				continue;
-
-			if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
-				rte_eal_device_insert(&dev->device);
-			} else { /* already registered */
-				dev2->kdrv = dev->kdrv;
-				dev2->max_vfs = dev->max_vfs;
-				memmove(dev2->mem_resource, dev->mem_resource,
-					sizeof(dev->mem_resource));
-				free(dev);
-			}
-			return 0;
-		}
-		rte_eal_device_insert(&dev->device);
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	}
-
-	return 0;
-}
-
 int
 pci_update_device(const struct rte_pci_addr *addr)
 {
@@ -433,93 +237,7 @@ pci_update_device(const struct rte_pci_addr *addr)
 				addr->function);
 }
 
-/*
- * split up a pci address into its constituent parts.
- */
-static int
-parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
-		uint8_t *bus, uint8_t *devid, uint8_t *function)
-{
-	/* first split on ':' */
-	union splitaddr {
-		struct {
-			char *domain;
-			char *bus;
-			char *devid;
-			char *function;
-		};
-		char *str[PCI_FMT_NVAL]; /* last element-separator is "." not ":" */
-	} splitaddr;
-
-	char *buf_copy = strndup(buf, bufsize);
-	if (buf_copy == NULL)
-		return -1;
-
-	if (rte_strsplit(buf_copy, bufsize, splitaddr.str, PCI_FMT_NVAL, ':')
-			!= PCI_FMT_NVAL - 1)
-		goto error;
-	/* final split is on '.' between devid and function */
-	splitaddr.function = strchr(splitaddr.devid,'.');
-	if (splitaddr.function == NULL)
-		goto error;
-	*splitaddr.function++ = '\0';
-
-	/* now convert to int values */
-	errno = 0;
-	*domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
-	*bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
-	*devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
-	*function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
-	if (errno != 0)
-		goto error;
-
-	free(buf_copy); /* free the copy made with strdup */
-	return 0;
-error:
-	free(buf_copy);
-	return -1;
-}
-
-/*
- * Scan the content of the PCI bus, and the devices in the devices
- * list
- */
-int
-rte_eal_pci_scan(void)
-{
-	struct dirent *e;
-	DIR *dir;
-	char dirname[PATH_MAX];
-	uint16_t domain;
-	uint8_t bus, devid, function;
-
-	dir = opendir(pci_get_sysfs_path());
-	if (dir == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
-			__func__, strerror(errno));
-		return -1;
-	}
 
-	while ((e = readdir(dir)) != NULL) {
-		if (e->d_name[0] == '.')
-			continue;
-
-		if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
-				&bus, &devid, &function) != 0)
-			continue;
-
-		snprintf(dirname, sizeof(dirname), "%s/%s",
-				pci_get_sysfs_path(), e->d_name);
-		if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
-			goto error;
-	}
-	closedir(dir);
-	return 0;
-
-error:
-	closedir(dir);
-	return -1;
-}
 
 /* Read PCI config space. */
 int rte_eal_pci_read_config(const struct rte_pci_device *device,
@@ -754,19 +472,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
 
 	return ret;
 }
-
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
-	/* for debug purposes, PCI can be disabled */
-	if (internal_config.no_pci)
-		return 0;
-
-	if (rte_eal_pci_scan() < 0) {
-		RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
-		return -1;
-	}
-
-	return 0;
-}
-- 
2.7.4

^ permalink raw reply related

* [RFC PATCH 4/6] eal/common: handle bus abstraction for device/driver objects
From: Shreyansh Jain @ 2016-11-17  5:30 UTC (permalink / raw)
  To: david.marchand; +Cc: dev, Shreyansh Jain
In-Reply-To: <1479360605-20558-1-git-send-email-shreyansh.jain@nxp.com>

Primary changes done by this patch are based on:
 - Devices belong to the bus hence the device list is bus instance
   specific
 - Similarly, drivers belong to the bus and thus they too are enclosed
   within the bus instance.
 - All device insertion and driver registration should proceed through
   bus APIs. A new file, eal_common_bus.c has been added for that.

Exiting driver registration and device insert/remove APIs have been
modified to work with bus on which device/driver belong. On the same
lines, the PCI common functions have been modified to work with bus rather
than device/driver directly.

rte_eal_pci_scan is no longer an exposed API. It is part of the bus
implementation. Though, probe continues to be part of the common PCI
operations.

Probe has been split into match and probe. Match has been moved to bus/*
code and is a hook now. EAL code would be modified to handle this hook.

Missing/Grey area:
 - Some API like inserting a device at a particular position in the device
   list are missing. Same for driver. These are needed for cases where
   device update is done rather than addition.
 - Probe is a property of a driver but it should be initiated from a bus,
   for example when added a new device (hotplugging). At present
   rte_driver has the probe hook. This should be wrapped around some API
   at the bus level so that bus can search through multiple drivers
   associated with it for calling probe.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/eal_common_bus.c  | 188 ++++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_dev.c  |  31 +++--
 lib/librte_eal/common/eal_common_pci.c  | 226 +++++++++++++++++++-------------
 lib/librte_eal/common/include/rte_pci.h |  11 +-
 4 files changed, 342 insertions(+), 114 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_bus.c

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
new file mode 100644
index 0000000..3de1ac7
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -0,0 +1,188 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 NXP
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of NXP nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_dev.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+#include <rte_devargs.h>
+#include <rte_log.h>
+
+#include "eal_private.h"
+
+/** @internal
+ * Add a device to a bus.
+ */
+void
+rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
+{
+	/* XXX all the additions can be address ordered ?
+	 * for example, calling rte_eal_compare_pci_addr and getting <=
+	 * and performing insert a specific location
+	 */
+	RTE_VERIFY(bus);
+	RTE_VERIFY(dev);
+
+	TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
+}
+
+/** @internal
+ * Remove a device from its bus.
+ */
+void
+rte_eal_bus_remove_device(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	RTE_VERIFY(bus);
+	RTE_VERIFY(dev);
+
+	bus = dev->bus;
+	TAILQ_REMOVE(&bus->device_list, dev, next);
+}
+
+/** @internal
+ * Associate a driver with a bus.
+ */
+void
+rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv)
+{
+	RTE_VERIFY(bus);
+	RTE_VERIFY(drv);
+
+	TAILQ_INSERT_TAIL(&bus->driver_list, drv, next);
+}
+
+/** @internal
+ * Disassociate a driver from bus.
+ */
+void
+rte_eal_bus_remove_driver(struct rte_driver *drv)
+{
+	struct rte_bus *bus;
+	RTE_VERIFY(bus);
+	RTE_VERIFY(drv);
+
+	bus = drv->bus;
+	TAILQ_REMOVE(&bus->driver_list, dev, next);
+}
+
+/**
+ * Scan all the associated buses
+ */
+int
+rte_eal_bus_scan(void)
+{
+	int ret = 0;
+	struct rte_bus *bus;
+
+	if (TAILQ_EMPTY(&rte_bus_list)) {
+		return 0;
+	}
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		ret = bus->scan();
+		if (ret) {
+			RTE_LOG(ERR, EAL, "Scan for [%s] bus failed.\n",
+				bus->name);
+			continue; /* not a reason to break other bus scan */
+		}
+	}
+
+	/* This function essentially never returns an error - in case of error
+	 * no devices (or limited devices) are available to the application
+	 * which then can fail/report error.
+	 */
+	return 0;
+}
+
+/**
+ * Get the bus handle using its name
+ */
+struct rte_bus *
+rte_eal_get_bus(const char *bus_name)
+{
+	struct rte_bus *bus;
+
+	RTE_VERIFY(bus_name);
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		RTE_VERIFY(bus->name);
+
+		if (!strncmp(bus_name, bus->name)) {
+			RTE_LOG(DEBUG, EAL, "Returning Bus object %p\n", bus);
+			return bus;
+		}
+	}
+
+	/* Unable to find bus requested */
+	return NULL;
+}
+
+/* register a bus */
+void
+rte_eal_bus_register(struct rte_bus *bus)
+{
+	/* 3 conditions must meet:
+	 * 1. scan hook should be defined.
+	 * 2. match hook should be defined.
+	 * 3. Name should be a valid string. (valid?)
+	 */
+	RTE_VERIFY(bus);
+	RTE_VERIFY(bus->scan);
+	RTE_VERIFY(bus->probe);
+	RTE_VERIFY(bus->name && strlen(bus->name));
+
+	/* Initialize the driver and device list associated with the bus */
+	TAILQ_HEAD_INITIALIZER(&(bus->driver_list));
+	TAILQ_HEAD_INITIALIZER(&(bus->device_list));
+
+	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
+	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
+}
+
+/* unregister a bus */
+void
+rte_eal_bus_unregister(struct rte_bus *bus)
+{
+	/* All devices and drivers associated with the bus should have been
+	 * 'device->uninit' and 'driver->remove()' already.
+	 */
+	RTE_VERIFY(TAILQ_EMPTY(&(bus->driver_list)));
+	RTE_VERIFY(TAILQ_EMPTY(&(bus->device_list)));
+
+	TAILQ_REMOVE(&rte_bus_list, bus, next);
+}
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 4f3b493..bb8d266 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -45,35 +45,44 @@
 
 #include "eal_private.h"
 
-/** Global list of device drivers. */
-static struct rte_driver_list dev_driver_list =
-	TAILQ_HEAD_INITIALIZER(dev_driver_list);
-/** Global list of device drivers. */
-static struct rte_device_list dev_device_list =
-	TAILQ_HEAD_INITIALIZER(dev_device_list);
-
 /* register a driver */
 void
 rte_eal_driver_register(struct rte_driver *driver)
 {
-	TAILQ_INSERT_TAIL(&dev_driver_list, driver, next);
+	struct rte_bus *bus;
+
+	RTE_VERIFY(driver && driver->bus);
+	bus = driver->bus;
+	TAILQ_INSERT_TAIL(&(bus->driver_list), driver, next);
 }
 
 /* unregister a driver */
 void
 rte_eal_driver_unregister(struct rte_driver *driver)
 {
-	TAILQ_REMOVE(&dev_driver_list, driver, next);
+	struct rte_bus;
+
+	RTE_VERIFY(driver && driver->bus);
+	bus = driver->bus;
+	TAILQ_REMOVE(&(bus->driver_list), driver, next);
 }
 
 void rte_eal_device_insert(struct rte_device *dev)
 {
-	TAILQ_INSERT_TAIL(&dev_device_list, dev, next);
+	struct rte_bus *bus;
+
+	RTE_VERIFY(dev && dev->bus);
+	bus = dev->bus;
+	TAILQ_INSERT_TAIL(&(bus->device_list), dev, next);
 }
 
 void rte_eal_device_remove(struct rte_device *dev)
 {
-	TAILQ_REMOVE(&dev_device_list, dev, next);
+	struct rte_bus *bus;
+
+	RTE_VERIFY(dev && dev->bus);
+	bus = dev->bus;
+	TAILQ_REMOVE(&(bus->device_list), dev, next);
 }
 
 int
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 971ad20..7a6d258 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -82,12 +82,13 @@
 
 #include "eal_private.h"
 
-struct pci_driver_list pci_driver_list =
-	TAILQ_HEAD_INITIALIZER(pci_driver_list);
-struct pci_device_list pci_device_list =
-	TAILQ_HEAD_INITIALIZER(pci_device_list);
-
 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
+#define PCI_BUS_NAME "pci_bus"
+
+static struct rte_bus_list pci_bus_list =
+	TAILQ_HEAD_INITIALIZER(pci_bus_list);
+
+struct rte_bus pci_bus;
 
 const char *pci_get_sysfs_path(void)
 {
@@ -160,64 +161,40 @@ static int
 rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	int ret;
-	const struct rte_pci_id *id_table;
-
-	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
-
-		/* check if device's identifiers match the driver's ones */
-		if (id_table->vendor_id != dev->id.vendor_id &&
-				id_table->vendor_id != PCI_ANY_ID)
-			continue;
-		if (id_table->device_id != dev->id.device_id &&
-				id_table->device_id != PCI_ANY_ID)
-			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
-			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
-			continue;
-		if (id_table->class_id != dev->id.class_id &&
-				id_table->class_id != RTE_CLASS_ANY_ID)
-			continue;
-
-		struct rte_pci_addr *loc = &dev->addr;
-
-		RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid, loc->function,
-				dev->device.numa_node);
-
-		/* no initialization when blacklisted, return without error */
-		if (dev->device.devargs != NULL &&
-			dev->device.devargs->type ==
-				RTE_DEVTYPE_BLACKLISTED_PCI) {
-			RTE_LOG(INFO, EAL, "  Device is blacklisted, not initializing\n");
-			return 1;
-		}
-
-		RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->driver.name);
+	struct rte_pci_addr *loc = &dev->addr;
+
+	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid, loc->function,
+			dev->device.numa_node);
+
+	/* no initialization when blacklisted, return without error */
+	if (dev->device.devargs != NULL &&
+		dev->device.devargs->type ==
+			RTE_DEVTYPE_BLACKLISTED_PCI) {
+		RTE_LOG(INFO, EAL, "  Device is blacklisted, not initializing\n");
+		return 1;
+	}
 
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
-			/* map resources for devices that use igb_uio */
-			ret = rte_eal_pci_map_device(dev);
-			if (ret != 0)
-				return ret;
-		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
-				rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			/* unbind current driver */
-			if (pci_unbind_kernel_driver(dev) < 0)
-				return -1;
-		}
+	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
+			dev->id.device_id, dr->driver.name);
+
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+		/* map resources for devices that use igb_uio */
+		ret = rte_eal_pci_map_device(dev);
+		if (ret != 0)
+			return ret;
+	} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
+			rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		/* unbind current driver */
+		if (pci_unbind_kernel_driver(dev) < 0)
+			return -1;
+	}
 
-		/* reference driver structure */
-		dev->driver = dr;
+	/* reference driver structure */
+	dev->driver = dr;
 
-		/* call the driver probe() function */
-		return dr->probe(dr, dev);
-	}
-	/* return positive value if driver doesn't support this device */
-	return 1;
+	/* call the driver probe() function */
+	return dr->probe(dr, dev);
 }
 
 /*
@@ -284,6 +261,7 @@ static int
 pci_probe_all_drivers(struct rte_pci_device *dev)
 {
 	struct rte_pci_driver *dr = NULL;
+	struct rte_bus *pci_bus;
 	int rc = 0;
 
 	if (dev == NULL)
@@ -293,15 +271,21 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	if (dev->driver != NULL)
 		return 0;
 
-	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		rc = rte_eal_pci_probe_one_driver(dr, dev);
-		if (rc < 0)
-			/* negative value is an error */
-			return -1;
-		if (rc > 0)
-			/* positive value means driver doesn't support it */
-			continue;
-		return 0;
+	TAILQ_FOREACH(dr, &(pci_bus->driver_list), next) {
+		rc = pci_bus->match(dr, dev);
+		if (!rc) {
+			rc = rte_eal_pci_probe_one_driver(dr, dev);
+			if (rc < 0)
+				/* negative value is an error */
+				return -1;
+			if (rc > 0)
+				/* positive value means driver doesn't support
+				 * it
+				 */
+				continue;
+			return 0;
+		}
+		/* Else, continue to next driver */
 	}
 	return 1;
 }
@@ -314,13 +298,19 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 static int
 pci_detach_all_drivers(struct rte_pci_device *dev)
 {
-	struct rte_pci_driver *dr = NULL;
+	struct rte_pci_driver *pci_drv= NULL;
+	struct rte_driver *drv = NULL;
+	struct rte_bus *pci_bus;
 	int rc = 0;
 
 	if (dev == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dr, &pci_driver_list, next) {
+	RTE_VERIFY(dev->device->bus);
+	pci_bus = dev->device->bus;
+
+	TAILQ_FOREACH(drv, &pci_bus->driver_list, next) {
+		pci_drv = container_of(drv, struct rte_pci_driver, driver);
 		rc = rte_eal_pci_detach_dev(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
@@ -340,19 +330,28 @@ pci_detach_all_drivers(struct rte_pci_device *dev)
 int
 rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 {
-	struct rte_pci_device *dev = NULL;
+	struct rte_bus *pci_bus = NULL;
+	struct rte_device *dev = NULL;
+	struct rte_pci_device *pci_dev = NULL;
 	int ret = 0;
 
 	if (addr == NULL)
 		return -1;
 
+	pci_bus = rte_eal_get_bus(PCI_BUS_NAME);
+	if (!pci_bus) {
+		RTE_LOG(DEBUG, EAL, "Unable to find PCI bus\n");
+		return -1;
+	}
+
 	/* update current pci device in global list, kernel bindings might have
 	 * changed since last time we looked at it.
 	 */
 	if (pci_update_device(addr) < 0)
 		goto err_return;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	TAILQ_FOREACH(dev, &pci_bus->device_list, next) {
+		pci_dev = container_of(dev, struct rte_pci_device, device);
 		if (rte_eal_compare_pci_addr(&dev->addr, addr))
 			continue;
 
@@ -376,22 +375,31 @@ err_return:
 int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
-	struct rte_pci_device *dev = NULL;
+	struct rte_pci_device *pci_dev = NULL;
+	struct rte_bus *pci_bus = NULL;
+	struct rte_device *dev = NULL;
 	int ret = 0;
 
 	if (addr == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
-		if (rte_eal_compare_pci_addr(&dev->addr, addr))
+	pci_bus = rte_eal_get_bus(PCI_BUS_NAME);
+	if (!pci_bus) {
+		RTE_LOG(DEBUG, EAL, "Unable to find PCI bus\n");
+		return -1;
+	}
+
+	TAILQ_FOREACH(dev, &(bus->device_list), next) {
+		pci_dev = container_of(dev, struct rte_pci_device, device);
+		if (rte_eal_compare_pci_addr(&pci_dev->addr, addr))
 			continue;
 
-		ret = pci_detach_all_drivers(dev);
+		ret = pci_detach_all_drivers(pci_dev);
 		if (ret < 0)
 			goto err_return;
 
-		TAILQ_REMOVE(&pci_device_list, dev, next);
-		free(dev);
+		TAILQ_REMOVE(&pci_bus->device_list, dev, next);
+		free(pci_dev);
 		return 0;
 	}
 	return -1;
@@ -411,31 +419,42 @@ err_return:
 int
 rte_eal_pci_probe(void)
 {
-	struct rte_pci_device *dev = NULL;
+	struct rte_device *dev = NULL;
+	struct rte_bus *pci_bus;
+	struct rte_pci_device *pci_dev;
 	struct rte_devargs *devargs;
 	int probe_all = 0;
 	int ret = 0;
 
+	pci_bus = rte_eal_get_bus(PCI_BUS_NAME);
+	if (!pci_bus) {
+		RTE_LOG(INFO, EAL, "(%s): No such bus exists\n", PCI_BUS_NAME);
+		/* Cannot continue ahead and this is not an error */
+		return;
+	}
+
 	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
 		probe_all = 1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	TAILQ_FOREACH(dev, &(pci_bus->device_list), next) {
+		pci_dev = container_of(dev, struct rte_pci_device, device);
 
 		/* set devargs in PCI structure */
-		devargs = pci_devargs_lookup(dev);
+		devargs = pci_devargs_lookup(pci_dev);
 		if (devargs != NULL)
-			dev->device.devargs = devargs;
+			pci_dev->device.devargs = devargs;
 
 		/* probe all or only whitelisted devices */
 		if (probe_all)
-			ret = pci_probe_all_drivers(dev);
+			ret = pci_probe_all_drivers(pci_dev);`
 		else if (devargs != NULL &&
 			devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
-			ret = pci_probe_all_drivers(dev);
+			ret = pci_probe_all_drivers(pci_dev);
 		if (ret < 0)
 			rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
-				 " cannot be used\n", dev->addr.domain, dev->addr.bus,
-				 dev->addr.devid, dev->addr.function);
+				 " cannot be used\n", pci_dev->addr.domain,
+				 pci_dev->addr.bus, pci_dev->addr.devid,
+				 pci_dev->addr.function);
 	}
 
 	return 0;
@@ -465,10 +484,19 @@ pci_dump_one_device(FILE *f, struct rte_pci_device *dev)
 void
 rte_eal_pci_dump(FILE *f)
 {
-	struct rte_pci_device *dev = NULL;
+	struct rte_device *dev = NULL;
+	struct rte_pci_device *pci_dev = NULL;
+	struct rte_bus *bus;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
-		pci_dump_one_device(f, dev);
+	bus = rte_eal_get_bus(PCI_BUS_NAME);
+	if (!bus) {
+		RTE_LOG(INFO, EAL, "(%s): No such bus exists\n", PCI_BUS_NAME);
+		return;
+	}
+
+	TAILQ_FOREACH(dev, &(bus->device_list), next) {
+		pci_dev = container_of(dev, struct rte_pci_device, device);
+		pci_dump_one_device(f, pci_dev);
 	}
 }
 
@@ -476,7 +504,16 @@ rte_eal_pci_dump(FILE *f)
 void
 rte_eal_pci_register(struct rte_pci_driver *driver)
 {
-	TAILQ_INSERT_TAIL(&pci_driver_list, driver, next);
+	struct rte_bus *pci_bus;
+
+	pci_bus = rte_eal_get_bus(PCI_BUS_NAME);
+	if (!pci_bus) {
+		RTE_LOG(INFO, EAL, "(%s) No such bus exists\n", PCI_BUS_NAME);
+		return;
+		/* TODO: How to return error from here? */
+	}
+
+	driver->driver->bus = pci_bus;
 	rte_eal_driver_register(&driver->driver);
 }
 
@@ -484,6 +521,9 @@ rte_eal_pci_register(struct rte_pci_driver *driver)
 void
 rte_eal_pci_unregister(struct rte_pci_driver *driver)
 {
-	rte_eal_driver_unregister(&driver->driver);
-	TAILQ_REMOVE(&pci_driver_list, driver, next);
+	struct rte_bus *pci_bus;
+
+	pci_bus = driver->driver->bus;
+	rte_eal_driver_unregister(&pci_bus, &driver->driver);
+	driver->driver->bus = NULL;
 }
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9ce8847..cffc449 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -110,7 +110,7 @@ const char *pci_get_sysfs_path(void);
 
 /** Maximum number of PCI resources. */
 #define PCI_MAX_RESOURCE 6
-
+/
 /**
  * A structure describing an ID for a PCI driver. Each driver provides a
  * table of these IDs for each device that it supports.
@@ -360,15 +360,6 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
 }
 
 /**
- * Scan the content of the PCI bus, and the devices in the devices
- * list
- *
- * @return
- *  0 on success, negative on error
- */
-int rte_eal_pci_scan(void);
-
-/**
  * Probe the PCI bus for registered drivers.
  *
  * Scan the content of the PCI bus, and call the probe() function for
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox