* [net-next-2.6 PATCH 2/2] ixgbe: declare functions as static
From: Jeff Kirsher @ 2010-10-13 8:20 UTC (permalink / raw)
To: davem
Cc: netdev, gospo, bphilips, Stephen Hemminger, Emil Tantilov,
Jeff Kirsher
In-Reply-To: <20101013081759.24491.46846.stgit@localhost.localdomain>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Following patch fixes warnings reported by `make namespacecheck`
Reported by Stephen Hemminger
CC: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_82599.c | 30 +++++++++++++++---------------
drivers/net/ixgbe/ixgbe_common.c | 5 +++--
drivers/net/ixgbe/ixgbe_common.h | 1 -
drivers/net/ixgbe/ixgbe_dcb_82598.c | 8 ++++----
drivers/net/ixgbe/ixgbe_dcb_82598.h | 11 -----------
drivers/net/ixgbe/ixgbe_dcb_82599.c | 12 ++++++------
drivers/net/ixgbe/ixgbe_dcb_82599.h | 12 ------------
drivers/net/ixgbe/ixgbe_mbx.c | 19 +++----------------
drivers/net/ixgbe/ixgbe_mbx.h | 3 ---
drivers/net/ixgbe/ixgbe_sriov.c | 17 +++++++++--------
drivers/net/ixgbe/ixgbe_sriov.h | 8 --------
11 files changed, 40 insertions(+), 86 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 344c5d6..0bd8fbb 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -39,20 +39,20 @@
#define IXGBE_82599_MC_TBL_SIZE 128
#define IXGBE_82599_VFT_TBL_SIZE 128
-void ixgbe_disable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
-void ixgbe_enable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
-void ixgbe_flap_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
-s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw,
- ixgbe_link_speed speed,
- bool autoneg,
- bool autoneg_wait_to_complete);
+static void ixgbe_disable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
+static void ixgbe_enable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
+static void ixgbe_flap_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
+static s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw,
+ ixgbe_link_speed speed,
+ bool autoneg,
+ bool autoneg_wait_to_complete);
static s32 ixgbe_setup_mac_link_smartspeed(struct ixgbe_hw *hw,
ixgbe_link_speed speed,
bool autoneg,
bool autoneg_wait_to_complete);
-s32 ixgbe_start_mac_link_82599(struct ixgbe_hw *hw,
- bool autoneg_wait_to_complete);
-s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_start_mac_link_82599(struct ixgbe_hw *hw,
+ bool autoneg_wait_to_complete);
+static s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
ixgbe_link_speed speed,
bool autoneg,
bool autoneg_wait_to_complete);
@@ -369,7 +369,7 @@ out:
* Configures link settings based on values in the ixgbe_hw struct.
* Restarts the link. Performs autonegotiation if needed.
**/
-s32 ixgbe_start_mac_link_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_start_mac_link_82599(struct ixgbe_hw *hw,
bool autoneg_wait_to_complete)
{
u32 autoc_reg;
@@ -418,7 +418,7 @@ s32 ixgbe_start_mac_link_82599(struct ixgbe_hw *hw,
* PHY states. This includes selectively shutting down the Tx
* laser on the PHY, effectively halting physical link.
**/
-void ixgbe_disable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
+static void ixgbe_disable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
{
u32 esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
@@ -437,7 +437,7 @@ void ixgbe_disable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
* PHY states. This includes selectively turning on the Tx
* laser on the PHY, effectively starting physical link.
**/
-void ixgbe_enable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
+static void ixgbe_enable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
{
u32 esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
@@ -460,7 +460,7 @@ void ixgbe_enable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
* end. This is consistent with true clause 37 autoneg, which also
* involves a loss of signal.
**/
-void ixgbe_flap_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
+static void ixgbe_flap_tx_laser_multispeed_fiber(struct ixgbe_hw *hw)
{
hw_dbg(hw, "ixgbe_flap_tx_laser_multispeed_fiber\n");
@@ -729,7 +729,7 @@ out:
*
* Set the link speed in the AUTOC register and restarts link.
**/
-s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
ixgbe_link_speed speed, bool autoneg,
bool autoneg_wait_to_complete)
{
diff --git a/drivers/net/ixgbe/ixgbe_common.c b/drivers/net/ixgbe/ixgbe_common.c
index 939e60f..e3eca13 100644
--- a/drivers/net/ixgbe/ixgbe_common.c
+++ b/drivers/net/ixgbe/ixgbe_common.c
@@ -52,6 +52,7 @@ static void ixgbe_disable_rar(struct ixgbe_hw *hw, u32 index);
static s32 ixgbe_mta_vector(struct ixgbe_hw *hw, u8 *mc_addr);
static void ixgbe_add_uc_addr(struct ixgbe_hw *hw, u8 *addr, u32 vmdq);
static s32 ixgbe_setup_fc(struct ixgbe_hw *hw, s32 packetbuf_num);
+static s32 ixgbe_poll_eerd_eewr_done(struct ixgbe_hw *hw, u32 ee_reg);
/**
* ixgbe_start_hw_generic - Prepare hardware for Tx/Rx
@@ -637,7 +638,7 @@ out:
* Polls the status bit (bit 1) of the EERD or EEWR to determine when the
* read or write is done respectively.
**/
-s32 ixgbe_poll_eerd_eewr_done(struct ixgbe_hw *hw, u32 ee_reg)
+static s32 ixgbe_poll_eerd_eewr_done(struct ixgbe_hw *hw, u32 ee_reg)
{
u32 i;
u32 reg;
@@ -2449,7 +2450,7 @@ s32 ixgbe_init_uta_tables_generic(struct ixgbe_hw *hw)
* return the VLVF index where this VLAN id should be placed
*
**/
-s32 ixgbe_find_vlvf_slot(struct ixgbe_hw *hw, u32 vlan)
+static s32 ixgbe_find_vlvf_slot(struct ixgbe_hw *hw, u32 vlan)
{
u32 bits = 0;
u32 first_empty_slot = 0;
diff --git a/drivers/net/ixgbe/ixgbe_common.h b/drivers/net/ixgbe/ixgbe_common.h
index 5cf15aa..424c223 100644
--- a/drivers/net/ixgbe/ixgbe_common.h
+++ b/drivers/net/ixgbe/ixgbe_common.h
@@ -52,7 +52,6 @@ s32 ixgbe_read_eeprom_bit_bang_generic(struct ixgbe_hw *hw, u16 offset,
s32 ixgbe_validate_eeprom_checksum_generic(struct ixgbe_hw *hw,
u16 *checksum_val);
s32 ixgbe_update_eeprom_checksum_generic(struct ixgbe_hw *hw);
-s32 ixgbe_poll_eerd_eewr_done(struct ixgbe_hw *hw, u32 ee_reg);
s32 ixgbe_set_rar_generic(struct ixgbe_hw *hw, u32 index, u8 *addr, u32 vmdq,
u32 enable_addr);
diff --git a/drivers/net/ixgbe/ixgbe_dcb_82598.c b/drivers/net/ixgbe/ixgbe_dcb_82598.c
index 6e9c8bb..50288bc 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_82598.c
+++ b/drivers/net/ixgbe/ixgbe_dcb_82598.c
@@ -78,7 +78,7 @@ static s32 ixgbe_dcb_config_packet_buffers_82598(struct ixgbe_hw *hw,
*
* Configure Rx Data Arbiter and credits for each traffic class.
*/
-s32 ixgbe_dcb_config_rx_arbiter_82598(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_rx_arbiter_82598(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
struct tc_bw_alloc *p;
@@ -135,7 +135,7 @@ s32 ixgbe_dcb_config_rx_arbiter_82598(struct ixgbe_hw *hw,
*
* Configure Tx Descriptor Arbiter and credits for each traffic class.
*/
-s32 ixgbe_dcb_config_tx_desc_arbiter_82598(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_tx_desc_arbiter_82598(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
struct tc_bw_alloc *p;
@@ -183,7 +183,7 @@ s32 ixgbe_dcb_config_tx_desc_arbiter_82598(struct ixgbe_hw *hw,
*
* Configure Tx Data Arbiter and credits for each traffic class.
*/
-s32 ixgbe_dcb_config_tx_data_arbiter_82598(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_tx_data_arbiter_82598(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
struct tc_bw_alloc *p;
@@ -296,7 +296,7 @@ out:
* Configure queue statistics registers, all queues belonging to same traffic
* class uses a single set of queue statistics counters.
*/
-s32 ixgbe_dcb_config_tc_stats_82598(struct ixgbe_hw *hw)
+static s32 ixgbe_dcb_config_tc_stats_82598(struct ixgbe_hw *hw)
{
u32 reg = 0;
u8 i = 0;
diff --git a/drivers/net/ixgbe/ixgbe_dcb_82598.h b/drivers/net/ixgbe/ixgbe_dcb_82598.h
index def907f..abc03cc 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_82598.h
+++ b/drivers/net/ixgbe/ixgbe_dcb_82598.h
@@ -73,17 +73,6 @@
/* DCB PFC functions */
s32 ixgbe_dcb_config_pfc_82598(struct ixgbe_hw *, struct ixgbe_dcb_config *);
-/* DCB traffic class stats */
-s32 ixgbe_dcb_config_tc_stats_82598(struct ixgbe_hw *);
-
-/* DCB config arbiters */
-s32 ixgbe_dcb_config_tx_desc_arbiter_82598(struct ixgbe_hw *,
- struct ixgbe_dcb_config *);
-s32 ixgbe_dcb_config_tx_data_arbiter_82598(struct ixgbe_hw *,
- struct ixgbe_dcb_config *);
-s32 ixgbe_dcb_config_rx_arbiter_82598(struct ixgbe_hw *,
- struct ixgbe_dcb_config *);
-
/* DCB hw initialization */
s32 ixgbe_dcb_hw_config_82598(struct ixgbe_hw *, struct ixgbe_dcb_config *);
diff --git a/drivers/net/ixgbe/ixgbe_dcb_82599.c b/drivers/net/ixgbe/ixgbe_dcb_82599.c
index 1364718..67c219f 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_82599.c
+++ b/drivers/net/ixgbe/ixgbe_dcb_82599.c
@@ -37,7 +37,7 @@
*
* Configure packet buffers for DCB mode.
*/
-s32 ixgbe_dcb_config_packet_buffers_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_packet_buffers_82599(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
s32 ret_val = 0;
@@ -79,7 +79,7 @@ s32 ixgbe_dcb_config_packet_buffers_82599(struct ixgbe_hw *hw,
*
* Configure Rx Packet Arbiter and credits for each traffic class.
*/
-s32 ixgbe_dcb_config_rx_arbiter_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_rx_arbiter_82599(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
struct tc_bw_alloc *p;
@@ -134,7 +134,7 @@ s32 ixgbe_dcb_config_rx_arbiter_82599(struct ixgbe_hw *hw,
*
* Configure Tx Descriptor Arbiter and credits for each traffic class.
*/
-s32 ixgbe_dcb_config_tx_desc_arbiter_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_tx_desc_arbiter_82599(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
struct tc_bw_alloc *p;
@@ -181,7 +181,7 @@ s32 ixgbe_dcb_config_tx_desc_arbiter_82599(struct ixgbe_hw *hw,
*
* Configure Tx Packet Arbiter and credits for each traffic class.
*/
-s32 ixgbe_dcb_config_tx_data_arbiter_82599(struct ixgbe_hw *hw,
+static s32 ixgbe_dcb_config_tx_data_arbiter_82599(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config)
{
struct tc_bw_alloc *p;
@@ -302,7 +302,7 @@ out:
* Configure queue statistics registers, all queues belonging to same traffic
* class uses a single set of queue statistics counters.
*/
-s32 ixgbe_dcb_config_tc_stats_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_dcb_config_tc_stats_82599(struct ixgbe_hw *hw)
{
u32 reg = 0;
u8 i = 0;
@@ -355,7 +355,7 @@ s32 ixgbe_dcb_config_tc_stats_82599(struct ixgbe_hw *hw)
*
* Configure general DCB parameters.
*/
-s32 ixgbe_dcb_config_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_dcb_config_82599(struct ixgbe_hw *hw)
{
u32 reg;
u32 q;
diff --git a/drivers/net/ixgbe/ixgbe_dcb_82599.h b/drivers/net/ixgbe/ixgbe_dcb_82599.h
index 88819b2..18d7fbf 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_82599.h
+++ b/drivers/net/ixgbe/ixgbe_dcb_82599.h
@@ -102,18 +102,6 @@
s32 ixgbe_dcb_config_pfc_82599(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *dcb_config);
-/* DCB traffic class stats */
-s32 ixgbe_dcb_config_tc_stats_82599(struct ixgbe_hw *hw);
-
-/* DCB config arbiters */
-s32 ixgbe_dcb_config_tx_desc_arbiter_82599(struct ixgbe_hw *hw,
- struct ixgbe_dcb_config *dcb_config);
-s32 ixgbe_dcb_config_tx_data_arbiter_82599(struct ixgbe_hw *hw,
- struct ixgbe_dcb_config *dcb_config);
-s32 ixgbe_dcb_config_rx_arbiter_82599(struct ixgbe_hw *hw,
- struct ixgbe_dcb_config *dcb_config);
-
-
/* DCB hw initialization */
s32 ixgbe_dcb_hw_config_82599(struct ixgbe_hw *hw,
struct ixgbe_dcb_config *config);
diff --git a/drivers/net/ixgbe/ixgbe_mbx.c b/drivers/net/ixgbe/ixgbe_mbx.c
index d75f914..435e028 100644
--- a/drivers/net/ixgbe/ixgbe_mbx.c
+++ b/drivers/net/ixgbe/ixgbe_mbx.c
@@ -200,7 +200,8 @@ out:
* returns SUCCESS if it successfully received a message notification and
* copied it into the receive buffer.
**/
-s32 ixgbe_read_posted_mbx(struct ixgbe_hw *hw, u32 *msg, u16 size, u16 mbx_id)
+static s32 ixgbe_read_posted_mbx(struct ixgbe_hw *hw, u32 *msg, u16 size,
+ u16 mbx_id)
{
struct ixgbe_mbx_info *mbx = &hw->mbx;
s32 ret_val = IXGBE_ERR_MBX;
@@ -227,7 +228,7 @@ out:
* returns SUCCESS if it successfully copied message into the buffer and
* received an ack to that message within delay * timeout period
**/
-s32 ixgbe_write_posted_mbx(struct ixgbe_hw *hw, u32 *msg, u16 size,
+static s32 ixgbe_write_posted_mbx(struct ixgbe_hw *hw, u32 *msg, u16 size,
u16 mbx_id)
{
struct ixgbe_mbx_info *mbx = &hw->mbx;
@@ -247,20 +248,6 @@ out:
return ret_val;
}
-/**
- * ixgbe_init_mbx_ops_generic - Initialize MB function pointers
- * @hw: pointer to the HW structure
- *
- * Setup the mailbox read and write message function pointers
- **/
-void ixgbe_init_mbx_ops_generic(struct ixgbe_hw *hw)
-{
- struct ixgbe_mbx_info *mbx = &hw->mbx;
-
- mbx->ops.read_posted = ixgbe_read_posted_mbx;
- mbx->ops.write_posted = ixgbe_write_posted_mbx;
-}
-
static s32 ixgbe_check_for_bit_pf(struct ixgbe_hw *hw, u32 mask, s32 index)
{
u32 mbvficr = IXGBE_READ_REG(hw, IXGBE_MBVFICR(index));
diff --git a/drivers/net/ixgbe/ixgbe_mbx.h b/drivers/net/ixgbe/ixgbe_mbx.h
index be7ab33..c5ae4b4 100644
--- a/drivers/net/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ixgbe/ixgbe_mbx.h
@@ -83,12 +83,9 @@
s32 ixgbe_read_mbx(struct ixgbe_hw *, u32 *, u16, u16);
s32 ixgbe_write_mbx(struct ixgbe_hw *, u32 *, u16, u16);
-s32 ixgbe_read_posted_mbx(struct ixgbe_hw *, u32 *, u16, u16);
-s32 ixgbe_write_posted_mbx(struct ixgbe_hw *, u32 *, u16, u16);
s32 ixgbe_check_for_msg(struct ixgbe_hw *, u16);
s32 ixgbe_check_for_ack(struct ixgbe_hw *, u16);
s32 ixgbe_check_for_rst(struct ixgbe_hw *, u16);
-void ixgbe_init_mbx_ops_generic(struct ixgbe_hw *hw);
void ixgbe_init_mbx_params_pf(struct ixgbe_hw *);
extern struct ixgbe_mbx_operations mbx_ops_82599;
diff --git a/drivers/net/ixgbe/ixgbe_sriov.c b/drivers/net/ixgbe/ixgbe_sriov.c
index 49661a1..a6b720a 100644
--- a/drivers/net/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ixgbe/ixgbe_sriov.c
@@ -43,8 +43,8 @@
#include "ixgbe_sriov.h"
-int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
- int entries, u16 *hash_list, u32 vf)
+static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
+ int entries, u16 *hash_list, u32 vf)
{
struct vf_data_storage *vfinfo = &adapter->vfinfo[vf];
struct ixgbe_hw *hw = &adapter->hw;
@@ -104,13 +104,14 @@ void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter)
}
}
-int ixgbe_set_vf_vlan(struct ixgbe_adapter *adapter, int add, int vid, u32 vf)
+static int ixgbe_set_vf_vlan(struct ixgbe_adapter *adapter, int add, int vid,
+ u32 vf)
{
return adapter->hw.mac.ops.set_vfta(&adapter->hw, vid, vf, (bool)add);
}
-void ixgbe_set_vmolr(struct ixgbe_hw *hw, u32 vf, bool aupe)
+static void ixgbe_set_vmolr(struct ixgbe_hw *hw, u32 vf, bool aupe)
{
u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
vmolr |= (IXGBE_VMOLR_ROMPE |
@@ -134,7 +135,7 @@ static void ixgbe_set_vmvir(struct ixgbe_adapter *adapter, u32 vid, u32 vf)
IXGBE_WRITE_REG(hw, IXGBE_VMVIR(vf), 0);
}
-inline void ixgbe_vf_reset_event(struct ixgbe_adapter *adapter, u32 vf)
+static inline void ixgbe_vf_reset_event(struct ixgbe_adapter *adapter, u32 vf)
{
struct ixgbe_hw *hw = &adapter->hw;
int rar_entry = hw->mac.num_rar_entries - (vf + 1);
@@ -162,8 +163,8 @@ inline void ixgbe_vf_reset_event(struct ixgbe_adapter *adapter, u32 vf)
hw->mac.ops.clear_rar(hw, rar_entry);
}
-int ixgbe_set_vf_mac(struct ixgbe_adapter *adapter,
- int vf, unsigned char *mac_addr)
+static int ixgbe_set_vf_mac(struct ixgbe_adapter *adapter,
+ int vf, unsigned char *mac_addr)
{
struct ixgbe_hw *hw = &adapter->hw;
int rar_entry = hw->mac.num_rar_entries - (vf + 1);
@@ -197,7 +198,7 @@ int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask)
return 0;
}
-inline void ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf)
+static inline void ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf)
{
struct ixgbe_hw *hw = &adapter->hw;
u32 reg;
diff --git a/drivers/net/ixgbe/ixgbe_sriov.h b/drivers/net/ixgbe/ixgbe_sriov.h
index 184730e..9a424bb 100644
--- a/drivers/net/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ixgbe/ixgbe_sriov.h
@@ -28,16 +28,8 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
-int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
- int entries, u16 *hash_list, u32 vf);
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
-int ixgbe_set_vf_vlan(struct ixgbe_adapter *adapter, int add, int vid, u32 vf);
-void ixgbe_set_vmolr(struct ixgbe_hw *hw, u32 vf, bool aupe);
-void ixgbe_vf_reset_event(struct ixgbe_adapter *adapter, u32 vf);
-void ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
-int ixgbe_set_vf_mac(struct ixgbe_adapter *adapter,
- int vf, unsigned char *mac_addr);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
void ixgbe_disable_tx_rx(struct ixgbe_adapter *adapter);
void ixgbe_ping_all_vfs(struct ixgbe_adapter *adapter);
^ permalink raw reply related
* [net-next-2.6 PATCH] igb: add check for fiber/serdes devices to igb_set_spd_dplx;
From: Jeff Kirsher @ 2010-10-13 8:27 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Carolyn Wyborny, Jeff Kirsher
From: Carolyn Wyborny <carolyn.wyborny@intel.com>
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/igb/igb_main.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 55edcb7..5b04eff 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -6115,6 +6115,13 @@ int igb_set_spd_dplx(struct igb_adapter *adapter, u16 spddplx)
mac->autoneg = 0;
+ /* Fiber NIC's only allow 1000 Gbps Full duplex */
+ if ((adapter->hw.phy.media_type == e1000_media_type_internal_serdes) &&
+ spddplx != (SPEED_1000 + DUPLEX_FULL)) {
+ dev_err(&pdev->dev, "Unsupported Speed/Duplex configuration\n");
+ return -EINVAL;
+ }
+
switch (spddplx) {
case SPEED_10 + DUPLEX_HALF:
mac->forced_speed_duplex = ADVERTISE_10_HALF;
^ permalink raw reply related
* [PATCH RESENT] ethtool: add stmmac support
From: Giuseppe CAVALLARO @ 2010-10-13 8:52 UTC (permalink / raw)
To: netdev; +Cc: jgarzik, Giuseppe Cavallaro
Add the stmmac support into the ethtool to
dump both the Mac Core and Dma registers.
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
Makefile.am | 2 +-
ethtool-util.h | 4 +++
ethtool.c | 2 +
stmmac.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 73 insertions(+), 1 deletions(-)
create mode 100644 stmmac.c
diff --git a/Makefile.am b/Makefile.am
index 632f054..a0d2116 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -8,7 +8,7 @@ ethtool_SOURCES = ethtool.c ethtool-copy.h ethtool-util.h \
amd8111e.c de2104x.c e100.c e1000.c igb.c \
fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c \
pcnet32.c realtek.c tg3.c marvell.c vioc.c \
- smsc911x.c at76c50x-usb.c sfc.c
+ smsc911x.c at76c50x-usb.c sfc.c stmmac.c
dist-hook:
cp $(top_srcdir)/ethtool.spec $(distdir)
diff --git a/ethtool-util.h b/ethtool-util.h
index 01b1d03..4ef3a9f 100644
--- a/ethtool-util.h
+++ b/ethtool-util.h
@@ -93,4 +93,8 @@ int at76c50x_usb_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *re
/* Solarflare Solarstorm controllers */
int sfc_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
+/* STMMAC embedded ethernet controller */
+int st_mac100_dump_regs(struct ethtool_drvinfo *info,
+ struct ethtool_regs *regs);
+int st_gmac_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
#endif
diff --git a/ethtool.c b/ethtool.c
index 6b2b7c8..ab69b95 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1539,6 +1539,8 @@ static struct {
{ "smsc911x", smsc911x_dump_regs },
{ "at76c50x-usb", at76c50x_usb_dump_regs },
{ "sfc", sfc_dump_regs },
+ { "st_mac100", st_mac100_dump_regs },
+ { "st_gmac", st_gmac_dump_regs },
};
static int dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
diff --git a/stmmac.c b/stmmac.c
new file mode 100644
index 0000000..ad4311c
--- /dev/null
+++ b/stmmac.c
@@ -0,0 +1,66 @@
+/****************************************************************************
+ * Support for the Synopsys MAC 10/100/1000 on-chip Ethernet controllers
+ *
+ * Copyright (C) 2007-2009 STMicroelectronics Ltd
+ *
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include "ethtool-util.h"
+
+int st_mac100_dump_regs(struct ethtool_drvinfo *info,
+ struct ethtool_regs *regs)
+{
+ int i;
+ unsigned int *stmmac_reg = (unsigned int *)regs->data;
+
+ fprintf(stdout, "ST MAC 10/100 Registers\n");
+ fprintf(stdout, "control reg 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "addr HI 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "addr LO 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "multicast hash HI 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "multicast hash LO 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "MII addr 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "MII data %08X\n", *stmmac_reg++);
+ fprintf(stdout, "flow control 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "VLAN1 tag 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "VLAN2 tag 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "mac wakeup frame 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "mac wakeup crtl 0x%08X\n", *stmmac_reg++);
+
+ fprintf(stdout, "\n");
+ fprintf(stdout, "DMA Registers\n");
+ for (i = 0; i < 9; i++)
+ fprintf(stdout, "CSR%d 0x%08X\n", i, *stmmac_reg++);
+
+ fprintf(stdout, "DMA cur tx buf addr 0x%08X\n", *stmmac_reg++);
+ fprintf(stdout, "DMA cur rx buf addr 0x%08X\n", *stmmac_reg++);
+
+ fprintf(stdout, "\n");
+
+ return 0;
+}
+
+int st_gmac_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
+{
+ int i;
+ unsigned int *stmmac_reg = (unsigned int *)regs->data;
+
+ fprintf(stdout, "ST GMAC Registers\n");
+ fprintf(stdout, "GMAC Registers\n");
+ for (i = 0; i < 55; i++)
+ fprintf(stdout, "Reg%d 0x%08X\n", i, *stmmac_reg++);
+
+ fprintf(stdout, "\n");
+ fprintf(stdout, "DMA Registers\n");
+ for (i = 0; i < 22; i++)
+ fprintf(stdout, "Reg%d 0x%08X\n", i, *stmmac_reg++);
+
+ return 0;
+}
--
1.5.5.6
^ permalink raw reply related
* Re: SCTP AUTO-ASCONF patch
From: jamal @ 2010-10-13 9:21 UTC (permalink / raw)
To: Michio Honda; +Cc: Vlad Yasevich, netdev
In-Reply-To: <EB46A57B-530A-4F52-8A29-120192604816@sfc.wide.ad.jp>
Hi Michio,
I think your description is sufficient (at least i understand it and I
am sure Vlad does), but the rest of the world may not. Maybe mention a
document which describes auto-asconf. Also a signed-off-by entry is
needed - and lastly even the email subject line is important.
Good idea to follow the output described in docs:
kernel-sources/Documentation/SubmitChecklist and SubmittingPatches
cheers,
jamal
On Tue, 2010-10-12 at 13:27 +0100, Michio Honda wrote:
> Hi,
>
> I'm resubmitting a patch to enable AUTO_ASCONF for Linux SCTP. (version is for 2.6.36-rc7).
>
> Thanks,
> - Michio
^ permalink raw reply
* [patch -next] pch_gbe: fix if condition in set_settings()
From: Dan Carpenter @ 2010-10-13 9:36 UTC (permalink / raw)
To: netdev; +Cc: Masayuki Ohtake, David S. Miller, kernel-janitors
There were no curly braces in this if condition so it always enabled full
duplex.
And ecmd->speed is an unsigned short so it is never equal to -1. The
effect is that mii_ethtool_sset() fails with -EINVAL and an error is
printed to dmesg.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/net/pch_gbe/pch_gbe_ethtool.c b/drivers/net/pch_gbe/pch_gbe_ethtool.c
index e06c6ae..c8cc32c 100644
--- a/drivers/net/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/pch_gbe/pch_gbe_ethtool.c
@@ -113,9 +113,10 @@ static int pch_gbe_set_settings(struct net_device *netdev,
pch_gbe_hal_write_phy_reg(hw, MII_BMCR, BMCR_RESET);
- if (ecmd->speed == -1)
+ if (ecmd->speed == USHRT_MAX) {
ecmd->speed = SPEED_1000;
ecmd->duplex = DUPLEX_FULL;
+ }
ret = mii_ethtool_sset(&adapter->mii, ecmd);
if (ret) {
pr_err("Error: mii_ethtool_sset\n");
^ permalink raw reply related
* Re: [PATCH v2] xps-mp: Transmit Packet Steering for multiqueue
From: Eric Dumazet @ 2010-10-13 9:39 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <alpine.DEB.1.00.1010121705500.15786@pokey.mtv.corp.google.com>
Le mardi 12 octobre 2010 à 17:20 -0700, Tom Herbert a écrit :
+#define netdev_get_xps_maps(dev) ((dev)->_tx[0].xps_maps)
Why is xps_maps stored on all struct netdev_queue and not in net_device
itself ?
> +#define QUEUE_MASK_SIZE(dev) (BITS_TO_LONGS(dev->num_tx_queues))
> +#define XPS_MAP_SIZE(dev) (sizeof(struct xps_map) + (num_possible_cpus() * \
> + QUEUE_MASK_SIZE(dev) * sizeof(unsigned long)))
> +#define XPS_ENTRY(map, offset, dev) \
> + (&map->queues[offset * QUEUE_MASK_SIZE(dev)])
> +#define netdev_get_xps_maps(dev) ((dev)->_tx[0].xps_maps)
> +
> +
num_possible_cpus() is a bit too expensive on some machines
(NR_CPUS=4096), please find a way to cache its result in XPS structs.
+ queues = XPS_ENTRY(maps, cpu, dev);
+
+ if (queue_index >= 0) {
+ if (test_bit(queue_index, queues)) {
+ rcu_read_unlock();
+ return queue_index;
+ }
+ }
+
+ weight = bitmap_weight(queues, dev->real_num_tx_queues);
Same here : Cant you store bitmap_weight() somewhere ?
+ select = ((u64) hash * weight) >> 32;
+ queue_index =
+ find_first_bit(queues, dev->real_num_tx_queues);
+ while (select--)
+ queue_index = find_next_bit(queues,
+ dev->real_num_tx_queues, queue_index + 1);
+ break;
Ouch, thats pretty expensive and not scalable (say device has 64 virtual
queues, on a 1024 cores machine), I think an array would be better
queue_index = array[select];
^ permalink raw reply
* Re: [MeeGo-Dev][PATCH v3] Topcliff: Update PCH_CAN driver to 2.6.35
From: Masayuki Ohtake @ 2010-10-13 10:09 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
qi.wang-ral2JQCrhuEAvxtiuMwx3w,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Christian Pellegrin,
Tomoya MORINAGA, David S. Miller,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz
In-Reply-To: <4CA4541F.5040804@grandegger.com>
On Thursday, September 30, 2010 6:10 PM, Wolfgang Grandegger wrote:
>
> + iowrite32(num, &(priv->regs)->if2_creq);
> + while (counter) {
>> + if2_creq = (ioread32(&(priv->regs)->if2_creq)) &
>> + CAN_IF_CREQ_BUSY;
>> + if (!if2_creq)
>> + break;
>> + counter--;
>> + }
>> + if (!counter)
>> + dev_err(&priv->ndev->dev, "IF2 BUSY Flag is set forever.\n");
>> +}
>
>Duplicated code!
No.
These are not the same.
Though it is possible to integrate to one function by adding parameter,
I think, current two function method is more easily to read.
>
>
>
>> + if (status & PCH_STUF_ERR)
>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>> +
>> + if (status & PCH_FORM_ERR)
>> + cf->data[2] |= CAN_ERR_PROT_FORM;
> +
> + if (status & PCH_ACK_ERR)
> + cf->data[2] |= CAN_ERR_PROT_LOC_ACK | CAN_ERR_PROT_LOC_ACK_DEL;
> +
> + if ((status & PCH_BIT1_ERR) || (status & PCH_BIT0_ERR))
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> +
> + if (status & PCH_CRC_ERR)
> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL;
> +
> + if (status & PCH_LEC_ALL)
> + iowrite32(status | PCH_LEC_ALL,
> + &(priv->regs)->stat);
>
>A bit-wise test of the above values is wrong, I believe. Please use the
>switch statement instead.
The above conditions are not only one time.
I think "switch" is not suitable for the above.
Thus, current "if" processing is better.
>
>
> + u32 brp;
> +
> + pch_can_get_run_mode(priv, &curr_mode);
> + if (curr_mode == PCH_CAN_RUN)
> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
>
>The device is stopped when this function is called. Please remove.
No.
The above is necessary.
Because this is our HW specification.
Before setting bitrate, run-mode must be "STOP".
>
>
> +static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> + canid_t id;
> + u32 id1 = 0;
> + u32 id2 = 0;
>
>Need these values to be preset?
These values are not essential.
But these help a engineer to read this code.
>
>
> + /* Enable CAN Interrupts */
> + pch_can_set_int_custom(priv);
> +
> + /* Restore Run Mode */
> + pch_can_set_run_mode(priv, PCH_CAN_RUN);
> +
> + return retval;
> +}
>
>Are the suspend and resume functions tested?
>
Yes, we tested before.
=========================================
Except the above, we are modifying for your indications.
I will resubmit soon.
Thanks, Ohtake(OKISemi)
^ permalink raw reply
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Denis Kirjanov @ 2010-10-13 10:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Ben Hutchings, Jeff Garzik
In-Reply-To: <1286951637.2703.141.camel@edumazet-laptop>
On 10/13/2010 10:33 AM, Eric Dumazet wrote:
> Le mercredi 13 octobre 2010 à 10:06 +0400, Denis Kirjanov a écrit :
>
>> @@ -1494,13 +1506,23 @@ static struct net_device_stats *get_stats(struct net_device *dev)
>> dev->stats.rx_missed_errors += ioread8(ioaddr + RxMissed);
>> dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
>> dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
>> - dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
>> - dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
>> - dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
>> dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
>> - ioread8(ioaddr + StatsTxDefer);
>> - for (i = StatsTxDefer; i <= StatsMcastRx; i++)
>> - ioread8(ioaddr + i);
>> +
>> + np->xstats.tx_multiple_collisions += ioread8(ioaddr + StatsMultiColl);
>> + np->xstats.tx_single_collisions += ioread8(ioaddr + StatsOneColl);
>> + np->xstats.tx_late_collisions += ioread8(ioaddr + StatsLateColl);
>> + dev->stats.collisions += np->xstats.tx_multiple_collisions
>> + + np->xstats.tx_single_collisions
>> + + np->xstats.tx_late_collisions;
>
>
> Oh well..., really ?
>
> Each time we are calling get_stats(), you do
>
> dev->stats.collisions += (number of accumulated collisions since device
> is up)
>
> instead of
>
> dev->stats.collisions += (number of accumulated collisions since last
> time we read counters)
Ok, this is a fixed patch.
Thanks.
Subject: [PATCH -next V5] sundance: Add ethtool stats support
Add ethtool stats support
Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
V2:
check for the ETH_SS_STATS in get_string()
use xstats struct for ethtool stats
V3:
make counters 64-bits wide
V4:
fixed some misspellings
V5:
fixed issue with collisions counting
drivers/net/sundance.c | 94 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 87 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c
index 4283cc5..bfa2a01 100644
--- a/drivers/net/sundance.c
+++ b/drivers/net/sundance.c
@@ -363,6 +363,19 @@ struct netdev_private {
dma_addr_t tx_ring_dma;
dma_addr_t rx_ring_dma;
struct timer_list timer; /* Media monitoring timer. */
+ /* ethtool extra stats */
+ struct {
+ u64 tx_multiple_collisions;
+ u64 tx_single_collisions;
+ u64 tx_late_collisions;
+ u64 tx_defered;
+ u64 tx_defered_excessive;
+ u64 tx_aborted;
+ u64 tx_bcasts;
+ u64 rx_bcasts;
+ u64 tx_mcasts;
+ u64 rx_mcasts;
+ } xstats;
/* Frequently used values: keep some adjacent for cache effect. */
spinlock_t lock;
int msg_enable;
@@ -1486,21 +1499,34 @@ static struct net_device_stats *get_stats(struct net_device *dev)
{
struct netdev_private *np = netdev_priv(dev);
void __iomem *ioaddr = np->base;
- int i;
unsigned long flags;
+ u8 late_coll, single_coll, mult_coll;
spin_lock_irqsave(&np->statlock, flags);
/* The chip only need report frame silently dropped. */
dev->stats.rx_missed_errors += ioread8(ioaddr + RxMissed);
dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
- dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
- dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
- dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
- ioread8(ioaddr + StatsTxDefer);
- for (i = StatsTxDefer; i <= StatsMcastRx; i++)
- ioread8(ioaddr + i);
+
+ mult_coll = ioread8(ioaddr + StatsMultiColl);
+ np->xstats.tx_multiple_collisions += mult_coll;
+ single_coll = ioread8(ioaddr + StatsOneColl);
+ np->xstats.tx_single_collisions += single_coll;
+ late_coll = ioread8(ioaddr + StatsLateColl);
+ np->xstats.tx_late_collisions += late_coll;
+ dev->stats.collisions += mult_coll
+ + single_coll
+ + late_coll;
+
+ np->xstats.tx_defered += ioread8(ioaddr + StatsTxDefer);
+ np->xstats.tx_defered_excessive += ioread8(ioaddr + StatsTxXSDefer);
+ np->xstats.tx_aborted += ioread8(ioaddr + StatsTxAbort);
+ np->xstats.tx_bcasts += ioread8(ioaddr + StatsBcastTx);
+ np->xstats.rx_bcasts += ioread8(ioaddr + StatsBcastRx);
+ np->xstats.tx_mcasts += ioread8(ioaddr + StatsMcastTx);
+ np->xstats.rx_mcasts += ioread8(ioaddr + StatsMcastRx);
+
dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsLow);
dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;
dev->stats.rx_bytes += ioread16(ioaddr + RxOctetsLow);
@@ -1566,6 +1592,21 @@ static int __set_mac_addr(struct net_device *dev)
return 0;
}
+static const struct {
+ const char name[ETH_GSTRING_LEN];
+} sundance_stats[] = {
+ { "tx_multiple_collisions" },
+ { "tx_single_collisions" },
+ { "tx_late_collisions" },
+ { "tx_defered" },
+ { "tx_defered_excessive" },
+ { "tx_aborted" },
+ { "tx_bcasts" },
+ { "rx_bcasts" },
+ { "tx_mcasts" },
+ { "rx_mcasts" },
+};
+
static int check_if_running(struct net_device *dev)
{
if (!netif_running(dev))
@@ -1624,6 +1665,42 @@ static void set_msglevel(struct net_device *dev, u32 val)
np->msg_enable = val;
}
+static void get_strings(struct net_device *dev, u32 stringset,
+ u8 *data)
+{
+ if (stringset == ETH_SS_STATS)
+ memcpy(data, sundance_stats, sizeof(sundance_stats));
+}
+
+static int get_sset_count(struct net_device *dev, int sset)
+{
+ switch (sset) {
+ case ETH_SS_STATS:
+ return ARRAY_SIZE(sundance_stats);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ int i = 0;
+
+ get_stats(dev);
+ data[i++] = np->xstats.tx_multiple_collisions;
+ data[i++] = np->xstats.tx_single_collisions;
+ data[i++] = np->xstats.tx_late_collisions;
+ data[i++] = np->xstats.tx_defered;
+ data[i++] = np->xstats.tx_defered_excessive;
+ data[i++] = np->xstats.tx_aborted;
+ data[i++] = np->xstats.tx_bcasts;
+ data[i++] = np->xstats.rx_bcasts;
+ data[i++] = np->xstats.tx_mcasts;
+ data[i++] = np->xstats.rx_mcasts;
+}
+
static const struct ethtool_ops ethtool_ops = {
.begin = check_if_running,
.get_drvinfo = get_drvinfo,
@@ -1633,6 +1710,9 @@ static const struct ethtool_ops ethtool_ops = {
.get_link = get_link,
.get_msglevel = get_msglevel,
.set_msglevel = set_msglevel,
+ .get_strings = get_strings,
+ .get_sset_count = get_sset_count,
+ .get_ethtool_stats = get_ethtool_stats,
};
static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
--
1.7.0
^ permalink raw reply related
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Eric Dumazet @ 2010-10-13 10:36 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: netdev, David Miller, Ben Hutchings, Jeff Garzik
In-Reply-To: <4CB589B2.50907@kernel.org>
Le mercredi 13 octobre 2010 à 14:28 +0400, Denis Kirjanov a écrit :
> + u64 tx_defered;
> + u64 tx_defered_excessive;
Joe said correct english word is "deferred", not "defered"
^ permalink raw reply
* Re: [PATCH 2/5] Fix deadlock in bonding driver resulting from internal locking when using netpoll
From: Neil Horman @ 2010-10-13 10:51 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, bonding-devel, fubar, davem, andy
In-Reply-To: <4CB51D27.8030703@redhat.com>
On Wed, Oct 13, 2010 at 10:44:55AM +0800, Cong Wang wrote:
> On 10/13/10 05:55, nhorman@tuxdriver.com wrote:
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -76,6 +76,7 @@
> > #include<linux/if_vlan.h>
> > #include<linux/if_bonding.h>
> > #include<linux/jiffies.h>
> >+#include<linux/preempt.h>
> > #include<net/route.h>
> > #include<net/net_namespace.h>
> > #include<net/netns/generic.h>
> >@@ -169,6 +170,35 @@ MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on link
> >
> > /*----------------------------- Global variables ----------------------------*/
> >
> >+#ifdef CONFIG_NET_POLL_CONTROLLER
> >+static cpumask_var_t netpoll_block_tx;
> >+
> >+static inline void block_netpoll_tx(void
> >+{
> >+ preempt_disable();
> >+ BUG_ON(cpumask_test_and_set_cpu(smp_processor_id(),
> >+ netpoll_block_tx));
> >+}
> >+
> >+static inline void unblock_netpoll_tx(void)
> >+{
> >+ BUG_ON(!cpumask_test_and_clear_cpu(smp_processor_id(),
> >+ netpoll_block_tx));
> >+ preempt_enable();
> >+}
> >+
> >+static inline int is_netpoll_tx_blocked(struct net_device *dev)
> >+{
> >+ if (unlikely(dev->priv_flags& IFF_IN_NETPOLL))
> >+ return cpumask_test_cpu(smp_processor_id(), netpoll_block_tx);
> >+ return 0;
> >+}
> >+#else
> >+#define block_netpoll_tx()
> >+#define unblock_netpoll_tx()
> >+#define is_netpoll_tx_blocked(dev)
> >+#endif
> >+
>
> These should go to netpoll.h, IMHO.
>
Doh, you're right, they should. Particularly because I just noticed there are
a few paths through sysfs for bonding that can recurse the same way the
monitoring code can. I'll respin this shortly, thanks.
Neil
^ permalink raw reply
* Re: [PATCH -next] sundance: Add initial ethtool stats support
From: Denis Kirjanov @ 2010-10-13 10:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Ben Hutchings, Jeff Garzik
In-Reply-To: <1286966186.3876.246.camel@edumazet-laptop>
On 10/13/2010 02:36 PM, Eric Dumazet wrote:
> Le mercredi 13 octobre 2010 à 14:28 +0400, Denis Kirjanov a écrit :
>
>> + u64 tx_defered;
>> + u64 tx_defered_excessive;
>
PATCH -next v5] sundance: Add ethtool stats support
Add ethtool stats support.
Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
V2:
check for the ETH_SS_STATS in get_string()
use xstats struct for ethtool stats
V3:
make counters 64-bits wide
V4:
fixed some misspellings
V5:
fixed issue with collisions counting
drivers/net/sundance.c | 94 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 87 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sundance.c b/drivers/net/sundance.c
index 4283cc5..3ed2a67 100644
--- a/drivers/net/sundance.c
+++ b/drivers/net/sundance.c
@@ -363,6 +363,19 @@ struct netdev_private {
dma_addr_t tx_ring_dma;
dma_addr_t rx_ring_dma;
struct timer_list timer; /* Media monitoring timer. */
+ /* ethtool extra stats */
+ struct {
+ u64 tx_multiple_collisions;
+ u64 tx_single_collisions;
+ u64 tx_late_collisions;
+ u64 tx_deferred;
+ u64 tx_deferred_excessive;
+ u64 tx_aborted;
+ u64 tx_bcasts;
+ u64 rx_bcasts;
+ u64 tx_mcasts;
+ u64 rx_mcasts;
+ } xstats;
/* Frequently used values: keep some adjacent for cache effect. */
spinlock_t lock;
int msg_enable;
@@ -1486,21 +1499,34 @@ static struct net_device_stats *get_stats(struct net_device *dev)
{
struct netdev_private *np = netdev_priv(dev);
void __iomem *ioaddr = np->base;
- int i;
unsigned long flags;
+ u8 late_coll, single_coll, mult_coll;
spin_lock_irqsave(&np->statlock, flags);
/* The chip only need report frame silently dropped. */
dev->stats.rx_missed_errors += ioread8(ioaddr + RxMissed);
dev->stats.tx_packets += ioread16(ioaddr + TxFramesOK);
dev->stats.rx_packets += ioread16(ioaddr + RxFramesOK);
- dev->stats.collisions += ioread8(ioaddr + StatsLateColl);
- dev->stats.collisions += ioread8(ioaddr + StatsMultiColl);
- dev->stats.collisions += ioread8(ioaddr + StatsOneColl);
dev->stats.tx_carrier_errors += ioread8(ioaddr + StatsCarrierError);
- ioread8(ioaddr + StatsTxDefer);
- for (i = StatsTxDefer; i <= StatsMcastRx; i++)
- ioread8(ioaddr + i);
+
+ mult_coll = ioread8(ioaddr + StatsMultiColl);
+ np->xstats.tx_multiple_collisions += mult_coll;
+ single_coll = ioread8(ioaddr + StatsOneColl);
+ np->xstats.tx_single_collisions += single_coll;
+ late_coll = ioread8(ioaddr + StatsLateColl);
+ np->xstats.tx_late_collisions += late_coll;
+ dev->stats.collisions += mult_coll
+ + single_coll
+ + late_coll;
+
+ np->xstats.tx_deferred += ioread8(ioaddr + StatsTxDefer);
+ np->xstats.tx_deferred_excessive += ioread8(ioaddr + StatsTxXSDefer);
+ np->xstats.tx_aborted += ioread8(ioaddr + StatsTxAbort);
+ np->xstats.tx_bcasts += ioread8(ioaddr + StatsBcastTx);
+ np->xstats.rx_bcasts += ioread8(ioaddr + StatsBcastRx);
+ np->xstats.tx_mcasts += ioread8(ioaddr + StatsMcastTx);
+ np->xstats.rx_mcasts += ioread8(ioaddr + StatsMcastRx);
+
dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsLow);
dev->stats.tx_bytes += ioread16(ioaddr + TxOctetsHigh) << 16;
dev->stats.rx_bytes += ioread16(ioaddr + RxOctetsLow);
@@ -1566,6 +1592,21 @@ static int __set_mac_addr(struct net_device *dev)
return 0;
}
+static const struct {
+ const char name[ETH_GSTRING_LEN];
+} sundance_stats[] = {
+ { "tx_multiple_collisions" },
+ { "tx_single_collisions" },
+ { "tx_late_collisions" },
+ { "tx_deferred" },
+ { "tx_deferred_excessive" },
+ { "tx_aborted" },
+ { "tx_bcasts" },
+ { "rx_bcasts" },
+ { "tx_mcasts" },
+ { "rx_mcasts" },
+};
+
static int check_if_running(struct net_device *dev)
{
if (!netif_running(dev))
@@ -1624,6 +1665,42 @@ static void set_msglevel(struct net_device *dev, u32 val)
np->msg_enable = val;
}
+static void get_strings(struct net_device *dev, u32 stringset,
+ u8 *data)
+{
+ if (stringset == ETH_SS_STATS)
+ memcpy(data, sundance_stats, sizeof(sundance_stats));
+}
+
+static int get_sset_count(struct net_device *dev, int sset)
+{
+ switch (sset) {
+ case ETH_SS_STATS:
+ return ARRAY_SIZE(sundance_stats);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ int i = 0;
+
+ get_stats(dev);
+ data[i++] = np->xstats.tx_multiple_collisions;
+ data[i++] = np->xstats.tx_single_collisions;
+ data[i++] = np->xstats.tx_late_collisions;
+ data[i++] = np->xstats.tx_deferred;
+ data[i++] = np->xstats.tx_deferred_excessive;
+ data[i++] = np->xstats.tx_aborted;
+ data[i++] = np->xstats.tx_bcasts;
+ data[i++] = np->xstats.rx_bcasts;
+ data[i++] = np->xstats.tx_mcasts;
+ data[i++] = np->xstats.rx_mcasts;
+}
+
static const struct ethtool_ops ethtool_ops = {
.begin = check_if_running,
.get_drvinfo = get_drvinfo,
@@ -1633,6 +1710,9 @@ static const struct ethtool_ops ethtool_ops = {
.get_link = get_link,
.get_msglevel = get_msglevel,
.set_msglevel = set_msglevel,
+ .get_strings = get_strings,
+ .get_sset_count = get_sset_count,
+ .get_ethtool_stats = get_ethtool_stats,
};
static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
--
1.7.0
^ permalink raw reply related
* Re: [MeeGo-Dev][PATCH v3] Topcliff: Update PCH_CAN driver to 2.6.35
From: Wolfgang Grandegger @ 2010-10-13 11:08 UTC (permalink / raw)
To: Masayuki Ohtake
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w,
qi.wang-ral2JQCrhuEAvxtiuMwx3w,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Christian Pellegrin,
Tomoya MORINAGA, David S. Miller,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz
In-Reply-To: <004a01cb6abe$c41a9cc0$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>
On 10/13/2010 12:09 PM, Masayuki Ohtake wrote:
> On Thursday, September 30, 2010 6:10 PM, Wolfgang Grandegger wrote:
>
>>
>> + iowrite32(num, &(priv->regs)->if2_creq);
>> + while (counter) {
>>> + if2_creq = (ioread32(&(priv->regs)->if2_creq)) &
>>> + CAN_IF_CREQ_BUSY;
>>> + if (!if2_creq)
>>> + break;
>>> + counter--;
>>> + }
>>> + if (!counter)
>>> + dev_err(&priv->ndev->dev, "IF2 BUSY Flag is set forever.\n");
>>> +}
>>
>> Duplicated code!
>
> No.
> These are not the same.
Of course they are not the same. The only difference is the register
offset (of if1 or if2). A common function with a pointer to the if
register as argument makes sense.
> Though it is possible to integrate to one function by adding parameter,
> I think, current two function method is more easily to read.
I disagree.
>>
>>
>>
>>> + if (status & PCH_STUF_ERR)
>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>> +
>>> + if (status & PCH_FORM_ERR)
>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>> +
>> + if (status & PCH_ACK_ERR)
>> + cf->data[2] |= CAN_ERR_PROT_LOC_ACK | CAN_ERR_PROT_LOC_ACK_DEL;
>> +
>> + if ((status & PCH_BIT1_ERR) || (status & PCH_BIT0_ERR))
>> + cf->data[2] |= CAN_ERR_PROT_BIT;
>> +
>> + if (status & PCH_CRC_ERR)
>> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
>> + CAN_ERR_PROT_LOC_CRC_DEL;
>> +
>> + if (status & PCH_LEC_ALL)
>> + iowrite32(status | PCH_LEC_ALL,
>> + &(priv->regs)->stat);
Well, if status==7 (PCH_LEC_ALL), all of the above conditions are true
as well... convinced now?
>> A bit-wise test of the above values is wrong, I believe. Please use the
>> switch statement instead.
>
> The above conditions are not only one time.
> I think "switch" is not suitable for the above.
> Thus, current "if" processing is better.
I don't understand! The Last Error Code (LEC) can have values from 0 to
7. A "switch" statement is therefore the right choice. Or have I missed
something.
>>
>>
>> + u32 brp;
>> +
>> + pch_can_get_run_mode(priv, &curr_mode);
>> + if (curr_mode == PCH_CAN_RUN)
>> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
>>
>> The device is stopped when this function is called. Please remove.
>
> No.
> The above is necessary.
Yes, because you started the device *too early* in pch_can_open() called
by pch_open(). See my other related comments of my previous mail.
> Because this is our HW specification.
> Before setting bitrate, run-mode must be "STOP".
I think it can be avoided easily.
>>
>>
>> +static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
>> +{
>> + canid_t id;
>> + u32 id1 = 0;
>> + u32 id2 = 0;
>>
>> Need these values to be preset?
>
> These values are not essential.
> But these help a engineer to read this code.
I disagree.
>> + /* Enable CAN Interrupts */
>> + pch_can_set_int_custom(priv);
>> +
>> + /* Restore Run Mode */
>> + pch_can_set_run_mode(priv, PCH_CAN_RUN);
>> +
>> + return retval;
>> +}
>>
>> Are the suspend and resume functions tested?
>>
> Yes, we tested before.
>
> =========================================
>
> Except the above, we are modifying for your indications.
>
> I will resubmit soon.
Thanks,
Wolfgang.
^ permalink raw reply
* Re: BUG ? ipip unregister_netdevice_many()
From: Jarek Poplawski @ 2010-10-13 11:19 UTC (permalink / raw)
To: David Miller; +Cc: ebiederm, hans.schillstrom, daniel.lezcano, netdev
In-Reply-To: <20101012.130520.48517464.davem@davemloft.net>
On 2010-10-12 22:05, David Miller wrote:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Fri, 08 Oct 2010 10:32:40 -0700
>
>> It is just dealing with not flushing the entire routing cache, just the
>> routes that have expired. Which prevents one network namespace from
>> flushing it's routes and DOS'ing another.
>
> That's a very indirect and obfuscated way of handling it.
>
> And I still don't know why we let the first contiguous set of expired
> entries in the chain get freed outside of the lock, and the rest
> inside the lock. That really isn't explained by anything I've read.
>
> How about we just do exactly what's intended, and with no ifdefs?
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
...
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 0755aa4..6ad730c 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -712,13 +712,14 @@ static inline int rt_is_expired(struct rtable *rth)
> * Can be called by a softirq or a process.
> * In the later case, we want to be reschedule if necessary
> */
> -static void rt_do_flush(int process_context)
> +static void rt_do_flush(struct net *net, int process_context)
> {
> unsigned int i;
> struct rtable *rth, *next;
> - struct rtable * tail;
>
> for (i = 0; i <= rt_hash_mask; i++) {
> + struct rtable *list, **pprev;
Isn't "list = NULL" needed here?
Jarek P.
...
> + rth->dst.rt_next = list;
> + list = rth;
> + } else
> + pprev = &rth->dst.rt_next;
> +
> + rth = next;
...
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Joe Buehler @ 2010-10-13 11:18 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1286905245.2703.3.camel@edumazet-laptop>
Eric Dumazet wrote:
> 2.6.27 is a bit old, you might try :
>
> commit 7fa7cb7109d07c29ab28bb877bc7049a0150dbe5
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon Sep 27 04:18:27 2010 +0000
>
Amazing -- a fix in 20 minutes -- thank you! I need to do more load
testing but it looks like it may have solved the issue.
Joe Buehler
^ permalink raw reply
* Re: [MeeGo-Dev][PATCH v3] Topcliff: Update PCH_CAN driver to 2.6.35
From: Masayuki Ohtake @ 2010-10-13 12:05 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, Tomoya MORINAGA,
David S. Miller, Christian Pellegrin,
qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CB5931E.8030103@grandegger.com>
Hi Wolfgang,
On Wednesday, October 13, 2010 8:08 PM, Wolfgang Grandegger wrote:
> I disagree.
> I think it can be avoided easily.
> I disagree.
I will modify like you are saying.
> I don't understand! The Last Error Code (LEC) can have values from 0 to
> 7. A "switch" statement is therefore the right choice. Or have I missed
> something.
Yes, you are right.
I miss-understood.
Thanks, Ohtake(OKISemi)
----- Original Message -----
From: "Wolfgang Grandegger" <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
To: "Masayuki Ohtake" <masa-korg-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Cc: <joel.clark-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; "Tomoya MORINAGA" <morinaga526-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>; <kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
<yong.y.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <margie.foster-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <qi.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; <socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org>; "Samuel Ortiz"
<sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>; "Barry Song" <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; "Christian Pellegrin" <chripell-VaTbYqLCNhc@public.gmane.org>; "Wolfram Sang"
<w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>; "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Sent: Wednesday, October 13, 2010 8:08 PM
Subject: Re: [MeeGo-Dev][PATCH v3] Topcliff: Update PCH_CAN driver to 2.6.35
> On 10/13/2010 12:09 PM, Masayuki Ohtake wrote:
> > On Thursday, September 30, 2010 6:10 PM, Wolfgang Grandegger wrote:
> >
> >>
> >> + iowrite32(num, &(priv->regs)->if2_creq);
> >> + while (counter) {
> >>> + if2_creq = (ioread32(&(priv->regs)->if2_creq)) &
> >>> + CAN_IF_CREQ_BUSY;
> >>> + if (!if2_creq)
> >>> + break;
> >>> + counter--;
> >>> + }
> >>> + if (!counter)
> >>> + dev_err(&priv->ndev->dev, "IF2 BUSY Flag is set forever.\n");
> >>> +}
> >>
> >> Duplicated code!
> >
> > No.
> > These are not the same.
>
> Of course they are not the same. The only difference is the register
> offset (of if1 or if2). A common function with a pointer to the if
> register as argument makes sense.
>
> > Though it is possible to integrate to one function by adding parameter,
> > I think, current two function method is more easily to read.
>
> I disagree.
>
> >>
> >>
> >>
> >>> + if (status & PCH_STUF_ERR)
> >>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> >>> +
> >>> + if (status & PCH_FORM_ERR)
> >>> + cf->data[2] |= CAN_ERR_PROT_FORM;
> >> +
> >> + if (status & PCH_ACK_ERR)
> >> + cf->data[2] |= CAN_ERR_PROT_LOC_ACK | CAN_ERR_PROT_LOC_ACK_DEL;
> >> +
> >> + if ((status & PCH_BIT1_ERR) || (status & PCH_BIT0_ERR))
> >> + cf->data[2] |= CAN_ERR_PROT_BIT;
> >> +
> >> + if (status & PCH_CRC_ERR)
> >> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> >> + CAN_ERR_PROT_LOC_CRC_DEL;
> >> +
> >> + if (status & PCH_LEC_ALL)
> >> + iowrite32(status | PCH_LEC_ALL,
> >> + &(priv->regs)->stat);
>
> Well, if status==7 (PCH_LEC_ALL), all of the above conditions are true
> as well... convinced now?
>
> >> A bit-wise test of the above values is wrong, I believe. Please use the
> >> switch statement instead.
> >
> > The above conditions are not only one time.
> > I think "switch" is not suitable for the above.
> > Thus, current "if" processing is better.
>
> I don't understand! The Last Error Code (LEC) can have values from 0 to
> 7. A "switch" statement is therefore the right choice. Or have I missed
> something.
>
> >>
> >>
> >> + u32 brp;
> >> +
> >> + pch_can_get_run_mode(priv, &curr_mode);
> >> + if (curr_mode == PCH_CAN_RUN)
> >> + pch_can_set_run_mode(priv, PCH_CAN_STOP);
> >>
> >> The device is stopped when this function is called. Please remove.
> >
> > No.
> > The above is necessary.
>
> Yes, because you started the device *too early* in pch_can_open() called
> by pch_open(). See my other related comments of my previous mail.
>
> > Because this is our HW specification.
> > Before setting bitrate, run-mode must be "STOP".
>
> I think it can be avoided easily.
>
> >>
> >>
> >> +static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
> >> +{
> >> + canid_t id;
> >> + u32 id1 = 0;
> >> + u32 id2 = 0;
> >>
> >> Need these values to be preset?
> >
> > These values are not essential.
> > But these help a engineer to read this code.
>
> I disagree.
>
> >> + /* Enable CAN Interrupts */
> >> + pch_can_set_int_custom(priv);
> >> +
> >> + /* Restore Run Mode */
> >> + pch_can_set_run_mode(priv, PCH_CAN_RUN);
> >> +
> >> + return retval;
> >> +}
> >>
> >> Are the suspend and resume functions tested?
> >>
> > Yes, we tested before.
> >
> > =========================================
> >
> > Except the above, we are modifying for your indications.
> >
> > I will resubmit soon.
>
> Thanks,
>
> Wolfgang.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 1/5] Fix bonding drivers improper modification of netpoll structure
From: nhorman @ 2010-10-13 12:35 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, andy, amwang, nhorman
In-Reply-To: <1286973334-4339-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
The bonding driver currently modifies the netpoll structure in its xmit path
while sending frames from netpoll. This is racy, as other cpus can access the
netpoll structure in parallel. Since the bonding driver points np->dev to a
slave device, other cpus can inadvertently attempt to send data directly to
slave devices, leading to improper locking with the bonding master, lost frames,
and deadlocks. This patch fixes that up.
This patch also removes the real_dev pointer from the netpoll structure as that
data is really only used by bonding in the poll_controller, and we can emulate
its behavior by check each slave for IS_UP.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/bonding/bond_main.c | 15 +++++++++------
include/linux/netpoll.h | 9 +++++++--
net/core/netpoll.c | 6 +++---
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a0bf35d..eb7d089 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -449,11 +449,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
if (unlikely(bond->dev->priv_flags & IFF_IN_NETPOLL)) {
struct netpoll *np = bond->dev->npinfo->netpoll;
slave_dev->npinfo = bond->dev->npinfo;
- np->real_dev = np->dev = skb->dev;
slave_dev->priv_flags |= IFF_IN_NETPOLL;
- netpoll_send_skb(np, skb);
+ netpoll_send_skb_on_dev(np, skb, slave_dev);
slave_dev->priv_flags &= ~IFF_IN_NETPOLL;
- np->dev = bond->dev;
} else
#endif
dev_queue_xmit(skb);
@@ -1332,9 +1330,14 @@ static bool slaves_support_netpoll(struct net_device *bond_dev)
static void bond_poll_controller(struct net_device *bond_dev)
{
- struct net_device *dev = bond_dev->npinfo->netpoll->real_dev;
- if (dev != bond_dev)
- netpoll_poll_dev(dev);
+ struct bonding *bond = netdev_priv(bond_dev);
+ struct slave *slave;
+ int i;
+
+ bond_for_each_slave(bond, slave, i) {
+ if (slave->dev && IS_UP(slave->dev))
+ netpoll_poll_dev(slave->dev);
+ }
}
static void bond_netpoll_cleanup(struct net_device *bond_dev)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 50d8009..79358bb 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -14,7 +14,6 @@
struct netpoll {
struct net_device *dev;
- struct net_device *real_dev;
char dev_name[IFNAMSIZ];
const char *name;
void (*rx_hook)(struct netpoll *, int, char *, int);
@@ -53,7 +52,13 @@ void netpoll_set_trap(int trap);
void __netpoll_cleanup(struct netpoll *np);
void netpoll_cleanup(struct netpoll *np);
int __netpoll_rx(struct sk_buff *skb);
-void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb);
+void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
+ struct net_device *dev);
+static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
+{
+ netpoll_send_skb_on_dev(np, skb, np->dev);
+}
+
#ifdef CONFIG_NETPOLL
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 537e01a..4e98ffa 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -288,11 +288,11 @@ static int netpoll_owner_active(struct net_device *dev)
return 0;
}
-void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
+void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
+ struct net_device *dev)
{
int status = NETDEV_TX_BUSY;
unsigned long tries;
- struct net_device *dev = np->dev;
const struct net_device_ops *ops = dev->netdev_ops;
/* It is up to the caller to keep npinfo alive. */
struct netpoll_info *npinfo = np->dev->npinfo;
@@ -346,7 +346,7 @@ void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
schedule_delayed_work(&npinfo->tx_work,0);
}
}
-EXPORT_SYMBOL(netpoll_send_skb);
+EXPORT_SYMBOL(netpoll_send_skb_on_dev);
void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
{
--
1.7.2.3
^ permalink raw reply related
* [PATCH] bonding: various fixes for bonding, netpoll & netconsole (v2)
From: nhorman @ 2010-10-13 12:35 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, andy, amwang, nhorman
Version 2, taking teh following changes into account:
1) Moved tx blocking/checking macros to netpoll.h as suggested by amwang
2) Added tx blocking macro calls to sysfs paths, as they can deadlock in the
same way that the link monitoring paths can.
Summary:
A while ago we tried to enable netpoll on the bonding driver to enable
netconsole. That worked well in a steady state, but deadlocked frequently in
failover conditions due to some recursive lock-taking (as well as a few other
problems). I've gone through the driver, netconsole and netpoll code, fixed up
those deadlocks, and confirmed that, with this patch series, we can use
netconsole on bonding without deadlock in all bonding modes with all slaves,
even accross failovers. I've also fixed up some incidental bugs that I ran
across while looking through this code, as described in individual patches
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* [PATCH 2/5] Fix deadlock in bonding driver resulting from internal locking when using netpoll
From: nhorman @ 2010-10-13 12:35 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, andy, amwang, nhorman
In-Reply-To: <1286973334-4339-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
The monitoring paths in the bonding driver take write locks that are shared by
the tx path. If netconsole is in use, these paths can call printk which puts us
in the netpoll tx path, which, if netconsole is attached to the bonding driver,
result in deadlock (the xmit_lock guards are useless in netpoll_send_skb, as the
monitor paths in the bonding driver don't claim the xmit_lock, nor should they).
The solution is to use a per cpu flag internal to the driver to indicate when a
cpu is holding the lock in a path that might recusrse into the tx path for the
driver via netconsole. By checking this flag on transmit, we can defer the
sending of the netconsole frames until a later time using the retransmit feature
of netpoll_send_skb that is triggered on the return code NETDEV_TX_BUSY. I've
tested this and am able to transmit via netconsole while causing failover
conditions on the bond slave links.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/bonding/bond_main.c | 42 ++++++++++++++++++++++++++++++++++---
drivers/net/bonding/bond_sysfs.c | 8 +++++++
drivers/net/bonding/bonding.h | 30 +++++++++++++++++++++++++++
3 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index eb7d089..bc38d69 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -76,6 +76,7 @@
#include <linux/if_vlan.h>
#include <linux/if_bonding.h>
#include <linux/jiffies.h>
+#include <linux/preempt.h>
#include <net/route.h>
#include <net/net_namespace.h>
#include <net/netns/generic.h>
@@ -169,6 +170,10 @@ MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on link
/*----------------------------- Global variables ----------------------------*/
+#ifdef CONFIG_NET_POLL_CONTROLLER
+cpumask_var_t netpoll_block_tx;
+#endif
+
static const char * const version =
DRV_DESCRIPTION ": v" DRV_VERSION " (" DRV_RELDATE ")\n";
@@ -310,6 +315,7 @@ static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
pr_debug("bond: %s, vlan id %d\n", bond->dev->name, vlan_id);
+ block_netpoll_tx();
write_lock_bh(&bond->lock);
list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
@@ -344,6 +350,7 @@ static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
out:
write_unlock_bh(&bond->lock);
+ unblock_netpoll_tx();
return res;
}
@@ -1804,10 +1811,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_set_carrier(bond);
#ifdef CONFIG_NET_POLL_CONTROLLER
- /*
- * Netpoll and bonding is broken, make sure it is not initialized
- * until it is fixed.
- */
if (disable_netpoll) {
bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
} else {
@@ -1892,6 +1895,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
return -EINVAL;
}
+ block_netpoll_tx();
netdev_bonding_change(bond_dev, NETDEV_BONDING_DESLAVE);
write_lock_bh(&bond->lock);
@@ -1901,6 +1905,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
pr_info("%s: %s not enslaved\n",
bond_dev->name, slave_dev->name);
write_unlock_bh(&bond->lock);
+ unblock_netpoll_tx();
return -EINVAL;
}
@@ -1994,6 +1999,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
}
write_unlock_bh(&bond->lock);
+ unblock_netpoll_tx();
/* must do this from outside any spinlocks */
bond_destroy_slave_symlinks(bond_dev, slave_dev);
@@ -2085,6 +2091,7 @@ static int bond_release_all(struct net_device *bond_dev)
struct net_device *slave_dev;
struct sockaddr addr;
+ block_netpoll_tx();
write_lock_bh(&bond->lock);
netif_carrier_off(bond_dev);
@@ -2183,6 +2190,7 @@ static int bond_release_all(struct net_device *bond_dev)
out:
write_unlock_bh(&bond->lock);
+ unblock_netpoll_tx();
return 0;
}
@@ -2232,9 +2240,11 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi
(old_active) &&
(new_active->link == BOND_LINK_UP) &&
IS_UP(new_active->dev)) {
+ block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
bond_change_active_slave(bond, new_active);
write_unlock_bh(&bond->curr_slave_lock);
+ unblock_netpoll_tx();
} else
res = -EINVAL;
@@ -2466,9 +2476,11 @@ static void bond_miimon_commit(struct bonding *bond)
do_failover:
ASSERT_RTNL();
+ block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
+ unblock_netpoll_tx();
}
bond_set_carrier(bond);
@@ -2911,11 +2923,13 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
}
if (do_failover) {
+ block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
+ unblock_netpoll_tx();
}
re_arm:
@@ -3074,9 +3088,11 @@ static void bond_ab_arp_commit(struct bonding *bond, int delta_in_ticks)
do_failover:
ASSERT_RTNL();
+ block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
+ unblock_netpoll_tx();
}
bond_set_carrier(bond);
@@ -4564,6 +4580,13 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct bonding *bond = netdev_priv(dev);
+ /*
+ * If we risk deadlock from transmitting this in the
+ * netpoll path, tell netpoll to queue the frame for later tx
+ */
+ if (is_netpoll_tx_blocked(dev))
+ return NETDEV_TX_BUSY;
+
if (TX_QUEUE_OVERRIDE(bond->params.mode)) {
if (!bond_slave_override(bond, skb))
return NETDEV_TX_OK;
@@ -5295,6 +5318,13 @@ static int __init bonding_init(void)
if (res)
goto err;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ if (!alloc_cpumask_var(&netpoll_block_tx, GFP_KERNEL)) {
+ res = -ENOMEM;
+ bond_destroy_sysfs();
+ goto err;
+ }
+#endif
register_netdevice_notifier(&bond_netdev_notifier);
register_inetaddr_notifier(&bond_inetaddr_notifier);
bond_register_ipv6_notifier();
@@ -5316,6 +5346,10 @@ static void __exit bonding_exit(void)
bond_destroy_sysfs();
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ free_cpumask_var(netpoll_block_tx);
+#endif
+
rtnl_link_unregister(&bond_link_ops);
unregister_pernet_subsys(&bond_net_ops);
}
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 01b4c3f..8fd0174 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1066,6 +1066,7 @@ static ssize_t bonding_store_primary(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
+ block_netpoll_tx();
read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
@@ -1101,6 +1102,7 @@ static ssize_t bonding_store_primary(struct device *d,
out:
write_unlock_bh(&bond->curr_slave_lock);
read_unlock(&bond->lock);
+ unblock_netpoll_tx();
rtnl_unlock();
return count;
@@ -1146,11 +1148,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
bond->dev->name, pri_reselect_tbl[new_value].modename,
new_value);
+ block_netpoll_tx();
read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
read_unlock(&bond->lock);
+ unblock_netpoll_tx();
out:
rtnl_unlock();
return ret;
@@ -1232,6 +1236,8 @@ static ssize_t bonding_store_active_slave(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
+
+ block_netpoll_tx();
read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
@@ -1288,6 +1294,8 @@ static ssize_t bonding_store_active_slave(struct device *d,
out:
write_unlock_bh(&bond->curr_slave_lock);
read_unlock(&bond->lock);
+ unblock_netpoll_tx();
+
rtnl_unlock();
return count;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index c15f213..deef1aa 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -19,6 +19,7 @@
#include <linux/proc_fs.h>
#include <linux/if_bonding.h>
#include <linux/kobject.h>
+#include <linux/cpumask.h>
#include <linux/in6.h>
#include "bond_3ad.h"
#include "bond_alb.h"
@@ -117,6 +118,35 @@
bond_for_each_slave_from(bond, pos, cnt, (bond)->first_slave)
+#ifdef CONFIG_NET_POLL_CONTROLLER
+extern cpumask_var_t netpoll_block_tx;
+
+static inline void block_netpoll_tx(void)
+{
+ preempt_disable();
+ BUG_ON(cpumask_test_and_set_cpu(smp_processor_id(),
+ netpoll_block_tx));
+}
+
+static inline void unblock_netpoll_tx(void)
+{
+ BUG_ON(!cpumask_test_and_clear_cpu(smp_processor_id(),
+ netpoll_block_tx));
+ preempt_enable();
+}
+
+static inline int is_netpoll_tx_blocked(struct net_device *dev)
+{
+ if (unlikely(dev->priv_flags & IFF_IN_NETPOLL))
+ return cpumask_test_cpu(smp_processor_id(), netpoll_block_tx);
+ return 0;
+}
+#else
+#define block_netpoll_tx()
+#define unblock_netpoll_tx()
+#define is_netpoll_tx_blocked(dev)
+#endif
+
struct bond_params {
int mode;
int xmit_policy;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/5] Fix napi poll for bonding driver
From: nhorman @ 2010-10-13 12:35 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, andy, amwang, nhorman
In-Reply-To: <1286973334-4339-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Usually the netpoll path, when preforming a napi poll can get away with just
polling all the napi instances of the configured device. Thats not the case for
the bonding driver however, as the napi instances which may wind up getting
flagged as needing polling after the poll_controller call don't belong to the
bonded device, but rather to the slave devices. Fix this by checking the device
in question for the IFF_MASTER flag, if set, we know we need to check the full
poll list for this cpu, rather than just the devices napi instance list.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
net/core/netpoll.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 4e98ffa..d79d221 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -156,8 +156,15 @@ static void poll_napi(struct net_device *dev)
{
struct napi_struct *napi;
int budget = 16;
+ struct softnet_data *sd = &__get_cpu_var(softnet_data);
+ struct list_head *nlist;
- list_for_each_entry(napi, &dev->napi_list, dev_list) {
+ if (dev->flags & IFF_MASTER)
+ nlist = &sd->poll_list;
+ else
+ nlist = &dev->napi_list;
+
+ list_for_each_entry(napi, nlist, dev_list) {
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
budget = poll_one_napi(dev->npinfo, napi, budget);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/5] Fix netconsole to not deadlock on rmmod
From: nhorman @ 2010-10-13 12:35 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, andy, amwang, nhorman
In-Reply-To: <1286973334-4339-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Netconsole calls netpoll_cleanup on receipt of a NETDEVICE_UNREGISTER event.
The notifier subsystem calls these event handlers with rtnl_lock held, which
netpoll_cleanup also takes, resulting in deadlock. Fix this by calling the
__netpoll_cleanup interior function instead, and fixing up the additional
pointers.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/netconsole.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index ca142c4..94255f0 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -678,7 +678,14 @@ static int netconsole_netdev_event(struct notifier_block *this,
strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
break;
case NETDEV_UNREGISTER:
- netpoll_cleanup(&nt->np);
+ /*
+ * rtnl_lock already held
+ */
+ if (nt->np.dev) {
+ __netpoll_cleanup(&nt->np);
+ dev_put(nt->np.dev);
+ nt->np.dev = NULL;
+ }
/* Fall through */
case NETDEV_GOING_DOWN:
case NETDEV_BONDING_DESLAVE:
--
1.7.2.3
^ permalink raw reply related
* [PATCH 5/5] Re-enable netpoll over bonding
From: nhorman @ 2010-10-13 12:35 UTC (permalink / raw)
To: netdev; +Cc: bonding-devel, fubar, davem, andy, amwang, nhorman
In-Reply-To: <1286973334-4339-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
With the inclusion of previous fixup patches, netpoll over bonding apears to
work reliably with failover conditions. This reverts Gospos previous commit
c22d7ac844f1cb9c6a5fd20f89ebadc2feef891b, and allows access again to the netpoll
functionality in the bonding driver.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/bonding/bond_main.c | 29 ++++++++++-------------------
1 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index bc38d69..d42c380 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -184,9 +184,6 @@ static int arp_ip_count;
static int bond_mode = BOND_MODE_ROUNDROBIN;
static int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
static int lacp_fast;
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static int disable_netpoll = 1;
-#endif
const struct bond_parm_tbl bond_lacp_tbl[] = {
{ "slow", AD_LACP_SLOW},
@@ -1811,19 +1808,15 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_set_carrier(bond);
#ifdef CONFIG_NET_POLL_CONTROLLER
- if (disable_netpoll) {
+ if (slaves_support_netpoll(bond_dev)) {
+ bond_dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
+ if (bond_dev->npinfo)
+ slave_dev->npinfo = bond_dev->npinfo;
+ } else if (!(bond_dev->priv_flags & IFF_DISABLE_NETPOLL)) {
bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
- } else {
- if (slaves_support_netpoll(bond_dev)) {
- bond_dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
- if (bond_dev->npinfo)
- slave_dev->npinfo = bond_dev->npinfo;
- } else if (!(bond_dev->priv_flags & IFF_DISABLE_NETPOLL)) {
- bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
- pr_info("New slave device %s does not support netpoll\n",
- slave_dev->name);
- pr_info("Disabling netpoll support for %s\n", bond_dev->name);
- }
+ pr_info("New slave device %s does not support netpoll\n",
+ slave_dev->name);
+ pr_info("Disabling netpoll support for %s\n", bond_dev->name);
}
#endif
read_unlock(&bond->lock);
@@ -2030,10 +2023,8 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
#ifdef CONFIG_NET_POLL_CONTROLLER
read_lock_bh(&bond->lock);
- /* Make sure netpoll over stays disabled until fixed. */
- if (!disable_netpoll)
- if (slaves_support_netpoll(bond_dev))
- bond_dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
+ if (slaves_support_netpoll(bond_dev))
+ bond_dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
read_unlock_bh(&bond->lock);
if (slave_dev->netdev_ops->ndo_netpoll_cleanup)
slave_dev->netdev_ops->ndo_netpoll_cleanup(slave_dev);
--
1.7.2.3
^ permalink raw reply related
* [PATCH net-next] fib6: use FIB_LOOKUP_NOREF in fib6_rule_lookup()
From: Eric Dumazet @ 2010-10-13 12:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Avoid two atomic ops on found rule in fib6_rule_lookup()
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/fib6_rules.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index b1108ed..d829874 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -34,11 +34,10 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi *fl,
{
struct fib_lookup_arg arg = {
.lookup_ptr = lookup,
+ .flags = FIB_LOOKUP_NOREF,
};
fib_rules_lookup(net->ipv6.fib6_rules_ops, fl, flags, &arg);
- if (arg.rule)
- fib_rule_put(arg.rule);
if (arg.result)
return arg.result;
^ permalink raw reply related
* Re: [PATCH net-next 1/5] tipc: Enhance enabling and disabling of Ethernet bearers
From: Neil Horman @ 2010-10-13 13:42 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: davem, netdev, allan.stephens
In-Reply-To: <1286929558-2954-1-git-send-email-paul.gortmaker@windriver.com>
On Tue, Oct 12, 2010 at 08:25:54PM -0400, Paul Gortmaker wrote:
> From: Allan Stephens <allan.stephens@windriver.com>
>
> Use work queue to eliminate release of TIPC's configuration lock when
> registering for device notifications while activating Ethernet media
> support. Optimize code that locates an unused bearer entry when enabling
> an Ethernet bearer. Use work queue to break the association between a
> newly disabled Ethernet bearer and its device driver, thereby allowing
> quicker reuse of the bearer entry.
>
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> net/tipc/config.c | 13 +------
> net/tipc/eth_media.c | 93 ++++++++++++++++++++++++++++++-------------------
> 2 files changed, 58 insertions(+), 48 deletions(-)
>
> diff --git a/net/tipc/config.c b/net/tipc/config.c
> index 961d1b0..a43450c 100644
> --- a/net/tipc/config.c
> +++ b/net/tipc/config.c
> @@ -332,19 +332,8 @@ static struct sk_buff *cfg_set_own_addr(void)
> return tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
> " (cannot change node address once assigned)");
>
> - /*
> - * Must release all spinlocks before calling start_net() because
> - * Linux version of TIPC calls eth_media_start() which calls
> - * register_netdevice_notifier() which may block!
> - *
> - * Temporarily releasing the lock should be harmless for non-Linux TIPC,
> - * but Linux version of eth_media_start() should really be reworked
> - * so that it can be called with spinlocks held.
> - */
> -
> - spin_unlock_bh(&config_lock);
> tipc_core_start_net(addr);
> - spin_lock_bh(&config_lock);
> +
> return tipc_cfg_reply_none();
> }
>
Why is the work queue needed at all? Looking at this path, the only entry to it
is from:
genl_rcv_msg
handle_cmd
tipc_cfg_do_cmd
cfg_set_own_addr
That path looks to only be callable from a user space context, so sleeping on
the rtnl lock in when you call register_netdevice_notifier in eth_media_start
should be perfectly fine. So you should just be able to remove the lock without
any additional work. Does doing so cause other problems?
> diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
> index 6e988ba..479dbc0 100644
> --- a/net/tipc/eth_media.c
> +++ b/net/tipc/eth_media.c
> @@ -51,17 +51,20 @@
> * @bearer: ptr to associated "generic" bearer structure
> * @dev: ptr to associated Ethernet network device
> * @tipc_packet_type: used in binding TIPC to Ethernet driver
> + * @cleanup: work item used when disabling bearer
> */
>
> struct eth_bearer {
> struct tipc_bearer *bearer;
> struct net_device *dev;
> struct packet_type tipc_packet_type;
> + struct work_struct cleanup;
> };
>
> static struct eth_bearer eth_bearers[MAX_ETH_BEARERS];
> static int eth_started = 0;
> static struct notifier_block notifier;
> +static struct work_struct reg_notifier;
>
> /**
> * send_msg - send a TIPC message out over an Ethernet interface
> @@ -157,22 +160,22 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
> if (!dev)
> return -ENODEV;
>
> - /* Find Ethernet bearer for device (or create one) */
> -
> - for (;(eb_ptr != stop) && eb_ptr->dev && (eb_ptr->dev != dev); eb_ptr++);
> - if (eb_ptr == stop)
> - return -EDQUOT;
> - if (!eb_ptr->dev) {
> - eb_ptr->dev = dev;
> - eb_ptr->tipc_packet_type.type = htons(ETH_P_TIPC);
> - eb_ptr->tipc_packet_type.dev = dev;
> - eb_ptr->tipc_packet_type.func = recv_msg;
> - eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
> - INIT_LIST_HEAD(&(eb_ptr->tipc_packet_type.list));
> - dev_hold(dev);
> - dev_add_pack(&eb_ptr->tipc_packet_type);
> + /* Create Ethernet bearer for device */
> +
> + while (eb_ptr->dev != NULL) {
> + if (++eb_ptr == stop)
> + return -EDQUOT;
> }
>
> + eb_ptr->dev = dev;
> + eb_ptr->tipc_packet_type.type = __constant_htons(ETH_P_TIPC);
> + eb_ptr->tipc_packet_type.dev = dev;
> + eb_ptr->tipc_packet_type.func = recv_msg;
> + eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
> + INIT_LIST_HEAD(&eb_ptr->tipc_packet_type.list);
> + dev_hold(dev);
> + dev_add_pack(&eb_ptr->tipc_packet_type);
> +
> /* Associate TIPC bearer with Ethernet bearer */
>
> eb_ptr->bearer = tb_ptr;
> @@ -185,16 +188,36 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
> }
>
> /**
> + * cleanup_bearer - break association between Ethernet bearer and interface
> + *
> + * This routine must be invoked from a work queue because it can sleep.
> + */
> +
> +static void cleanup_bearer(struct work_struct *work)
> +{
> + struct eth_bearer *eb_ptr =
> + container_of(work, struct eth_bearer, cleanup);
> +
> + dev_remove_pack(&eb_ptr->tipc_packet_type);
> + dev_put(eb_ptr->dev);
> + eb_ptr->dev = NULL;
> +}
> +
> +/**
> * disable_bearer - detach TIPC bearer from an Ethernet interface
> *
> - * We really should do dev_remove_pack() here, but this function can not be
> - * called at tasklet level. => Use eth_bearer->bearer as a flag to throw away
> - * incoming buffers, & postpone dev_remove_pack() to eth_media_stop() on exit.
> + * Mark Ethernet bearer as inactive so that incoming buffers are thrown away,
> + * then get worker thread to complete bearer cleanup. (Can't do cleanup
> + * here because cleanup code needs to sleep and caller holds spinlocks.)
> */
>
> static void disable_bearer(struct tipc_bearer *tb_ptr)
> {
> - ((struct eth_bearer *)tb_ptr->usr_handle)->bearer = NULL;
> + struct eth_bearer *eb_ptr = (struct eth_bearer *)tb_ptr->usr_handle;
> +
> + eb_ptr->bearer = NULL;
> + INIT_WORK(&eb_ptr->cleanup, cleanup_bearer);
If you do really need a workqueue for this, you should move this to someplace
like tipc_enable_bearers, in the restart loop, so you only initalize it once.
That also reduces the chance of a race if multiple processes attempt to disable
this barrier in rapid succession.
> + schedule_work(&eb_ptr->cleanup);
> }
>
> /**
> @@ -265,6 +288,19 @@ static char *eth_addr2str(struct tipc_media_addr *a, char *str_buf, int str_size
> }
>
> /**
> + * do_registration - register TIPC to receive device notifications
> + *
> + * This routine must be invoked from a work queue because it can sleep.
> + */
> +
> +static void do_registration(struct work_struct *dummy)
> +{
> + notifier.notifier_call = &recv_notification;
> + notifier.priority = 0;
> + register_netdevice_notifier(¬ifier);
> +}
> +
> +/**
> * tipc_eth_media_start - activate Ethernet bearer support
> *
> * Register Ethernet media type with TIPC bearer code. Also register
> @@ -291,11 +327,9 @@ int tipc_eth_media_start(void)
> if (res)
> return res;
>
> - notifier.notifier_call = &recv_notification;
> - notifier.priority = 0;
> - res = register_netdevice_notifier(¬ifier);
> - if (!res)
> - eth_started = 1;
> + INIT_WORK(®_notifier, do_registration);
> + schedule_work(®_notifier);
> + eth_started = 1;
This is racy. Even though tipc_cfg_do_cmd holds the config_lock, so only one
context can execute this at a time. two requests that execute this code in rapid
succession can re-initalize the reg_notifier work_struct while its sitting on a
work queue, causing an oops if the workqueue task tries to dequeue this entry
while its getting re-initalized. You need to move this to someplace like the
module_init routine.
> return res;
> }
>
> @@ -305,22 +339,9 @@ int tipc_eth_media_start(void)
>
> void tipc_eth_media_stop(void)
> {
> - int i;
> -
> if (!eth_started)
> return;
>
> unregister_netdevice_notifier(¬ifier);
> - for (i = 0; i < MAX_ETH_BEARERS ; i++) {
> - if (eth_bearers[i].bearer) {
> - eth_bearers[i].bearer->blocked = 1;
Where does this now get set when executing a tipc_eth_media_stop? Don't you
want to block access to all bearers immediately when you call this?
> - eth_bearers[i].bearer = NULL;
> - }
> - if (eth_bearers[i].dev) {
> - dev_remove_pack(ð_bearers[i].tipc_packet_type);
> - dev_put(eth_bearers[i].dev);
> - }
> - }
> - memset(ð_bearers, 0, sizeof(eth_bearers));
> eth_started = 0;
> }
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* xfrm by MARK: tcp problems when mark for in and out differ
From: Gerd v. Egidy @ 2010-10-13 13:57 UTC (permalink / raw)
To: jamal; +Cc: netdev, dev
Hi,
I use current strongswan git to set up ipsec connections with the xfrm by MARK
feature. When I configure xfrm policies with different marks for incoming and
outgoing packets, incoming tcp connections can't be established anymore. The
SYN-ACK packet is never sent through the tunnel.
An example policy looks like this:
src 192.168.5.0/24 dst 192.168.1.0/24
dir out priority 1760
mark 5/0xffffffff
tmpl src 172.16.1.131 dst 172.16.1.130
proto esp reqid 16384 mode tunnel
src 192.168.1.0/24 dst 192.168.5.0/24
dir fwd priority 1760
tmpl src 172.16.1.130 dst 172.16.1.131
proto esp reqid 16384 mode tunnel
src 192.168.1.0/24 dst 192.168.5.0/24
dir in priority 1760
tmpl src 172.16.1.130 dst 172.16.1.131
proto esp reqid 16384 mode tunnel
-> incoming packets are without mark, outgoing packets are marked with 5
I traced the packet in the xfrm code and found out that the problem is in the
flow data. When the SYN-ACK hits __xfrm_lookup, the value in fl->mark is 0
(more precisely: the mark value used in the incoming packet). This means that
xfrm_policy_match will not match on the correct policy because the mark values
differ.
I'm not too familiar with the kernel networking code. But I guess that the
flow for the SYN-ACK is set up based on the data used for the SYN and is not
updated when my iptables rule changes the mark of the packet:
iptables -t raw -A OUTPUT -s 192.168.5.0/255.255.255.0 -d
192.168.1.0/255.255.255.0 -j MARK --set-mark 5
I guess that the flow data should be updated somewhere. But I don't know what
the correct place for that code would be.
Can somebody more familiar with the network stack help me with this please?
Thank you very much.
Kind regards,
Gerd
--
Address (better: trap) for people I really don't want to get mail from:
jonas@cactusamerica.com
^ permalink raw reply
* Re: [PATCH] ehea: Fix a checksum issue on the receive path
From: Breno Leitao @ 2010-10-13 14:06 UTC (permalink / raw)
To: David Miller; +Cc: Stephen Hemminger, eric.dumazet, netdev, fubar
In-Reply-To: <20101009103136.341104c5@nehalam>
On 10/09/2010 02:31 PM, Stephen Hemminger wrote:
> Also hardware checksum can be wrong/broken. By passing up a packet
> which the driver thinks is bad, the software can still work.
I see. Unfortunately ehea driver is dropping the packets that have a
wrong checksum. I will work on the fix, and soon I will send the patch.
David,
Just to clarify, this patch that started this thead is not invalidated
by this "problem". So, I'd like to see this patch committed on your
tree. Does it make sense?
Thanks
Breno
^ 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