diff mbox series

[net-next,v2] net: stmmac: Programming sequence for VLAN packets with split header

Message ID 20241016234313.3992214-1-quic_abchauha@quicinc.com (mailing list archive)
State Accepted
Commit d10f1a4e44c3bf874701f86f8cc43490e1956acf
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v2] net: stmmac: Programming sequence for VLAN packets with split header | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
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: 6 this patch: 6
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 28 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-10-19--00-00 (tests: 777)

Commit Message

Abhishek Chauhan (ABC) Oct. 16, 2024, 11:43 p.m. UTC
Currently reset state configuration of split header works fine for
non-tagged packets and we see no corruption in payload of any size

We need additional programming sequence with reset configuration to
handle VLAN tagged packets to avoid corruption in payload for packets
of size greater than 256 bytes.

Without this change ping application complains about corruption
in payload when the size of the VLAN packet exceeds 256 bytes.

With this change tagged and non-tagged packets of any size works fine
and there is no corruption seen.

Current configuration which has the issue for VLAN packet
----------------------------------------------------------

Split happens at the position at Layer 3 header
|MAC-DA|MAC-SA|Vlan Tag|Ether type|IP header|IP data|Rest of the payload|
                         2 bytes            ^
                                            |

With the fix we are making sure that the split happens now at
Layer 2 which is end of ethernet header and start of IP payload

Ip traffic split
-----------------

Bits which take care of this are SPLM and SPLOFST
SPLM = Split mode is set to Layer 2
SPLOFST = These bits indicate the value of offset from the beginning
of Length/Type field at which header split should take place when the
appropriate SPLM is selected. Reset value is 2bytes.

Un-tagged data (without VLAN)
|MAC-DA|MAC-SA|Ether type|IP header|IP data|Rest of the payload|
                  2bytes ^
			 |

Tagged data (with VLAN)
|MAC-DA|MAC-SA|VLAN Tag|Ether type|IP header|IP data|Rest of the payload|
                          2bytes  ^
				  |

Non-IP traffic split such AV packet
------------------------------------

Bits which take care of this are
SAVE = Split AV Enable
SAVO = Split AV Offset, similar to SPLOFST but this is for AVTP
packets.

|Preamble|MAC-DA|MAC-SA|VLAN tag|Ether type|IEEE 1722 payload|CRC|
				    2bytes ^
					   |

Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
---
Changes since v1
- took care of comments from Simon on FIELD_PREP
- explained the details of l2 and l3 split as requested by Andrew
- Added folks from intel and Nvidia who disabled split header
  need to check if they faced similar issues and if this fix  
  can help them too. 

Changes since v0
- The reason for posting it on net-next is to enable this new feature.



 drivers/net/ethernet/stmicro/stmmac/dwmac4.h     | 5 +++++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 5 +++++
 2 files changed, 10 insertions(+)

Comments

Simon Horman Oct. 17, 2024, 2:25 p.m. UTC | #1
On Wed, Oct 16, 2024 at 04:43:13PM -0700, Abhishek Chauhan wrote:
> Currently reset state configuration of split header works fine for
> non-tagged packets and we see no corruption in payload of any size
> 
> We need additional programming sequence with reset configuration to
> handle VLAN tagged packets to avoid corruption in payload for packets
> of size greater than 256 bytes.
> 
> Without this change ping application complains about corruption
> in payload when the size of the VLAN packet exceeds 256 bytes.
> 
> With this change tagged and non-tagged packets of any size works fine
> and there is no corruption seen.
> 
> Current configuration which has the issue for VLAN packet
> ----------------------------------------------------------
> 
> Split happens at the position at Layer 3 header
> |MAC-DA|MAC-SA|Vlan Tag|Ether type|IP header|IP data|Rest of the payload|
>                          2 bytes            ^
>                                             |
> 
> With the fix we are making sure that the split happens now at
> Layer 2 which is end of ethernet header and start of IP payload
> 
> Ip traffic split
> -----------------
> 
> Bits which take care of this are SPLM and SPLOFST
> SPLM = Split mode is set to Layer 2
> SPLOFST = These bits indicate the value of offset from the beginning
> of Length/Type field at which header split should take place when the
> appropriate SPLM is selected. Reset value is 2bytes.
> 
> Un-tagged data (without VLAN)
> |MAC-DA|MAC-SA|Ether type|IP header|IP data|Rest of the payload|
>                   2bytes ^
> 			 |
> 
> Tagged data (with VLAN)
> |MAC-DA|MAC-SA|VLAN Tag|Ether type|IP header|IP data|Rest of the payload|
>                           2bytes  ^
> 				  |
> 
> Non-IP traffic split such AV packet
> ------------------------------------
> 
> Bits which take care of this are
> SAVE = Split AV Enable
> SAVO = Split AV Offset, similar to SPLOFST but this is for AVTP
> packets.
> 
> |Preamble|MAC-DA|MAC-SA|VLAN tag|Ether type|IEEE 1722 payload|CRC|
> 				    2bytes ^
> 					   |
> 
> Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
> ---
> Changes since v1
> - took care of comments from Simon on FIELD_PREP
> - explained the details of l2 and l3 split as requested by Andrew
> - Added folks from intel and Nvidia who disabled split header
>   need to check if they faced similar issues and if this fix  
>   can help them too. 
> 
> Changes since v0
> - The reason for posting it on net-next is to enable this new feature.

Reviewed-by: Simon Horman <horms@kernel.org>
patchwork-bot+netdevbpf@kernel.org Oct. 22, 2024, 11 a.m. UTC | #2
Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 16 Oct 2024 16:43:13 -0700 you wrote:
> Currently reset state configuration of split header works fine for
> non-tagged packets and we see no corruption in payload of any size
> 
> We need additional programming sequence with reset configuration to
> handle VLAN tagged packets to avoid corruption in payload for packets
> of size greater than 256 bytes.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: stmmac: Programming sequence for VLAN packets with split header
    https://git.kernel.org/netdev/net-next/c/d10f1a4e44c3

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 93a78fd0737b..28fff6cab812 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -44,6 +44,7 @@ 
 #define GMAC_MDIO_DATA			0x00000204
 #define GMAC_GPIO_STATUS		0x0000020C
 #define GMAC_ARP_ADDR			0x00000210
+#define GMAC_EXT_CFG1			0x00000238
 #define GMAC_ADDR_HIGH(reg)		(0x300 + reg * 8)
 #define GMAC_ADDR_LOW(reg)		(0x304 + reg * 8)
 #define GMAC_L3L4_CTRL(reg)		(0x900 + (reg) * 0x30)
@@ -284,6 +285,10 @@  enum power_event {
 #define GMAC_HW_FEAT_DVLAN		BIT(5)
 #define GMAC_HW_FEAT_NRVF		GENMASK(2, 0)
 
+/* MAC extended config 1 */
+#define GMAC_CONFIG1_SAVE_EN		BIT(24)
+#define GMAC_CONFIG1_SPLM(v)		FIELD_PREP(GENMASK(9, 8), v)
+
 /* GMAC GPIO Status reg */
 #define GMAC_GPO0			BIT(16)
 #define GMAC_GPO1			BIT(17)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index e0165358c4ac..7c895e0ae71f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -526,6 +526,11 @@  static void dwmac4_enable_sph(struct stmmac_priv *priv, void __iomem *ioaddr,
 	value |= GMAC_CONFIG_HDSMS_256; /* Segment max 256 bytes */
 	writel(value, ioaddr + GMAC_EXT_CONFIG);
 
+	value = readl(ioaddr + GMAC_EXT_CFG1);
+	value |= GMAC_CONFIG1_SPLM(1); /* Split mode set to L2OFST */
+	value |= GMAC_CONFIG1_SAVE_EN; /* Enable Split AV mode */
+	writel(value, ioaddr + GMAC_EXT_CFG1);
+
 	value = readl(ioaddr + DMA_CHAN_CONTROL(dwmac4_addrs, chan));
 	if (en)
 		value |= DMA_CONTROL_SPH;