2007-03-28 10:30:00

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH - try #2] sunrpc: fix typo in svc_udp_recvfrom

On Wednesday March 28, [email protected] wrote:
> Hi Neil,
>
> On Wednesday 28 March 2007 04:16, Neil Brown wrote:
> > What about this (on top of current -mm).
>
> Well, you just converted a while loop to an if. The old
> code would pull any ICMP signalled error condition out
> of the socket, and then try to get the data. The new
> code will return if it hits an ICMP error, without
^^^^^^^
> reasserting SK_DATA. Any pending UDP request will
^^^^^^^^^^^^^^^^^^^
> be stuck until the client retransmits.
>
> So I think you really need the loop here.
>
> Olaf

> > clear_bit(SK_DATA, &svsk->sk_flags);
> > - 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;
> > + skb = NULL;
> > + err = kernel_recvmsg(svsk->sk_sock, &msg, NULL,
> > + 0, 0, MSG_PEEK | MSG_DONTWAIT);
> > + if (err >= 0)
> > + skb = skb_recv_datagram(svsk->sk_sk, 0, 1, &err);
> > +
> > + if (skb == NULL) {
> > + if (err != -EAGAIN) {
> > + /* possibly an icmp error */
> > + dprintk("svc: recvfrom returned error %d\n", -err);
> > + set_bit(SK_DATA, &svsk->sk_flags);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

while ((err= get_packet()) && err != -EAGAIN)
;
works for me, but as the while loop gets bigger it (to me) becomes
less obvious why it is there...

Ho hum... maybe I'm aiming for perfection and missing "good" ??

NeilBrown


> > }
> > - /* possibly an icmp error */
> > - dprintk("svc: recvfrom returned error %d\n", -err);
> > + svc_sock_received(svsk);
> > + return -EAGAIN;
> > }
> > rqstp->rq_addrlen = sizeof(rqstp->rq_addr);
> > if (skb->tstamp.off_sec == 0) {
> >
>
> --
> Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
> [email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax

-------------------------------------------------------------------------
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


2007-03-28 13:52:26

by Olaf Kirch

[permalink] [raw]
Subject: Re: [PATCH - try #2] sunrpc: fix typo in svc_udp_recvfrom

On Wednesday 28 March 2007 12:29, Neil Brown wrote:

> > > + if (skb == NULL) {
> > > + if (err != -EAGAIN) {
> > > + /* possibly an icmp error */
> > > + dprintk("svc: recvfrom returned error %d\n", -err);
> > > + set_bit(SK_DATA, &svsk->sk_flags);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Okay, you're right - I missed that.

Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax

-------------------------------------------------------------------------
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