I have a few nits on my own patch. Noting them here for now. In case there is no further comment on the patch, I will send a new revision.
On Wed, Nov 9, 2022 at 9:39 PM David Marchand <[email protected]> wrote: > @@ -2845,8 +2779,7 @@ netdev_dpdk_vhost_send(struct netdev *netdev, int qid, > stats.tx_retries = MIN(retries, max_retries); > > rte_spinlock_lock(&dev->stats_lock); > - netdev_dpdk_vhost_update_tx_counters(dev, batch->packets, batch_cnt, > - &stats); > + netdev_dpdk_vhost_update_tx_counters(dev, &stats); > rte_spinlock_unlock(&dev->stats_lock); > > pkts = (struct rte_mbuf **) batch->packets; > @@ -3001,41 +2934,304 @@ netdev_dpdk_set_mtu(struct netdev *netdev, int mtu) > return 0; > } > > -static int > -netdev_dpdk_get_carrier(const struct netdev *netdev, bool *carrier); > - > static int > netdev_dpdk_vhost_get_stats(const struct netdev *netdev, > struct netdev_stats *stats) > { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + struct rte_vhost_stat_name *vhost_stats_names = NULL; > + struct rte_vhost_stat *vhost_stats = NULL; > + int vhost_stats_count; > + int err = -1; Rather than set to -1 ... > + int qid; > + int vid; > + (useless empty line) > > ovs_mutex_lock(&dev->mutex); > > - rte_spinlock_lock(&dev->stats_lock); > - /* Supported Stats */ > - stats->rx_packets = dev->stats.rx_packets; > - stats->tx_packets = dev->stats.tx_packets; > + if (!is_vhost_running(dev)) { ... it is more consistent to set err to EPROTO here. > + goto out; > + } > + > + vid = netdev_dpdk_get_vid(dev); > + > + /* We expect all rxqs have the same number of stats, only query rxq0. */ > + qid = 0 * VIRTIO_QNUM + VIRTIO_TXQ; > + err = rte_vhost_vring_stats_get_names(vid, qid, NULL, 0); > + if (err < 0) { > + err = EPROTO; > + goto out; > + } > + > + vhost_stats_count = err; > + vhost_stats_names = xcalloc(vhost_stats_count, sizeof > *vhost_stats_names); > + vhost_stats = xcalloc(vhost_stats_count, sizeof *vhost_stats); > + > + err = rte_vhost_vring_stats_get_names(vid, qid, vhost_stats_names, > + vhost_stats_count); > + if (err != vhost_stats_count) { > + err = EPROTO; > + goto out; > + } > + > +#define VHOST_PER_RXQ_STATS \ VHOST_RXQ_STATS is shorter and enough. > + VHOST_PER_RXQ_STAT(rx_packets, "good_packets") \ > + VHOST_PER_RXQ_STAT(rx_bytes, "good_bytes") \ > + VHOST_PER_RXQ_STAT(rx_broadcast_packets, "broadcast_packets") \ > + VHOST_PER_RXQ_STAT(multicast, "multicast_packets") \ > + VHOST_PER_RXQ_STAT(rx_undersized_errors, "undersize_packets") \ > + VHOST_PER_RXQ_STAT(rx_1_to_64_packets, "size_64_packets") \ > + VHOST_PER_RXQ_STAT(rx_65_to_127_packets, "size_65_127_packets") \ > + VHOST_PER_RXQ_STAT(rx_128_to_255_packets, "size_128_255_packets") \ > + VHOST_PER_RXQ_STAT(rx_256_to_511_packets, "size_256_511_packets") \ > + VHOST_PER_RXQ_STAT(rx_512_to_1023_packets, "size_512_1023_packets") \ > + VHOST_PER_RXQ_STAT(rx_1024_to_1522_packets, "size_1024_1518_packets") \ > + VHOST_PER_RXQ_STAT(rx_1523_to_max_packets, "size_1519_max_packets") -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
