[net-next 00/16][pull request] Intel Wired LAN Driver Updates 2015-02-05

[net-next 00/16][pull request] Intel Wired LAN Driver Updates 2015-02-05

[Jeff Kirsher]

This series contains updates to fm10k, ixgbe and ixgbevf.

Matthew fixes an issue where fm10k does not properly drop the upper-most four
bits on of the VLAN ID due to type promotion, so resolve the issue by not
masking off the bits, but by throwing an error if the VLAN ID is out-of-bounds.
Then cleans up two cases where variables were not being used, but were
being set, so just remove the unused variables.

Don cleans up sparse errors in the x550 family file for ixgbe.  Fixed up
a redundant setting of the default value for set_rxpba, which was done
twice accidentally.  Cleaned up the probe routine to remove a redundant
attempt to identify the PHY, which could lead to a panic on x550.  Added
support for VXLAN receive checksum offload in x550 hardware.  Added the
Ethertype Anti-spoofing feature for affected devices.

Emil enables ixgbe and ixgbevf to allow multiple queues in SRIOV mode.
Adds RSS support for x550 per VF.  Fixed up a couple of issues introduced
in commit 2b509c0cd292 ("ixgbe: cleanup ixgbe_ndo_set_vf_vlan"), fixed
setting of the VLAN inside ixgbe_enable_port_vlan() and disable the
"hide VLAN" bit in PFQDE when port VLAN is disabled.  Cleaned up the
setting of vlan_features by enabling all features at once.  Fixed the
ordering of the shutdown patch so that we attempt to shutdown the rings
more gracefully.  We shutdown the main Rx filter in the case of Rx and we
set the carrier_off state in the case of Tx so that packets stop being
delivered from outside the driver.  Then we shutdown interrupts and NAPI,
then finally stop the rings from performing DMA and clean them.  Added
code to allow for Tx hang checking to provide more robust debug info in
the event of a transmit unit hang in ixgbevf.  Cleaned up ixgbevf logic
dealing with link up/down by breaking down the link detection and up/down
events into separate functions, similar to how these events are handled
in other drivers.  Combined the ixgbevf reset and watchdog tasks into a
single task so that we can avoid multiple schedules of the reset task when
we have a reset event needed due to either the mailbox going down or
transmit packets being present on a link down.

The following are changes since commit db79a621835ee91d3e10177abd97f48e0a4dcf9b:
  vxlan: Only set has-GBP bit in header if any other bits would be set
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Don Skidmore (5):
  ixgbe: cleanup sparse errors in new ixgbe_x550.c file
  ixgbe: cleanup redundant default method set_rxpba
  ixgbe: Cleanup probe to remove redundant attempt to ID PHY
  ixgbe: add VXLAN offload support for X550 devices
  ixgbe: add Tx anti spoofing support

Emil Tantilov (9):
  ixgbe: allow multiple queues in SRIOV mode
  ixgbevf: enable multiple queue support
  ixgbevf: add RSS support for X550
  ixgbe: fix setting port VLAN
  ixgbevf: set vlan_features in a single write instead of several ORs
  ixgbevf: Fix ordering of shutdown to correctly disable Rx and Tx
  ixgbevf: Add code to check for Tx hang
  ixgbevf: rewrite watchdog task to function similar to igbvf
  ixgbevf: combine all of the tasks into a single service task

Matthew Vick (2):
  fm10k: Validate VLAN ID in fm10k_update_xc_addr_pf
  fm10k: Resolve compile warnings with W=1

 drivers/net/ethernet/intel/Kconfig                |  11 +
 drivers/net/ethernet/intel/fm10k/fm10k_mbx.c      |   5 +-
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c       |   7 +-
 drivers/net/ethernet/intel/fm10k/fm10k_ptp.c      |   3 -
 drivers/net/ethernet/intel/ixgbe/ixgbe.h          |   3 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     | 112 ++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  16 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h     |  16 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c     |   3 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c     |  90 ++--
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  36 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 495 +++++++++++++++-------
 drivers/net/ethernet/intel/ixgbevf/regs.h         |  10 +
 13 files changed, 586 insertions(+), 221 deletions(-)

-- 
1.9.3

[net-next 01/16] fm10k: Validate VLAN ID in fm10k_update_xc_addr_pf

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Currently, fm10k_update_xc_addr_pf has an issue where it does not
properly drop the upper-most four bits of the VLAN ID due to type
promotion. Resolve the issue not by masking off the bits, but by
throwing an error if the VLAN ID is out-of-bounds.

Reported-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
index 275423d..7e47119 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
@@ -330,13 +330,10 @@ static s32 fm10k_update_xc_addr_pf(struct fm10k_hw *hw, u16 glort,
 	struct fm10k_mac_update mac_update;
 	u32 msg[5];
 
-	/* if glort is not valid return error */
-	if (!fm10k_glort_valid_pf(hw, glort))
+	/* if glort or vlan are not valid return error */
+	if (!fm10k_glort_valid_pf(hw, glort) || vid >= FM10K_VLAN_TABLE_VID_MAX)
 		return FM10K_ERR_PARAM;
 
-	/* drop upper 4 bits of VLAN ID */
-	vid = (vid << 4) >> 4;
-
 	/* record fields */
 	mac_update.mac_lower = cpu_to_le32(((u32)mac[2] << 24) |
 						 ((u32)mac[3] << 16) |
-- 
1.9.3

[net-next 02/16] fm10k: Resolve compile warnings with W=1

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Fix two cases where variables are being set but not used.

Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Krishneil Singh<Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_mbx.c | 5 ++---
 drivers/net/ethernet/intel/fm10k/fm10k_ptp.c | 3 ---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
index 14a4ea7..9f5457c 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
@@ -1194,12 +1194,11 @@ static s32 fm10k_mbx_process_disconnect(struct fm10k_hw *hw,
 {
 	const enum fm10k_mbx_state state = mbx->state;
 	const u32 *hdr = &mbx->mbx_hdr;
-	u16 head, tail;
+	u16 head;
 	s32 err;
 
-	/* we will need to pull all of the fields for verification */
+	/* we will need to pull the header field for verification */
 	head = FM10K_MSG_HDR_FIELD_GET(*hdr, HEAD);
-	tail = FM10K_MSG_HDR_FIELD_GET(*hdr, TAIL);
 
 	/* We should not be receiving disconnect if Rx is incomplete */
 	if (mbx->pushed)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
index 7822809..d966044 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c
@@ -57,7 +57,6 @@ void fm10k_ts_tx_enqueue(struct fm10k_intfc *interface, struct sk_buff *skb)
 	struct sk_buff_head *list = &interface->ts_tx_skb_queue;
 	struct sk_buff *clone;
 	unsigned long flags;
-	__le16 dglort;
 
 	/* create clone for us to return on the Tx path */
 	clone = skb_clone_sk(skb);
@@ -65,8 +64,6 @@ void fm10k_ts_tx_enqueue(struct fm10k_intfc *interface, struct sk_buff *skb)
 		return;
 
 	FM10K_CB(clone)->ts_tx_timeout = jiffies + FM10K_TS_TX_TIMEOUT;
-	dglort = FM10K_CB(clone)->fi.w.dglort;
-
 	spin_lock_irqsave(&list->lock, flags);
 
 	/* attempt to locate any buffers with the same dglort,
-- 
1.9.3

[net-next 03/16] ixgbe: cleanup sparse errors in new ixgbe_x550.c file

[Jeff Kirsher]

From: Don Skidmore <donald.c.skidmore@intel.com>

This patch cleans up prototypes that should have been defined
as static.

Signed-off-by: Don Skidmore <donald.c.skidmore@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/ixgbe/ixgbe_type.h |  4 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 65 ++++++++++++++-------------
 2 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index d101b25..1f301ec 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -2451,8 +2451,8 @@ struct ixgbe_hic_read_shadow_ram {
 
 struct ixgbe_hic_write_shadow_ram {
 	union ixgbe_hic_hdr2 hdr;
-	u32 address;
-	u16 length;
+	__be32 address;
+	__be16 length;
 	u16 pad2;
 	u16 data;
 	u16 pad3;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index ffdd123..fcba952 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -80,7 +80,7 @@ static s32 ixgbe_write_phy_reg_x550em(struct ixgbe_hw *hw, u32 reg_addr,
  *  Initializes the EEPROM parameters ixgbe_eeprom_info within the
  *  ixgbe_hw struct in order to set up EEPROM access.
  **/
-s32 ixgbe_init_eeprom_params_X550(struct ixgbe_hw *hw)
+static s32 ixgbe_init_eeprom_params_X550(struct ixgbe_hw *hw)
 {
 	struct ixgbe_eeprom_info *eeprom = &hw->eeprom;
 	u32 eec;
@@ -110,8 +110,8 @@ s32 ixgbe_init_eeprom_params_X550(struct ixgbe_hw *hw)
  *  @device_type: 3 bit device type
  *  @phy_data: Pointer to read data from the register
  **/
-s32 ixgbe_read_iosf_sb_reg_x550(struct ixgbe_hw *hw, u32 reg_addr,
-				u32 device_type, u32 *data)
+static s32 ixgbe_read_iosf_sb_reg_x550(struct ixgbe_hw *hw, u32 reg_addr,
+				       u32 device_type, u32 *data)
 {
 	u32 i, command, error;
 
@@ -158,7 +158,8 @@ s32 ixgbe_read_iosf_sb_reg_x550(struct ixgbe_hw *hw, u32 reg_addr,
  *
  *  Reads a 16 bit word from the EEPROM using the hostif.
  **/
-s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)
+static s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset,
+					  u16 *data)
 {
 	s32 status;
 	struct ixgbe_hic_read_shadow_ram buffer;
@@ -193,8 +194,8 @@ s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)
  *
  *  Reads a 16 bit word(s) from the EEPROM using the hostif.
  **/
-s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
-				     u16 offset, u16 words, u16 *data)
+static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
+					    u16 offset, u16 words, u16 *data)
 {
 	struct ixgbe_hic_read_shadow_ram buffer;
 	u32 current_word = 0;
@@ -331,7 +332,8 @@ static s32 ixgbe_checksum_ptr_x550(struct ixgbe_hw *hw, u16 ptr,
  *
  *  Returns a negative error code on error, or the 16-bit checksum
  **/
-s32 ixgbe_calc_checksum_X550(struct ixgbe_hw *hw, u16 *buffer, u32 buffer_size)
+static s32 ixgbe_calc_checksum_X550(struct ixgbe_hw *hw, u16 *buffer,
+				    u32 buffer_size)
 {
 	u16 eeprom_ptrs[IXGBE_EEPROM_LAST_WORD + 1];
 	u16 *local_buffer;
@@ -407,7 +409,7 @@ s32 ixgbe_calc_checksum_X550(struct ixgbe_hw *hw, u16 *buffer, u32 buffer_size)
  *
  *  Returns a negative error code on error, or the 16-bit checksum
  **/
-s32 ixgbe_calc_eeprom_checksum_X550(struct ixgbe_hw *hw)
+static s32 ixgbe_calc_eeprom_checksum_X550(struct ixgbe_hw *hw)
 {
 	return ixgbe_calc_checksum_X550(hw, NULL, 0);
 }
@@ -419,7 +421,7 @@ s32 ixgbe_calc_eeprom_checksum_X550(struct ixgbe_hw *hw)
  *
  *   Reads a 16 bit word from the EEPROM using the hostif.
  **/
-s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)
+static s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)
 {
 	s32 status = 0;
 
@@ -440,7 +442,8 @@ s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)
  *  Performs checksum calculation and validates the EEPROM checksum.  If the
  *  caller does not need checksum_val, the value can be NULL.
  **/
-s32 ixgbe_validate_eeprom_checksum_X550(struct ixgbe_hw *hw, u16 *checksum_val)
+static s32 ixgbe_validate_eeprom_checksum_X550(struct ixgbe_hw *hw,
+					       u16 *checksum_val)
 {
 	s32 status;
 	u16 checksum;
@@ -489,7 +492,8 @@ s32 ixgbe_validate_eeprom_checksum_X550(struct ixgbe_hw *hw, u16 *checksum_val)
  *
  *  Write a 16 bit word to the EEPROM using the hostif.
  **/
-s32 ixgbe_write_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, u16 data)
+static s32 ixgbe_write_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset,
+					   u16 data)
 {
 	s32 status;
 	struct ixgbe_hic_write_shadow_ram buffer;
@@ -517,7 +521,7 @@ s32 ixgbe_write_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, u16 data)
  *
  *  Write a 16 bit word to the EEPROM using the hostif.
  **/
-s32 ixgbe_write_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 data)
+static s32 ixgbe_write_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 data)
 {
 	s32 status = 0;
 
@@ -537,7 +541,7 @@ s32 ixgbe_write_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 data)
  *
  *  Issue a shadow RAM dump to FW to copy EEPROM from shadow RAM to the flash.
  **/
-s32 ixgbe_update_flash_X550(struct ixgbe_hw *hw)
+static s32 ixgbe_update_flash_X550(struct ixgbe_hw *hw)
 {
 	s32 status = 0;
 	union ixgbe_hic_hdr2 buffer;
@@ -560,7 +564,7 @@ s32 ixgbe_update_flash_X550(struct ixgbe_hw *hw)
  *  checksum and updates the EEPROM and instructs the hardware to update
  *  the flash.
  **/
-s32 ixgbe_update_eeprom_checksum_X550(struct ixgbe_hw *hw)
+static s32 ixgbe_update_eeprom_checksum_X550(struct ixgbe_hw *hw)
 {
 	s32 status;
 	u16 checksum = 0;
@@ -600,8 +604,9 @@ s32 ixgbe_update_eeprom_checksum_X550(struct ixgbe_hw *hw)
  *
  *  Write a 16 bit word(s) to the EEPROM using the hostif.
  **/
-s32 ixgbe_write_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
-				      u16 offset, u16 words, u16 *data)
+static s32 ixgbe_write_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
+					     u16 offset, u16 words,
+					     u16 *data)
 {
 	s32 status = 0;
 	u32 i = 0;
@@ -630,7 +635,7 @@ s32 ixgbe_write_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
 /** ixgbe_init_mac_link_ops_X550em - init mac link function pointers
  *  @hw: pointer to hardware structure
  **/
-void ixgbe_init_mac_link_ops_X550em(struct ixgbe_hw *hw)
+static void ixgbe_init_mac_link_ops_X550em(struct ixgbe_hw *hw)
 {
 	struct ixgbe_mac_info *mac = &hw->mac;
 
@@ -647,7 +652,7 @@ void ixgbe_init_mac_link_ops_X550em(struct ixgbe_hw *hw)
 /** ixgbe_setup_sfp_modules_X550em - Setup SFP module
  * @hw: pointer to hardware structure
  */
-s32 ixgbe_setup_sfp_modules_X550em(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_sfp_modules_X550em(struct ixgbe_hw *hw)
 {
 	bool setup_linear;
 	u16 reg_slice, edc_mode;
@@ -703,9 +708,9 @@ s32 ixgbe_setup_sfp_modules_X550em(struct ixgbe_hw *hw)
  * @speed: pointer to link speed
  * @autoneg: true when autoneg or autotry is enabled
  **/
-s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
-				       ixgbe_link_speed *speed,
-				       bool *autoneg)
+static s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
+					      ixgbe_link_speed *speed,
+					      bool *autoneg)
 {
 	/* SFP */
 	if (hw->phy.media_type == ixgbe_media_type_fiber) {
@@ -740,8 +745,8 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
  *  @device_type: 3 bit device type
  *  @data: Data to write to the register
  **/
-s32 ixgbe_write_iosf_sb_reg_x550(struct ixgbe_hw *hw, u32 reg_addr,
-				 u32 device_type, u32 data)
+static s32 ixgbe_write_iosf_sb_reg_x550(struct ixgbe_hw *hw, u32 reg_addr,
+					u32 device_type, u32 data)
 {
 	u32 i, command, error;
 
@@ -904,7 +909,7 @@ static s32 ixgbe_setup_ixfi_x550em(struct ixgbe_hw *hw, ixgbe_link_speed *speed)
  *
  *   Configures the integrated KX4 PHY.
  **/
-s32 ixgbe_setup_kx4_x550em(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_kx4_x550em(struct ixgbe_hw *hw)
 {
 	s32 status;
 	u32 reg_val;
@@ -942,7 +947,7 @@ s32 ixgbe_setup_kx4_x550em(struct ixgbe_hw *hw)
  *
  *   Configures the integrated KR PHY.
  **/
-s32 ixgbe_setup_kr_x550em(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_kr_x550em(struct ixgbe_hw *hw)
 {
 	s32 status;
 	u32 reg_val;
@@ -987,7 +992,7 @@ s32 ixgbe_setup_kr_x550em(struct ixgbe_hw *hw)
  *  A return of a non-zero value indicates an error, and the base driver should
  *  not report link up.
  **/
-s32 ixgbe_setup_internal_phy_x550em(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_internal_phy_x550em(struct ixgbe_hw *hw)
 {
 	u32 status;
 	u16 lasi, autoneg_status, speed;
@@ -1049,7 +1054,7 @@ s32 ixgbe_setup_internal_phy_x550em(struct ixgbe_hw *hw)
  *  set during init_shared_code because the PHY/SFP type was
  *  not known.  Perform the SFP init if necessary.
  **/
-s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
+static s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
 {
 	struct ixgbe_phy_info *phy = &hw->phy;
 	s32 ret_val;
@@ -1102,7 +1107,7 @@ s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
  *  Returns the media type (fiber, copper, backplane)
  *
  */
-enum ixgbe_media_type ixgbe_get_media_type_X550em(struct ixgbe_hw *hw)
+static enum ixgbe_media_type ixgbe_get_media_type_X550em(struct ixgbe_hw *hw)
 {
 	enum ixgbe_media_type media_type;
 
@@ -1129,7 +1134,7 @@ enum ixgbe_media_type ixgbe_get_media_type_X550em(struct ixgbe_hw *hw)
 /** ixgbe_init_ext_t_x550em - Start (unstall) the external Base T PHY.
  ** @hw: pointer to hardware structure
  **/
-s32 ixgbe_init_ext_t_x550em(struct ixgbe_hw *hw)
+static s32 ixgbe_init_ext_t_x550em(struct ixgbe_hw *hw)
 {
 	u32 status;
 	u16 reg;
@@ -1202,7 +1207,7 @@ s32 ixgbe_init_ext_t_x550em(struct ixgbe_hw *hw)
  **  and clears all interrupts, perform a PHY reset, and perform a link (MAC)
  **  reset.
  **/
-s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
+static s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 {
 	ixgbe_link_speed link_speed;
 	s32 status;
-- 
1.9.3

[net-next 04/16] ixgbe: allow multiple queues in SRIOV mode

[Jeff Kirsher]

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

ixgbe_set_sriov_queues() has the logic to allow multiple queues, this patch
just removes the limitation.

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index c76ba90..a82a36b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -101,9 +101,6 @@ static int __ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
 			adapter->dcb_cfg.num_tcs.pfc_tcs = 1;
 		}
 
-		/* We do not support RSS w/ SR-IOV */
-		adapter->ring_feature[RING_F_RSS].limit = 1;
-
 		/* Disable RSC when in SR-IOV mode */
 		adapter->flags2 &= ~(IXGBE_FLAG2_RSC_CAPABLE |
 				     IXGBE_FLAG2_RSC_ENABLED);
-- 
1.9.3

[net-next 05/16] ixgbevf: enable multiple queue support

[Jeff Kirsher]

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

This patch enables multiple queues and RSS support for the VF.
Maximum of 2 queues are supported due to available vectors.

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  1 +
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 +++++++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 8c44ab2..65cda34 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -124,6 +124,7 @@ struct ixgbevf_ring {
 
 #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
 #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
+#define IXGBEVF_MAX_RSS_QUEUES 2
 
 #define IXGBEVF_DEFAULT_TXD   1024
 #define IXGBEVF_DEFAULT_RXD   512
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index c9b49bf..815808f 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
 	struct ixgbe_hw *hw = &adapter->hw;
 	unsigned int def_q = 0;
 	unsigned int num_tcs = 0;
-	unsigned int num_rx_queues = 1;
+	unsigned int num_rx_queues = adapter->num_rx_queues;
+	unsigned int num_tx_queues = adapter->num_tx_queues;
 	int err;
 
 	spin_lock_bh(&adapter->mbx_lock);
@@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
 		return err;
 
 	if (num_tcs > 1) {
+		/* we need only one Tx queue */
+		num_tx_queues = 1;
+
 		/* update default Tx ring register index */
 		adapter->tx_ring[0]->reg_idx = def_q;
 
@@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
 	}
 
 	/* if we have a bad config abort request queue reset */
-	if (adapter->num_rx_queues != num_rx_queues) {
+	if ((adapter->num_rx_queues != num_rx_queues) ||
+	    (adapter->num_tx_queues != num_tx_queues)) {
 		/* force mailbox timeout to prevent further messages */
 		hw->mbx.timeout = 0;
 
@@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct ixgbevf_adapter *adapter)
 		return;
 
 	/* we need as many queues as traffic classes */
-	if (num_tcs > 1)
+	if (num_tcs > 1) {
 		adapter->num_rx_queues = num_tcs;
+	} else {
+		u16 rss = min_t(u16, num_online_cpus(), IXGBEVF_MAX_RSS_QUEUES);
+
+		switch (hw->api_version) {
+		case ixgbe_mbox_api_11:
+			adapter->num_rx_queues = rss;
+			adapter->num_tx_queues = rss;
+		default:
+			break;
+		}
+	}
 }
 
 /**
-- 
1.9.3

[net-next 06/16] ixgbevf: add RSS support for X550

[Jeff Kirsher]

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

X550 provides RSS registers for configuring RSS per VF.

This patch introduces ixgbevf_setup_vfmrqc() which uses the VFRETA,
VFRSSRK and VFMRQC registers to configure RSS on X550.

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 35 +++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/regs.h         | 10 +++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 815808f..41fca1b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1584,6 +1584,39 @@ static void ixgbevf_rx_desc_queue_enable(struct ixgbevf_adapter *adapter,
 		       reg_idx);
 }
 
+static void ixgbevf_setup_vfmrqc(struct ixgbevf_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 vfmrqc = 0, vfreta = 0;
+	u32 rss_key[10];
+	u16 rss_i = adapter->num_rx_queues;
+	int i, j;
+
+	/* Fill out hash function seeds */
+	netdev_rss_key_fill(rss_key, sizeof(rss_key));
+	for (i = 0; i < 10; i++)
+		IXGBE_WRITE_REG(hw, IXGBE_VFRSSRK(i), rss_key[i]);
+
+	/* Fill out redirection table */
+	for (i = 0, j = 0; i < 64; i++, j++) {
+		if (j == rss_i)
+			j = 0;
+		vfreta = (vfreta << 8) | (j * 0x1);
+		if ((i & 3) == 3)
+			IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2), vfreta);
+	}
+
+	/* Perform hash on these packet types */
+	vfmrqc |= IXGBE_VFMRQC_RSS_FIELD_IPV4 |
+		IXGBE_VFMRQC_RSS_FIELD_IPV4_TCP |
+		IXGBE_VFMRQC_RSS_FIELD_IPV6 |
+		IXGBE_VFMRQC_RSS_FIELD_IPV6_TCP;
+
+	vfmrqc |= IXGBE_VFMRQC_RSSEN;
+
+	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
+}
+
 static void ixgbevf_configure_rx_ring(struct ixgbevf_adapter *adapter,
 				      struct ixgbevf_ring *ring)
 {
@@ -1640,6 +1673,8 @@ static void ixgbevf_configure_rx(struct ixgbevf_adapter *adapter)
 	struct net_device *netdev = adapter->netdev;
 
 	ixgbevf_setup_psrtype(adapter);
+	if (hw->mac.type >= ixgbe_mac_X550_vf)
+		ixgbevf_setup_vfmrqc(adapter);
 
 	/* notify the PF of our intent to use this size of frame */
 	ixgbevf_rlpml_set_vf(hw, netdev->mtu + ETH_HLEN + ETH_FCS_LEN);
diff --git a/drivers/net/ethernet/intel/ixgbevf/regs.h b/drivers/net/ethernet/intel/ixgbevf/regs.h
index 09dd8f6..3e712fd 100644
--- a/drivers/net/ethernet/intel/ixgbevf/regs.h
+++ b/drivers/net/ethernet/intel/ixgbevf/regs.h
@@ -69,6 +69,16 @@
 #define IXGBE_VFGOTC_LSB       0x02020
 #define IXGBE_VFGOTC_MSB       0x02024
 #define IXGBE_VFMPRC           0x01034
+#define IXGBE_VFMRQC           0x3000
+#define IXGBE_VFRSSRK(x)       (0x3100 + ((x) * 4))
+#define IXGBE_VFRETA(x)        (0x3200 + ((x) * 4))
+
+/* VFMRQC bits */
+#define IXGBE_VFMRQC_RSSEN              0x00000001  /* RSS Enable */
+#define IXGBE_VFMRQC_RSS_FIELD_IPV4_TCP 0x00010000
+#define IXGBE_VFMRQC_RSS_FIELD_IPV4     0x00020000
+#define IXGBE_VFMRQC_RSS_FIELD_IPV6     0x00100000
+#define IXGBE_VFMRQC_RSS_FIELD_IPV6_TCP 0x00200000
 
 #define IXGBE_WRITE_FLUSH(a) (IXGBE_READ_REG(a, IXGBE_VFSTATUS))
 
-- 
1.9.3

[net-next 07/16] ixgbe: fix setting port VLAN

[Jeff Kirsher]

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

This patch fixes couple of issues introduced by
commit 2b509c0cd292 ("ixgbe: cleanup ixgbe_ndo_set_vf_vlan")

- fix setting of the VLAN inside ixgbe_enable_port_vlan()
- disable the "hide VLAN" bit in PFQDE when port VLAN is disabled

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index a82a36b..7f37fe7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -1094,14 +1094,12 @@ static int ixgbe_enable_port_vlan(struct ixgbe_adapter *adapter, int vf,
 				  u16 vlan, u8 qos)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
-	int err = 0;
+	int err;
 
-	if (adapter->vfinfo[vf].pf_vlan)
-		err = ixgbe_set_vf_vlan(adapter, false,
-					adapter->vfinfo[vf].pf_vlan,
-					vf);
+	err = ixgbe_set_vf_vlan(adapter, true, vlan, vf);
 	if (err)
 		goto out;
+
 	ixgbe_set_vmvir(adapter, vlan, qos, vf);
 	ixgbe_set_vmolr(hw, vf, false);
 	if (adapter->vfinfo[vf].spoofchk_enabled)
@@ -1140,6 +1138,11 @@ static int ixgbe_disable_port_vlan(struct ixgbe_adapter *adapter, int vf)
 	hw->mac.ops.set_vlan_anti_spoofing(hw, false, vf);
 	if (adapter->vfinfo[vf].vlan_count)
 		adapter->vfinfo[vf].vlan_count--;
+
+	/* disable hide VLAN on X550 */
+	if (hw->mac.type >= ixgbe_mac_X550)
+		ixgbe_write_qde(adapter, vf, IXGBE_QDE_ENABLE);
+
 	adapter->vfinfo[vf].pf_vlan = 0;
 	adapter->vfinfo[vf].pf_qos = 0;
 
-- 
1.9.3

[net-next 08/16] ixgbe: cleanup redundant default method set_rxpba

[Jeff Kirsher]

From: Don Skidmore <donald.c.skidmore@intel.com>

My original patch 6a14ee0cfb19 "ixgbe: Add X550 support function pointers"
accidental set a default value for this structure member twice.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index fcba952..84affca 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -1350,7 +1350,6 @@ static struct ixgbe_mac_operations mac_ops_X550 = {
 	.get_san_mac_addr	= &ixgbe_get_san_mac_addr_generic,
 	.get_wwn_prefix		= &ixgbe_get_wwn_prefix_generic,
 	.setup_link		= &ixgbe_setup_mac_link_X540,
-	.set_rxpba		= &ixgbe_set_rxpba_generic,
 	.get_link_capabilities	= &ixgbe_get_copper_link_capabilities_generic,
 	.setup_sfp		= NULL,
 };
-- 
1.9.3

[net-next 09/16] ixgbe: Cleanup probe to remove redundant attempt to ID PHY

[Jeff Kirsher]

From: Don Skidmore <donald.c.skidmore@intel.com>

We always identify the PHY in our reset_hw path anyway so there is
no need to do it in get_invariants().  The reason I even noticed this
is that for new hardware (X550em) we don't assign some methods until
later in probe and calling phy.ops.read_reg could lead to a panic.

Signed-off-by: Don Skidmore <donald.c.skidmore@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/ixgbe/ixgbe_x540.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
index ba54ff0..4939542 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
@@ -55,9 +55,6 @@ s32 ixgbe_get_invariants_X540(struct ixgbe_hw *hw)
 {
 	struct ixgbe_mac_info *mac = &hw->mac;
 
-	/* Call PHY identify routine to get the phy type */
-	ixgbe_identify_phy_generic(hw);
-
 	mac->mcft_size = IXGBE_X540_MC_TBL_SIZE;
 	mac->vft_size = IXGBE_X540_VFT_TBL_SIZE;
 	mac->num_rar_entries = IXGBE_X540_RAR_ENTRIES;
-- 
1.9.3

[net-next 10/16] ixgbe: add VXLAN offload support for X550 devices

[Jeff Kirsher]

From: Don Skidmore <donald.c.skidmore@intel.com>

Add support VXLAN receive checksum offload in X550 hardware.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/Kconfig            | 11 +++
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 98 ++++++++++++++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  5 ++
 4 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index 4d61ef5..f4ff465 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -192,6 +192,17 @@ config IXGBE
 	  To compile this driver as a module, choose M here. The module
 	  will be called ixgbe.
 
+config IXGBE_VXLAN
+	bool "Virtual eXtensible Local Area Network Support"
+	default n
+	depends on IXGBE && VXLAN && !(IXGBE=y && VXLAN=m)
+	---help---
+	  This allows one to create VXLAN virtual interfaces that provide
+	  Layer 2 Networks over Layer 3 Networks. VXLAN is often used
+	  to tunnel virtual network infrastructure in virtualized environments.
+	  Say Y here if you want to use Virtual eXtensible Local Area Network
+	  (VXLAN) in the driver.
+
 config IXGBE_HWMON
 	bool "Intel(R) 10GbE PCI Express adapters HWMON support"
 	default y
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 38fc64c..699117af 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -753,6 +753,7 @@ struct ixgbe_adapter {
 	u32 timer_event_accumulator;
 	u32 vferr_refcount;
 	struct ixgbe_mac_addr *mac_table;
+	u16 vxlan_port;
 	struct kobject *info_kobj;
 #ifdef CONFIG_IXGBE_HWMON
 	struct hwmon_buff *ixgbe_hwmon_buff;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index e4086fe..065ff48 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -50,6 +50,7 @@
 #include <linux/if_bridge.h>
 #include <linux/prefetch.h>
 #include <scsi/fc/fc_fcoe.h>
+#include <net/vxlan.h>
 
 #ifdef CONFIG_OF
 #include <linux/of_net.h>
@@ -1396,12 +1397,23 @@ static inline void ixgbe_rx_checksum(struct ixgbe_ring *ring,
 				     union ixgbe_adv_rx_desc *rx_desc,
 				     struct sk_buff *skb)
 {
+	__le16 pkt_info = rx_desc->wb.lower.lo_dword.hs_rss.pkt_info;
+	__le16 hdr_info = rx_desc->wb.lower.lo_dword.hs_rss.hdr_info;
+	bool encap_pkt = false;
+
 	skb_checksum_none_assert(skb);
 
 	/* Rx csum disabled */
 	if (!(ring->netdev->features & NETIF_F_RXCSUM))
 		return;
 
+	if ((pkt_info & cpu_to_le16(IXGBE_RXDADV_PKTTYPE_VXLAN)) &&
+	    (hdr_info & cpu_to_le16(IXGBE_RXDADV_PKTTYPE_TUNNEL >> 16))) {
+		encap_pkt = true;
+		skb->encapsulation = 1;
+		skb->ip_summed = CHECKSUM_NONE;
+	}
+
 	/* if IP and error */
 	if (ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_IPCS) &&
 	    ixgbe_test_staterr(rx_desc, IXGBE_RXDADV_ERR_IPE)) {
@@ -1413,8 +1425,6 @@ static inline void ixgbe_rx_checksum(struct ixgbe_ring *ring,
 		return;
 
 	if (ixgbe_test_staterr(rx_desc, IXGBE_RXDADV_ERR_TCPE)) {
-		__le16 pkt_info = rx_desc->wb.lower.lo_dword.hs_rss.pkt_info;
-
 		/*
 		 * 82599 errata, UDP frames with a 0 checksum can be marked as
 		 * checksum errors.
@@ -1429,6 +1439,17 @@ static inline void ixgbe_rx_checksum(struct ixgbe_ring *ring,
 
 	/* It must be a TCP or UDP packet with a valid checksum */
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
+	if (encap_pkt) {
+		if (!ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_OUTERIPCS))
+			return;
+
+		if (ixgbe_test_staterr(rx_desc, IXGBE_RXDADV_ERR_OUTERIPER)) {
+			ring->rx_stats.csum_err++;
+			return;
+		}
+		/* If we checked the outer header let the stack know */
+		skb->csum_level = 1;
+	}
 }
 
 static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
@@ -5627,6 +5648,10 @@ static int ixgbe_open(struct net_device *netdev)
 
 	ixgbe_up_complete(adapter);
 
+#if IS_ENABLED(CONFIG_IXGBE_VXLAN)
+	vxlan_get_rx_port(netdev);
+
+#endif
 	return 0;
 
 err_set_queues:
@@ -7771,6 +7796,64 @@ static int ixgbe_set_features(struct net_device *netdev,
 	return 0;
 }
 
+/**
+ * ixgbe_add_vxlan_port - Get notifications about VXLAN ports that come up
+ * @dev: The port's netdev
+ * @sa_family: Socket Family that VXLAN is notifiying us about
+ * @port: New UDP port number that VXLAN started listening to
+ **/
+static void ixgbe_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
+				 __be16 port)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	u16 new_port = ntohs(port);
+
+	if (sa_family == AF_INET6)
+		return;
+
+	if (adapter->vxlan_port == new_port) {
+		netdev_info(dev, "Port %d already offloaded\n", new_port);
+		return;
+	}
+
+	if (adapter->vxlan_port) {
+		netdev_info(dev,
+			    "Hit Max num of UDP ports, not adding port %d\n",
+			    new_port);
+		return;
+	}
+
+	adapter->vxlan_port = new_port;
+	IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, new_port);
+}
+
+/**
+ * ixgbe_del_vxlan_port - Get notifications about VXLAN ports that go away
+ * @dev: The port's netdev
+ * @sa_family: Socket Family that VXLAN is notifying us about
+ * @port: UDP port number that VXLAN stopped listening to
+ **/
+static void ixgbe_del_vxlan_port(struct net_device *dev, sa_family_t sa_family,
+				 __be16 port)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	u16 new_port = ntohs(port);
+
+	if (sa_family == AF_INET6)
+		return;
+
+	if (adapter->vxlan_port != new_port) {
+		netdev_info(dev, "Port %d was not found, not deleting\n",
+			    new_port);
+		return;
+	}
+
+	adapter->vxlan_port = 0;
+	IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, 0);
+}
+
 static int ixgbe_ndo_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 			     struct net_device *dev,
 			     const unsigned char *addr, u16 vid,
@@ -7982,6 +8065,8 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_bridge_getlink	= ixgbe_ndo_bridge_getlink,
 	.ndo_dfwd_add_station	= ixgbe_fwd_add,
 	.ndo_dfwd_del_station	= ixgbe_fwd_del,
+	.ndo_add_vxlan_port	= ixgbe_add_vxlan_port,
+	.ndo_del_vxlan_port	= ixgbe_del_vxlan_port,
 };
 
 /**
@@ -8339,6 +8424,15 @@ skip_sriov:
 	netdev->priv_flags |= IFF_UNICAST_FLT;
 	netdev->priv_flags |= IFF_SUPP_NOFCS;
 
+	switch (adapter->hw.mac.type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+		netdev->hw_enc_features |= NETIF_F_RXCSUM;
+		break;
+	default:
+		break;
+	}
+
 #ifdef CONFIG_IXGBE_DCB
 	netdev->dcbnl_ops = &dcbnl_ops;
 #endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 1f301ec..38a0e64 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -399,6 +399,7 @@ struct ixgbe_thermal_sensor_data {
 
 #define IXGBE_WUPL      0x05900
 #define IXGBE_WUPM      0x05A00 /* wake up pkt memory 0x5A00-0x5A7C */
+#define IXGBE_VXLANCTRL	0x0000507C /* Rx filter VXLAN UDPPORT Register */
 #define IXGBE_FHFT(_n)	(0x09000 + ((_n) * 0x100)) /* Flex host filter table */
 #define IXGBE_FHFT_EXT(_n)	(0x09800 + ((_n) * 0x100)) /* Ext Flexible Host
 							    * Filter Table */
@@ -2122,6 +2123,7 @@ enum {
 #define IXGBE_RXD_STAT_IPCS     0x40    /* IP xsum calculated */
 #define IXGBE_RXD_STAT_PIF      0x80    /* passed in-exact filter */
 #define IXGBE_RXD_STAT_CRCV     0x100   /* Speculative CRC Valid */
+#define IXGBE_RXD_STAT_OUTERIPCS  0x100 /* Cloud IP xsum calculated */
 #define IXGBE_RXD_STAT_VEXT     0x200   /* 1st VLAN found */
 #define IXGBE_RXD_STAT_UDPV     0x400   /* Valid UDP checksum */
 #define IXGBE_RXD_STAT_DYNINT   0x800   /* Pkt caused INT via DYNINT */
@@ -2139,6 +2141,7 @@ enum {
 #define IXGBE_RXD_ERR_IPE       0x80    /* IP Checksum Error */
 #define IXGBE_RXDADV_ERR_MASK           0xfff00000 /* RDESC.ERRORS mask */
 #define IXGBE_RXDADV_ERR_SHIFT          20         /* RDESC.ERRORS shift */
+#define IXGBE_RXDADV_ERR_OUTERIPER	0x04000000 /* CRC IP Header error */
 #define IXGBE_RXDADV_ERR_FCEOFE         0x80000000 /* FCoEFe/IPE */
 #define IXGBE_RXDADV_ERR_FCERR          0x00700000 /* FCERR/FDIRERR */
 #define IXGBE_RXDADV_ERR_FDIR_LEN       0x00100000 /* FDIR Length error */
@@ -2227,6 +2230,8 @@ enum {
 #define IXGBE_RXDADV_PKTTYPE_UDP        0x00000200 /* UDP hdr present */
 #define IXGBE_RXDADV_PKTTYPE_SCTP       0x00000400 /* SCTP hdr present */
 #define IXGBE_RXDADV_PKTTYPE_NFS        0x00000800 /* NFS hdr present */
+#define IXGBE_RXDADV_PKTTYPE_VXLAN	0x00000800 /* VXLAN hdr present */
+#define IXGBE_RXDADV_PKTTYPE_TUNNEL	0x00010000 /* Tunnel type */
 #define IXGBE_RXDADV_PKTTYPE_IPSEC_ESP  0x00001000 /* IPSec ESP */
 #define IXGBE_RXDADV_PKTTYPE_IPSEC_AH   0x00002000 /* IPSec AH */
 #define IXGBE_RXDADV_PKTTYPE_LINKSEC    0x00004000 /* LinkSec Encap */
-- 
1.9.3

[net-next 11/16] ixgbevf: set vlan_features in a single write instead of several ORs

[Jeff Kirsher]

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

Clean up the setting of vlan_features by enabling all features at once.

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 | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 41fca1b..b1d25fd 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3873,11 +3873,11 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 			   NETIF_F_HW_VLAN_CTAG_RX |
 			   NETIF_F_HW_VLAN_CTAG_FILTER;
 
-	netdev->vlan_features |= NETIF_F_TSO;
-	netdev->vlan_features |= NETIF_F_TSO6;
-	netdev->vlan_features |= NETIF_F_IP_CSUM;
-	netdev->vlan_features |= NETIF_F_IPV6_CSUM;
-	netdev->vlan_features |= NETIF_F_SG;
+	netdev->vlan_features |= NETIF_F_TSO |
+				 NETIF_F_TSO6 |
+				 NETIF_F_IP_CSUM |
+				 NETIF_F_IPV6_CSUM |
+				 NETIF_F_SG;
 
 	if (pci_using_dac)
 		netdev->features |= NETIF_F_HIGHDMA;
-- 
1.9.3

[net-next 12/16] ixgbevf: Fix ordering of shutdown to correctly disable Rx and Tx

[Jeff Kirsher]

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

This patch updates the ordering of the shutdown path so that we attempt to
shutdown the rings more gracefully. Basically the big changes are that we
shutdown the main Rx filter in the case of Rx and we set the carrier_off
state in the case of Tx so that packets stop being delivered from outside
the driver.  Then we shut down interrupts and NAPI.  Finally we stop the
rings from performing DMA and clean them.  This is a bit more graceful than
the previous path.

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 | 38 ++++++++++-------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index b1d25fd..9281685 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1957,6 +1957,10 @@ static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
 	clear_bit(__IXGBEVF_DOWN, &adapter->state);
 	ixgbevf_napi_enable_all(adapter);
 
+	/* clear any pending interrupts, may auto mask */
+	IXGBE_READ_REG(hw, IXGBE_VTEICR);
+	ixgbevf_irq_enable(adapter);
+
 	/* enable transmits */
 	netif_tx_start_all_queues(netdev);
 
@@ -1969,16 +1973,9 @@ static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
 
 void ixgbevf_up(struct ixgbevf_adapter *adapter)
 {
-	struct ixgbe_hw *hw = &adapter->hw;
-
 	ixgbevf_configure(adapter);
 
 	ixgbevf_up_complete(adapter);
-
-	/* clear any pending interrupts, may auto mask */
-	IXGBE_READ_REG(hw, IXGBE_VTEICR);
-
-	ixgbevf_irq_enable(adapter);
 }
 
 /**
@@ -2085,17 +2082,20 @@ void ixgbevf_down(struct ixgbevf_adapter *adapter)
 	for (i = 0; i < adapter->num_rx_queues; i++)
 		ixgbevf_disable_rx_queue(adapter, adapter->rx_ring[i]);
 
-	netif_tx_disable(netdev);
-
-	msleep(10);
+	usleep_range(10000, 20000);
 
 	netif_tx_stop_all_queues(netdev);
 
+	/* call carrier off first to avoid false dev_watchdog timeouts */
+	netif_carrier_off(netdev);
+	netif_tx_disable(netdev);
+
 	ixgbevf_irq_disable(adapter);
 
 	ixgbevf_napi_disable_all(adapter);
 
 	del_timer_sync(&adapter->watchdog_timer);
+
 	/* can't call flush scheduled work here because it can deadlock
 	 * if linkwatch_event tries to acquire the rtnl_lock which we are
 	 * holding */
@@ -2110,8 +2110,6 @@ void ixgbevf_down(struct ixgbevf_adapter *adapter)
 				IXGBE_TXDCTL_SWFLSH);
 	}
 
-	netif_carrier_off(netdev);
-
 	if (!pci_channel_offline(adapter->pdev))
 		ixgbevf_reset(adapter);
 
@@ -2995,10 +2993,6 @@ static int ixgbevf_open(struct net_device *netdev)
 	if (!adapter->num_msix_vectors)
 		return -ENOMEM;
 
-	/* disallow open during test */
-	if (test_bit(__IXGBEVF_TESTING, &adapter->state))
-		return -EBUSY;
-
 	if (hw->adapter_stopped) {
 		ixgbevf_reset(adapter);
 		/* if adapter is still stopped then PF isn't up and
@@ -3011,6 +3005,12 @@ static int ixgbevf_open(struct net_device *netdev)
 		}
 	}
 
+	/* disallow open during test */
+	if (test_bit(__IXGBEVF_TESTING, &adapter->state))
+		return -EBUSY;
+
+	netif_carrier_off(netdev);
+
 	/* allocate transmit descriptors */
 	err = ixgbevf_setup_all_tx_resources(adapter);
 	if (err)
@@ -3030,15 +3030,11 @@ static int ixgbevf_open(struct net_device *netdev)
 	 */
 	ixgbevf_map_rings_to_vectors(adapter);
 
-	ixgbevf_up_complete(adapter);
-
-	/* clear any pending interrupts, may auto mask */
-	IXGBE_READ_REG(hw, IXGBE_VTEICR);
 	err = ixgbevf_request_irq(adapter);
 	if (err)
 		goto err_req_irq;
 
-	ixgbevf_irq_enable(adapter);
+	ixgbevf_up_complete(adapter);
 
 	return 0;
 
-- 
1.9.3

[net-next 13/16] ixgbevf: Add code to check for Tx hang

[Jeff Kirsher]

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

This patch adds code to allow for Tx hang checking.  The idea is to provide
more robust debug info in the event of a transmit unit hang. Similar to the
logic in ixgbe.

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      |  21 +++-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 113 ++++++++++++++++++----
 2 files changed, 115 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 65cda34..65c2aee 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -43,6 +43,13 @@
 #define BP_EXTENDED_STATS
 #endif
 
+#define IXGBE_MAX_TXD_PWR	14
+#define IXGBE_MAX_DATA_PER_TXD	BIT(IXGBE_MAX_TXD_PWR)
+
+/* Tx Descriptors needed, worst case */
+#define TXD_USE_COUNT(S) DIV_ROUND_UP((S), IXGBE_MAX_DATA_PER_TXD)
+#define DESC_NEEDED (MAX_SKB_FRAGS + 4)
+
 /* wrapper around a pointer to a socket buffer,
  * so a DMA handle can be stored along with the buffer */
 struct ixgbevf_tx_buffer {
@@ -85,6 +92,18 @@ struct ixgbevf_rx_queue_stats {
 	u64 csum_err;
 };
 
+enum ixgbevf_ring_state_t {
+	__IXGBEVF_TX_DETECT_HANG,
+	__IXGBEVF_HANG_CHECK_ARMED,
+};
+
+#define check_for_tx_hang(ring) \
+	test_bit(__IXGBEVF_TX_DETECT_HANG, &(ring)->state)
+#define set_check_for_tx_hang(ring) \
+	set_bit(__IXGBEVF_TX_DETECT_HANG, &(ring)->state)
+#define clear_check_for_tx_hang(ring) \
+	clear_bit(__IXGBEVF_TX_DETECT_HANG, &(ring)->state)
+
 struct ixgbevf_ring {
 	struct ixgbevf_ring *next;
 	struct net_device *netdev;
@@ -101,7 +120,7 @@ struct ixgbevf_ring {
 		struct ixgbevf_tx_buffer *tx_buffer_info;
 		struct ixgbevf_rx_buffer *rx_buffer_info;
 	};
-
+	unsigned long state;
 	struct ixgbevf_stats stats;
 	struct u64_stats_sync syncp;
 	union {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 9281685..12bcd9e 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -199,14 +199,64 @@ static void ixgbevf_unmap_and_free_tx_resource(struct ixgbevf_ring *tx_ring,
 	/* tx_buffer must be completely set up in the transmit path */
 }
 
-#define IXGBE_MAX_TXD_PWR	14
-#define IXGBE_MAX_DATA_PER_TXD	(1 << IXGBE_MAX_TXD_PWR)
+static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
+{
+	return ring->stats.packets;
+}
+
+static u32 ixgbevf_get_tx_pending(struct ixgbevf_ring *ring)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(ring->netdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+
+	u32 head = IXGBE_READ_REG(hw, IXGBE_VFTDH(ring->reg_idx));
+	u32 tail = IXGBE_READ_REG(hw, IXGBE_VFTDT(ring->reg_idx));
+
+	if (head != tail)
+		return (head < tail) ?
+			tail - head : (tail + ring->count - head);
+
+	return 0;
+}
+
+static inline bool ixgbevf_check_tx_hang(struct ixgbevf_ring *tx_ring)
+{
+	u32 tx_done = ixgbevf_get_tx_completed(tx_ring);
+	u32 tx_done_old = tx_ring->tx_stats.tx_done_old;
+	u32 tx_pending = ixgbevf_get_tx_pending(tx_ring);
+
+	clear_check_for_tx_hang(tx_ring);
 
-/* Tx Descriptors needed, worst case */
-#define TXD_USE_COUNT(S) DIV_ROUND_UP((S), IXGBE_MAX_DATA_PER_TXD)
-#define DESC_NEEDED (MAX_SKB_FRAGS + 4)
+	/* Check for a hung queue, but be thorough. This verifies
+	 * that a transmit has been completed since the previous
+	 * check AND there is at least one packet pending. The
+	 * ARMED bit is set to indicate a potential hang.
+	 */
+	if ((tx_done_old == tx_done) && tx_pending) {
+		/* make sure it is true for two checks in a row */
+		return test_and_set_bit(__IXGBEVF_HANG_CHECK_ARMED,
+					&tx_ring->state);
+	}
+	/* reset the countdown */
+	clear_bit(__IXGBEVF_HANG_CHECK_ARMED, &tx_ring->state);
+
+	/* update completed stats and continue */
+	tx_ring->tx_stats.tx_done_old = tx_done;
+
+	return false;
+}
+
+/**
+ * ixgbevf_tx_timeout - Respond to a Tx Hang
+ * @netdev: network interface device structure
+ **/
+static void ixgbevf_tx_timeout(struct net_device *netdev)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
 
-static void ixgbevf_tx_timeout(struct net_device *netdev);
+	/* Do the reset outside of interrupt context */
+	schedule_work(&adapter->reset_task);
+}
 
 /**
  * ixgbevf_clean_tx_irq - Reclaim resources after transmit completes
@@ -311,6 +361,37 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
 	q_vector->tx.total_bytes += total_bytes;
 	q_vector->tx.total_packets += total_packets;
 
+	if (check_for_tx_hang(tx_ring) && ixgbevf_check_tx_hang(tx_ring)) {
+		struct ixgbe_hw *hw = &adapter->hw;
+		union ixgbe_adv_tx_desc *eop_desc;
+
+		eop_desc = tx_ring->tx_buffer_info[i].next_to_watch;
+
+		pr_err("Detected Tx Unit Hang\n"
+		       "  Tx Queue             <%d>\n"
+		       "  TDH, TDT             <%x>, <%x>\n"
+		       "  next_to_use          <%x>\n"
+		       "  next_to_clean        <%x>\n"
+		       "tx_buffer_info[next_to_clean]\n"
+		       "  next_to_watch        <%p>\n"
+		       "  eop_desc->wb.status  <%x>\n"
+		       "  time_stamp           <%lx>\n"
+		       "  jiffies              <%lx>\n",
+		       tx_ring->queue_index,
+		       IXGBE_READ_REG(hw, IXGBE_VFTDH(tx_ring->reg_idx)),
+		       IXGBE_READ_REG(hw, IXGBE_VFTDT(tx_ring->reg_idx)),
+		       tx_ring->next_to_use, i,
+		       eop_desc, (eop_desc ? eop_desc->wb.status : 0),
+		       tx_ring->tx_buffer_info[i].time_stamp, jiffies);
+
+		netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
+
+		/* schedule immediate reset if we believe we hung */
+		schedule_work(&adapter->reset_task);
+
+		return true;
+	}
+
 #define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
 	if (unlikely(total_packets && netif_carrier_ok(tx_ring->netdev) &&
 		     (ixgbevf_desc_unused(tx_ring) >= TX_WAKE_THRESHOLD))) {
@@ -1479,6 +1560,8 @@ static void ixgbevf_configure_tx_ring(struct ixgbevf_adapter *adapter,
 	txdctl |= (1 << 8) |    /* HTHRESH = 1 */
 		  32;          /* PTHRESH = 32 */
 
+	clear_bit(__IXGBEVF_HANG_CHECK_ARMED, &ring->state);
+
 	IXGBE_WRITE_REG(hw, IXGBE_VFTXDCTL(reg_idx), txdctl);
 
 	/* poll to verify queue is enabled */
@@ -2643,6 +2726,12 @@ static void ixgbevf_watchdog(unsigned long data)
 	if (test_bit(__IXGBEVF_DOWN, &adapter->state))
 		goto watchdog_short_circuit;
 
+	/* Force detection of hung controller */
+	if (netif_carrier_ok(adapter->netdev)) {
+		for (i = 0; i < adapter->num_tx_queues; i++)
+			set_check_for_tx_hang(adapter->tx_ring[i]);
+	}
+
 	/* get one bit for every active tx/rx interrupt vector */
 	for (i = 0; i < adapter->num_msix_vectors - NON_Q_VECTORS; i++) {
 		struct ixgbevf_q_vector *qv = adapter->q_vector[i];
@@ -2656,18 +2745,6 @@ watchdog_short_circuit:
 	schedule_work(&adapter->watchdog_task);
 }
 
-/**
- * ixgbevf_tx_timeout - Respond to a Tx Hang
- * @netdev: network interface device structure
- **/
-static void ixgbevf_tx_timeout(struct net_device *netdev)
-{
-	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
-
-	/* Do the reset outside of interrupt context */
-	schedule_work(&adapter->reset_task);
-}
-
 static void ixgbevf_reset_task(struct work_struct *work)
 {
 	struct ixgbevf_adapter *adapter;
-- 
1.9.3

[net-next 14/16] ixgbevf: rewrite watchdog task to function similar to igbvf

[Jeff Kirsher]

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

This patch cleans up the logic dealing with link down/up by breaking down the
link detection and up/down events into separate functions - similar to how these
events are handled in other drivers.

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      |   1 +
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 187 +++++++++++++---------
 2 files changed, 113 insertions(+), 75 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 65c2aee..a41ab37 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -436,6 +436,7 @@ struct ixgbevf_adapter {
 	bool link_up;
 
 	spinlock_t mbx_lock;
+	unsigned long last_reset;
 
 	struct work_struct watchdog_task;
 };
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 12bcd9e..b13cae1 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -2231,6 +2231,8 @@ void ixgbevf_reset(struct ixgbevf_adapter *adapter)
 		memcpy(netdev->perm_addr, adapter->hw.mac.addr,
 		       netdev->addr_len);
 	}
+
+	adapter->last_reset = jiffies;
 }
 
 static int ixgbevf_acquire_msix_vectors(struct ixgbevf_adapter *adapter,
@@ -2684,7 +2686,8 @@ void ixgbevf_update_stats(struct ixgbevf_adapter *adapter)
 	struct ixgbe_hw *hw = &adapter->hw;
 	int i;
 
-	if (!adapter->link_up)
+	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+	    test_bit(__IXGBEVF_RESETTING, &adapter->state))
 		return;
 
 	UPDATE_VF_COUNTER_32bit(IXGBE_VFGPRC, adapter->stats.last_vfgprc,
@@ -2714,17 +2717,45 @@ void ixgbevf_update_stats(struct ixgbevf_adapter *adapter)
 static void ixgbevf_watchdog(unsigned long data)
 {
 	struct ixgbevf_adapter *adapter = (struct ixgbevf_adapter *)data;
+
+	/* Do the reset outside of interrupt context */
+	schedule_work(&adapter->watchdog_task);
+}
+
+static void ixgbevf_reset_task(struct work_struct *work)
+{
+	struct ixgbevf_adapter *adapter;
+
+	adapter = container_of(work, struct ixgbevf_adapter, reset_task);
+
+	/* If we're already down or resetting, just bail */
+	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+	    test_bit(__IXGBEVF_RESETTING, &adapter->state))
+		return;
+
+	adapter->tx_timeout_count++;
+
+	ixgbevf_reinit_locked(adapter);
+}
+
+/* ixgbevf_check_hang_subtask - check for hung queues and dropped interrupts
+ * @adapter - pointer to the device adapter structure
+ *
+ * This function serves two purposes.  First it strobes the interrupt lines
+ * in order to make certain interrupts are occurring.  Secondly it sets the
+ * bits needed to check for TX hangs.  As a result we should immediately
+ * determine if a hang has occurred.
+ */
+static void ixgbevf_check_hang_subtask(struct ixgbevf_adapter *adapter)
+{
 	struct ixgbe_hw *hw = &adapter->hw;
 	u32 eics = 0;
 	int i;
 
-	/*
-	 * Do the watchdog outside of interrupt context due to the lovely
-	 * delays that some of the newer hardware requires
-	 */
-
-	if (test_bit(__IXGBEVF_DOWN, &adapter->state))
-		goto watchdog_short_circuit;
+	/* If we're down or resetting, just bail */
+	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+	    test_bit(__IXGBEVF_RESETTING, &adapter->state))
+		return;
 
 	/* Force detection of hung controller */
 	if (netif_carrier_ok(adapter->netdev)) {
@@ -2739,26 +2770,80 @@ static void ixgbevf_watchdog(unsigned long data)
 			eics |= 1 << i;
 	}
 
+	/* Cause software interrupt to ensure rings are cleaned */
 	IXGBE_WRITE_REG(hw, IXGBE_VTEICS, eics);
+}
 
-watchdog_short_circuit:
-	schedule_work(&adapter->watchdog_task);
+/**
+ * ixgbevf_watchdog_update_link - update the link status
+ * @adapter - pointer to the device adapter structure
+ **/
+static void ixgbevf_watchdog_update_link(struct ixgbevf_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 link_speed = adapter->link_speed;
+	bool link_up = adapter->link_up;
+	s32 err;
+
+	spin_lock_bh(&adapter->mbx_lock);
+
+	err = hw->mac.ops.check_link(hw, &link_speed, &link_up, false);
+
+	spin_unlock_bh(&adapter->mbx_lock);
+
+	/* if check for link returns error we will need to reset */
+	if (err && time_after(jiffies, adapter->last_reset + (10 * HZ))) {
+		schedule_work(&adapter->reset_task);
+		link_up = false;
+	}
+
+	adapter->link_up = link_up;
+	adapter->link_speed = link_speed;
 }
 
-static void ixgbevf_reset_task(struct work_struct *work)
+/**
+ * ixgbevf_watchdog_link_is_up - update netif_carrier status and
+ *				 print link up message
+ * @adapter - pointer to the device adapter structure
+ **/
+static void ixgbevf_watchdog_link_is_up(struct ixgbevf_adapter *adapter)
 {
-	struct ixgbevf_adapter *adapter;
-	adapter = container_of(work, struct ixgbevf_adapter, reset_task);
+	struct net_device *netdev = adapter->netdev;
 
-	/* If we're already down or resetting, just bail */
-	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
-	    test_bit(__IXGBEVF_REMOVING, &adapter->state) ||
-	    test_bit(__IXGBEVF_RESETTING, &adapter->state))
+	/* only continue if link was previously down */
+	if (netif_carrier_ok(netdev))
 		return;
 
-	adapter->tx_timeout_count++;
+	dev_info(&adapter->pdev->dev, "NIC Link is Up %s\n",
+		 (adapter->link_speed == IXGBE_LINK_SPEED_10GB_FULL) ?
+		 "10 Gbps" :
+		 (adapter->link_speed == IXGBE_LINK_SPEED_1GB_FULL) ?
+		 "1 Gbps" :
+		 (adapter->link_speed == IXGBE_LINK_SPEED_100_FULL) ?
+		 "100 Mbps" :
+		 "unknown speed");
 
-	ixgbevf_reinit_locked(adapter);
+	netif_carrier_on(netdev);
+}
+
+/**
+ * ixgbevf_watchdog_link_is_down - update netif_carrier status and
+ *				   print link down message
+ * @adapter - pointer to the adapter structure
+ **/
+static void ixgbevf_watchdog_link_is_down(struct ixgbevf_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+
+	adapter->link_speed = 0;
+
+	/* only continue if link was up previously */
+	if (!netif_carrier_ok(netdev))
+		return;
+
+	dev_info(&adapter->pdev->dev, "NIC Link is Down\n");
+
+	netif_carrier_off(netdev);
 }
 
 /**
@@ -2770,11 +2855,7 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
 	struct ixgbevf_adapter *adapter = container_of(work,
 						       struct ixgbevf_adapter,
 						       watchdog_task);
-	struct net_device *netdev = adapter->netdev;
 	struct ixgbe_hw *hw = &adapter->hw;
-	u32 link_speed = adapter->link_speed;
-	bool link_up = adapter->link_up;
-	s32 need_reset;
 
 	if (IXGBE_REMOVED(hw->hw_addr)) {
 		if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
@@ -2784,66 +2865,22 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
 		}
 		return;
 	}
+
 	ixgbevf_queue_reset_subtask(adapter);
 
 	adapter->flags |= IXGBE_FLAG_IN_WATCHDOG_TASK;
 
-	/*
-	 * Always check the link on the watchdog because we have
-	 * no LSC interrupt
-	 */
-	spin_lock_bh(&adapter->mbx_lock);
-
-	need_reset = hw->mac.ops.check_link(hw, &link_speed, &link_up, false);
-
-	spin_unlock_bh(&adapter->mbx_lock);
-
-	if (need_reset) {
-		adapter->link_up = link_up;
-		adapter->link_speed = link_speed;
-		netif_carrier_off(netdev);
-		netif_tx_stop_all_queues(netdev);
-		schedule_work(&adapter->reset_task);
-		goto pf_has_reset;
-	}
-	adapter->link_up = link_up;
-	adapter->link_speed = link_speed;
+	ixgbevf_watchdog_update_link(adapter);
 
-	if (link_up) {
-		if (!netif_carrier_ok(netdev)) {
-			char *link_speed_string;
-			switch (link_speed) {
-			case IXGBE_LINK_SPEED_10GB_FULL:
-				link_speed_string = "10 Gbps";
-				break;
-			case IXGBE_LINK_SPEED_1GB_FULL:
-				link_speed_string = "1 Gbps";
-				break;
-			case IXGBE_LINK_SPEED_100_FULL:
-				link_speed_string = "100 Mbps";
-				break;
-			default:
-				link_speed_string = "unknown speed";
-				break;
-			}
-			dev_info(&adapter->pdev->dev,
-				"NIC Link is Up, %s\n", link_speed_string);
-			netif_carrier_on(netdev);
-			netif_tx_wake_all_queues(netdev);
-		}
-	} else {
-		adapter->link_up = false;
-		adapter->link_speed = 0;
-		if (netif_carrier_ok(netdev)) {
-			dev_info(&adapter->pdev->dev, "NIC Link is Down\n");
-			netif_carrier_off(netdev);
-			netif_tx_stop_all_queues(netdev);
-		}
-	}
+	if (adapter->link_up)
+		ixgbevf_watchdog_link_is_up(adapter);
+	else
+		ixgbevf_watchdog_link_is_down(adapter);
 
 	ixgbevf_update_stats(adapter);
 
-pf_has_reset:
+	ixgbevf_check_hang_subtask(adapter);
+
 	/* Reset the timer */
 	if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
 	    !test_bit(__IXGBEVF_REMOVING, &adapter->state))
-- 
1.9.3

[net-next 15/16] ixgbevf: combine all of the tasks into a single service task

[Jeff Kirsher]

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

This change combines the reset and watchdog tasklets into a single task.

The advantage of this is that we can avoid multiple schedules of the reset
task when we have a reset event needed due to either the mailbox going down
or transmit packets being present on a link down.

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      |  13 +-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 140 +++++++++++++---------
 2 files changed, 87 insertions(+), 66 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index a41ab37..3a9b356 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -367,8 +367,6 @@ struct ixgbevf_adapter {
 	/* 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];
 
 	/* Interrupt Throttle Rate */
@@ -398,8 +396,7 @@ struct ixgbevf_adapter {
 	 * thus the additional *_CAPABLE flags.
 	 */
 	u32 flags;
-#define IXGBE_FLAG_IN_WATCHDOG_TASK             (u32)(1)
-
+#define IXGBEVF_FLAG_RESET_REQUESTED		(u32)(1)
 #define IXGBEVF_FLAG_QUEUE_RESET_REQUESTED	(u32)(1 << 2)
 
 	struct msix_entry *msix_entries;
@@ -435,10 +432,11 @@ struct ixgbevf_adapter {
 	u32 link_speed;
 	bool link_up;
 
+	struct timer_list service_timer;
+	struct work_struct service_task;
+
 	spinlock_t mbx_lock;
 	unsigned long last_reset;
-
-	struct work_struct watchdog_task;
 };
 
 enum ixbgevf_state_t {
@@ -447,7 +445,8 @@ enum ixbgevf_state_t {
 	__IXGBEVF_DOWN,
 	__IXGBEVF_DISABLED,
 	__IXGBEVF_REMOVING,
-	__IXGBEVF_WORK_INIT,
+	__IXGBEVF_SERVICE_SCHED,
+	__IXGBEVF_SERVICE_INITED,
 };
 
 enum ixgbevf_boards {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index b13cae1..32e7625 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -98,6 +98,23 @@ static int debug = -1;
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
+static void ixgbevf_service_event_schedule(struct ixgbevf_adapter *adapter)
+{
+	if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
+	    !test_bit(__IXGBEVF_REMOVING, &adapter->state) &&
+	    !test_and_set_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state))
+		schedule_work(&adapter->service_task);
+}
+
+static void ixgbevf_service_event_complete(struct ixgbevf_adapter *adapter)
+{
+	BUG_ON(!test_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state));
+
+	/* flush memory to make sure state is correct before next watchdog */
+	smp_mb__before_atomic();
+	clear_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state);
+}
+
 /* forward decls */
 static void ixgbevf_queue_reset_subtask(struct ixgbevf_adapter *adapter);
 static void ixgbevf_set_itr(struct ixgbevf_q_vector *q_vector);
@@ -111,8 +128,8 @@ static void ixgbevf_remove_adapter(struct ixgbe_hw *hw)
 		return;
 	hw->hw_addr = NULL;
 	dev_err(&adapter->pdev->dev, "Adapter removed\n");
-	if (test_bit(__IXGBEVF_WORK_INIT, &adapter->state))
-		schedule_work(&adapter->watchdog_task);
+	if (test_bit(__IXGBEVF_SERVICE_INITED, &adapter->state))
+		ixgbevf_service_event_schedule(adapter);
 }
 
 static void ixgbevf_check_remove(struct ixgbe_hw *hw, u32 reg)
@@ -246,6 +263,15 @@ static inline bool ixgbevf_check_tx_hang(struct ixgbevf_ring *tx_ring)
 	return false;
 }
 
+static void ixgbevf_tx_timeout_reset(struct ixgbevf_adapter *adapter)
+{
+	/* Do the reset outside of interrupt context */
+	if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
+		adapter->flags |= IXGBEVF_FLAG_RESET_REQUESTED;
+		ixgbevf_service_event_schedule(adapter);
+	}
+}
+
 /**
  * ixgbevf_tx_timeout - Respond to a Tx Hang
  * @netdev: network interface device structure
@@ -254,8 +280,7 @@ static void ixgbevf_tx_timeout(struct net_device *netdev)
 {
 	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
 
-	/* Do the reset outside of interrupt context */
-	schedule_work(&adapter->reset_task);
+	ixgbevf_tx_timeout_reset(adapter);
 }
 
 /**
@@ -387,7 +412,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
 		netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
 
 		/* schedule immediate reset if we believe we hung */
-		schedule_work(&adapter->reset_task);
+		ixgbevf_tx_timeout_reset(adapter);
 
 		return true;
 	}
@@ -1239,9 +1264,7 @@ static irqreturn_t ixgbevf_msix_other(int irq, void *data)
 
 	hw->mac.get_link_status = 1;
 
-	if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
-	    !test_bit(__IXGBEVF_REMOVING, &adapter->state))
-		mod_timer(&adapter->watchdog_timer, jiffies);
+	ixgbevf_service_event_schedule(adapter);
 
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, adapter->eims_other);
 
@@ -2051,7 +2074,7 @@ static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
 	ixgbevf_init_last_counter_stats(adapter);
 
 	hw->mac.get_link_status = 1;
-	mod_timer(&adapter->watchdog_timer, jiffies);
+	mod_timer(&adapter->service_timer, jiffies);
 }
 
 void ixgbevf_up(struct ixgbevf_adapter *adapter)
@@ -2177,13 +2200,7 @@ void ixgbevf_down(struct ixgbevf_adapter *adapter)
 
 	ixgbevf_napi_disable_all(adapter);
 
-	del_timer_sync(&adapter->watchdog_timer);
-
-	/* can't call flush scheduled work here because it can deadlock
-	 * if linkwatch_event tries to acquire the rtnl_lock which we are
-	 * holding */
-	while (adapter->flags & IXGBE_FLAG_IN_WATCHDOG_TASK)
-		msleep(1);
+	del_timer_sync(&adapter->service_timer);
 
 	/* disable transmits in the hardware now that interrupts are off */
 	for (i = 0; i < adapter->num_tx_queues; i++) {
@@ -2711,22 +2728,25 @@ void ixgbevf_update_stats(struct ixgbevf_adapter *adapter)
 }
 
 /**
- * ixgbevf_watchdog - Timer Call-back
+ * ixgbevf_service_timer - Timer Call-back
  * @data: pointer to adapter cast into an unsigned long
  **/
-static void ixgbevf_watchdog(unsigned long data)
+static void ixgbevf_service_timer(unsigned long data)
 {
 	struct ixgbevf_adapter *adapter = (struct ixgbevf_adapter *)data;
 
-	/* Do the reset outside of interrupt context */
-	schedule_work(&adapter->watchdog_task);
+	/* Reset the timer */
+	mod_timer(&adapter->service_timer, (HZ * 2) + jiffies);
+
+	ixgbevf_service_event_schedule(adapter);
 }
 
-static void ixgbevf_reset_task(struct work_struct *work)
+static void ixgbevf_reset_subtask(struct ixgbevf_adapter *adapter)
 {
-	struct ixgbevf_adapter *adapter;
+	if (!(adapter->flags & IXGBEVF_FLAG_RESET_REQUESTED))
+		return;
 
-	adapter = container_of(work, struct ixgbevf_adapter, reset_task);
+	adapter->flags &= ~IXGBEVF_FLAG_RESET_REQUESTED;
 
 	/* If we're already down or resetting, just bail */
 	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
@@ -2766,6 +2786,7 @@ static void ixgbevf_check_hang_subtask(struct ixgbevf_adapter *adapter)
 	/* get one bit for every active tx/rx interrupt vector */
 	for (i = 0; i < adapter->num_msix_vectors - NON_Q_VECTORS; i++) {
 		struct ixgbevf_q_vector *qv = adapter->q_vector[i];
+
 		if (qv->rx.ring || qv->tx.ring)
 			eics |= 1 << i;
 	}
@@ -2793,7 +2814,7 @@ static void ixgbevf_watchdog_update_link(struct ixgbevf_adapter *adapter)
 
 	/* if check for link returns error we will need to reset */
 	if (err && time_after(jiffies, adapter->last_reset + (10 * HZ))) {
-		schedule_work(&adapter->reset_task);
+		adapter->flags |= IXGBEVF_FLAG_RESET_REQUESTED;
 		link_up = false;
 	}
 
@@ -2847,14 +2868,35 @@ static void ixgbevf_watchdog_link_is_down(struct ixgbevf_adapter *adapter)
 }
 
 /**
- * ixgbevf_watchdog_task - worker thread to bring link up
+ * ixgbevf_watchdog_subtask - worker thread to bring link up
+ * @work: pointer to work_struct containing our data
+ **/
+static void ixgbevf_watchdog_subtask(struct ixgbevf_adapter *adapter)
+{
+	/* if interface is down do nothing */
+	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
+	    test_bit(__IXGBEVF_RESETTING, &adapter->state))
+		return;
+
+	ixgbevf_watchdog_update_link(adapter);
+
+	if (adapter->link_up)
+		ixgbevf_watchdog_link_is_up(adapter);
+	else
+		ixgbevf_watchdog_link_is_down(adapter);
+
+	ixgbevf_update_stats(adapter);
+}
+
+/**
+ * ixgbevf_service_task - manages and runs subtasks
  * @work: pointer to work_struct containing our data
  **/
-static void ixgbevf_watchdog_task(struct work_struct *work)
+static void ixgbevf_service_task(struct work_struct *work)
 {
 	struct ixgbevf_adapter *adapter = container_of(work,
 						       struct ixgbevf_adapter,
-						       watchdog_task);
+						       service_task);
 	struct ixgbe_hw *hw = &adapter->hw;
 
 	if (IXGBE_REMOVED(hw->hw_addr)) {
@@ -2867,27 +2909,11 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
 	}
 
 	ixgbevf_queue_reset_subtask(adapter);
-
-	adapter->flags |= IXGBE_FLAG_IN_WATCHDOG_TASK;
-
-	ixgbevf_watchdog_update_link(adapter);
-
-	if (adapter->link_up)
-		ixgbevf_watchdog_link_is_up(adapter);
-	else
-		ixgbevf_watchdog_link_is_down(adapter);
-
-	ixgbevf_update_stats(adapter);
-
+	ixgbevf_reset_subtask(adapter);
+	ixgbevf_watchdog_subtask(adapter);
 	ixgbevf_check_hang_subtask(adapter);
 
-	/* Reset the timer */
-	if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
-	    !test_bit(__IXGBEVF_REMOVING, &adapter->state))
-		mod_timer(&adapter->watchdog_timer,
-			  round_jiffies(jiffies + (2 * HZ)));
-
-	adapter->flags &= ~IXGBE_FLAG_IN_WATCHDOG_TASK;
+	ixgbevf_service_event_complete(adapter);
 }
 
 /**
@@ -3994,17 +4020,17 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	netdev->priv_flags |= IFF_UNICAST_FLT;
 
-	init_timer(&adapter->watchdog_timer);
-	adapter->watchdog_timer.function = ixgbevf_watchdog;
-	adapter->watchdog_timer.data = (unsigned long)adapter;
-
 	if (IXGBE_REMOVED(hw->hw_addr)) {
 		err = -EIO;
 		goto err_sw_init;
 	}
-	INIT_WORK(&adapter->reset_task, ixgbevf_reset_task);
-	INIT_WORK(&adapter->watchdog_task, ixgbevf_watchdog_task);
-	set_bit(__IXGBEVF_WORK_INIT, &adapter->state);
+
+	setup_timer(&adapter->service_timer, &ixgbevf_service_timer,
+		    (unsigned long)adapter);
+
+	INIT_WORK(&adapter->service_task, ixgbevf_service_task);
+	set_bit(__IXGBEVF_SERVICE_INITED, &adapter->state);
+	clear_bit(__IXGBEVF_SERVICE_SCHED, &adapter->state);
 
 	err = ixgbevf_init_interrupt_scheme(adapter);
 	if (err)
@@ -4078,11 +4104,7 @@ static void ixgbevf_remove(struct pci_dev *pdev)
 	adapter = netdev_priv(netdev);
 
 	set_bit(__IXGBEVF_REMOVING, &adapter->state);
-
-	del_timer_sync(&adapter->watchdog_timer);
-
-	cancel_work_sync(&adapter->reset_task);
-	cancel_work_sync(&adapter->watchdog_task);
+	cancel_work_sync(&adapter->service_task);
 
 	if (netdev->reg_state == NETREG_REGISTERED)
 		unregister_netdev(netdev);
@@ -4116,7 +4138,7 @@ static pci_ers_result_t ixgbevf_io_error_detected(struct pci_dev *pdev,
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
 
-	if (!test_bit(__IXGBEVF_WORK_INIT, &adapter->state))
+	if (!test_bit(__IXGBEVF_SERVICE_INITED, &adapter->state))
 		return PCI_ERS_RESULT_DISCONNECT;
 
 	rtnl_lock();
-- 
1.9.3

[net-next 16/16] ixgbe: add Tx anti spoofing support

[Jeff Kirsher]

From: Don Skidmore <donald.c.skidmore@intel.com>

This patch enables the ethertype Anti-Spoofing feature for affected
devices. It is configured such that LLDP packets sent by a VF will
be dropped.

Signed-off-by: Don Skidmore <donald.c.skidmore@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/ixgbe/ixgbe.h      |  2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 ++++++++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |  7 +++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 24 ++++++++++++++++++++++++
 4 files changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 699117af..7dcbbec 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -76,6 +76,8 @@
 #define IXGBE_MAX_RXD			   4096
 #define IXGBE_MIN_RXD			     64
 
+#define IXGBE_ETH_P_LLDP		 0x88CC
+
 /* flow control */
 #define IXGBE_MIN_FCRTL			   0x40
 #define IXGBE_MAX_FCRTL			0x7FF80
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 065ff48..e05c846 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3585,10 +3585,24 @@ static void ixgbe_configure_virtualization(struct ixgbe_adapter *adapter)
 	/* Enable MAC Anti-Spoofing */
 	hw->mac.ops.set_mac_anti_spoofing(hw, (adapter->num_vfs != 0),
 					  adapter->num_vfs);
+
+	/* Ensure LLDP is set for Ethertype Antispoofing if we will be
+	 * calling set_ethertype_anti_spoofing for each VF in loop below
+	 */
+	if (hw->mac.ops.set_ethertype_anti_spoofing)
+		IXGBE_WRITE_REG(hw, IXGBE_ETQF(IXGBE_ETQF_FILTER_LLDP),
+				(IXGBE_ETQF_FILTER_EN    | /* enable filter */
+				 IXGBE_ETQF_TX_ANTISPOOF | /* tx antispoof */
+				 IXGBE_ETH_P_LLDP));	   /* LLDP eth type */
+
 	/* For VFs that have spoof checking turned off */
 	for (i = 0; i < adapter->num_vfs; i++) {
 		if (!adapter->vfinfo[i].spoofchk_enabled)
 			ixgbe_ndo_set_vf_spoofchk(adapter->netdev, i, false);
+
+		/* enable ethertype anti spoofing if hw supports it */
+		if (hw->mac.ops.set_ethertype_anti_spoofing)
+			hw->mac.ops.set_ethertype_anti_spoofing(hw, true, i);
 	}
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 38a0e64..0ede315 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -378,6 +378,8 @@ struct ixgbe_thermal_sensor_data {
 #define IXGBE_SPOOF_MACAS_MASK          0xFF
 #define IXGBE_SPOOF_VLANAS_MASK         0xFF00
 #define IXGBE_SPOOF_VLANAS_SHIFT        8
+#define IXGBE_SPOOF_ETHERTYPEAS		0xFF000000
+#define IXGBE_SPOOF_ETHERTYPEAS_SHIFT	16
 #define IXGBE_PFVFSPOOF_REG_COUNT       8
 
 #define IXGBE_DCA_TXCTRL(_i)    (0x07200 + ((_i) * 4)) /* 16 of these (0-15) */
@@ -1541,6 +1543,7 @@ enum {
 #define IXGBE_MAX_ETQF_FILTERS  8
 #define IXGBE_ETQF_FCOE         0x08000000 /* bit 27 */
 #define IXGBE_ETQF_BCN          0x10000000 /* bit 28 */
+#define IXGBE_ETQF_TX_ANTISPOOF	0x20000000 /* bit 29 */
 #define IXGBE_ETQF_1588         0x40000000 /* bit 30 */
 #define IXGBE_ETQF_FILTER_EN    0x80000000 /* bit 31 */
 #define IXGBE_ETQF_POOL_ENABLE   (1 << 26) /* bit 26 */
@@ -1566,6 +1569,9 @@ enum {
 #define IXGBE_ETQF_FILTER_FCOE           2
 #define IXGBE_ETQF_FILTER_1588           3
 #define IXGBE_ETQF_FILTER_FIP            4
+#define IXGBE_ETQF_FILTER_LLDP		 5
+#define IXGBE_ETQF_FILTER_LACP		 6
+
 /* VLAN Control Bit Masks */
 #define IXGBE_VLNCTRL_VET       0x0000FFFF  /* bits 0-15 */
 #define IXGBE_VLNCTRL_CFI       0x10000000  /* bit 28 */
@@ -3061,6 +3067,7 @@ struct ixgbe_mac_operations {
 	s32 (*set_fw_drv_ver)(struct ixgbe_hw *, u8, u8, u8, u8);
 	s32 (*get_thermal_sensor_data)(struct ixgbe_hw *);
 	s32 (*init_thermal_sensor_thresh)(struct ixgbe_hw *hw);
+	void (*set_ethertype_anti_spoofing)(struct ixgbe_hw *, bool, int);
 
 	/* DMA Coalescing */
 	s32 (*dmac_config)(struct ixgbe_hw *hw);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index 84affca..50bf819 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -1300,6 +1300,28 @@ mac_reset_top:
 	return status;
 }
 
+/** ixgbe_set_ethertype_anti_spoofing_X550 - Enable/Disable Ethertype
+ *	anti-spoofing
+ *  @hw:  pointer to hardware structure
+ *  @enable: enable or disable switch for Ethertype anti-spoofing
+ *  @vf: Virtual Function pool - VF Pool to set for Ethertype anti-spoofing
+ **/
+void ixgbe_set_ethertype_anti_spoofing_X550(struct ixgbe_hw *hw, bool enable,
+					    int vf)
+{
+	int vf_target_reg = vf >> 3;
+	int vf_target_shift = vf % 8 + IXGBE_SPOOF_ETHERTYPEAS_SHIFT;
+	u32 pfvfspoof;
+
+	pfvfspoof = IXGBE_READ_REG(hw, IXGBE_PFVFSPOOF(vf_target_reg));
+	if (enable)
+		pfvfspoof |= (1 << vf_target_shift);
+	else
+		pfvfspoof &= ~(1 << vf_target_shift);
+
+	IXGBE_WRITE_REG(hw, IXGBE_PFVFSPOOF(vf_target_reg), pfvfspoof);
+}
+
 #define X550_COMMON_MAC \
 	.init_hw			= &ixgbe_init_hw_generic, \
 	.start_hw			= &ixgbe_start_hw_X540, \
@@ -1334,6 +1356,8 @@ mac_reset_top:
 	.init_uta_tables		= &ixgbe_init_uta_tables_generic, \
 	.set_mac_anti_spoofing		= &ixgbe_set_mac_anti_spoofing, \
 	.set_vlan_anti_spoofing		= &ixgbe_set_vlan_anti_spoofing, \
+	.set_ethertype_anti_spoofing	= \
+				&ixgbe_set_ethertype_anti_spoofing_X550, \
 	.acquire_swfw_sync		= &ixgbe_acquire_swfw_sync_X540, \
 	.release_swfw_sync		= &ixgbe_release_swfw_sync_X540, \
 	.disable_rx_buff		= &ixgbe_disable_rx_buff_generic, \
-- 
1.9.3

RE: [net-next 03/16] ixgbe: cleanup sparse errors in new ixgbe_x550.c file

[David Laight]

From: Jeff Kirsher
> This patch cleans up prototypes that should have been defined
> as static.
> 
> Signed-off-by: Don Skidmore <donald.c.skidmore@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/ixgbe/ixgbe_type.h |  4 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 65 ++++++++++++++-------------
>  2 files changed, 37 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index d101b25..1f301ec 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -2451,8 +2451,8 @@ struct ixgbe_hic_read_shadow_ram {
> 
>  struct ixgbe_hic_write_shadow_ram {
>  	union ixgbe_hic_hdr2 hdr;
> -	u32 address;
> -	u16 length;
> +	__be32 address;
> +	__be16 length;
>  	u16 pad2;
>  	u16 data;
>  	u16 pad3;

Not related to commit message

	David

Re: [net-next 03/16] ixgbe: cleanup sparse errors in new ixgbe_x550.c file

[David Miller]

From: David Laight <David.Laight@ACULAB.COM>
Date: Thu, 5 Feb 2015 09:48:24 +0000

> From: Jeff Kirsher
>> This patch cleans up prototypes that should have been defined
>> as static.
>> 
>> Signed-off-by: Don Skidmore <donald.c.skidmore@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/ixgbe/ixgbe_type.h |  4 +-
>>  drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 65 ++++++++++++++-------------
>>  2 files changed, 37 insertions(+), 32 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> index d101b25..1f301ec 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> @@ -2451,8 +2451,8 @@ struct ixgbe_hic_read_shadow_ram {
>> 
>>  struct ixgbe_hic_write_shadow_ram {
>>  	union ixgbe_hic_hdr2 hdr;
>> -	u32 address;
>> -	u16 length;
>> +	__be32 address;
>> +	__be16 length;
>>  	u16 pad2;
>>  	u16 data;
>>  	u16 pad3;
> 
> Not related to commit message

Agreed, fix this up.

RE: [net-next 03/16] ixgbe: cleanup sparse errors in new ixgbe_x550.c file

[Skidmore, Donald C]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, February 05, 2015 2:40 PM
> To: David.Laight@ACULAB.COM
> Cc: Kirsher, Jeffrey T; Skidmore, Donald C; netdev@vger.kernel.org;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 03/16] ixgbe: cleanup sparse errors in new
> ixgbe_x550.c file
> 
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Thu, 5 Feb 2015 09:48:24 +0000
> 
> > From: Jeff Kirsher
> >> This patch cleans up prototypes that should have been defined as
> >> static.
> >>
> >> Signed-off-by: Don Skidmore <donald.c.skidmore@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/ixgbe/ixgbe_type.h |  4 +-
> >> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 65
> >> ++++++++++++++-------------
> >>  2 files changed, 37 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> index d101b25..1f301ec 100644
> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> >> @@ -2451,8 +2451,8 @@ struct ixgbe_hic_read_shadow_ram {
> >>
> >>  struct ixgbe_hic_write_shadow_ram {
> >>  	union ixgbe_hic_hdr2 hdr;
> >> -	u32 address;
> >> -	u16 length;
> >> +	__be32 address;
> >> +	__be16 length;
> >>  	u16 pad2;
> >>  	u16 data;
> >>  	u16 pad3;
> >
> > Not related to commit message
> 
> Agreed, fix this up.

I'll get a cleaned up patch to Jeff.

Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>

Re: [net-next 05/16] ixgbevf: enable multiple queue support

[Vlad Zolotarov]

On 02/05/15 11:35, Jeff Kirsher wrote:
> From: Emil Tantilov <emil.s.tantilov@intel.com>
>
> This patch enables multiple queues and RSS support for the VF.
> Maximum of 2 queues are supported due to available vectors.
>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>   drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  1 +
>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 +++++++++++++++++++---
>   2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index 8c44ab2..65cda34 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -124,6 +124,7 @@ struct ixgbevf_ring {
>   
>   #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
>   #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
> +#define IXGBEVF_MAX_RSS_QUEUES 2

According to the MRQE register description in 82599 and x540 specs 2 
(the value of IXGBEVF_MAX_RSS_QUEUES) is the minimal allowed number of 
queues in a single pool thus it's a minimal number of VF queues - not 
the maximal (see more comments on this below).

>   
>   #define IXGBEVF_DEFAULT_TXD   1024
>   #define IXGBEVF_DEFAULT_RXD   512
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index c9b49bf..815808f 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>   	struct ixgbe_hw *hw = &adapter->hw;
>   	unsigned int def_q = 0;
>   	unsigned int num_tcs = 0;
> -	unsigned int num_rx_queues = 1;
> +	unsigned int num_rx_queues = adapter->num_rx_queues;
> +	unsigned int num_tx_queues = adapter->num_tx_queues;
>   	int err;
>   
>   	spin_lock_bh(&adapter->mbx_lock);
> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>   		return err;
>   
>   	if (num_tcs > 1) {
> +		/* we need only one Tx queue */
> +		num_tx_queues = 1;
> +
>   		/* update default Tx ring register index */
>   		adapter->tx_ring[0]->reg_idx = def_q;
>   
> @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>   	}
>   
>   	/* if we have a bad config abort request queue reset */
> -	if (adapter->num_rx_queues != num_rx_queues) {
> +	if ((adapter->num_rx_queues != num_rx_queues) ||
> +	    (adapter->num_tx_queues != num_tx_queues)) {
>   		/* force mailbox timeout to prevent further messages */
>   		hw->mbx.timeout = 0;
>   
> @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct ixgbevf_adapter *adapter)
>   		return;
>   
>   	/* we need as many queues as traffic classes */
> -	if (num_tcs > 1)
> +	if (num_tcs > 1) {
>   		adapter->num_rx_queues = num_tcs;
> +	} else {
> +		u16 rss = min_t(u16, num_online_cpus(), IXGBEVF_MAX_RSS_QUEUES);

Emil,
Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored in 
the hw->mac.max_rx_queues). This will accidentally work if PF is driven 
by a current ixgbe Linux driver but what would happen if HV is not a 
Linux box? What if HV decides to configure the PSRTYPE[x].RQPL to 0 and 
decide that VF should be configured with a single queue? I maybe miss 
something here but the above line seems bogus. Please, comment.

thanks,
vlad

> +
> +		switch (hw->api_version) {
> +		case ixgbe_mbox_api_11:
> +			adapter->num_rx_queues = rss;
> +			adapter->num_tx_queues = rss;
> +		default:
> +			break;
> +		}
> +	}
>   }
>   
>   /**

RE: [net-next 05/16] ixgbevf: enable multiple queue support

[Tantilov, Emil S]

>-----Original Message-----
>From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com] 
>Sent: Wednesday, March 25, 2015 1:45 AM
>Subject: Re: [net-next 05/16] ixgbevf: enable multiple queue support
>
>
>On 02/05/15 11:35, Jeff Kirsher wrote:
>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>
> This patch enables multiple queues and RSS support for the VF.
>> Maximum of 2 queues are supported due to available vectors.
>>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>> Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  1 +
>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 +++++++++++++++++++---
>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> index 8c44ab2..65cda34 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> @@ -124,6 +124,7 @@ struct ixgbevf_ring {
>>   
>>   #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
>>   #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
>> +#define IXGBEVF_MAX_RSS_QUEUES 2
>
>According to the MRQE register description in 82599 and x540 specs 2 
> (the value of IXGBEVF_MAX_RSS_QUEUES) is the minimal allowed number of 
>queues in a single pool thus it's a minimal number of VF queues - not 
>the maximal (see more comments on this below).

The VF driver can have a maximum of 2 queues as mentioned in the description.
The driver will attempt to allocate the maximum number of queues allowed.

All of the virtualization enabled modes of MRQC.MRQE have at least 2 queues.

>>   #define IXGBEVF_DEFAULT_TXD   1024
>>   #define IXGBEVF_DEFAULT_RXD   512
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c >>b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> index c9b49bf..815808f 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>>   	struct ixgbe_hw *hw = &adapter->hw;
>>   	unsigned int def_q = 0;
>>   	unsigned int num_tcs = 0;
>> -	unsigned int num_rx_queues = 1;
>> +	unsigned int num_rx_queues = adapter->num_rx_queues;
>> +	unsigned int num_tx_queues = adapter->num_tx_queues;
>>   	int err;
>>   
>>   	spin_lock_bh(&adapter->mbx_lock);
>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>>   		return err;
>>   
>>   	if (num_tcs > 1) {
>> +		/* we need only one Tx queue */
>> +		num_tx_queues = 1;
>> +
>>   		/* update default Tx ring register index */
>>   		adapter->tx_ring[0]->reg_idx = def_q;
>>   
>> @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>>   	}
>>   
>>   	/* if we have a bad config abort request queue reset */
>> -	if (adapter->num_rx_queues != num_rx_queues) {
>> +	if ((adapter->num_rx_queues != num_rx_queues) ||
>> +	    (adapter->num_tx_queues != num_tx_queues)) {
>>   		/* force mailbox timeout to prevent further messages */
>>   		hw->mbx.timeout = 0;
>>   
>> @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct ixgbevf_adapter *adapter)
>>   		return;
>>   
>>   	/* we need as many queues as traffic classes */
>> -	if (num_tcs > 1)
>> +	if (num_tcs > 1) {
>>   		adapter->num_rx_queues = num_tcs;
>> +	} else {
>> +		u16 rss = min_t(u16, num_online_cpus(), IXGBEVF_MAX_RSS_QUEUES);
>
>Emil,
>Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored in 
>the hw->mac.max_rx_queues).

Again the VF driver allocates the maximum number of queues possible it is not driven by the PF.

>This will accidentally work if PF is driven 
>by a current ixgbe Linux driver but what would happen if HV is not a 
>Linux box?

Do you know of a driver that has an issue in this configuration?

>What if HV decides to configure the PSRTYPE[x].RQPL to 0 and 
>decide that VF should be configured with a single queue? I maybe miss 
>something here but the above line seems bogus. Please, comment.

PSRTYPE.RQPL - this field only has meaning when MRQC.MRQE is configured to allow multiple queues, so if I have to guess I would say that if there was such a driver it would be miss-configured, but even then the VF can have 2 queues, RSS just likely won't work.

Thanks,
Emil

Re: [net-next 05/16] ixgbevf: enable multiple queue support

[Vlad Zolotarov]

On 03/25/15 16:32, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Wednesday, March 25, 2015 1:45 AM
>> Subject: Re: [net-next 05/16] ixgbevf: enable multiple queue support
>>
>>
>> On 02/05/15 11:35, Jeff Kirsher wrote:
>>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>>
>> This patch enables multiple queues and RSS support for the VF.
>>> Maximum of 2 queues are supported due to available vectors.
>>>
>>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>>> Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>>    drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  1 +
>>>    drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 +++++++++++++++++++---
>>>    2 files changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> index 8c44ab2..65cda34 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> @@ -124,6 +124,7 @@ struct ixgbevf_ring {
>>>    
>>>    #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
>>>    #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
>>> +#define IXGBEVF_MAX_RSS_QUEUES 2
>> According to the MRQE register description in 82599 and x540 specs 2
>> (the value of IXGBEVF_MAX_RSS_QUEUES) is the minimal allowed number of
>> queues in a single pool thus it's a minimal number of VF queues - not
>> the maximal (see more comments on this below).
> The VF driver can have a maximum of 2 queues as mentioned in the description.
> The driver will attempt to allocate the maximum number of queues allowed.
>
> All of the virtualization enabled modes of MRQC.MRQE have at least 2 queues.

Exactly!
To summarize:

  - 2 is a minimum allowed number of queues that may be assigned to a 
single pool.
  - 2 is currently a maximal number of MSI-X interrupts chosen to be 
allowed to be configured by an ixgbevf driver for a single function.

That's why I said calling it a "maximum number of queues", while it is 
actually a minimum is a bit confusing.

>
>>>    #define IXGBEVF_DEFAULT_TXD   1024
>>>    #define IXGBEVF_DEFAULT_RXD   512
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c >>b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> index c9b49bf..815808f 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>>>    	struct ixgbe_hw *hw = &adapter->hw;
>>>    	unsigned int def_q = 0;
>>>    	unsigned int num_tcs = 0;
>>> -	unsigned int num_rx_queues = 1;
>>> +	unsigned int num_rx_queues = adapter->num_rx_queues;
>>> +	unsigned int num_tx_queues = adapter->num_tx_queues;
>>>    	int err;
>>>    
>>>    	spin_lock_bh(&adapter->mbx_lock);
>>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>>>    		return err;
>>>    
>>>    	if (num_tcs > 1) {
>>> +		/* we need only one Tx queue */
>>> +		num_tx_queues = 1;
>>> +
>>>    		/* update default Tx ring register index */
>>>    		adapter->tx_ring[0]->reg_idx = def_q;
>>>    
>>> @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>>>    	}
>>>    
>>>    	/* if we have a bad config abort request queue reset */
>>> -	if (adapter->num_rx_queues != num_rx_queues) {
>>> +	if ((adapter->num_rx_queues != num_rx_queues) ||
>>> +	    (adapter->num_tx_queues != num_tx_queues)) {
>>>    		/* force mailbox timeout to prevent further messages */
>>>    		hw->mbx.timeout = 0;
>>>    
>>> @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct ixgbevf_adapter *adapter)
>>>    		return;
>>>    
>>>    	/* we need as many queues as traffic classes */
>>> -	if (num_tcs > 1)
>>> +	if (num_tcs > 1) {
>>>    		adapter->num_rx_queues = num_tcs;
>>> +	} else {
>>> +		u16 rss = min_t(u16, num_online_cpus(), IXGBEVF_MAX_RSS_QUEUES);
>> Emil,
>> Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored in
>> the hw->mac.max_rx_queues).
> Again the VF driver allocates the maximum number of queues possible it is not driven by the PF.

AFAIK VF IS driven by a PF in all related to resources configuration 
including the queues assignment. That's why VF driver has to follow the 
configuration PF defines. Otherwise VF HW may just not work as expected 
(at least as expected by the system administrator controlling the 
Hypervisor).

>
>> This will accidentally work if PF is driven
>> by a current ixgbe Linux driver but what would happen if HV is not a
>> Linux box?
> Do you know of a driver that has an issue in this configuration?

Nope, but I know for sure that VF can't ignore what PF tells u and this 
is clearly a bug. I think I have described the possible configuration 
that may lead to VF malfunction below.

>
>> What if HV decides to configure the PSRTYPE[x].RQPL to 0 and
>> decide that VF should be configured with a single queue? I maybe miss
>> something here but the above line seems bogus. Please, comment.
> PSRTYPE.RQPL - this field only has meaning when MRQC.MRQE is configured to allow multiple queues, so if I have to guess I would say that if there was such a driver it would be miss-configured, but even then the VF can have 2 queues, RSS just likely won't work.

That's an absolutely legal configuration and u are missing the main 
point here - VF has to obey by the PF rules which are controlled by the 
Hypervisor's system administrator and administrator may decide (due to 
his/her reasons, e.g. in order to save some memory) to allow only a 
single queue for each VF device and VF device driver is expected to 
listen to this command.

U r right, since u configure the minimum allowed number of queues u are 
most likely won't cause your VF HW to hang (unless I miss anything) 
however u quite break the VF-PF negotiation rules here and as a result 
in the example above RSS won't work and u won't get any traffic on the 
second queue which will surely confuse the user. This is in addition to 
that u r going to waste the resources the system administrator 
explicitly told u not to...

thanks,
vlad

>
> Thanks,
> Emil
>
>
>

Re: [net-next 05/16] ixgbevf: enable multiple queue support

[Alexander Duyck]

On 03/25/2015 01:44 AM, Vlad Zolotarov wrote:
>
>
> On 02/05/15 11:35, Jeff Kirsher wrote:
>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>
>> This patch enables multiple queues and RSS support for the VF.
>> Maximum of 2 queues are supported due to available vectors.
>>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>> Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  1 +
>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 
>> +++++++++++++++++++---
>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h 
>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> index 8c44ab2..65cda34 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>> @@ -124,6 +124,7 @@ struct ixgbevf_ring {
>>     #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
>>   #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
>> +#define IXGBEVF_MAX_RSS_QUEUES 2
>
> According to the MRQE register description in 82599 and x540 specs 2 
> (the value of IXGBEVF_MAX_RSS_QUEUES) is the minimal allowed number of 
> queues in a single pool thus it's a minimal number of VF queues - not 
> the maximal (see more comments on this below).

This is the maximum since the VF only supports 3 interrupts, one for 
mailbox and 2 for queues, and RSS requires an interrupt per queue pair.  
The other possibility is that DCB has been enabled in which case there 
RSS is reduced to one queue pair per pool with one RSS pool for each 
traffic class.

>
>>     #define IXGBEVF_DEFAULT_TXD   1024
>>   #define IXGBEVF_DEFAULT_RXD   512
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c 
>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> index c9b49bf..815808f 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct 
>> ixgbevf_adapter *adapter)
>>       struct ixgbe_hw *hw = &adapter->hw;
>>       unsigned int def_q = 0;
>>       unsigned int num_tcs = 0;
>> -    unsigned int num_rx_queues = 1;
>> +    unsigned int num_rx_queues = adapter->num_rx_queues;
>> +    unsigned int num_tx_queues = adapter->num_tx_queues;
>>       int err;
>>         spin_lock_bh(&adapter->mbx_lock);
>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct 
>> ixgbevf_adapter *adapter)
>>           return err;
>>         if (num_tcs > 1) {
>> +        /* we need only one Tx queue */
>> +        num_tx_queues = 1;
>> +
>>           /* update default Tx ring register index */
>>           adapter->tx_ring[0]->reg_idx = def_q;
>>   @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct 
>> ixgbevf_adapter *adapter)
>>       }
>>         /* if we have a bad config abort request queue reset */
>> -    if (adapter->num_rx_queues != num_rx_queues) {
>> +    if ((adapter->num_rx_queues != num_rx_queues) ||
>> +        (adapter->num_tx_queues != num_tx_queues)) {
>>           /* force mailbox timeout to prevent further messages */
>>           hw->mbx.timeout = 0;
>>   @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct 
>> ixgbevf_adapter *adapter)
>>           return;
>>         /* we need as many queues as traffic classes */
>> -    if (num_tcs > 1)
>> +    if (num_tcs > 1) {
>>           adapter->num_rx_queues = num_tcs;
>> +    } else {
>> +        u16 rss = min_t(u16, num_online_cpus(), 
>> IXGBEVF_MAX_RSS_QUEUES);
>
> Emil,
> Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored in 
> the hw->mac.max_rx_queues). This will accidentally work if PF is 
> driven by a current ixgbe Linux driver but what would happen if HV is 
> not a Linux box? What if HV decides to configure the PSRTYPE[x].RQPL 
> to 0 and decide that VF should be configured with a single queue? I 
> maybe miss something here but the above line seems bogus. Please, comment.

It isn't ignored.  The fact that num_tcs is being tested for checks for 
the one case where the number of RSS queues available would be 1, 
otherwise the hardware will provide at least 2 queues.  If the RETA is 
not configured for 2 queues one of them will not be active, but that 
should not be any sort of actual issue and would be more of an 
administrative choice anyway as it would also limit the PF.

- Alex

Re: [net-next 05/16] ixgbevf: enable multiple queue support

[Vlad Zolotarov]

On 03/25/15 20:30, Alexander Duyck wrote:
> On 03/25/2015 01:44 AM, Vlad Zolotarov wrote:
>>
>>
>> On 02/05/15 11:35, Jeff Kirsher wrote:
>>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>>
>>> This patch enables multiple queues and RSS support for the VF.
>>> Maximum of 2 queues are supported due to available vectors.
>>>
>>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>>> Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  1 +
>>>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 
>>> +++++++++++++++++++---
>>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h 
>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> index 8c44ab2..65cda34 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
>>> @@ -124,6 +124,7 @@ struct ixgbevf_ring {
>>>     #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES
>>>   #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES
>>> +#define IXGBEVF_MAX_RSS_QUEUES 2
>>
>> According to the MRQE register description in 82599 and x540 specs 2 
>> (the value of IXGBEVF_MAX_RSS_QUEUES) is the minimal allowed number 
>> of queues in a single pool thus it's a minimal number of VF queues - 
>> not the maximal (see more comments on this below).
>
> This is the maximum since the VF only supports 3 interrupts, one for 
> mailbox and 2 for queues, and RSS requires an interrupt per queue 
> pair.  The other possibility is that DCB has been enabled in which 
> case there RSS is reduced to one queue pair per pool with one RSS pool 
> for each traffic class.
>
>>
>>>     #define IXGBEVF_DEFAULT_TXD   1024
>>>   #define IXGBEVF_DEFAULT_RXD   512
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c 
>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> index c9b49bf..815808f 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct 
>>> ixgbevf_adapter *adapter)
>>>       struct ixgbe_hw *hw = &adapter->hw;
>>>       unsigned int def_q = 0;
>>>       unsigned int num_tcs = 0;
>>> -    unsigned int num_rx_queues = 1;
>>> +    unsigned int num_rx_queues = adapter->num_rx_queues;
>>> +    unsigned int num_tx_queues = adapter->num_tx_queues;
>>>       int err;
>>>         spin_lock_bh(&adapter->mbx_lock);
>>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct 
>>> ixgbevf_adapter *adapter)
>>>           return err;
>>>         if (num_tcs > 1) {
>>> +        /* we need only one Tx queue */
>>> +        num_tx_queues = 1;
>>> +
>>>           /* update default Tx ring register index */
>>>           adapter->tx_ring[0]->reg_idx = def_q;
>>>   @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct 
>>> ixgbevf_adapter *adapter)
>>>       }
>>>         /* if we have a bad config abort request queue reset */
>>> -    if (adapter->num_rx_queues != num_rx_queues) {
>>> +    if ((adapter->num_rx_queues != num_rx_queues) ||
>>> +        (adapter->num_tx_queues != num_tx_queues)) {
>>>           /* force mailbox timeout to prevent further messages */
>>>           hw->mbx.timeout = 0;
>>>   @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct 
>>> ixgbevf_adapter *adapter)
>>>           return;
>>>         /* we need as many queues as traffic classes */
>>> -    if (num_tcs > 1)
>>> +    if (num_tcs > 1) {
>>>           adapter->num_rx_queues = num_tcs;
>>> +    } else {
>>> +        u16 rss = min_t(u16, num_online_cpus(), 
>>> IXGBEVF_MAX_RSS_QUEUES);
>>
>> Emil,
>> Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored in 
>> the hw->mac.max_rx_queues). This will accidentally work if PF is 
>> driven by a current ixgbe Linux driver but what would happen if HV is 
>> not a Linux box? What if HV decides to configure the PSRTYPE[x].RQPL 
>> to 0 and decide that VF should be configured with a single queue? I 
>> maybe miss something here but the above line seems bogus. Please, 
>> comment.
>
> It isn't ignored.  The fact that num_tcs is being tested for checks 
> for the one case where the number of RSS queues available would be 1, 
> otherwise the hardware will provide at least 2 queues. 

> If the RETA is not configured for 2 queues one of them will not be 
> active, but that should not be any sort of actual issue and would be 
> more of an administrative choice anyway as it would also limit the PF.

This would be an administrative choice ignored by the ixgbevf driver.
Frankly, I don't understand why are we arguing about this for so long? 
It's obvious that the safest and the most correct way to write it would 
be to limit the num_rx_queues by the value returned by PF. It would also 
remove any further questions and it's a one line patch.
However, if u still insist to keep it the way it is then let's drop this 
discussion and move on.

>
> - Alex

Re: [net-next 05/16] ixgbevf: enable multiple queue support

[Alexander Duyck]

On 03/25/2015 12:49 PM, Vlad Zolotarov wrote:
> On 03/25/15 20:30, Alexander Duyck wrote:
>> On 03/25/2015 01:44 AM, Vlad Zolotarov wrote:
>>> On 02/05/15 11:35, Jeff Kirsher wrote:
>>>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>
>>>
>>>>     #define IXGBEVF_DEFAULT_TXD   1024
>>>>   #define IXGBEVF_DEFAULT_RXD   512
>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c 
>>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>>> index c9b49bf..815808f 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct 
>>>> ixgbevf_adapter *adapter)
>>>>       struct ixgbe_hw *hw = &adapter->hw;
>>>>       unsigned int def_q = 0;
>>>>       unsigned int num_tcs = 0;
>>>> -    unsigned int num_rx_queues = 1;
>>>> +    unsigned int num_rx_queues = adapter->num_rx_queues;
>>>> +    unsigned int num_tx_queues = adapter->num_tx_queues;
>>>>       int err;
>>>>         spin_lock_bh(&adapter->mbx_lock);
>>>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct 
>>>> ixgbevf_adapter *adapter)
>>>>           return err;
>>>>         if (num_tcs > 1) {
>>>> +        /* we need only one Tx queue */
>>>> +        num_tx_queues = 1;
>>>> +
>>>>           /* update default Tx ring register index */
>>>>           adapter->tx_ring[0]->reg_idx = def_q;
>>>>   @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct 
>>>> ixgbevf_adapter *adapter)
>>>>       }
>>>>         /* if we have a bad config abort request queue reset */
>>>> -    if (adapter->num_rx_queues != num_rx_queues) {
>>>> +    if ((adapter->num_rx_queues != num_rx_queues) ||
>>>> +        (adapter->num_tx_queues != num_tx_queues)) {
>>>>           /* force mailbox timeout to prevent further messages */
>>>>           hw->mbx.timeout = 0;
>>>>   @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct 
>>>> ixgbevf_adapter *adapter)
>>>>           return;
>>>>         /* we need as many queues as traffic classes */
>>>> -    if (num_tcs > 1)
>>>> +    if (num_tcs > 1) {
>>>>           adapter->num_rx_queues = num_tcs;
>>>> +    } else {
>>>> +        u16 rss = min_t(u16, num_online_cpus(), 
>>>> IXGBEVF_MAX_RSS_QUEUES);
>>>
>>> Emil,
>>> Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored 
>>> in the hw->mac.max_rx_queues). This will accidentally work if PF is 
>>> driven by a current ixgbe Linux driver but what would happen if HV 
>>> is not a Linux box? What if HV decides to configure the 
>>> PSRTYPE[x].RQPL to 0 and decide that VF should be configured with a 
>>> single queue? I maybe miss something here but the above line seems 
>>> bogus. Please, comment.
>>
>> It isn't ignored.  The fact that num_tcs is being tested for checks 
>> for the one case where the number of RSS queues available would be 1, 
>> otherwise the hardware will provide at least 2 queues. 
>
>> If the RETA is not configured for 2 queues one of them will not be 
>> active, but that should not be any sort of actual issue and would be 
>> more of an administrative choice anyway as it would also limit the PF.
>
> This would be an administrative choice ignored by the ixgbevf driver.

What are you talking about?  How is this an administrative choice? When 
you configure SR-IOV one of the decisions is to select the queue 
layout.  There is no layout that gives the VF less than 2 queues.  Your 
argument makes no sense.  That is why this keeps going back and forth.

> Frankly, I don't understand why are we arguing about this for so long? 
> It's obvious that the safest and the most correct way to write it 
> would be to limit the num_rx_queues by the value returned by PF. It 
> would also remove any further questions and it's a one line patch.
> However, if u still insist to keep it the way it is then let's drop 
> this discussion and move on.

There is nothing the PF can do to deny access to the 2 queues that 
belong to the VF, they are there.  All Emil's patch does is give the VF 
the ability to use the 2 queues for RSS, if it decides to, in order to 
make better use of system resources.  If the RETA is populated for only 
one queue then worst case is that the VF is wasting resources on a queue 
that will go unused.

What concerns me is that you keep talking about a hypothetical PF driver 
which quite frankly can't be supported by the hardware.  If there is 
something you are working on that these changes break then I would 
recommend coming forward with the code for it via something such as an 
RFC so we can start a conversation, otherwise there is no way to help 
you since we have no idea what you are talking about.

- Alex

Re: [net-next 05/16] ixgbevf: enable multiple queue support

[Vlad Zolotarov]

On 03/25/15 22:16, Alexander Duyck wrote:
>
> On 03/25/2015 12:49 PM, Vlad Zolotarov wrote:
>> On 03/25/15 20:30, Alexander Duyck wrote:
>>> On 03/25/2015 01:44 AM, Vlad Zolotarov wrote:
>>>> On 02/05/15 11:35, Jeff Kirsher wrote:
>>>>> From: Emil Tantilov <emil.s.tantilov@intel.com>
>>>
>>>>
>>>>>     #define IXGBEVF_DEFAULT_TXD 1024
>>>>>   #define IXGBEVF_DEFAULT_RXD   512
>>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c 
>>>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>>>> index c9b49bf..815808f 100644
>>>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>>>> @@ -1794,7 +1794,8 @@ static int ixgbevf_configure_dcb(struct 
>>>>> ixgbevf_adapter *adapter)
>>>>>       struct ixgbe_hw *hw = &adapter->hw;
>>>>>       unsigned int def_q = 0;
>>>>>       unsigned int num_tcs = 0;
>>>>> -    unsigned int num_rx_queues = 1;
>>>>> +    unsigned int num_rx_queues = adapter->num_rx_queues;
>>>>> +    unsigned int num_tx_queues = adapter->num_tx_queues;
>>>>>       int err;
>>>>>         spin_lock_bh(&adapter->mbx_lock);
>>>>> @@ -1808,6 +1809,9 @@ static int ixgbevf_configure_dcb(struct 
>>>>> ixgbevf_adapter *adapter)
>>>>>           return err;
>>>>>         if (num_tcs > 1) {
>>>>> +        /* we need only one Tx queue */
>>>>> +        num_tx_queues = 1;
>>>>> +
>>>>>           /* update default Tx ring register index */
>>>>>           adapter->tx_ring[0]->reg_idx = def_q;
>>>>>   @@ -1816,7 +1820,8 @@ static int ixgbevf_configure_dcb(struct 
>>>>> ixgbevf_adapter *adapter)
>>>>>       }
>>>>>         /* if we have a bad config abort request queue reset */
>>>>> -    if (adapter->num_rx_queues != num_rx_queues) {
>>>>> +    if ((adapter->num_rx_queues != num_rx_queues) ||
>>>>> +        (adapter->num_tx_queues != num_tx_queues)) {
>>>>>           /* force mailbox timeout to prevent further messages */
>>>>>           hw->mbx.timeout = 0;
>>>>>   @@ -2181,8 +2186,19 @@ static void ixgbevf_set_num_queues(struct 
>>>>> ixgbevf_adapter *adapter)
>>>>>           return;
>>>>>         /* we need as many queues as traffic classes */
>>>>> -    if (num_tcs > 1)
>>>>> +    if (num_tcs > 1) {
>>>>>           adapter->num_rx_queues = num_tcs;
>>>>> +    } else {
>>>>> +        u16 rss = min_t(u16, num_online_cpus(), 
>>>>> IXGBEVF_MAX_RSS_QUEUES);
>>>>
>>>> Emil,
>>>> Here u ignore the IXGBE_VF_RX_QUEUES value returned the PF (stored 
>>>> in the hw->mac.max_rx_queues). This will accidentally work if PF is 
>>>> driven by a current ixgbe Linux driver but what would happen if HV 
>>>> is not a Linux box? What if HV decides to configure the 
>>>> PSRTYPE[x].RQPL to 0 and decide that VF should be configured with a 
>>>> single queue? I maybe miss something here but the above line seems 
>>>> bogus. Please, comment.
>>>
>>> It isn't ignored.  The fact that num_tcs is being tested for checks 
>>> for the one case where the number of RSS queues available would be 
>>> 1, otherwise the hardware will provide at least 2 queues. 
>>
>>> If the RETA is not configured for 2 queues one of them will not be 
>>> active, but that should not be any sort of actual issue and would be 
>>> more of an administrative choice anyway as it would also limit the PF.
>>
>> This would be an administrative choice ignored by the ixgbevf driver.
>
> What are you talking about?  How is this an administrative choice? 
> When you configure SR-IOV one of the decisions is to select the queue 
> layout.

And another administrative choice may be to allow the VF to use only a 
single queue due to reasons not related to Intel HW at all. I've told 
about it before.

> There is no layout that gives the VF less than 2 queues.  Your 
> argument makes no sense.  That is why this keeps going back and forth.

My argument makes perfect sense and u would see it if u stopped thinking 
only about the ixgbe when we talk about the PF driver. One day PF driver 
may be able to get the limitation of the VF queues number from the 
system administrator and it may decide to limit it by 1.

>
>> Frankly, I don't understand why are we arguing about this for so 
>> long? It's obvious that the safest and the most correct way to write 
>> it would be to limit the num_rx_queues by the value returned by PF. 
>> It would also remove any further questions and it's a one line patch.
>> However, if u still insist to keep it the way it is then let's drop 
>> this discussion and move on.
>
> There is nothing the PF can do to deny access to the 2 queues that 
> belong to the VF, they are there.  All Emil's patch does is give the 
> VF the ability to use the 2 queues for RSS, if it decides to, in order 
> to make better use of system resources.  If the RETA is populated for 
> only one queue then worst case is that the VF is wasting resources on 
> a queue that will go unused.
>
> What concerns me is that you keep talking about a hypothetical PF 
> driver which quite frankly can't be supported by the hardware.  If 
> there is something you are working on that these changes break then I 
> would recommend coming forward with the code for it via something such 
> as an RFC so we can start a conversation, otherwise there is no way to 
> help you since we have no idea what you are talking about.

Alex, I don't need any help - on the contrary, I'm trying to help u by 
reviewing your code. For me the problem is clear - u clearly ignore your 
own design: u query the PF about the VF queues number and ignore it. U 
won't utilize the system resources any worse if u wouldn't ignore it but 
If u think this is ok - then be it. It would be u, guys, that would have 
to deal with it when and if u get any complains from the users.

I have nothing to add any more here. Do whatever u think is right for u.

Best regards,
vlad

>
> - Alex