Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 11 Oct 2014 22:13:24 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r272969 - stable/10/sys/dev/e1000
Message-ID:  <201410112213.s9BMDO2k021815@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: adrian
Date: Sat Oct 11 22:13:24 2014
New Revision: 272969
URL: https://svnweb.freebsd.org/changeset/base/272969

Log:
  MFC r271784 - Fix the handling of EOP in status descriptors for if_igb(4)
  and don't double-free mbufs.
  
  Like ixgbe(4) chipsets, EOP is only set on the final descriptor
  in a chain of descriptors.  So, to free the whole list of descriptors,
  we should free the current slot _and_ the assembled list of descriptors
  that make up the fragment list.
  
  The existing code was setting discard once it saw EOP + an error status;
  it then freed all the subsequent descriptors until the next EOP. That's
  totally the wrong order.

Modified:
  stable/10/sys/dev/e1000/if_igb.c
  stable/10/sys/dev/e1000/if_igb.h
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/dev/e1000/if_igb.c
==============================================================================
--- stable/10/sys/dev/e1000/if_igb.c	Sat Oct 11 22:12:24 2014	(r272968)
+++ stable/10/sys/dev/e1000/if_igb.c	Sat Oct 11 22:13:24 2014	(r272969)
@@ -4397,7 +4397,6 @@ skip_head:
 
 	rxr->fmp = NULL;
 	rxr->lmp = NULL;
-	rxr->discard = FALSE;
 
 	bus_dmamap_sync(rxr->rxdma.dma_tag, rxr->rxdma.dma_map,
 	    BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
@@ -4873,15 +4872,16 @@ igb_rxeof(struct igb_queue *que, int cou
 		hdr = le16toh(cur->wb.lower.lo_dword.hs_rss.hdr_info);
 		eop = ((staterr & E1000_RXD_STAT_EOP) == E1000_RXD_STAT_EOP);
 
-		/* Make sure all segments of a bad packet are discarded */
-		if (((staterr & E1000_RXDEXT_ERR_FRAME_ERR_MASK) != 0) ||
-		    (rxr->discard)) {
+		/*
+		 * Free the frame (all segments) if we're at EOP and
+		 * it's an error.
+		 *
+		 * The datasheet states that EOP + status is only valid for
+		 * the final segment in a multi-segment frame.
+		 */
+		if (eop && ((staterr & E1000_RXDEXT_ERR_FRAME_ERR_MASK) != 0)) {
 			adapter->dropped_pkts++;
 			++rxr->rx_discarded;
-			if (!eop) /* Catch subsequent segs */
-				rxr->discard = TRUE;
-			else
-				rxr->discard = FALSE;
 			igb_rx_discard(rxr, i);
 			goto next_desc;
 		}

Modified: stable/10/sys/dev/e1000/if_igb.h
==============================================================================
--- stable/10/sys/dev/e1000/if_igb.h	Sat Oct 11 22:12:24 2014	(r272968)
+++ stable/10/sys/dev/e1000/if_igb.h	Sat Oct 11 22:13:24 2014	(r272969)
@@ -336,7 +336,6 @@ struct rx_ring {
 	struct lro_ctrl		lro;
 	bool			lro_enabled;
 	bool			hdr_split;
-	bool			discard;
 	struct mtx		rx_mtx;
 	char			mtx_name[16];
 	u32			next_to_refresh;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201410112213.s9BMDO2k021815>