Re: [net-next v2] net: ifb: support ethtools stats

On Fri, 26 Nov 2021 11:23:05 +0800 xiangxia.m.yue@gmail.com wrote:

From: Tonghao Zhang <redacted>

With this feature, we can use the ethtools to get tx/rx
queues stats. This patch, introduce the ifb_update_q_stats
helper to update the queues stats, and ifb_q_stats to simplify
the codes. In future, we can add more metrics in ifb_q_stats.

Cc: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Tonghao Zhang <redacted>

 static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev);
 static int ifb_open(struct net_device *dev);
 static int ifb_close(struct net_device *dev);
 
+static inline void ifb_update_q_stats(struct ifb_q_stats *stats, int len)

Please remove the "inline" keywords, we prefer to leave the choice to
the compiler (plus it hides "unused function" warnings if the caller 
is ever removed).

+static void ifb_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
+{
+	u8 *p = buf;
+	int i, j;
+
+	switch(stringset) {
+	case ETH_SS_STATS:
+		for (i = 0; i < dev->real_num_rx_queues; i++)
+			for (j = 0; j < IFB_Q_STATS_LEN; j++)
+				ethtool_sprintf(&p, "rx_queue_%u_%.18s",
+						i, ifb_q_stats_desc[j].desc);
+
+		for (i = 0; i < dev->real_num_tx_queues; i++)
+			for (j = 0; j < IFB_Q_STATS_LEN; j++)
+				ethtool_sprintf(&p, "tx_queue_%u_%.18s",
+						i, ifb_q_stats_desc[j].desc);
+
+		break;
+	}
+}
+
+static int ifb_get_sset_count(struct net_device *dev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_STATS:
+		return IFB_Q_STATS_LEN * (dev->real_num_rx_queues +
+		       dev->real_num_tx_queues);

Needs to align under opening bracket, try checkpatch --strict.

+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static inline void ifb_fill_stats_data(u64 **data,
+				       struct ifb_q_stats *q_stats)

another inline

The logic itself LGTM.