diff mbox

Qemu's internal TFTP server breaks lock-step-iness of TFTP

Message ID 1262815902.18404.341.camel@localhost
State New
Headers show

Commit Message

Milan Plzik Jan. 6, 2010, 10:11 p.m. UTC 
Hello,

  according to RFC 1350 and RFC 2347, TFTP server should answer RRQ by
either OACK or DATA packet. Qemu's internal TFTP server answers RRQ with
additional options by sending both OACK and DATA packet, thus breaking
the "lock-step" feature of the protocol, and also confuses client.

  Proposed solution would be to, in case of OACK packet, wait for ACK
from client and just then start sending data. Attached patch implements
this.

  I would like to thank to mbc and th1 (who is the rightful author of
the patch) from #gpxe for their time, effort and patience with me :)

	Milan Plzik

Comments

Anthony Liguori Jan. 6, 2010, 10:43 p.m. UTC | #1 
On 01/06/2010 04:11 PM, Milan Plzik wrote:
>    Hello,
>
>    according to RFC 1350 and RFC 2347, TFTP server should answer RRQ by
> either OACK or DATA packet. Qemu's internal TFTP server answers RRQ with
> additional options by sending both OACK and DATA packet, thus breaking
> the "lock-step" feature of the protocol, and also confuses client.
>
>    Proposed solution would be to, in case of OACK packet, wait for ACK
> from client and just then start sending data. Attached patch implements
> this.
>
>    I would like to thank to mbc and th1 (who is the rightful author of
> the patch) from #gpxe for their time, effort and patience with me :)
>    

Thanks for the patch, I assume this fixes -boot n when using -bootp and 
-tftp.  Can you add an appropriate Signed-off-by and resubmit?

Regards,

Anthony Liguori

> 	Milan Plzik
>
Milan Plzik Jan. 7, 2010, 12:51 p.m. UTC | #2 
V Streda, 6. január 2010 o 16:43 -0600, Anthony Liguori napísal(a):
> Thanks for the patch, I assume this fixes -boot n when using -bootp and 
> -tftp.  

  Not sure what was the original issue, but at least for me it fixed
booting pxegrub :)

> Can you add an appropriate Signed-off-by and resubmit?

  Added, re-submitted; I hope it will be acceptable as an attachment.

> 
> Regards,
> 
> Anthony Liguori
> 
> > 	Milan Plzik
> >    
>
H. Peter Anvin Jan. 7, 2010, 3:42 p.m. UTC | #3 
On 01/06/2010 02:11 PM, Milan Plzik wrote:
>   Hello,
> 
>   according to RFC 1350 and RFC 2347, TFTP server should answer RRQ by
> either OACK or DATA packet. Qemu's internal TFTP server answers RRQ with
> additional options by sending both OACK and DATA packet, thus breaking
> the "lock-step" feature of the protocol, and also confuses client.
> 
>   Proposed solution would be to, in case of OACK packet, wait for ACK
> from client and just then start sending data. Attached patch implements
> this.
> 
>   I would like to thank to mbc and th1 (who is the rightful author of
> the patch) from #gpxe for their time, effort and patience with me :)
> 
> 	Milan Plzik

The built-in TFTP server has at least two *serious* other problems:

a) It has the Sorcerer's Apprentice bug (See RFC 1123 and 1350).
b) It doesn't do time-based retransmits.

Note that (a) can't be fixed without fixing (b).

	-hpa
diff mbox

Patch

diff --git a/slirp/tftp.c b/slirp/tftp.c
index 082f5d0..db869fc 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -362,6 +362,7 @@  static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen)
 	  }
 
 	  tftp_send_oack(spt, "tsize", tsize, tp);
+	  return;
       }
   }