* [ofa-general] Re: [PATCH 00/10] Implement batching skb API
From: Krishna Kumar2 @ 2007-07-20 7:47 UTC (permalink / raw)
To: Stephen Hemminger
Cc: jagana, johnpol, herbert, gaagaan, Robert.Olsson, kumarkr,
mcarlson, peter.p.waskiewicz.jr, hadi, kaber, jeff, general,
mchan, tgraf, netdev, sri, davem, rdreier
In-Reply-To: <20070720081848.7cc652fb@oldman>
Stephen Hemminger <shemminger@linux-foundation.org> wrote on 07/20/2007
12:48:48 PM:
> You may see worse performance with batching in the real world when
> running over WAN's. Like TSO, batching will generate back to back packet
> trains that are subject to multi-packet synchronized loss. The problem is
that
> intermediate router queues are often close to full, and when a long
string
> of packets arrives back to back only the first ones will get in, the rest
> get dropped. Normal sends have at least minimal pacing so they are less
> likely do get synchronized drop.
Also forgot to mention in the previous mail, if performance is seen to be
dipping,
batching can be disabled on WAN's by:
echo 0 > /sys/class/net/<dev>/tx_batch_skbs
and use batching on local/site networks in that case.
^ permalink raw reply
* Re: ANNOUNCE: igb: Intel 82575 Gigabit Ethernet driver (PCI-Express)
From: Christoph Hellwig @ 2007-07-20 7:33 UTC (permalink / raw)
To: Kok, Auke
Cc: Jeff Garzik, NetDev, Arjan van de Ven, Ronciak, John,
Mitch Williams
In-Reply-To: <469FF922.2090900@intel.com>
On Thu, Jul 19, 2007 at 04:52:02PM -0700, Kok, Auke wrote:
> Why don't you accept it now and allow us the time to work on this in the
> coming period? The driver works, performs better than all 8257x hardware
> and uses less CPU utilization. That must be good for everyone. Keeping it
> outside of the linux tree is just going to postpone testing the
> non-internal API parts.
Well, these useless abstractions are really annoying and nothing we allow
into other drivers either. So I'd be miuch happier if you stripped them
out quickly. It's not like it's a whole lot of work.
^ permalink raw reply
* Re: [PATCH 00/10] Implement batching skb API
From: Krishna Kumar2 @ 2007-07-20 7:30 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, gaagaan, general, hadi, herbert, jagana, jeff, johnpol,
kaber, kumarkr, mcarlson, mchan, netdev, peter.p.waskiewicz.jr,
rdreier, rick.jones2, Robert.Olsson, sri, tgraf, xma
In-Reply-To: <20070720081848.7cc652fb@oldman>
Stephen Hemminger <shemminger@linux-foundation.org> wrote on 07/20/2007
12:48:48 PM:
> You may see worse performance with batching in the real world when
> running over WAN's. Like TSO, batching will generate back to back packet
> trains that are subject to multi-packet synchronized loss. The problem is
that
> intermediate router queues are often close to full, and when a long
string
> of packets arrives back to back only the first ones will get in, the rest
> get dropped. Normal sends have at least minimal pacing so they are less
> likely do get synchronized drop.
Hi Stephen,
OK. The difference that I could see is that in existing code, the "minimal
pacing" also could lead to (possibly slighly lesser) loss since sends are
quick iterations at the IP layer, while in batching sends are iterative at
the driver layer.
Is it an issue ? Any suggestions ?
Thanks,
- KK
^ permalink raw reply
* Results & Scripts for : "[PATCH 00/10] Implement batching skb API"
From: Krishna Kumar2 @ 2007-07-20 7:20 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, kumarkr, herbert,
gaagaan, mcarlson, xma, rick.jones2, hadi, jeff, general, mchan,
tgraf, netdev, jagana, kaber, sri
[-- Attachment #1: Type: text/plain, Size: 3632 bytes --]
Attached file contains scripts for running tests and parsing results :
(See attached file: scripts.tar)
The result of a 10 run (average) TCP iperf (and 1 netperf for UDP) is
given below.
Thanks,
- KK
-----------------------------------------------------------------------------
Test configuration : Single cross-over cable for MTHCA cards (MT23108) on
two PPC64 systems, both systems are 8-CPU P5 1.5 GHz processors with 8HB memory.
A. TCP results for a 10 run average are as follows (using iperf, could not
run netperf in parallel as it is not synchronized):
First number : Orig BW in KB/s.
Second number : New BW in KB/s.
Third number : Percentage change.
IPoIB was configured with 512 sendq size while default configuration (128)
gave positives for most test cases but more negatives for 512 and 4K buffer
sizes.
Buffer Size 32
TCP Threads:1 : 3126 3169 1.4
TCP Threads:4 : 9739 10889 11.8
TCP Threads:16 : 35383 47218 33.4
TCP Threads:64 : 85147 84196 -1.1
Average : 9.05%
TCP No Delay: Threads:1 : 1990 2976 49.5
TCP No Delay: Threads:4 : 8137 8770 7.7
TCP No Delay: Threads:16 : 31714 37308 17.63
TCP No Delay: Threads:64 : 72830 81892 12.44
Average : 14.19%
Buffer Size 128
TCP Threads:1 : 12674 13339 5.2
TCP Threads:4 : 37889 40816 7.7
TCP Threads:16 : 141342 165935 17.3
TCP Threads:64 : 199813 196283 -1.7
Average : 6.29%
TCP No Delay: Threads:1 : 7732 11272 45.7
TCP No Delay: Threads:4 : 33348 35222 5.6
TCP No Delay: Threads:16 : 120507 143960 19.5
TCP No Delay: Threads:64 : 195459 193875 -0.8
Average : 7.64%
Buffer Size 512
TCP Threads:1 : 42256 55735 31.9
TCP Threads:4 : 161237 161777 0.3
TCP Threads:16 : 227911 231781 1.7
TCP Threads:64 : 229779 223152 -2.9
Average : 1.70%
TCP No Delay: Threads:1 : 30065 42500 41.3
TCP No Delay: Threads:4 : 79076 125848 59.1
TCP No Delay: Threads:16 : 225725 224155 -0.7
TCP No Delay: Threads:64 : 231220 223664 -3.26
Average : 8.84%
Buffer Size 4096
TCP Threads:1 : 119364 135445 13.5
TCP Threads:4 : 261301 256754 -1.7
TCP Threads:16 : 246889 247065 0.07
TCP Threads:64 : 237613 234185 -1.4
Average : 0.95%
TCP No Delay: Threads:1 : 102187 104087 1.9
TCP No Delay: Threads:4 : 204139 243169 19.1
TCP No Delay: Threads:16 : 245529 242519 -1.2
TCP No Delay: Threads:64 : 236826 233382 -1.4
Average : 4.37%
-----------------------------------------------------------------------------
B. Using netperf to run 1 process UDP (1 run, measured with 128 sendq
size, will be re-doing with 512 sendq and for 10 runs average) :
----------------------------------------------------------
Org New Perc
BW Service BW Service BW Service
----------------------------------------------------------
6.40 1277.64 6.50 1272.41 1.56 -.40
24.80 663.01 25.80 318.13 4.03 -52.01
101.80 81.02 101.90 80.63 .09 -.48
395.70 20.77 395.90 20.74 .05 -.14
1172.90 7.00 1156.80 7.10 -1.37 1.42
---------------------------------------------------------------------
[-- Attachment #2: scripts.tar --]
[-- Type: application/octet-stream, Size: 10240 bytes --]
^ permalink raw reply
* AW: Problems receving multicasts with r8169.c driver from linux kernel (2.6.21.5)
From: Reither Robert @ 2007-07-20 7:19 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
Hi Francois ,
i can spare some time now, so i ask:
Would it help, if i code/send u a tool to simulate your multicast packet scenario ?
Buts its possible for me to reproduce the problem with e.g. nc -u 224.1.1.1 10500 <dummy_file (does show me the same bad behaviour).
Or can i help in another way ?
Greetings
Robert Reither
Research & Development
AV Digital Audio- Videotechnik GmbH
Rampengasse 3-5
A-1190 Wien/Austria
Commercial Register No.: FN 201615v
Commercial Court: Handelsgericht Wien
VAT-No.: ATU 50461904
Tel.: +43/1/3680368 43
email: robert.reither@avdigital.at
Visit: <http://www.av-digital.at>
^ permalink raw reply
* [ofa-general] Re: [PATCH 00/10] Implement batching skb API
From: Stephen Hemminger @ 2007-07-20 7:18 UTC (permalink / raw)
To: Krishna Kumar
Cc: johnpol, Robert.Olsson, herbert, gaagaan, kumarkr, rdreier,
peter.p.waskiewicz.jr, mcarlson, jagana, kaber, jeff, general,
mchan, tgraf, netdev, sri, hadi, davem
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
On Fri, 20 Jul 2007 12:01:49 +0530
Krishna Kumar <krkumar2@in.ibm.com> wrote:
> Hi Dave, Roland, everyone,
>
> In May, I had proposed creating an API for sending 'n' skbs to a driver to
> reduce lock overhead, DMA operations, and specific to drivers that have
> completion notification like IPoIB - reduce completion handling ("[RFC] New
> driver API to speed up small packets xmits" @
> http://marc.info/?l=linux-netdev&m=117880900818960&w=2). I had also sent
> initial test results for E1000 which showed minor improvements (but also
> got degradations) @http://marc.info/?l=linux-netdev&m=117887698405795&w=2.
>
> After fine-tuning qdisc and other changes, I modified IPoIB to use this API,
> and now get good gains. Summary for TCP & No Delay: 1 process improves for
> all cases from 1.4% to 49.5%; 4 process has almost identical improvements
> from -1.7% to 59.1%; 16 process case also improves in the range of -1.2% to
> 33.4%; while 64 process doesn't have much improvement (-3.3% to 12.4%). UDP
> was tested with 1 process netperf with small increase in BW but big
> improvement in Service Demand. Netperf latency tests show small drop in
> transaction rate (results in separate attachment).
>
You may see worse performance with batching in the real world when
running over WAN's. Like TSO, batching will generate back to back packet
trains that are subject to multi-packet synchronized loss. The problem is that
intermediate router queues are often close to full, and when a long string
of packets arrives back to back only the first ones will get in, the rest
get dropped. Normal sends have at least minimal pacing so they are less
likely do get synchronized drop.
^ permalink raw reply
* Re: 2.6.22-git: known regressions with patches
From: Cornelia Huck @ 2007-07-20 6:46 UTC (permalink / raw)
To: Michal Piotrowski
Cc: Linus Torvalds, Andrew Morton, LKML, Giacomo Catenazzi,
Jens Axboe, Bartlomiej Zolnierkiewicz, Stefan Richter,
Jan Engelhardt, Jens Axboe, Peter M. Jones, Larry Finger,
S.Çağlar Onur, Meelis Roos, David Howells, netdev,
Randy Dunlap, Trond Myklebust, J. Bruce Fields
In-Reply-To: <469F92B8.5090809@googlemail.com>
On Thu, 19 Jul 2007 18:35:04 +0200,
Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
> FS
>
> Subject : AFS compile broken
> References : http://lkml.org/lkml/2007/7/19/218
> Last known good : ?
> Submitter : Meelis Roos <mroos@linux.ee>
> Caused-By : ?
> Handled-By : Cornelia Huck <cornelia.huck@de.ibm.com>
> Patch : http://lkml.org/lkml/2007/7/19/223
> Status : patch available
>
Multiple people posted this fix, and Andrew's already made it to git
(commit 275afcac9953ece0828972edeab9684cfe1a5ef3).
^ permalink raw reply
* [PATCH 10/10] IPoIB batching in internal xmit/handler routines.
From: Krishna Kumar @ 2007-07-20 6:33 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, xma, rick.jones2, mcarlson, netdev, jagana, general,
mchan, tgraf, jeff, hadi, kaber, Krishna Kumar, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
Add batching support to IPoIB post_send and TX completion handler.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
ipoib_ib.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 187 insertions(+), 46 deletions(-)
diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib_ib.c new/drivers/infiniband/ulp/ipoib/ipoib_ib.c
--- org/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2007-07-20 07:49:28.000000000 +0530
+++ new/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2007-07-20 08:30:22.000000000 +0530
@@ -242,8 +242,9 @@ repost:
static void ipoib_ib_handle_tx_wc(struct net_device *dev, struct ib_wc *wc)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
+ int i = 0, num_completions;
+ int tx_ring_index = priv->tx_tail & (ipoib_sendq_size - 1);
unsigned int wr_id = wc->wr_id;
- struct ipoib_tx_buf *tx_req;
unsigned long flags;
ipoib_dbg_data(priv, "send completion: id %d, status: %d\n",
@@ -255,23 +256,60 @@ static void ipoib_ib_handle_tx_wc(struct
return;
}
- tx_req = &priv->tx_ring[wr_id];
+ num_completions = wr_id - tx_ring_index + 1;
+ if (num_completions <= 0)
+ num_completions += ipoib_sendq_size;
+
+ /*
+ * Handle skbs completion from tx_tail to wr_id. It is possible to
+ * handle WC's from earlier post_sends (possible multiple) in this
+ * iteration as we move from tx_tail to wr_id, since if the last
+ * WR (which is the one which had a completion request) failed to be
+ * sent for any of those earlier request(s), no completion
+ * notification is generated for successful WR's of those earlier
+ * request(s).
+ */
+ while (1) {
+ /*
+ * Could use while (i < num_completions), but it is costly
+ * since in most cases there is 1 completion, and we end up
+ * doing an extra "index = (index+1) & (ipoib_sendq_size-1)"
+ */
+ struct ipoib_tx_buf *tx_req = &priv->tx_ring[tx_ring_index];
+
+ if (likely(tx_req->skb)) {
+ ib_dma_unmap_single(priv->ca, tx_req->mapping,
+ tx_req->skb->len, DMA_TO_DEVICE);
- ib_dma_unmap_single(priv->ca, tx_req->mapping,
- tx_req->skb->len, DMA_TO_DEVICE);
+ ++priv->stats.tx_packets;
+ priv->stats.tx_bytes += tx_req->skb->len;
- ++priv->stats.tx_packets;
- priv->stats.tx_bytes += tx_req->skb->len;
+ dev_kfree_skb_any(tx_req->skb);
+ }
+ /*
+ * else this skb failed synchronously when posted and was
+ * freed immediately.
+ */
+
+ if (++i == num_completions)
+ break;
- dev_kfree_skb_any(tx_req->skb);
+ /* More WC's to handle */
+ tx_ring_index = (tx_ring_index + 1) & (ipoib_sendq_size - 1);
+ }
spin_lock_irqsave(&priv->tx_lock, flags);
- ++priv->tx_tail;
+
+ priv->tx_tail += num_completions;
if (unlikely(test_bit(IPOIB_FLAG_NETIF_STOPPED, &priv->flags)) &&
priv->tx_head - priv->tx_tail <= ipoib_sendq_size >> 1) {
clear_bit(IPOIB_FLAG_NETIF_STOPPED, &priv->flags);
netif_wake_queue(dev);
}
+
+ /* Make more slots available for posts */
+ dev->xmit_slots = ipoib_sendq_size - (priv->tx_head - priv->tx_tail);
+
spin_unlock_irqrestore(&priv->tx_lock, flags);
if (wc->status != IB_WC_SUCCESS &&
@@ -340,78 +378,181 @@ void ipoib_ib_completion(struct ib_cq *c
netif_rx_schedule(dev_ptr);
}
-static inline int post_send(struct ipoib_dev_priv *priv,
- unsigned int wr_id,
- struct ib_ah *address, u32 qpn,
- u64 addr, int len)
+/*
+ * post_send : Post WR(s) to the device.
+ *
+ * num_skbs is the number of WR's, 'start_index' is the first slot in
+ * tx_wr[] or tx_sge[]. Note: 'start_index' is normally zero, unless a
+ * previous post_send returned error and we are trying to send the untried
+ * WR's, in which case start_index will point to the first untried WR.
+ *
+ * We also break the WR link before posting so that the driver knows how
+ * many WR's to process, and this is set back after the post.
+ */
+static inline int post_send(struct ipoib_dev_priv *priv, u32 qpn,
+ int start_index, int num_skbs,
+ struct ib_send_wr **bad_wr)
{
- struct ib_send_wr *bad_wr;
+ int ret;
+ struct ib_send_wr *last_wr, *next_wr;
+
+ last_wr = &priv->tx_wr[start_index + num_skbs - 1];
+
+ /* Set Completion Notification for last WR */
+ last_wr->send_flags = IB_SEND_SIGNALED;
- priv->tx_sge.addr = addr;
- priv->tx_sge.length = len;
+ /* Terminate the last WR */
+ next_wr = last_wr->next;
+ last_wr->next = NULL;
- priv->tx_wr.wr_id = wr_id;
- priv->tx_wr.wr.ud.remote_qpn = qpn;
- priv->tx_wr.wr.ud.ah = address;
+ /* Send all the WR's in one doorbell */
+ ret = ib_post_send(priv->qp, &priv->tx_wr[start_index], bad_wr);
- return ib_post_send(priv->qp, &priv->tx_wr, &bad_wr);
+ /* Restore send_flags & WR chain */
+ last_wr->send_flags = 0;
+ last_wr->next = next_wr;
+
+ return ret;
}
-void ipoib_send(struct net_device *dev, struct sk_buff *skb,
- struct ipoib_ah *address, u32 qpn)
+/*
+ * Map skb & store skb/mapping in tx_req; and details of the WR in tx_wr
+ * to pass to the driver.
+ *
+ * Returns :
+ * - 0 on successful processing of the skb
+ * - 1 if the skb was freed.
+ */
+int ipoib_process_skb(struct net_device *dev, struct sk_buff *skb,
+ struct ipoib_dev_priv *priv, int wr_num,
+ int tx_ring_index, struct ipoib_ah *address, u32 qpn)
{
- struct ipoib_dev_priv *priv = netdev_priv(dev);
- struct ipoib_tx_buf *tx_req;
u64 addr;
+ struct ipoib_tx_buf *tx_req;
if (unlikely(skb->len > priv->mcast_mtu + IPOIB_ENCAP_LEN)) {
- ipoib_warn(priv, "packet len %d (> %d) too long to send, dropping\n",
+ ipoib_warn(priv, "packet len %d (> %d) too long to "
+ "send, dropping\n",
skb->len, priv->mcast_mtu + IPOIB_ENCAP_LEN);
++priv->stats.tx_dropped;
++priv->stats.tx_errors;
ipoib_cm_skb_too_long(dev, skb, priv->mcast_mtu);
- return;
+ return 1;
}
- ipoib_dbg_data(priv, "sending packet, length=%d address=%p qpn=0x%06x\n",
+ ipoib_dbg_data(priv, "sending packet, length=%d address=%p "
+ "qpn=0x%06x\n",
skb->len, address, qpn);
/*
* We put the skb into the tx_ring _before_ we call post_send()
* because it's entirely possible that the completion handler will
- * run before we execute anything after the post_send(). That
+ * run before we execute anything after the post_send(). That
* means we have to make sure everything is properly recorded and
* our state is consistent before we call post_send().
*/
- tx_req = &priv->tx_ring[priv->tx_head & (ipoib_sendq_size - 1)];
- tx_req->skb = skb;
- addr = ib_dma_map_single(priv->ca, skb->data, skb->len,
- DMA_TO_DEVICE);
+ addr = ib_dma_map_single(priv->ca, skb->data, skb->len, DMA_TO_DEVICE);
if (unlikely(ib_dma_mapping_error(priv->ca, addr))) {
++priv->stats.tx_errors;
dev_kfree_skb_any(skb);
- return;
+ return 1;
}
+
+ tx_req = &priv->tx_ring[tx_ring_index];
+ tx_req->skb = skb;
tx_req->mapping = addr;
+ priv->tx_sge[wr_num].addr = addr;
+ priv->tx_sge[wr_num].length = skb->len;
+ priv->tx_wr[wr_num].wr_id = tx_ring_index;
+ priv->tx_wr[wr_num].wr.ud.remote_qpn = qpn;
+ priv->tx_wr[wr_num].wr.ud.ah = address->ah;
- if (unlikely(post_send(priv, priv->tx_head & (ipoib_sendq_size - 1),
- address->ah, qpn, addr, skb->len))) {
- ipoib_warn(priv, "post_send failed\n");
- ++priv->stats.tx_errors;
- ib_dma_unmap_single(priv->ca, addr, skb->len, DMA_TO_DEVICE);
- dev_kfree_skb_any(skb);
- } else {
- dev->trans_start = jiffies;
+ return 0;
+}
- address->last_send = priv->tx_head;
- ++priv->tx_head;
+/*
+ * If an skb is passed to this function, it is the single, unprocessed skb
+ * send case. Otherwise if skb is NULL, it means that all skbs are already
+ * processed and put on the priv->tx_wr,tx_sge,tx_ring, etc.
+ */
+void ipoib_send(struct net_device *dev, struct sk_buff *skb,
+ struct ipoib_ah *address, u32 qpn, int num_skbs)
+{
+ struct ipoib_dev_priv *priv = netdev_priv(dev);
+ int start_index = 0;
+
+ if (skb && ipoib_process_skb(dev, skb, priv, 0, priv->tx_head &
+ (ipoib_sendq_size - 1), address, qpn))
+ return;
+
+ /* Send out all the skb's in one post */
+ while (num_skbs) {
+ struct ib_send_wr *bad_wr;
+
+ if (unlikely((post_send(priv, qpn, start_index, num_skbs,
+ &bad_wr)))) {
+ int done;
+
+ /*
+ * Better error handling can be done here, like free
+ * all untried skbs if err == -ENOMEM. However at this
+ * time, we re-try all the skbs, all of which will
+ * likely fail anyway (unless device finished sending
+ * some out in the meantime). This is not a regression
+ * since the earlier code is not doing this either.
+ */
+ ipoib_warn(priv, "post_send failed\n");
- if (priv->tx_head - priv->tx_tail == ipoib_sendq_size) {
- ipoib_dbg(priv, "TX ring full, stopping kernel net queue\n");
- netif_stop_queue(dev);
- set_bit(IPOIB_FLAG_NETIF_STOPPED, &priv->flags);
+ /* Get #WR's that finished successfully */
+ done = bad_wr - &priv->tx_wr[start_index];
+
+ /* Handle 1 error */
+ priv->stats.tx_errors++;
+ ib_dma_unmap_single(priv->ca,
+ priv->tx_sge[start_index + done].addr,
+ priv->tx_sge[start_index + done].length,
+ DMA_TO_DEVICE);
+
+ /* Handle 'n' successes */
+ if (done) {
+ dev->trans_start = jiffies;
+ address->last_send = priv->tx_head;
+ }
+
+ /* Free failed WR & reset for WC handler to recognize */
+ dev_kfree_skb_any(priv->tx_ring[bad_wr->wr_id].skb);
+ priv->tx_ring[bad_wr->wr_id].skb = NULL;
+
+ /* Move head to first untried WR */
+ priv->tx_head += (done + 1);
+ /* + 1 for WR that was tried & failed */
+
+ /* Get count of skbs that were not tried */
+ num_skbs -= (done + 1);
+
+ /* Get start index for next iteration */
+ start_index += (done + 1);
+ } else {
+ dev->trans_start = jiffies;
+
+ address->last_send = priv->tx_head;
+ priv->tx_head += num_skbs;
+ num_skbs = 0;
}
}
+
+ if (unlikely(priv->tx_head - priv->tx_tail == ipoib_sendq_size)) {
+ /*
+ * Not accurate as some intermediate slots could have been
+ * freed on error, but no harm - only queue stopped earlier.
+ */
+ ipoib_dbg(priv, "TX ring full, stopping kernel net queue\n");
+ netif_stop_queue(dev);
+ set_bit(IPOIB_FLAG_NETIF_STOPPED, &priv->flags);
+ }
+
+ /* Reduce the number of slots for sends */
+ dev->xmit_slots = ipoib_sendq_size - (priv->tx_head - priv->tx_tail);
}
static void __ipoib_reap_ah(struct net_device *dev)
^ permalink raw reply
* [PATCH 04/10] net-sysfs.c changes.
From: Krishna Kumar @ 2007-07-20 6:32 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, xma, rick.jones2, mcarlson, jagana, mchan, general,
netdev, tgraf, jeff, hadi, kaber, Krishna Kumar, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
Support to turn on/off batching from /sys.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
net-sysfs.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 70 insertions(+)
diff -ruNp org/net/core/net-sysfs.c new/net/core/net-sysfs.c
--- org/net/core/net-sysfs.c 2007-07-20 07:49:28.000000000 +0530
+++ new/net/core/net-sysfs.c 2007-07-20 08:34:45.000000000 +0530
@@ -230,6 +230,74 @@ static ssize_t store_weight(struct devic
return netdev_store(dev, attr, buf, len, change_weight);
}
+static ssize_t show_tx_batch_skbs(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct net_device *netdev = to_net_dev(dev);
+
+ return sprintf(buf, fmt_dec, netdev->skb_blist ? 1 : 0);
+}
+
+static int change_tx_batch_skbs(struct net_device *net,
+ unsigned long new_tx_batch_skbs)
+{
+ int ret = 0;
+ struct sk_buff_head *blist;
+
+ if (!(net->features & NETIF_F_BATCH_SKBS) ||
+ (new_tx_batch_skbs && net->tx_queue_len < MIN_QUEUE_LEN_BATCH)) {
+ /*
+ * Driver doesn't support batching SKBS, or the queue len
+ * is insufficient. TODO: Add similar check to disable
+ * batching in change_tx_queue_len() if queue_len becomes
+ * smaller than MIN_QUEUE_LEN_BATCH.
+ */
+ ret = -ENOTSUPP;
+ goto out;
+ }
+
+ /* Handle invalid argument */
+ if (new_tx_batch_skbs < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Check if new value is same as the current */
+ new_tx_batch_skbs = !!new_tx_batch_skbs;
+ if (!!net->skb_blist == new_tx_batch_skbs)
+ goto out;
+
+ if (new_tx_batch_skbs &&
+ (blist = kmalloc(sizeof *blist, GFP_KERNEL)) == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ spin_lock(&net->queue_lock);
+ if (new_tx_batch_skbs) {
+ skb_queue_head_init(blist);
+ net->skb_blist = blist;
+ net->tx_queue_len >>= 1;
+ } else {
+ if (!skb_queue_empty(net->skb_blist))
+ skb_queue_purge(net->skb_blist);
+ kfree(net->skb_blist);
+ net->skb_blist = NULL;
+ net->tx_queue_len <<= 1;
+ }
+ spin_unlock(&net->queue_lock);
+
+out:
+ return ret;
+}
+
+static ssize_t store_tx_batch_skbs(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return netdev_store(dev, attr, buf, len, change_tx_batch_skbs);
+}
+
static struct device_attribute net_class_attributes[] = {
__ATTR(addr_len, S_IRUGO, show_addr_len, NULL),
__ATTR(iflink, S_IRUGO, show_iflink, NULL),
@@ -246,6 +314,8 @@ static struct device_attribute net_class
__ATTR(flags, S_IRUGO | S_IWUSR, show_flags, store_flags),
__ATTR(tx_queue_len, S_IRUGO | S_IWUSR, show_tx_queue_len,
store_tx_queue_len),
+ __ATTR(tx_batch_skbs, S_IRUGO | S_IWUSR, show_tx_batch_skbs,
+ store_tx_batch_skbs),
__ATTR(weight, S_IRUGO | S_IWUSR, show_weight, store_weight),
{}
};
^ permalink raw reply
* [PATCH 09/10] IPoIB batching xmit handler support.
From: Krishna Kumar @ 2007-07-20 6:33 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, rick.jones2, herbert, gaagaan, kumarkr,
peter.p.waskiewicz.jr, mcarlson, kaber, jagana, general, mchan,
tgraf, jeff, sri, hadi, netdev, Krishna Kumar, xma
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
Add a IPoIB batching xmit handler.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
ipoib_main.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 210 insertions(+), 5 deletions(-)
diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib_main.c new/drivers/infiniband/ulp/ipoib/ipoib_main.c
--- org/drivers/infiniband/ulp/ipoib/ipoib_main.c 2007-07-20 07:49:28.000000000 +0530
+++ new/drivers/infiniband/ulp/ipoib/ipoib_main.c 2007-07-20 08:30:22.000000000 +0530
@@ -558,7 +558,8 @@ static void neigh_add_path(struct sk_buf
goto err_drop;
}
} else
- ipoib_send(dev, skb, path->ah, IPOIB_QPN(skb->dst->neighbour->ha));
+ ipoib_send(dev, skb, path->ah,
+ IPOIB_QPN(skb->dst->neighbour->ha), 1);
} else {
neigh->ah = NULL;
@@ -638,7 +639,7 @@ static void unicast_arp_send(struct sk_b
ipoib_dbg(priv, "Send unicast ARP to %04x\n",
be16_to_cpu(path->pathrec.dlid));
- ipoib_send(dev, skb, path->ah, IPOIB_QPN(phdr->hwaddr));
+ ipoib_send(dev, skb, path->ah, IPOIB_QPN(phdr->hwaddr), 1);
} else if ((path->query || !path_rec_start(dev, path)) &&
skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
/* put pseudoheader back on for next time */
@@ -704,7 +705,8 @@ static int ipoib_start_xmit(struct sk_bu
goto out;
}
- ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(skb->dst->neighbour->ha));
+ ipoib_send(dev, skb, neigh->ah,
+ IPOIB_QPN(skb->dst->neighbour->ha), 1);
goto out;
}
@@ -753,6 +755,177 @@ out:
return NETDEV_TX_OK;
}
+#define XMIT_QUEUED_SKBS() \
+ do { \
+ if (num_skbs) { \
+ ipoib_send(dev, NULL, old_neigh->ah, old_qpn, \
+ num_skbs); \
+ num_skbs = 0; \
+ } \
+ } while (0)
+
+/*
+ * TODO: Merge with ipoib_start_xmit to use the same code and have a
+ * transparent wrapper caller to xmit's, etc.
+ */
+static int ipoib_start_xmit_frames(struct net_device *dev)
+{
+ struct ipoib_dev_priv *priv = netdev_priv(dev);
+ struct sk_buff *skb;
+ struct sk_buff_head *blist;
+ int max_skbs, num_skbs = 0, tx_ring_index = -1;
+ u32 qpn, old_qpn = 0;
+ struct ipoib_neigh *neigh, *old_neigh = NULL;
+ unsigned long flags;
+
+ if (unlikely(!spin_trylock_irqsave(&priv->tx_lock, flags)))
+ return NETDEV_TX_LOCKED;
+
+ blist = dev->skb_blist;
+
+ /*
+ * Send atmost xmit_slots skbs. This also prevents the device getting
+ * full as ipoib_send modifies the xmit_slots and we use the same
+ * value to figure how many skbs to send.
+ */
+ max_skbs = dev->xmit_slots;
+
+ while (max_skbs-- > 0 && (skb = __skb_dequeue(blist)) != NULL) {
+ /*
+ * From here on, ipoib_send() cannot stop the queue as it
+ * uses the same initialization as 'max_skbs'. So we can
+ * optimize to not check for queue stopped for every skb.
+ */
+ if (likely(skb->dst && skb->dst->neighbour)) {
+ if (unlikely(!*to_ipoib_neigh(skb->dst->neighbour))) {
+ XMIT_QUEUED_SKBS();
+ ipoib_path_lookup(skb, dev);
+ continue;
+ }
+
+ neigh = *to_ipoib_neigh(skb->dst->neighbour);
+
+ if (ipoib_cm_get(neigh)) {
+ if (ipoib_cm_up(neigh)) {
+ XMIT_QUEUED_SKBS();
+ ipoib_cm_send(dev, skb,
+ ipoib_cm_get(neigh));
+ continue;
+ }
+ } else if (neigh->ah) {
+ if (unlikely(memcmp(&neigh->dgid.raw,
+ skb->dst->neighbour->ha + 4,
+ sizeof(union ib_gid)))) {
+ spin_lock(&priv->lock);
+ /*
+ * It's safe to call ipoib_put_ah()
+ * inside priv->lock here, because we
+ * know that path->ah will always hold
+ * one more reference, so ipoib_put_ah()
+ * will never do more than decrement
+ * the ref count.
+ */
+ ipoib_put_ah(neigh->ah);
+ list_del(&neigh->list);
+ ipoib_neigh_free(dev, neigh);
+ spin_unlock(&priv->lock);
+ XMIT_QUEUED_SKBS();
+ ipoib_path_lookup(skb, dev);
+ continue;
+ }
+
+ qpn = IPOIB_QPN(skb->dst->neighbour->ha);
+ if (neigh != old_neigh || qpn != old_qpn) {
+ /*
+ * Sending to a different destination
+ * from earlier skb's - send all
+ * existing skbs (if any).
+ */
+ if (tx_ring_index == -1) {
+ /*
+ * First time, find where to
+ * store skb.
+ */
+ tx_ring_index = priv->tx_head &
+ (ipoib_sendq_size - 1);
+ } else {
+ /* Some skbs to send */
+ XMIT_QUEUED_SKBS();
+ }
+ old_neigh = neigh;
+ old_qpn = IPOIB_QPN(skb->dst->neighbour->ha);
+ }
+
+ if (ipoib_process_skb(dev, skb, priv, num_skbs,
+ tx_ring_index, neigh->ah,
+ qpn))
+ continue;
+
+ num_skbs++;
+
+ /* Queue'd one skb, get index for next skb */
+ if (max_skbs)
+ tx_ring_index = (tx_ring_index + 1) &
+ (ipoib_sendq_size - 1);
+ continue;
+ }
+
+ if (skb_queue_len(&neigh->queue) <
+ IPOIB_MAX_PATH_REC_QUEUE) {
+ spin_lock(&priv->lock);
+ __skb_queue_tail(&neigh->queue, skb);
+ spin_unlock(&priv->lock);
+ } else {
+ dev_kfree_skb_any(skb);
+ ++priv->stats.tx_dropped;
+ ++max_skbs;
+ }
+ } else {
+ struct ipoib_pseudoheader *phdr =
+ (struct ipoib_pseudoheader *) skb->data;
+ skb_pull(skb, sizeof *phdr);
+
+ if (phdr->hwaddr[4] == 0xff) {
+ /* Add in the P_Key for multicast*/
+ phdr->hwaddr[8] = (priv->pkey >> 8) & 0xff;
+ phdr->hwaddr[9] = priv->pkey & 0xff;
+
+ XMIT_QUEUED_SKBS();
+ ipoib_mcast_send(dev, phdr->hwaddr + 4, skb);
+ } else {
+ /* unicast GID -- should be ARP or RARP reply */
+
+ if ((be16_to_cpup((__be16 *) skb->data) !=
+ ETH_P_ARP) &&
+ (be16_to_cpup((__be16 *) skb->data) !=
+ ETH_P_RARP)) {
+ ipoib_warn(priv, "Unicast, no %s: type %04x, QPN %06x "
+ IPOIB_GID_FMT "\n",
+ skb->dst ? "neigh" : "dst",
+ be16_to_cpup((__be16 *)
+ skb->data),
+ IPOIB_QPN(phdr->hwaddr),
+ IPOIB_GID_RAW_ARG(phdr->hwaddr
+ + 4));
+ dev_kfree_skb_any(skb);
+ ++priv->stats.tx_dropped;
+ ++max_skbs;
+ continue;
+ }
+ XMIT_QUEUED_SKBS();
+ unicast_arp_send(skb, dev, phdr);
+ }
+ }
+ }
+
+ /* Send out last packets (if any) */
+ XMIT_QUEUED_SKBS();
+
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+
+ return skb_queue_empty(blist) ? NETDEV_TX_OK : NETDEV_TX_BUSY;
+}
+
static struct net_device_stats *ipoib_get_stats(struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -898,11 +1071,35 @@ int ipoib_dev_init(struct net_device *de
/* priv->tx_head & tx_tail are already 0 */
- if (ipoib_ib_dev_init(dev, ca, port))
+ /* Allocate tx_sge */
+ priv->tx_sge = kmalloc(ipoib_sendq_size * sizeof *priv->tx_sge,
+ GFP_KERNEL);
+ if (!priv->tx_sge) {
+ printk(KERN_WARNING "%s: failed to allocate TX sge (%d entries)\n",
+ ca->name, ipoib_sendq_size);
goto out_tx_ring_cleanup;
+ }
+
+ /* Allocate tx_wr */
+ priv->tx_wr = kmalloc(ipoib_sendq_size * sizeof *priv->tx_wr,
+ GFP_KERNEL);
+ if (!priv->tx_wr) {
+ printk(KERN_WARNING "%s: failed to allocate TX wr (%d entries)\n",
+ ca->name, ipoib_sendq_size);
+ goto out_tx_sge_cleanup;
+ }
+
+ if (ipoib_ib_dev_init(dev, ca, port))
+ goto out_tx_wr_cleanup;
return 0;
+out_tx_wr_cleanup:
+ kfree(priv->tx_wr);
+
+out_tx_sge_cleanup:
+ kfree(priv->tx_sge);
+
out_tx_ring_cleanup:
kfree(priv->tx_ring);
@@ -930,9 +1127,13 @@ void ipoib_dev_cleanup(struct net_device
kfree(priv->rx_ring);
kfree(priv->tx_ring);
+ kfree(priv->tx_sge);
+ kfree(priv->tx_wr);
priv->rx_ring = NULL;
priv->tx_ring = NULL;
+ priv->tx_sge = NULL;
+ priv->tx_wr = NULL;
}
static void ipoib_setup(struct net_device *dev)
@@ -943,6 +1144,7 @@ static void ipoib_setup(struct net_devic
dev->stop = ipoib_stop;
dev->change_mtu = ipoib_change_mtu;
dev->hard_start_xmit = ipoib_start_xmit;
+ dev->hard_start_xmit_batch = ipoib_start_xmit_frames;
dev->get_stats = ipoib_get_stats;
dev->tx_timeout = ipoib_timeout;
dev->hard_header = ipoib_hard_header;
@@ -963,7 +1165,10 @@ static void ipoib_setup(struct net_devic
dev->addr_len = INFINIBAND_ALEN;
dev->type = ARPHRD_INFINIBAND;
dev->tx_queue_len = ipoib_sendq_size * 2;
- dev->features = NETIF_F_VLAN_CHALLENGED | NETIF_F_LLTX;
+ dev->features = NETIF_F_VLAN_CHALLENGED | NETIF_F_LLTX |
+ NETIF_F_BATCH_SKBS;
+
+ dev->xmit_slots = ipoib_sendq_size;
/* MTU will be reset when mcast join happens */
dev->mtu = IPOIB_PACKET_SIZE - IPOIB_ENCAP_LEN;
^ permalink raw reply
* [PATCH 02/10] Networking include file changes.
From: Krishna Kumar @ 2007-07-20 6:32 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, xma, rick.jones2, mcarlson, netdev, jagana, general,
mchan, tgraf, jeff, hadi, kaber, Krishna Kumar, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
Networking include file changes for batching.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
linux/netdevice.h | 10 ++++++++++
net/pkt_sched.h | 6 +++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff -ruNp org/include/linux/netdevice.h new/include/linux/netdevice.h
--- org/include/linux/netdevice.h 2007-07-20 07:49:28.000000000 +0530
+++ new/include/linux/netdevice.h 2007-07-20 08:30:55.000000000 +0530
@@ -264,6 +264,8 @@ enum netdev_state_t
__LINK_STATE_QDISC_RUNNING,
};
+/* Minimum length of device hardware queue for batching to work */
+#define MIN_QUEUE_LEN_BATCH 16
/*
* This structure holds at boot time configured netdevice settings. They
@@ -340,6 +342,7 @@ struct net_device
#define NETIF_F_VLAN_CHALLENGED 1024 /* Device cannot handle VLAN packets */
#define NETIF_F_GSO 2048 /* Enable software GSO. */
#define NETIF_F_LLTX 4096 /* LockLess TX */
+#define NETIF_F_BATCH_SKBS 8192 /* Driver supports batch skbs API */
#define NETIF_F_MULTI_QUEUE 16384 /* Has multiple TX/RX queues */
/* Segmentation offload features */
@@ -452,6 +455,8 @@ struct net_device
struct Qdisc *qdisc_sleeping;
struct list_head qdisc_list;
unsigned long tx_queue_len; /* Max frames per queue allowed */
+ unsigned long xmit_slots; /* Device free slots */
+ struct sk_buff_head *skb_blist; /* List of batch skbs */
/* Partially transmitted GSO packet. */
struct sk_buff *gso_skb;
@@ -472,6 +477,9 @@ struct net_device
void *priv; /* pointer to private data */
int (*hard_start_xmit) (struct sk_buff *skb,
struct net_device *dev);
+ int (*hard_start_xmit_batch) (struct net_device
+ *dev);
+
/* These may be needed for future network-power-down code. */
unsigned long trans_start; /* Time (in jiffies) of last Tx */
@@ -832,6 +840,8 @@ extern int dev_set_mac_address(struct n
struct sockaddr *);
extern int dev_hard_start_xmit(struct sk_buff *skb,
struct net_device *dev);
+extern int dev_add_skb_to_blist(struct sk_buff *skb,
+ struct net_device *dev);
extern void dev_init(void);
diff -ruNp org/include/net/pkt_sched.h new/include/net/pkt_sched.h
--- org/include/net/pkt_sched.h 2007-07-20 07:49:28.000000000 +0530
+++ new/include/net/pkt_sched.h 2007-07-20 08:30:22.000000000 +0530
@@ -80,13 +80,13 @@ extern struct qdisc_rate_table *qdisc_ge
struct rtattr *tab);
extern void qdisc_put_rtab(struct qdisc_rate_table *tab);
-extern void __qdisc_run(struct net_device *dev);
+extern void __qdisc_run(struct net_device *dev, struct sk_buff_head *blist);
-static inline void qdisc_run(struct net_device *dev)
+static inline void qdisc_run(struct net_device *dev, struct sk_buff_head *blist)
{
if (!netif_queue_stopped(dev) &&
!test_and_set_bit(__LINK_STATE_QDISC_RUNNING, &dev->state))
- __qdisc_run(dev);
+ __qdisc_run(dev, blist);
}
extern int tc_classify_compat(struct sk_buff *skb, struct tcf_proto *tp,
^ permalink raw reply
* [ofa-general] [PATCH 08/10] IPoIB multicast/CM changes.
From: Krishna Kumar @ 2007-07-20 6:33 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, herbert, gaagaan, kumarkr,
peter.p.waskiewicz.jr, mcarlson, kaber, jagana, general, mchan,
tgraf, jeff, sri, hadi, netdev
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
IPoIB Multicast and CM changes for batching support.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
ipoib_cm.c | 13 +++++++++----
ipoib_multicast.c | 4 ++--
2 files changed, 11 insertions(+), 6 deletions(-)
diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib_cm.c new/drivers/infiniband/ulp/ipoib/ipoib_cm.c
--- org/drivers/infiniband/ulp/ipoib/ipoib_cm.c 2007-07-20 07:49:28.000000000 +0530
+++ new/drivers/infiniband/ulp/ipoib/ipoib_cm.c 2007-07-20 08:30:22.000000000 +0530
@@ -493,14 +493,19 @@ static inline int post_send(struct ipoib
unsigned int wr_id,
u64 addr, int len)
{
+ int ret;
struct ib_send_wr *bad_wr;
- priv->tx_sge.addr = addr;
- priv->tx_sge.length = len;
+ priv->tx_sge[0].addr = addr;
+ priv->tx_sge[0].length = len;
+
+ priv->tx_wr[0].wr_id = wr_id;
- priv->tx_wr.wr_id = wr_id;
+ priv->tx_wr[0].next = NULL;
+ ret = ib_post_send(tx->qp, priv->tx_wr, &bad_wr);
+ priv->tx_wr[0].next = &priv->tx_wr[1];
- return ib_post_send(tx->qp, &priv->tx_wr, &bad_wr);
+ return ret;
}
void ipoib_cm_send(struct net_device *dev, struct sk_buff *skb, struct ipoib_cm_tx *tx)
diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib_multicast.c new/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
--- org/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2007-07-20 07:49:28.000000000 +0530
+++ new/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2007-07-20 08:30:22.000000000 +0530
@@ -217,7 +217,7 @@ static int ipoib_mcast_join_finish(struc
if (!memcmp(mcast->mcmember.mgid.raw, priv->dev->broadcast + 4,
sizeof (union ib_gid))) {
priv->qkey = be32_to_cpu(priv->broadcast->mcmember.qkey);
- priv->tx_wr.wr.ud.remote_qkey = priv->qkey;
+ priv->tx_wr[0].wr.ud.remote_qkey = priv->qkey;
}
if (!test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) {
@@ -736,7 +736,7 @@ out:
}
}
- ipoib_send(dev, skb, mcast->ah, IB_MULTICAST_QPN);
+ ipoib_send(dev, skb, mcast->ah, IB_MULTICAST_QPN, 1);
}
unlock:
^ permalink raw reply
* [ofa-general] [PATCH 07/10] IPoIB verb changes.
From: Krishna Kumar @ 2007-07-20 6:33 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, mcarlson, jagana, general, netdev, tgraf, jeff, hadi,
kaber, mchan, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
IPoIB verb changes to support batching.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
ipoib_verbs.c | 23 ++++++++++++++---------
1 files changed, 14 insertions(+), 9 deletions(-)
diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib_verbs.c new/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
--- org/drivers/infiniband/ulp/ipoib/ipoib_verbs.c 2007-07-20 07:49:28.000000000 +0530
+++ new/drivers/infiniband/ulp/ipoib/ipoib_verbs.c 2007-07-20 08:30:22.000000000 +0530
@@ -152,11 +152,11 @@ int ipoib_transport_dev_init(struct net_
.max_send_sge = 1,
.max_recv_sge = 1
},
- .sq_sig_type = IB_SIGNAL_ALL_WR,
+ .sq_sig_type = IB_SIGNAL_REQ_WR, /* 11.2.4.1 */
.qp_type = IB_QPT_UD
};
-
- int ret, size;
+ struct ib_send_wr *next_wr = NULL;
+ int i, ret, size;
priv->pd = ib_alloc_pd(priv->ca);
if (IS_ERR(priv->pd)) {
@@ -197,12 +197,17 @@ int ipoib_transport_dev_init(struct net_
priv->dev->dev_addr[2] = (priv->qp->qp_num >> 8) & 0xff;
priv->dev->dev_addr[3] = (priv->qp->qp_num ) & 0xff;
- priv->tx_sge.lkey = priv->mr->lkey;
-
- priv->tx_wr.opcode = IB_WR_SEND;
- priv->tx_wr.sg_list = &priv->tx_sge;
- priv->tx_wr.num_sge = 1;
- priv->tx_wr.send_flags = IB_SEND_SIGNALED;
+ for (i = ipoib_sendq_size - 1; i >= 0; i--) {
+ priv->tx_sge[i].lkey = priv->mr->lkey;
+ priv->tx_wr[i].opcode = IB_WR_SEND;
+ priv->tx_wr[i].sg_list = &priv->tx_sge[i];
+ priv->tx_wr[i].num_sge = 1;
+ priv->tx_wr[i].send_flags = 0;
+
+ /* Link the list properly for provider to use */
+ priv->tx_wr[i].next = next_wr;
+ next_wr = &priv->tx_wr[i];
+ }
return 0;
^ permalink raw reply
* [ofa-general] [PATCH 06/10] IPoIB header file changes.
From: Krishna Kumar @ 2007-07-20 6:33 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, herbert, gaagaan, kumarkr,
peter.p.waskiewicz.jr, mcarlson, jagana, general, netdev, tgraf,
jeff, sri, hadi, kaber, mchan
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
IPoIB header file changes.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
ipoib.h | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib.h new/drivers/infiniband/ulp/ipoib/ipoib.h
--- org/drivers/infiniband/ulp/ipoib/ipoib.h 2007-07-20 07:49:28.000000000 +0530
+++ new/drivers/infiniband/ulp/ipoib/ipoib.h 2007-07-20 08:30:22.000000000 +0530
@@ -269,8 +269,8 @@ struct ipoib_dev_priv {
struct ipoib_tx_buf *tx_ring;
unsigned tx_head;
unsigned tx_tail;
- struct ib_sge tx_sge;
- struct ib_send_wr tx_wr;
+ struct ib_sge *tx_sge;
+ struct ib_send_wr *tx_wr;
struct ib_wc ibwc[IPOIB_NUM_WC];
@@ -365,8 +365,11 @@ static inline void ipoib_put_ah(struct i
int ipoib_open(struct net_device *dev);
int ipoib_add_pkey_attr(struct net_device *dev);
+int ipoib_process_skb(struct net_device *dev, struct sk_buff *skb,
+ struct ipoib_dev_priv *priv, int snum, int tx_index,
+ struct ipoib_ah *address, u32 qpn);
void ipoib_send(struct net_device *dev, struct sk_buff *skb,
- struct ipoib_ah *address, u32 qpn);
+ struct ipoib_ah *address, u32 qpn, int num_skbs);
void ipoib_reap_ah(struct work_struct *work);
void ipoib_flush_paths(struct net_device *dev);
^ permalink raw reply
* [ofa-general] [PATCH 05/10] sch_generic.c changes.
From: Krishna Kumar @ 2007-07-20 6:32 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, mcarlson, netdev, jagana, general, mchan, tgraf, jeff,
hadi, kaber, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
net/sched/sch_generic.c changes to support batching. Adds a batch
aware function (get_skb) to get skbs to send.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
sch_generic.c | 94 +++++++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 71 insertions(+), 23 deletions(-)
diff -ruNp org/net/sched/sch_generic.c new/net/sched/sch_generic.c
--- org/net/sched/sch_generic.c 2007-07-20 07:49:28.000000000 +0530
+++ new/net/sched/sch_generic.c 2007-07-20 08:30:22.000000000 +0530
@@ -9,6 +9,11 @@
* Authors: Alexey Kuznetsov, <kuznet@ms2.inr.ac.ru>
* Jamal Hadi Salim, <hadi@cyberus.ca> 990601
* - Ingress support
+ *
+ * New functionality:
+ * Krishna Kumar, <krkumar2@in.ibm.com>, July 2007
+ * - Support for sending multiple skbs to devices that support
+ * new api - dev->hard_start_xmit_batch()
*/
#include <linux/bitops.h>
@@ -59,10 +64,12 @@ static inline int qdisc_qlen(struct Qdis
static inline int dev_requeue_skb(struct sk_buff *skb, struct net_device *dev,
struct Qdisc *q)
{
- if (unlikely(skb->next))
- dev->gso_skb = skb;
- else
- q->ops->requeue(skb, q);
+ if (likely(skb)) {
+ if (unlikely(skb->next))
+ dev->gso_skb = skb;
+ else
+ q->ops->requeue(skb, q);
+ }
netif_schedule(dev);
return 0;
@@ -91,18 +98,23 @@ static inline int handle_dev_cpu_collisi
/*
* Same CPU holding the lock. It may be a transient
* configuration error, when hard_start_xmit() recurses. We
- * detect it by checking xmit owner and drop the packet when
- * deadloop is detected. Return OK to try the next skb.
+ * detect it by checking xmit owner and drop skb (or all
+ * skbs in batching case) when deadloop is detected. Return
+ * OK to try the next skb.
*/
- kfree_skb(skb);
+ if (likely(skb))
+ kfree_skb(skb);
+ else if (!skb_queue_empty(dev->skb_blist))
+ skb_queue_purge(dev->skb_blist);
+
if (net_ratelimit())
printk(KERN_WARNING "Dead loop on netdevice %s, "
"fix it urgently!\n", dev->name);
ret = qdisc_qlen(q);
} else {
/*
- * Another cpu is holding lock, requeue & delay xmits for
- * some time.
+ * Another cpu is holding lock. Requeue skb and delay xmits
+ * for some time.
*/
__get_cpu_var(netdev_rx_stat).cpu_collision++;
ret = dev_requeue_skb(skb, dev, q);
@@ -112,6 +124,39 @@ static inline int handle_dev_cpu_collisi
}
/*
+ * Algorithm to get skb(s) is:
+ * - Non batching drivers, or if the batch list is empty and there is 1
+ * skb in the queue - dequeue skb and put it in *skbp to tell the
+ * caller to use the regular API.
+ * - Batching drivers where the batch list already contains atleast one
+ * skb or if there are multiple skbs in the queue: keep dequeue'ing
+ * skb's upto a limit and set *skbp to NULL to tell the caller to use
+ * the new API.
+ *
+ * Returns:
+ * 1 - atleast one skb is to be sent out, *skbp contains skb or NULL
+ * (in case >1 skbs present in blist for batching)
+ * 0 - no skbs to be sent.
+ */
+static inline int get_skb(struct net_device *dev, struct Qdisc *q,
+ struct sk_buff_head *blist,
+ struct sk_buff **skbp)
+{
+ if (likely(!blist) || (!skb_queue_len(blist) && qdisc_qlen(q) <= 1)) {
+ return likely((*skbp = dev_dequeue_skb(dev, q)) != NULL);
+ } else {
+ int max = dev->tx_queue_len - skb_queue_len(blist);
+ struct sk_buff *skb;
+
+ while (max > 0 && (skb = dev_dequeue_skb(dev, q)) != NULL)
+ max -= dev_add_skb_to_blist(skb, dev);
+
+ *skbp = NULL;
+ return 1; /* we have atleast one skb in blist */
+ }
+}
+
+/*
* NOTE: Called under dev->queue_lock with locally disabled BH.
*
* __LINK_STATE_QDISC_RUNNING guarantees only one CPU can process this
@@ -130,27 +175,28 @@ static inline int handle_dev_cpu_collisi
* >0 - queue is not empty.
*
*/
-static inline int qdisc_restart(struct net_device *dev)
+static inline int qdisc_restart(struct net_device *dev,
+ struct sk_buff_head *blist)
{
struct Qdisc *q = dev->qdisc;
struct sk_buff *skb;
- unsigned lockless;
+ unsigned getlock; /* whether we need to get lock or not */
int ret;
/* Dequeue packet */
- if (unlikely((skb = dev_dequeue_skb(dev, q)) == NULL))
+ if (unlikely(!get_skb(dev, q, blist, &skb)))
return 0;
/*
* When the driver has LLTX set, it does its own locking in
- * start_xmit. These checks are worth it because even uncongested
+ * start_xmit. These checks are worth it because even uncontested
* locks can be quite expensive. The driver can do a trylock, as
* is being done here; in case of lock contention it should return
* NETDEV_TX_LOCKED and the packet will be requeued.
*/
- lockless = (dev->features & NETIF_F_LLTX);
+ getlock = !(dev->features & NETIF_F_LLTX);
- if (!lockless && !netif_tx_trylock(dev)) {
+ if (getlock && !netif_tx_trylock(dev)) {
/* Another CPU grabbed the driver tx lock */
return handle_dev_cpu_collision(skb, dev, q);
}
@@ -158,9 +204,12 @@ static inline int qdisc_restart(struct n
/* And release queue */
spin_unlock(&dev->queue_lock);
- ret = dev_hard_start_xmit(skb, dev);
+ if (likely(skb))
+ ret = dev_hard_start_xmit(skb, dev);
+ else
+ ret = dev->hard_start_xmit_batch(dev);
- if (!lockless)
+ if (getlock)
netif_tx_unlock(dev);
spin_lock(&dev->queue_lock);
@@ -168,7 +217,7 @@ static inline int qdisc_restart(struct n
switch (ret) {
case NETDEV_TX_OK:
- /* Driver sent out skb successfully */
+ /* Driver sent out skb (or entire skb_blist) successfully */
ret = qdisc_qlen(q);
break;
@@ -179,10 +228,9 @@ static inline int qdisc_restart(struct n
default:
/* Driver returned NETDEV_TX_BUSY - requeue skb */
- if (unlikely (ret != NETDEV_TX_BUSY && net_ratelimit()))
- printk(KERN_WARNING "BUG %s code %d qlen %d\n",
+ if (unlikely(ret != NETDEV_TX_BUSY) && net_ratelimit())
+ printk(KERN_WARNING " %s: BUG. code %d qlen %d\n",
dev->name, ret, q->q.qlen);
-
ret = dev_requeue_skb(skb, dev, q);
break;
}
@@ -190,10 +238,10 @@ static inline int qdisc_restart(struct n
return ret;
}
-void __qdisc_run(struct net_device *dev)
+void __qdisc_run(struct net_device *dev, struct sk_buff_head *blist)
{
do {
- if (!qdisc_restart(dev))
+ if (!qdisc_restart(dev, blist))
break;
} while (!netif_queue_stopped(dev));
^ permalink raw reply
* [ofa-general] [PATCH 03/10] dev.c changes.
From: Krishna Kumar @ 2007-07-20 6:32 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, mcarlson, jagana, general, netdev, tgraf, jeff, hadi,
kaber, mchan, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
Changes in dev.c to support batching : add dev_add_skb_to_blist,
register_netdev recognizes batch aware drivers, and net_tx_action is
the sole user of batching.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
dev.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 74 insertions(+), 3 deletions(-)
diff -ruNp org/net/core/dev.c new/net/core/dev.c
--- org/net/core/dev.c 2007-07-20 07:49:28.000000000 +0530
+++ new/net/core/dev.c 2007-07-20 08:31:35.000000000 +0530
@@ -1414,6 +1414,45 @@ static int dev_gso_segment(struct sk_buf
return 0;
}
+/*
+ * Add skb (skbs in case segmentation is required) to dev->skb_blist. We are
+ * holding QDISC RUNNING bit, so no one else can add to this list. Also, skbs
+ * are dequeued from this list when we call the driver, so the list is safe
+ * from simultaneous deletes too.
+ *
+ * Returns count of successful skb(s) added to skb_blist.
+ */
+int dev_add_skb_to_blist(struct sk_buff *skb, struct net_device *dev)
+{
+ if (!list_empty(&ptype_all))
+ dev_queue_xmit_nit(skb, dev);
+
+ if (netif_needs_gso(dev, skb)) {
+ if (unlikely(dev_gso_segment(skb))) {
+ kfree(skb);
+ return 0;
+ }
+
+ if (skb->next) {
+ int count = 0;
+
+ do {
+ struct sk_buff *nskb = skb->next;
+
+ skb->next = nskb->next;
+ __skb_queue_tail(dev->skb_blist, nskb);
+ count++;
+ } while (skb->next);
+
+ skb->destructor = DEV_GSO_CB(skb)->destructor;
+ kfree_skb(skb);
+ return count;
+ }
+ }
+ __skb_queue_tail(dev->skb_blist, skb);
+ return 1;
+}
+
int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
if (likely(!skb->next)) {
@@ -1566,7 +1605,7 @@ gso:
/* reset queue_mapping to zero */
skb->queue_mapping = 0;
rc = q->enqueue(skb, q);
- qdisc_run(dev);
+ qdisc_run(dev, NULL);
spin_unlock(&dev->queue_lock);
rc = rc == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : rc;
@@ -1763,7 +1802,11 @@ static void net_tx_action(struct softirq
clear_bit(__LINK_STATE_SCHED, &dev->state);
if (spin_trylock(&dev->queue_lock)) {
- qdisc_run(dev);
+ /*
+ * Try to send out all skbs if batching is
+ * enabled.
+ */
+ qdisc_run(dev, dev->skb_blist);
spin_unlock(&dev->queue_lock);
} else {
netif_schedule(dev);
@@ -3397,6 +3440,28 @@ int register_netdevice(struct net_device
}
}
+ if (dev->features & NETIF_F_BATCH_SKBS) {
+ if (!dev->hard_start_xmit_batch ||
+ dev->tx_queue_len < MIN_QUEUE_LEN_BATCH) {
+ /*
+ * Batch TX requires API support in driver plus have
+ * a minimum sized queue.
+ */
+ printk(KERN_ERR "%s: Dropping NETIF_F_BATCH_SKBS "
+ "since no API support or queue len "
+ "is smaller than %d.\n",
+ dev->name, MIN_QUEUE_LEN_BATCH);
+ dev->features &= ~NETIF_F_BATCH_SKBS;
+ } else {
+ dev->skb_blist = kmalloc(sizeof *dev->skb_blist,
+ GFP_KERNEL);
+ if (dev->skb_blist) {
+ skb_queue_head_init(dev->skb_blist);
+ dev->tx_queue_len >>= 1;
+ }
+ }
+ }
+
/*
* nil rebuild_header routine,
* that should be never called and used as just bug trap.
@@ -3732,10 +3797,16 @@ void unregister_netdevice(struct net_dev
synchronize_net();
+ /* Deallocate batching structure */
+ if (dev->skb_blist) {
+ skb_queue_purge(dev->skb_blist);
+ kfree(dev->skb_blist);
+ dev->skb_blist = NULL;
+ }
+
/* Shutdown queueing discipline. */
dev_shutdown(dev);
-
/* Notify protocols, that we are about to destroy
this device. They should clean all the things.
*/
^ permalink raw reply
* [ofa-general] [PATCH 01/10] HOWTO documentation for Batching SKB.
From: Krishna Kumar @ 2007-07-20 6:32 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, peter.p.waskiewicz.jr, herbert, gaagaan,
kumarkr, mcarlson, netdev, jagana, general, mchan, tgraf, jeff,
hadi, kaber, sri
In-Reply-To: <20070720063149.26341.84076.sendpatchset@localhost.localdomain>
Add HOWTO documentation on what batching is, how to implement drivers to use
it, and how users can enable/disable batching.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
Batching_skb_API.txt | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 91 insertions(+)
diff -ruNp org/Documentation/networking/Batching_skb_API.txt new/Documentation/networking/Batching_skb_API.txt
--- org/Documentation/networking/Batching_skb_API.txt 1970-01-01 05:30:00.000000000 +0530
+++ new/Documentation/networking/Batching_skb_API.txt 2007-07-20 08:30:22.000000000 +0530
@@ -0,0 +1,91 @@
+ HOWTO for batching skb API support
+ -----------------------------------
+
+Section 1: What is batching skb API ?
+Section 2: How batching API works vs the original API ?
+Section 3: How drivers can support this API ?
+Section 4: How users can work with this API ?
+
+
+Introduction: Kernel support for batching skb
+-----------------------------------------------
+
+An extended API is supported in the netdevice layer, which is very similar
+to the existing hard_start_xmit() API. Drivers which wish to take advantage
+of this new API should implement this routine similar to how the
+hard_start_xmit handler is written. The difference between these API's is
+that while the existing hard_start_xmit processes one skb, the new API can
+process multiple skbs (or even one) in a single call. It is also possible
+for the driver writer to re-use most of the code from the existing API in
+the new API without having code duplication.
+
+
+Section 1: What is batching skb API ?
+-------------------------------------
+
+ This is a new API that is optionally exported by a driver. The pre-
+ requisite for a driver to use this API is that it should have a
+ reasonably sized hardware queue that can process multiple skbs.
+
+
+Section 2: How batching API works vs the original API ?
+-------------------------------------------------------
+
+ The networking stack normally gets called from upper layer protocols
+ with a single skb to xmit. This skb is first enqueue'd and an
+ attempt is next made to transmit it immediately (via qdisc_run).
+ However, events like driver lock contention, queue stopped, etc, can
+ result in the skb not getting sent out, and it remains in the queue.
+ When a new xmit is called or when the queue is re-enabled, qdisc_run
+ could potentially find multiple packets in the queue, and have to
+ send them all out one by one iteratively.
+
+ The batching skb API case was added to exploit this situation where
+ if there are multiple skbs, all of them can be sent to the device in
+ one shot. This reduces driver processing, locking at the driver (or
+ in stack for ~LLTX drivers) gets amortized over multiple skbs, and
+ in case of specific drivers where every xmit results in a completion
+ processing (like IPoIB), optimizations could be made in the driver
+ to get a completion for only the last skb that was sent which will
+ result in saving interrupts for every (but the last) skb that was
+ sent in the same batch.
+
+ This batching can result in significant performance gains for
+ systems that have multiple data stream paths over the same network
+ interface card.
+
+
+Section 3: How drivers can support this API ?
+---------------------------------------------
+
+ The new API - dev->hard_start_xmit_batch(struct net_device *dev),
+ simplistically, can be written almost identically to the regular
+ xmit API (hard_start_xmit), except that all skbs on dev->skb_blist
+ should be processed by the driver instead of just one skb. The new
+ API doesn't get any skb as argument to process, instead it picks up
+ all the skbs from dev->skb_blist, where it was added by the stack,
+ and tries to send them out.
+
+ Batching requires the driver to set the NETIF_F_BATCH_SKBS bit in
+ dev->features, and dev->hard_start_xmit_batch should point to the
+ new API implemented for that driver.
+
+
+Section 4: How users can work with this API ?
+---------------------------------------------
+
+ Batching could be disabled for a particular device, e.g. on desktop
+ systems if only one stream of network activity for that device is
+ taking place, since performance could be slightly affected due to
+ extra processing that batching adds. Batching can be enabled if
+ more than one stream of network activity per device is being done,
+ e.g. on servers, or even desktop usage with multiple browser, chat,
+ file transfer sessions, etc.
+
+ Per device batching can be enabled/disabled using:
+
+ echo 1 > /sys/class/net/<device-name>/tx_batch_skbs (enable)
+ echo 0 > /sys/class/net/<device-name>/tx_batch_skbs (disable)
+
+ E.g. to enable batching on eth0, run:
+ echo 1 > /sys/class/net/eth0/tx_batch_skbs
^ permalink raw reply
* [ofa-general] [PATCH 00/10] Implement batching skb API
From: Krishna Kumar @ 2007-07-20 6:31 UTC (permalink / raw)
To: davem, rdreier
Cc: johnpol, Robert.Olsson, herbert, gaagaan, kumarkr, jagana,
peter.p.waskiewicz.jr, mcarlson, kaber, jeff, general, mchan,
tgraf, sri, hadi, netdev
Hi Dave, Roland, everyone,
In May, I had proposed creating an API for sending 'n' skbs to a driver to
reduce lock overhead, DMA operations, and specific to drivers that have
completion notification like IPoIB - reduce completion handling ("[RFC] New
driver API to speed up small packets xmits" @
http://marc.info/?l=linux-netdev&m=117880900818960&w=2). I had also sent
initial test results for E1000 which showed minor improvements (but also
got degradations) @http://marc.info/?l=linux-netdev&m=117887698405795&w=2.
After fine-tuning qdisc and other changes, I modified IPoIB to use this API,
and now get good gains. Summary for TCP & No Delay: 1 process improves for
all cases from 1.4% to 49.5%; 4 process has almost identical improvements
from -1.7% to 59.1%; 16 process case also improves in the range of -1.2% to
33.4%; while 64 process doesn't have much improvement (-3.3% to 12.4%). UDP
was tested with 1 process netperf with small increase in BW but big
improvement in Service Demand. Netperf latency tests show small drop in
transaction rate (results in separate attachment).
To verify that performance does not degrade with batching turned off (as is
the case for all existing drivers), I ran tests with tx_batch_skbs=0 vs the
original code, without getting real degradation. Also enabled all kernel
debugs to catch panics, warnings, memory free use bugs, etc, and simulated
driver errors to get coverage on core & IPoIB error paths. Testing was on
2-CPU X-series systems and 8-CPU PPC64 Power5 systems using IPoIB over mthca,
and E1000 (used driver that Jamal had converted but didn't get improvement).
On i386, the size of the kernel (drivers are modules) increased by:
text: 0.007% data: 0.007% bss: 0% total: 0.03%.
There is a parallel WIP by Jamal but the two implementations are completely
different since the code bases from the start were separate. Key changes:
- Use a single qdisc interface to avoid code duplication and reduce
maintainability (sch_generic.c size reduces by ~9%).
- Has per device configurable parameter to turn on/off batching.
- qdisc_restart gets slightly modified while looking simple without
any checks for batching vs regular code (infact only two lines have
changed - 1. instead of dev_dequeue_skb, a new batch-aware function
is called; and 2. an extra call to hard_start_xmit_batch.
- Batching algo/processing is different (eg. if qdisc_restart() finds
one skb in the batch list, it will try to batch more (upto a limit)
instead of sending that out and batching the rest in the next call.
- No change in__qdisc_run other than a new argument (from DM's idea).
- Applies to latest net-2.6.23 compared to 2.6.22-rc4 code.
- Jamal's code has a separate hw prep handler called from the stack,
and results are accessed in driver during xmit later.
- Jamal's code has dev->xmit_win which is cached by the driver. Mine
has dev->xmit_slots but this is used only by the driver while the
core has a different mechanism to find how many skbs to batch.
- Completely different structure/design & coding styles.
(This patch will work with drivers updated by Jamal, Matt & Michael Chan with
minor modifications - rename xmit_win to xmit_slots & rename batch handler)
Patches are described as:
Mail 0/10 : This mail.
Mail 1/10 : HOWTO documentation.
Mail 2/10 : Networking include file changes.
Mail 3/10 : dev.c changes.
Mail 4/10 : net-sysfs.c changes.
Mail 5/10 : sch_generic.c changes.
Mail 6/10 : IPoIB include file changes.
Mail 7/10 : IPoIB verbs changes
Mail 8/10 : IPoIB multicast, CM changes
Mail 9/10 : IPoIB xmit API addition
Mail 10/10 : IPoIB xmit internals changes (ipoib_ib.c)
I am also sending separately an attachment with results (across 10 run
cycle), test scripts and a script to analyze results.
Thanks to Sridhar & Shirley Ma for code reviews; Evgeniy, Jamal & Sridhar for
suggesting to put driver skb list on netdev instead of on skb to avoid
requeue; and David Miller for explanation on using batching only when the
queue is woken up.
Please review and provide feedback/ideas; and consider for inclusion.
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH] [updated] PHY fixed driver: rework release path and update phy_id notation
From: Andrew Morton @ 2007-07-20 6:23 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev, linux-kernel, Jeff Garzik, netdev
In-Reply-To: <20070718233804.24451.22543.stgit@localhost.localdomain>
On Thu, 19 Jul 2007 03:38:04 +0400 Vitaly Bordug <vitb@kernel.crashing.org> wrote:
>
> device_bind_driver() error code returning has been fixed.
> release() function has been written, so that to free resources
> in correct way; the release path is now clean.
>
> Before the rework, it used to cause
> Device 'fixed@100:1' does not have a release() function, it is broken
> and must be fixed.
> BUG: at drivers/base/core.c:104 device_release()
>
> Call Trace:
> [<ffffffff802ec380>] kobject_cleanup+0x53/0x7e
> [<ffffffff802ec3ab>] kobject_release+0x0/0x9
> [<ffffffff802ecf3f>] kref_put+0x74/0x81
> [<ffffffff8035493b>] fixed_mdio_register_device+0x230/0x265
> [<ffffffff80564d31>] fixed_init+0x1f/0x35
> [<ffffffff802071a4>] init+0x147/0x2fb
> [<ffffffff80223b6e>] schedule_tail+0x36/0x92
> [<ffffffff8020a678>] child_rip+0xa/0x12
> [<ffffffff80311714>] acpi_ds_init_one_object+0x0/0x83
> [<ffffffff8020705d>] init+0x0/0x2fb
> [<ffffffff8020a66e>] child_rip+0x0/0x12
>
>
> Also changed the notation of the fixed phy definition on
> mdio bus to the form of <speed>+<duplex> to make it able to be used by
> gianfar and ucc_geth that define phy_id strictly as "%d:%d" and cleaned up
> the whitespace issues.
>
Confused. Does the above refer to the difference between this patch and
the previous version, or does it just describe this patch? Hopefully the
latter, because the former isn't interesting, long-term.
If is _is_ a full standalone description of this patch then it's a bit hard
to follow ;)
> +config FIXED_MII_1000_FDX
> + bool "Emulation for 1000M Fdx fixed PHY behavior"
> + depends on FIXED_PHY
> +
> +config FIXED_MII_AMNT
> + int "Number of emulated PHYs to allocate "
> + depends on FIXED_PHY
> + default "1"
> + ---help---
> + Sometimes it is required to have several independent emulated
> + PHYs on the bus (in case of multi-eth but phy-less HW for instance).
> + This control will have specified number allocated for each fixed
> + PHY type enabled.
Shouldn't these be runtime options (ie: module parameters)?
>
> ...
>
> + * Private information hoder for mii_bus
tpyo.
^ permalink raw reply
* Fwd: [Bug 7133] ibmtr_cs seams working as fine with 64bit kernel than with 32bits
From: Natalie Protasevich @ 2007-07-20 3:47 UTC (permalink / raw)
To: linux-tr, netdev
In-Reply-To: <20070720034327.AC795108093@picon.linux-foundation.org>
Can someone help to assess suggested in the bugzilla? The author found
that ibmtr_cs works fine in 64 but mode, and config parameters suggest
it doesn't.
Thanks!
---------- Forwarded message ----------
From: bugme-daemon@bugzilla.kernel.org <bugme-daemon@bugzilla.kernel.org>
Date: Jul 19, 2007 8:43 PM
Subject: [Bug 7133] ibmtr_cs seams working as fine with 64bit kernel
than with 32bits
To: protasnb@gmail.com
http://bugzilla.kernel.org/show_bug.cgi?id=7133
protasnb@gmail.com changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |protasnb@gmail.com
------- Comment #1 from protasnb@gmail.com 2007-07-19 20:48 -------
Probably needs review by those with knowledge of TR architecture. Will cc to
linux-tr@linuxtr.net
--
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
^ permalink raw reply
* Re: [PATCH] Merge GT/MV642xx Support into MV643xx Driver [7/8]
From: Dale Farnsworth @ 2007-07-20 3:40 UTC (permalink / raw)
To: Steven J. Hill; +Cc: netdev
In-Reply-To: <46A01992.1060907@realitydiluted.com>
On Thu, Jul 19, 2007 at 09:10:26PM -0500, Steven J. Hill wrote:
> Dale Farnsworth wrote:
> >
> > You have replaced the use of the global PHY spinlock with a per-port spinlock.
> > However, the SMI register is shared by all ports. The global lock is
> > needed to prevent simultaneous updates of the register by drivers for
> > multiple ports.
> >
> > NAK
> >
> Are you sure? Notice that a majority of the spinlocks were changed to disable
> IRQs. Secondly, the lowest level mv_read/mv_write functions have to acquire
> the big mv64x60_lock before they can read or write registers. I see the PHY
> spinlock as being an additional and unnecessary lock to contend with. Am I
> make an improper assumption?
I'm sure. (Of course, I could be wrong.) On an SMP (or fully
preemptive) system, disabling IRQs doesn't provide sufficient
protection. Nor does a per-port spinlock, since multiple ports
share the single register. It seems to me that a driver-scope
lock is required.
-Dale
^ permalink raw reply
* Re: [PATCH 1/2] net/core: merge the content of dev_mcast.c into dev.c
From: rae l @ 2007-07-20 3:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kuznet, kaber, pekkas, jmorris, linux-kernel
In-Reply-To: <20070719.200459.41636483.davem@davemloft.net>
On 7/20/07, David Miller <davem@davemloft.net> wrote:
> Please don't quote a big huge patch just to say one sentence that
> doesn't apply to any particular specific part of a patch.
>
> That's wastes bandwidth, annoys people you might actually want
> a response from, and is bad netiquette in general.
Thanks.
>
--
Denis Cheng
Linux Application Developer
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
^ permalink raw reply
* Possible eHEA performance issue
From: Michael Neuling @ 2007-07-20 3:02 UTC (permalink / raw)
To: Thomas Klein, Jan-Bernd Themann, netdev, Christoph Raisch,
Stefan Roscher, linux-ppc
Cc: anton
>From ehea_start_xmit in ehea_main.c we have:
if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
spin_lock_irqsave(&pr->netif_queue, flags);
if (unlikely(atomic_read(&pr->swqe_avail) <= 1)) {
pr->p_stats.queue_stopped++;
netif_stop_queue(dev);
pr->queue_stopped = 1;
}
spin_unlock_irqrestore(&pr->netif_queue, flags);
}
Since the conditions are the same, isn't it likely that the second 'if'
is going to be taken. Hence, shouldn't the second 'unlikely' hint be
removed or even changed to likely?
Either way, some documentation here as to why it's done this way would
be useful. I assume the atomic_read is cheap compared to the
spin_unlock_irqsave, so we quickly check swqe_avail before we check it
again properly with the lock on so we can change some stuff.
Mikey
^ permalink raw reply
* Re: [PATCH 1/2] net/core: merge the content of dev_mcast.c into dev.c
From: David Miller @ 2007-07-20 3:04 UTC (permalink / raw)
To: crquan; +Cc: netdev, kuznet, kaber, pekkas, jmorris, linux-kernel
In-Reply-To: <91b13c310707191851h3bffaf2au7443d32c725dfb45@mail.gmail.com>
From: "rae l" <crquan@gmail.com>
Date: Fri, 20 Jul 2007 09:51:05 +0800
> Is there any more comments on this?
Please don't quote a big huge patch just to say one sentence that
doesn't apply to any particular specific part of a patch.
That's wastes bandwidth, annoys people you might actually want
a response from, and is bad netiquette in general.
^ permalink raw reply
* Re: [PATCH] Merge GT/MV642xx Support into MV643xx Driver [7/8]
From: Steven J. Hill @ 2007-07-20 2:10 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: netdev
In-Reply-To: <20070719142159.GF12892@xyzzy.farnsworth.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Dale Farnsworth wrote:
>
> You have replaced the use of the global PHY spinlock with a per-port spinlock.
> However, the SMI register is shared by all ports. The global lock is
> needed to prevent simultaneous updates of the register by drivers for
> multiple ports.
>
> NAK
>
Are you sure? Notice that a majority of the spinlocks were changed to disable
IRQs. Secondly, the lowest level mv_read/mv_write functions have to acquire
the big mv64x60_lock before they can read or write registers. I see the PHY
spinlock as being an additional and unnecessary lock to contend with. Am I
make an improper assumption?
- -Steve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGoBmSgyK5H2Ic36cRAj8JAKCfmg/T2FgOdYZ5YfnXJsiyn3RkaQCfadSk
GS8ICyW0+qNRHr5QqnY0PUQ=
=nvSB
-----END PGP SIGNATURE-----
^ 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