* [dpdk-dev] [PATCH 2/2] net/sfc: free mbufs in bulks on simple EF10 Tx datapath reap
2017-09-08 14:15 [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap Andrew Rybchenko
@ 2017-09-08 14:15 ` Andrew Rybchenko
2017-09-12 18:28 ` Ferruh Yigit
2017-09-12 18:26 ` [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native " Ferruh Yigit
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Andrew Rybchenko @ 2017-09-08 14:15 UTC (permalink / raw)
To: dev; +Cc: Ivan Malov
From: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
doc/guides/nics/sfc_efx.rst | 4 +++-
drivers/net/sfc/sfc_dp_tx.h | 2 ++
drivers/net/sfc/sfc_ef10_tx.c | 15 ++++++++++++++-
drivers/net/sfc/sfc_ethdev.c | 6 ++++++
drivers/net/sfc/sfc_tx.c | 17 +++++++++++++++++
5 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 973a4a0..028b980 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -245,12 +245,14 @@ boolean parameters value.
features available and required by the datapath implementation.
**efx** chooses libefx-based datapath which supports VLAN insertion
(full-feature firmware variant only), TSO and multi-segment mbufs.
+ Mbuf segments may come from different mempools, and mbuf reference
+ counters are treated responsibly.
**ef10** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which is
more efficient than libefx-based but has no VLAN insertion and TSO
support yet.
**ef10_simple** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which
is even more faster then **ef10** but does not support multi-segment
- mbufs.
+ mbufs, disallows multiple mempools and neglects mbuf reference counters.
- ``perf_profile`` [auto|throughput|low-latency] (default **throughput**)
diff --git a/drivers/net/sfc/sfc_dp_tx.h b/drivers/net/sfc/sfc_dp_tx.h
index db2a70b..94d1b10 100644
--- a/drivers/net/sfc/sfc_dp_tx.h
+++ b/drivers/net/sfc/sfc_dp_tx.h
@@ -142,6 +142,8 @@ struct sfc_dp_tx {
#define SFC_DP_TX_FEAT_TSO 0x2
#define SFC_DP_TX_FEAT_MULTI_SEG 0x4
#define SFC_DP_TX_FEAT_MULTI_PROCESS 0x8
+#define SFC_DP_TX_FEAT_MULTI_POOL 0x10
+#define SFC_DP_TX_FEAT_REFCNT 0x20
sfc_dp_tx_qcreate_t *qcreate;
sfc_dp_tx_qdestroy_t *qdestroy;
sfc_dp_tx_qstart_t *qstart;
diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c
index 5127a7a..9047b3e 100644
--- a/drivers/net/sfc/sfc_ef10_tx.c
+++ b/drivers/net/sfc/sfc_ef10_tx.c
@@ -401,14 +401,25 @@ struct sfc_ef10_txq {
pending += sfc_ef10_tx_process_events(txq);
if (pending != completed) {
+ struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
+ unsigned int nb = 0;
+
do {
struct sfc_ef10_tx_sw_desc *txd;
txd = &txq->sw_ring[completed & ptr_mask];
- rte_pktmbuf_free_seg(txd->mbuf);
+ if (nb == RTE_DIM(bulk)) {
+ rte_mempool_put_bulk(bulk[0]->pool,
+ (void *)bulk, nb);
+ nb = 0;
+ }
+
+ bulk[nb++] = txd->mbuf;
} while (++completed != pending);
+ rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
+
txq->completed = completed;
}
@@ -614,6 +625,8 @@ struct sfc_dp_tx sfc_ef10_tx = {
.hw_fw_caps = SFC_DP_HW_FW_CAP_EF10,
},
.features = SFC_DP_TX_FEAT_MULTI_SEG |
+ SFC_DP_TX_FEAT_MULTI_POOL |
+ SFC_DP_TX_FEAT_REFCNT |
SFC_DP_TX_FEAT_MULTI_PROCESS,
.qcreate = sfc_ef10_tx_qcreate,
.qdestroy = sfc_ef10_tx_qdestroy,
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 2b037d8..26e8c93 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -145,6 +145,12 @@
if (~sa->dp_tx->features & SFC_DP_TX_FEAT_MULTI_SEG)
dev_info->default_txconf.txq_flags |= ETH_TXQ_FLAGS_NOMULTSEGS;
+ if (~sa->dp_tx->features & SFC_DP_TX_FEAT_MULTI_POOL)
+ dev_info->default_txconf.txq_flags |= ETH_TXQ_FLAGS_NOMULTMEMP;
+
+ if (~sa->dp_tx->features & SFC_DP_TX_FEAT_REFCNT)
+ dev_info->default_txconf.txq_flags |= ETH_TXQ_FLAGS_NOREFCOUNT;
+
#if EFSYS_OPT_RX_SCALE
if (sa->rss_support != EFX_RX_SCALE_UNAVAILABLE) {
dev_info->reta_size = EFX_RSS_TBL_SIZE;
diff --git a/drivers/net/sfc/sfc_tx.c b/drivers/net/sfc/sfc_tx.c
index bf59601..4ea7bd7 100644
--- a/drivers/net/sfc/sfc_tx.c
+++ b/drivers/net/sfc/sfc_tx.c
@@ -91,6 +91,21 @@
rc = EINVAL;
}
+ if (((flags & ETH_TXQ_FLAGS_NOMULTMEMP) == 0) &&
+ (~sa->dp_tx->features & SFC_DP_TX_FEAT_MULTI_POOL)) {
+ sfc_err(sa, "multi-mempool is not supported by %s datapath",
+ sa->dp_tx->dp.name);
+ rc = EINVAL;
+ }
+
+ if (((flags & ETH_TXQ_FLAGS_NOREFCOUNT) == 0) &&
+ (~sa->dp_tx->features & SFC_DP_TX_FEAT_REFCNT)) {
+ sfc_err(sa,
+ "mbuf reference counters are neglected by %s datapath",
+ sa->dp_tx->dp.name);
+ rc = EINVAL;
+ }
+
if ((flags & ETH_TXQ_FLAGS_NOVLANOFFL) == 0) {
if (!encp->enc_hw_tx_insert_vlan_enabled) {
sfc_err(sa, "VLAN offload is not supported");
@@ -1023,6 +1038,8 @@ struct sfc_dp_tx sfc_efx_tx = {
},
.features = SFC_DP_TX_FEAT_VLAN_INSERT |
SFC_DP_TX_FEAT_TSO |
+ SFC_DP_TX_FEAT_MULTI_POOL |
+ SFC_DP_TX_FEAT_REFCNT |
SFC_DP_TX_FEAT_MULTI_SEG,
.qcreate = sfc_efx_tx_qcreate,
.qdestroy = sfc_efx_tx_qdestroy,
--
1.8.2.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH 2/2] net/sfc: free mbufs in bulks on simple EF10 Tx datapath reap
2017-09-08 14:15 ` [dpdk-dev] [PATCH 2/2] net/sfc: free mbufs in bulks on simple EF10 " Andrew Rybchenko
@ 2017-09-12 18:28 ` Ferruh Yigit
2017-09-13 9:32 ` Andrew Rybchenko
0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2017-09-12 18:28 UTC (permalink / raw)
To: Andrew Rybchenko, dev; +Cc: Ivan Malov
On 9/8/2017 3:15 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> doc/guides/nics/sfc_efx.rst | 4 +++-
> drivers/net/sfc/sfc_dp_tx.h | 2 ++
> drivers/net/sfc/sfc_ef10_tx.c | 15 ++++++++++++++-
> drivers/net/sfc/sfc_ethdev.c | 6 ++++++
> drivers/net/sfc/sfc_tx.c | 17 +++++++++++++++++
> 5 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
> index 973a4a0..028b980 100644
> --- a/doc/guides/nics/sfc_efx.rst
> +++ b/doc/guides/nics/sfc_efx.rst
> @@ -245,12 +245,14 @@ boolean parameters value.
> features available and required by the datapath implementation.
> **efx** chooses libefx-based datapath which supports VLAN insertion
> (full-feature firmware variant only), TSO and multi-segment mbufs.
> + Mbuf segments may come from different mempools, and mbuf reference
> + counters are treated responsibly.
This is also the case for ef10 native, right? Does it make sense to
document it in below too?
> **ef10** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which is
> more efficient than libefx-based but has no VLAN insertion and TSO
> support yet.
> **ef10_simple** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which
> is even more faster then **ef10** but does not support multi-segment
> - mbufs.
> + mbufs, disallows multiple mempools and neglects mbuf reference counters.
>
> - ``perf_profile`` [auto|throughput|low-latency] (default **throughput**)
>
<...>
> --- a/drivers/net/sfc/sfc_ef10_tx.c
> +++ b/drivers/net/sfc/sfc_ef10_tx.c
> @@ -401,14 +401,25 @@ struct sfc_ef10_txq {
> pending += sfc_ef10_tx_process_events(txq);
>
> if (pending != completed) {
> + struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
> + unsigned int nb = 0;
> +
> do {
> struct sfc_ef10_tx_sw_desc *txd;
>
> txd = &txq->sw_ring[completed & ptr_mask];
>
> - rte_pktmbuf_free_seg(txd->mbuf);
> + if (nb == RTE_DIM(bulk)) {
> + rte_mempool_put_bulk(bulk[0]->pool,
> + (void *)bulk, nb);
same warning here, again false positive I think:
error #3656: variable "bulk" may be used before its value is set
The patch to ignore the warning will take care of this one too.
> + nb = 0;
> + }
> +
> + bulk[nb++] = txd->mbuf;
> } while (++completed != pending);
>
> + rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
> +
> txq->completed = completed;
> }
<...>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH 2/2] net/sfc: free mbufs in bulks on simple EF10 Tx datapath reap
2017-09-12 18:28 ` Ferruh Yigit
@ 2017-09-13 9:32 ` Andrew Rybchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2017-09-13 9:32 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: Ivan Malov
On 09/12/2017 09:28 PM, Ferruh Yigit wrote:
> On 9/8/2017 3:15 PM, Andrew Rybchenko wrote:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> doc/guides/nics/sfc_efx.rst | 4 +++-
>> drivers/net/sfc/sfc_dp_tx.h | 2 ++
>> drivers/net/sfc/sfc_ef10_tx.c | 15 ++++++++++++++-
>> drivers/net/sfc/sfc_ethdev.c | 6 ++++++
>> drivers/net/sfc/sfc_tx.c | 17 +++++++++++++++++
>> 5 files changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
>> index 973a4a0..028b980 100644
>> --- a/doc/guides/nics/sfc_efx.rst
>> +++ b/doc/guides/nics/sfc_efx.rst
>> @@ -245,12 +245,14 @@ boolean parameters value.
>> features available and required by the datapath implementation.
>> **efx** chooses libefx-based datapath which supports VLAN insertion
>> (full-feature firmware variant only), TSO and multi-segment mbufs.
>> + Mbuf segments may come from different mempools, and mbuf reference
>> + counters are treated responsibly.
> This is also the case for ef10 native, right? Does it make sense to
> document it in below too?
Thanks, will add.
>> **ef10** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which is
>> more efficient than libefx-based but has no VLAN insertion and TSO
>> support yet.
>> **ef10_simple** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which
>> is even more faster then **ef10** but does not support multi-segment
>> - mbufs.
>> + mbufs, disallows multiple mempools and neglects mbuf reference counters.
>>
>> - ``perf_profile`` [auto|throughput|low-latency] (default **throughput**)
>>
> <...>
>
>> --- a/drivers/net/sfc/sfc_ef10_tx.c
>> +++ b/drivers/net/sfc/sfc_ef10_tx.c
>> @@ -401,14 +401,25 @@ struct sfc_ef10_txq {
>> pending += sfc_ef10_tx_process_events(txq);
>>
>> if (pending != completed) {
>> + struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
>> + unsigned int nb = 0;
>> +
>> do {
>> struct sfc_ef10_tx_sw_desc *txd;
>>
>> txd = &txq->sw_ring[completed & ptr_mask];
>>
>> - rte_pktmbuf_free_seg(txd->mbuf);
>> + if (nb == RTE_DIM(bulk)) {
>> + rte_mempool_put_bulk(bulk[0]->pool,
>> + (void *)bulk, nb);
> same warning here, again false positive I think:
> error #3656: variable "bulk" may be used before its value is set
I think this one is false positive as well.
> The patch to ignore the warning will take care of this one too.
>
>> + nb = 0;
>> + }
>> +
>> + bulk[nb++] = txd->mbuf;
>> } while (++completed != pending);
>>
>> + rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
>> +
>> txq->completed = completed;
>> }
> <...>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap
2017-09-08 14:15 [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap Andrew Rybchenko
2017-09-08 14:15 ` [dpdk-dev] [PATCH 2/2] net/sfc: free mbufs in bulks on simple EF10 " Andrew Rybchenko
@ 2017-09-12 18:26 ` Ferruh Yigit
2017-09-13 9:31 ` Andrew Rybchenko
2017-09-12 21:51 ` Stephen Hemminger
2017-09-13 9:33 ` [dpdk-dev] [PATCH v2 " Andrew Rybchenko
3 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2017-09-12 18:26 UTC (permalink / raw)
To: Andrew Rybchenko, dev; +Cc: Ivan Malov
On 9/8/2017 3:15 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> drivers/net/sfc/sfc_ef10_tx.c | 48 ++++++++++++++++++++++++++++++++++++-------
> drivers/net/sfc/sfc_tweak.h | 3 +++
> 2 files changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c
> index 182fc23..5127a7a 100644
> --- a/drivers/net/sfc/sfc_ef10_tx.c
> +++ b/drivers/net/sfc/sfc_ef10_tx.c
> @@ -158,17 +158,35 @@ struct sfc_ef10_txq {
> pending += sfc_ef10_tx_process_events(txq);
>
> if (pending != completed) {
> + struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
> + unsigned int nb = 0;
> +
> do {
> struct sfc_ef10_tx_sw_desc *txd;
> + struct rte_mbuf *m;
>
> txd = &txq->sw_ring[completed & ptr_mask];
> + if (txd->mbuf == NULL)
> + continue;
>
> - if (txd->mbuf != NULL) {
> - rte_pktmbuf_free(txd->mbuf);
> - txd->mbuf = NULL;
> + m = rte_pktmbuf_prefree_seg(txd->mbuf);
> + txd->mbuf = NULL;
> + if (m == NULL)
> + continue;
> +
> + if ((nb == RTE_DIM(bulk)) ||
> + ((nb != 0) && (m->pool != bulk[0]->pool))) {
ICC is giving warning [1] here, as far as I can see this is false
positive but can you please double check in case I am missing something?
And unless if you don't see a way to convince icc without effecting
performance, would you mind updating patch to ignore warning [2] ?
[1]
error #3656: variable "bulk" may be used before its value is set
[2]
diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index 57aa963ba..359314809 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -65,6 +65,7 @@ CFLAGS += -Wbad-function-cast
CFLAGS_BASE_DRIVER += -Wno-empty-body
else ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
+CFLAGS_sfc_ef10_tx.o += -wd3656
endif
#
> + rte_mempool_put_bulk(bulk[0]->pool,
> + (void *)bulk, nb);
> + nb = 0;
> }
> +
> + bulk[nb++] = m;
> } while (++completed != pending);
>
> + if (nb != 0)
> + rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
> +
> txq->completed = completed;
> }
<...>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap
2017-09-12 18:26 ` [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native " Ferruh Yigit
@ 2017-09-13 9:31 ` Andrew Rybchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2017-09-13 9:31 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: Ivan Malov
On 09/12/2017 09:26 PM, Ferruh Yigit wrote:
> On 9/8/2017 3:15 PM, Andrew Rybchenko wrote:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> drivers/net/sfc/sfc_ef10_tx.c | 48 ++++++++++++++++++++++++++++++++++++-------
>> drivers/net/sfc/sfc_tweak.h | 3 +++
>> 2 files changed, 44 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c
>> index 182fc23..5127a7a 100644
>> --- a/drivers/net/sfc/sfc_ef10_tx.c
>> +++ b/drivers/net/sfc/sfc_ef10_tx.c
>> @@ -158,17 +158,35 @@ struct sfc_ef10_txq {
>> pending += sfc_ef10_tx_process_events(txq);
>>
>> if (pending != completed) {
>> + struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
>> + unsigned int nb = 0;
>> +
>> do {
>> struct sfc_ef10_tx_sw_desc *txd;
>> + struct rte_mbuf *m;
>>
>> txd = &txq->sw_ring[completed & ptr_mask];
>> + if (txd->mbuf == NULL)
>> + continue;
>>
>> - if (txd->mbuf != NULL) {
>> - rte_pktmbuf_free(txd->mbuf);
>> - txd->mbuf = NULL;
>> + m = rte_pktmbuf_prefree_seg(txd->mbuf);
>> + txd->mbuf = NULL;
>> + if (m == NULL)
>> + continue;
>> +
>> + if ((nb == RTE_DIM(bulk)) ||
>> + ((nb != 0) && (m->pool != bulk[0]->pool))) {
> ICC is giving warning [1] here, as far as I can see this is false
> positive but can you please double check in case I am missing something?
Yes, I think it is false positive.
> And unless if you don't see a way to convince icc without effecting
> performance, would you mind updating patch to ignore warning [2] ?
Thanks, done.
>
>
> [1]
> error #3656: variable "bulk" may be used before its value is set
>
> [2]
> diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
> index 57aa963ba..359314809 100644
> --- a/drivers/net/sfc/Makefile
> +++ b/drivers/net/sfc/Makefile
> @@ -65,6 +65,7 @@ CFLAGS += -Wbad-function-cast
> CFLAGS_BASE_DRIVER += -Wno-empty-body
> else ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
> CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
> +CFLAGS_sfc_ef10_tx.o += -wd3656
> endif
>
> #
<...>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap
2017-09-08 14:15 [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap Andrew Rybchenko
2017-09-08 14:15 ` [dpdk-dev] [PATCH 2/2] net/sfc: free mbufs in bulks on simple EF10 " Andrew Rybchenko
2017-09-12 18:26 ` [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native " Ferruh Yigit
@ 2017-09-12 21:51 ` Stephen Hemminger
2017-09-13 6:27 ` Andrew Rybchenko
2017-09-13 9:33 ` [dpdk-dev] [PATCH v2 " Andrew Rybchenko
3 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2017-09-12 21:51 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: dev, Ivan Malov
On Fri, 8 Sep 2017 15:15:50 +0100
Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> + txd->mbuf = NULL;
> + if (m == NULL)
> + continue;
> +
> + if ((nb == RTE_DIM(bulk)) ||
> + ((nb != 0) && (m->pool != bulk[0]->pool))) {
> + rte_mempool_put_bulk(bulk[0]->pool,
> + (void *)bulk, nb);
> + nb = 0;
> }
> +
Why not add rte_mbuf_free_bulk (inline) to base code, rather than recoding
everywhere?
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap
2017-09-12 21:51 ` Stephen Hemminger
@ 2017-09-13 6:27 ` Andrew Rybchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2017-09-13 6:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Ivan Malov
On 09/13/2017 12:51 AM, Stephen Hemminger wrote:
> On Fri, 8 Sep 2017 15:15:50 +0100
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> + txd->mbuf = NULL;
>> + if (m == NULL)
>> + continue;
>> +
>> + if ((nb == RTE_DIM(bulk)) ||
>> + ((nb != 0) && (m->pool != bulk[0]->pool))) {
>> + rte_mempool_put_bulk(bulk[0]->pool,
>> + (void *)bulk, nb);
>> + nb = 0;
>> }
>> +
> Why not add rte_mbuf_free_bulk (inline) to base code, rather than recoding
> everywhere?
I'm not 100% sure that I understand the question in a right way, but if
you're
talking about base driver code, it is not used in native datapath
implementations
at all (just header files with HW/SW interface definition). In fact
patches 1 and 2
of the series are slightly different and the difference is proved by
performance
measurements.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap
2017-09-08 14:15 [dpdk-dev] [PATCH 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap Andrew Rybchenko
` (2 preceding siblings ...)
2017-09-12 21:51 ` Stephen Hemminger
@ 2017-09-13 9:33 ` Andrew Rybchenko
2017-09-13 9:33 ` [dpdk-dev] [PATCH v2 2/2] net/sfc: free mbufs in bulks on simple EF10 " Andrew Rybchenko
2017-09-15 11:12 ` [dpdk-dev] [PATCH v2 1/2] net/sfc: free mbufs in bulks on EF10 native " Ferruh Yigit
3 siblings, 2 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2017-09-13 9:33 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit, Ivan Malov
From: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
drivers/net/sfc/Makefile | 3 +++
drivers/net/sfc/sfc_ef10_tx.c | 48 ++++++++++++++++++++++++++++++++++++-------
drivers/net/sfc/sfc_tweak.h | 3 +++
3 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index 57aa963..0adb786 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -65,6 +65,9 @@ CFLAGS += -Wbad-function-cast
CFLAGS_BASE_DRIVER += -Wno-empty-body
else ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
+# Suppress ICC false positive warning on 'bulk' may be used before its
+# value is set
+CFLAGS_sfc_ef10_tx.o += -wd3656
endif
#
diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c
index 182fc23..5127a7a 100644
--- a/drivers/net/sfc/sfc_ef10_tx.c
+++ b/drivers/net/sfc/sfc_ef10_tx.c
@@ -158,17 +158,35 @@ struct sfc_ef10_txq {
pending += sfc_ef10_tx_process_events(txq);
if (pending != completed) {
+ struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
+ unsigned int nb = 0;
+
do {
struct sfc_ef10_tx_sw_desc *txd;
+ struct rte_mbuf *m;
txd = &txq->sw_ring[completed & ptr_mask];
+ if (txd->mbuf == NULL)
+ continue;
- if (txd->mbuf != NULL) {
- rte_pktmbuf_free(txd->mbuf);
- txd->mbuf = NULL;
+ m = rte_pktmbuf_prefree_seg(txd->mbuf);
+ txd->mbuf = NULL;
+ if (m == NULL)
+ continue;
+
+ if ((nb == RTE_DIM(bulk)) ||
+ ((nb != 0) && (m->pool != bulk[0]->pool))) {
+ rte_mempool_put_bulk(bulk[0]->pool,
+ (void *)bulk, nb);
+ nb = 0;
}
+
+ bulk[nb++] = m;
} while (++completed != pending);
+ if (nb != 0)
+ rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
+
txq->completed = completed;
}
@@ -325,6 +343,7 @@ struct sfc_ef10_txq {
do {
phys_addr_t seg_addr = rte_mbuf_data_dma_addr(m_seg);
unsigned int seg_len = rte_pktmbuf_data_len(m_seg);
+ unsigned int id = added & ptr_mask;
SFC_ASSERT(seg_len <= SFC_EF10_TX_DMA_DESC_LEN_MAX);
@@ -332,15 +351,30 @@ struct sfc_ef10_txq {
sfc_ef10_tx_qdesc_dma_create(seg_addr,
seg_len, (pkt_len == 0),
- &txq->txq_hw_ring[added & ptr_mask]);
+ &txq->txq_hw_ring[id]);
+
+ /*
+ * rte_pktmbuf_free() is commonly used in DPDK for
+ * recycling packets - the function checks every
+ * segment's reference counter and returns the
+ * buffer to its pool whenever possible;
+ * nevertheless, freeing mbuf segments one by one
+ * may entail some performance decline;
+ * from this point, sfc_efx_tx_reap() does the same job
+ * on its own and frees buffers in bulks (all mbufs
+ * within a bulk belong to the same pool);
+ * from this perspective, individual segment pointers
+ * must be associated with the corresponding SW
+ * descriptors independently so that only one loop
+ * is sufficient on reap to inspect all the buffers
+ */
+ txq->sw_ring[id].mbuf = m_seg;
+
++added;
} while ((m_seg = m_seg->next) != 0);
dma_desc_space -= (added - pkt_start);
-
- /* Assign mbuf to the last used desc */
- txq->sw_ring[(added - 1) & ptr_mask].mbuf = *pktp;
}
if (likely(added != txq->added)) {
diff --git a/drivers/net/sfc/sfc_tweak.h b/drivers/net/sfc/sfc_tweak.h
index 4ef7fc8..fd2f75c 100644
--- a/drivers/net/sfc/sfc_tweak.h
+++ b/drivers/net/sfc/sfc_tweak.h
@@ -53,4 +53,7 @@
/** Default free threshold follows recommendations from DPDK documentation */
#define SFC_TX_DEFAULT_FREE_THRESH 32
+/** Number of mbufs to be freed in bulk in a single call */
+#define SFC_TX_REAP_BULK_SIZE 32
+
#endif /* _SFC_TWEAK_H_ */
--
1.8.2.3
^ permalink raw reply [flat|nested] 11+ messages in thread* [dpdk-dev] [PATCH v2 2/2] net/sfc: free mbufs in bulks on simple EF10 Tx datapath reap
2017-09-13 9:33 ` [dpdk-dev] [PATCH v2 " Andrew Rybchenko
@ 2017-09-13 9:33 ` Andrew Rybchenko
2017-09-15 11:12 ` [dpdk-dev] [PATCH v2 1/2] net/sfc: free mbufs in bulks on EF10 native " Ferruh Yigit
1 sibling, 0 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2017-09-13 9:33 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit, Ivan Malov
From: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
doc/guides/nics/sfc_efx.rst | 6 +++++-
drivers/net/sfc/sfc_dp_tx.h | 2 ++
drivers/net/sfc/sfc_ef10_tx.c | 15 ++++++++++++++-
drivers/net/sfc/sfc_ethdev.c | 6 ++++++
drivers/net/sfc/sfc_tx.c | 17 +++++++++++++++++
5 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 973a4a0..ae2b54a 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -245,12 +245,16 @@ boolean parameters value.
features available and required by the datapath implementation.
**efx** chooses libefx-based datapath which supports VLAN insertion
(full-feature firmware variant only), TSO and multi-segment mbufs.
+ Mbuf segments may come from different mempools, and mbuf reference
+ counters are treated responsibly.
**ef10** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which is
more efficient than libefx-based but has no VLAN insertion and TSO
support yet.
+ Mbuf segments may come from different mempools, and mbuf reference
+ counters are treated responsibly.
**ef10_simple** chooses EF10 (SFN7xxx, SFN8xxx) native datapath which
is even more faster then **ef10** but does not support multi-segment
- mbufs.
+ mbufs, disallows multiple mempools and neglects mbuf reference counters.
- ``perf_profile`` [auto|throughput|low-latency] (default **throughput**)
diff --git a/drivers/net/sfc/sfc_dp_tx.h b/drivers/net/sfc/sfc_dp_tx.h
index db2a70b..94d1b10 100644
--- a/drivers/net/sfc/sfc_dp_tx.h
+++ b/drivers/net/sfc/sfc_dp_tx.h
@@ -142,6 +142,8 @@ struct sfc_dp_tx {
#define SFC_DP_TX_FEAT_TSO 0x2
#define SFC_DP_TX_FEAT_MULTI_SEG 0x4
#define SFC_DP_TX_FEAT_MULTI_PROCESS 0x8
+#define SFC_DP_TX_FEAT_MULTI_POOL 0x10
+#define SFC_DP_TX_FEAT_REFCNT 0x20
sfc_dp_tx_qcreate_t *qcreate;
sfc_dp_tx_qdestroy_t *qdestroy;
sfc_dp_tx_qstart_t *qstart;
diff --git a/drivers/net/sfc/sfc_ef10_tx.c b/drivers/net/sfc/sfc_ef10_tx.c
index 5127a7a..9047b3e 100644
--- a/drivers/net/sfc/sfc_ef10_tx.c
+++ b/drivers/net/sfc/sfc_ef10_tx.c
@@ -401,14 +401,25 @@ struct sfc_ef10_txq {
pending += sfc_ef10_tx_process_events(txq);
if (pending != completed) {
+ struct rte_mbuf *bulk[SFC_TX_REAP_BULK_SIZE];
+ unsigned int nb = 0;
+
do {
struct sfc_ef10_tx_sw_desc *txd;
txd = &txq->sw_ring[completed & ptr_mask];
- rte_pktmbuf_free_seg(txd->mbuf);
+ if (nb == RTE_DIM(bulk)) {
+ rte_mempool_put_bulk(bulk[0]->pool,
+ (void *)bulk, nb);
+ nb = 0;
+ }
+
+ bulk[nb++] = txd->mbuf;
} while (++completed != pending);
+ rte_mempool_put_bulk(bulk[0]->pool, (void *)bulk, nb);
+
txq->completed = completed;
}
@@ -614,6 +625,8 @@ struct sfc_dp_tx sfc_ef10_tx = {
.hw_fw_caps = SFC_DP_HW_FW_CAP_EF10,
},
.features = SFC_DP_TX_FEAT_MULTI_SEG |
+ SFC_DP_TX_FEAT_MULTI_POOL |
+ SFC_DP_TX_FEAT_REFCNT |
SFC_DP_TX_FEAT_MULTI_PROCESS,
.qcreate = sfc_ef10_tx_qcreate,
.qdestroy = sfc_ef10_tx_qdestroy,
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 08299fe..9e65b6a 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -145,6 +145,12 @@
if (~sa->dp_tx->features & SFC_DP_TX_FEAT_MULTI_SEG)
dev_info->default_txconf.txq_flags |= ETH_TXQ_FLAGS_NOMULTSEGS;
+ if (~sa->dp_tx->features & SFC_DP_TX_FEAT_MULTI_POOL)
+ dev_info->default_txconf.txq_flags |= ETH_TXQ_FLAGS_NOMULTMEMP;
+
+ if (~sa->dp_tx->features & SFC_DP_TX_FEAT_REFCNT)
+ dev_info->default_txconf.txq_flags |= ETH_TXQ_FLAGS_NOREFCOUNT;
+
#if EFSYS_OPT_RX_SCALE
if (sa->rss_support != EFX_RX_SCALE_UNAVAILABLE) {
dev_info->reta_size = EFX_RSS_TBL_SIZE;
diff --git a/drivers/net/sfc/sfc_tx.c b/drivers/net/sfc/sfc_tx.c
index bf59601..4ea7bd7 100644
--- a/drivers/net/sfc/sfc_tx.c
+++ b/drivers/net/sfc/sfc_tx.c
@@ -91,6 +91,21 @@
rc = EINVAL;
}
+ if (((flags & ETH_TXQ_FLAGS_NOMULTMEMP) == 0) &&
+ (~sa->dp_tx->features & SFC_DP_TX_FEAT_MULTI_POOL)) {
+ sfc_err(sa, "multi-mempool is not supported by %s datapath",
+ sa->dp_tx->dp.name);
+ rc = EINVAL;
+ }
+
+ if (((flags & ETH_TXQ_FLAGS_NOREFCOUNT) == 0) &&
+ (~sa->dp_tx->features & SFC_DP_TX_FEAT_REFCNT)) {
+ sfc_err(sa,
+ "mbuf reference counters are neglected by %s datapath",
+ sa->dp_tx->dp.name);
+ rc = EINVAL;
+ }
+
if ((flags & ETH_TXQ_FLAGS_NOVLANOFFL) == 0) {
if (!encp->enc_hw_tx_insert_vlan_enabled) {
sfc_err(sa, "VLAN offload is not supported");
@@ -1023,6 +1038,8 @@ struct sfc_dp_tx sfc_efx_tx = {
},
.features = SFC_DP_TX_FEAT_VLAN_INSERT |
SFC_DP_TX_FEAT_TSO |
+ SFC_DP_TX_FEAT_MULTI_POOL |
+ SFC_DP_TX_FEAT_REFCNT |
SFC_DP_TX_FEAT_MULTI_SEG,
.qcreate = sfc_efx_tx_qcreate,
.qdestroy = sfc_efx_tx_qdestroy,
--
1.8.2.3
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH v2 1/2] net/sfc: free mbufs in bulks on EF10 native Tx datapath reap
2017-09-13 9:33 ` [dpdk-dev] [PATCH v2 " Andrew Rybchenko
2017-09-13 9:33 ` [dpdk-dev] [PATCH v2 2/2] net/sfc: free mbufs in bulks on simple EF10 " Andrew Rybchenko
@ 2017-09-15 11:12 ` Ferruh Yigit
1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2017-09-15 11:12 UTC (permalink / raw)
To: Andrew Rybchenko, dev; +Cc: Ivan Malov
On 9/13/2017 10:33 AM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Series applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread