[dpdk-dev] [PATCH v1 1/1] examples/quota_watermark: fix low_watermark variable placement

2016-06-08 Thread Thomas Monjalon
2016-05-23 14:19, Piotr Azarewicz:
> qw app at its init stage reserve 2*sizeof(int) memory space for quota
> and low_watermark shared variables, but both apps (qw and qwctl) assign
> wrong address for low_watermark pointer (out of reserved memzone space)
> due to wrong pointer arithmetic.
> 
> CID 30709 : Extra sizeof expression (SIZEOF_MISMATCH)
> suspicious_pointer_arithmetic: Adding 4UL /* sizeof (int) */ to pointer
> (unsigned int *)(*qw_memzone).addr of type unsigned int * is suspicious
> because adding an integral value to this pointer automatically scales
> that value by the size, 4 bytes, of the pointed-to type, unsigned int.
> Most likely, sizeof (int) is extraneous and should be replaced with 1.
> 
> Coverity issue: 30709
> Fixes: 1d6c3ee3321a ("examples/quota_watermark: initial import")
> 
> Signed-off-by: Piotr Azarewicz 

Applied, thanks

Is this example really useful?


[dpdk-dev] [PATCH v2] examples/performance-thread: fix logic in get_dst_port of l3fwd-thread

2016-06-08 Thread Thomas Monjalon
2016-04-29 18:30, Tomasz Kulasek:
> This fixes wrong logic in get_dst_port() on lpm path causing unpredictable
> return value when ipv4/ipv6 lookup success (return with no value).
> 
> Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> 
> Signed-off-by: Tomasz Kulasek 

Applied, thanks


[dpdk-dev] [PATCH v2] examples/performance-thread: fix size of destination port ids in l3fwd-thread

2016-06-08 Thread Thomas Monjalon
2016-04-29 18:30, Tomasz Kulasek:
> After extending IPv4 next hop in lpm library, size of dst_port array was
> changed from 16 to 32 bits in l3fwd-thread example, without modification
> of the rest of path written for 16 bit value.
> 
> This patch uses similar approach for fix, like in commit 8353a36a9b4b
> ("examples/l3fwd: fix size of destination port ids"), restoring 16 bit size
> for destination port ids and doing necessary conversion from 32 to 16 bit
> after lpm_lookupx4.
> 
> Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> 
> Signed-off-by: Tomasz Kulasek 

Applied, thanks


[dpdk-dev] [PATCH] examples/l3fwd: report error when no vector engine is available

2016-06-08 Thread Thomas Monjalon
2016-04-26 13:30, Jan Viktorin:
> If no SSE nor NEON are available the l3fwd should complain loudly to quickly
> find out the reason.
> 
> Signed-off-by: Jan Viktorin 

Applied, thanks


[dpdk-dev] [PATCH] examples/l2fwd-jobstats: Fix a typo.

2016-06-08 Thread Thomas Monjalon
2016-04-21 21:16, Rami Rosen:
> Signed-off-by: Rami Rosen 

Applied, thanks


[dpdk-dev] [PATCH v2 0/6] ip_pipeline: routing pipeline improvements

2016-06-08 Thread Thomas Monjalon
2016-06-01 15:00, Jasvinder Singh:
> This commit adds following features to the routing pipeline;
> 
> 1. Implements the tracking mechanism for the routing pipeline for identifying
> the physical nic port where a specific output ports of the routing pipeline 
> are
> eventually connected. Depending upon the application, tracking could involve
> traversing the other intermediate pipelines.
> 
> The pipelines such as pass-through allows tracking to be performaned through
> them becasue of straightforward connections between their input and output
> ports, while pipelines like flow-classifications, firewall fails the tracking
> because of dependency upon the table rule set. As a result of tracking
> mechainsm, routing pipeline uses the real MAC addresses of the network
> interfaces instead of hardcoded default value.
> 
> 2. Adds support for automatic route automatic update when physical NIC ports
> change their state (up/down) or configuration. Every time a physical port
> goes up/down, a call-back function that the specific pipeline type
> (e.g. routing) registered with NIC ports at init time; will simply add/delete
> a route associated with that output port.
> 
> Acked-by: Cristian Dumitrescu 
> 
> Jasvinder Singh (6):
>   ip_pipeline: increase macros values
>   ip_pipeline: linking routing pipeline output ports with NIC ports
>   ip_pipeline: assign nic ports mac address to the routing pipeline
> outports
>   ip_pipeline: automatic routes update with the change in nic ports
> state
>   ip_pipeline: sample config file on adding various network layer
> components
>   ip_pipeline: update release notes

Applied, thanks


[dpdk-dev] [PATCH] mempool: fix local cache initialization

2016-06-08 Thread Olivier Matz
Hi Sergio,

Good catch, thanks. The patch looks ok, just few comments
on the commit log:

On 06/08/2016 05:10 PM, Sergio Gonzalez Monroy wrote:
> The mempool local cache is not being initialize properly leading to

'initialize' -> 'initialized' ?
and maybe 'is not being' -> 'was not' ?

> undefined behavior in cases where the allocated memory was used and left
> with data.
> 
> Fixes: af75078fece3 ("first public release")

I think it fixes this one instead:

213af31e0960 ("mempool: reduce structure size if no cache needed")

> 
> Signed-off-by: Sergio Gonzalez Monroy 
> ---
>  lib/librte_mempool/rte_mempool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c 
> b/lib/librte_mempool/rte_mempool.c
> index b54de43..216514c 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -787,7 +787,7 @@ rte_mempool_create_empty(const char *name, unsigned n, 
> unsigned elt_size,
>  
>   /* init the mempool structure */
>   mp = mz->addr;
> - memset(mp, 0, sizeof(*mp));
> + memset(mp, 0, MEMPOOL_HEADER_SIZE(mp, cache_size));
>   ret = snprintf(mp->name, sizeof(mp->name), "%s", name);
>   if (ret < 0 || ret >= (int)sizeof(mp->name)) {
>   rte_errno = ENAMETOOLONG;
> 


[dpdk-dev] [PATCH v3] ip_pipeline: add rss support

2016-06-08 Thread Thomas Monjalon
2016-05-30 18:14, Jasvinder Singh:
> This patch enables rss (receive side scaling) per network interface
> through the configuration file. The user can specify following
> parameters in LINK section for enabling the rss feature - rss_qs,
> rss_proto_ipv4, rss_proto_ipv6 and ip_proto_l2.
> 
> The "rss_qs" is mandatory parameter which indicates the queues to be
> used for rss, while rest of the parameters are optional. When optional
> parameters are not provided in the configuration file, default setting
> (ETH_RSS_IPV4 | ETH_RSS_IPV6) is assumed for "rss_hf" field of the
> rss_conf structure.
> 
> For example, following configuration can be applied for using the rss
> on port 0 of the network interface;
> 
> [PIPELINE0]
> type = MASTER
> core = 0
> 
> [LINK0]
> rss_qs = 0 1
> 
> [PIPELINE1]
> type = PASS-THROUGH
> core = 1
> pktq_in = RXQ0.0 RXQ0.1 RXQ1.0
> pktq_out = TXQ0.0 TXQ1.0 TXQ0.1
> 
> Signed-off-by: Jasvinder Singh 
> Acked-by: Cristian Dumitrescu 

Applied, thanks


[dpdk-dev] [PATCH v5] ip_pipeline: configuration file parser cleanup

2016-06-08 Thread Thomas Monjalon
2016-06-08 17:30, Jasvinder Singh:
> This commit adds following changes to configuration file parsing of
> the ip pipeline application;
> 
> 1. Parsing routines related to packet queues (pktq_in/out fields in the
> PIPELINE section) and message queues (msgq_in/out fields of in the MSGQ
> Section) are updated.
> 
> In the parsing routines, function "strtok_r()" is used for parsing the
> string instead of manually checking the string termination, white
> spaces, tabs etc., between the string tokens. Each call to strtok_r()
> returns a pointer to a null-terminated string containing the next token.
> If no more tokens are found, strtok_r() returns NULL. As a result of
> using strtok_r(), the code size of the parsing routines is reduced
> significantly.
> 
> 2. Replace PARSER_PARAM_ADD_CHECK macro by more specific macros such as
> PARSE_CHECK_DUPLICATE_SECTION, PARSE_CHECK_DUPLICATE_SECTION_EAL to detect
> duplicate entries in the various sections of the configuration file
> 
> 3. Add new macros PARSER_ERROR_NO_ELEMENTS and PARSE_ERROR_TOO_MANY_ELEMENTS
> for detecting no element and more elements than allowed situations
> respectively, in the section entry.
> 
> 4. Add new macros APP_PARAM_ADD_LINK_FOR_RXQ, APP_PARAM_ADD_LINK_FOR_TXQ
> and APP_PARAM_ADD_LINK_FOR_TM which add corresponding nic ports entry to
> the application param structure while parsing rx/tx queues, TM (Traffic
> Manager) port sections and pktq_in/out entries of pipeline sections
> 
> Signed-off-by: Jasvinder Singh 
> Acked-by: Cristian Dumitrescu 

Applied, thanks


[dpdk-dev] [PATCH] testpmd: fix typo when closing port

2016-06-08 Thread Thomas Monjalon
2016-06-08 17:18, Mcnamara, John:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > 2016-06-08 16:20, Pablo de Lara:
> > >   if (rte_atomic16_cmpset(&(port->port_status),
> > >   RTE_PORT_HANDLING, RTE_PORT_CLOSED) == 0)
> > > - printf("Port %d can not be set into stopped\n", pi);
> > > + printf("Port %d can not be set into closed\n", pi);
> > 
> > My english would say "Port cannot be closed".
> > Am I wrong?
> 
> Yes, that is better or "Port cannot be set to closed" if "closed" is a
> state like here.

Applied with John's proposal.


[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Jerin Jacob
On Wed, Jun 08, 2016 at 02:22:55PM +0100, Bruce Richardson wrote:
> On Wed, Jun 08, 2016 at 06:13:21PM +0530, Jerin Jacob wrote:
> > On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
> > > On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> > > > Jerin Jacob (20):
> > > >   thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC
> > > >   thunderx/nicvf: add pmd skeleton
> > > >   thunderx/nicvf: add link status and link update support
> > > >   thunderx/nicvf: add get_reg and get_reg_length support
> > > >   thunderx/nicvf: add dev_configure support
> > > >   thunderx/nicvf: add dev_infos_get support
> > > >   thunderx/nicvf: add rx_queue_setup/release support
> > > >   thunderx/nicvf: add tx_queue_setup/release support
> > > >   thunderx/nicvf: add rss and reta query and update support
> > > >   thunderx/nicvf: add mtu_set and promiscuous_enable support
> > > >   thunderx/nicvf: add stats support
> > > >   thunderx/nicvf: add single and multi segment tx functions
> > > >   thunderx/nicvf: add single and multi segment rx functions
> > > >   thunderx/nicvf: add dev_supported_ptypes_get and rx_queue_count
> > > > support
> > > >   thunderx/nicvf: add rx queue start and stop support
> > > >   thunderx/nicvf: add tx queue start and stop support
> > > >   thunderx/nicvf: add device start,stop and close support
> > > >   thunderx/config: set max numa node to two
> > > >   thunderx/nicvf: updated driver documentation and release notes
> > > >   maintainers: claim responsibility for the ThunderX nicvf PMD
> > > > 
> > > 
> > > Hi Jerin,
> > > 
> > > In patch subject, as tag, other drivers are using only driver name, and
> > > Intel drivers also has "driver/base", since base code has some special
> > > case. For thunderx, what do you think about keeping subject as:
> > >  "thunderx: "
> > > 
> > 
> > Hi Ferruh,
> > 
> > We may add crypto or other builtin ThunderX HW accelerated block drivers
> > in future to DPDK.
> > So that is the reason why I thought of keeping the subject as 
> > thunderx/nicvf.
> > If you don't have any objection then I would like to keep it as
> > thunderx/nicvf or just nicvf.
> 
> Are you upstreaming kernel modules for this device? If so, what is the Linux
> kernel module-name for this device going to be, as perhaps that can help us
> here?

Yes, Kernel module has been upstreamed.
the commit log in linux kernel is "net: thunderx: ..."

> 
> Regards,
> /Bruce


[dpdk-dev] [PATCH v3 18/20] thunderx/config: set max numa node to two

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> ---
>  config/defconfig_arm64-thunderx-linuxapp-gcc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/config/defconfig_arm64-thunderx-linuxapp-gcc 
> b/config/defconfig_arm64-thunderx-linuxapp-gcc
> index 7940bbd..cc12cee 100644
> --- a/config/defconfig_arm64-thunderx-linuxapp-gcc
> +++ b/config/defconfig_arm64-thunderx-linuxapp-gcc
> @@ -34,6 +34,7 @@
>  CONFIG_RTE_MACHINE="thunderx"
>  
>  CONFIG_RTE_CACHE_LINE_SIZE=128
> +CONFIG_RTE_MAX_NUMA_NODES=2
Isn't this platform level configuration? And sets max numa nodes
independent from driver, right?

Can you please add some more information why this is required?

Also does it make sense to separate this patch from driver patchset?

>  
>  #
>  # Compile Cavium Thunderx NICVF PMD driver
> 



[dpdk-dev] [PATCH v3 16/20] thunderx/nicvf: add tx queue start and stop support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 

...

> +static inline int
> +nicvf_start_tx_queue(struct rte_eth_dev *dev, uint16_t qidx)
> +{
> + struct nicvf_txq *txq;
> + int ret;
> +
> + if (dev->data->tx_queue_state[qidx] == 
> + RTE_ETH_QUEUE_STATE_STARTED)
Is line wrap required?

...
>  
>  static inline int
>  nicvf_configure_cpi(struct rte_eth_dev *dev)
> @@ -912,6 +960,24 @@ nicvf_dev_rx_queue_stop(struct rte_eth_dev *dev, 
> uint16_t qidx)
>  }
>  
>  static int
> +nicvf_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t qidx)
> +{
> + if (qidx >= nicvf_pmd_priv(dev)->eth_dev->data->nb_tx_queues)
> + return -EINVAL;
This check already done by librte_ether

> +
> + return nicvf_start_tx_queue(dev, qidx);
> +}
> +
> +static int
> +nicvf_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t qidx)
> +{
> + if (qidx >= nicvf_pmd_priv(dev)->eth_dev->data->nb_tx_queues)
> + return -EINVAL;
Same here

> +
> + return nicvf_stop_tx_queue(dev, qidx);
> +}
> +
...


[dpdk-dev] [PATCH v2] doc: fix code section in abi versioning doc

2016-06-08 Thread Thomas Monjalon
2016-05-20 15:08, John McNamara:
> Fix broken cosole directive in the ABI validator section of the
> ABI versioning docs.
> 
> Fixes: f1ef9794f9bd ("doc: add ABI guidelines")
> 
> Signed-off-by: John McNamara 

Applied, thanks


[dpdk-dev] [PATCH] doc/tespmd: fix flow director examples

2016-06-08 Thread Thomas Monjalon
2016-06-08 00:39, Lu, Wenzhuo:
> From: Mauricio Vasquez B [mailto:mauricio.vasquezbernal at studenti.polito.it]
> > A previous patch modified the CLIs witout updating the examples.
> > 
> > Fixes: 53b2bb9b7ea7 ("app/testpmd: new flow director commands")
> > 
> > Signed-off-by: Mauricio Vasquez B  > studenti.polito.it>
> Acked-by: Wenzhuo Lu 
> Thanks for handling this problem.

Applied, thanks


[dpdk-dev] [PATCH v3 15/20] thunderx/nicvf: add rx queue start and stop support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
>  drivers/net/thunderx/nicvf_ethdev.c | 175 
> 
>  drivers/net/thunderx/nicvf_rxtx.c   |  18 
>  drivers/net/thunderx/nicvf_rxtx.h   |   1 +
>  3 files changed, 194 insertions(+)
> 
> diff --git a/drivers/net/thunderx/nicvf_ethdev.c 
> b/drivers/net/thunderx/nicvf_ethdev.c
> index 5da07da..ba32803 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -88,6 +88,8 @@ static int nicvf_dev_rss_hash_update(struct rte_eth_dev 
> *dev,
>struct rte_eth_rss_conf *rss_conf);
>  static int nicvf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
>  struct rte_eth_rss_conf *rss_conf);
> +static int nicvf_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t qidx);
> +static int nicvf_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t qidx);
These declarations not required, there are more usages in other patches.

...
>  
>  static int
> +nicvf_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t qidx)
> +{
> + int ret;
> +
> + if (qidx >= nicvf_pmd_priv(dev)->eth_dev->data->nb_rx_queues)
> + return -EINVAL;
This check already done by librte_ether

> +
> + ret = nicvf_start_rx_queue(dev, qidx);
> + if (ret)
> + return ret;
> +
> + ret = nicvf_configure_cpi(dev);
> + if (ret)
> + return ret;
> +
> + return nicvf_configure_rss_reta(dev);
> +}
> +
> +static int
> +nicvf_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t qidx)
> +{
> + int ret;
> +
> + if (qidx >= nicvf_pmd_priv(dev)->eth_dev->data->nb_rx_queues)
> + return -EINVAL;
Same for this case

> +
> + ret = nicvf_stop_rx_queue(dev, qidx);
> + ret |= nicvf_configure_cpi(dev);
> + ret |= nicvf_configure_rss_reta(dev);
> + return ret;
> +}
> +
...


[dpdk-dev] [PATCH] doc: clarify port detaching/attaching in testpmd

2016-06-08 Thread Thomas Monjalon
2016-06-08 16:40, Pablo de Lara:
> Port needs to be stopped and then closed before it can be detached,
> but the documentation was only saying to close the port.
> 
> Also, both sections for port detaching and attaching has been reformatted
> slightly, to show clearly how to use the commands.
> 
> Signed-off-by: Pablo de Lara 

Applied, thanks


[dpdk-dev] [PATCH] testpmd: fix typo when closing port

2016-06-08 Thread Thomas Monjalon
2016-06-08 16:20, Pablo de Lara:
>   if (rte_atomic16_cmpset(&(port->port_status),
>   RTE_PORT_HANDLING, RTE_PORT_CLOSED) == 0)
> - printf("Port %d can not be set into stopped\n", pi);
> + printf("Port %d can not be set into closed\n", pi);

My english would say "Port cannot be closed".
Am I wrong?


[dpdk-dev] [PATCH] app/test: reduced duration of red_autotest

2016-06-08 Thread Thomas Monjalon
2016-06-08 15:56, Kantecki, Tomasz:
> Hi Thomas,
>  
> > > 12s is long for a functional test. We are used to have tests of less than 
> > > 1s.
> > > Do you think this magnitude order can be achieved for red tests?
> > 
> > This is a real challenge :) Let me take another look into it.
> 
> I had a 2nd look into this test suite and reducing time for functional tests 
> is doable but labor intensive. Let me explain:
> RED/WRED algorithm is time based (packet queue events in time). For tracking 
> time the test uses TSC (x86 time stamp counter). This could be replaced with 
> some fake time stamp in functional tests and result in significant reduction 
> of execution time. It needs couple of days to carefully replace TSC with a 
> fake time stamp solution without breaking test logic.

Sorry, I haven't dived into the code, but I don't understand how we get
some seconds delays in a code for fast processing on nano-second CPU.
Are you sure you cannot have the same result with less iterations and
shorter timeouts?


[dpdk-dev] [PATCH v3 14/20] thunderx/nicvf: add dev_supported_ptypes_get and rx_queue_count support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---

...

> +uint32_t
> +nicvf_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t queue_idx)
> +{
> + struct nicvf_rxq *rxq;
> +
> + rxq = (struct nicvf_rxq *)dev->data->rx_queues[queue_idx];
Unnecessary cast

...



[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Jerin Jacob
On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
> On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> > Jerin Jacob (20):
> >   thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC
> >   thunderx/nicvf: add pmd skeleton
> >   thunderx/nicvf: add link status and link update support
> >   thunderx/nicvf: add get_reg and get_reg_length support
> >   thunderx/nicvf: add dev_configure support
> >   thunderx/nicvf: add dev_infos_get support
> >   thunderx/nicvf: add rx_queue_setup/release support
> >   thunderx/nicvf: add tx_queue_setup/release support
> >   thunderx/nicvf: add rss and reta query and update support
> >   thunderx/nicvf: add mtu_set and promiscuous_enable support
> >   thunderx/nicvf: add stats support
> >   thunderx/nicvf: add single and multi segment tx functions
> >   thunderx/nicvf: add single and multi segment rx functions
> >   thunderx/nicvf: add dev_supported_ptypes_get and rx_queue_count
> > support
> >   thunderx/nicvf: add rx queue start and stop support
> >   thunderx/nicvf: add tx queue start and stop support
> >   thunderx/nicvf: add device start,stop and close support
> >   thunderx/config: set max numa node to two
> >   thunderx/nicvf: updated driver documentation and release notes
> >   maintainers: claim responsibility for the ThunderX nicvf PMD
> > 
> 
> Hi Jerin,
> 
> In patch subject, as tag, other drivers are using only driver name, and
> Intel drivers also has "driver/base", since base code has some special
> case. For thunderx, what do you think about keeping subject as:
>  "thunderx: "
> 

Hi Ferruh,

We may add crypto or other builtin ThunderX HW accelerated block drivers
in future to DPDK.
So that is the reason why I thought of keeping the subject as thunderx/nicvf.
If you don't have any objection then I would like to keep it as
thunderx/nicvf or just nicvf.

> Thanks,
> ferruh


[dpdk-dev] [PATCH v3 13/20] thunderx/nicvf: add single and multi segment rx functions

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 

Reviewed-by: Ferruh Yigit 



[dpdk-dev] [PATCH 2/3] librte_eal: Import FreeBSD sys/tree.h into librte_eal/common

2016-06-08 Thread Stephen Hemminger
On Thu,  9 Jun 2016 02:53:53 +0200
Nikita Kozlov  wrote:

> This structure is used inside the rte_lpm6 lib for storing added rules.
> It's imported from FreeBSD-10.3 from /usr/include/sys/tree.h, another
> solution could have been to use on Linux the version from libbsd but it
> would create an external dependency.
> 
> Signed-off-by: Nikita Kozlov 

Using Red-black tree is a good idea, and we have been doing it for a while
both on v4 and v6.

But this is not the way to handle it.
Please don't copy a header file which is available already on both BSD and 
Linux.




[dpdk-dev] [PATCH v2] app/testpmd: add packet data pointer prefetch in the forwarding loop

2016-06-08 Thread Thomas Monjalon
2016-06-07 11:19, De Lara Guarch, Pablo:
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> > prefetch the next packet data address in advance in forwarding loop
> > for performance improvement.
> > 
> > Signed-off-by: Jerin Jacob 
> 
> Patch looks good, but there is going to be a conflict with 
> http://dpdk.org/dev/patchwork/patch/13134/,
> since that patch is removing the mac_retry forwarding mode.
> 
> Apart from that,
> Acked-by: Pablo de Lara 

Applied, thanks


[dpdk-dev] [PATCH v3 19/20] thunderx/nicvf: updated driver documentation and release notes

2016-06-08 Thread Jerin Jacob
On Wed, Jun 08, 2016 at 01:08:35PM +0100, Ferruh Yigit wrote:
> On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> > Updated doc/guides/nics/overview.rst, doc/guides/nics/thunderx.rst
> > and release notes
> > 
> > Changed "*" to "P" in overview.rst to capture the partially supported
> > feature as "*" creating alignment issues with Sphinx table
> > 
> > Signed-off-by: Jerin Jacob 
> > Signed-off-by: Slawomir Rosek 
> > Acked-by: John McNamara 
> > ---
> >  doc/guides/nics/index.rst  |   1 +
> >  doc/guides/nics/overview.rst   |  96 -
> >  doc/guides/nics/thunderx.rst   | 354 
> > +
> >  doc/guides/rel_notes/release_16_07.rst |   1 +
> >  4 files changed, 404 insertions(+), 48 deletions(-)
> >  create mode 100644 doc/guides/nics/thunderx.rst
> 
> Hi Jerin,
> 
> This patch doesn't apply on top of origin/rel_16_07:
> 
> Applying: thunderx/nicvf: updated driver documentation and release notes
> Using index info to reconstruct a base tree...
> M doc/guides/nics/overview.rst
> Falling back to patching base and 3-way merge...
> Auto-merging doc/guides/nics/overview.rst
> CONFLICT (content): Merge conflict in doc/guides/nics/overview.rst

Hi Ferruh,

Since docs files are files keep changing, this patch set
has been re-based on latest change-set i.e 
ca173a909538a2f1082cd0dcb4d778a97dab69c3
not origin/rel_16_07.

> 
> Regards,
> ferruh


[dpdk-dev] [PATCH v3 11/20] thunderx/nicvf: add stats support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 

Reviewed-by: Ferruh Yigit 


[dpdk-dev] [PATCH v3 10/20] thunderx/nicvf: add mtu_set and promiscuous_enable support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 

Reviewed-by: Ferruh Yigit 



[dpdk-dev] [PATCH v3 09/20] thunderx/nicvf: add rss and reta query and update support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 

Reviewed-by: Ferruh Yigit 



[dpdk-dev] [PATCH v3 07/20] thunderx/nicvf: add rx_queue_setup/release support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
Reviewed-by: Ferruh Yigit 



[dpdk-dev] [PATCH] i40e: fix link management

2016-06-08 Thread Bruce Richardson
On Thu, May 12, 2016 at 03:21:04PM +0800, Jingjing Wu wrote:
> Previously, there was a known issue "On Intel? 40G Ethernet
> Controller stopping the port does not really down the port link."
> There were two reasons why the port is always kept up.
> 1. Old version Firmware would cause issue when call "Set PHY config
> command" on 40G NIC.
> 2. Because linux kernel i40e driver didn?t call "Set PHY config
> command" when ifconfig up/down, and it assumes the link always up.
> While ports are forced down when DPDK quit. So if the port is switched
> to controlled by kernel driver, the port will not be up through
> "ifconfig  up".
> 
> This patch fixes this issue by reopening "Set PHY config command"
> because:
> 1. New firmware issue is already fixed.
> 2. After DPDK quit, "ethtool -s  autoneg on" can be used to
> turn on the auto negotiation, and then port can be up through
> "ifconfig  up" in new version kernel i40e driver( >1.4.X ).
> 
> Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
> Fixes: 16c979f9adf2 ("i40e: disable setting of PHY configuration")
> Signed-off-by: Jingjing Wu 

Patch applied to dpdk-next-net/rel_16_07, with some commit message cleanups
applied.

/Bruce


[dpdk-dev] [PATCH v5] ip_pipeline: configuration file parser cleanup

2016-06-08 Thread Jasvinder Singh
This commit adds following changes to configuration file parsing of
the ip pipeline application;

1. Parsing routines related to packet queues (pktq_in/out fields in the
PIPELINE section) and message queues (msgq_in/out fields of in the MSGQ
Section) are updated.

In the parsing routines, function "strtok_r()" is used for parsing the
string instead of manually checking the string termination, white
spaces, tabs etc., between the string tokens. Each call to strtok_r()
returns a pointer to a null-terminated string containing the next token.
If no more tokens are found, strtok_r() returns NULL. As a result of
using strtok_r(), the code size of the parsing routines is reduced
significantly.

2. Replace PARSER_PARAM_ADD_CHECK macro by more specific macros such as
PARSE_CHECK_DUPLICATE_SECTION, PARSE_CHECK_DUPLICATE_SECTION_EAL to detect
duplicate entries in the various sections of the configuration file

3. Add new macros PARSER_ERROR_NO_ELEMENTS and PARSE_ERROR_TOO_MANY_ELEMENTS
for detecting no element and more elements than allowed situations
respectively, in the section entry.

4. Add new macros APP_PARAM_ADD_LINK_FOR_RXQ, APP_PARAM_ADD_LINK_FOR_TXQ
and APP_PARAM_ADD_LINK_FOR_TM which add corresponding nic ports entry to
the application param structure while parsing rx/tx queues, TM (Traffic
Manager) port sections and pktq_in/out entries of pipeline sections

Signed-off-by: Jasvinder Singh 
Acked-by: Cristian Dumitrescu 
---
v5
- fix clang compilation error on enum type 'enum app_pktq_out_type'

v4
- update the commit message
- move APP_PARAM_ADD macro from app.h to config_parse.c as it is only used
  by the routines defined in this file
- remove extra newline character from error message display
- rebased on top of ip_pipeline CLI patchset
  (http://dpdk.org/dev/patchwork/patch/12911/) and NULL packet processing
  fix patch (http://dpdk.org/dev/patchwork/patch/12807/)

v3
- add check on the number of pktq_in/out entries
- add check on the number of msgq_in/out entries

v2
- update the commit message
- change the local variable name from "token" to "name"

 examples/ip_pipeline/app.h  |  25 +-
 examples/ip_pipeline/config_parse.c | 470 +++-
 2 files changed, 195 insertions(+), 300 deletions(-)

diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
index e775024..05d608b 100644
--- a/examples/ip_pipeline/app.h
+++ b/examples/ip_pipeline/app.h
@@ -50,6 +50,7 @@

 #define APP_PARAM_NAME_SIZE  PIPELINE_NAME_SIZE
 #define APP_LINK_PCI_BDF_SIZE16
+
 struct app_mempool_params {
char *name;
uint32_t parsed;
@@ -370,6 +371,8 @@ struct app_eal_params {
/* Support running on Xen dom0 without hugetlbfs */
uint32_t xen_dom0_present;
int xen_dom0;
+
+   uint32_t parsed;
 };

 #ifndef APP_APPNAME_SIZE
@@ -529,28 +532,6 @@ do 
\
sscanf(obj->name, prefix "%" SCNu32, &id);  
\
 while (0)  \

-#define APP_PARAM_ADD(obj_array, obj_name) \
-({ \
-   ssize_t obj_idx;\
-   const ssize_t obj_count = RTE_DIM(obj_array);   \
-   \
-   obj_idx = APP_PARAM_FIND(obj_array, obj_name);  \
-   if (obj_idx < 0) {  \
-   for (obj_idx = 0; obj_idx < obj_count; obj_idx++) { \
-   if (!APP_PARAM_VALID(&((obj_array)[obj_idx])))  \
-   break;  \
-   }   \
-   \
-   if (obj_idx < obj_count) {  \
-   (obj_array)[obj_idx].name = strdup(obj_name);   \
-   if ((obj_array)[obj_idx].name == NULL)  \
-   obj_idx = -EINVAL;  \
-   } else  \
-   obj_idx = -ENOMEM;  \
-   }   \
-   obj_idx;\
-})
-
 #defineAPP_CHECK(exp, fmt, ...)
\
 do {   \
if (!(exp)) {   \
diff --git a/examples/ip_pipeline/config_parse.c 
b/examples/ip_pipeline/config_parse.c
index 3951e1d..2a222b2 100644
--- a/examples/ip_pipeline/config_parse.c

[dpdk-dev] [PATCH v3 06/20] thunderx/nicvf: add dev_infos_get support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 

Reviewed-by: Ferruh Yigit 


[dpdk-dev] [PATCH] app/testpmd: log mbuf pool creation

2016-06-08 Thread Thomas Monjalon
2016-06-02 14:49, De Lara Guarch, Pablo:
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> > Enhance the logs related to mbuf pool creation. Display an info level
> > log when creating the mbuf, and display the error as a string on failure.
> > 
> > After the patch, we have:
> > 
> >   [...]
> >   EAL:   probe driver: 8086:10fb rte_ixgbe_pmd
> >   USER1: create a new mbuf pool : n=331456, \
> >   size=2176, socket=0
> >   EAL: Error - exiting with code: 1
> > Cause: Creation of mbuf pool for socket 0 failed: Cannot allocate \
> >   memory
> > 
> > Signed-off-by: Olivier Matz 
> 
> Acked-by: Pablo de Lara 

Applied, thanks


[dpdk-dev] [PATCH v3 05/20] thunderx/nicvf: add dev_configure support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
Reviewed-by: Ferruh Yigit 

...
>  
> +static int nicvf_dev_configure(struct rte_eth_dev *dev);
Same as previous, I think this is not required.

...


[dpdk-dev] [PATCH] testpmd: fix typo when closing port

2016-06-08 Thread Mcnamara, John


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, June 8, 2016 5:29 PM
> To: De Lara Guarch, Pablo 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] testpmd: fix typo when closing port
> 
> 2016-06-08 16:20, Pablo de Lara:
> > if (rte_atomic16_cmpset(&(port->port_status),
> > RTE_PORT_HANDLING, RTE_PORT_CLOSED) == 0)
> > -   printf("Port %d can not be set into stopped\n", pi);
> > +   printf("Port %d can not be set into closed\n", pi);
> 
> My english would say "Port cannot be closed".
> Am I wrong?

Yes, that is better or "Port cannot be set to closed" if "closed" is a
state like here.


[dpdk-dev] [PATCH v3 04/20] thunderx/nicvf: add get_reg and get_reg_length support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
>  drivers/net/thunderx/nicvf_ethdev.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/net/thunderx/nicvf_ethdev.c 
> b/drivers/net/thunderx/nicvf_ethdev.c
> index 5d28eea..34b4735 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -70,6 +70,9 @@
>  #include "nicvf_logs.h"
>  
>  static int nicvf_dev_link_update(struct rte_eth_dev *dev, int 
> wait_to_complete);
> +static int nicvf_dev_get_reg_length(struct rte_eth_dev *dev);
> +static int nicvf_dev_get_regs(struct rte_eth_dev *dev,
> +   struct rte_dev_reg_info *regs);

Is these declarations required, function order seems correct? Since
these are static functions, it is possible to remove these by
re-ordering function locations.

>  
>  static inline int
>  nicvf_atomic_write_link_status(struct rte_eth_dev *dev,
> @@ -145,9 +148,36 @@ nicvf_dev_link_update(struct rte_eth_dev *dev,
>   return nicvf_atomic_write_link_status(dev, &link);
>  }
>  
> +static int
> +nicvf_dev_get_reg_length(struct rte_eth_dev *dev  __rte_unused)
> +{
> + return nicvf_reg_get_count();
> +}
> +
> +static int
> +nicvf_dev_get_regs(struct rte_eth_dev *dev, struct rte_dev_reg_info *regs)
> +{
> + uint64_t *data = regs->data;
> + struct nicvf *nic = nicvf_pmd_priv(dev);
> +
> + if (data == NULL)
> + return -EINVAL;
> +
> + /* Support only full register dump */
> + if ((regs->length == 0) ||
> + (regs->length == (uint32_t)nicvf_reg_get_count())) {
> + regs->version = nic->vendor_id << 16 | nic->device_id;
> + nicvf_reg_dump(nic, data);
> + return 0;
> + }
> + return -ENOTSUP;
> +}
> +
>  /* Initialize and register driver with DPDK Application */
>  static const struct eth_dev_ops nicvf_eth_dev_ops = {
>   .link_update  = nicvf_dev_link_update,
> + .get_reg_length   = nicvf_dev_get_reg_length,
> + .get_reg  = nicvf_dev_get_regs,
>  };
>  
>  static int
> 



[dpdk-dev] [PATCH v5 1/5] bnx2x: fixed stats get

2016-06-08 Thread Bruce Richardson
On Wed, May 11, 2016 at 05:06:21PM -0700, Rasesh Mody wrote:
> Fix stats_get() routine to display drop counters under imissed counter.
> 
> Fixes: 540a211084a7 ("bnx2x: driver core")
> 
> Signed-off-by: Rasesh Mody 
> Signed-off-by: Harish Patil 

Patchset applied to dpdk-next-net/rel_16_07

/Bruce


[dpdk-dev] [PATCHv6 7/7] doc: Add prog_guide section documenting pmdinfo script

2016-06-08 Thread Mcnamara, John
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, May 31, 2016 2:58 PM
> To: dev at dpdk.org
> Cc: Neil Horman ; Richardson, Bruce
> ; Thomas Monjalon  6wind.com>;
> Stephen Hemminger ; Panu Matilainen
> 
> Subject: [dpdk-dev] [PATCHv6 7/7] doc: Add prog_guide section documenting
> pmdinfo script
> 
> Information on pmdinfogen may be useful to 3rd party driver developers.
> Include documentation on what it does

Hi,

There are some trailing whitespace warnings on merge. Some other, minor,
comments below.


> 
> +.. _Internally_Generated_Build_Tools

The target needs a colon at the end to make it valid:

.. _Internally_Generated_Build_Tools:


> +
> +buildtools/pmdinfogen

This might be better with some distinguishing highlighting, either
``buildtools/pmdinfogen`` or **buildtools/pmdinfogen**.


> +pmdinfogen scans an object (.o) file for various well known symbol

Instances of pmdinfogen would be better as a fixed width ``pmdinfogen``.


> +names.  These well know symbol names are defined by various macros and
> +used to export important information about hardware support and usage
> +for pmd files.  for instance the macro:

s/know/known
s/for/For


> +These strings can then be searched for by external tools to determine
> +the hardware support of a given library or application

Missing full stop.

Apart from these minor changes:


Acked-by: John McNamara 




[dpdk-dev] [PATCH 03/20] thunderx/nicvf: add link status and link update support

2016-06-08 Thread Ferruh Yigit
On 5/7/2016 4:16 PM, Jerin Jacob wrote:
> Extended the nicvf_interrupt function to respond
> NIC_MBOX_MSG_BGX_LINK_CHANGE mbox message from PF and update
> struct rte_eth_link accordingly.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
Reviewed-by: Ferruh Yigit 



[dpdk-dev] [PATCH v3 02/20] thunderx/nicvf: add pmd skeleton

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Introduce driver initialization and enable build infrastructure for
> nicvf pmd driver.
> 
> By default, It is enabled only for defconfig_arm64-thunderx-*
> config as it is an inbuilt NIC device.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
>  config/common_base |  10 +
>  config/defconfig_arm64-thunderx-linuxapp-gcc   |  10 +
>  drivers/net/Makefile   |   1 +
>  drivers/net/thunderx/Makefile  |  63 ++
>  drivers/net/thunderx/nicvf_ethdev.c| 251 
> +
>  drivers/net/thunderx/nicvf_ethdev.h|  48 
>  drivers/net/thunderx/nicvf_logs.h  |  83 +++
>  drivers/net/thunderx/nicvf_struct.h| 124 ++
>  .../thunderx/rte_pmd_thunderx_nicvf_version.map|   4 +
>  mk/rte.app.mk  |   2 +
>  10 files changed, 596 insertions(+)
>  create mode 100644 drivers/net/thunderx/Makefile
>  create mode 100644 drivers/net/thunderx/nicvf_ethdev.c
>  create mode 100644 drivers/net/thunderx/nicvf_ethdev.h
>  create mode 100644 drivers/net/thunderx/nicvf_logs.h
>  create mode 100644 drivers/net/thunderx/nicvf_struct.h
>  create mode 100644 drivers/net/thunderx/rte_pmd_thunderx_nicvf_version.map
> 
> diff --git a/config/common_base b/config/common_base
> index 47c26f6..ad5686b 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -259,6 +259,16 @@ CONFIG_RTE_LIBRTE_PMD_SZEDATA2=n
>  CONFIG_RTE_LIBRTE_PMD_SZEDATA2_AS=0
>  
>  #
> +# Compile burst-oriented Cavium Thunderx NICVF PMD driver
> +#
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_INIT=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_RX=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_TX=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_DRIVER=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_MBOX=n
> +
> +#
>  # Compile burst-oriented VIRTIO PMD driver
>  #
>  CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
> diff --git a/config/defconfig_arm64-thunderx-linuxapp-gcc 
> b/config/defconfig_arm64-thunderx-linuxapp-gcc
> index fe5e987..7940bbd 100644
> --- a/config/defconfig_arm64-thunderx-linuxapp-gcc
> +++ b/config/defconfig_arm64-thunderx-linuxapp-gcc
> @@ -34,3 +34,13 @@
>  CONFIG_RTE_MACHINE="thunderx"
>  
>  CONFIG_RTE_CACHE_LINE_SIZE=128
> +
> +#
> +# Compile Cavium Thunderx NICVF PMD driver
> +#
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD=y
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_INIT=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_RX=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_TX=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_DRIVER=n
> +CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_MBOX=n
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 6ba7658..0e29a33 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -50,6 +50,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += pcap
>  DIRS-$(CONFIG_RTE_LIBRTE_QEDE_PMD) += qede
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += ring
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += szedata2
> +DIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += thunderx
>  DIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio
>  DIRS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += vmxnet3
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += xenvirt
> diff --git a/drivers/net/thunderx/Makefile b/drivers/net/thunderx/Makefile
> new file mode 100644
> index 000..eb9f100
> --- /dev/null
> +++ b/drivers/net/thunderx/Makefile
> @@ -0,0 +1,63 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2016 Cavium Networks. 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 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 (INCLUDIN

[dpdk-dev] [PATCH v4] ip_pipeline: configuration file parser cleanup

2016-06-08 Thread Thomas Monjalon
clang compilation fails:
examples/ip_pipeline/config_parse.c:852:11: error:
implicit conversion from enumeration type 'enum app_pktq_out_type'
to different enumeration type 'enum app_pktq_in_type'


[dpdk-dev] [PATCH v3 0/7] examples/ip_pipeline: CLI rework and improvements

2016-06-08 Thread Thomas Monjalon
2016-06-08 12:35, Piotr Azarewicz:
> Using the latest librte_cmdline improvements, the CLI implementation of the
> ip_pipeline application is streamlined and improved, which results in
> eliminating thousands of lines of code from the application, thus leading to
> code that is easier to maintain and extend.
> 
> v3 changes:
> - fix the authorship in patches
> 
> v2 changes:
> - added functions for parsing hex values
> - added standard error messages for CLI and file bulk
> - for all CLI commands: separate code paths for each flavor of each command
> (e.g. route add, route add default, route ls, route del, route del default,
> etc do not share any line of code)
> - for bulk commands: simplified error checking
> - added additional config files
> 
> Acked-by: Cristian Dumitrescu 
> 
> Daniel Mrzyglod (1):
>   examples/ip_pipeline: modifies firewall pipeline CLI
> 
> Piotr Azarewicz (4):
>   examples/ip_pipeline: add helper functions for parsing string
>   examples/ip_pipeline: modifies flow action pipeline CLI
>   examples/ip_pipeline: modifies routing pipeline CLI
>   examples/ip_pipeline: update edge router usecase
> 
> Tomasz Kulasek (2):
>   examples/ip_pipeline: modifies common pipeline CLI
>   examples/ip_pipeline: modifies flow classifications pipeline CLI

Applied, thanks



[dpdk-dev] [PATCH v3 01/20] thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Adds hardware specific API for ThunderX nicvf inbuilt NIC device under
> drivers/net/thunderx/nicvf/base directory.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
>  drivers/net/thunderx/base/nicvf_hw.c  |  908 +
>  drivers/net/thunderx/base/nicvf_hw.h  |  240 ++
>  drivers/net/thunderx/base/nicvf_hw_defs.h | 1216 
> +
>  drivers/net/thunderx/base/nicvf_mbox.c|  416 ++
>  drivers/net/thunderx/base/nicvf_mbox.h|  232 ++
>  drivers/net/thunderx/base/nicvf_plat.h|  132 
>  6 files changed, 3144 insertions(+)
>  create mode 100644 drivers/net/thunderx/base/nicvf_hw.c
>  create mode 100644 drivers/net/thunderx/base/nicvf_hw.h
>  create mode 100644 drivers/net/thunderx/base/nicvf_hw_defs.h
>  create mode 100644 drivers/net/thunderx/base/nicvf_mbox.c
>  create mode 100644 drivers/net/thunderx/base/nicvf_mbox.h
>  create mode 100644 drivers/net/thunderx/base/nicvf_plat.h
> 
> diff --git a/drivers/net/thunderx/base/nicvf_hw.c 
> b/drivers/net/thunderx/base/nicvf_hw.c
> new file mode 100644
> index 000..24fe77d
> --- /dev/null
> +++ b/drivers/net/thunderx/base/nicvf_hw.c
> @@ -0,0 +1,908 @@
> +/*
> + *   BSD LICENSE
> + *
> + *   Copyright (C) Cavium networks Ltd. 2016.
> + *
> + *   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 
> +#include 
> +
> +#include "nicvf_plat.h"
> +
> +struct nicvf_reg_info {
> + uint32_t offset;
> + const char *name;
> +};
> +
> +#define NICVF_REG_INFO(reg) {reg, #reg}
> +
> +static const struct nicvf_reg_info nicvf_reg_tbl[] = {
> + NICVF_REG_INFO(NIC_VF_CFG),
> + NICVF_REG_INFO(NIC_VF_PF_MAILBOX_0_1),
> + NICVF_REG_INFO(NIC_VF_INT),
> + NICVF_REG_INFO(NIC_VF_INT_W1S),
> + NICVF_REG_INFO(NIC_VF_ENA_W1C),
> + NICVF_REG_INFO(NIC_VF_ENA_W1S),
> + NICVF_REG_INFO(NIC_VNIC_RSS_CFG),
> + NICVF_REG_INFO(NIC_VNIC_RQ_GEN_CFG),
> +};
> +
> +static const struct nicvf_reg_info nicvf_multi_reg_tbl[] = {
> + {NIC_VNIC_RSS_KEY_0_4 + 0,  "NIC_VNIC_RSS_KEY_0"},
> + {NIC_VNIC_RSS_KEY_0_4 + 8,  "NIC_VNIC_RSS_KEY_1"},
> + {NIC_VNIC_RSS_KEY_0_4 + 16, "NIC_VNIC_RSS_KEY_2"},
> + {NIC_VNIC_RSS_KEY_0_4 + 24, "NIC_VNIC_RSS_KEY_3"},
> + {NIC_VNIC_RSS_KEY_0_4 + 32, "NIC_VNIC_RSS_KEY_4"},
> + {NIC_VNIC_TX_STAT_0_4 + 0,  "NIC_VNIC_STAT_TX_OCTS"},
> + {NIC_VNIC_TX_STAT_0_4 + 8,  "NIC_VNIC_STAT_TX_UCAST"},
> + {NIC_VNIC_TX_STAT_0_4 + 16,  "NIC_VNIC_STAT_TX_BCAST"},
> + {NIC_VNIC_TX_STAT_0_4 + 24,  "NIC_VNIC_STAT_TX_MCAST"},
> + {NIC_VNIC_TX_STAT_0_4 + 32,  "NIC_VNIC_STAT_TX_DROP"},
> + {NIC_VNIC_RX_STAT_0_13 + 0,  "NIC_VNIC_STAT_RX_OCTS"},
> + {NIC_VNIC_RX_STAT_0_13 + 8,  "NIC_VNIC_STAT_RX_UCAST"},
> + {NIC_VNIC_RX_STAT_0_13 + 16, "NIC_VNIC_STAT_RX_BCAST"},
> + {NIC_VNIC_RX_STAT_0_13 + 24, "NIC_VNIC_STAT_RX_MCAST"},
> + {NIC_VNIC_RX_STAT_0_13 + 32, "NIC_VNIC_STAT_RX_RED"},
> + {NIC_VNIC_RX_STAT_0_13 + 40, "NIC_VNIC_STAT_RX_RED_OCTS"},
> + {NIC_VNIC_RX_STAT_0_13 + 48, "NIC_VNIC_STAT_RX_ORUN"},
> + {NIC_VNIC_RX_STAT_0_13 + 56, "NIC_VNIC_STAT_RX_ORUN_OCTS"},

[dpdk-dev] [PATCH] doc: remove reference to MATCH

2016-06-08 Thread Chen Jing D(Mark)
From: "Chen Jing D(Mark)" 

Intel stopped supporting MATCH, remove reference of MATCH in the
document.

Signed-off-by: Chen Jing D(Mark) 
---
 doc/guides/nics/fm10k.rst |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/fm10k.rst b/doc/guides/nics/fm10k.rst
index c4915d8..7fc4862 100644
--- a/doc/guides/nics/fm10k.rst
+++ b/doc/guides/nics/fm10k.rst
@@ -157,10 +157,9 @@ Switch manager
 The Intel FM1 family of NICs integrate a hardware switch and multiple host
 interfaces. The FM1 PMD driver only manages host interfaces. For the
 switch component another switch driver has to be loaded prior to to the
-FM1 PMD driver.  The switch driver can be acquired for Intel support or
-from the `Match Interface `_ project.
+FM1 PMD driver. The switch driver can be acquired from Intel support.
 Only Testpoint is validated with DPDK, the latest version that has been
-validated with DPDK2.2 is 4.1.6.
+validated with DPDK is 4.1.6.

 CRC striping
 
-- 
1.7.7.6



[dpdk-dev] [PATCH] doc: clarify port detaching/attaching in testpmd

2016-06-08 Thread Pablo de Lara
Port needs to be stopped and then closed before it can be detached,
but the documentation was only saying to close the port.

Also, both sections for port detaching and attaching has been reformatted
slightly, to show clearly how to use the commands.

Signed-off-by: Pablo de Lara 
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index aed5e47..c4048ae 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -980,7 +980,9 @@ The following sections show functions for configuring ports.
 port attach
 ~~~

-Attach a port specified by pci address or virtual device args.
+Attach a port specified by pci address or virtual device args::
+
+   testpmd> port attach (identifier)

 To attach a new pci device, the device should be recognized by kernel first.
 Then it should be moved under DPDK management.
@@ -1014,8 +1016,6 @@ For example, to move a pci device using ixgbe under DPDK 
management:

 To attach a port created by virtual device, above steps are not needed.

-port attach (identifier)
-
 For example, to attach a port whose pci address is :0a:00.0.

 .. code-block:: console
@@ -1061,16 +1061,19 @@ the mode and slave parameters must be given.
 port detach
 ~~~

-Detach a specific port.
-
-Before detaching a port, the port should be closed::
+Detach a specific port::

testpmd> port detach (port_id)

+Before detaching a port, the port should be stopped and closed.
+
 For example, to detach a pci device port 0.

 .. code-block:: console

+   testpmd> port stop 0
+   Stopping ports...
+   Done
testpmd> port close 0
Closing ports...
Done
@@ -1088,6 +1091,9 @@ For example, to detach a virtual device port 0.

 .. code-block:: console

+   testpmd> port stop 0
+   Stopping ports...
+   Done
testpmd> port close 0
Closing ports...
Done
-- 
2.5.5



[dpdk-dev] [PATCH 7/7] mlx5: update documentation part related to features and limitations

2016-06-08 Thread Mcnamara, John


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Nelio Laranjeiro
> Sent: Wednesday, June 8, 2016 10:44 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH 7/7] mlx5: update documentation part related to
> features and limitations
> 
> Signed-off-by: Nelio Laranjeiro 

Acked-by: John McNamara 


[dpdk-dev] [PATCH v2 4/4] app/test: add crypto AES-CBC-128 HMAC-SHA224 and HMAC-SHA384 unit tests

2016-06-08 Thread Fan Zhang
This patch adds the HMAC-SHA224 and HMAC-SHA384 digest generation and
verification tests to crypto

Signed-off-by: Fan Zhang 
---
 app/test/test_cryptodev_aes.c |  26 
 app/test/test_cryptodev_aes.h | 296 ++
 2 files changed, 322 insertions(+)

diff --git a/app/test/test_cryptodev_aes.c b/app/test/test_cryptodev_aes.c
index dad9f54..dbee8b8 100755
--- a/app/test/test_cryptodev_aes.c
+++ b/app/test/test_cryptodev_aes.c
@@ -211,6 +211,32 @@ static const struct aes_test_case aes_test_cases[] = {
.feature_mask = AES_TEST_FEATURE_OOP,
.pmd_mask = AES_TEST_TARGET_PMD_QAT
},
+   {
+   .test_descr = "AES-128-CBC HMAC-SHA224 Encryption Digest",
+   .test_data = &aes_test_data_8,
+   .op_mask = AES_TEST_OP_ENC_AUTH_GEN,
+   .pmd_mask = AES_TEST_TARGET_PMD_MB
+   },
+   {
+   .test_descr = "AES-128-CBC HMAC-SHA224 Decryption Digest "
+   "Verify",
+   .test_data = &aes_test_data_8,
+   .op_mask = AES_TEST_OP_AUTH_VERIFY_DEC,
+   .pmd_mask = AES_TEST_TARGET_PMD_MB
+   },
+   {
+   .test_descr = "AES-128-CBC HMAC-SHA384 Encryption Digest",
+   .test_data = &aes_test_data_9,
+   .op_mask = AES_TEST_OP_ENC_AUTH_GEN,
+   .pmd_mask = AES_TEST_TARGET_PMD_MB
+   },
+   {
+   .test_descr = "AES-128-CBC HMAC-SHA384 Decryption Digest "
+   "Verify",
+   .test_data = &aes_test_data_9,
+   .op_mask = AES_TEST_OP_AUTH_VERIFY_DEC,
+   .pmd_mask = AES_TEST_TARGET_PMD_MB
+   },
 };

 static int
diff --git a/app/test/test_cryptodev_aes.h b/app/test/test_cryptodev_aes.h
index c3b439b..ef518e0 100755
--- a/app/test/test_cryptodev_aes.h
+++ b/app/test/test_cryptodev_aes.h
@@ -825,4 +825,300 @@ static const struct aes_test_data aes_test_data_7 = {
}
 };

+/** AES-128-CBC SHA224 test vector */
+static const struct aes_test_data aes_test_data_8 = {
+   .crypto_algo = RTE_CRYPTO_CIPHER_AES_CBC,
+   .cipher_key = {
+   .data = {
+   0xE4, 0x23, 0x33, 0x8A, 0x35, 0x64, 0x61, 0xE2,
+   0x49, 0x03, 0xDD, 0xC6, 0xB8, 0xCA, 0x55, 0x7A
+   },
+   .len = 16
+   },
+   .iv = {
+   .data = {
+   0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+   0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F
+   },
+   .len = 16
+   },
+   .plaintext = {
+   .data = {
+   "What a lousy earth! He wondered how many people "
+   "were destitute that same night even in his own "
+   "prosperous country, how many homes were "
+   "shanties, how many husbands were drunk and "
+   "wives socked, and how many children were "
+   "bullied, abused, or abandoned. How many "
+   "families hungered for food they could not "
+   "afford to buy? How many hearts were broken? How "
+   "many suicides would take place that same night, "
+   "how many people would go insane? How many "
+   "cockroaches and landlords would triumph? How "
+   "many winners were losers, successes failures, "
+   "and rich men poor men? How many wise guys were "
+   "stupid? How many happy endings were unhappy "
+   "endings? How many honest men were liars, brave "
+   "men cowards, loyal men traitors, how many "
+   "sainted men were corrupt, how many people in "
+   "positions of trust had sold their souls to "
+   "bodyguards, how many had never had souls? How "
+   "many straight-and-narrow paths were crooked "
+   "paths? How many best families were worst "
+   "families and how many good people were bad "
+   "people? When you added them all up and then "
+   "subtracted, you might be left with only the "
+   "children, and perhaps with Albert Einstein and "
+   "an old violinist or sculptor somewhere."
+   },
+   .len = 512
+   },
+   .ciphertext = {
+   .data = {
+   0x8B, 0x4D, 0xDA, 0x1B, 0xCF, 0x04, 0xA0, 0x31,
+   0xB4, 0xBF, 0xBD, 0x68, 0x43, 0x20, 0x7E, 0x76,
+   0xB1, 0x96, 0x8B, 0xA2, 0x7C, 0xA2, 0x83, 0x9E,
+   0x39, 0x5A, 0x2F, 0x7E, 0x92, 0xB4, 0x48, 0x1A,
+   0x3F, 0x6B, 0x5D, 0xDF, 0x52, 0x85, 0x5F, 0

[dpdk-dev] [PATCH v2 3/4] app/test: utilize new unified crypto AES test function

2016-06-08 Thread Fan Zhang
This patch replaces the AES test code with new unified crypto AES test
function.

Signed-off-by: Fan Zhang 
---
 app/test/test_cryptodev.c  | 1613 ++--
 app/test/test_cryptodev_aes_ctr_test_vectors.h |  257 
 2 files changed, 122 insertions(+), 1748 deletions(-)
 delete mode 100644 app/test/test_cryptodev_aes_ctr_test_vectors.h

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 45e6daa..1730022 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -43,7 +43,7 @@
 #include "test.h"
 #include "test_cryptodev.h"

-#include "test_cryptodev_aes_ctr_test_vectors.h"
+#include "test_cryptodev_aes.h"
 #include "test_cryptodev_snow3g_test_vectors.h"
 #include "test_cryptodev_snow3g_hash_test_vectors.h"
 #include "test_cryptodev_gcm_test_vectors.h"
@@ -111,19 +111,6 @@ setup_test_string(struct rte_mempool *mpool,

return m;
 }
-static int
-setup_oop_test_mbufs(struct rte_mbuf **ibuf, struct rte_mbuf **obuf,
-   struct rte_mempool *mpool,  const char *string, size_t len,
-   uint8_t blocksize) {
-   *ibuf = setup_test_string(mpool, string, len, blocksize);
-   if (*ibuf == NULL)
-   return -(EFAULT);
-   *obuf = setup_test_string(mpool, NULL, len, blocksize);
-   if (*obuf == NULL)
-   return -(EFAULT);
-
-   return 0;
-}

 #if HEX_DUMP
 static void
@@ -890,1316 +877,80 @@ test_AES_CBC_HMAC_SHA1_encrypt_digest(void)
sym_op->cipher.iv.phys_addr = rte_pktmbuf_mtophys(ut_params->ibuf);
sym_op->cipher.iv.length = CIPHER_IV_LENGTH_AES_CBC;

-   rte_memcpy(sym_op->cipher.iv.data, aes_cbc_iv,
-   CIPHER_IV_LENGTH_AES_CBC);
-
-   sym_op->cipher.data.offset = CIPHER_IV_LENGTH_AES_CBC;
-   sym_op->cipher.data.length = QUOTE_512_BYTES;
-
-   /* Process crypto operation */
-   TEST_ASSERT_NOT_NULL(process_crypto_request(ts_params->valid_devs[0],
-   ut_params->op), "failed to process sym crypto op");
-
-   TEST_ASSERT_EQUAL(ut_params->op->status, RTE_CRYPTO_OP_STATUS_SUCCESS,
-   "crypto op processing failed");
-
-   /* Validate obuf */
-   uint8_t *ciphertext = rte_pktmbuf_mtod_offset(ut_params->op->sym->m_src,
-   uint8_t *, CIPHER_IV_LENGTH_AES_CBC);
-
-   TEST_ASSERT_BUFFERS_ARE_EQUAL(ciphertext,
-   catch_22_quote_2_512_bytes_AES_CBC_ciphertext,
-   QUOTE_512_BYTES,
-   "ciphertext data not as expected");
-
-   uint8_t *digest = ciphertext + QUOTE_512_BYTES;
-
-   TEST_ASSERT_BUFFERS_ARE_EQUAL(digest,
-   catch_22_quote_2_512_bytes_AES_CBC_HMAC_SHA1_digest,
-   gbl_cryptodev_type == RTE_CRYPTODEV_AESNI_MB_PMD ?
-   TRUNCATED_DIGEST_BYTE_LENGTH_SHA1 :
-   DIGEST_BYTE_LENGTH_SHA1,
-   "Generated digest data not as expected");
-
-   return TEST_SUCCESS;
-}
-
-
-static int
-test_AES_CBC_HMAC_SHA1_encrypt_digest_oop(void)
-{
-   struct crypto_testsuite_params *ts_params = &testsuite_params;
-   struct crypto_unittest_params *ut_params = &unittest_params;
-
-   /* Generate test mbuf data and space for digest */
-
-   TEST_ASSERT_EQUAL(setup_oop_test_mbufs(&ut_params->ibuf,
-   &ut_params->obuf, ts_params->mbuf_pool, catch_22_quote,
-   QUOTE_512_BYTES, 0), 0,
-   "Allocation of rte_mbuf failed");
-
-   ut_params->digest = (uint8_t *)rte_pktmbuf_append(ut_params->obuf,
-   DIGEST_BYTE_LENGTH_SHA1);
-
-   TEST_ASSERT_NOT_NULL(ut_params->digest, "no room to append digest");
-
-   ut_params->cipher_xform.type = RTE_CRYPTO_SYM_XFORM_CIPHER;
-   ut_params->cipher_xform.next = &ut_params->auth_xform;
-
-   ut_params->cipher_xform.cipher.algo = RTE_CRYPTO_CIPHER_AES_CBC;
-   ut_params->cipher_xform.cipher.op = RTE_CRYPTO_CIPHER_OP_ENCRYPT;
-   ut_params->cipher_xform.cipher.key.data = aes_cbc_key;
-   ut_params->cipher_xform.cipher.key.length = CIPHER_KEY_LENGTH_AES_CBC;
-
-   ut_params->auth_xform.type = RTE_CRYPTO_SYM_XFORM_AUTH;
-   ut_params->auth_xform.next = NULL;
-
-   ut_params->auth_xform.auth.op = RTE_CRYPTO_AUTH_OP_GENERATE;
-   ut_params->auth_xform.auth.algo = RTE_CRYPTO_AUTH_SHA1_HMAC;
-   ut_params->auth_xform.auth.key.length = HMAC_KEY_LENGTH_SHA1;
-   ut_params->auth_xform.auth.key.data = hmac_sha1_key;
-   ut_params->auth_xform.auth.digest_length = DIGEST_BYTE_LENGTH_SHA1;
-
-   ut_params->sess = rte_cryptodev_sym_session_create(
-   ts_params->valid_devs[0],
-   &ut_params->cipher_xform);
-
-   TEST_ASSERT_NOT_NULL(ut_params->sess, "Session creation failed");
-
-   ut_params->op = rte_crypto_op_alloc(ts_params->op_

[dpdk-dev] [PATCH v2 2/4] app/test: add unified crypto aes test

2016-06-08 Thread Fan Zhang
This patch adds a new crypto AES unified test function.

Signed-off-by: Fan Zhang 
---
 app/test/Makefile |   1 +
 app/test/test_cryptodev_aes.c | 663 ++
 app/test/test_cryptodev_aes.h |   6 +
 3 files changed, 670 insertions(+)
 create mode 100755 app/test/test_cryptodev_aes.c

diff --git a/app/test/Makefile b/app/test/Makefile
index f269fe0..fd193df 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -153,6 +153,7 @@ endif
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_RING) += test_pmd_ring.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_RING) += test_pmd_ring_perf.c

+SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_aes.c
 SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev.c

diff --git a/app/test/test_cryptodev_aes.c b/app/test/test_cryptodev_aes.c
new file mode 100755
index 000..dad9f54
--- /dev/null
+++ b/app/test/test_cryptodev_aes.c
@@ -0,0 +1,663 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2015-2016 Intel Corporation. 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 Intel Corporation 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 
+
+#include "test.h"
+#include "test_cryptodev_aes.h"
+
+#ifndef MAX_N_AES_TESTS
+#define MAX_N_AES_TESTS256
+#endif
+
+#ifndef AES_TEST_MSG_LEN
+#define AES_TEST_MSG_LEN   256
+#endif
+
+#define AES_TEST_OP_ENCRYPT0x01
+#define AES_TEST_OP_DECRYPT0x02
+#define AES_TEST_OP_AUTH_GEN   0x04
+#define AES_TEST_OP_AUTH_VERIFY0x08
+
+#define AES_TEST_FEATURE_OOP   0x01
+#define AES_TEST_FEATURE_SESSIONLESS   0x02
+#define AES_TEST_FEATURE_STOPPER   0x04 /* stop upon failing */
+
+#define AES_TEST_TARGET_PMD_MB 0x0001 /* Multi-buffer flag */
+#define AES_TEST_TARGET_PMD_QAT0x0002 /* QAT flag */
+
+#define AES_TEST_OP_CIPHER (AES_TEST_OP_ENCRYPT |  \
+   AES_TEST_OP_DECRYPT)
+
+#define AES_TEST_OP_AUTH   (AES_TEST_OP_AUTH_GEN | \
+   AES_TEST_OP_AUTH_VERIFY)
+
+#define AES_TEST_OP_ENC_AUTH_GEN   (AES_TEST_OP_ENCRYPT |  \
+   AES_TEST_OP_AUTH_GEN)
+
+#define AES_TEST_OP_AUTH_VERIFY_DEC(AES_TEST_OP_DECRYPT |  \
+   AES_TEST_OP_AUTH_VERIFY)
+
+struct aes_test_case {
+   const char *test_descr; /* test description */
+   const struct aes_test_data *test_data;
+   uint8_t op_mask; /* operation mask */
+   uint8_t feature_mask;
+   uint32_t pmd_mask;
+};
+
+static const struct aes_test_case aes_test_cases[] = {
+   {
+   .test_descr = "AES-128-CTR HMAC-SHA1 Encryption Digest",
+   .test_data = &aes_test_data_1,
+   .op_mask = AES_TEST_OP_ENC_AUTH_GEN,
+   .pmd_mask = AES_TEST_TARGET_PMD_MB |
+   AES_TEST_TARGET_PMD_QAT
+   },
+   {
+   .test_descr = "AES-128-CTR HMAC-SHA1 Decryption Digest "
+   "Verify",
+   .test_data = &aes_test_data_1,
+   .op_mask = AES_TEST_OP_AUTH_VERIFY_DEC,
+   .pmd_mask = AES_TEST_TARGET_PMD_MB |
+   AES_TEST_TARGET_PMD_QAT
+   },
+   {
+   .test_descr

[dpdk-dev] [PATCH v2 1/4] app/test: categorize crypto AES test vectors into new file

2016-06-08 Thread Fan Zhang
This patch accumulates crypto AES test vectors into a new header file.

Signed-off-by: Fan Zhang 
---
 app/test/test_cryptodev_aes.h | 822 ++
 1 file changed, 822 insertions(+)
 create mode 100755 app/test/test_cryptodev_aes.h

diff --git a/app/test/test_cryptodev_aes.h b/app/test/test_cryptodev_aes.h
new file mode 100755
index 000..8fc6dd0
--- /dev/null
+++ b/app/test/test_cryptodev_aes.h
@@ -0,0 +1,822 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Intel Corporation. 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 Intel Corporation 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 TEST_CRYPTODEV_AES_H_
+#define TEST_CRYPTODEV_AES_H_
+
+struct aes_test_data {
+   enum rte_crypto_cipher_algorithm crypto_algo;
+
+   struct {
+   uint8_t data[64];
+   unsigned len;
+   } cipher_key;
+
+   struct {
+   uint8_t data[64] __rte_aligned(16);
+   unsigned len;
+   } iv;
+
+   struct {
+   uint8_t data[2048];
+   unsigned len;
+   } plaintext;
+
+   struct {
+   uint8_t data[2048];
+   unsigned len;
+   } ciphertext;
+
+   enum rte_crypto_auth_algorithm auth_algo;
+
+   struct {
+   uint8_t data[128];
+   unsigned len;
+   } auth_key;
+
+   struct {
+   uint8_t data[128];
+   unsigned len;   /* for qat */
+   unsigned truncated_len; /* for mb */
+   } digest;
+};
+
+/* test vectors */
+/* AES128-CTR-SHA1 test vector */
+static const struct aes_test_data aes_test_data_1 = {
+   .crypto_algo = RTE_CRYPTO_CIPHER_AES_CTR,
+   .cipher_key = {
+   .data = {
+   0x2B, 0x7E, 0x15, 0x16, 0x28, 0xAE, 0xD2, 0xA6,
+   0xAB, 0xF7, 0x15, 0x88, 0x09, 0xCF, 0x4F, 0x3C
+   },
+   .len = 16
+   },
+   .iv = {
+   .data = {
+   0xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7,
+   0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF
+   },
+   .len = 16
+   },
+   .plaintext = {
+   .data = {
+   0x6B, 0xC1, 0xBE, 0xE2, 0x2E, 0x40, 0x9F, 0x96,
+   0xE9, 0x3D, 0x7E, 0x11, 0x73, 0x93, 0x17, 0x2A,
+   0xAE, 0x2D, 0x8A, 0x57, 0x1E, 0x03, 0xAC, 0x9C,
+   0x9E, 0xB7, 0x6F, 0xAC, 0x45, 0xAF, 0x8E, 0x51,
+   0x30, 0xC8, 0x1C, 0x46, 0xA3, 0x5C, 0xE4, 0x11,
+   0xE5, 0xFB, 0xC1, 0x19, 0x1A, 0x0A, 0x52, 0xEF,
+   0xF6, 0x9F, 0x24, 0x45, 0xDF, 0x4F, 0x9B, 0x17,
+   0xAD, 0x2B, 0x41, 0x7B, 0xE6, 0x6C, 0x37, 0x10
+   },
+   .len = 64
+   },
+   .ciphertext = {
+   .data = {
+   0x87, 0x4D, 0x61, 0x91, 0xB6, 0x20, 0xE3, 0x26,
+   0x1B, 0xEF, 0x68, 0x64, 0x99, 0x0D, 0xB6, 0xCE,
+   0x98, 0x06, 0xF6, 0x6B, 0x79, 0x70, 0xFD, 0xFF,
+   0x86, 0x17, 0x18, 0x7B, 0xB9, 0xFF, 0xFD, 0xFF,
+   0x5A, 0xE4, 0xDF, 0x3E, 0xDB, 0xD5, 0xD3, 0x5E,
+   0x5B, 0x4F, 0x09, 0x02, 0x0D, 0xB0, 0x3E, 0xAB,
+   0x1E, 0x03, 0x1D, 0xDA, 0x2F, 0xBE, 0x03, 0xD1,
+   0x79, 0x21, 0x70

[dpdk-dev] [PATCH v2 0/4] app/test: rework crypto AES unit test

2016-06-08 Thread Fan Zhang
This patchset reworks the crypto AES unit test by introducing a new
unified test function.

This patchset depends on the following patches/patchsets:

"qat: fix phys address of content descriptor"
(http://dpdk.org/dev/patchwork/patch/13137/)
and
"Add AES Counter mode support for AES-NI MB PMD"
(http://dpdk.org/ml/archives/dev/2016-June/040222.html)

v2
*fix session not freed after the test finished problem
*remove l2fwd-crypto sample application patch 

Fan Zhang (4):
  app/test: categorize crypto AES test vectors into new file
  app/test: add unified crypto aes test
  app/test: utilize new unified crypto AES test function
  app/test: add crypto AES-CBC-128 HMAC-SHA224 and HMAC-SHA384 unit
tests

 app/test/Makefile  |1 +
 app/test/test_cryptodev.c  | 1613 ++--
 app/test/test_cryptodev_aes.c  |  689 ++
 app/test/test_cryptodev_aes.h  | 1124 +
 app/test/test_cryptodev_aes_ctr_test_vectors.h |  257 
 5 files changed, 1936 insertions(+), 1748 deletions(-)
 create mode 100755 app/test/test_cryptodev_aes.c
 create mode 100755 app/test/test_cryptodev_aes.h
 delete mode 100644 app/test/test_cryptodev_aes_ctr_test_vectors.h

-- 
2.5.5



[dpdk-dev] [PATCH] doc: clarify port detaching/attaching in testpmd

2016-06-08 Thread Mcnamara, John
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Pablo de Lara
> Sent: Wednesday, June 8, 2016 4:40 PM
> To: dev at dpdk.org; Iremonger, Bernard ;
> mukawa at igel.co.jp
> Cc: De Lara Guarch, Pablo 
> Subject: [dpdk-dev] [PATCH] doc: clarify port detaching/attaching in
> testpmd
> 
> Port needs to be stopped and then closed before it can be detached, but
> the documentation was only saying to close the port.
> 
> Also, both sections for port detaching and attaching has been reformatted
> slightly, to show clearly how to use the commands.
> 
> Signed-off-by: Pablo de Lara 


Acked-by: John McNamara 


[dpdk-dev] [PATCH v4] ip_pipeline: configuration file parser cleanup

2016-06-08 Thread Singh, Jasvinder


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, June 8, 2016 3:48 PM
> To: Singh, Jasvinder 
> Cc: dev at dpdk.org; Dumitrescu, Cristian 
> Subject: Re: [dpdk-dev] [PATCH v4] ip_pipeline: configuration file parser
> cleanup
> 
> clang compilation fails:
>   examples/ip_pipeline/config_parse.c:852:11: error:
>   implicit conversion from enumeration type 'enum
> app_pktq_out_type'
>   to different enumeration type 'enum app_pktq_in_type'

Thanks, Thomas. V5 has been sent fixing this error.


[dpdk-dev] [PATCH] testpmd: fix typo when closing port

2016-06-08 Thread Pablo de Lara
Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Pablo de Lara 
---
 app/test-pmd/testpmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index da74280..2738805 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1515,7 +1515,7 @@ close_port(portid_t pid)

if (rte_atomic16_cmpset(&(port->port_status),
RTE_PORT_HANDLING, RTE_PORT_CLOSED) == 0)
-   printf("Port %d can not be set into stopped\n", pi);
+   printf("Port %d can not be set into closed\n", pi);
}

printf("Done\n");
-- 
2.5.5



[dpdk-dev] [PATCH v5 9/9] doc: announce ABI change for rte_eth_dev_info structure

2016-06-08 Thread Mcnamara, John
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Reshma Pattan
> Sent: Wednesday, June 8, 2016 2:38 PM
> To: dev at dpdk.org
> Cc: Pattan, Reshma 
> Subject: [dpdk-dev] [PATCH v5 9/9] doc: announce ABI change for
> rte_eth_dev_info structure
> 
> New fields nb_rx_queues and nb_tx_queues will be added to rte_eth_dev_info
> structure.
> Changes to API rte_eth_dev_info_get() will be done to update these new
> fields to rte_eth_dev_info object.
> 
> Signed-off-by: Reshma Pattan 
> ---
>  doc/guides/rel_notes/deprecation.rst | 6 ++
>  1 file changed, 6 insertions(+)
> 

Hi,

This isn't required in this patchset since the deprecation has already occurred 
in this release.

John



[dpdk-dev] [PATCH] mbuf: remove inconsistent assert statements

2016-06-08 Thread Olivier Matz
Hi Adrien, Konstantin,

I'm jumping in this (interesting) discussion. I already talked a
bit with Adrien in point to point, and I think its patch is valid.
Please see some comments below.


On 06/08/2016 03:57 PM, Adrien Mazarguil wrote:
> On Wed, Jun 08, 2016 at 01:09:18PM +, Ananyev, Konstantin wrote:
>>>
>>> Hi Konstantin,
>>>
>>> On Wed, Jun 08, 2016 at 10:34:17AM +, Ananyev, Konstantin wrote:
 Hi Adrien,

>
> An assertion failure occurs in __rte_mbuf_raw_free() (called by a few 
> PMDs)
> when compiling DPDK with CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG and starting
> applications with a log level high enough to trigger it.
>
> While rte_mbuf_raw_alloc() sets refcount to 1, __rte_mbuf_raw_free()
> expects it to be 0.
> Considering users are not expected to reset the
> reference count to satisfy assert() and that raw functions are designed on
> purpose without safety belts, remove these checks.

 Yes, it refcnt supposed to be set to 0 by __rte_pktmbuf_prefree_seg().
 Wright now, it is a user responsibility to make sure refcnt==0 before 
 pushing
 mbuf back to the pool.
 Not sure why do you consider that wrong?
>>>
>>> I do not consider this wrong and I'm all for using assert() to catch
>>> programming errors, however in this specific case, I think they are
>>> inconsistent and misleading.
>>
>> Honestly, I don't understand why.
>> Right now the rule of thumb is - when mbuf is in the pool, it's refcnt 
>> should be equal zero.

What is the purpose of this? Is there some code relying on this?

>> Yes, as you pointed below - that rule probably can be changed to: 
>> when mbuf is in the pool, it's refcnt should equal one, and that would 
>> probably allow us
>> to speedup things a bit, but I suppose that's the matter of another 
>> aptch/discussion.
> 
> Agreed.

I agree this is somehow another discussion, but let's dive into :)

But since [1], it is allowed to call rte_mbuf_raw_alloc() in PMDs (all
PMDs were calling an internal function before). We could argue that
rte_mbuf_raw_free() should also be made public for PMDs.

As you said below, no-one promised that the free() reverts the malloc(),
but given the function names, one can legitimately expect that the
following code is valid:

m = __rte_mbuf_raw_alloc();
/* do nothing here */
__rte_mbuf_raw_free(m);

If no code relies on having the refcnt set to 0 when a mbuf is in
the pool, I suggest to relax this constraint as Adrien proposed.

Then, my opinion is that the refcount should be set to 1 in
rte_pktmbuf_reset(). And rte_pktmbuf_free() should not be allowed on
an uninitialized mbuf (yes, this would require some changes in PMDs).
This would open the door for bulk allocation/free in the mbuf api.

This could be done in several steps:

1/ remove these assert(), add introduce a public rte_mbuf_raw_free()
2/ announce that rte_pktmbuf_free() won't work on uninitialized mbufs
in a future version, and ensure that all PMD are inline with this
requirement
3/ later, move refcount initialization in rte_pktmbuf_reset()

What do you think?

Another option is to remove the rte_mbuf_raw_alloc()/rte_mbuf_raw_free()
and use mempool calls. But having a mbuf wrapper does not seem a bad
thing to me.

By the way, __rte_pktmbuf_prefree_seg() is also an internal function.
I think we should try to update the mbuf API so that the PMDs do not
need to call these internal functions.


[1]
http://dpdk.org/browse/dpdk/commit/?id=fbfd99551ca370266f4bfff58ce441cf5cb1203a


Regards,
Olivier


> 
 If the user calls __rte_mbuf_raw_free() manualy it is his responsibility 
 to make
 sure mbuf's refcn==0.
>>>
>>> Sure, however what harm does it cause (besides assert() to fail), since the
>>> allocation function sets refcount to 1?
>>>
>>> Why having the allocation function set the refcount if we are sure it is
>>> already 0 (assert() proves it). Removing rte_mbuf_refcnt_set(m, 1) can
>>> surely improve performance.
>>
>> That's' just an assert() enabled when MBUF_DEBUG  is on.
>> Its sole purpose is to help troubleshoot the bugs and help to catch 
>> situations
>> when someone silently updates mbufs supposed to be free.  
> 
> I perfectly understand and I cannot agree more with this explanation,
> however the fact these functions are not symmetrical remains an issue that
> needs to be addressed somehow in my opinion.
> 
 BTW, why are you doing it?
 The comment clearly states that the function is for internal use:
 /**
  * @internal Put mbuf back into its original mempool.
  * The use of that function is reserved for RTE internal needs.
  * Please use rte_pktmbuf_free().
  *
  * @param m
  *   The mbuf to be freed.
  */
 static inline void __attribute__((always_inline))
 __rte_mbuf_raw_free(struct rte_mbuf *m)
>>>
>>> Several PMDs are using it anyway (won't name names, but I know one of them
>>> quite well).
>>
>> Then it probably is a bug in these PMDs t

[dpdk-dev] [PATCH] mempool: fix local cache initialization

2016-06-08 Thread Sergio Gonzalez Monroy
The mempool local cache is not being initialize properly leading to
undefined behavior in cases where the allocated memory was used and left
with data.

Fixes: af75078fece3 ("first public release")

Signed-off-by: Sergio Gonzalez Monroy 
---
 lib/librte_mempool/rte_mempool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index b54de43..216514c 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -787,7 +787,7 @@ rte_mempool_create_empty(const char *name, unsigned n, 
unsigned elt_size,

/* init the mempool structure */
mp = mz->addr;
-   memset(mp, 0, sizeof(*mp));
+   memset(mp, 0, MEMPOOL_HEADER_SIZE(mp, cache_size));
ret = snprintf(mp->name, sizeof(mp->name), "%s", name);
if (ret < 0 || ret >= (int)sizeof(mp->name)) {
rte_errno = ENAMETOOLONG;
-- 
2.4.11



[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Bruce Richardson
On Wed, Jun 08, 2016 at 03:42:14PM +0200, Thomas Monjalon wrote:
> 2016-06-08 18:13, Jerin Jacob:
> > On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
> > > Hi Jerin,
> > > 
> > > In patch subject, as tag, other drivers are using only driver name, and
> > > Intel drivers also has "driver/base", since base code has some special
> > > case. For thunderx, what do you think about keeping subject as:
> > >  "thunderx: "
> > > 
> > 
> > Hi Ferruh,
> > 
> > We may add crypto or other builtin ThunderX HW accelerated block drivers
> > in future to DPDK.
> > So that is the reason why I thought of keeping the subject as 
> > thunderx/nicvf.
> > If you don't have any objection then I would like to keep it as
> > thunderx/nicvf or just nicvf.
> 
> I don't like the name nicvf but I guess that's the official name?
> 
> Thus I agree the title should be thunderx/nicvf or thunderx_nicvf.

I think I'd prefer the underscore version.

/Bruce


[dpdk-dev] [PATCH] mbuf: remove inconsistent assert statements

2016-06-08 Thread Ananyev, Konstantin
Hi Olivier,

> 
> Hi Adrien, Konstantin,
> 
> I'm jumping in this (interesting) discussion. I already talked a
> bit with Adrien in point to point, and I think its patch is valid.
> Please see some comments below.
> 
> 
> On 06/08/2016 03:57 PM, Adrien Mazarguil wrote:
> > On Wed, Jun 08, 2016 at 01:09:18PM +, Ananyev, Konstantin wrote:
> >>>
> >>> Hi Konstantin,
> >>>
> >>> On Wed, Jun 08, 2016 at 10:34:17AM +, Ananyev, Konstantin wrote:
>  Hi Adrien,
> 
> >
> > An assertion failure occurs in __rte_mbuf_raw_free() (called by a few 
> > PMDs)
> > when compiling DPDK with CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG and starting
> > applications with a log level high enough to trigger it.
> >
> > While rte_mbuf_raw_alloc() sets refcount to 1, __rte_mbuf_raw_free()
> > expects it to be 0.
> > Considering users are not expected to reset the
> > reference count to satisfy assert() and that raw functions are designed 
> > on
> > purpose without safety belts, remove these checks.
> 
>  Yes, it refcnt supposed to be set to 0 by __rte_pktmbuf_prefree_seg().
>  Wright now, it is a user responsibility to make sure refcnt==0 before 
>  pushing
>  mbuf back to the pool.
>  Not sure why do you consider that wrong?
> >>>
> >>> I do not consider this wrong and I'm all for using assert() to catch
> >>> programming errors, however in this specific case, I think they are
> >>> inconsistent and misleading.
> >>
> >> Honestly, I don't understand why.
> >> Right now the rule of thumb is - when mbuf is in the pool, it's refcnt 
> >> should be equal zero.
> 
> What is the purpose of this? Is there some code relying on this?

The whole current implementation of mbuf_free code path relies on that.
Straight here: 
if (likely(NULL != (m = __rte_pktmbuf_prefree_seg(m {
m->next = NULL;
__rte_mbuf_raw_free(m);
}

If we'll exclude indirect mbuf logic, all it does:
if (rte_mbuf_refcnt_update(m, -1) == 0) {
m->next = NULL;
__rte_mbuf_raw_free(m);
}

I.E.:
decrement mbuf->refcnt.
If new value of refcnt is zero, then put it back into the pool.

So having ASERT(mbuf->refcnt==0) inside
__rte_mbuf_raw_free()/__rte_mbuf_raw_alloc()
looks absolutely valid to me.
I *has* to be zero at that point with current implementation,
And if it is not then we probably have (or will have) a silent memory 
corruption.

> 
> >> Yes, as you pointed below - that rule probably can be changed to:
> >> when mbuf is in the pool, it's refcnt should equal one, and that would 
> >> probably allow us
> >> to speedup things a bit, but I suppose that's the matter of another 
> >> aptch/discussion.
> >
> > Agreed.
> 
> I agree this is somehow another discussion, but let's dive into :)

OK :)

> 
> But since [1], it is allowed to call rte_mbuf_raw_alloc() in PMDs (all
> PMDs were calling an internal function before).

Yes, raw_alloc is public, NP with that.

> We could argue that
> rte_mbuf_raw_free() should also be made public for PMDs.

We could, but right now it is not.
Again, as I said, user could use it on his own but he obviously has to
obey the rules and do manually what __rte_pktmbuf_prefree_seg() does.
At least:

rte_mbuf_refcnt_set(m, 0);
__rte_mbuf_raw_free(m);

> 
> As you said below, no-one promised that the free() reverts the malloc(),
> but given the function names, one can legitimately expect that the
> following code is valid:
> 
> m = __rte_mbuf_raw_alloc();
> /* do nothing here */
> __rte_mbuf_raw_free(m);

Surely people could (and would) expect various things...
But the reality right now is: __rte_mbuf_raw_free() is an internal
function and not counterpart of __rte_mbuf_raw_alloc().
If the people don't bother to read API docs or/and the actual code,
I can't see how we can help them :)

> 
> If no code relies on having the refcnt set to 0 when a mbuf is in
> the pool, I suggest to relax this constraint as Adrien proposed.

Why not just rename it to __rte_mbuf_raw_free_dont_use_after_raw_alloc()?
To avoid any further confusion :)
Seriously speaking I would prefer to leave it as it is.
If you feel we have to introduce a counterpart of  rte_mbuf_raw_alloc(),
we can make a new public one:

rte_mbuf_raw_free(stuct rte_mbuf *m)
{
  if (rte_mbuf_refcnt_update(m, -1) == 0)
__rte_mbuf_raw_free(m);
}

> 
> Then, my opinion is that the refcount should be set to 1 in
> rte_pktmbuf_reset().

I don't think we need to update rte_pktmbuf_reset(),
it doesn't touch refcnt at all and probably better to keep it that way.
To achieve what you suggesting, we probably need to:
1) update  _rte_pktmbuf_prefree_seg and rte_pktmbuf_detach() to
set refcnt back to 1, i.e:

static inline struct rte_mbuf* __attribute__((always_inline))
__rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
{
__rte_mbuf_sanity_check(m, 0);

if (likely(rte_mbuf_refcnt_update(m, -1) == 0)) {
/* if this is an indirect mbuf, it is

[dpdk-dev] [PATCH v5 8/9] doc: update doc for packet capture framework

2016-06-08 Thread Mcnamara, John


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Reshma Pattan
> Sent: Wednesday, June 8, 2016 2:38 PM
> To: dev at dpdk.org
> Cc: Pattan, Reshma 
> Subject: [dpdk-dev] [PATCH v5 8/9] doc: update doc for packet capture
> framework
> 
> Added programmers guide for librte_pdump.
> Added sample application guide for app/pdump application.
> Updated release note for packet capture framework changes.
> 
> Signed-off-by: Reshma Pattan 

Acked-by: John McNamara 



[dpdk-dev] [PATCH] mbuf: remove inconsistent assert statements

2016-06-08 Thread Adrien Mazarguil
On Wed, Jun 08, 2016 at 01:09:18PM +, Ananyev, Konstantin wrote:
> > 
> > Hi Konstantin,
> > 
> > On Wed, Jun 08, 2016 at 10:34:17AM +, Ananyev, Konstantin wrote:
> > > Hi Adrien,
> > >
> > > >
> > > > An assertion failure occurs in __rte_mbuf_raw_free() (called by a few 
> > > > PMDs)
> > > > when compiling DPDK with CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG and starting
> > > > applications with a log level high enough to trigger it.
> > > >
> > > > While rte_mbuf_raw_alloc() sets refcount to 1, __rte_mbuf_raw_free()
> > > > expects it to be 0.
> > > >Considering users are not expected to reset the
> > > > reference count to satisfy assert() and that raw functions are designed 
> > > > on
> > > > purpose without safety belts, remove these checks.
> > >
> > > Yes, it refcnt supposed to be set to 0 by __rte_pktmbuf_prefree_seg().
> > > Wright now, it is a user responsibility to make sure refcnt==0 before 
> > > pushing
> > > mbuf back to the pool.
> > > Not sure why do you consider that wrong?
> > 
> > I do not consider this wrong and I'm all for using assert() to catch
> > programming errors, however in this specific case, I think they are
> > inconsistent and misleading.
> 
> Honestly, I don't understand why.
> Right now the rule of thumb is - when mbuf is in the pool, it's refcnt should 
> be equal zero.
> Yes, as you pointed below - that rule probably can be changed to: 
> when mbuf is in the pool, it's refcnt should equal one, and that would 
> probably allow us
> to speedup things a bit, but I suppose that's the matter of another 
> aptch/discussion.

Agreed.

> > > If the user calls __rte_mbuf_raw_free() manualy it is his responsibility 
> > > to make
> > > sure mbuf's refcn==0.
> > 
> > Sure, however what harm does it cause (besides assert() to fail), since the
> > allocation function sets refcount to 1?
> > 
> > Why having the allocation function set the refcount if we are sure it is
> > already 0 (assert() proves it). Removing rte_mbuf_refcnt_set(m, 1) can
> > surely improve performance.
> 
> That's' just an assert() enabled when MBUF_DEBUG  is on.
> Its sole purpose is to help troubleshoot the bugs and help to catch situations
> when someone silently updates mbufs supposed to be free.  

I perfectly understand and I cannot agree more with this explanation,
however the fact these functions are not symmetrical remains an issue that
needs to be addressed somehow in my opinion.

> > > BTW, why are you doing it?
> > > The comment clearly states that the function is for internal use:
> > > /**
> > >  * @internal Put mbuf back into its original mempool.
> > >  * The use of that function is reserved for RTE internal needs.
> > >  * Please use rte_pktmbuf_free().
> > >  *
> > >  * @param m
> > >  *   The mbuf to be freed.
> > >  */
> > > static inline void __attribute__((always_inline))
> > > __rte_mbuf_raw_free(struct rte_mbuf *m)
> > 
> > Several PMDs are using it anyway (won't name names, but I know one of them
> > quite well).
> 
> Then it probably is a bug in these PMDs that need to be fixed.
> 
> > I chose to modify this code instead of its users for the
> > following reasons:
> > 
> > - Considering their names, these functions should be opposites and able to
> >   work together like malloc()/free().
> 
> These are internal functions.
> Comments in mbuf clearly state that library users shouldn't call them 
> directly.
> They are written to fit internal librte_mbuf needs, and no-one ever promised
> malloc/free() compatibility here. 

So it cannot be provided for the sake of not providing it or is there a good
reason?

What I meant is that since PMDs already made the mistake of using these
functions to benefit from the improved performance, DPDK being all about
performance and stuff, let them use it as intended. Perhaps we should drop
those "__" like for rte_mbuf_raw_alloc().

> > 
> > - PMDs are relying on these functions for performance reasons, we can assume
> >   they took the extra care necessary to make sure it would work properly.
> 
> That just doesn't seem correct to me.
> The proper way to do free fo mbuf segment is:
> 
> static inline void __attribute__((always_inline))
> rte_pktmbuf_free_seg(struct rte_mbuf *m)
> {
> if (likely(NULL != (m = __rte_pktmbuf_prefree_seg(m {
> m->next = NULL;
> __rte_mbuf_raw_free(m);
> }
> }
> 
> If by some reason you choose not to use this function, then it is your
> responsibility to perform similar actions on your own before pushing mbuf 
> into the pool.
> That's what some TX functions for some Intel NICs do to improve performance:
> they call _prefree_seg() manually and try to put mbufs into the pool in 
> groups.

Not anymore it seems, but in the current code base both ena and mpipe PMDs
(soon mlx5 as well) seem to get this wrong.

> > - Preventing it would make these PMDs slower and is not acceptable either.
> 
> I can hardly imagine that __rte_pktmbuf_prefree_seg() impact would be that 
> severe...

[dpdk-dev] [PATCH] app/test: reduced duration of red_autotest

2016-06-08 Thread Kantecki, Tomasz
Hi Thomas,

> > 12s is long for a functional test. We are used to have tests of less than 
> > 1s.
> > Do you think this magnitude order can be achieved for red tests?
> 
> This is a real challenge :) Let me take another look into it.

I had a 2nd look into this test suite and reducing time for functional tests is 
doable but labor intensive. Let me explain:
RED/WRED algorithm is time based (packet queue events in time). For tracking 
time the test uses TSC (x86 time stamp counter). This could be replaced with 
some fake time stamp in functional tests and result in significant reduction of 
execution time. It needs couple of days to carefully replace TSC with a fake 
time stamp solution without breaking test logic.

Alternatively, one or two more test cases can be removed from the fast_test. A 
few experiments with this option give me test duration at 5[s] level. I guess 
this too much off from the 1[s] target.

Regards,
Tomasz


--
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.



[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Thomas Monjalon
2016-06-08 19:02, Jerin Jacob:
> On Wed, Jun 08, 2016 at 02:22:55PM +0100, Bruce Richardson wrote:
> > On Wed, Jun 08, 2016 at 06:13:21PM +0530, Jerin Jacob wrote:
> > > On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
> > > > On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> > > > Hi Jerin,
> > > > 
> > > > In patch subject, as tag, other drivers are using only driver name, and
> > > > Intel drivers also has "driver/base", since base code has some special
> > > > case. For thunderx, what do you think about keeping subject as:
> > > >  "thunderx: "
> > > > 
> > > 
> > > Hi Ferruh,
> > > 
> > > We may add crypto or other builtin ThunderX HW accelerated block drivers
> > > in future to DPDK.
> > > So that is the reason why I thought of keeping the subject as 
> > > thunderx/nicvf.
> > > If you don't have any objection then I would like to keep it as
> > > thunderx/nicvf or just nicvf.
> > 
> > Are you upstreaming kernel modules for this device? If so, what is the Linux
> > kernel module-name for this device going to be, as perhaps that can help us
> > here?
> 
> Yes, Kernel module has been upstreamed.
> the commit log in linux kernel is "net: thunderx: ..."

If you want to modify the conventions, we just need to agree on a patch
modifying the guidelines.
We can think about the proposal of Ferruh to use net/ and crypto/ prefixes.

The most important is to have something short and easy to parse when
quick browsing the git history.


[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Thomas Monjalon
2016-06-08 18:13, Jerin Jacob:
> On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
> > Hi Jerin,
> > 
> > In patch subject, as tag, other drivers are using only driver name, and
> > Intel drivers also has "driver/base", since base code has some special
> > case. For thunderx, what do you think about keeping subject as:
> >  "thunderx: "
> > 
> 
> Hi Ferruh,
> 
> We may add crypto or other builtin ThunderX HW accelerated block drivers
> in future to DPDK.
> So that is the reason why I thought of keeping the subject as thunderx/nicvf.
> If you don't have any objection then I would like to keep it as
> thunderx/nicvf or just nicvf.

I don't like the name nicvf but I guess that's the official name?

Thus I agree the title should be thunderx/nicvf or thunderx_nicvf.


[dpdk-dev] [PATCH] mk: generate internal library dependencies from DEPDIRS-y automatically

2016-06-08 Thread Thomas Monjalon
2016-06-08 14:34, Olivier Matz:
> On 06/07/2016 04:40 PM, Wiles, Keith wrote:
> > On 6/7/16, 9:19 AM, "dev on behalf of Thomas Monjalon"  > dpdk.org on behalf of thomas.monjalon at 6wind.com> wrote:
> > 
> >> 2016-06-07 15:07, Bruce Richardson:
> >>> On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
>  2016-06-07 14:36, Christian Ehrhardt:
> > But I still struggle to see how to fix the circular dependency between
> > librte_eal and librte_mempool.
> 
>  Why is there a circular dependency?
>  Only because of logs using mempool?
> 
> > Maybe now is a time to look at this part of the original threads again 
> > to
> > eventually get apps less overlinked?
> > => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> > My naive suggestions in generalized form can be found there (no answer 
> > yet):
> > =>
> > http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries
> 
>  I would prefer removing the circular dependency.
>  Maybe we can rewrite the code to not use mempool or move it outside of 
>  EAL.
> 
> Indeed, mempools are used in eal for history. Is this feature still
> useful now that logs are sent to syslog? Maybe we could deprecate this
> API, and remove mempool calls in a future release?

+1 to deprecate log history

> >>> Or else we can take the attitude that the mempools and the rings are just 
> >>> a core
> >>> part of DPDK and move them and the EAL into a dpdk_core library at link 
> >>> time.
> >>> Having the code separate in the git tree is good, but I'm not sure having
> >>> the resulting object files being in separate .a/.so files is particularly 
> >>> useful.
> >>> I can't see someone wanting to use one without the other.
> >>
> >> EAL could be used as an abstraction layer on top of systems and platforms.
> >> And I think keeping things separated and layered help to maintain a design
> >> easy to understand.
> 
> I like the idea to have one lib per directory (for consistency). It
> may also simplify the Makefiles.
> I'm in favor of keeping mempool and ring separated from eal if we
> can remove the circular dep.

+1 to keep bijectivity in src/lib.


[dpdk-dev] [PATCH v2] eal: out-of-bounds write

2016-06-08 Thread Sergio Gonzalez Monroy

I missed this patch at the time!

On 27/04/2016 12:29, Slawomir Mrozowicz wrote:
> Fix issue reported by Coverity.
>
> Coverity ID 13282: Out-of-bounds write
> overrun-local: Overrunning array mcfg->memseg of 256 44-byte elements
> at element index 257 using index j.
>
> Fixes: af75078fece3 ("first public release")
>
> Signed-off-by: Slawomir Mrozowicz 
> ---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 5 -
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c 
> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 5b9132c..715bd52 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1333,8 +1333,11 @@ rte_eal_hugepage_init(void)
>   
>   if (new_memseg) {
>   j += 1;
> - if (j == RTE_MAX_MEMSEG)
> + if (j >= RTE_MAX_MEMSEG) {
> + RTE_LOG(ERR, EAL,
> + "Failed: memseg reached 
> RTE_MAX_MEMSEG\n");
>   break;
> + }
>   
>   mcfg->memseg[j].phys_addr = hugepage[i].physaddr;
>   mcfg->memseg[j].addr = hugepage[i].final_va;

As Bruce was suggesting in his comment to the v1, it's more helpful to 
do a check before the loop
and print a message distinguishing the error case, something along the 
lines of: "all memsegs
used by ivshmem. Please either increase", returning with -ENOMEM error.

Sergio


[dpdk-dev] [PATCH] app/test: fix bond device name too long

2016-06-08 Thread Thomas Monjalon
2016-06-08 11:50, Jastrzebski, MichalX K:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-05-27 18:38, Thomas Monjalon:
> > > 2016-05-27 17:20, Michal Jastrzebski:
> > > > Bond device name was too long (grather than 32 signs) that
> > > > cause mempool allocation to fail.
> > >
> > > Maybe that this kind of failure would be avoided if the test
> > > was added to autotests (app/test/autotest_data.py).
> > >
> > > Generally speaking, it would be a good idea to make an audit
> > > on which tests are missing in "make fast_test" and "make test".
> > 
> > Any comment please?
> 
> Hi Thomas,
> 
> There is a small timeout in test_tlb_tx_burst - big burst has to be generated 
> to
> detect balancing and small timeout has to be included between each burst, 
> thus I am not sure if link_bonding_autotest can be classified to fast tests 
> (test takes about 3-4 seconds).
> We can add this test to autotests script for which time is not so critical.

The bug we see here could be detected by just initializing bonding.
Maybe we can consider having some basic/fast tests and others longer.


[dpdk-dev] QoS grinder vs pipe wrr_tokens

2016-06-08 Thread Dumitrescu, Cristian
Hi Alexey,

The WRR context is compressed to use less memory footprint in order to fit the 
entire pipe run-time context (struct rte_sched_pipe) into a single cache line 
for performance reasons. Basically we trade WRR accuracy for performance.

For some typical Telco use-cases, the WRR/WFQ accuracy for the traffic class 
queues is not that important, as usually the traffic class queue weight ratio 
is big, e.g. 1:4:20. Basically, whether the actual observed ratio at run-time 
is 1:4:20 or 1:5:18 or 1:3:22 is not that important, as the intention really is 
to source most of the traffic from the queue with the largest weight, some 
traffic from the queue with the medium weight and not starve the lowest weight 
queue; this mode is very similar to strict priority between traffic class 
queues, with the exception that the lowest priority queues are not starved for 
long time.

When WRR accuracy is more important than performance, this operation should be 
disabled.

Regards,
Cristian


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Alexey
> Bogdanenko
> Sent: Tuesday, June 7, 2016 8:29 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] QoS grinder vs pipe wrr_tokens
> 
> Hello,
> 
> I have a question regarding QoS grinder implementation, specifically,
> about the way queue WRR tokens are copied from pipe to grinder and back.
> 
> First, rte_sched_grinder uses uint16_t and rte_sched_pipe uses uint8_t
> to represent wrr_tokens. Second, instead of just copying the tokens, we
> shift bits by RTE_SCHED_WRR_SHIFT.
> 
> What does it accomplish? Can it lead to lower scheduler accuracy due to
> a round-off error?
> 
> version: v16.04
> 
> Thanks,
> 
> Alexey Bogdanenko


[dpdk-dev] about rx checksum flags

2016-06-08 Thread Olivier Matz
Hi,

On 06/08/2016 10:22 AM, Chandran, Sugesh wrote:
>>> I guess the IP checksum also important as L4. In some cases, UDP
>>> checksum is zero and no need to validate it. But Ip checksum is
>>> present on all the packets and that must be validated all  the time.
>>> At higher packet rate, the ip checksum offload can offer slight performance
>> improvement. What do you think??
>>>
>>
>> Agree, in some situations (and this is even more true with packet types /
>> smartnics), the application could process without accessing the packet data 
>> if
>> we keep the IP cksum flags.
> [Sugesh] True, If that's the case, Will you considering to implement IP
> checksum flags as well along with L4?
> As you said , this will be useful when we offload packet lookup itself into 
> the NICs(May be
> when using Flow director) ? 

Yes, I plan to implement the same rx status flags (good, bad, unknown,
none) for rx IP checksum too.

Regards,
Olivier


[dpdk-dev] [PATCH v3 08/20] vhost: introduce new API to export numa node

2016-06-08 Thread Rich Lane
On Mon, Jun 6, 2016 at 8:51 PM, Yuanhan Liu 
wrote:
>
> @@ -248,14 +248,9 @@ new_device(struct virtio_net *dev)
> internal = eth_dev->data->dev_private;
>
>  #ifdef RTE_LIBRTE_VHOST_NUMA
> -   ret  = get_mempolicy(&newnode, NULL, 0, dev,
> -   MPOL_F_NODE | MPOL_F_ADDR);
> -   if (ret < 0) {
> -   RTE_LOG(ERR, PMD, "Unknown numa node\n");
> -   return -1;
> -   }
> -
> -   eth_dev->data->numa_node = newnode;
> +   newnode = rte_vhost_get_numa_node(dev->vid);
> +   if (newnode > 0)
>

Should be "newnode >= 0".

+   eth_dev->data->numa_node = newnode;
>  #endif
>


[dpdk-dev] [PATCH v5 9/9] doc: announce ABI change for rte_eth_dev_info structure

2016-06-08 Thread Reshma Pattan
New fields nb_rx_queues and nb_tx_queues will be added to
rte_eth_dev_info structure.
Changes to API rte_eth_dev_info_get() will be done to update
these new fields to rte_eth_dev_info object.

Signed-off-by: Reshma Pattan 
---
 doc/guides/rel_notes/deprecation.rst | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index ad05eba..04316fb 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -57,3 +57,9 @@ Deprecation Notices
   a handle, like the way kernel exposes an fd to user for locating a
   specific file, and to keep all major structures internally, so that
   we are likely to be free from ABI violations in future.
+
+* A librte_ether public structure ``rte_eth_dev_info`` will be changed in 
16.07.
+  The proposed change will add new parameters ``nb_rx_queues``, 
``nb_tx_queues``
+  to the structure. These are the number of queues configured by software.
+  Modification to definition of ``rte_eth_dev_info_get()`` will be done
+  to update new parameters to ``rte_eth_dev_info`` object.
-- 
2.5.0



[dpdk-dev] [PATCH v5 8/9] doc: update doc for packet capture framework

2016-06-08 Thread Reshma Pattan
Added programmers guide for librte_pdump.
Added sample application guide for app/pdump application.
Updated release note for packet capture framework changes.

Signed-off-by: Reshma Pattan 
---
 MAINTAINERS |   3 +
 doc/guides/prog_guide/index.rst |   1 +
 doc/guides/prog_guide/pdump_library.rst | 107 
 doc/guides/rel_notes/release_16_07.rst  |  13 
 doc/guides/sample_app_ug/index.rst  |   1 +
 doc/guides/sample_app_ug/pdump.rst  | 122 
 6 files changed, 247 insertions(+)
 create mode 100644 doc/guides/prog_guide/pdump_library.rst
 create mode 100644 doc/guides/sample_app_ug/pdump.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index a48c8de..ce7c941 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -436,6 +436,9 @@ Pdump
 M: Reshma Pattan 
 F: lib/librte_pdump/
 F: app/pdump/
+F: doc/guides/prog_guide/pdump_library.rst
+F: doc/guides/sample_app_ug/pdump.rst
+

 Hierarchical scheduler
 M: Cristian Dumitrescu 
diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index b862d0c..4caf969 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -71,6 +71,7 @@ Programmer's Guide
 writing_efficient_code
 profile_app
 glossary
+pdump_library


 **Figures**
diff --git a/doc/guides/prog_guide/pdump_library.rst 
b/doc/guides/prog_guide/pdump_library.rst
new file mode 100644
index 000..1809234
--- /dev/null
+++ b/doc/guides/prog_guide/pdump_library.rst
@@ -0,0 +1,107 @@
+..  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 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 Intel Corporation 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.
+
+.. _pdump_library:
+
+The librte_pdump Library
+
+
+The ``librte_pdump`` library provides a framework for packet capturing in DPDK.
+The library provides the following APIs to initialize the packet capture 
framework, to enable
+or disable the packet capture, and to uninitialize it:
+
+* ``rte_pdump_init()``:
+  This API initializes the packet capture framework.
+
+* ``rte_pdump_enable()``:
+  This API enables the packet capture on a given port and queue.
+  Note: The filter option in the API is a place holder for future enhancements.
+
+* ``rte_pdump_enable_by_deviceid()``:
+  This API enables the packet capture on a given device id (``vdev name or pci 
address``) and queue.
+  Note: The filter option in the API is a place holder for future enhancements.
+
+* ``rte_pdump_disable()``:
+  This API disables the packet capture on a given port and queue.
+
+* ``rte_pdump_disable_by_deviceid()``:
+  This API disables the packet capture on a given device id (``vdev name or 
pci address``) and queue.
+
+* ``rte_pdump_uninit()``:
+  This API uninitializes the packet capture framework.
+
+
+Operation
+-
+
+The ``librte_pdump`` library works on a client/server model. The server is 
responsible for enabling or
+disabling the packet capture and the clients are responsible for requesting 
the enabling or disabling of
+the packet capture.
+
+The packet capture framework, as part of its initialization, creates the 
pthread and the server socket in
+the pthread. The application that calls the framework initialization first 
will have the server socket created.
+Further calls to the framework initialization by the same application or other 
applications is not allowed i.e., only
+one server socket is allowed on the system. So the other applications ca

[dpdk-dev] [PATCH v5 7/9] app/test-pmd: add pdump initialization uninitialization

2016-06-08 Thread Reshma Pattan
Call rte_pdump_init and rte_pdump_uninit for packet
capturing initialization and uninitialization.

Signed-off-by: Reshma Pattan 
---
 app/test-pmd/testpmd.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9d11830..645bf50 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -75,6 +75,7 @@
 #ifdef RTE_LIBRTE_PMD_XENVIRT
 #include 
 #endif
+#include 

 #include "testpmd.h"

@@ -2023,6 +2024,8 @@ signal_handler(int signum)
if (signum == SIGINT || signum == SIGTERM) {
printf("\nSignal %d received, preparing to exit...\n",
signum);
+   /* uninitialize packet capture framework */
+   rte_pdump_uninit();
force_quit();
/* exit with the expected status */
signal(signum, SIG_DFL);
@@ -2043,6 +2046,9 @@ main(int argc, char** argv)
if (diag < 0)
rte_panic("Cannot init EAL\n");

+   /* initialize packet capture framework */
+   rte_pdump_init();
+
nb_ports = (portid_t) rte_eth_dev_count();
if (nb_ports == 0)
RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
-- 
2.5.0



[dpdk-dev] [PATCH v5 6/9] app/pdump: add pdump tool for packet capturing

2016-06-08 Thread Reshma Pattan
New tool added for packet capturing on dpdk.
This tool supports command line options.
This tool runs as secondary process by default.

Command line supports various parameters to capture
the packets.

User should pass on a)port and queue (or) b)pci address
and queue (or) c)device name and queue to capture
the packets.

Users also need to pass on either pcap file name or
any linux iface, on to which packets captured from dpdk
ports will be sent on for the users to view using tcpdump.

Users have option to capture packets either a) in RX
direction, b)(or) in TX direction c)(or) from both the
directions.

User can pass on ring_size and mempool parameters using
command line, but these are optional parameters.
These are used to create ring and mempool objects for packet
mirroring from primary application to tool. If user doesn't
provide any values, default values will be used internally
for the creation of the ring and mempool.

Signed-off-by: Reshma Pattan 
---
 MAINTAINERS|   1 +
 app/Makefile   |   1 +
 app/pdump/Makefile |  45 +++
 app/pdump/main.c   | 814 +
 4 files changed, 861 insertions(+)
 create mode 100644 app/pdump/Makefile
 create mode 100644 app/pdump/main.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cc3ffdb..a48c8de 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -435,6 +435,7 @@ F: doc/guides/sample_app_ug/packet_ordering.rst
 Pdump
 M: Reshma Pattan 
 F: lib/librte_pdump/
+F: app/pdump/

 Hierarchical scheduler
 M: Cristian Dumitrescu 
diff --git a/app/Makefile b/app/Makefile
index 1151e09..c593efa 100644
--- a/app/Makefile
+++ b/app/Makefile
@@ -37,5 +37,6 @@ DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test-pipeline
 DIRS-$(CONFIG_RTE_TEST_PMD) += test-pmd
 DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += cmdline_test
 DIRS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += proc_info
+DIRS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += pdump

 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/app/pdump/Makefile b/app/pdump/Makefile
new file mode 100644
index 000..96bb4af
--- /dev/null
+++ b/app/pdump/Makefile
@@ -0,0 +1,45 @@
+#   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 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 Intel Corporation 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
+
+APP = dpdk_pdump
+
+CFLAGS += $(WERROR_FLAGS)
+
+# all source are stored in SRCS-y
+
+SRCS-y := main.c
+
+# this application needs libraries first
+DEPDIRS-y += lib
+
+include $(RTE_SDK)/mk/rte.app.mk
diff --git a/app/pdump/main.c b/app/pdump/main.c
new file mode 100644
index 000..a4a5ca2
--- /dev/null
+++ b/app/pdump/main.c
@@ -0,0 +1,814 @@
+/*
+ *   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 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 Intel Corporation nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from t

[dpdk-dev] [PATCH v5 5/9] lib/librte_pdump: add new library for packet capturing support

2016-06-08 Thread Reshma Pattan
Added new library for packet capturing support.

Added public api rte_pdump_init, applications should call
this as part of their application setup to have packet
capturing framework ready.

Added public api rte_pdump_uninit to uninitialize the packet
capturing framework.

Added public apis rte_pdump_enable and rte_pdump_disable to
enable and disable packet capturing on specific port and queue.

Added public apis rte_pdump_enable_by_deviceid and
rte_pdump_disable_by_deviceid to enable and disable packet
capturing on a specific device (pci address or name) and queue.

Signed-off-by: Reshma Pattan 
---
 MAINTAINERS|   4 +
 config/common_base |   5 +
 lib/Makefile   |   1 +
 lib/librte_pdump/Makefile  |  55 +++
 lib/librte_pdump/rte_pdump.c   | 841 +
 lib/librte_pdump/rte_pdump.h   | 186 
 lib/librte_pdump/rte_pdump_version.map |  12 +
 mk/rte.app.mk  |   1 +
 8 files changed, 1105 insertions(+)
 create mode 100644 lib/librte_pdump/Makefile
 create mode 100644 lib/librte_pdump/rte_pdump.c
 create mode 100644 lib/librte_pdump/rte_pdump.h
 create mode 100644 lib/librte_pdump/rte_pdump_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index 3e8558f..cc3ffdb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -432,6 +432,10 @@ F: app/test/test_reorder*
 F: examples/packet_ordering/
 F: doc/guides/sample_app_ug/packet_ordering.rst

+Pdump
+M: Reshma Pattan 
+F: lib/librte_pdump/
+
 Hierarchical scheduler
 M: Cristian Dumitrescu 
 F: lib/librte_sched/
diff --git a/config/common_base b/config/common_base
index 47c26f6..a2d5d72 100644
--- a/config/common_base
+++ b/config/common_base
@@ -484,6 +484,11 @@ CONFIG_RTE_LIBRTE_DISTRIBUTOR=y
 CONFIG_RTE_LIBRTE_REORDER=y

 #
+# Compile the pdump library
+#
+CONFIG_RTE_LIBRTE_PDUMP=y
+
+#
 # Compile librte_port
 #
 CONFIG_RTE_LIBRTE_PORT=y
diff --git a/lib/Makefile b/lib/Makefile
index f254dba..ca7c02f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -57,6 +57,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_PORT) += librte_port
 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

 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
new file mode 100644
index 000..af81a28
--- /dev/null
+++ b/lib/librte_pdump/Makefile
@@ -0,0 +1,55 @@
+#   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 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 Intel Corporation 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_pdump.a
+
+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
+CFLAGS += -D_GNU_SOURCE
+
+EXPORT_MAP := rte_pdump_version.map
+
+LIBABIVER := 1
+
+# all source are stored in SRCS-y
+SRCS-$(CONFIG_RTE_LIBRTE_PDUMP) := rte_pdump.c
+
+# install this header file
+SYMLINK-$(CONFIG_RTE_LIBRTE_PDUMP)-include := rte_pdump.h
+
+# this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += lib/librte_mbuf
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += lib/librte_ether
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
new file mode 100644
index 000..4

[dpdk-dev] [PATCH v5 4/9] librte_ether: make rte_eth_dev_get_port_by_name rte_eth_dev_get_name_by_port public

2016-06-08 Thread Reshma Pattan
Converted rte_eth_dev_get_port_by_name to a public API.
Converted rte_eth_dev_get_name_by_port to a public API.

Signed-off-by: Reshma Pattan 
---
 lib/librte_ether/rte_ethdev.c  |  4 ++--
 lib/librte_ether/rte_ethdev.h  | 29 +
 lib/librte_ether/rte_ether_version.map |  2 ++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 4d732e8..d8aac99 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -406,7 +406,7 @@ rte_eth_dev_get_addr_by_port(uint8_t port_id, struct 
rte_pci_addr *addr)
return 0;
 }

-static int
+int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 {
char *tmp;
@@ -425,7 +425,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
return 0;
 }

-static int
+int
 rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 {
int i;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 106318f..6f5a61a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -4283,6 +4283,35 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
  uint32_t mask,
  uint8_t en);

+/**
+* Get the port id from pci adrress or device name
+* Ex: :2:00.0 or vdev name eth_pcap0
+*
+* @param name
+*  pci address or name of the device
+* @param port_id
+*   pointer to port identifier of the device
+* @return
+*   - (0) if successful.
+*   - (-ENODEV or -EINVAL) on failure.
+*/
+int
+rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id);
+
+/**
+* Get the device name from port id
+*
+* @param port_id
+*   pointer to port identifier of the device
+* @param name
+*  pci address or name of the device
+* @return
+*   - (0) if successful.
+*   - (-EINVAL) on failure.
+*/
+int
+rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_ether/rte_ether_version.map 
b/lib/librte_ether/rte_ether_version.map
index d06d648..73e730d 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -137,5 +137,7 @@ DPDK_16.07 {
global:

rte_eth_add_first_rx_callback;
+   rte_eth_dev_get_name_by_port;
+   rte_eth_dev_get_port_by_name;
rte_eth_dev_info_get;
 } DPDK_16.04;
-- 
2.5.0



[dpdk-dev] [PATCH v5 3/9] librte_ether: add new fields to rte_eth_dev_info struct

2016-06-08 Thread Reshma Pattan
New fields nb_rx_queues and nb_tx_queues are added to
rte_eth_dev_info structure.
Changes to API rte_eth_dev_info_get() are done to update
these new fields to rte_eth_dev_info object.

Signed-off-by: Reshma Pattan 
---
 lib/librte_ether/rte_ethdev.c  | 2 ++
 lib/librte_ether/rte_ethdev.h  | 3 +++
 lib/librte_ether/rte_ether_version.map | 1 +
 3 files changed, 6 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f13c70a..4d732e8 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1661,6 +1661,8 @@ rte_eth_dev_info_get(uint8_t port_id, struct 
rte_eth_dev_info *dev_info)
(*dev->dev_ops->dev_infos_get)(dev, dev_info);
dev_info->pci_dev = dev->pci_dev;
dev_info->driver_name = dev->data->drv_name;
+   dev_info->nb_rx_queues = dev->data->nb_rx_queues;
+   dev_info->nb_tx_queues = dev->data->nb_tx_queues;
 }

 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 92b07a9..106318f 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -882,6 +882,9 @@ struct rte_eth_dev_info {
struct rte_eth_desc_lim rx_desc_lim;  /**< RX descriptors limits */
struct rte_eth_desc_lim tx_desc_lim;  /**< TX descriptors limits */
uint32_t speed_capa;  /**< Supported speeds bitmap (ETH_LINK_SPEED_). */
+   /** Configured number of rx/tx queues */
+   uint16_t nb_rx_queues; /**< Number of RX queues. */
+   uint16_t nb_tx_queues; /**< Number of TX queues. */
 };

 /**
diff --git a/lib/librte_ether/rte_ether_version.map 
b/lib/librte_ether/rte_ether_version.map
index c990b04..d06d648 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -137,4 +137,5 @@ DPDK_16.07 {
global:

rte_eth_add_first_rx_callback;
+   rte_eth_dev_info_get;
 } DPDK_16.04;
-- 
2.5.0



[dpdk-dev] [PATCH v5 2/9] librte_ether: add new api rte_eth_add_first_rx_callback

2016-06-08 Thread Reshma Pattan
Added new public api rte_eth_add_first_rx_callback to add given
callback as head of list.

Signed-off-by: Reshma Pattan 
---
 lib/librte_ether/rte_ethdev.c  | 35 ++
 lib/librte_ether/rte_ethdev.h  | 27 ++
 lib/librte_ether/rte_ether_version.map |  6 ++
 3 files changed, 68 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ce70d58..f13c70a 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2939,6 +2939,41 @@ rte_eth_add_rx_callback(uint8_t port_id, uint16_t 
queue_id,
 }

 void *
+rte_eth_add_first_rx_callback(uint8_t port_id, uint16_t queue_id,
+   rte_rx_callback_fn fn, void *user_param)
+{
+#ifndef RTE_ETHDEV_RXTX_CALLBACKS
+   rte_errno = ENOTSUP;
+   return NULL;
+#endif
+   /* check input parameters */
+   if (!rte_eth_dev_is_valid_port(port_id) || fn == NULL ||
+   queue_id >= 
rte_eth_devices[port_id].data->nb_rx_queues) {
+   rte_errno = EINVAL;
+   return NULL;
+   }
+
+   struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
+
+   if (cb == NULL) {
+   rte_errno = ENOMEM;
+   return NULL;
+   }
+
+   cb->fn.rx = fn;
+   cb->param = user_param;
+
+   rte_spinlock_lock(&rte_eth_rx_cb_lock);
+   /* Add the callbacks at fisrt position*/
+   cb->next = rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
+   rte_smp_wmb();
+   rte_eth_devices[port_id].post_rx_burst_cbs[queue_id] = cb;
+   rte_spinlock_unlock(&rte_eth_rx_cb_lock);
+
+   return cb;
+}
+
+void *
 rte_eth_add_tx_callback(uint8_t port_id, uint16_t queue_id,
rte_tx_callback_fn fn, void *user_param)
 {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 2757510..92b07a9 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3825,6 +3825,33 @@ int rte_eth_dev_get_dcb_info(uint8_t port_id,
 void *rte_eth_add_rx_callback(uint8_t port_id, uint16_t queue_id,
rte_rx_callback_fn fn, void *user_param);

+/*
+* Add a callback that must be called first on packet RX on a given port and 
queue.
+*
+* This API configures a first function to be called for each burst of
+* packets received on a given NIC port queue. The return value is a pointer
+* that can be used to later remove the callback using
+* rte_eth_remove_rx_callback().
+*
+* Multiple functions are called in the order that they are added.
+*
+* @param port_id
+*   The port identifier of the Ethernet device.
+* @param queue_id
+*   The queue on the Ethernet device on which the callback is to be added.
+* @param fn
+*   The callback function
+* @param user_param
+*   A generic pointer parameter which will be passed to each invocation of the
+*   callback function on this port and queue.
+*
+* @return
+*   NULL on error.
+*   On success, a pointer value which can later be used to remove the callback.
+*/
+void *rte_eth_add_first_rx_callback(uint8_t port_id, uint16_t queue_id,
+   rte_rx_callback_fn fn, void *user_param);
+
 /**
  * Add a callback to be called on packet TX on a given port and queue.
  *
diff --git a/lib/librte_ether/rte_ether_version.map 
b/lib/librte_ether/rte_ether_version.map
index 214ecc7..c990b04 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -132,3 +132,9 @@ DPDK_16.04 {
rte_eth_tx_buffer_set_err_callback;

 } DPDK_2.2;
+
+DPDK_16.07 {
+   global:
+
+   rte_eth_add_first_rx_callback;
+} DPDK_16.04;
-- 
2.5.0



[dpdk-dev] [PATCH v5 1/9] librte_ether: protect add/remove of rxtx callbacks with spinlocks

2016-06-08 Thread Reshma Pattan
Added spinlocks around add/remove logic of rxtx callbacks to
avoid corruption of callback lists in multithreaded context.

Signed-off-by: Reshma Pattan 
---
 lib/librte_ether/rte_ethdev.c | 82 +--
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e148028..ce70d58 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -77,6 +77,12 @@ static uint8_t nb_ports;
 /* spinlock for eth device callbacks */
 static rte_spinlock_t rte_eth_dev_cb_lock = RTE_SPINLOCK_INITIALIZER;

+/* spinlock for add/remove rx callbacks */
+static rte_spinlock_t rte_eth_rx_cb_lock = RTE_SPINLOCK_INITIALIZER;
+
+/* spinlock for add/remove tx callbacks */
+static rte_spinlock_t rte_eth_tx_cb_lock = RTE_SPINLOCK_INITIALIZER;
+
 /* store statistics names and its offset in stats structure  */
 struct rte_eth_xstats_name_off {
char name[RTE_ETH_XSTATS_NAME_SIZE];
@@ -1634,7 +1640,6 @@ rte_eth_dev_set_rx_queue_stats_mapping(uint8_t port_id, 
uint16_t rx_queue_id,
STAT_QMAP_RX);
 }

-
 void
 rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
 {
@@ -2905,7 +2910,6 @@ rte_eth_add_rx_callback(uint8_t port_id, uint16_t 
queue_id,
rte_errno = EINVAL;
return NULL;
}
-
struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);

if (cb == NULL) {
@@ -2916,6 +2920,7 @@ rte_eth_add_rx_callback(uint8_t port_id, uint16_t 
queue_id,
cb->fn.rx = fn;
cb->param = user_param;

+   rte_spinlock_lock(&rte_eth_rx_cb_lock);
/* Add the callbacks in fifo order. */
struct rte_eth_rxtx_callback *tail =
rte_eth_devices[port_id].post_rx_burst_cbs[queue_id];
@@ -2928,6 +2933,7 @@ rte_eth_add_rx_callback(uint8_t port_id, uint16_t 
queue_id,
tail = tail->next;
tail->next = cb;
}
+   rte_spinlock_unlock(&rte_eth_rx_cb_lock);

return cb;
 }
@@ -2957,6 +2963,7 @@ rte_eth_add_tx_callback(uint8_t port_id, uint16_t 
queue_id,
cb->fn.tx = fn;
cb->param = user_param;

+   rte_spinlock_lock(&rte_eth_tx_cb_lock);
/* Add the callbacks in fifo order. */
struct rte_eth_rxtx_callback *tail =
rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id];
@@ -2969,6 +2976,7 @@ rte_eth_add_tx_callback(uint8_t port_id, uint16_t 
queue_id,
tail = tail->next;
tail->next = cb;
}
+   rte_spinlock_unlock(&rte_eth_tx_cb_lock);

return cb;
 }
@@ -2987,29 +2995,24 @@ rte_eth_remove_rx_callback(uint8_t port_id, uint16_t 
queue_id,
return -EINVAL;

struct rte_eth_dev *dev = &rte_eth_devices[port_id];
-   struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
-   struct rte_eth_rxtx_callback *prev_cb;
-
-   /* Reset head pointer and remove user cb if first in the list. */
-   if (cb == user_cb) {
-   dev->post_rx_burst_cbs[queue_id] = user_cb->next;
-   return 0;
-   }
-
-   /* Remove the user cb from the callback list. */
-   do {
-   prev_cb = cb;
-   cb = cb->next;
-
+   struct rte_eth_rxtx_callback *cb;
+   struct rte_eth_rxtx_callback **prev_cb;
+   int ret = -EINVAL;
+
+   rte_spinlock_lock(&rte_eth_rx_cb_lock);
+   prev_cb = &dev->post_rx_burst_cbs[queue_id];
+   for (; *prev_cb != NULL; prev_cb = &cb->next) {
+   cb = *prev_cb;
if (cb == user_cb) {
-   prev_cb->next = user_cb->next;
-   return 0;
+   /* Remove the user cb from the callback list. */
+   *prev_cb = cb->next;
+   ret = 0;
+   break;
}
+   }
+   rte_spinlock_unlock(&rte_eth_rx_cb_lock);

-   } while (cb != NULL);
-
-   /* Callback wasn't found. */
-   return -EINVAL;
+   return ret;
 }

 int
@@ -3026,29 +3029,24 @@ rte_eth_remove_tx_callback(uint8_t port_id, uint16_t 
queue_id,
return -EINVAL;

struct rte_eth_dev *dev = &rte_eth_devices[port_id];
-   struct rte_eth_rxtx_callback *cb = dev->pre_tx_burst_cbs[queue_id];
-   struct rte_eth_rxtx_callback *prev_cb;
-
-   /* Reset head pointer and remove user cb if first in the list. */
-   if (cb == user_cb) {
-   dev->pre_tx_burst_cbs[queue_id] = user_cb->next;
-   return 0;
-   }
-
-   /* Remove the user cb from the callback list. */
-   do {
-   prev_cb = cb;
-   cb = cb->next;
-
+   int ret = -EINVAL;
+   struct rte_eth_rxtx_callback *cb;
+   struct rte_eth_rxtx_callback **prev_cb;
+
+   rte_spinlock_lock(&rte_eth_tx_cb_lock);
+   prev_cb = &dev->pre_tx_burst_cbs[queue

[dpdk-dev] [PATCH v5 0/9] add packet capture framework

2016-06-08 Thread Reshma Pattan
This patch set include below changes

1)Changes to librte_ether.
2)A new library librte_pdump added for packet capture framework.
3)A new app/pdump tool added for packet capturing.
4)Test pmd changes done to initialize packet capture framework.
5)Documentation update.

1)librte_pdump
==
To support packet capturing on dpdk Ethernet devices, a new library librte_pdump
is added.Users can develop their own packet capturing application using new 
library APIs.

Operation:
--
Pdump library provides APIs to support packet capturing on dpdk Ethernet 
devices.
Library provides APIs to initialize the packet capture framework, enable/disable
the packet capture and uninitialize the packet capture framework.

Pdump library works on client/server based model.

Sever is responsible for enabling/disabling the packet captures.
Clients are responsible for requesting enable/disable of the
packet captures.

As part of packet capture framework initialization, pthread and
the server socket is created. Only one server socket is allowed on the system.
As part of enabling/disabling the packet capture, client sockets are created
and multiple client sockets are allowed.
Who ever calls initialization first they will succeed with the initialization,
next subsequent calls of initialization are not allowed. So next users can only
request enabling/disabling the packet capture.

Applications using below APIs need to pass port/device_id, queue, mempool and
ring parameters. Library uses user provided ring and mempool to mirror the rx/tx
packets of the port for users. Users need to dequeue the rings and write the 
packets
to vdev(pcap/tuntap) to view the packets using any standard tools.

Note:
Mempool and Ring should be mc/mp supportable.
Mempool mbuf size should be big enough to handle the rx/tx packets of a port.

APIs:
-
rte_pdump_init()
rte_pdump_enable()
rte_pdump_enable_by_deviceid()
rte_pdump_disable()
rte_pdump_disable_by_deviceid()
rte_pdump_uninit()

2)app/pdump tool

Tool app/pdump is designed based on librte_pdump for packet capturing in DPDK.
This tool by default runs as secondary process, and provides the support for
the command line options for packet capture.

./build/app/dpdk_pdump --
   --pdump '(port= | device_id=),
(queue=),
(rx-dev= |
 tx-dev=),
[ring-size=],
[mbuf-size=],
[total-num-mbufs=]'

Parameters inside the parenthesis represents the mandatory parameters.
Parameters inside the square brackets represents optional parameters.
User has to pass on packet capture parameters under --pdump parameters, 
multiples of
--pdump can be passed to capture packets on different port and queue 
combinations

Operation:
--
*Tool parse the user command line arguments,
creates the mempool, ring and the PCAP PMD vdev with 'tx_stream' as either
of the device passed in rx-dev|tx-dev parameters.

*Then calls the APIs of librte_pdump i.e. 
rte_pdump_enable()/rte_pdump_enable_by_deviceid()
to enable packet capturing on a specific port/device_id and queue by passing on
port|device_id, queue, mempool and ring info.

*Tool runs in while loop to dequeue the packets from the ring and write them to 
pcap device.

*Tool can be stopped using SIGINT, upon which tool calls
rte_pdump_disable()/rte_pdump_disable_by_deviceid() and free the allocated 
resources.

Note:
CONFIG_RTE_LIBRTE_PMD_PCAP flag should be set to yes to compile and run the 
pdump tool.

3)Test-pmd changes
==
Changes are done to test-pmd application to initialize/uninitialize the packet 
capture framework.
So app/pdump tool can be run to see packets of dpdk ports that are used by 
test-pmd.

Similarly any application which needs packet capture should call 
initialize/uninitialize APIs of
librte_pdump and use pdump tool to start the capture.

4)Packet capture flow between pdump tool and librte_pdump
=
* Pdump tool (Secondary process) requests packet capture
for specific port|device_id and queue combinations.

*Library in secondary process context creates client socket and communicates
the port|device_id, queue, ring and mempool to server.

*Library initializes server in primary process 'test-pmd' context and server 
serves
the client request to enable Ethernet rxtx call-backs for a given 
port|device_id and queue.

*Copy the rx/tx packets to passed mempool and enqueue the packets to ring for 
secondary process.

*Pdump tool will dequeue the packets from ring and writes them to PCAPMD vdev,
so ultimately packets will be seen on the device that is passed in 
rx-dev|tx-dev.

*Once the pdump tool is terminated with SIGINT it will disable the packet 
capturing.

*Library receives the disable packet capture request, communicate the info to 
server,
server will rem

[dpdk-dev] [PATCH] mk: generate internal library dependencies from DEPDIRS-y automatically

2016-06-08 Thread Olivier Matz


On 06/07/2016 04:40 PM, Wiles, Keith wrote:
> On 6/7/16, 9:19 AM, "dev on behalf of Thomas Monjalon"  dpdk.org on behalf of thomas.monjalon at 6wind.com> wrote:
> 
>> 2016-06-07 15:07, Bruce Richardson:
>>> On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
 2016-06-07 14:36, Christian Ehrhardt:
> But I still struggle to see how to fix the circular dependency between
> librte_eal and librte_mempool.

 Why is there a circular dependency?
 Only because of logs using mempool?

> Maybe now is a time to look at this part of the original threads again to
> eventually get apps less overlinked?
> => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> My naive suggestions in generalized form can be found there (no answer 
> yet):
> =>
> http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries

 I would prefer removing the circular dependency.
 Maybe we can rewrite the code to not use mempool or move it outside of EAL.

Indeed, mempools are used in eal for history. Is this feature still
useful now that logs are sent to syslog? Maybe we could deprecate this
API, and remove mempool calls in a future release?


>>> Or else we can take the attitude that the mempools and the rings are just a 
>>> core
>>> part of DPDK and move them and the EAL into a dpdk_core library at link 
>>> time.
>>> Having the code separate in the git tree is good, but I'm not sure having
>>> the resulting object files being in separate .a/.so files is particularly 
>>> useful.
>>> I can't see someone wanting to use one without the other.
>>
>> EAL could be used as an abstraction layer on top of systems and platforms.
>> And I think keeping things separated and layered help to maintain a design
>> easy to understand.

I like the idea to have one lib per directory (for consistency). It
may also simplify the Makefiles.
I'm in favor of keeping mempool and ring separated from eal if we
can remove the circular dep.

Olivier


[dpdk-dev] [PATCH v8 1/3] mempool: support external mempool operations

2016-06-08 Thread Shreyansh Jain
Hi David,

Sorry for multiple mails on a patch. I forgot a trivial comment in previous 
mail.

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of David Hunt
> Sent: Friday, June 03, 2016 8:28 PM
> To: dev at dpdk.org
> Cc: olivier.matz at 6wind.com; viktorin at rehivetech.com;
> jerin.jacob at caviumnetworks.com; David Hunt 
> Subject: [dpdk-dev] [PATCH v8 1/3] mempool: support external mempool
> operations
> 
[...]
> +int
> +rte_mempool_ops_register(const struct rte_mempool_ops *h)
> +{
> + struct rte_mempool_ops *ops;
> + int16_t ops_index;
> +
> + rte_spinlock_lock(&rte_mempool_ops_table.sl);
> +
> + if (rte_mempool_ops_table.num_ops >=
> + RTE_MEMPOOL_MAX_OPS_IDX) {
> + rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> + RTE_LOG(ERR, MEMPOOL,
> + "Maximum number of mempool ops structs exceeded\n");
> + return -ENOSPC;
> + }
> +
> + if (h->put == NULL || h->get == NULL || h->get_count == NULL) {

I think 'h->alloc' should also be checked here.

> + rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> + RTE_LOG(ERR, MEMPOOL,
> + "Missing callback while registering mempool ops\n");
> + return -EINVAL;
> + }
> +
> + if (strlen(h->name) >= sizeof(ops->name) - 1) {
> + RTE_LOG(DEBUG, EAL, "%s(): mempool_ops <%s>: name too long\n",
> + __func__, h->name);
> + rte_errno = EEXIST;
> + return NULL;
> + }
> +
> + ops_index = rte_mempool_ops_table.num_ops++;
> + ops = &rte_mempool_ops_table.ops[ops_index];
> + snprintf(ops->name, sizeof(ops->name), "%s", h->name);
> + ops->alloc = h->alloc;
> + ops->put = h->put;
> + ops->get = h->get;
> + ops->get_count = h->get_count;
> +
> + rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> +
> + return ops_index;
> +}
> +
[...]

-
Shreyansh



[dpdk-dev] [PATCH] mbuf: remove inconsistent assert statements

2016-06-08 Thread Adrien Mazarguil
Hi Konstantin,

On Wed, Jun 08, 2016 at 10:34:17AM +, Ananyev, Konstantin wrote:
> Hi Adrien,
> 
> > 
> > An assertion failure occurs in __rte_mbuf_raw_free() (called by a few PMDs)
> > when compiling DPDK with CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG and starting
> > applications with a log level high enough to trigger it.
> > 
> > While rte_mbuf_raw_alloc() sets refcount to 1, __rte_mbuf_raw_free()
> > expects it to be 0. 
> >Considering users are not expected to reset the
> > reference count to satisfy assert() and that raw functions are designed on
> > purpose without safety belts, remove these checks.
> 
> Yes, it refcnt supposed to be set to 0 by __rte_pktmbuf_prefree_seg().
> Wright now, it is a user responsibility to make sure refcnt==0 before pushing
> mbuf back to the pool.
> Not sure why do you consider that wrong?

I do not consider this wrong and I'm all for using assert() to catch
programming errors, however in this specific case, I think they are
inconsistent and misleading.

> If the user calls __rte_mbuf_raw_free() manualy it is his responsibility to 
> make
> sure mbuf's refcn==0.

Sure, however what harm does it cause (besides assert() to fail), since the
allocation function sets refcount to 1?

Why having the allocation function set the refcount if we are sure it is
already 0 (assert() proves it). Removing rte_mbuf_refcnt_set(m, 1) can
surely improve performance.

> BTW, why are you doing it?
> The comment clearly states that the function is for internal use:
> /**
>  * @internal Put mbuf back into its original mempool.
>  * The use of that function is reserved for RTE internal needs.
>  * Please use rte_pktmbuf_free().
>  *
>  * @param m
>  *   The mbuf to be freed.
>  */
> static inline void __attribute__((always_inline))
> __rte_mbuf_raw_free(struct rte_mbuf *m)

Several PMDs are using it anyway (won't name names, but I know one of them
quite well). I chose to modify this code instead of its users for the
following reasons:

- Considering their names, these functions should be opposites and able to
  work together like malloc()/free().

- PMDs are relying on these functions for performance reasons, we can assume
  they took the extra care necessary to make sure it would work properly.

- Preventing it would make these PMDs slower and is not acceptable either.

What remains is the consistency issue, I think these statements were only
added to catch multiple frees, and those should be caught at a higher
level, where other consistency checks are also performed.

> > Signed-off-by: Adrien Mazarguil 
> > ---
> >  lib/librte_mbuf/rte_mbuf.h | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index 11fa06d..7070bb8 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -1108,7 +1108,6 @@ static inline struct rte_mbuf 
> > *rte_mbuf_raw_alloc(struct rte_mempool *mp)
> > if (rte_mempool_get(mp, &mb) < 0)
> > return NULL;
> > m = (struct rte_mbuf *)mb;
> > -   RTE_ASSERT(rte_mbuf_refcnt_read(m) == 0);
> > rte_mbuf_refcnt_set(m, 1);
> > __rte_mbuf_sanity_check(m, 0);
> > 
> > @@ -1133,7 +1132,6 @@ __rte_mbuf_raw_alloc(struct rte_mempool *mp)
> >  static inline void __attribute__((always_inline))
> >  __rte_mbuf_raw_free(struct rte_mbuf *m)
> >  {
> > -   RTE_ASSERT(rte_mbuf_refcnt_read(m) == 0);
> > rte_mempool_put(m->pool, m);
> >  }
> > 
> > --
> > 2.1.4
> 

-- 
Adrien Mazarguil
6WIND


[dpdk-dev] [PATCH] app/test: fix bond device name too long

2016-06-08 Thread Jastrzebski, MichalX K
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, June 08, 2016 3:31 PM
> To: Jastrzebski, MichalX K 
> Cc: Iremonger, Bernard ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/test: fix bond device name too long
> 
> 2016-06-08 11:50, Jastrzebski, MichalX K:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-05-27 18:38, Thomas Monjalon:
> > > > 2016-05-27 17:20, Michal Jastrzebski:
> > > > > Bond device name was too long (grather than 32 signs) that
> > > > > cause mempool allocation to fail.
> > > >
> > > > Maybe that this kind of failure would be avoided if the test
> > > > was added to autotests (app/test/autotest_data.py).
> > > >
> > > > Generally speaking, it would be a good idea to make an audit
> > > > on which tests are missing in "make fast_test" and "make test".
> > >
> > > Any comment please?
> >
> > Hi Thomas,
> >
> > There is a small timeout in test_tlb_tx_burst - big burst has to be
> generated to
> > detect balancing and small timeout has to be included between each
> burst,
> > thus I am not sure if link_bonding_autotest can be classified to fast tests
> > (test takes about 3-4 seconds).
> > We can add this test to autotests script for which time is not so critical.
> 
> The bug we see here could be detected by just initializing bonding.
> Maybe we can consider having some basic/fast tests and others longer.

Actually this particular bug does not happen during bonding initialization, but
only after user will configure bond device in mode 6 (ALB). As only in mode 6 
we need 
mempool allocation and we do rte_pktmbuf_pool_create giving as a mempool name -
bond device name BONDED_DEV_NAME.

Best regards
Michal


[dpdk-dev] [PATCH v3 09/10] remove name field from struct rte_eth_xstats

2016-06-08 Thread Thomas Monjalon
2016-05-30 11:48, Remy Horton:
>  struct rte_eth_xstats {
> - /* FIXME: Remove name[] once remaining drivers converted */
> - char name[RTE_ETH_XSTATS_NAME_SIZE];
>   uint64_t id;
>   uint64_t value;
>  };

While changing the content of this struct, it can be the opportunity
to fix its name from rte_eth_xstats to rte_eth_xstat.




[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Bruce Richardson
On Wed, Jun 08, 2016 at 06:13:21PM +0530, Jerin Jacob wrote:
> On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
> > On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> > > Jerin Jacob (20):
> > >   thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC
> > >   thunderx/nicvf: add pmd skeleton
> > >   thunderx/nicvf: add link status and link update support
> > >   thunderx/nicvf: add get_reg and get_reg_length support
> > >   thunderx/nicvf: add dev_configure support
> > >   thunderx/nicvf: add dev_infos_get support
> > >   thunderx/nicvf: add rx_queue_setup/release support
> > >   thunderx/nicvf: add tx_queue_setup/release support
> > >   thunderx/nicvf: add rss and reta query and update support
> > >   thunderx/nicvf: add mtu_set and promiscuous_enable support
> > >   thunderx/nicvf: add stats support
> > >   thunderx/nicvf: add single and multi segment tx functions
> > >   thunderx/nicvf: add single and multi segment rx functions
> > >   thunderx/nicvf: add dev_supported_ptypes_get and rx_queue_count
> > > support
> > >   thunderx/nicvf: add rx queue start and stop support
> > >   thunderx/nicvf: add tx queue start and stop support
> > >   thunderx/nicvf: add device start,stop and close support
> > >   thunderx/config: set max numa node to two
> > >   thunderx/nicvf: updated driver documentation and release notes
> > >   maintainers: claim responsibility for the ThunderX nicvf PMD
> > > 
> > 
> > Hi Jerin,
> > 
> > In patch subject, as tag, other drivers are using only driver name, and
> > Intel drivers also has "driver/base", since base code has some special
> > case. For thunderx, what do you think about keeping subject as:
> >  "thunderx: "
> > 
> 
> Hi Ferruh,
> 
> We may add crypto or other builtin ThunderX HW accelerated block drivers
> in future to DPDK.
> So that is the reason why I thought of keeping the subject as thunderx/nicvf.
> If you don't have any objection then I would like to keep it as
> thunderx/nicvf or just nicvf.

Are you upstreaming kernel modules for this device? If so, what is the Linux
kernel module-name for this device going to be, as perhaps that can help us
here?

Regards,
/Bruce


[dpdk-dev] [PATCH v3 01/10] rte: change xstats to use integer ids

2016-06-08 Thread Thomas Monjalon
2016-06-08 12:16, Remy Horton:
> 'noon,
> 
> On 08/06/2016 10:37, Thomas Monjalon wrote:
> > 2016-05-30 11:48, Remy Horton:
> >>   struct rte_eth_xstats {
> >> +  /* FIXME: Remove name[] once remaining drivers converted */
> >>char name[RTE_ETH_XSTATS_NAME_SIZE];
> >
> > What is the plan? This field must be deprecated with an attribute.
> > We cannot have 2 different APIs depending of the driver.
> 
> This is where it gets logistically tricky..
> 
> Since there's an API/ABI breakage notice in place on this, my own 
> preference would be to have the entire patchset quashed into a single 
> patch. Problem is that rte/app changes (patches 1 & 7-9) are normally 
> applied via master whereas driver changes (patches 2-6) go in via 
> dpdk-next-net - it is not clear to me how patches should be submitted 
> for this case..

Misunderstanding here. Patches are fine and will be integrated in the
main tree because they are not only some drivers changes.
I was talking about the old API with name in rte_eth_xstats.
I have not seen the patch 9 which removes it.

> >> +int rte_eth_xstats_count(uint8_t port_id);
> >
> > This function is useless because we can have the count with
> > rte_eth_xstats_get(p, NULL, 0)
> > By the way it would be more consistent to have the same behaviour
> > in rte_eth_xstats_names().
> 
> Feedback I got with earlier patches was that a seperate count function 
> was preferable to overloading the fetch function using *data==NULL - is 
> the use of the latter specifically preferred?

I prefer the fetch/NULL style to get a count.
It also handles nicely the fetch error because of a too small buffer.


[dpdk-dev] [PATCH v3 19/20] thunderx/nicvf: updated driver documentation and release notes

2016-06-08 Thread Bruce Richardson
On Wed, Jun 08, 2016 at 05:57:16PM +0530, Jerin Jacob wrote:
> On Wed, Jun 08, 2016 at 01:08:35PM +0100, Ferruh Yigit wrote:
> > On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> > > Updated doc/guides/nics/overview.rst, doc/guides/nics/thunderx.rst
> > > and release notes
> > > 
> > > Changed "*" to "P" in overview.rst to capture the partially supported
> > > feature as "*" creating alignment issues with Sphinx table
> > > 
> > > Signed-off-by: Jerin Jacob 
> > > Signed-off-by: Slawomir Rosek 
> > > Acked-by: John McNamara 
> > > ---
> > >  doc/guides/nics/index.rst  |   1 +
> > >  doc/guides/nics/overview.rst   |  96 -
> > >  doc/guides/nics/thunderx.rst   | 354 
> > > +
> > >  doc/guides/rel_notes/release_16_07.rst |   1 +
> > >  4 files changed, 404 insertions(+), 48 deletions(-)
> > >  create mode 100644 doc/guides/nics/thunderx.rst
> > 
> > Hi Jerin,
> > 
> > This patch doesn't apply on top of origin/rel_16_07:
> > 
> > Applying: thunderx/nicvf: updated driver documentation and release notes
> > Using index info to reconstruct a base tree...
> > M   doc/guides/nics/overview.rst
> > Falling back to patching base and 3-way merge...
> > Auto-merging doc/guides/nics/overview.rst
> > CONFLICT (content): Merge conflict in doc/guides/nics/overview.rst
> 
> Hi Ferruh,
> 
> Since docs files are files keep changing, this patch set
> has been re-based on latest change-set i.e 
> ca173a909538a2f1082cd0dcb4d778a97dab69c3
> not origin/rel_16_07.
>

The nic overview.rst doc causes lots of conflicts when merging, and those
I just fix on apply. There is no need to do a new patch revision solely for 
that.

So nothing to see here, move along... :-)

Regards,
/Bruce


[dpdk-dev] [PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing portlist

2016-06-08 Thread De Lara Guarch, Pablo
Hi Bernard,

> -Original Message-
> From: Iremonger, Bernard
> Sent: Thursday, May 05, 2016 4:40 PM
> To: dev at dpdk.org
> Cc: De Lara Guarch, Pablo; Iremonger, Bernard
> Subject: [PATCH v2 4/8] app/testpmd: reconfigure forwarding after changing
> portlist
> 
> Set nb_fwd_ports to zero on quit.
> Check portlist has been set before displaying forwarding configuration.
> 
> Fixes: d3a274ce9dee ("app/testpmd: handle SIGINT and SIGTERM")
> Fixes: af75078fece3 ("first public release")

This patch is not fixing any issue, right? You are trying to improve the 
behaviour when changing portlist.
Therefore, you don't need to use Fixes tag.

> 
> Signed-off-by: Bernard Iremonger 
> ---
>  app/test-pmd/config.c  | 8 ++--
>  app/test-pmd/testpmd.c | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index f434999..10ac768 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1424,8 +1424,10 @@ pkt_fwd_config_display(struct fwd_config *cfg)
>  void
>  fwd_config_display(void)
>  {
> - fwd_config_setup();
> - pkt_fwd_config_display(&cur_fwd_config);
> + if (cur_fwd_config.nb_fwd_ports)
> + pkt_fwd_config_display(&cur_fwd_config);
> + else
> + printf("Please set portlist first\n");
>  }

The problem of doing this is that if user starts testpmd, it is not possible to 
show
the configuration of the ports directly, since fwd_config_setup() has not being 
called
(because set_fwd_ports_list() has not being called), so it looks like portlist 
must be set,
but if user starts forwarding directly, then it is not necessary.
What I mean, is that by default, portlist should be all the ports.
Maybe we need to call fwd_config_setup after all the testpmd initialization.

> 
>  int
> @@ -1529,6 +1531,8 @@ set_fwd_ports_list(unsigned int *portlist, unsigned
> int nb_pt)
>  (unsigned int) nb_fwd_ports, nb_pt);
>   nb_fwd_ports = (portid_t) nb_pt;
>   }
> +
> + fwd_config_setup();
>  }

I understand what you are doing here, but there is a problem. If you use 
--portmask parameter,
this function gets called when the arguments are parsed, but at that point, the 
ports are not configured yet,
and you get the following:

Fail: nb_rxq(1) is greater than max_rx_queues(0)
Program received signal SIGSEGV, Segmentation fault.
0x004835c9 in setup_fwd_config_of_each_lcore (cfg=0xca4160 
) at /tmp/dpdk-latest/app/test-pmd/config.c:1073

Anyway, I like the idea of moving fwd_config_setup out of fwd_config_display().
The problem is that there are other functions that should call this, such as 
set_fwd_lcores_list
(so, with this patch, if coremask is changed and then we call "show config 
fwd", we will not see any change).
Basically, all that affects the forwarding configuration should reconfigure it.
That's why I think it was decided to reconfigure the configuration when 
starting the forwarding or when showing the configuration.

So, we have two options:
1 - We add fwd_config_setup() in all the functions that are changing the 
configurations.
2 - We leave it as it was, especially with this patch, it makes more sense: 
http://dpdk.org/dev/patchwork/patch/13132/


> 
>  void
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 11b4cf7..2c58075 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1560,6 +1560,7 @@ pmd_test_exit(void)
> 
>   if (ports != NULL) {
>   no_link_check = 1;
> + nb_fwd_ports = 0;

Is this really necessary? I have removed it and I can quit testpmd with no 
problem.

>   FOREACH_PORT(pt_id, ports) {
>   printf("\nShutting down port %d...\n", pt_id);
>   fflush(stdout);
> --
> 2.6.3



[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Ferruh Yigit
On 6/8/2016 1:43 PM, Jerin Jacob wrote:
> On Wed, Jun 08, 2016 at 01:30:28PM +0100, Ferruh Yigit wrote:
>> On 6/7/2016 5:40 PM, Jerin Jacob wrote:
>>> Jerin Jacob (20):
>>>   thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC
>>>   thunderx/nicvf: add pmd skeleton
>>>   thunderx/nicvf: add link status and link update support
>>>   thunderx/nicvf: add get_reg and get_reg_length support
>>>   thunderx/nicvf: add dev_configure support
>>>   thunderx/nicvf: add dev_infos_get support
>>>   thunderx/nicvf: add rx_queue_setup/release support
>>>   thunderx/nicvf: add tx_queue_setup/release support
>>>   thunderx/nicvf: add rss and reta query and update support
>>>   thunderx/nicvf: add mtu_set and promiscuous_enable support
>>>   thunderx/nicvf: add stats support
>>>   thunderx/nicvf: add single and multi segment tx functions
>>>   thunderx/nicvf: add single and multi segment rx functions
>>>   thunderx/nicvf: add dev_supported_ptypes_get and rx_queue_count
>>> support
>>>   thunderx/nicvf: add rx queue start and stop support
>>>   thunderx/nicvf: add tx queue start and stop support
>>>   thunderx/nicvf: add device start,stop and close support
>>>   thunderx/config: set max numa node to two
>>>   thunderx/nicvf: updated driver documentation and release notes
>>>   maintainers: claim responsibility for the ThunderX nicvf PMD
>>>
>>
>> Hi Jerin,
>>
>> In patch subject, as tag, other drivers are using only driver name, and
>> Intel drivers also has "driver/base", since base code has some special
>> case. For thunderx, what do you think about keeping subject as:
>>  "thunderx: "
>>
> 
> Hi Ferruh,
> 
> We may add crypto or other builtin ThunderX HW accelerated block drivers
> in future to DPDK.
> So that is the reason why I thought of keeping the subject as thunderx/nicvf.
> If you don't have any objection then I would like to keep it as
> thunderx/nicvf or just nicvf.
> 

Ring has similar problem, but we are using same tag "ring:" for both
ring_pmd and ring library.

For this case perhaps we can use net/thunderx, crypto/thunderx ?

I am not aware of any defined convention for the case.

Thanks,
ferruh





[dpdk-dev] [PATCH v8 1/3] mempool: support external mempool operations

2016-06-08 Thread Olivier Matz
Hi David,

Please find some comments below.

On 06/03/2016 04:58 PM, David Hunt wrote:

> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h

> +/**
> + * Prototype for implementation specific data provisioning function.
> + *
> + * The function should provide the implementation specific memory for
> + * for use by the other mempool ops functions in a given mempool ops struct.
> + * E.g. the default ops provides an instance of the rte_ring for this 
> purpose.
> + * it will mostlikely point to a different type of data structure, and
> + * will be transparent to the application programmer.
> + */
> +typedef void *(*rte_mempool_alloc_t)(struct rte_mempool *mp);

In http://dpdk.org/ml/archives/dev/2016-June/040233.html, I suggested
to change the prototype to return an int (-errno) and directly set
the set mp->pool_data (void *) or mp->pool_if (uint64_t). No cast
would be required in this latter case.

By the way, there is a typo in the comment:
"mostlikely" -> "most likely"

> --- /dev/null
> +++ b/lib/librte_mempool/rte_mempool_default.c

> +static void
> +common_ring_free(struct rte_mempool *mp)
> +{
> + rte_ring_free((struct rte_ring *)mp->pool_data);
> +}

I don't think the cast is needed here.
(same in other functions)


> --- /dev/null
> +++ b/lib/librte_mempool/rte_mempool_ops.c

> +/* add a new ops struct in rte_mempool_ops_table, return its index */
> +int
> +rte_mempool_ops_register(const struct rte_mempool_ops *h)
> +{
> + struct rte_mempool_ops *ops;
> + int16_t ops_index;
> +
> + rte_spinlock_lock(&rte_mempool_ops_table.sl);
> +
> + if (rte_mempool_ops_table.num_ops >=
> + RTE_MEMPOOL_MAX_OPS_IDX) {
> + rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> + RTE_LOG(ERR, MEMPOOL,
> + "Maximum number of mempool ops structs exceeded\n");
> + return -ENOSPC;
> + }
> +
> + if (h->put == NULL || h->get == NULL || h->get_count == NULL) {
> + rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> + RTE_LOG(ERR, MEMPOOL,
> + "Missing callback while registering mempool ops\n");
> + return -EINVAL;
> + }
> +
> + if (strlen(h->name) >= sizeof(ops->name) - 1) {
> + RTE_LOG(DEBUG, EAL, "%s(): mempool_ops <%s>: name too long\n",
> + __func__, h->name);
> + rte_errno = EEXIST;
> + return NULL;
> + }

This has already been noticed by Shreyansh, but in case of:

rte_mempool_ops.c:75:10: error: return makes integer from pointer
without a cast [-Werror=int-conversion]
   return NULL;
  ^


> +/* sets mempool ops previously registered by rte_mempool_ops_register */
> +int
> +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name)


When I compile with shared libraries enabled, I get the following error:

librte_reorder.so: undefined reference to `rte_mempool_ops_table'
librte_mbuf.so: undefined reference to `rte_mempool_set_ops_byname'
...

The new functions and global variables must be in
rte_mempool_version.map. This was in v5
( http://dpdk.org/ml/archives/dev/2016-May/039365.html ) but
was dropped in v6.




Regards,
Olivier


[dpdk-dev] [PATCH v3 12/20] thunderx/nicvf: add single and multi segment tx functions

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
>  drivers/net/thunderx/Makefile   |   2 +
>  drivers/net/thunderx/nicvf_ethdev.c |   5 +-
>  drivers/net/thunderx/nicvf_rxtx.c   | 256 
> 
>  drivers/net/thunderx/nicvf_rxtx.h   |  93 +
>  4 files changed, 355 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/thunderx/nicvf_rxtx.c
>  create mode 100644 drivers/net/thunderx/nicvf_rxtx.h
> 
> diff --git a/drivers/net/thunderx/Makefile b/drivers/net/thunderx/Makefile

...

> +
> +static inline uint32_t __hot
> +nicvf_free_xmittted_buffers(struct nicvf_txq *sq, struct rte_mbuf **tx_pkts,

again, although this is perfectly fine, any intention to say xmitted
instead of xmittted?

...


[dpdk-dev] [PATCH v8 1/3] mempool: support external mempool operations

2016-06-08 Thread Shreyansh Jain
Hi David,

Thanks for explanation. I have some comments inline...

> -Original Message-
> From: Hunt, David [mailto:david.hunt at intel.com]
> Sent: Tuesday, June 07, 2016 2:56 PM
> To: Shreyansh Jain ; dev at dpdk.org
> Cc: olivier.matz at 6wind.com; viktorin at rehivetech.com;
> jerin.jacob at caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v8 1/3] mempool: support external mempool
> operations
> 
> Hi Shreyansh,
> 
> On 6/6/2016 3:38 PM, Shreyansh Jain wrote:
> > Hi,
> >
> > (Apologies for overly-eager email sent on this thread earlier. Will be more
> careful in future).
> >
> > This is more of a question/clarification than a comment. (And I have taken
> only some snippets from original mail to keep it cleaner)
> >
> > 
> >> +MEMPOOL_REGISTER_OPS(ops_mp_mc);
> >> +MEMPOOL_REGISTER_OPS(ops_sp_sc);
> >> +MEMPOOL_REGISTER_OPS(ops_mp_sc);
> >> +MEMPOOL_REGISTER_OPS(ops_sp_mc);
> > 
> >
> >  From the above what I understand is that multiple packet pool handlers can
> be created.
> >
> > I have a use-case where application has multiple pools but only the packet
> pool is hardware backed. Using the hardware for general buffer requirements
> would prove costly.
> >  From what I understand from the patch, selection of the pool is based on
> the flags below.
> 
> The flags are only used to select one of the default handlers for
> backward compatibility through
> the rte_mempool_create call. If you wish to use a mempool handler that
> is not one of the
> defaults, (i.e. a new hardware handler), you would use the
> rte_create_mempool_empty
> followed by the rte_mempool_set_ops_byname call.
> So, for the external handlers, you create and empty mempool, then set
> the operations (ops)
> for that particular mempool.

I am concerned about the existing applications (for example, l3fwd).
Explicit calls to 'rte_create_mempool_empty->rte_mempool_set_ops_byname' model 
would require modifications to these applications.
Ideally, without any modifications, these applications should be able to use 
packet pools (backed by hardware) and buffer pools (backed by ring/others) - 
transparently.

If I go by your suggestions, what I understand is, doing the above without 
modification to applications would be equivalent to:

  struct rte_mempool_ops custom_hw_allocator = {...}

thereafter, in config/common_base:

  CONFIG_RTE_DEFAULT_MEMPOOL_OPS="custom_hw_allocator"

calls to rte_pktmbuf_pool_create would use the new allocator.

But, another problem arises here.

There are two distinct paths for allocations of a memory pool:
1. A 'pkt' pool:
   rte_pktmbuf_pool_create   
 \- rte_mempool_create_empty
 |   \- rte_mempool_set_ops_byname(..ring_mp_mc..)
 |
 `- rte_mempool_set_ops_byname
   (...RTE_MBUF_DEFAULT_MEMPOOL_OPS..)
   /* Override default 'ring_mp_mc' of
* rte_mempool_create */

2. Through generic mempool create API
   rte_mempool_create
 \- rte_mempool_create_empty
   (passing pktmbuf and pool constructors)

I found various instances in example applications where rte_mempool_create() is 
being called directly for packet pools - bypassing the more semantically 
correct call to rte_pktmbuf_* for packet pools.

In (2) control path, RTE_MBUF_DEFAULT_MEMPOOLS_OPS wouldn't be able to replace 
custom handler operations for packet buffer allocations.

>From a performance point-of-view, Applications should be able to select 
>between packet pools and non-packet pools.

> 
> > 
> >> +  /*
> >> +   * Since we have 4 combinations of the SP/SC/MP/MC examine the flags to
> >> +   * set the correct index into the table of ops structs.
> >> +   */
> >> +  if (flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET))
> >> +  rte_mempool_set_ops_byname(mp, "ring_sp_sc");
> >> +  else if (flags & MEMPOOL_F_SP_PUT)
> >> +  rte_mempool_set_ops_byname(mp, "ring_sp_mc");
> >> +  else if (flags & MEMPOOL_F_SC_GET)
> >> +  rte_mempool_set_ops_byname(mp, "ring_mp_sc");
> >> +  else
> >> +  rte_mempool_set_ops_byname(mp, "ring_mp_mc");
> >> +

My suggestion is to have an additional flag, 'MEMPOOL_F_PKT_ALLOC', which, if 
specified, would:

...
#define MEMPOOL_F_SC_GET0x0008
#define MEMPOOL_F_PKT_ALLOC 0x0010
...

in rte_mempool_create_empty:
   ... after checking the other MEMPOOL_F_* flags...

if (flags & MEMPOOL_F_PKT_ALLOC)
rte_mempool_set_ops_byname(mp, RTE_MBUF_DEFAULT_MEMPOOL_OPS)

And removing the redundant call to rte_mempool_set_ops_byname() in 
rte_pktmbuf_create_pool().

Thereafter, rte_pktmbuf_pool_create can be changed to:

  ...
mp = rte_mempool_create_empty(name, n, elt_size, cache_size,
-sizeof(struct rte_pktmbuf_pool_private), socket_id, 0);
+sizeof(struct rte_pktmbuf_pool_private), socket_id,
+MEMPOOL_F_PKT_ALLOC);
if (mp == NULL)
return NULL;

> > Is there any way I can achieve the above use case of multiple pools which
> can be selected by an application - something like a run-time toggl

[dpdk-dev] [PATCH] eal/linux: fix undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)

2016-06-08 Thread Sergio Gonzalez Monroy
On 27/04/2016 18:06, Daniel Mrzyglod wrote:
> Fix issue reported by clang scan-build
>
> there is a chance that nr_hugepages will be 0 if conditions for loop
> for (i = 0; i < (int) internal_config.num_hugepage_sizes; i++)
> will be unmeet.
>
> Fixes: b6a468ad41d5 ("memory: add --socket-mem option")
>
> Signed-off-by: Daniel Mrzyglod 
> ---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c 
> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 5b9132c..e94538e 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1114,6 +1114,8 @@ rte_eal_hugepage_init(void)
>* processing done on these pages, shared memory will be created
>* at a later stage.
>*/
> + if (nr_hugepages == 0)
> + goto fail;
>   tmp_hp = malloc(nr_hugepages * sizeof(struct hugepage_file));
>   if (tmp_hp == NULL)
>   goto fail;

The behavior of malloc(0) is implementation-defined, but on Linux man 
page it says that returns NULL.
So strictly speaking, without the patch the outcome is the same cause 
malloc(0) will return NULL.

Now, I'd consider the patch not needed but it doesn't really harm either.
Anyone else has comments/thoughts about it?

Regarding the patch itself, I think the title and commit message need to 
be modify to reflect that the patch
goal is to handle nr_hugepages = 0 case without relying in malloc to 
return NULL.

Sergio




[dpdk-dev] [PATCH v3 00/20] DPDK PMD for ThunderX NIC device

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> This patch set provides the initial version of DPDK PMD for the
> built-in NIC device in Cavium ThunderX SoC family.
> 
> Implemented features and ThunderX nicvf PMD documentation added
> in doc/guides/nics/overview.rst and doc/guides/nics/thunderx.rst
> respectively in this patch set.
> 
> These patches are checked using checkpatch.sh with following
> additional ignore option:
> options="$options --ignore=CAMELCASE,BRACKET_SPACE"
> CAMELCASE - To accommodate PRIx64
> BRACKET_SPACE - To accommodate AT&T inline line assembly in two places
> 
> This patch set is based on DPDK 16.07-RC1
> and tested with today's git HEAD change-set
> ca173a909538a2f1082cd0dcb4d778a97dab69c3 along with
> following depended patch
> 
> http://dpdk.org/dev/patchwork/patch/11826/
> ethdev: add tunnel and port RSS offload types
> 
> V1->V2
> 
> http://dpdk.org/dev/patchwork/patch/12609/
> -- added const for the const struct tables
> -- remove multiple blank lines
> -- addressed style comments
> http://dpdk.org/dev/patchwork/patch/12610/
> -- removed DEPDIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += lib/librte_net 
> lib/librte_malloc
> -- add const for table structs
> -- addressed style comments
> http://dpdk.org/dev/patchwork/patch/12614/
> -- s/DEFAULT_*/NICVF_DEFAULT_*/gc
> http://dpdk.org/dev/patchwork/patch/12615/
> -- Fix typos
> -- addressed style comments
> http://dpdk.org/dev/patchwork/patch/12616/
> -- removed redundant txq->tail = 0 and txq->head = 0
> http://dpdk.org/dev/patchwork/patch/12627/
> -- fixed the documentation changes
> 
> -- fixed TAB+space occurrences in functions
> -- rebased to c8c33ad7f94c59d1c0676af0cfd61207b3e808db
> 
> V2->V3
> 
> http://dpdk.org/dev/patchwork/patch/13060/
> -- Changed polling infrastructure to use rte_eal_alarm* instead of 
> timerfd_create API
> -- rebased to ca173a909538a2f1082cd0dcb4d778a97dab69c3
> 
> Jerin Jacob (20):
>   thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC
>   thunderx/nicvf: add pmd skeleton
>   thunderx/nicvf: add link status and link update support
>   thunderx/nicvf: add get_reg and get_reg_length support
>   thunderx/nicvf: add dev_configure support
>   thunderx/nicvf: add dev_infos_get support
>   thunderx/nicvf: add rx_queue_setup/release support
>   thunderx/nicvf: add tx_queue_setup/release support
>   thunderx/nicvf: add rss and reta query and update support
>   thunderx/nicvf: add mtu_set and promiscuous_enable support
>   thunderx/nicvf: add stats support
>   thunderx/nicvf: add single and multi segment tx functions
>   thunderx/nicvf: add single and multi segment rx functions
>   thunderx/nicvf: add dev_supported_ptypes_get and rx_queue_count
> support
>   thunderx/nicvf: add rx queue start and stop support
>   thunderx/nicvf: add tx queue start and stop support
>   thunderx/nicvf: add device start,stop and close support
>   thunderx/config: set max numa node to two
>   thunderx/nicvf: updated driver documentation and release notes
>   maintainers: claim responsibility for the ThunderX nicvf PMD
> 

Hi Jerin,

In patch subject, as tag, other drivers are using only driver name, and
Intel drivers also has "driver/base", since base code has some special
case. For thunderx, what do you think about keeping subject as:
 "thunderx: "

Thanks,
ferruh


[dpdk-dev] [PATCH v3 17/20] thunderx/nicvf: add device start, stop and close support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
...
> +
> + /* Userspace process exited witout proper shutdown in last run */
s/witout/without

> + if (nicvf_qset_rbdr_active(nic, 0))
> + nicvf_dev_stop(dev);
> +
...


[dpdk-dev] [PATCH v3 08/20] thunderx/nicvf: add tx_queue_setup/release support

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
...

> +
> + /* Roundup nb_desc to avilable qsize and validate max number of desc */
s/avilable/available ?

> + nb_desc = nicvf_qsize_sq_roundup(nb_desc);
> + if (nb_desc == 0) {
> + PMD_INIT_LOG(ERR, "Value of nb_desc beyond available sq qsize");
> + return -EINVAL;
> + }
...


[dpdk-dev] [PATCH v3 02/20] thunderx/nicvf: add pmd skeleton

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Introduce driver initialization and enable build infrastructure for
> nicvf pmd driver.
> 
> By default, It is enabled only for defconfig_arm64-thunderx-*
> config as it is an inbuilt NIC device.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---
>  config/common_base |  10 +
>  config/defconfig_arm64-thunderx-linuxapp-gcc   |  10 +
>  drivers/net/Makefile   |   1 +
>  drivers/net/thunderx/Makefile  |  63 ++
>  drivers/net/thunderx/nicvf_ethdev.c| 251 
> +
>  drivers/net/thunderx/nicvf_ethdev.h|  48 
>  drivers/net/thunderx/nicvf_logs.h  |  83 +++
>  drivers/net/thunderx/nicvf_struct.h| 124 ++
>  .../thunderx/rte_pmd_thunderx_nicvf_version.map|   4 +
>  mk/rte.app.mk  |   2 +
>  10 files changed, 596 insertions(+)
>  create mode 100644 drivers/net/thunderx/Makefile
>  create mode 100644 drivers/net/thunderx/nicvf_ethdev.c
>  create mode 100644 drivers/net/thunderx/nicvf_ethdev.h
>  create mode 100644 drivers/net/thunderx/nicvf_logs.h
>  create mode 100644 drivers/net/thunderx/nicvf_struct.h
>  create mode 100644 drivers/net/thunderx/rte_pmd_thunderx_nicvf_version.map
> 

...

> +
> + if (nic->sqs_mode) {
> + PMD_INIT_LOG(INFO, "Unsupported SQS VF detected, Detaching...");
> + /* Detach port by returning Postive error number */

s/Postive/Positive ?

...


[dpdk-dev] [PATCH v3 01/20] thunderx/nicvf/base: add hardware API for ThunderX nicvf inbuilt NIC

2016-06-08 Thread Ferruh Yigit
On 6/7/2016 5:40 PM, Jerin Jacob wrote:
> Adds hardware specific API for ThunderX nicvf inbuilt NIC device under
> drivers/net/thunderx/nicvf/base directory.
> 
> Signed-off-by: Jerin Jacob 
> Signed-off-by: Maciej Czekaj 
> Signed-off-by: Kamil Rytarowski 
> Signed-off-by: Zyta Szpak 
> Signed-off-by: Slawomir Rosek 
> Signed-off-by: Radoslaw Biernacki 
> ---

...

> +
> +struct pf_rq_cfg { union { struct {
> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> + uint64_t reserverd1:1;
doesn't really matter but, as a detail, s/reserved/reserverd ? A few
more occurrence below.

> + uint64_t reserverd0:34;
> + uint64_t strip_pre_l2:1;
> + uint64_t caching:2;
> + uint64_t cq_qs:7;
> + uint64_t cq_idx:3;
> + uint64_t rbdr_cont_qs:7;
> + uint64_t rbdr_cont_idx:1;
> + uint64_t rbdr_strt_qs:7;
> + uint64_t rbdr_strt_idx:1;
> +#else
> + uint64_t rbdr_strt_idx:1;
> + uint64_t rbdr_strt_qs:7;
> + uint64_t rbdr_cont_idx:1;
> + uint64_t rbdr_cont_qs:7;
> + uint64_t cq_idx:3;
> + uint64_t cq_qs:7;
> + uint64_t caching:2;
> + uint64_t strip_pre_l2:1;
> + uint64_t reserverd0:34;
> + uint64_t reserverd1:1;
> +#endif
> + };
> + uint64_t value;
> +}; };
> +

...



[dpdk-dev] [PATCH 0/2] l2fwd improvements

2016-06-08 Thread Thomas Monjalon
> > Jerin Jacob (2):
> >   examples/l2fwd: remove number of cycles per second hardcording
> >   examples/l2fwd: increase mempool cache size for better performance
> 
> Looks ok, small typo in patch 1 subject line "hardcoding"
> Acked-by: Reshma Pattan 

Applied, thanks


  1   2   >