* [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart @ 2017-05-19 17:55 Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams ` (8 more replies) 0 siblings, 9 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-05-19 17:55 UTC (permalink / raw) To: dev; +Cc: skhare, Nachiketa Prachanda From: Nachiketa Prachanda <nprachan@brocade.com> Most nics like virtio, igb/ixgbe etc. don't reset counters on dev_start and arguably this helps in monitoring the counters across a longer time span with multiple device start/stops. vmxnet3 behavior is opposite to that and counters are reset by the host side implementation each time the device is restarted. Change the driver to save the counters in its private context before it is reset by writing CMD_ACTIVATE to REG_CMD. Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 102 ++++++++++++++++++++++++++++------- drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 + 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 17b471f..4d2c024 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -85,6 +85,7 @@ static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev); static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev); static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw); static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, @@ -351,6 +352,10 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev) RTE_ASSERT((hw->rxdata_desc_size & ~VMXNET3_RXDATA_DESC_SIZE_MASK) == hw->rxdata_desc_size); + /* clear shadow stats */ + memset(hw->saved_tx_stats, 0, sizeof(hw->saved_tx_stats)); + memset(hw->saved_rx_stats, 0, sizeof(hw->saved_rx_stats)); + return 0; } @@ -707,6 +712,9 @@ vmxnet3_dev_start(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); + /* Save stats before it is reset by CMD_ACTIVATE */ + vmxnet3_hw_stats_save(hw); + ret = vmxnet3_setup_driver_shared(dev); if (ret != VMXNET3_SUCCESS) return ret; @@ -820,47 +828,105 @@ vmxnet3_dev_close(struct rte_eth_dev *dev) } static void +vmxnet3_hw_tx_stats_get(struct vmxnet3_hw *hw, unsigned int q, + struct UPT1_TxStats *res) +{ +#define VMXNET3_UPDATE_TX_STAT(h, i, f, r) \ + ((r)->f = (h)->tqd_start[(i)].stats.f + \ + (h)->saved_tx_stats[(i)].f) + + VMXNET3_UPDATE_TX_STAT(hw, q, ucastPktsTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, mcastPktsTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, bcastPktsTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, ucastBytesTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, mcastBytesTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, bcastBytesTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxError, res); + VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxDiscard, res); + +#undef VMXNET3_UPDATE_TX_STAT +} + +static void +vmxnet3_hw_rx_stats_get(struct vmxnet3_hw *hw, unsigned int q, + struct UPT1_RxStats *res) +{ +#define VMXNET3_UPDATE_RX_STAT(h, i, f, r) \ + ((r)->f = (h)->rqd_start[(i)].stats.f + \ + (h)->saved_rx_stats[(i)].f) + + VMXNET3_UPDATE_RX_STAT(hw, q, ucastPktsRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, mcastPktsRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, bcastPktsRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, ucastBytesRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, mcastBytesRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, bcastBytesRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxError, res); + VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxOutOfBuf, res); + +#undef VMXNET3_UPDATE_RX_STATS +} + +static void +vmxnet3_hw_stats_save(struct vmxnet3_hw *hw) +{ + unsigned int i; + + VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS); + + RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES); + + for (i = 0; i < hw->num_tx_queues; i++) + vmxnet3_hw_tx_stats_get(hw, i, &hw->saved_tx_stats[i]); + for (i = 0; i < hw->num_rx_queues; i++) + vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]); +} + +static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) { unsigned int i; struct vmxnet3_hw *hw = dev->data->dev_private; + struct UPT1_TxStats txStats; + struct UPT1_RxStats rxStats; VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS); RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES); for (i = 0; i < hw->num_tx_queues; i++) { - struct UPT1_TxStats *txStats = &hw->tqd_start[i].stats; + vmxnet3_hw_tx_stats_get(hw, i, &txStats); + + stats->q_opackets[i] = txStats.ucastPktsTxOK + + txStats.mcastPktsTxOK + + txStats.bcastPktsTxOK; - stats->q_opackets[i] = txStats->ucastPktsTxOK + - txStats->mcastPktsTxOK + - txStats->bcastPktsTxOK; - stats->q_obytes[i] = txStats->ucastBytesTxOK + - txStats->mcastBytesTxOK + - txStats->bcastBytesTxOK; + stats->q_obytes[i] = txStats.ucastBytesTxOK + + txStats.mcastBytesTxOK + + txStats.bcastBytesTxOK; stats->opackets += stats->q_opackets[i]; stats->obytes += stats->q_obytes[i]; - stats->oerrors += txStats->pktsTxError + txStats->pktsTxDiscard; + stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard; } RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_RX_QUEUES); for (i = 0; i < hw->num_rx_queues; i++) { - struct UPT1_RxStats *rxStats = &hw->rqd_start[i].stats; + vmxnet3_hw_rx_stats_get(hw, i, &rxStats); - stats->q_ipackets[i] = rxStats->ucastPktsRxOK + - rxStats->mcastPktsRxOK + - rxStats->bcastPktsRxOK; + stats->q_ipackets[i] = rxStats.ucastPktsRxOK + + rxStats.mcastPktsRxOK + + rxStats.bcastPktsRxOK; - stats->q_ibytes[i] = rxStats->ucastBytesRxOK + - rxStats->mcastBytesRxOK + - rxStats->bcastBytesRxOK; + stats->q_ibytes[i] = rxStats.ucastBytesRxOK + + rxStats.mcastBytesRxOK + + rxStats.bcastBytesRxOK; stats->ipackets += stats->q_ipackets[i]; stats->ibytes += stats->q_ibytes[i]; - stats->q_errors[i] = rxStats->pktsRxError; - stats->ierrors += rxStats->pktsRxError; - stats->rx_nombuf += rxStats->pktsRxOutOfBuf; + stats->q_errors[i] = rxStats.pktsRxError; + stats->ierrors += rxStats.pktsRxError; + stats->rx_nombuf += rxStats.pktsRxOutOfBuf; } } diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h index 7a03262..e93e4a7 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h @@ -122,6 +122,8 @@ struct vmxnet3_hw { Vmxnet3_MemRegs *memRegs; uint64_t memRegsPA; #define VMXNET3_VFT_TABLE_SIZE (VMXNET3_VFT_SIZE * sizeof(uint32_t)) + UPT1_TxStats saved_tx_stats[VMXNET3_MAX_TX_QUEUES]; + UPT1_RxStats saved_rx_stats[VMXNET3_MAX_RX_QUEUES]; }; #define VMXNET3_REV_3 2 /* Vmxnet3 Rev. 3 */ -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams @ 2017-05-19 17:55 ` Charles (Chas) Williams 2017-05-24 0:17 ` Shrikrishna Khare 2017-05-19 17:55 ` [dpdk-dev] [PATCH 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams ` (7 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-05-19 17:55 UTC (permalink / raw) To: dev; +Cc: skhare, Robert Shearman From: Robert Shearman <rshearma@brocade.com> Implement xstats_get() to allow a number of driver-specific tx and rx stats to be retrieved. Signed-off-by: Robert Shearman <rshearma@brocade.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 111 +++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 4d2c024..a3378ad 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -88,6 +88,11 @@ static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw); static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); +static int vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats, + unsigned int n); +static int vmxnet3_dev_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, unsigned int n); static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static const uint32_t * @@ -122,6 +127,8 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = { .allmulticast_disable = vmxnet3_dev_allmulticast_disable, .link_update = vmxnet3_dev_link_update, .stats_get = vmxnet3_dev_stats_get, + .xstats_get_names = vmxnet3_dev_xstats_get_names, + .xstats_get = vmxnet3_dev_xstats_get, .mac_addr_set = vmxnet3_mac_addr_set, .dev_infos_get = vmxnet3_dev_info_get, .dev_supported_ptypes_get = vmxnet3_dev_supported_ptypes_get, @@ -133,6 +140,27 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = { .tx_queue_release = vmxnet3_dev_tx_queue_release, }; +struct vmxnet3_xstats_name_off { + char name[RTE_ETH_XSTATS_NAME_SIZE]; + unsigned int offset; +}; + +/* tx_qX_ is prepended to the name string here */ +static const struct vmxnet3_xstats_name_off vmxnet3_txq_stat_strings[] = { + {"drop_total", offsetof(struct vmxnet3_txq_stats, drop_total)}, + {"drop_too_many_segs", offsetof(struct vmxnet3_txq_stats, drop_too_many_segs)}, + {"drop_tso", offsetof(struct vmxnet3_txq_stats, drop_tso)}, + {"tx_ring_full", offsetof(struct vmxnet3_txq_stats, tx_ring_full)}, +}; + +/* rx_qX_ is prepended to the name string here */ +static const struct vmxnet3_xstats_name_off vmxnet3_rxq_stat_strings[] = { + {"drop_total", offsetof(struct vmxnet3_rxq_stats, drop_total)}, + {"drop_err", offsetof(struct vmxnet3_rxq_stats, drop_err)}, + {"drop_fcs", offsetof(struct vmxnet3_rxq_stats, drop_fcs)}, + {"rx_buf_alloc_failure", offsetof(struct vmxnet3_rxq_stats, rx_buf_alloc_failure)}, +}; + static const struct rte_memzone * gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t size, const char *post_string, int socket_id, @@ -882,6 +910,89 @@ vmxnet3_hw_stats_save(struct vmxnet3_hw *hw) vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]); } +static int +vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int n) +{ + unsigned int i, t, count = 0; + unsigned int nstats = + dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) + + dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings); + + if (!xstats_names || n < nstats) + return nstats; + + for (i = 0; i < dev->data->nb_rx_queues; i++) { + if (!dev->data->rx_queues[i]) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) { + snprintf(xstats_names[count].name, + sizeof(xstats_names[count].name), + "rx_q%u_%s", i, + vmxnet3_rxq_stat_strings[t].name); + count++; + } + } + + for (i = 0; i < dev->data->nb_tx_queues; i++) { + if (!dev->data->tx_queues[i]) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) { + snprintf(xstats_names[count].name, + sizeof(xstats_names[count].name), + "tx_q%u_%s", i, + vmxnet3_txq_stat_strings[t].name); + count++; + } + } + + return count; +} + +static int +vmxnet3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int i, t, count = 0; + unsigned int nstats = + dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) + + dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings); + + if (n < nstats) + return nstats; + + for (i = 0; i < dev->data->nb_rx_queues; i++) { + struct vmxnet3_rx_queue *rxq = dev->data->rx_queues[i]; + + if (rxq == NULL) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) { + xstats[count].value = *(uint64_t *)(((char *)&rxq->stats) + + vmxnet3_rxq_stat_strings[t].offset); + count++; + } + } + + for (i = 0; i < dev->data->nb_tx_queues; i++) { + struct vmxnet3_tx_queue *txq = dev->data->tx_queues[i]; + + if (txq == NULL) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) { + xstats[count].value = *(uint64_t *)(((char *)&txq->stats) + + vmxnet3_txq_stat_strings[t].offset); + count++; + } + } + + return count; +} + static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) { -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats 2017-05-19 17:55 ` [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams @ 2017-05-24 0:17 ` Shrikrishna Khare 0 siblings, 0 replies; 30+ messages in thread From: Shrikrishna Khare @ 2017-05-24 0:17 UTC (permalink / raw) To: Charles (Chas) Williams; +Cc: dev, skhare, Robert Shearman On Fri, 19 May 2017, Charles (Chas) Williams wrote: > From: Robert Shearman <rshearma@brocade.com> > > Implement xstats_get() to allow a number of driver-specific tx and rx > stats to be retrieved. > > Signed-off-by: Robert Shearman <rshearma@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 3/6] net/vmxnet3: Generate link-state change notifications 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams @ 2017-05-19 17:55 ` Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams ` (6 subsequent siblings) 8 siblings, 0 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-05-19 17:55 UTC (permalink / raw) To: dev; +Cc: skhare, Robert Shearman From: Robert Shearman <rshearma@brocade.com> Generate link-state change notifications by listening to interrupts generated by the device. Make use of the existing vmxnet3_process_events function that was compiled out, but change it to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not be so noisy in its log messages. Enable interrupts on starting the device, using a new helper function, vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated against the FreeBSD driver. Keep track of the number of interrupts registered for to avoid hardcoding these in vmxnet3_enable/disable_intr and to provision for any future rxq intr support. Factor out the guts of vmxnet3_dev_link_update minus the started check to allow the new function to be called from vmxnet3_dev_start in the lsc-enabled case to ensure that the link state is correctly set from the actual state at that point. Signed-off-by: Robert Shearman <rshearma@brocade.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 116 +++++++++++++++++++++++++++-------- drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 + 2 files changed, 91 insertions(+), 27 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index a3378ad..8f6e0fc 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -83,6 +83,8 @@ static void vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev); static void vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev); static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev); static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int __vmxnet3_dev_link_update(struct rte_eth_dev *dev, + int wait_to_complete); static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw); @@ -102,10 +104,8 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev, static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); +static void vmxnet3_interrupt_handler(void *param); -#if PROCESS_SYS_EVENTS == 1 -static void vmxnet3_process_events(struct vmxnet3_hw *); -#endif /* * The set of PCI devices this driver supports */ @@ -250,10 +250,22 @@ vmxnet3_disable_intr(struct vmxnet3_hw *hw) PMD_INIT_FUNC_TRACE(); hw->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL; - for (i = 0; i < VMXNET3_MAX_INTRS; i++) + for (i = 0; i < hw->num_intrs; i++) VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 1); } +static void +vmxnet3_enable_intr(struct vmxnet3_hw *hw) +{ + int i; + + PMD_INIT_FUNC_TRACE(); + + hw->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL; + for (i = 0; i < hw->num_intrs; i++) + VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 0); +} + /* * Gets tx data ring descriptor size. */ @@ -425,7 +437,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) static struct rte_pci_driver rte_vmxnet3_pmd = { .id_table = pci_id_vmxnet3_map, - .drv_flags = RTE_PCI_DRV_NEED_MAPPING, + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC, .probe = eth_vmxnet3_pci_probe, .remove = eth_vmxnet3_pci_remove, }; @@ -648,11 +660,11 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev) /* * Set number of interrupts to 1 - * PMD disables all the interrupts but this is MUST to activate device - * It needs at least one interrupt for link events to handle - * So we'll disable it later after device activation if needed + * PMD by default disables all the interrupts but this is MUST + * to activate device. It needs at least one interrupt for + * link events to handle */ - devRead->intrConf.numIntrs = 1; + hw->num_intrs = devRead->intrConf.numIntrs = 1; devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL; for (i = 0; i < hw->num_tx_queues; i++) { @@ -747,6 +759,20 @@ vmxnet3_dev_start(struct rte_eth_dev *dev) if (ret != VMXNET3_SUCCESS) return ret; + /* check if lsc interrupt feature is enabled */ + if (dev->data->dev_conf.intr_conf.lsc) { + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device); + + /* Setup interrupt callback */ + rte_intr_callback_register(&pci_dev->intr_handle, + vmxnet3_interrupt_handler, dev); + + if (rte_intr_enable(&pci_dev->intr_handle) < 0) { + PMD_INIT_LOG(ERR, "interrupt enable failed"); + return -EIO; + } + } + /* Exchange shared data with device */ VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, VMXNET3_GET_ADDR_LO(hw->sharedPA)); @@ -794,14 +820,19 @@ vmxnet3_dev_start(struct rte_eth_dev *dev) /* Setting proper Rx Mode and issue Rx Mode Update command */ vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1); - /* - * Don't need to handle events for now - */ -#if PROCESS_SYS_EVENTS == 1 - events = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_ECR); - PMD_INIT_LOG(DEBUG, "Reading events: 0x%X", events); - vmxnet3_process_events(hw); -#endif + if (dev->data->dev_conf.intr_conf.lsc) { + vmxnet3_enable_intr(hw); + + /* + * Update link state from device since this won't be + * done upon starting with lsc in use. This is done + * only after enabling interrupts to avoid any race + * where the link state could change without an + * interrupt being fired. + */ + __vmxnet3_dev_link_update(dev, 0); + } + return VMXNET3_SUCCESS; } @@ -824,6 +855,15 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev) /* disable interrupts */ vmxnet3_disable_intr(hw); + if (dev->data->dev_conf.intr_conf.lsc) { + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device); + + rte_intr_disable(&pci_dev->intr_handle); + + rte_intr_callback_unregister(&pci_dev->intr_handle, + vmxnet3_interrupt_handler, dev); + } + /* quiesce the device first */ VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_QUIESCE_DEV); VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, 0); @@ -1108,17 +1148,13 @@ vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) /* return 0 means link status changed, -1 means not changed */ static int -vmxnet3_dev_link_update(struct rte_eth_dev *dev, - __rte_unused int wait_to_complete) +__vmxnet3_dev_link_update(struct rte_eth_dev *dev, + __rte_unused int wait_to_complete) { struct vmxnet3_hw *hw = dev->data->dev_private; struct rte_eth_link old = { 0 }, link; uint32_t ret; - /* Link status doesn't change for stopped dev */ - if (dev->data->dev_started == 0) - return -1; - memset(&link, 0, sizeof(link)); vmxnet3_dev_atomic_read_link_status(dev, &old); @@ -1137,6 +1173,16 @@ vmxnet3_dev_link_update(struct rte_eth_dev *dev, return (old.link_status == link.link_status) ? -1 : 0; } +static int +vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete) +{ + /* Link status doesn't change for stopped dev */ + if (dev->data->dev_started == 0) + return -1; + + return __vmxnet3_dev_link_update(dev, wait_to_complete); +} + /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */ static void vmxnet3_dev_set_rxmode(struct vmxnet3_hw *hw, uint32_t feature, int set) @@ -1253,10 +1299,10 @@ vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) } } -#if PROCESS_SYS_EVENTS == 1 static void -vmxnet3_process_events(struct vmxnet3_hw *hw) +vmxnet3_process_events(struct rte_eth_dev *dev) { + struct vmxnet3_hw *hw = dev->data->dev_private; uint32_t events = hw->shared->ecr; if (!events) { @@ -1271,10 +1317,15 @@ vmxnet3_process_events(struct vmxnet3_hw *hw) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_ECR, events); /* Check if link state has changed */ - if (events & VMXNET3_ECR_LINK) + if (events & VMXNET3_ECR_LINK) { PMD_INIT_LOG(ERR, "Process events in %s(): VMXNET3_ECR_LINK event", __func__); + if (vmxnet3_dev_link_update(dev, 0) == 0) + _rte_eth_dev_callback_process(dev, + RTE_ETH_EVENT_INTR_LSC, + NULL); + } /* Check if there is an error on xmit/recv queues */ if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) { @@ -1299,7 +1350,18 @@ vmxnet3_process_events(struct vmxnet3_hw *hw) if (events & VMXNET3_ECR_DEBUG) PMD_INIT_LOG(ERR, "Debug event generated by device."); } -#endif + +static void +vmxnet3_interrupt_handler(void *param) +{ + struct rte_eth_dev *dev = param; + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device); + + vmxnet3_process_events(dev); + + if (rte_intr_enable(&pci_dev->intr_handle) < 0) + PMD_DRV_LOG(ERR, "interrupt enable failed"); +} RTE_PMD_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd); RTE_PMD_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map); diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h index e93e4a7..b48058a 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h @@ -106,6 +106,8 @@ struct vmxnet3_hw { uint16_t txdata_desc_size; /* tx data ring buffer size */ uint16_t rxdata_desc_size; /* rx data ring buffer size */ + uint8_t num_intrs; + Vmxnet3_TxQueueDesc *tqd_start; /* start address of all tx queue desc */ Vmxnet3_RxQueueDesc *rqd_start; /* start address of all rx queue desc */ -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams @ 2017-05-19 17:55 ` Charles (Chas) Williams 2017-05-23 21:44 ` Shrikrishna Khare 2017-05-19 17:55 ` [dpdk-dev] [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak Charles (Chas) Williams ` (5 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-05-19 17:55 UTC (permalink / raw) To: dev; +Cc: skhare, Robert Shearman From: Robert Shearman <rshearma@brocade.com> Make vmxnet3_process_events less noisy by removing logging when there are no events to process and by making link, device-change and debug events DEBUG level rather than ERR. Change these to use PMD_DRV_LOG instead of PMD_INIT_LOG since they don't happen at device init. Signed-off-by: Robert Shearman <rshearma@brocade.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 8f6e0fc..d241499 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -1305,10 +1305,8 @@ vmxnet3_process_events(struct rte_eth_dev *dev) struct vmxnet3_hw *hw = dev->data->dev_private; uint32_t events = hw->shared->ecr; - if (!events) { - PMD_INIT_LOG(ERR, "No events to process"); + if (!events) return; - } /* * ECR bits when written with 1b are cleared. Hence write @@ -1318,9 +1316,7 @@ vmxnet3_process_events(struct rte_eth_dev *dev) /* Check if link state has changed */ if (events & VMXNET3_ECR_LINK) { - PMD_INIT_LOG(ERR, - "Process events in %s(): VMXNET3_ECR_LINK event", - __func__); + PMD_DRV_LOG(DEBUG, "Process events: VMXNET3_ECR_LINK event"); if (vmxnet3_dev_link_update(dev, 0) == 0) _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, @@ -1333,11 +1329,11 @@ vmxnet3_process_events(struct rte_eth_dev *dev) VMXNET3_CMD_GET_QUEUE_STATUS); if (hw->tqd_start->status.stopped) - PMD_INIT_LOG(ERR, "tq error 0x%x", - hw->tqd_start->status.error); + PMD_DRV_LOG(ERR, "tq error 0x%x", + hw->tqd_start->status.error); if (hw->rqd_start->status.stopped) - PMD_INIT_LOG(ERR, "rq error 0x%x", + PMD_DRV_LOG(ERR, "rq error 0x%x", hw->rqd_start->status.error); /* Reset the device */ @@ -1345,10 +1341,10 @@ vmxnet3_process_events(struct rte_eth_dev *dev) } if (events & VMXNET3_ECR_DIC) - PMD_INIT_LOG(ERR, "Device implementation change event."); + PMD_DRV_LOG(DEBUG, "Device implementation change event."); if (events & VMXNET3_ECR_DEBUG) - PMD_INIT_LOG(ERR, "Debug event generated by device."); + PMD_DRV_LOG(DEBUG, "Debug event generated by device."); } static void -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy 2017-05-19 17:55 ` [dpdk-dev] [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams @ 2017-05-23 21:44 ` Shrikrishna Khare 0 siblings, 0 replies; 30+ messages in thread From: Shrikrishna Khare @ 2017-05-23 21:44 UTC (permalink / raw) To: Charles (Chas) Williams; +Cc: dev, skhare, Robert Shearman On Fri, 19 May 2017, Charles (Chas) Williams wrote: > From: Robert Shearman <rshearma@brocade.com> > > Make vmxnet3_process_events less noisy by removing logging when there > are no events to process and by making link, device-change and debug > events DEBUG level rather than ERR. > > Change these to use PMD_DRV_LOG instead of PMD_INIT_LOG since they > don't happen at device init. > > Signed-off-by: Robert Shearman <rshearma@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (2 preceding siblings ...) 2017-05-19 17:55 ` [dpdk-dev] [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams @ 2017-05-19 17:55 ` Charles (Chas) Williams 2017-06-01 12:24 ` Charles (Chas) Williams 2017-05-24 21:09 ` [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Shrikrishna Khare ` (4 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-05-19 17:55 UTC (permalink / raw) To: dev; +Cc: skhare, Mandeep Rohilla From: Mandeep Rohilla <mrohilla@brocade.com> The receive queue can lockup if all the rx descriptors have lost their mbufs and temporarily there are no mbufs available. This can happen if there is an mbuf leak or if the application holds on to the mbuf for a while. This also addresses an mbuf leak in an error condition during packet receive. Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com> --- drivers/net/vmxnet3/vmxnet3_rxtx.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c index d8713a1..d21679d 100644 --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c @@ -731,6 +731,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) uint16_t nb_rx; uint32_t nb_rxd, idx; uint8_t ring_idx; + uint8_t i; vmxnet3_rx_queue_t *rxq; Vmxnet3_RxCompDesc *rcd; vmxnet3_buf_info_t *rbi; @@ -800,6 +801,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) (int)(rcd - (struct Vmxnet3_RxCompDesc *) rxq->comp_ring.base), rcd->rxdIdx); rte_pktmbuf_free_seg(rxm); + if (rxq->start_seg) { + struct rte_mbuf *start = rxq->start_seg; + + rxq->start_seg = NULL; + rte_pktmbuf_free(start); + } goto rcd_done; } @@ -893,6 +900,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) } } + /* + * Try to replenish the rx descriptors with the new mbufs + */ + for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++) { + vmxnet3_post_rx_bufs(rxq, i); + if (unlikely(rxq->shared->ctrl.updateRxProd)) { + VMXNET3_WRITE_BAR0_REG(hw, + rxprod_reg[i] + + (rxq->queue_id * VMXNET3_REG_ALIGN), + rxq->cmd_ring[i].next2fill); + } + } return nb_rx; } -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak 2017-05-19 17:55 ` [dpdk-dev] [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak Charles (Chas) Williams @ 2017-06-01 12:24 ` Charles (Chas) Williams 0 siblings, 0 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-01 12:24 UTC (permalink / raw) To: dev; +Cc: skhare, mrohilla While looking at another issue, I think one of the issues fixed in this commit has already been fixed in the last DPDK release by: commit 8fce14b789aecdb4345a62f6980e7b6e7f4ba245 Author: Stefan Puiu <stefan.puiu@gmail.com> Date: Mon Dec 19 11:40:53 2016 +0200 net/vmxnet3: fix Rx deadlock Our use case is that we have an app that needs to keep mbufs around for a while. We've seen cases when calling vmxnet3_post_rx_bufs() from vmxet3_recv_pkts(), it might not succeed to add any mbufs to any RX descriptors (where it returns -err). Since there are no mbufs that the virtual hardware can use, no packets will be received after this; the driver won't refill the mbuf after this so it gets stuck in this state. I call this a deadlock for lack of a better term - the virtual HW waits for free mbufs, while the app waits for the hardware to notify it for data (by flipping the generation bit on the used Rx descriptors). Note that after this, the app can't recover. ... The mbuf leak due to an error during receive still exists. That can be refactored into a new commit. On 05/19/2017 01:55 PM, Charles (Chas) Williams wrote: > From: Mandeep Rohilla <mrohilla@brocade.com> > > The receive queue can lockup if all the rx descriptors have lost > their mbufs and temporarily there are no mbufs available. This > can happen if there is an mbuf leak or if the application holds > on to the mbuf for a while. > > This also addresses an mbuf leak in an error condition during > packet receive. > > Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com> > --- > drivers/net/vmxnet3/vmxnet3_rxtx.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c > index d8713a1..d21679d 100644 > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > @@ -731,6 +731,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > uint16_t nb_rx; > uint32_t nb_rxd, idx; > uint8_t ring_idx; > + uint8_t i; > vmxnet3_rx_queue_t *rxq; > Vmxnet3_RxCompDesc *rcd; > vmxnet3_buf_info_t *rbi; > @@ -800,6 +801,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > (int)(rcd - (struct Vmxnet3_RxCompDesc *) > rxq->comp_ring.base), rcd->rxdIdx); > rte_pktmbuf_free_seg(rxm); > + if (rxq->start_seg) { > + struct rte_mbuf *start = rxq->start_seg; > + > + rxq->start_seg = NULL; > + rte_pktmbuf_free(start); > + } > goto rcd_done; > } > > @@ -893,6 +900,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > } > } > > + /* > + * Try to replenish the rx descriptors with the new mbufs > + */ > + for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++) { > + vmxnet3_post_rx_bufs(rxq, i); > + if (unlikely(rxq->shared->ctrl.updateRxProd)) { > + VMXNET3_WRITE_BAR0_REG(hw, > + rxprod_reg[i] + > + (rxq->queue_id * VMXNET3_REG_ALIGN), > + rxq->cmd_ring[i].next2fill); > + } > + } > return nb_rx; > } > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (3 preceding siblings ...) 2017-05-19 17:55 ` [dpdk-dev] [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak Charles (Chas) Williams @ 2017-05-24 21:09 ` Shrikrishna Khare 2017-05-25 18:31 ` Nachi Prachanda 2017-05-26 17:31 ` Shrikrishna Khare ` (3 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Shrikrishna Khare @ 2017-05-24 21:09 UTC (permalink / raw) To: Charles (Chas) Williams; +Cc: dev, skhare, Nachiketa Prachanda On Fri, 19 May 2017, Charles (Chas) Williams wrote: > From: Nachiketa Prachanda <nprachan@brocade.com> > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > dev_start and arguably this helps in monitoring the counters > across a longer time span with multiple device start/stops. > vmxnet3 behavior is opposite to that and counters are reset by > the host side implementation each time the device is restarted. > > Change the driver to save the counters in its private context > before it is reset by writing CMD_ACTIVATE to REG_CMD. > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> This won't be able to deal with vMotion or suspend/resume? ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-24 21:09 ` [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Shrikrishna Khare @ 2017-05-25 18:31 ` Nachi Prachanda 2017-05-25 20:27 ` Shrikrishna Khare 0 siblings, 1 reply; 30+ messages in thread From: Nachi Prachanda @ 2017-05-25 18:31 UTC (permalink / raw) To: Shrikrishna Khare, Chas Williams III; +Cc: dev, skhare > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > Sent: Wednesday, May 24, 2017 2:10 PM > > On Fri, 19 May 2017, Charles (Chas) Williams wrote: > > > From: Nachiketa Prachanda <nprachan@brocade.com> > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > > dev_start and arguably this helps in monitoring the counters across a > > longer time span with multiple device start/stops. > > vmxnet3 behavior is opposite to that and counters are reset by the > > host side implementation each time the device is restarted. > > > > Change the driver to save the counters in its private context before > > it is reset by writing CMD_ACTIVATE to REG_CMD. > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> > > This won't be able to deal with vMotion or suspend/resume? Correct - this can't deal with the VM suspend/resume unless hypervisor maintains the counter. But this patch doesn't make that behavior any worse than what it was before. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-25 18:31 ` Nachi Prachanda @ 2017-05-25 20:27 ` Shrikrishna Khare 2017-05-25 22:08 ` Nachi Prachanda 0 siblings, 1 reply; 30+ messages in thread From: Shrikrishna Khare @ 2017-05-25 20:27 UTC (permalink / raw) To: Nachi Prachanda; +Cc: Chas Williams III, dev, skhare On Thu, 25 May 2017, Nachi Prachanda wrote: > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > > Sent: Wednesday, May 24, 2017 2:10 PM > > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote: > > > > > From: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > > > dev_start and arguably this helps in monitoring the counters across a > > > longer time span with multiple device start/stops. > > > vmxnet3 behavior is opposite to that and counters are reset by the > > > host side implementation each time the device is restarted. > > > > > > Change the driver to save the counters in its private context before > > > it is reset by writing CMD_ACTIVATE to REG_CMD. > > > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> > > > > This won't be able to deal with vMotion or suspend/resume? > > Correct - this can't deal with the VM suspend/resume unless hypervisor maintains the counter. But this patch doesn't make that behavior any worse than what it was before. The current code always resets stats, but am concerned that this patch will make the behavior inconsistent for cases like suspend/resume. Wondering if this will be better handled by the device emulation instead of the driver (for igb/ixgbe, is this handled by the hardware?). If we were to handle this in the device emulation, what would be the goals/requirements: - device start/stop should not reset stats? - any other operations where we would like to maintain/reset stats? - what might be the expectation around how accurate the stats need to be? - any other requirement on the device? Also, note that if we proceed with this patch, and later extend device support to not reset stats, driver with this patch running on the extended device will report incorrect stats. Thanks, Shri ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-25 20:27 ` Shrikrishna Khare @ 2017-05-25 22:08 ` Nachi Prachanda 2017-05-26 17:29 ` Shrikrishna Khare 0 siblings, 1 reply; 30+ messages in thread From: Nachi Prachanda @ 2017-05-25 22:08 UTC (permalink / raw) To: skhare; +Cc: Chas Williams III, dev > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > Sent: Thursday, May 25, 2017 1:27 PM > > On Thu, 25 May 2017, Nachi Prachanda wrote: > > > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > > > Sent: Wednesday, May 24, 2017 2:10 PM > > > > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote: > > > > > > > From: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > > > > dev_start and arguably this helps in monitoring the counters > > > > across a longer time span with multiple device start/stops. > > > > vmxnet3 behavior is opposite to that and counters are reset by the > > > > host side implementation each time the device is restarted. > > > > > > > > Change the driver to save the counters in its private context > > > > before it is reset by writing CMD_ACTIVATE to REG_CMD. > > > > > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > This won't be able to deal with vMotion or suspend/resume? > > > > Correct - this can't deal with the VM suspend/resume unless hypervisor > maintains the counter. But this patch doesn't make that behavior any worse > than what it was before. > > The current code always resets stats, but am concerned that this patch will > make the behavior inconsistent for cases like suspend/resume. > > Wondering if this will be better handled by the device emulation instead of the > driver (for igb/ixgbe, is this handled by the hardware?). A little more nuanced.. - see below. > If we were to handle this in the device emulation, what would be the > goals/requirements: > - device start/stop should not reset stats? > - any other operations where we would like to maintain/reset stats? > - what might be the expectation around how accurate the stats need to be? > - any other requirement on the device? I haven't thought about dealing it at the emulation layer - but the expectation would be not clear counters not cleared for the lifetime of the device - and have a way to clear them from the driver when needed. don't know if there is a standard behavior about resetting counters. But for igb/ixgb the counters are read/clear registers and they are maintained at driver. May not be always accurate if the hardware is reset without updating the driver's counter - but at least ensures that it is monotonically increasing since on each read driver only gets the delta. virtio emulation doesn't provide the counters - mostly the receive/send functions updates the counters. > > Also, note that if we proceed with this patch, and later extend device support > to not reset stats, driver with this patch running on the extended device will > report incorrect stats. Agree - it will need some work if the emulation changes. Regards, Nachi > Thanks, > Shri ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-25 22:08 ` Nachi Prachanda @ 2017-05-26 17:29 ` Shrikrishna Khare 2017-05-26 19:01 ` Nachi Prachanda 0 siblings, 1 reply; 30+ messages in thread From: Shrikrishna Khare @ 2017-05-26 17:29 UTC (permalink / raw) To: Nachi Prachanda; +Cc: skhare, Chas Williams III, dev On Thu, 25 May 2017, Nachi Prachanda wrote: > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > > Sent: Thursday, May 25, 2017 1:27 PM > > > > On Thu, 25 May 2017, Nachi Prachanda wrote: > > > > > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > > > > Sent: Wednesday, May 24, 2017 2:10 PM > > > > > > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote: > > > > > > > > > From: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > > > > > dev_start and arguably this helps in monitoring the counters > > > > > across a longer time span with multiple device start/stops. > > > > > vmxnet3 behavior is opposite to that and counters are reset by the > > > > > host side implementation each time the device is restarted. > > > > > > > > > > Change the driver to save the counters in its private context > > > > > before it is reset by writing CMD_ACTIVATE to REG_CMD. > > > > > > > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > > > This won't be able to deal with vMotion or suspend/resume? > > > > > > Correct - this can't deal with the VM suspend/resume unless hypervisor > > maintains the counter. But this patch doesn't make that behavior any worse > > than what it was before. > > > > The current code always resets stats, but am concerned that this patch will > > make the behavior inconsistent for cases like suspend/resume. > > > > Wondering if this will be better handled by the device emulation instead of the > > driver (for igb/ixgbe, is this handled by the hardware?). > > A little more nuanced.. - see below. > > > If we were to handle this in the device emulation, what would be the > > goals/requirements: > > - device start/stop should not reset stats? > > - any other operations where we would like to maintain/reset stats? > > - what might be the expectation around how accurate the stats need to be? > > - any other requirement on the device? > > I haven't thought about dealing it at the emulation layer - but the expectation > would be not clear counters not cleared for the lifetime of the device - and have > a way to clear them from the driver when needed. > > don't know if there is a standard behavior about resetting counters. But > for igb/ixgb the counters are read/clear registers and they are maintained > at driver. May not be always accurate if the hardware is reset without updating > the driver's counter - but at least ensures that it is monotonically increasing since > on each read driver only gets the delta. > > virtio emulation doesn't provide the counters - mostly the receive/send functions > updates the counters. > > > > > Also, note that if we proceed with this patch, and later extend device support > > to not reset stats, driver with this patch running on the extended device will > > report incorrect stats. > > Agree - it will need some work if the emulation changes. I spent some time looking at the emulation code. It turns out that these stats are already retained across vMotion, suspend/resume. And we have no immediate plan to change the device behavior to retain these stats across device start/stop. So am fine with this patch. Thank you for being patient with this. Thanks, Shri > > Regards, > Nachi > > > Thanks, > > Shri > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-26 17:29 ` Shrikrishna Khare @ 2017-05-26 19:01 ` Nachi Prachanda 0 siblings, 0 replies; 30+ messages in thread From: Nachi Prachanda @ 2017-05-26 19:01 UTC (permalink / raw) To: skhare; +Cc: Chas Williams III, dev > -----Original Message----- > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > Sent: Friday, May 26, 2017 10:29 AM > To: Nachi Prachanda > Cc: skhare@vmware.com; Chas Williams III; dev@dpdk.org > Subject: RE: [PATCH 1/6] net/vmxnet3: retain counters on restart > > > > On Thu, 25 May 2017, Nachi Prachanda wrote: > > > > From: Shrikrishna Khare [mailto:skhare@shri-linux.eng.vmware.com] > > > Sent: Thursday, May 25, 2017 1:27 PM > > > > > > On Thu, 25 May 2017, Nachi Prachanda wrote: > > > > > > > > From: Shrikrishna Khare > > > > > [mailto:skhare@shri-linux.eng.vmware.com] > > > > > Sent: Wednesday, May 24, 2017 2:10 PM > > > > > > > > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote: > > > > > > > > > > > From: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > > > > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > > > > > > dev_start and arguably this helps in monitoring the counters > > > > > > across a longer time span with multiple device start/stops. > > > > > > vmxnet3 behavior is opposite to that and counters are reset by > > > > > > the host side implementation each time the device is restarted. > > > > > > > > > > > > Change the driver to save the counters in its private context > > > > > > before it is reset by writing CMD_ACTIVATE to REG_CMD. > > > > > > > > > > > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> > > > > > > > > > > This won't be able to deal with vMotion or suspend/resume? > > > > > > > > Correct - this can't deal with the VM suspend/resume unless > > > > hypervisor > > > maintains the counter. But this patch doesn't make that behavior any > > > worse than what it was before. > > > > > > The current code always resets stats, but am concerned that this > > > patch will make the behavior inconsistent for cases like suspend/resume. > > > > > > Wondering if this will be better handled by the device emulation > > > instead of the driver (for igb/ixgbe, is this handled by the hardware?). > > > > A little more nuanced.. - see below. > > > > > If we were to handle this in the device emulation, what would be the > > > goals/requirements: > > > - device start/stop should not reset stats? > > > - any other operations where we would like to maintain/reset stats? > > > - what might be the expectation around how accurate the stats need to > be? > > > - any other requirement on the device? > > > > I haven't thought about dealing it at the emulation layer - but the > > expectation would be not clear counters not cleared for the lifetime > > of the device - and have a way to clear them from the driver when needed. > > > > don't know if there is a standard behavior about resetting counters. > > But for igb/ixgb the counters are read/clear registers and they are > > maintained at driver. May not be always accurate if the hardware is > > reset without updating the driver's counter - but at least ensures > > that it is monotonically increasing since on each read driver only gets the > delta. > > > > virtio emulation doesn't provide the counters - mostly the > > receive/send functions updates the counters. > > > > > > > > Also, note that if we proceed with this patch, and later extend > > > device support to not reset stats, driver with this patch running on > > > the extended device will report incorrect stats. > > > > Agree - it will need some work if the emulation changes. > > I spent some time looking at the emulation code. It turns out that these stats > are already retained across vMotion, suspend/resume. And we have no > immediate plan to change the device behavior to retain these stats across > device start/stop. So am fine with this patch. Thank you for being patient with > this. Thanks for checking this out. Nachi > > Thanks, > Shri > > > > > > Regards, > > Nachi > > > > > Thanks, > > > Shri > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (4 preceding siblings ...) 2017-05-24 21:09 ` [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Shrikrishna Khare @ 2017-05-26 17:31 ` Shrikrishna Khare 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams ` (2 subsequent siblings) 8 siblings, 0 replies; 30+ messages in thread From: Shrikrishna Khare @ 2017-05-26 17:31 UTC (permalink / raw) To: Charles (Chas) Williams; +Cc: dev, skhare, Nachiketa Prachanda On Fri, 19 May 2017, Charles (Chas) Williams wrote: > From: Nachiketa Prachanda <nprachan@brocade.com> > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > dev_start and arguably this helps in monitoring the counters > across a longer time span with multiple device start/stops. > vmxnet3 behavior is opposite to that and counters are reset by > the host side implementation each time the device is restarted. > > Change the driver to save the counters in its private context > before it is reset by writing CMD_ACTIVATE to REG_CMD. > > Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (5 preceding siblings ...) 2017-05-26 17:31 ` Shrikrishna Khare @ 2017-06-15 12:16 ` Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (4 more replies) 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 5/6] net/vmxnet3: receive queue memory leak Charles (Chas) Williams 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 6/6] net/vmxnet3: preserve configured MAC address Charles (Chas) Williams 8 siblings, 5 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:16 UTC (permalink / raw) To: dev; +Cc: skhare This series addresses some local issues with the vmxnet3 driver that others might find of interest. Changes in v2: - net/vmxnet3: Implement retrieval of extended stats .id field wasn't being filled in xstats. - net/vmxnet3: receive queue memory leak The leak and buffer reallcation were split. The buffer reallocation patch may be superceded by another commit so isn't included in this series while under discussion. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 1/6] net/vmxnet3: retain counters on restart 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams @ 2017-06-15 12:16 ` Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams ` (3 subsequent siblings) 4 siblings, 0 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:16 UTC (permalink / raw) To: dev; +Cc: skhare, Nachiketa Prachanda From: Nachiketa Prachanda <nprachan@brocade.com> Most nics like virtio, igb/ixgbe etc. don't reset counters on dev_start and arguably this helps in monitoring the counters across a longer time span with multiple device start/stops. vmxnet3 behavior is opposite to that and counters are reset by the host side implementation each time the device is restarted. Change the driver to save the counters in its private context before it is reset by writing CMD_ACTIVATE to REG_CMD. Signed-off-by: Nachiketa Prachanda <nprachan@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 102 ++++++++++++++++++++++++++++------- drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 + 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 292a671..1301f50 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -85,6 +85,7 @@ static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev); static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev); static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw); static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, @@ -351,6 +352,10 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev) RTE_ASSERT((hw->rxdata_desc_size & ~VMXNET3_RXDATA_DESC_SIZE_MASK) == hw->rxdata_desc_size); + /* clear shadow stats */ + memset(hw->saved_tx_stats, 0, sizeof(hw->saved_tx_stats)); + memset(hw->saved_rx_stats, 0, sizeof(hw->saved_rx_stats)); + return 0; } @@ -707,6 +712,9 @@ vmxnet3_dev_start(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); + /* Save stats before it is reset by CMD_ACTIVATE */ + vmxnet3_hw_stats_save(hw); + ret = vmxnet3_setup_driver_shared(dev); if (ret != VMXNET3_SUCCESS) return ret; @@ -820,47 +828,105 @@ vmxnet3_dev_close(struct rte_eth_dev *dev) } static void +vmxnet3_hw_tx_stats_get(struct vmxnet3_hw *hw, unsigned int q, + struct UPT1_TxStats *res) +{ +#define VMXNET3_UPDATE_TX_STAT(h, i, f, r) \ + ((r)->f = (h)->tqd_start[(i)].stats.f + \ + (h)->saved_tx_stats[(i)].f) + + VMXNET3_UPDATE_TX_STAT(hw, q, ucastPktsTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, mcastPktsTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, bcastPktsTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, ucastBytesTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, mcastBytesTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, bcastBytesTxOK, res); + VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxError, res); + VMXNET3_UPDATE_TX_STAT(hw, q, pktsTxDiscard, res); + +#undef VMXNET3_UPDATE_TX_STAT +} + +static void +vmxnet3_hw_rx_stats_get(struct vmxnet3_hw *hw, unsigned int q, + struct UPT1_RxStats *res) +{ +#define VMXNET3_UPDATE_RX_STAT(h, i, f, r) \ + ((r)->f = (h)->rqd_start[(i)].stats.f + \ + (h)->saved_rx_stats[(i)].f) + + VMXNET3_UPDATE_RX_STAT(hw, q, ucastPktsRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, mcastPktsRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, bcastPktsRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, ucastBytesRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, mcastBytesRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, bcastBytesRxOK, res); + VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxError, res); + VMXNET3_UPDATE_RX_STAT(hw, q, pktsRxOutOfBuf, res); + +#undef VMXNET3_UPDATE_RX_STATS +} + +static void +vmxnet3_hw_stats_save(struct vmxnet3_hw *hw) +{ + unsigned int i; + + VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS); + + RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES); + + for (i = 0; i < hw->num_tx_queues; i++) + vmxnet3_hw_tx_stats_get(hw, i, &hw->saved_tx_stats[i]); + for (i = 0; i < hw->num_rx_queues; i++) + vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]); +} + +static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) { unsigned int i; struct vmxnet3_hw *hw = dev->data->dev_private; + struct UPT1_TxStats txStats; + struct UPT1_RxStats rxStats; VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS); RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_TX_QUEUES); for (i = 0; i < hw->num_tx_queues; i++) { - struct UPT1_TxStats *txStats = &hw->tqd_start[i].stats; + vmxnet3_hw_tx_stats_get(hw, i, &txStats); + + stats->q_opackets[i] = txStats.ucastPktsTxOK + + txStats.mcastPktsTxOK + + txStats.bcastPktsTxOK; - stats->q_opackets[i] = txStats->ucastPktsTxOK + - txStats->mcastPktsTxOK + - txStats->bcastPktsTxOK; - stats->q_obytes[i] = txStats->ucastBytesTxOK + - txStats->mcastBytesTxOK + - txStats->bcastBytesTxOK; + stats->q_obytes[i] = txStats.ucastBytesTxOK + + txStats.mcastBytesTxOK + + txStats.bcastBytesTxOK; stats->opackets += stats->q_opackets[i]; stats->obytes += stats->q_obytes[i]; - stats->oerrors += txStats->pktsTxError + txStats->pktsTxDiscard; + stats->oerrors += txStats.pktsTxError + txStats.pktsTxDiscard; } RTE_BUILD_BUG_ON(RTE_ETHDEV_QUEUE_STAT_CNTRS < VMXNET3_MAX_RX_QUEUES); for (i = 0; i < hw->num_rx_queues; i++) { - struct UPT1_RxStats *rxStats = &hw->rqd_start[i].stats; + vmxnet3_hw_rx_stats_get(hw, i, &rxStats); - stats->q_ipackets[i] = rxStats->ucastPktsRxOK + - rxStats->mcastPktsRxOK + - rxStats->bcastPktsRxOK; + stats->q_ipackets[i] = rxStats.ucastPktsRxOK + + rxStats.mcastPktsRxOK + + rxStats.bcastPktsRxOK; - stats->q_ibytes[i] = rxStats->ucastBytesRxOK + - rxStats->mcastBytesRxOK + - rxStats->bcastBytesRxOK; + stats->q_ibytes[i] = rxStats.ucastBytesRxOK + + rxStats.mcastBytesRxOK + + rxStats.bcastBytesRxOK; stats->ipackets += stats->q_ipackets[i]; stats->ibytes += stats->q_ibytes[i]; - stats->q_errors[i] = rxStats->pktsRxError; - stats->ierrors += rxStats->pktsRxError; - stats->rx_nombuf += rxStats->pktsRxOutOfBuf; + stats->q_errors[i] = rxStats.pktsRxError; + stats->ierrors += rxStats.pktsRxError; + stats->rx_nombuf += rxStats.pktsRxOutOfBuf; } } diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h index 7a03262..e93e4a7 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h @@ -122,6 +122,8 @@ struct vmxnet3_hw { Vmxnet3_MemRegs *memRegs; uint64_t memRegsPA; #define VMXNET3_VFT_TABLE_SIZE (VMXNET3_VFT_SIZE * sizeof(uint32_t)) + UPT1_TxStats saved_tx_stats[VMXNET3_MAX_TX_QUEUES]; + UPT1_RxStats saved_rx_stats[VMXNET3_MAX_RX_QUEUES]; }; #define VMXNET3_REV_3 2 /* Vmxnet3 Rev. 3 */ -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams @ 2017-06-15 12:16 ` Charles (Chas) Williams 2017-06-21 1:42 ` Shrikrishna Khare 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams ` (2 subsequent siblings) 4 siblings, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:16 UTC (permalink / raw) To: dev; +Cc: skhare, Robert Shearman From: Robert Shearman <rshearma@brocade.com> Implement xstats_get() to allow a number of driver-specific tx and rx stats to be retrieved. Signed-off-by: Robert Shearman <rshearma@brocade.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 113 +++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 1301f50..f5a9832 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -88,6 +88,11 @@ static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw); static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); +static int vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats, + unsigned int n); +static int vmxnet3_dev_xstats_get(struct rte_eth_dev *dev, + struct rte_eth_xstat *xstats, unsigned int n); static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static const uint32_t * @@ -122,6 +127,8 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = { .allmulticast_disable = vmxnet3_dev_allmulticast_disable, .link_update = vmxnet3_dev_link_update, .stats_get = vmxnet3_dev_stats_get, + .xstats_get_names = vmxnet3_dev_xstats_get_names, + .xstats_get = vmxnet3_dev_xstats_get, .mac_addr_set = vmxnet3_mac_addr_set, .dev_infos_get = vmxnet3_dev_info_get, .dev_supported_ptypes_get = vmxnet3_dev_supported_ptypes_get, @@ -133,6 +140,27 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = { .tx_queue_release = vmxnet3_dev_tx_queue_release, }; +struct vmxnet3_xstats_name_off { + char name[RTE_ETH_XSTATS_NAME_SIZE]; + unsigned int offset; +}; + +/* tx_qX_ is prepended to the name string here */ +static const struct vmxnet3_xstats_name_off vmxnet3_txq_stat_strings[] = { + {"drop_total", offsetof(struct vmxnet3_txq_stats, drop_total)}, + {"drop_too_many_segs", offsetof(struct vmxnet3_txq_stats, drop_too_many_segs)}, + {"drop_tso", offsetof(struct vmxnet3_txq_stats, drop_tso)}, + {"tx_ring_full", offsetof(struct vmxnet3_txq_stats, tx_ring_full)}, +}; + +/* rx_qX_ is prepended to the name string here */ +static const struct vmxnet3_xstats_name_off vmxnet3_rxq_stat_strings[] = { + {"drop_total", offsetof(struct vmxnet3_rxq_stats, drop_total)}, + {"drop_err", offsetof(struct vmxnet3_rxq_stats, drop_err)}, + {"drop_fcs", offsetof(struct vmxnet3_rxq_stats, drop_fcs)}, + {"rx_buf_alloc_failure", offsetof(struct vmxnet3_rxq_stats, rx_buf_alloc_failure)}, +}; + static const struct rte_memzone * gpa_zone_reserve(struct rte_eth_dev *dev, uint32_t size, const char *post_string, int socket_id, @@ -882,6 +910,91 @@ vmxnet3_hw_stats_save(struct vmxnet3_hw *hw) vmxnet3_hw_rx_stats_get(hw, i, &hw->saved_rx_stats[i]); } +static int +vmxnet3_dev_xstats_get_names(struct rte_eth_dev *dev, + struct rte_eth_xstat_name *xstats_names, + unsigned int n) +{ + unsigned int i, t, count = 0; + unsigned int nstats = + dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) + + dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings); + + if (!xstats_names || n < nstats) + return nstats; + + for (i = 0; i < dev->data->nb_rx_queues; i++) { + if (!dev->data->rx_queues[i]) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) { + snprintf(xstats_names[count].name, + sizeof(xstats_names[count].name), + "rx_q%u_%s", i, + vmxnet3_rxq_stat_strings[t].name); + count++; + } + } + + for (i = 0; i < dev->data->nb_tx_queues; i++) { + if (!dev->data->tx_queues[i]) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) { + snprintf(xstats_names[count].name, + sizeof(xstats_names[count].name), + "tx_q%u_%s", i, + vmxnet3_txq_stat_strings[t].name); + count++; + } + } + + return count; +} + +static int +vmxnet3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats, + unsigned int n) +{ + unsigned int i, t, count = 0; + unsigned int nstats = + dev->data->nb_tx_queues * RTE_DIM(vmxnet3_txq_stat_strings) + + dev->data->nb_rx_queues * RTE_DIM(vmxnet3_rxq_stat_strings); + + if (n < nstats) + return nstats; + + for (i = 0; i < dev->data->nb_rx_queues; i++) { + struct vmxnet3_rx_queue *rxq = dev->data->rx_queues[i]; + + if (rxq == NULL) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_rxq_stat_strings); t++) { + xstats[count].value = *(uint64_t *)(((char *)&rxq->stats) + + vmxnet3_rxq_stat_strings[t].offset); + xstats[count].id = count; + count++; + } + } + + for (i = 0; i < dev->data->nb_tx_queues; i++) { + struct vmxnet3_tx_queue *txq = dev->data->tx_queues[i]; + + if (txq == NULL) + continue; + + for (t = 0; t < RTE_DIM(vmxnet3_txq_stat_strings); t++) { + xstats[count].value = *(uint64_t *)(((char *)&txq->stats) + + vmxnet3_txq_stat_strings[t].offset); + xstats[count].id = count; + count++; + } + } + + return count; +} + static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats) { -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams @ 2017-06-21 1:42 ` Shrikrishna Khare 0 siblings, 0 replies; 30+ messages in thread From: Shrikrishna Khare @ 2017-06-21 1:42 UTC (permalink / raw) To: Charles (Chas) Williams; +Cc: dev, skhare, Robert Shearman On Thu, 15 Jun 2017, Charles (Chas) Williams wrote: > From: Robert Shearman <rshearma@brocade.com> > > Implement xstats_get() to allow a number of driver-specific tx and rx > stats to be retrieved. > > Signed-off-by: Robert Shearman <rshearma@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams @ 2017-06-15 12:16 ` Charles (Chas) Williams 2017-06-27 13:52 ` Ferruh Yigit 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams 2017-06-28 11:30 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Ferruh Yigit 4 siblings, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:16 UTC (permalink / raw) To: dev; +Cc: skhare, Robert Shearman From: Robert Shearman <rshearma@brocade.com> Generate link-state change notifications by listening to interrupts generated by the device. Make use of the existing vmxnet3_process_events function that was compiled out, but change it to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not be so noisy in its log messages. Enable interrupts on starting the device, using a new helper function, vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated against the FreeBSD driver. Keep track of the number of interrupts registered for to avoid hardcoding these in vmxnet3_enable/disable_intr and to provision for any future rxq intr support. Factor out the guts of vmxnet3_dev_link_update minus the started check to allow the new function to be called from vmxnet3_dev_start in the lsc-enabled case to ensure that the link state is correctly set from the actual state at that point. Signed-off-by: Robert Shearman <rshearma@brocade.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 116 +++++++++++++++++++++++++++-------- drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 + 2 files changed, 91 insertions(+), 27 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index f5a9832..73277fb 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -83,6 +83,8 @@ static void vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev); static void vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev); static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev); static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int __vmxnet3_dev_link_update(struct rte_eth_dev *dev, + int wait_to_complete); static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw); @@ -102,10 +104,8 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev, static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); +static void vmxnet3_interrupt_handler(void *param); -#if PROCESS_SYS_EVENTS == 1 -static void vmxnet3_process_events(struct vmxnet3_hw *); -#endif /* * The set of PCI devices this driver supports */ @@ -250,10 +250,22 @@ vmxnet3_disable_intr(struct vmxnet3_hw *hw) PMD_INIT_FUNC_TRACE(); hw->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL; - for (i = 0; i < VMXNET3_MAX_INTRS; i++) + for (i = 0; i < hw->num_intrs; i++) VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 1); } +static void +vmxnet3_enable_intr(struct vmxnet3_hw *hw) +{ + int i; + + PMD_INIT_FUNC_TRACE(); + + hw->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL; + for (i = 0; i < hw->num_intrs; i++) + VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 0); +} + /* * Gets tx data ring descriptor size. */ @@ -425,7 +437,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) static struct rte_pci_driver rte_vmxnet3_pmd = { .id_table = pci_id_vmxnet3_map, - .drv_flags = RTE_PCI_DRV_NEED_MAPPING, + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC, .probe = eth_vmxnet3_pci_probe, .remove = eth_vmxnet3_pci_remove, }; @@ -648,11 +660,11 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev) /* * Set number of interrupts to 1 - * PMD disables all the interrupts but this is MUST to activate device - * It needs at least one interrupt for link events to handle - * So we'll disable it later after device activation if needed + * PMD by default disables all the interrupts but this is MUST + * to activate device. It needs at least one interrupt for + * link events to handle */ - devRead->intrConf.numIntrs = 1; + hw->num_intrs = devRead->intrConf.numIntrs = 1; devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL; for (i = 0; i < hw->num_tx_queues; i++) { @@ -747,6 +759,20 @@ vmxnet3_dev_start(struct rte_eth_dev *dev) if (ret != VMXNET3_SUCCESS) return ret; + /* check if lsc interrupt feature is enabled */ + if (dev->data->dev_conf.intr_conf.lsc) { + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device); + + /* Setup interrupt callback */ + rte_intr_callback_register(&pci_dev->intr_handle, + vmxnet3_interrupt_handler, dev); + + if (rte_intr_enable(&pci_dev->intr_handle) < 0) { + PMD_INIT_LOG(ERR, "interrupt enable failed"); + return -EIO; + } + } + /* Exchange shared data with device */ VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, VMXNET3_GET_ADDR_LO(hw->sharedPA)); @@ -794,14 +820,19 @@ vmxnet3_dev_start(struct rte_eth_dev *dev) /* Setting proper Rx Mode and issue Rx Mode Update command */ vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1); - /* - * Don't need to handle events for now - */ -#if PROCESS_SYS_EVENTS == 1 - events = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_ECR); - PMD_INIT_LOG(DEBUG, "Reading events: 0x%X", events); - vmxnet3_process_events(hw); -#endif + if (dev->data->dev_conf.intr_conf.lsc) { + vmxnet3_enable_intr(hw); + + /* + * Update link state from device since this won't be + * done upon starting with lsc in use. This is done + * only after enabling interrupts to avoid any race + * where the link state could change without an + * interrupt being fired. + */ + __vmxnet3_dev_link_update(dev, 0); + } + return VMXNET3_SUCCESS; } @@ -824,6 +855,15 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev) /* disable interrupts */ vmxnet3_disable_intr(hw); + if (dev->data->dev_conf.intr_conf.lsc) { + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device); + + rte_intr_disable(&pci_dev->intr_handle); + + rte_intr_callback_unregister(&pci_dev->intr_handle, + vmxnet3_interrupt_handler, dev); + } + /* quiesce the device first */ VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_QUIESCE_DEV); VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, 0); @@ -1110,17 +1150,13 @@ vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) /* return 0 means link status changed, -1 means not changed */ static int -vmxnet3_dev_link_update(struct rte_eth_dev *dev, - __rte_unused int wait_to_complete) +__vmxnet3_dev_link_update(struct rte_eth_dev *dev, + __rte_unused int wait_to_complete) { struct vmxnet3_hw *hw = dev->data->dev_private; struct rte_eth_link old = { 0 }, link; uint32_t ret; - /* Link status doesn't change for stopped dev */ - if (dev->data->dev_started == 0) - return -1; - memset(&link, 0, sizeof(link)); vmxnet3_dev_atomic_read_link_status(dev, &old); @@ -1139,6 +1175,16 @@ vmxnet3_dev_link_update(struct rte_eth_dev *dev, return (old.link_status == link.link_status) ? -1 : 0; } +static int +vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete) +{ + /* Link status doesn't change for stopped dev */ + if (dev->data->dev_started == 0) + return -1; + + return __vmxnet3_dev_link_update(dev, wait_to_complete); +} + /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */ static void vmxnet3_dev_set_rxmode(struct vmxnet3_hw *hw, uint32_t feature, int set) @@ -1255,10 +1301,10 @@ vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) } } -#if PROCESS_SYS_EVENTS == 1 static void -vmxnet3_process_events(struct vmxnet3_hw *hw) +vmxnet3_process_events(struct rte_eth_dev *dev) { + struct vmxnet3_hw *hw = dev->data->dev_private; uint32_t events = hw->shared->ecr; if (!events) { @@ -1273,10 +1319,15 @@ vmxnet3_process_events(struct vmxnet3_hw *hw) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_ECR, events); /* Check if link state has changed */ - if (events & VMXNET3_ECR_LINK) + if (events & VMXNET3_ECR_LINK) { PMD_INIT_LOG(ERR, "Process events in %s(): VMXNET3_ECR_LINK event", __func__); + if (vmxnet3_dev_link_update(dev, 0) == 0) + _rte_eth_dev_callback_process(dev, + RTE_ETH_EVENT_INTR_LSC, + NULL); + } /* Check if there is an error on xmit/recv queues */ if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) { @@ -1301,7 +1352,18 @@ vmxnet3_process_events(struct vmxnet3_hw *hw) if (events & VMXNET3_ECR_DEBUG) PMD_INIT_LOG(ERR, "Debug event generated by device."); } -#endif + +static void +vmxnet3_interrupt_handler(void *param) +{ + struct rte_eth_dev *dev = param; + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device); + + vmxnet3_process_events(dev); + + if (rte_intr_enable(&pci_dev->intr_handle) < 0) + PMD_DRV_LOG(ERR, "interrupt enable failed"); +} RTE_PMD_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd); RTE_PMD_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map); diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h index e93e4a7..b48058a 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h @@ -106,6 +106,8 @@ struct vmxnet3_hw { uint16_t txdata_desc_size; /* tx data ring buffer size */ uint16_t rxdata_desc_size; /* rx data ring buffer size */ + uint8_t num_intrs; + Vmxnet3_TxQueueDesc *tqd_start; /* start address of all tx queue desc */ Vmxnet3_RxQueueDesc *rqd_start; /* start address of all rx queue desc */ -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams @ 2017-06-27 13:52 ` Ferruh Yigit 0 siblings, 0 replies; 30+ messages in thread From: Ferruh Yigit @ 2017-06-27 13:52 UTC (permalink / raw) To: Charles (Chas) Williams, dev; +Cc: skhare, Robert Shearman On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote: > From: Robert Shearman <rshearma@brocade.com> > > Generate link-state change notifications by listening to interrupts > generated by the device. Make use of the existing > vmxnet3_process_events function that was compiled out, but change it > to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not > be so noisy in its log messages. > > Enable interrupts on starting the device, using a new helper function, > vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated > against the FreeBSD driver. > > Keep track of the number of interrupts registered for to avoid > hardcoding these in vmxnet3_enable/disable_intr and to provision for > any future rxq intr support. > > Factor out the guts of vmxnet3_dev_link_update minus the started check > to allow the new function to be called from vmxnet3_dev_start in the > lsc-enabled case to ensure that the link state is correctly set from > the actual state at that point. > > Signed-off-by: Robert Shearman <rshearma@brocade.com> It is close to the integration deadline, and all patches in the set acked except this one, I am for getting the set and if any issue found related LSC, fix it after integration deadline. If there is an objection to get the patchset, please comment soon. Thanks, ferruh ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams ` (2 preceding siblings ...) 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams @ 2017-06-15 12:16 ` Charles (Chas) Williams 2017-06-28 11:30 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Ferruh Yigit 4 siblings, 0 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:16 UTC (permalink / raw) To: dev; +Cc: skhare, Robert Shearman From: Robert Shearman <rshearma@brocade.com> Make vmxnet3_process_events less noisy by removing logging when there are no events to process and by making link, device-change and debug events DEBUG level rather than ERR. Change these to use PMD_DRV_LOG instead of PMD_INIT_LOG since they don't happen at device init. Signed-off-by: Robert Shearman <rshearma@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 73277fb..e8cbc85 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -1307,10 +1307,8 @@ vmxnet3_process_events(struct rte_eth_dev *dev) struct vmxnet3_hw *hw = dev->data->dev_private; uint32_t events = hw->shared->ecr; - if (!events) { - PMD_INIT_LOG(ERR, "No events to process"); + if (!events) return; - } /* * ECR bits when written with 1b are cleared. Hence write @@ -1320,9 +1318,7 @@ vmxnet3_process_events(struct rte_eth_dev *dev) /* Check if link state has changed */ if (events & VMXNET3_ECR_LINK) { - PMD_INIT_LOG(ERR, - "Process events in %s(): VMXNET3_ECR_LINK event", - __func__); + PMD_DRV_LOG(DEBUG, "Process events: VMXNET3_ECR_LINK event"); if (vmxnet3_dev_link_update(dev, 0) == 0) _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, @@ -1335,11 +1331,11 @@ vmxnet3_process_events(struct rte_eth_dev *dev) VMXNET3_CMD_GET_QUEUE_STATUS); if (hw->tqd_start->status.stopped) - PMD_INIT_LOG(ERR, "tq error 0x%x", - hw->tqd_start->status.error); + PMD_DRV_LOG(ERR, "tq error 0x%x", + hw->tqd_start->status.error); if (hw->rqd_start->status.stopped) - PMD_INIT_LOG(ERR, "rq error 0x%x", + PMD_DRV_LOG(ERR, "rq error 0x%x", hw->rqd_start->status.error); /* Reset the device */ @@ -1347,10 +1343,10 @@ vmxnet3_process_events(struct rte_eth_dev *dev) } if (events & VMXNET3_ECR_DIC) - PMD_INIT_LOG(ERR, "Device implementation change event."); + PMD_DRV_LOG(DEBUG, "Device implementation change event."); if (events & VMXNET3_ECR_DEBUG) - PMD_INIT_LOG(ERR, "Debug event generated by device."); + PMD_DRV_LOG(DEBUG, "Debug event generated by device."); } static void -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams ` (3 preceding siblings ...) 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams @ 2017-06-28 11:30 ` Ferruh Yigit 2017-06-28 12:52 ` Ferruh Yigit 4 siblings, 1 reply; 30+ messages in thread From: Ferruh Yigit @ 2017-06-28 11:30 UTC (permalink / raw) To: Charles (Chas) Williams, dev; +Cc: skhare On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote: > This series addresses some local issues with the vmxnet3 driver that > others might find of interest. > > Changes in v2: > > - net/vmxnet3: Implement retrieval of extended stats > > .id field wasn't being filled in xstats. > > - net/vmxnet3: receive queue memory leak > > The leak and buffer reallcation were split. The buffer reallocation > patch may be superceded by another commit so isn't included in this > series while under discussion. > Series applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches 2017-06-28 11:30 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Ferruh Yigit @ 2017-06-28 12:52 ` Ferruh Yigit 2017-06-28 13:09 ` Charles (Chas) Williams 2017-06-28 17:15 ` Charles (Chas) Williams 0 siblings, 2 replies; 30+ messages in thread From: Ferruh Yigit @ 2017-06-28 12:52 UTC (permalink / raw) To: Charles (Chas) Williams, dev; +Cc: skhare On 6/28/2017 12:30 PM, Ferruh Yigit wrote: > On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote: >> This series addresses some local issues with the vmxnet3 driver that >> others might find of interest. >> >> Changes in v2: >> >> - net/vmxnet3: Implement retrieval of extended stats >> >> .id field wasn't being filled in xstats. >> >> - net/vmxnet3: receive queue memory leak >> >> The leak and buffer reallcation were split. The buffer reallocation >> patch may be superceded by another commit so isn't included in this >> series while under discussion. >> > > Series applied to dpdk-next-net/master, thanks. Hi Charles, Patches require vmxnet3.ini update, "Extended stats" and "Link status event" ones I guess. I missed them to before getting patchset. Would you mind sending another patch to update features document? Thanks, ferruh ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches 2017-06-28 12:52 ` Ferruh Yigit @ 2017-06-28 13:09 ` Charles (Chas) Williams 2017-06-28 17:15 ` Charles (Chas) Williams 1 sibling, 0 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-28 13:09 UTC (permalink / raw) To: Ferruh Yigit, dev; +Cc: skhare On 06/28/2017 08:52 AM, Ferruh Yigit wrote: > On 6/28/2017 12:30 PM, Ferruh Yigit wrote: >> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote: >>> This series addresses some local issues with the vmxnet3 driver that >>> others might find of interest. >>> >>> Changes in v2: >>> >>> - net/vmxnet3: Implement retrieval of extended stats >>> >>> .id field wasn't being filled in xstats. >>> >>> - net/vmxnet3: receive queue memory leak >>> >>> The leak and buffer reallcation were split. The buffer reallocation >>> patch may be superceded by another commit so isn't included in this >>> series while under discussion. >>> >> >> Series applied to dpdk-next-net/master, thanks. > > Hi Charles, > > Patches require vmxnet3.ini update, "Extended stats" and "Link status > event" ones I guess. I missed them to before getting patchset. > > Would you mind sending another patch to update features document? > > Thanks, > ferruh > I will get to it today. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches 2017-06-28 12:52 ` Ferruh Yigit 2017-06-28 13:09 ` Charles (Chas) Williams @ 2017-06-28 17:15 ` Charles (Chas) Williams 2017-06-28 17:54 ` Ferruh Yigit 1 sibling, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-28 17:15 UTC (permalink / raw) To: Ferruh Yigit, dev; +Cc: skhare On 06/28/2017 08:52 AM, Ferruh Yigit wrote: > On 6/28/2017 12:30 PM, Ferruh Yigit wrote: >> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote: >>> This series addresses some local issues with the vmxnet3 driver that >>> others might find of interest. >>> >>> Changes in v2: >>> >>> - net/vmxnet3: Implement retrieval of extended stats >>> >>> .id field wasn't being filled in xstats. >>> >>> - net/vmxnet3: receive queue memory leak >>> >>> The leak and buffer reallcation were split. The buffer reallocation >>> patch may be superceded by another commit so isn't included in this >>> series while under discussion. >>> >> >> Series applied to dpdk-next-net/master, thanks. > > Hi Charles, > > Patches require vmxnet3.ini update, "Extended stats" and "Link status > event" ones I guess. I missed them to before getting patchset. > > Would you mind sending another patch to update features document? > > Thanks, > ferruh > Curiously, the vmxnet3 already indicates that it is handling link status event interrupts. ; Supported features of the 'vmxnet3' network poll mode driver. ; ; Refer to default.ini for the full list of available PMD features. ; [Features] Speed capabilities = P Link status = Y Link status event = Y ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches 2017-06-28 17:15 ` Charles (Chas) Williams @ 2017-06-28 17:54 ` Ferruh Yigit 0 siblings, 0 replies; 30+ messages in thread From: Ferruh Yigit @ 2017-06-28 17:54 UTC (permalink / raw) To: Charles (Chas) Williams, dev; +Cc: skhare On 6/28/2017 6:15 PM, Charles (Chas) Williams wrote: > > > On 06/28/2017 08:52 AM, Ferruh Yigit wrote: >> On 6/28/2017 12:30 PM, Ferruh Yigit wrote: >>> On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote: >>>> This series addresses some local issues with the vmxnet3 driver that >>>> others might find of interest. >>>> >>>> Changes in v2: >>>> >>>> - net/vmxnet3: Implement retrieval of extended stats >>>> >>>> .id field wasn't being filled in xstats. >>>> >>>> - net/vmxnet3: receive queue memory leak >>>> >>>> The leak and buffer reallcation were split. The buffer reallocation >>>> patch may be superceded by another commit so isn't included in this >>>> series while under discussion. >>>> >>> >>> Series applied to dpdk-next-net/master, thanks. >> >> Hi Charles, >> >> Patches require vmxnet3.ini update, "Extended stats" and "Link status >> event" ones I guess. I missed them to before getting patchset. >> >> Would you mind sending another patch to update features document? >> >> Thanks, >> ferruh >> > > Curiously, the vmxnet3 already indicates that it is handling link > status event interrupts. You are right, this can be by mistake. > > > ; Supported features of the 'vmxnet3' network poll mode driver. > ; > ; Refer to default.ini for the full list of available PMD features. > ; > [Features] > Speed capabilities = P > Link status = Y > Link status event = Y > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 5/6] net/vmxnet3: receive queue memory leak 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (6 preceding siblings ...) 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams @ 2017-06-15 12:17 ` Charles (Chas) Williams 2017-06-23 23:00 ` Shrikrishna Khare 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 6/6] net/vmxnet3: preserve configured MAC address Charles (Chas) Williams 8 siblings, 1 reply; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:17 UTC (permalink / raw) To: dev; +Cc: skhare, Mandeep Rohilla From: Mandeep Rohilla <mrohilla@brocade.com> This addresses an mbuf leak in an error condition during packet receive. Fixes: dfaff37fc46d ("vmxnet3: import new vmxnet3 poll mode driver implementation") Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com> --- drivers/net/vmxnet3/vmxnet3_rxtx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c index d8713a1..13c73f6 100644 --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c @@ -800,6 +800,12 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) (int)(rcd - (struct Vmxnet3_RxCompDesc *) rxq->comp_ring.base), rcd->rxdIdx); rte_pktmbuf_free_seg(rxm); + if (rxq->start_seg) { + struct rte_mbuf *start = rxq->start_seg; + + rxq->start_seg = NULL; + rte_pktmbuf_free(start); + } goto rcd_done; } -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 5/6] net/vmxnet3: receive queue memory leak 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 5/6] net/vmxnet3: receive queue memory leak Charles (Chas) Williams @ 2017-06-23 23:00 ` Shrikrishna Khare 0 siblings, 0 replies; 30+ messages in thread From: Shrikrishna Khare @ 2017-06-23 23:00 UTC (permalink / raw) To: Charles (Chas) Williams; +Cc: dev, skhare, Mandeep Rohilla On Thu, 15 Jun 2017, Charles (Chas) Williams wrote: > From: Mandeep Rohilla <mrohilla@brocade.com> > > This addresses an mbuf leak in an error condition during packet > receive. > > Fixes: dfaff37fc46d ("vmxnet3: import new vmxnet3 poll mode driver implementation") > > Signed-off-by: Mandeep Rohilla <mrohilla@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 6/6] net/vmxnet3: preserve configured MAC address 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams ` (7 preceding siblings ...) 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 5/6] net/vmxnet3: receive queue memory leak Charles (Chas) Williams @ 2017-06-15 12:17 ` Charles (Chas) Williams 8 siblings, 0 replies; 30+ messages in thread From: Charles (Chas) Williams @ 2017-06-15 12:17 UTC (permalink / raw) To: dev; +Cc: skhare, George Wilkie From: George Wilkie <gwilkie@brocade.com> When starting a vmxnet3 device, it is always writing the permanent MAC address, even if a different MAC address was configured. Write from the device data instead which holds the current one. Signed-off-by: George Wilkie <gwilkie@brocade.com> Acked-by: Shrikrishna Khare <skhare@vmware.com> --- drivers/net/vmxnet3/vmxnet3_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index e8cbc85..265de35 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -734,7 +734,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev) vmxnet3_dev_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); - vmxnet3_write_mac(hw, hw->perm_addr); + vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); return VMXNET3_SUCCESS; } -- 2.1.4 ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2017-06-28 17:54 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-19 17:55 [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams 2017-05-24 0:17 ` Shrikrishna Khare 2017-05-19 17:55 ` [dpdk-dev] [PATCH 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams 2017-05-19 17:55 ` [dpdk-dev] [PATCH 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams 2017-05-23 21:44 ` Shrikrishna Khare 2017-05-19 17:55 ` [dpdk-dev] [PATCH 5/6] net/vmxnet3: receive queue lockup and memleak Charles (Chas) Williams 2017-06-01 12:24 ` Charles (Chas) Williams 2017-05-24 21:09 ` [dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart Shrikrishna Khare 2017-05-25 18:31 ` Nachi Prachanda 2017-05-25 20:27 ` Shrikrishna Khare 2017-05-25 22:08 ` Nachi Prachanda 2017-05-26 17:29 ` Shrikrishna Khare 2017-05-26 19:01 ` Nachi Prachanda 2017-05-26 17:31 ` Shrikrishna Khare 2017-06-15 12:16 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 1/6] net/vmxnet3: retain counters on restart Charles (Chas) Williams 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 2/6] net/vmxnet3: Implement retrieval of extended stats Charles (Chas) Williams 2017-06-21 1:42 ` Shrikrishna Khare 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 3/6] net/vmxnet3: Generate link-state change notifications Charles (Chas) Williams 2017-06-27 13:52 ` Ferruh Yigit 2017-06-15 12:16 ` [dpdk-dev] [PATCH v2 4/6] net/vmxnet3: Make vmxnet3_process_events less noisy Charles (Chas) Williams 2017-06-28 11:30 ` [dpdk-dev] [PATCH V2 0/6] some local vmxnet3 patches Ferruh Yigit 2017-06-28 12:52 ` Ferruh Yigit 2017-06-28 13:09 ` Charles (Chas) Williams 2017-06-28 17:15 ` Charles (Chas) Williams 2017-06-28 17:54 ` Ferruh Yigit 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 5/6] net/vmxnet3: receive queue memory leak Charles (Chas) Williams 2017-06-23 23:00 ` Shrikrishna Khare 2017-06-15 12:17 ` [dpdk-dev] [PATCH v2 6/6] net/vmxnet3: preserve configured MAC address Charles (Chas) Williams
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).