This series follows the Jakub's work on standard statistics and
unifies XDP statistics across [most of] the drivers.
The only driver left unconverted is mlx5 -- it has rather complex
statistics, so I believe it would be better to leave this up to
its developers.
The stats itself consists of 12 counters:
- packets: number of frames passed to bpf_prog_run_xdp();
- errors: number of general XDP errors, if driver has one unified counter;
- aborted: number of XDP_ABORTED returns;
- drop: number of XDP_DROP returns;
- invalid: number of returns of unallowed values (i.e. not XDP_*);
- pass: number of XDP_PASS returns;
- redirect: number of successfully performed XDP_REDIRECT requests;
- redirect_errors: number of failed XDP_REDIRECT requests;
- tx: number of successfully performed XDP_TX requests;
- tx_errors: number of failed XDP_TX requests;
- xmit: number of xdp_frames successfully transmitted via .ndo_xdp_xmit();
- xmit_drops: number of frames dropped from .ndo_xdp_xmit().
As most drivers stores them on a per-channel basis, Ethtool standard
stats infra has been expanded to support this. A new nested
attribute has been added which indicated that the fields enclosed
in this block are related to one particular channel. If Ethtool
utility is older than the kernel, those blocks will just be skipped
with no errors.
When the stats are not per-channel, Ethtool core treats them as
regular and so does Ethtool utility display them. Otherwise,
the example output looks like:
$ ./ethtool -S enp175s0f0 --all-groups
Standard stats for enp175s0f0:
[ snip ]
channel0-xdp-aborted: 1
channel0-xdp-drop: 2
channel0-xdp-illegal: 3
channel0-xdp-pass: 4
channel0-xdp-redirect: 5
[ snip ]
...and the JSON output looks like:
[ snip ]
"xdp": {
"per-channel": [
"channel0": {
"aborted": 1,
"drop": 2,
"illegal": 3,
"pass": 4,
"redirect": 5,
[ snip ]
} ]
}
[ snip ]
Rouhly half of the commits are present to unify XDP stats logics
across the drivers, and the first two are preparatory/housekeeping.
This set is also available here: [0]
[0] https://github.com/alobakin/linux/tree/xdp_stats
Alexander Lobakin (21):
ethtool, stats: use a shorthand pointer in stats_prepare_data()
ethtool, stats: add compile-time checks for standard stats
ethtool, stats: introduce standard XDP statistics
ethernet, dpaa2: simplify per-channel Ethtool stats counting
ethernet, dpaa2: convert to standard XDP stats
ethernet, ena: constify src and syncp args of ena_safe_update_stat()
ethernet, ena: convert to standard XDP stats
ethernet, enetc: convert to standard XDP stats
ethernet, mvneta: rename xdp_xmit_err to xdp_xmit_drops
ethernet, mvneta: convert to standard XDP stats
ethernet, mvpp2: rename xdp_xmit_err to xdp_xmit_drops
ethernet, mvpp2: convert to standard XDP stats
ethernet, sfc: convert to standard XDP stats
veth: rename rx_drops to xdp_errors
veth: rename xdp_xmit_errors to xdp_xmit_drops
veth: rename drop xdp_ suffix from packets and bytes stats
veth: convert to standard XDP stats
virtio-net: rename xdp_tx{,__drops} SQ stats to xdp_xmit{,__drops}
virtio-net: don't mix error-caused drops with XDP_DROP cases
virtio-net: convert to standard XDP stats
Documentation, ethtool-netlink: update standard statistics
documentation
Documentation/networking/ethtool-netlink.rst | 45 +++--
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 50 +++++-
.../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 7 +-
.../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 38 +++-
.../ethernet/freescale/enetc/enetc_ethtool.c | 58 ++++--
drivers/net/ethernet/marvell/mvneta.c | 112 ++++++------
drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 2 +-
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 96 +++-------
drivers/net/ethernet/sfc/ef100_ethtool.c | 2 +
drivers/net/ethernet/sfc/ethtool.c | 2 +
drivers/net/ethernet/sfc/ethtool_common.c | 35 +++-
drivers/net/ethernet/sfc/ethtool_common.h | 3 +
drivers/net/veth.c | 167 ++++++++++--------
drivers/net/virtio_net.c | 76 ++++++--
include/linux/ethtool.h | 36 ++++
include/uapi/linux/ethtool.h | 2 +
include/uapi/linux/ethtool_netlink.h | 34 ++++
net/ethtool/netlink.h | 1 +
net/ethtool/stats.c | 163 +++++++++++++++--
net/ethtool/strset.c | 5 +
20 files changed, 659 insertions(+), 275 deletions(-)
--
2.31.1
Make sure that the number of counters inside stats structures is
with the corresponding Ethtool Netlink definitions.
RMON stats is a special case -- don't take histogram fields into
account.
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
---
net/ethtool/stats.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/ethtool/stats.c b/net/ethtool/stats.c
index e35c87206b4c..8b5c27e034f9 100644
--- a/net/ethtool/stats.c
+++ b/net/ethtool/stats.c
@@ -117,6 +117,15 @@ static int stats_prepare_data(const struct ethnl_req_info *req_base,
ops = dev->ethtool_ops;
+ BUILD_BUG_ON(sizeof(data->phy_stats) / sizeof(u64) !=
+ __ETHTOOL_A_STATS_ETH_PHY_CNT);
+ BUILD_BUG_ON(sizeof(data->mac_stats) / sizeof(u64) !=
+ __ETHTOOL_A_STATS_ETH_MAC_CNT);
+ BUILD_BUG_ON(sizeof(data->ctrl_stats) / sizeof(u64) !=
+ __ETHTOOL_A_STATS_ETH_CTRL_CNT);
+ BUILD_BUG_ON(offsetof(typeof(data->rmon_stats), hist) / sizeof(u64) !=
+ __ETHTOOL_A_STATS_RMON_CNT);
+
/* Mark all stats as unset (see ETHTOOL_STAT_NOT_SET) to prevent them
* from being reported to user space in case driver did not set them.
*/
--
2.31.1
Just place dev->ethtool_ops on the stack and use it instead of
dereferencing the former a bunch of times to improve code
readability.
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
---
net/ethtool/stats.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/net/ethtool/stats.c b/net/ethtool/stats.c
index ec07f5765e03..e35c87206b4c 100644
--- a/net/ethtool/stats.c
+++ b/net/ethtool/stats.c
@@ -108,12 +108,15 @@ static int stats_prepare_data(const struct ethnl_req_info *req_base,
const struct stats_req_info *req_info = STATS_REQINFO(req_base);
struct stats_reply_data *data = STATS_REPDATA(reply_base);
struct net_device *dev = reply_base->dev;
+ const struct ethtool_ops *ops;
int ret;
ret = ethnl_ops_begin(dev);
if (ret < 0)
return ret;
+ ops = dev->ethtool_ops;
+
/* Mark all stats as unset (see ETHTOOL_STAT_NOT_SET) to prevent them
* from being reported to user space in case driver did not set them.
*/
@@ -123,18 +126,17 @@ static int stats_prepare_data(const struct ethnl_req_info *req_base,
memset(&data->rmon_stats, 0xff, sizeof(data->rmon_stats));
if (test_bit(ETHTOOL_STATS_ETH_PHY, req_info->stat_mask) &&
- dev->ethtool_ops->get_eth_phy_stats)
- dev->ethtool_ops->get_eth_phy_stats(dev, &data->phy_stats);
+ ops->get_eth_phy_stats)
+ ops->get_eth_phy_stats(dev, &data->phy_stats);
if (test_bit(ETHTOOL_STATS_ETH_MAC, req_info->stat_mask) &&
- dev->ethtool_ops->get_eth_mac_stats)
- dev->ethtool_ops->get_eth_mac_stats(dev, &data->mac_stats);
+ ops->get_eth_mac_stats)
+ ops->get_eth_mac_stats(dev, &data->mac_stats);
if (test_bit(ETHTOOL_STATS_ETH_CTRL, req_info->stat_mask) &&
- dev->ethtool_ops->get_eth_ctrl_stats)
- dev->ethtool_ops->get_eth_ctrl_stats(dev, &data->ctrl_stats);
+ ops->get_eth_ctrl_stats)
+ ops->get_eth_ctrl_stats(dev, &data->ctrl_stats);
if (test_bit(ETHTOOL_STATS_RMON, req_info->stat_mask) &&
- dev->ethtool_ops->get_rmon_stats)
- dev->ethtool_ops->get_rmon_stats(dev, &data->rmon_stats,
- &data->rmon_ranges);
+ ops->get_rmon_stats)
+ ops->get_rmon_stats(dev, &data->rmon_stats, &data->rmon_ranges);
ethnl_ops_complete(dev);
return 0;
--
2.31.1
Its 6 XDP per-channel counters align just fine with the standard
stats.
Drop them from the custom Ethtool statistics and expose to the
standard stats infra instead.
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
---
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 46 ++++++++++++++++---
1 file changed, 40 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
index 851a198cec82..1b6563641575 100644
--- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
+++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
@@ -90,12 +90,6 @@ static const struct ena_stats ena_stats_rx_strings[] = {
ENA_STAT_RX_ENTRY(bad_req_id),
ENA_STAT_RX_ENTRY(empty_rx_ring),
ENA_STAT_RX_ENTRY(csum_unchecked),
- ENA_STAT_RX_ENTRY(xdp_aborted),
- ENA_STAT_RX_ENTRY(xdp_drop),
- ENA_STAT_RX_ENTRY(xdp_pass),
- ENA_STAT_RX_ENTRY(xdp_tx),
- ENA_STAT_RX_ENTRY(xdp_invalid),
- ENA_STAT_RX_ENTRY(xdp_redirect),
};
static const struct ena_stats ena_stats_ena_com_strings[] = {
@@ -324,6 +318,44 @@ static void ena_get_ethtool_strings(struct net_device *netdev,
}
}
+static int ena_get_std_stats_channels(struct net_device *netdev, u32 sset)
+{
+ const struct ena_adapter *adapter = netdev_priv(netdev);
+
+ switch (sset) {
+ case ETH_SS_STATS_XDP:
+ return adapter->num_io_queues;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void ena_get_xdp_stats(struct net_device *netdev,
+ struct ethtool_xdp_stats *xdp_stats)
+{
+ const struct ena_adapter *adapter = netdev_priv(netdev);
+ const struct u64_stats_sync *syncp;
+ const struct ena_stats_rx *stats;
+ struct ethtool_xdp_stats *iter;
+ u32 i;
+
+ for (i = 0; i < adapter->num_io_queues; i++) {
+ stats = &adapter->rx_ring[i].rx_stats;
+ syncp = &adapter->rx_ring[i].syncp;
+ iter = xdp_stats + i;
+
+ ena_safe_update_stat(&stats->xdp_drop, &iter->drop, syncp);
+ ena_safe_update_stat(&stats->xdp_pass, &iter->pass, syncp);
+ ena_safe_update_stat(&stats->xdp_tx, &iter->tx, syncp);
+ ena_safe_update_stat(&stats->xdp_redirect, &iter->redirect,
+ syncp);
+ ena_safe_update_stat(&stats->xdp_aborted, &iter->aborted,
+ syncp);
+ ena_safe_update_stat(&stats->xdp_invalid, &iter->invalid,
+ syncp);
+ }
+}
+
static int ena_get_link_ksettings(struct net_device *netdev,
struct ethtool_link_ksettings *link_ksettings)
{
@@ -916,6 +948,8 @@ static const struct ethtool_ops ena_ethtool_ops = {
.get_tunable = ena_get_tunable,
.set_tunable = ena_set_tunable,
.get_ts_info = ethtool_op_get_ts_info,
+ .get_std_stats_channels = ena_get_std_stats_channels,
+ .get_xdp_stats = ena_get_xdp_stats,
};
void ena_set_ethtool_ops(struct net_device *netdev)
--
2.31.1
This driver has 6 per-channel XDP counters. Convert them to 5
standard XDP stats (redirect_sg and redirect_failures go under
the same redirect_errors).
As this driver theoretically can have different numbers of RX
and TX rings, collect statistics separately for each direction.
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
---
.../ethernet/freescale/enetc/enetc_ethtool.c | 58 ++++++++++++++-----
1 file changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
index ebccaf02411c..1b94cb488850 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
@@ -192,18 +192,12 @@ static const struct {
static const char rx_ring_stats[][ETH_GSTRING_LEN] = {
"Rx ring %2d frames",
"Rx ring %2d alloc errors",
- "Rx ring %2d XDP drops",
"Rx ring %2d recycles",
"Rx ring %2d recycle failures",
- "Rx ring %2d redirects",
- "Rx ring %2d redirect failures",
- "Rx ring %2d redirect S/G",
};
static const char tx_ring_stats[][ETH_GSTRING_LEN] = {
"Tx ring %2d frames",
- "Tx ring %2d XDP frames",
- "Tx ring %2d XDP drops",
};
static int enetc_get_sset_count(struct net_device *ndev, int sset)
@@ -275,21 +269,14 @@ static void enetc_get_ethtool_stats(struct net_device *ndev,
for (i = 0; i < ARRAY_SIZE(enetc_si_counters); i++)
data[o++] = enetc_rd64(hw, enetc_si_counters[i].reg);
- for (i = 0; i < priv->num_tx_rings; i++) {
+ for (i = 0; i < priv->num_tx_rings; i++)
data[o++] = priv->tx_ring[i]->stats.packets;
- data[o++] = priv->tx_ring[i]->stats.xdp_tx;
- data[o++] = priv->tx_ring[i]->stats.xdp_tx_drops;
- }
for (i = 0; i < priv->num_rx_rings; i++) {
data[o++] = priv->rx_ring[i]->stats.packets;
data[o++] = priv->rx_ring[i]->stats.rx_alloc_errs;
- data[o++] = priv->rx_ring[i]->stats.xdp_drops;
data[o++] = priv->rx_ring[i]->stats.recycles;
data[o++] = priv->rx_ring[i]->stats.recycle_failures;
- data[o++] = priv->rx_ring[i]->stats.xdp_redirect;
- data[o++] = priv->rx_ring[i]->stats.xdp_redirect_failures;
- data[o++] = priv->rx_ring[i]->stats.xdp_redirect_sg;
}
if (!enetc_si_is_pf(priv->si))
@@ -299,6 +286,45 @@ static void enetc_get_ethtool_stats(struct net_device *ndev,
data[o++] = enetc_port_rd(hw, enetc_port_counters[i].reg);
}
+static int enetc_get_std_stats_channels(struct net_device *ndev, u32 sset)
+{
+ const struct enetc_ndev_priv *priv = netdev_priv(ndev);
+
+ switch (sset) {
+ case ETH_SS_STATS_XDP:
+ return max(priv->num_rx_rings, priv->num_tx_rings);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void enetc_get_xdp_stats(struct net_device *ndev,
+ struct ethtool_xdp_stats *xdp_stats)
+{
+ const struct enetc_ndev_priv *priv = netdev_priv(ndev);
+ const struct enetc_ring_stats *stats;
+ struct ethtool_xdp_stats *iter;
+ u32 i;
+
+ for (i = 0; i < priv->num_tx_rings; i++) {
+ stats = &priv->tx_ring[i]->stats;
+ iter = xdp_stats + i;
+
+ iter->tx = stats->xdp_tx;
+ iter->tx_errors = stats->xdp_tx_drops;
+ }
+
+ for (i = 0; i < priv->num_rx_rings; i++) {
+ stats = &priv->rx_ring[i]->stats;
+ iter = xdp_stats + i;
+
+ iter->drop = stats->xdp_drops;
+ iter->redirect = stats->xdp_redirect;
+ iter->redirect_errors = stats->xdp_redirect_failures;
+ iter->redirect_errors += stats->xdp_redirect_sg;
+ }
+}
+
#define ENETC_RSSHASH_L3 (RXH_L2DA | RXH_VLAN | RXH_L3_PROTO | RXH_IP_SRC | \
RXH_IP_DST)
#define ENETC_RSSHASH_L4 (ENETC_RSSHASH_L3 | RXH_L4_B_0_1 | RXH_L4_B_2_3)
@@ -772,6 +798,8 @@ static const struct ethtool_ops enetc_pf_ethtool_ops = {
.set_wol = enetc_set_wol,
.get_pauseparam = enetc_get_pauseparam,
.set_pauseparam = enetc_set_pauseparam,
+ .get_std_stats_channels = enetc_get_std_stats_channels,
+ .get_xdp_stats = enetc_get_xdp_stats,
};
static const struct ethtool_ops enetc_vf_ethtool_ops = {
@@ -793,6 +821,8 @@ static const struct ethtool_ops enetc_vf_ethtool_ops = {
.set_coalesce = enetc_set_coalesce,
.get_link = ethtool_op_get_link,
.get_ts_info = enetc_get_ts_info,
+ .get_std_stats_channels = enetc_get_std_stats_channels,
+ .get_xdp_stats = enetc_get_xdp_stats,
};
void enetc_set_ethtool_ops(struct net_device *ndev)
--
2.31.1
veth has 7 per-channel XDP counters which could be aligned to the
standard XDP stats. Peer stats are here too as well, with the
original channel numbering logics.
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
---
drivers/net/veth.c | 91 +++++++++++++++++++++++++++++-----------------
1 file changed, 57 insertions(+), 34 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index d7e95f09e19d..79614d8e88bd 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -94,22 +94,10 @@ struct veth_q_stat_desc {
static const struct veth_q_stat_desc veth_rq_stats_desc[] = {
{ "packets", VETH_RQ_STAT(packets) },
{ "bytes", VETH_RQ_STAT(bytes) },
- { "xdp_errors", VETH_RQ_STAT(xdp_errors) },
- { "xdp_redirect", VETH_RQ_STAT(xdp_redirect) },
- { "xdp_drops", VETH_RQ_STAT(xdp_drops) },
- { "xdp_tx", VETH_RQ_STAT(xdp_tx) },
- { "xdp_tx_errors", VETH_RQ_STAT(xdp_tx_err) },
};
#define VETH_RQ_STATS_LEN ARRAY_SIZE(veth_rq_stats_desc)
-static const struct veth_q_stat_desc veth_tq_stats_desc[] = {
- { "xdp_xmit", VETH_RQ_STAT(peer_tq_xdp_xmit) },
- { "xdp_xmit_drops", VETH_RQ_STAT(peer_tq_xdp_xmit_drops) },
-};
-
-#define VETH_TQ_STATS_LEN ARRAY_SIZE(veth_tq_stats_desc)
-
static struct {
const char string[ETH_GSTRING_LEN];
} ethtool_stats_keys[] = {
@@ -149,14 +137,6 @@ static void veth_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
p += ETH_GSTRING_LEN;
}
}
- for (i = 0; i < dev->real_num_tx_queues; i++) {
- for (j = 0; j < VETH_TQ_STATS_LEN; j++) {
- snprintf(p, ETH_GSTRING_LEN,
- "tx_queue_%u_%.18s",
- i, veth_tq_stats_desc[j].desc);
- p += ETH_GSTRING_LEN;
- }
- }
break;
}
}
@@ -166,8 +146,7 @@ static int veth_get_sset_count(struct net_device *dev, int sset)
switch (sset) {
case ETH_SS_STATS:
return ARRAY_SIZE(ethtool_stats_keys) +
- VETH_RQ_STATS_LEN * dev->real_num_rx_queues +
- VETH_TQ_STATS_LEN * dev->real_num_tx_queues;
+ VETH_RQ_STATS_LEN * dev->real_num_rx_queues;
default:
return -EOPNOTSUPP;
}
@@ -176,8 +155,8 @@ static int veth_get_sset_count(struct net_device *dev, int sset)
static void veth_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats, u64 *data)
{
- struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
- struct net_device *peer = rtnl_dereference(priv->peer);
+ const struct veth_priv *priv = netdev_priv(dev);
+ const struct net_device *peer = rtnl_dereference(priv->peer);
int i, j, idx;
data[0] = peer ? peer->ifindex : 0;
@@ -197,25 +176,67 @@ static void veth_get_ethtool_stats(struct net_device *dev,
} while (u64_stats_fetch_retry_irq(&rq_stats->syncp, start));
idx += VETH_RQ_STATS_LEN;
}
+}
+
+static int veth_get_std_stats_channels(struct net_device *dev, u32 sset)
+{
+ switch (sset) {
+ case ETH_SS_STATS_XDP:
+ return max(dev->real_num_rx_queues, dev->real_num_tx_queues);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void veth_get_xdp_stats(struct net_device *dev,
+ struct ethtool_xdp_stats *xdp_stats)
+{
+ const struct veth_priv *priv = netdev_priv(dev);
+ const struct net_device *peer = rtnl_dereference(priv->peer);
+ const struct veth_rq_stats *rq_stats;
+ struct ethtool_xdp_stats *iter;
+ u64 xmit, xmit_drops;
+ u32 i, start;
+
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ rq_stats = &priv->rq[i].stats;
+ iter = xdp_stats + i;
+
+ do {
+ start = u64_stats_fetch_begin_irq(&rq_stats->syncp);
+
+ iter->errors = rq_stats->vs.xdp_errors;
+ iter->redirect = rq_stats->vs.xdp_redirect;
+ iter->drop = rq_stats->vs.xdp_drops;
+ iter->tx = rq_stats->vs.xdp_tx;
+ iter->tx_errors = rq_stats->vs.xdp_tx_err;
+ } while (u64_stats_fetch_retry_irq(&rq_stats->syncp, start));
+ }
if (!peer)
return;
- rcv_priv = netdev_priv(peer);
+ for (i = 0; i < dev->real_num_tx_queues; i++) {
+ iter = xdp_stats + i;
+ iter->xmit = 0;
+ iter->xmit_drops = 0;
+ }
+
+ priv = netdev_priv(peer);
+
for (i = 0; i < peer->real_num_rx_queues; i++) {
- const struct veth_rq_stats *rq_stats = &rcv_priv->rq[i].stats;
- const void *base = (void *)&rq_stats->vs;
- unsigned int start, tx_idx = idx;
- size_t offset;
+ rq_stats = &priv->rq[i].stats;
+ iter = xdp_stats + (i % dev->real_num_tx_queues);
- tx_idx += (i % dev->real_num_tx_queues) * VETH_TQ_STATS_LEN;
do {
start = u64_stats_fetch_begin_irq(&rq_stats->syncp);
- for (j = 0; j < VETH_TQ_STATS_LEN; j++) {
- offset = veth_tq_stats_desc[j].offset;
- data[tx_idx + j] += *(u64 *)(base + offset);
- }
+
+ xmit = rq_stats->vs.peer_tq_xdp_xmit;
+ xmit_drops = rq_stats->vs.peer_tq_xdp_xmit_drops;
} while (u64_stats_fetch_retry_irq(&rq_stats->syncp, start));
+
+ iter->xmit += xmit;
+ iter->xmit_drops += xmit_drops;
}
}
@@ -241,6 +262,8 @@ static const struct ethtool_ops veth_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
.get_channels = veth_get_channels,
.set_channels = veth_set_channels,
+ .get_std_stats_channels = veth_get_std_stats_channels,
+ .get_xdp_stats = veth_get_xdp_stats,
};
/* general routines */
--
2.31.1
It's pretty confusing to have just one field for tracking both
XDP_DROP cases and various errors which lead to the frame being
dropped.
Add a new field, xdp_errors, to separate error paths, and leave
xdp_drops purely for counting frames with the XDP_DROP verdict.
Signed-off-by: Alexander Lobakin <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
---
drivers/net/virtio_net.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8cbb4651ed75..acad099006cd 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -91,6 +91,7 @@ struct virtnet_rq_stats {
u64 xdp_tx;
u64 xdp_redirects;
u64 xdp_drops;
+ u64 xdp_errors;
u64 kicks;
};
@@ -113,6 +114,7 @@ static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = {
{ "xdp_tx", VIRTNET_RQ_STAT(xdp_tx) },
{ "xdp_redirects", VIRTNET_RQ_STAT(xdp_redirects) },
{ "xdp_drops", VIRTNET_RQ_STAT(xdp_drops) },
+ { "xdp_errors", VIRTNET_RQ_STAT(xdp_errors) },
{ "kicks", VIRTNET_RQ_STAT(kicks) },
};
@@ -804,7 +806,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
trace_xdp_exception(vi->dev, xdp_prog, act);
goto err_xdp;
case XDP_DROP:
- goto err_xdp;
+ stats->xdp_drops++;
+ goto xdp_drop;
}
}
rcu_read_unlock();
@@ -828,8 +831,9 @@ static struct sk_buff *receive_small(struct net_device *dev,
return skb;
err_xdp:
+ stats->xdp_errors++;
+xdp_drop:
rcu_read_unlock();
- stats->xdp_drops++;
err_len:
stats->drops++;
put_page(page);
@@ -1012,7 +1016,12 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
case XDP_DROP:
if (unlikely(xdp_page != page))
__free_pages(xdp_page, 0);
- goto err_xdp;
+
+ if (unlikely(act != XDP_DROP))
+ goto err_xdp;
+
+ stats->xdp_drops++;
+ goto xdp_drop;
}
}
rcu_read_unlock();
@@ -1081,8 +1090,9 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
return head_skb;
err_xdp:
+ stats->xdp_errors++;
+xdp_drop:
rcu_read_unlock();
- stats->xdp_drops++;
err_skb:
put_page(page);
while (num_buf-- > 1) {
--
2.31.1
Alexander Lobakin <[email protected]> writes:
>
>
>
> Its 6 XDP per-channel counters align just fine with the standard
> stats.
> Drop them from the custom Ethtool statistics and expose to the
> standard stats infra instead.
>
> Signed-off-by: Alexander Lobakin <[email protected]>
> Reviewed-by: Jesse Brandeburg <[email protected]>
> ---
> drivers/net/ethernet/amazon/ena/ena_ethtool.c | 46
> ++++++++++++++++---
> 1 file changed, 40 insertions(+), 6 deletions(-)
Hi,
thanks for making this patch. I like the idea of splitting stats
into a per-queue basis
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> index 851a198cec82..1b6563641575 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> @@ -90,12 +90,6 @@ static const struct ena_stats
> ena_stats_rx_strings[] = {
> ENA_STAT_RX_ENTRY(bad_req_id),
> ENA_STAT_RX_ENTRY(empty_rx_ring),
> ENA_STAT_RX_ENTRY(csum_unchecked),
> - ENA_STAT_RX_ENTRY(xdp_aborted),
> - ENA_STAT_RX_ENTRY(xdp_drop),
> - ENA_STAT_RX_ENTRY(xdp_pass),
> - ENA_STAT_RX_ENTRY(xdp_tx),
> - ENA_STAT_RX_ENTRY(xdp_invalid),
> - ENA_STAT_RX_ENTRY(xdp_redirect),
>
The ena_stats_rx_strings array is (indirectly) accessed through
ena_get_stats() function which is used for both fetching ethtool
stats and
for sharing the stats with the device in case of an error (through
ena_dump_stats_ex() function).
The latter use is broken by removing the XDP specific stats from
ena_stats_rx_strings array.
I can submit an adaptation for the new system later (similar to
mlx5) if you prefer
thanks,
Shay
> };
>
> static const struct ena_stats ena_stats_ena_com_strings[] = {
> @@ -324,6 +318,44 @@ static void ena_get_ethtool_strings(struct
> net_device *netdev,
> }
> }
>
> +static int ena_get_std_stats_channels(struct net_device
> *netdev, u32 sset)
> +{
> + const struct ena_adapter *adapter = netdev_priv(netdev);
> +
> + switch (sset) {
> + case ETH_SS_STATS_XDP:
> + return adapter->num_io_queues;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static void ena_get_xdp_stats(struct net_device *netdev,
> + struct ethtool_xdp_stats
> *xdp_stats)
> +{
> + const struct ena_adapter *adapter = netdev_priv(netdev);
> + const struct u64_stats_sync *syncp;
> + const struct ena_stats_rx *stats;
> + struct ethtool_xdp_stats *iter;
> + u32 i;
> +
...
> {
> @@ -916,6 +948,8 @@ static const struct ethtool_ops
> ena_ethtool_ops = {
> .get_tunable = ena_get_tunable,
> .set_tunable = ena_set_tunable,
> .get_ts_info = ethtool_op_get_ts_info,
> + .get_std_stats_channels = ena_get_std_stats_channels,
> + .get_xdp_stats = ena_get_xdp_stats,
> };
>
> void ena_set_ethtool_ops(struct net_device *netdev)
From: Shay Agroskin <[email protected]>
Date: Wed, 4 Aug 2021 16:04:59 +0300
> Alexander Lobakin <[email protected]> writes:
>
> >
> >
> >
> > Its 6 XDP per-channel counters align just fine with the standard
> > stats.
> > Drop them from the custom Ethtool statistics and expose to the
> > standard stats infra instead.
> >
> > Signed-off-by: Alexander Lobakin <[email protected]>
> > Reviewed-by: Jesse Brandeburg <[email protected]>
> > ---
> > drivers/net/ethernet/amazon/ena/ena_ethtool.c | 46
> > ++++++++++++++++---
> > 1 file changed, 40 insertions(+), 6 deletions(-)
>
> Hi,
> thanks for making this patch. I like the idea of splitting stats
> into a per-queue basis
>
> >
> > diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> > b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> > index 851a198cec82..1b6563641575 100644
> > --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> > +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> > @@ -90,12 +90,6 @@ static const struct ena_stats
> > ena_stats_rx_strings[] = {
> > ENA_STAT_RX_ENTRY(bad_req_id),
> > ENA_STAT_RX_ENTRY(empty_rx_ring),
> > ENA_STAT_RX_ENTRY(csum_unchecked),
> > - ENA_STAT_RX_ENTRY(xdp_aborted),
> > - ENA_STAT_RX_ENTRY(xdp_drop),
> > - ENA_STAT_RX_ENTRY(xdp_pass),
> > - ENA_STAT_RX_ENTRY(xdp_tx),
> > - ENA_STAT_RX_ENTRY(xdp_invalid),
> > - ENA_STAT_RX_ENTRY(xdp_redirect),
> >
>
> The ena_stats_rx_strings array is (indirectly) accessed through
> ena_get_stats() function which is used for both fetching ethtool
> stats and
> for sharing the stats with the device in case of an error (through
> ena_dump_stats_ex() function).
>
> The latter use is broken by removing the XDP specific stats from
> ena_stats_rx_strings array.
>
> I can submit an adaptation for the new system later (similar to
> mlx5) if you prefer
Feel free to either do that (I'll exclude this patch from that
series then) or you can give me some little tips or examples or
anything on how to improve this one, so ena would stay converted.
Both ways are fine for me.
> thanks,
> Shay
Thanks,
Al
> > };
> >
> > static const struct ena_stats ena_stats_ena_com_strings[] = {
> > @@ -324,6 +318,44 @@ static void ena_get_ethtool_strings(struct
> > net_device *netdev,
> > }
> > }
> >
> > +static int ena_get_std_stats_channels(struct net_device
> > *netdev, u32 sset)
> > +{
> > + const struct ena_adapter *adapter = netdev_priv(netdev);
> > +
> > + switch (sset) {
> > + case ETH_SS_STATS_XDP:
> > + return adapter->num_io_queues;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +}
> > +
> > +static void ena_get_xdp_stats(struct net_device *netdev,
> > + struct ethtool_xdp_stats
> > *xdp_stats)
> > +{
> > + const struct ena_adapter *adapter = netdev_priv(netdev);
> > + const struct u64_stats_sync *syncp;
> > + const struct ena_stats_rx *stats;
> > + struct ethtool_xdp_stats *iter;
> > + u32 i;
> > +
> ...
> > {
> > @@ -916,6 +948,8 @@ static const struct ethtool_ops
> > ena_ethtool_ops = {
> > .get_tunable = ena_get_tunable,
> > .set_tunable = ena_set_tunable,
> > .get_ts_info = ethtool_op_get_ts_info,
> > + .get_std_stats_channels = ena_get_std_stats_channels,
> > + .get_xdp_stats = ena_get_xdp_stats,
> > };
> >
> > void ena_set_ethtool_ops(struct net_device *netdev)
>
>