DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] doc: introduce PVP reference benchmark
From: Maxime Coquelin @ 2016-12-20 14:54 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Mcnamara, John, yuanhan.liu@linux.intel.com, Yang, Zhiyong,
	ktraynor@redhat.com, dev
In-Reply-To: <15125480.nLCo42mjm8@xps13>



On 12/20/2016 11:03 AM, Thomas Monjalon wrote:
>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>
>>>> There is one trailing whitespace warning but apart from that:
>>>>
>>>> Acked-by: John McNamara <john.mcnamara@intel.com>
>>>
>>> Thanks John,
>>>
>>> Do you want me to send a v3, fixing the trailing whitespace & collecting
>>> the acks?
>>
>> No need (unless the tree maintainer says otherwise).
>> It was one trailing whitespace. Just something to look out for in future.
>
> Removed when applying.
> Fixed some heading issues also:
>
> --- a/doc/guides/howto/pvp_reference_benchmark.rst
> +++ b/doc/guides/howto/pvp_reference_benchmark.rst
> @@ -160,7 +160,7 @@ Testpmd launch
>            --portmask=f --disable-hw-vlan -i --rxq=1 --txq=1
>            --nb-cores=4 --forward-mode=io
>
> -With this command, isolated CPUs 2 to 5 will be used as lcores for PMD threads.
> +   With this command, isolated CPUs 2 to 5 will be used as lcores for PMD threads.
>
>  #. In testpmd interactive mode, set the portlist to obtain the correct port
>     chaining:
> @@ -176,7 +176,8 @@ VM launch
>
>  The VM may be launched either by calling QEMU directly, or by using libvirt.
>
> -#. Qemu way:
> +Qemu way
> +^^^^^^^^
>
>  Launch QEMU with two Virtio-net devices paired to the vhost-user sockets
>  created by testpmd. Below example uses default Virtio-net options, but options
> @@ -210,7 +211,8 @@ where isolated CPUs 6 and 7 will be used as lcores for Virtio PMDs:
>        export PYTHONPATH=$PYTHONPATH:<QEMU path>/scripts/qmp
>        ./qmp-vcpu-pin -s /tmp/qmp.socket 1 6 7
>
> -#. Libvirt way:
> +Libvirt way
> +^^^^^^^^^^^
>
>  Some initial steps are required for libvirt to be able to connect to testpmd's
>  sockets.

Thanks Thomas for handling these fixes.

Maxime

^ permalink raw reply

* Re: [PATCH] net/af_packet: initialize link interrupt callback queue
From: Ferruh Yigit @ 2016-12-20 14:20 UTC (permalink / raw)
  To: Chas Williams, dev; +Cc: John W. Linville
In-Reply-To: <1481997835-23288-1-git-send-email-3chas3@gmail.com>

On 12/17/2016 6:03 PM, Chas Williams wrote:
> This patch initializes the eth_dev->link_intr_cbs queue which is
> used when af_packet is passed into rte_eth_ev_callback_register().

Why do you want to register callback to af_packet PMD, it won't be
calling them?

> 
> Fixes: 4dc294158cac ("ethdev: support optional Rx and Tx callbacks")
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>

Please cc the maintainers...

CC: John W. Linville <linville@tuxdriver.com>

> ---
>  drivers/net/af_packet/rte_eth_af_packet.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index a1e13ff..ea5070a 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -708,6 +708,7 @@ rte_pmd_init_internals(const char *name,
>  	(*eth_dev)->data->drv_name = pmd_af_packet_drv.driver.name;
>  	(*eth_dev)->data->kdrv = RTE_KDRV_NONE;
>  	(*eth_dev)->data->numa_node = numa_node;
> +	TAILQ_INIT(&((*eth_dev)->link_intr_cbs));
>  
>  	return 0;
>  
> 

^ permalink raw reply

* Re: [PATCH 1/3] ethdev: New API to free consumed buffers in TX ring
From: Billy McFall @ 2016-12-20 14:15 UTC (permalink / raw)
  To: Adrien Mazarguil
  Cc: Ananyev, Konstantin, thomas.monjalon@6wind.com, Lu, Wenzhuo,
	dev@dpdk.org, Stephen Hemminger
In-Reply-To: <20161220125823.GU10340@6wind.com>

Thank you for your responses, see inline.

On Tue, Dec 20, 2016 at 7:58 AM, Adrien Mazarguil
<adrien.mazarguil@6wind.com> wrote:
> On Tue, Dec 20, 2016 at 12:17:10PM +0000, Ananyev, Konstantin wrote:
>>
>>
>> > -----Original Message-----
>> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
>> > Sent: Tuesday, December 20, 2016 11:28 AM
>> > To: Billy McFall <bmcfall@redhat.com>
>> > Cc: thomas.monjalon@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org; Stephen Hemminger
>> > <stephen@networkplumber.org>
>> > Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: New API to free consumed buffers in TX ring
>> >
>> > Hi Billy,
>> >
>> > On Fri, Dec 16, 2016 at 07:48:49AM -0500, Billy McFall wrote:
>> > > Add a new API to force free consumed buffers on TX ring. API will return
>> > > the number of packets freed (0-n) or error code if feature not supported
>> > > (-ENOTSUP) or input invalid (-ENODEV).
>> > >
>> > > Because rte_eth_tx_buffer() may be used, and mbufs may still be held
>> > > in local buffer, the API also accepts *buffer and *sent. Before
>> > > attempting to free, rte_eth_tx_buffer_flush() is called to make sure
>> > > all mbufs are sent to Tx ring. rte_eth_tx_buffer_flush() is called even
>> > > if threshold is not met.
>> > >
>> > > Signed-off-by: Billy McFall <bmcfall@redhat.com>
>> > > ---
>> > >  lib/librte_ether/rte_ethdev.h | 56 +++++++++++++++++++++++++++++++++++++++++++
>> > >  1 file changed, 56 insertions(+)
>> > >
>> > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>> > > index 9678179..e3f2be4 100644
>> > > --- a/lib/librte_ether/rte_ethdev.h
>> > > +++ b/lib/librte_ether/rte_ethdev.h
>> > > @@ -1150,6 +1150,9 @@ 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 int (*eth_tx_done_cleanup_t)(void *txq, uint32_t free_cnt);
>> > > +/**< @internal Force mbufs to be from TX ring. */
>> > > +
>> > >  typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
>> > >   uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
>> > >
>> > > @@ -1467,6 +1470,7 @@ struct eth_dev_ops {
>> > >   eth_rx_disable_intr_t      rx_queue_intr_disable;
>> > >   eth_tx_queue_setup_t       tx_queue_setup;/**< Set up device TX queue.*/
>> > >   eth_queue_release_t        tx_queue_release;/**< Release TX queue.*/
>> > > + eth_tx_done_cleanup_t      tx_done_cleanup;/**< Free tx ring mbufs */
>> > >   eth_dev_led_on_t           dev_led_on;    /**< Turn on LED. */
>> > >   eth_dev_led_off_t          dev_led_off;   /**< Turn off LED. */
>> > >   flow_ctrl_get_t            flow_ctrl_get; /**< Get flow control. */
>> > > @@ -2943,6 +2947,58 @@ rte_eth_tx_buffer(uint8_t port_id, uint16_t queue_id,
>> > >  }
>> > >
>> > >  /**
>> > > + * Request the driver to free mbufs currently cached by the driver. The
>> > > + * driver will only free the mbuf if it is no longer in use.
>> > > + *
>> > > + * @param port_id
>> > > + *   The port identifier of the Ethernet device.
>> > > + * @param queue_id
>> > > + *   The index of the transmit queue through which output packets must be
>> > > + *   sent.
>> > > + *   The value must be in the range [0, nb_tx_queue - 1] previously supplied
>> > > + *   to rte_eth_dev_configure().
>> > > + * @param free_cnt
>> > > + *   Maximum number of packets to free. Use 0 to indicate all possible packets
>> > > + *   should be freed. Note that a packet may be using multiple mbufs.
>> > > + * @param buffer
>> > > + *   Buffer used to collect packets to be sent. If provided, the buffer will
>> > > + *   be flushed, even if the current length is less than buffer->size. Pass NULL
>> > > + *   if buffer has already been flushed.
>> > > + * @param sent
>> > > + *   Pointer to return number of packets sent if buffer has packets to be sent.
>> > > + *   If *buffer is supplied, *sent must also be supplied.
>> > > + * @return
>> > > + *   Failure: < 0
>> > > + *     -ENODEV: Invalid interface
>> > > + *     -ENOTSUP: Driver does not support function
>> > > + *   Success: >= 0
>> > > + *     0-n: Number of packets freed. More packets may still remain in ring that
>> > > + *     are in use.
>> > > + */
>> > > +
>> > > +static inline int
>> > > +rte_eth_tx_done_cleanup(uint8_t port_id, uint16_t queue_id,  uint32_t free_cnt,
>> > > +         struct rte_eth_dev_tx_buffer *buffer, uint16_t *sent)
>> > > +{
>> > > + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>> > > +
>> > > + /* Validate Input Data. Bail if not valid or not supported. */
>> > > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> > > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_done_cleanup, -ENOTSUP);
>> > > +
>> > > + /*
>> > > +  * If transmit buffer is provided and there are still packets to be
>> > > +  * sent, then send them before attempting to free pending mbufs.
>> > > +  */
>> > > + if (buffer && sent)
>> > > +         *sent = rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>> > > +
>> > > + /* Call driver to free pending mbufs. */
>> > > + return (*dev->dev_ops->tx_done_cleanup)(dev->data->tx_queues[queue_id],
>> > > +                 free_cnt);
>> > > +}
>> > > +
>> > > +/**
>> > >   * Configure a callback for buffered packets which cannot be sent
>> > >   *
>> > >   * Register a specific callback to be called when an attempt is made to send
>> >

I will remove the buffer/sent parameters. It will be the applications
responsibility
to make sure rte_eth_tx_buffer_flush() is called.

I don't feel strongly about the free_cnt parameter. It was in the
original request
so that if there was a large ring buffer, the API could bail early
without having
to go through all the entire ring. It might be a little unrealistic
for the application
to truly know how many mbufs it wants freed. Also, as an example, the I40e
driver already has a i40e_tx_free_bufs(...) function, so by dropping
the free_cnt
parameter, this function could be reused without having to account for
the free_cnt.

>> > Just a thought to follow-up on Stephen's comment to further simplify this
>> > API, how about not adding any new eth_dev_ops but instead defining what
>> > should happen during an empty TX burst call (tx_burst() with 0 packets).
>> >

In the original API request thread, see dpdk-dev mailing list from 11/21/2016
with subject "Adding API to force freeing consumed buffers in TX ring",
overloading the existing API with nb_pkts == 0 was suggested and consensus
was to go with new API. I lean towards a new API since this is a special case
most applications won't use, but I will go with the community on whether to
enhance the existing burst functionality or add a new API.

>> > Several PMDs already have a check for this scenario and start by cleaning up
>> > completed packets anyway, they effectively partially implement this
>> > definition for free already.
>>
>> Many PMDs  start by cleaning up only when number of free entries
>> drop below some point.

True, but the original request for this API was for the scenario where packets
are being flooded and the application wanted to reuse mbuf to avoid a packet
copy. So the API was to request the driver to free "done" mbufs outside of any
threshold.

>> Also in that case the author would have to modify (and test) all existing TX routinies.
>> So I think a separate API call seems more plausible.
>
> Not necessarily, as I understand this API in its current form only suggests
> that a PMD should release a few mbufs from a queue if possible, without any
> guarantee, PMDs are not forced to comply.
>
> I think the threshold you mention is a valid reason not to release them, and
> it wouldn't change a thing to existing tx_burst() implementations in the
> meantime (only documentation).
>
> This threshold could also be bypassed rather painlessly in the
> "if (unlikely(nb_pkts == 0))" case that all PMDs already check for in a
> way or another.
>
>> Though I am agree with previous comment from Stephen that last two parameters
>> are redundant and would just overcomplicate things.
>> tin
>>
>> >
>> > The main difference with this API would be that you wouldn't know how many
>> > mbufs were freed and wouldn't collect them into an array. However most
>> > applications have one mbuf pool and/or know where they come from, so they
>> > can just query the pool or attempt to re-allocate from it after doing empty
>> > bursts in case of starvation.
>> >
>> > [1] http://dpdk.org/ml/archives/dev/2016-December/052469.html
>
> --
> Adrien Mazarguil
> 6WIND

^ permalink raw reply

* [PATCH] nfp: add support for new metadata api
From: Alejandro Lucero @ 2016-12-20 14:13 UTC (permalink / raw)
  To: dev

NFP is a smart programmable NIC and firmware is deployed for specific
system needs, like offloading OVS, vRouter, contrack or eBPF into the
hardware. This often requires to give metadata to the host within
packets delivered. Last NFP firmware implementations support richer
metadata api facilitating interaction between firmware and host code.

Old way of handling metadata needs to be still there for supporting
old firmware.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c      | 33 +++++++++++++++++++++++++++------
 drivers/net/nfp/nfp_net_ctrl.h |  6 ++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index cee8f63..69ae6d5 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1726,6 +1726,7 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 
 #define NFP_HASH_OFFSET      ((uint8_t *)mbuf->buf_addr + mbuf->data_off - 4)
 #define NFP_HASH_TYPE_OFFSET ((uint8_t *)mbuf->buf_addr + mbuf->data_off - 8)
+#define NFP_DESC_META_LEN(d) (d->rxd.meta_len_dd & PCIE_DESC_RX_META_LEN_MASK)
 
 /*
  * nfp_net_set_hash - Set mbuf hash data
@@ -1739,16 +1740,38 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 {
 	uint32_t hash;
 	uint32_t hash_type;
+	uint32_t meta_info;
+	uint8_t *meta_offset;
 	struct nfp_net_hw *hw = rxq->hw;
 
 	if (!(hw->ctrl & NFP_NET_CFG_CTRL_RSS))
 		return;
 
-	if (!(rxd->rxd.flags & PCIE_DESC_RX_RSS))
+	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) <= 3) {
+		if (!(rxd->rxd.flags & PCIE_DESC_RX_RSS))
+			return;
+
+		hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
+		hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
+
+	} else if (NFP_DESC_META_LEN(rxd)) {
+		meta_offset = (uint8_t *)mbuf->buf_addr;
+		meta_info = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
+		meta_offset += 4;
+		/* NFP PMD just supports metadata for hashing */
+		switch (meta_info & NFP_NET_META_FIELD_MASK) {
+		case NFP_NET_META_HASH:
+			meta_info >>= NFP_NET_META_FIELD_SIZE;
+			hash = rte_be_to_cpu_32(*(uint32_t *)meta_offset);
+			hash_type = meta_info && NFP_NET_META_FIELD_MASK;
+			break;
+		default:
+			/* Unsupported metadata can be a performance issue */
+			return;
+		}
+	} else {
 		return;
-
-	hash = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_OFFSET);
-	hash_type = rte_be_to_cpu_32(*(uint32_t *)NFP_HASH_TYPE_OFFSET);
+	}
 
 	mbuf->hash.rss = hash;
 	mbuf->ol_flags |= PKT_RX_RSS_HASH;
@@ -1774,8 +1797,6 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
 }
 
-#define NFP_DESC_META_LEN(d) (d->rxd.meta_len_dd & PCIE_DESC_RX_META_LEN_MASK)
-
 /*
  * RX path design:
  *
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index 2c50043..281205d 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -52,6 +52,12 @@
 /* Offset in Freelist buffer where packet starts on RX */
 #define NFP_NET_RX_OFFSET               32
 
+/* Prepend field types */
+#define NFP_NET_META_FIELD_SIZE         4
+#define NFP_NET_META_HASH               1 /* next field carries hash type */
+#define NFP_NET_META_MARK               2
+#define NFP_NET_META_FIELD_MASK         (0xf)
+
 /* Hash type pre-pended when a RSS hash was computed */
 #define NFP_NET_RSS_NONE                0
 #define NFP_NET_RSS_IPV4                1
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] doc: Fix a typo in testpmd application user guide.
From: Thomas Monjalon @ 2016-12-20 13:55 UTC (permalink / raw)
  To: Rosen, Rami; +Cc: dev, Mcnamara, John
In-Reply-To: <B27915DBBA3421428155699D51E4CFE2026869FD@IRSMSX103.ger.corp.intel.com>

> > This patch fixes a trivial typo in testpmd application user guide.
> > 
> > Signed-off-by: Rami Rosen <rami.rosen@intel.com>
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>

Applied, thanks

^ permalink raw reply

* Re: [PATCH v3 02/12] eal/bus: introduce bus abstraction
From: Shreyansh Jain @ 2016-12-20 13:51 UTC (permalink / raw)
  To: Jan Blunck
  Cc: dev@dpdk.org, David Marchand, Thomas Monjalon, Ferruh Yigit,
	jianbo.liu@linaro.org
In-Reply-To: <CALe+Z00LYrfzQE1916LoOqytcFjJ1xRXFL-wLvX03eham11Vvw@mail.gmail.com>

> -----Original Message-----
> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck
> Sent: Tuesday, December 20, 2016 6:47 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org; David Marchand <david.marchand@6wind.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>; Ferruh Yigit <ferruh.yigit@intel.com>;
> jianbo.liu@linaro.org
> Subject: Re: [dpdk-dev] [PATCH v3 02/12] eal/bus: introduce bus abstraction
> 
> On Fri, Dec 16, 2016 at 2:10 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> wrote:
> > This patch introduces the rte_bus abstraction for devices and drivers in
> > EAL framework. The model is:
> >  - One or more buses are connected to a CPU (or core)
> >  - One or more devices are conneted to a Bus
> >  - Drivers are running instances which manage one or more devices
> >  - Bus is responsible for identifying devices (and interrupt propogation)
> >  - Driver is responsible for initializing the device
> >
> > This patch adds a 'rte_bus' class which rte_driver and rte_device refer.
> > This way, each device (rte_xxx_device) would have reference to the bus
> > it is based on. As well as, each driver (rte_xxx_driver) would have link
> > to the bus and devices on it for servicing.
> >
> >                                   __ rte_bus_list
> >                                  /
> >                      +----------'---+
> >                      |rte_bus       |
> >                      | driver_list------> List of rte_bus specific
> >                      | device_list----    devices
> >                      |              | `-> List of rte_bus associated
> >                      |              |     drivers
> >                      +--|------|----+
> >               _________/        \_________
> >     +--------/----+                     +-\---------------+
> >     |rte_device   |                     |rte_driver       |
> >     | rte_bus     |                     | rte_bus         |
> >     | rte_driver  |                     | ...             |
> >     | ...         |                     +---------...-----+
> >     |             |                               |||
> >     +---||--------+                               |||
> >         ||                                        |||
> >         | \                                        \\\
> >         |  \_____________                           \\\
> >         |                \                          |||
> >  +------|---------+ +----|----------+               |||
> >  |rte_pci_device  | |rte_xxx_device |               |||
> >  | ....           | | ....          |               |||
> >  +----------------+ +---------------+              / | \
> >                                                   /  |  \
> >                             _____________________/  /    \
> >                            /                    ___/      \
> >             +-------------'--+    +------------'---+    +--'------------+
> >             |rte_pci_driver  |    |rte_vdev_driver |    |rte_xxx_driver |
> >             | ....           |    | ....           |    | ....          |
> >             +----------------+    +----------------+    +---------------+
> >
> > This patch only enables the bus references on rte_driver and rte_driver.
> > EAL wide global device and driver list continue to exist until an instance
> > of bus is added in subsequent patches.
> >
> > This patch also introduces RTE_REGISTER_BUS macro on the lines of
> > RTE_PMD_REGISTER_XXX. Key difference is that the constructor priority has
> > been explicitly set to 101 so as to execute bus registration before PMD.
> >
> > Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >
> > --
> > v2:
> >  - fix bsdapp compilation issue because of missing export symbols in map
> >    file
> > ---
> >  lib/librte_eal/bsdapp/eal/Makefile              |   1 +
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  15 ++
> >  lib/librte_eal/common/Makefile                  |   2 +-
> >  lib/librte_eal/common/eal_common_bus.c          | 192
> ++++++++++++++++++++++++
> >  lib/librte_eal/common/include/rte_bus.h         | 174
> +++++++++++++++++++++
> >  lib/librte_eal/common/include/rte_dev.h         |   2 +
> >  lib/librte_eal/linuxapp/eal/Makefile            |   1 +
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  15 ++
> >  8 files changed, 401 insertions(+), 1 deletion(-)
> >  create mode 100644 lib/librte_eal/common/eal_common_bus.c
> >  create mode 100644 lib/librte_eal/common/include/rte_bus.h
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/Makefile
> b/lib/librte_eal/bsdapp/eal/Makefile
> > index a15b762..cce99f7 100644
> > --- a/lib/librte_eal/bsdapp/eal/Makefile
> > +++ b/lib/librte_eal/bsdapp/eal/Makefile
> > @@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) +=
> eal_common_cpuflags.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c
> > +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_thread.c
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index 2f81f7c..23fc1c1 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -174,3 +174,18 @@ DPDK_16.11 {
> >         rte_eal_vdrv_unregister;
> >
> >  } DPDK_16.07;
> > +
> > +DPDK_17.02 {
> > +       global:
> > +
> > +       rte_bus_list;
> > +       rte_eal_bus_add_device;
> > +       rte_eal_bus_add_driver;
> > +       rte_eal_get_bus;
> > +       rte_eal_bus_dump;
> > +       rte_eal_bus_register;
> > +       rte_eal_bus_remove_device;
> > +       rte_eal_bus_remove_driver;
> > +       rte_eal_bus_unregister;
> > +
> > +} DPDK_16.11;
> > diff --git a/lib/librte_eal/common/Makefile
> b/lib/librte_eal/common/Makefile
> > index a92c984..0c39414 100644
> > --- a/lib/librte_eal/common/Makefile
> > +++ b/lib/librte_eal/common/Makefile
> > @@ -38,7 +38,7 @@ INC += rte_per_lcore.h rte_random.h
> >  INC += rte_tailq.h rte_interrupts.h rte_alarm.h
> >  INC += rte_string_fns.h rte_version.h
> >  INC += rte_eal_memconfig.h rte_malloc_heap.h
> > -INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
> > +INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_vdev.h
> >  INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
> >  INC += rte_malloc.h rte_keepalive.h rte_time.h
> >
> > 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..612f64e
> > --- /dev/null
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -0,0 +1,192 @@
> > +/*-
> > + *   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_bus.h>
> > +#include <rte_devargs.h>
> > +#include <rte_debug.h>
> > +
> > +#include "eal_private.h"
> > +
> > +struct rte_bus_list rte_bus_list =
> > +       TAILQ_HEAD_INITIALIZER(rte_bus_list);
> > +
> > +/** @internal
> > + * Add a device to a bus.
> > + */
> > +void
> > +rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
> > +{
> > +       RTE_VERIFY(bus);
> > +       RTE_VERIFY(dev);
> > +
> > +       TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
> > +       dev->bus = bus;
> > +}
> > +
> > +/** @internal
> > + * Remove a device from its bus.
> > + */
> > +void
> > +rte_eal_bus_remove_device(struct rte_device *dev)
> > +{
> > +       struct rte_bus *bus;
> > +
> > +       RTE_VERIFY(dev);
> > +       RTE_VERIFY(dev->bus);
> > +
> > +       bus = dev->bus;
> > +       TAILQ_REMOVE(&bus->device_list, dev, next);
> > +       dev->bus = NULL;
> > +}
> > +
> > +/** @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);
> > +       drv->bus = bus;
> > +}
> > +
> > +/** @internal
> > + * Disassociate a driver from bus.
> > + */
> > +void
> > +rte_eal_bus_remove_driver(struct rte_driver *drv)
> > +{
> > +       struct rte_bus *bus;
> > +
> > +       RTE_VERIFY(drv);
> > +       RTE_VERIFY(drv->bus);
> > +
> > +       bus = drv->bus;
> > +       TAILQ_REMOVE(&bus->driver_list, drv, next);
> > +       drv->bus = NULL;
> > +}
> > +
> > +/**
> > + * 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 (!strcmp(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)
> > +{
> > +       RTE_VERIFY(bus);
> > +       RTE_VERIFY(bus->name && strlen(bus->name));
> > +
> > +       /* Initialize the driver and device list associated with the bus */
> > +       TAILQ_INIT(&(bus->driver_list));
> > +       TAILQ_INIT(&(bus->device_list));
> > +
> > +       TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
> > +       RTE_LOG(INFO, 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)));
> > +
> > +       /* TODO: For each device, call its rte_device->driver->remove()
> > +        * and rte_eal_bus_remove_driver()
> > +        */
> > +
> > +       TAILQ_REMOVE(&rte_bus_list, bus, next);
> > +       RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
> > +}
> > +
> > +/* dump one bus info */
> > +static int
> > +bus_dump_one(FILE *f, struct rte_bus *bus)
> > +{
> > +       int ret;
> > +
> > +       /* For now, dump only the bus name */
> > +       ret = fprintf(f, " %s\n", bus->name);
> > +
> > +       /* Error in case of inability in writing to stream */
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       return 0;
> > +}
> > +
> > +void
> > +rte_eal_bus_dump(FILE *f)
> > +{
> > +       int ret;
> > +       struct rte_bus *bus;
> > +
> > +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> > +               ret = bus_dump_one(f, bus);
> > +               if (ret) {
> > +                       RTE_LOG(ERR, EAL, "Unable to write to stream
> (%d)\n",
> > +                               ret);
> > +                       break;
> > +               }
> > +       }
> > +}
> > 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..f0297a9
> > --- /dev/null
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -0,0 +1,174 @@
> > +/*-
> > + *   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);
> > +
> > +/* Global Bus list */
> > +extern struct rte_bus_list rte_bus_list;
> > +
> > +struct rte_bus {
> > +       TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
> > +       struct rte_driver_list driver_list;
> > +                                    /**< List of all drivers on bus */
> > +       struct rte_device_list device_list;
> > +                                    /**< List of all devices on bus */
> > +       const char *name;            /**< Name of the 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);
> > +
> > +/** @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);
> > +
> > +/** @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);
> > +
> > +/**
> > + * Dump information of all the buses registered with EAL.
> > + *
> > + * @param f
> > + *     A valid and open output stream handle
> > + *
> > + * @return
> > + *      0 in case of success
> > + *     !0 in case there is error in opening the output stream
> > + */
> > +void rte_eal_bus_dump(FILE *f);
> > +
> > +/** Helper for Bus registration. The constructor has higher priority than
> > + * PMD constructors
> > + */
> > +#define RTE_REGISTER_BUS(nm, bus) \
> > +static void __attribute__((constructor(101), used)) 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..4004f9a 100644
> > --- a/lib/librte_eal/common/include/rte_dev.h
> > +++ b/lib/librte_eal/common/include/rte_dev.h
> > @@ -122,6 +122,7 @@ struct rte_driver;
> >   */
> >  struct rte_device {
> >         TAILQ_ENTRY(rte_device) next; /**< Next device */
> > +       struct rte_bus *bus;          /**< Device connected to this bus */
> 
> Is there a reason why this isn't const?

No reason. Miss from my end. I will fix this.
(I also took hint from Stephen's recent patches) 

> 
> 
> >         struct rte_driver *driver;    /**< Associated driver */
> >         int numa_node;                /**< NUMA node connection */
> >         struct rte_devargs *devargs;  /**< Device user arguments */
> > @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
> >   */
> >  struct rte_driver {
> >         TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> > +       struct rte_bus *bus;           /**< Bus serviced by this driver */
> 
> Same thing here.

I will send out v4 with this. Thanks for comments.

> 
> >         const char *name;                   /**< Driver name. */
> >         const char *alias;              /**< Driver alias. */
> >  };
> > diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> b/lib/librte_eal/linuxapp/eal/Makefile
> > index 4e206f0..aa874a5 100644
> > --- a/lib/librte_eal/linuxapp/eal/Makefile
> > +++ b/lib/librte_eal/linuxapp/eal/Makefile
> > @@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) +=
> eal_common_cpuflags.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c
> > +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
> >  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_thread.c
> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > index 83721ba..c873a7f 100644
> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -178,3 +178,18 @@ DPDK_16.11 {
> >         rte_eal_vdrv_unregister;
> >
> >  } DPDK_16.07;
> > +
> > +DPDK_17.02 {
> > +       global:
> > +
> > +       rte_bus_list;
> > +       rte_eal_bus_add_device;
> > +       rte_eal_bus_add_driver;
> > +       rte_eal_get_bus;
> > +       rte_eal_bus_dump;
> > +       rte_eal_bus_register;
> > +       rte_eal_bus_remove_device;
> > +       rte_eal_bus_remove_driver;
> > +       rte_eal_bus_unregister;
> > +
> > +} DPDK_16.11;
> > --
> > 2.7.4
> >

^ permalink raw reply

* Re: [PATCH v2] nfp: extend speed capabilities advertised
From: Ferruh Yigit @ 2016-12-20 13:44 UTC (permalink / raw)
  To: Alejandro Lucero, dev
In-Reply-To: <1482238365-28082-1-git-send-email-alejandro.lucero@netronome.com>

On 12/20/2016 12:52 PM, Alejandro Lucero wrote:
> NFP supports more speeds than just 40 and 100GB, which were
> what was advertised before.
> 
> v2: add feature to nfp.ini
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> ---

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply

* Re: [PATCH v6] nfp: report link speed using hardware info
From: Ferruh Yigit @ 2016-12-20 13:44 UTC (permalink / raw)
  To: Alejandro Lucero, dev
In-Reply-To: <1482240120-28274-1-git-send-email-alejandro.lucero@netronome.com>

On 12/20/2016 1:22 PM, Alejandro Lucero wrote:
> Previous reported speed was hardcoded because there was not firmware
> support for getting this information. This change needs also to support
> old firmware versions, but instead of the previous hardcoded report, no
> speed is reported to the user avoiding to give the wrong speed when link
> is not configured to 40G.
> 
> v6: avoid to report speed with old firmware
> v5: Fix missing parenthesis
> v4: Make conditional simple and more ellaborated commit comment.
> v3: remove unsed macro
> v2: use RTE_DIM instead of own macro
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Applied to dpdk-next-net/master, thanks. (replaced v5)

^ permalink raw reply

* Re: [PATCH] doc: fix environment variable typo
From: Thomas Monjalon @ 2016-12-20 13:42 UTC (permalink / raw)
  To: Baruch Siach; +Cc: dev, Remy Horton, John McNamara
In-Reply-To: <4db72670-0a8f-ac0a-b7bf-5671487630a9@intel.com>

> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >  doc/guides/sample_app_ug/ethtool.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Missing fixline
> 
> Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample 
> application")
> 
> Otherwise, well spotted.. :)
> 
> Acked-by: Remy Horton <remy.horton@intel.com>

Applied, thanks

^ permalink raw reply

* Re: [PATCH v5 29/29] net/i40e: set/clear VF stats from PF
From: Iremonger, Bernard @ 2016-12-20 13:40 UTC (permalink / raw)
  To: Yigit, Ferruh, dev@dpdk.org
  Cc: Wu, Jingjing, Zhang, Helin, Zhang, Qi Z, Lu, Wenzhuo,
	Chen, Jing D
In-Reply-To: <587a2f08-0541-3f99-d16f-1c14206f11e9@intel.com>

Hi Ferruh,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Tuesday, December 20, 2016 1:25 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Chen, Jing D <jing.d.chen@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v5 29/29] net/i40e: set/clear VF stats from
> PF
> 
> On 12/16/2016 7:02 PM, Ferruh Yigit wrote:
> > From: Qi Zhang <qi.z.zhang@intel.com>
> >
> > This patch add support to get/clear VF statistics from PF side.
> > Two APIs are added:
> > rte_pmd_i40e_get_vf_stats.
> > rte_pmd_i40e_reset_vf_stats.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> 
> <...>
> 
> > diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map
> > b/drivers/net/i40e/rte_pmd_i40e_version.map
> > index 8ac1bc8..7a5d211 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e_version.map
> > +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
> > @@ -6,7 +6,9 @@ DPDK_2.0 {
> >  DPDK_17.02 {
> >  	global:
> >
> > +	rte_pmd_i40e_get_vf_stats;
> >  	rte_pmd_i40e_ping_vfs;
> > +	rte_pmd_i40e_reset_vf_stats;
> >  	rte_pmd_i40e_set_tx_loopback;
> >  	rte_pmd_i40e_set_vf_broadcast;
> >  	rte_pmd_i40e_set_vf_mac_addr;
> 
> Hi Wenzhuo, Mark,
> 
> I think this is the list of all APIs added with this patchset.
> 
> Just a question, what do you think following a logic in API naming as:
> <name_space>_<object>_<action> ?
> 
> So API names become:
> rte_pmd_i40e_tx_loopback_set;
> rte_pmd_i40e_vf_broadcast_set;
> rte_pmd_i40e_vf_mac_addr_set;
> rte_pmd_i40e_vfs_ping;
> rte_pmd_i40e_vf_stats_get;
> rte_pmd_i40e_vf_stats_reset;
> 
> 
> After above rename, rte_pmd_i40e_tx_loopback_set() is not giving a hint
> that this is something related to the PF controlling VF, perhaps we can
> rename the API ?
> 
> Also rte_pmd_i40e_vfs_ping() can become rte_pmd_i40e_vf_ping_all() to
> be more consistent about _vf_ usage.
> 
> Overall, they can be something like:
> rte_pmd_i40e_vf_broadcast_set;
> rte_pmd_i40e_vf_mac_addr_set;
> rte_pmd_i40e_vf_ping_all;
> rte_pmd_i40e_vf_stats_get;
> rte_pmd_i40e_vf_stats_reset;
> rte_pmd_i40e_vf_tx_loopback_set;
> 
> What do you think?
> 

I think the naming should be consistent with what has already been implemented for the ixgbe PMD.
	rte_pmd_ixgbe_set_all_queues_drop_en;
	rte_pmd_ixgbe_set_tx_loopback;
	rte_pmd_ixgbe_set_vf_mac_addr;
	rte_pmd_ixgbe_set_vf_mac_anti_spoof;
	rte_pmd_ixgbe_set_vf_split_drop_en;
	rte_pmd_ixgbe_set_vf_vlan_anti_spoof;
	rte_pmd_ixgbe_set_vf_vlan_insert;
	rte_pmd_ixgbe_set_vf_vlan_stripq;

	rte_pmd_ixgbe_set_vf_rate_limit;
	rte_pmd_ixgbe_set_vf_rx;
	rte_pmd_ixgbe_set_vf_rxmode;
	rte_pmd_ixgbe_set_vf_tx;
	rte_pmd_ixgbe_set_vf_vlan_filter;

Regards,

Bernard.

^ permalink raw reply

* Re: [PATCH v13 6/7] vmxnet3: add Tx preparation
From: Ferruh Yigit @ 2016-12-20 13:36 UTC (permalink / raw)
  To: Tomasz Kulasek, dev; +Cc: Ananyev, Konstantin
In-Reply-To: <1481650914-40324-7-git-send-email-tomaszx.kulasek@intel.com>

On 12/13/2016 5:41 PM, Tomasz Kulasek wrote:
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> 
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---

<...>

>  
>  uint16_t
> +vmxnet3_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> +	uint16_t nb_pkts)
> +{
<...>
> +
> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> +		ret = rte_validate_tx_offload(m);
> +		if (ret != 0) {
> +			rte_errno = ret;
> +			return i;
> +		}
> +#endif
> +		ret = rte_net_intel_cksum_prepare(m);

Since this API used beyond Intel drivers, what do you think renaming it?
rte_net_generic_cksum_prepare() ?

> +		if (ret != 0) {
> +			rte_errno = ret;
> +			return i;
> +		}
> +	}
> +
> +	return i;
> +}
<...>

^ permalink raw reply

* Re: [PATCH v5 29/29] net/i40e: set/clear VF stats from PF
From: Ferruh Yigit @ 2016-12-20 13:24 UTC (permalink / raw)
  To: dev; +Cc: Jingjing Wu, Helin Zhang, Qi Zhang, Wenzhuo Lu, Chen, Jing D
In-Reply-To: <20161216190257.6921-30-ferruh.yigit@intel.com>

On 12/16/2016 7:02 PM, Ferruh Yigit wrote:
> From: Qi Zhang <qi.z.zhang@intel.com>
> 
> This patch add support to get/clear VF statistics
> from PF side.
> Two APIs are added:
> rte_pmd_i40e_get_vf_stats.
> rte_pmd_i40e_reset_vf_stats.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---

<...>

> diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
> index 8ac1bc8..7a5d211 100644
> --- a/drivers/net/i40e/rte_pmd_i40e_version.map
> +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
> @@ -6,7 +6,9 @@ DPDK_2.0 {
>  DPDK_17.02 {
>  	global:
>  
> +	rte_pmd_i40e_get_vf_stats;
>  	rte_pmd_i40e_ping_vfs;
> +	rte_pmd_i40e_reset_vf_stats;
>  	rte_pmd_i40e_set_tx_loopback;
>  	rte_pmd_i40e_set_vf_broadcast;
>  	rte_pmd_i40e_set_vf_mac_addr;

Hi Wenzhuo, Mark,

I think this is the list of all APIs added with this patchset.

Just a question, what do you think following a logic in API naming as:
<name_space>_<object>_<action> ?

So API names become:
rte_pmd_i40e_tx_loopback_set;
rte_pmd_i40e_vf_broadcast_set;
rte_pmd_i40e_vf_mac_addr_set;
rte_pmd_i40e_vfs_ping;
rte_pmd_i40e_vf_stats_get;
rte_pmd_i40e_vf_stats_reset;


After above rename, rte_pmd_i40e_tx_loopback_set() is not giving a hint
that this is something related to the PF controlling VF, perhaps we can
rename the API ?

Also rte_pmd_i40e_vfs_ping() can become rte_pmd_i40e_vf_ping_all() to be
more consistent about _vf_ usage.

Overall, they can be something like:
rte_pmd_i40e_vf_broadcast_set;
rte_pmd_i40e_vf_mac_addr_set;
rte_pmd_i40e_vf_ping_all;
rte_pmd_i40e_vf_stats_get;
rte_pmd_i40e_vf_stats_reset;
rte_pmd_i40e_vf_tx_loopback_set;

What do you think?


> 

^ permalink raw reply

* Re: [PATCH v3 0/6] libeventdev API and northbound implementation
From: Bruce Richardson @ 2016-12-20 13:22 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: dev, thomas.monjalon, hemant.agrawal, gage.eads, harry.van.haaren
In-Reply-To: <20161220130929.GA5901@localhost.localdomain>

On Tue, Dec 20, 2016 at 06:39:30PM +0530, Jerin Jacob wrote:
> On Tue, Dec 20, 2016 at 11:13:42AM +0000, Bruce Richardson wrote:
> > On Sun, Dec 18, 2016 at 07:51:29PM +0530, Jerin Jacob wrote:
> > > As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> > > described in [3] (also pasted below), here is the first non-draft series
> > > for this new API.
> > > 
> > > [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
> > > [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
> > > [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html
> > > 
> > > v2..v3:
> > > 
> > > - This patch set is check-patch clean with an exception that
> > > 03/06 has one WARNING:MACRO_WITH_FLOW_CONTROL
> > > - Looking forward to getting additional maintainers for libeventdev
> > > 
> > > TODO:
> > > 1) Create user guide
> > > 
> > > Jerin Jacob (6):
> > >   eventdev: introduce event driven programming model
> > >   eventdev: define southbound driver interface
> > >   eventdev: implement the northbound APIs
> > >   eventdev: implement PMD registration functions
> > >   event/skeleton: add skeleton eventdev driver
> > >   app/test: unit test case for eventdev APIs
> > > 
> > Hi Jerin,
> 
> Hi Bruce,
> 
> > 
> > other than the couple of comments I've made in replies to the individual
> > patches, this looks pretty good to me. Only additional comment I have is
> 
> Thanks
> 
> > that some of the macro names are a little long, and maybe we can shorten
> > them  For example, you've added "_FLAG_" into the config flag macros,
> > and I'm not sure that is necessary. Similarly, I think we can drop
> > "_DEV_" from the PRIORITY names to shorten them.
> 
> OK. I will remove the explicit _FLAG_  to shorten macro name.
> The _DEV_ in PRIORITY is not that long. So I would like to keep it for
> consistency and to denote it across priorities in event dev.
> 
> > 
> > Irrespective of these naming suggestions, once the other couple of
> > comments are taken care of, I think this set is suitable for merging to
> > the next-event tree.
> 
> I will send v4 with fixes and your suggestions. If their is no further
> comment on that, we will merge to next-event tree
> 
I'm not sure a v4 is needed, unless you especially want to do one. 
Given the scope of the suggested changes I think you can just make
those changes on apply to the next-event tree.

/Bruce

^ permalink raw reply

* [PATCH v6] nfp: report link speed using hardware info
From: Alejandro Lucero @ 2016-12-20 13:22 UTC (permalink / raw)
  To: dev

Previous reported speed was hardcoded because there was not firmware
support for getting this information. This change needs also to support
old firmware versions, but instead of the previous hardcoded report, no
speed is reported to the user avoiding to give the wrong speed when link
is not configured to 40G.

v6: avoid to report speed with old firmware
v5: Fix missing parenthesis
v4: Make conditional simple and more ellaborated commit comment.
v3: remove unsed macro
v2: use RTE_DIM instead of own macro

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c      | 28 ++++++++++++++++++++++++++--
 drivers/net/nfp/nfp_net_ctrl.h | 11 +++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 2ae7f87..b001a46 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -816,6 +816,17 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	struct rte_eth_link link, old;
 	uint32_t nn_link_status;
 
+	static const uint32_t ls_to_ethtool[] = {
+		[NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE,
+		[NFP_NET_CFG_STS_LINK_RATE_UNKNOWN]     = ETH_SPEED_NUM_NONE,
+		[NFP_NET_CFG_STS_LINK_RATE_1G]          = ETH_SPEED_NUM_1G,
+		[NFP_NET_CFG_STS_LINK_RATE_10G]         = ETH_SPEED_NUM_10G,
+		[NFP_NET_CFG_STS_LINK_RATE_25G]         = ETH_SPEED_NUM_25G,
+		[NFP_NET_CFG_STS_LINK_RATE_40G]         = ETH_SPEED_NUM_40G,
+		[NFP_NET_CFG_STS_LINK_RATE_50G]         = ETH_SPEED_NUM_50G,
+		[NFP_NET_CFG_STS_LINK_RATE_100G]        = ETH_SPEED_NUM_100G,
+	};
+
 	PMD_DRV_LOG(DEBUG, "Link update\n");
 
 	hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -831,8 +842,21 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 		link.link_status = ETH_LINK_UP;
 
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	/* Other cards can limit the tx and rx rate per VF */
-	link.link_speed = ETH_SPEED_NUM_40G;
+
+	nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
+			 NFP_NET_CFG_STS_LINK_RATE_MASK;
+
+	if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) ||
+	    ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) &&
+	    (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0)))
+		/* We really do not know the speed wil old firmware */
+		link.link_speed = ETH_SPEED_NUM_NONE;
+	else {
+		if (nn_link_status >= RTE_DIM(ls_to_ethtool))
+			link.link_speed = ETH_SPEED_NUM_NONE;
+		else
+			link.link_speed = ls_to_ethtool[nn_link_status];
+	}
 
 	if (old.link_status != link.link_status) {
 		nfp_net_dev_atomic_write_link_status(dev, &link);
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index fce8251..426402b 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -157,6 +157,17 @@
 #define   NFP_NET_CFG_VERSION_MINOR(x)    (((x) & 0xff) <<  0)
 #define NFP_NET_CFG_STS                 0x0034
 #define   NFP_NET_CFG_STS_LINK            (0x1 << 0) /* Link up or down */
+/* Link rate */
+#define   NFP_NET_CFG_STS_LINK_RATE_SHIFT 1
+#define   NFP_NET_CFG_STS_LINK_RATE_MASK  0xF
+#define   NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED   0
+#define   NFP_NET_CFG_STS_LINK_RATE_UNKNOWN       1
+#define   NFP_NET_CFG_STS_LINK_RATE_1G            2
+#define   NFP_NET_CFG_STS_LINK_RATE_10G           3
+#define   NFP_NET_CFG_STS_LINK_RATE_25G           4
+#define   NFP_NET_CFG_STS_LINK_RATE_40G           5
+#define   NFP_NET_CFG_STS_LINK_RATE_50G           6
+#define   NFP_NET_CFG_STS_LINK_RATE_100G          7
 #define NFP_NET_CFG_CAP                 0x0038
 #define NFP_NET_CFG_MAX_TXRINGS         0x003c
 #define NFP_NET_CFG_MAX_RXRINGS         0x0040
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v3 02/12] eal/bus: introduce bus abstraction
From: Jan Blunck @ 2016-12-20 13:17 UTC (permalink / raw)
  To: Shreyansh Jain
  Cc: dev, David Marchand, Thomas Monjalon, Ferruh Yigit, jianbo.liu
In-Reply-To: <1481893853-31790-3-git-send-email-shreyansh.jain@nxp.com>

On Fri, Dec 16, 2016 at 2:10 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> This patch introduces the rte_bus abstraction for devices and drivers in
> EAL framework. The model is:
>  - One or more buses are connected to a CPU (or core)
>  - One or more devices are conneted to a Bus
>  - Drivers are running instances which manage one or more devices
>  - Bus is responsible for identifying devices (and interrupt propogation)
>  - Driver is responsible for initializing the device
>
> This patch adds a 'rte_bus' class which rte_driver and rte_device refer.
> This way, each device (rte_xxx_device) would have reference to the bus
> it is based on. As well as, each driver (rte_xxx_driver) would have link
> to the bus and devices on it for servicing.
>
>                                   __ rte_bus_list
>                                  /
>                      +----------'---+
>                      |rte_bus       |
>                      | driver_list------> List of rte_bus specific
>                      | device_list----    devices
>                      |              | `-> List of rte_bus associated
>                      |              |     drivers
>                      +--|------|----+
>               _________/        \_________
>     +--------/----+                     +-\---------------+
>     |rte_device   |                     |rte_driver       |
>     | rte_bus     |                     | rte_bus         |
>     | rte_driver  |                     | ...             |
>     | ...         |                     +---------...-----+
>     |             |                               |||
>     +---||--------+                               |||
>         ||                                        |||
>         | \                                        \\\
>         |  \_____________                           \\\
>         |                \                          |||
>  +------|---------+ +----|----------+               |||
>  |rte_pci_device  | |rte_xxx_device |               |||
>  | ....           | | ....          |               |||
>  +----------------+ +---------------+              / | \
>                                                   /  |  \
>                             _____________________/  /    \
>                            /                    ___/      \
>             +-------------'--+    +------------'---+    +--'------------+
>             |rte_pci_driver  |    |rte_vdev_driver |    |rte_xxx_driver |
>             | ....           |    | ....           |    | ....          |
>             +----------------+    +----------------+    +---------------+
>
> This patch only enables the bus references on rte_driver and rte_driver.
> EAL wide global device and driver list continue to exist until an instance
> of bus is added in subsequent patches.
>
> This patch also introduces RTE_REGISTER_BUS macro on the lines of
> RTE_PMD_REGISTER_XXX. Key difference is that the constructor priority has
> been explicitly set to 101 so as to execute bus registration before PMD.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> --
> v2:
>  - fix bsdapp compilation issue because of missing export symbols in map
>    file
> ---
>  lib/librte_eal/bsdapp/eal/Makefile              |   1 +
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  15 ++
>  lib/librte_eal/common/Makefile                  |   2 +-
>  lib/librte_eal/common/eal_common_bus.c          | 192 ++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 174 +++++++++++++++++++++
>  lib/librte_eal/common/include/rte_dev.h         |   2 +
>  lib/librte_eal/linuxapp/eal/Makefile            |   1 +
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  15 ++
>  8 files changed, 401 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_eal/common/eal_common_bus.c
>  create mode 100644 lib/librte_eal/common/include/rte_bus.h
>
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index a15b762..cce99f7 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_cpuflags.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_thread.c
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2f81f7c..23fc1c1 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -174,3 +174,18 @@ DPDK_16.11 {
>         rte_eal_vdrv_unregister;
>
>  } DPDK_16.07;
> +
> +DPDK_17.02 {
> +       global:
> +
> +       rte_bus_list;
> +       rte_eal_bus_add_device;
> +       rte_eal_bus_add_driver;
> +       rte_eal_get_bus;
> +       rte_eal_bus_dump;
> +       rte_eal_bus_register;
> +       rte_eal_bus_remove_device;
> +       rte_eal_bus_remove_driver;
> +       rte_eal_bus_unregister;
> +
> +} DPDK_16.11;
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index a92c984..0c39414 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -38,7 +38,7 @@ INC += rte_per_lcore.h rte_random.h
>  INC += rte_tailq.h rte_interrupts.h rte_alarm.h
>  INC += rte_string_fns.h rte_version.h
>  INC += rte_eal_memconfig.h rte_malloc_heap.h
> -INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
> +INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_vdev.h
>  INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
>  INC += rte_malloc.h rte_keepalive.h rte_time.h
>
> 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..612f64e
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -0,0 +1,192 @@
> +/*-
> + *   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_bus.h>
> +#include <rte_devargs.h>
> +#include <rte_debug.h>
> +
> +#include "eal_private.h"
> +
> +struct rte_bus_list rte_bus_list =
> +       TAILQ_HEAD_INITIALIZER(rte_bus_list);
> +
> +/** @internal
> + * Add a device to a bus.
> + */
> +void
> +rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
> +{
> +       RTE_VERIFY(bus);
> +       RTE_VERIFY(dev);
> +
> +       TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
> +       dev->bus = bus;
> +}
> +
> +/** @internal
> + * Remove a device from its bus.
> + */
> +void
> +rte_eal_bus_remove_device(struct rte_device *dev)
> +{
> +       struct rte_bus *bus;
> +
> +       RTE_VERIFY(dev);
> +       RTE_VERIFY(dev->bus);
> +
> +       bus = dev->bus;
> +       TAILQ_REMOVE(&bus->device_list, dev, next);
> +       dev->bus = NULL;
> +}
> +
> +/** @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);
> +       drv->bus = bus;
> +}
> +
> +/** @internal
> + * Disassociate a driver from bus.
> + */
> +void
> +rte_eal_bus_remove_driver(struct rte_driver *drv)
> +{
> +       struct rte_bus *bus;
> +
> +       RTE_VERIFY(drv);
> +       RTE_VERIFY(drv->bus);
> +
> +       bus = drv->bus;
> +       TAILQ_REMOVE(&bus->driver_list, drv, next);
> +       drv->bus = NULL;
> +}
> +
> +/**
> + * 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 (!strcmp(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)
> +{
> +       RTE_VERIFY(bus);
> +       RTE_VERIFY(bus->name && strlen(bus->name));
> +
> +       /* Initialize the driver and device list associated with the bus */
> +       TAILQ_INIT(&(bus->driver_list));
> +       TAILQ_INIT(&(bus->device_list));
> +
> +       TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
> +       RTE_LOG(INFO, 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)));
> +
> +       /* TODO: For each device, call its rte_device->driver->remove()
> +        * and rte_eal_bus_remove_driver()
> +        */
> +
> +       TAILQ_REMOVE(&rte_bus_list, bus, next);
> +       RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
> +}
> +
> +/* dump one bus info */
> +static int
> +bus_dump_one(FILE *f, struct rte_bus *bus)
> +{
> +       int ret;
> +
> +       /* For now, dump only the bus name */
> +       ret = fprintf(f, " %s\n", bus->name);
> +
> +       /* Error in case of inability in writing to stream */
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +void
> +rte_eal_bus_dump(FILE *f)
> +{
> +       int ret;
> +       struct rte_bus *bus;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +               ret = bus_dump_one(f, bus);
> +               if (ret) {
> +                       RTE_LOG(ERR, EAL, "Unable to write to stream (%d)\n",
> +                               ret);
> +                       break;
> +               }
> +       }
> +}
> 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..f0297a9
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -0,0 +1,174 @@
> +/*-
> + *   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);
> +
> +/* Global Bus list */
> +extern struct rte_bus_list rte_bus_list;
> +
> +struct rte_bus {
> +       TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
> +       struct rte_driver_list driver_list;
> +                                    /**< List of all drivers on bus */
> +       struct rte_device_list device_list;
> +                                    /**< List of all devices on bus */
> +       const char *name;            /**< Name of the 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);
> +
> +/** @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);
> +
> +/** @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);
> +
> +/**
> + * Dump information of all the buses registered with EAL.
> + *
> + * @param f
> + *     A valid and open output stream handle
> + *
> + * @return
> + *      0 in case of success
> + *     !0 in case there is error in opening the output stream
> + */
> +void rte_eal_bus_dump(FILE *f);
> +
> +/** Helper for Bus registration. The constructor has higher priority than
> + * PMD constructors
> + */
> +#define RTE_REGISTER_BUS(nm, bus) \
> +static void __attribute__((constructor(101), used)) 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..4004f9a 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -122,6 +122,7 @@ struct rte_driver;
>   */
>  struct rte_device {
>         TAILQ_ENTRY(rte_device) next; /**< Next device */
> +       struct rte_bus *bus;          /**< Device connected to this bus */

Is there a reason why this isn't const?


>         struct rte_driver *driver;    /**< Associated driver */
>         int numa_node;                /**< NUMA node connection */
>         struct rte_devargs *devargs;  /**< Device user arguments */
> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>   */
>  struct rte_driver {
>         TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> +       struct rte_bus *bus;           /**< Bus serviced by this driver */

Same thing here.

>         const char *name;                   /**< Driver name. */
>         const char *alias;              /**< Driver alias. */
>  };
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 4e206f0..aa874a5 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_cpuflags.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_thread.c
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 83721ba..c873a7f 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -178,3 +178,18 @@ DPDK_16.11 {
>         rte_eal_vdrv_unregister;
>
>  } DPDK_16.07;
> +
> +DPDK_17.02 {
> +       global:
> +
> +       rte_bus_list;
> +       rte_eal_bus_add_device;
> +       rte_eal_bus_add_driver;
> +       rte_eal_get_bus;
> +       rte_eal_bus_dump;
> +       rte_eal_bus_register;
> +       rte_eal_bus_remove_device;
> +       rte_eal_bus_remove_driver;
> +       rte_eal_bus_unregister;
> +
> +} DPDK_16.11;
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH v3 0/6] libeventdev API and northbound implementation
From: Jerin Jacob @ 2016-12-20 13:09 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, thomas.monjalon, hemant.agrawal, gage.eads, harry.van.haaren
In-Reply-To: <20161220111342.GA201880@bricha3-MOBL3.ger.corp.intel.com>

On Tue, Dec 20, 2016 at 11:13:42AM +0000, Bruce Richardson wrote:
> On Sun, Dec 18, 2016 at 07:51:29PM +0530, Jerin Jacob wrote:
> > As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> > described in [3] (also pasted below), here is the first non-draft series
> > for this new API.
> > 
> > [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
> > [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
> > [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html
> > 
> > v2..v3:
> > 
> > - This patch set is check-patch clean with an exception that
> > 03/06 has one WARNING:MACRO_WITH_FLOW_CONTROL
> > - Looking forward to getting additional maintainers for libeventdev
> > 
> > TODO:
> > 1) Create user guide
> > 
> > Jerin Jacob (6):
> >   eventdev: introduce event driven programming model
> >   eventdev: define southbound driver interface
> >   eventdev: implement the northbound APIs
> >   eventdev: implement PMD registration functions
> >   event/skeleton: add skeleton eventdev driver
> >   app/test: unit test case for eventdev APIs
> > 
> Hi Jerin,

Hi Bruce,

> 
> other than the couple of comments I've made in replies to the individual
> patches, this looks pretty good to me. Only additional comment I have is

Thanks

> that some of the macro names are a little long, and maybe we can shorten
> them  For example, you've added "_FLAG_" into the config flag macros,
> and I'm not sure that is necessary. Similarly, I think we can drop
> "_DEV_" from the PRIORITY names to shorten them.

OK. I will remove the explicit _FLAG_  to shorten macro name.
The _DEV_ in PRIORITY is not that long. So I would like to keep it for
consistency and to denote it across priorities in event dev.

> 
> Irrespective of these naming suggestions, once the other couple of
> comments are taken care of, I think this set is suitable for merging to
> the next-event tree.

I will send v4 with fixes and your suggestions. If their is no further
comment on that, we will merge to next-event tree

> 
> Series Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Regards,
> /Bruce
> 

^ permalink raw reply

* Re: [PATCH 04/13] eal: introduce driver type
From: Ferruh Yigit @ 2016-12-20 13:04 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Stephen Hemminger
In-Reply-To: <20161219215944.17226-5-sthemmin@microsoft.com>

On 12/19/2016 9:59 PM, Stephen Hemminger wrote:
> Since multiple buses and device types need to be supported.
> Provide type field in driver.
> ---
>  lib/librte_eal/common/include/rte_dev.h  | 15 ++++++++++++---
>  lib/librte_eal/common/include/rte_pci.h  |  1 +
>  lib/librte_eal/common/include/rte_vdev.h |  1 +
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index e5471a22..3f4e26e6 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -144,12 +144,21 @@ void rte_eal_device_insert(struct rte_device *dev);
>  void rte_eal_device_remove(struct rte_device *dev);
>  
>  /**
> + * Type of device driver
> + */
> +enum rte_driver_type {
> +	PMD_VIRTUAL,
> +	PMD_PCI,
> +};

There were types in v16.07 and previous.
Types has been removed in v16.11.
Now are we sure we want to add them back?

^ permalink raw reply

* Re: [PATCH 04/13] eal: introduce driver type
From: Jan Blunck @ 2016-12-20 13:00 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger
In-Reply-To: <20161219215944.17226-5-sthemmin@microsoft.com>

On Mon, Dec 19, 2016 at 10:59 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> Since multiple buses and device types need to be supported.
> Provide type field in driver.
> ---
>  lib/librte_eal/common/include/rte_dev.h  | 15 ++++++++++++---
>  lib/librte_eal/common/include/rte_pci.h  |  1 +
>  lib/librte_eal/common/include/rte_vdev.h |  1 +
>  3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index e5471a22..3f4e26e6 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -144,12 +144,21 @@ void rte_eal_device_insert(struct rte_device *dev);
>  void rte_eal_device_remove(struct rte_device *dev);
>
>  /**
> + * Type of device driver
> + */
> +enum rte_driver_type {
> +       PMD_VIRTUAL,
> +       PMD_PCI,
> +};
> +
> +/**
>   * A structure describing a device driver.
>   */
>  struct rte_driver {
>         TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> -       const char *name;                   /**< Driver name. */
> -       const char *alias;              /**< Driver alias. */
> +       const char *name;              /**< Driver name. */
> +       const char *alias;             /**< Driver alias. */
> +       enum rte_driver_type type;     /**< Driver type. */
>  };

I wonder who is the user of the type. It can't be the driver itself
because it must be aware of what kind of low-level interface it
serves. So this seems to be a helper to allow users of rte_driver to
upcast to the object that embeds the rte_driver at runtime. I don't
believe that this is a good interface because it often leads to ugly
and error prone code.

>
>  /**
> @@ -243,4 +252,4 @@ __attribute__((used)) = str
>  }
>  #endif
>
> -#endif /* _RTE_VDEV_H_ */
> +#endif /* _RTE_DEV_H_ */
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 9ce88472..d377d539 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -492,6 +492,7 @@ RTE_INIT(pciinitfn_ ##nm); \
>  static void pciinitfn_ ##nm(void) \
>  {\
>         (pci_drv).driver.name = RTE_STR(nm);\
> +       (pci_drv).driver.type = PMD_PCI; \
>         rte_eal_pci_register(&pci_drv); \
>  } \
>  RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> diff --git a/lib/librte_eal/common/include/rte_vdev.h b/lib/librte_eal/common/include/rte_vdev.h
> index 784e837d..98fb5bb5 100644
> --- a/lib/librte_eal/common/include/rte_vdev.h
> +++ b/lib/librte_eal/common/include/rte_vdev.h
> @@ -88,6 +88,7 @@ static void vdrvinitfn_ ##vdrv(void)\
>  {\
>         (vdrv).driver.name = RTE_STR(nm);\
>         (vdrv).driver.alias = vdrvinit_ ## nm ## _alias;\
> +       (vdrv).driver.type = PMD_VIRTUAL;\
>         rte_eal_vdrv_register(&vdrv);\
>  } \
>  RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH 1/3] ethdev: New API to free consumed buffers in TX ring
From: Adrien Mazarguil @ 2016-12-20 12:58 UTC (permalink / raw)
  To: Ananyev, Konstantin
  Cc: Billy McFall, thomas.monjalon@6wind.com, Lu, Wenzhuo,
	dev@dpdk.org, Stephen Hemminger
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0F2D8A@irsmsx105.ger.corp.intel.com>

On Tue, Dec 20, 2016 at 12:17:10PM +0000, Ananyev, Konstantin wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Tuesday, December 20, 2016 11:28 AM
> > To: Billy McFall <bmcfall@redhat.com>
> > Cc: thomas.monjalon@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org; Stephen Hemminger
> > <stephen@networkplumber.org>
> > Subject: Re: [dpdk-dev] [PATCH 1/3] ethdev: New API to free consumed buffers in TX ring
> > 
> > Hi Billy,
> > 
> > On Fri, Dec 16, 2016 at 07:48:49AM -0500, Billy McFall wrote:
> > > Add a new API to force free consumed buffers on TX ring. API will return
> > > the number of packets freed (0-n) or error code if feature not supported
> > > (-ENOTSUP) or input invalid (-ENODEV).
> > >
> > > Because rte_eth_tx_buffer() may be used, and mbufs may still be held
> > > in local buffer, the API also accepts *buffer and *sent. Before
> > > attempting to free, rte_eth_tx_buffer_flush() is called to make sure
> > > all mbufs are sent to Tx ring. rte_eth_tx_buffer_flush() is called even
> > > if threshold is not met.
> > >
> > > Signed-off-by: Billy McFall <bmcfall@redhat.com>
> > > ---
> > >  lib/librte_ether/rte_ethdev.h | 56 +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 56 insertions(+)
> > >
> > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > > index 9678179..e3f2be4 100644
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -1150,6 +1150,9 @@ 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 int (*eth_tx_done_cleanup_t)(void *txq, uint32_t free_cnt);
> > > +/**< @internal Force mbufs to be from TX ring. */
> > > +
> > >  typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
> > >  	uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
> > >
> > > @@ -1467,6 +1470,7 @@ struct eth_dev_ops {
> > >  	eth_rx_disable_intr_t      rx_queue_intr_disable;
> > >  	eth_tx_queue_setup_t       tx_queue_setup;/**< Set up device TX queue.*/
> > >  	eth_queue_release_t        tx_queue_release;/**< Release TX queue.*/
> > > +	eth_tx_done_cleanup_t      tx_done_cleanup;/**< Free tx ring mbufs */
> > >  	eth_dev_led_on_t           dev_led_on;    /**< Turn on LED. */
> > >  	eth_dev_led_off_t          dev_led_off;   /**< Turn off LED. */
> > >  	flow_ctrl_get_t            flow_ctrl_get; /**< Get flow control. */
> > > @@ -2943,6 +2947,58 @@ rte_eth_tx_buffer(uint8_t port_id, uint16_t queue_id,
> > >  }
> > >
> > >  /**
> > > + * Request the driver to free mbufs currently cached by the driver. The
> > > + * driver will only free the mbuf if it is no longer in use.
> > > + *
> > > + * @param port_id
> > > + *   The port identifier of the Ethernet device.
> > > + * @param queue_id
> > > + *   The index of the transmit queue through which output packets must be
> > > + *   sent.
> > > + *   The value must be in the range [0, nb_tx_queue - 1] previously supplied
> > > + *   to rte_eth_dev_configure().
> > > + * @param free_cnt
> > > + *   Maximum number of packets to free. Use 0 to indicate all possible packets
> > > + *   should be freed. Note that a packet may be using multiple mbufs.
> > > + * @param buffer
> > > + *   Buffer used to collect packets to be sent. If provided, the buffer will
> > > + *   be flushed, even if the current length is less than buffer->size. Pass NULL
> > > + *   if buffer has already been flushed.
> > > + * @param sent
> > > + *   Pointer to return number of packets sent if buffer has packets to be sent.
> > > + *   If *buffer is supplied, *sent must also be supplied.
> > > + * @return
> > > + *   Failure: < 0
> > > + *     -ENODEV: Invalid interface
> > > + *     -ENOTSUP: Driver does not support function
> > > + *   Success: >= 0
> > > + *     0-n: Number of packets freed. More packets may still remain in ring that
> > > + *     are in use.
> > > + */
> > > +
> > > +static inline int
> > > +rte_eth_tx_done_cleanup(uint8_t port_id, uint16_t queue_id,  uint32_t free_cnt,
> > > +		struct rte_eth_dev_tx_buffer *buffer, uint16_t *sent)
> > > +{
> > > +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > > +
> > > +	/* Validate Input Data. Bail if not valid or not supported. */
> > > +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > > +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_done_cleanup, -ENOTSUP);
> > > +
> > > +	/*
> > > +	 * If transmit buffer is provided and there are still packets to be
> > > +	 * sent, then send them before attempting to free pending mbufs.
> > > +	 */
> > > +	if (buffer && sent)
> > > +		*sent = rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
> > > +
> > > +	/* Call driver to free pending mbufs. */
> > > +	return (*dev->dev_ops->tx_done_cleanup)(dev->data->tx_queues[queue_id],
> > > +			free_cnt);
> > > +}
> > > +
> > > +/**
> > >   * Configure a callback for buffered packets which cannot be sent
> > >   *
> > >   * Register a specific callback to be called when an attempt is made to send
> > 
> > Just a thought to follow-up on Stephen's comment to further simplify this
> > API, how about not adding any new eth_dev_ops but instead defining what
> > should happen during an empty TX burst call (tx_burst() with 0 packets).
> > 
> > Several PMDs already have a check for this scenario and start by cleaning up
> > completed packets anyway, they effectively partially implement this
> > definition for free already.
> 
> Many PMDs  start by cleaning up only when number of free entries
> drop below some point.
> Also in that case the author would have to modify (and test) all existing TX routinies.
> So I think a separate API call seems more plausible.

Not necessarily, as I understand this API in its current form only suggests
that a PMD should release a few mbufs from a queue if possible, without any
guarantee, PMDs are not forced to comply.

I think the threshold you mention is a valid reason not to release them, and
it wouldn't change a thing to existing tx_burst() implementations in the
meantime (only documentation).

This threshold could also be bypassed rather painlessly in the
"if (unlikely(nb_pkts == 0))" case that all PMDs already check for in a
way or another.

> Though I am agree with previous comment from Stephen that last two parameters
> are redundant and would just overcomplicate things.
> tin
> 
> > 
> > The main difference with this API would be that you wouldn't know how many
> > mbufs were freed and wouldn't collect them into an array. However most
> > applications have one mbuf pool and/or know where they come from, so they
> > can just query the pool or attempt to re-allocate from it after doing empty
> > bursts in case of starvation.
> > 
> > [1] http://dpdk.org/ml/archives/dev/2016-December/052469.html

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH v3 4/9] virtio: Don't fill dev_info->driver_name
From: Ferruh Yigit @ 2016-12-20 12:58 UTC (permalink / raw)
  To: Jan Blunck; +Cc: dev, Shreyansh Jain, David Marchand, Stephen Hemminger
In-Reply-To: <CALe+Z021O=MyCLnPg0RYy6eKH2vTPazKD7i-nDvyGtFsFWJ4Ow@mail.gmail.com>

On 12/20/2016 12:40 PM, Jan Blunck wrote:
> On Tue, Dec 20, 2016 at 1:17 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> On 12/20/2016 11:11 AM, Jan Blunck wrote:
>>> This is overwritten in rte_eth_dev_info_get().
>>>
>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>> Reviewed-by: David Marchand <david.marchand@6wind.com>
>>> ---
>>>  drivers/net/virtio/virtio_ethdev.c | 4 ----
>>>  1 file changed, 4 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>>> index 079fd6c..741688e 100644
>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>> @@ -1624,10 +1624,6 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>>>       uint64_t tso_mask;
>>>       struct virtio_hw *hw = dev->data->dev_private;
>>>
>>> -     if (dev->pci_dev)
>>> -             dev_info->driver_name = dev->driver->pci_drv.driver.name;
>>> -     else
>>> -             dev_info->driver_name = "virtio_user PMD";
>>>       dev_info->max_rx_queues =
>>>               RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
>>>       dev_info->max_tx_queues =
>>>
>>
>>
>> This has been already removed in next-net with following commit:
>> 7c6c1857358c ("net: remove dead driver names")
> 
> Thanks. Do you want me to rebase against next-net and resend the
> series before continuing to review further?
> 

No, I thought this patch can be dropped from patchset, but no, you need
this for decoupling pci_dev.

So, this needs to be resolved in integration time.

I believe this patchset should be against main tree, because of eal and
ethdev patches in it.
And there are already a few patchset around for this area, let's not add
more complexity by adding next-net into the picture.

^ permalink raw reply

* [PATCH v2] nfp: extend speed capabilities advertised
From: Alejandro Lucero @ 2016-12-20 12:52 UTC (permalink / raw)
  To: dev

NFP supports more speeds than just 40 and 100GB, which were
what was advertised before.

v2: add feature to nfp.ini

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 doc/guides/nics/features/nfp.ini | 1 +
 drivers/net/nfp/nfp_net.c        | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/features/nfp.ini b/doc/guides/nics/features/nfp.ini
index 476ed31..25a4e18 100644
--- a/doc/guides/nics/features/nfp.ini
+++ b/doc/guides/nics/features/nfp.ini
@@ -6,6 +6,7 @@
 [Features]
 Link status          = Y
 Link status event    = Y
+Speed capabilities   = Y
 Queue start/stop     = Y
 MTU update           = Y
 Jumbo frame          = Y
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index b001a46..7a636fb 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1078,7 +1078,9 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
 	dev_info->reta_size = NFP_NET_CFG_RSS_ITBL_SZ;
 	dev_info->hash_key_size = NFP_NET_CFG_RSS_KEY_SZ;
 
-	dev_info->speed_capa = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_100G;
+	dev_info->speed_capa = ETH_SPEED_NUM_1G | ETH_LINK_SPEED_10G |
+			       ETH_SPEED_NUM_25G | ETH_SPEED_NUM_40G |
+			       ETH_SPEED_NUM_50G | ETH_LINK_SPEED_100G;
 }
 
 static const uint32_t *
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC v2 00/13] Generalize rte_eth_dev model
From: Jan Blunck @ 2016-12-20 12:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger
In-Reply-To: <20161219215944.17226-1-sthemmin@microsoft.com>

On Mon, Dec 19, 2016 at 10:59 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> This set of patches generalize the ethernet device model sufficently
> to support virtual, pci, and vmbus devices.
>
> All this is based on the XP principal of only building what is
> necessary and no more than that. The rationale is NOT to rework the
> DPDK to have same level of bus and parent information as the Linux
> device model. This would be unnecessary, fragile and unnecessarily OS
> dependent.
>
> Instead, this set of patches does the simple change of refactoring
> so that the object relationship between ethernet (rte_eth_dev),
> PCI (rte_pci_device) and generic (rte_device) is properly broken
> apart.

Yes, as I pointed out when reviewing the bus related changes the
refactoring of the ethdev doesn't depend on the bus work to get merged
first.

I've sent a similar series in November already. I guess that is a sign
that the review got stuck somewhere.

Jan


> It does make the simplifying assumption that the device driver knows
> what type of bus it is on. For a future case where a device has to
> be able to handle different bus families; then the device driver
> can have multiple device drivers in one code base (like ixgbe PF/VF).
>
>
> Jan Blunck (1):
>   eal: define container_of macro
>
> Stephen Hemminger (12):
>   ethdev: increase length ethernet device internal name
>   rte_device: make driver pointer const
>   eal: introduce driver type
>   pmd: remove useless reset of dev_info->dev_pci
>   ethdev: make dev_info generic (not just PCI)
>   e1000: localize mapping from eth_dev to pci
>   ixgbe: localize mapping from eth_dev to pci_device
>   i40e: localize mapping of eth_dev to pci
>   virtio: localize mapping from rte_eth to pci
>   broadcom: localize mapping from eth_dev to pci
>   ethdev: change pci_dev to generic device
>   hyperv: VMBUS support infrastucture
>
>  app/test-pmd/config.c                       |  32 +++++++-
>  app/test-pmd/testpmd.c                      |  11 ++-
>  app/test-pmd/testpmd.h                      |  32 ++++----
>  app/test/test_kni.c                         |  39 +++++++--
>  app/test/virtual_pmd.c                      |   4 +-
>  doc/guides/rel_notes/deprecation.rst        |   3 +
>  doc/guides/rel_notes/release_17_02.rst      |  10 +--
>  drivers/net/af_packet/rte_eth_af_packet.c   |   1 -
>  drivers/net/bnxt/bnxt_ethdev.c              |  24 +++---
>  drivers/net/bnxt/bnxt_ring.c                |  16 ++--
>  drivers/net/bonding/rte_eth_bond_args.c     |  12 ++-
>  drivers/net/bonding/rte_eth_bond_pmd.c      |   1 -
>  drivers/net/cxgbe/cxgbe_ethdev.c            |   2 +-
>  drivers/net/cxgbe/cxgbe_main.c              |   5 +-
>  drivers/net/e1000/e1000_ethdev.h            |   2 +
>  drivers/net/e1000/em_ethdev.c               |  50 +++++++-----
>  drivers/net/e1000/igb_ethdev.c              |  99 ++++++++++++-----------
>  drivers/net/e1000/igb_pf.c                  |   4 +-
>  drivers/net/ena/ena_ethdev.c                |   2 +-
>  drivers/net/enic/enic_ethdev.c              |   2 +-
>  drivers/net/enic/enic_rxtx.c                |   2 -
>  drivers/net/fm10k/fm10k_ethdev.c            |  90 +++++++++++++--------
>  drivers/net/i40e/i40e_ethdev.c              |  77 +++++++++++-------
>  drivers/net/i40e/i40e_ethdev.h              |   3 +
>  drivers/net/i40e/i40e_ethdev_vf.c           |  59 ++++++++------
>  drivers/net/ixgbe/ixgbe_ethdev.c            | 120 ++++++++++++++++------------
>  drivers/net/ixgbe/ixgbe_ethdev.h            |   3 +
>  drivers/net/ixgbe/ixgbe_pf.c                |   4 +-
>  drivers/net/null/rte_eth_null.c             |   1 -
>  drivers/net/pcap/rte_eth_pcap.c             |   1 -
>  drivers/net/qede/qede_ethdev.c              |  24 +++---
>  drivers/net/qede/qede_ethdev.h              |   4 +
>  drivers/net/ring/rte_eth_ring.c             |   1 -
>  drivers/net/virtio/virtio_ethdev.c          |  95 +++++++++++++++-------
>  drivers/net/virtio/virtio_user_ethdev.c     |   2 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c        |   3 +-
>  drivers/net/xenvirt/rte_eth_xenvirt.c       |   1 -
>  lib/librte_eal/common/Makefile              |   2 +-
>  lib/librte_eal/common/eal_common_devargs.c  |   7 ++
>  lib/librte_eal/common/eal_common_options.c  |  38 +++++++++
>  lib/librte_eal/common/eal_internal_cfg.h    |   3 +-
>  lib/librte_eal/common/eal_options.h         |   6 ++
>  lib/librte_eal/common/eal_private.h         |   5 ++
>  lib/librte_eal/common/include/rte_common.h  |  20 +++++
>  lib/librte_eal/common/include/rte_dev.h     |  17 +++-
>  lib/librte_eal/common/include/rte_devargs.h |   8 ++
>  lib/librte_eal/common/include/rte_pci.h     |   1 +
>  lib/librte_eal/common/include/rte_vdev.h    |   1 +
>  lib/librte_eal/linuxapp/eal/Makefile        |   6 ++
>  lib/librte_eal/linuxapp/eal/eal.c           |  11 +++
>  lib/librte_ether/rte_ethdev.c               | 114 ++++++++++++++++++++++++--
>  lib/librte_ether/rte_ethdev.h               |  38 +++++++--
>  mk/rte.app.mk                               |   1 +
>  53 files changed, 776 insertions(+), 343 deletions(-)
>
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH v2 8/8] ethdev: Decouple interrupt handling from PCI device
From: Shreyansh Jain @ 2016-12-20 12:45 UTC (permalink / raw)
  To: Jan Blunck; +Cc: dev, David Marchand
In-Reply-To: <CALe+Z01Nv12g41v4Fpe=ddSNxru39Y=VXpjF+D9WRbpDF2AJLg@mail.gmail.com>

On Tuesday 20 December 2016 04:21 PM, Jan Blunck wrote:
> On Tue, Nov 22, 2016 at 1:57 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> On Monday 21 November 2016 10:25 PM, Jan Blunck wrote:
>>>
>>> The struct rte_intr_handle is an abstraction layer for different types of
>>> interrupt mechanisms. It is embedded in the low-level device (e.g. PCI).
>>> On allocation of a struct rte_eth_dev a reference to the intr_handle
>>> should be stored for devices supporting interrupts.
>>>
>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>> ---
>>>  lib/librte_ether/rte_ethdev.c | 18 ++++++++++++++++--
>>>  lib/librte_ether/rte_ethdev.h |  1 +
>>>  2 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
>>> index 4288577..4ecea50 100644
>>> --- a/lib/librte_ether/rte_ethdev.c
>>> +++ b/lib/librte_ether/rte_ethdev.c
>>> @@ -258,6 +258,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>>>                         rte_panic("Cannot allocate memzone for private
>>> port data\n");
>>>         }
>>>         eth_dev->pci_dev = pci_dev;
>>> +       eth_dev->intr_handle = &pci_dev->intr_handle;

(#) See below.

>>>         eth_dev->driver = eth_drv;
>>>         eth_dev->data->rx_mbuf_alloc_failed = 0;
>>>
>>> @@ -2543,7 +2544,13 @@ 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];
>>> -       intr_handle = &dev->pci_dev->intr_handle;
>>> +
>>> +       if (!dev->intr_handle) {
>>> +               RTE_PMD_DEBUG_TRACE("RX Intr handle unset\n");
>>> +               return -ENOTSUP;
>>> +       }
>>> +
>>> +       intr_handle = dev->intr_handle;
>>>         if (!intr_handle->intr_vec) {
>>>                 RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>>>                 return -EPERM;
>>> @@ -2603,7 +2610,12 @@ rte_eth_dev_rx_intr_ctl_q(uint8_t port_id, uint16_t
>>> queue_id,
>>>                 return -EINVAL;
>>>         }
>>>
>>> -       intr_handle = &dev->pci_dev->intr_handle;
>>> +       if (!dev->intr_handle) {
>>> +               RTE_PMD_DEBUG_TRACE("RX Intr handle unset\n");
>>> +               return -ENOTSUP;
>>> +       }
>>> +
>>> +       intr_handle = dev->intr_handle;
>>>         if (!intr_handle->intr_vec) {
>>>                 RTE_PMD_DEBUG_TRACE("RX Intr vector unset\n");
>>>                 return -EPERM;
>>> @@ -3205,6 +3217,8 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
>>> struct rte_pci_device *pci_de
>>>                 return;
>>>         }
>>>
>>> +       eth_dev->intr_handle = &pci_dev->intr_handle;
>>> +
>>>         eth_dev->data->dev_flags = 0;
>>>         if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
>>>                 eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
>>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
>>> index 3adbb2b..f1f656a 100644
>>> --- a/lib/librte_ether/rte_ethdev.h
>>> +++ b/lib/librte_ether/rte_ethdev.h
>>> @@ -1629,6 +1629,7 @@ struct rte_eth_dev {
>>>         const struct eth_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_intr_handle *intr_handle; /**< Device interrupt handle
>>> */
>>>         /** User application callbacks for NIC interrupts */
>>>         struct rte_eth_dev_cb_list link_intr_cbs;
>>>         /**
>>>
>>
>> Is there another patch which replaces all uses of
>> eth_dev->pci_dev->intr_handle with that of eth_dev->intr_handle?
>>
>> Now that eth_dev has a reference which is initialized as early as probe, we
>> should use that rather than pci_dev->intr_handle for PCI PMDs.
>>
>
> I've added this indirection because it is required for the ethdev
> function. The drivers shouldn't use the indirection through ethdev to
> access the intr_handle because they do have direct access to the
> device.

This is my mistake - I posted my review at a wrong location. It should 
have been at (#) above.

My intention was that we should use ETH_DEV_INTR_HANDLE like macro for 
accessing intr_handle.

Anyways, I see that you have already posted v3. I will review that. You 
can consider this comment as retracted from my side.

>
>
>> OR maybe, ETH_DEV_INTR_HANDLE() like macro which you have introduced in
>> i40e_ethdev.h.
>>
>> -
>> Shreyansh
>

-
Shreyansh

^ permalink raw reply

* Re: [PATCH v3 4/9] virtio: Don't fill dev_info->driver_name
From: Jan Blunck @ 2016-12-20 12:40 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, Shreyansh Jain, David Marchand, Stephen Hemminger
In-Reply-To: <dfdd8c5c-2b15-e9e4-03f6-805ce4788442@intel.com>

On Tue, Dec 20, 2016 at 1:17 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 12/20/2016 11:11 AM, Jan Blunck wrote:
>> This is overwritten in rte_eth_dev_info_get().
>>
>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>> Reviewed-by: David Marchand <david.marchand@6wind.com>
>> ---
>>  drivers/net/virtio/virtio_ethdev.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index 079fd6c..741688e 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -1624,10 +1624,6 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>>       uint64_t tso_mask;
>>       struct virtio_hw *hw = dev->data->dev_private;
>>
>> -     if (dev->pci_dev)
>> -             dev_info->driver_name = dev->driver->pci_drv.driver.name;
>> -     else
>> -             dev_info->driver_name = "virtio_user PMD";
>>       dev_info->max_rx_queues =
>>               RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
>>       dev_info->max_tx_queues =
>>
>
>
> This has been already removed in next-net with following commit:
> 7c6c1857358c ("net: remove dead driver names")

Thanks. Do you want me to rebase against next-net and resend the
series before continuing to review further?

^ permalink raw reply

* Re: [PATCH v3 02/12] eal/bus: introduce bus abstraction
From: Hemant Agrawal @ 2016-12-20 12:37 UTC (permalink / raw)
  To: Shreyansh Jain, dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu
In-Reply-To: <1481893853-31790-3-git-send-email-shreyansh.jain@nxp.com>

On 12/16/2016 6:40 PM, Shreyansh Jain wrote:
> This patch introduces the rte_bus abstraction for devices and drivers in
> EAL framework. The model is:
>  - One or more buses are connected to a CPU (or core)
>  - One or more devices are conneted to a Bus
>  - Drivers are running instances which manage one or more devices
>  - Bus is responsible for identifying devices (and interrupt propogation)
>  - Driver is responsible for initializing the device
>
> This patch adds a 'rte_bus' class which rte_driver and rte_device refer.
> This way, each device (rte_xxx_device) would have reference to the bus
> it is based on. As well as, each driver (rte_xxx_driver) would have link
> to the bus and devices on it for servicing.
>
>                                   __ rte_bus_list
>                                  /
>                      +----------'---+
>                      |rte_bus       |
>                      | driver_list------> List of rte_bus specific
>                      | device_list----    devices
>                      |              | `-> List of rte_bus associated
>                      |              |     drivers
>                      +--|------|----+
>               _________/        \_________
>     +--------/----+                     +-\---------------+
>     |rte_device   |                     |rte_driver       |
>     | rte_bus     |                     | rte_bus         |
>     | rte_driver  |                     | ...             |
>     | ...         |                     +---------...-----+
>     |             |                               |||
>     +---||--------+                               |||
>         ||                                        |||
>         | \                                        \\\
>         |  \_____________                           \\\
>         |                \                          |||
>  +------|---------+ +----|----------+               |||
>  |rte_pci_device  | |rte_xxx_device |               |||
>  | ....           | | ....          |               |||
>  +----------------+ +---------------+              / | \
>                                                   /  |  \
>                             _____________________/  /    \
>                            /                    ___/      \
>             +-------------'--+    +------------'---+    +--'------------+
>             |rte_pci_driver  |    |rte_vdev_driver |    |rte_xxx_driver |
>             | ....           |    | ....           |    | ....          |
>             +----------------+    +----------------+    +---------------+
>
> This patch only enables the bus references on rte_driver and rte_driver.
> EAL wide global device and driver list continue to exist until an instance
> of bus is added in subsequent patches.
>
> This patch also introduces RTE_REGISTER_BUS macro on the lines of
> RTE_PMD_REGISTER_XXX. Key difference is that the constructor priority has
> been explicitly set to 101 so as to execute bus registration before PMD.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> --
> v2:
>  - fix bsdapp compilation issue because of missing export symbols in map
>    file
> ---
>  lib/librte_eal/bsdapp/eal/Makefile              |   1 +
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  15 ++
>  lib/librte_eal/common/Makefile                  |   2 +-
>  lib/librte_eal/common/eal_common_bus.c          | 192 ++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 174 +++++++++++++++++++++
>  lib/librte_eal/common/include/rte_dev.h         |   2 +
>  lib/librte_eal/linuxapp/eal/Makefile            |   1 +
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  15 ++
>  8 files changed, 401 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_eal/common/eal_common_bus.c
>  create mode 100644 lib/librte_eal/common/include/rte_bus.h
>
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index a15b762..cce99f7 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_cpuflags.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_thread.c
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2f81f7c..23fc1c1 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -174,3 +174,18 @@ DPDK_16.11 {
>  	rte_eal_vdrv_unregister;
>
>  } DPDK_16.07;
> +
> +DPDK_17.02 {
> +	global:
> +
> +	rte_bus_list;
> +	rte_eal_bus_add_device;
> +	rte_eal_bus_add_driver;
> +	rte_eal_get_bus;
> +	rte_eal_bus_dump;
> +	rte_eal_bus_register;
> +	rte_eal_bus_remove_device;
> +	rte_eal_bus_remove_driver;
> +	rte_eal_bus_unregister;
> +
> +} DPDK_16.11;
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index a92c984..0c39414 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -38,7 +38,7 @@ INC += rte_per_lcore.h rte_random.h
>  INC += rte_tailq.h rte_interrupts.h rte_alarm.h
>  INC += rte_string_fns.h rte_version.h
>  INC += rte_eal_memconfig.h rte_malloc_heap.h
> -INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
> +INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_vdev.h
>  INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
>  INC += rte_malloc.h rte_keepalive.h rte_time.h
>
> 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..612f64e
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -0,0 +1,192 @@
> +/*-
> + *   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_bus.h>
> +#include <rte_devargs.h>
> +#include <rte_debug.h>
> +
> +#include "eal_private.h"
> +
> +struct rte_bus_list rte_bus_list =
> +	TAILQ_HEAD_INITIALIZER(rte_bus_list);
> +
> +/** @internal
> + * Add a device to a bus.
> + */
> +void
> +rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
> +{
> +	RTE_VERIFY(bus);
> +	RTE_VERIFY(dev);
> +
> +	TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
> +	dev->bus = bus;
> +}
> +
> +/** @internal
> + * Remove a device from its bus.
> + */
> +void
> +rte_eal_bus_remove_device(struct rte_device *dev)
> +{
> +	struct rte_bus *bus;
> +
> +	RTE_VERIFY(dev);
> +	RTE_VERIFY(dev->bus);
> +
> +	bus = dev->bus;
> +	TAILQ_REMOVE(&bus->device_list, dev, next);
> +	dev->bus = NULL;
> +}
> +
> +/** @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);
> +	drv->bus = bus;
> +}
> +
> +/** @internal
> + * Disassociate a driver from bus.
> + */
> +void
> +rte_eal_bus_remove_driver(struct rte_driver *drv)
> +{
> +	struct rte_bus *bus;
> +
> +	RTE_VERIFY(drv);
> +	RTE_VERIFY(drv->bus);
> +
> +	bus = drv->bus;
> +	TAILQ_REMOVE(&bus->driver_list, drv, next);
> +	drv->bus = NULL;
> +}
> +
> +/**
> + * 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 (!strcmp(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)
> +{
> +	RTE_VERIFY(bus);
> +	RTE_VERIFY(bus->name && strlen(bus->name));
> +
> +	/* Initialize the driver and device list associated with the bus */
> +	TAILQ_INIT(&(bus->driver_list));
> +	TAILQ_INIT(&(bus->device_list));
> +
> +	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
> +	RTE_LOG(INFO, 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)));
> +
> +	/* TODO: For each device, call its rte_device->driver->remove()
> +	 * and rte_eal_bus_remove_driver()
> +	 */
> +
> +	TAILQ_REMOVE(&rte_bus_list, bus, next);
> +	RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
> +}
> +
> +/* dump one bus info */
> +static int
> +bus_dump_one(FILE *f, struct rte_bus *bus)
> +{
> +	int ret;
> +
> +	/* For now, dump only the bus name */
> +	ret = fprintf(f, " %s\n", bus->name);
> +
> +	/* Error in case of inability in writing to stream */
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +void
> +rte_eal_bus_dump(FILE *f)
> +{
> +	int ret;
> +	struct rte_bus *bus;
> +
> +	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +		ret = bus_dump_one(f, bus);
> +		if (ret) {
> +			RTE_LOG(ERR, EAL, "Unable to write to stream (%d)\n",
> +				ret);
> +			break;
> +		}
> +	}
> +}
> 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..f0297a9
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -0,0 +1,174 @@
> +/*-
> + *   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);
> +
> +/* Global Bus list */
> +extern struct rte_bus_list rte_bus_list;
> +
> +struct rte_bus {
> +	TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
> +	struct rte_driver_list driver_list;
> +				     /**< List of all drivers on bus */
> +	struct rte_device_list device_list;
> +				     /**< List of all devices on bus */
> +	const char *name;            /**< Name of the 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);
> +
> +/** @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);
> +
> +/** @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);
> +
> +/**
> + * Dump information of all the buses registered with EAL.
> + *
> + * @param f
> + *	A valid and open output stream handle
> + *
> + * @return
> + *	 0 in case of success
> + *	!0 in case there is error in opening the output stream
> + */
> +void rte_eal_bus_dump(FILE *f);
> +
> +/** Helper for Bus registration. The constructor has higher priority than
> + * PMD constructors
> + */
> +#define RTE_REGISTER_BUS(nm, bus) \
> +static void __attribute__((constructor(101), used)) 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..4004f9a 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -122,6 +122,7 @@ struct rte_driver;
>   */
>  struct rte_device {
>  	TAILQ_ENTRY(rte_device) next; /**< Next device */
> +	struct rte_bus *bus;          /**< Device connected to this bus */
>  	struct rte_driver *driver;    /**< Associated driver */
>  	int numa_node;                /**< NUMA node connection */
>  	struct rte_devargs *devargs;  /**< Device user arguments */
> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>   */
>  struct rte_driver {
>  	TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> +	struct rte_bus *bus;           /**< Bus serviced by this driver */
>  	const char *name;                   /**< Driver name. */
>  	const char *alias;              /**< Driver alias. */
>  };
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 4e206f0..aa874a5 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_cpuflags.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_thread.c
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 83721ba..c873a7f 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -178,3 +178,18 @@ DPDK_16.11 {
>  	rte_eal_vdrv_unregister;
>
>  } DPDK_16.07;
> +
> +DPDK_17.02 {
> +	global:
> +
> +	rte_bus_list;
> +	rte_eal_bus_add_device;
> +	rte_eal_bus_add_driver;
> +	rte_eal_get_bus;

This should be rte_eal_bus_get

Also you are missing rte_eal_bus_insert_device

> +	rte_eal_bus_dump;
> +	rte_eal_bus_register;
> +	rte_eal_bus_remove_device;
> +	rte_eal_bus_remove_driver;
> +	rte_eal_bus_unregister;
> +
> +} DPDK_16.11;
>

^ permalink raw reply


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