* [net-next PATCH 0/4] qlge: Add ethtool self-test.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
Changes for QLGE.
1) Add ethtool selftest.
2) Reduce debug print output.
3) Generic cleanup.
^ permalink raw reply
* [net-next PATCH 1/4] qlge: Add ethtool self-test.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge.h | 6 ++
drivers/net/qlge/qlge_ethtool.c | 112 +++++++++++++++++++++++++++++++++++++++
drivers/net/qlge/qlge_main.c | 20 +++++++-
3 files changed, 137 insertions(+), 1 deletions(-)
diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 4b954e1..b9f65e0 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -1580,6 +1580,8 @@ enum {
QL_ALLMULTI = 6,
QL_PORT_CFG = 7,
QL_CAM_RT_SET = 8,
+ QL_SELFTEST = 9,
+ QL_LB_LINK_UP = 10,
};
/* link_status bit definitions */
@@ -1716,6 +1718,7 @@ struct ql_adapter {
struct completion ide_completion;
struct nic_operations *nic_ops;
u16 device_id;
+ atomic_t lb_count;
};
/*
@@ -1807,6 +1810,9 @@ int ql_mb_set_port_cfg(struct ql_adapter *qdev);
int ql_wait_fifo_empty(struct ql_adapter *qdev);
void ql_gen_reg_dump(struct ql_adapter *qdev,
struct ql_reg_dump *mpi_coredump);
+netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev);
+void ql_check_lb_frame(struct ql_adapter *, struct sk_buff *);
+int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget);
#if 1
#define QL_ALL_DUMP
diff --git a/drivers/net/qlge/qlge_ethtool.c b/drivers/net/qlge/qlge_ethtool.c
index 62c4af0..058fa0a 100644
--- a/drivers/net/qlge/qlge_ethtool.c
+++ b/drivers/net/qlge/qlge_ethtool.c
@@ -36,6 +36,11 @@
#include "qlge.h"
+static const char ql_gstrings_test[][ETH_GSTRING_LEN] = {
+ "Loopback test (offline)"
+};
+#define QLGE_TEST_LEN (sizeof(ql_gstrings_test) / ETH_GSTRING_LEN)
+
static int ql_update_ring_coalescing(struct ql_adapter *qdev)
{
int i, status = 0;
@@ -251,6 +256,8 @@ static void ql_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
static int ql_get_sset_count(struct net_device *dev, int sset)
{
switch (sset) {
+ case ETH_SS_TEST:
+ return QLGE_TEST_LEN;
case ETH_SS_STATS:
return ARRAY_SIZE(ql_stats_str_arr);
default:
@@ -429,6 +436,110 @@ static int ql_phys_id(struct net_device *ndev, u32 data)
return 0;
}
+static int ql_start_loopback(struct ql_adapter *qdev)
+{
+ if (netif_carrier_ok(qdev->ndev)) {
+ set_bit(QL_LB_LINK_UP, &qdev->flags);
+ netif_carrier_off(qdev->ndev);
+ } else
+ clear_bit(QL_LB_LINK_UP, &qdev->flags);
+ qdev->link_config |= CFG_LOOPBACK_PCS;
+ return ql_mb_set_port_cfg(qdev);
+}
+
+static void ql_stop_loopback(struct ql_adapter *qdev)
+{
+ qdev->link_config &= ~CFG_LOOPBACK_PCS;
+ ql_mb_set_port_cfg(qdev);
+ if (test_bit(QL_LB_LINK_UP, &qdev->flags)) {
+ netif_carrier_on(qdev->ndev);
+ clear_bit(QL_LB_LINK_UP, &qdev->flags);
+ }
+}
+
+static void ql_create_lb_frame(struct sk_buff *skb,
+ unsigned int frame_size)
+{
+ memset(skb->data, 0xFF, frame_size);
+ frame_size &= ~1;
+ memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
+ memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
+ memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
+}
+
+void ql_check_lb_frame(struct ql_adapter *qdev,
+ struct sk_buff *skb)
+{
+ unsigned int frame_size = skb->len;
+
+ if ((*(skb->data + 3) == 0xFF) &&
+ (*(skb->data + frame_size / 2 + 10) == 0xBE) &&
+ (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+ atomic_dec(&qdev->lb_count);
+ return;
+ }
+}
+
+static int ql_run_loopback_test(struct ql_adapter *qdev)
+{
+ int i;
+ netdev_tx_t rc;
+ struct sk_buff *skb;
+ unsigned int size = SMALL_BUF_MAP_SIZE;
+
+ for (i = 0; i < 64; i++) {
+ skb = netdev_alloc_skb(qdev->ndev, size);
+ if (!skb)
+ return -ENOMEM;
+
+ skb->queue_mapping = 0;
+ skb_put(skb, size);
+ ql_create_lb_frame(skb, size);
+ rc = ql_lb_send(skb, qdev->ndev);
+ if (rc != NETDEV_TX_OK)
+ return -EPIPE;
+ atomic_inc(&qdev->lb_count);
+ }
+
+ ql_clean_lb_rx_ring(&qdev->rx_ring[0], 128);
+ return atomic_read(&qdev->lb_count) ? -EIO : 0;
+}
+
+static int ql_loopback_test(struct ql_adapter *qdev, u64 *data)
+{
+ *data = ql_start_loopback(qdev);
+ if (*data)
+ goto out;
+ *data = ql_run_loopback_test(qdev);
+out:
+ ql_stop_loopback(qdev);
+ return *data;
+}
+
+static void ql_self_test(struct net_device *ndev,
+ struct ethtool_test *eth_test, u64 *data)
+{
+ struct ql_adapter *qdev = netdev_priv(ndev);
+
+ if (netif_running(ndev)) {
+ set_bit(QL_SELFTEST, &qdev->flags);
+ if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
+ /* Offline tests */
+ if (ql_loopback_test(qdev, &data[0]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+
+ } else {
+ /* Online tests */
+ data[0] = 0;
+ }
+ clear_bit(QL_SELFTEST, &qdev->flags);
+ } else {
+ QPRINTK(qdev, DRV, ERR,
+ "%s: is down, Loopback test will fail.\n", ndev->name);
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ }
+}
+
static int ql_get_regs_len(struct net_device *ndev)
{
return sizeof(struct ql_reg_dump);
@@ -575,6 +686,7 @@ const struct ethtool_ops qlge_ethtool_ops = {
.set_msglevel = ql_set_msglevel,
.get_link = ethtool_op_get_link,
.phys_id = ql_phys_id,
+ .self_test = ql_self_test,
.get_pauseparam = ql_get_pauseparam,
.set_pauseparam = ql_set_pauseparam,
.get_rx_csum = ql_get_rx_csum,
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index dd0ea02..9e4d3dc 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1680,6 +1680,13 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
return;
}
+ /* loopback self test for ethtool */
+ if (test_bit(QL_SELFTEST, &qdev->flags)) {
+ ql_check_lb_frame(qdev, skb);
+ dev_kfree_skb_any(skb);
+ return;
+ }
+
prefetch(skb->data);
skb->dev = ndev;
if (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) {
@@ -2248,6 +2255,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
+
static void ql_free_shadow_space(struct ql_adapter *qdev)
{
if (qdev->rx_ring_shadow_reg_area) {
@@ -4173,7 +4181,6 @@ err_out:
return err;
}
-
static const struct net_device_ops qlge_netdev_ops = {
.ndo_open = qlge_open,
.ndo_stop = qlge_close,
@@ -4242,10 +4249,21 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
}
ql_link_off(qdev);
ql_display_dev_info(ndev);
+ atomic_set(&qdev->lb_count, 0);
cards_found++;
return 0;
}
+netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev)
+{
+ return qlge_send(skb, ndev);
+}
+
+int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
+{
+ return ql_clean_inbound_rx_ring(rx_ring, budget);
+}
+
static void __devexit qlge_remove(struct pci_dev *pdev)
{
struct net_device *ndev = pci_get_drvdata(pdev);
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 2/4] qlge: Change naming on vlan API.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>
Change name on vlan_rx_add, kill, register to match other driver API.
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_main.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 9e4d3dc..01f78a9 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1985,7 +1985,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
return work_done;
}
-static void ql_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
+static void qlge_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
{
struct ql_adapter *qdev = netdev_priv(ndev);
@@ -2001,7 +2001,7 @@ static void ql_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
}
}
-static void ql_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
+static void qlge_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
{
struct ql_adapter *qdev = netdev_priv(ndev);
u32 enable_bit = MAC_ADDR_E;
@@ -2017,7 +2017,7 @@ static void ql_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
}
-static void ql_vlan_rx_kill_vid(struct net_device *ndev, u16 vid)
+static void qlge_vlan_rx_kill_vid(struct net_device *ndev, u16 vid)
{
struct ql_adapter *qdev = netdev_priv(ndev);
u32 enable_bit = 0;
@@ -4191,9 +4191,9 @@ static const struct net_device_ops qlge_netdev_ops = {
.ndo_set_mac_address = qlge_set_mac_address,
.ndo_validate_addr = eth_validate_addr,
.ndo_tx_timeout = qlge_tx_timeout,
- .ndo_vlan_rx_register = ql_vlan_rx_register,
- .ndo_vlan_rx_add_vid = ql_vlan_rx_add_vid,
- .ndo_vlan_rx_kill_vid = ql_vlan_rx_kill_vid,
+ .ndo_vlan_rx_register = qlge_vlan_rx_register,
+ .ndo_vlan_rx_add_vid = qlge_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = qlge_vlan_rx_kill_vid,
};
static int __devinit qlge_probe(struct pci_dev *pdev,
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 3/4] qlge: Reduce debug print output.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge.h | 12 +++++
drivers/net/qlge/qlge_main.c | 101 ++++++++++++++++++++++-------------------
drivers/net/qlge/qlge_mpi.c | 14 +++---
3 files changed, 73 insertions(+), 54 deletions(-)
diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index b9f65e0..502c3af 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -27,6 +27,18 @@
dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
"%s: " fmt, __func__, ##args); \
} while (0)
+#if 0
+#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...) \
+ do { \
+ if (!((qdev)->msg_enable & NETIF_MSG_##nlevel)) \
+ ; \
+ else \
+ dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
+ "%s: " fmt, __func__, ##args); \
+ } while (0)
+#else
+#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
+#endif
#define WQ_ADDR_ALIGN 0x3 /* 4 byte alignment */
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 01f78a9..57bbb90 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -359,7 +359,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
(addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) |
(addr[5]);
- QPRINTK(qdev, IFUP, DEBUG,
+ QPRINTK_DBG(qdev, IFUP, DEBUG,
"Adding %s address %pM"
" at index %d in the CAM.\n",
((type ==
@@ -414,7 +414,8 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
* addressing. It's either MAC_ADDR_E on or off.
* That's bit-27 we're talking about.
*/
- QPRINTK(qdev, IFUP, INFO, "%s VLAN ID %d %s the CAM.\n",
+ QPRINTK_DBG(qdev, IFUP, INFO,
+ "%s VLAN ID %d %s the CAM.\n",
(enable_bit ? "Adding" : "Removing"),
index, (enable_bit ? "to" : "from"));
@@ -522,9 +523,9 @@ static int ql_set_routing_reg(struct ql_adapter *qdev, u32 index, u32 mask,
int status = -EINVAL; /* Return error if no mask match. */
u32 value = 0;
- QPRINTK(qdev, IFUP, DEBUG,
- "%s %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s mask %s the routing reg.\n",
- (enable ? "Adding" : "Removing"),
+ QPRINTK_DBG(qdev, IFUP, DEBUG,
+ "%s %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s mask %s the routing "
+ "reg.\n", (enable ? "Adding" : "Removing"),
((index == RT_IDX_ALL_ERR_SLOT) ? "MAC ERROR/ALL ERROR" : ""),
((index == RT_IDX_IP_CSUM_ERR_SLOT) ? "IP CSUM ERROR" : ""),
((index ==
@@ -959,8 +960,9 @@ static int ql_8012_port_initialize(struct ql_adapter *qdev)
/* Another function has the semaphore, so
* wait for the port init bit to come ready.
*/
- QPRINTK(qdev, LINK, INFO,
- "Another function has the semaphore, so wait for the port init bit to come ready.\n");
+ QPRINTK_DBG(qdev, LINK, INFO,
+ "Another function has the semaphore, so wait for the "
+ "port init bit to come ready.\n");
status = ql_wait_reg_rdy(qdev, STS, qdev->port_init, 0);
if (status) {
QPRINTK(qdev, LINK, CRIT,
@@ -969,7 +971,7 @@ static int ql_8012_port_initialize(struct ql_adapter *qdev)
return status;
}
- QPRINTK(qdev, LINK, INFO, "Got xgmac semaphore!.\n");
+ QPRINTK_DBG(qdev, LINK, INFO, "Got xgmac semaphore!.\n");
/* Set the core reset. */
status = ql_read_xgmac_reg(qdev, GLOBAL_CFG, &data);
if (status)
@@ -1148,7 +1150,7 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
while (rx_ring->lbq_free_cnt > 32) {
for (i = 0; i < 16; i++) {
- QPRINTK(qdev, RX_STATUS, DEBUG,
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
"lbq: try cleaning clean_idx = %d.\n",
clean_idx);
lbq_desc = &rx_ring->lbq[clean_idx];
@@ -1181,7 +1183,7 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
}
if (start_idx != clean_idx) {
- QPRINTK(qdev, RX_STATUS, DEBUG,
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
"lbq: updating prod idx = %d.\n",
rx_ring->lbq_prod_idx);
ql_write_db_reg(rx_ring->lbq_prod_idx,
@@ -1201,7 +1203,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
while (rx_ring->sbq_free_cnt > 16) {
for (i = 0; i < 16; i++) {
sbq_desc = &rx_ring->sbq[clean_idx];
- QPRINTK(qdev, RX_STATUS, DEBUG,
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
"sbq: try cleaning clean_idx = %d.\n",
clean_idx);
if (sbq_desc->p.skb == NULL) {
@@ -1247,7 +1249,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
}
if (start_idx != clean_idx) {
- QPRINTK(qdev, RX_STATUS, DEBUG,
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
"sbq: updating prod idx = %d.\n",
rx_ring->sbq_prod_idx);
ql_write_db_reg(rx_ring->sbq_prod_idx,
@@ -1281,7 +1283,7 @@ static void ql_unmap_send(struct ql_adapter *qdev,
* then its an OAL.
*/
if (i == 7) {
- QPRINTK(qdev, TX_DONE, DEBUG,
+ QPRINTK_DBG(qdev, TX_DONE, DEBUG,
"unmapping OAL area.\n");
}
pci_unmap_single(qdev->pdev,
@@ -1291,8 +1293,8 @@ static void ql_unmap_send(struct ql_adapter *qdev,
maplen),
PCI_DMA_TODEVICE);
} else {
- QPRINTK(qdev, TX_DONE, DEBUG, "unmapping frag %d.\n",
- i);
+ QPRINTK_DBG(qdev, TX_DONE, DEBUG,
+ "unmapping frag %d.\n", i);
pci_unmap_page(qdev->pdev,
pci_unmap_addr(&tx_ring_desc->map[i],
mapaddr),
@@ -1316,9 +1318,9 @@ static int ql_map_send(struct ql_adapter *qdev,
struct tx_buf_desc *tbd = mac_iocb_ptr->tbd;
int frag_cnt = skb_shinfo(skb)->nr_frags;
- if (frag_cnt) {
- QPRINTK(qdev, TX_QUEUED, DEBUG, "frag_cnt = %d.\n", frag_cnt);
- }
+ if (frag_cnt)
+ QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "frag_cnt = %d.\n",
+ frag_cnt);
/*
* Map the skb buffer first.
*/
@@ -1857,7 +1859,7 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
/* While there are entries in the completion queue. */
while (prod != rx_ring->cnsmr_idx) {
- QPRINTK(qdev, RX_STATUS, DEBUG,
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
"cq_id = %d, prod = %d, cnsmr = %d.\n.", rx_ring->cq_id,
prod, rx_ring->cnsmr_idx);
@@ -1871,7 +1873,8 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
break;
default:
QPRINTK(qdev, RX_STATUS, DEBUG,
- "Hit default case, not handled! dropping the packet, opcode = %x.\n",
+ "Hit default case, not handled! "
+ "dropping the packet, opcode = %x.\n",
net_rsp->opcode);
}
count++;
@@ -1904,7 +1907,7 @@ static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
/* While there are entries in the completion queue. */
while (prod != rx_ring->cnsmr_idx) {
- QPRINTK(qdev, RX_STATUS, DEBUG,
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
"cq_id = %d, prod = %d, cnsmr = %d.\n.", rx_ring->cq_id,
prod, rx_ring->cnsmr_idx);
@@ -1922,11 +1925,10 @@ static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
net_rsp);
break;
default:
- {
- QPRINTK(qdev, RX_STATUS, DEBUG,
- "Hit default case, not handled! dropping the packet, opcode = %x.\n",
+ QPRINTK(qdev, RX_STATUS, DEBUG,
+ "Hit default case, not handled! "
+ "dropping the packet, opcode = %x.\n",
net_rsp->opcode);
- }
}
count++;
ql_update_cq(rx_ring);
@@ -1947,7 +1949,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
int i, work_done = 0;
struct intr_context *ctx = &qdev->intr_context[rx_ring->cq_id];
- QPRINTK(qdev, RX_STATUS, DEBUG, "Enter, NAPI POLL cq_id = %d.\n",
+ QPRINTK_DBG(qdev, RX_STATUS, DEBUG, "Enter, NAPI POLL cq_id = %d.\n",
rx_ring->cq_id);
/* Service the TX rings first. They start
@@ -1960,7 +1962,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
if ((ctx->irq_mask & (1 << trx_ring->cq_id)) &&
(ql_read_sh_reg(trx_ring->prod_idx_sh_reg) !=
trx_ring->cnsmr_idx)) {
- QPRINTK(qdev, INTR, DEBUG,
+ QPRINTK_DBG(qdev, INTR, DEBUG,
"%s: Servicing TX completion ring %d.\n",
__func__, trx_ring->cq_id);
ql_clean_outbound_rx_ring(trx_ring);
@@ -1972,7 +1974,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
*/
if (ql_read_sh_reg(rx_ring->prod_idx_sh_reg) !=
rx_ring->cnsmr_idx) {
- QPRINTK(qdev, INTR, DEBUG,
+ QPRINTK_DBG(qdev, INTR, DEBUG,
"%s: Servicing RX completion ring %d.\n",
__func__, rx_ring->cq_id);
work_done = ql_clean_inbound_rx_ring(rx_ring, budget);
@@ -1991,11 +1993,12 @@ static void qlge_vlan_rx_register(struct net_device *ndev, struct vlan_group *gr
qdev->vlgrp = grp;
if (grp) {
- QPRINTK(qdev, IFUP, DEBUG, "Turning on VLAN in NIC_RCV_CFG.\n");
+ QPRINTK_DBG(qdev, IFUP, DEBUG,
+ "Turning on VLAN in NIC_RCV_CFG.\n");
ql_write32(qdev, NIC_RCV_CFG, NIC_RCV_CFG_VLAN_MASK |
NIC_RCV_CFG_VLAN_MATCH_AND_NON);
} else {
- QPRINTK(qdev, IFUP, DEBUG,
+ QPRINTK_DBG(qdev, IFUP, DEBUG,
"Turning off VLAN in NIC_RCV_CFG.\n");
ql_write32(qdev, NIC_RCV_CFG, NIC_RCV_CFG_VLAN_MASK);
}
@@ -2058,7 +2061,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
spin_lock(&qdev->hw_lock);
if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
- QPRINTK(qdev, INTR, DEBUG, "Shared Interrupt, Not ours!\n");
+ QPRINTK_DBG(qdev, INTR, DEBUG, "Shared Interrupt, Not ours!\n");
spin_unlock(&qdev->hw_lock);
return IRQ_NONE;
}
@@ -2102,8 +2105,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
*/
var = ql_read32(qdev, ISR1);
if (var & intr_context->irq_mask) {
- QPRINTK(qdev, INTR, INFO,
- "Waking handler for rx_ring[0].\n");
+ QPRINTK_DBG(qdev, INTR, INFO, "Waking handler for rx_ring[0].\n");
ql_disable_completion_interrupt(qdev, intr_context->intr);
napi_schedule(&rx_ring->napi);
work_done++;
@@ -2200,9 +2202,9 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
if (unlikely(atomic_read(&tx_ring->tx_count) < 2)) {
- QPRINTK(qdev, TX_QUEUED, INFO,
- "%s: shutting down tx queue %d du to lack of resources.\n",
- __func__, tx_ring_idx);
+ QPRINTK_DBG(qdev, TX_QUEUED, INFO,
+ "shutting down tx queue %d du to lack of resources.\n",
+ tx_ring_idx);
netif_stop_subqueue(ndev, tx_ring->wq_id);
atomic_inc(&tx_ring->queue_stopped);
return NETDEV_TX_BUSY;
@@ -2222,7 +2224,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
mac_iocb_ptr->frame_len = cpu_to_le16((u16) skb->len);
if (qdev->vlgrp && vlan_tx_tag_present(skb)) {
- QPRINTK(qdev, TX_QUEUED, DEBUG, "Adding a vlan tag %d.\n",
+ QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "Adding a vlan tag %d.\n",
vlan_tx_tag_get(skb));
mac_iocb_ptr->flags3 |= OB_MAC_IOCB_V;
mac_iocb_ptr->vlan_tci = cpu_to_le16(vlan_tx_tag_get(skb));
@@ -2248,7 +2250,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
wmb();
ql_write_db_reg(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
- QPRINTK(qdev, TX_QUEUED, DEBUG, "tx queued, slot %d, len %d\n",
+ QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "tx queued, slot %d, len %d\n",
tx_ring->prod_idx, skb->len);
atomic_dec(&tx_ring->tx_count);
@@ -2783,10 +2785,10 @@ static int ql_start_rx_ring(struct ql_adapter *qdev, struct rx_ring *rx_ring)
cqicb->pkt_delay = cpu_to_le16(qdev->rx_max_coalesced_frames);
break;
default:
- QPRINTK(qdev, IFUP, DEBUG, "Invalid rx_ring->type = %d.\n",
+ QPRINTK_DBG(qdev, IFUP, DEBUG, "Invalid rx_ring->type = %d.\n",
rx_ring->type);
}
- QPRINTK(qdev, IFUP, DEBUG, "Initializing rx work queue.\n");
+ QPRINTK_DBG(qdev, IFUP, DEBUG, "Initializing rx work queue.\n");
err = ql_write_cfg(qdev, cqicb, sizeof(struct cqicb),
CFG_LCQ, rx_ring->cq_id);
if (err) {
@@ -2839,7 +2841,7 @@ static int ql_start_tx_ring(struct ql_adapter *qdev, struct tx_ring *tx_ring)
QPRINTK(qdev, IFUP, ERR, "Failed to load tx_ring.\n");
return err;
}
- QPRINTK(qdev, IFUP, DEBUG, "Successfully loaded WQICB.\n");
+ QPRINTK_DBG(qdev, IFUP, DEBUG, "Successfully loaded WQICB.\n");
return err;
}
@@ -3088,11 +3090,11 @@ static void ql_free_irq(struct ql_adapter *qdev)
if (test_bit(QL_MSIX_ENABLED, &qdev->flags)) {
free_irq(qdev->msi_x_entry[i].vector,
&qdev->rx_ring[i]);
- QPRINTK(qdev, IFDOWN, DEBUG,
+ QPRINTK_DBG(qdev, IFDOWN, DEBUG,
"freeing msix interrupt %d.\n", i);
} else {
free_irq(qdev->pdev->irq, &qdev->rx_ring[0]);
- QPRINTK(qdev, IFDOWN, DEBUG,
+ QPRINTK_DBG(qdev, IFDOWN, DEBUG,
"freeing msi interrupt %d.\n", i);
}
}
@@ -3200,14 +3202,14 @@ static int ql_start_rss(struct ql_adapter *qdev)
memcpy((void *)&ricb->ipv6_hash_key[0], init_hash_seed, 40);
memcpy((void *)&ricb->ipv4_hash_key[0], init_hash_seed, 16);
- QPRINTK(qdev, IFUP, DEBUG, "Initializing RSS.\n");
+ QPRINTK_DBG(qdev, IFUP, DEBUG, "Initializing RSS.\n");
status = ql_write_cfg(qdev, ricb, sizeof(*ricb), CFG_LR, 0);
if (status) {
QPRINTK(qdev, IFUP, ERR, "Failed to load RICB.\n");
return status;
}
- QPRINTK(qdev, IFUP, DEBUG, "Successfully loaded RICB.\n");
+ QPRINTK_DBG(qdev, IFUP, DEBUG, "Successfully loaded RICB.\n");
return status;
}
@@ -3679,7 +3681,7 @@ static int ql_configure_rings(struct ql_adapter *qdev)
rx_ring->lbq_size =
rx_ring->lbq_len * sizeof(__le64);
rx_ring->lbq_buf_size = (u16)lbq_buf_len;
- QPRINTK(qdev, IFUP, DEBUG,
+ QPRINTK_DBG(qdev, IFUP, DEBUG,
"lbq_buf_size %d, order = %d\n",
rx_ring->lbq_buf_size, qdev->lbq_buf_order);
rx_ring->sbq_len = NUM_SMALL_BUFFERS;
@@ -3917,8 +3919,13 @@ static int qlge_set_mac_address(struct net_device *ndev, void *p)
if (netif_running(ndev))
return -EBUSY;
- if (!is_valid_ether_addr(addr->sa_data))
+ if (!is_valid_ether_addr(addr->sa_data)) {
+ QPRINTK_DBG(qdev, DRV, ERR,
+ "Invalid Mac addr %02x:%02x:%02x:%02x:%02x:%02x\n",
+ addr->sa_data[0], addr->sa_data[1], addr->sa_data[2],
+ addr->sa_data[3], addr->sa_data[4], addr->sa_data[5]);
return -EADDRNOTAVAIL;
+ }
memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
status = ql_sem_spinlock(qdev, SEM_MAC_ADDR_MASK);
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 80b6853..0a26055 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -207,7 +207,7 @@ static void ql_link_up(struct ql_adapter *qdev, struct mbox_params *mbcp)
* to our liking.
*/
if (!test_bit(QL_PORT_CFG, &qdev->flags)) {
- QPRINTK(qdev, DRV, ERR, "Queue Port Config Worker!\n");
+ QPRINTK_DBG(qdev, DRV, DEBUG, "Queue Port Config Worker!\n");
set_bit(QL_PORT_CFG, &qdev->flags);
/* Begin polled mode early so
* we don't get another interrupt
@@ -277,7 +277,7 @@ static int ql_aen_lost(struct ql_adapter *qdev, struct mbox_params *mbcp)
int i;
QPRINTK(qdev, DRV, ERR, "Lost AEN detected.\n");
for (i = 0; i < mbcp->out_count; i++)
- QPRINTK(qdev, DRV, ERR, "mbox_out[%d] = 0x%.08x.\n",
+ QPRINTK_DBG(qdev, DRV, ERR, "mbox_out[%d] = 0x%.08x.\n",
i, mbcp->mbox_out[i]);
}
@@ -658,7 +658,7 @@ int ql_mb_set_port_cfg(struct ql_adapter *qdev)
return status;
if (mbcp->mbox_out[0] == MB_CMD_STS_INTRMDT) {
- QPRINTK(qdev, DRV, ERR,
+ QPRINTK_DBG(qdev, DRV, ERR,
"Port Config sent, wait for IDC.\n");
} else if (mbcp->mbox_out[0] != MB_CMD_STS_GOOD) {
QPRINTK(qdev, DRV, ERR,
@@ -694,7 +694,7 @@ int ql_mb_get_port_cfg(struct ql_adapter *qdev)
"Failed Get Port Configuration.\n");
status = -EIO;
} else {
- QPRINTK(qdev, DRV, DEBUG,
+ QPRINTK_DBG(qdev, DRV, DEBUG,
"Passed Get Port Configuration.\n");
qdev->link_config = mbcp->mbox_out[1];
qdev->max_frame_size = mbcp->mbox_out[2];
@@ -898,7 +898,7 @@ int ql_mb_set_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 control)
return status;
if (mbcp->mbox_out[0] == MB_CMD_STS_INVLD_CMD) {
- QPRINTK(qdev, DRV, ERR,
+ QPRINTK_DBG(qdev, DRV, ERR,
"Command not supported by firmware.\n");
status = -EINVAL;
} else if (mbcp->mbox_out[0] == MB_CMD_STS_ERR) {
@@ -906,7 +906,7 @@ int ql_mb_set_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 control)
* already in the state we are trying to
* change it to.
*/
- QPRINTK(qdev, DRV, ERR,
+ QPRINTK_DBG(qdev, DRV, ERR,
"Command parameters make no change.\n");
}
return status;
@@ -937,7 +937,7 @@ static int ql_mb_get_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 *control)
}
if (mbcp->mbox_out[0] == MB_CMD_STS_INVLD_CMD) {
- QPRINTK(qdev, DRV, ERR,
+ QPRINTK_DBG(qdev, DRV, ERR,
"Command not supported by firmware.\n");
status = -EINVAL;
} else if (mbcp->mbox_out[0] == MB_CMD_STS_ERR) {
--
1.6.0.2
^ permalink raw reply related
* [net-next PATCH 4/4] qlge: Fix indentations.
From: Ron Mercer @ 2009-10-30 22:13 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_main.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 57bbb90..224fb7c 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -2107,9 +2107,9 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
if (var & intr_context->irq_mask) {
QPRINTK_DBG(qdev, INTR, INFO, "Waking handler for rx_ring[0].\n");
ql_disable_completion_interrupt(qdev, intr_context->intr);
- napi_schedule(&rx_ring->napi);
- work_done++;
- }
+ napi_schedule(&rx_ring->napi);
+ work_done++;
+ }
ql_enable_completion_interrupt(qdev, intr_context->intr);
return work_done ? IRQ_HANDLED : IRQ_NONE;
}
--
1.6.0.2
^ permalink raw reply related
* Re: [net-next PATCH 3/4] qlge: Reduce debug print output.
From: Joe Perches @ 2009-10-30 22:44 UTC (permalink / raw)
To: Ron Mercer; +Cc: davem, netdev
In-Reply-To: <1256940816-27540-4-git-send-email-ron.mercer@qlogic.com>
On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
[]
> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
> index b9f65e0..502c3af 100644
> --- a/drivers/net/qlge/qlge.h
> +++ b/drivers/net/qlge/qlge.h
> @@ -27,6 +27,18 @@
> dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
> "%s: " fmt, __func__, ##args); \
> } while (0)
> +#if 0
> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...) \
> + do { \
> + if (!((qdev)->msg_enable & NETIF_MSG_##nlevel)) \
> + ; \
> + else \
> + dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
> + "%s: " fmt, __func__, ##args); \
> + } while (0)
> +#else
> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
> +#endif
This uses an inverted test and it doesn't verify the args to
dev_printk when not #defined.
How about:
#ifdef DEBUG
#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...) do { \
if ((qdev)->msg_enable & NETIF_MSG_##nlevel) \
dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
"%s: " fmt, __func__, ##args); \
} while (0)
#else
#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...) do { \
if (0 && (qdev)->msg_enable & NETIF_MSG_##nlevel) \
dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
"%s: " fmt, __func__, ##args); \
} while (0)
#endif
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-30 22:57 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: David Miller, netdev
In-Reply-To: <21200.1256937150@death.nxdomain.ibm.com>
Jay Vosburgh <fubar@us.ibm.com> writes:
> David Miller <davem@davemloft.net> wrote:
>
>>From: ebiederm@xmission.com (Eric W. Biederman)
>>Date: Thu, 29 Oct 2009 17:16:54 -0700
>>
>>> I recently had it pointed out to me that the bonding driver does not
>>> work in a network namespace. So I have simplified the bonding driver
>>> a bit, added support for ip link add and ip link del, and finally made
>>> the bonding driver work in multiple network namespaces.
>>>
>>> The most note worthy change in the patchset is the addition of support
>>> in the networking core for registering a sysfs group for a device.
>>>
>>> Using this in the bonding driver simplifies the code and removes a
>>> userspace race between actions triggered by the netlink event and the
>>> bonding sysfs attributes appearing.
>>
>>I've tossed patches 1-7 into net-next-2.6, thanks Eric.
>
> I put patches 1-7 on a recent net-next-2.6, and from a simple
> "insmod bonding.ko; rmmod bonding" I'm seeing the following:
>
> ------------[ cut here ]------------
> WARNING: at fs/proc/generic.c:847 remove_proc_entry+0x1a8/0x1c7()
> Hardware name: IBM eserver xSeries 220 -[8645]-
> remove_proc_entry: removing non-empty directory 'net/bonding', leaking at least
> 'bond0'
> Modules linked in: bonding(-) ipv6 microcode loop ppdev sworks_agp parport_pc tg
> 3 e100 agpgart parport mii libphy e1000 edd pata_serverworks [last unloaded: spe
> edstep_lib]
> Pid: 6216, comm: rmmod Not tainted 2.6.32-rc3-devel #19
> Call Trace:
> [<c012ec9d>] warn_slowpath_common+0x60/0x90
> [<c012ed01>] warn_slowpath_fmt+0x24/0x27
> [<c01e3e55>] remove_proc_entry+0x1a8/0x1c7
> [<e0906435>] ? bond_net_exit+0x0/0xa3 [bonding]
> [<e09064c3>] bond_net_exit+0x8e/0xa3 [bonding]
> [<c02e6ee8>] unregister_pernet_gen_subsys+0x23/0x3d
> [<e0910baa>] bonding_exit+0x3a/0x66 [bonding]
> [<c015ccf3>] sys_delete_module+0x191/0x1f1
> [<c0147a30>] ? up_read+0x16/0x2a
> [<c0102a18>] ? restore_all_notrace+0x0/0x18
> [<c0353357>] ? do_page_fault+0x0/0x393
> [<c0102904>] sysenter_do_call+0x12/0x32
> ---[ end trace 8f3eaeee682a572c ]---
>
> Any thoughts? I have not as yet investigated further.
Weird.
We have already run:
rtnl_link_unregister.
rtnl_kill_links
dellink(bond0)
unregister_netdevice(bond0)
bond_uninit
bond_remove_proc_entry
So the proc entry should no longer be there. I'm a little nervous about
the new unregister_netdevice_many but I don't see any obvious problems with
that code.
Were there by any chance any earlier errors that could have prevented the uninit?
You weren't inserting multiple copies of the bonding driver?
Eric
^ permalink raw reply
* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
From: Eric W. Biederman @ 2009-10-30 23:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
In-Reply-To: <4AEA8F0F.4030100@gmail.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> In commit 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b
> (net: add a list_head parameter to dellink() method),
> I forgot to actually use this parameter in veth_dellink.
>
> I remember feeling a bit uncomfortable about veth_close(),
> because it does :
>
> netif_carrier_off(dev);
> netif_carrier_off(priv->peer);
>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index ffb502d..9bed694 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
> priv = netdev_priv(dev);
> peer = priv->peer;
>
> - unregister_netdevice(dev);
> - unregister_netdevice(peer);
> + unregister_netdevice_queue(dev, head);
> + unregister_netdevice_queue(peer, head);
Unless I am mistaken you need to change the list_add_tail to
list_move_tail in unregister_netdevice_queue because we will be
adding each veth device twice.
Eric
^ permalink raw reply
* Re: Connection tracking and vlan
From: Eric W. Biederman @ 2009-10-30 23:15 UTC (permalink / raw)
To: Adayadil Thomas; +Cc: Eric Dumazet, Herbert Xu, netdev, Patrick McHardy
In-Reply-To: <fb7befa20910301220y53e43608u3b37de17661feb4e@mail.gmail.com>
Adayadil Thomas <adayadil.thomas@gmail.com> writes:
> On Fri, Oct 30, 2009 at 11:31 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Very strange, this question about vlan looks like discussion we had
>> yesterday (or the day before...) about interfaces (versus packet defragmentation)
>>
>> "IP conntracking" is about IP, and [V]LAN doesnt matter at all at this protocol level.
>>
>> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
>> include interface name/index
>
> I am concerned about the following situation -
>
> The linux device is configured as a bridge and is deployed between the
> trunk ports
> of 2 switches. In this situation the linux device will be seeing 802.1q packets
> with vlan headers specifying the vlanids.
>
> It is valid to have an environment where private IP addresses are duplicated
> on different virtual LANs. i.e. it is valid to have machine A (
> 10.10.10.1) talking to
> machine B (10.10.10.2) on vlan 1,
> and
> at the same time machine C ( 10.10.10.1) talking to machine D
> (10.10.10.2) on vlan 2.
>
> Since they are on different LANs (VLANs), there should not be any issues.
>
> Now when VLANs are shared across switches, the trunk port will sent
> 802.1q tagged
> packets between the switches. Imagine these packets when going through
> the linux bridge.
> The 802.1q header should identify the separate vlans by the vlan id.
>
> If ip_conntrack does not consider vlans, it is possible that all 5
> tuple are the same
> and thus affect the connection tracking.
>
> I hope I have described the scenario well. If not I can explain in a
> more detailed fashion.
Unless you have multiple network namespaces linux assumes all packets are
in the same ip space. And 10.10.10.1 is the same machine no matter
which interface you talk to it on.
Eric
^ permalink raw reply
* Re: Connection tracking and vlan
From: Ben Greear @ 2009-10-30 23:25 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Adayadil Thomas, Eric Dumazet, Herbert Xu, netdev,
Patrick McHardy
In-Reply-To: <m1639wmq1f.fsf@fess.ebiederm.org>
On 10/30/2009 04:15 PM, Eric W. Biederman wrote:
>> If ip_conntrack does not consider vlans, it is possible that all 5
>> tuple are the same
>> and thus affect the connection tracking.
>>
>> I hope I have described the scenario well. If not I can explain in a
>> more detailed fashion.
>
> Unless you have multiple network namespaces linux assumes all packets are
> in the same ip space. And 10.10.10.1 is the same machine no matter
> which interface you talk to it on.
It only takes a relatively small patch that lets conn-track hash on a
skb->foo_mark, and allow that mark to be set on incoming packets
based on netdevice or whatever, (before the conn-track lookup is
done).
This is logically somewhat similar to using multiple routing
tables and has been working well for me for several years....
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Eric W. Biederman @ 2009-10-30 23:25 UTC (permalink / raw)
To: Benjamin LaHaise; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <20091030143527.GA3141@kvack.org>
Benjamin LaHaise <bcrl@lhnet.ca> writes:
> On Thu, Oct 29, 2009 at 06:45:32PM -0700, Eric W. Biederman wrote:
>> The reason for the existence of sysfs_dirent is as things grow larger
>> we want to keep the amount of RAM consumed down. So we don't pin
>> everything in the dcache. So we try and keep the amount of memory
>> consumed down.
>
> I'm aware of that, but for users running into this sort of scaling issue,
> the amount of RAM required is a non-issue (30,000 interfaces require about
> 1GB of RAM at present), making the question more one of how to avoid the
> overhead for users who don't require it. I'd prefer a config option. The
> only way I can really see saving memory usage is to somehow tie sysfs dirent
> lookups into the network stack's own tables for looking up device entries.
> The network stack already has to cope with this kind of scaling, and that
> would save the RAM.
There is that. I'm trying to figure out how to add the improvements
without making sysfs_dirent larger. Which I think that is doable.
>> So I would like to see how much we can par down.
>
>> For dealing with seeks in the middle of readdir I expect the best way
>> to do that is to be inspired by htrees in extNfs and return a hash of
>> the filename as our position, and keep the filename list sorted by
>> that hash. Since we are optimizing for size we don't need to store
>> that hash. Then we can turn that list into a some flavor of sorted
>> binary tree.
>
> readdir() generally isn't an issue at present.
Supporting seekdir into the middle of a directory is the entire reason
I keep the entries sorted by inode. If we sort by a hash of the name.
We can use the hash to support directory position in readdir and seekdir.
And we can completely remove the linear list when the rb_tree is introduced.
>> I'm surprised sysfs_count_nlink shows up, as it is not directly on the
>> add or remove path. I think the answer there is to change s_flags
>> into a set of bitfields and make link_count one of them, perhaps
>> 16bits long. If we ever overflow our bitfield we can just set link
>> count to 0, and userspace (aka find) will know it can't optimized
>> based on link count.
>
> It shows up because of the bits of userspace (udev) touching the directory
> from things like the hotplug code path.
I realized after sending the message that s_mode in sysfs_dirent is a
real size offense. It is a 16bit field packed in between two longs.
So in practice it is possible to move the s_mode up next to s_flags
and add a s_nlink after it both unsigned short and get a cheap sysfs_nlink.
>> I was expecting someone to run into problems with the linear directory
>> of sysfs someday.
>
> Alas, sysfs isn't the only offender.
Agreed. Sysfs is probably the easiest to untangle.
Since I'm not quite ready to post my patches. I will briefly
mention what I have in my queue and hopefully get things posted.
I have changes to make it so that sysfs never has to go from
the sysfs_dirent to the sysfs inode.
I have changes to sys_sysctl() so that it becomes a filesystem lookup
under /proc/sys. Which ultimately makes the code easier to maintain
and debug.
Now back to getting things forward ported and ready to post.
Eric
^ permalink raw reply
* Re: [RFC, PATCH] net: suspicious test in dev_change_name()
From: Jarek Poplawski @ 2009-10-30 23:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AEAAFC4.9050309@gmail.com>
Eric Dumazet wrote, On 10/30/2009 10:20 AM:
> While preparing a patch for net-next-2.6, I noticed following code in dev_change_name()
>
> int err = 0;
> ...
> ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
> ret = notifier_to_errno(ret);
>
> if (ret) {
> << HERE >> if (err) {
> printk(KERN_ERR
> "%s: name change rollback failed: %d.\n",
> dev->name, ret);
> } else {
> err = ret;
> memcpy(dev->name, oldname, IFNAMSIZ);
> goto rollback;
> }
> }
>
>
> It seems intent was to test if notifier_to_errno() was null ?
I don't think so: err stores the previous ret meaning rollback and
is checked for this later. But somebody forgot err can store previous
(positive) value here, so IMHO you're right: there is a bug in this
place ;-)
Jarek P.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b8f74cf..029cd41 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -939,9 +939,9 @@ rollback:
> write_unlock_bh(&dev_base_lock);
>
> ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
> - ret = notifier_to_errno(ret);
>
> if (ret) {
> + err = notifier_to_errno(ret);
> if (err) {
> printk(KERN_ERR
> "%s: name change rollback failed: %d.\n",
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Benjamin LaHaise @ 2009-10-30 23:53 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <m1my38lb0f.fsf@fess.ebiederm.org>
On Fri, Oct 30, 2009 at 04:25:52PM -0700, Eric W. Biederman wrote:
> I realized after sending the message that s_mode in sysfs_dirent is a
> real size offense. It is a 16bit field packed in between two longs.
> So in practice it is possible to move the s_mode up next to s_flags
> and add a s_nlink after it both unsigned short and get a cheap sysfs_nlink.
That doesn't work -- the number of directory entries can easily exceed 65535.
Current mid range hardware is good enough to terminate 100,000 network
interfaces on a single host.
> Since I'm not quite ready to post my patches. I will briefly
> mention what I have in my queue and hopefully get things posted.
>
> I have changes to make it so that sysfs never has to go from
> the sysfs_dirent to the sysfs inode.
Ah, interesting.
> I have changes to sys_sysctl() so that it becomes a filesystem lookup
> under /proc/sys. Which ultimately makes the code easier to maintain
> and debug.
That sounds like a much saner approach, but has the wrinkle that procfs can
be configured out.
> Now back to getting things forward ported and ready to post.
I'm looking forward to those changes. I've been ignoring procfs for the
time being by disabling the per-interface entries in the network stack,
but there is some desire to be able to enable rp_filter on a per-interface
radius config at runtime. rp_filter has to be disabled across the board
on my access routers, as there are several places where assymetric routing
is used for performance reasons.
-ben
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Jay Vosburgh @ 2009-10-31 0:10 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev
In-Reply-To: <m1639wo5gb.fsf@fess.ebiederm.org>
Eric W. Biederman <ebiederm@xmission.com> wrote:
>Jay Vosburgh <fubar@us.ibm.com> writes:
>
>> David Miller <davem@davemloft.net> wrote:
>>
>>>From: ebiederm@xmission.com (Eric W. Biederman)
>>>Date: Thu, 29 Oct 2009 17:16:54 -0700
>>>
>>>> I recently had it pointed out to me that the bonding driver does not
>>>> work in a network namespace. So I have simplified the bonding driver
>>>> a bit, added support for ip link add and ip link del, and finally made
>>>> the bonding driver work in multiple network namespaces.
>>>>
>>>> The most note worthy change in the patchset is the addition of support
>>>> in the networking core for registering a sysfs group for a device.
>>>>
>>>> Using this in the bonding driver simplifies the code and removes a
>>>> userspace race between actions triggered by the netlink event and the
>>>> bonding sysfs attributes appearing.
>>>
>>>I've tossed patches 1-7 into net-next-2.6, thanks Eric.
>>
>> I put patches 1-7 on a recent net-next-2.6, and from a simple
>> "insmod bonding.ko; rmmod bonding" I'm seeing the following:
>>
>> ------------[ cut here ]------------
>> WARNING: at fs/proc/generic.c:847 remove_proc_entry+0x1a8/0x1c7()
>> Hardware name: IBM eserver xSeries 220 -[8645]-
>> remove_proc_entry: removing non-empty directory 'net/bonding', leaking at least
>> 'bond0'
>> Modules linked in: bonding(-) ipv6 microcode loop ppdev sworks_agp parport_pc tg
>> 3 e100 agpgart parport mii libphy e1000 edd pata_serverworks [last unloaded: spe
>> edstep_lib]
>> Pid: 6216, comm: rmmod Not tainted 2.6.32-rc3-devel #19
>> Call Trace:
>> [<c012ec9d>] warn_slowpath_common+0x60/0x90
>> [<c012ed01>] warn_slowpath_fmt+0x24/0x27
>> [<c01e3e55>] remove_proc_entry+0x1a8/0x1c7
>> [<e0906435>] ? bond_net_exit+0x0/0xa3 [bonding]
>> [<e09064c3>] bond_net_exit+0x8e/0xa3 [bonding]
>> [<c02e6ee8>] unregister_pernet_gen_subsys+0x23/0x3d
>> [<e0910baa>] bonding_exit+0x3a/0x66 [bonding]
>> [<c015ccf3>] sys_delete_module+0x191/0x1f1
>> [<c0147a30>] ? up_read+0x16/0x2a
>> [<c0102a18>] ? restore_all_notrace+0x0/0x18
>> [<c0353357>] ? do_page_fault+0x0/0x393
>> [<c0102904>] sysenter_do_call+0x12/0x32
>> ---[ end trace 8f3eaeee682a572c ]---
>>
>> Any thoughts? I have not as yet investigated further.
>
>Weird.
>
>We have already run:
>rtnl_link_unregister.
> rtnl_kill_links
> dellink(bond0)
> unregister_netdevice(bond0)
> bond_uninit
> bond_remove_proc_entry
>
>
>So the proc entry should no longer be there. I'm a little nervous about
>the new unregister_netdevice_many but I don't see any obvious problems with
>that code.
>
>Were there by any chance any earlier errors that could have prevented the uninit?
>You weren't inserting multiple copies of the bonding driver?
No, to both questions. Also, if I back out the 7 bonding
patches, the same insmod / rmmod does not panic.
I just set it up and did it again. Fresh boot of the system
(which doesn't load bonding); "insmod drivers/net/bonding/bonding.ko;
rmmod bonding" and blammo.
A little bisect action reveals that the problem first appears
after applying the fifth patch (below). Does a basic insmod / rmmod
cycle work ok for you? I'm specifying no options to bonding.
-J
Subject: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
[...]
This implements a basic set of rtnl link ops and takes advantage of
the fact that rtnl_link_unregister kills all of the surviving
devices to all us to kill bond_free_all. A module alias
is added so ip link add can pull in the bonding module.
Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
---
drivers/net/bonding/bond_main.c | 55 +++++++++++++++++++++-----------------
1 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7a37ecf..6da2a82 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4612,22 +4612,6 @@ static void bond_uninit(struct net_device *bond_dev)
netif_addr_unlock_bh(bond_dev);
}
-/* Unregister and free all bond devices.
- * Caller must hold rtnl_lock.
- */
-static void bond_free_all(void)
-{
- struct bonding *bond, *nxt;
-
- list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
- struct net_device *bond_dev = bond->dev;
-
- unregister_netdevice(bond_dev);
- }
-
- bond_destroy_proc_dir();
-}
-
/*------------------------- Module initialization ---------------------------*/
/*
@@ -5065,6 +5049,23 @@ static int bond_init(struct net_device *bond_dev)
return 0;
}
+static int bond_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+ if (tb[IFLA_ADDRESS]) {
+ if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+ return -EINVAL;
+ if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+ return -EADDRNOTAVAIL;
+ }
+ return 0;
+}
+
+static struct rtnl_link_ops bond_link_ops __read_mostly = {
+ .kind = "bond",
+ .setup = bond_setup,
+ .validate = bond_validate,
+};
+
/* Create a new bond based on the specified name and bonding parameters.
* If name is NULL, obtain a suitable "bond%d" name for us.
* Caller must NOT hold rtnl_lock; we need to release it here before we
@@ -5086,6 +5087,8 @@ int bond_create(const char *name)
goto out;
}
+ bond_dev->rtnl_link_ops = &bond_link_ops;
+
if (!name) {
res = dev_alloc_name(bond_dev, "bond%d");
if (res < 0)
@@ -5115,6 +5118,10 @@ static int __init bonding_init(void)
bond_create_proc_dir();
+ res = rtnl_link_register(&bond_link_ops);
+ if (res)
+ goto err;
+
for (i = 0; i < max_bonds; i++) {
res = bond_create(NULL);
if (res)
@@ -5128,14 +5135,12 @@ static int __init bonding_init(void)
register_netdevice_notifier(&bond_netdev_notifier);
register_inetaddr_notifier(&bond_inetaddr_notifier);
bond_register_ipv6_notifier();
-
- goto out;
-err:
- rtnl_lock();
- bond_free_all();
- rtnl_unlock();
out:
return res;
+err:
+ rtnl_link_unregister(&bond_link_ops);
+ bond_destroy_proc_dir();
+ goto out;
}
@@ -5147,9 +5152,8 @@ static void __exit bonding_exit(void)
bond_destroy_sysfs();
- rtnl_lock();
- bond_free_all();
- rtnl_unlock();
+ rtnl_link_unregister(&bond_link_ops);
+ bond_destroy_proc_dir();
}
module_init(bonding_init);
@@ -5158,3 +5162,4 @@ MODULE_LICENSE("GPL");
MODULE_VERSION(DRV_VERSION);
MODULE_DESCRIPTION(DRV_DESCRIPTION ", v" DRV_VERSION);
MODULE_AUTHOR("Thomas Davis, tadavis@lbl.gov and many others");
+MODULE_ALIAS_RTNL_LINK("bond");
--
1.6.3.1.54.g99dd.dirty
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply related
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-31 0:27 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: David Miller, netdev
In-Reply-To: <21200.1256937150@death.nxdomain.ibm.com>
Jay Vosburgh <fubar@us.ibm.com> writes:
> I put patches 1-7 on a recent net-next-2.6, and from a simple
> "insmod bonding.ko; rmmod bonding" I'm seeing the following:
>
> Any thoughts? I have not as yet investigated further.
I just tried to reproduce this, without luck. My best guess is that there is
something odd particular to your setup.
A simple while :; modprobe bonding ; rmmod bonding ; done runs without problems
for me.
Eric
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Eric W. Biederman @ 2009-10-31 0:37 UTC (permalink / raw)
To: Benjamin LaHaise; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <20091030235314.GH3141@kvack.org>
Benjamin LaHaise <bcrl@lhnet.ca> writes:
> On Fri, Oct 30, 2009 at 04:25:52PM -0700, Eric W. Biederman wrote:
>> I realized after sending the message that s_mode in sysfs_dirent is a
>> real size offense. It is a 16bit field packed in between two longs.
>> So in practice it is possible to move the s_mode up next to s_flags
>> and add a s_nlink after it both unsigned short and get a cheap sysfs_nlink.
>
> That doesn't work -- the number of directory entries can easily exceed 65535.
> Current mid range hardware is good enough to terminate 100,000 network
> interfaces on a single host.
On overflow you nlink becomes zero and you leave it there. That is how
ondisk filesystems handle that case on directories, and find etc
knows how to deal.
>> Since I'm not quite ready to post my patches. I will briefly
>> mention what I have in my queue and hopefully get things posted.
>>
>> I have changes to make it so that sysfs never has to go from
>> the sysfs_dirent to the sysfs inode.
>
> Ah, interesting.
I have to cleanup sysfs before I merge changes for supporting
multiple network namespaces.
>> I have changes to sys_sysctl() so that it becomes a filesystem lookup
>> under /proc/sys. Which ultimately makes the code easier to maintain
>> and debug.
>
> That sounds like a much saner approach, but has the wrinkle that procfs can
> be configured out.
So I will add the dependency. There are very few serious users of sys_sysctl,
and all of them have been getting a deprecated interface warning every
time they use it for the last several years.
>> Now back to getting things forward ported and ready to post.
>
> I'm looking forward to those changes. I've been ignoring procfs for the
> time being by disabling the per-interface entries in the network stack,
> but there is some desire to be able to enable rp_filter on a per-interface
> radius config at runtime. rp_filter has to be disabled across the board
> on my access routers, as there are several places where assymetric routing
> is used for performance reasons.
Just out of curiosity does the loose rp_filter mode work for you?
Eric
^ permalink raw reply
* [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
From: Eric W. Biederman @ 2009-10-31 0:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
In-Reply-To: <m1hbtgmqew.fsf@fess.ebiederm.org>
I tested the recent unregister many changes and got a weird,
nasty and seemingly unrelasted kernel oops. Changing
unregister_netdevice_queue to use list_move_tail fixes
the problem for me.
ip link add type veth
rmmod veth
ls /sys/class/net/
showed one of the veth devices still present.
A subsequent ip link oopsed the box.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index 94f42a1..9c0b202 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5227,6 +5227,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
netdev_init_queues(dev);
INIT_LIST_HEAD(&dev->napi_list);
+ INIT_LIST_HEAD(&dev->unreg_list);
dev->priv_flags = IFF_XMIT_DST_RELEASE;
setup(dev);
strcpy(dev->name, name);
@@ -5308,7 +5309,7 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head)
ASSERT_RTNL();
if (head) {
- list_add_tail(&dev->unreg_list, head);
+ list_move_tail(&dev->unreg_list, head);
} else {
rollback_registered(dev);
/* Finish processing unregister after unlock */
^ permalink raw reply related
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-31 1:06 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: David Miller, netdev
In-Reply-To: <32609.1256947853@death.nxdomain.ibm.com>
Jay Vosburgh <fubar@us.ibm.com> writes:
> No, to both questions. Also, if I back out the 7 bonding
> patches, the same insmod / rmmod does not panic.
>
> I just set it up and did it again. Fresh boot of the system
> (which doesn't load bonding); "insmod drivers/net/bonding/bonding.ko;
> rmmod bonding" and blammo.
>
> A little bisect action reveals that the problem first appears
> after applying the fifth patch (below). Does a basic insmod / rmmod
> cycle work ok for you? I'm specifying no options to bonding.
It works here. The only issue I found was that veth wasn't quite
working. I am wondering if there was some version of the tree
where rtnl_link_unregister is broken and you applied the patches to that.
I tested the net-next tree with my patches at the top:
There are some other differences like I am running a 64bit kernel but
I don't expect that would make a difference in practice.
Eric
commit 6639104bd826e0b1388c69a6b7564fffc636c8a8
Author: Eric W. Biederman <ebiederm@xmission.com>
Date: Thu Oct 29 23:58:54 2009 +0000
bond: Get the rtnl_link_ops support correct
- Don't call rtnl_link_unregister if rtnl_link_register fails
- Set .priv_size so we aren't stomping on uninitialized memory
when we use netdev_priv, on bond devices created with
ip link add type bond.
Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Jay Vosburgh @ 2009-10-31 1:45 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev
In-Reply-To: <m1ljisid8n.fsf@fess.ebiederm.org>
Eric W. Biederman <ebiederm@xmission.com> wrote:
>Jay Vosburgh <fubar@us.ibm.com> writes:
>
>> No, to both questions. Also, if I back out the 7 bonding
>> patches, the same insmod / rmmod does not panic.
>>
>> I just set it up and did it again. Fresh boot of the system
>> (which doesn't load bonding); "insmod drivers/net/bonding/bonding.ko;
>> rmmod bonding" and blammo.
>>
>> A little bisect action reveals that the problem first appears
>> after applying the fifth patch (below). Does a basic insmod / rmmod
>> cycle work ok for you? I'm specifying no options to bonding.
>
>It works here. The only issue I found was that veth wasn't quite
>working. I am wondering if there was some version of the tree
>where rtnl_link_unregister is broken and you applied the patches to that.
Must have been, I did a pull of net-next-2.6 and it seems to
work ok now. Not sure what it was; I was only a day or so behind.
Anyway, it doesn't panic now; I'll give it some further testing next
week.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Eric Dumazet @ 2009-10-31 9:20 UTC (permalink / raw)
To: David Miller; +Cc: francis.moro, linux-kernel, netdev
In-Reply-To: <20091030.122640.137286305.davem@davemloft.net>
David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 30 Oct 2009 16:03:53 +0100
>
>> [PATCH take2] net: fix sk_forward_alloc corruption
>>
>> On UDP sockets, we must call skb_free_datagram() with socket locked,
>> or risk sk_forward_alloc corruption. This requirement is not respected
>> in SUNRPC.
>>
>> Add a convenient helper, skb_free_datagram_locked() and use it in SUNRPC
>>
>> Reported-by: Francis Moreau <francis.moro@gmail.com>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> I've tentatively applied this to my net-2.6 tree, I won't
> push it out until we have positive testing results.
I tested nfs/nfsd with fillowing additional debugging patch.
Without the fix (net: fix sk_forward_alloc corruption), I got warnings,
while with fix applied, I got no warnings. Its probably hard to hit
original bug since its a very small race window.
Trace without fix :
[ 226.686788] RPC: Registered udp transport module.
[ 226.686791] RPC: Registered tcp transport module.
[ 226.686792] RPC: Registered tcp NFSv4.1 backchannel transport module.
[ 226.851677] Installing knfsd (copyright (C) 1996 okir@monad.swb.de).
[ 226.869381] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state recovery directory
[ 226.869443] NFSD: unable to find recovery directory /var/lib/nfs/v4recovery
[ 226.869488] NFSD: starting 90-second grace period
[ 226.870564] svc: failed to register lockdv1 RPC service (errno 97).
[ 244.401237] ------------[ cut here ]------------
[ 244.401288] WARNING: at include/net/sock.h:886 sock_rfree+0x54/0x70()
[ 244.401334] Hardware name: ProLiant BL460c G1
[ 244.401379] Modules linked in: nfsd lockd auth_rpcgss sunrpc exportfs bonding ipv6
[ 244.401699] Pid: 5295, comm: nfsd Not tainted 2.6.32-rc3-00920-gbdd6be3-dirty #342
[ 244.401745] Call Trace:
[ 244.401784] [<c055355a>] ? printk+0x1d/0x23
[ 244.401827] [<c023c3f2>] warn_slowpath_common+0x72/0xa0
[ 244.401869] [<c04b0d54>] ? sock_rfree+0x54/0x70
[ 244.401909] [<c04b0d54>] ? sock_rfree+0x54/0x70
[ 244.401950] [<c023c43a>] warn_slowpath_null+0x1a/0x20
[ 244.401991] [<c04b0d54>] sock_rfree+0x54/0x70
[ 244.402033] [<c04b4cf5>] skb_release_head_state+0x45/0xe0
[ 244.402084] [<c04b49f0>] __kfree_skb+0x10/0x90
[ 244.402125] [<c04b4a8c>] consume_skb+0x1c/0x40
[ 244.402166] [<c04b7c62>] skb_free_datagram+0x12/0x70
[ 244.402217] [<fc9fa177>] svc_release_skb+0x37/0x60 [sunrpc]
[ 244.402261] [<c026f452>] ? module_put+0x62/0xf0
[ 244.402308] [<fca05c98>] svc_send+0x28/0xc0 [sunrpc]
[ 244.402356] [<fc9f8acb>] svc_process+0x22b/0x770 [sunrpc]
[ 244.402416] [<fd04895c>] nfsd+0xac/0x140 [nfsd]
[ 244.402484] [<fd0488b0>] ? nfsd+0x0/0x140 [nfsd]
[ 244.402543] [<c0257ab4>] kthread+0x74/0x80
[ 244.402602] [<c0257a40>] ? kthread+0x0/0x80
[ 244.402656] [<c020390f>] kernel_thread_helper+0x7/0x10
[ 244.402711] ---[ end trace e202cdb1b491aa15 ]---
Debugging patch :
Intent is to make sure sock is locked by user or by softirq handler,
unless we are currently dismantling socket (sk_refcnt==0)
To speedup this test, we could use several bits of sk_lock.owned instead
of one for the user case.
(one for lock owned by user, one for lowk owned by softirq, one in dismantle phase)
diff --git a/include/net/sock.h b/include/net/sock.h
index 55de3bd..08ecb0e 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -849,10 +849,15 @@ static inline int sk_rmem_schedule(struct sock *sk, int size)
__sk_mem_schedule(sk, size, SK_MEM_RECV);
}
+#define sock_owned_by_user(sk) ((sk)->sk_lock.owned)
+#define sock_owned(sk) ((sk)->sk_lock.owned || \
+ spin_is_locked(&(sk)->sk_lock.slock))
+
static inline void sk_mem_reclaim(struct sock *sk)
{
if (!sk_has_account(sk))
return;
+ WARN_ON(!(sock_owned(sk) || atomic_read(&sk->sk_refcnt) == 0));
if (sk->sk_forward_alloc >= SK_MEM_QUANTUM)
__sk_mem_reclaim(sk);
}
@@ -861,6 +866,7 @@ static inline void sk_mem_reclaim_partial(struct sock *sk)
{
if (!sk_has_account(sk))
return;
+ WARN_ON(!(sock_owned(sk) || atomic_read(&sk->sk_refcnt) == 0));
if (sk->sk_forward_alloc > SK_MEM_QUANTUM)
__sk_mem_reclaim(sk);
}
@@ -869,6 +875,7 @@ static inline void sk_mem_charge(struct sock *sk, int size)
{
if (!sk_has_account(sk))
return;
+ WARN_ON(!sock_owned(sk));
sk->sk_forward_alloc -= size;
}
@@ -876,6 +883,7 @@ static inline void sk_mem_uncharge(struct sock *sk, int size)
{
if (!sk_has_account(sk))
return;
+ WARN_ON(!sock_owned(sk));
sk->sk_forward_alloc += size;
}
@@ -900,7 +908,6 @@ static inline void sk_wmem_free_skb(struct sock *sk, struct sk_buff *skb)
* Since ~2.3.5 it is also exclusive sleep lock serializing
* accesses from user process context.
*/
-#define sock_owned_by_user(sk) ((sk)->sk_lock.owned)
/*
* Macro so as to not evaluate some arguments when
^ permalink raw reply related
* [PATCH] e1000: the power down when running ifdown command
From: Naohiro Ooiwa @ 2009-10-31 9:39 UTC (permalink / raw)
To: jeffrey.t.kirsher, jesse.brandeburg, peter.p.waskiewicz.jr,
john.ronciak, davem
Cc: Andrew Morton, netdev, svaidy, e1000-devel
Hi All
I resend my patch.
Sorry, my previous mail lacked an explanation.
The e1000 driver doesn't let the power down when running ifdown command.
So, I set to the D3hot state of a PCI device at the end of e1000_close().
With this modification, e1000 driver reduces power by ifdown.
It's about 6 watts when I measured a total power of one server machine
in my environment.
I tested this patch. The result is good enough to me.
Could you please check my patch ?
If I should have other considerations, please tell me.
Thanks you.
Naohiro Ooiwa
Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
---
drivers/net/e1000/e1000_main.c | 32 ++++++++++++++++++++++++++++++++
1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index bcd192c..12e1a42 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -26,6 +26,11 @@
*******************************************************************************/
+/*
+ * define this if you want pci save power while ifdown.
+ */
+#define E1000_PCI_POWER_SAVE
+
#include "e1000.h"
#include <net/ip6_checksum.h>
@@ -1248,6 +1253,23 @@ static int e1000_open(struct net_device *netdev)
struct e1000_hw *hw = &adapter->hw;
int err;
+#ifdef E1000_PCI_POWER_SAVE
+ struct pci_dev *pdev = adapter->pdev;
+
+ pci_set_power_state(pdev, PCI_D0);
+ pci_restore_state(pdev);
+
+ if (adapter->need_ioport)
+ err = pci_enable_device(pdev);
+ else
+ err = pci_enable_device_mem(pdev);
+ if (err) {
+ printk(KERN_ERR "e1000: Cannot enable PCI device from power-save\n");
+ return err;
+ }
+ pci_set_master(pdev);
+#endif
+
/* disallow open during test */
if (test_bit(__E1000_TESTING, &adapter->flags))
return -EBUSY;
@@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
goto err_setup_rx;
e1000_power_up_phy(adapter);
+ e1000_reset(adapter);
adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
if ((hw->mng_cookie.status &
@@ -1341,6 +1364,15 @@ static int e1000_close(struct net_device *netdev)
e1000_vlan_rx_kill_vid(netdev, adapter->mng_vlan_id);
}
+#ifdef E1000_PCI_POWER_SAVE
+#ifdef CONFIG_PM
+ pci_save_state(adapter->pdev);
+#endif
+ pci_disable_device(adapter->pdev);
+ pci_wake_from_d3(adapter->pdev, true);
+ pci_set_power_state(adapter->pdev, PCI_D3hot);
+#endif
+
return 0;
}
--
1.5.4.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
From: Eric Dumazet @ 2009-10-31 9:41 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
In-Reply-To: <m1639wjshq.fsf_-_@fess.ebiederm.org>
Eric W. Biederman a écrit :
> I tested the recent unregister many changes and got a weird,
> nasty and seemingly unrelasted kernel oops. Changing
> unregister_netdevice_queue to use list_move_tail fixes
> the problem for me.
>
> ip link add type veth
> rmmod veth
>
> ls /sys/class/net/
> showed one of the veth devices still present.
>
> A subsequent ip link oopsed the box.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
Yes, problem is __rtnl_kill_links() doesnt anymore restart
its loop and does :
for_each_netdev(net, dev) {
if (dev->rtnl_link_ops == ops)
ops->dellink(dev, &list_kill);
}
unregister_netdevice_many(&list_kill);
As veth wants in its dellink() method to unregister two netdevices,
we really want your fix or corrupt list_kill
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks Eric
^ permalink raw reply
* Re: [PATCH net-next-2.6] veth: Fix veth_dellink method
From: Eric Dumazet @ 2009-10-31 9:43 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David S. Miller, Linux Netdev List, Pavel Emelyanov
In-Reply-To: <m1hbtgmqew.fsf@fess.ebiederm.org>
Eric W. Biederman a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>
>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>> index ffb502d..9bed694 100644
>> --- a/drivers/net/veth.c
>> +++ b/drivers/net/veth.c
>> @@ -450,8 +450,8 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
>> priv = netdev_priv(dev);
>> peer = priv->peer;
>>
>> - unregister_netdevice(dev);
>> - unregister_netdevice(peer);
>> + unregister_netdevice_queue(dev, head);
>> + unregister_netdevice_queue(peer, head);
>
> Unless I am mistaken you need to change the list_add_tail to
> list_move_tail in unregister_netdevice_queue because we will be
> adding each veth device twice.
>
You are right, and your patch is the solution to this problem
Thanks
^ permalink raw reply
* Net driver module dependencies on PHY driver modules
From: Ben Hutchings @ 2009-10-31 11:42 UTC (permalink / raw)
To: Matt Carlson, Michael Chan, netdev; +Cc: 553024
[-- Attachment #1: Type: text/plain, Size: 686 bytes --]
I'm handling a bug report <http://bugs.debian.org/553024> relating to
tg3 using phylib and missing the broadcom module.
There seem to be two general problems here:
1. PHY drivers are not auto-loaded
2. An initramfs builder cannot statically determine that a net device
requires a PHY driver
It seems to me that PHY drivers should have aliases based on MDIO PHY
ids, and phylib should request_module() if no suitable driver is
registered. This would solve (1) and go some way to solving (2).
Ben.
--
Ben Hutchings
The generation of random numbers is too important to be left to chance.
- Robert Coveyou
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: Net driver module dependencies on PHY driver modules
From: Tim Gardner @ 2009-10-31 15:34 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Matt Carlson, Michael Chan, netdev, 553024
In-Reply-To: <1256989326.3136.284.camel@localhost>
Ben Hutchings wrote:
> I'm handling a bug report <http://bugs.debian.org/553024> relating to
> tg3 using phylib and missing the broadcom module.
>
> There seem to be two general problems here:
> 1. PHY drivers are not auto-loaded
> 2. An initramfs builder cannot statically determine that a net device
> requires a PHY driver
>
> It seems to me that PHY drivers should have aliases based on MDIO PHY
> ids, and phylib should request_module() if no suitable driver is
> registered. This would solve (1) and go some way to solving (2).
>
> Ben.
>
I encountered the same issue with Karmic and worked around it by
building in all of the phylib modules, e.g., CONFIG_BROADCOM_PHY=y. I
agree that its not the best long term solution.
rtg
--
Tim Gardner tim.gardner@canonical.com
^ 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