diff mbox

[dpdk-dev,1/5] Revert "ixgbe: check mbuf refcnt when clearing a ring"

Message ID 1437667506-3890-2-git-send-email-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Bruce Richardson July 23, 2015, 4:05 p.m. UTC 
  This reverts commit b35d0d80f0a809939549ddde99c1a76b7e38bff3.
The bug fix was incorrect as it did not take account of the fact that
the mbufs that were previously freed may have since be re-allocated.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c     | 3 +--
 drivers/net/ixgbe/ixgbe_rxtx_vec.c | 8 +-------
 2 files changed, 2 insertions(+), 9 deletions(-)

Comments

Ananyev, Konstantin July 24, 2015, 1:58 p.m. UTC | #1 
Konstantin has correctly pointed out that the previously applied fix:
b35d0d80f0a8 ("ixgbe: check mbuf refcnt when clearing a ring")
is not a proper fix for the reported issue at all.
Ref: http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/21932

This patch set reverts the original fix, and applies a better fix for the
issue, as well as performing other cleanups in the code in question, to
try and avoid future issues.

v2 chages:
- Make sure that rx_using_sse is reset to zero if scalar RX function was chosen.
- fix checkpatch.pl errors.
- fix remaining wrong typecast.

Konstantin Ananyev (5):
  Revert "ixgbe: check mbuf refcnt when clearing a ring"
  ixgbe: fix comments on rx_queue fields
  ixgbe: fix bug on release of mbufs from queue
  ixgbe: rename tx queue release function for consistency
  ixgbe: remove awkward typecasts from ixgbe SSE PMD

 drivers/net/ixgbe/ixgbe_rxtx.c     | 23 ++++++++++-
 drivers/net/ixgbe/ixgbe_rxtx.h     | 12 ++++--
 drivers/net/ixgbe/ixgbe_rxtx_vec.c | 80 +++++++++++++++++++++-----------------
 3 files changed, 75 insertions(+), 40 deletions(-)
Thomas Monjalon July 26, 2015, 8:48 a.m. UTC | #2 
2015-07-24 14:58, Konstantin Ananyev:
> Konstantin has correctly pointed out that the previously applied fix:
> b35d0d80f0a8 ("ixgbe: check mbuf refcnt when clearing a ring")
> is not a proper fix for the reported issue at all.
> Ref: http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/21932
> 
> This patch set reverts the original fix, and applies a better fix for the
> issue, as well as performing other cleanups in the code in question, to
> try and avoid future issues.
> 
> v2 chages:
> - Make sure that rx_using_sse is reset to zero if scalar RX function was chosen.
> - fix checkpatch.pl errors.
> - fix remaining wrong typecast.

Applied, thanks
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index af7e222..75c5555 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2272,8 +2272,7 @@  ixgbe_rx_queue_release_mbufs(struct ixgbe_rx_queue *rxq)
 
 	if (rxq->sw_ring != NULL) {
 		for (i = 0; i < rxq->nb_rx_desc; i++) {
-			if (rxq->sw_ring[i].mbuf != NULL &&
-					rte_mbuf_refcnt_read(rxq->sw_ring[i].mbuf)) {
+			if (rxq->sw_ring[i].mbuf != NULL) {
 				rte_pktmbuf_free_seg(rxq->sw_ring[i].mbuf);
 				rxq->sw_ring[i].mbuf = NULL;
 			}
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
index d3ac74a..47ff990 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
@@ -736,13 +736,7 @@  ixgbe_tx_queue_release_mbufs(struct ixgbe_tx_queue *txq)
 		     nb_free < max_desc && i != txq->tx_tail;
 		     i = (i + 1) & max_desc) {
 			txe = (struct ixgbe_tx_entry_v *)&txq->sw_ring[i];
-			/*
-			 * Check for already freed packets.
-			 * Note: ixgbe_tx_free_bufs does not NULL after free,
-			 * so we actually have to check the reference count.
-			 */
-			if (txe->mbuf != NULL &&
-					rte_mbuf_refcnt_read(txe->mbuf) != 0)
+			if (txe->mbuf != NULL)
 				rte_pktmbuf_free_seg(txe->mbuf);
 		}
 		/* reset tx_entry */