* [dpdk-dev] [PATCH v2] add drop statistic for af_packet [not found] <20191125180325.2E2284C9D@dpdk.org> @ 2019-11-26 14:32 ` Vadim 2019-11-26 14:45 ` Ferruh Yigit 0 siblings, 1 reply; 6+ messages in thread From: Vadim @ 2019-11-26 14:32 UTC (permalink / raw) To: linville; +Cc: dev, Vadim Signed-off-by: Vadim <podovinnikov@protei.ru> --- drivers/net/af_packet/rte_eth_af_packet.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c index f5806bf42..da54f82f7 100644 --- a/drivers/net/af_packet/rte_eth_af_packet.c +++ b/drivers/net/af_packet/rte_eth_af_packet.c @@ -327,8 +327,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) { unsigned i, imax; unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0; - unsigned long rx_bytes_total = 0, tx_bytes_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_v3); + struct tpacket_stats_v3 st; imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS); @@ -337,6 +339,12 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes; 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; } imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? @@ -349,6 +357,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) tx_bytes_total += igb_stats->q_obytes[i]; } + igb_stats->imissed = rx_drop; igb_stats->ipackets = rx_total; igb_stats->ibytes = rx_bytes_total; igb_stats->opackets = tx_total; -- 2.24.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] add drop statistic for af_packet 2019-11-26 14:32 ` [dpdk-dev] [PATCH v2] add drop statistic for af_packet Vadim @ 2019-11-26 14:45 ` Ferruh Yigit 2019-11-26 15:47 ` [dpdk-dev] [PATCH v3] " Vadim Podovinnikov 0 siblings, 1 reply; 6+ messages in thread From: Ferruh Yigit @ 2019-11-26 14:45 UTC (permalink / raw) To: Vadim, linville; +Cc: dev On 11/26/2019 2:32 PM, Vadim wrote: > Signed-off-by: Vadim <podovinnikov@protei.ru> Can you please provide full "Name Surname <email address>" syntax? > --- > drivers/net/af_packet/rte_eth_af_packet.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c > index f5806bf42..da54f82f7 100644 > --- a/drivers/net/af_packet/rte_eth_af_packet.c > +++ b/drivers/net/af_packet/rte_eth_af_packet.c > @@ -327,8 +327,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) > { > unsigned i, imax; > unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0; > - unsigned long rx_bytes_total = 0, tx_bytes_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_v3); > + struct tpacket_stats_v3 st; It seems you have missed comment from previous version, copying here: This will work but since the PMD is implementing 'TPACKET_V2', this may be confusing, what about using 'struct tpacket_stats'? Can you also update the patchwork [1] when a new version sent please, to update the status of the old one as "superseded". [1] https://patches.dpdk.org/project/dpdk/list/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v3] add drop statistic for af_packet 2019-11-26 14:45 ` Ferruh Yigit @ 2019-11-26 15:47 ` Vadim Podovinnikov 2019-11-28 15:03 ` Ferruh Yigit 0 siblings, 1 reply; 6+ messages in thread From: Vadim Podovinnikov @ 2019-11-26 15:47 UTC (permalink / raw) To: linville; +Cc: dev, Vadim From: Vadim <podovinnikov@protei.ru> add drop statistic for af_packet Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru> --- drivers/net/af_packet/rte_eth_af_packet.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c index f5806bf42..eee0fbce2 100644 --- a/drivers/net/af_packet/rte_eth_af_packet.c +++ b/drivers/net/af_packet/rte_eth_af_packet.c @@ -327,8 +327,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) { unsigned i, imax; unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0; - unsigned long rx_bytes_total = 0, tx_bytes_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; imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS); @@ -337,6 +339,12 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes; 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; } imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? @@ -349,6 +357,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) tx_bytes_total += igb_stats->q_obytes[i]; } + igb_stats->imissed = rx_drop; igb_stats->ipackets = rx_total; igb_stats->ibytes = rx_bytes_total; igb_stats->opackets = tx_total; -- 2.24.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v3] add drop statistic for af_packet 2019-11-26 15:47 ` [dpdk-dev] [PATCH v3] " Vadim Podovinnikov @ 2019-11-28 15:03 ` Ferruh Yigit 2019-11-29 14:59 ` [dpdk-dev] [PATCH v4] " Vadim Podovinnikov 0 siblings, 1 reply; 6+ messages in thread From: Ferruh Yigit @ 2019-11-28 15:03 UTC (permalink / raw) To: Vadim Podovinnikov, linville; +Cc: dev On 11/26/2019 3:47 PM, Vadim Podovinnikov wrote: > From: Vadim <podovinnikov@protei.ru> > > add drop statistic for af_packet > > Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru> > --- > drivers/net/af_packet/rte_eth_af_packet.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c > index f5806bf42..eee0fbce2 100644 > --- a/drivers/net/af_packet/rte_eth_af_packet.c > +++ b/drivers/net/af_packet/rte_eth_af_packet.c > @@ -327,8 +327,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) > { > unsigned i, imax; > unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0; > - unsigned long rx_bytes_total = 0, tx_bytes_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; > > imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? > internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS); > @@ -337,6 +339,12 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) > igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes; > 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; stats values should be preserved until it is reset, in this implementation 'imissed' values are values between two stats calls (since PACKET_STATISTICS resets the stats after retrieved). Can you add a field to "struct pkt_rx_queue" to save the dropped packet values and send the accumulated values in each call? Also need to add resetting this value in 'eth_stats_reset()' > } > > imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ? > @@ -349,6 +357,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats) > tx_bytes_total += igb_stats->q_obytes[i]; > } > > + igb_stats->imissed = rx_drop; > igb_stats->ipackets = rx_total; > igb_stats->ibytes = rx_bytes_total; > igb_stats->opackets = tx_total; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v4] add drop statistic for af_packet 2019-11-28 15:03 ` Ferruh Yigit @ 2019-11-29 14:59 ` Vadim Podovinnikov 2019-12-02 9:40 ` Ferruh Yigit 0 siblings, 1 reply; 6+ messages in thread From: Vadim Podovinnikov @ 2019-11-29 14:59 UTC (permalink / raw) To: linville; +Cc: dev, Vadim Podovinnikov Signed-off-by: Vadim Podovinnikov <podovinnikov@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); 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; 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; + rx_drop += igb_stats->q_errors[i]; } 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++) { -- 2.24.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v4] add drop statistic for af_packet 2019-11-29 14:59 ` [dpdk-dev] [PATCH v4] " Vadim Podovinnikov @ 2019-12-02 9:40 ` Ferruh Yigit 0 siblings, 0 replies; 6+ messages in thread From: Ferruh Yigit @ 2019-12-02 9:40 UTC (permalink / raw) To: Vadim Podovinnikov, linville; +Cc: dev On 11/29/2019 2:59 PM, Vadim Podovinnikov wrote: > Signed-off-by: Vadim Podovinnikov <podovinnikov@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++) { > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-02 9:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20191125180325.2E2284C9D@dpdk.org>
2019-11-26 14:32 ` [dpdk-dev] [PATCH v2] add drop statistic for af_packet Vadim
2019-11-26 14:45 ` Ferruh Yigit
2019-11-26 15:47 ` [dpdk-dev] [PATCH v3] " Vadim Podovinnikov
2019-11-28 15:03 ` Ferruh Yigit
2019-11-29 14:59 ` [dpdk-dev] [PATCH v4] " Vadim Podovinnikov
2019-12-02 9:40 ` Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).