[PATCH net] netdev: add qstat for csum complete

[PATCH net] netdev: add qstat for csum complete

[Jakub Kicinski]

Recent commit 0cfe71f45f42 ("netdev: add queue stats") added
a lot of useful stats, but only those immediately needed by virtio.
Presumably virtio does not support CHECKSUM_COMPLETE,
so statistic for that form of checksumming wasn't included.
Other drivers will definitely need it, in fact we expect it
to be needed in net-next soon (mlx5). So let's add the definition
of the counter for CHECKSUM_COMPLETE to uAPI in net already,
so that the counters are in a more natural order (all subsequent
counters have not been present in any released kernel, yet).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: donald.hunter@gmail.com
CC: sdf@google.com
CC: amritha.nambiar@intel.com
CC: hawk@kernel.org
CC: sridhar.samudrala@intel.com
CC: jdamato@fastly.com
---
 Documentation/netlink/specs/netdev.yaml | 4 ++++
 include/uapi/linux/netdev.h             | 1 +
 tools/include/uapi/linux/netdev.h       | 1 +
 3 files changed, 6 insertions(+)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 11a32373365a..959755be4d7f 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -349,6 +349,10 @@ name: netdev
           Number of packets dropped due to transient lack of resources, such as
           buffer space, host descriptors etc.
         type: uint
+      -
+        name: rx-csum-complete
+        doc: Number of packets that were marked as CHECKSUM_COMPLETE.
+        type: uint
       -
         name: rx-csum-unnecessary
         doc: Number of packets that were marked as CHECKSUM_UNNECESSARY.
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index a8188202413e..43742ac5b00d 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -148,6 +148,7 @@ enum {
 	NETDEV_A_QSTATS_RX_ALLOC_FAIL,
 	NETDEV_A_QSTATS_RX_HW_DROPS,
 	NETDEV_A_QSTATS_RX_HW_DROP_OVERRUNS,
+	NETDEV_A_QSTATS_RX_CSUM_COMPLETE,
 	NETDEV_A_QSTATS_RX_CSUM_UNNECESSARY,
 	NETDEV_A_QSTATS_RX_CSUM_NONE,
 	NETDEV_A_QSTATS_RX_CSUM_BAD,
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index a8188202413e..43742ac5b00d 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -148,6 +148,7 @@ enum {
 	NETDEV_A_QSTATS_RX_ALLOC_FAIL,
 	NETDEV_A_QSTATS_RX_HW_DROPS,
 	NETDEV_A_QSTATS_RX_HW_DROP_OVERRUNS,
+	NETDEV_A_QSTATS_RX_CSUM_COMPLETE,
 	NETDEV_A_QSTATS_RX_CSUM_UNNECESSARY,
 	NETDEV_A_QSTATS_RX_CSUM_NONE,
 	NETDEV_A_QSTATS_RX_CSUM_BAD,
-- 
2.45.1

Re: [PATCH net] netdev: add qstat for csum complete

[Joe Damato]

On Wed, May 29, 2024 at 09:35:47AM -0700, Jakub Kicinski wrote:
> Recent commit 0cfe71f45f42 ("netdev: add queue stats") added
> a lot of useful stats, but only those immediately needed by virtio.
> Presumably virtio does not support CHECKSUM_COMPLETE,
> so statistic for that form of checksumming wasn't included.
> Other drivers will definitely need it, in fact we expect it
> to be needed in net-next soon (mlx5). So let's add the definition
> of the counter for CHECKSUM_COMPLETE to uAPI in net already,
> so that the counters are in a more natural order (all subsequent
> counters have not been present in any released kernel, yet).

As you mentioned, the counters are not in any released kernel yet,
so adding it to the uAPI makes sense to me.

Are you planning to submit a separate change to add csum_complete to
struct netdev_queue_stats_rx, as well? Just wanted to double check,
but I assume that is a net-next thing.

Reviewed-by: Joe Damato <jdamato@fastly.com>

Re: [PATCH net] netdev: add qstat for csum complete

[Jakub Kicinski]

On Wed, 29 May 2024 11:23:49 -0700 Joe Damato wrote:
> Are you planning to submit a separate change to add csum_complete to
> struct netdev_queue_stats_rx, as well? Just wanted to double check,
> but I assume that is a net-next thing.

I figured we can add the handling with the first user. I can, unless
you're already planning to add more counters yourself once the base 
set is merged.

Re: [PATCH net] netdev: add qstat for csum complete

[Joe Damato]

On Wed, May 29, 2024 at 11:57:36AM -0700, Jakub Kicinski wrote:
> On Wed, 29 May 2024 11:23:49 -0700 Joe Damato wrote:
> > Are you planning to submit a separate change to add csum_complete to
> > struct netdev_queue_stats_rx, as well? Just wanted to double check,
> > but I assume that is a net-next thing.
> 
> I figured we can add the handling with the first user. I can, unless
> you're already planning to add more counters yourself once the base 
> set is merged.

Sure, adding with the first user makes sense to me.

On my side: I honestly have no idea how close or far away I am from
landing the mlx5 qstats stuff, but if I do somehow get that in, I
would be fine adding the extra stats at that point.

Hoping the RFCv3 I have out now is closer/almost there.

Re: [PATCH net] netdev: add qstat for csum complete

[Paolo Abeni]

On Wed, 2024-05-29 at 09:35 -0700, Jakub Kicinski wrote:
> Recent commit 0cfe71f45f42 ("netdev: add queue stats") added
> a lot of useful stats, but only those immediately needed by virtio.
> Presumably virtio does not support CHECKSUM_COMPLETE,
> so statistic for that form of checksumming wasn't included.
> Other drivers will definitely need it, in fact we expect it
> to be needed in net-next soon (mlx5). So let's add the definition
> of the counter for CHECKSUM_COMPLETE to uAPI in net already,
> so that the counters are in a more natural order (all subsequent
> counters have not been present in any released kernel, yet).
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Given the uAPI implication I believe its better to let this land into
today PR. Also adding

Fixes: 0cfe71f45f42 ("netdev: add queue stats")

To try to reduce the chance that backports will end-up with
inconsistent uAPI.

Cheers,

Paolo

Re: [PATCH net] netdev: add qstat for csum complete

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 29 May 2024 09:35:47 -0700 you wrote:
> Recent commit 0cfe71f45f42 ("netdev: add queue stats") added
> a lot of useful stats, but only those immediately needed by virtio.
> Presumably virtio does not support CHECKSUM_COMPLETE,
> so statistic for that form of checksumming wasn't included.
> Other drivers will definitely need it, in fact we expect it
> to be needed in net-next soon (mlx5). So let's add the definition
> of the counter for CHECKSUM_COMPLETE to uAPI in net already,
> so that the counters are in a more natural order (all subsequent
> counters have not been present in any released kernel, yet).
> 
> [...]

Here is the summary with links:
  - [net] netdev: add qstat for csum complete
    https://git.kernel.org/netdev/net/c/13c7c941e729

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