Merge lp:~gabriel-samfira/python-tx-tftp/python-tx-tftp into lp:python-tx-tftp

[Gabriel Samfira]

Increase MAX_BLOCK_SIZE to 8192

The current MAX_BLOCK_SIZE only allows files of up to ~87 MB in size.
Considering that a windows winpe ramdisk can grow to about ~270 MB,
it would just fail after block nr 65535. Increasing this constant
will allow larger files to be transmited.

It is not mandatory for the client to ask for a block size as large as this. It
is just a higher cap, up to which a client may negotiate.

Preferably, this would be configurable per backend, and not hardcoded
in session.py.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

[Gavin Panella (allenap)]

I wonder why it was set to 1400. It's close to the typical Ethernet LAN MTU of 1500, so I wonder if there's a reason for this setting, but this might well be a coincidence too. The upstream maintainer and original author is https://github.com/shylent, and the upstream branch is https://github.com/shylent/python-tx-tftp. It's worth making a pull request out of this change and see what he/she thinks. Ultimately we'd want to get this change upstream anyway.

Another question: is TFTP really a good choice for moving files of ~270MB across a network, or are you forced to do this? I suspect a TCP-based mechanism, HTTP perhaps, would be quicker and possibly more reliable.

review: Needs Information

[Alessandro Pilotti (alexpilotti)]

This is the way in which Windows handles the initial WinPE image deployment. SMB file transfer is used for the remaining steps.

As a comparison, the Microsoft TFTP service has a default block size of 16384.

A higher value then 1400 (in the UPD packet size limits) speeds up the deployment in scenarios where fragmentation is avoided (e.g. LAN) as the TFTP protocol expects an ACK for every UDP packet sent.

I agree that it looks like they preferred to stay in the default MTU safety margins.

We sent a pull request upstream as well: https://github.com/shylent/python-tx-tftp/pull/17

Thanks!

[Gabriel Samfira (gabriel-samfira)]

Hello Gavin,

Unfortunately there is no other way to transfer the ramdisk using officially supported methods. In the case of a Windows PXE boot, pxeboot.0 gets loaded first which pulls bootmgr.exe. This binary is responsible for pulling in the winpe ramdisk via TFTP, which is...huge. With that said, transferring the ramdisk via TFTP with a block size of 8192 should not be a problem. The maximum size for a UDP message is 65507 bytes (http://en.wikipedia.org/wiki/User_Datagram_Protocol).

Any frame larger then 1500 will be fragmented by the TCP/IP stack. I suspect that this operation is faster then sending out individual smaller frames, but I will have to test this. If need be, I can have a stab at TFTP roll-over, and reset the blocknum back to 0 once it reaches 65535. This will allow, in theory, an unlimited filesize, but to be honest, I do not believe this to be necessary.

I would have loved to transfer this behemoth via HTTP.

[Gabriel Samfira (gabriel-samfira)]

So, I implemented rollover. It works as well, but compared to just making MAX_BLOCK_SIZE bigger, its painfully slow. We can have both, to be honest...

I updated the pull request here:

https://github.com/shylent/python-tx-tftp/pull/17

Feel free to test the changes. They work well in my setup, but having someone else have a stab at it can not hurt.

[Gavin Panella (allenap)]

The larger block size works fine for me on my home network, using tftp-hpa as the client. I'll try to give it a go next week on another network.

The change to rollover probably deserves a test - the maintainer will probably ask for one - so it might be worth splitting the changes up again.

[Gabriel Samfira (gabriel-samfira)]

I split up the changes into 2 pull requests. Also added tests for the rollover bit. You can find them here:

https://github.com/shylent/python-tx-tftp/pull/17
https://github.com/shylent/python-tx-tftp/pull/18

[Andres Rodriguez (andreserl)]

@Gavin,

CAn you please land this so I can go ahead and upload to Ubuntu? Thanks

[Gavin Panella (allenap)]

The target branch is an import from GitHub. Gabriel has submitted the change upstream, and the maintainer seems happy with it, but has not yet merged it.

[Gabriel Samfira (gabriel-samfira)]

Changes have been merged.

[Gabriel Samfira (gabriel-samfira)]

Hello there,

The changes I requested upstream have been merged:

https://github.com/shylent/python-tx-tftp/pull/17

Would it be possible to sync the above revision? This merge implements rollover tftp, which allows transfering files of any size, without changing the MAX_BLOCK_SIZE.

[Andres Rodriguez (andreserl)]

IIRC,

This broke other environments. Let's test various other environments before
we pull it in
Thanks
On Oct 20, 2014 11:15 AM, "Gabriel Samfira" <email address hidden>
wrote:

> Hello there,
>
> The changes I requested upstream have been merged:
>
> https://github.com/shylent/python-tx-tftp/pull/17
>
> Would it be possible to sync the above revision? This merge implements
> rollover tftp, which allows transfering files of any size, without changing
> the MAX_BLOCK_SIZE.
> --
>
> https://code.launchpad.net/~gabriel-samfira/python-tx-tftp/python-tx-tftp/+merge/208862
> You are reviewing the proposed merge of
> lp:~gabriel-samfira/python-tx-tftp/python-tx-tftp into lp:python-tx-tftp.
>

[Gabriel Samfira (gabriel-samfira)]

The BLOCK_MAX_SIZE (PR #18) broke some environs, the rollover thing (PR #17) should not break anything :-). Feel free to test on any setup available to you. On esxi, kvm and our NUC setup, I had no issues.

Unmerged revisions

39. By Gabriel Samfira on 2014-02-28

Increase MAX_BLOCK_SIZE to 8192

The current MAX_BLOCK_SIZE only allows files of up to ~87 MB in size.
Considering that a windows winpe ramdisk can grow to about ~270 MB,
it would just fail after block nr 65535. Increasing this constant
will allow larger files to be transmited.

It is not mandatory for the client to ask for a block size as large as this. It
is just a higher cap, up to which a client may negotiate.

Preferably, this would be configurable per backend, and not hardcoded
in session.py.

=== modified file 'tftp/session.py'
--- tftp/session.py	2012-09-25 05:29:40 +0000
+++ tftp/session.py	2014-02-28 18:06:19 +0000
@@ -9,7 +9,7 @@
 from twisted.internet.protocol import DatagramProtocol
 from twisted.python import log
 
-MAX_BLOCK_SIZE = 1400
+MAX_BLOCK_SIZE = 8192
 
 
 class WriteSession(DatagramProtocol):