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

[PATCH net-next] 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>
---
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..0b311dc49691 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 (which means pretty much all of them).
         type: uint
       -
         name: tx-hw-gso-packets
-- 
2.48.1

Re: [PATCH net-next] 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 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>
> ---
> 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..0b311dc49691 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

Good to call out wire packets, as opposed to the GSO packet itself. That is
sometimes left ambiguous in GSO counting.

> +          calculated the L4 checksum (which means pretty much all of them).

Can we clarify what pretty much here means?

TSO requires checksum offload. USO with zero checksum is the only exception that
I can think of right now.

>          type: uint
>        -
>          name: tx-hw-gso-packets
> -- 
> 2.48.1
> 

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

[Jakub Kicinski]

On Thu, 13 Feb 2025 10:39:30 -0500 Willem de Bruijn wrote:
> > +          calculated the L4 checksum (which means pretty much all of them).  
> 
> Can we clarify what pretty much here means?
> 
> TSO requires checksum offload. USO with zero checksum is the only exception that
> I can think of right now.

I was worried that somehow another case will appear and the comment
will become stale. I will just delete the part in the parenthesis.