Netdev List
 help / color / mirror / Atom feed
* Re: IPV6 RFC3542 compliance [PATCH]
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2005-06-07  7:05 UTC (permalink / raw)
  To: dlstevens; +Cc: davem, netdev, yoshfuji
In-Reply-To: <OF4AE6E385.33144EB9-ON88257019.0024BCE9-88257019.00258FFE@us.ibm.com>

In article <OF4AE6E385.33144EB9-ON88257019.0024BCE9-88257019.00258FFE@us.ibm.com> (at Mon, 6 Jun 2005 23:50:16 -0700), David Stevens <dlstevens@us.ibm.com> says:

> > Portable applications do like this:
> 
> > #ifdef IPV6_RECVHOPOPTS
> > // RFC2292bis
> > #else
> > // RFC2292
> > #endif
> 
> > --yoshfuji
> 
> I don't understand. If they do this, they'll
> work already when recompiled (with the patch
> I sent), won't they?

Yes (or they should do so before your favorite distro start shipping with
new constants).

> How does it help to renumber? I can renumber,
> of course-- I just don't see how that does
> anything.

We can still keep old binaries if we renumber.
This is important point.
e.g. people, including myself, can keep using old binaries on new kernels.

--yoshfuji

^ permalink raw reply

* Re: IPV6 RFC3542 compliance [PATCH]
From: David Stevens @ 2005-06-07  6:50 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / 吉藤英明; +Cc: davem, netdev
In-Reply-To: <20050607.153359.82068814.yoshfuji@linux-ipv6.org>

> Portable applications do like this:

> #ifdef IPV6_RECVHOPOPTS
> // RFC2292bis
> #else
> // RFC2292
> #endif

> --yoshfuji

I don't understand. If they do this, they'll
work already when recompiled (with the patch
I sent), won't they?

If they don't do this, old binaries will
return EINVAL on the setsockopt() calls that
have changed. And if they're going to edit
the source, they can do #ifdefs as above and
work again.

How does it help to renumber? I can renumber,
of course-- I just don't see how that does
anything.

                                        +-DLS

^ permalink raw reply

* Re: IPV6 RFC3542 compliance [PATCH]
From: David Stevens @ 2005-06-07  6:35 UTC (permalink / raw)
  To: David Stevens; +Cc: davem, netdev
In-Reply-To: <OF9F62ECDB.67FFA191-ON88257019.001F5EA8-88257019.00234AB6@us.ibm.com>

PS -

I should've said clearly; with the patch I submitted as-is, all
old binaries should return EINVAL on the socket options
that have changed. That's because all of those (except the
new IPV6_TCLASS, which didn't exist before) have option
arguments greater than int-size.

Recompiling those programs will still result in the setsockopt()
returning EINVAL, until the source is fixed to change the
socket options to the IPV6_RECVx.

sendmsg() and recvmsg() processing in old binaries should
still work, as-is.

So, with that patch, programs using the old names will give
a strong indication of what needs fixing.

                                        +-DLS

^ permalink raw reply

* Re: IPV6 RFC3542 compliance [PATCH]
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2005-06-07  6:33 UTC (permalink / raw)
  To: dlstevens; +Cc: davem, netdev, yoshfuji
In-Reply-To: <OF9F62ECDB.67FFA191-ON88257019.001F5EA8-88257019.00234AB6@us.ibm.com>

In article <OF9F62ECDB.67FFA191-ON88257019.001F5EA8-88257019.00234AB6@us.ibm.com> (at Mon, 6 Jun 2005 23:25:28 -0700), David Stevens <dlstevens@us.ibm.com> says:

> And those same binaries would not work when recompiled,
> because the option names in the source would match the
> new numbers, but still have the old arguments-- an error to
> be detected at run-time, only.

It is not good at all to break API at this moment (2.6.x).

Portable applications do like this:

#ifdef IPV6_RECVHOPOPTS
   // RFC2292bis
#else
   // RFC2292
#endif

--yoshfuji

^ permalink raw reply

* Re: IPV6 RFC3542 compliance [PATCH]
From: David Stevens @ 2005-06-07  6:25 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / 吉藤英明; +Cc: davem, netdev
In-Reply-To: <20050607.141922.65612976.yoshfuji@linux-ipv6.org>

RFC 3542 broke the API-- they've defined options with
the same name, but different semantics.

Binaries using the old numbers would not work, unless
we return the old numbers in the control message types,
but in the new API, those have to be different from the
boolean option value (and equal to the sticky option value).

And those same binaries would not work when recompiled,
because the option names in the source would match the
new numbers, but still have the old arguments-- an error to
be detected at run-time, only.

My guess is that existing use of these is pretty limited, so
I'm not sure backward compatibility is worth it.

If we wanted to get really ugly, we could use the size of
the option value to determine what to do. Only two new
ancillary message types are int-sized (TCLASS and
HOPLIMIT). HOPLIMIT is not a valid socket option,
(done with IPV6_UNICAST_HOPS instead) and TCLASS
was not implemented at all-- not a problem. Then, in the
receive processing, we'd have to return the old message
type for programs using the sticky options as boolean,
and the new message type otherwise.

It's really ugly, but possible, I believe; then it would break
RFC 3542 compliance only in not treating boolean-sized
options as an error. But I think the better way is to fix programs
that use these right away. A program that uses "IPV6_RTHDR"
with a boolean argment is not portable (which is the whole
point of having a common API). We shouldn't encourage it
by making it continue to work.
                                        +-DLS

^ permalink raw reply

* Re: [PATCH 0/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  5:51 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, herbert, jheffner
In-Reply-To: <42A528F0.2090208@osdl.org>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Mon, 06 Jun 2005 21:56:16 -0700

> I'll merge these with the TCP infrastructure stuff and
> send it off to Andrew. Actually, it is more of fix the TCP
> infrastructure to match TSO + rc6 but you get the ida.

Probably not a good idea, it's %75 of the implementation
of Super TSO and totally conflicts with the super TSO patch.

Probably best to keep the existing Super TSO stuff in there
until I'm done with this stuff. :)

^ permalink raw reply

* Re: 2.6.12-rcx networking oops
From: randy_dunlap @ 2005-06-07  5:46 UTC (permalink / raw)
  To: Phil Oester; +Cc: herbert, netdev, akpm
In-Reply-To: <20050601170058.GA20112@linuxace.com>

On Wed, 1 Jun 2005 10:00:58 -0700 Phil Oester wrote:

| On Wed, Jun 01, 2005 at 03:49:55PM +1000, Herbert Xu wrote:
| > This looks like stack overflow.  %esi is meant to be "res" which is
| > a local variable.  As you can see, it's pointing below %esp and
| > threadinfo.

Agreed, the stack trace is suspicious.  (more below)

| Ok, so I enabled DEBUG_STACKOVERFLOW in addition to CONFIG_DEBUG_SLAB
| and CONFIG_DEBUG_PAGEALLOC, and got the below today...so maybe it
| is a slab issue?
| 
| 0xc0238cdd is in dst_alloc (net/core/dst.c:124).
| 119             if (ops->gc && atomic_read(&ops->entries) > ops->gc_thresh) {
| 120                     if (ops->gc())
| 121                             return NULL;
| 122             }
| 123             dst = kmem_cache_alloc(ops->kmem_cachep, SLAB_ATOMIC);
| 
| 0xc013912b is at mm/slab.c:3077.
| 3072                    size = kmem_cache_size(c);
| 3073                    local_irq_restore(flags);
| 3074            }
| 3075
| 3076            return size;
| 3077    }
| 
| 
| Phil

This is with NAPI, right?  Would it make sense to try it with that
disabled?  (I don't recall you saying it's NAPI, but the e1000
functions seem to indicate that.)

and how about enabling CONFIG_FRAME_POINTER ?


| invalid operand: 0000 [#1]
| SMP DEBUG_PAGEALLOC
| CPU:    1
| EIP:    0060:[<c013912b>]    Not tainted VLI
| EFLAGS: 00016292   (2.6.12-rc5-git5) 
| EIP is at ksize+0x7b/0x100

ksize() isn't that large.  In my build this offset and the
Code: 8d 05 0c.... (below)
point to the lock slow paths in mm/slab.c (fwiw).


| eax: c0238cdd   ebx: f7ba9c20   ecx: f7babf78   edx: dcc59000
| esi: 00000020   edi: 0000e3ba   ebp: c0338d98   esp: c0338d88
| ds: 007b   es: 007b   ss: 0068
| Process swapper (pid: 0, threadinfo=c0338000 task=c1989b00)
| Stack: 00000000 04000000 c02d1a00 ffffff97 c0338db0 c0238cdd c0338e58 04000000 
|        00000000 ffffff97 c0338eb4 c0245cb7 00000002 f7b01000 c0338dec c0338df0 
|        f7318ef8 00000000 00000000 00000001 f72dbef8 0000a704 103c243b f27ceec0 
| Call Trace:
|  [<c010389a>] show_stack+0x7a/0x90
|  [<c0103a1d>] show_registers+0x14d/0x1b0
|  [<c0103c29>] die+0xf9/0x180
|  [<c0103d50>] do_trap+0xa0/0xb0
|  [<c0104039>] do_invalid_op+0xa9/0xc0
|  [<c01034e3>] error_code+0x4f/0x54
|  [<c0238cdd>] dst_alloc+0x2d/0xa0
|  [<c0245cb7>] ip_route_input_slow+0x4a7/0x840
|  [<c02460ea>] ip_route_input+0x9a/0x160
|  [<c02481c0>] ip_rcv+0x3b0/0x4d0
|  [<c02357aa>] netif_receive_skb+0x13a/0x1a0
|  [<c01fa1d0>] e1000_clean_rx_irq+0x180/0x4d0
|  [<c01f9a10>] e1000_clean+0x40/0xe0
|  [<c02359c0>] net_rx_action+0x90/0x130
|  [<c011a8c4>] __do_softirq+0xd4/0xf0
|  [<c0104fc2>] do_softirq+0x52/0x70
|  =======================
|  [<c011a9aa>] irq_exit+0x3a/0x40
|  [<c0104e98>] do_IRQ+0x68/0xa0
|  [<c010338a>] common_interrupt+0x1a/0x20
|  [<c0100a8b>] cpu_idle+0x7b/0x80
|  [<c0305c13>] start_secondary+0x73/0x90
|  [<00000000>] stext+0x3feffd6c/0xc
|  [<c198afb4>] 0xc198afb4
| Code: 8d 05 0c e2 34 c0 e8 e9 25 15 00 e9 96 dd ff ff 8d 05 0c e2 34 c0 e8 a9 25 15 00 e9 00 e2 ff 
| 
| ff 8d 05 0c e2 34 c0 e8 c9 25 15 00 <e9> 23 e2 ff ff 8d 05 0c e2 34 c0 e8 89 25 15 00 e9 84 e2 ff ff 
|  <0>Kernel panic - not syncing: Fatal exception in interrupt


---
~Randy

^ permalink raw reply

* Re: IPV6 RFC3542 compliance [PATCH]
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2005-06-07  5:19 UTC (permalink / raw)
  To: dlstevens; +Cc: davem, netdev, yoshfuji
In-Reply-To: <OF68DFE8BB.D1DDCA2A-ON88257018.0068C3C7-88257018.006CCE31@us.ibm.com>

In article <OF68DFE8BB.D1DDCA2A-ON88257018.0068C3C7-88257018.006CCE31@us.ibm.com> (at Mon, 6 Jun 2005 13:48:26 -0600), David Stevens <dlstevens@us.ibm.com> says:

> I've been looking at RFC 3542 (Advanced Sockets API) compliance,
> and found the following:
> 
> ("x" is one of {PKTINFO, HOPLIMIT, RTHDR, DSTOPTS, TCLASS })

Well, this breaks API.

Please rename old options, say:

  IPV6_PKTINFO => IPV6_2292PKTINFO
  IPV6_HOPLIMIT => IPV6_2292HOPLIMI
  IPV6_RTHDR => IPV6_2292RTHDR
  IPV6_DSTOPTS => IPV6_2292DSTOPTS

And, add allocate new values for 2292bis options like:

#define IPV6_RECVPKTINFO               48      /* RFC2292bis */
#define IPV6_PKTINFO                   49      /* RFC2292bis */
#define IPV6_RECVHOPLIMIT              50      /* RFC2292bis */
#define IPV6_HOPLIMIT                  51      /* RFC2292bis */
#define IPV6_RECVRTHDR                 52      /* RFC2292bis */
#define IPV6_RTHDR                     53      /* RFC2292bis */
#define IPV6_RECVHOPOPTS               54      /* RFC2292bis */
#define IPV6_HOPOPTS                   55      /* RFC2292bis */
#define IPV6_RECVDSTOPTS               56      /* RFC2292bis */
#define IPV6_DSTOPTS                   57      /* RFC2292bis */
#define IPV6_RECVRTHDRDSTOPTS          58      /* RFC2292bis */
#define IPV6_RTHDRDSTOPTS              59      /* RFC2292bis */

(This is what KAME people did, and I believe that it is the best way to keep
backward compatibility.)

-- 
YOSHIFUJI Hideaki @ USAGI Project  <yoshfuji@linux-ipv6.org>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

^ permalink raw reply

* Re: [PATCH 0/9]: TCP: The Road to Super TSO
From: Stephen Hemminger @ 2005-06-07  4:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>

I'll merge these with the TCP infrastructure stuff and
send it off to Andrew. Actually, it is more of fix the TCP
infrastructure to match TSO + rc6 but you get the ida.

^ permalink raw reply

* Re: RFC: NAPI packet weighting patch
From: Stephen Hemminger @ 2005-06-07  4:53 UTC (permalink / raw)
  To: Mitch Williams
  Cc: Ronciak, John, David S. Miller, mchan, hadi, buytenh, jdmason,
	netdev, Robert.Olsson, Venkatesan, Ganesh, Brandeburg, Jesse
In-Reply-To: <Pine.CYG.4.58.0506061647340.128@mawilli1-desk2.amr.corp.intel.com>

I noticed that the tg3 driver copies packets less than a certain
threshold to a new buffer, but e1000 always passes the big buffer up
the stack. Could this be having an impact?

^ permalink raw reply

* [PATCH 9/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:23 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Fix __tcp_push_pending_frames() 'nonagle' handling.

'nonagle' should be passed to the tcp_snd_test() function
as 'TCP_NAGLE_PUSH' if we are checking an SKB not at the
tail of the write_queue.  This is because Nagle does not
apply to such frames since we cannot possibly tack more
data onto them.

However, while doing this __tcp_push_pending_frames() makes
all of the packets in the write_queue use this modified
'nonagle' value.

Fix the bug and simplify this function by just calling
tcp_write_xmit() directly if sk_send_head is non-NULL.

As a result, we can now make tcp_data_snd_check() just call
tcp_push_pending_frames() instead of the specialized
__tcp_data_snd_check().

Signed-off-by: David S. Miller <davem@davemloft.net>

45d0377c7d18e1a036b0a1f96788a998dccf73cf (from f22c7890049ef8c51b0cdcc5d7e0cd06333de6b0)
diff --git a/include/net/tcp.h b/include/net/tcp.h
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -944,7 +944,6 @@ extern __u32 cookie_v4_init_sequence(str
 
 /* tcp_output.c */
 
-extern void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb);
 extern void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
 				      unsigned int cur_mss, int nonagle);
 extern int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3975,12 +3975,9 @@ static inline void tcp_check_space(struc
 	}
 }
 
-static __inline__ void tcp_data_snd_check(struct sock *sk)
+static __inline__ void tcp_data_snd_check(struct sock *sk, struct tcp_sock *tp)
 {
-	struct sk_buff *skb = sk->sk_send_head;
-
-	if (skb != NULL)
-		__tcp_data_snd_check(sk, skb);
+	tcp_push_pending_frames(sk, tp);
 	tcp_check_space(sk);
 }
 
@@ -4274,7 +4271,7 @@ int tcp_rcv_established(struct sock *sk,
 				 */
 				tcp_ack(sk, skb, 0);
 				__kfree_skb(skb); 
-				tcp_data_snd_check(sk);
+				tcp_data_snd_check(sk, tp);
 				return 0;
 			} else { /* Header too small */
 				TCP_INC_STATS_BH(TCP_MIB_INERRS);
@@ -4340,7 +4337,7 @@ int tcp_rcv_established(struct sock *sk,
 			if (TCP_SKB_CB(skb)->ack_seq != tp->snd_una) {
 				/* Well, only one small jumplet in fast path... */
 				tcp_ack(sk, skb, FLAG_DATA);
-				tcp_data_snd_check(sk);
+				tcp_data_snd_check(sk, tp);
 				if (!tcp_ack_scheduled(tp))
 					goto no_ack;
 			}
@@ -4418,7 +4415,7 @@ step5:
 	/* step 7: process the segment text */
 	tcp_data_queue(sk, skb);
 
-	tcp_data_snd_check(sk);
+	tcp_data_snd_check(sk, tp);
 	tcp_ack_snd_check(sk);
 	return 0;
 
@@ -4732,7 +4729,7 @@ int tcp_rcv_state_process(struct sock *s
 		/* Do step6 onward by hand. */
 		tcp_urg(sk, skb, th);
 		__kfree_skb(skb);
-		tcp_data_snd_check(sk);
+		tcp_data_snd_check(sk, tp);
 		return 0;
 	}
 
@@ -4921,7 +4918,7 @@ int tcp_rcv_state_process(struct sock *s
 
 	/* tcp_data could move socket to TIME-WAIT */
 	if (sk->sk_state != TCP_CLOSE) {
-		tcp_data_snd_check(sk);
+		tcp_data_snd_check(sk, tp);
 		tcp_ack_snd_check(sk);
 	}
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -899,24 +899,11 @@ void __tcp_push_pending_frames(struct so
 	struct sk_buff *skb = sk->sk_send_head;
 
 	if (skb) {
-		if (!tcp_skb_is_last(sk, skb))
-			nonagle = TCP_NAGLE_PUSH;
-		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
-		    tcp_write_xmit(sk, cur_mss, nonagle))
+		if (tcp_write_xmit(sk, cur_mss, nonagle))
 			tcp_check_probe_timer(sk, tp);
 	}
 }
 
-void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb)
-{
-	struct tcp_sock *tp = tcp_sk(sk);
-
-	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd) ||
-	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
-	    tcp_write_xmit(sk, tcp_current_mss(sk, 1), tp->nonagle))
-		tcp_check_probe_timer(sk, tp);
-}
-
 /* This function returns the amount that we can raise the
  * usable window based on the following constraints
  *  

^ permalink raw reply

* [PATCH 8/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:22 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Fix redundant calculations of tcp_current_mss()

tcp_write_xmit() uses tcp_current_mss(), but some of it's callers,
namely __tcp_push_pending_frames(), already has this value available
already.

While we're here, fix the "cur_mss" argument to be "unsigned int"
instead of plain "unsigned".

Signed-off-by: David S. Miller <davem@davemloft.net>

f22c7890049ef8c51b0cdcc5d7e0cd06333de6b0 (from b8d892e4dc753d796e80da6e17f2a88aede0695e)
diff --git a/include/net/tcp.h b/include/net/tcp.h
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -946,7 +946,7 @@ extern __u32 cookie_v4_init_sequence(str
 
 extern void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb);
 extern void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
-				      unsigned cur_mss, int nonagle);
+				      unsigned int cur_mss, int nonagle);
 extern int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp);
 extern int tcp_retransmit_skb(struct sock *, struct sk_buff *);
 extern void tcp_xmit_retransmit_queue(struct sock *);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -844,11 +844,10 @@ static inline void tcp_cwnd_validate(str
  * Returns 1, if no segments are in flight and we have queued segments, but
  * cannot send anything now because of SWS or another problem.
  */
-static int tcp_write_xmit(struct sock *sk, int nonagle)
+static int tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct sk_buff *skb;
-	unsigned int mss_now;
 	int sent_pkts;
 
 	/* If we are closed, the bytes will have to remain here.
@@ -858,13 +857,6 @@ static int tcp_write_xmit(struct sock *s
 	if (unlikely(sk->sk_state == TCP_CLOSE))
 		return 0;
 
-
-	/* Account for SACKS, we may need to fragment due to this.
-	 * It is just like the real MSS changing on us midstream.
-	 * We also handle things correctly when the user adds some
-	 * IP options mid-stream.  Silly to do, but cover it.
-	 */
-	mss_now = tcp_current_mss(sk, 1);
 	sent_pkts = 0;
 	while ((skb = sk->sk_send_head) &&
 	       tcp_snd_test(sk, skb, mss_now,
@@ -902,7 +894,7 @@ static int tcp_write_xmit(struct sock *s
  * The socket must be locked by the caller.
  */
 void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
-			       unsigned cur_mss, int nonagle)
+			       unsigned int cur_mss, int nonagle)
 {
 	struct sk_buff *skb = sk->sk_send_head;
 
@@ -910,7 +902,7 @@ void __tcp_push_pending_frames(struct so
 		if (!tcp_skb_is_last(sk, skb))
 			nonagle = TCP_NAGLE_PUSH;
 		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
-		    tcp_write_xmit(sk, nonagle))
+		    tcp_write_xmit(sk, cur_mss, nonagle))
 			tcp_check_probe_timer(sk, tp);
 	}
 }
@@ -921,7 +913,7 @@ void __tcp_data_snd_check(struct sock *s
 
 	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd) ||
 	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
-	    tcp_write_xmit(sk, tp->nonagle))
+	    tcp_write_xmit(sk, tcp_current_mss(sk, 1), tp->nonagle))
 		tcp_check_probe_timer(sk, tp);
 }
 

^ permalink raw reply

* [PATCH 7/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:21 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: tcp_write_xmit() tabbing cleanup

Put the main basic block of work at the top-level of
tabbing, and mark the TCP_CLOSE test with unlikely().

Signed-off-by: David S. Miller <davem@davemloft.net>

b8d892e4dc753d796e80da6e17f2a88aede0695e (from ae083bd3447865cbaf0996a69ba03807fd9fce01)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -847,54 +847,54 @@ static inline void tcp_cwnd_validate(str
 static int tcp_write_xmit(struct sock *sk, int nonagle)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
+	struct sk_buff *skb;
 	unsigned int mss_now;
+	int sent_pkts;
 
 	/* If we are closed, the bytes will have to remain here.
 	 * In time closedown will finish, we empty the write queue and all
 	 * will be happy.
 	 */
-	if (sk->sk_state != TCP_CLOSE) {
-		struct sk_buff *skb;
-		int sent_pkts = 0;
+	if (unlikely(sk->sk_state == TCP_CLOSE))
+		return 0;
 
-		/* Account for SACKS, we may need to fragment due to this.
-		 * It is just like the real MSS changing on us midstream.
-		 * We also handle things correctly when the user adds some
-		 * IP options mid-stream.  Silly to do, but cover it.
-		 */
-		mss_now = tcp_current_mss(sk, 1);
 
-		while ((skb = sk->sk_send_head) &&
-		       tcp_snd_test(sk, skb, mss_now,
-			       	    tcp_skb_is_last(sk, skb) ? nonagle :
-				    			       TCP_NAGLE_PUSH)) {
-			if (skb->len > mss_now) {
-				if (tcp_fragment(sk, skb, mss_now))
-					break;
-			}
-
-			TCP_SKB_CB(skb)->when = tcp_time_stamp;
-			tcp_tso_set_push(skb);
-			if (tcp_transmit_skb(sk, skb_clone(skb, GFP_ATOMIC)))
+	/* Account for SACKS, we may need to fragment due to this.
+	 * It is just like the real MSS changing on us midstream.
+	 * We also handle things correctly when the user adds some
+	 * IP options mid-stream.  Silly to do, but cover it.
+	 */
+	mss_now = tcp_current_mss(sk, 1);
+	sent_pkts = 0;
+	while ((skb = sk->sk_send_head) &&
+	       tcp_snd_test(sk, skb, mss_now,
+			    tcp_skb_is_last(sk, skb) ? nonagle :
+			    TCP_NAGLE_PUSH)) {
+		if (skb->len > mss_now) {
+			if (tcp_fragment(sk, skb, mss_now))
 				break;
+		}
 
-			/* Advance the send_head.  This one is sent out.
-			 * This call will increment packets_out.
-			 */
-			update_send_head(sk, tp, skb);
+		TCP_SKB_CB(skb)->when = tcp_time_stamp;
+		tcp_tso_set_push(skb);
+		if (tcp_transmit_skb(sk, skb_clone(skb, GFP_ATOMIC)))
+			break;
 
-			tcp_minshall_update(tp, mss_now, skb);
-			sent_pkts = 1;
-		}
+		/* Advance the send_head.  This one is sent out.
+		 * This call will increment packets_out.
+		 */
+		update_send_head(sk, tp, skb);
 
-		if (sent_pkts) {
-			tcp_cwnd_validate(sk, tp);
-			return 0;
-		}
+		tcp_minshall_update(tp, mss_now, skb);
+		sent_pkts = 1;
+	}
 
-		return !tp->packets_out && sk->sk_send_head;
+	if (sent_pkts) {
+		tcp_cwnd_validate(sk, tp);
+		return 0;
 	}
-	return 0;
+
+	return !tp->packets_out && sk->sk_send_head;
 }
 
 /* Push out any pending frames which were held back due to

^ permalink raw reply

* [PATCH 6/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:20 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Kill extra cwnd validate in __tcp_push_pending_frames().

The tcp_cwnd_validate() function should only be invoked
if we actually send some frames, yet __tcp_push_pending_frames()
will always invoke it.  tcp_write_xmit() does the call for us,
so the call here can simply be removed.

Also, tcp_write_xmit() can be marked static.

Signed-off-by: David S. Miller <davem@davemloft.net>

ae083bd3447865cbaf0996a69ba03807fd9fce01 (from 79eb6b25499ed5470cb7b20428c435288fcb3502)
diff --git a/include/net/tcp.h b/include/net/tcp.h
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -944,7 +944,6 @@ extern __u32 cookie_v4_init_sequence(str
 
 /* tcp_output.c */
 
-extern int tcp_write_xmit(struct sock *, int nonagle);
 extern void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb);
 extern void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
 				      unsigned cur_mss, int nonagle);
@@ -964,6 +963,9 @@ extern void tcp_push_one(struct sock *, 
 extern void tcp_send_ack(struct sock *sk);
 extern void tcp_send_delayed_ack(struct sock *sk);
 
+/* tcp_input.c */
+extern void tcp_cwnd_application_limited(struct sock *sk);
+
 /* tcp_timer.c */
 extern void tcp_init_xmit_timers(struct sock *);
 extern void tcp_clear_xmit_timers(struct sock *);
@@ -1339,28 +1341,6 @@ static inline void tcp_sync_left_out(str
 	tp->left_out = tp->sacked_out + tp->lost_out;
 }
 
-extern void tcp_cwnd_application_limited(struct sock *sk);
-
-/* Congestion window validation. (RFC2861) */
-
-static inline void tcp_cwnd_validate(struct sock *sk, struct tcp_sock *tp)
-{
-	__u32 packets_out = tp->packets_out;
-
-	if (packets_out >= tp->snd_cwnd) {
-		/* Network is feed fully. */
-		tp->snd_cwnd_used = 0;
-		tp->snd_cwnd_stamp = tcp_time_stamp;
-	} else {
-		/* Network starves. */
-		if (tp->packets_out > tp->snd_cwnd_used)
-			tp->snd_cwnd_used = tp->packets_out;
-
-		if ((s32)(tcp_time_stamp - tp->snd_cwnd_stamp) >= tp->rto)
-			tcp_cwnd_application_limited(sk);
-	}
-}
-
 /* Set slow start threshould and cwnd not falling to slow start */
 static inline void __tcp_enter_cwr(struct tcp_sock *tp)
 {
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -517,35 +517,6 @@ static inline int tcp_skb_is_last(const 
 	return skb->next == (struct sk_buff *)&sk->sk_write_queue;
 }
 
-/* Push out any pending frames which were held back due to
- * TCP_CORK or attempt at coalescing tiny packets.
- * The socket must be locked by the caller.
- */
-void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
-			       unsigned cur_mss, int nonagle)
-{
-	struct sk_buff *skb = sk->sk_send_head;
-
-	if (skb) {
-		if (!tcp_skb_is_last(sk, skb))
-			nonagle = TCP_NAGLE_PUSH;
-		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
-		    tcp_write_xmit(sk, nonagle))
-			tcp_check_probe_timer(sk, tp);
-	}
-	tcp_cwnd_validate(sk, tp);
-}
-
-void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb)
-{
-	struct tcp_sock *tp = tcp_sk(sk);
-
-	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd) ||
-	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
-	    tcp_write_xmit(sk, tp->nonagle))
-		tcp_check_probe_timer(sk, tp);
-}
-
 int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp)
 {
 	struct sk_buff *skb = sk->sk_send_head;
@@ -846,6 +817,26 @@ unsigned int tcp_current_mss(struct sock
 	return mss_now;
 }
 
+/* Congestion window validation. (RFC2861) */
+
+static inline void tcp_cwnd_validate(struct sock *sk, struct tcp_sock *tp)
+{
+	__u32 packets_out = tp->packets_out;
+
+	if (packets_out >= tp->snd_cwnd) {
+		/* Network is feed fully. */
+		tp->snd_cwnd_used = 0;
+		tp->snd_cwnd_stamp = tcp_time_stamp;
+	} else {
+		/* Network starves. */
+		if (tp->packets_out > tp->snd_cwnd_used)
+			tp->snd_cwnd_used = tp->packets_out;
+
+		if ((s32)(tcp_time_stamp - tp->snd_cwnd_stamp) >= tp->rto)
+			tcp_cwnd_application_limited(sk);
+	}
+}
+
 /* This routine writes packets to the network.  It advances the
  * send_head.  This happens as incoming acks open up the remote
  * window for us.
@@ -853,7 +844,7 @@ unsigned int tcp_current_mss(struct sock
  * Returns 1, if no segments are in flight and we have queued segments, but
  * cannot send anything now because of SWS or another problem.
  */
-int tcp_write_xmit(struct sock *sk, int nonagle)
+static int tcp_write_xmit(struct sock *sk, int nonagle)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	unsigned int mss_now;
@@ -906,6 +897,34 @@ int tcp_write_xmit(struct sock *sk, int 
 	return 0;
 }
 
+/* Push out any pending frames which were held back due to
+ * TCP_CORK or attempt at coalescing tiny packets.
+ * The socket must be locked by the caller.
+ */
+void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
+			       unsigned cur_mss, int nonagle)
+{
+	struct sk_buff *skb = sk->sk_send_head;
+
+	if (skb) {
+		if (!tcp_skb_is_last(sk, skb))
+			nonagle = TCP_NAGLE_PUSH;
+		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
+		    tcp_write_xmit(sk, nonagle))
+			tcp_check_probe_timer(sk, tp);
+	}
+}
+
+void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb)
+{
+	struct tcp_sock *tp = tcp_sk(sk);
+
+	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd) ||
+	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
+	    tcp_write_xmit(sk, tp->nonagle))
+		tcp_check_probe_timer(sk, tp);
+}
+
 /* This function returns the amount that we can raise the
  * usable window based on the following constraints
  *  

^ permalink raw reply

* [PATCH 5/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:19 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Add missing skb_header_release() call to tcp_fragment().

When we add any new packet to the TCP socket write queue,
we must call skb_header_release() on it in order for the
TSO sharing checks in the drivers to work.

Signed-off-by: David S. Miller <davem@davemloft.net>

79eb6b25499ed5470cb7b20428c435288fcb3502 (from bdbf09522de5be3ada129dceaa3ad9da9be078bc)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -660,6 +660,7 @@ static int tcp_fragment(struct sock *sk,
 	}
 
 	/* Link BUFF into the send queue. */
+	skb_header_release(buff);
 	__skb_append(skb, buff);
 
 	return 0;

^ permalink raw reply

* [PATCH 4/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:18 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Move __tcp_data_snd_check into tcp_output.c

It reimplements portions of tcp_snd_check(), so it
we move it to tcp_output.c we can consolidate it's
logic much easier in a later change.

Signed-off-by: David S. Miller <davem@davemloft.net>

bdbf09522de5be3ada129dceaa3ad9da9be078bc (from cba5d690f46699d37df7dc087247d1f7c7155692)
diff --git a/include/net/tcp.h b/include/net/tcp.h
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -945,6 +945,7 @@ extern __u32 cookie_v4_init_sequence(str
 /* tcp_output.c */
 
 extern int tcp_write_xmit(struct sock *, int nonagle);
+extern void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb);
 extern void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
 				      unsigned cur_mss, int nonagle);
 extern int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3975,16 +3975,6 @@ static inline void tcp_check_space(struc
 	}
 }
 
-static void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb)
-{
-	struct tcp_sock *tp = tcp_sk(sk);
-
-	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd) ||
-	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
-	    tcp_write_xmit(sk, tp->nonagle))
-		tcp_check_probe_timer(sk, tp);
-}
-
 static __inline__ void tcp_data_snd_check(struct sock *sk)
 {
 	struct sk_buff *skb = sk->sk_send_head;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -536,6 +536,16 @@ void __tcp_push_pending_frames(struct so
 	tcp_cwnd_validate(sk, tp);
 }
 
+void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb)
+{
+	struct tcp_sock *tp = tcp_sk(sk);
+
+	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd) ||
+	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
+	    tcp_write_xmit(sk, tp->nonagle))
+		tcp_check_probe_timer(sk, tp);
+}
+
 int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp)
 {
 	struct sk_buff *skb = sk->sk_send_head;

^ permalink raw reply

* [PATCH 3/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:17 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Move send test logic out of net/tcp.h

This just moves the code into tcp_output.c, no code logic changes are
made by this patch.

Using this as a baseline, we can begin to untangle the mess of
comparisons for the Nagle test et al.  We will also be able to reduce
all of the redundant computation that occurs when outputting data
packets.

Signed-off-by: David S. Miller <davem@davemloft.net>

cba5d690f46699d37df7dc087247d1f7c7155692 (from 00cb08b2ec091f4b461210026392edeaccf31d9c)
diff --git a/include/net/tcp.h b/include/net/tcp.h
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -945,6 +945,9 @@ extern __u32 cookie_v4_init_sequence(str
 /* tcp_output.c */
 
 extern int tcp_write_xmit(struct sock *, int nonagle);
+extern void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
+				      unsigned cur_mss, int nonagle);
+extern int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp);
 extern int tcp_retransmit_skb(struct sock *, struct sk_buff *);
 extern void tcp_xmit_retransmit_queue(struct sock *);
 extern void tcp_simple_retransmit(struct sock *);
@@ -1389,12 +1392,6 @@ static __inline__ __u32 tcp_max_burst(co
 	return 3;
 }
 
-static __inline__ int tcp_minshall_check(const struct tcp_sock *tp)
-{
-	return after(tp->snd_sml,tp->snd_una) &&
-		!after(tp->snd_sml, tp->snd_nxt);
-}
-
 static __inline__ void tcp_minshall_update(struct tcp_sock *tp, int mss, 
 					   const struct sk_buff *skb)
 {
@@ -1402,122 +1399,18 @@ static __inline__ void tcp_minshall_upda
 		tp->snd_sml = TCP_SKB_CB(skb)->end_seq;
 }
 
-/* Return 0, if packet can be sent now without violation Nagle's rules:
-   1. It is full sized.
-   2. Or it contains FIN.
-   3. Or TCP_NODELAY was set.
-   4. Or TCP_CORK is not set, and all sent packets are ACKed.
-      With Minshall's modification: all sent small packets are ACKed.
- */
-
-static __inline__ int
-tcp_nagle_check(const struct tcp_sock *tp, const struct sk_buff *skb, 
-		unsigned mss_now, int nonagle)
-{
-	return (skb->len < mss_now &&
-		!(TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN) &&
-		((nonagle&TCP_NAGLE_CORK) ||
-		 (!nonagle &&
-		  tp->packets_out &&
-		  tcp_minshall_check(tp))));
-}
-
-extern void tcp_set_skb_tso_segs(struct sock *, struct sk_buff *);
-
-/* This checks if the data bearing packet SKB (usually sk->sk_send_head)
- * should be put on the wire right now.
- */
-static __inline__ int tcp_snd_test(struct sock *sk,
-				   struct sk_buff *skb,
-				   unsigned cur_mss, int nonagle)
-{
-	struct tcp_sock *tp = tcp_sk(sk);
-	int pkts = tcp_skb_pcount(skb);
-
-	if (!pkts) {
-		tcp_set_skb_tso_segs(sk, skb);
-		pkts = tcp_skb_pcount(skb);
-	}
-
-	/*	RFC 1122 - section 4.2.3.4
-	 *
-	 *	We must queue if
-	 *
-	 *	a) The right edge of this frame exceeds the window
-	 *	b) There are packets in flight and we have a small segment
-	 *	   [SWS avoidance and Nagle algorithm]
-	 *	   (part of SWS is done on packetization)
-	 *	   Minshall version sounds: there are no _small_
-	 *	   segments in flight. (tcp_nagle_check)
-	 *	c) We have too many packets 'in flight'
-	 *
-	 * 	Don't use the nagle rule for urgent data (or
-	 *	for the final FIN -DaveM).
-	 *
-	 *	Also, Nagle rule does not apply to frames, which
-	 *	sit in the middle of queue (they have no chances
-	 *	to get new data) and if room at tail of skb is
-	 *	not enough to save something seriously (<32 for now).
-	 */
-
-	/* Don't be strict about the congestion window for the
-	 * final FIN frame.  -DaveM
-	 */
-	return (((nonagle&TCP_NAGLE_PUSH) || tp->urg_mode
-		 || !tcp_nagle_check(tp, skb, cur_mss, nonagle)) &&
-		(((tcp_packets_in_flight(tp) + (pkts-1)) < tp->snd_cwnd) ||
-		 (TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN)) &&
-		!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd));
-}
-
 static __inline__ void tcp_check_probe_timer(struct sock *sk, struct tcp_sock *tp)
 {
 	if (!tp->packets_out && !tp->pending)
 		tcp_reset_xmit_timer(sk, TCP_TIME_PROBE0, tp->rto);
 }
 
-static __inline__ int tcp_skb_is_last(const struct sock *sk, 
-				      const struct sk_buff *skb)
-{
-	return skb->next == (struct sk_buff *)&sk->sk_write_queue;
-}
-
-/* Push out any pending frames which were held back due to
- * TCP_CORK or attempt at coalescing tiny packets.
- * The socket must be locked by the caller.
- */
-static __inline__ void __tcp_push_pending_frames(struct sock *sk,
-						 struct tcp_sock *tp,
-						 unsigned cur_mss,
-						 int nonagle)
-{
-	struct sk_buff *skb = sk->sk_send_head;
-
-	if (skb) {
-		if (!tcp_skb_is_last(sk, skb))
-			nonagle = TCP_NAGLE_PUSH;
-		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
-		    tcp_write_xmit(sk, nonagle))
-			tcp_check_probe_timer(sk, tp);
-	}
-	tcp_cwnd_validate(sk, tp);
-}
-
 static __inline__ void tcp_push_pending_frames(struct sock *sk,
 					       struct tcp_sock *tp)
 {
 	__tcp_push_pending_frames(sk, tp, tcp_current_mss(sk, 1), tp->nonagle);
 }
 
-static __inline__ int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp)
-{
-	struct sk_buff *skb = sk->sk_send_head;
-
-	return (skb &&
-		tcp_snd_test(sk, skb, tcp_current_mss(sk, 1),
-			     tcp_skb_is_last(sk, skb) ? TCP_NAGLE_PUSH : tp->nonagle));
-}
-
 static __inline__ void tcp_init_wl(struct tcp_sock *tp, u32 ack, u32 seq)
 {
 	tp->snd_wl1 = seq;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -419,6 +419,135 @@ static inline void tcp_tso_set_push(stru
 		TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_PSH;
 }
 
+static void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb)
+{
+	struct tcp_sock *tp = tcp_sk(sk);
+
+	if (skb->len <= tp->mss_cache_std ||
+	    !(sk->sk_route_caps & NETIF_F_TSO)) {
+		/* Avoid the costly divide in the normal
+		 * non-TSO case.
+		 */
+		skb_shinfo(skb)->tso_segs = 1;
+		skb_shinfo(skb)->tso_size = 0;
+	} else {
+		unsigned int factor;
+
+		factor = skb->len + (tp->mss_cache_std - 1);
+		factor /= tp->mss_cache_std;
+		skb_shinfo(skb)->tso_segs = factor;
+		skb_shinfo(skb)->tso_size = tp->mss_cache_std;
+	}
+}
+
+static inline int tcp_minshall_check(const struct tcp_sock *tp)
+{
+	return after(tp->snd_sml,tp->snd_una) &&
+		!after(tp->snd_sml, tp->snd_nxt);
+}
+
+/* Return 0, if packet can be sent now without violation Nagle's rules:
+ * 1. It is full sized.
+ * 2. Or it contains FIN.
+ * 3. Or TCP_NODELAY was set.
+ * 4. Or TCP_CORK is not set, and all sent packets are ACKed.
+ *    With Minshall's modification: all sent small packets are ACKed.
+ */
+
+static inline int tcp_nagle_check(const struct tcp_sock *tp,
+				  const struct sk_buff *skb, 
+				  unsigned mss_now, int nonagle)
+{
+	return (skb->len < mss_now &&
+		!(TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN) &&
+		((nonagle&TCP_NAGLE_CORK) ||
+		 (!nonagle &&
+		  tp->packets_out &&
+		  tcp_minshall_check(tp))));
+}
+
+/* This checks if the data bearing packet SKB (usually sk->sk_send_head)
+ * should be put on the wire right now.
+ */
+static int tcp_snd_test(struct sock *sk, struct sk_buff *skb,
+			unsigned cur_mss, int nonagle)
+{
+	struct tcp_sock *tp = tcp_sk(sk);
+	int pkts = tcp_skb_pcount(skb);
+
+	if (!pkts) {
+		tcp_set_skb_tso_segs(sk, skb);
+		pkts = tcp_skb_pcount(skb);
+	}
+
+	/*	RFC 1122 - section 4.2.3.4
+	 *
+	 *	We must queue if
+	 *
+	 *	a) The right edge of this frame exceeds the window
+	 *	b) There are packets in flight and we have a small segment
+	 *	   [SWS avoidance and Nagle algorithm]
+	 *	   (part of SWS is done on packetization)
+	 *	   Minshall version sounds: there are no _small_
+	 *	   segments in flight. (tcp_nagle_check)
+	 *	c) We have too many packets 'in flight'
+	 *
+	 * 	Don't use the nagle rule for urgent data (or
+	 *	for the final FIN -DaveM).
+	 *
+	 *	Also, Nagle rule does not apply to frames, which
+	 *	sit in the middle of queue (they have no chances
+	 *	to get new data) and if room at tail of skb is
+	 *	not enough to save something seriously (<32 for now).
+	 */
+
+	/* Don't be strict about the congestion window for the
+	 * final FIN frame.  -DaveM
+	 */
+	return (((nonagle&TCP_NAGLE_PUSH) || tp->urg_mode
+		 || !tcp_nagle_check(tp, skb, cur_mss, nonagle)) &&
+		(((tcp_packets_in_flight(tp) + (pkts-1)) < tp->snd_cwnd) ||
+		 (TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN)) &&
+		!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + tp->snd_wnd));
+}
+
+static inline int tcp_skb_is_last(const struct sock *sk, 
+				  const struct sk_buff *skb)
+{
+	return skb->next == (struct sk_buff *)&sk->sk_write_queue;
+}
+
+/* Push out any pending frames which were held back due to
+ * TCP_CORK or attempt at coalescing tiny packets.
+ * The socket must be locked by the caller.
+ */
+void __tcp_push_pending_frames(struct sock *sk, struct tcp_sock *tp,
+			       unsigned cur_mss, int nonagle)
+{
+	struct sk_buff *skb = sk->sk_send_head;
+
+	if (skb) {
+		if (!tcp_skb_is_last(sk, skb))
+			nonagle = TCP_NAGLE_PUSH;
+		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
+		    tcp_write_xmit(sk, nonagle))
+			tcp_check_probe_timer(sk, tp);
+	}
+	tcp_cwnd_validate(sk, tp);
+}
+
+int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp)
+{
+	struct sk_buff *skb = sk->sk_send_head;
+
+	return (skb &&
+		tcp_snd_test(sk, skb, tcp_current_mss(sk, 1),
+			     (tcp_skb_is_last(sk, skb) ?
+			      TCP_NAGLE_PUSH :
+			      tp->nonagle)));
+}
+
+
 /* Send _single_ skb sitting at the send head. This function requires
  * true push pending frames to setup probe timer etc.
  */
@@ -440,27 +569,6 @@ void tcp_push_one(struct sock *sk, unsig
 	}
 }
 
-void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb)
-{
-	struct tcp_sock *tp = tcp_sk(sk);
-
-	if (skb->len <= tp->mss_cache_std ||
-	    !(sk->sk_route_caps & NETIF_F_TSO)) {
-		/* Avoid the costly divide in the normal
-		 * non-TSO case.
-		 */
-		skb_shinfo(skb)->tso_segs = 1;
-		skb_shinfo(skb)->tso_size = 0;
-	} else {
-		unsigned int factor;
-
-		factor = skb->len + (tp->mss_cache_std - 1);
-		factor /= tp->mss_cache_std;
-		skb_shinfo(skb)->tso_segs = factor;
-		skb_shinfo(skb)->tso_size = tp->mss_cache_std;
-	}
-}
-
 /* Function to create two new TCP segments.  Shrinks the given segment
  * to the specified size and appends a new segment with the rest of the
  * packet to the list.  This won't be called frequently, I hope. 

^ permalink raw reply

* [PATCH 2/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:17 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Fix quick-ack decrementing with TSO.

On each packet output, we call tcp_dec_quickack_mode()
if the ACK flag is set.  It drops tp->ack.quick until
it hits zero, at which time we deflate the ATO value.

When doing TSO, we are emitting multiple packets with
ACK set, so we should decrement tp->ack.quick that many
segments.

Note that, unlike this case, tcp_enter_cwr() should not
take the tcp_skb_pcount(skb) into consideration.  That
function, one time, readjusts tp->snd_cwnd and moves
into TCP_CA_CWR state.

Signed-off-by: David S. Miller <davem@davemloft.net>

00cb08b2ec091f4b461210026392edeaccf31d9c (from 28f78ef8dcc90a2a26499dab76678bd6813d7793)
diff --git a/include/net/tcp.h b/include/net/tcp.h
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -817,11 +817,16 @@ static inline int tcp_ack_scheduled(stru
 	return tp->ack.pending&TCP_ACK_SCHED;
 }
 
-static __inline__ void tcp_dec_quickack_mode(struct tcp_sock *tp)
+static __inline__ void tcp_dec_quickack_mode(struct tcp_sock *tp, unsigned int pkts)
 {
-	if (tp->ack.quick && --tp->ack.quick == 0) {
-		/* Leaving quickack mode we deflate ATO. */
-		tp->ack.ato = TCP_ATO_MIN;
+	if (tp->ack.quick) {
+		if (pkts >= tp->ack.quick) {
+			tp->ack.quick = 0;
+
+			/* Leaving quickack mode we deflate ATO. */
+			tp->ack.ato = TCP_ATO_MIN;
+		} else
+			tp->ack.quick -= pkts;
 	}
 }
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -141,11 +141,11 @@ static inline void tcp_event_data_sent(s
 		tp->ack.pingpong = 1;
 }
 
-static __inline__ void tcp_event_ack_sent(struct sock *sk)
+static __inline__ void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	tcp_dec_quickack_mode(tp);
+	tcp_dec_quickack_mode(tp, pkts);
 	tcp_clear_xmit_timer(sk, TCP_TIME_DACK);
 }
 
@@ -361,7 +361,7 @@ static int tcp_transmit_skb(struct sock 
 		tp->af_specific->send_check(sk, th, skb->len, skb);
 
 		if (tcb->flags & TCPCB_FLAG_ACK)
-			tcp_event_ack_sent(sk);
+			tcp_event_ack_sent(sk, tcp_skb_pcount(skb));
 
 		if (skb->len != tcp_header_size)
 			tcp_event_data_sent(tp, skb, sk);

^ permalink raw reply

* [PATCH 1/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:16 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner
In-Reply-To: <20050606.210846.07641049.davem@davemloft.net>


[TCP]: Simplify SKB data portion allocation with NETIF_F_SG.

The ideal and most optimal layout for an SKB when doing
scatter-gather is to put all the headers at skb->data, and
all the user data in the page array.

This makes SKB splitting and combining extremely simple,
especially before a packet goes onto the wire the first
time.

So, when sk_stream_alloc_pskb() is given a zero size, make
sure there is no skb_tailroom().  This is achieved by applying
SKB_DATA_ALIGN() to the header length used here.

Next, make select_size() in TCP output segmentation use a
length of zero when NETIF_F_SG is true on the outgoing
interface.

Signed-off-by: David S. Miller <davem@davemloft.net>

28f78ef8dcc90a2a26499dab76678bd6813d7793 (from 3f5948fa2cbbda1261eec9a39ef3004b3caf73fb)
diff --git a/include/net/sock.h b/include/net/sock.h
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1130,13 +1130,16 @@ static inline void sk_stream_moderate_sn
 static inline struct sk_buff *sk_stream_alloc_pskb(struct sock *sk,
 						   int size, int mem, int gfp)
 {
-	struct sk_buff *skb = alloc_skb(size + sk->sk_prot->max_header, gfp);
+	struct sk_buff *skb;
+	int hdr_len;
 
+	hdr_len = SKB_DATA_ALIGN(sk->sk_prot->max_header);
+	skb = alloc_skb(size + hdr_len, gfp);
 	if (skb) {
 		skb->truesize += mem;
 		if (sk->sk_forward_alloc >= (int)skb->truesize ||
 		    sk_stream_mem_schedule(sk, skb->truesize, 0)) {
-			skb_reserve(skb, sk->sk_prot->max_header);
+			skb_reserve(skb, hdr_len);
 			return skb;
 		}
 		__kfree_skb(skb);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -775,13 +775,9 @@ static inline int select_size(struct soc
 {
 	int tmp = tp->mss_cache_std;
 
-	if (sk->sk_route_caps & NETIF_F_SG) {
-		int pgbreak = SKB_MAX_HEAD(MAX_TCP_HEADER);
+	if (sk->sk_route_caps & NETIF_F_SG)
+		tmp = 0;
 
-		if (tmp >= pgbreak &&
-		    tmp <= pgbreak + (MAX_SKB_FRAGS - 1) * PAGE_SIZE)
-			tmp = pgbreak;
-	}
 	return tmp;
 }
 
@@ -891,11 +887,6 @@ new_segment:
 					tcp_mark_push(tp, skb);
 					goto new_segment;
 				} else if (page) {
-					/* If page is cached, align
-					 * offset to L1 cache boundary
-					 */
-					off = (off + L1_CACHE_BYTES - 1) &
-					      ~(L1_CACHE_BYTES - 1);
 					if (off == PAGE_SIZE) {
 						put_page(page);
 						TCP_PAGE(sk) = page = NULL;

^ permalink raw reply

* [PATCH 0/9]: TCP: The Road to Super TSO
From: David S. Miller @ 2005-06-07  4:08 UTC (permalink / raw)
  To: netdev; +Cc: herbert, jheffner


Some folks, notable the S2IO guys, get performance degradation
from the Super TSO v2 patch (they get it from the first version
as well).  It's a real pain to spot what causes such things
in such a huge patch... so I started splitting things up in
a very fine grained manner so we can catch regressions more
precisely.

There are several bugs spotted by this first set of 9 patches,
and I'd really appreciate good high-quality testing reports.
Please do not mail such reports privately to me, as some have
done, always include netdev@oss.sgi.com, thanks a lot.

Herbert, I'm CC:'ing you because one of the bugs fixed here
has to do with the TSO header COW'ing stuff you did.  You
missed one case where a skb_header_release() call was needed,
namely tcp_fragment() where it does it's __skb_append().

John, I'm CC:'ing you because there are several cwnd handling
related cures in here.  I did _not_ fix the TSO cwnd growth
bug yet in these patches, but it is at the very top of my
TODO list for my next batch of work on this stuff.  The most
notable fix here is the bogus extra cwnd validation done by
__tcp_push_pending_frames().  That validation should only
occur if we _do_ send some packets, and tcp_write_xmit() takes
care of that just fine.  The other one is that the 'nonagle'
argument to __tcp_push_pending_frames() is clobbered by it's
tcp_skb_is_last() logic, causing TCP_NAGLE_PUSH to be used for
all packets processed by tcp_write_xmit(), whoops...

Please help me review this stuff, thanks.

The patches will show up as followups to this email.

^ permalink raw reply

* Re: RFC: NAPI packet weighting patch
From: Ben Greear @ 2005-06-07  0:08 UTC (permalink / raw)
  To: Mitch Williams
  Cc: Ronciak, John, David S. Miller, mchan, hadi, buytenh, jdmason,
	shemminger, netdev, Robert.Olsson, Venkatesan, Ganesh,
	Brandeburg, Jesse
In-Reply-To: <Pine.CYG.4.58.0506061647340.128@mawilli1-desk2.amr.corp.intel.com>

Mitch Williams wrote:
> 
> On Mon, 6 Jun 2005, Ronciak, John wrote:
> 
> 
>>>	If you force the e1000 driver to do RX replenishment every N
>>>	packets it should reduce the packet drops the same (in the
>>>	single NIC case) as if you reduced the dev->weight to that
>>>	same value N.
>>
>>But this isn't what we are seeing.  Even if we just reduce the weight
>>value to 32 from 64, all of the drops go away.  So there seems to be
>>other things affecting this.
> 
> 
> Some quickie results for everybody -- I've been working on other stuff this
> morning and haven't had much time in the lab.
> 
> Increasing the RX ring to 512 descriptors eliminates dropped packets, but
> performance goes down.  When I mentioned this, John and Jesse both nodded
> and said, "Yep.  That's what happens when the descriptor ring grows past
> one page."
> 
> Reducing the weight to 32 got rid of almost all of the dropped packets
> (down to < 1 per second); reducing it to 20 eliminated all of them.  In
> both cases performance rose as compared to the default weight of 64.
> 
> Tests were run on 2.6.12rc5 on a dual Xeon 2.8GHz PCI-X system.  We run
> Chariot for performance testing, using TCP/IP large file transfers with 10
> Windows 2000 clients.

So is the Linux server reading/writing these large files to/from the disk?

Can you tell us how much performance went down when you increased the
descriptors to 512?

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* RE: RFC: NAPI packet weighting patch
From: Mitch Williams @ 2005-06-06 23:55 UTC (permalink / raw)
  To: Ronciak, John
  Cc: David S. Miller, mchan, hadi, buytenh, Williams, Mitch A, jdmason,
	shemminger, netdev, Robert.Olsson, Venkatesan, Ganesh,
	Brandeburg, Jesse
In-Reply-To: <468F3FDA28AA87429AD807992E22D07E0450C00B@orsmsx408>



On Mon, 6 Jun 2005, Ronciak, John wrote:

> > 	If you force the e1000 driver to do RX replenishment every N
> > 	packets it should reduce the packet drops the same (in the
> > 	single NIC case) as if you reduced the dev->weight to that
> > 	same value N.
>
> But this isn't what we are seeing.  Even if we just reduce the weight
> value to 32 from 64, all of the drops go away.  So there seems to be
> other things affecting this.

Some quickie results for everybody -- I've been working on other stuff this
morning and haven't had much time in the lab.

Increasing the RX ring to 512 descriptors eliminates dropped packets, but
performance goes down.  When I mentioned this, John and Jesse both nodded
and said, "Yep.  That's what happens when the descriptor ring grows past
one page."

Reducing the weight to 32 got rid of almost all of the dropped packets
(down to < 1 per second); reducing it to 20 eliminated all of them.  In
both cases performance rose as compared to the default weight of 64.

Tests were run on 2.6.12rc5 on a dual Xeon 2.8GHz PCI-X system.  We run
Chariot for performance testing, using TCP/IP large file transfers with 10
Windows 2000 clients.

We're still looking at some methods of returning RX resources to the
hardware more often, but we don't have results on that yet.

> I also like your idea about the weight value being adjusted based on
> real work done using some measurable metric.  This seems like a good
> path to explore as well.


Agreed.  I think NAPI can be a lot smarter than it is today.

-Mitch

^ permalink raw reply

* Re: [RFC] Replace scatterlist with crypto_frag
From: Herbert Xu @ 2005-06-06 23:20 UTC (permalink / raw)
  To: David S. Miller; +Cc: hch, jmorris, linux-crypto, netdev
In-Reply-To: <20050606.161814.130845728.davem@davemloft.net>

On Mon, Jun 06, 2005 at 04:18:14PM -0700, David S. Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue, 7 Jun 2005 09:14:05 +1000
> 
> > 1) We use it directly as a scratch buffer for now since the
> > frags are always linearised currently.
> 
> And since skb_shinfo(skb)->nr_frags will be zero, nobody
> will mistakedly look at the contents and interpret it as
> some valid frags.  Right?

Yep.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Fwd: [Bug 4615] Modem connection stalls out.
From: Herbert Xu @ 2005-06-06 23:19 UTC (permalink / raw)
  To: Russell King; +Cc: netdev
In-Reply-To: <20050606224729.B12034@flint.arm.linux.org.uk>

Russell King <rmk@arm.linux.org.uk> wrote:
> 
> The "No buffer space available" looks like the system is running low on
> memory.  Would networking folk concur with that?

It's probably not running low of system memory.  However, it might
be running out of things such as routing cache entries.

Check the dmesg output, it might have a clue on what went wrong.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [RFC] Replace scatterlist with crypto_frag
From: David S. Miller @ 2005-06-06 23:18 UTC (permalink / raw)
  To: herbert; +Cc: hch, jmorris, linux-crypto, netdev
In-Reply-To: <20050606231405.GA7385@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 7 Jun 2005 09:14:05 +1000

> 1) We use it directly as a scratch buffer for now since the
> frags are always linearised currently.

And since skb_shinfo(skb)->nr_frags will be zero, nobody
will mistakedly look at the contents and interpret it as
some valid frags.  Right?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox