* [net-next 03/15] e1000e: Invalid Image CSUM bit changed for I217
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
On I217, the bit that indicates an invalid EEPROM (NVM) image checksum has
changed from previous ICH/PCH LOMs. When validating the EEPROM checksum,
check the appropriate bit on different devices.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/defines.h | 4 ++++
drivers/net/ethernet/intel/e1000e/ich8lan.c | 29 +++++++++++++++++++++--------
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 2073532..7326ea2 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -639,6 +639,10 @@
/* NVM Word Offsets */
#define NVM_COMPAT 0x0003
#define NVM_ID_LED_SETTINGS 0x0004
+#define NVM_FUTURE_INIT_WORD1 0x0019
+#define NVM_COMPAT_VALID_CSUM 0x0001
+#define NVM_FUTURE_INIT_WORD1_VALID_CSUM 0x0040
+
#define NVM_INIT_CONTROL2_REG 0x000F
#define NVM_INIT_CONTROL3_PORT_B 0x0014
#define NVM_INIT_3GIO_3 0x001A
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 9763365..7d5f6b7 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -2949,19 +2949,32 @@ static s32 e1000_validate_nvm_checksum_ich8lan(struct e1000_hw *hw)
{
s32 ret_val;
u16 data;
+ u16 word;
+ u16 valid_csum_mask;
- /* Read 0x19 and check bit 6. If this bit is 0, the checksum
- * needs to be fixed. This bit is an indication that the NVM
- * was prepared by OEM software and did not calculate the
- * checksum...a likely scenario.
+ /* Read NVM and check Invalid Image CSUM bit. If this bit is 0,
+ * the checksum needs to be fixed. This bit is an indication that
+ * the NVM was prepared by OEM software and did not calculate
+ * the checksum...a likely scenario.
*/
- ret_val = e1000_read_nvm(hw, 0x19, 1, &data);
+ switch (hw->mac.type) {
+ case e1000_pch_lpt:
+ word = NVM_COMPAT;
+ valid_csum_mask = NVM_COMPAT_VALID_CSUM;
+ break;
+ default:
+ word = NVM_FUTURE_INIT_WORD1;
+ valid_csum_mask = NVM_FUTURE_INIT_WORD1_VALID_CSUM;
+ break;
+ }
+
+ ret_val = e1000_read_nvm(hw, word, 1, &data);
if (ret_val)
return ret_val;
- if (!(data & 0x40)) {
- data |= 0x40;
- ret_val = e1000_write_nvm(hw, 0x19, 1, &data);
+ if (!(data & valid_csum_mask)) {
+ data |= valid_csum_mask;
+ ret_val = e1000_write_nvm(hw, word, 1, &data);
if (ret_val)
return ret_val;
ret_val = e1000e_update_nvm_checksum(hw);
--
1.7.11.7
^ permalink raw reply related
* [net-next 04/15] e1000e: helper functions for accessing EMI registers
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
The Extended Management Interface (EMI) registers are accessed by first
writing the EMI register offset to the EMI_ADDR regiter and then either
reading or writing the data to/from the EMI_DATA register. Add helper
functions for performing these steps and convert existing EMI register
accesses accordingly.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 84 ++++++++++++++++++++++-------
1 file changed, 65 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 7d5f6b7..8004d71 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -153,7 +153,7 @@
#define I82579_LPI_CTRL_ENABLE_MASK 0x6000
#define I82579_LPI_CTRL_FORCE_PLL_LOCK_COUNT 0x80
-/* EMI Registers */
+/* Extended Management Interface (EMI) Registers */
#define I82579_EMI_ADDR 0x10
#define I82579_EMI_DATA 0x11
#define I82579_LPI_UPDATE_TIMER 0x4805 /* in 40ns units + 40 ns base value */
@@ -789,6 +789,58 @@ static s32 e1000_init_mac_params_ich8lan(struct e1000_hw *hw)
}
/**
+ * __e1000_access_emi_reg_locked - Read/write EMI register
+ * @hw: pointer to the HW structure
+ * @addr: EMI address to program
+ * @data: pointer to value to read/write from/to the EMI address
+ * @read: boolean flag to indicate read or write
+ *
+ * This helper function assumes the SW/FW/HW Semaphore is already acquired.
+ **/
+static s32 __e1000_access_emi_reg_locked(struct e1000_hw *hw, u16 address,
+ u16 *data, bool read)
+{
+ s32 ret_val = 0;
+
+ ret_val = e1e_wphy_locked(hw, I82579_EMI_ADDR, address);
+ if (ret_val)
+ return ret_val;
+
+ if (read)
+ ret_val = e1e_rphy_locked(hw, I82579_EMI_DATA, data);
+ else
+ ret_val = e1e_wphy_locked(hw, I82579_EMI_DATA, *data);
+
+ return ret_val;
+}
+
+/**
+ * e1000_read_emi_reg_locked - Read Extended Management Interface register
+ * @hw: pointer to the HW structure
+ * @addr: EMI address to program
+ * @data: value to be read from the EMI address
+ *
+ * Assumes the SW/FW/HW Semaphore is already acquired.
+ **/
+static s32 e1000_read_emi_reg_locked(struct e1000_hw *hw, u16 addr, u16 *data)
+{
+ return __e1000_access_emi_reg_locked(hw, addr, data, true);
+}
+
+/**
+ * e1000_write_emi_reg_locked - Write Extended Management Interface register
+ * @hw: pointer to the HW structure
+ * @addr: EMI address to program
+ * @data: value to be written to the EMI address
+ *
+ * Assumes the SW/FW/HW Semaphore is already acquired.
+ **/
+static s32 e1000_write_emi_reg_locked(struct e1000_hw *hw, u16 addr, u16 data)
+{
+ return __e1000_access_emi_reg_locked(hw, addr, &data, false);
+}
+
+/**
* e1000_set_eee_pchlan - Enable/disable EEE support
* @hw: pointer to the HW structure
*
@@ -823,11 +875,11 @@ static s32 e1000_set_eee_pchlan(struct e1000_hw *hw)
ret_val = hw->phy.ops.acquire(hw);
if (ret_val)
return ret_val;
- ret_val = e1e_wphy_locked(hw, I82579_EMI_ADDR,
- I217_EEE_LP_ABILITY);
+ ret_val = e1000_read_emi_reg_locked(hw,
+ I217_EEE_LP_ABILITY,
+ &dev_spec->eee_lp_ability);
if (ret_val)
goto release;
- e1e_rphy_locked(hw, I82579_EMI_DATA, &dev_spec->eee_lp_ability);
/* EEE is not supported in 100Half, so ignore partner's EEE
* in 100 ability if full-duplex is not advertised.
@@ -1987,18 +2039,12 @@ static s32 e1000_lv_phy_workarounds_ich8lan(struct e1000_hw *hw)
ret_val = hw->phy.ops.acquire(hw);
if (ret_val)
return ret_val;
- ret_val = e1e_wphy_locked(hw, I82579_EMI_ADDR, I82579_MSE_THRESHOLD);
- if (ret_val)
- goto release;
/* set MSE higher to enable link to stay up when noise is high */
- ret_val = e1e_wphy_locked(hw, I82579_EMI_DATA, 0x0034);
- if (ret_val)
- goto release;
- ret_val = e1e_wphy_locked(hw, I82579_EMI_ADDR, I82579_MSE_LINK_DOWN);
+ ret_val = e1000_write_emi_reg_locked(hw, I82579_MSE_THRESHOLD, 0x0034);
if (ret_val)
goto release;
/* drop link after 5 times MSE threshold was reached */
- ret_val = e1e_wphy_locked(hw, I82579_EMI_DATA, 0x0005);
+ ret_val = e1000_write_emi_reg_locked(hw, I82579_MSE_LINK_DOWN, 0x0005);
release:
hw->phy.ops.release(hw);
@@ -2172,10 +2218,9 @@ static s32 e1000_post_phy_reset_ich8lan(struct e1000_hw *hw)
ret_val = hw->phy.ops.acquire(hw);
if (ret_val)
return ret_val;
- ret_val = e1e_wphy_locked(hw, I82579_EMI_ADDR,
- I82579_LPI_UPDATE_TIMER);
- if (!ret_val)
- ret_val = e1e_wphy_locked(hw, I82579_EMI_DATA, 0x1387);
+ ret_val = e1000_write_emi_reg_locked(hw,
+ I82579_LPI_UPDATE_TIMER,
+ 0x1387);
hw->phy.ops.release(hw);
}
@@ -4013,11 +4058,12 @@ void e1000_suspend_workarounds_ich8lan(struct e1000_hw *hw)
if (!dev_spec->eee_disable) {
u16 eee_advert;
- ret_val = e1e_wphy_locked(hw, I82579_EMI_ADDR,
- I217_EEE_ADVERTISEMENT);
+ ret_val =
+ e1000_read_emi_reg_locked(hw,
+ I217_EEE_ADVERTISEMENT,
+ &eee_advert);
if (ret_val)
goto release;
- e1e_rphy_locked(hw, I82579_EMI_DATA, &eee_advert);
/* Disable LPLU if both link partners support 100BaseT
* EEE and 100Full is advertised on both ends of the
--
1.7.11.7
^ permalink raw reply related
* [net-next 05/15] e1000e: 82577: workaround for link drop issue
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
When connected to certain switches, the 82577 PHY might drop link
unexpectedly. Work around the issue by setting the Mean Square Error
higher than the hardware default.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 8004d71..a7d6c87 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -157,7 +157,8 @@
#define I82579_EMI_ADDR 0x10
#define I82579_EMI_DATA 0x11
#define I82579_LPI_UPDATE_TIMER 0x4805 /* in 40ns units + 40 ns base value */
-#define I82579_MSE_THRESHOLD 0x084F /* Mean Square Error Threshold */
+#define I82579_MSE_THRESHOLD 0x084F /* 82579 Mean Square Error Threshold */
+#define I82577_MSE_THRESHOLD 0x0887 /* 82577 Mean Square Error Threshold */
#define I82579_MSE_LINK_DOWN 0x2411 /* MSE count before dropping link */
#define I217_EEE_ADVERTISEMENT 0x8001 /* IEEE MMD Register 7.60 */
#define I217_EEE_LP_ABILITY 0x8002 /* IEEE MMD Register 7.61 */
@@ -1809,6 +1810,11 @@ static s32 e1000_hv_phy_workarounds_ich8lan(struct e1000_hw *hw)
if (ret_val)
goto release;
ret_val = e1e_wphy_locked(hw, BM_PORT_GEN_CFG, phy_data & 0x00FF);
+ if (ret_val)
+ goto release;
+
+ /* set MSE higher to enable link to stay up when noise is high */
+ ret_val = e1000_write_emi_reg_locked(hw, I82577_MSE_THRESHOLD, 0x0034);
release:
hw->phy.ops.release(hw);
--
1.7.11.7
^ permalink raw reply related
* [net-next 06/15] e1000e: fix enabling of EEE on 82579 and I217
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
Energy Efficient Ethernet on 82579 and I217 should only be enabled if not
disabled by the user, if the link is full duplex and the link partner has
similar EEE capabilities (stored in different EMI registers on the two
different parts).
After enabling EEE, read the IEEE MMD register 3.1 (which is also stored in
different EMI registers on the two different parts) to clear the count of
received Tx/Rx LPI indications.
Also, rename I217_EEE_100_SUPPORTED to I82579_EEE_100_SUPPORTED to indicate
the bit is valid starting with I82579 (released before I217).
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 94 ++++++++++++++++++++---------
1 file changed, 65 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index a7d6c87..15d9314 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -150,6 +150,8 @@
/* PHY Low Power Idle Control */
#define I82579_LPI_CTRL PHY_REG(772, 20)
+#define I82579_LPI_CTRL_100_ENABLE 0x2000
+#define I82579_LPI_CTRL_1000_ENABLE 0x4000
#define I82579_LPI_CTRL_ENABLE_MASK 0x6000
#define I82579_LPI_CTRL_FORCE_PLL_LOCK_COUNT 0x80
@@ -160,9 +162,13 @@
#define I82579_MSE_THRESHOLD 0x084F /* 82579 Mean Square Error Threshold */
#define I82577_MSE_THRESHOLD 0x0887 /* 82577 Mean Square Error Threshold */
#define I82579_MSE_LINK_DOWN 0x2411 /* MSE count before dropping link */
+#define I82579_EEE_PCS_STATUS 0x182D /* IEEE MMD Register 3.1 >> 8 */
+#define I82579_EEE_LP_ABILITY 0x040F /* IEEE MMD Register 7.61 */
+#define I82579_EEE_100_SUPPORTED (1 << 1) /* 100BaseTx EEE supported */
+#define I82579_EEE_1000_SUPPORTED (1 << 2) /* 1000BaseTx EEE supported */
+#define I217_EEE_PCS_STATUS 0x9401 /* IEEE MMD Register 3.1 */
#define I217_EEE_ADVERTISEMENT 0x8001 /* IEEE MMD Register 7.60 */
#define I217_EEE_LP_ABILITY 0x8002 /* IEEE MMD Register 7.61 */
-#define I217_EEE_100_SUPPORTED (1 << 1) /* 100BaseTx EEE supported */
/* Intel Rapid Start Technology Support */
#define I217_PROXY_CTRL BM_PHY_REG(BM_WUC_PAGE, 70)
@@ -845,54 +851,84 @@ static s32 e1000_write_emi_reg_locked(struct e1000_hw *hw, u16 addr, u16 data)
* e1000_set_eee_pchlan - Enable/disable EEE support
* @hw: pointer to the HW structure
*
- * Enable/disable EEE based on setting in dev_spec structure. The bits in
- * the LPI Control register will remain set only if/when link is up.
+ * Enable/disable EEE based on setting in dev_spec structure, the duplex of
+ * the link and the EEE capabilities of the link partner. The LPI Control
+ * register bits will remain set only if/when link is up.
**/
static s32 e1000_set_eee_pchlan(struct e1000_hw *hw)
{
struct e1000_dev_spec_ich8lan *dev_spec = &hw->dev_spec.ich8lan;
- s32 ret_val = 0;
- u16 phy_reg;
+ s32 ret_val;
+ u16 lpi_ctrl;
if ((hw->phy.type != e1000_phy_82579) &&
(hw->phy.type != e1000_phy_i217))
return 0;
- ret_val = e1e_rphy(hw, I82579_LPI_CTRL, &phy_reg);
+ ret_val = hw->phy.ops.acquire(hw);
if (ret_val)
return ret_val;
- if (dev_spec->eee_disable)
- phy_reg &= ~I82579_LPI_CTRL_ENABLE_MASK;
- else
- phy_reg |= I82579_LPI_CTRL_ENABLE_MASK;
-
- ret_val = e1e_wphy(hw, I82579_LPI_CTRL, phy_reg);
+ ret_val = e1e_rphy_locked(hw, I82579_LPI_CTRL, &lpi_ctrl);
if (ret_val)
- return ret_val;
+ goto release;
+
+ /* Clear bits that enable EEE in various speeds */
+ lpi_ctrl &= ~I82579_LPI_CTRL_ENABLE_MASK;
+
+ /* Enable EEE if not disabled by user */
+ if (!dev_spec->eee_disable) {
+ u16 lpa, pcs_status, data;
- if ((hw->phy.type == e1000_phy_i217) && !dev_spec->eee_disable) {
/* Save off link partner's EEE ability */
- ret_val = hw->phy.ops.acquire(hw);
- if (ret_val)
- return ret_val;
- ret_val = e1000_read_emi_reg_locked(hw,
- I217_EEE_LP_ABILITY,
+ switch (hw->phy.type) {
+ case e1000_phy_82579:
+ lpa = I82579_EEE_LP_ABILITY;
+ pcs_status = I82579_EEE_PCS_STATUS;
+ break;
+ case e1000_phy_i217:
+ lpa = I217_EEE_LP_ABILITY;
+ pcs_status = I217_EEE_PCS_STATUS;
+ break;
+ default:
+ ret_val = -E1000_ERR_PHY;
+ goto release;
+ }
+ ret_val = e1000_read_emi_reg_locked(hw, lpa,
&dev_spec->eee_lp_ability);
if (ret_val)
goto release;
- /* EEE is not supported in 100Half, so ignore partner's EEE
- * in 100 ability if full-duplex is not advertised.
+ /* Enable EEE only for speeds in which the link partner is
+ * EEE capable.
*/
- e1e_rphy_locked(hw, PHY_LP_ABILITY, &phy_reg);
- if (!(phy_reg & NWAY_LPAR_100TX_FD_CAPS))
- dev_spec->eee_lp_ability &= ~I217_EEE_100_SUPPORTED;
-release:
- hw->phy.ops.release(hw);
+ if (dev_spec->eee_lp_ability & I82579_EEE_1000_SUPPORTED)
+ lpi_ctrl |= I82579_LPI_CTRL_1000_ENABLE;
+
+ if (dev_spec->eee_lp_ability & I82579_EEE_100_SUPPORTED) {
+ e1e_rphy_locked(hw, PHY_LP_ABILITY, &data);
+ if (data & NWAY_LPAR_100TX_FD_CAPS)
+ lpi_ctrl |= I82579_LPI_CTRL_100_ENABLE;
+ else
+ /* EEE is not supported in 100Half, so ignore
+ * partner's EEE in 100 ability if full-duplex
+ * is not advertised.
+ */
+ dev_spec->eee_lp_ability &=
+ ~I82579_EEE_100_SUPPORTED;
+ }
+
+ /* R/Clr IEEE MMD 3.1 bits 11:10 - Tx/Rx LPI Received */
+ ret_val = e1000_read_emi_reg_locked(hw, pcs_status, &data);
+ if (ret_val)
+ goto release;
}
- return 0;
+ ret_val = e1e_wphy_locked(hw, I82579_LPI_CTRL, lpi_ctrl);
+release:
+ hw->phy.ops.release(hw);
+
+ return ret_val;
}
/**
@@ -4075,9 +4111,9 @@ void e1000_suspend_workarounds_ich8lan(struct e1000_hw *hw)
* EEE and 100Full is advertised on both ends of the
* link.
*/
- if ((eee_advert & I217_EEE_100_SUPPORTED) &&
+ if ((eee_advert & I82579_EEE_100_SUPPORTED) &&
(dev_spec->eee_lp_ability &
- I217_EEE_100_SUPPORTED) &&
+ I82579_EEE_100_SUPPORTED) &&
(hw->phy.autoneg_advertised & ADVERTISE_100_FULL))
phy_ctrl &= ~(E1000_PHY_CTRL_D0A_LPLU |
E1000_PHY_CTRL_NOND0A_LPLU);
--
1.7.11.7
^ permalink raw reply related
* [net-next 07/15] e1000e: unexpected "Reset adapter" message when cable pulled
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
When there is heavy traffic and the cable is pulled, the driver must reset
the adapter to flush the Tx queue in hardware. This causes the reset path
to be scheduled and logs the message "Reset adapter" which could be mis-
interpreted as an error by the user. Change how the reset path is invoked
for this scenario by using the same method done in an existing work-around
for 80003es2lan (i.e. set a flag and if the flag is set in the reset code
do not log the "Reset adapter" message since the reset is expected).
Re-name the FLAG_RX_RESTART_NOW to FLAG_RESTART_NOW since it is used for
resets in both the Rx and Tx specific code.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 2 +-
drivers/net/ethernet/intel/e1000e/netdev.c | 39 +++++++++++++++---------------
2 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 6782a2e..3f8bbc3 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -447,7 +447,7 @@ struct e1000_info {
#define FLAG_MSI_ENABLED (1 << 27)
/* reserved (1 << 28) */
#define FLAG_TSO_FORCE (1 << 29)
-#define FLAG_RX_RESTART_NOW (1 << 30)
+#define FLAG_RESTART_NOW (1 << 30)
#define FLAG_MSI_TEST_FAILED (1 << 31)
#define FLAG2_CRC_STRIPPING (1 << 0)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 337644d..7ad8ef6 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1671,7 +1671,7 @@ static irqreturn_t e1000_intr_msi(int irq, void *data)
/* disable receives */
u32 rctl = er32(RCTL);
ew32(RCTL, rctl & ~E1000_RCTL_EN);
- adapter->flags |= FLAG_RX_RESTART_NOW;
+ adapter->flags |= FLAG_RESTART_NOW;
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
@@ -1734,7 +1734,7 @@ static irqreturn_t e1000_intr(int irq, void *data)
/* disable receives */
rctl = er32(RCTL);
ew32(RCTL, rctl & ~E1000_RCTL_EN);
- adapter->flags |= FLAG_RX_RESTART_NOW;
+ adapter->flags |= FLAG_RESTART_NOW;
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
@@ -3013,7 +3013,7 @@ static void e1000_setup_rctl(struct e1000_adapter *adapter)
ew32(RCTL, rctl);
/* just started the receive unit, no need to restart */
- adapter->flags &= ~FLAG_RX_RESTART_NOW;
+ adapter->flags &= ~FLAG_RESTART_NOW;
}
/**
@@ -4355,11 +4355,11 @@ static void e1000e_enable_receives(struct e1000_adapter *adapter)
{
/* make sure the receive unit is started */
if ((adapter->flags & FLAG_RX_NEEDS_RESTART) &&
- (adapter->flags & FLAG_RX_RESTART_NOW)) {
+ (adapter->flags & FLAG_RESTART_NOW)) {
struct e1000_hw *hw = &adapter->hw;
u32 rctl = er32(RCTL);
ew32(RCTL, rctl | E1000_RCTL_EN);
- adapter->flags &= ~FLAG_RX_RESTART_NOW;
+ adapter->flags &= ~FLAG_RESTART_NOW;
}
}
@@ -4528,8 +4528,16 @@ static void e1000_watchdog_task(struct work_struct *work)
mod_timer(&adapter->phy_info_timer,
round_jiffies(jiffies + 2 * HZ));
- if (adapter->flags & FLAG_RX_NEEDS_RESTART)
- schedule_work(&adapter->reset_task);
+ /* The link is lost so the controller stops DMA.
+ * If there is queued Tx work that cannot be done
+ * or if on an 8000ES2LAN which requires a Rx packet
+ * buffer work-around on link down event, reset the
+ * controller to flush the Tx/Rx packet buffers.
+ * (Do the reset outside of interrupt context).
+ */
+ if ((adapter->flags & FLAG_RX_NEEDS_RESTART) ||
+ (e1000_desc_unused(tx_ring) + 1 < tx_ring->count))
+ adapter->flags |= FLAG_RESTART_NOW;
else
pm_schedule_suspend(netdev->dev.parent,
LINK_TIMEOUT);
@@ -4551,20 +4559,14 @@ link_up:
adapter->gotc_old = adapter->stats.gotc;
spin_unlock(&adapter->stats64_lock);
- e1000e_update_adaptive(&adapter->hw);
-
- if (!netif_carrier_ok(netdev) &&
- (e1000_desc_unused(tx_ring) + 1 < tx_ring->count)) {
- /* We've lost link, so the controller stops DMA,
- * but we've got queued Tx work that's never going
- * to get done, so reset controller to flush Tx.
- * (Do the reset outside of interrupt context).
- */
+ if (adapter->flags & FLAG_RESTART_NOW) {
schedule_work(&adapter->reset_task);
/* return immediately since reset is imminent */
return;
}
+ e1000e_update_adaptive(&adapter->hw);
+
/* Simple mode for Interrupt Throttle Rate (ITR) */
if (adapter->itr_setting == 4) {
/* Symmetric Tx/Rx gets a reduced ITR=2000;
@@ -5134,10 +5136,9 @@ static void e1000_reset_task(struct work_struct *work)
if (test_bit(__E1000_DOWN, &adapter->state))
return;
- if (!((adapter->flags & FLAG_RX_NEEDS_RESTART) &&
- (adapter->flags & FLAG_RX_RESTART_NOW))) {
+ if (!(adapter->flags & FLAG_RESTART_NOW)) {
e1000e_dump(adapter);
- e_err("Reset adapter\n");
+ e_err("Reset adapter unexpectedly\n");
}
e1000e_reinit_locked(adapter);
}
--
1.7.11.7
^ permalink raw reply related
* [net-next 08/15] e1000e: add missing bailout on error
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
...discovered during code inspection.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 15d9314..051dfda 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -2077,6 +2077,8 @@ static s32 e1000_lv_phy_workarounds_ich8lan(struct e1000_hw *hw)
/* Set MDIO slow mode before any other MDIO access */
ret_val = e1000_set_mdio_slow_mode_hv(hw);
+ if (ret_val)
+ return ret_val;
ret_val = hw->phy.ops.acquire(hw);
if (ret_val)
--
1.7.11.7
^ permalink raw reply related
* [net-next v2 09/15] e1000e: resolve checkpatch PREFER_PR_LEVEL warning
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Joe Perches, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ...
then pr_info(... to printk(KERN_INFO ...
v2 - remove unnecessary "e1000e:" prefix as pointed out by Joe Perches
since that produces a redundant "e1000e:" in the log message
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 7ad8ef6..b22eb04 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4300,9 +4300,8 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
u32 ctrl = er32(CTRL);
/* Link status message must follow this format for user tools */
- printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
- adapter->netdev->name,
- adapter->link_speed,
+ pr_info("%s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
+ adapter->netdev->name, adapter->link_speed,
adapter->link_duplex == FULL_DUPLEX ? "Full" : "Half",
(ctrl & E1000_CTRL_TFCE) && (ctrl & E1000_CTRL_RFCE) ? "Rx/Tx" :
(ctrl & E1000_CTRL_RFCE) ? "Rx" :
@@ -4521,8 +4520,7 @@ static void e1000_watchdog_task(struct work_struct *work)
adapter->link_speed = 0;
adapter->link_duplex = 0;
/* Link status message must follow this format */
- printk(KERN_INFO "e1000e: %s NIC Link is Down\n",
- adapter->netdev->name);
+ pr_info("%s NIC Link is Down\n", adapter->netdev->name);
netif_carrier_off(netdev);
if (!test_bit(__E1000_DOWN, &adapter->state))
mod_timer(&adapter->phy_info_timer,
--
1.7.11.7
^ permalink raw reply related
* [net-next 10/15] e1000e: cleanup redundant statistics counter
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
rx_long_byte_count can be removed since it is duplicated in rx_bytes
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ethtool.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index f95bc6e..2225603 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -98,7 +98,6 @@ static const struct e1000_stats e1000_gstrings_stats[] = {
E1000_STAT("rx_flow_control_xoff", stats.xoffrxc),
E1000_STAT("tx_flow_control_xon", stats.xontxc),
E1000_STAT("tx_flow_control_xoff", stats.xofftxc),
- E1000_STAT("rx_long_byte_count", stats.gorc),
E1000_STAT("rx_csum_offload_good", hw_csum_good),
E1000_STAT("rx_csum_offload_errors", hw_csum_err),
E1000_STAT("rx_header_split", rx_hdr_split),
--
1.7.11.7
^ permalink raw reply related
* [net-next 11/15] e1000e: cleanup unusual comment placement
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index b22eb04..18790a6 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -765,7 +765,7 @@ static void e1000_alloc_jumbo_rx_buffers(struct e1000_ring *rx_ring,
struct e1000_buffer *buffer_info;
struct sk_buff *skb;
unsigned int i;
- unsigned int bufsz = 256 - 16 /* for skb_reserve */;
+ unsigned int bufsz = 256 - 16; /* for skb_reserve */
i = rx_ring->next_to_use;
buffer_info = &rx_ring->buffer_info[i];
--
1.7.11.7
^ permalink raw reply related
* [net-next 12/15] e1000e: cleanup unnecessary line wrap
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/param.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c
index 89d536d..b29a8a5 100644
--- a/drivers/net/ethernet/intel/e1000e/param.c
+++ b/drivers/net/ethernet/intel/e1000e/param.c
@@ -447,8 +447,7 @@ void e1000e_check_options(struct e1000_adapter *adapter)
if (num_SmartPowerDownEnable > bd) {
unsigned int spd = SmartPowerDownEnable[bd];
e1000_validate_option(&spd, &opt, adapter);
- if ((adapter->flags & FLAG_HAS_SMART_POWER_DOWN)
- && spd)
+ if ((adapter->flags & FLAG_HAS_SMART_POWER_DOWN) && spd)
adapter->flags |= FLAG_SMART_POWER_DOWN;
}
}
--
1.7.11.7
^ permalink raw reply related
* [net-next 13/15] e1000e: cleanup magic number
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/nvm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c b/drivers/net/ethernet/intel/e1000e/nvm.c
index 71938ed..1e7882c 100644
--- a/drivers/net/ethernet/intel/e1000e/nvm.c
+++ b/drivers/net/ethernet/intel/e1000e/nvm.c
@@ -464,8 +464,8 @@ s32 e1000_read_pba_string_generic(struct e1000_hw *hw, u8 *pba_num,
if (nvm_data != NVM_PBA_PTR_GUARD) {
e_dbg("NVM PBA number is not stored as string\n");
- /* we will need 11 characters to store the PBA */
- if (pba_num_size < 11) {
+ /* make sure callers buffer is big enough to store the PBA */
+ if (pba_num_size < E1000_PBANUM_LENGTH) {
e_dbg("PBA string buffer too small\n");
return E1000_ERR_NO_SPACE;
}
--
1.7.11.7
^ permalink raw reply related
* [net-next 14/15] e1000e: cleanup code duplication
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
The removed code block is duplicated in e1000e_write_itr() so use that
instead.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 18790a6..6aa9c5d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2405,7 +2405,6 @@ static unsigned int e1000_update_itr(struct e1000_adapter *adapter,
static void e1000_set_itr(struct e1000_adapter *adapter)
{
- struct e1000_hw *hw = &adapter->hw;
u16 current_itr;
u32 new_itr = adapter->itr;
@@ -2468,10 +2467,7 @@ set_itr_now:
if (adapter->msix_entries)
adapter->rx_ring->set_itr = 1;
else
- if (new_itr)
- ew32(ITR, 1000000000 / (new_itr * 256));
- else
- ew32(ITR, 0);
+ e1000e_write_itr(adapter, new_itr);
}
}
--
1.7.11.7
^ permalink raw reply related
* [net-next 15/15] e1000e: merge multiple conditional statements into one
From: Jeff Kirsher @ 2013-01-16 9:10 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
Cleanup a set of conditional tests.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 6aa9c5d..05538e6 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4914,12 +4914,11 @@ static int e1000_transfer_dhcp_info(struct e1000_adapter *adapter,
struct e1000_hw *hw = &adapter->hw;
u16 length, offset;
- if (vlan_tx_tag_present(skb)) {
- if (!((vlan_tx_tag_get(skb) == adapter->hw.mng_cookie.vlan_id) &&
- (adapter->hw.mng_cookie.status &
- E1000_MNG_DHCP_COOKIE_STATUS_VLAN)))
- return 0;
- }
+ if (vlan_tx_tag_present(skb) &&
+ !((vlan_tx_tag_get(skb) == adapter->hw.mng_cookie.vlan_id) &&
+ (adapter->hw.mng_cookie.status &
+ E1000_MNG_DHCP_COOKIE_STATUS_VLAN)))
+ return 0;
if (skb->len <= MINIMUM_DHCP_PACKET_SIZE)
return 0;
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] ipv4: Remove output route check in ipv4_mtu
From: Steffen Klassert @ 2013-01-16 9:59 UTC (permalink / raw)
To: Julian Anastasov; +Cc: David Miller, timo.teras, luky-37, pupilla, netdev
In-Reply-To: <alpine.LFD.2.00.1301160915030.1621@ja.ssi.bg>
On Wed, Jan 16, 2013 at 10:58:13AM +0200, Julian Anastasov wrote:
>
> This fix looks good to me. But I see that we
> have another problem here. doc/ip-cref.tex in iproute2
> claims that a locked MTU value has priority and
> PMTU should not be considered.
>
> IIRC, rt_pmtu is valid only for output routes but
> we do not check the lock flag here. What about such
> variant:
>
> static unsigned int ipv4_mtu(const struct dst_entry *dst)
> {
> const struct rtable *rt = (const struct rtable *) dst;
> unsigned int mtu = rt->rt_pmtu;
>
> if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
> mtu = dst_metric_raw(dst, RTAX_MTU);
> if (!mtu) {
> mtu = dst->dev->mtu;
> if (rt->rt_uses_gateway && mtu > 576)
> mtu = 576;
> }
> } else if (!mtu || time_after_eq(jiffies, rt->dst.expires)) {
> mtu = dst_metric_raw(dst, RTAX_MTU);
> if (!mtu)
> mtu = dst->dev->mtu;
> }
> if (mtu > IP_MAX_MTU)
> mtu = IP_MAX_MTU;
>
> return mtu;
> }
>
> I.e. order becomes:
>
> mtu lock non-zero => fixed fib_mtu
> mtu lock 0 => device MTU, up to 576 if via GW, ignore PMTU
> PMTU (output routes)
> mtu non-zero => fib_mtu
> device MTU
If this is the desired order, I'm fine with the above variant.
>
> Also, it seems __ip_rt_update_pmtu should start
> with such dst_metric_locked(dst, RTAX_MTU) check?
>
Not absolutely sure what you want to do if the mtu is locked.
But if you don't want to genetate a nh exception and don't update
rt->rt_pmtu in this case, rt->rt_pmtu is never set on routes with
locked mtu. We probably would not need to change ipv4_mtu() to the
above variant.
^ permalink raw reply
* [PATCH V5 0/3] handle polling errors in vhost/vhost_net
From: Jason Wang @ 2013-01-16 10:33 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
This is an update version of last version to fix the handling of polling errors
in vhost/vhost_net.
Currently, vhost and vhost_net ignore polling errors which can lead kernel
crashing when it tries to remove itself from waitqueue after the polling
failure. Fix this by:
- examing the POLLERR when set backend and report erros to userspace
- let tun always add to waitqueue in .poll() after the queue is created even if
it was detached.
Changes from V4:
- check the detached state by tfile->detached and protect it by RCU
Changes from V3:
- make a smaller patch that doesn't touch the whole polling state and only check
the polliner errors in backend setting.
- add a patch that allows tuntap to do polling/reading/writing when detached
which could simplify the work of its user.
Changes from v2:
- check poll->wqh instead of the wrong assumption about POLLERR and waitqueue
- drop the whole tx polling state check since it was replaced by the wqh
checking
- drop the buggy tuntap patch
Changes from v1:
- restore the state before the ioctl when vhost_init_used() fails
- log the error when meet polling errors in the data path
- don't put into waitqueue when tun_chr_poll() return POLLERR
Jason Wang (3):
vhost_net: correct error handling in vhost_net_set_backend()
vhost_net: handle polling errors when setting backend
tuntap: allow polling/writing/reading when detached
drivers/net/tun.c | 43 +++++++++++++++++++++++++------------------
drivers/vhost/net.c | 41 ++++++++++++++++++++++++++++-------------
drivers/vhost/vhost.c | 18 +++++++++++++++---
drivers/vhost/vhost.h | 2 +-
4 files changed, 69 insertions(+), 35 deletions(-)
^ permalink raw reply
* [PATCH V5 1/3] vhost_net: correct error handling in vhost_net_set_backend()
From: Jason Wang @ 2013-01-16 10:33 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358332441-26859-1-git-send-email-jasowang@redhat.com>
Currently, when vhost_init_used() fails the sock refcnt and ubufs were
leaked. Correct this by calling vhost_init_used() before assign ubufs and
restore the oldsock when it fails.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ebd08b2..d10ad6f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -827,15 +827,16 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
r = PTR_ERR(ubufs);
goto err_ubufs;
}
- oldubufs = vq->ubufs;
- vq->ubufs = ubufs;
+
vhost_net_disable_vq(n, vq);
rcu_assign_pointer(vq->private_data, sock);
- vhost_net_enable_vq(n, vq);
-
r = vhost_init_used(vq);
if (r)
- goto err_vq;
+ goto err_used;
+ vhost_net_enable_vq(n, vq);
+
+ oldubufs = vq->ubufs;
+ vq->ubufs = ubufs;
n->tx_packets = 0;
n->tx_zcopy_err = 0;
@@ -859,6 +860,11 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
mutex_unlock(&n->dev.mutex);
return 0;
+err_used:
+ rcu_assign_pointer(vq->private_data, oldsock);
+ vhost_net_enable_vq(n, vq);
+ if (ubufs)
+ vhost_ubuf_put_and_wait(ubufs);
err_ubufs:
fput(sock->file);
err_vq:
--
1.7.1
^ permalink raw reply related
* [PATCH V5 2/3] vhost_net: handle polling errors when setting backend
From: Jason Wang @ 2013-01-16 10:34 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358332441-26859-1-git-send-email-jasowang@redhat.com>
Currently, the polling errors were ignored, which can lead following issues:
- vhost remove itself unconditionally from waitqueue when stopping the poll,
this may crash the kernel since the previous attempt of starting may fail to
add itself to the waitqueue
- userspace may think the backend were successfully set even when the polling
failed.
Solve this by:
- check poll->wqh before trying to remove from waitqueue
- report polling errors in vhost_poll_start(), tx_poll_start(), the return value
will be checked and returned when userspace want to set the backend
After this fix, there still could be a polling failure after backend is set, it
will addressed by the next patch.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 27 ++++++++++++++++++---------
drivers/vhost/vhost.c | 18 +++++++++++++++---
drivers/vhost/vhost.h | 2 +-
3 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index d10ad6f..959b1cd 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -165,12 +165,16 @@ static void tx_poll_stop(struct vhost_net *net)
}
/* Caller must have TX VQ lock */
-static void tx_poll_start(struct vhost_net *net, struct socket *sock)
+static int tx_poll_start(struct vhost_net *net, struct socket *sock)
{
+ int ret;
+
if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
- return;
- vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
- net->tx_poll_state = VHOST_NET_POLL_STARTED;
+ return 0;
+ ret = vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
+ if (!ret)
+ net->tx_poll_state = VHOST_NET_POLL_STARTED;
+ return ret;
}
/* In case of DMA done not in order in lower device driver for some reason.
@@ -642,20 +646,23 @@ static void vhost_net_disable_vq(struct vhost_net *n,
vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
}
-static void vhost_net_enable_vq(struct vhost_net *n,
+static int vhost_net_enable_vq(struct vhost_net *n,
struct vhost_virtqueue *vq)
{
struct socket *sock;
+ int ret;
sock = rcu_dereference_protected(vq->private_data,
lockdep_is_held(&vq->mutex));
if (!sock)
- return;
+ return 0;
if (vq == n->vqs + VHOST_NET_VQ_TX) {
n->tx_poll_state = VHOST_NET_POLL_STOPPED;
- tx_poll_start(n, sock);
+ ret = tx_poll_start(n, sock);
} else
- vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
+ ret = vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
+
+ return ret;
}
static struct socket *vhost_net_stop_vq(struct vhost_net *n,
@@ -833,7 +840,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
r = vhost_init_used(vq);
if (r)
goto err_used;
- vhost_net_enable_vq(n, vq);
+ r = vhost_net_enable_vq(n, vq);
+ if (r)
+ goto err_used;
oldubufs = vq->ubufs;
vq->ubufs = ubufs;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 34389f7..9759249 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -77,26 +77,38 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
init_poll_funcptr(&poll->table, vhost_poll_func);
poll->mask = mask;
poll->dev = dev;
+ poll->wqh = NULL;
vhost_work_init(&poll->work, fn);
}
/* Start polling a file. We add ourselves to file's wait queue. The caller must
* keep a reference to a file until after vhost_poll_stop is called. */
-void vhost_poll_start(struct vhost_poll *poll, struct file *file)
+int vhost_poll_start(struct vhost_poll *poll, struct file *file)
{
unsigned long mask;
+ int ret = 0;
mask = file->f_op->poll(file, &poll->table);
if (mask)
vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
+ if (mask & POLLERR) {
+ if (poll->wqh)
+ remove_wait_queue(poll->wqh, &poll->wait);
+ ret = -EINVAL;
+ }
+
+ return ret;
}
/* Stop polling a file. After this function returns, it becomes safe to drop the
* file reference. You must also flush afterwards. */
void vhost_poll_stop(struct vhost_poll *poll)
{
- remove_wait_queue(poll->wqh, &poll->wait);
+ if (poll->wqh) {
+ remove_wait_queue(poll->wqh, &poll->wait);
+ poll->wqh = NULL;
+ }
}
static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
@@ -792,7 +804,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
fput(filep);
if (pollstart && vq->handle_kick)
- vhost_poll_start(&vq->poll, vq->kick);
+ r = vhost_poll_start(&vq->poll, vq->kick);
mutex_unlock(&vq->mutex);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 2639c58..17261e2 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
unsigned long mask, struct vhost_dev *dev);
-void vhost_poll_start(struct vhost_poll *poll, struct file *file);
+int vhost_poll_start(struct vhost_poll *poll, struct file *file);
void vhost_poll_stop(struct vhost_poll *poll);
void vhost_poll_flush(struct vhost_poll *poll);
void vhost_poll_queue(struct vhost_poll *poll);
--
1.7.1
^ permalink raw reply related
* [PATCH V5 3/3] tuntap: allow polling/writing/reading when detached
From: Jason Wang @ 2013-01-16 10:34 UTC (permalink / raw)
To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358332441-26859-1-git-send-email-jasowang@redhat.com>
We forbid polling, writing and reading when the file were detached, this may
complex the user in several cases:
- when guest pass some buffers to vhost/qemu and then disable some queues,
host/qemu needs to do its own cleanup on those buffers which is complex
sometimes. We can do this simply by allowing a user can still write to an
disabled queue. Write to an disabled queue will cause the packet pass to the
kernel and read will get nothing.
- align the polling behavior with macvtap which never fails when the queue is
created. This can simplify the polling errors handling of its user (e.g vhost)
In order to achieve this, tfile->tun were not assign to NULL when detached. And
tfile->tun were converted to be RCU protected in order to let the data path can
check whether the file is deated in a lockless manner. This will be used to
prevent the flow caches from being updated for a detached queue.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 43 +++++++++++++++++++++++++------------------
1 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index c81680d..3f011e0 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -139,7 +139,7 @@ struct tun_file {
unsigned int flags;
u16 queue_index;
struct list_head next;
- struct tun_struct *detached;
+ struct tun_struct __rcu *detached;
};
struct tun_flow_entry {
@@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
}
static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
- u16 queue_index)
+ struct tun_file *tfile)
{
struct hlist_head *head;
struct tun_flow_entry *e;
unsigned long delay = tun->ageing_time;
+ u16 queue_index = tfile->queue_index;
if (!rxhash)
return;
@@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
rcu_read_lock();
- if (tun->numqueues == 1)
+ if (tun->numqueues == 1 || !rtnl_dereference(tfile->detached))
goto unlock;
e = tun_flow_find(head, rxhash);
@@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
{
- tfile->detached = tun;
+ rcu_assign_pointer(tfile->detached, tun);
list_add_tail(&tfile->next, &tun->disabled);
++tun->numdisabled;
}
static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
{
- struct tun_struct *tun = tfile->detached;
+ struct tun_struct *tun = rtnl_dereference(tfile->detached);
- tfile->detached = NULL;
+ rcu_assign_pointer(tfile->detached, NULL);
list_del_init(&tfile->next);
--tun->numdisabled;
return tun;
@@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
static void __tun_detach(struct tun_file *tfile, bool clean)
{
struct tun_file *ntfile;
- struct tun_struct *tun;
+ struct tun_struct *tun, *detached;
struct net_device *dev;
tun = rtnl_dereference(tfile->tun);
+ detached = rtnl_dereference(tfile->detached);
- if (tun) {
+ if (tun && !detached) {
u16 index = tfile->queue_index;
BUG_ON(index >= tun->numqueues);
dev = tun->dev;
rcu_assign_pointer(tun->tfiles[index],
tun->tfiles[tun->numqueues - 1]);
- rcu_assign_pointer(tfile->tun, NULL);
ntfile = rtnl_dereference(tun->tfiles[index]);
ntfile->queue_index = index;
--tun->numqueues;
- if (clean)
+ if (clean) {
+ rcu_assign_pointer(tfile->tun, NULL);
sock_put(&tfile->sk);
- else
+ } else
tun_disable_queue(tun, tfile);
synchronize_net();
@@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
/* Drop read queue */
skb_queue_purge(&tfile->sk.sk_receive_queue);
tun_set_real_num_queues(tun);
- } else if (tfile->detached && clean) {
+ } else if (detached && clean) {
tun = tun_enable_queue(tfile);
sock_put(&tfile->sk);
}
@@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
rcu_assign_pointer(tfile->tun, NULL);
--tun->numqueues;
}
+ list_for_each_entry(tfile, &tun->disabled, next) {
+ wake_up_all(&tfile->wq.wait);
+ rcu_assign_pointer(tfile->tun, NULL);
+ }
BUG_ON(tun->numqueues != 0);
synchronize_net();
@@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
goto out;
err = -EINVAL;
- if (rtnl_dereference(tfile->tun))
+ if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
goto out;
err = -EBUSY;
@@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
goto out;
err = -E2BIG;
- if (!tfile->detached &&
+ if (!rtnl_dereference(tfile->detached) &&
tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
goto out;
@@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
tun->numqueues++;
- if (tfile->detached)
+ if (rtnl_dereference(tfile->detached))
tun_enable_queue(tfile);
else
sock_hold(&tfile->sk);
@@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
tun->dev->stats.rx_packets++;
tun->dev->stats.rx_bytes += len;
- tun_flow_update(tun, rxhash, tfile->queue_index);
+ tun_flow_update(tun, rxhash, tfile);
return total_len;
}
@@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
rtnl_lock();
if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
- tun = tfile->detached;
+ tun = rtnl_dereference(tfile->detached);
if (!tun) {
ret = -EINVAL;
goto unlock;
@@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
ret = tun_attach(tun, file);
} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
tun = rtnl_dereference(tfile->tun);
- if (!tun || !(tun->flags & TUN_TAP_MQ))
+ if (!tun || !(tun->flags & TUN_TAP_MQ) ||
+ rtnl_dereference(tfile->detached))
ret = -EINVAL;
else
__tun_detach(tfile, false);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH V5 3/3] tuntap: allow polling/writing/reading when detached
From: Michael S. Tsirkin @ 2013-01-16 10:59 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358332441-26859-4-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 06:34:01PM +0800, Jason Wang wrote:
> We forbid polling, writing and reading when the file were detached, this may
> complex the user in several cases:
>
> - when guest pass some buffers to vhost/qemu and then disable some queues,
> host/qemu needs to do its own cleanup on those buffers which is complex
> sometimes. We can do this simply by allowing a user can still write to an
> disabled queue. Write to an disabled queue will cause the packet pass to the
> kernel and read will get nothing.
> - align the polling behavior with macvtap which never fails when the queue is
> created. This can simplify the polling errors handling of its user (e.g vhost)
>
> In order to achieve this, tfile->tun were not assign to NULL when detached. And
> tfile->tun were converted to be RCU protected in order to let the data path can
> check whether the file is deated in a lockless manner. This will be used to
> prevent the flow caches from being updated for a detached queue.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
NAK
> ---
> drivers/net/tun.c | 43 +++++++++++++++++++++++++------------------
> 1 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index c81680d..3f011e0 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -139,7 +139,7 @@ struct tun_file {
> unsigned int flags;
> u16 queue_index;
> struct list_head next;
> - struct tun_struct *detached;
> + struct tun_struct __rcu *detached;
> };
>
> struct tun_flow_entry {
> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
> }
>
> static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
> - u16 queue_index)
> + struct tun_file *tfile)
> {
> struct hlist_head *head;
> struct tun_flow_entry *e;
> unsigned long delay = tun->ageing_time;
> + u16 queue_index = tfile->queue_index;
>
> if (!rxhash)
> return;
> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>
> rcu_read_lock();
>
> - if (tun->numqueues == 1)
> + if (tun->numqueues == 1 || !rtnl_dereference(tfile->detached))
> goto unlock;
>
> e = tun_flow_find(head, rxhash);
Did you try to run this with lockdep enabled?
tun_flow_update is called from tun_get_user without rtnl so
rtnl_dereference is arguably wrong, and will cause a lockdep warning.
> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
>
> static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
> {
> - tfile->detached = tun;
> + rcu_assign_pointer(tfile->detached, tun);
> list_add_tail(&tfile->next, &tun->disabled);
> ++tun->numdisabled;
> }
>
> static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> {
> - struct tun_struct *tun = tfile->detached;
> + struct tun_struct *tun = rtnl_dereference(tfile->detached);
>
> - tfile->detached = NULL;
> + rcu_assign_pointer(tfile->detached, NULL);
> list_del_init(&tfile->next);
> --tun->numdisabled;
> return tun;
> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> static void __tun_detach(struct tun_file *tfile, bool clean)
> {
> struct tun_file *ntfile;
> - struct tun_struct *tun;
> + struct tun_struct *tun, *detached;
> struct net_device *dev;
>
> tun = rtnl_dereference(tfile->tun);
> + detached = rtnl_dereference(tfile->detached);
>
> - if (tun) {
> + if (tun && !detached) {
> u16 index = tfile->queue_index;
> BUG_ON(index >= tun->numqueues);
> dev = tun->dev;
>
> rcu_assign_pointer(tun->tfiles[index],
> tun->tfiles[tun->numqueues - 1]);
> - rcu_assign_pointer(tfile->tun, NULL);
> ntfile = rtnl_dereference(tun->tfiles[index]);
> ntfile->queue_index = index;
>
> --tun->numqueues;
> - if (clean)
> + if (clean) {
> + rcu_assign_pointer(tfile->tun, NULL);
> sock_put(&tfile->sk);
> - else
> + } else
> tun_disable_queue(tun, tfile);
>
> synchronize_net();
> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
> /* Drop read queue */
> skb_queue_purge(&tfile->sk.sk_receive_queue);
> tun_set_real_num_queues(tun);
> - } else if (tfile->detached && clean) {
> + } else if (detached && clean) {
> tun = tun_enable_queue(tfile);
> sock_put(&tfile->sk);
> }
> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
> rcu_assign_pointer(tfile->tun, NULL);
> --tun->numqueues;
> }
> + list_for_each_entry(tfile, &tun->disabled, next) {
> + wake_up_all(&tfile->wq.wait);
> + rcu_assign_pointer(tfile->tun, NULL);
> + }
> BUG_ON(tun->numqueues != 0);
>
> synchronize_net();
> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> goto out;
>
> err = -EINVAL;
> - if (rtnl_dereference(tfile->tun))
> + if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
> goto out;
>
> err = -EBUSY;
> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> goto out;
>
> err = -E2BIG;
> - if (!tfile->detached &&
> + if (!rtnl_dereference(tfile->detached) &&
> tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
> goto out;
>
> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
> rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
> tun->numqueues++;
>
> - if (tfile->detached)
> + if (rtnl_dereference(tfile->detached))
> tun_enable_queue(tfile);
> else
> sock_hold(&tfile->sk);
> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> tun->dev->stats.rx_packets++;
> tun->dev->stats.rx_bytes += len;
>
> - tun_flow_update(tun, rxhash, tfile->queue_index);
> + tun_flow_update(tun, rxhash, tfile);
> return total_len;
> }
>
> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> rtnl_lock();
>
> if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
> - tun = tfile->detached;
> + tun = rtnl_dereference(tfile->detached);
> if (!tun) {
> ret = -EINVAL;
> goto unlock;
> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> ret = tun_attach(tun, file);
> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
> tun = rtnl_dereference(tfile->tun);
> - if (!tun || !(tun->flags & TUN_TAP_MQ))
> + if (!tun || !(tun->flags & TUN_TAP_MQ) ||
> + rtnl_dereference(tfile->detached))
> ret = -EINVAL;
> else
> __tun_detach(tfile, false);
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH V5 1/3] vhost_net: correct error handling in vhost_net_set_backend()
From: Michael S. Tsirkin @ 2013-01-16 10:59 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358332441-26859-2-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 06:33:59PM +0800, Jason Wang wrote:
> Currently, when vhost_init_used() fails the sock refcnt and ubufs were
> leaked. Correct this by calling vhost_init_used() before assign ubufs and
> restore the oldsock when it fails.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 16 +++++++++++-----
> 1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index ebd08b2..d10ad6f 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -827,15 +827,16 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> r = PTR_ERR(ubufs);
> goto err_ubufs;
> }
> - oldubufs = vq->ubufs;
> - vq->ubufs = ubufs;
> +
> vhost_net_disable_vq(n, vq);
> rcu_assign_pointer(vq->private_data, sock);
> - vhost_net_enable_vq(n, vq);
> -
> r = vhost_init_used(vq);
> if (r)
> - goto err_vq;
> + goto err_used;
> + vhost_net_enable_vq(n, vq);
> +
> + oldubufs = vq->ubufs;
> + vq->ubufs = ubufs;
>
> n->tx_packets = 0;
> n->tx_zcopy_err = 0;
> @@ -859,6 +860,11 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> mutex_unlock(&n->dev.mutex);
> return 0;
>
> +err_used:
> + rcu_assign_pointer(vq->private_data, oldsock);
> + vhost_net_enable_vq(n, vq);
> + if (ubufs)
> + vhost_ubuf_put_and_wait(ubufs);
> err_ubufs:
> fput(sock->file);
> err_vq:
> --
> 1.7.1
^ permalink raw reply
* Re: [PATCH V5 2/3] vhost_net: handle polling errors when setting backend
From: Michael S. Tsirkin @ 2013-01-16 11:00 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358332441-26859-3-git-send-email-jasowang@redhat.com>
On Wed, Jan 16, 2013 at 06:34:00PM +0800, Jason Wang wrote:
> Currently, the polling errors were ignored, which can lead following issues:
>
> - vhost remove itself unconditionally from waitqueue when stopping the poll,
> this may crash the kernel since the previous attempt of starting may fail to
> add itself to the waitqueue
> - userspace may think the backend were successfully set even when the polling
> failed.
>
> Solve this by:
>
> - check poll->wqh before trying to remove from waitqueue
> - report polling errors in vhost_poll_start(), tx_poll_start(), the return value
> will be checked and returned when userspace want to set the backend
>
> After this fix, there still could be a polling failure after backend is set, it
> will addressed by the next patch.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Looks ok but needs to be considered together with the next one,
to make sure we cover all cases.
> ---
> drivers/vhost/net.c | 27 ++++++++++++++++++---------
> drivers/vhost/vhost.c | 18 +++++++++++++++---
> drivers/vhost/vhost.h | 2 +-
> 3 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index d10ad6f..959b1cd 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -165,12 +165,16 @@ static void tx_poll_stop(struct vhost_net *net)
> }
>
> /* Caller must have TX VQ lock */
> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> +static int tx_poll_start(struct vhost_net *net, struct socket *sock)
> {
> + int ret;
> +
> if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> - return;
> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
> + return 0;
> + ret = vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> + if (!ret)
> + net->tx_poll_state = VHOST_NET_POLL_STARTED;
> + return ret;
> }
>
> /* In case of DMA done not in order in lower device driver for some reason.
> @@ -642,20 +646,23 @@ static void vhost_net_disable_vq(struct vhost_net *n,
> vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> }
>
> -static void vhost_net_enable_vq(struct vhost_net *n,
> +static int vhost_net_enable_vq(struct vhost_net *n,
> struct vhost_virtqueue *vq)
> {
> struct socket *sock;
> + int ret;
>
> sock = rcu_dereference_protected(vq->private_data,
> lockdep_is_held(&vq->mutex));
> if (!sock)
> - return;
> + return 0;
> if (vq == n->vqs + VHOST_NET_VQ_TX) {
> n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> - tx_poll_start(n, sock);
> + ret = tx_poll_start(n, sock);
> } else
> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> + ret = vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> +
> + return ret;
> }
>
> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> @@ -833,7 +840,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> r = vhost_init_used(vq);
> if (r)
> goto err_used;
> - vhost_net_enable_vq(n, vq);
> + r = vhost_net_enable_vq(n, vq);
> + if (r)
> + goto err_used;
>
> oldubufs = vq->ubufs;
> vq->ubufs = ubufs;
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 34389f7..9759249 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -77,26 +77,38 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> init_poll_funcptr(&poll->table, vhost_poll_func);
> poll->mask = mask;
> poll->dev = dev;
> + poll->wqh = NULL;
>
> vhost_work_init(&poll->work, fn);
> }
>
> /* Start polling a file. We add ourselves to file's wait queue. The caller must
> * keep a reference to a file until after vhost_poll_stop is called. */
> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> {
> unsigned long mask;
> + int ret = 0;
>
> mask = file->f_op->poll(file, &poll->table);
> if (mask)
> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> + if (mask & POLLERR) {
> + if (poll->wqh)
> + remove_wait_queue(poll->wqh, &poll->wait);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> }
>
> /* Stop polling a file. After this function returns, it becomes safe to drop the
> * file reference. You must also flush afterwards. */
> void vhost_poll_stop(struct vhost_poll *poll)
> {
> - remove_wait_queue(poll->wqh, &poll->wait);
> + if (poll->wqh) {
> + remove_wait_queue(poll->wqh, &poll->wait);
> + poll->wqh = NULL;
> + }
> }
>
> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> @@ -792,7 +804,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> fput(filep);
>
> if (pollstart && vq->handle_kick)
> - vhost_poll_start(&vq->poll, vq->kick);
> + r = vhost_poll_start(&vq->poll, vq->kick);
>
> mutex_unlock(&vq->mutex);
>
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 2639c58..17261e2 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
>
> void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> unsigned long mask, struct vhost_dev *dev);
> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> +int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> void vhost_poll_stop(struct vhost_poll *poll);
> void vhost_poll_flush(struct vhost_poll *poll);
> void vhost_poll_queue(struct vhost_poll *poll);
> --
> 1.7.1
^ permalink raw reply
* [RFC Patch net-next] tcp: add a global sysctl to control TCP delayed ack
From: Cong Wang @ 2013-01-16 11:05 UTC (permalink / raw)
To: netdev
Cc: Eric Dumazet, Rick Jones, Stephen Hemminger, David S. Miller,
Thomas Graf, David Laight, Cong Wang
According to previous discussion, it seems there is no
reasonable heuristics.
Similar to TCP_QUICK_ACK option, but for people who can't
modify the source code and still wants to control
TCP delayed ACK behavior.
Makes any sense?
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Rick Jones <rick.jones2@hp.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Graf <tgraf@suug.ch>
CC: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 4976564..8fc96f2 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -605,6 +605,11 @@ tcp_challenge_ack_limit - INTEGER
in RFC 5961 (Improving TCP's Robustness to Blind In-Window Attacks)
Default: 100
+tcp_quick_ack - BOOLEAN
+ Globally enables or disables TCP delayed ACK. The applications
+ can still change the quick ACK mode by TCP_QUICK_ACK option.
+ Default: off
+
UDP variables:
udp_mem - vector of 3 INTEGERs: min, pressure, max
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 614af8b..0ba0c26 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -291,6 +291,7 @@ extern int sysctl_tcp_thin_dupack;
extern int sysctl_tcp_early_retrans;
extern int sysctl_tcp_limit_output_bytes;
extern int sysctl_tcp_challenge_ack_limit;
+extern int sysctl_tcp_quick_ack;
extern atomic_long_t tcp_memory_allocated;
extern struct percpu_counter tcp_sockets_allocated;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index a25e1d2..9b4bb75 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -767,6 +767,13 @@ static struct ctl_table ipv4_table[] = {
.extra2 = &two,
},
{
+ .procname = "tcp_quick_ack",
+ .data = &sysctl_tcp_quick_ack,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
.procname = "udp_mem",
.data = &sysctl_udp_mem,
.maxlen = sizeof(sysctl_udp_mem),
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0905997..3f68482 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -100,6 +100,7 @@ int sysctl_tcp_thin_dupack __read_mostly;
int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
int sysctl_tcp_abc __read_mostly;
int sysctl_tcp_early_retrans __read_mostly = 2;
+int sysctl_tcp_quick_ack __read_mostly;
#define FLAG_DATA 0x01 /* Incoming frame contained data. */
#define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a window update. */
@@ -4081,7 +4082,8 @@ static void tcp_fin(struct sock *sk)
case TCP_ESTABLISHED:
/* Move to CLOSE_WAIT */
tcp_set_state(sk, TCP_CLOSE_WAIT);
- inet_csk(sk)->icsk_ack.pingpong = 1;
+ if (!sysctl_tcp_quick_ack)
+ inet_csk(sk)->icsk_ack.pingpong = 1;
break;
case TCP_CLOSE_WAIT:
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 667a6ad..44eff34 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -174,8 +174,9 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
/* If it is a reply for ato after last received
* packet, enter pingpong mode.
*/
- if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
- icsk->icsk_ack.pingpong = 1;
+ if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato &&
+ !sysctl_tcp_quick_ack)
+ icsk->icsk_ack.pingpong = 1;
}
/* Account for an ACK we sent. */
^ permalink raw reply related
* [PATCH] Remove excessive punctuation from ping flood warning
From: Benjamin Kerensa @ 2013-01-16 11:10 UTC (permalink / raw)
To: netdev
This patch removes excessive punctuation from the flood warning.
From 932b1fc65d0f93531d2277831c6a584c9b1cfe63 Mon Sep 17 00:00:00 2001
From: Benjamin Kerensa <bkerensa@ubuntu.com>
Date: Wed, 16 Jan 2013 02:53:38 -0800
Subject: [PATCH] Remove excessive punctuation from ping flood warning
Signed-off-by: Benjamin Kerensa <bkerensa@ubuntu.com
---
ping_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ping_common.c b/ping_common.c
index 7f82851..261aafc 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -608,7 +608,7 @@ void setup(int icmp_sock)
interval = 0;
if (uid && interval < MINUSERINTERVAL) {
- fprintf(stderr, "ping: cannot flood; minimal interval, allowed
for user, is %dms\n", MINUSERINTERVAL);
+ fprintf(stderr, "ping: cannot flood; minimal interval allowed
for user is %dms\n", MINUSERINTERVAL);
exit(2);
}
--
1.8.0
^ permalink raw reply related
* (unknown),
From: fjiban @ 2013-01-16 10:47 UTC (permalink / raw)
Dear Sir/Madam,
I saw your email address during the course of my research today.My Name
is Allen my wife and I won a Jackpot Lottery 11.3 million in july and during
the process my wife passed away as a result of cancer illness, we are donating
the sum of 1.million dollars to 6 lucky individual over the world and if you
received this email then you are one of the lucky recipients and all
you have to
do is get back to us so that we can send your details to the payout bank.
Please note that you have to contact my private email for more information
(violetlargeallen116@yahoo.ie)
You can verify this by visiting the web pages below.
http://www.dailymail.co.uk/news/article-1326473/Canadian-couple-Allen-Violet-Large-away-entire-11-2m-lottery-win.html
Goodluck,
Allen and Violet Large
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
^ permalink raw reply
* RE: [RFC Patch net-next] tcp: add a global sysctl to control TCP delayed ack
From: David Laight @ 2013-01-16 12:22 UTC (permalink / raw)
To: Cong Wang, netdev
Cc: Eric Dumazet, Rick Jones, Stephen Hemminger, David S. Miller,
Thomas Graf
In-Reply-To: <1358334345-28980-1-git-send-email-amwang@redhat.com>
> According to previous discussion, it seems there is no
> reasonable heuristics.
>
> Similar to TCP_QUICK_ACK option, but for people who can't
> modify the source code and still wants to control
> TCP delayed ACK behavior.
>
> Makes any sense?
A sysctl is a bit of a big hammer, it probably isn't necessary
to disable delayed acks on all connections.
IIRC the related problems I saw were really on the sending
side when Nagle is disabled and it is doing 'slow start'.
Globally disabling on connections that have Nagle disabled
might be a possibility - but it is the Nagle parameter
at the other end that matters.
Perhaps the sending side, after sending 4 small frames immediately,
could send 1 or 2 additional full sized frames in order to
provoke an ack (IIRC an ack is sent if there are 2 full sized
frames of data unacked).
The other problem is that 'slow start' is restarted very
aggressively - whenever there is no unacked data.
If you have a very low latency connection and aren't doing
continuous bulk transfer it is restarted for every short
burst of transmits - effectively after every received ack.
There really ought to have to be a moderate idle time
before 'slow start' is restarted.
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox