diff mbox series

[net] net: fec: change the default rx copybreak length to 1518

Message ID 20220819090041.1541422-1-wei.fang@nxp.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net] net: fec: change the default rx copybreak length to 1518 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 85 this patch: 85
netdev/cc_maintainers warning 1 maintainers not CCed: qiangqing.zhang@nxp.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 85 this patch: 85
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Wei Fang Aug. 19, 2022, 9 a.m. UTC
From: Wei Fang <wei.fang@nxp.com>

Set the default rx copybreak value to 1518 so that improve the
performance when SMMU is enabled. User can change the copybreak
length in dynamically by ethtool.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Aug. 19, 2022, 3:57 p.m. UTC | #1
On Fri, Aug 19, 2022 at 05:00:41PM +0800, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> Set the default rx copybreak value to 1518 so that improve the
> performance when SMMU is enabled. User can change the copybreak
> length in dynamically by ethtool.

Please provide some benchmark for this. And include a range of SoCs
which include the FEC. Maybe this helps for the platform you are
testing on, but is bad for imx25, Vybrid etc?

> + * The driver support .set_tunable() interface for ethtool, user
> + * can dynamicly change the copybreak value.
> + */

Which also means you could change it for your platform. So a patch
like this needs justifying.

     Andrew
Jakub Kicinski Aug. 19, 2022, 10:04 p.m. UTC | #2
On Fri, 19 Aug 2022 17:57:07 +0200 Andrew Lunn wrote:
> On Fri, Aug 19, 2022 at 05:00:41PM +0800, wei.fang@nxp.com wrote:
> > Set the default rx copybreak value to 1518 so that improve the
> > performance when SMMU is enabled. User can change the copybreak
> > length in dynamically by ethtool.  
> 
> Please provide some benchmark for this. And include a range of SoCs
> which include the FEC. Maybe this helps for the platform you are
> testing on, but is bad for imx25, Vybrid etc?
> 
> > + * The driver support .set_tunable() interface for ethtool, user
> > + * can dynamicly change the copybreak value.
> > + */  
> 
> Which also means you could change it for your platform. So a patch
> like this needs justifying.

Fully agreed, perhaps if the DMA mapping on the platform is extremely
slow we're better off making the platform use bounce buffers for
everything.

Another though is that you can try to manually sync only the parts 
of the buffers that the device actually touched instead of full 2kB,
and tell the DMA core to skip the sync of the full buffer.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e8e2aa1e7f01..f5f34cdba131 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -299,7 +299,15 @@  MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #define FEC_WOL_FLAG_ENABLE		(0x1 << 1)
 #define FEC_WOL_FLAG_SLEEP_ON		(0x1 << 2)
 
-#define COPYBREAK_DEFAULT	256
+/* By default, set the copybreak to 1518,
+ * then the RX path always keep DMA memory unchanged, and
+ * allocate one new skb and copy DMA memory data to the new skb
+ * buffer, which can improve the performance when SMMU is enabled.
+ *
+ * The driver support .set_tunable() interface for ethtool, user
+ * can dynamicly change the copybreak value.
+ */
+#define COPYBREAK_DEFAULT	1518
 
 /* Max number of allowed TCP segments for software TSO */
 #define FEC_MAX_TSO_SEGS	100