| Message ID | 20240326222022.27926-6-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mlx5e misc patches | expand |
On Wed, Mar 27, 2024 at 12:20:19AM +0200, Tariq Toukan wrote: > From: Carolina Jubran <cjubran@nvidia.com> > > Q counters are device-level counters that track specific > events, among which are out_of_buffer events. These events > occur when packets are dropped due to a lack of receive > buffer in the RX queue. > > Expose the total number of out_of_buffer events on the > VF/SF to their respective representor, using the > "ethtool -S" under the name of "rx_vport_out_of_buffer". > > The "rx_vport_out_of_buffer" equals the sum of all > Q counters out_of_buffer values allocated on the VF/SF. Hi Carolina and Tariq, I am wondering if any consideration was given to making this a generic counter. Buffer exhaustion sounds like something that other NICs may report too. > > Signed-off-by: Carolina Jubran <cjubran@nvidia.com> > Reviewed-by: Gal Pressman <gal@nvidia.com> > Reviewed-by: Aya Levin <ayal@nvidia.com> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com> ...
On Thu, 28 Mar 2024 11:18:31 +0000 Simon Horman wrote: > > The "rx_vport_out_of_buffer" equals the sum of all > > Q counters out_of_buffer values allocated on the VF/SF. > > Hi Carolina and Tariq, > > I am wondering if any consideration was given to making this > a generic counter. Buffer exhaustion sounds like something that > other NICs may report too. I think it's basically rx_missed_errors from rtnl_link_stats64. mlx5 doesn't currently report it at all, AFAICT.
On 28/03/2024 18:21, Jakub Kicinski wrote: > On Thu, 28 Mar 2024 11:18:31 +0000 Simon Horman wrote: >>> The "rx_vport_out_of_buffer" equals the sum of all >>> Q counters out_of_buffer values allocated on the VF/SF. >> >> Hi Carolina and Tariq, >> >> I am wondering if any consideration was given to making this >> a generic counter. Buffer exhaustion sounds like something that >> other NICs may report too. > > I think it's basically rx_missed_errors from rtnl_link_stats64. > mlx5 doesn't currently report it at all, AFAICT. > We expose it in ethtool stats. Note that the "local" RX buffer exhaustion counter exists for a long time. Here we introduce in the representor kind of a "remote" version of the counter, to help providers monitor RX drops that occur in the customers' side. It follows the local counter hence currently it is not generic.
On Sun, 31 Mar 2024 21:52:06 +0300 Tariq Toukan wrote: > >> Hi Carolina and Tariq, > >> > >> I am wondering if any consideration was given to making this > >> a generic counter. Buffer exhaustion sounds like something that > >> other NICs may report too. > > > > I think it's basically rx_missed_errors from rtnl_link_stats64. > > mlx5 doesn't currently report it at all, AFAICT. > > We expose it in ethtool stats. > Note that the "local" RX buffer exhaustion counter exists for a long time. > > Here we introduce in the representor kind of a "remote" version of the > counter, to help providers monitor RX drops that occur in the customers' > side. > > It follows the local counter hence currently it is not generic. I thought you'd say that, but we can't apply the rules selectively. Everyone has "local" counters already when we add the generic ones. Especially when we start broadening the "have" to encompass other generations of HW / netdev instance types.
On Mon, 01 Apr, 2024 08:03:15 -0700 Jakub Kicinski <kuba@kernel.org> wrote: > On Sun, 31 Mar 2024 21:52:06 +0300 Tariq Toukan wrote: >> >> Hi Carolina and Tariq, >> >> >> >> I am wondering if any consideration was given to making this >> >> a generic counter. Buffer exhaustion sounds like something that >> >> other NICs may report too. >> > >> > I think it's basically rx_missed_errors from rtnl_link_stats64. >> > mlx5 doesn't currently report it at all, AFAICT. >> >> We expose it in ethtool stats. >> Note that the "local" RX buffer exhaustion counter exists for a long time. >> >> Here we introduce in the representor kind of a "remote" version of the >> counter, to help providers monitor RX drops that occur in the customers' >> side. >> >> It follows the local counter hence currently it is not generic. > > I thought you'd say that, but we can't apply the rules selectively. > Everyone has "local" counters already when we add the generic ones. > Especially when we start broadening the "have" to encompass other > generations of HW / netdev instance types. In the normal case, we actual misreport out_of_buffer as rx_dropped.... Carolina is working on a fix for that. https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/ethernet/mellanox/mlx5/core/en_main.c?id=ea2a1cfc3b2019bdea6324acd3c03606b60d71ad#n3793 With regards to the case for VF/SF representors, we will be adding support for reporting the out_of_buffer statistics through the rx_missed_errors counter as well. -- Thanks, Rahul Rameshbabu
diff --git a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst index f69ee1ebee01..fa7c00f6d0ce 100644 --- a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst +++ b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst @@ -826,6 +826,11 @@ Counters on the NIC port that is connected to a eSwitch. and transmitted), IB/Eth [#accel]_. - Acceleration + * - `rx_vport_out_of_buffer` + - Number of times receive queue on the associated vport had no software buffers allocated for the + adapter's incoming traffic. + - Error + * - `rx_steer_missed_packets` - Number of packets that was received by the NIC, however was discarded because it did not match any flow in the NIC flow table. diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c index 55b7efe21624..dd74fd82707c 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c @@ -112,6 +112,10 @@ static const struct counter_desc vport_rep_stats_desc[] = { tx_vport_rdma_multicast_bytes) }, }; +static const struct counter_desc vport_qcounter_rep_stats_desc[] = { + { MLX5E_DECLARE_STAT(struct mlx5e_rep_stats, rx_vport_out_of_buffer) }, +}; + static const struct counter_desc vport_rep_loopback_stats_desc[] = { { MLX5E_DECLARE_STAT(struct mlx5e_rep_stats, vport_loopback_packets) }, @@ -121,6 +125,9 @@ static const struct counter_desc vport_rep_loopback_stats_desc[] = { #define NUM_VPORT_REP_SW_COUNTERS ARRAY_SIZE(sw_rep_stats_desc) #define NUM_VPORT_REP_HW_COUNTERS ARRAY_SIZE(vport_rep_stats_desc) +#define NUM_VPORT_QCOUNTER_REP_COUNTERS(dev) \ + ((MLX5_CAP_GEN(dev, q_counter_other_vport) && MLX5_CAP_GEN(dev, q_counter_aggregation)) ? \ + ARRAY_SIZE(vport_qcounter_rep_stats_desc) : 0) #define NUM_VPORT_REP_LOOPBACK_COUNTERS(dev) \ (MLX5_CAP_GEN(dev, vport_counter_local_loopback) ? \ ARRAY_SIZE(vport_rep_loopback_stats_desc) : 0) @@ -273,6 +280,62 @@ static MLX5E_DECLARE_STATS_GRP_OP_UPDATE_STATS(vport_rep) kvfree(out); } +static MLX5E_DECLARE_STATS_GRP_OP_NUM_STATS(vport_qcounter_rep) +{ + return NUM_VPORT_QCOUNTER_REP_COUNTERS(priv->mdev); +} + +static MLX5E_DECLARE_STATS_GRP_OP_FILL_STRS(vport_qcounter_rep) +{ + int i; + + for (i = 0; i < NUM_VPORT_QCOUNTER_REP_COUNTERS(priv->mdev); i++) + ethtool_puts(data, vport_qcounter_rep_stats_desc[i].format); +} + +static MLX5E_DECLARE_STATS_GRP_OP_FILL_STATS(vport_qcounter_rep) +{ + int i; + + for (i = 0; i < NUM_VPORT_QCOUNTER_REP_COUNTERS(priv->mdev); i++) + mlx5e_ethtool_put_stat( + data, MLX5E_READ_CTR64_CPU(&priv->stats.rep_stats, + vport_qcounter_rep_stats_desc, i)); +} + +static int mlx5e_rep_query_q_counter(struct mlx5_core_dev *dev, int vport, void *out) +{ + u32 in[MLX5_ST_SZ_DW(query_q_counter_in)] = {}; + + MLX5_SET(query_q_counter_in, in, opcode, MLX5_CMD_OP_QUERY_Q_COUNTER); + MLX5_SET(query_q_counter_in, in, other_vport, 1); + MLX5_SET(query_q_counter_in, in, vport_number, vport); + MLX5_SET(query_q_counter_in, in, aggregate, 1); + + return mlx5_cmd_exec_inout(dev, query_q_counter, in, out); +} + +static MLX5E_DECLARE_STATS_GRP_OP_UPDATE_STATS(vport_qcounter_rep) +{ + struct mlx5e_rep_stats *rep_stats = &priv->stats.rep_stats; + u32 out[MLX5_ST_SZ_DW(query_q_counter_out)] = {}; + struct mlx5e_rep_priv *rpriv = priv->ppriv; + struct mlx5_eswitch_rep *rep = rpriv->rep; + int err; + + if (!MLX5_CAP_GEN(priv->mdev, q_counter_other_vport) || + !MLX5_CAP_GEN(priv->mdev, q_counter_aggregation)) + return; + + err = mlx5e_rep_query_q_counter(priv->mdev, rep->vport, out); + if (err) { + netdev_warn(priv->netdev, "failed reading stats on vport %d, error %d\n", + rep->vport, err); + return; + } + rep_stats->rx_vport_out_of_buffer = MLX5_GET(query_q_counter_out, out, out_of_buffer); +} + static void mlx5e_rep_get_strings(struct net_device *dev, u32 stringset, uint8_t *data) { @@ -1326,11 +1389,13 @@ static void mlx5e_uplink_rep_disable(struct mlx5e_priv *priv) static MLX5E_DEFINE_STATS_GRP(sw_rep, 0); static MLX5E_DEFINE_STATS_GRP(vport_rep, MLX5E_NDO_UPDATE_STATS); +static MLX5E_DEFINE_STATS_GRP(vport_qcounter_rep, 0); /* The stats groups order is opposite to the update_stats() order calls */ static mlx5e_stats_grp_t mlx5e_rep_stats_grps[] = { &MLX5E_STATS_GRP(sw_rep), &MLX5E_STATS_GRP(vport_rep), + &MLX5E_STATS_GRP(vport_qcounter_rep), }; static unsigned int mlx5e_rep_stats_grps_num(struct mlx5e_priv *priv) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h index b71e3fdf92c5..ef88ce4f0200 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h @@ -480,6 +480,7 @@ struct mlx5e_rep_stats { u64 tx_vport_rdma_multicast_bytes; u64 vport_loopback_packets; u64 vport_loopback_bytes; + u64 rx_vport_out_of_buffer; }; struct mlx5e_stats {