| Message ID | 20240528142807.903965-12-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/mlx5e: SHAMPO, Enable HW GRO once more | expand |
On Tue, 28 May 2024 17:28:03 +0300 Tariq Toukan wrote: > + * - `rx[i]_hds_nosplit_packets` > + - Number of packets that were not split in modes that do header/data split > + [#accel]_. > + - Informative > + > + * - `rx[i]_hds_nosplit_bytes` > + - Number of bytes that were not split in modes that do header/data split > + [#accel]_. > + - Informative This is too vague. The ethtool HDS feature is for TCP only. What does this count? Non-TCP packets basically? Given this is a HW-GRO series, are HDS packets == HW-GRO eligible packets?
On 29 May 18:22, Jakub Kicinski wrote: >On Tue, 28 May 2024 17:28:03 +0300 Tariq Toukan wrote: >> + * - `rx[i]_hds_nosplit_packets` >> + - Number of packets that were not split in modes that do header/data split >> + [#accel]_. >> + - Informative >> + >> + * - `rx[i]_hds_nosplit_bytes` >> + - Number of bytes that were not split in modes that do header/data split >> + [#accel]_. >> + - Informative > >This is too vague. The ethtool HDS feature is for TCP only. >What does this count? Non-TCP packets basically? > But this is not the ethtool HDS, this is the mlx5 HW GRO hds. On the sane note, are we planning to have different control knobs/stats for tcp/udp/ip HDS? ConnectX supports both TCP and UDP on the same queue, the driver has no control on which protocol gets HDS and which doesn't. >Given this is a HW-GRO series, are HDS packets == HW-GRO eligible >packets? > No, UDP will also get header data split or other TCP packets that don't belong to any aggregation context in the HW.
On Wed, 29 May 2024 20:32:23 -0700 Saeed Mahameed wrote: > On 29 May 18:22, Jakub Kicinski wrote: > >On Tue, 28 May 2024 17:28:03 +0300 Tariq Toukan wrote: > >> + * - `rx[i]_hds_nosplit_packets` > >> + - Number of packets that were not split in modes that do header/data split > >> + [#accel]_. > >> + - Informative > >> + > >> + * - `rx[i]_hds_nosplit_bytes` > >> + - Number of bytes that were not split in modes that do header/data split > >> + [#accel]_. > >> + - Informative > > > >This is too vague. The ethtool HDS feature is for TCP only. > >What does this count? Non-TCP packets basically? > > But this is not the ethtool HDS, this is the mlx5 HW GRO hds. Okay, but you need to put more detail into the description. "not split in modes which do split" is going to immediately make the reader ask themselves "but why?". > On the sane note, are we planning to have different control knobs/stats for > tcp/udp/ip HDS? ConnectX supports both TCP and UDP on the same queue, > the driver has no control on which protocol gets HDS and which doesn't. No plans at this stage. The ethtool HDS is specifically there to tell user space whether it should bother trying to use TCP mmap. > >Given this is a HW-GRO series, are HDS packets == HW-GRO eligible > >packets? > > No, UDP will also get header data split or other TCP packets that don't > belong to any aggregation context in the HW. I see.
On Thu, 2024-05-30 at 08:31 -0700, Jakub Kicinski wrote: > On Wed, 29 May 2024 20:32:23 -0700 Saeed Mahameed wrote: > > On 29 May 18:22, Jakub Kicinski wrote: > > > On Tue, 28 May 2024 17:28:03 +0300 Tariq Toukan wrote: > > > > + * - `rx[i]_hds_nosplit_packets` > > > > + - Number of packets that were not split in modes that do header/data split > > > > + [#accel]_. > > > > + - Informative > > > > + > > > > + * - `rx[i]_hds_nosplit_bytes` > > > > + - Number of bytes that were not split in modes that do header/data split > > > > + [#accel]_. > > > > + - Informative > > > > > > This is too vague. The ethtool HDS feature is for TCP only. > > > What does this count? Non-TCP packets basically? > > > > But this is not the ethtool HDS, this is the mlx5 HW GRO hds. > > Okay, but you need to put more detail into the description. > "not split in modes which do split" is going to immediately > make the reader ask themselves "but why?". > We discussed internally and decided to drop this counter and patch for now. This will be added back in the HDS series so that we have more time to converge on a the documentation part. > > On the sane note, are we planning to have different control knobs/stats for > > tcp/udp/ip HDS? ConnectX supports both TCP and UDP on the same queue, > > the driver has no control on which protocol gets HDS and which doesn't. > > No plans at this stage. The ethtool HDS is specifically there > to tell user space whether it should bother trying to use TCP mmap. > > > > Given this is a HW-GRO series, are HDS packets == HW-GRO eligible > > > packets? > > > > No, UDP will also get header data split or other TCP packets that don't > > belong to any aggregation context in the HW. > > I see. Thanks, Dragos
diff --git a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst index 18638a8e7c73..deb0e07432c4 100644 --- a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst +++ b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst @@ -209,6 +209,16 @@ the software port. headers that require additional memory to be allocated. - Informative + * - `rx[i]_hds_nosplit_packets` + - Number of packets that were not split in modes that do header/data split + [#accel]_. + - Informative + + * - `rx[i]_hds_nosplit_bytes` + - Number of bytes that were not split in modes that do header/data split + [#accel]_. + - Informative + * - `rx[i]_lro_packets` - The number of LRO packets received on ring i [#accel]_. - Acceleration diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c index 7ab7215843b6..f40f34877904 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c @@ -2332,6 +2332,9 @@ static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cq frag_page = &wi->alloc_units.frag_pages[page_idx]; mlx5e_shampo_fill_skb_data(*skb, rq, frag_page, data_bcnt, data_offset); } + } else { + stats->hds_nosplit_packets++; + stats->hds_nosplit_bytes += data_bcnt; } mlx5e_shampo_complete_rx_cqe(rq, cqe, cqe_bcnt, *skb); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c index a1657fad9a0d..96ecf675f90d 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c @@ -343,6 +343,8 @@ static void mlx5e_stats_grp_sw_update_stats_rq_stats(struct mlx5e_sw_stats *s, s->rx_gro_bytes += rq_stats->gro_bytes; s->rx_gro_skbs += rq_stats->gro_skbs; s->rx_gro_large_hds += rq_stats->gro_large_hds; + s->rx_hds_nosplit_packets += rq_stats->hds_nosplit_packets; + s->rx_hds_nosplit_bytes += rq_stats->hds_nosplit_bytes; s->rx_ecn_mark += rq_stats->ecn_mark; s->rx_removed_vlan_packets += rq_stats->removed_vlan_packets; s->rx_csum_none += rq_stats->csum_none; @@ -2052,6 +2054,8 @@ static const struct counter_desc rq_stats_desc[] = { { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, gro_bytes) }, { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, gro_skbs) }, { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, gro_large_hds) }, + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, hds_nosplit_packets) }, + { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, hds_nosplit_bytes) }, { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, ecn_mark) }, { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, removed_vlan_packets) }, { MLX5E_DECLARE_RX_STAT(struct mlx5e_rq_stats, wqe_err) }, diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h index 25daae526caa..6967c8c91f9a 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h @@ -154,6 +154,8 @@ struct mlx5e_sw_stats { u64 rx_gro_bytes; u64 rx_gro_skbs; u64 rx_gro_large_hds; + u64 rx_hds_nosplit_packets; + u64 rx_hds_nosplit_bytes; u64 rx_mcast_packets; u64 rx_ecn_mark; u64 rx_removed_vlan_packets; @@ -352,6 +354,8 @@ struct mlx5e_rq_stats { u64 gro_bytes; u64 gro_skbs; u64 gro_large_hds; + u64 hds_nosplit_packets; + u64 hds_nosplit_bytes; u64 mcast_packets; u64 ecn_mark; u64 removed_vlan_packets;