Re: TCP window updates [Was: PROBLEM: Sending mail-attachment]

Andrea Arcangeli (andrea@e-mind.com)
Mon, 8 Mar 1999 14:27:32 +0100 (CET)


On Sun, 7 Mar 1999, David Miller wrote:

> larger patches, because you believe all of your fixes
> are correct, many of them are not. I refuse to have to

Could you tell me what's wrong with this patch (excexpt the #if 1 that
should be set to #if 0)?

Index: tcp_input.c
===================================================================
RCS file: /var/cvs/linux/net/ipv4/tcp_input.c,v
retrieving revision 1.1.1.6
diff -u -r1.1.1.6 tcp_input.c
--- tcp_input.c 1999/02/23 16:48:26 1.1.1.6
+++ tcp_input.c 1999/03/08 13:12:15
@@ -1354,7 +1354,7 @@
}
}

-static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
+static int tcp_data_queue(struct sock *sk, struct sk_buff *skb)
{
struct sk_buff *skb1;
struct tcp_opt *tp = &(sk->tp_pinfo.af_tcp);
@@ -1384,7 +1384,7 @@
tp->pred_flags = htonl(((tp->tcp_header_len >> 2) << 28) |
(0x10 << 16) |
tp->snd_wnd);
- return;
+ return 0;
}

/* An old packet, either a retransmit or some packet got lost. */
@@ -1393,7 +1393,7 @@
SOCK_DEBUG(sk, "retransmit received: seq %X\n", TCP_SKB_CB(skb)->seq);
tcp_enter_quickack_mode(tp);
kfree_skb(skb);
- return;
+ return 1;
}

if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
@@ -1415,7 +1415,7 @@
SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
tp->rcv_nxt, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);

- if (skb_peek(&tp->out_of_order_queue) == NULL) {
+ if (!skb_queue_len(&tp->out_of_order_queue)) {
/* Initial out of order segment, build 1 SACK. */
if(tp->sack_ok) {
tp->num_sacks = 1;
@@ -1458,6 +1458,7 @@
}
}
}
+ return 1;
}


@@ -1471,6 +1472,7 @@
{
struct tcphdr *th;
struct tcp_opt *tp = &(sk->tp_pinfo.af_tcp);
+ int ret;

th = skb->h.th;
skb_pull(skb, th->doff*4);
@@ -1484,16 +1486,35 @@
* Make sure to do this before moving snd_nxt, otherwise
* data might be acked for that we don't have enough room.
*/
- if (atomic_read(&sk->rmem_alloc) > sk->rcvbuf) {
- if (prune_queue(sk) < 0) {
- /* Still not enough room. That can happen when
- * skb->true_size differs significantly from skb->len.
+ bigger_rcvbuf:
+ if (atomic_read(&sk->rmem_alloc) >= sk->rcvbuf)
+ {
+ extern __u32 sysctl_rmem_max;
+ int good_rcvbuf = min(tp->pmtu_cookie * 3, sysctl_rmem_max);
+#if 1
+ printk(KERN_DEBUG "tcp_data: rmem %d, rcvbuf %d, len %d, "
+ "skb->len %d, skb->truesize %d, good_rcvbuf %d\n",
+ atomic_read(&sk->rmem_alloc), sk->rcvbuf, len,
+ skb->len, skb->truesize, good_rcvbuf);
+#endif
+ if (sk->rcvbuf < good_rcvbuf)
+ {
+ sk->rcvbuf = good_rcvbuf;
+ goto bigger_rcvbuf;
+ }
+ /*
+ * It seems we can't queue/receive the packet, so try to
+ * shrink something if possible. -arca
+ */
+ if (prune_queue(sk) < 0)
+ /*
+ * Bad: we aren't been able to shrink the out of
+ * order queue so we _must_ drop the packet.
*/
return 0;
- }
}

- tcp_data_queue(sk, skb);
+ ret = tcp_data_queue(sk, skb);

if (before(tp->rcv_nxt, tp->copied_seq)) {
printk(KERN_DEBUG "*** tcp.c:tcp_data bug acked < copied\n");
@@ -1503,7 +1524,7 @@
/* Above, tcp_data_queue() increments delayed_acks appropriately.
* Now tell the user we may have some data.
*/
- if (!sk->dead) {
+ if (!sk->dead && !ret) {
SOCK_DEBUG(sk, "Data wakeup.\n");
sk->data_ready(sk,0);
}
Index: tcp_ipv4.c
===================================================================
RCS file: /var/cvs/linux/net/ipv4/tcp_ipv4.c,v
retrieving revision 1.1.1.4
retrieving revision 1.1.2.6
diff -u -r1.1.1.4 -r1.1.2.6
--- tcp_ipv4.c 1999/02/20 15:43:23 1.1.1.4
+++ tcp_ipv4.c 1999/03/07 18:09:32 1.1.2.6
@@ -566,7 +566,7 @@
struct sk_buff *buff;
struct rtable *rt;
u32 daddr, nexthop;
- int tmp;
+ int tmp, pmtu;

if (sk->state != TCP_CLOSE)
return(-EISCONN);
@@ -642,8 +642,9 @@
/* Reset mss clamp */
tp->mss_clamp = ~0;

+ pmtu = rt->u.dst.pmtu;
if (!ip_dont_fragment(sk, &rt->u.dst) &&
- rt->u.dst.pmtu > 576 && rt->rt_dst != rt->rt_gateway) {
+ pmtu > 576 && rt->rt_dst != rt->rt_gateway) {
/* Clamp mss at maximum of 536 and user_mss.
Probably, user ordered to override tiny segment size
in gatewayed case.
@@ -651,7 +652,17 @@
tp->mss_clamp = max(tp->user_mss, 536);
}

- tcp_connect(sk, buff, rt->u.dst.pmtu);
+ tcp_connect(sk, buff, pmtu);
+
+ /*
+ * set a rasonable sndbuf/rcvbuf to improve performances and
+ * make sure to avoid future deadlocks. -arca
+ */
+ pmtu *= 3;
+ if (sk->rcvbuf < pmtu)
+ sk->rcvbuf = min(pmtu, sysctl_rmem_max);
+ if (sk->sndbuf < pmtu)
+ sk->sndbuf = min(pmtu, sysctl_wmem_max);
return 0;
}

If something is wrong with this patch I'd like to know why because I
passed the whole last three days of my spare time into trying to fixing
this bug with the help of Andi and Alan. I hope I am not causing you a
waste of time.

The patch above incidentally also avoid us to wakup the reader if we
received an out of order packet, maybe this is plain buggy but I don't
think because if we got an out of order packet it menas we are always
going to receive the gap-packet very soon.

Thanks.

Andrea Arcangeli

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/