On Fri, Dec 17, 2021 at 4:10 PM Maxime Coquelin <[email protected]> wrote: > > This patch adds Rx and Tx per-queue statistics. It will be > used to test hash-based Tx packet steering. Only "bytes", > and "packets" per-queue custom statistics are added, as > there are no global "errors" counters in netdev-dummy. > > Signed-off-by: Maxime Coquelin <[email protected]>
Similar to previous patch, I only have some nits. The patch lgtm. Reviewed-by: David Marchand <[email protected]> > @@ -954,6 +966,8 @@ static int > netdev_dummy_reconfigure(struct netdev *netdev_) > { > struct netdev_dummy *netdev = netdev_dummy_cast(netdev_); > + int old_n_txq = netdev_->n_txq; > + int old_n_rxq = netdev_->n_rxq; > > ovs_mutex_lock(&netdev->mutex); > > @@ -961,6 +975,24 @@ netdev_dummy_reconfigure(struct netdev *netdev_) > netdev_->n_rxq = netdev->requested_n_rxq; > netdev->numa_id = netdev->requested_numa_id; > > + if (netdev_->n_txq != old_n_txq || netdev_->n_rxq != old_n_rxq) { > + /* Resize the per queue stats arrays */ Nit: comments must end with a . > + netdev->txq_stats = xrealloc(netdev->txq_stats, > + netdev_->n_txq * sizeof *netdev->txq_stats); > + netdev->rxq_stats = xrealloc(netdev->rxq_stats, > + netdev_->n_rxq * sizeof *netdev->rxq_stats); Nit: Indent is off. > + > + /* Reset all stats for consistency between per-queue and global > + * counters */ Nit: comments must end with a . > + memset(&netdev->stats, 0, sizeof netdev->stats); > + netdev->custom_stats[0].value = 0; > + netdev->custom_stats[1].value = 0; > + memset(netdev->txq_stats, 0, > + netdev_->n_txq * sizeof *netdev->txq_stats); > + memset(netdev->rxq_stats, 0, > + netdev_->n_rxq * sizeof *netdev->rxq_stats); Nit: Indent is off. > + } > + > ovs_mutex_unlock(&netdev->mutex); > return 0; > } > @@ -1252,22 +1288,54 @@ static int > netdev_dummy_get_custom_stats(const struct netdev *netdev, > struct netdev_custom_stats *custom_stats) > { > - int i; > + int i,j; Nit: missing a space after , > > struct netdev_dummy *dev = netdev_dummy_cast(netdev); > > - custom_stats->size = 2; > + ovs_mutex_lock(&dev->mutex); > + > +#define DUMMY_Q_STATS \ > + DUMMY_Q_STAT(bytes) \ > + DUMMY_Q_STAT(packets) > + > + custom_stats->size = C_STATS_SIZE; > +#define DUMMY_Q_STAT(NAME) + netdev->n_rxq > + custom_stats->size += DUMMY_Q_STATS; > +#undef DUMMY_Q_STAT > +#define DUMMY_Q_STAT(NAME) + netdev->n_txq > + custom_stats->size += DUMMY_Q_STATS; > +#undef DUMMY_Q_STAT > + > custom_stats->counters = > - (struct netdev_custom_counter *) xcalloc(C_STATS_SIZE, > + (struct netdev_custom_counter *) xcalloc(custom_stats->size, Nit: it was there before your patch, but I don't think this cast is needed. > sizeof(struct netdev_custom_counter)); > > - ovs_mutex_lock(&dev->mutex); > + j = 0; > for (i = 0 ; i < C_STATS_SIZE ; i++) { > - custom_stats->counters[i].value = dev->custom_stats[i].value; > - ovs_strlcpy(custom_stats->counters[i].name, > + custom_stats->counters[j].value = dev->custom_stats[i].value; > + ovs_strlcpy(custom_stats->counters[j++].name, > dev->custom_stats[i].name, > NETDEV_CUSTOM_STATS_NAME_SIZE); > } > + > + for (i = 0; i < netdev->n_rxq; i++) { > +#define DUMMY_Q_STAT(NAME) \ > + snprintf(custom_stats->counters[j].name, \ > + NETDEV_CUSTOM_STATS_NAME_SIZE, "rx_q%d_"#NAME, i); \ Nit: indent is off. > + custom_stats->counters[j++].value = dev->rxq_stats[i].NAME; > + DUMMY_Q_STATS > +#undef DUMMY_Q_STAT > + } > + > + for (i = 0; i < netdev->n_txq; i++) { > +#define DUMMY_Q_STAT(NAME) \ > + snprintf(custom_stats->counters[j].name, \ > + NETDEV_CUSTOM_STATS_NAME_SIZE, "tx_q%d_"#NAME, i); \ Nit: indent is off. > + custom_stats->counters[j++].value = dev->txq_stats[i].NAME; > + DUMMY_Q_STATS > +#undef DUMMY_Q_STAT > + } > + > ovs_mutex_unlock(&dev->mutex); > > return 0; -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
