fm10k: conditionally compile DCB and DebugFS support

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 05/17] fm10k: always check init_hw for errors

A recent change to the SHARED code makes it such that in some flows,

init_hw may fail. For example, init_hw_vf will fail if the 1st queue is not owned by the VF. However, previously some flows called init_hw without checking the return code, or checked it but only to display a diagnostic message.

Fix this by (a) always returning and preventing the netdevice from going up, and (b) printing the diagnostic in every flow for consistency. This should resolve an issue where VF drivers would attempt to come up before the PF has finished assigning queues.

In addition, change the dmesg output to explicitly show the actual function that failed, instead of combining reset_hw and init_hw into a single check.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 34
++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 5 deletions(-)

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +10 -2

index 48f0ef4545a0..9ad9f9164d91 100644
@@ -166,9 +166,17 @@ static void fm10k_reinit(struct fm10k_intfc
 *interface)
 	interface->last_reset = jiffies + (10 * HZ);
 
 	/* reset and initialize the hardware so it is in a known state */
-	err = hw->mac.ops.reset_hw(hw) ? : hw->mac.ops.init_hw(hw);
-	if (err)
+	err = hw->mac.ops.reset_hw(hw);
+	if (err) {
+		dev_err(&interface->pdev->dev, "reset_hw failed: %d\n",
 err);
+		goto reinit_err;
+	}
+
+	err = hw->mac.ops.init_hw(hw);
+	if (err) {
 		dev_err(&interface->pdev->dev, "init_hw failed: %d\n",
 err);
+		goto reinit_err;
+	}

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4 -0

 	err = fm10k_init_queueing_scheme(interface);
 	if (err) {

@@ -202,6 +210,10 @@ static void fm10k_reinit(struct fm10k_intfc
 *interface)
 
 	fm10k_iov_resume(interface->pdev);
 
+reinit_err:
+	if (err)
+		netif_device_detach(netdev);
+
 	rtnl_unlock();
 

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +7 -1

 	clear_bit(__FM10K_RESETTING, &interface->state);

@@ -1693,7 +1705,13 @@ static int fm10k_sw_init(struct fm10k_intfc
 *interface,
 	interface->last_reset = jiffies + (10 * HZ);
 
 	/* reset and initialize the hardware so it is in a known state */
-	err = hw->mac.ops.reset_hw(hw) ? : hw->mac.ops.init_hw(hw);
+	err = hw->mac.ops.reset_hw(hw);
+	if (err) {
+		dev_err(&pdev->dev, "reset_hw failed: %d\n", err);
+		return err;
+	}
+
+	err = hw->mac.ops.init_hw(hw);
 	if (err) {
 		dev_err(&pdev->dev, "init_hw failed: %d\n", err);

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +8 -2

 		return err;

@@ -2077,8 +2095,10 @@ static int fm10k_resume(struct pci_dev *pdev)
 
 	/* reset hardware to known state */
 	err = hw->mac.ops.init_hw(&interface->hw);
-	if (err)
+	if (err) {
+		dev_err(&pdev->dev, "init_hw failed: %d\n", err);
 		return err;
+	}
 
 	/* reset statistics starting values */
 	hw->mac.ops.rebind_hw_stats(hw, &interface->stats);

@@ -2257,7 +2277,11 @@ static void fm10k_io_resume(struct pci_dev
 *pdev)
 	int err = 0;
 
 	/* reset hardware to known state */
-	hw->mac.ops.init_hw(&interface->hw);
+	err = hw->mac.ops.init_hw(&interface->hw);
+	if (err) {
+		dev_err(&pdev->dev, "init_hw failed: %d\n", err);
+		return;
+	}
 
 	/* reset statistics starting values */

hw->mac.ops.rebind_hw_stats(hw, &interface->stats);
--
2.6.1.264.gbab76a9

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Quoted 10 lines

-----Original Message-----
From: Intel-wired-lan [mailto:
intel-wired-lan-bounces at lists.osuosl.org] On
Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 05/17] fm10k: always check init_hw
for errors

Quoted 2 lines

A recent change to the SHARED code makes it such that in some flows,

"SHARED code" has no meaning in the upstream driver, please change this description.

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 06/17] fm10k: Correct typecast in fm10k_update_xc_addr_pf

Since the resultant data type of the mac_update.mac_upper field is u16, it does not make sense to typecast u8 variables to u32 first.

Reported-by Matthew Vick <[email protected]>

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/fm10k/fm10k_pf.c +2 -2

index 8c0bdc4e4edd..789ffd9d7fd9 100644
@@ -334,8 +334,8 @@ static s32 fm10k_update_xc_addr_pf(struct
 fm10k_hw *hw, u16 glort,
 						 ((u32)mac[3] << 16) |
 						 ((u32)mac[4] << 8) |
 						 ((u32)mac[5]));
-	mac_update.mac_upper = cpu_to_le16(((u32)mac[0] << 8) |
-						 ((u32)mac[1]));
+	mac_update.mac_upper = cpu_to_le16(((u16)mac[0] << 8) |
+					   ((u16)mac[1]));
 	mac_update.vlan = cpu_to_le16(vid);
 	mac_update.glort = cpu_to_le16(glort);

mac_update.action = add ? 0 : 1;
--
2.6.1.264.gbab76a9

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 08/17] fm10k: add statistics for actual DWORD count of mbmem mailbox

A previous bug was uncovered by addition of a debug stat to indicate the actual number of DWORDS we pulled from the mbmem. It turned out this was
not the same as the tx_dwords counter. While the previous bug fix should have corrected this in all cases, add some debug stats that count the number of DWORDs pushed or pulled from the mbmem. A future debugger may
take advantage of this statistic for debugging purposes.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 2 ++
 drivers/net/ethernet/intel/fm10k/fm10k_mbx.c     | 4 ++++
 drivers/net/ethernet/intel/fm10k/fm10k_mbx.h     | 2 ++

 3 files changed, 8 insertions(+)

drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c +2 -0

index 447a5f8da42f..5db5a97cf28e 100644
@@ -114,9 +114,11 @@ static const struct fm10k_stats
 fm10k_gstrings_mbx_stats[] = {
 	FM10K_MBX_STAT("mbx_tx_oversized", tx_dropped),
 	FM10K_MBX_STAT("mbx_tx_messages", tx_messages),
 	FM10K_MBX_STAT("mbx_tx_dwords", tx_dwords),
+	FM10K_MBX_STAT("mbx_tx_mbmem_pulled",
 tx_mbmem_pulled),
 	FM10K_MBX_STAT("mbx_rx_messages", rx_messages),
 	FM10K_MBX_STAT("mbx_rx_dwords", rx_dwords),
 	FM10K_MBX_STAT("mbx_rx_parse_err", rx_parse_err),
+	FM10K_MBX_STAT("mbx_rx_mbmem_pushed",
 rx_mbmem_pushed),

};

#define FM10K_GLOBAL_STATS_LEN
ARRAY_SIZE(fm10k_gstrings_global_stats)

drivers/net/ethernet/intel/fm10k/fm10k_mbx.c +2 -0

index af09a1b272e6..2bce47490723 100644
@@ -375,6 +375,8 @@ static void fm10k_mbx_write_copy(struct fm10k_hw
 *hw,
 			if (!tail)
 				tail++;
 
+			mbx->tx_mbmem_pulled++;
+
 			/* write message to hardware FIFO */
 			fm10k_write_reg(hw, mbmem + tail++, *(head++));

drivers/net/ethernet/intel/fm10k/fm10k_mbx.c +2 -0

 		} while (--len && --end);

@@ -459,6 +461,8 @@ static void fm10k_mbx_read_copy(struct fm10k_hw
 *hw,
 			if (!head)
 				head++;
 
+			mbx->rx_mbmem_pushed++;
+
 			/* read message from hardware FIFO */
 			*(tail++) = fm10k_read_reg(hw, mbmem +

head++);

drivers/net/ethernet/intel/fm10k/fm10k_mbx.c +0 -0

 		} while (--len && --end);

drivers/net/ethernet/intel/fm10k/fm10k_mbx.h +2 -1

index 0419a7f0035e..b8eb6b5be894 100644
@@ -291,8 +291,10 @@ struct fm10k_mbx_info {
 	u64 tx_dropped;
 	u64 tx_messages;
 	u64 tx_dwords;
+	u64 tx_mbmem_pulled;
 	u64 rx_messages;
 	u64 rx_dwords;
+	u64 rx_mbmem_pushed;
 	u64 rx_parse_err;
 
 	/* Buffer to store messages */
--

2.6.1.264.gbab76a9

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 10/17] fm10k: add TEB check to fm10k_gre_is_nvgre

The NVGRE protocol should be run over transparent ethernet bridge protocol, so we should verify this in our check whether the GRE tunnel is NVGRE.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 ++++

 1 file changed, 4 insertions(+)

drivers/net/ethernet/intel/fm10k/fm10k_main.c +4 -0

index 746a1986690b..5dfcba72825d 100644
@@ -708,6 +708,10 @@ static struct ethhdr *fm10k_gre_is_nvgre(struct
 sk_buff *skb)
 	if (nvgre_hdr->flags & FM10K_NVGRE_RESERVED0_FLAGS)
 		return NULL;
 
+	/* verify protocol is transparent Ethernet bridging */
+	if (nvgre_hdr->proto != htons(ETH_P_TEB))
+		return NULL;
+
 	/* report start of ethernet header */
 	if (nvgre_hdr->flags & NVGRE_TNI)

return (struct ethhdr *)(nvgre_hdr + 1);
--
2.6.1.264.gbab76a9

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 11/17] fm10k: Add support for ITR scaling based on PCIe link speed

Red Rock Canyon's interrupt throttle timers are based on the PCIe link speed. Because of this, the value being programmed into the ITR registers must be scaled.

For the PF, this is as simple as reading the PCIe link speed and storing the result. However, in the case of SR-IOV, the VF's interrupt throttle timers are based on the link speed of the PF. However, the VF is unable to get the link speed information from its configuration space, so the PF must inform it of what scale to use.

Rather than pass this scale via mailbox message, take advantage of unused bits in the TDLEN register to pass the scale. It is the responsibility of the PF to program this for the VF while setting up the VF queues and the responsibility of the VF to get the information accordingly. This is preferable because it allows the VF to set up the interrupts properly during initialization and matches how the MAC address is passed in the TDBAL/TDBAH registers.

Reported-by Matthew Vick <[email protected]>

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c   | 22
+++++++++++++++++++++-
 drivers/net/ethernet/intel/fm10k/fm10k_type.h | 15 +++++++++++++++

 drivers/net/ethernet/intel/fm10k/fm10k_vf.c   | 20
++++++++++++++++++--
 3 files changed, 54 insertions(+), 3 deletions(-)

drivers/net/ethernet/intel/fm10k/fm10k_pf.c +9 -1

index 52f754b35972..9c3a48f04056 100644
@@ -150,19 +150,26 @@ static s32 fm10k_init_hw_pf(struct fm10k_hw
 *hw)
 				FM10K_TPH_RXCTRL_HDR_WROEN);
 	}
 
-	/* set max hold interval to align with 1.024 usec in all modes */
+	/* set max hold interval to align with 1.024 usec in all modes and
+	 * store ITR scale
+	 */
 	switch (hw->bus.speed) {
 	case fm10k_bus_speed_2500:
 		dma_ctrl = FM10K_DMA_CTRL_MAX_HOLD_1US_GEN1;
+		hw->mac.itr_scale = FM10K_TDLEN_ITR_SCALE_GEN1;
 		break;
 	case fm10k_bus_speed_5000:
 		dma_ctrl = FM10K_DMA_CTRL_MAX_HOLD_1US_GEN2;
+		hw->mac.itr_scale = FM10K_TDLEN_ITR_SCALE_GEN2;
 		break;
 	case fm10k_bus_speed_8000:
 		dma_ctrl = FM10K_DMA_CTRL_MAX_HOLD_1US_GEN3;
+		hw->mac.itr_scale = FM10K_TDLEN_ITR_SCALE_GEN3;
 		break;
 	default:
 		dma_ctrl = 0;
+		/* just in case, assume Gen3 ITR scale */
+		hw->mac.itr_scale = FM10K_TDLEN_ITR_SCALE_GEN3;
 		break;
 	}

@@ -903,6 +910,13 @@ static s32
 fm10k_iov_assign_default_mac_vlan_pf(struct fm10k_hw *hw,
 	fm10k_write_reg(hw, FM10K_TDBAL(vf_q_idx), tdbal);
 	fm10k_write_reg(hw, FM10K_TDBAH(vf_q_idx), tdbah);
 
+	/* Provide the VF the ITR scale, using software-defined fields in

TDLEN

drivers/net/ethernet/intel/fm10k/fm10k_pf.c +4 -0

+	 * to pass the information during VF initialization. See definition of
+	 * FM10K_TDLEN_ITR_SCALE_SHIFT for more details.
+	 */
+	fm10k_write_reg(hw, FM10K_TDLEN(vf_q_idx), hw->mac.itr_scale

<<
+
FM10K_TDLEN_ITR_SCALE_SHIFT);

drivers/net/ethernet/intel/fm10k/fm10k_pf.c +7 -0

+
 err_out:
 	/* configure Queue control register */
 	txqctl = ((u32)vf_vid << FM10K_TXQCTL_VID_SHIFT) &

@@ -1035,6 +1049,12 @@ static s32 fm10k_iov_reset_resources_pf(struct
 fm10k_hw *hw,
 	for (i = queues_per_pool; i--;) {
 		fm10k_write_reg(hw, FM10K_TDBAL(vf_q_idx + i), tdbal);
 		fm10k_write_reg(hw, FM10K_TDBAH(vf_q_idx + i), tdbah);
+		/* See definition of FM10K_TDLEN_ITR_SCALE_SHIFT for an
+		 * explanation of how TDLEN is used.
+		 */
+		fm10k_write_reg(hw, FM10K_TDLEN(vf_q_idx + i),
+				hw->mac.itr_scale <<
+				FM10K_TDLEN_ITR_SCALE_SHIFT);
 		fm10k_write_reg(hw, FM10K_TQMAP(qmap_idx + i),
 vf_q_idx + i);

fm10k_write_reg(hw, FM10K_RQMAP(qmap_idx + i),
vf_q_idx + i);

drivers/net/ethernet/intel/fm10k/fm10k_pf.c +0 -0

 	}

drivers/net/ethernet/intel/fm10k/fm10k_type.h +2 -0

index 35afd711d144..02d370fb8bbb 100644
@@ -272,6 +272,20 @@ struct fm10k_hw;
 #define FM10K_TDBAL(_n)		((0x40 * (_n)) + 0x8000)
 #define FM10K_TDBAH(_n)		((0x40 * (_n)) + 0x8001)
 #define FM10K_TDLEN(_n)		((0x40 * (_n)) + 0x8002)
+/* When fist initialized, VFs need to know the Interrupt Throttle Rate (ITR)
+ * scale which is based on the PCIe speed but the speed information in the

PCI
+ * configuration space may not be accurate. The PF already knows the ITR scale
+ * but there is no defined method to pass that information from the PF to the
+ * VF. This is accomplished during VF initialization by temporarily co-opting
+ * the yet-to-be-used TDLEN register to have the PF store the ITR shift for
+ * the VF to retrieve before the VF needs to use the TDLEN register for its
+ * intended purpose, i.e. before the Tx resources are allocated.
+ */

drivers/net/ethernet/intel/fm10k/fm10k_type.h +6 -0

+#define FM10K_TDLEN_ITR_SCALE_SHIFT		9
+#define FM10K_TDLEN_ITR_SCALE_MASK		0x00000E00
+#define FM10K_TDLEN_ITR_SCALE_GEN1		2
+#define FM10K_TDLEN_ITR_SCALE_GEN2		1
+#define FM10K_TDLEN_ITR_SCALE_GEN3		0
 #define FM10K_TPH_TXCTRL(_n)	((0x40 * (_n)) + 0x8003)
 #define FM10K_TPH_TXCTRL_DESC_TPHEN		0x00000020
 #define FM10K_TPH_TXCTRL_DESC_RROEN		0x00000200

@@ -560,6 +574,7 @@ struct fm10k_mac_info {
 	bool get_host_state;
 	bool tx_ready;
 	u32 dglort_map;
+	u8 itr_scale;
 };
 
 struct fm10k_swapi_table_info {

drivers/net/ethernet/intel/fm10k/fm10k_vf.c +5 -1

index d512575c33f3..2af697df5abc 100644
@@ -28,7 +28,7 @@
 static s32 fm10k_stop_hw_vf(struct fm10k_hw *hw)
 {
 	u8 *perm_addr = hw->mac.perm_addr;
-	u32 bal = 0, bah = 0;
+	u32 bal = 0, bah = 0, tdlen;
 	s32 err;
 	u16 i;
 

@@ -48,6 +48,9 @@ static s32 fm10k_stop_hw_vf(struct fm10k_hw *hw)
 		       ((u32)perm_addr[2]);
 	}
 
+	/* restore default itr_scale for next VF initialization */
+	tdlen = hw->mac.itr_scale << FM10K_TDLEN_ITR_SCALE_SHIFT;
+
 	/* The queues have already been disabled so we just need to
 	 * update their base address registers
 	 */

@@ -56,6 +59,12 @@ static s32 fm10k_stop_hw_vf(struct fm10k_hw *hw)
 		fm10k_write_reg(hw, FM10K_TDBAH(i), bah);
 		fm10k_write_reg(hw, FM10K_RDBAL(i), bal);
 		fm10k_write_reg(hw, FM10K_RDBAH(i), bah);
+		/* Restore ITR scale in software-defined mechanism in

TDLEN

drivers/net/ethernet/intel/fm10k/fm10k_vf.c +2 -0

+		 * for next VF initialization. See definition of
+		 * FM10K_TDLEN_ITR_SCALE_SHIFT for more details on the

use of

drivers/net/ethernet/intel/fm10k/fm10k_vf.c +6 -1

+		 * TDLEN here.
+		 */
+		fm10k_write_reg(hw, FM10K_TDLEN(i), tdlen);
 	}
 
 	return 0;

@@ -131,9 +140,16 @@ static s32 fm10k_init_hw_vf(struct fm10k_hw *hw)
 	/* record maximum queue count */
 	hw->mac.max_queues = i;
 
-	/* fetch default VLAN */
+	/* fetch default VLAN and ITR scale */
 	hw->mac.default_vid = (fm10k_read_reg(hw, FM10K_TXQCTL(0)) &
 			       FM10K_TXQCTL_VID_MASK) >>
 FM10K_TXQCTL_VID_SHIFT;
+	/* Read the ITR scale from TDLEN. See the definition of
+	 * FM10K_TDLEN_ITR_SCALE_SHIFT for more information about

how TDLEN is

drivers/net/ethernet/intel/fm10k/fm10k_vf.c +5 -1

+	 * used here.
+	 */
+	hw->mac.itr_scale = (fm10k_read_reg(hw, FM10K_TDLEN(0)) &
+			     FM10K_TDLEN_ITR_SCALE_MASK) >>
+			    FM10K_TDLEN_ITR_SCALE_SHIFT;
 
 	return 0;
 
--

2.6.1.264.gbab76a9

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

The existing adaptive ITR algorithm is overly restrictive. It throttles incorrectly for various traffic rates, and does not produce good performance. The algorithm now allows for more interrupts per second, and does some calculation to help improve for smaller packet loads. In addition, take into account the new itr_scale from the hardware which indicates how much to scale due to PCIe link speed.

A single thread of receiving TCP_STREAM in netperf:
- Before: 450 Mbps
- After: 20,000 Mbps

Reported-by Matthew Vick <[email protected]>

Signed-off-by Jacob Keller <[email protected]>

---
  drivers/net/ethernet/intel/fm10k/fm10k.h      |  1 +
  drivers/net/ethernet/intel/fm10k/fm10k_main.c | 29 +++++++++++++++++++++++----
  drivers/net/ethernet/intel/fm10k/fm10k_pci.c  |  6 ++++--
  3 files changed, 30 insertions(+), 6 deletions(-)

drivers/net/ethernet/intel/fm10k/fm10k.h +1 -0

index c40f50737d17..ceddf39d7cec 100644
@@ -164,6 +164,7 @@ struct fm10k_ring_container {
  	unsigned int total_packets;	/* total packets processed this int */
  	u16 work_limit;			/* total work allowed per interrupt */
  	u16 itr;			/* interrupt throttle rate value */
+	u8 itr_scale;			/* ITR adjustment scaler based on PCI speed */
  	u8 count;			/* total number of rings in vector */
  };
 

drivers/net/ethernet/intel/fm10k/fm10k_main.c +9 -3

index babde3e4b2bb..cae6b4e309a9 100644
@@ -1386,11 +1386,30 @@ static void fm10k_update_itr(struct fm10k_ring_container *ring_container)
  	if (avg_wire_size > 3000)
  		avg_wire_size = 3000;
 
-	/* Give a little boost to mid-size frames */
-	if ((avg_wire_size > 300) && (avg_wire_size < 1200))
-		avg_wire_size /= 3;
+	/* Throttle rate management based on average wire size, attempting to
+	 * slightly boost small and medium packet loads. Divide the average
+	 * wire size by a small factor to calculate the minimum time until the
+	 * next interrupt in microseconds. Save some cycles by using a
+	 * multiply then a shift, which also accounts for difference due to
+	 * PCIe link speed.
+	 */
+#define FM10K_ITR_SCALE_SMALL	6
+#define FM10K_ITR_SCALE_MEDIUM	5

drivers/net/ethernet/intel/fm10k/fm10k_main.c +2 -0

  	/* write back value and retain adaptive flag */
  	ring_container->itr = avg_wire_size | FM10K_ITR_ADAPTIVE;

@@ -1608,6 +1627,7 @@ static int fm10k_alloc_q_vector(struct fm10k_intfc *interface,
  	q_vector->tx.ring = ring;
  	q_vector->tx.work_limit = FM10K_DEFAULT_TX_WORK;
  	q_vector->tx.itr = interface->tx_itr;
+	q_vector->tx.itr_scale = interface->hw.mac.itr_scale;
  	q_vector->tx.count = txr_count;
 
  	while (txr_count) {

@@ -1636,6 +1656,7 @@ static int fm10k_alloc_q_vector(struct fm10k_intfc *interface,
  	/* save Rx ring container info */
  	q_vector->rx.ring = ring;
  	q_vector->rx.itr = interface->rx_itr;
+	q_vector->rx.itr_scale = interface->hw.mac.itr_scale;
  	q_vector->rx.count = rxr_count;
 
  	while (rxr_count) {

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4 -2

index 9ad9f9164d91..cbf38da0ada7 100644
@@ -880,7 +880,8 @@ static irqreturn_t fm10k_msix_mbx_vf(int __always_unused irq, void *data)
 
  	/* re-enable mailbox interrupt and indicate 20us delay */
  	fm10k_write_reg(hw, FM10K_VFITR(FM10K_MBX_VECTOR),
-			FM10K_ITR_ENABLE | FM10K_MBX_INT_DELAY);
+			FM10K_ITR_ENABLE | (FM10K_MBX_INT_DELAY >>
+					    hw->mac.itr_scale));
 
  	/* service upstream mailbox */
  	if (fm10k_mbx_trylock(interface)) {

@@ -1111,7 +1112,8 @@ static irqreturn_t fm10k_msix_mbx_pf(int __always_unused irq, void *data)
 
  	/* re-enable mailbox interrupt and indicate 20us delay */
  	fm10k_write_reg(hw, FM10K_ITR(FM10K_MBX_VECTOR),
-			FM10K_ITR_ENABLE | FM10K_MBX_INT_DELAY);
+			FM10K_ITR_ENABLE | (FM10K_MBX_INT_DELAY >>
+					    hw->mac.itr_scale));
 
  	return IRQ_HANDLED;
  }

On 10/13/2015 04:39 PM, Jacob Keller wrote:

(quoted diff) +1 -0

+	 */+#define FM10K_ITR_SCALE_SMALL	6

Quoted 9 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +7 -1

+#define FM10K_ITR_SCALE_MEDIUM	5
+#define FM10K_ITR_SCALE_LARGE	4
+
+	if (avg_wire_size < 300)
+		avg_wire_size *= FM10K_ITR_SCALE_SMALL;
+	else if ((avg_wire_size >= 300) && (avg_wire_size < 1200))
+		avg_wire_size *= FM10K_ITR_SCALE_MEDIUM;
  	else
-		avg_wire_size /= 2;

drivers/net/ethernet/intel/fm10k/fm10k_main.c static void fm10k_update_itr(struct fm10k_ring_container *ring_container) · line 1406

Quoted 2 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +2 -0

+		avg_wire_size *= FM10K_ITR_SCALE_LARGE;
+

Where is it these scaling values originated from? Just looking through
the values I am not sure having this broken out like it is provides much
value.

What I am getting at is that the input is a packet size, and the output
is a value somewhere between 2 and 47. (I think that 47 is still a bit
high by the way and probably should be something more like 25 which I believe you established as the minimum Tx interrupt rate in a later patch.)

What you may want to do is look at pulling in the upper limit to something more reasonable like 1536 for avg_wire_size, and then simplify
this logic a bit. Specifically what is it you are trying to accomplish
by tweaking the scale factor like you are? I assume you are wanting to
approximate a curve. If so you might wan to look at possibly including
an offset value so that you can smooth out the points where your intersections occur.

For example what you may want to consider doing would be to instead use
a multiplication factor for small, an addition value for medium, and for
large you simply cap it at a certain value.

drivers/net/ethernet/intel/fm10k/fm10k_main.c static void fm10k_update_itr(struct fm10k_ring_container *ring_container) · line 1412

Quoted 11 lines

(quoted diff) +1 -0

+	/* Round up average wire size, then perform bit shift, to

ensure that

(quoted diff) +1 -0

+	 * the calculation will never get below 1. Account for

changes in ITR

drivers/net/ethernet/intel/fm10k/fm10k_main.c +3 -0

+	 * value due to PCIe link speed.
+	 */
+	avg_wire_size += (1 << (ring_container->itr_scale + 8)) -

1;

drivers/net/ethernet/intel/fm10k/fm10k_main.c +1 -0

+	avg_wire_size >>= ring_container->itr_scale + 8;

You might want to store off the value for itr_scale + 8 somewhere.
It
is likely you might save a cycle or two, especially if the compiler thinks it has to read itr_scale twice.

Quoted 10 lines · Alexander Duyck

On 10/13/2015 04:39 PM, Jacob Keller wrote:

(quoted diff) +1 -0

+	 */+#define FM10K_ITR_SCALE_SMALL	6

Quoted 8 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +7 -0

+#define FM10K_ITR_SCALE_MEDIUM	5
+#define FM10K_ITR_SCALE_LARGE	4
+
+	if (avg_wire_size < 300)
+		avg_wire_size *= FM10K_ITR_SCALE_SMALL;
+	else if ((avg_wire_size >= 300) && (avg_wire_size < 1200))
+		avg_wire_size *= FM10K_ITR_SCALE_MEDIUM;
   	else

Quoted 9 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c static void fm10k_update_itr(struct fm10k_ring_container *ring_container) · line 1406

Quoted 3 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +2 -1

-		avg_wire_size /= 2;
+		avg_wire_size *= FM10K_ITR_SCALE_LARGE;
+

Where is it these scaling values originated from? Just looking through
the values I am not sure having this broken out like it is provides much
value.

I am not really sure what exactly you mean here?

Quoted 9 lines

What I am getting at is that the input is a packet size, and the output
is a value somewhere between 2 and 47. (I think that 47 is still a bit
high by the way and probably should be something more like 25 which I believe you established as the minimum Tx interrupt rate in a later patch.)

The input is two fold for calculation, packet size, and number of packets.

Quoted 12 lines

What you may want to do is look at pulling in the upper limit to something more reasonable like 1536 for avg_wire_size, and then simplify
this logic a bit. Specifically what is it you are trying to accomplish
by tweaking the scale factor like you are? I assume you are wanting to
approximate a curve. If so you might wan to look at possibly including
an offset value so that you can smooth out the points where your intersections occur.

I honestly don't know. I mostly took the original work from 6-7 months ago, and added your suggestions from that series, but maybe that isn't valid now?

Quoted 6 lines

For example what you may want to consider doing would be to instead use
a multiplication factor for small, an addition value for medium, and for
large you simply cap it at a certain value.

So, how would this look? The capping makes sense, we should probably cap it at around 30 or something? I'm not sure that 25 is the limit, since for some workloads I think we did calculate that we could use that few interrupts.. maybe the CPU savings aren't worth it though if it messes up too many other flows?

I could also try to take a completely different algorithm say from i40e instead? This one really has limited testing.

Quoted 22 lines · Jacob E

Quoted 12 lines · Alexander Duyck

On 10/13/2015 04:39 PM, Jacob Keller wrote:

(quoted diff) +1 -0

+	 */+#define FM10K_ITR_SCALE_SMALL	6

Quoted 10 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +6 -0

+#define FM10K_ITR_SCALE_MEDIUM	5
+#define FM10K_ITR_SCALE_LARGE	4
+
+	if (avg_wire_size < 300)
+		avg_wire_size *= FM10K_ITR_SCALE_SMALL;
+	else if ((avg_wire_size >= 300) && (avg_wire_size <

1200))

drivers/net/ethernet/intel/fm10k/fm10k_main.c +1 -1

+		avg_wire_size *= FM10K_ITR_SCALE_MEDIUM;
   	else
-		avg_wire_size /= 2;

Quoted 9 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c static void fm10k_update_itr(struct fm10k_ring_container *ring_container) · line 1406

Quoted 2 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +2 -0

+		avg_wire_size *= FM10K_ITR_SCALE_LARGE;
+

Where is it these scaling values originated from? Just looking through
the values I am not sure having this broken out like it is provides
much
value.

Quoted 1 line

I am not really sure what exactly you mean here?

The numbers are kind of all over the place. The result of the math above swings back and forth in a saw tooth between values and doesn't seem to do anything really consistent.

For example a packet that is 275 in size will have an interrupt rate of
125K interrupts per second, while a packet that is 276 bytes in size with be closer to 166K. It is basically creating some odd peaks and valleys.

Quoted 12 lines

Quoted 12 lines

What I am getting at is that the input is a packet size, and the output
is a value somewhere between 2 and 47. (I think that 47 is still a
bit
high by the way and probably should be something more like 25 which I
believe you established as the minimum Tx interrupt rate in a later
patch.)

Quoted 2 lines

The input is two fold for calculation, packet size, and number of packets.

Last I knew though the average packet size is the only portion we end up
using to actually set the interrupt moderation rate.

Quoted 13 lines

Quoted 13 lines

What you may want to do is look at pulling in the upper limit to something more reasonable like 1536 for avg_wire_size, and then simplify
this logic a bit. Specifically what is it you are trying to accomplish
by tweaking the scale factor like you are? I assume you are wanting
to
approximate a curve. If so you might wan to look at possibly including
an offset value so that you can smooth out the points where your intersections occur.

Quoted 5 lines

I honestly don't know. I mostly took the original work from 6-7 months
ago, and added your suggestions from that series, but maybe that isn't
valid now?

I suspect some of my views have changed over the last several months after dealing with interrupt moderation issues on ixgbe.

The thing I started realizing is that with full sized Ethernet frames,
1514 bytes in size, you have a maximum rate of something like 4 million
packets per second at 50Gbps. It seems like you should be firing an interrupt once every 100 packets at least don't you think? That is why
I am now thinking at minimum 40K interrupts per second is necessary. However a side effect of that is that 100 packets will overrun a UDP buffer in most cases as there is only room for about 70 frames based on
math I saw when I submitted the patch for ixgbe

(https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/comm
it/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?id=8ac34f10a5ea4c7b6 f57dfd52b0693a2b67d9ac4).
So if I want to take UDP rmem_default into account we would be looking
at something more like 60K interrupts per second which is getting fairly
tiny at 17us

Quoted 8 lines

Quoted 8 lines

For example what you may want to consider doing would be to instead
use
a multiplication factor for small, an addition value for medium, and
for
large you simply cap it at a certain value.

Quoted 8 lines

So, how would this look? The capping makes sense, we should probably
cap it at around 30 or something? I'm not sure that 25 is the limit,
since for some workloads I think we did calculate that we could use that few interrupts.. maybe the CPU savings aren't worth it though if
it messes up too many other flows?

So I have done some math based on an assumption of a 212992 wmem_default
and a overhead of 640 bytes per packet (256 skb, 64 headroom, 320 shared
info). Breaking it all down goes a little something like this:

wmem_default / (size + overhead) = desired_pkts_per_int rate / bits_per_byte / (size + ethernet_overhead) = pkt_rate (desired_pkts_per_int / pkt_rate) * usec_per_sec = ITR value

So then when we start plugging in the numbers:

     212992 / (size + 640) = desired_pkts_per_int

we can simplify the second expression by just doing the division now.

     50,000,000,000 / 8 / (size + 24) = pkt_rate
     6,250,000,000/(size + 24) = pkt_rate

Then we just need to plug in the values and reduce:

     (212,992 / (size + 640)) / (6,250,000,000/(size + 24)) *
1,000,000 
= ITR value
     (212,992/(size + 640)) / (6,250/(size + 24)) = ITR value
     (34.078/(size + 640))/(1/(size+24) = ITR value
     (34 * (size + 24))/(size + 640) = ITR value

So now we have the expression we are looking for in order to determine
the minimum interrupt rate, but we want to avoid the division. So in the end we have something that looks like this in order to generate an
approximation of the curve without having to do any unnecessary math (dropping the +24, and the 3000 limit from earlier):

     /* the following is a crude approximation for the following
expression:
      * (34 * (size + 24))/(size + 640) = ITR value
      * /
     if (avg_wire_size <= 360) {
         /* start 333K ints/sec and gradually drop to 77K ints/sec */
         avg_wire_size *= 8;
         avg_wire_size += 376;
     } else if (avg_wire_size <= 1152) {
         /* 77K ints/sec to 45K ints/sec
         avg_wire_size *= 3;

         avg_wire_size += 2176;
     } else if (avg_wire_size <= 1920) {
         /* 45K ints/sec to 38K ints/sec
         avg_wire_size += 4480;
     } else {
         /* plateau at a limit of 38K ints/sec */
         avg_wire_size = 6656;
     }

Mind you the above is a crude approximation, but it should give decent
performance and it only strays from the values of the original function
by 1us or 2us and it stays under the curve. It could probably use some
tuning and tweaking as well but you get the general idea.

You may even want to tune this to be a bit more aggressive in terms of
interrupts per second. I know some distros such as RHEL are still running around with an untuned skb_shared_info and such and as a result
they take up more space resulting in a larger memory footprint. What this would represent is modifying the 640 value in the original function
to increase based on the extra overhead. Then it would be necessary to
modify the slopes, offsets, and transitions points to get the right approximation for the new curve.

Quoted 3 lines

I could also try to take a completely different algorithm say from i40e
instead? This one really has limited testing.

Yeah, this one was a "good enough" solution at the time and as I recall
it was a clone of the igb interrupt moderation. Now that you have real
ports you probably need something better than 1Gbps.

The i40e one is from ixgbe, which was inherited from e1000. The problem
with the interrupt moderation in that design is that for any high throughput usage everything becomes bulk (8K ints per second). For 1G that works fine. But I can tell you from what I have seen on 10Gbps NICs it doesn't do well under any kind of small packet, or single threaded throughput test.

Quoted 29 lines · Alexander Duyck

Quoted 22 lines · Jacob E

Quoted 12 lines · Alexander Duyck

On 10/13/2015 04:39 PM, Jacob Keller wrote:

(quoted diff) +1 -0

+	 */+#define FM10K_ITR_SCALE_SMALL	6

Quoted 10 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +6 -0

+#define FM10K_ITR_SCALE_MEDIUM	5
+#define FM10K_ITR_SCALE_LARGE	4
+
+	if (avg_wire_size < 300)
+		avg_wire_size *= FM10K_ITR_SCALE_SMALL;
+	else if ((avg_wire_size >= 300) && (avg_wire_size <

1200))

drivers/net/ethernet/intel/fm10k/fm10k_main.c +1 -1

+		avg_wire_size *= FM10K_ITR_SCALE_MEDIUM;
    	else
-		avg_wire_size /= 2;

Quoted 9 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c static void fm10k_update_itr(struct fm10k_ring_container *ring_container) · line 1406

Quoted 2 lines

drivers/net/ethernet/intel/fm10k/fm10k_main.c +2 -0

+		avg_wire_size *= FM10K_ITR_SCALE_LARGE;
+

Where is it these scaling values originated from? Just looking through
the values I am not sure having this broken out like it is provides
much
value.

Quoted 1 line

I am not really sure what exactly you mean here?

The numbers are kind of all over the place. The result of the math above swings back and forth in a saw tooth between values and doesn't seem to do anything really consistent.

Quoted 6 lines

For example a packet that is 275 in size will have an interrupt rate of
125K interrupts per second, while a packet that is 276 bytes in size with be closer to 166K. It is basically creating some odd peaks and valleys.

Yea ok this does do a lot of weird things. I can definitely look at implementing what you suggest below and see how it goes..

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

What I am getting at is that the input is a packet size, and the output
is a value somewhere between 2 and 47. (I think that 47 is still a
bit
high by the way and probably should be something more like 25 which I
believe you established as the minimum Tx interrupt rate in a later
patch.)

Quoted 7 lines

Quoted 2 lines

The input is two fold for calculation, packet size, and number of packets.

Last I knew though the average packet size is the only portion we end up
using to actually set the interrupt moderation rate.

You're correct right now we calculate average wiresize and only use that, no mention of total packet as well.

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

What you may want to do is look at pulling in the upper limit to something more reasonable like 1536 for avg_wire_size, and then simplify
this logic a bit. Specifically what is it you are trying to accomplish
by tweaking the scale factor like you are? I assume you are wanting
to
approximate a curve. If so you might wan to look at possibly including
an offset value so that you can smooth out the points where your intersections occur.

Quoted 9 lines

Quoted 5 lines

I honestly don't know. I mostly took the original work from 6-7 months
ago, and added your suggestions from that series, but maybe that isn't
valid now?

I suspect some of my views have changed over the last several months after dealing with interrupt moderation issues on ixgbe.

Sure. It's a complicated problem.

Quoted 12 lines

The thing I started realizing is that with full sized Ethernet frames,
1514 bytes in size, you have a maximum rate of something like 4 million
packets per second at 50Gbps. It seems like you should be firing an interrupt once every 100 packets at least don't you think? That is why
I am now thinking at minimum 40K interrupts per second is necessary. However a side effect of that is that 100 packets will overrun a UDP buffer in most cases as there is only room for about 70 frames based on
math I saw when I submitted the patch for ixgbe

Quoted 9 lines

(https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/comm
it/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?id=8ac34f10a5ea4c7b6 f57dfd52b0693a2b67d9ac4).
So if I want to take UDP rmem_default into account we would be looking
at something more like 60K interrupts per second which is getting fairly
tiny at 17us

Ok.

Quoted 61 lines

Quoted 8 lines

Quoted 8 lines

For example what you may want to consider doing would be to instead
use
a multiplication factor for small, an addition value for medium, and
for
large you simply cap it at a certain value.

Quoted 8 lines

So, how would this look? The capping makes sense, we should probably
cap it at around 30 or something? I'm not sure that 25 is the limit,
since for some workloads I think we did calculate that we could use that few interrupts.. maybe the CPU savings aren't worth it though if
it messes up too many other flows?

So I have done some math based on an assumption of a 212992 wmem_default
and a overhead of 640 bytes per packet (256 skb, 64 headroom, 320 shared
info). Breaking it all down goes a little something like this:

wmem_default / (size + overhead) = desired_pkts_per_int rate / bits_per_byte / (size + ethernet_overhead) = pkt_rate (desired_pkts_per_int / pkt_rate) * usec_per_sec = ITR value

So then when we start plugging in the numbers:

      212992 / (size + 640) = desired_pkts_per_int

we can simplify the second expression by just doing the division now.

      50,000,000,000 / 8 / (size + 24) = pkt_rate
      6,250,000,000/(size + 24) = pkt_rate

Then we just need to plug in the values and reduce:

      (212,992 / (size + 640)) / (6,250,000,000/(size + 24)) *
1,000,000
= ITR value
      (212,992/(size + 640)) / (6,250/(size + 24)) = ITR value
      (34.078/(size + 640))/(1/(size+24) = ITR value
      (34 * (size + 24))/(size + 640) = ITR value

So now we have the expression we are looking for in order to determine
the minimum interrupt rate, but we want to avoid the division. So in the end we have something that looks like this in order to generate an
approximation of the curve without having to do any unnecessary math (dropping the +24, and the 3000 limit from earlier):

      /* the following is a crude approximation for the following
expression:
       * (34 * (size + 24))/(size + 640) = ITR value
       * /
      if (avg_wire_size <= 360) {
          /* start 333K ints/sec and gradually drop to 77K ints/sec */
          avg_wire_size *= 8;
          avg_wire_size += 376;
      } else if (avg_wire_size <= 1152) {
          /* 77K ints/sec to 45K ints/sec
          avg_wire_size *= 3;

Quoted 9 lines

          avg_wire_size += 2176;
      } else if (avg_wire_size <= 1920) {
          /* 45K ints/sec to 38K ints/sec
          avg_wire_size += 4480;
      } else {
          /* plateau at a limit of 38K ints/sec */
          avg_wire_size = 6656;
      }

So this is calculating the inverse ints/sec? where do we end up converting this to microseconds. Hmm.

Quoted 31 lines

Mind you the above is a crude approximation, but it should give decent
performance and it only strays from the values of the original function
by 1us or 2us and it stays under the curve. It could probably use some
tuning and tweaking as well but you get the general idea.

You may even want to tune this to be a bit more aggressive in terms of
interrupts per second. I know some distros such as RHEL are still running around with an untuned skb_shared_info and such and as a result
they take up more space resulting in a larger memory footprint. What this would represent is modifying the 640 value in the original function
to increase based on the extra overhead. Then it would be necessary to
modify the slopes, offsets, and transitions points to get the right approximation for the new curve.

Quoted 3 lines

I could also try to take a completely different algorithm say from i40e
instead? This one really has limited testing.

Yeah, this one was a "good enough" solution at the time and as I recall
it was a clone of the igb interrupt moderation. Now that you have real
ports you probably need something better than 1Gbps.

Quoted 9 lines

The i40e one is from ixgbe, which was inherited from e1000. The problem
with the interrupt moderation in that design is that for any high throughput usage everything becomes bulk (8K ints per second). For 1G that works fine. But I can tell you from what I have seen on 10Gbps NICs it doesn't do well under any kind of small packet, or single threaded throughput test.

Agreed ok. I will look into this more.

Regards,
Jake

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, October 14, 2015 7:17 PM
To: Keller, Jacob E; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next-queue 13/17] fm10k: Update adaptive ITR algorithm

Quoted 3 lines

So this is calculating the inverse ints/sec? where do we end up converting this to microseconds. Hmm.

The value generated at the end is in microseconds. Basically this would replace all the code that is located before your shift at the end that uses the PCIe factor + 8. So everything from the size > 3000 check down to the adjustments for the other sizes.

I kind of always hated the algorithms that try to work everything out in terms of interrupts and then have to flip it over and into microseconds.
This is just working with the raw microseconds like the fm10k driver
currently does.

The current default ITR for Tx is overly restrictive. Using a simple netperf TCP_STREAM test, we top out at about 10Gb/s for a single thread when running using 1500 byte frames. By reducing the ITR value to 25usec (up to 40K interrupts a second from 10K), we are able to achieve 36Gb/s for a single thread TCP stream test.

Signed-off-by Jacob Keller <[email protected]>

---
  drivers/net/ethernet/intel/fm10k/fm10k.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/ethernet/intel/fm10k/fm10k.h +0 -0

index bf2cc6f78418..5eb42fd21d3f 100644
@@ -171,11 +171,12 @@ struct fm10k_ring_container {
  #define FM10K_ITR_MAX		0x0FFF	/* maximum value for ITR */
  #define FM10K_ITR_10K		100	/* 100us */

Even 40K might be a bit restrictive. I've found that 10Gb NICs need about 12K interrupts per second to avoid depleting the socket buffers in
the case of a UDP test. You might want to see if you see performance gains by taking it up to 50K maybe even 60K. As far as test cases to use for tuning it would be useful to use a UDP_STREAM test instead of TCP_STREAM since UDP is not bound by the link partner throughput.

- Alex

Quoted 8 lines · Alexander Duyck

Even 40K might be a bit restrictive. I've found that 10Gb NICs need about 12K interrupts per second to avoid depleting the socket buffers in
the case of a UDP test. You might want to see if you see performance gains by taking it up to 50K maybe even 60K. As far as test cases to use for tuning it would be useful to use a UDP_STREAM test instead of TCP_STREAM since UDP is not bound by the link partner throughput.

Quoted 1 line

- Alex

Thanks Alex. I tried with Tx set to 0usec up to 25, and for my tests with both TCP and UDP trials, I topped out at 25 and didn't see improvement beyond that. I think it's a reasonable default, but I likely have other areas on my system that need tuning first.

I can give some more tests specifically with UDP again to see if any other changes made that not true.

The 36Gb/s number is pretty impressive and may be pushing you into other
limits. What does the CPU utilization look like for your test? Do you
see the thread handling the workload get fully utilized? Also have you
tried enabling XPS to see if maybe you could squeeze a bit more out of
the core that is doing the test by improving locality?

- Alex

The 36Gb/s number is pretty impressive and may be pushing you into other
limits. What does the CPU utilization look like for your test? Do you
see the thread handling the workload get fully utilized? Also have you
tried enabling XPS to see if maybe you could squeeze a bit more out of
the core that is doing the test by improving locality?

- Alex

Quoted 11 lines · Alexander Duyck

The 36Gb/s number is pretty impressive and may be pushing you into other
limits. What does the CPU utilization look like for your test? Do you
see the thread handling the workload get fully utilized? Also have you
tried enabling XPS to see if maybe you could squeeze a bit more out of
the core that is doing the test by improving locality?

- Alex

Hello,

So what I am getting now with a few things. First, I set rx-usecs and tx-usecs to both 25, and disabled adaptive mode for now...

TCP_STREAM => 33Gb/s, but sometimes drops to 27Gb/s, I believe this is a result of no rx_flow_steering (Flow director / ATR) support in the host interface. So sometimes the queue we pick is more local other times it is not. If I reduce the number of queues and ensure only CPUs local to the one I am running netperf, I get 33Gb/s pretty straight. I have not been able to reproduce the 36Gb/s test above, so I may re-word the commit message...

I used:

./netperf -T0,5 -t TCP_STREAM -f m -c -C -H 192.168.21.2 -- -m 64K

So this is with rather large messages that can be run through TSO.

CPU utilization here tops says: 10% local, 10% remote, but if I look at top on both ends, it shows ~80% CPU utilization on receiver, and about 50% on the transmit end.

I've got a weird issue right now where sometimes it seems to drop to half and I haven't determined exactly why yet. But I am pretty sure it's due to queue assignment

I've been getting pretty inconsistent performance results over the last few tests.

I tried these tests with interrupt moderation disabled completely and I generally got less performance.

Interestingly, I just set both rx and tx to 10, and got one test through to report 39Gb/s... But I am definitely not able to consistently hit this value.

I generally seem to range pretty wide over tests.

For UDP I used:

./netperf -T0,5 -t UDP_STREAM -f m -c -C -H 192.168.21.2 -- -m 64k

For this test, I see 80% CPU utilization on the sender, and 50% on the receiver, when bound as above.

I seem to get ~16Gb/s send and receive here, with no variance...

I suspect part of this is due to the fact that TCP can do hardware TSO, which we don't have in UDP? I'm not sure here..

UDP is significantly more stable than TCP was. but it doesn't seem to ever go above 16Gb/s for a single stream.

I'm still a bit concerned over the instability produced by TCP_STREAM, but it should be noted that my test setup is far from ideal:

I currently only have a single host interface, and have used network namespacing to separate the two devices so that it routes over the physical hardware. So it's a single system test which impacts irq to CPU binding, as well as queue to CPU binding, and so on. There are a lot of issues here that impact, but I'm happy to be able to get much better than 2-3Gb/s like I was before.

Any further suggestions would be appreciated.

Regards,
Jake

Sounds reasonable. With TCP loss can also play a huge factor, although
I would assume you probably have no dropped packets correct?

Quoted 4 lines

I've been getting pretty inconsistent performance results over the last
few tests.

Quoted 3 lines

I tried these tests with interrupt moderation disabled completely and I
generally got less performance.

Completely disabling it will usually do that. The problem is the rates
for 50Gbs are insane. You are looking at 4Mpps even with 1514 byte packets.

Quoted 4 lines

Interestingly, I just set both rx and tx to 10, and got one test through to report 39Gb/s... But I am definitely not able to consistently hit this value.

The 10us range should be excessive. I would expect you would see the best performance right around the amount of time it should take to almost fill the ring or socket buffers without actually ever filling them. Basically it is a game of get as close as you can without going
over in order to get the fewest interrupts possible.

Quoted 1 line

I generally seem to range pretty wide over tests.

CPU affinity along with everything else can always make these kind of tests pretty messy. I'm assuming you have power management also disabled? If not that could also cause some pretty wide swings due to
processor C states and P states.

Quoted 9 lines

For UDP I used:

./netperf -T0,5 -t UDP_STREAM -f m -c -C -H 192.168.21.2 -- -m 64k

For this test, I see 80% CPU utilization on the sender, and 50% on the
receiver, when bound as above.

I seem to get ~16Gb/s send and receive here, with no variance...

The fact that there is no variance likely means something is bottlenecking this somewhere early on in the Tx.

Quoted 3 lines

I suspect part of this is due to the fact that TCP can do hardware TSO,
which we don't have in UDP? I'm not sure here..

TCP will also allow you to have significantly more data in flight in many cases. UDP is normally confined to a fairly small window.

Quoted 3 lines

UDP is significantly more stable than TCP was. but it doesn't seem to
ever go above 16Gb/s for a single stream.

I'd be interested in seeing the actual numbers. I know for some UDP_STREAM tests I have run it ends up being that one side is transmitting a significant amount, while the receiving side is only getting a fraction of it because packets are being dropped due to overrunning the socket.

Quoted 3 lines

I'm still a bit concerned over the instability produced by TCP_STREAM,
but it should be noted that my test setup is far from ideal:

Agreed.

Quoted 11 lines

I currently only have a single host interface, and have used network
namespacing to separate the two devices so that it routes over the physical hardware. So it's a single system test which impacts irq to
CPU binding, as well as queue to CPU binding, and so on. There are a
lot of issues here that impact, but I'm happy to be able to get much
better than 2-3Gb/s like I was before.

Quoted 4 lines

Any further suggestions would be appreciated.

Regards,
Jake

The only other thing I can think of is to check flow control, but as I
recall that is disabled by default with fm10k.

- Alex

Quoted 4 lines · Alexander Duyck

Sounds reasonable. With TCP loss can also play a huge factor, although
I would assume you probably have no dropped packets correct?

No drops for UDP, but with TCP I see drops on the receiving partner... no more than about 200 total though.

Quoted 4 lines

Quoted 4 lines

I've been getting pretty inconsistent performance results over the last
few tests.

Quoted 8 lines

Quoted 3 lines

I tried these tests with interrupt moderation disabled completely and I
generally got less performance.

Completely disabling it will usually do that. The problem is the rates
for 50Gbs are insane. You are looking at 4Mpps even with 1514 byte packets.

Ok that makes sense, yea. Too much wakeup causes us to waste a lot.

Quoted 5 lines

Quoted 4 lines

Interestingly, I just set both rx and tx to 10, and got one test through to report 39Gb/s... But I am definitely not able to consistently hit this value.

Quoted 7 lines

The 10us range should be excessive. I would expect you would see the best performance right around the amount of time it should take to almost fill the ring or socket buffers without actually ever filling them. Basically it is a game of get as close as you can without going
over in order to get the fewest interrupts possible.

I am not sure what is best, but 10 so far as been I will try a few others...

Quoted 21 lines

Quoted 1 line

I generally seem to range pretty wide over tests.

CPU affinity along with everything else can always make these kind of tests pretty messy. I'm assuming you have power management also disabled? If not that could also cause some pretty wide swings due to
processor C states and P states.

Quoted 9 lines

For UDP I used:

./netperf -T0,5 -t UDP_STREAM -f m -c -C -H 192.168.21.2 -- -m 64k

For this test, I see 80% CPU utilization on the sender, and 50% on the
receiver, when bound as above.

I seem to get ~16Gb/s send and receive here, with no variance...

The fact that there is no variance likely means something is bottlenecking this somewhere early on in the Tx.

Quoted 7 lines

Quoted 3 lines

I suspect part of this is due to the fact that TCP can do hardware TSO,
which we don't have in UDP? I'm not sure here..

TCP will also allow you to have significantly more data in flight in many cases. UDP is normally confined to a fairly small window.

Makes sense.

Quoted 10 lines

Quoted 3 lines

UDP is significantly more stable than TCP was. but it doesn't seem to
ever go above 16Gb/s for a single stream.

I'd be interested in seeing the actual numbers. I know for some UDP_STREAM tests I have run it ends up being that one side is transmitting a significant amount, while the receiving side is only getting a fraction of it because packets are being dropped due to overrunning the socket.

According to netperf, it doesn't have any dropped packets doing UDP, ethtool agrees:

MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.21.2 () port 0 AF_INET : cpu bind

Socket  Message  Elapsed      Messages                   CPU      Service
Size    Size     Time         Okay Errors   Throughput   Util     Demand
bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB

212992   64000   10.00      319414      0    16349.8     9.57     0.959
212992           10.00      319407           16349.4     9.57     0.959

So this looks quite low comapred to TCP, but it has no variance.

Quoted 6 lines

Quoted 3 lines

I'm still a bit concerned over the instability produced by TCP_STREAM,
but it should be noted that my test setup is far from ideal:

Agreed.

I can't really get a better one at present because we don't have hardware with multiple host interfaces on different boxes that is available to me for long term usage for test case here..

Quoted 21 lines

Quoted 11 lines

I currently only have a single host interface, and have used network
namespacing to separate the two devices so that it routes over the physical hardware. So it's a single system test which impacts irq to
CPU binding, as well as queue to CPU binding, and so on. There are a
lot of issues here that impact, but I'm happy to be able to get much
better than 2-3Gb/s like I was before.

Quoted 4 lines

Any further suggestions would be appreciated.

Regards,
Jake

The only other thing I can think of is to check flow control, but as I
recall that is disabled by default with fm10k.

Quoted 2 lines

- Alex

There is no hardware ethernet flow control at all for the fm10k interface.

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, October 14, 2015 7:24 PM
To: Keller, Jacob E; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next-queue 15/17] fm10k: change default Tx ITR to 25usec

Quoted 2 lines

There is no hardware ethernet flow control at all for the fm10k interface.

There is a flow control mechanism in the form of DMA back pressure that can be exerted by the PEP interfaces. It is controlled via the ethtool flow control interface. I believe it is defaulted to off though since that is what is in the current upstream driver.

- Alex

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, October 13, 2015 4:39 PM
To: Intel Wired LAN
Subject: [Intel-wired-lan] [next-queue 01/17] fm10k: conditionally compile DCB and DebugFS support

Rather than wrapping fm10k_dcbnl.c and fm10k_debugfs.c support with
#ifdef blocks, just conditionally include the .o files in the Makefile.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/Makefile        | 20 +++++++++++++++----

(quoted diff) +0 -1

-
 drivers/net/ethernet/intel/fm10k/fm10k.h         |  2 ++
 drivers/net/ethernet/intel/fm10k/fm10k_dcbnl.c   |  4 ----
 drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c |  4 ----
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c     |  4 ++++
 5 files changed, 21 insertions(+), 13 deletions(-)

drivers/net/ethernet/intel/fm10k/Makefile +0 -0

index 08859dd220a8..b006ff66d028 100644

drivers/net/ethernet/intel/fm10k/Makefile +0 -0

 #

# This program is free software; you can redistribute it and/or modify it

 # under the terms and conditions of the GNU General Public License,

drivers/net/ethernet/intel/fm10k/Makefile +14 -4

@@ -27,7 +27,17 @@
 
 obj-$(CONFIG_FM10K) += fm10k.o
 
-fm10k-objs := fm10k_main.o fm10k_common.o fm10k_pci.o \
-	      fm10k_netdev.o fm10k_ethtool.o fm10k_pf.o fm10k_vf.o \
-	      fm10k_mbx.o fm10k_iov.o fm10k_tlv.o \
-	      fm10k_debugfs.o fm10k_ptp.o fm10k_dcbnl.o
+fm10k-y := fm10k_main.o \
+	   fm10k_common.o \
+	   fm10k_pci.o \
+	   fm10k_ptp.o \
+	   fm10k_netdev.o \
+	   fm10k_ethtool.o \
+	   fm10k_pf.o \
+	   fm10k_vf.o \
+	   fm10k_mbx.o \
+	   fm10k_iov.o \
+	   fm10k_tlv.o
+
+fm10k-$(CONFIG_DEBUG_FS) += fm10k_debugfs.o
+fm10k-$(CONFIG_DCB) += fm10k_dcbnl.o

drivers/net/ethernet/intel/fm10k/fm10k.h +0 -0

index 48809e5d3f79..504d487fa800 100644

#endif /* _FM10K_H_ */

drivers/net/ethernet/intel/fm10k/fm10k_dcbnl.c +0 -2

index 5c7a4d7662d8..2be4361839db 100644
@@ -20,7 +20,6 @@
 
 #include "fm10k.h"
 
-#ifdef CONFIG_DCB
 /**
  * fm10k_dcbnl_ieee_getets - get the ETS configuration for the device
  * @dev: netdev interface for the device

@@ -155,7 +154,6 @@ static const struct dcbnl_rtnl_ops fm10k_dcbnl_ops
 = {
 	.setdcbx	= fm10k_dcbnl_setdcbx,
 };
 
-#endif /* CONFIG_DCB */
 /**
  * fm10k_dcbnl_set_ops - Configures dcbnl ops pointer for netdev

* @dev: netdev interface for the device

drivers/net/ethernet/intel/fm10k/fm10k_dcbnl.c +0 -2

@@ -164,11 +162,9 @@ static const struct dcbnl_rtnl_ops fm10k_dcbnl_ops
 = {
  **/
 void fm10k_dcbnl_set_ops(struct net_device *dev)
 {
-#ifdef CONFIG_DCB
 	struct fm10k_intfc *interface = netdev_priv(dev);
 	struct fm10k_hw *hw = &interface->hw;
 
 	if (hw->mac.type == fm10k_mac_pf)
 		dev->dcbnl_ops = &fm10k_dcbnl_ops;
-#endif /* CONFIG_DCB */

}

drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c +0 -0

index 5304bc1fbecd..5d6137faf7d1 100644
@@ -18,8 +18,6 @@
  * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-

6497
*/

drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c +0 -4

-#ifdef CONFIG_DEBUG_FS
-
 #include "fm10k.h"
 
 #include <linux/debugfs.h>

@@ -258,5 +256,3 @@ void fm10k_dbg_exit(void)
 	debugfs_remove_recursive(dbg_root);
 	dbg_root = NULL;
 }
-
-#endif /* CONFIG_DEBUG_FS */

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0 -0

index 5fbffbaefe32..0dece3c07727 100644

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0 -1

 	err = fm10k_init_queueing_scheme(interface);
 	if (err)
--

2.6.1.264.gbab76a9

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan