| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240314162142.36b8ff02@kernel.org>
Date: Thu, 14 Mar 2024 16:21:42 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, "Michael S.
Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>, Alexei
Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend
<john.fastabend@...il.com>, Stanislav Fomichev <sdf@...gle.com>, Amritha
Nambiar <amritha.nambiar@...el.com>, Larysa Zaremba
<larysa.zaremba@...el.com>, Sridhar Samudrala
<sridhar.samudrala@...el.com>, Maciej Fijalkowski
<maciej.fijalkowski@...el.com>, virtualization@...ts.linux.dev,
bpf@...r.kernel.org
Subject: Re: [PATCH net-next v4 7/8] netdev: add queue stats
On Thu, 14 Mar 2024 16:54:58 +0800 Xuan Zhuo wrote:
> + -
> + name: rx-hw-drops
> + doc: |
> + Number of packets that arrived at the device but never left it,
> + encompassing packets dropped for reasons such as insufficient buffer
s/encompassing/including/
> + space, processing errors, as well as those affected by explicitly
> + defined policies and packet filtering criteria.
I'm afraid the attempt to "correct my English" ended up decreasing the
clarity. Maybe use what I suggested more directly?
Number of all packets which entered the device, but never left it,
including but not limited to: packets dropped due to lack of buffer
space, processing errors, explicit or implicit policies and packet filters.
> + type: uint
> + -
> + name: rx-hw-drop-overruns
> + doc: |
> + Number of packets that arrived at the device but never left it due to
> + no descriptors.
I put a paragraph in my previous email explaining how tricky this is to
state for HW devices and you just went with the virtio spec :|
Number of packets dropped due to transient lack of resources, such as
buffer space, host descriptors etc.
> + -
> + name: rx-csum-bad
> + doc: |
> + Number of packets that are dropped by device due to invalid checksum.
Quoting myself:
>> Maybe add a note in "bad" that packets with bad csum are not
>> discarded, but still delivered to the stack.
Devices should not drop packets due to invalid L4 checksums.
> + type: uint
> + -
> + name: rx-hw-gro-packets
> + doc: |
> + Number of packets that area coalesced from smaller packets by the device,
> + that do not cover LRO packets.
s/area/were/ ?
s/that do not cover LRO packets/Counts only packets coalesced with the
HW-GRO netdevice feature, LRO-coalesced packets are not counted./
> + type: uint
> + -
> + name: rx-hw-gro-bytes
> + doc: see `rx-hw-gro-packets`.
> + type: uint
> + -
> + name: rx-hw-gro-wire-packets
> + doc: |
> + Number of packets that area coalesced to bigger packets(no LRO packets)
> + by the device.
s/area/were/
s/packets(no LRO packets)/packets by the HW-GRO feature of the device/
> + type: uint
> + -
> + name: rx-hw-gro-wire-bytes
> + doc: see `rx-hw-gro-wire-packets`.
Please make sure the "See `xyz`." references are capitalized (See)
and end with a full stop.
> + type: uint
> + -
> + name: rx-hw-drop-ratelimits
> + doc: |
> + Number of the packets dropped by the device due to the received
> + packets bitrate exceeding the device speed limit.
Maybe s/speed/rate/
> + type: uint
> + -
> + name: tx-hw-drops
> + doc: |
> + Number of packets that arrived at the device but never left it,
> + encompassing packets dropped for reasons such as processing errors, as
> + well as those affected by explicitly defined policies and packet
> + filtering criteria.
> + type: uint
> + -
> + name: tx-hw-drop-errors
> + doc: Number of packets dropped because they were invalid or malformed.
> + type: uint
> + -
> + name: tx-csum-none
> + doc: |
> + Number of packets that do not require the device to calculate the
> + checksum.
I think we should use past tense everywhere.
> + type: uint
> + -
> + name: tx-needs-csum
> + doc: |
> + Number of packets that require the device to calculate the
> + checksum.
> + type: uint
> + -
> + name: tx-hw-gso-packets
> + doc: |
> + Number of packets that necessitated segmentation into smaller packets
> + by the device.
> + type: uint
> + -
> + name: tx-hw-gso-bytes
> + doc: |
> + Successfully segmented into smaller packets by the device, see
> + `tx-hw-gso-packets`.
Maybe stick to "see `tx-hw-gso-packets`", none of the other counters
mention the send being "successful".
> + type: uint
> + -
> + name: tx-hw-gso-wire-packets
> + doc: |
> + Number of the small packets that segmented from bigger packets by the
> + device.
Maybe
Number of wire-sized packets generated by processing
`tx-hw-gso-packets`
?
> + type: uint
> + -
> + name: tx-hw-gso-wire-bytes
> + doc: See `tx-hw-gso-wire-packets`.
> +
> + type: uint
> + -
> + name: tx-hw-drop-ratelimits
> + doc: |
> + Number of the packets dropped by the device due to the transmit
> + packets bitrate exceeding the device speed limit.
s/speed/rate/
> + type: uint
>
> operations:
> list:
> diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
> index 1ec408585373..c7ac4539eafc 100644
> --- a/include/net/netdev_queues.h
> +++ b/include/net/netdev_queues.h
> @@ -9,11 +9,38 @@ struct netdev_queue_stats_rx {
> u64 bytes;
> u64 packets;
> u64 alloc_fail;
> +
> + u64 hw_drops;
> + u64 hw_drop_overruns;
> +
> + u64 csum_unnecessary;
> + u64 csum_none;
> + u64 csum_bad;
> +
> + u64 hw_gro_packets;
> + u64 hw_gro_bytes;
> + u64 hw_gro_wire_packets;
> + u64 hw_gro_wire_bytes;
> +
> + u64 hw_drop_ratelimits;
> };
>
> struct netdev_queue_stats_tx {
> u64 bytes;
> u64 packets;
> +
> + u64 hw_drops;
> + u64 hw_drop_errors;
> +
> + u64 csum_none;
> + u64 needs_csum;
> +
> + u64 hw_gso_packets;
> + u64 hw_gso_bytes;
> + u64 hw_gso_wire_packets;
> + u64 hw_gso_wire_bytes;
> +
> + u64 hw_drop_ratelimits;
> };
>
> /**
> diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
> index bb65ee840cda..702d69b09f14 100644
> --- a/include/uapi/linux/netdev.h
> +++ b/include/uapi/linux/netdev.h
> @@ -147,6 +147,26 @@ enum {
> NETDEV_A_QSTATS_TX_BYTES,
> NETDEV_A_QSTATS_RX_ALLOC_FAIL,
>
Looks hand written. Once you update the yaml file, you should run:
./tools/net/ynl/ynl-regen.sh
This will generate the uAPI changes for you.
> + NETDEV_A_QSTATS_RX_HW_DROPS,
> + NETDEV_A_QSTATS_RX_HW_DROP_OVERRUNS,
> + NETDEV_A_QSTATS_RX_CSUM_UNNECESSARY,
> + NETDEV_A_QSTATS_RX_CSUM_NONE,
> + NETDEV_A_QSTATS_RX_CSUM_BAD,
> + NETDEV_A_QSTATS_RX_HW_GRO_PACKETS,
> + NETDEV_A_QSTATS_RX_HW_GRO_BYTES,
> + NETDEV_A_QSTATS_RX_HW_GRO_WIRE_PACKETS,
> + NETDEV_A_QSTATS_RX_HW_GRO_WIRE_BYTES,
> + NETDEV_A_QSTATS_RX_HW_DROP_RATELIMITS,
> + NETDEV_A_QSTATS_TX_HW_DROPS,
> + NETDEV_A_QSTATS_TX_HW_DROP_ERRORS,
> + NETDEV_A_QSTATS_TX_CSUM_NONE,
> + NETDEV_A_QSTATS_TX_NEEDS_CSUM,
> + NETDEV_A_QSTATS_TX_HW_GSO_PACKETS,
> + NETDEV_A_QSTATS_TX_HW_GSO_BYTES,
> + NETDEV_A_QSTATS_TX_HW_GSO_WIRE_PACKETS,
> + NETDEV_A_QSTATS_TX_HW_GSO_WIRE_BYTES,
> + NETDEV_A_QSTATS_TX_HW_DROP_RATELIMITS,
> +
> __NETDEV_A_QSTATS_MAX,
> NETDEV_A_QSTATS_MAX = (__NETDEV_A_QSTATS_MAX - 1)
> };
Powered by blists - more mailing lists