ter_set can return an error
> - bit-fields vlan_macip have moved
>
> Signed-off-by: Damien Millescamps
> Signed-off-by: Thomas Monjalon
[...]
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
time, this commit
adds type kni_netdev_features_t, which, depending on the kernel version,
translates either to u32 or netdev_features_t.
Signed-off-by: Adrien Mazarguil
---
lib/librte_eal/linuxapp/kni/ethtool/igb/igb_main.c | 6 +++---
lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h |
++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
tions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
+), 3 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
t; lib/librte_eal/linuxapp/eal/eal_lcore.c | 12 ++++----
> 2 files changed, 13 insertions(+), 7 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
.
Signed-off-by: Adrien Mazarguil
---
lib/librte_eal/common/eal_common_memzone.c | 1 +
lib/librte_eal/linuxapp/eal/eal_memory.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_memzone.c
b/lib/librte_eal/common/eal_common_memzone.c
index
heap.c |2 +-
> lib/librte_malloc/malloc_heap.h |2 +-
> lib/librte_malloc/rte_malloc.c |4 ++--
> lib/librte_malloc/rte_malloc.h |2 +-
> 5 files changed, 8 insertions(+), 8 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
-
> lib/librte_eal/linuxapp/eal/eal.c |4
> 2 files changed, 16 insertions(+), 1 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
2 +-
> lib/librte_mempool/rte_mempool.c | 54
> +-
> lib/librte_mempool/rte_mempool.h | 20 +++----
> 3 files changed, 67 insertions(+), 9 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
x, it's much slower than
__rte_mbuf_raw_alloc() due to the unnecessary call to rte_pktmbuf_reset().
Since testpmd is often used for performance testing, we should consider a
wrapper function calling __rte_mbuf_raw_alloc() directly instead, as in
rte_rxmbuf_alloc() implemented in igb and ixgbe PMDs.
--
Adrien Mazarguil
6WIND
rte_mbuf *)mb;
> + m = __rte_mbuf_raw_alloc(mp);
> __rte_mbuf_sanity_check(m, RTE_MBUF_PKT, 1);
> return m;
> }
> --
> 1.7.10.4
Looks good.
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
ompared against C_TO_O instead.
Signed-off-by: Adrien Mazarguil
---
mk/internal/rte.compile-pre.mk | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index ab9a4a8..67454d6 100644
--- a/mk/internal/rte.co
: Adrien Mazarguil
---
doc/guides/nics/mlx4.rst | 85
1 file changed, 78 insertions(+), 7 deletions(-)
diff --git a/doc/guides/nics/mlx4.rst b/doc/guides/nics/mlx4.rst
index b26c219..93a7f57 100644
--- a/doc/guides/nics/mlx4.rst
+++ b/doc/guides
o its devices, the PMD
relies on it.
> I would be happy to contribute to the help page with end-to-end
> instructions for using the Mellanox with testpmd.
I've just submitted a patch to provide more details about the installation
process, you should have a look.
--
Adrien Mazarguil
6WIND
{} while (0)
How about defining it as (void)0 instead of an empty do/while block?
Doing so will prevent warnings if this macro happens to be used in an
expression. RTE_LOG() supports it.
> +#endif
>
> /*
> * The set of PCI devices this driver supports
> --
> 2.2.2
--
Adrien Mazarguil
6WIND
On Thu, Apr 09, 2015 at 09:28:53AM -0700, Stephen Hemminger wrote:
> On Thu, 9 Apr 2015 10:32:24 +0200
> Adrien Mazarguil wrote:
>
> > >
> > > +#ifdef RTE_LIBRTE_ENIC_DEBUG
> > > #define ENICPMD_FUNC_TRACE() \
> > > RTE_LOG(DEBUG, PMD,
This field is not supposed to contain the RX queue index. Applications can
rely on it to determine the port a given mbuf comes from.
Signed-off-by: Adrien Mazarguil
---
lib/librte_pmd_enic/enic.h| 1 +
lib/librte_pmd_enic/enic_ethdev.c | 1 +
lib/librte_pmd_enic/enic_main.c | 4
Signed-off-by: Nelio Laranjeiro
> > Acked-by: Adrien Mazarguil
> >
>
> Applied to dpdk-next-net/master, thanks.
>
> Is not CC'ing stable intentional, since this patch depends on a patch
> introduced in this release? If not intentional, please CC stable.
I
On Thu, Jan 05, 2017 at 05:32:26PM +0100, Adrien Mazarguil wrote:
> Hi Ferruh,
>
> On Thu, Jan 05, 2017 at 03:19:35PM +, Ferruh Yigit wrote:
> > On 12/9/2016 1:27 PM, Nelio Laranjeiro wrote:
> > > Too much data is uselessly written to the Tx doorbell.
> > >
On Thu, Jan 05, 2017 at 05:01:27PM +, Ferruh Yigit wrote:
> On 1/5/2017 4:52 PM, Adrien Mazarguil wrote:
> > On Thu, Jan 05, 2017 at 05:32:26PM +0100, Adrien Mazarguil wrote:
> >> Hi Ferruh,
> >>
> >> On Thu, Jan 05, 2017 at 03:19:35PM +, Ferruh Yig
h may be temporary and
whose symbols are not versioned.
Consider this:
rte_mbuf.h:
#define PKT_TX_RESERVED_0 (1 << 42)
rte_pmd_ixgbe.h:
#define PKT_TX_MACSEC PKT_TX_RESERVED_0
That way, applications have to get the PKT_TX_MACSEC definition where the
rest of the API is also defined.
Other PMDs may reuse PKT_TX_RESERVED_0 and other reserved flags to implement
their own experimental APIs.
Applications and the bonding PMD can easily be made aware that such reserved
flags cannot be shared between ports unless they know what the underlying
PMD is, which is already a requirement to use this API in the first place
(for instance, calling rte_pmd_ixgbe_macsec_*() functions with another
vendor's port_id may crash the application).
So the idea if/when the API is made global is to rename PKT_TX_RESERVED_0 to
PKT_TX_MACSEC and keep its original value.
If other PMDs also implemented PKT_TX_RESERVED_0 in the meantime, it is
redefined using a different value. If there is no room left to do so, these
PMDs are out of luck I guess, and their specific API is disabled/removed
until something gets re-designed.
How about this?
--
Adrien Mazarguil
6WIND
Features
> A new network PMD which supports Solarflare SFN7xxx and SFN8xxx family
>of 10/40 Gbps adapters has been added.
>
> +* **Added support for Mellanox ConnectX-5 adpaters (mlx5).**
Typo, "adpaters".
--
Adrien Mazarguil
6WIND
On Thu, Jan 05, 2017 at 04:49:31PM -0800, Yongseok Koh wrote:
> Add PCI device ID for ConnectX-5 and enable multi-packet send for PF and VF
> along with changing documentation and release note.
>
> Signed-off-by: Yongseok Koh
Acked-by: Adrien Mazarguil
Thanks!
--
Adrien Mazarguil
6WIND
e PKT_TX_RESERVED_0 to
> > PKT_TX_MACSEC and keep its original value.
> >
> > If other PMDs also implemented PKT_TX_RESERVED_0 in the meantime, it is
> > redefined using a different value. If there is no room left to do so, these
> > PMDs are out of luck I guess, and their specific API is disabled/removed
> > until something gets re-designed.
> >
> > How about this?
>
> I still think that we shouldn't allow PMDs to redefine mbuf.olflags and
> dev_info.(rx|tx)_offload_capa.
> See above for my reasons.
I'm still not comfortable with partially global/specific APIs such as this,
because we assume applications are fully aware of the underlying port in
order to use them, while at the same time the related flags are part of the
global namespace, I find it confusing.
If application developers have no problem with that (so far they haven't
commented on this topic, which means they likely agree to go with anything),
I have no reason left to go against the majority.
--
Adrien Mazarguil
6WIND
[1]),
PCI devices must remain bound to their original kernel module (mlx4_core),
however you have to additionally load mlx4_ib, mlx4_en and ib_uverbs [2].
[1] http://dpdk.org/doc/guides/nics/mlx4.html
[2] http://dpdk.org/doc/guides/nics/mlx4.html#prerequisites
--
Adrien Mazarguil
6WIND
Hi Ferruh,
On Fri, Jan 06, 2017 at 01:52:53PM +, Ferruh Yigit wrote:
> On 1/4/2017 6:42 PM, Adrien Mazarguil wrote:
> > Hi Ferruh,
> >
> > On Wed, Jan 04, 2017 at 05:49:46PM +, Ferruh Yigit wrote:
> >> Hi Nelio,
> >>
> >> A quick questio
t?
That's difficult to say without knowing your specific setup or application,
however 6 Mpps seems abnormally slow assuming testpmd performing basic I/O
forwarding using a single thread and two ports.
> On Mon, Jan 9, 2017 at 11:13 PM, Adrien Mazarguil <
> adrien.mazarg...@6wind.com> wr
This series makes a few adjustments to the flow API (rte_flow) based on
issues encountered by preliminary PMD implementations and addresses bugs
reported by Coverity in the testpmd flow command.
Adrien Mazarguil (5):
app/testpmd: fix array bounds checks
ethdev: modify flow API's error se
This commit addresses several obvious issues reported by Coverity (139596,
139597, 139598 and 139599) with array bounds checks in functions related to
the flow API.
Fixes: 938a184a1870 ("app/testpmd: implement basic support for flow API")
Signed-off-by: Adrien Mazarguil
---
ap
Based on initial PMD implementations of the flow API, returning the error
structure which may be NULL is useless and always discarded.
Returning the error code instead appears to be much more convenient.
Signed-off-by: Adrien Mazarguil
Cc: Nelio Laranjeiro
Cc: Wei Zhao
Cc: Beilei Xing
Both actions share the PKT_RX_FDIR mbuf flag, as a result there is no way
to tell them apart. Moreover, the maximum allowed value for the MARK action
may not necessarily cover the entire 32-bit space.
Signed-off-by: Adrien Mazarguil
Cc: John McNamara
---
doc/guides/prog_guide/rte_flow.rst | 24
Contrary to the current description, mbuf RSS hash result storage does not
overlap with the returned MARK value (hash.fdir.lo vs. hash.fdir.hi), and
both may be combined.
Reflect this change by allowing testpmd to display both values
simultaneously.
Signed-off-by: Adrien Mazarguil
Cc: Jingjing
-off-by: Adrien Mazarguil
Cc: John McNamara
Cc: Nelio Laranjeiro
Cc: Wei Zhao
Cc: Beilei Xing
---
doc/guides/prog_guide/rte_flow.rst | 25 ++--
lib/librte_ether/rte_flow.h| 110 +---
2 files changed, 121 insertions(+), 14 deletions(-)
diff --git a/doc
{
> + struct priv *priv = mlx5_get_priv(dev);
> + int ret = xstats_n;
> +
> + if (n >= xstats_n && stats) {
> + priv_lock(priv);
> + ret = priv_xstats_get(priv, stats);
> + priv_unlock(priv);
> + }
> + ret
r own separate
patch?
You could use VLAN PCP/DEI/VID definitions as an example to expose partial
bit-fields (e.g. epcp_edei_in_ecid_b) in testpmd, see:
1419fd5a6c9f ("app/testpmd: add protocol fields to flow command")
Now if re-spinning this series yet again is too much work, you can go ahead
with this commit as long as you do not forget to submit the rest later,
thanks.
--
Adrien Mazarguil
6WIND
+++
> drivers/net/mlx5/mlx5_trigger.c | 1 +
> 5 files changed, 346 insertions(+)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
y: Olivier Matz
> ---
> drivers/net/mlx4/mlx4.c| 32 ++--
> drivers/net/mlx5/mlx5.c| 2 +-
> drivers/net/mlx5/mlx5.h| 1 -
> drivers/net/mlx5/mlx5_ethdev.c | 30 ++--------
> 4 files changed, 13 insertions(+), 52 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
+++--
> drivers/net/mlx5/mlx5_rxtx.c | 12 ++--
> 2 files changed, 18 insertions(+), 12 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
On Mon, Jan 16, 2017 at 04:39:08PM +, Ferruh Yigit wrote:
> On 1/16/2017 1:03 PM, Adrien Mazarguil wrote:
> > Hi,
> >
> > On Fri, Jan 13, 2017 at 04:13:08PM +0800, Wei Zhao wrote:
> >> check if the rule is a L2 tunnel rule, and get the L2 tunnel info.
>
time. Not sure we should prevent this change as long as such rules can
be configured through rte_flow as well.
[1]
http://dpdk.org/doc/guides/prog_guide/rte_flow.html#tunnel-to-eth-ipv4-ipv6-vxlan-or-other-queue
--
Adrien Mazarguil
6WIND
; +++-----
> 2 files changed, 50 insertions(+), 47 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
On Fri, Jan 20, 2017 at 04:27:28PM +0100, Nelio Laranjeiro wrote:
> Fixes: ea3bc3b1df94 ("net/mlx5: support mark flow action")
>
> Signed-off-by: Nelio Laranjeiro
> ---
> drivers/net/mlx5/mlx5_flow.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
ions(+), 4 deletions(-)
Thanks for addressing this issue.
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
gt; drivers/net/mlx5/mlx5.c | 5 +++--
> drivers/net/mlx5/mlx5_defs.h | 3 ---
> 2 files changed, 3 insertions(+), 5 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
t; to reduce jitter in rx_burst.
>
> Signed-off-by: Yongseok Koh
> ---
> drivers/net/mlx5/mlx5_rxtx.c | 23 ---
> 1 file changed, 20 insertions(+), 3 deletions(-)
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
Promote Nelio as additional maintainer for mlx4 and mlx5 PMDs.
Signed-off-by: Adrien Mazarguil
Acked-by: Nelio Laranjeiro
Acked-by: Olga Shern
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index f071138..541c888 100644
--- a/MAINTAINERS
+++ b
are unlikely to be fully met (i.e. PMDs usually do not check for
completions in the middle of a TX burst).
> > Konstantin
> >
> The main advantage I see is that it should make it a bit easier for
> driver writers, since they have a tighter set of constraints to work
> with for packet RX and Tx. The routines only have to handle requests for
> packets in the range 0-N, rather than not having an upper bound on the
> request. It also then saves code duplicating with having multiple
> drivers having the same outer-loop code for handling arbitrarily large
> requests.
>
> No big deal to me either way though.
>
> /Bruce
Right but there is a cost in doing so, as unlikely() as the additional code
is. We should leave that choice to applications.
--
Adrien Mazarguil
6WIND
think assert()/rte_assert() would make more sense and should fool Coverity
into understanding the expected behavior.
The commit log should reflect that we are addressing a false positive since
there is no problem with the code logic (of course unless I missed anything
obvious).
Thanks.
--
Adrien Mazarguil
6WIND
that file and cause the entire
PMD to be even more OS-dependent.
We'll move this check elsewhere in the future if we need several such
workarounds, thanks.
> > > struct priv *priv = mlx5_get_priv(dev);
> > > struct ethtool_link_settings edata = {
> > > .cmd = ETHTOOL_GLINKSETTINGS,
> > <...>
--
Adrien Mazarguil
6WIND
s follows with minimal impact
on their performance and no ABI change:
uint16_t sent = 0;
int error; // new variable
[process burst]
if (unlikely([something went wrong])) { // this check already exists
error = EPROBLEM; // new assignment
goto error; // instead of "return sent"
}
[process burst]
return sent;
error:
rte_errno = error;
return sent;
--
Adrien Mazarguil
6WIND
On Wed, Feb 01, 2017 at 11:13:59AM +, Ferruh Yigit wrote:
> On 2/1/2017 9:07 AM, Adrien Mazarguil wrote:
> > On Wed, Feb 01, 2017 at 06:53:55AM +, Shahaf Shuler wrote:
> >> : Tuesday, January 31, 2017 6:17 PM, Ferruh Yigit:
> >>> On 1/31/2017 11:45 AM, Shahaf
On Wed, Feb 01, 2017 at 06:11:17PM +, Ferruh Yigit wrote:
> On 2/1/2017 12:57 PM, Adrien Mazarguil wrote:
> > On Wed, Feb 01, 2017 at 11:13:59AM +, Ferruh Yigit wrote:
> >> On 2/1/2017 9:07 AM, Adrien Mazarguil wrote:
> >>> On Wed, Feb 01, 2017 at 06:53:5
On Thu, Feb 02, 2017 at 10:15:08AM +, Ferruh Yigit wrote:
> On 2/2/2017 8:45 AM, Adrien Mazarguil wrote:
> > On Wed, Feb 01, 2017 at 06:11:17PM +, Ferruh Yigit wrote:
> >> On 2/1/2017 12:57 PM, Adrien Mazarguil wrote:
> >>> On Wed, Feb 01, 2017 at 11:13:
On Thu, Sep 29, 2016 at 07:30:31PM +, De Lara Guarch, Pablo wrote:
> > -Original Message-
> > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > Sent: Tuesday, September 27, 2016 12:45 AM
> > To: De Lara Guarch, Pablo
> > Cc: dev at dpdk.or
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
> 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
> > > > resul
t; >
> > > > In my opinion, users should not do preparation on their own.
> > >
> > > People already do it now.
> >
> > But we do not want them to anymore thanks to this new API, for reasons
> > described in the motivation section of the cover letter, right?
>
> We probably wouldn't recommend that, but if people would like to use their
> own stuff,
> or shortcuts - I don't want to stop them here.
>
> >
> > > > 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.
> >
> > It should work, only if they keep doing it as well as call tx_burst()
> > directly, they will likely get lower performance.
> >
> > > > 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.
> >
> > You're right, however applications might find it convenient. I think most
> > will end up with something like the following:
> >
> > if (tx_prepare(pkts))
> > tx_burst(pkts));
>
> Looking at existing DPDK apps - most of them do use some sort of TX
> bufferization.
> So, even in a simplistic app it would probably be:
>
> tx_prepare(pkts);
> tx_buffer(pkts);
We're down to my word against yours here I guess, to leave the choice to
application developers, we'd need to provide tx_prepare() and a simpler
tx_burst() as well as the ability to call tx_burst() directly and still get
offloads.
> > > > 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.
> >
> > I think it would be easier to document what offload flags may cause the
> > tx_burst() function to modify mbuf contents, so applications have the
> > ability to set or strip these flags on a mbuf basis. That way there is no
> > need to call tx_prepare() without knowing exactly what it's going to do.
>
> Not sure I understand what exactly do you propose in the last paragraph?
That for each TX offload flag, we document whether preparation might cause a
mbuf to be written to during the tx_prepare()/tx_burst() phase. One of the
reasons for tx_prepare() being:
4) Fields in packet may require different initialization (like e.g. will
require pseudo-header checksum precalculation, sometimes in a
different way depending on packet type, and so on). Now application
needs to care about it.
If we determine what offloads may cause mbuf contents to change (all of them
perhaps?), then applications can easily strip those flags from outgoing
"const" mbufs. Then it becomes acceptable for tx_burst() to modify mbuf
contents as per user request, which removes one reason to rely on
tx_prepare() for these.
--
Adrien Mazarguil
6WIND
o this new API, for reasons
> > > > described in the motivation section of the cover letter, right?
> > >
> > > We probably wouldn't recommend that, but if people would like to use
> > > their own stuff,
> > > or shortcuts - I don't want to stop them here.
> > >
> > > >
> > > > > > 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.
> > > >
> > > > It should work, only if they keep doing it as well as call tx_burst()
> > > > directly, they will likely get lower performance.
> > > >
> > > > > > 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.
> > > >
> > > > You're right, however applications might find it convenient. I think
> > > > most
> > > > will end up with something like the following:
> > > >
> > > > if (tx_prepare(pkts))
> > > > tx_burst(pkts));
> > >
> > > Looking at existing DPDK apps - most of them do use some sort of TX
> > > bufferization.
> > > So, even in a simplistic app it would probably be:
> > >
> > > tx_prepare(pkts);
> > > tx_buffer(pkts);
> >
> > We're down to my word against yours here I guess, to leave the choice to
> > application developers, we'd need to provide tx_prepare() and a simpler
> > tx_burst() as well as the ability to call tx_burst() directly and still get
> > offloads.
>
> From what I've seen, most DPDK libs/apps do buffer data packets for TX in one
> or another way:
> mtcp, warp17, seastar.
I was arguing most applications wouldn't call tx_prepare() far from
tx_burst() and that merging them would therefore make sense, which seems to
be the case in these examples. Obviously they're doing bufferization.
- warp17 would call tx_prepare() inside pkt_flush_tx_q() right before
tx_burst(), it wouldn't make sense elsewhere.
- mtcp has an interesting loop on tx_burst(), however again, tx_prepare()
would be performed inside dpdk_send_pkts(), not where mbufs are
populated.
- seastar seems to expose the ability to send bursts as well,
dpdk_qp::_send() would also call tx_prepare() before tx_burst().
> Not to mention sample apps.
> But ok, as said above, if you can prove that tx_burst() you are proposing is
> really much faster then
> tx_prepare(); tx_burst();
> I'll be happy to reconsider.
Nowhere I wrote "much" faster, I was just pointing out it would be more
efficient if the work was done in a single function, at least for the use
cases found in several examples you've provided.
Now if you tell me it cannot be significant, fine, but still.
> > > > > > 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.
> > > >
> > > > I think it would be easier to document what offload flags may cause the
> > > > tx_burst() function to modify mbuf contents, so applications have the
> > > > ability to set or strip these flags on a mbuf basis. That way there is
> > > > no
> > > > need to call tx_prepare() without knowing exactly what it's going to do.
> > >
> > > Not sure I understand what exactly do you propose in the last paragraph?
> >
> > That for each TX offload flag, we document whether preparation might cause a
> > mbuf to be written to during the tx_prepare()/tx_burst() phase. One of the
> > reasons for tx_prepare() being:
> >
> > 4) Fields in packet may require different initialization (like e.g. will
> > require pseudo-header checksum precalculation, sometimes in a
> > different way depending on packet type, and so on). Now application
> > needs to care about it.
> >
> > If we determine what offloads may cause mbuf contents to change (all of them
> > perhaps?), then applications can easily strip those flags from outgoing
> > "const" mbufs. Then it becomes acceptable for tx_burst() to modify mbuf
> > contents as per user request, which removes one reason to rely on
> > tx_prepare() for these.
>
> Hmm, I didn't get you here.
> If let say I don't need TX TCP cksum offload, why would I set this flag
> inside mbuf at first place?
> If I do expect PMD to do TX TCP cksum offload for me, then I have to set
> that flag,
> otherwise how PMD would know that I did require that offload?
Let me take an example, if you assume a cloned mbuf must be sent, pointed
data is not owned by the TX function, and software preparation for TX
offloads might cause side effects elsewhere, I'm not talking about the
changes performed by HW to the outgoing frame here.
One reason for tx_prepare() according to 4) is to make this clear; by
calling tx_prepare(), applications fully acknowledge and take responsibility
for possible side-effects.
Hence my suggestion, we could make this acceptable for tx_burst() by
documenting that requesting offloads may cause it to modify mbuf contents as
well.
Before you reply, note that I've neither acked nor nacked that series, and
intend to leave this to other people. So far it's received several acks
already so I guess it's probably ready for inclusion.
I'm not comfortable with it as an application developer because of the extra
API call that I think could be included in tx_burst(), but as a PMD
maintainer I do not mind. Might even prove useful someday if some
corner-case offload too expensive to fully handle in tx_burst() had to be
supported. An optional offload if you will.
--
Adrien Mazarguil
6WIND
ination MAC. */
> > + struct ether_addr src; /**< Source MAC. */
> > + unsigned int type; /**< EtherType. */
> Hi Adrien,
>
> ETHERTYPE in ether header is 2 bytes, so I think "uint16_t type" is more
> appropriate here, what do you think?
You're right, thanks for catching this. I'll update it in v2 (soon).
--
Adrien Mazarguil
6WIND
hey may even switch to
another mode if necessary assuming this does not break existing constraints.
I think we've discussed an atomic (commit-based) mode of operation through
separate functions as well, where the application would attempt to create a
bunch of rules at once, possibly making it easier for PMDs to determine the
most appropriate mode of operation for the device.
All of these may be added later according to users feedback once the basic
API has settled.
--
Adrien Mazarguil
6WIND
Hi Ferruh,
On Fri, Dec 02, 2016 at 04:58:53PM +, Ferruh Yigit wrote:
> Hi Adrien,
>
> On 11/16/2016 4:23 PM, Adrien Mazarguil wrote:
> > As previously discussed in RFC v1 [1], RFC v2 [2], with changes
> > described in [3] (also pasted below), here is the first non-draf
On Fri, Dec 02, 2016 at 09:06:42PM +, Kevin Traynor wrote:
> On 12/01/2016 08:36 AM, Adrien Mazarguil wrote:
> > Hi Kevin,
> >
> > On Wed, Nov 30, 2016 at 05:47:17PM +, Kevin Traynor wrote:
> >> Hi Adrien,
> >>
> >> On 11/16/2016 04:2
think?
What you suggest (device profile configuration) would be done by a separate
function in any case, so as long as everyone agrees on a generic method to
do so, no problem with extending rte_flow. By default in the meantime we'll
have to rely on PMDs to make the right decision.
Do you think it has to be defined from the beginning?
--
Adrien Mazarguil
6WIND
Hi Sugesh,
On Mon, Dec 12, 2016 at 10:20:18AM +, Chandran, Sugesh wrote:
> Hi Adrien,
>
> Regards
> _Sugesh
>
> > -Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com]
> > Sent: Friday, December 9, 2016 4:39 PM
> &
re, not query the rules I think I
> >> have. I don't see a way to do it or something to build a helper on top of?
> >
> > Generic helper functions would exist on top of this API and would likely
> > maintain a list of flow rules themselves. The dump in that case would be
> > entirely implemented in software. I think that recovering flow rules from HW
> > may be complicated in many cases (even without taking storage allocation and
> > rules conversion issues into account), therefore if there is really a need
> > for it, we could perhaps add a dump() function that PMDs are free to
> > implement later.
> >
>
> ok. Maybe there are some more generic stats that can be got from the
> hardware that would help debugging that would suffice, like total flow
> rule hits/misses (i.e. not on a per flow rule basis).
>
> You can get this from the software flow caches and it's widely used for
> debugging. e.g.
>
> pmd thread numa_id 0 core_id 3:
> emc hits:0
> megaflow hits:0
> avg. subtable lookups per hit:0.00
> miss:0
>
Perhaps a rule such as the following could do the trick:
group: 42 (or priority 42)
pattern: void
actions: count / passthru
Assuming useful flow rules are defined with higher priorities (using lower
group ID or priority level) and provide a terminating action, this one would
count all packets that were not caught by them.
That is one example to illustrate how "global" counters can be requested by
applications.
Otherwise you could just make sure all rules contain mark / flag actions, in
which case mbufs would tell directly if they went through them or need
additional SW processing.
--
Adrien Mazarguil
6WIND
On Thu, Dec 15, 2016 at 12:20:36PM +, Ferruh Yigit wrote:
> On 12/8/2016 3:19 PM, Adrien Mazarguil wrote:
> > Hi Ferruh,
> >
> > On Fri, Dec 02, 2016 at 04:58:53PM +, Ferruh Yigit wrote:
> >> Hi Adrien,
> >>
> >> On 11/16/2016 4:23 PM, Adrie
> Acked-by: Fiona Trahe
> ---
>
> v2 -> v3:
> - fix kmods deps advertised by mellanox drivers as pointed out
> by Adrien
Acked-by: Adrien Mazarguil
--
Adrien Mazarguil
6WIND
> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix
00:00:00:00:09:00
All these will be properly documented in the v2 patchset. Note, this version
will replace the "fix" keyword with "is" ("fix" made no sense according to
feedback).
--
Adrien Mazarguil
6WIND
discussed this before.
> >
> >
> > I too thought something similar was discussed. I tried searching the
> > archives but couldn't find anything - thus, I thought probably I was
> > hallucinating :P
> >
> > So, you want me to revert back the '()' change? Does it impact the expansion
> > of this macro?
>
> We haven't added this on any other usage of the __extension__ keyword
> in the existing code. From my perspective it is more consistent to
> revert it.
>
> Anyone else with an opinion here? David? Thomas?
As an exported header, rte_common.h must pass check-includes.sh. Both
typeof() and the ({ ... }) construct are non-standard GCC extensions and
would fail to compile with pedantic options.
--
Adrien Mazarguil
6WIND
On Fri, Dec 16, 2016 at 11:47:14AM +0100, Jan Blunck wrote:
> On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
> wrote:
> > On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> >> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain
> >> wrote:
> >>
e check? Thanks.
There is indeed a bug [1]. It is fixed in v2.
Thanks.
[1] http://dpdk.org/ml/archives/dev/2016-November/050435.html
--
Adrien Mazarguil
6WIND
self-standing, no need to look up elsewhere.
Existing filter types will be deprecated and removed in the near future.
Signed-off-by: Adrien Mazarguil
---
MAINTAINERS| 4 +
doc/api/doxy-api-index.md | 2 +
lib/librte_ether/Makefile | 3
API documentation (based on RFC).
- testpmd flow command documentation (although context-aware command
completion should already help quite a bit in this regard).
- A few pattern item / action properties cannot be configured yet
(e.g. rss_conf parameter for RSS action) and a few completions
This documentation is based on the latest RFC submission, subsequently
updated according to feedback from the community.
Signed-off-by: Adrien Mazarguil
---
doc/guides/prog_guide/index.rst|1 +
doc/guides/prog_guide/rte_flow.rst | 1853 +++
2 files changed
They are superseded by the generic flow API (rte_flow). Target release is
not defined yet.
Suggested-by: Kevin Traynor
Signed-off-by: Adrien Mazarguil
---
doc/guides/rel_notes/deprecation.rst | 7 +++
1 file changed, 7 insertions(+)
diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc
).
Because no structures are modified (existing fields are reused), this
commit has no impact on the current ABI.
Signed-off-by: Adrien Mazarguil
---
lib/librte_cmdline/cmdline_parse.c | 60 +
lib/librte_cmdline/cmdline_parse.h | 21
2 files changed, 74
This prevents sigbus errors on architectures that cannot handle unexpected
unaligned accesses to the output buffer.
Signed-off-by: Adrien Mazarguil
---
lib/librte_cmdline/cmdline_parse.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/librte_cmdline
Add basic management functions for the generic flow API (validate, create,
destroy, flush, query and list). Flow rule objects and properties are
arranged in lists associated with each port.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline.c | 1 +
app/test-pmd/config.c | 485
Managing generic flow API functions from command line requires the use of
dynamic tokens for convenience as flow rules are not fixed and cannot be
defined statically.
This commit adds specific flexible parser code and object for a new "flow"
command in separate file.
Signed-off-
Syntax:
flow destroy {port_id} rule {rule_id} [...]
Destroy a given set of flow rules associated with a port.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline.c | 3 ++
app/test-pmd/cmdline_flow.c | 106 ++-
2 files changed, 108 insertions
Parse all integer types and handle conversion to network byte order in a
single function.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 148 +++
1 file changed, 148 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd
Syntax:
flow flush {port_id}
Destroy all flow rules on a port.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline.c | 3 +++
app/test-pmd/cmdline_flow.c | 43 +++-
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd
Syntax:
flow query {port_id} {rule_id} {action}
Query a specific action of an existing flow rule.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline.c | 3 +
app/test-pmd/cmdline_flow.c | 121 ++-
2 files changed, 123 insertions(+), 1 deletion
match.
- PASSTHRU: action that leaves packets up for additional processing by
subsequent flow rules.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline.c | 14 ++
app/test-pmd/cmdline_flow.c | 314 ++-
2 files changed, 327 insertions(+), 1
ask: sets bit-mask affecting both spec and last from arbitrary value.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 111 +++
1 file changed, 111 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8f7e
Several rte_flow structures expose bit-fields that cannot be set in a
generic fashion at byte level. Add bit-mask support to handle them.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 59
1 file changed, 59 insertions(+)
diff --git a
Generating bit-masks from prefix lengths is often more convenient than
providing them entirely (e.g. to define IPv4 and IPv6 subnets).
This commit adds the "prefix" operator that assigns generated bit-masks to
any pattern item specification field.
Signed-off-by: Adrien Mazarguil
---
This pattern item matches any protocol in place of the current layer and
has two properties:
- min: minimum number of layers covered (0 or more).
- max: maximum number of layers covered (0 means infinity).
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 23
- PF: match packets addressed to the physical function.
- VF: match packets addressed to a virtual function ID.
- PORT: device-specific physical port index to use.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 53
1 file changed, 53
: byte string to look for.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 208 +++
1 file changed, 208 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index ac93679..dafb07f 100644
--- a/app/test-pmd
Add the ability to match basic fields from IPv4 and IPv6 headers (source
and destination addresses only).
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 177 +++
1 file changed, 177 insertions(+)
diff --git a/app/test-pmd/cmdline_flow.c b
These pattern items match basic Ethernet headers (source, destination and
type) and related 802.1Q/ad VLAN headers.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 126 +++
1 file changed, 126 insertions(+)
diff --git a/app/test-pmd
- MARK: attach 32 bit value to packets.
- FLAG: flag packets.
- DROP: drop packets.
- COUNT: enable counters for a rule.
- PF: redirect packets to physical device function.
- VF: redirect packets to virtual device function.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 121
Add the ability to match a few properties of common L4[.5] protocol
headers:
- ICMP: type and code.
- UDP: source and destination ports.
- TCP: source and destination ports.
- SCTP: source and destination ports.
- VXLAN: network identifier.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd
- QUEUE: assign packets to a given queue index.
- DUP: duplicate packets to a given queue index.
- RSS: spread packets among several queues.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline_flow.c | 152 +++
1 file changed, 152 insertions(+)
diff
Document syntax, interaction with rte_flow and provide usage examples.
Signed-off-by: Adrien Mazarguil
---
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 612 +++
1 file changed, 612 insertions(+)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides
Syntax:
flow list {port_id} [group {group_id}] [...]
List configured flow rules on a port. Output can optionally be limited to a
given set of group identifiers.
Signed-off-by: Adrien Mazarguil
---
app/test-pmd/cmdline.c | 4 ++
app/test-pmd/cmdline_flow.c | 141
Hi Beilei,
On Mon, Dec 19, 2016 at 08:37:20AM +, Xing, Beilei wrote:
> Hi Adrien,
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Saturday, December 17, 2016 12:25 AM
> > To: dev@dpdk.org
>
Hi John,
On Mon, Dec 19, 2016 at 10:45:32AM +, Mcnamara, John wrote:
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Friday, December 16, 2016 4:25 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-
1 - 100 of 1899 matches
Mail list logo
