[PATCH net-next v2] netdev: clarify GSO vs csum in qstats

[PATCH net-next v2] netdev: clarify GSO vs csum in qstats

[Jakub Kicinski]

Could be just me, but I had to pause and double check that the Tx csum
counter in qstat should include GSO'd packets. GSO pretty much implies
csum so one could possibly interpret the csum counter as pure csum offload.

But the counters are based on virtio:

  [tx_needs_csum]
      The number of packets which require checksum calculation by the device.

  [rx_needs_csum]
      The number of packets with VIRTIO_NET_HDR_F_NEEDS_CSUM.

and VIRTIO_NET_HDR_F_NEEDS_CSUM gets set on GSO packets virtio sends.

Clarify this in the spec to avoid any confusion.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - remove the note that almost all GSO types need L4 csum
v1: https://lore.kernel.org/20250213010457.1351376-1-kuba@kernel.org

CC: willemb@google.com
CC: ecree.xilinx@gmail.com
CC: neescoba@cisco.com
---
 Documentation/netlink/specs/netdev.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 288923e965ae..48159eb116a4 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -457,6 +457,8 @@ name: netdev
         name: tx-needs-csum
         doc: |
           Number of packets that required the device to calculate the checksum.
+          This counter includes the number of GSO wire packets for which device
+          calculated the L4 checksum.
         type: uint
       -
         name: tx-hw-gso-packets
-- 
2.48.1

Re: [PATCH net-next v2] netdev: clarify GSO vs csum in qstats

[Willem de Bruijn]

Jakub Kicinski wrote:
> Could be just me, but I had to pause and double check that the Tx csum
> counter in qstat should include GSO'd packets. GSO pretty much implies
> csum

Unfortunately specifically to virtio_net, this sensible limitation was
not enforced from the start. Which is why virtio_net_hdr_to_skb has a
branch for !VIRTIO_NET_HDR_F_NEEDS_CSUM && gso_type. Mainly "used" by
syzkaller afaik.

With the addition of USO besides TSO that could also eschew L4 checksum
offload. But the local stack does not generate those (udp_send_skb),
nor does UDP GRO (first branch in udp_gro_receive_segment).

In any case, the new comment clearly mentions this limitation on L4
checksum.

> so one could possibly interpret the csum counter as pure csum offload.
> 
> But the counters are based on virtio:
> 
>   [tx_needs_csum]
>       The number of packets which require checksum calculation by the device.
> 
>   [rx_needs_csum]
>       The number of packets with VIRTIO_NET_HDR_F_NEEDS_CSUM.
> 
> and VIRTIO_NET_HDR_F_NEEDS_CSUM gets set on GSO packets virtio sends.
> 
> Clarify this in the spec to avoid any confusion.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
> v2:
>  - remove the note that almost all GSO types need L4 csum
> v1: https://lore.kernel.org/20250213010457.1351376-1-kuba@kernel.org
> 
> CC: willemb@google.com
> CC: ecree.xilinx@gmail.com
> CC: neescoba@cisco.com
> ---
>  Documentation/netlink/specs/netdev.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> index 288923e965ae..48159eb116a4 100644
> --- a/Documentation/netlink/specs/netdev.yaml
> +++ b/Documentation/netlink/specs/netdev.yaml
> @@ -457,6 +457,8 @@ name: netdev
>          name: tx-needs-csum
>          doc: |
>            Number of packets that required the device to calculate the checksum.
> +          This counter includes the number of GSO wire packets for which device
> +          calculated the L4 checksum.
>          type: uint
>        -
>          name: tx-hw-gso-packets
> -- 
> 2.48.1
> 

Re: [PATCH net-next v2] netdev: clarify GSO vs csum in qstats

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 14 Feb 2025 14:46:01 -0800 you wrote:
> Could be just me, but I had to pause and double check that the Tx csum
> counter in qstat should include GSO'd packets. GSO pretty much implies
> csum so one could possibly interpret the csum counter as pure csum offload.
> 
> But the counters are based on virtio:
> 
>   [tx_needs_csum]
>       The number of packets which require checksum calculation by the device.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] netdev: clarify GSO vs csum in qstats
    https://git.kernel.org/netdev/net-next/c/b5e489003abc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html