Re: [RFC][PATCH 0/3] net: a lighter UDP-Lite (RFC 3828)

2006-08-28 Thread Arnaldo Carvalho de Melo

On 8/28/06, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

Quoting Arnaldo Carvalho de Melo:
|  Avoid these changes to reduce patch file size, please

I apologize for the bad patch format - I am revising the entire
patch to improve readability and will resend.


No need for apologies and thanks for taking my suggestions into account.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 0/3] net: a lighter UDP-Lite (RFC 3828)

2006-08-28 Thread gerrit
Quoting Arnaldo Carvalho de Melo:
|  Avoid these changes to reduce patch file size, please

I apologize for the bad patch format - I am revising the entire
patch to improve readability and will resend.

- Gerrit
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 0/3] net: a lighter UDP-Lite (RFC 3828)

2006-08-28 Thread Arnaldo Carvalho de Melo

On 8/28/06, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

[NET/IPv4]: update for udp.c only, to match 2.6.18-rc4-mm3

This is an update only, as the previous patch can not cope
with recent changes to udp.c (all other files remain the same).

Up-to-date, complete patches can always be taken from
http://www.erg.abdn.ac.uk/users/gerrit/udp-lite/files/udplite_linux.tar.gz

Signed-off-by: Gerrit Renker <[EMAIL PROTECTED]>
---
  udp.c |  606 
--
 1 file changed, 410 insertions(+), 196 deletions(-)


diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 514c1e9..4ddd8e6 100644




@@ -731,12 +801,12 @@ out:
 }

 /*
- * IOCTL requests applicable to the UDP protocol
+ * IOCTL requests applicable to the UDP(-Lite) protocol
  */


Avoid these changes to reduce patch file size, please


-
+
 int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
 {
-   switch(cmd)
+   switch(cmd)


Ditto



-/*
- * This should be easy, if there is something there we
- * return it, otherwise we block.
+/**
+ * udp_recvmsg  -  generic UDP/-Lite receive processing
+ *
+ * This routine is udplite-aware and works for both protocols.




@@ -980,7 +1055,11 @@ #else
 #endif
 }

-/* returns:
+/**
+ * udp_queue_rcv_skb  -  receive queue processing
+ *
+ * This routine is udplite-aware and works on both sockets.




if (up->encap_type) {
@@ -1010,7 +1087,7 @@ static int udp_queue_rcv_skb(struct sock
 * If it's an encapsulateed packet, then pass it to the
 * IPsec xfrm input and return the response
 * appropriately.  Otherwise, just fall through and
-* pass this up the UDP socket.
+* pass this up the UDP/-Lite socket.
 */



-   /* FALLTHROUGH -- it's a UDP Packet */
+   /* FALLTHROUGH -- it's a UDP/-Lite Packet */
}




 /*
- * All we need to do is get the socket, and then do a checksum.
+ * All we need to do is get the socket, and then do a checksum.
  */
-


Huh, what was this one? trailing whitespace? Can you leave this for
another cset doing just the reformatting?


@@ -1219,7 +1363,7 @@ static int udp_destroy_sock(struct sock
 }

 /*
- * Socket option code for UDP
+ * Socket option code for UDP and UDP-Lite (shared).
  */



 #endif
+
 /**
- * udp_poll - wait for a UDP event.
+ * udp_poll  -  wait for a UDP(-Lite) event.


See next comment


  * @file - file struct
  * @sock - socket
  * @wait - poll table
@@ -1348,11 +1528,14 @@ #endif
  * then it could get return from select indicating data available
  * but then block when reading it. Add special case code
  * to work around these arguably broken applications.
+ *
+ * The routine is udplite-aware and works for both protocols.


I guess these comments can go as well, as one can quickly realise the
functions handles UDP lite with all the "IS_UDPLITE(sk)" calls and
"is_{udp}lite" variables :-)


  */
 unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait)
 {
unsigned int mask = datagram_poll(file, sock, wait);
struct sock *sk = sock->sk;
+   int is_lite = IS_UDPLITE(sk);


Regards,

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 0/3] net: a lighter UDP-Lite (RFC 3828)

2006-08-28 Thread gerrit
[NET/IPv4]: update for udp.c only, to match 2.6.18-rc4-mm3

This is an update only, as the previous patch can not cope
with recent changes to udp.c (all other files remain the same).

Up-to-date, complete patches can always be taken from 
http://www.erg.abdn.ac.uk/users/gerrit/udp-lite/files/udplite_linux.tar.gz

Signed-off-by: Gerrit Renker <[EMAIL PROTECTED]>
---
  udp.c |  606 
--
 1 file changed, 410 insertions(+), 196 deletions(-)


diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 514c1e9..4ddd8e6 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -92,10 +92,8 @@ #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -121,7 +119,19 @@ DEFINE_RWLOCK(udp_hash_lock);
 /* Shared by v4/v6 udp. */
 int udp_port_rover;
 
-static int udp_v4_get_port(struct sock *sk, unsigned short snum)
+/* the extensions for UDP-Lite (RFC 3828) */
+#include "udplite.c"
+
+/**
+ * __udp_get_port  -  find an unbound UDP(-Lite) port
+ *
+ * @sk: udp_sock
+ * @snum:   port number to look up
+ * @udptable:   hash list table, must be of UDP_HTABLE_SIZE
+ * @port_rover: pointer to record of last unallocated port
+ */
+int __udp_get_port(struct sock *sk, unsigned short snum,
+ struct hlist_head udptable[], int *port_rover)
 {
struct hlist_node *node;
struct sock *sk2;
@@ -131,16 +141,16 @@ static int udp_v4_get_port(struct sock *
if (snum == 0) {
int best_size_so_far, best, result, i;
 
-   if (udp_port_rover > sysctl_local_port_range[1] ||
-   udp_port_rover < sysctl_local_port_range[0])
-   udp_port_rover = sysctl_local_port_range[0];
+   if (*port_rover > sysctl_local_port_range[1] ||
+   *port_rover < sysctl_local_port_range[0])
+   *port_rover = sysctl_local_port_range[0];
best_size_so_far = 32767;
-   best = result = udp_port_rover;
+   best = result = *port_rover;
for (i = 0; i < UDP_HTABLE_SIZE; i++, result++) {
struct hlist_head *list;
int size;
 
-   list = &udp_hash[result & (UDP_HTABLE_SIZE - 1)];
+   list = &udptable[result & (UDP_HTABLE_SIZE - 1)];
if (hlist_empty(list)) {
if (result > sysctl_local_port_range[1])
result = sysctl_local_port_range[0] +
@@ -162,16 +172,16 @@ static int udp_v4_get_port(struct sock *
result = sysctl_local_port_range[0]
+ ((result - 
sysctl_local_port_range[0]) &
   (UDP_HTABLE_SIZE - 1));
-   if (!udp_lport_inuse(result))
+   if (! __udp_lport_inuse(result, udptable))
break;
}
if (i >= (1 << 16) / UDP_HTABLE_SIZE)
goto fail;
 gotit:
-   udp_port_rover = snum = result;
+   *port_rover = snum = result;
} else {
sk_for_each(sk2, node,
-   &udp_hash[snum & (UDP_HTABLE_SIZE - 1)]) {
+   &udptable[snum & (UDP_HTABLE_SIZE - 1)]) {
struct inet_sock *inet2 = inet_sk(sk2);
 
if (inet2->num == snum &&
@@ -189,7 +199,7 @@ gotit:
}
inet->num = snum;
if (sk_unhashed(sk)) {
-   struct hlist_head *h = &udp_hash[snum & (UDP_HTABLE_SIZE - 1)];
+   struct hlist_head *h = &udptable[snum & (UDP_HTABLE_SIZE - 1)];
 
sk_add_node(sk, h);
sock_prot_inc_use(sk->sk_prot);
@@ -202,6 +212,11 @@ fail:
return 1;
 }
 
+static __inline__ int udp_v4_get_port(struct sock *sk, unsigned short snum)
+{
+   return  __udp_get_port(sk, snum, udp_hash, &udp_port_rover);
+}
+
 static void udp_v4_hash(struct sock *sk)
 {
BUG();
@@ -217,18 +232,24 @@ static void udp_v4_unhash(struct sock *s
write_unlock_bh(&udp_hash_lock);
 }
 
-/* UDP is nearly always wildcards out the wazoo, it makes no sense to try
- * harder than this. -DaveM
+/**
+ * __udp_lookup  -  find UDP(-Lite) socket
+ *
+ * @udptable:   hash list table, must be of UDP_HTABLE_SIZE
+ *
+ * UDP nearly always wildcards out the wazoo, it makes no sense to try
+ * harder than this. -DaveM
  */
-static struct sock *udp_v4_lookup_longway(u32 saddr, u16 sport,
- u32 daddr, u16 dport, int dif)
+struct sock *__udp_lookup(u32 saddr, u16 sport, u32 daddr, u16 dport, int dif,
+ struct hlist_head udptable[])
 {
struct sock *sk, *result = NULL;
   

[RFC][PATCH 0/3] net: a lighter UDP-Lite (RFC 3828)

2006-08-23 Thread gerrit
[NET/IPV4]: a lighter UDP-Lite (RFC 3828)

This is a revised RFC resubmission of the UDP-Lite code which, thanks
to suggestions by David Miller, is now drastically reduced in size:

   ``A fully functional UDP-Lite module in a mere 209 lines !''

I feel that not much more can be removed without making the code obfuscated,
but would like to challenge people on this list to look out for further 
possible integration and reductions. 

I would further like to hear suggestions for a common naming scheme, after 
some of the UDP functions have been made generic, shared between both UDP
and UDP-Lite. 

I will wait with the UDP(-Lite)v6 part until feedback and comments have been
received: the v6-side will mirror the format of the v4-side.

To get a quick idea of what is happening, it is best to start with udplite.c,
since this also lists all the shared functions. This file is #included into 
udp.c -- I did want to keep functionally different blocks of code logically 
separate, but could not see the need for separate compilation.

A detailed changelog is included below. 

The code has been tested over several days on i686, i386-SMP, AMD,
and sparc64 platforms; using various userland and kernel applications
such as multicast streaming, DNS, socket programs, NFS client/server
(different file sizes); and on hardware with TX/RX UDP checksums (tg3).

Enclosed patch can be applied to Torvald's tree. Application code for testing 
is on
http://www.erg.abdn.ac.uk/users/gerrit/udp-lite/files/udplite_linux.tar.gz


*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*
Important things that need to be resolved
*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*

a)  Naming scheme: Several functions are now generic, shared between UDP and 
UDP-Lite. 
They have not been given new names so far. Which naming scheme should be 
used ???
[e.g.  `udpl_checksum_complete() instead of `udp_checksum_complete()' ?] 

b)  udp_v{4,6}_get_port(): raised earlier, this function appears almost 
identical in
two places. There has been discussion, resubmission, but no final opinion 
yet. 
Can people please decide whether the suggested integration is OK or not -- 
the single
get_port algorithm now has a total of four customers: udp4, udplite4, udp6, 
udplite6.

c)  Code cosmetics: I have left out any cosmetical changes for later, to 
minimize patch 
size. But eventually I would like to tidy up the code, in particular add 
more 
documentation to the structs and some of the (shared) functions. 
Suggestions ?

d)  Shared udp_hash_lock: Is it worth to implement separate rwlocks for UDP and 
UDP-Lite?
This would make the code quite a lot more complicated and disadvantages 
will only occur 
in the borderline case when many UDP applications have to compete at the 
same time with
many UDP-Lite applications. But will this result in noticeable performance 
loss at all?

 C h a n g e l o g

1/ Code integration.

 The patch follows David's suggestions. Additionally, the implementation
 was made simpler by exploiting the new `pcflag' member which
struct udp_sock
 now contains. This flag can only be set by UDP-Lite and so uniquely
 distinguishes UDP and UDP-Lite sockets. On UDP sockets, pcflag will
 always be 0 since the structure is zeroed out upon allocation.


2/ No separate UDP-Lite header.

 UDP-Lite does not really define a new header structure, rather it 
re-interprets the 
 `len' header field of UDP with a different semantics. Therefore, a separate 
`struct 
 udplitehdr' is not really  necessary  and hides the fact that 75% of the 
header 
 structures have exactly the same meaning.  Thus UDP-Lite now also uses `struct 
udphdr', 
 the semantic difference is taken care of by the code.


3/ Code-sharing.

 The following functions can now be shared due to reliance on common structures:
   * udp_disconnect() (thanks to unified struct udp_sock )
   * udp_v4_mcast_next()  (thanks to unified struct udp_sock )
   * udp_getsockopt() (thanks to unified struct udp_sock )
   * do_udp_getsockopt()  (thanks to unified struct udp_sock )
   * compat_udp_getsockopt()  (thanks to unified struct udp_sock )
   * udp_encap_rcv()  (thanks to unified struct udphdr )
   * udp_ioctl()  (thanks to unifying both structures)

 The following functions have been turned into parameterised ones:

   * udp_v4_get_port()  -  parameterised as __udp_get_port()
   * udp_v4_lookup()-  parameterised as __udp_lookup()
   * udp_err()  -  parameterised as __udp_err()
   * udp_v4_mcast_deliver() -  parameterised as __udp_mcast_deliver()
 This was possible thanks to common use of udp_v4_mcast_next(see above).
   * udp_lport_inuse()  -  parameterised as __udp_lport_inuse()
 This function is unnecessary in net/udp.h ! See earlier patch / discussion
 on udp_get_port() in net/i