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

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

[Milan Plzik]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

  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

[-- Attachment #2: qemu_tftp_handle_oack_correctly.patch --]
[-- Type: text/x-patch, Size: 283 bytes --]

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

Re: [Qemu-devel] Qemu's internal TFTP server breaks lock-step-iness of TFTP

[Anthony Liguori]

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
>    

Re: [Qemu-devel] Qemu's internal TFTP server breaks lock-step-iness of TFTP

[Milan Plzik]

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

Re: [Qemu-devel] Qemu's internal TFTP server breaks lock-step-iness of TFTP

[H. Peter Anvin]

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

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

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

[Milan Plzik]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

  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.

Signed-off-by: Thomas Horsten <thomas@horsten.com>
Signed-off-by: Milan Plzik <milan.plzik@gmail.com>


[-- Attachment #2: qemu_tftp_handle_oack_correctly.patch --]
[-- Type: text/x-patch, Size: 283 bytes --]

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

Re: [Qemu-devel] Qemu's internal TFTP server breaks lock-step-iness of TFTP

[Anthony Liguori]

On 01/07/2010 06:39 AM, Milan Plzik wrote:
>    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.
>
> Signed-off-by: Thomas Horsten<thomas@horsten.com>
> Signed-off-by: Milan Plzik<milan.plzik@gmail.com>
>
>    

Applied.  Thanks.

Regards,

Anthony Liguori