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

[Ramon Fried]

On Sat, Jan 30, 2021 at 11:26 PM Ramon Fried <rfried.dev at gmail.com> wrote:
>
> On Sat, Jan 30, 2021 at 10:39 PM Suneel Garapati <suneelglinux at 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 at 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.
> >
> > Regards,
> > Suneel
> Thanks for spotting this, I'll check it out.