[dpdk-dev] [PATCH 00/22] Generic flow API (rte_flow)

2016-11-21 Thread Nélio Laranjeiro
Hi,

I found some small issues:

 - In rte_flow_error_set(), *cause argument should be const to avoid a
   compilation error when we implement it.

 - In port_flow_create(), the flow is not stored in the pf structure
   which ends by providing a null pointer to port_flow_destroy()
   function.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 2/2] doc: add mlx5 release notes

2016-11-03 Thread Nélio Laranjeiro
On Thu, Nov 03, 2016 at 12:25:47PM +, Mcnamara, John wrote:
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Nelio Laranjeiro
> > Sent: Wednesday, November 2, 2016 1:47 PM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH 2/2] doc: add mlx5 release notes
> > 
> > ...
> > 
> > -   This section is a comment. Make sure to start the actual text at the 
> > margin.
> > +#. Mellanox(R) ConnectX(R)-4 25G MCX4111A-ACAT (1x25G)
> 
> Release note patches shouldn't remove or overwrite the comments sections.

Ok, I will re-send a v2.

Thanks.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] Red Hat 6.5 compilation failure

2016-10-31 Thread Nélio Laranjeiro
Hi all,

Am facing an issue with compilation on redhat 6.5 of DPDK v16.11-rc2,
compilation fails with:

  cc1: warnings being treated as errors
  /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c: In function 
?i40evf_dev_interrupt_handler?:
  /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1391: error: dereferencing 
pointer ?v_msg? does break strict-aliasing rules
  /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1397: error: dereferencing 
pointer ?v_msg? does break strict-aliasing rules
  /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1398: error: dereferencing 
pointer ?v_msg? does break strict-aliasing rules
  /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1376: note: initialized from here


  cc1: warnings being treated as errors
  /root/dpdk/drivers/net/bnxt/bnxt_hwrm.c: In function 
?bnxt_hwrm_func_driver_unregister?:
  /root/dpdk/drivers/net/bnxt/bnxt_hwrm.c:105: error: dereferencing pointer 
?req? does break strict-aliasing rules
  /root/dpdk/drivers/net/bnxt/bnxt_hwrm.c:66: note: initialized from here


  cc1: warnings being treated as errors
  /root/dpdk/drivers/net/qede/base/ecore_mcp.c: In function 
?ecore_mcp_nvm_rd_cmd?:
  /root/dpdk/build/include/rte_memcpy.h:740: error: array subscript is above 
array bounds
  /root/dpdk/drivers/net/qede/base/ecore_mcp.c: In function 
?ecore_mcp_nvm_wr_cmd?:
  /root/dpdk/build/include/rte_memcpy.h:740: error: array subscript is above 
array bounds
  At top level:
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
  cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"

In doc/guides/rel_notes/supported_os.rst we still support
"Red Hat Enterprise Linux 6.5".

Do we still want to support it, or should we update the documentation to
remove it?

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 0/3] Improvements in packet timestamps support

2016-10-17 Thread Nélio Laranjeiro
On Thu, Oct 13, 2016 at 02:35:05PM +, Oleg Kuporosov wrote:
> 
> Hello DPDK Developers,
> 
> Financial Services Industry which is pretty eager for several DPDK
> features especially low latency while high throughput. The major issue
> so far for increasing DPDK adoption there is requirement for several
> applications (capturers, some trading algorithms) to support of accurate
> timestamping. The requirement mostly came from regulatory and customers
> need strictly follow it.
> 
> Current state of timestamping support in DPDK looks pretty good:
>  - there is API to enable/disable timestamping acquisition by 
>rte_eth_timesync_enable/disable
>  - get timestamps itself by timesync_read_rx/tx_timestamp
>  - and even implementation of NTP IEEE 1588 for time synchronization
>by rte_eth_timesync_adjust_read/write_time APIs.
>
> But it misses the most important feature there ? embedding timestamp
> in rte_mbuf aligning it with packet.
> 
> We would like to change this to increase DPDK adoption for several new
> DPDK-based applications for FSI segment. It also might be very
> applicable for several RT media and debugging purposes of network
> flows/streams in other segments like HPC.
> 
> There are several thoughts how to improve there:
>  - include uint64_t timestamp field into rte_mbuf with minimal impact
>to throughput/latency. Keep it just simple uint64_t in ns (more than
>580 years) would be enough for immediate needs while using full
>struct timespec with twice bigger size would have much stronger
>performance impact as missed cacheline0. It is possible as there is
>6-bytes gap in 1st cacheline (fast path) and moving uint16_t
>vlan_tci_outer field to 2nd cacheline. 
>  - such move will only impact for pretty rare usable VLAN RX stripping
>mode for outer TCI (it used only for one NIC i40e from the whole
>set and keep minimal performance impact for timestamps.  
>  - PMD can fill the field in their callback completion routines
>depending on enabling this feature in runtime.
> 
> We evaluated other possible options but looks it will have even worse
> performance impact.
> 
> 
> Oleg Kuporosov (3):
>   mbuf: embedding timestamp into the packet
>   app/testpmd: enabled control for packet timestamps
>   net/mlx5: implementation of Rx packet timestamping support
> 
>  app/test-pmd/cmdline.c  | 122 +++
>  app/test-pmd/parameters.c   |   4 +
>  app/test-pmd/testpmd.c  |   5 +
>  app/test-pmd/testpmd.h  |   1 +
>  doc/guides/testpmd_app_ug/run_app.rst   |   4 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   7 +
>  drivers/net/mlx5/mlx5.c |   7 +-
>  drivers/net/mlx5/mlx5.h |  10 +-
>  drivers/net/mlx5/mlx5_defs.h|   4 +
>  drivers/net/mlx5/mlx5_ethdev.c  | 222 
> +++-
>  drivers/net/mlx5/mlx5_rxq.c |   2 +
>  drivers/net/mlx5/mlx5_rxtx.c|  19 ++-
>  drivers/net/mlx5/mlx5_rxtx.h|   7 +-
>  drivers/net/mlx5/mlx5_time.h|  53 +++
>  drivers/net/mlx5/mlx5_trigger.c |   1 +
>  lib/librte_eal/common/include/rte_time.h|  45 ++
>  lib/librte_mbuf/rte_mbuf.h  |   6 +-
>  17 files changed, 507 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/net/mlx5/mlx5_time.h
> 
> Thanks,
> Oleg.
> -- 
> 1.8.3.1

Reviewed-by: Nelio Laranjeiro 

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH V2 0/8] net/mlx5: various fixes

2016-09-14 Thread Nélio Laranjeiro
On Wed, Sep 14, 2016 at 01:53:47PM +0200, Nelio Laranjeiro wrote:
>  - Flow director
>  - Rx Capabilities
>  - Inline
> 
> Changes in V2:
> 
>  - Fix a compilation error.
> 
> Adrien Mazarguil (1):
>   net/mlx5: fix Rx VLAN offload capability report
> 
> Nelio Laranjeiro (3):
>   net/mlx5: force inline for completion function
>   net/mlx5: re-factorize functions
>   net/mlx5: fix inline logic
> 
> Raslan Darawsheh (1):
>   net/mlx5: fix removing VLAN filter
> 
> Yaacov Hazan (3):
>   net/mlx5: fix inconsistent return value in Flow Director
>   net/mlx5: refactor allocation of flow director queues
>   net/mlx5: fix support for flow director drop mode
> 
>  doc/guides/nics/mlx5.rst   |   3 +-
>  drivers/net/mlx5/mlx5.h|   2 +
>  drivers/net/mlx5/mlx5_ethdev.c |   7 +-
>  drivers/net/mlx5/mlx5_fdir.c   | 270 +++---
>  drivers/net/mlx5/mlx5_rxq.c|   2 +
>  drivers/net/mlx5/mlx5_rxtx.c   | 497 
> +
>  drivers/net/mlx5/mlx5_rxtx.h   |   7 +-
>  drivers/net/mlx5/mlx5_txq.c|   9 +-
>  drivers/net/mlx5/mlx5_vlan.c   |   3 +-
>  9 files changed, 317 insertions(+), 483 deletions(-)
> 
> -- 
> 2.1.4
> 

Sorry Ferruh, I forgot to add you in this serie.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 8/8] net/mlx5: fix inline logic

2016-09-14 Thread Nélio Laranjeiro
On Wed, Sep 14, 2016 at 11:43:35AM +0100, Ferruh Yigit wrote:
> Hi Nelio,
> 
> On 9/7/2016 8:02 AM, Nelio Laranjeiro wrote:
> > To improve performance the NIC expects for large packets to have a pointer
> > to a cache aligned address, old inline code could break this assumption
> > which hurts performance.
> > 
> > Fixes: 2a66cf378954 ("net/mlx5: support inline send")
> > 
> > Signed-off-by: Nelio Laranjeiro 
> > Signed-off-by: Vasily Philipov 
> > ---
> 
> ...
> 
> > @@ -607,8 +523,13 @@ mlx5_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, 
> > uint16_t pkts_n)
> > MLX5_ETH_WQE_L3_CSUM |
> > MLX5_ETH_WQE_L4_CSUM;
> > } else {
> > -   wqe->wqe.eseg.cs_flags = 0;
> > +   wqe->eseg.cs_flags = 0;
> 
> This cause a compilation error, and looks like a typo:
> 
> .../drivers/net/mlx5/mlx5_rxtx.c: In function ?mlx5_tx_burst?:
> .../drivers/net/mlx5/mlx5_rxtx.c:526:7: error: ?volatile union mlx5_wqe?
> has no member named ?eseg?
> wqe->eseg.cs_flags = 0;
>^~

Hi Ferruh,

You are totally right.  I will fix it, re-test and send a V2 today.

Thanks,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v2] doc: update guide and release notes for mlx5

2016-07-27 Thread Nélio Laranjeiro
On Wed, Jul 27, 2016 at 12:27:26PM +0300, Olga Shern wrote:
> Signed-off-by: Olga Shern 
> ---
>  doc/guides/nics/mlx5.rst   |7 ++-
>  doc/guides/rel_notes/release_16_07.rst |   14 ++
>  2 files changed, 20 insertions(+), 1 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index 063c4a5..5c10cd3 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -154,6 +154,11 @@ Run-time configuration
>allows to save PCI bandwidth and improve performance at the cost of a
>slightly higher CPU usage.  Enabled by default.
>  
> +  Supported on:
> +
> +  - x86_64 with ConnectX4 and ConnectX4 LX
> +  - Power8 with ConnectX4 LX
> +
>  - ``txq_inline`` parameter [int]
>  
>Amount of data to be inlined during TX operations. Improves latency.
> @@ -234,7 +239,7 @@ DPDK and must be installed separately:
>  
>  Currently supported by DPDK:
>  
> -- Mellanox OFED **3.3-1.0.0.0**.
> +- Mellanox OFED **3.3-1.0.0.0** and **3.3-2.0.0.0**.
>  
>  - Minimum firmware version:
>  
> diff --git a/doc/guides/rel_notes/release_16_07.rst 
> b/doc/guides/rel_notes/release_16_07.rst
> index d00a6ed..4b43f8d 100644
> --- a/doc/guides/rel_notes/release_16_07.rst
> +++ b/doc/guides/rel_notes/release_16_07.rst
> @@ -191,6 +191,20 @@ New Features
>  
>Live migration of a VM with Virtio and VF PMD's using the bonding PMD.
>  
> +* **Updated the mlx5 driver.**
> +
> +  The mlx5 driver was updated with changes including the following:
> +
> +  * Data path was refactored to bypass Verbs to improve RX and TX 
> performance.
> +  * Removed compilation parameters for inline send, ``MLX5_MAX_INLINE``, and
> +added command line parameter instead, ``txq_inline``.
> +  * Improved TX scatter gather support:
> +Removed compilation parameter ``MLX5_PMD_SGE_WR_N``.
> +Scatter-gather elements is set to the maximum value the NIC supports.
> +Removed linearization logic, this decreases the memory consumption of 
> the PMD.
> +  * Improved jumbo frames support, by dynamically setting RX scatter gather 
> elements
> +according to the MTU and mbuf size,
> +no need for compilation parameter ``MLX5_PMD_SGE_WR_N``
>  
>  Resolved Issues
>  ---
> -- 
> 1.7.8.2

Acked-by: Nelio Laranjeiro 

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] Building connectx-3 fails on Fedora 22 and Fedora 24 with latest git tree

2016-07-18 Thread Nélio Laranjeiro
Hi Kevin,

On Sat, Jul 16, 2016 at 07:38:50PM +0300, Kevin Wilson wrote:
> Hello,
> 
> I had tried to build the connectx-3 driver  with  latest git tree and
> it fails with
> ...
> ...
> == Build drivers/net/mlx4
>   CC mlx4.o
> /work/down/temp/dpdk/drivers/net/mlx4/mlx4.c: In function ?txq_cleanup?:
> /work/down/temp/dpdk/drivers/net/mlx4/mlx4.c:1106:37: error: storage
> size of ?params? isn?t known
>   struct ibv_exp_release_intf_params params;
>  ^
> /work/down/temp/dpdk/drivers/net/mlx4/mlx4.c:1116:4: error: unknown
> field ?comp_mask? specified in initializer
> .comp_mask = 0,
> ^
> /work/down/temp/dpdk/drivers/net/mlx4/mlx4.c:1116:17: error: excess
> elements in struct initializer [-Werror]
> .comp_mask = 0,
>  ^
> /work/down/temp/dpdk/drivers/net/mlx4/mlx4.c:1116:17: note: (near
> initialization for ?(anonymous)?)
> /work/down/temp/dpdk/drivers/net/mlx4/mlx4.c:1117:3: error: invalid
> use of undefined type ?struct ibv_exp_release_intf_params?
>};
> 
> 
> The steps that I took is (after clonning the dpdk tree) are:
> make config T=x86_64-native-linuxapp-gcc
> sed -ri 's,(PMD_PCAP=).*,\1y,' build/.config
> Then make
> Then edited build/.config and set:
> CONFIG_RTE_LIBRTE_MLX4_PMD=y
> and then make.
> 
> Any ideas?
> 
> regards,
> Kevin

It looks like you did not installed Mellanox OFED on your system.
Did you followed the documentation[1] for this NIC?

[1] http://dpdk.org/doc/guides/nics/mlx4.html

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 3/3] scripts: check headline of drivers commits

2016-07-06 Thread Nélio Laranjeiro
Thomas,

On Wed, Jul 06, 2016 at 12:38:50PM +0200, Thomas Monjalon wrote:
> From: Nelio Laranjeiro 
> 
> A driver patch under net should start with "net/" or if
> a patch touch multiple drivers, it should only start with "net:".
> The same apply for crypto.
> A patch touching all drivers (net + crypto) should start with "drivers:".
> 
> Longer prefixes like "net/mlx:" (for mlx4/mlx5) or "net/e1000/base:" are
> handled by not checking the colon.
> 
> Signed-off-by: Nelio Laranjeiro 
> Signed-off-by: Thomas Monjalon 
> ---
>  scripts/check-git-log.sh | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
> index 833aa39..3f3cf19 100755
> --- a/scripts/check-git-log.sh
> +++ b/scripts/check-git-log.sh
> @@ -70,6 +70,23 @@ bad=$(echo "$headlines" | grep --color=always \
>   | sed 's,^,\t,')
>  [ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
>  
> +# check headline prefix when touching only drivers/, e.g. net/
> +bad=$(for commit in $commits ; do
> + headline=$(git log --format='%s' -1 $commit)
> + files=$(git diff-tree --no-commit-id --name-only -r $commit)
> + [ -z "$(echo "$files" | grep -v '^drivers/')" ] || continue
> + driversgrp=$(echo "$files" | cut -d "/" -f 2 | sort -u)
> + drivers=$(echo "$files" | cut -d "/" -f 2,3 | sort -u)
> + if [ $(echo "$driversgrp" | wc -l) -gt 1 ] ; then
> + bad=$(echo "$headline" | grep -v '^drivers:')

I think you forgot the '$' for drivers in the line just above.

> + elif [ $(echo "$drivers" | wc -l) -gt 1 ] ; then
> + bad=$(echo "$headline" | grep -v "^$driversgrp")
> + else
> + bad=$(echo "$headline" | grep -v "^$drivers")
> + fi
> +done | sed 's,^,\t,')
> +[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
> +
>  # check headline label for common typos
>  bad=$(echo "$headlines" | grep --color=always \
>   -e '^example[:/]' \
> -- 
> 2.7.0

Otherwise it seems good.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH] app/testpmd: limit number of forwarding cores

2016-06-28 Thread Nélio Laranjeiro
On Mon, Jun 27, 2016 at 11:35:19PM +0100, Pablo de Lara wrote:
> Number of forwarding cores must be equal or less than
> number of forwarding streams, otherwise two cores
> would try to use a same queue on a port, which is not allowed.
> 
> Fixes: f2bb7ae1d204 ("app/testpmd: handle all Rx queues in RSS setup")
> 
> Signed-off-by: Pablo de Lara 
> ---
>  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 9ccabf9..de60284 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1236,6 +1236,10 @@ rss_fwd_config_setup(void)
>   cur_fwd_config.nb_fwd_streams =
>   (streamid_t) (nb_q * cur_fwd_config.nb_fwd_ports);
>  
> + if (cur_fwd_config.nb_fwd_streams < cur_fwd_config.nb_fwd_lcores)
> + cur_fwd_config.nb_fwd_lcores =
> + (lcoreid_t)cur_fwd_config.nb_fwd_streams;
> +
>   /* reinitialize forwarding streams */
>   init_fwd_streams();
>  
> -- 
> 2.5.5

Tested with:
 - coremask allowing to use 12 cores for 4 txqs and 4 rxqs.
 - coremask allowing to use 12 cores for 6 txqs and 6 rxqs.
 - coremask allowing to use 12 cores for 8 txqs and 8 rxqs.

It fixes the issue.

Tested-by: Nelio Laranjeiro  

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v3 4/5] testpmd: handle all rxqs in rss setup

2016-06-28 Thread Nélio Laranjeiro
Hi Pablo,

On Mon, Jun 27, 2016 at 10:36:38PM +, De Lara Guarch, Pablo wrote:
> Hi Nelio,
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of N?lio Laranjeiro
> > Sent: Monday, June 27, 2016 3:24 PM
> > To: Wang, Zhihong
> > Cc: dev at dpdk.org; Ananyev, Konstantin; Richardson, Bruce; De Lara Guarch,
> > Pablo; thomas.monjalon at 6wind.com
> > Subject: Re: [dpdk-dev] [PATCH v3 4/5] testpmd: handle all rxqs in rss setup
> > 
> > On Tue, Jun 14, 2016 at 07:08:05PM -0400, Zhihong Wang wrote:
> > > This patch removes constraints in rxq handling when multiqueue is enabled
> > > to handle all the rxqs.
> > >
> > > Current testpmd forces a dedicated core for each rxq, some rxqs may be
> > > ignored when core number is less than rxq number, and that causes
> > confusion
> > > and inconvenience.
> > >
> > > One example: One Red Hat engineer was doing multiqueue test, there're 2
> > > ports in guest each with 4 queues, and testpmd was used as the forwarding
> > > engine in guest, as usual he used 1 core for forwarding, as a results he
> > > only saw traffic from port 0 queue 0 to port 1 queue 0, then a lot of
> > > emails and quite some time are spent to root cause it, and of course it's
> > > caused by this unreasonable testpmd behavior.
> > >
> > > Moreover, even if we understand this behavior, if we want to test the
> > > above case, we still need 8 cores for a single guest to poll all the
> > > rxqs, obviously this is too expensive.
> > >
> > > We met quite a lot cases like this, one recent example:
> > > http://openvswitch.org/pipermail/dev/2016-June/072110.html
> > >
> > >
> > > Signed-off-by: Zhihong Wang 
> > > ---
> > >  app/test-pmd/config.c | 8 +---
> > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > >
> > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > > index ede7c78..4719a08 100644
> > > --- a/app/test-pmd/config.c
> > > +++ b/app/test-pmd/config.c
> > > @@ -1199,19 +1199,13 @@ rss_fwd_config_setup(void)
> > >   cur_fwd_config.nb_fwd_ports = nb_fwd_ports;
> > >   cur_fwd_config.nb_fwd_streams =
> > >   (streamid_t) (nb_q * cur_fwd_config.nb_fwd_ports);
> > > - if (cur_fwd_config.nb_fwd_streams > cur_fwd_config.nb_fwd_lcores)
> > > - cur_fwd_config.nb_fwd_streams =
> > > - (streamid_t)cur_fwd_config.nb_fwd_lcores;
> > > - else
> > > - cur_fwd_config.nb_fwd_lcores =
> > > - (lcoreid_t)cur_fwd_config.nb_fwd_streams;
> > >
> > >   /* reinitialize forwarding streams */
> > >   init_fwd_streams();
> > >
> > >   setup_fwd_config_of_each_lcore(_fwd_config);
> > >   rxp = 0; rxq = 0;
> > > - for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++) {
> > > + for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_streams; lc_id++) {
> > >   struct fwd_stream *fs;
> > >
> > >   fs = fwd_streams[lc_id];
> > > --
> > > 2.5.0
> > 
> > Hi Zhihong,
> > 
> > It seems this commits introduce a bug in pkt_burst_transmit(), this only
> > occurs when the number of cores present in the coremask is greater than
> > the number of queues i.e. coremask=0xffe --txq=4 --rxq=4.
> > 
> >   Port 0 Link Up - speed 4 Mbps - full-duplex
> >   Port 1 Link Up - speed 4 Mbps - full-duplex
> >   Done
> >   testpmd> start tx_first
> > io packet forwarding - CRC stripping disabled - packets/burst=64
> > nb forwarding cores=10 - nb forwarding ports=2
> > RX queues=4 - RX desc=256 - RX free threshold=0
> > RX threshold registers: pthresh=0 hthresh=0 wthresh=0
> > TX queues=4 - TX desc=256 - TX free threshold=0
> > TX threshold registers: pthresh=0 hthresh=0 wthresh=0
> > TX RS bit threshold=0 - TXQ flags=0x0
> >   Segmentation fault (core dumped)
> > 
> > 
> > If I start testpmd with a coremask with at most as many cores as queues,
> > everything works well (i.e. coremask=0xff0, or 0xf00).
> > 
> > Are you able to reproduce the same issue?
> > Note: It only occurs on dpdk/master branch (commit f2bb7ae1d204).
> 
> Thanks for reporting this. I was able to reproduce this issue and
> sent a patch that should fix it. Could you verify it?
> http://dpdk.org/dev/patchwork/patch/14430/


I have tested it, it works, I will add a test report on the
corresponding email.

Thanks
> 
> 
> Thanks
> Pablo
> > 
> > Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v3 4/5] testpmd: handle all rxqs in rss setup

2016-06-27 Thread Nélio Laranjeiro
On Tue, Jun 14, 2016 at 07:08:05PM -0400, Zhihong Wang wrote:
> This patch removes constraints in rxq handling when multiqueue is enabled
> to handle all the rxqs.
> 
> Current testpmd forces a dedicated core for each rxq, some rxqs may be
> ignored when core number is less than rxq number, and that causes confusion
> and inconvenience.
> 
> One example: One Red Hat engineer was doing multiqueue test, there're 2
> ports in guest each with 4 queues, and testpmd was used as the forwarding
> engine in guest, as usual he used 1 core for forwarding, as a results he
> only saw traffic from port 0 queue 0 to port 1 queue 0, then a lot of
> emails and quite some time are spent to root cause it, and of course it's
> caused by this unreasonable testpmd behavior.  
> 
> Moreover, even if we understand this behavior, if we want to test the
> above case, we still need 8 cores for a single guest to poll all the
> rxqs, obviously this is too expensive.
> 
> We met quite a lot cases like this, one recent example:
> http://openvswitch.org/pipermail/dev/2016-June/072110.html
> 
> 
> Signed-off-by: Zhihong Wang 
> ---
>  app/test-pmd/config.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index ede7c78..4719a08 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1199,19 +1199,13 @@ rss_fwd_config_setup(void)
>   cur_fwd_config.nb_fwd_ports = nb_fwd_ports;
>   cur_fwd_config.nb_fwd_streams =
>   (streamid_t) (nb_q * cur_fwd_config.nb_fwd_ports);
> - if (cur_fwd_config.nb_fwd_streams > cur_fwd_config.nb_fwd_lcores)
> - cur_fwd_config.nb_fwd_streams =
> - (streamid_t)cur_fwd_config.nb_fwd_lcores;
> - else
> - cur_fwd_config.nb_fwd_lcores =
> - (lcoreid_t)cur_fwd_config.nb_fwd_streams;
>  
>   /* reinitialize forwarding streams */
>   init_fwd_streams();
>  
>   setup_fwd_config_of_each_lcore(_fwd_config);
>   rxp = 0; rxq = 0;
> - for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++) {
> + for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_streams; lc_id++) {
>   struct fwd_stream *fs;
>  
>   fs = fwd_streams[lc_id];
> -- 
> 2.5.0

Hi Zhihong,

It seems this commits introduce a bug in pkt_burst_transmit(), this only
occurs when the number of cores present in the coremask is greater than
the number of queues i.e. coremask=0xffe --txq=4 --rxq=4.

  Port 0 Link Up - speed 4 Mbps - full-duplex
  Port 1 Link Up - speed 4 Mbps - full-duplex
  Done
  testpmd> start tx_first
io packet forwarding - CRC stripping disabled - packets/burst=64
nb forwarding cores=10 - nb forwarding ports=2
RX queues=4 - RX desc=256 - RX free threshold=0
RX threshold registers: pthresh=0 hthresh=0 wthresh=0
TX queues=4 - TX desc=256 - TX free threshold=0
TX threshold registers: pthresh=0 hthresh=0 wthresh=0
TX RS bit threshold=0 - TXQ flags=0x0
  Segmentation fault (core dumped)


If I start testpmd with a coremask with at most as many cores as queues,
everything works well (i.e. coremask=0xff0, or 0xf00).

Are you able to reproduce the same issue?
Note: It only occurs on dpdk/master branch (commit f2bb7ae1d204).

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v7 17/25] mlx5: add support for inline send

2016-06-27 Thread Nélio Laranjeiro
On Mon, Jun 27, 2016 at 01:17:42PM +0100, Bruce Richardson wrote:
> On Fri, Jun 24, 2016 at 03:17:56PM +0200, Nelio Laranjeiro wrote:
> > From: Yaacov Hazan 
> > 
> > Implement send inline feature which copies packet data directly into WQEs
> > for improved latency. The maximum packet size and the minimum number of Tx
> > queues to qualify for inline send are user-configurable.
> > 
> 
> WQE?

WQE is a Work Queue Entry.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v6 00/25] Refactor mlx5 to improve performance

2016-06-24 Thread Nélio Laranjeiro
Sorry about this, it will need a v7, a bug was introduced in the
previous version to fix a checkpatch warning.  kvargs were no more
interpreted.

I will resend a v7 after verifying everything.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v5 00/25] Refactor mlx5 to improve performance

2016-06-23 Thread Nélio Laranjeiro
Hi,

I got an connection failure to my stmp server in the middle.

If you want a v6 tell me.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] librte_meter compilation fails on IBM Power8

2016-06-22 Thread Nélio Laranjeiro
Hi Cristian, Chao,

I have encountered a compilation failure on IBM Power8 when compiling
master branch with EXTRA_CFLAGS='-O0 -g':

  /root/nl/dpdk.org/build/lib/librte_meter.a(rte_meter.o): In function 
`rte_meter_get_tb_params':
  /root/nl/dpdk.org/lib/librte_meter/rte_meter.c:57: undefined reference to 
`ceil'

Seems related to commit 43f4364d.

I don't have the time to search more deeply, I hope it can help.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v3 11/25] mlx5: add support for configuration through kvargs

2016-06-22 Thread Nélio Laranjeiro
On Tue, Jun 21, 2016 at 05:42:42PM +0100, Ferruh Yigit wrote:
> On 6/21/2016 8:23 AM, Nelio Laranjeiro wrote:
> > The intent is to replace the remaining compile-time options and environment
> > variables with a common mean of runtime configuration. This commit only
> > adds the kvargs handling code, subsequent commits will update the rest.
> > 
> > Signed-off-by: Nelio Laranjeiro 
> > Signed-off-by: Adrien Mazarguil 
> > ---
> 
> ...
> 
> > +static int
> > +mlx5_args_check(const char *key, const char *val, void *opaque)
> > +{
> > +   struct priv *priv = opaque;
> > +
> > +   /* No parameters are expected at the moment. */
> > +   (void)priv;
> > +   (void)val;
> > +   WARN("%s: unknown parameter", key);
> > +   return EINVAL;
> Returning positive value here will prevent rte_kvargs_process() to fail,
> I guess that is the intention but returning EINVAL is misleading.
> 
> Also generating the checkpatch warning:
> WARNING:USE_NEGATIVE_ERRNO: return of an errno should typically be
> negative (ie: return -EINVAL)
> #71: FILE: drivers/net/mlx5/mlx5.c:264:
> +   return EINVAL;
> 

Good catch, in fact as it is not processed by the PMD itself, it must be
compliant with the rte_kvargs_process().

I will fix in in the v4.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v3 00/25] Refactor mlx5 to improve performance

2016-06-21 Thread Nélio Laranjeiro
On Tue, Jun 21, 2016 at 04:05:44PM +0800, Yuanhan Liu wrote:
> On Tue, Jun 21, 2016 at 10:00:34AM +0200, N?lio Laranjeiro wrote:
> > On Tue, Jun 21, 2016 at 03:43:08PM +0800, Yuanhan Liu wrote:
> > > Hi,
> > > 
> > > Here is an off-topic comment: would you please add following line to
> > > the sendemail section of your git config file?
> > > 
> > > chainreplyto = false
> > > 
> > > That would let me to break the long threads in my client much easier.
> > > Otherwise, it's hard for me to do it, leading that your thread occupies
> > > several screens on my side.
> > > 
> > > It seems that Tetsuya also has the issue, thus CC'ed.
> > > 
> > > Thanks.
> > > 
> > >   --yliu
> > 
> > I already have it in my sendemail section (copied from
> > http://dpdk.org/dev web page).
> > 
> > I was wondering it did not split correctly the patchset threads.
> 
> No idea, and here is a blind guess: maybe you have a local git config
> file that overwrites the globle options?

No, my local git/config does not have any sendemail section.  It worked
once, maybe an update of the package on my machine broke the script.

> > I will try to use the command line "--no-chain-reply-to" option next
> > time.
> 
> Thanks!
> 
>   --yliu

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v3 00/25] Refactor mlx5 to improve performance

2016-06-21 Thread Nélio Laranjeiro
On Tue, Jun 21, 2016 at 03:43:08PM +0800, Yuanhan Liu wrote:
> Hi,
> 
> Here is an off-topic comment: would you please add following line to
> the sendemail section of your git config file?
> 
> chainreplyto = false
> 
> That would let me to break the long threads in my client much easier.
> Otherwise, it's hard for me to do it, leading that your thread occupies
> several screens on my side.
> 
> It seems that Tetsuya also has the issue, thus CC'ed.
> 
> Thanks.
> 
>   --yliu

I already have it in my sendemail section (copied from
http://dpdk.org/dev web page).

I was wondering it did not split correctly the patchset threads.

I will try to use the command line "--no-chain-reply-to" option next
time.

Thanks.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v2 00/25] Refactor mlx5 to improve performance

2016-06-21 Thread Nélio Laranjeiro
Hi Ferruh,

On Mon, Jun 20, 2016 at 06:01:49PM +0100, Ferruh Yigit wrote:
> Hi Nelio,
> 
> On 6/20/2016 5:10 PM, Nelio Laranjeiro wrote:
> > Enhance mlx5 with a data path that bypasses Verbs.
> > 
> > The first half of this patchset removes support for functionality completely
> > rewritten in the second half (scatter/gather, inline send), while the data
> > path is refactored without Verbs.
> > 
> > The PMD remains usable during the transition.
> > 
> > This patchset must be applied after "Miscellaneous fixes for mlx4 and mlx5".
> > 
> > Changes in v2:
> > - Rebased patchset on top of dpdk/master.
> 
> This is driver patch, and should be rebased on top of
> dpdk-next-net/rel_16_07.

I just applied it on this branch, in fact some on the patch fails to
apply.

> I tried to apply to a few branches but all failed, am I missing something?

No I missed something, I did not notice it should be rebased on top of
rel_16_07.

> The error log for applying to dpdk-next-net/rel_16_07:
> 
> Applying patch #14086 using 'git am'
> Description: [dpdk-dev,v2,01/25] drivers: fix PCI class id support
> Applying: drivers: fix PCI class id support
> 
> Applying patch #14087 using 'git am'
> Description: [dpdk-dev,v2,02/25] mlx5: split memory registration function
> Applying: mlx5: split memory registration function
> 
> Applying patch #14088 using 'git am'
> Description: [dpdk-dev,v2,03/25] mlx5: remove Tx gather support
> Applying: mlx5: remove Tx gather support
> 
> Applying patch #14089 using 'git am'
> Description: [dpdk-dev,v2,04/25] mlx5: remove Rx scatter support
> Applying: mlx5: remove Rx scatter support
> error: patch failed: drivers/net/mlx5/mlx5_rxtx.c:502
> error: drivers/net/mlx5/mlx5_rxtx.c: patch does not apply
> Patch failed at 0001 mlx5: remove Rx scatter support
> The copy of the patch that failed is found in:
>/tmp/dpdk-b/.git/rebase-apply/patch


I prepare a v3 on top on this branch.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 00/24] Refactor mlx5 to improve performance

2016-06-20 Thread Nélio Laranjeiro
On Mon, Jun 20, 2016 at 04:03:00PM +0100, Ferruh Yigit wrote:
> On 6/20/2016 8:38 AM, N?lio Laranjeiro wrote:
> > On Fri, Jun 17, 2016 at 05:09:43PM +0100, Ferruh Yigit wrote:
> >> On 6/8/2016 10:47 AM, Nelio Laranjeiro wrote:
> >>> Enhance mlx5 with a data path that bypasses Verbs.
> >>>
> >>> The first half of this patchset removes support for functionality 
> >>> completely
> >>> rewritten in the second half (scatter/gather, inline send), while the data
> >>> path is refactored without Verbs.
> >>>
> >>> The PMD remains usable during the transition.
> >>>
> >>> This patchset must be applied after "Miscellaneous fixes for mlx4 and 
> >>> mlx5".
> >>>
> >>> Adrien Mazarguil (8):
> >>>   mlx5: replace countdown with threshold for TX completions
> >>>   mlx5: add debugging information about TX queues capabilities
> >>>   mlx5: check remaining space while processing TX burst
> >>>   mlx5: resurrect TX gather support
> >>>   mlx5: work around spurious compilation errors
> >>>   mlx5: remove redundant RX queue initialization code
> >>>   mlx5: make RX queue reinitialization safer
> >>>   mlx5: resurrect RX scatter support
> >>>
> >>> Nelio Laranjeiro (15):
> >>>   mlx5: split memory registration function for better performance
> >>>   mlx5: remove TX gather support
> >>>   mlx5: remove RX scatter support
> >>>   mlx5: remove configuration variable for maximum number of segments
> >>>   mlx5: remove inline TX support
> >>>   mlx5: split TX queue structure
> >>>   mlx5: split RX queue structure
> >>>   mlx5: update prerequisites for upcoming enhancements
> >>>   mlx5: add definitions for data path without Verbs
> >>>   mlx5: add support for configuration through kvargs
> >>>   mlx5: add TX/RX burst function selection wrapper
> >>>   mlx5: refactor RX data path
> >>>   mlx5: refactor TX data path
> >>>   mlx5: handle RX CQE compression
> >>>   mlx5: add support for multi-packet send
> >>>
> >>> Yaacov Hazan (1):
> >>>   mlx5: add support for inline send
> >>>
> >>
> >> I run basic checks to the patchset:
> >>
> >> There are various checkpatch warnings, all are warning or check level
> >>
> >> Patch 8 and 13 failed to apply with via git, -looks line line numbers
> >> shifted a little, this is not a problem since eventually it applies but
> >> just for your information.
> >>
> >> check-git-log is giving following errors, it is mainly case issue in Rx/Tx:
> >> Wrong headline lowercase:
> >> mlx5: resurrect RX scatter support
> >> mlx5: make RX queue reinitialization safer
> >> mlx5: remove redundant RX queue initialization code
> >> mlx5: resurrect TX gather support
> >> mlx5: check remaining space while processing TX burst
> >> mlx5: add debugging information about TX queues capabilities
> >> mlx5: replace countdown with threshold for TX completions
> >> mlx5: handle RX CQE compression
> >> mlx5: refactor RX data path
> >> mlx5: add TX/RX burst function selection wrapper
> >> mlx5: split RX queue structure
> >> mlx5: split TX queue structure
> >> mlx5: remove inline TX support
> >> mlx5: remove RX scatter support
> >> mlx5: remove TX gather support
> >> Headline too long:
> >> mlx5: remove configuration variable for maximum number of segments
> >> mlx5: split memory registration function for better performance
> >>
> >>
> >> It compiles fine.
> >>
> >> Regards,
> >> ferruh
> > 
> > Hi ferruh,
> > 
> > In fact, It does not apply well on top the current DPDK master branch.
> > 
> 
> I did able to apply on top of rel_16_07 branch using "patch" binary. but
> if you think it doesn't apply well, any plan to send a new version?
> 
> Thanks,
> ferruh

I am finishing the V2, with some small fixes (it will be more detailed
in the cover letter).

It will be sent in few minutes the necessary time to run the check-*
scripts on it.

Thanks,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 00/24] Refactor mlx5 to improve performance

2016-06-20 Thread Nélio Laranjeiro
On Fri, Jun 17, 2016 at 05:09:43PM +0100, Ferruh Yigit wrote:
> On 6/8/2016 10:47 AM, Nelio Laranjeiro wrote:
> > Enhance mlx5 with a data path that bypasses Verbs.
> > 
> > The first half of this patchset removes support for functionality completely
> > rewritten in the second half (scatter/gather, inline send), while the data
> > path is refactored without Verbs.
> > 
> > The PMD remains usable during the transition.
> > 
> > This patchset must be applied after "Miscellaneous fixes for mlx4 and mlx5".
> > 
> > Adrien Mazarguil (8):
> >   mlx5: replace countdown with threshold for TX completions
> >   mlx5: add debugging information about TX queues capabilities
> >   mlx5: check remaining space while processing TX burst
> >   mlx5: resurrect TX gather support
> >   mlx5: work around spurious compilation errors
> >   mlx5: remove redundant RX queue initialization code
> >   mlx5: make RX queue reinitialization safer
> >   mlx5: resurrect RX scatter support
> > 
> > Nelio Laranjeiro (15):
> >   mlx5: split memory registration function for better performance
> >   mlx5: remove TX gather support
> >   mlx5: remove RX scatter support
> >   mlx5: remove configuration variable for maximum number of segments
> >   mlx5: remove inline TX support
> >   mlx5: split TX queue structure
> >   mlx5: split RX queue structure
> >   mlx5: update prerequisites for upcoming enhancements
> >   mlx5: add definitions for data path without Verbs
> >   mlx5: add support for configuration through kvargs
> >   mlx5: add TX/RX burst function selection wrapper
> >   mlx5: refactor RX data path
> >   mlx5: refactor TX data path
> >   mlx5: handle RX CQE compression
> >   mlx5: add support for multi-packet send
> > 
> > Yaacov Hazan (1):
> >   mlx5: add support for inline send
> > 
> 
> I run basic checks to the patchset:
> 
> There are various checkpatch warnings, all are warning or check level
> 
> Patch 8 and 13 failed to apply with via git, -looks line line numbers
> shifted a little, this is not a problem since eventually it applies but
> just for your information.
> 
> check-git-log is giving following errors, it is mainly case issue in Rx/Tx:
> Wrong headline lowercase:
> mlx5: resurrect RX scatter support
> mlx5: make RX queue reinitialization safer
> mlx5: remove redundant RX queue initialization code
> mlx5: resurrect TX gather support
> mlx5: check remaining space while processing TX burst
> mlx5: add debugging information about TX queues capabilities
> mlx5: replace countdown with threshold for TX completions
> mlx5: handle RX CQE compression
> mlx5: refactor RX data path
> mlx5: add TX/RX burst function selection wrapper
> mlx5: split RX queue structure
> mlx5: split TX queue structure
> mlx5: remove inline TX support
> mlx5: remove RX scatter support
> mlx5: remove TX gather support
> Headline too long:
> mlx5: remove configuration variable for maximum number of segments
> mlx5: split memory registration function for better performance
> 
> 
> It compiles fine.
> 
> Regards,
> ferruh

Hi ferruh,

In fact, It does not apply well on top the current DPDK master branch.

Thanks.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 00/24] Refactor mlx5 to improve performance

2016-06-14 Thread Nélio Laranjeiro
On Mon, Jun 13, 2016 at 11:50:48AM -0700, Javier Blazquez wrote:
>[...] 
> This is a very exciting patch. I applied it and reran some microbenchmarks
> of mine that test the TX and RX paths separately. These are the results I
> got:
> 
> TX path (burst = 64 packets)
> 
> 1 thread - 2 ports - 4 queues per port: 39Mpps => 48Mpps
> 2 threads - 2 ports - 2 queues per port: 60Mpps => 60Mpps (hardware
> limitation?)

To be able to reach higher values you will need to configure the inline
feature with the device argument txq_inline, and only activate it with
more than 1 queue, this can be done with the txq_min_inline argument.

This feature helps the NIC by reducing the PCI back-pressure, in
counterpart it will consume more CPU cycles.

You can take a look to the NIC documentation (doc/guides/nics/mlx5.rst)
updated in this path-set which explains both txq_inline and
txqs_min_inline device arguments.

> RX path (burst = 32 packets)
> 
> 1 thread - 2 ports - 4 queues per port: 38Mpps => 46Mpps
> 2 threads - 2 ports - 2 queues per port: 43Mpps => 50Mpps
> 
> The tests were run on the following hardware, using DPDK master with this
> patch and the "Miscellaneous fixes for mlx4 and mlx5" patch applied:
> 
> 2x Intel Xeon E5-2680 v3 2.5GHz
> 64GB DDR4-2133
> 1x Mellanox ConnectX-4 EN, 40/56GbE dual-port, PCIe3.0 x8 (MCX414A-BCAT)
> 
> I haven't test it extensively outside of these microbenchmarks, but so far
> this patch has been working great on my end, so:
> 
> tested-by: Javier Blazquez 

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v10 0/8] ethdev: 100G and link speed API refactoring

2016-03-15 Thread Nélio Laranjeiro
On Mon, Mar 14, 2016 at 10:55:38PM +0100, Thomas Monjalon wrote:
> Re-spin of the Marc's patchset.
> The first version was sent 10 months ago!
> There are still too few tests and reviews but it is now time to move
> forward with this rework.
> Some issues were remaining in v9 and were difficult to see because it
> was mainly one big patch. That's why I've split it in several steps
> and fixed/reworked some pieces.
> There will be an exception to integrate this feature in 16.04-rc2.
> Please test and review shortly, thanks!
>[...] 

Tested-by: Nelio Laranjeiro 

 1. On Mellanox ConnectX-3, Connexct-4 100G, ConnectX-4 Lx, speed is
correctly displayed according to the negotiated link speed.

 2. Configuring speed tested, not working as expected.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 1/2] ethdev: bump library version

2016-03-09 Thread Nélio Laranjeiro
On Wed, Mar 09, 2016 at 03:14:07PM +0100, Thomas Monjalon wrote:
> There was an ABI change and more are coming in the release 16.04.
> 
> Fixes: a9963a86b2e1 ("ethdev: increase RETA entry size")
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  doc/guides/rel_notes/release_16_04.rst | 5 -
>  lib/librte_ether/Makefile  | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_16_04.rst 
> b/doc/guides/rel_notes/release_16_04.rst
> index 96f144e..8101f4c 100644
> --- a/doc/guides/rel_notes/release_16_04.rst
> +++ b/doc/guides/rel_notes/release_16_04.rst
> @@ -148,6 +148,9 @@ ABI Changes
>  * The fields in ethdev structure ``rte_eth_fdir_masks`` were changed
>to be in big endian.
>  
> +* The RETA entry size in ``rte_eth_rss_reta_entry64`` has been increased
> +  from 8-bit to 16-bit.
> +
>  
>  Shared Library Versions
>  ---
> @@ -158,7 +161,7 @@ The libraries prepended with a plus sign were incremented 
> in this version.
>  
>  .. code-block:: diff
>  
> - libethdev.so.2
> +   + libethdev.so.3
>   librte_acl.so.2
>   librte_cfgfile.so.2
>   librte_cmdline.so.1
> diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
> index 3e81a0e..e810284 100644
> --- a/lib/librte_ether/Makefile
> +++ b/lib/librte_ether/Makefile
> @@ -41,7 +41,7 @@ CFLAGS += $(WERROR_FLAGS)
>  
>  EXPORT_MAP := rte_ether_version.map
>  
> -LIBABIVER := 2
> +LIBABIVER := 3
>  
>  SRCS-y += rte_ethdev.c
>  
> -- 
> 2.7.0
> 

Series Acked-by: Nelio Laranjeiro 

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v9 3/4] ethdev: redesign link speed config API

2016-03-09 Thread Nélio Laranjeiro
On Wed, Mar 09, 2016 at 11:09:55AM +0100, Marc wrote:
>On 9 March 2016 at 09:45, N?lio Laranjeiro <[1]nelio.laranjeiro at 
> 6wind.com>
>wrote:
> 
>  Hi Marc,
> 
>  A small remark bellow.
>  On Tue, Mar 01, 2016 at 01:45:50AM +0100, Marc Sune wrote:
>  > This patch redesigns the API to set the link speed/s configure
>  > for an ethernet port. Specifically:
>  >
>  > - it allows to define a set of advertised speeds for
>  >? ?auto-negociation.
>  > - it allows to disable link auto-negociation (single fixed speed).
>  > - default: auto-negociate all supported speeds.
>  >
>  > Other changes:
>  >
>  > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
>  >? ?values of all supported link speeds, in Mbps.
>  > * Converted link_speed to uint32_t to accomodate 100G speeds
>  >? ?and beyond (bug).
>  > * Added autoneg flag in struct rte_eth_link to indicate if
>  >? ?link speed was a result of auto-negociation or was fixed
>  >? ?by configuration.
>  > * Added utility function to convert numeric speeds to bitmap
>  >? ?fields.
>  > * Added rte_eth_speed_to_bm_flag() to version map.
>  >
>  > Signed-off-by: Marc Sune <[2]marcdevel at gmail.com>
>  > ---
>  >? app/test-pipeline/init.c? ? ? ? ? ? ? ? ? |? ?2 +-
>  >? app/test-pmd/cmdline.c? ? ? ? ? ? ? ? ? ? | 124
>  +++---
>  >? app/test-pmd/config.c? ? ? ? ? ? ? ? ? ? ?|? ?4 +-
>  >? app/test/virtual_pmd.c? ? ? ? ? ? ? ? ? ? |? ?4 +-
>  >? drivers/net/af_packet/rte_eth_af_packet.c |? ?5 +-
>  >? drivers/net/bnx2x/bnx2x_ethdev.c? ? ? ? ? |? ?8 +-
>  >? drivers/net/bonding/rte_eth_bond_8023ad.c |? 14 ++--
>  >? drivers/net/cxgbe/base/t4_hw.c? ? ? ? ? ? |? ?8 +-
>  >? drivers/net/cxgbe/cxgbe_ethdev.c? ? ? ? ? |? ?2 +-
>  >? drivers/net/e1000/em_ethdev.c? ? ? ? ? ? ?| 116
>  ++--
>  >? drivers/net/e1000/igb_ethdev.c? ? ? ? ? ? | 111
>  +-
>  >? drivers/net/fm10k/fm10k_ethdev.c? ? ? ? ? |? ?8 +-
>  >? drivers/net/i40e/i40e_ethdev.c? ? ? ? ? ? |? 73 +-
>  >? drivers/net/i40e/i40e_ethdev_vf.c? ? ? ? ?|? 11 +--
>  >? drivers/net/ixgbe/ixgbe_ethdev.c? ? ? ? ? |? 78 ---
>  >? drivers/net/mlx4/mlx4.c? ? ? ? ? ? ? ? ? ?|? ?6 +-
>  >? drivers/net/mlx5/mlx5_ethdev.c? ? ? ? ? ? |? 10 +--
>  >? drivers/net/mpipe/mpipe_tilegx.c? ? ? ? ? |? ?6 +-
>  >? drivers/net/nfp/nfp_net.c? ? ? ? ? ? ? ? ?|? ?4 +-
>  >? drivers/net/null/rte_eth_null.c? ? ? ? ? ?|? ?5 +-
>  >? drivers/net/pcap/rte_eth_pcap.c? ? ? ? ? ?|? ?9 ++-
>  >? drivers/net/ring/rte_eth_ring.c? ? ? ? ? ?|? ?5 +-
>  >? drivers/net/virtio/virtio_ethdev.c? ? ? ? |? ?2 +-
>  >? drivers/net/virtio/virtio_ethdev.h? ? ? ? |? ?2 -
>  >? drivers/net/vmxnet3/vmxnet3_ethdev.c? ? ? |? ?5 +-
>  >? drivers/net/xenvirt/rte_eth_xenvirt.c? ? ?|? ?5 +-
>  >? examples/ip_pipeline/config_parse.c? ? ? ?|? ?3 +-
>  >? lib/librte_ether/rte_ethdev.c? ? ? ? ? ? ?|? 49 
>  >? lib/librte_ether/rte_ethdev.h? ? ? ? ? ? ?| 113
>  +--
>  >? lib/librte_ether/rte_ether_version.map? ? |? ?6 ++
>  >? 30 files changed, 448 insertions(+), 350 deletions(-)
>  >
>  > diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
>  > index db2196b..6a69fe2 100644
>  > --- a/app/test-pipeline/init.c
>  > +++ b/app/test-pipeline/init.c
>  > @@ -200,7 +200,7 @@ app_ports_check_link(void)
>  >? ? ? ? ? ? ? ?port = (uint8_t) app.ports[i];
>  >? ? ? ? ? ? ? ?memset(, 0, sizeof(link));
>  >? ? ? ? ? ? ? ?rte_eth_link_get_nowait(port, );
>  > -? ? ? ? ? ? ?RTE_LOG(INFO, USER1, "Port %u (%u Gbps) %s\n",
>  > +? ? ? ? ? ? ?RTE_LOG(INFO, USER1, "Port %u (%d Gbps) %s\n",
>  >? ? ? ? ? ? ? ? ? ? ? ?port,
>  >? ? ? ? ? ? ? ? ? ? ? ?link.link_speed / 1000,
>  >? ? ? ? ? ? ? ? ? ? ? ?link.link_status ? "UP" : "DOWN");
>  > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>  > index 52e9f5f..57ad25f 100644
>  > --- a/app/test-pmd/cmdline.c
>  > +++ b/app/test-pmd/cmdline.c
>  > @@ -956,14 +956,65 @@ struct cmd_config_speed_all {
>  >? ? ? ?cmdline_fixed_string_t value2;
>  >? };
>  >
>  > +static int
>  > +parse_and_check_speed_duplex(char *value1, char *value2, uint32_t
>  *link_speed)
> 
>  "value" variables should have the more friendly name, we need to read
>  the
>  code to understand what is value1 and value2.
> 
>Hello,
>Ok, but note that the entire source code of cmdline.c uses value1 and
>value2 to reference speed and duplex mode. This is not something I
>introduced in this patch.
>Marc

Ok, I did not look in the whole file only in your patch.  You kept
the logic of the file, it is good for me.

>  >[...]
>  --
>  N?lio 

[dpdk-dev] [PATCH v9 0/4] ethdev: add speed capabilities and refactor link API

2016-03-09 Thread Nélio Laranjeiro
On Wed, Mar 09, 2016 at 10:29:38AM +0100, N?lio Laranjeiro wrote:
> On Tue, Mar 08, 2016 at 05:53:05PM +0100, N?lio Laranjeiro wrote:
> > On Tue, Mar 08, 2016 at 04:00:29PM +0100, Marc Sune wrote:
> > > 2016-03-01 1:45 GMT+01:00 Marc Sune :
> > > 
> > > > The current rte_eth_dev_info abstraction does not provide any mechanism 
> > > > to
> > > > get the supported speed(s) of an ethdev.
> > > >
> > > > For some drivers (e.g. ixgbe), an educated guess could be done based on 
> > > > the
> > > > driver's name (driver_name in rte_eth_dev_info), see:
> > > >
> > > > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> > > >
> > > > However, i) doing string comparisons is annoying, and can silently
> > > > break existing applications if PMDs change their names ii) it does not
> > > > provide all the supported capabilities of the ethdev iii) for some 
> > > > drivers
> > > > it
> > > > is impossible determine correctly the (max) speed by the application
> > > > (e.g. in i40, distinguish between XL710 and X710).
> > > >
> > > > In addition, the link APIs do not allow to define a set of advertised 
> > > > link
> > > > speeds for autonegociation.
> > > >
> > > > This series of patches adds the following capabilities:
> > > >
> > > > * speed_capa bitmap in rte_eth_dev_info, which is filled by the PMDs
> > > >   according to the physical device capabilities.
> > > > * refactors link API in ethdev to allow the definition of the advertised
> > > >   link speeds, fix speed (no auto-negociation) or advertise all 
> > > > supported
> > > >   speeds (default).
> > > >
> > > > WARNING: this patch series, specifically 3/4, is NOT tested for most of 
> > > > the
> > > > PMDs, due to the lack of hardware. Only generic EM is tested (VM).
> > > > Reviewing
> > > > and testing required by PMD maintainers.
> > > >
> > > > * * * * *
> > > >
> > > > v2: rebase, converted speed_capa into 32 bits bitmap, fixed alignment
> > > > (checkpatch).
> > > >
> > > > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP into
> > > > ETH_SPEED.
> > > > Converted field speed in struct rte_eth_conf to speed, to allow a
> > > > bitmap
> > > > for defining the announced speeds, as suggested M. Brorup. Fixed
> > > > spelling
> > > > issues.
> > > >
> > > > v4: fixed errata in the documentation of field speeds of rte_eth_conf, 
> > > > and
> > > > commit 1/2 message. rebased to v2.1.0. v3 was incorrectly based on
> > > > ~2.1.0-rc1.
> > > >
> > > > v5: revert to v2 speed capabilities patch. Fixed MLX4 speed capabilities
> > > > (thanks N. Laranjeiro). Refactored link speed API to allow setting
> > > > advertised speeds (3/4). Added NO_AUTONEG option to explicitely 
> > > > disable
> > > > auto-negociation. Updated 2.2 rel. notes (4/4). Rebased to current
> > > > HEAD.
> > > >
> > > > v6: Move link_duplex to be part of bitfield. Fixed i40 autoneg flag link
> > > > update code. Added rte_eth_speed_to_bm_flag() to .map file. Fixed 
> > > > other
> > > > spelling issues. Rebased to current HEAD.
> > > >
> > > > v7: Rebased to current HEAD. Moved documentation to v2.3. Still needs
> > > > testing
> > > > from PMD maintainers.
> > > >
> > > > v8: Rebased to current HEAD. Modified em driver impl. to not touch base
> > > > files.
> > > > Merged patch 5 into 3 (map file). Changed numeric speed to a 64 bit
> > > > value.
> > > > Filled-in speed capabilities for drivers bnx2x, cxgbe, mlx5 and nfp 
> > > > in
> > > > addition to the ones of previous patch sets.
> > > >
> > > > v9: rebased to current HEAD. Reverted numeric speed to 32 bit in struct
> > > > rte_eth_link (no atomic link get > 64bit). Fixed mlx5 driver
> > > > compilation
> > > > and link speeds. Moved documentation to release_16_04.rst and fixed
> > > > several
> > > > issues. Upgrade NIC notes with speed capabilities.
> > > >
> > > 
> > > Anyone interested in reviewing and _testing_ this series?
> > > 
> > > Thank you
> > > Marc
> > 
> > Hi Marc,
> > 
> > I will take a look tomorrow morning and run test on Mellanox NICs
> > (ConnectX 3 and 4).
> > 
> > I do not have access to the others NICs, if those who have can do
> > it, could be really great.
> > 
> > Regards,
> 
> It works as expected with Mellanox NICs.
> 
> Regards,
> 
> -- 
> N?lio Laranjeiro
> 6WIND

Tested-by: Nelio Laranjeiro 

- OS/Kernel: Debian 8/3.16.0-4-amd64
- GCC: gcc (Debian 4.9.2-10) 4.9.2
- CPU: Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz
- MLNX OFED: 3.2-2.0.0.0
- NIC: ConnectX 4 100G

- OS/Kernel: Debian 7/3.16.0-0.bpo.4-amd64
- GCC: gcc (Debian 4.7.2-5) 4.7.2
- CPU: Intel(R) Xeon(R) CPU E5-2648L 0 @ 1.80GHz
- MLNX OFED: 3.2-2.0.0.0
- NIC: ConnectX 3 Pro


1. Link displayed at the correct negotiated speed:
   - 40Gbps for ConnectX3 Pro
   - 100Gbps for ConnectX4 100G
   - 40Gbps for ConnectX4 100G on a 40G link.

2. Configuring speed:
   - Not supported yet for Mellanox as expected.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v9 0/4] ethdev: add speed capabilities and refactor link API

2016-03-09 Thread Nélio Laranjeiro
On Tue, Mar 08, 2016 at 05:53:05PM +0100, N?lio Laranjeiro wrote:
> On Tue, Mar 08, 2016 at 04:00:29PM +0100, Marc Sune wrote:
> > 2016-03-01 1:45 GMT+01:00 Marc Sune :
> > 
> > > The current rte_eth_dev_info abstraction does not provide any mechanism to
> > > get the supported speed(s) of an ethdev.
> > >
> > > For some drivers (e.g. ixgbe), an educated guess could be done based on 
> > > the
> > > driver's name (driver_name in rte_eth_dev_info), see:
> > >
> > > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> > >
> > > However, i) doing string comparisons is annoying, and can silently
> > > break existing applications if PMDs change their names ii) it does not
> > > provide all the supported capabilities of the ethdev iii) for some drivers
> > > it
> > > is impossible determine correctly the (max) speed by the application
> > > (e.g. in i40, distinguish between XL710 and X710).
> > >
> > > In addition, the link APIs do not allow to define a set of advertised link
> > > speeds for autonegociation.
> > >
> > > This series of patches adds the following capabilities:
> > >
> > > * speed_capa bitmap in rte_eth_dev_info, which is filled by the PMDs
> > >   according to the physical device capabilities.
> > > * refactors link API in ethdev to allow the definition of the advertised
> > >   link speeds, fix speed (no auto-negociation) or advertise all supported
> > >   speeds (default).
> > >
> > > WARNING: this patch series, specifically 3/4, is NOT tested for most of 
> > > the
> > > PMDs, due to the lack of hardware. Only generic EM is tested (VM).
> > > Reviewing
> > > and testing required by PMD maintainers.
> > >
> > > * * * * *
> > >
> > > v2: rebase, converted speed_capa into 32 bits bitmap, fixed alignment
> > > (checkpatch).
> > >
> > > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP into
> > > ETH_SPEED.
> > > Converted field speed in struct rte_eth_conf to speed, to allow a
> > > bitmap
> > > for defining the announced speeds, as suggested M. Brorup. Fixed
> > > spelling
> > > issues.
> > >
> > > v4: fixed errata in the documentation of field speeds of rte_eth_conf, and
> > > commit 1/2 message. rebased to v2.1.0. v3 was incorrectly based on
> > > ~2.1.0-rc1.
> > >
> > > v5: revert to v2 speed capabilities patch. Fixed MLX4 speed capabilities
> > > (thanks N. Laranjeiro). Refactored link speed API to allow setting
> > > advertised speeds (3/4). Added NO_AUTONEG option to explicitely 
> > > disable
> > > auto-negociation. Updated 2.2 rel. notes (4/4). Rebased to current
> > > HEAD.
> > >
> > > v6: Move link_duplex to be part of bitfield. Fixed i40 autoneg flag link
> > > update code. Added rte_eth_speed_to_bm_flag() to .map file. Fixed 
> > > other
> > > spelling issues. Rebased to current HEAD.
> > >
> > > v7: Rebased to current HEAD. Moved documentation to v2.3. Still needs
> > > testing
> > > from PMD maintainers.
> > >
> > > v8: Rebased to current HEAD. Modified em driver impl. to not touch base
> > > files.
> > > Merged patch 5 into 3 (map file). Changed numeric speed to a 64 bit
> > > value.
> > > Filled-in speed capabilities for drivers bnx2x, cxgbe, mlx5 and nfp in
> > > addition to the ones of previous patch sets.
> > >
> > > v9: rebased to current HEAD. Reverted numeric speed to 32 bit in struct
> > > rte_eth_link (no atomic link get > 64bit). Fixed mlx5 driver
> > > compilation
> > > and link speeds. Moved documentation to release_16_04.rst and fixed
> > > several
> > > issues. Upgrade NIC notes with speed capabilities.
> > >
> > 
> > Anyone interested in reviewing and _testing_ this series?
> > 
> > Thank you
> > Marc
> 
> Hi Marc,
> 
> I will take a look tomorrow morning and run test on Mellanox NICs
> (ConnectX 3 and 4).
> 
> I do not have access to the others NICs, if those who have can do
> it, could be really great.
> 
> Regards,

It works as expected with Mellanox NICs.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v9 3/4] ethdev: redesign link speed config API

2016-03-09 Thread Nélio Laranjeiro
Hi Marc,

A small remark bellow.

On Tue, Mar 01, 2016 at 01:45:50AM +0100, Marc Sune wrote:
> This patch redesigns the API to set the link speed/s configure
> for an ethernet port. Specifically:
> 
> - it allows to define a set of advertised speeds for
>   auto-negociation.
> - it allows to disable link auto-negociation (single fixed speed).
> - default: auto-negociate all supported speeds.
> 
> Other changes:
> 
> * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
>   values of all supported link speeds, in Mbps.
> * Converted link_speed to uint32_t to accomodate 100G speeds
>   and beyond (bug).
> * Added autoneg flag in struct rte_eth_link to indicate if
>   link speed was a result of auto-negociation or was fixed
>   by configuration.
> * Added utility function to convert numeric speeds to bitmap
>   fields.
> * Added rte_eth_speed_to_bm_flag() to version map.
> 
> Signed-off-by: Marc Sune 
> ---
>  app/test-pipeline/init.c  |   2 +-
>  app/test-pmd/cmdline.c| 124 
> +++---
>  app/test-pmd/config.c |   4 +-
>  app/test/virtual_pmd.c|   4 +-
>  drivers/net/af_packet/rte_eth_af_packet.c |   5 +-
>  drivers/net/bnx2x/bnx2x_ethdev.c  |   8 +-
>  drivers/net/bonding/rte_eth_bond_8023ad.c |  14 ++--
>  drivers/net/cxgbe/base/t4_hw.c|   8 +-
>  drivers/net/cxgbe/cxgbe_ethdev.c  |   2 +-
>  drivers/net/e1000/em_ethdev.c | 116 ++--
>  drivers/net/e1000/igb_ethdev.c| 111 +-
>  drivers/net/fm10k/fm10k_ethdev.c  |   8 +-
>  drivers/net/i40e/i40e_ethdev.c|  73 +-
>  drivers/net/i40e/i40e_ethdev_vf.c |  11 +--
>  drivers/net/ixgbe/ixgbe_ethdev.c  |  78 ---
>  drivers/net/mlx4/mlx4.c   |   6 +-
>  drivers/net/mlx5/mlx5_ethdev.c|  10 +--
>  drivers/net/mpipe/mpipe_tilegx.c  |   6 +-
>  drivers/net/nfp/nfp_net.c |   4 +-
>  drivers/net/null/rte_eth_null.c   |   5 +-
>  drivers/net/pcap/rte_eth_pcap.c   |   9 ++-
>  drivers/net/ring/rte_eth_ring.c   |   5 +-
>  drivers/net/virtio/virtio_ethdev.c|   2 +-
>  drivers/net/virtio/virtio_ethdev.h|   2 -
>  drivers/net/vmxnet3/vmxnet3_ethdev.c  |   5 +-
>  drivers/net/xenvirt/rte_eth_xenvirt.c |   5 +-
>  examples/ip_pipeline/config_parse.c   |   3 +-
>  lib/librte_ether/rte_ethdev.c |  49 
>  lib/librte_ether/rte_ethdev.h | 113 +--
>  lib/librte_ether/rte_ether_version.map|   6 ++
>  30 files changed, 448 insertions(+), 350 deletions(-)
> 
> diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
> index db2196b..6a69fe2 100644
> --- a/app/test-pipeline/init.c
> +++ b/app/test-pipeline/init.c
> @@ -200,7 +200,7 @@ app_ports_check_link(void)
>   port = (uint8_t) app.ports[i];
>   memset(, 0, sizeof(link));
>   rte_eth_link_get_nowait(port, );
> - RTE_LOG(INFO, USER1, "Port %u (%u Gbps) %s\n",
> + RTE_LOG(INFO, USER1, "Port %u (%d Gbps) %s\n",
>   port,
>   link.link_speed / 1000,
>   link.link_status ? "UP" : "DOWN");
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 52e9f5f..57ad25f 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -956,14 +956,65 @@ struct cmd_config_speed_all {
>   cmdline_fixed_string_t value2;
>  };
>  
> +static int
> +parse_and_check_speed_duplex(char *value1, char *value2, uint32_t 
> *link_speed)

"value" variables should have the more friendly name, we need to read the
code to understand what is value1 and value2.

>[...] 

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v9 0/4] ethdev: add speed capabilities and refactor link API

2016-03-08 Thread Nélio Laranjeiro
On Tue, Mar 08, 2016 at 04:00:29PM +0100, Marc Sune wrote:
> 2016-03-01 1:45 GMT+01:00 Marc Sune :
> 
> > The current rte_eth_dev_info abstraction does not provide any mechanism to
> > get the supported speed(s) of an ethdev.
> >
> > For some drivers (e.g. ixgbe), an educated guess could be done based on the
> > driver's name (driver_name in rte_eth_dev_info), see:
> >
> > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> >
> > However, i) doing string comparisons is annoying, and can silently
> > break existing applications if PMDs change their names ii) it does not
> > provide all the supported capabilities of the ethdev iii) for some drivers
> > it
> > is impossible determine correctly the (max) speed by the application
> > (e.g. in i40, distinguish between XL710 and X710).
> >
> > In addition, the link APIs do not allow to define a set of advertised link
> > speeds for autonegociation.
> >
> > This series of patches adds the following capabilities:
> >
> > * speed_capa bitmap in rte_eth_dev_info, which is filled by the PMDs
> >   according to the physical device capabilities.
> > * refactors link API in ethdev to allow the definition of the advertised
> >   link speeds, fix speed (no auto-negociation) or advertise all supported
> >   speeds (default).
> >
> > WARNING: this patch series, specifically 3/4, is NOT tested for most of the
> > PMDs, due to the lack of hardware. Only generic EM is tested (VM).
> > Reviewing
> > and testing required by PMD maintainers.
> >
> > * * * * *
> >
> > v2: rebase, converted speed_capa into 32 bits bitmap, fixed alignment
> > (checkpatch).
> >
> > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP into
> > ETH_SPEED.
> > Converted field speed in struct rte_eth_conf to speed, to allow a
> > bitmap
> > for defining the announced speeds, as suggested M. Brorup. Fixed
> > spelling
> > issues.
> >
> > v4: fixed errata in the documentation of field speeds of rte_eth_conf, and
> > commit 1/2 message. rebased to v2.1.0. v3 was incorrectly based on
> > ~2.1.0-rc1.
> >
> > v5: revert to v2 speed capabilities patch. Fixed MLX4 speed capabilities
> > (thanks N. Laranjeiro). Refactored link speed API to allow setting
> > advertised speeds (3/4). Added NO_AUTONEG option to explicitely disable
> > auto-negociation. Updated 2.2 rel. notes (4/4). Rebased to current
> > HEAD.
> >
> > v6: Move link_duplex to be part of bitfield. Fixed i40 autoneg flag link
> > update code. Added rte_eth_speed_to_bm_flag() to .map file. Fixed other
> > spelling issues. Rebased to current HEAD.
> >
> > v7: Rebased to current HEAD. Moved documentation to v2.3. Still needs
> > testing
> > from PMD maintainers.
> >
> > v8: Rebased to current HEAD. Modified em driver impl. to not touch base
> > files.
> > Merged patch 5 into 3 (map file). Changed numeric speed to a 64 bit
> > value.
> > Filled-in speed capabilities for drivers bnx2x, cxgbe, mlx5 and nfp in
> > addition to the ones of previous patch sets.
> >
> > v9: rebased to current HEAD. Reverted numeric speed to 32 bit in struct
> > rte_eth_link (no atomic link get > 64bit). Fixed mlx5 driver
> > compilation
> > and link speeds. Moved documentation to release_16_04.rst and fixed
> > several
> > issues. Upgrade NIC notes with speed capabilities.
> >
> 
> Anyone interested in reviewing and _testing_ this series?
> 
> Thank you
> Marc

Hi Marc,

I will take a look tomorrow morning and run test on Mellanox NICs
(ConnectX 3 and 4).

I do not have access to the others NICs, if those who have can do
it, could be really great.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v2 1/3] cmdline: increase command line buffer

2016-02-26 Thread Nélio Laranjeiro
On Mon, Jan 18, 2016 at 03:38:43PM +0100, Olivier MATZ wrote:
> Hi,
> 
> On 01/15/2016 10:00 AM, Panu Matilainen wrote:
>  diff --git a/lib/librte_cmdline/cmdline_rdline.h
>  b/lib/librte_cmdline/cmdline_rdline.h
>  index b9aad9b..72e2dad 100644
>  --- a/lib/librte_cmdline/cmdline_rdline.h
>  +++ b/lib/librte_cmdline/cmdline_rdline.h
>  @@ -93,7 +93,7 @@ extern "C" {
>    #endif
> 
>    /* configuration */
>  -#define RDLINE_BUF_SIZE 256
>  +#define RDLINE_BUF_SIZE 512
>    #define RDLINE_PROMPT_SIZE  32
>    #define RDLINE_VT100_BUF_SIZE  8
>    #define RDLINE_HISTORY_BUF_SIZE BUFSIZ
> >>>
> >>> Having to break a library ABI for a change like this is a bit
> >>> ridiculous.
> >>
> >> Sure, but John McNamara needed it to handle flow director with IPv6[1].
> >>
> >> For my part, I was needing it to manipulate the RETA table, but as I
> >> wrote in the cover letter, it ends by breaking other commands.
> >> Olivier Matz, has proposed another way to handle long commands lines[2],
> >> it could be a good idea to go on this direction.
> >>
> >> For RETA situation, we already discussed on a new API, but for now, I
> >> do not have time for it (and as it is another ABI breakage it could only
> >> be done for 16.07 or 2.4)[3].
> >>
> >> If this patch is no more needed we can just drop it, for that I would
> >> like to have the point of view from John.
> > 
> > Note that I was not objecting to the patch as such, I can easily see 256
> > characters not being enough for commandline buffer.
> > 
> > I was merely noting that having to break an ABI to increase an
> > effectively internal buffer size is a sign of a, um, less-than-optimal
> > library design.
> 
> You are right about the cmdline ABI. Changing this buffer size
> should not imply an ABI change. I'll try to find some time to
> investigate this issue.
> 
> Another question we could raise is: should we export the API of
> librte_cmdline to external applications? Now that baremetal dpdk is
> not supported, having this library in dpdk is probably useless as
> we can surely find standard replacements for it. A first step could
> be to mark it as "internal".
> 
> About the patch N?lio's patch itself, I'm not so convinced having more
> than 256 characters is absolutely required, and I would prefer to see
> the commands beeing reworked to be more human-readable. On the other
> hand, the ABI breakage was announced so there is no reason to nack
> this patch now.
> 
> Regards,
> Olivier

John,

What is your position about this patch?
Is it still needed?

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v8 2/4] ethdev: Fill speed capability bitmaps in the PMDs

2016-02-16 Thread Nélio Laranjeiro
Hi Marc,

On Mon, Feb 15, 2016 at 06:14:42PM +0100, Marc wrote:
> Rahul, Neilo, Jing D, et al
> 
> On 15 February 2016 at 15:43, Rahul Lakkireddy  chelsio.com
> > wrote:
> 
> > Hi Marc,
> >
> > On Sunday, February 02/14/16, 2016 at 23:17:37 +0100, Marc Sune wrote:
> > > Added speed capabilities to all pmds supporting physical NICs:
> > >
> > > * e1000
> > > * ixgbe
> > > * i40
> > > * bnx2x
> > > * cxgbe
> > > * mlx4
> > > * mlx5
> > > * nfp
> > > * fm10k
> > >
> > > Signed-off-by: Marc Sune 
> > > ---
> > >  drivers/net/bnx2x/bnx2x_ethdev.c |  1 +
> > >  drivers/net/cxgbe/cxgbe_ethdev.c |  1 +
> > >  drivers/net/e1000/em_ethdev.c|  6 ++
> > >  drivers/net/e1000/igb_ethdev.c   |  6 ++
> > >  drivers/net/fm10k/fm10k_ethdev.c |  4 
> > >  drivers/net/i40e/i40e_ethdev.c   |  9 +
> > >  drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++
> > >  drivers/net/mlx4/mlx4.c  |  4 
> > >  drivers/net/mlx5/mlx5_ethdev.c   |  5 +
> > >  drivers/net/nfp/nfp_net.c|  2 ++
> > >  10 files changed, 48 insertions(+)
> > >
> >
> > [...]
> >
> > > diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c
> > b/drivers/net/cxgbe/cxgbe_ethdev.c
> > > index 97ef152..203e119 100644
> > > --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> > > +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> > > @@ -171,6 +171,7 @@ static void cxgbe_dev_info_get(struct rte_eth_dev
> > *eth_dev,
> > >
> > >   device_info->rx_desc_lim = cxgbe_desc_lim;
> > >   device_info->tx_desc_lim = cxgbe_desc_lim;
> > > + device_info->speed_capa = ETH_SPEED_CAP_10G | ETH_SPEED_CAP_40G;
> > >  }
> > >
> >
> > Not all Chelsio NICs support _both_ 10G and 40G speed capabilities on a
> > single card.  You can query pi->link_cfg.supported to get the supported
> > speeds.  Check out print_port_info() in cxgbe_main.c to help you fill
> > in your speed capabilities for Chelsio NICs.
> >
> 
> This patch series has been long delayed, and I've been requested to merge
> it for next release if possible. Most of the feedback has been coming late
> (not for cxgbe, which is introduced in this new v8, but it did for most of
> the rest of drivers).
> 
> My proposal is simply to add in this patch series ALL possible speeds for
> that driver. Other patches can be later submitted to adjust speeds
> according to specific device model.

I agree with this.  I was asking in order to understand what your were
expecting from this API, for me it is clear.

You should just maintain the current situation i.e.
rte_eth_link.link_speed.  This is already what your patches do.

For the link speed capability (aka device_info->speed_capa), you should
add a new line "speed capability" in the doc/guides/nics/overview.rst.
Those who think it is useful will implement it in their PMD.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v8 3/4] ethdev: redesign link speed config API

2016-02-15 Thread Nélio Laranjeiro
On Sun, Feb 14, 2016 at 11:17:38PM +0100, Marc Sune wrote:
> This patch redesigns the API to set the link speed/s configure
> for an ethernet port. Specifically:
> 
> - it allows to define a set of advertised speeds for
>   auto-negociation.
> - it allows to disable link auto-negociation (single fixed speed).
> - default: auto-negociate all supported speeds.
> 
>[...]
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index c5688a7..01c3a5c 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -4265,8 +4265,8 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *info)
>   if (priv_get_ifname(priv, ) == 0)
>   info->if_index = if_nametoindex(ifname);
>  
> - info->speed_capa = ETH_SPEED_CAP_10G |ETH_SPEED_CAP_40G |
> - ETH_SPEED_CAP_56G;
> + info->speed_capa = ETH_LINK_SPEED_10G |ETH_LINK_SPEED_40G |
> + ETH_LINK_SPEED_56G;
>  
>   priv_unlock(priv);
>  }
> @@ -4636,6 +4636,8 @@ mlx4_link_update_unlocked(struct rte_eth_dev *dev, int 
> wait_to_complete)
>   dev_link.link_speed = link_speed;
>   dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
>   ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> + dev_link.link_autoneg = ~(dev->data->dev_conf.link_speeds &
> + ETH_LINK_SPEED_NO_AUTONEG);
>   if (memcmp(_link, >data->dev_link, sizeof(dev_link))) {
>   /* Link status changed. */
>   dev->data->dev_link = dev_link;
>[...]

The same modification are missing in mlx5.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v8 2/4] ethdev: Fill speed capability bitmaps in the PMDs

2016-02-15 Thread Nélio Laranjeiro
On Sun, Feb 14, 2016 at 11:17:37PM +0100, Marc Sune wrote:
> Added speed capabilities to all pmds supporting physical NICs:
> 
> * e1000
> * ixgbe
> * i40
> * bnx2x
> * cxgbe
> * mlx4
> * mlx5
> * nfp
> * fm10k
>[...]
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 1159fa3..99dac09 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -523,6 +523,11 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *info)
>* size if it is not fixed.
>* The API should be updated to solve this problem. */
>   info->reta_size = priv->ind_table_max_size;
> +
> + info->speed_capa = ETH_SPEED_CAP_1G | ETH_SPEED_CAP_10G |
> + ETH_SPEED_CAP_10G | ETH_SPEED_CAP_40G |
> + ETH_SPEED_CAP_56G;
> +
>   priv_unlock(priv);
>  }

Hi Marc,

I have a question about this information, is it a list of the
capabilities of the family or the capability of the NIC?
Because with ConnectX4 family we have a range of NICs which does not
support all this kind of speeds.

The speeds above are not completed the range is 1/10/25/40/50/100G.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2016-02-11 Thread Nélio Laranjeiro
On Tue, Feb 02, 2016 at 11:30:59PM +0100, Marc wrote:
> On 2 February 2016 at 03:20, Stephen Hemminger 
> wrote:
> 
> > On Thu, 28 Jan 2016 17:33:20 +
> > Harish Patil  wrote:
> >
> > > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric
> > >   values of all supported link speeds, in Mbps.
> >
> > I would prefer that there were no speed value macros.
> > Linux used to have these, but people kept adding new hardware speeds
> > and it soon gets out of date.
> >
> 
> I see what you mean, but I am not sure I agree. Link speeds are generally a
> reduced amount of items (~20). Though it is true it can eventually grow,
> but at small rate. Having numeric constants all over the source seems less
> readable and less maintainable (e.g. less "grepable"/"sedable") to me.
> 
> 
> >
> > If you are going to redo it, then just increase speed to 64 bit, and allow
> > any non-zero value.
> >
> 
> Value is now 32 bits, which I think is enough for future rates in mbps.
> Since these constants were there, and before doing something to have to
> revert it, can someone else give his/her opinion on this?

For non 64bit architecture it is better to keep it on 32 bit but, if this
field is only used on control plane we can afford 64 bit field and avoid
another ABI breakage (in a far future).

Even if this 32 bit field seems large enough you can already find on
Internet some reports of transmission of petabit/s [1], we can imagine a
NIC which provide this possibility by aggregating a lot of optical links
and DPDK only will see it as single one.

> If there is consensus, I've no problem on removing it for v8
> 
> Thanks
> marc

[1] http://optics.org/news/4/1/29

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v7 3/5] ethdev: redesign link speed config API

2016-01-29 Thread Nélio Laranjeiro
Hi Marc,

On Fri, Jan 29, 2016 at 01:42:05AM +0100, Marc Sune wrote:
>[...]
>  /**
> - * Device supported speeds
> - */
> -#define ETH_SPEED_CAP_NOT_PHY(0)  /*< No phy media > */
> -#define ETH_SPEED_CAP_10M_HD (1 << 0)  /*< 10 Mbps half-duplex> */
> -#define ETH_SPEED_CAP_10M_FD (1 << 1)  /*< 10 Mbps full-duplex> */
> -#define ETH_SPEED_CAP_100M_HD(1 << 2)  /*< 100 Mbps half-duplex> */
> -#define ETH_SPEED_CAP_100M_FD(1 << 3)  /*< 100 Mbps full-duplex> */
> -#define ETH_SPEED_CAP_1G (1 << 4)  /*< 1 Gbps > */
> -#define ETH_SPEED_CAP_2_5G   (1 << 5)  /*< 2.5 Gbps > */
> -#define ETH_SPEED_CAP_5G (1 << 6)  /*< 5 Gbps > */
> -#define ETH_SPEED_CAP_10G(1 << 7)  /*< 10 Mbps > */
> -#define ETH_SPEED_CAP_20G(1 << 8)  /*< 20 Gbps > */
> -#define ETH_SPEED_CAP_25G(1 << 9)  /*< 25 Gbps > */
> -#define ETH_SPEED_CAP_40G(1 << 10)  /*< 40 Gbps > */
> -#define ETH_SPEED_CAP_50G(1 << 11)  /*< 50 Gbps > */
> -#define ETH_SPEED_CAP_56G(1 << 12)  /*< 56 Gbps > */
> -#define ETH_SPEED_CAP_100G   (1 << 13)  /*< 100 Gbps > */
>[...]

In the previous commit you update mlx4.c to use those define, in this
commit it should update it again.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v2 1/3] cmdline: increase command line buffer

2016-01-15 Thread Nélio Laranjeiro
On Tue, Jan 12, 2016 at 02:46:07PM +0200, Panu Matilainen wrote:
> On 01/12/2016 12:49 PM, Nelio Laranjeiro wrote:
> >Allow long command lines in testpmd (like flow director with IPv6, ...).
> >
> >Signed-off-by: John McNamara 
> >Signed-off-by: Nelio Laranjeiro 
> >---
> >  doc/guides/rel_notes/deprecation.rst | 5 -
> >  lib/librte_cmdline/cmdline_rdline.h  | 2 +-
> >  2 files changed, 1 insertion(+), 6 deletions(-)
> >
> >diff --git a/doc/guides/rel_notes/deprecation.rst 
> >b/doc/guides/rel_notes/deprecation.rst
> >index e94d4a2..9cb288c 100644
> >--- a/doc/guides/rel_notes/deprecation.rst
> >+++ b/doc/guides/rel_notes/deprecation.rst
> >@@ -44,8 +44,3 @@ Deprecation Notices
> >and table action handlers will be updated:
> >the pipeline parameter will be added, the packets mask parameter will be
> >either removed (for input port action handler) or made input-only.
> >-
> >-* ABI changes are planned in cmdline buffer size to allow the use of long
> >-  commands (such as RETA update in testpmd).  This should impact
> >-  CMDLINE_PARSE_RESULT_BUFSIZE, STR_TOKEN_SIZE and RDLINE_BUF_SIZE.
> >-  It should be integrated in release 2.3.
> >diff --git a/lib/librte_cmdline/cmdline_rdline.h 
> >b/lib/librte_cmdline/cmdline_rdline.h
> >index b9aad9b..72e2dad 100644
> >--- a/lib/librte_cmdline/cmdline_rdline.h
> >+++ b/lib/librte_cmdline/cmdline_rdline.h
> >@@ -93,7 +93,7 @@ extern "C" {
> >  #endif
> >
> >  /* configuration */
> >-#define RDLINE_BUF_SIZE 256
> >+#define RDLINE_BUF_SIZE 512
> >  #define RDLINE_PROMPT_SIZE  32
> >  #define RDLINE_VT100_BUF_SIZE  8
> >  #define RDLINE_HISTORY_BUF_SIZE BUFSIZ
> 
> Having to break a library ABI for a change like this is a bit ridiculous.

Sure, but John McNamara needed it to handle flow director with IPv6[1].

For my part, I was needing it to manipulate the RETA table, but as I
wrote in the cover letter, it ends by breaking other commands.
Olivier Matz, has proposed another way to handle long commands lines[2],
it could be a good idea to go on this direction.

For RETA situation, we already discussed on a new API, but for now, I
do not have time for it (and as it is another ABI breakage it could only
be done for 16.07 or 2.4)[3].

If this patch is no more needed we can just drop it, for that I would
like to have the point of view from John.

> 
> I didn't try it so could be wrong, but based on a quick look, struct rdline
> could easily be made opaque to consumers by just adding functions for
> allocating and freeing it.
> 
>   - Panu -
> 

[1] http://dpdk.org/ml/archives/dev/2015-November/027643.html
[2] http://dpdk.org/ml/archives/dev/2015-November/028557.html
[3] http://dpdk.org/ml/archives/dev/2015-October/025294.html

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if packet type is set

2016-01-05 Thread Nélio Laranjeiro
On Mon, Jan 04, 2016 at 02:36:14PM +, Ananyev, Konstantin wrote:
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Monday, January 04, 2016 11:38 AM
> > To: Tan, Jianfeng
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if 
> > packet type is set
> > 
> > I'm not sure about the usefulness of this new callback, but one issue I see
> > with rte_eth_dev_get_ptype_info() is that determining the proper size for
> > ptypes[] according to a mask is awkward. For instance suppose
> > RTE_PTYPE_L4_MASK is redefined to a different size at some point, the caller
> > must dynamically adjust its ptypes[] array size to avoid a possible
> > overflow, just in case.
> > 
> > I suggest one of these solutions:
> > 
> > - A callback to query for a single type at once instead (easiest method in
> >   my opinion).
> > 
> > - An additional argument with the number of entries in ptypes[], in which
> >   case rte_eth_dev_get_ptype_info() should return the number of entries that
> >   would have been filled regardless, a bit like snprintf().
> 
> +1 for the second option.
> Also not sure you really need: RTE_PTYPE_*_MAX_NUM macros.
> Konstantin

+1 for the second option.  But see below.

> > 
> > On Thu, Dec 31, 2015 at 02:53:08PM +0800, Jianfeng Tan wrote:
> > > Add a new API rte_eth_dev_get_ptype_info to query what/if packet type will
> > > be set by current rx burst function.
> > >
> > > Signed-off-by: Jianfeng Tan 
> > > ---
> > >  lib/librte_ether/rte_ethdev.c | 12 
> > >  lib/librte_ether/rte_ethdev.h | 22 ++
> > >  lib/librte_mbuf/rte_mbuf.h| 13 +
> > >  3 files changed, 47 insertions(+)
> > >
> > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> > > index ed971b4..1885374 100644
> > > --- a/lib/librte_ether/rte_ethdev.c
> > > +++ b/lib/librte_ether/rte_ethdev.c
> > > @@ -1614,6 +1614,18 @@ rte_eth_dev_info_get(uint8_t port_id, struct 
> > > rte_eth_dev_info *dev_info)
> > >   dev_info->driver_name = dev->data->drv_name;
> > >  }
> > >
> > > +int
> > > +rte_eth_dev_get_ptype_info(uint8_t port_id, uint32_t ptype_mask,
> > > + uint32_t ptypes[])
> > > +{
> > > + struct rte_eth_dev *dev;
> > > +
> > > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > > + dev = _eth_devices[port_id];
> > > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_ptype_info_get, -ENOTSUP);
> > > + return (*dev->dev_ops->dev_ptype_info_get)(dev, ptype_mask, ptypes);
> > > +}
> > > +
> > >  void
> > >  rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr)
> > >  {
> > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > > index bada8ad..e97b632 100644
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -1021,6 +1021,10 @@ typedef void (*eth_dev_infos_get_t)(struct 
> > > rte_eth_dev *dev,
> > >   struct rte_eth_dev_info *dev_info);
> > >  /**< @internal Get specific informations of an Ethernet device. */
> > >
> > > +typedef int (*eth_dev_ptype_info_get_t)(struct rte_eth_dev *dev,
> > > + uint32_t ptype_mask, uint32_t ptypes[]);
> > > +/**< @internal Get ptype info of eth_rx_burst_t. */
> > > +
> > >  typedef int (*eth_queue_start_t)(struct rte_eth_dev *dev,
> > >   uint16_t queue_id);
> > >  /**< @internal Start rx and tx of a queue of an Ethernet device. */
> > > @@ -1347,6 +1351,7 @@ struct eth_dev_ops {
> > >   eth_queue_stats_mapping_set_t queue_stats_mapping_set;
> > >   /**< Configure per queue stat counter mapping. */
> > >   eth_dev_infos_get_tdev_infos_get; /**< Get device info. */
> > > + eth_dev_ptype_info_get_t   dev_ptype_info_get; /** Get ptype info */
> > >   mtu_set_t  mtu_set; /**< Set MTU. */
> > >   vlan_filter_set_t  vlan_filter_set;  /**< Filter VLAN Setup. */
> > >   vlan_tpid_set_tvlan_tpid_set;  /**< Outer VLAN TPID 
> > > Setup. */
> > > @@ -2273,6 +2278,23 @@ extern void rte_eth_dev_info_get(uint8_t port_id,
> > >struct rte_eth_dev_info *dev_info);
> > >
> > >  /**
> > > + * Retrieve the contextual information of an Ethernet device.
> > > + *
> > > + * @param port_id
> > > + *   The port identifier of the Ethernet device.
> > > + * @param ptype_mask
> > > + *   A hint of what kind of packet type which the caller is interested in
> > > + * @param ptypes
> > > + *   An array of packet types to be filled with
> > > + * @return
> > > + *   - (>=0) if successful. Indicate number of valid values in ptypes 
> > > array.
> > > + *   - (-ENOTSUP) if hardware-assisted VLAN stripping not configured.
> > > + *   - (-ENODEV) if *port_id* invalid.
> > > + */
> > > +extern int rte_eth_dev_get_ptype_info(uint8_t port_id,
> > > +  uint32_t ptype_mask, uint32_t ptypes[]);
> > > +
> > > +/**
> > >   

[dpdk-dev] [PATCH] cmdline: increase commandline buffer size to 512

2015-11-09 Thread Nélio Laranjeiro
On Mon, Nov 09, 2015 at 11:45:04AM +, Mcnamara, John wrote:
> 
> 
> > -Original Message-
> > From: N?lio Laranjeiro [mailto:nelio.laranjeiro at 6wind.com]
> > Sent: Thursday, November 5, 2015 4:51 PM
> > To: Mcnamara, John; Olivier Matz; Thomas Monjalon
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] cmdline: increase commandline buffer size
> > to 512
> > 
> > On Thu, Nov 05, 2015 at 09:58:36AM +, John McNamara wrote:
> > > Increase commandline buffer size to 512 to allow for longer
> > > commandlines required by testpmd when dealing with flow director and
> > > IPv6 addresses.
> > >
> > > ...
> > >
> > >  /* configuration */
> > > -#define RDLINE_BUF_SIZE 256
> > > +#define RDLINE_BUF_SIZE 512
> > >
> > 
> > I have faced the same issue.  For some commands this value is still too
> > low.  For example,  RETA update can not be used, the command line limits
> > he configuration to 34 entries.
> > 
> > See http://dpdk.org/ml/archives/dev/2015-October/024681.html
> > and particularly
> > http://dpdk.org/ml/archives/dev/2015-October/024682.html
> > 
> > It was too late to announce an ABI change so we left out the cmdline
> > size update.
> > 
> > Olivier, Thomas, what do you think about this?  It seems this is really
> > blocking.
> 
> Hi,
> 
> Since the change breaks ABI I'll self nack.
> 
> However, this is something that should be fixed since it will affect any DPDK 
> application with a long commandline.
> 
> I can put a warning of future ABI breakage in the release notes but I see 
> that there are other #defines that you need to increase in your patchset so 
> do you want to submit the ABI deprecation notice?
> 
> John.
> -- 

Hi John,

I will do it, I have another depreciation related to RETA.

Thanks.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH] cmdline: increase commandline buffer size to 512

2015-11-05 Thread Nélio Laranjeiro
On Thu, Nov 05, 2015 at 09:58:36AM +, John McNamara wrote:
> Increase commandline buffer size to 512 to allow for longer
> commandlines required by testpmd when dealing with flow
> director and IPv6 addresses.
> 
> Signed-off-by: John McNamara 
> ---
>  lib/librte_cmdline/cmdline_rdline.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cmdline/cmdline_rdline.h 
> b/lib/librte_cmdline/cmdline_rdline.h
> index b9aad9b..72e2dad 100644
> --- a/lib/librte_cmdline/cmdline_rdline.h
> +++ b/lib/librte_cmdline/cmdline_rdline.h
> @@ -93,7 +93,7 @@ extern "C" {
>  #endif
>  
>  /* configuration */
> -#define RDLINE_BUF_SIZE 256
> +#define RDLINE_BUF_SIZE 512
>  #define RDLINE_PROMPT_SIZE  32
>  #define RDLINE_VT100_BUF_SIZE  8
>  #define RDLINE_HISTORY_BUF_SIZE BUFSIZ
> -- 
> 1.8.1.4
> 

I have faced the same issue.  For some commands this value is still too 
low.  For example,  RETA update can not be used, the command line limits 
he configuration to 34 entries.

See http://dpdk.org/ml/archives/dev/2015-October/024681.html
and particularly 
http://dpdk.org/ml/archives/dev/2015-October/024682.html

It was too late to announce an ABI change so we left out the cmdline 
size update.

Olivier, Thomas, what do you think about this?  It seems this is really 
blocking.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] RSS API discussion

2015-10-16 Thread Nélio Laranjeiro
On Fri, Oct 16, 2015 at 01:40:00PM +, Lu, Wenzhuo wrote:
> Hi N?lio,
>  
> > Hi Wenzhuo,
> > 
> > We should discuss about this API for a future release of DPDK because this 
> > one
> > lacks in flexibility.  Some other NICs have indirection tables with a
> > different/configurable size, and the current API does not help to manage it.
> > 
> > For ConnectX-4 I have made a lot of hacks to avoid changing the DPDK API,
> > "[dpdk-dev] [PATCH 0/3] Add RETA configuration to MLX5".
> > http://dpdk.org/ml/archives/dev/2015-October/024681.html
> > 
> > From a user point of view, to update the RETA table, the API expects the 
> > user to
> > know the size of it to update or query.  With your patchset, Intel have two
> > indirection table sizes now, with Mellanox ConnectX-4, I fixed to the size 
> > of 512
> > entries because it is not fixed by default.
> > 
> > How about discussing this in a separate thread?
> Sure, I changed the tittle.
> But I'm afraid I don't catch up with you. I have 2 questions, 1, Why the reta 
> size cannot be predict on  Mellanox ConnectX-4? 2, I don't understand the 
> meaning when you say the reta size is not fixed by default on Mellanox 
> ConnectX-4.
> Would you like to give more details of the Mellanox ConnectX-4's behavior? 
> It'll be helpful to understand this problem.

On Connexct-4 we can configure the indirection table size with any power
of two up to 512 entries.

The current API only expects RETA tables size to be a multiple of
RTE_RETA_GROUP_SIZE i.e. 64.

To handle any kind of NICs, a simple array of N (RETA table size)
elements with the data, instead of something in multiple of 64 would be
easier to use, something like:

 struct rte_eth_rss_reta_entry {
uint16_t idx; /* index to query/update. */
uint16_t reta; /* redirection value. */
 };

We should update the API to take an array of such entries plus its size.
It would also make update and query easier.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC

2015-10-16 Thread Nélio Laranjeiro
On Mon, Sep 28, 2015 at 03:52:27PM +0800, Wenzhuo Lu wrote:
> This patch set implements the RSS enhancement on x550.
> The enhancement includes, the PF RSS redirection table is enlarged
> from 128 entries to 512 entries, the VF doesn't share the same
> registers with PF and per VF RSS redirection table is provided.
>[...] 

Hi Wenzhuo, 

We should discuss about this API for a future release of DPDK because
this one lacks in flexibility.  Some other NICs have indirection tables
with a different/configurable size, and the current API does not help
to manage it.

For ConnectX-4 I have made a lot of hacks to avoid changing the DPDK
API, "[dpdk-dev] [PATCH 0/3] Add RETA configuration to MLX5".
http://dpdk.org/ml/archives/dev/2015-October/024681.html

>From a user point of view, to update the RETA table, the API expects the
user to know the size of it to update or query.  With your patchset,
Intel have two indirection table sizes now, with Mellanox ConnectX-4, I
fixed to the size of 512 entries because it is not fixed by default.

How about discussing this in a separate thread?

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API

2015-10-06 Thread Nélio Laranjeiro
Hi Marc,

On Sun, Oct 04, 2015 at 11:12:46PM +0200, Marc Sune wrote:
>[...]
>  /**
> + * Device supported speeds bitmap flags
> + */
> +#define ETH_LINK_SPEED_AUTONEG   (0 << 0)  /*< Autonegociate 
> (all speeds)  */
> +#define ETH_LINK_SPEED_NO_AUTONEG(1 << 0)  /*< Disable autoneg (fixed 
> speed)  */
> +#define ETH_LINK_SPEED_10M_HD(1 << 1)  /*< 10 Mbps 
> half-duplex */
> +#define ETH_LINK_SPEED_10M   (1 << 2)  /*< 10 Mbps full-duplex */
> +#define ETH_LINK_SPEED_100M_HD   (1 << 3)  /*< 100 Mbps 
> half-duplex */
> +#define ETH_LINK_SPEED_100M  (1 << 4)  /*< 100 Mbps full-duplex */
> +#define ETH_LINK_SPEED_1G(1 << 5)  /*< 1 Gbps */
> +#define ETH_LINK_SPEED_2_5G  (1 << 6)  /*< 2.5 Gbps */
> +#define ETH_LINK_SPEED_5G(1 << 7)  /*< 5 Gbps */
> +#define ETH_LINK_SPEED_10G   (1 << 8)  /*< 10 Mbps */
> +#define ETH_LINK_SPEED_20G   (1 << 9)  /*< 20 Gbps */
> +#define ETH_LINK_SPEED_25G   (1 << 10)  /*< 25 Gbps */
> +#define ETH_LINK_SPEED_40G   (1 << 11)  /*< 40 Gbps */
> +#define ETH_LINK_SPEED_50G   (1 << 12)  /*< 50 Gbps */
> +#define ETH_LINK_SPEED_56G   (1 << 13)  /*< 56 Gbps */
> +#define ETH_LINK_SPEED_100G  (1 << 14)  /*< 100 Gbps */
> +
> +/**
> + * Ethernet numeric link speeds in Mbps
> + */
> +#define ETH_SPEED_NUM_NONE   0  /*< Not defined */
> +#define ETH_SPEED_NUM_10M10 /*< 10 Mbps */
> +#define ETH_SPEED_NUM_100M   100/*< 100 Mbps */
> +#define ETH_SPEED_NUM_1G 1000   /*< 1 Gbps */
> +#define ETH_SPEED_NUM_2_5G   2500   /*< 2.5 Gbps */
> +#define ETH_SPEED_NUM_5G 5000   /*< 5 Gbps */
> +#define ETH_SPEED_NUM_10G1  /*< 10 Mbps */
> +#define ETH_SPEED_NUM_20G2  /*< 20 Gbps */
> +#define ETH_SPEED_NUM_25G25000  /*< 25 Gbps */
> +#define ETH_SPEED_NUM_40G4  /*< 40 Gbps */
> +#define ETH_SPEED_NUM_50G5  /*< 50 Gbps */
> +#define ETH_SPEED_NUM_56G56000  /*< 56 Gbps */
> +#define ETH_SPEED_NUM_100G   10 /*< 100 Gbps */
> +
> +/**
>   * A structure used to retrieve link-level information of an Ethernet port.
>   */
>  struct rte_eth_link {
> - uint16_t link_speed;  /**< ETH_LINK_SPEED_[10, 100, 1000, 1] */
> - uint16_t link_duplex; /**< ETH_LINK_[HALF_DUPLEX, FULL_DUPLEX] */
> - uint8_t  link_status : 1; /**< 1 -> link up, 0 -> link down */
> -}__attribute__((aligned(8))); /**< aligned for atomic64 read/write */
> -
> -#define ETH_LINK_SPEED_AUTONEG  0   /**< Auto-negotiate link speed. */
> -#define ETH_LINK_SPEED_10   10  /**< 10 megabits/second. */
> -#define ETH_LINK_SPEED_100  100 /**< 100 megabits/second. */
> -#define ETH_LINK_SPEED_1000 1000/**< 1 gigabits/second. */
> -#define ETH_LINK_SPEED_11   /**< 10 gigabits/second. */
> -#define ETH_LINK_SPEED_10G  1   /**< alias of 10 gigabits/second. */
> -#define ETH_LINK_SPEED_20G  2   /**< 20 gigabits/second. */
> -#define ETH_LINK_SPEED_40G  4   /**< 40 gigabits/second. */
> + uint32_t link_speed;   /**< Link speed (ETH_SPEED_NUM_) */
> + uint16_t link_duplex;  /**< 1 -> full duplex, 0 -> half duplex */
> + uint8_t link_autoneg : 1;  /**< 1 -> link speed has been autoneg */
> + uint8_t link_status  : 1;  /**< 1 -> link up, 0 -> link down */
> +} __attribute__((aligned(8)));  /**< aligned for atomic64 read/write */
>[...]

Pretty good.  One question, why did you not merge link_duplex, autoneg,
and status like:

struct rte_eth_link {
uint32_t link_speed;
uint32_t link_duplex:1;
uint32_t link_autoneg:1;
uint32_t link_status:1;
};

is it really useful to keep a uint16_t for the duplex alone?

Another point, the comment about link_duplex field should point to the
defines you have changed i.e. ETH_LINK_HALF_DUPLEX, ETH_LINK_FULL_DUPLEX.

Regards,

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap

2015-09-15 Thread Nélio Laranjeiro
On Tue, Sep 15, 2015 at 12:50:11AM +0200, Morten Br?rup wrote:
> Comments inline, marked MB>.
> 
> Med venlig hilsen / kind regards
> - Morten Br?rup
> 
> Marc Sune  on 14. september 2015 23:34 wrote:
> 
> 2015-09-14 12:52 GMT+02:00 Morten Br?rup :
> > It is important to consider that a multipath link (bonding etc.) is not a 
> > physical link, but a logical link (built on top of multiple physical 
> > links). Regardless whether it is a Layer2 link aggregate (IEEE 802.1ad, 
> > Ethernet bonding, EtherChannel, DSL pair bonding, etc.) or a Layer3 
> > multipath link (e.g. simultaneously using Wi-Fi and mobile networks). So it 
> > doesn't make sense trying to impose physical link properties on a purely 
> > logical link. Likewise, it doesn't make sense to impose logical link 
> > properties on physical links. In other words: Don't consider bonding or any 
> > other logical link types when designing the PHY API.
> 
> +1

+1.

> ?
> 
> > I think there is consensus that 1/ (PHY capabilities) and 2/ (PHY 
> > advertisements) should use the same definitions, specifically a bitmap 
> > field. And when you disregard bonding, I don't see any reason to use 
> > different definitions for 3/ (PHY negotiation result). This makes it one 
> > unified API for all three purposes.
> 
> Agree.

I don't agree with this one, some PMDs don't use the advertise of
autoneg result to get the speed or the duplex.  You make a
generality from your case above all PMDs.

Mellanox get the speed, duplex and status information from IOCTLs
which are not related to your bitmap.  So at least for this PMD, there
is already a conversion from 3 fields to a bitmap, knowing that it will
use the speed as an integer after.  What is the benefit of your solution?

> > Nelio suggested adding a support function to convert the bitmap field to a 
> > speed value as an integer. I strongly support this, because you cannot 
> > expect the bitmap to be ordered by speed. 
> 
> Agree with Nelio This is useful.

It was exactly the extreme opposite, a function which takes a
rte_eth_link to a bitmap i.e. speed_to_bm (rte_eth_link link) because,
the speed is mostly used as an integer and not some kind of bitmap.

> > This support function will be able to determine which speed is higher when 
> > exotic speeds are added to the bitmap. Please extend this conversion 
> > function to give three output parameters: speed, full/half duplex, auto 
> > negotiation/non-auto negotiation, or add two separate functions to get the 
> > duplex and auto-negotiation.
> 
> Since, Full/Half duplex is for legacy 10/100Mbps only (afaik), I have my 
> doubts on using a bit for all speeds. I would suggest to define (unroll) 100M 
> (or 100M_FD) and 100M_HD, and the same 10Mbps/1gbps, as Thomas was suggesting 
> some mails ago.
> 
> This was done in v4 (implicitely 100M == 100M_FD). See below.
> ?
> MB> I didn't intend two bits to be allocated in the bitmap for all speeds to 
> support full/half duplex, only for the relevant speeds. Since full duplex is 
> dominant, I agree with the previous decision (originally suggested by Thomas, 
> I think) to make full duplex implicit unless half duplex is explicitly 
> specified. E.g. 10M_HD, 10M (alias 10M_FD), 100M_HD, 100M (alias 100M_FD), 
> 1000M (or 1G), 2500M, 10G, 40G, 100G, etc.
> 
> 
> > I haven't read the suggested code, but there should be some means in 2/ 
> > (advertisements) to disable auto negotiation, e.g. a single bit in the 
> > bitmap to indicate if the speed/duplex-indicating bits in the bitmap means 
> > forced speed/duplex (in which case only a single speed/duplex-bit should be 
> > set) or auto negotiation advertised speed/duplex (in which case multiple 
> > speed/duplex-bits can be set). 
> 
> Agree.
> 
> v3/4 of this patch adds the bitmap in the advertised, as per discussed, to 
> select a group of speeds This is not implemented by drivers yet (!).
> 
> So, as of v4 of this patch, there could be: a) autoneg any supported speed 
> (=> bitmap == 0) b) autoneg over group of speeds (=> more than one bit set in 
> the bitmap) c) forced speed (one and only one set in the bitmap).
> 
> I think this is precisely what you meant + b) as a bonus
> 
> MB> This was not what I meant, but it wasn't very clearly written, so I'll 
> try again: Add an additional single bit "NO_AUTONEG" (or whatever you want to 
> name it) to the 2/ (advertisements) bitmap that explicitly turns off auto 
> negotiation and tries to force the selected speed/duplex (i.e. only one other 
> bit can be set in the bitmap when the NO_AUTONEG bit is set). Your c) makes 
> it impossible to use auto negotiation to advertise a specific speed/duplex, 
> e.g. 100M_FD. My suggested NO_AUTONEG bit can also be used in 3/ (result) to 
> indicate that the speed was a result of Parallel Detection, i.e. that auto 
> negotiation failed or was disabled in either end of the link.
> 
> MB> However, I like your suggestion a).
> 
> ?
> > And some means in 3/ (result) and maybe 2/ 

[dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap

2015-09-14 Thread Nélio Laranjeiro
On Sun, Sep 13, 2015 at 11:18:33PM +0200, Thomas Monjalon wrote:
> 2015-09-13 21:14, Marc Sune:
> > 2015-09-09 15:33 GMT+02:00 Thomas Monjalon :
> > > 2015-09-09 15:10, N?lio Laranjeiro:
> > > > I think V2 is better, maybe you can add a function to convert a single
> > > > bitmap value to the equivalent integer and get rid of ETH_SPEED_XXX
> > > macros.
> > > >
> > > > Thomas what is your opinion?
> > >
> > > Your proposal looks good Nelio.
> > 
> > I am confused, specially since you were the one advocating for having a
> > unified set of constants for speeds (discussion in v2).
> 
> Yes, my first thought was advocating an unification between capabilities and
> negotiated link properties.
> After I was convinced by Nelio's arguments: bitmap is good for capabilities
> (especially to describe every capabilities in one field) but integer is better
> for negotiated speed (especially for aggregated links).
> Converting bitmap speed into integer should be easy to implement in a 
> function.
> 
> > In any case, as I see it, if we want to address the comments of  M. Brorup:
> > 
> > http://comments.gmane.org/gmane.comp.networking.dpdk.devel/19664

I read it.

> > 
> > we need bitmaps for rte_eth_conf link_speed to set the advertised speeds.
> 
> Yes I forgot this interesting comment. It is saying we need
>   1/ capabilities
>   2/ advertised modes (for auto-negotiation or fixed config)
>   3/ negotiated mode
> Previously we were focused only on 1/ and 3/.
> 2/ was only limited to a mode configured without negotiation and was using the
> same field as 3/.
> Maybe we should really have 3 different fields. 1/ and 2/ would use a bitmap.
> 
> > Let me know if you really want to come back to v2 or not.
> 
> It needs more discussion. What do you think of the above proposal?
> What is the opinion of Nelio? Morten?

I agree with Morten, and with this proposition (we should be possible to
disable some capabilities in the advertise field).

For that we should have those 3 fields:
  1/ Capability (as a bitmap),  necessary for the user to configure the
 behavior of the PHY.
  2/ Advertise (as a bitmap) to configure the PHY.
  3/ speed, duplex, status (as rte_eth_link structure), necessary to
 verify that the configuration corresponds to what has been set and
 for other purposes.

I still think Marc needs to go back to V2 and continue from this one.
And as Thomas suggested, he could have a function to get rid of the
double defines and use the sames ones for bitmap and non bitmap fields.

Just for information, before I started this discussion I have worked on
a patch to change the rte_eth_link.link_speed from 16 to 32 bit, I did
not pushed it because, it should be pushed afters Marc's one, and it
will need some rework after that.

-- 
N?lio Laranjeiro
6WIND


[dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap

2015-09-09 Thread Nélio Laranjeiro
Marc,

(making this discussion public again)

On Wed, Sep 09, 2015 at 12:07:01PM +0200, Marc Sune wrote:
> Hi Nelio
> 
> 2015-09-09 11:08 GMT+02:00 N?lio Laranjeiro :
> 
> Marc,
> 
> On Tue, Sep 08, 2015 at 10:24:36PM +0200, Marc Sune wrote:
> > Neilo,
> >
> > 2015-09-08 12:03 GMT+02:00 N?lio Laranjeiro  6wind.com>:
> >
> >? ? ?On Mon, Sep 07, 2015 at 10:52:53PM +0200, Marc Sune wrote:
> >? ? ?> 2015-08-29 2:16 GMT+02:00 Marc Sune :
> >? ? ?>
> >? ? ?> > The current rte_eth_dev_info abstraction does not provide any
> mechanism
> >? ? ?to
> >? ? ?> > get the supported speed(s) of an ethdev.
> >? ? ?> >
> >? ? ?> > For some drivers (e.g. ixgbe), an educated guess can be done
> based on
> >? ? ?the
> >? ? ?> > driver's name (driver_name in rte_eth_dev_info), see:
> >? ? ?> >
> >? ? ?> > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> >? ? ?> >
> >? ? ?> > However, i) doing string comparisons is annoying, and can
> silently
> >? ? ?> > break existing applications if PMDs change their names ii) it
> does not
> >? ? ?> > provide all the supported capabilities of the ethdev iii) for
> some
> >? ? ?drivers
> >? ? ?> > it
> >? ? ?> > is impossible determine correctly the (max) speed by the
> application
> >? ? ?> > (e.g. in i40, distinguish between XL710 and X710).
> >? ? ?> >
> >? ? ?> > This small patch adds speed_capa bitmap in rte_eth_dev_info,
> which is
> >? ? ?> > filled
> >? ? ?> > by the PMDs according to the physical device capabilities.
> >? ? ?> >
> >? ? ?> > v2: rebase, converted speed_capa into 32 bits bitmap, fixed
> alignment
> >? ? ?> > (checkpatch).
> >? ? ?> >
> >? ? ?> > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP 
> into
> >? ? ?> > ETH_SPEED.
> >? ? ?> >? ? ?Converted field speed in struct rte_eth_conf to speeds, to
> allow a
> >? ? ?> > bitmap
> >? ? ?> >? ? ?for defining the announced speeds, as suggested by M. 
> Brorup.
> Fixed
> >? ? ?> >? ? ?spelling issues.
> >? ? ?> >
> >? ? ?> > v4: fixed errata in the documentation of field speeds of
> rte_eth_conf,
> >? ? ?and
> >? ? ?> >? ? ?commit 1/2 message. rebased to v2.1.0. v3 was incorrectly
> based on
> >? ? ?> >? ? ?~2.1.0-rc1.
> >? ? ?> >
> >? ? ?>
> >? ? ?> Thomas,
> >? ? ?>
> >? ? ?> Since mostly you were commenting for v1 and v2; any opinion on 
> this
> one?
> >? ? ?>
> >? ? ?> Regards
> >? ? ?> marc
> >
> >? ? ?Hi Marc,
> >
> >? ? ?I have read your patches, and there are a few mistakes, for instance
> mlx4
> >? ? ?(ConnectX-3 devices) does not support 100Gbps.
> >
> >
> > When I circulated v1 and v2 I was kindly asking maintainers and 
> reviewers
> of
> > the drivers to fix any mistakes in SPEED capabilities, since I was 
> taking
> the
> > speeds from the online websites Some were fixed, but
> apparently
> > some were still missing. I will remove 100Gbps. Please circulate any
> other
> > error you have spotted.
> 
> From Mellanox website:
> ?- ConnectX-3 EN: 10/40/56Gb/s
> ?- ConnectX-3 Pro EN 10GBASE-T: 10G/s
> ?- ConnectX-3 Pro: EN 10/40/56GbE
> ?- ConnectX-3 Pro Programmable: 10/40Gb/s
> 
> This PMD works with any of the ConnectX-3 adapters, so the announce speed
> should be 10/40/56Gb/s.
>
> 
> 
> I will change this
> ?
> 
> >? ? ?In addition, it seems your new bitmap does not support all kind of
> >? ? ?speeds, take a look at the header of Ethtool, in the Linux kernel
> >? ? ?(include/uapi/linux/ethtool.h) which already consumes 30bits without
> even
> >? ? ?managing speeds above 56Gbps.
> >
> >
> > The bitmaps you are referring is SUPPORTED_ and ADVERTISED_. These
> bitmaps not
> > only contain the speeds but PHY properties (e.g. BASE for ETH).
> >
> > The intention of this patch was to expose speed capabilities, similar to
> the
> > bitmap SPEED_ in include/uapi/linux/ethtool.h, which as you see maps
> closely to
> > ETH_SPEED_ proposed in this patch.
> >
> > I think the encoding of other things, like the exact model of the
> interface and
> > its PHY details should go somewhere else. But I might be wrong here, so
> open to
> > hear opinions.
> 
> I understand the need to have capability fields, but I don't understand
> why you want to mix speeds and duplex mode in something which was
> previously only handling speeds.
> 
> 
> Please refer to the comments from Thomas. He was arguing the duplicity in
> speeds between link and capabilities was not necessary, hence patch v3 and 4
> are unifying. The reason why there is only 100 and 100_HD is because of that
> and the "solution" Thomas was proposing.
>
> I was originally doing as you suggested, 

[dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability

2015-09-09 Thread Nélio Laranjeiro
bitmap
Reply-To: 

Shern , Adrien
Mazarguil 
Bcc: 
Subject: Re: [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap
Reply-To: 
In-Reply-To: <20150909090855.GC17463 at autoinstall.dev.6wind.com>

Marc,

On Tue, Sep 08, 2015 at 10:24:36PM +0200, Marc Sune wrote:
> Neilo,
> 
> 2015-09-08 12:03 GMT+02:00 N?lio Laranjeiro :
> 
> On Mon, Sep 07, 2015 at 10:52:53PM +0200, Marc Sune wrote:
> > 2015-08-29 2:16 GMT+02:00 Marc Sune :
> >
> > > The current rte_eth_dev_info abstraction does not provide any 
> mechanism
> to
> > > get the supported speed(s) of an ethdev.
> > >
> > > For some drivers (e.g. ixgbe), an educated guess can be done based on
> the
> > > driver's name (driver_name in rte_eth_dev_info), see:
> > >
> > > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> > >
> > > However, i) doing string comparisons is annoying, and can silently
> > > break existing applications if PMDs change their names ii) it does not
> > > provide all the supported capabilities of the ethdev iii) for some
> drivers
> > > it
> > > is impossible determine correctly the (max) speed by the application
> > > (e.g. in i40, distinguish between XL710 and X710).
> > >
> > > This small patch adds speed_capa bitmap in rte_eth_dev_info, which is
> > > filled
> > > by the PMDs according to the physical device capabilities.
> > >
> > > v2: rebase, converted speed_capa into 32 bits bitmap, fixed alignment
> > > (checkpatch).
> > >
> > > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP into
> > > ETH_SPEED.
> > >? ? ?Converted field speed in struct rte_eth_conf to speeds, to allow a
> > > bitmap
> > >? ? ?for defining the announced speeds, as suggested by M. Brorup. 
> Fixed
> > >? ? ?spelling issues.
> > >
> > > v4: fixed errata in the documentation of field speeds of rte_eth_conf,
> and
> > >? ? ?commit 1/2 message. rebased to v2.1.0. v3 was incorrectly based on
> > >? ? ?~2.1.0-rc1.
> > >
> >
> > Thomas,
> >
> > Since mostly you were commenting for v1 and v2; any opinion on this one?
> >
> > Regards
> > marc
> 
> Hi Marc,
> 
> I have read your patches, and there are a few mistakes, for instance mlx4
> (ConnectX-3 devices) does not support 100Gbps.
> 
> 
> When I circulated v1 and v2 I was kindly asking maintainers and reviewers of
> the drivers to fix any mistakes in SPEED capabilities, since I was taking the
> speeds from the online websites Some were fixed, but apparently
> some were still missing. I will remove 100Gbps. Please circulate any other
> error you have spotted.

>From Mellanox website:
 - ConnectX-3 EN: 10/40/56Gb/s
 - ConnectX-3 Pro EN 10GBASE-T: 10G/s
 - ConnectX-3 Pro: EN 10/40/56GbE
 - ConnectX-3 Pro Programmable: 10/40Gb/s 

This PMD works with any of the ConnectX-3 adapters, so the announce speed
should be 10/40/56Gb/s.

> In addition, it seems your new bitmap does not support all kind of
> speeds, take a look at the header of Ethtool, in the Linux kernel
> (include/uapi/linux/ethtool.h) which already consumes 30bits without even
> managing speeds above 56Gbps.
> 
> 
> The bitmaps you are referring is SUPPORTED_ and ADVERTISED_. These bitmaps not
> only contain the speeds but PHY properties (e.g. BASE for ETH).
> 
> The intention of this patch was to expose speed capabilities, similar to the
> bitmap SPEED_ in include/uapi/linux/ethtool.h, which as you see maps closely 
> to
> ETH_SPEED_ proposed in this patch.
> 
> I think the encoding of other things, like the exact model of the interface 
> and
> its PHY details should go somewhere else. But I might be wrong here, so open 
> to
> hear opinions.

I understand the need to have capability fields, but I don't understand
why you want to mix speeds and duplex mode in something which was
previously only handling speeds.

We now have redundant information in struct rte_eth_conf, whereas
that structure has a speed field which embeds the duplex mode and
a duplex field which does the same, which one should be used? 

> It would be nice to keep the field to represent the real speed of the
> link, in case it is not represented by the bitmap, it could be also
> useful for aggregated links (bonding for instance).? The current API
> already works this way, it just needs to be extended from 16 to 32 bit
> to manage speed above 64Gbps.
> 
> 
> This patch does not remove rte_eth_link_get() API. It just changes the 
> encoding
> of speed in struct rte_eth_link, to have an homogeneous set of constants with
> the speed capabilities bitmap, as discussed previously in the thread (see
> Thomas comments). IOW, it returns now a single SPEED_ value in the struct
> rte_eth_link's link_speed field.

You change the coding of the speed field, but applications still expect
an integer, see port_infos_display function in 

[dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap

2015-09-08 Thread Nélio Laranjeiro
On Mon, Sep 07, 2015 at 10:52:53PM +0200, Marc Sune wrote:
> 2015-08-29 2:16 GMT+02:00 Marc Sune :
> 
> > The current rte_eth_dev_info abstraction does not provide any mechanism to
> > get the supported speed(s) of an ethdev.
> >
> > For some drivers (e.g. ixgbe), an educated guess can be done based on the
> > driver's name (driver_name in rte_eth_dev_info), see:
> >
> > http://dpdk.org/ml/archives/dev/2013-August/000412.html
> >
> > However, i) doing string comparisons is annoying, and can silently
> > break existing applications if PMDs change their names ii) it does not
> > provide all the supported capabilities of the ethdev iii) for some drivers
> > it
> > is impossible determine correctly the (max) speed by the application
> > (e.g. in i40, distinguish between XL710 and X710).
> >
> > This small patch adds speed_capa bitmap in rte_eth_dev_info, which is
> > filled
> > by the PMDs according to the physical device capabilities.
> >
> > v2: rebase, converted speed_capa into 32 bits bitmap, fixed alignment
> > (checkpatch).
> >
> > v3: rebase to v2.1. unified ETH_LINK_SPEED and ETH_SPEED_CAP into
> > ETH_SPEED.
> > Converted field speed in struct rte_eth_conf to speeds, to allow a
> > bitmap
> > for defining the announced speeds, as suggested by M. Brorup. Fixed
> > spelling issues.
> >
> > v4: fixed errata in the documentation of field speeds of rte_eth_conf, and
> > commit 1/2 message. rebased to v2.1.0. v3 was incorrectly based on
> > ~2.1.0-rc1.
> >
> 
> Thomas,
> 
> Since mostly you were commenting for v1 and v2; any opinion on this one?
> 
> Regards
> marc

Hi Marc,

I have read your patches, and there are a few mistakes, for instance mlx4
(ConnectX-3 devices) does not support 100Gbps.

In addition, it seems your new bitmap does not support all kind of
speeds, take a look at the header of Ethtool, in the Linux kernel
(include/uapi/linux/ethtool.h) which already consumes 30bits without even
managing speeds above 56Gbps.  

It would be nice to keep the field to represent the real speed of the
link, in case it is not represented by the bitmap, it could be also
useful for aggregated links (bonding for instance).  The current API
already works this way, it just needs to be extended from 16 to 32 bit
to manage speed above 64Gbps.

>[...]

N?lio
-- 
N?lio Laranjeiro
6WIND