* [dpdk-dev] [PATCH] net/sfc: enable TSO by default
@ 2017-01-19 15:52 Andrew Rybchenko
2017-01-20 14:29 ` Thomas Monjalon
2017-01-20 15:22 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Rybchenko @ 2017-01-19 15:52 UTC (permalink / raw)
To: dev
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
It is a mistake that TSO support is compiled out by default.
We would be happy to enable it by deafult, but strictly speaking
it is not a bug fix.
Arguments to enable are:
- be more feature-rich (and user-friendly) in default config
- the most of internal testing is done with TSO enabled
config/common_base | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config/common_base b/config/common_base
index b9fb8e2..134e9b9 100644
--- a/config/common_base
+++ b/config/common_base
@@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
#
CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
-CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
+CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y
#
# Compile software PMD backed by SZEDATA2 device
--
1.8.2.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/sfc: enable TSO by default
2017-01-19 15:52 [dpdk-dev] [PATCH] net/sfc: enable TSO by default Andrew Rybchenko
@ 2017-01-20 14:29 ` Thomas Monjalon
2017-01-20 14:37 ` Andrew Rybchenko
2017-01-20 15:22 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2017-01-20 14:29 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: dev, ferruh.yigit
2017-01-19 15:52, Andrew Rybchenko:
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> It is a mistake that TSO support is compiled out by default.
> We would be happy to enable it by deafult, but strictly speaking
> it is not a bug fix.
>
> Arguments to enable are:
> - be more feature-rich (and user-friendly) in default config
> - the most of internal testing is done with TSO enabled
>
> config/common_base | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/config/common_base b/config/common_base
> index b9fb8e2..134e9b9 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
> #
> CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
> CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
> -CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
> +CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y
There should not be such option in the build-time configuration.
Why keeping it (even enabled by default)?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/sfc: enable TSO by default
2017-01-20 14:29 ` Thomas Monjalon
@ 2017-01-20 14:37 ` Andrew Rybchenko
2017-01-20 14:47 ` Ferruh Yigit
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Rybchenko @ 2017-01-20 14:37 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, ferruh.yigit
On 01/20/2017 05:29 PM, Thomas Monjalon wrote:
> 2017-01-19 15:52, Andrew Rybchenko:
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> It is a mistake that TSO support is compiled out by default.
>> We would be happy to enable it by deafult, but strictly speaking
>> it is not a bug fix.
>>
>> Arguments to enable are:
>> - be more feature-rich (and user-friendly) in default config
>> - the most of internal testing is done with TSO enabled
>>
>> config/common_base | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/config/common_base b/config/common_base
>> index b9fb8e2..134e9b9 100644
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
>> #
>> CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
>> CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
>> -CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
>> +CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y
> There should not be such option in the build-time configuration.
> Why keeping it (even enabled by default)?
Initially it was introduced since packet rates with TSO compiled out
were slightly but noticeable better. Finally we have changed approach
and performance improvements planned in 17.05 do not require it.
I'll be happy to remove it completely, if there is no problems to
include it in 17.02 (it will be a bit bigger but functionally equal to
this one).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/sfc: enable TSO by default
2017-01-20 14:37 ` Andrew Rybchenko
@ 2017-01-20 14:47 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2017-01-20 14:47 UTC (permalink / raw)
To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev
On 1/20/2017 2:37 PM, Andrew Rybchenko wrote:
> On 01/20/2017 05:29 PM, Thomas Monjalon wrote:
>> 2017-01-19 15:52, Andrew Rybchenko:
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>> It is a mistake that TSO support is compiled out by default.
>>> We would be happy to enable it by deafult, but strictly speaking
>>> it is not a bug fix.
>>>
>>> Arguments to enable are:
>>> - be more feature-rich (and user-friendly) in default config
>>> - the most of internal testing is done with TSO enabled
>>>
>>> config/common_base | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/config/common_base b/config/common_base
>>> index b9fb8e2..134e9b9 100644
>>> --- a/config/common_base
>>> +++ b/config/common_base
>>> @@ -261,7 +261,7 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
>>> #
>>> CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
>>> CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
>>> -CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
>>> +CONFIG_RTE_LIBRTE_SFC_EFX_TSO=y
>> There should not be such option in the build-time configuration.
>> Why keeping it (even enabled by default)?
>
> Initially it was introduced since packet rates with TSO compiled out
> were slightly but noticeable better. Finally we have changed approach
> and performance improvements planned in 17.05 do not require it.
> I'll be happy to remove it completely, if there is no problems to
> include it in 17.02 (it will be a bit bigger but functionally equal to
> this one).
If it will be just removing ifdef, with keeping same functionality, I
wouldn't mind bigger patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v3] net/sfc: enable TSO by default
2017-01-19 15:52 [dpdk-dev] [PATCH] net/sfc: enable TSO by default Andrew Rybchenko
2017-01-20 14:29 ` Thomas Monjalon
@ 2017-01-20 15:22 ` Andrew Rybchenko
2017-01-23 9:56 ` Ferruh Yigit
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Rybchenko @ 2017-01-20 15:22 UTC (permalink / raw)
To: dev; +Cc: Thomas Monjalon, Ferruh Yigit
Remove RTE_LIBRTE_SFC_EFX_TSO config option since it is not
required any more:
- unreasonable limit on number of Tx queues when TSO is not
actually required should be solved using per-device parameter
- performance difference with and without TSO compiled in is small
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
config/common_base | 1 -
doc/guides/nics/sfc_efx.rst | 6 ------
drivers/net/sfc/Makefile | 6 ++----
drivers/net/sfc/sfc.c | 4 ----
drivers/net/sfc/sfc_tx.c | 2 --
drivers/net/sfc/sfc_tx.h | 18 ------------------
6 files changed, 2 insertions(+), 35 deletions(-)
diff --git a/config/common_base b/config/common_base
index b9fb8e2..c33179b 100644
--- a/config/common_base
+++ b/config/common_base
@@ -261,7 +261,6 @@ CONFIG_RTE_LIBRTE_BNXT_PMD=y
#
CONFIG_RTE_LIBRTE_SFC_EFX_PMD=y
CONFIG_RTE_LIBRTE_SFC_EFX_DEBUG=n
-CONFIG_RTE_LIBRTE_SFC_EFX_TSO=n
#
# Compile software PMD backed by SZEDATA2 device
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 6be4fba..0a05a0a 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -171,12 +171,6 @@ Please note that enabling debugging options may affect system performance.
Enable compilation of the extra run-time consistency checks.
-- ``CONFIG_RTE_LIBRTE_SFC_EFX_TSO`` (default **n**)
-
- Toggle TCP segmentation offload support.
- Enabling the feature limits the number of available transmit queues
- significantly due to the limited number of adapter TSO contexts.
-
Per-Device Parameters
~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index 14d6536..619a0ed 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -89,8 +89,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_ev.c
SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_port.c
SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_rx.c
SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_tx.c
-
-SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_TSO) += sfc_tso.c
+SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_tso.c
VPATH += $(SRCDIR)/base
@@ -140,7 +139,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_kvargs
DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_ether
DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_mempool
DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_mbuf
-
-DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_TSO) += lib/librte_net
+DEPDIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += lib/librte_net
include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 648ad8c..457a53e 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -621,13 +621,9 @@
if (rc != 0)
goto fail_set_rss_defaults;
-#ifdef RTE_LIBRTE_SFC_EFX_TSO
sa->tso = efx_nic_cfg_get(sa->nic)->enc_fw_assisted_tso_v2_enabled;
if (!sa->tso)
sfc_warn(sa, "TSO support isn't available on this adapter");
-#else /* !RTE_LIBRTE_SFC_EFX_TSO */
- sa->tso = B_FALSE;
-#endif /* RTE_LIBRTE_SFC_EFX_TSO */
sfc_log_init(sa, "fini nic");
efx_nic_fini(enp);
diff --git a/drivers/net/sfc/sfc_tx.c b/drivers/net/sfc/sfc_tx.c
index 3e64c0f..e772584 100644
--- a/drivers/net/sfc/sfc_tx.c
+++ b/drivers/net/sfc/sfc_tx.c
@@ -650,7 +650,6 @@
*/
pkt_descs += sfc_tx_maybe_insert_tag(txq, m_seg, &pend);
-#ifdef RTE_LIBRTE_SFC_EFX_TSO
if (m_seg->ol_flags & PKT_TX_TCP_SEG) {
/*
* We expect correct 'pkt->l[2, 3, 4]_len' values
@@ -688,7 +687,6 @@
* as for the usual non-TSO path
*/
}
-#endif /* RTE_LIBRTE_SFC_EFX_TSO */
for (; m_seg != NULL; m_seg = m_seg->next) {
efsys_dma_addr_t next_frag;
diff --git a/drivers/net/sfc/sfc_tx.h b/drivers/net/sfc/sfc_tx.h
index 581e2aa..39977a5 100644
--- a/drivers/net/sfc/sfc_tx.h
+++ b/drivers/net/sfc/sfc_tx.h
@@ -50,9 +50,7 @@
struct sfc_tx_sw_desc {
struct rte_mbuf *mbuf;
-#ifdef RTE_LIBRTE_SFC_EFX_TSO
uint8_t *tsoh; /* Buffer to store TSO header */
-#endif /* RTE_LIBRTE_SFC_EFX_TSO */
};
enum sfc_txq_state_bit {
@@ -116,7 +114,6 @@ int sfc_tx_qinit(struct sfc_adapter *sa, unsigned int sw_index,
uint16_t sfc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
-#ifdef RTE_LIBRTE_SFC_EFX_TSO
/* From 'sfc_tso.c' */
int sfc_tso_alloc_tsoh_objs(struct sfc_tx_sw_desc *sw_ring,
unsigned int txq_entries, unsigned int socket_id);
@@ -125,21 +122,6 @@ void sfc_tso_free_tsoh_objs(struct sfc_tx_sw_desc *sw_ring,
int sfc_tso_do(struct sfc_txq *txq, unsigned int idx, struct rte_mbuf **in_seg,
size_t *in_off, efx_desc_t **pend, unsigned int *pkt_descs,
size_t *pkt_len);
-#else /* !RTE_LIBRTE_SFC_EFX_TSO */
-static inline int
-sfc_tso_alloc_tsoh_objs(__rte_unused struct sfc_tx_sw_desc *sw_ring,
- __rte_unused unsigned int txq_entries,
- __rte_unused unsigned int socket_id)
-{
- return 0;
-}
-
-static inline void
-sfc_tso_free_tsoh_objs(__rte_unused struct sfc_tx_sw_desc *sw_ring,
- __rte_unused unsigned int txq_entries)
-{
-}
-#endif /* RTE_LIBRTE_SFC_EFX_TSO */
#ifdef __cplusplus
}
--
1.8.2.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/sfc: enable TSO by default
2017-01-20 15:22 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
@ 2017-01-23 9:56 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2017-01-23 9:56 UTC (permalink / raw)
To: Andrew Rybchenko, dev; +Cc: Thomas Monjalon
On 1/20/2017 3:22 PM, Andrew Rybchenko wrote:
> Remove RTE_LIBRTE_SFC_EFX_TSO config option since it is not
> required any more:
> - unreasonable limit on number of Tx queues when TSO is not
> actually required should be solved using per-device parameter
> - performance difference with and without TSO compiled in is small
>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-23 9:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19 15:52 [dpdk-dev] [PATCH] net/sfc: enable TSO by default Andrew Rybchenko
2017-01-20 14:29 ` Thomas Monjalon
2017-01-20 14:37 ` Andrew Rybchenko
2017-01-20 14:47 ` Ferruh Yigit
2017-01-20 15:22 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
2017-01-23 9:56 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).