Re: Patch for tftp client code

[Eric Doenges <Eric dot Doenges at DynaPel dot com>]

Andrew Lunn wrote:
> Thinking about this some more, i don't see how the scenario you have
> fixed could actually happen. tftp sends a data packet and expects an
> ACK. It should not send the next data packet in sequence until it
> receives the ACK for the previous packet. 

It doesn't. It repeats the data packet. In all cases that I observed.
the count in the header corresponded to the count of the last packet
received. In this case, the original code does not copy the data (which
is correct), acknowledges the DATA received by sending an ACK for the
last valid packet received (which I think is also correct), and does
not set actual_len to the length of the DATA packet just received. This
means that actual_len will be zero whenever a packet is received with
the wrong sequence count. Next, the code checks if actual_len is
greater or equal to zero and smaller than the segment size of 512 bytes,
in which case the TFTP protocoll assumes the transmission is complete.
As actual_len is zero (with the original code) whenever a packet with
the wrong sequence number is received, this will always signal end
of transmission regardless of wether the server has more DATA packets
to send or not.
> For it to close the
> connection too soon, it must of received the last packeted before the
> next to last packet.

No it doesn't, see above. In fact, I have had transmission aborted after
only half the file was transferred. I also added diagnosis code that
printed the sequence number of out of sequence packets to stdout, and
could thus see that the affected packets were not the last packet.

> But this is not possible with this simple protocol. Something else is
> going on here.
> I would check the sequence number of the packet that is received and
> it not a full size DATA packet. Is it really the last packet? Or is it
> some other packet which has been truncated?

A non-full sized DATA packet is always the last packet for TFTP.
However, in the original code a full-sized packet that is received out
of sequence is always taken to be a zero-length DATA packet, due to
the (incorrect) handling of actual_len. The packets themselves are
not truncated. The reason I do not set actual_len to data_len - 4
instead of SEGSIZE is to guard against the case when the last packet
is received out of sequence (or a packet in between is truncated).
Basically, I'm just using actual_len to avoid having to put an extra
check in the termination condition to figure out if the packet was
valid or not.


--
--------------------------------------------------------------------
|     Eric Doenges              |     DynaPel Laboratories GmbH    |
|     Tel: +49 89 962428 23     |     Fraunhoferstrasse 9/2        |
|     Fax: +49 89 962428 90     |     D - 85737 Ismaning, Germany  |
--------------------------------------------------------------------