Hi, > Subject: Re: [dpdk-dev] [memnic PATCH 3/5] pmd: implement stats of MEMNIC > > Hi, > > 11/03/2014 05:38, Hiroshi Shimamoto: > > From: Hiroshi Shimamoto <h-shimamoto at ct.jp.nec.com> > > > > Implement missing feature to account statistics. > > This patch adds just an infrastructure. > > > > Signed-off-by: Hiroshi Shimamoto <h-shimamoto at ct.jp.nec.com> > > Reviewed-by: Hayato Momma <h-momma at ce.jp.nec.com> > > [...] > > > @@ -51,6 +51,7 @@ struct memnic_adapter { > > int up_idx, down_idx; > > struct rte_mempool *mp; > > struct ether_addr mac_addr; > > + struct rte_eth_stats stats[RTE_MAX_LCORE]; > > }; > > Could you make a comment to explain why you allocate a structure per core? > It is easier to read when locking strategy is described.
sure, could you please see the new one? > > > + for (i = 0; i < RTE_MAX_LCORE; i++) { > > + struct rte_eth_stats *st = &adapter->stats[i]; > > + > > + memset(st, 0, sizeof(*st)); > > + } > > Could you use only one memset for the array? > Yep, it's reasonable. The below is the updated patch. Is it okay for you? thanks, Hiroshi == From: Hiroshi Shimamoto <h-shimam...@ct.jp.nec.com> Subject: [PATCH v2] pmd: Implement stats of MEMNIC Implement missing feature to account statistics. This patch adds just an infrastructure. Allocating per core stats area to avoid locking. Signed-off-by: Hiroshi Shimamoto <h-shimamoto at ct.jp.nec.com> Reviewed-by: Hayato Momma <h-momma at ce.jp.nec.com> --- pmd/pmd_memnic.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/pmd/pmd_memnic.c b/pmd/pmd_memnic.c index bf5fc2e..facaf54 100644 --- a/pmd/pmd_memnic.c +++ b/pmd/pmd_memnic.c @@ -51,6 +51,12 @@ struct memnic_adapter { int up_idx, down_idx; struct rte_mempool *mp; struct ether_addr mac_addr; + /* + * Allocate per core stats to avoid lock for accounting. + * Incrementing stats doesn't require lock, because only one thread + * is running on per core. + */ + struct rte_eth_stats stats[RTE_MAX_LCORE]; }; static inline struct memnic_adapter *get_adapter(const struct rte_eth_dev *dev) @@ -126,13 +132,46 @@ static void memnic_dev_infos_get(struct rte_eth_dev *dev, dev_info->max_mac_addrs = 1; } -static void memnic_dev_stats_get(__rte_unused struct rte_eth_dev *dev, - __rte_unused struct rte_eth_stats *stats) +static void memnic_dev_stats_get(struct rte_eth_dev *dev, + struct rte_eth_stats *stats) { + struct memnic_adapter *adapter = get_adapter(dev); + int i; + + memset(stats, 0, sizeof(*stats)); + for (i = 0; i < RTE_MAX_LCORE; i++) { + struct rte_eth_stats *st = &adapter->stats[i]; + + stats->ipackets += st->ipackets; + stats->opackets += st->opackets; + stats->ibytes += st->ibytes; + stats->obytes += st->obytes; + stats->ierrors += st->ierrors; + stats->oerrors += st->oerrors; + stats->imcasts += st->imcasts; + stats->rx_nombuf += st->rx_nombuf; + stats->fdirmatch += st->fdirmatch; + stats->fdirmiss += st->fdirmiss; + + /* no multiqueue support now */ + stats->q_ipackets[0] = st->q_ipackets[0]; + stats->q_opackets[0] = st->q_opackets[0]; + stats->q_ibytes[0] = st->q_ibytes[0]; + stats->q_obytes[0] = st->q_obytes[0]; + stats->q_errors[0] = st->q_errors[0]; + + stats->ilbpackets += st->ilbpackets; + stats->olbpackets += st->olbpackets; + stats->ilbbytes += st->ilbbytes; + stats->olbbytes += st->olbbytes; + } } -static void memnic_dev_stats_reset(__rte_unused struct rte_eth_dev *dev) +static void memnic_dev_stats_reset(struct rte_eth_dev *dev) { + struct memnic_adapter *adapter = get_adapter(dev); + + memset(adapter->stats, 0, sizeof(adapter->stats)); } static int memnic_dev_link_update(struct rte_eth_dev *dev, -- 1.8.4