[PATCH v5] net: tftp: Add client support for RFC 7440

[Suneel Garapati <suneelglinux@gmail.com>]

On Mon, Feb 1, 2021 at 2:27 PM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Sat, Jan 30, 2021 at 11:26 PM Ramon Fried <rfried.dev@gmail.com> wrote:
> >
> > On Sat, Jan 30, 2021 at 10:39 PM Suneel Garapati <suneelglinux@gmail.com> wrote:
> > >
> > > Hello Ramon,
> > >
> > > With TFTP window size as default 1 and enabling TFTPPUT config option
> > > results in tftpboot command failure.
> > >
> > > Attached the pcap files for with and without TFTPPUT enabled.
> > > Both captures are the same except that the TFTPPUT option enables ICMP handler
> > > and for the response from the server triggers a restart of netloop
> > > operation and eventually fails as below.
> > >
> > > > tftpboot 0x30000000 192.168.1.1:Image
> > > Waiting for RPM0 LMAC0 link status... 10G_R [10G]
> > > Using rvu_pf#0 device
> > > TFTP from server 192.168.1.1; our IP address is 192.168.1.16
> > > Filename 'Image'. Load address: 0x30000000
> > > Loading: ################################################## 15.8 MiB
> > > 787.1 KiB/s
> > > done
> > > TFTP server died; starting again
> > > >
> > >
> > > I see the code issue as below on net/tftp.c [v2020.10] ?
> > >                 /*
> > >                  *      Acknowledge the block just received, which will prompt
> > >                  *      the remote for the next one.
> > >                  */
> > >                 if (tftp_cur_block == tftp_next_ack) {
> > >                         tftp_send();
> > >                         tftp_next_ack += tftp_windowsize;
> > >                 }
> > >
> > >                 if (len < tftp_block_size) {
> > >                         //if (tftp_windowsize > 1) [Hack in use for
> > > now to work around this issue]
> > >                         tftp_send();                      [ This
> > > causes extra ACK packet send with same block number and causes server
> > > to respond with ICMP error]
> > >                         tftp_complete();
> > >                 }
> > >
> > > I couldn?t try with tftp_windowsize > 1 as the test servers don?t support.
> > > I tried with all latest commits on net/tftp.c on top of v2020.10 and
> > > still see the issue.
> > >
> > > This change is introduced in
> > > commit cc6b87ecaa96325577a8fafabc0d5972b816bc6c
> > > Author: Ramon Fried <rfried.dev@gmail.com>
> > > Date:   Sat Jul 18 23:31:46 2020 +0300
> > >
> > >     net: tftp: Add client support for RFC 7440
> > >
> > >     Add support for RFC 7440: "TFTP Windowsize Option".
> > >
> > > Reverting this commit on v2020.10 also fixes the issue.
> > >
> > > I would like to know if this extra tftp_send is needed at all or only
> > > for window size > 1
> RFC7440 is not supported by most TFTP Servers, when adding support for
> this feature I didn't even consider TFTPPUT.
> I think that the best thing to do is to only support RFC7440 on TFTPGET.
> In this case we should remove the tftp_send() when sending a file.
> I will create a fix, would you mind testing to see if it works for you ?
> Thanks,
> Ramon.
Yes, I can test the fix.

Thanks,
Suneel
> > >
> > > Regards,
> > > Suneel
> > Thanks for spotting this, I'll check it out.