On 11/29/2019 2:59 PM, Vadim Podovinnikov wrote: > Signed-off-by: Vadim Podovinnikov <podovinni...@protei.ru> > --- > drivers/net/af_packet/rte_eth_af_packet.c | 33 +++++++++++++++++------ > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c > b/drivers/net/af_packet/rte_eth_af_packet.c > index eee0fbce2..2aa7c0fcc 100644 > --- a/drivers/net/af_packet/rte_eth_af_packet.c > +++ b/drivers/net/af_packet/rte_eth_af_packet.c > @@ -52,6 +52,7 @@ struct pkt_rx_queue { > > volatile unsigned long rx_pkts; > volatile unsigned long rx_bytes; > + volatile unsigned long rx_drop; > }; > > struct pkt_tx_queue { > @@ -322,6 +323,25 @@ eth_dev_info(struct rte_eth_dev *dev, struct > rte_eth_dev_info *dev_info) > return 0; > } > > +static void > +fill_eth_drop_stats(struct rte_eth_dev *dev) > +{ > + unsigned int i, imax; > + struct pmd_internals *internal = dev->data->dev_private; > + socklen_t sock_len = sizeof(struct tpacket_stats); > + struct tpacket_stats st; > + > + imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? > + internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS); > + for (i = 0; i < imax; i++) { > + memset(&st, 0, sock_len); > + int rc = getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET, > + PACKET_STATISTICS, &st, &sock_len); > + if (rc == 0) > + internal->rx_queue[i].rx_drop += st.tp_drops; > + } > +} > + > static int > eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) > { > @@ -329,22 +349,18 @@ eth_stats_get(struct rte_eth_dev *dev, struct > rte_eth_stats *igb_stats) > unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0; > unsigned long rx_bytes_total = 0, tx_bytes_total = 0, rx_drop = 0; > const struct pmd_internals *internal = dev->data->dev_private; > - socklen_t sock_len = sizeof(struct tpacket_stats); > - struct tpacket_stats st; > + > + fill_eth_drop_stats(dev);
There is already a loop per queue, instead of maxing a function for all queues and duplicate 'imax' calculation, what about making a function that gets the queue_id as parameter and called in the blow loop? > > imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? > internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS); > for (i = 0; i < imax; i++) { > igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts; > igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes; > + igb_stats->q_errors[i] = internal->rx_queue[i].rx_drop; 'q_errors' is for the packets that are received but erroneous, what is the af_packet 'tp_drops' stats for? Is it for wrong packets or the packets dropped because receive rate was too high? > rx_total += igb_stats->q_ipackets[i]; > rx_bytes_total += igb_stats->q_ibytes[i]; > - > - memset(&st, 0, sock_len); > - int rc = getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET, > - PACKET_STATISTICS, &st, &sock_len); > - if (rc == 0) > - rx_drop += st.tp_drops; The patch looks like on top of your previous version, please make new versions always on top of latest head, so not incremental patches but from scratch each time. > + rx_drop += igb_stats->q_errors[i]; What are you doing with 'rx_drop'? Not using it at all? > } > > imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? > @@ -375,6 +391,7 @@ eth_stats_reset(struct rte_eth_dev *dev) > for (i = 0; i < internal->nb_queues; i++) { > internal->rx_queue[i].rx_pkts = 0; > internal->rx_queue[i].rx_bytes = 0; > + internal->rx_queue[i].rx_drop = 0; > } > > for (i = 0; i < internal->nb_queues; i++) { >