* [PATCH v3 5/6] net/virtio: fix multiple process support
From: Yuanhan Liu @ 2017-01-06 10:16 UTC (permalink / raw)
To: dev; +Cc: Yuanhan Liu, stable, Juho Snellman, Yaron Illouz
In-Reply-To: <1483697780-12088-1-git-send-email-yuanhan.liu@linux.intel.com>
The introduce of virtio 1.0 support brings yet another set of ops, badly,
it's not handled correctly, that it breaks the multiple process support.
The issue is the data/function pointer may vary from different processes,
and the old used to do one time set (for primary process only). That
said, the function pointer the secondary process saw is actually from the
primary process space. Accessing it could likely result to a crash.
Kudos to the last patches, we now be able to maintain those info that may
vary among different process locally, meaning every process could have its
own copy for each of them, with the correct value set. And this is what
this patch does:
- remap the PCI (IO port for legacy device and memory map for modern
device)
- set vtpci_ops correctly
After that, multiple process would work like a charm. (At least, it
passed my fuzzy test)
Fixes: b8f04520ad71 ("virtio: use PCI ioport API")
Fixes: d5bbeefca826 ("virtio: introduce PCI implementation structure")
Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0")
Cc: stable@dpdk.org
Cc: Juho Snellman <jsnell@iki.fi>
Cc: Yaron Illouz <yaroni@radcom.com>
Reported-by: Juho Snellman <jsnell@iki.fi>
Reported-by: Yaron Illouz <yaroni@radcom.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
v3: - update release note and nic matrix
v2: - fixed PCI remap, so that virtio 1.0 also works
---
doc/guides/nics/features/virtio.ini | 1 +
doc/guides/rel_notes/release_17_02.rst | 5 ++++
drivers/net/virtio/virtio_ethdev.c | 53 +++++++++++++++++++++++++++++++--
drivers/net/virtio/virtio_pci.c | 4 +--
drivers/net/virtio/virtio_pci.h | 4 +++
drivers/net/virtio/virtio_user_ethdev.c | 2 +-
6 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 41830c1..f5de291 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -22,3 +22,4 @@ ARMv8 = Y
x86-32 = Y
x86-64 = Y
Usage doc = Y
+Multiprocess aware = Y
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 3b65038..a4260de 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -54,6 +54,11 @@ Resolved Issues
Also, make sure to start the actual text at the margin.
=========================================================
+ * **fixed virtio multiple process support.**
+
+ Fixed few regressions introduced in recent releases that break the virtio
+ multiple process support.
+
EAL
~~~
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 5567aa2..19d4348 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1289,6 +1289,49 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
}
/*
+ * Remap the PCI device again (IO port map for legacy device and
+ * memory map for modern device), so that the secondary process
+ * could have the PCI initiated correctly.
+ */
+static int
+virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+{
+ if (hw->modern) {
+ /*
+ * We don't have to re-parse the PCI config space, since
+ * rte_eal_pci_map_device() makes sure the mapped address
+ * in secondary process would equal to the one mapped in
+ * the primary process: error will be returned if that
+ * requirement is not met.
+ *
+ * That said, we could simply reuse all cap pointers
+ * (such as dev_cfg, common_cfg, etc.) parsed from the
+ * primary process, which is stored in shared memory.
+ */
+ if (rte_eal_pci_map_device(pci_dev)) {
+ PMD_INIT_LOG(DEBUG, "failed to map pci device!");
+ return -1;
+ }
+ } else {
+ if (rte_eal_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+static void
+virtio_set_vtpci_ops(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+{
+ if (pci_dev == NULL)
+ VTPCI_OPS(hw) = &virtio_user_ops;
+ else if (hw->modern)
+ VTPCI_OPS(hw) = &modern_ops;
+ else
+ VTPCI_OPS(hw) = &legacy_ops;
+}
+
+/*
* This function is based on probe() function in virtio_pci.c
* It returns 0 on success.
*/
@@ -1296,7 +1339,7 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
{
struct virtio_hw *hw = eth_dev->data->dev_private;
- struct rte_pci_device *pci_dev;
+ struct rte_pci_device *pci_dev = eth_dev->pci_dev;
uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
int ret;
@@ -1306,6 +1349,13 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ if (pci_dev) {
+ ret = virtio_remap_pci(pci_dev, hw);
+ if (ret)
+ return ret;
+ }
+
+ virtio_set_vtpci_ops(pci_dev, hw);
if (hw->use_simple_rxtx) {
eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;
eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
@@ -1325,7 +1375,6 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
}
hw->port_id = eth_dev->data->port_id;
- pci_dev = eth_dev->pci_dev;
if (pci_dev) {
ret = vtpci_init(pci_dev, hw, &dev_flags);
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index d1e9c05..f5754e5 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -303,7 +303,7 @@
return 0;
}
-static const struct virtio_pci_ops legacy_ops = {
+const struct virtio_pci_ops legacy_ops = {
.read_dev_cfg = legacy_read_dev_config,
.write_dev_cfg = legacy_write_dev_config,
.reset = legacy_reset,
@@ -519,7 +519,7 @@
io_write16(1, vq->notify_addr);
}
-static const struct virtio_pci_ops modern_ops = {
+const struct virtio_pci_ops modern_ops = {
.read_dev_cfg = modern_read_dev_config,
.write_dev_cfg = modern_write_dev_config,
.reset = modern_reset,
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 6b9aecf..511a1c8 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -333,4 +333,8 @@ int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
uint16_t vtpci_irq_config(struct virtio_hw *, uint16_t);
+extern const struct virtio_pci_ops legacy_ops;
+extern const struct virtio_pci_ops modern_ops;
+extern const struct virtio_pci_ops virtio_user_ops;
+
#endif /* _VIRTIO_PCI_H_ */
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 7d2a9d9..3563952 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -212,7 +212,7 @@
strerror(errno));
}
-static const struct virtio_pci_ops virtio_user_ops = {
+const struct virtio_pci_ops virtio_user_ops = {
.read_dev_cfg = virtio_user_read_dev_config,
.write_dev_cfg = virtio_user_write_dev_config,
.reset = virtio_user_reset,
--
1.9.0
^ permalink raw reply related
* Re: [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes for GCM
From: De Lara Guarch, Pablo @ 2017-01-06 10:17 UTC (permalink / raw)
To: Azarewicz, PiotrX T, Kusztal, ArkadiuszX, dev@dpdk.org
Cc: Trahe, Fiona, Griffin, John, Jain, Deepak K, Doherty, Declan,
Kusztal, ArkadiuszX
In-Reply-To: <4837007523CC9A4B9414D20C13DE6E6413758F5E@IRSMSX102.ger.corp.intel.com>
> -----Original Message-----
> From: Azarewicz, PiotrX T
> Sent: Monday, January 02, 2017 8:51 AM
> To: Azarewicz, PiotrX T; Kusztal, ArkadiuszX; dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John; Jain, Deepak K;
> Doherty, Declan; Kusztal, ArkadiuszX
> Subject: RE: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding
> bytes for GCM
>
> Hi Arek,
>
> > > Subject: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding
> > > bytes for GCM
> > >
> > > This commit fixes pre-counter block (J0) padding by clearing four most
> > > significant bytes before setting initial counter value.
> > >
> > > Fixes: b2bb3597470c ("crypto/aesni_gcm: move pre-counter block to
> > > driver")
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
This should be CC'd to the stable subtree. I will do it for you this time.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes for GCM
From: De Lara Guarch, Pablo @ 2017-01-06 10:27 UTC (permalink / raw)
To: Kusztal, ArkadiuszX, dev@dpdk.org, stable@dpdk.org
Cc: Trahe, Fiona, Griffin, John, Jain, Deepak K, Doherty, Declan
In-Reply-To: <1482481493-4369-2-git-send-email-arkadiuszx.kusztal@intel.com>
CC'ing stable mailing list.
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, December 23, 2016 8:25 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John; Jain, Deepak K;
> Doherty, Declan; Kusztal, ArkadiuszX
> Subject: [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes for GCM
>
> This commit fixes pre-counter block (J0) padding by clearing
> four most significant bytes before setting initial counter value.
>
> Fixes: b2bb3597470c ("crypto/aesni_gcm: move pre-counter block to
> driver")
>
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
> drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> index dba5e15..af3d60f 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> @@ -40,6 +40,7 @@
> #include <rte_vdev.h>
> #include <rte_malloc.h>
> #include <rte_cpuflags.h>
> +#include <rte_byteorder.h>
>
> #include "aesni_gcm_pmd_private.h"
>
> @@ -241,7 +242,8 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> struct rte_crypto_sym_op *op,
> * to set BE LSB to 1, driver expects that 16B is allocated
> */
> if (op->cipher.iv.length == 12) {
> - op->cipher.iv.data[15] = 1;
> + uint32_t *iv_padd = (uint32_t *)&op->cipher.iv.data[12];
> + *iv_padd = rte_bswap32(1);
> }
>
> if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
> --
> 2.1.0
^ permalink raw reply
* Re: [PATCH v5 01/12] eal/bus: introduce bus abstraction
From: Shreyansh Jain @ 2017-01-06 10:31 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <12579803.ZHBMo4GdSA@xps13>
On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote:
> 2016-12-26 18:53, Shreyansh Jain:
>> +DPDK_17.02 {
>> + global:
>> +
>> + rte_bus_list;
>> + rte_eal_bus_add_device;
>> + rte_eal_bus_add_driver;
>> + rte_eal_bus_get;
>> + rte_eal_bus_dump;
>> + rte_eal_bus_register;
>> + rte_eal_bus_insert_device;
>> + rte_eal_bus_remove_device;
>> + rte_eal_bus_remove_driver;
>> + rte_eal_bus_unregister;
>
> I think the prefix can be just rte_bus_ instead of rte_eal_bus_.
Ok. I will change these.
I thought as these symbols are part of librte_eal/*, naming should
reflect that.
>
>> +/** Double linked list of buses */
>> +TAILQ_HEAD(rte_bus_list, rte_bus);
>> +
>> +/* Global Bus list */
>> +extern struct rte_bus_list rte_bus_list;
>
> Why the bus list is public?
I will revisit this. When I made it public, intention was that some
element external to EAL (e.g. drivers/bus) might require to iterate
over this list. But, as of now I don't think any such user/case exists
(except test*).
>
>> +/**
>> + * A structure describing a generic bus.
>> + */
>> +struct rte_bus {
>> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
>> + struct rte_driver_list driver_list;
>> + /**< List of all drivers on bus */
>> + struct rte_device_list device_list;
>> + /**< List of all devices on bus */
>> + const char *name; /**< Name of the bus */
>> +};
>
> I am not convinced we should link a generic bus to drivers and devices.
> What do you think of having rte_pci_bus being a rte_bus and linking
> with rte_pci_device and rte_pci_driver lists?
This is different from what I had in mind.
You are saying:
Class: rte_bus
`-> No object instantiated for this class
Class: rte_pci_bus inheriting rte_bus
`-> object instantiated for this class.
Here, rte_bus is being treated as an abstract class which is only
inherited and rte_pci_bus is the base class which is instantiated.
And I was thinking:
Class: rte_bus
`-> Object: pci_bus (defined in */eal/eal_pci.c)
Here, rte_bus is that base class which is instantiated.
I agree that what you are suggesting is inline with current model:
rte_device -> abstract class (no one instantiates it)
`-> rte_pci_device -> Base class which inherits rte_device and
is instantiated.
When I choose not to create rte_pci_bus, it was because I didn't want
another indirection in form of rte_bus->rte_pci_bus->object.
There were no 'non-generic' bus functions which were only applicable for
rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance
of rte_bus.
>
> I'm thinking to something like that:
>
> struct rte_bus {
> TAILQ_ENTRY(rte_bus) next;
> const char *name;
> rte_bus_scan_t scan;
> rte_bus_match_t match;
> };
> struct rte_pci_bus {
> struct rte_bus bus;
> struct rte_pci_driver_list pci_drivers;
> struct rte_pci_device_list pci_devices;
> };
if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is
fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI)
because I don't see any 'non-generic' information in rte_pci_bus which
can't be put in rte_bus.
>
>> +/** Helper for Bus registration. The constructor has higher priority than
>> + * PMD constructors
>> + */
>> +#define RTE_REGISTER_BUS(nm, bus) \
>> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
>> +{\
>> + (bus).name = RTE_STR(nm);\
>> + rte_eal_bus_register(&bus); \
>> +}
>
> By removing the lists from rte_bus as suggested above, do you still need
> a priority for this constructor?
I think yes.
Even if we have rte_pci_bus as a class, object of rte_bus should be part
of Bus list *before* registration of a driver (because, driver
registration searches for bus by name).
(This is assuming that no global PCI/VDEV/XXX bus object is created
which is directly used within all PCI specific bus operations).
There was another suggestion on list which was to check for existence of
bus _within_ each driver registration and create/instantiate an object
in case no bus is registered. I didn't like the approach so I didn't use
it. From David [1], and me [2]
[1] http://dpdk.org/ml/archives/dev/2016-December/051689.html
[2] http://dpdk.org/ml/archives/dev/2016-December/051698.html
>
>> struct rte_device {
>> TAILQ_ENTRY(rte_device) next; /**< Next device */
>> + struct rte_bus *bus; /**< Device connected to this bus */
>> const struct rte_driver *driver;/**< Associated driver */
>> int numa_node; /**< NUMA node connection */
>> struct rte_devargs *devargs; /**< Device user arguments */
>> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>> */
>> struct rte_driver {
>> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
>> + struct rte_bus *bus; /**< Bus serviced by this driver */
>> const char *name; /**< Driver name. */
>> const char *alias; /**< Driver alias. */
>> };
>
> Do we need to know the bus associated to a driver in rte_driver?
> Bus and driver are already associated in rte_device.
>
>
Two reasons:
1/ A driver should be associated with a bus so that if required, all bus
can be directly extracted - even when probing has not been done.
2/ device->driver would only be updated after probe. device->driver->bus
would not be valid in such cases, if required.
Overall, I don't have objections for rte_bus->rte_pci_bus=>object as
compared to rte_bus=>PCI-object. But, I would still like to get a final
confirmation of a more preferred way.
Meanwhile, I will make changes to accommodate this change to save time
in case rte_pci_bus class is final/preferred method.
-
Shreyansh
^ permalink raw reply
* Re: [PATCH v2 3/3] crypto/qat: fix iv size in PMD capabilities
From: De Lara Guarch, Pablo @ 2017-01-06 10:31 UTC (permalink / raw)
To: Kusztal, ArkadiuszX, dev@dpdk.org
Cc: Trahe, Fiona, Griffin, John, Jain, Deepak K, Doherty, Declan,
stable@dpdk.org
In-Reply-To: <1482481493-4369-4-git-send-email-arkadiuszx.kusztal@intel.com>
CC'ing stable mailing list.
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, December 23, 2016 8:25 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John; Jain, Deepak K;
> Doherty, Declan; Kusztal, ArkadiuszX
> Subject: [PATCH v2 3/3] crypto/qat: fix iv size in PMD capabilities
>
> This patch sets iv size in qat PMD to 12 bytes to be
> conformant with nist SP800-38D.
>
> Fixes: 26c2e4ad5ad4 ("cryptodev: add capabilities discovery")
>
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
> drivers/crypto/qat/qat_crypto.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/qat/qat_crypto.c
> b/drivers/crypto/qat/qat_crypto.c
> index fa78c60..0b714ad 100644
> --- a/drivers/crypto/qat/qat_crypto.c
> +++ b/drivers/crypto/qat/qat_crypto.c
> @@ -303,8 +303,8 @@ static const struct rte_cryptodev_capabilities
> qat_pmd_capabilities[] = {
> .increment = 8
> },
> .iv_size = {
> - .min = 16,
> - .max = 16,
> + .min = 12,
> + .max = 12,
> .increment = 0
> }
> }, }
> --
> 2.1.0
^ permalink raw reply
* Re: [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD capabilities
From: De Lara Guarch, Pablo @ 2017-01-06 10:31 UTC (permalink / raw)
To: Kusztal, ArkadiuszX, dev@dpdk.org
Cc: Trahe, Fiona, Griffin, John, Jain, Deepak K, Doherty, Declan,
stable@dpdk.org
In-Reply-To: <1482481493-4369-3-git-send-email-arkadiuszx.kusztal@intel.com>
CC'ing stable mailing list.
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, December 23, 2016 8:25 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John; Jain, Deepak K;
> Doherty, Declan; Kusztal, ArkadiuszX
> Subject: [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD capabilities
>
> This patch sets iv size in aesni gcm PMD to 12 bytes to be
> conformant with nist SP800-38D.
>
> Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto
> operations")
>
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
> drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> index e824d4b..c51f82a 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> @@ -77,8 +77,8 @@ static const struct rte_cryptodev_capabilities
> aesni_gcm_pmd_capabilities[] = {
> .increment = 0
> },
> .iv_size = {
> - .min = 16,
> - .max = 16,
> + .min = 12,
> + .max = 12,
> .increment = 0
> }
> }, }
> --
> 2.1.0
^ permalink raw reply
* Re: [PATCH v5 04/12] eal: integrate bus scan and probe with EAL
From: Shreyansh Jain @ 2017-01-06 10:38 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <1763533.RfUee4nzA4@xps13>
On Wednesday 04 January 2017 03:16 AM, Thomas Monjalon wrote:
> 2016-12-26 18:53, Shreyansh Jain:
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -844,6 +845,9 @@ rte_eal_init(int argc, char **argv)
>> if (rte_eal_intr_init() < 0)
>> rte_panic("Cannot init interrupt-handling thread\n");
>>
>> + if (rte_eal_bus_scan())
>> + rte_panic("Cannot scan the buses for devices\n");
>
> Yes, definitely. Just one scan functions which scan registered bus.
>
>> @@ -884,6 +888,9 @@ rte_eal_init(int argc, char **argv)
>> if (rte_eal_pci_probe())
>> rte_panic("Cannot probe PCI\n");
>>
>> + if (rte_eal_bus_probe())
>> + rte_panic("Cannot probe devices\n");
>> +
>> if (rte_eal_dev_init() < 0)
>> rte_panic("Cannot init pmd devices\n");
>
> What is the benefit of initializing (probe) a device outside of the scan?
> Currently, it is done in two steps, so you are keeping the same behaviour.
Yes, only for compatibility to existing model of two-step process.
Ideally, only a single step is enough (init->probe).
During the discussion in [1] also this point was raised - at that time
for VDEV and applicability to PCI.
[1] http://dpdk.org/ml/archives/dev/2016-December/051306.html
If you want, I can merge these two. I postponed it because 1) it is an
independent change and should really impact bus and 2) I was not sure
of dependency of init *before* pthread_create for all workers.
>
> I imagine a model where the scan function decide to initialize the
> device and can require some help from a callback to make this decision.
> So the whitelist/blacklist policy can be implemented with callbacks at
> the scan level and possibly the responsibility of the application.
> Note that the callback model would be a change for a next release.
>
Agree. But, that is not really part of Bus patches - isn't it? Or, you
want to change that with this series?
^ permalink raw reply
* Re: [PATCH v2 0/3] Fix iv sizes in crypto drivers capabilities
From: De Lara Guarch, Pablo @ 2017-01-06 10:35 UTC (permalink / raw)
To: Kusztal, ArkadiuszX, dev@dpdk.org
Cc: Trahe, Fiona, Griffin, John, Jain, Deepak K, Doherty, Declan
In-Reply-To: <1482481493-4369-1-git-send-email-arkadiuszx.kusztal@intel.com>
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, December 23, 2016 8:25 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona; De Lara Guarch, Pablo; Griffin, John; Jain, Deepak K;
> Doherty, Declan; Kusztal, ArkadiuszX
> Subject: [PATCH v2 0/3] Fix iv sizes in crypto drivers capabilities
>
> This patchset fixes iv (initialization vector) size values in qat
> and aesni gcm pmds to be conformant with nist SP800-38D.
>
> v2:
> - added missing signed-off-by line
>
> Arek Kusztal (3):
> crypto/aesni_gcm: fix J0 padding bytes for GCM
> crypto/aesni_gcm: fix iv size in PMD capabilities
> crypto/qat: fix iv size in PMD capabilities
>
> drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 +++-
> drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 4 ++--
> drivers/crypto/qat/qat_crypto.c | 4 ++--
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
> --
> 2.1.0
Applied to dpdk-next-crypto.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH] pmdinfogen: Resolve coverity scan forward null issue
From: Thomas Monjalon @ 2017-01-06 10:41 UTC (permalink / raw)
To: Neil Horman; +Cc: dev, Neil Horman, john.mcnamara
In-Reply-To: <20170105192241.29213-1-nhorman@tuxdriver.com>
2017-01-05 14:22, Neil Horman:
> From: Neil Horman <nhorman@redhat.com>
>
> Coverity issue 139593 reports a forward null dereference from a for loop
> that works with a variable previously tested for null that had no error
> handling or condition to prevent it. Pretty obvious fix below.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Coverity issue: 139593
Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
Applied, thanks
^ permalink raw reply
* Re: [PATCH v7 14/27] net/i40e: set VF VLAN insertion from PF
From: Iremonger, Bernard @ 2017-01-06 11:28 UTC (permalink / raw)
To: Lu, Wenzhuo, Wu, Jingjing, dev@dpdk.org
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B55669B@shsmsx102.ccr.corp.intel.com>
Hi Jinqjing, Wenzhuo,
> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Friday, January 6, 2017 8:20 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v7 14/27] net/i40e: set VF VLAN insertion
> from PF
>
> Hi Jingjing, Bernard,
>
>
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Friday, January 6, 2017 8:33 AM
> > To: Lu, Wenzhuo; dev@dpdk.org
> > Cc: Iremonger, Bernard
> > Subject: RE: [dpdk-dev] [PATCH v7 14/27] net/i40e: set VF VLAN
> > insertion from PF
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> > > Sent: Tuesday, January 3, 2017 2:55 PM
> > > To: dev@dpdk.org
> > > Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> > > Subject: [dpdk-dev] [PATCH v7 14/27] net/i40e: set VF VLAN insertion
> > > from PF
> > >
> > > From: Bernard Iremonger <bernard.iremonger@intel.com>
> > >
> > > Support inserting VF VLAN id from PF.
> > > User can call the API on PF to insert a VLAN id to a specific VF.
> > >
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > > drivers/net/i40e/i40e_ethdev.c | 56
> > > +++++++++++++++++++++++++++++++
> > > drivers/net/i40e/rte_pmd_i40e.h | 19 +++++++++++
> > > drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> > > 3 files changed, 76 insertions(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 7ab1c93..31c387d 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -10266,3 +10266,59 @@ static void
> > > i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> > > else
> > > return -EINVAL;
> > > }
> > > +
> > > +int rte_pmd_i40e_set_vf_vlan_insert(uint8_t port, uint16_t vf_id,
> > > + uint16_t vlan_id)
> > > +{
> > > + struct rte_eth_dev *dev;
> > > + struct rte_eth_dev_info dev_info;
> > > + struct i40e_pf *pf;
> > > + struct i40e_hw *hw;
> > > + struct i40e_vsi *vsi;
> > > + struct i40e_vsi_context ctxt;
> > > + int ret;
> > > +
> > > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > > +
> > > + dev = &rte_eth_devices[port];
> > > + rte_eth_dev_info_get(port, &dev_info);
> >
> > It looks dev_info is not used in this function.
> I'll delete it.
It was intended to use dev_info have a check on max_vfs.
if (vf_id >= dev_info.max_vfs)
return -EINVAL;
Should this check be added instead of deleting dev_info ?
> >
> > > + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > > + hw = I40E_PF_TO_HW(pf);
> > > +
> > > + /**
> > > + * return -ENODEV if SRIOV not enabled, VF number not configured
> > > + * or no queue assigned.
> > > + */
> > > + if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 ||
> > > + pf->vf_nb_qps == 0)
> > > + return -ENODEV;
> > > +
> > > + if (vf_id >= pf->vf_num || !pf->vfs)
> > > + return -EINVAL;
> > > +
> > > + if (vlan_id > ETHER_MAX_VLAN_ID)
> > > + return -EINVAL;
> > > +
> > > + vsi = pf->vfs[vf_id].vsi;
> > > + if (!vsi)
> > > + return -EINVAL;
> > > +
> > > + vsi->info.valid_sections =
> > > cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
> > > + vsi->info.pvid = vlan_id;
> > > + if (vlan_id > 0)
> > > + vsi->info.port_vlan_flags |=
> > I40E_AQ_VSI_PVLAN_INSERT_PVID;
> > > + else
> > > + vsi->info.port_vlan_flags &=
> > > ~I40E_AQ_VSI_PVLAN_INSERT_PVID;
> > So, Pvid is used here for insert. Does it has any relationship with
> > vlan anti- spoof patch?
> > If so, it's better to consider how to deal with that.
> It's vlan insertion not filtering. So I think not related.
>
> >
> > Thanks
> > Jingjing
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v7 17/27] net/i40e: set VF VLAN filter from PF
From: Iremonger, Bernard @ 2017-01-06 11:38 UTC (permalink / raw)
To: Wu, Jingjing, Lu, Wenzhuo, dev@dpdk.org
In-Reply-To: <9BB6961774997848B5B42BEC655768F810CC3D0F@SHSMSX103.ccr.corp.intel.com>
Hi Jinqjing,
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, January 6, 2017 12:37 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v7 17/27] net/i40e: set VF VLAN filter from
> PF
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> > Sent: Tuesday, January 3, 2017 2:55 PM
> > To: dev@dpdk.org
> > Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> > Subject: [dpdk-dev] [PATCH v7 17/27] net/i40e: set VF VLAN filter from
> > PF
> >
> > From: Bernard Iremonger <bernard.iremonger@intel.com>
> >
> > add rte_pmd_i40e_set_vf_vlan_filter API.
> > User can call the API on PF to enable/disable a set of VF's VLAN filters.
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 52
> > +++++++++++++++++++++++++++++++
> > drivers/net/i40e/rte_pmd_i40e.h | 22 +++++++++++++
> > drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 4d2fb20..47e03d6 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -10428,3 +10428,55 @@ int rte_pmd_i40e_set_vf_vlan_tag(uint8_t
> > port, uint16_t vf_id, uint8_t on)
> >
> > return ret;
> > }
> > +
> > +int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
> > + uint64_t vf_mask, uint8_t on) {
> > + struct rte_eth_dev *dev;
> > + struct rte_eth_dev_info dev_info;
> > + struct i40e_pf *pf;
> > + uint16_t pool_idx;
> > + int ret = I40E_SUCCESS;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port];
> > + rte_eth_dev_info_get(port, &dev_info);
> > +
> > + if (vlan_id > ETHER_MAX_VLAN_ID)
> > + return -EINVAL;
> > +
> > + if (on > 1)
> > + return -EINVAL;
> > +
> > + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > + if ((pf->flags & I40E_FLAG_VMDQ) == 0) {
> > + PMD_INIT_LOG(ERR, "Firmware doesn't support VMDQ");
> > + return -ENOTSUP;
> > + }
> > +
> > + if (!pf->vmdq) {
> > + PMD_INIT_LOG(INFO, "VMDQ not configured");
> > + return -ENOTSUP;
> > + }
> > +
> > + for (pool_idx = 0;
> > + pool_idx < ETH_64_POOLS &&
> > + pool_idx < pf->nb_cfg_vmdq_vsi &&
> > + ret == I40E_SUCCESS;
> > + pool_idx++) {
> > + if (vf_mask & ((uint64_t)(1ULL << pool_idx))) {
> > + if (on)
> > + ret = i40e_vsi_add_vlan(pf-
> >vmdq[pool_idx].vsi,
> > + vlan_id);
> > + else
> > + ret = i40e_vsi_delete_vlan(
> > + pf->vmdq[pool_idx].vsi, vlan_id);
> > + }
> > + }
>
> The head is saying "set VF VLAN filter", why do you deal with VMDQ VSIs?
This API is modelled on the rte_pmd_ixgbe_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id, uint64_t vf_mask, uint8_t on) which seems to use VMDq. More investigation is needed here.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v5 05/12] eal: add probe and remove support for rte_driver
From: Shreyansh Jain @ 2017-01-06 11:44 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <5088350.GnaeIEgOPh@xps13>
On Wednesday 04 January 2017 03:35 AM, Thomas Monjalon wrote:
> 2016-12-26 18:53, Shreyansh Jain:
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -152,6 +162,8 @@ struct rte_driver {
>> struct rte_bus *bus; /**< Bus serviced by this driver */
>> const char *name; /**< Driver name. */
>> const char *alias; /**< Driver alias. */
>> + driver_probe_t *probe; /**< Probe the device */
>> + driver_remove_t *remove; /**< Remove/hotplugging the device */
>> };
>
> If I understand well, this probe function does neither scan nor match.
> So it could be named init.
Current model is:
After scanning for devices and populating bus->device_list,
Bus probe does:
`-> bus->match()
`-> rte_driver->probe() for matched driver
For PCI drivers, '.probe = rte_eal_pci_probe'.
For example, igb_ethdev.c:
--->8---
static struct eth_driver rte_igb_pmd = {
.pci_drv = {
.driver = {
.probe = rte_eal_pci_probe,
.remove = rte_eal_pci_remove,
},
...
--->8---
>
> I think the probe (init) and remove ops must be specific to the bus.
> We can have them in rte_bus, and as an example, the pci implementation
> would call the pci probe and remove ops of rte_pci_driver.
So,
---
After scanning for devices (bus->scan()):
Bus probe (rte_eal_bus_probe()):
`-> bus->match()
`-> bus->init() - a new fn rte_bus_pci_init()
-> which calls rte_eal_pci_probe()
-> and rte_pci_driver->probe()
and remove rte_driver probe and remove callbacks because they are now
redundant. (they were added in bus patches itself)
---
Is the above correct understanding of your statement?
Somehow I don't remember why I didn't do this in first place - it seems
to be better option than introducing a rte_driver->probe()/remove()
layer. I will change it (and think again why I rejected this idea in
first place). Thanks.
>
> Please use rte_ prefix in public headers.
>
I am assuming you are referring to driver_probe_t/driver_remove_t.
^ permalink raw reply
* Re: [PATCH v7 00/17] net/i40e: consistent filter API
From: Ferruh Yigit @ 2017-01-06 11:54 UTC (permalink / raw)
To: Beilei Xing, jingjing.wu, helin.zhang; +Cc: dev
In-Reply-To: <1483680439-82227-1-git-send-email-beilei.xing@intel.com>
On 1/6/2017 5:27 AM, Beilei Xing wrote:
> The patch set depends on Adrien's Generic flow API(rte_flow).
>
> The patches mainly finish following functions:
> 1) Store and restore all kinds of filters.
> 2) Parse all kinds of filters.
> 3) Add flow validate function.
> 4) Add flow create function.
> 5) Add flow destroy function.
> 6) Add flow flush function.
>
> v7 changes:
> Separate filter related code from eth_i40e_dev_init().
> Change struct i40e_flow to ret_flow.
>
> v6 changes:
> Change functions' name to be more readable.
> Add comments for parse_pattern functions to list supported rules.
> Add comments for parse_action functions to list supported actions.
> Add ETHTYPE check when parsing ethertype pattern.
>
> v5 changes:
> Change some local variable name.
> Add removing i40e_flow_list during device unint.
> Fix compile error when gcc compile option isn't '-O0'.
>
> v4 changes:
> Change I40E_TCI_MASK with 0xFFFF to align with testpmd.
> Modidy the stats show when restoring filters.
>
> v3 changes:
> Set the related cause pointer to a non-NULL value when error happens.
> Change return value when error happens.
> Modify filter_del parameter with key.
> Malloc filter after checking when delete a filter.
> Delete meaningless initialization.
> Add return value when there's error.
> Change global variable definition.
> Modify some function declaration.
>
> v2 changes:
> Add i40e_flow.c, all flow ops are implemented in the file.
> Change the whole implementation of all parse flow functions.
> Update error info for all flow ops.
> Add flow_list to store flows created.
>
> Beilei Xing (17):
> net/i40e: store ethertype filter
> net/i40e: store tunnel filter
> net/i40e: store flow director filter
> net/i40e: restore ethertype filter
> net/i40e: restore tunnel filter
> net/i40e: restore flow director filter
> net/i40e: add flow validate function
> net/i40e: parse flow director filter
> net/i40e: parse tunnel filter
> net/i40e: add flow create function
> net/i40e: add flow destroy function
> net/i40e: destroy ethertype filter
> net/i40e: destroy tunnel filter
> net/i40e: destroy flow directory filter
> net/i40e: add flow flush function
> net/i40e: flush ethertype filters
> net/i40e: flush tunnel filters
>
> drivers/net/i40e/Makefile | 2 +
> drivers/net/i40e/i40e_ethdev.c | 594 +++++++++++--
> drivers/net/i40e/i40e_ethdev.h | 178 ++++
> drivers/net/i40e/i40e_fdir.c | 140 ++-
> drivers/net/i40e/i40e_flow.c | 1849 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 2692 insertions(+), 71 deletions(-)
> create mode 100644 drivers/net/i40e/i40e_flow.c
>
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>
>
Series applied to dpdk-next-net/master, thanks.
(extern moved into header file)
^ permalink raw reply
* Re: [PATCH v5 04/12] eal: integrate bus scan and probe with EAL
From: Shreyansh Jain @ 2017-01-06 12:00 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <654c9419-247a-90b5-11d0-a8e3551ce582@nxp.com>
On Friday 06 January 2017 04:08 PM, Shreyansh Jain wrote:
> On Wednesday 04 January 2017 03:16 AM, Thomas Monjalon wrote:
>> 2016-12-26 18:53, Shreyansh Jain:
>>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>>> @@ -844,6 +845,9 @@ rte_eal_init(int argc, char **argv)
>>> if (rte_eal_intr_init() < 0)
>>> rte_panic("Cannot init interrupt-handling thread\n");
>>>
>>> + if (rte_eal_bus_scan())
>>> + rte_panic("Cannot scan the buses for devices\n");
>>
>> Yes, definitely. Just one scan functions which scan registered bus.
>>
>>> @@ -884,6 +888,9 @@ rte_eal_init(int argc, char **argv)
>>> if (rte_eal_pci_probe())
>>> rte_panic("Cannot probe PCI\n");
>>>
>>> + if (rte_eal_bus_probe())
>>> + rte_panic("Cannot probe devices\n");
>>> +
>>> if (rte_eal_dev_init() < 0)
>>> rte_panic("Cannot init pmd devices\n");
>>
>> What is the benefit of initializing (probe) a device outside of the scan?
>> Currently, it is done in two steps, so you are keeping the same
>> behaviour.
>
> Yes, only for compatibility to existing model of two-step process.
> Ideally, only a single step is enough (init->probe).
>
> During the discussion in [1] also this point was raised - at that time
> for VDEV and applicability to PCI.
>
> [1] http://dpdk.org/ml/archives/dev/2016-December/051306.html
>
> If you want, I can merge these two. I postponed it because 1) it is an
> independent change and should really impact bus and 2) I was not sure
> of dependency of init *before* pthread_create for all workers.
I forgot _not_ in above - rephrasing:
If you want, I can merge these two. I postponed it because 1) it is an
independent change and should _not_ really impact bus and 2) I was not
sure of dependency of init *before* pthread_create for all workers.
>
>>
>> I imagine a model where the scan function decide to initialize the
>> device and can require some help from a callback to make this decision.
>> So the whitelist/blacklist policy can be implemented with callbacks at
>> the scan level and possibly the responsibility of the application.
>> Note that the callback model would be a change for a next release.
>>
>
> Agree. But, that is not really part of Bus patches - isn't it? Or, you
> want to change that with this series?
>
>
>
^ permalink raw reply
* Re: [PATCH v7 26/27] net/i40e: fix segmentation fault in close
From: Iremonger, Bernard @ 2017-01-06 12:00 UTC (permalink / raw)
To: Wu, Jingjing, Lu, Wenzhuo, dev@dpdk.org; +Cc: stable@dpdk.org
In-Reply-To: <9BB6961774997848B5B42BEC655768F810CC3E79@SHSMSX103.ccr.corp.intel.com>
Hi Jingjing,
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, January 6, 2017 1:29 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v7 26/27] net/i40e: fix segmentation fault in
> close
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> > Sent: Tuesday, January 3, 2017 2:55 PM
> > To: dev@dpdk.org
> > Cc: Iremonger, Bernard <bernard.iremonger@intel.com>;
> stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v7 26/27] net/i40e: fix segmentation fault
> > in close
> >
> > From: Bernard Iremonger <bernard.iremonger@intel.com>
> >
> > The vsi's have already been released, so the second call to
> > i40e_vsi_release results in a segmentation fault.
> > The second call to i40e_vsi_release has been removed.
> Where is the first call to release vmdq vsi?
All of the VSI's are released in the call to i40e_vsi_release(pf->main_vsi) at line 1895.
This function is recursive and release all the VSI's.
There is still a VSI address in pf->vmdq[i].vsi but calling
i40e_vsi_release(pf->vmdq[i].vsi);
Results in a segmentation fault.
>
> Thanks
> Jingjing
> >
> > Fixes: 3cb446b4aeb2 ("i40e: free vmdq vsi when closing")
> >
> > CC: stable@dpdk.org
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index be45cfa..0b7c366 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -1882,7 +1882,6 @@ static inline void i40e_GLQF_reg_init(struct
> > i40e_hw
> > *hw)
> > i40e_vsi_release(pf->main_vsi);
> >
> > for (i = 0; i < pf->nb_cfg_vmdq_vsi; i++) {
> > - i40e_vsi_release(pf->vmdq[i].vsi);
> > pf->vmdq[i].vsi = NULL;
> > }
> >
> > @@ -4137,6 +4136,9 @@ enum i40e_status_code
> > if (!vsi)
> > return I40E_SUCCESS;
> >
> > + if (!vsi->adapter)
> > + return I40E_ERR_BAD_PTR;
> > +
> > user_param = vsi->user_param;
> >
> > pf = I40E_VSI_TO_PF(vsi);
> > --
> > 1.9.3
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v5 08/12] eal/pci: generalize args of PCI scan/match towards RTE device/driver
From: Shreyansh Jain @ 2017-01-06 12:03 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: david.marchand, dev
In-Reply-To: <32257536.1rHviD2EUb@xps13>
On Wednesday 04 January 2017 03:43 AM, Thomas Monjalon wrote:
> 2016-12-26 18:54, Shreyansh Jain:
>> PCI scan and match now work on rte_device/rte_driver rather than PCI
>> specific objects. These functions can now be plugged to the generic
>> bus callbacks for scanning and matching devices/drivers.
>
> These sentences looks weird :)
> PCI functions must work with PCI objects, it's simpler.
> However I agree to register PCI scan, match, init and remove functions
> with the generic rte_bus. Then the rte_device object is casted into
> rte_pci_device inside these functions.
>
Ok. I will rephrase.
^ permalink raw reply
* [PATCH] examples/ip_pipeline: check vlan and mpls params
From: Jyoti, Anand B @ 2017-01-06 12:05 UTC (permalink / raw)
To: dev@dpdk.org; +Cc: Jyoti, Anand B, Dumitrescu, Cristian
The attached patch checks VLAN IDs and MPLS label for max value and also checks the max number of supported MPLS labels to avoid array overflow in the CLI command line parameters.
Regards,
Anand B Jyoti
^ permalink raw reply
* Re: [PATCH v7 10/27] net/i40e: set VF MAC from PF support
From: Ferruh Yigit @ 2017-01-06 12:07 UTC (permalink / raw)
To: Wu, Jingjing, Lu, Wenzhuo, dev@dpdk.org
In-Reply-To: <9BB6961774997848B5B42BEC655768F810CC3CAA@SHSMSX103.ccr.corp.intel.com>
On 1/6/2017 12:32 AM, Wu, Jingjing wrote:
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
>> Sent: Tuesday, January 3, 2017 2:55 PM
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: [dpdk-dev] [PATCH v7 10/27] net/i40e: set VF MAC from PF support
>>
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Support setting VF MAC address from PF.
>> User can call the API on PF to set a specific VF's MAC address.
>>
>> This will remove all existing MAC filters.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> drivers/net/i40e/i40e_ethdev.c | 42
>> +++++++++++++++++++++++++++++++
>> drivers/net/i40e/rte_pmd_i40e.h | 19 ++++++++++++++
>> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
>> 3 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index 9d050c8..758b574 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -10198,3 +10198,45 @@ static void i40e_set_default_mac_addr(struct
>> rte_eth_dev *dev,
>>
>> return ret;
>> }
>> +
>> +int
>> +rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
>> + struct ether_addr *mac_addr)
>> +{
>> + struct rte_eth_dev_info dev_info;
>> + struct i40e_mac_filter *f;
>> + struct rte_eth_dev *dev;
>> + struct i40e_pf_vf *vf;
>> + struct i40e_vsi *vsi;
>> + struct i40e_pf *pf;
>> + void *temp;
>> +
>> + if (i40e_validate_mac_addr((u8 *)mac_addr) != I40E_SUCCESS)
>> + return -EINVAL;
>> +
>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
>> +
>> + dev = &rte_eth_devices[port];
>> + rte_eth_dev_info_get(port, &dev_info);
>> +
>> + if (vf_id >= dev_info.max_vfs)
>> + return -EINVAL;
>> +
>> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>> +
>> + if (vf_id > pf->vf_num - 1 || !pf->vfs)
>> + return -EINVAL;
>> +
>> + vf = &pf->vfs[vf_id];
>> + vsi = vf->vsi;
>> + if (!vsi)
>> + return -EINVAL;
>> +
>> + ether_addr_copy(mac_addr, &vf->mac_addr);
>
> Only store the mac address in vf struct?
> Are you supposing the API is called before VF is initialized?
Yes.
PF should set the VF MAC before VF initialized.
If PF sets the VF MAC after VF already initialized, new MAC address
won't be effective until next VF initialization.
> If so, it's better to comment it.
Good idea, I will.
>
> Thanks
> Jingjing
>
^ permalink raw reply
* Re: [PATCH v2 2/3] lib/librte_cryptodev: functions for new performance test application
From: De Lara Guarch, Pablo @ 2017-01-06 12:21 UTC (permalink / raw)
To: Mrozowicz, SlawomirX, dev@dpdk.org
Cc: Mrozowicz, SlawomirX, Doherty, Declan, Kerlin, Marcin
In-Reply-To: <1483635001-15473-3-git-send-email-slawomirx.mrozowicz@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Slawomir
> Mrozowicz
> Sent: Thursday, January 05, 2017 4:50 PM
> To: dev@dpdk.org
> Cc: Mrozowicz, SlawomirX; Doherty, Declan; Kerlin, Marcin
> Subject: [dpdk-dev] [PATCH v2 2/3] lib/librte_cryptodev: functions for new
> performance test application
>
> This patch adds helper functions for new performance application.
> Application can be used to measute throughput and latency of
> cryptography operation performed by crypto device.
>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> Signed-off-by: Marcin Kerlin <marcinx.kerlin@intel.com>
Hi Slawomir,
You should change the title of this patch to something like: "cryptodev: add functions for...".
Also, maybe it would be better to say "add functions to retrieve device info",
since you are not saying much with the current title.
I have seen also that this is based on the SGL code, so this patch didn't apply cleanly.
Next time, specify it in the email the dependencies of the patch.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH v3 1/6] ethdev: fix port data mismatched in multiple process model
From: Thomas Monjalon @ 2017-01-06 13:12 UTC (permalink / raw)
To: Yuanhan Liu, Bruce Richardson; +Cc: dev, Ferruh Yigit
In-Reply-To: <1483697780-12088-2-git-send-email-yuanhan.liu@linux.intel.com>
2017-01-06 18:16, Yuanhan Liu:
> +static void
> +eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name)
> +{
> + eth_dev->data = &rte_eth_dev_data[port_id];
> + eth_dev->attached = DEV_ATTACHED;
> + eth_dev_last_created_port = port_id;
> + nb_ports++;
> +
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> + snprintf(eth_dev->data->name, sizeof(eth_dev->data->name),
> + "%s", name);
> + eth_dev->data->port_id = port_id;
> + }
Why not keeping eth_dev->data filling in rte_eth_dev_allocate?
> +}
> +
> struct rte_eth_dev *
> rte_eth_dev_allocate(const char *name)
> {
> @@ -211,12 +226,41 @@ struct rte_eth_dev *
> }
>
> eth_dev = &rte_eth_devices[port_id];
Why not moving this line in eth_dev_init?
> - eth_dev->data = &rte_eth_dev_data[port_id];
> - snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
> - eth_dev->data->port_id = port_id;
> - eth_dev->attached = DEV_ATTACHED;
> - eth_dev_last_created_port = port_id;
> - nb_ports++;
> + eth_dev_init(eth_dev, port_id, name);
> +
> + return eth_dev;
> +}
[...]
> +/*
> + * Attach to a port already registered by the primary process, which
> + * makes sure that the same device would have the same port id both
> + * in the primary and secondary process.
> + */
> +static struct rte_eth_dev *
> +eth_dev_attach_secondary(const char *name)
OK, good description
[...]
> - eth_dev = rte_eth_dev_allocate(ethdev_name);
> - if (eth_dev == NULL)
> - return -ENOMEM;
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> + eth_dev = rte_eth_dev_allocate(ethdev_name);
> + if (eth_dev == NULL)
> + return -ENOMEM;
You could merge here the rest of primary init below.
> + } else {
> + eth_dev = eth_dev_attach_secondary(ethdev_name);
> + if (eth_dev == NULL) {
> + /*
> + * if we failed to attach a device, it means
> + * the device is skipped, due to some errors.
> + * Take virtio-net device as example, it could
> + * due to the device is managed by virtio-net
> + * kernel driver. For such case, we return a
> + * positive value, to let EAL skip it as well.
> + */
I'm not sure we need an example here.
Is the virtio case special?
nit: "it could due" looks to be a typo
> + return 1;
> + }
> + }
>
> if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
^ permalink raw reply
* Re: [PATCH v2 1/5] net/qede: fix scatter-gather issue
From: Ferruh Yigit @ 2017-01-06 13:31 UTC (permalink / raw)
To: Rasesh Mody; +Cc: Harish Patil, dev, stable, Dept-EngDPDKDev
In-Reply-To: <1483690609-15125-1-git-send-email-rasesh.mody@cavium.com>
On 1/6/2017 8:16 AM, Rasesh Mody wrote:
> From: Harish Patil <harish.patil@qlogic.com>
>
> - Make qede_process_sg_pkts() inline and add unlikely check
> - Fix mbuf segment chaining logic in qede_process_sg_pkts()
> - Change qede_encode_sg_bd() to return total segments required
> - Fix first TX buffer descriptor's length
> - Replace repeatitive code using a macro
>
> Fixes: bec0228816c0 ("net/qede: support scatter gather")
>
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
Series applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH v3] net/mlx5: add support for ConnectX-5 NICs
From: Ferruh Yigit @ 2017-01-06 13:32 UTC (permalink / raw)
To: Adrien Mazarguil, Yongseok Koh; +Cc: dev
In-Reply-To: <20170106085042.GU12822@6wind.com>
On 1/6/2017 8:50 AM, Adrien Mazarguil wrote:
> On Thu, Jan 05, 2017 at 04:49:31PM -0800, Yongseok Koh wrote:
>> Add PCI device ID for ConnectX-5 and enable multi-packet send for PF and VF
>> along with changing documentation and release note.
>>
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH v5 04/12] eal: integrate bus scan and probe with EAL
From: Thomas Monjalon @ 2017-01-06 13:46 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: david.marchand, dev
In-Reply-To: <12807c5e-24b4-796b-5c1e-8c08090f4d06@nxp.com>
2017-01-06 17:30, Shreyansh Jain:
> On Friday 06 January 2017 04:08 PM, Shreyansh Jain wrote:
> > On Wednesday 04 January 2017 03:16 AM, Thomas Monjalon wrote:
> >> 2016-12-26 18:53, Shreyansh Jain:
> >>> --- a/lib/librte_eal/linuxapp/eal/eal.c
> >>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> >>> @@ -844,6 +845,9 @@ rte_eal_init(int argc, char **argv)
> >>> if (rte_eal_intr_init() < 0)
> >>> rte_panic("Cannot init interrupt-handling thread\n");
> >>>
> >>> + if (rte_eal_bus_scan())
> >>> + rte_panic("Cannot scan the buses for devices\n");
> >>
> >> Yes, definitely. Just one scan functions which scan registered bus.
> >>
> >>> @@ -884,6 +888,9 @@ rte_eal_init(int argc, char **argv)
> >>> if (rte_eal_pci_probe())
> >>> rte_panic("Cannot probe PCI\n");
> >>>
> >>> + if (rte_eal_bus_probe())
> >>> + rte_panic("Cannot probe devices\n");
> >>> +
> >>> if (rte_eal_dev_init() < 0)
> >>> rte_panic("Cannot init pmd devices\n");
> >>
> >> What is the benefit of initializing (probe) a device outside of the scan?
> >> Currently, it is done in two steps, so you are keeping the same
> >> behaviour.
> >
> > Yes, only for compatibility to existing model of two-step process.
> > Ideally, only a single step is enough (init->probe).
> >
> > During the discussion in [1] also this point was raised - at that time
> > for VDEV and applicability to PCI.
> >
> > [1] http://dpdk.org/ml/archives/dev/2016-December/051306.html
> >
> > If you want, I can merge these two. I postponed it because 1) it is an
> > independent change and should really impact bus and 2) I was not sure
> > of dependency of init *before* pthread_create for all workers.
>
> I forgot _not_ in above - rephrasing:
>
> If you want, I can merge these two. I postponed it because 1) it is an
> independent change and should _not_ really impact bus and 2) I was not
> sure of dependency of init *before* pthread_create for all workers.
I'm OK with your approach.
> >> I imagine a model where the scan function decide to initialize the
> >> device and can require some help from a callback to make this decision.
> >> So the whitelist/blacklist policy can be implemented with callbacks at
> >> the scan level and possibly the responsibility of the application.
> >> Note that the callback model would be a change for a next release.
> >
> > Agree. But, that is not really part of Bus patches - isn't it? Or, you
> > want to change that with this series?
No it is not the scope of this series.
Please could you add it in the cover letter as a next step?
Thanks
^ permalink raw reply
* Re: [PATCH v5 2/6] net/mlx5: support basic flow items and actions
From: Ferruh Yigit @ 2017-01-06 13:52 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Nelio Laranjeiro, dev
In-Reply-To: <20170104184222.GI12822@6wind.com>
On 1/4/2017 6:42 PM, Adrien Mazarguil wrote:
> Hi Ferruh,
>
> On Wed, Jan 04, 2017 at 05:49:46PM +0000, Ferruh Yigit wrote:
>> Hi Nelio,
>>
>> A quick question.
>
> I'll reply since it's related to the API.
>
>> On 12/29/2016 3:15 PM, Nelio Laranjeiro wrote:
>>> Introduce initial software for rte_flow rules.
>>>
>>> VLAN, VXLAN are still not supported.
>>>
>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>>> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>>
>> <...>
>>
>>> +static int
>>> +priv_flow_validate(struct priv *priv,
>>> + const struct rte_flow_attr *attr,
>>> + const struct rte_flow_item items[],
>>> + const struct rte_flow_action actions[],
>>> + struct rte_flow_error *error,
>>> + struct mlx5_flow *flow)
>>> +{
>>> + const struct mlx5_flow_items *cur_item = mlx5_flow_items;
>>
>> <...>
>>
>>> + for (; items->type != RTE_FLOW_ITEM_TYPE_END; ++items) {
>> <...>
>>> + }
>>> + for (; actions->type != RTE_FLOW_ACTION_TYPE_END; ++actions) {
>> <...>
>>> + }
>>
>> Is it guarantied in somewhere that items or actions terminated with
>> TYPE_END?
>
> Yes, since it's now the only way to terminate items/actions lists [1][2].
> There used to be a "max" value in the original draft but it seemed redundant
> and proved annoying to use, and was therefore dropped.
>
> END items/actions behave like a NUL terminator for C strings. They are
> likewise defined with value 0 for convenience.
At least it is good idea to set END values to 0, but still if user not
set it, most probably this will crash the app.
Although most probably this kind of error will be detected easily in
development phase, still it would be nice to return an error instead of
crashing when user provide wrong input.
>
>> And these fields are direct inputs from user.
>> Is there a way to verify user provided values are with TYPE_END terminated?
>
> No, applications must check for its presence (they normally add it
> themselves) before feeding these lists to PMDs. I think that's safe enough.
>
> Note the testpmd flow command does not allow entering a flow rule without
> "end" tokens in both lists, there is no way around this restriction.
>
> [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#matching-pattern
> [2] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#actions
>
^ permalink raw reply
* Re: [PATCH RFC 0/2] Allow vectorized Rx with 4096 desc ring size on Intel NICs.
From: Ferruh Yigit @ 2017-01-06 13:56 UTC (permalink / raw)
To: Wu, Jingjing, Thomas Monjalon, Ilya Maximets
Cc: dev@dpdk.org, Zhang, Helin, Ananyev, Konstantin, Heetae Ahn,
Richardson, Bruce, Lu, Wenzhuo
In-Reply-To: <9BB6961774997848B5B42BEC655768F810CC3F98@SHSMSX103.ccr.corp.intel.com>
On 1/6/2017 2:29 AM, Wu, Jingjing wrote:
>
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, January 4, 2017 1:25 AM
>> To: Thomas Monjalon <thomas.monjalon@6wind.com>; Ilya Maximets
>> <i.maximets@samsung.com>
>> Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
>> Heetae Ahn <heetae82.ahn@samsung.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH RFC 0/2] Allow vectorized Rx with 4096 desc ring
>> size on Intel NICs.
>>
>> On 1/2/2017 3:40 PM, Thomas Monjalon wrote:
>>> 2016-12-27 08:03, Ilya Maximets:
>>>> Hello.
>>>> Ferruh, Thomas, is there a chance for this to be accepted to 17.02?
>>>> Maybe I should resend this patch-set without 'RFC' tag?
>>>
>>> Yes it should be integrated in 17.02.
>>> Ferruh, any news?
>>>
>>
>> I was waiting for a review from driver maintainers
>
> The patch looks good me, both ixgbe and i40e.
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Series applied to dpdk-next-net/master, thanks.
>
> Thanks, Ilya.
>
> Another idea is like, now the memory zone can be freed now.
> So maybe later, we don't need to reserve a maximum size for rx ring, and can change it when number of desc are changed.
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox