[dpdk-dev] [dpdk-announce] Reminder: DPDK North America CFP closes this week

2018-10-23 Thread Trishan de Lanerolle
With the community in the midst of 18.11 release cycle, we have
extended the CFP for DPDK North America until the end of the week,
*Friday, October 26th.*

If you’re planning to submit a proposal you have less than 5 days to
do so. You can submit proposals at:
https://events.linuxfoundation.org/events/dpdknorthamerica2018/dpdk-na-program/cfp/

If you’re attending, you can also register at:
https://events.linuxfoundation.org/events/dpdknorthamerica2018/dpdkattend/register/

We encourage all those interested to submit speaking proposals, and
hope to see most of the community onsite. If you have any questions,
please contact our events team at eve...@dpdk.org


Re: [dpdk-dev] [PATCH] net/i40e: cancel alarm handler at the end of device closure

2018-10-23 Thread Xing, Beilei



> -Original Message-
> From: Ye, Xiaolong
> Sent: Tuesday, October 23, 2018 8:54 AM
> To: Zhang, Qi Z ; Xing, Beilei 
> Cc: dev@dpdk.org; Ye, Xiaolong ; sta...@dpdk.org
> Subject: [PATCH] net/i40e: cancel alarm handler at the end of device closure
> 
> Some operations in i40evf_dev_close like i40evf_dev_promiscuous_disable
> still need alarm handler to clear the pending cmd, if alarm handler is
> canceled in early stage of i40evf_dev_close, i40evf_dev_promiscuous_disable
> will result in failure.
> 
> Fixes: 864a800d706d ("net/i40e: remove VF interrupt handler")
> 
> Cc: sta...@dpdk.org
> Signed-off-by: Xiaolong Ye 
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index baa6cc58f..52ae578d6 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -2249,7 +2249,6 @@ i40evf_dev_close(struct rte_eth_dev *dev)  {
>   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> 
> - rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
>   i40evf_dev_stop(dev);
>   i40e_dev_free_queues(dev);
>   /*
> @@ -2263,6 +2262,7 @@ i40evf_dev_close(struct rte_eth_dev *dev)
>   i40evf_reset_vf(hw);
>   i40e_shutdown_adminq(hw);
>   i40evf_disable_irq0(hw);
> + rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
>  }
> 
>  /*
> --
> 2.17.1

Acked-by: Beilei Xing 



Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508

2018-10-23 Thread Shreyansh Jain
Besides the comment I sent before about 'Fixes' before sign-off, a 
single trivial comment inline ...

On Tuesday 23 October 2018 07:20 AM, Rosen Xu wrote:
> This patch fixes rte_eal_hotplug_add without checking return value issue
> 
> Signed-off-by: Rosen Xu 
> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
> Cc: rosen...@intel.com
> ---
>   drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c 
> b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
> index 3fed057..32e318f 100644
> --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
> @@ -542,6 +542,7 @@
>   int port;
>   char *name = NULL;
>   char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> + int ret = -1;
>   
>   devargs = dev->device.devargs;
>   
> @@ -583,7 +584,7 @@
>   snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
>   port, name);
>   
> - rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
> + ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>   dev_name, devargs->args);

Ideally, the function argument spreading on next line should start 
underneath the previous arguments - something like:

ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
  dev_name, devargs->args);

But, in this file this is not being done at multiple places (for 
example, the snprintf in this code snippet). So, either you can ignore 
this comment, or fix it for just this change.

>   end:
>   if (kvlist)
> @@ -591,7 +592,7 @@
>   if (name)
>   free(name);
>   
> - return 0;
> + return ret;
>   }
>   
>   static int
> 

Otherwise, the patch is simple enough.

Acked-by: Shreyansh Jain 


Re: [dpdk-dev] [PATCH v3 4/4] ethdev: support MAC address as iterator filter

2018-10-23 Thread Thomas Monjalon
22/10/2018 23:24, Ananyev, Konstantin:
> From: Thomas Monjalon
> > 22/10/2018 15:37, Andrew Rybchenko:
> > > On 10/22/18 4:15 PM, Thomas Monjalon wrote:
> > > > The MAC addresses of a port can be matched with devargs.
> > > >
> > > > As the conflict between rte_ether.h and netinet/ether.h is not resolved,
> > > > the MAC parsing is done with a rte_cmdline function.
> > > > As a result, cmdline library becomes a dependency of ethdev.
> > > >
> > > > Signed-off-by: Thomas Monjalon 
> > >
> > > I'd like to share my thought about a new dependency.
> > > Looking at cmdline I think that it is a bad and strange
> > > dependency for kvargs. IMHO, even duplication of the
> > > code to parse MAC address it less evil in this case.
> > 
> > cmdline is not a dependency for kvargs.
> > I have added it as a dependency for ethdev.
> > 
> > > May be it is possible to provide internal wrapper
> > > which is implemented using ether_aton_r() and located
> > > in a separate C file which does not include rte_ether.h etc?
> > 
> > I raised the issue in technical board and it has been decided to fix the
> > conflict with libc in the next release (with Olivier's help).
> > So Bruce and me decided to use cmdline function in the meantime.
> 
>  As I can see, cmdline_parse_etheraddr() uses 
> static struct ether_addr *
> my_ether_aton(const char *a)
> internally.
> Why not to make it public, rename to rte_ethet_aton(),
> and move into rte_net?
> And use that one instead?
> Later if/when we'll have name conflict with libc resolved,
> It can become just a wrapper around ether_aton_r().
> Konstantin

Several reasons:
- It would be one more (useless) wrapper
- cmdline_parse_etheraddr is already used in several places
- ether_aton_r and my_ether_aton may have a different behaviour

When the libc conflict will be solved, I prefer replacing uses of
cmdline_parse_etheraddr one by one.




Re: [dpdk-dev] [PATCH v2] ethdev: fix device info getting

2018-10-23 Thread Thomas Monjalon
23/10/2018 03:25, Lu, Wenzhuo:
> Hi Thomas, Ferruh, Andrew,
> 
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > 22/10/2018 14:01, Ferruh Yigit:
> > > On 8/23/2018 9:58 AM, Andrew Rybchenko wrote:
> > > > On 22.08.2018 19:55, Ferruh Yigit wrote:
> > > >> On 8/14/2018 1:57 AM, Lu, Wenzhuo wrote:
> > > >>> Hi Andrew,
> > > >>> From: Andrew Rybchenko [mailto:arybche...@solarflare.com]
> > >  On 13.08.2018 05:50, Lu, Wenzhuo wrote:
> > > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > >> 16/07/2018 03:58, Lu, Wenzhuo:
> > > >>> Hi Andrew,
> > > >>> From: Lu, Wenzhuo
> > >  From: Andrew Rybchenko [mailto:arybche...@solarflare.com]
> > > >
> > > > Hi, Wenzhuo,
> > > >
> > > > I'm sorry, but I have more even harder questions than the
> > > > previous
> > >  one.
> > > > This questions are rather generic and mainly to ethdev
> > maintainers.
> > > >
> > > > On 13.07.2018 05:42, Wenzhuo Lu wrote:
> > > >> The device information cannot be gotten correctly before
> > > >> the configuration is set. Because on some NICs the
> > > >> information has dependence on the configuration.
> > > > Thinking about it I have the following question. Is it valid
> > > > behaviour of the dev_info if it changes after configuration?
> > > > I always thought that the primary goal of the dev_info is to
> > > > provide information to app about device capabilities to
> > > > allow app configure device and queues correctly. Now we see
> > > > the case when dev_info changes on configure. May be it is
> > > > acceptable, but it is really suspicious. If we accept it, it 
> > > > should
> > be documented.
> > > > May be dev_info should be split into parts: part which is
> > > > persistent and part which may depend on device configuration.
> > >  As I remember, the similar discussion has happened :) I've
> > >  raised the similar suggestion like this. But we don’t make it
> > happen.
> > >  The reason is, you see, this is the rte layer's behavior. So
> > >  the user doesn't have to know it. From APP's PoV, it inputs
> > >  the configuration, it calls this API "rte_eth_dev_configure".
> > >  It doesn't know  the configuration is copied before getting the
> > info or not.
> > >  So, to my opinion, we can still keep the behavior. We only
> > >  need to split it into parts when we do see the case that cannot
> > make it.
> > > >>> Maybe I talked too much about the patch. Think about it again.
> > > >>> Your comments is about how to use the APIs,
> > > >>> rte_eth_dev_info_get,
> > > >> rte_eth_dev_configure. To my opinion, rte_eth_dev_info_get is
> > > >> just to get the info. It can be called anywhere, before
> > > >> configuration or after. It's reasonable the info changes with the
> > configuration changing.
> > > >>> But we do have something missing, like,
> > > >>> rte_eth_dev_capability_get which
> > > >> should be stable. APP can use this API to get the necessary
> > > >> info before configuration.
> > > >>> A question, maybe a little divergent thinking, that APP should
> > > >>> have some
> > > >> intelligence to handle the capability automatically. So getting
> > > >> the capability is not so good and effective, looks like we
> > > >> still need the human
> > >  involvement.
> > > >> Maybe that the reason currently we suppose APP know the
> > > >> capability from the paper copies, examples...
> > > >>
> > > >> I am not sure to understand all the sentences.
> > > >> But I agree that we should take a decision about the stability
> > > >> of these
> > >  infos.
> > > >> Either infos cannot change after probing, or we must document
> > > >> that the app must request infos regularly (when?).
> > > > Sorry, I missed this mail.
> > > >
> > > > I have the concern that different NICs have different behavior.
> > > > One info
> > >  can be stable on a NIC but dynamic on another. Considering this,
> > >  we may better not splitting the rte_eth_dev_info_get to 2 APIs.
> > >  And comparing with handling this in rte layer, maybe we can let every
> > NIC has its own decision.
> > > > I have an idea. Maybe we can add a parameter for potential
> > > > dynamic fields. Like, Changing uint16_t nb_rx_queues; to struct
> > > > nb_rx_queues { uint16_t value; bool stable; }
> > >  May be it is just very bad example, but as I understand
> > >  nb_rx_queues is mainly required to configure the device properly.
> > >  Or should app configure, get new value, reconfigure again, get
> > >  new value and so on and stop when previous is equal to the new one.
> > Yes, I dramatise and it sounds really bad.
> > >  In any case it would over-co

Re: [dpdk-dev] How to replace rte_eth_dev_attach with rte_eal_hotplug_add

2018-10-23 Thread Thomas Monjalon
Hi,

23/10/2018 03:52, Hideyuki Yamashita:
> Hi,
> 
> Thanks for your guidance again.
> 
> Q1.
> Is following my understanding correct?
> If a device has multiple port, then "rte_eth_dev_get_port_by_name"
> will NOT work becauase it uses strcmp and needs "exact match"
> of the device name.
> New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
> of this issue and even if the name is "parially matched" with 
> the given parameter(user provided devargs).

Yes

> Q2. 
> If my program only handles devices which create only one port,
> then "rte_eth_dev_get_port_by_name" may work.
> (Though such a program does not have extensibility and
> only workable under certain limitations)

Yes it may work with most of the drivers.

> Q3.
> When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
> will be available?
> Do I have to wait 18.11 release or can I get those in git before release?

Better to wait 18.11.

> I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
> is much better than using "rte_eth_dev_get_port_by_name" with regard
> to handle devices which create multiple port.
> But I need to replace existing deprecated attach/detach APIs to new
> codes to maintain continuity of my product.

We do not remove a method without a replacement.
You can use rte_eth_dev_attach() until 18.08,
and switch to the new iterator with 18.11.


> > Hi,
> > 
> > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> > - it is a loop, so work if multiple ports are matching
> > - it uses devargs parameter, which is what the user requests
> > 
> > Note: your code assumes that the ethdev name is devargs.name.
> > It can be true by chance, but nothing forces drivers to assign port names
> > this way. It will be wrong, for sure, if a device has multiple ports.
> > 
> > 
> > 22/10/2018 13:24, Hideyuki Yamashita:
> > > Hello Thomas,
> > > 
> > > Thanks for your info.
> > > What is the difference between using 
> > > rte_eth_dev_get_port_by_name and 
> > > RTE_ETH_FOREACH_MATCHING_DEV?
> > > 
> > > I think using rte_eth_dev_get_port_by_name is 
> > > workable.
> > > (In fact I modified my code already and it worked with no problem)
> > > 
> > > So my question is "what is the difference" and "which is better approach".
> > > 
> > > Thanks and BR,
> > > Hideyuki Yamashita
> > > NTT TechnoCross
> > > 
> > > > Hi,
> > > > 
> > > > I am actively working on it.
> > > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > > https://patches.dpdk.org/patch/47019/
> > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > > 
> > > > 
> > > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > > Dear Thomas and all,
> > > > > 
> > > > > About a month ago, I posted the topic related with 
> > > > > how to replace rte_eth_dev_attach.
> > > > > 
> > > > > Following your advice,
> > > > > my code would be as below:
> > > > > (Old code using deprecated API is commented out)
> > > > > 
> > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > > after rte_eal_hotplug_add.
> > > > > Note that my application is just one of the dpdk applications(in the 
> > > > > host)
> > > > > and within the process, only one thread handles device attatch/detach.
> > > > > (No race condition with regard to device hot_plug will
> > > > > not take place)
> > > > > ---
> > > > > //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > > > //if (ret < 0)
> > > > > //  return ret;
> > > > > 
> > > > > struct rte_devargs da;
> > > > > 
> > > > > memset(&da, 0, sizeof(da));
> > > > > 
> > > > > /* parse devargs */
> > > > > if (rte_devargs_parse(&da, devargs))
> > > > > return -1;
> > > > > ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args);
> > > > > if (ret < 0) {
> > > > > free(da.args);
> > > > > return ret;
> > > > > }
> > > > > free(da.args);
> > > > > ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > > > if (ret < 0)
> > > > > return ret;
> > > > > -
> > > > > 
> > > > > If you have any concerns/additional advices, please let me know.
> > > > > 
> > > > > BR,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > > 
> > > > > 
> > > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > > Dear Thomas,
> > > > > > > 
> > > > > > > Thansk for your answer.
> > > > > > > Please see inline.
> > > > > > > 
> > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > > Dear Thomas,
> > > > > > > > > 
> > > > > > > > > Thanks for your answer.
> > > > > > > > > It took me a little time to digest answer.
> > > > > > > > > Please see inline.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > > Dear G

[dpdk-dev] [PATCH] test/test_external_mem: fix loop initialization

2018-10-23 Thread Ali Alnubani
Loop initial declarations are only allowed in C99 mode,
which causes a compilation failure on non C99 compliant systems:

test/test/test_external_mem.c: In function ‘test_external_mem’:
test/test/test_external_mem.c:375:2: error: ‘for’
loop initial declarations are only allowed in C99 mode
  for (int i = 0; i < n_pages; i++) {
  ^

Reproduces on RHEL 7.4 with GCC 4.8.5 20150623 (Red Hat 4.8.5-16).

Fixes: b270daa43b3d ("test: support external memory")
Cc: anatoly.bura...@intel.com

Signed-off-by: Ali Alnubani 
---
 test/test/test_external_mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test/test_external_mem.c b/test/test/test_external_mem.c
index d0837aa35..d740960ce 100644
--- a/test/test/test_external_mem.c
+++ b/test/test/test_external_mem.c
@@ -361,7 +361,7 @@ test_external_mem(void)
size_t pgsz = RTE_PGSIZE_4K;
rte_iova_t iova[len / pgsz];
void *addr;
-   int ret, n_pages;
+   int ret, n_pages, i;
 
/* create external memory area */
n_pages = RTE_DIM(iova);
@@ -372,7 +372,7 @@ test_external_mem(void)
__func__, __LINE__);
return -1;
}
-   for (int i = 0; i < n_pages; i++) {
+   for (i = 0; i < n_pages; i++) {
/* arbitrary IOVA */
rte_iova_t tmp = 0x1 + i * pgsz;
iova[i] = tmp;
-- 
2.19.1



Re: [dpdk-dev] [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs

2018-10-23 Thread Shahaf Shuler
Wednesday, October 17, 2018 5:08 AM, Yongseok Koh:
> Subject: [PATCH 4/5] net/mlx5: fix wildcard item for Direct Verbs
> 
> If a network layer is specified with no spec, it means wildcard match.
> flow_dv_translate_item_*() returns without writing anything if spec is null
> and it causes creation of wrong flow. E.g., the following flow has to patch
> with any ipv4 packet.
> 
>   flow create 0 ingress pattern eth / ipv4 / end actions ...
> 
> But, with the current code, it matches any packet because PMD doesn't write
> anything about IPv4. The matcher value and mask becomes completely zero.
> It should have written the IP version at least. It is same for the rest of 
> items.
> 
> Even if the spec is null, PMD has to write constant fields before return, 
> e.g. IP
> version and IP protocol number.
> 
> Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> Cc: or...@mellanox.com
> 
> Signed-off-by: Yongseok Koh 
> Acked-by: Ori Kam 

[...]

>  #include 
>  #include 
>  #include 
> @@ -474,10 +473,6 @@ flow_dv_translate_item_ipv4(void *matcher, void
> *key,
>   char *l24_v;
>   uint8_t tos;
> 
> - if (!ipv4_v)
> - return;
> - if (!ipv4_m)
> - ipv4_m = &nic_mask;
>   if (inner) {
>   headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>inner_headers);
> @@ -489,6 +484,10 @@ flow_dv_translate_item_ipv4(void *matcher, void
> *key,
>   }
>   MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
>   MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 4);

Is matching on the ip version is enough? Don't we need to match also the 
ethertype? 
Meaning maybe the value on the IP offset can be 4 even though it is not a IPv4 
header.  

Same question for IPv6. 

> + if (!ipv4_v)
> + return;
> + if (!ipv4_m)
> + ipv4_m = &nic_mask;
>   l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
>dst_ipv4_dst_ipv6.ipv4_layout.ipv4);
>   l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v, @@ -
> 557,10 +556,6 @@ flow_dv_translate_item_ipv6(void *matcher, void *key,
>   int i;
>   int size;
> 
> - if (!ipv6_v)
> - return;
> - if (!ipv6_m)
> - ipv6_m = &nic_mask;
>   if (inner) {
>   headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>inner_headers);
> @@ -570,6 +565,12 @@ flow_dv_translate_item_ipv6(void *matcher, void
> *key,
>outer_headers);
>   headers_v = MLX5_ADDR_OF(fte_match_param, key,
> outer_headers);
>   }
> + MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> + MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
> + if (!ipv6_v)
> + return;
> + if (!ipv6_m)
> + ipv6_m = &nic_mask;
>   size = sizeof(ipv6_m->hdr.dst_addr);
>   l24_m = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_m,
>dst_ipv4_dst_ipv6.ipv6_layout.ipv6);
> @@ -585,8 +586,6 @@ flow_dv_translate_item_ipv6(void *matcher, void
> *key,
>   memcpy(l24_m, ipv6_m->hdr.src_addr, size);
>   for (i = 0; i < size; ++i)
>   l24_v[i] = l24_m[i] & ipv6_v->hdr.src_addr[i];
> - MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_version, 0xf);
> - MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_version, 6);
>   /* TOS. */
>   vtc_m = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow);
>   vtc_v = rte_be_to_cpu_32(ipv6_m->hdr.vtc_flow & ipv6_v-
> >hdr.vtc_flow); @@ -635,10 +634,6 @@ flow_dv_translate_item_tcp(void
> *matcher, void *key,
>   void *headers_m;
>   void *headers_v;
> 
> - if (!tcp_v)
> - return;
> - if (!tcp_m)
> - tcp_m = &rte_flow_item_tcp_mask;
>   if (inner) {
>   headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>inner_headers);
> @@ -650,6 +645,10 @@ flow_dv_translate_item_tcp(void *matcher, void
> *key,
>   }
>   MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
>   MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> IPPROTO_TCP);
> + if (!tcp_v)
> + return;
> + if (!tcp_m)
> + tcp_m = &rte_flow_item_tcp_mask;
>   MLX5_SET(fte_match_set_lyr_2_4, headers_m, tcp_sport,
>rte_be_to_cpu_16(tcp_m->hdr.src_port));
>   MLX5_SET(fte_match_set_lyr_2_4, headers_v, tcp_sport, @@ -
> 682,10 +681,6 @@ flow_dv_translate_item_udp(void *matcher, void *key,
>   void *headers_m;
>   void *headers_v;
> 
> - if (!udp_v)
> - return;
> - if (!udp_m)
> - udp_m = &rte_flow_item_udp_mask;
>   if (inner) {
>   headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
>inner_headers);
> @@ -697,6 +692,10 @@ flow_dv_translate_item_udp(void *matcher, void

Re: [dpdk-dev] [PATCH 1/2] mbuf: add FCoE packet type

2018-10-23 Thread Olivier Matz
On Sun, Aug 26, 2018 at 06:24:54PM +0530, Jerin Jacob wrote:
> Add support of FCoE packet type.
> 
> Signed-off-by: Jerin Jacob 

Acked-by: Olivier Matz 


Re: [dpdk-dev] [PATCH 2/2] mbuf: add MPLS packet type

2018-10-23 Thread Olivier Matz
On Sun, Aug 26, 2018 at 06:24:55PM +0530, Jerin Jacob wrote:
> Add support of MPLS packet type.
> 
> Signed-off-by: Jerin Jacob 

Acked-by: Olivier Matz 


[dpdk-dev] [PATCH 2/2] net: support Mpls in software packet type parser

2018-10-23 Thread Olivier Matz
Add a new RTE_PTYPE_L2_ETHER_MPLS packet type, and its support in
rte_net_get_ptype().

Signed-off-by: Didier Pallard 
Signed-off-by: Olivier Matz 
---
 lib/librte_mbuf/rte_mbuf_ptype.h |  7 +++
 lib/librte_net/rte_ether.h   |  2 ++
 lib/librte_net/rte_net.c | 21 -
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h b/lib/librte_mbuf/rte_mbuf_ptype.h
index 01acc66e2..3029777e2 100644
--- a/lib/librte_mbuf/rte_mbuf_ptype.h
+++ b/lib/librte_mbuf/rte_mbuf_ptype.h
@@ -131,6 +131,13 @@ extern "C" {
  */
 #define RTE_PTYPE_L2_ETHER_PPPOE0x0008
 /**
+ * MPLS packet type.
+ *
+ * Packet format:
+ * <'ether type'=[0x8847|0x0x8848]>
+ */
+#define RTE_PTYPE_L2_ETHER_MPLS 0x0009
+/**
  * Mask of layer 2 packet types.
  * It is used for outer packet for tunneling cases.
  */
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index bee2b34f0..c2c5e249f 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -306,6 +306,8 @@ struct vxlan_hdr {
 #define ETHER_TYPE_SLOW 0x8809 /**< Slow protocols (LACP and Marker). */
 #define ETHER_TYPE_TEB  0x6558 /**< Transparent Ethernet Bridging. */
 #define ETHER_TYPE_LLDP 0x88CC /**< LLDP Protocol. */
+#define ETHER_TYPE_MPLS 0x8847 /**< MPLS ethertype. */
+#define ETHER_TYPE_MPLSM 0x8848 /**< MPLS multicast ethertype. */
 
 #define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct vxlan_hdr))
 /**< VXLAN tunnel header length. */
diff --git a/lib/librte_net/rte_net.c b/lib/librte_net/rte_net.c
index 9eb7c7438..378a4126c 100644
--- a/lib/librte_net/rte_net.c
+++ b/lib/librte_net/rte_net.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* get l3 packet type from ip6 next protocol */
@@ -274,9 +275,27 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf *m,
off += 2 * sizeof(*vh);
hdr_lens->l2_len += 2 * sizeof(*vh);
proto = vh->eth_proto;
+   } else if ((proto == rte_cpu_to_be_16(ETHER_TYPE_MPLS)) ||
+   (proto == rte_cpu_to_be_16(ETHER_TYPE_MPLSM))) {
+   unsigned int i;
+   const struct mpls_hdr *mh;
+   struct mpls_hdr mh_copy;
+
+#define MAX_MPLS_HDR 5
+   for (i = 0; i < MAX_MPLS_HDR; i++) {
+   mh = rte_pktmbuf_read(m, off + (i * sizeof(*mh)),
+   sizeof(*mh), &mh_copy);
+   if (unlikely(mh == NULL))
+   return pkt_type;
+   }
+   if (i == MAX_MPLS_HDR)
+   return pkt_type;
+   pkt_type = RTE_PTYPE_L2_ETHER_MPLS;
+   hdr_lens->l2_len += (sizeof(*mh) * i);
+   return pkt_type;
}
 
- l3:
+l3:
if ((layers & RTE_PTYPE_L3_MASK) == 0)
return pkt_type;
 
-- 
2.11.0



[dpdk-dev] [PATCH 1/2] net: add Mpls header structure

2018-10-23 Thread Olivier Matz
Add the Mpls header structure in librte_net. It will be used by next
patch that adds the support of Mpls L2 layer in the software packet
type parser.

Signed-off-by: Olivier Matz 
---
 lib/librte_net/Makefile|  2 +-
 lib/librte_net/meson.build |  3 ++-
 lib/librte_net/rte_mpls.h  | 42 ++
 3 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_net/rte_mpls.h

diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index 85e403f41..c3082069a 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -20,6 +20,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_arp.c
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h rte_tcp.h rte_udp.h 
rte_esp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h
-SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_net_crc.h rte_mpls.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build
index d3ea1feb5..7d66f693c 100644
--- a/lib/librte_net/meson.build
+++ b/lib/librte_net/meson.build
@@ -13,7 +13,8 @@ headers = files('rte_ip.h',
'rte_ether.h',
'rte_gre.h',
'rte_net.h',
-   'rte_net_crc.h')
+   'rte_net_crc.h',
+   'rte_mpls.h')
 
 sources = files('rte_arp.c', 'rte_net.c', 'rte_net_crc.c')
 deps += ['mbuf']
diff --git a/lib/librte_net/rte_mpls.h b/lib/librte_net/rte_mpls.h
new file mode 100644
index 0..11d26ba35
--- /dev/null
+++ b/lib/librte_net/rte_mpls.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2016 6WIND S.A.
+ */
+
+#ifndef _RTE_MPLS_H_
+#define _RTE_MPLS_H_
+
+/**
+ * @file
+ *
+ * MPLS-related defines
+ */
+
+#include 
+#include 
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * MPLS header.
+ */
+struct mpls_hdr {
+   uint16_t tag_msb;   /**< Label(msb). */
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+   uint8_t tag_lsb:4;  /**< Label(lsb). */
+   uint8_t tc:3;   /**< Traffic class. */
+   uint8_t bs:1;   /**< Bottom of stack. */
+#else
+   uint8_t bs:1;   /**< Bottom of stack. */
+   uint8_t tc:3;   /**< Traffic class. */
+   uint8_t tag_lsb:4;  /**< label(lsb) */
+#endif
+   uint8_t  ttl;   /**< Time to live. */
+} __attribute__((__packed__));
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_MPLS_H_ */
-- 
2.11.0



Re: [dpdk-dev] [PATCH 2/2] mbuf: add MPLS packet type

2018-10-23 Thread Olivier Matz
On Tue, Oct 23, 2018 at 09:45:33AM +0200, Olivier Matz wrote:
> On Sun, Aug 26, 2018 at 06:24:55PM +0530, Jerin Jacob wrote:
> > Add support of MPLS packet type.
> > 
> > Signed-off-by: Jerin Jacob 
> 
> Acked-by: Olivier Matz 

By the way, I've just submitted an implementation of the software parser
for MPLS.

https://mails.dpdk.org/archives/dev/2018-October/116691.html


Re: [dpdk-dev] [PATCH] vhost: fix vector filling for packed ring

2018-10-23 Thread Maxime Coquelin

Hi Tiwei,

On 10/23/2018 08:07 AM, Tiwei Bie wrote:

We should return the length of the buffers described by
the current descriptor chain after filling the buffer
vector. So we need to zero the *len first.

Fixes: 2f3225a7d69b ("vhost: add vector filling support for packed ring")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/virtio_net.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 48228f16b..4e6c35d56 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -521,6 +521,7 @@ fill_vec_buf_packed(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
return -1;
  
  	*desc_count = 0;

+   *len = 0;
  
  	while (1) {

if (unlikely(vec_id >= BUF_VECTOR_MAX))



Good catch, I guess I didn't felt into that bug because my test-cases
were too simple (single descriptor were enough).

Reviewed-by: Maxime Coquelin 

Thanks!
Maxime


[dpdk-dev] DPDK techboard minutes of October 10

2018-10-23 Thread Hemant Agrawal
Meeting notes for the DPDK technical board meeting held on 2018-10-10

Attendees:
- Bruce Richardson
- Ferruh Yigit
- Hemant Agrawal
- Jerin Jacob
- Konstantin Ananyev
- Maxime Coquelin
- Olivier Matz
- Stephen Hemminger
- Thomas Monjalon


1) rte_ether.h conflict with standard headers

- At present, one cannot use netinet with DPDK 
- Plan is to add rte_prefix to the current definitions in this header file.
- Olivier will send a deprecation notice for 18.11 timeframe.
- It will be fixed in 19.02 timeframe


2) 18.11 preparedness

- rte_flow needs backup maintainer. Ori's name was suggested.
- ipsec library will not make it for 18.11
- json - we don't want to write our own parser, distros are ok to support 
jansson.
  -  ARM community to check for dependency w.r.t cross compilation case.

3) US DPDK Summit

- it was suggested to do the CFP review on time and send the notifications asap.


4) Techboard rep in Governing Board.

- Konstantin will replace Jerin for next 4 month as TB rep in GB.
- Konstantin to confirm. 

5) EAL Split: proposal to divide the EAL and move high level init in a separate 
library.
- it can be taken for 19.02. It will help libraries like telemetry.

6) Misc 
  - Stephen raised issue with constructors
- they are causing issues in some environments.
- an alternate can be doing it via EAL with some kind of registration 
mechanism. 
- need a more solid alternate proposal before we pursue this change.

 - it was suggested to remove experimental tags from some of the APIs before 
18.11, specially the ones used by OVS.


7) Next techboard meeting

- It will be on October 24
- Jerin will chair it


[dpdk-dev] [PATCH v7 4/7] doc: replace doxygen example in contribution guide

2018-10-23 Thread Thomas Monjalon
The provided example of doxygen header is about a deprecated function.
It is replaced by rte_spinlock_trylock() which is small and
good enough for the purpose.

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
---
 doc/guides/contributing/documentation.rst | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/doc/guides/contributing/documentation.rst 
b/doc/guides/contributing/documentation.rst
index 097575ad7..0165990ed 100644
--- a/doc/guides/contributing/documentation.rst
+++ b/doc/guides/contributing/documentation.rst
@@ -615,19 +615,14 @@ The following are some guidelines for use of Doxygen in 
the DPDK API documentati
   .. code-block:: c
 
  /**
-  * Attach a new Ethernet device specified by arguments.
-  *
-  * @param devargs
-  *  A pointer to a strings array describing the new device
-  *  to be attached. The strings should be a pci address like
-  *  `:01:00.0` or **virtual** device name like `net_pcap0`.
-  * @param port_id
-  *  A pointer to a port identifier actually attached.
+  * Try to take the lock.
   *
+  * @param sl
+  *   A pointer to the spinlock.
   * @return
-  *  0 on success and port_id is filled, negative on error.
+  *   1 if the lock is successfully taken; 0 otherwise.
   */
- int rte_eth_dev_attach(const char *devargs, uint8_t *port_id);
+ int rte_spinlock_trylock(rte_spinlock_t *sl);
 
 * Doxygen supports Markdown style syntax such as bold, italics, fixed width 
text and lists.
   For example the second line in the ``devargs`` parameter in the previous 
example will be rendered as:
-- 
2.19.0



[dpdk-dev] [PATCH v7 3/7] ethdev: allow iterating with pure class filter

2018-10-23 Thread Thomas Monjalon
If no rte_device is given in the iterator,
eth_dev_match() is looking at all ports without any restriction,
except the ethdev kvargs filter.

It allows to iterate with a devargs filter referencing only
some ethdev parameters. The format (from the new devargs syntax) is:
class=eth,paramY=Y

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
---
 lib/librte_ethdev/rte_class_eth.c |  2 +-
 lib/librte_ethdev/rte_ethdev.c| 25 ++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ethdev/rte_class_eth.c 
b/lib/librte_ethdev/rte_class_eth.c
index c04279ec6..58fed694b 100644
--- a/lib/librte_ethdev/rte_class_eth.c
+++ b/lib/librte_ethdev/rte_class_eth.c
@@ -42,7 +42,7 @@ eth_dev_match(const struct rte_eth_dev *edev,
 
if (edev->state == RTE_ETH_DEV_UNUSED)
return -1;
-   if (edev->device != arg->device)
+   if (arg->device != NULL && arg->device != edev->device)
return -1;
if (kvlist == NULL)
/* Empty string matches everything. */
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 3550aa696..7ed71744c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -202,10 +202,24 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, 
const char *devargs_str)
 * The devargs string may use various syntaxes:
 *   - :08:00.0,representor=[1-3]
 *   - pci::06:00.0,representor=[0,5]
+*   - class=eth,mac=00:11:22:33:44:55
 * A new syntax is in development (not yet supported):
 *   - bus=X,paramX=x/class=Y,paramY=y/driver=Z,paramZ=z
 */
 
+   /*
+* Handle pure class filter (i.e. without any bus-level argument),
+* from future new syntax.
+* rte_devargs_parse() is not yet supporting the new syntax,
+* that's why this simple case is temporarily parsed here.
+*/
+#define iter_anybus_str "class=eth,"
+   if (strncmp(devargs_str, iter_anybus_str,
+   strlen(iter_anybus_str)) == 0) {
+   iter->cls_str = devargs_str + strlen(iter_anybus_str);
+   goto end;
+   }
+
/* Split bus, device and parameters. */
ret = rte_devargs_parse(&devargs, devargs_str);
if (ret != 0)
@@ -259,6 +273,7 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const 
char *devargs_str)
}
iter->bus_str = bus_str;
 
+end:
iter->cls = rte_class_find_by_name("eth");
return 0;
 
@@ -279,8 +294,10 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
return RTE_MAX_ETHPORTS;
 
do { /* loop to try all matching rte_device */
-   /* If not in middle of rte_eth_dev iteration, */
-   if (iter->class_device == NULL) {
+   /* If not pure ethdev filter and */
+   if (iter->bus != NULL &&
+   /* not in middle of rte_eth_dev iteration, */
+   iter->class_device == NULL) {
/* get next rte_device to try. */
iter->device = iter->bus->dev_iterate(
iter->device, iter->bus_str, iter);
@@ -292,7 +309,7 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
iter->class_device, iter->cls_str, iter);
if (iter->class_device != NULL)
return eth_dev_to_id(iter->class_device); /* match */
-   } while (1); /* need to try next rte_device */
+   } while (iter->bus != NULL); /* need to try next rte_device */
 
/* No more ethdev port to iterate. */
rte_eth_iterator_cleanup(iter);
@@ -302,6 +319,8 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
 void __rte_experimental
 rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
 {
+   if (iter->bus_str == NULL)
+   return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
free(RTE_CAST_FIELD(iter, cls_str, char *)); /* workaround const */
memset(iter, 0, sizeof(*iter));
-- 
2.19.0



[dpdk-dev] [PATCH v7 1/7] bus/vdev: add iteration filter on name

2018-10-23 Thread Thomas Monjalon
A virtual device can be matched with following syntax:
bus=vdev,name=X

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
---
 drivers/bus/vdev/vdev_params.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/vdev/vdev_params.c b/drivers/bus/vdev/vdev_params.c
index da270f2ec..6f74704d1 100644
--- a/drivers/bus/vdev/vdev_params.c
+++ b/drivers/bus/vdev/vdev_params.c
@@ -2,6 +2,8 @@
  * Copyright 2018 Gaëtan Rivet
  */
 
+#include 
+
 #include 
 #include 
 #include 
@@ -11,10 +13,12 @@
 #include "vdev_private.h"
 
 enum vdev_params {
+   RTE_VDEV_PARAM_NAME,
RTE_VDEV_PARAM_MAX,
 };
 
 static const char * const vdev_params_keys[] = {
+   [RTE_VDEV_PARAM_NAME] = "name",
[RTE_VDEV_PARAM_MAX] = NULL,
 };
 
@@ -22,10 +26,21 @@ static int
 vdev_dev_match(const struct rte_device *dev,
   const void *_kvlist)
 {
+   int ret;
const struct rte_kvargs *kvlist = _kvlist;
+   char *name;
+
+   /* cannot pass const dev->name to rte_kvargs_process() */
+   name = strdup(dev->name);
+   if (name == NULL)
+   return -1;
+   ret = rte_kvargs_process(kvlist,
+   vdev_params_keys[RTE_VDEV_PARAM_NAME],
+   rte_kvargs_strcmp, name);
+   free(name);
+   if (ret != 0)
+   return -1;
 
-   (void) kvlist;
-   (void) dev;
return 0;
 }
 
-- 
2.19.0



[dpdk-dev] [PATCH v7 0/7] replace attach/detach functions

2018-10-23 Thread Thomas Monjalon
The functions for EAL attach/detach had already some replacements,
so they are removed.
The functions for ethdev attach/detach are removed and replaced
thanks to a new ethdev iterator working with devargs.

rte_eth_dev_attach(devargs, &port_id)
is replaced by:
rte_dev_probe(devargs);
RTE_ETH_FOREACH_MATCHING_DEV(port_id, devargs, &iterator) {
do what you want with the new port_id
}

The biggest benefit is to be able to manage devargs string
matching several ports to probe.


Changes in v7:
  - remove use of eth_dev->data->name after detach in testpmd

Changes in v6:
  - improve testpmd logs
  - update testpmd port_status when detaching multiple ports

Changes in v5:
  - check double detach in testpmd

Changes in v4:
  - fix compilation (due to stricter checks in v3)

Changes in v3 - after Andrew's review:
  - coding rules compliance
  - stricter checks for snprintf
  - rte_eth_iterator_free() becomes rte_eth_iterator_cleanup()
  - add rte_eth_iterator_cleanup in .map

Changes in v2 - after Andrew's review:
  - return only 0 or -1 in vdev_dev_match()
  - fix freeing of strings in the iterator
  - add rte_eth_iterator_free()
  - tolerate extra parameters from old syntax
  - add more comments
  - separate contributing guide patch


Thomas Monjalon (7):
  bus/vdev: add iteration filter on name
  ethdev: add iterator to match devargs input
  ethdev: allow iterating with pure class filter
  doc: replace doxygen example in contribution guide
  ethdev: remove deprecated attach/detach functions
  eal: remove deprecated attach/detach functions
  app/testpmd: check not detaching device twice

 app/test-pmd/testpmd.c|  42 +++-
 doc/guides/contributing/documentation.rst |  15 +-
 doc/guides/prog_guide/index.rst   |   1 -
 .../prog_guide/port_hotplug_framework.rst | 106 -
 doc/guides/rel_notes/deprecation.rst  |  12 -
 doc/guides/rel_notes/release_18_11.rst|  12 +
 drivers/bus/vdev/vdev_params.c|  19 +-
 drivers/net/virtio/virtio_user_ethdev.c   |   1 -
 lib/librte_eal/common/eal_common_dev.c|  53 -
 lib/librte_eal/common/include/rte_common.h|   6 +
 lib/librte_eal/common/include/rte_dev.h   |  27 ---
 lib/librte_eal/rte_eal_version.map|   2 -
 lib/librte_ethdev/ethdev_private.c|  10 +-
 lib/librte_ethdev/ethdev_private.h|   6 +
 lib/librte_ethdev/rte_class_eth.c |   9 +-
 lib/librte_ethdev/rte_ethdev.c| 223 +++---
 lib/librte_ethdev/rte_ethdev.h| 110 ++---
 lib/librte_ethdev/rte_ethdev_version.map  |   5 +-
 18 files changed, 321 insertions(+), 338 deletions(-)
 delete mode 100644 doc/guides/prog_guide/port_hotplug_framework.rst

-- 
2.19.0



[dpdk-dev] [PATCH v7 2/7] ethdev: add iterator to match devargs input

2018-10-23 Thread Thomas Monjalon
The iterator will return the ethdev port ids matching a devargs string.
It is recommended to use the macro RTE_ETH_FOREACH_MATCHING_DEV()
for usage convenience.

The class string is prefixed with '+' in order to skip the validation
of the parameter keys. It is tolerated for the compatibility with
the old (current) syntax where all parameters (bus, class and driver)
are mixed in the same string without any delimiter.
Thanks to this compatibility prefix, the driver parameters will be
skipped during the ethdev parsing, and not considered invalid.

A macro is introduced in rte_common.h to workaround a const field.
This hack is needed to free const strings in the iterator.
It is preferred to keep the const for these fields, because it gives
a hint that they are not changed at each iteration.

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
---
 lib/librte_eal/common/include/rte_common.h |   6 +
 lib/librte_ethdev/ethdev_private.c |  10 +-
 lib/librte_ethdev/ethdev_private.h |   6 +
 lib/librte_ethdev/rte_class_eth.c  |   7 +-
 lib/librte_ethdev/rte_ethdev.c | 123 +
 lib/librte_ethdev/rte_ethdev.h |  79 +
 lib/librte_ethdev/rte_ethdev_version.map   |   3 +
 7 files changed, 232 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_common.h 
b/lib/librte_eal/common/include/rte_common.h
index 069c13ec7..e3c0407a9 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -164,6 +164,12 @@ static void __attribute__((destructor(RTE_PRIO(prio)), 
used)) func(void)
  */
 #define RTE_PTR_DIFF(ptr1, ptr2) ((uintptr_t)(ptr1) - (uintptr_t)(ptr2))
 
+/**
+ * Workaround to cast a const field of a structure to non-const type.
+ */
+#define RTE_CAST_FIELD(var, field, type) \
+   (*(type *)((uintptr_t)(var) + offsetof(typeof(*(var)), field)))
+
 /*** Macros/static functions for doing alignment /
 
 
diff --git a/lib/librte_ethdev/ethdev_private.c 
b/lib/librte_ethdev/ethdev_private.c
index 768c8b2ed..acc787dba 100644
--- a/lib/librte_ethdev/ethdev_private.c
+++ b/lib/librte_ethdev/ethdev_private.c
@@ -5,6 +5,14 @@
 #include "rte_ethdev.h"
 #include "ethdev_private.h"
 
+uint16_t
+eth_dev_to_id(const struct rte_eth_dev *dev)
+{
+   if (dev == NULL)
+   return RTE_MAX_ETHPORTS;
+   return dev - rte_eth_devices;
+}
+
 struct rte_eth_dev *
 eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
const void *data)
@@ -18,7 +26,7 @@ eth_find_device(const struct rte_eth_dev *start, 
rte_eth_cmp_t cmp,
 start > &rte_eth_devices[RTE_MAX_ETHPORTS]))
return NULL;
if (start != NULL)
-   idx = start - &rte_eth_devices[0] + 1;
+   idx = eth_dev_to_id(start) + 1;
else
idx = 0;
for (; idx < RTE_MAX_ETHPORTS; idx++) {
diff --git a/lib/librte_ethdev/ethdev_private.h 
b/lib/librte_ethdev/ethdev_private.h
index 0f5c6d5c4..e67cf6831 100644
--- a/lib/librte_ethdev/ethdev_private.h
+++ b/lib/librte_ethdev/ethdev_private.h
@@ -11,6 +11,12 @@
 extern "C" {
 #endif
 
+/*
+ * Convert rte_eth_dev pointer to port id.
+ * NULL will be translated to RTE_MAX_ETHPORTS.
+ */
+uint16_t eth_dev_to_id(const struct rte_eth_dev *dev);
+
 /* Generic rte_eth_dev comparison function. */
 typedef int (*rte_eth_cmp_t)(const struct rte_eth_dev *, const void *);
 
diff --git a/lib/librte_ethdev/rte_class_eth.c 
b/lib/librte_ethdev/rte_class_eth.c
index 84b646291..c04279ec6 100644
--- a/lib/librte_ethdev/rte_class_eth.c
+++ b/lib/librte_ethdev/rte_class_eth.c
@@ -57,9 +57,14 @@ eth_dev_iterate(const void *start,
 {
struct rte_kvargs *kvargs = NULL;
struct rte_eth_dev *edev = NULL;
+   const char * const *valid_keys = NULL;
 
if (str != NULL) {
-   kvargs = rte_kvargs_parse(str, eth_params_keys);
+   if (str[0] == '+') /* no validation of keys */
+   str++;
+   else
+   valid_keys = eth_params_keys;
+   kvargs = rte_kvargs_parse(str, valid_keys);
if (kvargs == NULL) {
RTE_LOG(ERR, EAL, "cannot parse argument list\n");
rte_errno = EINVAL;
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 0979c0c7b..3550aa696 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -36,11 +36,13 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
 #include "rte_ethdev_driver.h"
 #include "ethdev_profile.h"
+#include "ethdev_private.h"
 
 int rte_eth_dev_logtype;
 
@@ -184,6 +186,127 @@ enum {
STAT_QMAP_RX
 };
 
+int __rte_experimental
+rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
+{
+   int ret;
+   struct rte_devargs devargs = {.args = NULL}

[dpdk-dev] [PATCH v7 5/7] ethdev: remove deprecated attach/detach functions

2018-10-23 Thread Thomas Monjalon
The hotplug attach/detach features are implemented in EAL layer.
There is a new ethdev iterator to retrieve ports from ethdev layer.

As announced earlier, the (buggy) ethdev functions are now removed.

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
---
 app/test-pmd/testpmd.c|  22 +++-
 doc/guides/prog_guide/index.rst   |   1 -
 .../prog_guide/port_hotplug_framework.rst | 106 --
 doc/guides/rel_notes/deprecation.rst  |   7 --
 doc/guides/rel_notes/release_18_11.rst|   6 +
 drivers/net/virtio/virtio_user_ethdev.c   |   1 -
 lib/librte_ethdev/rte_ethdev.c|  81 -
 lib/librte_ethdev/rte_ethdev.h|  31 -
 lib/librte_ethdev/rte_ethdev_version.map  |   2 -
 9 files changed, 22 insertions(+), 235 deletions(-)
 delete mode 100644 doc/guides/prog_guide/port_hotplug_framework.rst

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index eea6df0fa..14647fa19 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -481,6 +481,7 @@ struct nvgre_encap_conf nvgre_encap_conf = {
 };
 
 /* Forward function declarations */
+static void setup_attached_port(portid_t pi);
 static void map_port_queue_stats_mapping_registers(portid_t pi,
   struct rte_port *port);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -2308,7 +2309,7 @@ void
 attach_port(char *identifier)
 {
portid_t pi = 0;
-   unsigned int socket_id;
+   struct rte_dev_iterator iterator;
 
printf("Attaching a new port...\n");
 
@@ -2317,8 +2318,19 @@ attach_port(char *identifier)
return;
}
 
-   if (rte_eth_dev_attach(identifier, &pi))
+   if (rte_dev_probe(identifier) != 0) {
+   TESTPMD_LOG(ERR, "Failed to attach port %s\n", identifier);
return;
+   }
+
+   RTE_ETH_FOREACH_MATCHING_DEV(pi, identifier, &iterator)
+   setup_attached_port(pi);
+}
+
+static void
+setup_attached_port(portid_t pi)
+{
+   unsigned int socket_id;
 
socket_id = (unsigned)rte_eth_dev_socket_id(pi);
/* if socket_id is invalid, set to the first available socket. */
@@ -2341,9 +2353,7 @@ attach_port(char *identifier)
 void
 detach_port(portid_t port_id)
 {
-   char name[RTE_ETH_NAME_MAX_LEN];
-
-   printf("Detaching a port...\n");
+   printf("Removing a device...\n");
 
if (ports[port_id].port_status != RTE_PORT_CLOSED) {
if (ports[port_id].port_status != RTE_PORT_STOPPED) {
@@ -2355,7 +2365,7 @@ detach_port(portid_t port_id)
port_flow_flush(port_id);
}
 
-   if (rte_eth_dev_detach(port_id, name)) {
+   if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
return;
}
diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index c81d9c54f..2086e2442 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -53,7 +53,6 @@ Programmer's Guide
 packet_framework
 vhost_lib
 metrics_lib
-port_hotplug_framework
 bpf_lib
 source_org
 dev_kit_build_system
diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst 
b/doc/guides/prog_guide/port_hotplug_framework.rst
deleted file mode 100644
index fb0efc18f..0
--- a/doc/guides/prog_guide/port_hotplug_framework.rst
+++ /dev/null
@@ -1,106 +0,0 @@
-..  BSD LICENSE
-Copyright(c) 2015 IGEL Co.,Ltd. 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 IGEL Co.,Ltd. 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 L

[dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching device twice

2018-10-23 Thread Thomas Monjalon
The command "port detach" is removing the EAL rte_device
of the ethdev port specified as parameter.

After detaching, the pointer, which maps a port to its device,
is resetted. This way, it is possible to check whether a port
is still associated to a (not removed) device.

Signed-off-by: Thomas Monjalon 
---
 app/test-pmd/testpmd.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 14647fa19..d5998fddc 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)
 void
 detach_port(portid_t port_id)
 {
+   struct rte_device *dev;
+   portid_t sibling;
+
printf("Removing a device...\n");
 
+   dev = rte_eth_devices[port_id].device;
+   if (dev == NULL) {
+   printf("Device already removed\n");
+   return;
+   }
+
if (ports[port_id].port_status != RTE_PORT_CLOSED) {
if (ports[port_id].port_status != RTE_PORT_STOPPED) {
printf("Port not stopped\n");
@@ -2365,15 +2374,24 @@ detach_port(portid_t port_id)
port_flow_flush(port_id);
}
 
-   if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
+   if (rte_dev_remove(dev) != 0) {
TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
return;
}
 
+   /* reset mapping between old ports and removed device */
+   for (sibling = 0; sibling < RTE_MAX_ETHPORTS; sibling++)
+   if (rte_eth_devices[sibling].device == dev) {
+   rte_eth_devices[sibling].device = NULL;
+   if (ports[sibling].port_status != RTE_PORT_CLOSED) {
+   ports[sibling].port_status = RTE_PORT_CLOSED;
+   printf("Port %u is closed\n", sibling);
+   }
+   }
+
remove_unused_fwd_ports();
 
-   printf("Port %u is detached. Now total ports is %d\n",
-   port_id, nb_ports);
+   printf("Now total ports is %d\n", nb_ports);
printf("Done\n");
return;
 }
-- 
2.19.0



[dpdk-dev] [PATCH v7 6/7] eal: remove deprecated attach/detach functions

2018-10-23 Thread Thomas Monjalon
These hotplug functions were deprecated and have some new replacements.
As announced earlier, the oldest ones are now removed.

Signed-off-by: Thomas Monjalon 
Reviewed-by: Andrew Rybchenko 
---
 doc/guides/rel_notes/deprecation.rst|  5 ---
 doc/guides/rel_notes/release_18_11.rst  |  6 +++
 lib/librte_eal/common/eal_common_dev.c  | 53 -
 lib/librte_eal/common/include/rte_dev.h | 27 -
 lib/librte_eal/rte_eal_version.map  |  2 -
 5 files changed, 6 insertions(+), 87 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 6b51c9d28..34b28234c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -49,11 +49,6 @@ Deprecation Notices
   Target release for removal of the legacy API will be defined once most
   PMDs have switched to rte_flow.
 
-* eal: In v18.11 ``rte_eal_dev_attach()`` and ``rte_eal_dev_detach()``
-  will be removed.
-  Hotplug functions ``rte_eal_hotplug_add()`` and ``rte_eal_hotplug_remove()``
-  should be used directly.
-
 * pdump: As we changed to use generic IPC, some changes in APIs and structure
   are expected in subsequent release.
 
diff --git a/doc/guides/rel_notes/release_18_11.rst 
b/doc/guides/rel_notes/release_18_11.rst
index e7e676a05..36c00dbcc 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -233,6 +233,12 @@ API Changes
 * eal: The parameters of the function ``rte_devargs_remove()`` have changed
   from bus and device names to ``struct rte_devargs``.
 
+* eal: The deprecated functions attach/detach were removed in 18.11.
+  ``rte_eal_dev_attach`` can be replaced by
+  ``rte_dev_probe`` or ``rte_eal_hotplug_add``.
+  ``rte_eal_dev_detach`` can be replaced by
+  ``rte_dev_remove`` or ``rte_eal_hotplug_remove``.
+
 * eal: The scope of ``rte_eal_hotplug_add()``/``rte_dev_probe()``
   and ``rte_eal_hotplug_remove()``/``rte_dev_remove()`` is extended.
   In multi-process model, they will guarantee that the device is
diff --git a/lib/librte_eal/common/eal_common_dev.c 
b/lib/librte_eal/common/eal_common_dev.c
index 6ac3ee859..62e9ed477 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -83,59 +83,6 @@ rte_dev_is_probed(const struct rte_device *dev)
return dev->driver != NULL;
 }
 
-int rte_eal_dev_attach(const char *name, const char *devargs)
-{
-   struct rte_bus *bus;
-
-   if (name == NULL || devargs == NULL) {
-   RTE_LOG(ERR, EAL, "Invalid device or arguments provided\n");
-   return -EINVAL;
-   }
-
-   bus = rte_bus_find_by_device_name(name);
-   if (bus == NULL) {
-   RTE_LOG(ERR, EAL, "Unable to find a bus for the device '%s'\n",
-   name);
-   return -EINVAL;
-   }
-   if (strcmp(bus->name, "pci") == 0 || strcmp(bus->name, "vdev") == 0)
-   return rte_eal_hotplug_add(bus->name, name, devargs);
-
-   RTE_LOG(ERR, EAL,
-   "Device attach is only supported for PCI and vdev devices.\n");
-
-   return -ENOTSUP;
-}
-
-int rte_eal_dev_detach(struct rte_device *dev)
-{
-   struct rte_bus *bus;
-   int ret;
-
-   if (dev == NULL) {
-   RTE_LOG(ERR, EAL, "Invalid device provided.\n");
-   return -EINVAL;
-   }
-
-   bus = rte_bus_find_by_device(dev);
-   if (bus == NULL) {
-   RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
-   dev->name);
-   return -EINVAL;
-   }
-
-   if (bus->unplug == NULL) {
-   RTE_LOG(ERR, EAL, "Bus function not supported\n");
-   return -ENOTSUP;
-   }
-
-   ret = bus->unplug(dev);
-   if (ret)
-   RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
-   dev->name);
-   return ret;
-}
-
 /* helper function to build devargs, caller should free the memory */
 static int
 build_devargs(const char *busname, const char *devname,
diff --git a/lib/librte_eal/common/include/rte_dev.h 
b/lib/librte_eal/common/include/rte_dev.h
index 4e549e802..cd6c187cc 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -176,33 +176,6 @@ struct rte_device {
 __rte_experimental
 int rte_dev_is_probed(const struct rte_device *dev);
 
-/**
- * Attach a device to a registered driver.
- *
- * @param name
- *   The device name, that refers to a pci device (or some private
- *   way of designating a vdev device). Based on this device name, eal
- *   will identify a driver capable of handling it and pass it to the
- *   driver probing function.
- * @param devargs
- *   Device arguments to be passed to the driver.
- * @return
- *   0 on success, negative on error.
- */
-__rte_deprecated
-int rte_eal_dev_attach(const char *name, const char *devargs);
-
-/**
- * Detach a device from its driver.

Re: [dpdk-dev] [PATCH v3 4/4] ethdev: support MAC address as iterator filter

2018-10-23 Thread Ananyev, Konstantin



> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Tuesday, October 23, 2018 8:21 AM
> To: Ananyev, Konstantin 
> Cc: Andrew Rybchenko ; dev@dpdk.org; 
> gaetan.ri...@6wind.com; ophi...@mellanox.com; Yigit, Ferruh
> ; olivier.m...@6wind.com; Horton, Remy 
> ; Richardson, Bruce
> 
> Subject: Re: [dpdk-dev] [PATCH v3 4/4] ethdev: support MAC address as 
> iterator filter
> 
> 22/10/2018 23:24, Ananyev, Konstantin:
> > From: Thomas Monjalon
> > > 22/10/2018 15:37, Andrew Rybchenko:
> > > > On 10/22/18 4:15 PM, Thomas Monjalon wrote:
> > > > > The MAC addresses of a port can be matched with devargs.
> > > > >
> > > > > As the conflict between rte_ether.h and netinet/ether.h is not 
> > > > > resolved,
> > > > > the MAC parsing is done with a rte_cmdline function.
> > > > > As a result, cmdline library becomes a dependency of ethdev.
> > > > >
> > > > > Signed-off-by: Thomas Monjalon 
> > > >
> > > > I'd like to share my thought about a new dependency.
> > > > Looking at cmdline I think that it is a bad and strange
> > > > dependency for kvargs. IMHO, even duplication of the
> > > > code to parse MAC address it less evil in this case.
> > >
> > > cmdline is not a dependency for kvargs.
> > > I have added it as a dependency for ethdev.
> > >
> > > > May be it is possible to provide internal wrapper
> > > > which is implemented using ether_aton_r() and located
> > > > in a separate C file which does not include rte_ether.h etc?
> > >
> > > I raised the issue in technical board and it has been decided to fix the
> > > conflict with libc in the next release (with Olivier's help).
> > > So Bruce and me decided to use cmdline function in the meantime.
> >
> >  As I can see, cmdline_parse_etheraddr() uses
> > static struct ether_addr *
> > my_ether_aton(const char *a)
> > internally.
> > Why not to make it public, rename to rte_ethet_aton(),
> > and move into rte_net?
> > And use that one instead?
> > Later if/when we'll have name conflict with libc resolved,
> > It can become just a wrapper around ether_aton_r().
> > Konstantin
> 
> Several reasons:
>   - It would be one more (useless) wrapper

Well, it would be *when* will have libc naming conflict resolved.
Till that it would be pretty useful I think.

>   - cmdline_parse_etheraddr is already used in several places


As I can see right now it is used just by bond PMD:
$ find lib drivers -type f | xargs grep -l cmdline_parse_etheraddr | grep -v 
librte_cmdline
drivers/net/bonding/rte_eth_bond_args.c

Again if that function is *really* used in several places to convert string to 
mac,
then I suppose it is an indication that rte_ether_aton() would be useful.
Without forcing cmdline dependency.

>   - ether_aton_r and my_ether_aton may have a different behavior

Could you elaborate here?
What exactly would be different?
Both supposed to convert string to ether addr.
If our function can't do that properly, then I think it is a bug in our side.
If ether_aton_r() just supports extra formats(::), then it
would be extension to current behavior.

> 
> When the libc conflict will be solved, I prefer replacing uses of
> cmdline_parse_etheraddr one by one.

We can do the same with rte_ether_aton() too, if we really want to.
 



Re: [dpdk-dev] Change ether_addr to avoid conflict

2018-10-23 Thread Thomas Monjalon
22/10/2018 17:06, Wiles, Keith:
> 
> > On Oct 21, 2018, at 8:39 AM, Lewis Donzis  wrote:
> > 
> > Please consider changing “struct ether_addr” in rte_ether.h to “struct 
> > rte_ether_addr”, in order to avoid conflicts with the same structure name 
> > /usr/include/net/ethernet.h.
> > 
> > This is kind of a pain for us since we would like to include ethernet.h in 
> > order to get some other definitions, but we can’t because it conflicts with 
> > the same structure name in rte_ether.h.
> 
> I agree, but it will require a deprecation notice as a lot of code uses it.

Yes, it is a very long standing issue.

During the last technical board meeting, we discussed a plan to fix it:
http://mails.dpdk.org/archives/dev/2018-October/116695.html





Re: [dpdk-dev] [PATCH v5 4/4] net/mlx5: support e-switch flow count action

2018-10-23 Thread Ferruh Yigit
On 10/22/2018 9:10 PM, Shahaf Shuler wrote:
> Ferruh,
> 
> Monday, October 22, 2018 12:15 PM, Ferruh Yigit:
>> Subject: Re: [dpdk-dev] [PATCH v5 4/4] net/mlx5: support e-switch flow
>> count action
>>
>> Getting following build error with clang, will not pull from mlx tree until 
>> issue
>> resolved, thanks.
>>
>> .../drivers/net/mlx5/mlx5_flow_tcf.c:2507:6: warning: cast from 'const struct
>> rtattr *' to 'char *' drops const qualifier [-Wcast-qual]
>>  RTA_DATA(arg), RTA_PAYLOAD(arg));
>>  ^
>> /usr/include/linux/rtnetlink.h:183:42: note: expanded from macro
>> 'RTA_DATA'
>> #define RTA_DATA(rta)   ((void*)(((char*)(rta)) + RTA_LENGTH(0)))
>>  ^
>> .../drivers/net/mlx5/mlx5_flow_tcf.c:2593:31: warning: cast from 'const
>> struct nlmsghdr *' to 'char *' drops const qualifier [-Wcast-qual]
>> struct tcmsg *t = NLMSG_DATA(nlh);
> 
> I updated next-net-mlx with the fix for this issue. Can you confirm it is 
> fixed? 

I confirm it is good, pulled from next-net-mlx.


Re: [dpdk-dev] [PATCH v6 05/13] telemetry: add client feature and sockets

2018-10-23 Thread Laatz, Kevin

Thanks for the review Mattias.

Will address the comments both here and in the 4/13 review.

Best regards,

Kevin


On 22/10/2018 15:05, Mattias Rönnblom wrote:

On 2018-10-22 13:00, Kevin Laatz wrote:


  @@ -131,6 +217,38 @@ rte_telemetry_initial_accept(struct 
telemetry_impl *telemetry)

  }
    static int32_t
+rte_telemetry_read_client(struct telemetry_impl *telemetry)
+{
+    char buf[BUF_SIZE];
+    int ret, buffer_read = 0;


No need to set it to zero.


+
+    buffer_read = read(telemetry->accept_fd, buf, BUF_SIZE-1);
+
+    if (buffer_read == -1) {
+    TELEMETRY_LOG_ERR("Read error");
+    return -1;
+    } else if (buffer_read == 0) {
+    goto close_socket;
+    } else {


I would have moved the 'ret' variable to this scope.



+static int32_t
+rte_telemetry_read_client_sockets(struct telemetry_impl *telemetry)
+{
+    telemetry_client *client;
+    char client_buf[BUF_SIZE];
+    int bytes;
+
+    TAILQ_FOREACH(client, &telemetry->client_list_head, client_list) {
+    bytes = read(client->fd, client_buf, BUF_SIZE-1);
+    client_buf[bytes] = '\0';


read() might return -1, and you'll be writing out-of-bounds.



+int32_t
+rte_telemetry_parse_client_message(struct telemetry_impl *telemetry, 
char *buf)

+{
+    int ret, action_int;
+    json_error_t error;
+    json_t *root = json_loads(buf, 0, &error);
+
+    if (root == NULL) {
+    TELEMETRY_LOG_WARN("Could not load JSON object from data 
passed in : %s",

+    error.text);
+    goto fail;
+    } else if (!json_is_object(root)) {
+    TELEMETRY_LOG_WARN("JSON Request is not a JSON object");
+    json_decref(root);
+    goto fail;
+    }
+
+    json_t *action = json_object_get(root, "action");
+    if (action == NULL) {
+    TELEMETRY_LOG_WARN("Request does not have action field");
+    goto fail;
+    } else if (!json_is_integer(action)) {
+    TELEMETRY_LOG_WARN("Action value is not an integer");
+    goto fail;
+    }
+
+    json_t *command = json_object_get(root, "command");
+    if (command == NULL) {
+    TELEMETRY_LOG_WARN("Request does not have command field");
+    goto fail;
+    } else if (!json_is_string(command)) {
+    TELEMETRY_LOG_WARN("Command value is not a string");
+    goto fail;
+    }
+
+    action_int = json_integer_value(action);
+    if (action_int != ACTION_POST) {
+    TELEMETRY_LOG_WARN("Invalid action code");
+    goto fail;
+    }
+
+    if (strcmp(json_string_value(command), "clients") != 0) {
+    TELEMETRY_LOG_WARN("Invalid command");
+    goto fail;
+    }
+
+    json_t *data = json_object_get(root, "data");
+    if (data == NULL) {
+    TELEMETRY_LOG_WARN("Request does not have data field");
+    goto fail;
+    }
+
+    json_t *client_path = json_object_get(data, "client_path");
+    if (client_path == NULL) {
+    TELEMETRY_LOG_WARN("Request does not have client_path field");
+    goto fail;
+    }
+
+    if (!json_is_string(client_path)) {
+    TELEMETRY_LOG_WARN("Client_path value is not a string");
+    goto fail;
+    }
+
+    ret = rte_telemetry_register_client(telemetry,
+    json_string_value(client_path));
+    if (ret < 0) {
+    TELEMETRY_LOG_ERR("Could not register client");
+    telemetry->register_fail_count++;
+    goto fail;
+    }
+


You're leaking the root object here, but maybe that's fixed in 
subsequent patches.



+    return 0;
+
+fail:
+    TELEMETRY_LOG_WARN("Client attempted to register with invalid 
message");

+    return -1;
+}
+




Re: [dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds symmetric capabilities

2018-10-23 Thread Joseph, Anoob
Hi Thomas,

I had replaced macro with functions when I revised the patch. But when more 
devices (with varying capabilities) need to be supported, this can get 
complicated. Macro approach would be simpler in that case. Right now QAT has 
macros and we would like to stick to what is being followed in the community.

Following would be the use case for macros,

#define QAT_BASE_GEN1_SYM_CAPABILITIES, \
{   CAPABILITES,\
... \
}

#define QAT_EXTRA_GEN2_SYM_CAPABILITIES \
{   CAPABILITES,\
... \
}

static const struct rte_cryptodev_capabilities qat_gen1_sym_capabilities[] = {
QAT_BASE_GEN1_SYM_CAPABILITIES,
RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
};

static const struct rte_cryptodev_capabilities qat_gen2_sym_capabilities[] = {
QAT_BASE_GEN1_SYM_CAPABILITIES,
QAT_EXTRA_GEN2_SYM_CAPABILITIES,
RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
};

Without the macros, we will be required to populate the array there itself and 
would mean repetition of GEN1 capabilities. Either approach is fine for us, but 
this could complicate things when we add support for ASYM. Since QAT is already 
doing this, is it fine to move to that approach as we add support for ASYM? Or 
if QAT is to follow any other scheme, I'm fine with adopting that as well. 
Whatever is simple and uniform would work. 

Thanks,
Anoob

> -Original Message-
> From: Thomas Monjalon 
> Sent: 22 October 2018 12:22
> To: Joseph, Anoob 
> Cc: Trahe, Fiona ; dev@dpdk.org; Akhil Goyal
> ; De Lara Guarch, Pablo
> ; Murthy, Nidadavolu
> ; Jacob, Jerin
> ; Athreya, Narayana Prasad
> ; Dwivedi, Ankur
> ; Dabilpuram, Nithin
> ; Jayaraman, Ragothaman
> ; Srinivasan, Srisivasubramanian
> ; Tejasree, Kondoj
> 
> Subject: Re: [dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds symmetric
> capabilities
> 
> External Email
> 
> 22/10/2018 05:49, Joseph, Anoob:
> > Hi Fiona,
> >
> > I do agree that your solution seems to be a neat way for organizing
> capabilities. But Akhil & Thomas were against that idea and we had to come up
> with one array with all capabilities. This would not scale well when we start
> supporting devices with varying capabilities.
> >
> > If your plan is to follow the same approach for asym support, maybe we will
> also follow suit and submit the required patches.
> >
> > @Akhil, Thomas, thoughts?
> 
> Generally speaking, macros are bad.
> 
> Why cannot you just write functions?
> I don't understand your issue.
> 



[dpdk-dev] [PATCH] devtools: disable coloring of git-log output

2018-10-23 Thread Ali Alnubani
Colored git-log outputs will have escape characters
appended to hashes, causing check-git-log.sh to fail
with `error: malformed object name ?[33m?[m` when doing
`git tag -l --contains `.

Signed-off-by: Ali Alnubani 
---
 devtools/git-log-fixes.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/git-log-fixes.sh b/devtools/git-log-fixes.sh
index e37ee2260..464b0a41a 100755
--- a/devtools/git-log-fixes.sh
+++ b/devtools/git-log-fixes.sh
@@ -94,7 +94,7 @@ stable_tag () # 
fi
 }
 
-git log --oneline --reverse $range |
+git log --no-color --oneline --reverse $range |
 while read id headline ; do
origins=$(origin_filter $id)
stable=$(stable_tag $id)
-- 
2.19.1



Re: [dpdk-dev] [PATCH v8 1/4] hash: fix race condition in iterate

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:39:45AM -0700, Yipeng Wang wrote:
> In rte_hash_iterate, the reader lock did not protect the
> while loop which checks empty entry. This created a race
> condition that the entry may become empty when enters
> the lock, then a wrong key data value would be read out.
> 
> This commit reads out the position in the while condition,
> which makes sure that the position will not be changed
> to empty before entering the lock.
> 
> Fixes: f2e3001b53ec ("hash: support read/write concurrency")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yipeng Wang 
> Reported-by: Honnappa Nagarahalli 
> Reviewed-by: Honnappa Nagarahalli 
> Acked-by: Dharmik Thakkar 
> ---
Acked-by: Bruce Richardson 


Re: [dpdk-dev] [PATCH v3 4/4] ethdev: support MAC address as iterator filter

2018-10-23 Thread Thomas Monjalon
23/10/2018 10:33, Ananyev, Konstantin:
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > 22/10/2018 23:24, Ananyev, Konstantin:
> > > From: Thomas Monjalon
> > > > 22/10/2018 15:37, Andrew Rybchenko:
> > > > > On 10/22/18 4:15 PM, Thomas Monjalon wrote:
> > > > > > The MAC addresses of a port can be matched with devargs.
> > > > > >
> > > > > > As the conflict between rte_ether.h and netinet/ether.h is not 
> > > > > > resolved,
> > > > > > the MAC parsing is done with a rte_cmdline function.
> > > > > > As a result, cmdline library becomes a dependency of ethdev.
> > > > > >
> > > > > > Signed-off-by: Thomas Monjalon 
> > > > >
> > > > > I'd like to share my thought about a new dependency.
> > > > > Looking at cmdline I think that it is a bad and strange
> > > > > dependency for kvargs. IMHO, even duplication of the
> > > > > code to parse MAC address it less evil in this case.
> > > >
> > > > cmdline is not a dependency for kvargs.
> > > > I have added it as a dependency for ethdev.
> > > >
> > > > > May be it is possible to provide internal wrapper
> > > > > which is implemented using ether_aton_r() and located
> > > > > in a separate C file which does not include rte_ether.h etc?
> > > >
> > > > I raised the issue in technical board and it has been decided to fix the
> > > > conflict with libc in the next release (with Olivier's help).
> > > > So Bruce and me decided to use cmdline function in the meantime.
> > >
> > >  As I can see, cmdline_parse_etheraddr() uses
> > > static struct ether_addr *
> > > my_ether_aton(const char *a)
> > > internally.
> > > Why not to make it public, rename to rte_ethet_aton(),
> > > and move into rte_net?
> > > And use that one instead?
> > > Later if/when we'll have name conflict with libc resolved,
> > > It can become just a wrapper around ether_aton_r().
> > > Konstantin
> > 
> > Several reasons:
> > - It would be one more (useless) wrapper
> 
> Well, it would be *when* will have libc naming conflict resolved.
> Till that it would be pretty useful I think.

It is planned to be fixed in the next release.

> > - cmdline_parse_etheraddr is already used in several places
> 
> As I can see right now it is used just by bond PMD:
> $ find lib drivers -type f | xargs grep -l cmdline_parse_etheraddr | grep -v 
> librte_cmdline
> drivers/net/bonding/rte_eth_bond_args.c

It is also used in examples:

examples/bond/main.c
examples/ethtool/ethtool-app/ethapp.c
examples/l3fwd/main.c
examples/performance-thread/l3fwd-thread/main.c

> Again if that function is *really* used in several places to convert string 
> to mac,
> then I suppose it is an indication that rte_ether_aton() would be useful.
> Without forcing cmdline dependency.

I don't like wrappers and reinventing the wheel.
If we introduce this new wrapper, then we will have to deprecate it,
and break the API to remove it.

> > - ether_aton_r and my_ether_aton may have a different behavior
> 
> Could you elaborate here?
> What exactly would be different?

The error path might be slightly different,
and...

> Both supposed to convert string to ether addr.
> If our function can't do that properly, then I think it is a bug in our side.
> If ether_aton_r() just supports extra formats(::), then it
> would be extension to current behavior.

... yes there is this extension.

> > When the libc conflict will be solved, I prefer replacing uses of
> > cmdline_parse_etheraddr one by one.
> 
> We can do the same with rte_ether_aton() too, if we really want to.

At the price of breaking the API again.





Re: [dpdk-dev] [PATCH v8 2/4] hash: add extendable bucket feature

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:39:46AM -0700, Yipeng Wang wrote:
> In use cases that hash table capacity needs to be guaranteed,
> the extendable bucket feature can be used to contain extra
> keys in linked lists when conflict happens. This is similar
> concept to the extendable bucket hash table in packet
> framework.
> 
> This commit adds the extendable bucket feature. User can turn
> it on or off through the extra flag field during table
> creation time.
> 
> Extendable bucket table composes of buckets that can be
> linked list to current main table. When extendable bucket
> is enabled, the hash table load can always achieve 100%.
> In other words, the table can always accommodate the same
> number of keys as the specified table size. This provides
> 100% table capacity guarantee.
> 
> Although keys ending up in the ext buckets may have longer
> look up time, they should be rare due to the cuckoo
> algorithm.
> 
> Signed-off-by: Yipeng Wang 
> Acked-by: Dharmik Thakkar 
> ---
Acked-by: Bruce Richardson 


Re: [dpdk-dev] [dpdk-users] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs

2018-10-23 Thread Olivier Matz
Hi,

You are right, the current code does not take IP or IPv6 options
in account. I think this should be considered as a bug.

The fix for IPv4 is not complicated, I did a quick draft here:
http://git.droids-corp.org/?p=dpdk.git;a=commitdiff;h=96a6978ef6814e1450e1bd65fbce91c3d85b3121

For IPv6, it is more complex than expected:
https://tools.ietf.org/html/rfc2460.html#section-8.1

- we need to skip extension headers
- we need to parse routing headers and use the proper destination
  address in the pseudo header checksum

This makes me think that the API is not adequate. Asking the user
to provide the headers in a contiguous memory without specifying
the length is quite dangerous, especially if the header comes from
outside, as it can trigger out of bound accesses.

I wonder if we shouldn't switch to a mbuf based API instead, and
deprecate the old one.

Thoughts?
Olivier


On Mon, Oct 22, 2018 at 09:03:14AM +0100, Ferruh Yigit wrote:
> On 10/20/2018 7:30 AM, Shyam Shrivastav wrote:
> > Yes you are right, I misread, following code (ipv4 case) assumes no ip
> > options while calculating pseudo hdr length field
> > 
> > psd_hdr.len = rte_cpu_to_be_16(
> > (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length)
> > - sizeof(struct ipv4_hdr)));
> > 
> > should be
> > 
> >  psd_hdr.len = rte_cpu_to_be_16(
> > (uint16_t)(rte_be_to_cpu_16(ipv4_hdr->total_length)
> > - (ipv4_hdr->version_ihl & 0x0f)*4)));
> 
> cc'ed maintainer of the that part, Olivier.
> 
> > 
> > On Sat, Oct 20, 2018 at 11:44 AM lidejun  wrote:
> > 
> >> I mean DPDK APIs do not exclude ipv4 options or ipv6 extension headers, it
> >> is bug?
> >>
> >>
> >>
> >> *发件人:* Shyam Shrivastav [mailto:shrivastav.sh...@gmail.com]
> >> *发送时间:* 2018年10月20日 13:23
> >> *收件人:* lidejun 
> >> *抄送:* users ; dev@dpdk.org; Lichunhe (Cloud Networking) <
> >> lichu...@huawei.com>; Wangliefeng 
> >> *主题:* Re: [dpdk-dev] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs
> >>
> >>
> >>
> >> Realized my answer is confusing, I meant to say that code is correct as
> >> pseudo ipv4/ipv6 headers for the purpose of checksum calculations doesn't
> >> include options or extension headers, see udp wiki or corresponding rfcs
> >>
> >>
> >>
> >> https://en.wikipedia.org/wiki/User_Datagram_Protocol
> >>
> >>
> >>
> >> On Sat, Oct 20, 2018 at 10:42 AM Shyam Shrivastav <
> >> shrivastav.sh...@gmail.com> wrote:
> >>
> >> that is correct , pseudo header doesn't include ipv4 options or ipv6
> >> extension headers ..
> >>
> >>
> >>
> >> On Sat, Oct 20, 2018 at 9:02 AM lidejun  wrote:
> >>
> >> Has anybody used the following two APIs calculating ipv4&ipv6 tcp/udp
> >> pseudo header checksum?
> >>
> >> 1.rte_ipv4_phdr_cksum
> >>
> >> 2.rte_ipv6_phdr_cksum
> >> The ipv4 version does not exclude ip options and ipv6 version does not
> >> exclude extersion headers.
> >>
> >>
> 


Re: [dpdk-dev] [PATCH v2 09/33] crypto/octeontx: adds symmetric capabilities

2018-10-23 Thread Thomas Monjalon
23/10/2018 10:48, Joseph, Anoob:
> From: Thomas Monjalon 
> > 22/10/2018 05:49, Joseph, Anoob:
> > > Hi Fiona,
> > >
> > > I do agree that your solution seems to be a neat way for organizing
> > capabilities. But Akhil & Thomas were against that idea and we had to come 
> > up
> > with one array with all capabilities. This would not scale well when we 
> > start
> > supporting devices with varying capabilities.
> > >
> > > If your plan is to follow the same approach for asym support, maybe we 
> > > will
> > also follow suit and submit the required patches.
> > >
> > > @Akhil, Thomas, thoughts?
> > 
> > Generally speaking, macros are bad.
> > 
> > Why cannot you just write functions?
> > I don't understand your issue.
> 
> I had replaced macro with functions when I revised the patch. But when more 
> devices (with varying capabilities) need to be supported, this can get 
> complicated. Macro approach would be simpler in that case. Right now QAT has 
> macros and we would like to stick to what is being followed in the community.
> 
> Following would be the use case for macros,
> 
> #define QAT_BASE_GEN1_SYM_CAPABILITIES,   \
> { CAPABILITES,\
>   ... \
> }
> 
> #define QAT_EXTRA_GEN2_SYM_CAPABILITIES   \
> { CAPABILITES,\
>   ... \
> }
> 
> static const struct rte_cryptodev_capabilities qat_gen1_sym_capabilities[] = {
>   QAT_BASE_GEN1_SYM_CAPABILITIES,
>   RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
> };
> 
> static const struct rte_cryptodev_capabilities qat_gen2_sym_capabilities[] = {
>   QAT_BASE_GEN1_SYM_CAPABILITIES,
>   QAT_EXTRA_GEN2_SYM_CAPABILITIES,
>   RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
> };
> 
> Without the macros, we will be required to populate the array there itself 
> and would mean repetition of GEN1 capabilities. Either approach is fine for 
> us, but this could complicate things when we add support for ASYM. Since QAT 
> is already doing this, is it fine to move to that approach as we add support 
> for ASYM? Or if QAT is to follow any other scheme, I'm fine with adopting 
> that as well. Whatever is simple and uniform would work.

You can use simple assignments in functions and avoid repetition.

There is a warning in checkpatch about macros.
I think we should be more strict with this warning.




Re: [dpdk-dev] [PATCH v8 3/4] test/hash: implement extendable bucket hash test

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:39:47AM -0700, Yipeng Wang wrote:
> This commit changes the current rte_hash unit test to
> test the extendable table feature and performance.
> 
> Signed-off-by: Yipeng Wang 
> Reviewed-by: Honnappa Nagarahalli 
> Acked-by: Dharmik Thakkar 
> ---
Acked-by: Bruce Richardson 



Re: [dpdk-dev] [PATCH] test/test_external_mem: fix loop initialization

2018-10-23 Thread Thomas Monjalon
23/10/2018 09:35, Ali Alnubani:
> Loop initial declarations are only allowed in C99 mode,
> which causes a compilation failure on non C99 compliant systems:
> 
> test/test/test_external_mem.c: In function ‘test_external_mem’:
> test/test/test_external_mem.c:375:2: error: ‘for’
> loop initial declarations are only allowed in C99 mode
>   for (int i = 0; i < n_pages; i++) {
>   ^
> 
> Reproduces on RHEL 7.4 with GCC 4.8.5 20150623 (Red Hat 4.8.5-16).
> 
> Fixes: b270daa43b3d ("test: support external memory")
> Cc: anatoly.bura...@intel.com
> 
> Signed-off-by: Ali Alnubani 

There was already a patch from Dan Gora:
http://patches.dpdk.org/patch/46921/





Re: [dpdk-dev] [PATCH v8 4/4] hash: use partial-key hashing

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:39:48AM -0700, Yipeng Wang wrote:
> This commit changes the hashing mechanism to "partial-key
> hashing" to calculate bucket index and signature of key.
> 
> This is  proposed in Bin Fan, et al's paper
> "MemC3: Compact and Concurrent MemCache with Dumber Caching
> and Smarter Hashing". Basically the idea is to use "xor" to
> derive alternative bucket from current bucket index and
> signature.
> 
> With "partial-key hashing", it reduces the bucket memory
> requirement from two cache lines to one cache line, which
> improves the memory efficiency and thus the lookup speed.
> 
> Signed-off-by: Yipeng Wang 
> Reviewed-by: Honnappa Nagarahalli 
> Acked-by: Dharmik Thakkar 

Acked-by: Bruce Richardson 



Re: [dpdk-dev] [PATCH] test: fix compilation error in external memory test

2018-10-23 Thread Thomas Monjalon
17/10/2018 12:12, Burakov, Anatoly:
> On 17-Oct-18 1:22 AM, Dan Gora wrote:
> > Fix a compilation error in test_external_mem.c:
> > 
> >CC test_external_mem.o
> > test_external_mem.c: In function ‘test_external_mem’:
> > test_external_mem.c:375:2: error: ‘for’ loop initial declarations are
> > only allowed in C99 mode
> > 
> >for (int i = 0; i < n_pages; i++) {
> >^
> > test_external_mem.c:375:2: note: use option -std=c99 or -std=gnu99 to
> > compile your code
> > 
> > Fixes: b270daa43b3d ("test: support external memory")
> > Cc: anatoly.bura...@intel.com
> > 
> > Signed-off-by: Dan Gora 
> > ---
> 
> Oops
> 
> Acked-by: Anatoly Burakov 

Applied, thanks




Re: [dpdk-dev] [PATCH] mbuf: add IGMP packet type

2018-10-23 Thread Olivier Matz
On Mon, Aug 27, 2018 at 06:08:35PM +0530, Jerin Jacob wrote:
> Add support for IGMP packet type.
> 
> Signed-off-by: Jerin Jacob 

Acked-by: Olivier Matz 


Re: [dpdk-dev] [PATCH] mem: fix resource leak

2018-10-23 Thread Thomas Monjalon
22/10/2018 22:34, Thomas Monjalon:
> 22/10/2018 14:57, Anatoly Burakov:
> > Segment preallocation code allocates an array of structures on the
> > heap but does not free the memory afterwards. Fix it by freeing it
> > at the end of the function, and changing control flow to always go
> > through that code path.
> > 
> > Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")
> > 
> > Coverity ID: 323524
> > 
> > Cc: sta...@dpdk.org
> 
> The right order for these tags is:
> 
> Coverity issue: 323524
> Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")
> Cc: sta...@dpdk.org
> 
> > Signed-off-by: Anatoly Burakov 

Applied, thanks




Re: [dpdk-dev] [PATCH] app/testpmd: support more types for flow RSS

2018-10-23 Thread Zhao1, Wei
Hi, Iremonger  Bernard

> -Original Message-
> From: Iremonger, Bernard
> Sent: Monday, October 22, 2018 6:45 PM
> To: Zhao1, Wei ; dev@dpdk.org
> Cc: Zhang, Qi Z ; sta...@dpdk.org; Peng, Yuan
> ; Zhao1, Wei 
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: support more types for flow
> RSS
> 
> Hi Zhao,
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Monday, October 22, 2018 8:49 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Qi Z ; sta...@dpdk.org; Peng, Yuan
> > ; Zhao1, Wei 
> > Subject: [dpdk-dev] [PATCH] app/testpmd: support more types for flow
> > RSS
> >
> > Some user and tester require flow RSS to support more types, so and
> > "all" and
> 
> typo in commit message "and "all"  " should be "add "all""

Yes, I will commit v2 for that. 
Thank you!

> 
> > "none" to make configuration more easy for users.
> >
> > Signed-off-by: Wei Zhao 
> > ---
> >  app/test-pmd/config.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > bf3cd0a..f068da1 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -74,6 +74,10 @@ static const struct {  };
> >
> >  const struct rss_type_info rss_type_table[] = {
> > +   { "all", ETH_RSS_IP | ETH_RSS_TCP |
> > +   ETH_RSS_UDP | ETH_RSS_SCTP |
> > +   ETH_RSS_L2_PAYLOAD },
> > +   { "none", 0 },
> > { "ipv4", ETH_RSS_IPV4 },
> > { "ipv4-frag", ETH_RSS_FRAG_IPV4 },
> > { "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP },
> > --
> > 2.7.5
> 
> The "all" and "none" options are already documented in testpmd.
> This looks like a fix, if so should there be a fixes line?
> 

Not a fix, this is used by rte_flow command line, not old CLI.

> Regards,
> 
> Bernard.
> 



Re: [dpdk-dev] [PATCH] vhost: fix vector filling for packed ring

2018-10-23 Thread Maxime Coquelin




On 10/23/2018 08:07 AM, Tiwei Bie wrote:

We should return the length of the buffers described by
the current descriptor chain after filling the buffer
vector. So we need to zero the *len first.

Fixes: 2f3225a7d69b ("vhost: add vector filling support for packed ring")
Cc: sta...@dpdk.org

Signed-off-by: Tiwei Bie 
---
  lib/librte_vhost/virtio_net.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 48228f16b..4e6c35d56 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -521,6 +521,7 @@ fill_vec_buf_packed(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
return -1;
  
  	*desc_count = 0;

+   *len = 0;
  
  	while (1) {

if (unlikely(vec_id >= BUF_VECTOR_MAX))



Applied to dpdk-next-virtio/master

Thanks,
Maxime


Re: [dpdk-dev] [PATCH v2] vhost/crypto: fix build without cryptodev (shared lib)

2018-10-23 Thread Maxime Coquelin




On 10/16/2018 10:47 PM, Timothy Redaelli wrote:

Currently it's not possible to build DPDK as shared library with
cryptodev disabled since vhost is trying to link with rte_crypto,
but rte_crypto and rte_hash are only needed when you build vhost_crypto
and so only when cryptodev is enabled.

This patch fix this by linking rte_vhost with rte_crypto and rte_hash only
when cryptodev is enabled.

Fixes: b4ca81298613 ("vhost/crypto: fix build without cryptodev")
Fixes: 939066d96563 ("vhost/crypto: add public function implementation")
Cc: sta...@dpdk.org

Signed-off-by: Timothy Redaelli 
Reviewed-by: Maxime Coquelin 
---
v2: Fix the second 'Fixes' line. I wrongly copied that commit id from
commit b4ca81298613, but it seems it's a commit not contained in any branch
(as reported by Maxime and check-git-log.sh).
---
  lib/librte_vhost/Makefile | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Applied to dpdk-next-virtio/master

Thanks,
Maxime


Re: [dpdk-dev] [PATCH v2 1/3] common/dpaax: reduce logging level

2018-10-23 Thread Jerin Jacob
-Original Message-
> Date: Wed, 17 Oct 2018 10:10:34 +
> From: Shreyansh Jain 
> To: "tho...@monjalon.net" 
> CC: "dev@dpdk.org" , Shreyansh Jain 
> Subject: [dpdk-dev] [PATCH v2 1/3] common/dpaax: reduce logging level
> x-mailer: git-send-email 2.17.1
> 
> 
> DPAAX is a library used by various NXP drivers. In case of non-NXP
> environment, this start spewing message about unavailability of
> necessary environment.
> 
> This patch reduces the log level for certain messages as well as
> reduces overall log-level. As a library, these message are not
> necessarily relevant at higher log level, either.
> 
> Signed-off-by: Shreyansh Jain 

Acked-by: Jerin Jacob 
Test-by: Jerin Jacob 



[dpdk-dev] [PATCH v2] app/testpmd: support more types for flow RSS

2018-10-23 Thread Wei Zhao
Some user and tester require flow RSS to support more types,
so add "all" and "none" to make configuration more easy for users.

Tested-by: Peng Yuan 
Signed-off-by: Wei Zhao 

---

v2:
-fix typo in commit message.
---
 app/test-pmd/config.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index bf3cd0a..f068da1 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -74,6 +74,10 @@ static const struct {
 };
 
 const struct rss_type_info rss_type_table[] = {
+   { "all", ETH_RSS_IP | ETH_RSS_TCP |
+   ETH_RSS_UDP | ETH_RSS_SCTP |
+   ETH_RSS_L2_PAYLOAD },
+   { "none", 0 },
{ "ipv4", ETH_RSS_IPV4 },
{ "ipv4-frag", ETH_RSS_FRAG_IPV4 },
{ "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP },
-- 
2.7.5



Re: [dpdk-dev] [PATCH v2 2/3] bus/fslmc: ignore dpaax pa-va table errors

2018-10-23 Thread Jerin Jacob
-Original Message-
> Date: Wed, 17 Oct 2018 10:10:36 +
> From: Shreyansh Jain 
> To: "tho...@monjalon.net" 
> CC: "dev@dpdk.org" , Shreyansh Jain 
> Subject: [dpdk-dev] [PATCH v2 2/3] bus/fslmc: ignore dpaax pa-va table
>  errors
> x-mailer: git-send-email 2.17.1
> 
> 
> Presence of PA-VA Table is transparent to the drivers. Ignoring the
> return values from table update call.
> 
> Signed-off-by: Shreyansh Jain 

Acked-by: Jerin Jacob 

> ---
>  drivers/bus/fslmc/fslmc_bus.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
> index 5ba5ce96b..db3026f4e 100644
> --- a/drivers/bus/fslmc/fslmc_bus.c
> +++ b/drivers/bus/fslmc/fslmc_bus.c
> @@ -386,11 +386,11 @@ rte_fslmc_probe(void)
>  * This has to be done before probe as some device initialization
>  * (during) probe allocate memory (dpaa2_sec) which needs to be pinned
>  * to this table.
> +*
> +* Error is ignored as relevant logs are handled within dpaax and
> +* handling for unavailable dpaax table too is transparent to caller.
>  */
> -   ret = dpaax_iova_table_populate();
> -   if (ret) {
> -   DPAA2_BUS_WARN("PA->VA Translation table not available;");
> -   }
> +   dpaax_iova_table_populate();
> 
> TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) {
> TAILQ_FOREACH(drv, &rte_fslmc_bus.driver_list, next) {
> --
> 2.17.1
> 


Re: [dpdk-dev] [PATCH v2] app/testpmd: support more types for flow RSS

2018-10-23 Thread Zhao1, Wei
Add Bernard into mailing list.


> -Original Message-
> From: Zhao1, Wei
> Sent: Tuesday, October 23, 2018 5:21 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z ; sta...@dpdk.org; Peng, Yuan
> ; Zhao1, Wei 
> Subject: [PATCH v2] app/testpmd: support more types for flow RSS
> 
> Some user and tester require flow RSS to support more types, so add "all"
> and "none" to make configuration more easy for users.
> 
> Tested-by: Peng Yuan 
> Signed-off-by: Wei Zhao 
> 
> ---
> 
> v2:
> -fix typo in commit message.
> ---
>  app/test-pmd/config.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> bf3cd0a..f068da1 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -74,6 +74,10 @@ static const struct {  };
> 
>  const struct rss_type_info rss_type_table[] = {
> + { "all", ETH_RSS_IP | ETH_RSS_TCP |
> + ETH_RSS_UDP | ETH_RSS_SCTP |
> + ETH_RSS_L2_PAYLOAD },
> + { "none", 0 },
>   { "ipv4", ETH_RSS_IPV4 },
>   { "ipv4-frag", ETH_RSS_FRAG_IPV4 },
>   { "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP },
> --
> 2.7.5



Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508

2018-10-23 Thread Ferruh Yigit
On 10/23/2018 8:09 AM, Shreyansh Jain wrote:
> Besides the comment I sent before about 'Fixes' before sign-off, a 
> single trivial comment inline ...
> 
> On Tuesday 23 October 2018 07:20 AM, Rosen Xu wrote:
>> This patch fixes rte_eal_hotplug_add without checking return value issue
>>
>> Signed-off-by: Rosen Xu 
>> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
>> Cc: rosen...@intel.com
>> ---
>>   drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c 
>> b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>> index 3fed057..32e318f 100644
>> --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>> +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>> @@ -542,6 +542,7 @@
>>  int port;
>>  char *name = NULL;
>>  char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
>> +int ret = -1;
>>   
>>  devargs = dev->device.devargs;
>>   
>> @@ -583,7 +584,7 @@
>>  snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
>>  port, name);
>>   
>> -rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>> +ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>>  dev_name, devargs->args);
> 
> Ideally, the function argument spreading on next line should start 
> underneath the previous arguments - something like:
> 
>   ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
> dev_name, devargs->args);

Hi Shreyansh,

According dpdk coding convention [1], indentation done by hard tab, code seems
inline with coding convention, only perhaps can be done single tab instead of
double.

And to remind again, I am not for syntax discussions but just defining one and
consistently follow it .

[1]
https://doc.dpdk.org/guides/contributing/coding_style.html#c-indentation
https://doc.dpdk.org/guides/contributing/coding_style.html#prototypes

> 
> But, in this file this is not being done at multiple places (for 
> example, the snprintf in this code snippet). So, either you can ignore 
> this comment, or fix it for just this change.
> 
>>   end:
>>  if (kvlist)
>> @@ -591,7 +592,7 @@
>>  if (name)
>>  free(name);
>>   
>> -return 0;
>> +return ret;
>>   }
>>   
>>   static int
>>
> 
> Otherwise, the patch is simple enough.
> 
> Acked-by: Shreyansh Jain 
> 



Re: [dpdk-dev] [PATCH v6 2/3] app/testpmd: add MPLSoUDP encapsulation

2018-10-23 Thread Ferruh Yigit
On 10/22/2018 6:38 PM, Ori Kam wrote:
> MPLSoUDP is an example for L3 tunnel encapsulation.
> 
> L3 tunnel type is a tunnel that is missing the layer 2 header of the
> inner packet.
> 
> Example for MPLSoUDP tunnel:
> ETH / IPV4 / UDP / MPLS / IP / L4..L7
> 
> In order to encapsulate such a tunnel there is a need to remove L2 of
> the inner packet and encap the remaining tunnel, this is done by
> applying 2 rte flow commands l2_decap followed by mplsoudp_encap.
> Both commands must appear in the same flow, and from the point of the
> packet it both actions are applyed at the same time. (There is no part
> where a packet doesn't have L2 header).
> 
> Decapsulating such a tunnel works the other way, first we need to decap
> the outer tunnel header and then apply the new L2.
> So the commands will be mplsoudp_decap / l2_encap
> 
> Due to the complex encapsulation of MPLSoUDP and L2  flow actions and
> based on the fact testpmd does not allocate memory, this patch adds a new
> command in testpmd to initialise a global structurs containing the
> necessary information to make the outer layer of the packet.  This same
> global structures will then be used by the flow commands in testpmd when
> the action mplsoudp_encap, mplsoudp_decap, l2_encap, l2_decap, will be
> parsed, at this point, the conversion into such action becomes trivial.
> 
> The l2_encap and l2_decap actions can also be used for other L3 tunnel
> types.
> 
> Signed-off-by: Ori Kam 

Reviewed-by: Ferruh Yigit 


Re: [dpdk-dev] [PATCH v6 3/3] app/testpmd: add MPLSoGRE encapsulation

2018-10-23 Thread Ferruh Yigit
On 10/22/2018 6:38 PM, Ori Kam wrote:
> Example for MPLSoGRE tunnel:
> ETH / IPV4 / GRE / MPLS / IP / L4..L7
> 
> In order to encapsulate such a tunnel there is a need to remove L2 of
> the inner packet and encap the remaining tunnel, this is done by
> applying 2 rte flow commands l2_decap followed by mplsogre_encap.
> Both commands must appear in the same flow, and from the point of the
> packet it both actions are applyed at the same time. (There is no part
> where a packet doesn't have L2 header).
> 
> Decapsulating such a tunnel works the other way, first we need to decap
> the outer tunnel header and then apply the new L2.
> So the commands will be mplsogre_decap / l2_encap
> 
> Due to the complex encapsulation of MPLSoGRE flow action and
> based on the fact testpmd does not allocate memory, this patch adds a
> new command in testpmd to initialise a global structure containing the
> necessary information to make the outer layer of the packet.  This same
> global structures will then be used by the flow commands in testpmd when
> the action mplsogre_encap, mplsogre_decap, will be parsed, at this point,
> the conversion into such action becomes trivial.
> 
> Signed-off-by: Ori Kam 

Reviewed-by: Ferruh Yigit 


Re: [dpdk-dev] [PATCH v6 0/3] ethdev: add generic raw tunnel encapsulation

2018-10-23 Thread Ferruh Yigit
On 10/22/2018 6:38 PM, Ori Kam wrote:
> This series implement the raw tunnel encapsulation actions
> and is based on rfc [1] "add generic L2/L3 tunnel encapsulation actions"
> 
> Currenlty the encap/decap actions only support encapsulation
> of VXLAN and NVGRE L2 packets (L2 encapsulation is where
> the inner packet has a valid Ethernet header, while L3 encapsulation
> is where the inner packet doesn't have the Ethernet header).
> In addtion the parameter to to the encap action is a list of rte items,
> this results in 2 extra translation, between the application to the action
> and from the action to the NIC. This results in negetive impact on the
> insertion performance.
> 
> Looking forward there are going to be a need to support many more tunnel
> encapsulations. For example MPLSoGRE, MPLSoUDP.
> Adding the new encapsulation will result in duplication of code.
> For example the code for handling NVGRE and VXLAN are exactly the same,
> and each new tunnel will have the same exact structure.
> 
> This series introduce a raw encapsulation that can support both L2 and L3
> tunnel encapsulation.
> In order to encap l3 tunnel for example MPLSoDUP:
> ETH / IPV4 / UDP / MPLS / IPV4 / L4 .. L7
> When creating the flow rule we add 2 actions, the first one is decap in order
> to remove the original L2 of the packet and then the encap with the tunnel 
> data.
> Decapsulating such a tunnel is done in the following order, first decap the
> outer tunnel and then encap the packet with the L2 header.
> It is important to notice that from the Nic and PMD both actionsn happens
> simultaneously, meaning that at we are always having a valid packet.
> 
> This series also inroduce the following commands for testpmd:
> * l2_encap
> * l2_decap
> * mplsogre_encap
> * mplsogre_decap
> * mplsoudp_encap
> * mplsoudp_decap
> 
> along with helper function to set teh headers that will be used for the 
> actions,
> the same as with vxlan_encap.
> 
> [1]https://mails.dpdk.org/archives/dev/2018-August/109944.html
> 
> v6:
>  * fix compilation error
>  * fix typo.
> 
> v5:
>  * fix typos.
> 
> v4:
>  * convert to raw encap/decap, according to Adrien suggestion.
>  * keep the old vxlan and nvgre encapsulation commands.
> 
> v3:
>  * rebase on tip.
> 
> v2:
>  * add missing decap_l3 structure.
>  * fix typo.
> 
> 
> 
> Ori Kam (3):
>   ethdev: add raw encapsulation action
>   app/testpmd: add MPLSoUDP encapsulation
>   app/testpmd: add MPLSoGRE encapsulation

Series applied to dpdk-next-net/master, thanks.


Re: [dpdk-dev] Memory allocation in dpdk at basic level

2018-10-23 Thread Burakov, Anatoly

On 22-Oct-18 10:08 PM, Avinash Chaurasia wrote:

Hello,
I am not sure whether this is right list for posting this problem. I am
trying to understand how dpdk allocate memory. I tried digging code to
understand memory allocation of DPDK. So far I understood that memory is
allocated from a heap that dpdk maintains. However, this heap must be
allocated at some place. I failed to traceback any function (called from
heap_alloc()) that calls mmap to allocate memory. Please let me know when
this heap is created, which function call does that.
Thanks
Avinash



Hi Avinash,

Here's a very high level overview.

At initialization, we mmap() anonymous memory regions (eal_memory.c 
contains the code - both for legacy and non-legacy mode). Then, we map 
actual pages into that space either at init (in legacy mode) or as 
needed (in non-legacy mode).


In legacy mode, pages are mapped into anonymous memory straight away 
(see legacy init code in eal_memory.c). For non-legacy, page allocation 
happens in eal_memalloc.c on request from malloc (see malloc_heap.c). In 
both cases, newly allocated pages are added to DPDK heap, and from there 
they can be used by the rest of DPDK using rte_malloc or 
rte_memzone_reserve API's.


--
Thanks,
Anatoly


Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching device twice

2018-10-23 Thread Iremonger, Bernard
Hi Thomas,

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Tuesday, October 23, 2018 9:29 AM
> To: dev@dpdk.org
> Cc: gaetan.ri...@6wind.com; ophi...@mellanox.com;
> wis...@mellanox.com; Yigit, Ferruh ;
> arybche...@solarflare.com; Iremonger, Bernard
> 
> Subject: [PATCH v7 7/7] app/testpmd: check not detaching device twice
> 
> The command "port detach" is removing the EAL rte_device of the ethdev
> port specified as parameter.
> 
> After detaching, the pointer, which maps a port to its device, is resetted. 
> This

Typo:  "resetted" should be "reset"

> way, it is possible to check whether a port is still associated to a (not
> removed) device.
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  app/test-pmd/testpmd.c | 24 +---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 14647fa19..d5998fddc 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)  void
> detach_port(portid_t port_id)  {
> + struct rte_device *dev;
> + portid_t sibling;
> +
>   printf("Removing a device...\n");

The functionality of the detach_port() function has changed now to removing a 
device, should the function name be changed to reflect the new functionality.
How about detach_device() instead of detach detach_port().

> 
> + dev = rte_eth_devices[port_id].device;
> + if (dev == NULL) {
> + printf("Device already removed\n");
> + return;
> + }
> +
>   if (ports[port_id].port_status != RTE_PORT_CLOSED) {
>   if (ports[port_id].port_status != RTE_PORT_STOPPED) {
>   printf("Port not stopped\n");
> @@ -2365,15 +2374,24 @@ detach_port(portid_t port_id)
>   port_flow_flush(port_id);
>   }
> 
> - if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
> + if (rte_dev_remove(dev) != 0) {
>   TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);

Should the log message be ( ERR "Failed to detach device %s\n", dev->name) ?

>   return;
>   }
> 
> + /* reset mapping between old ports and removed device */
> + for (sibling = 0; sibling < RTE_MAX_ETHPORTS; sibling++)
> + if (rte_eth_devices[sibling].device == dev) {
> + rte_eth_devices[sibling].device = NULL;
> + if (ports[sibling].port_status != RTE_PORT_CLOSED) {
> + ports[sibling].port_status =
> RTE_PORT_CLOSED;
> + printf("Port %u is closed\n", sibling);
> + }
> + }
> +
>   remove_unused_fwd_ports();
> 
> - printf("Port %u is detached. Now total ports is %d\n",
> - port_id, nb_ports);

How about printf("Device %s is detached, Now total ports is %d\n"
dev->name, nb_ports);
 
> + printf("Now total ports is %d\n", nb_ports);
>   printf("Done\n");
>   return;
>  }
> --
> 2.19.0

Regards,

Bernard.




Re: [dpdk-dev] [PATCH v2 1/7] net/mlx5: e-switch VXLAN configuration and definitions

2018-10-23 Thread Yongseok Koh
On Mon, Oct 15, 2018 at 02:13:29PM +, Viacheslav Ovsiienko wrote:
> This part of patchset adds configuration changes in makefile and
> meson.build for Mellanox MLX5 PMD. Also necessary defenitions
> for VXLAN support are made and appropriate data structures
> are presented.
> 
> Suggested-by: Adrien Mazarguil 
> Signed-off-by: Viacheslav Ovsiienko 
> ---
>  drivers/net/mlx5/Makefile|  80 ++
>  drivers/net/mlx5/meson.build |  32 +++
>  drivers/net/mlx5/mlx5_flow.h |  11 +++
>  drivers/net/mlx5/mlx5_flow_tcf.c | 175 
> +++
>  4 files changed, 298 insertions(+)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index 1e9c0b4..fec7779 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -207,6 +207,11 @@ mlx5_autoconf.h.new: 
> $(RTE_SDK)/buildtools/auto-config-h.sh
>   enum IFLA_PHYS_PORT_NAME \
>   $(AUTOCONF_OUTPUT)
>   $Q sh -- '$<' '$@' \
> + HAVE_IFLA_VXLAN_COLLECT_METADATA \
> + linux/if_link.h \
> + enum IFLA_VXLAN_COLLECT_METADATA \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
>   HAVE_TCA_CHAIN \
>   linux/rtnetlink.h \
>   enum TCA_CHAIN \
> @@ -367,6 +372,81 @@ mlx5_autoconf.h.new: 
> $(RTE_SDK)/buildtools/auto-config-h.sh
>   enum TCA_VLAN_PUSH_VLAN_PRIORITY \
>   $(AUTOCONF_OUTPUT)
>   $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_KEY_ID \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_KEY_ID \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV4_SRC \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV4_SRC \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV4_DST \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV4_DST \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV4_DST_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV4_DST_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV6_SRC \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV6_SRC \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV6_DST \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV6_DST \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_IPV6_DST_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_IPV6_DST_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_UDP_SRC_PORT \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_UDP_SRC_PORT \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_UDP_DST_PORT \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_UDP_DST_PORT \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TC_ACT_TUNNEL_KEY \
> + linux/tc_act/tc_tunnel_key.h \
> + define TCA_ACT_TUNNEL_KEY \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT \
> + linux/tc_act/tc_tunnel_key.h \
> + enum TCA_TUNNEL_KEY_ENC_DST_PORT \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
>   HAVE_TC_ACT_PEDIT \
>   linux/tc_act/tc_pedit.h \
>   enum TCA_PEDIT_KEY_EX_HDR_TYPE_UDP \
> diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> index c192d44..43aabf2 100644
> --- a/drivers/net/mlx5/meson.build
> +++ b/drivers/net/mlx5/meson.build
> @@ -126,6 +126,8 @@ if build
>   'IFLA_PHYS_SWITCH_ID' ],
>

[dpdk-dev] net/mlx5: flow counters support for Linux-rdma v19

2018-10-23 Thread Slava Ovsiienko
Mellanox mlx5 PMD supports Flow counters via Verbs library.
The current implementation is based on the Mellanox proprietary
Verbs library included in MLNX OFED packages. The Flow counter
support is recently added into linux-rdma release (v19),
so the mlx5 PMD update is needed to provide Counter feature
on the base of linux-rdma.

mlx5 PMD can be compiled with MLNX OFED or linux-rdma v19+
and provide flow counters for both.

Signed-off-by: Viacheslav Ovsiienko 

---
v4:
- minor patcset parts reorganization
- rewritten headlines to be more clear
- ifdef blocks minor cleanups

v3:
- http://patches.dpdk.org/patch/47085/
- mlx5 glue issue resolved correctly
- patch is reorganized info small isolated parts
  
v2:
- http://patches.dpdk.org/patch/46989/
- rebased on top of master-net-mlx branch
- new compilation flags are introduced:
- HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V42, kernel/verbs
  library provides the flow counter support in style of
  MLNX_OFED_4.2 to MLNX_OFED_4.4
- HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT_V45, kernel/verbs
  library provides the flow counter support in style of
  MLNX_OFED_4.5 or higher

v1:
- http://patches.dpdk.org/patch/45972/

Viacheslav Ovsiienko (8):
  net/mlx5: fix flow counters creation
  net/mlx5: rename flow counter configuration macro
  net/mlx5: introduce new flow counters configuration macro
  net/mlx5: simplify flow counters support check
  net/mlx5: relocate flow counters query function
  net/mlx5: add new flow counter Verbs API to glue library
  net/mlx5: remove unnecessary structure initializers
  net/mlx5: support new flow counter API

 drivers/net/mlx5/Makefile  |   7 +-
 drivers/net/mlx5/meson.build   |   4 +-
 drivers/net/mlx5/mlx5.c|  12 +-
 drivers/net/mlx5/mlx5.h|   1 -
 drivers/net/mlx5/mlx5_flow.c   |   8 +-
 drivers/net/mlx5/mlx5_flow.h   |   4 +
 drivers/net/mlx5/mlx5_flow_tcf.c   |   5 -
 drivers/net/mlx5/mlx5_flow_verbs.c | 236 -
 drivers/net/mlx5/mlx5_glue.c   |  68 ++-
 drivers/net/mlx5/mlx5_glue.h   |  19 ++-
 10 files changed, 254 insertions(+), 110 deletions(-)

-- 
1.8.3.1



[dpdk-dev] [PATCH v4 2/8] net/mlx5: rename flow counter configuration macro

2018-10-23 Thread Slava Ovsiienko
The HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT is replaced with
HAVE_IBV_DEVICE_COUNTERS_SET_V42. At this stage it is just
macro renaming. This macro is defined if system supports
the "old" Flow counters functionality, MLNX_OFED version
from 4.2 to 4.4 is required.

We need to do this preparation before introducing the new
configuration macro (HAVE_IBV_DEVICE_COUNTERS_SET_V45) for
the "new" Flow counters support.

Both makefile and meson.build are changed.

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/Makefile  |  2 +-
 drivers/net/mlx5/meson.build   |  2 +-
 drivers/net/mlx5/mlx5.c|  4 ++--
 drivers/net/mlx5/mlx5_flow_verbs.c | 10 +-
 drivers/net/mlx5/mlx5_glue.c   |  8 
 drivers/net/mlx5/mlx5_glue.h   |  2 +-
 6 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index 1e9c0b4..124ee4e 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -157,7 +157,7 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
enum ETHTOOL_LINK_MODE_10baseKR4_Full_BIT \
$(AUTOCONF_OUTPUT)
$Q sh -- '$<' '$@' \
-   HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT \
+   HAVE_IBV_DEVICE_COUNTERS_SET_V42 \
infiniband/verbs.h \
type 'struct ibv_counter_set_init_attr' \
$(AUTOCONF_OUTPUT)
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index c192d44..e613a21 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -79,7 +79,7 @@ if build
has_member_args = [
[ 'HAVE_IBV_MLX5_MOD_SWP', 'infiniband/mlx5dv.h',
'struct mlx5dv_sw_parsing_caps', 'sw_parsing_offloads' ],
-   [ 'HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT', 'infiniband/verbs.h',
+   [ 'HAVE_IBV_DEVICE_COUNTERS_SET_V42', 'infiniband/verbs.h',
'struct ibv_counter_set_init_attr', 'counter_set_id' ],
]
# input array for meson symbol search:
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index cfff54e..cb86a69 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -739,7 +739,7 @@
unsigned int mprq_max_stride_size_n = 0;
unsigned int mprq_min_stride_num_n = 0;
unsigned int mprq_max_stride_num_n = 0;
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
struct ibv_counter_set_description cs_desc = { .counter_type = 0 };
 #endif
struct ether_addr mac;
@@ -1009,7 +1009,7 @@
config.hw_csum = !!(attr.device_cap_flags_ex & IBV_DEVICE_RAW_IP_CSUM);
DRV_LOG(DEBUG, "checksum offloading is %ssupported",
(config.hw_csum ? "" : "not "));
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
config.flow_counter_en = !!attr.max_counter_sets;
mlx5_glue->describe_counter_set(ctx, 0, &cs_desc);
DRV_LOG(DEBUG, "counter type = %d, num of cs = %ld, attributes = %d",
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c 
b/drivers/net/mlx5/mlx5_flow_verbs.c
index 6ddb13b..3d6fedb 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -60,7 +60,7 @@
cnt->ref_cnt++;
return cnt;
}
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
 
struct mlx5_flow_counter tmpl = {
.shared = shared,
@@ -938,7 +938,7 @@
 {
const struct rte_flow_action_count *count = action->conf;
struct rte_flow *flow = dev_flow->flow;
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
unsigned int size = sizeof(struct ibv_flow_spec_counter_action);
struct ibv_flow_spec_counter_action counter = {
.type = IBV_FLOW_SPEC_ACTION_COUNT,
@@ -957,7 +957,7 @@
  " context.");
}
*action_flags |= MLX5_FLOW_ACTION_COUNT;
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
counter.counter_set_handle = flow->counter->cs->handle;
flow_verbs_spec_add(dev_flow, &counter, size);
 #endif
@@ -1222,7 +1222,7 @@
detected_actions |= MLX5_FLOW_ACTION_RSS;
break;
case RTE_FLOW_ACTION_TYPE_COUNT:
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
size += sizeof(struct ibv_flow_spec_counter_action);
 #endif
detected_actions |= MLX5_FLOW_ACTION_COUNT;
@@ -1665,7 +1665,7 @@
   void *data __rte_unused,
   struct rte_flow_error *error)
 {
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
if (flow->actions & MLX5_FLOW_ACTION_COUNT) {

[dpdk-dev] [PATCH v4 7/8] net/mlx5: remove unnecessary structure initializers

2018-10-23 Thread Slava Ovsiienko
The unnecessary structure filed initializers to zero are removed.
We need to do this minor preparation before the following
mlx5 counter structure modification.

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_flow_tcf.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index a0680af..4076400 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -399,11 +399,6 @@ struct pedit_parser {
 */
struct mlx5_flow_counter tmpl = {
.ref_cnt = 1,
-   .shared = 0,
-   .id = 0,
-   .cs = NULL,
-   .hits = 0,
-   .bytes = 0,
};
cnt = rte_calloc(__func__, 1, sizeof(*cnt), 0);
if (!cnt) {
-- 
1.8.3.1



[dpdk-dev] [PATCH v4 5/8] net/mlx5: relocate flow counters query function

2018-10-23 Thread Slava Ovsiienko
The flow_verbs_query_count() is moved into the the group of counter
managing functions and renamed to flow_verbs_counter_query() in order
to be in unified fashion with others.

Also minor function modification is made to avoid unreachable code
warnings if there is no counter support at compile time.

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_flow_verbs.c | 111 +++--
 1 file changed, 56 insertions(+), 55 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c 
b/drivers/net/mlx5/mlx5_flow_verbs.c
index 3d6fedb..f720c35 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -110,6 +110,61 @@
 }
 
 /**
+ * Query a flow counter via Verbs library call.
+ *
+ * @see rte_flow_query()
+ * @see rte_flow_ops
+ */
+static int
+flow_verbs_counter_query(struct rte_eth_dev *dev __rte_unused,
+struct rte_flow *flow __rte_unused,
+void *data __rte_unused,
+struct rte_flow_error *error)
+{
+#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
+   if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
+   struct rte_flow_query_count *qc = data;
+   uint64_t counters[2] = {0, 0};
+   struct ibv_query_counter_set_attr query_cs_attr = {
+   .cs = flow->counter->cs,
+   .query_flags = IBV_COUNTER_SET_FORCE_UPDATE,
+   };
+   struct ibv_counter_set_data query_out = {
+   .out = counters,
+   .outlen = 2 * sizeof(uint64_t),
+   };
+   int err = mlx5_glue->query_counter_set(&query_cs_attr,
+  &query_out);
+
+   if (err)
+   return rte_flow_error_set
+   (error, err,
+RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+NULL,
+"cannot read counter");
+   qc->hits_set = 1;
+   qc->bytes_set = 1;
+   qc->hits = counters[0] - flow->counter->hits;
+   qc->bytes = counters[1] - flow->counter->bytes;
+   if (qc->reset) {
+   flow->counter->hits = counters[0];
+   flow->counter->bytes = counters[1];
+   }
+   return 0;
+   }
+   return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL,
+ "flow does not have counter");
+#else
+   return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL,
+ "counters are not available");
+#endif
+}
+
+/**
  * Add a verbs item specification into @p flow.
  *
  * @param[in, out] flow
@@ -1654,60 +1709,6 @@
 }
 
 /**
- * Query a flows.
- *
- * @see rte_flow_query()
- * @see rte_flow_ops
- */
-static int
-flow_verbs_query_count(struct rte_eth_dev *dev __rte_unused,
-  struct rte_flow *flow __rte_unused,
-  void *data __rte_unused,
-  struct rte_flow_error *error)
-{
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
-   if (flow->actions & MLX5_FLOW_ACTION_COUNT) {
-   struct rte_flow_query_count *qc = data;
-   uint64_t counters[2] = {0, 0};
-   struct ibv_query_counter_set_attr query_cs_attr = {
-   .cs = flow->counter->cs,
-   .query_flags = IBV_COUNTER_SET_FORCE_UPDATE,
-   };
-   struct ibv_counter_set_data query_out = {
-   .out = counters,
-   .outlen = 2 * sizeof(uint64_t),
-   };
-   int err = mlx5_glue->query_counter_set(&query_cs_attr,
-  &query_out);
-
-   if (err)
-   return rte_flow_error_set
-   (error, err,
-RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-NULL,
-"cannot read counter");
-   qc->hits_set = 1;
-   qc->bytes_set = 1;
-   qc->hits = counters[0] - flow->counter->hits;
-   qc->bytes = counters[1] - flow->counter->bytes;
-   if (qc->reset) {
-   flow->counter->hits = counters[0];
-   flow->counter->bytes = counters[1];
-   }
-   return 0;
-   }
-   return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
- NULL,
- "flow does not have counter");
-#endif
-   return rte_fl

[dpdk-dev] [PATCH v4 1/8] net/mlx5: fix flow counters creation

2018-10-23 Thread Slava Ovsiienko
The Flow counter creation function contains two problems:

  - Flow counter object in Verbs is not freed in case of memory
allocation error. The call of counter Verbs object deallocating
function is added to fix.

  - The initial value of reference counter is set to one in order
to provide the correct counter object freeing in the
flow_verbs_counter_release() function. The reference counter
field should be initialized to one.

Fixes: 60bd8c9747e8 ("net/mlx5: add count flow action")
Cc: sta...@dpdk.org

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_flow_verbs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c 
b/drivers/net/mlx5/mlx5_flow_verbs.c
index 4ae974b..6ddb13b 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -72,6 +72,7 @@
 }),
.hits = 0,
.bytes = 0,
+   .ref_cnt = 1,
};
 
if (!tmpl.cs) {
@@ -80,6 +81,7 @@
}
cnt = rte_calloc(__func__, 1, sizeof(*cnt), 0);
if (!cnt) {
+   claim_zero(mlx5_glue->destroy_counter_set(tmpl.cs));
rte_errno = ENOMEM;
return NULL;
}
-- 
1.8.3.1



[dpdk-dev] [PATCH v4 3/8] net/mlx5: introduce new flow counters configuration macro

2018-10-23 Thread Slava Ovsiienko
The new configuration macro HAVE_IBV_DEVICE_COUNTERS_SET_V45 is
introduced. Both makefile and meson.build are changed.

Flow counter support code depends on the following configuration
macros:

- HAVE_IBV_DEVICE_COUNTERS_SET_V42 - is defined if system supports
  the "old" flow counters functionality, MLNX_OFED version from
  4.2 to 4.4 is required.

- HAVE_IBV_DEVICE_COUNTERS_SET_V45 - is defined if system supports
  the "new" flow counters functionality, MLNX_OVED 4.5 (or higher)
  or Linux rdma-core v19 (or higher) is required.

Neither HAVE_IBV_DEVICE_COUNTERS_SET_V42 nor
HAVE_IBV_DEVICE_COUNTERS_SET_V45 is defined if there is no
counters support.

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/Makefile| 5 +
 drivers/net/mlx5/meson.build | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index 124ee4e..fecb57c 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -162,6 +162,11 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
type 'struct ibv_counter_set_init_attr' \
$(AUTOCONF_OUTPUT)
$Q sh -- '$<' '$@' \
+   HAVE_IBV_DEVICE_COUNTERS_SET_V45 \
+   infiniband/verbs.h \
+   type 'struct ibv_counters_init_attr' \
+   $(AUTOCONF_OUTPUT)
+   $Q sh -- '$<' '$@' \
HAVE_RDMA_NL_NLDEV \
rdma/rdma_netlink.h \
enum RDMA_NL_NLDEV \
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index e613a21..e8cbe3e 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -81,6 +81,8 @@ if build
'struct mlx5dv_sw_parsing_caps', 'sw_parsing_offloads' ],
[ 'HAVE_IBV_DEVICE_COUNTERS_SET_V42', 'infiniband/verbs.h',
'struct ibv_counter_set_init_attr', 'counter_set_id' ],
+   [ 'HAVE_IBV_DEVICE_COUNTERS_SET_V45', 'infiniband/verbs.h',
+   'struct ibv_counters_init_attr', 'comp_mask' ],
]
# input array for meson symbol search:
# [ "MACRO to define if found", "header for the search",
-- 
1.8.3.1



[dpdk-dev] [PATCH v4 4/8] net/mlx5: simplify flow counters support check

2018-10-23 Thread Slava Ovsiienko
The redundant check of Flow counters support in runtime is removed.
The flag flow_counter_en is eliminated from the code. The Verbs
create counter function just returns an error if no counter
support presented in the system.

If there is no any of Flow counters configuration macro defined
the log message is emited, indicating the missing counter support.

mlx5_flow_validate_action_count() fuctnion is also updated due to
flow_counter_en flag removal.

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5.c  | 12 +++-
 drivers/net/mlx5/mlx5.h  |  1 -
 drivers/net/mlx5/mlx5_flow.c |  8 +---
 3 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index cb86a69..a6cae69 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -739,9 +739,6 @@
unsigned int mprq_max_stride_size_n = 0;
unsigned int mprq_min_stride_num_n = 0;
unsigned int mprq_max_stride_num_n = 0;
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
-   struct ibv_counter_set_description cs_desc = { .counter_type = 0 };
-#endif
struct ether_addr mac;
char name[RTE_ETH_NAME_MAX_LEN];
int own_domain_id = 0;
@@ -1009,12 +1006,9 @@
config.hw_csum = !!(attr.device_cap_flags_ex & IBV_DEVICE_RAW_IP_CSUM);
DRV_LOG(DEBUG, "checksum offloading is %ssupported",
(config.hw_csum ? "" : "not "));
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
-   config.flow_counter_en = !!attr.max_counter_sets;
-   mlx5_glue->describe_counter_set(ctx, 0, &cs_desc);
-   DRV_LOG(DEBUG, "counter type = %d, num of cs = %ld, attributes = %d",
-   cs_desc.counter_type, cs_desc.num_of_cs,
-   cs_desc.attributes);
+#if !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) && \
+   !defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
+   DRV_LOG(DEBUG, "counters are not supported");
 #endif
config.ind_table_max_size =
attr.rss_caps.max_rwq_indirection_table_size;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index d14239c..74d87c0 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -114,7 +114,6 @@ struct mlx5_dev_config {
unsigned int tunnel_en:1;
/* Whether tunnel stateless offloads are supported. */
unsigned int mpls_en:1; /* MPLS over GRE/UDP is enabled. */
-   unsigned int flow_counter_en:1; /* Whether flow counter is supported. */
unsigned int cqe_comp:1; /* CQE compression is enabled. */
unsigned int tso:1; /* Whether TSO is supported. */
unsigned int tx_vec_en:1; /* Tx vector is enabled. */
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index fcabab0..14288e4 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -921,16 +921,10 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev 
*dev, int32_t priority,
  *   0 on success, a negative errno value otherwise and rte_ernno is set.
  */
 int
-mlx5_flow_validate_action_count(struct rte_eth_dev *dev,
+mlx5_flow_validate_action_count(struct rte_eth_dev *dev __rte_unused,
const struct rte_flow_attr *attr,
struct rte_flow_error *error)
 {
-   struct priv *priv = dev->data->dev_private;
-
-   if (!priv->config.flow_counter_en)
-   return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION, NULL,
- "flow counters are not supported.");
if (attr->egress)
return rte_flow_error_set(error, ENOTSUP,
  RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
-- 
1.8.3.1



Re: [dpdk-dev] [PATCH v2 2/7] net/mlx5: e-switch VXLAN flow validation routine

2018-10-23 Thread Yongseok Koh
On Mon, Oct 15, 2018 at 02:13:30PM +, Viacheslav Ovsiienko wrote:
> This part of patchset adds support for flow item/action lists
> validation. The following entities are now supported:
> 
> - RTE_FLOW_ITEM_TYPE_VXLAN, contains the tunnel VNI
> 
> - RTE_FLOW_ACTION_TYPE_VXLAN_DECAP, if this action is specified
>   the items in the flow items list treated as outer network
>   parameters for tunnel outer header match. The ethernet layer
>   addresses always are treated as inner ones.
> 
> - RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP, contains the item list to
>   build the encapsulation header. In current implementation the
>   values is the subject for some constraints:
> - outer source MAC address will be always unconditionally
>   set to the one of MAC addresses of outer egress interface
> - no way to specify source UDP port
> - all abovementioned parameters are ignored if specified
>   in the rule, warning messages are sent to the log
> 
> Suggested-by: Adrien Mazarguil 
> Signed-off-by: Viacheslav Ovsiienko 
> ---
>  drivers/net/mlx5/mlx5_flow_tcf.c | 711 
> ++-
>  1 file changed, 705 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c 
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 8f9c78a..0055417 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -430,6 +430,7 @@ struct mlx5_flow_tcf_context {
>   struct rte_flow_item_ipv6 ipv6;
>   struct rte_flow_item_tcp tcp;
>   struct rte_flow_item_udp udp;
> + struct rte_flow_item_vxlan vxlan;
>  } flow_tcf_mask_empty;
>  
>  /** Supported masks for known item types. */
> @@ -441,6 +442,7 @@ struct mlx5_flow_tcf_context {
>   struct rte_flow_item_ipv6 ipv6;
>   struct rte_flow_item_tcp tcp;
>   struct rte_flow_item_udp udp;
> + struct rte_flow_item_vxlan vxlan;
>  } flow_tcf_mask_supported = {
>   .port_id = {
>   .id = 0x,
> @@ -478,6 +480,9 @@ struct mlx5_flow_tcf_context {
>   .src_port = RTE_BE16(0x),
>   .dst_port = RTE_BE16(0x),
>   },
> + .vxlan = {
> +.vni = "\xff\xff\xff",
> + },
>  };
>  
>  #define SZ_NLATTR_HDR MNL_ALIGN(sizeof(struct nlattr))
> @@ -943,6 +948,615 @@ struct pedit_parser {
>  }
>  
>  /**
> + * Validate VXLAN_ENCAP action RTE_FLOW_ITEM_TYPE_ETH item for E-Switch.

How about mentioning it is to validate items of the "encap header"? Same for the
rest.

> + *
> + * @param[in] item
> + *   Pointer to the itemn structure.

Typo. Same for the rest.

> + * @param[out] error
> + *   Pointer to the error structure.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + **/
> +static int
> +flow_tcf_validate_vxlan_encap_eth(const struct rte_flow_item *item,
> +   struct rte_flow_error *error)
> +{
> + const struct rte_flow_item_eth *spec = item->spec;
> + const struct rte_flow_item_eth *mask = item->mask;
> +
> + if (!spec)
> + /*
> +  * Specification for L2 addresses can be empty
> +  * because these ones are optional and not
> +  * required directly by tc rule.
> +  */
> + return 0;
> + if (!mask)
> + /* If mask is not specified use the default one. */
> + mask = &rte_flow_item_eth_mask;
> + if (memcmp(&mask->dst,
> +&flow_tcf_mask_empty.eth.dst,
> +sizeof(flow_tcf_mask_empty.eth.dst))) {
> + if (memcmp(&mask->dst,
> +&rte_flow_item_eth_mask.dst,
> +sizeof(rte_flow_item_eth_mask.dst)))
> + return rte_flow_error_set(error, ENOTSUP,
> +  RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
> +  "no support for partial mask on"
> +  " \"eth.dst\" field");
> + }
> + if (memcmp(&mask->src,
> +&flow_tcf_mask_empty.eth.src,
> +sizeof(flow_tcf_mask_empty.eth.src))) {
> + if (memcmp(&mask->src,
> +&rte_flow_item_eth_mask.src,
> +sizeof(rte_flow_item_eth_mask.src)))
> + return rte_flow_error_set(error, ENOTSUP,
> +  RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
> +  "no support for partial mask on"
> +  " \"eth.src\" field");
> + }
> + if (mask->type != RTE_BE16(0x)) {
> + if (mask->type != RTE_BE16(0x))
> + return rte_flow_error_set(error, ENOTSUP,
> +  RTE_FLOW_ERROR_TYPE_ITEM_MASK, mask,
> +  "no support for partial mask on"
> +  " \"eth.type\" field");
> + DRV_LOG(WARNING,
> + "outer ethernet type field "

[dpdk-dev] [PATCH v4 8/8] net/mlx5: support new flow counter API

2018-10-23 Thread Slava Ovsiienko
This patch updates the functions performing the Verbs ibrary calls
in order to support different versions of the library.
The functions:
  - flow_verbs_counter_new()
  - flow_verbs_counter_release()
  - flow_verbs_counter_query()
now have the several compilation branches, depending on the
counters support found in the system at compile time.

The flow_verbs_counter_create() function is introduced as
helper for flow_verbs_counter_new(), actually this helper
create the counters with Verbs.

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_flow.h   |   4 ++
 drivers/net/mlx5/mlx5_flow_verbs.c | 135 -
 2 files changed, 107 insertions(+), 32 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index af0a125..69ef0ef 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -228,7 +228,11 @@ struct mlx5_flow_counter {
uint32_t shared:1; /**< Share counter ID with other flow rules. */
uint32_t ref_cnt:31; /**< Reference counter. */
uint32_t id; /**< Counter ID. */
+#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
struct ibv_counter_set *cs; /**< Holds the counters for the rule. */
+#elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
+   struct ibv_counters *cs; /**< Holds the counters for the rule. */
+#endif
uint64_t hits; /**< Number of packets matched by the rule. */
uint64_t bytes; /**< Number of bytes matched by the rule. */
 };
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c 
b/drivers/net/mlx5/mlx5_flow_verbs.c
index f720c35..2547fb8 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -34,6 +34,70 @@
 #include "mlx5_flow.h"
 
 /**
+ * Create Verbs flow counter with Verbs library.
+ *
+ * @param[in] dev
+ *   Pointer to the Ethernet device structure.
+ * @param[in, out] counter
+ *   mlx5 flow counter object, contains the counter id,
+ *   handle of created Verbs flow counter is returned
+ *   in cs field (if counters are supported).
+ *
+ * @return
+ *   0 On success else a negative errno value is returned
+ *   and rte_errno is set.
+ */
+static int
+flow_verbs_counter_create(struct rte_eth_dev *dev,
+ struct mlx5_flow_counter *counter)
+{
+#if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
+   struct priv *priv = dev->data->dev_private;
+   struct ibv_counter_set_init_attr init = {
+.counter_set_id = counter->id};
+
+   counter->cs = mlx5_glue->create_counter_set(priv->ctx, &init);
+   if (!counter->cs) {
+   rte_errno = ENOTSUP;
+   return -ENOTSUP;
+   }
+   return 0;
+#elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
+   struct priv *priv = dev->data->dev_private;
+   struct ibv_counters_init_attr init = {0};
+   struct ibv_counter_attach_attr attach = {0};
+   int ret;
+
+   counter->cs = mlx5_glue->create_counters(priv->ctx, &init);
+   if (!counter->cs) {
+   rte_errno = ENOTSUP;
+   return -ENOTSUP;
+   }
+   attach.counter_desc = IBV_COUNTER_PACKETS;
+   attach.index = 0;
+   ret = mlx5_glue->attach_counters(counter->cs, &attach, NULL);
+   if (!ret) {
+   attach.counter_desc = IBV_COUNTER_BYTES;
+   attach.index = 1;
+   ret = mlx5_glue->attach_counters
+   (counter->cs, &attach, NULL);
+   }
+   if (ret) {
+   claim_zero(mlx5_glue->destroy_counters(counter->cs));
+   counter->cs = NULL;
+   rte_errno = ret;
+   return -ret;
+   }
+   return 0;
+#else
+   (void)dev;
+   (void)counter;
+   rte_errno = ENOTSUP;
+   return -ENOTSUP;
+#endif
+}
+
+/**
  * Get a flow counter.
  *
  * @param[in] dev
@@ -51,6 +115,7 @@
 {
struct priv *priv = dev->data->dev_private;
struct mlx5_flow_counter *cnt;
+   int ret;
 
LIST_FOREACH(cnt, &priv->flow_counters, next) {
if (!cnt->shared || cnt->shared != shared)
@@ -60,36 +125,25 @@
cnt->ref_cnt++;
return cnt;
}
-#ifdef HAVE_IBV_DEVICE_COUNTERS_SET_V42
-
-   struct mlx5_flow_counter tmpl = {
-   .shared = shared,
-   .id = id,
-   .cs = mlx5_glue->create_counter_set
-   (priv->ctx,
-&(struct ibv_counter_set_init_attr){
-.counter_set_id = id,
-}),
-   .hits = 0,
-   .bytes = 0,
-   .ref_cnt = 1,
-   };
-
-   if (!tmpl.cs) {
-   rte_errno = errno;
-   return NULL;
-   }
cnt = rte_calloc(__func__, 1, sizeof(*cnt), 0);
if (!cnt) {
-   claim_zero(mlx5_glue->destroy_counter_set(tmpl.cs));
rte_errno = ENOMEM;
return NULL;
 

[dpdk-dev] [PATCH v4 6/8] net/mlx5: add new flow counter Verbs API to glue library

2018-10-23 Thread Slava Ovsiienko
This patch updates the mlx5 glue library, new counter support
Verbs function pointers are added to the glue linking structure
mlx5_glue. This structure now contains the pointers to the both
versions of counter supporting functions due to compatibility
issues. Depending on configuration macros the functions perform
actual Verbs library calls or return an error with meaning
"feature is not supported" (NULL or ENOTSUP).

Signed-off-by: Viacheslav Ovsiienko 
---
 drivers/net/mlx5/mlx5_glue.c | 60 
 drivers/net/mlx5/mlx5_glue.h | 17 +
 2 files changed, 77 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
index 889e074..1afb114 100644
--- a/drivers/net/mlx5/mlx5_glue.c
+++ b/drivers/net/mlx5/mlx5_glue.c
@@ -263,6 +263,62 @@
 #endif
 }
 
+static struct ibv_counters *
+mlx5_glue_create_counters(struct ibv_context *context,
+ struct ibv_counters_init_attr *init_attr)
+{
+#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_V45
+   (void)context;
+   (void)init_attr;
+   return NULL;
+#else
+   return ibv_create_counters(context, init_attr);
+#endif
+}
+
+static int
+mlx5_glue_destroy_counters(struct ibv_counters *counters)
+{
+#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_V45
+   (void)counters;
+   return ENOTSUP;
+#else
+   return ibv_destroy_counters(counters);
+#endif
+}
+
+static int
+mlx5_glue_attach_counters(struct ibv_counters *counters,
+ struct ibv_counter_attach_attr *attr,
+ struct ibv_flow *flow)
+{
+#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_V45
+   (void)counters;
+   (void)attr;
+   (void)flow;
+   return ENOTSUP;
+#else
+   return ibv_attach_counters_point_flow(counters, attr, flow);
+#endif
+}
+
+static int
+mlx5_glue_query_counters(struct ibv_counters *counters,
+uint64_t *counters_value,
+uint32_t ncounters,
+uint32_t flags)
+{
+#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_V45
+   (void)counters;
+   (void)counters_value;
+   (void)ncounters;
+   (void)flags;
+   return ENOTSUP;
+#else
+   return ibv_read_counters(counters, counters_value, ncounters, flags);
+#endif
+}
+
 static void
 mlx5_glue_ack_async_event(struct ibv_async_event *event)
 {
@@ -424,6 +480,10 @@
.destroy_counter_set = mlx5_glue_destroy_counter_set,
.describe_counter_set = mlx5_glue_describe_counter_set,
.query_counter_set = mlx5_glue_query_counter_set,
+   .create_counters = mlx5_glue_create_counters,
+   .destroy_counters = mlx5_glue_destroy_counters,
+   .attach_counters = mlx5_glue_attach_counters,
+   .query_counters = mlx5_glue_query_counters,
.ack_async_event = mlx5_glue_ack_async_event,
.get_async_event = mlx5_glue_get_async_event,
.port_state_str = mlx5_glue_port_state_str,
diff --git a/drivers/net/mlx5/mlx5_glue.h b/drivers/net/mlx5/mlx5_glue.h
index adee972..44bfefe 100644
--- a/drivers/net/mlx5/mlx5_glue.h
+++ b/drivers/net/mlx5/mlx5_glue.h
@@ -31,6 +31,12 @@
 struct ibv_query_counter_set_attr;
 #endif
 
+#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_V45
+struct ibv_counters;
+struct ibv_counters_init_attr;
+struct ibv_counter_attach_attr;
+#endif
+
 #ifndef HAVE_IBV_DEVICE_TUNNEL_SUPPORT
 struct mlx5dv_qp_init_attr;
 #endif
@@ -106,6 +112,17 @@ struct mlx5_glue {
 struct ibv_counter_set_description *cs_desc);
int (*query_counter_set)(struct ibv_query_counter_set_attr *query_attr,
 struct ibv_counter_set_data *cs_data);
+   struct ibv_counters *(*create_counters)
+   (struct ibv_context *context,
+struct ibv_counters_init_attr *init_attr);
+   int (*destroy_counters)(struct ibv_counters *counters);
+   int (*attach_counters)(struct ibv_counters *counters,
+  struct ibv_counter_attach_attr *attr,
+  struct ibv_flow *flow);
+   int (*query_counters)(struct ibv_counters *counters,
+ uint64_t *counters_value,
+ uint32_t ncounters,
+ uint32_t flags);
void (*ack_async_event)(struct ibv_async_event *event);
int (*get_async_event)(struct ibv_context *context,
   struct ibv_async_event *event);
-- 
1.8.3.1



Re: [dpdk-dev] [PATCH v2 3/7] net/mlx5: e-switch VXLAN flow translation routine

2018-10-23 Thread Yongseok Koh
On Mon, Oct 15, 2018 at 02:13:31PM +, Viacheslav Ovsiienko wrote:
> This part of patchset adds support of VXLAN-related items and
> actions to the flow translation routine. If some of them are
> specified in the rule, the extra space for tunnel description
> structure is allocated. Later some tunnel types, other than
> VXLAN can be addedd (GRE). No VTEP devices are created at this
> point, the flow rule is just translated, not applied yet.
> 
> Suggested-by: Adrien Mazarguil 
> Signed-off-by: Viacheslav Ovsiienko 
> ---
>  drivers/net/mlx5/mlx5_flow_tcf.c | 641 
> +++
>  1 file changed, 578 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c 
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 0055417..660d45e 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -2094,6 +2094,265 @@ struct pedit_parser {
>  }
>  
>  /**
> + * Helper function to process RTE_FLOW_ITEM_TYPE_ETH entry in configuration
> + * of action RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP. Fills the MAC address fields
> + * in the encapsulation parameters structure. The item must be prevalidated,
> + * no any validation checks performed by function.
> + *
> + * @param[in] spec
> + *   RTE_FLOW_ITEM_TYPE_ETH entry specification.
> + * @param[in] mask
> + *   RTE_FLOW_ITEM_TYPE_ETH entry mask.
> + * @param[out] encap
> + *   Structure to fill the gathered MAC address data.
> + *
> + * @return
> + *   The size needed the Netlink message tunnel_key
> + *   parameter buffer to store the item attributes.
> + */
> +static int
> +flow_tcf_parse_vxlan_encap_eth(const struct rte_flow_item_eth *spec,
> +const struct rte_flow_item_eth *mask,
> +struct mlx5_flow_tcf_vxlan_encap *encap)
> +{
> + /* Item must be validated before. No redundant checks. */
> + assert(spec);
> + if (!mask || !memcmp(&mask->dst,
> +  &rte_flow_item_eth_mask.dst,
> +  sizeof(rte_flow_item_eth_mask.dst))) {
> + /*
> +  * Ethernet addresses are not supported by
> +  * tc as tunnel_key parameters. Destination
> +  * address is needed to form encap packet
> +  * header and retrieved by kernel from
> +  * implicit sources (ARP table, etc),
> +  * address masks are not supported at all.
> +  */
> + encap->eth.dst = spec->dst;
> + encap->mask |= MLX5_FLOW_TCF_ENCAP_ETH_DST;
> + }
> + if (!mask || !memcmp(&mask->src,
> +  &rte_flow_item_eth_mask.src,
> +  sizeof(rte_flow_item_eth_mask.src))) {
> + /*
> +  * Ethernet addresses are not supported by
> +  * tc as tunnel_key parameters. Source ethernet
> +  * address is ignored anyway.
> +  */
> + encap->eth.src = spec->src;
> + encap->mask |= MLX5_FLOW_TCF_ENCAP_ETH_SRC;
> + }
> + /*
> +  * No space allocated for ethernet addresses within Netlink
> +  * message tunnel_key record - these ones are not
> +  * supported by tc.
> +  */
> + return 0;
> +}
> +
> +/**
> + * Helper function to process RTE_FLOW_ITEM_TYPE_IPV4 entry in configuration
> + * of action RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP. Fills the IPV4 address fields
> + * in the encapsulation parameters structure. The item must be prevalidated,
> + * no any validation checks performed by function.
> + *
> + * @param[in] spec
> + *   RTE_FLOW_ITEM_TYPE_IPV4 entry specification.
> + * @param[out] encap
> + *   Structure to fill the gathered IPV4 address data.
> + *
> + * @return
> + *   The size needed the Netlink message tunnel_key
> + *   parameter buffer to store the item attributes.
> + */
> +static int
> +flow_tcf_parse_vxlan_encap_ipv4(const struct rte_flow_item_ipv4 *spec,
> + struct mlx5_flow_tcf_vxlan_encap *encap)
> +{
> + /* Item must be validated before. No redundant checks. */
> + assert(spec);
> + encap->ipv4.dst = spec->hdr.dst_addr;
> + encap->ipv4.src = spec->hdr.src_addr;
> + encap->mask |= MLX5_FLOW_TCF_ENCAP_IPV4_SRC |
> +MLX5_FLOW_TCF_ENCAP_IPV4_DST;
> + return 2 * SZ_NLATTR_TYPE_OF(uint32_t);
> +}
> +
> +/**
> + * Helper function to process RTE_FLOW_ITEM_TYPE_IPV6 entry in configuration
> + * of action RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP. Fills the IPV6 address fields
> + * in the encapsulation parameters structure. The item must be prevalidated,
> + * no any validation checks performed by function.
> + *
> + * @param[in] spec
> + *   RTE_FLOW_ITEM_TYPE_IPV6 entry specification.
> + * @param[out] encap
> + *   Structure to fill the gathered IPV6 address data.
> + *
> + * @return
> + *   The size needed the Netlink message tunnel_key
> + *   parameter buffer to store the item attributes.
> + */
> +static int
> +f

Re: [dpdk-dev] [PATCH v2 4/7] net/mlx5: e-switch VXLAN netlink routines update

2018-10-23 Thread Yongseok Koh
On Mon, Oct 15, 2018 at 02:13:32PM +, Viacheslav Ovsiienko wrote:
> This part of patchset updates Netlink exchange routine. Message
> sequence numbers became not random ones, the multipart reply messages
> are supported, not propagating errors to the following socket calls,
> Netlink replies buffer size is increased to MNL_SOCKET_BUFFER_SIZE
> and now is preallocated at context creation time instead of stack
> usage. This update is needed to support Netlink query operations.
> 
> Suggested-by: Adrien Mazarguil 
> Signed-off-by: Viacheslav Ovsiienko 
> ---
Acked-by: Yongseok Koh 

Thanks

>  drivers/net/mlx5/mlx5_flow_tcf.c | 82 
> +---
>  1 file changed, 60 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c 
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 660d45e..d6840d5 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -3372,37 +3372,75 @@ struct pedit_parser {
>  /**
>   * Send Netlink message with acknowledgment.
>   *
> - * @param ctx
> + * @param tcf
>   *   Flow context to use.
>   * @param nlh
>   *   Message to send. This function always raises the NLM_F_ACK flag before
>   *   sending.
> + * @param[in] msglen
> + *   Message length. Message buffer may contain multiple commands and
> + *   nlmsg_len field not always corresponds to actual message length.
> + *   If 0 specified the nlmsg_len field in header is used as message length.
> + * @param[in] cb
> + *   Callback handler for received message.
> + * @param[in] arg
> + *   Context pointer for callback handler.
>   *
>   * @return
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
>  static int
> -flow_tcf_nl_ack(struct mlx5_flow_tcf_context *ctx, struct nlmsghdr *nlh)
> +flow_tcf_nl_ack(struct mlx5_flow_tcf_context *tcf,
> + struct nlmsghdr *nlh,
> + uint32_t msglen,
> + mnl_cb_t cb, void *arg)
>  {
> - alignas(struct nlmsghdr)
> - uint8_t ans[mnl_nlmsg_size(sizeof(struct nlmsgerr)) +
> - nlh->nlmsg_len - sizeof(*nlh)];
> - uint32_t seq = ctx->seq++;
> - struct mnl_socket *nl = ctx->nl;
> - int ret;
> -
> - nlh->nlmsg_flags |= NLM_F_ACK;
> + unsigned int portid = mnl_socket_get_portid(tcf->nl);
> + uint32_t seq = tcf->seq++;
> + int err, ret;
> +
> + assert(tcf->nl);
> + assert(tcf->buf);
> + if (!seq)
> + seq = tcf->seq++;
>   nlh->nlmsg_seq = seq;
> - ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
> - if (ret != -1)
> - ret = mnl_socket_recvfrom(nl, ans, sizeof(ans));
> - if (ret != -1)
> - ret = mnl_cb_run
> - (ans, ret, seq, mnl_socket_get_portid(nl), NULL, NULL);
> + if (!msglen) {
> + msglen = nlh->nlmsg_len;
> + nlh->nlmsg_flags |= NLM_F_ACK;
> + }
> + ret = mnl_socket_sendto(tcf->nl, nlh, msglen);
> + err = (ret <= 0) ? errno : 0;
> + nlh = (struct nlmsghdr *)(tcf->buf);
> + /*
> +  * The following loop postpones non-fatal errors until multipart
> +  * messages are complete.
> +  */
>   if (ret > 0)
> + while (true) {
> + ret = mnl_socket_recvfrom(tcf->nl, tcf->buf,
> +   tcf->buf_size);
> + if (ret < 0) {
> + err = errno;
> + if (err != ENOSPC)
> + break;
> + }
> + if (!err) {
> + ret = mnl_cb_run(nlh, ret, seq, portid,
> +  cb, arg);
> + if (ret < 0) {
> + err = errno;
> + break;
> + }
> + }
> + /* Will receive till end of multipart message */
> + if (!(nlh->nlmsg_flags & NLM_F_MULTI) ||
> +   nlh->nlmsg_type == NLMSG_DONE)
> + break;
> + }
> + if (!err)
>   return 0;
> - rte_errno = errno;
> - return -rte_errno;
> + rte_errno = err;
> + return -err;
>  }
>  
>  /**
> @@ -3433,7 +3471,7 @@ struct pedit_parser {
>   nlh = dev_flow->tcf.nlh;
>   nlh->nlmsg_type = RTM_NEWTFILTER;
>   nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
> - if (!flow_tcf_nl_ack(nl, nlh))
> + if (!flow_tcf_nl_ack(nl, nlh, 0, NULL, NULL))
>   return 0;
>   return rte_flow_error_set(error, rte_errno,
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> @@ -3466,7 +3504,7 @@ struct pedit_parser {
>   nlh = dev_flow->tcf.nlh;
>   nlh->nlmsg_type = RTM_DELTFILTER;
>   nlh->nlmsg_flags = NLM_F_REQUEST;
> - flow_tcf_nl_ack(nl, nlh);

[dpdk-dev] [PATCH v2] vhost: avoid memory barriers when no descriptors dequeued

2018-10-23 Thread Maxime Coquelin
In both split and packed dequeue paths, flush_shadow_used_ring
and vhost_ring_call variants gets called even if not packets
have been dequeued, and so no descriptors updates happened.

It has an impact on CPU pipeline, as memory barriers are used
in these functions.

This patch don't call these functions if no descriptors have
been dequeued. The performance gain with split ring when
dequeue zero-copy is disabled should be null, but should be
noticeable with packed ring or dequeue zero-copy enabled.

Fixes: ae999ce49dcb ("vhost: add Tx support for packed ring")
Fixes: 915cf9404225 ("vhost: use shadow used ring in dequeue path")
Cc: sta...@dpdk.org

Signed-off-by: Maxime Coquelin 
---

Changes in v2:
- Fix shadow_used_idx reset in error path (Tiwei)

 lib/librte_vhost/virtio_net.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 1c31c0562..8ad30c94a 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -1360,8 +1360,10 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
}
}
 
-   flush_shadow_used_ring_split(dev, vq);
-   vhost_vring_call_split(dev, vq);
+   if (likely(vq->shadow_used_idx)) {
+   flush_shadow_used_ring_split(dev, vq);
+   vhost_vring_call_split(dev, vq);
+   }
}
 
rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
@@ -1440,8 +1442,10 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
do_data_copy_dequeue(vq);
if (unlikely(i < count))
vq->shadow_used_idx = i;
-   flush_shadow_used_ring_split(dev, vq);
-   vhost_vring_call_split(dev, vq);
+   if (likely(vq->shadow_used_idx)) {
+   flush_shadow_used_ring_split(dev, vq);
+   vhost_vring_call_split(dev, vq);
+   }
}
 
return i;
@@ -1476,8 +1480,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
}
}
 
-   flush_shadow_used_ring_packed(dev, vq);
-   vhost_vring_call_packed(dev, vq);
+   if (likely(vq->shadow_used_idx)) {
+   flush_shadow_used_ring_packed(dev, vq);
+   vhost_vring_call_packed(dev, vq);
+   }
}
 
VHOST_LOG_DEBUG(VHOST_DATA, "(%d) %s\n", dev->vid, __func__);
@@ -1555,8 +1561,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
do_data_copy_dequeue(vq);
if (unlikely(i < count))
vq->shadow_used_idx = i;
-   flush_shadow_used_ring_packed(dev, vq);
-   vhost_vring_call_packed(dev, vq);
+   if (likely(vq->shadow_used_idx)) {
+   flush_shadow_used_ring_packed(dev, vq);
+   vhost_vring_call_packed(dev, vq);
+   }
}
 
return i;
-- 
2.17.1



Re: [dpdk-dev] [PATCH v2] app/testpmd: support more types for flow RSS

2018-10-23 Thread Iremonger, Bernard
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wei Zhao
> Sent: Tuesday, October 23, 2018 10:21 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z ; sta...@dpdk.org; Peng, Yuan
> ; Zhao1, Wei 
> Subject: [dpdk-dev] [PATCH v2] app/testpmd: support more types for flow
> RSS
> 
> Some user and tester require flow RSS to support more types, so add "all"
> and "none" to make configuration more easy for users.
> 
> Tested-by: Peng Yuan 
> Signed-off-by: Wei Zhao 

Acked-by: Bernard Iremonger 


Re: [dpdk-dev] [PATCH v5 1/5] hash: separate multi-writer from rw-concurrency

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:22:00PM -0500, Honnappa Nagarahalli wrote:
> RW concurrency is required with single writer and multiple reader
> usecase as well. Hence, multi-writer should not be enabled by default when
> RW concurrency is enabled.
> 
> Fixes: f2e3001b53ec ("hash: support read/write concurrency")
> Cc: yipeng1.w...@intel.com
> 
> Signed-off-by: Honnappa Nagarahalli 
> Reviewed-by: Gavin Hu 
> Reviewed-by: Yipeng Wang 
> ---
Need to fix Yipeng's email address in the reviewed-by line.

Otherwise:
Acked-by: Bruce Richardson 


Re: [dpdk-dev] [PATCH v5 2/5] hash: support do not free on delete

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:22:01PM -0500, Honnappa Nagarahalli wrote:
> rte_hash_lookup_xxx APIs return the index of slot in
> the key store. Application(reader) can use that index to reference
> other data structures in its scope. Because of this, the
> index should not be freed till the application completes
> using the index.
> RTE_HASH_EXTRA_FLAGS_NO_FREE_ON_DEL is introduced to support this.
> When this flag is enabled rte_hash_del_xxx APIs do not free the
> key-store index/internal memory associated with the deleted
> entry. The new API rte_hash_free_key_with_position should be called
> to free the key-store index/internal memory after calling
> rte_hash_del_xxx APIs.
> 
> Suggested-by: Yipeng Wang 
> Signed-off-by: Honnappa Nagarahalli 
> Reviewed-by: Gavin Hu 
> Reviewed-by: Yipeng Wang 
> ---
As before, fix email in reviewed-by.

Acked-by: Bruce Richardson 


Re: [dpdk-dev] [PATCH v5 3/5] hash: fix key store element alignment

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:22:02PM -0500, Honnappa Nagarahalli wrote:
> Fix the key store array element alignment such that every array
> element is aligned on KEY_ALIGNMENT boundary. This is required to
> make 'pdata' in 'struct rte_hash_key' align on its natural boundary
> for atomic load/store.
> 
> Fixes: 473d1bebce43 ("hash: allow to store data in hash table")
> Cc: Pablo de Lara 
> 
> Signed-off-by: Honnappa Nagarahalli 
> Reviewed-by: Gavin Hu 
> Reviewed-by: Ola Liljedahl 
> Reviewed-by: Steve Capper 
> Reviewed-by: Yipeng Wang  
> ---
Fix email.

Acked-by: Bruce Richardson 


Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508

2018-10-23 Thread Shreyansh Jain
On Tuesday 23 October 2018 03:21 PM, Ferruh Yigit wrote:
> On 10/23/2018 8:09 AM, Shreyansh Jain wrote:
>> Besides the comment I sent before about 'Fixes' before sign-off, a
>> single trivial comment inline ...
>>
>> On Tuesday 23 October 2018 07:20 AM, Rosen Xu wrote:
>>> This patch fixes rte_eal_hotplug_add without checking return value issue
>>>
>>> Signed-off-by: Rosen Xu 
>>> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
>>> Cc: rosen...@intel.com
>>> ---
>>>drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 5 +++--
>>>1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c 
>>> b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>>> index 3fed057..32e318f 100644
>>> --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>>> +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
>>> @@ -542,6 +542,7 @@
>>> int port;
>>> char *name = NULL;
>>> char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
>>> +   int ret = -1;
>>>
>>> devargs = dev->device.devargs;
>>>
>>> @@ -583,7 +584,7 @@
>>> snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
>>> port, name);
>>>
>>> -   rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>>> +   ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>>> dev_name, devargs->args);
>>
>> Ideally, the function argument spreading on next line should start
>> underneath the previous arguments - something like:
>>
>>  ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>>dev_name, devargs->args);
> 
> Hi Shreyansh,
> 
> According dpdk coding convention [1], indentation done by hard tab, code seems
> inline with coding convention, only perhaps can be done single tab instead of
> double.
> 
> And to remind again, I am not for syntax discussions but just defining one and
> consistently follow it .
> 
> [1]
> https://doc.dpdk.org/guides/contributing/coding_style.html#c-indentation
> https://doc.dpdk.org/guides/contributing/coding_style.html#prototypes
> 

Oh!. Thanks - something I had missed reading.

I don't want to hijack the conversation, but for my clarity, I think

 >>>snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
 >>>port, name);

won't be correct. Right?
I am not suggesting that it should be changed now that it is already 
part of code.

>>
>> But, in this file this is not being done at multiple places (for
>> example, the snprintf in this code snippet). So, either you can ignore
>> this comment, or fix it for just this change.
>>
>>>end:
>>> if (kvlist)
>>> @@ -591,7 +592,7 @@
>>> if (name)
>>> free(name);
>>>
>>> -   return 0;
>>> +   return ret;
>>>}
>>>
>>>static int
>>>
>>
>> Otherwise, the patch is simple enough.
>>
>> Acked-by: Shreyansh Jain 
>>
> 



Re: [dpdk-dev] [dpdk-users] IPV4/IPV6 TCP/UDP Pseudo Header Checksum APIs

2018-10-23 Thread Shyam Shrivastav
These fixes/modifications should include the upper level APIs,
rte_ipv4_udptcp_cksum and rte_ipv6_udptcp_cksum.

Even for ipv4 following API is more/really useful if changed to take mbufs

rte_ipv4_udptcp_cksum(const struct ipv4_hdr *ipv4_hdr, const void *l4_hdr)

I can not use it in present form as it requires single contiguous l4
buffer, instead using rte_raw_cksum_mbuf.


[dpdk-dev] [PATCH v6] net/mlx5: support metadata as flow rule criteria

2018-10-23 Thread Dekel Peled
As described in series starting at [1], it adds option to set
metadata value as match pattern when creating a new flow rule.

This patch adds metadata support in mlx5 driver, in two parts:
- Add the validation and setting of metadata value in matcher,
  when creating a new flow rule.
- Add the passing of metadata value from mbuf to wqe when
  indicated by ol_flag, in different burst functions.

[1] "ethdev: support metadata as flow rule criteria"
http://mails.dpdk.org/archives/dev/2018-September/113269.html

---
v6:
- Correct indentation.
- Fix setting data in matcher to include mask.
v5:
Apply code review comments:
 Coding style (indentation, redundant blank lines, clear comments).
 txq_calc_offload() logic updated.
 rte_be32_t type used instead of uint32_t.
v4:
- Rebase.
- Apply code review comments.
v3:
- Update meta item validation.
v2:
- Split the support of egress rules to a different patch.
---

Signed-off-by: Dekel Peled 
---
 drivers/net/mlx5/mlx5_flow.c  |   2 +-
 drivers/net/mlx5/mlx5_flow.h  |   8 +++
 drivers/net/mlx5/mlx5_flow_dv.c   | 106 ++
 drivers/net/mlx5/mlx5_prm.h   |   2 +-
 drivers/net/mlx5/mlx5_rxtx.c  |  32 --
 drivers/net/mlx5/mlx5_rxtx_vec.c  |  46 +++
 drivers/net/mlx5/mlx5_rxtx_vec.h  |   1 +
 drivers/net/mlx5/mlx5_rxtx_vec_neon.h |   9 ++-
 drivers/net/mlx5/mlx5_rxtx_vec_sse.h  |  10 ++--
 drivers/net/mlx5/mlx5_txq.c   |   5 +-
 10 files changed, 192 insertions(+), 29 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index fcabab0..df5c34e 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -418,7 +418,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, 
int32_t priority,
  * @return
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
-static int
+int
 mlx5_flow_item_acceptable(const struct rte_flow_item *item,
  const uint8_t *mask,
  const uint8_t *nic_mask,
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index af0a125..38635c9 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -43,6 +43,9 @@
 #define MLX5_FLOW_LAYER_GRE (1u << 14)
 #define MLX5_FLOW_LAYER_MPLS (1u << 15)
 
+/* General pattern items bits. */
+#define MLX5_FLOW_ITEM_METADATA (1u << 16)
+
 /* Outer Masks. */
 #define MLX5_FLOW_LAYER_OUTER_L3 \
(MLX5_FLOW_LAYER_OUTER_L3_IPV4 | MLX5_FLOW_LAYER_OUTER_L3_IPV6)
@@ -316,6 +319,11 @@ int mlx5_flow_validate_action_rss(const struct 
rte_flow_action *action,
 int mlx5_flow_validate_attributes(struct rte_eth_dev *dev,
  const struct rte_flow_attr *attributes,
  struct rte_flow_error *error);
+int mlx5_flow_item_acceptable(const struct rte_flow_item *item,
+ const uint8_t *mask,
+ const uint8_t *nic_mask,
+ unsigned int size,
+ struct rte_flow_error *error);
 int mlx5_flow_validate_item_eth(const struct rte_flow_item *item,
uint64_t item_flags,
struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 58e3c33..e8f409f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -36,6 +36,67 @@
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 
 /**
+ * Validate META item.
+ *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
+ * @param[in] item
+ *   Item specification.
+ * @param[in] attr
+ *   Attributes of flow that includes this item.
+ * @param[out] error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_dv_validate_item_meta(struct rte_eth_dev *dev,
+  const struct rte_flow_item *item,
+  const struct rte_flow_attr *attr,
+  struct rte_flow_error *error)
+{
+   const struct rte_flow_item_meta *spec = item->spec;
+   const struct rte_flow_item_meta *mask = item->mask;
+   const struct rte_flow_item_meta nic_mask = {
+   .data = RTE_BE32(UINT32_MAX)
+   };
+   int ret;
+   uint64_t offloads = dev->data->dev_conf.txmode.offloads;
+
+   if (!(offloads & DEV_TX_OFFLOAD_MATCH_METADATA))
+   return rte_flow_error_set(error, EPERM,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ NULL,
+ "match on metadata offload "
+ "configuration is off for this port");
+   if (!spec)
+   return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR

[dpdk-dev] [PATCH v3 1/5] test: remove existing testcases for categorization

2018-10-23 Thread Hari kumar Vemula
From: Hari Kumar Vemula 

Removed testcase list from meson build to categorize testcases
into suites

Signed-off-by: Hari Kumar Vemula 
---
 test/test/meson.build | 111 --
 1 file changed, 111 deletions(-)

diff --git a/test/test/meson.build b/test/test/meson.build
index 7c6e3b00b..088a44726 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -122,110 +122,6 @@ test_deps = ['acl',
'timer'
 ]
 
-test_names = [
-   'acl_autotest',
-   'alarm_autotest',
-   'atomic_autotest',
-   'barrier_autotest',
-   'byteorder_autotest',
-   'cmdline_autotest',
-   'common_autotest',
-   'cpuflags_autotest',
-   'crc_autotest',
-   'cryptodev_qat_autotest',
-   'cryptodev_aesni_mb_autotest',
-   'cryptodev_openssl_autotest',
-   'cryptodev_openssl_asym_autotest',
-   'cryptodev_aesni_gcm_autotest',
-   'cryptodev_null_autotest',
-   'cryptodev_sw_snow3g_autotest',
-   'cryptodev_sw_kasumi_autotest',
-   'cryptodev_sw_zuc_autotest',
-   'cryptodev_sw_armv8_autotest',
-   'cryptodev_sw_mvsam_autotest',
-   'cryptodev_dpaa2_sec_autotest',
-   'cryptodev_dpaa_sec_autotest',
-   'cryptodev_octeontx_autotest',
-   'cycles_autotest',
-   'debug_autotest',
-   'devargs_autotest',
-   'distributor_autotest',
-   'distributor_perf_autotest',
-   'eal_flags_autotest',
-   'eal_fs_autotest',
-   'efd_autotest',
-   'efd_perf_autotest',
-   'errno_autotest',
-   'event_crypto_adapter_autotest',
-   'event_eth_rx_adapter_autotest',
-   'event_eth_rx_intr_adapter_autotest',
-   'event_ring_autotest',
-   'event_eth_tx_adapter_autotest',
-   'event_timer_adapter_autotest',
-   'eventdev_common_autotest',
-   'eventdev_octeontx_autotest',
-   'eventdev_sw_autotest',
-   'external_mem_autotest',
-   'func_reentrancy_autotest',
-   'flow_classify_autotest',
-   'hash_scaling_autotest',
-   'hash_autotest',
-   'hash_functions_autotest',
-   'hash_multiwriter_autotest',
-   'hash_perf_autotest',
-   'interrupt_autotest',
-   'kni_autotest',
-   'kvargs_autotest',
-   'link_bonding_autotest',
-   'link_bonding_mode4_autotest',
-   'logs_autotest',
-   'lpm6_autotest',
-   'lpm6_perf_autotest',
-   'lpm_autotest',
-   'lpm_perf_autotest',
-   'malloc_autotest',
-   'mbuf_autotest',
-   'member_autotest',
-   'member_perf_autotest',
-   'memcpy_autotest',
-   'memcpy_perf_autotest',
-   'memory_autotest',
-   'mempool_autotest',
-   'mempool_perf_autotest',
-   'memzone_autotest',
-   'meter_autotest',
-   'multiprocess_autotest',
-   'per_lcore_autotest',
-   'pmd_perf_autotest',
-   'power_acpi_cpufreq_autotest',
-   'power_autotest',
-   'power_kvm_vm_autotest',
-   'prefetch_autotest',
-   'reciprocal_division',
-   'reciprocal_division_perf',
-   'red_all',
-   'red_autotest',
-   'red_perf',
-   'reorder_autotest',
-   'ring_autotest',
-   'ring_perf_autotest',
-   'ring_pmd_autotest',
-   'ring_pmd_perf_autotest',
-   'rwlock_autotest',
-   'sched_autotest',
-   'service_autotest',
-   'spinlock_autotest',
-   'string_autotest',
-   'table_autotest',
-   'tailq_autotest',
-   'thash_autotest',
-   'timer_autotest',
-   'timer_perf__autotest',
-   'timer_racecond_autotest',
-   'user_delay_us',
-   'version_autotest',
-]
-
 if dpdk_conf.has('RTE_LIBRTE_PDUMP')
test_deps += 'pdump'
 endif
@@ -262,7 +158,6 @@ if compress_test_dep.found()
test_dep_objs += compress_test_dep
test_sources += 'test_compressdev.c'
test_deps += 'compressdev'
-   test_names += 'compressdev_autotest'
 endif
 
 foreach d:test_deps
@@ -289,10 +184,4 @@ if get_option('tests')
# to complete, so timeout to 10 minutes
timeout_seconds = 600
 
-   foreach t:test_names
-   test(t, dpdk_test,
-   env : ['DPDK_TEST='+t],
-   timeout : timeout_seconds,
-   is_parallel : false)
-   endforeach
 endif
-- 
2.13.6



[dpdk-dev] [PATCH v3 2/5] test: add quick run tests under test-fast suite

2018-10-23 Thread Hari kumar Vemula
From: Hari Kumar Vemula 

Added test cases that runs quickly under test fast category

Signed-off-by: Hari Kumar Vemula 
---
 test/test/meson.build | 99 ---
 1 file changed, 94 insertions(+), 5 deletions(-)

diff --git a/test/test/meson.build b/test/test/meson.build
index 088a44726..825e84fce 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -122,6 +122,75 @@ test_deps = ['acl',
'timer'
 ]
 
+#All test cases in fast_parallel_test_names list are parallel
+fast_parallel_test_names =[
+'acl_autotest',
+'alarm_autotest',
+'atomic_autotest',
+'byteorder_autotest',
+'cmdline_autotest',
+'common_autotest',
+'cpuflags_autotest',
+'cycles_autotest',
+'debug_autotest',
+'eal_flags_autotest',
+'eal_fs_autotest',
+'errno_autotest',
+'event_ring_autotest',
+'func_reentrancy_autotest',
+'flow_classify_autotest',
+'hash_autotest',
+'interrupt_autotest',
+'logs_autotest',
+'lpm6_autotest',
+'lpm_autotest',
+'malloc_autotest',
+'mbuf_autotest',
+'memcpy_autotest',
+'memory_autotest',
+'mempool_autotest',
+'memzone_autotest',
+'meter_autotest',
+'multiprocess_autotest',
+'per_lcore_autotest',
+'prefetch_autotest',
+'red_autotest',
+'ring_autotest',
+'ring_pmd_autotest',
+'rwlock_autotest',
+'sched_autotest',
+'spinlock_autotest',
+'string_autotest',
+'table_autotest',
+'tailq_autotest',
+'timer_autotest',
+'user_delay_us',
+'version_autotest',
+]
+
+#All test cases in fast_non_parallel_test_names list are non-parallel
+fast_non_parallel_test_names =[
+'cryptodev_sw_armv8_autotest',
+'crc_autotest',
+'cryptodev_openssl_asym_autotest',
+'cryptodev_sw_mvsam_autotest',
+'devargs_autotest',
+'distributor_autotest',
+'eventdev_common_autotest',
+'eventdev_octeontx_autotest',
+'eventdev_sw_autotest',
+'hash_scaling_autotest',
+'kni_autotest',
+'kvargs_autotest',
+'member_autotest',
+'power_acpi_cpufreq_autotest',
+'power_autotest',
+'power_kvm_vm_autotest',
+'reorder_autotest',
+'service_autotest',
+'thash_autotest',
+]
+
 if dpdk_conf.has('RTE_LIBRTE_PDUMP')
test_deps += 'pdump'
 endif
@@ -153,11 +222,14 @@ endif
 default_cflags += '-D_GNU_SOURCE'
 
 test_dep_objs = []
-compress_test_dep = dependency('zlib', required: false)
-if compress_test_dep.found()
-   test_dep_objs += compress_test_dep
-   test_sources += 'test_compressdev.c'
-   test_deps += 'compressdev'
+if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
+   compress_test_dep = dependency('zlib', required: false)
+   if compress_test_dep.found()
+   test_dep_objs += compress_test_dep
+   test_sources += 'test_compressdev.c'
+   test_deps += 'compressdev'
+   fast_non_parallel_test_names += 'compressdev_autotest'
+   endif
 endif
 
 foreach d:test_deps
@@ -183,5 +255,22 @@ if get_option('tests')
# some perf tests (eg: memcpy perf autotest)take very long
# to complete, so timeout to 10 minutes
timeout_seconds = 600
+timeout_seconds_fast = 10
+
+foreach arg : fast_parallel_test_names
+test(arg, dpdk_test,
+env : ['DPDK_TEST=' + arg],
+args : ['-c f','-n 4', 
'--file-prefix=test@0@'.format(arg)],
+timeout : timeout_seconds_fast,
+suite : 'test-fast')
+endforeach
+
+foreach arg : fast_non_parallel_test_names
+test(arg, dpdk_test,
+env : ['DPDK_TEST=' + arg],
+timeout : timeout_seconds_fast,
+is_parallel : false,
+suite : 'test-fast')
+endforeach
 
 endif
-- 
2.13.6



[dpdk-dev] [PATCH v3 3/5] test: add performance tests under test-perf suite

2018-10-23 Thread Hari kumar Vemula
From: Hari Kumar Vemula 

Grouped performace test cases under test-perf category

Signed-off-by: Hari Kumar Vemula 
---
 test/test/meson.build | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/test/test/meson.build b/test/test/meson.build
index 825e84fce..70a4c6bed 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -191,6 +191,32 @@ fast_non_parallel_test_names =[
 'thash_autotest',
 ]
 
+#All test cases in perf_test_names list are non-parallel
+perf_test_names = [
+'ring_perf_autotest',
+'mempool_perf_autotest',
+'memcpy_perf_autotest',
+'hash_perf_autotest',
+'timer_perf_autotest',
+'reciprocal_division',
+'reciprocal_division_perf',
+'lpm_perf_autotest',
+'red_all',
+'barrier_autotest',
+'hash_multiwriter_autotest',
+'timer_racecond_autotest',
+'efd_autotest',
+'hash_functions_autotest',
+'eventdev_selftest_sw',
+'member_perf_autotest',
+'efd_perf_autotest',
+'lpm6_perf_autotest',
+'red_perf',
+'distributor_perf_autotest',
+'ring_pmd_perf_autotest',
+'pmd_perf_autotest',
+]
+
 if dpdk_conf.has('RTE_LIBRTE_PDUMP')
test_deps += 'pdump'
 endif
@@ -273,4 +299,12 @@ if get_option('tests')
 suite : 'test-fast')
 endforeach
 
+foreach arg : perf_test_names
+test(arg, dpdk_test,
+env : ['DPDK_TEST=' + arg],
+timeout : timeout_seconds,
+is_parallel : false,
+suite : 'test-perf')
+endforeach
+
 endif
-- 
2.13.6



[dpdk-dev] [PATCH v3 0/5] create different meson test targets

2018-10-23 Thread Hari kumar Vemula
From: Hari Kumar Vemula 

1/5: remove existing test cases to reorganize further
2/5: add test-fast suite to meson targets
3/5: add test-perf suite to meson targets
4/5: add test-driver suite to meson targets
5/5: add test-dump suite to meson targets

--
v3: Updated testcase names in file prefix option
v2: Divided fast-test list into two lists
--

Hari Kumar Vemula (5):
  test: remove existing testcases for categorization
  test: add quick run tests under test-fast suite
  test: add performance tests under test-perf suite
  test: add library dependent tests under test-driver suite
  test: add dump test cases under test-dump suite

 test/test/meson.build | 288 ++
 1 file changed, 174 insertions(+), 114 deletions(-)

-- 
2.13.6



[dpdk-dev] [PATCH v3 4/5] test: add library dependent tests under test-driver suite

2018-10-23 Thread Hari kumar Vemula
From: Hari Kumar Vemula 

Added test cases that depend on library as cryptodev

Signed-off-by: Hari Kumar Vemula 
---
 test/test/meson.build | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/test/test/meson.build b/test/test/meson.build
index 70a4c6bed..a727cf461 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -217,6 +217,25 @@ perf_test_names = [
 'pmd_perf_autotest',
 ]
 
+#All test cases in driver_test_names list are non-parallel
+driver_test_names = [
+'link_bonding_autotest',
+'link_bonding_mode4_autotest',
+'link_bonding_rssconf_autotest',
+'cryptodev_sw_mrvl_autotest',
+'cryptodev_dpaa2_sec_autotest',
+'cryptodev_dpaa_sec_autotest',
+'cryptodev_qat_autotest',
+'cryptodev_aesni_mb_autotest',
+'cryptodev_openssl_autotest',
+'cryptodev_scheduler_autotest',
+'cryptodev_aesni_gcm_autotest',
+'cryptodev_null_autotest',
+'cryptodev_sw_snow3g_autotest',
+'cryptodev_sw_kasumi_autotest',
+'cryptodev_sw_zuc_autotest',
+]
+
 if dpdk_conf.has('RTE_LIBRTE_PDUMP')
test_deps += 'pdump'
 endif
@@ -307,4 +326,12 @@ if get_option('tests')
 suite : 'test-perf')
 endforeach
 
+foreach arg : driver_test_names
+test(arg, dpdk_test,
+env : ['DPDK_TEST=' + arg],
+timeout : timeout_seconds,
+is_parallel : false,
+suite : 'test-driver')
+endforeach
+
 endif
-- 
2.13.6



[dpdk-dev] [PATCH v3 5/5] test: add dump test cases under test-dump suite

2018-10-23 Thread Hari kumar Vemula
From: Hari Kumar Vemula 

Grouped logging or dump related test cases to test-dump category

Signed-off-by: Hari Kumar Vemula 
---
 test/test/meson.build | 21 +
 1 file changed, 21 insertions(+)

diff --git a/test/test/meson.build b/test/test/meson.build
index a727cf461..9132ad0c1 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -236,6 +236,20 @@ driver_test_names = [
 'cryptodev_sw_zuc_autotest',
 ]
 
+#All test cases in dump_test_names list are non-parallel
+dump_test_names = [
+'dump_struct_sizes',
+'dump_mempool',
+'dump_malloc_stats',
+'dump_devargs',
+'dump_log_types',
+'dump_ring',
+'quit_autotest',
+'dump_physmem',
+'dump_memzone',
+'devargs_autotest',
+]
+
 if dpdk_conf.has('RTE_LIBRTE_PDUMP')
test_deps += 'pdump'
 endif
@@ -334,4 +348,11 @@ if get_option('tests')
 suite : 'test-driver')
 endforeach
 
+foreach arg : dump_test_names
+test(arg, dpdk_test,
+env : ['DPDK_TEST=' + arg],
+timeout : timeout_seconds,
+is_parallel : false,
+suite : 'test-dump')
+endforeach
 endif
-- 
2.13.6



Re: [dpdk-dev] [PATCH v5 4/5] hash: add lock-free read-write concurrency

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:22:03PM -0500, Honnappa Nagarahalli wrote:
> Add lock-free read-write concurrency. This is achieved by the
> following changes.
> 
> 1) Add memory ordering to avoid race conditions. The only race
> condition that can occur is -  using the key store element
> before the key write is completed. Hence, while inserting the element
> the release memory order is used. Any other race condition is caught
> by the key comparison. Memory orderings are added only where needed.
> For ex: reads in the writer's context do not need memory ordering
> as there is a single writer.
> 
> key_idx in the bucket entry and pdata in the key store element are
> used for synchronisation. key_idx is used to release an inserted
> entry in the bucket to the reader. Use of pdata for synchronisation
> is required due to updation of an existing entry where-in only
> the pdata is updated without updating key_idx.
> 
> 2) Reader-writer concurrency issue, caused by moving the keys
> to their alternative locations during key insert, is solved
> by introducing a global counter(tbl_chng_cnt) indicating a
> change in table.
> 
> 3) Add the flag to enable reader-writer concurrency during
> run time.
> 
> Signed-off-by: Honnappa Nagarahalli 
> Reviewed-by: Gavin Hu 
> Reviewed-by: Ola Liljedahl 
> Reviewed-by: Steve Capper 
> Reviewed-by: Yipeng Wang 
> ---
Acked-by: Bruce Richardson 


Re: [dpdk-dev] [PATCH v5 5/5] test/hash: read-write lock-free concurrency test

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:22:04PM -0500, Honnappa Nagarahalli wrote:
> From: Dharmik Thakkar 
> 
> Unit tests to check for hash lookup and bulk-lookup perf
> with lock-free enabled and with lock-free disabled.
> Unit tests performed with readers running in parallel with writers.
> 
> Tests include:
> 
> - hash lookup on existing keys with:
>   - hash add causing NO key-shifts of existing keys in the table
> 
> - hash lookup on existing keys likely to be on shift-path with:
>   - hash add causing key-shifts of existing keys in the table
> 
> - hash lookup on existing keys NOT likely to be on shift-path with:
>   - hash add causing key-shifts of existing keys in the table
> 
> - hash lookup on non-existing keys with:
>   - hash add causing NO key-shifts of existing keys in the table
>   - hash add causing key-shifts of existing keys in the table
> 
> - hash lookup on keys likely to be on shift-path with:
>   - multiple writers causing key-shifts of existing keys in the table
> 
> Signed-off-by: Dharmik Thakkar 
> Reviewed-by: Honnappa Nagarahalli 
> Reviewed-by: Gavin Hu 
> Reviewed-by: Yipeng Wang 
> ---

Does this need to be done via a completely new test case? Given the number
of unit tests for the hash table structure, I'm wondering if we can
consolidate things a bit. Any thoughts?

/Bruce


Re: [dpdk-dev] [PATCH v5 5/5] test/hash: read-write lock-free concurrency test

2018-10-23 Thread Bruce Richardson
On Mon, Oct 22, 2018 at 11:22:04PM -0500, Honnappa Nagarahalli wrote:
> From: Dharmik Thakkar 
> 
> Unit tests to check for hash lookup and bulk-lookup perf
> with lock-free enabled and with lock-free disabled.
> Unit tests performed with readers running in parallel with writers.
> 
> Tests include:
> 
> - hash lookup on existing keys with:
>   - hash add causing NO key-shifts of existing keys in the table
> 
> - hash lookup on existing keys likely to be on shift-path with:
>   - hash add causing key-shifts of existing keys in the table
> 
> - hash lookup on existing keys NOT likely to be on shift-path with:
>   - hash add causing key-shifts of existing keys in the table
> 
> - hash lookup on non-existing keys with:
>   - hash add causing NO key-shifts of existing keys in the table
>   - hash add causing key-shifts of existing keys in the table
> 
> - hash lookup on keys likely to be on shift-path with:
>   - multiple writers causing key-shifts of existing keys in the table
> 
> Signed-off-by: Dharmik Thakkar 
> Reviewed-by: Honnappa Nagarahalli 
> Reviewed-by: Gavin Hu 
> Reviewed-by: Yipeng Wang 
> ---
>  test/test/Makefile |1 +
>  test/test/meson.build  |1 +
>  test/test/test_hash_readwrite_lf.c | 1220 
> 
>  3 files changed, 1222 insertions(+)
>  create mode 100644 test/test/test_hash_readwrite_lf.c
> 
> diff --git a/test/test/Makefile b/test/test/Makefile
> index 8c347e4..1b5c070 100644
> --- a/test/test/Makefile
> +++ b/test/test/Makefile
> @@ -116,6 +116,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c
>  SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_scaling.c
>  SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c
>  SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite.c
> +SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf.c
>  
>  SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm.c
>  SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm_perf.c
> diff --git a/test/test/meson.build b/test/test/meson.build
> index 90d6607..d352b94 100644
> --- a/test/test/meson.build
> +++ b/test/test/meson.build
> @@ -46,6 +46,7 @@ test_sources = files('commands.c',
>   'test_hash_multiwriter.c',
>   'test_hash_readwrite.c',
>   'test_hash_perf.c',
> + 'test_hash_readwrite_lf.c',
>   'test_hash_scaling.c',
>   'test_interrupts.c',
>   'test_kni.c',

Two minor issues I spot, both related:

* You also need to add the test to the list of test names, so that it can be
called using "meson test".

* For running the autotests built using make, a new entry should be added to
autotest_data.py to run this new test case.

/Bruce


Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching device twice

2018-10-23 Thread Thomas Monjalon
23/10/2018 12:01, Iremonger, Bernard:
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > The command "port detach" is removing the EAL rte_device of the ethdev
> > port specified as parameter.
> > 
> > After detaching, the pointer, which maps a port to its device, is resetted. 
> > This
> 
> Typo:  "resetted" should be "reset"
> 
> > way, it is possible to check whether a port is still associated to a (not
> > removed) device.
> > 
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  app/test-pmd/testpmd.c | 24 +---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 14647fa19..d5998fddc 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)  void
> > detach_port(portid_t port_id)  {
> > +   struct rte_device *dev;
> > +   portid_t sibling;
> > +
> > printf("Removing a device...\n");
> 
> The functionality of the detach_port() function has changed now to
> removing a device, should the function name be changed to reflect
> the new functionality.

No it doesn't change, it has always removed the rte_device of the port.
But the naming is a bit strange, I agree.
I just changed the log to make it a bit clearer.

> How about detach_device() instead of detach detach_port().

The strange thing with testpmd is that every commands take a port id.
The rte_device is hidden in testpmd.
So the detach command is detaching the underlying device of the port,
and all its sibling ports of course.

What about detach_device_of_port() ?

[...]
> > -   if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
> > +   if (rte_dev_remove(dev) != 0) {
> > TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
> 
> Should the log message be ( ERR "Failed to detach device %s\n", dev->name) ?

Yes!

[...]
> > -   printf("Port %u is detached. Now total ports is %d\n",
> > -   port_id, nb_ports);
> 
> How about printf("Device %s is detached, Now total ports is %d\n"
>   dev->name, nb_ports);

The issue is that we cannot get the device name after detach.
I can reword it differently:
Device of port %u is detached, Now total ports is %d





Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching device twice

2018-10-23 Thread Thomas Monjalon
23/10/2018 14:03, Thomas Monjalon:
> 23/10/2018 12:01, Iremonger, Bernard:
> > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > The command "port detach" is removing the EAL rte_device of the ethdev
> > > port specified as parameter.
> > > 
> > > After detaching, the pointer, which maps a port to its device, is 
> > > resetted. This
> > 
> > Typo:  "resetted" should be "reset"
> > 
> > > way, it is possible to check whether a port is still associated to a (not
> > > removed) device.
> > > 
> > > Signed-off-by: Thomas Monjalon 
> > > ---
> > >  app/test-pmd/testpmd.c | 24 +---
> > >  1 file changed, 21 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 14647fa19..d5998fddc 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)  void
> > > detach_port(portid_t port_id)  {
> > > + struct rte_device *dev;
> > > + portid_t sibling;
> > > +
> > >   printf("Removing a device...\n");
> > 
> > The functionality of the detach_port() function has changed now to
> > removing a device, should the function name be changed to reflect
> > the new functionality.
> 
> No it doesn't change, it has always removed the rte_device of the port.
> But the naming is a bit strange, I agree.
> I just changed the log to make it a bit clearer.
> 
> > How about detach_device() instead of detach detach_port().
> 
> The strange thing with testpmd is that every commands take a port id.
> The rte_device is hidden in testpmd.
> So the detach command is detaching the underlying device of the port,
> and all its sibling ports of course.
> 
> What about detach_device_of_port() ?

Or detach_port_device()?

> [...]
> > > - if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
> > > + if (rte_dev_remove(dev) != 0) {
> > >   TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
> > 
> > Should the log message be ( ERR "Failed to detach device %s\n", dev->name) ?
> 
> Yes!
> 
> [...]
> > > - printf("Port %u is detached. Now total ports is %d\n",
> > > - port_id, nb_ports);
> > 
> > How about printf("Device %s is detached, Now total ports is %d\n"
> > dev->name, nb_ports);
> 
> The issue is that we cannot get the device name after detach.
> I can reword it differently:
>   Device of port %u is detached, Now total ports is %d
> 
> 
> 
> 







Re: [dpdk-dev] [PATCH] drivers/raw/ifpga_rawdev: fix coverity issue 323508

2018-10-23 Thread Ferruh Yigit
On 10/23/2018 11:43 AM, Shreyansh Jain wrote:
> On Tuesday 23 October 2018 03:21 PM, Ferruh Yigit wrote:
>> On 10/23/2018 8:09 AM, Shreyansh Jain wrote:
>>> Besides the comment I sent before about 'Fixes' before sign-off, a
>>> single trivial comment inline ...
>>>
>>> On Tuesday 23 October 2018 07:20 AM, Rosen Xu wrote:
 This patch fixes rte_eal_hotplug_add without checking return value issue

 Signed-off-by: Rosen Xu 
 Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
 Cc: rosen...@intel.com
 ---
drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c 
 b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
 index 3fed057..32e318f 100644
 --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
 +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c
 @@ -542,6 +542,7 @@
int port;
char *name = NULL;
char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
 +  int ret = -1;

devargs = dev->device.devargs;

 @@ -583,7 +584,7 @@
snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
port, name);

 -  rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
 +  ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
dev_name, devargs->args);
>>>
>>> Ideally, the function argument spreading on next line should start
>>> underneath the previous arguments - something like:
>>>
>>> ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
>>>   dev_name, devargs->args);
>>
>> Hi Shreyansh,
>>
>> According dpdk coding convention [1], indentation done by hard tab, code 
>> seems
>> inline with coding convention, only perhaps can be done single tab instead of
>> double.
>>
>> And to remind again, I am not for syntax discussions but just defining one 
>> and
>> consistently follow it .
>>
>> [1]
>> https://doc.dpdk.org/guides/contributing/coding_style.html#c-indentation
>> https://doc.dpdk.org/guides/contributing/coding_style.html#prototypes
>>
> 
> Oh!. Thanks - something I had missed reading.
> 
> I don't want to hijack the conversation, but for my clarity, I think
> 
>  >>>  snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
>  >>>  port, name);
> 
> won't be correct. Right?

You are right, it doesn't look correct.

> I am not suggesting that it should be changed now that it is already 
> part of code.
> 
>>>
>>> But, in this file this is not being done at multiple places (for
>>> example, the snprintf in this code snippet). So, either you can ignore
>>> this comment, or fix it for just this change.
>>>
end:
if (kvlist)
 @@ -591,7 +592,7 @@
if (name)
free(name);

 -  return 0;
 +  return ret;
}

static int

>>>
>>> Otherwise, the patch is simple enough.
>>>
>>> Acked-by: Shreyansh Jain 
>>>
>>
> 



Re: [dpdk-dev] [PATCH v6] net/mlx5: support metadata as flow rule criteria

2018-10-23 Thread Shahaf Shuler
Tuesday, October 23, 2018 1:48 PM, Dekel Peled:
> Subject: [PATCH v6] net/mlx5: support metadata as flow rule criteria
> 
> As described in series starting at [1], it adds option to set metadata value 
> as
> match pattern when creating a new flow rule.
> 
> This patch adds metadata support in mlx5 driver, in two parts:
> - Add the validation and setting of metadata value in matcher,
>   when creating a new flow rule.
> - Add the passing of metadata value from mbuf to wqe when
>   indicated by ol_flag, in different burst functions.
> 
> [1] "ethdev: support metadata as flow rule criteria"
> http://mails.dpdk.org/archives/dev/2018-September/113269.html
> 
> ---
> v6:
> - Correct indentation.
> - Fix setting data in matcher to include mask.
> v5:
> Apply code review comments:
>  Coding style (indentation, redundant blank lines, clear comments).
>  txq_calc_offload() logic updated.
>  rte_be32_t type used instead of uint32_t.
> v4:
> - Rebase.
> - Apply code review comments.
> v3:
> - Update meta item validation.
> v2:
> - Split the support of egress rules to a different patch.
> ---
> 
> Signed-off-by: Dekel Peled 
> ---

Applied to next-net-mlx, thanks.



Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching device twice

2018-10-23 Thread Thomas Monjalon
I want to submit two more patches to clean testpmd for attach/detach.

I propose to drop this patch from this series,
and I will submit a new series dedicated to testpmd cleanup,
including this patch.


23/10/2018 14:13, Thomas Monjalon:
> 23/10/2018 14:03, Thomas Monjalon:
> > 23/10/2018 12:01, Iremonger, Bernard:
> > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > The command "port detach" is removing the EAL rte_device of the ethdev
> > > > port specified as parameter.
> > > > 
> > > > After detaching, the pointer, which maps a port to its device, is 
> > > > resetted. This
> > > 
> > > Typo:  "resetted" should be "reset"
> > > 
> > > > way, it is possible to check whether a port is still associated to a 
> > > > (not
> > > > removed) device.
> > > > 
> > > > Signed-off-by: Thomas Monjalon 
> > > > ---
> > > >  app/test-pmd/testpmd.c | 24 +---
> > > >  1 file changed, 21 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > > 14647fa19..d5998fddc 100644
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> > > > @@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)  void
> > > > detach_port(portid_t port_id)  {
> > > > +   struct rte_device *dev;
> > > > +   portid_t sibling;
> > > > +
> > > > printf("Removing a device...\n");
> > > 
> > > The functionality of the detach_port() function has changed now to
> > > removing a device, should the function name be changed to reflect
> > > the new functionality.
> > 
> > No it doesn't change, it has always removed the rte_device of the port.
> > But the naming is a bit strange, I agree.
> > I just changed the log to make it a bit clearer.
> > 
> > > How about detach_device() instead of detach detach_port().
> > 
> > The strange thing with testpmd is that every commands take a port id.
> > The rte_device is hidden in testpmd.
> > So the detach command is detaching the underlying device of the port,
> > and all its sibling ports of course.
> > 
> > What about detach_device_of_port() ?
> 
> Or detach_port_device()?
> 
> > [...]
> > > > -   if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
> > > > +   if (rte_dev_remove(dev) != 0) {
> > > > TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
> > > 
> > > Should the log message be ( ERR "Failed to detach device %s\n", 
> > > dev->name) ?
> > 
> > Yes!
> > 
> > [...]
> > > > -   printf("Port %u is detached. Now total ports is %d\n",
> > > > -   port_id, nb_ports);
> > > 
> > > How about printf("Device %s is detached, Now total ports is %d\n"
> > >   dev->name, nb_ports);
> > 
> > The issue is that we cannot get the device name after detach.
> > I can reword it differently:
> > Device of port %u is detached, Now total ports is %d






Re: [dpdk-dev] [dpdk-stable] [PATCH v2] vhost: avoid memory barriers when no descriptors dequeued

2018-10-23 Thread Jens Freimann

On Tue, Oct 23, 2018 at 12:07:10PM +0200, Maxime Coquelin wrote:

In both split and packed dequeue paths, flush_shadow_used_ring
and vhost_ring_call variants gets called even if not packets
have been dequeued, and so no descriptors updates happened.

It has an impact on CPU pipeline, as memory barriers are used
in these functions.

This patch don't call these functions if no descriptors have
been dequeued. The performance gain with split ring when
dequeue zero-copy is disabled should be null, but should be
noticeable with packed ring or dequeue zero-copy enabled.


I tried this with packed ring pmd patch series v8 and it works fine.
It doesn't hurt performance either, I see improvements in sending
packets from guest to host. 

Reviewed-by: Jens Freimann  
Tested-by: Jens Freimann 


regards,
Jens 


Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching device twice

2018-10-23 Thread Iremonger, Bernard
Hi Thomas


> Subject: Re: [dpdk-dev] [PATCH v7 7/7] app/testpmd: check not detaching
> device twice
> 
> 23/10/2018 14:03, Thomas Monjalon:
> > 23/10/2018 12:01, Iremonger, Bernard:
> > > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > > > The command "port detach" is removing the EAL rte_device of the
> > > > ethdev port specified as parameter.
> > > >
> > > > After detaching, the pointer, which maps a port to its device, is
> > > > resetted. This
> > >
> > > Typo:  "resetted" should be "reset"
> > >
> > > > way, it is possible to check whether a port is still associated to
> > > > a (not
> > > > removed) device.
> > > >
> > > > Signed-off-by: Thomas Monjalon 
> > > > ---
> > > >  app/test-pmd/testpmd.c | 24 +---
> > > >  1 file changed, 21 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > > 14647fa19..d5998fddc 100644
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> > > > @@ -2353,8 +2353,17 @@ setup_attached_port(portid_t pi)  void
> > > > detach_port(portid_t port_id)  {
> > > > +   struct rte_device *dev;
> > > > +   portid_t sibling;
> > > > +
> > > > printf("Removing a device...\n");
> > >
> > > The functionality of the detach_port() function has changed now to
> > > removing a device, should the function name be changed to reflect
> > > the new functionality.
> >
> > No it doesn't change, it has always removed the rte_device of the port.
> > But the naming is a bit strange, I agree.
> > I just changed the log to make it a bit clearer.
> >
> > > How about detach_device() instead of detach detach_port().
> >
> > The strange thing with testpmd is that every commands take a port id.
> > The rte_device is hidden in testpmd.
> > So the detach command is detaching the underlying device of the port,
> > and all its sibling ports of course.
> >
> > What about detach_device_of_port() ?
> 
> Or detach_port_device()?

detach_port_device() looks fine to me.

> 
> > [...]
> > > > -   if (rte_dev_remove(rte_eth_devices[port_id].device) != 0) {
> > > > +   if (rte_dev_remove(dev) != 0) {
> > > > TESTPMD_LOG(ERR, "Failed to detach port %u\n", port_id);
> > >
> > > Should the log message be ( ERR "Failed to detach device %s\n", dev-
> >name) ?
> >
> > Yes!
> >
> > [...]
> > > > -   printf("Port %u is detached. Now total ports is %d\n",
> > > > -   port_id, nb_ports);
> > >
> > > How about printf("Device %s is detached, Now total ports is %d\n"
> > >   dev->name, nb_ports);
> >
> > The issue is that we cannot get the device name after detach.
> > I can reword it differently:
> > Device of port %u is detached, Now total ports is %d

Looks fine to me.

Regards,

Bernard
 



[dpdk-dev] [PATCH] crypto/caam_jr: fix redefinition error

2018-10-23 Thread Gagandeep Singh
dma_addr_t is already defined in compat.h.
so removing the local definition from caam_jr_config.h

Fixes: b70a67ab70 ("crypto/caam_jr: add HW tuning options")

Signed-off-by: Gagandeep Singh 
---
 drivers/crypto/caam_jr/caam_jr_config.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/caam_jr/caam_jr_config.h 
b/drivers/crypto/caam_jr/caam_jr_config.h
index e7855cee6..041187a84 100644
--- a/drivers/crypto/caam_jr/caam_jr_config.h
+++ b/drivers/crypto/caam_jr/caam_jr_config.h
@@ -7,6 +7,8 @@
 
 #include 
 
+#include 
+
 #ifdef RTE_LIBRTE_PMD_CAAM_JR_BE
 #define CAAM_BYTE_ORDER __BIG_ENDIAN
 #else
@@ -19,8 +21,6 @@
 #define CORE_BYTE_ORDER __LITTLE_ENDIAN
 #endif
 
-typedef uint64_t   dma_addr_t;
-
 #if CORE_BYTE_ORDER != CAAM_BYTE_ORDER
 
 #define cpu_to_caam64 rte_cpu_to_be_64
-- 
2.17.1



[dpdk-dev] [PATCH] drivers: fix compilation if security disabled

2018-10-23 Thread Akhil Goyal
RTE_SECURITY is enabled by default. If it is disabled, dpaa2_sec,
dpaa_sec and caam_jr compilation fails.

This patch fixes compilation by disabling these drivers
when rte_security is not available.

Fixes: 1ee9569576f6 ("config: enable dpaaX drivers for generic ARMv8")
Fixes: 09e1e8d256b0 ("mk: fix dependencies of dpaaX drivers")
Fixes: af7c9b5e9ce7 ("crypto/caam_jr: introduce basic driver")
Cc: sta...@dpdk.org

Signed-off-by: Akhil Goyal 
---
 drivers/crypto/Makefile  | 8 +---
 drivers/crypto/dpaa2_sec/Makefile| 7 ---
 drivers/event/dpaa2/Makefile | 5 -
 drivers/event/dpaa2/dpaa2_eventdev.c | 6 ++
 mk/rte.app.mk| 3 +++
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index c25a19fb0..009f8443d 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -6,7 +6,6 @@ include $(RTE_SDK)/mk/rte.vars.mk
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += aesni_gcm
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += aesni_mb
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += armv8
-DIRS-$(CONFIG_RTE_LIBRTE_PMD_CAAM_JR) += caam_jr
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_CCP) += ccp
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_CRYPTO) += octeontx
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_OPENSSL) += openssl
@@ -16,12 +15,15 @@ DIRS-$(CONFIG_RTE_LIBRTE_PMD_KASUMI) += kasumi
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_ZUC) += zuc
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_MVSAM_CRYPTO) += mvsam
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += null
+ifeq ($(CONFIG_RTE_LIBRTE_SECURITY),y)
 ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC) += dpaa2_sec
-endif
+endif # CONFIG_RTE_LIBRTE_FSLMC_BUS
 ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_DPAA_SEC) += dpaa_sec
-endif
+DIRS-$(CONFIG_RTE_LIBRTE_PMD_CAAM_JR) += caam_jr
+endif # CONFIG_RTE_LIBRTE_PMD_DPAA_SEC
+endif # CONFIG_RTE_LIBRTE_SECURITY
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_VIRTIO_CRYPTO) += virtio
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/crypto/dpaa2_sec/Makefile 
b/drivers/crypto/dpaa2_sec/Makefile
index 8b8dfd2ab..f537f76a6 100644
--- a/drivers/crypto/dpaa2_sec/Makefile
+++ b/drivers/crypto/dpaa2_sec/Makefile
@@ -4,13 +4,6 @@
 #
 
 include $(RTE_SDK)/mk/rte.vars.mk
-
-ifneq ($(MAKECMDGOALS),clean)
-ifneq ($(CONFIG_RTE_LIBRTE_SECURITY),y)
-$(error "RTE_LIBRTE_SECURITY is required to build RTE_LIBRTE_PMD_DPAA2_SEC")
-endif
-endif
-
 #
 # library name
 #
diff --git a/drivers/event/dpaa2/Makefile b/drivers/event/dpaa2/Makefile
index 73e8fd8a7..e0134cc46 100644
--- a/drivers/event/dpaa2/Makefile
+++ b/drivers/event/dpaa2/Makefile
@@ -22,10 +22,13 @@ LDLIBS += -lrte_eal -lrte_eventdev
 LDLIBS += -lrte_bus_fslmc -lrte_mempool_dpaa2 -lrte_pmd_dpaa2
 LDLIBS += -lrte_bus_vdev
 LDLIBS += -lrte_common_dpaax
-LDLIBS += -lrte_pmd_dpaa2_sec
 CFLAGS += -I$(RTE_SDK)/drivers/net/dpaa2
 CFLAGS += -I$(RTE_SDK)/drivers/net/dpaa2/mc
+
+ifeq ($(CONFIG_RTE_LIBRTE_SECURITY),y)
+LDLIBS += -lrte_pmd_dpaa2_sec
 CFLAGS += -I$(RTE_SDK)/drivers/crypto/dpaa2_sec
+endif
 
 # versioning export map
 EXPORT_MAP := rte_pmd_dpaa2_event_version.map
diff --git a/drivers/event/dpaa2/dpaa2_eventdev.c 
b/drivers/event/dpaa2/dpaa2_eventdev.c
index a89d1be5c..8d168b028 100644
--- a/drivers/event/dpaa2/dpaa2_eventdev.c
+++ b/drivers/event/dpaa2/dpaa2_eventdev.c
@@ -35,7 +35,9 @@
 #include 
 #include 
 #include 
+#ifdef RTE_LIBRTE_SECURITY
 #include 
+#endif
 #include "dpaa2_eventdev.h"
 #include "dpaa2_eventdev_logs.h"
 #include 
@@ -795,6 +797,7 @@ dpaa2_eventdev_eth_stop(const struct rte_eventdev *dev,
return 0;
 }
 
+#ifdef RTE_LIBRTE_SECURITY
 static int
 dpaa2_eventdev_crypto_caps_get(const struct rte_eventdev *dev,
const struct rte_cryptodev *cdev,
@@ -937,6 +940,7 @@ dpaa2_eventdev_crypto_stop(const struct rte_eventdev *dev,
 
return 0;
 }
+#endif
 
 static struct rte_eventdev_ops dpaa2_eventdev_ops = {
.dev_infos_get= dpaa2_eventdev_info_get,
@@ -959,11 +963,13 @@ static struct rte_eventdev_ops dpaa2_eventdev_ops = {
.eth_rx_adapter_queue_del = dpaa2_eventdev_eth_queue_del,
.eth_rx_adapter_start = dpaa2_eventdev_eth_start,
.eth_rx_adapter_stop = dpaa2_eventdev_eth_stop,
+#ifdef RTE_LIBRTE_SECURITY
.crypto_adapter_caps_get= dpaa2_eventdev_crypto_caps_get,
.crypto_adapter_queue_pair_add  = dpaa2_eventdev_crypto_queue_add,
.crypto_adapter_queue_pair_del  = dpaa2_eventdev_crypto_queue_del,
.crypto_adapter_start   = dpaa2_eventdev_crypto_start,
.crypto_adapter_stop= dpaa2_eventdev_crypto_stop,
+#endif
 };
 
 static int
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 3203cf0ad..c0036daf8 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -234,12 +234,15 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)+= 
-L$(ARMV8_CRYPTO_LIB_PATH) -
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_MVSAM_CRYPTO) += -L$(LIBMUSDK_PATH)/lib

Re: [dpdk-dev] [PATCH v3 2/5] test: add quick run tests under test-fast suite

2018-10-23 Thread Pattan, Reshma
Hi,

> -Original Message-
> From: Vemula, Hari KumarX
> Sent: Tuesday, October 23, 2018 12:01 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce ; Pattan, Reshma
> ; Vemula, Hari KumarX
> 
> Subject: [PATCH v3 2/5] test: add quick run tests under test-fast suite
> 
> From: Hari Kumar Vemula 
> 
> Added test cases that runs quickly under test fast category
> 
> Signed-off-by: Hari Kumar Vemula 
> +
> +foreach arg : fast_parallel_test_names
> +test(arg, dpdk_test,
> +env : ['DPDK_TEST=' + arg],
> +args : ['-c f','-n 4', 
> '--file-prefix=test@0@'.format(arg)],

Small nit, just remove test in file prefix, and how about having this like this 
'--file-prefix=@0@'.format(arg)] ?


[dpdk-dev] [PATCH 1/2] app/testpmd: fix metadata API and Tx insertion

2018-10-23 Thread Dekel Peled
Previous patch introduces the Tx metadata feature, with unnecessary
restrictions on data entry.
It also used the metadata in txonly fwd engine only.

This fix removes the data entry restrictions on metadata item.
It also implements callback function to add the metadata in every
Tx packet, sent by any fwd engine.

Fixes: 32eef22f0b79 ("app/testpmd: support metadata as flow rule item")
Cc: dek...@mellanox.com

Signed-off-by: Dekel Peled 
---
 app/test-pmd/cmdline.c  |  5 +
 app/test-pmd/cmdline_flow.c |  7 +--
 app/test-pmd/testpmd.h  |  7 +++
 app/test-pmd/txonly.c   |  9 
 app/test-pmd/util.c | 51 +
 5 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index fc74b95..27264eb 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -17718,6 +17718,11 @@ struct cmd_config_tx_metadata_specific_result {
if (port_id_is_invalid(res->port_id, ENABLED_WARN))
return;
ports[res->port_id].tx_metadata = rte_cpu_to_be_32(res->value);
+   /* Add/remove callback to insert valid metadata in every Tx packet. */
+   if (ports[res->port_id].tx_metadata)
+   add_tx_md_callback(res->port_id);
+   else
+   remove_tx_md_callback(res->port_id);
 }
 
 cmdline_parse_token_string_t cmd_config_tx_metadata_specific_port =
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 7932b54..dfb7c147 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -548,11 +548,6 @@ struct parse_action_priv {
ZERO,
 };
 
-static const enum index item_param_is[] = {
-   ITEM_PARAM_IS,
-   ZERO,
-};
-
 static const enum index next_item[] = {
ITEM_END,
ITEM_VOID,
@@ -2094,7 +2089,7 @@ static int comp_vc_action_rss_queue(struct context *, 
const struct token *,
[ITEM_META_DATA] = {
.name = "data",
.help = "metadata value",
-   .next = NEXT(item_meta, NEXT_ENTRY(UNSIGNED), item_param_is),
+   .next = NEXT(item_meta, NEXT_ENTRY(UNSIGNED), item_param),
.args = ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_meta,
  data, "\xff\xff\xff\xff")),
},
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index c07548e..102f2f4 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -198,6 +198,7 @@ struct rte_port {
 #endif
/**< metadata value to insert in Tx packets. */
rte_be32_t  tx_metadata;
+   const struct rte_eth_rxtx_callback *tx_set_md_cb[MAX_QUEUE_ID+1];
 };
 
 /**
@@ -781,6 +782,12 @@ uint16_t dump_tx_pkts(uint16_t port_id, uint16_t queue, 
struct rte_mbuf *pkts[],
 void remove_tx_dump_callbacks(portid_t portid);
 void configure_rxtx_dump_callbacks(uint16_t verbose);
 
+uint16_t tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
+  struct rte_mbuf *pkts[], uint16_t nb_pkts,
+  __rte_unused void *user_param);
+void add_tx_md_callback(portid_t portid);
+void remove_tx_md_callback(portid_t portid);
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index fae84ca..1f08b6e 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -253,15 +253,6 @@
pkt->l2_len = sizeof(struct ether_hdr);
pkt->l3_len = sizeof(struct ipv4_hdr);
pkts_burst[nb_pkt] = pkt;
-
-   /*
-* If user configured metadata value add it to packet
-* and set ol_flags accordingly
-*/
-   if (ports[fs->tx_port].tx_metadata) {
-   pkt->tx_metadata = ports[fs->tx_port].tx_metadata;
-   pkt->ol_flags |= PKT_TX_METADATA;
-   }
}
nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
/*
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index f4125df..687bfa4 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -166,3 +166,54 @@
dump_pkt_burst(port_id, queue, pkts, nb_pkts, 0);
return nb_pkts;
 }
+
+uint16_t
+tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
+ struct rte_mbuf *pkts[], uint16_t nb_pkts,
+ __rte_unused void *user_param)
+{
+   uint16_t i = 0;
+
+   /*
+* Add metadata value to every Tx packet,
+* and set ol_flags accordingly.
+*/
+   for (i = 0; i < nb_pkts; i++) {
+   pkts[i]->tx_metadata = ports[port_id].tx_metadata;
+   pkts[i]->ol_flags |= PKT_TX_METADATA;
+   }
+   return nb_pkts;
+}
+
+void
+add_tx_md_callback(portid_t portid)
+{
+   struct rte_eth_dev_info dev_info;
+   uint16_t queue;

[dpdk-dev] [PATCH 2/2] ethdev: fix metadata documentation

2018-10-23 Thread Dekel Peled
Previous patch introduced the Tx metadata feature, with unnecessary
restrictions on data entry.

This fix updates the documentation, removing the data entry
restrictions on metadata item.

Fixes: aa0b9484eb5f ("ethdev: support metadata as flow rule criteria")
Cc: dek...@mellanox.com

Signed-off-by: Dekel Peled 
---
 doc/guides/prog_guide/rte_flow.rst | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst 
b/doc/guides/prog_guide/rte_flow.rst
index 6fb0535..d6683e4 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1196,21 +1196,21 @@ Item: ``META``
 
 Matches an application specific 32 bit metadata item.
 
-- Default ``mask`` matches the specified metadata value.
+- Default ``mask`` matches any 32 bit value.
 
 .. _table_rte_flow_item_meta:
 
 .. table:: META
 
-   +--+--+---+
-   | Field| Subfield | Value |
-   +==+==+===+
-   | ``spec`` | ``data`` | 32 bit metadata value |
-   +--+--+
-   | ``last`` | ``data`` | ignored   |
-   +--+--+---+
-   | ``mask`` | ``data`` | ignored   |
-   +--+--+---+
+   +--+--+---+
+   | Field| Subfield | Value |
+   +==+==+===+
+   | ``spec`` | ``data`` | 32 bit metadata value |
+   +--+--+
+   | ``last`` | ``data`` | upper range value |
+   +--+--+---+
+   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
+   +--+--+---+
 
 Actions
 ~~~
-- 
1.8.3.1



[dpdk-dev] [PATCH v1 1/9] app/procinfo: add usage for new debug

2018-10-23 Thread Vipin Varghese
Update the file with MACRO for stats border, usage text information
and string comparision.

Signed-off-by: Vipin Varghese 
---
 app/proc-info/main.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index c20effa4f..92854f5ba 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -36,6 +36,10 @@
 
 #define MAX_STRING_LEN 256
 
+#define STATS_BDR_FMT ""
+#define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
+   STATS_BDR_FMT, s, w, STATS_BDR_FMT)
+
 /**< mask of enabled ports */
 static uint32_t enabled_port_mask;
 /**< Enable stats. */
@@ -83,7 +87,12 @@ proc_info_usage(const char *prgname)
"  --stats-reset: to reset port statistics\n"
"  --xstats-reset: to reset port extended statistics\n"
"  --collectd-format: to print statistics to STDOUT in expected 
by collectd format\n"
-   "  --host-id STRING: host id used to identify the system 
process is running on\n",
+   "  --host-id STRING: host id used to identify the system 
process is running on\n"
+   "  --dbg-port to display information for all available ports\n"
+   "  --dbg-tm to display information for traffic manager\n"
+   "  --dbg-crypto to display information for crypto\n"
+   "  --dbg-ring[=name] to display information for ring\n"
+   "  --dbg-mempool[=name] to display information for mempool\n",
prgname);
 }
 
@@ -190,6 +199,11 @@ proc_info_parse_args(int argc, char **argv)
{"collectd-format", 0, NULL, 0},
{"xstats-ids", 1, NULL, 1},
{"host-id", 0, NULL, 0},
+   {"dbg-port", 0, NULL, 0},
+   {"dbg-tm", 0, NULL, 0},
+   {"dbg-crypto", 0, NULL, 0},
+   {"dbg-ring", optional_argument, NULL, 0},
+   {"dbg-mempool", optional_argument, NULL, 0},
{NULL, 0, 0, 0}
};
 
-- 
2.17.1



[dpdk-dev] [PATCH v1 2/9] app/procinfo: add compare for new options

2018-10-23 Thread Vipin Varghese
Add code for new debug options to compare usage strings and set
enable flag.

Signed-off-by: Vipin Varghese 
---
 app/proc-info/main.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 92854f5ba..5505d3fe3 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -63,6 +63,18 @@ static uint32_t mem_info;
 /**< Enable displaying xstat name. */
 static uint32_t enable_xstats_name;
 static char *xstats_name;
+/**< Enable port debug. */
+static uint32_t enable_dbg_port;
+/**< Enable tm debug. */
+static uint32_t enable_dbg_tm;
+/**< Enable crypto debug. */
+static uint32_t enable_dbg_crypto;
+/**< Enable ring debug. */
+static uint32_t enable_dbg_ring;
+static char *ring_name;
+/**< Enable mempool debug. */
+static uint32_t enable_dbg_mempool;
+static char *mempool_name;
 
 /**< Enable xstats by ids. */
 #define MAX_NB_XSTATS_IDS 1024
@@ -247,6 +259,24 @@ proc_info_parse_args(int argc, char **argv)
else if (!strncmp(long_option[option_index].name, 
"xstats-reset",
MAX_LONG_OPT_SZ))
reset_xstats = 1;
+   else if (!strncmp(long_option[option_index].name,
+   "dbg-port", MAX_LONG_OPT_SZ))
+   enable_dbg_port = 1;
+   else if (!strncmp(long_option[option_index].name,
+   "dbg-tm", MAX_LONG_OPT_SZ))
+   enable_dbg_tm = 1;
+   else if (!strncmp(long_option[option_index].name,
+   "dbg-crypto", MAX_LONG_OPT_SZ))
+   enable_dbg_crypto = 1;
+   else if (!strncmp(long_option[option_index].name,
+   "dbg-ring", MAX_LONG_OPT_SZ)) {
+   enable_dbg_ring = 1;
+   ring_name = optarg;
+   } else if (!strncmp(long_option[option_index].name,
+   "dbg-mempool", MAX_LONG_OPT_SZ)) {
+   enable_dbg_mempool = 1;
+   mempool_name = optarg;
+   }
break;
case 1:
/* Print xstat single value given by name*/
-- 
2.17.1



[dpdk-dev] [PATCH v1 3/9] app/procinfo: add prototype for debug instances

2018-10-23 Thread Vipin Varghese
Add prototype and function calls for the debug functions.

Signed-off-by: Vipin Varghese 
---
 app/proc-info/main.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 5505d3fe3..5511fcb71 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -628,6 +628,36 @@ metrics_display(int port_id)
rte_free(names);
 }
 
+static void
+debug_port(void)
+{
+   printf(" port");
+}
+
+static void
+debug_tm(void)
+{
+   printf(" tm");
+}
+
+static void
+debug_crypto(void)
+{
+   printf(" crypto");
+}
+
+static void
+debug_ring(char *name)
+{
+   printf(" rings Name (%s)", name);
+}
+
+static void
+debug_mempool(char *name)
+{
+   printf(" mempools Name (%s)", name);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -707,6 +737,17 @@ main(int argc, char **argv)
/* print port independent stats */
if (enable_metrics)
metrics_display(RTE_METRICS_GLOBAL);
+   /* print debug information for PMD */
+   if (enable_dbg_port)
+   debug_port();
+   if (enable_dbg_tm)
+   debug_tm();
+   if (enable_dbg_crypto)
+   debug_crypto();
+   if (enable_dbg_ring)
+   debug_ring(ring_name);
+   if (enable_dbg_mempool)
+   debug_mempool(mempool_name);
 
ret = rte_eal_cleanup();
if (ret)
-- 
2.17.1



[dpdk-dev] [PATCH v1 6/9] app/procinfo: add code for debug crypto

2018-10-23 Thread Vipin Varghese
Function debug_crypto is used for displaying the crypto PMD under
the primary process.

Signed-off-by: Vipin Varghese 
---
 app/proc-info/main.c | 70 +++-
 1 file changed, 69 insertions(+), 1 deletion(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 4766a5ea3..0f2045858 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -999,7 +999,75 @@ x.cman_wred_context_shared_n_max); \
 static void
 debug_crypto(void)
 {
-   printf(" crypto");
+   uint8_t crypto_dev_count = rte_cryptodev_count(), i;
+
+   snprintf(bdr_str, 100, "debug - CRYPTO PMD %"PRIu64, rte_get_tsc_hz());
+   STATS_BDR_STR(10, bdr_str);
+
+   for (i = 0; i < crypto_dev_count; i++) {
+   struct rte_cryptodev_info dev_info = {0};
+   struct rte_cryptodev_stats stats = {0};
+
+   rte_cryptodev_info_get(i, &dev_info);
+
+   printf("  - device (%u)\n", i);
+   printf("\t  -- name (%s) driver (%s)\n"
+  "\t  -- id (%u) flags (0x%"PRIx64") socket (%d)\n"
+  "\t  -- queue pairs (%d)\n",
+  rte_cryptodev_name_get(i),
+  dev_info.driver_name, dev_info.driver_id,
+  dev_info.feature_flags, dev_info.device->numa_node,
+  rte_cryptodev_queue_pair_count(i));
+
+#define DSP_CRYPTO_FLAG(x) do { \
+printf("  - feature flags\n"); \
+printf("\t  -- symmetric (%c) asymmetric (%c)" \
+" symmetric operation chaining (%c)\n", \
+(x & RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING)?'y':'n'); \
+printf("\t  -- CPU SSE (%c) AVX (%c) AVX2 (%c) AVX512 (%c)\n", \
+(x & RTE_CRYPTODEV_FF_CPU_SSE)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_CPU_AVX)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_CPU_AVX2)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_CPU_AVX512)?'y':'n'); \
+printf("\t  -- Acclerate CPU AESNI (%c) HW (%c)\n", \
+(x & RTE_CRYPTODEV_FF_CPU_AESNI)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_HW_ACCELERATED)?'y':'n'); \
+printf("\t  -- INLINE (%c)\n", \
+(x & RTE_CRYPTODEV_FF_SECURITY)?'y':'n'); \
+printf("\t  -- ARM NEON (%c) CE (%c)\n", \
+(x & RTE_CRYPTODEV_FF_CPU_NEON)?'y':'n', \
+(x & RTE_CRYPTODEV_FF_CPU_ARM_CE)?'y':'n'); \
+printf("  - buffer offload\n"); \
+printf("\t  -- IN_PLACE_SGL (%c)\n", \
+(x & RTE_CRYPTODEV_FF_IN_PLACE_SGL)?'y':'n'); \
+printf("\t  -- OOP_SGL_IN_SGL_OUT (%c)\n", \
+(x & RTE_CRYPTODEV_FF_OOP_SGL_IN_SGL_OUT)?'y':'n'); \
+printf("\t  -- OOP_SGL_IN_LB_OUT (%c)\n", \
+(x & RTE_CRYPTODEV_FF_OOP_SGL_IN_LB_OUT)?'y':'n'); \
+printf("\t  -- OOP_LB_IN_SGL_OUT (%c)\n", \
+(x & RTE_CRYPTODEV_FF_OOP_LB_IN_SGL_OUT)?'y':'n'); \
+printf("\t  -- OOP_LB_IN_LB_OUT (%c)\n", \
+(x & RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT)?'y':'n'); \
+} while (0)
+
+   DSP_CRYPTO_FLAG(dev_info.feature_flags);
+
+   printf("  - stats\n");
+   if (rte_cryptodev_stats_get(i, &stats) == 0) {
+   printf("\t  -- enqueue count (%"PRIu64")"
+  " error (%"PRIu64")\n",
+  stats.enqueued_count,
+  stats.enqueue_err_count);
+   printf("\t  -- dequeue count (%"PRIu64")"
+  " error (%"PRIu64")\n",
+  stats.dequeued_count,
+  stats.dequeue_err_count);
+   }
+   }
+
+   STATS_BDR_STR(50, "");
 }
 
 static void
-- 
2.17.1



[dpdk-dev] [PATCH v1 4/9] app/procinfo: add code for debug port

2018-10-23 Thread Vipin Varghese
Function debug_port is used for displaying the port PMD under the
primary process. This covers basic and per queue configuration.

Signed-off-by: Vipin Varghese 
---
 app/proc-info/main.c | 113 ++-
 1 file changed, 112 insertions(+), 1 deletion(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 5511fcb71..668f7febf 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -29,6 +29,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
@@ -40,6 +43,8 @@
 #define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
STATS_BDR_FMT, s, w, STATS_BDR_FMT)
 
+char bdr_str[100];
+
 /**< mask of enabled ports */
 static uint32_t enabled_port_mask;
 /**< Enable stats. */
@@ -631,7 +636,113 @@ metrics_display(int port_id)
 static void
 debug_port(void)
 {
-   printf(" port");
+   uint16_t i = 0;
+   int ret = 0, j, k;
+
+   snprintf(bdr_str, 100, " debug - Port PMD %"PRIu64, rte_get_tsc_hz());
+   STATS_BDR_STR(10, bdr_str);
+
+   RTE_ETH_FOREACH_DEV(i) {
+   uint16_t mtu = 0;
+   struct rte_eth_link link = {0};
+   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_rxq_info qinfo = {0};
+   struct rte_eth_stats stats = {0};
+   struct rte_eth_rss_conf rss_conf = {0};
+
+   snprintf(bdr_str, 100, " Port (%u)", i);
+   STATS_BDR_STR(5, bdr_str);
+   printf("  - generic config\n");
+
+   printf("\t  -- Socket %d\n", rte_eth_dev_socket_id(i));
+   rte_eth_link_get(i, &link);
+   printf("\t  -- link speed %d duplex %d,"
+   " auto neg %d status %d\n",
+   link.link_speed,
+   link.link_duplex,
+   link.link_autoneg,
+   link.link_status);
+   printf("\t  -- promiscuous (%d)\n",
+   rte_eth_promiscuous_get(i));
+   ret = rte_eth_dev_get_mtu(i, &mtu);
+   if (ret == 0)
+   printf("\t  -- mtu (%d)\n", mtu);
+
+   printf("  - queue\n");
+
+   rte_eth_dev_info_get(i, &dev_info);
+
+   for (j = 0; j < dev_info.nb_rx_queues; j++) {
+   ret = rte_eth_rx_queue_info_get(i, j, &qinfo);
+   if (ret == 0) {
+   printf("\t  -- queue %d rx scatter %d"
+  " descriptors %d offloads 0x%"PRIx64
+  " mempool socket %d\n",
+  j,
+  qinfo.scattered_rx,
+  qinfo.nb_desc,
+  qinfo.conf.offloads,
+  qinfo.mp->socket_id);
+
+   ret = rte_eth_stats_get(i, &stats);
+   if (ret == 0) {
+   printf("\t  -- packet input %"PRIu64
+  " output %"PRIu64""
+  " error %"PRIu64"\n",
+  stats.q_ipackets[j],
+  stats.q_opackets[j],
+  stats.q_errors[j]);
+   }
+   }
+
+   ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
+   if ((ret) || (rss_conf.rss_key == NULL))
+   continue;
+
+   printf("\t  -- RSS len %u key (hex):",
+  rss_conf.rss_key_len);
+   for (k = 0; k < rss_conf.rss_key_len; k++)
+   printf(" %x", rss_conf.rss_key[k]);
+   printf("\t  -- hf 0x%"PRIx64"\n",
+  rss_conf.rss_hf);
+   }
+
+   printf("  - drop packet information\n");
+   ret = rte_eth_stats_get(i, &stats);
+   if (ret == 0) {
+   printf("\t  -- input %"PRIu64
+  " output %"PRIu64"\n",
+  stats.ipackets,
+  stats.opackets);
+   printf("\t  -- error input %"PRIu64
+  " output %"PRIu64"\n",
+  stats.ierrors,
+  stats.oerrors);
+   printf("\t  -- RX no mbuf %"PRIu64"\n",
+  stats.rx_nombuf);
+   }
+
+   printf("  - cyrpto context\n");
+   void *ptr_ctx = rte_eth_dev_get_sec_ctx(i);
+   printf("\t  -- security context - %p\n",

  1   2   >