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

[Ramon Fried]

On Tue, Feb 2, 2021 at 6:02 PM Suneel Garapati <suneelglinux at gmail.com> wrote:
>
> On Mon, Feb 1, 2021 at 2:27 PM Ramon Fried <rfried.dev at gmail.com> wrote:
> >
> > 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.
> Yes, I can test the fix.
>
> Thanks,
> Suneel
> > > >
> > > > Regards,
> > > > Suneel
> > > Thanks for spotting this, I'll check it out.
Hey Suneel
Now when I dived deeper, I just figured out that __tftpboot__ doesn't
work for you with window size = 1.
This is very strange, my current setup is working also with this configuration.
Can you please check which TFTP server you're using on the other end ?

Thanks,
Ramon.