DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] use config flags for target OS checks
From: Thomas Monjalon @ 2016-12-05 17:50 UTC (permalink / raw)
  To: dev

Clean up the code to always use the flags RTE_EXEC_ENV_*APP
set up in the DPDK configuration instead of the native ones.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test-pmd/cmdline.c                       | 4 ++--
 app/test/commands.c                          | 4 ++--
 app/test/test_cmdline_ipaddr.c               | 4 ++--
 drivers/net/bnx2x/bnx2x.c                    | 4 ++--
 drivers/net/bnx2x/bnx2x.h                    | 4 ++--
 examples/cmdline/commands.c                  | 4 ++--
 lib/librte_cmdline/cmdline_parse_etheraddr.c | 2 +-
 lib/librte_cmdline/cmdline_parse_ipaddr.c    | 4 ++--
 lib/librte_eal/common/include/rte_lcore.h    | 4 ++--
 9 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 63b55dc..7f3667d 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -41,8 +41,8 @@
 #include <termios.h>
 #include <unistd.h>
 #include <inttypes.h>
-#ifndef __linux__
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_LINUXAPP
+#ifndef RTE_EXEC_ENV_BSDAPP
 #include <net/socket.h>
 #else
 #include <sys/socket.h>
diff --git a/app/test/commands.c b/app/test/commands.c
index 2df46b0..da0c035 100644
--- a/app/test/commands.c
+++ b/app/test/commands.c
@@ -39,8 +39,8 @@
 #include <stdlib.h>
 #include <netinet/in.h>
 #include <termios.h>
-#ifndef __linux__
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_LINUXAPP
+#ifndef RTE_EXEC_ENV_BSDAPP
 #include <net/socket.h>
 #endif
 #endif
diff --git a/app/test/test_cmdline_ipaddr.c b/app/test/test_cmdline_ipaddr.c
index 471d2ff..16570fb 100644
--- a/app/test/test_cmdline_ipaddr.c
+++ b/app/test/test_cmdline_ipaddr.c
@@ -36,8 +36,8 @@
 #include <inttypes.h>
 #include <netinet/in.h>
 
-#ifndef __linux__
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_LINUXAPP
+#ifndef RTE_EXEC_ENV_BSDAPP
 #include <net/socket.h>
 #else
 #include <sys/socket.h>
diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 2856630..47cf5d3 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -9484,7 +9484,7 @@ static int bnx2x_pci_get_caps(struct bnx2x_softc *sc)
 		return -ENOMEM;
 	}
 
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_BSDAPP
 	pci_read(sc, PCI_STATUS, &status, 2);
 	if (!(status & PCI_STATUS_CAP_LIST)) {
 #else
@@ -9495,7 +9495,7 @@ static int bnx2x_pci_get_caps(struct bnx2x_softc *sc)
 		return -1;
 	}
 
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_BSDAPP
 	pci_read(sc, PCI_CAPABILITY_LIST, &pci_cap.next, 1);
 #else
 	pci_read(sc, PCIR_CAP_PTR, &pci_cap.next, 1);
diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 5cefea4..006ad28 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -41,7 +41,7 @@
 
 #include "elink.h"
 
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_BSDAPP
 #include <linux/pci_regs.h>
 
 #define PCIY_PMG                       PCI_CAP_ID_PM
@@ -71,7 +71,7 @@
 #define IFM_10G_TWINAX                 22 /* 10GBase Twinax copper */
 #define IFM_10G_T                      26 /* 10GBase-T - RJ45 */
 
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_BSDAPP
 #define PCIR_EXPRESS_DEVICE_STA        PCI_EXP_TYPE_RC_EC
 #define PCIM_EXP_STA_TRANSACTION_PND   PCI_EXP_DEVSTA_TRPND
 #define PCIR_EXPRESS_LINK_STA          PCI_EXP_LNKSTA
diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c
index f3ba247..205a205 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -66,8 +66,8 @@
 #include <errno.h>
 #include <netinet/in.h>
 #include <termios.h>
-#ifndef __linux__
-	#ifdef __FreeBSD__
+#ifndef RTE_EXEC_ENV_LINUXAPP
+	#ifdef RTE_EXEC_ENV_BSDAPP
 		#include <sys/socket.h>
 	#else
 		#include <net/socket.h>
diff --git a/lib/librte_cmdline/cmdline_parse_etheraddr.c b/lib/librte_cmdline/cmdline_parse_etheraddr.c
index dbfe4a6..1f26426 100644
--- a/lib/librte_cmdline/cmdline_parse_etheraddr.c
+++ b/lib/librte_cmdline/cmdline_parse_etheraddr.c
@@ -85,7 +85,7 @@ struct cmdline_token_ops cmdline_token_etheraddr_ops = {
 #define ETHER_ADDRSTRLENLONG 18
 #define ETHER_ADDRSTRLENSHORT 15
 
-#ifdef __linux__
+#ifdef RTE_EXEC_ENV_LINUXAPP
 #define ea_oct ether_addr_octet
 #else
 #define ea_oct octet
diff --git a/lib/librte_cmdline/cmdline_parse_ipaddr.c b/lib/librte_cmdline/cmdline_parse_ipaddr.c
index d3d3e04..4ef1c57 100644
--- a/lib/librte_cmdline/cmdline_parse_ipaddr.c
+++ b/lib/librte_cmdline/cmdline_parse_ipaddr.c
@@ -86,8 +86,8 @@
 #include <string.h>
 #include <errno.h>
 #include <netinet/in.h>
-#ifndef __linux__
-#ifndef __FreeBSD__
+#ifndef RTE_EXEC_ENV_LINUXAPP
+#ifndef RTE_EXEC_ENV_BSDAPP
 #include <net/socket.h>
 #else
 #include <sys/socket.h>
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index fe7b586..f74bc0c 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -50,9 +50,9 @@ extern "C" {
 
 #define LCORE_ID_ANY     UINT32_MAX       /**< Any lcore. */
 
-#if defined(__linux__)
+#if defined(RTE_EXEC_ENV_LINUXAPP)
 	typedef	cpu_set_t rte_cpuset_t;
-#elif defined(__FreeBSD__)
+#elif defined(RTE_EXEC_ENV_BSDAPP)
 #include <pthread_np.h>
 	typedef cpuset_t rte_cpuset_t;
 #endif
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH 01/32] doc: add dpaa2 nic details
From: Mcnamara, John @ 2016-12-05 17:12 UTC (permalink / raw)
  To: Hemant Agrawal, dev@dpdk.org
  Cc: thomas.monjalon@6wind.com, Richardson, Bruce,
	shreyansh.jain@nxp.com
In-Reply-To: <1480875447-23680-2-git-send-email-hemant.agrawal@nxp.com>

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> Sent: Sunday, December 4, 2016 6:17 PM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; Richardson, Bruce
> <bruce.richardson@intel.com>; shreyansh.jain@nxp.com; Hemant Agrawal
> <hemant.agrawal@nxp.com>
> Subject: [dpdk-dev] [PATCH 01/32] doc: add dpaa2 nic details
> 
> Add the dpaa2 architecture and pmd details
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>


Thanks for the detailed doc. 

There is a whitespace warning on merge. Also, some comments below.

> +
> +DPAA2 Poll Mode Driver
> +===============================
> +

The underline should match the length of the title. Also, there should be
1 newline after each section header. This is in this example but there are
several without it.



> +
> +NXP DPAA2 (Data Path Acceleration Architecture Gen2)
> +----------------------------------------------------
> +
> +This section provides an overview of the NXP DPAA2 architecture and how
> +it is integrated into the DPDK.

The text says "This section" which implies that the following sections are
subsections. In that case the underline for the sub sections should probably 
be: ~~~~~~~~~~~~~~~~

http://dpdk.org/doc/guides/contributing/documentation.html#section-headers

This applies to some other section introductions below as well.

> +
> +Contents summary
> +- DPAA2 overview
> +- Overview of DPAA2 objects
> +- DPAA2 driver architecture overview

This needs a newline after "Contents summary" to generate a list.


> +
> +DPAA2 Overview
> +--------------
> +Refer: `FSL MC BUS in Linux Kernel
> <https://www.kernel.org/doc/readme/drivers-staging-fsl-mc-README.txt>`_.

s/Refer/Reference/

Note, I was very impressed with this doc until I saw that the text was all
in the other README file. :-)


> +DPAA2 is a hardware architecture designed for high-speeed network

s/speeed/speed/



> +  Toggle display of generic debugging messages
> +
> +- ``CONFIG_RTE_LIBRTE_DPAA2_USE_PHYS_IOVA`` (default ``y``)
> +
> +  Toggle to use physical address vs virtual address for hardware
> acceleraters.

s/acceleraters/accelerators/



> +
> +Driver Compilation
> +~~~~~~~~~~~~~~~~~~
> +
> +To compile the DPAA2 PMD for Linux arm64 gcc target, run the following
> +“make” command:

Non-ascii/non-Unicode characters here. Use ``make`` instead.


> +
> +# configure the resource container
> +
> +      configure resources in MC and create the DPRC container
> +      export the DPRC container
> +      e.g. export DPRCT=dprc.2

Should be #. and have a console block or directive like the example below.



> +
> +#. Start ``testpmd`` with basic parameters:
> +
> +   .. code-block:: console
> +
> +      ./arm64-dpaa2-linuxapp-gcc/testpmd -c 0xff -n 1 \
> +        -- -i --portmask=0x1 --nb-cores=1 --no-flush-rx
> +
> +




^ permalink raw reply

* Re: [PATCH v12 0/6] add Tx preparation
From: Ananyev, Konstantin @ 2016-12-05 16:43 UTC (permalink / raw)
  To: Adrien Mazarguil
  Cc: Thomas Monjalon, dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd,
	Jan Medala, Jakub Palider, John Daley, Alejandro Lucero,
	Harish Patil, Rasesh Mody, Jerin Jacob, Yuanhan Liu, Yong Wang,
	Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <20161205150327.GP10340@6wind.com>

Hi Adrien,

> 
> Hi Konstantin,
> 
> On Fri, Dec 02, 2016 at 01:00:55AM +0000, Ananyev, Konstantin wrote:
> [...]
> > > On Wed, Nov 30, 2016 at 10:54:50AM +0000, Ananyev, Konstantin wrote:
> > > [...]
> > > > > Something is definitely needed here, and only PMDs can provide it. I think
> > > > > applications should not have to clear checksum fields or initialize them to
> > > > > some magic value, same goes for any other offload or hardware limitation
> > > > > that needs to be worked around.
> > > > >
> > > > > tx_prep() is one possible answer to this issue, however as mentioned in the
> > > > > original patch it can be very expensive if exposed by the PMD.
> > > > >
> > > > > Another issue I'm more concerned about is the way limitations are managed
> > > > > (struct rte_eth_desc_lim). While not officially tied to tx_prep(), this
> > > > > structure contains new fields that are only relevant to a few devices, and I
> > > > > fear it will keep growing with each new hardware quirk to manage, breaking
> > > > > ABIs in the process.
> > > >
> > > > Well, if some new HW capability/limitation would arise and we'd like to support
> > > > it in DPDK, then yes we probably would need to think how to incorporate it here.
> > > > Do you have anything particular in mind here?
> > >
> > > Nothing in particular, so for the sake of the argument, let's suppose that I
> > > would like to add a field to expose some limitation that only applies to my
> > > PMD during TX but looks generic enough to make sense, e.g. maximum packet
> > > size when VLAN tagging is requested.
> >
> > Hmm, I didn't hear about such limitations so far, but if it is real case -
> > sure, feel free to submit the patch.
> 
> I won't, that was hypothetical.

Then why we discussing it? :)

> 
> > > PMDs are free to set that field to some
> > > special value (say, 0) if they do not care.
> > >
> > > Since that field exists however, conscious applications should check its
> > > value for each packet that needs to be transmitted. This extra code causes a
> > > slowdown just by sitting in the data path. Since it is not the only field in
> > > that structure, the performance impact can be significant.
> > >
> > > Even though this code is inside applications, it remains unfair to PMDs for
> > > which these tests are irrelevant. This problem is identified and addressed
> > > by tx_prepare().
> >
> > I suppose the question is why do we need:
> > uint16_t nb_seg_max;
> > uint16_t nb_mtu_seg_max;
> > as we now have tx_prepare(), right?
> >
> > For two reasons:
> > 1. Some people might feel that tx_prepare() is not good (smart/fast) enough
> > for them and would prefer to do necessary preparations for TX offloads themselves.
> >
> > 2. Even if people do use tx_prepare() they still should take this information into accout.
> > As an example ixgbe can't TX packets with then 40 segments.
> > Obviously ixbge_tx_prep() performs that check and returns an error.
> 
> Problem is that tx_prepare() also provides safeties which are not part of
> tx_burst(), such as not going over nb_mtu_seg_max. Because of this and the
> fact struct rte_eth_desc_lim can grow new fields anytime, application
> developers will be tempted to just call tx_prepare() and focus on more
> useful things.

NP with that, that was an intention beyond introducing it.

> 
> Put another way, from a user's point of view, tx_prepare() is an opaque
> function that greatly increases tx_burst()'s ability to send mbufs as
> requested, with extra error checking on top; applications not written to run
> on a specific PMD/device (all of them ideally) will thus call tx_prepare()
> at some point.
> 
> > But it wouldn't try to merge/reallocate mbufs for you.
> > User still has to do it himself, or just prevent creating such long chains somehow.
> 
> Yes, that's another debate. PMDs could still implement a software fallback
> for unlikely slow events like these. The number of PMDs is not going to
> decrease, each device having its own set of weird limitations in specific
> cases, PMDs should do their best to process mbufs even if that means slowly
> due to the lack of preparation.
> 
> tx_prepare() has its uses but should really be optional, in the sense that
> if that function is not called, tx_burst() should deal with it somehow.

As I said before, I don't think it is a good idea to put everything in tx_burst().
If PMD driver prefer things that way, yes tx_burst() can deal with each and
possible offload requirement itself, but it shouldn't be mandatory. 

> 
> > > Thanks to tx_prepare(), these checks are moved back into PMDs where they
> > > belong. PMDs that do not need them do not have to provide support for
> > > tx_prepare() and do not suffer any performance impact as result;
> > > applications only have to make sure tx_prepare() is always called at some
> > > point before tx_burst().
> > >
> > > Once you reach this stage, you've effectively made tx_prepare() mandatory
> > > before tx_burst(). If some bug occurs, then perhaps you forgot to call
> > > tx_prepare(), you just need to add it. The total cost for doing TX is
> > > therefore tx_prepare() + tx_burst().
> > >
> > > I'm perhaps a bit pessimistic mind you, but I do not think tx_prepare() will
> > > remain optional for long. Sure, PMDs that do not implement it do not care,
> > > I'm focusing on applications, for which the performance impact of calling
> > > tx_prepare() followed by tx_burst() is higher than a single tx_burst()
> > > performing all the necessary preparation at once.
> > >
> > > [...]
> > > > > Following the same logic, why can't such a thing be made part of the TX
> > > > > burst function as well (through a direct call to rte_phdr_cksum_fix()
> > > > > whenever necessary). From an application standpoint, what are the advantages
> > > > > of having to:
> > > > >
> > > > >  if (tx_prep()) // iterate and update mbufs as needed
> > > > >      tx_burst(); // iterate and send
> > > > >
> > > > > Compared to:
> > > > >
> > > > >  tx_burst(); // iterate, update as needed and send
> > > >
> > > > I think that was discussed extensively quite a lot previously here:
> > > > As Thomas already replied - main motivation is to allow user
> > > > to execute them on different stages of packet TX pipeline,
> > > > and probably on different cores.
> > > > I think that provides better flexibility to the user to when/where
> > > > do these preparations and hopefully would lead to better performance.
> > >
> > > And I agree, I think this use case is valid but does not warrant such a high
> > > penalty when your application does not need that much flexibility. Simple
> > > (yet conscious) applications need the highest performance. Complex ones as
> > > you described already suffer quite a bit from IPCs and won't mind a couple
> > > of extra CPU cycles right?
> >
> > It would mean an extra cache-miss for every packet, so I think performance hit
> > would be quite significant.
> 
> A performance hit has to occur somewhere regardless, because something has
> to be done in order to send packets that need it. Whether this cost is in
> application code or in a PMD function, it remains part of TX.

Depending on the place the final cost would differ quite a lot.
If you call tx_prepare() somewhere close to the place where you fill the packet header
contents, then most likely the data that tx_prepare() has to access will be already in the cache.
So the performance penalty will be minimal.
If you'll try to access the same data later (at tx_burst), then the possibility that it would still
be in cache is much less.
If you calling tx_burst() from other core then data would for sure be out of cache,
and  even worse can still be in another core cache.

> 
> > About the 'simple' case when tx_prep() and tx_burst() are called on the same core,
> > Why do you believe that:
> > tx_prep(); tx_burst(); would be much slower than tx_burst() {tx_prep(), ...}?
> 
> I mean instead of two function calls with their own loops:
> 
>  tx_prepare() { foreach (pkt) { check(); extra_check(); ... } }
> 
>  tx_burst() { foreach (pkt) { check(); stuff(); ... } }
> 
> You end up with one:
> 
>  tx_burst() { foreach (pkt) { check(); extra_check(); stuff(); ... } }
> 
> Which usually is more efficient.

I really doubt that.
If it would be that, what is the point to process packet in bulks?
Usually dividing processing into different stages and at each stage processing
multiple packet at once helps to improve performance.
At  least for IA.
Look for example how we had to change l3fwd to improve its performance.

> 
> > tx_prep() itself is quite expensive, let say for Intel HW it includes:
> > - read mbuf fileds (2 cache-lines),
> > - read packet header (1/2 cache-lines)
> > - calculate pseudo-header csum
> >  - update packet header
> > Comparing to that price of extra function call seems neglectable
> > (if we TX packets in bursts of course).
> 
> We agree its performance is a critical issue then, sharing half the read
> steps with tx_burst() would make sense to me.

I didn't understand that sentence.

> 
> > > Yes they will, therefore we need a method that satisfies both cases.
> > >
> > > As a possible solution, a special mbuf flag could be added to each mbuf
> > > having gone through tx_prepare(). That way, tx_burst() could skip some
> > > checks and things it would otherwise have done.
> >
> > That's an interesting idea, but it has one drawback:
> > As I understand, it means that from now on if user doing preparations on his own,
> > he had to setup this flag, otherwise tx_burst() would do extra unnecessary work.
> > So any existing applications that using TX offloads and do preparation by themselves
> > would have to be modified to avoid performance loss.
> 
> In my opinion, users should not do preparation on their own.

People already do it now.

> If we provide a
> generic method, it has to be fast enough to replace theirs. Perhaps not as
> fast since it would work with all PMDs (usual trade-off), but acceptably so.
> 
> > > Another possibility, telling the PMD first that you always intend to use
> > > tx_prepare() and getting a simpler/faster tx_burst() callback as a result.
> >
> > That what we have right now (at least for Intel HW):
> > it is a user responsibility to do the necessary preparations/checks before calling tx_burst().
> > With tx_prepare() we just remove from user the headache to implement tx_prepare() on his own.
> > Now he can use a 'proper' PMD provided function.
> >
> > My vote still would be for that model.
> 
> OK, then in a nutshell:
> 
> 1. Users are not expected to perform preparation/checks themselves anymore,
>    if they do, it's their problem.

I think we need to be backward compatible here.
If the existing app doing what tx_prepare() supposed to do, it should keep working.
 
> 
> 2. If configured through an API to be defined, tx_burst() can be split in
>    two and applications must call tx_prepare() at some point before
>    tx_burst().
> 
> 3. Otherwise tx_burst() should perform the necessary preparation and checks
>    on its own by default (when tx_prepare() is not expected).

As I said before, I don't think it should be mandatory for tx_burst() to do what tx_prepare() does.
If some particular implementation of tx_burst() prefers to do things that way - that's fine.
But it shouldn't be required to.

> 
> 4. We probably still need some mbuf flag to mark mbufs that cannot be
>    modified, the refcount could also serve as a hint.

If mbuf can't be modified, you probably just wouldn't call the function that supposed to do that,
tx_prepare() in that case.  

Konstantin

^ permalink raw reply

* Re: [PATCH 3/8] doc: Adding NXP DPAA2_SEC in cryptodev
From: Mcnamara, John @ 2016-12-05 16:42 UTC (permalink / raw)
  To: Mcnamara, John, Akhil Goyal, dev@dpdk.org
  Cc: thomas.monjalon@6wind.com, Doherty, Declan, De Lara Guarch, Pablo,
	hemant.agrawal@nxp.com
In-Reply-To: <B27915DBBA3421428155699D51E4CFE20266ECA7@IRSMSX103.ger.corp.intel.com>

P.S. There was also a whitespace warning on commit.

^ permalink raw reply

* Re: [PATCH 3/8] doc: Adding NXP DPAA2_SEC in cryptodev
From: Mcnamara, John @ 2016-12-05 16:40 UTC (permalink / raw)
  To: Akhil Goyal, dev@dpdk.org
  Cc: thomas.monjalon@6wind.com, Doherty, Declan, De Lara Guarch, Pablo,
	hemant.agrawal@nxp.com
In-Reply-To: <20161205125540.6419-4-akhil.goyal@nxp.com>

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
> Sent: Monday, December 5, 2016 12:56 PM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; eclan.doherty@intel.com; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com; Akhil
> Goyal <akhil.goyal@nxp.com>
> Subject: [dpdk-dev] [PATCH 3/8] doc: Adding NXP DPAA2_SEC in cryptodev
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> Reviewed-by: Hemant Agrawal <hemant.agrawal@nxp.com>


> +
> +NXP(R) DPAA2 CAAM Accelerartor Based (DPAA2_SEC) Crypto Poll Mode
> +Driver
> +=======================================================================

Small typo here: /Accelerartor/Accelerator /


...

> +Installations
> +-------------
> +
> +

This section shouldn't be blank.

^ permalink raw reply

* Re: [PATCH] doc: fix wrong verbatim text paragraphs
From: Mcnamara, John @ 2016-12-05 16:16 UTC (permalink / raw)
  To: Baruch Siach, dev@dpdk.org; +Cc: Olivier Matz
In-Reply-To: <8c110dd5908c2fd031a0d8001293413491f579c8.1480952455.git.baruch@tkos.co.il>

> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Monday, December 5, 2016 3:41 PM
> To: dev@dpdk.org
> Cc: Olivier Matz <olivier.matz@6wind.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Baruch Siach <baruch@tkos.co.il>
> Subject: [PATCH] doc: fix wrong verbatim text paragraphs
> 
> Reduce the indentation of these paragraphs since they are not part of the
> verbatim block.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply

* Re: [PATCH] use generated flags for SSE and AVX checks
From: Thomas Monjalon @ 2016-12-05 15:59 UTC (permalink / raw)
  To: dev
In-Reply-To: <1480952058-13591-1-git-send-email-thomas.monjalon@6wind.com>

2016-12-05 16:34, Thomas Monjalon:
> Clean up the code to always use the flags RTE_MACHINE_CPUFLAG_*
> generated by the DPDK makefile rte.cpuflags.mk.

This patch does not work because RTE_MACHINE_CPUFLAG_* are generated
for the whole library when including rte.vars.mk.
So the flags are not accurate when overriding the flags per file like
it is done in rte_acl.

So the questions are:
	- should we use RTE_MACHINE_CPUFLAG_?
	- should we override the flags per file?
	- will we be able to use the function attribute __target__?

^ permalink raw reply

* [PATCH] doc: fix wrong verbatim text paragraphs
From: Baruch Siach @ 2016-12-05 15:40 UTC (permalink / raw)
  To: dev; +Cc: Olivier Matz, John McNamara, Baruch Siach

Reduce the indentation of these paragraphs since they are not part of the
verbatim block.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 doc/guides/prog_guide/mbuf_lib.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guides/prog_guide/mbuf_lib.rst b/doc/guides/prog_guide/mbuf_lib.rst
index 8e616826c9b9..f0aa21a6d4aa 100644
--- a/doc/guides/prog_guide/mbuf_lib.rst
+++ b/doc/guides/prog_guide/mbuf_lib.rst
@@ -175,8 +175,8 @@ a vxlan-encapsulated tcp packet:
     set out_ip checksum to 0 in the packet
     set out_udp checksum to pseudo header using rte_ipv4_phdr_cksum()
 
-   This is supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM
-   and DEV_TX_OFFLOAD_UDP_CKSUM.
+  This is supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM
+  and DEV_TX_OFFLOAD_UDP_CKSUM.
 
 - calculate checksum of in_ip::
 
@@ -228,8 +228,8 @@ a vxlan-encapsulated tcp packet:
     set in_ip checksum to 0 in the packet
     set in_tcp checksum to pseudo header using rte_ipv4_phdr_cksum()
 
-   This is supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM,
-   DEV_TX_OFFLOAD_UDP_CKSUM and DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM.
+  This is supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM,
+  DEV_TX_OFFLOAD_UDP_CKSUM and DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM.
 
 The list of flags and their precise meaning is described in the mbuf API
 documentation (rte_mbuf.h). Also refer to the testpmd source code
-- 
2.10.2

^ permalink raw reply related

* [PATCH] use generated flags for SSE and AVX checks
From: Thomas Monjalon @ 2016-12-05 15:34 UTC (permalink / raw)
  To: dev

Clean up the code to always use the flags RTE_MACHINE_CPUFLAG_*
generated by the DPDK makefile rte.cpuflags.mk.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 examples/l3fwd/l3fwd_em.c                         |  8 ++++----
 examples/l3fwd/l3fwd_lpm.c                        |  6 +++---
 examples/performance-thread/l3fwd-thread/main.c   |  2 +-
 lib/librte_eal/common/include/arch/x86/rte_vect.h | 14 +++++++-------
 lib/librte_eal/common/include/rte_common.h        |  2 +-
 lib/librte_hash/rte_thash.h                       |  8 +++-----
 lib/librte_sched/rte_sched.c                      |  2 +-
 lib/librte_table/rte_lru.h                        |  2 +-
 8 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/examples/l3fwd/l3fwd_em.c b/examples/l3fwd/l3fwd_em.c
index 9cc4460..6714430 100644
--- a/examples/l3fwd/l3fwd_em.c
+++ b/examples/l3fwd/l3fwd_em.c
@@ -246,7 +246,7 @@ static rte_xmm_t mask0;
 static rte_xmm_t mask1;
 static rte_xmm_t mask2;
 
-#if defined(__SSE2__)
+#if defined(RTE_MACHINE_CPUFLAG_SSE2)
 static inline xmm_t
 em_mask_key(void *key, xmm_t mask)
 {
@@ -328,7 +328,7 @@ em_get_ipv6_dst_port(void *ipv6_hdr,  uint8_t portid, void *lookup_struct)
 	return (uint8_t)((ret < 0) ? portid : ipv6_l3fwd_out_if[ret]);
 }
 
-#if defined(__SSE4_1__)
+#if defined(RTE_MACHINE_CPUFLAG_SSE4_1)
 #if defined(NO_HASH_MULTI_LOOKUP)
 #include "l3fwd_em_sse.h"
 #else
@@ -709,13 +709,13 @@ em_main_loop(__attribute__((unused)) void *dummy)
 			if (nb_rx == 0)
 				continue;
 
-#if defined(__SSE4_1__)
+#if defined(RTE_MACHINE_CPUFLAG_SSE4_1)
 			l3fwd_em_send_packets(nb_rx, pkts_burst,
 							portid, qconf);
 #else
 			l3fwd_em_no_opt_send_packets(nb_rx, pkts_burst,
 							portid, qconf);
-#endif /* __SSE_4_1__ */
+#endif /* SSE_4_1 */
 		}
 	}
 
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index f621269..005534d 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -104,7 +104,7 @@ static struct ipv6_l3fwd_lpm_route ipv6_l3fwd_lpm_route_array[] = {
 struct rte_lpm *ipv4_l3fwd_lpm_lookup_struct[NB_SOCKETS];
 struct rte_lpm6 *ipv6_l3fwd_lpm_lookup_struct[NB_SOCKETS];
 
-#if defined(__SSE4_1__)
+#if defined(RTE_MACHINE_CPUFLAG_SSE4_1)
 #include "l3fwd_lpm_sse.h"
 #else
 #include "l3fwd_lpm.h"
@@ -178,13 +178,13 @@ lpm_main_loop(__attribute__((unused)) void *dummy)
 			if (nb_rx == 0)
 				continue;
 
-#if defined(__SSE4_1__)
+#if defined(RTE_MACHINE_CPUFLAG_SSE4_1)
 			l3fwd_lpm_send_packets(nb_rx, pkts_burst,
 						portid, qconf);
 #else
 			l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
 							portid, qconf);
-#endif /* __SSE_4_1__ */
+#endif /* SSE_4_1 */
 		}
 	}
 
diff --git a/examples/performance-thread/l3fwd-thread/main.c b/examples/performance-thread/l3fwd-thread/main.c
index fdc90b2..0917aa1 100644
--- a/examples/performance-thread/l3fwd-thread/main.c
+++ b/examples/performance-thread/l3fwd-thread/main.c
@@ -95,7 +95,7 @@
  *  When set to one, optimized forwarding path is enabled.
  *  Note that LPM optimisation path uses SSE4.1 instructions.
  */
-#if ((APP_LOOKUP_METHOD == APP_LOOKUP_LPM) && !defined(__SSE4_1__))
+#if ((APP_LOOKUP_METHOD == APP_LOOKUP_LPM) && !defined(RTE_MACHINE_CPUFLAG_SSE4_1))
 #define ENABLE_MULTI_BUFFER_OPTIMIZE	0
 #else
 #define ENABLE_MULTI_BUFFER_OPTIMIZE	1
diff --git a/lib/librte_eal/common/include/arch/x86/rte_vect.h b/lib/librte_eal/common/include/arch/x86/rte_vect.h
index 77f2e25..56b53b7 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_vect.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_vect.h
@@ -44,23 +44,23 @@
 
 #if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
 
-#ifdef __SSE__
+#ifdef RTE_MACHINE_CPUFLAG_SSE
 #include <xmmintrin.h>
 #endif
 
-#ifdef __SSE2__
+#ifdef RTE_MACHINE_CPUFLAG_SSE2
 #include <emmintrin.h>
 #endif
 
-#ifdef __SSE3__
+#ifdef RTE_MACHINE_CPUFLAG_SSE3
 #include <tmmintrin.h>
 #endif
 
-#if defined(__SSE4_2__) || defined(__SSE4_1__)
+#if defined(RTE_MACHINE_CPUFLAG_SSE4_2) || defined(RTE_MACHINE_CPUFLAG_SSE4_1)
 #include <smmintrin.h>
 #endif
 
-#if defined(__AVX__)
+#if defined(RTE_MACHINE_CPUFLAG_AVX)
 #include <immintrin.h>
 #endif
 
@@ -88,7 +88,7 @@ typedef union rte_xmm {
 	double   pd[XMM_SIZE / sizeof(double)];
 } rte_xmm_t;
 
-#ifdef __AVX__
+#ifdef RTE_MACHINE_CPUFLAG_AVX
 
 typedef __m256i ymm_t;
 
@@ -105,7 +105,7 @@ typedef union rte_ymm {
 	double   pd[YMM_SIZE / sizeof(double)];
 } rte_ymm_t;
 
-#endif /* __AVX__ */
+#endif /* AVX */
 
 #ifdef RTE_ARCH_I686
 #define _mm_cvtsi128_si64(a)    \
diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index db5ac91..bc0f4cd 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -294,7 +294,7 @@ rte_align64pow2(uint64_t v)
 
 /*********** Other general functions / macros ********/
 
-#ifdef __SSE2__
+#ifdef RTE_MACHINE_CPUFLAG_SSE2
 #include <emmintrin.h>
 /**
  * PAUSE instruction for tight loops (avoid busy waiting)
diff --git a/lib/librte_hash/rte_thash.h b/lib/librte_hash/rte_thash.h
index a4886a8..9a352bd 100644
--- a/lib/librte_hash/rte_thash.h
+++ b/lib/librte_hash/rte_thash.h
@@ -56,11 +56,9 @@ extern "C" {
 #include <rte_ip.h>
 #include <rte_common.h>
 
-#ifdef __SSE3__
+#ifdef RTE_MACHINE_CPUFLAG_SSE3
 #include <rte_vect.h>
-#endif
 
-#ifdef __SSE3__
 /* Byte swap mask used for converting IPv6 address
  * 4-byte chunks to CPU byte order
  */
@@ -134,7 +132,7 @@ struct rte_ipv6_tuple {
 union rte_thash_tuple {
 	struct rte_ipv4_tuple	v4;
 	struct rte_ipv6_tuple	v6;
-#ifdef __SSE3__
+#ifdef RTE_MACHINE_CPUFLAG_SSE3
 } __attribute__((aligned(XMM_SIZE)));
 #else
 };
@@ -169,7 +167,7 @@ rte_convert_rss_key(const uint32_t *orig, uint32_t *targ, int len)
 static inline void
 rte_thash_load_v6_addrs(const struct ipv6_hdr *orig, union rte_thash_tuple *targ)
 {
-#ifdef __SSE3__
+#ifdef RTE_MACHINE_CPUFLAG_SSE3
 	__m128i ipv6 = _mm_loadu_si128((const __m128i *)orig->src_addr);
 	*(__m128i *)targ->v6.src_addr =
 			_mm_shuffle_epi8(ipv6, rte_thash_ipv6_bswap_mask);
diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index e6dace2..c593363 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -56,7 +56,7 @@
 #ifdef RTE_SCHED_VECTOR
 #include <rte_vect.h>
 
-#if defined(__SSE4__)
+#ifdef RTE_MACHINE_CPUFLAG_SSE4
 #define SCHED_VECTOR_SSE4
 #endif
 
diff --git a/lib/librte_table/rte_lru.h b/lib/librte_table/rte_lru.h
index e87e062..3d677c8 100644
--- a/lib/librte_table/rte_lru.h
+++ b/lib/librte_table/rte_lru.h
@@ -47,7 +47,7 @@ extern "C" {
 #endif
 
 #ifndef RTE_TABLE_HASH_LRU_STRATEGY
-#ifdef __SSE4_2__
+#ifdef RTE_MACHINE_CPUFLAG_SSE4_2
 #define RTE_TABLE_HASH_LRU_STRATEGY                        2
 #else /* if no SSE, use simple scalar version */
 #define RTE_TABLE_HASH_LRU_STRATEGY                        1
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH v1] doc: fix relative path of Nic table input file
From: De Lara Guarch, Pablo @ 2016-12-05 15:15 UTC (permalink / raw)
  To: Mcnamara, John, dev@dpdk.org; +Cc: stable@dpdk.org, Mcnamara, John
In-Reply-To: <1480870057-1674-1-git-send-email-john.mcnamara@intel.com>



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John McNamara
> Sent: Sunday, December 04, 2016 4:48 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Mcnamara, John
> Subject: [dpdk-dev] [PATCH v1] doc: fix relative path of Nic table input file
> 
> Fix relative path between sphinx conf.py file and Nic table file
> to allow automatic build on ReadTheDocs.
> 
> Fixes: 9db3f52126fb ("doc: generate NIC overview table from ini files")
> 
> Signed-off-by: John McNamara <john.mcnamara@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply

* Re: [PATCH] Scheduler: add driver for scheduler crypto pmd
From: Neil Horman @ 2016-12-05 15:12 UTC (permalink / raw)
  To: Declan Doherty; +Cc: Bruce Richardson, Thomas Monjalon, Fan Zhang, dev
In-Reply-To: <63671b1d-52e0-e653-1323-5d9513c0b9dc@intel.com>

On Fri, Dec 02, 2016 at 04:22:16PM +0000, Declan Doherty wrote:
> On 02/12/16 14:57, Bruce Richardson wrote:
> > On Fri, Dec 02, 2016 at 03:31:24PM +0100, Thomas Monjalon wrote:
> > > 2016-12-02 14:15, Fan Zhang:
> > > > This patch provides the initial implementation of the scheduler poll mode
> > > > driver using DPDK cryptodev framework.
> > > > 
> > > > Scheduler PMD is used to schedule and enqueue the crypto ops to the
> > > > hardware and/or software crypto devices attached to it (slaves). The
> > > > dequeue operation from the slave(s), and the possible dequeued crypto op
> > > > reordering, are then carried out by the scheduler.
> > > > 
> > > > The scheduler PMD can be used to fill the throughput gap between the
> > > > physical core and the existing cryptodevs to increase the overall
> > > > performance. For example, if a physical core has higher crypto op
> > > > processing rate than a cryptodev, the scheduler PMD can be introduced to
> > > > attach more than one cryptodevs.
> > > > 
> > > > This initial implementation is limited to supporting the following
> > > > scheduling modes:
> > > > 
> > > > - CRYPTO_SCHED_SW_ROUND_ROBIN_MODE (round robin amongst attached software
> > > >     slave cryptodevs, to set this mode, the scheduler should have been
> > > >     attached 1 or more software cryptodevs.
> > > > 
> > > > - CRYPTO_SCHED_HW_ROUND_ROBIN_MODE (round robin amongst attached hardware
> > > >     slave cryptodevs (QAT), to set this mode, the scheduler should have
> > > >     been attached 1 or more QATs.
> > > 
> > > Could it be implemented on top of the eventdev API?
> > > 
> > Not really. The eventdev API is for different types of scheduling
> > between multiple sources that are all polling for packets, compared to
> > this, which is more analgous - as I understand it - to the bonding PMD
> > for ethdev.
> > 
> > To make something like this work with an eventdev API you would need to
> > use one of the following models:
> > * have worker cores for offloading packets to the different crypto
> >   blocks pulling from the eventdev APIs. This would make it difficult to
> >   do any "smart" scheduling of crypto operations between the blocks,
> >   e.g. that one crypto instance may be better at certain types of
> >   operations than another.
> > * move the logic in this driver into an existing eventdev instance,
> >   which uses the eventdev api rather than the crypto APIs and so has an
> >   extra level of "structure abstraction" that has to be worked though.
> >   It's just not really a good fit.
> > 
> > So for this workload, I believe the pseudo-cryptodev instance is the
> > best way to go.
> > 
> > /Bruce
> > 
> 
> 
> As Bruce says this is much more analogous to the ethdev bonding driver, the
> main idea is to allow different crypto op scheduling mechanisms to be
> defined transparently to an application. This could be load-balancing across
> multiple hw crypto devices, or having a software crypto device to act as a
> backup device for a hw accelerator if it becomes oversubscribed. I think the
> main advantage of a crypto-scheduler approach means that the data path of
> the application doesn't need to have any knowledge that scheduling is
> happening at all, it is just using a different crypto device id, which is
> then manages the distribution of crypto work.
> 
> 
> 
This is a good deal like the bonding pmd, and so from a certain standpoint it
makes sense to do this, but whereas the bonding pmd is meant to create a single
path to a logical network over several physical networks, this pmd really only
focuses on maximizing througput, and for that we already have tools.  As Thomas
mentions, there is the eventdev library, but from my view the distributor
library already fits this bill.  It already is a basic framework to process
mbufs in parallel according to whatever policy you want to implement, which
sounds like exactly what the goal of this pmd is.  

Neil
 

^ permalink raw reply

* Re: [PATCH v12 0/6] add Tx preparation
From: Adrien Mazarguil @ 2016-12-05 15:03 UTC (permalink / raw)
  To: Ananyev, Konstantin
  Cc: Thomas Monjalon, dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd,
	Jan Medala, Jakub Palider, John Daley, Alejandro Lucero,
	Harish Patil, Rasesh Mody, Jerin Jacob, Yuanhan Liu, Yong Wang,
	Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E2A9A@irsmsx105.ger.corp.intel.com>

Hi Konstantin,

On Fri, Dec 02, 2016 at 01:00:55AM +0000, Ananyev, Konstantin wrote:
[...]
> > On Wed, Nov 30, 2016 at 10:54:50AM +0000, Ananyev, Konstantin wrote:
> > [...]
> > > > Something is definitely needed here, and only PMDs can provide it. I think
> > > > applications should not have to clear checksum fields or initialize them to
> > > > some magic value, same goes for any other offload or hardware limitation
> > > > that needs to be worked around.
> > > >
> > > > tx_prep() is one possible answer to this issue, however as mentioned in the
> > > > original patch it can be very expensive if exposed by the PMD.
> > > >
> > > > Another issue I'm more concerned about is the way limitations are managed
> > > > (struct rte_eth_desc_lim). While not officially tied to tx_prep(), this
> > > > structure contains new fields that are only relevant to a few devices, and I
> > > > fear it will keep growing with each new hardware quirk to manage, breaking
> > > > ABIs in the process.
> > >
> > > Well, if some new HW capability/limitation would arise and we'd like to support
> > > it in DPDK, then yes we probably would need to think how to incorporate it here.
> > > Do you have anything particular in mind here?
> > 
> > Nothing in particular, so for the sake of the argument, let's suppose that I
> > would like to add a field to expose some limitation that only applies to my
> > PMD during TX but looks generic enough to make sense, e.g. maximum packet
> > size when VLAN tagging is requested.
> 
> Hmm, I didn't hear about such limitations so far, but if it is real case -
> sure, feel free to submit the patch.   

I won't, that was hypothetical.

> > PMDs are free to set that field to some
> > special value (say, 0) if they do not care.
> > 
> > Since that field exists however, conscious applications should check its
> > value for each packet that needs to be transmitted. This extra code causes a
> > slowdown just by sitting in the data path. Since it is not the only field in
> > that structure, the performance impact can be significant.
> > 
> > Even though this code is inside applications, it remains unfair to PMDs for
> > which these tests are irrelevant. This problem is identified and addressed
> > by tx_prepare().
> 
> I suppose the question is why do we need:
> uint16_t nb_seg_max;
> uint16_t nb_mtu_seg_max;
> as we now have tx_prepare(), right?
> 
> For two reasons:
> 1. Some people might feel that tx_prepare() is not good (smart/fast) enough
> for them and would prefer to do necessary preparations for TX offloads themselves.
> 
> 2. Even if people do use tx_prepare() they still should take this information into accout.
> As an example ixgbe can't TX packets with then 40 segments.
> Obviously ixbge_tx_prep() performs that check and returns an error.

Problem is that tx_prepare() also provides safeties which are not part of
tx_burst(), such as not going over nb_mtu_seg_max. Because of this and the
fact struct rte_eth_desc_lim can grow new fields anytime, application
developers will be tempted to just call tx_prepare() and focus on more
useful things.

Put another way, from a user's point of view, tx_prepare() is an opaque
function that greatly increases tx_burst()'s ability to send mbufs as
requested, with extra error checking on top; applications not written to run
on a specific PMD/device (all of them ideally) will thus call tx_prepare()
at some point.

> But it wouldn't try to merge/reallocate mbufs for you.
> User still has to do it himself, or just prevent creating such long chains somehow.

Yes, that's another debate. PMDs could still implement a software fallback
for unlikely slow events like these. The number of PMDs is not going to
decrease, each device having its own set of weird limitations in specific
cases, PMDs should do their best to process mbufs even if that means slowly
due to the lack of preparation.

tx_prepare() has its uses but should really be optional, in the sense that
if that function is not called, tx_burst() should deal with it somehow.

> > Thanks to tx_prepare(), these checks are moved back into PMDs where they
> > belong. PMDs that do not need them do not have to provide support for
> > tx_prepare() and do not suffer any performance impact as result;
> > applications only have to make sure tx_prepare() is always called at some
> > point before tx_burst().
> > 
> > Once you reach this stage, you've effectively made tx_prepare() mandatory
> > before tx_burst(). If some bug occurs, then perhaps you forgot to call
> > tx_prepare(), you just need to add it. The total cost for doing TX is
> > therefore tx_prepare() + tx_burst().
> > 
> > I'm perhaps a bit pessimistic mind you, but I do not think tx_prepare() will
> > remain optional for long. Sure, PMDs that do not implement it do not care,
> > I'm focusing on applications, for which the performance impact of calling
> > tx_prepare() followed by tx_burst() is higher than a single tx_burst()
> > performing all the necessary preparation at once.
> > 
> > [...]
> > > > Following the same logic, why can't such a thing be made part of the TX
> > > > burst function as well (through a direct call to rte_phdr_cksum_fix()
> > > > whenever necessary). From an application standpoint, what are the advantages
> > > > of having to:
> > > >
> > > >  if (tx_prep()) // iterate and update mbufs as needed
> > > >      tx_burst(); // iterate and send
> > > >
> > > > Compared to:
> > > >
> > > >  tx_burst(); // iterate, update as needed and send
> > >
> > > I think that was discussed extensively quite a lot previously here:
> > > As Thomas already replied - main motivation is to allow user
> > > to execute them on different stages of packet TX pipeline,
> > > and probably on different cores.
> > > I think that provides better flexibility to the user to when/where
> > > do these preparations and hopefully would lead to better performance.
> > 
> > And I agree, I think this use case is valid but does not warrant such a high
> > penalty when your application does not need that much flexibility. Simple
> > (yet conscious) applications need the highest performance. Complex ones as
> > you described already suffer quite a bit from IPCs and won't mind a couple
> > of extra CPU cycles right?
> 
> It would mean an extra cache-miss for every packet, so I think performance hit
> would be quite significant. 

A performance hit has to occur somewhere regardless, because something has
to be done in order to send packets that need it. Whether this cost is in
application code or in a PMD function, it remains part of TX.

> About the 'simple' case when tx_prep() and tx_burst() are called on the same core,
> Why do you believe that:
> tx_prep(); tx_burst(); would be much slower than tx_burst() {tx_prep(), ...}?

I mean instead of two function calls with their own loops:

 tx_prepare() { foreach (pkt) { check(); extra_check(); ... } }

 tx_burst() { foreach (pkt) { check(); stuff(); ... } }

You end up with one:

 tx_burst() { foreach (pkt) { check(); extra_check(); stuff(); ... } }

Which usually is more efficient.

> tx_prep() itself is quite expensive, let say for Intel HW it includes:
> - read mbuf fileds (2 cache-lines),
> - read packet header (1/2 cache-lines)
> - calculate pseudo-header csum
>  - update packet header 
> Comparing to that price of extra function call seems neglectable
> (if we TX packets in bursts of course). 

We agree its performance is a critical issue then, sharing half the read
steps with tx_burst() would make sense to me.

> > Yes they will, therefore we need a method that satisfies both cases.
> > 
> > As a possible solution, a special mbuf flag could be added to each mbuf
> > having gone through tx_prepare(). That way, tx_burst() could skip some
> > checks and things it would otherwise have done.
> 
> That's an interesting idea, but it has one drawback:
> As I understand, it means that from now on if user doing preparations on his own,
> he had to setup this flag, otherwise tx_burst() would do extra unnecessary work.
> So any existing applications that using TX offloads and do preparation by themselves
> would have to be modified to avoid performance loss.

In my opinion, users should not do preparation on their own. If we provide a
generic method, it has to be fast enough to replace theirs. Perhaps not as
fast since it would work with all PMDs (usual trade-off), but acceptably so.

> > Another possibility, telling the PMD first that you always intend to use
> > tx_prepare() and getting a simpler/faster tx_burst() callback as a result.
> 
> That what we have right now (at least for Intel HW):  
> it is a user responsibility to do the necessary preparations/checks before calling tx_burst().  
> With tx_prepare() we just remove from user the headache to implement tx_prepare() on his own.
> Now he can use a 'proper' PMD provided function.
> 
> My vote still would be for that model.

OK, then in a nutshell:

1. Users are not expected to perform preparation/checks themselves anymore,
   if they do, it's their problem.

2. If configured through an API to be defined, tx_burst() can be split in
   two and applications must call tx_prepare() at some point before
   tx_burst().

3. Otherwise tx_burst() should perform the necessary preparation and checks
   on its own by default (when tx_prepare() is not expected).

4. We probably still need some mbuf flag to mark mbufs that cannot be
   modified, the refcount could also serve as a hint.

Anything else?

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply

* Re: [PATCH 31/31] net/i40e: remove unused marco from PMD
From: Ferruh Yigit @ 2016-12-05 14:57 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang
In-Reply-To: <1480727953-92137-32-git-send-email-jingjing.wu@intel.com>

On 12/3/2016 1:19 AM, Jingjing Wu wrote:

s/marco/macro in commit title

It is good to see that this unused macro removed, thanks.

> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>

<...>

^ permalink raw reply

* Re: [PATCH 30/31] net/i40e/base: remove unused marco
From: Ferruh Yigit @ 2016-12-05 14:55 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang
In-Reply-To: <1480727953-92137-31-git-send-email-jingjing.wu@intel.com>

On 12/3/2016 1:19 AM, Jingjing Wu wrote:
> remove X722_SUPPORT and I40E_NDIS_SUPPORT MARCOs

s/marco/macro, same for patch subject

> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
<...>

^ permalink raw reply

* Re: [PATCH 25/31] net/i40e/base: remove duplicate definitions
From: Ferruh Yigit @ 2016-12-05 14:52 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang
In-Reply-To: <1480727953-92137-26-git-send-email-jingjing.wu@intel.com>

On 12/3/2016 1:19 AM, Jingjing Wu wrote:
> We already define I40E_AQ_PHY_TYPE_EXT_25G* flags in the response adminq
> structure above, and do not need to re-define these. See eee_capability
> for an example where we didn't re-define these. This prevents Linux
> driver from complaining about using these flags in an #ifdef when
> running cppkeep tool, and generally makes more sense to avoid
> duplicating the definitions.

Although this is base driver and used in various platforms, mentioning
from the Linux driver and cppkeep here can be confusing, what do you
think removing that part?

> 
> While we are here, replace 0X with 0x as normal style.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
<...>

^ permalink raw reply

* Re: [PATCH 04/31] net/i40e/base: fix bit test mask
From: Ferruh Yigit @ 2016-12-05 14:37 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang
In-Reply-To: <1480727953-92137-5-git-send-email-jingjing.wu@intel.com>

On 12/3/2016 1:18 AM, Jingjing Wu wrote:
> Incorrect bit mask was used for testing "get link status" response.
> Instead of I40E_AQ_LSE_ENABLE (which is actually 0x03) it most probably

most probably J

> should be I40E_AQ_LSE_IS_ENABLED (which is defined as 0x01).
> 
> Fixes: 8db9e2a1b232 ("i40e: base driver")

This is detail, but defined syntax requires an empty line between
"Fixes" tag and "Signed-off-by" tag. You will see
/scripts/check-git-log.sh complain about it.

> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/i40e/base/i40e_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
> index aa346d1..a2661cf 100644
> --- a/drivers/net/i40e/base/i40e_common.c
> +++ b/drivers/net/i40e/base/i40e_common.c
> @@ -1975,7 +1975,7 @@ enum i40e_status_code i40e_aq_get_link_info(struct i40e_hw *hw,
>  	else
>  		hw_link_info->crc_enable = false;
>  
> -	if (resp->command_flags & CPU_TO_LE16(I40E_AQ_LSE_ENABLE))
> +	if (resp->command_flags & CPU_TO_LE16(I40E_AQ_LSE_IS_ENABLED))
>  		hw_link_info->lse_enable = true;
>  	else
>  		hw_link_info->lse_enable = false;
> 

^ permalink raw reply

* Re: [PATCH 02/31] net/i40e/base: preserve extended PHY type field
From: Ferruh Yigit @ 2016-12-05 14:34 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang
In-Reply-To: <1480727953-92137-3-git-send-email-jingjing.wu@intel.com>

On 12/3/2016 1:18 AM, Jingjing Wu wrote:
> Prevents 25G PHY types from being disabled when setting
> the flow control modes.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/i40e/base/i40e_common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
> index 9a6b3ed..d67ad90 100644
> --- a/drivers/net/i40e/base/i40e_common.c
> +++ b/drivers/net/i40e/base/i40e_common.c
> @@ -1789,6 +1789,7 @@ enum i40e_status_code i40e_set_fc(struct i40e_hw *hw, u8 *aq_failures,
>  			config.abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>  		/* Copy over all the old settings */
>  		config.phy_type = abilities.phy_type;
> +		config.phy_type_ext = abilities.phy_type_ext;

http://dpdk.org/dev/patchwork/patch/17338/ does something similar in
i40e_phy_conf_link(), can you please double check if these two works
fine together?

Thanks,
ferruh

^ permalink raw reply

* Re: [PATCH 15/31] net/i40e/base: add FEC bits to PHY capabilities
From: Ferruh Yigit @ 2016-12-05 14:30 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang
In-Reply-To: <1480727953-92137-16-git-send-email-jingjing.wu@intel.com>

Hi Jingjing,

On 12/3/2016 1:18 AM, Jingjing Wu wrote:
> Add FEC bits to the PHY capabilities AQ command struct. This is required
> for 25GbE support. Change the name of the generic mod_type_ext field to
> indicate that it is now used for handling FEC.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/i40e/base/i40e_adminq_cmd.h | 13 ++++++++++++-
>  drivers/net/i40e/base/i40e_common.c     |  2 ++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_adminq_cmd.h b/drivers/net/i40e/base/i40e_adminq_cmd.h
> index 4f06772..1884758 100644
> --- a/drivers/net/i40e/base/i40e_adminq_cmd.h
> +++ b/drivers/net/i40e/base/i40e_adminq_cmd.h
> @@ -1785,7 +1785,16 @@ struct i40e_aq_get_phy_abilities_resp {
>  #define I40E_AQ_PHY_TYPE_EXT_25G_CR	0X02
>  #define I40E_AQ_PHY_TYPE_EXT_25G_SR	0x04
>  #define I40E_AQ_PHY_TYPE_EXT_25G_LR	0x08
> -	u8	mod_type_ext;
> +	u8	fec_cfg_curr_mod_ext_info;

This is causing a compilation error in next-net [1] after Qi's patch for
25G auto link enable patch: http://dpdk.org/dev/patchwork/patch/17338/

Can you please rebase the patchset on top op next-net ?

Thanks,
ferruh

[1]
.../drivers/net/i40e/i40e_ethdev.c: In function ‘i40e_phy_conf_link’:
.../drivers/net/i40e/i40e_ethdev.c:1632:30: error: ‘struct
i40e_aq_get_phy_abilities_resp’ has no member named ‘mod_type_ext’; did
you mean ‘phy_type_ext’?
  phy_conf.fec_config = phy_ab.mod_type_ext;
                              ^

^ permalink raw reply

* Re: Intent to upstream Atomic Rules net/ark "Arkville" in DPDK 17.05
From: Ferruh Yigit @ 2016-12-05 14:10 UTC (permalink / raw)
  To: Shepard Siegel, dev
In-Reply-To: <CAMMLSKD9bGz9NxO3p3oLQU4uaB2zMr+D1eOCg9bs1s9oH16Ynw@mail.gmail.com>

On 12/3/2016 3:14 PM, Shepard Siegel wrote:
> Atomic Rules would like to include our Arkville DPDK PMD net/ark in the
> DPDK 17.05 release.

Welcome to the DPDK community, it is great to see new hardware support.

> We have been watching the recent process of
> Solarflare’s net/sfc upstreaming and we decided it would be too aggressive
> for us to get in on 17.02. Rather than be the last in queue for 17.02, we
> would prefer to be one of the first in the queue for 17.05. This post is
> our statement of that intent.

We already have two new physical PMDs this release: sfc and DPAA2 and a
few virtual ones, it is good for reviewers to have some in 17.05 J

> 
> 
> Arkville is a product from Atomic Rules which is a combination of hardware
> and software. In the DPDK community, the easy way to describe Arkville is
> that it is a line-rate agnostic FPGA-based NIC that does include any
> specific MAC. Arkville is unique in that the design process worked backward
> from the DPDK API/ABI to allow us to design RTL DPDK-aware data movers.
> Arkville’s customers are the small and brave set of users that demand an
> FPGA exist between their MAC ports and their host. A link to a slide deck
> and product preview shown last month at SC16 is at the end of this post.
> 
> 
> Although we’ve done substantial testing; we are just now setting up a
> proper DTS environment. Our first course of business is to add two 10 GbE
> ports and make Arkville look like a Fortville X710-DA2. This is strange for
> us because we started out with four 100 GbE ports, and not much else to
> talk to! We are eager to work with merchant 100 GbE ASIC NICs to help bring
> DTS into the 100 GbE realm. But 100 GbE aside, as soon as we see our
> net/ark PMD playing nice in DTS with a Fortville, and the 17.05 aperture
> opens;  we will commence the patch submission process.

You don't have to wait for 17.05 merge window, you can send the patches
with note that it is targeted for 17.05. It is good to send patches as
early as possible.

> 
> 
> Thanks all who have helped us get this far so soon. Anyone needing
> additional details that aren’t DPDK community wide, please contact me
> directly.
> 
> 
> Shep for AR Team
> 
> 
> Shepard Siegel, CTO
> 
> atomicrules.com
> 
> 
> 
> Links:
> 
> https://dl.dropboxusercontent.com/u/5548901/share/AtomicRules_Arkville_SC16.pdf
> 
> 
> <https://dl.dropboxusercontent.com/u/5548901/share/AtomicRules_Arkville_SC16.pdf>
> 
> https://forums.xilinx.com/t5/Xcell-Daily-Blog/BittWare-s-UltraScale-XUPP3R-board-and-Atomic-Rules-IP-run-Intel/ba-p/734110
> 

^ permalink raw reply

* Re: [PATCH v2] log: do not drop debug logs at compile time
From: Thomas Monjalon @ 2016-12-05 14:08 UTC (permalink / raw)
  To: Olivier Matz; +Cc: david.marchand, dev, keith.wiles
In-Reply-To: <1479915265-17547-1-git-send-email-olivier.matz@6wind.com>

2016-11-23 16:34, Olivier Matz:
> Today, all logs whose level is lower than INFO are dropped at
> compile-time. This prevents from enabling debug logs at runtime using
> --log-level=8.
> 
> The rationale was to remove debug logs from the data path at
> compile-time, avoiding a test at run-time.
> 
> This patch changes the behavior of RTE_LOG() to avoid the compile-time
> optimization, and introduces the RTE_LOG_DP() macro that has the same
> behavior than the previous RTE_LOG(), for the rare cases where debug
> logs are in the data path.
> 
> So it is now possible to enable debug logs at run-time by just
> specifying --log-level=8. Some drivers still have special compile-time
> options to enable more debug log. Maintainers may consider to
> remove/reduce them.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks

^ permalink raw reply

* Re: [PATCH v2 2/2] eal: rename dev init API for consistency
From: Shreyansh Jain @ 2016-12-05 14:03 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: dev, declan.doherty, david.marchand, thomas.monjalon
In-Reply-To: <20161205102404.GA29487@localhost.localdomain>

On Monday 05 December 2016 03:54 PM, Jerin Jacob wrote:
> On Mon, Dec 05, 2016 at 03:42:18PM +0530, Shreyansh Jain wrote:
>> Hello Jerin,
>
> Hello Shreyansh,
>
>>
>> On Sunday 04 December 2016 02:25 AM, Jerin Jacob wrote:
>>> rte_eal_dev_init() is a misleading name.
>>> It actually performs the driver->probe for vdev,
>>> which is parallel to rte_eal_pci_probe.
>>>
>>> Changed to rte_eal_vdev_probe for consistency and
>>> moved the vdev specific probe to eal_common_vdev.c
>>>
>>> Suggested-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>> ---
>>> +int
>>> +rte_eal_vdev_probe(void)
>>> +{
>>> +	struct rte_devargs *devargs;
>>> +
>>> +	/*
>>> +	 * Note that the dev_driver_list is populated here
>>> +	 * from calls made to rte_eal_driver_register from constructor functions
>>> +	 * embedded into PMD modules via the RTE_PMD_REGISTER_VDEV macro
>>> +	 */
>>> +
>>> +	/* call the init function for each virtual device */
>>> +	TAILQ_FOREACH(devargs, &devargs_list, next) {
>>> +
>>> +		if (devargs->type != RTE_DEVTYPE_VIRTUAL)
>>> +			continue;
>>> +
>>> +		if (rte_eal_vdev_init(devargs->virt.drv_name,
>>
>> The situation now is:
>> rte_eal_init=>rte_eal_vdev_probe()=>rte_eal_vdev_init()=> driver->probe()
>>
>> Even though I had suggested this, my intention was to completely do away
>> with rte_*_[v]dev_init as it is misleading.
>>
>> rte_eal_init=>rte_eal_vdev_probe=>driver->probe()
>
> IMO, We don't need to remove rte_eal_vdev_init() as it is an
> application API that uses to create vdev driver instance.Moreover,
> change and removing that name will result in ABI breakage.
>
> grep -ri "rte_eal_vdev_init" app/
> app/test/test_cryptodev.c:				ret = rte_eal_vdev_init(
> app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
> app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
> app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
> app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
> app/test/test_cryptodev.c: int dev_id = rte_eal_vdev_init(
> app/test/test_cryptodev.c: ret = rte_eal_vdev_init(
> app/test/test_cryptodev_perf.c: ret = rte_eal_vdev_init(
> app/test/test_cryptodev_perf.c:	ret = rte_eal_vdev_init(
> app/test/test_cryptodev_perf.c:	ret = rte_eal_vdev_init(
> app/test/test_cryptodev_perf.c:	ret = rte_eal_vdev_init(
>

Got it.

Have you noticed patches from Ben which actually merges init and probe 
all together? [1]. It is for PCI right now (and that too would break 
ABIs, I am assuming).

[1] http://dpdk.org/dev/patchwork/patch/17206/

>
>>
>> should be the ideal order, IMO.
>> Apologies, I was not completely clear then.
>>
>>> +					devargs->args)) {
>>> +			RTE_LOG(ERR, EAL, "failed to initialize %s device\n",
>>> +					devargs->virt.drv_name);
>>> +			return -1;
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
>>> index 8840380..146f505 100644
>>> --- a/lib/librte_eal/common/include/rte_dev.h
>>> +++ b/lib/librte_eal/common/include/rte_dev.h
>>> @@ -171,9 +171,9 @@ void rte_eal_driver_register(struct rte_driver *driver);
>>>  void rte_eal_driver_unregister(struct rte_driver *driver);
>>>
>>>  /**
>>> - * Initalize all the registered drivers in this process
>>> + * Probe all the registered vdev drivers in this process
>>>   */
>>> -int rte_eal_dev_init(void);
>>> +int rte_eal_vdev_probe(void);
>>>
>>>  /**
>>>   * Initialize a driver specified by name.
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
>>> index 16dd5b9..faf75cf 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>>> @@ -884,8 +884,8 @@ rte_eal_init(int argc, char **argv)
>>>  	if (rte_eal_pci_probe())
>>>  		rte_panic("Cannot probe PCI\n");
>>>
>>> -	if (rte_eal_dev_init() < 0)
>>> -		rte_panic("Cannot init pmd devices\n");
>>> +	if (rte_eal_vdev_probe() < 0)
>>> +		rte_panic("Cannot probe vdev drivers\n");
>>>
>>>  	rte_eal_mcfg_complete();
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> index 83721ba..67fc95b 100644
>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> @@ -22,7 +22,7 @@ DPDK_2.0 {
>>>  	rte_dump_tailq;
>>>  	rte_eal_alarm_cancel;
>>>  	rte_eal_alarm_set;
>>> -	rte_eal_dev_init;
>>> +	rte_eal_vdev_probe;
>>>  	rte_eal_devargs_add;
>>>  	rte_eal_devargs_dump;
>>>  	rte_eal_devargs_type_count;
>>>
>>
>

^ permalink raw reply

* Re: [PATCH v2 00/55] Solarflare libefx-based PMD
From: Ferruh Yigit @ 2016-12-05 13:38 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
In-Reply-To: <4ff65a8f-cc8c-b906-9b0a-5e23bb803570@intel.com>

On 12/2/2016 2:55 PM, Ferruh Yigit wrote:
> On 11/29/2016 4:18 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-42) implement minimal device level operations in steps
>>  4. (43-50) implement Rx subsystem
>>  5. (51-55) 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. 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.
>>
>>  3. 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/
>>
>> ---
>>
>> v2:
>> * Fix spelling
>> * Fix ICC and clang build warnings
>> * Remove efx subdirectory
>> * Fix bug in Makefile which results in duplication of warnings options
>> * Use WERROR_FLAGS and remove warnings disabling for the PMD itself
>> * Comment enabled EFSYS_OPT_
>> * Use space after #define instead of TAB
>> * Update version map to upcoming release
>> * Use bool for boolean_t in efsys.h
>>
>>
>> Andrew Rybchenko (48):
>>   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 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 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.yigit@intel.com>

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

^ permalink raw reply

* Re: [PATCH v2] ethdev: check number of queues less than RTE_ETHDEV_QUEUE_STAT_CNTRS
From: Olivier Matz @ 2016-12-05 12:53 UTC (permalink / raw)
  To: Alejandro Lucero, dev; +Cc: Bert van Leeuwen, thomas.monjalon
In-Reply-To: <1480006746.31853.14.camel@6wind.com>

On Thu, 24 Nov 2016 17:59:06 +0100, Olivier Matz
<olivier.matz@6wind.com> wrote:
> Hi,
> 
> On Mon, 2016-11-21 at 09:59 +0000, Alejandro Lucero wrote:
> > From: Bert van Leeuwen <bert.vanleeuwen@netronome.com>
> > 
> > Arrays inside rte_eth_stats have size=RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > Some devices report more queues than that and this code blindly uses
> > the reported number of queues by the device to fill those arrays up.
> > This patch fixes the problem using MIN between the reported number
> > of queues and RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > 
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> >   
> 
> Reviewed-by: Olivier Matz <olivier.matz@6wind.com>

Fixes: ce757f5c9a4d ("ethdev: new method to retrieve extended statistics")

^ permalink raw reply

* Re: [PATCH] net: introduce big and little endian types
From: Nélio Laranjeiro @ 2016-12-05 12:06 UTC (permalink / raw)
  To: Ananyev, Konstantin
  Cc: dev@dpdk.org, Olivier Matz, Lu, Wenzhuo, Adrien Mazarguil
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E3F68@irsmsx105.ger.corp.intel.com>

On Mon, Dec 05, 2016 at 10:09:05AM +0000, Ananyev, Konstantin wrote:
> Hi Neilo,
> 
> > 
> > This commit introduces new rte_{le,be}{16,32,64}_t types and updates
> > rte_{le,be,cpu}_to_{le,be,cpu}_*() and network header structures
> > accordingly.
> > 
> > Specific big/little endian types avoid uncertainty and conversion mistakes.
> > 
> > No ABI change since these are simply typedefs to the original types.
> 
> It seems like quite a lot of changes...
> Could you probably explain what will be the benefit in return?
> Konstantin

Hi Konstantin,

The benefit is to provide documented byte ordering for data types
software is manipulating to determine when network to CPU (or CPU to
network) conversion must be performed.

Regards,

-- 
Nélio Laranjeiro
6WIND

^ permalink raw reply

* [dpdk-dev v2] net/ixgbe:fix incorrect max packet length in ixgbevf
From: Yi Zhang @ 2016-12-06  0:12 UTC (permalink / raw)
  To: maintainer; +Cc: dev, Yi Zhang

Current ixgbevf driver get max_rx_pktlen = 15872, but in fact PF supports
15872-byte jumbo frame and VF only supports 9728-byte jumbo frame. If VF is running
DPDK driver and set frame_size > 9728 ,PF running kernel ixgbe driver will report
an error and set VF failed.
This patch fixs DPDK ixgbevf driver to get correct jumbo frame size of VF.

Signed-off-by: Yi Zhang <zhang.yi75@zte.com.cn>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..573252c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3168,7 +3168,7 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = (uint16_t)hw->mac.max_rx_queues;
 	dev_info->max_tx_queues = (uint16_t)hw->mac.max_tx_queues;
 	dev_info->min_rx_bufsize = 1024; /* cf BSIZEPACKET in SRRCTL reg */
-	dev_info->max_rx_pktlen = 15872; /* includes CRC, cf MAXFRS reg */
+	dev_info->max_rx_pktlen = 9728; /* includes CRC, cf MAXFRS reg */
 	dev_info->max_mac_addrs = hw->mac.num_rar_entries;
 	dev_info->max_hash_mac_addrs = IXGBE_VMDQ_NUM_UC_MAC;
 	dev_info->max_vfs = dev->pci_dev->max_vfs;
-- 
2.9.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox