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

[Suneel Garapati]

On Tue, Feb 2, 2021 at 11:46 AM Ramon Fried <rfried.dev at gmail.com> wrote:
>
> 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 ?

Did it work with/without CMD_TFTPPUT config option?
For me it fails only when this option is enabled.
What is the response on your setup from the server for the extra ack
packet sent on the last block?
TFTP server is a tftpd-hpa service on ubuntu 18.04 machine with basic setup.

>
> Thanks,
> Ramon.