[dpdk-dev] [PATCH v2] ena: fix doorbell submission when not needed
On Fri, Jul 08, 2016 at 01:11:30PM +0200, Jan Medala wrote:
> Avoid submitting doorbell when:
> * no packets have been submitted to TX
> * no free resources have been submitted while RX
>
> Sending doorbell without actual work to be performed by device
> violates ENA specification and can lead to unpredictable behavior.
>
> Fixes: 1173fca25af9 ("ena: add polling-mode driver")
>
> Signed-off-by: Alexander Matushevsky
> Signed-off-by: Jan Medala
> ---
Applied to dpdk-net-net/rel_16_07
/Bruce
[dpdk-dev] [PATCH 1/2] net/enic: fix setting MAC address when a port is restarted
On Thu, Jul 07, 2016 at 06:10:21PM -0700, Nelson Escobar wrote:
> enic_disable() removed the MAC address when a port was shut down but
> enic_enable() didn't add the MAC address back when the port was
> started again. Move where we set the MAC address for the adapter from
> enic_setup_finish() to a enic_enable() so that port restarting works
> properly.
>
> Fixes: fefed3d1e62c ("enic: new driver")
>
> Signed-off-by: Nelson Escobar
> Reviewed-by: John Daley
> ---
Patchset applied to dpdk-next-net/rel_16_07
/Bruce
[dpdk-dev] [PATCH 2/2] bnx2x: add support for xstats
On Fri, Jul 08, 2016 at 11:36:58AM +0100, Remy Horton wrote: > > On 07/07/2016 23:50, Rasesh Mody wrote: > >This patch adds support for extended statistics for BNX2X PMD. > > > >Signed-off-by: Rasesh Mody > > Acked-by: Remy Horton Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH 1/2] qede: add support for xstats
On Fri, Jul 08, 2016 at 11:36:54AM +0100, Remy Horton wrote: > > On 07/07/2016 23:50, Rasesh Mody wrote: > >This patch adds support for extended statistics for QEDE PMD. > > > >Signed-off-by: Rasesh Mody > > Acked-by: Remy Horton Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH] net/mlx5: fix a segmentation fault in Rx
On Fri, Jul 08, 2016 at 03:29:02PM +0200, Adrien Mazarguil wrote:
> On Fri, Jul 08, 2016 at 02:43:26PM +0200, Nelio Laranjeiro wrote:
> > Fixed issue could occur when a Mbuf starvation happens in a middle of
> > reception of a segmented packet, in such situation, the PMD has to release
> > all segments of such packet. The end condition was wrong causing it to
> > free a Mbuf still handled by the NIC.
> >
> > Fixes: 9964b965ad69 ("net/mlx5: re-add Rx scatter support")
> >
> > Reported-by: Yongseok Koh
> > Signed-off-by: Nelio Laranjeiro
> > ---
> > drivers/net/mlx5/mlx5_rxtx.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> > index 0c352f3..3564937 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.c
> > +++ b/drivers/net/mlx5/mlx5_rxtx.c
> > @@ -1572,7 +1572,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
> > uint16_t pkts_n)
> > rte_prefetch0(wqe);
> > rep = rte_mbuf_raw_alloc(rxq->mp);
> > if (unlikely(rep == NULL)) {
> > - while (pkt) {
> > + while (pkt != seg) {
> > + assert(pkt != (*rxq->elts)[idx]);
> > seg = NEXT(pkt);
> > rte_mbuf_refcnt_set(pkt, 0);
> > __rte_mbuf_raw_free(pkt);
> > --
> > 2.1.4
> >
>
> Acked-by: Adrien Mazarguil
>
Applied to dpdk-next-net/rel_16_07
/Bruce
[dpdk-dev] [PATCH] net/mlx5: fix packet type and offload flags in Rx
On Fri, Jul 08, 2016 at 09:09:32AM +0200, Adrien Mazarguil wrote:
> On Thu, Jul 07, 2016 at 06:34:45PM +0200, Maxime Leroy wrote:
> > In mlx5 rx function, the packet_type and ol_flags mbuf fields are not
> > properly initialized when no rx offload feature is enabled (checksum, l2
> > tun checksum, vlan_strip, crc). Thus, these fields can have a value
> > different of 0 depending on their value when the mbuf was freed.
> >
> > This can result in an incorrect application behavior if invalid
> > ol_flags/ptype are set, or memory corruptions if IND_ATTACHED_MBUF is
> > set in ol_flags.
> >
> > Fixes: 081f7eae242e ("mlx5: process offload flags only when requested")
> >
> > Signed-off-by: Maxime Leroy
> > ---
> > drivers/net/mlx5/mlx5_rxtx.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> > index 0c352f3f..4132fd74 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.c
> > +++ b/drivers/net/mlx5/mlx5_rxtx.c
> > @@ -1599,6 +1599,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
> > uint16_t pkts_n)
> > pkt = seg;
> > assert(len >= (rxq->crc_present << 2));
> > /* Update packet information. */
> > + pkt->packet_type = 0;
> > + pkt->ol_flags = 0;
> > if (rxq->csum | rxq->csum_l2tun | rxq->vlan_strip |
> > rxq->crc_present) {
> > if (rxq->csum) {
> > --
> > 2.1.4
> >
>
> Acked-by: Adrien Mazarguil
>
Applied to dpdk-next-net/rel_16_07
/Bruce
[dpdk-dev] [PATCH] ixgbe: support checksum flags in sse vector Rx function
On Thu, Jul 07, 2016 at 02:19:02PM +0200, Olivier Matz wrote:
> Update desc_to_olflags_v() to set PKT_RX_IP_CKSUM_BAD and
> PKT_RX_L4_CKSUM_BAD in the ol_fags of the mbuf.
>
> The Rx vector function can now be used with hw_ip_checksum
> enabled.
>
> Tested with:
>
> cd dpdk.org/
> make config T=x86_64-native-linuxapp-gcc
> make -j32
> mkdir -p /mnt/huge
> mount -t hugetlbfs nodev /mnt/huge
> echo 256 >
> /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
> modprobe uio_pci_generic
> python tools/dpdk_nic_bind.py -b uio_pci_generic :04:00.0
> ./build/app/testpmd -l 2,4 -- --total-num-mbufs=65536 -i
> --port-topology=chained --enable-rx-cksum --disable-hw-vlan-filter
> --disable-hw-vlan-strip
> set fwd rxonly
> set verbose 1
> start
>
> # send packets to testpmd using scapy
> eh = Ether(src="00:01:02:03:04:05", dst="00:1B:21:AB:8F:10")
> p = Ether()/IP(src="1.1.1.1", dst="1.1.1.2")/UDP()/Raw("x"*50)
> sendp(p, iface="ixgbe2")
> p = Ether()/IP(src="1.1.1.1", dst="1.1.1.2", chksum=42)/UDP()/Raw("x"*50)
> sendp(p, iface="ixgbe2")
> p = Ether()/IP(src="1.1.1.1", dst="1.1.1.2")/UDP(chksum=42)/Raw("x"*50)
> sendp(p, iface="ixgbe2")
> p = Ether()/IP(src="1.1.1.1", dst="1.1.1.2",
> chksum=45)/UDP(chksum=42)/Raw("x"*50)
> sendp(p, iface="ixgbe2")
>
> # result
> port 0/queue 0: received 1 packets
> src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=92 -
> nb_segs=1Unknown packet type
>- Receive queue=0x0
> port 0/queue 0: received 1 packets
> src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=92 -
> nb_segs=1Unknown packet type
>- Receive queue=0x0
> PKT_RX_IP_CKSUM_BAD
> port 0/queue 0: received 1 packets
> src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=92 -
> nb_segs=1Unknown packet type
>- Receive queue=0x0
> PKT_RX_L4_CKSUM_BAD
> port 0/queue 0: received 1 packets
> src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=92 -
> nb_segs=1Unknown packet type
>- Receive queue=0x0
> PKT_RX_L4_CKSUM_BAD
> PKT_RX_IP_CKSUM_BAD
>
> Signed-off-by: Maxime Leroy
> Signed-off-by: Olivier Matz
> ---
Given this looks a significant change to a complicated piece of code, which is
also very performance sensitive, I believe this change needs to be deferred to
16.11 release, as there is not enough time to properly review and test it for
16.07.
/Bruce
[dpdk-dev] [PATCH 2/2] scripts: fix pmdinfo for FreeBSD
There were a couple of issues which prevented pmdinfo.py from running on
FreeBSD, both of which are fixed by this patch.
* The path to python is not /usr/bin/python as on Linux, so use
/usr/bin/env to find it on both OS's.
* The path to the pci ids DB is in a different location on FreeBSD,
so use the platform python library to look in different default
locations depending on the underlying OS. [There are two possible
locations to look on FreeBSD, as defined by pciconf manpage, so
check in both in order of better to worse]
Fixes: c67c9a5c646a ("tools: query binaries for HW and other support
information")
Signed-off-by: Bruce Richardson
---
tools/pmdinfo.py | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/pmdinfo.py b/tools/pmdinfo.py
index e531154..662034a 100755
--- a/tools/pmdinfo.py
+++ b/tools/pmdinfo.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
# -
# scripts/pmdinfo.py
#
@@ -10,6 +10,7 @@ import sys
from optparse import OptionParser
import string
import json
+import platform
# For running from development directory. It should take precedence over the
# installed pyelftools.
@@ -557,6 +558,14 @@ def main(stream=None):
global raw_output
global pcidb
+pcifile_default = "./pci.ids" # for unknown OS's assume local file
+if platform.system() == 'Linux':
+pcifile_default = "/usr/share/hwdata/pci.ids"
+elif platform.system() == 'FreeBSD':
+pcifile_default = "/usr/local/share/pciids/pci.ids"
+if not os.path.exists(pcifile_default):
+pcifile_default = "/usr/share/misc/pci_vendors"
+
optparser = OptionParser(
usage='usage: %prog [-hrtp] [-d ',
description="Dump pmd hardware support info",
@@ -568,7 +577,7 @@ def main(stream=None):
optparser.add_option("-d", "--pcidb", dest="pcifile",
help="specify a pci database "
"to get vendor names from",
- default="/usr/share/hwdata/pci.ids", metavar="FILE")
+ default=pcifile_default, metavar="FILE")
optparser.add_option("-t", "--table", dest="tblout",
help="output information on hw support as a hex
table",
action='store_true')
--
2.7.4
[dpdk-dev] [PATCH 1/2] pmdinfogen: fix build on FreeBSD
error on compilation caused by missing include for libgen.h.
HOSTCC pmdinfogen.o
/usr/home/bruce/dpdk/buildtools/pmdinfogen/pmdinfogen.c:402:4: error: implicit
declaration of function 'basename' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
basename(argv[0]));
Fixes: 840e5dfea3f8 ("pmdinfogen: fix usage message")
Signed-off-by: Bruce Richardson
---
buildtools/pmdinfogen/pmdinfogen.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/buildtools/pmdinfogen/pmdinfogen.c
b/buildtools/pmdinfogen/pmdinfogen.c
index 717c8d4..e1bf2e4 100644
--- a/buildtools/pmdinfogen/pmdinfogen.c
+++ b/buildtools/pmdinfogen/pmdinfogen.c
@@ -15,6 +15,8 @@
#include
#include
#include
+#include
+
#include
#include "pmdinfogen.h"
--
2.7.4
[dpdk-dev] [PATCH 0/2] enable pmdinfo support on FreeBSD
This patchset fixes pmdinfogen and pmdinfo.py so that they can work on FreeBSD. It's been tested with static binaries compiled using clang and gcc on FreeBSD, as well as gcc-generated dynamic libraries. For some reason clang wasn't able to build dynamic libraries for DPDK on FreeBSD so that is untested. [Investigation of that issue is for another time] Bruce Richardson (2): pmdinfogen: fix build on FreeBSD scripts: fix pmdinfo for FreeBSD buildtools/pmdinfogen/pmdinfogen.c | 2 ++ tools/pmdinfo.py | 13 +++-- 2 files changed, 13 insertions(+), 2 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v1 15/15] eal/pci: insert rte_device on scan
Signed-off-by: Jan Viktorin
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 6f1a28a..ab08a16 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -390,6 +390,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t
bus,
/* device is valid, add in list (sorted) */
if (TAILQ_EMPTY(_device_list)) {
+ rte_eal_device_insert(>device);
TAILQ_INSERT_TAIL(_device_list, dev, next);
} else {
struct rte_pci_device *dev2;
@@ -402,6 +403,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t
bus,
if (ret < 0) {
TAILQ_INSERT_BEFORE(dev2, dev, next);
+ rte_eal_device_insert(>device);
} else { /* already registered */
dev2->kdrv = dev->kdrv;
dev2->max_vfs = dev->max_vfs;
@@ -411,6 +413,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t
bus,
}
return 0;
}
+ rte_eal_device_insert(>device);
TAILQ_INSERT_TAIL(_device_list, dev, next);
}
--
2.9.0
[dpdk-dev] [PATCH v1 14/15] eal/pci: inherit rte_device by rte_pci_device
Signed-off-by: Jan Viktorin
---
app/test/virtual_pmd.c | 4 ++--
drivers/net/fm10k/fm10k_ethdev.c| 6 +++---
drivers/net/virtio/virtio_pci.c | 2 +-
lib/librte_cryptodev/rte_cryptodev.c| 2 +-
lib/librte_eal/common/eal_common_pci.c | 14 +++---
lib/librte_eal/common/include/rte_pci.h | 4 +---
lib/librte_eal/linuxapp/eal/eal_pci.c | 4 ++--
lib/librte_ether/rte_ethdev.c | 14 +-
8 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index 56eeb99..4831113 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -585,7 +585,7 @@ virtual_ethdev_create(const char *name, struct ether_addr
*mac_addr,
if (eth_dev == NULL)
goto err;
- pci_dev->numa_node = socket_id;
+ pci_dev->device.numa_node = socket_id;
pci_drv->driver.name = virtual_ethdev_driver_name;
pci_drv->id_table = id_table;
@@ -626,7 +626,7 @@ virtual_ethdev_create(const char *name, struct ether_addr
*mac_addr,
eth_dev->dev_ops = _private->dev_ops;
eth_dev->pci_dev = pci_dev;
- eth_dev->pci_dev->driver = _drv->pci_drv;
+ eth_dev->pci_dev->device.driver = _drv->pci_drv.driver;
eth_dev->rx_pkt_burst = virtual_ethdev_rx_burst_success;
eth_dev->tx_pkt_burst = virtual_ethdev_tx_burst_success;
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 4ffa66c..4230a75 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -675,7 +675,7 @@ fm10k_dev_tx_init(struct rte_eth_dev *dev)
/* Enable use of FTAG bit in TX descriptor, PFVTCTL
* register is read-only for VF.
*/
- if (fm10k_check_ftag(dev->pci_dev->devargs)) {
+ if (fm10k_check_ftag(dev->pci_dev->device.devargs)) {
if (hw->mac.type == fm10k_mac_pf) {
FM10K_WRITE_REG(hw, FM10K_PFVTCTL(i),
FM10K_PFVTCTL_FTAG_DESC_ENABLE);
@@ -2734,7 +2734,7 @@ fm10k_set_tx_function(struct rte_eth_dev *dev)
int use_sse = 1;
uint16_t tx_ftag_en = 0;
- if (fm10k_check_ftag(dev->pci_dev->devargs))
+ if (fm10k_check_ftag(dev->pci_dev->device.devargs))
tx_ftag_en = 1;
for (i = 0; i < dev->data->nb_tx_queues; i++) {
@@ -2767,7 +2767,7 @@ fm10k_set_rx_function(struct rte_eth_dev *dev)
uint16_t i, rx_using_sse;
uint16_t rx_ftag_en = 0;
- if (fm10k_check_ftag(dev->pci_dev->devargs))
+ if (fm10k_check_ftag(dev->pci_dev->device.devargs))
rx_ftag_en = 1;
/* In order to allow Vector Rx there are a few configuration
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 9c9a3dd..6fbfa39 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -719,7 +719,7 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
if (legacy_virtio_resource_init(dev, hw, dev_flags) < 0) {
if (dev->kdrv == RTE_KDRV_UNKNOWN &&
- dev->devargs->type != RTE_DEVTYPE_WHITELISTED_PCI) {
+ dev->device.devargs->type != RTE_DEVTYPE_WHITELISTED_PCI) {
PMD_INIT_LOG(INFO,
"skip kernel managed virtio device.");
return 1;
diff --git a/lib/librte_cryptodev/rte_cryptodev.c
b/lib/librte_cryptodev/rte_cryptodev.c
index afb13d7..d2a2a60 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -401,7 +401,7 @@ rte_cryptodev_pci_remove(struct rte_pci_device *pci_dev)
if (cryptodev == NULL)
return -ENODEV;
- cryptodrv = (const struct rte_cryptodev_driver *)pci_dev->driver;
+ cryptodrv = (const struct rte_cryptodev_driver
*)to_pci_driver(pci_dev->device.driver);
if (cryptodrv == NULL)
return -ENODEV;
diff --git a/lib/librte_eal/common/eal_common_pci.c
b/lib/librte_eal/common/eal_common_pci.c
index 144479e..66ceba6 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -183,11 +183,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
struct rte_pci_device *d
RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket
%i\n",
loc->domain, loc->bus, loc->devid,
loc->function,
- dev->numa_node);
+ dev->device.numa_node);
/* no initialization when blacklisted, return without error */
- if (dev->devargs != NULL &&
- dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
+ if (dev->device.devargs != NULL &&
[dpdk-dev] [PATCH v1 13/15] eal: introduce rte_device
Signed-off-by: Jan Viktorin
---
lib/librte_eal/common/eal_common_dev.c | 13 +
lib/librte_eal/common/include/rte_dev.h | 31 +++
2 files changed, 44 insertions(+)
diff --git a/lib/librte_eal/common/eal_common_dev.c
b/lib/librte_eal/common/eal_common_dev.c
index f4c880c..2c2a1bd 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -48,6 +48,9 @@
/** Global list of device drivers. */
static struct rte_driver_list dev_driver_list =
TAILQ_HEAD_INITIALIZER(dev_driver_list);
+/** Global list of device drivers. */
+static struct rte_device_list dev_device_list =
+ TAILQ_HEAD_INITIALIZER(dev_device_list);
/* register a driver */
void
@@ -63,6 +66,16 @@ rte_eal_driver_unregister(struct rte_driver *driver)
TAILQ_REMOVE(_driver_list, driver, next);
}
+void rte_eal_device_insert(struct rte_device *dev)
+{
+ TAILQ_INSERT_TAIL(_device_list, dev, next);
+}
+
+void rte_eal_device_remove(struct rte_device *dev)
+{
+ TAILQ_REMOVE(_device_list, dev, next);
+}
+
int
rte_eal_dev_init(void)
{
diff --git a/lib/librte_eal/common/include/rte_dev.h
b/lib/librte_eal/common/include/rte_dev.h
index 2a0d326..d767012 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -112,6 +112,37 @@ struct rte_mem_resource {
/** Double linked list of device drivers. */
TAILQ_HEAD(rte_driver_list, rte_driver);
+/** Double linked list of devices. */
+TAILQ_HEAD(rte_device_list, rte_device);
+
+/* Forward declaration */
+struct rte_driver;
+
+/**
+ * A structure describing a generic device.
+ */
+struct rte_device {
+ TAILQ_ENTRY(rte_device) next; /**< Next device */
+ struct rte_driver *driver;/**< Associated driver */
+ int numa_node;/**< NUMA node connection */
+ struct rte_devargs *devargs; /**< Device user arguments */
+};
+
+/**
+ * Insert a device detected by a bus scanning.
+ *
+ * @param dev
+ * A pointer to a rte_device structure describing the detected device.
+ */
+void rte_eal_device_insert(struct rte_device *dev);
+
+/**
+ * Remove a device (e.g. when being unplugged).
+ *
+ * @param dev
+ * A pointer to a rte_device structure describing the device to be removed.
+ */
+void rte_eal_device_remove(struct rte_device *dev);
/**
* A structure describing a device driver.
--
2.9.0
[dpdk-dev] [PATCH v1 12/15] eal: call rte_eal_driver_register
To register both vdev and pci drivers into the list of all rte_driver,
we have to call rte_eal_driver_register explicitly.
Signed-off-by: Jan Viktorin
---
lib/librte_eal/common/eal_common_pci.c | 2 ++
lib/librte_eal/common/eal_common_vdev.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/librte_eal/common/eal_common_pci.c
b/lib/librte_eal/common/eal_common_pci.c
index 3bcb059..144479e 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -468,11 +468,13 @@ void
rte_eal_pci_register(struct rte_pci_driver *driver)
{
TAILQ_INSERT_TAIL(_driver_list, driver, next);
+ rte_eal_driver_register(>driver);
}
/* unregister a driver */
void
rte_eal_pci_unregister(struct rte_pci_driver *driver)
{
+ rte_eal_driver_unregister(>driver);
TAILQ_REMOVE(_driver_list, driver, next);
}
diff --git a/lib/librte_eal/common/eal_common_vdev.c
b/lib/librte_eal/common/eal_common_vdev.c
index 7e7ddd1..389db3e 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -42,12 +42,14 @@ void
rte_eal_vdrv_register(struct rte_vdev_driver *driver)
{
TAILQ_INSERT_TAIL(_driver_list, driver, next);
+ rte_eal_driver_register(>driver);
}
/* unregister a driver */
void
rte_eal_vdrv_unregister(struct rte_vdev_driver *driver)
{
+ rte_eal_driver_unregister(>driver);
TAILQ_REMOVE(_driver_list, driver, next);
}
--
2.9.0
[dpdk-dev] [PATCH v1 11/15] eal/pci: inherit rte_driver by rte_pci_driver
Signed-off-by: Jan Viktorin
---
app/test/test_pci.c | 10 +++---
app/test/virtual_pmd.c | 2 +-
drivers/crypto/qat/rte_qat_cryptodev.c | 4 +++-
drivers/net/bnx2x/bnx2x_ethdev.c| 8 ++--
drivers/net/cxgbe/cxgbe_ethdev.c| 4 +++-
drivers/net/cxgbe/sge.c | 6 +++---
drivers/net/e1000/em_ethdev.c | 4 +++-
drivers/net/e1000/igb_ethdev.c | 8 ++--
drivers/net/ena/ena_ethdev.c| 4 +++-
drivers/net/enic/enic_ethdev.c | 4 +++-
drivers/net/fm10k/fm10k_ethdev.c| 4 +++-
drivers/net/i40e/i40e_ethdev.c | 4 +++-
drivers/net/i40e/i40e_ethdev_vf.c | 4 +++-
drivers/net/i40e/i40e_fdir.c| 2 +-
drivers/net/ixgbe/ixgbe_ethdev.c| 8 ++--
drivers/net/mlx4/mlx4.c | 4 +++-
drivers/net/mlx5/mlx5.c | 4 +++-
drivers/net/nfp/nfp_net.c | 4 +++-
drivers/net/qede/qede_ethdev.c | 4
drivers/net/szedata2/rte_eth_szedata2.c | 4 +++-
drivers/net/virtio/virtio_ethdev.c | 6 --
drivers/net/vmxnet3/vmxnet3_ethdev.c| 7 +--
drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
lib/librte_cryptodev/rte_cryptodev.c| 4 ++--
lib/librte_eal/common/eal_common_pci.c | 4 ++--
lib/librte_eal/common/include/rte_pci.h | 4 +++-
lib/librte_ether/rte_ethdev.c | 4 ++--
27 files changed, 89 insertions(+), 38 deletions(-)
diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index 354a0ad..9be08f9 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -78,14 +78,18 @@ struct rte_pci_id my_driver_id2[] = {
};
struct rte_pci_driver my_driver = {
- .name = "test_driver",
+ .driver = {
+ .name = "test_driver",
+ },
.devinit = my_driver_init,
.id_table = my_driver_id,
.drv_flags = 0,
};
struct rte_pci_driver my_driver2 = {
- .name = "test_driver2",
+ .driver = {
+ .name = "test_driver2",
+ },
.devinit = my_driver_init,
.id_table = my_driver_id2,
.drv_flags = 0,
@@ -95,7 +99,7 @@ static int
my_driver_init(__attribute__((unused)) struct rte_pci_driver *dr,
struct rte_pci_device *dev)
{
- printf("My driver init called in %s\n", dr->name);
+ printf("My driver init called in %s\n", dr->driver.name);
printf("%x:%x:%x.%d", dev->addr.domain, dev->addr.bus,
dev->addr.devid, dev->addr.function);
printf(" - vendor:%x device:%x\n", dev->id.vendor_id,
dev->id.device_id);
diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index 8a1f0d0..56eeb99 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -586,7 +586,7 @@ virtual_ethdev_create(const char *name, struct ether_addr
*mac_addr,
goto err;
pci_dev->numa_node = socket_id;
- pci_drv->name = virtual_ethdev_driver_name;
+ pci_drv->driver.name = virtual_ethdev_driver_name;
pci_drv->id_table = id_table;
if (isr_support)
diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c
b/drivers/crypto/qat/rte_qat_cryptodev.c
index 970970a..fab8bd8 100644
--- a/drivers/crypto/qat/rte_qat_cryptodev.c
+++ b/drivers/crypto/qat/rte_qat_cryptodev.c
@@ -114,7 +114,9 @@ crypto_qat_dev_init(__attribute__((unused)) struct
rte_cryptodev_driver *crypto_
static struct rte_cryptodev_driver rte_qat_pmd = {
.pci_drv = {
- .name = "rte_qat_pmd",
+ .driver = {
+ .name = "rte_qat_pmd",
+ },
.id_table = pci_id_qat_map,
.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.devinit = rte_cryptodev_pci_probe,
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 5ab3c75..062a93f 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -503,7 +503,9 @@ eth_bnx2xvf_dev_init(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_bnx2x_pmd = {
.pci_drv = {
- .name = "rte_bnx2x_pmd",
+ .driver = {
+ .name = "rte_bnx2x_pmd",
+ },
.id_table = pci_id_bnx2x_map,
.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.devinit = rte_eth_dev_pci_probe,
@@ -520,7 +522,9 @@ RTE_EAL_PCI_REGISTER(rte_bnx2x_pmd);
*/
static struct eth_driver rte_bnx2xvf_pmd = {
.pci_drv = {
- .name = "rte_bnx2xvf_pmd",
+ .driver = {
+ .name = "rte_bnx2xvf_pmd",
+ },
.id_table = pci_id_bnx2xvf_map,
.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.devinit = rte_eth_dev_pci_probe,
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 1389371..33934c3 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++
[dpdk-dev] [PATCH v1 10/15] eal: rename and move rte_pci_resource
There is no need to have a custom memory resource representation for
each infrastructure (PCI, ...) as it would always have the same members.
Signed-off-by: Jan Viktorin
---
drivers/net/szedata2/rte_eth_szedata2.c | 4 ++--
lib/librte_eal/common/include/rte_dev.h | 9 +
lib/librte_eal/common/include/rte_pci.h | 11 +--
lib/librte_eal/linuxapp/eal/eal_ivshmem.c | 2 +-
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c
b/drivers/net/szedata2/rte_eth_szedata2.c
index 6815dbb..d925bc6 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1416,7 +1416,7 @@ rte_szedata2_eth_dev_init(struct rte_eth_dev *dev)
int ret;
uint32_t szedata2_index;
struct rte_pci_addr *pci_addr = >pci_dev->addr;
- struct rte_pci_resource *pci_rsc =
+ struct rte_mem_resource *pci_rsc =
>pci_dev->mem_resource[PCI_RESOURCE_NUMBER];
char rsc_filename[PATH_MAX];
void *pci_resource_ptr = NULL;
@@ -1473,7 +1473,7 @@ rte_szedata2_eth_dev_init(struct rte_eth_dev *dev)
rte_eth_copy_pci_info(dev, dev->pci_dev);
- /* mmap pci resource0 file to rte_pci_resource structure */
+ /* mmap pci resource0 file to rte_mem_resource structure */
if (dev->pci_dev->mem_resource[PCI_RESOURCE_NUMBER].phys_addr ==
0) {
RTE_LOG(ERR, PMD, "Missing resource%u file\n",
diff --git a/lib/librte_eal/common/include/rte_dev.h
b/lib/librte_eal/common/include/rte_dev.h
index 225257e..2a0d326 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -100,6 +100,15 @@ rte_pmd_debug_trace(const char *func_name, const char
*fmt, ...)
} \
} while (0)
+/**
+ * A generic memory resource representation.
+ */
+struct rte_mem_resource {
+ uint64_t phys_addr; /**< Physical address, 0 if not resource. */
+ uint64_t len; /**< Length of the resource. */
+ void *addr; /**< Virtual address, NULL when not mapped. */
+};
+
/** Double linked list of device drivers. */
TAILQ_HEAD(rte_driver_list, rte_driver);
diff --git a/lib/librte_eal/common/include/rte_pci.h
b/lib/librte_eal/common/include/rte_pci.h
index ec77cbc..950ea89 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -111,15 +111,6 @@ const char *pci_get_sysfs_path(void);
/** Default sysfs path for PCI device search. */
#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
-/**
- * A structure describing a PCI resource.
- */
-struct rte_pci_resource {
- uint64_t phys_addr; /**< Physical address, 0 if no resource. */
- uint64_t len; /**< Length of the resource. */
- void *addr; /**< Virtual address, NULL when not mapped. */
-};
-
/** Maximum number of PCI resources. */
#define PCI_MAX_RESOURCE 6
@@ -163,7 +154,7 @@ struct rte_pci_device {
TAILQ_ENTRY(rte_pci_device) next; /**< Next probed PCI device. */
struct rte_pci_addr addr; /**< PCI location. */
struct rte_pci_id id; /**< PCI ID. */
- struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE]; /**< PCI
Memory Resource */
+ struct rte_mem_resource mem_resource[PCI_MAX_RESOURCE]; /**< PCI
Memory Resource */
struct rte_intr_handle intr_handle; /**< Interrupt handle */
struct rte_pci_driver *driver; /**< Associated driver */
uint16_t max_vfs; /**< sriov enable if not zero */
diff --git a/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
b/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
index 67b3caf..20f9413 100644
--- a/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
+++ b/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
@@ -834,7 +834,7 @@ rte_eal_ivshmem_obj_init(void)
int rte_eal_ivshmem_init(void)
{
struct rte_pci_device * dev;
- struct rte_pci_resource * res;
+ struct rte_mem_resource * res;
int fd, ret;
char path[PATH_MAX];
--
2.9.0
[dpdk-dev] [PATCH v1 08/15] eal: define macro container_of
Signed-off-by: Jan Viktorin
---
lib/librte_eal/common/include/rte_common.h | 16
1 file changed, 16 insertions(+)
diff --git a/lib/librte_eal/common/include/rte_common.h
b/lib/librte_eal/common/include/rte_common.h
index 332f2a4..a9b6792 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -322,6 +322,22 @@ rte_bsf32(uint32_t v)
#define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
#endif
+/**
+ * Return pointer to the wrapping struct instance.
+ * Example:
+ *
+ * struct wrapper {
+ * ...
+ * struct child c;
+ * ...
+ * };
+ *
+ * struct child *x = obtain(...);
+ * struct wrapper *w = container_of(x, struct wrapper, c);
+ */
+#define container_of(p, type, member) \
+ ((type *) (((char *) (p)) - offsetof(type, member)))
+
#define _RTE_STR(x) #x
/** Take a macro value and get a string version of it */
#define RTE_STR(x) _RTE_STR(x)
--
2.9.0
[dpdk-dev] [PATCH v1 07/15] eal: get rid of pmd_type
There is no need to determine a PMD type any more. The PMD_VDEV devices
has its own list of drivers. And all PMD_PDEV are PCI devices using
a different way of registering themselfs.
Signed-off-by: Jan Viktorin
---
drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 1 -
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 1 -
drivers/crypto/kasumi/rte_kasumi_pmd.c | 1 -
drivers/crypto/null/null_crypto_pmd.c | 1 -
drivers/crypto/snow3g/rte_snow3g_pmd.c | 1 -
drivers/net/af_packet/rte_eth_af_packet.c | 1 -
drivers/net/bonding/rte_eth_bond_pmd.c | 1 -
drivers/net/mpipe/mpipe_tilegx.c | 2 --
drivers/net/null/rte_eth_null.c| 1 -
drivers/net/pcap/rte_eth_pcap.c| 1 -
drivers/net/ring/rte_eth_ring.c| 1 -
drivers/net/vhost/rte_eth_vhost.c | 1 -
drivers/net/xenvirt/rte_eth_xenvirt.c | 1 -
lib/librte_eal/common/include/rte_dev.h| 9 -
14 files changed, 23 deletions(-)
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index 6e45293..dfa1001 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -517,7 +517,6 @@ aesni_gcm_uninit(const char *name)
static struct rte_vdev_driver aesni_gcm_pmd_drv = {
.driver = {
.name = CRYPTODEV_NAME_AESNI_GCM_PMD,
- .type = PMD_VDEV,
},
.init = aesni_gcm_init,
.uninit = aesni_gcm_uninit,
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index e7cdcfd..3265f83 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -717,7 +717,6 @@ cryptodev_aesni_mb_uninit(const char *name)
static struct rte_vdev_driver cryptodev_aesni_mb_pmd_drv = {
.driver = {
.name = CRYPTODEV_NAME_AESNI_MB_PMD,
- .type = PMD_VDEV,
},
.init = cryptodev_aesni_mb_init,
.uninit = cryptodev_aesni_mb_uninit,
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c
b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index 792184a..c39b0a1 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -651,7 +651,6 @@ cryptodev_kasumi_uninit(const char *name)
static struct rte_vdev_driver cryptodev_kasumi_pmd_drv = {
.driver = {
.name = CRYPTODEV_NAME_KASUMI_PMD,
- .type = PMD_VDEV,
},
.init = cryptodev_kasumi_init,
.uninit = cryptodev_kasumi_uninit,
diff --git a/drivers/crypto/null/null_crypto_pmd.c
b/drivers/crypto/null/null_crypto_pmd.c
index 5250e3f..5ad8b86 100644
--- a/drivers/crypto/null/null_crypto_pmd.c
+++ b/drivers/crypto/null/null_crypto_pmd.c
@@ -271,7 +271,6 @@ cryptodev_null_uninit(const char *name)
static struct rte_vdev_driver cryptodev_null_pmd_drv = {
.driver = {
.name = CRYPTODEV_NAME_NULL_PMD,
- .type = PMD_VDEV,
},
.init = cryptodev_null_init,
.uninit = cryptodev_null_uninit,
diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd.c
b/drivers/crypto/snow3g/rte_snow3g_pmd.c
index 6dc9a2e..9fc5c64 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd.c
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd.c
@@ -642,7 +642,6 @@ cryptodev_snow3g_uninit(const char *name)
static struct rte_vdev_driver cryptodev_snow3g_pmd_drv = {
.driver = {
.name = CRYPTODEV_NAME_SNOW3G_PMD,
- .type = PMD_VDEV,
},
.init = cryptodev_snow3g_init,
.uninit = cryptodev_snow3g_uninit,
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c
b/drivers/net/af_packet/rte_eth_af_packet.c
index 66aaf99..9d0c6d3 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -874,7 +874,6 @@ rte_pmd_af_packet_devuninit(const char *name)
static struct rte_vdev_driver pmd_af_packet_drv = {
.driver = {
.name = "eth_af_packet",
- .type = PMD_VDEV,
},
.init = rte_pmd_af_packet_devinit,
.uninit = rte_pmd_af_packet_devuninit,
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
b/drivers/net/bonding/rte_eth_bond_pmd.c
index 801a481..a3846e6 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2507,7 +2507,6 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
static struct rte_vdev_driver bond_drv = {
.driver = {
.name = "eth_bond",
- .type = PMD_VDEV,
},
.init = bond_init,
.uninit = bond_uninit,
diff --git a/drivers/net/mpipe/mpipe_tilegx.c b/drivers/net/mpipe/mpipe_tilegx.c
index fe7c0c6..9d5cc6d 100644
--- a/drivers/net/mpipe/mpipe_tilegx.c
+++ b/drivers/net/mpipe/mpipe_tilegx.c
@@ -1626,7 +1626,6 @@ rte_pmd_mpipe_devinit(const char *ifname,
static struct rte_vdev_driver pmd_mpipe_xgbe_drv = {
.driver = {
[dpdk-dev] [PATCH v1 04/15] drivers: convert PMD_VDEV drivers to use rte_vdev_driver
All PMD_VDEV drivers can now use rte_vdev_driver instead of the
rte_driver (which is embedded in the rte_vdev_driver).
Signed-off-by: Jan Viktorin
---
drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 16 +---
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 16 +---
drivers/crypto/kasumi/rte_kasumi_pmd.c | 16 +---
drivers/crypto/null/null_crypto_pmd.c | 16 +---
drivers/crypto/snow3g/rte_snow3g_pmd.c | 16 +---
drivers/net/af_packet/rte_eth_af_packet.c | 16 +---
drivers/net/bonding/rte_eth_bond_pmd.c | 16 +---
drivers/net/mpipe/mpipe_tilegx.c | 26 +++---
drivers/net/null/rte_eth_null.c| 16 +---
drivers/net/pcap/rte_eth_pcap.c| 16 +---
drivers/net/ring/rte_eth_ring.c| 16 +---
drivers/net/vhost/rte_eth_vhost.c | 16 +---
drivers/net/xenvirt/rte_eth_xenvirt.c | 16 +---
13 files changed, 123 insertions(+), 95 deletions(-)
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index 2987ef6..e6720e8 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -37,7 +37,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -514,11 +514,13 @@ aesni_gcm_uninit(const char *name)
return 0;
}
-static struct rte_driver aesni_gcm_pmd_drv = {
- .name = CRYPTODEV_NAME_AESNI_GCM_PMD,
- .type = PMD_VDEV,
- .init = aesni_gcm_init,
- .uninit = aesni_gcm_uninit
+static struct rte_vdev_driver aesni_gcm_pmd_drv = {
+ .driver = {
+ .name = CRYPTODEV_NAME_AESNI_GCM_PMD,
+ .type = PMD_VDEV,
+ .init = aesni_gcm_init,
+ .uninit = aesni_gcm_uninit
+ },
};
-PMD_REGISTER_DRIVER(aesni_gcm_pmd_drv);
+RTE_EAL_VDRV_REGISTER(aesni_gcm_pmd_drv);
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index 6554fc4..6c6420b 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -34,7 +34,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -714,11 +714,13 @@ cryptodev_aesni_mb_uninit(const char *name)
return 0;
}
-static struct rte_driver cryptodev_aesni_mb_pmd_drv = {
- .name = CRYPTODEV_NAME_AESNI_MB_PMD,
- .type = PMD_VDEV,
- .init = cryptodev_aesni_mb_init,
- .uninit = cryptodev_aesni_mb_uninit
+static struct rte_vdev_driver cryptodev_aesni_mb_pmd_drv = {
+ .driver = {
+ .name = CRYPTODEV_NAME_AESNI_MB_PMD,
+ .type = PMD_VDEV,
+ .init = cryptodev_aesni_mb_init,
+ .uninit = cryptodev_aesni_mb_uninit
+ },
};
-PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drv);
+RTE_EAL_VDRV_REGISTER(cryptodev_aesni_mb_pmd_drv);
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c
b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index 0bf415d..91181c4 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -35,7 +35,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -648,11 +648,13 @@ cryptodev_kasumi_uninit(const char *name)
return 0;
}
-static struct rte_driver cryptodev_kasumi_pmd_drv = {
- .name = CRYPTODEV_NAME_KASUMI_PMD,
- .type = PMD_VDEV,
- .init = cryptodev_kasumi_init,
- .uninit = cryptodev_kasumi_uninit
+static struct rte_vdev_driver cryptodev_kasumi_pmd_drv = {
+ .driver = {
+ .name = CRYPTODEV_NAME_KASUMI_PMD,
+ .type = PMD_VDEV,
+ .init = cryptodev_kasumi_init,
+ .uninit = cryptodev_kasumi_uninit
+ },
};
-PMD_REGISTER_DRIVER(cryptodev_kasumi_pmd_drv);
+RTE_EAL_VDRV_REGISTER(cryptodev_kasumi_pmd_drv);
diff --git a/drivers/crypto/null/null_crypto_pmd.c
b/drivers/crypto/null/null_crypto_pmd.c
index bdaf13c..a015ee5 100644
--- a/drivers/crypto/null/null_crypto_pmd.c
+++ b/drivers/crypto/null/null_crypto_pmd.c
@@ -33,7 +33,7 @@
#include
#include
#include
-#include
+#include
#include
#include "null_crypto_pmd_private.h"
@@ -268,11 +268,13 @@ cryptodev_null_uninit(const char *name)
return 0;
}
-static struct rte_driver cryptodev_null_pmd_drv = {
- .name = CRYPTODEV_NAME_NULL_PMD,
- .type = PMD_VDEV,
- .init = cryptodev_null_init,
- .uninit = cryptodev_null_uninit
+static struct rte_vdev_driver cryptodev_null_pmd_drv = {
+ .driver = {
+ .name = CRYPTODEV_NAME_NULL_PMD,
+ .type = PMD_VDEV,
+ .init = cryptodev_null_init,
+ .uninit = cryptodev_null_uninit
+ },
};
-PMD_REGISTER_DRIVER(cryptodev_null_pmd_drv);
+RTE_EAL_VDRV_REGISTER(cryptodev_null_pmd_drv);
diff --git
[dpdk-dev] [PATCH v1 03/15] eal: do not call init for PMD_PDEV drivers
There is no way how to call an init on a PMD_PDEV driver as those
drivers are all PCI drivers and they do not register any rte_driver
with EAL. We can drop the loop over PMD_PDEV drivers entirely.
Signed-off-by: Jan Viktorin
---
lib/librte_eal/common/eal_common_dev.c | 8
1 file changed, 8 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_dev.c
b/lib/librte_eal/common/eal_common_dev.c
index f9b3d1d..f4c880c 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -67,7 +67,6 @@ int
rte_eal_dev_init(void)
{
struct rte_devargs *devargs;
- struct rte_driver *driver;
/*
* Note that the dev_driver_list is populated here
@@ -89,13 +88,6 @@ rte_eal_dev_init(void)
}
}
- /* Once the vdevs are initalized, start calling all the pdev drivers */
- TAILQ_FOREACH(driver, _driver_list, next) {
- if (driver->type != PMD_PDEV)
- continue;
- /* PDEV drivers don't get passed any parameters */
- driver->init(NULL, NULL);
- }
return 0;
}
--
2.9.0
[dpdk-dev] [PATCH v1 02/15] eal: no need to test for PMD_VDEV anymore
All devices in the rte_eal_vdev_init/uninit are always virtual devices.
Signed-off-by: Jan Viktorin
---
lib/librte_eal/common/eal_common_vdev.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_vdev.c
b/lib/librte_eal/common/eal_common_vdev.c
index ea83c41..5a74da8 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -60,9 +60,6 @@ rte_eal_vdev_init(const char *name, const char *args)
return -EINVAL;
TAILQ_FOREACH(driver, _driver_list, next) {
- if (driver->driver.type != PMD_VDEV)
- continue;
-
/*
* search a driver prefix in virtual device name.
* For example, if the driver is pcap PMD, driver->name
@@ -86,9 +83,6 @@ rte_eal_vdev_uninit(const char *name)
return -EINVAL;
TAILQ_FOREACH(driver, _driver_list, next) {
- if (driver->driver.type != PMD_VDEV)
- continue;
-
/*
* search a driver prefix in virtual device name.
* For example, if the driver is pcap PMD, driver->name
--
2.9.0
[dpdk-dev] [PATCH v1 00/15] rte_driver/device infrastructure
Hello, based on the discussions with Shreyansh, I propose a patchset with the important EAL changes. It is incomplete and I suppose to extend and change certain things in the foreseeable future. Important notes: * pmd_type is removed * introduced rte_vdev_driver inheriting rte_driver * PMD_REGISTER_DRIVER is replaced by RTE_EAL_VDRV_REGISTER * rte_driver/device integrated into rte_pci_driver/device * all drivers and devices are in 2 lists - general and bus-specific Shreyansh, I hope I do not duplicate your work. I tried to avoid touching pmd_type but it quite complicated... There is also an initial generalization of rte_pci_resource. More such generalizations are to be done. The init/uninit functions cannot be generalized easily, I think. Both PCI and VDEV have different requirements. No idea about hotplug... The patchset is based on (all rebased on top of 34d279): [PATCH v5 00/17] Prepare for rte_device / rte_driver Thanks anybody for some quick review and notes. Regards Jan -- Jan Viktorin (15): eal: extract vdev infra eal: no need to test for PMD_VDEV anymore eal: do not call init for PMD_PDEV drivers drivers: convert PMD_VDEV drivers to use rte_vdev_driver eal: move init/uninit to rte_vdev_driver eal: remove PMD_REGISTER_DRIVER eal: get rid of pmd_type eal: define macro container_of eal: rte_pci.h includes rte_dev.h eal: rename and move rte_pci_resource eal/pci: inherit rte_driver by rte_pci_driver eal: call rte_eal_driver_register eal: introduce rte_device eal/pci: inherit rte_device by rte_pci_device eal/pci: insert rte_device on scan app/test/test_pci.c| 10 ++- app/test/virtual_pmd.c | 6 +- drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 13 ++-- drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 13 ++-- drivers/crypto/kasumi/rte_kasumi_pmd.c | 13 ++-- drivers/crypto/null/null_crypto_pmd.c | 13 ++-- drivers/crypto/qat/rte_qat_cryptodev.c | 4 +- drivers/crypto/snow3g/rte_snow3g_pmd.c | 13 ++-- drivers/net/af_packet/rte_eth_af_packet.c | 11 ++-- drivers/net/bnx2x/bnx2x_ethdev.c | 8 ++- drivers/net/bonding/rte_eth_bond_pmd.c | 11 ++-- drivers/net/cxgbe/cxgbe_ethdev.c | 4 +- drivers/net/cxgbe/sge.c| 6 +- drivers/net/e1000/em_ethdev.c | 4 +- drivers/net/e1000/igb_ethdev.c | 8 ++- drivers/net/ena/ena_ethdev.c | 4 +- drivers/net/enic/enic_ethdev.c | 4 +- drivers/net/fm10k/fm10k_ethdev.c | 10 +-- drivers/net/i40e/i40e_ethdev.c | 4 +- drivers/net/i40e/i40e_ethdev_vf.c | 4 +- drivers/net/i40e/i40e_fdir.c | 2 +- drivers/net/ixgbe/ixgbe_ethdev.c | 8 ++- drivers/net/mlx4/mlx4.c| 4 +- drivers/net/mlx5/mlx5.c| 4 +- drivers/net/mpipe/mpipe_tilegx.c | 20 +++--- drivers/net/nfp/nfp_net.c | 4 +- drivers/net/null/rte_eth_null.c| 11 ++-- drivers/net/pcap/rte_eth_pcap.c| 11 ++-- drivers/net/qede/qede_ethdev.c | 4 ++ drivers/net/ring/rte_eth_ring.c| 11 ++-- drivers/net/szedata2/rte_eth_szedata2.c| 8 ++- drivers/net/vhost/rte_eth_vhost.c | 11 ++-- drivers/net/virtio/virtio_ethdev.c | 6 +- drivers/net/virtio/virtio_pci.c| 2 +- drivers/net/vmxnet3/vmxnet3_ethdev.c | 7 +- drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +- drivers/net/xenvirt/rte_eth_xenvirt.c | 11 ++-- lib/librte_cryptodev/rte_cryptodev.c | 6 +- lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/Makefile | 2 +- lib/librte_eal/common/eal_common_dev.c | 67 +++ lib/librte_eal/common/eal_common_pci.c | 20 +++--- lib/librte_eal/common/eal_common_vdev.c| 100 + lib/librte_eal/common/include/rte_common.h | 16 + lib/librte_eal/common/include/rte_dev.h| 54 +--- lib/librte_eal/common/include/rte_pci.h| 20 ++ lib/librte_eal/common/include/rte_vdev.h | 96 +++ lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/linuxapp/eal/eal_ivshmem.c | 2 +- lib/librte_eal/linuxapp/eal/eal_pci.c | 7 +- lib/librte_ether/rte_ethdev.c | 16 +++-- 51 files changed, 468 insertions(+), 229 deletions(-) create mode 100644 lib/librte_eal/common/eal_common_vdev.c create mode 100644 lib/librte_eal/common/include/rte_vdev.h -- 2.9.0
[dpdk-dev] [PATCH] net/enic: remove useless assert macro
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, July 06, 2016 8:25 AM > To: John Daley (johndale) ; Nelson Escobar > (neescoba) > Cc: dev at dpdk.org > Subject: [PATCH] net/enic: remove useless assert macro > > The macro ENIC_ASSERT does the same thing as RTE_ASSERT, thus it can be > removed. > > Signed-off-by: Thomas Monjalon Acked-by: John Daley
[dpdk-dev] [PATCH v1] doc: autogenerate nic overview table from ini files
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, June 30, 2016 7:25 PM > To: Mcnamara, John > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1] doc: autogenerate nic overview table > from ini files > > 2016-06-30 19:03, John McNamara: > > This patch converts the NIC feature table in the overview doc into a > > set of ini files and adds some functions into the Sphinx conf.py file > > to convert them back into an RST table. > > > Would it be possible to make it a bit more generic and reusable to > generate other tables of this kind? It would be possible (in the technical sense) but probably not worth the effort. :-) > > * Blank entries in the PMD ini files are optional. They will get a > default > > blank entry in the RST table based on the entries in the default.ini > file. > > The ini files in this patch were generated programmatically from the > > original RST table. > > I don't think there is a benefit to have blank entries in the .ini file. > And there would be less conflicts if the guideline was to avoid blank > entries when adding a new feature. I can generate a set of ini files without blank entries if that is the preference. For some PMDs that will mean that there are no entries in the file, apart from the heading, but I guess that is okay. I'll change that in v2. > > PDF output is restrictive :) Yes, and the PDF infrastructure is a bit of a pain but we still get a lot of people in the field who like them and ask for them after releases. Maybe they read them on the beach, on holidays. John
[dpdk-dev] [PATCH] cryptodev: move new cryptodev type to bottom of enum
2016-07-06 15:05, Pablo de Lara:
> New cryptodev type for the new KASUMI PMD was added
> in the cryptodev type enum, but not at the end of it,
> causing an ABI breakage.
>
> Fixes: 2773c86d061a ("crypto/kasumi: add driver for KASUMI library")
>
> Signed-off-by: Pablo de Lara
> Reported-by: Ferruh Yigit
Applied, thanks
[dpdk-dev] [PATCH] scripts: fix commit check for empty list
When running check-git-log.sh on a clean tree it was complaining of a wrong empty headline because '^[^:]*$' was matching. It is fixed by matching at least one character with +. Signed-off-by: Thomas Monjalon --- scripts/check-git-log.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh index 7d2c7ee..c2a3ac0 100755 --- a/scripts/check-git-log.sh +++ b/scripts/check-git-log.sh @@ -62,7 +62,7 @@ bad=$(echo "$headlines" | grep --color=always \ -e '\.$' \ -e '[,;!?&|]' \ -e ':.*_' \ - -e '^[^:]*$' \ + -e '^[^:]\+$' \ -e ':[^ ]' \ -e ' :' \ | sed 's,^,\t,') -- 2.7.0
[dpdk-dev] [dpdk-dev, 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread
Cunming, what is the status of this patchset, please? 2016-03-23 11:24, Liang, Cunming: > Hi Mattew, > > Thank you for your time. > > On 3/22/2016 3:39 PM, Matthew Hall wrote: > > On Mon, Mar 21, 2016 at 03:58:44PM +0800, Liang, Cunming wrote: > >> the default termination handler > > I am not so experienced with this "default termination handler". Can someone > > clarify what it is so I could comment better about it? > For example, you're handling SIGINT. After finishing your necessary app > cleanup, then 'signal(SIGINT, SIG_DFL); raise(SIGINT);'. > The default signal handler can terminate the interrupt thread. > > > > >> If EINTR is caused by some non-term purpose signals, are you going > >> to exit the interrupt thread any way? > > We should discuss what makes sense here. I'm just trying to get some things > > working and finding EINTR was getting eaten and causing infinite looping. > SIGINT/SIGTERM causes EINTR return, while SIGUSR1 also can cause the > EINTR return. For the dedicated EAL interrupt thread, it won't be > expected to exit for all kinds of the cause. > On this view, I'm in favor of your patch which cancel the interrupt > thread, but don't directly return by the EINTR. > > > > >> Without setting 'PTHREAD_CREATE_DETACHED' won't cause the infinite > >> loop. However by using pthread_cancel to terminate the thread, > >> indeed it's necessary to set 'PTHREAD_CREATE_DETACHED'. > > My general understanding is that PTHREAD_CREATE_DETACHED should be used for > > any thread, which should not keep a process open by itself if it is > > executing, > > i.e. a "daemon thread". I believe the interrupt thread qualifies as such a > > thread if I have understood everything right (which is hard to promise when > > you only work in DPDK in spare time). > > > >> It looks like 'pthread_cancel' is the right way and I saw it > >> continue keeps current EINTR handling in EAL interrupt thread. > > It is one option. Depending what makes the most sense. > > > >> 1. Can you explain and add patch comments why default signal handler > >> is not good enough to terminate app. > > Yes if someone call tell me more about what it is so I can check it. > > > >> 2. I propose to add addition comments on rte_epoll_wait() API > >> description. For any signal, it causes an error return, user needs > >> to handle. > > Agreed. > > > >> 3. Will you do a favorite to add 'PTHREAD_CREATE_DETACHED' to all > >> EAL pthread too. > > As a spare time developer I am a bit conservative about too large of a scope > > and messing with code for other threads or features I didn't personally use > > or > > test. This is because I don't have the same QA resources as Intel / 6WIND / > > etc.. Some help from a full time developer would be great here. > All right, reasonable to me. > > > > >> Cunming > > Matthew. >
[dpdk-dev] [PATCH v1] igu_uio: fix IOMMU domain issue
Ping, this patch is stalled. 2016-05-11 18:24, Ferruh Yigit: > On 5/11/2016 8:35 AM, Alejandro Lucero wrote: > > On Tue, May 10, 2016 at 4:59 PM, Stephen Hemminger < > > stephen at networkplumber.org> wrote: > > > >> On Tue, 10 May 2016 19:21:41 +0800 > >> Zhe Tao wrote: > >> > >>> Problem: > >>> The following operations will cause the igb_uio based DPDK > >>> operation failed. > >>> --Any device assignment through the kvm_assign_device interface, > >>> this can be the pci-assign method in QEMU > >>> --VFIO group attachment operation(attach to the container) > >>> this can happens in vfio-pci assignment in QEMU > >> > >> > >> If you have an IOMMU why not use VFIO instead, it is better. > >> > > > > It is not about VFIO against UIO but about how iommu domains are created > > and destroyed by the (old) kernel when iommu=pt. So even with VFIO you can > > have problems. > > Problem is in IOMMU driver but we are adding a workaround to igb_uio, if > using VFIO solves the issue, I believe that is better workaround. > > 1) Is there any case IOMMU supported but VFIO is not supported? Is there > anything forces to use igb_uio? > > 2) Does using VFIO solves the issue defined in problem statement? > > > > > We have had problems like this and other due to our device (NFP) just > > mapping up to 40 bits of address space. Old kernels used in LTS > > distributions like Ubuntu are iommu buggy and you need to do things like > > this mapping inside the driver for solving problems. By the way, using > > SRIOV just adds more problems. It is not safe to use iommu=pt with 3.13.x > > Ubuntu kernels. > > > > It would be a good thing for the original patch to identify those kernels > > where the problem was detected. Of course, there could be more kernels with > > the same problem but that is more work to do. > > > > Thanks, > ferruh
[dpdk-dev] [PATCH v3] crypto: normalize cryptodev pmd names with macros
> Recently reported, the introduction of pmd information exports led to a > breakage of cryptodev unit tests because the test infrastructure relies on the > cryptodev names being available in macros. This patch fixes the pmd naming to > use the macro names. Note that the macro names were already pre-stringified, > which won't work as the PMD_REGISTER_DRIVER macro requires the name in both a > processing token and stringified form. As such the names are defined now as > tokens, and converted where needed to stringified form on demand using > RTE_STR. > > Tested using the null and aesni_mb crypto pmds, as I don't have hardware for > any > other device. Also not build tested on snow3g or kasumi pmd because building > those requires external libraries that appear to not be open source licensed. > > Signed-off-by: Neil Horman > Signed-off-by: Pablo de Lara Applied, thanks
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Adrien, On 7/6/2016 2:16 AM, Adrien Mazarguil wrote: > Hi All, > > First, forgive me for this large message, I know our mailboxes already > suffer quite a bit from the amount of traffic on this ML. > > This is not exactly yet another thread about how flow director should be > extended, rather about a brand new API to handle filtering and > classification for incoming packets in the most PMD-generic and > application-friendly fashion we can come up with. Reasons described below. > > I think this topic is important enough to include both the users of this API > as well as PMD maintainers. So far I have CC'ed librte_ether (especially > rte_eth_ctrl.h contributors), testpmd and PMD maintainers (with and without > a .filter_ctrl implementation), but if you know application maintainers > other than testpmd who use FDIR or might be interested in this discussion, > feel free to add them. > > The issues we found with the current approach are already summarized in the > following document, but here is a quick summary for TL;DR folks: > > - PMDs do not expose a common set of filter types and even when they do, >their behavior more or less differs. > > - Applications need to determine and adapt to device-specific limitations >and quirks on their own, without help from PMDs. > > - Writing an application that creates flow rules targeting all devices >supported by DPDK is thus difficult, if not impossible. > > - The current API has too many unspecified areas (particularly regarding >side effects of flow rules) that make PMD implementation tricky. > > This RFC API handles everything currently supported by .filter_ctrl, the > idea being to reimplement all of these to make them fully usable by > applications in a more generic and well defined fashion. It has a very small > set of mandatory features and an easy method to let applications probe for > supported capabilities. > > The only downside is more work for the software control side of PMDs because > they have to adapt to the API instead of the reverse. I think helpers can be > added to EAL to assist with this. > > HTML version: > > https://rawgit.com/6WIND/rte_flow/master/rte_flow.html > > PDF version: > > https://rawgit.com/6WIND/rte_flow/master/rte_flow.pdf > > Related draft header file (for reference while reading the specification): > > https://raw.githubusercontent.com/6WIND/rte_flow/master/rte_flow.h > > Git tree for completeness (latest .rst version can be retrieved from here): > > https://github.com/6WIND/rte_flow > > What follows is the ReST source of the above, for inline comments and > discussion. I intend to update that specification accordingly. > > > Generic filter interface > > > .. footer:: > > v0.6 > > .. contents:: > .. sectnum:: > .. raw:: pdf > > PageBreak > > Overview > > > DPDK provides several competing interfaces added over time to perform packet > matching and related actions such as filtering and classification. > > They must be extended to implement the features supported by newer devices > in order to expose them to applications, however the current design has > several drawbacks: > > - Complicated filter combinations which have not been hard-coded cannot be >expressed. > - Prone to API/ABI breakage when new features must be added to an existing >filter type, which frequently happens. > > From an application point of view: > > - Having disparate interfaces, all optional and lacking in features does not >make this API easy to use. > - Seemingly arbitrary built-in limitations of filter types based on the >device they were initially designed for. > - Undefined relationship between different filter types. > - High complexity, considerable undocumented and/or undefined behavior. > > Considering the growing number of devices supported by DPDK, adding a new > filter type each time a new feature must be implemented is not sustainable > in the long term. Applications not written to target a specific device > cannot really benefit from such an API. > > For these reasons, this document defines an extensible unified API that > encompasses and supersedes these legacy filter types. > > .. raw:: pdf > > PageBreak > > Current API > === > > Rationale > - > > The reason several competing (and mostly overlapping) filtering APIs are > present in DPDK is due to its nature as a thin layer between hardware and > software. > > Each subsequent interface has been added to better match the capabilities > and limitations of the latest supported device, which usually happened to > need an incompatible configuration approach. Because of this, many ended up > device-centric and not usable by applications that were not written for that > particular device. > > This document is not the first attempt to address this proliferation issue, > in fact a lot of work has already been done both to create a more generic > interface while somewhat keeping
[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Neil Horman > Sent: Friday, July 8, 2016 2:42 PM > To: Thomas Monjalon > Cc: De Lara Guarch, Pablo ; dev at > dpdk.org; > Richardson, Bruce ; Stephen Hemminger > ; Panu Matilainen > Subject: Re: [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with > macros > > On Fri, Jul 08, 2016 at 02:40:01PM +0200, Thomas Monjalon wrote: > > 2016-07-08 08:17, Neil Horman: > > > I suppose thats fine, sure, though I'm really not comfortable with > > > an open source project requiring what appears to be non-open source > > > components (though I can't really tell what the snow3g or kasumi > license is). > > > > Yes you're right. > > IMHO, it is acceptable because it is free (just need a registration) > > but it can be discussed. > > I think there are some legal export issues because of crypto aim. > Yeah, crypto usually has munitions export restrictions. I'm fine with > that, as long as the license to the code is GPL. And I can't tell what > the license is because you have to register to get it before you can see > it. > > If you, or someone can confirm that the snow3g and kasumi ssl libraries > are open source compatible (or better still if the license can be > announced prior to registration on the web site), then I'm fine with it, > and am happy to test with it, I just don't want to register without > knowing that beforehand. Hi Neil, The license is "Intel Pre-Release Software License Agreement". We will send a patch to make this this clear in the documentation. We will also see if it can be made visible on the download page. I can send you a copy of the license if you wish. Pablo has already tested compilation with the latest version of the patch. John
[dpdk-dev] [PATCH v6 2/2] examples/ethtool: use rte_eth_dev_get_reg_info for reg params
2016-07-04 14:24, Remy Horton: > On 04/07/2016 12:36, Zyta Szpak wrote: > > From: Zyta Szpak > > > > This change deals with hard-coded register width. > > The app was allocating too little space for 64-bit registers > > which resulted in memory corruption. This commit resolves > > this by getting the number of registers and size of register > > by rte_eth_dev_get_reg_info function called first time > > with data=NULL. > > > > Signed-off-by: Zyta Szpak > > Acked-by: Remy Horton Series squashed (to avoid breaking example build) and applied, thanks
[dpdk-dev] [PATCH v3 00/10] additions to pmdinfogen
2016-07-08 16:42, Thomas Monjalon: > These are some patches that I think needed for RC2. Applied > There are other patches needed as discussed on the mailing-list which > are not part of this series: > - overwritten driver name (reported by Pablo) > - pmdinfo.py on FreeBSD (reported by Bruce) Hope we will have these patches ready soon.
[dpdk-dev] [PATCH v2] crypto: normalize cryptodev pmd names with macros
Recently reported, the introduction of pmd information exports led to a
breakage of cryptodev unit tests because the test infrastructure relies on the
cryptodev names being available in macros. This patch fixes the pmd naming to
use the macro names. Note that the macro names were already pre-stringified,
which won't work as the PMD_REGISTER_DRIVER macro requires the name in both a
processing token and stringified form. As such the names are defined now as
tokens, and converted where needed to stringified form on demand using RTE_STR.
Tested using the null and aesni_mb crypto pmds, as I don't have hardware for any
other device. Also not build tested on snow3g or kasumi pmd because building
those requires external libraries that appear to not be open source licensed.
Signed-off-by: Neil Horman
Signed-off-by: Pablo de Lara
CC: Bruce Richardson
CC: Thomas Monjalon
CC: "De Lara Guarch, Pablo"
CC: Stephen Hemminger
CC: Panu Matilainen
---
Changes in v2:
- Fixed compilation issue on snow3g/kasumi pmd
- Removed static name of qat pmd
app/test/test_cryptodev.c | 20 ++--
app/test/test_cryptodev_perf.c | 18 +-
drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 +++
drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 6 +++---
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 7 +++
drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 2 +-
drivers/crypto/kasumi/rte_kasumi_pmd.c | 7 +++
drivers/crypto/kasumi/rte_kasumi_pmd_private.h | 6 +++---
drivers/crypto/null/null_crypto_pmd.c | 7 +++
drivers/crypto/null/null_crypto_pmd_private.h | 6 +++---
drivers/crypto/qat/rte_qat_cryptodev.c | 5 ++---
drivers/crypto/snow3g/rte_snow3g_pmd.c | 7 +++
drivers/crypto/snow3g/rte_snow3g_pmd_private.h | 6 +++---
lib/librte_cryptodev/rte_cryptodev.h | 12 ++--
14 files changed, 55 insertions(+), 61 deletions(-)
diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index fbfe1d0..1e1f887 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -186,12 +186,12 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
ret = rte_eal_vdev_init(
- CRYPTODEV_NAME_AESNI_MB_PMD, NULL);
+ RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD),
NULL);
TEST_ASSERT(ret == 0,
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_AESNI_MB_PMD);
+ i,
RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD));
}
}
}
@@ -203,10 +203,10 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
- CRYPTODEV_NAME_AESNI_GCM_PMD, NULL),
+ RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD),
NULL),
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_AESNI_GCM_PMD);
+ i,
RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD));
}
}
}
@@ -217,10 +217,10 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
- CRYPTODEV_NAME_SNOW3G_PMD, NULL),
+ RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD),
NULL),
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_SNOW3G_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD));
}
}
}
@@ -231,10 +231,10 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
- CRYPTODEV_NAME_KASUMI_PMD, NULL),
+ RTE_STR(CRYPTODEV_NAME_KASUMI_PMD),
NULL),
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_KASUMI_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_KASUMI_PMD));
[dpdk-dev] [PATCH v3 00/11] Fix build errors related to exported headers
On Fri, Jul 08, 2016 at 03:45:50PM +0100, Ferruh Yigit wrote:
> On 7/8/2016 3:35 PM, Adrien Mazarguil wrote:
> > On Fri, Jul 08, 2016 at 02:15:37PM +, Wiles, Keith wrote:
> >>
> >>> On Jul 8, 2016, at 4:56 AM, Yigit, Ferruh
> >>> wrote:
> >>>
> >>> On 7/8/2016 9:05 AM, Adrien Mazarguil wrote:
> On Thu, Jul 07, 2016 at 06:33:17PM +, Wiles, Keith wrote:
> >
> >> On Jul 7, 2016, at 10:49 AM, Adrien Mazarguil >> 6wind.com> wrote:
> >>
> >> DPDK uses GNU C language extensions in most of its code base. This is
> >> fine
> >> for internal source files whose compilation flags are controlled by
> >> DPDK,
> >> however user applications that use exported "public" headers may
> >> experience
> >> compilation failures when enabling strict error/standard checks (-std
> >> and
> >> -pedantic for instance).
> >
> > Do you try compiling these changes with CLANG and/or ICC compilers?
>
> clang/clang++ yes, works fine. I did not try with ICC however.
> >>>
> >>> I tested with icc, getting following error [1], compiler warning seems
> >>> valid, but didn't investigate what in your patch cause this.
> >>>
> >>> [1]
> >>> .../app/test/test_table_acl.c(487): error #2405: array of elements
> >>> containing a flexible array member is nonstandard
> >>>struct rte_pipeline_table_entry entries[5];
> >>>^
> >>>
> >>> .../app/test/test_table_acl.c(492): error #2405: array of elements
> >>> containing a flexible array member is nonstandard
> >>>struct rte_pipeline_table_entry entries_ptr[5];
> >>
> >> I am guessing it does not like the uint8_t action_data[0] in the
> >> rte_pipeline_table_entry structure. I can see why it would be non-standard
> >> allocated on the stack in this case. Maybe a keyword like __extension__
> >> needs to be added or pragma.
> >
> > Hmmm, right, a variable array cannot be embedded like that. I think a change
> > in test_table_acl.c would be better.
> Correct, I just wonder why not getting this same compiler warning before
> your changes.
>
> ...
I guess declaring a zero-sized array does not give the same compiler
constraints as declaring a variable-sized one even though the structure has
the same size in both cases. C99 does not allow it. This is probably the
only difference.
>
> > Is that the only issue you saw?
> >
> Yes.
OK then, prepare for v4.
> Note that considering "({ ... })" is a GNU extension, compilers that do
> support this syntax also support the GNU __extension__ keyword. As a
> result,
> those that do not support this keyword most likely already cannot compile
> DPDK at all.
--
Adrien Mazarguil
6WIND
[dpdk-dev] [PATCH v6 0/4] support reset of VF link
On Fri, 2016-07-08 at 00:14 +, Lu, Wenzhuo wrote: > > > > > > > > > > Hello Wenzhuo, > > > > > > > > > > > > > > > > > > > > I'm testing this patchset, but I am sporadically running > > > > > > > > > > into an issue where the VFs reset fails after the PF flaps. > > > > > > > > > > > > > > > > > > > > I have a VM running on a KVM box with a X540-AT2, passing 2 > > > > > > > > > > VFs > > in. > > > > > > > > > > > > > > > > > > > > I am using calling rte_eth_dev_reset in response to a > > > > > > > > > > RTE_ETH_EVENT_INTR_RESET callback, and the following > > > > > > > > > > errors appear in the > > > > > > > > > > log: > > > > > > > > > > > > > > > > > > > > PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to update > > > > > > > > > > link. > > > > > > > > > > PMD: ixgbe_alloc_rx_queue_mbufs(): RX mbuf alloc failed > > > > > > > > > > queue_id=0 > > > > > > > > > > PMD: ixgbevf_dev_start(): Unable to initialize RX > > > > > > > > > > hardware > > > > > > > > > > (-12) > > > > > > > > > > PMD: ixgbevf_dev_reset(): Ixgbe VF reset: Failed to start > > > > > > > > > > device. > > > > > > > > > > > > > > > > > > > > Jumping in with GDB, it seems that the rte_rxmbuf_alloc > > > > > > > > > > call in ixgbe_alloc_rx_queue_mbufs returns NULL at > > > > > > > > > > iteration 64 out of > > > > 2048. > > > > > > > > > > The application has ~500 2MB hugepages, and there's 2GB > > > > > > > > > > of free memory available on top of that. > > > > > > > > > > > > > > > > > > > > Have you seen this before? Any pointer or suggestion for > > debugging? > > > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > Kind regards, > > > > > > > > > > Luca Boccassi > > > > > > > > > I think the problem is the mbuf occupied by the packets is > > > > > > > > > not released. This > > > > > > > > memory has to be released by the APP, so my patches haven?t > > > > > > > > covered > > > > this. > > > > > > > > Actually an example is needed to show how to use the reset API. > > > > > > > > I plan to modify the testpmd. > > > > > > > > > You may notice this feature is postponed to 16.11. Would > > > > > > > > > you like to wait for > > > > > > > > the new version that will include an example? > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > Unfortunately we need the VF reset working sooner than that, > > > > > > > > so one way or the other I'll need to sort it out. Given I've > > > > > > > > got a use case where this is happening, if it can be helpful > > > > > > > > for you I'm more than happy to help as a guinea pig. If you > > > > > > > > could please give some guidance/guidelines with regards to > > > > > > > > which API to use to sort the mbuf > > > > > > problem, I can try it out and give back some feedback. > > > > > > > > > > > > > > > > Thanks! > > > > > > > I made a stupid mistake and deleted all my code. So, I have to > > > > > > > take some time to rewrite it :( Attached the example I used to > > > > > > > test the reset API. It's > > > > > > modified from the l2fwd example. So you can compare it with > > > > > > l2fwd to see what need to be added. > > > > > > > Hopefully it can help :) > > > > > > > > > > > > Thanks! That made me understand a couple of things more, and > > > > > > I've got past the problem. > > > > > > > > > > > > Unfortunately now there's a bigger issue - rte_eth_dev_reset is > > > > > > a blocking > > > > call. > > > > > > the _RESET event callback is fired when the PF goes down, but > > > > > > when I call rte_eth_dev_reset it will block until the PF goes back > > > > > > up. > > > > > > There is no way, as far as I can see, to know if the PF is back > > > > > > up before > > > > calling rte_eth_dev_reset. > > > > > > > > > > > > This is a problem because, as far as I understand, I have to > > > > > > call all the rte_eth_dev_ APIs from the same thread, in my case > > > > > > the master thread, and I can't have that block potentially > > > > > > indefinitely. > > > > > > > > > > > > Would it be possible to have 2 events instead of 1, one when the > > > > > > PF goes down and one when it goes up? This way an application > > > > > > would be able to soft-stop the port (drain queues, etc) when the > > > > > > PF is down, and then call the reset API when it goes back up. > > > > > > > > > > > > Thanks! > > > > > Sorry we cannot have 2 events now. There're 2 problems to have 2 > > > > > events. > > > > > 1, Normally we use kernel driver for PF. Now the kernel driver > > > > > only have one > > > > kind of message for link down and up. So we cannot tell if it's down or > > > > up. > > > > > 2, When the PF is down, if we don't reset the VF, VF is not working. > > > > > It cannot receive any message from PF. So we cannot know that when > > > > > PF is up. It means normally we have to reset VF twice when PF down > > > > > and up. (Surely we can wait a while when we receive the message > > > > > from PF until PF is up. But we cannot
[dpdk-dev] [PATCH] net/enic: fix checking for sufficient resources
On Wed, Jul 06, 2016 at 04:21:59PM -0700, Nelson Escobar wrote:
> The enic PMD was using the same variables in the enic structure to
> track two different things. Initially rq_count, wq_count, cq_count,
> and intr_count were set to the values obtained from the VIC adapters
> as the maximum resources allocated on the VIC, then in
> enic_set_vnic_res(), they were set to the counts of resources actually
> used, discarding the initial values. The checks in enic_set_vnic_res()
> were technically incorrect if it is called more than once on a port,
> which happens when using bonding, but were harmless in practice as the
> checks couldn't fail on the second call.
>
> The enic rx-scatter patch misunderstood the subtleties of
> enic_set_vnic_res(), and naively added a multiply by two to the
> rq_count check. This resulted in the rq_count check failing when
> enic_set_vnic_res() was called a second time, ie when using bonding.
>
> This patch adds new variables to the enic structure to track the
> maximum resources the VIC is configured to provide so that the
> information isn't later lost and calls to enic_set_vnic_res() do
> the expected thing.
>
> Fixes: 856d7ba7ed22 ("net/enic: support scattered Rx")
>
> Signed-off-by: Nelson Escobar
Applied to dpdk-next-net/rel_16_07
/Bruce
[dpdk-dev] [PATCH v2] doc: update GSG for i40e firmware version.
On Wed, Jul 06, 2016 at 11:53:13AM +, Mcnamara, John wrote: > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ian Stokes > > Sent: Wednesday, July 6, 2016 11:30 AM > > To: dev at dpdk.org > > Cc: Stokes, Ian > > Subject: [dpdk-dev] [PATCH v2] doc: update GSG for i40e firmware version. > > > > Update the 'High Performance of Small Packets on 40G NIC' section of the > > GSG as the firmware version referenced for a NIC using the i40e driver was > > firmware version 4.2.5 which is no longer validated. Instruct users to > > consult release notes for current validated firmware versions. > > > > Signed-off-by: Ian Stokes > > Acked-by: John McNamara > Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH v1] ethdev: fix xstats id mismatch
When fetching xstats values the driver specific parameters are
placed after the generic ones, but when fetching xstats names
the driver specific parameter names came first. This patch fixes
the resulting id mismatch between names and values.
Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids")
Signed-off-by: Remy Horton
---
lib/librte_ether/rte_ethdev.c | 32 +++-
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eac260f..80b4d90 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1536,6 +1536,7 @@ rte_eth_xstats_get_names(uint8_t port_id,
struct rte_eth_dev *dev;
int cnt_used_entries;
int cnt_expected_entries;
+ int cnt_driver_entries;
uint32_t idx, id_queue;
cnt_expected_entries = get_xstats_count(port_id);
@@ -1545,16 +1546,7 @@ rte_eth_xstats_get_names(uint8_t port_id,
/* port_id checked in get_xstats_count() */
dev = _eth_devices[port_id];
- if (dev->dev_ops->xstats_get_names != NULL) {
- cnt_used_entries = (*dev->dev_ops->xstats_get_names)(
- dev, xstats_names, size);
- if (cnt_used_entries < 0)
- return cnt_used_entries;
- } else
- /* Driver itself does not support extended stats, but
-* still have basic stats.
-*/
- cnt_used_entries = 0;
+ cnt_used_entries = 0;
for (idx = 0; idx < RTE_NB_STATS; idx++) {
snprintf(xstats_names[cnt_used_entries].name,
@@ -1581,6 +1573,20 @@ rte_eth_xstats_get_names(uint8_t port_id,
cnt_used_entries++;
}
}
+
+ if (dev->dev_ops->xstats_get_names != NULL) {
+ /* If there are any driver-specific xstats, append them
+* to end of list.
+*/
+ cnt_driver_entries = (*dev->dev_ops->xstats_get_names)(
+ dev,
+ xstats_names + cnt_used_entries,
+ size - cnt_used_entries);
+ if (cnt_driver_entries < 0)
+ return cnt_driver_entries;
+ cnt_used_entries += cnt_driver_entries;
+ }
+
return cnt_used_entries;
}
@@ -1628,7 +1634,6 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat
*xstats,
stats_ptr = RTE_PTR_ADD(_stats,
rte_stats_strings[i].offset);
val = *stats_ptr;
- xstats[count].id = count + xcount;
xstats[count++].value = val;
}
@@ -1639,7 +1644,6 @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat
*xstats,
rte_rxq_stats_strings[i].offset +
q * sizeof(uint64_t));
val = *stats_ptr;
- xstats[count].id = count + xcount;
xstats[count++].value = val;
}
}
@@ -1651,11 +1655,13 @@ rte_eth_xstats_get(uint8_t port_id, struct
rte_eth_xstat *xstats,
rte_txq_stats_strings[i].offset +
q * sizeof(uint64_t));
val = *stats_ptr;
- xstats[count].id = count + xcount;
xstats[count++].value = val;
}
}
+ for (i = 0; i < count + xcount; i++)
+ xstats[i].id = i;
+
return count + xcount;
}
--
2.5.5
[dpdk-dev] [PATCH v3 10/10] maintainers: add section for pmdinfo
The author of this feature is Neil Horman. Signed-off-by: Thomas Monjalon Acked-by: Neil Horman --- MAINTAINERS | 4 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a59191e..f996c2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -68,6 +68,10 @@ F: lib/librte_compat/ F: doc/guides/rel_notes/deprecation.rst F: scripts/validate-abi.sh +Driver information +F: buildtools/pmdinfogen/ +F: tools/pmdinfo.py + Environment Abstraction Layer - -- 2.7.0
[dpdk-dev] [PATCH v3 09/10] doc: fix syntax in pmdinfogen guide
Sphynx reports this error:
doc/guides/prog_guide/dev_kit_build_system.rst:337: WARNING:
Pygments lexer name u'C' is not known
Fixes: 737ddf3fb ("doc: add prog guide section documenting pmdinfo script")
Reported-by: Bernard Iremonger
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
doc/guides/prog_guide/dev_kit_build_system.rst | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst
b/doc/guides/prog_guide/dev_kit_build_system.rst
index 1dc1388..18a3010 100644
--- a/doc/guides/prog_guide/dev_kit_build_system.rst
+++ b/doc/guides/prog_guide/dev_kit_build_system.rst
@@ -319,8 +319,7 @@ instance the macro:
.. code-block:: c
-PMD_REGISTER_DRIVER(drv, name)
-
+ PMD_REGISTER_DRIVER(drv, name)
Creates the following symbol:
@@ -334,7 +333,7 @@ can be exported from the object file and used to produce a
hardware support
description, that pmdinfogen then encodes into a json formatted string in the
following format:
-.. code-block:: C
+.. code-block:: c
static char ="PMD_INFO_STRING=\"{'name' : '',
...}\"";
--
2.7.0
[dpdk-dev] [PATCH v3 08/10] eal: remove PCI include from generic driver header
Remove include of rte_pci.h in the generic header rte_dev.h.
Fixes: cb6696d22023 ("drivers: update registration macro usage")
Suggested-by: David Marchand
Signed-off-by: Thomas Monjalon
---
lib/librte_eal/common/include/rte_dev.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/include/rte_dev.h
b/lib/librte_eal/common/include/rte_dev.h
index e6f0d4c..95789f9 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -48,7 +48,7 @@ extern "C" {
#include
#include
-#include
+
#include
__attribute__((format(printf, 2, 0)))
--
2.7.0
[dpdk-dev] [PATCH v3 07/10] pmdinfogen: fix usage message
The name of the tool is pmdinfogen.
Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
Signed-off-by: Thomas Monjalon
---
buildtools/pmdinfogen/pmdinfogen.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/buildtools/pmdinfogen/pmdinfogen.c
b/buildtools/pmdinfogen/pmdinfogen.c
index beb06f1..717c8d4 100644
--- a/buildtools/pmdinfogen/pmdinfogen.c
+++ b/buildtools/pmdinfogen/pmdinfogen.c
@@ -398,7 +398,8 @@ int main(int argc, char **argv)
if (argc < 3) {
fprintf(stderr,
- "usage: pmdinfo \n");
+ "usage: %s \n",
+ basename(argv[0]));
exit(127);
}
parse_elf(, argv[1]);
--
2.7.0
[dpdk-dev] [PATCH v3 06/10] pmdinfogen: fix build warnings
When compiled with a standard clang, pmdinfogen can raise a warning:
buildtools/pmdinfogen/pmdinfogen.c:365:1: warning:
control reaches end of non-void function
Actually there can be more warnings with stricter compilers.
In order to catch them early and fix most of them, the DPDK standard flags
WERROR_FLAGS are used.
The warnings fixed are:
no previous prototype for ...
no return statement in function returning non-void
variable ?secstrings? set but not used
?sec_name? defined but not used
?get_symbol_index? defined but not used
pointer of type ?void *? used in arithmetic
Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
Signed-off-by: Thomas Monjalon
---
buildtools/pmdinfogen/Makefile | 4 +--
buildtools/pmdinfogen/pmdinfogen.c | 58 --
2 files changed, 20 insertions(+), 42 deletions(-)
diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile
index 125901b..3885d3b 100644
--- a/buildtools/pmdinfogen/Makefile
+++ b/buildtools/pmdinfogen/Makefile
@@ -41,9 +41,9 @@ HOSTAPP = pmdinfogen
#
SRCS-y += pmdinfogen.c
-HOST_EXTRA_CFLAGS += -g -I${RTE_OUTPUT}/include
+HOST_CFLAGS += $(WERROR_FLAGS) -g
+HOST_CFLAGS += -I$(RTE_OUTPUT)/include
DEPDIRS-y += lib/librte_eal
include $(RTE_SDK)/mk/rte.hostapp.mk
-
diff --git a/buildtools/pmdinfogen/pmdinfogen.c
b/buildtools/pmdinfogen/pmdinfogen.c
index 0947dc6..beb06f1 100644
--- a/buildtools/pmdinfogen/pmdinfogen.c
+++ b/buildtools/pmdinfogen/pmdinfogen.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include "pmdinfogen.h"
#ifdef RTE_ARCH_64
@@ -32,7 +33,7 @@ static const char *sym_name(struct elf_info *elf, Elf_Sym
*sym)
return "(unknown)";
}
-void *grab_file(const char *filename, unsigned long *size)
+static void *grab_file(const char *filename, unsigned long *size)
{
struct stat st;
void *map = MAP_FAILED;
@@ -59,7 +60,7 @@ failed:
* spaces in the beginning of the line is trimmed away.
* Return a pointer to a static buffer.
**/
-void release_file(void *file, unsigned long size)
+static void release_file(void *file, unsigned long size)
{
munmap(file, size);
}
@@ -67,9 +68,8 @@ void release_file(void *file, unsigned long size)
static void *get_sym_value(struct elf_info *info, const Elf_Sym *sym)
{
- void *ptr = (void *)info->hdr + info->sechdrs[sym->st_shndx].sh_offset;
-
- return (void *)(ptr + sym->st_value);
+ return RTE_PTR_ADD(info->hdr,
+ info->sechdrs[sym->st_shndx].sh_offset + sym->st_value);
}
static Elf_Sym *find_sym_in_symtab(struct elf_info *info,
@@ -95,7 +95,6 @@ static int parse_elf(struct elf_info *info, const char
*filename)
Elf_Ehdr *hdr;
Elf_Shdr *sechdrs;
Elf_Sym *sym;
- const char *secstrings;
int endian;
unsigned int symtab_idx = ~0U, symtab_shndx_idx = ~0U;
@@ -140,7 +139,7 @@ static int parse_elf(struct elf_info *info, const char
*filename)
hdr->e_shnum = TO_NATIVE(endian, 16, hdr->e_shnum);
hdr->e_shstrndx = TO_NATIVE(endian, 16, hdr->e_shstrndx);
- sechdrs = (void *)hdr + hdr->e_shoff;
+ sechdrs = RTE_PTR_ADD(hdr, hdr->e_shoff);
info->sechdrs = sechdrs;
/* Check if file offset is correct */
@@ -191,7 +190,6 @@ static int parse_elf(struct elf_info *info, const char
*filename)
TO_NATIVE(endian, ADDR_SIZE, sechdrs[i].sh_entsize);
}
/* Find symbol table. */
- secstrings = (void *)hdr + sechdrs[info->secindex_strings].sh_offset;
for (i = 1; i < info->num_sections; i++) {
int nobits = sechdrs[i].sh_type == SHT_NOBITS;
@@ -206,22 +204,22 @@ static int parse_elf(struct elf_info *info, const char
*filename)
if (sechdrs[i].sh_type == SHT_SYMTAB) {
unsigned int sh_link_idx;
symtab_idx = i;
- info->symtab_start = (void *)hdr +
- sechdrs[i].sh_offset;
- info->symtab_stop = (void *)hdr +
- sechdrs[i].sh_offset + sechdrs[i].sh_size;
+ info->symtab_start = RTE_PTR_ADD(hdr,
+ sechdrs[i].sh_offset);
+ info->symtab_stop = RTE_PTR_ADD(hdr,
+ sechdrs[i].sh_offset + sechdrs[i].sh_size);
sh_link_idx = sechdrs[i].sh_link;
- info->strtab = (void *)hdr +
- sechdrs[sh_link_idx].sh_offset;
+ info->strtab = RTE_PTR_ADD(hdr,
+ sechdrs[sh_link_idx].sh_offset);
}
/* 32bit section no. table? ("more than 64k sections") */
if (sechdrs[i].sh_type == SHT_SYMTAB_SHNDX) {
symtab_shndx_idx = i;
-
[dpdk-dev] [PATCH v3 05/10] mk: fix verbose pmdinfogen run
When building with "make V=1" it is expected to see the output of each
compiler command in order to debug them.
Unfortunately the pmdinfogen related commands were always quiet.
It is fixed by defining the commands in some Makefile variables.
They are printed if the verbose mode is enabled.
The other benefit of this rework is to stop compilation after a
failure with pmdinfogen.
The command readlink is removed in this rework because it seems useless.
Fixes: 3d781ca32874 ("mk: do post processing on objects that register a driver")
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
mk/internal/rte.compile-pre.mk | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index 87d2d93..9c25ff6 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -84,28 +84,26 @@ C_TO_O = $(CC) -Wp,-MD,$(call obj2dep,$(@)).tmp $(CFLAGS) \
C_TO_O_STR = $(subst ','\'',$(C_TO_O)) #'# fix syntax highlight
C_TO_O_DISP = $(if $(V),"$(C_TO_O_STR)"," CC $(@)")
endif
+PMDINFO_GEN = $(RTE_SDK_BIN)/app/pmdinfogen $@ $@.pmd.c
+PMDINFO_CC = $(CC) $(CFLAGS) -c -o $@.pmd.o $@.pmd.c
+PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
+PMDINFO_TO_O = if grep -q 'PMD_REGISTER_DRIVER(.*)' $<; then \
+ echo "$(if $V,$(PMDINFO_GEN), PMDINFO $@.pmd.c)" && \
+ $(PMDINFO_GEN) && \
+ echo "$(if $V,$(PMDINFO_CC), CC $@.pmd.o)" && \
+ $(PMDINFO_CC) && \
+ echo "$(if $V,$(PMDINFO_LD), LD $@)" && \
+ $(PMDINFO_LD) && \
+ mv -f $@.o $@; fi
C_TO_O_CMD = 'cmd_$@ = $(C_TO_O_STR)'
C_TO_O_DO = @set -e; \
echo $(C_TO_O_DISP); \
$(C_TO_O) && \
- sh -c "grep -q \"PMD_REGISTER_DRIVER(.*)\" $<; \
- if [ \$$? -eq 0 ]; then \
- echo \" PMDINFOGEN\" $@; \
- OBJF=`readlink -f $@`; \
- $(RTE_SDK_BIN)/app/pmdinfogen \$$OBJF \$$OBJF.pmd.c; \
- if [ \$$? -eq 0 ]; \
- then \
- echo \" PMDINFOBUILD\" $@; \
- $(CC) $(CFLAGS) -c -o \$$OBJF.pmd.o \$$OBJF.pmd.c; \
- $(CROSS)ld $(LDFLAGS) -r -o \$$OBJF.o \$$OBJF.pmd.o
\$$OBJF; \
- mv -f \$$OBJF.o \$$OBJF; \
- fi; \
- fi;" && \
+ $(PMDINFO_TO_O) && \
echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call
obj2dep,$(@)) && \
rm -f $(call obj2dep,$(@)).tmp
-
# return an empty string if string are equal
compare = $(strip $(subst $(1),,$(2)) $(subst $(2),,$(1)))
--
2.7.0
[dpdk-dev] [PATCH v3 04/10] mk: fix driver build with installed SDK
The tool pmdinfogen was called from RTE_OUTPUT/app/ which does not exist
if building a driver outside of the SDK build.
When building DPDK, RTE_SDK_BIN is RTE_OUTPUT. When building an external
driver, RTE_SDK_BIN must point to the installed DPDK directory containing
includes, libs, etc.
That's why pmdinfogen must be installed in the SDK directory and be part
of the SDK installation.
Fixes: 3d781ca32874 ("mk: do post processing on objects that register a driver")
Signed-off-by: Thomas Monjalon
---
mk/internal/rte.compile-pre.mk | 2 +-
mk/rte.sdkinstall.mk | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index 5632d6e..87d2d93 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -92,7 +92,7 @@ C_TO_O_DO = @set -e; \
if [ \$$? -eq 0 ]; then \
echo \" PMDINFOGEN\" $@; \
OBJF=`readlink -f $@`; \
- ${RTE_OUTPUT}/app/pmdinfogen \$$OBJF \$$OBJF.pmd.c; \
+ $(RTE_SDK_BIN)/app/pmdinfogen \$$OBJF \$$OBJF.pmd.c; \
if [ \$$? -eq 0 ]; \
then \
echo \" PMDINFOBUILD\" $@; \
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index b0d985c..7cd352c 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -117,6 +117,7 @@ install-runtime:
$(Q)cp -a$O/lib/* $(DESTDIR)$(libdir)
$(Q)$(call rte_mkdir, $(DESTDIR)$(bindir))
$(Q)tar -cf - -C $O --exclude 'app/*.map' \
+ --exclude app/pmdinfogen \
--exclude 'app/cmdline*' --exclude app/test \
--exclude app/testacl --exclude app/testpipeline app | \
tar -xf - -C $(DESTDIR)$(bindir) --strip-components=1 \
@@ -126,10 +127,8 @@ install-runtime:
$(Q)$(call rte_mkdir, $(DESTDIR)$(sbindir))
$(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk_nic_bind.py,
\
$(DESTDIR)$(sbindir)/dpdk_nic_bind)
- $(Q)$(call rte_symlink,$(DESTDIR)$(bindir)/pmdinfogen, \
- $(DESTDIR)$(bindir)/dpdk_pmdinfogen)
$(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/pmdinfo.py, \
- $(DESTDIR)$(bindir)/dpdk_pmdinfo)
+ $(DESTDIR)$(bindir)/dpdk_pmdinfo)
install-kmod:
ifneq ($(wildcard $O/kmod/*),)
@@ -145,8 +144,9 @@ install-sdk:
$(Q)$(call rte_mkdir,$(DESTDIR)$(sdkdir))
$(Q)cp -a $(RTE_SDK)/mk$(DESTDIR)$(sdkdir)
$(Q)cp -a $(RTE_SDK)/scripts $(DESTDIR)$(sdkdir)
- $(Q)$(call rte_mkdir,$(DESTDIR)$(targetdir))
+ $(Q)$(call rte_mkdir,
$(DESTDIR)$(targetdir)/app)
$(Q)cp -a $O/.config $(DESTDIR)$(targetdir)
+ $(Q)cp -a $O/app/pmdinfogen
$(DESTDIR)$(targetdir)/app
$(Q)$(call rte_symlink, $(DESTDIR)$(includedir),
$(DESTDIR)$(targetdir)/include)
$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),
$(DESTDIR)$(targetdir)/lib)
--
2.7.0
[dpdk-dev] [PATCH v3 03/10] mk: remove traces of hostapp build directory
The recipe rte.hostapp.mk does not build in hostapp/ anymore.
Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
doc/guides/freebsd_gsg/build_dpdk.rst | 2 +-
doc/guides/linux_gsg/build_dpdk.rst | 2 +-
mk/rte.sdkbuild.mk| 5 ++---
mk/rte.sdkconfig.mk | 2 +-
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/doc/guides/freebsd_gsg/build_dpdk.rst
b/doc/guides/freebsd_gsg/build_dpdk.rst
index 1d92c08..93c4366 100644
--- a/doc/guides/freebsd_gsg/build_dpdk.rst
+++ b/doc/guides/freebsd_gsg/build_dpdk.rst
@@ -183,7 +183,7 @@ contains the kernel modules to install:
ls x86_64-native-bsdapp-gcc
-app build hostapp include kmod lib Makefile
+app build include kmod lib Makefile
.. _loading_contigmem:
diff --git a/doc/guides/linux_gsg/build_dpdk.rst
b/doc/guides/linux_gsg/build_dpdk.rst
index 198c0b6..fb2c481 100644
--- a/doc/guides/linux_gsg/build_dpdk.rst
+++ b/doc/guides/linux_gsg/build_dpdk.rst
@@ -152,7 +152,7 @@ A kmod directory is also present that contains kernel
modules which may be load
ls x86_64-native-linuxapp-gcc
-app build hostapp include kmod lib Makefile
+app build include kmod lib Makefile
Loading Modules to Enable Userspace IO for DPDK
---
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index 354f006..23fcf1e 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -64,9 +64,8 @@ build: $(ROOTDIRS-y)
.PHONY: clean
clean: $(CLEANDIRS)
@rm -rf $(RTE_OUTPUT)/include $(RTE_OUTPUT)/app \
- $(RTE_OUTPUT)/hostapp $(RTE_OUTPUT)/lib \
- $(RTE_OUTPUT)/hostlib $(RTE_OUTPUT)/kmod \
- $(RTE_OUTPUT)/buildtools
+ $(RTE_OUTPUT)/lib \
+ $(RTE_OUTPUT)/hostlib $(RTE_OUTPUT)/kmod
@[ -d $(RTE_OUTPUT)/include ] || mkdir -p $(RTE_OUTPUT)/include
@$(RTE_SDK)/scripts/gen-config-h.sh $(RTE_OUTPUT)/.config \
> $(RTE_OUTPUT)/include/rte_config.h
diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..068d787 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -108,7 +108,7 @@ $(RTE_OUTPUT)/Makefile: | $(RTE_OUTPUT)
# if NODOTCONF variable is defined, don't try to rebuild .config
$(RTE_OUTPUT)/include/rte_config.h: $(RTE_OUTPUT)/.config
$(Q)rm -rf $(RTE_OUTPUT)/include $(RTE_OUTPUT)/app \
- $(RTE_OUTPUT)/hostapp $(RTE_OUTPUT)/lib \
+ $(RTE_OUTPUT)/lib \
$(RTE_OUTPUT)/hostlib $(RTE_OUTPUT)/kmod $(RTE_OUTPUT)/build
$(Q)mkdir -p $(RTE_OUTPUT)/include
$(Q)$(RTE_SDK)/scripts/gen-config-h.sh $(RTE_OUTPUT)/.config \
--
2.7.0
[dpdk-dev] [PATCH v3 02/10] mk: fix build dependency of drivers on pmdinfogen
When compiling the drivers, some code is generated with pmdinfogen.
A fresh parallel build can fail if a driver is compiled before pmdinfogen:
build/buildtools/dpdk-pmdinfogen: Permission denied
There was a dependency declared in drivers/Makefile but it cannot work
because this file is based on mk/rte.subdir.mk which do not handle
dependencies.
It is fixed by declaring the whole buildtools as (order only) prerequisite
of drivers.
Fixes: cb6696d22023 ("drivers: update registration macro usage")
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
drivers/Makefile | 2 --
mk/rte.sdkbuild.mk | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile
index 75a3168..81c03a8 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -34,6 +34,4 @@ include $(RTE_SDK)/mk/rte.vars.mk
DIRS-y += net
DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
-DEPDIRS-y += buildtools/pmdinfo
-
include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index fb68af2..354f006 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -49,6 +49,7 @@ $(1): $(sort $(LOCAL_DEPDIRS-$(1)))
endef
$(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d
+drivers: | buildtools
#
# build and clean targets
--
2.7.0
[dpdk-dev] [PATCH v3 01/10] drivers: fix build with new register macro
Compilation fails because of some typos.
Fixes: cb6696d22023 ("drivers: update registration macro usage")
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
Tested-by: Adrien Mazarguil
---
drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 2 +-
drivers/net/mlx4/mlx4.c| 2 +-
drivers/net/mlx5/mlx5.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index db3e562..859a04b 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -721,6 +721,6 @@ static struct rte_driver cryptodev_aesni_mb_pmd_drv = {
.uninit = cryptodev_aesni_mb_uninit
};
-PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drvi, aesni_mb);
+PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drv, aesni_mb);
DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs= "
"max_nb_sessions= socket_id=");
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 8758eac..d9ffb13 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -5861,5 +5861,5 @@ static struct rte_driver rte_mlx4_driver = {
.init = rte_mlx4_pmd_init,
};
-PMD_REGISTER_DRIVER(rte_mlx4_driveri, mlx4)
+PMD_REGISTER_DRIVER(rte_mlx4_driver, mlx4);
DRIVER_REGISTER_PCI_TABLE(mlx4, mlx4_pci_id_map);
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 358e9b4..8d32252 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -762,5 +762,5 @@ static struct rte_driver rte_mlx5_driver = {
.init = rte_mlx5_pmd_init,
};
-PMD_REGISTER_DRIVER(rte_mlx5_driveri, mlx5)
+PMD_REGISTER_DRIVER(rte_mlx5_driver, mlx5);
DRIVER_REGISTER_PCI_TABLE(mlx5, mlx5_pci_id_map);
--
2.7.0
[dpdk-dev] [PATCH v3 00/10] additions to pmdinfogen
These are some patches that I think needed for RC2. There are other patches needed as discussed on the mailing-list which are not part of this series: - overwritten driver name (reported by Pablo) - pmdinfo.py on FreeBSD (reported by Bruce) Note: RC2 is planned to be out at the end of this week. v3 change: - do not move PCI registration macro in rte_pci.h v2 changes: - use argv[0] in usage message - drop fix of parameters registration which is better managed in Neil's patch "crypto: normalize cryptodev pmd names with macros" http://dpdk.org/ml/archives/dev/2016-July/043506.html Thomas Monjalon (10): drivers: fix build with new register macro mk: fix build dependency of drivers on pmdinfogen mk: remove traces of hostapp build directory mk: fix driver build with installed SDK mk: fix verbose pmdinfogen run pmdinfogen: fix build warnings pmdinfogen: fix usage message eal: remove PCI include from generic driver header doc: fix syntax in pmdinfogen guide maintainers: add section for pmdinfo MAINTAINERS| 4 ++ buildtools/pmdinfogen/Makefile | 4 +- buildtools/pmdinfogen/pmdinfogen.c | 61 +- doc/guides/freebsd_gsg/build_dpdk.rst | 2 +- doc/guides/linux_gsg/build_dpdk.rst| 2 +- doc/guides/prog_guide/dev_kit_build_system.rst | 5 +-- drivers/Makefile | 2 - drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 2 +- drivers/net/mlx4/mlx4.c| 2 +- drivers/net/mlx5/mlx5.c| 2 +- lib/librte_eal/common/include/rte_dev.h| 2 +- mk/internal/rte.compile-pre.mk | 26 +-- mk/rte.sdkbuild.mk | 6 +-- mk/rte.sdkconfig.mk| 2 +- mk/rte.sdkinstall.mk | 8 ++-- 15 files changed, 54 insertions(+), 76 deletions(-) -- 2.7.0
[dpdk-dev] [PATCH v3 00/11] Fix build errors related to exported headers
On Fri, Jul 08, 2016 at 02:15:37PM +, Wiles, Keith wrote:
>
> > On Jul 8, 2016, at 4:56 AM, Yigit, Ferruh wrote:
> >
> > On 7/8/2016 9:05 AM, Adrien Mazarguil wrote:
> >> On Thu, Jul 07, 2016 at 06:33:17PM +, Wiles, Keith wrote:
> >>>
> On Jul 7, 2016, at 10:49 AM, Adrien Mazarguil 6wind.com> wrote:
>
> DPDK uses GNU C language extensions in most of its code base. This is
> fine
> for internal source files whose compilation flags are controlled by DPDK,
> however user applications that use exported "public" headers may
> experience
> compilation failures when enabling strict error/standard checks (-std and
> -pedantic for instance).
> >>>
> >>> Do you try compiling these changes with CLANG and/or ICC compilers?
> >>
> >> clang/clang++ yes, works fine. I did not try with ICC however.
> >
> > I tested with icc, getting following error [1], compiler warning seems
> > valid, but didn't investigate what in your patch cause this.
> >
> > [1]
> > .../app/test/test_table_acl.c(487): error #2405: array of elements
> > containing a flexible array member is nonstandard
> >struct rte_pipeline_table_entry entries[5];
> >^
> >
> > .../app/test/test_table_acl.c(492): error #2405: array of elements
> > containing a flexible array member is nonstandard
> >struct rte_pipeline_table_entry entries_ptr[5];
>
> I am guessing it does not like the uint8_t action_data[0] in the
> rte_pipeline_table_entry structure. I can see why it would be non-standard
> allocated on the stack in this case. Maybe a keyword like __extension__ needs
> to be added or pragma.
Hmmm, right, a variable array cannot be embedded like that. I think a change
in test_table_acl.c would be better. I guess I missed a compilation check on
these tests programs. Is that the only issue you saw?
> >> Note that considering "({ ... })" is a GNU extension, compilers that do
> >> support this syntax also support the GNU __extension__ keyword. As a
> >> result,
> >> those that do not support this keyword most likely already cannot compile
> >> DPDK at all.
--
Adrien Mazarguil
6WIND
[dpdk-dev] [PATCH v2 0/3] fix coverity defects
On Tue, Jul 05, 2016 at 02:10:02PM +0800, Beilei Xing wrote: > Fix some open coverity defects. > > V2 changes: > Rework commit log. > Refactor patchset. > > Beilei Xing (3): > i40e: fix log error > i40e: fix dereference before null check > i40e: fix out-of-bounds access > > drivers/net/i40e/i40e_ethdev.c| 35 +-- > drivers/net/i40e/i40e_ethdev_vf.c | 7 +-- > 2 files changed, 26 insertions(+), 16 deletions(-) > Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] remove rte_pci_dev_ids.h
Some drivers define the supported PCI IDs, others define them in the common file rte_pci_dev_ids.h. This situation brings some confusion in the layering design. In order to clean up things, I've suggested a plan to remove rte_pci_dev_ids.h: http://dpdk.org/ml/archives/dev/2016-July/043596.html I bring it here in a new thread to give more visibility. There were 4 usages of this file: - PMDs - KNI ethtool (igb/ixgbe only) - testpmd to use ixgbe bypass - PCI autotest This is the plan to get rid of them: 1/ remove need in PCI autotest (done: http://dpdk.org/commit/1dbba165) 2/ move PCI ids in bnx2x, bnxt, e1000, enic, fm10k, i40e, virtio, vmxnet3 3/ remove KNI ethtool (only igb/ixgbe support) 4/ remove bypass API or move it to ixgbe specific API 5/ move remaining PCI ids in igb and ixgbe PMDs To make it short, please driver maintainers, move your PCI ids in your drivers as soon as possible, in order to achieve step 2. Thanks
[dpdk-dev] [PATCH v2 3/3] i40e: fix out-of-bounds access
On Wed, Jul 06, 2016 at 03:00:17AM +0100, Xing, Beilei wrote:
>
>
> > -Original Message-
> > From: Richardson, Bruce
> > Sent: Tuesday, July 5, 2016 9:26 PM
> > To: Xing, Beilei
> > Cc: Wu, Jingjing ; Jastrzebski, MichalX K
> > ; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 3/3] i40e: fix out-of-bounds access
> >
> > On Tue, Jul 05, 2016 at 02:10:05PM +0800, Beilei Xing wrote:
> > > When calling i40e_flowtype_to_pctype in
> > > i40e_get_hash_filter_global_config and
> > > i40e_set_hash_filter_global_config, function i40e_flowtype_to_pctype
> > > will be possibly out-of-bounds accessed, because size of callee's
> > > array is 15. So judge flow type before calling
> > > i40e_flowtype_to_pctype.
> > > Meanwhile do the same change in other functions.
> > >
> > > Coverity issue: 37793, 37794
> > >
> > > Fixes: 782c8c92f13f ("i40e: add hash configuration")
> > > Fixes: f2b2e2354bbd ("i40e: split function for hash and flow director
> > > input")
> > > Fixes: 98f055707685 ("i40e: configure input fields for RSS or flow
> > > director")
> > >
> > > Signed-off-by: Beilei Xing
> > > ---
> > > drivers/net/i40e/i40e_ethdev.c | 21 -
> > > 1 file changed, 12 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index a1cad37..111a552 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -6908,6 +6908,9 @@ i40e_get_hash_filter_global_config(struct
> > i40e_hw *hw,
> > > mask &= ~(1UL << i);
> > > /* Bit set indicats the coresponding flow type is supported */
> > > g_cfg->valid_bit_mask[0] |= (1UL << i);
> > > + /* if flowtype is invalid, continue */
> > > + if (!I40E_VALID_FLOW(i))
> > > + continue;
> > > pctype = i40e_flowtype_to_pctype(i);
> > > reg = i40e_read_rx_ctl(hw, I40E_GLQF_HSYM(pctype));
> > > if (reg & I40E_GLQF_HSYM_SYMH_ENA_MASK)
> >
> > Rather than having the same check done in multiple places, is there a reason
> > why we can't just put the check once in i40e_flowtype_to_pctype?
>
> Since the return value type of i40e_flowtype_to_pctype is " enum
> i40e_filter_pctype ", although put the check in i40e_flowtype_to_pctype,
> we should check return value after every i40e_flowtype_to_pctype calling.
Ok, point taken. If we don't check before the call, then the call itself could
return an error which would then need to be checked anyway.
> I think there's no more improvement.
> Besides, check valid flow type is called before i40e_flowtype_to_pctype in
> some places previously, such as function i40e_hash_filter_inset_select
> and i40e_fdir_filter_inset_select.
That's not a reason to keep on putting more checks in other places. It's more
of a reason why the check needs to be centralised. However, from your reasoning
above it doesn't help much, since for these existing cases you'd be just
changing
one check for another.
Acked-by: Bruce Richarson
[dpdk-dev] QoS config variables
Hi Can someone throw some light on the following DPDK QoS config variables? A) In struct *rte_sched_subport_params* AND *rte_sched_pipe_params* 1) tb_rate - Is the provisioned rate at which the TB can be filled while enqueuing OR the TB rate slice allotted to this subport ? 2) tb_size 3) tc_rate - Is tc_period the time slice given to each class within a subport? 4) tc_period - Does tc_rate represent the bw rate provisioning for that class? It would be nice if someone could throw the relationship between the above parameters and the QoS scheduling algorithm Thanks ! -- regards sreenaath
[dpdk-dev] [PATCH 09/11] eal: move PCI table macro
2016-07-08 09:56, Neil Horman: > On Fri, Jul 08, 2016 at 10:49:25AM +0200, David Marchand wrote: > > Hello Thomas, Neil, > > > > (will be back in a couple of days, thanks Thomas for pointing this thread) > > > > > > On Thu, Jul 7, 2016 at 6:25 PM, Thomas Monjalon > > wrote: > > > 2016-07-07 12:11, Neil Horman: > > >> On Thu, Jul 07, 2016 at 05:36:28PM +0200, Thomas Monjalon wrote: > > >> > Remove include of rte_pci.h in the generic header rte_dev.h > > >> > and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h. > > > [...] > > >> > > >> This seems strange to me, in that its odd for the driver information > > >> export > > >> macros to be spread out in multiple locations. Specifically it enjoins > > >> the use > > >> of the DRV_EXP_TAG macro, which helps centralize tag naming. Perhaps > > >> the happy > > >> medium is to place all the export macros (includnig PMD_REGISTER_DRIVER) > > >> into > > >> its own pmd_register.h header? > > > > > > I don't know. > > > David, your opinion? > > > > - The suggestion I did offline to Thomas was to move pci stuff in pci > > headers. > > We are trying to move from the "all pci" code in eal to accomodate for > > other "buses" / architectures. > I get that, but I'm not sure that applies here. The macro in question is > specific to pci busses, and if there is additional bus information to export, > it > will have its own macro (e.g. DRIVER_REGISTER_USB_TABLE or some such). While > I could see that being an argument for putting each macro in with its own bus > type, I think thats the wrong organization here, in that people writing > drivers > will want to know what export macros are available and will expect to look in > a > single place for it. > > > Having a pci macro in a generic header like rte_dev.h is wrong to me. > > Moving this to a new header like pmd_register.h sounds like a new > > generic header with pci specific stuff in it. > Well, yes, but I see that as no different than rte_ethdev.c or rte_pdump.c. > both of those files will need to know about all the different types of busses > you support and have to include those corresponding header files (i.e. they > will > have to include rte_pci.h, rte_usb.h, rte_i2c.h, etc). This is really no > different in my mind. > > > So, I am not sure I follow you Neil. > > > > Can you elaborate ? > > > I suppose the best way to describe it is that while I understand and support > the > desire to separate and abstract bus information away from device function, I > think theres a pragmatic descision here to prioritize functional domain over > header inclusion. That is to say, I think when people are writing a driver, > it > will be helpful to have all the export macros in a single location so they > know > what information they can export, and that includes registration of various > bus > type identifiers. So a file like pmd_registration.h that includes rte_pci.h, > rte_usb.h, rte_i2c.h, etc is more useful to a developer, than spreading these > macros out to those various header files, for the sake of avoiding a > potentially > unneeded include. > > > > > - Why do you want to centralise the tag naming ? > > To avoid collisions ? > Yes, and to centralize that information. Since the pmdinfogen tool needs to > know what those tag names are as well, its useful to keep them in the same > area > to maintain co-ordination. Its also useful because it means we can use one > macro to define tag naming convention, instead of having to re-implement or > dead-reckon it in multiple files. > > > Well, adding those tags should not happen that often and I think we > > can maintain this with careful reviews. > I don't agree with that. This discussion is based on the fact that you expect > that we will be adding additional bus types in the future correct? Well, > given > that we have a pci bus specific export macro, I would expect that to > proliferate > to every other bus type as well, and so we can expect to at least have a new > tag added for every bus that is added, in addition to any other bus agnostic > information people wish to export (just off hand, looking at the linux modinfo > section, we might expect module author, module version, alias names, licensing > infomration, and others to be potential export candidates). So, depending on > how much this is adopted, I think we can potentially expect a great deal of > additional tagging to be needed. Anyway, this macro do not need rte_pci.h. So the minimal patch can be to just remove this include.
[dpdk-dev] [PATCH v3 00/11] Fix build errors related to exported headers
On 7/8/2016 3:35 PM, Adrien Mazarguil wrote:
> On Fri, Jul 08, 2016 at 02:15:37PM +, Wiles, Keith wrote:
>>
>>> On Jul 8, 2016, at 4:56 AM, Yigit, Ferruh wrote:
>>>
>>> On 7/8/2016 9:05 AM, Adrien Mazarguil wrote:
On Thu, Jul 07, 2016 at 06:33:17PM +, Wiles, Keith wrote:
>
>> On Jul 7, 2016, at 10:49 AM, Adrien Mazarguil > 6wind.com> wrote:
>>
>> DPDK uses GNU C language extensions in most of its code base. This is
>> fine
>> for internal source files whose compilation flags are controlled by DPDK,
>> however user applications that use exported "public" headers may
>> experience
>> compilation failures when enabling strict error/standard checks (-std and
>> -pedantic for instance).
>
> Do you try compiling these changes with CLANG and/or ICC compilers?
clang/clang++ yes, works fine. I did not try with ICC however.
>>>
>>> I tested with icc, getting following error [1], compiler warning seems
>>> valid, but didn't investigate what in your patch cause this.
>>>
>>> [1]
>>> .../app/test/test_table_acl.c(487): error #2405: array of elements
>>> containing a flexible array member is nonstandard
>>>struct rte_pipeline_table_entry entries[5];
>>>^
>>>
>>> .../app/test/test_table_acl.c(492): error #2405: array of elements
>>> containing a flexible array member is nonstandard
>>>struct rte_pipeline_table_entry entries_ptr[5];
>>
>> I am guessing it does not like the uint8_t action_data[0] in the
>> rte_pipeline_table_entry structure. I can see why it would be non-standard
>> allocated on the stack in this case. Maybe a keyword like __extension__
>> needs to be added or pragma.
>
> Hmmm, right, a variable array cannot be embedded like that. I think a change
> in test_table_acl.c would be better.
Correct, I just wonder why not getting this same compiler warning before
your changes.
...
> Is that the only issue you saw?
>
Yes.
Note that considering "({ ... })" is a GNU extension, compilers that do
support this syntax also support the GNU __extension__ keyword. As a
result,
those that do not support this keyword most likely already cannot compile
DPDK at all.
>
[dpdk-dev] [PATCH v3 01/13] e1000: move pci device ids to driver
2016-04-20 14:43, David Marchand: > test application and kni still want to know e1000 pci devices. > So let's create headers in the driver that will be used by them. There is also an usage of ixgbe ID for the broken bypass API. Sharing those PCI ids outside of the drivers was really a wrong idea. So this a plan to get rid of them: 1/ remove need in PCI autotest (done: http://dpdk.org/commit/1dbba165) 2/ move PCI ids in bnx2x, bnxt, e1000, enic, fm10k, i40e, virtio, vmxnet3 3/ remove KNI ethtool (only igb/ixgbe support) 4/ remove bypass API or move it to ixgbe specific API 5/ move remaining PCI ids in igb and ixgbe drivers Please driver maintainers, move your PCI ids in your drivers as soon as possible. Thanks
[dpdk-dev] [PATCH] net/mlx5: fix a segmentation fault in Rx
On Fri, Jul 08, 2016 at 02:43:26PM +0200, Nelio Laranjeiro wrote:
> Fixed issue could occur when a Mbuf starvation happens in a middle of
> reception of a segmented packet, in such situation, the PMD has to release
> all segments of such packet. The end condition was wrong causing it to
> free a Mbuf still handled by the NIC.
>
> Fixes: 9964b965ad69 ("net/mlx5: re-add Rx scatter support")
>
> Reported-by: Yongseok Koh
> Signed-off-by: Nelio Laranjeiro
> ---
> drivers/net/mlx5/mlx5_rxtx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index 0c352f3..3564937 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1572,7 +1572,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
> uint16_t pkts_n)
> rte_prefetch0(wqe);
> rep = rte_mbuf_raw_alloc(rxq->mp);
> if (unlikely(rep == NULL)) {
> - while (pkt) {
> + while (pkt != seg) {
> + assert(pkt != (*rxq->elts)[idx]);
> seg = NEXT(pkt);
> rte_mbuf_refcnt_set(pkt, 0);
> __rte_mbuf_raw_free(pkt);
> --
> 2.1.4
>
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Cunming, I agree with Bruce, I'll start snipping non relevant parts considering the size of this message. Please see below. On Fri, Jul 08, 2016 at 07:11:28PM +0800, Liang, Cunming wrote: [...] > >Meta item types > >~~~ > > > >These do not match packet data but affect how the pattern is processed, most > >of them do not need a specification structure. This particularity allows > >them to be specified anywhere without affecting other item types. > [LC] For the meta item(END, VOID, INVERT) and some data matching type like > ANY and RAW, > it's all PMD responsible to understand the key character and to parse the > header graph? We haven't started on the PMD side of things yet (the public API described here does not discuss it), but I think PMDs will see the pattern as-is, untranslated (to answer your question, yes, it will be the case for END, VOID, INVERT, ANY and RAW like all others). However I intend to add private helper functions as needed in librte_ether (or anywhere else in EAL) that PMDs can call to ease parsing and validation of flow rules, otherwise most of them will end up implementing redundant functions. [...] > >When several actions are combined in a flow rule, they should all have > >different types (e.g. dropping a packet twice is not possible). However > >considering the VOID type is an exception to this rule, the defined behavior > >is for PMDs to only take into account the last action of a given type found > >in the list. PMDs still perform error checking on the entire list. > > > >*Note that PASSTHRU is the only action able to override a terminating rule.* > [LC] I'm wondering how to address the meta data carried by mbuf, there's no > mentioned here. > For packets hit one specific flow, usually there's something for CPU to > identify the flow. > FDIR and RSS as an example, has id or key in mbuf. In addition, some meta > may pointed by userdata in mbuf. > Any view on it ? Yes, this is defined as the ID action. It is described in 4.1.6.4 (ID) and there is an example how a FDIR rule would be converted to use it in 5.7 (FDIR to most item types ? QUEUE, DROP, PASSTHRU). It is basically described as a flow rule with two actions: queue redirection and packet tagging. Does it answer your question? -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH] net/enic: increment filter failure counter
One instance of a filter add failure was not incrementing the
the fail counter.
Fixes: 4c2c7bf41f5a ("net/enic: fix negative array index write")
Signed-off-by: John Daley
---
drivers/net/enic/enic_clsf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index 3365176..e6f57be 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -218,6 +218,7 @@ int enic_fdir_add_fltr(struct enic *enic, struct
rte_eth_fdir_filter *params)
pos = rte_hash_add_key(enic->fdir.hash, params);
if (pos < 0) {
+ enic->fdir.stats.f_add++;
dev_err(enic, "Add hash key failed\n");
return pos;
}
--
2.7.0
[dpdk-dev] [PATCH] net/enic: decrement Tx mbuf reference count before recycling
In the Tx cleanup function, the reference count in mbufs to be
returned to the pool should to be decremented before they are
returned. Decrementing is not done by rte_mempool_put_bulk()
so it must be done separately using __rte_pktmbuf_prefree_seg().
If decrementing does not result in a 0 reference count the mbuf
is not returned to the pool and whatever has the last reference
is responsible for freeing.
Fixes: 36935afbc53c ("net/enic: refactor Tx mbuf recycling")
Reviewed-by: Nelson Escobar
Signed-off-by: John Daley
---
Since reference counts are set to 0 when mbufs are reallocated from the
pool, and sending packets with reference count not equal to 1 is probably
an application error, this patch may not be critical. But a debug ASSERT
caught it and it would be nice to have it fixed in 16.07.
drivers/net/enic/enic_rxtx.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/enic/enic_rxtx.c b/drivers/net/enic/enic_rxtx.c
index 5ac1d69..96be478 100644
--- a/drivers/net/enic/enic_rxtx.c
+++ b/drivers/net/enic/enic_rxtx.c
@@ -398,7 +398,14 @@ static inline void enic_free_wq_bufs(struct vnic_wq *wq,
u16 completed_index)
pool = ((struct rte_mbuf *)buf->mb)->pool;
for (i = 0; i < nb_to_free; i++) {
buf = >bufs[tail_idx];
- m = (struct rte_mbuf *)(buf->mb);
+ m = __rte_pktmbuf_prefree_seg((struct rte_mbuf *)(buf->mb));
+ buf->mb = NULL;
+
+ if (unlikely(m == NULL)) {
+ tail_idx = enic_ring_incr(desc_count, tail_idx);
+ continue;
+ }
+
if (likely(m->pool == pool)) {
ENIC_ASSERT(nb_free < ENIC_MAX_WQ_DESCS);
free[nb_free++] = m;
@@ -409,7 +416,6 @@ static inline void enic_free_wq_bufs(struct vnic_wq *wq,
u16 completed_index)
pool = m->pool;
}
tail_idx = enic_ring_incr(desc_count, tail_idx);
- buf->mb = NULL;
}
rte_mempool_put_bulk(pool, (void **)free, nb_free);
--
2.7.0
[dpdk-dev] [PATCH v3 6/6] ena: fix for icc compiler
On 7/8/2016 2:23 PM, Bruce Richardson wrote:
> On Tue, Jul 05, 2016 at 09:52:09AM +0100, Ferruh Yigit wrote:
>> On 6/30/2016 4:04 PM, Jan Medala wrote:
>>> Signed-off-by: Alexander Matushevsky
>>> Signed-off-by: Jakub Palider
>>> Signed-off-by: Jan Medala
>>
>> The compilation error to fix is [1], it may be good to add what to fix
>> into commit log.
>>
>> [1]
>> == Build drivers/net/ena
>> CC ena_ethdev.o
>> /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(943): error
>> #188: enumerated type mixed with another type
>> struct ena_com_create_io_ctx ctx = { 0 };
>> ^
>>
>> /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(1036): error
>> #188: enumerated type mixed with another type
>> struct ena_com_create_io_ctx ctx = { 0 };
>> ^
>> ...
>>
>>> --- a/drivers/net/ena/ena_ethdev.c
>>> +++ b/drivers/net/ena/ena_ethdev.c
>>> @@ -940,7 +940,10 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
>>> __rte_unused unsigned int socket_id,
>>> __rte_unused const struct rte_eth_txconf *tx_conf)
>>> {
>>> - struct ena_com_create_io_ctx ctx = { 0 };
>>> + struct ena_com_create_io_ctx ctx =
>>> + /* policy set to _HOST just to satisfy icc compiler */
>>> + { ENA_ADMIN_PLACEMENT_POLICY_HOST,
>>> + ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 };
>>
>> Trailing "0" are not required, compiler will take care of them.
>>
> Actually, given that this is a structure init and not an array, the trailing
> zeros are actually required, since we are not using C99 designated
> initializers.
As far as I know same thing is valid for both array and struct.
uninitialized values set to default values.
c.c:
struct mys {
int a;
int b;
int c;
};
int main(int argc, char *argv[])
{
struct mys s = {99};
return 0;
}
gcc -S -std=c90 c.c
c.s:
...
movl%edi, -20(%rbp)
movq%rsi, -32(%rbp)
movq$0, -16(%rbp) <
movl$0, -8(%rbp)<
movl$99, -16(%rbp)
movl$0, %eax
popq%rbp
ret
...
> Therefore, I'll leave the zeros in - if anyone wants to convert the format of
> the initializers to c99 style later, and remove the trailing zeros then, it
> can
> go as a separate patch. I'll just merge this fix in with patch 1 that
> introduces
> the issue.
>
> /Bruce
>
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
Hi Sugesh, On Thu, Jul 07, 2016 at 11:15:07PM +, Chandran, Sugesh wrote: > Hi Adrien, > > Thank you for proposing this. It would be really useful for application such > as OVS-DPDK. > Please find my comments and questions inline below prefixed with [Sugesh]. > Most of them are from the perspective of enabling these APIs in application > such as OVS-DPDK. Thanks, I'm replying below. > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil > > Sent: Tuesday, July 5, 2016 7:17 PM > > To: dev at dpdk.org > > Cc: Thomas Monjalon ; Zhang, Helin > > ; Wu, Jingjing ; Rasesh > > Mody ; Ajit Khaparde > > ; Rahul Lakkireddy > > ; Lu, Wenzhuo ; > > Jan Medala ; John Daley ; Chen, > > Jing D ; Ananyev, Konstantin > > ; Matej Vido ; > > Alejandro Lucero ; Sony Chacko > > ; Jerin Jacob > > ; De Lara Guarch, Pablo > > ; Olga Shern > > Subject: [dpdk-dev] [RFC] Generic flow director/filtering/classification API > > > > Hi All, > > > > First, forgive me for this large message, I know our mailboxes already > > suffer quite a bit from the amount of traffic on this ML. > > > > This is not exactly yet another thread about how flow director should be > > extended, rather about a brand new API to handle filtering and > > classification for incoming packets in the most PMD-generic and > > application-friendly fashion we can come up with. Reasons described below. > > > > I think this topic is important enough to include both the users of this API > > as well as PMD maintainers. So far I have CC'ed librte_ether (especially > > rte_eth_ctrl.h contributors), testpmd and PMD maintainers (with and > > without > > a .filter_ctrl implementation), but if you know application maintainers > > other than testpmd who use FDIR or might be interested in this discussion, > > feel free to add them. > > > > The issues we found with the current approach are already summarized in > > the > > following document, but here is a quick summary for TL;DR folks: > > > > - PMDs do not expose a common set of filter types and even when they do, > > their behavior more or less differs. > > > > - Applications need to determine and adapt to device-specific limitations > > and quirks on their own, without help from PMDs. > > > > - Writing an application that creates flow rules targeting all devices > > supported by DPDK is thus difficult, if not impossible. > > > > - The current API has too many unspecified areas (particularly regarding > > side effects of flow rules) that make PMD implementation tricky. > > > > This RFC API handles everything currently supported by .filter_ctrl, the > > idea being to reimplement all of these to make them fully usable by > > applications in a more generic and well defined fashion. It has a very small > > set of mandatory features and an easy method to let applications probe for > > supported capabilities. > > > > The only downside is more work for the software control side of PMDs > > because > > they have to adapt to the API instead of the reverse. I think helpers can be > > added to EAL to assist with this. > > > > HTML version: > > > > https://rawgit.com/6WIND/rte_flow/master/rte_flow.html > > > > PDF version: > > > > https://rawgit.com/6WIND/rte_flow/master/rte_flow.pdf > > > > Related draft header file (for reference while reading the specification): > > > > https://raw.githubusercontent.com/6WIND/rte_flow/master/rte_flow.h > > > > Git tree for completeness (latest .rst version can be retrieved from here): > > > > https://github.com/6WIND/rte_flow > > > > What follows is the ReST source of the above, for inline comments and > > discussion. I intend to update that specification accordingly. > > > > > > Generic filter interface > > > > > > .. footer:: > > > >v0.6 > > > > .. contents:: > > .. sectnum:: > > .. raw:: pdf > > > >PageBreak > > > > Overview > > > > > > DPDK provides several competing interfaces added over time to perform > > packet > > matching and related actions such as filtering and classification. > > > > They must be extended to implement the features supported by newer > > devices > > in order to expose them to applications, however the current design has > > several drawbacks: > > > > - Complicated filter combinations which have not been hard-coded cannot be > > expressed. > > - Prone to API/ABI breakage when new features must be added to an > > existing > > filter type, which frequently happens. > > > > From an application point of view: > > > > - Having disparate interfaces, all optional and lacking in features does not > > make this API easy to use. > > - Seemingly arbitrary built-in limitations of filter types based on the > > device they were initially designed for. > > - Undefined relationship between different filter types. > > - High complexity, considerable undocumented and/or undefined behavior. > > > > Considering the
[dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address.
On 06.07.2016 15:24, Yuanhan Liu wrote:
> On Wed, Jul 06, 2016 at 02:19:12PM +0300, Ilya Maximets wrote:
>> On 01.07.2016 10:35, Yuanhan Liu wrote:
>>> Hi,
>>>
>>> Sorry for the long delay.
>>>
>>> On Fri, May 20, 2016 at 03:50:04PM +0300, Ilya Maximets wrote:
In current implementation guest application can reinitialize vrings
by executing start after stop. In the same time host application
can still poll virtqueue while device stopped in guest and it will
crash with segmentation fault while vring reinitialization because
of dereferencing of bad descriptor addresses.
>>>
>>> Yes, you are right that vring will be reinitialized after restart.
>>> But even though, I don't see the reason it will cause a vhost crash,
>>> since the reinitialization will reset all the vring memeory by 0:
>>>
>>> memset(vq->vq_ring_virt_mem, 0, vq->vq_ring_size);
>>>
>>> That means those bad descriptors will be skipped, safely, at vhost
>>> side by:
>>>
>>> if (unlikely(desc->len < dev->vhost_hlen))
>>> return -1;
>>>
OVS crash for example:
<>
[test-pmd inside guest VM]
testpmd> port stop all
Stopping ports...
Checking link statuses...
Port 0 Link Up - speed 1 Mbps - full-duplex
Done
testpmd> port config all rxq 2
testpmd> port config all txq 2
testpmd> port start all
Configuring Port 0 (socket 0)
Port 0: 52:54:00:CB:44:C8
Checking link statuses...
Port 0 Link Up - speed 1 Mbps - full-duplex
Done
[OVS on host]
Program received signal SIGSEGV, Segmentation fault.
rte_memcpy (n=2056, src=0xc, dst=0x7ff4d5247000) at rte_memcpy.h
>>>
>>> Interesting, so it bypasses the above check since desc->len is non-zero
>>> while desc->addr is zero. The size (2056) also looks weird.
>>>
>>> Do you mind to check this issue a bit deeper, say why desc->addr is
>>> zero, however, desc->len is not?
>>
>> OK. I checked this few more times.
>
> Thanks!
>
>> Actually, I see, that desc->addr is
>> not zero. All desc memory looks like some rubbish:
>>
>> <-->
>> (gdb)
>> #3 copy_desc_to_mbuf (mbuf_pool=0x7fe9da9f4480, desc_idx=65363,
>> m=0x7fe9db269400, vq=0x7fe9fff7bac0,
>> dev=0x7fe9fff7cbc0)
>> desc = 0x2aabc00ff530
>> desc_addr = 0
>> mbuf_offset = 0
>> prev = 0x7fe9db269400
>> nr_desc = 1
>> desc_offset = 12
>> cur = 0x7fe9db269400
>> hdr = 0x0
>> desc_avail = 1012591375
>> mbuf_avail = 1526
>> cpy_len = 1526
>>
>> (gdb) p *desc
>> $2 = {addr = 8507655620301055744, len = 1012591387, flags = 3845, next =
>> 48516}
>>
>> <-->
>>
>> And 'desc_addr' equals zero because 'gpa_to_vva' just can't map this huge
>> address to host's.
>>
>> Scenario was the same. SIGSEGV received right after 'port start all'.
>>
>> Another thought:
>>
>> Actually, there is a race window between 'memset' in guest and reading
>> of 'desc->len' and 'desc->addr' on host. So, it's possible to read non
>> zero 'len' and zero 'addr' right after that.
>
> That's also what I was thinking, that it should the only reason caused
> such issue.
>
>> But you're right, this case should be very rare.
>
> Yes, it should be very rare. What troubles me is that seems you can
> reproduce this issue very easily, that I doubt it's caused by this
> rare race. The reason could be something else?
I don't know what exactly happens, but it constantly happens on 'port start all'
command execution. Descriptors becomes broken for some time and after that time
descriptors becomes normal. I think so, because with my patch applied network
is working. It means, that broken descriptors appears for some little time
while virtio restarting with new configuration.
Another point is that crash constantly happens on queue_id=3 (second RX queue)
in
my scenario. It is newly allocated virtqueue while reconfiguration from rxq=1 to
rxq=2.
Obviously, virtio needs to be fixed, but we need to check address anyway on
vhost side, because we don't know what happens in guest in common case.
Best regards, Ilya Maximets.
[dpdk-dev] [PATCH] net/mlx5: fix a segmentation fault in Rx
Fixed issue could occur when a Mbuf starvation happens in a middle of
reception of a segmented packet, in such situation, the PMD has to release
all segments of such packet. The end condition was wrong causing it to
free a Mbuf still handled by the NIC.
Fixes: 9964b965ad69 ("net/mlx5: re-add Rx scatter support")
Reported-by: Yongseok Koh
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5_rxtx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 0c352f3..3564937 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1572,7 +1572,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts,
uint16_t pkts_n)
rte_prefetch0(wqe);
rep = rte_mbuf_raw_alloc(rxq->mp);
if (unlikely(rep == NULL)) {
- while (pkt) {
+ while (pkt != seg) {
+ assert(pkt != (*rxq->elts)[idx]);
seg = NEXT(pkt);
rte_mbuf_refcnt_set(pkt, 0);
__rte_mbuf_raw_free(pkt);
--
2.1.4
[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros
2016-07-08 08:17, Neil Horman: > I suppose thats fine, sure, though I'm really not comfortable with an open > source project requiring what appears to be non-open source components > (though I > can't really tell what the snow3g or kasumi license is). Yes you're right. IMHO, it is acceptable because it is free (just need a registration) but it can be discussed. I think there are some legal export issues because of crypto aim. I suggest to avoid discussing this topic here and launch a dedicated thread (with better visibility).
[dpdk-dev] [PATCH] bnxt: Fix a bug in broadcast/multicast setting
On Mon, Jul 04, 2016 at 01:52:04PM -0500, Ajit Khaparde wrote:
> Currently we are wrongly setting HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_MCAST
> flag in bnxt_hwrm_cfa_l2_set_rx_mask() which is preventing promiscuous
> and multicast promiscuous settings from working correctly.
> This patch fixes it.
>
> Signed-off-by: Ajit Khaparde
Fixes: 244bc98b0da7 ("net/bnxt: set L2 Rx mask")
Applied to dpdk-next-net/rel_16_07
/Bruce
[dpdk-dev] [PATCH v3 0/6] ena: update PMD to cooperate with latest ENA firmware
On Thu, Jun 30, 2016 at 05:04:53PM +0200, Jan Medala wrote: > v2: As requested, big patch splitted into logical pieces for easier review. > Improved style and fixed icc compiler issues. > > v3: Added commit messages to clarify patch purpose and minor changes > > Jan Medala (6): > ena: update of ENA communication layer > ena: add debug area and host information > ena: disable readless communication regarding to HW revision > ena: allocate coherent memory in node-aware way > ena: fix memory management issue > ena: fix for icc compiler > Applied to dpdk-next-net/rel_16_07 with patch 6 squashed into patch 1 /Bruce
[dpdk-dev] [PATCH v3 6/6] ena: fix for icc compiler
On Tue, Jul 05, 2016 at 09:52:09AM +0100, Ferruh Yigit wrote:
> On 6/30/2016 4:04 PM, Jan Medala wrote:
> > Signed-off-by: Alexander Matushevsky
> > Signed-off-by: Jakub Palider
> > Signed-off-by: Jan Medala
>
> The compilation error to fix is [1], it may be good to add what to fix
> into commit log.
>
> [1]
> == Build drivers/net/ena
> CC ena_ethdev.o
> /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(943): error
> #188: enumerated type mixed with another type
> struct ena_com_create_io_ctx ctx = { 0 };
> ^
>
> /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(1036): error
> #188: enumerated type mixed with another type
> struct ena_com_create_io_ctx ctx = { 0 };
> ^
> ...
>
> > --- a/drivers/net/ena/ena_ethdev.c
> > +++ b/drivers/net/ena/ena_ethdev.c
> > @@ -940,7 +940,10 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
> > __rte_unused unsigned int socket_id,
> > __rte_unused const struct rte_eth_txconf *tx_conf)
> > {
> > - struct ena_com_create_io_ctx ctx = { 0 };
> > + struct ena_com_create_io_ctx ctx =
> > + /* policy set to _HOST just to satisfy icc compiler */
> > + { ENA_ADMIN_PLACEMENT_POLICY_HOST,
> > + ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 };
>
> Trailing "0" are not required, compiler will take care of them.
>
Actually, given that this is a structure init and not an array, the trailing
zeros are actually required, since we are not using C99 designated initializers.
Therefore, I'll leave the zeros in - if anyone wants to convert the format of
the initializers to c99 style later, and remove the trailing zeros then, it can
go as a separate patch. I'll just merge this fix in with patch 1 that introduces
the issue.
/Bruce
[dpdk-dev] [PATCH v3 00/11] Fix build errors related to exported headers
> On Jul 8, 2016, at 4:56 AM, Yigit, Ferruh wrote:
>
> On 7/8/2016 9:05 AM, Adrien Mazarguil wrote:
>> On Thu, Jul 07, 2016 at 06:33:17PM +, Wiles, Keith wrote:
>>>
On Jul 7, 2016, at 10:49 AM, Adrien Mazarguil >>> 6wind.com> wrote:
DPDK uses GNU C language extensions in most of its code base. This is fine
for internal source files whose compilation flags are controlled by DPDK,
however user applications that use exported "public" headers may experience
compilation failures when enabling strict error/standard checks (-std and
-pedantic for instance).
>>>
>>> Do you try compiling these changes with CLANG and/or ICC compilers?
>>
>> clang/clang++ yes, works fine. I did not try with ICC however.
>
> I tested with icc, getting following error [1], compiler warning seems
> valid, but didn't investigate what in your patch cause this.
>
> [1]
> .../app/test/test_table_acl.c(487): error #2405: array of elements
> containing a flexible array member is nonstandard
>struct rte_pipeline_table_entry entries[5];
>^
>
> .../app/test/test_table_acl.c(492): error #2405: array of elements
> containing a flexible array member is nonstandard
>struct rte_pipeline_table_entry entries_ptr[5];
I am guessing it does not like the uint8_t action_data[0] in the
rte_pipeline_table_entry structure. I can see why it would be non-standard
allocated on the stack in this case. Maybe a keyword like __extension__ needs
to be added or pragma.
>
>>
>> Note that considering "({ ... })" is a GNU extension, compilers that do
>> support this syntax also support the GNU __extension__ keyword. As a result,
>> those that do not support this keyword most likely already cannot compile
>> DPDK at all.
>>
>
[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros
> -Original Message-
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Friday, July 08, 2016 1:18 PM
> To: De Lara Guarch, Pablo
> Cc: dev at dpdk.org; Richardson, Bruce; Thomas Monjalon; Stephen
> Hemminger; Panu Matilainen
> Subject: Re: [PATCH] crypto: normalize cryptodev pmd names with macros
>
> On Fri, Jul 08, 2016 at 09:09:10AM +, De Lara Guarch, Pablo wrote:
> > Hi Neil,
> >
> > > -Original Message-
> > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > Sent: Thursday, July 07, 2016 4:38 PM
> > > To: dev at dpdk.org
> > > Cc: Neil Horman; Richardson, Bruce; Thomas Monjalon; De Lara Guarch,
> > > Pablo; Stephen Hemminger; Panu Matilainen
> > > Subject: [PATCH] crypto: normalize cryptodev pmd names with macros
> > >
> > > Recently reported, the introduction of pmd information exports led to a
> > > breakage of cryptodev unit tests because the test infrastructure relies on
> the
> > > cryptodev names being available in macros. This patch fixes the pmd
> naming
> > > to
> > > use the macro names. Note that the macro names were already pre-
> > > stringified,
> > > which won't work as the PMD_REGISTER_DRIVER macro requires the
> name in
> > > both a
> > > processing token and stringified form. As such the names are defined now
> as
> > > tokens, and converted where needed to stringified form on demand using
> > > RTE_STR.
> > >
> > > Tested using the null and aesni_mb crypto pmds, as I don't have hardware
> for
> > > any
> > > other device. Also not build tested on snow3g or kasumi pmd because
> > > building
> > > those requires external libraries that appear to not be open source
> licensed.
> > >
> > > Signed-off-by: Neil Horman
> > > CC: Bruce Richardson
> > > CC: Thomas Monjalon
> > > CC: "De Lara Guarch, Pablo"
> > > CC: Stephen Hemminger
> > > CC: Panu Matilainen
> > > ---
> > > app/test/test_cryptodev.c | 20
> > > ++--
> > > app/test/test_cryptodev_perf.c | 18
> > > +-
> > > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 +++
> > > drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 6 +++---
> > > drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 7 +++
> > > drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 2 +-
> > > drivers/crypto/kasumi/rte_kasumi_pmd.c | 5 ++---
> > > drivers/crypto/null/null_crypto_pmd.c | 7 +++
> > > drivers/crypto/null/null_crypto_pmd_private.h | 6 +++---
> > > drivers/crypto/qat/rte_qat_cryptodev.c | 4 ++--
> > > drivers/crypto/snow3g/rte_snow3g_pmd.c | 4 ++--
> > > lib/librte_cryptodev/rte_cryptodev.h | 12 ++--
> > > 12 files changed, 47 insertions(+), 51 deletions(-)
> >
> > Thanks for this patch. I tested snow3g and kasumi, and they don't compile.
> > I have a fix for that, so I can send a v2 of this patch if it is OK for you.
> >
>
> I suppose thats fine, sure, though I'm really not comfortable with an open
> source project requiring what appears to be non-open source components
> (though I
> can't really tell what the snow3g or kasumi license is). Regardless, whats
> the
> compilation error?
drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function
'snow3g_pmd_qp_create_processed_ops_ring':
drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: error:
'cryptodev_snow3g_pmd' undeclared (first use in this function)
SNOW3G_LOG_ERR("Unable to reuse existing ring %s"
^
dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: note: each
undeclared identifier is reported only once for each function it appears in
dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function
'snow3g_pmd_session_configure':
dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:296:117: error:
'cryptodev_snow3g_pmd' undeclared (first use in this function)
SNOW3G_LOG_ERR("invalid session struct");
...
The solution is adding RTE_STR in SNOW3G_LOG_ERR.
...
>
>
> > Also, we should make these changes in the other PMDs as well, right?
> > I mean, avoid setting the name of the driver directly in the structure and
> > go
> back to the original name.
> > I can do that as well, if you want (maybe a separate patch, as this one is
> only related to crypto).
> >
> I think thats kind of two questions:
>
> 1) Should we remove the static setting of the name in the pmd_driver
> structure
> in favor of doing it in the registration macro?
>
> 2) Should we be consistent in the name conversion (from the setting in the
> structure instance definition to the string in the macro parameter)?
>
> The answer to (1) is yes, though having it in both places is harmless, since
> the
> former will just get overridden. We should definately remove the static
> setting, to avoid
[dpdk-dev] [RFC] Generic flow director/filtering/classification API
On Fri, Jul 08, 2016 at 07:11:28PM +0800, Liang, Cunming wrote: Please Cunming, when replying in the middle of a long email - of which this is a perfect example - delete the large chunks of content you are not replying to. I had to page down multiple times to find the new comments you wrote, and even then I wasn't sure that I hadn't missed something along the way. /Bruce > Hi Adrien, > > On 7/6/2016 2:16 AM, Adrien Mazarguil wrote: > >Hi All, > >
[dpdk-dev] [PATCH v2] ena: fix doorbell submission when not needed
Avoid submitting doorbell when:
* no packets have been submitted to TX
* no free resources have been submitted while RX
Sending doorbell without actual work to be performed by device
violates ENA specification and can lead to unpredictable behavior.
Fixes: 1173fca25af9 ("ena: add polling-mode driver")
Signed-off-by: Alexander Matushevsky
Signed-off-by: Jan Medala
---
drivers/net/ena/ena_ethdev.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 702289b..d68e7ec 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -920,10 +920,14 @@ static int ena_populate_rx_queue(struct ena_ring *rxq,
unsigned int count)
next_to_use = ENA_RX_RING_IDX_NEXT(next_to_use, ring_size);
}
- rte_wmb();
- rxq->next_to_use = next_to_use;
- /* let HW know that it can fill buffers with data */
- ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
+ /* When we submitted free recources to device... */
+ if (i > 0) {
+ /* ...let HW know that it can fill buffers with data */
+ rte_wmb();
+ ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
+
+ rxq->next_to_use = next_to_use;
+ }
return i;
}
@@ -1316,7 +1320,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct
rte_mbuf **tx_pkts,
struct ena_tx_buffer *tx_info;
struct ena_com_buf *ebuf;
uint16_t rc, req_id, total_tx_descs = 0;
- int sent_idx = 0;
+ uint16_t sent_idx = 0;
int nb_hw_desc;
/* Check adapter state */
@@ -1395,9 +1399,14 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct
rte_mbuf **tx_pkts,
next_to_use = ENA_TX_RING_IDX_NEXT(next_to_use, ring_size);
}
- /* Let HW do it's best :-) */
- rte_wmb();
- ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+ /* If there are ready packets to be xmitted... */
+ if (sent_idx > 0) {
+ /* ...let HW do its best :-) */
+ rte_wmb();
+ ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+
+ tx_ring->next_to_use = next_to_use;
+ }
/* Clear complete packets */
while (ena_com_tx_comp_req_id_get(tx_ring->ena_com_io_cq, _id) >=
0) {
@@ -1420,9 +1429,11 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct
rte_mbuf **tx_pkts,
break;
}
- /* acknowledge completion of sent packets */
- ena_com_comp_ack(tx_ring->ena_com_io_sq, total_tx_descs);
- tx_ring->next_to_use = next_to_use;
+ if (total_tx_descs > 0) {
+ /* acknowledge completion of sent packets */
+ ena_com_comp_ack(tx_ring->ena_com_io_sq, total_tx_descs);
+ }
+
return sent_idx;
}
--
2.9.0
[dpdk-dev] [PATCH v2] ena: doorbell fix
v2: resend patch, as previous one probably had some format corruption. Jan Medala (1): ena: fix doorbell submission when not needed drivers/net/ena/ena_ethdev.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) -- 2.9.0
[dpdk-dev] xstats performance
On 07/07/2016 23:59, Rasesh Mody wrote: [..] > We have submitted the QEDE and BNX2X PMD xstats patches with recent > API changes. Seen & Acked. > During the xstats change verification, it was observed that the order > in which xstats names are fetched don't seem to match the order in > which xstats values are fetched. When populating the xstats names, we > order them as driver specific xstats names and then standard stats > names. Whereas, while populating the xstats values, we populate the > standard stats values and then the driver specific xstats values. Had a look. Patch on the way.. ..Remy
[dpdk-dev] [PATCH v2 10/10] maintainers: add section for pmdinfo
The author of this feature is Neil Horman. Signed-off-by: Thomas Monjalon Acked-by: Neil Horman --- MAINTAINERS | 4 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a59191e..f996c2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -68,6 +68,10 @@ F: lib/librte_compat/ F: doc/guides/rel_notes/deprecation.rst F: scripts/validate-abi.sh +Driver information +F: buildtools/pmdinfogen/ +F: tools/pmdinfo.py + Environment Abstraction Layer - -- 2.7.0
[dpdk-dev] [PATCH v2 09/10] doc: fix syntax in pmdinfogen guide
Sphynx reports this error:
doc/guides/prog_guide/dev_kit_build_system.rst:337: WARNING:
Pygments lexer name u'C' is not known
Fixes: 737ddf3fb ("doc: add prog guide section documenting pmdinfo script")
Reported-by: Bernard Iremonger
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
doc/guides/prog_guide/dev_kit_build_system.rst | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst
b/doc/guides/prog_guide/dev_kit_build_system.rst
index 1dc1388..18a3010 100644
--- a/doc/guides/prog_guide/dev_kit_build_system.rst
+++ b/doc/guides/prog_guide/dev_kit_build_system.rst
@@ -319,8 +319,7 @@ instance the macro:
.. code-block:: c
-PMD_REGISTER_DRIVER(drv, name)
-
+ PMD_REGISTER_DRIVER(drv, name)
Creates the following symbol:
@@ -334,7 +333,7 @@ can be exported from the object file and used to produce a
hardware support
description, that pmdinfogen then encodes into a json formatted string in the
following format:
-.. code-block:: C
+.. code-block:: c
static char ="PMD_INFO_STRING=\"{'name' : '',
...}\"";
--
2.7.0
[dpdk-dev] [PATCH v2 08/10] eal: move PCI table macro
Remove include of rte_pci.h in the generic header rte_dev.h
and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h.
Fixes: cb6696d22023 ("drivers: update registration macro usage")
Suggested-by: David Marchand
Signed-off-by: Thomas Monjalon
---
lib/librte_eal/common/include/rte_dev.h | 6 +-
lib/librte_eal/common/include/rte_pci.h | 5 +
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_dev.h
b/lib/librte_eal/common/include/rte_dev.h
index e6f0d4c..68ca7ef 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -48,7 +48,7 @@ extern "C" {
#include
#include
-#include
+
#include
__attribute__((format(printf, 2, 0)))
@@ -195,10 +195,6 @@ DRIVER_EXPORT_NAME(nm, __COUNTER__)
#define DRV_EXP_TAG(name, tag) __##name##_##tag
-#define DRIVER_REGISTER_PCI_TABLE(name, table) \
-static const char DRV_EXP_TAG(name, pci_tbl_export)[] __attribute__((used)) = \
-RTE_STR(table)
-
#define DRIVER_REGISTER_PARAM_STRING(name, str) \
static const char DRV_EXP_TAG(name, param_string_export)[] \
__attribute__((used)) = str
diff --git a/lib/librte_eal/common/include/rte_pci.h
b/lib/librte_eal/common/include/rte_pci.h
index fa74962..3b0d26a 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -188,6 +188,11 @@ struct rte_pci_device {
.subsystem_device_id = PCI_ANY_ID
#endif
+#define DRIVER_REGISTER_PCI_TABLE(name, table) \
+static const char __##name##_pci_tbl_export[] \
+ __attribute__((used)) = \
+ RTE_STR(table)
+
struct rte_pci_driver;
/**
--
2.7.0
[dpdk-dev] [PATCH v2 07/10] pmdinfogen: fix usage message
The name of the tool is pmdinfogen.
Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
Signed-off-by: Thomas Monjalon
---
buildtools/pmdinfogen/pmdinfogen.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/buildtools/pmdinfogen/pmdinfogen.c
b/buildtools/pmdinfogen/pmdinfogen.c
index beb06f1..717c8d4 100644
--- a/buildtools/pmdinfogen/pmdinfogen.c
+++ b/buildtools/pmdinfogen/pmdinfogen.c
@@ -398,7 +398,8 @@ int main(int argc, char **argv)
if (argc < 3) {
fprintf(stderr,
- "usage: pmdinfo \n");
+ "usage: %s \n",
+ basename(argv[0]));
exit(127);
}
parse_elf(, argv[1]);
--
2.7.0
[dpdk-dev] [PATCH v2 06/10] pmdinfogen: fix build warnings
When compiled with a standard clang, pmdinfogen can raise a warning:
buildtools/pmdinfogen/pmdinfogen.c:365:1: warning:
control reaches end of non-void function
Actually there can be more warnings with stricter compilers.
In order to catch them early and fix most of them, the DPDK standard flags
WERROR_FLAGS are used.
The warnings fixed are:
no previous prototype for ...
no return statement in function returning non-void
variable ?secstrings? set but not used
?sec_name? defined but not used
?get_symbol_index? defined but not used
pointer of type ?void *? used in arithmetic
Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
Signed-off-by: Thomas Monjalon
---
buildtools/pmdinfogen/Makefile | 4 +--
buildtools/pmdinfogen/pmdinfogen.c | 58 --
2 files changed, 20 insertions(+), 42 deletions(-)
diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile
index 125901b..3885d3b 100644
--- a/buildtools/pmdinfogen/Makefile
+++ b/buildtools/pmdinfogen/Makefile
@@ -41,9 +41,9 @@ HOSTAPP = pmdinfogen
#
SRCS-y += pmdinfogen.c
-HOST_EXTRA_CFLAGS += -g -I${RTE_OUTPUT}/include
+HOST_CFLAGS += $(WERROR_FLAGS) -g
+HOST_CFLAGS += -I$(RTE_OUTPUT)/include
DEPDIRS-y += lib/librte_eal
include $(RTE_SDK)/mk/rte.hostapp.mk
-
diff --git a/buildtools/pmdinfogen/pmdinfogen.c
b/buildtools/pmdinfogen/pmdinfogen.c
index 0947dc6..beb06f1 100644
--- a/buildtools/pmdinfogen/pmdinfogen.c
+++ b/buildtools/pmdinfogen/pmdinfogen.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include "pmdinfogen.h"
#ifdef RTE_ARCH_64
@@ -32,7 +33,7 @@ static const char *sym_name(struct elf_info *elf, Elf_Sym
*sym)
return "(unknown)";
}
-void *grab_file(const char *filename, unsigned long *size)
+static void *grab_file(const char *filename, unsigned long *size)
{
struct stat st;
void *map = MAP_FAILED;
@@ -59,7 +60,7 @@ failed:
* spaces in the beginning of the line is trimmed away.
* Return a pointer to a static buffer.
**/
-void release_file(void *file, unsigned long size)
+static void release_file(void *file, unsigned long size)
{
munmap(file, size);
}
@@ -67,9 +68,8 @@ void release_file(void *file, unsigned long size)
static void *get_sym_value(struct elf_info *info, const Elf_Sym *sym)
{
- void *ptr = (void *)info->hdr + info->sechdrs[sym->st_shndx].sh_offset;
-
- return (void *)(ptr + sym->st_value);
+ return RTE_PTR_ADD(info->hdr,
+ info->sechdrs[sym->st_shndx].sh_offset + sym->st_value);
}
static Elf_Sym *find_sym_in_symtab(struct elf_info *info,
@@ -95,7 +95,6 @@ static int parse_elf(struct elf_info *info, const char
*filename)
Elf_Ehdr *hdr;
Elf_Shdr *sechdrs;
Elf_Sym *sym;
- const char *secstrings;
int endian;
unsigned int symtab_idx = ~0U, symtab_shndx_idx = ~0U;
@@ -140,7 +139,7 @@ static int parse_elf(struct elf_info *info, const char
*filename)
hdr->e_shnum = TO_NATIVE(endian, 16, hdr->e_shnum);
hdr->e_shstrndx = TO_NATIVE(endian, 16, hdr->e_shstrndx);
- sechdrs = (void *)hdr + hdr->e_shoff;
+ sechdrs = RTE_PTR_ADD(hdr, hdr->e_shoff);
info->sechdrs = sechdrs;
/* Check if file offset is correct */
@@ -191,7 +190,6 @@ static int parse_elf(struct elf_info *info, const char
*filename)
TO_NATIVE(endian, ADDR_SIZE, sechdrs[i].sh_entsize);
}
/* Find symbol table. */
- secstrings = (void *)hdr + sechdrs[info->secindex_strings].sh_offset;
for (i = 1; i < info->num_sections; i++) {
int nobits = sechdrs[i].sh_type == SHT_NOBITS;
@@ -206,22 +204,22 @@ static int parse_elf(struct elf_info *info, const char
*filename)
if (sechdrs[i].sh_type == SHT_SYMTAB) {
unsigned int sh_link_idx;
symtab_idx = i;
- info->symtab_start = (void *)hdr +
- sechdrs[i].sh_offset;
- info->symtab_stop = (void *)hdr +
- sechdrs[i].sh_offset + sechdrs[i].sh_size;
+ info->symtab_start = RTE_PTR_ADD(hdr,
+ sechdrs[i].sh_offset);
+ info->symtab_stop = RTE_PTR_ADD(hdr,
+ sechdrs[i].sh_offset + sechdrs[i].sh_size);
sh_link_idx = sechdrs[i].sh_link;
- info->strtab = (void *)hdr +
- sechdrs[sh_link_idx].sh_offset;
+ info->strtab = RTE_PTR_ADD(hdr,
+ sechdrs[sh_link_idx].sh_offset);
}
/* 32bit section no. table? ("more than 64k sections") */
if (sechdrs[i].sh_type == SHT_SYMTAB_SHNDX) {
symtab_shndx_idx = i;
-
[dpdk-dev] [PATCH v2 05/10] mk: fix verbose pmdinfogen run
When building with "make V=1" it is expected to see the output of each
compiler command in order to debug them.
Unfortunately the pmdinfogen related commands were always quiet.
It is fixed by defining the commands in some Makefile variables.
They are printed if the verbose mode is enabled.
The other benefit of this rework is to stop compilation after a
failure with pmdinfogen.
The command readlink is removed in this rework because it seems useless.
Fixes: 3d781ca32874 ("mk: do post processing on objects that register a driver")
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
mk/internal/rte.compile-pre.mk | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index 87d2d93..9c25ff6 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -84,28 +84,26 @@ C_TO_O = $(CC) -Wp,-MD,$(call obj2dep,$(@)).tmp $(CFLAGS) \
C_TO_O_STR = $(subst ','\'',$(C_TO_O)) #'# fix syntax highlight
C_TO_O_DISP = $(if $(V),"$(C_TO_O_STR)"," CC $(@)")
endif
+PMDINFO_GEN = $(RTE_SDK_BIN)/app/pmdinfogen $@ $@.pmd.c
+PMDINFO_CC = $(CC) $(CFLAGS) -c -o $@.pmd.o $@.pmd.c
+PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
+PMDINFO_TO_O = if grep -q 'PMD_REGISTER_DRIVER(.*)' $<; then \
+ echo "$(if $V,$(PMDINFO_GEN), PMDINFO $@.pmd.c)" && \
+ $(PMDINFO_GEN) && \
+ echo "$(if $V,$(PMDINFO_CC), CC $@.pmd.o)" && \
+ $(PMDINFO_CC) && \
+ echo "$(if $V,$(PMDINFO_LD), LD $@)" && \
+ $(PMDINFO_LD) && \
+ mv -f $@.o $@; fi
C_TO_O_CMD = 'cmd_$@ = $(C_TO_O_STR)'
C_TO_O_DO = @set -e; \
echo $(C_TO_O_DISP); \
$(C_TO_O) && \
- sh -c "grep -q \"PMD_REGISTER_DRIVER(.*)\" $<; \
- if [ \$$? -eq 0 ]; then \
- echo \" PMDINFOGEN\" $@; \
- OBJF=`readlink -f $@`; \
- $(RTE_SDK_BIN)/app/pmdinfogen \$$OBJF \$$OBJF.pmd.c; \
- if [ \$$? -eq 0 ]; \
- then \
- echo \" PMDINFOBUILD\" $@; \
- $(CC) $(CFLAGS) -c -o \$$OBJF.pmd.o \$$OBJF.pmd.c; \
- $(CROSS)ld $(LDFLAGS) -r -o \$$OBJF.o \$$OBJF.pmd.o
\$$OBJF; \
- mv -f \$$OBJF.o \$$OBJF; \
- fi; \
- fi;" && \
+ $(PMDINFO_TO_O) && \
echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call
obj2dep,$(@)) && \
rm -f $(call obj2dep,$(@)).tmp
-
# return an empty string if string are equal
compare = $(strip $(subst $(1),,$(2)) $(subst $(2),,$(1)))
--
2.7.0
[dpdk-dev] [PATCH v2 04/10] mk: fix driver build with installed SDK
The tool pmdinfogen was called from RTE_OUTPUT/app/ which does not exist
if building a driver outside of the SDK build.
When building DPDK, RTE_SDK_BIN is RTE_OUTPUT. When building an external
driver, RTE_SDK_BIN must point to the installed DPDK directory containing
includes, libs, etc.
That's why pmdinfogen must be installed in the SDK directory and be part
of the SDK installation.
Fixes: 3d781ca32874 ("mk: do post processing on objects that register a driver")
Signed-off-by: Thomas Monjalon
---
mk/internal/rte.compile-pre.mk | 2 +-
mk/rte.sdkinstall.mk | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index 5632d6e..87d2d93 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -92,7 +92,7 @@ C_TO_O_DO = @set -e; \
if [ \$$? -eq 0 ]; then \
echo \" PMDINFOGEN\" $@; \
OBJF=`readlink -f $@`; \
- ${RTE_OUTPUT}/app/pmdinfogen \$$OBJF \$$OBJF.pmd.c; \
+ $(RTE_SDK_BIN)/app/pmdinfogen \$$OBJF \$$OBJF.pmd.c; \
if [ \$$? -eq 0 ]; \
then \
echo \" PMDINFOBUILD\" $@; \
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index b0d985c..7cd352c 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -117,6 +117,7 @@ install-runtime:
$(Q)cp -a$O/lib/* $(DESTDIR)$(libdir)
$(Q)$(call rte_mkdir, $(DESTDIR)$(bindir))
$(Q)tar -cf - -C $O --exclude 'app/*.map' \
+ --exclude app/pmdinfogen \
--exclude 'app/cmdline*' --exclude app/test \
--exclude app/testacl --exclude app/testpipeline app | \
tar -xf - -C $(DESTDIR)$(bindir) --strip-components=1 \
@@ -126,10 +127,8 @@ install-runtime:
$(Q)$(call rte_mkdir, $(DESTDIR)$(sbindir))
$(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk_nic_bind.py,
\
$(DESTDIR)$(sbindir)/dpdk_nic_bind)
- $(Q)$(call rte_symlink,$(DESTDIR)$(bindir)/pmdinfogen, \
- $(DESTDIR)$(bindir)/dpdk_pmdinfogen)
$(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/pmdinfo.py, \
- $(DESTDIR)$(bindir)/dpdk_pmdinfo)
+ $(DESTDIR)$(bindir)/dpdk_pmdinfo)
install-kmod:
ifneq ($(wildcard $O/kmod/*),)
@@ -145,8 +144,9 @@ install-sdk:
$(Q)$(call rte_mkdir,$(DESTDIR)$(sdkdir))
$(Q)cp -a $(RTE_SDK)/mk$(DESTDIR)$(sdkdir)
$(Q)cp -a $(RTE_SDK)/scripts $(DESTDIR)$(sdkdir)
- $(Q)$(call rte_mkdir,$(DESTDIR)$(targetdir))
+ $(Q)$(call rte_mkdir,
$(DESTDIR)$(targetdir)/app)
$(Q)cp -a $O/.config $(DESTDIR)$(targetdir)
+ $(Q)cp -a $O/app/pmdinfogen
$(DESTDIR)$(targetdir)/app
$(Q)$(call rte_symlink, $(DESTDIR)$(includedir),
$(DESTDIR)$(targetdir)/include)
$(Q)$(call rte_symlink, $(DESTDIR)$(libdir),
$(DESTDIR)$(targetdir)/lib)
--
2.7.0
[dpdk-dev] [PATCH v2 02/10] mk: fix build dependency of drivers on pmdinfogen
When compiling the drivers, some code is generated with pmdinfogen.
A fresh parallel build can fail if a driver is compiled before pmdinfogen:
build/buildtools/dpdk-pmdinfogen: Permission denied
There was a dependency declared in drivers/Makefile but it cannot work
because this file is based on mk/rte.subdir.mk which do not handle
dependencies.
It is fixed by declaring the whole buildtools as (order only) prerequisite
of drivers.
Fixes: cb6696d22023 ("drivers: update registration macro usage")
Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
---
drivers/Makefile | 2 --
mk/rte.sdkbuild.mk | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile
index 75a3168..81c03a8 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -34,6 +34,4 @@ include $(RTE_SDK)/mk/rte.vars.mk
DIRS-y += net
DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
-DEPDIRS-y += buildtools/pmdinfo
-
include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index fb68af2..354f006 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -49,6 +49,7 @@ $(1): $(sort $(LOCAL_DEPDIRS-$(1)))
endef
$(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d
+drivers: | buildtools
#
# build and clean targets
--
2.7.0
[dpdk-dev] [PATCH v2 00/10] additions to pmdinfogen
These are some patches that I think needed for RC2. There are other patches needed as discussed on the mailing-list which are not part of this series: - overwritten driver name (reported by Pablo) - pmdinfo.py on FreeBSD (reported by Bruce) Note: RC2 is planned to be out at the end of this week. v2 changes: - use argv[0] in usage message - drop fix of parameters registration which is better managed in Neil's patch "crypto: normalize cryptodev pmd names with macros" http://dpdk.org/ml/archives/dev/2016-July/043506.html Thomas Monjalon (10): drivers: fix build with new register macro mk: fix build dependency of drivers on pmdinfogen mk: remove traces of hostapp build directory mk: fix driver build with installed SDK mk: fix verbose pmdinfogen run pmdinfogen: fix build warnings pmdinfogen: fix usage message eal: move PCI table macro doc: fix syntax in pmdinfogen guide maintainers: add section for pmdinfo MAINTAINERS| 4 ++ buildtools/pmdinfogen/Makefile | 4 +- buildtools/pmdinfogen/pmdinfogen.c | 61 +- doc/guides/freebsd_gsg/build_dpdk.rst | 2 +- doc/guides/linux_gsg/build_dpdk.rst| 2 +- doc/guides/prog_guide/dev_kit_build_system.rst | 5 +-- drivers/Makefile | 2 - drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 2 +- drivers/net/mlx4/mlx4.c| 2 +- drivers/net/mlx5/mlx5.c| 2 +- lib/librte_eal/common/include/rte_dev.h| 6 +-- lib/librte_eal/common/include/rte_pci.h| 5 +++ mk/internal/rte.compile-pre.mk | 26 +-- mk/rte.sdkbuild.mk | 6 +-- mk/rte.sdkconfig.mk| 2 +- mk/rte.sdkinstall.mk | 8 ++-- 16 files changed, 59 insertions(+), 80 deletions(-) -- 2.7.0
[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros
2016-07-07 11:37, Neil Horman: > -PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drvi, aesni_mb); > -DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs= " > +PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drv, CRYPTODEV_NAME_AESNI_MB_PMD); > +DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD, > "max_nb_queue_pairs= " > "max_nb_sessions= socket_id="); A coding style opinion: The line DRIVER_REGISTER_PARAM_STRING is a bit long and I think it would be nice to have only one param per line with an indent like this: DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD, "max_nb_queue_pairs= " "max_nb_sessions= " "socket_id=");
[dpdk-dev] [PATCH 2/2] bnx2x: add support for xstats
On 07/07/2016 23:50, Rasesh Mody wrote: > This patch adds support for extended statistics for BNX2X PMD. > > Signed-off-by: Rasesh Mody Acked-by: Remy Horton
[dpdk-dev] [PATCH 1/2] qede: add support for xstats
On 07/07/2016 23:50, Rasesh Mody wrote: > This patch adds support for extended statistics for QEDE PMD. > > Signed-off-by: Rasesh Mody Acked-by: Remy Horton
[dpdk-dev] [PATCH v3 08/10] eal: remove PCI include from generic driver header
On Fri, Jul 08, 2016 at 04:42:22PM +0200, Thomas Monjalon wrote:
> Remove include of rte_pci.h in the generic header rte_dev.h.
>
> Fixes: cb6696d22023 ("drivers: update registration macro usage")
>
> Suggested-by: David Marchand
> Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
> ---
> lib/librte_eal/common/include/rte_dev.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/include/rte_dev.h
> b/lib/librte_eal/common/include/rte_dev.h
> index e6f0d4c..95789f9 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -48,7 +48,7 @@ extern "C" {
>
> #include
> #include
> -#include
> +
> #include
>
> __attribute__((format(printf, 2, 0)))
> --
> 2.7.0
>
>
[dpdk-dev] [PATCH v3 06/10] pmdinfogen: fix build warnings
On Fri, Jul 08, 2016 at 04:42:20PM +0200, Thomas Monjalon wrote:
> When compiled with a standard clang, pmdinfogen can raise a warning:
> buildtools/pmdinfogen/pmdinfogen.c:365:1: warning:
> control reaches end of non-void function
>
> Actually there can be more warnings with stricter compilers.
> In order to catch them early and fix most of them, the DPDK standard flags
> WERROR_FLAGS are used.
>
> The warnings fixed are:
> no previous prototype for ...
> no return statement in function returning non-void
> variable ?secstrings? set but not used
> ?sec_name? defined but not used
> ?get_symbol_index? defined but not used
> pointer of type ?void *? used in arithmetic
>
> Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
>
> Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
[dpdk-dev] [PATCH v2 1/2] doc: add bifurcated driver guide on ixgbe nic
2016-07-08 08:42, Wu, Jingjing: > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > Thank you for providing more doc. > > I am just not convinced it is the right place > > and it must be discussed. > > Some questions below to help deciding where it must be. > > > > 2016-06-23 11:02, Jingjing Wu: > > > Bifurcated driver is a mechanism which depends the advanced > > > Ethernet device to split traffic between queues. It provides > > > the capability to let the kernel driver and DPDK driver co-exist > > > and take their advantage. > > > > The name "bifurcated driver" was used for a real driver proposal > > in Linux. I don't think it applies here. Where does the "bifurcated" name come from? Could we rename it to "flow bifurcation" or "partial kernel bypass"? > > > It is achieved by using SRIOV and NIC's advanced filtering. This > > > patch describes it and adds the user guide on ixgbe NICs. > > > > It is not a DPDK feature, just a use case. > > What do we learn about DPDK? > > Yes, it is an feature in Linux driver. And the typical use case how DPDK > co-work with Linux networking. > I think it will be a good reference. When I preparde this patch, I'm also > doubt where to put, and the behavior > may be NIC specific, so I added it here. > > Any other suggestion? Bernard is creating an howto section.
[dpdk-dev] [PATCH v3 00/11] Fix build errors related to exported headers
On 7/8/2016 9:05 AM, Adrien Mazarguil wrote:
> On Thu, Jul 07, 2016 at 06:33:17PM +, Wiles, Keith wrote:
>>
>>> On Jul 7, 2016, at 10:49 AM, Adrien Mazarguil >> 6wind.com> wrote:
>>>
>>> DPDK uses GNU C language extensions in most of its code base. This is fine
>>> for internal source files whose compilation flags are controlled by DPDK,
>>> however user applications that use exported "public" headers may experience
>>> compilation failures when enabling strict error/standard checks (-std and
>>> -pedantic for instance).
>>
>> Do you try compiling these changes with CLANG and/or ICC compilers?
>
> clang/clang++ yes, works fine. I did not try with ICC however.
I tested with icc, getting following error [1], compiler warning seems
valid, but didn't investigate what in your patch cause this.
[1]
.../app/test/test_table_acl.c(487): error #2405: array of elements
containing a flexible array member is nonstandard
struct rte_pipeline_table_entry entries[5];
^
.../app/test/test_table_acl.c(492): error #2405: array of elements
containing a flexible array member is nonstandard
struct rte_pipeline_table_entry entries_ptr[5];
>
> Note that considering "({ ... })" is a GNU extension, compilers that do
> support this syntax also support the GNU __extension__ keyword. As a result,
> those that do not support this keyword most likely already cannot compile
> DPDK at all.
>
[dpdk-dev] [PATCH v3 04/10] mk: fix driver build with installed SDK
On Fri, Jul 08, 2016 at 04:42:18PM +0200, Thomas Monjalon wrote:
> The tool pmdinfogen was called from RTE_OUTPUT/app/ which does not exist
> if building a driver outside of the SDK build.
> When building DPDK, RTE_SDK_BIN is RTE_OUTPUT. When building an external
> driver, RTE_SDK_BIN must point to the installed DPDK directory containing
> includes, libs, etc.
>
> That's why pmdinfogen must be installed in the SDK directory and be part
> of the SDK installation.
>
> Fixes: 3d781ca32874 ("mk: do post processing on objects that register a
> driver")
>
> Signed-off-by: Thomas Monjalon
Acked-by: Neil Horman
[dpdk-dev] [PATCH 07/11] pmdinfogen: fix build warnings
On Thu, Jul 07, 2016 at 09:25:27PM +, Mcnamara, John wrote:
>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Neil Horman
> > Sent: Thursday, July 7, 2016 6:55 PM
> > To: Thomas Monjalon
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 07/11] pmdinfogen: fix build warnings
> >
> > On Thu, Jul 07, 2016 at 05:36:26PM +0200, Thomas Monjalon wrote:
> > > When compiled with a standard clang, pmdinfogen can raise a warning:
> > > buildtools/pmdinfogen/pmdinfogen.c:365:1: warning:
> > > control reaches end of non-void function
> > >
> > > Actually there can be more warnings with stricter compilers.
> > > In order to catch them early and fix most of them, the DPDK standard
> > > flags WERROR_FLAGS are used.
> > >
> > > The warnings fixed are:
> > > no previous prototype for ...
> > > no return statement in function returning non-void
> > > variable ?secstrings? set but not used
> > > ?sec_name? defined but not used
> > > ?get_symbol_index? defined but not used
> > > pointer of type ?void *? used in arithmetic
> > >
> > > Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen
> > > utility")
> > >
> > > Signed-off-by: Thomas Monjalon
> > > ---
> > I'm not opposed to any of these changes, but I'm really starting to wonder
> > how well used/maintained clang is as a toolchain target. I assert that
> > because, with my admittedly broken dependency rule, a native clang build
> > for me errors out in any number of places:
> >
> > /home/nhorman/git/dpdk/lib/librte_eal/linuxapp/eal/eal_pci.c:392:37:
> > error:
> > equality comparison with extraneous parentheses [-Werror,-Wparentheses-
> > equality] if (((_device_list)->tqh_first == ((void*)0))) {
> > ~~^
> > /home/nhorman/git/dpdk/lib/librte_eal/linuxapp/eal/eal_pci.c:392:37: note:
> > remove extraneous parentheses around the comparison to silence this
> > warning if (((_device_list)->tqh_first == ((void*)0))) {
>
>
> It is due to the clang/ccache "issue" that is tripping up everyone. Exporting
> CCACHE_CPP2=yes should fix it. There was a thread about this earlier in the
> week.
>
> John.
>
>
Ah, that did it, yes. Though I didnt' get any of the warnings the Thomas
encountered when he posted those changes to pmdinfogen either
Regardless, theres nothing egregious in the chagnes, so I don't see the harm.
Acked-by: Neil Horman
[dpdk-dev] [PATCH 09/11] eal: move PCI table macro
Hello Thomas, Neil, (will be back in a couple of days, thanks Thomas for pointing this thread) On Thu, Jul 7, 2016 at 6:25 PM, Thomas Monjalon wrote: > 2016-07-07 12:11, Neil Horman: >> On Thu, Jul 07, 2016 at 05:36:28PM +0200, Thomas Monjalon wrote: >> > Remove include of rte_pci.h in the generic header rte_dev.h >> > and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h. > [...] >> >> This seems strange to me, in that its odd for the driver information export >> macros to be spread out in multiple locations. Specifically it enjoins the >> use >> of the DRV_EXP_TAG macro, which helps centralize tag naming. Perhaps the >> happy >> medium is to place all the export macros (includnig PMD_REGISTER_DRIVER) into >> its own pmd_register.h header? > > I don't know. > David, your opinion? - The suggestion I did offline to Thomas was to move pci stuff in pci headers. We are trying to move from the "all pci" code in eal to accomodate for other "buses" / architectures. Having a pci macro in a generic header like rte_dev.h is wrong to me. Moving this to a new header like pmd_register.h sounds like a new generic header with pci specific stuff in it. So, I am not sure I follow you Neil. Can you elaborate ? - Why do you want to centralise the tag naming ? To avoid collisions ? Well, adding those tags should not happen that often and I think we can maintain this with careful reviews. -- David Marchand
[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros
On Fri, Jul 08, 2016 at 02:00:20PM +, De Lara Guarch, Pablo wrote:
>
>
> > -Original Message-
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > Sent: Friday, July 08, 2016 1:18 PM
> > To: De Lara Guarch, Pablo
> > Cc: dev at dpdk.org; Richardson, Bruce; Thomas Monjalon; Stephen
> > Hemminger; Panu Matilainen
> > Subject: Re: [PATCH] crypto: normalize cryptodev pmd names with macros
> >
> > On Fri, Jul 08, 2016 at 09:09:10AM +, De Lara Guarch, Pablo wrote:
> > > Hi Neil,
> > >
> > > > -Original Message-
> > > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > > Sent: Thursday, July 07, 2016 4:38 PM
> > > > To: dev at dpdk.org
> > > > Cc: Neil Horman; Richardson, Bruce; Thomas Monjalon; De Lara Guarch,
> > > > Pablo; Stephen Hemminger; Panu Matilainen
> > > > Subject: [PATCH] crypto: normalize cryptodev pmd names with macros
> > > >
> > > > Recently reported, the introduction of pmd information exports led to a
> > > > breakage of cryptodev unit tests because the test infrastructure relies
> > > > on
> > the
> > > > cryptodev names being available in macros. This patch fixes the pmd
> > naming
> > > > to
> > > > use the macro names. Note that the macro names were already pre-
> > > > stringified,
> > > > which won't work as the PMD_REGISTER_DRIVER macro requires the
> > name in
> > > > both a
> > > > processing token and stringified form. As such the names are defined
> > > > now
> > as
> > > > tokens, and converted where needed to stringified form on demand using
> > > > RTE_STR.
> > > >
> > > > Tested using the null and aesni_mb crypto pmds, as I don't have hardware
> > for
> > > > any
> > > > other device. Also not build tested on snow3g or kasumi pmd because
> > > > building
> > > > those requires external libraries that appear to not be open source
> > licensed.
> > > >
> > > > Signed-off-by: Neil Horman
> > > > CC: Bruce Richardson
> > > > CC: Thomas Monjalon
> > > > CC: "De Lara Guarch, Pablo"
> > > > CC: Stephen Hemminger
> > > > CC: Panu Matilainen
> > > > ---
> > > > app/test/test_cryptodev.c | 20
> > > > ++--
> > > > app/test/test_cryptodev_perf.c | 18
> > > > +-
> > > > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 +++
> > > > drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 6 +++---
> > > > drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 7 +++
> > > > drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 2 +-
> > > > drivers/crypto/kasumi/rte_kasumi_pmd.c | 5 ++---
> > > > drivers/crypto/null/null_crypto_pmd.c | 7 +++
> > > > drivers/crypto/null/null_crypto_pmd_private.h | 6 +++---
> > > > drivers/crypto/qat/rte_qat_cryptodev.c | 4 ++--
> > > > drivers/crypto/snow3g/rte_snow3g_pmd.c | 4 ++--
> > > > lib/librte_cryptodev/rte_cryptodev.h | 12 ++--
> > > > 12 files changed, 47 insertions(+), 51 deletions(-)
> > >
> > > Thanks for this patch. I tested snow3g and kasumi, and they don't compile.
> > > I have a fix for that, so I can send a v2 of this patch if it is OK for
> > > you.
> > >
> >
> > I suppose thats fine, sure, though I'm really not comfortable with an open
> > source project requiring what appears to be non-open source components
> > (though I
> > can't really tell what the snow3g or kasumi license is). Regardless, whats
> > the
> > compilation error?
>
> drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function
> 'snow3g_pmd_qp_create_processed_ops_ring':
> drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: error:
> 'cryptodev_snow3g_pmd' undeclared (first use in this function)
>SNOW3G_LOG_ERR("Unable to reuse existing ring %s"
>
> ^
>
> dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: note: each
> undeclared identifier is reported only once for each function it appears in
> dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function
> 'snow3g_pmd_session_configure':
> dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:296:117: error:
> 'cryptodev_snow3g_pmd' undeclared (first use in this function)
>SNOW3G_LOG_ERR("invalid session struct");
>
> ...
>
> The solution is adding RTE_STR in SNOW3G_LOG_ERR.
>
Ah,thanks. Its odd, I used cscope to do a find and replace for all the other
instances of the RTE_STR conversion, I wonder how I missed that.
Neil
> ...
> >
> >
> > > Also, we should make these changes in the other PMDs as well, right?
> > > I mean, avoid setting the name of the driver directly in the structure
> > > and go
> > back to the original name.
> > > I can do that as well, if you want (maybe a separate patch, as this one is
> > only related to crypto).
> > >
> > I think thats kind of two questions:
> >
> > 1)
[dpdk-dev] [PATCH 09/11] eal: move PCI table macro
On Fri, Jul 08, 2016 at 04:03:40PM +0200, Thomas Monjalon wrote: > 2016-07-08 09:56, Neil Horman: > > On Fri, Jul 08, 2016 at 10:49:25AM +0200, David Marchand wrote: > > > Hello Thomas, Neil, > > > > > > (will be back in a couple of days, thanks Thomas for pointing this thread) > > > > > > > > > On Thu, Jul 7, 2016 at 6:25 PM, Thomas Monjalon > > > wrote: > > > > 2016-07-07 12:11, Neil Horman: > > > >> On Thu, Jul 07, 2016 at 05:36:28PM +0200, Thomas Monjalon wrote: > > > >> > Remove include of rte_pci.h in the generic header rte_dev.h > > > >> > and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h. > > > > [...] > > > >> > > > >> This seems strange to me, in that its odd for the driver information > > > >> export > > > >> macros to be spread out in multiple locations. Specifically it > > > >> enjoins the use > > > >> of the DRV_EXP_TAG macro, which helps centralize tag naming. Perhaps > > > >> the happy > > > >> medium is to place all the export macros (includnig > > > >> PMD_REGISTER_DRIVER) into > > > >> its own pmd_register.h header? > > > > > > > > I don't know. > > > > David, your opinion? > > > > > > - The suggestion I did offline to Thomas was to move pci stuff in pci > > > headers. > > > We are trying to move from the "all pci" code in eal to accomodate for > > > other "buses" / architectures. > > I get that, but I'm not sure that applies here. The macro in question is > > specific to pci busses, and if there is additional bus information to > > export, it > > will have its own macro (e.g. DRIVER_REGISTER_USB_TABLE or some such). > > While > > I could see that being an argument for putting each macro in with its own > > bus > > type, I think thats the wrong organization here, in that people writing > > drivers > > will want to know what export macros are available and will expect to look > > in a > > single place for it. > > > > > Having a pci macro in a generic header like rte_dev.h is wrong to me. > > > Moving this to a new header like pmd_register.h sounds like a new > > > generic header with pci specific stuff in it. > > Well, yes, but I see that as no different than rte_ethdev.c or rte_pdump.c. > > both of those files will need to know about all the different types of > > busses > > you support and have to include those corresponding header files (i.e. they > > will > > have to include rte_pci.h, rte_usb.h, rte_i2c.h, etc). This is really no > > different in my mind. > > > > > So, I am not sure I follow you Neil. > > > > > > Can you elaborate ? > > > > > I suppose the best way to describe it is that while I understand and > > support the > > desire to separate and abstract bus information away from device function, I > > think theres a pragmatic descision here to prioritize functional domain over > > header inclusion. That is to say, I think when people are writing a > > driver, it > > will be helpful to have all the export macros in a single location so they > > know > > what information they can export, and that includes registration of various > > bus > > type identifiers. So a file like pmd_registration.h that includes > > rte_pci.h, > > rte_usb.h, rte_i2c.h, etc is more useful to a developer, than spreading > > these > > macros out to those various header files, for the sake of avoiding a > > potentially > > unneeded include. > > > > > > > > - Why do you want to centralise the tag naming ? > > > To avoid collisions ? > > Yes, and to centralize that information. Since the pmdinfogen tool needs to > > know what those tag names are as well, its useful to keep them in the same > > area > > to maintain co-ordination. Its also useful because it means we can use one > > macro to define tag naming convention, instead of having to re-implement or > > dead-reckon it in multiple files. > > > > > Well, adding those tags should not happen that often and I think we > > > can maintain this with careful reviews. > > I don't agree with that. This discussion is based on the fact that you > > expect > > that we will be adding additional bus types in the future correct? Well, > > given > > that we have a pci bus specific export macro, I would expect that to > > proliferate > > to every other bus type as well, and so we can expect to at least have a new > > tag added for every bus that is added, in addition to any other bus agnostic > > information people wish to export (just off hand, looking at the linux > > modinfo > > section, we might expect module author, module version, alias names, > > licensing > > infomration, and others to be potential export candidates). So, depending > > on > > how much this is adopted, I think we can potentially expect a great deal of > > additional tagging to be needed. > > Anyway, this macro do not need rte_pci.h. > So the minimal patch can be to just remove this include. > Oh my gosh! I've been an idiot! you're absolutely right. The macro is just defining a string to point to the pci_table symbol name, its not
[dpdk-dev] [PATCH 05/18] mbuf: add function to get packet type from data
Hi Olivier,
> -Original Message-
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Thursday, July 07, 2016 11:49 PM
> To: Liang, Cunming ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 05/18] mbuf: add function to get packet type
> from data
>
> Hi Cunming,
>
> Thank you for your feedback.
>
> On 07/07/2016 10:19 AM, Liang, Cunming wrote:
> > Hi Olivier,
> >
> >> -Original Message-
> >> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> >> Sent: Wednesday, July 06, 2016 3:43 PM
> >> To: Liang, Cunming ; dev at dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH 05/18] mbuf: add function to get packet type
> >> from data
> >>
> >> Hi Cunming,
> >>
> >> On 07/06/2016 08:44 AM, Liang, Cunming wrote:
> >>> Hi Olivier,
> >>>
> >>> On 7/5/2016 11:41 PM, Olivier Matz wrote:
> Introduce the function rte_pktmbuf_get_ptype() that parses a
> mbuf and returns its packet type. For now, the following packet
> types are parsed:
> L2: Ether
> L3: IPv4, IPv6
> L4: TCP, UDP, SCTP
>
> The goal here is to provide a reference implementation for packet type
> parsing. This function will be used by testpmd in next commits, allowing
> to compare its result with the value given by the hardware.
>
> This function will also be useful when implementing Rx offload support
> in virtio pmd. Indeed, the virtio protocol gives the csum start and
> offset, but it does not give the L4 protocol nor it tells if the
> checksum is relevant for inner or outer. This information has to be
> known to properly set the ol_flags in mbuf.
>
> Signed-off-by: Didier Pallard
> Signed-off-by: Jean Dao
> Signed-off-by: Olivier Matz
> ---
> doc/guides/rel_notes/release_16_11.rst | 5 +
> lib/librte_mbuf/Makefile | 5 +-
> lib/librte_mbuf/rte_mbuf_ptype.c | 234
> +
> lib/librte_mbuf/rte_mbuf_ptype.h | 43 ++
> lib/librte_mbuf/rte_mbuf_version.map | 1 +
> 5 files changed, 286 insertions(+), 2 deletions(-)
> create mode 100644 lib/librte_mbuf/rte_mbuf_ptype.c
>
> [...]
> +
> +/* parse mbuf data to get packet type */
> +uint32_t rte_pktmbuf_get_ptype(const struct rte_mbuf *m,
> +struct rte_mbuf_hdr_lens *hdr_lens)
> +{
> +struct rte_mbuf_hdr_lens local_hdr_lens;
> +const struct ether_hdr *eh;
> +struct ether_hdr eh_copy;
> +uint32_t pkt_type = RTE_PTYPE_L2_ETHER;
> +uint32_t off = 0;
> +uint16_t proto;
> +
> +if (hdr_lens == NULL)
> +hdr_lens = _hdr_lens;
> +
> +eh = rte_pktmbuf_read(m, off, sizeof(*eh), _copy);
> +if (unlikely(eh == NULL))
> +return 0;
> +proto = eh->ether_type;
> +off = sizeof(*eh);
> +hdr_lens->l2_len = off;
> +
> +if (proto == rte_cpu_to_be_16(ETHER_TYPE_IPv4)) {
> +const struct ipv4_hdr *ip4h;
> +struct ipv4_hdr ip4h_copy;
> +
> +ip4h = rte_pktmbuf_read(m, off, sizeof(*ip4h), _copy);
> +if (unlikely(ip4h == NULL))
> +return pkt_type;
> +
> +pkt_type |= ptype_l3_ip(ip4h->version_ihl);
> +hdr_lens->l3_len = ip4_hlen(ip4h);
> +off += hdr_lens->l3_len;
> +if (ip4h->fragment_offset &
> +rte_cpu_to_be_16(IPV4_HDR_OFFSET_MASK |
> +IPV4_HDR_MF_FLAG)) {
> +pkt_type |= RTE_PTYPE_L4_FRAG;
> +hdr_lens->l4_len = 0;
> +return pkt_type;
> +}
> +proto = ip4h->next_proto_id;
> +pkt_type |= ptype_l4(proto);
> +} else if (proto == rte_cpu_to_be_16(ETHER_TYPE_IPv6)) {
> +const struct ipv6_hdr *ip6h;
> +struct ipv6_hdr ip6h_copy;
> +int frag = 0;
> +
> +ip6h = rte_pktmbuf_read(m, off, sizeof(*ip6h), _copy);
> +if (unlikely(ip6h == NULL))
> +return pkt_type;
> +
> +proto = ip6h->proto;
> +hdr_lens->l3_len = sizeof(*ip6h);
> +off += hdr_lens->l3_len;
> +pkt_type |= ptype_l3_ip6(proto);
> +if ((pkt_type & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_IPV6_EXT) {
> +proto = skip_ip6_ext(proto, m, , );
> +hdr_lens->l3_len = off - hdr_lens->l2_len;
> +}
> +if (proto == 0)
> +return pkt_type;
> +if (frag) {
> +pkt_type |= RTE_PTYPE_L4_FRAG;
> +hdr_lens->l4_len = 0;
> +return pkt_type;
> +}
> +pkt_type |= ptype_l4(proto);
> +}
> +
> +if ((pkt_type & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP) {
> +
[dpdk-dev] [PATCH v3 00/11] Fix build errors related to exported headers
On Thu, Jul 07, 2016 at 06:33:17PM +, Wiles, Keith wrote:
>
> > On Jul 7, 2016, at 10:49 AM, Adrien Mazarguil > 6wind.com> wrote:
> >
> > DPDK uses GNU C language extensions in most of its code base. This is fine
> > for internal source files whose compilation flags are controlled by DPDK,
> > however user applications that use exported "public" headers may experience
> > compilation failures when enabling strict error/standard checks (-std and
> > -pedantic for instance).
>
> Do you try compiling these changes with CLANG and/or ICC compilers?
clang/clang++ yes, works fine. I did not try with ICC however.
Note that considering "({ ... })" is a GNU extension, compilers that do
support this syntax also support the GNU __extension__ keyword. As a result,
those that do not support this keyword most likely already cannot compile
DPDK at all.
--
Adrien Mazarguil
6WIND
[dpdk-dev] [PATCH 09/11] eal: move PCI table macro
On Fri, Jul 08, 2016 at 10:49:25AM +0200, David Marchand wrote: > Hello Thomas, Neil, > > (will be back in a couple of days, thanks Thomas for pointing this thread) > > > On Thu, Jul 7, 2016 at 6:25 PM, Thomas Monjalon > wrote: > > 2016-07-07 12:11, Neil Horman: > >> On Thu, Jul 07, 2016 at 05:36:28PM +0200, Thomas Monjalon wrote: > >> > Remove include of rte_pci.h in the generic header rte_dev.h > >> > and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h. > > [...] > >> > >> This seems strange to me, in that its odd for the driver information export > >> macros to be spread out in multiple locations. Specifically it enjoins > >> the use > >> of the DRV_EXP_TAG macro, which helps centralize tag naming. Perhaps the > >> happy > >> medium is to place all the export macros (includnig PMD_REGISTER_DRIVER) > >> into > >> its own pmd_register.h header? > > > > I don't know. > > David, your opinion? > > - The suggestion I did offline to Thomas was to move pci stuff in pci headers. > We are trying to move from the "all pci" code in eal to accomodate for > other "buses" / architectures. I get that, but I'm not sure that applies here. The macro in question is specific to pci busses, and if there is additional bus information to export, it will have its own macro (e.g. DRIVER_REGISTER_USB_TABLE or some such). While I could see that being an argument for putting each macro in with its own bus type, I think thats the wrong organization here, in that people writing drivers will want to know what export macros are available and will expect to look in a single place for it. > Having a pci macro in a generic header like rte_dev.h is wrong to me. > Moving this to a new header like pmd_register.h sounds like a new > generic header with pci specific stuff in it. Well, yes, but I see that as no different than rte_ethdev.c or rte_pdump.c. both of those files will need to know about all the different types of busses you support and have to include those corresponding header files (i.e. they will have to include rte_pci.h, rte_usb.h, rte_i2c.h, etc). This is really no different in my mind. > So, I am not sure I follow you Neil. > > Can you elaborate ? > I suppose the best way to describe it is that while I understand and support the desire to separate and abstract bus information away from device function, I think theres a pragmatic descision here to prioritize functional domain over header inclusion. That is to say, I think when people are writing a driver, it will be helpful to have all the export macros in a single location so they know what information they can export, and that includes registration of various bus type identifiers. So a file like pmd_registration.h that includes rte_pci.h, rte_usb.h, rte_i2c.h, etc is more useful to a developer, than spreading these macros out to those various header files, for the sake of avoiding a potentially unneeded include. > > - Why do you want to centralise the tag naming ? > To avoid collisions ? Yes, and to centralize that information. Since the pmdinfogen tool needs to know what those tag names are as well, its useful to keep them in the same area to maintain co-ordination. Its also useful because it means we can use one macro to define tag naming convention, instead of having to re-implement or dead-reckon it in multiple files. > Well, adding those tags should not happen that often and I think we > can maintain this with careful reviews. I don't agree with that. This discussion is based on the fact that you expect that we will be adding additional bus types in the future correct? Well, given that we have a pci bus specific export macro, I would expect that to proliferate to every other bus type as well, and so we can expect to at least have a new tag added for every bus that is added, in addition to any other bus agnostic information people wish to export (just off hand, looking at the linux modinfo section, we might expect module author, module version, alias names, licensing infomration, and others to be potential export candidates). So, depending on how much this is adopted, I think we can potentially expect a great deal of additional tagging to be needed. Neil
