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

[Ramon Fried]

On Wed, Feb 3, 2021 at 12:23 AM Suneel Garapati <suneelglinux at gmail.com> wrote:
>
> 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.
OK. Found the problem. sending a patch.