| Message ID | 20250214224601.2271201-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b5e489003abcff4b65088d49fce78dcb3cc1ae06 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] netdev: clarify GSO vs csum in qstats | expand |
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 >
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!
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
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(+)