2002-11-15 06:49:40

by Hirokazu Takahashi

[permalink] [raw]
Subject: reduce copying data in svc_udp_recvfrom

Hello Neil,

rqstp->rq_arg.head[0] can use data in a skbuff directly instead of
copying the data into xdr when the skbuff is linear.
New kNFSd can do it like old kNFSd did.

Is there any problem?

Thank you,
Hirokazu Takahashi.


--- net/sunrpc/svcsock.c.NONZERO Fri Nov 15 11:26:18 2030
+++ net/sunrpc/svcsock.c Fri Nov 15 11:30:14 2030
@@ -564,21 +564,35 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
set_bit(SK_DATA, &svsk->sk_flags); /* there may be more data... */

len = skb->len - sizeof(struct udphdr);
+ rqstp->rq_arg.len = len;

- if (csum_partial_copy_to_xdr(&rqstp->rq_arg, skb)) {
- /* checksum error */
+ if (skb_is_nonlinear(skb)) {
+ if (csum_partial_copy_to_xdr(&rqstp->rq_arg, skb)) {
+ /* checksum error */
+ skb_free_datagram(svsk->sk_sk, skb);
+ svc_sock_received(svsk);
+ return 0;
+ }
+ if (len <= rqstp->rq_arg.head[0].iov_len) {
+ rqstp->rq_arg.head[0].iov_len = len;
+ rqstp->rq_arg.page_len = 0;
+ } else {
+ rqstp->rq_arg.page_len = len - rqstp->rq_arg.head[0].iov_len;
+ rqstp->rq_argused += (rqstp->rq_arg.page_len + PAGE_SIZE - 1)/ PAGE_SIZE;
+ }
skb_free_datagram(svsk->sk_sk, skb);
- svc_sock_received(svsk);
- return 0;
- }
-
- rqstp->rq_arg.len = len;
- if (len <= rqstp->rq_arg.head[0].iov_len) {
- rqstp->rq_arg.head[0].iov_len = len;
- rqstp->rq_arg.page_len = 0;
} else {
- rqstp->rq_arg.page_len = len - rqstp->rq_arg.head[0].iov_len;
- rqstp->rq_argused += (rqstp->rq_arg.page_len + PAGE_SIZE - 1)/ PAGE_SIZE;
+ if (skb->ip_summed != CHECKSUM_UNNECESSARY) {
+ if ((unsigned short)csum_fold(skb_checksum(skb, 0, skb->len, skb->csum))) {
+ skb_free_datagram(svsk->sk_sk, skb);
+ svc_sock_received(svsk);
+ return 0;
+ }
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
+ rqstp->rq_skbuff = skb;
+ rqstp->rq_arg.head[0].iov_base = skb->data + sizeof(struct udphdr);
+ rqstp->rq_arg.head[0].iov_len = len;
}

rqstp->rq_prot = IPPROTO_UDP;
@@ -587,8 +601,6 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
rqstp->rq_addr.sin_family = AF_INET;
rqstp->rq_addr.sin_port = skb->h.uh->source;
rqstp->rq_addr.sin_addr.s_addr = skb->nh.iph->saddr;
-
- skb_free_datagram(svsk->sk_sk, skb);

if (serv->sv_stats)
serv->sv_stats->netudpcnt++;


-------------------------------------------------------
This sf.net email is sponsored by: To learn the basics of securing
your web site with SSL, click here to get a FREE TRIAL of a Thawte
Server Certificate: http://www.gothawte.com/rd524.html
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2002-11-17 23:46:08

by NeilBrown

[permalink] [raw]
Subject: Re: reduce copying data in svc_udp_recvfrom

On Friday November 15, [email protected] wrote:
> Hello Neil,
>
> rqstp->rq_arg.head[0] can use data in a skbuff directly instead of
> copying the data into xdr when the skbuff is linear.
> New kNFSd can do it like old kNFSd did.
>
> Is there any problem?

Only one:-)

In the case where the skb is nonlinear you skb_free_datagram before
the port/address are copies into rqstp->rq_addr.

Apart from that, the patch is fine, thanks.

Here is my version.

NeilBrown

--------------------------------------------------
Status: compiles

Avoid copying unfragmented udp NFS requests.

If an NFS request arrives in a linear skb,
we don't need to copy it, particularly if the
network card has already done the DUB checksum.

This patch only copies a request if it is
already non-linear.

----------- Diffstat output ------------
net/sunrpc/svcsock.c | 45 ++++++++++++++++++++++++++++++---------------
1 files changed, 30 insertions(+), 15 deletions(-)

diff net/sunrpc/svcsock.c~current~ net/sunrpc/svcsock.c
--- net/sunrpc/svcsock.c~current~ 2002-11-18 10:24:26.000000000 +1100
+++ net/sunrpc/svcsock.c 2002-11-18 10:40:09.000000000 +1100
@@ -564,16 +564,39 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
set_bit(SK_DATA, &svsk->sk_flags); /* there may be more data... */

len = skb->len - sizeof(struct udphdr);
+ rqstp->rq_arg.len = len;

- if (csum_partial_copy_to_xdr(&rqstp->rq_arg, skb)) {
- /* checksum error */
- skb_free_datagram(svsk->sk_sk, skb);
- svc_sock_received(svsk);
- return 0;
- }
+ rqstp->rq_prot = IPPROTO_UDP;

+ /* Get sender address */
+ rqstp->rq_addr.sin_family = AF_INET;
+ rqstp->rq_addr.sin_port = skb->h.uh->source;
+ rqstp->rq_addr.sin_addr.s_addr = skb->nh.iph->saddr;
+
+ if (skb_is_nonlinear(skb)) {
+ /* we have to copy */
+ if (csum_partial_copy_to_xdr(&rqstp->rq_arg, skb)) {
+ /* checksum error */
+ skb_free_datagram(svsk->sk_sk, skb);
+ svc_sock_received(svsk);
+ return 0;
+ }
+ skb_free_datagram(svsk->sk_sk, skb);
+ } else {
+ /* we can use it in-place */
+ rqstp->rq_arg.head[0].iov_base = skb->data + sizeof(struct udphdr);
+ rqstp->rq_arg.head[0].iov_len = len;
+ if (skb->ip_summed != CHECKSUM_UNNECESSARY) {
+ if ((unsigned short)csum_fold(skb_checksum(skb, 0, skb->len, skb->csum))) {
+ skb_free_datagram(svsk->sk_sk, skb);
+ svc_sock_received(svsk);
+ return 0;
+ }
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
+ rqstp->rq_skbuff = skb;
+ }

- rqstp->rq_arg.len = len;
rqstp->rq_arg.page_base = 0;
if (len <= rqstp->rq_arg.head[0].iov_len) {
rqstp->rq_arg.head[0].iov_len = len;
@@ -582,14 +605,6 @@ svc_udp_recvfrom(struct svc_rqst *rqstp)
rqstp->rq_arg.page_len = len - rqstp->rq_arg.head[0].iov_len;
rqstp->rq_argused += (rqstp->rq_arg.page_len + PAGE_SIZE - 1)/ PAGE_SIZE;
}
- rqstp->rq_prot = IPPROTO_UDP;
-
- /* Get sender address */
- rqstp->rq_addr.sin_family = AF_INET;
- rqstp->rq_addr.sin_port = skb->h.uh->source;
- rqstp->rq_addr.sin_addr.s_addr = skb->nh.iph->saddr;
-
- skb_free_datagram(svsk->sk_sk, skb);

if (serv->sv_stats)
serv->sv_stats->netudpcnt++;


-------------------------------------------------------
This sf.net email is sponsored by: To learn the basics of securing
your web site with SSL, click here to get a FREE TRIAL of a Thawte
Server Certificate: http://www.gothawte.com/rd524.html
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2002-11-18 11:19:03

by Hirokazu Takahashi

[permalink] [raw]
Subject: Re: reduce copying data in svc_udp_recvfrom

Hello,

> > rqstp->rq_arg.head[0] can use data in a skbuff directly instead of
> > copying the data into xdr when the skbuff is linear.
> > New kNFSd can do it like old kNFSd did.
> >
> > Is there any problem?
>
> Only one:-)
>
> In the case where the skb is nonlinear you skb_free_datagram before
> the port/address are copies into rqstp->rq_addr.

Yes, you're right. Thank you.

> Apart from that, the patch is fine, thanks.
>
> Here is my version.

Looks fine.


-------------------------------------------------------
This sf.net email is sponsored by: To learn the basics of securing
your web site with SSL, click here to get a FREE TRIAL of a Thawte
Server Certificate: http://www.gothawte.com/rd524.html
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs