Re: [RFC][PATCH 0/3] net: a lighter UDP-Lite (RFC 3828)
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)
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)
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)
[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)
[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