[Intel-wired-lan] [next PATCH S63 1/6] i40e/i40evf: Use length to determine if descriptor is done

[Intel-wired-lan] [next PATCH S63 1/6] i40e/i40evf: Use length to determine if descriptor is done

[Bimmy Pujari]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change makes it so that we use the length of the packet instead of the
DD status bit to determine if a new descriptor is ready to be processed.
The obvious advantage is that it cuts down on reads as we don't really even
need the DD bit if going from a 0 to a non-zero value on size is enough to
inform us that the packet has been completed.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: Iebdf9cdb36c454ef092df27199b92ad09c374231
---
Testing Hints:
        Perform various Rx tests to verify we are not corrupting any Rx data.

 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 24 ++++++++++++------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 24 ++++++++++++------------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 9742beb..68936b6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1757,6 +1757,7 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
  * i40e_fetch_rx_buffer - Allocate skb and populate it
  * @rx_ring: rx descriptor ring to transact packets on
  * @rx_desc: descriptor containing info written by hardware
+ * @size: size of buffer to add to skb
  *
  * This function allocates an skb on the fly, and populates it with the page
  * data from the current receive descriptor, taking care to set up the skb
@@ -1766,13 +1767,9 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 static inline
 struct sk_buff *i40e_fetch_rx_buffer(struct i40e_ring *rx_ring,
 				     union i40e_rx_desc *rx_desc,
-				     struct sk_buff *skb)
+				     struct sk_buff *skb,
+				     unsigned int size)
 {
-	u64 local_status_error_len =
-		le64_to_cpu(rx_desc->wb.qword1.status_error_len);
-	unsigned int size =
-		(local_status_error_len & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
-		I40E_RXD_QW1_LENGTH_PBUF_SHIFT;
 	struct i40e_rx_buffer *rx_buffer;
 	struct page *page;
 
@@ -1890,6 +1887,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 	while (likely(total_rx_packets < budget)) {
 		union i40e_rx_desc *rx_desc;
+		unsigned int size;
 		u16 vlan_tag;
 		u8 rx_ptype;
 		u64 qword;
@@ -1906,19 +1904,21 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		/* status_error_len will always be zero for unused descriptors
 		 * because it's cleared in cleanup, and overlaps with hdr_addr
 		 * which is always zero because packet split isn't used, if the
-		 * hardware wrote DD then it will be non-zero
+		 * hardware wrote DD then the length will be non-zero
 		 */
-		if (!i40e_test_staterr(rx_desc,
-				       BIT(I40E_RX_DESC_STATUS_DD_SHIFT)))
+		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
+		size = (qword & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
+		       I40E_RXD_QW1_LENGTH_PBUF_SHIFT;
+		if (!size)
 			break;
 
 		/* This memory barrier is needed to keep us from reading
-		 * any other fields out of the rx_desc until we know the
-		 * DD bit is set.
+		 * any other fields out of the rx_desc until we have
+		 * verified the descriptor has been written back.
 		 */
 		dma_rmb();
 
-		skb = i40e_fetch_rx_buffer(rx_ring, rx_desc, skb);
+		skb = i40e_fetch_rx_buffer(rx_ring, rx_desc, skb, size);
 		if (!skb)
 			break;
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index f1a99a8..e41eb46 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1116,6 +1116,7 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
  * i40evf_fetch_rx_buffer - Allocate skb and populate it
  * @rx_ring: rx descriptor ring to transact packets on
  * @rx_desc: descriptor containing info written by hardware
+ * @size: size of buffer to add to skb
  *
  * This function allocates an skb on the fly, and populates it with the page
  * data from the current receive descriptor, taking care to set up the skb
@@ -1125,13 +1126,9 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 static inline
 struct sk_buff *i40evf_fetch_rx_buffer(struct i40e_ring *rx_ring,
 				       union i40e_rx_desc *rx_desc,
-				       struct sk_buff *skb)
+				       struct sk_buff *skb,
+				       unsigned int size)
 {
-	u64 local_status_error_len =
-		le64_to_cpu(rx_desc->wb.qword1.status_error_len);
-	unsigned int size =
-		(local_status_error_len & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
-		I40E_RXD_QW1_LENGTH_PBUF_SHIFT;
 	struct i40e_rx_buffer *rx_buffer;
 	struct page *page;
 
@@ -1244,6 +1241,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 	while (likely(total_rx_packets < budget)) {
 		union i40e_rx_desc *rx_desc;
+		unsigned int size;
 		u16 vlan_tag;
 		u8 rx_ptype;
 		u64 qword;
@@ -1260,19 +1258,21 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		/* status_error_len will always be zero for unused descriptors
 		 * because it's cleared in cleanup, and overlaps with hdr_addr
 		 * which is always zero because packet split isn't used, if the
-		 * hardware wrote DD then it will be non-zero
+		 * hardware wrote DD then the length will be non-zero
 		 */
-		if (!i40e_test_staterr(rx_desc,
-				       BIT(I40E_RX_DESC_STATUS_DD_SHIFT)))
+		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
+		size = (qword & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
+		       I40E_RXD_QW1_LENGTH_PBUF_SHIFT;
+		if (!size)
 			break;
 
 		/* This memory barrier is needed to keep us from reading
-		 * any other fields out of the rx_desc until we know the
-		 * DD bit is set.
+		 * any other fields out of the rx_desc until we have
+		 * verified the descriptor has been written back.
 		 */
 		dma_rmb();
 
-		skb = i40evf_fetch_rx_buffer(rx_ring, rx_desc, skb);
+		skb = i40evf_fetch_rx_buffer(rx_ring, rx_desc, skb, size);
 		if (!skb)
 			break;
 
-- 
2.4.11

[Intel-wired-lan] [next PATCH S63 2/6] i40e/i40evf: Pull code for grabbing and syncing rx_buffer from fetch_buffer

[Bimmy Pujari]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch pulls the code responsible for fetching the Rx buffer and
synchronizing DMA into a function, specifically called i40e_get_rx_buffer.

The general idea is to allow for better code reuse by pulling this out of
i40e_fetch_rx_buffer.  We dropped a couple of prefetches since the time
between the prefetch being called and the data being accessed was too small
to be useful.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: I4885fce4b2637dbedc8e16431169d23d3d7e79b9
---
Testing Hints:
        Basic Rx testing should be enough to verify this is working
        correctly.

 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 58 ++++++++++++++++-----------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 58 ++++++++++++++++-----------
 2 files changed, 68 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 68936b6..d1fc0f0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1754,9 +1754,35 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 }
 
 /**
+ * i40e_get_rx_buffer - Fetch Rx buffer and synchronize data for use
+ * @rx_ring: rx descriptor ring to transact packets on
+ * @size: size of buffer to add to skb
+ *
+ * This function will pull an Rx buffer from the ring and synchronize it
+ * for use by the CPU.
+ */
+static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
+						 const unsigned int size)
+{
+	struct i40e_rx_buffer *rx_buffer;
+
+	rx_buffer = &rx_ring->rx_bi[rx_ring->next_to_clean];
+	prefetchw(rx_buffer->page);
+
+	/* we are reusing so sync this buffer for CPU use */
+	dma_sync_single_range_for_cpu(rx_ring->dev,
+				      rx_buffer->dma,
+				      rx_buffer->page_offset,
+				      size,
+				      DMA_FROM_DEVICE);
+
+	return rx_buffer;
+}
+
+/**
  * i40e_fetch_rx_buffer - Allocate skb and populate it
  * @rx_ring: rx descriptor ring to transact packets on
- * @rx_desc: descriptor containing info written by hardware
+ * @rx_buffer: rx buffer to pull data from
  * @size: size of buffer to add to skb
  *
  * This function allocates an skb on the fly, and populates it with the page
@@ -1766,19 +1792,13 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
  */
 static inline
 struct sk_buff *i40e_fetch_rx_buffer(struct i40e_ring *rx_ring,
-				     union i40e_rx_desc *rx_desc,
+				     struct i40e_rx_buffer *rx_buffer,
 				     struct sk_buff *skb,
 				     unsigned int size)
 {
-	struct i40e_rx_buffer *rx_buffer;
-	struct page *page;
-
-	rx_buffer = &rx_ring->rx_bi[rx_ring->next_to_clean];
-	page = rx_buffer->page;
-	prefetchw(page);
-
 	if (likely(!skb)) {
-		void *page_addr = page_address(page) + rx_buffer->page_offset;
+		void *page_addr = page_address(rx_buffer->page) +
+				  rx_buffer->page_offset;
 
 		/* prefetch first cache line of first page */
 		prefetch(page_addr);
@@ -1794,21 +1814,8 @@ struct sk_buff *i40e_fetch_rx_buffer(struct i40e_ring *rx_ring,
 			rx_ring->rx_stats.alloc_buff_failed++;
 			return NULL;
 		}
-
-		/* we will be copying header into skb->data in
-		 * pskb_may_pull so it is in our interest to prefetch
-		 * it now to avoid a possible cache miss
-		 */
-		prefetchw(skb->data);
 	}
 
-	/* we are reusing so sync this buffer for CPU use */
-	dma_sync_single_range_for_cpu(rx_ring->dev,
-				      rx_buffer->dma,
-				      rx_buffer->page_offset,
-				      size,
-				      DMA_FROM_DEVICE);
-
 	/* pull page into skb */
 	if (i40e_add_rx_frag(rx_ring, rx_buffer, size, skb)) {
 		/* hand second half of page back to the ring */
@@ -1886,6 +1893,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 	bool failure = false;
 
 	while (likely(total_rx_packets < budget)) {
+		struct i40e_rx_buffer *rx_buffer;
 		union i40e_rx_desc *rx_desc;
 		unsigned int size;
 		u16 vlan_tag;
@@ -1918,7 +1926,9 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		 */
 		dma_rmb();
 
-		skb = i40e_fetch_rx_buffer(rx_ring, rx_desc, skb, size);
+		rx_buffer = i40e_get_rx_buffer(rx_ring, size);
+
+		skb = i40e_fetch_rx_buffer(rx_ring, rx_buffer, skb, size);
 		if (!skb)
 			break;
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index e41eb46..2320ec4 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1113,9 +1113,35 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 }
 
 /**
+ * i40e_get_rx_buffer - Fetch Rx buffer and synchronize data for use
+ * @rx_ring: rx descriptor ring to transact packets on
+ * @size: size of buffer to add to skb
+ *
+ * This function will pull an Rx buffer from the ring and synchronize it
+ * for use by the CPU.
+ */
+static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
+						 const unsigned int size)
+{
+	struct i40e_rx_buffer *rx_buffer;
+
+	rx_buffer = &rx_ring->rx_bi[rx_ring->next_to_clean];
+	prefetchw(rx_buffer->page);
+
+	/* we are reusing so sync this buffer for CPU use */
+	dma_sync_single_range_for_cpu(rx_ring->dev,
+				      rx_buffer->dma,
+				      rx_buffer->page_offset,
+				      size,
+				      DMA_FROM_DEVICE);
+
+	return rx_buffer;
+}
+
+/**
  * i40evf_fetch_rx_buffer - Allocate skb and populate it
  * @rx_ring: rx descriptor ring to transact packets on
- * @rx_desc: descriptor containing info written by hardware
+ * @rx_buffer: rx buffer to pull data from
  * @size: size of buffer to add to skb
  *
  * This function allocates an skb on the fly, and populates it with the page
@@ -1125,19 +1151,13 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
  */
 static inline
 struct sk_buff *i40evf_fetch_rx_buffer(struct i40e_ring *rx_ring,
-				       union i40e_rx_desc *rx_desc,
+				       struct i40e_rx_buffer *rx_buffer,
 				       struct sk_buff *skb,
 				       unsigned int size)
 {
-	struct i40e_rx_buffer *rx_buffer;
-	struct page *page;
-
-	rx_buffer = &rx_ring->rx_bi[rx_ring->next_to_clean];
-	page = rx_buffer->page;
-	prefetchw(page);
-
 	if (likely(!skb)) {
-		void *page_addr = page_address(page) + rx_buffer->page_offset;
+		void *page_addr = page_address(rx_buffer->page) +
+				  rx_buffer->page_offset;
 
 		/* prefetch first cache line of first page */
 		prefetch(page_addr);
@@ -1153,21 +1173,8 @@ struct sk_buff *i40evf_fetch_rx_buffer(struct i40e_ring *rx_ring,
 			rx_ring->rx_stats.alloc_buff_failed++;
 			return NULL;
 		}
-
-		/* we will be copying header into skb->data in
-		 * pskb_may_pull so it is in our interest to prefetch
-		 * it now to avoid a possible cache miss
-		 */
-		prefetchw(skb->data);
 	}
 
-	/* we are reusing so sync this buffer for CPU use */
-	dma_sync_single_range_for_cpu(rx_ring->dev,
-				      rx_buffer->dma,
-				      rx_buffer->page_offset,
-				      size,
-				      DMA_FROM_DEVICE);
-
 	/* pull page into skb */
 	if (i40e_add_rx_frag(rx_ring, rx_buffer, size, skb)) {
 		/* hand second half of page back to the ring */
@@ -1240,6 +1247,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 	bool failure = false;
 
 	while (likely(total_rx_packets < budget)) {
+		struct i40e_rx_buffer *rx_buffer;
 		union i40e_rx_desc *rx_desc;
 		unsigned int size;
 		u16 vlan_tag;
@@ -1272,7 +1280,9 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		 */
 		dma_rmb();
 
-		skb = i40evf_fetch_rx_buffer(rx_ring, rx_desc, skb, size);
+		rx_buffer = i40e_get_rx_buffer(rx_ring, size);
+
+		skb = i40evf_fetch_rx_buffer(rx_ring, rx_buffer, skb, size);
 		if (!skb)
 			break;
 
-- 
2.4.11

[Intel-wired-lan] [next PATCH S63 3/6] i40e/i40evf: Pull out code for cleaning up Rx buffers

[Bimmy Pujari]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch pulls out the code responsible for handling buffer recycling and
page counting and distributes it through several functions.  This allows us
to commonize the bits that handle either freeing or recycling the buffers.

As far as the page count tracking one change to the logic is that
pagecnt_bias is decremented as soon as we call i40e_get_rx_buffer.  It is
then the responsibility of the function that pulls the data to either
increment the pagecnt_bias if the buffer can be recycled as-is, or to
update page_offset so that we are pointing at the correct location for
placement of the next buffer.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: Ibac576360cb7f0b1627f2a993d13c1a8a2bf60af
---
Testing Hints:
        The greatest risk with this patch is a memory leak of some sort.
        I already caught one spot where I hadn't fully thought things out
        in regards to the path where we don't support bulk page updates.
        My advice would be to test on a RHEL 6.X kernel as well as a RHEL
        7.X kernel as the 6.X won't support bulk page count updates while
        the 7.3 and later kernels do.

 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 73 +++++++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 72 ++++++++++++++++----------
 2 files changed, 89 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index d1fc0f0..d7c4e1e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1294,6 +1294,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring,
 	bi->dma = dma;
 	bi->page = page;
 	bi->page_offset = 0;
+
+	/* initialize pagecnt_bias to 1 representing we fully own page */
 	bi->pagecnt_bias = 1;
 
 	return true;
@@ -1622,8 +1624,6 @@ static inline bool i40e_page_is_reusable(struct page *page)
  * the adapter for another receive
  *
  * @rx_buffer: buffer containing the page
- * @page: page address from rx_buffer
- * @truesize: actual size of the buffer in this page
  *
  * If page is reusable, rx_buffer->page_offset is adjusted to point to
  * an unused region in the page.
@@ -1646,14 +1646,13 @@ static inline bool i40e_page_is_reusable(struct page *page)
  *
  * In either case, if the page is reusable its refcount is increased.
  **/
-static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
-				   struct page *page,
-				   const unsigned int truesize)
+static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
 {
 #if (PAGE_SIZE >= 8192)
 	unsigned int last_offset = PAGE_SIZE - I40E_RXBUFFER_2048;
 #endif
-	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias--;
+	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias;
+	struct page *page = rx_buffer->page;
 
 	/* Is any reuse possible? */
 	if (unlikely(!i40e_page_is_reusable(page)))
@@ -1661,15 +1660,9 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
 
 #if (PAGE_SIZE < 8192)
 	/* if we are only owner of page we can reuse it */
-	if (unlikely(page_count(page) != pagecnt_bias))
+	if (unlikely((page_count(page) - pagecnt_bias) > 1))
 		return false;
-
-	/* flip page offset to other buffer */
-	rx_buffer->page_offset ^= truesize;
 #else
-	/* move offset up to the next cache line */
-	rx_buffer->page_offset += truesize;
-
 	if (rx_buffer->page_offset > last_offset)
 		return false;
 #endif
@@ -1678,10 +1671,11 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
 	 * the pagecnt_bias and page count so that we fully restock the
 	 * number of references the driver holds.
 	 */
-	if (unlikely(pagecnt_bias == 1)) {
+	if (unlikely(!pagecnt_bias)) {
 		page_ref_add(page, USHRT_MAX);
 		rx_buffer->pagecnt_bias = USHRT_MAX;
 	}
+
 	return true;
 }
 
@@ -1689,8 +1683,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
  * i40e_add_rx_frag - Add contents of Rx buffer to sk_buff
  * @rx_ring: rx descriptor ring to transact packets on
  * @rx_buffer: buffer containing page to add
- * @size: packet length from rx_desc
  * @skb: sk_buff to place the data into
+ * @size: packet length from rx_desc
  *
  * This function will add the data contained in rx_buffer->page to the skb.
  * This is done either through a direct copy if the data in the buffer is
@@ -1700,10 +1694,10 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
  * The function will then update the page offset if necessary and return
  * true if the buffer can be reused by the adapter.
  **/
-static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
+static void i40e_add_rx_frag(struct i40e_ring *rx_ring,
 			     struct i40e_rx_buffer *rx_buffer,
-			     unsigned int size,
-			     struct sk_buff *skb)
+			     struct sk_buff *skb,
+			     unsigned int size)
 {
 	struct page *page = rx_buffer->page;
 	unsigned char *va = page_address(page) + rx_buffer->page_offset;
@@ -1723,12 +1717,11 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 	if (size <= I40E_RX_HDR_SIZE) {
 		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
 
-		/* page is reusable, we can reuse buffer as-is */
-		if (likely(i40e_page_is_reusable(page)))
-			return true;
-
-		/* this page cannot be reused so discard it */
-		return false;
+		/* page is to be freed, increase pagecnt_bias instead of
+		 * decreasing page count.
+		 */
+		rx_buffer->pagecnt_bias++;
+		return;
 	}
 
 	/* we need the header to contain the greater of either
@@ -1750,7 +1743,12 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
 			(unsigned long)va & ~PAGE_MASK, size, truesize);
 
-	return i40e_can_reuse_rx_page(rx_buffer, page, truesize);
+	/* page is being used so we must update the page offset */
+#if (PAGE_SIZE < 8192)
+	rx_buffer->page_offset ^= truesize;
+#else
+	rx_buffer->page_offset += truesize;
+#endif
 }
 
 /**
@@ -1776,6 +1774,9 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
 				      size,
 				      DMA_FROM_DEVICE);
 
+	/* We have pulled a buffer for use, so decrement pagecnt_bias */
+	rx_buffer->pagecnt_bias--;
+
 	return rx_buffer;
 }
 
@@ -1812,12 +1813,29 @@ struct sk_buff *i40e_fetch_rx_buffer(struct i40e_ring *rx_ring,
 				       GFP_ATOMIC | __GFP_NOWARN);
 		if (unlikely(!skb)) {
 			rx_ring->rx_stats.alloc_buff_failed++;
+			rx_buffer->pagecnt_bias++;
 			return NULL;
 		}
 	}
 
 	/* pull page into skb */
-	if (i40e_add_rx_frag(rx_ring, rx_buffer, size, skb)) {
+	i40e_add_rx_frag(rx_ring, rx_buffer, skb, size);
+
+	return skb;
+}
+
+/**
+ * i40e_put_rx_buffer - Clean up used buffer and either recycle or free
+ * @rx_ring: rx descriptor ring to transact packets on
+ * @rx_buffer: rx buffer to pull data from
+ *
+ * This function will clean up the contents of the rx_buffer.  It will
+ * either recycle the bufer or unmap it and free the associated resources.
+ */
+static void i40e_put_rx_buffer(struct i40e_ring *rx_ring,
+			       struct i40e_rx_buffer *rx_buffer)
+{
+	if (i40e_can_reuse_rx_page(rx_buffer)) {
 		/* hand second half of page back to the ring */
 		i40e_reuse_rx_page(rx_ring, rx_buffer);
 		rx_ring->rx_stats.page_reuse_count++;
@@ -1831,8 +1849,6 @@ struct sk_buff *i40e_fetch_rx_buffer(struct i40e_ring *rx_ring,
 
 	/* clear contents of buffer_info */
 	rx_buffer->page = NULL;
-
-	return skb;
 }
 
 /**
@@ -1932,6 +1948,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		if (!skb)
 			break;
 
+		i40e_put_rx_buffer(rx_ring, rx_buffer);
 		cleaned_count++;
 
 		if (i40e_is_non_eop(rx_ring, rx_desc, skb))
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 2320ec4..06b3779 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -662,6 +662,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring,
 	bi->dma = dma;
 	bi->page = page;
 	bi->page_offset = 0;
+
+	/* initialize pagecnt_bias to 1 representing we fully own page */
 	bi->pagecnt_bias = 1;
 
 	return true;
@@ -980,8 +982,6 @@ static inline bool i40e_page_is_reusable(struct page *page)
  * the adapter for another receive
  *
  * @rx_buffer: buffer containing the page
- * @page: page address from rx_buffer
- * @truesize: actual size of the buffer in this page
  *
  * If page is reusable, rx_buffer->page_offset is adjusted to point to
  * an unused region in the page.
@@ -1004,14 +1004,13 @@ static inline bool i40e_page_is_reusable(struct page *page)
  *
  * In either case, if the page is reusable its refcount is increased.
  **/
-static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
-				   struct page *page,
-				   const unsigned int truesize)
+static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
 {
 #if (PAGE_SIZE >= 8192)
 	unsigned int last_offset = PAGE_SIZE - I40E_RXBUFFER_2048;
 #endif
-	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias--;
+	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias;
+	struct page *page = rx_buffer->page;
 
 	/* Is any reuse possible? */
 	if (unlikely(!i40e_page_is_reusable(page)))
@@ -1019,15 +1018,9 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
 
 #if (PAGE_SIZE < 8192)
 	/* if we are only owner of page we can reuse it */
-	if (unlikely(page_count(page) != pagecnt_bias))
+	if (unlikely((page_count(page) - pagecnt_bias) > 1))
 		return false;
-
-	/* flip page offset to other buffer */
-	rx_buffer->page_offset ^= truesize;
 #else
-	/* move offset up to the next cache line */
-	rx_buffer->page_offset += truesize;
-
 	if (rx_buffer->page_offset > last_offset)
 		return false;
 #endif
@@ -1036,7 +1029,7 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
 	 * the pagecnt_bias and page count so that we fully restock the
 	 * number of references the driver holds.
 	 */
-	if (unlikely(pagecnt_bias == 1)) {
+	if (unlikely(!pagecnt_bias)) {
 		page_ref_add(page, USHRT_MAX);
 		rx_buffer->pagecnt_bias = USHRT_MAX;
 	}
@@ -1048,8 +1041,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
  * i40e_add_rx_frag - Add contents of Rx buffer to sk_buff
  * @rx_ring: rx descriptor ring to transact packets on
  * @rx_buffer: buffer containing page to add
- * @size: packet length from rx_desc
  * @skb: sk_buff to place the data into
+ * @size: packet length from rx_desc
  *
  * This function will add the data contained in rx_buffer->page to the skb.
  * This is done either through a direct copy if the data in the buffer is
@@ -1059,10 +1052,10 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
  * The function will then update the page offset if necessary and return
  * true if the buffer can be reused by the adapter.
  **/
-static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
+static void i40e_add_rx_frag(struct i40e_ring *rx_ring,
 			     struct i40e_rx_buffer *rx_buffer,
-			     unsigned int size,
-			     struct sk_buff *skb)
+			     struct sk_buff *skb,
+			     unsigned int size)
 {
 	struct page *page = rx_buffer->page;
 	unsigned char *va = page_address(page) + rx_buffer->page_offset;
@@ -1082,12 +1075,11 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 	if (size <= I40E_RX_HDR_SIZE) {
 		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
 
-		/* page is reusable, we can reuse buffer as-is */
-		if (likely(i40e_page_is_reusable(page)))
-			return true;
-
-		/* this page cannot be reused so discard it */
-		return false;
+		/* page is to be freed, increase pagecnt_bias instead of
+		 * decreasing page count.
+		 */
+		rx_buffer->pagecnt_bias++;
+		return;
 	}
 
 	/* we need the header to contain the greater of either
@@ -1109,7 +1101,12 @@ static bool i40e_add_rx_frag(struct i40e_ring *rx_ring,
 	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
 			(unsigned long)va & ~PAGE_MASK, size, truesize);
 
-	return i40e_can_reuse_rx_page(rx_buffer, page, truesize);
+	/* page is being used so we must update the page offset */
+#if (PAGE_SIZE < 8192)
+	rx_buffer->page_offset ^= truesize;
+#else
+	rx_buffer->page_offset += truesize;
+#endif
 }
 
 /**
@@ -1135,6 +1132,9 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
 				      size,
 				      DMA_FROM_DEVICE);
 
+	/* We have pulled a buffer for use, so decrement pagecnt_bias */
+	rx_buffer->pagecnt_bias--;
+
 	return rx_buffer;
 }
 
@@ -1171,12 +1171,29 @@ struct sk_buff *i40evf_fetch_rx_buffer(struct i40e_ring *rx_ring,
 				       GFP_ATOMIC | __GFP_NOWARN);
 		if (unlikely(!skb)) {
 			rx_ring->rx_stats.alloc_buff_failed++;
+			rx_buffer->pagecnt_bias++;
 			return NULL;
 		}
 	}
 
 	/* pull page into skb */
-	if (i40e_add_rx_frag(rx_ring, rx_buffer, size, skb)) {
+	i40e_add_rx_frag(rx_ring, rx_buffer, skb, size);
+
+	return skb;
+}
+
+/**
+ * i40e_put_rx_buffer - Clean up used buffer and either recycle or free
+ * @rx_ring: rx descriptor ring to transact packets on
+ * @rx_buffer: rx buffer to pull data from
+ *
+ * This function will clean up the contents of the rx_buffer.  It will
+ * either recycle the bufer or unmap it and free the associated resources.
+ */
+static void i40e_put_rx_buffer(struct i40e_ring *rx_ring,
+			       struct i40e_rx_buffer *rx_buffer)
+{
+	if (i40e_can_reuse_rx_page(rx_buffer)) {
 		/* hand second half of page back to the ring */
 		i40e_reuse_rx_page(rx_ring, rx_buffer);
 		rx_ring->rx_stats.page_reuse_count++;
@@ -1190,8 +1207,6 @@ struct sk_buff *i40evf_fetch_rx_buffer(struct i40e_ring *rx_ring,
 
 	/* clear contents of buffer_info */
 	rx_buffer->page = NULL;
-
-	return skb;
 }
 
 /**
@@ -1286,6 +1301,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		if (!skb)
 			break;
 
+		i40e_put_rx_buffer(rx_ring, rx_buffer);
 		cleaned_count++;
 
 		if (i40e_is_non_eop(rx_ring, rx_desc, skb))
-- 
2.4.11

[Intel-wired-lan] [next PATCH S63 4/6] i40e/i40evf: Break i40e_fetch_rx_buffer up to allow for reuse of frag code

[Bimmy Pujari]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch is meant to clean up the code in preparation for us adding
support for build_skb.  Specifically we deconstruct i40e_fetch_buffer into
several functions so that those functions can later be reused when we add a
path for build_skb.

Specifically with this change we split out the code for adding a page to an
exiting skb.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: Iab1efbab6b8b97cb60ab9fdd0be1d37a056a154d
---
Testing Hints:
        The greatest risk with this patch is a memory leak of some sort.
        I already caught one spot where I hadn't fully thought things out
        in regards to the path where we don't support bulk page updates.
        My advice would be to test on a RHEL 6.X kernel as well as a RHEL
        7.X kernel as the 6.X won't support bulk page count updates while
        the 7.3 and later kernels do.

 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 138 ++++++++++++--------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 138 ++++++++++++--------------
 2 files changed, 130 insertions(+), 146 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index d7c4e1e..433309f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1687,61 +1687,23 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
  * @size: packet length from rx_desc
  *
  * This function will add the data contained in rx_buffer->page to the skb.
- * This is done either through a direct copy if the data in the buffer is
- * less than the skb header size, otherwise it will just attach the page as
- * a frag to the skb.
+ * It will just attach the page as a frag to the skb.
  *
- * The function will then update the page offset if necessary and return
- * true if the buffer can be reused by the adapter.
+ * The function will then update the page offset.
  **/
 static void i40e_add_rx_frag(struct i40e_ring *rx_ring,
 			     struct i40e_rx_buffer *rx_buffer,
 			     struct sk_buff *skb,
 			     unsigned int size)
 {
-	struct page *page = rx_buffer->page;
-	unsigned char *va = page_address(page) + rx_buffer->page_offset;
 #if (PAGE_SIZE < 8192)
 	unsigned int truesize = I40E_RXBUFFER_2048;
 #else
-	unsigned int truesize = ALIGN(size, L1_CACHE_BYTES);
+	unsigned int truesize = SKB_DATA_ALIGN(size);
 #endif
-	unsigned int pull_len;
-
-	if (unlikely(skb_is_nonlinear(skb)))
-		goto add_tail_frag;
-
-	/* will the data fit in the skb we allocated? if so, just
-	 * copy it as it is pretty small anyway
-	 */
-	if (size <= I40E_RX_HDR_SIZE) {
-		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
-
-		/* page is to be freed, increase pagecnt_bias instead of
-		 * decreasing page count.
-		 */
-		rx_buffer->pagecnt_bias++;
-		return;
-	}
-
-	/* we need the header to contain the greater of either
-	 * ETH_HLEN or 60 bytes if the skb->len is less than
-	 * 60 for skb_pad.
-	 */
-	pull_len = eth_get_headlen(va, I40E_RX_HDR_SIZE);
-
-	/* align pull length to size of long to optimize
-	 * memcpy performance
-	 */
-	memcpy(__skb_put(skb, pull_len), va, ALIGN(pull_len, sizeof(long)));
-
-	/* update all of the pointers */
-	va += pull_len;
-	size -= pull_len;
 
-add_tail_frag:
-	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
-			(unsigned long)va & ~PAGE_MASK, size, truesize);
+	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, rx_buffer->page,
+			rx_buffer->page_offset, size, truesize);
 
 	/* page is being used so we must update the page offset */
 #if (PAGE_SIZE < 8192)
@@ -1781,45 +1743,66 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
 }
 
 /**
- * i40e_fetch_rx_buffer - Allocate skb and populate it
+ * i40e_construct_skb - Allocate skb and populate it
  * @rx_ring: rx descriptor ring to transact packets on
  * @rx_buffer: rx buffer to pull data from
  * @size: size of buffer to add to skb
  *
- * This function allocates an skb on the fly, and populates it with the page
- * data from the current receive descriptor, taking care to set up the skb
- * correctly, as well as handling calling the page recycle function if
- * necessary.
+ * This function allocates an skb.  It then populates it with the page
+ * data from the current receive descriptor, taking care to set up the
+ * skb correctly.
  */
-static inline
-struct sk_buff *i40e_fetch_rx_buffer(struct i40e_ring *rx_ring,
-				     struct i40e_rx_buffer *rx_buffer,
-				     struct sk_buff *skb,
-				     unsigned int size)
+static struct sk_buff *i40e_construct_skb(struct i40e_ring *rx_ring,
+					  struct i40e_rx_buffer *rx_buffer,
+					  unsigned int size)
 {
-	if (likely(!skb)) {
-		void *page_addr = page_address(rx_buffer->page) +
-				  rx_buffer->page_offset;
+	void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
+#if (PAGE_SIZE < 8192)
+	unsigned int truesize = I40E_RXBUFFER_2048;
+#else
+	unsigned int truesize = SKB_DATA_ALIGN(size);
+#endif
+	unsigned int headlen;
+	struct sk_buff *skb;
 
-		/* prefetch first cache line of first page */
-		prefetch(page_addr);
+	/* prefetch first cache line of first page */
+	prefetch(va);
 #if L1_CACHE_BYTES < 128
-		prefetch(page_addr + L1_CACHE_BYTES);
+	prefetch(va + L1_CACHE_BYTES);
 #endif
 
-		/* allocate a skb to store the frags */
-		skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-				       I40E_RX_HDR_SIZE,
-				       GFP_ATOMIC | __GFP_NOWARN);
-		if (unlikely(!skb)) {
-			rx_ring->rx_stats.alloc_buff_failed++;
-			rx_buffer->pagecnt_bias++;
-			return NULL;
-		}
-	}
+	/* allocate a skb to store the frags */
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
+			       I40E_RX_HDR_SIZE,
+			       GFP_ATOMIC | __GFP_NOWARN);
+	if (unlikely(!skb))
+		return NULL;
+
+	/* Determine available headroom for copy */
+	headlen = size;
+	if (headlen > I40E_RX_HDR_SIZE)
+		headlen = eth_get_headlen(va, I40E_RX_HDR_SIZE);
 
-	/* pull page into skb */
-	i40e_add_rx_frag(rx_ring, rx_buffer, skb, size);
+	/* align pull length to size of long to optimize memcpy performance */
+	memcpy(__skb_put(skb, headlen), va, ALIGN(headlen, sizeof(long)));
+
+	/* update all of the pointers */
+	size -= headlen;
+	if (size) {
+		skb_add_rx_frag(skb, 0, rx_buffer->page,
+				rx_buffer->page_offset + headlen,
+				size, truesize);
+
+		/* buffer is used by skb, update page_offset */
+#if (PAGE_SIZE < 8192)
+		rx_buffer->page_offset ^= truesize;
+#else
+		rx_buffer->page_offset += truesize;
+#endif
+	} else {
+		/* buffer is unused, reset bias back to rx_buffer */
+		rx_buffer->pagecnt_bias++;
+	}
 
 	return skb;
 }
@@ -1944,9 +1927,18 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 		rx_buffer = i40e_get_rx_buffer(rx_ring, size);
 
-		skb = i40e_fetch_rx_buffer(rx_ring, rx_buffer, skb, size);
-		if (!skb)
+		/* retrieve a buffer from the ring */
+		if (skb)
+			i40e_add_rx_frag(rx_ring, rx_buffer, skb, size);
+		else
+			skb = i40e_construct_skb(rx_ring, rx_buffer, size);
+
+		/* exit if we failed to retrieve a buffer */
+		if (!skb) {
+			rx_ring->rx_stats.alloc_buff_failed++;
+			rx_buffer->pagecnt_bias++;
 			break;
+		}
 
 		i40e_put_rx_buffer(rx_ring, rx_buffer);
 		cleaned_count++;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 06b3779..95e383a 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1045,61 +1045,23 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
  * @size: packet length from rx_desc
  *
  * This function will add the data contained in rx_buffer->page to the skb.
- * This is done either through a direct copy if the data in the buffer is
- * less than the skb header size, otherwise it will just attach the page as
- * a frag to the skb.
+ * It will just attach the page as a frag to the skb.
  *
- * The function will then update the page offset if necessary and return
- * true if the buffer can be reused by the adapter.
+ * The function will then update the page offset.
  **/
 static void i40e_add_rx_frag(struct i40e_ring *rx_ring,
 			     struct i40e_rx_buffer *rx_buffer,
 			     struct sk_buff *skb,
 			     unsigned int size)
 {
-	struct page *page = rx_buffer->page;
-	unsigned char *va = page_address(page) + rx_buffer->page_offset;
 #if (PAGE_SIZE < 8192)
 	unsigned int truesize = I40E_RXBUFFER_2048;
 #else
-	unsigned int truesize = ALIGN(size, L1_CACHE_BYTES);
+	unsigned int truesize = SKB_DATA_ALIGN(size);
 #endif
-	unsigned int pull_len;
-
-	if (unlikely(skb_is_nonlinear(skb)))
-		goto add_tail_frag;
-
-	/* will the data fit in the skb we allocated? if so, just
-	 * copy it as it is pretty small anyway
-	 */
-	if (size <= I40E_RX_HDR_SIZE) {
-		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
-
-		/* page is to be freed, increase pagecnt_bias instead of
-		 * decreasing page count.
-		 */
-		rx_buffer->pagecnt_bias++;
-		return;
-	}
-
-	/* we need the header to contain the greater of either
-	 * ETH_HLEN or 60 bytes if the skb->len is less than
-	 * 60 for skb_pad.
-	 */
-	pull_len = eth_get_headlen(va, I40E_RX_HDR_SIZE);
-
-	/* align pull length to size of long to optimize
-	 * memcpy performance
-	 */
-	memcpy(__skb_put(skb, pull_len), va, ALIGN(pull_len, sizeof(long)));
-
-	/* update all of the pointers */
-	va += pull_len;
-	size -= pull_len;
 
-add_tail_frag:
-	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
-			(unsigned long)va & ~PAGE_MASK, size, truesize);
+	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, rx_buffer->page,
+			rx_buffer->page_offset, size, truesize);
 
 	/* page is being used so we must update the page offset */
 #if (PAGE_SIZE < 8192)
@@ -1139,45 +1101,66 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
 }
 
 /**
- * i40evf_fetch_rx_buffer - Allocate skb and populate it
+ * i40e_construct_skb - Allocate skb and populate it
  * @rx_ring: rx descriptor ring to transact packets on
  * @rx_buffer: rx buffer to pull data from
  * @size: size of buffer to add to skb
  *
- * This function allocates an skb on the fly, and populates it with the page
- * data from the current receive descriptor, taking care to set up the skb
- * correctly, as well as handling calling the page recycle function if
- * necessary.
+ * This function allocates an skb.  It then populates it with the page
+ * data from the current receive descriptor, taking care to set up the
+ * skb correctly.
  */
-static inline
-struct sk_buff *i40evf_fetch_rx_buffer(struct i40e_ring *rx_ring,
-				       struct i40e_rx_buffer *rx_buffer,
-				       struct sk_buff *skb,
-				       unsigned int size)
+static struct sk_buff *i40e_construct_skb(struct i40e_ring *rx_ring,
+					  struct i40e_rx_buffer *rx_buffer,
+					  unsigned int size)
 {
-	if (likely(!skb)) {
-		void *page_addr = page_address(rx_buffer->page) +
-				  rx_buffer->page_offset;
+	void *va = page_address(rx_buffer->page) + rx_buffer->page_offset;
+#if (PAGE_SIZE < 8192)
+	unsigned int truesize = I40E_RXBUFFER_2048;
+#else
+	unsigned int truesize = SKB_DATA_ALIGN(size);
+#endif
+	unsigned int headlen;
+	struct sk_buff *skb;
 
-		/* prefetch first cache line of first page */
-		prefetch(page_addr);
+	/* prefetch first cache line of first page */
+	prefetch(va);
 #if L1_CACHE_BYTES < 128
-		prefetch(page_addr + L1_CACHE_BYTES);
+	prefetch(va + L1_CACHE_BYTES);
 #endif
 
-		/* allocate a skb to store the frags */
-		skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
-				       I40E_RX_HDR_SIZE,
-				       GFP_ATOMIC | __GFP_NOWARN);
-		if (unlikely(!skb)) {
-			rx_ring->rx_stats.alloc_buff_failed++;
-			rx_buffer->pagecnt_bias++;
-			return NULL;
-		}
-	}
+	/* allocate a skb to store the frags */
+	skb = __napi_alloc_skb(&rx_ring->q_vector->napi,
+			       I40E_RX_HDR_SIZE,
+			       GFP_ATOMIC | __GFP_NOWARN);
+	if (unlikely(!skb))
+		return NULL;
+
+	/* Determine available headroom for copy */
+	headlen = size;
+	if (headlen > I40E_RX_HDR_SIZE)
+		headlen = eth_get_headlen(va, I40E_RX_HDR_SIZE);
 
-	/* pull page into skb */
-	i40e_add_rx_frag(rx_ring, rx_buffer, skb, size);
+	/* align pull length to size of long to optimize memcpy performance */
+	memcpy(__skb_put(skb, headlen), va, ALIGN(headlen, sizeof(long)));
+
+	/* update all of the pointers */
+	size -= headlen;
+	if (size) {
+		skb_add_rx_frag(skb, 0, rx_buffer->page,
+				rx_buffer->page_offset + headlen,
+				size, truesize);
+
+		/* buffer is used by skb, update page_offset */
+#if (PAGE_SIZE < 8192)
+		rx_buffer->page_offset ^= truesize;
+#else
+		rx_buffer->page_offset += truesize;
+#endif
+	} else {
+		/* buffer is unused, reset bias back to rx_buffer */
+		rx_buffer->pagecnt_bias++;
+	}
 
 	return skb;
 }
@@ -1297,9 +1280,18 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 		rx_buffer = i40e_get_rx_buffer(rx_ring, size);
 
-		skb = i40evf_fetch_rx_buffer(rx_ring, rx_buffer, skb, size);
-		if (!skb)
+		/* retrieve a buffer from the ring */
+		if (skb)
+			i40e_add_rx_frag(rx_ring, rx_buffer, skb, size);
+		else
+			skb = i40e_construct_skb(rx_ring, rx_buffer, size);
+
+		/* exit if we failed to retrieve a buffer */
+		if (!skb) {
+			rx_ring->rx_stats.alloc_buff_failed++;
+			rx_buffer->pagecnt_bias++;
 			break;
+		}
 
 		i40e_put_rx_buffer(rx_ring, rx_buffer);
 		cleaned_count++;
-- 
2.4.11

[Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx private flag to allow fallback to old Rx flow

[Bimmy Pujari]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch adds a control which will allow us to toggle into and out of the
legacy Rx mode.  The legacy Rx mode is what we currently do when performing
Rx.  As I make further changes what should happen is that the driver will
fall back to the behavior for Rx as of this patch should the "legacy-rx"
flag be set to on.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: I0342998849bbb31351cce05f6e182c99174e7751
---
Testing Hints:
        This flag should not appear on kernels prior to 4.10.
        On kernels 4.10 and later the "legacy-rx" flag should appear
        and can be toggled between on and off. As we add more features the
        behavior of the driver should change when this value is toggled.

        Toggling this for now will only cause a reset of the interface.

        For the VF we are specifically replacing the define that was
        stripping the code for the flag needed for the "legacy-rx" flag
        since it doesn't make sense to have the interface defined when
        there are no flags present.  In the case of a kernel build the
        #ifdefs should be stripped and the code will remain.

 drivers/net/ethernet/intel/i40e/i40e.h             |   1 +
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |   5 +-
 drivers/net/ethernet/intel/i40evf/i40evf.h         |   2 +
 drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 104 +++++++++++++++++++++
 4 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index dcba258..561fe1f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -430,6 +430,7 @@ struct i40e_pf {
 #define I40E_FLAG_TEMP_LINK_POLLING		BIT_ULL(55)
 #define I40E_FLAG_CLIENT_L2_CHANGE		BIT_ULL(56)
 #define I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE		BIT_ULL(57)
+#define I40E_FLAG_LEGACY_RX			BIT_ULL(57)
 
        /* Tracks features that are disabled due to hw limitations.
 	* If a bit is set here, it means that the corresponding
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 56dccf8..0db6b36 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -226,6 +226,7 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
 	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
 	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
 	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_CAPABLE, 0),
+	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
 };
 
 #define I40E_PRIV_FLAGS_STR_LEN ARRAY_SIZE(i40e_gstrings_priv_flags)
@@ -4048,6 +4049,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 	}
 
 flags_complete:
+	/* check for flags that changed */
 	changed_flags ^= pf->flags;
 
 	/* Process any additional changes needed as a result of flag changes.
@@ -4088,7 +4090,8 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 	/* Issue reset to cause things to take effect, as additional bits
 	 * are added we will need to create a mask of bits requiring reset
 	 */
-	if (changed_flags & I40E_FLAG_VEB_STATS_ENABLED)
+	if ((changed_flags & I40E_FLAG_VEB_STATS_ENABLED) ||
+	    ((changed_flags & I40E_FLAG_LEGACY_RX) && netif_running(dev)))
 		i40e_do_reset(pf, BIT(__I40E_PF_RESET_REQUESTED));
 
 	return 0;
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index b2b4851..e60cbfa 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -222,6 +222,7 @@ struct i40evf_adapter {
 #define I40EVF_FLAG_CLIENT_NEEDS_L2_PARAMS	BIT(17)
 #define I40EVF_FLAG_PROMISC_ON			BIT(18)
 #define I40EVF_FLAG_ALLMULTI_ON			BIT(19)
+#define I40EVF_FLAG_LEGACY_RX			BIT(20)
 /* duplicates for common code */
 #define I40E_FLAG_FDIR_ATR_ENABLED		0
 #define I40E_FLAG_DCB_ENABLED			0
@@ -229,6 +230,7 @@ struct i40evf_adapter {
 #define I40E_FLAG_RX_CSUM_ENABLED		I40EVF_FLAG_RX_CSUM_ENABLED
 #define I40E_FLAG_WB_ON_ITR_CAPABLE		I40EVF_FLAG_WB_ON_ITR_CAPABLE
 #define I40E_FLAG_OUTER_UDP_CSUM_CAPABLE	I40EVF_FLAG_OUTER_UDP_CSUM_CAPABLE
+#define I40E_FLAG_LEGACY_RX			I40EVF_FLAG_LEGACY_RX
 	/* flags for admin queue service task */
 	u32 aq_required;
 #define I40EVF_FLAG_AQ_ENABLE_QUEUES		BIT(0)
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
index 122efbd..9bb2cc7 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
@@ -63,6 +63,29 @@ static const struct i40evf_stats i40evf_gstrings_stats[] = {
 #define I40EVF_STATS_LEN(_dev) \
 	(I40EVF_GLOBAL_STATS_LEN + I40EVF_QUEUE_STATS_LEN(_dev))
 
+/* For now we have one and only one private flag and it is only defined
+ * when we have support for the SKIP_CPU_SYNC DMA attribute.  Instead
+ * of leaving all this code sitting around empty we will strip it unless
+ * our one private flag is actually available.
+ */
+struct i40evf_priv_flags {
+	char flag_string[ETH_GSTRING_LEN];
+	u32 flag;
+	bool read_only;
+};
+
+#define I40EVF_PRIV_FLAG(_name, _flag, _read_only) { \
+	.flag_string = _name, \
+	.flag = _flag, \
+	.read_only = _read_only, \
+}
+
+static const struct i40evf_priv_flags i40evf_gstrings_priv_flags[] = {
+	I40EVF_PRIV_FLAG("legacy-rx", I40EVF_FLAG_LEGACY_RX, 0),
+};
+
+#define I40EVF_PRIV_FLAGS_STR_LEN ARRAY_SIZE(i40evf_gstrings_priv_flags)
+
 /**
  * i40evf_get_link_ksettings - Get Link Speed and Duplex settings
  * @netdev: network interface device structure
@@ -124,6 +147,8 @@ static int i40evf_get_sset_count(struct net_device *netdev, int sset)
 {
 	if (sset == ETH_SS_STATS)
 		return I40EVF_STATS_LEN(netdev);
+	else if (sset == ETH_SS_PRIV_FLAGS)
+		return I40EVF_PRIV_FLAGS_STR_LEN;
 	else
 		return -EINVAL;
 }
@@ -189,7 +214,83 @@ static void i40evf_get_strings(struct net_device *netdev, u32 sset, u8 *data)
 			snprintf(p, ETH_GSTRING_LEN, "rx-%u.bytes", i);
 			p += ETH_GSTRING_LEN;
 		}
+	} else if (sset == ETH_SS_PRIV_FLAGS) {
+		for (i = 0; i < I40EVF_PRIV_FLAGS_STR_LEN; i++) {
+			snprintf(p, ETH_GSTRING_LEN, "%s",
+				 i40evf_gstrings_priv_flags[i].flag_string);
+			p += ETH_GSTRING_LEN;
+		}
+	}
+}
+
+/**
+ * i40evf_get_priv_flags - report device private flags
+ * @dev: network interface device structure
+ *
+ * The get string set count and the string set should be matched for each
+ * flag returned.  Add new strings for each flag to the i40e_gstrings_priv_flags
+ * array.
+ *
+ * Returns a u32 bitmap of flags.
+ **/
+static u32 i40evf_get_priv_flags(struct net_device *netdev)
+{
+	struct i40evf_adapter *adapter = netdev_priv(netdev);
+	u32 i, ret_flags = 0;
+
+	for (i = 0; i < I40EVF_PRIV_FLAGS_STR_LEN; i++) {
+		const struct i40evf_priv_flags *priv_flags;
+
+		priv_flags = &i40evf_gstrings_priv_flags[i];
+
+		if (priv_flags->flag & adapter->flags)
+			ret_flags |= BIT(i);
+	}
+
+	return ret_flags;
+}
+
+/**
+ * i40evf_set_priv_flags - set private flags
+ * @dev: network interface device structure
+ * @flags: bit flags to be set
+ **/
+static int i40evf_set_priv_flags(struct net_device *netdev, u32 flags)
+{
+	struct i40evf_adapter *adapter = netdev_priv(netdev);
+	u64 changed_flags;
+	u32 i;
+
+	changed_flags = adapter->flags;
+
+	for (i = 0; i < I40EVF_PRIV_FLAGS_STR_LEN; i++) {
+		const struct i40evf_priv_flags *priv_flags;
+
+		priv_flags = &i40evf_gstrings_priv_flags[i];
+
+		if (priv_flags->read_only)
+			continue;
+
+		if (flags & BIT(i))
+			adapter->flags |= priv_flags->flag;
+		else
+			adapter->flags &= ~(priv_flags->flag);
+	}
+
+	/* check for flags that changed */
+	changed_flags ^= adapter->flags;
+
+	/* Process any additional changes needed as a result of flag changes. */
+
+	/* issue a reset to force legacy-rx change to take effect */
+	if (changed_flags & I40EVF_FLAG_LEGACY_RX) {
+		if (netif_running(netdev)) {
+			adapter->flags |= I40EVF_FLAG_RESET_NEEDED;
+			schedule_work(&adapter->reset_task);
+		}
 	}
+
+	return 0;
 }
 
 /**
@@ -238,6 +339,7 @@ static void i40evf_get_drvinfo(struct net_device *netdev,
 	strlcpy(drvinfo->version, i40evf_driver_version, 32);
 	strlcpy(drvinfo->fw_version, "N/A", 4);
 	strlcpy(drvinfo->bus_info, pci_name(adapter->pdev), 32);
+	drvinfo->n_priv_flags = I40EVF_PRIV_FLAGS_STR_LEN;
 }
 
 /**
@@ -649,6 +751,8 @@ static const struct ethtool_ops i40evf_ethtool_ops = {
 	.get_strings		= i40evf_get_strings,
 	.get_ethtool_stats	= i40evf_get_ethtool_stats,
 	.get_sset_count		= i40evf_get_sset_count,
+	.get_priv_flags		= i40evf_get_priv_flags,
+	.set_priv_flags		= i40evf_set_priv_flags,
 	.get_msglevel		= i40evf_get_msglevel,
 	.set_msglevel		= i40evf_set_msglevel,
 	.get_coalesce		= i40evf_get_coalesce,
-- 
2.4.11

[Intel-wired-lan] [next PATCH S63 6/6] i40e/i40evf: Change the way we limit the maximum frame size for Rx

[Bimmy Pujari]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch changes the way we handle the maximum frame size for the Rx
path.  Previously we were rounding up to 2K for a 1500 MTU and then brining
the max frame size down to MTU plus a fixed amount.  With this patch
applied what we now do is limit the maximum frame to 1.5K minus the value
for NET_IP_ALIGN for standard MTU, and for any MTU greater than 1500 we
allow up to the maximum frame size.  This makes the behavior more
consistent with the other drivers such as igb which had similar logic.  In
addition it reduces the test matrix for MTU since we only have two max
frame sizes that are handled for Rx now.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Change-ID: I23a9d3c857e7df04b0ef28c64df63e659c013f3f
---
Testing Hints:
        Verify MTU handling for MTU = 1500.  You should be able to receive
        frames up to 1536 in size on a standard x86 system as NET_IP_ALIGN
        is 0.

        When jumbo frames are enabled you should be able to receive anything
        up to the maximum size supported by hardware which is 9.5K or 9726B
        in size.

        When the "legacy-rx" private flag is set you should be able to
        receive anything up to 9.5K in size.

        The behavior for MTU mismatches will change.  A failing case is any
        in which we trigger a kernel panic.  Receiving a frame larger than
        what was configured for the MTU is not a failure. 

 drivers/net/ethernet/intel/i40e/i40e_main.c        | 26 ++++++++++++----------
 drivers/net/ethernet/intel/i40e/i40e_txrx.h        |  4 +---
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h      |  4 +---
 drivers/net/ethernet/intel/i40evf/i40evf.h         |  4 ----
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 16 ++++++++++++-
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 14 ++++++++----
 6 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 4cfe9491..724a5a4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3002,7 +3002,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 
 	ring->rx_buf_len = vsi->rx_buf_len;
 
-	rx_ctx.dbuff = ring->rx_buf_len >> I40E_RXQ_CTX_DBUFF_SHIFT;
+	rx_ctx.dbuff = DIV_ROUND_UP(ring->rx_buf_len,
+				    BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
 
 	rx_ctx.base = (ring->dma / 128);
 	rx_ctx.qlen = ring->count;
@@ -3082,17 +3083,18 @@ static int i40e_vsi_configure_rx(struct i40e_vsi *vsi)
 	int err = 0;
 	u16 i;
 
-	if (vsi->netdev && (vsi->netdev->mtu > ETH_DATA_LEN))
-		vsi->max_frame = vsi->netdev->mtu + ETH_HLEN
-			       + ETH_FCS_LEN + VLAN_HLEN;
-	else
-		vsi->max_frame = I40E_RXBUFFER_2048;
-
-	vsi->rx_buf_len = I40E_RXBUFFER_2048;
-
-	/* round up for the chip's needs */
-	vsi->rx_buf_len = ALIGN(vsi->rx_buf_len,
-				BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
+	if (!vsi->netdev || (vsi->back->flags & I40E_FLAG_LEGACY_RX)) {
+		vsi->max_frame = I40E_MAX_RXBUFFER;
+		vsi->rx_buf_len = I40E_RXBUFFER_2048;
+#if (PAGE_SIZE < 8192)
+	} else if (vsi->netdev->mtu <= ETH_DATA_LEN) {
+		vsi->max_frame = I40E_RXBUFFER_1536 - NET_IP_ALIGN;
+		vsi->rx_buf_len = I40E_RXBUFFER_1536 - NET_IP_ALIGN;
+#endif
+	} else {
+		vsi->max_frame = I40E_MAX_RXBUFFER;
+		vsi->rx_buf_len = I40E_RXBUFFER_2048;
+	}
 
 	/* set up individual rings */
 	for (i = 0; i < vsi->num_queue_pairs && !err; i++)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index eb73372..d6609de 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -117,10 +117,8 @@ enum i40e_dyn_idx_t {
 
 /* Supported Rx Buffer Sizes (a multiple of 128) */
 #define I40E_RXBUFFER_256   256
+#define I40E_RXBUFFER_1536  1536  /* 128B aligned standard Ethernet frame */
 #define I40E_RXBUFFER_2048  2048
-#define I40E_RXBUFFER_3072  3072   /* For FCoE MTU of 2158 */
-#define I40E_RXBUFFER_4096  4096
-#define I40E_RXBUFFER_8192  8192
 #define I40E_MAX_RXBUFFER   9728  /* largest size for single descriptor */
 
 /* NOTE: netdev_alloc_skb reserves up to 64 bytes, NET_IP_ALIGN means we
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index aba40ed..3bb4d73 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -104,10 +104,8 @@ enum i40e_dyn_idx_t {
 
 /* Supported Rx Buffer Sizes (a multiple of 128) */
 #define I40E_RXBUFFER_256   256
+#define I40E_RXBUFFER_1536  1536  /* 128B aligned standard Ethernet frame */
 #define I40E_RXBUFFER_2048  2048
-#define I40E_RXBUFFER_3072  3072   /* For FCoE MTU of 2158 */
-#define I40E_RXBUFFER_4096  4096
-#define I40E_RXBUFFER_8192  8192
 #define I40E_MAX_RXBUFFER   9728  /* largest size for single descriptor */
 
 /* NOTE: netdev_alloc_skb reserves up to 64 bytes, NET_IP_ALIGN means we
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index e60cbfa..d61ecf6 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -72,10 +72,6 @@ struct i40e_vsi {
 #define I40EVF_MAX_RXD		4096
 #define I40EVF_MIN_RXD		64
 #define I40EVF_REQ_DESCRIPTOR_MULTIPLE	32
-
-/* Supported Rx Buffer Sizes */
-#define I40EVF_RXBUFFER_2048	2048
-#define I40EVF_MAX_RXBUFFER	16384  /* largest size for single descriptor */
 #define I40EVF_MAX_AQ_BUF_SIZE	4096
 #define I40EVF_AQ_LEN		32
 #define I40EVF_AQ_MAX_ERR	20 /* times to try before resetting AQ */
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 6d666bd..fb2811c 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -686,12 +686,26 @@ static void i40evf_configure_tx(struct i40evf_adapter *adapter)
  **/
 static void i40evf_configure_rx(struct i40evf_adapter *adapter)
 {
+	unsigned int rx_buf_len = I40E_RXBUFFER_2048;
+	struct net_device *netdev = adapter->netdev;
 	struct i40e_hw *hw = &adapter->hw;
 	int i;
 
+	/* Legacy Rx will always default to a 2048 buffer size. */
+#if (PAGE_SIZE < 8192)
+	if (!(adapter->flags & I40EVF_FLAG_LEGACY_RX)) {
+		/* We use a 1536 buffer size for configurations with
+		 * standard Ethernet mtu.  On x86 this gives us enough room
+		 * for shared info and 192 bytes of padding.
+		 */
+		if (netdev->mtu <= ETH_DATA_LEN)
+			rx_buf_len = I40E_RXBUFFER_1536 - NET_IP_ALIGN;
+	}
+#endif
+
 	for (i = 0; i < adapter->num_active_queues; i++) {
 		adapter->rx_rings[i].tail = hw->hw_addr + I40E_QRX_TAIL1(i);
-		adapter->rx_rings[i].rx_buf_len = I40EVF_RXBUFFER_2048;
+		adapter->rx_rings[i].rx_buf_len = rx_buf_len;
 	}
 }
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 4bc2488..032be8d 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -234,7 +234,7 @@ void i40evf_configure_queues(struct i40evf_adapter *adapter)
 	struct i40e_virtchnl_vsi_queue_config_info *vqci;
 	struct i40e_virtchnl_queue_pair_info *vqpi;
 	int pairs = adapter->num_active_queues;
-	int i, len;
+	int i, len, max_frame = I40E_MAX_RXBUFFER;
 
 	if (adapter->current_op != I40E_VIRTCHNL_OP_UNKNOWN) {
 		/* bail because we already have a command pending */
@@ -249,6 +249,11 @@ void i40evf_configure_queues(struct i40evf_adapter *adapter)
 	if (!vqci)
 		return;
 
+	/* Limit maximum frame size when jumbo frames is not enabled */
+	if (!(adapter->flags & I40EVF_FLAG_LEGACY_RX) &&
+	    (adapter->netdev->mtu <= ETH_DATA_LEN))
+		max_frame = I40E_RXBUFFER_1536 - NET_IP_ALIGN;
+
 	vqci->vsi_id = adapter->vsi_res->vsi_id;
 	vqci->num_queue_pairs = pairs;
 	vqpi = vqci->qpair;
@@ -264,9 +269,10 @@ void i40evf_configure_queues(struct i40evf_adapter *adapter)
 		vqpi->rxq.queue_id = i;
 		vqpi->rxq.ring_len = adapter->rx_rings[i].count;
 		vqpi->rxq.dma_ring_addr = adapter->rx_rings[i].dma;
-		vqpi->rxq.max_pkt_size = adapter->netdev->mtu
-					+ ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN;
-		vqpi->rxq.databuffer_size = adapter->rx_rings[i].rx_buf_len;
+		vqpi->rxq.max_pkt_size = max_frame;
+		vqpi->rxq.databuffer_size =
+			ALIGN(adapter->rx_rings[i].rx_buf_len,
+			      BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
 		vqpi++;
 	}
 
-- 
2.4.11

[Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx private flag to allow fallback to old Rx flow

[Alexander Duyck]

On Tue, Mar 14, 2017 at 10:15 AM, Bimmy Pujari <bimmy.pujari@intel.com> wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> This patch adds a control which will allow us to toggle into and out of the
> legacy Rx mode.  The legacy Rx mode is what we currently do when performing
> Rx.  As I make further changes what should happen is that the driver will
> fall back to the behavior for Rx as of this patch should the "legacy-rx"
> flag be set to on.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Change-ID: I0342998849bbb31351cce05f6e182c99174e7751
> ---
> Testing Hints:
>         This flag should not appear on kernels prior to 4.10.
>         On kernels 4.10 and later the "legacy-rx" flag should appear
>         and can be toggled between on and off. As we add more features the
>         behavior of the driver should change when this value is toggled.
>
>         Toggling this for now will only cause a reset of the interface.
>
>         For the VF we are specifically replacing the define that was
>         stripping the code for the flag needed for the "legacy-rx" flag
>         since it doesn't make sense to have the interface defined when
>         there are no flags present.  In the case of a kernel build the
>         #ifdefs should be stripped and the code will remain.
>
>  drivers/net/ethernet/intel/i40e/i40e.h             |   1 +
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |   5 +-
>  drivers/net/ethernet/intel/i40evf/i40evf.h         |   2 +
>  drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 104 +++++++++++++++++++++
>  4 files changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index dcba258..561fe1f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -430,6 +430,7 @@ struct i40e_pf {
>  #define I40E_FLAG_TEMP_LINK_POLLING            BIT_ULL(55)
>  #define I40E_FLAG_CLIENT_L2_CHANGE             BIT_ULL(56)
>  #define I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE                BIT_ULL(57)
> +#define I40E_FLAG_LEGACY_RX                    BIT_ULL(57)

It looks like we lost sync between upstream and out-of-tree at some
point.  We are defining both of these using the same bit.  We need to
use a different value for the LEGACY_RX bit in the upstream it looks
like.

[Intel-wired-lan] [next PATCH S63 1/6] i40e/i40evf: Use length to determine if descriptor is done

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Tuesday, March 14, 2017 10:15 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S63 1/6] i40e/i40evf: Use length to
> determine if descriptor is done
> 
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change makes it so that we use the length of the packet instead of the
> DD status bit to determine if a new descriptor is ready to be processed.
> The obvious advantage is that it cuts down on reads as we don't really even
> need the DD bit if going from a 0 to a non-zero value on size is enough to
> inform us that the packet has been completed.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Change-ID: Iebdf9cdb36c454ef092df27199b92ad09c374231
> ---
> Testing Hints:
>         Perform various Rx tests to verify we are not corrupting any Rx data.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 24 ++++++++++++------------
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 24 ++++++++++++------------
>  2 files changed, 24 insertions(+), 24 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S63 2/6] i40e/i40evf: Pull code for grabbing and syncing rx_buffer from fetch_buffer

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Tuesday, March 14, 2017 10:15 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S63 2/6] i40e/i40evf: Pull code for
> grabbing and syncing rx_buffer from fetch_buffer
> 
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This patch pulls the code responsible for fetching the Rx buffer and
> synchronizing DMA into a function, specifically called i40e_get_rx_buffer.
> 
> The general idea is to allow for better code reuse by pulling this out of
> i40e_fetch_rx_buffer.  We dropped a couple of prefetches since the time
> between the prefetch being called and the data being accessed was too
> small to be useful.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Change-ID: I4885fce4b2637dbedc8e16431169d23d3d7e79b9
> ---
> Testing Hints:
>         Basic Rx testing should be enough to verify this is working
>         correctly.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 58 ++++++++++++++++-------
> ----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 58 ++++++++++++++++------
> -----
>  2 files changed, 68 insertions(+), 48 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx private flag to allow fallback to old Rx flow

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Friday, March 17, 2017 8:58 AM
> To: Pujari, Bimmy <bimmy.pujari@intel.com>
> Cc: intel-wired-lan <intel-wired-lan@lists.osuosl.org>
> Subject: Re: [Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx
> private flag to allow fallback to old Rx flow
> 
> On Tue, Mar 14, 2017 at 10:15 AM, Bimmy Pujari <bimmy.pujari@intel.com>
> wrote:
> > From: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > This patch adds a control which will allow us to toggle into and out
> > of the legacy Rx mode.  The legacy Rx mode is what we currently do
> > when performing Rx.  As I make further changes what should happen is
> > that the driver will fall back to the behavior for Rx as of this patch should
> the "legacy-rx"
> > flag be set to on.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > Change-ID: I0342998849bbb31351cce05f6e182c99174e7751
> > ---
> > Testing Hints:
> >         This flag should not appear on kernels prior to 4.10.
> >         On kernels 4.10 and later the "legacy-rx" flag should appear
> >         and can be toggled between on and off. As we add more features the
> >         behavior of the driver should change when this value is toggled.
> >
> >         Toggling this for now will only cause a reset of the interface.
> >
> >         For the VF we are specifically replacing the define that was
> >         stripping the code for the flag needed for the "legacy-rx" flag
> >         since it doesn't make sense to have the interface defined when
> >         there are no flags present.  In the case of a kernel build the
> >         #ifdefs should be stripped and the code will remain.
> >
> >  drivers/net/ethernet/intel/i40e/i40e.h             |   1 +
> >  drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |   5 +-
> >  drivers/net/ethernet/intel/i40evf/i40evf.h         |   2 +
> >  drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 104
> > +++++++++++++++++++++
> >  4 files changed, 111 insertions(+), 1 deletion(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S63 6/6] i40e/i40evf: Change the way we limit the maximum frame size for Rx

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Tuesday, March 14, 2017 10:15 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S63 6/6] i40e/i40evf: Change the way
> we limit the maximum frame size for Rx
> 
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This patch changes the way we handle the maximum frame size for the Rx
> path.  Previously we were rounding up to 2K for a 1500 MTU and then brining
> the max frame size down to MTU plus a fixed amount.  With this patch
> applied what we now do is limit the maximum frame to 1.5K minus the value
> for NET_IP_ALIGN for standard MTU, and for any MTU greater than 1500 we
> allow up to the maximum frame size.  This makes the behavior more
> consistent with the other drivers such as igb which had similar logic.  In
> addition it reduces the test matrix for MTU since we only have two max
> frame sizes that are handled for Rx now.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Change-ID: I23a9d3c857e7df04b0ef28c64df63e659c013f3f
> ---
> Testing Hints:
>         Verify MTU handling for MTU = 1500.  You should be able to receive
>         frames up to 1536 in size on a standard x86 system as NET_IP_ALIGN
>         is 0.
> 
>         When jumbo frames are enabled you should be able to receive anything
>         up to the maximum size supported by hardware which is 9.5K or 9726B
>         in size.
> 
>         When the "legacy-rx" private flag is set you should be able to
>         receive anything up to 9.5K in size.
> 
>         The behavior for MTU mismatches will change.  A failing case is any
>         in which we trigger a kernel panic.  Receiving a frame larger than
>         what was configured for the MTU is not a failure.
> 
>  drivers/net/ethernet/intel/i40e/i40e_main.c        | 26 ++++++++++++---------
> -
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h        |  4 +---
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.h      |  4 +---
>  drivers/net/ethernet/intel/i40evf/i40evf.h         |  4 ----
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 16 ++++++++++++-
>  .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 14 ++++++++----
>  6 files changed, 41 insertions(+), 27 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S63 3/6] i40e/i40evf: Pull out code for cleaning up Rx buffers

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Tuesday, March 14, 2017 10:15 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S63 3/6] i40e/i40evf: Pull out code for
> cleaning up Rx buffers
> 
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This patch pulls out the code responsible for handling buffer recycling and
> page counting and distributes it through several functions.  This allows us to
> commonize the bits that handle either freeing or recycling the buffers.
> 
> As far as the page count tracking one change to the logic is that pagecnt_bias
> is decremented as soon as we call i40e_get_rx_buffer.  It is then the
> responsibility of the function that pulls the data to either increment the
> pagecnt_bias if the buffer can be recycled as-is, or to update page_offset so
> that we are pointing at the correct location for placement of the next buffer.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Change-ID: Ibac576360cb7f0b1627f2a993d13c1a8a2bf60af
> ---
> Testing Hints:
>         The greatest risk with this patch is a memory leak of some sort.
>         I already caught one spot where I hadn't fully thought things out
>         in regards to the path where we don't support bulk page updates.
>         My advice would be to test on a RHEL 6.X kernel as well as a RHEL
>         7.X kernel as the 6.X won't support bulk page count updates while
>         the 7.3 and later kernels do.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 73 +++++++++++++++++-----
> -----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 72 ++++++++++++++++------
> ----
>  2 files changed, 89 insertions(+), 56 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S63 4/6] i40e/i40evf: Break i40e_fetch_rx_buffer up to allow for reuse of frag code

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Bimmy Pujari
> Sent: Tuesday, March 14, 2017 10:15 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S63 4/6] i40e/i40evf: Break
> i40e_fetch_rx_buffer up to allow for reuse of frag code
> 
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This patch is meant to clean up the code in preparation for us adding support
> for build_skb.  Specifically we deconstruct i40e_fetch_buffer into several
> functions so that those functions can later be reused when we add a path for
> build_skb.
> 
> Specifically with this change we split out the code for adding a page to an
> exiting skb.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Change-ID: Iab1efbab6b8b97cb60ab9fdd0be1d37a056a154d
> ---
> Testing Hints:
>         The greatest risk with this patch is a memory leak of some sort.
>         I already caught one spot where I hadn't fully thought things out
>         in regards to the path where we don't support bulk page updates.
>         My advice would be to test on a RHEL 6.X kernel as well as a RHEL
>         7.X kernel as the 6.X won't support bulk page count updates while
>         the 7.3 and later kernels do.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 138 ++++++++++++------------
> --
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 138 ++++++++++++-----------
> ---
>  2 files changed, 130 insertions(+), 146 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx private flag to allow fallback to old Rx flow

[Alexander Duyck]

On Fri, Mar 17, 2017 at 8:57 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Tue, Mar 14, 2017 at 10:15 AM, Bimmy Pujari <bimmy.pujari@intel.com> wrote:
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> This patch adds a control which will allow us to toggle into and out of the
>> legacy Rx mode.  The legacy Rx mode is what we currently do when performing
>> Rx.  As I make further changes what should happen is that the driver will
>> fall back to the behavior for Rx as of this patch should the "legacy-rx"
>> flag be set to on.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> Change-ID: I0342998849bbb31351cce05f6e182c99174e7751
>> ---
>> Testing Hints:
>>         This flag should not appear on kernels prior to 4.10.
>>         On kernels 4.10 and later the "legacy-rx" flag should appear
>>         and can be toggled between on and off. As we add more features the
>>         behavior of the driver should change when this value is toggled.
>>
>>         Toggling this for now will only cause a reset of the interface.
>>
>>         For the VF we are specifically replacing the define that was
>>         stripping the code for the flag needed for the "legacy-rx" flag
>>         since it doesn't make sense to have the interface defined when
>>         there are no flags present.  In the case of a kernel build the
>>         #ifdefs should be stripped and the code will remain.
>>
>>  drivers/net/ethernet/intel/i40e/i40e.h             |   1 +
>>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |   5 +-
>>  drivers/net/ethernet/intel/i40evf/i40evf.h         |   2 +
>>  drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 104 +++++++++++++++++++++
>>  4 files changed, 111 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index dcba258..561fe1f 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -430,6 +430,7 @@ struct i40e_pf {
>>  #define I40E_FLAG_TEMP_LINK_POLLING            BIT_ULL(55)
>>  #define I40E_FLAG_CLIENT_L2_CHANGE             BIT_ULL(56)
>>  #define I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE                BIT_ULL(57)
>> +#define I40E_FLAG_LEGACY_RX                    BIT_ULL(57)
>
> It looks like we lost sync between upstream and out-of-tree at some
> point.  We are defining both of these using the same bit.  We need to
> use a different value for the LEGACY_RX bit in the upstream it looks
> like.

Any chance we can get a fix for this?  It has been almost a week since
I pointed out that we have a duplicate bit here, and I haven't seen
any updates.

We either need to submit a new version of this patch with us setting
I40E_FLAG_LEGACY_RX as bit 58, or if we can just get the tweak made to
the patch while it is in the tree I would be good with that as well.
We have some other patches that are going to be adding some more flags
in the pipeline and I would rather not have to push a fix for this and
instead have it fixed in the patch.

- Alex

[Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx private flag to allow fallback to old Rx flow

[Jeff Kirsher]

On Thu, 2017-03-23 at 21:23 -0700, Alexander Duyck wrote:
> On Fri, Mar 17, 2017 at 8:57 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > On Tue, Mar 14, 2017 at 10:15 AM, Bimmy Pujari <bimmy.pujari@intel.
> > com> wrote:
> > > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > > 
> > > This patch adds a control which will allow us to toggle into and
> > > out of the
> > > legacy Rx mode.? The legacy Rx mode is what we currently do when
> > > performing
> > > Rx.? As I make further changes what should happen is that the
> > > driver will
> > > fall back to the behavior for Rx as of this patch should the
> > > "legacy-rx"
> > > flag be set to on.
> > > 
> > > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > Change-ID: I0342998849bbb31351cce05f6e182c99174e7751
> > > ---
> > > Testing Hints:
> > > ???????? This flag should not appear on kernels prior to 4.10.
> > > ???????? On kernels 4.10 and later the "legacy-rx" flag should
> > > appear
> > > ???????? and can be toggled between on and off. As we add more
> > > features the
> > > ???????? behavior of the driver should change when this value is
> > > toggled.
> > > 
> > > ???????? Toggling this for now will only cause a reset of the
> > > interface.
> > > 
> > > ???????? For the VF we are specifically replacing the define that
> > > was
> > > ???????? stripping the code for the flag needed for the "legacy-
> > > rx" flag
> > > ???????? since it doesn't make sense to have the interface
> > > defined when
> > > ???????? there are no flags present.? In the case of a kernel
> > > build the
> > > ???????? #ifdefs should be stripped and the code will remain.
> > > 
> > > ? drivers/net/ethernet/intel/i40e/i40e.h???????????? |?? 1 +
> > > ? drivers/net/ethernet/intel/i40e/i40e_ethtool.c???? |?? 5 +-
> > > ? drivers/net/ethernet/intel/i40evf/i40evf.h???????? |?? 2 +
> > > ? drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 104
> > > +++++++++++++++++++++
> > > ? 4 files changed, 111 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> > > b/drivers/net/ethernet/intel/i40e/i40e.h
> > > index dcba258..561fe1f 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e.h
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> > > @@ -430,6 +430,7 @@ struct i40e_pf {
> > > ? #define I40E_FLAG_TEMP_LINK_POLLING??????????? BIT_ULL(55)
> > > ? #define I40E_FLAG_CLIENT_L2_CHANGE???????????? BIT_ULL(56)
> > > ? #define I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE???????????????
> > > BIT_ULL(57)
> > > +#define I40E_FLAG_LEGACY_RX??????????????????? BIT_ULL(57)
> > 
> > It looks like we lost sync between upstream and out-of-tree at some
> > point.? We are defining both of these using the same bit.? We need
> > to
> > use a different value for the LEGACY_RX bit in the upstream it
> > looks
> > like.
> 
> Any chance we can get a fix for this?? It has been almost a week
> since
> I pointed out that we have a duplicate bit here, and I haven't seen
> any updates.
> 
> We either need to submit a new version of this patch with us setting
> I40E_FLAG_LEGACY_RX as bit 58, or if we can just get the tweak made
> to
> the patch while it is in the tree I would be good with that as well.
> We have some other patches that are going to be adding some more
> flags
> in the pipeline and I would rather not have to push a fix for this
> and
> instead have it fixed in the patch.

I will fix this inline, so no need to resubmit with the desired fix.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20170323/2e958d58/attachment.asc>

[Intel-wired-lan] [next PATCH S63 5/6] i40e/i40evf: Add legacy-rx private flag to allow fallback to old Rx flow

[Jeff Kirsher]

On Thu, 2017-03-23 at 22:17 -0700, Jeff Kirsher wrote:
> On Thu, 2017-03-23 at 21:23 -0700, Alexander Duyck wrote:
> > On Fri, Mar 17, 2017 at 8:57 AM, Alexander Duyck
> > <alexander.duyck@gmail.com> wrote:
> > > On Tue, Mar 14, 2017 at 10:15 AM, Bimmy Pujari <bimmy.pujari@inte
> l.
> > > com> wrote:
> > > > From: Alexander Duyck <alexander.h.duyck@intel.com>
> > > >?
> > > > This patch adds a control which will allow us to toggle into
> and
> > > > out of the
> > > > legacy Rx mode.? The legacy Rx mode is what we currently do
> when
> > > > performing
> > > > Rx.? As I make further changes what should happen is that the
> > > > driver will
> > > > fall back to the behavior for Rx as of this patch should the
> > > > "legacy-rx"
> > > > flag be set to on.
> > > >?
> > > > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > > Change-ID: I0342998849bbb31351cce05f6e182c99174e7751
> > > > ---
> > > > Testing Hints:
> > > > ???????? This flag should not appear on kernels prior to 4.10.
> > > > ???????? On kernels 4.10 and later the "legacy-rx" flag should
> > > > appear
> > > > ???????? and can be toggled between on and off. As we add more
> > > > features the
> > > > ???????? behavior of the driver should change when this value
> is
> > > > toggled.
> > > >?
> > > > ???????? Toggling this for now will only cause a reset of the
> > > > interface.
> > > >?
> > > > ???????? For the VF we are specifically replacing the define
> that
> > > > was
> > > > ???????? stripping the code for the flag needed for the
> "legacy-
> > > > rx" flag
> > > > ???????? since it doesn't make sense to have the interface
> > > > defined when
> > > > ???????? there are no flags present.? In the case of a kernel
> > > > build the
> > > > ???????? #ifdefs should be stripped and the code will remain.
> > > >?
> > > > ? drivers/net/ethernet/intel/i40e/i40e.h???????????? |?? 1 +
> > > > ? drivers/net/ethernet/intel/i40e/i40e_ethtool.c???? |?? 5 +-
> > > > ? drivers/net/ethernet/intel/i40evf/i40evf.h???????? |?? 2 +
> > > > ? drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 104
> > > > +++++++++++++++++++++
> > > > ? 4 files changed, 111 insertions(+), 1 deletion(-)
> > > >?
> > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> > > > b/drivers/net/ethernet/intel/i40e/i40e.h
> > > > index dcba258..561fe1f 100644
> > > > --- a/drivers/net/ethernet/intel/i40e/i40e.h
> > > > +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> > > > @@ -430,6 +430,7 @@ struct i40e_pf {
> > > > ? #define I40E_FLAG_TEMP_LINK_POLLING??????????? BIT_ULL(55)
> > > > ? #define I40E_FLAG_CLIENT_L2_CHANGE???????????? BIT_ULL(56)
> > > > ? #define I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE???????????????
> > > > BIT_ULL(57)
> > > > +#define I40E_FLAG_LEGACY_RX??????????????????? BIT_ULL(57)
> > >?
> > > It looks like we lost sync between upstream and out-of-tree at
> some
> > > point.? We are defining both of these using the same bit.? We
> need
> > > to
> > > use a different value for the LEGACY_RX bit in the upstream it
> > > looks
> > > like.
> >?
> > Any chance we can get a fix for this?? It has been almost a week
> > since
> > I pointed out that we have a duplicate bit here, and I haven't seen
> > any updates.
> >?
> > We either need to submit a new version of this patch with us
> setting
> > I40E_FLAG_LEGACY_RX as bit 58, or if we can just get the tweak made
> > to
> > the patch while it is in the tree I would be good with that as
> well.
> > We have some other patches that are going to be adding some more
> > flags
> > in the pipeline and I would rather not have to push a fix for this
> > and
> > instead have it fixed in the patch.
> 
> I will fix this inline, so no need to resubmit with the desired fix.

Ok I have made the change, but in the process of making this fix, I see
there is a fairly significant disconnect between upstream and the out-
of-tree driver when it looking at these flag values.

I will see if I can come up with either a internal patch or kernel
patch to at least clean up these flag value out of sync issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20170323/3ea061b9/attachment.asc>