Re: [dpdk-dev] [PATCH v2 00/55] Solarflare libefx-based PMD

[Ferruh Yigit]

ich may be used
>   net/sfc: validate Rx queue buffers setup
>   net/sfc: implement Rx queue start and stop operations
>   net/sfc: implement device callback to Rx burst of packets
>   net/sfc: discard scattered packet on Rx correctly
> 
> Artem Andreev (2):
>   net/sfc: include libefx in build
>   net/sfc: implement device operation to retrieve link info
> 
> Ivan Malov (5):
>   net/sfc: provide basic stubs for Tx subsystem
>   net/sfc: add function to check configured Tx mode
>   net/sfc: add callbacks to set up and release Tx queues
>   net/sfc: implement transmit path start / stop
>   net/sfc: add callback to send bursts of packets
> 

Series Reviewed-by: Ferruh Yigit <ferruh.yi...@intel.com>

Re: [dpdk-dev] [PATCH 30/31] net/i40e: support Linux VF to configure IRQ link list

[Ferruh Yigit]

On 12/2/2016 12:12 AM, Wenzhuo Lu wrote:
> i40e PF host only support to work with DPDK VF driver, Linux
> VF driver is not supported. This change will enhance in
> configuring IRQ link list.
> 
> This Change will identify VF client by number of vector
> requested. DPDK VF will ask only single one while Linux VF
> will request at least 2. It will have different configuration
> for different clients. DPDK VF will be configured to link all
> queue together, while Linux VF will be configured per request.
> 
> Signed-off-by: Chen Jing D(Mark) 
> ---
>  drivers/net/i40e/i40e_pf.c | 151 
> +
>  1 file changed, 138 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> index 1ad5ed1..4b0da75 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -585,14 +585,116 @@
>   return ret;
>  }
>  
> +static void
> +i40e_pf_config_irq_link_list(struct i40e_pf_vf *vf,
> +  struct i40e_virtchnl_vector_map *vvm)
> +{
> + uint64_t linklistmap = 0, tempmap;
> + struct i40e_hw *hw = I40E_PF_TO_HW(vf->pf);
> + uint16_t qid;
> + bool b_first_q = true;
> + enum i40e_queue_type qtype;
> + uint16_t vector_id;
> + uint32_t reg, reg_idx;
> + uint16_t itr_idx = 0, i;
> +
> + vector_id = vvm->vector_id;
> + /* setup the head */
> + if (!vector_id)
> + reg_idx = I40E_VPINT_LNKLST0(vf->vf_idx);
> + else
> + reg_idx = I40E_VPINT_LNKLSTN(
> + ((hw->func_caps.num_msix_vectors_vf - 1) * vf->vf_idx)
> + + (vector_id - 1));
> +
> + if (vvm->rxq_map == 0 && vvm->txq_map == 0) {
> + I40E_WRITE_REG(hw, reg_idx,
> +I40E_VPINT_LNKLST0_FIRSTQ_INDX_MASK);
> + goto cfg_irq_done;
> + }
> +
> + /* sort all rx and tx queues */
> + tempmap = vvm->rxq_map;
> + for (i = 0; i < sizeof(vvm->rxq_map) * 8; i++) {
> + if (tempmap & 0x1)
> + linklistmap |= (1 << (2 * i));
> + tempmap >>= 1;
> + }
> +
> + tempmap = vvm->txq_map;
> + for (i = 0; i < sizeof(vvm->txq_map) * 8; i++) {
> + if (tempmap & 0x1)
> + linklistmap |= (1 << (2 * i + 1));
> + tempmap >>= 1;
> + }
> +
> + /* Link all rx and tx queues into a chained list */
> + tempmap = linklistmap;
> + i = 0;
> + b_first_q = true;
> + do {
> + if (tempmap & 0x1) {
> + qtype = i % 2;

This cause ICC compilation error:

.../app/test-pmd/cmdline.c:(.text+0x79d4): undefined reference to
`rte_pmd_i40e_set_vf_vlan_stripq'

Re: [dpdk-dev] [PATCH 12/31] net/i40e: set VF MAC from PF support

[Ferruh Yigit]

On 12/2/2016 12:11 AM, Wenzhuo Lu wrote:
> Support setting VF MAC address from PF.
> User can call the API on PF to set a speific VF's

s/speific/specific

> MAC address.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yi...@intel.com>

<..>

Re: [dpdk-dev] [PATCH 05/31] net/i40e: set TX loopback from PF

[Ferruh Yigit]

On 12/2/2016 12:11 AM, Wenzhuo Lu wrote:
> Support enabling/disabling TX loopback from PF.
> User can call the API on PF to enable/disable TX loopback
> for all the PF and VFs.
> 
> Signed-off-by: Wenzhuo Lu 
> ---
>  drivers/net/i40e/i40e_ethdev.c| 219 
> ++
>  drivers/net/i40e/rte_pmd_i40e.h   |  16 +++
>  drivers/net/i40e/rte_pmd_i40e_version.map |   1 +
>  3 files changed, 236 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index ec863b9..9fe9672 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -9938,3 +9938,222 @@ static void i40e_set_default_mac_addr(struct 
> rte_eth_dev *dev,

<...>

> +static int
> +i40e_vsi_set_tx_loopback(struct i40e_vsi *vsi, uint8_t on)
> +{
> + struct i40e_vsi_context ctxt;
> + struct i40e_hw *hw;
> + int ret;
> +
> + hw = I40E_VSI_TO_HW(vsi);
> +
> + /* Use the FW API if FW >= v5.0 */
> + if (hw->aq.fw_maj_ver < 5) {
> + PMD_INIT_LOG(ERR, "FW < v5.0, cannot enable loopback");
> + return -ENOSYS;

Checkpatch complains about ENOSYS usage:
WARNING:ENOSYS: ENOSYS means 'invalid syscall nr' and nothing else

What is intended error code here?

[dpdk-dev] [PATCH v2] scripts: check cc stable mailing list in commit

[Ferruh Yigit]

On 12/1/2016 1:43 PM, Thomas Monjalon wrote:
> Add a check for commits fixing a released bug.
> Such commits are found thanks to scripts/git-log-fixes.sh.
> They must be sent CC: stable at dpdk.org.
> In order to avoid forgetting CC, this mail header can be written
> in the git commit message.
> 
> Signed-off-by: Thomas Monjalon 

Tested-by: Ferruh Yigit 

[dpdk-dev] [PATCH v2] doc: add pdump library to API doxygen

[Ferruh Yigit]

From: Reshma Pattan 

Add pdump library to API doxygen.

Signed-off-by: Reshma Pattan 
---

v2:
* Move pdump higher position in the index
---
 doc/api/doxy-api-index.md | 1 +
 doc/api/doxy-api.conf | 1 +
 2 files changed, 2 insertions(+)

diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 6675f96..ed1a204 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -136,6 +136,7 @@ There are many libraries, so their headers may be grouped 
by topics:

 - **debug**:
   [jobstats]   (@ref rte_jobstats.h),
+  [pdump]  (@ref rte_pdump.h),
   [hexdump](@ref rte_hexdump.h),
   [debug]  (@ref rte_debug.h),
   [log](@ref rte_log.h),
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index 9dc7ae5..b340fcf 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -51,6 +51,7 @@ INPUT   = doc/api/doxy-api-index.md \
   lib/librte_mempool \
   lib/librte_meter \
   lib/librte_net \
+  lib/librte_pdump \
   lib/librte_pipeline \
   lib/librte_port \
   lib/librte_power \
-- 
2.9.3

[dpdk-dev] [PATCH v12 0/6] add Tx preparation

[Ferruh Yigit]

On 11/30/2016 6:26 PM, Thomas Monjalon wrote:
> 2016-11-30 17:42, Ananyev, Konstantin:
 Please, we need a comment for each driver saying
 "it is OK, we do not need any checksum preparation for TSO"
 or
 "yes we have to implement tx_prepare or TSO will not work in this mode"

>>>
>>> qede PMD doesn?t currently support TSO yet, it only supports Tx TCP/UDP/IP
>>> csum offloads.
>>> So Tx preparation isn?t applicable. So as of now -
>>> "it is OK, we do not need any checksum preparation for TSO"
>>
>> Thanks for the answer.
>> Though please note that it not only for TSO.
> 
> Oh yes, sorry, my wording was incorrect.
> We need to know if any checksum preparation is needed prior
> offloading its final computation to the hardware or driver.
> So the question applies to TSO and simple checksum offload.
> 
> We are still waiting answers for
>   bnxt, cxgbe, ena, nfp, thunderx, virtio and vmxnet3.
> 

Remaining ones:

ena
nfp
virtio
vmxnet3

[dpdk-dev] apply commit e30a0178d290a4e83dc01f9c2170d4859339c9cf "kni: support RHEL 7.3" to dpdk-stable?

[Ferruh Yigit]

On 11/30/2016 8:54 PM, Roberts, Lee A. wrote:
> Does it make sense to apply the commit for "kni: support RHEL 7.3" 
> (http://www.dpdk.org/browse/dpdk/commit/lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h?id=e30a0178d290a4e83dc01f9c2170d4859339c9cf)
> to the stable tree to enable clean compilation on RHEL 7.3?

Yes, good idea, but these requests should go into stable mail list:
CC: stable at dpdk.org

As far as I can see that patch can be applied to stable tree without
modification, so I guess request is enough, no patch required, but
Yuanhan please correct me if this is wrong.

And 16.07.2 released yesterday, so this patch missed that release, not
sure when next stable release is scheduled.

> 
>  - Lee Roberts
> 

[dpdk-dev] [PATCH v4] net/kni: add KNI PMD

[Ferruh Yigit]

Add KNI PMD which wraps librte_kni for ease of use.

KNI PMD can be used as any regular PMD to send / receive packets to the
Linux networking stack.

Signed-off-by: Ferruh Yigit 
---

v4:
* allow only single queue
* use driver.name as name

v3:
* rebase on top of latest master

v2:
* updated driver name eth_kni -> net_kni
---
 config/common_base  |   1 +
 config/common_linuxapp  |   1 +
 drivers/net/Makefile|   1 +
 drivers/net/kni/Makefile|  63 +
 drivers/net/kni/rte_eth_kni.c   | 462 
 drivers/net/kni/rte_pmd_kni_version.map |   4 +
 mk/rte.app.mk   |  10 +-
 7 files changed, 537 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/kni/Makefile
 create mode 100644 drivers/net/kni/rte_eth_kni.c
 create mode 100644 drivers/net/kni/rte_pmd_kni_version.map

diff --git a/config/common_base b/config/common_base
index 4bff83a..3385879 100644
--- a/config/common_base
+++ b/config/common_base
@@ -543,6 +543,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n
 # Compile librte_kni
 #
 CONFIG_RTE_LIBRTE_KNI=n
+CONFIG_RTE_LIBRTE_PMD_KNI=n
 CONFIG_RTE_KNI_KMOD=n
 CONFIG_RTE_KNI_PREEMPT_DEFAULT=y
 CONFIG_RTE_KNI_VHOST=n
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 2483dfa..2ecd510 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y
 CONFIG_RTE_EAL_VFIO=y
 CONFIG_RTE_KNI_KMOD=y
 CONFIG_RTE_LIBRTE_KNI=y
+CONFIG_RTE_LIBRTE_PMD_KNI=y
 CONFIG_RTE_LIBRTE_VHOST=y
 CONFIG_RTE_LIBRTE_PMD_VHOST=y
 CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index bc93230..c4771cd 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -41,6 +41,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic
 DIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k
 DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e
 DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe
+DIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += kni
 DIRS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4
 DIRS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5
 DIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += mpipe
diff --git a/drivers/net/kni/Makefile b/drivers/net/kni/Makefile
new file mode 100644
index 000..0b7cf91
--- /dev/null
+++ b/drivers/net/kni/Makefile
@@ -0,0 +1,63 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2016 Intel Corporation. All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+# * Redistributions of source code must retain the above copyright
+#   notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+#   notice, this list of conditions and the following disclaimer in
+#   the documentation and/or other materials provided with the
+#   distribution.
+# * Neither the name of Intel Corporation nor the names of its
+#   contributors may be used to endorse or promote products derived
+#   from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_pmd_kni.a
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lpthread
+
+EXPORT_MAP := rte_pmd_kni_version.map
+
+LIBABIVER := 1
+
+#
+# all source are stored in SRCS-y
+#
+SRCS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += rte_eth_kni.c
+
+#
+# Export include files
+#
+SYMLINK-y-include +=
+
+# this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_ether
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_kni
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_mbuf
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_mempool
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
new file mode 100644
index 000..6c4df96
--- /dev/null
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -0,0 +1,462 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Intel Corporation. All rights reserved.
+ *

[dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit

[Ferruh Yigit]

On 11/21/2016 10:43 PM, Thomas Monjalon wrote:
> Add a check for commits fixing a released bug.
> Such commits are found thanks to scripts/git-log-fixes.sh.
> They must be sent CC: stable at dpdk.org.
> In order to avoid forgetting CC, this mail header can be written
> in the git commit message.
> 
> Signed-off-by: Thomas Monjalon 

I think this is useful, thanks for the patch.

> ---
>  scripts/check-git-log.sh | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
> index 5f8a9fc..4f98a7a 100755
> --- a/scripts/check-git-log.sh
> +++ b/scripts/check-git-log.sh
> @@ -47,12 +47,14 @@ if [ "$1" = '-h' -o "$1" = '--help' ] ; then
>   exit
>  fi
>  
> +selfdir=$(dirname $(readlink -e $0))
>  range=${1:-origin/master..}
>  
>  commits=$(git log --format='%h' --reverse $range)
>  headlines=$(git log --format='%s' --reverse $range)
>  bodylines=$(git log --format='%b' --reverse $range)
>  fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut 
> -d' ' -f1)
> +stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d'  | cut -d' 
> ' -f2)

This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
range for git-log-fixes.sh.
Generates warning:
.../scripts/git-log-fixes.sh: illegal option -- 6
usage: git-log-fixes.sh [-h] 

>  tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 
> 'fix.*:')
>  bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'
>  
> @@ -191,3 +193,10 @@ bad=$(for fixtag in $fixtags ; do
>   printf "$fixtag" | grep -v "^$good$"
>  done | sed 's,^,\t,')
>  [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
> +
> +# check CC:stable for fixes
> +bad=$(for fix in $stablefixes ; do
> + git log --format='%b' -1 $fix | grep -qi '^CC: *stable at dpdk.org' ||
> + git log --format='\t%s' -1 $fix
> +done)
> +[ -z "$bad" ] || printf "Should CC: stable at dpdk.org\n$bad\n"

This is good for developer, but since "CC: xx" tags removed when patch
applied, this will generate warnings when run against existing history.

I don't know what can be done for this.

Or should we keep CC: tags in commit log perhaps?

> 

[dpdk-dev] [PATCH v2] net/i40evf: fix casting between structs

[Ferruh Yigit]

On 11/30/2016 2:02 AM, Jingjing Wu wrote:
> Casting from structs which lay out data in typed members
> to structs which have flat memory buffers, will cause
> problems if the alignment of the former isn't as expected.
> This patch removes the casting between structs.
> 
> Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
> Signed-off-by: Jingjing Wu 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH] e1000/base: announce supported NICs

[Ferruh Yigit]

On 11/27/2016 6:11 PM, Wenzhuo Lu wrote:
> Announce the support of I219 NICs. Also add all the
> other supported NICs.
> 
> Add Intel I219 NICs support in release note too.
> 
> Signed-off-by: Wenzhuo Lu 

Commit log and release notes wording updated.

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v2 1/1] net/i40e: enable auto link update for 25G

[Ferruh Yigit]

On 11/29/2016 8:26 PM, Qi Zhang wrote:
> In previous patch for 25G (XXV710) enable
> 75d133dd329: ("net/i40e: enable 25G device"),
> we intend to disable the auto linke update as a work around
> for the issue that link can't be turn on when auto link update
> is enabled. Now we know the root cause, there are interface 
> changes of AQ command "set_phy_config" and "get_phy_capabilities" 
> for 25G. So, this patch remove this limitation.
> 
> Signed-off-by: Qi Zhang 

Commit log updated a little.

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH 1/1] net/i40e: enable auto link update for XXV710

[Ferruh Yigit]

On 11/30/2016 2:26 AM, Zhang, Qi Z wrote:
> Hi Ferruh:
> 
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Wednesday, November 30, 2016 1:46 AM
>> To: Zhang, Qi Z ; Wu, Jingjing > intel.com>;
>> Zhang, Helin 
>> Cc: dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/1] net/i40e: enable auto link update for
>> XXV710
>>
>> Hi Qi,
>>
>> On 11/24/2016 11:43 PM, Qi Zhang wrote:
>>> This patch remove the limitation that XXV710 device does
>>
>> XXV710 is 25G device, and support added in 16.11 (please correct me if this 
>> is
>> wrong.), but I can't find any DPDK documentation for this device.
>>
>> Can you please add some documentation, at least to http://dpdk.org/doc/nics
>> and http://dpdk.org/doc/guides/nics/i40e.html
>> in a different patch?
> 
> For 16.11, since XXV710 is not officially supported, so they are missing in 
> document.
> For 17.02 this will be updated. Thanks for remind.

If officially will be added on 17.02, can you please update release
notes too, to announce new device support?

>>
>>> not support auto link update.
>>
>> Can you please add more details that why we can remove the limitation now?
> Ok, will update in v2.
>>
>>>
>>> Signed-off-by: Qi Zhang 
>>> ---
>>>  drivers/net/i40e/i40e_ethdev.c | 8 
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
>>> b/drivers/net/i40e/i40e_ethdev.c index 67778ba..b7a916d 100644
>>> --- a/drivers/net/i40e/i40e_ethdev.c
>>> +++ b/drivers/net/i40e/i40e_ethdev.c
>>> @@ -1628,6 +1628,8 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>>>
>>> /* use get_phy_abilities_resp value for the rest */
>>> phy_conf.phy_type = phy_ab.phy_type;
>>> +   phy_conf.phy_type_ext = phy_ab.phy_type_ext;
>>> +   phy_conf.fec_config = phy_ab.mod_type_ext;
>>
>> And these changes look like called for all device types, just to double 
>> check, are
>> these 25G specific?
> 
> Actually only XXV710 need this two lines, but base on firmware engineer's 
> input, 
> this could be implemented in generic way since no impact for other i40e 
> devices.
>>
>>> phy_conf.eee_capability = phy_ab.eee_capability;
>>> phy_conf.eeer = phy_ab.eeer_val;
>>> phy_conf.low_power_ctrl = phy_ab.d3_lpan; @@ -1653,8 +1655,7 @@
>>> i40e_apply_link_speed(struct rte_eth_dev *dev)
>>> struct rte_eth_conf *conf = >data->dev_conf;
>>>
>>> speed = i40e_parse_link_speeds(conf->link_speeds);
>>> -   if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
>>> -   abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> +   abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
>>> abilities |= I40E_AQ_PHY_AN_ENABLED;
>>> abilities |= I40E_AQ_PHY_LINK_ENABLED; @@ -1990,8 +1991,7 @@
>>> i40e_dev_set_link_down(struct rte_eth_dev *dev)
>>> uint8_t abilities = 0;
>>> struct i40e_hw *hw =
>> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>>
>>> -   if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
>>> -   abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> +   abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> return i40e_phy_conf_link(hw, abilities, speed);  }
>>>
>>>
> Thanks!
> Qi
> 

[dpdk-dev] [PATCH 1/1] net/i40e: enable auto link update for XXV710

[Ferruh Yigit]

Hi Qi,

On 11/24/2016 11:43 PM, Qi Zhang wrote:
> This patch remove the limitation that XXV710 device does

XXV710 is 25G device, and support added in 16.11 (please correct me if
this is wrong.), but I can't find any DPDK documentation for this device.

Can you please add some documentation, at least to
http://dpdk.org/doc/nics and http://dpdk.org/doc/guides/nics/i40e.html
in a different patch?

> not support auto link update.

Can you please add more details that why we can remove the limitation now?

> 
> Signed-off-by: Qi Zhang 
> ---
>  drivers/net/i40e/i40e_ethdev.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 67778ba..b7a916d 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1628,6 +1628,8 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>  
>   /* use get_phy_abilities_resp value for the rest */
>   phy_conf.phy_type = phy_ab.phy_type;
> + phy_conf.phy_type_ext = phy_ab.phy_type_ext;
> + phy_conf.fec_config = phy_ab.mod_type_ext;

And these changes look like called for all device types, just to double
check, are these 25G specific?

>   phy_conf.eee_capability = phy_ab.eee_capability;
>   phy_conf.eeer = phy_ab.eeer_val;
>   phy_conf.low_power_ctrl = phy_ab.d3_lpan;
> @@ -1653,8 +1655,7 @@ i40e_apply_link_speed(struct rte_eth_dev *dev)
>   struct rte_eth_conf *conf = >data->dev_conf;
>  
>   speed = i40e_parse_link_speeds(conf->link_speeds);
> - if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
> - abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> + abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>   if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
>   abilities |= I40E_AQ_PHY_AN_ENABLED;
>   abilities |= I40E_AQ_PHY_LINK_ENABLED;
> @@ -1990,8 +1991,7 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
>   uint8_t abilities = 0;
>   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  
> - if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
> - abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> + abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>   return i40e_phy_conf_link(hw, abilities, speed);
>  }
>  
> 

[dpdk-dev] [PATCH] i40evf: add set maximum frame size support

[Ferruh Yigit]

On 11/25/2016 8:47 PM, Michael Bieniek wrote:
> This adds the ability to set maximum frame size for an i40e virtual
> interface. This patch is based on the i40e physical function maximum
> frame size implementation. This was tested on an system configured with
> multiple i40e virtual functions. Verified that the MTU was configurable
> and that sending packets greater than the configured MTU resulted in a
> drop.
> 
> Signed-off-by: Michael Bieniek 

Hi Michael,

I guess this is the first patch, welcome to the DPDK community.

> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index aa306d6..8477c98 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -158,6 +158,7 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, 
> uint16_t queue_id);
>  static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
>  uint8_t *msg,
>  uint16_t msglen);
> +static int i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t);

This is not something functional but, in declaration, if you used
parameter name for first argument, let's use parameter name for second too.

>  
>  /* Default hash key buffer for RSS */
>  static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
> @@ -225,6 +226,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
>   .reta_query   = i40evf_dev_rss_reta_query,
>   .rss_hash_update  = i40evf_dev_rss_hash_update,
>   .rss_hash_conf_get= i40evf_dev_rss_hash_conf_get,
> + .mtu_set  = i40evf_dev_mtu_set,
>  };
>  
>  /*
> @@ -2635,3 +2637,34 @@ i40evf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
>  
>   return 0;
>  }
> +
> +static int
> +i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);

hw is not used, and causing a build error.

> + struct rte_eth_dev_data *dev_data = dev->data;
> + uint32_t frame_size = mtu + ETHER_HDR_LEN
> +   + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
> + int ret = 0;
> +
> + /* check if mtu is within the allowed range */
> + if ((mtu < ETHER_MIN_MTU) || (frame_size > I40E_FRAME_SIZE_MAX))
> + return -EINVAL;
> +
> + /* mtu setting is forbidden if port is started */
> + if (dev_data->dev_started) {
> + PMD_DRV_LOG(ERR,
> + "port %d must be stopped before configuration\n",
> + dev_data->port_id);
> + return -EBUSY;
> + }
> +
> + if (frame_size > ETHER_MAX_LEN)
> + dev_data->dev_conf.rxmode.jumbo_frame = 1;
> + else
> + dev_data->dev_conf.rxmode.jumbo_frame = 0;
> +
> + dev_data->dev_conf.rxmode.max_rx_pkt_len = frame_size;
> +
> + return ret;
> +}
> 

[dpdk-dev] [PATCH] net/i40evf: fix casting between structs

[Ferruh Yigit]

On 11/27/2016 9:35 AM, Jingjing Wu wrote:
> Casting from structs which lay out data in typed members
> to structs which have flat memory buffers, will cause
> problems if the alignment of the former isn't as expected.
> This patch removes the casting between structs.
> 
> Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
> Signed-off-by: Jingjing Wu 
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 27 +++
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index aa306d6..53d7c87 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
>   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>   struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>   struct i40e_arq_event_info info;
> - struct i40e_virtchnl_msg *v_msg;
> - uint16_t pending, opcode;
> + uint16_t pending, aq_opc;
> + enum i40e_virtchnl_ops msg_opc;
> + enum i40e_status_code msg_ret;
>   int ret;
>  
>   info.buf_len = I40E_AQ_BUF_SZ;
> @@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
>   return;
>   }
>   info.msg_buf = vf->aq_resp;
> - v_msg = (struct i40e_virtchnl_msg *)
>  
>   pending = 1;
>   while (pending) {
> @@ -1357,32 +1357,35 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
>   "ret: %d", ret);
>   break;
>   }
> - opcode = rte_le_to_cpu_16(info.desc.opcode);
> -
> - switch (opcode) {
> + aq_opc = rte_le_to_cpu_16(info.desc.opcode);
> + msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
> +   info.desc.cookie_high);
> + msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
> +   info.desc.cookie_low);

What do you think commenting cookie_high is opcode and cookie_low is
return_value?

> + switch (aq_opc) {
>   case i40e_aqc_opc_send_msg_to_vf:
> - if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
> + if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
>   /* process event*/
>   i40evf_handle_pf_event(dev, info.msg_buf,
>  info.msg_len);
>   else {
>   /* read message and it's expected one */
> - if (v_msg->v_opcode == vf->pend_cmd) {
> - vf->cmd_retval = v_msg->v_retval;
> + if (msg_opc == vf->pend_cmd) {
> + vf->cmd_retval = msg_ret;
>   /* prevent compiler reordering */
>   rte_compiler_barrier();
>   _clear_cmd(vf);
>   } else
>   PMD_DRV_LOG(ERR, "command mismatch,"
>   "expect %u, get %u",
> - vf->pend_cmd, v_msg->v_opcode);
> + vf->pend_cmd, msg_ret);

s/msg_ret/msg_opc/ ?

>   PMD_DRV_LOG(DEBUG, "adminq response is 
> received,"
> -  " opcode = %d\n", v_msg->v_opcode);
> +  " opcode = %d\n", msg_ret);

s/msg_ret/msg_opc/ ?

>   }
>   break;
>   default:
>   PMD_DRV_LOG(ERR, "Request %u is not supported yet",
> - opcode);
> + aq_opc);
>   break;
>   }
>   }
> 

[dpdk-dev] [PATCH] net/i40e: fix log when check Tx free thresh

[Ferruh Yigit]

On 11/27/2016 9:11 AM, Jingjing Wu wrote:
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> Signed-off-by: Jingjing Wu 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH 00/16] e1000 base code update

[Ferruh Yigit]

Hi Wenzhuo,

On 11/23/2016 5:22 PM, Wenzhuo Lu wrote:
> Updated e1000 base code to fix several bugs and support
> i219 NICs.
> 
> Wenzhuo Lu (16):
>   e1000/base: increased ULP timer
>   e1000/base: increase PHY PLL clock gate timing
>   e1000/base: try more times to get HW mailbox lock
>   e1000/base: add getting HW version support for i354
>   e1000/base: expose e1000_write_vfta_i350
>   e1000/base: add max RX jumbo frame define
>   e1000/base: restore link speed after ULP exit
>   e1000/base: clear ULP configuration register on ULP exit
>   e1000/base: increase LANPHYPC low duration
>   e1000/base: workaround for ULP entry flow
>   e1000/base: enable new i219 devices
>   e1000/base: always request clock during K1 at 1G link speed
>   e1000/base: ability to force K1-off disabled
>   e1000/base: support more i219 devices
>   e1000/base: update readme
>   e1000: add new i219 devices
> 
>  drivers/net/e1000/base/README  |   4 +-
>  drivers/net/e1000/base/e1000_82575.c   |   1 -
>  drivers/net/e1000/base/e1000_82575.h   |   1 +
>  drivers/net/e1000/base/e1000_api.c |  19 +
>  drivers/net/e1000/base/e1000_defines.h |   9 +
>  drivers/net/e1000/base/e1000_hw.h  |  21 +-
>  drivers/net/e1000/base/e1000_ich8lan.c | 865 
> +++--
>  drivers/net/e1000/base/e1000_ich8lan.h |  21 +-
>  drivers/net/e1000/base/e1000_mbx.c |  36 +-
>  drivers/net/e1000/base/e1000_nvm.c |   1 +
>  drivers/net/e1000/base/e1000_regs.h|   7 +
>  drivers/net/e1000/em_ethdev.c  |  34 +-
>  12 files changed, 949 insertions(+), 70 deletions(-)
> 

Based on this pathset.

Can you also please send another patch to:
1- add I219 to supported nics list
2- announce new supported nic in release notes.

Also as far as I can see there is no igb/e1000 documentation under
doc/guides/nics/*, it can good to provide one, not with above requested
patches perhaps, but in some suitable time.

Thanks,
ferruh

[dpdk-dev] [PATCH 00/16] e1000 base code update

[Ferruh Yigit]

On 11/23/2016 5:22 PM, Wenzhuo Lu wrote:
> Updated e1000 base code to fix several bugs and support
> i219 NICs.
> 
> Wenzhuo Lu (16):
>   e1000/base: increased ULP timer
>   e1000/base: increase PHY PLL clock gate timing
>   e1000/base: try more times to get HW mailbox lock
>   e1000/base: add getting HW version support for i354
>   e1000/base: expose e1000_write_vfta_i350
>   e1000/base: add max RX jumbo frame define
>   e1000/base: restore link speed after ULP exit
>   e1000/base: clear ULP configuration register on ULP exit
>   e1000/base: increase LANPHYPC low duration
>   e1000/base: workaround for ULP entry flow
>   e1000/base: enable new i219 devices
>   e1000/base: always request clock during K1 at 1G link speed
>   e1000/base: ability to force K1-off disabled
>   e1000/base: support more i219 devices
>   e1000/base: update readme
>   e1000: add new i219 devices
> 
>  drivers/net/e1000/base/README  |   4 +-
>  drivers/net/e1000/base/e1000_82575.c   |   1 -
>  drivers/net/e1000/base/e1000_82575.h   |   1 +
>  drivers/net/e1000/base/e1000_api.c |  19 +
>  drivers/net/e1000/base/e1000_defines.h |   9 +
>  drivers/net/e1000/base/e1000_hw.h  |  21 +-
>  drivers/net/e1000/base/e1000_ich8lan.c | 865 
> +++--
>  drivers/net/e1000/base/e1000_ich8lan.h |  21 +-
>  drivers/net/e1000/base/e1000_mbx.c |  36 +-
>  drivers/net/e1000/base/e1000_nvm.c |   1 +
>  drivers/net/e1000/base/e1000_regs.h|   7 +
>  drivers/net/e1000/em_ethdev.c  |  34 +-
>  12 files changed, 949 insertions(+), 70 deletions(-)
> 

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

Some modifications done in both commit subject and logs, can you please
double check the updates.

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

[Ferruh Yigit]

On 11/25/2016 12:02 PM, Andrew Rybchenko wrote:
> On 11/25/2016 01:24 PM, Ferruh Yigit wrote:
>> On 11/23/2016 7:49 AM, Andrew Rybchenko wrote:
>>> On 11/23/2016 03:02 AM, Ferruh Yigit wrote:
>>>> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
>>>>> The patch series adds Solarflare libefx-based network PMD.
>>>>>
>>>>> This version of the driver supports Solarflare SFN7xxx and SFN8xxx
>>>>> families of 10/40 Gbps adapters.
>>>>>
>>>>> libefx is a platform-independent library to implement drivers for
>>>>> Solarflare network adapters. It provides unified adapter family
>>>>> independent interface (if possible). FreeBSD [1] and illumos [2]
>>>>> drivers are built on top of the library.
>>>>>
>>>>> The patch series could be logically structured into 5 sub-series:
>>>>>   1. (1) add the driver skeleton including documentation
>>>>>   2. (2-30) import libefx and include it in build with the latest patch
>>>>>   3. (31-43) implement minimal device level operations in steps
>>>>>   4. (44-51) implement Rx subsystem
>>>>>   5. (52-56) implement Tx subsystem
>>>>>
>>>>> Functional driver with multi-queue support capable to send and receive
>>>>> traffic appears with the last patch in the series.
>>>>>
>>>>> The following design decisions are made during development:
>>>>>
>>>>>   1. Since libefx uses positive errno return codes, positive errno
>>>>>  return codes are used inside the driver and coversion to negative
>>>>>  is done on return from eth_dev_ops callbacks. We think that it
>>>>>  is the less error-prone way.
>>>>>
>>>>>   2. Another Solarflare PMD with in-kernel part (for control operations)
>>>>>  is considered and could be added in the future. Code for data path
>>>>>  should be shared by these two drivers. libefx-based PMD is put into
>>>>>  'efx' subdirectory to have a space for another PMD and shared code.
>>>>>
>>>>>   3. Own event queue (a way to deliver events from HW to host CPU) is
>>>>>  used for house-keeping (e.g. link status notifications), each Tx
>>>>>  and each Rx queue. No locks on datapath are requires in this case.
>>>>>
>>>>>   4. Alarm is used to periodically poll house-keeping event queue.
>>>>>  The event queue is used to deliver link status change notifications,
>>>>>  Rx/Tx queue flush events, SRAM events. It is not used on datapath.
>>>>>  The event queue polling is protected using spin-lock since
>>>>>  concurrent access from different contexts is possible (e.g. device
>>>>>  stop when polling alarm is running).
>>>>>
>>>>> [1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
>>>>> [2] 
>>>>> https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/
>>>>>
>>>>> ---
>>>>>
>>>>> Andrew Rybchenko (49):
>>>>>net/sfc: libefx-based PMD stub sufficient to build and init
>>>>>net/sfc: import libefx base
>>>>>net/sfc: import libefx register definitions
>>>>>net/sfc: import libefx filters support
>>>>>net/sfc: import libefx MCDI definition
>>>>>net/sfc: import libefx MCDI implementation
>>>>>net/sfc: import libefx MCDI logging support
>>>>>net/sfc: import libefx MCDI proxy authorization support
>>>>>net/sfc: import libefx 5xxx/6xxx family support
>>>>>net/sfc: import libefx SFN7xxx family support
>>>>>net/sfc: import libefx SFN8xxx family support
>>>>>net/sfc: import libefx diagnostics support
>>>>>net/sfc: import libefx built-in selftest support
>>>>>net/sfc: import libefx software per-queue statistics support
>>>>>net/sfc: import libefx PHY flags control support
>>>>>net/sfc: import libefx PHY statistics support
>>>>>net/sfc: import libefx PHY LEDs control support
>>>>>net/sfc: import libefx MAC statistics support
>>>>>net/sfc: import libefx event prefetch support
>>>>>net/sfc: import libefx Rx scatter support
>>>>>net/sfc: import l

[dpdk-dev] [PATCH v3] kni: avoid using lsb_release script

[Ferruh Yigit]

On 11/25/2016 12:22 PM, Robin Jarry wrote:
> The lsb_release script is part of an optional package which is not
> always installed. On the other hand, /etc/lsb-release is always present
> even on minimal Ubuntu installations.
> 
> root at ubuntu1604:~# dpkg -S /etc/lsb-release
> base-files: /etc/lsb-release
> 
> Read the file if present and use the variables defined in it.
> 
> Signed-off-by: Robin Jarry 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH v2] kni: avoid using lsb_release script

[Ferruh Yigit]

On 11/25/2016 11:33 AM, Robin Jarry wrote:
> The lsb_release script is part of an optional package which is not
> always installed. On the other hand, /etc/lsb-release is always present
> even on minimal Ubuntu installations.
> 
> root at ubuntu1604:~# dpkg -S /etc/lsb-release
> base-files: /etc/lsb-release
> 
> Read the file if present and use the variables defined in it.
> 
> Signed-off-by: Robin Jarry 
> ---
>  lib/librte_eal/linuxapp/kni/Makefile | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/Makefile 
> b/lib/librte_eal/linuxapp/kni/Makefile
> index 4e99e07e7aec..62a957ce8534 100644
> --- a/lib/librte_eal/linuxapp/kni/Makefile
> +++ b/lib/librte_eal/linuxapp/kni/Makefile
> @@ -44,8 +44,12 @@ MODULE_CFLAGS += -I$(RTE_OUTPUT)/include 
> -I$(SRCDIR)/ethtool/ixgbe -I$(SRCDIR)/e
>  MODULE_CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
>  MODULE_CFLAGS += -Wall -Werror
>  
> -ifeq ($(shell lsb_release -si 2>/dev/null),Ubuntu)
> -MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(shell lsb_release -sr | tr -d .)
> +ifneq ($(wildcard /etc/lsb-release),)

I mean removing this check completely, and having only below line, do
you think does it works that way?

> +-include /etc/lsb-release
> +endif
> +
> +ifeq ($(DISTRIB_ID),Ubuntu)
> +MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(subst .,,$(DISTRIB_RELEASE))
>  UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE 
> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
>| cut -d '"' -f2 | cut -d- -f1,2 | tr .- ,`,1)
>  MODULE_CFLAGS += 
> -D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_CODE))"
> 

[dpdk-dev] [PATCH] kni: avoid using lsb_release script

[Ferruh Yigit]

On 11/25/2016 9:52 AM, Robin Jarry wrote:
> The lsb_release script is part of an optional package which is not
> always installed. On the other hand, /etc/lsb-release is always present
> even on minimal Ubuntu installations.
> 
> root at ubuntu1604:~# dpkg -S /etc/lsb-release
> base-files: /etc/lsb-release
> 
> Read the file if present and use the variables defined in it.
> 
> Signed-off-by: Robin Jarry 
> ---
>  lib/librte_eal/linuxapp/kni/Makefile | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/Makefile 
> b/lib/librte_eal/linuxapp/kni/Makefile
> index 4e99e07e7aec..63b9ac8779f8 100644
> --- a/lib/librte_eal/linuxapp/kni/Makefile
> +++ b/lib/librte_eal/linuxapp/kni/Makefile
> @@ -44,8 +44,12 @@ MODULE_CFLAGS += -I$(RTE_OUTPUT)/include 
> -I$(SRCDIR)/ethtool/ixgbe -I$(SRCDIR)/e
>  MODULE_CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
>  MODULE_CFLAGS += -Wall -Werror
>  
> -ifeq ($(shell lsb_release -si 2>/dev/null),Ubuntu)
> -MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(shell lsb_release -sr | tr -d .)
> +ifneq ($(wildcard /etc/lsb-release),)
> +include /etc/lsb-release
> +endif

What about:
+-include /etc/lsb-release

> +
> +ifeq ($(DISTRIB_ID),Ubuntu)
> +MODULE_CFLAGS += -DUBUNTU_RELEASE_CODE=$(subst .,,$(DISTRIB_ID))

s/DISTRIB_ID/DISTRIB_RELEASE

>  UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE 
> $(RTE_KERNELDIR)/include/generated/utsrelease.h \
>| cut -d '"' -f2 | cut -d- -f1,2 | tr .- ,`,1)
>  MODULE_CFLAGS += 
> -D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_CODE))"
> 

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

[Ferruh Yigit]

On 11/24/2016 4:15 PM, Andrew Rybchenko wrote:
> On 11/23/2016 06:29 PM, Ferruh Yigit wrote:
>> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
>>> The patch series adds Solarflare libefx-based network PMD.
>>>
>>> This version of the driver supports Solarflare SFN7xxx and SFN8xxx
>>> families of 10/40 Gbps adapters.
>>>
>>> libefx is a platform-independent library to implement drivers for
>>> Solarflare network adapters. It provides unified adapter family
>>> independent interface (if possible). FreeBSD [1] and illumos [2]
>>> drivers are built on top of the library.
>>>
>>> The patch series could be logically structured into 5 sub-series:
>>>  1. (1) add the driver skeleton including documentation
>>>  2. (2-30) import libefx and include it in build with the latest patch
>>>  3. (31-43) implement minimal device level operations in steps
>>>  4. (44-51) implement Rx subsystem
>>>  5. (52-56) implement Tx subsystem
>>>
>>> Functional driver with multi-queue support capable to send and receive
>>> traffic appears with the last patch in the series.
>>>
>>> The following design decisions are made during development:
>>>
>>>  1. Since libefx uses positive errno return codes, positive errno
>>> return codes are used inside the driver and coversion to negative
>>> is done on return from eth_dev_ops callbacks. We think that it
>>> is the less error-prone way.
>>>
>>>  2. Another Solarflare PMD with in-kernel part (for control operations)
>>> is considered and could be added in the future. Code for data path
>>> should be shared by these two drivers. libefx-based PMD is put into
>>> 'efx' subdirectory to have a space for another PMD and shared code.
>>>
>>>  3. Own event queue (a way to deliver events from HW to host CPU) is
>>> used for house-keeping (e.g. link status notifications), each Tx
>>> and each Rx queue. No locks on datapath are requires in this case.
>>>
>>>  4. Alarm is used to periodically poll house-keeping event queue.
>>> The event queue is used to deliver link status change notifications,
>>> Rx/Tx queue flush events, SRAM events. It is not used on datapath.
>>> The event queue polling is protected using spin-lock since
>>> concurrent access from different contexts is possible (e.g. device
>>> stop when polling alarm is running).
>>>
>>> [1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
>>> [2] 
>>> https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/
>>>
>>> ---
>> I would like to note that very well organized patchset. Thank you for
>> your effort.
> 
> Thanks a lot, it is very pleasant to read it.
> 
> Please, see my questions in thread for patches 01/56 (about compiler
> versions to test on) and 30/56 (about libefx configuration documentation).
> 
> Also I'd like to ask a question about the further patches submission.
> We have about 40 patches which support various features (RSS, stats,
> flow control and many others). What is the preferred way to submit it?
> A. Separately since they are not so tightly related (but in fact cannot be
> applied in random order since some touch the same lines in code)
> B. As a series to process everything in one go.

If they can't be applied in random order, it is better send as a patchset.

But if you can make multiple logically independent patchsets with small
effort, please do, smaller patchsets are easier to chew.

> 
> Thanks,
> Andrew.

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

[Ferruh Yigit]

On 11/23/2016 7:49 AM, Andrew Rybchenko wrote:
> On 11/23/2016 03:02 AM, Ferruh Yigit wrote:
>> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
>>> The patch series adds Solarflare libefx-based network PMD.
>>>
>>> This version of the driver supports Solarflare SFN7xxx and SFN8xxx
>>> families of 10/40 Gbps adapters.
>>>
>>> libefx is a platform-independent library to implement drivers for
>>> Solarflare network adapters. It provides unified adapter family
>>> independent interface (if possible). FreeBSD [1] and illumos [2]
>>> drivers are built on top of the library.
>>>
>>> The patch series could be logically structured into 5 sub-series:
>>>   1. (1) add the driver skeleton including documentation
>>>   2. (2-30) import libefx and include it in build with the latest patch
>>>   3. (31-43) implement minimal device level operations in steps
>>>   4. (44-51) implement Rx subsystem
>>>   5. (52-56) implement Tx subsystem
>>>
>>> Functional driver with multi-queue support capable to send and receive
>>> traffic appears with the last patch in the series.
>>>
>>> The following design decisions are made during development:
>>>
>>>   1. Since libefx uses positive errno return codes, positive errno
>>>  return codes are used inside the driver and coversion to negative
>>>  is done on return from eth_dev_ops callbacks. We think that it
>>>  is the less error-prone way.
>>>
>>>   2. Another Solarflare PMD with in-kernel part (for control operations)
>>>  is considered and could be added in the future. Code for data path
>>>  should be shared by these two drivers. libefx-based PMD is put into
>>>  'efx' subdirectory to have a space for another PMD and shared code.
>>>
>>>   3. Own event queue (a way to deliver events from HW to host CPU) is
>>>  used for house-keeping (e.g. link status notifications), each Tx
>>>  and each Rx queue. No locks on datapath are requires in this case.
>>>
>>>   4. Alarm is used to periodically poll house-keeping event queue.
>>>  The event queue is used to deliver link status change notifications,
>>>  Rx/Tx queue flush events, SRAM events. It is not used on datapath.
>>>  The event queue polling is protected using spin-lock since
>>>  concurrent access from different contexts is possible (e.g. device
>>>  stop when polling alarm is running).
>>>
>>> [1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
>>> [2] 
>>> https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/
>>>
>>> ---
>>>
>>> Andrew Rybchenko (49):
>>>net/sfc: libefx-based PMD stub sufficient to build and init
>>>net/sfc: import libefx base
>>>net/sfc: import libefx register definitions
>>>net/sfc: import libefx filters support
>>>net/sfc: import libefx MCDI definition
>>>net/sfc: import libefx MCDI implementation
>>>net/sfc: import libefx MCDI logging support
>>>net/sfc: import libefx MCDI proxy authorization support
>>>net/sfc: import libefx 5xxx/6xxx family support
>>>net/sfc: import libefx SFN7xxx family support
>>>net/sfc: import libefx SFN8xxx family support
>>>net/sfc: import libefx diagnostics support
>>>net/sfc: import libefx built-in selftest support
>>>net/sfc: import libefx software per-queue statistics support
>>>net/sfc: import libefx PHY flags control support
>>>net/sfc: import libefx PHY statistics support
>>>net/sfc: import libefx PHY LEDs control support
>>>net/sfc: import libefx MAC statistics support
>>>net/sfc: import libefx event prefetch support
>>>net/sfc: import libefx Rx scatter support
>>>net/sfc: import libefx RSS support
>>>net/sfc: import libefx loopback control support
>>>net/sfc: import libefx monitors statistics support
>>>net/sfc: import libefx support to access monitors via MCDI
>>>net/sfc: import libefx support for Rx packed stream mode
>>>net/sfc: import libefx NVRAM support
>>>net/sfc: import libefx VPD support
>>>net/sfc: import libefx bootrom configuration support
>>>net/sfc: import libefx licensing support
>>>net/sfc: implement dummy callback to get device information
>>>net/sfc: implement driver operation to init device on attach
>>>

[dpdk-dev] [PATCH 30/56] net/sfc: include libefx in build

[Ferruh Yigit]

On 11/24/2016 3:44 PM, Andrew Rybchenko wrote:
> See one question below.
> 
> On 11/23/2016 06:26 PM, Ferruh Yigit wrote:
>> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
>>> From: Artem Andreev 
>>>
>>> Implement efsys.h for the PMD.
>>>
>>> Reviewed-by: Andy Moreton 
>>> Signed-off-by: Artem Andreev 
>>> Signed-off-by: Andrew Rybchenko 
>>> ---
>>>   drivers/net/sfc/efx/Makefile |  54 +++
>>>   drivers/net/sfc/efx/efsys.h  | 767 
>>> +++
>>>   2 files changed, 821 insertions(+)
>>>   create mode 100644 drivers/net/sfc/efx/efsys.h
>>>
>>> diff --git a/drivers/net/sfc/efx/Makefile b/drivers/net/sfc/efx/Makefile
>>> index 71f07ca..de95ea8 100644
>>> --- a/drivers/net/sfc/efx/Makefile
>>> +++ b/drivers/net/sfc/efx/Makefile
>>> @@ -33,6 +33,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
>>>   #
>>>   LIB = librte_pmd_sfc_efx.a
>>>   
>>> +CFLAGS += -I$(SRCDIR)/base/
>>> +CFLAGS += -I$(SRCDIR)
>>>   CFLAGS += -O3
>>>   
>>>   # Enable basic warnings but disable some which are accepted
>>> @@ -60,6 +62,17 @@ CFLAGS += -Wstrict-prototypes
>>>   CFLAGS += -Wundef
>>>   CFLAGS += -Wwrite-strings
>>>   
>>> +# Extra CFLAGS for base driver files
>>> +CFLAGS_BASE_DRIVER += -Wno-unused-variable
>>> +CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
>> clang complain about this one:
>> warning: unknown warning option '-Wno-unused-but-set-variable'; did you
>> mean '-Wno-unused-const-variable'? [-Wunknown-warning-option]
> 
> Will fix in v2
> 
>>> +
>>> +#
>>> +# List of base driver object files for which
>>> +# special CFLAGS above should be applied
>>> +#
>>> +BASE_DRIVER_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard 
>>> $(SRCDIR)/base/*.c)))
>>> +$(foreach obj, $(BASE_DRIVER_OBJS), $(eval CFLAGS+=$(CFLAGS_BASE_DRIVER)))
>> This cause multiple "-Wno-unused-variable -Wno-unused-but-set-variable"
>> params in final command, I guess the intention is:
>>
>> $(foreach obj, $(BASE_DRIVER_OBJS), $(eval
>> CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))
>>
>> Fixing this may generate a few compiler warnings.
> 
> Many thanks, will fix in v2.
> 
>> <...>
>>
>>> diff --git a/drivers/net/sfc/efx/efsys.h b/drivers/net/sfc/efx/efsys.h
>>> new file mode 100644
>>> index 000..2eef996
>>> --- /dev/null
>>> +++ b/drivers/net/sfc/efx/efsys.h
>>> @@ -0,0 +1,767 @@
>> <...>
>>
>> I guess below is hardcoded compile time configuration for libefx, do you
>> think does it make sense to document this default configuration?
> 
> Yes, it is libefx configuration and more options will be enabled when 
> corresponding
> functionality is supported in the PMD.
> I'm sorry, but I don't understand what would you like to see in the 
> documentation.
> Could you clarify, please?

This is mostly a question, following defines how libefx behaves, and a
little hard to find, do you think does it make sense to document these
in nic documentation, guides/nics/sfc_efx.rst, to highlight default
configuration.

Like by default filtering capabilities and SFN7xxx family support
enabled but 5xxx/6xxx family support disabled... These can be listed in
a bullet listed way in two groups (default enabled / default disabled) ?

> 
>>> +
>>> +#defineEFSYS_OPT_NAMES 0
>>> +
>>> +#defineEFSYS_OPT_SIENA 0
>>> +#defineEFSYS_OPT_HUNTINGTON 1
>>> +#defineEFSYS_OPT_MEDFORD 1
>>> +#ifdef RTE_LIBRTE_SFC_EFX_DEBUG
>>> +#defineEFSYS_OPT_CHECK_REG 1
>>> +#else
>>> +#defineEFSYS_OPT_CHECK_REG 0
>>> +#endif
>>> +
>>> +#defineEFSYS_OPT_MCDI 1
>>> +#defineEFSYS_OPT_MCDI_LOGGING 0
>>> +#defineEFSYS_OPT_MCDI_PROXY_AUTH 0
>>> +
>>> +#defineEFSYS_OPT_MAC_STATS 0
>>> +
>>> +#defineEFSYS_OPT_LOOPBACK 0
>>> +
>>> +#defineEFSYS_OPT_MON_MCDI 0
>>> +#defineEFSYS_OPT_MON_STATS 0
>>> +
>>> +#defineEFSYS_OPT_PHY_STATS 0
>>> +#defineEFSYS_OPT_BIST 0
>>> +#defineEFSYS_OPT_PHY_LED_CONTROL 0
>>> +#defineEFSYS_OPT_PHY_FLAGS 0
>>> +
>>> +#defineEFSYS_OPT_VPD 0
>>> +#defineEFSYS_OPT_NVRAM 0
>>> +#defineEFSYS_OPT_BOOTCFG 0
>>> +
>>> +#defineEFSYS_OPT_DIAG 0
>>> +#defineEFSYS_OPT_RX_SCALE 0
>>> +#defineEFSYS_OPT_QSTATS 0
>>> +#defineEFSYS_OPT_FILTER 1
>>> +#defineEFSYS_OPT_RX_SCATTER 0
>>> +
>>> +#defineEFSYS_OPT_EV_PREFETCH 0
>>> +
>>> +#defineEFSYS_OPT_DECODE_INTR_FATAL 0
>>> +
>>> +#defineEFSYS_OPT_LICENSING 0
>>> +
>>> +#defineEFSYS_OPT_ALLOW_UNCONFIGURED_NIC 0
>>> +
>>> +#defineEFSYS_OPT_RX_PACKED_STREAM 0
>> <...>
> 
> 

[dpdk-dev] [PATCH v2 1/2] net: remove dead driver names

[Ferruh Yigit]

On 11/21/2016 6:06 PM, David Marchand wrote:
> Since commit b1fb53a39d88 ("ethdev: remove some PCI specific handling"),
> rte_eth_dev_info_get() relies on dev->data->drv_name to report the driver
> name to caller.
> 
> Having the pmds set driver_info->driver_name in the pmds is useless,
> since ethdev overwrites it right after.
> The only thing the pmd must do is:
> - for pci drivers, call rte_eth_copy_pci_info() which then sets
>   data->drv_name
> - for vdev drivers, manually set data->drv_name
> 
> At this stage, virtio-user does not properly report a driver name (fixed in
> next commit).
> 
> Signed-off-by: David Marchand 
> Reviewed-by: Ferruh Yigit 
> Reviewed-by: Jan Blunck 

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

[dpdk-dev] [RFC 2/9] ethdev: move queue id check in generic layer

[Ferruh Yigit]

On 11/24/2016 9:54 AM, Olivier Matz wrote:
> The check of queue_id is done in all drivers implementing
> rte_eth_rx_queue_count(). Factorize this check in the generic function.
> 
> Note that the nfp driver was doing the check differently, which could
> induce crashes if the queue index was too big.
> 
> By the way, also move the is_supported test before the port valid and
> queue valid test.
> 
> PR=52423
> Signed-off-by: Olivier Matz 
> Acked-by: Ivan Boule 
> ---

<...>

> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index c3edc23..9551cfd 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -2693,7 +2693,7 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
>   *  The queue id on the specific port.
>   * @return
>   *  The number of used descriptors in the specific queue, or:
> - * (-EINVAL) if *port_id* is invalid
> + * (-EINVAL) if *port_id* or *queue_id* is invalid
>   * (-ENOTSUP) if the device does not support this function
>   */
>  static inline int
> @@ -2701,8 +2701,10 @@ rte_eth_rx_queue_count(uint8_t port_id, uint16_t 
> queue_id)
>  {
>   struct rte_eth_dev *dev = _eth_devices[port_id];
>  
> - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_count, -ENOTSUP);
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);

Doing port validity check before accessing dev->dev_ops->rx_queue_count
can be good idea.

What about validating port_id even before accessing
rte_eth_devices[port_id]?

> + if (queue_id >= dev->data->nb_rx_queues)
> + return -EINVAL;
>  
>   return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
>  }
> 

[dpdk-dev] [RFC 1/9] ethdev: clarify api comments of rx queue count

[Ferruh Yigit]

On 11/24/2016 9:54 AM, Olivier Matz wrote:
> The API comments are not consistent between each other.
> 
> The function rte_eth_rx_queue_count() returns the number of used
> descriptors on a receive queue.
> 
> PR=52423

What is this marker?

> Signed-off-by: Olivier Matz 
> Acked-by: Ivan Boule 

Acked-by: Ferruh Yigit 

[dpdk-dev] Proposal for a new Committer model

[Ferruh Yigit]

On 11/23/2016 3:33 PM, Neil Horman wrote:
> On Wed, Nov 23, 2016 at 02:01:44PM +0000, Ferruh Yigit wrote:
>> On 11/23/2016 1:48 PM, Neil Horman wrote:
>>> On Tue, Nov 22, 2016 at 08:56:23PM +, Ferruh Yigit wrote:
>>>> On 11/22/2016 7:52 PM, Neil Horman wrote:
>>>>> On Mon, Nov 21, 2016 at 09:52:41AM +0100, Thomas Monjalon wrote:
>>>>>> 2016-11-18 13:09, Neil Horman:
>>>>>>> A) Further promote subtree maintainership.  This was a conversation 
>>>>>>> that I
>>>>>>> proposed some time ago, but my proposed granularity was discarded in 
>>>>>>> favor
>>>>>>> of something that hasn't worked as well (in my opinion).  That is to 
>>>>>>> say a
>>>>>>> few driver pmds (i40e and fm10k come to mind) have their own tree that
>>>>>>> send pull requests to Thomas.
>>>>>>
>>>>>> Yes we tried this fine granularity and stated that it was not working 
>>>>>> well.
>>>>>> We are now using the bigger granularity that you describe below.
>>>>>>
>>>>> Ok, thats good, but that must be _very_ new.  Looking at your git tree, I 
>>>>> see no
>>>>> merge commits.  How are you pulling from those subtrees?
>>>>
>>>> next-net tree is active for last three releases.
>>>>
>>> What!?  What is the purpose of holding patches in a subtree for multiple
>>> releases?  
>>
>> :) Of course not holding them in the sub-tree.
>>
> Ok, glad that we're on the same page.
> 
>> Briefly, process is:
>> - sub-tree gets patches during merge window
>> - sub-tree first merged into main tree in -rc1 and later in -r2
>>
>> next-net tree is actively in use for last three releases, and driver/net
>> patches delegated to this tree. You can see different commiters in main
>> tree.
>>
>>> If a given changeset isn't ready for merge to Thomas tree the people
>>> working on it should clone the subtree to some place they can all 
>>> collaborate on
>>> it.  Once it goes into a subtree there needs to be a defined workflow to 
>>> get it
>>> into the canonical tree that Thomas maintains on a regular, short time 
>>> frame.
>>> to do less is to confuse the process for everyone involved, and slow people
>>> down, rather than accelerate their work.
>>>
>>>> I guess following is the first commit to the sub-tree:
>>>> http://dpdk.org/ml/archives/dev/2016-February/032580.html
>>>>
>>>> sub-trees rebase on top of main tree regularly, that is why there is no
>>>> merge commit.
>>>>
>>> I'm not asking about merge commits in the sub-tree, I'm asking about merge
>>> commits in thomas's tree.
>>
>> Same, talking about Thomas' tree.
>>
>>>  There should be a merge commit every time he pulls
>>> from a sub-tree (unless its a fast-forward I think, but with multiple 
>>> subtrees
>>> and commits going to thomas directly, that should never really happen).  
>>
>> That is what happening. Since sub-tree's rebase on top of main tree,
>> when Thomas merges, it is just plain fast-forward. So it is allowed to
>> re-write to history in sub-trees.
>>
> ok, I see what you're saying here, but I still don't see how this results in 
> no
> merge commits.  From what I can see we have at least 4 subtrees (next-crypto,
> next-net, next-eventdev, next-virtio).  If you rebase all on lastest version 
> of
> thomas's tree, and then they all make changes and send a pull request, the 
> first
> to be merged will, by definition be a fast forward.  The remaining three
> however, cannot be, because the prior merge has advanced the HEAD commit in
> Thomas's tree such that its divergent from the subsequent tree to be merged.  
> So there
> should be some merge commits in Thomas's tree.

This is simple indeed, all can do fast-forward, because all sub-trees
touch to different files.

Currently:
next-net: drivers/net/* [except virtio and vhost]
next-crypto: drivers/crypto/*
next-virtio: drivers/net/virtio/*, drivers/net/vhost/*

Common files are in main tree, when all rebased on top of it, they all
can be merged as fast-forward.

> 
> The only way I see around that, is if the merges are serialized (i.e. if you
> provide an order to the subtrees and each subtree rebases from thomas prior to
> applying its patches, then merges back to thomas's tree).  Thats rather 
> defeating
> of the purpose of parallel trees though.  You can all rebase, but then you
> operate independently of each other.  Thats how we realize a speedup here
> 
> 
> Neil
> 

[dpdk-dev] [PATCH] examples: fix ip_pipeline makefile typo

[Ferruh Yigit]

On 11/23/2016 12:56 PM, Ilya Matveychikov wrote:
> Signed-off-by: Ilya V. Matveychikov 
> ---
>  examples/ip_pipeline/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
> index 5827117..6657237 100644
> --- a/examples/ip_pipeline/Makefile
> +++ b/examples/ip_pipeline/Makefile
> @@ -36,7 +36,7 @@ endif
>  # Default target, can be overridden by command line or environment
>  RTE_TARGET ?= x86_64-native-linuxapp-gcc
> 
> -DIRS-(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline
> +DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline

No need to fix, this line can be removed completely.

Because of:
VPATH += $(SRCDIR)/pipeline

and files handled in this makefile:
SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_passthrough.c

When you fix DIR-y, you also need to add a makefile to pipeline folder
and update this makefile. I guess all those are not required. Just
removing that line is easier J

> 
>  include $(RTE_SDK)/mk/rte.vars.mk
> 
> --
> 2.5.5
> 

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> The patch series adds Solarflare libefx-based network PMD.
> 
> This version of the driver supports Solarflare SFN7xxx and SFN8xxx
> families of 10/40 Gbps adapters.
> 
> libefx is a platform-independent library to implement drivers for
> Solarflare network adapters. It provides unified adapter family
> independent interface (if possible). FreeBSD [1] and illumos [2]
> drivers are built on top of the library.
> 
> The patch series could be logically structured into 5 sub-series:
>  1. (1) add the driver skeleton including documentation
>  2. (2-30) import libefx and include it in build with the latest patch
>  3. (31-43) implement minimal device level operations in steps
>  4. (44-51) implement Rx subsystem
>  5. (52-56) implement Tx subsystem
> 
> Functional driver with multi-queue support capable to send and receive
> traffic appears with the last patch in the series.
> 
> The following design decisions are made during development:
> 
>  1. Since libefx uses positive errno return codes, positive errno
> return codes are used inside the driver and coversion to negative
> is done on return from eth_dev_ops callbacks. We think that it
> is the less error-prone way.
> 
>  2. Another Solarflare PMD with in-kernel part (for control operations)
> is considered and could be added in the future. Code for data path
> should be shared by these two drivers. libefx-based PMD is put into
> 'efx' subdirectory to have a space for another PMD and shared code.
> 
>  3. Own event queue (a way to deliver events from HW to host CPU) is
> used for house-keeping (e.g. link status notifications), each Tx
> and each Rx queue. No locks on datapath are requires in this case.
> 
>  4. Alarm is used to periodically poll house-keeping event queue.
> The event queue is used to deliver link status change notifications,
> Rx/Tx queue flush events, SRAM events. It is not used on datapath.
> The event queue polling is protected using spin-lock since
> concurrent access from different contexts is possible (e.g. device
> stop when polling alarm is running).
> 
> [1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
> [2] 
> https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/
> 
> ---

I would like to note that very well organized patchset. Thank you for
your effort.

Thanks,
ferruh

[dpdk-dev] [PATCH 33/56] net/sfc: add device configure and close stubs

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> Reviewed-by: Andy Moreton 
> Signed-off-by: Andrew Rybchenko 
> ---

<...>

> diff --git a/drivers/net/sfc/efx/sfc.h b/drivers/net/sfc/efx/sfc.h
> index 01d652d..d040f98 100644
> --- a/drivers/net/sfc/efx/sfc.h
> +++ b/drivers/net/sfc/efx/sfc.h
<...>
> @@ -131,7 +147,7 @@ sfc_adapter_unlock(struct sfc_adapter *sa)
>  }
>  
>  static inline void
> -sfc_adapter_lock_destroy(struct sfc_adapter *sa)
> +sfc_adapter_lock_fini(struct sfc_adapter *sa)

Why not do proper naming in 32/56 at first place?

<...>

[dpdk-dev] [PATCH 32/56] net/sfc: implement driver operation to init device on attach

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> The setup and configuration of the PMD is not performance sensitive,
> but is not thread safe either. It is possible that the multiple
> read/writes during PMD setup and configuration could be corrupted
> in a multi-thread environment.  

Right, this is not thread-safe, but this is valid for all PMDs, it is
not expected to have initialization in multi-threaded environment, that
said so, synchronization also won't hurt, as you said this is not fast
path, just may not be necessary.

> Since this is not performance
> sensitive, the developer can choose to add their own layer to provide
> thread-safe setup and configuration. It is expected that, in most
> applications, the initial configuration of the network ports would be
> done by a single thread at startup.
> 
> Reviewed-by: Andy Moreton 
> Signed-off-by: Andrew Rybchenko 

<...>

> diff --git a/drivers/net/sfc/efx/sfc_ethdev.c 
> b/drivers/net/sfc/efx/sfc_ethdev.c
> index 0deff07..e5b609c 100644
> --- a/drivers/net/sfc/efx/sfc_ethdev.c
> +++ b/drivers/net/sfc/efx/sfc_ethdev.c
> @@ -31,6 +31,8 @@
>  #include 
>  #include 
>  
> +#include "efx.h"
> +
>  #include "sfc.h"
>  #include "sfc_debug.h"
>  #include "sfc_log.h"
> @@ -55,6 +57,8 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
>   struct sfc_adapter *sa = dev->data->dev_private;
>   struct rte_pci_device *pci_dev = dev->pci_dev;
>   int rc;
> + const efx_nic_cfg_t *encp;
> + const struct ether_addr *from;
>  
>   /* Required for logging */
>   sa->eth_dev = dev;
> @@ -73,11 +77,43 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
>  
>   sfc_log_init(sa, "entry");
>  
> + dev->data->mac_addrs = rte_zmalloc("sfc", ETHER_ADDR_LEN, 0);
> + if (dev->data->mac_addrs == NULL) {
> + rc = ENOMEM;
> + goto fail_mac_addrs;
> + }
> +
> + sfc_adapter_lock_init(sa);
> + sfc_adapter_lock(sa);
> +
> + sfc_log_init(sa, "attaching");
> + rc = sfc_attach(sa);
> + if (rc != 0)
> + goto fail_attach;
> +
> + encp = efx_nic_cfg_get(sa->nic);
> +
> + /*
> +  * The arguments are really reverse order in comparison to
> +  * Linux kernel. Copy from NIC config to Ethernet device data.
> +  */

Yes it is J, and agreed this is confusing.

> + from = (const struct ether_addr *)(encp->enc_mac_addr);
> + ether_addr_copy(from, >data->mac_addrs[0]);
> +

<...>

[dpdk-dev] [PATCH 31/56] net/sfc: implement dummy callback to get device information

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> Just a stub to be filled in when corresponding functionality is
> implemented.

What about merging this stub with real implementation?
Or perhaps replace with code that adds dummy .dev_configure?

> 
> Reviewed-by: Andy Moreton 
> Signed-off-by: Andrew Rybchenko 
> ---
>  drivers/net/sfc/efx/sfc_ethdev.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/sfc/efx/sfc_ethdev.c 
> b/drivers/net/sfc/efx/sfc_ethdev.c
> index ff20a13..0deff07 100644
> --- a/drivers/net/sfc/efx/sfc_ethdev.c
> +++ b/drivers/net/sfc/efx/sfc_ethdev.c
> @@ -37,9 +37,16 @@
>  #include "sfc_kvargs.h"
>  
>  
> +static void
> +sfc_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> +{
> + struct sfc_adapter *sa = dev->data->dev_private;
> +
> + sfc_log_init(sa, "entry");
> +}
> +
>  static const struct eth_dev_ops sfc_eth_dev_ops = {
> - /* Just dummy init to avoid build-time warning */
> - .dev_configure  = NULL,
> + .dev_infos_get  = sfc_dev_infos_get,
>  };
>  
>  static int
> 

[dpdk-dev] [PATCH 30/56] net/sfc: include libefx in build

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> From: Artem Andreev 
> 
> Implement efsys.h for the PMD.
> 
> Reviewed-by: Andy Moreton 
> Signed-off-by: Artem Andreev 
> Signed-off-by: Andrew Rybchenko 
> ---
>  drivers/net/sfc/efx/Makefile |  54 +++
>  drivers/net/sfc/efx/efsys.h  | 767 
> +++
>  2 files changed, 821 insertions(+)
>  create mode 100644 drivers/net/sfc/efx/efsys.h
> 
> diff --git a/drivers/net/sfc/efx/Makefile b/drivers/net/sfc/efx/Makefile
> index 71f07ca..de95ea8 100644
> --- a/drivers/net/sfc/efx/Makefile
> +++ b/drivers/net/sfc/efx/Makefile
> @@ -33,6 +33,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  #
>  LIB = librte_pmd_sfc_efx.a
>  
> +CFLAGS += -I$(SRCDIR)/base/
> +CFLAGS += -I$(SRCDIR)
>  CFLAGS += -O3
>  
>  # Enable basic warnings but disable some which are accepted
> @@ -60,6 +62,17 @@ CFLAGS += -Wstrict-prototypes
>  CFLAGS += -Wundef
>  CFLAGS += -Wwrite-strings
>  
> +# Extra CFLAGS for base driver files
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable

clang complain about this one:
warning: unknown warning option '-Wno-unused-but-set-variable'; did you
mean '-Wno-unused-const-variable'? [-Wunknown-warning-option]


> +
> +#
> +# List of base driver object files for which
> +# special CFLAGS above should be applied
> +#
> +BASE_DRIVER_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard $(SRCDIR)/base/*.c)))
> +$(foreach obj, $(BASE_DRIVER_OBJS), $(eval CFLAGS+=$(CFLAGS_BASE_DRIVER)))

This cause multiple "-Wno-unused-variable -Wno-unused-but-set-variable"
params in final command, I guess the intention is:

$(foreach obj, $(BASE_DRIVER_OBJS), $(eval
CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))

Fixing this may generate a few compiler warnings.

<...>

> diff --git a/drivers/net/sfc/efx/efsys.h b/drivers/net/sfc/efx/efsys.h
> new file mode 100644
> index 000..2eef996
> --- /dev/null
> +++ b/drivers/net/sfc/efx/efsys.h
> @@ -0,0 +1,767 @@
<...>

I guess below is hardcoded compile time configuration for libefx, do you
think does it make sense to document this default configuration?

> +
> +#define  EFSYS_OPT_NAMES 0
> +
> +#define  EFSYS_OPT_SIENA 0
> +#define  EFSYS_OPT_HUNTINGTON 1
> +#define  EFSYS_OPT_MEDFORD 1
> +#ifdef RTE_LIBRTE_SFC_EFX_DEBUG
> +#define  EFSYS_OPT_CHECK_REG 1
> +#else
> +#define  EFSYS_OPT_CHECK_REG 0
> +#endif
> +
> +#define  EFSYS_OPT_MCDI 1
> +#define  EFSYS_OPT_MCDI_LOGGING 0
> +#define  EFSYS_OPT_MCDI_PROXY_AUTH 0
> +
> +#define  EFSYS_OPT_MAC_STATS 0
> +
> +#define  EFSYS_OPT_LOOPBACK 0
> +
> +#define  EFSYS_OPT_MON_MCDI 0
> +#define  EFSYS_OPT_MON_STATS 0
> +
> +#define  EFSYS_OPT_PHY_STATS 0
> +#define  EFSYS_OPT_BIST 0
> +#define  EFSYS_OPT_PHY_LED_CONTROL 0
> +#define  EFSYS_OPT_PHY_FLAGS 0
> +
> +#define  EFSYS_OPT_VPD 0
> +#define  EFSYS_OPT_NVRAM 0
> +#define  EFSYS_OPT_BOOTCFG 0
> +
> +#define  EFSYS_OPT_DIAG 0
> +#define  EFSYS_OPT_RX_SCALE 0
> +#define  EFSYS_OPT_QSTATS 0
> +#define  EFSYS_OPT_FILTER 1
> +#define  EFSYS_OPT_RX_SCATTER 0
> +
> +#define  EFSYS_OPT_EV_PREFETCH 0
> +
> +#define  EFSYS_OPT_DECODE_INTR_FATAL 0
> +
> +#define  EFSYS_OPT_LICENSING 0
> +
> +#define  EFSYS_OPT_ALLOW_UNCONFIGURED_NIC 0
> +
> +#define  EFSYS_OPT_RX_PACKED_STREAM 0

<...>

[dpdk-dev] [PATCH 01/56] net/sfc: libefx-based PMD stub sufficient to build and init

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> The PMD is put into the sfc/efx subdirectory to have a place for
> the second PMD and library shared by both.
> 
> Enable the PMD by default on supported configuratons.
> 
> Reviewed-by: Andy Moreton 
> Signed-off-by: Andrew Rybchenko 
> ---
>  MAINTAINERS |   6 ++
>  config/common_base  |   6 ++
>  config/defconfig_arm-armv7a-linuxapp-gcc|   1 +
>  config/defconfig_arm64-armv8a-linuxapp-gcc  |   1 +
>  config/defconfig_i686-native-linuxapp-gcc   |   5 +
>  config/defconfig_i686-native-linuxapp-icc   |   5 +
>  config/defconfig_ppc_64-power8-linuxapp-gcc |   1 +
>  config/defconfig_tile-tilegx-linuxapp-gcc   |   1 +
>  config/defconfig_x86_64-native-linuxapp-icc |   5 +
>  config/defconfig_x86_x32-native-linuxapp-gcc|   5 +
>  doc/guides/nics/features/sfc_efx.ini|  10 ++
>  doc/guides/nics/index.rst   |   1 +
>  doc/guides/nics/sfc_efx.rst | 109 +

Can you also update release notes please, to announce new driver.

<...>

> diff --git a/drivers/net/sfc/efx/Makefile b/drivers/net/sfc/efx/Makefile
> new file mode 100644
> index 000..71f07ca
> --- /dev/null
> +++ b/drivers/net/sfc/efx/Makefile
> @@ -0,0 +1,81 @@
<...>
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_sfc_efx.a
> +
> +CFLAGS += -O3
> +
> +# Enable basic warnings but disable some which are accepted
> +CFLAGS += -Wall

It is possible to use $(WERROR_FLAGS), which set automatically based on
selected compiler. See mk/toolchain/* .

And you can add extra options here, please keep in mind that there are
three compiler supported right now: gcc, clang and icc. You may require
to add compiler and version checks..


> +CFLAGS += -Wno-strict-aliasing
> +
> +# Enable extra warnings but disable some which are accepted
> +CFLAGS += -Wextra
> +CFLAGS += -Wno-empty-body
> +CFLAGS += -Wno-sign-compare
> +CFLAGS += -Wno-type-limits
> +CFLAGS += -Wno-unused-parameter

Is there a way to not disable these warnings but fix in the source code?
Or move to CFLAGS_BASE_DRIVER, if the reason is the base driver?

> +
> +# More warnings not enabled by above aggregators
> +CFLAGS += -Waggregate-return
> +CFLAGS += -Wbad-function-cast
> +CFLAGS += -Wcast-qual
> +CFLAGS += -Wdisabled-optimization
> +CFLAGS += -Wmissing-declarations
> +CFLAGS += -Wmissing-prototypes
> +CFLAGS += -Wnested-externs
> +CFLAGS += -Wold-style-definition
> +CFLAGS += -Wpointer-arith
> +CFLAGS += -Wstrict-prototypes
> +CFLAGS += -Wundef
> +CFLAGS += -Wwrite-strings

If you believe some can be useful for everybody, please feel free to add
to mk/toolchain/* .

> +
> +EXPORT_MAP := rte_pmd_sfc_efx_version.map
> +
> +LIBABIVER := 1
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_ethdev.c
> +SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_kvargs.c
> +
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_eal
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_kvargs
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_mempool
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_mbuf
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/net/sfc/efx/rte_pmd_sfc_efx_version.map 
> b/drivers/net/sfc/efx/rte_pmd_sfc_efx_version.map
> new file mode 100644
> index 000..1901bcb
> --- /dev/null
> +++ b/drivers/net/sfc/efx/rte_pmd_sfc_efx_version.map
> @@ -0,0 +1,4 @@
> +DPDK_16.07 {

Now this become 17.02

> +
> + local: *;
> +};
> diff --git a/drivers/net/sfc/efx/sfc.h b/drivers/net/sfc/efx/sfc.h
> new file mode 100644
> index 000..16fd2bb
> --- /dev/null
> +++ b/drivers/net/sfc/efx/sfc.h
> @@ -0,0 +1,53 @@
<..>
> +
> +#ifndef _SFC_H
> +#define  _SFC_H
s/^I/ /
This also exists in other locations and files..

<...>

[dpdk-dev] Proposal for a new Committer model

[Ferruh Yigit]

On 11/23/2016 1:48 PM, Neil Horman wrote:
> On Tue, Nov 22, 2016 at 08:56:23PM +0000, Ferruh Yigit wrote:
>> On 11/22/2016 7:52 PM, Neil Horman wrote:
>>> On Mon, Nov 21, 2016 at 09:52:41AM +0100, Thomas Monjalon wrote:
>>>> 2016-11-18 13:09, Neil Horman:
>>>>> A) Further promote subtree maintainership.  This was a conversation that I
>>>>> proposed some time ago, but my proposed granularity was discarded in favor
>>>>> of something that hasn't worked as well (in my opinion).  That is to say a
>>>>> few driver pmds (i40e and fm10k come to mind) have their own tree that
>>>>> send pull requests to Thomas.
>>>>
>>>> Yes we tried this fine granularity and stated that it was not working well.
>>>> We are now using the bigger granularity that you describe below.
>>>>
>>> Ok, thats good, but that must be _very_ new.  Looking at your git tree, I 
>>> see no
>>> merge commits.  How are you pulling from those subtrees?
>>
>> next-net tree is active for last three releases.
>>
> What!?  What is the purpose of holding patches in a subtree for multiple
> releases?  

:) Of course not holding them in the sub-tree.

Briefly, process is:
- sub-tree gets patches during merge window
- sub-tree first merged into main tree in -rc1 and later in -r2

next-net tree is actively in use for last three releases, and driver/net
patches delegated to this tree. You can see different commiters in main
tree.

> If a given changeset isn't ready for merge to Thomas tree the people
> working on it should clone the subtree to some place they can all collaborate 
> on
> it.  Once it goes into a subtree there needs to be a defined workflow to get 
> it
> into the canonical tree that Thomas maintains on a regular, short time frame.
> to do less is to confuse the process for everyone involved, and slow people
> down, rather than accelerate their work.
> 
>> I guess following is the first commit to the sub-tree:
>> http://dpdk.org/ml/archives/dev/2016-February/032580.html
>>
>> sub-trees rebase on top of main tree regularly, that is why there is no
>> merge commit.
>>
> I'm not asking about merge commits in the sub-tree, I'm asking about merge
> commits in thomas's tree.

Same, talking about Thomas' tree.

>  There should be a merge commit every time he pulls
> from a sub-tree (unless its a fast-forward I think, but with multiple subtrees
> and commits going to thomas directly, that should never really happen).  

That is what happening. Since sub-tree's rebase on top of main tree,
when Thomas merges, it is just plain fast-forward. So it is allowed to
re-write to history in sub-trees.

> I don't
> see any Merge commits in the master branch of his tree, so I'm left wondering
> what mechanic is being used to migrate patches from net-next or crypo-next to
> his tree.  Thomas, can you comment here?
> 
>>>
>>>
>>>>> We should be sharding that at a much higher
>>>>> granularity and using it much more consistently.  That is to say, that we
>>>>> should have a maintainer for all the ethernet pmds, and another for the
>>>>> crypto pmds, another for the core eal layer, another for misc libraries
>>>>> that have low patch volumes, etc.
>>>>
>>>> Yes we could open a tree for EAL and another one for the core libraries.
>>>>
>>> That could be worthwhile.  Lets see how the net and crypto subtrees work out
>>> (assuming again that these trees are newly founded)
>>>
>>>
>>>>> Each of those subdivisions should have
>>>>> their own list to communicate on, and each should have a tree that
>>>>> integrates patches for their own subsystem, and they should on a regular
>>>>> cycle send pull requests to Thomas.
>>>>
>>>> Yes I think it is now a good idea to split the mailing list traffic,
>>>> at least for netdev and cryptodev.
>>>>
>>> Agreed, that serves two purposes, it lowers the volume for people with a
>>> specific interest (i.e. its a rudimentary filter), and it avoids confusion
>>> between you and the subtree maintainer (that is to say, you don't have to 
>>> even
>>> consider pulling patches that go to the crypo and net lists, you just have 
>>> to
>>> trust that they pull those patches in and send you appropriate pull 
>>> requests).
>>
>> I still find single mail list more useful.
> Why?  If you have interest in all the subsystems of a project, then its a 
> small
> amount of overhead to subsc

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

[Ferruh Yigit]

On 11/23/2016 12:02 AM, Ferruh Yigit wrote:
> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
>> The patch series adds Solarflare libefx-based network PMD.
>>
>> This version of the driver supports Solarflare SFN7xxx and SFN8xxx
>> families of 10/40 Gbps adapters.
>>
>> libefx is a platform-independent library to implement drivers for
>> Solarflare network adapters. It provides unified adapter family
>> independent interface (if possible). FreeBSD [1] and illumos [2]
>> drivers are built on top of the library.
>>
>> The patch series could be logically structured into 5 sub-series:
>>  1. (1) add the driver skeleton including documentation
>>  2. (2-30) import libefx and include it in build with the latest patch
>>  3. (31-43) implement minimal device level operations in steps
>>  4. (44-51) implement Rx subsystem
>>  5. (52-56) implement Tx subsystem
>>
>> Functional driver with multi-queue support capable to send and receive
>> traffic appears with the last patch in the series.
>>
>> The following design decisions are made during development:
>>
>>  1. Since libefx uses positive errno return codes, positive errno
>> return codes are used inside the driver and coversion to negative
>> is done on return from eth_dev_ops callbacks. We think that it
>> is the less error-prone way.
>>
>>  2. Another Solarflare PMD with in-kernel part (for control operations)
>> is considered and could be added in the future. Code for data path
>> should be shared by these two drivers. libefx-based PMD is put into
>> 'efx' subdirectory to have a space for another PMD and shared code.
>>
>>  3. Own event queue (a way to deliver events from HW to host CPU) is
>> used for house-keeping (e.g. link status notifications), each Tx
>> and each Rx queue. No locks on datapath are requires in this case.
>>
>>  4. Alarm is used to periodically poll house-keeping event queue.
>> The event queue is used to deliver link status change notifications,
>> Rx/Tx queue flush events, SRAM events. It is not used on datapath.
>> The event queue polling is protected using spin-lock since
>> concurrent access from different contexts is possible (e.g. device
>> stop when polling alarm is running).
>>
>> [1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
>> [2] 
>> https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/
>>
>> ---
>>
>> Andrew Rybchenko (49):
>>   net/sfc: libefx-based PMD stub sufficient to build and init
>>   net/sfc: import libefx base
>>   net/sfc: import libefx register definitions
>>   net/sfc: import libefx filters support
>>   net/sfc: import libefx MCDI definition
>>   net/sfc: import libefx MCDI implementation
>>   net/sfc: import libefx MCDI logging support
>>   net/sfc: import libefx MCDI proxy authorization support
>>   net/sfc: import libefx 5xxx/6xxx family support
>>   net/sfc: import libefx SFN7xxx family support
>>   net/sfc: import libefx SFN8xxx family support
>>   net/sfc: import libefx diagnostics support
>>   net/sfc: import libefx built-in selftest support
>>   net/sfc: import libefx software per-queue statistics support
>>   net/sfc: import libefx PHY flags control support
>>   net/sfc: import libefx PHY statistics support
>>   net/sfc: import libefx PHY LEDs control support
>>   net/sfc: import libefx MAC statistics support
>>   net/sfc: import libefx event prefetch support
>>   net/sfc: import libefx Rx scatter support
>>   net/sfc: import libefx RSS support
>>   net/sfc: import libefx loopback control support
>>   net/sfc: import libefx monitors statistics support
>>   net/sfc: import libefx support to access monitors via MCDI
>>   net/sfc: import libefx support for Rx packed stream mode
>>   net/sfc: import libefx NVRAM support
>>   net/sfc: import libefx VPD support
>>   net/sfc: import libefx bootrom configuration support
>>   net/sfc: import libefx licensing support
>>   net/sfc: implement dummy callback to get device information
>>   net/sfc: implement driver operation to init device on attach
>>   net/sfc: add device configure and close stubs
>>   net/sfc: add device configuration checks
>>   net/sfc: implement device start and stop operations
>>   net/sfc: make available resources estimation and allocation
>>   net/sfc: interrupts support sufficient for event queue init
>>   net/sfc: implement event queue support
>>   net/sfc: implement EVQ dummy exception handling
>>   net/sfc: mai

[dpdk-dev] [PATCH 00/56] Solarflare libefx-based PMD

[Ferruh Yigit]

On 11/21/2016 3:00 PM, Andrew Rybchenko wrote:
> The patch series adds Solarflare libefx-based network PMD.
> 
> This version of the driver supports Solarflare SFN7xxx and SFN8xxx
> families of 10/40 Gbps adapters.
> 
> libefx is a platform-independent library to implement drivers for
> Solarflare network adapters. It provides unified adapter family
> independent interface (if possible). FreeBSD [1] and illumos [2]
> drivers are built on top of the library.
> 
> The patch series could be logically structured into 5 sub-series:
>  1. (1) add the driver skeleton including documentation
>  2. (2-30) import libefx and include it in build with the latest patch
>  3. (31-43) implement minimal device level operations in steps
>  4. (44-51) implement Rx subsystem
>  5. (52-56) implement Tx subsystem
> 
> Functional driver with multi-queue support capable to send and receive
> traffic appears with the last patch in the series.
> 
> The following design decisions are made during development:
> 
>  1. Since libefx uses positive errno return codes, positive errno
> return codes are used inside the driver and coversion to negative
> is done on return from eth_dev_ops callbacks. We think that it
> is the less error-prone way.
> 
>  2. Another Solarflare PMD with in-kernel part (for control operations)
> is considered and could be added in the future. Code for data path
> should be shared by these two drivers. libefx-based PMD is put into
> 'efx' subdirectory to have a space for another PMD and shared code.
> 
>  3. Own event queue (a way to deliver events from HW to host CPU) is
> used for house-keeping (e.g. link status notifications), each Tx
> and each Rx queue. No locks on datapath are requires in this case.
> 
>  4. Alarm is used to periodically poll house-keeping event queue.
> The event queue is used to deliver link status change notifications,
> Rx/Tx queue flush events, SRAM events. It is not used on datapath.
> The event queue polling is protected using spin-lock since
> concurrent access from different contexts is possible (e.g. device
> stop when polling alarm is running).
> 
> [1] https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/
> [2] 
> https://github.com/illumos/illumos-gate/tree/master/usr/src/uts/common/io/sfxge/common/
> 
> ---
> 
> Andrew Rybchenko (49):
>   net/sfc: libefx-based PMD stub sufficient to build and init
>   net/sfc: import libefx base
>   net/sfc: import libefx register definitions
>   net/sfc: import libefx filters support
>   net/sfc: import libefx MCDI definition
>   net/sfc: import libefx MCDI implementation
>   net/sfc: import libefx MCDI logging support
>   net/sfc: import libefx MCDI proxy authorization support
>   net/sfc: import libefx 5xxx/6xxx family support
>   net/sfc: import libefx SFN7xxx family support
>   net/sfc: import libefx SFN8xxx family support
>   net/sfc: import libefx diagnostics support
>   net/sfc: import libefx built-in selftest support
>   net/sfc: import libefx software per-queue statistics support
>   net/sfc: import libefx PHY flags control support
>   net/sfc: import libefx PHY statistics support
>   net/sfc: import libefx PHY LEDs control support
>   net/sfc: import libefx MAC statistics support
>   net/sfc: import libefx event prefetch support
>   net/sfc: import libefx Rx scatter support
>   net/sfc: import libefx RSS support
>   net/sfc: import libefx loopback control support
>   net/sfc: import libefx monitors statistics support
>   net/sfc: import libefx support to access monitors via MCDI
>   net/sfc: import libefx support for Rx packed stream mode
>   net/sfc: import libefx NVRAM support
>   net/sfc: import libefx VPD support
>   net/sfc: import libefx bootrom configuration support
>   net/sfc: import libefx licensing support
>   net/sfc: implement dummy callback to get device information
>   net/sfc: implement driver operation to init device on attach
>   net/sfc: add device configure and close stubs
>   net/sfc: add device configuration checks
>   net/sfc: implement device start and stop operations
>   net/sfc: make available resources estimation and allocation
>   net/sfc: interrupts support sufficient for event queue init
>   net/sfc: implement event queue support
>   net/sfc: implement EVQ dummy exception handling
>   net/sfc: maintain management event queue
>   net/sfc: periodic management EVQ polling using alarm
>   net/sfc: minimum port control sufficient to receive traffic
>   net/sfc: implement Rx subsystem stubs
>   net/sfc: check configured rxmode
>   net/sfc: implement Rx queue setup release operations
>   net/sfc: calculate Rx buffer size which may be used
>   net/sfc: validate Rx queue buffers setup
>   net/sfc: implement Rx queue start and stop operations
>   net/sfc: implement device callback to Rx burst of packets
>   net/sfc: discard scattered packet on Rx correctly
> 
> Artem Andreev (2):
>   net/sfc: include libefx in build
>   net/sfc: implement device operation 

[dpdk-dev] Proposal for a new Committer model

[Ferruh Yigit]

On 11/22/2016 7:52 PM, Neil Horman wrote:
> On Mon, Nov 21, 2016 at 09:52:41AM +0100, Thomas Monjalon wrote:
>> 2016-11-18 13:09, Neil Horman:
>>> A) Further promote subtree maintainership.  This was a conversation that I
>>> proposed some time ago, but my proposed granularity was discarded in favor
>>> of something that hasn't worked as well (in my opinion).  That is to say a
>>> few driver pmds (i40e and fm10k come to mind) have their own tree that
>>> send pull requests to Thomas.
>>
>> Yes we tried this fine granularity and stated that it was not working well.
>> We are now using the bigger granularity that you describe below.
>>
> Ok, thats good, but that must be _very_ new.  Looking at your git tree, I see 
> no
> merge commits.  How are you pulling from those subtrees?

next-net tree is active for last three releases.

I guess following is the first commit to the sub-tree:
http://dpdk.org/ml/archives/dev/2016-February/032580.html

sub-trees rebase on top of main tree regularly, that is why there is no
merge commit.

> 
> 
>>> We should be sharding that at a much higher
>>> granularity and using it much more consistently.  That is to say, that we
>>> should have a maintainer for all the ethernet pmds, and another for the
>>> crypto pmds, another for the core eal layer, another for misc libraries
>>> that have low patch volumes, etc.
>>
>> Yes we could open a tree for EAL and another one for the core libraries.
>>
> That could be worthwhile.  Lets see how the net and crypto subtrees work out
> (assuming again that these trees are newly founded)
> 
> 
>>> Each of those subdivisions should have
>>> their own list to communicate on, and each should have a tree that
>>> integrates patches for their own subsystem, and they should on a regular
>>> cycle send pull requests to Thomas.
>>
>> Yes I think it is now a good idea to split the mailing list traffic,
>> at least for netdev and cryptodev.
>>
> Agreed, that serves two purposes, it lowers the volume for people with a
> specific interest (i.e. its a rudimentary filter), and it avoids confusion
> between you and the subtree maintainer (that is to say, you don't have to even
> consider pulling patches that go to the crypo and net lists, you just have to
> trust that they pull those patches in and send you appropriate pull requests).

I still find single mail list more useful.
Also with current process, after -rc2 release, patches directly merged
into main tree instead of sub-trees...

> 
>>> Thomas in turn should by and large,
>>> only be integrating pull requests.  This should address our high-
>>> throughput issue, in that it will allow multiple maintainers to share the
>>> workload, and integration should be relatively easy.
>>
>> Yes in an ideal organization, the last committer does only a last check
>> that technical plan and fairness are respected.
>> So it gives more time to coordinate the plans :)
>>
> Correct.  Thats never 100% accurate of course, some things will still have to
> come to you directly, simply by virtue of the fact that they don't completely
> fit anywhere else, but thats ok, the goal is really just to get your total 
> patch
> volume lower, and replace it with pull requests that you can either trivialy
> mere or figure out with the help of the subtree maintainer.
> 
>>> B) Designate alternates to serve as backups for the maintainer when they
>>> are unavailable.  This provides high-availablility, and sounds very much
>>> like your proposal, but in the interests of clarity, there is still a
>>> single maintainer at any one time, it just may change to ensure the
>>> continued merging of patches, if the primary maintainer isn't available.
>>> Ideally however, those backup alternates arent needed, because most of the
>>> primary maintainers work in merging pull requests, which are done based on
>>> the trust of the submaintainer, and done during a very limited window of
>>> time.  This also partially addreses multi-vendor fairness if your subtree
>>> maintainers come from multiple participating companies.
>>
>> About the merge window, I do not have a strong opinion about how it can be
>> improved. However, I know that closing the window too early makes developer
>> unhappy because it makes wait - between development start and its release -
>> longer.
> 
> This is a fair point, but I'm not talking about closing it early here, all
> I'm suggesting is that, if you do proper pull requests from subtrees, your 
> tree
> Thomas will only need a reasonably small window of time to accept new 
> features,
> because you'll just merge the subtrees, rather than integrating individual
> patches.  E.g. you won't be constantly merging patches over the course of a
> development cycle, your tree's HEAD will mostly consist of merge commits as
> subtree maintainers send you pull requests, and ideally they will send those
> near the start of a window.  How long you keep your merge window open after 
> that
> is up to you.
> 
> Neil
> 
> 
>  
>>

[dpdk-dev] [PATCH] mk: remove make target for examples

[Ferruh Yigit]

On 11/22/2016 9:38 AM, Thomas Monjalon wrote:
> 2016-11-22 00:34, Ferruh Yigit:
>> On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
>>> The command
>>>   make examples
>>> works only if target directories have the exact name of configs.
>>>
>>> It is more flexible to use
>>>   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
>>>
>>> Signed-off-by: Thomas Monjalon 
>>
>> Instead of removing examples & examples_clean targets, what do you think
>> keeping them as wrapper to suggested usage, for backward compatibility.
>>
>> Something like:
>> "
>> BUILDING_RTE_SDK :=
>> export BUILDING_RTE_SDK
>>
>> # Build directory is given with O=
>> O ?= $(RTE_SDK)/examples
>>
>> # Target for which examples should be built.
>> T ?= build
>>
>> .PHONY: examples
>> examples:
>> @echo == Build examples for $(T)
>> $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);
>>
>> .PHONY: examples_clean
>> examples_clean:
>> @echo == Clean examples for $(T)
>> $(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
>> "
> 
> What is the benefit of this makefile? Just remove -C ?

To keep existing targets, in case somebody use them.

> It is not compatible with the old behaviour, so I'm afraid it would be
> confusing for no real benefit.

Right, not fully compatible, but still can do:
make examples / make examples_clean
make examples T=x86_64-native-linuxapp-gcc

Overall, if you believe keeping them is confusing, I am OK with it, just
may need to update doc/build-sdk-quick.txt to fix "make help" output.

Thanks,
ferruh

[dpdk-dev] [PATCH] mk: remove make target for examples

[Ferruh Yigit]

On 11/21/2016 11:47 PM, Thomas Monjalon wrote:
> The command
>   make examples
> works only if target directories have the exact name of configs.
> 
> It is more flexible to use
>   make -C examples RTE_SDK=$(pwd) RTE_TARGET=build
> 
> Signed-off-by: Thomas Monjalon 

Instead of removing examples & examples_clean targets, what do you think
keeping them as wrapper to suggested usage, for backward compatibility.

Something like:
"
BUILDING_RTE_SDK :=
export BUILDING_RTE_SDK

# Build directory is given with O=
O ?= $(RTE_SDK)/examples

# Target for which examples should be built.
T ?= build

.PHONY: examples
examples:
@echo == Build examples for $(T)
$(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T);

.PHONY: examples_clean
examples_clean:
@echo == Clean examples for $(T)
$(MAKE) -C examples O=$(abspath $(O)) RTE_TARGET=$(T) clean;
"

[dpdk-dev] [PATCH] eal: postpone vdev initialization

[Ferruh Yigit]

On 11/21/2016 5:02 PM, Jerin Jacob wrote:
> On Mon, Nov 21, 2016 at 09:54:57AM +0000, Ferruh Yigit wrote:
>> On 11/20/2016 8:00 AM, Jerin Jacob wrote:
>>> Some platform like octeontx may use pci and
>>> vdev based combined device to represent a logical
>>> dpdk functional device.In such case, postponing the
>>> vdev initialization after pci device
>>> initialization will provide the better view of
>>> the pci device resources in the system in
>>> vdev's probe function, and it allows better
>>> functional subsystem registration in vdev probe
>>> function.
>>>
>>> As a bonus, This patch fixes a bond device
>>> initialization use case.
>>>
>>> example command to reproduce the issue:
>>> ./testpmd -c 0x2  --vdev 'eth_bond0,mode=0,
>>> slave=:02:00.0,slave=:03:00.0' --
>>> --port-topology=chained
>>>
>>> root cause:
>>> In existing case(vdev initialization and then pci
>>> initialization), creates three Ethernet ports with
>>> following port ids
>>> 0 - Bond device
>>> 1 - PCI device 0
>>> 2 - PCI devive 1
>>>
>>> Since testpmd, calls the configure/start on all the ports on
>>> start up,it will translate to following illegal setup sequence
>>>
>>> 1)bond device configure/start
>>> 1.1) pci device0 stop/configure/start
>>> 1.2) pci device1 stop/configure/start
>>> 2)pci device 0 configure(illegal setup case,
>>> as device in start state)
>>>
>>> The fix changes the initialization sequence and
>>> allow initialization in following valid setup order
>>> 1) pcie device 0 configure/start
>>> 2) pcie device 1 configure/start
>>> 3) bond device 2 configure/start
>>> 3.1) pcie device 0/stop/configure/start
>>> 3.2) pcie device 1/stop/configure/start
>>>
>>> Signed-off-by: Jerin Jacob 
>>> ---
>>
>> This changes the port id assignments to the devices, right?
>>
>> Previously virtual devices get first available port ids (0..N1), later
>> physical devices (N1..N2). Now this becomes reverse.
>>
>> Can this change break some existing user applications?
> 
> I guess it may be effected only to ethdev bond pmd based application,
> which is broken anyway.

My concern is, this may effect the applications that use "--vdev" eal
parameter and has an assumption about port assignment.

And if this breaks any userspace application, does it require a
deprecation notice?

> Let me know what it takes to make forward progress on this patch. I can
> fix the same in v2.
> 
> Jerin
> 

[dpdk-dev] [PATCH RFC 0/2] Allow vectorized Rx with 4096 desc ring size on Intel NICs.

[Ferruh Yigit]

On 10/19/2016 3:30 PM, Ferruh Yigit wrote:
> On 10/19/2016 3:07 PM, Ilya Maximets wrote:
>> Ilya Maximets (2):
>>   net/i40e: allow bulk alloc for the max size desc ring
>>   net/ixgbe: allow bulk alloc for the max size desc ring
>>
>>  drivers/net/i40e/i40e_rxtx.c   | 24 +---
>>  drivers/net/ixgbe/ixgbe_rxtx.c | 17 +
>>  drivers/net/ixgbe/ixgbe_rxtx.h |  2 +-
>>  3 files changed, 15 insertions(+), 28 deletions(-)
>>
> 
> Hi Ilya,
> 
> Thank you for the patch, we are in post rc1 phase and this is a new
> feature, so this patchset will be considered for next release (v17.02).
> 

Reminder for this patch (RFC) which has been sent in 16.11 time frame
and we postponed to 17.02 release.

Thanks,
ferruh

[dpdk-dev] [PATCH v3] ntnic: add PMD driver

[Ferruh Yigit]

On 9/12/2016 8:34 AM, fc at napatech.com (Finn Christensen) wrote:
>> -Original Message-
>> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
>> Sent: 10. september 2016 10:20
>> To: Finn Christensen 
>> Cc: Neil Horman ; dev at dpdk.org;
>> stephen at networkplumber.org
>> Subject: Re: [PATCH v3] ntnic: add PMD driver
>>
>> 2016-09-10 07:58, Finn Christensen:
>>> From: Neil Horman [mailto:nhorman at tuxdriver.com]
 On Fri, Sep 09, 2016 at 12:48:38PM +, Finn Christensen wrote:
> This is the Napatech NTNIC Poll Mode Driver (PMD) for DPDK.
>
> This patch adds support for Napatech NICs to DPDK. This is the
> initial implementation.
>
> Signed-off-by: Finn Christensen 
> ---
> v3:
>   * Removed the need for binary libraries on build
> v2:
>   * Added information how to build the PMD without NIC
> Board Support Package
>   * Fixed some formatting issues

 So, this is a step in the right direction, but I think its solving
 the wrong problem.  If you have a dependency on an external library,
 thats ok, and accessing it via dlopen makes it possible to build the
 library without having that library present, but it not really in
 keeping with the spirit of what I meant.  This driver is still
 effectively dependent on a binary blob that we have no visibility
 into.  The better solution is releasing the source for the ntnic and
 ntos libraries.  The license file in the referenced git tree
 indicates its BSD licensed, so I don't think there should be a problem in
>> doing that.

 Neil

>>> No, unfortunately the ntapi is not BSD licensed, only the header files
>>> that you can freely download are.
>>> We are building this NT NIC by using parts or our technology from our
>>> capture adapters and that is using closed source software.
>>>
>>> We are new to opensource and we want to go that way, but we haven't
>>> yet a complete stand-alone driver ready that we can put into the DPDK
>>> PMD to have a complete self contained and open sourced DPDK PMD, that
>>> only needs the actual HW NIC plugged in to run.
>>> Therefore this version is implemented as a virtual device, exactly
>>> like the PCAP PMD driver is, and it runs on top of a driver that follows the
>> NIC itself.
>>>
>>> In regards to the DPDK functionality we do not see that anything is missing.
>>> I cannot either see where we should add source code, because it is not
>>> part of the DPDK package and it should not be either.
>>>
>>> One of the things I really liked about the DPDK open source project is
>>> that it uses BSD licensing not GPL. Therefore, I must admit, we
>>> completely failed to see that the "spirit" of the DPDK community is
>>> not really BSD. Our view of this community was that the main driving
>>> force of it was to be able to make DPDK run on everything anywhere
>>> effectively, in a global contributing community, without  any legally
>> constrains prohibiting us to do so.
>>
>> It is difficult to define what is the spirit of a community, especially only 
>> after
>> few mail exchanges.
>> I agree that running on everything anywhere is a nice goal.
>> Here Neil, as a RedHat developer, is probably concerned about enabling your
>> driver in a distribution. It seems your model is not compatible with the
>> "anywhere goal" and will be disabled in that case, until it is fully open.
> 
> The ntnic PMD is not enabled by default and I think it should not be either. 
> To
> enable it in a distribution for general purposes seems wrong. In that respect
> we see no difference between the PCAP PMD and this ntnic PMD.
> 
>>> However, this is our standing, and I don't know what else to do.
>>> Please advise or NAK this PMD.
>>
>> I do not remember having already seen such model in DPDK.
>> So we need to think about the implications a bit more.
>> (Comments/discussions are welcome)
>> Thanks for your patience.
> 
> Thanks. I will be happy to discuss this further, so that we can get to a 
> conclusion.
> If the outcome is that the majority of the community does not like the idea 
> that
> upstream supported PMDs has external linking dependencies to closed source
> libraries, then it is ok with us(a pity though). But then it might be a good 
> idea to
> make that decision clear to everybody else by putting in a clause into the
> contribution section of the DPDK guide, or somewhere else in the guide.
> 
> In our opinion, the inclusion of the ntnic PMD into upstream DPDK, does not
> seem to be any different than that of the PCAP PMD, since that is also
> dependent on external header files and externally built libraries.
> Of course we see the difference in open source vs close source library. But we
> cannot see that is has any influence in the usage or functionality of the 
> DPDK.
> 
> Thanks for this discussion!
> 

The patch is still waiting in the patchwork.

This requires a high level discussion. Any suggestion on how to 

[dpdk-dev] [PATCH v9] drivers/net:new PMD using tun/tap host interface

[Ferruh Yigit]

On 10/13/2016 11:03 PM, Keith Wiles wrote:
> The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces
> on the local host. The PMD allows for DPDK and the host to
> communicate using a raw device interface on the host and in
> the DPDK application. The device created is a Tap device with
> a L2 packet header.
> 
> v9 - Fix up the docs to use correct syntax
> v8 - Fix issue with tap_tx_queue_setup() not return zero on success.
> v7 - Reword the comment in common_base and fix the data->name issue
> v6 - fixed the checkpatch issues
> v5 - merge in changes from list review see related emails
>  fixed many minor edits
> v4 - merge with latest driver changes
> v3 - fix includes by removing ifdef for other type besides Linux
>  Fix the copyright notice in the Makefile
> v2 - merge all of the patches into one patch
>  Fix a typo on naming the tap device
>  Update the maintainers list
> 
> Signed-off-by: Keith Wiles 
> ---

Just a reminder, this is a new PMD and waiting for community review.

[dpdk-dev] [dpdk-stable] [PATCH] Revert "bonding: use existing enslaved device queues"

[Ferruh Yigit]

On 10/25/2016 3:00 PM, Bruce Richardson wrote:
> On Tue, Oct 25, 2016 at 02:48:04PM +0100, Declan Doherty wrote:
>> On 25/10/16 13:57, Bruce Richardson wrote:
>>> On Mon, Oct 24, 2016 at 04:07:17PM +0100, Declan Doherty wrote:
 On 24/10/16 15:51, Jan Blunck wrote:
> On Mon, Oct 24, 2016 at 7:02 AM, Declan Doherty
>  wrote:
>> On 14/10/16 00:37, Eric Kinzie wrote:
>>>
>>> On Wed Oct 12 16:24:21 +0100 2016, Bruce Richardson wrote:

 On Wed, Oct 12, 2016 at 04:24:54PM +0300, Ilya Maximets wrote:
>
> On 07.10.2016 05:02, Eric Kinzie wrote:
>>
>> On Wed Sep 07 15:28:10 +0300 2016, Ilya Maximets wrote:
>>>
>>> This reverts commit 5b7bb2bda5519b7800f814df64d4e015282140e5.
>>>
>>> It is necessary to reconfigure all queues every time because
>>> configuration
>>> can be changed.
>>>
>>> For example, if we're reconfiguring bonding device with new memory
>>> pool,
>>> already configured queues will still use the old one. And if the old
>>> mempool be freed, application likely will panic in attempt to use
>>> freed mempool.
>>>
>>> This happens when we use the bonding device with OVS 2.6 while MTU
>>> reconfiguration:
>>>
>>> PANIC in rte_mempool_get_ops():
>>> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)"
>>> failed
>>>
>>> Cc: 
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>  drivers/net/bonding/rte_eth_bond_pmd.c | 10 ++
>>>  1 file changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
>>> b/drivers/net/bonding/rte_eth_bond_pmd.c
>>> index b20a272..eb5b6d1 100644
>>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>>> @@ -1305,8 +1305,6 @@ slave_configure(struct rte_eth_dev
>>> *bonded_eth_dev,
>>> struct bond_rx_queue *bd_rx_q;
>>> struct bond_tx_queue *bd_tx_q;
>>>
>>> -   uint16_t old_nb_tx_queues = 
>>> slave_eth_dev->data->nb_tx_queues;
>>> -   uint16_t old_nb_rx_queues = 
>>> slave_eth_dev->data->nb_rx_queues;
>>> int errval;
>>> uint16_t q_id;
>>>
>>> @@ -1347,9 +1345,7 @@ slave_configure(struct rte_eth_dev
>>> *bonded_eth_dev,
>>> }
>>>
>>> /* Setup Rx Queues */
>>> -   /* Use existing queues, if any */
>>> -   for (q_id = old_nb_rx_queues;
>>> -q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
>>> +   for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues;
>>> q_id++) {
>>> bd_rx_q = (struct bond_rx_queue
>>> *)bonded_eth_dev->data->rx_queues[q_id];
>>>
>>> errval =
>>> rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>> @@ -1365,9 +1361,7 @@ slave_configure(struct rte_eth_dev
>>> *bonded_eth_dev,
>>> }
>>>
>>> /* Setup Tx Queues */
>>> -   /* Use existing queues, if any */
>>> -   for (q_id = old_nb_tx_queues;
>>> -q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
>>> +   for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues;
>>> q_id++) {
>>> bd_tx_q = (struct bond_tx_queue
>>> *)bonded_eth_dev->data->tx_queues[q_id];
>>>
>>> errval =
>>> rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>> --
>>> 2.7.4
>>>
>>
>> NAK
>>
>> There are still some users of this code.  Let's give them a chance to
>> comment before removing it.
>
>
> Hi Eric,
>
> Are these users in CC-list? If not, could you, please, add them?
> This patch awaits in mail-list already more than a month. I think, 
> it's
> enough
> time period for all who wants to say something. Patch fixes a real bug
> that
> prevent using of DPDK bonding in all applications that reconfigures
> devices
> in runtime including OVS.
>
 Agreed.

 Eric, does reverting this patch cause you problems directly, or is your
 concern
 just with regards to potential impact to others?

 Thanks,
 /Bruce
>>>
>>>
>>> This won't impact me directly.  The users are CCed (different thread)
>>> and I haven't seen any comment, so I no longer have any objection to
>>> reverting this change.
>>>
>>> Eric
>>>
>>
>> As there has been no 

[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info

[Ferruh Yigit]

On 11/18/2016 4:06 PM, Alejandro Lucero wrote:
> Previous reported speed was hardcoded.
> 
> Signed-off-by: Alejandro Lucero 
> ---
>  drivers/net/nfp/nfp_net.c  | 28 ++--
>  drivers/net/nfp/nfp_net_ctrl.h | 13 +
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index c6b1587..24f3164 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -816,6 +816,17 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
>   struct rte_eth_link link, old;
>   uint32_t nn_link_status;
>  
> + static const uint32_t ls_to_ethtool[] = {
> + [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE,
> + [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE,
> + [NFP_NET_CFG_STS_LINK_RATE_1G]  = ETH_SPEED_NUM_1G,
> + [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G,
> + [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G,
> + [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G,
> + [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G,
> + [NFP_NET_CFG_STS_LINK_RATE_100G]= ETH_SPEED_NUM_100G,
> + };
> +
>   PMD_DRV_LOG(DEBUG, "Link update\n");
>  
>   hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> @@ -831,8 +842,21 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw)
>   link.link_status = ETH_LINK_UP;
>  
>   link.link_duplex = ETH_LINK_FULL_DUPLEX;
> - /* Other cards can limit the tx and rx rate per VF */
> - link.link_speed = ETH_SPEED_NUM_40G;
> +
> + nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
> +  NFP_NET_CFG_STS_LINK_RATE_MASK;
> +
> + if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) ||
> + ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) &&
> + (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0)))
> + link.link_speed = ETH_SPEED_NUM_40G;

For specific firmware version, speed is still hardcoded to 40G, can you
please mention from this and if possible its reason in commit log?

> + else {
> + if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN ||

This check can be redundant, since
ls_to_ethtool[NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] => ETH_SPEED_NUM_NONE

> + nn_link_status >= RTE_DIM(ls_to_ethtool))
> + link.link_speed = ETH_SPEED_NUM_NONE;
> + else
> + link.link_speed = ls_to_ethtool[nn_link_status];
> + }
>  
>   if (old.link_status != link.link_status) {
>   nfp_net_dev_atomic_write_link_status(dev, );
> diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h
> index fce8251..f9aaba3 100644
> --- a/drivers/net/nfp/nfp_net_ctrl.h
> +++ b/drivers/net/nfp/nfp_net_ctrl.h
> @@ -157,6 +157,19 @@
>  #define   NFP_NET_CFG_VERSION_MINOR(x)(((x) & 0xff) <<  0)
>  #define NFP_NET_CFG_STS 0x0034
>  #define   NFP_NET_CFG_STS_LINK(0x1 << 0) /* Link up or down */
> +/* Link rate */
> +#define   NFP_NET_CFG_STS_LINK_RATE_SHIFT 1
> +#define   NFP_NET_CFG_STS_LINK_RATE_MASK  0xF
> +#define   NFP_NET_CFG_STS_LINK_RATE   \
> +   (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT)

This macro is not used at all, just fyi.

> +#define   NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED   0
> +#define   NFP_NET_CFG_STS_LINK_RATE_UNKNOWN   1
> +#define   NFP_NET_CFG_STS_LINK_RATE_1G2
> +#define   NFP_NET_CFG_STS_LINK_RATE_10G   3
> +#define   NFP_NET_CFG_STS_LINK_RATE_25G   4
> +#define   NFP_NET_CFG_STS_LINK_RATE_40G   5
> +#define   NFP_NET_CFG_STS_LINK_RATE_50G   6
> +#define   NFP_NET_CFG_STS_LINK_RATE_100G  7
>  #define NFP_NET_CFG_CAP 0x0038
>  #define NFP_NET_CFG_MAX_TXRINGS 0x003c
>  #define NFP_NET_CFG_MAX_RXRINGS 0x0040
> 

[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info

[Ferruh Yigit]

On 11/18/2016 4:50 PM, Alejandro Lucero wrote:
> On Fri, Nov 18, 2016 at 4:29 PM, Thomas Monjalon  6wind.com>
> wrote:
> 
>> 2016-11-18 16:06, Alejandro Lucero:
>>> Previous reported speed was hardcoded.
>>>
>>> Signed-off-by: Alejandro Lucero 
>>> ---
>>>  drivers/net/nfp/nfp_net.c  | 28 ++--
>>>  drivers/net/nfp/nfp_net_ctrl.h | 13 +
>>>  2 files changed, 39 insertions(+), 2 deletions(-)
>>
>> You should update the doc in the same patch:
>> doc/guides/nics/features/nfp.ini
>> It will be the first feature as the file appears to be empty.
>> So you will need another patch to fill other existing features.
>>
> 
> Yes. I'm just working on updating that file properly.
> May I delay this doc change for including it with that other one?
> It will be a bit weird to just have one feature there.

I think yes, but can you please send the documentation patch before
integration deadline of this release?

> 
> 
>>
>> I have an unrelated question: why nfp is disabled in the default build?
>>
>>
> Because NFP PMD can just work if Netronome BSP is installed in the system.

Is this BSP open source and freely available? If so, can you update the
document with details (how/where to get it) please?

> We do not support PF with the PMD, so it requires a Linux PF driver which
> comes with the BSP.
> 
> The compilation has no dependencies, but we had our own UIO driver before
> (now using igb_uio). So basically, we wanted the people aware of this
> dependency and to specifically configure this option.
> 
> I know what you are surely going to ask about DPDK in Linux distributions,
> and that this being a bad idea. The fact is, we have people using NFP PMD
> as part of a product, so installing that product implies to (automatically)
> install the BSP and a specific DPDK version with the NFP PMD enabled. But
> yes, maybe we should modify this and to add some sort of BSP check inside
> the PMD.

Is there a DPDK version <-> BSP version dependency?

> 
> So, thanks for the heads up. I will think about this.
> 

[dpdk-dev] Solarflare PMD submission question

[Ferruh Yigit]

On 11/21/2016 8:59 AM, Thomas Monjalon wrote:
> 2016-11-21 11:46, Andrew Rybchenko:
>> On 11/21/2016 11:19 AM, Thomas Monjalon wrote:
 Before submitting 56 patches I'd like to double-check that checkpatch.pl
 errors (for example, because of assignments in the 'if' condition,
 parenthesis around return value) is not a show-stopper for base driver
 import.
>>> You can run checkpatches.sh or send the patches to checkpatch at dpdk.org.
>>> The script check-git-log.sh can also guide you for the expected formatting.
>>
>> Yes, I did it and it helped me to find and fix some coding standard 
>> violations.
>>
>> The problem with libefx (base driver) is that it is existing code which 
>> follows FreeBSD and illumos coding conventions which contradict to 
>> checkpatches.sh sometimes (e.g. require parenthesis around return 
>> value). Other example of error produced by checkpatches.sh is assign in 
>> if. It is widely used in the code to assign return code value and 
>> compare it vs 0 in one line. It is not a coding standard conflict, but 
>> it is very wide-spread in the code (so changing it will produce too many 
>> changes not strictly required/useful).
>>
>> So, may I repeat my question if it is a show-stopper for base driver or 
>> acceptable.
> 
> I would vote to accept these minor style warnings for the base driver.
> Ferruh, any comment?
> 

For _base driver_, I am also OK for having checkpatch warnings.

[dpdk-dev] [PATCH] eal: postpone vdev initialization

[Ferruh Yigit]

On 11/20/2016 8:00 AM, Jerin Jacob wrote:
> Some platform like octeontx may use pci and
> vdev based combined device to represent a logical
> dpdk functional device.In such case, postponing the
> vdev initialization after pci device
> initialization will provide the better view of
> the pci device resources in the system in
> vdev's probe function, and it allows better
> functional subsystem registration in vdev probe
> function.
> 
> As a bonus, This patch fixes a bond device
> initialization use case.
> 
> example command to reproduce the issue:
> ./testpmd -c 0x2  --vdev 'eth_bond0,mode=0,
> slave=:02:00.0,slave=:03:00.0' --
> --port-topology=chained
> 
> root cause:
> In existing case(vdev initialization and then pci
> initialization), creates three Ethernet ports with
> following port ids
> 0 - Bond device
> 1 - PCI device 0
> 2 - PCI devive 1
> 
> Since testpmd, calls the configure/start on all the ports on
> start up,it will translate to following illegal setup sequence
> 
> 1)bond device configure/start
> 1.1) pci device0 stop/configure/start
> 1.2) pci device1 stop/configure/start
> 2)pci device 0 configure(illegal setup case,
> as device in start state)
> 
> The fix changes the initialization sequence and
> allow initialization in following valid setup order
> 1) pcie device 0 configure/start
> 2) pcie device 1 configure/start
> 3) bond device 2 configure/start
> 3.1) pcie device 0/stop/configure/start
> 3.2) pcie device 1/stop/configure/start
> 
> Signed-off-by: Jerin Jacob 
> ---

This changes the port id assignments to the devices, right?

Previously virtual devices get first available port ids (0..N1), later
physical devices (N1..N2). Now this becomes reverse.

Can this change break some existing user applications?

Thanks,
ferruh

[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

[Ferruh Yigit]

On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> Hi Thomas,
> 
> I got this email when sending a patch some minutes ago.
> 
> The point is I trusted script/checkpatches.sh which did not report those
> warnings.
> Am I doing anything wrong when using checkpatches.sh?
> 
> 
> -- Forwarded message --
> From: 
> Date: Fri, Nov 18, 2016 at 3:04 PM
> Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> To: test-report at dpdk.org
> Cc: Alejandro Lucero 
> 
> 
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/17091
> 
> _coding style issues_
> 
> 
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
> #53: FILE: drivers/net/nfp/nfp_net.c:806:
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))

btw, you can benefit from RTE_DIM:

lib/librte_eal/common/include/rte_common.h:352:
#define  RTE_DIM(a)  (sizeof (a) / sizeof ((a)[0]))

> 
> CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> #91: FILE: drivers/net/nfp/nfp_net.c:856:
> +   else {
> +
> 
> total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> 

[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info

[Ferruh Yigit]

On 11/18/2016 3:10 PM, Alejandro Lucero wrote:
> Hi Thomas,
> 
> I got this email when sending a patch some minutes ago.
> 
> The point is I trusted script/checkpatches.sh which did not report those
> warnings.
> Am I doing anything wrong when using checkpatches.sh?

I am also getting same warnings as below, this can be related to the
checkpatch.pl version.

I have: Version: 0.32
(./scripts/checkpatch.pl --version)

> 
> 
> -- Forwarded message --
> From: 
> Date: Fri, Nov 18, 2016 at 3:04 PM
> Subject: |WARNING| [PATCH] nfp: report link speed using hardware info
> To: test-report at dpdk.org
> Cc: Alejandro Lucero 
> 
> 
> Test-Label: checkpatch
> Test-Status: WARNING
> http://dpdk.org/patch/17091
> 
> _coding style issues_
> 
> 
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects?
> #53: FILE: drivers/net/nfp/nfp_net.c:806:
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> 
> CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
> #91: FILE: drivers/net/nfp/nfp_net.c:856:
> +   else {
> +
> 
> total: 0 errors, 0 warnings, 2 checks, 68 lines checked
> 

[dpdk-dev] [PATCH 3/3] net/mlx5: do not invalidate title CQE

[Ferruh Yigit]

On 11/17/2016 10:38 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:56AM +0100, Nelio Laranjeiro wrote:
>> We can leave the title completion queue entry untouched since its contents
>> are not modified.
>>
>> Reported-by: Liming Sun 
>> Signed-off-by: Nelio Laranjeiro 
<...>
> Acked-by: Adrien Mazarguil 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [dpdk-stable] [PATCH 2/3] net/mlx5: fix wrong htons

[Ferruh Yigit]

On 11/17/2016 10:38 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:55AM +0100, Nelio Laranjeiro wrote:
>> Completion queue entry data uses network endian, to access them we should use
>> ntoh*().
>>
>> Fixes: c305090bbaf8 ("net/mlx5: replace countdown with threshold for Tx 
>> completions")
>>
>> CC: stable at dpdk.org
>> Reported-by: Liming Sun 
>> Signed-off-by: Nelio Laranjeiro 
<...>
> Acked-by: Adrien Mazarguil 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [dpdk-stable] [PATCH 1/3] net/mlx5: fix leak when starvation occurs

[Ferruh Yigit]

On 11/17/2016 10:37 AM, Adrien Mazarguil wrote:
> On Thu, Nov 17, 2016 at 10:49:54AM +0100, Nelio Laranjeiro wrote:
>> The list of segments to free was wrongly manipulated ending by only freeing
>> the first segment instead of freeing all of them.  The last one still
>> belongs to the NIC and thus should not be freed.
>>
>> Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx")
>>
>> CC: stable at dpdk.org
>> Reported-by: Liming Sun 
>> Signed-off-by: Nelio Laranjeiro 
<...>
> Acked-by: Adrien Mazarguil 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v2 2/2] Move non-PCI related eth_dev initialization to rte_eth_dev_allocate()

[Ferruh Yigit]

On 11/17/2016 5:16 PM, Jan Blunck wrote:
> This moves the non-PCI related initialization of the link state interrupt
> callback list and the setting of the default MTU to rte_eth_dev_allocate()
> so that drivers only need to set non-default values.
> 
> Signed-off-by: Jan Blunck 

Reviewed-by: Ferruh Yigit 

[dpdk-dev] [PATCH v2 1/2] Clear eth_dev->data in rte_eth_dev_allocate()

[Ferruh Yigit]

On 11/17/2016 5:16 PM, Jan Blunck wrote:
> Lets clear the eth_dev->data when allocating a new rte_eth_dev so that
> drivers only need to set non-zero values.
> 
> Signed-off-by: Jan Blunck 
> ---

Reviewed-by: Ferruh Yigit 

[dpdk-dev] [PATCH 1/2] Clear eth_dev->data in rte_eth_dev_allocate()

[Ferruh Yigit]

On 11/17/2016 2:24 PM, Jan Blunck wrote:
> Lets clear the eth_dev->data when allocating a new rte_eth_dev so that
> drivers only need to set non-zero values.
> 
> Signed-off-by: Jan Blunck 
> ---
>  drivers/net/mlx4/mlx4.c   | 1 -
>  drivers/net/mlx5/mlx5.c   | 1 -
>  lib/librte_ether/rte_ethdev.c | 2 +-

+ drivers/net/bonding/rte_eth_bond_api.c ?

-   eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
-
...
-   eth_dev->data->dev_started = 0;
-   eth_dev->data->promiscuous = 0;
-   eth_dev->data->scattered_rx = 0;
-   eth_dev->data->all_multicast = 0;
-

+ drivers/net/mpipe/mpipe_tilegx.c ?

-   eth_dev->data->dev_flags = 0;


<...>

> @@ -212,6 +212,7 @@ rte_eth_dev_allocate(const char *name)
>  
>   eth_dev = _eth_devices[port_id];
>   eth_dev->data = _eth_dev_data[port_id];
> + memset(eth_dev->data, 0, sizeof(*eth_dev->data));
>   snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
>   eth_dev->data->port_id = port_id;
>   eth_dev->attached = DEV_ATTACHED;
> @@ -259,7 +260,6 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>   }
>   eth_dev->pci_dev = pci_dev;
>   eth_dev->driver = eth_drv;
> - eth_dev->data->rx_mbuf_alloc_failed = 0;
>  
>   /* init user callbacks */
>   TAILQ_INIT(&(eth_dev->link_intr_cbs));
> 

[dpdk-dev] [PATCH 2/2] Move non-PCI related eth_dev initialization to rte_eth_dev_allocate()

[Ferruh Yigit]

On 11/17/2016 2:24 PM, Jan Blunck wrote:
> This moves the non-PCI related initialization of the link state interrupt
> callback list and the setting of the default MTU to rte_eth_dev_allocate()
> so that drivers only need to set non-default values.
> 
> Signed-off-by: Jan Blunck 
> ---
>  drivers/net/bonding/rte_eth_bond_api.c |  2 --
>  drivers/net/cxgbe/cxgbe_main.c |  2 --
>  drivers/net/mlx4/mlx4.c|  2 --
>  drivers/net/mlx5/mlx5.c|  3 ---
>  drivers/net/null/rte_eth_null.c|  2 --
>  drivers/net/ring/rte_eth_ring.c|  2 --
>  drivers/net/vhost/rte_eth_vhost.c  |  2 --
>  lib/librte_ether/rte_ethdev.c  | 12 
>  8 files changed, 4 insertions(+), 23 deletions(-)

I think following also redundant and can be removed:
app/test/virtual_pmd.c:
604:TAILQ_INIT(&(eth_dev->link_intr_cbs));

<...>

> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 12af4b1..f58a995 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c

What do you think doing same thing for rte_cryptodev.c J

> @@ -215,6 +215,10 @@ rte_eth_dev_allocate(const char *name)
>   memset(eth_dev->data, 0, sizeof(*eth_dev->data));
>   snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
>   eth_dev->data->port_id = port_id;
> + eth_dev->data->rx_mbuf_alloc_failed = 0;

This is no more required, because of memset

> + eth_dev->data->mtu = ETHER_MTU;
> + TAILQ_INIT(&(eth_dev->link_intr_cbs));
> +
>   eth_dev->attached = DEV_ATTACHED;
>   eth_dev_last_created_port = port_id;
>   nb_ports++;
> @@ -261,14 +265,6 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>   eth_dev->pci_dev = pci_dev;
>   eth_dev->driver = eth_drv;
>  
> - /* init user callbacks */
> - TAILQ_INIT(&(eth_dev->link_intr_cbs));
> -
> - /*
> -  * Set the default MTU.
> -  */
> - eth_dev->data->mtu = ETHER_MTU;
> -
>   /* Invoke PMD device initialization function */
>   diag = (*eth_drv->eth_dev_init)(eth_dev);
>   if (diag == 0)
> 

[dpdk-dev] [PATCH] doc: announce kni_vhost removal

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit 
---
 doc/guides/prog_guide/kernel_nic_interface.rst | 2 ++
 doc/guides/rel_notes/deprecation.rst   | 6 ++
 2 files changed, 8 insertions(+)

diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst 
b/doc/guides/prog_guide/kernel_nic_interface.rst
index eb16e2e..4f25595 100644
--- a/doc/guides/prog_guide/kernel_nic_interface.rst
+++ b/doc/guides/prog_guide/kernel_nic_interface.rst
@@ -169,6 +169,8 @@ This provides flexibility in multiprocess scenarios
 (where the KNI is created in the primary process but the callbacks are handled 
in the secondary one).
 The constraint is that a single process can register and handle the requests.

+.. _kni_vhost_backend-label:
+
 KNI Working as a Kernel vHost Backend
 -

diff --git a/doc/guides/rel_notes/deprecation.rst 
b/doc/guides/rel_notes/deprecation.rst
index 2d17bc6..956473a 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -71,3 +71,9 @@ Deprecation Notices
 * mempool: The functions for single/multi producer/consumer are deprecated
   and will be removed in 17.02.
   It is replaced by ``rte_mempool_generic_get/put`` functions.
+
+* kni: Remove :ref:`kni_vhost_backend-label` feature (KNI_VHOST) in 17.05 
release.
+  :doc:`Vhost Library ` is currently preferred method 
for
+  guest - host communication. Just for clarification, this is not to remove KNI
+  or VHOST feature, but KNI_VHOST which is a KNI feature enabled via a compile
+  time option, and disabled by default.
-- 
2.9.3

[dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[Ferruh Yigit]

Hi Wenzhuo,

On 11/16/2016 3:28 AM, Lu, Wenzhuo wrote:
> Hi Wei,
> 
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Dai
>> Sent: Wednesday, November 16, 2016 10:41 AM
>> To: dev at dpdk.org; Burakov, Anatoly; david.marchand at 6wind.com; Dai, Wei
>> Subject: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of 
>> mknod()
>>
>> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c, The 
>> return
>> value of mknod() is ret, not f got by fopen().
>> So the value of ret should be checked for mknod().
>>
>> Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")
>>
>> Signed-off-by: Wei Dai 
>> ---
>> fix my local git setting and send same patch again to make merging easier
>>
>>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> index 1786b75..3e4ffb5 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> @@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path,
>> unsigned uio_num)
>>  snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
>>  dev = makedev(major, minor);
>>  ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
>> -if (f == NULL) {
>> +if (ret != 0) {
> I think checkpatch will suggest to just use if (ret)

Your are right, default checkpatch.pl complains about this usage (with
--strict option), but:

- According DPDK coding style this usage is preferred (although I
personally prefer kernel one..)

http://dpdk.org/doc/guides/contributing/coding_style.html#null-pointers

"
if (p == NULL) /* Good, compare pointer to NULL */

if (!p) /* Bad, using ! on pointer */
"

- This warning disabled in dpdk scripts/checkpatches.sh by "--ignore
COMPARISON_TO_NULL", so it shouldn't complain.

[dpdk-dev] [PATCH] net/i40e: add additional prefetch instructions for bulk rx

[Ferruh Yigit]

On 10/13/2016 11:30 AM, Ananyev, Konstantin wrote:

<...>


 Actually I can see some valid use cases where it is beneficial to have 
 this prefetch in driver.
 In our sw distributor case it is trivial to just prefetch next packet on 
 each iteration because packets are processed one by one.
 However when we move this functionality to hw by means of 
 RSS/vfunction/FlowDirector(our long term goal) worker threads will
>> receive
 packets directly from rx queues of NIC.
 First operation of worker thread is to perform bulk lookup in hash table 
 by destination MAC. This will cause cache miss on accessing
>> each
 eth header and can't be easily mitigated in application code.
 I assume it is ubiquitous use case for DPDK.
>>>
>>> Yes it is a quite common use-case.
>>> Though I many cases it is possible to reorder user code to hide (or 
>>> minimize) that data-access latency.
>>> From other side there are scenarios where this prefetch is excessive and 
>>> can cause some drop in performance.
>>> Again, as I know, none of PMDs for Intel devices prefetches packet's data 
>>> in  simple (single segment) RX mode.
>>> Another thing that some people may argue then - why only one cache line is 
>>> prefetched,
>>> in some use-cases might need to look at 2-nd one.
>>>
>> There is a build-time config setting for this behaviour for exactly the 
>> reasons
>> called out here - in some apps you get a benefit, in others you see a perf
>> hit. The default is "on", which makes sense for most cases, I think.
>> From common_base:
>>
>> CONFIG_RTE_PMD_PACKET_PREFETCH=y$
> 
> Yes, but right now i40e and ixgbe non-scattered RX (both vector and scalar) 
> just ignore that flag.
> Though yes, might be a good thing to make them to obey that flag properly.

Hi Vladyslav,

According Konstantin's comment, what do you think updating patch to do
prefetch within CONFIG_RTE_PMD_PACKET_PREFETCH ifdef?

But since config option is enabled by default, performance concern is
still valid and needs to be investigated.

Thanks,
ferruh

[dpdk-dev] [PATCH v1 0/2] XStats fixes

[Ferruh Yigit]

On 11/14/2016 6:14 AM, Remy Horton wrote:
> The offsets used in rte_i40evf_stats_strings for transmission
> statistics were wrong, returning the total byte count rather than
> the respective (unicast, multicast, broadcast, drop, & error)
> packet counts.
> 
> This patchset also fixes some spelling errors.
> 
> Fixes: da61cd084976 ("i40evf: add extended stats")
> Fixes: 0eedec25ea36 ("i40e: clean log messages")
> 
> Remy Horton (2):
>   net/i40e: fix incorrect xstats value mapping
>   net/i40e: fix spelling errors
> 
>  drivers/net/i40e/i40e_ethdev.c|  2 +-
>  drivers/net/i40e/i40e_ethdev_vf.c | 16 
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 

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

[dpdk-dev] Clarification for eth_driver changes

[Ferruh Yigit]

On 11/10/2016 11:05 AM, Shreyansh Jain wrote:
> Hello David,
> 
> On Thursday 10 November 2016 01:46 PM, David Marchand wrote:
>> Hello Shreyansh,
>>
>> On Thu, Nov 10, 2016 at 8:26 AM, Shreyansh Jain  
>> wrote:
>>> I need some help and clarification regarding some changes I am doing to
>>> cleanup the EAL code.
>>>
>>> There are some changes which should be done for eth_driver/rte_eth_device
>>> structures:
>>>
>>> 1. most obvious, eth_driver should be renamed to rte_eth_driver.
>>> 2. eth_driver currently has rte_pci_driver embedded in it
>>>  - there can be ethernet devices which are _not_ PCI
>>>  - in which case, this structure should be removed.
>>
>> Do we really need to keep a eth_driver ?
> 
> No. As you have rightly mentioned below (as well as in your Jan'16 
> post), it is a mere convenience.

Isn't it good to separate the logic related which bus device connected
and what functionality it provides. Because these two can be flexible:

device -> virtual_bus -> ethernet_functionality
device -> pci_bus -> crypto_functionality
device -> x_bus   -> y_function


what about:

create generic bus driver/device and all eal level deal with generic
bus. different buses inherit from generic bus logic

create generic functionality device/driver and pmd level deal with
these. different functionalities inherit from generic functionality logic

and use rte_device/rte_driver as glue logic for all these.

This makes easy to add new bus or functionality device/drivers without
breaking existing logic.


Something like this:

struct rte_device {
char *name;
struct rte_driver *drv;
struct rte_bus_device *bus_dev;
struct rte_funcional_device *func_dev;
*devargs
}

struct rte_bus_device {
struct rte_device *dev;
/* generic bus device */
}

struct rte_pci_device {
struct rte_bus_device bus_dev;
/* generic pci bus */
}

struct rte_vdev_device {
struct rte_bus_device bus_dev;
/* generic vdev bus */
}

struct rte_funcional_device {
struct rte_device *dev;
}

struct rte_eth_device {
struct rte_funcional_device func_dev;
/* generic eth device */
}

struct rte_crypto_device {
struct rte_funcional_device func_dev;
/* generic crypto device */
}


struct rte_driver {
char *name;
struct rte_device *dev;
struct rte_bus_driver *bus_drv;
struct rte_funcional_driver *func_drv;
}

struct rte_bus_driver {
struct rte_driver *drv;
rte_bus_probe_t *probe(dev, drv);
rte_bus_remove_t *remove(dev);
/* generic bus driver */
}

struct rte_pci_driver {
struct rte_bus_driver bus_drv;
*id_table;
/* generic pci bus */
}

struct rte_vdev_driver {
struct rte_bus_driver bus_drv;
/* generic vdev bus */
}

struct rte_funcional_driver {
struct rte_driver *drv;
rte_funcional_init_t *init;
rte_funcional_uninit_t *uninit;
}

struct rte_eth_driver {
struct rte_funcional_driver func_drv;
/* generic eth driver */
}

struct rte_crypto_driver {
struct rte_funcional_driver func_drv;
/* generic crypto driver */
}

pci_scan_one()
{
dev = create();
pci_dev = create();

dev->bus_dev = pci_dev;
pci_dev->bus_dev.dev = dev;

insert(bus_dev_list);
}

register_drv(drv)
{
insert(bus_drv_list)
insert(func_drv_list)
insert(drv_list)
}

rte_eal_bus_probe()
  for bus_dev in bus_dev_list
bus_probe_all_drivers(bus_dev)
  for bus_drv in bus_drv_list
bus_probe_one_driver(bus_drv, bus_dev)
  bus_dev->dev->drv = bus_drv->drv;
  bus_drv->drv->dev = bus_dev->dev;
  probe(drv, dev)

probe(drv, dev)
{
dev->func_dev = create();
func_dev->dev = dev;

func_drv = drv->func_drv;
func_drv->init(func_dev);
}

eht_init(func_dev)
{
eth_dev = (struct rte_eth_dev)func_dev;
drv = func_dev->dev->drv;
}

[dpdk-dev] [PATCH] improve git diff

[Ferruh Yigit]

On 11/11/2016 4:21 PM, Thomas Monjalon wrote:
> 2016-11-11 11:22, Ferruh Yigit:
>> On 11/9/2016 3:44 PM, Thomas Monjalon wrote:
>>> Sometimes git does not print the name of the function being changed
>>> after @@. It happens especially after a goto label which is not indented.
>>> Giving a hint about the languages of files .c, .h and .py
>>> will improve hunk headers of "git diff" rendering.
>>>
>>> Signed-off-by: Thomas Monjalon 
> [...]
>>> --- /dev/null
>>> +++ b/.gitattributes
>>> @@ -0,0 +1,3 @@
>>> +*.c   diff=cpp
>>> +*.h   diff=cpp
>>
>> Can't git auto detect to use C/C++ language diff use for .c/.h files?
> 
> No
> 
>> Do you have a sample that generates bad hunk header, just to test?
> 
> Yes, you'll find a lot of them with "git log -p | grep '@@.*:'"
> Example:
>   git show bb6722f | grep '@@.*:'
> Without the patch, it is a goto label in the hunk header.
> 

You are right, I was expecting better from git J

[dpdk-dev] [PATCH v2] doc: add sub-repositories information

[Ferruh Yigit]

DPDK switched to main and sub-repositories approach, this patch
documents new approach and updates development process according.

Signed-off-by: Ferruh Yigit 
Acked-by: John McNamara 
---
 doc/guides/contributing/patches.rst | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 729aea7..fabddbe 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -20,7 +20,14 @@ The DPDK development process has the following features:
 * There is a mailing list where developers submit patches.
 * There are maintainers for hierarchical components.
 * Patches are reviewed publicly on the mailing list.
-* Successfully reviewed patches are merged to the master branch of the 
repository.
+* Successfully reviewed patches are merged to the repository.
+
+|
+
+* There are main repository ``dpdk`` and sub-repositories ``dpdk-next-*``.
+* A patch should be sent for its target repository. Like net drivers should be 
on top of dpdk-next-net repository.
+* All sub-repositories are merged into main repository for -rc1 and -rc2 
versions of the release.
+* After -rc2 release all patches should target main repository.

 The mailing list for DPDK development is `dev at dpdk.org 
<http://dpdk.org/ml/archives/dev/>`_.
 Contributors will need to `register for the mailing list 
<http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
@@ -33,12 +40,17 @@ Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ 
for further informat
 Getting the Source Code
 ---

-The source code can be cloned using either of the following::
+The source code can be cloned using either of the following:

-git clone git://dpdk.org/dpdk
+main repository::

+git clone git://dpdk.org/dpdk
 git clone http://dpdk.org/git/dpdk

+sub-repositories (`list <http://dpdk.org/browse/next>`_)::
+
+git clone git://dpdk.org/next/dpdk-next-*
+git clone http://dpdk.org/git/next/dpdk-next-*

 Make your Changes
 -
-- 
2.9.3

[dpdk-dev] [PATCH] doc: add sub-repositories information

[Ferruh Yigit]

Hi Shreyansh,

On 11/11/2016 1:15 PM, Shreyansh Jain wrote:
>> +* All sub-repositories merged into main repository for -rc1 and -rc2 
>> versions of the release.
> 'All sub-repositories *are* merged into ...'?

I will send a fixed version.

Thanks,
ferruh

[dpdk-dev] [PATCH] doc: announce API and ABI changes for librte_eal

[Ferruh Yigit]

On 11/10/2016 3:51 PM, David Marchand wrote:
> On Thu, Nov 10, 2016 at 12:17 PM, Shreyansh Jain  
> wrote:
>> Signed-off-by: Shreyansh Jain 
>> ---
>>  doc/guides/rel_notes/deprecation.rst | 10 ++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst 
>> b/doc/guides/rel_notes/deprecation.rst
>> index 1a9e1ae..2af2476 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -35,3 +35,13 @@ Deprecation Notices
>>  * mempool: The functions for single/multi producer/consumer are deprecated
>>and will be removed in 17.02.
>>It is replaced by ``rte_mempool_generic_get/put`` functions.
>> +
>> +* ABI/API changes are planned for 17.02: ``rte_device``, ``rte_driver`` 
>> will be
>> +  impacted because of introduction of a new ``rte_bus`` hierarchy. This 
>> would
>> +  also impact the way devices are identified by EAL. A bus-device-driver 
>> model
>> +  will be introduced providing a hierarchical view of devices.
>> +
>> +* ``eth_driver`` is planned to be removed in 17.02. This currently serves as
>> +  a placeholder for PMDs to register themselves. Changes for ``rte_bus`` 
>> will
>> +  provide a way to handle device initialization currently being done in
>> +  ``eth_driver``.
>> --
>> 2.7.4
>>
> 
> Acked-by: David Marchand 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH 2/2] net: align ethdev and eal driver names

[Ferruh Yigit]

On 11/10/2016 1:51 PM, David Marchand wrote:
> Some virtual pmds report a different name than the vdev driver name
> registered in eal.
> While it does not hurt, let's try to be consistent.
> 
> Signed-off-by: David Marchand 
> ---

Since you did all the work, instead of second patch what do you think
doing something like [1] (basically adding eth_dev->rte_driver link) and
when done for all vdevs, remove eth_dev->data->drv_name completely?


[1]
diff --git a/drivers/net/null/rte_eth_null.c
b/drivers/net/null/rte_eth_null.c
index e4fd68f..d657133 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -553,9 +553,9 @@ eth_dev_null_create(const char *name,
TAILQ_INIT(_dev->link_intr_cbs);

eth_dev->driver = NULL;
+   eth_dev->rte_driver = _null_drv.driver;
data->dev_flags = RTE_ETH_DEV_DETACHABLE;
data->kdrv = RTE_KDRV_NONE;
-   data->drv_name = pmd_null_drv.driver.name;
data->numa_node = numa_node;

/* finally assign rx and tx ops */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..0527c4a 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -259,6 +259,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
}
eth_dev->pci_dev = pci_dev;
eth_dev->driver = eth_drv;
+   eth_dev->rte_driver = pci_drv->driver.name;
eth_dev->data->rx_mbuf_alloc_failed = 0;

/* init user callbacks */
@@ -1557,7 +1558,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct
rte_eth_dev_info *dev_info)
RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
(*dev->dev_ops->dev_infos_get)(dev, dev_info);
dev_info->pci_dev = dev->pci_dev;
-   dev_info->driver_name = dev->data->drv_name;
+   dev_info->driver_name = dev->rte_driver->name;
dev_info->nb_rx_queues = dev->data->nb_rx_queues;
dev_info->nb_tx_queues = dev->data->nb_tx_queues;
 }
@@ -3214,7 +3215,6 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
struct rte_pci_device *pci_de

eth_dev->data->kdrv = pci_dev->kdrv;
eth_dev->data->numa_node = pci_dev->device.numa_node;
-   eth_dev->data->drv_name = pci_dev->driver->driver.name;
 }

 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..63e7931 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1642,6 +1642,7 @@ struct rte_eth_dev {
 */
struct rte_eth_rxtx_callback
*pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
uint8_t attached; /**< Flag indicating the port is attached */
+   struct rte_driver *rte_driver;
 } __rte_cache_aligned;

 struct rte_eth_dev_sriov {

[dpdk-dev] [PATCH] doc: add sub-repositories information

[Ferruh Yigit]

DPDK switched to main and sub-repositories approach, this patch
documents new approach and updates development process according.

Signed-off-by: Ferruh Yigit 
---
 doc/guides/contributing/patches.rst | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/doc/guides/contributing/patches.rst 
b/doc/guides/contributing/patches.rst
index 729aea7..bec9bfc 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -20,7 +20,14 @@ The DPDK development process has the following features:
 * There is a mailing list where developers submit patches.
 * There are maintainers for hierarchical components.
 * Patches are reviewed publicly on the mailing list.
-* Successfully reviewed patches are merged to the master branch of the 
repository.
+* Successfully reviewed patches are merged to the repository.
+
+|
+
+* There are main repository ``dpdk`` and sub-repositories ``dpdk-next-*``.
+* A patch should be sent for its target repository. Like net drivers should be 
on top of dpdk-next-net repository.
+* All sub-repositories merged into main repository for -rc1 and -rc2 versions 
of the release.
+* After -rc2 release all patches should target main repository.

 The mailing list for DPDK development is `dev at dpdk.org 
<http://dpdk.org/ml/archives/dev/>`_.
 Contributors will need to `register for the mailing list 
<http://dpdk.org/ml/listinfo/dev>`_ in order to submit patches.
@@ -33,12 +40,17 @@ Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ 
for further informat
 Getting the Source Code
 ---

-The source code can be cloned using either of the following::
+The source code can be cloned using either of the following:

-git clone git://dpdk.org/dpdk
+main repository::

+git clone git://dpdk.org/dpdk
 git clone http://dpdk.org/git/dpdk

+sub-repositories (`list <http://dpdk.org/browse/next>`_)::
+
+git clone git://dpdk.org/next/dpdk-next-*
+git clone http://dpdk.org/git/next/dpdk-next-*

 Make your Changes
 -
-- 
2.9.3

[dpdk-dev] [PATCH] maintainers: add staging tree for network drivers

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit 
---
CC: Bruce Richardson 
CC: Thomas Monjalon 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 065397b..d6bb8f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -252,6 +252,8 @@ F: examples/l2fwd-crypto/

 Networking Drivers
 --
+M: Ferruh Yigit 
+T: git://dpdk.org/next/dpdk-next-net

 Link bonding
 M: Declan Doherty 
-- 
2.9.3

[dpdk-dev] [PATCH v1] doc: announce API and ABI change for librte_ether

[Ferruh Yigit]

On 11/4/2016 1:39 PM, Mcnamara, John wrote:
> 
> 
>> -Original Message-
>> From: Iremonger, Bernard
>> Sent: Tuesday, October 18, 2016 2:38 PM
>> To: dev at dpdk.org; Mcnamara, John 
>> Cc: Iremonger, Bernard 
>> Subject: [PATCH v1] doc: announce API and ABI change for librte_ether
>>
>> In 17.02 five rte_eth_dev_set_vf_*** functions will be removed from
>> librte_ether, renamed and moved to the ixgbe PMD.
>>
>> Signed-off-by: Bernard Iremonger 
> 
> Acked-by: John McNamara 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH v3] examples/ipsec-secgw: fix pointer to local outside scope

[Ferruh Yigit]

On 11/7/2016 5:25 PM, Fan Zhang wrote:
> Coverity issue: 137871
> Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
> 
> Signed-off-by: Fan Zhang 
> ---

Although checkpatch will complain about long lines, I believe it is
better not to wrap log messages.

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix copy into fixed size buffer issue

[Ferruh Yigit]

On 11/7/2016 2:21 PM, Fan Zhang wrote:
> Fixes: 0d547ed0 ("examples/ipsec-secgw: support configuration
> file")
> Coverity issue: 137875
> 
> Signed-off-by: Fan Zhang 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH 2/2] bnxt: add a FALLTHROUGH comment in the cascading switch statement

[Ferruh Yigit]

On 11/3/2016 6:58 PM, Ajit Khaparde wrote:
> The cascading switch statement in bnxt_hwrm.c is missing the FALLTHROUGH
> comment. Adding that.
> 
> Coverity: 127552
> 
> Signed-off-by: Ajit Khaparde 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH] examples/ipsec-secgw: fix buffer not null terminated

[Ferruh Yigit]

On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
> Coverity issue: 137854
> 
> Signed-off-by: Fan Zhang 

Acked-by: Ferruh Yigit 


Minor nit, for all coverity fixes, defined commit log format is:

Coverity issue: x
Fixes: .

Basically two lines should be swapped, but I guess this can be fixed
while applying instead of sending a new version for this.

[dpdk-dev] [PATCH] examples/ipsec-secgw: fix buffer not terminated issue

[Ferruh Yigit]

On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
> Coverity issue: 137855
> 
> Signed-off-by: Fan Zhang 

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH] examples/ipsec-secgw: fix copy into fixed size buffer issue

[Ferruh Yigit]

On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Coverity issue: 137875
> Fixes: 0d547ed0 ("examples/ipsec-secgw: support configuration
> file")
> 
> Signed-off-by: Fan Zhang 
> ---
>  examples/ipsec-secgw/sa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 9e2c8a9..c891be2 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -177,7 +177,7 @@ parse_key_string(const char *key_str, uint8_t *key)
>   pt_end = strchr(pt_start, ':');
>  
>   if (pt_end == NULL)
> - strncpy(sub_str, pt_start, strlen(pt_start));
> + strncpy(sub_str, pt_start, strlen(sub_str) - 1);

sub_str initial value is not known, "strlen(sub_str) - 1" leaves last
byte of the string random, instead of intended NULL.

Also sub_str has hardcoded length of 3, it can be good to confirm NULL
terminated byte taken into account, and number actually can be maximum
two digits long.

>   else {
>   if (pt_end - pt_start > 2)
>   return 0;
> 

[dpdk-dev] [PATCH v3] net/kni: add KNI PMD

[Ferruh Yigit]

Hi Yong,

Thank you for the review.

On 11/3/2016 1:24 AM, Yong Wang wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Monday, October 10, 2016 6:20 AM
>> To: dev at dpdk.org
>> Cc: Ferruh Yigit 
>> Subject: [dpdk-dev] [PATCH v3] net/kni: add KNI PMD
>>
>> Add KNI PMD which wraps librte_kni for ease of use.
>>
>> KNI PMD can be used as any regular PMD to send / receive packets to the
>> Linux networking stack.
>>
>> Signed-off-by: Ferruh Yigit 
>> ---
>>
>> v3:
>> * rebase on top of latest master
>>
>> v2:
>> * updated driver name eth_kni -> net_kni
>> ---

<...>

>>  CONFIG_RTE_LIBRTE_KNI=n
>> +CONFIG_RTE_LIBRTE_PMD_KNI=n
> 
> Nit: change this to CONFIG_RTE_LIBRTE_KNI_PMD instead to be consistent with 
> all other pmds.

There is an inconsistency between virtual and physical PMD config options.

Physical ones: xxx_PMD=
*IXGBE_PMD, *I40E_PMD, *ENA_PMD, ...

Virtual ones: PMD_xxx=
*PMD_RING, *PMD_PCAP, *PMD_NULL, ...

So I am consistent with inconsistency J

<...>

>> +#define DRV_NAME net_kni
> 
> The name generated this way is not consistent with other vdevs.  Why not 
> simply assign "KNI PMD" to drv_name?

Right, it is not consistent but intentionaly.

With macro RTE_PMD_REGISTER_VDEV(net_kni, xxx), rte_driver.name set to
"net_kni"

and if you set drivername to "KNI PMD", pmd will report driver name as
"KNI PMD"

so there will be two different driver names, I tried to unify them to a
single name.
And some physical drivers already does same thing.


<...>

>> +static uint16_t
>> +eth_kni_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>> +{
>> +struct pmd_queue *kni_q = q;
>> +struct rte_kni *kni = kni_q->internals->kni;
>> +uint16_t nb_pkts;
>> +
>> +nb_pkts = rte_kni_rx_burst(kni, bufs, nb_bufs);
>> +
>> +kni_q->rx.pkts += nb_pkts;
>> +kni_q->rx.err_pkts += nb_bufs - nb_pkts;
>> +
>> +return nb_pkts;
>> +}
>> +
> 
> I don't think it's safe to do receive from two queues concurrently on two 
> cores sharing the same underlying KNI device due to the current limitation of 
> KNI user-space queues not being multi-thread safe.

You are right, above code is not safe.
It is possible to create a KNI interface per queue, but I don't see any
advantage of this against creating a new virtual KNI port.

So I will limit to single queue.

> Is the proposed plan to have the application layer implement
synchronization logic?
> If that's the case, it needs to be clearly documented and depending on
the implementation, measurable overhead will be incurred.
> Otherwise (only single-queue supported), could you check queue number
if the application tries to configure multi-queue?
> 



<...>

>> +static struct rte_eth_dev *
>> +eth_kni_create(const char *name, unsigned int numa_node)
>> +{
>> +struct pmd_internals *internals = NULL;
>> +struct rte_eth_dev_data *data;
>> +struct rte_eth_dev *eth_dev;
>> +uint16_t nb_rx_queues = 1;
>> +uint16_t nb_tx_queues = 1;
> 
> Since these two values are always 1 here, I think they could be removed.

I will remove them.


Thanks,
ferruh

[dpdk-dev] [PATCH 1/2 v2] bnxt: use appropriate data type in bnxt_alloc_vnic_attributes

[Ferruh Yigit]

On 11/3/2016 6:58 PM, Ajit Khaparde wrote:
> Prevent the arithmetic in bnxt_alloc_vnic_attributes from causing
> any unintentional havoc because of the usage of a signed variable.
> 
> Coverity: 137874
> 
> Signed-off-by: Ajit Khaparde 
> 
> --
> v2: Previous attempt did not seem complete.
> ---
>  drivers/net/bnxt/bnxt_vnic.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
> index 205a940..23c85af 100644
> --- a/drivers/net/bnxt/bnxt_vnic.c
> +++ b/drivers/net/bnxt/bnxt_vnic.c
> @@ -179,7 +179,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
>   HW_HASH_INDEX_SIZE * sizeof(*vnic->rss_table) +
>   HW_HASH_KEY_SIZE);
>   uint16_t max_vnics;
> - int i;
> + uint16_t i;
>  
>   if (BNXT_PF(bp)) {
>   struct bnxt_pf_info *pf = >pf;
> @@ -197,7 +197,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
>   mz = rte_memzone_lookup(mz_name);
>   if (!mz) {
>   mz = rte_memzone_reserve(mz_name,
> -  entry_length * max_vnics,
> +  (uint32_t) entry_length * max_vnics,
>SOCKET_ID_ANY,
>RTE_MEMZONE_2MB |
>RTE_MEMZONE_SIZE_HINT_ONLY);
> @@ -210,10 +210,11 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
>  
>   /* Allocate rss table and hash key */
>   vnic->rss_table =
> - (void *)((char *)mz->addr + (entry_length * i));
> + (void *)((char *)mz->addr + ((uint32_t) entry_length * i));
>   memset(vnic->rss_table, -1, entry_length);
>  
> - vnic->rss_table_dma_addr = mz->phys_addr + (entry_length * i);
> + vnic->rss_table_dma_addr =
> + mz->phys_addr + ((uint32_t) entry_length * i);

((uint32_t) entry_length * i)
casting entry_length to uint32_t will prevent the integer promotion and
fix the coverity issue, but if this is the case, why not make
entry_length uint32_t at first place?

It seems "entry_length" converted from "int" to "uint16_t" to prevent
integer promotion (e8a197d2aa9a), but
"entry_length * max_vnics" => "uint16_t * uint16_t" or
"entry_length * i" => "uint16_t * int"
still causing the promotion.

I guess converting "entry_length" to uint32_t (instead of uint16_t) will
fix both triaged 127557 and new introduced 137874 coverity issues, and
it is simpler modification, - if I don't miss anything J

Thanks,
ferruh


>   vnic->rss_hash_key = (void *)((char *)vnic->rss_table +
>HW_HASH_INDEX_SIZE * sizeof(*vnic->rss_table));
>  
> 

[dpdk-dev] [PATCH] E1000: fix for forced speed/duplex config

[Ferruh Yigit]

Hi Ananda,

Thank you for the patch. Can you please take care a few minor issues?

Patch tag should be: "net/e1000:", so patch title becomes:
"net/e1000: fix for forced speed/duplex config"

On 11/1/2016 10:47 PM, Ananda Sathyanarayana wrote:
> From the code, it looks like, hw->mac.autoneg, variable is used to
> switch between calling either autoneg function or forcing
> speed/duplex function. But this variable is not modified in
> eth_em_start/eth_igb_start routines (it is always set to 1)
> even while forcing the link speed.
> 
> Following discussion thread has some more information on
> this
> 
> http://dpdk.org/ml/archives/dev/2016-October/049272.html

Requires a fixes line:
http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body

> 
> Signed-off-by: Ananda Sathyanarayana 

You can keep Wenzhuo's ack for next version of the patch.
Acked-by: Wenzhuo Lu 

> ---
>  drivers/net/e1000/em_ethdev.c  | 16 ++--
>  drivers/net/e1000/igb_ethdev.c | 16 ++--
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 7cf5f0c..a2412f5 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -639,6 +639,7 @@ eth_em_start(struct rte_eth_dev *dev)
>   speeds = >data->dev_conf.link_speeds;
>   if (*speeds == ETH_LINK_SPEED_AUTONEG) {
>   hw->phy.autoneg_advertised = E1000_ALL_SPEED_DUPLEX;
> +hw->mac.autoneg = 1;

checkpatch gives many whitespace errors.
>From coding style document:
"Global whitespace rule in DPDK, use tabs for indentation, spaces for
alignment."

And how to use checkpatch:
http://dpdk.org/doc/guides/contributing/patches.html#checking-the-patches

>   } else {
>   num_speeds = 0;
>   autoneg = (*speeds & ETH_LINK_SPEED_FIXED) == 0;
> @@ -672,9 +673,20 @@ eth_em_start(struct rte_eth_dev *dev)
>   hw->phy.autoneg_advertised |= ADVERTISE_1000_FULL;
>   num_speeds++;
>   }
> - if (num_speeds == 0 || (!autoneg && (num_speeds > 1)))
> + if (num_speeds == 0 || (!autoneg && (num_speeds > 1))) {

No need to update this line, dpdk coding style doesn't require
parenthesis for single line statement:
http://dpdk.org/doc/guides/contributing/coding_style.html#control-statements-and-loops

>   goto error_invalid_config;
> - }
> +}
> +/*
> + * Set/reset the mac.autoneg based on the link speed,
> + * fixed or not
> + */
> +if (!autoneg) {
> +hw->mac.autoneg = 0;
> +hw->mac.forced_speed_duplex = 
> hw->phy.autoneg_advertised;

This line over 80 character limit.

> +} else {
> +hw->mac.autoneg = 1;
> +}
> +}
>  
>   e1000_setup_link(hw);
>  
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c

Same comments valid for this file too.

Thanks,
ferruh

[dpdk-dev] [PATCH] igb_uio: fix build with backported kernel

[Ferruh Yigit]

On 11/2/2016 4:19 PM, martin_curran-gray at keysight.com wrote:
> Hi ,
> 
> Sorry, struggling to see what happened to this thread
> 
> I managed to get dpdk 2.2.0 to build on CentOs 6.8 by sorting the 
> MSIX_ENTRY_CTRL_MASKBIT
> 
> But I'm trying to get 16.7 to run on 6.8, and am hitting the   
> vlan_tx_tag_present(_skb)
> 
> I tried just putting a bare
> #define  vlan_tx_tag_present(_skb) 0
> line in the two kcompat.h files
> one for igb and one for ixgbe
> 
> but I'm hitting other issues now.
> 
> /root/mcgray/dpdk-16.07/x86_64-native-linuxapp-gcc/build/lib/librte_eal/linuxapp/kni/kni_misc.c:441:20:
>  error: macro "alloc_netdev" passed 4 arguments, but takes just 3
> 
> I had already turned of KNI in my config file, so why is the dpdk-setup.sh 
> even trying to build this stuff??

I guess it is not disabled properly. How are you disabling KNI?

> 
> I don't need KNI as far as I know
> 
> I saw mention of backported kernel?
> 
> I guess my 16.7 is a few months old now, if I go and get another download 
> will this all just go away?
> 
> Thanks
> 
> Sry, this stuff all a bit beyond my experience so far.
> 
> 
> 
> Martin Curran-Gray
> 

[dpdk-dev] [PATCH v3] net/ring: remove unnecessary NULL check

[Ferruh Yigit]

On 11/2/2016 1:46 PM, Mauricio Vasquez B wrote:
> Coverity detected this as an issue because internals->data will never be NULL,
> then the check is not necessary.
> 
> Fixes: d082c0395bf6 ("ring: fix memory leak when detaching")
> Coverity issue: 137873
> 
> Signed-off-by: Mauricio Vasquez B 
> ---

Acked-by: Ferruh Yigit 

[dpdk-dev] [PATCH v2] net/ring: remove unnecessary NULL check

[Ferruh Yigit]

On 11/2/2016 12:49 PM, Fulvio Risso wrote:
> Dear Ferruh,
> Maybe I'm wrong, but I cannot see your point.
> The code is absolutely the same, only the following line
> 
> if (eth_dev->data) {
> 
> is actually removed.

Please double check the condition "rx_queues" freed:

before the patch:
==
if (eth_dev->data) {
  internals = eth_dev->data->dev_private;
  if (internals->action == DEV_CREATE) {
/*
 * it is only necessary to delete the rings in rx_queues because
 * they are the same used in tx_queues
 */
for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
  r = eth_dev->data->rx_queues[i];
  rte_ring_free(r->rng);
}
  }

  rte_free(eth_dev->data->rx_queues);
  rte_free(eth_dev->data->tx_queues);
  rte_free(eth_dev->data->dev_private);
}
==


After the patch:
==
internals = eth_dev->data->dev_private;
if (internals->action == DEV_CREATE) {
  /*
   * it is only necessary to delete the rings in rx_queues because
   * they are the same used in tx_queues
   */
  for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
r = eth_dev->data->rx_queues[i];
rte_ring_free(r->rng);
  }

  rte_free(eth_dev->data->rx_queues);
  rte_free(eth_dev->data->tx_queues);
  rte_free(eth_dev->data->dev_private);
}
==


Thanks,
ferruh

[dpdk-dev] [PATCH v2] net/ring: remove unnecessary NULL check

[Ferruh Yigit]

Hi Mauricio,

On 11/1/2016 7:55 PM, Mauricio Vasquez B wrote:
> Coverity detected this as an issue because internals->data will never be NULL,
> then the check is not necessary.
> 
> Fixes: d082c0395bf6 ("ring: fix memory leak when detaching")
> Coverity issue: 137873
> 
> Signed-off-by: Mauricio Vasquez B 
> ---
>  drivers/net/ring/rte_eth_ring.c | 20 +---
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> index 6d2a8c1..5ca00ed 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -599,17 +599,15 @@ rte_pmd_ring_remove(const char *name)
>  
>   eth_dev_stop(eth_dev);
>  
> - if (eth_dev->data) {
> - internals = eth_dev->data->dev_private;
> - if (internals->action == DEV_CREATE) {
> - /*
> -  * it is only necessary to delete the rings in 
> rx_queues because
> -  * they are the same used in tx_queues
> -  */
> - for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> - r = eth_dev->data->rx_queues[i];
> - rte_ring_free(r->rng);
> - }
> + internals = eth_dev->data->dev_private;
> + if (internals->action == DEV_CREATE) {
> + /*
> +  * it is only necessary to delete the rings in rx_queues because
> +  * they are the same used in tx_queues
> +  */
> + for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> + r = eth_dev->data->rx_queues[i];
> + rte_ring_free(r->rng);
>   }
>  
>   rte_free(eth_dev->data->rx_queues);

This patch not only removes the NULL check but also changes the logic.
after patch rx_queues, tx_queues and dev_private only freed if action is
DEV_CREATE which is wrong.

> 

[dpdk-dev] [PATCH] net: remove mempool as a dependency

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit 
---
 lib/librte_net/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile
index e5758ce..20cf664 100644
--- a/lib/librte_net/Makefile
+++ b/lib/librte_net/Makefile
@@ -45,7 +45,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include := rte_ip.h 
rte_tcp.h rte_udp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_sctp.h rte_icmp.h rte_arp.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include += rte_ether.h rte_gre.h rte_net.h

-DEPDIRS-$(CONFIG_RTE_LIBRTE_NET) += lib/librte_eal lib/librte_mempool
-DEPDIRS-$(CONFIG_RTE_LIBRTE_NET) += lib/librte_mbuf
+DEPDIRS-$(CONFIG_RTE_LIBRTE_NET) += lib/librte_eal lib/librte_mbuf

 include $(RTE_SDK)/mk/rte.lib.mk
-- 
2.7.4

[dpdk-dev] [PATCH] doc: add missing library to release notes

[Ferruh Yigit]

Signed-off-by: Ferruh Yigit 
---
CC: Olivier Matz 
---
 doc/guides/rel_notes/release_16_11.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/rel_notes/release_16_11.rst 
b/doc/guides/rel_notes/release_16_11.rst
index aa0c09a..5f185be 100644
--- a/doc/guides/rel_notes/release_16_11.rst
+++ b/doc/guides/rel_notes/release_16_11.rst
@@ -248,6 +248,7 @@ The libraries prepended with a plus sign were incremented 
in this version.
  librte_mbuf.so.2
  librte_mempool.so.2
  librte_meter.so.1
+ librte_net.so.1
  librte_pdump.so.1
  librte_pipeline.so.3
  librte_pmd_bond.so.1
-- 
2.7.4

[dpdk-dev] [PATCH] net/mlx5: fix wrong use of vector instruction

[Ferruh Yigit]

On 11/1/2016 8:13 AM, Elad Persiko wrote:
> Constraint alignment was not respected in Tx.
> 
> Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> 
> Signed-off-by: Elad Persiko 
> ---
>  drivers/net/mlx5/mlx5_rxtx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index 21164ba..ba8e202 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -309,7 +309,7 @@ mlx5_tx_dbrec(struct txq *txq)
>   *txq->qp_db = htonl(txq->wqe_ci);
>   /* Ensure ordering between DB record and BF copy. */
>   rte_wmb();
> - rte_mov16(dst, (uint8_t *)data);
> + memcpy(dst, (uint8_t *)data, 16);
>   txq->bf_offset ^= (1 << txq->bf_buf_size);
>  }
>  
> @@ -449,7 +449,7 @@ mlx5_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, 
> uint16_t pkts_n)
>   wqe->eseg.mss = 0;
>   wqe->eseg.rsvd2 = 0;
>   /* Start by copying the Ethernet Header. */
> - rte_mov16((uint8_t *)raw, (uint8_t *)addr);
> + memcpy((uint8_t *)raw, ((uint8_t *)addr), 16);
>   length -= MLX5_WQE_DWORD_SIZE;
>   addr += MLX5_WQE_DWORD_SIZE;
>   /* Replace the Ethernet type by the VLAN if necessary. */
> 

CC: Maintainers (Adrien Mazarguil )

[dpdk-dev] [PATCH] pmd_ring: fix coverity issue

[Ferruh Yigit]

Hi Mauricio,

On 11/1/2016 3:48 AM, Mauricio Vasquez B wrote:
> internals->data will never be NULL, so the check is not necessary.
> 
> Fixes: d082c0395bf6 ("ring: fix memory leak when detaching")
> Coverity issue: 137873
> 
> Signed-off-by: Mauricio Vasquez B 
> ---

Thank you for the patch.
But "fix coverity issue" is not very descriptive title on its own.

Can you please describe what is really done in the patch, perhaps
something like:
"net/ring: remove unnecessary NULL check" ?

Thanks,
ferruh

[dpdk-dev] [PATCH] net/bonding: not handle vlan slow packet

[Ferruh Yigit]

Hi Haifeng,

On 10/31/2016 3:52 AM, linhaifeng wrote:
> From: Haifeng Lin 
> 
> if rx vlan offload is enable we should not handle vlan slow
> packets too.
> 
> Signed-off-by: Haifeng Lin 
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
> b/drivers/net/bonding/rte_eth_bond_pmd.c
> index 09ce7bf..ca17898 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -169,7 +169,8 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf 
> **bufs,
>   /* Remove packet from array if it is slow packet or 
> slave is not
>* in collecting state or bondign interface is not in 
> promiscus
>* mode and packet address does not match. */
> - if (unlikely(hdr->ether_type == ether_type_slow_be ||
> + if (unlikely((hdr->ether_type == ether_type_slow_be &&
> + !bufs[j]->vlan_tci) ||
>   !collecting || (!promisc &&
>   !is_multicast_ether_addr(>d_addr) 
> &&
>   !is_same_ether_addr(_mac, 
> >d_addr {
> 

There are a few version of this patch, I guess this one is the correct
one, can you please confirm?
Also this one supersede following one, right?
http://dpdk.org/dev/patchwork/patch/16840/

It helps a lot if you use versioning in the patches [PATCH -vN] and add
a description of changes in commit log (after "---") between patch versions.

Thanks,
ferruh

[dpdk-dev] KNI discussion in userspace event

[Ferruh Yigit]

Hi,

There was an "Interworking with the Linux Kernel" discussion in the DPDK
userspace event, this mail is to summarize the output and to get more
comments from community.


Briefly, KNI mostly will stay as it is as an interworking with the Linux
kernel solution.
Out of tree kernel module concern is still there, but there is no clear
alternative to switch. And community still care about performance of KNI
and control path of KNI. Only KNI VHOST may go away. KNI PMD depends on
community interest. There was no modification request on KNI library and
sample app.


Discussed alternatives were:
* Tun/Tap
This won't be as fast as KNI and performance is an issue.

* virtio-user + vhost-net
This can be valid alternative, removes the out of tree kernel module
need. But missing control path. Proof of concept work will be done.

* Bifurcated driver
Not able to filter all traffic, not a full functional alternative.

* Upstreaming kernel module:
Stephen suggested upstreaming a generic shim layer and use it.


Future of the KNI:
* Remove ethtool support ?
Still there is some interest, will keep it. But not able to extend it to
other drivers with current design.

* Remove KNI VHOST?
There were no interest for this feature. I will send a deprecation
notice to remove this, and we can discuss more there.

* What to do with out of tree kernel module
It is still problem for OSVs and unfortunately it is staying.

* Switch completely to an alternative approach?
There won't be an action for a switch. virtio-user + vhost-net
alternative will be investigated.

*KNI PMD
Patch is in the mail list, missing comments. If it gets some
interest/comments/acks it may go in to next release.

* Any improvement on library or sample app?
Nothing listed.


Thanks,
ferruh

[dpdk-dev] [PATCH v2] eal: fix libabi macro for device generalization patches

[Ferruh Yigit]

Hi Shreyansh,

On 10/26/2016 2:12 PM, Shreyansh Jain wrote:
> On Wednesday 26 October 2016 06:30 PM, Shreyansh Jain wrote:
>> rte_device/driver generalization patches [1] were merged without a change
>> in the LIBABIVER macro. This patches bumps the macro of affected libs.
>>
>> Also, deprecation notice from 16.07 has been removed and release notes for
>> 16.11 added.
>>
>> Signed-off-by: Shreyansh Jain 
>> --
>> v2:
>>  - Mark bumped libraries in release_16_11.rst file
>>  - change code symbol names from text to code layout
>>
>> ---

<...>

>>  .. code-block:: diff
>>
>> - libethdev.so.4
>> +   + libethdev.so.4
> 
> Just noticed:
> Should the '4' here reflect the current LIBABIVER number?
> If so, I will send this patch again.

Yes, as you guessed, it should be:
- libethdev.so.4
+   + libethdev.so.5

<...>

>> diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
>> b/lib/librte_eal/bsdapp/eal/Makefile
>> index a15b762..122798c 100644
>> --- a/lib/librte_eal/bsdapp/eal/Makefile
>> +++ b/lib/librte_eal/bsdapp/eal/Makefile
>> @@ -48,7 +48,7 @@ LDLIBS += -lgcc_s
>>
>>  EXPORT_MAP := rte_eal_version.map
>>
>> -LIBABIVER := 3
>> +LIBABIVER := 4

eal version seems already increased for this release, 2 => 3, in:
d7e61ad3ae36 ("log: remove deprecated history dump")

So NO need to increase it again, sorry for late notice, I just
recognized it.
Only librte_ether and librte_cryptodev requires the increase.

<...>

Thanks,
ferruh

[dpdk-dev] [PATCH RFC 0/2] Allow vectorized Rx with 4096 desc ring size on Intel NICs.

[Ferruh Yigit]

On 10/19/2016 3:07 PM, Ilya Maximets wrote:
> Ilya Maximets (2):
>   net/i40e: allow bulk alloc for the max size desc ring
>   net/ixgbe: allow bulk alloc for the max size desc ring
> 
>  drivers/net/i40e/i40e_rxtx.c   | 24 +---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 17 +
>  drivers/net/ixgbe/ixgbe_rxtx.h |  2 +-
>  3 files changed, 15 insertions(+), 28 deletions(-)
> 

Hi Ilya,

Thank you for the patch, we are in post rc1 phase and this is a new
feature, so this patchset will be considered for next release (v17.02).

Thanks,
ferruh

[dpdk-dev] [PATCH v4 04/32] net/qede/base: add HSI changes and register defines

[Ferruh Yigit]

On 10/19/2016 5:11 AM, Rasesh Mody wrote:
>  - add the hardware software interface(HSI) changes
>  - add register definitions
> 
> These will be required for 8.10.9.0 FW upgrade.
> 
> Signed-off-by: Rasesh Mody 

<...>

>  /*
>   * Igu cleanup bit values to distinguish between clean or producer consumer
>   */
> @@ -1376,7 +1572,7 @@ struct dmae_cmd {
>   __le32 src_addr_hi;
>   __le32 dst_addr_lo;
>   __le32 dst_addr_hi;
> - __le16 length /* Length in DW */;
> + __le16 length_dw /* Length in DW */;

This cause a compilation error [1] in patch by patch compilation, when
debug is enabled. Fix seems straightforward [2].

Do you confirm the fix?
If you confirm, you don't need to send a new version, fix can be applied
while getting the patch.

Thanks,
ferruh


[1]:

In file included from .../drivers/net/qede/base/ecore.h:36:0,
 from .../drivers/net/qede/base/ecore_hw.c:12:
.../drivers/net/qede/base/ecore_hw.c: In function ?ecore_dmae_post_command?:
.../drivers/net/qede/base/ecore_hw.c:478:20: error: ?struct dmae_cmd?
has no member named ?length?; did you mean ?length_dw??
  (int)p_command->length,
^
.../drivers/net/qede/base/../qede_logs.h:47:11: note: in definition of
macro ?DP_VERBOSE?
 ##__VA_ARGS__); \
   ^~~
.../mk/internal/rte.compile-pre.mk:138: recipe for target
'base/ecore_hw.o' failed


[2]:

diff --git a/drivers/net/qede/base/ecore_hw.c
b/drivers/net/qede/base/ecore_hw.c
index 72bc6de..5412ed1 100644
--- a/drivers/net/qede/base/ecore_hw.c
+++ b/drivers/net/qede/base/ecore_hw.c
@@ -475,7 +475,7 @@ ecore_dmae_post_command(struct ecore_hwfn *p_hwfn,
struct ecore_ptt *p_ptt)
   "len=0x%x src=0x%x:%x dst=0x%x:%x\n",
   idx_cmd, (u32)p_command->opcode,
   (u16)p_command->opcode_b,
-  (int)p_command->length,
+  (int)p_command->length_dw,
   (int)p_command->src_addr_hi,
   (int)p_command->src_addr_lo,
   (int)p_command->dst_addr_hi,
(int)p_command->dst_addr_lo);

[dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt

[Ferruh Yigit]

Hi Qiming,

On 10/13/2016 7:07 AM, Qiming Yang wrote:
> Previously, link status interrupt in i40e is achieved by checking
> LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only
> for diagnostic use. Instead, drivers need to get the link status
> change notification by using LSE (Link Status Event).
> 
> This patch enables LSE and calls LSC callback when the event is
> received. This patch also removes the processing on
> LINK_STAT_CHANGE_MASK.
> 
> Fixes: 5c9222058df7 ("i40e: move to drivers/net/")
> 
> Signed-off-by: Qiming Yang 

CC: Jingjing Wu 

Can you please add maintainer(s) to CC when sending a patch, mail
traffic in dpdk.org is keep increasing and it is hard to follow. Adding
maintainer to the CC helps a lot to the maintainer.

I guess everybody knows, but just as a reminder, maintainer list kept in
file: http://dpdk.org/browse/dpdk/tree/MAINTAINERS


Thanks,
ferruh

[dpdk-dev] [PATCH v2 2/2] drivers/i40e: fix the hash filter invalid calculation in X722

[Ferruh Yigit]

On 10/16/2016 2:40 AM, Jeff Guo wrote:
> As X722 extracts IPv4 header to Field Vector different with XL710/X710,
> need to corresponding to modify the fields of IPv4 header in input set
> to map different default Field Vector Table of different NICs.
> 
> Signed-off-by: Jeff Guo 
> 
> ---
> v2:
> fix compile error when x722 macro is not defined and simplify
> the code to avoid duplication.
> ---
>  drivers/net/i40e/i40e_ethdev.c | 73 
> ++
>  1 file changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 920fd6d..7895c11 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -211,6 +211,16 @@
>  #define I40E_REG_INSET_L3_SRC_IP40x00018000ULL
>  /* Destination IPv4 address */
>  #define I40E_REG_INSET_L3_DST_IP40x0018ULL
> +#ifdef X722_SUPPORT

If agreed on removing #ifdef X722_SUPPORT, please apply this into this
patch too.

> +/* Source IPv4 address for X722 */
> +#define I40E_X722_REG_INSET_L3_SRC_IP4   0x0006ULL
> +/* Destination IPv4 address for X722 */
> +#define I40E_X722_REG_INSET_L3_DST_IP4   0x0600ULL
> +/* IPv4 Protocol */
> +#define I40E_X722_REG_INSET_L3_IP4_PROTO 0x0010ULL
> +/* IPv4 Time to Live */
> +#define I40E_X722_REG_INSET_L3_IP4_TTL   0x0010ULL
> +#endif
>  /* IPv4 Type of Service (TOS) */
>  #define I40E_REG_INSET_L3_IP4_TOS0x0040ULL
>  /* IPv4 Protocol */
> @@ -7581,7 +7591,7 @@ i40e_parse_input_set(uint64_t *inset,
>   * and vice versa
>   */
>  static uint64_t
> -i40e_translate_input_set_reg(uint64_t input)
> +i40e_translate_input_set_reg(enum i40e_mac_type type, uint64_t input)
>  {
>   uint64_t val = 0;
>   uint16_t i;
> @@ -7589,17 +7599,13 @@ i40e_translate_input_set_reg(uint64_t input)
>   static const struct {
>   uint64_t inset;
>   uint64_t inset_reg;
> - } inset_map[] = {
> + } inset_map_common[] = {
>   {I40E_INSET_DMAC, I40E_REG_INSET_L2_DMAC},
>   {I40E_INSET_SMAC, I40E_REG_INSET_L2_SMAC},
>   {I40E_INSET_VLAN_OUTER, I40E_REG_INSET_L2_OUTER_VLAN},
>   {I40E_INSET_VLAN_INNER, I40E_REG_INSET_L2_INNER_VLAN},
>   {I40E_INSET_LAST_ETHER_TYPE, I40E_REG_INSET_LAST_ETHER_TYPE},
> - {I40E_INSET_IPV4_SRC, I40E_REG_INSET_L3_SRC_IP4},
> - {I40E_INSET_IPV4_DST, I40E_REG_INSET_L3_DST_IP4},
>   {I40E_INSET_IPV4_TOS, I40E_REG_INSET_L3_IP4_TOS},
> - {I40E_INSET_IPV4_PROTO, I40E_REG_INSET_L3_IP4_PROTO},
> - {I40E_INSET_IPV4_TTL, I40E_REG_INSET_L3_IP4_TTL},
>   {I40E_INSET_IPV6_SRC, I40E_REG_INSET_L3_SRC_IP6},
>   {I40E_INSET_IPV6_DST, I40E_REG_INSET_L3_DST_IP6},
>   {I40E_INSET_IPV6_TC, I40E_REG_INSET_L3_IP6_TC},
> @@ -7627,16 +7633,56 @@ i40e_translate_input_set_reg(uint64_t input)
>   {I40E_INSET_FLEX_PAYLOAD_W7, I40E_REG_INSET_FLEX_PAYLOAD_WORD7},
>   {I40E_INSET_FLEX_PAYLOAD_W8, I40E_REG_INSET_FLEX_PAYLOAD_WORD8},
>   };
> +#ifdef X722_SUPPORT
> +
> +/* some different registers map in x722*/
> + static const struct {
> + uint64_t inset;
> + uint64_t inset_reg;
> + } inset_map_diff_x722[] = {

Can you please extract struct declaration, and re-use that to create
instances.

> + {I40E_INSET_IPV4_SRC, I40E_X722_REG_INSET_L3_SRC_IP4},
> + {I40E_INSET_IPV4_DST, I40E_X722_REG_INSET_L3_DST_IP4},
> + {I40E_INSET_IPV4_PROTO, I40E_X722_REG_INSET_L3_IP4_PROTO},
> + {I40E_INSET_IPV4_TTL, I40E_X722_REG_INSET_L3_IP4_TTL},
> + };
> +#endif
> +
> + static const struct {
> + uint64_t inset;
> + uint64_t inset_reg;
> + } inset_map_diff_not_x722[] = {
> + {I40E_INSET_IPV4_SRC, I40E_REG_INSET_L3_SRC_IP4},
> + {I40E_INSET_IPV4_DST, I40E_REG_INSET_L3_DST_IP4},
> + {I40E_INSET_IPV4_PROTO, I40E_REG_INSET_L3_IP4_PROTO},
> + {I40E_INSET_IPV4_TTL, I40E_REG_INSET_L3_IP4_TTL},
> + };
>  
>   if (input == 0)
>   return val;
>  
>   /* Translate input set to register aware inset */
> - for (i = 0; i < RTE_DIM(inset_map); i++) {
> - if (input & inset_map[i].inset)
> - val |= inset_map[i].inset_reg;
> +#ifdef X722_SUPPORT
> + if (type == I40E_MAC_X722) {
> + for (i = 0; i < RTE_DIM(inset_map_diff_x722); i++) {
> + if (input & inset_map_diff_x722[i].inset)
> + val |= inset_map_diff_x722[i].inset_reg;
> + }
> + } else {
> + for (i = 0; i < RTE_DIM(inset_map_diff_not_x722); i++) {
> + if (input & inset_map_diff_not_x722[i].inset)
> +

[dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile

[Ferruh Yigit]

On 10/17/2016 10:54 AM, Ananyev, Konstantin wrote:
> Hi Jeff,
> 
>>
>> hi, Konstantin
>> Thanks your constructive suggestion. I don't think your question is
>> silly and we also think about the code style simply and effective, but
>> may be i would interpret the reason why we do that.
>>
>> 1) Sure, user definitely can choose to define the macro or not when
>> building dpdk i40e PMD, but i don't think it is
>> necessary to invoke a ret_config option to let up layer user freedom use
>> it,  because only the older version i40e driver does not support X722,
>> the newer version i40e driver will always support X722, so the macro
>> will be default hard code in the makefile. and we will use mac.type to
>> distinguish the difference register configure in run time. So we may
>> consider the macro just like a flag that highlight the difference of the
>> shared code between X710 and X722, that would benify the X710/X722 pmd
>> development but hardly no use to exposure to the up layer user.
>>
>> 2)  i think the answer also could find from above. But i think if we
>> develop go to a certain stage in the future, mute the macro or use
>> script to remove them like the way from hw driver, for support all
>> device types maybe not a bad idea, right?
> 
> Sorry, but I still didn't get it.
> If i40e driver will always support X722 then why do we need that macro at all?
> Why just not to remove it completely then?
> Same about run-time vs build-time choice:
> If let say i40e_get_rss_key() has to behave in a different way, why not to 
> create
> i40e_get_rss_key_x722() and use it when hw mactype is x7222?
> Or at least inside i40e_get_rss_key() do something like:
> if (hw->mac.type == I40E_MAC_X722) {...} else {...}
> ?
> Why instead you have to pollute whole i40e code with all these #ifdef 
> x7222/#else ...?
> Obviously that looks pretty ugly and hard to maintain.

It is not possible to remove "#ifdef x7222" from shared code, but what
about removing it from DPDK piece of the code, and code as it is always
defined?

If this is OK, this patch is not more required.
And the removing #ifdef work can be done in another patch later.

> Konstantin