From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by dpdk.org (Postfix) with ESMTP id 909FB2A7 for ; Fri, 23 May 2014 13:09:09 +0200 (CEST) Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 23 May 2014 04:09:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.98,893,1392192000"; d="scan'208";a="436011358" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by azsmga001.ch.intel.com with ESMTP; 23 May 2014 04:09:14 -0700 Received: from sivswdev02.ir.intel.com (sivswdev02.ir.intel.com [10.237.217.46]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id s4NB9CML007535; Fri, 23 May 2014 12:09:13 +0100 Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1]) by sivswdev02.ir.intel.com with ESMTP id s4NB9CbW025586; Fri, 23 May 2014 12:09:12 +0100 Received: (from kananye1@localhost) by sivswdev02.ir.intel.com with id s4NB9CNa025582; Fri, 23 May 2014 12:09:12 +0100 From: Konstantin Ananyev To: dev@dpdk.org Date: Fri, 23 May 2014 12:08:51 +0100 Message-Id: <1400843331-24944-1-git-send-email-konstantin.ananyev@intel.com> X-Mailer: git-send-email 1.7.0.7 To: dev@dpdk.org Subject: [dpdk-dev] [PATCH] fix for 2 consecutive rte_eth_dev_start() can cause a SIGSEGV X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 May 2014 11:09:10 -0000 1)If igb_alloc_rx_queue_mbufs() would fail to allocate an mbuf for RX queue, it calls igb_rx_queue_release(rxq). That causes rxq to be silently freed, without updating dev->data->rx_queues[]. So any firther reference to it will trigger the SIGSEGV. Same thing in em PMD too. To fix: igb_alloc_rx_queue_mbufs() should just return an error to the caller and let upper layer to deal with the probem. That's what ixgbe PMD doing right now. 2)In tx_queue_setup (for all 3 PMDs: ixgbe, igb, em) we call tx_queue_release(dev->data->tx_queues[queue_idx]) without setting dev->data->tx_queues[queue_idx] = NULL afterwards. 3)Prevent rte_eth_dev_start/stop to call underneath dev_start/dev_stop for already started/stopped device. 4) fix compiler warning on PMD_DEBUG_TRACE() formats. Signed-off-by: Konstantin Ananyev --- lib/librte_ether/rte_ethdev.c | 29 ++++++++++++++++++++++++----- lib/librte_pmd_e1000/em_rxtx.c | 1 - lib/librte_pmd_e1000/igb_rxtx.c | 5 +++-- lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++++-- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0ddedfb..c052283 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -448,7 +448,8 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, (dev_conf->rxmode.mq_mode == ETH_MQ_RX_DCB_RSS) || (dev_conf->txmode.mq_mode == ETH_MQ_TX_DCB)) { /* SRIOV only works in VMDq enable mode */ - PMD_DEBUG_TRACE("ethdev port_id=%hhu SRIOV active, " + PMD_DEBUG_TRACE("ethdev port_id=%" PRIu8 + " SRIOV active, " "wrong VMDQ mq_mode rx %u tx %u\n", port_id, dev_conf->rxmode.mq_mode, @@ -461,7 +462,8 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, case ETH_MQ_RX_VMDQ_DCB: case ETH_MQ_RX_VMDQ_DCB_RSS: /* DCB/RSS VMDQ in SRIOV mode, not implement yet */ - PMD_DEBUG_TRACE("ethdev port_id=%hhu SRIOV active, " + PMD_DEBUG_TRACE("ethdev port_id=%" PRIu8 + " SRIOV active, " "unsupported VMDQ mq_mode rx %u\n", port_id, dev_conf->rxmode.mq_mode); return (-EINVAL); @@ -476,7 +478,8 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, switch (dev_conf->txmode.mq_mode) { case ETH_MQ_TX_VMDQ_DCB: /* DCB VMDQ in SRIOV mode, not implement yet */ - PMD_DEBUG_TRACE("ethdev port_id=%hhu SRIOV active, " + PMD_DEBUG_TRACE("ethdev port_id=%" PRIu8 + " SRIOV active, " "unsupported VMDQ mq_mode tx %u\n", port_id, dev_conf->txmode.mq_mode); return (-EINVAL); @@ -759,12 +762,20 @@ rte_eth_dev_start(uint8_t port_id) PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY); if (port_id >= nb_ports) { - PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); + PMD_DEBUG_TRACE("Invalid port_id=%" PRIu8 "\n", port_id); return (-EINVAL); } dev = &rte_eth_devices[port_id]; FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_start, -ENOTSUP); + + if (dev->data->dev_started != 0) { + PMD_DEBUG_TRACE("Device with port_id=%" PRIu8 + " already started\n", + port_id); + return (0); + } + diag = (*dev->dev_ops->dev_start)(dev); if (diag == 0) dev->data->dev_started = 1; @@ -786,12 +797,20 @@ rte_eth_dev_stop(uint8_t port_id) PROC_PRIMARY_OR_RET(); if (port_id >= nb_ports) { - PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); + PMD_DEBUG_TRACE("Invalid port_id=%" PRIu8 "\n", port_id); return; } dev = &rte_eth_devices[port_id]; FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop); + + if (dev->data->dev_started == 0) { + PMD_DEBUG_TRACE("Device with port_id=%" PRIu8 + " already stopped\n", + port_id); + return; + } + dev->data->dev_started = 0; (*dev->dev_ops->dev_stop)(dev); } diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c index 4f98a3f..5ed4def 100644 --- a/lib/librte_pmd_e1000/em_rxtx.c +++ b/lib/librte_pmd_e1000/em_rxtx.c @@ -1580,7 +1580,6 @@ em_alloc_rx_queue_mbufs(struct em_rx_queue *rxq) if (mbuf == NULL) { PMD_INIT_LOG(ERR, "RX mbuf alloc failed " "queue_id=%hu\n", rxq->queue_id); - em_rx_queue_release(rxq); return (-ENOMEM); } diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c index 4608595..877513e 100644 --- a/lib/librte_pmd_e1000/igb_rxtx.c +++ b/lib/librte_pmd_e1000/igb_rxtx.c @@ -1212,8 +1212,10 @@ eth_igb_tx_queue_setup(struct rte_eth_dev *dev, "the TX WTHRESH value to 4, 8, or 16.\n"); /* Free memory prior to re-allocation if needed */ - if (dev->data->tx_queues[queue_idx] != NULL) + if (dev->data->tx_queues[queue_idx] != NULL) { igb_tx_queue_release(dev->data->tx_queues[queue_idx]); + dev->data->tx_queues[queue_idx] = NULL; + } /* First allocate the tx queue data structure */ txq = rte_zmalloc("ethdev TX queue", sizeof(struct igb_tx_queue), @@ -1721,7 +1723,6 @@ igb_alloc_rx_queue_mbufs(struct igb_rx_queue *rxq) if (mbuf == NULL) { PMD_INIT_LOG(ERR, "RX mbuf alloc failed " "queue_id=%hu\n", rxq->queue_id); - igb_rx_queue_release(rxq); return (-ENOMEM); } dma_addr = diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c index c3bdffd..83cfbd1 100644 --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c @@ -1802,8 +1802,10 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, } /* Free memory prior to re-allocation if needed... */ - if (dev->data->tx_queues[queue_idx] != NULL) + if (dev->data->tx_queues[queue_idx] != NULL) { ixgbe_tx_queue_release(dev->data->tx_queues[queue_idx]); + dev->data->tx_queues[queue_idx] = NULL; + } /* First allocate the tx queue data structure */ txq = rte_zmalloc_socket("ethdev TX queue", sizeof(struct igb_tx_queue), @@ -2061,8 +2063,10 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev, } /* Free memory prior to re-allocation if needed... */ - if (dev->data->rx_queues[queue_idx] != NULL) + if (dev->data->rx_queues[queue_idx] != NULL) { ixgbe_rx_queue_release(dev->data->rx_queues[queue_idx]); + dev->data->rx_queues[queue_idx] = NULL; + } /* First allocate the rx queue data structure */ rxq = rte_zmalloc_socket("ethdev RX queue", sizeof(struct igb_rx_queue), -- 1.7.7.6