[dpdk-dev] 16.07-rc2 issue with rte_rtm_init(void) constructor

2016-07-13 Thread Damjan Marion (damarion)

Folks,

I have issues with linking application to 16.07-rc2.

Looks like reason is constructor function in include file,
so our unit test apps are failing to link as they are not linked with dpdk libs.
(and they should not be as they are not calling any dpdk function).


static inline void __attribute__((constructor))
rte_rtm_init(void)
{
rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
}

linking fails with:
dpdk/include/rte_spinlock.h:103: undefined reference to 
`rte_cpu_get_flag_enabled?

Is there any chance that this one is moved to some .c file, so it is loaded
only when it is really needed?

Thanks,

Damjan




[dpdk-dev] [PATCH] virtio: fix missing curly braces

2016-07-13 Thread Maxime Coquelin


On 07/13/2016 07:42 PM, Jan Viktorin wrote:
> On Wed, 13 Jul 2016 11:27:18 +0200
> Maxime Coquelin  wrote:
>
>> Hi Jan,
>>
>> On 07/13/2016 11:24 AM, Jan Viktorin wrote:
>>> GCC 6 is complaining and seems to be correct here.
>>>
>>> virtio_user_ethdev.c:345:2: error:
>>> this ?if? clause does not guard... [-Werror=misleading-indentation]
>>>if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
>>>^~
>>>
>>> virtio_user_ethdev.c:348:3: note:
>>> ...this statement, but the latter is misleadingly indented
>>> as if it is guarded by the ?if?
>>> if (ret < 0) {
>>>
>>> Fixes: 404bd6bfe360 ("net/virtio-user: fix return value not checked")
>>> Signed-off-by: Jan Viktorin 
>>> ---
>>>
>> I already fixed it yesterday:
>> http://dpdk.org/dev/patchwork/patch/14780/
>
> Sorry, I didn't find it quickly. My fault. Thanks.
Oh no problem! Better having two fixes than none :)

Thanks,
Maxime


[dpdk-dev] [RFC] Generic flow director/filtering/classification API

2016-07-13 Thread Adrien Mazarguil
On Mon, Jul 11, 2016 at 10:42:36AM +, Chandran, Sugesh wrote:
> Hi Adrien,
> 
> Thank you for your response,
> Please see my comments inline.

Hi Sugesh,

Sorry for the delay, please see my answers inline as well.

[...]
> > > > Flow director
> > > > -
> > > >
> > > > Flow director (FDIR) is the name of the most capable filter type, which
> > > > covers most features offered by others. As such, it is the most
> > widespread
> > > > in PMDs that support filtering (i.e. all of them besides **e1000**).
> > > >
> > > > It is also the only type that allows an arbitrary 32 bits value 
> > > > provided by
> > > > applications to be attached to a filter and returned with matching 
> > > > packets
> > > > instead of relying on the destination queue to recognize flows.
> > > >
> > > > Unfortunately, even FDIR requires applications to be aware of low-level
> > > > capabilities and limitations (most of which come directly from **ixgbe**
> > and
> > > > **i40e**):
> > > >
> > > > - Bitmasks are set globally per device (port?), not per filter.
> > > [Sugesh] This means application cannot define filters that matches on
> > arbitrary different offsets?
> > > If that?s the case, I assume the application has to program bitmask in
> > advance. Otherwise how
> > > the API framework deduce this bitmask information from the rules?? Its
> > not very clear to me
> > > that how application pass down the bitmask information for multiple 
> > > filters
> > on same port?
> > 
> > This is my understanding of how flow director currently works, perhaps
> > someome more familiar with it can answer this question better than I could.
> > 
> > Let me take an example, if particular device can only handle a single IPv4
> > mask common to all flow rules (say only to match destination addresses),
> > updating that mask to also match the source address affects all defined and
> > future flow rules simultaneously.
> > 
> > That is how FDIR currently works and I think it is wrong, as it penalizes
> > devices that do support individual bit-masks per rule, and is a little
> > awkward from an application point of view.
> > 
> > What I suggest for the new API instead is the ability to specify one
> > bit-mask per rule, and let the PMD deal with HW limitations by automatically
> > configuring global bitmasks from the first added rule, then refusing to add
> > subsequent rules if they specify a conflicting bit-mask. Existing rules
> > remain unaffected that way, and applications do not have to be extra
> > cautious.
> > 
> [Sugesh] The issue with that approach is, the hardware simply discards the 
> rule
> when it is a super set of first one eventhough the hardware is capable of 
> handling it. How its guaranteed the first rule will set the bitmask for all 
> the
> subsequent rules. 

Just to clarify, the API only says that new rules cannot affect existing
ones (which I think makes sense from a user's perspective), so as long as
the PMD does whatever is needed to make all rules work together, there
should not be any problem with this approach.

Even if the PMD has to temporarily remove an existing rule and reconfigure
global masks in order to add subsequent rules, it is fine as long as packets
aren't misdirected in the meantime (they may be dropped if there is no other
choice).

> How about having a CLASSIFER_TYPE for the classifier. Every port can have 
> set of supported flow types(for eg: L3_TYPE, L4_TYPE, L4_TYPE_8BYTE_FLEX,
> L4_TYPE_16BYTE_FLEX) based on the underlying FDIR support. Application can 
> query 
> this and set the type accordingly while initializing the port. This way the 
> first rule need 
> not set all the bits that may needed in the future rules. 

Again from a user's POV, I think doing so would add unwanted HW-specific
complexity. 

However this concern can be handled through a different approach. Let's say
user creates a pattern that only specifies a IP header with a given
bit-mask.

In FDIR language this translates to:

- Set global mask for IPv4 accordingly, remaining global masks all zeroed
  (assumed default value).

- Create an IPv4 flow.

>From now on, all rules specifying a IPv4 header must have this exact
bitmask (implicitly or explicitly), otherwise they cannot be created,
i.e. the global bitmask for IPv4 becomes immutable.

Now user creates a TCPv4 rule (as long as it uses the same IPv4 mask), to
handle this FDIR would:

- Keep global immutable mask for IPv4 unchanged, set global TCP mask
  according to the flow rule.

- Create a TCPv4 flow.

>From this point on, like IPv4, subsequent TCP rules must have this exact
bitmask and so on as the global bitmask becomes immutable.

Basically, only protocol bit-masks affected by existing flow rules are
immutable, others can be changed later. Global flow masks for protocols
become mutable again when no existing flow rule uses them.

Does it look fine for you?

[...]
> > > > +--+
> > > > | Copy to queue 8  |
> > > > 

[dpdk-dev] [dpdk-users] Intel X710 dropping 802.1ad packets?

2016-07-13 Thread Sean Hope (shope)
I sent this to dpdk-users, but maybe dpdk-dev is a better target list?

Thanks,
Sean

>Hello,
>
>I am seeing a problem with receiving 802.1ad packets on an intel X710
>card, running the 16.04 dpdk release. All the 802.1ad packets are silently
>dropped. The port is in promiscuous mode. As far as counters go, ibytes
>are incremented for these packets, but not ipackets. Some of the extended
>stats are incremented as well (the histogram sizes, etc.)
>
>Has anyone else seen this?
>
>Thanks,
>Sean
>
>



[dpdk-dev] [PATCH] virtio: fix missing curly braces

2016-07-13 Thread Jan Viktorin
On Wed, 13 Jul 2016 11:27:18 +0200
Maxime Coquelin  wrote:

> Hi Jan,
> 
> On 07/13/2016 11:24 AM, Jan Viktorin wrote:
> > GCC 6 is complaining and seems to be correct here.
> >
> > virtio_user_ethdev.c:345:2: error:
> > this ?if? clause does not guard... [-Werror=misleading-indentation]
> >if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
> >^~
> >
> > virtio_user_ethdev.c:348:3: note:
> > ...this statement, but the latter is misleadingly indented
> > as if it is guarded by the ?if?
> > if (ret < 0) {
> >
> > Fixes: 404bd6bfe360 ("net/virtio-user: fix return value not checked")
> > Signed-off-by: Jan Viktorin 
> > ---
> >  
> I already fixed it yesterday:
> http://dpdk.org/dev/patchwork/patch/14780/

Sorry, I didn't find it quickly. My fault. Thanks.

Jan

> 
> Thanks,
> Maxime


[dpdk-dev] [PATCH] virtio: fix missing curly braces

2016-07-13 Thread Jan Viktorin
On Wed, 13 Jul 2016 11:27:18 +0200
Maxime Coquelin  wrote:

> Hi Jan,
> 
> On 07/13/2016 11:24 AM, Jan Viktorin wrote:
> > GCC 6 is complaining and seems to be correct here.
> >
> > virtio_user_ethdev.c:345:2: error:
> > this ?if? clause does not guard... [-Werror=misleading-indentation]
> >if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
> >^~
> >
> > virtio_user_ethdev.c:348:3: note:
> > ...this statement, but the latter is misleadingly indented
> > as if it is guarded by the ?if?
> > if (ret < 0) {
> >
> > Fixes: 404bd6bfe360 ("net/virtio-user: fix return value not checked")
> > Signed-off-by: Jan Viktorin 
> > ---
> >  
> I already fixed it yesterday:
> http://dpdk.org/dev/patchwork/patch/14780/

Sorry, I didn't find it quickly. My fault. Thanks.

Jan

> 
> Thanks,
> Maxime



-- 
  Jan ViktorinE-mail: Viktorin at RehiveTech.com
  System ArchitectWeb:www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic


[dpdk-dev] [PATCH v6 05/17] eal: introduce init macros

2016-07-13 Thread Jan Viktorin
On Wed, 13 Jul 2016 11:20:43 +0200
Jan Viktorin  wrote:

> Hello Shreyansh,
> 
> On Tue, 12 Jul 2016 11:31:10 +0530
> Shreyansh Jain  wrote:
> 
> > Introduce a RTE_INIT macro used to mark an init function as a constructor.
> > Current eal macros have been converted to use this (no functional impact).
> > DRIVER_REGISTER_PCI is added as a helper for pci drivers.
> > 
> > Suggested-by: Jan Viktorin 
> > Signed-off-by: David Marchand 
> > Signed-off-by: Shreyansh Jain 
> > ---  
> 
> [...]
> 
> > +#define RTE_INIT(func) \
> > +static void __attribute__((constructor, used)) func(void)
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > diff --git a/lib/librte_eal/common/include/rte_pci.h 
> > b/lib/librte_eal/common/include/rte_pci.h
> > index fa74962..3027adf 100644
> > --- a/lib/librte_eal/common/include/rte_pci.h
> > +++ b/lib/librte_eal/common/include/rte_pci.h
> > @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f);
> >   */
> >  void rte_eal_pci_register(struct rte_pci_driver *driver);
> >  
> > +/** Helper for PCI device registeration from driver (eth, crypto) instance 
> > */
> > +#define DRIVER_REGISTER_PCI(nm, drv) \
> > +RTE_INIT(pciinitfn_ ##nm); \
> > +static void pciinitfn_ ##nm(void) \
> > +{ \  
> 
> You are missing setting the name here like PMD_REGISTER_DRIVER does
> now. Or should I include it in my patch set?
> 
>   (drv).name = RTE_STR(nm);

Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it
expects a wrapper around it (eth_driver)... I now, my SoC patches were
supposing the some... but I think it is wrong.

The original David's patch set contains calls like this:

  RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv);

So, I think, we should go the original way.

Jan

> 
> > +   rte_eal_pci_register(_drv); \
> > +}
> > +
> >  /**
> >   * Unregister a PCI driver.
> >   *
> > diff --git a/lib/librte_eal/common/include/rte_tailq.h 
> > b/lib/librte_eal/common/include/rte_tailq.h
> > index 4a686e6..71ed3bb 100644
> > --- a/lib/librte_eal/common/include/rte_tailq.h
> > +++ b/lib/librte_eal/common/include/rte_tailq.h
> > @@ -148,8 +148,8 @@ struct rte_tailq_head *rte_eal_tailq_lookup(const char 
> > *name);
> >  int rte_eal_tailq_register(struct rte_tailq_elem *t);
> >  
> >  #define EAL_REGISTER_TAILQ(t) \
> > -void tailqinitfn_ ##t(void); \
> > -void __attribute__((constructor, used)) tailqinitfn_ ##t(void) \
> > +RTE_INIT(tailqinitfn_ ##t); \
> > +static void tailqinitfn_ ##t(void) \
> >  { \
> > if (rte_eal_tailq_register() < 0) \
> > rte_panic("Cannot initialize tailq: %s\n", t.name); \  
> 
> 
> 



-- 
  Jan ViktorinE-mail: Viktorin at RehiveTech.com
  System ArchitectWeb:www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic


[dpdk-dev] [PATCH] net/i40e: revert VLAN filtering fix

2016-07-13 Thread Jingjing Wu
This reverts commit 4761f57d58c6f52543738dbe299f846d62d75895.
Introducing VLAN table by adding VLAN adminq command will cause NIC's
throughput drop obviously. It's a hardware issue.
With this revert, VLAN filtering can only work when promiscuous mode
is disabled.

Reverts: 4761f57d58c6 ("net/i40e: fix VLAN filtering in promiscuous mode")

Reported-by: Jeffrey Shaw 
Signed-off-by: Jingjing Wu 
---
 drivers/net/i40e/i40e_ethdev.c | 23 ---
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index daac236..3f9f05e 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2718,16 +2718,12 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 {
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
struct i40e_vsi *vsi = pf->main_vsi;
-   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);

if (mask & ETH_VLAN_FILTER_MASK) {
-   if (dev->data->dev_conf.rxmode.hw_vlan_filter) {
-   i40e_aq_set_vsi_vlan_promisc(hw, vsi->seid, false, 
NULL);
+   if (dev->data->dev_conf.rxmode.hw_vlan_filter)
i40e_vsi_config_vlan_filter(vsi, TRUE);
-   } else {
-   i40e_aq_set_vsi_vlan_promisc(hw, vsi->seid, true, NULL);
+   else
i40e_vsi_config_vlan_filter(vsi, FALSE);
-   }
}

if (mask & ETH_VLAN_STRIP_MASK) {
@@ -5777,28 +5773,17 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,
 uint16_t vlan_id, bool on)
 {
uint32_t vid_idx, vid_bit;
-   struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
-   struct i40e_aqc_add_remove_vlan_element_data vlan_data = {0};
-   int ret;

if (vlan_id > ETH_VLAN_ID_MAX)
return;

vid_idx = I40E_VFTA_IDX(vlan_id);
vid_bit = I40E_VFTA_BIT(vlan_id);
-   vlan_data.vlan_tag = rte_cpu_to_le_16(vlan_id);

-   if (on) {
-   ret = i40e_aq_add_vlan(hw, vsi->seid, _data, 1, NULL);
-   if (ret != I40E_SUCCESS)
-   PMD_DRV_LOG(ERR, "Failed to add vlan filter");
+   if (on)
vsi->vfta[vid_idx] |= vid_bit;
-   } else {
-   ret = i40e_aq_remove_vlan(hw, vsi->seid, _data, 1, NULL);
-   if (ret != I40E_SUCCESS)
-   PMD_DRV_LOG(ERR, "Failed to remove vlan filter");
+   else
vsi->vfta[vid_idx] &= ~vid_bit;
-   }
 }

 /**
-- 
2.4.0



[dpdk-dev] [PATCH] net/enic: fix segfault after receiving error interrupt

2016-07-13 Thread Nelson Escobar
When enic's interrupt handler is called indicating an error, it scans
through the receive queues (RQs) on the adapter looking for errors.
But since the inclusion of rx scatter, some of the RQs may not be in
use, and you shouldn't check them for errors.

Fixes: 856d7ba7ed22 ("net/enic: support scattered Rx")

Signed-off-by: Nelson Escobar 
Reviewed-by: John Daley 
---
 drivers/net/enic/enic_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 9ec2a2d..fbd4089 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -123,6 +123,8 @@ static void enic_log_q_error(struct enic *enic)
}

for (i = 0; i < enic_vnic_rq_count(enic); i++) {
+   if (!enic->rq[i].in_use)
+   continue;
error_status = vnic_rq_error_status(>rq[i]);
if (error_status)
dev_err(enic, "RQ[%d] error_status %d\n", i,
-- 
2.7.0



[dpdk-dev] [PATCH 3/3] doc: fix default socket path names

2016-07-13 Thread Reshma Pattan
Fixed default socket path name "/var/run" to "/var/run/.dpdk" and
"$HOME" to "~/.dpdk".

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan 
---
 doc/guides/prog_guide/pdump_lib.rst | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/pdump_lib.rst 
b/doc/guides/prog_guide/pdump_lib.rst
index 580ffcb..0136781 100644
--- a/doc/guides/prog_guide/pdump_lib.rst
+++ b/doc/guides/prog_guide/pdump_lib.rst
@@ -75,13 +75,13 @@ the packet capture.

 The packet capture framework, as part of its initialization, creates the 
pthread and the server socket in
 the pthread. The application that calls the framework initialization will have 
the server socket created,
-either under the path that the application has passed or under the default 
path i.e. either ``/var/run`` for
-root user or ``$HOME`` for non root user.
+either under the path that the application has passed or under the default 
path i.e. either ``/var/run/.dpdk`` for
+root user or ``~/.dpdk`` for non root user.

 Applications that request enabling or disabling of the packet capture will 
have the client socket created either under
-the path that the application has passed or under the default path i.e. either 
``/var/run/`` for root user or ``$HOME``
-for not root user to send the requests to the server.
-The server socket will listen for client requests for enabling or disabling 
the packet capture.
+the path that the application has passed or under the default path i.e. either 
``/var/run/.dpdk`` for root user or
+``~/.dpdk`` for not root user to send the requests to the server. The server 
socket will listen for client requests for
+enabling or disabling the packet capture.


 Implementation Details
@@ -111,7 +111,7 @@ server socket.

 The library API ``rte_pdump_set_socket_dir()``, sets the given path as either 
server socket path
 or client socket path based on the ``type`` argument of the API.
-If the given path is ``NULL``, default path will be selected, i.e. either 
``/var/run/`` for root user or ``$HOME``
+If the given path is ``NULL``, default path will be selected, i.e. either 
``/var/run/.dpdk`` for root user or ``~/.dpdk``
 for non root user. Clients also need to call this API to set their server 
socket path if the server socket
 path is different from default path.

-- 
2.5.0



[dpdk-dev] [PATCH 2/3] app/pdump: add new command line options for socket paths

2016-07-13 Thread Reshma Pattan
Since users of the pdump library and tool can chose to have their own
server and client paths, it is must for the pdump tool to use the same
server socket path that was used by primary application while
initializing packet capture framework by rte_pdump_init() or
rte_pdump_set_socket_dir() APIs.

To pass the socket path info to pdump tool a new optional command
line options "server-socket-path" and "client-socket-path" are added.

"client-socket-path" is also added, if the users want to have client
sockets in their own defined paths.

Updated pdump tool guide with the new changes.

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan 
---
 app/pdump/main.c   | 57 --
 doc/guides/sample_app_ug/pdump.rst | 31 +++--
 2 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index 2087c15..e0ff8be 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -55,6 +55,7 @@
 #include 
 #include 

+#define CMD_LINE_OPT_PDUMP "pdump"
 #define PDUMP_PORT_ARG "port"
 #define PDUMP_PCI_ARG "device_id"
 #define PDUMP_QUEUE_ARG "queue"
@@ -64,6 +65,8 @@
 #define PDUMP_RING_SIZE_ARG "ring-size"
 #define PDUMP_MSIZE_ARG "mbuf-size"
 #define PDUMP_NUM_MBUFS_ARG "total-num-mbufs"
+#define CMD_LINE_OPT_SER_SOCK_PATH "server-socket-path"
+#define CMD_LINE_OPT_CLI_SOCK_PATH "client-socket-path"

 #define VDEV_PCAP "eth_pcap_%s_%d,tx_pcap=%s"
 #define VDEV_IFACE "eth_pcap_%s_%d,tx_iface=%s"
@@ -166,6 +169,8 @@ struct parse_val {
 int num_tuples;
 static struct rte_eth_conf port_conf_default;
 volatile uint8_t quit_signal;
+static char server_socket_path[PATH_MAX];
+static char client_socket_path[PATH_MAX];

 /**< display usage */
 static void
@@ -178,8 +183,11 @@ pdump_usage(const char *prgname)
" tx-dev=,"
"[ring-size=default:16384],"
"[mbuf-size=default:2176],"
-   "[total-num-mbufs=default:65535]"
-   "'\n",
+   "[total-num-mbufs=default:65535]'\n"
+   "[--server-socket-path="
+   "default:/var/run/.dpdk/ (or) ~/.dpdk/]\n"
+   "[--client-socket-path="
+   "default:/var/run/.dpdk/ (or) ~/.dpdk/]\n",
prgname);
 }

@@ -226,9 +234,6 @@ parse_rxtxdev(const char *key, const char *value, void 
*extra_args)
/* identify the tx stream type for pcap vdev */
if (if_nametoindex(pt->tx_dev))
pt->tx_vdev_stream_type = IFACE;
-   } else {
-   printf("invalid dev type %s, must be rx or tx\n", value);
-   return -1;
}

return 0;
@@ -407,6 +412,8 @@ launch_args_parse(int argc, char **argv, char *prgname)
int option_index;
static struct option long_option[] = {
{"pdump", 1, 0, 0},
+   {"server-socket-path", 1, 0, 0},
+   {"client-socket-path", 1, 0, 0},
{NULL, 0, 0, 0}
};

@@ -418,14 +425,32 @@ launch_args_parse(int argc, char **argv, char *prgname)
long_option, _index)) != EOF) {
switch (opt) {
case 0:
-   if (!strncmp(long_option[option_index].name, "pdump",
-   MAX_LONG_OPT_SZ)) {
+   if (!strncmp(long_option[option_index].name,
+   CMD_LINE_OPT_PDUMP,
+   sizeof(CMD_LINE_OPT_PDUMP))) {
ret = parse_pdump(optarg);
if (ret) {
pdump_usage(prgname);
return -1;
}
}
+
+   if (!strncmp(long_option[option_index].name,
+   CMD_LINE_OPT_SER_SOCK_PATH,
+   sizeof(CMD_LINE_OPT_SER_SOCK_PATH))) {
+   snprintf(server_socket_path,
+   sizeof(server_socket_path), "%s",
+   optarg);
+   }
+
+   if (!strncmp(long_option[option_index].name,
+   CMD_LINE_OPT_CLI_SOCK_PATH,
+   sizeof(CMD_LINE_OPT_CLI_SOCK_PATH))) {
+   snprintf(client_socket_path,
+   sizeof(client_socket_path), "%s",
+   optarg);
+   }
+
break;
default:
pdump_usage(prgname);
@@ -719,6 +744,22 @@ enable_pdump(void)
struct pdump_tuples *pt;
   

[dpdk-dev] [PATCH 1/3] pdump: fix error handlings

2016-07-13 Thread Reshma Pattan
The changes include
1)If mkdir fails for user passed socket paths log error and return.

2)At some places return value was set to errno and that non-negative number
was returned, but the intention was to return negative value.
So now rte_errno was set to errno and returning the actual negative value
that the APIs has returned.

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Reshma Pattan 
---
 lib/librte_pdump/rte_pdump.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 22ed476..9b921ce 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -449,6 +449,7 @@ pdump_get_socket_path(char *buffer, int bufsz, enum 
rte_pdump_socktype type)
char dpdk_dir[PATH_MAX] = {0};
char dir[PATH_MAX] = {0};
char *dir_home = NULL;
+   int ret = 0;

if (type == RTE_PDUMP_SOCKET_SERVER && server_socket_dir[0] != 0)
snprintf(dir, sizeof(dir), "%s", server_socket_dir);
@@ -475,7 +476,16 @@ pdump_get_socket_path(char *buffer, int bufsz, enum 
rte_pdump_socktype type)
dpdk_dir, SOCKET_DIR);
}

-   mkdir(dir, 700);
+   ret =  mkdir(dir, 700);
+   /* if user passed socket path is invalid, return immediately */
+   if (ret < 0 && errno != EEXIST) {
+   RTE_LOG(ERR, PDUMP,
+   "Failed to create dir:%s:%s\n", dir,
+   strerror(errno));
+   rte_errno = errno;
+   return -1;
+   }
+
if (type == RTE_PDUMP_SOCKET_SERVER)
snprintf(buffer, bufsz, SERVER_SOCKET, dir);
else
@@ -667,8 +677,8 @@ pdump_create_client_socket(struct pdump_request *p)
"client socket(): %s:pid(%d):tid(%u), %s:%d\n",
strerror(errno), pid, rte_sys_gettid(),
__func__, __LINE__);
-   ret = errno;
-   return ret;
+   rte_errno = errno;
+   return -1;
}

ret = pdump_get_socket_path(addr.sun_path, sizeof(addr.sun_path),
@@ -677,6 +687,7 @@ pdump_create_client_socket(struct pdump_request *p)
RTE_LOG(ERR, PDUMP,
"Failed to get client socket path: %s:%d\n",
__func__, __LINE__);
+   rte_errno = errno;
goto exit;
}
addr.sun_family = AF_UNIX;
@@ -688,7 +699,7 @@ pdump_create_client_socket(struct pdump_request *p)
RTE_LOG(ERR, PDUMP,
"client bind(): %s, %s:%d\n",
strerror(errno), __func__, __LINE__);
-   ret = errno;
+   rte_errno = errno;
break;
}

@@ -701,6 +712,7 @@ pdump_create_client_socket(struct pdump_request *p)
RTE_LOG(ERR, PDUMP,
"Failed to get server socket path: %s:%d\n",
__func__, __LINE__);
+   rte_errno = errno;
break;
}
serv_addr.sun_family = AF_UNIX;
@@ -711,7 +723,8 @@ pdump_create_client_socket(struct pdump_request *p)
RTE_LOG(ERR, PDUMP,
"failed to send to server:%s, %s:%d\n",
strerror(errno), __func__, __LINE__);
-   ret =  errno;
+   rte_errno = errno;
+   ret = -1;
break;
}

@@ -722,7 +735,8 @@ pdump_create_client_socket(struct pdump_request *p)
RTE_LOG(ERR, PDUMP,
"failed to recv from server:%s, %s:%d\n",
strerror(errno), __func__, __LINE__);
-   ret = errno;
+   rte_errno = errno;
+   ret = -1;
break;
}
ret = server_resp.err_value;
-- 
2.5.0



[dpdk-dev] [PATCH 0/3] add new command line options and error handling in pdump

2016-07-13 Thread Reshma Pattan
This patch set contains
1)Error handling fixes in pdump library.
2)Support of server and client socket path command line options in pdump tool.
3)Default socket path name fixes in pdump library doc.

Reshma Pattan (3):
  pdump: fix error handlings
  app/pdump: add new command line options for socket paths
  doc: fix default socket path names

 app/pdump/main.c| 57 +++--
 doc/guides/prog_guide/pdump_lib.rst | 12 
 doc/guides/sample_app_ug/pdump.rst  | 31 ++--
 lib/librte_pdump/rte_pdump.c| 26 +
 4 files changed, 97 insertions(+), 29 deletions(-)

-- 
2.5.0



[dpdk-dev] [PATCH v1 1/1] examples/l2fwd-crypto: improve random key generator

2016-07-13 Thread Declan Doherty
On 05/25/2016 02:34 PM, Piotr Azarewicz wrote:
> This patch improve generate_random_key() function by replacing rand()
> function with reading from /dev/urandom.
>
> CID 120136 : Calling risky function (DC.WEAK_CRYPTO)
> dont_call: rand should not be used for security related applications, as
> linear congruential algorithms are too easy to break
>
> Coverity issue: 120136
>
> Signed-off-by: Piotr Azarewicz 
> ---
>  examples/l2fwd-crypto/main.c |   18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
> index d18c813..e1f0a1e 100644
> --- a/examples/l2fwd-crypto/main.c
> +++ b/examples/l2fwd-crypto/main.c
> @@ -45,6 +45,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>
>  #include 
>  #include 
> @@ -581,10 +583,18 @@ l2fwd_simple_forward(struct rte_mbuf *m, unsigned 
> portid)
>  static void
>  generate_random_key(uint8_t *key, unsigned length)
>  {
> - unsigned i;
> + int fd;
> + int ret;
> +
> + fd = open("/dev/urandom", O_RDONLY);
> + if (fd < 0)
> + rte_exit(EXIT_FAILURE, "Failed to generate random key\n");
>
> - for (i = 0; i < length; i++)
> - key[i] = rand() % 0xff;
> + ret = read(fd, key, length);
> + close(fd);
> +
> + if (ret != (signed)length)
> + rte_exit(EXIT_FAILURE, "Failed to generate random key\n");
>  }
>
>  static struct rte_cryptodev_sym_session *
> @@ -1180,8 +1190,6 @@ l2fwd_crypto_parse_timer_period(struct 
> l2fwd_crypto_options *options,
>  static void
>  l2fwd_crypto_default_options(struct l2fwd_crypto_options *options)
>  {
> - srand(time(NULL));
> -
>   options->portmask = 0x;
>   options->nb_ports_per_lcore = 1;
>   options->refresh_period = 1;
>

Acked-by: Declan Doherty 


[dpdk-dev] [PATCH v1 1/1] examples/l2fwd-crypto: improve random key generator

2016-07-13 Thread Declan Doherty
On 07/11/2016 03:17 PM, Thomas Monjalon wrote:
> 2016-06-08 07:46, Azarewicz, PiotrX T:
>>> 2016-05-25 15:34, Piotr Azarewicz:
 This patch improve generate_random_key() function by replacing rand()
 function with reading from /dev/urandom.

 CID 120136 : Calling risky function (DC.WEAK_CRYPTO)
 dont_call: rand should not be used for security related applications,
 as linear congruential algorithms are too easy to break

 Coverity issue: 120136

 Signed-off-by: Piotr Azarewicz 
>>>
>>> Is it relevant for this example?
>>
>> Maybe not. But it don't break anything, and in the end make Coverity tool 
>> happy.
>>
>> Declan, please share your opinion.
>
> Declan?
>

sorry I'm missed this thread. While not strictly necessary for the 
example app, I don't see a problem applying it, as coverity points out 
it is a bad idea to use rand() for crypto purposes.

Declan




[dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address.

2016-07-13 Thread Yuanhan Liu
On Wed, Jul 13, 2016 at 10:34:07AM +0300, Ilya Maximets wrote:
> On 12.07.2016 08:53, Ilya Maximets wrote:
> > On 12.07.2016 05:43, Yuanhan Liu wrote:
> >> On Mon, Jul 11, 2016 at 02:47:56PM +0300, Ilya Maximets wrote:
> >>> On 11.07.2016 14:05, Yuanhan Liu wrote:
>  On Mon, Jul 11, 2016 at 12:50:24PM +0300, Ilya Maximets wrote:
> > On 11.07.2016 11:38, Yuanhan Liu wrote:
> >> On Sun, Jul 10, 2016 at 09:17:31PM +0800, Yuanhan Liu wrote:
> >>> On Fri, Jul 08, 2016 at 02:48:56PM +0300, Ilya Maximets wrote:
> 
>  Another point is that crash constantly happens on queue_id=3 (second 
>  RX queue) in
>  my scenario. It is newly allocated virtqueue while reconfiguration 
>  from rxq=1 to
>  rxq=2.
> >>>
> >>> That's a valuable message: what's your DPDK HEAD commit while 
> >>> triggering
> >>> this issue?
> >
> > fbfd99551ca3 ("mbuf: add raw allocation function")
> >
> >>
> >> I guess I have understood what goes wrong in you case.
> >>
> >> I would guess that your vhost has 2 queues (here I mean queue-pairs,
> >> including one Tx and Rx queue; below usage is the same) configured,
> >> so does to your QEMU. However, you just enabled 1 queue while starting
> >> testpmd inside the guest, and you want to enable 2 queues by running
> >> following testpmd commands:
> >>
> >> stop
> >> port stop all
> >> port config all rxq 2
> >> port config all txq 2
> >> port start all
> >>
> >> Badly, that won't work for current virtio PMD implementation, and 
> >> what's
> >> worse, it triggers a vhost crash, the one you saw.
> >>
> >> Here is how it comes. Since you just enabled 1 queue while starting
> >> testpmd, it will setup 1 queue only, meaning only one queue's **valid**
> >> information will be sent to vhost. You might see SET_VRING_ADDR
> >> (and related vhost messages) for the other queue as well, but they
> >> are just the dummy messages: they don't include any valid/real
> >> information about the 2nd queue: the driver don't setup it after all.
> >>
> >> So far, so good. It became broken when you run above commands. Those
> >> commands do setup for the 2nd queue, however, they failed to trigger
> >> the QEMU virtio device to start the vhost-user negotiation, meaning
> >> no SET_VRING_ADDR will be sent for the 2nd queue, leaving vhost
> >> untold and not updated.
> >>
> >> What's worse, above commands trigger the QEMU to send SET_VRING_ENABLE
> >> messages, to enable all the vrings. And since the vrings for the 2nd
> >> queue are not properly configured, the crash happens.
> >
> > Hmm, why 2nd queue works properly with my fix to vhost in this case?
> 
>  Hmm, really? You are sure that data flows in your 2nd queue after those
>  commands? From what I know is that your patch just avoid a crash, but
>  does not fix it.
> >>>
> >>> Oh, sorry. Yes, it doesn't work. With my patch applied I have a QEMU hang.
> >>
> >> The crash actually could be avoided by commit 0823c1cb0a73 ("vhost:
> >> workaround stale vring base"), accidentally. That's why I asked you
> >> above the HEAD commit you were using.
> > 
> > Thanks for pointing this. I'll check it.
> 
> I've checked my DPDK HEAD with above commit backported. Yes, it helps to
> avoid vhost crash in my scenario. As expected, after reconfiguration new
> virtqueue doesn't work, QEMU hangs sometimes.
> >> So maybe we should do virtio reset on port start?
> >
> > I guess it was removed by this patch:
> > a85786dc816f ("virtio: fix states handling during initialization").
> 
>  Seems yes.
> >>
> >> Actually, we should not do that: do reset on port start. The right fix
> >> should be allocating MAX queues virtio device supports (2 here). This
> >> would allow us changing the queue number dynamically.
> > 
> > Yes, I agree that this is the right way to fix this issue.
> >  
> >> But this doesn't sound a simple fix; it involves many code changes, due
> >> to it was not designed this way before. Therefore, we will not fix it
> >> in this release, due to it's too late. Let's fix it in the next release
> >> instead. For the crash issue, it will not happen with the latest HEAD.
> >> Though it's accident fix, I think we are fine here.
> 
> This scenario fixed somehow, I agree. But this patch still needed to protect
> vhost from untrusted VM, from malicious or buggy virtio application.
> Maybe we could change the commit-message and resend this patch as a
> security enhancement? What do you think?

Indeed, but I'm a bit concerned about the performance regression found
by Rich, yet I am not quite sure why it happens, though Rich claimed
that it seems to be a problem related to compiler.

--yliu


[dpdk-dev] [PATCH v2 2/2] doc: add vhost_user live migration image

2016-07-13 Thread Bernard Iremonger
This patch adds an image of the Live Migration of a VM using vhost_user
on the host, test configuration.

Signed-off-by: Bernard Iremonger 
---
 doc/guides/howto/img/lm_vhost_user.svg| 644 ++
 doc/guides/howto/lm_virtio_vhost_user.rst |   4 +
 2 files changed, 648 insertions(+)
 create mode 100644 doc/guides/howto/img/lm_vhost_user.svg

diff --git a/doc/guides/howto/img/lm_vhost_user.svg 
b/doc/guides/howto/img/lm_vhost_user.svg
new file mode 100644
index 000..3601cf1
--- /dev/null
+++ b/doc/guides/howto/img/lm_vhost_user.svg
@@ -0,0 +1,644 @@
+
+
+
+http://purl.org/dc/elements/1.1/;
+   xmlns:cc="http://creativecommons.org/ns#;
+   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#;
+   xmlns:svg="http://www.w3.org/2000/svg;
+   xmlns="http://www.w3.org/2000/svg;
+   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd;
+   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape;
+   width="1052.8693"
+   height="762.99158"
+   id="svg2"
+   version="1.1"
+   inkscape:version="0.48.4 r9939"
+   sodipodi:docname="lm_vhost_user.svg">
+  
+
+  
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+  
+image/svg+xml
+http://purl.org/dc/dcmitype/StillImage; />
+
+  
+
+  
+  
+
+
+
+
+
+
+
+
+VM 1 
+Switch with 10Gb ports
+Server 1
+Server 2
+  10 Gb Traffic Generator
+VM 2 
+Linux, KVM, QEMU 2.5 
+10 Gb NIC
+10 Gb NIC
+
+
+10 Gb NIC
+10 Gb NIC
+
+
+DPDK Testpmd App
+DPDK virtio PMD's 
+DPDK PF PMD and vhost_user
+DPDK PF PMD and vhost_user
+NFS ServerVM disk image
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+10 Gb Migration Link
+DPDK Testpmd App
+DPDK virtio PMD's  
+Linux, KVM, QEMU 2.5 
+  
+
diff --git a/doc/guides/howto/lm_virtio_vhost_user.rst 
b/doc/guides/howto/lm_virtio_vhost_user.rst
index 2de3ef7..d8f6365 100644
--- a/doc/guides/howto/lm_virtio_vhost_user.rst
+++ b/doc/guides/howto/lm_virtio_vhost_user.rst
@@ -49,6 +49,10 @@ The switch is configured to broadcast traffic on all the NIC 
ports.
 Live Migration with Virtio and vhost_user test setup:
 -

+.. _figure_lm_vhost_user:
+
+.. figure:: img/lm_vhost_user.*
+
 Live Migration steps for VM with Virtio PMD and vhost_user on host:
 ---

-- 
2.9.0



[dpdk-dev] [PATCH v2 1/2] doc: live migration of VM with vhost_user on host

2016-07-13 Thread Bernard Iremonger
This patch describes the procedure to be be followed to perform
Live Migration of a VM with Virtio PMD running on a host which
is running the vhost_user sample application (vhost-switch).

It includes sample host and VM scripts used in the procedure.

Signed-off-by: Bernard Iremonger 
---
 doc/guides/howto/index.rst|   1 +
 doc/guides/howto/lm_virtio_vhost_user.rst | 455 ++
 2 files changed, 456 insertions(+)
 create mode 100644 doc/guides/howto/lm_virtio_vhost_user.rst

diff --git a/doc/guides/howto/index.rst b/doc/guides/howto/index.rst
index 4b97a32..d3e3a90 100644
--- a/doc/guides/howto/index.rst
+++ b/doc/guides/howto/index.rst
@@ -36,3 +36,4 @@ How To User Guide
 :numbered:

 lm_bond_virtio_sriov
+lm_virtio_vhost_user
diff --git a/doc/guides/howto/lm_virtio_vhost_user.rst 
b/doc/guides/howto/lm_virtio_vhost_user.rst
new file mode 100644
index 000..2de3ef7
--- /dev/null
+++ b/doc/guides/howto/lm_virtio_vhost_user.rst
@@ -0,0 +1,455 @@
+..  BSD LICENSE
+Copyright(c) 2016 Intel Corporation. All rights reserved.
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+* Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+* Redistributions in binary form must reproduce the above copyright
+notice, this list of conditions and the following disclaimer in
+the documentation and/or other materials provided with the
+distribution.
+* Neither the name of Intel Corporation nor the names of its
+contributors may be used to endorse or promote products derived
+from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+Live Migration of VM with Virtio on host running vhost_user:
+
+
+Live Migration overview for VM with Virtio:
+---
+
+To test the Live Migration two servers with identical operating systems 
installed are used.
+KVM and QEMU is also required on the servers.
+
+QEMU 2.5 is required for Live Migration of a VM with vhost_user running on the 
hosts.
+
+The servers have Niantic and or Fortville NIC's installed.
+The NIC's on both servers are connected to a switch
+which is also connected to the traffic generator.
+
+The switch is configured to broadcast traffic on all the NIC ports.
+
+Live Migration with Virtio and vhost_user test setup:
+-
+
+Live Migration steps for VM with Virtio PMD and vhost_user on host:
+---
+
+The host is running the DPDK PMD (ixgbe or i40e) and the DPDK vhost_user
+sample application (vhost-switch).
+
+The ip address of host_server_1 is 10.237.212.46
+
+The ip address of host_server_2 is 10.237.212.131
+
+The sample scripts mentioned in the steps below can be found in the 
host_scripts
+and vm_scripts sections.
+
+On host_server_1: Terminal 1
+
+
+Setup DPDK on host_server_1
+
+.. code-block:: console
+
+cd /root/dpdk/host_scripts
+./setup_dpdk_on_host.sh
+
+On host_server_1: Terminal 2
+
+
+Bind the Niantic or Fortville NIC to igb_uio on host_server_1.
+
+For Fortville NIC
+
+.. code-block:: console
+
+   cd /root/dpdk/tools
+   ./dpdk_nic_bind.py -b igb_uio :02:00.0
+
+For Niantic NIC
+
+.. code-block:: console
+
+   cd /root/dpdk/tools
+   ./dpdk_nic_bind.py -b igb_uio :09:00.0
+
+On host_server_1: Terminal 3
+
+
+For Fortville and Niantic NIC's reset SRIOV and run the
+vhost_user sample application (vhost-switch) on host_server_1.
+
+.. code-block:: console
+
+ cd /root/dpdk/host_scripts
+./reset_vf_on_212_46.sh
+./run_vhost_switch_on_host.sh
+
+On host_server_1: Terminal 1
+
+
+Start the VM on host_server_1
+
+.. code-block:: console
+
+   ./vm_virtio_vhost_user.sh
+
+On host_server_1: Terminal 4

[dpdk-dev] [PATCH v2 0/2] doc: live migration procedure with vhost_user

2016-07-13 Thread Bernard Iremonger
This patchset describes the procedure to Live migrate a VM with
Virtio PMD's with the vhost_user sample application (vhost-switch)
running on the host.

This patchset depends on the following patch:
http://dpdk.org/dev/patchwork/patch/14625

Changes in v2:
removed l2fwd information
minor changes to svg file

Bernard Iremonger (2):
  doc: live migration of VM with vhost_user on host
  doc: add vhost_user live migration image

 doc/guides/howto/img/lm_vhost_user.svg| 644 ++
 doc/guides/howto/index.rst|   1 +
 doc/guides/howto/lm_virtio_vhost_user.rst | 459 +
 3 files changed, 1104 insertions(+)
 create mode 100644 doc/guides/howto/img/lm_vhost_user.svg
 create mode 100644 doc/guides/howto/lm_virtio_vhost_user.rst

-- 
2.9.0



[dpdk-dev] [PATCH] net/mlx5: work around compilation issue

2016-07-13 Thread Adrien Mazarguil
From: Olga Shern 

RHEL 7.1's GCC for POWER8 reports the following error in one rte_memcpy()
macro call by mlx5:

 error: array subscript is above array bounds [-Werror=array-bounds]

It appears to be a GCC bug which can be worked around by making parentheses
more explicit.

Signed-off-by: Olga Shern 
Signed-off-by: Adrien Mazarguil 
---
 drivers/net/mlx5/mlx5_rxtx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 615de94..fce3381 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -472,8 +472,8 @@ mlx5_wqe_write_inline_vlan(struct txq *txq, volatile union 
mlx5_wqe *wqe,
   (uint8_t *)addr, 12);
rte_memcpy((uint8_t *)(uintptr_t)wqe->inl.eseg.inline_hdr_start + 12,
   , sizeof(vlan));
-   rte_memcpy((uint8_t *)(uintptr_t)wqe->inl.eseg.inline_hdr_start + 16,
-  ((uint8_t *)addr + 12), 2);
+   rte_memcpy((uint8_t *)((uintptr_t)wqe->inl.eseg.inline_hdr_start + 16),
+  (uint8_t *)(addr + 12), 2);
addr += MLX5_ETH_VLAN_INLINE_HEADER_SIZE - sizeof(vlan);
length -= MLX5_ETH_VLAN_INLINE_HEADER_SIZE - sizeof(vlan);
size = (sizeof(wqe->inl.ctrl.ctrl) +
-- 
2.1.4



[dpdk-dev] [PATCH] mempool: fix empty structure definition

2016-07-13 Thread Olivier MATZ

On 07/13/2016 02:30 PM, Adrien Mazarguil wrote:
> This commit addresses the following warning reported by clang, which
> happens by default, as long as CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG is disabled:
>
>   warning: empty struct has size 0 in C, size 1 in C++
>
> C and C++ must use the same size for objects to avoid corruption during run
> time.
>
> Fixes: 97e7e685bfcd ("mempool: add structure for object trailers")
>
> Signed-off-by: Adrien Mazarguil 

Acked-by: Olivier Matz 



[dpdk-dev] [PATCH v4 10/10] scripts: check compilation of exported header files

2016-07-13 Thread Adrien Mazarguil
This script checks that header files in a given directory do not miss
dependencies when included on their own, do not conflict and accept being
compiled with the strictest possible flags.

Signed-off-by: Adrien Mazarguil 
---
 MAINTAINERS   |   1 +
 scripts/check-includes.sh | 286 +
 scripts/test-build.sh |  14 ++
 3 files changed, 301 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f996c2e..e2933c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26,6 +26,7 @@ T: git://dpdk.org/dpdk
 F: MAINTAINERS
 F: scripts/check-maintainers.sh
 F: scripts/check-git-log.sh
+F: scripts/check-includes.sh
 F: scripts/checkpatches.sh
 F: scripts/load-devel-config.sh
 F: scripts/test-build.sh
diff --git a/scripts/check-includes.sh b/scripts/check-includes.sh
new file mode 100755
index 000..d65adc6
--- /dev/null
+++ b/scripts/check-includes.sh
@@ -0,0 +1,286 @@
+#!/bin/sh -e
+#
+#   BSD LICENSE
+#
+#   Copyright 2016 6WIND S.A.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+# * Redistributions of source code must retain the above copyright
+#   notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+#   notice, this list of conditions and the following disclaimer in
+#   the documentation and/or other materials provided with the
+#   distribution.
+# * Neither the name of 6WIND S.A. nor the names of its
+#   contributors may be used to endorse or promote products derived
+#   from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# This script checks that header files in a given directory do not miss
+# dependencies when included on their own, do not conflict and accept being
+# compiled with the strictest possible flags.
+#
+# Files are looked up in the directory provided as the first argument,
+# otherwise build/include by default.
+#
+# Recognized environment variables:
+#
+# VERBOSE=1 is the same as -v.
+#
+# QUIET=1 is the same as -q.
+#
+# SUMMARY=1 is the same as -s.
+#
+# CC, CPPFLAGS, CFLAGS, EXTRA_CPPFLAGS, EXTRA_CFLAGS, CXX, CXXFLAGS and
+# EXTRA_CXXFLAGS are taken into account.
+#
+# PEDANTIC_CFLAGS, PEDANTIC_CXXFLAGS and PEDANTIC_CPPFLAGS provide strict
+# C/C++ compilation flags.
+#
+# IGNORE contains a list of shell patterns matching files (relative to the
+# include directory) to avoid. It is set by default to known DPDK headers
+# which must not be included on their own.
+#
+# IGNORE_CXX provides additional files for C++.
+
+while getopts hqvs arg; do
+   case $arg in
+   h)
+   cat < /dev/null
+
+[ "$VERBOSE" = 1 ] &&
+output ()
+{
+   local CCV
+   local CXXV
+
+   shift
+   CCV=$CC
+   CXXV=$CXX
+   CC="echo $CC" CXX="echo $CXX" "$@"
+   CC=$CCV
+   CXX=$CXXV
+
+   "$@"
+} ||
+output ()
+{
+
+   printf '  %s\n' "$1"
+   shift
+   "$@"
+}
+
+trap 'rm -f "$temp_cc" "$temp_cxx"' EXIT
+
+compile_cc ()
+{
+   ${CC} -I"$include_dir" \
+   ${PEDANTIC_CPPFLAGS} ${CPPFLAGS} ${EXTRA_CPPFLAGS} \
+   ${PEDANTIC_CFLAGS} ${CFLAGS} ${EXTRA_CFLAGS} \
+   -c -o /dev/null "${temp_cc}"
+}
+
+compile_cxx ()
+{
+   ${CXX} -I"$include_dir" \
+   ${PEDANTIC_CPPFLAGS} ${CPPFLAGS} ${EXTRA_CPPFLAGS} \
+   ${PEDANTIC_CXXFLAGS} ${CXXFLAGS} ${EXTRA_CXXFLAGS} \
+   -c -o /dev/null "${temp_cxx}"
+}
+
+ignore ()
+{
+   file="$1"
+   shift
+   while [ $# -ne 0 ]; do
+   case "$file" in
+   $1)
+   return 0
+   ;;
+   esac
+   shift
+   done
+   return 1
+}
+
+# Check C/C++ compilation for each header file.
+
+while read -r path
+do
+   file=${path#$include_dir}
+   file=${file##/}
+   if ignore "$file" $IGNORE; then
+   output "SKIP $file" :
+   continue
+   fi
+   if printf "\
+#include <%s>
+
+int main(void)
+{
+   return 0;
+}
+" 

[dpdk-dev] [PATCH v4 09/10] lib: hide static functions never defined

2016-07-13 Thread Adrien Mazarguil
Arch-specific functions not defined for all architectures (missing on x86
in this case) and not used anywhere should not expose a prototype.

This commit prevents the following error:

 error: `rte_mov48' declared `static' but never defined

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_eal/common/include/generic/rte_memcpy.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_eal/common/include/generic/rte_memcpy.h 
b/lib/librte_eal/common/include/generic/rte_memcpy.h
index afb0afe..4e9d879 100644
--- a/lib/librte_eal/common/include/generic/rte_memcpy.h
+++ b/lib/librte_eal/common/include/generic/rte_memcpy.h
@@ -64,6 +64,8 @@ rte_mov16(uint8_t *dst, const uint8_t *src);
 static inline void
 rte_mov32(uint8_t *dst, const uint8_t *src);

+#ifdef __DOXYGEN__
+
 /**
  * Copy 48 bytes from one location to another using optimised
  * instructions. The locations should not overlap.
@@ -76,6 +78,8 @@ rte_mov32(uint8_t *dst, const uint8_t *src);
 static inline void
 rte_mov48(uint8_t *dst, const uint8_t *src);

+#endif /* __DOXYGEN__ */
+
 /**
  * Copy 64 bytes from one location to another using optimised
  * instructions. The locations should not overlap.
-- 
2.1.4



[dpdk-dev] [PATCH v4 08/10] lib: remove named variadic macros in exported headers

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.

Since there is no way to force named variadic macros as extensions, use a
a standard __VA_ARGS__ with an extra dummy argument to format strings.

This commit prevents the following errors:

 error: ISO C does not permit named variadic macros

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_cryptodev/rte_cryptodev.h   | 32 ++---
 lib/librte_cryptodev/rte_cryptodev_pmd.h   |  2 +-
 lib/librte_eal/common/include/rte_common.h |  9 +++
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
b/lib/librte_cryptodev/rte_cryptodev.h
index cf28541..d047ba8 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -77,26 +77,30 @@ extern const char **rte_cyptodev_names;

 /* Logging Macros */

-#define CDEV_LOG_ERR(fmt, args...) \
-   RTE_LOG(ERR, CRYPTODEV, "%s() line %u: " fmt "\n",  \
-   __func__, __LINE__, ## args)
+#define CDEV_LOG_ERR(...) \
+   RTE_LOG(ERR, CRYPTODEV, \
+   RTE_FMT("%s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
+   __func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))

-#define CDEV_PMD_LOG_ERR(dev, fmt, args...)\
-   RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
-   dev, __func__, __LINE__, ## args)
+#define CDEV_PMD_LOG_ERR(dev, ...) \
+   RTE_LOG(ERR, CRYPTODEV, \
+   RTE_FMT("[%s] %s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
+   dev, __func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))

 #ifdef RTE_LIBRTE_CRYPTODEV_DEBUG
-#define CDEV_LOG_DEBUG(fmt, args...)   \
-   RTE_LOG(DEBUG, CRYPTODEV, "%s() line %u: " fmt "\n",\
-   __func__, __LINE__, ## args)\
+#define CDEV_LOG_DEBUG(...) \
+   RTE_LOG(DEBUG, CRYPTODEV, \
+   RTE_FMT("%s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
+   __func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))

-#define CDEV_PMD_TRACE(fmt, args...)   \
-   RTE_LOG(DEBUG, CRYPTODEV, "[%s] %s: " fmt "\n", \
-   dev, __func__, ## args)
+#define CDEV_PMD_TRACE(...) \
+   RTE_LOG(DEBUG, CRYPTODEV, \
+   RTE_FMT("[%s] %s: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
+   dev, __func__, RTE_FMT_TAIL(__VA_ARGS__,)))

 #else
-#define CDEV_LOG_DEBUG(fmt, args...)
-#define CDEV_PMD_TRACE(fmt, args...)
+#define CDEV_LOG_DEBUG(...) (void)0
+#define CDEV_PMD_TRACE(...) (void)0
 #endif

 /**
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h 
b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index cf08a50..4a07362 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -62,7 +62,7 @@ extern "C" {
 #define RTE_PMD_DEBUG_TRACE(...) \
rte_pmd_debug_trace(__func__, __VA_ARGS__)
 #else
-#define RTE_PMD_DEBUG_TRACE(fmt, args...)
+#define RTE_PMD_DEBUG_TRACE(...)
 #endif

 struct rte_cryptodev_session {
diff --git a/lib/librte_eal/common/include/rte_common.h 
b/lib/librte_eal/common/include/rte_common.h
index 98ecc1c..db5ac91 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -335,6 +335,15 @@ rte_bsf32(uint32_t v)
 /** Take a macro value and get a string version of it */
 #define RTE_STR(x) _RTE_STR(x)

+/**
+ * ISO C helpers to modify format strings using variadic macros.
+ * This is a replacement for the ", ## __VA_ARGS__" GNU extension.
+ * An empty %s argument is appended to avoid a dangling comma.
+ */
+#define RTE_FMT(fmt, ...) fmt "%.0s", __VA_ARGS__ ""
+#define RTE_FMT_HEAD(fmt, ...) fmt
+#define RTE_FMT_TAIL(fmt, ...) __VA_ARGS__
+
 /** Mask value of type "tp" for the first "ln" bit set. */
 #defineRTE_LEN2MASK(ln, tp)\
((tp)((uint64_t)-1 >> (sizeof(uint64_t) * CHAR_BIT - (ln
-- 
2.1.4



[dpdk-dev] [PATCH v4 07/10] lib: work around forward reference to enum types

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.

This commit prevents the following errors:

 error: ISO C forbids forward references to `enum' types

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_eal/common/include/generic/rte_cpuflags.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h 
b/lib/librte_eal/common/include/generic/rte_cpuflags.h
index c1da357..71321f3 100644
--- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
+++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
@@ -44,6 +44,7 @@
 /**
  * Enumeration of all CPU features supported
  */
+__extension__
 enum rte_cpu_flag_t;

 /**
@@ -55,6 +56,7 @@ enum rte_cpu_flag_t;
  * flag name
  * NULL if flag ID is invalid
  */
+__extension__
 const char *
 rte_cpu_get_flag_name(enum rte_cpu_flag_t feature);

@@ -68,6 +70,7 @@ rte_cpu_get_flag_name(enum rte_cpu_flag_t feature);
  * 0 if flag is not available
  * -ENOENT if flag is invalid
  */
+__extension__
 int
 rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature);

-- 
2.1.4



[dpdk-dev] [PATCH v4 06/10] lib: add missing include dependencies

2016-07-13 Thread Adrien Mazarguil
Exported header files for use by applications should be self sufficient and
allow out of order inclusion. Moreover, they must include all the system
headers they need for types and macros.

This commit prevents the following errors:

 error: `RTE_MAX_LCORE' undeclared here (not in a function)
 error: `RTE_LPM_VALID_EXT_ENTRY_BITMASK' undeclared
  (first use in this function)
 error: #error "Unsupported cache line size"
 error: `asm' undeclared (first use in this function)
 error: implicit declaration of function `[...]'
 error: unknown type name `[...]'
 error: field `mac_addr' has incomplete type
 error: `CHAR_BIT' undeclared here (not in a function)
 error: `struct [...]' declared inside parameter list
 error: unknown type name `uint8_t'

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_cfgfile/rte_cfgfile.h  | 2 ++
 lib/librte_cmdline/cmdline.h  | 1 +
 lib/librte_cmdline/cmdline_parse_portlist.h   | 1 +
 lib/librte_cmdline/cmdline_socket.h   | 3 +++
 lib/librte_eal/common/include/arch/arm/rte_byteorder.h| 2 ++
 lib/librte_eal/common/include/arch/arm/rte_prefetch_32.h  | 1 +
 lib/librte_eal/common/include/arch/arm/rte_prefetch_64.h  | 1 +
 lib/librte_eal/common/include/arch/arm/rte_vect.h | 1 +
 lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h| 1 +
 lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h | 1 +
 lib/librte_eal/common/include/arch/ppc_64/rte_prefetch.h  | 1 +
 lib/librte_eal/common/include/arch/x86/rte_atomic.h   | 2 ++
 lib/librte_eal/common/include/arch/x86/rte_atomic_32.h| 6 ++
 lib/librte_eal/common/include/arch/x86/rte_atomic_64.h| 8 
 lib/librte_eal/common/include/arch/x86/rte_byteorder.h| 2 ++
 lib/librte_eal/common/include/arch/x86/rte_byteorder_32.h | 7 +++
 lib/librte_eal/common/include/arch/x86/rte_byteorder_64.h | 7 +++
 lib/librte_eal/common/include/arch/x86/rte_prefetch.h | 1 +
 lib/librte_eal/common/include/arch/x86/rte_rtm.h  | 1 +
 lib/librte_eal/common/include/arch/x86/rte_vect.h | 2 ++
 lib/librte_eal/common/include/generic/rte_atomic.h| 1 +
 lib/librte_eal/common/include/generic/rte_byteorder.h | 2 ++
 lib/librte_eal/common/include/rte_eal.h   | 1 +
 lib/librte_eal/common/include/rte_memory.h| 2 ++
 lib/librte_eal/common/include/rte_time.h  | 8 
 lib/librte_eal/common/include/rte_version.h   | 1 +
 lib/librte_ether/rte_dev_info.h   | 2 ++
 lib/librte_ether/rte_eth_ctrl.h   | 4 
 lib/librte_lpm/rte_lpm_neon.h | 1 +
 lib/librte_lpm/rte_lpm_sse.h  | 1 +
 lib/librte_pdump/rte_pdump.h  | 4 
 lib/librte_reorder/rte_reorder.h  | 2 ++
 lib/librte_sched/rte_bitmap.h | 1 +
 lib/librte_sched/rte_reciprocal.h | 2 ++
 lib/librte_sched/rte_sched_common.h   | 1 +
 35 files changed, 84 insertions(+)

diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
index f649836..e81a5a2 100644
--- a/lib/librte_cfgfile/rte_cfgfile.h
+++ b/lib/librte_cfgfile/rte_cfgfile.h
@@ -34,6 +34,8 @@
 #ifndef __INCLUDE_RTE_CFGFILE_H__
 #define __INCLUDE_RTE_CFGFILE_H__

+#include 
+
 #ifdef __cplusplus
 extern "C" {
 #endif
diff --git a/lib/librte_cmdline/cmdline.h b/lib/librte_cmdline/cmdline.h
index 2578ca8..65d73b0 100644
--- a/lib/librte_cmdline/cmdline.h
+++ b/lib/librte_cmdline/cmdline.h
@@ -63,6 +63,7 @@

 #include 
 #include 
+#include 

 /**
  * @file
diff --git a/lib/librte_cmdline/cmdline_parse_portlist.h 
b/lib/librte_cmdline/cmdline_parse_portlist.h
index 73d70e0..058df3e 100644
--- a/lib/librte_cmdline/cmdline_parse_portlist.h
+++ b/lib/librte_cmdline/cmdline_parse_portlist.h
@@ -61,6 +61,7 @@
 #ifndef _PARSE_PORTLIST_H_
 #define _PARSE_PORTLIST_H_

+#include 
 #include 

 #ifdef __cplusplus
diff --git a/lib/librte_cmdline/cmdline_socket.h 
b/lib/librte_cmdline/cmdline_socket.h
index 8cc2dfb..aa6068e 100644
--- a/lib/librte_cmdline/cmdline_socket.h
+++ b/lib/librte_cmdline/cmdline_socket.h
@@ -61,6 +61,9 @@
 #ifndef _CMDLINE_SOCKET_H_
 #define _CMDLINE_SOCKET_H_

+#include 
+#include 
+
 #ifdef __cplusplus
 extern "C" {
 #endif
diff --git a/lib/librte_eal/common/include/arch/arm/rte_byteorder.h 
b/lib/librte_eal/common/include/arch/arm/rte_byteorder.h
index 3f2dd1f..1b312b3 100644
--- a/lib/librte_eal/common/include/arch/arm/rte_byteorder.h
+++ b/lib/librte_eal/common/include/arch/arm/rte_byteorder.h
@@ -41,6 +41,8 @@
 extern "C" {
 #endif

+#include 
+#include 
 #include "generic/rte_byteorder.h"

 /* fix missing __builtin_bswap16 for gcc older then 4.8 */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_prefetch_32.h 
b/lib/librte_eal/common/include/arch/arm/rte_prefetch_32.h
index 

[dpdk-dev] [PATCH v4 05/10] lib: work around unnamed structs/unions

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked to avoid warnings and compilation failures.

Unnamed structs/unions are allowed since C11, however many compiler
versions do not use this mode by default.

This commit prevents the following errors:

 error: ISO C99 doesn't support unnamed structs/unions
 error: struct has no named members

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_cryptodev/rte_crypto.h | 2 ++
 lib/librte_cryptodev/rte_crypto_sym.h | 3 +++
 lib/librte_cryptodev/rte_cryptodev.h  | 4 
 lib/librte_cryptodev/rte_cryptodev_pmd.h  | 2 ++
 lib/librte_eal/common/include/arch/ppc_64/rte_cycles.h| 2 ++
 lib/librte_eal/common/include/arch/x86/rte_atomic_32.h| 3 +++
 lib/librte_eal/common/include/arch/x86/rte_cycles.h   | 2 ++
 lib/librte_eal/common/include/rte_common.h| 7 +++
 lib/librte_eal/common/include/rte_devargs.h   | 1 +
 lib/librte_eal/common/include/rte_interrupts.h| 2 ++
 lib/librte_eal/common/include/rte_memory.h| 1 +
 lib/librte_eal/common/include/rte_memzone.h   | 2 ++
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h | 1 +
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 4 
 lib/librte_hash/rte_thash.h   | 3 +++
 lib/librte_lpm/rte_lpm.h  | 1 +
 lib/librte_mbuf/rte_mbuf.h| 5 +
 lib/librte_mempool/rte_mempool.h  | 2 ++
 lib/librte_pipeline/rte_pipeline.h| 2 ++
 lib/librte_timer/rte_timer.h  | 2 ++
 20 files changed, 51 insertions(+)

diff --git a/lib/librte_cryptodev/rte_crypto.h 
b/lib/librte_cryptodev/rte_crypto.h
index 5bc3eaa..9019518 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -48,6 +48,7 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 

 #include "rte_crypto_sym.h"

@@ -111,6 +112,7 @@ struct rte_crypto_op {
void *opaque_data;
/**< Opaque pointer for user data */

+   RTE_STD_C11
union {
struct rte_crypto_sym_op *sym;
/**< Symmetric operation parameters */
diff --git a/lib/librte_cryptodev/rte_crypto_sym.h 
b/lib/librte_cryptodev/rte_crypto_sym.h
index d9bd821..8178e5a 100644
--- a/lib/librte_cryptodev/rte_crypto_sym.h
+++ b/lib/librte_cryptodev/rte_crypto_sym.h
@@ -51,6 +51,7 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 


 /** Symmetric Cipher Algorithms */
@@ -333,6 +334,7 @@ struct rte_crypto_sym_xform {
/**< next xform in chain */
enum rte_crypto_sym_xform_type type
; /**< xform type */
+   RTE_STD_C11
union {
struct rte_crypto_auth_xform auth;
/**< Authentication / hash xform */
@@ -371,6 +373,7 @@ struct rte_crypto_sym_op {

enum rte_crypto_sym_op_sess_type sess_type;

+   RTE_STD_C11
union {
struct rte_cryptodev_sym_session *session;
/**< Handle for the initialised session context */
diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
b/lib/librte_cryptodev/rte_cryptodev.h
index 957bdd7..cf28541 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -48,6 +48,7 @@ extern "C" {
 #include "rte_kvargs.h"
 #include "rte_crypto.h"
 #include "rte_dev.h"
+#include 

 #define CRYPTODEV_NAME_NULL_PMDcryptodev_null_pmd
 /**< Null crypto PMD device name */
@@ -104,6 +105,7 @@ extern const char **rte_cyptodev_names;
 struct rte_cryptodev_symmetric_capability {
enum rte_crypto_sym_xform_type xform_type;
/**< Transform type : Authentication / Cipher */
+   RTE_STD_C11
union {
struct {
enum rte_crypto_auth_algorithm algo;
@@ -177,6 +179,7 @@ struct rte_cryptodev_capabilities {
enum rte_crypto_op_type op;
/**< Operation type */

+   RTE_STD_C11
union {
struct rte_cryptodev_symmetric_capability sym;
/**< Symmetric operation capability parameters */
@@ -751,6 +754,7 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,

 /** Cryptodev symmetric crypto session */
 struct rte_cryptodev_sym_session {
+   RTE_STD_C11
struct {
uint8_t dev_id;
/**< Device Id */
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h 
b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index 3a3845c..cf08a50 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -52,6 +52,7 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 

 #include 

[dpdk-dev] [PATCH v4 04/10] lib: work around nonstandard bit-fields

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.

This commit prevents the following errors:

 error: type of bit-field `[...]' is a GCC extension

Note: the standard does not require implementations to issue a diagnostic
message with these, and such errors do not occur with recent GCC or clang
versions. However, GCC 4.7 is still common and using the extension keyword
is easier than checking compiler version.

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_cryptodev/rte_cryptodev.h  | 2 ++
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 1 +
 lib/librte_ether/rte_ethdev.h | 4 
 lib/librte_kni/rte_kni.h  | 1 +
 lib/librte_lpm/rte_lpm.h  | 4 
 lib/librte_mbuf/rte_mbuf.h| 1 +
 6 files changed, 13 insertions(+)

diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
b/lib/librte_cryptodev/rte_cryptodev.h
index 1e30a19..957bdd7 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -619,6 +619,7 @@ struct rte_cryptodev {
struct rte_cryptodev_cb_list link_intr_cbs;
/**< User application callback for interrupts if present */

+   __extension__
uint8_t attached : 1;
/**< Flag indicating the device is attached */
 } __rte_cache_aligned;
@@ -642,6 +643,7 @@ struct rte_cryptodev_data {
char name[RTE_CRYPTODEV_NAME_MAX_LEN];
/**< Unique identifier name */

+   __extension__
uint8_t dev_started : 1;
/**< Device state: STARTED(1)/STOPPED(0) */

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h 
b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 7f458a3..2ef0506 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -159,6 +159,7 @@ struct rte_kni_device_info {
uint16_t group_id;/**< Group ID */
uint32_t core_id; /**< core ID to bind for kernel thread */

+   __extension__
uint8_t force_bind : 1;   /**< Flag for kernel thread binding */

/* mbuf size */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4dac364..6059dd5 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -255,6 +255,7 @@ struct rte_eth_stats {
 /**
  * A structure used to retrieve link-level information of an Ethernet port.
  */
+__extension__
 struct rte_eth_link {
uint32_t link_speed;/**< ETH_SPEED_NUM_ */
uint16_t link_duplex  : 1;  /**< ETH_LINK_[HALF/FULL]_DUPLEX */
@@ -346,6 +347,7 @@ struct rte_eth_rxmode {
enum rte_eth_rx_mq_mode mq_mode;
uint32_t max_rx_pkt_len;  /**< Only used if jumbo_frame enabled. */
uint16_t split_hdr_size;  /**< hdr buf size (header_split enabled).*/
+   __extension__
uint16_t header_split : 1, /**< Header Split enable. */
hw_ip_checksum   : 1, /**< IP/UDP/TCP checksum offload enable. 
*/
hw_vlan_filter   : 1, /**< VLAN filter enable. */
@@ -645,6 +647,7 @@ struct rte_eth_txmode {

/* For i40e specifically */
uint16_t pvid;
+   __extension__
uint8_t hw_vlan_reject_tagged : 1,
/**< If set, reject sending out tagged pkts */
hw_vlan_reject_untagged : 1,
@@ -1691,6 +1694,7 @@ struct rte_eth_dev_data {
struct ether_addr* hash_mac_addrs;
/** Device Ethernet MAC addresses of hash filtering. */
uint8_t port_id;   /**< Device [external] port identifier. */
+   __extension__
uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0). */
scattered_rx : 1,  /**< RX of scattered packets is ON(1) / 
OFF(0) */
all_multicast : 1, /**< RX all multicast mode ON(1) / OFF(0). */
diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 7363e6c..5f6f9e4 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -88,6 +88,7 @@ struct rte_kni_conf {
struct rte_pci_addr addr;
struct rte_pci_id id;

+   __extension__
uint8_t force_bind : 1; /* Flag to bind kernel thread */
 };

diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h
index 79a4593..28668a3 100644
--- a/lib/librte_lpm/rte_lpm.h
+++ b/lib/librte_lpm/rte_lpm.h
@@ -93,6 +93,7 @@ extern "C" {

 #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 /** @internal Tbl24 entry structure. */
+__extension__
 struct rte_lpm_tbl_entry_v20 {
/**
 * Stores Next hop (tbl8 or tbl24 when valid_group is not set) or
@@ -116,6 +117,7 @@ struct rte_lpm_tbl_entry_v20 {
uint8_t depth   :6; /**< 

[dpdk-dev] [PATCH v4 03/10] lib: use C99 syntax for zero-size arrays

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.

The extension keyword is used whenever the C99 syntax cannot do it.

This commit prevents the following errors:

 error: ISO C forbids zero-size array `[...]'

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_acl/rte_acl.h  | 2 +-
 lib/librte_cryptodev/rte_cryptodev.h  | 2 +-
 lib/librte_cryptodev/rte_cryptodev_pmd.h  | 2 +-
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 2 +-
 lib/librte_hash/rte_fbk_hash.h| 2 +-
 lib/librte_ip_frag/rte_ip_frag.h  | 2 +-
 lib/librte_lpm/rte_lpm.h  | 2 +-
 lib/librte_mbuf/rte_mbuf.h| 3 +++
 lib/librte_pipeline/rte_pipeline.h| 2 +-
 lib/librte_ring/rte_ring.h| 2 +-
 lib/librte_sched/rte_bitmap.h | 2 +-
 11 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/lib/librte_acl/rte_acl.h b/lib/librte_acl/rte_acl.h
index 0979a09..c059dc3 100644
--- a/lib/librte_acl/rte_acl.h
+++ b/lib/librte_acl/rte_acl.h
@@ -144,7 +144,7 @@ struct rte_acl_rule_data {
struct rte_acl_field field[fld_num]; \
 }

-RTE_ACL_RULE_DEF(rte_acl_rule, 0);
+RTE_ACL_RULE_DEF(rte_acl_rule,);

 #defineRTE_ACL_RULE_SZ(fld_num)\
(sizeof(struct rte_acl_rule) + sizeof(struct rte_acl_field) * (fld_num))
diff --git a/lib/librte_cryptodev/rte_cryptodev.h 
b/lib/librte_cryptodev/rte_cryptodev.h
index affbdec..1e30a19 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -759,7 +759,7 @@ struct rte_cryptodev_sym_session {
} __rte_aligned(8);
/**< Public symmetric session details */

-   char _private[0];
+   char _private[];
/**< Private session material */
 };

diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h 
b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index 7d049ea..3a3845c 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -71,7 +71,7 @@ struct rte_cryptodev_session {
struct rte_mempool *mp;
} __rte_aligned(8);

-   char _private[0];
+   char _private[];
 };

 struct rte_cryptodev_driver;
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h 
b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 2acdfd9..7f458a3 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -102,7 +102,7 @@ struct rte_kni_fifo {
volatile unsigned read;  /**< Next position to be read */
unsigned len;/**< Circular buffer length */
unsigned elem_size;  /**< Pointer size - for 32/64 bit OS */
-   void * volatile buffer[0];   /**< The buffer contains mbuf pointers */
+   void *volatile buffer[]; /**< The buffer contains mbuf pointers */
 };

 /*
diff --git a/lib/librte_hash/rte_fbk_hash.h b/lib/librte_hash/rte_fbk_hash.h
index a430961..bd46048 100644
--- a/lib/librte_hash/rte_fbk_hash.h
+++ b/lib/librte_hash/rte_fbk_hash.h
@@ -115,7 +115,7 @@ struct rte_fbk_hash_table {
uint32_t init_val;  /**< For initialising hash function. */

/** A flat table of all buckets. */
-   union rte_fbk_hash_entry t[0];
+   union rte_fbk_hash_entry t[];
 };

 /**
diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
index 92cedf2..4c3faad 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -124,7 +124,7 @@ struct rte_ip_frag_tbl {
struct ip_frag_pkt *last; /**< last used entry. */
struct ip_pkt_list lru;   /**< LRU list for table entries. */
struct ip_frag_tbl_stat stat; /**< statistics counters. */
-   struct ip_frag_pkt pkt[0];/**< hash table. */
+   struct ip_frag_pkt pkt[]; /**< hash table. */
 };

 /** IPv6 fragment extension header */
diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h
index 2df1d67..79a4593 100644
--- a/lib/librte_lpm/rte_lpm.h
+++ b/lib/librte_lpm/rte_lpm.h
@@ -193,7 +193,7 @@ struct rte_lpm_v20 {
__rte_cache_aligned; /**< LPM tbl24 table. */
struct rte_lpm_tbl_entry_v20 tbl8[RTE_LPM_TBL8_NUM_ENTRIES]
__rte_cache_aligned; /**< LPM tbl8 table. */
-   struct rte_lpm_rule_v20 rules_tbl[0] \
+   struct rte_lpm_rule_v20 rules_tbl[]
__rte_cache_aligned; /**< LPM rules. */
 };

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 

[dpdk-dev] [PATCH v4 02/10] lib: work around large enum values

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.

This commit prevents the following errors:

 error: ISO C restricts enumerator values to range of `int'

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_eal/common/include/rte_memory.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_eal/common/include/rte_memory.h 
b/lib/librte_eal/common/include/rte_memory.h
index 0661109..6a2b3f1 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -54,6 +54,7 @@ extern "C" {

 #include 

+__extension__
 enum rte_page_sizes {
RTE_PGSIZE_4K= 1ULL << 12,
RTE_PGSIZE_64K   = 1ULL << 16,
-- 
2.1.4



[dpdk-dev] [PATCH v4 01/10] lib: work around braced-groups within expressions

2016-07-13 Thread Adrien Mazarguil
Exported header files used by applications should allow the strictest
compiler flags. Language extensions used in many places must be explicitly
marked or removed to avoid warnings and compilation failures.

This commit prevents the following errors:

 error: ISO C forbids braced-groups within expressions

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h | 3 ++-
 lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h | 3 ++-
 lib/librte_eal/common/include/arch/x86/rte_memcpy.h| 4 ++--
 lib/librte_eal/common/include/arch/x86/rte_vect.h  | 6 --
 lib/librte_eal/common/include/rte_common.h | 6 --
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h 
b/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h
index da6c233..c3a2619 100644
--- a/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h
+++ b/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h
@@ -148,7 +148,8 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
 }

 #define rte_memcpy(dst, src, n)  \
-   ({ (__builtin_constant_p(n)) ?   \
+   __extension__ ({ \
+   (__builtin_constant_p(n)) ?  \
memcpy((dst), (src), (n)) :  \
rte_memcpy_func((dst), (src), (n)); })

diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h 
b/lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h
index acf7aac..ca9d1dc 100644
--- a/lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h
@@ -95,7 +95,8 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
 }

 #define rte_memcpy(dst, src, n)  \
-   ({ (__builtin_constant_p(n)) ?   \
+   __extension__ ({ \
+   (__builtin_constant_p(n)) ?  \
memcpy((dst), (src), (n)) :  \
rte_memcpy_func((dst), (src), (n)); })

diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h 
b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
index 413035e..b3bfc23 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -594,7 +594,7 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
  * - __m128i  ~  must be pre-defined
  */
 #define MOVEUNALIGNED_LEFT47_IMM(dst, src, len, offset)
 \
-({ 
 \
+__extension__ ({   
 \
 int tmp;   
 \
 while (len >= 128 + 16 - offset) { 
 \
 xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset 
+ 0 * 16));  \
@@ -655,7 +655,7 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
  * - __m128i  ~  used in MOVEUNALIGNED_LEFT47_IMM must be 
pre-defined
  */
 #define MOVEUNALIGNED_LEFT47(dst, src, len, offset)   \
-({\
+__extension__ ({  \
 switch (offset) { \
 case 0x01: MOVEUNALIGNED_LEFT47_IMM(dst, src, n, 0x01); break;\
 case 0x02: MOVEUNALIGNED_LEFT47_IMM(dst, src, n, 0x02); break;\
diff --git a/lib/librte_eal/common/include/arch/x86/rte_vect.h 
b/lib/librte_eal/common/include/arch/x86/rte_vect.h
index b698797..2836f2c 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_vect.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_vect.h
@@ -106,7 +106,8 @@ typedef union rte_ymm {
 #endif /* __AVX__ */

 #ifdef RTE_ARCH_I686
-#define _mm_cvtsi128_si64(a) ({ \
+#define _mm_cvtsi128_si64(a)\
+__extension__ ({\
rte_xmm_t m;\
m.x = (a);  \
(m.u64[0]); \
@@ -117,7 +118,8 @@ typedef union rte_ymm {
  * Prior to version 12.1 icc doesn't support _mm_set_epi64x.
  */
 #if (defined(__ICC) && __ICC < 1210)
-#define _mm_set_epi64x(a, b)  ({ \
+#define _mm_set_epi64x(a, b) \
+__extension__ ({ \
rte_xmm_t m; \
m.u64[0] = b;\
m.u64[1] = a;\
diff --git a/lib/librte_eal/common/include/rte_common.h 
b/lib/librte_eal/common/include/rte_common.h
index 332f2a4..477472b 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -268,7 +268,8 @@ rte_align64pow2(uint64_t v)
 /**
  * Macro to return the minimum of two numbers
  */
-#define RTE_MIN(a, b) ({ \
+#define RTE_MIN(a, b) \
+   __extension__ ({ \
typeof (a) _a = (a); \

[dpdk-dev] [PATCH v4 00/10] Fix build errors related to exported headers

2016-07-13 Thread Adrien Mazarguil
DPDK uses GNU C language extensions in most of its code base. This is fine
for internal source files whose compilation flags are controlled by DPDK,
however user applications that use exported "public" headers may experience
compilation failures when enabling strict error/standard checks (-std and
-pedantic for instance).

Exported headers are installed system-wide and must be as clean as possible
so applications do not have to resort to workarounds.

This patchset affects exported headers only, compilation problems are
addressed as follows:

- Adding the __extension__ keyword to nonstandard constructs (same method
  as existing libraries when there is no other choice).
- Adding the __extension__ keyword to C11 constructs to remain compatible
  with pure C99.
- Adding missing includes so exported files can be included out of order
  and on their own.
- Fixing GNU printf-like variadic macros as there is no magic keyword for
  these.

Changes in v4:

- Dropped "lib: work around structs with no members" patch, now addressed as
  a separate issue outside of this patchset by "mempool: fix empty structure
  definition".
- Fixed remaining compilation error with ICC reported by Ferruh. Finally
  settled on using the __extension__ keyword directly in struct
  rte_pipeline_table_entry as converting it to a standard flexible array may
  break existing programs.

Changes in v3:

- Fixed compilation issue on ARM and POWER8 due to missing parenthesis.
- Added bit-field fix for rte_kni.h.

Changes in v2:

- Rebased on top of the current HEAD.
- Added script to check headers automatically (check-includes.sh), for both
  C and C++ compilation.
- Updated test-build.sh to use it.
- Fixed consistency of new #include directives, now inside extern "C"
  blocks for files that already do that (Jan, fixing these was too much
  work for this patchset so I settled on this solution in the meantime).
- Updated headlines to address check-git-log.sh complaints.

Adrien Mazarguil (10):
  lib: work around braced-groups within expressions
  lib: work around large enum values
  lib: use C99 syntax for zero-size arrays
  lib: work around nonstandard bit-fields
  lib: work around unnamed structs/unions
  lib: add missing include dependencies
  lib: work around forward reference to enum types
  lib: remove named variadic macros in exported headers
  lib: hide static functions never defined
  scripts: check compilation of exported header files

 MAINTAINERS |   1 +
 lib/librte_acl/rte_acl.h|   2 +-
 lib/librte_cfgfile/rte_cfgfile.h|   2 +
 lib/librte_cmdline/cmdline.h|   1 +
 lib/librte_cmdline/cmdline_parse_portlist.h |   1 +
 lib/librte_cmdline/cmdline_socket.h |   3 +
 lib/librte_cryptodev/rte_crypto.h   |   2 +
 lib/librte_cryptodev/rte_crypto_sym.h   |   3 +
 lib/librte_cryptodev/rte_cryptodev.h|  40 ++-
 lib/librte_cryptodev/rte_cryptodev_pmd.h|   6 +-
 .../common/include/arch/arm/rte_byteorder.h |   2 +
 .../common/include/arch/arm/rte_memcpy_32.h |   3 +-
 .../common/include/arch/arm/rte_prefetch_32.h   |   1 +
 .../common/include/arch/arm/rte_prefetch_64.h   |   1 +
 .../common/include/arch/arm/rte_vect.h  |   1 +
 .../common/include/arch/ppc_64/rte_atomic.h |   1 +
 .../common/include/arch/ppc_64/rte_byteorder.h  |   1 +
 .../common/include/arch/ppc_64/rte_cycles.h |   2 +
 .../common/include/arch/ppc_64/rte_memcpy.h |   3 +-
 .../common/include/arch/ppc_64/rte_prefetch.h   |   1 +
 .../common/include/arch/x86/rte_atomic.h|   2 +
 .../common/include/arch/x86/rte_atomic_32.h |   9 +
 .../common/include/arch/x86/rte_atomic_64.h |   8 +
 .../common/include/arch/x86/rte_byteorder.h |   2 +
 .../common/include/arch/x86/rte_byteorder_32.h  |   7 +
 .../common/include/arch/x86/rte_byteorder_64.h  |   7 +
 .../common/include/arch/x86/rte_cycles.h|   2 +
 .../common/include/arch/x86/rte_memcpy.h|   4 +-
 .../common/include/arch/x86/rte_prefetch.h  |   1 +
 .../common/include/arch/x86/rte_rtm.h   |   1 +
 .../common/include/arch/x86/rte_vect.h  |   8 +-
 .../common/include/generic/rte_atomic.h |   1 +
 .../common/include/generic/rte_byteorder.h  |   2 +
 .../common/include/generic/rte_cpuflags.h   |   3 +
 .../common/include/generic/rte_memcpy.h |   4 +
 lib/librte_eal/common/include/rte_common.h  |  22 +-
 lib/librte_eal/common/include/rte_devargs.h |   1 +
 lib/librte_eal/common/include/rte_eal.h |   1 +
 lib/librte_eal/common/include/rte_interrupts.h  |   2 +
 lib/librte_eal/common/include/rte_memory.h  |   4 +
 lib/librte_eal/common/include/rte_memzone.h |   2 +
 lib/librte_eal/common/include/rte_time.h|   8 +
 lib/librte_eal/common/include/rte_version.h |   1 +
 .../eal/include/exec-env/rte_interrupts.h   |   1 +
 

[dpdk-dev] [PATCH] net/i40e: revert VLAN filtering fix

2016-07-13 Thread Chen, Jing D
Hi,

> -Original Message-
> From: Wu, Jingjing
> Sent: Wednesday, July 13, 2016 6:28 PM
> To: Richardson, Bruce 
> Cc: dev at dpdk.org; Wu, Jingjing ; Shaw, Jeffrey B
> ; Zhang, Helin ; Chen,
> Jing D ; Ananyev, Konstantin
> 
> Subject: [PATCH] net/i40e: revert VLAN filtering fix
> 
> This reverts commit 4761f57d58c6f52543738dbe299f846d62d75895.
> Introducing VLAN table by adding VLAN adminq command will cause NIC's
> throughput drop obviously. It's a hardware issue.
> With this revert, VLAN filtering can only work when promiscuous mode is
> disabled.
> 
> Reverts: 4761f57d58c6 ("net/i40e: fix VLAN filtering in promiscuous mode")
> 
> Reported-by: Jeffrey Shaw 
> Signed-off-by: Jingjing Wu 
Acked-by : Jing Chen 


[dpdk-dev] [PATCH] mempool: fix empty structure definition

2016-07-13 Thread Adrien Mazarguil
This commit addresses the following warning reported by clang, which
happens by default, as long as CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG is disabled:

 warning: empty struct has size 0 in C, size 1 in C++

C and C++ must use the same size for objects to avoid corruption during run
time.

Fixes: 97e7e685bfcd ("mempool: add structure for object trailers")

Signed-off-by: Adrien Mazarguil 
---
 lib/librte_mempool/rte_mempool.c | 4 
 lib/librte_mempool/rte_mempool.h | 6 --
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 6ec0906..8806633 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -199,7 +199,11 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t 
flags,
sz->header_size = RTE_ALIGN_CEIL(sz->header_size,
RTE_MEMPOOL_ALIGN);

+#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
sz->trailer_size = sizeof(struct rte_mempool_objtlr);
+#else
+   sz->trailer_size = 0;
+#endif

/* element size is 8 bytes-aligned at least */
sz->elt_size = RTE_ALIGN_CEIL(elt_size, sizeof(uint64_t));
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index fb7052e..4a8fbb1 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -163,6 +163,8 @@ struct rte_mempool_objhdr {
  */
 STAILQ_HEAD(rte_mempool_objhdr_list, rte_mempool_objhdr);

+#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
+
 /**
  * Mempool object trailer structure
  *
@@ -170,11 +172,11 @@ STAILQ_HEAD(rte_mempool_objhdr_list, rte_mempool_objhdr);
  * trailer structure containing a cookie preventing memory corruptions.
  */
 struct rte_mempool_objtlr {
-#ifdef RTE_LIBRTE_MEMPOOL_DEBUG
uint64_t cookie; /**< Debug cookie. */
-#endif
 };

+#endif
+
 /**
  * A list of memory where objects are stored
  */
-- 
2.1.4



[dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location

2016-07-13 Thread Ananyev, Konstantin

Hi Juhamatti,

> 
> Hello,
> 
> > > Hi Juhamatti,
> > >
> > > >
> > > > Hello,
> > > >
> > > > > > > > -Original Message-
> > > > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of
> > > > > > > > Juhamatti Kuusisaari
> > > > > > > > Sent: Monday, July 11, 2016 11:21 AM
> > > > > > > > To: dev at dpdk.org
> > > > > > > > Subject: [dpdk-dev] [PATCH] lib: move rte_ring read barrier
> > > > > > > > to correct location
> > > > > > > >
> > > > > > > > Fix the location of the rte_ring data dependency read barrier.
> > > > > > > > It needs to be called before accessing indexed data to
> > > > > > > > ensure that the data itself is guaranteed to be correctly 
> > > > > > > > updated.
> > > > > > > >
> > > > > > > > See more details at kernel/Documentation/memory-barriers.txt
> > > > > > > > section 'Data dependency barriers'.
> > > > > > >
> > > > > > >
> > > > > > > Any explanation why?
> > > > > > > From my point smp_rmb()s are on the proper places here :)
> > > > > > > Konstantin
> > > > > >
> > > > > > The problem here is that on a weak memory model system the CPU
> > > > > > is allowed to load the address data out-of-order in advance.
> > > > > > If the read barrier is after the DEQUEUE, you might end up
> > > > > > having the old data there on a race situation when the buffer is
> > continuously full.
> > > > > > Having it before the DEQUEUE guarantees that the load is not
> > > > > > done in advance.
> > > > >
> > > > > Sorry, still didn't see any race condition in the current code.
> > > > > Can you provide any particular example?
> > > > > From other side, moving smp_rmb() before dequeueing the objects,
> > > > > could introduce a race condition, on cpus where later writes can
> > > > > be reordered with earlier reads.
> > > >
> > > > Here is a simplified example sequence from time perspective:
> > > > 1. Consumer CPU (CCPU) loads value y from r->ring[x] out-of-order
> > > > (the key of the problem)
> > >
> > > To read the value of ring[x] cpu has to calculate x first.
> > > And to calculate x it needs to read cons.head and prod.tail first.
> > > Are you saying that some modern cpu can:
> > >  -'speculate' value of cons.head and  prod.tail
> > >   (based on what?)
> > >  -calculate x based on these speculated values.
> > > - read ring[x]
> > > - read cons.head and prod.tail
> > > - if read values are not equal to speculated ones , then
> > >   re-caluclate x and re-read ring[x]
> > > - else use speculatively read ring[x]
> > > ?
> > > If such thing is possible  (is it really? and if yes on which cpu?),
> >
> > As I can see, neither ARM or PPC support  such things.
> > Both of them do obey address dependency.
> > (ARM & PPC guys feel free to correct me here, if I am wrong here).
> > So what cpu we are talking about?
> 
> I checked that too, indeed the problem I described seems to be more academic
> than even theoretical and does not apply to current CPUs. So I agree here and
> this makes this patch unneeded, I'll withdraw it. However, the implementation
> may still have another issue, see below.
> 
> > > then yes, we might need an extra  smp_rmb() before DEQUEUE_PTRS() for
> > > __rte_ring_sc_do_dequeue().
> > > For __rte_ring_mc_do_dequeue(), I think we are ok, as there is CAS
> > > just before DEQUEUE_PTRS().
> > >
> > > > 2. Producer CPU (PCPU) updates r->ring[x] to value be z 3. PCPU
> > > > updates prod_tail to be x 4. CCPU updates cons_head to be x 5. CCPU
> > > > loads r->ring[x] by using out-of-order loaded value y [is z in
> > > > reality]
> > > >
> > > > The problem here is that on weak memory model, the CCPU is allowed
> > > > to load
> > > > r->ring[x] value in advance, if it decides to do so (CCPU needs to
> > > > r->be able to see
> > > > in advance that x will be an interesting index worth loading). The
> > > > index value x is updated atomically,  but it does not matter here.
> > > > Also, the write barrier on PCPU side guarantees that CCPU cannot see
> > > > update of x before PCPU has really updated the r->ring[x] to z and
> > > > moved the tail, but still allows to do the out-of-order loads without
> > proper read barrier.
> > > >
> > > > When the read barrier is moved between steps 4 and 5, it disallows
> > > > to use any out-of-order loads so far and forces to drop r->ring[x] y
> > > > value and load current value z.
> > > >
> > > > The ring queue appears to work well as this is a rare corner case.
> > > > Due to the head,tail-structure the problem needs queue to be full
> > > > and also CCPU needs to see r->ring[x] update later than it does the
> > > > out-of-order load. In addition, the HW needs to be able to predict
> > > > and choose the load to the future index (which should be quite
> > > > possible, considering modern CPUs). If you have seen in the past
> > > > problems and noticed that a larger ring queue works better as a
> > workaround, you may have encountered the problem already.
> > >
> > > I don't understand what means 'larger rings works better' 

[dpdk-dev] [PATCH] virtio: fix missing curly braces

2016-07-13 Thread Maxime Coquelin
Hi Jan,

On 07/13/2016 11:24 AM, Jan Viktorin wrote:
> GCC 6 is complaining and seems to be correct here.
>
> virtio_user_ethdev.c:345:2: error:
>   this ?if? clause does not guard... [-Werror=misleading-indentation]
>if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
>^~
>
> virtio_user_ethdev.c:348:3: note:
>   ...this statement, but the latter is misleadingly indented
>   as if it is guarded by the ?if?
> if (ret < 0) {
>
> Fixes: 404bd6bfe360 ("net/virtio-user: fix return value not checked")
> Signed-off-by: Jan Viktorin 
> ---
>
I already fixed it yesterday:
http://dpdk.org/dev/patchwork/patch/14780/

Thanks,
Maxime


[dpdk-dev] [PATCH] virtio: fix missing curly braces

2016-07-13 Thread Jan Viktorin
GCC 6 is complaining and seems to be correct here.

virtio_user_ethdev.c:345:2: error:
this ?if? clause does not guard... [-Werror=misleading-indentation]
  if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
  ^~

virtio_user_ethdev.c:348:3: note:
...this statement, but the latter is misleadingly indented
as if it is guarded by the ?if?
   if (ret < 0) {

Fixes: 404bd6bfe360 ("net/virtio-user: fix return value not checked")
Signed-off-by: Jan Viktorin 
---
 drivers/net/virtio/virtio_user_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c 
b/drivers/net/virtio/virtio_user_ethdev.c
index 17d5848..1b903f3 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -342,7 +342,7 @@ virtio_user_pmd_devinit(const char *name, const char 
*params)
goto end;
}

-   if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
+   if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1) {
ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH,
 _string_arg, );
if (ret < 0) {
@@ -350,7 +350,7 @@ virtio_user_pmd_devinit(const char *name, const char 
*params)
 VIRTIO_USER_ARG_PATH);
goto end;
}
-   else {
+   } else {
PMD_INIT_LOG(ERR, "arg %s is mandatory for virtio-user\n",
  VIRTIO_USER_ARG_QUEUE_SIZE);
goto end;
-- 
2.9.0



[dpdk-dev] [PATCH v6 05/17] eal: introduce init macros

2016-07-13 Thread Jan Viktorin
Hello Shreyansh,

On Tue, 12 Jul 2016 11:31:10 +0530
Shreyansh Jain  wrote:

> Introduce a RTE_INIT macro used to mark an init function as a constructor.
> Current eal macros have been converted to use this (no functional impact).
> DRIVER_REGISTER_PCI is added as a helper for pci drivers.
> 
> Suggested-by: Jan Viktorin 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---

[...]

> +#define RTE_INIT(func) \
> +static void __attribute__((constructor, used)) func(void)
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_eal/common/include/rte_pci.h 
> b/lib/librte_eal/common/include/rte_pci.h
> index fa74962..3027adf 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f);
>   */
>  void rte_eal_pci_register(struct rte_pci_driver *driver);
>  
> +/** Helper for PCI device registeration from driver (eth, crypto) instance */
> +#define DRIVER_REGISTER_PCI(nm, drv) \
> +RTE_INIT(pciinitfn_ ##nm); \
> +static void pciinitfn_ ##nm(void) \
> +{ \

You are missing setting the name here like PMD_REGISTER_DRIVER does
now. Or should I include it in my patch set?

(drv).name = RTE_STR(nm);

> + rte_eal_pci_register(_drv); \
> +}
> +
>  /**
>   * Unregister a PCI driver.
>   *
> diff --git a/lib/librte_eal/common/include/rte_tailq.h 
> b/lib/librte_eal/common/include/rte_tailq.h
> index 4a686e6..71ed3bb 100644
> --- a/lib/librte_eal/common/include/rte_tailq.h
> +++ b/lib/librte_eal/common/include/rte_tailq.h
> @@ -148,8 +148,8 @@ struct rte_tailq_head *rte_eal_tailq_lookup(const char 
> *name);
>  int rte_eal_tailq_register(struct rte_tailq_elem *t);
>  
>  #define EAL_REGISTER_TAILQ(t) \
> -void tailqinitfn_ ##t(void); \
> -void __attribute__((constructor, used)) tailqinitfn_ ##t(void) \
> +RTE_INIT(tailqinitfn_ ##t); \
> +static void tailqinitfn_ ##t(void) \
>  { \
>   if (rte_eal_tailq_register() < 0) \
>   rte_panic("Cannot initialize tailq: %s\n", t.name); \



-- 
  Jan ViktorinE-mail: Viktorin at RehiveTech.com
  System ArchitectWeb:www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic


[dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address.

2016-07-13 Thread Ilya Maximets
On 12.07.2016 08:53, Ilya Maximets wrote:
> On 12.07.2016 05:43, Yuanhan Liu wrote:
>> On Mon, Jul 11, 2016 at 02:47:56PM +0300, Ilya Maximets wrote:
>>> On 11.07.2016 14:05, Yuanhan Liu wrote:
 On Mon, Jul 11, 2016 at 12:50:24PM +0300, Ilya Maximets wrote:
> On 11.07.2016 11:38, Yuanhan Liu wrote:
>> On Sun, Jul 10, 2016 at 09:17:31PM +0800, Yuanhan Liu wrote:
>>> On Fri, Jul 08, 2016 at 02:48:56PM +0300, Ilya Maximets wrote:

 Another point is that crash constantly happens on queue_id=3 (second 
 RX queue) in
 my scenario. It is newly allocated virtqueue while reconfiguration 
 from rxq=1 to
 rxq=2.
>>>
>>> That's a valuable message: what's your DPDK HEAD commit while triggering
>>> this issue?
>
> fbfd99551ca3 ("mbuf: add raw allocation function")
>
>>
>> I guess I have understood what goes wrong in you case.
>>
>> I would guess that your vhost has 2 queues (here I mean queue-pairs,
>> including one Tx and Rx queue; below usage is the same) configured,
>> so does to your QEMU. However, you just enabled 1 queue while starting
>> testpmd inside the guest, and you want to enable 2 queues by running
>> following testpmd commands:
>>
>> stop
>> port stop all
>> port config all rxq 2
>> port config all txq 2
>> port start all
>>
>> Badly, that won't work for current virtio PMD implementation, and what's
>> worse, it triggers a vhost crash, the one you saw.
>>
>> Here is how it comes. Since you just enabled 1 queue while starting
>> testpmd, it will setup 1 queue only, meaning only one queue's **valid**
>> information will be sent to vhost. You might see SET_VRING_ADDR
>> (and related vhost messages) for the other queue as well, but they
>> are just the dummy messages: they don't include any valid/real
>> information about the 2nd queue: the driver don't setup it after all.
>>
>> So far, so good. It became broken when you run above commands. Those
>> commands do setup for the 2nd queue, however, they failed to trigger
>> the QEMU virtio device to start the vhost-user negotiation, meaning
>> no SET_VRING_ADDR will be sent for the 2nd queue, leaving vhost
>> untold and not updated.
>>
>> What's worse, above commands trigger the QEMU to send SET_VRING_ENABLE
>> messages, to enable all the vrings. And since the vrings for the 2nd
>> queue are not properly configured, the crash happens.
>
> Hmm, why 2nd queue works properly with my fix to vhost in this case?

 Hmm, really? You are sure that data flows in your 2nd queue after those
 commands? From what I know is that your patch just avoid a crash, but
 does not fix it.
>>>
>>> Oh, sorry. Yes, it doesn't work. With my patch applied I have a QEMU hang.
>>
>> The crash actually could be avoided by commit 0823c1cb0a73 ("vhost:
>> workaround stale vring base"), accidentally. That's why I asked you
>> above the HEAD commit you were using.
> 
> Thanks for pointing this. I'll check it.

I've checked my DPDK HEAD with above commit backported. Yes, it helps to
avoid vhost crash in my scenario. As expected, after reconfiguration new
virtqueue doesn't work, QEMU hangs sometimes.

>> So maybe we should do virtio reset on port start?
>
> I guess it was removed by this patch:
> a85786dc816f ("virtio: fix states handling during initialization").

 Seems yes.
>>
>> Actually, we should not do that: do reset on port start. The right fix
>> should be allocating MAX queues virtio device supports (2 here). This
>> would allow us changing the queue number dynamically.
> 
> Yes, I agree that this is the right way to fix this issue.
>  
>> But this doesn't sound a simple fix; it involves many code changes, due
>> to it was not designed this way before. Therefore, we will not fix it
>> in this release, due to it's too late. Let's fix it in the next release
>> instead. For the crash issue, it will not happen with the latest HEAD.
>> Though it's accident fix, I think we are fine here.

This scenario fixed somehow, I agree. But this patch still needed to protect
vhost from untrusted VM, from malicious or buggy virtio application.
Maybe we could change the commit-message and resend this patch as a
security enhancement? What do you think?


[dpdk-dev] DPDK on Xen maintenance

2016-07-13 Thread Jan Blunck
On Di, 2016-07-12 at 11:34 +0200, Thomas Monjalon wrote:
> Hi all,
> 
> We are facing some issues with Xen dom0.
> Some were fixed in RC2:
>   https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_ml
> _archives_dev_2016-
> 2DJuly_043760.html=CwICAg=IL_XqQWOjubgfqINi2jTzg=eGq7Pg5OSP440n
> QUcjb02I48YXd7Ce6OiSj9BMaGZiE=98n7rM-Tjze4nm2MUpU8Etvo5lAB-
> oe0KXsst27ujkw=AHzBPDmKhqbsIc8pb2x_s7ShJvDhf93AsXRAm43EXlM=?
> and there still have some other issues.
> 
> It seems Xen is becoming less attractive:
>   - we do not have a lot of test reports or feedbacks
>   - there is no maintainer for DPDK on Xen
>   - we are still waiting for the Xen netfront PMD
> 

Although progress is slow I'm still working on upstreaming the Xen
netfront PMD. I will continue to maintain it afterwards of course.


> I wonder wether it still makes sense to maintain this code or not?
> In case of positive reply, it would be nice to have the name of
> someone responsible for this code, i.e. a maintainer.
> 
> Thanks


[dpdk-dev] Valgrind and DPDK

2016-07-13 Thread Eelco Chaudron
Hi All,

Has some one successfully ran DPDK 16.04 with Valgrind (latest SVN copy)? 
I?m still getting issues even with running it without the RDRAND CPU flag.

Thanks,

Eelco




[dpdk-dev] [PATCH] net/enic: fix calculation of truncated packets

2016-07-13 Thread Nelson Escobar
The calculation of truncated packets didn't take into account packet
errors due to the adapter not having buffers, causing both the
ipackets, and imissed counts to be wrong if such errors occurred.  In
order to properly calculate the number of packets truncated, we need
to subtract the count of errors due to no buffers.

Fixes: c44d9f01adf3 ("net/enic: count truncated packets")

Signed-off-by: Nelson Escobar 
---
 drivers/net/enic/enic_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index d8669cc..9ec2a2d 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -172,7 +172,8 @@ void enic_dev_stats_get(struct enic *enic, struct 
rte_eth_stats *r_stats)
 * which can make ibytes be slightly higher than it should be.
 */
rx_packet_errors = rte_atomic64_read(_stats->rx_packet_errors);
-   rx_truncated = rx_packet_errors - stats->rx.rx_errors;
+   rx_truncated = rx_packet_errors - stats->rx.rx_errors -
+   stats->rx.rx_no_bufs;

r_stats->ipackets = stats->rx.rx_frames_ok - rx_truncated;
r_stats->opackets = stats->tx.tx_frames_ok;
-- 
2.7.0



[dpdk-dev] Compiling DPDK is not working on Red Hat 6.7

2016-07-13 Thread Thomas Monjalon
2016-07-13 06:01, Raslan Darawsheh:
> It seems that the patch fixed the issue.

Thanks, I've sent the patches:
http://dpdk.org/ml/archives/dev/2016-July/043917.html

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] 
> 2016-07-12 11:35, Raslan Darawsheh:
> > I think the option is there as you see:
> > 
> [...]
> > -Wl,--as-needed  -Wl,-lrt -Wl,-lm |...] -Wl,-lrte_eal
> [...]
> > eal_timer.c:(.text+0x152): undefined reference to `clock_gettime'
> 
> I suspect we need -lrt after -lrte_eal.

Do you know where the flag -lrt is set?
Is it in your environment LDFLAGS? (it is my guess for the fix).


[dpdk-dev] [PATCH 1/2] mk: clean up application linker flags

2016-07-13 Thread Thomas Monjalon
Make some cleaning before fixing the link dependency ordering
in the next commit.

- Move flags for creating a map file in the variable MAPFLAGS.
- Make only one call to linkerprefix macro.
- Group linker flags on the same line.

Signed-off-by: Thomas Monjalon 
---
 mk/rte.app.mk | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index ea22961..f9acb74 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -176,6 +176,8 @@ ifeq ($(RTE_DEVEL_BUILD)$(CONFIG_RTE_BUILD_SHARED_LIB),yy)
 LDFLAGS += -rpath=$(RTE_SDK_BIN)/lib
 endif

+MAPFLAGS = -Map=$@.map --cref
+
 .PHONY: all
 all: install

@@ -190,13 +192,13 @@ build: _postbuild
 exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1

 ifeq ($(LINK_USING_CC),1)
-override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
-O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
-   -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call 
linkerprefix,$(LDFLAGS)) \
-   $(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
+O_TO_EXE = $(CC) -o $@ $(CFLAGS) $(OBJS-y) $(call linkerprefix, \
+   $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) $(LDLIBS) \
+   $(MAPFLAGS))
 else
-O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-   -Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
+O_TO_EXE = $(LD) -o $@ $(OBJS-y) \
+   $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) $(LDLIBS) \
+   $(MAPFLAGS)
 endif
 O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
 O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
-- 
2.7.0



[dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address.

2016-07-13 Thread Rich Lane
On Wednesday, July 13, 2016, Yuanhan Liu 
wrote:

> On Wed, Jul 13, 2016 at 10:34:07AM +0300, Ilya Maximets wrote:
> > This scenario fixed somehow, I agree. But this patch still needed to
> protect
> > vhost from untrusted VM, from malicious or buggy virtio application.
> > Maybe we could change the commit-message and resend this patch as a
> > security enhancement? What do you think?
>
> Indeed, but I'm a bit concerned about the performance regression found
> by Rich, yet I am not quite sure why it happens, though Rich claimed
> that it seems to be a problem related to compiler.


The workaround I suggested solves the performance regression. But even if
it hadn't, this is a security fix that should be merged regardless of the
performance impact.


[dpdk-dev] Compiling DPDK is not working on Red Hat 6.7

2016-07-13 Thread Raslan Darawsheh
It seems that the patch fixed the issue. 

Kindest regards 
Raslan Darawsheh


-Original Message-
From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] 
Sent: Tuesday, July 12, 2016 3:16 PM
To: Raslan Darawsheh
Cc: dev at dpdk.org; Christian Ehrhardt
Subject: Re: [dpdk-dev] Compiling DPDK is not working on Red Hat 6.7

Hi,

2016-07-12 11:35, Raslan Darawsheh:
> I think the option is there as you see:
> 
[...]
> -Wl,--as-needed  -Wl,-lrt -Wl,-lm |...] -Wl,-lrte_eal
[...]
> eal_timer.c:(.text+0x152): undefined reference to `clock_gettime'

I suspect we need -lrt after -lrte_eal.

Please could you try the following patch?


--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -176,6 +176,8 @@ ifeq ($(RTE_DEVEL_BUILD)$(CONFIG_RTE_BUILD_SHARED_LIB),yy)
 LDFLAGS += -rpath=$(RTE_SDK_BIN)/lib
 endif

+MAPFLAGS = -Map=$@.map --cref
+
 .PHONY: all
 all: install

@@ -190,15 +192,13 @@ build: _postbuild
 exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1

 ifeq ($(LINK_USING_CC),1)
-override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS)) -O_TO_EXE = 
$(CC) $(CFLAGS) \
-   $(call linkerprefix,$(LDLIBS)) \
-   $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-   -Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y)
+O_TO_EXE = $(CC) -o $@ $(CFLAGS) $(OBJS-y) $(call linkerprefix, \
+   $(LDLIBS) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
+   $(MAPFLAGS))
 else
-O_TO_EXE = $(LD) $(LDLIBS) \
-   $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-   -Map=$(@).map --cref -o $@ $(OBJS-y)
+O_TO_EXE = $(LD) -o $@ $(OBJS-y)
+   $(LDLIBS) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
+   $(MAPFLAGS)
 endif
 O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight  
O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")



[dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location

2016-07-13 Thread Kuusisaari, Juhamatti

Hello,

> > Hi Juhamatti,
> >
> > >
> > > Hello,
> > >
> > > > > > > -Original Message-
> > > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of
> > > > > > > Juhamatti Kuusisaari
> > > > > > > Sent: Monday, July 11, 2016 11:21 AM
> > > > > > > To: dev at dpdk.org
> > > > > > > Subject: [dpdk-dev] [PATCH] lib: move rte_ring read barrier
> > > > > > > to correct location
> > > > > > >
> > > > > > > Fix the location of the rte_ring data dependency read barrier.
> > > > > > > It needs to be called before accessing indexed data to
> > > > > > > ensure that the data itself is guaranteed to be correctly updated.
> > > > > > >
> > > > > > > See more details at kernel/Documentation/memory-barriers.txt
> > > > > > > section 'Data dependency barriers'.
> > > > > >
> > > > > >
> > > > > > Any explanation why?
> > > > > > From my point smp_rmb()s are on the proper places here :)
> > > > > > Konstantin
> > > > >
> > > > > The problem here is that on a weak memory model system the CPU
> > > > > is allowed to load the address data out-of-order in advance.
> > > > > If the read barrier is after the DEQUEUE, you might end up
> > > > > having the old data there on a race situation when the buffer is
> continuously full.
> > > > > Having it before the DEQUEUE guarantees that the load is not
> > > > > done in advance.
> > > >
> > > > Sorry, still didn't see any race condition in the current code.
> > > > Can you provide any particular example?
> > > > From other side, moving smp_rmb() before dequeueing the objects,
> > > > could introduce a race condition, on cpus where later writes can
> > > > be reordered with earlier reads.
> > >
> > > Here is a simplified example sequence from time perspective:
> > > 1. Consumer CPU (CCPU) loads value y from r->ring[x] out-of-order
> > > (the key of the problem)
> >
> > To read the value of ring[x] cpu has to calculate x first.
> > And to calculate x it needs to read cons.head and prod.tail first.
> > Are you saying that some modern cpu can:
> >  -'speculate' value of cons.head and  prod.tail
> >   (based on what?)
> >  -calculate x based on these speculated values.
> > - read ring[x]
> > - read cons.head and prod.tail
> > - if read values are not equal to speculated ones , then
> >   re-caluclate x and re-read ring[x]
> > - else use speculatively read ring[x]
> > ?
> > If such thing is possible  (is it really? and if yes on which cpu?),
> 
> As I can see, neither ARM or PPC support  such things.
> Both of them do obey address dependency.
> (ARM & PPC guys feel free to correct me here, if I am wrong here).
> So what cpu we are talking about?

I checked that too, indeed the problem I described seems to be more academic 
than even theoretical and does not apply to current CPUs. So I agree here and 
this makes this patch unneeded, I'll withdraw it. However, the implementation 
may still have another issue, see below.

> > then yes, we might need an extra  smp_rmb() before DEQUEUE_PTRS() for
> > __rte_ring_sc_do_dequeue().
> > For __rte_ring_mc_do_dequeue(), I think we are ok, as there is CAS
> > just before DEQUEUE_PTRS().
> >
> > > 2. Producer CPU (PCPU) updates r->ring[x] to value be z 3. PCPU
> > > updates prod_tail to be x 4. CCPU updates cons_head to be x 5. CCPU
> > > loads r->ring[x] by using out-of-order loaded value y [is z in
> > > reality]
> > >
> > > The problem here is that on weak memory model, the CCPU is allowed
> > > to load
> > > r->ring[x] value in advance, if it decides to do so (CCPU needs to
> > > r->be able to see
> > > in advance that x will be an interesting index worth loading). The
> > > index value x is updated atomically,  but it does not matter here.
> > > Also, the write barrier on PCPU side guarantees that CCPU cannot see
> > > update of x before PCPU has really updated the r->ring[x] to z and
> > > moved the tail, but still allows to do the out-of-order loads without
> proper read barrier.
> > >
> > > When the read barrier is moved between steps 4 and 5, it disallows
> > > to use any out-of-order loads so far and forces to drop r->ring[x] y
> > > value and load current value z.
> > >
> > > The ring queue appears to work well as this is a rare corner case.
> > > Due to the head,tail-structure the problem needs queue to be full
> > > and also CCPU needs to see r->ring[x] update later than it does the
> > > out-of-order load. In addition, the HW needs to be able to predict
> > > and choose the load to the future index (which should be quite
> > > possible, considering modern CPUs). If you have seen in the past
> > > problems and noticed that a larger ring queue works better as a
> workaround, you may have encountered the problem already.
> >
> > I don't understand what means 'larger rings works better' here.
> > What we are talking about is  race condition, that if hit, would cause
> > data corruption and most likely a crash.

The larger ring queue length makes the problem more infrequent as the queue 
has more available free space and the 

[dpdk-dev] [PATCH] doc: fix mailing list address typo.

2016-07-13 Thread Jeff Shaw
The correct mailing list dev at dpdk.org, not dev at dpkg.org.

Signed-off-by: Jeff Shaw 
---
 doc/guides/contributing/patches.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 06af91d..16a21a5 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -22,7 +22,7 @@ The DPDK development process has the following features:
 * Patches are reviewed publicly on the mailing list.
 * Successfully reviewed patches are merged to the master branch of the 
repository.

-The mailing list for DPDK development is `dev at dpkg.org 
`_.
+The mailing list for DPDK development is `dev at dpdk.org 
`_.
 Contributors will need to `register for the mailing list 
`_ in order to submit patches.
 It is also worth registering for the DPDK `Patchwork 
`_

-- 
2.5.0