diff mbox series

[net-next] docs: net: dsa: describe issues with checksum offload

Message ID 20220309180148.13286-1-luizluca@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] docs: net: dsa: describe issues with checksum offload | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
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: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
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/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 18 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Luiz Angelo Daros de Luca March 9, 2022, 6:01 p.m. UTC
DSA tags before IP header (categories 1 and 2) or after the payload (3)
might introduce offload checksum issues.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Reviewed-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 Documentation/networking/dsa/dsa.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Vladimir Oltean March 9, 2022, 11:48 p.m. UTC | #1
On Wed, Mar 09, 2022 at 03:01:49PM -0300, Luiz Angelo Daros de Luca wrote:
> DSA tags before IP header (categories 1 and 2) or after the payload (3)
> might introduce offload checksum issues.
> 
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Reviewed-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 89bb4fa4c362..c8885e60eac5 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -193,6 +193,18 @@ protocol. If not all packets are of equal size, the tagger can implement the
>  default behavior by specifying the correct offset incurred by each individual
>  RX packet. Tail taggers do not cause issues to the flow dissector.
>  
> +Tagging protocols might also break checksum offload. The offload hardware must
> +either be able to parse the proprietary tag, usually when it matches the switch
> +vendor, or, in the case of category 1 and 2, the driver and the hardware must
> +be able to use the sum_start/csum_offset adjusted by the DSA framework. Drivers

s/sum_start/csum_start/

Also, I wouldn't necessarily put things in this light. I wouldn't start
off by saying what's broken, but rather what works. I would say that
category 1 and 2 taggers are expected to work with a DSA master that
declares NETIF_F_HW_CSUM and looks at csum_start/csum_offset, since DSA
adjusts that correctly. For DSA masters which declare the legacy
NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM in vlan_features, DSA inherits this
flag assuming that the master either has knowledge of the tagging
protocol it is paired with (perhaps due to matching vendors), or falls
back to software checksumming for packets where the IP header isn't at
the offset expected by the hardware (which is also the case for IP
tunneling protocols).

> +that enable the checksum offload based only on the network/transport headers
> +might wrongly delegate the checksum to incompatible hardware, sending packets
> +with an uncalculated checksum to the network. For category 3, when the offload
> +hardware cannot parse the proprietary tag, the checksum must be calculated

To be consistent with the rest of the document:
s/proprietary tag/switch tag/

> +before any tag is inserted because both software and hardware checksums will
> +include any trailing tag as part of the payload. When the switch strips the tag,
> +the packet sent to the network will not match the checksum.

I would say: "Otherwise, the DSA master would include the tail tag in
the checksum calculation as well, but this gets stripped by the switch
during transmission which leaves an incorrect IP checksum in place".

> +
>  Due to various reasons (most common being category 1 taggers being associated
>  with DSA-unaware masters, mangling what the master perceives as MAC DA), the
>  tagging protocol may require the DSA master to operate in promiscuous mode, to
> -- 
> 2.35.1
>
Luiz Angelo Daros de Luca April 13, 2022, 6:40 p.m. UTC | #2
Vladimir,

I sent a v2 with your suggestions.

Regards,

Luiz
Vladimir Oltean April 13, 2022, 8:15 p.m. UTC | #3
On Wed, Apr 13, 2022 at 03:40:16PM -0300, Luiz Angelo Daros de Luca wrote:
> Vladimir,
> 
> I sent a v2 with your suggestions.
> 
> Regards,
> 
> Luiz

Sorry, I had put it in the folder with read emails but didn't actually
respond to it.
diff mbox series

Patch

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 89bb4fa4c362..c8885e60eac5 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -193,6 +193,18 @@  protocol. If not all packets are of equal size, the tagger can implement the
 default behavior by specifying the correct offset incurred by each individual
 RX packet. Tail taggers do not cause issues to the flow dissector.
 
+Tagging protocols might also break checksum offload. The offload hardware must
+either be able to parse the proprietary tag, usually when it matches the switch
+vendor, or, in the case of category 1 and 2, the driver and the hardware must
+be able to use the sum_start/csum_offset adjusted by the DSA framework. Drivers
+that enable the checksum offload based only on the network/transport headers
+might wrongly delegate the checksum to incompatible hardware, sending packets
+with an uncalculated checksum to the network. For category 3, when the offload
+hardware cannot parse the proprietary tag, the checksum must be calculated
+before any tag is inserted because both software and hardware checksums will
+include any trailing tag as part of the payload. When the switch strips the tag,
+the packet sent to the network will not match the checksum.
+
 Due to various reasons (most common being category 1 taggers being associated
 with DSA-unaware masters, mangling what the master perceives as MAC DA), the
 tagging protocol may require the DSA master to operate in promiscuous mode, to