| Message ID | 20240414104607.5966-1-yasuharu.shibata@gmail.com |
|---|---|
| State | Accepted |
| Commit | cab7867cff33f9c21025102ea9ca3b26e362fb52 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | net: wget: Support retransmission a dropped packet | expand |
Hi Yasuharu, On Sun, Apr 14, 2024 at 9:46 AM Yasuharu Shibata <yasuharu.shibata@gmail.com> wrote: > > The server sends multiple packets without waiting for an ACK > by window control and if some packets are dropped, > wget will return an ACK including the dropped packets. > > Following log indicates this issue. > > wget_handler() wget: Transferring, seq=97bbdd4a, ack=30,len=580 > wget_handler() wget: Transferring, seq=97bbedca, ack=30,len=580 > > First packet of TCP sequence number is 0x97bbdd4a. > Second packet of TCP sequence number should be 0x97bbe2ca, > however it is 0x97bbedca and returns its ACK, so the server > suppose that 0x97bbe2ca and 0x97bbedca are received appropriately. > In this case, 0x97bbe2ca was lost and the data of wget was broken. > > In this patch, next_data_seq_num holds the next expected > TCP sequence number. > If the TCP sequence number different from next_data_seq_num, > trying to retransmit the packet. Thanks for your patch. I tested it in the hope that it would fix the following issue: https://lore.kernel.org/u-boot/CAJ+vNU2U9W2NRT6hf1CAEQ_56SDQviUEzuDD1iYOpdf1CNaZBw@mail.gmail.com/ but I still get wget corruption when loading large files multiple times in a row. Would you happen to have any suggestions? Thanks
Hi Fabio, On Mon, 15 Apr 2024 at 04:04, Fabio Estevam <festevam@gmail.com> wrote: > > I tested it in the hope that it would fix the following issue: > https://lore.kernel.org/u-boot/CAJ+vNU2U9W2NRT6hf1CAEQ_56SDQviUEzuDD1iYOpdf1CNaZBw@mail.gmail.com/ > > but I still get wget corruption when loading large files multiple > times in a row. > > Would you happen to have any suggestions? I have seen similar issue apart from this patch and will investigate it as a next step. If I have any information about the issue, I will send it to the thread. -- Best regards, Yasuharu Shibata
Hi Yasuharu, On Sun, Apr 14, 2024 at 9:46 AM Yasuharu Shibata <yasuharu.shibata@gmail.com> wrote: > > The server sends multiple packets without waiting for an ACK > by window control and if some packets are dropped, > wget will return an ACK including the dropped packets. > > Following log indicates this issue. > > wget_handler() wget: Transferring, seq=97bbdd4a, ack=30,len=580 > wget_handler() wget: Transferring, seq=97bbedca, ack=30,len=580 > > First packet of TCP sequence number is 0x97bbdd4a. > Second packet of TCP sequence number should be 0x97bbe2ca, > however it is 0x97bbedca and returns its ACK, so the server > suppose that 0x97bbe2ca and 0x97bbedca are received appropriately. > In this case, 0x97bbe2ca was lost and the data of wget was broken. > > In this patch, next_data_seq_num holds the next expected > TCP sequence number. > If the TCP sequence number different from next_data_seq_num, > trying to retransmit the packet. > > Signed-off-by: Yasuharu Shibata <yasuharu.shibata@gmail.com> Tested-by: Fabio Estevam <festevam@gmail.com>
Hi Yasuharu, On Mon, Apr 15, 2024 at 7:10 AM Yasuharu Shibata <yasuharu.shibata@gmail.com> wrote: > I have seen similar issue apart from this patch and will investigate > it as a next step. > If I have any information about the issue, I will send it to the thread. Thanks, if you have any updates about this wget corruption, please keep me on Cc. Thanks, Fabio Estevam
On Sun, 14 Apr 2024 19:46:07 +0900, Yasuharu Shibata wrote: > The server sends multiple packets without waiting for an ACK > by window control and if some packets are dropped, > wget will return an ACK including the dropped packets. > > Following log indicates this issue. > > wget_handler() wget: Transferring, seq=97bbdd4a, ack=30,len=580 > wget_handler() wget: Transferring, seq=97bbedca, ack=30,len=580 > > [...] Applied to u-boot/master, thanks!
diff --git a/net/wget.c b/net/wget.c index 817c5ebd5d..71bac92d84 100644 --- a/net/wget.c +++ b/net/wget.c @@ -50,6 +50,7 @@ static unsigned long content_length; static unsigned int packets; static unsigned int initial_data_seq_num; +static unsigned int next_data_seq_num; static enum wget_state current_wget_state; @@ -272,17 +273,18 @@ static void wget_connected(uchar *pkt, unsigned int tcp_seq_num, current_wget_state = WGET_TRANSFERRING; + initial_data_seq_num = tcp_seq_num + hlen; + next_data_seq_num = tcp_seq_num + len; + if (strstr((char *)pkt, http_ok) == 0) { debug_cond(DEBUG_WGET, "wget: Connected Bad Xfer\n"); - initial_data_seq_num = tcp_seq_num + hlen; wget_loop_state = NETLOOP_FAIL; wget_send(action, tcp_seq_num, tcp_ack_num, len); } else { debug_cond(DEBUG_WGET, "wget: Connctd pkt %p hlen %x\n", pkt, hlen); - initial_data_seq_num = tcp_seq_num + hlen; pos = strstr((char *)pkt, content_len); if (!pos) { @@ -396,6 +398,12 @@ static void wget_handler(uchar *pkt, u16 dport, "wget: Transferring, seq=%x, ack=%x,len=%x\n", tcp_seq_num, tcp_ack_num, len); + if (next_data_seq_num != tcp_seq_num) { + debug_cond(DEBUG_WGET, "wget: seq=%x packet was lost\n", next_data_seq_num); + return; + } + next_data_seq_num = tcp_seq_num + len; + if (tcp_seq_num >= initial_data_seq_num && store_block(pkt, tcp_seq_num - initial_data_seq_num, len) != 0) {
The server sends multiple packets without waiting for an ACK by window control and if some packets are dropped, wget will return an ACK including the dropped packets. Following log indicates this issue. wget_handler() wget: Transferring, seq=97bbdd4a, ack=30,len=580 wget_handler() wget: Transferring, seq=97bbedca, ack=30,len=580 First packet of TCP sequence number is 0x97bbdd4a. Second packet of TCP sequence number should be 0x97bbe2ca, however it is 0x97bbedca and returns its ACK, so the server suppose that 0x97bbe2ca and 0x97bbedca are received appropriately. In this case, 0x97bbe2ca was lost and the data of wget was broken. In this patch, next_data_seq_num holds the next expected TCP sequence number. If the TCP sequence number different from next_data_seq_num, trying to retransmit the packet. Signed-off-by: Yasuharu Shibata <yasuharu.shibata@gmail.com> --- net/wget.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)