Hi, Qiming, > -----Original Message----- > From: Yang, Qiming <qiming.y...@intel.com> > Sent: Thursday, June 18, 2020 2:32 PM > To: Xu, Ting <ting...@intel.com>; dev@dpdk.org > Cc: Zhang, Qi Z <qi.z.zh...@intel.com>; Wu, Jingjing <jingjing...@intel.com>; > Xing, Beilei <beilei.x...@intel.com>; Kovacevic, Marko > <marko.kovace...@intel.com>; Mcnamara, John <john.mcnam...@intel.com>; > Ye, Xiaolong <xiaolong...@intel.com> > Subject: RE: [PATCH v3 10/12] net/ice: enable stats for DCF > > > > > -----Original Message----- > > From: Xu, Ting <ting...@intel.com> > > Sent: Tuesday, June 16, 2020 20:41 > > To: dev@dpdk.org > > Cc: Zhang, Qi Z <qi.z.zh...@intel.com>; Yang, Qiming > > <qiming.y...@intel.com>; Wu, Jingjing <jingjing...@intel.com>; Xing, > > Beilei <beilei.x...@intel.com>; Kovacevic, Marko > > <marko.kovace...@intel.com>; Mcnamara, John > <john.mcnam...@intel.com>; > > Ye, Xiaolong <xiaolong...@intel.com> > > Subject: [PATCH v3 10/12] net/ice: enable stats for DCF > > > > From: Qi Zhang <qi.z.zh...@intel.com> > > > > Add support to get and reset Rx/Tx stats in DCF. Query stats from PF. > > > > Signed-off-by: Qi Zhang <qi.z.zh...@intel.com> > > Signed-off-by: Ting Xu <ting...@intel.com> > > --- > > drivers/net/ice/ice_dcf.c | 27 ++++++++ > > drivers/net/ice/ice_dcf.h | 4 ++ > > drivers/net/ice/ice_dcf_ethdev.c | 102 +++++++++++++++++++++++++++-- > > -- > > 3 files changed, 120 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c > > index > > f18c0f16a..fbeb58ee1 100644 > > --- a/drivers/net/ice/ice_dcf.c > > +++ b/drivers/net/ice/ice_dcf.c > > @@ -993,3 +993,30 @@ ice_dcf_disable_queues(struct ice_dcf_hw *hw) > > > > return err; > > } > > + > > +int > > +ice_dcf_query_stats(struct ice_dcf_hw *hw, > > + struct virtchnl_eth_stats *pstats) { struct virtchnl_queue_select > > +q_stats; struct dcf_virtchnl_cmd args; int err; > > + > > +memset(&q_stats, 0, sizeof(q_stats)); q_stats.vsi_id = > > +hw->vsi_res->vsi_id; > > + > > +args.v_op = VIRTCHNL_OP_GET_STATS; > > +args.req_msg = (uint8_t *)&q_stats; > > +args.req_msglen = sizeof(q_stats); > > +args.rsp_msglen = sizeof(*pstats); > > +args.rsp_msgbuf = (uint8_t *)pstats; > > +args.rsp_buflen = sizeof(*pstats); > > + > > +err = ice_dcf_execute_virtchnl_cmd(hw, &args); > > Why don't use ice_dcf_send_cmd_req_no_irq? all the other virtual channel > interface are called by ice_dcf_send_cmd_req_no_irq in dcf, like > ice_dcf_get_vf_vsi_map >
ice_dcf_send_cmd_req_no_irq() are always used combined with ice_dcf_recv_cmd_rsp_no_irq(), their function is the same as ice_dcf_execute_virtchnl_cmd(). The difference is that ice_dcf_send_cmd_req_no_irq() is used in initialization, before the interrupt is enabled. ice_dcf_execute_virtchnl_cmd() should be used after interrupt enable for safety. In this way, I think it is better to use ice_dcf_execute_virtchnl_cmd() here since these functions will be called after initialization. Best Regards, Xu, Ting > > +if (err) { > > +PMD_DRV_LOG(ERR, "fail to execute command > > OP_GET_STATS"); > > +return err; > > +} > > + > > +return 0; > > +} > > diff --git a/drivers/net/ice/ice_dcf.h b/drivers/net/ice/ice_dcf.h > > index > > 68e1661c0..e82bc7748 100644 > > --- a/drivers/net/ice/ice_dcf.h > > +++ b/drivers/net/ice/ice_dcf.h > > @@ -58,6 +58,7 @@ struct ice_dcf_hw { > > uint16_t msix_base; > > uint16_t nb_msix; > > uint16_t rxq_map[16]; > > +struct virtchnl_eth_stats eth_stats_offset; > > }; > > > > int ice_dcf_execute_virtchnl_cmd(struct ice_dcf_hw *hw, @@ -72,4 > > +73,7 @@ int ice_dcf_configure_queues(struct ice_dcf_hw *hw); int > > ice_dcf_config_irq_map(struct ice_dcf_hw *hw); int > > ice_dcf_switch_queue(struct ice_dcf_hw *hw, uint16_t qid, bool rx, > > bool on); int ice_dcf_disable_queues(struct ice_dcf_hw *hw); > > +int ice_dcf_query_stats(struct ice_dcf_hw *hw, struct > > +virtchnl_eth_stats *pstats); > > + > > #endif /* _ICE_DCF_H_ */ > > diff --git a/drivers/net/ice/ice_dcf_ethdev.c > > b/drivers/net/ice/ice_dcf_ethdev.c > > index 239426b09..1a675064a 100644 > > --- a/drivers/net/ice/ice_dcf_ethdev.c > > +++ b/drivers/net/ice/ice_dcf_ethdev.c > > @@ -695,19 +695,6 @@ ice_dcf_dev_info_get(struct rte_eth_dev *dev, > > return 0; } > > > > -static int > > -ice_dcf_stats_get(__rte_unused struct rte_eth_dev *dev, > > - __rte_unused struct rte_eth_stats *igb_stats) -{ -return 0; -} > > - > > -static int > > -ice_dcf_stats_reset(__rte_unused struct rte_eth_dev *dev) -{ -return > > 0; -} > > - > > static int > > ice_dcf_dev_promiscuous_enable(__rte_unused struct rte_eth_dev *dev) > > { @@ -760,6 +747,95 @@ ice_dcf_dev_filter_ctrl(struct rte_eth_dev > > *dev, return ret; } > > > > +#define ICE_DCF_32_BIT_WIDTH (CHAR_BIT * 4) #define > > +ICE_DCF_48_BIT_WIDTH (CHAR_BIT * 6) #define ICE_DCF_48_BIT_MASK > > +RTE_LEN2MASK(ICE_DCF_48_BIT_WIDTH, uint64_t) > > + > > +static void > > +ice_dcf_stat_update_48(uint64_t *offset, uint64_t *stat) { if (*stat > > +>= *offset) *stat = *stat - *offset; else *stat = (uint64_t)((*stat + > > +((uint64_t)1 << ICE_DCF_48_BIT_WIDTH)) - *offset); > > + > > +*stat &= ICE_DCF_48_BIT_MASK; > > +} > > + > > +static void > > +ice_dcf_stat_update_32(uint64_t *offset, uint64_t *stat) { if (*stat > > +>= *offset) *stat = (uint64_t)(*stat - *offset); else *stat = > > +(uint64_t)((*stat + > > +((uint64_t)1 << ICE_DCF_32_BIT_WIDTH)) - *offset); } > > + > > +static void > > +ice_dcf_update_stats(struct virtchnl_eth_stats *oes, > > + struct virtchnl_eth_stats *nes) > > +{ > > +ice_dcf_stat_update_48(&oes->rx_bytes, &nes->rx_bytes); > > +ice_dcf_stat_update_48(&oes->rx_unicast, &nes->rx_unicast); > > +ice_dcf_stat_update_48(&oes->rx_multicast, &nes->rx_multicast); > > +ice_dcf_stat_update_48(&oes->rx_broadcast, &nes->rx_broadcast); > > +ice_dcf_stat_update_32(&oes->rx_discards, &nes->rx_discards); > > +ice_dcf_stat_update_48(&oes->tx_bytes, &nes->tx_bytes); > > +ice_dcf_stat_update_48(&oes->tx_unicast, &nes->tx_unicast); > > +ice_dcf_stat_update_48(&oes->tx_multicast, &nes->tx_multicast); > > +ice_dcf_stat_update_48(&oes->tx_broadcast, &nes->tx_broadcast); > > +ice_dcf_stat_update_32(&oes->tx_errors, &nes->tx_errors); > > +ice_dcf_stat_update_32(&oes->tx_discards, &nes->tx_discards); } > > + > > + > > +static int > > +ice_dcf_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats > > +*stats) { struct ice_dcf_adapter *ad = dev->data->dev_private; struct > > +ice_dcf_hw *hw = &ad->real_hw; struct virtchnl_eth_stats pstats; int > > +ret; > > + > > +ret = ice_dcf_query_stats(hw, &pstats); if (ret == 0) { > > +ice_dcf_update_stats(&hw->eth_stats_offset, &pstats); > > +stats->ipackets = pstats.rx_unicast + pstats.rx_multicast + > > +pstats.rx_broadcast - pstats.rx_discards; > > +stats->opackets = pstats.tx_broadcast + pstats.tx_multicast + > > +pstats.tx_unicast; > > +stats->imissed = pstats.rx_discards; > > +stats->oerrors = pstats.tx_errors + pstats.tx_discards; ibytes = > > +stats->pstats.rx_bytes; ibytes -= stats->ipackets * > > +stats->RTE_ETHER_CRC_LEN; obytes = pstats.tx_bytes; > > +} else { > > +PMD_DRV_LOG(ERR, "Get statistics failed"); } return ret; } > > + > > +static int > > +ice_dcf_stats_reset(struct rte_eth_dev *dev) { struct ice_dcf_adapter > > +*ad = dev->data->dev_private; struct ice_dcf_hw *hw = &ad->real_hw; > > +struct virtchnl_eth_stats pstats; int ret; > > + > > +/* read stat values to clear hardware registers */ ret = > > +ice_dcf_query_stats(hw, &pstats); if (ret != 0) return ret; > > + > > +/* set stats offset base on current values */ > > +hw->eth_stats_offset = pstats; > > + > > +return 0; > > +} > > + > > static void > > ice_dcf_dev_close(struct rte_eth_dev *dev) { > > -- > > 2.17.1 >