knfsd: revised patches to fix IPv6/sunrpc link error and other fixes
Subsystems (4)
PATCH 1/1 knfsd: revised patches to fix IPv6/sunrpc link error and other fixes
Revised version of previous patches:
- Fixed the 'nfsd hang when stopped' problem
- Dropped the 'avoid checking udp checksum' as I'm no longer convinced it is the right way to go.
- Added a paranoia check that the control message returned by recvfrom is in-fact a PKTINFO message.
Tested that a udp mount still works, and the the nfsd server can be stopped and restarted successfully. Found that it still doesn't if there is active traffic so fixed that.
Also patch from Greg Banks to make the thread-pool stuff non-default as it is not optimal on all machines, and often needs tuning where it can be optimal.
Thanks,
NeilBrown
[PATCH 001 of 4] knfsd: Use recv_msg to get peer address for NFSD instead of code-copying [PATCH 002 of 4] knfsd: Remove CONFIG_IPV6 ifdefs from sunrpc server code. [PATCH 003 of 4] knfsd: Fix recently introduced problem with shutting down a busy NFS server. [PATCH 004 of 4] knfsd: Provide sunrpc pool_mode module option
PATCH 1/1 knfsd: Use recv_msg to get peer address for NFSD instead of code-copying
The sunrpc server code needs to know the source and destination address for UDP packets so it can reply properly.
It currently copies code out of the network stack to pick the pieces out of the skb.
This is ugly and causes compile problems with the IPv6 stuff.
So, rip that out and use recv_msg instead. This is a much cleaner interface, but has a slight cost in that the checksum is now checked before the copy, so we don't benefit from doing both at the same time. This can probably be fixed.
### Diffstat output
./net/sunrpc/svcsock.c | 72 +++++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 32 deletions(-)
net/sunrpc/svcsock.c +40 -32
@@ -721,45 +721,23 @@ svc_write_space(struct sock *sk) } } -static void svc_udp_get_sender_address (struct svc_rqst *rqstp, -struct sk_buff *skb) +static inline void svc_udp_get_dest_address (struct svc_rqst *rqstp, +struct cmsghdr *cmh) {switch (rqstp->rq_sock->sk_sk->sk_family) {case AF_INET : { -/* this seems to come from net/ipv4/udp.c:udp_recvmsg */ -struct sockaddr_in *sin =svc_addr_in (rqstp); - - sin->sin_family =AF_INET ; - sin->sin_port = skb->h.uh->source; - sin->sin_addr.s_addr = skb->nh.iph->saddr; - rqstp->rq_addrlen =sizeof (struct sockaddr_in ); -/* Remember which interface received this request */ - rqstp->rq_daddr.addr.s_addr = skb->nh.iph->daddr; - } +struct in_pktinfo *pki =CMSG_DATA (cmh); + rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;break ; + }#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE )case AF_INET6 : { -/* this is derived from net/ipv6/udp.c:udpv6_recvmesg */ -struct sockaddr_in6 *sin6 =svc_addr_in6 (rqstp); - - sin6->sin6_family =AF_INET6 ; - sin6->sin6_port = skb->h.uh->source; - sin6->sin6_flowinfo =0 ; - sin6->sin6_scope_id =0 ; -if (ipv6_addr_type (&sin6->sin6_addr) & -IPV6_ADDR_LINKLOCAL ) - sin6->sin6_scope_id =IP6CB (skb)->iif; -ipv6_addr_copy (&sin6->sin6_addr, - &skb->nh.ipv6h->saddr); - rqstp->rq_addrlen =sizeof (struct sockaddr_in ); -/* Remember which interface received this request */ -ipv6_addr_copy (&rqstp->rq_daddr.addr6, - &skb->nh.ipv6h->saddr); - } +struct in6_pktinfo *pki =CMSG_DATA (cmh); +ipv6_addr_copy (&rqstp->rq_daddr.addr6, &pki->ipi6_addr);break ; + }#endif } -return ; }/*
@@ -771,7 +749,15 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)struct svc_sock *svsk = rqstp->rq_sock;struct svc_serv *serv = svsk->sk_server;struct sk_buff *skb; +char buffer[CMSG_SPACE (sizeof (union svc_pktinfo_u ))]; +struct cmsghdr *cmh = (struct cmsghdr *)buffer;int err, len; +struct msghdr msg = { + .msg_name =svc_addr (rqstp), + .msg_control = cmh, + .msg_controllen =sizeof (buffer), + .msg_flags =MSG_DONTWAIT , + };if (test_and_clear_bit (SK_CHNGBUF , &svsk->sk_flags))/* udp sockets need large rcvbuf as all pending
@@ -797,7 +783,9 @@ svc_udp_recvfrom(struct svc_rqst *rqstp) }clear_bit (SK_DATA , &svsk->sk_flags); -while ((skb =skb_recv_datagram (svsk->sk_sk,0 ,1 , &err)) ==NULL ) { +while ((err ==kernel_recvmsg (svsk->sk_sock, &msg,NULL , +0 ,0 ,MSG_PEEK |MSG_DONTWAIT )) <0 || + (skb =skb_recv_datagram (svsk->sk_sk,0 ,1 , &err)) ==NULL ) {if (err == -EAGAIN ) {svc_sock_received (svsk);return err;
@@ -805,6 +793,7 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)/* possibly an icmp error */ dprintk ("svc: recvfrom returned error %d\n" , -err); } + rqstp->rq_addrlen =sizeof (rqstp->rq_addr);if (skb->tstamp.off_sec ==0 ) {struct timeval tv;
@@ -827,7 +816,16 @@ svc_udp_recvfrom(struct svc_rqst *rqstp) rqstp->rq_prot =IPPROTO_UDP ; -svc_udp_get_sender_address (rqstp, skb); +if (cmh->cmsg_level !=IPPROTO_IP || + cmh->cmsg_type !=IP_PKTINFO ) { +if (net_ratelimit ()) +printk ("rpcsvc: received unknown control message:" +"%d/%d\n" , + cmh->cmsg_level, cmh->cmsg_type); +skb_free_datagram (svsk->sk_sk, skb); +return 0 ; + } +svc_udp_get_dest_address (rqstp, cmh);if (skb_is_nonlinear (skb)) {/* we have to copy */
@@ -884,6 +882,9 @@ svc_udp_sendto(struct svc_rqst *rqstp)static void svc_udp_init (struct svc_sock *svsk) { +int one =1 ; +mm_segment_t oldfs; + svsk->sk_sk->sk_data_ready = svc_udp_data_ready; svsk->sk_sk->sk_write_space = svc_write_space; svsk->sk_recvfrom = svc_udp_recvfrom;
@@ -899,6 +900,13 @@ svc_udp_init(struct svc_sock *svsk)set_bit (SK_DATA , &svsk->sk_flags);/* might have come in before data_ready set up */ set_bit (SK_CHNGBUF , &svsk->sk_flags); + + oldfs =get_fs (); +set_fs (KERNEL_DS ); +/* make sure we get destination address info */ + svsk->sk_sock->ops->setsockopt (svsk->sk_sock,IPPROTO_IP ,IP_PKTINFO , + (char __user *)&one,sizeof (one)); +set_fs (oldfs); }/*
PATCH 1/1 knfsd: Remove CONFIG_IPV6 ifdefs from sunrpc server code.
In reply to NeilBrownThey don't really save that much, and aren't worth the hassle.
### Diffstat output
./include/linux/sunrpc/svc.h | 2 -- ./net/sunrpc/svcsock.c | 13 +++---------- 2 files changed, 3 insertions(+), 12 deletions(-)
include/linux/sunrpc/svc.h +0 -2
@@ -194,9 +194,7 @@ static inline void svc_putu32(struct kveunion svc_addr_u {struct in_addr addr; -#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE )struct in6_addr addr6; -#endif };/*
net/sunrpc/svcsock.c +3 -10
@@ -131,13 +131,13 @@ static char *__svc_print_addr(struct socNIPQUAD (((struct sockaddr_in *) addr)->sin_addr), htons(((struct sockaddr_in *) addr)->sin_port));break ; -#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE ) +case AF_INET6 :snprintf (buf, len,"%x:%x:%x:%x:%x:%x:%x:%x, port=%u" ,NIP6 (((struct sockaddr_in6 *) addr)->sin6_addr),htons (((struct sockaddr_in6 *) addr)->sin6_port));break ; -#endif +default :snprintf (buf, len,"unknown address type: %d" , addr->sa_family);break ;
@@ -449,9 +449,7 @@ svc_wake_up(struct svc_serv *serv)union svc_pktinfo_u {struct in_pktinfo pkti; -#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE )struct in6_pktinfo pkti6; -#endif };static void svc_set_cmsg_data (struct svc_rqst *rqstp,struct cmsghdr *cmh)
@@ -467,7 +465,7 @@ static void svc_set_cmsg_data(struct svc cmh->cmsg_len =CMSG_LEN (sizeof (*pki)); }break ; -#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE ) +case AF_INET6 : {struct in6_pktinfo *pki =CMSG_DATA (cmh);
@@ -479,7 +477,6 @@ static void svc_set_cmsg_data(struct svc cmh->cmsg_len =CMSG_LEN (sizeof (*pki)); }break ; -#endif }return ; }
@@ -730,13 +727,11 @@ static inline void svc_udp_get_dest_addr rqstp->rq_daddr.addr.s_addr = pki->ipi_spec_dst.s_addr;break ; } -#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE )case AF_INET6 : {struct in6_pktinfo *pki =CMSG_DATA (cmh);ipv6_addr_copy (&rqstp->rq_daddr.addr6, &pki->ipi6_addr);break ; } -#endif } }
@@ -985,11 +980,9 @@ static inline int svc_port_is_privilegedcase AF_INET :return ntohs (((struct sockaddr_in *)sin)->sin_port) <PROT_SOCK ; -#if defined(CONFIG_IPV6 ) || defined(CONFIG_IPV6_MODULE )case AF_INET6 :return ntohs (((struct sockaddr_in6 *)sin)->sin6_port) <PROT_SOCK ; -#endif default :return 0 ; }
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
PATCH 1/1 knfsd: Fix recently introduced problem with shutting down a busy NFS server.
In reply to NeilBrownWhen the last thread of nfsd exits, it shuts down all related sockets. It currently uses svc_close_socket to do this, but that only is immediately effective if the socket is not SK_BUSY.
If the socket is busy - i.e. if a request has arrived that has not yet been processes - svc_close_socket is not effective and the shutdown process spins.
So create a new svc_force_close_socket which removes the SK_BUSY flag is set and then calls svc_close_socket.
Also change some open-codes loops in svc_destroy to use list_for_each_entry_safe.
### Diffstat output
./include/linux/sunrpc/svcsock.h | 2 +- ./net/sunrpc/svc.c | 21 +++++++++------------ ./net/sunrpc/svcsock.c | 16 +++++++++++++++- 3 files changed, 25 insertions(+), 14 deletions(-)
include/linux/sunrpc/svcsock.h +1 -1
@@ -66,7 +66,7 @@ struct svc_sock {* Function prototypes. */ int svc_makesock (struct svc_serv *,int ,unsigned short ,int flags); -void svc_close_socket (struct svc_sock *); +void svc_force_close_socket (struct svc_sock *);int svc_recv (struct svc_rqst *,long );int svc_send (struct svc_rqst *);void svc_drop (struct svc_rqst *);
net/sunrpc/svc.c +9 -12
@@ -367,6 +367,7 @@ void svc_destroy(struct svc_serv *serv) {struct svc_sock *svsk; +struct svc_sock *tmp; dprintk("svc: svc_destroy(%s, %d)\n", serv->sv_program->pg_name,
@@ -382,21 +383,17 @@ svc_destroy(struct svc_serv *serv)del_timer_sync (&serv->sv_temptimer); -while (!list_empty (&serv->sv_tempsocks)) { - svsk =list_entry (serv->sv_tempsocks.next, - struct svc_sock, - sk_list); -svc_close_socket (svsk); - } +list_for_each_entry_safe (svsk, tmp, &serv->sv_tempsocks, sk_list) +svc_force_close_socket (svsk); +if (serv->sv_shutdown) serv->sv_shutdown (serv); -while (!list_empty (&serv->sv_permsocks)) { - svsk =list_entry (serv->sv_permsocks.next, - struct svc_sock, - sk_list); -svc_close_socket (svsk); - } +list_for_each_entry_safe (svsk, tmp, &serv->sv_permsocks, sk_list) +svc_force_close_socket (svsk); + +BUG_ON (!list_empty (&serv->sv_permsocks)); +BUG_ON (!list_empty (&serv->sv_tempsocks));cache_clean_deferred (serv);
net/sunrpc/svcsock.c +15 -1
@@ -82,6 +82,7 @@ static void svc_delete_socket(struct svstatic void svc_udp_data_ready (struct sock *,int );static int svc_udp_recvfrom (struct svc_rqst *);static int svc_udp_sendto (struct svc_rqst *); +static void svc_close_socket (struct svc_sock *svsk);static struct svc_deferred_req *svc_deferred_dequeue (struct svc_sock *svsk);static int svc_deferred_recv (struct svc_rqst *rqstp);
@@ -1787,7 +1788,7 @@ svc_delete_socket(struct svc_sock *svsk)spin_unlock_bh (&serv->sv_lock); } -void svc_close_socket (struct svc_sock *svsk) +static void svc_close_socket (struct svc_sock *svsk) {set_bit (SK_CLOSE , &svsk->sk_flags);if (test_and_set_bit (SK_BUSY , &svsk->sk_flags))
@@ -1800,6 +1801,19 @@ void svc_close_socket(struct svc_sock *ssvc_sock_put (svsk); } +void svc_force_close_socket (struct svc_sock *svsk) +{ +set_bit (SK_CLOSE , &svsk->sk_flags); +if (test_bit (SK_BUSY , &svsk->sk_flags)) { +/* Waiting to be processed, but no threads left, +* So just remove it from the waiting list +*/ +list_del_init (&svsk->sk_ready); +clear_bit (SK_BUSY , &svsk->sk_flags); + } +svc_close_socket (svsk); +} +/** * svc_makesock - Make a socket for nfsd and lockd * @serv: RPC server structure
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
PATCH 1/1 knfsd: Provide sunrpc pool_mode module option
In reply to NeilBrownFrom: Greg Banks <[email protected]>
Provide a module param "pool_mode" for sunrpc.ko which allows a sysadmin to choose the mode for mapping NFS thread service pools to CPUs. Values are:
auto choose a mapping mode heuristically
global (default, same as the pre-2.6.19 code) a single global pool
percpu one pool per CPU
pernode one pool per NUMA node
Note that since 2.6.19 the hardcoded behaviour has been "auto", this patch makes the default "global".
The pool mode can be changed after boot/modprobe using /sys, if the NFS and lockd services have been shut down. A useful side effect of this change is to fix a small memory leak when unloading the module.
### Diffstat output
./Documentation/kernel-parameters.txt | 16 ++++ ./net/sunrpc/svc.c | 133 +++++++++++++++++++++++++++++----- 2 files changed, 130 insertions(+), 19 deletions(-)
Documentation/kernel-parameters.txt +16 -0
@@ -1692,6 +1692,22 @@ and is between 256 and 4096 characters. stifb= [HW] Format: bpp:<bpp1>[:<bpp2>[:<bpp3>...]] + sunrpc.pool_mode= + [NFS] + Control how the NFS server code allocates CPUs to + service thread pools. Depending on how many NICs + you have and where their interrupts are bound, this + option will affect which CPUs will do NFS serving. + Note: this parameter cannot be changed while the + NFS server is running. + + auto the server chooses an appropriate mode + automatically using heuristics + global a single global pool contains all CPUs + percpu one pool for each CPU + pernode one pool for each NUMA node (equivalent + to global on non-NUMA machines) + swiotlb= [IA-64] Number of I/O TLB slabs switches= [HW,M68k]
net/sunrpc/svc.c +114 -19
213 lines not on this page · raw message
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
Re: [PATCH 004 of 4] knfsd: Provide sunrpc pool_mode module option
Quoted 9 lines · On Tue, 6 Mar 2007 13:15:20 +1100 NeilBrown <[email protected]> wrote:
Provide a module param "
pool_mode" for sunrpc.ko which allows a sysadmin to choose the mode for mapping NFS thread service pools to CPUs. Values are:auto choose a mapping mode heuristically
global (default, same as the pre-2.6.19 code) a single global pool
percpu one pool per CPU
pernode one pool per NUMA node
Quoted 6 lines
Note that since 2.6.19 the hardcoded behaviour has been "auto", this patch makes the default "global".
The pool mode can be changed after boot/modprobe using /sys, if the NFS and lockd services have been shut down. A useful side effect of this change is to fix a small memory leak when unloading the module.
Mutter. Is this really suitable and needed for 2.6.21 at this stage in its life?
Re: [PATCH 004 of 4] knfsd: Provide sunrpc pool_mode module option
Quoted 14 lines · On Monday March 5, [email protected] wrote:
Quoted 12 lines · On Tue, 6 Mar 2007 13:15:20 +1100 NeilBrown <[email protected]> wrote:
Provide a module param "
pool_mode" for sunrpc.ko which allows a sysadmin to choose the mode for mapping NFS thread service pools to CPUs. Values are:auto choose a mapping mode heuristically
global (default, same as the pre-2.6.19 code) a single global pool
percpu one pool per CPU
pernode one pool per NUMA nodeNote that since 2.6.19 the hardcoded behaviour has been "auto", this patch makes the default "global".
Quoted 6 lines
Quoted 3 lines
The pool mode can be changed after boot/modprobe using /sys, if the NFS and lockd services have been shut down. A useful side effect of this change is to fix a small memory leak when unloading the module.
Mutter. Is this really suitable and needed for 2.6.21 at this stage in its life?
Something is definitely needed. Currently on a 4-way SMP machine, nfsd might only use 1 CPU (depends a bit on irq routing I think).
If that patch is too big, maybe this one?
NeilBrown
--------------------------------
Avoid using nfsd process pools on SMP machines.
From: Neil Brown <[email protected]>
process-pools have real benefits for NUMA, but on SMP machines they only work if network interface interrupts go to all CPUs (via round-robin or multiple nics). This is not always the case, so disable the pools in this case until a better solution is developped.
### Diffstat output
./net/sunrpc/svc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
net/sunrpc/svc.c +5 -1
@@ -79,7 +79,11 @@ svc_pool_map_choose_mode(void)* x86_64 kernel on Xeons. In this case we * want to divide the pools on cpu boundaries. */ -return SVC_POOL_PERCPU ; +/* actually, unless your IRQs round-robin nicely, +* this turns out to be really bad, so just +* go GLOBAL for now until a better fix can be developped +*/ +return SVC_POOL_GLOBAL ; }/* default: one global pool */