| Message ID | 20191028183714.50528-1-jesse.brandeburg@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Jeff Kirsher |
| Headers | show |
| Series | [net-next,v1] i40e: implement VF stats NDO | expand |
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
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
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_ */
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(+)