Re: [dpdk-dev] [PATCH v2 2/2] examples/ipsec-secgw: add target queues in flow actions

[Nelio Laranjeiro]

Hi Anoob,

On Fri, Dec 08, 2017 at 10:10:28PM +0530, Anoob Joseph wrote:
> HI Nelio,
> 
> 
> On 08-12-2017 20:10, Nelio Laranjeiro wrote:
> > On Fri, Dec 08, 2017 at 07:30:03PM +0530, Anoob wrote:
> > > Hi Nelio,
> > > 
> > > 
> > [...]
> > > > +   goto flow_create;
> > > > +   /* Try Queue. */
> > > > +   for (i = 0;
> > > > +i < eth_dev->data->nb_rx_queues; 
> > > > ++i)
> > > > +   if (eth_dev->data->rx_queues[i])
> > > > +   break;
> > > Is the following check correct?
> > [...]
> > 
> > For an application, it seems not necessary.  The application knows which
> > queues are configured in the drivers has it has made the configuration.
> > 
> > Removing it in the v3.
> I think you misunderstood me here.

Indeed, I misunderstood,

> I was talking about the following line.
> 
> + if (i != eth_dev->data->nb_rx_queues)
> + return -1;
> 
> Shouldn't it be?
> 
> + if (i == eth_dev->data->nb_rx_queues)
> + return -1;

Yes it should.

Anyway, I don't thing it is necessary to keep this check, from what I
saw in the application source code, it initialise all Rx queues up to
nb_rx_queues without leaving any hole.
According to this, I'll just remove this verification,  is it okay for
you?

Thanks,

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH] net/i40e: exclude LLDP packet count

[Xing, Beilei]

> -Original Message-
> From: Zhang, Qi Z
> Sent: Tuesday, December 5, 2017 9:30 AM
> To: Xing, Beilei 
> Cc: dev@dpdk.org; Zhang, Qi Z 
> Subject: [PATCH] net/i40e: exclude LLDP packet count
> 
> When use port stats register to calcuate the packet count,  LLDP packets are

Patch looks OK for me except some typos.
calcuate -> calculate

> counted in statistics which is not expected, the patch exclude this number
> from total number.
> 
> Fixes: 763de290cbd1 ("net/i40e: fix packet count for PF") Cc sta...@dpdk.org
> 
> Signed-off-by: Qi Zhang 
> ---
>  drivers/net/i40e/i40e_ethdev.c | 58
> +++---
>  1 file changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 811cc9f..4cbb259 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2531,6 +2531,22 @@ i40e_read_stats_registers(struct i40e_pf *pf,
> struct i40e_hw *hw)
>   pf->offset_loaded,
>   &pf->internal_stats_offset.rx_broadcast,
>   &pf->internal_stats.rx_broadcast);
> + /* Get total internal tx packet count */
> + i40e_stat_update_48(hw, I40E_GLV_UPTCH(hw->port),
> + I40E_GLV_UPTCL(hw->port),
> + pf->offset_loaded,
> + &pf->internal_stats_offset.tx_unicast,
> + &pf->internal_stats.tx_unicast);
> + i40e_stat_update_48(hw, I40E_GLV_MPTCH(hw->port),
> + I40E_GLV_MPTCL(hw->port),
> + pf->offset_loaded,
> + &pf->internal_stats_offset.tx_multicast,
> + &pf->internal_stats.tx_multicast);
> + i40e_stat_update_48(hw, I40E_GLV_BPTCH(hw->port),
> + I40E_GLV_BPTCL(hw->port),
> + pf->offset_loaded,
> + &pf->internal_stats_offset.tx_broadcast,
> + &pf->internal_stats.tx_broadcast);
> 
>   /* exclude CRC size */
>   pf->internal_stats.rx_bytes -= (pf->internal_stats.rx_unicast + @@ -
> 2560,16 +2576,32 @@ i40e_read_stats_registers(struct i40e_pf *pf, struct
> i40e_hw *hw)
>   ns->eth.rx_bytes -= (ns->eth.rx_unicast + ns->eth.rx_multicast +
>   ns->eth.rx_broadcast) * ETHER_CRC_LEN;
> 
> - /* Workaround: it is possible I40E_GLV_GORCH[H/L] is updated
> before
> + /* exlude internal rx bytes

exclude

> +  * Workaround: it is possible I40E_GLV_GORCH[H/L] is updated
> before
>* I40E_GLPRT_GORCH[H/L], so there is a small window that cause
> negtive

negative

>* value.
> +  * same to I40E_GLV_UPRC[H/L], I40E_GLV_MPRC[H/L],
> I40E_GLV_BPRC[H/L].
>*/
>   if (ns->eth.rx_bytes < pf->internal_stats.rx_bytes)
>   ns->eth.rx_bytes = 0;
> - /* exlude internal rx bytes */
>   else
>   ns->eth.rx_bytes -= pf->internal_stats.rx_bytes;
> 
> + if (ns->eth.rx_unicast < pf->internal_stats.rx_unicast)
> + ns->eth.rx_unicast = 0;
> + else
> + ns->eth.rx_unicast -= pf->internal_stats.rx_unicast;
> +
> + if (ns->eth.rx_multicast < pf->internal_stats.rx_multicast)
> + ns->eth.rx_multicast = 0;
> + else
> + ns->eth.rx_multicast -= pf->internal_stats.rx_multicast;
> +
> + if (ns->eth.rx_broadcast < pf->internal_stats.rx_broadcast)
> + ns->eth.rx_broadcast = 0;
> + else
> + ns->eth.rx_broadcast -= pf->internal_stats.rx_broadcast;
> +
>   i40e_stat_update_32(hw, I40E_GLPRT_RDPC(hw->port),
>   pf->offset_loaded, &os->eth.rx_discards,
>   &ns->eth.rx_discards);
> @@ -2598,12 +2630,32 @@ i40e_read_stats_registers(struct i40e_pf *pf,
> struct i40e_hw *hw)
>   ns->eth.tx_bytes -= (ns->eth.tx_unicast + ns->eth.tx_multicast +
>   ns->eth.tx_broadcast) * ETHER_CRC_LEN;
> 
> - /* exclude internal tx bytes */
> + /* exlude internal tx bytes

exclude

> +  * Workaround: it is possible I40E_GLV_GOTCH[H/L] is updated
> before
> +  * I40E_GLPRT_GOTCH[H/L], so there is a small window that cause
> negtive

negative.

> +  * value.
> +  * same to I40E_GLV_UPTC[H/L], I40E_GLV_MPTC[H/L],
> I40E_GLV_BPTC[H/L].
> +  */
>   if (ns->eth.tx_bytes < pf->internal_stats.tx_bytes)
>   ns->eth.tx_bytes = 0;
>   else
>   ns->eth.tx_bytes -= pf->internal_stats.tx_bytes;
> 
> + if (ns->eth.tx_unicast < pf->internal_stats.tx_unicast)
> + ns->eth.tx_unicast = 0;
> + else
> + ns->eth.tx_unicast -= pf->internal_stats.tx_unicast;
> +
> + if (ns->eth.tx_multicast < pf->internal_stats.tx_multicast)
> + ns->eth.tx_multicast = 0;
> + else
> + ns->eth.tx_multicast -= pf->internal_stats.tx_multicast;
> +
> + if (ns->eth.tx_broadca

Re: [dpdk-dev] [PATCH] net/i40e: exclude LLDP packet count

[Zhang, Qi Z]

Thanks will fix.

> -Original Message-
> From: Xing, Beilei
> Sent: Monday, December 11, 2017 4:50 PM
> To: Zhang, Qi Z 
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/i40e: exclude LLDP packet count
> 
> 
> > -Original Message-
> > From: Zhang, Qi Z
> > Sent: Tuesday, December 5, 2017 9:30 AM
> > To: Xing, Beilei 
> > Cc: dev@dpdk.org; Zhang, Qi Z 
> > Subject: [PATCH] net/i40e: exclude LLDP packet count
> >
> > When use port stats register to calcuate the packet count,  LLDP
> > packets are
> 
> Patch looks OK for me except some typos.
> calcuate -> calculate
> 
> > counted in statistics which is not expected, the patch exclude this
> > number from total number.
> >
> > Fixes: 763de290cbd1 ("net/i40e: fix packet count for PF") Cc
> > sta...@dpdk.org
> >
> > Signed-off-by: Qi Zhang 
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 58
> > +++---
> >  1 file changed, 55 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 811cc9f..4cbb259 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2531,6 +2531,22 @@ i40e_read_stats_registers(struct i40e_pf *pf,
> > struct i40e_hw *hw)
> > pf->offset_loaded,
> > &pf->internal_stats_offset.rx_broadcast,
> > &pf->internal_stats.rx_broadcast);
> > +   /* Get total internal tx packet count */
> > +   i40e_stat_update_48(hw, I40E_GLV_UPTCH(hw->port),
> > +   I40E_GLV_UPTCL(hw->port),
> > +   pf->offset_loaded,
> > +   &pf->internal_stats_offset.tx_unicast,
> > +   &pf->internal_stats.tx_unicast);
> > +   i40e_stat_update_48(hw, I40E_GLV_MPTCH(hw->port),
> > +   I40E_GLV_MPTCL(hw->port),
> > +   pf->offset_loaded,
> > +   &pf->internal_stats_offset.tx_multicast,
> > +   &pf->internal_stats.tx_multicast);
> > +   i40e_stat_update_48(hw, I40E_GLV_BPTCH(hw->port),
> > +   I40E_GLV_BPTCL(hw->port),
> > +   pf->offset_loaded,
> > +   &pf->internal_stats_offset.tx_broadcast,
> > +   &pf->internal_stats.tx_broadcast);
> >
> > /* exclude CRC size */
> > pf->internal_stats.rx_bytes -= (pf->internal_stats.rx_unicast + @@ -
> > 2560,16 +2576,32 @@ i40e_read_stats_registers(struct i40e_pf *pf,
> > struct i40e_hw *hw)
> > ns->eth.rx_bytes -= (ns->eth.rx_unicast + ns->eth.rx_multicast +
> > ns->eth.rx_broadcast) * ETHER_CRC_LEN;
> >
> > -   /* Workaround: it is possible I40E_GLV_GORCH[H/L] is updated
> > before
> > +   /* exlude internal rx bytes
> 
> exclude
> 
> > +* Workaround: it is possible I40E_GLV_GORCH[H/L] is updated
> > before
> >  * I40E_GLPRT_GORCH[H/L], so there is a small window that cause
> > negtive
> 
> negative
> 
> >  * value.
> > +* same to I40E_GLV_UPRC[H/L], I40E_GLV_MPRC[H/L],
> > I40E_GLV_BPRC[H/L].
> >  */
> > if (ns->eth.rx_bytes < pf->internal_stats.rx_bytes)
> > ns->eth.rx_bytes = 0;
> > -   /* exlude internal rx bytes */
> > else
> > ns->eth.rx_bytes -= pf->internal_stats.rx_bytes;
> >
> > +   if (ns->eth.rx_unicast < pf->internal_stats.rx_unicast)
> > +   ns->eth.rx_unicast = 0;
> > +   else
> > +   ns->eth.rx_unicast -= pf->internal_stats.rx_unicast;
> > +
> > +   if (ns->eth.rx_multicast < pf->internal_stats.rx_multicast)
> > +   ns->eth.rx_multicast = 0;
> > +   else
> > +   ns->eth.rx_multicast -= pf->internal_stats.rx_multicast;
> > +
> > +   if (ns->eth.rx_broadcast < pf->internal_stats.rx_broadcast)
> > +   ns->eth.rx_broadcast = 0;
> > +   else
> > +   ns->eth.rx_broadcast -= pf->internal_stats.rx_broadcast;
> > +
> > i40e_stat_update_32(hw, I40E_GLPRT_RDPC(hw->port),
> > pf->offset_loaded, &os->eth.rx_discards,
> > &ns->eth.rx_discards);
> > @@ -2598,12 +2630,32 @@ i40e_read_stats_registers(struct i40e_pf *pf,
> > struct i40e_hw *hw)
> > ns->eth.tx_bytes -= (ns->eth.tx_unicast + ns->eth.tx_multicast +
> > ns->eth.tx_broadcast) * ETHER_CRC_LEN;
> >
> > -   /* exclude internal tx bytes */
> > +   /* exlude internal tx bytes
> 
> exclude
> 
> > +* Workaround: it is possible I40E_GLV_GOTCH[H/L] is updated
> > before
> > +* I40E_GLPRT_GOTCH[H/L], so there is a small window that cause
> > negtive
> 
> negative.
> 
> > +* value.
> > +* same to I40E_GLV_UPTC[H/L], I40E_GLV_MPTC[H/L],
> > I40E_GLV_BPTC[H/L].
> > +*/
> > if (ns->eth.tx_bytes < pf->internal_stats.tx_bytes)
> > ns->eth.tx_bytes = 0;
> > else
> > ns->eth.tx_bytes -= pf->internal_stats.tx_bytes;
> >
> > +   if (ns->eth.tx_unicast < pf->internal_stats.tx_unicast)
> > +   ns->eth.t

Re: [dpdk-dev] [PATCH] net: update licence for network headers

[Olivier MATZ]

On Mon, Dec 11, 2017 at 10:57:41AM +0530, Hemant Agrawal wrote:
[...]
> > License text example in [1] starts from Copyright and has All rights
> > reserved.
> > I agree that template should be clearly specified from the very beginning.
> > 
> > [1] https://spdx.org/licenses/BSD-3-Clause#licenseText
> > 
> Hi all,
>   Most templates are showing copyright first and SPDX later i.e. the 
> typical
> way for writing the license.
> 
> However some projects has followed it other way around to make it easy for
> tools i.e. the TOP line.
> 
> I agree with Ferruh that we shall follow single convention.  I will prefer
> to do it in following way to make it consistent. (I will also fix my change
> patches).
> 
> >> Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
> >> Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER-2
> >> Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER-3
> >> SPDX-License-Identifier:BSD-3-Clause

OK for me, I'll send a v2.

Olivier

Re: [dpdk-dev] [PATCH v2 2/2] examples/ipsec-secgw: add target queues in flow actions

[Anoob]

Hi Nelio,


On 12/11/2017 01:51 PM, Nelio Laranjeiro wrote:

Hi Anoob,

On Fri, Dec 08, 2017 at 10:10:28PM +0530, Anoob Joseph wrote:

HI Nelio,


On 08-12-2017 20:10, Nelio Laranjeiro wrote:

On Fri, Dec 08, 2017 at 07:30:03PM +0530, Anoob wrote:

Hi Nelio,



[...]

+   goto flow_create;
+   /* Try Queue. */
+   for (i = 0;
+i < eth_dev->data->nb_rx_queues; ++i)
+   if (eth_dev->data->rx_queues[i])
+   break;

Is the following check correct?

[...]

For an application, it seems not necessary.  The application knows which
queues are configured in the drivers has it has made the configuration.

Removing it in the v3.

I think you misunderstood me here.

Indeed, I misunderstood,


I was talking about the following line.

+   if (i != eth_dev->data->nb_rx_queues)
+   return -1;

Shouldn't it be?

+   if (i == eth_dev->data->nb_rx_queues)
+   return -1;

Yes it should.

Anyway, I don't thing it is necessary to keep this check, from what I
saw in the application source code, it initialise all Rx queues up to
nb_rx_queues without leaving any hole.
According to this, I'll just remove this verification,  is it okay for
you?
I think you can just use Queue 0 here. So you can get rid of the checks 
etc. For real applications, we should have an entry in SA structure 
which will determine the Queue to be used. Even for RSS, something like 
that would be required.


Thanks,
Anoob

Re: [dpdk-dev] [PATCH] vhost: fix error code check when creating pthread

[Jens Freimann]

On Fri, Dec 08, 2017 at 11:19:49AM +0100, Olivier Matz wrote:

On error, pthread_create() returns a positive number (errno).
Fix the test on the return value.

Fixes: af1475918124 ("vhost: introduce API to start a specific driver")
Fixes: e623e0c6d8a5 ("vhost: add reconnect ability")
Cc: sta...@dpdk.org

Signed-off-by: Olivier Matz 
---
lib/librte_vhost/socket.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 422da002f..811e6bf16 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -472,7 +472,7 @@ vhost_user_reconnect_init(void)

ret = pthread_create(&reconn_tid, NULL,
 vhost_user_client_reconnect, NULL);
-   if (ret < 0) {
+   if (ret != 0) {
RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
if (pthread_mutex_destroy(&reconn_list.mutex)) {
RTE_LOG(ERR, VHOST_CONFIG,
@@ -678,9 +678,8 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
if ((flags & RTE_VHOST_USER_CLIENT) != 0) {
vsocket->reconnect = !(flags & RTE_VHOST_USER_NO_RECONNECT);
if (vsocket->reconnect && reconn_tid == 0) {
-   if (vhost_user_reconnect_init() < 0) {
+   if (vhost_user_reconnect_init() != 0)
goto out_mutex;
-   }
}
} else {
vsocket->is_server = true;
@@ -837,7 +836,7 @@ rte_vhost_driver_start(const char *path)
if (fdset_tid == 0) {
int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
 &vhost_user.fdset);
-   if (ret < 0)
+   if (ret != 0)
RTE_LOG(ERR, VHOST_CONFIG,
"failed to create fdset handling thread");
}


Reviewed-by: Jens Freimann  

Re: [dpdk-dev] DPDK Amazon AWS ENA driver problem with ping

[Burakov, Anatoly]

On 04-Dec-17 10:11 PM, Jordan Rhody wrote:

Hi,

I have been testing the DPDK ENA driver and am hoping that someone can help
in assisting an observation I have.

Using the DPDK ENA driver, I can send a ping request/reply from client A to
client B through an Edge device running DPDK.

However, the ping request that I observe always contains a '0x' for the
sequence id of the icmp request:

Internet Control Message Protocol
 Type: 8 (Echo (ping) request)
 Code: 0
 Checksum: 0xf9a1 incorrect, should be 0xfa8c
 [Checksum Status: Bad] <--- checksum is
original and unmodified
 Identifier (BE): 2385 (0x0951)
 Identifier (LE): 20745 (0x5109)
 Sequence number (BE): 65535 (0x)<--- here
 Sequence number (LE): 65535 (0x)<--- here
 [No response seen]
 Timestamp from icmp data: Nov 21, 2017 16:41:20.0 PST
 [Timestamp from icmp data (relative): 1.042436000 seconds]
 Data (48 bytes)
 Data: 612d0f00101112131415161718191a1b1c1d1e1f...
 [Length: 48]

I have traced the packet to the entry point of eth_ena_xmit_pkts( ) and it
does *not* have sequence id 0x.  The sequence id seems sane and valid
at that point.

Turning off DPDK and using the Linux Amazon AWS ENA driver yields no
problems at all.  ping request/reply works without a problem.  As an aside,
this same code works for the DPDK igb, ixgbe and virtio driver.

I have patched the DPDK ENA driver with all patches through DPDK 17.05 to
no avail and with the same results.

Question -- the basic: Could there be something 'wrong' with my AWS network
interface with regard to setup, perhaps something that may be obvious to
other folks more familiar with AWS?

If not, has anyone else experienced problems with DPDK AWS ENA driver?

Are there any possible patches out there that have not been commit to the
git repo that may fix this particular issue?

Thanks in advance,

Jordan Rhode


Hi Jordan,

I have no experience with the AWS ENA driver and i thus may be quite off 
the mark here, but presumably to respond to pings you would have to have 
an IP stack running on top of DPDK. Otherwise if you are running e.g. 
l2fwd on that interface, it'll just forward your ping requests back 
unmodified.


--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 0/3] generic channel for multi-process communication

[Burakov, Anatoly]

On 30-Nov-17 6:44 PM, Jianfeng Tan wrote:

This patchset adds a generic channel for multi-process (primary/secondary)
communication.

Patch 1: addess the purpose and howto;
Patch 2: add a syncrhonous way for those messages which need a response 
immediately.
Patch 3: Rework vfio to use this generic communication channel.



Hi Jianfeng,

Just a general comment: I am assuming this has the limitation of 
"everything happens through primary process's involvement". This will 
work for VFIO, as secondary always needs to ask the primary before doing 
anything, but it doesn't address other issues that could have been 
addressed with IPC.


For example, if a primary process would've hotplugged a device, it can't 
notify all secondary processes about this; rather, it has to wait until 
secondary processes ask for this info. Neither can it do anything if 
secondary requests a primary to do something, and notify other secondary 
processes about it (i.e. if secondary wants to hotplug a device, but 
there are other secondaries also running). It would be great to have a 
standard way of doing things like this in future revisions of our IPC.


--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 1/2] doc: fix lists of supported algorithms

[De Lara Guarch, Pablo]

Hi Andrea,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andrea Grandi
> Sent: Wednesday, November 22, 2017 6:03 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] doc: fix lists of supported algorithms
> 
> Add a missing space must before the first item of the list to display it
> correctly in the User Guide.
> 
> Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
> Fixes: b79e4c00af0e ("cryptodev: use AES-GCM/CCM as AEAD algorithms")
> 
> Signed-off-by: Andrea Grandi 

For next time, add CC: sta...@dpdk.org under the Fixes line.

Thanks,

Acked-by: Pablo de Lara 

Re: [dpdk-dev] [PATCH 2/2] doc: fix format in OpenSSL installation guide

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andrea Grandi
> Sent: Wednesday, November 22, 2017 6:03 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/2] doc: fix format in OpenSSL installation
> guide
> 
> List of supported OpenSSL versions and code block with dependencies were
> not properly formatted.
> 
> Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
> 
> Signed-off-by: Andrea Grandi 

Acked-by: Pablo de Lara 

Re: [dpdk-dev] [PATCH v2 4/6] net/representor: Implement port representor PMD

[Remy Horton]

On 08/12/2017 16:56, Mohammad Abdul Awal wrote:

On 08/12/2017 15:02, Remy Horton wrote:

[..]

I think it possible to create the representor without pulling in driver
codes. We probably can avoid using the rte_eth_vdev_allocate by calling
the rte_eth_dev_allocate() directly.


This is my general thinking.



int
rte_representor_port_register(const char *devargs);


Question is where this *devargs comes from - I don't think there is 
currently any commandline option to specify a parameter string for PFs.


Could go down the zero-conf route of always creating representors for 
all available VFs, but in that case may as well strip out the parsing code.

Re: [dpdk-dev] [PATCH 1/2] doc: fix lists of supported algorithms

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andrea Grandi
> Sent: Wednesday, November 22, 2017 6:03 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] doc: fix lists of supported algorithms
> 
> Add a missing space must before the first item of the list to display it
> correctly in the User Guide.
> 
> Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
> Fixes: b79e4c00af0e ("cryptodev: use AES-GCM/CCM as AEAD algorithms")
> 
> Signed-off-by: Andrea Grandi 

Series applied to dpdk-next-crypto.
Thanks,

Pablo

Re: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: fix usage of incorrect port

[Radu Nicolau]

Hi,


On 12/6/2017 11:08 AM, Anoob wrote:

Hi Akhil,

On 12/04/2017 01:19 PM, Akhil Goyal wrote:

Hi Anoob,
On 11/29/2017 9:51 AM, Anoob Joseph wrote:

Hi Akhil,


On 24-11-2017 16:19, Akhil Goyal wrote:

Hi Anoob,

On 11/24/2017 3:28 PM, Anoob wrote:

  static inline void
  route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[], 
uint8_t nb_pkts)

  {
  uint32_t hop[MAX_PKT_BURST * 2];
  uint32_t dst_ip[MAX_PKT_BURST * 2];
+    int32_t pkt_hop = 0;
  uint16_t i, offset;
+    uint16_t lpm_pkts = 0;
    if (nb_pkts == 0)
  return;
  +    /* Need to do an LPM lookup for non-offload packets. 
Offload packets

+ * will have port ID in the SA
+ */
+
  for (i = 0; i < nb_pkts; i++) {
-    offset = offsetof(struct ip, ip_dst);
-    dst_ip[i] = *rte_pktmbuf_mtod_offset(pkts[i],
-    uint32_t *, offset);
-    dst_ip[i] = rte_be_to_cpu_32(dst_ip[i]);
+    if (!(pkts[i]->ol_flags & PKT_TX_SEC_OFFLOAD)) {
+    /* Security offload not enabled. So an LPM lookup is
+ * required to get the hop
+ */
+    offset = offsetof(struct ip, ip_dst);
+    dst_ip[lpm_pkts] = *rte_pktmbuf_mtod_offset(pkts[i],
+    uint32_t *, offset);
+    dst_ip[lpm_pkts] = rte_be_to_cpu_32(dst_ip[lpm_pkts]);
+    lpm_pkts++;
+    }
  }
  -    rte_lpm_lookup_bulk((struct rte_lpm *)rt_ctx, dst_ip, 
hop, nb_pkts);
+    rte_lpm_lookup_bulk((struct rte_lpm *)rt_ctx, dst_ip, hop, 
lpm_pkts);

+
+    lpm_pkts = 0;
    for (i = 0; i < nb_pkts; i++) {
-    if ((hop[i] & RTE_LPM_LOOKUP_SUCCESS) == 0) {
+    if (pkts[i]->ol_flags & PKT_TX_SEC_OFFLOAD) {
+    /* Read hop from the SA */
+    pkt_hop = get_hop_for_offload_pkt(pkts[i]);
+    } else {
+    /* Need to use hop returned by lookup */
+    pkt_hop = hop[lpm_pkts++];
+    if ((pkt_hop & RTE_LPM_LOOKUP_SUCCESS) == 0)
+    pkt_hop = -1;
+    }
+
I believe the following check is redundant for non inline case. I 
believe get_hop_for_offload_pkt can also set the 
RTE_LPM_LOOKUP_SUCCESS if route is success and take the (pkt_hop 
& RTE_LPM_LOOKUP_SUCCESS) == 0 check outside the if else block 
and free the packet if it is unsuccessful.


Same comment for route6_pkts. Checking with -1 may not be a good 
idea if we have a flag available for the same.

Others can comment.
The problem is ipv4 & ipv6 LPM lookups return different error 
values, but we are using a single routine to get the hop for 
offload packets. The flag(RTE_LPM_LOOKUP_SUCCESS) is only for ipv4 
lookups. For ipv6, error is -1. If we need a cleaner solution, we 
can have ipv4 & ipv6 variants of "get_hop_for_offload_pkt". But 
that would be repetition of some code.


my concern over this patch is that there is an addition of an extra 
check in the non inline case and we can get rid of that with some 
changes in the code(lib/app). Regarding route6_pkts, the code looks 
cleaner than route4_pkts
If we have ipv4 and ipv6 variants of the 
"get_hop_for_offload_packet" function, the code would look much 
cleaner. Shall I update the patch with such a change and send v4?


I believe we shall get rid of "RTE_LPM_LOOKUP_SUCCESS" from the 
rte_lpm_lookup_bulk(), we shall have similar error flags for v4 and 
v6 APIs. Either we can have RTE_LPM_LOOKUP_SUCCESS or -1 as check for 
errors.
This will call for an ABI change. And LPM library has multiple 
variants for v4 & v6 lookups. We will need to modify all such 
instances. I've CCed Bruce for his opinion on this matter. If 
maintainers can decide on how to address this properly, I can plan my 
next steps accordingly.
Maybe this alternative approach will help: change the 
get_hop_for_offload_packet to return -1 for v6 and clear 
RTE_LPM_LOOKUP_SUCCESS flag for v4 errors. This will be on the error 
path so the extra code to check the pkt type will have no performance 
impact, and the route function can be cleaner and we can lose the extra 
if in the v4 one.

Sergio can comment on this.

Duplicating code for get_hop_for_offload_packet may not be a good idea.

-Akhil

Re: [dpdk-dev] [PATCH v4] mbuf: fix mbuf free performance with non atomic refcnt

[Olivier MATZ]

On Fri, Dec 08, 2017 at 04:46:51PM +0100, Olivier Matz wrote:
> When RTE_MBUF_REFCNT_ATOMIC=n, the decrement of the mbuf reference
> counter uses an atomic operation. This is not necessary and impacts
> the performance (seen with TRex traffic generator).
> 
> We cannot replace rte_atomic16_add_return() by rte_mbuf_refcnt_update()
> because it would add an additional check.
> 
> Solves this by introducing __rte_mbuf_refcnt_update(), which
> updates the reference counter without doing anything else.
> 
> Fixes: 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool")
> Suggested-by: Hanoch Haim 
> Signed-off-by: Olivier Matz 

Forgot to Cc sta...@dpdk.org

Re: [dpdk-dev] [PATCH v3] examples/ipsec-secgw: fix usage of incorrect port

[Anoob Joseph]

Hi,


On 12/11/2017 03:56 PM, Radu Nicolau wrote:

Hi,


On 12/6/2017 11:08 AM, Anoob wrote:

Hi Akhil,

On 12/04/2017 01:19 PM, Akhil Goyal wrote:

Hi Anoob,
On 11/29/2017 9:51 AM, Anoob Joseph wrote:

Hi Akhil,


On 24-11-2017 16:19, Akhil Goyal wrote:

Hi Anoob,

On 11/24/2017 3:28 PM, Anoob wrote:

  static inline void
  route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[], 
uint8_t nb_pkts)

  {
  uint32_t hop[MAX_PKT_BURST * 2];
  uint32_t dst_ip[MAX_PKT_BURST * 2];
+    int32_t pkt_hop = 0;
  uint16_t i, offset;
+    uint16_t lpm_pkts = 0;
    if (nb_pkts == 0)
  return;
  +    /* Need to do an LPM lookup for non-offload packets. 
Offload packets

+ * will have port ID in the SA
+ */
+
  for (i = 0; i < nb_pkts; i++) {
-    offset = offsetof(struct ip, ip_dst);
-    dst_ip[i] = *rte_pktmbuf_mtod_offset(pkts[i],
-    uint32_t *, offset);
-    dst_ip[i] = rte_be_to_cpu_32(dst_ip[i]);
+    if (!(pkts[i]->ol_flags & PKT_TX_SEC_OFFLOAD)) {
+    /* Security offload not enabled. So an LPM lookup is
+ * required to get the hop
+ */
+    offset = offsetof(struct ip, ip_dst);
+    dst_ip[lpm_pkts] = *rte_pktmbuf_mtod_offset(pkts[i],
+    uint32_t *, offset);
+    dst_ip[lpm_pkts] = 
rte_be_to_cpu_32(dst_ip[lpm_pkts]);

+    lpm_pkts++;
+    }
  }
  -    rte_lpm_lookup_bulk((struct rte_lpm *)rt_ctx, dst_ip, 
hop, nb_pkts);
+    rte_lpm_lookup_bulk((struct rte_lpm *)rt_ctx, dst_ip, hop, 
lpm_pkts);

+
+    lpm_pkts = 0;
    for (i = 0; i < nb_pkts; i++) {
-    if ((hop[i] & RTE_LPM_LOOKUP_SUCCESS) == 0) {
+    if (pkts[i]->ol_flags & PKT_TX_SEC_OFFLOAD) {
+    /* Read hop from the SA */
+    pkt_hop = get_hop_for_offload_pkt(pkts[i]);
+    } else {
+    /* Need to use hop returned by lookup */
+    pkt_hop = hop[lpm_pkts++];
+    if ((pkt_hop & RTE_LPM_LOOKUP_SUCCESS) == 0)
+    pkt_hop = -1;
+    }
+
I believe the following check is redundant for non inline case. 
I believe get_hop_for_offload_pkt can also set the 
RTE_LPM_LOOKUP_SUCCESS if route is success and take the (pkt_hop 
& RTE_LPM_LOOKUP_SUCCESS) == 0 check outside the if else block 
and free the packet if it is unsuccessful.


Same comment for route6_pkts. Checking with -1 may not be a good 
idea if we have a flag available for the same.

Others can comment.
The problem is ipv4 & ipv6 LPM lookups return different error 
values, but we are using a single routine to get the hop for 
offload packets. The flag(RTE_LPM_LOOKUP_SUCCESS) is only for 
ipv4 lookups. For ipv6, error is -1. If we need a cleaner 
solution, we can have ipv4 & ipv6 variants of 
"get_hop_for_offload_pkt". But that would be repetition of some 
code.


my concern over this patch is that there is an addition of an 
extra check in the non inline case and we can get rid of that with 
some changes in the code(lib/app). Regarding route6_pkts, the code 
looks cleaner than route4_pkts
If we have ipv4 and ipv6 variants of the 
"get_hop_for_offload_packet" function, the code would look much 
cleaner. Shall I update the patch with such a change and send v4?


I believe we shall get rid of "RTE_LPM_LOOKUP_SUCCESS" from the 
rte_lpm_lookup_bulk(), we shall have similar error flags for v4 and 
v6 APIs. Either we can have RTE_LPM_LOOKUP_SUCCESS or -1 as check 
for errors.
This will call for an ABI change. And LPM library has multiple 
variants for v4 & v6 lookups. We will need to modify all such 
instances. I've CCed Bruce for his opinion on this matter. If 
maintainers can decide on how to address this properly, I can plan my 
next steps accordingly.
Maybe this alternative approach will help: change the 
get_hop_for_offload_packet to return -1 for v6 and clear 
RTE_LPM_LOOKUP_SUCCESS flag for v4 errors. This will be on the error 
path so the extra code to check the pkt type will have no performance 
impact, and the route function can be cleaner and we can lose the 
extra if in the v4 one.
That should be fine I guess. So the get_hop_for_offload_packet will have 
one more argument to specify whether it is ipv4 or ipv6, right?


I'll revise the patch with this suggestion.

Sergio can comment on this.

Duplicating code for get_hop_for_offload_packet may not be a good idea.

-Akhil

Re: [dpdk-dev] [PATCH] cryptodev: Fix typo in qat.rst

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Monday, November 27, 2017 11:20 AM
> To: O Mahony, Billy ; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] cryptodev: Fix typo in qat.rst
> 
> 
> 
> > -Original Message-
> > From: O Mahony, Billy
> > Sent: Monday, November 27, 2017 11:14 AM
> > To: De Lara Guarch, Pablo ;
> > dev@dpdk.org
> > Cc: O Mahony, Billy 
> > Subject: [PATCH] cryptodev: Fix typo in qat.rst
> >
> > Signed-off-by: Billy O'Mahony 
> > ---
> >  doc/guides/cryptodevs/qat.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/cryptodevs/qat.rst
> > b/doc/guides/cryptodevs/qat.rst index cb17b6b..c8ef378 100644
> > --- a/doc/guides/cryptodevs/qat.rst
> > +++ b/doc/guides/cryptodevs/qat.rst
> > @@ -166,7 +166,7 @@ Next, you need to expose the Virtual Functions
> > (VFs) using the sysfs file system  First find the BDFs
> > (Bus-Device-Function) of the physical functions (PFs) of  your device, 
> > e.g.::
> >
> > -lspci -d : 37c8
> > +lspci -d:37c8
> >
> >  You should see output similar to::
> >
> > --
> > 2.7.4
> 
> Hi Billy,
> 
> Since this is a patch for documentation, the title should start with "doc:", 
> as
> "cryptodev: ..." suggests that the patch is fixing something in the library
> code.
> 
> I will fix it for you, since the rest of the patch looks ok to me.
> 
> Acked-by: Pablo de Lara 

Applied to dpdk-next-crypto.
Thanks,

Pablo

Re: [dpdk-dev] [PATCH v2 1/4] power: changed unsigned to unsigned int

[Hunt, David]

Hi Marko,

On 28/11/2017 1:22 PM, Marko Kovacevic wrote:

--snip--

  
  int

-guest_channel_host_connect(const char *path, unsigned lcore_id)
+guest_channel_host_connect(const char *path, unsigned int lcore_id)


--snip--

I'd suggest adding the following into the commit message. It would help 
explain why you're doing all the "unsigned" to "unsigned int" changes up 
front rather than including them in the other patches.


"Since this patch-set attempts to clean up the power library, and there 
are many instances of "unsigned" caught by checkpatch, it was decided to 
clean these up first rather than have them included in the later patches 
in the patch set. And would also minimise this type of error being 
caught by checkpatch in the future."


Regards,
Dave.

Re: [dpdk-dev] [PATCH] cryptodev: Add useful comment to _sym_session

[De Lara Guarch, Pablo]

> -Original Message-
> From: Trahe, Fiona
> Sent: Monday, November 27, 2017 11:45 AM
> To: O Mahony, Billy ; De Lara Guarch, Pablo
> ; dev@dpdk.org
> Cc: O Mahony, Billy 
> Subject: RE: [dpdk-dev] [PATCH] cryptodev: Add useful comment to
> _sym_session
> 
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Billy O'Mahony
> > Sent: Monday, November 27, 2017 11:40 AM
> > To: De Lara Guarch, Pablo ;
> dev@dpdk.org
> > Cc: O Mahony, Billy 
> > Subject: [dpdk-dev] [PATCH] cryptodev: Add useful comment to
> _sym_session
> >
> > Signed-off-by: Billy O'Mahony 
> Acked-by: Fiona Trahe 

Applied to dpdk-next-crypto.
Thanks,

Pablo

Re: [dpdk-dev] [PATCH v2 2/4] power: clean up of acpi files

[Hunt, David]

Hi Marko,


On 28/11/2017 1:22 PM, Marko Kovacevic wrote:

Signed-off-by: Marko Kovacevic 
---
  lib/librte_power/Makefile  |  2 +-
  ...e_power_acpi_cpufreq.c => power_acpi_cpufreq.c} | 32 +++---
  ...e_power_acpi_cpufreq.h => power_acpi_cpufreq.h} | 28 +--
  lib/librte_power/rte_power.c   | 28 +--
  4 files changed, 45 insertions(+), 45 deletions(-)
  rename lib/librte_power/{rte_power_acpi_cpufreq.c => power_acpi_cpufreq.c} 
(94%)
  rename lib/librte_power/{rte_power_acpi_cpufreq.h => power_acpi_cpufreq.h} 
(88%)

diff --git a/lib/librte_power/Makefile b/lib/librte_power/Makefile
index 1b1491d..bf5a55e 100644
--- a/lib/librte_power/Makefile
+++ b/lib/librte_power/Makefile
@@ -42,7 +42,7 @@ EXPORT_MAP := rte_power_version.map
  LIBABIVER := 1
  
  # all source are stored in SRCS-y

-SRCS-$(CONFIG_RTE_LIBRTE_POWER) := rte_power.c rte_power_acpi_cpufreq.c
+SRCS-$(CONFIG_RTE_LIBRTE_POWER) := rte_power.c power_acpi_cpufreq.c
  SRCS-$(CONFIG_RTE_LIBRTE_POWER) += rte_power_kvm_vm.c guest_channel.c
  
  # install this header file

diff --git a/lib/librte_power/rte_power_acpi_cpufreq.c 
b/lib/librte_power/power_acpi_cpufreq.c
similarity index 94%
rename from lib/librte_power/rte_power_acpi_cpufreq.c
rename to lib/librte_power/power_acpi_cpufreq.c


--snip--

Could I suggest adding the following to the commit message?

"Rename private header file rte_power_acpi_cpufreq.c to 
power_acpi_cpufreq.c. This prevents the private functions from leaking 
into the documentation.
Change any private functions from rte_ to just 
. Reserve the rte_ for public functions. "


Regards,
Dave.

Re: [dpdk-dev] [PATCH v2 3/4] power: clean up of kvm files

[Hunt, David]

Hi Marko,


On 28/11/2017 1:22 PM, Marko Kovacevic wrote:

Signed-off-by: Marko Kovacevic 
---
  lib/librte_power/Makefile  |  2 +-
  .../{rte_power_kvm_vm.c => power_kvm_vm.c} | 28 +++---
  .../{rte_power_kvm_vm.h => power_kvm_vm.h} | 28 +++---
  lib/librte_power/rte_power.c   | 28 +++---
  4 files changed, 43 insertions(+), 43 deletions(-)
  rename lib/librte_power/{rte_power_kvm_vm.c => power_kvm_vm.c} (83%)
  rename lib/librte_power/{rte_power_kvm_vm.h => power_kvm_vm.h} (85%)

diff --git a/lib/librte_power/Makefile b/lib/librte_power/Makefile
index bf5a55e..a35c50a 100644
--- a/lib/librte_power/Makefile
+++ b/lib/librte_power/Makefile
@@ -43,7 +43,7 @@ LIBABIVER := 1
  
  # all source are stored in SRCS-y

  SRCS-$(CONFIG_RTE_LIBRTE_POWER) := rte_power.c power_acpi_cpufreq.c
-SRCS-$(CONFIG_RTE_LIBRTE_POWER) += rte_power_kvm_vm.c guest_channel.c
+SRCS-$(CONFIG_RTE_LIBRTE_POWER) += power_kvm_vm.c guest_channel.c
  
  # install this header file

  SYMLINK-$(CONFIG_RTE_LIBRTE_POWER)-include := rte_power.h
diff --git a/lib/librte_power/rte_power_kvm_vm.c 
b/lib/librte_power/power_kvm_vm.c
similarity index 83%
rename from lib/librte_power/rte_power_kvm_vm.c
rename to lib/librte_power/power_kvm_vm.c



--snip--

Could I suggest you add the following into the commit message?

"rename private header file rte_power_kvm_vm.c to power_kvm_vm.c. This 
prevents the private functions from leaking into the documentation.
Change any private functions from rte_ to just 
. Reserve the rte_ for public functions. "


Regards,
Dave.

Re: [dpdk-dev] [PATCH v2 4/4] power: clean up of power common header

[Hunt, David]

Hi Marko,


On 28/11/2017 1:22 PM, Marko Kovacevic wrote:

Signed-off-by: Marko Kovacevic 
---
  lib/librte_power/power_acpi_cpufreq.c   | 2 +-
  lib/librte_power/{rte_power_common.h => power_common.h} | 6 +++---
  lib/librte_power/rte_power.c| 2 +-
  3 files changed, 5 insertions(+), 5 deletions(-)
  rename lib/librte_power/{rte_power_common.h => power_common.h} (95%)

diff --git a/lib/librte_power/power_acpi_cpufreq.c 
b/lib/librte_power/power_acpi_cpufreq.c
index 165ec97..fd1931f 100644
--- a/lib/librte_power/power_acpi_cpufreq.c
+++ b/lib/librte_power/power_acpi_cpufreq.c
@@ -45,7 +45,7 @@
  #include 
  
  #include "power_acpi_cpufreq.h"

-#include "rte_power_common.h"
+#include "power_common.h"
  
  #ifdef RTE_LIBRTE_POWER_DEBUG

  #define POWER_DEBUG_TRACE(fmt, args...) do { \
diff --git a/lib/librte_power/rte_power_common.h 
b/lib/librte_power/power_common.h
similarity index 95%
rename from lib/librte_power/rte_power_common.h
rename to lib/librte_power/power_common.h



--snip--

Could I suggest adding the following into the commit message?

"Rename private header file rte_power_common.h to power_common.h to 
prevent private functions from leaking into the documentation."


Apart from that, if you make those changes, you can include my Ack in 
the next revision.


Series-Acked-By: David Hunt 

Re: [dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add support for inline protocol

[Radu Nicolau]

On 11/23/2017 11:19 AM, Anoob Joseph wrote:

Adding support for inline protocol processing

In ingress side, application will receive regular IP packets, without
any IPsec related info. Application will do a selector check (SP-SA
check) by making use of the metadata from the packet.

In egress side, the plain packet would be submitted to the driver. The
packet will have optional metadata, which could be used to identify the
security session associated with the packet.

Signed-off-by: Anoob Joseph 
---


Acked-by: Radu Nicolau 

Re: [dpdk-dev] [PATCH 1/3] eal: add channel for multi-process communication

[Burakov, Anatoly]

On 30-Nov-17 6:44 PM, Jianfeng Tan wrote:

Previouly, there are three channels for multi-process
(i.e., primary/secondary) communication.
   1. Config-file based channel, in which, the primary process writes
  info into a pre-defined config file, and the secondary process
  reads info out.
   2. vfio submodule has its own channel based on unix socket for the
  secondary process to get container fd and group fd from the
  primary process.
   3. pdump submodule also has its own channel based on unix socket for
  packet dump.

It'll be good to have a generic communication channel for multi-process
communication to accomodate the requirements including:
   a. Secondary wants to send info to primary, for example, secondary
  would like to send request (about some specific vdev to primary).
   b. Sending info at any time, instead of just initialization time.
   c. Share FDs with the other side, for vdev like vhost, related FDs
  (memory region, kick) should be shared.
   d. A send message request needs the other side to response immediately.

This patch proposes to create a communication channel, as an unix
socket connection, for above requirements. Primary will listen on
the unix socket; secondary will connect this socket to talk.

Three new APIs are added:

   1. rte_eal_mp_action_register is used to register an action,
  indexed by a string; if the calling component wants to
  response the messages from the corresponding component in
  its primary process or secondary processes.
   2. rte_eal_mp_action_unregister is used to unregister the action
  if the calling component does not want to response the messages.
   3. rte_eal_mp_sendmsg is used to send a message.

Signed-off-by: Jianfeng Tan 
---


<...snip...>


+
+int
+rte_eal_mp_action_register(const char *action_name, rte_eal_mp_t action)
+{
+   struct action_entry *entry = malloc(sizeof(struct action_entry));
+
+   if (entry == NULL)
+   return -ENOMEM;
+
+   if (find_action_entry_by_name(action_name) != NULL)
+   return -EEXIST;


This should probably do a free(entry).


+
+   strncpy(entry->action_name, action_name, MAX_ACTION_NAME_LEN);
+   entry->action = action;
+   TAILQ_INSERT_TAIL(&action_entry_list, entry, next);
+   return 0;
+}
+


<...snip...>


+
+static int
+add_secondary(void)
+{
+   int fd;
+   struct epoll_event ev;
+
+   while (1) {
+   fd = accept(mp_fds.listen, NULL, NULL);
+   if (fd < 0 && errno == EAGAIN)
+   break;
+   else if (fd < 0) {
+   RTE_LOG(ERR, EAL, "primary failed to accept: %s\n",
+   strerror(errno));
+   return -1;
+   }
+
+   ev.events = EPOLLIN | EPOLLRDHUP;
+   ev.data.fd = fd;
+   if (epoll_ctl(mp_fds.efd, EPOLL_CTL_ADD, fd, &ev) < 0) {
+   RTE_LOG(ERR, EAL, "failed to add secondary: %s\n",
+   strerror(errno));
+   break;
+   }
+   if (add_sec_proc(fd) < 0) {
+   RTE_LOG(ERR, EAL, "too many secondary processes\n");
+   close(fd);
+   break;
+   }
+   }
+
+   return 0;
+}
+
+static void *
+mp_handler(void *arg __rte_unused)
+{
+   int fd;
+   int i, n;
+   struct epoll_event ev;
+   struct epoll_event *events;
+   int is_primary = rte_eal_process_type() == RTE_PROC_PRIMARY;
+
+   ev.events = EPOLLIN | EPOLLRDHUP;
+   ev.data.fd = (is_primary) ? mp_fds.listen : mp_fds.primary;
+   if (epoll_ctl(mp_fds.efd, EPOLL_CTL_ADD, ev.data.fd, &ev) < 0) {
+   RTE_LOG(ERR, EAL, "failed to epoll_ctl: %s\n",
+   strerror(errno));
+   exit(EXIT_FAILURE);


rte_exit?


+   }
+
+   events = calloc(20, sizeof ev);
+
+   while (1) {
+   n = epoll_wait(mp_fds.efd, events, 20, -1);
+   for (i = 0; i < n; i++) {
+   if (is_primary && events[i].data.fd == mp_fds.listen) {
+   if (events[i].events != EPOLLIN) {
+   RTE_LOG(ERR, EAL, "what happens?\n");


More descriptive error message would be nice :)


+   exit(EXIT_FAILURE);


rte_exit?


+   }
+
+   if (add_secondary() < 0)
+   break;


Doing epoll_ctl in multiple different places hurts readability IMO. 
Might be a good idea to refactor add_secondary and mp_handler in a way 
that keeps all epoll handling in one place.



+
+   continue;
+   }
+
+   fd = events[i].data.fd;
+
+   if ((events[i].events & EPOLLIN)) {
+   

Re: [dpdk-dev] [PATCH 09/39] examples/bond: convert to new ethdev offloads API

[Radu Nicolau]

On 11/23/2017 12:14 PM, Shahaf Shuler wrote:

Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler 
---
  examples/bond/main.c | 68 ---
  1 file changed, 58 insertions(+), 10 deletions(-)

diff --git a/examples/bond/main.c b/examples/bond/main.c
index 8e3b1f340..306447e6b 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -151,11 +151,8 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_NONE,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
-   .header_split   = 0, /**< Header Split disabled */
-   .hw_ip_checksum = 0, /**< IP checksum offload enabled */
-   .hw_vlan_filter = 0, /**< VLAN filtering disabled */
-   .jumbo_frame= 0, /**< Jumbo Frame Support disabled */
-   .hw_strip_crc   = 1, /**< CRC stripped by hardware */
+   .ignore_offload_bitfield = 1,
+   .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -174,10 +171,30 @@ slave_port_init(uint16_t portid, struct rte_mempool 
*mbuf_pool)
int retval;
uint16_t nb_rxd = RTE_RX_DESC_DEFAULT;
uint16_t nb_txd = RTE_TX_DESC_DEFAULT;
+   struct rte_eth_dev_info dev_info;
+   struct rte_eth_rxconf rxq_conf;
+   struct rte_eth_txconf txq_conf;
  
  	if (portid >= rte_eth_dev_count())

rte_exit(EXIT_FAILURE, "Invalid port\n");
  
+	rte_eth_dev_info_get(portid, &dev_info);

+   if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
+   port_conf.rxmode.offloads) {
+   printf("Some Rx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  portid, port_conf.rxmode.offloads,
+  dev_info.rx_offload_capa);
+   port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
+   }
+   if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
+port_conf.txmode.offloads) {
+   printf("Some Tx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  portid, port_conf.txmode.offloads,
+  dev_info.tx_offload_capa);
+   port_conf.txmode.offloads &= dev_info.tx_offload_capa;
+   }
retval = rte_eth_dev_configure(portid, 1, 1, &port_conf);
if (retval != 0)
rte_exit(EXIT_FAILURE, "port %u: configuration failed 
(res=%d)\n",
@@ -189,16 +206,22 @@ slave_port_init(uint16_t portid, struct rte_mempool 
*mbuf_pool)
"failed (res=%d)\n", portid, retval);
  
  	/* RX setup */

+   rxq_conf = dev_info.default_rxconf;
+   rxq_conf.offloads = port_conf.rxmode.offloads;
retval = rte_eth_rx_queue_setup(portid, 0, nb_rxd,
-   rte_eth_dev_socket_id(portid), NULL,
+   rte_eth_dev_socket_id(portid),
+   &rxq_conf,
mbuf_pool);
if (retval < 0)
rte_exit(retval, " port %u: RX queue 0 setup failed (res=%d)",
portid, retval);
  
  	/* TX setup */

+   txq_conf = dev_info.default_txconf;
+   txq_conf.txq_flags = ETH_TXQ_FLAGS_IGNORE;
+   txq_conf.offloads = port_conf.txmode.offloads;
retval = rte_eth_tx_queue_setup(portid, 0, nb_txd,
-   rte_eth_dev_socket_id(portid), NULL);
+   rte_eth_dev_socket_id(portid), &txq_conf);
  
  	if (retval < 0)

rte_exit(retval, "port %u: TX queue 0 setup failed (res=%d)",
@@ -225,6 +248,9 @@ bond_port_init(struct rte_mempool *mbuf_pool)
uint8_t i;
uint16_t nb_rxd = RTE_RX_DESC_DEFAULT;
uint16_t nb_txd = RTE_TX_DESC_DEFAULT;
+   struct rte_eth_dev_info dev_info;
+   struct rte_eth_rxconf rxq_conf;
+   struct rte_eth_txconf txq_conf;
  
  	retval = rte_eth_bond_create("bond0", BONDING_MODE_ALB,

0 /*SOCKET_ID_ANY*/);
@@ -234,6 +260,23 @@ bond_port_init(struct rte_mempool *mbuf_pool)
  
  	BOND_PORT = retval;
  
+	rte_eth_dev_info_get(BOND_PORT, &dev_info);

+   if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
+   port_conf.rxmode.offloads) {
+   printf("Some Rx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  BOND_PORT, port_conf.rxmode.offloads,
+  dev_info.rx_offload_capa);
+   port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
+   }
+   if (

[dpdk-dev] [PATCH v2 1/8] app/eventdev: add ethernet device producer option

[Pavan Nikhilesh]

Add command line option --prod_type_ethdev to specify that the events
are generated by ethernet device.

Signed-off-by: Pavan Nikhilesh 
Acked-by: Jerin Jacob 
---

 v2 Changes:
  - Modified 'evt_parse_prod_type' to 'evt_parse_eth_prod_type' to accommodate
  other kinds of producers in future.

 app/test-eventdev/evt_options.c  | 11 +++
 app/test-eventdev/evt_options.h  | 29 +
 app/test-eventdev/test_perf_common.c | 34 --
 3 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/app/test-eventdev/evt_options.c b/app/test-eventdev/evt_options.c
index e2187dfc3..86e428905 100644
--- a/app/test-eventdev/evt_options.c
+++ b/app/test-eventdev/evt_options.c
@@ -55,6 +55,7 @@ evt_options_default(struct evt_options *opt)
opt->pool_sz = 16 * 1024;
opt->wkr_deq_dep = 16;
opt->nb_pkts = (1ULL << 26); /* do ~64M packets */
+   opt->prod_type = EVT_PROD_TYPE_SYNT;
 }

 typedef int (*option_parser_t)(struct evt_options *opt,
@@ -106,6 +107,13 @@ evt_parse_queue_priority(struct evt_options *opt, const 
char *arg __rte_unused)
return 0;
 }

+static int
+evt_parse_eth_prod_type(struct evt_options *opt, const char *arg __rte_unused)
+{
+   opt->prod_type = EVT_PROD_TYPE_ETH_RX_ADPTR;
+   return 0;
+}
+
 static int
 evt_parse_test_name(struct evt_options *opt, const char *arg)
 {
@@ -189,6 +197,7 @@ usage(char *program)
"\t--worker_deq_depth : dequeue depth of the worker\n"
"\t--fwd_latency  : perform fwd_latency measurement\n"
"\t--queue_priority   : enable queue priority\n"
+   "\t--prod_type_ethdev : use ethernet device as producer\n."
);
printf("available tests:\n");
evt_test_dump_names();
@@ -249,6 +258,7 @@ static struct option lgopts[] = {
{ EVT_SCHED_TYPE_LIST,  1, 0, 0 },
{ EVT_FWD_LATENCY,  0, 0, 0 },
{ EVT_QUEUE_PRIORITY,   0, 0, 0 },
+   { EVT_PROD_ETHDEV,  0, 0, 0 },
{ EVT_HELP, 0, 0, 0 },
{ NULL, 0, 0, 0 }
 };
@@ -272,6 +282,7 @@ evt_opts_parse_long(int opt_idx, struct evt_options *opt)
{ EVT_SCHED_TYPE_LIST, evt_parse_sched_type_list},
{ EVT_FWD_LATENCY, evt_parse_fwd_latency},
{ EVT_QUEUE_PRIORITY, evt_parse_queue_priority},
+   { EVT_PROD_ETHDEV, evt_parse_eth_prod_type},
};

for (i = 0; i < RTE_DIM(parsermap); i++) {
diff --git a/app/test-eventdev/evt_options.h b/app/test-eventdev/evt_options.h
index a9a91252b..da7a6f863 100644
--- a/app/test-eventdev/evt_options.h
+++ b/app/test-eventdev/evt_options.h
@@ -58,8 +58,16 @@
 #define EVT_SCHED_TYPE_LIST  ("stlist")
 #define EVT_FWD_LATENCY  ("fwd_latency")
 #define EVT_QUEUE_PRIORITY   ("queue_priority")
+#define EVT_PROD_ETHDEV  ("prod_type_ethdev")
 #define EVT_HELP ("help")

+enum evt_prod_type {
+   EVT_PROD_TYPE_NONE,
+   EVT_PROD_TYPE_SYNT,  /* Producer type Synthetic i.e. CPU. */
+   EVT_PROD_TYPE_ETH_RX_ADPTR,  /* Producer type Eth Rx Adapter. */
+   EVT_PROD_TYPE_MAX,
+};
+
 struct evt_options {
 #define EVT_TEST_NAME_MAX_LEN 32
char test_name[EVT_TEST_NAME_MAX_LEN];
@@ -76,6 +84,7 @@ struct evt_options {
uint8_t dev_id;
uint32_t fwd_latency:1;
uint32_t q_priority:1;
+   enum evt_prod_type prod_type;
 };

 void evt_options_default(struct evt_options *opt);
@@ -266,4 +275,24 @@ evt_dump_sched_type_list(struct evt_options *opt)
evt_dump_end;
 }

+#define EVT_PROD_MAX_NAME_LEN 50
+static inline void
+evt_dump_producer_type(struct evt_options *opt)
+{
+   char name[EVT_PROD_MAX_NAME_LEN];
+
+   switch (opt->prod_type) {
+   default:
+   case EVT_PROD_TYPE_SYNT:
+   snprintf(name, EVT_PROD_MAX_NAME_LEN,
+   "Synthetic producer lcores");
+   break;
+   case EVT_PROD_TYPE_ETH_RX_ADPTR:
+   snprintf(name, EVT_PROD_MAX_NAME_LEN,
+   "Ethdev Rx Adapter producers");
+   break;
+   }
+   evt_dump("prod_type", "%s", name);
+}
+
 #endif /* _EVT_OPTIONS_ */
diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index e77b4723e..9d2865ed5 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -287,8 +287,10 @@ perf_opt_check(struct evt_options *opt, uint64_t nb_queues)
 {
unsigned int lcores;

-   /* N producer + N worker + 1 master */
-   lcores = 3;
+   /* N producer + N worker + 1 master when producer cores are used
+* Else N worker + 1 master when Rx adapter is used
+*/
+   lcores = opt->prod_type == EVT_PROD_TYPE_SYNT ? 3 : 2;

if (rte_lcore_count() < lcores) {
evt_err("test need minimum %d lcores", lcores);
@@ -3

[dpdk-dev] [PATCH v2 2/8] app/eventdev: modify app setup to support ethdev

[Pavan Nikhilesh]

Modify app setup to accommodate event port and queue setup based on the
number of ethernet ports.

Signed-off-by: Pavan Nikhilesh 
Acked-by: Jerin Jacob 
---

 v2 Changes:
  - Removed unnecessary RTE_SET_USED() macros.

 app/test-eventdev/test_perf_atq.c| 17 +
 app/test-eventdev/test_perf_common.c | 26 --
 app/test-eventdev/test_perf_common.h |  1 +
 app/test-eventdev/test_perf_queue.c  | 16 +---
 4 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/app/test-eventdev/test_perf_atq.c 
b/app/test-eventdev/test_perf_atq.c
index 0e9f2db0e..6082d4ff3 100644
--- a/app/test-eventdev/test_perf_atq.c
+++ b/app/test-eventdev/test_perf_atq.c
@@ -185,10 +185,19 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
 {
int ret;
uint8_t queue;
+   uint8_t nb_queues;
+   uint8_t nb_ports;
+
+   nb_ports = evt_nr_active_lcores(opt->wlcores);
+   nb_ports += opt->prod_type == EVT_PROD_TYPE_ETH_RX_ADPTR ? 0 :
+   evt_nr_active_lcores(opt->plcores);
+
+   nb_queues = opt->prod_type == EVT_PROD_TYPE_ETH_RX_ADPTR ?
+   rte_eth_dev_count() : atq_nb_event_queues(opt);

const struct rte_event_dev_config config = {
-   .nb_event_queues = atq_nb_event_queues(opt),
-   .nb_event_ports = perf_nb_event_ports(opt),
+   .nb_event_queues = nb_queues,
+   .nb_event_ports = nb_ports,
.nb_events_limit  = 4096,
.nb_event_queue_flows = opt->nb_flows,
.nb_event_port_dequeue_depth = 128,
@@ -208,7 +217,7 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
.nb_atomic_order_sequences = opt->nb_flows,
};
/* queue configurations */
-   for (queue = 0; queue < atq_nb_event_queues(opt); queue++) {
+   for (queue = 0; queue < nb_queues; queue++) {
ret = rte_event_queue_setup(opt->dev_id, queue, &q_conf);
if (ret) {
evt_err("failed to setup queue=%d", queue);
@@ -217,7 +226,7 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
}

ret = perf_event_dev_port_setup(test, opt, 1 /* stride */,
-   atq_nb_event_queues(opt));
+   nb_queues);
if (ret)
return ret;

diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index 9d2865ed5..114210ea6 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -88,6 +88,17 @@ perf_producer(void *arg)
return 0;
 }

+static int
+perf_producer_wrapper(void *arg)
+{
+   struct prod_data *p  = arg;
+   struct test_perf *t = p->t;
+   /* Launch the producer function only in case of synthetic producer. */
+   if (t->opt->prod_type == EVT_PROD_TYPE_SYNT)
+   return perf_producer(arg);
+   return 0;
+}
+
 static inline uint64_t
 processed_pkts(struct test_perf *t)
 {
@@ -142,8 +153,8 @@ perf_launch_lcores(struct evt_test *test, struct 
evt_options *opt,
if (!(opt->plcores[lcore_id]))
continue;

-   ret = rte_eal_remote_launch(perf_producer, &t->prod[port_idx],
-lcore_id);
+   ret = rte_eal_remote_launch(perf_producer_wrapper,
+   &t->prod[port_idx], lcore_id);
if (ret) {
evt_err("failed to launch perf_producer %d", lcore_id);
return ret;
@@ -193,14 +204,17 @@ perf_launch_lcores(struct evt_test *test, struct 
evt_options *opt,
fflush(stdout);

if (remaining <= 0) {
-   t->done = true;
t->result = EVT_TEST_SUCCESS;
-   rte_smp_wmb();
-   break;
+   if (opt->prod_type == EVT_PROD_TYPE_SYNT) {
+   t->done = true;
+   rte_smp_wmb();
+   break;
+   }
}
}

-   if (new_cycles - dead_lock_cycles > dead_lock_sample) {
+   if (new_cycles - dead_lock_cycles > dead_lock_sample &&
+   opt->prod_type == EVT_PROD_TYPE_SYNT) {
remaining = t->outstand_pkts - processed_pkts(t);
if (dead_lock_remaining == remaining) {
rte_event_dev_dump(opt->dev_id, stdout);
diff --git a/app/test-eventdev/test_perf_common.h 
b/app/test-eventdev/test_perf_common.h
index c6fc70cd7..ab2e59988 100644
--- a/app/tes

[dpdk-dev] [PATCH v2 4/8] app/eventdev: add ethernet device setup helpers

[Pavan Nikhilesh]

Add ethernet device setup functions to configure ethdev ports incase
prod_type_ethdev option is enabled.

Signed-off-by: Pavan Nikhilesh 
Acked-by: Jerin Jacob 
---

 v2 Changes:
  - Removed unnecessary "\n" from evt_err, reworked ethernet queue setup.

 app/test-eventdev/test_perf_atq.c|  1 +
 app/test-eventdev/test_perf_common.c | 65 
 app/test-eventdev/test_perf_common.h |  1 +
 app/test-eventdev/test_perf_queue.c  |  1 +
 4 files changed, 68 insertions(+)

diff --git a/app/test-eventdev/test_perf_atq.c 
b/app/test-eventdev/test_perf_atq.c
index 6082d4ff3..a2067345d 100644
--- a/app/test-eventdev/test_perf_atq.c
+++ b/app/test-eventdev/test_perf_atq.c
@@ -280,6 +280,7 @@ static const struct evt_test_ops perf_atq =  {
.opt_check  = perf_atq_opt_check,
.opt_dump   = perf_atq_opt_dump,
.test_setup = perf_test_setup,
+   .ethdev_setup   = perf_ethdev_setup,
.mempool_setup  = perf_mempool_setup,
.eventdev_setup = perf_atq_eventdev_setup,
.launch_lcores  = perf_atq_launch_lcores,
diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index 18945c0eb..ff5c499f9 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -407,6 +407,71 @@ perf_elt_init(struct rte_mempool *mp, void *arg 
__rte_unused,
memset(obj, 0, mp->elt_size);
 }

+#define NB_RX_DESC 128
+#define NB_TX_DESC 512
+int
+perf_ethdev_setup(struct evt_test *test, struct evt_options *opt)
+{
+   int i;
+   struct test_perf *t = evt_test_priv(test);
+   struct rte_eth_conf port_conf = {
+   .rxmode = {
+   .mq_mode = ETH_MQ_RX_RSS,
+   .max_rx_pkt_len = ETHER_MAX_LEN,
+   .split_hdr_size = 0,
+   .header_split   = 0,
+   .hw_ip_checksum = 0,
+   .hw_vlan_filter = 0,
+   .hw_vlan_strip  = 0,
+   .hw_vlan_extend = 0,
+   .jumbo_frame= 0,
+   .hw_strip_crc   = 1,
+   },
+   .rx_adv_conf = {
+   .rss_conf = {
+   .rss_key = NULL,
+   .rss_hf = ETH_RSS_IP,
+   },
+   },
+   };
+
+   if (opt->prod_type == EVT_PROD_TYPE_SYNT)
+   return 0;
+
+   if (!rte_eth_dev_count()) {
+   evt_err("No ethernet ports found.");
+   return -ENODEV;
+   }
+
+   for (i = 0; i < rte_eth_dev_count(); i++) {
+
+   if (rte_eth_dev_configure(i, 1, 1,
+   &port_conf)
+   < 0) {
+   evt_err("Failed to configure eth port [%d]", i);
+   return -EINVAL;
+   }
+
+   if (rte_eth_rx_queue_setup(i, 0, NB_RX_DESC,
+   rte_socket_id(), NULL, t->pool) < 0) {
+   evt_err("Failed to setup eth port [%d] rx_queue: %d.",
+   i, 0);
+   return -EINVAL;
+   }
+
+   if (rte_eth_tx_queue_setup(i, 0, NB_TX_DESC,
+   rte_socket_id(), NULL) < 0) {
+   evt_err("Failed to setup eth port [%d] tx_queue: %d.",
+   i, 0);
+   return -EINVAL;
+   }
+
+   rte_eth_promiscuous_enable(i);
+   }
+
+   return 0;
+}
+
 int
 perf_mempool_setup(struct evt_test *test, struct evt_options *opt)
 {
diff --git a/app/test-eventdev/test_perf_common.h 
b/app/test-eventdev/test_perf_common.h
index ab2e59988..5c6a615ef 100644
--- a/app/test-eventdev/test_perf_common.h
+++ b/app/test-eventdev/test_perf_common.h
@@ -157,6 +157,7 @@ perf_nb_event_ports(struct evt_options *opt)
 int perf_test_result(struct evt_test *test, struct evt_options *opt);
 int perf_opt_check(struct evt_options *opt, uint64_t nb_queues);
 int perf_test_setup(struct evt_test *test, struct evt_options *opt);
+int perf_ethdev_setup(struct evt_test *test, struct evt_options *opt);
 int perf_mempool_setup(struct evt_test *test, struct evt_options *opt);
 int perf_event_dev_port_setup(struct evt_test *test, struct evt_options *opt,
uint8_t stride, uint8_t nb_queues);
diff --git a/app/test-eventdev/test_perf_queue.c 
b/app/test-eventdev/test_perf_queue.c
index 0caf5757c..18e1bd59b 100644
--- a/app/test-eventdev/test_perf_queue.c
+++ b/app/test-eventdev/test_perf_queue.c
@@ -293,6 +293,7 @@ static const struct evt_test_ops perf_queue =  {
.opt_dump   = perf_queue_opt_dump,
.test_setup = perf_test_setup,
.mempool_setup  = perf_mempoo

[dpdk-dev] [PATCH v2 3/8] app/eventdev: add pktmbuf pool for ethdev

[Pavan Nikhilesh]

Add pktmbuf pool creation used when configuring ethernet device as event
producer.

Signed-off-by: Pavan Nikhilesh 
Acked-by: Jerin Jacob 
---

 v2 Changes:
  - Set cache size as 512.

 app/test-eventdev/test_perf_common.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index 114210ea6..18945c0eb 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -412,13 +412,24 @@ perf_mempool_setup(struct evt_test *test, struct 
evt_options *opt)
 {
struct test_perf *t = evt_test_priv(test);

-   t->pool = rte_mempool_create(test->name, /* mempool name */
+   if (opt->prod_type == EVT_PROD_TYPE_SYNT) {
+   t->pool = rte_mempool_create(test->name, /* mempool name */
opt->pool_sz, /* number of elements*/
sizeof(struct perf_elt), /* element size*/
512, /* cache size*/
0, NULL, NULL,
perf_elt_init, /* obj constructor */
NULL, opt->socket_id, 0); /* flags */
+   } else {
+   t->pool = rte_pktmbuf_pool_create(test->name, /* mempool name */
+   opt->pool_sz, /* number of elements*/
+   512, /* cache size*/
+   0,
+   RTE_MBUF_DEFAULT_BUF_SIZE,
+   opt->socket_id); /* flags */
+
+   }
+
if (t->pool == NULL) {
evt_err("failed to create mempool");
return -ENOMEM;
--
2.14.1

[dpdk-dev] [PATCH v2 5/8] app/eventdev: add ethernet device tear down

[Pavan Nikhilesh]

Add ethernet device destroy functions to stop and close ethdev ports
if they are configured when prod_type_ethdev option is enabled.

Signed-off-by: Pavan Nikhilesh 
Acked-by: Jerin Jacob 
---
 app/test-eventdev/test_perf_atq.c|  1 +
 app/test-eventdev/test_perf_common.c | 13 +
 app/test-eventdev/test_perf_common.h |  1 +
 app/test-eventdev/test_perf_queue.c  |  1 +
 4 files changed, 16 insertions(+)

diff --git a/app/test-eventdev/test_perf_atq.c 
b/app/test-eventdev/test_perf_atq.c
index a2067345d..3aa12f56f 100644
--- a/app/test-eventdev/test_perf_atq.c
+++ b/app/test-eventdev/test_perf_atq.c
@@ -286,6 +286,7 @@ static const struct evt_test_ops perf_atq =  {
.launch_lcores  = perf_atq_launch_lcores,
.eventdev_destroy   = perf_eventdev_destroy,
.mempool_destroy= perf_mempool_destroy,
+   .ethdev_destroy = perf_ethdev_destroy,
.test_result= perf_test_result,
.test_destroy   = perf_test_destroy,
 };
diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index ff5c499f9..03be8171c 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -472,6 +472,19 @@ perf_ethdev_setup(struct evt_test *test, struct 
evt_options *opt)
return 0;
 }
 
+void perf_ethdev_destroy(struct evt_test *test, struct evt_options *opt)
+{
+   int i;
+   RTE_SET_USED(test);
+
+   if (opt->prod_type == EVT_PROD_TYPE_ETH_RX_ADPTR) {
+   for (i = 0; i < rte_eth_dev_count(); i++) {
+   rte_eth_dev_stop(i);
+   rte_eth_dev_close(i);
+   }
+   }
+}
+
 int
 perf_mempool_setup(struct evt_test *test, struct evt_options *opt)
 {
diff --git a/app/test-eventdev/test_perf_common.h 
b/app/test-eventdev/test_perf_common.h
index 5c6a615ef..f33365628 100644
--- a/app/test-eventdev/test_perf_common.h
+++ b/app/test-eventdev/test_perf_common.h
@@ -167,6 +167,7 @@ int perf_launch_lcores(struct evt_test *test, struct 
evt_options *opt,
 void perf_opt_dump(struct evt_options *opt, uint8_t nb_queues);
 void perf_test_destroy(struct evt_test *test, struct evt_options *opt);
 void perf_eventdev_destroy(struct evt_test *test, struct evt_options *opt);
+void perf_ethdev_destroy(struct evt_test *test, struct evt_options *opt);
 void perf_mempool_destroy(struct evt_test *test, struct evt_options *opt);
 
 #endif /* _TEST_PERF_COMMON_ */
diff --git a/app/test-eventdev/test_perf_queue.c 
b/app/test-eventdev/test_perf_queue.c
index 18e1bd59b..d606878a1 100644
--- a/app/test-eventdev/test_perf_queue.c
+++ b/app/test-eventdev/test_perf_queue.c
@@ -298,6 +298,7 @@ static const struct evt_test_ops perf_queue =  {
.launch_lcores  = perf_queue_launch_lcores,
.eventdev_destroy   = perf_eventdev_destroy,
.mempool_destroy= perf_mempool_destroy,
+   .ethdev_destroy = perf_ethdev_destroy,
.test_result= perf_test_result,
.test_destroy   = perf_test_destroy,
 };
-- 
2.14.1

[dpdk-dev] [PATCH v2 6/8] app/eventdev: add event Rx adapter setup

[Pavan Nikhilesh]

Add functions to setup and configure Rx adapter based on the number of
ethdev ports setup.

Signed-off-by: Pavan Nikhilesh 
Acked-by: Jerin Jacob 
---

 v2 Changes:
  - Used default eventdev config values instead of hardcoded values.

 app/test-eventdev/test_perf_atq.c|  19 --
 app/test-eventdev/test_perf_common.c | 111 +--
 app/test-eventdev/test_perf_common.h |   1 +
 app/test-eventdev/test_perf_queue.c  |  16 -
 4 files changed, 120 insertions(+), 27 deletions(-)

diff --git a/app/test-eventdev/test_perf_atq.c 
b/app/test-eventdev/test_perf_atq.c
index 3aa12f56f..1fe16ed63 100644
--- a/app/test-eventdev/test_perf_atq.c
+++ b/app/test-eventdev/test_perf_atq.c
@@ -187,6 +187,7 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
uint8_t queue;
uint8_t nb_queues;
uint8_t nb_ports;
+   struct rte_event_dev_info dev_info;

nb_ports = evt_nr_active_lcores(opt->wlcores);
nb_ports += opt->prod_type == EVT_PROD_TYPE_ETH_RX_ADPTR ? 0 :
@@ -195,13 +196,22 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
nb_queues = opt->prod_type == EVT_PROD_TYPE_ETH_RX_ADPTR ?
rte_eth_dev_count() : atq_nb_event_queues(opt);

+   memset(&dev_info, 0, sizeof(struct rte_event_dev_info));
+   ret = rte_event_dev_info_get(opt->dev_id, &dev_info);
+   if (ret) {
+   evt_err("failed to get eventdev info %d", opt->dev_id);
+   return ret;
+   }
+
const struct rte_event_dev_config config = {
.nb_event_queues = nb_queues,
.nb_event_ports = nb_ports,
-   .nb_events_limit  = 4096,
+   .nb_events_limit  = dev_info.max_num_events,
.nb_event_queue_flows = opt->nb_flows,
-   .nb_event_port_dequeue_depth = 128,
-   .nb_event_port_enqueue_depth = 128,
+   .nb_event_port_dequeue_depth =
+   dev_info.max_event_port_dequeue_depth,
+   .nb_event_port_enqueue_depth =
+   dev_info.max_event_port_enqueue_depth,
};

ret = rte_event_dev_configure(opt->dev_id, &config);
@@ -225,8 +235,7 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
}
}

-   ret = perf_event_dev_port_setup(test, opt, 1 /* stride */,
-   nb_queues);
+   ret = perf_event_dev_port_setup(test, opt, 1 /* stride */, nb_queues);
if (ret)
return ret;

diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index 03be8171c..483103b11 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -231,19 +231,79 @@ perf_launch_lcores(struct evt_test *test, struct 
evt_options *opt,
return 0;
 }

+static int
+perf_event_rx_adapter_setup(struct evt_options *opt, uint8_t stride,
+   struct rte_event_port_conf prod_conf)
+{
+   int ret = 0;
+   uint16_t prod;
+
+   struct rte_event_eth_rx_adapter_queue_conf queue_conf = {
+   .ev.event = 0,
+   .ev.sched_type = opt->sched_type_list[0],
+   };
+
+   for (prod = 0; prod < rte_eth_dev_count(); prod++) {
+   uint32_t cap;
+
+   ret = rte_event_eth_rx_adapter_caps_get(opt->dev_id,
+   prod, &cap);
+   if (ret) {
+   evt_err("failed to get event rx adapter[%d]"
+   " capabilities",
+   opt->dev_id);
+   return ret;
+   }
+   queue_conf.ev.queue_id = prod * stride;
+   ret = rte_event_eth_rx_adapter_create(prod, opt->dev_id,
+   &prod_conf);
+   if (ret) {
+   evt_err("failed to create rx adapter[%d]", prod);
+   return ret;
+   }
+   ret = rte_event_eth_rx_adapter_queue_add(prod, prod, -1,
+   &queue_conf);
+   if (ret) {
+   evt_err("failed to add rx queues to adapter[%d]", prod);
+   return ret;
+   }
+
+   ret = rte_eth_dev_start(prod);
+   if (ret) {
+   evt_err("Ethernet dev [%d] failed to start."
+   " Using synthetic producer", prod);
+   return ret;
+   }
+
+   ret = rte_event_eth_rx_adapter_start(prod);
+   if (ret) {
+   evt_err("Rx adapter[%d] start failed", prod);
+   return ret;
+   }
+   printf("%s: Port[%d] using Rx adapter[%d] started\n

[dpdk-dev] [PATCH v2 8/8] doc: update app eventdev options

[Pavan Nikhilesh]

Update documentation about new --prod_type_ethdev option in app/eventdev.

Signed-off-by: Pavan Nikhilesh 
Acked-by: John McNamara 
Acked-by: Jerin Jacob 
---
 doc/guides/tools/testeventdev.rst | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/doc/guides/tools/testeventdev.rst 
b/doc/guides/tools/testeventdev.rst
index 5aa2237b5..11e608b74 100644
--- a/doc/guides/tools/testeventdev.rst
+++ b/doc/guides/tools/testeventdev.rst
@@ -146,6 +146,9 @@ The following are the application command-line options:
 
 Enable queue priority.
 
+* ``--prod_type_ethdev``
+
+Use ethernet device as producer.
 
 Eventdev Tests
 --
@@ -348,6 +351,10 @@ the timestamp in the event on the first stage and then on 
termination, it
 updates the number of cycles to forward a packet. The application uses this
 value to compute the average latency to a forward packet.
 
+When ``--prod_type_ethdev`` command line option is selected, the application
+uses the probed ethernet devices as producers by configuring them as Rx
+adapters instead of using synthetic producers.
+
 Application options
 ^^^
 
@@ -366,6 +373,7 @@ Supported application command line options are following::
 --worker_deq_depth
 --fwd_latency
 --queue_priority
+--prod_type_ethdev
 
 Example
 ^^^
@@ -377,6 +385,12 @@ Example command to run perf queue test:
sudo build/app/dpdk-test-eventdev -c 0xf -s 0x1 --vdev=event_sw0 -- \
 --test=perf_queue --plcores=2 --wlcore=3 --stlist=p --nb_pkts=0
 
+Example command to run perf queue test with ethernet ports:
+
+.. code-block:: console
+
+   sudo build/app/dpdk-test-eventdev --vdev=event_sw0 -- \
+--test=perf_queue --plcores=2 --wlcore=3 --stlist=p --prod_type_ethdev
 
 PERF_ATQ Test
 ~~~
@@ -443,6 +457,7 @@ Supported application command line options are following::
 --nb_pkts
 --worker_deq_depth
 --fwd_latency
+--prod_type_ethdev
 
 Example
 ^^^
-- 
2.14.1

[dpdk-dev] [PATCH v2 7/8] app/eventdev: add service core configuration

[Pavan Nikhilesh]

Add service core configuration for Rx adapter. The configuration picks
the least used service core to run the service on.

Signed-off-by: Pavan Nikhilesh 
---
 app/test-eventdev/evt_common.h   | 41 
 app/test-eventdev/test_perf_atq.c| 12 +++
 app/test-eventdev/test_perf_common.c | 13 
 app/test-eventdev/test_perf_queue.c  | 12 +++
 4 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/app/test-eventdev/evt_common.h b/app/test-eventdev/evt_common.h
index 0fadab4a0..042823a52 100644
--- a/app/test-eventdev/evt_common.h
+++ b/app/test-eventdev/evt_common.h
@@ -94,41 +94,36 @@ evt_has_all_types_queue(uint8_t dev_id)
 }
 
 static inline int
-evt_service_setup(uint8_t dev_id)
+evt_service_setup(uint32_t service_id)
 {
-   uint32_t service_id;
int32_t core_cnt;
unsigned int lcore = 0;
uint32_t core_array[RTE_MAX_LCORE];
uint8_t cnt;
uint8_t min_cnt = UINT8_MAX;
 
-   if (evt_has_distributed_sched(dev_id))
-   return 0;
-
if (!rte_service_lcore_count())
return -ENOENT;
 
-   if (!rte_event_dev_service_id_get(dev_id, &service_id)) {
-   core_cnt = rte_service_lcore_list(core_array,
-   RTE_MAX_LCORE);
-   if (core_cnt < 0)
-   return -ENOENT;
-   /* Get the core which has least number of services running. */
-   while (core_cnt--) {
-   /* Reset default mapping */
-   rte_service_map_lcore_set(service_id,
-   core_array[core_cnt], 0);
-   cnt = rte_service_lcore_count_services(
-   core_array[core_cnt]);
-   if (cnt < min_cnt) {
-   lcore = core_array[core_cnt];
-   min_cnt = cnt;
-   }
+   core_cnt = rte_service_lcore_list(core_array,
+   RTE_MAX_LCORE);
+   if (core_cnt < 0)
+   return -ENOENT;
+   /* Get the core which has least number of services running. */
+   while (core_cnt--) {
+   /* Reset default mapping */
+   rte_service_map_lcore_set(service_id,
+   core_array[core_cnt], 0);
+   cnt = rte_service_lcore_count_services(
+   core_array[core_cnt]);
+   if (cnt < min_cnt) {
+   lcore = core_array[core_cnt];
+   min_cnt = cnt;
}
-   if (rte_service_map_lcore_set(service_id, lcore, 1))
-   return -ENOENT;
}
+   if (rte_service_map_lcore_set(service_id, lcore, 1))
+   return -ENOENT;
+
return 0;
 }
 
diff --git a/app/test-eventdev/test_perf_atq.c 
b/app/test-eventdev/test_perf_atq.c
index 1fe16ed63..8ca07d4f8 100644
--- a/app/test-eventdev/test_perf_atq.c
+++ b/app/test-eventdev/test_perf_atq.c
@@ -239,10 +239,14 @@ perf_atq_eventdev_setup(struct evt_test *test, struct 
evt_options *opt)
if (ret)
return ret;
 
-   ret = evt_service_setup(opt->dev_id);
-   if (ret) {
-   evt_err("No service lcore found to run event dev.");
-   return ret;
+   if (!evt_has_distributed_sched(opt->dev_id)) {
+   uint32_t service_id;
+   rte_event_dev_service_id_get(opt->dev_id, &service_id);
+   ret = evt_service_setup(service_id);
+   if (ret) {
+   evt_err("No service lcore found to run event dev.");
+   return ret;
+   }
}
 
ret = rte_event_dev_start(opt->dev_id);
diff --git a/app/test-eventdev/test_perf_common.c 
b/app/test-eventdev/test_perf_common.c
index 483103b11..e03a80272 100644
--- a/app/test-eventdev/test_perf_common.c
+++ b/app/test-eventdev/test_perf_common.c
@@ -268,6 +268,19 @@ perf_event_rx_adapter_setup(struct evt_options *opt, 
uint8_t stride,
return ret;
}
 
+   if (!(cap & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT)) {
+   uint32_t service_id;
+
+   rte_event_eth_rx_adapter_service_id_get(prod,
+   &service_id);
+   ret = evt_service_setup(service_id);
+   if (ret) {
+   evt_err("Failed to setup service core"
+   " for Rx adapter\n");
+   return ret;
+   }
+   }
+
ret = rte_eth_dev_start(prod);
if (ret) {
evt_err("Ethernet dev [%d] failed to start."
diff --git a/app/test-eventdev/test_perf_queue.c 
b/app/test-eventdev/test_perf_queue.c

Re: [dpdk-dev] [PATCH 3/3] doc: add description of traffic metering and policing funcs in testpmd

[Marko Kovacevic]

On 20/11/2017 16:39, Jasvinder Singh wrote:

Signed-off-by: Jasvinder Singh 

    Acked-by: Marko Kovacevic 

Re: [dpdk-dev] [PATCH 3/3] doc: add description of traffic metering and policing funcs in testpmd

[Mcnamara, John]

> -Original Message-
> From: Singh, Jasvinder
> Sent: Monday, November 20, 2017 4:39 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian ; Wu, Jingjing
> ; Mcnamara, John 
> Subject: [PATCH 3/3] doc: add description of traffic metering and policing
> funcs in testpmd
> 
> Signed-off-by: Jasvinder Singh 


Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH] eventdev: fix doxygen comments

[Laatz, Kevin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pavan Nikhilesh
> Sent: Saturday, December 9, 2017 2:40 PM
> To: jerin.ja...@caviumnetworks.com
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH] eventdev: fix doxygen comments
> 
> Fix doxygen return values and indentation.
> 
> Fixes: 64103dbcd673 ("eventdev: add dev attribute get function")
> 
> Signed-off-by: Pavan Nikhilesh 
> ---

Acked-by: Kevin Laatz 

Re: [dpdk-dev] [PATCH 13/13] doc: update example eventdev_pipeline

[Laatz, Kevin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Pavan Nikhilesh
> Sent: Thursday, December 7, 2017 8:37 PM
> To: Eads, Gage ;
> jerin.jacobkollanukka...@cavium.com; Van Haaren, Harry
> ; Rao, Nikhil ;
> hemant.agra...@nxp.com; Ma, Liang J 
> Cc: dev@dpdk.org; Pavan Nikhilesh 
> Subject: [dpdk-dev] [PATCH 13/13] doc: update example eventdev_pipeline
> 
> Removed eventdev sw pmd specific information in document, renamed the
> document from eventdev_pipeline_sw_pmd to eventdev_pipeline.
> 
> Signed-off-by: Pavan Nikhilesh 
> ---

Acked-by: Kevin Laatz 

Re: [dpdk-dev] [PATCH 3/3] doc: add description of traffic metering and policing funcs in testpmd

[Mcnamara, John]

Note, there is a devtools/check-git-log.sh warning on the commit subject. Maybe 
use
something like this instead, if you update the patchset, or the maintainer can 
fix
inline:

doc: add metering and policing funcs to testpmd docs

Re: [dpdk-dev] [PATCHv2 3/4] Makefiles: Add experimental tag check and warnings to trigger on use

[Bruce Richardson]

On Fri, Dec 08, 2017 at 12:14:34PM -0500, Neil Horman wrote:
> Add checks during build to ensure that all symbols in the EXPERIMENTAL
> version map section have __experimental tags on their definitions, and
> enable the warnings needed to announce their use.  Also add a
> ALLOW_EXPERIMENTAL_FUNCTIONS variable check to allow for in-tree dpdk
> libraries to override those checks.
> 
> Signed-off-by: Neil Horman 
> CC: Thomas Monjalon 
> CC: "Mcnamara, John" 
> ---

Hi Neil,

if I read the patch functionality correctly, the
ALLOW_EXPERIMENTAL_FUNCTIONS variable simply suppresses all errors for
deprecated functions. However, what we really want is just to suppress
the errors for the experimental functions, and not any that really are
deprecated. While we may not have any now in DPDK, that doesn't mean it
might not be useful to have some in future.

Therefore, would an alternative scheme work where the experimental tag
is set to empty if the ALLOW_EXPERIMENTAL_FUNCTIONS define is set when
compiling?

/Bruce

Re: [dpdk-dev] [PATCH 2/3] eal: add synchronous multi-process communication

[Burakov, Anatoly]

On 30-Nov-17 6:44 PM, Jianfeng Tan wrote:

We need the synchronous way for multi-process communication, that
is to say we need an immediate response after we send a message
to the other side.

We will stop the mp_handler thread, and after sending message,
the send thread will wait there for reponse and process the
respond.

Suggested-by: Anatoly Burakov 
Signed-off-by: Jianfeng Tan 
---
  lib/librte_eal/common/eal_common_proc.c | 53 +++--
  lib/librte_eal/common/include/rte_eal.h |  5 +++-
  2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_proc.c 
b/lib/librte_eal/common/eal_common_proc.c
index 5d0a095..65ebaf2 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -30,6 +30,8 @@
   *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
   */
  
+#define _GNU_SOURCE

+


shouldn't this be in Makefile flags?


  #include 
  #include 
  #include 
@@ -41,6 +43,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  
  #include 

  #include 
@@ -134,6 +138,7 @@ rte_eal_mp_action_unregister(const char *name)
  
  struct mp_fds {

int efd;
+   int evfd; /* eventfd used for pausing mp_handler thread */
  
  	union {

/* fds for primary process */
@@ -331,6 +336,13 @@ mp_handler(void *arg __rte_unused)
exit(EXIT_FAILURE);
}
  
+	ev.data.fd = mp_fds.evfd;

+   if (epoll_ctl(mp_fds.efd, EPOLL_CTL_ADD, ev.data.fd, &ev) < 0) {
+   RTE_LOG(ERR, EAL, "epoll_ctl failed: %s\n",
+   strerror(errno));
+   exit(EXIT_FAILURE);


here and in other places - rte_exit?


+   }
+
events = calloc(20, sizeof ev);
  
  	while (1) {

@@ -348,6 +360,14 @@ mp_handler(void *arg __rte_unused)
continue;
}
  
+			if (events[i].data.fd == mp_fds.evfd) {

+   RTE_LOG(INFO, EAL, "mp_handler thread will 
pause\n");
+   pause();
+   RTE_LOG(INFO, EAL, "mp_handler thread stops 
pausing\n");
+
+   continue;
+   }
+
fd = events[i].data.fd;
  
  			if ((events[i].events & EPOLLIN)) {

@@ -377,13 +397,14 @@ mp_handler(void *arg __rte_unused)
return NULL;
  }
  
+static pthread_t tid;

+
  int
  rte_eal_mp_channel_init(void)
  {
int i, fd, ret;
const char *path;
struct sockaddr_un un;
-   pthread_t tid;
char thread_name[RTE_MAX_THREAD_NAME_LEN];
  
  	mp_fds.efd = epoll_create1(0);

@@ -462,6 +483,8 @@ rte_eal_mp_channel_init(void)
return -1;
}
  
+	mp_fds.evfd = eventfd(0, 0);

+
return 0;
  }
  
@@ -485,7 +508,8 @@ rte_eal_mp_sendmsg(const char *action_name,

   const void *params,
   int len_params,
   int fds[],
-  int fds_num)
+  int fds_num,
+  int need_ack)


I think "need_ack" is a misnomer because what we really want is not 
"ack" but a response.


More importantly, i think for clarity's sake, this should be a separate 
function - something like rte_eal_mp_sendreq() or maybe a better name 
(reqdata? communicate?). Also, i don't think reusing send parameters is 
a good idea - a user is expecting a response, so a user allocates data 
for a response separately from requests, and passes it explicitly.



  {
int i;
int ret = 0;
@@ -511,6 +535,11 @@ rte_eal_mp_sendmsg(const char *action_name,
  
  	RTE_LOG(INFO, EAL, "send msg: %s, %d\n", action_name, len_msg);
  
+	if (need_ack) {

+   // stop mp_handler thread.


Do we accept C++-style comments?


+   eventfd_write(mp_fds.evfd, (eventfd_t)1);
+   }
+
msg = malloc(len_msg);
if (!msg) {
RTE_LOG(ERR, EAL, "Cannot alloc memory for msg\n");
@@ -547,12 +576,32 @@ rte_eal_mp_sendmsg(const char *action_name,
ret = send_msg(mp_fds.secondaries[i], &msgh);
if (ret < 0)
break;
+
+   if (need_ack) {
+   /* We will hang there until the other side
+* responses and what if other side is sending
+* msg at the same time?
+*/
+   process_msg(mp_fds.secondaries[i]);
+   }
}
} else {
ret = send_msg(mp_fds.primary, &msgh);
+
+   if (ret > 0 && need_ack) {
+   // We will hang there until the other side responses
+   ret = process_msg(mp_fds.primary);
+   }
}
  
  	free(msg);
  
+	if (need_ack) {

+   // start mp_handler 

Re: [dpdk-dev] [PATCH v1 0/8] Bus control framework

[Shreyansh Jain]

Hello Gaetan,

(I am assuming that this series is still valid for 18.02 and you will 
spin a new version of this.)


On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:

Probing policy was introduced in the previous release as a configuration item.
It was thus added to the generic bus structure, breaking its ABI.

In this release, the IOVA mode can be read from a bus to configure the
EAL. This new configuration element also broke the bus ABI when it was
added.

As new operators had to be implemented for the probe policy item, these
patches were developed to help mitigate this issue.

This control framework allows to expand the rte_bus API without breaking
its ABI. It is meant to be used with configuration elements that may
only be valid for a few buses, while the others would remain untouched
and unaware of the evolution.

A central control operator is used, similarly to the working of rte_flow
API in the ether layer. Each driver thus chooses to expose a set of
operators relevant to its implementation. The caller is then free to use
those if they are available.


I like the overall idea - similar to an ioctl.
It would help extend the control knobs of buses (drivers?) without 
adding additional dependency on ABI/API.


+1



Both Probe mode and IOVA mode operators are implemented for the PCI bus. >

[...]

Re: [dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework

[Shreyansh Jain]

On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:

New configuration elements are added to the buses. They make the ABI
unstable and will continue to do so.

This new control scheme allows to add new bus operators without
breaking the ABI and by only expanding the API.

This helps having more stability in core EAL subsystems, while allowing
flexibility for future evolutions.

Signed-off-by: Gaetan Rivet 
---
  lib/librte_eal/common/eal_common_bus.c  |  9 +++
  lib/librte_eal/common/include/rte_bus.h | 46 +
  2 files changed, 55 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_bus.c 
b/lib/librte_eal/common/eal_common_bus.c
index 3c66a02..65d7229 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -42,6 +42,13 @@
  struct rte_bus_list rte_bus_list =
TAILQ_HEAD_INITIALIZER(rte_bus_list);
  
+static rte_bus_ctrl_t

+rte_bus_default_ctrl(enum rte_bus_ctrl_op op __rte_unused,
+enum rte_bus_ctrl_item item __rte_unused)
+{
+   return NULL;
+}
+
  void
  rte_bus_register(struct rte_bus *bus)
  {
@@ -53,6 +60,8 @@ rte_bus_register(struct rte_bus *bus)
RTE_VERIFY(bus->find_device);
/* Buses supporting driver plug also require unplug. */
RTE_VERIFY(!bus->plug || bus->unplug);
+   if (bus->ctrl == NULL)
+   bus->ctrl = &rte_bus_default_ctrl;
  
  	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);

RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
diff --git a/lib/librte_eal/common/include/rte_bus.h 
b/lib/librte_eal/common/include/rte_bus.h
index 331d954..bd3c28e 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -183,6 +183,51 @@ struct rte_bus_conf {
enum rte_bus_probe_mode probe_mode; /**< Probe policy. */
  };
  
+/**

+ * Bus configuration items.
+ */
+enum rte_bus_ctrl_item {
+   RTE_BUS_CTRL_PROBE_MODE = 0,
+   RTE_BUS_CTRL_ITEM_MAX,
+};


I am assuming that a driver implementation can take more than ITEM_MAX 
control knobs. It is opaque to the library. Are we on same page?


For example, a bus driver can implement:

rte_bus_XXX_ctrl_item {

RTE_BUS_XYZ_KNOB_1 = 100,
RTE_BUS_XYZ_KNOB_2,
RTE_BUS_XYZ_KNOB_3,
};

without the library knowing or restricting the API to RTE_BUS_CTRL_ITEM_MAX.

I see that in your code for PCI (Patch 5/8: pci_ctrl) you have 
restricted the control knob to RTE_BUS_CTRL_ITEM_MAX.

I hope that such restrictions would not float to library layer.

If we are on same page, should this be documented as a code comment 
somewhere?

if not, do you think what I am stating makes sense?


+
+/**
+ * Bus configuration operations.
+ */
+enum rte_bus_ctrl_op {
+   RTE_BUS_CTRL_GET = 0,
+   RTE_BUS_CTRL_SET,
+   RTE_BUS_CTRL_RESET,
+   RTE_BUS_CTRL_OP_MAX,
+};


Similarly, the driver implementation can choose to implement a operation 
which is not defined in the above structures. Obviously, the application 
is expected to know - it being a custom knob.


[...]

Re: [dpdk-dev] [PATCH 15/39] examples/ipsec-secgw: convert to new ethdev offloads API

[Radu Nicolau]

Hi,

Comment inline


On 11/23/2017 12:19 PM, Shahaf Shuler wrote:

Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler 
---
  examples/ipsec-secgw/ipsec-secgw.c | 27 +--
  1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
b/examples/ipsec-secgw/ipsec-secgw.c
index c98454a90..6e538a1ab 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -217,6 +217,8 @@ static struct rte_eth_conf port_conf = {
},
.txmode = {
.mq_mode = ETH_MQ_TX_NONE,
+   .offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
+DEV_TX_OFFLOAD_MULTI_SEGS),
},
  };
  
@@ -1394,6 +1396,22 @@ port_init(uint16_t portid)

if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_SECURITY)
port_conf.txmode.offloads |= DEV_TX_OFFLOAD_SECURITY;
  
+	if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=

+   port_conf.rxmode.offloads) {
+   printf("Some Rx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  portid, port_conf.rxmode.offloads,
+  dev_info.rx_offload_capa);
+   port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
+   }
+   if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
+port_conf.txmode.offloads) {
+   printf("Some Tx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  portid, port_conf.txmode.offloads,
+  dev_info.tx_offload_capa);
+   port_conf.txmode.offloads &= dev_info.tx_offload_capa;
+   }
I don't think that clearing the offload flags that are not advertised in 
the capabilities is a good approach, although it may be the right one. 
From what I can see there are more PMDs that don't fully populate the 
offload capabilities, but actually check for them in the configure/start 
function. One of them is ixgbe, which needs CRC strip enabled when IPSec 
is enabled, and will fail to start otherwise. So although it supports 
CRC strip it does not set the flag in the capabilities, but checks it in 
the start function.
I would propose to just print a warning if a requested offload is not 
set in the capabilities, but let the pmd start fail if it is not really 
supported.



ret = rte_eth_dev_configure(portid, nb_rx_queue, nb_tx_queue,
&port_conf);
if (ret < 0)
@@ -1420,7 +1438,8 @@ port_init(uint16_t portid)
printf("Setup txq=%u,%d,%d\n", lcore_id, tx_queueid, socket_id);
  
  		txconf = &dev_info.default_txconf;

-   txconf->txq_flags = 0;
+   txconf->txq_flags = ETH_TXQ_FLAGS_IGNORE;
+   txconf->offloads = port_conf.txmode.offloads;
  
  		ret = rte_eth_tx_queue_setup(portid, tx_queueid, nb_txd,

socket_id, txconf);
@@ -1434,6 +1453,8 @@ port_init(uint16_t portid)
  
  		/* init RX queues */

for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
+   struct rte_eth_rxconf rxq_conf;
+
if (portid != qconf->rx_queue_list[queue].port_id)
continue;
  
@@ -1442,8 +1463,10 @@ port_init(uint16_t portid)

printf("Setup rxq=%d,%d,%d\n", portid, rx_queueid,
socket_id);
  
+			rxq_conf = dev_info.default_rxconf;

+   rxq_conf.offloads = port_conf.rxmode.offloads;
ret = rte_eth_rx_queue_setup(portid, rx_queueid,
-   nb_rxd, socket_id, NULL,
+   nb_rxd, socket_id, &rxq_conf,
socket_ctx[socket_id].mbuf_pool);
if (ret < 0)
rte_exit(EXIT_FAILURE,

Re: [dpdk-dev] [PATCH v2 2/3] crypto: fix pedantic compilation errors

[De Lara Guarch, Pablo]

> -Original Message-
> From: Nelio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> Sent: Thursday, November 23, 2017 10:03 AM
> To: Akhil Goyal ; Doherty, Declan
> 
> Cc: dev@dpdk.org; Gaetan Rivet ; De Lara
> Guarch, Pablo ; sta...@dpdk.org
> Subject: [PATCH v2 2/3] crypto: fix pedantic compilation errors
> 

...

> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -121,7 +121,7 @@ struct rte_crypto_op {
>   rte_iova_t phys_addr;
>   /**< physical address of crypto operation */
> 
> - RTE_STD_C11
> + __extension__

Hi Nelio,

Since RTE_STD_C11 is basically __extension__ when __STDC_VERSION__ is not 
defined,
Is this forcing __extension__ to be used no matter what? (even if C11 is not 
supported).

Thanks,
Pablo

Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix missing ingress flow attribute

[Radu Nicolau]

On 12/4/2017 2:11 PM, Nelio Laranjeiro wrote:

Generic flow API have both direction bits, ingress and egress for rules
which may work on both sides.

Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
Cc: akhil.go...@nxp.com

Signed-off-by: Nelio Laranjeiro 
---
  

Acked-by: Radu Nicolau 

Re: [dpdk-dev] [PATCH 1/6] doc: add empty PMDs todo list

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 1:43 AM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh 
> Subject: [PATCH 1/6] doc: add empty PMDs todo list
> 
> Some library modifications require extra checks or modifications in PMDs
> but currently we don't have a good way to trace these modifications.
> 
> And number of these kind of updates increasing by time.
> 
> This is an effort to document and track library updates that requires
> attention in PMDs.
> 
> Although this is under documentation main use case is for internal
> development.
> 
> Signed-off-by: Ferruh Yigit 

>...
>
> +PMDs internal TODO list
> +===
> +
> +This is the list for tracking required PMD changes triggered by library
> modifications.
> +
> +.. table:: PMDs internal TODO list
> +
> + +--+--+--++--+
> + | TODO | PMDs | Deadline | Related Commit | Note |
> + +==+==+==++==+
> + |  |  |  ||  |
> + +--+--+--++--+


The table should have a label, and it should be indented to the level of .. 
table
as shown in the guidelines: 
http://dpdk.org/doc/guides/contributing/documentation.html#rst-guidelines

Something like this:

.. _table_nic_todo:

.. table:: PMDs internal TODO list

   +--+--+--++--+
   | TODO | PMDs | Deadline | Related Commit | Note |
   +==+==+==++==+
   |  |  |  ||  |
   +--+--+--++--+

However, it isn't an error so:


Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH 3/3] vfio: use the generic multi-process channel

[Burakov, Anatoly]

On 30-Nov-17 6:44 PM, Jianfeng Tan wrote:

Previously, vfio has its own channel for the secondary process to
get container fd and group fd from the primary process.

This patch changes to use the generic mp channel.

Signed-off-by: Jianfeng Tan 
---
  lib/librte_eal/linuxapp/eal/eal.c  |  14 +-
  lib/librte_eal/linuxapp/eal/eal_vfio.c | 139 +++--
  lib/librte_eal/linuxapp/eal/eal_vfio.h |  15 +-
  lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 416 -
  4 files changed, 109 insertions(+), 475 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
b/lib/librte_eal/linuxapp/eal/eal.c
index a84eab4..93824bf 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c


<...snip...>


-   default:
-   RTE_LOG(ERR, EAL, "  cannot get container fd!\n");
-   close(socket_fd);
-   return -1;
-   }
+   ret = rte_eal_mp_sendmsg("vfio", &p, sizeof(p), NULL, 0, 1);
+   if (ret < 0) {
+   RTE_LOG(ERR, EAL, "  cannot request group fd!\n");
+   cur_grp->group_no = -1;
+   } else {
+   cur_grp->group_no = iommu_group_no;
+   vfio_cfg.vfio_active_groups++;
}


Either i'm missing something here, or i don't see where we actually 
store the group fd (e.g. the "cur_gtp->fd = vfio_group_fd" part from the 
previous code).


Also, this is why i mentioned "receive parameters" in comments to 
previous patch - looking at this code, it is quite unclear that the 
return from rte_eal_mp_sendmsg is either error or, well, "something", 
defined as "whatever mp_action returns". It would be much clearer if we 
were explicitly getting some data in response.



-   return -1;
+
+   return ret;
  }
  
  


<...snip...>


+   /* For secondary process, request container fd from primary process */
+
+   p.req = SOCKET_REQ_CONTAINER;
+
+   ret = rte_eal_mp_sendmsg("vfio", &p, sizeof(p), NULL, 0, 1);
+   if (ret < 0)
+   RTE_LOG(ERR, EAL, "  cannot request container fd!\n");


Again here, looks counter-intuitive to get container fd in return - it 
would've been much clearer to have a separate response parameter.



+
+   return ret;
  }
  


<...snip...>


  
  static int

-vfio_mp_sync_socket_setup(void)
+vfio_mp_secondary(const void *params, int len, int fds[],
+ int fds_num __rte_unused)


fds_num isn't unused here.


  {



--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 29/39] examples/vm_power_manager: convert to new offloads API

[Hunt, David]

On 23/11/2017 12:19 PM, Shahaf Shuler wrote:

Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler 
---
  examples/vm_power_manager/main.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/examples/vm_power_manager/main.c b/examples/vm_power_manager/main.c
index 399fbdd43..53d587d83 100644
--- a/examples/vm_power_manager/main.c
+++ b/examples/vm_power_manager/main.c
@@ -74,7 +74,10 @@ static volatile bool force_quit;
  
  //

  static const struct rte_eth_conf port_conf_default = {
-   .rxmode = { .max_rx_pkt_len = ETHER_MAX_LEN }
+   .rxmode = {
+   .max_rx_pkt_len = ETHER_MAX_LEN,
+   .ignore_offload_bitfield = 1,
+   }
  };
  
  static inline int

@@ -84,6 +87,8 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
const uint16_t rx_rings = 1, tx_rings = 1;
int retval;
uint16_t q;
+   struct rte_eth_dev_info dev_info;
+   struct rte_eth_txconf txq_conf;
  
  	if (port >= rte_eth_dev_count())

return -1;
@@ -101,10 +106,13 @@ port_init(uint16_t port, struct rte_mempool *mbuf_pool)
return retval;
}
  
+	rte_eth_dev_info_get(port, &dev_info);

+   txq_conf = dev_info.default_txconf;
+   txq_conf.txq_flags = ETH_TXQ_FLAGS_IGNORE;
/* Allocate and set up 1 TX queue per Ethernet port. */
for (q = 0; q < tx_rings; q++) {
retval = rte_eth_tx_queue_setup(port, q, TX_RING_SIZE,
-   rte_eth_dev_socket_id(port), NULL);
+   rte_eth_dev_socket_id(port), &txq_conf);
if (retval < 0)
return retval;
}


Looks good to me.

Acked-by: David Hunt 

Re: [dpdk-dev] [PATCH] test/test_cryptodev: fix missing include

[De Lara Guarch, Pablo]

Hi Jerin,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jerin Jacob
> Sent: Tuesday, November 28, 2017 12:51 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan ; Jerin Jacob
> 
> Subject: [dpdk-dev] [PATCH] test/test_cryptodev: fix missing include
> 
> time() is defined in time.h
> 
> Fixes: ffbe3be0d4 ("app/test: add libcrypto")

Missing Cc: sta...@dpdk.org, will add it for you.

Apart from that.

Applied to dpdk-next-crypto.
Thanks,

Pablo

Re: [dpdk-dev] [PATCH v1 4/8] bus: add probe mode setter

[Shreyansh Jain]

On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:

Introduce new rte_bus operation to configure the probe policy.

Implementation is required from buses interested in supporting
this configuration element.



[...]


diff --git a/lib/librte_eal/common/eal_common_options.c 
b/lib/librte_eal/common/eal_common_options.c
index e40c049..630c9d2 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -997,29 +997,24 @@ int
  eal_parse_common_option(int opt, const char *optarg,
struct internal_config *conf)
  {
-   static int b_used;
-   static int w_used;
-
switch (opt) {
/* blacklist */
case 'b':
-   if (w_used)
-   goto bw_used;
+   if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0)
+   return -1;


Generic layer shouldn't be concerned about "pci" or other bus.
Problem would be to find which bus this option needs to be set.

What I can think of as options is:
1. Storing this configuration until we can parse the argument for which 
 the argument has been created. That would mean changing the way 
the "-b" and "-w" are passed and to allow non-PCI device identifier to 
be passed.

2. Call each bus bus->ctrl and let it decide what to do based on the args.
 - so, have a wrapper over rte_bus_probe_mode_set for all buses rather 
than taking any one bus as option.


(2) sounds most plausible for now as the application will not send the 
bus name as argument.
And if brute force is not required, we need to split the argument to 
know the bus - after making the device naming standardized (:<...>)


Even before that, we need to agree that "-w' and "-b" are not more valid 
only for PCIs.


Above is more of loud thinging - I don't have concrete thought on this 
for now. I'll revisit this after reviewing the patches in this series.



if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
optarg) < 0) {
return -1;
}
-   b_used = 1;
break;
/* whitelist */
case 'w':
-   if (b_used)
-   goto bw_used;
+   if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0)
+   return -1;
if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
optarg) < 0) {
return -1;
}
-   w_used = 1;
break;
/* coremask */
case 'c':
@@ -1165,10 +1160,6 @@ eal_parse_common_option(int opt, const char *optarg,


[...]

Re: [dpdk-dev] [PATCH 1/3] lib/cmdline: add echo support in batch loading from file

[Burakov, Anatoly]

On 15-Nov-17 3:45 PM, Xueming Li wrote:

Add echo option to echo commandline to screen when running loaded
scripts from file.

Signed-off-by: Xueming Li 
---


<...snip...>


@@ -86,7 +87,7 @@ cmdline_file_new(cmdline_parse_ctx_t *ctx, const char 
*prompt, const char *path)
dprintf("open() failed\n");
return NULL;
}
-   return cmdline_new(ctx, prompt, fd, -1);
+   return cmdline_new(ctx, prompt, fd, echo ? 1 : -1);


This should probably be "echo ? STDOUT_FILENO : -1", to make the 
intention clearer.







--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v1 4/8] bus: add probe mode setter

[Shreyansh Jain]

On Monday 11 December 2017 06:09 PM, Shreyansh Jain wrote:

On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:

Introduce new rte_bus operation to configure the probe policy.

Implementation is required from buses interested in supporting
this configuration element.



[...]

diff --git a/lib/librte_eal/common/eal_common_options.c 
b/lib/librte_eal/common/eal_common_options.c

index e40c049..630c9d2 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -997,29 +997,24 @@ int
  eal_parse_common_option(int opt, const char *optarg,
  struct internal_config *conf)
  {
-    static int b_used;
-    static int w_used;
-
  switch (opt) {
  /* blacklist */
  case 'b':
-    if (w_used)
-    goto bw_used;
+    if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0)
+    return -1;


Generic layer shouldn't be concerned about "pci" or other bus.
Problem would be to find which bus this option needs to be set.

What I can think of as options is:
1. Storing this configuration until we can parse the argument for which 
 the argument has been created. That would mean changing the way 
the "-b" and "-w" are passed and to allow non-PCI device identifier to 
be passed.

2. Call each bus bus->ctrl and let it decide what to do based on the args.
  - so, have a wrapper over rte_bus_probe_mode_set for all buses rather 
than taking any one bus as option.


(2) sounds most plausible for now as the application will not send the 
bus name as argument.
And if brute force is not required, we need to split the argument to 
know the bus - after making the device naming standardized (:<...>)


Even before that, we need to agree that "-w' and "-b" are not more valid 
only for PCIs.


Above is more of loud thinging - I don't have concrete thought on this 
for now. I'll revisit this after reviewing the patches in this series.


Apologies. I see that some work has been done for this in devargs series 
- I was too quick to reply on this.

Let me come back to you on this after reading through that series.




  if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
  optarg) < 0) {
  return -1;
  }
-    b_used = 1;
  break;
  /* whitelist */
  case 'w':
-    if (b_used)
-    goto bw_used;
+    if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0)
+    return -1;
  if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
  optarg) < 0) {
  return -1;
  }
-    w_used = 1;
  break;
  /* coremask */
  case 'c':
@@ -1165,10 +1160,6 @@ eal_parse_common_option(int opt, const char 
*optarg,


[...]

Re: [dpdk-dev] [PATCH 3/3] test: update batch loading test

[Burakov, Anatoly]

On 15-Nov-17 3:45 PM, Xueming Li wrote:

Support echo back in batch loading.

Signed-off-by: Xueming Li 
---


Acked-by: Anatoly Burakov 

--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 15/39] examples/ipsec-secgw: convert to new ethdev offloads API

[Shahaf Shuler]

Hi Radu,

Monday, December 11, 2017 1:48 PM, Radu Nicolau :
> Hi,
> 
> Comment inline
> 
> 
> On 11/23/2017 12:19 PM, Shahaf Shuler wrote:
> > Ethdev offloads API has changed since:
> >
> > commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
> > cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> >
> > This commit support the new API.
> >
> > Signed-off-by: Shahaf Shuler 
> > ---
> >   examples/ipsec-secgw/ipsec-secgw.c | 27
> +--
> >   1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> > b/examples/ipsec-secgw/ipsec-secgw.c
> > index c98454a90..6e538a1ab 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > @@ -217,6 +217,8 @@ static struct rte_eth_conf port_conf = {
> > },
> > .txmode = {
> > .mq_mode = ETH_MQ_TX_NONE,
> > +   .offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
> > +DEV_TX_OFFLOAD_MULTI_SEGS),
> > },
> >   };
> >
> > @@ -1394,6 +1396,22 @@ port_init(uint16_t portid)
> > if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_SECURITY)
> > port_conf.txmode.offloads |= DEV_TX_OFFLOAD_SECURITY;
> >
> > +   if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
> > +   port_conf.rxmode.offloads) {
> > +   printf("Some Rx offloads are not supported "
> > +  "by port %d: requested 0x%lx supported 0x%lx\n",
> > +  portid, port_conf.rxmode.offloads,
> > +  dev_info.rx_offload_capa);
> > +   port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
> > +   }
> > +   if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
> > +port_conf.txmode.offloads) {
> > +   printf("Some Tx offloads are not supported "
> > +  "by port %d: requested 0x%lx supported 0x%lx\n",
> > +  portid, port_conf.txmode.offloads,
> > +  dev_info.tx_offload_capa);
> > +   port_conf.txmode.offloads &= dev_info.tx_offload_capa;
> > +   }
> I don't think that clearing the offload flags that are not advertised in the
> capabilities is a good approach, although it may be the right one.
>  From what I can see there are more PMDs that don't fully populate the
> offload capabilities, but actually check for them in the configure/start
> function. One of them is ixgbe, which needs CRC strip enabled when IPSec is
> enabled, and will fail to start otherwise. So although it supports CRC strip 
> it
> does not set the flag in the capabilities, but checks it in the start 
> function.

Why ixgbe don't expose the CRC cap then? It seems wrong behavior to expect the 
application to set it without any cap reported. 

> I would propose to just print a warning if a requested offload is not set in 
> the
> capabilities, but let the pmd start fail if it is not really supported.


I think I agree, however not from the reason you mentioned.
It is bad to mask the un-supported offloads because the application relies on 
them to be set successfully. The application will not run successfully if the 
IPV4 checksum is not actually set (for example).

On v2 I will print just the warn. 

> 
> > ret = rte_eth_dev_configure(portid, nb_rx_queue, nb_tx_queue,
> > &port_conf);
> > if (ret < 0)
> > @@ -1420,7 +1438,8 @@ port_init(uint16_t portid)
> > printf("Setup txq=%u,%d,%d\n", lcore_id, tx_queueid,
> socket_id);
> >
> > txconf = &dev_info.default_txconf;
> > -   txconf->txq_flags = 0;
> > +   txconf->txq_flags = ETH_TXQ_FLAGS_IGNORE;
> > +   txconf->offloads = port_conf.txmode.offloads;
> >
> > ret = rte_eth_tx_queue_setup(portid, tx_queueid, nb_txd,
> > socket_id, txconf);
> > @@ -1434,6 +1453,8 @@ port_init(uint16_t portid)
> >
> > /* init RX queues */
> > for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
> > +   struct rte_eth_rxconf rxq_conf;
> > +
> > if (portid != qconf->rx_queue_list[queue].port_id)
> > continue;
> >
> > @@ -1442,8 +1463,10 @@ port_init(uint16_t portid)
> > printf("Setup rxq=%d,%d,%d\n", portid, rx_queueid,
> > socket_id);
> >
> > +   rxq_conf = dev_info.default_rxconf;
> > +   rxq_conf.offloads = port_conf.rxmode.offloads;
> > ret = rte_eth_rx_queue_setup(portid, rx_queueid,
> > -   nb_rxd, socket_id, NULL,
> > +   nb_rxd, socket_id,
> &rxq_conf,
> > socket_ctx[socket_id].mbuf_pool);
> > if (ret < 0)
> > rte_exit(EXIT_FAILURE,

Re: [dpdk-dev] [PATCH 1/2] eventdev: add implicit release disable capability

[Van Haaren, Harry]

> -Original Message-
> From: Eads, Gage
> Sent: Thursday, November 30, 2017 4:21 AM
> To: dev@dpdk.org
> Cc: jerin.ja...@caviumnetworks.com; Van Haaren, Harry
> ; Richardson, Bruce
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com;
> santosh.shu...@caviumnetworks.com
> Subject: [PATCH 1/2] eventdev: add implicit release disable capability
> 
> This commit introduces a capability for disabling the "implicit" release
> functionality for a port, which prevents the eventdev PMD from issuing
> outstanding releases for previously dequeued events when dequeuing a new
> batch of events.
> 
> If a PMD does not support this capability, the application will receive an
> error if it attempts to setup a port with implicit releases disabled.
> Otherwise, if the port is configured with implicit releases disabled, the
> application must release each dequeued event by invoking
> rte_event_enqueue_burst() with RTE_EVENT_OP_RELEASE or
> RTE_EVENT_OP_FORWARD.
> 
> Signed-off-by: Gage Eads 

Some comments inline. In general, I think this makes sense, and is the easiest 
solution that I can see.


>  drivers/event/dpaa2/dpaa2_eventdev.c   |  2 ++
>  drivers/event/octeontx/ssovf_evdev.c   |  1 +
>  drivers/event/skeleton/skeleton_eventdev.c |  1 +
>  drivers/event/sw/sw_evdev.c| 10 +++---
>  drivers/event/sw/sw_evdev.h|  1 +
>  drivers/event/sw/sw_evdev_worker.c | 16 
>  examples/eventdev_pipeline_sw_pmd/main.c   | 24 +++-
>  lib/librte_eventdev/rte_eventdev.c |  9 +
>  lib/librte_eventdev/rte_eventdev.h | 23 ---
>  test/test/test_eventdev.c  |  9 +
>  test/test/test_eventdev_sw.c   | 20 ++--
>  11 files changed, 99 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/event/dpaa2/dpaa2_eventdev.c
> b/drivers/event/dpaa2/dpaa2_eventdev.c
> index eeeb231..236b211 100644
> --- a/drivers/event/dpaa2/dpaa2_eventdev.c
> +++ b/drivers/event/dpaa2/dpaa2_eventdev.c
> @@ -440,6 +440,8 @@ dpaa2_eventdev_port_def_conf(struct rte_eventdev *dev,
> uint8_t port_id,
>   DPAA2_EVENT_MAX_PORT_DEQUEUE_DEPTH;
>   port_conf->enqueue_depth =
>   DPAA2_EVENT_MAX_PORT_ENQUEUE_DEPTH;
> + port_conf->disable_implicit_release =
> + 0;

Merge "0;" onto previous line?



> --- a/drivers/event/skeleton/skeleton_eventdev.c
> +++ b/drivers/event/skeleton/skeleton_eventdev.c
> @@ -237,6 +237,7 @@ skeleton_eventdev_port_def_conf(struct rte_eventdev
> *dev, uint8_t port_id,
>   port_conf->new_event_threshold = 32 * 1024;
>   port_conf->dequeue_depth = 16;
>   port_conf->enqueue_depth = 16;
> + port_conf->disable_implicit_release = false;

Prefer 0 to false.



> diff --git a/examples/eventdev_pipeline_sw_pmd/main.c
> b/examples/eventdev_pipeline_sw_pmd/main.c
> index 5f431d8..3910b53 100644
> --- a/examples/eventdev_pipeline_sw_pmd/main.c
> +++ b/examples/eventdev_pipeline_sw_pmd/main.c
> @@ -62,6 +62,7 @@ struct prod_data {
>  struct cons_data {
>   uint8_t dev_id;
>   uint8_t port_id;
> + bool release;

I'd personally uint8_t this instead of bool, which requires . I 
haven't seen stdbool.h in other DPDK headers, so suggesting stick with the good 
old byte-sized integers for flags.. 


>  } __rte_cache_aligned;
> 
>  static struct prod_data prod_data;
> @@ -167,6 +168,18 @@ consumer(void)
>   uint8_t outport = packets[i].mbuf->port;
>   rte_eth_tx_buffer(outport, 0, fdata->tx_buf[outport],
>   packets[i].mbuf);
> +
> + packets[i].op = RTE_EVENT_OP_RELEASE;
> + }
> +
> + if (cons_data.release) {
> + uint16_t nb_tx;
> +
> + nb_tx = rte_event_enqueue_burst(dev_id, port_id, packets, n);
> + while (nb_tx < n)
> + nb_tx += rte_event_enqueue_burst(dev_id, port_id,
> +  packets + nb_tx,
> +  n - nb_tx);
>   }
> 
>   /* Print out mpps every 1<22 packets */
> @@ -702,6 +715,7 @@ setup_eventdev(struct prod_data *prod_data,
>   };
> 
>   struct port_link worker_queues[MAX_NUM_STAGES];
> + bool disable_implicit_release;

Same uint8_t over stdbool.h comment as above


> @@ -3240,7 +3244,7 @@ test_sw_eventdev(void)
>   if (rte_lcore_count() >= 3) {
>   printf("*** Running Worker loopback test...\n");
> - ret = worker_loopback(t);
> + ret = worker_loopback(t, 0);
>   if (ret != 0) {
>   printf("ERROR - Worker loopback test FAILED.\n");
>   return ret;
> @@ -3249,6 +3253,18 @@ test_sw_eventdev(void)
>   printf("### Not enough cores for worker loopback test.\n");
>   printf("### Need at least 3 cores for test.\n");
>   }
> + if (rte_lcore_count() >= 3

Re: [dpdk-dev] [PATCH 2/3] app/testpmd: support command echo in CLI batch loading

[Burakov, Anatoly]

On 15-Nov-17 3:45 PM, Xueming Li wrote:

Use first bit of verbose_level to enable CLI echo of batch loading.

Signed-off-by: Xueming Li 
---
  app/test-pmd/cmdline.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f5a483ad7..b40fe1ac7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -15853,7 +15853,8 @@ cmdline_read_from_file(const char *filename)
  {
struct cmdline *cl;
  
-	cl = cmdline_file_new(main_ctx, "testpmd> ", filename);

+   cl = cmdline_file_new(main_ctx, "testpmd> ", filename,
+ verbose_level & 0x8000);
if (cl == NULL) {
printf("Failed to create file based cmdline context: %s\n",
   filename);



I don't see verbose_level being used in testpmd other than checking if 
it's zero, so maybe just verbose level >= 2 instead of highest 
significant bit?


--
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 1/5] pmdinfogen: fix cross compilation for ARM BE

[Bruce Richardson]

On Thu, Nov 02, 2017 at 03:38:51PM +0530, Hemant Agrawal wrote:
> cross compiling DPDK for BE mode on ARM results into errors
> 
> "PMDINFO portal/dpaa2_hw_dpio.o.pmd.c No drivers registered"
> 
> Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility")
> Cc: Neil Horman 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Jun Yang 
> Signed-off-by: Hemant Agrawal 
> ---
>  buildtools/pmdinfogen/pmdinfogen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Comment could be a bit more specific about what the problem is and how
changing the hard-coded "32" fixes it.

Haven't tested the cross compilation part myself, but this causes no
errors for 32-bit or 64-bit builds on my system. So, with some more
detail on the specifics of the fix in the commit message:

Acked-by: Bruce Richardson 

> diff --git a/buildtools/pmdinfogen/pmdinfogen.c 
> b/buildtools/pmdinfogen/pmdinfogen.c
> index e73fc76..9119e52 100644
> --- a/buildtools/pmdinfogen/pmdinfogen.c
> +++ b/buildtools/pmdinfogen/pmdinfogen.c
> @@ -181,7 +181,7 @@ static int parse_elf(struct elf_info *info, const char 
> *filename)
>   sechdrs[i].sh_offset=
>   TO_NATIVE(endian, ADDR_SIZE, sechdrs[i].sh_offset);
>   sechdrs[i].sh_size  =
> - TO_NATIVE(endian, 32, sechdrs[i].sh_size);
> + TO_NATIVE(endian, ADDR_SIZE, sechdrs[i].sh_size);
>   sechdrs[i].sh_link  =
>   TO_NATIVE(endian, 32, sechdrs[i].sh_link);
>   sechdrs[i].sh_info  =
> -- 
> 2.7.4
> 

Re: [dpdk-dev] [PATCH 2/5] lpm: fix compilation on ARM BE

[Bruce Richardson]

On Thu, Nov 02, 2017 at 03:38:52PM +0530, Hemant Agrawal wrote:
> Compiling on ARM BE using Linaro toolchain caused following
> error/warnings.
> 
> rte_lpm.c: In function ‘add_depth_big_v20’:
> rte_lpm.c:911:4: error: braces around scalar initializer [-Werror]
> { .group_idx = (uint8_t)tbl8_group_index, },
> ^
> rte_lpm.c:911:4: note: (near initialization for
>   ‘new_tbl24_entry.depth’)
> rte_lpm.c:911:6:error: field name not in record or union initializer
> { .group_idx = (uint8_t)tbl8_group_index, },
>   ^
> rte_lpm.c:911:6: note: (near initialization for
>   ‘new_tbl24_entry.depth’)
> rte_lpm.c:914:13: error: initialized field overwritten
>   [-Werror=override-init]
> .depth = 0,
> 
> Fixes: dc81ebbacaeb ("lpm: extend IPv4 next hop field")
> Cc: Michal Kobylinski 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Hemant Agrawal 
> ---

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCHv2 3/4] Makefiles: Add experimental tag check and warnings to trigger on use

[Neil Horman]

On Mon, Dec 11, 2017 at 11:35:57AM +, Bruce Richardson wrote:
> On Fri, Dec 08, 2017 at 12:14:34PM -0500, Neil Horman wrote:
> > Add checks during build to ensure that all symbols in the EXPERIMENTAL
> > version map section have __experimental tags on their definitions, and
> > enable the warnings needed to announce their use.  Also add a
> > ALLOW_EXPERIMENTAL_FUNCTIONS variable check to allow for in-tree dpdk
> > libraries to override those checks.
> > 
> > Signed-off-by: Neil Horman 
> > CC: Thomas Monjalon 
> > CC: "Mcnamara, John" 
> > ---
> 
> Hi Neil,
> 
> if I read the patch functionality correctly, the
> ALLOW_EXPERIMENTAL_FUNCTIONS variable simply suppresses all errors for
> deprecated functions. However, what we really want is just to suppress
> the errors for the experimental functions, and not any that really are
> deprecated. While we may not have any now in DPDK, that doesn't mean it
> might not be useful to have some in future.
> 
Well, they are all deprecated by the stict definition of the term, which is to
say that their usage is discouraged/disapproved.  What I think you are saying is
that you would like to differentiate reasons for deprecation (i.e. deprecation
because an api is experimental, vs deprecation because an api is broken or going
to be removed).  I'm not sure I see that we will get there anytime soon, but
yes, that seems like it would be a good ability to add in here.

> Therefore, would an alternative scheme work where the experimental tag
> is set to empty if the ALLOW_EXPERIMENTAL_FUNCTIONS define is set when
> compiling?
> 
Yes, I think that could work.  Perhaps what we can do is modify
ALLOW_EXPERIMENTAL_FUNCTIONS to set a CFLAGS like:
-DSUPPRESS_EXPERIMENTAL

And use that to toggle the definition on rte_compat.h between something that
deprecates the API call and an empty definition

Neil

> /Bruce
> 

Re: [dpdk-dev] [PATCH v2 2/3] crypto: fix pedantic compilation errors

[Nelio Laranjeiro]

Hi Pablo,

On Mon, Dec 11, 2017 at 11:49:39AM +, De Lara Guarch, Pablo wrote:
> 
> 
> > -Original Message-
> > From: Nelio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> > Sent: Thursday, November 23, 2017 10:03 AM
> > To: Akhil Goyal ; Doherty, Declan
> > 
> > Cc: dev@dpdk.org; Gaetan Rivet ; De Lara
> > Guarch, Pablo ; sta...@dpdk.org
> > Subject: [PATCH v2 2/3] crypto: fix pedantic compilation errors
> > 
> 
> ...
> 
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -121,7 +121,7 @@ struct rte_crypto_op {
> > rte_iova_t phys_addr;
> > /**< physical address of crypto operation */
> > 
> > -   RTE_STD_C11
> > +   __extension__
> 
> Hi Nelio,
> 
> Since RTE_STD_C11 is basically __extension__ when __STDC_VERSION__ is not 
> defined,
> Is this forcing __extension__ to be used no matter what? (even if C11 is not 
> supported).

Yes

Thanks,

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH 5/5] net/ixgbe: fix compilation on ARM BE

[Bruce Richardson]

On Thu, Nov 02, 2017 at 03:38:55PM +0530, Hemant Agrawal wrote:
> fixes the following compilation error on compiling with ARM BE compiler
> 
> ixgbe_common.c: In function ‘ixgbe_host_interface_command’:
> ixgbe_common.c:4610:22: error: passing argument 1 of
> ‘__builtin_bswap32’ makes integer from pointer without a cast
> [-Werror=int-conversion]
>IXGBE_LE32_TO_CPUS(&buffer[bi]);
>   ^
> Fixes: aa4fc14d2cee ("ixgbe: update base driver")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Hemant Agrawal 

Looks harmless!

Acked-by: Bruce Richardson 

[dpdk-dev] [PATCH] net/failsafe: add Rx interrupts

[Moti Haimovsky]

This patch adds support for registering and waiting for Rx
interrupts in failsafe PMD. This allows applications to wait
for Rx events from the PMD using the DPDK rte_epoll subsystem.
The failsafe PMD presents to the application a facade of a single
device to be handled by the application while internally it manages
several devices on behalf of the application including packets
transmission and reception.
The Proposed failsafe Rx interrupt scheme follows this approach.
The failsafe PMD will present the application with a single set of Rx
interrupt vectors representing the failsafe Rx queues, while internally
it will serve as an interrupt proxy for its subdevices.
This will allow applications to wait for Rx traffic from the failsafe
PMD by registering and waiting for Rx events from its Rx queues.
In order to support this the following is suggested:
  * Every Rx queue in the failsafe (virtual) device will be assigned a
Linux event file descriptor (efd) and an enable_interrupts flag.
  * The failsafe PMD will fill in its rte_intr_handle structure with
the Rx efds assigned previously and register them with the EAL.
  * The failsafe driver will create a private epoll fd (epfd) and will
allocate enough space to handle all the Rx events from all its
subdevices.
  * Acting as an application,
for each Rx queue in each active subdevice the failsafe will:
  o Register the Rx queue with the EAL.
  o Pass the EAL the failsafe private epoll fd as the epfd to
register the Rx queue event on.
  o Pass the EAL, as a parameter, the pointer to the failsafe Rx
queue that handles this Rx queue.
  o Using the DPDK service callbacks, the failsafe PMD will launch
an Rx proxy service that will Wait on the epoll fd for Rx events
from the sub-devices.
  o For each Rx event received the proxy service will
 - Retrieve the pointer to failsafe Rx queue that handles this
   subdevice Rx queue from the user info returned by the EAL.
 - Trigger a failsafe Rx event on that queue by writing to the
   event fd unless interrupts are disabled for that queue.
  * The failsafe pmd will also implement the rx_queue_intr_enable and
rx_queue_intr_disable routines that will enable and disable Rx
interrupts respectively on both on the failsafe and its subdevices.

Signed-off-by: Moti Haimovsky 
---
 doc/guides/nics/features/failsafe.ini   |   1 +
 drivers/net/failsafe/Makefile   |   1 +
 drivers/net/failsafe/failsafe.c |   4 +
 drivers/net/failsafe/failsafe_ether.c   |   1 +
 drivers/net/failsafe/failsafe_intr.c| 596 
 drivers/net/failsafe/failsafe_ops.c |  21 ++
 drivers/net/failsafe/failsafe_private.h |  44 +++
 7 files changed, 668 insertions(+)
 create mode 100644 drivers/net/failsafe/failsafe_intr.c

diff --git a/doc/guides/nics/features/failsafe.ini 
b/doc/guides/nics/features/failsafe.ini
index a42e344..39ee579 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -6,6 +6,7 @@
 [Features]
 Link status  = Y
 Link status event= Y
+Rx interrupt = Y
 MTU update   = Y
 Jumbo frame  = Y
 Promiscuous mode = Y
diff --git a/drivers/net/failsafe/Makefile b/drivers/net/failsafe/Makefile
index ea2a8fe..91a734b 100644
--- a/drivers/net/failsafe/Makefile
+++ b/drivers/net/failsafe/Makefile
@@ -46,6 +46,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE) += failsafe_ops.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE) += failsafe_rxtx.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE) += failsafe_ether.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE) += failsafe_flow.c
+SRCS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE) += failsafe_intr.c
 
 # No exported include files
 
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 6bc5aba..3b5e059 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -239,6 +239,10 @@
mac->addr_bytes[2], mac->addr_bytes[3],
mac->addr_bytes[4], mac->addr_bytes[5]);
dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
+   PRIV(dev)->intr_handle = (struct rte_intr_handle){
+   .fd = -1,
+   .type = RTE_INTR_HANDLE_EXT,
+   };
return 0;
 free_args:
failsafe_args_free(dev);
diff --git a/drivers/net/failsafe/failsafe_ether.c 
b/drivers/net/failsafe/failsafe_ether.c
index 21392e5..80741ba 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -283,6 +283,7 @@
return;
switch (sdev->state) {
case DEV_STARTED:
+   failsafe_rx_intr_uninstall_subdevice(sdev);
rte_eth_dev_stop(PORT_ID(sdev));
sdev->state = DEV_ACTIVE;
/* fallthrough */
diff --git a/drivers/net/failsafe/failsafe_intr.c 
b/drivers/net/failsafe/failsafe_intr.c
new file mode 100644
index 000..2e395db
--

Re: [dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework

[Gaëtan Rivet]

On Mon, Dec 11, 2017 at 05:30:16PM +0530, Shreyansh Jain wrote:
> On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:
> > New configuration elements are added to the buses. They make the ABI
> > unstable and will continue to do so.
> > 
> > This new control scheme allows to add new bus operators without
> > breaking the ABI and by only expanding the API.
> > 
> > This helps having more stability in core EAL subsystems, while allowing
> > flexibility for future evolutions.
> > 
> > Signed-off-by: Gaetan Rivet 
> > ---
> >   lib/librte_eal/common/eal_common_bus.c  |  9 +++
> >   lib/librte_eal/common/include/rte_bus.h | 46 
> > +
> >   2 files changed, 55 insertions(+)
> > 
> > diff --git a/lib/librte_eal/common/eal_common_bus.c 
> > b/lib/librte_eal/common/eal_common_bus.c
> > index 3c66a02..65d7229 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -42,6 +42,13 @@
> >   struct rte_bus_list rte_bus_list =
> > TAILQ_HEAD_INITIALIZER(rte_bus_list);
> > +static rte_bus_ctrl_t
> > +rte_bus_default_ctrl(enum rte_bus_ctrl_op op __rte_unused,
> > +enum rte_bus_ctrl_item item __rte_unused)
> > +{
> > +   return NULL;
> > +}
> > +
> >   void
> >   rte_bus_register(struct rte_bus *bus)
> >   {
> > @@ -53,6 +60,8 @@ rte_bus_register(struct rte_bus *bus)
> > RTE_VERIFY(bus->find_device);
> > /* Buses supporting driver plug also require unplug. */
> > RTE_VERIFY(!bus->plug || bus->unplug);
> > +   if (bus->ctrl == NULL)
> > +   bus->ctrl = &rte_bus_default_ctrl;
> > TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
> > RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
> > diff --git a/lib/librte_eal/common/include/rte_bus.h 
> > b/lib/librte_eal/common/include/rte_bus.h
> > index 331d954..bd3c28e 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -183,6 +183,51 @@ struct rte_bus_conf {
> > enum rte_bus_probe_mode probe_mode; /**< Probe policy. */
> >   };
> > +/**
> > + * Bus configuration items.
> > + */
> > +enum rte_bus_ctrl_item {
> > +   RTE_BUS_CTRL_PROBE_MODE = 0,
> > +   RTE_BUS_CTRL_ITEM_MAX,
> > +};
> 
> I am assuming that a driver implementation can take more than ITEM_MAX
> control knobs. It is opaque to the library. Are we on same page?
> 
> For example, a bus driver can implement:
> 
> rte_bus_XXX_ctrl_item {
>   
>   RTE_BUS_XYZ_KNOB_1 = 100,
>   RTE_BUS_XYZ_KNOB_2,
>   RTE_BUS_XYZ_KNOB_3,
> };
> 
> without the library knowing or restricting the API to RTE_BUS_CTRL_ITEM_MAX.
> 
> I see that in your code for PCI (Patch 5/8: pci_ctrl) you have restricted
> the control knob to RTE_BUS_CTRL_ITEM_MAX.
> I hope that such restrictions would not float to library layer.
> 
> If we are on same page, should this be documented as a code comment
> somewhere?
> if not, do you think what I am stating makes sense?
> 

I see what you mean, but I'm not sure it would be a good thing.
Actually, I think proposing this ITEM_MAX was a mistake.

Regarding the specific bus knobs:

- If a single bus needs this knob, then it would be better for the dev
  to add it as part of the bus' public API, following the correct
  library versioning processes. This would not break this bus control
  structure ABI.

- If more than one bus implement this knob, then it should be proposed
  as part of the library API. Buses adding this new knob would break
  their ABI, other buses would be left untouched.

This makes me realize that proposing this ITEM_MAX value is not good to
the intended purpose of this patchset:

- If a bus implementation use a reference to ITEM_MAX, then the control
  structure ABI would be broken by any new control knob added, even if the
  bus does not implement it. Granted, it would not break the driver
  structure itself, but still. My PCI implementation is thus incorrect.

Therefore I think that it would be best to remove this ITEM_MAX altogether,
forcing bus developpers to use other ways that would not break their
ABIs every other release.

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH 2/2] crypto/mrvl: update MRVL CRYPTO PMD documentation

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Duszynski
> Sent: Thursday, November 30, 2017 1:33 PM
> To: dev@dpdk.org
> Cc: m...@semihalf.com; j...@semihalf.com; d...@marvell.com;
> nsams...@marvell.com; jianbo@arm.com; Tomasz Duszynski
> 
> Subject: [dpdk-dev] [PATCH 2/2] crypto/mrvl: update MRVL CRYPTO PMD
> documentation
> 
> Update MRVL CRYPTO PMD documentation.
> 
> Signed-off-by: Tomasz Duszynski 


This is depending on the network driver update, right?
You should say it in the cover letter. I will wait until it is applied then.

...

> -
>  Installation
>  
> 
> @@ -92,20 +85,11 @@ Currently there are two driver specific compilation
> options in
> 
>  Toggle display of debugging messages.
> 
> -During compilation external MUSDK library, which provides direct access -
> to Marvell's EIP197 cryptographic engine, is necessary. Library sources are -
> available `here  marvell/tree/musdk-armada-17.08>`__.
> +For a list of prerequisites please refer to `Prerequisites` section in
> +:ref:`MRVL Poll Mode Driver ` guide.

Maybe say that it is the network PMD?

Thanks,
Pablo

Re: [dpdk-dev] [PATCHv2 3/4] Makefiles: Add experimental tag check and warnings to trigger on use

[Bruce Richardson]

On Mon, Dec 11, 2017 at 07:40:43AM -0500, Neil Horman wrote:
> On Mon, Dec 11, 2017 at 11:35:57AM +, Bruce Richardson wrote:
> > On Fri, Dec 08, 2017 at 12:14:34PM -0500, Neil Horman wrote:
> > > Add checks during build to ensure that all symbols in the EXPERIMENTAL
> > > version map section have __experimental tags on their definitions, and
> > > enable the warnings needed to announce their use.  Also add a
> > > ALLOW_EXPERIMENTAL_FUNCTIONS variable check to allow for in-tree dpdk
> > > libraries to override those checks.
> > > 
> > > Signed-off-by: Neil Horman 
> > > CC: Thomas Monjalon 
> > > CC: "Mcnamara, John" 
> > > ---
> > 
> > Hi Neil,
> > 
> > if I read the patch functionality correctly, the
> > ALLOW_EXPERIMENTAL_FUNCTIONS variable simply suppresses all errors for
> > deprecated functions. However, what we really want is just to suppress
> > the errors for the experimental functions, and not any that really are
> > deprecated. While we may not have any now in DPDK, that doesn't mean it
> > might not be useful to have some in future.
> > 
> Well, they are all deprecated by the stict definition of the term, which is to
> say that their usage is discouraged/disapproved.  What I think you are saying 
> is
> that you would like to differentiate reasons for deprecation (i.e. deprecation
> because an api is experimental, vs deprecation because an api is broken or 
> going
> to be removed).  I'm not sure I see that we will get there anytime soon, but
> yes, that seems like it would be a good ability to add in here.
> 
> > Therefore, would an alternative scheme work where the experimental tag
> > is set to empty if the ALLOW_EXPERIMENTAL_FUNCTIONS define is set when
> > compiling?
> > 
> Yes, I think that could work.  Perhaps what we can do is modify
> ALLOW_EXPERIMENTAL_FUNCTIONS to set a CFLAGS like:
> -DSUPPRESS_EXPERIMENTAL
> 
> And use that to toggle the definition on rte_compat.h between something that
> deprecates the API call and an empty definition
> 

Yes, exactly what I was thinking, but failed to express so clearly.

Also, would the flag be best set globally when building DPDK itself,
rather than changing individual makefiles per library?

Re: [dpdk-dev] [PATCH 15/39] examples/ipsec-secgw: convert to new ethdev offloads API

[Radu Nicolau]

On 12/11/2017 12:33 PM, Shahaf Shuler wrote:

Hi Radu,

Monday, December 11, 2017 1:48 PM, Radu Nicolau :

Hi,

Comment inline


On 11/23/2017 12:19 PM, Shahaf Shuler wrote:

Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler 
---
   examples/ipsec-secgw/ipsec-secgw.c | 27

+--

   1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c
b/examples/ipsec-secgw/ipsec-secgw.c
index c98454a90..6e538a1ab 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -217,6 +217,8 @@ static struct rte_eth_conf port_conf = {
},
.txmode = {
.mq_mode = ETH_MQ_TX_NONE,
+   .offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
+DEV_TX_OFFLOAD_MULTI_SEGS),
},
   };

@@ -1394,6 +1396,22 @@ port_init(uint16_t portid)
if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_SECURITY)
port_conf.txmode.offloads |= DEV_TX_OFFLOAD_SECURITY;

+   if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
+   port_conf.rxmode.offloads) {
+   printf("Some Rx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  portid, port_conf.rxmode.offloads,
+  dev_info.rx_offload_capa);
+   port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
+   }
+   if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
+port_conf.txmode.offloads) {
+   printf("Some Tx offloads are not supported "
+  "by port %d: requested 0x%lx supported 0x%lx\n",
+  portid, port_conf.txmode.offloads,
+  dev_info.tx_offload_capa);
+   port_conf.txmode.offloads &= dev_info.tx_offload_capa;
+   }

I don't think that clearing the offload flags that are not advertised in the
capabilities is a good approach, although it may be the right one.
  From what I can see there are more PMDs that don't fully populate the
offload capabilities, but actually check for them in the configure/start
function. One of them is ixgbe, which needs CRC strip enabled when IPSec is
enabled, and will fail to start otherwise. So although it supports CRC strip it
does not set the flag in the capabilities, but checks it in the start function.

Why ixgbe don't expose the CRC cap then? It seems wrong behavior to expect the 
application to set it without any cap reported.
It is bad behavior but from what I can see most, if not all, PMDs don't 
expose CRC strip (or jumbo frames) while still supporting it.



I would propose to just print a warning if a requested offload is not set in the
capabilities, but let the pmd start fail if it is not really supported.


I think I agree, however not from the reason you mentioned.
It is bad to mask the un-supported offloads because the application relies on 
them to be set successfully. The application will not run successfully if the 
IPV4 checksum is not actually set (for example).

On v2 I will print just the warn.


ret = rte_eth_dev_configure(portid, nb_rx_queue, nb_tx_queue,
&port_conf);
if (ret < 0)
@@ -1420,7 +1438,8 @@ port_init(uint16_t portid)
printf("Setup txq=%u,%d,%d\n", lcore_id, tx_queueid,

socket_id);

txconf = &dev_info.default_txconf;
-   txconf->txq_flags = 0;
+   txconf->txq_flags = ETH_TXQ_FLAGS_IGNORE;
+   txconf->offloads = port_conf.txmode.offloads;

ret = rte_eth_tx_queue_setup(portid, tx_queueid, nb_txd,
socket_id, txconf);
@@ -1434,6 +1453,8 @@ port_init(uint16_t portid)

/* init RX queues */
for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
+   struct rte_eth_rxconf rxq_conf;
+
if (portid != qconf->rx_queue_list[queue].port_id)
continue;

@@ -1442,8 +1463,10 @@ port_init(uint16_t portid)
printf("Setup rxq=%d,%d,%d\n", portid, rx_queueid,
socket_id);

+   rxq_conf = dev_info.default_rxconf;
+   rxq_conf.offloads = port_conf.rxmode.offloads;
ret = rte_eth_rx_queue_setup(portid, rx_queueid,
-   nb_rxd, socket_id, NULL,
+   nb_rxd, socket_id,

&rxq_conf,

socket_ctx[socket_id].mbuf_pool);
if (ret < 0)
rte_exit(EXIT_FAILURE,

Re: [dpdk-dev] [PATCH 1/3] hash: run-time function selection

[Bruce Richardson]

On Mon, Nov 06, 2017 at 10:04:02AM -0800, Elza Mathew wrote:
> Compile-time function selection can potentially lead to
> lower performance on generic builds done by distros.
> Replaced compile time flag checks with run-time function
> selection.
> 
> Signed-off-by: Elza Mathew 
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 10 +-
>  lib/librte_hash/rte_cuckoo_hash.h |  6 --
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 

Looks good to me.

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework

[Shreyansh Jain]

On Monday 11 December 2017 06:13 PM, Gaëtan Rivet wrote:

On Mon, Dec 11, 2017 at 05:30:16PM +0530, Shreyansh Jain wrote:

On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:


[...]


diff --git a/lib/librte_eal/common/include/rte_bus.h 
b/lib/librte_eal/common/include/rte_bus.h
index 331d954..bd3c28e 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -183,6 +183,51 @@ struct rte_bus_conf {
enum rte_bus_probe_mode probe_mode; /**< Probe policy. */
   };
+/**
+ * Bus configuration items.
+ */
+enum rte_bus_ctrl_item {
+   RTE_BUS_CTRL_PROBE_MODE = 0,
+   RTE_BUS_CTRL_ITEM_MAX,
+};


I am assuming that a driver implementation can take more than ITEM_MAX
control knobs. It is opaque to the library. Are we on same page?

For example, a bus driver can implement:

rte_bus_XXX_ctrl_item {

RTE_BUS_XYZ_KNOB_1 = 100,
RTE_BUS_XYZ_KNOB_2,
RTE_BUS_XYZ_KNOB_3,
};

without the library knowing or restricting the API to RTE_BUS_CTRL_ITEM_MAX.

I see that in your code for PCI (Patch 5/8: pci_ctrl) you have restricted
the control knob to RTE_BUS_CTRL_ITEM_MAX.
I hope that such restrictions would not float to library layer.

If we are on same page, should this be documented as a code comment
somewhere?
if not, do you think what I am stating makes sense?



I see what you mean, but I'm not sure it would be a good thing.
Actually, I think proposing this ITEM_MAX was a mistake.

Regarding the specific bus knobs:

- If a single bus needs this knob, then it would be better for the dev
   to add it as part of the bus' public API, following the correct
   library versioning processes. This would not break this bus control
   structure ABI.


Sorry, but can you elaborate on "...add it as part of bus' public API"?

This is what I had in mind:

ctrl_fn = rte_bus_control_get(busname, RTE_BUS_XYZ_KNOB_1);

(unlike specific functions like probe_mode_get/set and iova_mode_get/set)

Where ctrl_fn would then point to a method specific to bus for KNOB_1 
configuration parameter.

Thereafter, ctrl_fn(KNOB_1, void *arg).

What other public API method are you hinting at?




- If more than one bus implement this knob, then it should be proposed
   as part of the library API. Buses adding this new knob would break
   their ABI, other buses would be left untouched.


Agree, if more than one bus implements same operation.



This makes me realize that proposing this ITEM_MAX value is not good to
the intended purpose of this patchset:

- If a bus implementation use a reference to ITEM_MAX, then the control
   structure ABI would be broken by any new control knob added, even if the
   bus does not implement it. Granted, it would not break the driver
   structure itself, but still. My PCI implementation is thus incorrect.


Changes to enum wouldn't break ABI as far as I understand. Adding a new 
entry only expands it to a new declaration without impacting its size or 
signature.




Therefore I think that it would be best to remove this ITEM_MAX altogether,
forcing bus developpers to use other ways that would not break their
ABIs every other release.



Removing ITEM_MAX is OK from my side. It doesn't serve much purpose. 
But, not for the "ABI break" reason.

Re: [dpdk-dev] [PATCH 2/3] hash: run-time function selection

[Bruce Richardson]

On Mon, Nov 06, 2017 at 10:04:49AM -0800, Elza Mathew wrote:
> Compile-time function selection can potentially lead to
> lower performance on generic builds done by distros.
> Replaced compile time flag checks with run-time function
> selection.
> 
> Signed-off-by: Elza Mathew 
> ---
>  lib/librte_hash/rte_fbk_hash.c | 11 ++-
>  lib/librte_hash/rte_fbk_hash.h |  8 
>  2 files changed, 10 insertions(+), 9 deletions(-)
>
Title needs an update to indicate this change is for fbk-hash. I suspect
that can be fixed on apply.

Acked-by: Bruce Richardson 

Re: [dpdk-dev] [PATCH] vhost: fix error code check when creating pthread

[Maxime Coquelin]

On 12/08/2017 11:19 AM, Olivier Matz wrote:

On error, pthread_create() returns a positive number (errno).
Fix the test on the return value.

Fixes: af1475918124 ("vhost: introduce API to start a specific driver")
Fixes: e623e0c6d8a5 ("vhost: add reconnect ability")
Cc: sta...@dpdk.org

Signed-off-by: Olivier Matz 
---
  lib/librte_vhost/socket.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)



Reviewed-by: Maxime Coquelin 

Thanks,
Maxime

Re: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code

[Daly, Lee]

> Subject: [dpdk-dev] [PATCH 1/2] lib: refactor basic stats code
> 
> Moved the code to get the basic stats names and values into static functions.
> 
> Signed-off-by: Elza Mathew 

Reviewed-by: Lee Daly 

Re: [dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs

[Daly, Lee]

> Subject: [dpdk-dev] [PATCH 2/2] lib: optimize _xstats_by_ids APIs
> 
> Introduced a check to detect if the stats IDs being requested are all basic
> stats IDs. In that case, ensured that only the basic stats would be retrieved.
> Previously, both basic stats and xstats were being retrieved even if all the 
> IDs
> were basic stats IDs.
> 
> Signed-off-by: Elza Mathew 

Reviewed-by: Lee Daly 

Re: [dpdk-dev] [PATCH 01/11] cryptodev: add compile support for AMD CCP crypto PMD

[De Lara Guarch, Pablo]

Hi Ravi,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kumar
> Sent: Thursday, November 30, 2017 1:12 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 01/11] cryptodev: add compile support for
> AMD CCP crypto PMD
> 

Thanks for splitting the original patch into multiple ones.
However, in this case, this should get merged to the other patches,
as all the changes are referencing files/folders that do not exist yet.

Apart from this, two comments below.

Thanks,
Pablo

> Signed-off-by: Ravi Kumar 
> ---
>  MAINTAINERS | 6 ++
>  config/common_base  | 5 +
>  drivers/crypto/Makefile | 1 +
>  mk/rte.app.mk   | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f0baeb4..daac82e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -588,6 +588,12 @@ M: Pablo de Lara
> 
>  T: git://dpdk.org/next/dpdk-next-crypto
>  F: doc/guides/cryptodevs/features/default.ini
> 
> +AMD CCP Crypto PMD

Remove trailing whitespace here.

> +M: Ravi Kumar 
> +F: drivers/crypto/ccp/
> +F: doc/guides/cryptodevs/ccp.rst
> +F: doc/guides/cryptodevs/features/ccp.ini

Add these lines, as you add the files.

> +

Re: [dpdk-dev] [PATCH 01/11] cryptodev: add compile support for AMD CCP crypto PMD

[Kumar, Ravi1]

>Hi Ravi,
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kumar
>> Sent: Thursday, November 30, 2017 1:12 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH 01/11] cryptodev: add compile support for 
>> AMD CCP crypto PMD
>> 
>
>Thanks for splitting the original patch into multiple ones.
>However, in this case, this should get merged to the other patches, as all the 
>changes are referencing files/folders that do not exist yet.
>
>Apart from this, two comments below.
>
>Thanks,
>Pablo
>
>> Signed-off-by: Ravi Kumar 
>> ---
>>  MAINTAINERS | 6 ++
>>  config/common_base  | 5 +
>>  drivers/crypto/Makefile | 1 +
>>  mk/rte.app.mk   | 2 ++
>>  4 files changed, 14 insertions(+)
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS index f0baeb4..daac82e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -588,6 +588,12 @@ M: Pablo de Lara
>> 
>>  T: git://dpdk.org/next/dpdk-next-crypto
>>  F: doc/guides/cryptodevs/features/default.ini
>> 
>> +AMD CCP Crypto PMD
>
>Remove trailing whitespace here.
>
>> +M: Ravi Kumar 
>> +F: drivers/crypto/ccp/
>> +F: doc/guides/cryptodevs/ccp.rst
>> +F: doc/guides/cryptodevs/features/ccp.ini
>
>Add these lines, as you add the files.
>
>> +
>
>
Hi Pablo,

Thanks for the review comments. Will work on it.

Regards,
Ravi

Re: [dpdk-dev] [PATCH 2/2] event/sw: simplify credit scheme

[Van Haaren, Harry]

> -Original Message-
> From: Eads, Gage
> Sent: Thursday, November 30, 2017 4:21 AM
> To: dev@dpdk.org
> Cc: jerin.ja...@caviumnetworks.com; Van Haaren, Harry
> ; Richardson, Bruce
> ; hemant.agra...@nxp.com; nipun.gu...@nxp.com;
> santosh.shu...@caviumnetworks.com
> Subject: [PATCH 2/2] event/sw: simplify credit scheme
> 
> This commit modifies the sw PMD credit scheme such that credits are
> consumed when enqueueing a NEW event and released when an event is
> released -- typically, the beginning and end of a pipeline. Workers that
> simply forward events do not interact with the credit pool.
> 
> Signed-off-by: Gage Eads 

Acked-by: Harry van Haaren 

Re: [dpdk-dev] [PATCH 2/2] crypto/mrvl: update MRVL CRYPTO PMD documentation

[Tomasz Duszynski]

On Mon, Dec 11, 2017 at 12:44:43PM +, De Lara Guarch, Pablo wrote:
>
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Duszynski
> > Sent: Thursday, November 30, 2017 1:33 PM
> > To: dev@dpdk.org
> > Cc: m...@semihalf.com; j...@semihalf.com; d...@marvell.com;
> > nsams...@marvell.com; jianbo@arm.com; Tomasz Duszynski
> > 
> > Subject: [dpdk-dev] [PATCH 2/2] crypto/mrvl: update MRVL CRYPTO PMD
> > documentation
> >
> > Update MRVL CRYPTO PMD documentation.
> >
> > Signed-off-by: Tomasz Duszynski 
>
>
> This is depending on the network driver update, right?
> You should say it in the cover letter. I will wait until it is applied then.

Right, I've missed that. Anyway, from what I see network driver
patches have been already applied.

>
> ...
>
> > -
> >  Installation
> >  
> >
> > @@ -92,20 +85,11 @@ Currently there are two driver specific compilation
> > options in
> >
> >  Toggle display of debugging messages.
> >
> > -During compilation external MUSDK library, which provides direct access -
> > to Marvell's EIP197 cryptographic engine, is necessary. Library sources are 
> > -
> > available `here  > marvell/tree/musdk-armada-17.08>`__.
> > +For a list of prerequisites please refer to `Prerequisites` section in
> > +:ref:`MRVL Poll Mode Driver ` guide.
>
> Maybe say that it is the network PMD?

OK. I'll send v2 then.

>
> Thanks,
> Pablo
>

--
- Tomasz Duszyński

Re: [dpdk-dev] [PATCH v2 01/18] eal: prepend busname on legacy device declaration

[Shreyansh Jain]

One very quick comment:

On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:

Legacy device options (-b, -w, --vdev) need to prepend their bus name to
user parameters for backward compatibility.

Signed-off-by: Gaetan Rivet 
---
  lib/librte_eal/common/eal_common_options.c | 17 -
  1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c 
b/lib/librte_eal/common/eal_common_options.c
index 630c9d2..d57cb5d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -143,13 +143,16 @@ static int mem_parsed;
  static int core_parsed;
  
  static int

-eal_option_device_add(enum rte_devtype type, const char *optarg)
+eal_option_device_add(enum rte_devtype type,
+ const char *busname, const char *optarg)
  {
struct device_option *devopt;
size_t optlen;
int ret;
  
  	optlen = strlen(optarg) + 1;

+   if (busname != NULL)
+   optlen += strlen(optarg) + 1;


I think you want "optlen += strlen(busname) + 1";


devopt = calloc(1, sizeof(*devopt) + optlen);
if (devopt == NULL) {
RTE_LOG(ERR, EAL, "Unable to allocate device option\n");


[...]

Re: [dpdk-dev] [PATCH v2 1/6] ethdev: added switch_domain and representor port flag

[Alex Rosenbaum]

Mohammad,

I did not see a v2 change log. did I miss it? please send.

I don't understand who this v2 addresses the comments by Alejandro Lucero
 from netronome [1].
These are critical points which your proposal does not handle. It is
related to the switch_domain member exposed here.

thanks,
Alex

[1] http://dpdk.org/ml/archives/dev/2017-September/074904.html

Re: [dpdk-dev] [PATCH v2 2/3] crypto: fix pedantic compilation errors

[De Lara Guarch, Pablo]

> -Original Message-
> From: Nelio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> Sent: Monday, December 11, 2017 12:43 PM
> To: De Lara Guarch, Pablo 
> Cc: Akhil Goyal ; Doherty, Declan
> ; dev@dpdk.org; Gaetan Rivet
> ; sta...@dpdk.org
> Subject: Re: [PATCH v2 2/3] crypto: fix pedantic compilation errors
> 
> Hi Pablo,
> 
> On Mon, Dec 11, 2017 at 11:49:39AM +, De Lara Guarch, Pablo wrote:
> >
> >
> > > -Original Message-
> > > From: Nelio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> > > Sent: Thursday, November 23, 2017 10:03 AM
> > > To: Akhil Goyal ; Doherty, Declan
> > > 
> > > Cc: dev@dpdk.org; Gaetan Rivet ; De Lara
> > > Guarch, Pablo ; sta...@dpdk.org
> > > Subject: [PATCH v2 2/3] crypto: fix pedantic compilation errors
> > >
> >
> > ...
> >
> > > --- a/lib/librte_cryptodev/rte_crypto.h
> > > +++ b/lib/librte_cryptodev/rte_crypto.h
> > > @@ -121,7 +121,7 @@ struct rte_crypto_op {
> > >   rte_iova_t phys_addr;
> > >   /**< physical address of crypto operation */
> > >
> > > - RTE_STD_C11
> > > + __extension__
> >
> > Hi Nelio,
> >
> > Since RTE_STD_C11 is basically __extension__ when __STDC_VERSION__
> is
> > not defined, Is this forcing __extension__ to be used no matter what?
> (even if C11 is not supported).
> 
> Yes
> 

Right, and are we sure that this is OK? If C11 is supported, do we still want 
extension?

Thanks,
Pablo

> Thanks,
> 
> --
> Nélio Laranjeiro
> 6WIND

Re: [dpdk-dev] [PATCH 2/6] doc: add mbuf reorg to PMD todo list

[Marko Kovacevic]

On 21/11/2017 01:42, Ferruh Yigit wrote:

To track modification:
8f094a9ac5d7 ("mbuf: set mbuf fields while in pool")

Proposed deadline for PMDs is v18.02

Signed-off-by: Ferruh Yigit 


Acked-by:Marko Kovacevic

Re: [dpdk-dev] [PATCH 07/39] examples/l3fwd-power: convert to new ethdev offloads API

[Hunt, David]

Hi Shahaf,


On 23/11/2017 12:14 PM, Shahaf Shuler wrote:

Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler 
---
  examples/l3fwd-power/main.c | 42 ++--
  1 file changed, 32 insertions(+), 10 deletions(-)



--snip--

Looks good to me.

Acked-by: David Hunt 

[dpdk-dev] [PATCH v3 1/2] examples/ipsec-secgw: fix missing ingress flow attribute

[Nelio Laranjeiro]

Generic flow API have both direction bits, ingress and egress for rules
which may work on both sides.

Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
Cc: akhil.go...@nxp.com

Signed-off-by: Nelio Laranjeiro 
Acked-by: Radu Nicolau 
---
 examples/ipsec-secgw/ipsec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index ec8bf95e1..17bd7620d 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -205,6 +205,8 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa 
*sa)
 
sa->attr.egress = (sa->direction ==
RTE_SECURITY_IPSEC_SA_DIR_EGRESS);
+   sa->attr.ingress = (sa->direction ==
+   RTE_SECURITY_IPSEC_SA_DIR_INGRESS);
sa->flow = rte_flow_create(sa->portid,
&sa->attr, sa->pattern, sa->action, &err);
if (sa->flow == NULL) {
-- 
2.11.0

[dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add target queues in flow actions

[Nelio Laranjeiro]

Mellanox INNOVA NIC needs to have final target queue actions to perform
inline crypto.

Signed-off-by: Nelio Laranjeiro 

---

Changes in v3:

 * removed PASSTHRU test for ingress.
 * removed check on configured queues for the queue action.

Changes in v2:

 * Test the rule by PASSTHRU/RSS/QUEUE and apply the first one validated.
---
 examples/ipsec-secgw/ipsec.c | 57 ++--
 examples/ipsec-secgw/ipsec.h |  2 +-
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 17bd7620d..1b8b251c8 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -142,6 +142,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa 
*sa)
rte_eth_dev_get_sec_ctx(
sa->portid);
const struct rte_security_capability *sec_cap;
+   int ret = 0;
 
sa->sec_session = rte_security_session_create(ctx,
&sess_conf, ipsec_ctx->session_pool);
@@ -201,15 +202,67 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct 
ipsec_sa *sa)
sa->action[0].type = RTE_FLOW_ACTION_TYPE_SECURITY;
sa->action[0].conf = sa->sec_session;
 
-   sa->action[1].type = RTE_FLOW_ACTION_TYPE_END;
-
sa->attr.egress = (sa->direction ==
RTE_SECURITY_IPSEC_SA_DIR_EGRESS);
sa->attr.ingress = (sa->direction ==
RTE_SECURITY_IPSEC_SA_DIR_INGRESS);
+   if (sa->attr.ingress) {
+   uint8_t rss_key[40];
+   struct rte_eth_rss_conf rss_conf = {
+   .rss_key = rss_key,
+   .rss_key_len = 40,
+   };
+   struct rte_eth_dev *eth_dev;
+   union {
+   struct rte_flow_action_rss rss;
+   struct {
+   const struct rte_eth_rss_conf *rss_conf;
+   uint16_t num;
+   uint16_t queue[RTE_MAX_QUEUES_PER_PORT];
+   } local;
+   } action_rss;
+   unsigned int i;
+   unsigned int j;
+
+   sa->action[2].type = RTE_FLOW_ACTION_TYPE_END;
+   /* Try RSS. */
+   sa->action[1].type = RTE_FLOW_ACTION_TYPE_RSS;
+   sa->action[1].conf = &action_rss;
+   eth_dev = ctx->device;
+   rte_eth_dev_rss_hash_conf_get(sa->portid,
+ &rss_conf);
+   for (i = 0, j = 0;
+i < eth_dev->data->nb_rx_queues; ++i)
+   if (eth_dev->data->rx_queues[i])
+   action_rss.local.queue[j++] = i;
+   action_rss.local.num = j;
+   action_rss.local.rss_conf = &rss_conf;
+   ret = rte_flow_validate(sa->portid, &sa->attr,
+   sa->pattern, sa->action,
+   &err);
+   if (!ret)
+   goto flow_create;
+   /* Try Queue. */
+   sa->action[1].type = RTE_FLOW_ACTION_TYPE_QUEUE;
+   sa->action[1].conf =
+   &(struct rte_flow_action_queue){
+   .index = 0,
+   };
+   ret = rte_flow_validate(sa->portid, &sa->attr,
+   sa->pattern, sa->action,
+   &err);
+   if (ret)
+   goto flow_create_failure;
+   } else {
+   sa->action[1].type =
+   RTE_FLOW_ACTION_TYPE_PASSTHRU;
+   sa->action[2].type = RTE_FLOW_ACTION_TYPE_END;
+   }
+flow_create:
sa->flow = rte_flow_create(sa->portid,
&sa->attr, sa->pattern, sa->action, &er

[dpdk-dev] [PATCH v2 0/2] Sync compilation with MUSDK-17.10

[Tomasz Duszynski]

This patchset comes with the following changes:

o MUSDK-17.10 is the latest version of the library. Since it brings
  improvements and fixes switch is necessary.

o Some minor updates to the documentation.

Note that this series should be applied after net pmd patch series.

Changes since v1:
- Rename net pmd doc reference.

Tomasz Duszynski (2):
  crypto/mrvl: sync compilation with musdk-17.10
  crypto/mrvl: update MRVL CRYPTO PMD documentation


 doc/guides/cryptodevs/mrvl.rst| 28 
 doc/guides/nics/mrvl.rst  |  2 ++
 drivers/crypto/mrvl/Makefile  |  3 ++-
 drivers/crypto/mrvl/rte_mrvl_compat.h |  1 +
 4 files changed, 9 insertions(+), 25 deletions(-)

--
2.7.4

[dpdk-dev] [PATCH v2 1/2] crypto/mrvl: sync compilation with musdk-17.10

[Tomasz Duszynski]

With a new version of the musdk library it's no longer necessary to
explicitly define MVCONF_ARCH_DMA_ADDR_T_64BIT.

Proper defines are autogenerated by ./configure script based on
passed options and available after mv_autogen_comp_flags.h inclusion.

Signed-off-by: Tomasz Duszynski 
Acked-by: Jianbo Liu 
---
 drivers/crypto/mrvl/Makefile  | 3 ++-
 drivers/crypto/mrvl/rte_mrvl_compat.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/mrvl/Makefile b/drivers/crypto/mrvl/Makefile
index 3532f7c..5515b40 100644
--- a/drivers/crypto/mrvl/Makefile
+++ b/drivers/crypto/mrvl/Makefile
@@ -47,7 +47,8 @@ LIB = librte_pmd_mrvl_crypto.a
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 CFLAGS += -I$(LIBMUSDK_PATH)/include
-CFLAGS += -DMVCONF_ARCH_DMA_ADDR_T_64BIT
+CFLAGS += -DMVCONF_TYPES_PUBLIC
+CFLAGS += -DMVCONF_DMA_PHYS_ADDR_T_PUBLIC

 # library version
 LIBABIVER := 1
diff --git a/drivers/crypto/mrvl/rte_mrvl_compat.h 
b/drivers/crypto/mrvl/rte_mrvl_compat.h
index c29fa10..22cd184 100644
--- a/drivers/crypto/mrvl/rte_mrvl_compat.h
+++ b/drivers/crypto/mrvl/rte_mrvl_compat.h
@@ -43,6 +43,7 @@
 #ifdef container_of
 #undef container_of
 #endif
+#include "env/mv_autogen_comp_flags.h"
 #include "drivers/mv_sam.h"
 #include "drivers/mv_sam_cio.h"
 #include "drivers/mv_sam_session.h"
--
2.7.4

[dpdk-dev] [PATCH v2 2/2] crypto/mrvl: update MRVL CRYPTO PMD documentation

[Tomasz Duszynski]

Update MRVL CRYPTO PMD documentation.

Signed-off-by: Tomasz Duszynski 
Acked-by: Jianbo Liu 
---
 doc/guides/cryptodevs/mrvl.rst | 28 
 doc/guides/nics/mrvl.rst   |  2 ++
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/doc/guides/cryptodevs/mrvl.rst b/doc/guides/cryptodevs/mrvl.rst
index 4e992fb..22a48e4 100644
--- a/doc/guides/cryptodevs/mrvl.rst
+++ b/doc/guides/cryptodevs/mrvl.rst
@@ -70,13 +70,6 @@ Limitations
 * Hardware only supports scenarios where ICV (digest buffer) is placed just
   after the authenticated data. Other placement will result in error.

-* Before running crypto test suite it is advised to increase limit of
-  opened files:
-
-  .. code-block:: console
-
- ulimit -n 2
-
 Installation
 

@@ -92,20 +85,11 @@ Currently there are two driver specific compilation options 
in

 Toggle display of debugging messages.

-During compilation external MUSDK library, which provides direct access
-to Marvell's EIP197 cryptographic engine, is necessary. Library sources are
-available `here 
`__.
+For a list of prerequisites please refer to `Prerequisites` section in
+:ref:`MRVL NET Poll Mode Driver ` guide.

-Alternatively, prebuilt library can be downloaded from
-`Marvell Extranet `_. Once approval has been
-granted, library can be found by typing ``musdk`` in the search box.
-
-For MUSDK library build instructions please refer to 
``doc/musdk_get_started.txt``
-in library sources directory.
-
-MUSDK requires out of tree kernel modules to work. Kernel tree needed to build
-them is available
-`here 
`__.
+For `crypto_safexcel.ko` module build instructions please refer
+to `doc/musdk_get_started.txt`.

 Initialization
 --
@@ -121,10 +105,6 @@ loaded:
insmod mv_sam_uio.ko
insmod crypto_safexcel.ko

-- `musdk_uio.ko`, `mv_pp2_uio.ko` and `mv_sam_uio.ko` are distributed together 
with MUSDK library.
-- `crypto_safexcel.ko` is an in-kernel module.
-- `mvpp2x_sysfs.ko` can be build from sources available `here 
`__.
-
 The following parameters (all optional) are exported by the driver:

 * max_nb_queue_pairs: maximum number of queue pairs in the device (8 by 
default).
diff --git a/doc/guides/nics/mrvl.rst b/doc/guides/nics/mrvl.rst
index 67b254c..892097d 100644
--- a/doc/guides/nics/mrvl.rst
+++ b/doc/guides/nics/mrvl.rst
@@ -29,6 +29,8 @@
 (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

+.. _mrvl_poll_mode_driver:
+
 MRVL Poll Mode Driver
 ==

--
2.7.4

Re: [dpdk-dev] [PATCH 2/6] doc: add mbuf reorg to PMD todo list

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 1:43 AM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh 
> Subject: [PATCH 2/6] doc: add mbuf reorg to PMD todo list
> 
> To track modification:
> 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool")
> 
> Proposed deadline for PMDs is v18.02
> 
> Signed-off-by: Ferruh Yigit 

Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH 3/6] doc: add dynamic logging to PMD todo list

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 1:43 AM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh 
> Subject: [PATCH 3/6] doc: add dynamic logging to PMD todo list
> 
> To track modification:
> c1b5fa94a46f ("eal: support dynamic log types")
> 
> Proposed deadline for PMDs is v18.08
> 
> Signed-off-by: Ferruh Yigit 

Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH 5/6] doc: add offload flag to PMD todo list

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 1:43 AM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh 
> Subject: [PATCH 5/6] doc: add offload flag to PMD todo list
> 
> To track modification:
> ce17eddefc20 ("ethdev: introduce Rx queue offloads API") cba7f53b717d
> ("ethdev: introduce Tx queue offloads API")
> 
> Proposed deadline for PMDs is v18.05
> 
> Signed-off-by: Ferruh Yigit 

Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH 4/6] doc: add descriptor status API to PMD todo list

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 1:43 AM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh 
> Subject: [PATCH 4/6] doc: add descriptor status API to PMD todo list
> 
> To track modification:
> b1b700ce7d6f ("ethdev: add descriptor status API")
> 
> Proposed deadline for PMDs is v19.02
> 
> Signed-off-by: Ferruh Yigit 

Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH v2 2/3] crypto: fix pedantic compilation errors

[Nelio Laranjeiro]

On Mon, Dec 11, 2017 at 01:54:22PM +, De Lara Guarch, Pablo wrote:
> 
> 
> > -Original Message-
> > From: Nelio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> > Sent: Monday, December 11, 2017 12:43 PM
> > To: De Lara Guarch, Pablo 
> > Cc: Akhil Goyal ; Doherty, Declan
> > ; dev@dpdk.org; Gaetan Rivet
> > ; sta...@dpdk.org
> > Subject: Re: [PATCH v2 2/3] crypto: fix pedantic compilation errors
> > 
> > Hi Pablo,
> > 
> > On Mon, Dec 11, 2017 at 11:49:39AM +, De Lara Guarch, Pablo wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Nelio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> > > > Sent: Thursday, November 23, 2017 10:03 AM
> > > > To: Akhil Goyal ; Doherty, Declan
> > > > 
> > > > Cc: dev@dpdk.org; Gaetan Rivet ; De Lara
> > > > Guarch, Pablo ; sta...@dpdk.org
> > > > Subject: [PATCH v2 2/3] crypto: fix pedantic compilation errors
> > > >
> > >
> > > ...
> > >
> > > > --- a/lib/librte_cryptodev/rte_crypto.h
> > > > +++ b/lib/librte_cryptodev/rte_crypto.h
> > > > @@ -121,7 +121,7 @@ struct rte_crypto_op {
> > > > rte_iova_t phys_addr;
> > > > /**< physical address of crypto operation */
> > > >
> > > > -   RTE_STD_C11
> > > > +   __extension__
> > >
> > > Hi Nelio,
> > >
> > > Since RTE_STD_C11 is basically __extension__ when __STDC_VERSION__
> > is
> > > not defined, Is this forcing __extension__ to be used no matter what?
> > (even if C11 is not supported).
> > 
> > Yes
> > 
> 
> Right, and are we sure that this is OK? If C11 is supported, do we
> still want extension?

Having an array with an empty size inside a union does not make part of
any standard, it is an extension of the language.

Regards,

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH 6/6] doc: add mbuf VLAN flag to PMD todo list

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 1:43 AM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh 
> Subject: [PATCH 6/6] doc: add mbuf VLAN flag to PMD todo list
> 
> To track modification:
> 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
> 
> Proposed deadline for PMDs is v18.02
> 
> Signed-off-by: Ferruh Yigit 


It might be worth putting | before all the text in the first column,
even if it is just a single item, since it will justify the text in
the same way.

Also it might be worth adding it to the text in the last column so
that the column isn't too wide.

Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH] doc: update contribution guideline for dependent work

[Mcnamara, John]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Tuesday, November 21, 2017 7:59 PM
> To: Thomas Monjalon ; Mcnamara, John
> 
> Cc: dev@dpdk.org; Yigit, Ferruh ;
> techbo...@dpdk.org
> Subject: [PATCH] doc: update contribution guideline for dependent work
> 
> Changing some part of the libraries but not updating all dependent code
> cause maintenance problems.
> 
> ...
> 
> Signed-off-by: Ferruh Yigit 
> 


integration testing.
> 
> +* If changes effect other parts of the project, update all those parts as
> well unless updating requires special knowledge.
> +  For the cases not all effected code updated, ensure that changes don't
> break existing code.
> +
>  * Add tests to the the ``app/test`` unit test framework where possible.
> 
>  * Add documentation, if relevant, in the form of Doxygen comments or a
> User Guide in RST format.


Second line would be better as:


For the cases where not all the effected code is updated, the submitter should 
ensure that changes don't break existing code.


Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH 10/11] test/test: add test for AMD CCP crypto PMD

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kumar
> Sent: Thursday, November 30, 2017 1:13 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 10/11] test/test: add test for AMD CCP crypto
> PMD
> 
> Signed-off-by: Ravi Kumar 

Hi Ravi,

There is a compilation issue here:

test/test/test_cryptodev_blockcipher.c:127:27: error:
expected expression before '||' token
driver_id == mrvl_pmd) ||

Also, for crypto tests like this, you should use "test/crypto: "

Thanks,
Pablo

Re: [dpdk-dev] [PATCH 08/11] crypto/ccp: rename CCP crypto driver id

[De Lara Guarch, Pablo]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kumar
> Sent: Thursday, November 30, 2017 1:13 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 08/11] crypto/ccp: rename CCP crypto driver id
> 
> Signed-off-by: Ravi Kumar 

Hi Ravi,

I think this patch is not necessary, you can just use this new driver id
directly from the beginning of the patchset, right (although I don't see why 
this is necessary).

Thanks,
Pablo

Re: [dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework

[Gaëtan Rivet]

On Mon, Dec 11, 2017 at 07:06:55PM +0530, Shreyansh Jain wrote:
> On Monday 11 December 2017 06:13 PM, Gaëtan Rivet wrote:
> > On Mon, Dec 11, 2017 at 05:30:16PM +0530, Shreyansh Jain wrote:
> > > On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:
> 
> [...]
> 
> > > > diff --git a/lib/librte_eal/common/include/rte_bus.h 
> > > > b/lib/librte_eal/common/include/rte_bus.h
> > > > index 331d954..bd3c28e 100644
> > > > --- a/lib/librte_eal/common/include/rte_bus.h
> > > > +++ b/lib/librte_eal/common/include/rte_bus.h
> > > > @@ -183,6 +183,51 @@ struct rte_bus_conf {
> > > > enum rte_bus_probe_mode probe_mode; /**< Probe policy. */
> > > >};
> > > > +/**
> > > > + * Bus configuration items.
> > > > + */
> > > > +enum rte_bus_ctrl_item {
> > > > +   RTE_BUS_CTRL_PROBE_MODE = 0,
> > > > +   RTE_BUS_CTRL_ITEM_MAX,
> > > > +};
> > > 
> > > I am assuming that a driver implementation can take more than ITEM_MAX
> > > control knobs. It is opaque to the library. Are we on same page?
> > > 
> > > For example, a bus driver can implement:
> > > 
> > > rte_bus_XXX_ctrl_item {
> > >   
> > >   RTE_BUS_XYZ_KNOB_1 = 100,
> > >   RTE_BUS_XYZ_KNOB_2,
> > >   RTE_BUS_XYZ_KNOB_3,
> > > };
> > > 
> > > without the library knowing or restricting the API to 
> > > RTE_BUS_CTRL_ITEM_MAX.
> > > 
> > > I see that in your code for PCI (Patch 5/8: pci_ctrl) you have restricted
> > > the control knob to RTE_BUS_CTRL_ITEM_MAX.
> > > I hope that such restrictions would not float to library layer.
> > > 
> > > If we are on same page, should this be documented as a code comment
> > > somewhere?
> > > if not, do you think what I am stating makes sense?
> > > 
> > 
> > I see what you mean, but I'm not sure it would be a good thing.
> > Actually, I think proposing this ITEM_MAX was a mistake.
> > 
> > Regarding the specific bus knobs:
> > 
> > - If a single bus needs this knob, then it would be better for the dev
> >to add it as part of the bus' public API, following the correct
> >library versioning processes. This would not break this bus control
> >structure ABI.
> 
> Sorry, but can you elaborate on "...add it as part of bus' public API"?
> 
> This is what I had in mind:
> 
> ctrl_fn = rte_bus_control_get(busname, RTE_BUS_XYZ_KNOB_1);
> 
> (unlike specific functions like probe_mode_get/set and iova_mode_get/set)
> 
> Where ctrl_fn would then point to a method specific to bus for KNOB_1
> configuration parameter.
> Thereafter, ctrl_fn(KNOB_1, void *arg).
> 
> What other public API method are you hinting at?
> 
> 

I was thinking that buses would simply expose a function

rte_busname_xyz_knob1(void *arg);

as part of their public API. This would not require an ABI break for
this bus, as it would only be an API extension and would not use
callbacks within the bus structure.

Thus, I think that for buses tempted to propose a specific API, this would be
the cleanest way.

The bus proposing it as part of a custom control section would only be
interesting when the operation is expected to become a standard API for
other buses but was not yet accepted. Applications would be able to use
the interface and the ITEM could be added later. But I doubt this is
encouraging best practices as far as API evolution go.

> > 
> > - If more than one bus implement this knob, then it should be proposed
> >as part of the library API. Buses adding this new knob would break
> >their ABI, other buses would be left untouched.
> 
> Agree, if more than one bus implements same operation.
> 
> > 
> > This makes me realize that proposing this ITEM_MAX value is not good to
> > the intended purpose of this patchset:
> > 
> > - If a bus implementation use a reference to ITEM_MAX, then the control
> >structure ABI would be broken by any new control knob added, even if the
> >bus does not implement it. Granted, it would not break the driver
> >structure itself, but still. My PCI implementation is thus incorrect.
> 
> Changes to enum wouldn't break ABI as far as I understand. Adding a new
> entry only expands it to a new declaration without impacting its size or
> signature.
> 
> > 
> > Therefore I think that it would be best to remove this ITEM_MAX altogether,
> > forcing bus developpers to use other ways that would not break their
> > ABIs every other release.
> > 
> 
> Removing ITEM_MAX is OK from my side. It doesn't serve much purpose. But,
> not for the "ABI break" reason.

Adding the enum would not break ABI indeed, but I was thinking about the
way the bus control structure would be declared.

However, upon second inspection on the my PCI implementation, I did not
actually use ITEM_MAX:

> static rte_bus_ctrl_t pci_ctrl_ops[][RTE_BUS_CTRL_OP_MAX] = {
> »  [RTE_BUS_CTRL_PROBE_MODE] = {
> »  »   [RTE_BUS_CTRL_GET] = pci_probe_mode_get,
> »  »   [RTE_BUS_CTRL_SET] = pci_probe_mode_set,
> »  },
> };

I just thought I had used RTE_BUS_CTRL_ITEM_MAX instead of RTE_BUS_CTRL_OP_MAX.
So my line of tho

Re: [dpdk-dev] [PATCH 06/39] examples/l3fwd-acl: convert to new ethdev offloads API

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Thursday, November 23, 2017 12:14 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 06/39] examples/l3fwd-acl: convert to new ethdev 
> offloads API
> 
> Ethdev offloads API has changed since:
> 
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> 
> This commit support the new API.
> 
> Signed-off-by: Shahaf Shuler 
> ---
>  examples/l3fwd-acl/main.c | 41 -
>  1 file changed, 32 insertions(+), 9 deletions(-)
> 

Acked-by: Konstantin Ananyev 

Re: [dpdk-dev] [PATCH 3/3] doc: update documentation for flow classify lib

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jasvinder Singh
> Sent: Thursday, November 23, 2017 11:32 AM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard 
> Subject: [dpdk-dev] [PATCH 3/3] doc: update documentation for flow
> classify lib
> 
> Updates the documentation for flow classification library and sample
> application.
> 
> Signed-off-by: Jasvinder Singh 


>
> +370,24 @@ parses the Flow rule.
> 
>  struct rte_flow_classify {
>  uint32_t id;  /* unique ID of classify object */
> -struct rte_flow_action action; /* action when match found */
> - struct classify_rules rules; /* union of rules */
> +enum rte_flow_classify_table_type tbl_type; /* rule table */
> +struct classify_rules rules; /* union of rules */
>  union {
>  struct acl_keys key;
>  } u;
>  int key_found; /* rule key found in table */
> -void *entry; /* pointer to buffer to hold rule meta data */
> -void *entry_ptr; /* handle to the table entry for rule meta data
> */
> +struct rte_flow_classify_table_entry entry;  /* rule meta data */
> +   void *entry_ptr; /* handle to the table entry for rule meta data


The last line here is indented less than the others. Also there is a stray tab
a few lines above it (in the existing text). Can you replace that as well.

Re: [dpdk-dev] [PATCH 12/39] examples/ip_fragmentation: convert to new offloads API

[Ananyev, Konstantin]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Thursday, November 23, 2017 12:19 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 12/39] examples/ip_fragmentation: convert to new 
> offloads API
> 
> Ethdev offloads API has changed since:
> 
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> 
> This commit support the new API.
> 
> Signed-off-by: Shahaf Shuler 
> ---

Acked-by: Konstantin Ananyev 

Re: [dpdk-dev] [PATCH 2/2] doc: add description of raw mode in flow director in testpmd

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Kirill Rybalchenko
> Sent: Thursday, November 23, 2017 4:15 PM
> To: dev@dpdk.org
> Cc: Rybalchenko, Kirill ; Chilikin, Andrey
> ; Xing, Beilei ; Wu,
> Jingjing 
> Subject: [dpdk-dev] [PATCH 2/2] doc: add description of raw mode in flow
> director in testpmd
> 
> Add description of raw flow type mode for flow_director_filter command in
> testpmd. Modify description of flow type parameter for functions
> set_hash_global_config, set_hash_input_set and set_fdir_input_set
> 
> Signed-off-by: Kirill Rybalchenko 


Acked-by: John McNamara 

Re: [dpdk-dev] [PATCH 14/39] examples/ip_reassembly: convert to new ethdev offloads API

[Ananyev, Konstantin]

Hi Shahaf,


> + if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
> + port_conf.rxmode.offloads) {
> + printf("Some Rx offloads are not supported "
> +"by port %d: requested 0x%lx supported 0x%lx\n",
> +portid, port_conf.rxmode.offloads,
> +dev_info.rx_offload_capa);
> + port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
> + }
> + if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
> + port_conf.txmode.offloads) {
> + printf("Some Tx offloads are not supported "
> +"by port %d: requested 0x%lx supported 0x%lx\n",
> +portid, port_conf.txmode.offloads,
> +dev_info.tx_offload_capa);
> + port_conf.txmode.offloads &= dev_info.tx_offload_capa;
> + }

Sort of generic question regarding most examples - wouldn't it be better to do 
rte_exit() if device doesn't
support the offloads we expect instead of masking off unsupported offloads and 
continue?
Konstantin 

>   ret = rte_eth_dev_configure(portid, 1, (uint16_t)n_tx_queue,
>   &port_conf);