[Intel-wired-lan] [PATCH net-next v1] i40e: implement VF stats NDO

[Intel-wired-lan] [PATCH net-next v1] i40e: implement VF stats NDO

[Jesse Brandeburg]

Implement the VF stats gathering via the kernel via ndo_get_vf_stats().
The driver will show per-VF stats in the output of the
ip -s link show dev <PF>
command.

Testing Hints (Required if no HSD): ip -s link show dev eth0,
will return non-zero VF stats.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  1 +
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 48 +++++++++++++++++++
 .../ethernet/intel/i40e/i40e_virtchnl_pf.h    |  2 +
 3 files changed, 51 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a6f60e8a6026..56cf7bc8add3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -12870,6 +12870,7 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_set_features	= i40e_set_features,
 	.ndo_set_vf_mac		= i40e_ndo_set_vf_mac,
 	.ndo_set_vf_vlan	= i40e_ndo_set_vf_port_vlan,
+	.ndo_get_vf_stats	= i40e_get_vf_stats,
 	.ndo_set_vf_rate	= i40e_ndo_set_vf_bw,
 	.ndo_get_vf_config	= i40e_ndo_get_vf_config,
 	.ndo_set_vf_link_state	= i40e_ndo_set_vf_link_state,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 7787766d0eb8..84cf851cbbb7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4527,3 +4527,51 @@ int i40e_ndo_set_vf_trust(struct net_device *netdev, int vf_id, bool setting)
 	clear_bit(__I40E_VIRTCHNL_OP_PENDING, pf->state);
 	return ret;
 }
+
+/**
+ * i40e_get_vf_stats - populate some stats for the VF
+ * @netdev: the netdev of the PF
+ * @vf_id: the host OS identifier (0-127)
+ * @vf_stats: pointer to the OS memory to be initialized
+ */
+int i40e_get_vf_stats(struct net_device *netdev, int vf_id,
+		      struct ifla_vf_stats *vf_stats)
+{
+	struct i40e_netdev_priv *np = netdev_priv(netdev);
+	struct i40e_pf *pf = np->vsi->back;
+	struct i40e_eth_stats *stats;
+	struct i40e_vsi *vsi;
+	struct i40e_vf *vf;
+
+	/* validate the request */
+	if (i40e_validate_vf(pf, vf_id))
+		return -EINVAL;
+
+	vf = &pf->vf[vf_id];
+	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states)) {
+		dev_err(&pf->pdev->dev, "VF %d in reset. Try again.\n", vf_id);
+		return -EBUSY;
+	}
+
+	vsi = pf->vsi[vf->lan_vsi_idx];
+	if (!vsi)
+		return -EINVAL;
+
+	i40e_update_eth_stats(vsi);
+	stats = &vsi->eth_stats;
+
+	memset(vf_stats, 0, sizeof(*vf_stats));
+
+	vf_stats->rx_packets = stats->rx_unicast + stats->rx_broadcast +
+		stats->rx_multicast;
+	vf_stats->tx_packets = stats->tx_unicast + stats->tx_broadcast +
+		stats->tx_multicast;
+	vf_stats->rx_bytes   = stats->rx_bytes;
+	vf_stats->tx_bytes   = stats->tx_bytes;
+	vf_stats->broadcast  = stats->rx_broadcast + stats->tx_broadcast;
+	vf_stats->multicast  = stats->rx_multicast + stats->tx_multicast;
+	vf_stats->rx_dropped = stats->rx_discards;
+	vf_stats->tx_dropped = stats->tx_discards;
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index 1ce06240a702..631248c0981a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -138,5 +138,7 @@ int i40e_ndo_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool enable);
 
 void i40e_vc_notify_link_state(struct i40e_pf *pf);
 void i40e_vc_notify_reset(struct i40e_pf *pf);
+int i40e_get_vf_stats(struct net_device *netdev, int vf_id,
+		      struct ifla_vf_stats *vf_stats);
 
 #endif /* _I40E_VIRTCHNL_PF_H_ */
-- 
2.17.2

[Intel-wired-lan] [PATCH net-next v1] i40e: implement VF stats NDO

[Shannon Nelson]

On Mon, Oct 28, 2019 at 11:37 AM Jesse Brandeburg
<jesse.brandeburg@intel.com> wrote:
>
> Implement the VF stats gathering via the kernel via ndo_get_vf_stats().
> The driver will show per-VF stats in the output of the
> ip -s link show dev <PF>
> command.
>
> Testing Hints (Required if no HSD): ip -s link show dev eth0,
> will return non-zero VF stats.

Oh, I remember the "HSD" acronym... but it isn't needed in an external patch.

>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---

[...]

> +       vf_stats->rx_packets = stats->rx_unicast + stats->rx_broadcast +
> +               stats->rx_multicast;
> +       vf_stats->tx_packets = stats->tx_unicast + stats->tx_broadcast +
> +               stats->tx_multicast;
> +       vf_stats->rx_bytes   = stats->rx_bytes;
> +       vf_stats->tx_bytes   = stats->tx_bytes;
> +       vf_stats->broadcast  = stats->rx_broadcast + stats->tx_broadcast;
> +       vf_stats->multicast  = stats->rx_multicast + stats->tx_multicast;

Are you sure these are supposed to count up both Tx and Rx multicast
and broadcast?  With a quick peek at VF stats output from "ip -s link"
I see that only the Rx line mcast and bcast, the Tx line does not.  I
would infer that we're only counting what has been received.

    vf 0 MAC 00:00:00:00:00:00, spoof checking off, link-state auto, trust off
    RX: bytes  packets  mcast   bcast
    0          0        0       0
    TX: bytes  packets
    0          0

I suspect there's a definition somewhere, but I haven't stumbled
across it recently.

(yes, I still occasionally read these patches)

sln

[Intel-wired-lan] [PATCH net-next v1] i40e: implement VF stats NDO

[Jesse Brandeburg]

On Mon, 28 Oct 2019 15:02:12 -0700 Shannon wrote:
> On Mon, Oct 28, 2019 at 11:37 AM Jesse Brandeburg
> <jesse.brandeburg@intel.com> wrote:
> >
> > Implement the VF stats gathering via the kernel via ndo_get_vf_stats().
> > The driver will show per-VF stats in the output of the
> > ip -s link show dev <PF>
> > command.
> >
> > Testing Hints (Required if no HSD): ip -s link show dev eth0,
> > will return non-zero VF stats.  
> 
> Oh, I remember the "HSD" acronym... but it isn't needed in an external patch.

Dang, stupid template, will fix.
 
> > +       vf_stats->rx_packets = stats->rx_unicast + stats->rx_broadcast +
> > +               stats->rx_multicast;
> > +       vf_stats->tx_packets = stats->tx_unicast + stats->tx_broadcast +
> > +               stats->tx_multicast;
> > +       vf_stats->rx_bytes   = stats->rx_bytes;
> > +       vf_stats->tx_bytes   = stats->tx_bytes;
> > +       vf_stats->broadcast  = stats->rx_broadcast + stats->tx_broadcast;
> > +       vf_stats->multicast  = stats->rx_multicast + stats->tx_multicast;  
> 
> Are you sure these are supposed to count up both Tx and Rx multicast
> and broadcast?  With a quick peek at VF stats output from "ip -s link"
> I see that only the Rx line mcast and bcast, the Tx line does not.  I
> would infer that we're only counting what has been received.
> 
>     vf 0 MAC 00:00:00:00:00:00, spoof checking off, link-state auto, trust off
>     RX: bytes  packets  mcast   bcast
>     0          0        0       0
>     TX: bytes  packets
>     0          0
> 
> I suspect there's a definition somewhere, but I haven't stumbled
> across it recently.
> 
> (yes, I still occasionally read these patches)

Thanks for the feedback, it's not very clear from the code if those
variables named "broadcast" and "multicast" are for receive, transmit,
or both.  That's how I got into trouble here at least.  I spent some
time investigating and it seems that they're only mentioned and used in
the context of receive most elsewhere in the kernel, so v2 on its way!

Thanks!
Jesse