ipv4 skbuff locking scope

Hello,

This fixes tests of a socket buffer done without holding the lock. tcp_data_wait() and wait_for_tcp_memory() both had unguarded refs in their sleep conditionals.

Tom

@@ -204,6 +204,9 @@
  *		Andi Kleen 	:	Make poll agree with SIGIO
  *	Salvatore Sanfilippo	:	Support SO_LINGER with linger == 1 and
  *					lingertime == 0 (RFC 793 ABORT Call)
+ *              Tom Leete       :       Fix locking scope in
+ *                                      wait_for_tcp_memory, tcp_data_wait
+ *
  *					
  *		This program is free software; you can redistribute it and/or
  *		modify it under the terms of the GNU General Public License

@@ -871,10 +874,11 @@
 			break;
 		if (sk->err)
 			break;
-		release_sock(sk);
-		if (!tcp_memory_free(sk) || vm_wait)
+		if (!tcp_memory_free(sk) || vm_wait) {
+			release_sock(sk);
 			current_timeo = schedule_timeout(current_timeo);
-		lock_sock(sk);
+			lock_sock(sk);
+		}
 		if (vm_wait) {
 			if (timeo != MAX_SCHEDULE_TIMEOUT &&
 			    (timeo -= vm_wait-current_timeo) < 0)

@@ -1273,12 +1277,12 @@
 	__set_current_state(TASK_INTERRUPTIBLE);
 
 	set_bit(SOCK_ASYNC_WAITDATA, &sk->socket->flags);
-	release_sock(sk);
 
-	if (skb_queue_empty(&sk->receive_queue))
+	if (skb_queue_empty(&sk->receive_queue)){
+		release_sock(sk);
 		timeo = schedule_timeout(timeo);
-
-	lock_sock(sk);
+		lock_sock(sk);
+	}
 	clear_bit(SOCK_ASYNC_WAITDATA, &sk->socket->flags);
 
 	remove_wait_queue(sk->sleep, &wait);

Date: Mon, 30 Oct 2000 17:24:24 -0500
From: Tom Leete <[email protected]>

This fixes tests of a socket buffer done without holding the lock. tcp_data_wait() and wait_for_tcp_memory() both had unguarded refs in their sleep conditionals.

These are not buggy at all, see the discussion which took place here over the past few days.

Look, if the sleep condition test "races" due to not holding the lock, the schedule() just returns because if the sleep condidion test passes then by definition this means we were also woken up, see?

BTW, while we're on the topic of people not understanding the networking locking and proposing bogus patches, does anyone know who sent the bogon IP tunneling locking "fixes" to Linus behind my back?

They were crap too, and I had to revert them in test10-pre7. It's another case of people just not understanding how the code works and thus that it is correct without any changes.

Please send such fixes to me, and I'll set you straight with a description as to why your change is unnecessary :-)

Later,
David S. Miller
[email protected]

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

"David S. Miller" <[email protected]> said:

[...]

Could you please Cc: them into the kernel? ;-)

Horst von Brand                             [email protected]
Casilla 9G, Vin~a del Mar, Chile                               +56 32 672616

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

...

You can prevent future "fixes" by putting your comments in the code.

/* like this */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I'll post traces privately. It was my lockups that got Rick interested.

Would you explain what is accomplished by toggling the lock every time through? What breaks by not doing so?

Not me, but see below.

If it's perfect why can't I use it without locking up the machine? BTW, I'm currently running my patch. I can now flood ping 100000 packets in either direction. With unmodified tcp that is a red switcher (reliably hard locks all i/o including the serial console).

Perhaps that's why somebody wants to bypass you.

Tom Leete
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Date: Mon, 30 Oct 2000 22:18:07 -0500
From: Tom Leete <[email protected]>

Would you explain what is accomplished by toggling the lock every time through? What breaks by not doing so?

Incoming packets are not processed while the lock is held, they get queued to the backlog. When the lock is released, the backlog packets get processed.

For a TCP sender, these backlog packets are ACKs making room in the send buffer so that the application can put more data into the send buffer.

Later,
David S. Miller
[email protected]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/