Netdev List
 help / color / mirror / Atom feed
* [net-next v2 09/15] ixgbevf: add netpoll support
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This patch adds ixgbevf_netpoll() a callback for .ndo_poll_controller to
allow for the VF interface to be used with netconsole.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  2 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 29 ++++++++++++++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 6660498..bb6726c 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -379,7 +379,7 @@ struct ixgbevf_adapter {
 	 */
 	u32 flags;
 #define IXGBE_FLAG_IN_WATCHDOG_TASK             (u32)(1)
-#define IXGBE_FLAG_IN_NETPOLL                   (u32)(1 << 1)
+
 #define IXGBEVF_FLAG_QUEUE_RESET_REQUESTED	(u32)(1 << 2)
 
 	struct msix_entry *msix_entries;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 70a2b8f..755f71f 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -344,10 +344,8 @@ static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
 		return;
 	}
 #endif /* CONFIG_NET_RX_BUSY_POLL */
-	if (!(q_vector->adapter->flags & IXGBE_FLAG_IN_NETPOLL))
-		napi_gro_receive(&q_vector->napi, skb);
-	else
-		netif_rx(skb);
+
+	napi_gro_receive(&q_vector->napi, skb);
 }
 
 /* ixgbevf_rx_checksum - indicate in skb if hw indicated a good cksum
@@ -919,12 +917,10 @@ static int ixgbevf_poll(struct napi_struct *napi, int budget)
 	else
 		per_ring_budget = budget;
 
-	adapter->flags |= IXGBE_FLAG_IN_NETPOLL;
 	ixgbevf_for_each_ring(ring, q_vector->rx)
 		clean_complete &= (ixgbevf_clean_rx_irq(q_vector, ring,
 							per_ring_budget)
 				   < per_ring_budget);
-	adapter->flags &= ~IXGBE_FLAG_IN_NETPOLL;
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	ixgbevf_qv_unlock_napi(q_vector);
@@ -3553,6 +3549,24 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
 	return 0;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/* Polling 'interrupt' - used by things like netconsole to send skbs
+ * without having to re-enable interrupts. It's not called while
+ * the interrupt routine is executing.
+ */
+static void ixgbevf_netpoll(struct net_device *netdev)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+	int i;
+
+	/* if interface is down do nothing */
+	if (test_bit(__IXGBEVF_DOWN, &adapter->state))
+		return;
+	for (i = 0; i < adapter->num_rx_queues; i++)
+		ixgbevf_msix_clean_rings(0, adapter->q_vector[i]);
+}
+#endif /* CONFIG_NET_POLL_CONTROLLER */
+
 static int ixgbevf_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -3689,6 +3703,9 @@ static const struct net_device_ops ixgbevf_netdev_ops = {
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	.ndo_busy_poll		= ixgbevf_busy_poll_recv,
 #endif
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller	= ixgbevf_netpoll,
+#endif
 };
 
 static void ixgbevf_assign_netdev_ops(struct net_device *dev)
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 11/15] i40evf: update header comments
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem; +Cc: Mitch Williams, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mitch Williams <mitch.a.williams@intel.com>

No code changes. Update comments to match actual function declarations.

Change-ID: Ib830d2f154ee917a104955c0914267fc98f3d2c8
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
index 876411c..b2cd9ea 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
@@ -621,7 +621,7 @@ static u32 i40evf_get_rxfh_indir_size(struct net_device *netdev)
  * i40evf_get_rxfh - get the rx flow hash indirection table
  * @netdev: network interface device structure
  * @indir: indirection table
- * @key: hash key (will be %NULL until get_rxfh_key_size is implemented)
+ * @key: hash key
  *
  * Reads the indirection table directly from the hardware. Always returns 0.
  **/
@@ -646,7 +646,7 @@ static int i40evf_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key)
  * i40evf_set_rxfh - set the rx flow hash indirection table
  * @netdev: network interface device structure
  * @indir: indirection table
- * @key: hash key (will be %NULL until get_rxfh_key_size is implemented)
+ * @key: hash key
  *
  * Returns -EINVAL if the table specifies an inavlid queue id, otherwise
  * returns 0 after programming the table.
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 10/15] i40e: don't overload fields
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem; +Cc: Mitch Williams, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mitch Williams <mitch.a.williams@intel.com>

Overloading the msg_size field in the arq_event_info struct is just a
bad idea. It leads to repeated bugs when the structure is used in a
loop, since the input value (buffer size) is overwritten by the output
value (actual message length).

Fix this by splitting the field into two and renaming to indicate the
actual function of each field.

Since the arq_event struct has now changed, we need to change the drivers
to support this. Note that we no longer need to initialize the buffer size
each time we go through a loop as this value is no longer destroyed by
arq processing.

In the process, we also fix a bug in i40evf_verify_api_ver where the
buffer size was not correctly reinitialized each time through the loop.

Change-ID: Ic7f9633cdd6f871f93e698dfb095e29c696f5581
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Acked-by: Shannon Nelson <shannon.nelson@intel.com>
Acked-by: Ashish Shah <ashish.n.shah@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c       |  6 +++---
 drivers/net/ethernet/intel/i40e/i40e_adminq.h       |  3 ++-
 drivers/net/ethernet/intel/i40e/i40e_main.c         |  7 +++----
 drivers/net/ethernet/intel/i40evf/i40e_adminq.c     |  6 +++---
 drivers/net/ethernet/intel/i40evf/i40e_adminq.h     |  3 ++-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c     |  7 +++----
 drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c | 11 +++++------
 7 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index f7f6206..5bb4914 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -980,10 +980,10 @@ i40e_status i40e_clean_arq_element(struct i40e_hw *hw,
 
 	e->desc = *desc;
 	datalen = le16_to_cpu(desc->datalen);
-	e->msg_size = min(datalen, e->msg_size);
-	if (e->msg_buf != NULL && (e->msg_size != 0))
+	e->msg_len = min(datalen, e->buf_len);
+	if (e->msg_buf != NULL && (e->msg_len != 0))
 		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
-		       e->msg_size);
+		       e->msg_len);
 
 	i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: desc and buffer:\n");
 	i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, e->msg_buf,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
index df0bd09..003a227 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
@@ -76,7 +76,8 @@ struct i40e_asq_cmd_details {
 /* ARQ event information */
 struct i40e_arq_event_info {
 	struct i40e_aq_desc desc;
-	u16 msg_size;
+	u16 msg_len;
+	u16 buf_len;
 	u8 *msg_buf;
 };
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c998d82..3ebab03 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5750,13 +5750,12 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
 	if (oldval != val)
 		wr32(&pf->hw, pf->hw.aq.asq.len, val);
 
-	event.msg_size = I40E_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = I40E_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf)
 		return;
 
 	do {
-		event.msg_size = I40E_MAX_AQ_BUF_SIZE; /* reinit each time */
 		ret = i40e_clean_arq_element(hw, &event, &pending);
 		if (ret == I40E_ERR_ADMIN_QUEUE_NO_WORK)
 			break;
@@ -5777,7 +5776,7 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
 					le32_to_cpu(event.desc.cookie_high),
 					le32_to_cpu(event.desc.cookie_low),
 					event.msg_buf,
-					event.msg_size);
+					event.msg_len);
 			break;
 		case i40e_aqc_opc_lldp_update_mib:
 			dev_dbg(&pf->pdev->dev, "ARQ: Update LLDP MIB event received\n");
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
index 500ca21..d7e446f 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
@@ -929,10 +929,10 @@ i40e_status i40evf_clean_arq_element(struct i40e_hw *hw,
 
 	e->desc = *desc;
 	datalen = le16_to_cpu(desc->datalen);
-	e->msg_size = min(datalen, e->msg_size);
-	if (e->msg_buf != NULL && (e->msg_size != 0))
+	e->msg_len = min(datalen, e->buf_len);
+	if (e->msg_buf != NULL && (e->msg_len != 0))
 		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
-		       e->msg_size);
+		       e->msg_len);
 
 	if (i40e_is_nvm_update_op(&e->desc))
 		hw->aq.nvm_busy = false;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
index f40cfac..0d58378 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
@@ -76,7 +76,8 @@ struct i40e_asq_cmd_details {
 /* ARQ event information */
 struct i40e_arq_event_info {
 	struct i40e_aq_desc desc;
-	u16 msg_size;
+	u16 msg_len;
+	u16 buf_len;
 	u8 *msg_buf;
 };
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 4892278..d378e13 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -1632,8 +1632,8 @@ static void i40evf_adminq_task(struct work_struct *work)
 	if (adapter->flags & I40EVF_FLAG_PF_COMMS_FAILED)
 		return;
 
-	event.msg_size = I40EVF_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = I40EVF_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf)
 		return;
 
@@ -1645,10 +1645,9 @@ static void i40evf_adminq_task(struct work_struct *work)
 
 		i40evf_virtchnl_completion(adapter, v_msg->v_opcode,
 					   v_msg->v_retval, event.msg_buf,
-					   event.msg_size);
+					   event.msg_len);
 		if (pending != 0) {
 			memset(event.msg_buf, 0, I40EVF_MAX_AQ_BUF_SIZE);
-			event.msg_size = I40EVF_MAX_AQ_BUF_SIZE;
 		}
 	} while (pending);
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 49bfdb5..422dd2d 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -92,8 +92,8 @@ int i40evf_verify_api_ver(struct i40evf_adapter *adapter)
 	enum i40e_virtchnl_ops op;
 	i40e_status err;
 
-	event.msg_size = I40EVF_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = I40EVF_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf) {
 		err = -ENOMEM;
 		goto out;
@@ -169,15 +169,14 @@ int i40evf_get_vf_config(struct i40evf_adapter *adapter)
 
 	len =  sizeof(struct i40e_virtchnl_vf_resource) +
 		I40E_MAX_VF_VSI * sizeof(struct i40e_virtchnl_vsi_resource);
-	event.msg_size = len;
-	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
+	event.buf_len = len;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
 	if (!event.msg_buf) {
 		err = -ENOMEM;
 		goto out;
 	}
 
 	while (1) {
-		event.msg_size = len;
 		/* When the AQ is empty, i40evf_clean_arq_element will return
 		 * nonzero and this loop will terminate.
 		 */
@@ -191,7 +190,7 @@ int i40evf_get_vf_config(struct i40evf_adapter *adapter)
 	}
 
 	err = (i40e_status)le32_to_cpu(event.desc.cookie_low);
-	memcpy(adapter->vf_res, event.msg_buf, min(event.msg_size, len));
+	memcpy(adapter->vf_res, event.msg_buf, min(event.msg_len, len));
 
 	i40e_vf_parse_hw_config(hw, adapter->vf_res);
 out_alloc:
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 05/15] ixgbevf: reorder main loop in ixgbe_clean_rx_irq to allow for do/while/continue
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This change allows us to go from a loop based on the descriptor to one
primarily based on the budget. The advantage to this is that we can avoid
carrying too many values from one iteration to the next.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 64 ++++++++++++-----------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index f864da9..2206992 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -517,35 +517,48 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 				struct ixgbevf_ring *rx_ring,
 				int budget)
 {
-	union ixgbe_adv_rx_desc *rx_desc;
 	unsigned int i;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
 	u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
 
 	i = rx_ring->next_to_clean;
-	rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
 
-	while (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_DD)) {
-		union ixgbe_adv_rx_desc *next_rxd;
-		struct ixgbevf_rx_buffer *rx_buffer_info;
+	do {
+		union ixgbe_adv_rx_desc *rx_desc, *next_rxd;
+		struct ixgbevf_rx_buffer *rx_buffer;
 		struct sk_buff *skb;
 
-		if (!budget)
+		/* return some buffers to hardware, one at a time is too slow */
+		if (cleaned_count >= IXGBEVF_RX_BUFFER_WRITE) {
+			ixgbevf_alloc_rx_buffers(rx_ring, cleaned_count);
+			cleaned_count = 0;
+		}
+
+		rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
+		rx_buffer = &rx_ring->rx_buffer_info[i];
+
+		if (!ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_DD))
 			break;
-		budget--;
 
-		rmb(); /* read descriptor and rx_buffer_info after status DD */
+		/* This memory barrier is needed to keep us from reading
+		 * any other fields out of the rx_desc until we know the
+		 * RXD_STAT_DD bit is set
+		 */
+		rmb();
 
-		rx_buffer_info = &rx_ring->rx_buffer_info[i];
-		skb = rx_buffer_info->skb;
+		skb = rx_buffer->skb;
 		prefetch(skb->data);
-		rx_buffer_info->skb = NULL;
 
-		dma_unmap_single(rx_ring->dev, rx_buffer_info->dma,
+		/* pull the header of the skb in */
+		__skb_put(skb, le16_to_cpu(rx_desc->wb.upper.length));
+
+		dma_unmap_single(rx_ring->dev, rx_buffer->dma,
 				 rx_ring->rx_buf_len,
 				 DMA_FROM_DEVICE);
-		rx_buffer_info->dma = 0;
-		skb_put(skb, le16_to_cpu(rx_desc->wb.upper.length));
+
+		/* clear skb reference in buffer info structure */
+		rx_buffer->skb = NULL;
+		rx_buffer->dma = 0;
 
 		cleaned_count++;
 
@@ -560,7 +573,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 			skb->next = rx_ring->rx_buffer_info[i].skb;
 			IXGBE_CB(skb->next)->prev = skb;
 			rx_ring->rx_stats.non_eop_descs++;
-			goto next_desc;
+			continue;
 		}
 
 		/* we should not be chaining buffers, if we did drop the skb */
@@ -570,14 +583,14 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 				skb = IXGBE_CB(skb)->prev;
 				dev_kfree_skb(this);
 			} while (skb);
-			goto next_desc;
+			continue;
 		}
 
 		/* ERR_MASK will only have valid bits if EOP set */
 		if (unlikely(ixgbevf_test_staterr(rx_desc,
 					    IXGBE_RXDADV_ERR_FRAME_ERR_MASK))) {
 			dev_kfree_skb_irq(skb);
-			goto next_desc;
+			continue;
 		}
 
 		/* probably a little skewed due to removing CRC */
@@ -592,7 +605,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		    ether_addr_equal(rx_ring->netdev->dev_addr,
 				     eth_hdr(skb)->h_source)) {
 			dev_kfree_skb_irq(skb);
-			goto next_desc;
+			continue;
 		}
 
 		/* populate checksum, VLAN, and protocol */
@@ -600,18 +613,9 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 
 		ixgbevf_rx_skb(q_vector, skb);
 
-next_desc:
-		/* return some buffers to hardware, one at a time is too slow */
-		if (cleaned_count >= IXGBEVF_RX_BUFFER_WRITE) {
-			ixgbevf_alloc_rx_buffers(rx_ring, cleaned_count);
-			cleaned_count = 0;
-		}
-
-		/* use prefetched values */
-		rx_desc = next_rxd;
-		rx_buffer_info = &rx_ring->rx_buffer_info[i];
-		rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
-	}
+		/* update budget accounting */
+		budget--;
+	} while (likely(budget));
 
 	rx_ring->next_to_clean = i;
 	u64_stats_update_begin(&rx_ring->syncp);
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 06/15] ixgbevf: Update Rx next to clean in real time
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

Since the next_to_clean value is only accessed by the Rx interrupt handler
we can save on stack space by just storing our updated values back in
next_to_clean instead of using the stack variable i.  This should help to
reduce stack space and we can further collapse the size of the function.

Also removed  non_eop_descs counter as it was never shown in the stats.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  2 -
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 55 +++++++++++++++--------
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 90d5751..72a354b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -79,7 +79,6 @@ struct ixgbevf_tx_queue_stats {
 };
 
 struct ixgbevf_rx_queue_stats {
-	u64 non_eop_descs;
 	u64 alloc_rx_page_failed;
 	u64 alloc_rx_buff_failed;
 	u64 csum_err;
@@ -372,7 +371,6 @@ struct ixgbevf_adapter {
 	struct ixgbevf_ring *rx_ring[MAX_TX_QUEUES]; /* One per active queue */
 	u64 hw_csum_rx_error;
 	u64 hw_rx_no_dma_resources;
-	u64 non_eop_descs;
 	int num_msix_vectors;
 	u32 alloc_rx_page_failed;
 	u32 alloc_rx_buff_failed;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 2206992..20bebd2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -410,6 +410,35 @@ static void ixgbevf_process_skb_fields(struct ixgbevf_ring *rx_ring,
 	skb->protocol = eth_type_trans(skb, rx_ring->netdev);
 }
 
+/**
+ * ixgbevf_is_non_eop - process handling of non-EOP buffers
+ * @rx_ring: Rx ring being processed
+ * @rx_desc: Rx descriptor for current buffer
+ * @skb: current socket buffer containing buffer in progress
+ *
+ * This function updates next to clean.  If the buffer is an EOP buffer
+ * this function exits returning false, otherwise it will place the
+ * sk_buff in the next buffer to be chained and return true indicating
+ * that this is in fact a non-EOP buffer.
+ **/
+static bool ixgbevf_is_non_eop(struct ixgbevf_ring *rx_ring,
+			       union ixgbe_adv_rx_desc *rx_desc,
+			       struct sk_buff *skb)
+{
+	u32 ntc = rx_ring->next_to_clean + 1;
+
+	/* fetch, update, and store next to clean */
+	ntc = (ntc < rx_ring->count) ? ntc : 0;
+	rx_ring->next_to_clean = ntc;
+
+	prefetch(IXGBEVF_RX_DESC(rx_ring, ntc));
+
+	if (likely(ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP)))
+		return false;
+
+	return true;
+}
+
 static bool ixgbevf_alloc_mapped_skb(struct ixgbevf_ring *rx_ring,
 				     struct ixgbevf_rx_buffer *bi)
 {
@@ -517,16 +546,14 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 				struct ixgbevf_ring *rx_ring,
 				int budget)
 {
-	unsigned int i;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
 	u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
 
-	i = rx_ring->next_to_clean;
-
 	do {
-		union ixgbe_adv_rx_desc *rx_desc, *next_rxd;
+		union ixgbe_adv_rx_desc *rx_desc;
 		struct ixgbevf_rx_buffer *rx_buffer;
 		struct sk_buff *skb;
+		u16 ntc;
 
 		/* return some buffers to hardware, one at a time is too slow */
 		if (cleaned_count >= IXGBEVF_RX_BUFFER_WRITE) {
@@ -534,8 +561,9 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 			cleaned_count = 0;
 		}
 
-		rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
-		rx_buffer = &rx_ring->rx_buffer_info[i];
+		ntc = rx_ring->next_to_clean;
+		rx_desc = IXGBEVF_RX_DESC(rx_ring, ntc);
+		rx_buffer = &rx_ring->rx_buffer_info[ntc];
 
 		if (!ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_DD))
 			break;
@@ -562,19 +590,9 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 
 		cleaned_count++;
 
-		i++;
-		if (i == rx_ring->count)
-			i = 0;
-
-		next_rxd = IXGBEVF_RX_DESC(rx_ring, i);
-		prefetch(next_rxd);
-
-		if (!(ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP))) {
-			skb->next = rx_ring->rx_buffer_info[i].skb;
-			IXGBE_CB(skb->next)->prev = skb;
-			rx_ring->rx_stats.non_eop_descs++;
+		/* place incomplete frames back on ring for completion */
+		if (ixgbevf_is_non_eop(rx_ring, rx_desc, skb))
 			continue;
-		}
 
 		/* we should not be chaining buffers, if we did drop the skb */
 		if (IXGBE_CB(skb)->prev) {
@@ -617,7 +635,6 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		budget--;
 	} while (likely(budget));
 
-	rx_ring->next_to_clean = i;
 	u64_stats_update_begin(&rx_ring->syncp);
 	rx_ring->stats.packets += total_rx_packets;
 	rx_ring->stats.bytes += total_rx_bytes;
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 08/15] ixgbevf: compare total_rx_packets and budget in ixgbevf_clean_rx_irq
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

total_rx_packets is the number of packets we had cleaned, and budget is
the total number of packets that we could clean per poll. Instead of
altering both of these values we can save ourselves one write to memory by
just comparing total_rx_packets to the budget and as long as we are less
than budget we continue cleaning.

Also change the do{}while logic to while{} in order to avoid processing
packets when budget is 0.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 07222b8..70a2b8f 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -809,7 +809,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 	u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
 	struct sk_buff *skb = rx_ring->skb;
 
-	do {
+	while (likely(total_rx_packets < budget)) {
 		union ixgbe_adv_rx_desc *rx_desc;
 
 		/* return some buffers to hardware, one at a time is too slow */
@@ -850,7 +850,6 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 
 		/* probably a little skewed due to removing CRC */
 		total_rx_bytes += skb->len;
-		total_rx_packets++;
 
 		/* Workaround hardware that can't do proper VEPA multicast
 		 * source pruning.
@@ -872,8 +871,8 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		skb = NULL;
 
 		/* update budget accounting */
-		budget--;
-	} while (likely(budget));
+		total_rx_packets++;
+	}
 
 	/* place incomplete frames back on ring for completion */
 	rx_ring->skb = skb;
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 04/15] ixgbevf: Cleanup variable usage, improve stack performance
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This change is meant to help cleanup the usage of temporary variables
within the Rx hot-path by removing unnecessary variables and reducing
the scope of variables that do not need to exist outside the main loop.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 36b005e..f864da9 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -517,26 +517,28 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 				struct ixgbevf_ring *rx_ring,
 				int budget)
 {
-	union ixgbe_adv_rx_desc *rx_desc, *next_rxd;
-	struct ixgbevf_rx_buffer *rx_buffer_info, *next_buffer;
-	struct sk_buff *skb;
+	union ixgbe_adv_rx_desc *rx_desc;
 	unsigned int i;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
 	u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
 
 	i = rx_ring->next_to_clean;
 	rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
-	rx_buffer_info = &rx_ring->rx_buffer_info[i];
 
 	while (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_DD)) {
+		union ixgbe_adv_rx_desc *next_rxd;
+		struct ixgbevf_rx_buffer *rx_buffer_info;
+		struct sk_buff *skb;
+
 		if (!budget)
 			break;
 		budget--;
 
 		rmb(); /* read descriptor and rx_buffer_info after status DD */
 
+		rx_buffer_info = &rx_ring->rx_buffer_info[i];
 		skb = rx_buffer_info->skb;
-		prefetch(skb->data - NET_IP_ALIGN);
+		prefetch(skb->data);
 		rx_buffer_info->skb = NULL;
 
 		dma_unmap_single(rx_ring->dev, rx_buffer_info->dma,
@@ -545,18 +547,17 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		rx_buffer_info->dma = 0;
 		skb_put(skb, le16_to_cpu(rx_desc->wb.upper.length));
 
+		cleaned_count++;
+
 		i++;
 		if (i == rx_ring->count)
 			i = 0;
 
 		next_rxd = IXGBEVF_RX_DESC(rx_ring, i);
 		prefetch(next_rxd);
-		cleaned_count++;
-
-		next_buffer = &rx_ring->rx_buffer_info[i];
 
 		if (!(ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP))) {
-			skb->next = next_buffer->skb;
+			skb->next = rx_ring->rx_buffer_info[i].skb;
 			IXGBE_CB(skb->next)->prev = skb;
 			rx_ring->rx_stats.non_eop_descs++;
 			goto next_desc;
@@ -609,6 +610,7 @@ next_desc:
 		/* use prefetched values */
 		rx_desc = next_rxd;
 		rx_buffer_info = &rx_ring->rx_buffer_info[i];
+		rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
 	}
 
 	rx_ring->next_to_clean = i;
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 03/15] ixgbevf: Combine the logic for post Rx processing into single function
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This patch cleans up ixgbevf_clean_rx_irq() by merging several similar
operations into a new function - ixgbevf_process_skb_fields().

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  4 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 68 ++++++++++++-----------
 2 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 5f7d2f3..90d5751 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -346,8 +346,10 @@ static inline void ixgbevf_write_tail(struct ixgbevf_ring *ring, u32 value)
 
 /* board specific private data structure */
 struct ixgbevf_adapter {
-	struct timer_list watchdog_timer;
+	/* this field must be first, see ixgbevf_process_skb_fields */
 	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
+
+	struct timer_list watchdog_timer;
 	struct work_struct reset_task;
 	struct ixgbevf_q_vector *q_vector[MAX_MSIX_Q_VECTORS];
 
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 19062dc..36b005e 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -328,37 +328,12 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
 }
 
 /**
- * ixgbevf_receive_skb - Send a completed packet up the stack
- * @q_vector: structure containing interrupt and ring information
- * @skb: packet to send up
- * @rx_desc: rx descriptor
- **/
-static void ixgbevf_receive_skb(struct ixgbevf_q_vector *q_vector,
-				struct sk_buff *skb,
-				union ixgbe_adv_rx_desc *rx_desc)
-{
-	struct ixgbevf_adapter *adapter = q_vector->adapter;
-	bool is_vlan = !!ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_VP);
-	u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
-
-	if (is_vlan && test_bit(tag & VLAN_VID_MASK, adapter->active_vlans))
-		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), tag);
-
-	if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL))
-		napi_gro_receive(&q_vector->napi, skb);
-	else
-		netif_rx(skb);
-}
-
-/**
  * ixgbevf_rx_skb - Helper function to determine proper Rx method
  * @q_vector: structure containing interrupt and ring information
  * @skb: packet to send up
- * @rx_desc: rx descriptor
  **/
 static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
-			   struct sk_buff *skb,
-			   union ixgbe_adv_rx_desc *rx_desc)
+			   struct sk_buff *skb)
 {
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	skb_mark_napi_id(skb, &q_vector->napi);
@@ -369,8 +344,10 @@ static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
 		return;
 	}
 #endif /* CONFIG_NET_RX_BUSY_POLL */
-
-	ixgbevf_receive_skb(q_vector, skb, rx_desc);
+	if (!(q_vector->adapter->flags & IXGBE_FLAG_IN_NETPOLL))
+		napi_gro_receive(&q_vector->napi, skb);
+	else
+		netif_rx(skb);
 }
 
 /* ixgbevf_rx_checksum - indicate in skb if hw indicated a good cksum
@@ -407,6 +384,32 @@ static inline void ixgbevf_rx_checksum(struct ixgbevf_ring *ring,
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 }
 
+/* ixgbevf_process_skb_fields - Populate skb header fields from Rx descriptor
+ * @rx_ring: rx descriptor ring packet is being transacted on
+ * @rx_desc: pointer to the EOP Rx descriptor
+ * @skb: pointer to current skb being populated
+ *
+ * This function checks the ring, descriptor, and packet information in
+ * order to populate the checksum, VLAN, protocol, and other fields within
+ * the skb.
+ */
+static void ixgbevf_process_skb_fields(struct ixgbevf_ring *rx_ring,
+				       union ixgbe_adv_rx_desc *rx_desc,
+				       struct sk_buff *skb)
+{
+	ixgbevf_rx_checksum(rx_ring, rx_desc, skb);
+
+	if (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_VP)) {
+		u16 vid = le16_to_cpu(rx_desc->wb.upper.vlan);
+		unsigned long *active_vlans = netdev_priv(rx_ring->netdev);
+
+		if (test_bit(vid & VLAN_VID_MASK, active_vlans))
+			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
+	}
+
+	skb->protocol = eth_type_trans(skb, rx_ring->netdev);
+}
+
 static bool ixgbevf_alloc_mapped_skb(struct ixgbevf_ring *rx_ring,
 				     struct ixgbevf_rx_buffer *bi)
 {
@@ -576,14 +579,10 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 			goto next_desc;
 		}
 
-		ixgbevf_rx_checksum(rx_ring, rx_desc, skb);
-
 		/* probably a little skewed due to removing CRC */
 		total_rx_bytes += skb->len;
 		total_rx_packets++;
 
-		skb->protocol = eth_type_trans(skb, rx_ring->netdev);
-
 		/* Workaround hardware that can't do proper VEPA multicast
 		 * source pruning.
 		 */
@@ -595,7 +594,10 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 			goto next_desc;
 		}
 
-		ixgbevf_rx_skb(q_vector, skb, rx_desc);
+		/* populate checksum, VLAN, and protocol */
+		ixgbevf_process_skb_fields(rx_ring, rx_desc, skb);
+
+		ixgbevf_rx_skb(q_vector, skb);
 
 next_desc:
 		/* return some buffers to hardware, one at a time is too slow */
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 01/15] ixgbevf: Update ixgbevf_alloc_rx_buffers to handle clearing of status bits
From: Jeff Kirsher @ 2014-11-20 23:08 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

Instead of clearing the status bits in the cleanup it makes more sense to
just clear the status bits on allocation.  This way we can leave the Rx
descriptor rings as a read only memory block until we actually have buffers
to give back to the hardware.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 132 +++++++++++++---------
 1 file changed, 80 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 030a219..deda74d 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -143,21 +143,6 @@ u32 ixgbevf_read_reg(struct ixgbe_hw *hw, u32 reg)
 	return value;
 }
 
-static inline void ixgbevf_release_rx_desc(struct ixgbevf_ring *rx_ring,
-					   u32 val)
-{
-	rx_ring->next_to_use = val;
-
-	/*
-	 * Force memory writes to complete before letting h/w
-	 * know there are new descriptors to fetch.  (Only
-	 * applicable for weak-ordered memory model archs,
-	 * such as IA-64).
-	 */
-	wmb();
-	ixgbevf_write_tail(rx_ring, val);
-}
-
 /**
  * ixgbevf_set_ivar - set IVAR registers - maps interrupt causes to vectors
  * @adapter: pointer to adapter struct
@@ -424,52 +409,99 @@ static inline void ixgbevf_rx_checksum(struct ixgbevf_ring *ring,
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 }
 
+static bool ixgbevf_alloc_mapped_skb(struct ixgbevf_ring *rx_ring,
+				     struct ixgbevf_rx_buffer *bi)
+{
+	struct sk_buff *skb = bi->skb;
+	dma_addr_t dma = bi->dma;
+
+	if (unlikely(skb))
+		return true;
+
+	skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
+					rx_ring->rx_buf_len);
+	if (unlikely(!skb)) {
+		rx_ring->rx_stats.alloc_rx_buff_failed++;
+		return false;
+	}
+
+	dma = dma_map_single(rx_ring->dev, skb->data,
+			     rx_ring->rx_buf_len, DMA_FROM_DEVICE);
+
+	/* if mapping failed free memory back to system since
+	 * there isn't much point in holding memory we can't use
+	 */
+	if (dma_mapping_error(rx_ring->dev, dma)) {
+		dev_kfree_skb_any(skb);
+
+		rx_ring->rx_stats.alloc_rx_buff_failed++;
+		return false;
+	}
+
+	bi->skb = skb;
+	bi->dma = dma;
+
+	return true;
+}
+
 /**
  * ixgbevf_alloc_rx_buffers - Replace used receive buffers; packet split
  * @rx_ring: rx descriptor ring (for a specific queue) to setup buffers on
+ * @cleaned_count: number of buffers to replace
  **/
 static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
-				     int cleaned_count)
+				     u16 cleaned_count)
 {
 	union ixgbe_adv_rx_desc *rx_desc;
 	struct ixgbevf_rx_buffer *bi;
 	unsigned int i = rx_ring->next_to_use;
 
-	while (cleaned_count--) {
-		rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
-		bi = &rx_ring->rx_buffer_info[i];
-
-		if (!bi->skb) {
-			struct sk_buff *skb;
+	/* nothing to do or no valid netdev defined */
+	if (!cleaned_count || !rx_ring->netdev)
+		return;
 
-			skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
-							rx_ring->rx_buf_len);
-			if (!skb)
-				goto no_buffers;
+	rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
+	bi = &rx_ring->rx_buffer_info[i];
+	i -= rx_ring->count;
 
-			bi->skb = skb;
+	do {
+		if (!ixgbevf_alloc_mapped_skb(rx_ring, bi))
+			break;
 
-			bi->dma = dma_map_single(rx_ring->dev, skb->data,
-						 rx_ring->rx_buf_len,
-						 DMA_FROM_DEVICE);
-			if (dma_mapping_error(rx_ring->dev, bi->dma)) {
-				dev_kfree_skb(skb);
-				bi->skb = NULL;
-				dev_err(rx_ring->dev, "Rx DMA map failed\n");
-				break;
-			}
-		}
+		/* Refresh the desc even if pkt_addr didn't change
+		 * because each write-back erases this info.
+		 */
 		rx_desc->read.pkt_addr = cpu_to_le64(bi->dma);
 
+		rx_desc++;
+		bi++;
 		i++;
-		if (i == rx_ring->count)
-			i = 0;
-	}
+		if (unlikely(!i)) {
+			rx_desc = IXGBEVF_RX_DESC(rx_ring, 0);
+			bi = rx_ring->rx_buffer_info;
+			i -= rx_ring->count;
+		}
+
+		/* clear the hdr_addr for the next_to_use descriptor */
+		rx_desc->read.hdr_addr = 0;
+
+		cleaned_count--;
+	} while (cleaned_count);
 
-no_buffers:
-	rx_ring->rx_stats.alloc_rx_buff_failed++;
-	if (rx_ring->next_to_use != i)
-		ixgbevf_release_rx_desc(rx_ring, i);
+	i += rx_ring->count;
+
+	if (rx_ring->next_to_use != i) {
+		/* record the next descriptor to use */
+		rx_ring->next_to_use = i;
+
+		/* Force memory writes to complete before letting h/w
+		 * know there are new descriptors to fetch.  (Only
+		 * applicable for weak-ordered memory model archs,
+		 * such as IA-64).
+		 */
+		wmb();
+		ixgbevf_write_tail(rx_ring, i);
+	}
 }
 
 static inline void ixgbevf_irq_enable_queues(struct ixgbevf_adapter *adapter,
@@ -489,8 +521,8 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 	struct sk_buff *skb;
 	unsigned int i;
 	u32 len, staterr;
-	int cleaned_count = 0;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
+	u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
 
 	i = rx_ring->next_to_clean;
 	rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
@@ -571,8 +603,6 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		ixgbevf_rx_skb(q_vector, skb, staterr, rx_desc);
 
 next_desc:
-		rx_desc->wb.upper.status_error = 0;
-
 		/* return some buffers to hardware, one at a time is too slow */
 		if (cleaned_count >= IXGBEVF_RX_BUFFER_WRITE) {
 			ixgbevf_alloc_rx_buffers(rx_ring, cleaned_count);
@@ -587,11 +617,6 @@ next_desc:
 	}
 
 	rx_ring->next_to_clean = i;
-	cleaned_count = ixgbevf_desc_unused(rx_ring);
-
-	if (cleaned_count)
-		ixgbevf_alloc_rx_buffers(rx_ring, cleaned_count);
-
 	u64_stats_update_begin(&rx_ring->syncp);
 	rx_ring->stats.packets += total_rx_packets;
 	rx_ring->stats.bytes += total_rx_bytes;
@@ -599,6 +624,9 @@ next_desc:
 	q_vector->rx.total_packets += total_rx_packets;
 	q_vector->rx.total_bytes += total_rx_bytes;
 
+	if (cleaned_count)
+		ixgbevf_alloc_rx_buffers(rx_ring, cleaned_count);
+
 	return total_rx_packets;
 }
 
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 02/15] ixgbevf: Test Rx status bits directly out of the descriptor
From: Jeff Kirsher @ 2014-11-20 23:09 UTC (permalink / raw)
  To: davem
  Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene,
	Alexander Duyck, Jeff Kirsher
In-Reply-To: <1416524953-15161-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

Instead of keeping a local copy of the status bits from the descriptor
we can just read them directly - this is accomplished with the addition
of ixgbevf_test_staterr().

In addition instead of doing a byteswap on the status bits value, we
can byteswap the constant values we are testing since that can be done
at compile time which should help to improve performance on big-endian
systems.

CC: Alexander Duyck <alexander.h.duyck@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  7 +++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 59 ++++++++++-------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index ba96cb5..5f7d2f3 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -307,6 +307,13 @@ static inline bool ixgbevf_qv_disable(struct ixgbevf_q_vector *q_vector)
 	((_eitr) ? (1000000000 / ((_eitr) * 256)) : 8)
 #define EITR_REG_TO_INTS_PER_SEC EITR_INTS_PER_SEC_TO_REG
 
+/* ixgbevf_test_staterr - tests bits in Rx descriptor status and error fields */
+static inline __le32 ixgbevf_test_staterr(union ixgbe_adv_rx_desc *rx_desc,
+					  const u32 stat_err_bits)
+{
+	return rx_desc->wb.upper.status_error & cpu_to_le32(stat_err_bits);
+}
+
 static inline u16 ixgbevf_desc_unused(struct ixgbevf_ring *ring)
 {
 	u16 ntc = ring->next_to_clean;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index deda74d..19062dc 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -331,15 +331,14 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
  * ixgbevf_receive_skb - Send a completed packet up the stack
  * @q_vector: structure containing interrupt and ring information
  * @skb: packet to send up
- * @status: hardware indication of status of receive
  * @rx_desc: rx descriptor
  **/
 static void ixgbevf_receive_skb(struct ixgbevf_q_vector *q_vector,
-				struct sk_buff *skb, u8 status,
+				struct sk_buff *skb,
 				union ixgbe_adv_rx_desc *rx_desc)
 {
 	struct ixgbevf_adapter *adapter = q_vector->adapter;
-	bool is_vlan = (status & IXGBE_RXD_STAT_VP);
+	bool is_vlan = !!ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_VP);
 	u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
 
 	if (is_vlan && test_bit(tag & VLAN_VID_MASK, adapter->active_vlans))
@@ -355,11 +354,10 @@ static void ixgbevf_receive_skb(struct ixgbevf_q_vector *q_vector,
  * ixgbevf_rx_skb - Helper function to determine proper Rx method
  * @q_vector: structure containing interrupt and ring information
  * @skb: packet to send up
- * @status: hardware indication of status of receive
  * @rx_desc: rx descriptor
  **/
 static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
-			   struct sk_buff *skb, u8 status,
+			   struct sk_buff *skb,
 			   union ixgbe_adv_rx_desc *rx_desc)
 {
 #ifdef CONFIG_NET_RX_BUSY_POLL
@@ -372,17 +370,17 @@ static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
 	}
 #endif /* CONFIG_NET_RX_BUSY_POLL */
 
-	ixgbevf_receive_skb(q_vector, skb, status, rx_desc);
+	ixgbevf_receive_skb(q_vector, skb, rx_desc);
 }
 
-/**
- * ixgbevf_rx_checksum - indicate in skb if hw indicated a good cksum
- * @ring: pointer to Rx descriptor ring structure
- * @status_err: hardware indication of status of receive
+/* ixgbevf_rx_checksum - indicate in skb if hw indicated a good cksum
+ * @ring: structure containig ring specific data
+ * @rx_desc: current Rx descriptor being processed
  * @skb: skb currently being received and modified
- **/
+ */
 static inline void ixgbevf_rx_checksum(struct ixgbevf_ring *ring,
-				       u32 status_err, struct sk_buff *skb)
+				       union ixgbe_adv_rx_desc *rx_desc,
+				       struct sk_buff *skb)
 {
 	skb_checksum_none_assert(skb);
 
@@ -391,16 +389,16 @@ static inline void ixgbevf_rx_checksum(struct ixgbevf_ring *ring,
 		return;
 
 	/* if IP and error */
-	if ((status_err & IXGBE_RXD_STAT_IPCS) &&
-	    (status_err & IXGBE_RXDADV_ERR_IPE)) {
+	if (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_IPCS) &&
+	    ixgbevf_test_staterr(rx_desc, IXGBE_RXDADV_ERR_IPE)) {
 		ring->rx_stats.csum_err++;
 		return;
 	}
 
-	if (!(status_err & IXGBE_RXD_STAT_L4CS))
+	if (!ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_L4CS))
 		return;
 
-	if (status_err & IXGBE_RXDADV_ERR_TCPE) {
+	if (ixgbevf_test_staterr(rx_desc, IXGBE_RXDADV_ERR_TCPE)) {
 		ring->rx_stats.csum_err++;
 		return;
 	}
@@ -520,33 +518,29 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 	struct ixgbevf_rx_buffer *rx_buffer_info, *next_buffer;
 	struct sk_buff *skb;
 	unsigned int i;
-	u32 len, staterr;
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
 	u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
 
 	i = rx_ring->next_to_clean;
 	rx_desc = IXGBEVF_RX_DESC(rx_ring, i);
-	staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
 	rx_buffer_info = &rx_ring->rx_buffer_info[i];
 
-	while (staterr & IXGBE_RXD_STAT_DD) {
+	while (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_DD)) {
 		if (!budget)
 			break;
 		budget--;
 
 		rmb(); /* read descriptor and rx_buffer_info after status DD */
-		len = le16_to_cpu(rx_desc->wb.upper.length);
+
 		skb = rx_buffer_info->skb;
 		prefetch(skb->data - NET_IP_ALIGN);
 		rx_buffer_info->skb = NULL;
 
-		if (rx_buffer_info->dma) {
-			dma_unmap_single(rx_ring->dev, rx_buffer_info->dma,
-					 rx_ring->rx_buf_len,
-					 DMA_FROM_DEVICE);
-			rx_buffer_info->dma = 0;
-			skb_put(skb, len);
-		}
+		dma_unmap_single(rx_ring->dev, rx_buffer_info->dma,
+				 rx_ring->rx_buf_len,
+				 DMA_FROM_DEVICE);
+		rx_buffer_info->dma = 0;
+		skb_put(skb, le16_to_cpu(rx_desc->wb.upper.length));
 
 		i++;
 		if (i == rx_ring->count)
@@ -558,7 +552,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 
 		next_buffer = &rx_ring->rx_buffer_info[i];
 
-		if (!(staterr & IXGBE_RXD_STAT_EOP)) {
+		if (!(ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP))) {
 			skb->next = next_buffer->skb;
 			IXGBE_CB(skb->next)->prev = skb;
 			rx_ring->rx_stats.non_eop_descs++;
@@ -576,12 +570,13 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		}
 
 		/* ERR_MASK will only have valid bits if EOP set */
-		if (unlikely(staterr & IXGBE_RXDADV_ERR_FRAME_ERR_MASK)) {
+		if (unlikely(ixgbevf_test_staterr(rx_desc,
+					    IXGBE_RXDADV_ERR_FRAME_ERR_MASK))) {
 			dev_kfree_skb_irq(skb);
 			goto next_desc;
 		}
 
-		ixgbevf_rx_checksum(rx_ring, staterr, skb);
+		ixgbevf_rx_checksum(rx_ring, rx_desc, skb);
 
 		/* probably a little skewed due to removing CRC */
 		total_rx_bytes += skb->len;
@@ -600,7 +595,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 			goto next_desc;
 		}
 
-		ixgbevf_rx_skb(q_vector, skb, staterr, rx_desc);
+		ixgbevf_rx_skb(q_vector, skb, rx_desc);
 
 next_desc:
 		/* return some buffers to hardware, one at a time is too slow */
@@ -612,8 +607,6 @@ next_desc:
 		/* use prefetched values */
 		rx_desc = next_rxd;
 		rx_buffer_info = &rx_ring->rx_buffer_info[i];
-
-		staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
 	}
 
 	rx_ring->next_to_clean = i;
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 00/15][pull request] Intel Wired LAN Driver Updates 2014-11-20
From: Jeff Kirsher @ 2014-11-20 23:08 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene

This series contains updates to ixgbevf, i40e and i40evf.

Emil updates ixgbevf with much of the work that Alex Duyck did while at
Intel.  First updates the driver to clear the status bits on allocation
instead of in the cleanup routine, this way we can leave the recieve
descriptor rings as a read only memory block until we actually have
buffers to give back to the hardware.  Clean up ixgbevf_clean_rx_irq()
by creating ixgbevf_process_skb_field() to merge several similar
operations into this new function.  Cleanup temporary variables within
the receive hot-path and reducing the scope of variables that do not
need to exist outside the main loop.  Save on stack space by just
storing our updated values back in next_to_clean instead of using
a stack variable, which also collapses the size the function.  Improve
performace on IOMMU enabled systems and reduce cache misses by changing
the basic receive patch for ixgbevf so that instead of receiving the
data into an skb, it is received into a double buffered page.  Add
netpoll support by creating ixgbevf_netpoll(), which is a callback for
.ndo_poll_controller to allow for the VF interface to be used with
netconsole.

Mitch provides several cleanups and trivial fixes for i40e and i40evf.
First is a fix the overloading of the msg_size field in the
arq_event_info struct by splitting the field into two and renaming to
indicate the actual function of each field.  Updates code comments
to match the actual function.  Cleanup several checkpatch.pl warnings
by adding or removing blank lines, aligning function parameters, and
correcting over-long lines (which makes the code more readable).

Shannon provides a patch for i40e to write the extra bits that will
turn off the ITR wait for the interrupt, since we want the SW INT to
go off as soon as possible.

v2: updated patch 07 based on feedback from Alex Duyck by
 - adding pfmemalloc check to a new function for reusable page
 - moved atomic_inc outside of #if/else in ixgbevf_add_rx_frag()
 - reverted the removal of the API check in ixgbevf_change_mtu()

The following are changes since commit daaf427c6ab392bedcd018e326b2ffa1e1110cd6:
  bpf: fix arraymap NULL deref and missing overflow and zero size checks
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Emil Tantilov (9):
  ixgbevf: Update ixgbevf_alloc_rx_buffers to handle clearing of status
    bits
  ixgbevf: Test Rx status bits directly out of the descriptor
  ixgbevf: Combine the logic for post Rx processing into single function
  ixgbevf: Cleanup variable usage, improve stack performance
  ixgbevf: reorder main loop in ixgbe_clean_rx_irq to allow for
    do/while/continue
  ixgbevf: Update Rx next to clean in real time
  ixgbevf: Change receive model to use double buffered page based
    receives
  ixgbevf: compare total_rx_packets and budget in ixgbevf_clean_rx_irq
  ixgbevf: add netpoll support

Mitch Williams (5):
  i40e: don't overload fields
  i40evf: update header comments
  i40evf: make checkpatch happy
  i40evf: make comparisons consistent
  i40evf: remove unnecessary else

Shannon Nelson (1):
  i40e: trigger SW INT with no ITR wait

 drivers/net/ethernet/intel/i40e/i40e_adminq.c      |   6 +-
 drivers/net/ethernet/intel/i40e/i40e_adminq.h      |   3 +-
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |   5 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c        |  17 +-
 drivers/net/ethernet/intel/i40evf/i40e_adminq.c    |   6 +-
 drivers/net/ethernet/intel/i40evf/i40e_adminq.h    |   3 +-
 drivers/net/ethernet/intel/i40evf/i40evf.h         |   2 +-
 drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c |  14 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    |  71 +-
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    |  23 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h       |  39 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c  | 720 ++++++++++++++-------
 12 files changed, 595 insertions(+), 314 deletions(-)

-- 
1.9.3

^ permalink raw reply

* Re: [PATCH net-next] tcp: remove from tcp_sendmsg() some fastopen code
From: Eric Dumazet @ 2014-11-20 23:05 UTC (permalink / raw)
  To: David Miller, Al Viro; +Cc: ycheng, nuclearcat, netdev, ncardwell
In-Reply-To: <1416500634.8629.31.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, 2014-11-20 at 08:23 -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> If we consume iovec bytes in tcp_send_syn_data(), we can remove
> annoying fastopen code in tcp_sendmsg() skipping over the already
> consumed bytes.
> 
> Also add an unlikely(flags & MSG_FASTOPEN), as most TCP sendmsg() do not
> ask for FASTOPEN.
> 
> Tested:
>  Ran our 125 packetdrill fastopen tests
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---

Please disregard this patch, Al Viro said sendmsg() was apparently not
allowed to change iovec.

Some callers depend on it staying constant.

Not clear why memcpy_fromiovec() even exists.

^ permalink raw reply

* Re: [RFC] situation with csum_and_copy_... API
From: Eric Dumazet @ 2014-11-20 22:53 UTC (permalink / raw)
  To: Al Viro
  Cc: David Miller, torvalds, netdev, linux-kernel, target-devel,
	Nicholas A. Bellinger, Christoph Hellwig
In-Reply-To: <20141120222506.GS7996@ZenIV.linux.org.uk>

On Thu, 2014-11-20 at 22:25 +0000, Al Viro wrote:

> Yes, it is.  You are breaking several _other_ kernel_sendmsg() users.
> They are already slightly broken, but that'll make breakage much more
> common.
> 
> Please, don't - the right thing to do is to have iov_iter in msghdr
> (we already have the kernel and userland ones with different types and
> we do not assume their layouts to be identical - currently they are,
> but it's easy to change), keep iovec constant in all cases and advance
> ->msg_iter.  Also in all cases.
> 
> Note that direct manipulations of what's currently in ->msg_iov are
> wrong - all those loops over vector elements, etc., belong in low-level
> primitives.  The main missing ones right now are csum_and_copy_{from,to}_iter()
> - I have those in local queue, but I'm still trying to get a reasonably
> clean mm/iov_iter.c without ridiculous amounts of boilerplating.  A bit more
> massage is needed there...
> 
> Seriously, take a look at vfs.git#iov_iter-net; it's preparations for the
> one that'll introduce ->msg_iter.  Right now that branch has local iov_iter
> declared and initialized in several ->sendmsg() and ->recvmsg() instances and
> fed to primitives that work with it; after the conversion it'll be in
> msg->msg_iter and it will be initialized by sock_sendmsg()/sock_recvmsg().
> 
> The tricky part is how to get through that without temporary breaking the
> existing sendmsg/recvmsg users in the kernel *and* without a patch size from
> hell.  I more or less see how to carve the remaining steps into
> reasonably-sized chunks; iscsi is one of the tricky ones and it, AFAICS,
> is genuinely broken in mainline and will need fixes that can go into -stable.
> 
> And no, your solution doesn't work.  Sorry.  You'll break e.g. smb_send_kvec()
> that way.  ceph_tcp_sendmsg() as well, IIRC.

Nowhere in tcp_sendmsg() the iov had const qualifier.

If it was declared as const, this discussion would not happen,
we would know we are not allowed to modify it.

iov_iter is nice, but not a single time it is used in net/ 

Please make sure to add const where appropriate.

Thanks.

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Jay Vosburgh @ 2014-11-20 22:53 UTC (permalink / raw)
  To: Cong Wang; +Cc: Eric Dumazet, Wengang, netdev
In-Reply-To: <CAHA+R7P9XhGQZXFy6frxOyZJ4rX_MdVgY3kAPE1_wDNcvYd_yA@mail.gmail.com>

Cong Wang <cwang@twopensource.com> wrote:

>Also, no one seems to care about my previous question:
>why only bonding has the problem?

	Bonding has the problem because it stashes a pointer to a data
structure (the header_ops) from another module, and when that module is
unloaded the dangling pointer may be dereferenced if it's not either
cleared or made to never go away.

	Setting the bonding->header_ops to NULL (to avoid the current
problem with pktgen) has a race in dev_hard_header between where the
header_ops pointer is checked and where the ->create function is called.

	This pointer business is the main reason the bonding path for
"not ARPHRD_ETHER" (i.e., ipoib) has extra complexity in the open/close
path, e.g.,

bond_slave_netdev_event():
[...]
        switch (event) {
        case NETDEV_UNREGISTER:
                if (bond_dev->type != ARPHRD_ETHER)
                        bond_release_and_destroy(bond_dev, slave_dev);
                else
                        bond_release(bond_dev, slave_dev);

	If the ipoib ops were static in vmlinux, that would resolve the
pktgen problem, and also may eliminate the need for some of the ugly
bits like what I've pasted in above.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* [PATCH net] cxgb4 : Fix DCB priority groups being returned in wrong order
From: Anish Bhatt @ 2014-11-20 22:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, hariprasad, Anish Bhatt

Peer priority groups were being reversed, but this was missed in the previous
fix sent out for this issue.

Fixes :	ee7bc3cdc270 ('cxgb4 : dcb open-lldp interop fixes')

Signed-off-by: Anish Bhatt <anish@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
index cca6049..d8e60b3 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
@@ -1082,7 +1082,7 @@ static int cxgb4_cee_peer_getpg(struct net_device *dev, struct cee_pg *pg)
 	pgid = be32_to_cpu(pcmd.u.dcb.pgid.pgid);
 
 	for (i = 0; i < CXGB4_MAX_PRIORITY; i++)
-		pg->prio_pg[i] = (pgid >> (i * 4)) & 0xF;
+		pg->prio_pg[7 - i] = (pgid >> (i * 4)) & 0xF;
 
 	INIT_PORT_DCB_READ_PEER_CMD(pcmd, pi->port_id);
 	pcmd.u.dcb.pgrate.type = FW_PORT_DCB_TYPE_PGRATE;
@@ -1094,7 +1094,8 @@ static int cxgb4_cee_peer_getpg(struct net_device *dev, struct cee_pg *pg)
 	}
 
 	for (i = 0; i < CXGB4_MAX_PRIORITY; i++)
-		pg->pg_bw[i] = pcmd.u.dcb.pgrate.pgrate[i];
+		pg->pg_bw[7 - i] = pcmd.u.dcb.pgrate.pgrate[
+			(pgid >> (i * 4)) & 0xF];
 
 	return 0;
 }
-- 
2.1.3

^ permalink raw reply related

* Re: [linux-nics] [PATCH 0/4] move pci_assivned_vfs() check (while disabling VFs) to pci sub-system
From: Jeff Kirsher @ 2014-11-20 22:42 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Sathya Perla, linux.nics, shahed.shaikh, linux-pci, ddutile,
	ariel.elior, netdev
In-Reply-To: <20141120222127.GB7987@google.com>

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Thu, 2014-11-20 at 15:21 -0700, Bjorn Helgaas wrote:
> [+cc Jeff]
> 
> On Mon, Nov 10, 2014 at 05:23:26PM +0530, Sathya Perla wrote:
> > A user must not be allowed to disable VFs while they are already assigned to
> > a guest. This check is being made in each individual driver that implements
> > the sriov_configure PCI method.
> > This patch-set fixes this code duplication by moving this check from
> > drivers to the sriov_nuvfs_store() routine just before invoking
> > sriov_configure() when num_vfs is equal to 0.
> > 
> > Vasundhara Volam (4):
> >   pci: move pci_assivned_vfs() check while disabling VFs to pci
> >     sub-system
> >   bnx2x: remove pci_assigned_vfs() check while disabling VFs
> >   i40e: remove pci_assigned_vfs() check while disabling VFs
> >   qlcnic: remove pci_assigned_vfs() check while disabling VFs
> > 
> >  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c  |    2 +-
> >  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |    7 +------
> >  .../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c   |   10 ----------
> >  drivers/pci/pci-sysfs.c                            |    5 +++++
> >  4 files changed, 7 insertions(+), 17 deletions(-)
> 
> I'm dropping these for the reasons Don & Alex outlined -- they don't fix
> the problem for VFIO, so this amounts to shuffling around code that's known
> to be broken, which seems more confusing than worthwhile.
> 
> Jeff, if I were you I would drop the i40e patch.  I don't think it makes
> sense to remove the check from i40e before adding it to the PCI core.
> 

Thanks for the heads up, I will drop the i40e patch as well.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-20 22:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, torvalds, netdev, linux-kernel, target-devel,
	Nicholas A. Bellinger, Christoph Hellwig
In-Reply-To: <1416520542.8629.46.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 20, 2014 at 01:55:42PM -0800, Eric Dumazet wrote:
> On Thu, 2014-11-20 at 21:47 +0000, Al Viro wrote:
> 
> > As far as I can see, these retries on the send side are simply broken -
> > normally we are talking to TCP sockets there and tcp_sendmsg() does *not*
> > modify iovec in normal case. 
> 
> Arg... I sent this morning something doing this (against net-next tree)
> 
> Is it a problem ?

Yes, it is.  You are breaking several _other_ kernel_sendmsg() users.
They are already slightly broken, but that'll make breakage much more
common.

Please, don't - the right thing to do is to have iov_iter in msghdr
(we already have the kernel and userland ones with different types and
we do not assume their layouts to be identical - currently they are,
but it's easy to change), keep iovec constant in all cases and advance
->msg_iter.  Also in all cases.

Note that direct manipulations of what's currently in ->msg_iov are
wrong - all those loops over vector elements, etc., belong in low-level
primitives.  The main missing ones right now are csum_and_copy_{from,to}_iter()
- I have those in local queue, but I'm still trying to get a reasonably
clean mm/iov_iter.c without ridiculous amounts of boilerplating.  A bit more
massage is needed there...

Seriously, take a look at vfs.git#iov_iter-net; it's preparations for the
one that'll introduce ->msg_iter.  Right now that branch has local iov_iter
declared and initialized in several ->sendmsg() and ->recvmsg() instances and
fed to primitives that work with it; after the conversion it'll be in
msg->msg_iter and it will be initialized by sock_sendmsg()/sock_recvmsg().

The tricky part is how to get through that without temporary breaking the
existing sendmsg/recvmsg users in the kernel *and* without a patch size from
hell.  I more or less see how to carve the remaining steps into
reasonably-sized chunks; iscsi is one of the tricky ones and it, AFAICS,
is genuinely broken in mainline and will need fixes that can go into -stable.

And no, your solution doesn't work.  Sorry.  You'll break e.g. smb_send_kvec()
that way.  ceph_tcp_sendmsg() as well, IIRC.

^ permalink raw reply

* Re: [PATCH 0/4] move pci_assivned_vfs() check (while disabling VFs) to pci sub-system
From: Bjorn Helgaas @ 2014-11-20 22:21 UTC (permalink / raw)
  To: Sathya Perla
  Cc: linux-pci, netdev, ariel.elior, linux.nics, shahed.shaikh,
	ddutile, Jeff Kirsher
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>

[+cc Jeff]

On Mon, Nov 10, 2014 at 05:23:26PM +0530, Sathya Perla wrote:
> A user must not be allowed to disable VFs while they are already assigned to
> a guest. This check is being made in each individual driver that implements
> the sriov_configure PCI method.
> This patch-set fixes this code duplication by moving this check from
> drivers to the sriov_nuvfs_store() routine just before invoking
> sriov_configure() when num_vfs is equal to 0.
> 
> Vasundhara Volam (4):
>   pci: move pci_assivned_vfs() check while disabling VFs to pci
>     sub-system
>   bnx2x: remove pci_assigned_vfs() check while disabling VFs
>   i40e: remove pci_assigned_vfs() check while disabling VFs
>   qlcnic: remove pci_assigned_vfs() check while disabling VFs
> 
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c  |    2 +-
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |    7 +------
>  .../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c   |   10 ----------
>  drivers/pci/pci-sysfs.c                            |    5 +++++
>  4 files changed, 7 insertions(+), 17 deletions(-)

I'm dropping these for the reasons Don & Alex outlined -- they don't fix
the problem for VFIO, so this amounts to shuffling around code that's known
to be broken, which seems more confusing than worthwhile.

Jeff, if I were you I would drop the i40e patch.  I don't think it makes
sense to remove the check from i40e before adding it to the PCI core.

Bjorn

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Cong Wang @ 2014-11-20 22:13 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Wengang, netdev
In-Reply-To: <1416521035.8629.49.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 20, 2014 at 2:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Provide a patch, please.
>
>

Don't blame me.

I want to provide a real fix, you want a minimum fix for stable.
We agree that we disagree on this point, right? What's
more, according to your rule, I should yield to you when I
touch something you want to touch.

Also, no one seems to care about my previous question:
why only bonding has the problem?

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Eric Dumazet @ 2014-11-20 22:03 UTC (permalink / raw)
  To: Cong Wang; +Cc: Wengang, netdev
In-Reply-To: <CAHA+R7Me4zPdGpeQVHFLLGVq=4cVar09jQFyjA81nxF_Fmuppw@mail.gmail.com>

On Thu, 2014-11-20 at 13:57 -0800, Cong Wang wrote:
> On Thu, Nov 20, 2014 at 12:41 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Thu, 2014-11-20 at 09:34 -0800, Cong Wang wrote:
> >
> >> I didn't look into ipoib_header_ops, thought it might have some dependency
> >> on symbols.
> >
> > I did look before answering and suggesting this, you really should do
> > the same instead of giving advices of over engineering the stack.
> >
> > Best is the enemy of the good.
> >
> > Its hard to find some networking function trivial than this one.
> 
> What about other modules defining *header_ops? Don't they
> need to move to vmlinux as well?

Yep, if they can be in a bonding device, for practical reasons, not to
prove your point.

> 
> I still don't like this workaround even just for stable. Although
> definitely a real fix could be harder to backport, for me it is normal
> backport 8+ patches to stable:
> 
> http://www.spinics.net/lists/stable/msg66122.html
> http://www.spinics.net/lists/linux-fsdevel/msg79967.html
> 
> I know you disagree, I don't even want to waste time on arguing it.

Whatever, I really don't care.

Do your stuff, but don't ask people asking for an easy fix to do the
heart surgery.

Provide a patch, please.

^ permalink raw reply

* Re: [PATCH net-next] filter: add bpf_optimize_div()
From: Daniel Borkmann @ 2014-11-20 21:57 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: Denis Kirjanov, Network Development
In-Reply-To: <CAMEtUuw1q_=CuZOHqd1WDd8ZE+bo-CNsbQ4rFPgRRVy7H1Q4uA@mail.gmail.com>

On 11/20/2014 10:07 PM, Alexei Starovoitov wrote:
> On Thu, Nov 20, 2014 at 11:42 AM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
...
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index ca95abd..b385637 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> +
>> +static inline int bpf_optimize_div(u32 *k)
>
> 'inline' is unnecessary

Btw, it's a header file here, so it's necessary here.

...
> In general I don't think such optimizations
> belong in kernel at all. User space should be doing this.

+1

^ permalink raw reply

* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Cong Wang @ 2014-11-20 21:57 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Wengang, netdev
In-Reply-To: <1416516104.8629.39.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Nov 20, 2014 at 12:41 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2014-11-20 at 09:34 -0800, Cong Wang wrote:
>
>> I didn't look into ipoib_header_ops, thought it might have some dependency
>> on symbols.
>
> I did look before answering and suggesting this, you really should do
> the same instead of giving advices of over engineering the stack.
>
> Best is the enemy of the good.
>
> Its hard to find some networking function trivial than this one.

What about other modules defining *header_ops? Don't they
need to move to vmlinux as well?

I still don't like this workaround even just for stable. Although
definitely a real fix could be harder to backport, for me it is normal
backport 8+ patches to stable:

http://www.spinics.net/lists/stable/msg66122.html
http://www.spinics.net/lists/linux-fsdevel/msg79967.html

I know you disagree, I don't even want to waste time on arguing it.

^ permalink raw reply

* Re: [RFC] situation with csum_and_copy_... API
From: Eric Dumazet @ 2014-11-20 21:55 UTC (permalink / raw)
  To: Al Viro
  Cc: David Miller, torvalds, netdev, linux-kernel, target-devel,
	Nicholas A. Bellinger, Christoph Hellwig
In-Reply-To: <20141120214753.GR7996@ZenIV.linux.org.uk>

On Thu, 2014-11-20 at 21:47 +0000, Al Viro wrote:

> As far as I can see, these retries on the send side are simply broken -
> normally we are talking to TCP sockets there and tcp_sendmsg() does *not*
> modify iovec in normal case. 

Arg... I sent this morning something doing this (against net-next tree)

Is it a problem ?

Or can we consider FASTOPEN being not normal case ? ;)

https://patchwork.ozlabs.org/patch/412776/

^ permalink raw reply

* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Rick Jones @ 2014-11-20 21:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1416520109.8629.44.camel@edumazet-glaptop2.roam.corp.google.com>

On 11/20/2014 01:48 PM, Eric Dumazet wrote:
> On Thu, 2014-11-20 at 13:31 -0800, Rick Jones wrote:
>
>> Do you mean when there is actually data in the SYN?
>
> No, I mean the packet can be _dropped_ here, really.
>
> SYN flood for example.

Ah.  I did not pick-up on that.  Seeing the kfree_skb() in the original 
code I thought it was the end of the line for the segment.

rick

^ permalink raw reply

* Re: [PATCH net-next] tcp: Remove some spurious dropped packet profile hits from the passive connection accept path
From: Eric Dumazet @ 2014-11-20 21:48 UTC (permalink / raw)
  To: Rick Jones; +Cc: Rick Jones, netdev, davem
In-Reply-To: <546E5DAF.5090907@hp.com>

On Thu, 2014-11-20 at 13:31 -0800, Rick Jones wrote:

> Do you mean when there is actually data in the SYN?

No, I mean the packet can be _dropped_ here, really.

SYN flood for example.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox