* Congratulation
From: charles.nichols @ 2009-08-25 14:50 UTC (permalink / raw)
To: contact
750,000 GBP has been Awarded to your Email. send your Names/Tel/Address/Country to slyvester.howard01@gala.net
^ permalink raw reply
* [net-next PATCH 3/3] igb/ixgbe: add IPV6_CSUM support to vlan_features
From: Jeff Kirsher @ 2009-08-25 14:47 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Alexander Duyck, Jeff Kirsher
In-Reply-To: <20090825144709.15560.28811.stgit@localhost.localdomain>
From: Alexander Duyck <alexander.h.duyck@intel.com>
We were already exporting TSO6 to the vlan, but we weren't exporting the
checksum support for IPV6 which was causing warning messages to be
displayed when doing IPv6 TSO over a vlan.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/igb/igb_main.c | 1 +
drivers/net/ixgbe/ixgbe_main.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index fb32735..cef4289 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1380,6 +1380,7 @@ static int __devinit igb_probe(struct pci_dev *pdev,
netdev->vlan_features |= NETIF_F_TSO;
netdev->vlan_features |= NETIF_F_TSO6;
netdev->vlan_features |= NETIF_F_IP_CSUM;
+ netdev->vlan_features |= NETIF_F_IPV6_CSUM;
netdev->vlan_features |= NETIF_F_SG;
if (pci_using_dac)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 3f17706..0bea096 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -5575,6 +5575,7 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
netdev->vlan_features |= NETIF_F_TSO;
netdev->vlan_features |= NETIF_F_TSO6;
netdev->vlan_features |= NETIF_F_IP_CSUM;
+ netdev->vlan_features |= NETIF_F_IPV6_CSUM;
netdev->vlan_features |= NETIF_F_SG;
if (adapter->flags & IXGBE_FLAG_DCB_ENABLED)
^ permalink raw reply related
* [net-next PATCH 2/3] ixgbe: cleanup functions that should have been defined static
From: Jeff Kirsher @ 2009-08-25 14:47 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Don Skidmore, Jeff Kirsher
In-Reply-To: <20090825144709.15560.28811.stgit@localhost.localdomain>
From: Don Skidmore <donald.c.skidmore@intel.com>
We have some ~40 functions that were being called out with 'make
namespacecheck'. This patch changes these functions to be static.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe.h | 35 ---------
drivers/net/ixgbe/ixgbe_82598.c | 6 +
drivers/net/ixgbe/ixgbe_82599.c | 156 ++++++++++++++++++--------------------
drivers/net/ixgbe/ixgbe_common.c | 3 -
drivers/net/ixgbe/ixgbe_common.h | 1
drivers/net/ixgbe/ixgbe_main.c | 2
6 files changed, 81 insertions(+), 122 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index 8f1f8ba..c983c89 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -424,55 +424,20 @@ extern s32 ixgbe_init_fdir_perfect_82599(struct ixgbe_hw *hw, u32 pballoc);
extern s32 ixgbe_fdir_add_signature_filter_82599(struct ixgbe_hw *hw,
struct ixgbe_atr_input *input,
u8 queue);
-extern s32 ixgbe_fdir_add_perfect_filter_82599(struct ixgbe_hw *hw,
- struct ixgbe_atr_input *input,
- u16 soft_id,
- u8 queue);
-extern u16 ixgbe_atr_compute_hash_82599(struct ixgbe_atr_input *input, u32 key);
extern s32 ixgbe_atr_set_vlan_id_82599(struct ixgbe_atr_input *input,
u16 vlan_id);
extern s32 ixgbe_atr_set_src_ipv4_82599(struct ixgbe_atr_input *input,
u32 src_addr);
extern s32 ixgbe_atr_set_dst_ipv4_82599(struct ixgbe_atr_input *input,
u32 dst_addr);
-extern s32 ixgbe_atr_set_src_ipv6_82599(struct ixgbe_atr_input *input,
- u32 src_addr_1, u32 src_addr_2,
- u32 src_addr_3, u32 src_addr_4);
-extern s32 ixgbe_atr_set_dst_ipv6_82599(struct ixgbe_atr_input *input,
- u32 dst_addr_1, u32 dst_addr_2,
- u32 dst_addr_3, u32 dst_addr_4);
extern s32 ixgbe_atr_set_src_port_82599(struct ixgbe_atr_input *input,
u16 src_port);
extern s32 ixgbe_atr_set_dst_port_82599(struct ixgbe_atr_input *input,
u16 dst_port);
extern s32 ixgbe_atr_set_flex_byte_82599(struct ixgbe_atr_input *input,
u16 flex_byte);
-extern s32 ixgbe_atr_set_vm_pool_82599(struct ixgbe_atr_input *input,
- u8 vm_pool);
extern s32 ixgbe_atr_set_l4type_82599(struct ixgbe_atr_input *input,
u8 l4type);
-extern s32 ixgbe_atr_get_vlan_id_82599(struct ixgbe_atr_input *input,
- u16 *vlan_id);
-extern s32 ixgbe_atr_get_src_ipv4_82599(struct ixgbe_atr_input *input,
- u32 *src_addr);
-extern s32 ixgbe_atr_get_dst_ipv4_82599(struct ixgbe_atr_input *input,
- u32 *dst_addr);
-extern s32 ixgbe_atr_get_src_ipv6_82599(struct ixgbe_atr_input *input,
- u32 *src_addr_1, u32 *src_addr_2,
- u32 *src_addr_3, u32 *src_addr_4);
-extern s32 ixgbe_atr_get_dst_ipv6_82599(struct ixgbe_atr_input *input,
- u32 *dst_addr_1, u32 *dst_addr_2,
- u32 *dst_addr_3, u32 *dst_addr_4);
-extern s32 ixgbe_atr_get_src_port_82599(struct ixgbe_atr_input *input,
- u16 *src_port);
-extern s32 ixgbe_atr_get_dst_port_82599(struct ixgbe_atr_input *input,
- u16 *dst_port);
-extern s32 ixgbe_atr_get_flex_byte_82599(struct ixgbe_atr_input *input,
- u16 *flex_byte);
-extern s32 ixgbe_atr_get_vm_pool_82599(struct ixgbe_atr_input *input,
- u8 *vm_pool);
-extern s32 ixgbe_atr_get_l4type_82599(struct ixgbe_atr_input *input,
- u8 *l4type);
#ifdef IXGBE_FCOE
extern void ixgbe_configure_fcoe(struct ixgbe_adapter *adapter);
extern int ixgbe_fso(struct ixgbe_adapter *adapter,
diff --git a/drivers/net/ixgbe/ixgbe_82598.c b/drivers/net/ixgbe/ixgbe_82598.c
index 1c227b0..916430f 100644
--- a/drivers/net/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ixgbe/ixgbe_82598.c
@@ -59,7 +59,7 @@ static s32 ixgbe_read_i2c_eeprom_82598(struct ixgbe_hw *hw, u8 byte_offset,
* increase the value to either 10ms to 250ms for capability version 1 config,
* or 16ms to 55ms for version 2.
**/
-void ixgbe_set_pcie_completion_timeout(struct ixgbe_hw *hw)
+static void ixgbe_set_pcie_completion_timeout(struct ixgbe_hw *hw)
{
struct ixgbe_adapter *adapter = hw->back;
u32 gcr = IXGBE_READ_REG(hw, IXGBE_GCR);
@@ -143,7 +143,7 @@ static s32 ixgbe_get_invariants_82598(struct ixgbe_hw *hw)
* not known. Perform the SFP init if necessary.
*
**/
-s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
+static s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
{
struct ixgbe_mac_info *mac = &hw->mac;
struct ixgbe_phy_info *phy = &hw->phy;
@@ -204,7 +204,7 @@ out:
* Starts the hardware using the generic start_hw function.
* Then set pcie completion timeout
**/
-s32 ixgbe_start_hw_82598(struct ixgbe_hw *hw)
+static s32 ixgbe_start_hw_82598(struct ixgbe_hw *hw)
{
s32 ret_val = 0;
diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 1984cab..364b6d2 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -38,22 +38,15 @@
#define IXGBE_82599_MC_TBL_SIZE 128
#define IXGBE_82599_VFT_TBL_SIZE 128
-s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
- ixgbe_link_speed *speed,
- bool *autoneg);
-enum ixgbe_media_type ixgbe_get_media_type_82599(struct ixgbe_hw *hw);
-s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw);
-s32 ixgbe_setup_mac_link_speed_multispeed_fiber(struct ixgbe_hw *hw,
+static s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw);
+static s32 ixgbe_setup_mac_link_speed_multispeed_fiber(struct ixgbe_hw *hw,
ixgbe_link_speed speed, bool autoneg,
bool autoneg_wait_to_complete);
-s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw);
-s32 ixgbe_check_mac_link_82599(struct ixgbe_hw *hw,
- ixgbe_link_speed *speed,
- bool *link_up, bool link_up_wait_to_complete);
-s32 ixgbe_setup_mac_link_speed_82599(struct ixgbe_hw *hw,
- ixgbe_link_speed speed,
- bool autoneg,
- bool autoneg_wait_to_complete);
+static s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw);
+static s32 ixgbe_setup_mac_link_speed_82599(struct ixgbe_hw *hw,
+ ixgbe_link_speed speed,
+ bool autoneg,
+ bool autoneg_wait_to_complete);
static s32 ixgbe_get_copper_link_capabilities_82599(struct ixgbe_hw *hw,
ixgbe_link_speed *speed,
bool *autoneg);
@@ -62,21 +55,9 @@ static s32 ixgbe_setup_copper_link_speed_82599(struct ixgbe_hw *hw,
ixgbe_link_speed speed,
bool autoneg,
bool autoneg_wait_to_complete);
-s32 ixgbe_reset_hw_82599(struct ixgbe_hw *hw);
-s32 ixgbe_set_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq);
-s32 ixgbe_clear_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq);
-s32 ixgbe_set_vfta_82599(struct ixgbe_hw *hw, u32 vlan,
- u32 vind, bool vlan_on);
-s32 ixgbe_clear_vfta_82599(struct ixgbe_hw *hw);
-s32 ixgbe_init_uta_tables_82599(struct ixgbe_hw *hw);
-s32 ixgbe_read_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 *val);
-s32 ixgbe_write_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 val);
-s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw);
-s32 ixgbe_start_hw_82599(struct ixgbe_hw *hw);
-u32 ixgbe_get_supported_physical_layer_82599(struct ixgbe_hw *hw);
static s32 ixgbe_verify_fw_version_82599(struct ixgbe_hw *hw);
-void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
+static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
{
struct ixgbe_mac_info *mac = &hw->mac;
if (hw->phy.multispeed_fiber) {
@@ -93,7 +74,7 @@ void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
}
}
-s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_sfp_modules_82599(struct ixgbe_hw *hw)
{
s32 ret_val = 0;
u16 list_offset, data_offset, data_value;
@@ -143,7 +124,7 @@ setup_sfp_out:
* Read PCIe configuration space, and get the MSI-X vector count from
* the capabilities table.
**/
-u32 ixgbe_get_pcie_msix_count_82599(struct ixgbe_hw *hw)
+static u32 ixgbe_get_pcie_msix_count_82599(struct ixgbe_hw *hw)
{
struct ixgbe_adapter *adapter = hw->back;
u16 msix_count;
@@ -182,7 +163,7 @@ static s32 ixgbe_get_invariants_82599(struct ixgbe_hw *hw)
* not known. Perform the SFP init if necessary.
*
**/
-s32 ixgbe_init_phy_ops_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_init_phy_ops_82599(struct ixgbe_hw *hw)
{
struct ixgbe_mac_info *mac = &hw->mac;
struct ixgbe_phy_info *phy = &hw->phy;
@@ -225,9 +206,9 @@ s32 ixgbe_init_phy_ops_82599(struct ixgbe_hw *hw)
*
* Determines the link capabilities by reading the AUTOC register.
**/
-s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
- ixgbe_link_speed *speed,
- bool *negotiation)
+static s32 ixgbe_get_link_capabilities_82599(struct ixgbe_hw *hw,
+ ixgbe_link_speed *speed,
+ bool *negotiation)
{
s32 status = 0;
u32 autoc = 0;
@@ -344,7 +325,7 @@ static s32 ixgbe_get_copper_link_capabilities_82599(struct ixgbe_hw *hw,
*
* Returns the media type (fiber, copper, backplane)
**/
-enum ixgbe_media_type ixgbe_get_media_type_82599(struct ixgbe_hw *hw)
+static enum ixgbe_media_type ixgbe_get_media_type_82599(struct ixgbe_hw *hw)
{
enum ixgbe_media_type media_type;
@@ -379,7 +360,7 @@ out:
* Configures link settings based on values in the ixgbe_hw struct.
* Restarts the link. Performs autonegotiation if needed.
**/
-s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw)
{
u32 autoc_reg;
u32 links_reg;
@@ -428,7 +409,7 @@ s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw)
* fails at 10G.
* Performs autonegotiation if needed.
**/
-s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw)
+static s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw)
{
s32 status = 0;
ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_82599_AUTONEG;
@@ -446,7 +427,7 @@ s32 ixgbe_setup_mac_link_multispeed_fiber(struct ixgbe_hw *hw)
*
* Set the link speed in the AUTOC register and restarts link.
**/
-s32 ixgbe_setup_mac_link_speed_multispeed_fiber(struct ixgbe_hw *hw,
+static s32 ixgbe_setup_mac_link_speed_multispeed_fiber(struct ixgbe_hw *hw,
ixgbe_link_speed speed,
bool autoneg,
bool autoneg_wait_to_complete)
@@ -613,8 +594,10 @@ out:
*
* Reads the links register to determine if link is up and the current speed
**/
-s32 ixgbe_check_mac_link_82599(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
- bool *link_up, bool link_up_wait_to_complete)
+static s32 ixgbe_check_mac_link_82599(struct ixgbe_hw *hw,
+ ixgbe_link_speed *speed,
+ bool *link_up,
+ bool link_up_wait_to_complete)
{
u32 links_reg;
u32 i;
@@ -665,9 +648,10 @@ s32 ixgbe_check_mac_link_82599(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
*
* Set the link speed in the AUTOC register and restarts link.
**/
-s32 ixgbe_setup_mac_link_speed_82599(struct ixgbe_hw *hw,
- ixgbe_link_speed speed, bool autoneg,
- bool autoneg_wait_to_complete)
+static s32 ixgbe_setup_mac_link_speed_82599(struct ixgbe_hw *hw,
+ ixgbe_link_speed speed,
+ bool autoneg,
+ bool autoneg_wait_to_complete)
{
s32 status = 0;
u32 autoc = IXGBE_READ_REG(hw, IXGBE_AUTOC);
@@ -818,7 +802,7 @@ static s32 ixgbe_setup_copper_link_speed_82599(struct ixgbe_hw *hw,
* and clears all interrupts, perform a PHY reset, and perform a link (MAC)
* reset.
**/
-s32 ixgbe_reset_hw_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_reset_hw_82599(struct ixgbe_hw *hw)
{
s32 status = 0;
u32 ctrl, ctrl_ext;
@@ -943,7 +927,7 @@ reset_hw_out:
* @rar: receive address register index to disassociate
* @vmdq: VMDq pool index to remove from the rar
**/
-s32 ixgbe_clear_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq)
+static s32 ixgbe_clear_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq)
{
u32 mpsar_lo, mpsar_hi;
u32 rar_entries = hw->mac.num_rar_entries;
@@ -989,7 +973,7 @@ done:
* @rar: receive address register index to associate with a VMDq index
* @vmdq: VMDq pool index
**/
-s32 ixgbe_set_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq)
+static s32 ixgbe_set_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq)
{
u32 mpsar;
u32 rar_entries = hw->mac.num_rar_entries;
@@ -1019,8 +1003,8 @@ s32 ixgbe_set_vmdq_82599(struct ixgbe_hw *hw, u32 rar, u32 vmdq)
*
* Turn on/off specified VLAN in the VLAN filter table.
**/
-s32 ixgbe_set_vfta_82599(struct ixgbe_hw *hw, u32 vlan, u32 vind,
- bool vlan_on)
+static s32 ixgbe_set_vfta_82599(struct ixgbe_hw *hw, u32 vlan, u32 vind,
+ bool vlan_on)
{
u32 regindex;
u32 bitindex;
@@ -1133,7 +1117,7 @@ out:
*
* Clears the VLAN filer table, and the VMDq index associated with the filter
**/
-s32 ixgbe_clear_vfta_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_clear_vfta_82599(struct ixgbe_hw *hw)
{
u32 offset;
@@ -1153,7 +1137,7 @@ s32 ixgbe_clear_vfta_82599(struct ixgbe_hw *hw)
* ixgbe_init_uta_tables_82599 - Initialize the Unicast Table Array
* @hw: pointer to hardware structure
**/
-s32 ixgbe_init_uta_tables_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_init_uta_tables_82599(struct ixgbe_hw *hw)
{
int i;
hw_dbg(hw, " Clearing UTA\n");
@@ -1430,7 +1414,8 @@ s32 ixgbe_init_fdir_perfect_82599(struct ixgbe_hw *hw, u32 pballoc)
* @stream: input bitstream to compute the hash on
* @key: 32-bit hash key
**/
-u16 ixgbe_atr_compute_hash_82599(struct ixgbe_atr_input *atr_input, u32 key)
+static u16 ixgbe_atr_compute_hash_82599(struct ixgbe_atr_input *atr_input,
+ u32 key)
{
/*
* The algorithm is as follows:
@@ -1602,8 +1587,8 @@ s32 ixgbe_atr_set_dst_ipv4_82599(struct ixgbe_atr_input *input, u32 dst_addr)
* @src_addr_4: the fourth 4 bytes of the IP address to load
**/
s32 ixgbe_atr_set_src_ipv6_82599(struct ixgbe_atr_input *input,
- u32 src_addr_1, u32 src_addr_2,
- u32 src_addr_3, u32 src_addr_4)
+ u32 src_addr_1, u32 src_addr_2,
+ u32 src_addr_3, u32 src_addr_4)
{
input->byte_stream[IXGBE_ATR_SRC_IPV6_OFFSET] = src_addr_4 & 0xff;
input->byte_stream[IXGBE_ATR_SRC_IPV6_OFFSET + 1] =
@@ -1645,8 +1630,8 @@ s32 ixgbe_atr_set_src_ipv6_82599(struct ixgbe_atr_input *input,
* @dst_addr_4: the fourth 4 bytes of the IP address to load
**/
s32 ixgbe_atr_set_dst_ipv6_82599(struct ixgbe_atr_input *input,
- u32 dst_addr_1, u32 dst_addr_2,
- u32 dst_addr_3, u32 dst_addr_4)
+ u32 dst_addr_1, u32 dst_addr_2,
+ u32 dst_addr_3, u32 dst_addr_4)
{
input->byte_stream[IXGBE_ATR_DST_IPV6_OFFSET] = dst_addr_4 & 0xff;
input->byte_stream[IXGBE_ATR_DST_IPV6_OFFSET + 1] =
@@ -1723,7 +1708,8 @@ s32 ixgbe_atr_set_flex_byte_82599(struct ixgbe_atr_input *input, u16 flex_byte)
* @input: input stream to modify
* @vm_pool: the Virtual Machine pool to load
**/
-s32 ixgbe_atr_set_vm_pool_82599(struct ixgbe_atr_input *input, u8 vm_pool)
+s32 ixgbe_atr_set_vm_pool_82599(struct ixgbe_atr_input *input,
+ u8 vm_pool)
{
input->byte_stream[IXGBE_ATR_VM_POOL_OFFSET] = vm_pool;
@@ -1747,7 +1733,8 @@ s32 ixgbe_atr_set_l4type_82599(struct ixgbe_atr_input *input, u8 l4type)
* @input: input stream to search
* @vlan: the VLAN id to load
**/
-s32 ixgbe_atr_get_vlan_id_82599(struct ixgbe_atr_input *input, u16 *vlan)
+static s32 ixgbe_atr_get_vlan_id_82599(struct ixgbe_atr_input *input,
+ u16 *vlan)
{
*vlan = input->byte_stream[IXGBE_ATR_VLAN_OFFSET];
*vlan |= input->byte_stream[IXGBE_ATR_VLAN_OFFSET + 1] << 8;
@@ -1760,7 +1747,8 @@ s32 ixgbe_atr_get_vlan_id_82599(struct ixgbe_atr_input *input, u16 *vlan)
* @input: input stream to search
* @src_addr: the IP address to load
**/
-s32 ixgbe_atr_get_src_ipv4_82599(struct ixgbe_atr_input *input, u32 *src_addr)
+static s32 ixgbe_atr_get_src_ipv4_82599(struct ixgbe_atr_input *input,
+ u32 *src_addr)
{
*src_addr = input->byte_stream[IXGBE_ATR_SRC_IPV4_OFFSET];
*src_addr |= input->byte_stream[IXGBE_ATR_SRC_IPV4_OFFSET + 1] << 8;
@@ -1775,7 +1763,8 @@ s32 ixgbe_atr_get_src_ipv4_82599(struct ixgbe_atr_input *input, u32 *src_addr)
* @input: input stream to search
* @dst_addr: the IP address to load
**/
-s32 ixgbe_atr_get_dst_ipv4_82599(struct ixgbe_atr_input *input, u32 *dst_addr)
+static s32 ixgbe_atr_get_dst_ipv4_82599(struct ixgbe_atr_input *input,
+ u32 *dst_addr)
{
*dst_addr = input->byte_stream[IXGBE_ATR_DST_IPV4_OFFSET];
*dst_addr |= input->byte_stream[IXGBE_ATR_DST_IPV4_OFFSET + 1] << 8;
@@ -1793,9 +1782,9 @@ s32 ixgbe_atr_get_dst_ipv4_82599(struct ixgbe_atr_input *input, u32 *dst_addr)
* @src_addr_3: the third 4 bytes of the IP address to load
* @src_addr_4: the fourth 4 bytes of the IP address to load
**/
-s32 ixgbe_atr_get_src_ipv6_82599(struct ixgbe_atr_input *input,
- u32 *src_addr_1, u32 *src_addr_2,
- u32 *src_addr_3, u32 *src_addr_4)
+static s32 ixgbe_atr_get_src_ipv6_82599(struct ixgbe_atr_input *input,
+ u32 *src_addr_1, u32 *src_addr_2,
+ u32 *src_addr_3, u32 *src_addr_4)
{
*src_addr_1 = input->byte_stream[IXGBE_ATR_SRC_IPV6_OFFSET + 12];
*src_addr_1 = input->byte_stream[IXGBE_ATR_SRC_IPV6_OFFSET + 13] << 8;
@@ -1829,8 +1818,8 @@ s32 ixgbe_atr_get_src_ipv6_82599(struct ixgbe_atr_input *input,
* @dst_addr_4: the fourth 4 bytes of the IP address to load
**/
s32 ixgbe_atr_get_dst_ipv6_82599(struct ixgbe_atr_input *input,
- u32 *dst_addr_1, u32 *dst_addr_2,
- u32 *dst_addr_3, u32 *dst_addr_4)
+ u32 *dst_addr_1, u32 *dst_addr_2,
+ u32 *dst_addr_3, u32 *dst_addr_4)
{
*dst_addr_1 = input->byte_stream[IXGBE_ATR_DST_IPV6_OFFSET + 12];
*dst_addr_1 = input->byte_stream[IXGBE_ATR_DST_IPV6_OFFSET + 13] << 8;
@@ -1865,7 +1854,8 @@ s32 ixgbe_atr_get_dst_ipv6_82599(struct ixgbe_atr_input *input,
* endianness when retrieving the data. This can be confusing since the
* internal hash engine expects it to be big-endian.
**/
-s32 ixgbe_atr_get_src_port_82599(struct ixgbe_atr_input *input, u16 *src_port)
+static s32 ixgbe_atr_get_src_port_82599(struct ixgbe_atr_input *input,
+ u16 *src_port)
{
*src_port = input->byte_stream[IXGBE_ATR_SRC_PORT_OFFSET] << 8;
*src_port |= input->byte_stream[IXGBE_ATR_SRC_PORT_OFFSET + 1];
@@ -1883,7 +1873,8 @@ s32 ixgbe_atr_get_src_port_82599(struct ixgbe_atr_input *input, u16 *src_port)
* endianness when retrieving the data. This can be confusing since the
* internal hash engine expects it to be big-endian.
**/
-s32 ixgbe_atr_get_dst_port_82599(struct ixgbe_atr_input *input, u16 *dst_port)
+static s32 ixgbe_atr_get_dst_port_82599(struct ixgbe_atr_input *input,
+ u16 *dst_port)
{
*dst_port = input->byte_stream[IXGBE_ATR_DST_PORT_OFFSET] << 8;
*dst_port |= input->byte_stream[IXGBE_ATR_DST_PORT_OFFSET + 1];
@@ -1896,7 +1887,8 @@ s32 ixgbe_atr_get_dst_port_82599(struct ixgbe_atr_input *input, u16 *dst_port)
* @input: input stream to modify
* @flex_bytes: the flexible bytes to load
**/
-s32 ixgbe_atr_get_flex_byte_82599(struct ixgbe_atr_input *input, u16 *flex_byte)
+static s32 ixgbe_atr_get_flex_byte_82599(struct ixgbe_atr_input *input,
+ u16 *flex_byte)
{
*flex_byte = input->byte_stream[IXGBE_ATR_FLEX_BYTE_OFFSET];
*flex_byte |= input->byte_stream[IXGBE_ATR_FLEX_BYTE_OFFSET + 1] << 8;
@@ -1909,7 +1901,8 @@ s32 ixgbe_atr_get_flex_byte_82599(struct ixgbe_atr_input *input, u16 *flex_byte)
* @input: input stream to modify
* @vm_pool: the Virtual Machine pool to load
**/
-s32 ixgbe_atr_get_vm_pool_82599(struct ixgbe_atr_input *input, u8 *vm_pool)
+s32 ixgbe_atr_get_vm_pool_82599(struct ixgbe_atr_input *input,
+ u8 *vm_pool)
{
*vm_pool = input->byte_stream[IXGBE_ATR_VM_POOL_OFFSET];
@@ -1921,7 +1914,8 @@ s32 ixgbe_atr_get_vm_pool_82599(struct ixgbe_atr_input *input, u8 *vm_pool)
* @input: input stream to modify
* @l4type: the layer 4 type value to load
**/
-s32 ixgbe_atr_get_l4type_82599(struct ixgbe_atr_input *input, u8 *l4type)
+static s32 ixgbe_atr_get_l4type_82599(struct ixgbe_atr_input *input,
+ u8 *l4type)
{
*l4type = input->byte_stream[IXGBE_ATR_L4TYPE_OFFSET];
@@ -2002,9 +1996,9 @@ s32 ixgbe_fdir_add_signature_filter_82599(struct ixgbe_hw *hw,
* hardware writes must be protected from one another.
**/
s32 ixgbe_fdir_add_perfect_filter_82599(struct ixgbe_hw *hw,
- struct ixgbe_atr_input *input,
- u16 soft_id,
- u8 queue)
+ struct ixgbe_atr_input *input,
+ u16 soft_id,
+ u8 queue)
{
u32 fdircmd = 0;
u32 fdirhash;
@@ -2097,7 +2091,7 @@ s32 ixgbe_fdir_add_perfect_filter_82599(struct ixgbe_hw *hw,
*
* Performs read operation to Omer analog register specified.
**/
-s32 ixgbe_read_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 *val)
+static s32 ixgbe_read_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 *val)
{
u32 core_ctl;
@@ -2119,7 +2113,7 @@ s32 ixgbe_read_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 *val)
*
* Performs write operation to Omer analog register specified.
**/
-s32 ixgbe_write_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 val)
+static s32 ixgbe_write_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 val)
{
u32 core_ctl;
@@ -2139,7 +2133,7 @@ s32 ixgbe_write_analog_reg8_82599(struct ixgbe_hw *hw, u32 reg, u8 val)
* Then performs device-specific:
* Clears the rate limiter registers.
**/
-s32 ixgbe_start_hw_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_start_hw_82599(struct ixgbe_hw *hw)
{
u32 q_num;
s32 ret_val;
@@ -2168,7 +2162,7 @@ s32 ixgbe_start_hw_82599(struct ixgbe_hw *hw)
*
* Determines the physical layer module found on the current adapter.
**/
-s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
+static s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
{
s32 status = IXGBE_ERR_PHY_ADDR_INVALID;
status = ixgbe_identify_phy_generic(hw);
@@ -2183,7 +2177,7 @@ s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
*
* Determines physical layer capabilities of the current configuration.
**/
-u32 ixgbe_get_supported_physical_layer_82599(struct ixgbe_hw *hw)
+static u32 ixgbe_get_supported_physical_layer_82599(struct ixgbe_hw *hw)
{
u32 physical_layer = IXGBE_PHYSICAL_LAYER_UNKNOWN;
u32 autoc = IXGBE_READ_REG(hw, IXGBE_AUTOC);
@@ -2290,7 +2284,7 @@ out:
*
* Enables the Rx DMA unit for 82599
**/
-s32 ixgbe_enable_rx_dma_82599(struct ixgbe_hw *hw, u32 regval)
+static s32 ixgbe_enable_rx_dma_82599(struct ixgbe_hw *hw, u32 regval)
{
#define IXGBE_MAX_SECRX_POLL 30
int i;
@@ -2335,7 +2329,7 @@ s32 ixgbe_enable_rx_dma_82599(struct ixgbe_hw *hw, u32 regval)
* This function will read the EEPROM location for the device capabilities,
* and return the word through device_caps.
**/
-s32 ixgbe_get_device_caps_82599(struct ixgbe_hw *hw, u16 *device_caps)
+static s32 ixgbe_get_device_caps_82599(struct ixgbe_hw *hw, u16 *device_caps)
{
hw->eeprom.ops.read(hw, IXGBE_DEVICE_CAPS, device_caps);
@@ -2351,8 +2345,8 @@ s32 ixgbe_get_device_caps_82599(struct ixgbe_hw *hw, u16 *device_caps)
* pointer, and returns the value at that location. This is used in both
* get and set mac_addr routines.
**/
-s32 ixgbe_get_san_mac_addr_offset_82599(struct ixgbe_hw *hw,
- u16 *san_mac_offset)
+static s32 ixgbe_get_san_mac_addr_offset_82599(struct ixgbe_hw *hw,
+ u16 *san_mac_offset)
{
/*
* First read the EEPROM pointer to see if the MAC addresses are
@@ -2373,7 +2367,7 @@ s32 ixgbe_get_san_mac_addr_offset_82599(struct ixgbe_hw *hw,
* set_lan_id() is called by identify_sfp(), but this cannot be relied
* upon for non-SFP connections, so we must call it here.
**/
-s32 ixgbe_get_san_mac_addr_82599(struct ixgbe_hw *hw, u8 *san_mac_addr)
+static s32 ixgbe_get_san_mac_addr_82599(struct ixgbe_hw *hw, u8 *san_mac_addr)
{
u16 san_mac_data, san_mac_offset;
u8 i;
diff --git a/drivers/net/ixgbe/ixgbe_common.c b/drivers/net/ixgbe/ixgbe_common.c
index 96a1859..6621e17 100644
--- a/drivers/net/ixgbe/ixgbe_common.c
+++ b/drivers/net/ixgbe/ixgbe_common.c
@@ -53,6 +53,7 @@ static void ixgbe_enable_rar(struct ixgbe_hw *hw, u32 index);
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);
/**
* ixgbe_start_hw_generic - Prepare hardware for Tx/Rx
@@ -1815,7 +1816,7 @@ out:
*
* Called at init time to set up flow control.
**/
-s32 ixgbe_setup_fc(struct ixgbe_hw *hw, s32 packetbuf_num)
+static s32 ixgbe_setup_fc(struct ixgbe_hw *hw, s32 packetbuf_num)
{
s32 ret_val = 0;
u32 reg;
diff --git a/drivers/net/ixgbe/ixgbe_common.h b/drivers/net/ixgbe/ixgbe_common.h
index 0d34d4d..27f3214 100644
--- a/drivers/net/ixgbe/ixgbe_common.h
+++ b/drivers/net/ixgbe/ixgbe_common.h
@@ -64,7 +64,6 @@ s32 ixgbe_update_uc_addr_list_generic(struct ixgbe_hw *hw,
s32 ixgbe_enable_mc_generic(struct ixgbe_hw *hw);
s32 ixgbe_disable_mc_generic(struct ixgbe_hw *hw);
s32 ixgbe_enable_rx_dma_generic(struct ixgbe_hw *hw, u32 regval);
-s32 ixgbe_setup_fc(struct ixgbe_hw *hw, s32 packetbuf_num);
s32 ixgbe_fc_enable_generic(struct ixgbe_hw *hw, s32 packtetbuf_num);
s32 ixgbe_fc_autoneg(struct ixgbe_hw *hw);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index d69d277..3f17706 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -3615,7 +3615,7 @@ static void ixgbe_free_q_vectors(struct ixgbe_adapter *adapter)
}
}
-void ixgbe_reset_interrupt_capability(struct ixgbe_adapter *adapter)
+static void ixgbe_reset_interrupt_capability(struct ixgbe_adapter *adapter)
{
if (adapter->flags & IXGBE_FLAG_MSIX_ENABLED) {
adapter->flags &= ~IXGBE_FLAG_MSIX_ENABLED;
^ permalink raw reply related
* [net-next PATCH 1/3] ixgbe: Fix isues while reporting 8259x backplane link capabilities
From: Jeff Kirsher @ 2009-08-25 14:47 UTC (permalink / raw)
To: davem
Cc: netdev, gospo, Mallikarjuna R Chilakala, Peter P Waskiewicz Jr,
Jeff Kirsher
From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Fix ethtool get_settings logic to report 10G & 1G advertised and
supported link modes in all 8259x 10G backplane connection types
except for 82598EB BX network connection type.
Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_ethtool.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index dd221bb..1444ec5 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -169,23 +169,20 @@ static int ixgbe_get_settings(struct net_device *netdev,
}
} else if (hw->phy.media_type == ixgbe_media_type_backplane) {
/* Set as FIBRE until SERDES defined in kernel */
- switch (hw->device_id) {
- case IXGBE_DEV_ID_82598:
- ecmd->supported |= (SUPPORTED_1000baseT_Full |
- SUPPORTED_FIBRE);
- ecmd->advertising = (ADVERTISED_10000baseT_Full |
- ADVERTISED_1000baseT_Full |
- ADVERTISED_FIBRE);
- ecmd->port = PORT_FIBRE;
- break;
- case IXGBE_DEV_ID_82598_BX:
+ if (hw->device_id == IXGBE_DEV_ID_82598_BX) {
ecmd->supported = (SUPPORTED_1000baseT_Full |
SUPPORTED_FIBRE);
ecmd->advertising = (ADVERTISED_1000baseT_Full |
ADVERTISED_FIBRE);
ecmd->port = PORT_FIBRE;
ecmd->autoneg = AUTONEG_DISABLE;
- break;
+ } else {
+ ecmd->supported |= (SUPPORTED_1000baseT_Full |
+ SUPPORTED_FIBRE);
+ ecmd->advertising = (ADVERTISED_10000baseT_Full |
+ ADVERTISED_1000baseT_Full |
+ ADVERTISED_FIBRE);
+ ecmd->port = PORT_FIBRE;
}
} else {
ecmd->supported |= SUPPORTED_FIBRE;
^ permalink raw reply related
* [net-next PATCH] e1000: Remove unused function e1000_mta_set.
From: Jeff Kirsher @ 2009-08-25 14:43 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Dave Graham, Jeff Kirsher
From: Graham, David <david.graham@intel.com>
Remove function e1000_mta_set, as it is no longer called
Signed-off-by: Dave Graham <david.graham@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000/e1000_hw.c | 46 ------------------------------------------
1 files changed, 0 insertions(+), 46 deletions(-)
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 1e5ae11..cda6b39 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -5759,52 +5759,6 @@ u32 e1000_hash_mc_addr(struct e1000_hw *hw, u8 *mc_addr)
}
/******************************************************************************
- * Sets the bit in the multicast table corresponding to the hash value.
- *
- * hw - Struct containing variables accessed by shared code
- * hash_value - Multicast address hash value
- *****************************************************************************/
-void e1000_mta_set(struct e1000_hw *hw, u32 hash_value)
-{
- u32 hash_bit, hash_reg;
- u32 mta;
- u32 temp;
-
- /* The MTA is a register array of 128 32-bit registers.
- * It is treated like an array of 4096 bits. We want to set
- * bit BitArray[hash_value]. So we figure out what register
- * the bit is in, read it, OR in the new bit, then write
- * back the new value. The register is determined by the
- * upper 7 bits of the hash value and the bit within that
- * register are determined by the lower 5 bits of the value.
- */
- hash_reg = (hash_value >> 5) & 0x7F;
- if (hw->mac_type == e1000_ich8lan)
- hash_reg &= 0x1F;
-
- hash_bit = hash_value & 0x1F;
-
- mta = E1000_READ_REG_ARRAY(hw, MTA, hash_reg);
-
- mta |= (1 << hash_bit);
-
- /* If we are on an 82544 and we are trying to write an odd offset
- * in the MTA, save off the previous entry before writing and
- * restore the old value after writing.
- */
- if ((hw->mac_type == e1000_82544) && ((hash_reg & 0x1) == 1)) {
- temp = E1000_READ_REG_ARRAY(hw, MTA, (hash_reg - 1));
- E1000_WRITE_REG_ARRAY(hw, MTA, hash_reg, mta);
- E1000_WRITE_FLUSH();
- E1000_WRITE_REG_ARRAY(hw, MTA, (hash_reg - 1), temp);
- E1000_WRITE_FLUSH();
- } else {
- E1000_WRITE_REG_ARRAY(hw, MTA, hash_reg, mta);
- E1000_WRITE_FLUSH();
- }
-}
-
-/******************************************************************************
* Puts an ethernet address into a receive address register.
*
* hw - Struct containing variables accessed by shared code
^ permalink raw reply related
* RE: TCP keepalive timer problem
From: Li_Xin2 @ 2009-08-25 14:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: linux-kernel, netdev
In-Reply-To: <4A93E36C.8070502@gmail.com>
Thanks for your quick reply, let me explain my problem in detail.
Suppose the client side of communication sets the keep alive socket option, connects to server, then we pulls out the network cable of server box. After the connection is idle for TCP_KEEPIDLE seconds, the first keepalive probe packet is sent, and of course no reply is received. Just after the first probe packet, the client sends some data. No response is received, and as you said, the normal retransmission takes place and no further keepalive probe will be sent.
The problem is: application that tries the keepalive mechanism expects communication peer crash detection within TCP_KEEPIDLE + TCP_KEEPCNT * TCP_KEEPINTVL seconds. Application may set relative smaller TCP_KEEPIDLE, TCP_KEEPCNT and TCP_KEEPINTVL value so that peer crash can be detected quickly, for example, 60 seconds. But if the keepalive is intervened with retransmission, the latter takes higher priority, so that peer crash will be detected after 13 to 30 minutes, which may not be acceptable for some applications.
We tried TCP implementation on Windows XP SP3, the keepalive and retransmission don't intervene.
Regards,
Xin Li
EMC Shanghai R&D Centre
Email: Li_Xin2@emc.com
Tel: 86 21 6095 1100 x 2257
-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: 2009年8月25日 21:13
To: Li, Xin
Cc: linux-kernel@vger.kernel.org; Linux Netdev List
Subject: Re: TCP keepalive timer problem
Li_Xin2@emc.com a écrit :
> Greetings,
>
> I found one problem in Linux TCP keepalive timer processing, after
> searching on google, I found Daniel Stempel reported the same problem in
> 2007 (http://lkml.indiana.edu/hypermail/linux/kernel/0702.2/1136.html),
> but got no answer. So I have to reraise it.
>
> Can anyone help answer this two-years long question?
>
>
You should explain your problem in detail, since Daniel one was probably different.
He mentioned "(timeout is set to e.g. 30 seconds)" which is kind of nasty, given normal one is 7200
If some packets are in flight, keepalive is not fired at all, since normal
retransmits should take place (check tcp_retries2 sysctl).
TCP Keepalive is only fired when no trafic occurred for a long time, only if
SO_KEEPALIVE socket option was enabled by application.
tcp_retries2 (integer; default: 15)
The maximum number of times a TCP packet is retransmitted in established state
before giving up. The default value is 15, which corresponds to a duration of
approximately between 13 to 30 minutes, depending on the retransmission timeout.
The RFC 1122 specified minimum limit of 100 seconds is typically deemed too short.
^ permalink raw reply
* Re: TCP keepalive timer problem
From: Andi Kleen @ 2009-08-25 14:04 UTC (permalink / raw)
To: Li_Xin2; +Cc: linux-kernel, netdev
In-Reply-To: <0939B589FC103041945B9F13274963E303B1A9D4@CORPUSMX90A.corp.emc.com>
<Li_Xin2@emc.com> writes:
[cc netdev]
> Greetings,
>
> I found one problem in Linux TCP keepalive timer processing, after
> searching on google, I found Daniel Stempel reported the same problem in
> 2007 (http://lkml.indiana.edu/hypermail/linux/kernel/0702.2/1136.html),
> but got no answer. So I have to reraise it.
>
> Can anyone help answer this two-years long question?
I think the idea behind the tcp_keepalive_timer check referrenced in the
other mail is: when there are outstanding non acked packets then
the normal retransmit timer will do keep alive because it will
retransmit and retransmit in exponential backoff and eventually notice
something is wrong.
The obvious hole is that if the keepalive is shorter than the worst
case retransmit timeout then you'll have to wait for the longer
timeout. I presume that's what is happening for you? You set the
keep alive timeout very low and expect the timeout to be very low,
but it's 30+mins (default retransmit timeout)?
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [Bridge] VLANs and bridge
From: Patrick McHardy @ 2009-08-25 14:00 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: Simon Barber, bridge, netdev
In-Reply-To: <OF7400A5E8.F4AC001F-ONC125761C.00674A25-C125761C.00679758@transmode.se>
Joakim Tjernlund wrote:
> Simon Barber <simon@superduper.net> wrote on 22/08/2009 19:12:10:
>> Looking through B.1.3 it looks like the patch would need some
>> enhancement. It provides a good basis - handling tagging/untagging and
>> filtering, but would need a way to specify the untagged vlan separately
>> for in and out.
>
> I see. Perhaps the VLAN maintainer(CC:ed) can comment too. Especially about extending
> the VLAN code to allowed several VLANs in one interface?
Just accepting additional VIDs on one VLAN device should be
a relatively trivial change, all you need to do is call
vlan_group_set_device() with the additional VIDs.
I'd suggest to add something similar to the QoS-mapping lists
in vlan_netlink.c for the additional VIDs.
^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Christoph Lameter @ 2009-08-25 14:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sridhar Samudrala, Nivedita Singhvi, netdev, David S. Miller
In-Reply-To: <4A93EB9B.5020600@gmail.com>
On Tue, 25 Aug 2009, Eric Dumazet wrote:
> Please hold on, I would like to fully understand what's happening,
> and test the patch :)
Ok. It would be good if the drops would also be somehow noted by the UDP
subsystem (one should see something with netstat -su) and may be even the
socket. I see a drops column in /proc/net/udp. rx_drops, tx_drops?
^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Eric Dumazet @ 2009-08-25 13:48 UTC (permalink / raw)
To: Christoph Lameter
Cc: Sridhar Samudrala, Nivedita Singhvi, netdev, David S. Miller
In-Reply-To: <alpine.DEB.1.10.0908250945460.5972@gentwo.org>
Christoph Lameter a écrit :
> On Tue, 25 Aug 2009, Eric Dumazet wrote:
>
>> NET_XMIT_CN strikes again :)
>>
>> Well, if ip_local_out() returns a negative error (say -EPERM for example),
>> your patch disables OUTDISCARDS increments.
>>
>> Maybe a simpler patch like this one ?
>
> Yes looks good. Can we get this in soon?
>
Please hold on, I would like to fully understand what's happening,
and test the patch :)
^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Christoph Lameter @ 2009-08-25 13:48 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: Eric Dumazet, Nivedita Singhvi, netdev, David S. Miller
In-Reply-To: <1251153361.29173.25.camel@w-sridhar.beaverton.ibm.com>
On Mon, 24 Aug 2009, Sridhar Samudrala wrote:
> So are you hitting this case with your workload and does this account for all the
> packet losses you are seeing?
Yes.
> If we are dropping the packet and returing NET_XMIT_DROP, should
> we also increment qdisc drop stats (sch->qstats.drops)?
I think so but I am no expert. I was surprised to not even see counter
increments at that level. But I was content to fix up the higher level
tracking to have at least one counter that showed the packet loss.
> If we count these drops as qdisc drops, should we also count them as IP OUTDISCARDS?
Yes.
^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Christoph Lameter @ 2009-08-25 13:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sridhar Samudrala, Nivedita Singhvi, netdev, David S. Miller
In-Reply-To: <4A930DEF.5000008@gmail.com>
On Tue, 25 Aug 2009, Eric Dumazet wrote:
> NET_XMIT_CN strikes again :)
>
> Well, if ip_local_out() returns a negative error (say -EPERM for example),
> your patch disables OUTDISCARDS increments.
>
> Maybe a simpler patch like this one ?
Yes looks good. Can we get this in soon?
^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Christoph Lameter @ 2009-08-25 13:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sridhar Samudrala, Nivedita Singhvi, netdev
In-Reply-To: <4A931ED3.1090206@gmail.com>
On Tue, 25 Aug 2009, Eric Dumazet wrote:
> Thread model: posix
> gcc version 4.4.1 (GCC)
> # pwd
> /opt/src/lldiag-0.14
> # make
> gcc -Wall -omcast mcast.c -lrt -lm
> mcast.c: In function ?set_ip?:
> mcast.c:121: warning: implicit declaration of function ?htons?
> mcast.c: In function ?build_pattern_array?:
> mcast.c:168: warning: implicit declaration of function ?htonl?
> /tmp/cc4sYCDr.o: In function `lock':
> mcast.c:(.text+0xcad): undefined reference to `__sync_fetch_and_add_4'
> collect2: ld returned 1 exit status
> make: *** [mcast] Error 1
>
>
> I have no idea where is defined sync_fetch_and_add
Its a GCC builtin for x86 sigh. Ok will put out 0.15 that disables their
use by default.
^ permalink raw reply
* Re: [PATCH v2] sctp: fix the check for path failure detection
From: Vlad Yasevich @ 2009-08-25 13:31 UTC (permalink / raw)
To: Luo Chunbo; +Cc: Wei Yongjun, davem, netdev, linux-sctp, linux-kernel
In-Reply-To: <1251188548.6498.28.camel@pek-cluo-desktop>
Luo Chunbo wrote:
> On Tue, 2009-08-25 at 15:36 +0800, Wei Yongjun wrote:
>> Chunbo Luo wrote:
>>> The transport error count should be incremented when an outstanding
>>> HB is not acknowledged. And the path failure detection should be done
>>> before sending out the HB.
>>>
>>> Signed-off-by: Chunbo Luo <chunbo.luo@windriver.com>
>>> ---
>>> net/sctp/sm_sideeffect.c | 6 +++++-
>>> net/sctp/sm_statefuns.c | 8 ++++----
>>> 2 files changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>>> index 86426aa..fbdf4de 100644
>>> --- a/net/sctp/sm_sideeffect.c
>>> +++ b/net/sctp/sm_sideeffect.c
>>> @@ -446,7 +446,11 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
>>> if (transport->state != SCTP_UNCONFIRMED)
>>> asoc->overall_error_count++;
>>>
>>> - if (transport->state != SCTP_INACTIVE &&
>>> + /*
>>> + * The transport error count is incremented when an outstanding HB
>>> + * is not acknowledged.
>>> + */
>>>
>> T3-rtx timer expires also need to increment error count.
>
> Yes, T3-rtx timer expires need to increment error count.
>
>>> + if (transport->hb_sent && transport->state != SCTP_INACTIVE &&
>>> (transport->error_count++ >= transport->pathmaxrxt)) {
>>> SCTP_DEBUG_PRINTK_IPADDR("transport_strike:association %p",
>>> " transport IP: port:%d failed.\n",
>>> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
>>> index 7288192..7f77099 100644
>>> --- a/net/sctp/sm_statefuns.c
>>> +++ b/net/sctp/sm_statefuns.c
>>> @@ -981,10 +981,6 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const struct sctp_endpoint *ep,
>>> */
>>>
>>> if (transport->param_flags & SPP_HB_ENABLE) {
>>> - if (SCTP_DISPOSITION_NOMEM ==
>>> - sctp_sf_heartbeat(ep, asoc, type, arg,
>>> - commands))
>>> - return SCTP_DISPOSITION_NOMEM;
>>> /* Set transport error counter and association error counter
>>> * when sending heartbeat.
>>> */
>>> @@ -992,6 +988,10 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const struct sctp_endpoint *ep,
>>> SCTP_TRANSPORT(transport));
>>> sctp_add_cmd_sf(commands, SCTP_CMD_TRANSPORT_HB_SENT,
>>> SCTP_TRANSPORT(transport));
>>> + if (SCTP_DISPOSITION_NOMEM ==
>>> + sctp_sf_heartbeat(ep, asoc, type, arg,
>>> + commands))
>>> + return SCTP_DISPOSITION_NOMEM;
>>> }
>>> sctp_add_cmd_sf(commands, SCTP_CMD_HB_TIMER_UPDATE,
>>> SCTP_TRANSPORT(transport));
>>>
>> How about this one:
>
> It is ok to increment the error count. But the path failure detection
> should be done before send HB. Otherwise, an extra HB chunk will be sent
> out before the transport is marked DOWN . So the changes in
> net/sctp/sm_statefuns.c is also needed.
After running a few simulations with all the different patches from this thread,
what we are really after was achieved by the original patch. Looking back over
the code history, it looks like this was try to implement the old text of
section 8.3 (prior to errata that fixed it).
We still have to send HB on an INACTIVE transport to see if it comes back, so
that comment of my was entirely wrong. Sorry, don't know what I was thinking at
that time...
So I'll take the original patch Luo submitted.
Thanks
-vlad
>
>
>
>> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
>> index 86426aa..fb723dd 100644
>> --- a/net/sctp/sm_sideeffect.c
>> +++ b/net/sctp/sm_sideeffect.c
>> @@ -447,7 +447,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
>> asoc->overall_error_count++;
>>
>> if (transport->state != SCTP_INACTIVE &&
>> - (transport->error_count++ >= transport->pathmaxrxt)) {
>> + (transport->error_count >= transport->pathmaxrxt)) {
>> SCTP_DEBUG_PRINTK_IPADDR("transport_strike:association %p",
>> " transport IP: port:%d failed.\n",
>> asoc,
>> @@ -468,6 +468,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
>> * that indicates that we have an outstanding HB.
>> */
>> if (!is_hb || transport->hb_sent) {
>> + transport->error_count++;
>> transport->last_rto = transport->rto;
>> transport->rto = min((transport->rto * 2), transport->asoc->rto_max);
>> }
>>
>>
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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
* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-25 13:16 UTC (permalink / raw)
To: Rusty Russell
Cc: virtualization, netdev, kvm, linux-kernel, mingo, linux-mm, akpm,
hpa, gregory.haskins
In-Reply-To: <200908252140.41295.rusty@rustcorp.com.au>
Thanks for the comments, I'll work on them ASAP.
Answers to questions and more comments below.
On Tue, Aug 25, 2009 at 09:40:40PM +0930, Rusty Russell wrote:
> On Thu, 20 Aug 2009 12:33:09 am Michael S. Tsirkin wrote:
> > What it is: vhost net is a character device that can be used to reduce
> > the number of system calls involved in virtio networking.
> > Existing virtio net code is used in the guest without modification.
>
> ...
>
> > +config VHOST_NET
> > + tristate "Host kernel accelerator for virtio net"
> > + depends on NET && EVENTFD
> > + ---help---
> > + This kernel module can be loaded in host kernel to accelerate
> > + guest networking with virtio_net. Not to be confused with virtio_net
> > + module itself which needs to be loaded in guest kernel.
> > +
> > + To compile this driver as a module, choose M here: the module will
> > + be called vhost_net.
>
> Just want to note that the patch explanation and the Kconfig help text are
> exceptional examples of reader-oriented text. Nice!
High praise indeed from the author of the lguest Quest :).
> > + /* Sanity check */
> > + if (vq->iov->iov_len != sizeof(struct virtio_net_hdr)) {
> > + vq_err(vq, "Unexpected header len for TX: "
> > + "%ld expected %zd\n", vq->iov->iov_len,
> > + sizeof(struct virtio_net_hdr));
> > + break;
> > + }
>
> OK, this check, which is in the qemu version, is *wrong*. There should be
> no assumption on sg boundaries. For example, the guest should be able to
> put the virtio_net_hdr in the front of the skbuf data if there is room.
>
> You should try to explicitly "consume" sizeof(struct virtio_net_hdr) of the
> iov, and if fail, do this message. You can skip the out <= 1 test then,
> too.
>
> Anyway, I really prefer vq->iov[0]. to vq->iov-> here.
I'll fix that. Probably should fix qemu as well.
> > + /* Sanity check */
> > + if (vq->iov->iov_len != sizeof(struct virtio_net_hdr)) {
> > + vq_err(vq, "Unexpected header len for RX: "
> > + "%ld expected %zd\n",
> > + vq->iov->iov_len, sizeof(struct virtio_net_hdr));
> > + break;
>
> Here too.
>
> > + u32 __user *featurep = argp;
> > + int __user *fdp = argp;
> > + u32 features;
> > + int fd, r;
> > + switch (ioctl) {
> > + case VHOST_NET_SET_SOCKET:
> > + r = get_user(fd, fdp);
> > + if (r < 0)
> > + return r;
> > + return vhost_net_set_socket(n, fd);
> > + case VHOST_GET_FEATURES:
> > + /* No features for now */
> > + features = 0;
> > + return put_user(features, featurep);
>
> We may well get more than 32 feature bits, at least for virtio_net, which will
> force us to do some trickery in virtio_pci. I'd like to avoid that here,
> though it's kind of ugly. We'd need VHOST_GET_FEATURES (and ACK) to take a
> struct like:
>
> u32 feature_size;
> u32 features[];
Do you feel just making it 64 bit won't be enough? How about 128 bit?
> > +int vhost_net_init(void)
>
> static?
>
> > +void vhost_net_exit(void)
>
> static?
Good catch.
> > +/* Start polling a file. We add ourselves to file's wait queue. The user must
> > + * keep a reference to a file until after vhost_poll_stop is called. */
>
> I experienced minor confusion from the comments in this file. Where you said
> "user" I think "caller". No biggie though.
>
> > + memory->nregions = 2;
> > + memory->regions[0].guest_phys_addr = 1;
> > + memory->regions[0].userspace_addr = 1;
> > + memory->regions[0].memory_size = ~0ULL;
> > + memory->regions[1].guest_phys_addr = 0;
> > + memory->regions[1].userspace_addr = 0;
> > + memory->regions[1].memory_size = 1;
>
> Not sure I understand why there are two regions to start?
We are trying to cover a whole 2^64 range here. It's size does not fit
in a single 64 bit length value. I could special case 0 length to mean
2^64, but decided against it.
> > + case VHOST_SET_VRING_BASE:
> > + r = copy_from_user(&s, argp, sizeof s);
> > + if (r < 0)
> > + break;
> > + if (s.num > 0xffff) {
> > + r = -EINVAL;
> > + break;
> > + }
> > + vq->last_avail_idx = s.num;
> > + break;
> > + case VHOST_GET_VRING_BASE:
> > + s.index = idx;
> > + s.num = vq->last_avail_idx;
> > + r = copy_to_user(argp, &s, sizeof s);
> > + break;
>
> Ah, this is my fault. I didn't expose the last_avail_idx in the ring
> because the other side doesn't need it; but without it the ring state is not
> fully observable from outside (no external save / restore!).
>
> I have a patch which published these indices (we have room), see:
> http://ozlabs.org/~rusty/kernel/rr-2009-08-12-1/virtio:ring-publish-indices.patch
>
> Perhaps we should use that mechanism instead? We don't actually have to
> offer the feature (we don't care about the guest state), but it's nice as
> documentation. I've been waiting for an excuse to use that patch.
Good idea and might be handy for optimizations as well, and I
would have used it if it was there. I'd like to support existing guests
in vhost though, so I think we need to support this ioctl for now.
> > +long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg)
> > +{
> > + void __user *argp = (void __user *)arg;
> > + long r;
> > +
> > + mutex_lock(&d->mutex);
> > + if (ioctl == VHOST_SET_OWNER) {
> > + r = vhost_dev_set_owner(d);
> > + goto done;
> > + }
> > +
> > + r = vhost_dev_check_owner(d);
>
> You can do a VHOST_SET_OWNER without being the owner?
Only if no one else is the owner. It has a mutual exclusion
mechanism.
> So really,
> the -EPERM from all the vhost_dev_check_owner() is not a security thing,
> but a "I don't think you mean to do that" thing?
Mostly that. I started by assuming that an open fd can't be
passed around at all, but talking with management guys here,
they really like being able to open fds and pass them around
through unix domain sockets.
Since using this fd from multiple processes does not work well,
I decided to make this explicit in the interface. I find it quite
possible that there's no security thing here, but this is just a simpler
model to think about than guessing whether some crash is exploitable or
not.
> If so, a comment above it might help?
Yes.
> > +static const struct vhost_memory_region *find_region(struct vhost_memory *mem,
> > + __u64 addr, __u32 len)
> > +{
> > + struct vhost_memory_region *reg;
> > + int i;
> > + /* linear search is not brilliant, but we really have on the order of 6
> > + * regions in practice */
>
> Ah, you actually mean "this code has been carefully cache-optimized for the
> common case" :)
>
> > +/* FIXME: this does not handle a region that spans multiple
> > + * address/len pairs */
> > +int translate_desc(struct vhost_dev *dev, u64 addr, u32 len,
> > + struct iovec iov[], int iov_count, int iov_size,
> > + unsigned *num)
> > +{
> > + const struct vhost_memory_region *reg;
> > + struct vhost_memory *mem;
> > + struct iovec *_iov;
> > + u64 s = 0;
> > + int ret = 0;
>
> Would this be neater if it returned the num iovecs used? And offsetting
> iov in the caller, rather than passing iov_count?
iov_size would have to be tweaked instead as well.
> > + /* If their number is silly, that's a fatal mistake. */
> > + if (head >= vq->num) {
> > + vq_err(vq, "Guest says index %u > %u is available",
> > + head, vq->num);
> > + return vq->num;
> > + }
>
> Not a fatal mistake in this code/
>
> > + vq->inflight++;
>
> vq->inflight was a brain fart in the old lguest code. See
> commit ebf9a5a99c1a464afe0b4dfa64416fc8b273bc5c.
I wondered about that. OK.
> Also, see other fixes to the lguest launcher since then which might
> be relevant to this code:
> lguest: get more serious about wmb() in example Launcher code
> lguest: clean up length-used value in example launcher
I'll go over them, thanks!
I did look over barriers, but this is tricky stuff.
> > + /* If they don't want an interrupt, don't send one, unless empty. */
> > + if ((flags & VRING_AVAIL_F_NO_INTERRUPT) && vq->inflight)
> > + return;
>
> And I wouldn't support notify on empty at all, TBH.
If I don't, virtio net in guest uses a timer, which might be expensive.
Will need to check what this does.
> It should
> definitely be conditional on the guest accepting the NOTIFY_ON_EMPTY
> feature.
Good point.
> > +/* The virtqueue structure describes a queue attached to a device. */
> > +struct vhost_virtqueue {
> ...
> > +} ____cacheline_aligned;
>
> Really? I'd want to see numbers on this one. False sharing vs. more cache
> utilization.
I don't yet want to focus on micro optimizations. What's your guess?
That it's better without? I'll kill it then ...
> > +#define vq_err(vq, fmt, ...) do { \
> > + printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__); \
> > + if ((vq)->error_ctx) \
> > + eventfd_signal((vq)->error_ctx, 1);\
> > + } while (0)
>
> Mmm... guests should not be able to create unlimited printks in the host.
Yes, but handy for debugging :)
I guess I'll just make it use pr_debug instead?
> But really, nothing major to object to in here...
>
> Thanks!
> Rusty.
^ permalink raw reply
* Re: TCP keepalive timer problem
From: Eric Dumazet @ 2009-08-25 13:13 UTC (permalink / raw)
To: Li_Xin2; +Cc: linux-kernel, Linux Netdev List
In-Reply-To: <0939B589FC103041945B9F13274963E303B1A9D4@CORPUSMX90A.corp.emc.com>
Li_Xin2@emc.com a écrit :
> Greetings,
>
> I found one problem in Linux TCP keepalive timer processing, after
> searching on google, I found Daniel Stempel reported the same problem in
> 2007 (http://lkml.indiana.edu/hypermail/linux/kernel/0702.2/1136.html),
> but got no answer. So I have to reraise it.
>
> Can anyone help answer this two-years long question?
>
>
You should explain your problem in detail, since Daniel one was probably different.
He mentioned "(timeout is set to e.g. 30 seconds)" which is kind of nasty, given normal one is 7200
If some packets are in flight, keepalive is not fired at all, since normal
retransmits should take place (check tcp_retries2 sysctl).
TCP Keepalive is only fired when no trafic occurred for a long time, only if
SO_KEEPALIVE socket option was enabled by application.
tcp_retries2 (integer; default: 15)
The maximum number of times a TCP packet is retransmitted in established state
before giving up. The default value is 15, which corresponds to a duration of
approximately between 13 to 30 minutes, depending on the retransmission timeout.
The RFC 1122 specified minimum limit of 100 seconds is typically deemed too short.
^ permalink raw reply
* Re: iproute2 / tbf with large burst seems broken again
From: Denys Fedoryschenko @ 2009-08-25 12:18 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <20090825121344.GA12320@ff.dom.local>
On Tuesday 25 August 2009 15:13:44 Jarek Poplawski wrote:
> I'll need to find more time to rethink your problem (and this patch),
> because it really looks like your tbf usage is very uncommon. Anyway,
> I guess these changes aren't for 2.6.31, unless there're similar
> requests from other users soon.
>
>Thanks for testing,
>Jarek P.
I guess even no need for any kernel :-)
Just maybe good idea in next kernels to document this or handle overflow, that
it will give warning in kernel. Otherwise user will have non-functional qdisc
that will not pass traffic.
^ permalink raw reply
* Re: iproute2 / tbf with large burst seems broken again
From: Jarek Poplawski @ 2009-08-25 12:13 UTC (permalink / raw)
To: Denys Fedoryschenko; +Cc: netdev
In-Reply-To: <200908251416.13888.denys@visp.net.lb>
On Tue, Aug 25, 2009 at 02:16:13PM +0300, Denys Fedoryschenko wrote:
> Done, tested, it doesn't stale and fix the issue but:
>
> PPPoE_146 ~ # ./tc qdisc del dev ppp13 root;./tc qdisc add dev ppp13 root tbf
> rate 96000 burst 2048000 latency 500ms
>
> This one is ok
> PPPoE_146 ~ # ./tc -s -d qdisc show dev ppp13
> qdisc tbf 8005: root rate 96000bit burst 2000Kb/8 mpu 0b lat 500.0ms
> Sent 55260 bytes 65 pkt (dropped 0, overlimits 0 requeues 0)
> rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc ingress ffff: parent ffff:fff1 ----------------
> Sent 641055 bytes 2485 pkt (dropped 0, overlimits 0 requeues 0)
> rate 0bit 0pps backlog 0b 0p requeues 0
>
> PPPoE_146 ~ # ./tc qdisc del dev ppp13 root;./tc qdisc add dev ppp13 root tbf
> rate 96000 burst 4096000 latency 500ms
>
> But this one maybe will overflow because of limitations in iproute2.
Sure, as I wrote before:
"It shouldn't be so difficult just for 2000kb buffer here, but of course
there're some limits."
I tried to optimize sch_tbf variables usage only, but this is still
mostly within 32 bits.
>
> PPoE_146 ~ # ./tc -s -d qdisc show dev ppp13
> qdisc tbf 8004: root rate 96000bit burst 797465b/8 mpu 0b lat 275.4s
> Sent 82867 bytes 123 pkt (dropped 0, overlimits 0 requeues 0)
> rate 0bit 0pps backlog 0b 0p requeues 0
> qdisc ingress ffff: parent ffff:fff1 ----------------
> Sent 506821 bytes 1916 pkt (dropped 0, overlimits 0 requeues 0)
> rate 0bit 0pps backlog 0b 0p requeues 0
>
> So maybe all of that just wrong way of using TBF.
> At same time this means, if HTB and policers in filters done same way, that
> QoS in Linux cannot do similar to squid delay pools feature:
>
> First 10Mb give with 1Mbit/s, then slow 64Kbit/s. If user use less than 64K -
> recharge with that unused bandwidth a "10 Mb / 1Mbit bucket".
I'll need to find more time to rethink your problem (and this patch),
because it really looks like your tbf usage is very uncommon. Anyway,
I guess these changes aren't for 2.6.31, unless there're similar
requests from other users soon.
Thanks for testing,
Jarek P.
>
> On Tuesday 25 August 2009 12:41:20 Jarek Poplawski wrote:
> > On Tue, Aug 25, 2009 at 09:00:35AM +0000, Jarek Poplawski wrote:
> > > On Tue, Aug 25, 2009 at 08:43:06AM +0000, Jarek Poplawski wrote:
> > > ...
> > >
> > > > since these 64 bits will be needed soon for higher rates anyway, I
> > > > guess we could try some change like the patch below, if you find it
> > > > works for you (I didn't test it yet.)
> >
> > I hope this time it works...
> >
> > Jarek P.
> >
> > --- (take 2)
> >
> > net/sched/sch_tbf.c | 14 +++++++-------
> > 1 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> > index e22dfe8..7d0fe69 100644
> > --- a/net/sched/sch_tbf.c
> > +++ b/net/sched/sch_tbf.c
> > @@ -108,8 +108,8 @@ struct tbf_sched_data
> > struct qdisc_rate_table *P_tab;
> >
> > /* Variables */
> > - long tokens; /* Current number of B tokens */
> > - long ptokens; /* Current number of P tokens */
> > + u32 tokens; /* Current number of B tokens */
> > + u32 ptokens; /* Current number of P tokens */
> > psched_time_t t_c; /* Time check-point */
> > struct Qdisc *qdisc; /* Inner qdisc, default - bfifo queue */
> > struct qdisc_watchdog watchdog; /* Watchdog timer */
> > @@ -160,21 +160,21 @@ static struct sk_buff *tbf_dequeue(struct Qdisc* sch)
> >
> > if (skb) {
> > psched_time_t now;
> > - long toks;
> > - long ptoks = 0;
> > + long long toks;
> > + long long ptoks = 0;
> > unsigned int len = qdisc_pkt_len(skb);
> >
> > now = psched_get_time();
> > - toks = psched_tdiff_bounded(now, q->t_c, q->buffer);
> > + toks = min_t(u32, now - q->t_c, q->buffer);
> >
> > if (q->P_tab) {
> > ptoks = toks + q->ptokens;
> > - if (ptoks > (long)q->mtu)
> > + if (ptoks > q->mtu)
> > ptoks = q->mtu;
> > ptoks -= L2T_P(q, len);
> > }
> > toks += q->tokens;
> > - if (toks > (long)q->buffer)
> > + if (toks > q->buffer)
> > toks = q->buffer;
> > toks -= L2T(q, len);
>
>
^ permalink raw reply
* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Rusty Russell @ 2009-08-25 12:10 UTC (permalink / raw)
To: virtualization
Cc: Michael S. Tsirkin, netdev, kvm, linux-kernel, mingo, linux-mm,
akpm, hpa, gregory.haskins
In-Reply-To: <20090819150309.GC4236@redhat.com>
On Thu, 20 Aug 2009 12:33:09 am Michael S. Tsirkin wrote:
> What it is: vhost net is a character device that can be used to reduce
> the number of system calls involved in virtio networking.
> Existing virtio net code is used in the guest without modification.
...
> +config VHOST_NET
> + tristate "Host kernel accelerator for virtio net"
> + depends on NET && EVENTFD
> + ---help---
> + This kernel module can be loaded in host kernel to accelerate
> + guest networking with virtio_net. Not to be confused with virtio_net
> + module itself which needs to be loaded in guest kernel.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called vhost_net.
Just want to note that the patch explanation and the Kconfig help text are
exceptional examples of reader-oriented text. Nice!
> + /* Sanity check */
> + if (vq->iov->iov_len != sizeof(struct virtio_net_hdr)) {
> + vq_err(vq, "Unexpected header len for TX: "
> + "%ld expected %zd\n", vq->iov->iov_len,
> + sizeof(struct virtio_net_hdr));
> + break;
> + }
OK, this check, which is in the qemu version, is *wrong*. There should be
no assumption on sg boundaries. For example, the guest should be able to
put the virtio_net_hdr in the front of the skbuf data if there is room.
You should try to explicitly "consume" sizeof(struct virtio_net_hdr) of the
iov, and if fail, do this message. You can skip the out <= 1 test then,
too.
Anyway, I really prefer vq->iov[0]. to vq->iov-> here.
> + /* Sanity check */
> + if (vq->iov->iov_len != sizeof(struct virtio_net_hdr)) {
> + vq_err(vq, "Unexpected header len for RX: "
> + "%ld expected %zd\n",
> + vq->iov->iov_len, sizeof(struct virtio_net_hdr));
> + break;
Here too.
> + u32 __user *featurep = argp;
> + int __user *fdp = argp;
> + u32 features;
> + int fd, r;
> + switch (ioctl) {
> + case VHOST_NET_SET_SOCKET:
> + r = get_user(fd, fdp);
> + if (r < 0)
> + return r;
> + return vhost_net_set_socket(n, fd);
> + case VHOST_GET_FEATURES:
> + /* No features for now */
> + features = 0;
> + return put_user(features, featurep);
We may well get more than 32 feature bits, at least for virtio_net, which will
force us to do some trickery in virtio_pci. I'd like to avoid that here,
though it's kind of ugly. We'd need VHOST_GET_FEATURES (and ACK) to take a
struct like:
u32 feature_size;
u32 features[];
> +int vhost_net_init(void)
static?
> +void vhost_net_exit(void)
static?
> +/* Start polling a file. We add ourselves to file's wait queue. The user must
> + * keep a reference to a file until after vhost_poll_stop is called. */
I experienced minor confusion from the comments in this file. Where you said
"user" I think "caller". No biggie though.
> + memory->nregions = 2;
> + memory->regions[0].guest_phys_addr = 1;
> + memory->regions[0].userspace_addr = 1;
> + memory->regions[0].memory_size = ~0ULL;
> + memory->regions[1].guest_phys_addr = 0;
> + memory->regions[1].userspace_addr = 0;
> + memory->regions[1].memory_size = 1;
Not sure I understand why there are two regions to start?
> + case VHOST_SET_VRING_BASE:
> + r = copy_from_user(&s, argp, sizeof s);
> + if (r < 0)
> + break;
> + if (s.num > 0xffff) {
> + r = -EINVAL;
> + break;
> + }
> + vq->last_avail_idx = s.num;
> + break;
> + case VHOST_GET_VRING_BASE:
> + s.index = idx;
> + s.num = vq->last_avail_idx;
> + r = copy_to_user(argp, &s, sizeof s);
> + break;
Ah, this is my fault. I didn't expose the last_avail_idx in the ring
because the other side doesn't need it; but without it the ring state is not
fully observable from outside (no external save / restore!).
I have a patch which published these indices (we have room), see:
http://ozlabs.org/~rusty/kernel/rr-2009-08-12-1/virtio:ring-publish-indices.patch
Perhaps we should use that mechanism instead? We don't actually have to
offer the feature (we don't care about the guest state), but it's nice as
documentation. I've been waiting for an excuse to use that patch.
> +long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> + long r;
> +
> + mutex_lock(&d->mutex);
> + if (ioctl == VHOST_SET_OWNER) {
> + r = vhost_dev_set_owner(d);
> + goto done;
> + }
> +
> + r = vhost_dev_check_owner(d);
You can do a VHOST_SET_OWNER without being the owner? So really,
the -EPERM from all the vhost_dev_check_owner() is not a security thing,
but a "I don't think you mean to do that" thing?
If so, a comment above it might help?
> +static const struct vhost_memory_region *find_region(struct vhost_memory *mem,
> + __u64 addr, __u32 len)
> +{
> + struct vhost_memory_region *reg;
> + int i;
> + /* linear search is not brilliant, but we really have on the order of 6
> + * regions in practice */
Ah, you actually mean "this code has been carefully cache-optimized for the
common case" :)
> +/* FIXME: this does not handle a region that spans multiple
> + * address/len pairs */
> +int translate_desc(struct vhost_dev *dev, u64 addr, u32 len,
> + struct iovec iov[], int iov_count, int iov_size,
> + unsigned *num)
> +{
> + const struct vhost_memory_region *reg;
> + struct vhost_memory *mem;
> + struct iovec *_iov;
> + u64 s = 0;
> + int ret = 0;
Would this be neater if it returned the num iovecs used? And offsetting
iov in the caller, rather than passing iov_count?
> + /* If their number is silly, that's a fatal mistake. */
> + if (head >= vq->num) {
> + vq_err(vq, "Guest says index %u > %u is available",
> + head, vq->num);
> + return vq->num;
> + }
Not a fatal mistake in this code/
> + vq->inflight++;
vq->inflight was a brain fart in the old lguest code. See
commit ebf9a5a99c1a464afe0b4dfa64416fc8b273bc5c.
Also, see other fixes to the lguest launcher since then which might
be relevant to this code:
lguest: get more serious about wmb() in example Launcher code
lguest: clean up length-used value in example launcher
> + /* If they don't want an interrupt, don't send one, unless empty. */
> + if ((flags & VRING_AVAIL_F_NO_INTERRUPT) && vq->inflight)
> + return;
And I wouldn't support notify on empty at all, TBH. It should
definitely be conditional on the guest accepting the NOTIFY_ON_EMPTY
feature.
> +/* The virtqueue structure describes a queue attached to a device. */
> +struct vhost_virtqueue {
...
> +} ____cacheline_aligned;
Really? I'd want to see numbers on this one. False sharing vs. more cache
utilization.
> +#define vq_err(vq, fmt, ...) do { \
> + printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__); \
> + if ((vq)->error_ctx) \
> + eventfd_signal((vq)->error_ctx, 1);\
> + } while (0)
Mmm... guests should not be able to create unlimited printks in the host.
But really, nothing major to object to in here...
Thanks!
Rusty.
^ permalink raw reply
* Re: TSecr != 0 check in inet_lro.c
From: Octavian Purdila @ 2009-08-25 11:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jan-Bernd Themann, netdev, Christoph Raisch
In-Reply-To: <4A9379C9.6050108@gmail.com>
On Tuesday 25 August 2009 08:42:33 Eric Dumazet wrote:
> Octavian Purdila a écrit :
> > Hi,
> >
> > We are seeing a performance issue with TSO/LRO which we tracked down to
> > the TSecr !=0 check in lro_tcp_ip_check.
>
> ouch...
>
> > It happens when the LRO side's TSval wraps around and gets to 0. That
> > triggers the TSO side to send packets with TSecr set to 0, which means
> > that such packets won't be aggregated - and that will put a lot of burden
> > on the stack which will result in lots of drops.
>
> Probability of such event is 1 / 2^32 or so ?
>
Yes, its pretty low, but the timestamps are taken from jiffies and jiffies are
initialized to -300*HZ so it will happen in 5 minutes after every reboot :)
> > I'm failing to understand the purpose of this check. Any hints? :)
>
> rfc1323 badly interpreted ?
>
> I remember tsecr=0 was forbidden by Linux, while apparently rfc is not
> so clear.
>
> rfc1323 : 3.2
> The Timestamp Echo Reply field (TSecr) is only valid if the ACK
> bit is set in the TCP header; if it is valid, it echos a times-
> tamp value that was sent by the remote TCP in the TSval field
> of a Timestamps option. When TSecr is not valid, its value
> must be zero. The TSecr value will generally be from the most
> recent Timestamp option that was received; however, there are
> exceptions that are explained below.
>
> Note how this is not saying "a zero Tsecr value is not valid"
That is my understanding as well.
> I could not find why : "When TSecr is not valid, its value
> must be zero", and why we consider a zero value to be not meaningfull...
>
> ...
>
> So we dont have a bit saying we received a tsecr, we use the
> 'if saw_tstamp AND tsecr is not null' convention...
Alright, its starting to make sense. So, it looks like we can remove the check
from inet_lro, and that may even reduce the probability of receiving a zero
TSecr in the stack. Right?
Thanks for you help!
tavi
^ permalink raw reply
* Re: [RFC] defer skb allocation in virtio_net -- mergable buff part
From: Michael S. Tsirkin @ 2009-08-25 11:41 UTC (permalink / raw)
To: Shirley Ma; +Cc: netdev, kvm, linux-kernel
In-Reply-To: <1250145231.6653.29.camel@localhost.localdomain>
Wanted to try this awhile now, good to see this worked on!
Some comments inline.
On Wed, Aug 12, 2009 at 11:33:51PM -0700, Shirley Ma wrote:
> Guest virtio_net receives packets from its pre-allocated vring
> buffers, then it delivers these packets to upper layer protocols
> as skb buffs. So it's not necessary to pre-allocate skb for each
> mergable buffer, then frees it when it's useless.
>
> This patch has deferred skb allocation to when receiving packets,
> it reduces skb pre-allocations and skb_frees. And it induces two
> page list: freed_pages and used_page list, used_pages is used to
> track pages pre-allocated, it is only useful when removing virtio_net.
>
> This patch has tested and measured against 2.6.31-rc4 git,
> I thought this patch will improve large packet performance, but I saw
> netperf TCP_STREAM performance improved for small packet for both
> local guest to host and host to local guest cases. It also reduces
> UDP packets drop rate from host to local guest. I am not fully understand
> why.
>
> The netperf results from my laptop are:
>
> mtu=1500
> netperf -H xxx -l 120
>
> w/o patch w/i patch (two runs)
> guest to host: 3336.84Mb/s 3730.14Mb/s ~ 3582.88Mb/s
>
> host to guest: 3165.10Mb/s 3370.39Mb/s ~ 3407.96Mb/s
>
> Here is the patch for your review. The same approach can apply to non-mergable
> buffs too, so we can use code in common.
Yes, we should do that. The way to test is to hack virtio to ignore
host support for mergeable buffers.
> If there is no objection, I will
> submit the non-mergable buffs patch later.
>
The whole page cache management in virtio net is too complex.
Since it seems you bypass it for some cases, this might be where
savings come from?
> Signed-off-by: Shirley Ma <xma@us.ibm.com>
> ---
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 2a6e81d..e31ebc9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -17,6 +17,7 @@
> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> */
> //#define DEBUG
> +#include <linux/list.h>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> #include <linux/ethtool.h>
> @@ -39,6 +40,12 @@ module_param(gso, bool, 0444);
>
> #define VIRTNET_SEND_COMMAND_SG_MAX 2
>
> +struct page_list
> +{
Kernel style is "struct page_list {".
Also, prefix with virtnet_?
> + struct page *page;
> + struct list_head list;
> +};
> +
> struct virtnet_info
> {
> struct virtio_device *vdev;
> @@ -72,6 +79,8 @@ struct virtnet_info
>
> /* Chain pages by the private ptr. */
> struct page *pages;
Do we need the pages list now? Can we do without?
Pls document fields below.
> + struct list_head used_pages;
Seems a waste to have this list just for dev down.
Extend virtio to give us all buffers from vq
on shutdown?
> + struct list_head freed_pages;
> };
>
> static inline void *skb_vnet_hdr(struct sk_buff *skb)
> @@ -106,6 +115,26 @@ static struct page *get_a_page(struct virtnet_info *vi, gfp_t gfp_mask)
> return p;
> }
>
> +static struct page_list *get_a_free_page(struct virtnet_info *vi, gfp_t gfp_mask)
always called with gfp_mask GFP_ATOMIC?
Also - line too long here and elsewere. Run checkpatch?
> +{
> + struct page_list *plist;
> +
> + if (list_empty(&vi->freed_pages)) {
special handling for empty list looks a bit ugly
really necessary?
> + plist = kmalloc(sizeof(struct page_list), gfp_mask);
sizeof *plist
> + if (!plist)
> + return NULL;
> + list_add_tail(&plist->list, &vi->freed_pages);
> + plist->page = alloc_page(gfp_mask);
> + } else {
> + plist = list_first_entry(&vi->freed_pages, struct page_list, list);
is the first entry special? pls document in struct definition
> + if (!plist->page)
> + plist->page = alloc_page(gfp_mask);
what does it mean plist->page == NULL? Please document this
in struct definition.
> + }
> + if (plist->page)
> + list_move_tail(&plist->list, &vi->used_pages);
> + return plist;
> +}
> +
> static void skb_xmit_done(struct virtqueue *svq)
> {
> struct virtnet_info *vi = svq->vdev->priv;
> @@ -121,14 +150,14 @@ static void skb_xmit_done(struct virtqueue *svq)
> tasklet_schedule(&vi->tasklet);
> }
>
> -static void receive_skb(struct net_device *dev, struct sk_buff *skb,
> +static void receive_skb(struct net_device *dev, void *buf,
what is buf, here? can it have proper type and not void*?
> unsigned len)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> - struct virtio_net_hdr *hdr = skb_vnet_hdr(skb);
> int err;
> int i;
> -
> + struct sk_buff *skb = NULL;
> + struct virtio_net_hdr *hdr = NULL;
do we have to init these to NULL?
> if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
> pr_debug("%s: short packet %i\n", dev->name, len);
> dev->stats.rx_length_errors++;
> @@ -136,15 +165,30 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
> }
>
> if (vi->mergeable_rx_bufs) {
> - struct virtio_net_hdr_mrg_rxbuf *mhdr = skb_vnet_hdr(skb);
> + struct virtio_net_hdr_mrg_rxbuf *mhdr;
> unsigned int copy;
> - char *p = page_address(skb_shinfo(skb)->frags[0].page);
> + skb_frag_t *f;
> + struct page_list *plist = (struct page_list *)buf;
cast not necessary
> + char *p = page_address(plist->page);
> +
> + skb = netdev_alloc_skb(vi->dev, GOOD_COPY_LEN + NET_IP_ALIGN);
> + if (unlikely(!skb)) {
> + /* drop the packet */
obvious, but okay ...
> + dev->stats.rx_dropped++;
but what does the below do? Maybe add a comment ...
> + list_move_tail(&plist->list, &vi->freed_pages);
> + return;
> + }
> +
> + skb_reserve(skb, NET_IP_ALIGN);
>
> if (len > PAGE_SIZE)
> len = PAGE_SIZE;
> len -= sizeof(struct virtio_net_hdr_mrg_rxbuf);
>
> - memcpy(hdr, p, sizeof(*mhdr));
> + mhdr = skb_vnet_hdr(skb);
> + memcpy(mhdr, p, sizeof(*mhdr));
> + hdr = (struct virtio_net_hdr *)mhdr;
> +
> p += sizeof(*mhdr);
>
> copy = len;
> @@ -155,20 +199,20 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
>
> len -= copy;
>
> - if (!len) {
> - give_a_page(vi, skb_shinfo(skb)->frags[0].page);
> - skb_shinfo(skb)->nr_frags--;
> - } else {
> - skb_shinfo(skb)->frags[0].page_offset +=
> + if (len) {
> + f = &skb_shinfo(skb)->frags[0];
> + f->page = plist->page;
> + skb_shinfo(skb)->frags[0].page_offset =
> sizeof(*mhdr) + copy;
> skb_shinfo(skb)->frags[0].size = len;
> skb->data_len += len;
> skb->len += len;
> + skb_shinfo(skb)->nr_frags++;
> + plist->page = NULL;
> }
> + list_move_tail(&plist->list, &vi->freed_pages);
>
> while (--mhdr->num_buffers) {
> - struct sk_buff *nskb;
> -
> i = skb_shinfo(skb)->nr_frags;
> if (i >= MAX_SKB_FRAGS) {
> pr_debug("%s: packet too long %d\n", dev->name,
> @@ -177,30 +221,30 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
> goto drop;
> }
>
> - nskb = vi->rvq->vq_ops->get_buf(vi->rvq, &len);
> - if (!nskb) {
> + plist = vi->rvq->vq_ops->get_buf(vi->rvq, &len);
> + if (!plist) {
> pr_debug("%s: rx error: %d buffers missing\n",
> dev->name, mhdr->num_buffers);
> dev->stats.rx_length_errors++;
> goto drop;
> }
> -
> - __skb_unlink(nskb, &vi->recv);
> vi->num--;
> -
> - skb_shinfo(skb)->frags[i] = skb_shinfo(nskb)->frags[0];
> - skb_shinfo(nskb)->nr_frags = 0;
> - kfree_skb(nskb);
> -
> + f = &skb_shinfo(skb)->frags[i];
> + f->page = plist->page;
> + f->page_offset = 0;
I though skb is pre-zeroed anyway. No?
> +
> if (len > PAGE_SIZE)
> len = PAGE_SIZE;
> -
please don't change empty lines arbitrarily
> skb_shinfo(skb)->frags[i].size = len;
> skb_shinfo(skb)->nr_frags++;
> skb->data_len += len;
> skb->len += len;
> + plist->page = NULL;
> + list_move_tail(&plist->list, &vi->freed_pages);
did we set all fields here? how about truesize?
Does skb_add_rx_frag do what we want by any chance?
> }
> } else {
> + skb = (struct sk_buff *)buf;
don't cast away void
> + hdr = skb_vnet_hdr(skb);
> len -= sizeof(struct virtio_net_hdr);
>
> if (len <= MAX_PACKET_LEN)
> @@ -329,7 +373,6 @@ static void try_fill_recv_maxbufs(struct virtnet_info *vi)
>
> static void try_fill_recv(struct virtnet_info *vi)
> {
> - struct sk_buff *skb;
> struct scatterlist sg[1];
> int err;
>
> @@ -339,39 +382,21 @@ static void try_fill_recv(struct virtnet_info *vi)
> }
>
> for (;;) {
> - skb_frag_t *f;
> -
> - skb = netdev_alloc_skb(vi->dev, GOOD_COPY_LEN + NET_IP_ALIGN);
> - if (unlikely(!skb))
> - break;
> -
> - skb_reserve(skb, NET_IP_ALIGN);
> -
> - f = &skb_shinfo(skb)->frags[0];
> - f->page = get_a_page(vi, GFP_ATOMIC);
> - if (!f->page) {
> - kfree_skb(skb);
> + struct page_list *plist = get_a_free_page(vi, GFP_ATOMIC);
> + if (!plist || !plist->page)
> break;
> - }
> -
> - f->page_offset = 0;
> - f->size = PAGE_SIZE;
> -
> - skb_shinfo(skb)->nr_frags++;
> -
> - sg_init_one(sg, page_address(f->page), PAGE_SIZE);
> - skb_queue_head(&vi->recv, skb);
> -
> - err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, 1, skb);
> + sg_init_one(sg, page_address(plist->page), PAGE_SIZE);
> + err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, 1, plist);
> if (err) {
> - skb_unlink(skb, &vi->recv);
> - kfree_skb(skb);
> + list_move_tail(&plist->list, &vi->freed_pages);
> break;
> }
> vi->num++;
> }
> +
> if (unlikely(vi->num > vi->max))
> vi->max = vi->num;
> +
unnecessary empty lines - if you like them, make a
separate patch. and seems to have trailing whitespace.
> vi->rvq->vq_ops->kick(vi->rvq);
> }
>
> @@ -388,14 +413,15 @@ static void skb_recv_done(struct virtqueue *rvq)
> static int virtnet_poll(struct napi_struct *napi, int budget)
> {
> struct virtnet_info *vi = container_of(napi, struct virtnet_info, napi);
> - struct sk_buff *skb = NULL;
> + void *buf = NULL;
Does it need to be initialized?
> unsigned int len, received = 0;
>
> again:
> while (received < budget &&
> - (skb = vi->rvq->vq_ops->get_buf(vi->rvq, &len)) != NULL) {
> - __skb_unlink(skb, &vi->recv);
> - receive_skb(vi->dev, skb, len);
> + (buf = vi->rvq->vq_ops->get_buf(vi->rvq, &len)) != NULL) {
> + if (!vi->mergeable_rx_bufs)
> + __skb_unlink((struct sk_buff *)buf, &vi->recv);
cast not necessary
> + receive_skb(vi->dev, buf, len);
> vi->num--;
> received++;
> }
> @@ -893,6 +919,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> vi->vdev = vdev;
> vdev->priv = vi;
> vi->pages = NULL;
> + INIT_LIST_HEAD(&vi->used_pages);
> + INIT_LIST_HEAD(&vi->freed_pages);
>
> /* If they give us a callback when all buffers are done, we don't need
> * the timer. */
> @@ -969,6 +997,7 @@ static void virtnet_remove(struct virtio_device *vdev)
> {
> struct virtnet_info *vi = vdev->priv;
> struct sk_buff *skb;
> + struct page_list *plist, *tp;
>
> /* Stop all the virtqueues. */
> vdev->config->reset(vdev);
> @@ -977,14 +1006,23 @@ static void virtnet_remove(struct virtio_device *vdev)
> del_timer_sync(&vi->xmit_free_timer);
>
> /* Free our skbs in send and recv queues, if any. */
> - while ((skb = __skb_dequeue(&vi->recv)) != NULL) {
> - kfree_skb(skb);
> - vi->num--;
> + if (!vi->mergeable_rx_bufs) {
> + while ((skb = __skb_dequeue(&vi->recv)) != NULL) {
> + kfree_skb(skb);
> + vi->num--;
> + }
> + BUG_ON(vi->num != 0);
> + } else {
> + list_splice_init(&vi->used_pages, &vi->freed_pages);
> + list_for_each_entry_safe(plist, tp, &vi->freed_pages, list) {
> + vi->num--;
> + if (plist->page)
> + __free_pages(plist->page, 0);
> + kfree(plist);
> + }
BUG_ON here as well?
> }
So, you are fixing this to share code. Good.
> __skb_queue_purge(&vi->send);
>
> - BUG_ON(vi->num != 0);
> -
> unregister_netdev(vi->dev);
>
> vdev->config->del_vqs(vi->vdev);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" 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
* Re: [PATCH] TI DaVinci EMAC: delay DaVinci EMAC initialization
From: Kevin Hilman @ 2009-08-25 11:21 UTC (permalink / raw)
To: Sudhakar Rajashekhara; +Cc: netdev, davinci-linux-open-source, davem
In-Reply-To: <1250714395-9588-1-git-send-email-sudhakar.raj@ti.com>
Sudhakar Rajashekhara <sudhakar.raj@ti.com> writes:
> On TI's DA850/OMAP-L138 EVM, MAC address is stored in SPI
> flash which is accessed using MTD interface.
>
> This patch delays the initialization of DaVinci EMAC driver
> by changing module_init to late_initcall. This helps SPI and
> MTD drivers to get initialized before EMAC thereby enabling
> EMAC driver to read the MAC address while booting and use it.
>
> Tested with NFS on DM644x, DM6467, DA830/OMAP-L137 and
> DA850/OMAP-L138 EVMs.
>
> Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Reviewed-by: Chaithrika U S <chaithrika@ti.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> drivers/net/davinci_emac.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
> index 12fd446..5e6652b 100644
> --- a/drivers/net/davinci_emac.c
> +++ b/drivers/net/davinci_emac.c
> @@ -2817,7 +2817,7 @@ static int __init davinci_emac_init(void)
> {
> return platform_driver_register(&davinci_emac_driver);
> }
> -module_init(davinci_emac_init);
> +late_initcall(davinci_emac_init);
>
> /**
> * davinci_emac_exit: EMAC driver module exit
> --
> 1.5.6
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
^ permalink raw reply
* Re: iproute2 / tbf with large burst seems broken again
From: Denys Fedoryschenko @ 2009-08-25 11:16 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <20090825094120.GA11478@ff.dom.local>
Done, tested, it doesn't stale and fix the issue but:
PPPoE_146 ~ # ./tc qdisc del dev ppp13 root;./tc qdisc add dev ppp13 root tbf
rate 96000 burst 2048000 latency 500ms
This one is ok
PPPoE_146 ~ # ./tc -s -d qdisc show dev ppp13
qdisc tbf 8005: root rate 96000bit burst 2000Kb/8 mpu 0b lat 500.0ms
Sent 55260 bytes 65 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
qdisc ingress ffff: parent ffff:fff1 ----------------
Sent 641055 bytes 2485 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
PPPoE_146 ~ # ./tc qdisc del dev ppp13 root;./tc qdisc add dev ppp13 root tbf
rate 96000 burst 4096000 latency 500ms
But this one maybe will overflow because of limitations in iproute2.
PPoE_146 ~ # ./tc -s -d qdisc show dev ppp13
qdisc tbf 8004: root rate 96000bit burst 797465b/8 mpu 0b lat 275.4s
Sent 82867 bytes 123 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
qdisc ingress ffff: parent ffff:fff1 ----------------
Sent 506821 bytes 1916 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
So maybe all of that just wrong way of using TBF.
At same time this means, if HTB and policers in filters done same way, that
QoS in Linux cannot do similar to squid delay pools feature:
First 10Mb give with 1Mbit/s, then slow 64Kbit/s. If user use less than 64K -
recharge with that unused bandwidth a "10 Mb / 1Mbit bucket".
On Tuesday 25 August 2009 12:41:20 Jarek Poplawski wrote:
> On Tue, Aug 25, 2009 at 09:00:35AM +0000, Jarek Poplawski wrote:
> > On Tue, Aug 25, 2009 at 08:43:06AM +0000, Jarek Poplawski wrote:
> > ...
> >
> > > since these 64 bits will be needed soon for higher rates anyway, I
> > > guess we could try some change like the patch below, if you find it
> > > works for you (I didn't test it yet.)
>
> I hope this time it works...
>
> Jarek P.
>
> --- (take 2)
>
> net/sched/sch_tbf.c | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> index e22dfe8..7d0fe69 100644
> --- a/net/sched/sch_tbf.c
> +++ b/net/sched/sch_tbf.c
> @@ -108,8 +108,8 @@ struct tbf_sched_data
> struct qdisc_rate_table *P_tab;
>
> /* Variables */
> - long tokens; /* Current number of B tokens */
> - long ptokens; /* Current number of P tokens */
> + u32 tokens; /* Current number of B tokens */
> + u32 ptokens; /* Current number of P tokens */
> psched_time_t t_c; /* Time check-point */
> struct Qdisc *qdisc; /* Inner qdisc, default - bfifo queue */
> struct qdisc_watchdog watchdog; /* Watchdog timer */
> @@ -160,21 +160,21 @@ static struct sk_buff *tbf_dequeue(struct Qdisc* sch)
>
> if (skb) {
> psched_time_t now;
> - long toks;
> - long ptoks = 0;
> + long long toks;
> + long long ptoks = 0;
> unsigned int len = qdisc_pkt_len(skb);
>
> now = psched_get_time();
> - toks = psched_tdiff_bounded(now, q->t_c, q->buffer);
> + toks = min_t(u32, now - q->t_c, q->buffer);
>
> if (q->P_tab) {
> ptoks = toks + q->ptokens;
> - if (ptoks > (long)q->mtu)
> + if (ptoks > q->mtu)
> ptoks = q->mtu;
> ptoks -= L2T_P(q, len);
> }
> toks += q->tokens;
> - if (toks > (long)q->buffer)
> + if (toks > q->buffer)
> toks = q->buffer;
> toks -= L2T(q, len);
^ permalink raw reply
* Re: [PATCH v2] sctp: fix the check for path failure detection
From: Luo Chunbo @ 2009-08-25 8:22 UTC (permalink / raw)
To: Wei Yongjun; +Cc: davem, netdev, linux-sctp, linux-kernel
In-Reply-To: <4A93949B.2020404@cn.fujitsu.com>
On Tue, 2009-08-25 at 15:36 +0800, Wei Yongjun wrote:
> Chunbo Luo wrote:
> > The transport error count should be incremented when an outstanding
> > HB is not acknowledged. And the path failure detection should be done
> > before sending out the HB.
> >
> > Signed-off-by: Chunbo Luo <chunbo.luo@windriver.com>
> > ---
> > net/sctp/sm_sideeffect.c | 6 +++++-
> > net/sctp/sm_statefuns.c | 8 ++++----
> > 2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> > index 86426aa..fbdf4de 100644
> > --- a/net/sctp/sm_sideeffect.c
> > +++ b/net/sctp/sm_sideeffect.c
> > @@ -446,7 +446,11 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
> > if (transport->state != SCTP_UNCONFIRMED)
> > asoc->overall_error_count++;
> >
> > - if (transport->state != SCTP_INACTIVE &&
> > + /*
> > + * The transport error count is incremented when an outstanding HB
> > + * is not acknowledged.
> > + */
> >
>
> T3-rtx timer expires also need to increment error count.
Yes, T3-rtx timer expires need to increment error count.
>
> > + if (transport->hb_sent && transport->state != SCTP_INACTIVE &&
> > (transport->error_count++ >= transport->pathmaxrxt)) {
> > SCTP_DEBUG_PRINTK_IPADDR("transport_strike:association %p",
> > " transport IP: port:%d failed.\n",
> > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> > index 7288192..7f77099 100644
> > --- a/net/sctp/sm_statefuns.c
> > +++ b/net/sctp/sm_statefuns.c
> > @@ -981,10 +981,6 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const struct sctp_endpoint *ep,
> > */
> >
> > if (transport->param_flags & SPP_HB_ENABLE) {
> > - if (SCTP_DISPOSITION_NOMEM ==
> > - sctp_sf_heartbeat(ep, asoc, type, arg,
> > - commands))
> > - return SCTP_DISPOSITION_NOMEM;
> > /* Set transport error counter and association error counter
> > * when sending heartbeat.
> > */
> > @@ -992,6 +988,10 @@ sctp_disposition_t sctp_sf_sendbeat_8_3(const struct sctp_endpoint *ep,
> > SCTP_TRANSPORT(transport));
> > sctp_add_cmd_sf(commands, SCTP_CMD_TRANSPORT_HB_SENT,
> > SCTP_TRANSPORT(transport));
> > + if (SCTP_DISPOSITION_NOMEM ==
> > + sctp_sf_heartbeat(ep, asoc, type, arg,
> > + commands))
> > + return SCTP_DISPOSITION_NOMEM;
> > }
> > sctp_add_cmd_sf(commands, SCTP_CMD_HB_TIMER_UPDATE,
> > SCTP_TRANSPORT(transport));
> >
>
> How about this one:
It is ok to increment the error count. But the path failure detection
should be done before send HB. Otherwise, an extra HB chunk will be sent
out before the transport is marked DOWN . So the changes in
net/sctp/sm_statefuns.c is also needed.
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 86426aa..fb723dd 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -447,7 +447,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
> asoc->overall_error_count++;
>
> if (transport->state != SCTP_INACTIVE &&
> - (transport->error_count++ >= transport->pathmaxrxt)) {
> + (transport->error_count >= transport->pathmaxrxt)) {
> SCTP_DEBUG_PRINTK_IPADDR("transport_strike:association %p",
> " transport IP: port:%d failed.\n",
> asoc,
> @@ -468,6 +468,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
> * that indicates that we have an outstanding HB.
> */
> if (!is_hb || transport->hb_sent) {
> + transport->error_count++;
> transport->last_rto = transport->rto;
> transport->rto = min((transport->rto * 2), transport->asoc->rto_max);
> }
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/2] trace_events: fix napi's tracepoint
From: Neil Horman @ 2009-08-25 10:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: Xiao Guangrong, Steven Rostedt, Frederic Weisbecker, Wei Yongjun,
David Miller, Netdev, LKML
In-Reply-To: <20090825075816.GB857@elte.hu>
On Tue, Aug 25, 2009 at 09:58:16AM +0200, Ingo Molnar wrote:
>
> * Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> wrote:
>
> > Currently, the napi's tracepoint works will is depend on
> > "DECLARE_TRACE" definiens in include/trace/define_trace.h,
> > like below:
> >
> > #include <trace/events/skb.h> // include define_trace.h
> > #include <trace/events/napi.h>
> >
> > there have error, if we remove "#include <trace/events/skb.h>"
> > or include napi.h in the front of include skb.h, It should
> > depend on the definiens in include/linux/tracepoint.h and we
> > can remove the "DECLARE_TRACE" definiens in
> > include/trace/define_trace.h, because "TRACE_EVENT" not use it
> >
> > Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> > ---
> > include/trace/define_trace.h | 4 ----
> > net/core/net-traces.c | 4 +++-
> > 2 files changed, 3 insertions(+), 5 deletions(-)
>
> This will collide with tracing bits in the networking tree. The
> skb-tracing plugin there should be turned into proper TRACE_EVENT()
> tracepoints.
>
> Neil was away for some time but i think soon we should see some
> movement here.
>
> Ingo
>
Thank you Ingo, yes, I'm back from the beach now and will look at this shortly.
I concur, we should just convert the napi_poll tracepoint to be defined via the
TRACE_EVENT macro, same as the kfree_skb tracepoint. Xaio would you like to
take care of that, or shall I?
Best
Neil
^ 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