* [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).