* [PATCH] ehea: DLPAR memory add fix
From: Jan-Bernd Themann @ 2007-10-01 14:33 UTC (permalink / raw)
To: Jeff Garzik
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher
Due to stability issues in high load situations the HW queue handling
has to be changed. The HW queues are now stopped and restarted again instead
of destroying and allocating new HW queues.
Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>
---
drivers/net/ehea/ehea.h | 4 +-
drivers/net/ehea/ehea_main.c | 276 +++++++++++++++++++++++++++++++++++++-----
drivers/net/ehea/ehea_phyp.h | 1 +
drivers/net/ehea/ehea_qmr.c | 20 ++--
drivers/net/ehea/ehea_qmr.h | 4 +-
5 files changed, 259 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index c0cbd94..3022089 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -40,13 +40,13 @@
#include <asm/io.h>
#define DRV_NAME "ehea"
-#define DRV_VERSION "EHEA_0074"
+#define DRV_VERSION "EHEA_0077"
/* eHEA capability flags */
#define DLPAR_PORT_ADD_REM 1
#define DLPAR_MEM_ADD 2
#define DLPAR_MEM_REM 4
-#define EHEA_CAPABILITIES (DLPAR_PORT_ADD_REM)
+#define EHEA_CAPABILITIES (DLPAR_PORT_ADD_REM | DLPAR_MEM_ADD)
#define EHEA_MSG_DEFAULT (NETIF_MSG_LINK | NETIF_MSG_TIMER \
| NETIF_MSG_RX_ERR | NETIF_MSG_TX_ERR)
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 62d6c1e..5bc0a15 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -97,6 +97,7 @@ u64 ehea_driver_flags = 0;
struct workqueue_struct *ehea_driver_wq;
struct work_struct ehea_rereg_mr_task;
+struct semaphore dlpar_mem_lock;
static int __devinit ehea_probe_adapter(struct ibmebus_dev *dev,
const struct of_device_id *id);
@@ -177,16 +178,24 @@ static void ehea_refill_rq1(struct ehea_port_res *pr, int index, int nr_of_wqes)
struct sk_buff **skb_arr_rq1 = pr->rq1_skba.arr;
struct net_device *dev = pr->port->netdev;
int max_index_mask = pr->rq1_skba.len - 1;
+ int fill_wqes = pr->rq1_skba.os_skbs + nr_of_wqes;
+ int adder = 0;
int i;
- if (!nr_of_wqes)
+ pr->rq1_skba.os_skbs = 0;
+
+ if (unlikely(test_bit(__EHEA_STOP_XFER, &ehea_driver_flags))) {
+ pr->rq1_skba.index = index;
+ pr->rq1_skba.os_skbs = fill_wqes;
return;
+ }
- for (i = 0; i < nr_of_wqes; i++) {
+ for (i = 0; i < fill_wqes; i++) {
if (!skb_arr_rq1[index]) {
skb_arr_rq1[index] = netdev_alloc_skb(dev,
EHEA_L_PKT_SIZE);
if (!skb_arr_rq1[index]) {
+ pr->rq1_skba.os_skbs = fill_wqes - i;
ehea_error("%s: no mem for skb/%d wqes filled",
dev->name, i);
break;
@@ -194,9 +203,14 @@ static void ehea_refill_rq1(struct ehea_port_res *pr, int index, int nr_of_wqes)
}
index--;
index &= max_index_mask;
+ adder++;
}
+
+ if (adder == 0)
+ return;
+
/* Ring doorbell */
- ehea_update_rq1a(pr->qp, i);
+ ehea_update_rq1a(pr->qp, adder);
}
static int ehea_init_fill_rq1(struct ehea_port_res *pr, int nr_rq1a)
@@ -230,16 +244,21 @@ static int ehea_refill_rq_def(struct ehea_port_res *pr,
struct sk_buff **skb_arr = q_skba->arr;
struct ehea_rwqe *rwqe;
int i, index, max_index_mask, fill_wqes;
+ int adder = 0;
int ret = 0;
fill_wqes = q_skba->os_skbs + num_wqes;
+ q_skba->os_skbs = 0;
- if (!fill_wqes)
+ if (unlikely(test_bit(__EHEA_STOP_XFER, &ehea_driver_flags))) {
+ q_skba->os_skbs = fill_wqes;
return ret;
+ }
index = q_skba->index;
max_index_mask = q_skba->len - 1;
for (i = 0; i < fill_wqes; i++) {
+ u64 tmp_addr;
struct sk_buff *skb = netdev_alloc_skb(dev, packet_size);
if (!skb) {
ehea_error("%s: no mem for skb/%d wqes filled",
@@ -251,30 +270,37 @@ static int ehea_refill_rq_def(struct ehea_port_res *pr,
skb_reserve(skb, NET_IP_ALIGN);
skb_arr[index] = skb;
+ tmp_addr = ehea_map_vaddr(skb->data);
+ if (tmp_addr == -1) {
+ dev_kfree_skb(skb);
+ q_skba->os_skbs = fill_wqes - i;
+ ret = 0;
+ break;
+ }
rwqe = ehea_get_next_rwqe(qp, rq_nr);
rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, wqe_type)
| EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
rwqe->sg_list[0].l_key = pr->recv_mr.lkey;
- rwqe->sg_list[0].vaddr = ehea_map_vaddr(skb->data);
+ rwqe->sg_list[0].vaddr = tmp_addr;
rwqe->sg_list[0].len = packet_size;
rwqe->data_segments = 1;
index++;
index &= max_index_mask;
-
- if (unlikely(test_bit(__EHEA_STOP_XFER, &ehea_driver_flags)))
- goto out;
+ adder++;
}
q_skba->index = index;
+ if (adder == 0)
+ goto out;
/* Ring doorbell */
iosync();
if (rq_nr == 2)
- ehea_update_rq2a(pr->qp, i);
+ ehea_update_rq2a(pr->qp, adder);
else
- ehea_update_rq3a(pr->qp, i);
+ ehea_update_rq3a(pr->qp, adder);
out:
return ret;
}
@@ -1967,11 +1993,12 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
ehea_dump(swqe, 512, "swqe");
}
- if (unlikely(test_bit(__EHEA_STOP_XFER, &ehea_driver_flags)))
- goto out;
+ if (unlikely(test_bit(__EHEA_STOP_XFER, &ehea_driver_flags))) {
+ netif_stop_queue(dev);
+ swqe->tx_control |= EHEA_SWQE_PURGE;
+ }
ehea_post_swqe(pr->qp, swqe);
- pr->tx_packets++;
if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
spin_lock_irqsave(&pr->netif_queue, flags);
@@ -1984,7 +2011,7 @@ static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
}
dev->trans_start = jiffies;
spin_unlock(&pr->xmit_lock);
-out:
+
return NETDEV_TX_OK;
}
@@ -2376,6 +2403,192 @@ static int ehea_stop(struct net_device *dev)
return ret;
}
+void ehea_purge_sq(struct ehea_qp *orig_qp)
+{
+ struct ehea_qp qp = *orig_qp;
+ struct ehea_qp_init_attr *init_attr = &qp.init_attr;
+ struct ehea_swqe *swqe;
+ int wqe_index;
+ int i;
+
+ for (i = 0; i < init_attr->act_nr_send_wqes; i++) {
+ swqe = ehea_get_swqe(&qp, &wqe_index);
+ swqe->tx_control |= EHEA_SWQE_PURGE;
+ }
+}
+
+int ehea_stop_qps(struct net_device *dev)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ struct ehea_adapter *adapter = port->adapter;
+ struct hcp_modify_qp_cb0* cb0;
+ int ret = -EIO;
+ int dret;
+ int i;
+ u64 hret;
+ u64 dummy64 = 0;
+ u16 dummy16 = 0;
+
+ cb0 = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!cb0) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < (port->num_def_qps + port->num_add_tx_qps); i++) {
+ struct ehea_port_res *pr = &port->port_res[i];
+ struct ehea_qp *qp = pr->qp;
+
+ /* Purge send queue */
+ ehea_purge_sq(qp);
+
+ /* Disable queue pair */
+ hret = ehea_h_query_ehea_qp(adapter->handle, 0, qp->fw_handle,
+ EHEA_BMASK_SET(H_QPCB0_ALL, 0xFFFF),
+ cb0);
+ if (hret != H_SUCCESS) {
+ ehea_error("query_ehea_qp failed (1)");
+ goto out;
+ }
+
+ cb0->qp_ctl_reg = (cb0->qp_ctl_reg & H_QP_CR_RES_STATE) << 8;
+ cb0->qp_ctl_reg &= ~H_QP_CR_ENABLED;
+
+ hret = ehea_h_modify_ehea_qp(adapter->handle, 0, qp->fw_handle,
+ EHEA_BMASK_SET(H_QPCB0_QP_CTL_REG,
+ 1), cb0, &dummy64,
+ &dummy64, &dummy16, &dummy16);
+ if (hret != H_SUCCESS) {
+ ehea_error("modify_ehea_qp failed (1)");
+ goto out;
+ }
+
+ hret = ehea_h_query_ehea_qp(adapter->handle, 0, qp->fw_handle,
+ EHEA_BMASK_SET(H_QPCB0_ALL, 0xFFFF),
+ cb0);
+ if (hret != H_SUCCESS) {
+ ehea_error("query_ehea_qp failed (2)");
+ goto out;
+ }
+
+ /* deregister shared memory regions */
+ dret = ehea_rem_smrs(pr);
+ if (dret) {
+ ehea_error("unreg shared memory region failed");
+ goto out;
+ }
+ }
+
+ ret = 0;
+out:
+ kfree(cb0);
+
+ return ret;
+}
+
+void ehea_update_rqs(struct ehea_qp *orig_qp, struct ehea_port_res * pr)
+{
+ struct ehea_qp qp = *orig_qp;
+ struct ehea_qp_init_attr *init_attr = &qp.init_attr;
+ struct ehea_rwqe *rwqe;
+ struct sk_buff **skba_rq2 = pr->rq2_skba.arr;
+ struct sk_buff **skba_rq3 = pr->rq3_skba.arr;
+ struct sk_buff *skb;
+ u32 lkey = pr->recv_mr.lkey;
+
+
+ int i;
+ int index;
+
+ for (i = 0; i < init_attr->act_nr_rwqes_rq2 + 1; i++) {
+ rwqe = ehea_get_next_rwqe(&qp, 2);
+ rwqe->sg_list[0].l_key = lkey;
+ index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX, rwqe->wr_id);
+ skb = skba_rq2[index];
+ if (skb)
+ rwqe->sg_list[0].vaddr = ehea_map_vaddr(skb->data);
+ }
+
+ for (i = 0; i < init_attr->act_nr_rwqes_rq3 + 1; i++) {
+ rwqe = ehea_get_next_rwqe(&qp, 3);
+ rwqe->sg_list[0].l_key = lkey;
+ index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX, rwqe->wr_id);
+ skb = skba_rq3[index];
+ if (skb)
+ rwqe->sg_list[0].vaddr = ehea_map_vaddr(skb->data);
+ }
+}
+
+int ehea_restart_qps(struct net_device *dev)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ struct ehea_adapter *adapter = port->adapter;
+ int ret = 0;
+ int i;
+
+ struct hcp_modify_qp_cb0* cb0;
+ u64 hret;
+ u64 dummy64 = 0;
+ u16 dummy16 = 0;
+
+ cb0 = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!cb0) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < (port->num_def_qps + port->num_add_tx_qps); i++) {
+ struct ehea_port_res *pr = &port->port_res[i];
+ struct ehea_qp *qp = pr->qp;
+
+ ret = ehea_gen_smrs(pr);
+ if (ret) {
+ ehea_error("creation of shared memory regions failed");
+ goto out;
+ }
+
+ ehea_update_rqs(qp, pr);
+
+ /* Enable queue pair */
+ hret = ehea_h_query_ehea_qp(adapter->handle, 0, qp->fw_handle,
+ EHEA_BMASK_SET(H_QPCB0_ALL, 0xFFFF),
+ cb0);
+ if (hret != H_SUCCESS) {
+ ehea_error("query_ehea_qp failed (1)");
+ goto out;
+ }
+
+ cb0->qp_ctl_reg = (cb0->qp_ctl_reg & H_QP_CR_RES_STATE) << 8;
+ cb0->qp_ctl_reg |= H_QP_CR_ENABLED;
+
+ hret = ehea_h_modify_ehea_qp(adapter->handle, 0, qp->fw_handle,
+ EHEA_BMASK_SET(H_QPCB0_QP_CTL_REG,
+ 1), cb0, &dummy64,
+ &dummy64, &dummy16, &dummy16);
+ if (hret != H_SUCCESS) {
+ ehea_error("modify_ehea_qp failed (1)");
+ goto out;
+ }
+
+ hret = ehea_h_query_ehea_qp(adapter->handle, 0, qp->fw_handle,
+ EHEA_BMASK_SET(H_QPCB0_ALL, 0xFFFF),
+ cb0);
+ if (hret != H_SUCCESS) {
+ ehea_error("query_ehea_qp failed (2)");
+ goto out;
+ }
+
+ /* refill entire queue */
+ ehea_refill_rq1(pr, pr->rq1_skba.index, 0);
+ ehea_refill_rq2(pr, 0);
+ ehea_refill_rq3(pr, 0);
+ }
+out:
+ kfree(cb0);
+
+ return ret;
+}
+
static void ehea_reset_port(struct work_struct *work)
{
int ret;
@@ -2395,6 +2608,8 @@ static void ehea_reset_port(struct work_struct *work)
if (ret)
goto out;
+ ehea_set_multicast_list(dev);
+
if (netif_msg_timer(port))
ehea_info("Device %s resetted successfully", dev->name);
@@ -2411,6 +2626,7 @@ static void ehea_rereg_mrs(struct work_struct *work)
int ret, i;
struct ehea_adapter *adapter;
+ down(&dlpar_mem_lock);
ehea_info("LPAR memory enlarged - re-initializing driver");
list_for_each_entry(adapter, &adapter_list, list)
@@ -2423,14 +2639,14 @@ static void ehea_rereg_mrs(struct work_struct *work)
struct net_device *dev = port->netdev;
if (dev->flags & IFF_UP) {
- ehea_info("stopping %s",
- dev->name);
down(&port->port_lock);
netif_stop_queue(dev);
-
+ ret = ehea_stop_qps(dev);
+ if (ret) {
+ up(&port->port_lock);
+ goto out;
+ }
port_napi_disable(port);
-
- ehea_down(dev);
up(&port->port_lock);
}
}
@@ -2446,10 +2662,11 @@ static void ehea_rereg_mrs(struct work_struct *work)
}
ehea_destroy_busmap();
-
ret = ehea_create_busmap();
- if (ret)
+ if (ret) {
+ ehea_error("creating ehea busmap failed");
goto out;
+ }
clear_bit(__EHEA_STOP_XFER, &ehea_driver_flags);
@@ -2471,21 +2688,18 @@ static void ehea_rereg_mrs(struct work_struct *work)
struct net_device *dev = port->netdev;
if (dev->flags & IFF_UP) {
- ehea_info("restarting %s",
- dev->name);
down(&port->port_lock);
-
- ret = ehea_up(dev);
- if (!ret) {
- port_napi_enable(port);
+ port_napi_enable(port);
+ ret = ehea_restart_qps(dev);
+ if (!ret)
netif_wake_queue(dev);
- }
-
up(&port->port_lock);
}
}
}
}
+ up(&dlpar_mem_lock);
+ ehea_info("re-initializing driver complete");
out:
return;
}
@@ -2494,7 +2708,8 @@ static void ehea_tx_watchdog(struct net_device *dev)
{
struct ehea_port *port = netdev_priv(dev);
- if (netif_carrier_ok(dev))
+ if (netif_carrier_ok(dev) &&
+ !test_bit(__EHEA_STOP_XFER, &ehea_driver_flags))
queue_work(port->adapter->ehea_wq, &port->reset_task);
}
@@ -3139,6 +3354,7 @@ int __init ehea_module_init(void)
ehea_driver_wq = create_workqueue("ehea_driver_wq");
INIT_WORK(&ehea_rereg_mr_task, ehea_rereg_mrs);
+ sema_init(&dlpar_mem_lock, 1);
ret = check_module_parm();
if (ret)
diff --git a/drivers/net/ehea/ehea_phyp.h b/drivers/net/ehea/ehea_phyp.h
index 89b6353..faa191d 100644
--- a/drivers/net/ehea/ehea_phyp.h
+++ b/drivers/net/ehea/ehea_phyp.h
@@ -126,6 +126,7 @@ struct hcp_modify_qp_cb0 {
#define H_QP_CR_STATE_RDY2RCV 0x0000030000000000ULL /* Ready to recv */
#define H_QP_CR_STATE_RDY2SND 0x0000050000000000ULL /* Ready to send */
#define H_QP_CR_STATE_ERROR 0x0000800000000000ULL /* Error */
+#define H_QP_CR_RES_STATE 0x0000007F00000000ULL /* Resultant state */
struct hcp_modify_qp_cb1 {
u32 qpn; /* 00 */
diff --git a/drivers/net/ehea/ehea_qmr.c b/drivers/net/ehea/ehea_qmr.c
index c82e245..329a252 100644
--- a/drivers/net/ehea/ehea_qmr.c
+++ b/drivers/net/ehea/ehea_qmr.c
@@ -563,8 +563,7 @@ int ehea_destroy_qp(struct ehea_qp *qp)
int ehea_create_busmap( void )
{
u64 vaddr = EHEA_BUSMAP_START;
- unsigned long abs_max_pfn = 0;
- unsigned long sec_max_pfn;
+ unsigned long high_section_index = 0;
int i;
/*
@@ -574,14 +573,10 @@ int ehea_create_busmap( void )
ehea_bmap.valid_sections = 0;
for (i = 0; i < NR_MEM_SECTIONS; i++)
- if (valid_section_nr(i)) {
- sec_max_pfn = section_nr_to_pfn(i);
- if (sec_max_pfn > abs_max_pfn)
- abs_max_pfn = sec_max_pfn;
- ehea_bmap.valid_sections++;
- }
+ if (valid_section_nr(i))
+ high_section_index = i;
- ehea_bmap.entries = abs_max_pfn / EHEA_PAGES_PER_SECTION + 1;
+ ehea_bmap.entries = high_section_index + 1;
ehea_bmap.vaddr = vmalloc(ehea_bmap.entries * sizeof(*ehea_bmap.vaddr));
if (!ehea_bmap.vaddr)
@@ -593,6 +588,7 @@ int ehea_create_busmap( void )
if (pfn_valid(pfn)) {
ehea_bmap.vaddr[i] = vaddr;
vaddr += EHEA_SECTSIZE;
+ ehea_bmap.valid_sections++;
} else
ehea_bmap.vaddr[i] = 0;
}
@@ -637,7 +633,7 @@ int ehea_reg_kernel_mr(struct ehea_adapter *adapter, struct ehea_mr *mr)
mr_len = ehea_bmap.valid_sections * EHEA_SECTSIZE;
- pt = kzalloc(EHEA_MAX_RPAGE * sizeof(u64), GFP_KERNEL);
+ pt = kzalloc(PAGE_SIZE, GFP_KERNEL);
if (!pt) {
ehea_error("no mem");
ret = -ENOMEM;
@@ -660,8 +656,8 @@ int ehea_reg_kernel_mr(struct ehea_adapter *adapter, struct ehea_mr *mr)
void *sectbase = __va(i << SECTION_SIZE_BITS);
unsigned long k = 0;
- for (j = 0; j < (PAGES_PER_SECTION / EHEA_MAX_RPAGE);
- j++) {
+ for (j = 0; j < (EHEA_PAGES_PER_SECTION /
+ EHEA_MAX_RPAGE); j++) {
for (m = 0; m < EHEA_MAX_RPAGE; m++) {
pg = sectbase + ((k++) * EHEA_PAGESIZE);
diff --git a/drivers/net/ehea/ehea_qmr.h b/drivers/net/ehea/ehea_qmr.h
index b71f845..562de0e 100644
--- a/drivers/net/ehea/ehea_qmr.h
+++ b/drivers/net/ehea/ehea_qmr.h
@@ -39,7 +39,7 @@
#define EHEA_PAGESHIFT 12
#define EHEA_PAGESIZE (1UL << EHEA_PAGESHIFT)
#define EHEA_SECTSIZE (1UL << 24)
-#define EHEA_PAGES_PER_SECTION (EHEA_SECTSIZE >> PAGE_SHIFT)
+#define EHEA_PAGES_PER_SECTION (EHEA_SECTSIZE >> EHEA_PAGESHIFT)
#if (1UL << SECTION_SIZE_BITS) < EHEA_SECTSIZE
#error eHEA module can't work if kernel sectionsize < ehea sectionsize
@@ -145,7 +145,7 @@ struct ehea_rwqe {
#define EHEA_CQE_VLAN_TAG_XTRACT 0x0400
#define EHEA_CQE_TYPE_RQ 0x60
-#define EHEA_CQE_STAT_ERR_MASK 0x721F
+#define EHEA_CQE_STAT_ERR_MASK 0x720F
#define EHEA_CQE_STAT_FAT_ERR_MASK 0x1F
#define EHEA_CQE_STAT_ERR_TCP 0x4000
#define EHEA_CQE_STAT_ERR_IP 0x2000
--
1.5.2
^ permalink raw reply related
* Re: [PATCH 1/1][TCP]: break missing at end of switch statement
From: Arnaldo Carvalho de Melo @ 2007-10-01 14:31 UTC (permalink / raw)
To: Gerrit Renker; +Cc: YOSHIFUJI Hideaki, netdev, yoshfuji
In-Reply-To: <200710011439.28676@strip-the-willow>
Em Mon, Oct 01, 2007 at 02:39:28PM +0100, Gerrit Renker escreveu:
> Quoting YOSHIFUJI Hideaki:
> |
> | > [TCP]: break missing at end of switch statement
> | >
> | > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> | > ---
> | > --- a/net/ipv4/tcp_input.c
> | > +++ b/net/ipv4/tcp_input.c
> | > @@ -3129,6 +3129,7 @@ static void tcp_reset(struct sock *sk)
> | > return;
> | > default:
> | > sk->sk_err = ECONNRESET;
> | > + break;
> | > }
> | >
> | > if (!sock_flag(sk, SOCK_DEAD))
> |
> | NAK; it is not required at all.
> |
> | --yoshfuji
> |
> If it were true what you are saying then the statement
>
> `sk->sk_err = ECONNRESET;'
>
> can go as well since it will always be overridden.
Gerrit,
It is not required. The statement you mention will be executed
when the sk_state is not one of TCP_SYN_SENT, TCP_CLOSE_WAIT or
TCP_CLOSE.
A 'break' is only needed in a label block if it is not the last
one.
- Arnaldo
^ permalink raw reply
* Re: [PATCH] make netlink processing routines semi-synchronious (inspired by rtnl) v2
From: Patrick McHardy @ 2007-10-01 14:27 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: davem, johannes, tgraf, pablo, netdev
In-Reply-To: <20071001142558.GA9997@iris.sw.ru>
Denis V. Lunev wrote:
> The code in netfilter/nfnetlink.c and in ./net/netlink/genetlink.c looks
> like outdated copy/paste from rtnetlink.c. Push them into sync with the
> original.
>
> Changes from v1:
> - deleted comment in nfnetlink_rcv_msg by request of Patrick McHardy
Thanks.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
Acked-by: Patrick McHardy <kaber@trash.net>
^ permalink raw reply
* [PATCH] memory leak in netlink user->kernel processing
From: Denis V. Lunev @ 2007-10-01 14:29 UTC (permalink / raw)
To: davem; +Cc: netdev
netlink_kernel_create can be called with NULL as an input callback in several
places, f.e. in kobject_uevent_init. This means that if one sends packet from
user to kernel for such a socket, the packet will be leaked in the socket
queue forever.
This patch adds a simple generic cleanup callback for these sockets.
Signed-off-by: Denis V. Lunev <den@openvz.org>
--- ./net/netlink/af_netlink.c.nlk4 2007-08-26 19:30:38.000000000 +0400
+++ ./net/netlink/af_netlink.c 2007-10-01 18:00:58.000000000 +0400
@@ -1301,6 +1301,13 @@ out:
return err ? : copied;
}
+static void netlink_rcv_drop(struct sock *sk, int len)
+{
+ struct sk_buff *skb;
+ while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL)
+ kfree_skb(skb);
+}
+
static void netlink_data_ready(struct sock *sk, int len)
{
struct netlink_sock *nlk = nlk_sk(sk);
@@ -1346,8 +1353,7 @@ netlink_kernel_create(struct net *net, i
sk = sock->sk;
sk->sk_data_ready = netlink_data_ready;
- if (input)
- nlk_sk(sk)->data_ready = input;
+ nlk_sk(sk)->data_ready = input != NULL ? input : netlink_rcv_drop;
if (netlink_insert(sk, net, 0))
goto out_sock_release;
^ permalink raw reply
* [PATCH] make netlink processing routines semi-synchronious (inspired by rtnl) v2
From: Denis V. Lunev @ 2007-10-01 14:25 UTC (permalink / raw)
To: davem; +Cc: johannes, tgraf, pablo, kaber, netdev
The code in netfilter/nfnetlink.c and in ./net/netlink/genetlink.c looks
like outdated copy/paste from rtnetlink.c. Push them into sync with the
original.
Changes from v1:
- deleted comment in nfnetlink_rcv_msg by request of Patrick McHardy
Signed-off-by: Denis V. Lunev <den@openvz.org>
--- ./net/netfilter/nfnetlink.c.nlk3 2007-10-01 09:47:53.000000000 +0400
+++ ./net/netfilter/nfnetlink.c 2007-10-01 17:13:09.000000000 +0400
@@ -44,26 +44,14 @@ static struct sock *nfnl = NULL;
static const struct nfnetlink_subsystem *subsys_table[NFNL_SUBSYS_COUNT];
static DEFINE_MUTEX(nfnl_mutex);
-static void nfnl_lock(void)
+static inline void nfnl_lock(void)
{
mutex_lock(&nfnl_mutex);
}
-static int nfnl_trylock(void)
-{
- return !mutex_trylock(&nfnl_mutex);
-}
-
-static void __nfnl_unlock(void)
-{
- mutex_unlock(&nfnl_mutex);
-}
-
-static void nfnl_unlock(void)
+static inline void nfnl_unlock(void)
{
mutex_unlock(&nfnl_mutex);
- if (nfnl->sk_receive_queue.qlen)
- nfnl->sk_data_ready(nfnl, 0);
}
int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n)
@@ -147,9 +135,7 @@ static int nfnetlink_rcv_msg(struct sk_b
ss = nfnetlink_get_subsys(type);
if (!ss) {
#ifdef CONFIG_KMOD
- /* don't call nfnl_unlock, since it would reenter
- * with further packet processing */
- __nfnl_unlock();
+ nfnl_unlock();
request_module("nfnetlink-subsys-%d", NFNL_SUBSYS_ID(type));
nfnl_lock();
ss = nfnetlink_get_subsys(type);
@@ -188,10 +174,9 @@ static void nfnetlink_rcv(struct sock *s
unsigned int qlen = 0;
do {
- if (nfnl_trylock())
- return;
+ nfnl_lock();
qlen = netlink_run_queue(sk, qlen, nfnetlink_rcv_msg);
- __nfnl_unlock();
+ nfnl_unlock();
} while (qlen);
}
--- ./net/netlink/genetlink.c.nlk3 2007-08-26 19:30:38.000000000 +0400
+++ ./net/netlink/genetlink.c 2007-10-01 16:05:29.000000000 +0400
@@ -22,22 +22,14 @@ struct sock *genl_sock = NULL;
static DEFINE_MUTEX(genl_mutex); /* serialization of message processing */
-static void genl_lock(void)
+static inline void genl_lock(void)
{
mutex_lock(&genl_mutex);
}
-static int genl_trylock(void)
-{
- return !mutex_trylock(&genl_mutex);
-}
-
-static void genl_unlock(void)
+static inline void genl_unlock(void)
{
mutex_unlock(&genl_mutex);
-
- if (genl_sock && genl_sock->sk_receive_queue.qlen)
- genl_sock->sk_data_ready(genl_sock, 0);
}
#define GENL_FAM_TAB_SIZE 16
@@ -483,8 +475,7 @@ static void genl_rcv(struct sock *sk, in
unsigned int qlen = 0;
do {
- if (genl_trylock())
- return;
+ genl_lock();
qlen = netlink_run_queue(sk, qlen, genl_rcv_msg);
genl_unlock();
} while (qlen && genl_sock && genl_sock->sk_receive_queue.qlen);
^ permalink raw reply
* Re: [RFC/PATCH 0/3] UDP memory usage accounting
From: Satoshi OSHIMA @ 2007-10-01 13:57 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, johnpol, netdev, haoki, yoshfuji, yumiko.sugita.yf
In-Reply-To: <20070929055515.GB1192@gondor.apana.org.au>
> On Fri, Sep 28, 2007 at 09:47:37PM -0700, David Miller wrote:
>> There are two things we (might) need to guard against, one local and
>> one remote.
>
> Right I was focusing on the local threat.
>
>> If you do a per-user limit, apache would basically just stop at that
>> redzone point. In some sense making the attack more effective because
>> then it's trivial to shut down an entire web server this way.
>
> Having a per-user limit doesn't necessarily mean that we have
> to apply the limit differently to how we apply the system-wide
> limits. We could keep exactly the same code as we have now but
> check against a per-user limit instead of a system-wide one.
>
> In other words your apache scenario will continue to work as is
> even with a per-user limit.
I'm afraid that per-user limit won't work for system administrator,
because he can't know who is the rogue user in advance (before
such attack is made). And once the attack is made, system will
not responce because of the lack of memory for slab.
So if he only has per-user limit, he need to split the memory
budget for UDP to each user. The limit per user will be very
small if number of users in the system is large.
> Now where it does become useful is when we have a rogue local
> user. As it is that user can chew up all of the budgeted TCP
> memory by simply not calling recvmsg. As I've stated in the
> other email, the existing rlimits don't help because they're
> per-process rather than per-user.
>
> BTW, this is not fatal for TCP because TCP provides a minimum
> amount of memory for each socket even when we are over the
> limit. However, if we this was implemented for UDP without
> a minimum guarantee then it'd be quite useless.
Hmm, I didn't realize that. Thank you for your good
suggestion. I will think of it.
>> I see no valid argument against doing something similar for sockets.
>> Such a register_shrinker() handler for TCP could, for example, look
>> for TCP flows which haven't made forward progress in more than a
>> certain amount of time and attempt to trim SKB memory from them.
>
> Yes I agree this would be quite useful for sending. However, it'll
> be tough to shrink skbs that we've already acked for but the app
> for some reason has decided to leave in the socket by not calling
> recvmsg.
>
>> UDP and other datagram sockets are troublesome because the memory
>> gets wholly tied up immediately during the send call and it's not
>> easy to liberate anything. The nice part about datagram sockets,
>> however, is that they make forward progress quickly and their
>> memory is liberated as soon as the device transmits the packet.
>> They don't have to wait for ACKs, windows openning up, or anything
>> like that to happen.
>
> Agreed. Also the recvmsg case I've described above is much
> simpler for UDP as we can just go through all the sockets and
> free skbs at random :)
>
>> To be honest I don't even think UDP is much of a real problem for this
>> reason.
>
> It's not a hard problem but we do need to have some code for it.
I believe so. Currently, a nasty user can easily stop the system
without root privilege. This may not be a serious problem, but
this is the problem to be fixed.
^ permalink raw reply
* Re: [RFC/PATCH 0/3] UDP memory usage accounting
From: Satoshi OSHIMA @ 2007-10-01 13:56 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, johnpol, netdev, haoki, yoshfuji, yumiko.sugita.yf,
Evgeniy Polyakov
In-Reply-To: <20070929052008.GA1192@gondor.apana.org.au>
Herbert Xu wrote:
> On Fri, Sep 28, 2007 at 09:51:59PM -0700, David Miller wrote:
>> There is a per-socket send buffer limit, and there is a per-user open
>> file descriptor limit. Multiply the two to determine how much system
>> memory the user can consume using sockets.
>
> We do have these limits but they're per-process, not per-user.
> Unless you lock down the number of processes each user can have
> to no more than a handful then this is basically useless.
>
> For example, let's say each socket can lock down 64K of kernel
> memory (which is quite easy to do BTW, just open a TCP/UDP socket,
> send data to it from another socket but keep the data in the
> socket by not calling recvmsg), and that each process can have
> 1024 file descriptors (the default), then each process can pin
>
> 64K x 1024 = 64M
>
> of memory. So if the user can have 10 processes, then that's
> 640M of kernel memory that can be pinned down. Usually the
> process limit is at least 10 times higher.
Thank you very mush for your comment.
What you pointed out is my motivation to make this patch.
I think that per-process limits won't help to solve this
problem.
^ permalink raw reply
* Re: [PATCH 10/10] Use nested compat attributes to pass parameters.
From: Patrick McHardy @ 2007-10-01 13:54 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <11910199782593-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> This fixes the ambiguity between, for example:
> tc qdisc change ... perturb 0
> tc qdisc change ...
>
> Without this patch, there is no way for SFQ to differentiate between
> a parameter specified to be 0 and a parameter that was omitted.
>
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index 170fd37..36197f6 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -428,25 +428,31 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
> * the previous values (sfq_change). So, overwrite the parameters as
> * specified. */
> if (opt) {
> - struct tc_sfq_qopt *ctl = RTA_DATA(opt);
> -
> - if (opt->rta_len < RTA_LENGTH(sizeof(*ctl)))
> - return -EINVAL;
> -
> - if (ctl->quantum)
> - q->quantum = ctl->quantum;
> - if (ctl->perturb_period)
> - q->perturb_period = ctl->perturb_period;
> - if (ctl->divisor)
> - q->hash_divisor = ctl->divisor;
> - if (ctl->flows)
> - q->depth = ctl->flows;
> - if (ctl->limit)
> - q->limit = ctl->limit;
> -
> + struct tc_sfq_qopt *ctl;
> + struct rtattr *tb[TCA_SFQ_MAX];
> +
> + if (rtattr_parse_nested_compat(tb, TCA_SFQ_MAX, opt, ctl,
> + sizeof(*ctl)))
> + goto rtattr_failure;
> +
> +#define GET_PARAM(dst, nest, compat) do { \
> + struct rtattr *rta = tb[(nest) - 1]; \
> + if (rta) \
> + (dst) = RTA_GET_U32(rta); \
> + else if ((compat)) \
> + (dst) = (compat); \
> +} while (0)
An inline function and a comment why this is done would increase
readability.
> +
> + GET_PARAM(q->quantum, TCA_SFQ_QUANTUM, ctl->quantum);
> + GET_PARAM(q->perturb_period, TCA_SFQ_PERTURB,
> + ctl->perturb_period);
> + GET_PARAM(q->hash_divisor, TCA_SFQ_DIVISOR, ctl->divisor);
> + GET_PARAM(q->depth, TCA_SFQ_FLOWS, ctl->flows);
> + GET_PARAM(q->limit, TCA_SFQ_LIMIT, ctl->limit);
> +
> if (q->perturb_period > SFQ_MAX_PERTURB ||
> q->depth > SFQ_MAX_DEPTH)
> - return -EINVAL;
> + goto rtattr_failure;
> }
> q->limit = min_t(u32, q->limit, q->depth - 2);
> q->tail = q->depth;
> @@ -482,6 +488,8 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
> for (i=0; i < q->depth; i++)
> sfq_link(q, i);
> return 0;
> +rtattr_failure:
> + return -EINVAL;
> err_case:
> sfq_q_destroy(q);
> return -ENOBUFS;
> @@ -559,17 +567,26 @@ static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
> {
> struct sfq_sched_data *q = qdisc_priv(sch);
> unsigned char *b = skb_tail_pointer(skb);
> + struct rtattr *nest;
> struct tc_sfq_qopt opt;
>
> opt.quantum = q->quantum;
> opt.perturb_period = q->perturb_period;
> -
> opt.limit = q->limit;
> opt.divisor = q->hash_divisor;
> opt.flows = q->depth;
>
> + nest = RTA_NEST_COMPAT(skb, TCA_OPTIONS, sizeof(opt), &opt);
> +
> + RTA_PUT_U32(skb, TCA_SFQ_QUANTUM, q->quantum);
> + RTA_PUT_U32(skb, TCA_SFQ_PERTURB, q->perturb_period);
> + RTA_PUT_U32(skb, TCA_SFQ_LIMIT, q->limit);
> + RTA_PUT_U32(skb, TCA_SFQ_DIVISOR, q->hash_divisor);
> + RTA_PUT_U32(skb, TCA_SFQ_FLOWS, q->depth);
> RTA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
This is wrong, RTA_NEST_COMPAT already dumps the structure.
>
> + RTA_NEST_COMPAT_END(skb, nest);
> +
> return skb->len;
>
> rtattr_failure:
^ permalink raw reply
* Re: [PATCH 1/1][TCP]: break missing at end of switch statement
From: YOSHIFUJI Hideaki @ 2007-10-01 13:20 UTC (permalink / raw)
To: gerrit; +Cc: netdev, yoshfuji
In-Reply-To: <200710011332.43657@strip-the-willow>
In article <200710011332.43657@strip-the-willow> (at Mon, 1 Oct 2007 13:32:43 +0100), Gerrit Renker <gerrit@erg.abdn.ac.uk> says:
> [TCP]: break missing at end of switch statement
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3129,6 +3129,7 @@ static void tcp_reset(struct sock *sk)
> return;
> default:
> sk->sk_err = ECONNRESET;
> + break;
> }
>
> if (!sock_flag(sk, SOCK_DEAD))
NAK; it is not required at all.
--yoshfuji
^ permalink raw reply
* Re: [RFC/PATCH 3/3] UDP memory usage accounting (take 2): measurement
From: Satoshi OSHIMA @ 2007-10-01 13:52 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Andi Kleen, netdev, ?? ??, Yumiko SUGITA, "??@RedHat",
David Miller, Herbert Xu
In-Reply-To: <20070928143710.GA16747@2ka.mipt.ru>
Evgeniy Polyakov wrote:
> On Fri, Sep 28, 2007 at 10:41:31PM +0900, Satoshi OSHIMA
(satoshi.oshima.fk@hitachi.com) wrote:
>> This patch introduces memory usage measurement for UDP.
>>
>> These 3 points were updated.
>>
>> - UDP specific codes in IP layer were removed.
>>
>> - atomic_sub() in a loop was removed
>>
>> - accounting during socket destruction
>
> Another approach is to account only at the highest UDP layer and having
> datagram skb destructor just like it is done in TCP, but this approach
> is also resonable.
This patch set try to introduce a memory accounting by the page
because TCP does. And ip_append_data() merges payloads to a sk_buff
if previous sk_buff has enough space. The problem is that
udp_append_data() doesn't recognize whether this merge happens or not.
If the accounting must be in UDP layer, we need to change
the interface of ip_append_data() to know this merge happens.
Once the interface is changed, we have to maintain other
protocol stacks to keep up with the change.
But I didn't want to do it to keep this patch set small
in the first step.
> I already told that patches 1 and 3 have broken indent, please fix that.
Oops! I will fix that.
> A hint: when you are about to submit something network related for
inclusion,
> and strongly believes it is ready, it can be a not that bad idea to add
> David Miller <davem@davemloft.net> to copy list, he can complain about
> backlog and so on, but will read you mail twice :) but do not tell anyone.
Thank you for your advice. I will do that!
Satoshi Oshima
^ permalink raw reply
* Re: [PATCH 09/10] Change perturb_period to unsigned.
From: Patrick McHardy @ 2007-10-01 13:45 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <11910199784097-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> perturb_period is currently a signed integer, but I can't see any good
> reason why this is so--a negative perturbation period will add a timer
> that expires in the past, causing constant perturbation, which makes
> hashing useless.
>
> if (q->perturb_period) {
> q->perturb_timer.expires = jiffies + q->perturb_period;
> add_timer(&q->perturb_timer);
> }
>
> Strictly speaking, this will break binary compatibility with older
> versions of tc, but that ought not to be a problem because (a) there's
> no valid use for a negative perturb_period, and (b) negative values
> will be seen as high values (> INT_MAX), which don't work anyway.
>
> If perturb_period is too large, (perturb_period * HZ) will overflow the
> size of an unsigned int and wrap around. So, check for thet and reject
> values that are too high.
Sounds reasonable.
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -74,6 +74,9 @@
> typedef unsigned int sfq_index;
> #define SFQ_MAX_DEPTH (UINT_MAX / 2 - 1)
>
> +/* We don't want perturb_period * HZ to overflow an unsigned int. */
> +#define SFQ_MAX_PERTURB (UINT_MAX / HZ)
jiffies are unsigned long.
^ permalink raw reply
* Re: [PATCH 06/10] Make qdisc changeable.
From: Patrick McHardy @ 2007-10-01 13:43 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <11910199772562-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> Re-implement sfq_change() and enable Qdisc_opts.change so "tc qdisc
> change" will work.
>
> +static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
> +{
> + ...
> +
> + /* finish up */
> + if (q->perturb_period) {
> + q->perturb_timer.expires = jiffies + q->perturb_period;
> + add_timer(&q->perturb_timer);
> + } else {
> + q->perturbation = 0;
Seems counter-productive to explicitly set it to zero since
it was still used during tranfering the packets with the
old value. So I'd suggest to remove this or alternatively
set it to the final value *before* transfering the packets.
^ permalink raw reply
* Re: [PATCH 1/1][TCP]: break missing at end of switch statement
From: Gerrit Renker @ 2007-10-01 13:39 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev, yoshfuji
In-Reply-To: <20071001.222059.11266966.yoshfuji@st-paulia.net>
Quoting YOSHIFUJI Hideaki:
|
| > [TCP]: break missing at end of switch statement
| >
| > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
| > ---
| > --- a/net/ipv4/tcp_input.c
| > +++ b/net/ipv4/tcp_input.c
| > @@ -3129,6 +3129,7 @@ static void tcp_reset(struct sock *sk)
| > return;
| > default:
| > sk->sk_err = ECONNRESET;
| > + break;
| > }
| >
| > if (!sock_flag(sk, SOCK_DEAD))
|
| NAK; it is not required at all.
|
| --yoshfuji
|
If it were true what you are saying then the statement
`sk->sk_err = ECONNRESET;'
can go as well since it will always be overridden.
^ permalink raw reply
* Re: [PATCH 05/10] Add divisor.
From: Patrick McHardy @ 2007-10-01 13:38 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <11910199772834-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> Make hash divisor user-configurable.
>
> @@ -120,7 +121,7 @@ static __inline__ unsigned sfq_fold_hash(struct sfq_sched_data *q, u32 h, u32 h1
> /* Have we any rotation primitives? If not, WHY? */
> h ^= (h1<<pert) ^ (h1>>(0x1F - pert));
> h ^= h>>10;
> - return h & 0x3FF;
> + return h & (q->hash_divisor-1);
This assumes that hash_divisor is a power of two, but this is
not enforced anywhere.
^ permalink raw reply
* Re: [PATCH 02/10] Preparatory refactoring part 2.
From: Patrick McHardy @ 2007-10-01 13:33 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <11910199773306-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> The sfq_destroy() --> sfq_q_destroy() change looks pointless here,
> but it's cleaner to split now and add code to sfq_q_destroy() in a
> later patch.
>
> +static void sfq_destroy(struct Qdisc *sch)
> +{
> + struct sfq_sched_data *q = qdisc_priv(sch);
> + sfq_q_destroy(q);
> +}
It does look pointless, after applying all patches sfq_destroy still
remains a simply wrapper around sfq_q_destroy.
^ permalink raw reply
* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-10-01 13:30 UTC (permalink / raw)
To: Bill Fink
Cc: David Miller, krkumar2, johnpol, herbert, kaber, shemminger,
jagana, Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
tgraf, randy.dunlap, sri
In-Reply-To: <20071001001135.75d2b984.billfink@mindspring.com>
On Mon, 2007-01-10 at 00:11 -0400, Bill Fink wrote:
> Have you done performance comparisons for the case of using 9000-byte
> jumbo frames?
I havent, but will try if any of the gige cards i have support it.
As a side note: I have not seen any useful gains or losses as the packet
size approaches even 1500B MTU. For example, post about 256B neither the
batching nor the non-batching give much difference in either throughput
or cpu use. Below 256B, theres a noticeable gain for batching.
Note, in the cases of my tests all 4 CPUs are in full-throttle UDP and
so the occupancy of both the qdisc queue(s) and ethernet ring is
constantly high. For example at 512B, the app is 80% idle on all 4 CPUs
and we are hitting in the range of wire speed. We are at 90% idle at
1024B. This is the case with or without batching. So my suspicion is
that with that trend a 9000B packet will just follow the same pattern.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 03/10] Move two functions.
From: Patrick McHardy @ 2007-10-01 13:27 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <1191019977559-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> Move sfq_q_destroy() to above sfq_q_init() so that it can be used
> by an error case in a later patch.
>
> Move sfq_destroy() as well, for clarity.
This patch looks pointless, just put them where you need them
in the patch introducing them.
^ permalink raw reply
* Re: 2.6.21 -> 2.6.22 & 2.6.23-rc8 performance regression
From: Denys @ 2007-10-01 13:26 UTC (permalink / raw)
To: linux-kernel, netdev
In-Reply-To: <4700ADB7.7050102@cosmosbay.com>
Resend for maillists (was discareded cause of encoding issues as SPAM).
Everything looks fine, for sure. Confirmed on second server.
On Mon, 01 Oct 2007 10:20:07 +0200, Eric Dumazet wrote
> > Well, i can play a bit more on "live" servers. I have now hot-swap server with
> > full gentoo, where i can rebuild any kernel you want, with any applied patch.
> > But it looks more like not overhead, load becoming high too "spiky", and it is
> > not just permantenly higher. Also it is not normal that all system becoming
> > unresposive (for example ping 127.0.0.1 becoming 300ms for period, when usage
> > softirq jumps to 100%).
> >
> >
> Could you try a pristine 2.6.22.9 and some patch in
> secure_tcp_sequence_number() like :
>
> --- drivers/char/random.c.orig 2007-10-01 10:18:42.000000000 +0200
> +++ drivers/char/random.c 2007-10-01 10:19:58.000000000 +0200
> @@ -1554,7 +1554,7 @@
> * That's funny, Linux has one built in! Use it!
> * (Networks are faster now - should this be increased?)
> */
> - seq += ktime_get_real().tv64;
> + seq += ktime_get_real().tv64 / 1000;
> #if 0
> printk("init_seq(%lx, %lx, %d, %d) = %d\n",
> saddr, daddr, sport, dport, seq);
>
> Thank you
>
> > On Mon, 01 Oct 2007 00:12:59 -0700 (PDT), David Miller wrote
> >
> >> From: Eric Dumazet <dada1@cosmosbay.com>
> >> Date: Mon, 01 Oct 2007 07:59:12 +0200
> >>
> >>
> >>> No problem here on bigger servers, so I CC David Miller and netdev
> >>> on this one. AFAIK do_gettimeofday() and ktime_get_real() should
> >>> use the same underlying hardware functions on PC and no performance
> >>> problem should happen here.
> >>>
> >> One thing that jumps out at me is that on 32-bit (and to a certain
> >> extent on 64-bit) there is a lot of stack accesses and missed
> >> optimizations because all of the work occurs, and gets expanded,
> >> inside of ktime_get_real().
> >>
> >> The timespec_to_ktime() inside of there constructs the ktime_t return
> >> value on the stack, then returns that as an aggregate to the caller.
> >>
> >> That cannot be without some cost.
> >>
> >> ktime_get_real() is definitely a candidate for inlining especially in
> >> these kinds of cases where we'll happily get computations in local
> >> registers instead of all of this on-stack nonsense. And in several
> >> cases (if the caller only needs the tv_sec value, for example)
> >> computations can be elided entirely.
> >>
> >> It would be constructive to experiment and see if this is in fact
> >> part of the problem.
> >>
> >
> >
> > --
> > Denys Fedoryshchenko
> > Technical Manager
> > Virtual ISP S.A.L.
> >
> >
> >
--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.
^ permalink raw reply
* Re: [PATCH 01/10] Preparatory refactoring part 1.
From: Patrick McHardy @ 2007-10-01 13:25 UTC (permalink / raw)
To: Corey Hickey; +Cc: netdev
In-Reply-To: <11910199771607-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> Make a new function sfq_q_enqueue() that operates directly on the
> queue data. This will be useful for implementing sfq_change() in
> a later patch. A pleasant side-effect is reducing most of the
> duplicate code in sfq_enqueue() and sfq_requeue().
>
> Similarly, make a new function sfq_q_dequeue().
>
> Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
> ---
> net/sched/sch_sfq.c | 72 +++++++++++++++++++++++++++------------------------
> 1 files changed, 38 insertions(+), 34 deletions(-)
>
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index 3a23e30..57485ef 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
>
The sfq_q_enqueue part looks fine.
>
> - sch->qstats.drops++;
A line in the changelog explaining that this was increased twice
would have been nice.
> sfq_drop(sch);
> return NET_XMIT_CN;
> }
>
> -
> -
> -
> -static struct sk_buff *
> -sfq_dequeue(struct Qdisc* sch)
> +static struct
> +sk_buff *sfq_q_dequeue(struct sfq_sched_data *q)
What is this function needed for?
^ permalink raw reply
* [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-10-01 13:21 UTC (permalink / raw)
To: Patrick McHardy
Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
general, mchan, tgraf, jeff, sri, shemminger, David Miller
In-Reply-To: <4700CF14.2010809@trash.net>
On Mon, 2007-01-10 at 12:42 +0200, Patrick McHardy wrote:
> jamal wrote:
> > + while ((skb = __skb_dequeue(skbs)) != NULL)
> > + q->ops->requeue(skb, q);
>
>
> ->requeue queues at the head, so this looks like it would reverse
> the order of the skbs.
Excellent catch! thanks; i will fix.
As a side note: Any batching driver should _never_ have to requeue; if
it does it is buggy. And the non-batching ones if they ever requeue will
be a single packet, so not much reordering.
Thanks again Patrick.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 1/1][TCP]: break missing at end of switch statement
From: Al Viro @ 2007-10-01 13:17 UTC (permalink / raw)
To: Gerrit Renker; +Cc: netdev
In-Reply-To: <200710011402.10377@strip-the-willow>
On Mon, Oct 01, 2007 at 02:02:10PM +0100, Gerrit Renker wrote:
> Quoting Al Viro:
> | On Mon, Oct 01, 2007 at 01:32:43PM +0100, Gerrit Renker wrote:
> | > [TCP]: break missing at end of switch statement
> | >
> | > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> | > ---
> | > --- a/net/ipv4/tcp_input.c
> | > +++ b/net/ipv4/tcp_input.c
> | > @@ -3129,6 +3129,7 @@ static void tcp_reset(struct sock *sk)
> | > return;
> | > default:
> | > sk->sk_err = ECONNRESET;
> | > + break;
> | > }
> |
> | Huh? Why on the Earth would that be a problem?
> |
> |
> Sorry what is your question?
Why the hell is $Subject a problem that warrants any patches whatsoever?
^ permalink raw reply
* Re: [PATCH] make netlink processing routines semi-synchronious (inspired by rtnl)
From: Patrick McHardy @ 2007-10-01 13:10 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: davem, johannes, tgraf, pablo, netdev
In-Reply-To: <20071001124042.GA8878@iris.sw.ru>
Denis V. Lunev wrote:
> The code in netfilter/nfnetlink.c and in ./net/netlink/genetlink.c looks
> like outdated copy/paste from rtnetlink.c. Push them into sync with the
> original.
>
> int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n)
> @@ -149,7 +137,7 @@ static int nfnetlink_rcv_msg(struct sk_b
> #ifdef CONFIG_KMOD
> /* don't call nfnl_unlock, since it would reenter
> * with further packet processing */
> - __nfnl_unlock();
> + nfnl_unlock();
That comment should be updated/deleted. Rest looks good to me.
^ permalink raw reply
* Re: [PATCH 1/1][TCP]: break missing at end of switch statement
From: Gerrit Renker @ 2007-10-01 13:02 UTC (permalink / raw)
To: Al Viro; +Cc: netdev
In-Reply-To: <20071001123430.GZ8181@ftp.linux.org.uk>
Quoting Al Viro:
| On Mon, Oct 01, 2007 at 01:32:43PM +0100, Gerrit Renker wrote:
| > [TCP]: break missing at end of switch statement
| >
| > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
| > ---
| > --- a/net/ipv4/tcp_input.c
| > +++ b/net/ipv4/tcp_input.c
| > @@ -3129,6 +3129,7 @@ static void tcp_reset(struct sock *sk)
| > return;
| > default:
| > sk->sk_err = ECONNRESET;
| > + break;
| > }
|
| Huh? Why on the Earth would that be a problem?
|
|
Sorry what is your question?
^ permalink raw reply
* [PATCH] make netlink processing routines semi-synchronious (inspired by rtnl)
From: Denis V. Lunev @ 2007-10-01 12:40 UTC (permalink / raw)
To: davem; +Cc: johannes, tgraf, pablo, kaber, netdev
The code in netfilter/nfnetlink.c and in ./net/netlink/genetlink.c looks
like outdated copy/paste from rtnetlink.c. Push them into sync with the
original.
Signed-off-by: Denis V. Lunev <den@openvz.org>
--- ./net/netfilter/nfnetlink.c.nlk3 2007-10-01 09:47:53.000000000 +0400
+++ ./net/netfilter/nfnetlink.c 2007-10-01 16:09:44.000000000 +0400
@@ -44,26 +44,14 @@ static struct sock *nfnl = NULL;
static const struct nfnetlink_subsystem *subsys_table[NFNL_SUBSYS_COUNT];
static DEFINE_MUTEX(nfnl_mutex);
-static void nfnl_lock(void)
+static inline void nfnl_lock(void)
{
mutex_lock(&nfnl_mutex);
}
-static int nfnl_trylock(void)
-{
- return !mutex_trylock(&nfnl_mutex);
-}
-
-static void __nfnl_unlock(void)
-{
- mutex_unlock(&nfnl_mutex);
-}
-
-static void nfnl_unlock(void)
+static inline void nfnl_unlock(void)
{
mutex_unlock(&nfnl_mutex);
- if (nfnl->sk_receive_queue.qlen)
- nfnl->sk_data_ready(nfnl, 0);
}
int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n)
@@ -149,7 +137,7 @@ static int nfnetlink_rcv_msg(struct sk_b
#ifdef CONFIG_KMOD
/* don't call nfnl_unlock, since it would reenter
* with further packet processing */
- __nfnl_unlock();
+ nfnl_unlock();
request_module("nfnetlink-subsys-%d", NFNL_SUBSYS_ID(type));
nfnl_lock();
ss = nfnetlink_get_subsys(type);
@@ -188,10 +176,9 @@ static void nfnetlink_rcv(struct sock *s
unsigned int qlen = 0;
do {
- if (nfnl_trylock())
- return;
+ nfnl_lock();
qlen = netlink_run_queue(sk, qlen, nfnetlink_rcv_msg);
- __nfnl_unlock();
+ nfnl_unlock();
} while (qlen);
}
--- ./net/netlink/genetlink.c.nlk3 2007-08-26 19:30:38.000000000 +0400
+++ ./net/netlink/genetlink.c 2007-10-01 16:05:29.000000000 +0400
@@ -22,22 +22,14 @@ struct sock *genl_sock = NULL;
static DEFINE_MUTEX(genl_mutex); /* serialization of message processing */
-static void genl_lock(void)
+static inline void genl_lock(void)
{
mutex_lock(&genl_mutex);
}
-static int genl_trylock(void)
-{
- return !mutex_trylock(&genl_mutex);
-}
-
-static void genl_unlock(void)
+static inline void genl_unlock(void)
{
mutex_unlock(&genl_mutex);
-
- if (genl_sock && genl_sock->sk_receive_queue.qlen)
- genl_sock->sk_data_ready(genl_sock, 0);
}
#define GENL_FAM_TAB_SIZE 16
@@ -483,8 +475,7 @@ static void genl_rcv(struct sock *sk, in
unsigned int qlen = 0;
do {
- if (genl_trylock())
- return;
+ genl_lock();
qlen = netlink_run_queue(sk, qlen, genl_rcv_msg);
genl_unlock();
} while (qlen && genl_sock && genl_sock->sk_receive_queue.qlen);
^ permalink raw reply
* RE: [ofa-general] [PATCH v3] iw_cxgb3: Support"iwarp-only"interfacestoavoid 4-tuple conflicts.
From: Kanevsky, Arkady @ 2007-10-01 12:34 UTC (permalink / raw)
To: Sean Hefty; +Cc: netdev, rdreier, linux-kernel, general
In-Reply-To: <46FD7380.6050107@ichips.intel.com>
Sean,
Not so simple.
How does client application knows where to connect?
Does this proposal forces applications to choose
the "right" network?
Currently, MPA or ULP and not applications handle it.
Why would we want to change that?
Sean,
I may be beating the dead horse,
but I recall that one of the main selling points
of RDMA that it magical bust to performance with
no changes applications. Just plug it in an viola,
performances goes up and CPU utilization for network
stack goes does. Win-Win.
Thanks,
Arkady Kanevsky email: arkady@netapp.com
Network Appliance Inc. phone: 781-768-5395
1601 Trapelo Rd. - Suite 16. Fax: 781-895-1195
Waltham, MA 02451 central phone: 781-768-5300
> -----Original Message-----
> From: Sean Hefty [mailto:mshefty@ichips.intel.com]
> Sent: Friday, September 28, 2007 5:35 PM
> To: Kanevsky, Arkady
> Cc: netdev@vger.kernel.org; rdreier@cisco.com;
> linux-kernel@vger.kernel.org; general@lists.openfabrics.org
> Subject: Re: [ofa-general] [PATCH v3] iw_cxgb3:
> Support"iwarp-only"interfacestoavoid 4-tuple conflicts.
>
> Kanevsky, Arkady wrote:
> > Exactly,
> > it forces the burden on administrator.
> > And one will be forced to try one mount for iWARP and it
> does not work
> > issue another one TCP or UDP if it fails.
> > Yack!
> >
> > And server will need to listen on different IP address and simple
> > * will not work since it will need to listen in two
> different domains.
>
> The server already has to call listen twice. Once for the
> rdma_cm and once for sockets. Similarly on the client side,
> connect must be made over rdma_cm or sockets. I really don't
> see any impact on the application for this approach.
>
> We just end up separating the port space based on networking
> addresses, rather than keeping the problem at the transport
> level. If you have an alternate approach that will be
> accepted upstream, feel free to post it.
>
> - Sean
> _______________________________________________
> general mailing list
> general@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>
^ 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