* [PATCH net-next v2 0/2] eth: Add basic ethtool support for fbnic
@ 2024-08-27 20:59 Mohsin Bashir
2024-08-27 20:59 ` [PATCH net-next v2 1/2] eth: fbnic: Add " Mohsin Bashir
2024-08-27 20:59 ` [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats Mohsin Bashir
0 siblings, 2 replies; 11+ messages in thread
From: Mohsin Bashir @ 2024-08-27 20:59 UTC (permalink / raw)
To: netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato, mohsin.bashr
This patch series adds basic ethtool support for fbnic. Specifically,
the two patches focus on the following two features respectively:
1: Enable 'ethtool -i <dev>' to provide driver, firmware and bus information.
2: Provide mac group stats.
Changes since v1:
- Update the emptiness check for firmware version commit string
- Rebase the patch series to the latest
- Add cover letter
v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
Thanks, Mohsin Bashir
-------
drivers/net/ethernet/meta/fbnic/Makefile | 2 +
drivers/net/ethernet/meta/fbnic/fbnic.h | 7 ++
drivers/net/ethernet/meta/fbnic/fbnic_csr.h | 37 +++++++++
.../net/ethernet/meta/fbnic/fbnic_ethtool.c | 75 +++++++++++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 13 ++++
drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 6 +-
.../net/ethernet/meta/fbnic/fbnic_hw_stats.c | 27 +++++++
.../net/ethernet/meta/fbnic/fbnic_hw_stats.h | 40 ++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 50 +++++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 3 +
.../net/ethernet/meta/fbnic/fbnic_netdev.c | 2 +
.../net/ethernet/meta/fbnic/fbnic_netdev.h | 1 +
12 files changed, 260 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
--
2.43.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 1/2] eth: fbnic: Add ethtool support for fbnic
2024-08-27 20:59 [PATCH net-next v2 0/2] eth: Add basic ethtool support for fbnic Mohsin Bashir
@ 2024-08-27 20:59 ` Mohsin Bashir
2024-08-28 6:29 ` Michal Swiatkowski
2024-08-27 20:59 ` [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats Mohsin Bashir
1 sibling, 1 reply; 11+ messages in thread
From: Mohsin Bashir @ 2024-08-27 20:59 UTC (permalink / raw)
To: netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato, mohsin.bashr
Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
provides firmware version information while the driver name and bus
information is provided by ethtool_get_drvinfo().
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
v2:
- Update the emptiness check for firmware version commit string
- Rebase to the latest
v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
---
drivers/net/ethernet/meta/fbnic/Makefile | 1 +
drivers/net/ethernet/meta/fbnic/fbnic.h | 3 +++
.../net/ethernet/meta/fbnic/fbnic_ethtool.c | 26 +++++++++++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 13 ++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 6 ++---
.../net/ethernet/meta/fbnic/fbnic_netdev.c | 2 ++
.../net/ethernet/meta/fbnic/fbnic_netdev.h | 1 +
7 files changed, 49 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
index 9373b558fdc9..37cfc34a5118 100644
--- a/drivers/net/ethernet/meta/fbnic/Makefile
+++ b/drivers/net/ethernet/meta/fbnic/Makefile
@@ -8,6 +8,7 @@
obj-$(CONFIG_FBNIC) += fbnic.o
fbnic-y := fbnic_devlink.o \
+ fbnic_ethtool.o \
fbnic_fw.o \
fbnic_irq.o \
fbnic_mac.o \
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index ad2689bfd6cb..28d970f81bfc 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -132,6 +132,9 @@ void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
void fbnic_free_irqs(struct fbnic_dev *fbd);
int fbnic_alloc_irqs(struct fbnic_dev *fbd);
+void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
+ const size_t str_sz);
+
enum fbnic_boards {
fbnic_board_asic
};
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
new file mode 100644
index 000000000000..7064dfc9f5b0
--- /dev/null
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -0,0 +1,26 @@
+#include <linux/ethtool.h>
+#include <linux/netdevice.h>
+#include <linux/pci.h>
+
+#include "fbnic.h"
+#include "fbnic_netdev.h"
+#include "fbnic_tlv.h"
+
+static void
+fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
+{
+ struct fbnic_net *fbn = netdev_priv(netdev);
+ struct fbnic_dev *fbd = fbn->fbd;
+
+ fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
+ sizeof(drvinfo->fw_version));
+}
+
+static const struct ethtool_ops fbnic_ethtool_ops = {
+ .get_drvinfo = fbnic_get_drvinfo,
+};
+
+void fbnic_set_ethtool_ops(struct net_device *dev)
+{
+ dev->ethtool_ops = &fbnic_ethtool_ops;
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
index 0c6e1b4c119b..8f7a2a19ddf8 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
@@ -789,3 +789,16 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
} while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
}
+
+void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
+ const size_t str_sz)
+{
+ struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
+ const char *delim = "";
+
+ if (mgmt->commit[0])
+ delim = "_";
+
+ fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
+ fw_version, str_sz);
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
index c65bca613665..221faf8c6756 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
@@ -53,10 +53,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
-#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str) \
+#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
do { \
const u32 __rev_id = _rev_id; \
- snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s", \
+ snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s", \
FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id), \
FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id), \
FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id), \
@@ -65,7 +65,7 @@ do { \
} while (0)
#define fbnic_mk_fw_ver_str(_rev_id, _str) \
- fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
+ fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
#define FW_HEARTBEAT_PERIOD (10 * HZ)
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index 571374361259..a400616a24d4 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -521,6 +521,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
netdev->netdev_ops = &fbnic_netdev_ops;
netdev->stat_ops = &fbnic_stat_ops;
+ fbnic_set_ethtool_ops(netdev);
+
fbn = netdev_priv(netdev);
fbn->netdev = netdev;
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
index 60199e634468..6c27da09a612 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
@@ -58,6 +58,7 @@ int fbnic_netdev_register(struct net_device *netdev);
void fbnic_netdev_unregister(struct net_device *netdev);
void fbnic_reset_queues(struct fbnic_net *fbn,
unsigned int tx, unsigned int rx);
+void fbnic_set_ethtool_ops(struct net_device *dev);
void __fbnic_set_rx_mode(struct net_device *netdev);
void fbnic_clear_rx_mode(struct net_device *netdev);
--
2.43.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats
2024-08-27 20:59 [PATCH net-next v2 0/2] eth: Add basic ethtool support for fbnic Mohsin Bashir
2024-08-27 20:59 ` [PATCH net-next v2 1/2] eth: fbnic: Add " Mohsin Bashir
@ 2024-08-27 20:59 ` Mohsin Bashir
2024-08-28 0:30 ` Nelson, Shannon
2024-08-28 14:37 ` Simon Horman
1 sibling, 2 replies; 11+ messages in thread
From: Mohsin Bashir @ 2024-08-27 20:59 UTC (permalink / raw)
To: netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato, mohsin.bashr
Add support for group stats for mac. The fbnic_set_counter helps prevent
overriding the default values for counters which are not collected by the device.
The 'reset' flag in 'get_eth_mac_stats' allows choosing between
resetting the counter to recent most value or fecthing the aggregate
values of counters. This is important to cater for cases such as
device reset.
The 'fbnic_stat_rd64' read 64b stats counters in a consistent fashion using
high-low-high approach. This allows to isolate cases where counter is
wrapped between the reads.
Command: ethtool -S eth0 --groups eth-mac
Example Output:
eth-mac-FramesTransmittedOK: 421644
eth-mac-FramesReceivedOK: 3849708
eth-mac-FrameCheckSequenceErrors: 0
eth-mac-AlignmentErrors: 0
eth-mac-OctetsTransmittedOK: 64799060
eth-mac-FramesLostDueToIntMACXmitError: 0
eth-mac-OctetsReceivedOK: 5134513531
eth-mac-FramesLostDueToIntMACRcvError: 0
eth-mac-MulticastFramesXmittedOK: 568
eth-mac-BroadcastFramesXmittedOK: 454
eth-mac-MulticastFramesReceivedOK: 276106
eth-mac-BroadcastFramesReceivedOK: 26119
eth-mac-FrameTooLongErrors: 0
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
v2: Rebase to the latest
v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
---
drivers/net/ethernet/meta/fbnic/Makefile | 1 +
drivers/net/ethernet/meta/fbnic/fbnic.h | 4 ++
drivers/net/ethernet/meta/fbnic/fbnic_csr.h | 37 ++++++++++++++
.../net/ethernet/meta/fbnic/fbnic_ethtool.c | 49 ++++++++++++++++++
.../net/ethernet/meta/fbnic/fbnic_hw_stats.c | 27 ++++++++++
.../net/ethernet/meta/fbnic/fbnic_hw_stats.h | 40 +++++++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 50 +++++++++++++++++++
drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 3 ++
8 files changed, 211 insertions(+)
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
index 37cfc34a5118..ed4533a73c57 100644
--- a/drivers/net/ethernet/meta/fbnic/Makefile
+++ b/drivers/net/ethernet/meta/fbnic/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_FBNIC) += fbnic.o
fbnic-y := fbnic_devlink.o \
fbnic_ethtool.o \
fbnic_fw.o \
+ fbnic_hw_stats.o \
fbnic_irq.o \
fbnic_mac.o \
fbnic_netdev.o \
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index 28d970f81bfc..0f9e8d79461c 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -11,6 +11,7 @@
#include "fbnic_csr.h"
#include "fbnic_fw.h"
+#include "fbnic_hw_stats.h"
#include "fbnic_mac.h"
#include "fbnic_rpc.h"
@@ -47,6 +48,9 @@ struct fbnic_dev {
/* Number of TCQs/RCQs available on hardware */
u16 max_num_queues;
+
+ /* Local copy of hardware statistics */
+ struct fbnic_hw_stats hw_stats;
};
/* Reserve entry 0 in the MSI-X "others" array until we have filled all
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
index a64360de0552..21db509acbc1 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
@@ -660,6 +660,43 @@ enum {
#define FBNIC_SIG_PCS_INTR_MASK 0x11816 /* 0x46058 */
#define FBNIC_CSR_END_SIG 0x1184e /* CSR section delimiter */
+#define FBNIC_CSR_START_MAC_STAT 0x11a00
+#define FBNIC_MAC_STAT_RX_BYTE_COUNT_L 0x11a08 /* 0x46820 */
+#define FBNIC_MAC_STAT_RX_BYTE_COUNT_H 0x11a09 /* 0x46824 */
+#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_L \
+ 0x11a0a /* 0x46828 */
+#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_H \
+ 0x11a0b /* 0x4682c */
+#define FBNIC_MAC_STAT_RX_TOOLONG_L 0x11a0e /* 0x46838 */
+#define FBNIC_MAC_STAT_RX_TOOLONG_H 0x11a0f /* 0x4683c */
+#define FBNIC_MAC_STAT_RX_RECEIVED_OK_L \
+ 0x11a12 /* 0x46848 */
+#define FBNIC_MAC_STAT_RX_RECEIVED_OK_H \
+ 0x11a13 /* 0x4684c */
+#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_L \
+ 0x11a14 /* 0x46850 */
+#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_H \
+ 0x11a15 /* 0x46854 */
+#define FBNIC_MAC_STAT_RX_IFINERRORS_L 0x11a18 /* 0x46860 */
+#define FBNIC_MAC_STAT_RX_IFINERRORS_H 0x11a19 /* 0x46864 */
+#define FBNIC_MAC_STAT_RX_MULTICAST_L 0x11a1c /* 0x46870 */
+#define FBNIC_MAC_STAT_RX_MULTICAST_H 0x11a1d /* 0x46874 */
+#define FBNIC_MAC_STAT_RX_BROADCAST_L 0x11a1e /* 0x46878 */
+#define FBNIC_MAC_STAT_RX_BROADCAST_H 0x11a1f /* 0x4687c */
+#define FBNIC_MAC_STAT_TX_BYTE_COUNT_L 0x11a3e /* 0x468f8 */
+#define FBNIC_MAC_STAT_TX_BYTE_COUNT_H 0x11a3f /* 0x468fc */
+#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_L \
+ 0x11a42 /* 0x46908 */
+#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_H \
+ 0x11a43 /* 0x4690c */
+#define FBNIC_MAC_STAT_TX_IFOUTERRORS_L \
+ 0x11a46 /* 0x46918 */
+#define FBNIC_MAC_STAT_TX_IFOUTERRORS_H \
+ 0x11a47 /* 0x4691c */
+#define FBNIC_MAC_STAT_TX_MULTICAST_L 0x11a4a /* 0x46928 */
+#define FBNIC_MAC_STAT_TX_MULTICAST_H 0x11a4b /* 0x4692c */
+#define FBNIC_MAC_STAT_TX_BROADCAST_L 0x11a4c /* 0x46930 */
+#define FBNIC_MAC_STAT_TX_BROADCAST_H 0x11a4d /* 0x46934 */
/* PUL User Registers */
#define FBNIC_CSR_START_PUL_USER 0x31000 /* CSR section delimiter */
#define FBNIC_PUL_OB_TLP_HDR_AW_CFG 0x3103d /* 0xc40f4 */
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
index 7064dfc9f5b0..5d980e178941 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -16,8 +16,57 @@ fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
sizeof(drvinfo->fw_version));
}
+static void fbnic_set_counter(u64 *stat, struct fbnic_stat_counter *counter)
+{
+ if (counter->reported)
+ *stat = counter->value;
+}
+
+static void
+fbnic_get_eth_mac_stats(struct net_device *netdev,
+ struct ethtool_eth_mac_stats *eth_mac_stats)
+{
+ struct fbnic_net *fbn = netdev_priv(netdev);
+ struct fbnic_mac_stats *mac_stats;
+ struct fbnic_dev *fbd = fbn->fbd;
+ const struct fbnic_mac *mac;
+
+ mac_stats = &fbd->hw_stats.mac;
+ mac = fbd->mac;
+
+ mac->get_eth_mac_stats(fbd, false, &mac_stats->eth_mac);
+
+ fbnic_set_counter(ð_mac_stats->FramesTransmittedOK,
+ &mac_stats->eth_mac.FramesTransmittedOK);
+ fbnic_set_counter(ð_mac_stats->FramesReceivedOK,
+ &mac_stats->eth_mac.FramesReceivedOK);
+ fbnic_set_counter(ð_mac_stats->FrameCheckSequenceErrors,
+ &mac_stats->eth_mac.FrameCheckSequenceErrors);
+ fbnic_set_counter(ð_mac_stats->AlignmentErrors,
+ &mac_stats->eth_mac.AlignmentErrors);
+ fbnic_set_counter(ð_mac_stats->OctetsTransmittedOK,
+ &mac_stats->eth_mac.OctetsTransmittedOK);
+ fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACXmitError,
+ &mac_stats->eth_mac.FramesLostDueToIntMACXmitError);
+ fbnic_set_counter(ð_mac_stats->OctetsReceivedOK,
+ &mac_stats->eth_mac.OctetsReceivedOK);
+ fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACRcvError,
+ &mac_stats->eth_mac.FramesLostDueToIntMACRcvError);
+ fbnic_set_counter(ð_mac_stats->MulticastFramesXmittedOK,
+ &mac_stats->eth_mac.MulticastFramesXmittedOK);
+ fbnic_set_counter(ð_mac_stats->BroadcastFramesXmittedOK,
+ &mac_stats->eth_mac.BroadcastFramesXmittedOK);
+ fbnic_set_counter(ð_mac_stats->MulticastFramesReceivedOK,
+ &mac_stats->eth_mac.MulticastFramesReceivedOK);
+ fbnic_set_counter(ð_mac_stats->BroadcastFramesReceivedOK,
+ &mac_stats->eth_mac.BroadcastFramesReceivedOK);
+ fbnic_set_counter(ð_mac_stats->FrameTooLongErrors,
+ &mac_stats->eth_mac.FrameTooLongErrors);
+}
+
static const struct ethtool_ops fbnic_ethtool_ops = {
.get_drvinfo = fbnic_get_drvinfo,
+ .get_eth_mac_stats = fbnic_get_eth_mac_stats,
};
void fbnic_set_ethtool_ops(struct net_device *dev)
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
new file mode 100644
index 000000000000..a0acc7606aa1
--- /dev/null
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
@@ -0,0 +1,27 @@
+#include "fbnic.h"
+
+u64 fbnic_stat_rd64(struct fbnic_dev *fbd, u32 reg, u32 offset)
+{
+ u32 prev_upper, upper, lower, diff;
+
+ prev_upper = rd32(fbd, reg + offset);
+ lower = rd32(fbd, reg);
+ upper = rd32(fbd, reg + offset);
+
+ diff = upper - prev_upper;
+ if (!diff)
+ return ((u64)upper << 32) | lower;
+
+ if (diff > 1)
+ dev_warn_once(fbd->dev,
+ "Stats inconsistent, upper 32b of %#010x updating too quickly\n",
+ reg * 4);
+
+ /* Return only the upper bits as we cannot guarantee
+ * the accuracy of the lower bits. We will add them in
+ * when the counter slows down enough that we can get
+ * a snapshot with both upper values being the same
+ * between reads.
+ */
+ return ((u64)upper << 32);
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
new file mode 100644
index 000000000000..30348904b510
--- /dev/null
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
@@ -0,0 +1,40 @@
+#include <linux/ethtool.h>
+
+#include "fbnic_csr.h"
+
+struct fbnic_stat_counter {
+ u64 value;
+ union {
+ u32 old_reg_value_32;
+ u64 old_reg_value_64;
+ } u;
+ bool reported;
+};
+
+struct fbnic_eth_mac_stats {
+ struct fbnic_stat_counter FramesTransmittedOK;
+ struct fbnic_stat_counter FramesReceivedOK;
+ struct fbnic_stat_counter FrameCheckSequenceErrors;
+ struct fbnic_stat_counter AlignmentErrors;
+ struct fbnic_stat_counter OctetsTransmittedOK;
+ struct fbnic_stat_counter FramesLostDueToIntMACXmitError;
+ struct fbnic_stat_counter OctetsReceivedOK;
+ struct fbnic_stat_counter FramesLostDueToIntMACRcvError;
+ struct fbnic_stat_counter MulticastFramesXmittedOK;
+ struct fbnic_stat_counter BroadcastFramesXmittedOK;
+ struct fbnic_stat_counter MulticastFramesReceivedOK;
+ struct fbnic_stat_counter BroadcastFramesReceivedOK;
+ struct fbnic_stat_counter FrameTooLongErrors;
+};
+
+struct fbnic_mac_stats {
+ struct fbnic_eth_mac_stats eth_mac;
+};
+
+struct fbnic_hw_stats {
+ struct fbnic_mac_stats mac;
+};
+
+u64 fbnic_stat_rd64(struct fbnic_dev *fbd, u32 reg, u32 offset);
+
+void fbnic_get_hw_stats(struct fbnic_dev *fbd);
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
index 7920e7af82d9..7b654d0a6dac 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
@@ -403,6 +403,21 @@ static void fbnic_mac_init_regs(struct fbnic_dev *fbd)
fbnic_mac_init_txb(fbd);
}
+static void __fbnic_mac_stat_rd64(struct fbnic_dev *fbd, bool reset, u32 reg,
+ struct fbnic_stat_counter *stat)
+{
+ u64 new_reg_value;
+
+ new_reg_value = fbnic_stat_rd64(fbd, reg, 1);
+ if (!reset)
+ stat->value += new_reg_value - stat->u.old_reg_value_64;
+ stat->u.old_reg_value_64 = new_reg_value;
+ stat->reported = true;
+}
+
+#define fbnic_mac_stat_rd64(fbd, reset, __stat, __CSR) \
+ __fbnic_mac_stat_rd64(fbd, reset, FBNIC_##__CSR##_L, &(__stat))
+
static void fbnic_mac_tx_pause_config(struct fbnic_dev *fbd, bool tx_pause)
{
u32 rxb_pause_ctrl;
@@ -637,12 +652,47 @@ static void fbnic_mac_link_up_asic(struct fbnic_dev *fbd,
wr32(fbd, FBNIC_MAC_COMMAND_CONFIG, cmd_cfg);
}
+static void
+fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset,
+ struct fbnic_eth_mac_stats *mac_stats)
+{
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->OctetsReceivedOK,
+ MAC_STAT_RX_BYTE_COUNT);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->AlignmentErrors,
+ MAC_STAT_RX_ALIGN_ERROR);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->FrameTooLongErrors,
+ MAC_STAT_RX_TOOLONG);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->FramesReceivedOK,
+ MAC_STAT_RX_RECEIVED_OK);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->FrameCheckSequenceErrors,
+ MAC_STAT_RX_PACKET_BAD_FCS);
+ fbnic_mac_stat_rd64(fbd, reset,
+ mac_stats->FramesLostDueToIntMACRcvError,
+ MAC_STAT_RX_IFINERRORS);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->MulticastFramesReceivedOK,
+ MAC_STAT_RX_MULTICAST);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->BroadcastFramesReceivedOK,
+ MAC_STAT_RX_BROADCAST);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->OctetsTransmittedOK,
+ MAC_STAT_TX_BYTE_COUNT);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->FramesTransmittedOK,
+ MAC_STAT_TX_TRANSMITTED_OK);
+ fbnic_mac_stat_rd64(fbd, reset,
+ mac_stats->FramesLostDueToIntMACXmitError,
+ MAC_STAT_TX_IFOUTERRORS);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->MulticastFramesXmittedOK,
+ MAC_STAT_TX_MULTICAST);
+ fbnic_mac_stat_rd64(fbd, reset, mac_stats->BroadcastFramesXmittedOK,
+ MAC_STAT_TX_BROADCAST);
+}
+
static const struct fbnic_mac fbnic_mac_asic = {
.init_regs = fbnic_mac_init_regs,
.pcs_enable = fbnic_pcs_enable_asic,
.pcs_disable = fbnic_pcs_disable_asic,
.pcs_get_link = fbnic_pcs_get_link_asic,
.pcs_get_link_event = fbnic_pcs_get_link_event_asic,
+ .get_eth_mac_stats = fbnic_mac_get_eth_mac_stats,
.link_down = fbnic_mac_link_down_asic,
.link_up = fbnic_mac_link_up_asic,
};
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
index f53be6e6aef9..476239a9d381 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
@@ -78,6 +78,9 @@ struct fbnic_mac {
bool (*pcs_get_link)(struct fbnic_dev *fbd);
int (*pcs_get_link_event)(struct fbnic_dev *fbd);
+ void (*get_eth_mac_stats)(struct fbnic_dev *fbd, bool reset,
+ struct fbnic_eth_mac_stats *mac_stats);
+
void (*link_down)(struct fbnic_dev *fbd);
void (*link_up)(struct fbnic_dev *fbd, bool tx_pause, bool rx_pause);
};
--
2.43.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats
2024-08-27 20:59 ` [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats Mohsin Bashir
@ 2024-08-28 0:30 ` Nelson, Shannon
2024-08-28 16:41 ` Alexander H Duyck
2024-08-28 14:37 ` Simon Horman
1 sibling, 1 reply; 11+ messages in thread
From: Nelson, Shannon @ 2024-08-28 0:30 UTC (permalink / raw)
To: Mohsin Bashir, netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On 8/27/2024 1:59 PM, Mohsin Bashir wrote:
>
> Add support for group stats for mac. The fbnic_set_counter helps prevent
> overriding the default values for counters which are not collected by the device.
>
> The 'reset' flag in 'get_eth_mac_stats' allows choosing between
> resetting the counter to recent most value or fecthing the aggregate
> values of counters. This is important to cater for cases such as
> device reset.
>
> The 'fbnic_stat_rd64' read 64b stats counters in a consistent fashion using
> high-low-high approach. This allows to isolate cases where counter is
> wrapped between the reads.
>
> Command: ethtool -S eth0 --groups eth-mac
> Example Output:
> eth-mac-FramesTransmittedOK: 421644
> eth-mac-FramesReceivedOK: 3849708
> eth-mac-FrameCheckSequenceErrors: 0
> eth-mac-AlignmentErrors: 0
> eth-mac-OctetsTransmittedOK: 64799060
> eth-mac-FramesLostDueToIntMACXmitError: 0
> eth-mac-OctetsReceivedOK: 5134513531
> eth-mac-FramesLostDueToIntMACRcvError: 0
> eth-mac-MulticastFramesXmittedOK: 568
> eth-mac-BroadcastFramesXmittedOK: 454
> eth-mac-MulticastFramesReceivedOK: 276106
> eth-mac-BroadcastFramesReceivedOK: 26119
> eth-mac-FrameTooLongErrors: 0
>
> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> ---
> v2: Rebase to the latest
>
> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
> ---
> drivers/net/ethernet/meta/fbnic/Makefile | 1 +
> drivers/net/ethernet/meta/fbnic/fbnic.h | 4 ++
> drivers/net/ethernet/meta/fbnic/fbnic_csr.h | 37 ++++++++++++++
> .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 49 ++++++++++++++++++
> .../net/ethernet/meta/fbnic/fbnic_hw_stats.c | 27 ++++++++++
> .../net/ethernet/meta/fbnic/fbnic_hw_stats.h | 40 +++++++++++++++
> drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 50 +++++++++++++++++++
> drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 3 ++
> 8 files changed, 211 insertions(+)
> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
>
> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
> index 37cfc34a5118..ed4533a73c57 100644
> --- a/drivers/net/ethernet/meta/fbnic/Makefile
> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_FBNIC) += fbnic.o
> fbnic-y := fbnic_devlink.o \
> fbnic_ethtool.o \
> fbnic_fw.o \
> + fbnic_hw_stats.o \
> fbnic_irq.o \
> fbnic_mac.o \
> fbnic_netdev.o \
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index 28d970f81bfc..0f9e8d79461c 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -11,6 +11,7 @@
>
> #include "fbnic_csr.h"
> #include "fbnic_fw.h"
> +#include "fbnic_hw_stats.h"
> #include "fbnic_mac.h"
> #include "fbnic_rpc.h"
>
> @@ -47,6 +48,9 @@ struct fbnic_dev {
>
> /* Number of TCQs/RCQs available on hardware */
> u16 max_num_queues;
> +
> + /* Local copy of hardware statistics */
> + struct fbnic_hw_stats hw_stats;
> };
>
> /* Reserve entry 0 in the MSI-X "others" array until we have filled all
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> index a64360de0552..21db509acbc1 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> @@ -660,6 +660,43 @@ enum {
> #define FBNIC_SIG_PCS_INTR_MASK 0x11816 /* 0x46058 */
> #define FBNIC_CSR_END_SIG 0x1184e /* CSR section delimiter */
>
> +#define FBNIC_CSR_START_MAC_STAT 0x11a00
> +#define FBNIC_MAC_STAT_RX_BYTE_COUNT_L 0x11a08 /* 0x46820 */
> +#define FBNIC_MAC_STAT_RX_BYTE_COUNT_H 0x11a09 /* 0x46824 */
> +#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_L \
> + 0x11a0a /* 0x46828 */
> +#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_H \
> + 0x11a0b /* 0x4682c */
> +#define FBNIC_MAC_STAT_RX_TOOLONG_L 0x11a0e /* 0x46838 */
> +#define FBNIC_MAC_STAT_RX_TOOLONG_H 0x11a0f /* 0x4683c */
> +#define FBNIC_MAC_STAT_RX_RECEIVED_OK_L \
> + 0x11a12 /* 0x46848 */
> +#define FBNIC_MAC_STAT_RX_RECEIVED_OK_H \
> + 0x11a13 /* 0x4684c */
> +#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_L \
> + 0x11a14 /* 0x46850 */
> +#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_H \
> + 0x11a15 /* 0x46854 */
> +#define FBNIC_MAC_STAT_RX_IFINERRORS_L 0x11a18 /* 0x46860 */
> +#define FBNIC_MAC_STAT_RX_IFINERRORS_H 0x11a19 /* 0x46864 */
> +#define FBNIC_MAC_STAT_RX_MULTICAST_L 0x11a1c /* 0x46870 */
> +#define FBNIC_MAC_STAT_RX_MULTICAST_H 0x11a1d /* 0x46874 */
> +#define FBNIC_MAC_STAT_RX_BROADCAST_L 0x11a1e /* 0x46878 */
> +#define FBNIC_MAC_STAT_RX_BROADCAST_H 0x11a1f /* 0x4687c */
> +#define FBNIC_MAC_STAT_TX_BYTE_COUNT_L 0x11a3e /* 0x468f8 */
> +#define FBNIC_MAC_STAT_TX_BYTE_COUNT_H 0x11a3f /* 0x468fc */
> +#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_L \
> + 0x11a42 /* 0x46908 */
> +#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_H \
> + 0x11a43 /* 0x4690c */
> +#define FBNIC_MAC_STAT_TX_IFOUTERRORS_L \
> + 0x11a46 /* 0x46918 */
> +#define FBNIC_MAC_STAT_TX_IFOUTERRORS_H \
> + 0x11a47 /* 0x4691c */
> +#define FBNIC_MAC_STAT_TX_MULTICAST_L 0x11a4a /* 0x46928 */
> +#define FBNIC_MAC_STAT_TX_MULTICAST_H 0x11a4b /* 0x4692c */
> +#define FBNIC_MAC_STAT_TX_BROADCAST_L 0x11a4c /* 0x46930 */
> +#define FBNIC_MAC_STAT_TX_BROADCAST_H 0x11a4d /* 0x46934 */
These might be more readable if you add another tab between the name and
the value, then you wouldn't need to do line wraps.
> /* PUL User Registers */
> #define FBNIC_CSR_START_PUL_USER 0x31000 /* CSR section delimiter */
> #define FBNIC_PUL_OB_TLP_HDR_AW_CFG 0x3103d /* 0xc40f4 */
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> index 7064dfc9f5b0..5d980e178941 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> @@ -16,8 +16,57 @@ fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
> sizeof(drvinfo->fw_version));
> }
>
> +static void fbnic_set_counter(u64 *stat, struct fbnic_stat_counter *counter)
> +{
> + if (counter->reported)
> + *stat = counter->value;
> +}
> +
> +static void
> +fbnic_get_eth_mac_stats(struct net_device *netdev,
> + struct ethtool_eth_mac_stats *eth_mac_stats)
> +{
> + struct fbnic_net *fbn = netdev_priv(netdev);
> + struct fbnic_mac_stats *mac_stats;
> + struct fbnic_dev *fbd = fbn->fbd;
> + const struct fbnic_mac *mac;
> +
> + mac_stats = &fbd->hw_stats.mac;
> + mac = fbd->mac;
> +
> + mac->get_eth_mac_stats(fbd, false, &mac_stats->eth_mac);
> +
> + fbnic_set_counter(ð_mac_stats->FramesTransmittedOK,
> + &mac_stats->eth_mac.FramesTransmittedOK);
> + fbnic_set_counter(ð_mac_stats->FramesReceivedOK,
> + &mac_stats->eth_mac.FramesReceivedOK);
> + fbnic_set_counter(ð_mac_stats->FrameCheckSequenceErrors,
> + &mac_stats->eth_mac.FrameCheckSequenceErrors);
> + fbnic_set_counter(ð_mac_stats->AlignmentErrors,
> + &mac_stats->eth_mac.AlignmentErrors);
> + fbnic_set_counter(ð_mac_stats->OctetsTransmittedOK,
> + &mac_stats->eth_mac.OctetsTransmittedOK);
> + fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACXmitError,
> + &mac_stats->eth_mac.FramesLostDueToIntMACXmitError);
> + fbnic_set_counter(ð_mac_stats->OctetsReceivedOK,
> + &mac_stats->eth_mac.OctetsReceivedOK);
> + fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACRcvError,
> + &mac_stats->eth_mac.FramesLostDueToIntMACRcvError);
> + fbnic_set_counter(ð_mac_stats->MulticastFramesXmittedOK,
> + &mac_stats->eth_mac.MulticastFramesXmittedOK);
> + fbnic_set_counter(ð_mac_stats->BroadcastFramesXmittedOK,
> + &mac_stats->eth_mac.BroadcastFramesXmittedOK);
> + fbnic_set_counter(ð_mac_stats->MulticastFramesReceivedOK,
> + &mac_stats->eth_mac.MulticastFramesReceivedOK);
> + fbnic_set_counter(ð_mac_stats->BroadcastFramesReceivedOK,
> + &mac_stats->eth_mac.BroadcastFramesReceivedOK);
> + fbnic_set_counter(ð_mac_stats->FrameTooLongErrors,
> + &mac_stats->eth_mac.FrameTooLongErrors);
> +}
> +
> static const struct ethtool_ops fbnic_ethtool_ops = {
> .get_drvinfo = fbnic_get_drvinfo,
> + .get_eth_mac_stats = fbnic_get_eth_mac_stats,
> };
>
> void fbnic_set_ethtool_ops(struct net_device *dev)
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
> new file mode 100644
> index 000000000000..a0acc7606aa1
> --- /dev/null
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
> @@ -0,0 +1,27 @@
> +#include "fbnic.h"
> +
> +u64 fbnic_stat_rd64(struct fbnic_dev *fbd, u32 reg, u32 offset)
> +{
> + u32 prev_upper, upper, lower, diff;
> +
> + prev_upper = rd32(fbd, reg + offset);
> + lower = rd32(fbd, reg);
> + upper = rd32(fbd, reg + offset);
> +
> + diff = upper - prev_upper;
> + if (!diff)
> + return ((u64)upper << 32) | lower;
Is there any particular reason you didn't use u64_stats_fetch_begin()
and u64_stats_fetch_retry() around these to protect the reads?
sln
> +
> + if (diff > 1)
> + dev_warn_once(fbd->dev,
> + "Stats inconsistent, upper 32b of %#010x updating too quickly\n",
> + reg * 4);
> +
> + /* Return only the upper bits as we cannot guarantee
> + * the accuracy of the lower bits. We will add them in
> + * when the counter slows down enough that we can get
> + * a snapshot with both upper values being the same
> + * between reads.
> + */
> + return ((u64)upper << 32);
> +}
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
> new file mode 100644
> index 000000000000..30348904b510
> --- /dev/null
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
> @@ -0,0 +1,40 @@
> +#include <linux/ethtool.h>
> +
> +#include "fbnic_csr.h"
> +
> +struct fbnic_stat_counter {
> + u64 value;
> + union {
> + u32 old_reg_value_32;
> + u64 old_reg_value_64;
> + } u;
> + bool reported;
> +};
> +
> +struct fbnic_eth_mac_stats {
> + struct fbnic_stat_counter FramesTransmittedOK;
> + struct fbnic_stat_counter FramesReceivedOK;
> + struct fbnic_stat_counter FrameCheckSequenceErrors;
> + struct fbnic_stat_counter AlignmentErrors;
> + struct fbnic_stat_counter OctetsTransmittedOK;
> + struct fbnic_stat_counter FramesLostDueToIntMACXmitError;
> + struct fbnic_stat_counter OctetsReceivedOK;
> + struct fbnic_stat_counter FramesLostDueToIntMACRcvError;
> + struct fbnic_stat_counter MulticastFramesXmittedOK;
> + struct fbnic_stat_counter BroadcastFramesXmittedOK;
> + struct fbnic_stat_counter MulticastFramesReceivedOK;
> + struct fbnic_stat_counter BroadcastFramesReceivedOK;
> + struct fbnic_stat_counter FrameTooLongErrors;
> +};
> +
> +struct fbnic_mac_stats {
> + struct fbnic_eth_mac_stats eth_mac;
> +};
> +
> +struct fbnic_hw_stats {
> + struct fbnic_mac_stats mac;
> +};
> +
> +u64 fbnic_stat_rd64(struct fbnic_dev *fbd, u32 reg, u32 offset);
> +
> +void fbnic_get_hw_stats(struct fbnic_dev *fbd);
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
> index 7920e7af82d9..7b654d0a6dac 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
> @@ -403,6 +403,21 @@ static void fbnic_mac_init_regs(struct fbnic_dev *fbd)
> fbnic_mac_init_txb(fbd);
> }
>
> +static void __fbnic_mac_stat_rd64(struct fbnic_dev *fbd, bool reset, u32 reg,
> + struct fbnic_stat_counter *stat)
> +{
> + u64 new_reg_value;
> +
> + new_reg_value = fbnic_stat_rd64(fbd, reg, 1);
> + if (!reset)
> + stat->value += new_reg_value - stat->u.old_reg_value_64;
> + stat->u.old_reg_value_64 = new_reg_value;
> + stat->reported = true;
> +}
> +
> +#define fbnic_mac_stat_rd64(fbd, reset, __stat, __CSR) \
> + __fbnic_mac_stat_rd64(fbd, reset, FBNIC_##__CSR##_L, &(__stat))
> +
> static void fbnic_mac_tx_pause_config(struct fbnic_dev *fbd, bool tx_pause)
> {
> u32 rxb_pause_ctrl;
> @@ -637,12 +652,47 @@ static void fbnic_mac_link_up_asic(struct fbnic_dev *fbd,
> wr32(fbd, FBNIC_MAC_COMMAND_CONFIG, cmd_cfg);
> }
>
> +static void
> +fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset,
> + struct fbnic_eth_mac_stats *mac_stats)
> +{
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->OctetsReceivedOK,
> + MAC_STAT_RX_BYTE_COUNT);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->AlignmentErrors,
> + MAC_STAT_RX_ALIGN_ERROR);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->FrameTooLongErrors,
> + MAC_STAT_RX_TOOLONG);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->FramesReceivedOK,
> + MAC_STAT_RX_RECEIVED_OK);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->FrameCheckSequenceErrors,
> + MAC_STAT_RX_PACKET_BAD_FCS);
> + fbnic_mac_stat_rd64(fbd, reset,
> + mac_stats->FramesLostDueToIntMACRcvError,
> + MAC_STAT_RX_IFINERRORS);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->MulticastFramesReceivedOK,
> + MAC_STAT_RX_MULTICAST);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->BroadcastFramesReceivedOK,
> + MAC_STAT_RX_BROADCAST);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->OctetsTransmittedOK,
> + MAC_STAT_TX_BYTE_COUNT);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->FramesTransmittedOK,
> + MAC_STAT_TX_TRANSMITTED_OK);
> + fbnic_mac_stat_rd64(fbd, reset,
> + mac_stats->FramesLostDueToIntMACXmitError,
> + MAC_STAT_TX_IFOUTERRORS);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->MulticastFramesXmittedOK,
> + MAC_STAT_TX_MULTICAST);
> + fbnic_mac_stat_rd64(fbd, reset, mac_stats->BroadcastFramesXmittedOK,
> + MAC_STAT_TX_BROADCAST);
> +}
> +
> static const struct fbnic_mac fbnic_mac_asic = {
> .init_regs = fbnic_mac_init_regs,
> .pcs_enable = fbnic_pcs_enable_asic,
> .pcs_disable = fbnic_pcs_disable_asic,
> .pcs_get_link = fbnic_pcs_get_link_asic,
> .pcs_get_link_event = fbnic_pcs_get_link_event_asic,
> + .get_eth_mac_stats = fbnic_mac_get_eth_mac_stats,
> .link_down = fbnic_mac_link_down_asic,
> .link_up = fbnic_mac_link_up_asic,
> };
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
> index f53be6e6aef9..476239a9d381 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
> @@ -78,6 +78,9 @@ struct fbnic_mac {
> bool (*pcs_get_link)(struct fbnic_dev *fbd);
> int (*pcs_get_link_event)(struct fbnic_dev *fbd);
>
> + void (*get_eth_mac_stats)(struct fbnic_dev *fbd, bool reset,
> + struct fbnic_eth_mac_stats *mac_stats);
> +
> void (*link_down)(struct fbnic_dev *fbd);
> void (*link_up)(struct fbnic_dev *fbd, bool tx_pause, bool rx_pause);
> };
> --
> 2.43.5
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] eth: fbnic: Add ethtool support for fbnic
2024-08-27 20:59 ` [PATCH net-next v2 1/2] eth: fbnic: Add " Mohsin Bashir
@ 2024-08-28 6:29 ` Michal Swiatkowski
2024-08-28 17:49 ` Mohsin Bashir
0 siblings, 1 reply; 11+ messages in thread
From: Michal Swiatkowski @ 2024-08-28 6:29 UTC (permalink / raw)
To: Mohsin Bashir
Cc: netdev, alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On Tue, Aug 27, 2024 at 01:59:03PM -0700, Mohsin Bashir wrote:
> Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
> provides firmware version information while the driver name and bus
> information is provided by ethtool_get_drvinfo().
>
> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> ---
> v2:
> - Update the emptiness check for firmware version commit string
> - Rebase to the latest
>
> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
Correct link:
https://lore.kernel.org/netdev/20240822184944.3882360-1-mohsin.bashr@gmail.com/
> ---
> drivers/net/ethernet/meta/fbnic/Makefile | 1 +
> drivers/net/ethernet/meta/fbnic/fbnic.h | 3 +++
> .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 26 +++++++++++++++++++
> drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 13 ++++++++++
> drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 6 ++---
> .../net/ethernet/meta/fbnic/fbnic_netdev.c | 2 ++
> .../net/ethernet/meta/fbnic/fbnic_netdev.h | 1 +
> 7 files changed, 49 insertions(+), 3 deletions(-)
> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>
> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
> index 9373b558fdc9..37cfc34a5118 100644
> --- a/drivers/net/ethernet/meta/fbnic/Makefile
> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
> @@ -8,6 +8,7 @@
> obj-$(CONFIG_FBNIC) += fbnic.o
>
> fbnic-y := fbnic_devlink.o \
> + fbnic_ethtool.o \
> fbnic_fw.o \
> fbnic_irq.o \
> fbnic_mac.o \
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index ad2689bfd6cb..28d970f81bfc 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -132,6 +132,9 @@ void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
> void fbnic_free_irqs(struct fbnic_dev *fbd);
> int fbnic_alloc_irqs(struct fbnic_dev *fbd);
>
> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
> + const size_t str_sz);
> +
> enum fbnic_boards {
> fbnic_board_asic
> };
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> new file mode 100644
> index 000000000000..7064dfc9f5b0
> --- /dev/null
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> @@ -0,0 +1,26 @@
> +#include <linux/ethtool.h>
> +#include <linux/netdevice.h>
> +#include <linux/pci.h>
> +
> +#include "fbnic.h"
> +#include "fbnic_netdev.h"
> +#include "fbnic_tlv.h"
> +
> +static void
> +fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
> +{
> + struct fbnic_net *fbn = netdev_priv(netdev);
> + struct fbnic_dev *fbd = fbn->fbd;
> +
> + fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
> + sizeof(drvinfo->fw_version));
> +}
> +
> +static const struct ethtool_ops fbnic_ethtool_ops = {
> + .get_drvinfo = fbnic_get_drvinfo,
> +};
> +
> +void fbnic_set_ethtool_ops(struct net_device *dev)
> +{
> + dev->ethtool_ops = &fbnic_ethtool_ops;
> +}
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> index 0c6e1b4c119b..8f7a2a19ddf8 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> @@ -789,3 +789,16 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
> count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
> } while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
> }
> +
> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
> + const size_t str_sz)
Don't you need a prototype for this function?
> +{
> + struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
> + const char *delim = "";
> +
> + if (mgmt->commit[0])
> + delim = "_";
> +
> + fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
> + fw_version, str_sz);
> +}
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> index c65bca613665..221faf8c6756 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> @@ -53,10 +53,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
> int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
> void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
>
> -#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str) \
> +#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
> do { \
> const u32 __rev_id = _rev_id; \
> - snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s", \
> + snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s", \
> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id), \
> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id), \
> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id), \
> @@ -65,7 +65,7 @@ do { \
> } while (0)
>
> #define fbnic_mk_fw_ver_str(_rev_id, _str) \
> - fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
> + fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
>
> #define FW_HEARTBEAT_PERIOD (10 * HZ)
>
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> index 571374361259..a400616a24d4 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> @@ -521,6 +521,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
> netdev->netdev_ops = &fbnic_netdev_ops;
> netdev->stat_ops = &fbnic_stat_ops;
>
> + fbnic_set_ethtool_ops(netdev);
> +
> fbn = netdev_priv(netdev);
>
> fbn->netdev = netdev;
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
> index 60199e634468..6c27da09a612 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
> @@ -58,6 +58,7 @@ int fbnic_netdev_register(struct net_device *netdev);
> void fbnic_netdev_unregister(struct net_device *netdev);
> void fbnic_reset_queues(struct fbnic_net *fbn,
> unsigned int tx, unsigned int rx);
> +void fbnic_set_ethtool_ops(struct net_device *dev);
Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
you will have more functions there in the future.
Thanks,
Michal
>
> void __fbnic_set_rx_mode(struct net_device *netdev);
> void fbnic_clear_rx_mode(struct net_device *netdev);
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats
2024-08-27 20:59 ` [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats Mohsin Bashir
2024-08-28 0:30 ` Nelson, Shannon
@ 2024-08-28 14:37 ` Simon Horman
2024-08-28 17:50 ` Mohsin Bashir
1 sibling, 1 reply; 11+ messages in thread
From: Simon Horman @ 2024-08-28 14:37 UTC (permalink / raw)
To: Mohsin Bashir
Cc: netdev, alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On Tue, Aug 27, 2024 at 01:59:04PM -0700, Mohsin Bashir wrote:
> Add support for group stats for mac. The fbnic_set_counter helps prevent
> overriding the default values for counters which are not collected by the device.
>
> The 'reset' flag in 'get_eth_mac_stats' allows choosing between
> resetting the counter to recent most value or fecthing the aggregate
nit: fetching
> values of counters. This is important to cater for cases such as
> device reset.
>
> The 'fbnic_stat_rd64' read 64b stats counters in a consistent fashion using
> high-low-high approach. This allows to isolate cases where counter is
> wrapped between the reads.
>
> Command: ethtool -S eth0 --groups eth-mac
> Example Output:
> eth-mac-FramesTransmittedOK: 421644
> eth-mac-FramesReceivedOK: 3849708
> eth-mac-FrameCheckSequenceErrors: 0
> eth-mac-AlignmentErrors: 0
> eth-mac-OctetsTransmittedOK: 64799060
> eth-mac-FramesLostDueToIntMACXmitError: 0
> eth-mac-OctetsReceivedOK: 5134513531
> eth-mac-FramesLostDueToIntMACRcvError: 0
> eth-mac-MulticastFramesXmittedOK: 568
> eth-mac-BroadcastFramesXmittedOK: 454
> eth-mac-MulticastFramesReceivedOK: 276106
> eth-mac-BroadcastFramesReceivedOK: 26119
> eth-mac-FrameTooLongErrors: 0
>
> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats
2024-08-28 0:30 ` Nelson, Shannon
@ 2024-08-28 16:41 ` Alexander H Duyck
2024-08-29 0:47 ` Mohsin Bashir
0 siblings, 1 reply; 11+ messages in thread
From: Alexander H Duyck @ 2024-08-28 16:41 UTC (permalink / raw)
To: Nelson, Shannon, Mohsin Bashir, netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On Tue, 2024-08-27 at 17:30 -0700, Nelson, Shannon wrote:
> On 8/27/2024 1:59 PM, Mohsin Bashir wrote:
> >
> > Add support for group stats for mac. The fbnic_set_counter helps prevent
> > overriding the default values for counters which are not collected by the device.
> >
> > The 'reset' flag in 'get_eth_mac_stats' allows choosing between
> > resetting the counter to recent most value or fecthing the aggregate
> > values of counters. This is important to cater for cases such as
> > device reset.
> >
> > The 'fbnic_stat_rd64' read 64b stats counters in a consistent fashion using
> > high-low-high approach. This allows to isolate cases where counter is
> > wrapped between the reads.
> >
> > Command: ethtool -S eth0 --groups eth-mac
> > Example Output:
> > eth-mac-FramesTransmittedOK: 421644
> > eth-mac-FramesReceivedOK: 3849708
> > eth-mac-FrameCheckSequenceErrors: 0
> > eth-mac-AlignmentErrors: 0
> > eth-mac-OctetsTransmittedOK: 64799060
> > eth-mac-FramesLostDueToIntMACXmitError: 0
> > eth-mac-OctetsReceivedOK: 5134513531
> > eth-mac-FramesLostDueToIntMACRcvError: 0
> > eth-mac-MulticastFramesXmittedOK: 568
> > eth-mac-BroadcastFramesXmittedOK: 454
> > eth-mac-MulticastFramesReceivedOK: 276106
> > eth-mac-BroadcastFramesReceivedOK: 26119
> > eth-mac-FrameTooLongErrors: 0
> >
> > Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> > ---
> > v2: Rebase to the latest
> >
> > v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
> > ---
> > drivers/net/ethernet/meta/fbnic/Makefile | 1 +
> > drivers/net/ethernet/meta/fbnic/fbnic.h | 4 ++
> > drivers/net/ethernet/meta/fbnic/fbnic_csr.h | 37 ++++++++++++++
> > .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 49 ++++++++++++++++++
> > .../net/ethernet/meta/fbnic/fbnic_hw_stats.c | 27 ++++++++++
> > .../net/ethernet/meta/fbnic/fbnic_hw_stats.h | 40 +++++++++++++++
> > drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 50 +++++++++++++++++++
> > drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 3 ++
> > 8 files changed, 211 insertions(+)
> > create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
> > create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
> >
> > diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
> > index 37cfc34a5118..ed4533a73c57 100644
> > --- a/drivers/net/ethernet/meta/fbnic/Makefile
> > +++ b/drivers/net/ethernet/meta/fbnic/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_FBNIC) += fbnic.o
> > fbnic-y := fbnic_devlink.o \
> > fbnic_ethtool.o \
> > fbnic_fw.o \
> > + fbnic_hw_stats.o \
> > fbnic_irq.o \
> > fbnic_mac.o \
> > fbnic_netdev.o \
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> > index 28d970f81bfc..0f9e8d79461c 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> > @@ -11,6 +11,7 @@
> >
> > #include "fbnic_csr.h"
> > #include "fbnic_fw.h"
> > +#include "fbnic_hw_stats.h"
> > #include "fbnic_mac.h"
> > #include "fbnic_rpc.h"
> >
> > @@ -47,6 +48,9 @@ struct fbnic_dev {
> >
> > /* Number of TCQs/RCQs available on hardware */
> > u16 max_num_queues;
> > +
> > + /* Local copy of hardware statistics */
> > + struct fbnic_hw_stats hw_stats;
> > };
> >
> > /* Reserve entry 0 in the MSI-X "others" array until we have filled all
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> > index a64360de0552..21db509acbc1 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> > @@ -660,6 +660,43 @@ enum {
> > #define FBNIC_SIG_PCS_INTR_MASK 0x11816 /* 0x46058 */
> > #define FBNIC_CSR_END_SIG 0x1184e /* CSR section delimiter */
> >
> > +#define FBNIC_CSR_START_MAC_STAT 0x11a00
> > +#define FBNIC_MAC_STAT_RX_BYTE_COUNT_L 0x11a08 /* 0x46820 */
> > +#define FBNIC_MAC_STAT_RX_BYTE_COUNT_H 0x11a09 /* 0x46824 */
> > +#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_L \
> > + 0x11a0a /* 0x46828 */
> > +#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_H \
> > + 0x11a0b /* 0x4682c */
> > +#define FBNIC_MAC_STAT_RX_TOOLONG_L 0x11a0e /* 0x46838 */
> > +#define FBNIC_MAC_STAT_RX_TOOLONG_H 0x11a0f /* 0x4683c */
> > +#define FBNIC_MAC_STAT_RX_RECEIVED_OK_L \
> > + 0x11a12 /* 0x46848 */
> > +#define FBNIC_MAC_STAT_RX_RECEIVED_OK_H \
> > + 0x11a13 /* 0x4684c */
> > +#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_L \
> > + 0x11a14 /* 0x46850 */
> > +#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_H \
> > + 0x11a15 /* 0x46854 */
> > +#define FBNIC_MAC_STAT_RX_IFINERRORS_L 0x11a18 /* 0x46860 */
> > +#define FBNIC_MAC_STAT_RX_IFINERRORS_H 0x11a19 /* 0x46864 */
> > +#define FBNIC_MAC_STAT_RX_MULTICAST_L 0x11a1c /* 0x46870 */
> > +#define FBNIC_MAC_STAT_RX_MULTICAST_H 0x11a1d /* 0x46874 */
> > +#define FBNIC_MAC_STAT_RX_BROADCAST_L 0x11a1e /* 0x46878 */
> > +#define FBNIC_MAC_STAT_RX_BROADCAST_H 0x11a1f /* 0x4687c */
> > +#define FBNIC_MAC_STAT_TX_BYTE_COUNT_L 0x11a3e /* 0x468f8 */
> > +#define FBNIC_MAC_STAT_TX_BYTE_COUNT_H 0x11a3f /* 0x468fc */
> > +#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_L \
> > + 0x11a42 /* 0x46908 */
> > +#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_H \
> > + 0x11a43 /* 0x4690c */
> > +#define FBNIC_MAC_STAT_TX_IFOUTERRORS_L \
> > + 0x11a46 /* 0x46918 */
> > +#define FBNIC_MAC_STAT_TX_IFOUTERRORS_H \
> > + 0x11a47 /* 0x4691c */
> > +#define FBNIC_MAC_STAT_TX_MULTICAST_L 0x11a4a /* 0x46928 */
> > +#define FBNIC_MAC_STAT_TX_MULTICAST_H 0x11a4b /* 0x4692c */
> > +#define FBNIC_MAC_STAT_TX_BROADCAST_L 0x11a4c /* 0x46930 */
> > +#define FBNIC_MAC_STAT_TX_BROADCAST_H 0x11a4d /* 0x46934 */
>
> These might be more readable if you add another tab between the name and
> the value, then you wouldn't need to do line wraps.
We were trying to keep the format consistent from the top of these
defines to the bottom. If I recall the comment on the byte offset would
end up going past 80 characters if we shifted that over.
> > /* PUL User Registers */
> > #define FBNIC_CSR_START_PUL_USER 0x31000 /* CSR section delimiter */
> > #define FBNIC_PUL_OB_TLP_HDR_AW_CFG 0x3103d /* 0xc40f4 */
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> > index 7064dfc9f5b0..5d980e178941 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> > @@ -16,8 +16,57 @@ fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
> > sizeof(drvinfo->fw_version));
> > }
> >
> > +static void fbnic_set_counter(u64 *stat, struct fbnic_stat_counter *counter)
> > +{
> > + if (counter->reported)
> > + *stat = counter->value;
> > +}
> > +
> > +static void
> > +fbnic_get_eth_mac_stats(struct net_device *netdev,
> > + struct ethtool_eth_mac_stats *eth_mac_stats)
> > +{
> > + struct fbnic_net *fbn = netdev_priv(netdev);
> > + struct fbnic_mac_stats *mac_stats;
> > + struct fbnic_dev *fbd = fbn->fbd;
> > + const struct fbnic_mac *mac;
> > +
> > + mac_stats = &fbd->hw_stats.mac;
> > + mac = fbd->mac;
> > +
> > + mac->get_eth_mac_stats(fbd, false, &mac_stats->eth_mac);
> > +
> > + fbnic_set_counter(ð_mac_stats->FramesTransmittedOK,
> > + &mac_stats->eth_mac.FramesTransmittedOK);
> > + fbnic_set_counter(ð_mac_stats->FramesReceivedOK,
> > + &mac_stats->eth_mac.FramesReceivedOK);
> > + fbnic_set_counter(ð_mac_stats->FrameCheckSequenceErrors,
> > + &mac_stats->eth_mac.FrameCheckSequenceErrors);
> > + fbnic_set_counter(ð_mac_stats->AlignmentErrors,
> > + &mac_stats->eth_mac.AlignmentErrors);
> > + fbnic_set_counter(ð_mac_stats->OctetsTransmittedOK,
> > + &mac_stats->eth_mac.OctetsTransmittedOK);
> > + fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACXmitError,
> > + &mac_stats->eth_mac.FramesLostDueToIntMACXmitError);
> > + fbnic_set_counter(ð_mac_stats->OctetsReceivedOK,
> > + &mac_stats->eth_mac.OctetsReceivedOK);
> > + fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACRcvError,
> > + &mac_stats->eth_mac.FramesLostDueToIntMACRcvError);
> > + fbnic_set_counter(ð_mac_stats->MulticastFramesXmittedOK,
> > + &mac_stats->eth_mac.MulticastFramesXmittedOK);
> > + fbnic_set_counter(ð_mac_stats->BroadcastFramesXmittedOK,
> > + &mac_stats->eth_mac.BroadcastFramesXmittedOK);
> > + fbnic_set_counter(ð_mac_stats->MulticastFramesReceivedOK,
> > + &mac_stats->eth_mac.MulticastFramesReceivedOK);
> > + fbnic_set_counter(ð_mac_stats->BroadcastFramesReceivedOK,
> > + &mac_stats->eth_mac.BroadcastFramesReceivedOK);
> > + fbnic_set_counter(ð_mac_stats->FrameTooLongErrors,
> > + &mac_stats->eth_mac.FrameTooLongErrors);
> > +}
> > +
> > static const struct ethtool_ops fbnic_ethtool_ops = {
> > .get_drvinfo = fbnic_get_drvinfo,
> > + .get_eth_mac_stats = fbnic_get_eth_mac_stats,
> > };
> >
> > void fbnic_set_ethtool_ops(struct net_device *dev)
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
> > new file mode 100644
> > index 000000000000..a0acc7606aa1
> > --- /dev/null
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
> > @@ -0,0 +1,27 @@
> > +#include "fbnic.h"
> > +
> > +u64 fbnic_stat_rd64(struct fbnic_dev *fbd, u32 reg, u32 offset)
> > +{
> > + u32 prev_upper, upper, lower, diff;
> > +
> > + prev_upper = rd32(fbd, reg + offset);
> > + lower = rd32(fbd, reg);
> > + upper = rd32(fbd, reg + offset);
> > +
> > + diff = upper - prev_upper;
> > + if (!diff)
> > + return ((u64)upper << 32) | lower;
>
> Is there any particular reason you didn't use u64_stats_fetch_begin()
> and u64_stats_fetch_retry() around these to protect the reads?
>
> sln
The thing is there isn't another thread to race against. The checking
here is against hardware so it cannot cooperate with the
stats_fetch_begin/retry.
That said we do have a function that is collecting the 32b register
stats that we probably do need to add this wrapper for as it has to run
in the service task to update the stats.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] eth: fbnic: Add ethtool support for fbnic
2024-08-28 6:29 ` Michal Swiatkowski
@ 2024-08-28 17:49 ` Mohsin Bashir
2024-08-28 18:18 ` Jakub Kicinski
0 siblings, 1 reply; 11+ messages in thread
From: Mohsin Bashir @ 2024-08-28 17:49 UTC (permalink / raw)
To: Michal Swiatkowski, netdev
Cc: netdev, alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On 8/27/24 11:29 PM, Michal Swiatkowski wrote:
> On Tue, Aug 27, 2024 at 01:59:03PM -0700, Mohsin Bashir wrote:
>> Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
>> provides firmware version information while the driver name and bus
>> information is provided by ethtool_get_drvinfo().
>>
>> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
>> ---
>> v2:
>> - Update the emptiness check for firmware version commit string
>> - Rebase to the latest
>>
>> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
> Correct link:
> https://lore.kernel.org/netdev/20240822184944.3882360-1-mohsin.bashr@gmail.com/
Thank you for pointing this out.
>
>> ---
>> drivers/net/ethernet/meta/fbnic/Makefile | 1 +
>> drivers/net/ethernet/meta/fbnic/fbnic.h | 3 +++
>> .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 26 +++++++++++++++++++
>> drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 13 ++++++++++
>> drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 6 ++---
>> .../net/ethernet/meta/fbnic/fbnic_netdev.c | 2 ++
>> .../net/ethernet/meta/fbnic/fbnic_netdev.h | 1 +
>> 7 files changed, 49 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>>
>> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
>> index 9373b558fdc9..37cfc34a5118 100644
>> --- a/drivers/net/ethernet/meta/fbnic/Makefile
>> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
>> @@ -8,6 +8,7 @@
>> obj-$(CONFIG_FBNIC) += fbnic.o
>>
>> fbnic-y := fbnic_devlink.o \
>> + fbnic_ethtool.o \
>> fbnic_fw.o \
>> fbnic_irq.o \
>> fbnic_mac.o \
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
>> index ad2689bfd6cb..28d970f81bfc 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
>> @@ -132,6 +132,9 @@ void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
>> void fbnic_free_irqs(struct fbnic_dev *fbd);
>> int fbnic_alloc_irqs(struct fbnic_dev *fbd);
>>
>> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
>> + const size_t str_sz);
>> +
>> enum fbnic_boards {
>> fbnic_board_asic
>> };
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> new file mode 100644
>> index 000000000000..7064dfc9f5b0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> @@ -0,0 +1,26 @@
>> +#include <linux/ethtool.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/pci.h>
>> +
>> +#include "fbnic.h"
>> +#include "fbnic_netdev.h"
>> +#include "fbnic_tlv.h"
>> +
>> +static void
>> +fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
>> +{
>> + struct fbnic_net *fbn = netdev_priv(netdev);
>> + struct fbnic_dev *fbd = fbn->fbd;
>> +
>> + fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
>> + sizeof(drvinfo->fw_version));
>> +}
>> +
>> +static const struct ethtool_ops fbnic_ethtool_ops = {
>> + .get_drvinfo = fbnic_get_drvinfo,
>> +};
>> +
>> +void fbnic_set_ethtool_ops(struct net_device *dev)
>> +{
>> + dev->ethtool_ops = &fbnic_ethtool_ops;
>> +}
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> index 0c6e1b4c119b..8f7a2a19ddf8 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> @@ -789,3 +789,16 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
>> count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
>> } while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
>> }
>> +
>> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
>> + const size_t str_sz)
> Don't you need a prototype for this function?
We do have the prototype in fbnic.h
>> +{
>> + struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
>> + const char *delim = "";
>> +
>> + if (mgmt->commit[0])
>> + delim = "_";
>> +
>> + fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
>> + fw_version, str_sz);
>> +}
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> index c65bca613665..221faf8c6756 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> @@ -53,10 +53,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
>> int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
>> void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
>>
>> -#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str) \
>> +#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
>> do { \
>> const u32 __rev_id = _rev_id; \
>> - snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s", \
>> + snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s", \
>> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id), \
>> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id), \
>> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id), \
>> @@ -65,7 +65,7 @@ do { \
>> } while (0)
>>
>> #define fbnic_mk_fw_ver_str(_rev_id, _str) \
>> - fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
>> + fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
>>
>> #define FW_HEARTBEAT_PERIOD (10 * HZ)
>>
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> index 571374361259..a400616a24d4 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> @@ -521,6 +521,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
>> netdev->netdev_ops = &fbnic_netdev_ops;
>> netdev->stat_ops = &fbnic_stat_ops;
>>
>> + fbnic_set_ethtool_ops(netdev);
>> +
>> fbn = netdev_priv(netdev);
>>
>> fbn->netdev = netdev;
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> index 60199e634468..6c27da09a612 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> @@ -58,6 +58,7 @@ int fbnic_netdev_register(struct net_device *netdev);
>> void fbnic_netdev_unregister(struct net_device *netdev);
>> void fbnic_reset_queues(struct fbnic_net *fbn,
>> unsigned int tx, unsigned int rx);
>> +void fbnic_set_ethtool_ops(struct net_device *dev);
> Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
> you will have more functions there in the future.
>
> Thanks,
> Michal
It would definitely be cleaner if have a large number of functions which
isn't the case hence, did not add fbnic_ethtool.h
>
>>
>> void __fbnic_set_rx_mode(struct net_device *netdev);
>> void fbnic_clear_rx_mode(struct net_device *netdev);
>> --
>> 2.43.5
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats
2024-08-28 14:37 ` Simon Horman
@ 2024-08-28 17:50 ` Mohsin Bashir
0 siblings, 0 replies; 11+ messages in thread
From: Mohsin Bashir @ 2024-08-28 17:50 UTC (permalink / raw)
To: Simon Horman, netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On 8/28/24 7:37 AM, Simon Horman wrote:
> On Tue, Aug 27, 2024 at 01:59:04PM -0700, Mohsin Bashir wrote:
>> Add support for group stats for mac. The fbnic_set_counter helps prevent
>> overriding the default values for counters which are not collected by the device.
>>
>> The 'reset' flag in 'get_eth_mac_stats' allows choosing between
>> resetting the counter to recent most value or fecthing the aggregate
> nit: fetching
Thank you for pointing.
>
>> values of counters. This is important to cater for cases such as
>> device reset.
>>
>> The 'fbnic_stat_rd64' read 64b stats counters in a consistent fashion using
>> high-low-high approach. This allows to isolate cases where counter is
>> wrapped between the reads.
>>
>> Command: ethtool -S eth0 --groups eth-mac
>> Example Output:
>> eth-mac-FramesTransmittedOK: 421644
>> eth-mac-FramesReceivedOK: 3849708
>> eth-mac-FrameCheckSequenceErrors: 0
>> eth-mac-AlignmentErrors: 0
>> eth-mac-OctetsTransmittedOK: 64799060
>> eth-mac-FramesLostDueToIntMACXmitError: 0
>> eth-mac-OctetsReceivedOK: 5134513531
>> eth-mac-FramesLostDueToIntMACRcvError: 0
>> eth-mac-MulticastFramesXmittedOK: 568
>> eth-mac-BroadcastFramesXmittedOK: 454
>> eth-mac-MulticastFramesReceivedOK: 276106
>> eth-mac-BroadcastFramesReceivedOK: 26119
>> eth-mac-FrameTooLongErrors: 0
>>
>> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> ...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 1/2] eth: fbnic: Add ethtool support for fbnic
2024-08-28 17:49 ` Mohsin Bashir
@ 2024-08-28 18:18 ` Jakub Kicinski
0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2024-08-28 18:18 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: Mohsin Bashir, netdev, alexanderduyck, andrew, davem, edumazet,
pabeni, kernel-team, sanmanpradhan, sdf, jdamato
On Wed, 28 Aug 2024 10:49:06 -0700 Mohsin Bashir wrote:
> > Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
> > you will have more functions there in the future.
>
> It would definitely be cleaner if have a large number of functions which
> isn't the case hence, did not add fbnic_ethtool.h
To further clarify, if I'm grepping right - the more fully featured
"prototype" driver out of tree only exposes this one function from
fbnic_ethtool.c
Most ethtool functions get hooked in via ops, and ethtool code calls
out, rather than getting called by the driver itself. So it's probably
fine (and I only mean in this particular case, IDK if all the headers
in fbnic are well organized, it's been a while.)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats
2024-08-28 16:41 ` Alexander H Duyck
@ 2024-08-29 0:47 ` Mohsin Bashir
0 siblings, 0 replies; 11+ messages in thread
From: Mohsin Bashir @ 2024-08-29 0:47 UTC (permalink / raw)
To: Alexander H Duyck, Nelson, Shannon, netdev
Cc: alexanderduyck, kuba, andrew, davem, edumazet, pabeni,
kernel-team, sanmanpradhan, sdf, jdamato
On 8/28/24 9:41 AM, Alexander H Duyck wrote:
> On Tue, 2024-08-27 at 17:30 -0700, Nelson, Shannon wrote:
>> On 8/27/2024 1:59 PM, Mohsin Bashir wrote:
>>> Add support for group stats for mac. The fbnic_set_counter helps prevent
>>> overriding the default values for counters which are not collected by the device.
>>>
>>> The 'reset' flag in 'get_eth_mac_stats' allows choosing between
>>> resetting the counter to recent most value or fecthing the aggregate
>>> values of counters. This is important to cater for cases such as
>>> device reset.
>>>
>>> The 'fbnic_stat_rd64' read 64b stats counters in a consistent fashion using
>>> high-low-high approach. This allows to isolate cases where counter is
>>> wrapped between the reads.
>>>
>>> Command: ethtool -S eth0 --groups eth-mac
>>> Example Output:
>>> eth-mac-FramesTransmittedOK: 421644
>>> eth-mac-FramesReceivedOK: 3849708
>>> eth-mac-FrameCheckSequenceErrors: 0
>>> eth-mac-AlignmentErrors: 0
>>> eth-mac-OctetsTransmittedOK: 64799060
>>> eth-mac-FramesLostDueToIntMACXmitError: 0
>>> eth-mac-OctetsReceivedOK: 5134513531
>>> eth-mac-FramesLostDueToIntMACRcvError: 0
>>> eth-mac-MulticastFramesXmittedOK: 568
>>> eth-mac-BroadcastFramesXmittedOK: 454
>>> eth-mac-MulticastFramesReceivedOK: 276106
>>> eth-mac-BroadcastFramesReceivedOK: 26119
>>> eth-mac-FrameTooLongErrors: 0
>>>
>>> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
>>> ---
>>> v2: Rebase to the latest
>>>
>>> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
>>> ---
>>> drivers/net/ethernet/meta/fbnic/Makefile | 1 +
>>> drivers/net/ethernet/meta/fbnic/fbnic.h | 4 ++
>>> drivers/net/ethernet/meta/fbnic/fbnic_csr.h | 37 ++++++++++++++
>>> .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 49 ++++++++++++++++++
>>> .../net/ethernet/meta/fbnic/fbnic_hw_stats.c | 27 ++++++++++
>>> .../net/ethernet/meta/fbnic/fbnic_hw_stats.h | 40 +++++++++++++++
>>> drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 50 +++++++++++++++++++
>>> drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 3 ++
>>> 8 files changed, 211 insertions(+)
>>> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
>>> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.h
>>>
>>> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
>>> index 37cfc34a5118..ed4533a73c57 100644
>>> --- a/drivers/net/ethernet/meta/fbnic/Makefile
>>> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
>>> @@ -10,6 +10,7 @@ obj-$(CONFIG_FBNIC) += fbnic.o
>>> fbnic-y := fbnic_devlink.o \
>>> fbnic_ethtool.o \
>>> fbnic_fw.o \
>>> + fbnic_hw_stats.o \
>>> fbnic_irq.o \
>>> fbnic_mac.o \
>>> fbnic_netdev.o \
>>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
>>> index 28d970f81bfc..0f9e8d79461c 100644
>>> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
>>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
>>> @@ -11,6 +11,7 @@
>>>
>>> #include "fbnic_csr.h"
>>> #include "fbnic_fw.h"
>>> +#include "fbnic_hw_stats.h"
>>> #include "fbnic_mac.h"
>>> #include "fbnic_rpc.h"
>>>
>>> @@ -47,6 +48,9 @@ struct fbnic_dev {
>>>
>>> /* Number of TCQs/RCQs available on hardware */
>>> u16 max_num_queues;
>>> +
>>> + /* Local copy of hardware statistics */
>>> + struct fbnic_hw_stats hw_stats;
>>> };
>>>
>>> /* Reserve entry 0 in the MSI-X "others" array until we have filled all
>>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
>>> index a64360de0552..21db509acbc1 100644
>>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
>>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
>>> @@ -660,6 +660,43 @@ enum {
>>> #define FBNIC_SIG_PCS_INTR_MASK 0x11816 /* 0x46058 */
>>> #define FBNIC_CSR_END_SIG 0x1184e /* CSR section delimiter */
>>>
>>> +#define FBNIC_CSR_START_MAC_STAT 0x11a00
>>> +#define FBNIC_MAC_STAT_RX_BYTE_COUNT_L 0x11a08 /* 0x46820 */
>>> +#define FBNIC_MAC_STAT_RX_BYTE_COUNT_H 0x11a09 /* 0x46824 */
>>> +#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_L \
>>> + 0x11a0a /* 0x46828 */
>>> +#define FBNIC_MAC_STAT_RX_ALIGN_ERROR_H \
>>> + 0x11a0b /* 0x4682c */
>>> +#define FBNIC_MAC_STAT_RX_TOOLONG_L 0x11a0e /* 0x46838 */
>>> +#define FBNIC_MAC_STAT_RX_TOOLONG_H 0x11a0f /* 0x4683c */
>>> +#define FBNIC_MAC_STAT_RX_RECEIVED_OK_L \
>>> + 0x11a12 /* 0x46848 */
>>> +#define FBNIC_MAC_STAT_RX_RECEIVED_OK_H \
>>> + 0x11a13 /* 0x4684c */
>>> +#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_L \
>>> + 0x11a14 /* 0x46850 */
>>> +#define FBNIC_MAC_STAT_RX_PACKET_BAD_FCS_H \
>>> + 0x11a15 /* 0x46854 */
>>> +#define FBNIC_MAC_STAT_RX_IFINERRORS_L 0x11a18 /* 0x46860 */
>>> +#define FBNIC_MAC_STAT_RX_IFINERRORS_H 0x11a19 /* 0x46864 */
>>> +#define FBNIC_MAC_STAT_RX_MULTICAST_L 0x11a1c /* 0x46870 */
>>> +#define FBNIC_MAC_STAT_RX_MULTICAST_H 0x11a1d /* 0x46874 */
>>> +#define FBNIC_MAC_STAT_RX_BROADCAST_L 0x11a1e /* 0x46878 */
>>> +#define FBNIC_MAC_STAT_RX_BROADCAST_H 0x11a1f /* 0x4687c */
>>> +#define FBNIC_MAC_STAT_TX_BYTE_COUNT_L 0x11a3e /* 0x468f8 */
>>> +#define FBNIC_MAC_STAT_TX_BYTE_COUNT_H 0x11a3f /* 0x468fc */
>>> +#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_L \
>>> + 0x11a42 /* 0x46908 */
>>> +#define FBNIC_MAC_STAT_TX_TRANSMITTED_OK_H \
>>> + 0x11a43 /* 0x4690c */
>>> +#define FBNIC_MAC_STAT_TX_IFOUTERRORS_L \
>>> + 0x11a46 /* 0x46918 */
>>> +#define FBNIC_MAC_STAT_TX_IFOUTERRORS_H \
>>> + 0x11a47 /* 0x4691c */
>>> +#define FBNIC_MAC_STAT_TX_MULTICAST_L 0x11a4a /* 0x46928 */
>>> +#define FBNIC_MAC_STAT_TX_MULTICAST_H 0x11a4b /* 0x4692c */
>>> +#define FBNIC_MAC_STAT_TX_BROADCAST_L 0x11a4c /* 0x46930 */
>>> +#define FBNIC_MAC_STAT_TX_BROADCAST_H 0x11a4d /* 0x46934 */
>> These might be more readable if you add another tab between the name and
>> the value, then you wouldn't need to do line wraps.
> We were trying to keep the format consistent from the top of these
> defines to the bottom. If I recall the comment on the byte offset would
> end up going past 80 characters if we shifted that over.
>
>>> /* PUL User Registers */
>>> #define FBNIC_CSR_START_PUL_USER 0x31000 /* CSR section delimiter */
>>> #define FBNIC_PUL_OB_TLP_HDR_AW_CFG 0x3103d /* 0xc40f4 */
>>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>>> index 7064dfc9f5b0..5d980e178941 100644
>>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>>> @@ -16,8 +16,57 @@ fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
>>> sizeof(drvinfo->fw_version));
>>> }
>>>
>>> +static void fbnic_set_counter(u64 *stat, struct fbnic_stat_counter *counter)
>>> +{
>>> + if (counter->reported)
>>> + *stat = counter->value;
>>> +}
>>> +
>>> +static void
>>> +fbnic_get_eth_mac_stats(struct net_device *netdev,
>>> + struct ethtool_eth_mac_stats *eth_mac_stats)
>>> +{
>>> + struct fbnic_net *fbn = netdev_priv(netdev);
>>> + struct fbnic_mac_stats *mac_stats;
>>> + struct fbnic_dev *fbd = fbn->fbd;
>>> + const struct fbnic_mac *mac;
>>> +
>>> + mac_stats = &fbd->hw_stats.mac;
>>> + mac = fbd->mac;
>>> +
>>> + mac->get_eth_mac_stats(fbd, false, &mac_stats->eth_mac);
>>> +
>>> + fbnic_set_counter(ð_mac_stats->FramesTransmittedOK,
>>> + &mac_stats->eth_mac.FramesTransmittedOK);
>>> + fbnic_set_counter(ð_mac_stats->FramesReceivedOK,
>>> + &mac_stats->eth_mac.FramesReceivedOK);
>>> + fbnic_set_counter(ð_mac_stats->FrameCheckSequenceErrors,
>>> + &mac_stats->eth_mac.FrameCheckSequenceErrors);
>>> + fbnic_set_counter(ð_mac_stats->AlignmentErrors,
>>> + &mac_stats->eth_mac.AlignmentErrors);
>>> + fbnic_set_counter(ð_mac_stats->OctetsTransmittedOK,
>>> + &mac_stats->eth_mac.OctetsTransmittedOK);
>>> + fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACXmitError,
>>> + &mac_stats->eth_mac.FramesLostDueToIntMACXmitError);
>>> + fbnic_set_counter(ð_mac_stats->OctetsReceivedOK,
>>> + &mac_stats->eth_mac.OctetsReceivedOK);
>>> + fbnic_set_counter(ð_mac_stats->FramesLostDueToIntMACRcvError,
>>> + &mac_stats->eth_mac.FramesLostDueToIntMACRcvError);
>>> + fbnic_set_counter(ð_mac_stats->MulticastFramesXmittedOK,
>>> + &mac_stats->eth_mac.MulticastFramesXmittedOK);
>>> + fbnic_set_counter(ð_mac_stats->BroadcastFramesXmittedOK,
>>> + &mac_stats->eth_mac.BroadcastFramesXmittedOK);
>>> + fbnic_set_counter(ð_mac_stats->MulticastFramesReceivedOK,
>>> + &mac_stats->eth_mac.MulticastFramesReceivedOK);
>>> + fbnic_set_counter(ð_mac_stats->BroadcastFramesReceivedOK,
>>> + &mac_stats->eth_mac.BroadcastFramesReceivedOK);
>>> + fbnic_set_counter(ð_mac_stats->FrameTooLongErrors,
>>> + &mac_stats->eth_mac.FrameTooLongErrors);
>>> +}
>>> +
>>> static const struct ethtool_ops fbnic_ethtool_ops = {
>>> .get_drvinfo = fbnic_get_drvinfo,
>>> + .get_eth_mac_stats = fbnic_get_eth_mac_stats,
>>> };
>>>
>>> void fbnic_set_ethtool_ops(struct net_device *dev)
>>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
>>> new file mode 100644
>>> index 000000000000..a0acc7606aa1
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hw_stats.c
>>> @@ -0,0 +1,27 @@
>>> +#include "fbnic.h"
>>> +
>>> +u64 fbnic_stat_rd64(struct fbnic_dev *fbd, u32 reg, u32 offset)
>>> +{
>>> + u32 prev_upper, upper, lower, diff;
>>> +
>>> + prev_upper = rd32(fbd, reg + offset);
>>> + lower = rd32(fbd, reg);
>>> + upper = rd32(fbd, reg + offset);
>>> +
>>> + diff = upper - prev_upper;
>>> + if (!diff)
>>> + return ((u64)upper << 32) | lower;
>> Is there any particular reason you didn't use u64_stats_fetch_begin()
>> and u64_stats_fetch_retry() around these to protect the reads?
>>
>> sln
> The thing is there isn't another thread to race against. The checking
> here is against hardware so it cannot cooperate with the
> stats_fetch_begin/retry.
>
> That said we do have a function that is collecting the 32b register
> stats that we probably do need to add this wrapper for as it has to run
> in the service task to update the stats.
At the moment, the service task is not updating the stats hence, this
patch should work fine. I'll make sure to include the required support
in the respective patch when we will start updating stats in the service
task.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-08-29 0:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-27 20:59 [PATCH net-next v2 0/2] eth: Add basic ethtool support for fbnic Mohsin Bashir
2024-08-27 20:59 ` [PATCH net-next v2 1/2] eth: fbnic: Add " Mohsin Bashir
2024-08-28 6:29 ` Michal Swiatkowski
2024-08-28 17:49 ` Mohsin Bashir
2024-08-28 18:18 ` Jakub Kicinski
2024-08-27 20:59 ` [PATCH net-next v2 2/2] eth: fbnic: Add support to fetch group stats Mohsin Bashir
2024-08-28 0:30 ` Nelson, Shannon
2024-08-28 16:41 ` Alexander H Duyck
2024-08-29 0:47 ` Mohsin Bashir
2024-08-28 14:37 ` Simon Horman
2024-08-28 17:50 ` Mohsin Bashir
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).