Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait 1 millisecond between duplicate packets #7

Merged
merged 3 commits into from
Oct 9, 2023

Conversation

lupyuen
Copy link
Contributor

@lupyuen lupyuen commented Oct 8, 2023

Hi: Thanks for adding the option for Duplicate Packets. Based on my testing, we need to insert 1 millisecond delay between the Duplicate Packets. (Should this be another option?)

Without Delay: U-Boot JH7110 boots slower (5.7 Mbps), with 2 TFTP Timeouts

Filename 'initrd'.
Loading: 711.9 KiB/s
Bytes transferred = 8100864 (7b9c00 hex)

With Delay (1 millisecond): U-Boot JH7110 boots faster (8.8 Mbps), with No TFTP Timeouts

Filename 'initrd'.
Loading: 1.1 MiB/s
Bytes transferred = 8100864 (7b9c00 hex)

This was tested on macOS over Wired Ethernet. Thanks :-)

cd ~/rs-tftpd
sudo cargo run -- \
  --duplicate-packets 2 \
  -i 0.0.0.0 \
  -p 69 \
  -d "$HOME/tftproot"

Copy link
Owner

@altugbakan altugbakan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few remarks.

src/worker.rs Outdated Show resolved Hide resolved
src/worker.rs Show resolved Hide resolved
@stappersg
Copy link
Contributor

stappersg commented Oct 8, 2023 via email

@stappersg
Copy link
Contributor

stappersg commented Oct 8, 2023 via email

Based on my testing, we need to insert 1 millisecond delay between the Original and Duplicate Packets.

__Without Delay:__ U-Boot JH7110 boots slower (5.7 Mbps), with 2 TFTP Timeouts

```text
Filename 'initrd'.
Loading: 711.9 KiB/s
Bytes transferred = 8100864 (7b9c00 hex)
```

__With Delay (1 millisecond):__ U-Boot JH7110 boots faster (8.8 Mbps), with No TFTP Timeouts

```text
Filename 'initrd'.
Loading: 1.1 MiB/s
Bytes transferred = 8100864 (7b9c00 hex)
```
@lupyuen lupyuen marked this pull request as draft October 9, 2023 05:12
@lupyuen lupyuen marked this pull request as ready for review October 9, 2023 05:22
Based on my testing, we need to insert 1 millisecond delay between the Original and Duplicate Packets to eliminate the TFTP Timeouts.

__Without Delay:__ U-Boot JH7110 boots slower (5.7 Mbps), with 2 TFTP Timeouts

```text
Filename 'initrd'.
Loading: 711.9 KiB/s
Bytes transferred = 8100864 (7b9c00 hex)
```

__With Delay (1 millisecond):__ U-Boot JH7110 boots faster (8.8 Mbps), with No TFTP Timeouts

```text
Filename 'initrd'.
Loading: 1.1 MiB/s
Bytes transferred = 8100864 (7b9c00 hex)
```
@lupyuen
Copy link
Contributor Author

lupyuen commented Oct 9, 2023

Hi @stappersg: I hope I understood your question correctly, suppose our tftpd sends this sequence of packets to the client:

  1. Original Packet #1
  2. Delay 1 millisecond
  3. Duplicate Packet #1
  4. Original Packet #2
  5. Delay 1 millisecond
  6. Duplicate Packet #2

Our question: If the client correctly receives Original Packet #1, will it handle Duplicate Packet #1 correctly?

Yes the TFTP Client should drop Duplicate Packet #1, because the Block Number is encoded inside the packet. We see this behaviour in curl:

$ curl -v --output initrd tftp://192.168.31.10/initrd
* Received last DATA packet block 1 again.
* Received last DATA packet block 2 again.

(Source)

Apparently U-Boot will drop Duplicate Packets too, because otherwise our Kernel Image would have been corrupted over TFTP and our device will fail to boot. (So far I haven't seen any boot failures due to corrupted Kernel Image)

Which leads to another question: Why don't we throttle the Original Packets and omit the Duplicate Packets?

  1. Original Packet #1
  2. Delay 2 milliseconds
  3. Original Packet #2
  4. Delay 2 milliseconds

We tried throttling tftpd to do the above, but it doesn't fix the TFTP Timeouts. (Details here)

So I agree with you, there seems to be a problem with the Network Drivers in some ports of U-Boot. (Maybe it's not responding to I/O Interrupts quick enough? Or it has run out of Network Buffers?)

Copy link
Owner

@altugbakan altugbakan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for testing it out.

@altugbakan altugbakan merged commit 5ef2ffa into altugbakan:main Oct 9, 2023
1 check passed
@stappersg
Copy link
Contributor

stappersg commented Oct 9, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants