[dpdk-dev] Solarflare PMD submission question

2016-11-18 Thread Andrew Rybchenko
On 10/28/2016 05:43 PM, Andrew Rybchenko wrote:
> On 10/28/2016 03:33 PM, Thomas Monjalon wrote:
>> 2016-10-28 13:50, Andrew Rybchenko:
>>> The only thing which comes to my mind is to split libefx import on 
>>> subsystem
>>> basis (few files per subsystem). It is artificial and added files will
>>> be abandoned
>>> until the patch which adds them into build. It could be something like:
>>>1. External interfaces definition
>>>2. Internal interfaces definition
>>>3. Registers definition (hardware interface)
>>>4. Management CPU interface definition (it is one file, but still 
>>> big
>>> 650K)
>>>5. Management CPU interface implementation
>>> and so on for NIC global controls, interrupts, event queue, transmit,
>>> receive,
>>>filtering etc.
>> Yes it is artificial.
>> The most valuable would be a transversal logical split, kind of feature
>> per feature, in order to explain how the device works.
>
> I'm not the main author of the libefx and personally would consider it 
> very useful.
> From the other hand I understand that it is a huge amount of work to 
> make it.
>
>> Such commit is also the opportunity to explain acronyms and so on.
>
> Good. We'll go this way and 'll do my best to make it useful to 
> understand
> overall structure of the code and how the device works.

Now we have a split of the base driver import in big feature steps. The 
base driver is split into 28 patches. Just only 1 patch exceeds 300K 
boundary (which add MCDI definitions header).

Before submitting 56 patches I'd like to double-check that checkpatch.pl 
errors (for example, because of assignments in the 'if' condition, 
parenthesis around return value) is not a show-stopper for base driver 
import.

Andrew.




[dpdk-dev] [RFC PATCH 6/6] eal: removing eth_driver

2016-11-18 Thread Shreyansh Jain
sorry for delay in responding; somehow I didn't notice this email.

On Thursday 17 November 2016 06:23 PM, Jan Blunck wrote:
> On Thu, Nov 17, 2016 at 6:30 AM, Shreyansh Jain  
> wrote:
>> This patch demonstrates how eth_driver can be replaced with appropriate
>> changes for rte_xxx_driver from the PMD itself. It uses ixgbe_ethernet as
>> an example.
>>
>> A large set of changes exists in the rte_ethdev.c - primarily because too
>> much PCI centric code (names, assumption of rte_pci_device) still exists
>> in it. Most, except symbol naming, has been changed in this patch.
>>
>> This proposes that:
>>  - PMD would declare the rte_xxx_driver. In case of ixgbe, it would be
>>rte_pci_driver.
>>  - Probe and remove continue to exists in rte_pci_driver. But, the
>>rte_driver has new hooks for init and uninit. The rationale is that
>>once a ethernet or cryto device is created, the rte_driver->init would
>>be responsible for initializing the device.
>>-- Eth_dev -> rte_driver -> rte_pci_driver
>>   |`-> probe/remove
>>   `--> init/uninit
>
> Hmm, from my perspective this moves struct rte_driver a step closer to
> struct rte_eth_dev instead of decoupling them. It is up to the
> rte_driver->probe if it wants to allocate a struct rte_eth_dev,
> rte_crypto_dev or the famous rte_foo_dev.

That 'closeness' was my intention - to make rte_eth_dev an 
implementation of rte_device type.

rte_eth_dev == rte_cryptodev == rte_anyother_functional_device
- for the above context. All would include rte_device.

As for rte_driver->probe(), it still comes in the rte_driver->init()'s 
role to initialize the 'generic' functional device associated with the 
driver. And, allowing bus specific driver (like PCI) for its individual 
initialization using rte_xxx_driver->probe.

>
> Instead of explicitly modelling rte_eth_dev specifics like init, unit
> or dev_private_size I think we should delegate this to the
> rte_driver->probe instead. Most of what is in rte_eth_dev_pci_probe()
> today is anyway a rte_eth_dev_allocate_priv() anyway. I already have
> some patches in this area in my patch stack.

Can be done - either way rte_pci_driver->probe() ends up calling 
driver->init() (or erstwhile eth_driver->eth_dev_init()).

But, I still think it is better to keep them separate.
A PCI device is type of rte_device, physically.
A ethernet device is type of rte_device, logically.
They both should exist independently. It will help in splitting the 
functionality from physical layout in future - if need be.

>
>
>>  - necessary changes in the rte_eth_dev have also been done so that it
>>refers to the rte_device and rte_driver rather than rte_xxx_*. This
>>would imply, ethernet device is 'linked' to a rte_device/rte_driver
>>which in turn is a rte_xxx_device/rte_xxx_driver type.
>>- for all operations related to extraction relvant xxx type,
>>  container_of would have to be used.
>>
>> Signed-off-by: Shreyansh Jain 
>> ---
>>  drivers/net/ixgbe/ixgbe_ethdev.c | 49 
>> +---
>>  lib/librte_ether/rte_ethdev.c| 36 +
>>  lib/librte_ether/rte_ethdev.h|  6 ++---
>>  3 files changed, 51 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c 
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index edc9b22..acead31 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -1419,7 +1419,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
>> return 0;
>> }
>>
>> -   pci_dev = eth_dev->pci_dev;
>> +   pci_dev = container_of(eth_dev->device, struct rte_pci_device, 
>> device);
>>
>> rte_eth_copy_pci_info(eth_dev, pci_dev);
>>
>> @@ -1532,7 +1532,9 @@ static int
>>  eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>>  {
>> struct ixgbe_hw *hw;
>> -   struct rte_pci_device *pci_dev = eth_dev->pci_dev;
>> +   struct rte_pci_device *pci_dev;
>> +
>> +   pci_dev = container_of(eth_dev->device, struct rte_pci_device, 
>> device);
>>
>> PMD_INIT_FUNC_TRACE();
>>
>> @@ -1562,32 +1564,33 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>> return 0;
>>  }
>>
>> -static struct eth_driver rte_ixgbe_pmd = {
>> -   .pci_drv = {
>> -   .id_table = pci_id_ixgbe_map,
>> -   .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC 
>> |
>> -   RTE_PCI_DRV_DETACHABLE,
>> -   .probe = rte_eth_dev_pci_probe,
>> -   .remove = rte_eth_dev_pci_remove,
>> +static struct rte_pci_driver rte_ixgbe_pci_driver = {
>> +   .id_table = pci_id_ixgbe_map,
>> +   .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
>> +RTE_PCI_DRV_DETACHABLE,
>> +   .probe = rte_eth_dev_pci_probe,
>> +   .remove = rte_eth_dev_pci_remove,
>> +   .driver = {
>> +   

[dpdk-dev] [PATCH] crypto: remove unused digest-appended feature

2016-11-18 Thread John Griffin
On 17/11/16 17:33, Fiona Trahe wrote:
> The cryptodev API had specified that if the digest address field was
> left empty on an authentication operation, then the PMD would assume
> the digest was appended to the source or destination data.
> This case was not handled at all by most PMDs and incorrectly handled
> by the QAT PMD.
> As no bugs were raised, it is assumed to be not needed, so this patch
> removes it, rather than add handling for the case on all PMDs.
> The digest can still be appended to the data, but its
> address must now be provided in the op.
>
> Signed-off-by: Fiona Trahe 
> ---
>   drivers/crypto/qat/qat_adf/qat_algs_build_desc.c |  2 ++
>   drivers/crypto/qat/qat_crypto.c  | 18 +-
>   lib/librte_cryptodev/rte_crypto_sym.h| 10 +-
>   3 files changed, 4 insertions(+), 26 deletions(-)
>

Acked-by: John Griffin 



[dpdk-dev] [PATCH] examples/ethtool: fix bug in drvinfo callback

2016-11-18 Thread Qiming Yang
Function pcmd_drvinfo_callback uses struct info to get
the ethtool infomation of each port. Struct info will
store the infomation of previous port until this
infomation be updated. This patch fixes this issue.

Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample 
application")

Signed-off-by: Qiming Yang 
---
 examples/ethtool/ethtool-app/ethapp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/ethtool/ethtool-app/ethapp.c 
b/examples/ethtool/ethtool-app/ethapp.c
index 9b77385..192d941 100644
--- a/examples/ethtool/ethtool-app/ethapp.c
+++ b/examples/ethtool/ethtool-app/ethapp.c
@@ -177,6 +177,7 @@ pcmd_drvinfo_callback(__rte_unused void *ptr_params,
int id_port;

for (id_port = 0; id_port < rte_eth_dev_count(); id_port++) {
+   memset(, 0, sizeof(info));
if (rte_ethtool_get_drvinfo(id_port, )) {
printf("Error getting info for port %i\n", id_port);
return;
-- 
2.7.4



[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info

2016-11-18 Thread Thomas Monjalon
2016-11-18 16:06, Alejandro Lucero:
> Previous reported speed was hardcoded.
> 
> Signed-off-by: Alejandro Lucero 
> ---
>  drivers/net/nfp/nfp_net.c  | 28 ++--
>  drivers/net/nfp/nfp_net_ctrl.h | 13 +
>  2 files changed, 39 insertions(+), 2 deletions(-)

You should update the doc in the same patch:
doc/guides/nics/features/nfp.ini
It will be the first feature as the file appears to be empty.
So you will need another patch to fill other existing features.

I have an unrelated question: why nfp is disabled in the default build?



[dpdk-dev] [PATCH v1] maintainers: update lthreads maintainer

2016-11-18 Thread John McNamara
Signed-off-by: John McNamara 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6bb8f8..2744212 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -677,7 +677,7 @@ F: examples/netmap_compat/
 F: doc/guides/sample_app_ug/netmap_compatibility.rst

 L-threads - EXPERIMENTAL
-M: Ian Betts 
+M: John McNamara 
 F: examples/performance-thread/
 F: doc/guides/sample_app_ug/performance_thread.rst

-- 
2.7.4



[dpdk-dev] [PATCH v1] maintainers: update procinfo maintainer

2016-11-18 Thread John McNamara
Update procinfo maintainer and name of the application.

Signed-off-by: John McNamara 
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6bb8f8..c40e9e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -618,9 +618,9 @@ M: Pablo de Lara 
 F: app/test-pmd/
 F: doc/guides/testpmd_app_ug/

-Dump tool
+Procinfo tool
 M: Maryam Tahhan 
-M: John McNamara 
+M: Reshma Pattan 
 F: app/proc_info/
 F: doc/guides/tools/proc_info.rst

-- 
2.7.4



[dpdk-dev] [PATCH] net/bonding: improve non-ip packets RSS

2016-11-18 Thread Haifeng Lin
Most ethernet not support non-ip packets RSS and only first
queue can used to receive. In this scenario lacp bond can
only use one queue even if multi queue configured.

We use below formula to change the map between bond_qid and
slave_qid to let at least slave_num queues to receive packets:

slave_qid = (bond_qid + slave_id) % queue_num

Signed-off-by: Haifeng Lin 
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
b/drivers/net/bonding/rte_eth_bond_pmd.c
index 09ce7bf..8ad843a 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -141,6 +141,8 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf 
**bufs,
uint8_t collecting;  /* current slave collecting status */
const uint8_t promisc = internals->promiscuous_en;
uint8_t i, j, k;
+   int slave_qid, bond_qid = bd_rx_q->queue_id;
+   int queue_num = internals->nb_rx_queues;

rte_eth_macaddr_get(internals->port_id, _mac);
/* Copy slave list to protect against slave up/down changes during tx
@@ -154,7 +156,9 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf 
**bufs,
collecting = ACTOR_STATE(_8023ad_ports[slaves[i]], 
COLLECTING);

/* Read packets from this slave */
-   num_rx_total += rte_eth_rx_burst(slaves[i], bd_rx_q->queue_id,
+   slave_qid = queue_num ? (bond_qid + slaves[i]) % queue_num :
+   bond_qid;
+   num_rx_total += rte_eth_rx_burst(slaves[i], slave_qid,
[num_rx_total], nb_pkts - num_rx_total);

for (k = j; k < 2 && k < num_rx_total; k++)
-- 
1.8.3.1




[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
On Fri, Nov 18, 2016 at 4:29 PM, Thomas Monjalon 
wrote:

> 2016-11-18 16:06, Alejandro Lucero:
> > Previous reported speed was hardcoded.
> >
> > Signed-off-by: Alejandro Lucero 
> > ---
> >  drivers/net/nfp/nfp_net.c  | 28 ++--
> >  drivers/net/nfp/nfp_net_ctrl.h | 13 +
> >  2 files changed, 39 insertions(+), 2 deletions(-)
>
> You should update the doc in the same patch:
> doc/guides/nics/features/nfp.ini
> It will be the first feature as the file appears to be empty.
> So you will need another patch to fill other existing features.
>

Yes. I'm just working on updating that file properly.
May I delay this doc change for including it with that other one?
It will be a bit weird to just have one feature there.


>
> I have an unrelated question: why nfp is disabled in the default build?
>
>
Because NFP PMD can just work if Netronome BSP is installed in the system.
We do not support PF with the PMD, so it requires a Linux PF driver which
comes with the BSP.

The compilation has no dependencies, but we had our own UIO driver before
(now using igb_uio). So basically, we wanted the people aware of this
dependency and to specifically configure this option.

I know what you are surely going to ask about DPDK in Linux distributions,
and that this being a bad idea. The fact is, we have people using NFP PMD
as part of a product, so installing that product implies to (automatically)
install the BSP and a specific DPDK version with the NFP PMD enabled. But
yes, maybe we should modify this and to add some sort of BSP check inside
the PMD.

So, thanks for the heads up. I will think about this.


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Thomas Monjalon
2016-11-18 15:31, Alejandro Lucero:
> On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit 
> wrote:
> 
> > On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > > Hi Thomas,
> > >
> > > I got this email when sending a patch some minutes ago.
> > >
> > > The point is I trusted script/checkpatches.sh which did not report those
> > > warnings.
> > > Am I doing anything wrong when using checkpatches.sh?
> >
> > I am also getting same warnings as below, this can be related to the
> > checkpatch.pl version.
> >
> > I have: Version: 0.32
> > (./scripts/checkpatch.pl --version)
> >
> Uhmm, I got same one.

The last update of this version number is from 2011...
I guess we have to live without checkpatch versioning.


[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler

2016-11-18 Thread Bruce Richardson
On Fri, Nov 18, 2016 at 08:50:52AM -0500, Neil Horman wrote:
> On Fri, Nov 18, 2016 at 12:03:19PM +, Hemant Agrawal wrote:
> > > -Original Message-
> > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote:
> > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote:
> > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote:
> > > > > > > Hi Neil,
> > > > > > >Pmdinfogen compiles with host compiler. It usages
> > > rte_byteorder.h
> > > > > of the target platform.
> > > > > > > However, if the host compiler is older than 4.8, it will be an 
> > > > > > > issue during
> > > cross
> > > > > compilation for some platforms.
> > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler 
> > > > > > > will not
> > > > > understand the arm asm instructions.
> > > > > > >
> > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if
> > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static
> > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > > >register uint16_t x = _x;
> > > > > > >asm volatile ("rev16 %0,%1"
> > > > > > > : "=r" (x)
> > > > > > > : "r" (x)
> > > > > > > );
> > > > > > >return x;
> > > > > > > }
> > > > > > > #endif
> > > > > > >
> > > > > > > One easy solution is that we add compiler platform check in this
> > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || 
> > > > > > > defined
> > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) 
> > > > > > > {
> > > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else ?.
> > > > > > >
> > > > > > > Is there a better way to fix it?
> > > > > >
> > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using
> > > > > > the dpdk service then it should compile and link against HOST
> > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I
> > > > > > think, introducing the HOSTTARGET kind of scheme is a clean 
> > > > > > solution.
> > > > > >
> > > > > > /Jerin
> > > > > >
> > > > > >
> > > > > That would be accurate.  That is to say, pmdinfogen is a tool that 
> > > > > should only
> > > be
> > > > > run on the host doing the build, by the host doing the build, and so 
> > > > > should be
> > > > > compiled to run on the host, not on the target being built for.
> > > > >
> > > > > Yeah, so what we need is a way to get to the host version of 
> > > > > rte_byteorder.h
> > > > > when building in a cross environment
> > > > >
> > > > +1
> > > >
> > > > > Neil
> > > >
> > > 
> > > Give this a try, I've tested it on linux, but not BSD.  From what I read 
> > > the
> > > functions are not posix compliant, though they should exist on all BSD 
> > > and Linux
> > > systems in recent history.  There may be some fiddling needed for Net and
> > > OpenBSD variants, but I think this is the right general direction.
> > 
> > + 1
> > This patch works good for Linux. 
> > 
> Can someone test it on BSD?  I'd like to ensure we don't need to modify it for
> that platform
> 
> Neil
> 
> > > 
> > > 
> > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h
> > > b/buildtools/pmdinfogen/pmdinfogen.h
> > > index 1da2966..c5ef89d 100644
> > > --- a/buildtools/pmdinfogen/pmdinfogen.h
> > > +++ b/buildtools/pmdinfogen/pmdinfogen.h
> > > @@ -21,7 +21,6 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > -#include 
> > > 
> > >  /* On BSD-alike OSes elf.h defines these according to host's word size */
> > >  #undef ELF_ST_BIND
> > > @@ -75,9 +74,9 @@
> > >  #define CONVERT_NATIVE(fend, width, x) ({ \
> > >  typeof(x) ___x; \
> > >  if ((fend) == ELFDATA2LSB) \
> > > - ___x = rte_le_to_cpu_##width(x); \
> > > + ___x = le##width##toh(x); \
> > >  else \
> > > - ___x = rte_be_to_cpu_##width(x); \
> > > + ___x = be##width##toh(x); \
> > >   ___x; \
> > >  })
> > > 

For compile on FreeBSD 10 we need "#include " and this
works.

/Bruce


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Thomas Monjalon
2016-11-18 15:24, Ferruh Yigit:
> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> > 
> > I got this email when sending a patch some minutes ago.
> > 
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
> 
> I am also getting same warnings as below, this can be related to the
> checkpatch.pl version.
> 
> I have: Version: 0.32
> (./scripts/checkpatch.pl --version)

Yes checkpatch at dpdk.org uses the version 0.32.
I could try to add it in the mail report.


[dpdk-dev] [PATCH] vdev: fix missing alias check on uninit

2016-11-18 Thread Anatoly Burakov
Fixes: d63eed6b2dca ("eal: add driver name alias")

Signed-off-by: Anatoly Burakov 
---
 lib/librte_eal/common/eal_common_vdev.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c 
b/lib/librte_eal/common/eal_common_vdev.c
index 0ff2377..7d6e54f 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -111,6 +111,14 @@ rte_eal_vdev_uninit(const char *name)
return driver->remove(name);
}

+   /* Give new names precedence over aliases. */
+   TAILQ_FOREACH(driver, _driver_list, next) {
+   if (driver->driver.alias &&
+   !strncmp(driver->driver.alias, name,
+   strlen(driver->driver.alias)))
+   return driver->remove(name);
+   }
+
RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
return -EINVAL;
 }
-- 
2.5.5



[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
Previous reported speed was hardcoded.

Signed-off-by: Alejandro Lucero 
---
 drivers/net/nfp/nfp_net.c  | 28 ++--
 drivers/net/nfp/nfp_net_ctrl.h | 13 +
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..24f3164 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -816,6 +816,17 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
struct rte_eth_link link, old;
uint32_t nn_link_status;

+   static const uint32_t ls_to_ethtool[] = {
+   [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE,
+   [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE,
+   [NFP_NET_CFG_STS_LINK_RATE_1G]  = ETH_SPEED_NUM_1G,
+   [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G,
+   [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G,
+   [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G,
+   [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G,
+   [NFP_NET_CFG_STS_LINK_RATE_100G]= ETH_SPEED_NUM_100G,
+   };
+
PMD_DRV_LOG(DEBUG, "Link update\n");

hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -831,8 +842,21 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
link.link_status = ETH_LINK_UP;

link.link_duplex = ETH_LINK_FULL_DUPLEX;
-   /* Other cards can limit the tx and rx rate per VF */
-   link.link_speed = ETH_SPEED_NUM_40G;
+
+   nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
+NFP_NET_CFG_STS_LINK_RATE_MASK;
+
+   if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) ||
+   ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) &&
+   (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0)))
+   link.link_speed = ETH_SPEED_NUM_40G;
+   else {
+   if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN ||
+   nn_link_status >= RTE_DIM(ls_to_ethtool))
+   link.link_speed = ETH_SPEED_NUM_NONE;
+   else
+   link.link_speed = ls_to_ethtool[nn_link_status];
+   }

if (old.link_status != link.link_status) {
nfp_net_dev_atomic_write_link_status(dev, );
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index fce8251..f9aaba3 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -157,6 +157,19 @@
 #define   NFP_NET_CFG_VERSION_MINOR(x)(((x) & 0xff) <<  0)
 #define NFP_NET_CFG_STS 0x0034
 #define   NFP_NET_CFG_STS_LINK(0x1 << 0) /* Link up or down */
+/* Link rate */
+#define   NFP_NET_CFG_STS_LINK_RATE_SHIFT 1
+#define   NFP_NET_CFG_STS_LINK_RATE_MASK  0xF
+#define   NFP_NET_CFG_STS_LINK_RATE   \
+ (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT)
+#define   NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED   0
+#define   NFP_NET_CFG_STS_LINK_RATE_UNKNOWN   1
+#define   NFP_NET_CFG_STS_LINK_RATE_1G2
+#define   NFP_NET_CFG_STS_LINK_RATE_10G   3
+#define   NFP_NET_CFG_STS_LINK_RATE_25G   4
+#define   NFP_NET_CFG_STS_LINK_RATE_40G   5
+#define   NFP_NET_CFG_STS_LINK_RATE_50G   6
+#define   NFP_NET_CFG_STS_LINK_RATE_100G  7
 #define NFP_NET_CFG_CAP 0x0038
 #define NFP_NET_CFG_MAX_TXRINGS 0x003c
 #define NFP_NET_CFG_MAX_RXRINGS 0x0040
-- 
1.9.1



[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation

2016-11-18 Thread Bruce Richardson
+Thomas

On Fri, Nov 18, 2016 at 03:25:18PM +, Bruce Richardson wrote:
> On Fri, Nov 18, 2016 at 11:14:58AM +0530, Jerin Jacob wrote:
> > As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> > described in [3] (also pasted below), here is the first non-draft series
> > for this new API.
> > 
> > [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
> > [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
> > [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html
> > 
> > Changes since RFC v2:
> > 
> > - Updated the documentation to define the need for this library[Jerin]
> > - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in
> >   struct rte_event_queue_conf to enable optimized sw implementation [Bruce]
> > - Introduced RTE_EVENT_OP* ops [Bruce]
> > - Added nb_event_queue_flows,nb_event_port_dequeue_depth, 
> > nb_event_port_enqueue_depth
> >   in rte_event_dev_configure() like ethdev and crypto library[Jerin]
> > - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to
> >   reduce fast path APIs and it is redundant too[Jerin]
> > - In the view of better application portability, Removed pin_event
> >   from rte_event_enqueue as it is just hint and Intel/NXP can not support 
> > it[Jerin]
> > - Added rte_event_port_links_get()[Jerin]
> > - Added rte_event_dev_dump[Harry]
> > 
> > Notes:
> > 
> > - This patch set is check-patch clean with an exception that
> > 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL
> > - Looking forward to getting additional maintainers for libeventdev
> > 
> > 
> > Possible next steps:
> > 1) Review this patch set
> > 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/]
> > 3) Review proposed examples/eventdev_pipeline 
> > application[http://dpdk.org/dev/patchwork/patch/17053/]
> > 4) Review proposed functional 
> > tests[http://dpdk.org/dev/patchwork/patch/17051/]
> > 5) Cavium's HW based eventdev driver
> > 
> > I am planning to work on (3),(4) and (5)
> > 
> Thanks Jerin,
> 
> we'll review and get back to you with any comments or feedback (1), and
> obviously start working on item (2) also! :-)
> 
> I'm also wonder whether we should have a staging tree for this work to
> make interaction between us easier. Although this may not be
> finalised enough for 17.02 release, do you think having an
> dpdk-eventdev-next tree would be a help? My thinking is that once we get
> the eventdev library itself in reasonable shape following our review, we
> could commit that and make any changes thereafter as new patches, rather
> than constantly respinning the same set. It also gives us a clean git
> tree to base the respective driver implementations on from our two sides.
> 
> Thomas, any thoughts here on your end - or from anyone else?
> 
> Regards,
> /Bruce
> 


[dpdk-dev] [PATCH v5 4/4] latencystats: added new library for latency stats

2016-11-18 Thread Remy Horton
Add a library designed to calculate latency statistics and report them
to the application when queried. The library measures minimum, average and
maximum latencies, and jitter in nano seconds. The current implementation
supports global latency stats, i.e. per application stats.

Added new field to mbuf struct to mark the packet arrival time on Rx.

Modify testpmd code to initialize/uninitialize latency statistics
calulation.

Modify the dpdk-procinfo process to display the newly added metrics.
Added new command line option "--metrics" to display metrics.

APIs:

* Added APIs to initialize and un initialize latency stats
  calculation.
* Added API to retrieve latency stats names and values.

Functionality:

* The library will register ethdev Rx/Tx callbacks for each active port,
  queue combinations.
* The library will register latency stats names with new metrics library.
* Rx packets will be marked with time stamp on each sampling interval.
* On Tx side, packets with time stamp will be considered for calculating
  the minimum, maximum, average latencies and also jitter.
* Average latency is calculated using exponential weighted moving average
  method.
* Minimum and maximum latencies will be low and high latency values
  observed so far.
* Jitter calculation is done based on inter packet delay variation.
* Measured stats are reported to the metrics library in a separate
  pthread.
* Measured stats can be retrieved via get API of the libray (or)
  by calling generic get API of the new metrics library.

Signed-off-by: Reshma Pattan 
Signed-off-by: Remy Horton 
---
 MAINTAINERS|   4 +
 app/proc_info/main.c   |  70 
 app/test-pmd/testpmd.c |  10 +
 config/common_base |   5 +
 doc/api/doxy-api-index.md  |   1 +
 doc/api/doxy-api.conf  |   1 +
 doc/guides/rel_notes/release_17_02.rst |   5 +
 lib/Makefile   |   1 +
 lib/librte_latencystats/Makefile   |  57 +++
 lib/librte_latencystats/rte_latencystats.c | 389 +
 lib/librte_latencystats/rte_latencystats.h | 146 
 .../rte_latencystats_version.map   |  10 +
 lib/librte_mbuf/rte_mbuf.h |   3 +
 mk/rte.app.mk  |   2 +-
 14 files changed, 703 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_latencystats/Makefile
 create mode 100644 lib/librte_latencystats/rte_latencystats.c
 create mode 100644 lib/librte_latencystats/rte_latencystats.h
 create mode 100644 lib/librte_latencystats/rte_latencystats_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index d6bbdd5..da7f9d1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -713,3 +713,7 @@ F: examples/tep_termination/
 F: examples/vmdq/
 F: examples/vmdq_dcb/
 F: doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst
+
+Latency Stats
+M: Reshma Pattan 
+F: lib/librte_latencystats/
diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2c56d10..33a4b39 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -57,6 +57,7 @@
 #include 
 #include 
 #include 
+#include 

 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
@@ -68,6 +69,8 @@ static uint32_t enabled_port_mask;
 static uint32_t enable_stats;
 /**< Enable xstats. */
 static uint32_t enable_xstats;
+/**< Enable metrics. */
+static uint32_t enable_metrics;
 /**< Enable stats reset. */
 static uint32_t reset_stats;
 /**< Enable xstats reset. */
@@ -85,6 +88,8 @@ proc_info_usage(const char *prgname)
"  --stats: to display port statistics, enabled by default\n"
"  --xstats: to display extended port statistics, disabled by "
"default\n"
+   "  --metrics: to display derived metrics of the ports, disabled 
by "
+   "default\n"
"  --stats-reset: to reset port statistics\n"
"  --xstats-reset: to reset port extended statistics\n",
prgname);
@@ -127,6 +132,7 @@ proc_info_parse_args(int argc, char **argv)
{"stats", 0, NULL, 0},
{"stats-reset", 0, NULL, 0},
{"xstats", 0, NULL, 0},
+   {"metrics", 0, NULL, 0},
{"xstats-reset", 0, NULL, 0},
{NULL, 0, 0, 0}
};
@@ -159,6 +165,10 @@ proc_info_parse_args(int argc, char **argv)
else if (!strncmp(long_option[option_index].name, 
"xstats",
MAX_LONG_OPT_SZ))
enable_xstats = 1;
+   else if (!strncmp(long_option[option_index].name,
+   "metrics",
+   MAX_LONG_OPT_SZ))
+   enable_metrics = 1;

[dpdk-dev] [PATCH v5 3/4] app/test-pmd: add support for bitrate statistics

2016-11-18 Thread Remy Horton
Signed-off-by: Remy Horton 
---
 app/test-pmd/testpmd.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index a0332c2..60c635f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -78,6 +78,10 @@
 #ifdef RTE_LIBRTE_PDUMP
 #include 
 #endif
+#include 
+#ifdef RTE_LIBRTE_BITRATE
+#include 
+#endif

 #include "testpmd.h"

@@ -322,6 +326,9 @@ uint16_t nb_rx_queue_stats_mappings = 0;

 unsigned max_socket = 0;

+/* Bitrate statistics */
+struct rte_stats_bitrates_s *bitrate_data;
+
 /* Forward function declarations */
 static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port 
*port);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -921,12 +928,32 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t 
pkt_fwd)
struct fwd_stream **fsm;
streamid_t nb_fs;
streamid_t sm_id;
+#ifdef RTE_LIBRTE_BITRATE
+   uint64_t tics_per_1sec;
+   uint64_t tics_datum;
+   uint64_t tics_current;
+   uint8_t idx_port, cnt_ports;
+#endif

+#ifdef RTE_LIBRTE_BITRATE
+   cnt_ports = rte_eth_dev_count();
+   tics_datum = rte_rdtsc();
+   tics_per_1sec = rte_get_timer_hz();
+#endif
fsm = _streams[fc->stream_idx];
nb_fs = fc->stream_nb;
do {
for (sm_id = 0; sm_id < nb_fs; sm_id++)
(*pkt_fwd)(fsm[sm_id]);
+#ifdef RTE_LIBRTE_BITRATE
+   tics_current = rte_rdtsc();
+   if (tics_current - tics_datum >= tics_per_1sec) {
+   /* Periodic bitrate calculation */
+   for (idx_port = 0; idx_port < cnt_ports; idx_port++)
+   rte_stats_bitrate_calc(bitrate_data, idx_port);
+   tics_datum = tics_current;
+   }
+#endif
} while (! fc->stopped);
 }

@@ -2133,6 +2160,15 @@ main(int argc, char** argv)
FOREACH_PORT(port_id, ports)
rte_eth_promiscuous_enable(port_id);

+   /* Setup bitrate stats */
+#ifdef RTE_LIBRTE_BITRATE
+   bitrate_data = rte_stats_bitrate_create();
+   if (bitrate_data == NULL)
+   rte_exit(EXIT_FAILURE, "Could not allocate bitrate data.\n");
+   rte_stats_bitrate_reg(bitrate_data);
+#endif
+
+
 #ifdef RTE_LIBRTE_CMDLINE
if (interactive == 1) {
if (auto_start) {
-- 
2.5.5



[dpdk-dev] [PATCH v5 1/4] lib: add information metrics library

2016-11-18 Thread Remy Horton
This patch adds a new information metric library that allows other
modules to register named metrics and update their values. It is
intended to be independent of ethdev, rather than mixing ethdev
and non-ethdev information in xstats.

Signed-off-by: Remy Horton 
---
 MAINTAINERS|   5 +
 config/common_base |   5 +
 doc/api/doxy-api-index.md  |   1 +
 doc/api/doxy-api.conf  |   1 +
 doc/guides/rel_notes/release_17_02.rst |   7 +
 lib/Makefile   |   1 +
 lib/librte_metrics/Makefile|  51 +
 lib/librte_metrics/rte_metrics.c   | 308 +
 lib/librte_metrics/rte_metrics.h   | 190 ++
 lib/librte_metrics/rte_metrics_version.map |  13 ++
 mk/rte.app.mk  |   2 +
 11 files changed, 584 insertions(+)
 create mode 100644 lib/librte_metrics/Makefile
 create mode 100644 lib/librte_metrics/rte_metrics.c
 create mode 100644 lib/librte_metrics/rte_metrics.h
 create mode 100644 lib/librte_metrics/rte_metrics_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index d6bb8f8..52bd8a9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -595,6 +595,11 @@ F: lib/librte_jobstats/
 F: examples/l2fwd-jobstats/
 F: doc/guides/sample_app_ug/l2_forward_job_stats.rst

+Metrics
+M: Remy Horton 
+F: lib/librte_metrics/
+F: doc/guides/sample_app_ug/keep_alive.rst
+

 Test Applications
 -
diff --git a/config/common_base b/config/common_base
index 4bff83a..dedc4c3 100644
--- a/config/common_base
+++ b/config/common_base
@@ -589,3 +589,8 @@ CONFIG_RTE_APP_TEST_RESOURCE_TAR=n
 CONFIG_RTE_TEST_PMD=y
 CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES=n
 CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS=n
+
+#
+# Compile the device metrics library
+#
+CONFIG_RTE_LIBRTE_METRICS=y
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 6675f96..ca50fa6 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -147,4 +147,5 @@ There are many libraries, so their headers may be grouped 
by topics:
   [common] (@ref rte_common.h),
   [ABI compat] (@ref rte_compat.h),
   [keepalive]  (@ref rte_keepalive.h),
+  [Device Metrics] (@ref rte_metrics.h),
   [version](@ref rte_version.h)
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index 9dc7ae5..fe830eb 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -57,6 +57,7 @@ INPUT   = doc/api/doxy-api-index.md \
   lib/librte_reorder \
   lib/librte_ring \
   lib/librte_sched \
+  lib/librte_metrics \
   lib/librte_table \
   lib/librte_timer \
   lib/librte_vhost
diff --git a/doc/guides/rel_notes/release_17_02.rst 
b/doc/guides/rel_notes/release_17_02.rst
index 3b65038..2d82dd1 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -34,6 +34,12 @@ New Features

  Refer to the previous release notes for examples.

+   * **Added information metric library.**
+
+ A library that allows information metrics to be added and update. It is
+ intended to provide a reporting mechanism that is independent of the
+ ethdev library.
+
  This section is a comment. do not overwrite or remove it.
  Also, make sure to start the actual text at the margin.
  =
@@ -152,6 +158,7 @@ The libraries prepended with a plus sign were incremented 
in this version.
  librte_mbuf.so.2
  librte_mempool.so.2
  librte_meter.so.1
+   + librte_metrics.so.1
  librte_net.so.1
  librte_pdump.so.1
  librte_pipeline.so.3
diff --git a/lib/Makefile b/lib/Makefile
index 990f23a..5d85dcf 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -58,6 +58,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TABLE) += librte_table
 DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += librte_pipeline
 DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder
 DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump
+DIRS-$(CONFIG_RTE_LIBRTE_METRICS) += librte_metrics

 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_metrics/Makefile b/lib/librte_metrics/Makefile
new file mode 100644
index 000..8d6e23a
--- /dev/null
+++ b/lib/librte_metrics/Makefile
@@ -0,0 +1,51 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2016 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+# * Redistributions of source code must retain the above copyright
+#   notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form 

[dpdk-dev] [PATCH v5 0/4] Expanded statistics reporting

2016-11-18 Thread Remy Horton
This patchset extends statistics reporting to include peak and
average data-rate metrics. It comes in two parts: a statistics
reporting library, and a bitrate calculation library that uses
it. This structure is intended to seperate statistic reporting
from ethdev and allow more flexible metric registration.

Due to merge issues Reshma's latency statistics, which depends
on the reporting library, has been merged into this patchset.

--
v5 changes:
* Updated Shared Library Versions in release notes
* Merged in Reshma's latencystats library

v4 changes:
* References to 16.11 changed to 17.02
* Fetching of non-port values was broken
* Added sanity checks to value fetching
* rte_stat_value renamed to rte_metric_value
* Corrected doxygen descriptions
* Added MAINTAINERS entries
* Added #ifdef directives to bitrate code in test-pmd

v3 changes:
* Marked rte_stats_bitrate_s as internal
* Minor integer roundoff correction
* Coding style corrections
* Removed spurious object allocation
* Changes to rte_metrics.[ch] moved from Patch 2/3 to 1/3.
* Reintroduced non-port values (RTE_METRICS_NONPORT)
* Added spinlocks to metric library
* Removed spurious test registration/update
* Added release notes entries

v2 changes:
* Uses a new metrics library rather than being part of ethdev


Remy Horton (4):
  lib: add information metrics library
  lib: add bitrate statistics library
  app/test-pmd: add support for bitrate statistics
  latencystats: added new library for latency stats

 MAINTAINERS|  13 +
 app/proc_info/main.c   |  70 
 app/test-pmd/testpmd.c |  46 +++
 config/common_base |  15 +
 doc/api/doxy-api-index.md  |   3 +
 doc/api/doxy-api.conf  |   3 +
 doc/guides/rel_notes/release_17_02.rst |  18 +
 lib/Makefile   |   3 +
 lib/librte_bitratestats/Makefile   |  53 +++
 lib/librte_bitratestats/rte_bitrate.c  | 128 +++
 lib/librte_bitratestats/rte_bitrate.h  |  80 +
 .../rte_bitratestats_version.map   |   9 +
 lib/librte_latencystats/Makefile   |  57 +++
 lib/librte_latencystats/rte_latencystats.c | 389 +
 lib/librte_latencystats/rte_latencystats.h | 146 
 .../rte_latencystats_version.map   |  10 +
 lib/librte_mbuf/rte_mbuf.h |   3 +
 lib/librte_metrics/Makefile|  51 +++
 lib/librte_metrics/rte_metrics.c   | 308 
 lib/librte_metrics/rte_metrics.h   | 190 ++
 lib/librte_metrics/rte_metrics_version.map |  13 +
 mk/rte.app.mk  |   3 +
 22 files changed, 1611 insertions(+)
 create mode 100644 lib/librte_bitratestats/Makefile
 create mode 100644 lib/librte_bitratestats/rte_bitrate.c
 create mode 100644 lib/librte_bitratestats/rte_bitrate.h
 create mode 100644 lib/librte_bitratestats/rte_bitratestats_version.map
 create mode 100644 lib/librte_latencystats/Makefile
 create mode 100644 lib/librte_latencystats/rte_latencystats.c
 create mode 100644 lib/librte_latencystats/rte_latencystats.h
 create mode 100644 lib/librte_latencystats/rte_latencystats_version.map
 create mode 100644 lib/librte_metrics/Makefile
 create mode 100644 lib/librte_metrics/rte_metrics.c
 create mode 100644 lib/librte_metrics/rte_metrics.h
 create mode 100644 lib/librte_metrics/rte_metrics_version.map

-- 
2.5.5



[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
On Fri, Nov 18, 2016 at 3:31 PM, Alejandro Lucero <
alejandro.lucero at netronome.com> wrote:

>
>
> On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit 
> wrote:
>
>> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
>> > Hi Thomas,
>> >
>> > I got this email when sending a patch some minutes ago.
>> >
>> > The point is I trusted script/checkpatches.sh which did not report those
>> > warnings.
>> > Am I doing anything wrong when using checkpatches.sh?
>>
>> I am also getting same warnings as below, this can be related to the
>> checkpatch.pl version.
>>
>> I have: Version: 0.32
>> (./scripts/checkpatch.pl --version)
>>
>>
> Uhmm, I got same one.
>
>

Ok. It seems I suffered a temporal blindness. I though the automatic report
was about warnings but it is about checks. But I got just one of the checks
messages. This is the output with -v and adding OPTIONS used:

### [PATCH] nfp: report link speed using hardware info


OPTIONS: --no-tree --max-line-length=80 --show-types
--ignore=LINUX_VERSION_CODE,FILE_PATH_CHANGES,VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,SPLIT_STRING,LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,NEW_TYPEDEFS,COMPARISON_TO_NULL

CHECK:BRACES: Blank lines aren't necessary after an open brace '{'

#60: FILE: drivers/net/nfp/nfp_net.c:856:

+ else {

+


total: 0 errors, 0 warnings, 1 checks, 68 lines checked


0/1 valid patch






> >
>> >
>> > -- Forwarded message --
>> > From: 
>> > Date: Fri, Nov 18, 2016 at 3:04 PM
>> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
>> > To: test-report at dpdk.org
>> > Cc: Alejandro Lucero 
>> >
>> >
>> > Test-Label: checkpatch
>> > Test-Status: WARNING
>> > http://dpdk.org/patch/17091
>> >
>> > _coding style issues_
>> >
>> >
>> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible
>> side-effects?
>> > #53: FILE: drivers/net/nfp/nfp_net.c:806:
>> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>> >
>> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
>> > #91: FILE: drivers/net/nfp/nfp_net.c:856:
>> > +   else {
>> > +
>> >
>> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked
>> >
>>
>>
>


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
On Fri, Nov 18, 2016 at 3:26 PM, Ferruh Yigit 
wrote:

> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> >
> > I got this email when sending a patch some minutes ago.
> >
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
> >
> >
> > -- Forwarded message --
> > From: 
> > Date: Fri, Nov 18, 2016 at 3:04 PM
> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> > To: test-report at dpdk.org
> > Cc: Alejandro Lucero 
> >
> >
> > Test-Label: checkpatch
> > Test-Status: WARNING
> > http://dpdk.org/patch/17091
> >
> > _coding style issues_
> >
> >
> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible
> side-effects?
> > #53: FILE: drivers/net/nfp/nfp_net.c:806:
> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>
> btw, you can benefit from RTE_DIM:
>
> lib/librte_eal/common/include/rte_common.h:352:
> #define  RTE_DIM(a)  (sizeof (a) / sizeof ((a)[0]))
>
>
Thanks!

I will use it in the next patch version.


> >
> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> > #91: FILE: drivers/net/nfp/nfp_net.c:856:
> > +   else {
> > +
> >
> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> >
>
>


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit 
wrote:

> On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> > Hi Thomas,
> >
> > I got this email when sending a patch some minutes ago.
> >
> > The point is I trusted script/checkpatches.sh which did not report those
> > warnings.
> > Am I doing anything wrong when using checkpatches.sh?
>
> I am also getting same warnings as below, this can be related to the
> checkpatch.pl version.
>
> I have: Version: 0.32
> (./scripts/checkpatch.pl --version)
>
>
Uhmm, I got same one.


> >
> >
> > -- Forwarded message --
> > From: 
> > Date: Fri, Nov 18, 2016 at 3:04 PM
> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> > To: test-report at dpdk.org
> > Cc: Alejandro Lucero 
> >
> >
> > Test-Label: checkpatch
> > Test-Status: WARNING
> > http://dpdk.org/patch/17091
> >
> > _coding style issues_
> >
> >
> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible
> side-effects?
> > #53: FILE: drivers/net/nfp/nfp_net.c:806:
> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> >
> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> > #91: FILE: drivers/net/nfp/nfp_net.c:856:
> > +   else {
> > +
> >
> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> >
>
>


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Ferruh Yigit
On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> Hi Thomas,
> 
> I got this email when sending a patch some minutes ago.
> 
> The point is I trusted script/checkpatches.sh which did not report those
> warnings.
> Am I doing anything wrong when using checkpatches.sh?
> 
> 
> -- Forwarded message --
> From: 
> Date: Fri, Nov 18, 2016 at 3:04 PM
> Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> To: test-report at dpdk.org
> Cc: Alejandro Lucero 
> 
> 
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/17091
> 
> _coding style issues_
> 
> 
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
> #53: FILE: drivers/net/nfp/nfp_net.c:806:
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))

btw, you can benefit from RTE_DIM:

lib/librte_eal/common/include/rte_common.h:352:
#define  RTE_DIM(a)  (sizeof (a) / sizeof ((a)[0]))

> 
> CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> #91: FILE: drivers/net/nfp/nfp_net.c:856:
> +   else {
> +
> 
> total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> 



[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation

2016-11-18 Thread Bruce Richardson
On Fri, Nov 18, 2016 at 11:14:58AM +0530, Jerin Jacob wrote:
> As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> described in [3] (also pasted below), here is the first non-draft series
> for this new API.
> 
> [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
> [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
> [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html
> 
> Changes since RFC v2:
> 
> - Updated the documentation to define the need for this library[Jerin]
> - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in
>   struct rte_event_queue_conf to enable optimized sw implementation [Bruce]
> - Introduced RTE_EVENT_OP* ops [Bruce]
> - Added nb_event_queue_flows,nb_event_port_dequeue_depth, 
> nb_event_port_enqueue_depth
>   in rte_event_dev_configure() like ethdev and crypto library[Jerin]
> - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to
>   reduce fast path APIs and it is redundant too[Jerin]
> - In the view of better application portability, Removed pin_event
>   from rte_event_enqueue as it is just hint and Intel/NXP can not support 
> it[Jerin]
> - Added rte_event_port_links_get()[Jerin]
> - Added rte_event_dev_dump[Harry]
> 
> Notes:
> 
> - This patch set is check-patch clean with an exception that
> 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL
> - Looking forward to getting additional maintainers for libeventdev
> 
> 
> Possible next steps:
> 1) Review this patch set
> 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/]
> 3) Review proposed examples/eventdev_pipeline 
> application[http://dpdk.org/dev/patchwork/patch/17053/]
> 4) Review proposed functional 
> tests[http://dpdk.org/dev/patchwork/patch/17051/]
> 5) Cavium's HW based eventdev driver
> 
> I am planning to work on (3),(4) and (5)
> 
Thanks Jerin,

we'll review and get back to you with any comments or feedback (1), and
obviously start working on item (2) also! :-)

I'm also wonder whether we should have a staging tree for this work to
make interaction between us easier. Although this may not be
finalised enough for 17.02 release, do you think having an
dpdk-eventdev-next tree would be a help? My thinking is that once we get
the eventdev library itself in reasonable shape following our review, we
could commit that and make any changes thereafter as new patches, rather
than constantly respinning the same set. It also gives us a clean git
tree to base the respective driver implementations on from our two sides.

Thomas, any thoughts here on your end - or from anyone else?

Regards,
/Bruce



[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Ferruh Yigit
On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> Hi Thomas,
> 
> I got this email when sending a patch some minutes ago.
> 
> The point is I trusted script/checkpatches.sh which did not report those
> warnings.
> Am I doing anything wrong when using checkpatches.sh?

I am also getting same warnings as below, this can be related to the
checkpatch.pl version.

I have: Version: 0.32
(./scripts/checkpatch.pl --version)

> 
> 
> -- Forwarded message --
> From: 
> Date: Fri, Nov 18, 2016 at 3:04 PM
> Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> To: test-report at dpdk.org
> Cc: Alejandro Lucero 
> 
> 
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/17091
> 
> _coding style issues_
> 
> 
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
> #53: FILE: drivers/net/nfp/nfp_net.c:806:
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> 
> CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> #91: FILE: drivers/net/nfp/nfp_net.c:856:
> +   else {
> +
> 
> total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> 



[dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in comment

2016-11-18 Thread Olivier Matz
Hi Wei,

2lOn 11/15/2016 07:54 AM, Zhao1, Wei wrote:
> Hi, john
> 
>> -Original Message-
>> From: Mcnamara, John
>> Sent: Monday, November 14, 2016 6:30 PM
>> To: Zhao1, Wei ; dev at dpdk.org
>> Cc: olivier.matz at 6wind.com; Zhao1, Wei 
>> Subject: RE: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in
>> comment
>>
>>
>>
>>> -Original Message-
>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao
>>> Sent: Monday, November 14, 2016 2:47 AM
>>> To: dev at dpdk.org
>>> Cc: olivier.matz at 6wind.com; Zhao1, Wei 
>>> Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in
>>> comment
>>>
>>> From: zhao wei 
>>
>> I think you need to add your name to gitconfig file on the sending machine to
>> avoid this "From:"
>>
>>>
>>> There is a redundant repetition word "for" in commnet line the file
>>> rte_mempool.h after the definition of RTE_MEMPOOL_OPS_NAMESIZE.
>>> The word "for"appear twice in line 359 and 360.One of them is
>>> redundant, so delete it.
>>>
>>> Fixes: 449c49b93a6b ("lib/librte_mempool: mempool: support handler
>>> operations")

The proper fixline should be:
  Fixes: 449c49b93a6b ("mempool: support handler operations")

(no need to add "lib/librte_mempool:")
This comment also applies to the other patch, I missed it.


>>>
>>> Signed-off-by: zhao wei 
>>
>> /commnet/comment/
>>
>> And same comment as before about the title. Apart from that:
>>
>> Acked-by: John McNamara 
>>
>>
> 
> Thank you for your suggestion,  I will change as your comment in following 
> patch!
> 

Also same comment about "mempool:" instead of "lib/librte_mempool: mempool:"


Thanks,
Olivier


[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
Hi Thomas,

I got this email when sending a patch some minutes ago.

The point is I trusted script/checkpatches.sh which did not report those
warnings.
Am I doing anything wrong when using checkpatches.sh?


-- Forwarded message --
From: 
Date: Fri, Nov 18, 2016 at 3:04 PM
Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
To: test-report at dpdk.org
Cc: Alejandro Lucero 


Test-Label: checkpatch
Test-Status: WARNING
http://dpdk.org/patch/17091

_coding style issues_


CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
#53: FILE: drivers/net/nfp/nfp_net.c:806:
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))

CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#91: FILE: drivers/net/nfp/nfp_net.c:856:
+   else {
+

total: 0 errors, 0 warnings, 2 checks, 68 lines checked


[dpdk-dev] [PATCH] lib/librte_mempool: a redundant of socket_id assignment

2016-11-18 Thread Olivier Matz
Hi Wei,

On 11/14/2016 11:25 AM, Mcnamara, John wrote:
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao
>> Sent: Monday, November 14, 2016 2:16 AM
>> To: dev at dpdk.org
>> Cc: olivier.matz at 6wind.com; Zhao1, Wei 
>> Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant of socket_id
>> assignment
>>
>> From: zhao wei 
>>
>> There is a redundant repetition mempool socket_id assignment in the file
>> rte_mempool.c in function rte_mempool_create_empty.The statement "mp-
>>> socket_id = socket_id;"appear twice in line 821 and 824.One of them is
>> redundant, so delete it.
>>
>> Fixes: 85226f9c526b ("lib/librte_mempool:  mempool:introduce a function to
>> create an empty pool")
>>
>> Signed-off-by: zhao wei 
> 
> Titles should generally start with a verb to indicate what is being done.
> Something like:
> 
> lib/librte_mempool: remove redundant socket_id assignment
> 
> Apart from that. 
> 
> Acked-by: John McNamara 

I would even say:
  mempool: remove redundant socket_id assignment

Acked-by: Olivier Matz 


[dpdk-dev] [PATCH] nfp: report link speed using hardware info

2016-11-18 Thread Alejandro Lucero
Previous reported speed was hardcoded.

Signed-off-by: Alejandro Lucero 
---
 drivers/net/nfp/nfp_net.c  | 31 +--
 drivers/net/nfp/nfp_net_ctrl.h | 13 +
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..d5ec0ff 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -803,6 +803,7 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
hw->ctrl = new_ctrl;
 }

+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 /*
  * return 0 means link status changed, -1 means not changed
  *
@@ -816,6 +817,18 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
struct rte_eth_link link, old;
uint32_t nn_link_status;

+   static const uint32_t ls_to_ethtool[] = {
+   [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE,
+   [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE,
+   [NFP_NET_CFG_STS_LINK_RATE_1G]  = ETH_SPEED_NUM_1G,
+   [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G,
+   [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G,
+   [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G,
+   [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G,
+   [NFP_NET_CFG_STS_LINK_RATE_100G]= ETH_SPEED_NUM_100G,
+   };
+
+
PMD_DRV_LOG(DEBUG, "Link update\n");

hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -831,8 +844,22 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
link.link_status = ETH_LINK_UP;

link.link_duplex = ETH_LINK_FULL_DUPLEX;
-   /* Other cards can limit the tx and rx rate per VF */
-   link.link_speed = ETH_SPEED_NUM_40G;
+
+   nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
+NFP_NET_CFG_STS_LINK_RATE_MASK;
+
+   if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) ||
+   ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) &&
+   (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0)))
+   link.link_speed = ETH_SPEED_NUM_40G;
+   else {
+
+   if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN ||
+   nn_link_status >= ARRAY_SIZE(ls_to_ethtool))
+   link.link_speed = ETH_SPEED_NUM_NONE;
+   else
+   link.link_speed = ls_to_ethtool[nn_link_status];
+   }

if (old.link_status != link.link_status) {
nfp_net_dev_atomic_write_link_status(dev, );
diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
index fce8251..f9aaba3 100644
--- a/drivers/net/nfp/nfp_net_ctrl.h
+++ b/drivers/net/nfp/nfp_net_ctrl.h
@@ -157,6 +157,19 @@
 #define   NFP_NET_CFG_VERSION_MINOR(x)(((x) & 0xff) <<  0)
 #define NFP_NET_CFG_STS 0x0034
 #define   NFP_NET_CFG_STS_LINK(0x1 << 0) /* Link up or down */
+/* Link rate */
+#define   NFP_NET_CFG_STS_LINK_RATE_SHIFT 1
+#define   NFP_NET_CFG_STS_LINK_RATE_MASK  0xF
+#define   NFP_NET_CFG_STS_LINK_RATE   \
+ (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT)
+#define   NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED   0
+#define   NFP_NET_CFG_STS_LINK_RATE_UNKNOWN   1
+#define   NFP_NET_CFG_STS_LINK_RATE_1G2
+#define   NFP_NET_CFG_STS_LINK_RATE_10G   3
+#define   NFP_NET_CFG_STS_LINK_RATE_25G   4
+#define   NFP_NET_CFG_STS_LINK_RATE_40G   5
+#define   NFP_NET_CFG_STS_LINK_RATE_50G   6
+#define   NFP_NET_CFG_STS_LINK_RATE_100G  7
 #define NFP_NET_CFG_CAP 0x0038
 #define NFP_NET_CFG_MAX_TXRINGS 0x003c
 #define NFP_NET_CFG_MAX_RXRINGS 0x0040
-- 
1.9.1



[dpdk-dev] [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX

2016-11-18 Thread Bruce Richardson
On Fri, Nov 18, 2016 at 04:52:13AM -0800, Satha Rao wrote:
> i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
>results. To solve this include rte memory barriers
> 
> Signed-off-by: Satha Rao 
> ---
>  drivers/net/i40e/base/i40e_osdep.h | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/i40e/base/i40e_osdep.h 
> b/drivers/net/i40e/base/i40e_osdep.h
> index 38e7ba5..ffa3160 100644
> --- a/drivers/net/i40e/base/i40e_osdep.h
> +++ b/drivers/net/i40e/base/i40e_osdep.h
> @@ -158,7 +158,13 @@ do { 
>\
>   ((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
>  static inline uint32_t i40e_read_addr(volatile void *addr)
>  {
> +#if defined(RTE_ARCH_ARM64)
> + uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr));
> + rte_rmb();
> + return val;
> +#else
>   return rte_le_to_cpu_32(I40E_PCI_REG(addr));
> +#endif
>  }
>  #define I40E_PCI_REG_WRITE(reg, value) \
>   do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
> @@ -171,8 +177,16 @@ static inline uint32_t i40e_read_addr(volatile void 
> *addr)
>   I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((hw), (reg)), (value))
>  
>  #define rd32(a, reg) i40e_read_addr(I40E_PCI_REG_ADDR((a), (reg)))
> +#if defined(RTE_ARCH_ARM64)
> +#define wr32(a, reg, value) \
> + do { \
> + I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)); \
> + rte_wmb(); \
> + } while (0)
> +#else
>  #define wr32(a, reg, value) \
>   I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value))
> +#endif
>  #define flush(a) i40e_read_addr(I40E_PCI_REG_ADDR((a), (I40E_GLGEN_STAT)))
>  
>  #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
> -- 

Would rte_smp_*mb() functions allow you to get a similar result without
the need for #ifdefs? It should be a full barrier on weakly ordered
platforms which just a compiler barrier on IA.

/Bruce


[dpdk-dev] Clarification for - SoC specific driver based common sub component placing

2016-11-18 Thread Thomas Monjalon
2016-11-18 14:38, Thomas Monjalon:
> 2016-11-18 12:44, Hemant Agrawal:
> > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) 
> > Poll mode drivers into the DPDK.
> > 
> > We need some clarification w.r.t the right placing of some dependent 
> > components, which can be common across drivers. E.g. We have hardware queue 
> > and buffer manager driver. This will be used by both network driver and 
> > crypto driver. But it is specific to NXP platform only.
> > 
> > What is the right place for such common hardware specific components in 
> > DPDK? 
> > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For 
> > each soc configuration only the required components will be compiled-in. 
> > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. 
> > And link the network and crypto drivers to it.
> > 3. Add it to main network driver and make the crypto driver dependent 
> > on it.
> 
> Your question is more generic than SoC context.
> You just want to share some code between drivers, right?
> What about building a library located in drivers/common/nxp/ ?

I'm a bit reluctant to have company name in file hierarchy,
as it not something stable. And especially for NXP/Qualcomm...

In this case would it be better to name the directory
drivers/common/dpaa2/ ?


[dpdk-dev] Clarification for - SoC specific driver based common sub component placing

2016-11-18 Thread Thomas Monjalon
2016-11-18 12:44, Hemant Agrawal:
> We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) 
> Poll mode drivers into the DPDK.
> 
> We need some clarification w.r.t the right placing of some dependent 
> components, which can be common across drivers. E.g. We have hardware queue 
> and buffer manager driver. This will be used by both network driver and 
> crypto driver. But it is specific to NXP platform only.
> 
> What is the right place for such common hardware specific components in DPDK? 
>   1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For 
> each soc configuration only the required components will be compiled-in. 
>   2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. 
> And link the network and crypto drivers to it.
>   3. Add it to main network driver and make the crypto driver dependent 
> on it.

Your question is more generic than SoC context.
You just want to share some code between drivers, right?
What about building a library located in drivers/common/nxp/ ?


[dpdk-dev] Clarification for - SoC specific driver based common sub component placing

2016-11-18 Thread Hemant Agrawal


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Friday, November 18, 2016 7:13 PM
> To: Hemant Agrawal 
> Cc: dev at dpdk.org
> Subject: Re: Clarification for - SoC specific driver based common sub 
> component
> placing
> 
> 2016-11-18 14:38, Thomas Monjalon:
> > 2016-11-18 12:44, Hemant Agrawal:
> > > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2)
> Poll mode drivers into the DPDK.
> > >
> > > We need some clarification w.r.t the right placing of some dependent
> components, which can be common across drivers. E.g. We have hardware
> queue and buffer manager driver. This will be used by both network driver and
> crypto driver. But it is specific to NXP platform only.
> > >
> > > What is the right place for such common hardware specific components in
> DPDK?
> > >   1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For 
> > > each
> soc configuration only the required components will be compiled-in.
> > >   2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs.
> And link the network and crypto drivers to it.
> > >   3. Add it to main network driver and make the crypto driver dependent
> on it.
> >
> > Your question is more generic than SoC context.
> > You just want to share some code between drivers, right?
> > What about building a library located in drivers/common/nxp/ ?
> 
> I'm a bit reluctant to have company name in file hierarchy, as it not 
> something
> stable. And especially for NXP/Qualcomm...
> 
> In this case would it be better to name the directory drivers/common/dpaa2/ ?

[Hemant] Sounds good. 

Thanks! 


[dpdk-dev] [PATCH v5] latencystats: added new library for latency stats

2016-11-18 Thread Remy Horton

On 15/11/2016 21:37, Reshma Pattan wrote:
[..]
> This pacth is dependent on http://dpdk.org/dev/patchwork/patch/16927/

There are merge issues, so will combine this patch into the v6 of the 
above patchset.

..Remy


[dpdk-dev] [PATCH] pmdinfogen: Fix pmdinfogen to select proper endianess on cross-compile

2016-11-18 Thread Neil Horman
pmdinfogen has a bug in which, during build, it pulls in rte_byteorder.h to
obtain the rte macros for byteswapping between the cpu byte order and big or
little endian.  Unfortunately, pmdinfogen is a tool that is only meant to be run
during the build of dpdk components, and so, it runs on the host.  In cross
compile environments however, the rte_byteorder.h is configured using a target
cpu, who's endianess may differ from that of the host, leading to improper
swapping.

The fix is to use host system defined byte swapping routines rather than the
dpdk provided routines.  Note that we are using non posix compliant routines, as
the posix compliant api only addresses 16 and 32 bit swaps, and we also need 64
bit swaps.  Those macros exist (via endian.h), but BSD and Linux put that header
in different locations so some ifdeffery is required.

Tested successfully by myself on Linux and BSD systems.

Signed-off-by: Neil Horman 
CC: Hemant Agrawal 
CC: Jerin Jacob 
CC: Bruce Richardson 
CC: Thomas Monjalon 
---
 buildtools/pmdinfogen/pmdinfogen.h | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/buildtools/pmdinfogen/pmdinfogen.h 
b/buildtools/pmdinfogen/pmdinfogen.h
index 1da2966..7c787c1 100644
--- a/buildtools/pmdinfogen/pmdinfogen.h
+++ b/buildtools/pmdinfogen/pmdinfogen.h
@@ -16,12 +16,16 @@
 #include 
 #include 
 #include 
+#ifdef __linux__ 
+#include 
+#else
+#include 
+#endif
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 

 /* On BSD-alike OSes elf.h defines these according to host's word size */
 #undef ELF_ST_BIND
@@ -75,9 +79,9 @@
 #define CONVERT_NATIVE(fend, width, x) ({ \
 typeof(x) ___x; \
 if ((fend) == ELFDATA2LSB) \
-   ___x = rte_le_to_cpu_##width(x); \
+   ___x = le##width##toh(x); \
 else \
-   ___x = rte_be_to_cpu_##width(x); \
+   ___x = be##width##toh(x); \
___x; \
 })

-- 
2.7.4



[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler

2016-11-18 Thread Neil Horman
On Fri, Nov 18, 2016 at 04:37:38PM +, Bruce Richardson wrote:
> On Fri, Nov 18, 2016 at 08:50:52AM -0500, Neil Horman wrote:
> > On Fri, Nov 18, 2016 at 12:03:19PM +, Hemant Agrawal wrote:
> > > > -Original Message-
> > > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote:
> > > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote:
> > > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote:
> > > > > > > > Hi Neil,
> > > > > > > >Pmdinfogen compiles with host compiler. It usages
> > > > rte_byteorder.h
> > > > > > of the target platform.
> > > > > > > > However, if the host compiler is older than 4.8, it will be an 
> > > > > > > > issue during
> > > > cross
> > > > > > compilation for some platforms.
> > > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler 
> > > > > > > > will not
> > > > > > understand the arm asm instructions.
> > > > > > > >
> > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if
> > > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static
> > > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > > > >register uint16_t x = _x;
> > > > > > > >asm volatile ("rev16 %0,%1"
> > > > > > > > : "=r" (x)
> > > > > > > > : "r" (x)
> > > > > > > > );
> > > > > > > >return x;
> > > > > > > > }
> > > > > > > > #endif
> > > > > > > >
> > > > > > > > One easy solution is that we add compiler platform check in this
> > > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || 
> > > > > > > > defined
> > > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t 
> > > > > > > > _x) {
> > > > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else 
> > > > > > > > ?.
> > > > > > > >
> > > > > > > > Is there a better way to fix it?
> > > > > > >
> > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is 
> > > > > > > using
> > > > > > > the dpdk service then it should compile and link against HOST
> > > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I
> > > > > > > think, introducing the HOSTTARGET kind of scheme is a clean 
> > > > > > > solution.
> > > > > > >
> > > > > > > /Jerin
> > > > > > >
> > > > > > >
> > > > > > That would be accurate.  That is to say, pmdinfogen is a tool that 
> > > > > > should only
> > > > be
> > > > > > run on the host doing the build, by the host doing the build, and 
> > > > > > so should be
> > > > > > compiled to run on the host, not on the target being built for.
> > > > > >
> > > > > > Yeah, so what we need is a way to get to the host version of 
> > > > > > rte_byteorder.h
> > > > > > when building in a cross environment
> > > > > >
> > > > > +1
> > > > >
> > > > > > Neil
> > > > >
> > > > 
> > > > Give this a try, I've tested it on linux, but not BSD.  From what I 
> > > > read the
> > > > functions are not posix compliant, though they should exist on all BSD 
> > > > and Linux
> > > > systems in recent history.  There may be some fiddling needed for Net 
> > > > and
> > > > OpenBSD variants, but I think this is the right general direction.
> > > 
> > > + 1
> > > This patch works good for Linux. 
> > > 
> > Can someone test it on BSD?  I'd like to ensure we don't need to modify it 
> > for
> > that platform
> > 
> > Neil
> > 
> > > > 
> > > > 
> > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h
> > > > b/buildtools/pmdinfogen/pmdinfogen.h
> > > > index 1da2966..c5ef89d 100644
> > > > --- a/buildtools/pmdinfogen/pmdinfogen.h
> > > > +++ b/buildtools/pmdinfogen/pmdinfogen.h
> > > > @@ -21,7 +21,6 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > -#include 
> > > > 
> > > >  /* On BSD-alike OSes elf.h defines these according to host's word size 
> > > > */
> > > >  #undef ELF_ST_BIND
> > > > @@ -75,9 +74,9 @@
> > > >  #define CONVERT_NATIVE(fend, width, x) ({ \
> > > >  typeof(x) ___x; \
> > > >  if ((fend) == ELFDATA2LSB) \
> > > > -   ___x = rte_le_to_cpu_##width(x); \
> > > > +   ___x = le##width##toh(x); \
> > > >  else \
> > > > -   ___x = rte_be_to_cpu_##width(x); \
> > > > +   ___x = be##width##toh(x); \
> > > > ___x; \
> > > >  })
> > > > 
> 
> For compile on FreeBSD 10 we need "#include " and this
> works.
> 
Yeah, but that will break linux, because there endian.h is in the base include
directory.  I'll have to do some ifdeffing

Neil

> /Bruce
> 


[dpdk-dev] Proposal for a new Committer model

2016-11-18 Thread Neil Horman
On Thu, Nov 17, 2016 at 09:20:50AM +, Mcnamara, John wrote:
> Repost from the moving at dpdk.org mailing list to get a wider audience.
> Original thread: http://dpdk.org/ml/archives/moving/2016-November/59.html
> 
> 
> Hi,
> 
> I'd like to propose a change to the DPDK committer model. Currently we have 
> one committer for the master branch of the DPDK project. 
> 
> One committer to master represents a single point of failure and at times can 
> be inefficient. There is also no agreed cover for times when the committer is 
> unavailable such as vacation, public holidays, etc. I propose that we change 
> to a multi-committer model for the DPDK project. We should have three 
> committers for each release that can commit changes to the master branch.
>  
> There are a number of benefits:
>  
> 1. Greater capacity to commit patches.
> 2. No single points of failure - a committer should always be available if we 
> have three.
> 3. A more timely committing of patches. More committers should equal a faster 
> turnaround - ideally, maintainers should also provide feedback on patches 
> submitted within a 2-3 day period, as much as possible, to facilitate this. 
> 4. It follows best practice in creating a successful multi-vendor community - 
> to achieve this we must ensure there is a level playing field for all 
> participants, no single person should be required to make all of the 
> decisions on patches to be included in the release.  
> 
> Having multiple committers will require some degree of co-ordination but 
> there are a number of other communities successfully following this model 
> such as Apache, OVS, FD.io, OpenStack etc. so the approach is workable.
> 
> John

I agree that the problems you are attempting to address exist and are
worth finding a solution for.  That said, I don't think the solution you
are proposing is the ideal, or complete fix for any of the issues being
addressed.

If I may, I'd like to ennumerate the issues I think you are trying to
address based on your comments above, then make a counter-proposal for a
solution:

Problems to address:

1) high-availability - There is a desire to make sure that, when patches
are proposed, they are integrated in a timely fashion.

2) high-throughput - DPDK has a large volume of patches, more than one
person can normally integrate.  There is a desire to shard that work such
that it is handled by multiple individuals

3) Multi-Vendor fairness - There is a desire for multiple vendors to feel
as though the project tree maintainer isn't biased toward any individual
vendor.

To solve these I would propose the following solution (which is simmilar
to, but not quite identical, to yours).

A) Further promote subtree maintainership.  This was a conversation that I
proposed some time ago, but my proposed granularity was discarded in favor
of something that hasn't worked as well (in my opinion).  That is to say a
few driver pmds (i40e and fm10k come to mind) have their own tree that
send pull requests to Thomas.  We should be sharding that at a much higher
granularity and using it much more consistently.  That is to say, that we
should have a maintainer for all the ethernet pmds, and another for the
crypto pmds, another for the core eal layer, another for misc libraries
that have low patch volumes, etc.  Each of those subdivisions should have
their own list to communicate on, and each should have a tree that
integrates patches for their own subsystem, and they should on a regular
cycle send pull requests to Thomas.  Thomas in turn should by and large,
only be integrating pull requests.  This should address our high-
throughput issue, in that it will allow multiple maintainers to share the
workload, and integration should be relatively easy.

B) Designate alternates to serve as backups for the maintainer when they
are unavailable.  This provides high-availablility, and sounds very much
like your proposal, but in the interests of clarity, there is still a
single maintainer at any one time, it just may change to ensure the
continued merging of patches, if the primary maintainer isn't available.
Ideally however, those backup alternates arent needed, because most of the
primary maintainers work in merging pull requests, which are done based on
the trust of the submaintainer, and done during a very limited window of
time.  This also partially addreses multi-vendor fairness if your subtree
maintainers come from multiple participating companies.

Regards
Neil




[dpdk-dev] Clarification for - SoC specific driver based common sub component placing

2016-11-18 Thread Hemant Agrawal
Hi all,

We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) Poll 
mode drivers into the DPDK.

We need some clarification w.r.t the right placing of some dependent 
components, which can be common across drivers. E.g. We have hardware queue and 
buffer manager driver. This will be used by both network driver and crypto 
driver. But it is specific to NXP platform only.

What is the right place for such common hardware specific components in DPDK? 
1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For 
each soc configuration only the required components will be compiled-in. 
2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. 
And link the network and crypto drivers to it.
3. Add it to main network driver and make the crypto driver dependent 
on it.

Regards,
Hemant


[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler

2016-11-18 Thread Hemant Agrawal
> -Original Message-
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote:
> > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote:
> > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote:
> > > > > Hi Neil,
> > > > >Pmdinfogen compiles with host compiler. It usages
> rte_byteorder.h
> > > of the target platform.
> > > > > However, if the host compiler is older than 4.8, it will be an issue 
> > > > > during
> cross
> > > compilation for some platforms.
> > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will 
> > > > > not
> > > understand the arm asm instructions.
> > > > >
> > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if
> > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static
> > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > >register uint16_t x = _x;
> > > > >asm volatile ("rev16 %0,%1"
> > > > > : "=r" (x)
> > > > > : "r" (x)
> > > > > );
> > > > >return x;
> > > > > }
> > > > > #endif
> > > > >
> > > > > One easy solution is that we add compiler platform check in this
> > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined
> > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else ?.
> > > > >
> > > > > Is there a better way to fix it?
> > > >
> > > > IMO, It is a HOST build infrastructure issue. If a host app is using
> > > > the dpdk service then it should compile and link against HOST
> > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I
> > > > think, introducing the HOSTTARGET kind of scheme is a clean solution.
> > > >
> > > > /Jerin
> > > >
> > > >
> > > That would be accurate.  That is to say, pmdinfogen is a tool that should 
> > > only
> be
> > > run on the host doing the build, by the host doing the build, and so 
> > > should be
> > > compiled to run on the host, not on the target being built for.
> > >
> > > Yeah, so what we need is a way to get to the host version of 
> > > rte_byteorder.h
> > > when building in a cross environment
> > >
> > +1
> >
> > > Neil
> >
> 
> Give this a try, I've tested it on linux, but not BSD.  From what I read the
> functions are not posix compliant, though they should exist on all BSD and 
> Linux
> systems in recent history.  There may be some fiddling needed for Net and
> OpenBSD variants, but I think this is the right general direction.

+ 1
This patch works good for Linux. 

> 
> 
> diff --git a/buildtools/pmdinfogen/pmdinfogen.h
> b/buildtools/pmdinfogen/pmdinfogen.h
> index 1da2966..c5ef89d 100644
> --- a/buildtools/pmdinfogen/pmdinfogen.h
> +++ b/buildtools/pmdinfogen/pmdinfogen.h
> @@ -21,7 +21,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
> 
>  /* On BSD-alike OSes elf.h defines these according to host's word size */
>  #undef ELF_ST_BIND
> @@ -75,9 +74,9 @@
>  #define CONVERT_NATIVE(fend, width, x) ({ \
>  typeof(x) ___x; \
>  if ((fend) == ELFDATA2LSB) \
> - ___x = rte_le_to_cpu_##width(x); \
> + ___x = le##width##toh(x); \
>  else \
> - ___x = rte_be_to_cpu_##width(x); \
> + ___x = be##width##toh(x); \
>   ___x; \
>  })
> 


[dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API

2016-11-18 Thread Adrien Mazarguil
Hi Beilei,

On Fri, Nov 18, 2016 at 06:36:31AM +, Xing, Beilei wrote:
> Hi Adrien,
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Thursday, November 17, 2016 12:23 AM
> > To: dev at dpdk.org
> > Cc: Thomas Monjalon ; De Lara Guarch,
> > Pablo ; Olivier Matz
> > 
> > Subject: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
> > 
> > This new API supersedes all the legacy filter types described in 
> > rte_eth_ctrl.h.
> > It is slightly higher level and as a result relies more on PMDs to process 
> > and
> > validate flow rules.
> > 
> > Benefits:
> > 
> > - A unified API is easier to program for, applications do not have to be
> >   written for a specific filter type which may or may not be supported by
> >   the underlying device.
> > 
> > - The behavior of a flow rule is the same regardless of the underlying
> >   device, applications do not need to be aware of hardware quirks.
> > 
> > - Extensible by design, API/ABI breakage should rarely occur if at all.
> > 
> > - Documentation is self-standing, no need to look up elsewhere.
> > 
> > Existing filter types will be deprecated and removed in the near future.
> > 
> > Signed-off-by: Adrien Mazarguil 
> 
> 
> > +
> > +/**
> > + * Opaque type returned after successfully creating a flow.
> > + *
> > + * This handle can be used to manage and query the related flow (e.g.
> > +to
> > + * destroy it or retrieve counters).
> > + */
> > +struct rte_flow;
> > +
> 
> As we talked before, we use attr/pattern/actions to create and destroy a flow 
> in PMD, 
> but I don't think it's easy to clone the user-provided parameters and return 
> the result
> to the application as a rte_flow pointer.  As you suggested:
> /* PMD-specific code. */
>  struct rte_flow {
> struct rte_flow_attr attr;
> struct rte_flow_item *pattern;
> struct rte_flow_action *actions;
>  };

Just to provide some context to the community since the above snippet comes
from private exchanges, I've suggested the above structure as a mean to
create and remove rules in the same fashion as FDIR, by providing the rule
used for creation to the destroy callback.

As an opaque type, each PMD currently needs to implement its own version of
struct rte_flow. The above definition may ease transition from FDIR to
rte_flow for some PMDs, however they need to clone the entire
application-provided rule to do so because there is no requirement for it to
be kept allocated.

I've implemented such a function in testpmd (port_flow_new() in commit [1])
as an example.

 [1] http://dpdk.org/ml/archives/dev/2016-November/050266.html

However my suggestion is for PMDs to use their own HW-specific structure
that only contains relevant information instead of being forced to drag
large, non-native data around, missing useful context and that requires
parsing every time. This is one benefit of using an opaque type in the first
place, the other being ABI breakage avoidance.

> Because both pattern and actions are pointers, and there're also pointers in 
> structure
> rte_flow_item and struct rte_flow_action. We need to iterate allocation 
> during clone
> and iterate free during destroy, then seems that the code is something ugly, 
> right?

Well since I wrote that code, I won't easily admit it's ugly. I think PMDs
should not require the duplication of generic rules actually, which are only
defined as a common language between applications and PMDs. Both are free to
store rules in their own preferred and efficient format internally.

> I think application saves info when creating a flow rule, so why not 
> application provide
> attr/pattern/actions info to PMD before calling PMD API?

They have to do so temporarily (e.g. allocated on the stack) while calling
rte_flow_create() and rte_flow_validate(), that's it. Once a rule is
created, there's no requirement for applications to keep anything around.

For simple applications such as testpmd, the generic format is probably
enough. More complex and existing applications such as ovs-dpdk may rather
choose to keep using their internal format that already fits their needs,
partially duplicating this information in rte_flow_attr and
rte_flow_item/rte_flow_action lists would waste memory. The conversion in
this case should only be performed when creating/validating flow rules.

In short, I fail to see any downside with maintaining struct rte_flow opaque
to applications.

Best regards,

-- 
Adrien Mazarguil
6WIND


[dpdk-dev] [PATCH 4/4] app/test: unit test case for eventdev APIs

2016-11-18 Thread Jerin Jacob
This commit adds basic unit tests for the eventdev API.

commands to run the test app:
./build/app/test -c 2
RTE>>eventdev_common_autotest

Signed-off-by: Jerin Jacob 
---
 MAINTAINERS  |   1 +
 app/test/Makefile|   2 +
 app/test/test_eventdev.c | 776 +++
 3 files changed, 779 insertions(+)
 create mode 100644 app/test/test_eventdev.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c594a23..887f133 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -252,6 +252,7 @@ F: examples/l2fwd-crypto/
 Eventdev API - EXPERIMENTAL
 M: Jerin Jacob 
 F: lib/librte_eventdev/
+F: app/test/test_eventdev*
 F: drivers/event/skeleton/

 Networking Drivers
diff --git a/app/test/Makefile b/app/test/Makefile
index 5be023a..e28c079 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -197,6 +197,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += 
test_cryptodev_blockcipher.c
 SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev.c

+SRCS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += test_eventdev.c
+
 SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) += test_kvargs.c

 CFLAGS += -O3
diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
new file mode 100644
index 000..e876804
--- /dev/null
+++ b/app/test/test_eventdev.c
@@ -0,0 +1,776 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Cavium networks. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in
+ *the documentation and/or other materials provided with the
+ *distribution.
+ *  * Neither the name of Cavium networks nor the names of its
+ *contributors may be used to endorse or promote products derived
+ *from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "test.h"
+
+#define TEST_DEV_NAME EVENTDEV_NAME_SKELETON_PMD
+
+static inline uint8_t
+test_dev_id_get(void)
+{
+   return rte_event_dev_get_dev_id(RTE_STR(TEST_DEV_NAME)"_0");
+}
+
+static int
+testsuite_setup(void)
+{
+   return rte_eal_vdev_init(RTE_STR(TEST_DEV_NAME), NULL);
+}
+
+static void
+testsuite_teardown(void)
+{
+}
+
+static int
+test_eventdev_count(void)
+{
+   uint8_t count;
+   count = rte_event_dev_count();
+   TEST_ASSERT(count > 0, "Invalid eventdev count %" PRIu8, count);
+   return TEST_SUCCESS;
+}
+
+static int
+test_eventdev_get_dev_id(void)
+{
+   int ret;
+   ret = rte_event_dev_get_dev_id(RTE_STR(TEST_DEV_NAME)"_0");
+   TEST_ASSERT(ret >= 0, "Failed to get dev_id %d", ret);
+   ret = rte_event_dev_get_dev_id("not_a_valid_ethdev_driver");
+   TEST_ASSERT_FAIL(ret, "Expected <0 for invalid dev name ret=%d", ret);
+   return TEST_SUCCESS;
+}
+
+static int
+test_eventdev_socket_id(void)
+{
+   int ret, socket_id;
+   ret = rte_event_dev_get_dev_id(RTE_STR(TEST_DEV_NAME)"_0");
+   socket_id = rte_event_dev_socket_id(ret);
+   TEST_ASSERT(socket_id != -EINVAL, "Failed to get socket_id %d",
+   socket_id);
+   socket_id = rte_event_dev_socket_id(RTE_EVENT_MAX_DEVS);
+   TEST_ASSERT(socket_id == -EINVAL, "Expected -EINVAL %d", socket_id);
+
+   return TEST_SUCCESS;
+}
+
+static int
+test_eventdev_info_get(void)
+{
+   int ret;
+   struct rte_event_dev_info info;
+   ret = rte_event_dev_info_get(test_dev_id_get(), NULL);
+   TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret);
+   ret = rte_event_dev_info_get(test_dev_id_get(), );
+   TEST_ASSERT_SUCCESS(ret, "Failed to get event dev info");
+   TEST_ASSERT(info.max_event_ports > 0,
+   "Not enough event ports %d", 

[dpdk-dev] [PATCH 2/4] eventdev: implement the northbound APIs

2016-11-18 Thread Jerin Jacob
This patch set defines the southbound driver interface
and implements the common code required for northbound
eventdev API interface.

Signed-off-by: Jerin Jacob 
---
 config/common_base   |6 +
 lib/Makefile |1 +
 lib/librte_eal/common/include/rte_log.h  |1 +
 lib/librte_eventdev/Makefile |   57 ++
 lib/librte_eventdev/rte_eventdev.c   | 1211 ++
 lib/librte_eventdev/rte_eventdev_pmd.h   |  504 +++
 lib/librte_eventdev/rte_eventdev_version.map |   39 +
 mk/rte.app.mk|1 +
 8 files changed, 1820 insertions(+)
 create mode 100644 lib/librte_eventdev/Makefile
 create mode 100644 lib/librte_eventdev/rte_eventdev.c
 create mode 100644 lib/librte_eventdev/rte_eventdev_pmd.h
 create mode 100644 lib/librte_eventdev/rte_eventdev_version.map

diff --git a/config/common_base b/config/common_base
index 4bff83a..7a8814e 100644
--- a/config/common_base
+++ b/config/common_base
@@ -411,6 +411,12 @@ CONFIG_RTE_LIBRTE_PMD_ZUC_DEBUG=n
 CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO=y

 #
+# Compile generic event device library
+#
+CONFIG_RTE_LIBRTE_EVENTDEV=y
+CONFIG_RTE_LIBRTE_EVENTDEV_DEBUG=n
+CONFIG_RTE_EVENT_MAX_DEVS=16
+CONFIG_RTE_EVENT_MAX_QUEUES_PER_DEV=64
 # Compile librte_ring
 #
 CONFIG_RTE_LIBRTE_RING=y
diff --git a/lib/Makefile b/lib/Makefile
index 990f23a..1a067bf 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -41,6 +41,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += librte_cfgfile
 DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += librte_cmdline
 DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev
+DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += librte_eventdev
 DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += librte_vhost
 DIRS-$(CONFIG_RTE_LIBRTE_HASH) += librte_hash
 DIRS-$(CONFIG_RTE_LIBRTE_LPM) += librte_lpm
diff --git a/lib/librte_eal/common/include/rte_log.h 
b/lib/librte_eal/common/include/rte_log.h
index 29f7d19..9a07d92 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -79,6 +79,7 @@ extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_PIPELINE 0x8000 /**< Log related to pipeline. */
 #define RTE_LOGTYPE_MBUF0x0001 /**< Log related to mbuf. */
 #define RTE_LOGTYPE_CRYPTODEV 0x0002 /**< Log related to cryptodev. */
+#define RTE_LOGTYPE_EVENTDEV 0x0004 /**< Log related to eventdev. */

 /* these log types can be used in an application */
 #define RTE_LOGTYPE_USER1   0x0100 /**< User-defined log type 1. */
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
new file mode 100644
index 000..dac0663
--- /dev/null
+++ b/lib/librte_eventdev/Makefile
@@ -0,0 +1,57 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2016 Cavium networks. All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+# * Redistributions of source code must retain the above copyright
+#   notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+#   notice, this list of conditions and the following disclaimer in
+#   the documentation and/or other materials provided with the
+#   distribution.
+# * Neither the name of Cavium networks nor the names of its
+#   contributors may be used to endorse or promote products derived
+#   from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_eventdev.a
+
+# library version
+LIBABIVER := 1
+
+# build flags
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+# library source files
+SRCS-y += rte_eventdev.c
+
+# export include files
+SYMLINK-y-include += rte_eventdev.h
+SYMLINK-y-include += rte_eventdev_pmd.h
+
+# versioning export map
+EXPORT_MAP := rte_eventdev_version.map
+
+# library dependencies
+DEPDIRS-y += lib/librte_eal
+DEPDIRS-y += lib/librte_mbuf
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_eventdev/rte_eventdev.c 

[dpdk-dev] [PATCH 1/4] eventdev: introduce event driven programming model

2016-11-18 Thread Jerin Jacob
In a polling model, lcores poll ethdev ports and associated
rx queues directly to look for packet. In an event driven model,
by contrast, lcores call the scheduler that selects packets for
them based on programmer-specified criteria. Eventdev library
adds support for event driven programming model, which offer
applications automatic multicore scaling, dynamic load balancing,
pipelining, packet ingress order maintenance and
synchronization services to simplify application packet processing.

By introducing event driven programming model, DPDK can support
both polling and event driven programming models for packet processing,
and applications are free to choose whatever model
(or combination of the two) that best suits their needs.

Signed-off-by: Jerin Jacob 
---
 MAINTAINERS|3 +
 doc/api/doxy-api-index.md  |1 +
 doc/api/doxy-api.conf  |1 +
 lib/librte_eventdev/rte_eventdev.h | 1439 
 4 files changed, 1444 insertions(+)
 create mode 100644 lib/librte_eventdev/rte_eventdev.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d6bb8f8..e430ca7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -249,6 +249,9 @@ F: lib/librte_cryptodev/
 F: app/test/test_cryptodev*
 F: examples/l2fwd-crypto/

+Eventdev API - EXPERIMENTAL
+M: Jerin Jacob 
+F: lib/librte_eventdev/

 Networking Drivers
 --
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 6675f96..28c1329 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -40,6 +40,7 @@ There are many libraries, so their headers may be grouped by 
topics:
   [ethdev] (@ref rte_ethdev.h),
   [ethctrl](@ref rte_eth_ctrl.h),
   [cryptodev]  (@ref rte_cryptodev.h),
+  [eventdev]   (@ref rte_eventdev.h),
   [devargs](@ref rte_devargs.h),
   [bond]   (@ref rte_eth_bond.h),
   [vhost]  (@ref rte_virtio_net.h),
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index 9dc7ae5..9841477 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -41,6 +41,7 @@ INPUT   = doc/api/doxy-api-index.md \
   lib/librte_cryptodev \
   lib/librte_distributor \
   lib/librte_ether \
+  lib/librte_eventdev \
   lib/librte_hash \
   lib/librte_ip_frag \
   lib/librte_jobstats \
diff --git a/lib/librte_eventdev/rte_eventdev.h 
b/lib/librte_eventdev/rte_eventdev.h
new file mode 100644
index 000..778d6dc
--- /dev/null
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -0,0 +1,1439 @@
+/*
+ *   BSD LICENSE
+ *
+ *   Copyright 2016 Cavium.
+ *   Copyright 2016 Intel Corporation.
+ *   Copyright 2016 NXP.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ * * Neither the name of Cavium nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_EVENTDEV_H_
+#define _RTE_EVENTDEV_H_
+
+/**
+ * @file
+ *
+ * RTE Event Device API
+ *
+ * In a polling model, lcores poll ethdev ports and associated rx queues
+ * directly to look for packet. In an event driven model, by contrast, lcores
+ * call the scheduler that selects packets for them based on programmer
+ * specified criteria. Eventdev library adds support for event driven
+ * programming model, which offer applications automatic multicore scaling,
+ * dynamic load balancing, pipelining, packet ingress order 

[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation

2016-11-18 Thread Jerin Jacob
As previously discussed in RFC v1 [1], RFC v2 [2], with changes
described in [3] (also pasted below), here is the first non-draft series
for this new API.

[1] http://dpdk.org/ml/archives/dev/2016-August/045181.html
[2] http://dpdk.org/ml/archives/dev/2016-October/048592.html
[3] http://dpdk.org/ml/archives/dev/2016-October/048196.html

Changes since RFC v2:

- Updated the documentation to define the need for this library[Jerin]
- Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in
  struct rte_event_queue_conf to enable optimized sw implementation [Bruce]
- Introduced RTE_EVENT_OP* ops [Bruce]
- Added nb_event_queue_flows,nb_event_port_dequeue_depth, 
nb_event_port_enqueue_depth
  in rte_event_dev_configure() like ethdev and crypto library[Jerin]
- Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to
  reduce fast path APIs and it is redundant too[Jerin]
- In the view of better application portability, Removed pin_event
  from rte_event_enqueue as it is just hint and Intel/NXP can not support 
it[Jerin]
- Added rte_event_port_links_get()[Jerin]
- Added rte_event_dev_dump[Harry]

Notes:

- This patch set is check-patch clean with an exception that
02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL
- Looking forward to getting additional maintainers for libeventdev


Possible next steps:
1) Review this patch set
2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/]
3) Review proposed examples/eventdev_pipeline 
application[http://dpdk.org/dev/patchwork/patch/17053/]
4) Review proposed functional tests[http://dpdk.org/dev/patchwork/patch/17051/]
5) Cavium's HW based eventdev driver

I am planning to work on (3),(4) and (5)

TODO:
1) Example applications for pipelining, packet ingress order maintenance with
ORDERED type and ATOMIC synchronization services.
2) Create user guide


Jerin Jacob (4):
  eventdev: introduce event driven programming model
  eventdev: implement the northbound APIs
  event/skeleton: add skeleton eventdev driver
  app/test: unit test case for eventdev APIs

 MAINTAINERS|5 +
 app/test/Makefile  |2 +
 app/test/test_eventdev.c   |  776 +++
 config/common_base |   14 +
 doc/api/doxy-api-index.md  |1 +
 doc/api/doxy-api.conf  |1 +
 drivers/Makefile   |1 +
 drivers/event/Makefile |   36 +
 drivers/event/skeleton/Makefile|   55 +
 .../skeleton/rte_pmd_skeleton_event_version.map|4 +
 drivers/event/skeleton/skeleton_eventdev.c |  535 
 drivers/event/skeleton/skeleton_eventdev.h |   72 +
 lib/Makefile   |1 +
 lib/librte_eal/common/include/rte_log.h|1 +
 lib/librte_eventdev/Makefile   |   57 +
 lib/librte_eventdev/rte_eventdev.c | 1211 
 lib/librte_eventdev/rte_eventdev.h | 1439 
 lib/librte_eventdev/rte_eventdev_pmd.h |  504 +++
 lib/librte_eventdev/rte_eventdev_version.map   |   39 +
 mk/rte.app.mk  |5 +
 20 files changed, 4759 insertions(+)
 create mode 100644 app/test/test_eventdev.c
 create mode 100644 drivers/event/Makefile
 create mode 100644 drivers/event/skeleton/Makefile
 create mode 100644 drivers/event/skeleton/rte_pmd_skeleton_event_version.map
 create mode 100644 drivers/event/skeleton/skeleton_eventdev.c
 create mode 100644 drivers/event/skeleton/skeleton_eventdev.h
 create mode 100644 lib/librte_eventdev/Makefile
 create mode 100644 lib/librte_eventdev/rte_eventdev.c
 create mode 100644 lib/librte_eventdev/rte_eventdev.h
 create mode 100644 lib/librte_eventdev/rte_eventdev_pmd.h
 create mode 100644 lib/librte_eventdev/rte_eventdev_version.map

-- 
2.5.5



[dpdk-dev] [PATCH 3/3] net/mlx5: do not invalidate title CQE

2016-11-18 Thread Ferruh Yigit
On 11/17/2016 10:38 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:56AM +0100, Nelio Laranjeiro wrote:
>> We can leave the title completion queue entry untouched since its contents
>> are not modified.
>>
>> Reported-by: Liming Sun 
>> Signed-off-by: Nelio Laranjeiro 
<...>
> Acked-by: Adrien Mazarguil 

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



[dpdk-dev] [dpdk-stable] [PATCH 2/3] net/mlx5: fix wrong htons

2016-11-18 Thread Ferruh Yigit
On 11/17/2016 10:38 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:55AM +0100, Nelio Laranjeiro wrote:
>> Completion queue entry data uses network endian, to access them we should use
>> ntoh*().
>>
>> Fixes: c305090bbaf8 ("net/mlx5: replace countdown with threshold for Tx 
>> completions")
>>
>> CC: stable at dpdk.org
>> Reported-by: Liming Sun 
>> Signed-off-by: Nelio Laranjeiro 
<...>
> Acked-by: Adrien Mazarguil 

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



[dpdk-dev] [dpdk-stable] [PATCH 1/3] net/mlx5: fix leak when starvation occurs

2016-11-18 Thread Ferruh Yigit
On 11/17/2016 10:37 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:54AM +0100, Nelio Laranjeiro wrote:
>> The list of segments to free was wrongly manipulated ending by only freeing
>> the first segment instead of freeing all of them.  The last one still
>> belongs to the NIC and thus should not be freed.
>>
>> Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx")
>>
>> CC: stable at dpdk.org
>> Reported-by: Liming Sun 
>> Signed-off-by: Nelio Laranjeiro 
<...>
> Acked-by: Adrien Mazarguil 

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



[dpdk-dev] [PATCH 5/5] ethtool: dispaly bus info and firmware version

2016-11-18 Thread Remy Horton

On 17/11/2016 17:42, Qiming Yang wrote:
> This patch enhances the ethtool-API to support to show the
> bus information and firmware version as same as kernel
> version ethtool displayed.

Suggest rewording as:

This patch enhances the ethtool example to support to show
bus information and firmware version, in the same way that
the Linux kernel ethtool does.


[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get

2016-11-18 Thread Remy Horton

On 17/11/2016 17:42, Qiming Yang wrote:
> This patch added API for 'rte_eth_dev_fwver_get'
>
> void rte_eth_dev_fwver_get(uint8_t port_id,
> char *fw_version, int fw_length);

Suggest description such as:

This patch adds ethdev API for fetching firmware version.

Also see Thomas's comments. Looks like some stale patches got picked up 
by 'git send-mail *.patch'..

..Remy


[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler

2016-11-18 Thread Neil Horman
On Fri, Nov 18, 2016 at 12:03:19PM +, Hemant Agrawal wrote:
> > -Original Message-
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote:
> > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote:
> > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote:
> > > > > > Hi Neil,
> > > > > >Pmdinfogen compiles with host compiler. It usages
> > rte_byteorder.h
> > > > of the target platform.
> > > > > > However, if the host compiler is older than 4.8, it will be an 
> > > > > > issue during
> > cross
> > > > compilation for some platforms.
> > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler 
> > > > > > will not
> > > > understand the arm asm instructions.
> > > > > >
> > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if
> > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static
> > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > >register uint16_t x = _x;
> > > > > >asm volatile ("rev16 %0,%1"
> > > > > > : "=r" (x)
> > > > > > : "r" (x)
> > > > > > );
> > > > > >return x;
> > > > > > }
> > > > > > #endif
> > > > > >
> > > > > > One easy solution is that we add compiler platform check in this
> > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined
> > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) {
> > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else ?.
> > > > > >
> > > > > > Is there a better way to fix it?
> > > > >
> > > > > IMO, It is a HOST build infrastructure issue. If a host app is using
> > > > > the dpdk service then it should compile and link against HOST
> > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I
> > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution.
> > > > >
> > > > > /Jerin
> > > > >
> > > > >
> > > > That would be accurate.  That is to say, pmdinfogen is a tool that 
> > > > should only
> > be
> > > > run on the host doing the build, by the host doing the build, and so 
> > > > should be
> > > > compiled to run on the host, not on the target being built for.
> > > >
> > > > Yeah, so what we need is a way to get to the host version of 
> > > > rte_byteorder.h
> > > > when building in a cross environment
> > > >
> > > +1
> > >
> > > > Neil
> > >
> > 
> > Give this a try, I've tested it on linux, but not BSD.  From what I read the
> > functions are not posix compliant, though they should exist on all BSD and 
> > Linux
> > systems in recent history.  There may be some fiddling needed for Net and
> > OpenBSD variants, but I think this is the right general direction.
> 
> + 1
> This patch works good for Linux. 
> 
Can someone test it on BSD?  I'd like to ensure we don't need to modify it for
that platform

Neil

> > 
> > 
> > diff --git a/buildtools/pmdinfogen/pmdinfogen.h
> > b/buildtools/pmdinfogen/pmdinfogen.h
> > index 1da2966..c5ef89d 100644
> > --- a/buildtools/pmdinfogen/pmdinfogen.h
> > +++ b/buildtools/pmdinfogen/pmdinfogen.h
> > @@ -21,7 +21,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> > 
> >  /* On BSD-alike OSes elf.h defines these according to host's word size */
> >  #undef ELF_ST_BIND
> > @@ -75,9 +74,9 @@
> >  #define CONVERT_NATIVE(fend, width, x) ({ \
> >  typeof(x) ___x; \
> >  if ((fend) == ELFDATA2LSB) \
> > -   ___x = rte_le_to_cpu_##width(x); \
> > +   ___x = le##width##toh(x); \
> >  else \
> > -   ___x = rte_be_to_cpu_##width(x); \
> > +   ___x = be##width##toh(x); \
> > ___x; \
> >  })
> > 


[dpdk-dev] [PATCH v2 1/1] mempool: Add sanity check when secondary link in less mempools than primary

2016-11-18 Thread Jean Tourrilhes
On Fri, Nov 18, 2016 at 04:11:12PM +0100, Olivier Matz wrote:
> Hi Jean,
> 
> 
> Do you mind if we put back this conversation on the ML?

Oh, I forgot to do it ? I intended to. Bummer. Please do so.

> I think your example shows that there is no linker magic: you just
> need the same linker flags for dpdk libraries than in the dpdk
> framework. I suppose we need something in the build framework
> to provide these flags externally,

Good luck integrating that in all foreign build system (I'm
looking at you, Snort).

> but I don't think we need to patch mempool for that.

This sanity check is just that, a sanity check. I don't
understand what's bad about a sanity check, it does not change
functionality, it does not fix anything and just warn users about
those issues.
Please look at the patch itself at face value.

> Regards,
> Olivier

Regards,

Jean


[dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API

2016-11-18 Thread Xing, Beilei
Hi Adrien,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Thursday, November 17, 2016 12:23 AM
> To: dev at dpdk.org
> Cc: Thomas Monjalon ; De Lara Guarch,
> Pablo ; Olivier Matz
> 
> Subject: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
> 
> This new API supersedes all the legacy filter types described in 
> rte_eth_ctrl.h.
> It is slightly higher level and as a result relies more on PMDs to process and
> validate flow rules.
> 
> Benefits:
> 
> - A unified API is easier to program for, applications do not have to be
>   written for a specific filter type which may or may not be supported by
>   the underlying device.
> 
> - The behavior of a flow rule is the same regardless of the underlying
>   device, applications do not need to be aware of hardware quirks.
> 
> - Extensible by design, API/ABI breakage should rarely occur if at all.
> 
> - Documentation is self-standing, no need to look up elsewhere.
> 
> Existing filter types will be deprecated and removed in the near future.
> 
> Signed-off-by: Adrien Mazarguil 


> +
> +/**
> + * Opaque type returned after successfully creating a flow.
> + *
> + * This handle can be used to manage and query the related flow (e.g.
> +to
> + * destroy it or retrieve counters).
> + */
> +struct rte_flow;
> +

As we talked before, we use attr/pattern/actions to create and destroy a flow 
in PMD, 
but I don't think it's easy to clone the user-provided parameters and return 
the result
to the application as a rte_flow pointer.  As you suggested:
/* PMD-specific code. */
 struct rte_flow {
struct rte_flow_attr attr;
struct rte_flow_item *pattern;
struct rte_flow_action *actions;
 };

Because both pattern and actions are pointers, and there're also pointers in 
structure
rte_flow_item and struct rte_flow_action. We need to iterate allocation during 
clone
and iterate free during destroy, then seems that the code is something ugly, 
right?

I think application saves info when creating a flow rule, so why not 
application provide
attr/pattern/actions info to PMD before calling PMD API?

Thanks,
Beilei Xing


[dpdk-dev] [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX

2016-11-18 Thread Satha Rao
i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
   results. To solve this include rte memory barriers

Signed-off-by: Satha Rao 
---
 drivers/net/i40e/base/i40e_osdep.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/i40e/base/i40e_osdep.h 
b/drivers/net/i40e/base/i40e_osdep.h
index 38e7ba5..ffa3160 100644
--- a/drivers/net/i40e/base/i40e_osdep.h
+++ b/drivers/net/i40e/base/i40e_osdep.h
@@ -158,7 +158,13 @@ do {   
 \
((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
 static inline uint32_t i40e_read_addr(volatile void *addr)
 {
+#if defined(RTE_ARCH_ARM64)
+   uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr));
+   rte_rmb();
+   return val;
+#else
return rte_le_to_cpu_32(I40E_PCI_REG(addr));
+#endif
 }
 #define I40E_PCI_REG_WRITE(reg, value) \
do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
@@ -171,8 +177,16 @@ static inline uint32_t i40e_read_addr(volatile void *addr)
I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((hw), (reg)), (value))

 #define rd32(a, reg) i40e_read_addr(I40E_PCI_REG_ADDR((a), (reg)))
+#if defined(RTE_ARCH_ARM64)
+#define wr32(a, reg, value) \
+   do { \
+   I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)); \
+   rte_wmb(); \
+   } while (0)
+#else
 #define wr32(a, reg, value) \
I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value))
+#endif
 #define flush(a) i40e_read_addr(I40E_PCI_REG_ADDR((a), (I40E_GLGEN_STAT)))

 #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
-- 
2.7.4



[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get

2016-11-18 Thread Yang, Qiming
Yes, that's my fault. I have set their statuses to 'suspended'

Qiming

-Original Message-
From: Horton, Remy 
Sent: Friday, November 18, 2016 9:10 AM
To: Yang, Qiming ; dev at dpdk.org
Cc: Wu, Jingjing ; Chen, Jing D 
Subject: Re: [PATCH 1/5] ethdev: add firmware version get


On 17/11/2016 17:42, Qiming Yang wrote:
> This patch added API for 'rte_eth_dev_fwver_get'
>
> void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int 
> fw_length);

Suggest description such as:

This patch adds ethdev API for fetching firmware version.

Also see Thomas's comments. Looks like some stale patches got picked up by 'git 
send-mail *.patch'..

..Remy


[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get

2016-11-18 Thread Yang, Qiming
Hi, Thomas

-Original Message-
From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] 
Sent: Thursday, November 17, 2016 9:37 PM
To: Yang, Qiming 
Cc: dev at dpdk.org; Horton, Remy ; Wu, Jingjing 
; Chen, Jing D 
Subject: Re: [dpdk-dev] [PATCH 1/5] ethdev: add firmware version get

2016-11-17 17:42, Qiming Yang:
> This patch added API for 'rte_eth_dev_fwver_get'
> 
> void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int 
> fw_length);

Copying some code here doesn't help really help.
Could you describe what we can expect in this string?
How can we compare this version number across different devices of the same 
driver?
How does it apply to FPGA devices?
What is the potential use?

Qiming: I'll add more describes in the next version.

[...]
>  /**
> + * Retrieve the firmware version of an Ethernet device.

We should stop talking about Ethernet device.
Networking device is more appropriate.
And in this case, "device" is enough.

Qiming: Got it.

> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param fw_version
> + *   A pointer the firmware version of an Ethernet device
> + * @param fw_length
> + *   The size of the firmware version, which should be large enough to store
> + *   the firmware version of the device.

How do we know that the length is too small?

Qiming: The length is a number large enough to store the firmware version. 
Original I use a fixed number 20, but in order to be more flexible, I replace 
it into a variable. 

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

Why not returning some errors?

Qiming: It's a good advice.

You forgot to remove the deprecation notice in this patch.

PS: the series is broken as some patches are not numbered and this one is not 
the first one. Please use a cover-letter to introduce such series.

Qiming: It's my fault. Thank you for your reminder, I will pay attention next 
time.