From patchwork Sat Jul 18 20:31:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ramon Fried X-Patchwork-Id: 1331608 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=85.214.62.61; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=KfwqNHwV; dkim-atps=neutral Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4B8KRK4pbBz9sRR for ; Sun, 19 Jul 2020 06:32:11 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 69DFD8158B; Sat, 18 Jul 2020 22:32:02 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="KfwqNHwV"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BCB59819E8; Sat, 18 Jul 2020 22:32:00 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,SPF_HELO_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id AF8BE80E47 for ; Sat, 18 Jul 2020 22:31:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=rfried.dev@gmail.com Received: by mail-wm1-x342.google.com with SMTP id q15so18816882wmj.2 for ; Sat, 18 Jul 2020 13:31:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=HfXrklsrN0Bg3p7wuPcImcWG8RjD00rQPvMTQC8HZng=; b=KfwqNHwVPOC/ejLUyZxzQ2skWZm00li+maW0CrGcMBDT7G3Qml/1AjzGgM8Q7N4jrG qmmPRUv5L/Qeb5vd5GY7T8slrgoIGg5UpH6UpYFDWkYOU2Xugy6m16sMhSo2sNXr8cnW LF/lc+fnTqNQPQAul/1UllzbBb0AxpeVI9q9sApwU+3xLNZaqhNSExVBkwPXMCsN5fSe c1+E4xzXQt94FxDTlduEgDSKgnmQnyy6s6AUpcQ7tEMYHEO+61pzUQyBKROyHo7mPoWL Y9ybCpfT5MLRHPj3/W1XUp8EaGst70Kb6wVGK28PBUZtSFXDfVKQQC8G7iV051DLg1// EJ3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=HfXrklsrN0Bg3p7wuPcImcWG8RjD00rQPvMTQC8HZng=; b=tWv54aK0EEpx2M5shvUcf3LX6KTI5DtyxG9Hr2jjSAY33yIflvyjJ7+np3clvjqmVs zhHgC4x84+FZueq7EVqYT8UPmWkZF5l+hhe+AFADzU4CrFAnA/6ew28bM227PVwVEINt 3jbhsdXg4BKb+vrsnIrDfqoXwHVOlOwCE6baSOBH1ms8hB1O05PCzO/X1rf7S594Cywu chtrV6Nx063vJ/AbyBGWeauzQK68cE0StYyw0IMwxecdXl4Wr1592lmZ+v71u4ECjxSd Z0fHI69BXi0z6/uXKRflgmhHv5Wl+WL+/RTyvqm8Mfutj2yn26kg28oVMUyeQLP3Di8B Z+jg== X-Gm-Message-State: AOAM530+QxaFX0nDZJ2LkyE2L+5v9S/5g80XdT6ArdUEjG596RI15EbU UE3bYE8qmGxG6W3Bb8eRVvI= X-Google-Smtp-Source: ABdhPJz06l/d40PHPUZX8ISa7GDNb6JKUsvfEuj5CECXRg9Bm6mozXGO7wvH+OZtEhNeyt39RzS7zw== X-Received: by 2002:a7b:cf18:: with SMTP id l24mr14661881wmg.116.1595104317164; Sat, 18 Jul 2020 13:31:57 -0700 (PDT) Received: from localhost.localdomain ([37.142.6.51]) by smtp.gmail.com with ESMTPSA id 26sm20806258wmj.25.2020.07.18.13.31.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Jul 2020 13:31:56 -0700 (PDT) From: Ramon Fried To: joe.hershberger@ni.com, marex@denx.de, b.galvani@gmail.com, rahasij@linux.microsoft.com Cc: u-boot@lists.denx.de, Ramon Fried Subject: [PATCH v5] net: tftp: Add client support for RFC 7440 Date: Sat, 18 Jul 2020 23:31:46 +0300 Message-Id: <20200718203146.317254-1-rfried.dev@gmail.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.3 at phobos.denx.de X-Virus-Status: Clean Add support for RFC 7440: "TFTP Windowsize Option". This optional feature allows the client and server to negotiate a window size of consecutive blocks to send as an alternative for replacing the single-block lockstep schema. windowsize can be defined statically during compilation by setting CONFIG_TFTP_WINDOWSIZE, or defined in runtime by setting an environment variable: "tftpwindowsize" If not defined, the windowsize is set to 1, meaning that it behaves as it was never defined. Choosing the appropriate windowsize depends on the specific network topology, underlying NIC. You should test various windowsize scenarios and see which best work for you. Setting a windowsize too big can actually decreases performance. Signed-off-by: Ramon Fried Reviewed-by: Marek Vasut --- v2: * Don't send windowsize option on tftpput, as it's not implemented yet. * Don't send NACK for every out of order block that arrives, one nack is enough. v3: * Add option CONFIG_TFTP_WINDOWSIZE to kconfig with default 1. * Fixed some spelling errors. * Took assignment out of a loop. * simplified variable increment. v4: * send ack for last packet, so the server can finish the tranfer gracefully and not in timeout. v5: * rebase the patch on top of latest tftp changes. * Fix wraparound issue in tftp_cur_block increment. * Change strcmp to strcasecmp README | 5 ++++ net/Kconfig | 9 ++++++ net/tftp.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/README b/README index 2384966a39..2ebf664848 100644 --- a/README +++ b/README @@ -3473,6 +3473,11 @@ List of environment variables (most likely not complete): downloads succeed with high packet loss rates, or with unreliable TFTP servers or client hardware. + tftpwindowsize - if this is set, the value is used for TFTP's + window size as described by RFC 7440. + This means the count of blocks we can receive before + sending ack to server. + vlan - When set to a value < 4095 the traffic over Ethernet is encapsulated/received over 802.1q VLAN tagged frames. diff --git a/net/Kconfig b/net/Kconfig index ac6d0cf8a6..7916ae305f 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -49,4 +49,13 @@ config TFTP_BLOCKSIZE almost-MTU block sizes. You can also activate CONFIG_IP_DEFRAG to set a larger block. +config TFTP_WINDOWSIZE + int "TFTP window size" + default 1 + help + Default TFTP window size. + RFC7440 defines an optional window size of transmits, + before an ack response is required. + The default TFTP implementation implies a window size of 1. + endif # if NET diff --git a/net/tftp.c b/net/tftp.c index c05b7b5532..84e970bec1 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -5,7 +5,6 @@ * Copyright 2011 Comelit Group SpA, * Luca Ceresoli */ - #include #include #include @@ -98,6 +97,12 @@ static int tftp_tsize; /* The number of hashes we printed */ static short tftp_tsize_num_hash; #endif +/* The window size negotiated */ +static ushort tftp_windowsize; +/* Next block to send ack to */ +static ushort tftp_next_ack; +/* Last nack block we send */ +static ushort tftp_last_nack; #ifdef CONFIG_CMD_TFTPPUT /* 1 if writing, else 0 */ static int tftp_put_active; @@ -138,8 +143,19 @@ static char tftp_filename[MAX_LEN]; * (but those using CONFIG_IP_DEFRAG may want to set a larger block in cfg file) */ +/* When windowsize is defined to 1, + * tftp behaves the same way as it was + * never declared + */ +#ifdef CONFIG_TFTP_WINDOWSIZE +#define TFTP_WINDOWSIZE CONFIG_TFTP_WINDOWSIZE +#else +#define TFTP_WINDOWSIZE 1 +#endif + static unsigned short tftp_block_size = TFTP_BLOCK_SIZE; static unsigned short tftp_block_size_option = CONFIG_TFTP_BLOCKSIZE; +static unsigned short tftp_window_size_option = TFTP_WINDOWSIZE; static inline int store_block(int block, uchar *src, unsigned int len) { @@ -356,6 +372,14 @@ static void tftp_send(void) /* try for more effic. blk size */ pkt += sprintf((char *)pkt, "blksize%c%d%c", 0, tftp_block_size_option, 0); + + /* try for more effic. window size. + * Implemented only for tftp get. + * Don't bother sending if it's 1 + */ + if (tftp_state == STATE_SEND_RRQ && tftp_window_size_option > 1) + pkt += sprintf((char *)pkt, "windowsize%c%d%c", + 0, tftp_window_size_option, 0); len = pkt - xp; break; @@ -550,7 +574,17 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, (char *)pkt + i + 6, tftp_tsize); } #endif + if (strcasecmp((char *)pkt + i, "windowsize") == 0) { + tftp_windowsize = + simple_strtoul((char *)pkt + i + 11, + NULL, 10); + debug("windowsize = %s, %d\n", + (char *)pkt + i + 11, tftp_windowsize); + } } + + tftp_next_ack = tftp_windowsize; + #ifdef CONFIG_CMD_TFTPPUT if (tftp_put_active && tftp_state == STATE_OACK) { /* Get ready to send the first block */ @@ -564,7 +598,28 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, if (len < 2) return; len -= 2; - tftp_cur_block = ntohs(*(__be16 *)pkt); + + if (ntohs(*(__be16 *)pkt) != (ushort)(tftp_cur_block + 1)) { + debug("Received unexpected block: %d, expected: %d\n", + ntohs(*(__be16 *)pkt), + (ushort)(tftp_cur_block + 1)); + /* + * If one packet is dropped most likely + * all other buffers in the window + * that will arrive will cause a sending NACK. + * This just overwellms the server, let's just send one. + */ + if (tftp_last_nack != tftp_cur_block) { + tftp_send(); + tftp_last_nack = tftp_cur_block; + tftp_next_ack = (ushort)(tftp_cur_block + + tftp_windowsize); + } + break; + } + + tftp_cur_block++; + tftp_cur_block %= TFTP_SEQUENCE_SIZE; if (tftp_state == STATE_SEND_RRQ) debug("Server did not acknowledge any options!\n"); @@ -606,10 +661,15 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, * Acknowledge the block just received, which will prompt * the remote for the next one. */ - tftp_send(); + if (tftp_cur_block == tftp_next_ack) { + tftp_send(); + tftp_next_ack += tftp_windowsize; + } - if (len < tftp_block_size) + if (len < tftp_block_size) { + tftp_send(); tftp_complete(); + } break; case TFTP_ERROR: @@ -683,6 +743,10 @@ void tftp_start(enum proto_t protocol) if (ep != NULL) tftp_block_size_option = simple_strtol(ep, NULL, 10); + ep = env_get("tftpwindowsize"); + if (ep != NULL) + tftp_window_size_option = simple_strtol(ep, NULL, 10); + ep = env_get("tftptimeout"); if (ep != NULL) timeout_ms = simple_strtol(ep, NULL, 10); @@ -704,8 +768,8 @@ void tftp_start(enum proto_t protocol) } #endif - debug("TFTP blocksize = %i, timeout = %ld ms\n", - tftp_block_size_option, timeout_ms); + debug("TFTP blocksize = %i, TFTP windowsize = %d timeout = %ld ms\n", + tftp_block_size_option, tftp_window_size_option, timeout_ms); tftp_remote_ip = net_server_ip; if (!net_parse_bootfile(&tftp_remote_ip, tftp_filename, MAX_LEN)) { @@ -801,7 +865,8 @@ void tftp_start(enum proto_t protocol) tftp_our_port = simple_strtol(ep, NULL, 10); #endif tftp_cur_block = 0; - + tftp_windowsize = 1; + tftp_last_nack = 0; /* zero out server ether in case the server ip has changed */ memset(net_server_ethaddr, 0, 6); /* Revert tftp_block_size to dflt */