Skip to content

net: tftp: Fix incorrect tftp_next_ack on no OACK - #8

Open
lorc wants to merge 1 commit into
renesas-rcar:v2020.10/rcar-5.1.0_S4.rc8from
lorc:pr_rcar_fix_tftp
Open

net: tftp: Fix incorrect tftp_next_ack on no OACK#8
lorc wants to merge 1 commit into
renesas-rcar:v2020.10/rcar-5.1.0_S4.rc8from
lorc:pr_rcar_fix_tftp

Conversation

@lorc

@lorc lorc commented Feb 4, 2022

Copy link
Copy Markdown

When the tftp server did not send any OACK, the tftp_next_ack variable
was not set to the correct value . As the server was transmitting
blocks we generated a lot of 'Received unexpected block: $n, expected
$n+1' error messages. Depending on the timeout setting the transfer
could still complete though.

Signed-off-by: Harm Berntsen harm.berntsen@nedap.com
CC: Ramon Fried rfried.dev@gmail.com
Reviewed-By: Ramon Fried rfried.dev@gmail.com


It appears that some old TFTP server do not support OPTIONS extension for the protocol, which causes problems in our case.

When the tftp server did not send any OACK, the tftp_next_ack variable
was not set to the correct value . As the server was transmitting
blocks we generated a lot of 'Received unexpected block: $n, expected
$n+1' error messages. Depending on the timeout setting the transfer
could still complete though.

Signed-off-by: Harm Berntsen <harm.berntsen@nedap.com>
CC: Ramon Fried <rfried.dev@gmail.com>
Reviewed-By: Ramon Fried <rfried.dev@gmail.com>
@HaiNguyenPham

Copy link
Copy Markdown
Collaborator

Hello @lorc
Does the generated error messages 'Received unexpected block: $n, expected $n+1'
come from commit https://source.denx.de/u-boot/u-boot/-/commit/cc6b87ecaa96325577a8fafabc0d5972b816bc6c?
If yes, is this fix https://source.denx.de/u-boot/u-boot/-/commit/2dddc1bb296308b48f89f31e711965fa2c8091a0
also required for your use-case?

@lorc

lorc commented Oct 17, 2022

Copy link
Copy Markdown
Author

Hi @HaiNguyenPham.

Does the generated error messages 'Received unexpected block: $n, expected $n+1'
come from commit https://source.denx.de/u-boot/u-boot/-/commit/cc6b87ecaa96325577a8fafabc0d5972b816bc6c?

Yes, this is correct.

If yes, is this fix https://source.denx.de/u-boot/u-boot/-/commit/2dddc1bb296308b48f89f31e711965fa2c8091a0
also required for your use-case?

I'm not sure, but this looks like valid fix, so it is better to include this as well.

Also, please note that patch in this PR is not authored by me. I took it from upstream U-Boot code.

@HaiNguyenPham

Copy link
Copy Markdown
Collaborator

Hi @lorc
Got it, we will take care of it in next merge windows

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