diff mbox series

[v5,18/20] net-lwip: add TFTP_BLOCKSIZE

Message ID 4dee5a89157f3dd0e0a8d17ab6ff42292bb7ce79.1721910373.git.jerome.forissier@linaro.org
State Superseded
Headers show
Series Introduce the lwIP network stack | expand

Commit Message

Jérôme Forissier July 25, 2024, 12:57 p.m. UTC
Add support for setting the TFTP block size. The default value (1468)
is fine for Ethernet and allows a better throughput than the TFTP
default (512), if the server supports the blksize option of course.

I tested this change with qemu_arm64_lwip_defconfig. The throughput is
now 875 KiB/s vs. 313 KiB/s before. That is still a low number, but I
think we can't expect more without implementing the windowsize option.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 net/Kconfig     | 20 ++++++++++----------
 net/lwip/tftp.c |  2 ++
 2 files changed, 12 insertions(+), 10 deletions(-)

Comments

Ilias Apalodimas July 29, 2024, 12:29 p.m. UTC | #1
Hi Jerome,

On Thu, 25 Jul 2024 at 15:59, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Add support for setting the TFTP block size. The default value (1468)
> is fine for Ethernet and allows a better throughput than the TFTP
> default (512), if the server supports the blksize option of course.
>
> I tested this change with qemu_arm64_lwip_defconfig. The throughput is
> now 875 KiB/s vs. 313 KiB/s before. That is still a low number, but I
> think we can't expect more without implementing the windowsize option.
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  net/Kconfig     | 20 ++++++++++----------
>  net/lwip/tftp.c |  2 ++
>  2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/net/Kconfig b/net/Kconfig
> index 952690d677d..7e5406bb923 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -69,16 +69,6 @@ config SYS_FAULT_ECHO_LINK_DOWN
>           this option is active, then CONFIG_SYS_FAULT_MII_ADDR also needs to
>           be configured.
>
> -config TFTP_BLOCKSIZE
> -       int "TFTP block size"
> -       default 1468
> -       help
> -         Default TFTP block size.
> -         The MTU is typically 1500 for ethernet, so a TFTP block of
> -         1468 (MTU minus eth.hdrs) provides a good throughput with
> -         almost-MTU block sizes.
> -         You can also activate CONFIG_IP_DEFRAG to set a larger block.
> -

Why do you have to move the config option?

Cheers
/Ilias
>  config TFTP_PORT
>         bool "Set TFTP UDP source/destination ports via the environment"
>         help
> @@ -263,4 +253,14 @@ config SYS_RX_ETH_BUFFER
>           since all buffers can be full shortly after enabling the interface on
>           high Ethernet traffic.
>
> +config TFTP_BLOCKSIZE
> +       int "TFTP block size"
> +       default 1468
> +       help
> +         Default TFTP block size.
> +         The MTU is typically 1500 for ethernet, so a TFTP block of
> +         1468 (MTU minus eth.hdrs) provides a good throughput with
> +         almost-MTU block sizes.
> +         You can also activate CONFIG_IP_DEFRAG to set a larger block.
> +
>  endif   # if NET || NET_LWIP
> diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c
> index 78de0bd0dba..1fe948c84ec 100644
> --- a/net/lwip/tftp.c
> +++ b/net/lwip/tftp.c
> @@ -136,6 +136,8 @@ static int tftp_loop(struct udevice *udev, ulong addr, char *fname,
>         if (!(err == ERR_OK || err == ERR_USE))
>                 log_err("tftp_init_client err: %d\n", err);
>
> +       tftp_client_set_blksize(CONFIG_TFTP_BLOCKSIZE);
> +
>         ctx.start_time = get_timer(0);
>         err = tftp_get(&ctx, &srvip, srvport, fname, TFTP_MODE_OCTET);
>         /* might return different errors, like routing problems */
> --
> 2.40.1
>
Jérôme Forissier July 29, 2024, 3:18 p.m. UTC | #2
Hi Ilias,

On 7/29/24 14:29, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> On Thu, 25 Jul 2024 at 15:59, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>>
>> Add support for setting the TFTP block size. The default value (1468)
>> is fine for Ethernet and allows a better throughput than the TFTP
>> default (512), if the server supports the blksize option of course.
>>
>> I tested this change with qemu_arm64_lwip_defconfig. The throughput is
>> now 875 KiB/s vs. 313 KiB/s before. That is still a low number, but I
>> think we can't expect more without implementing the windowsize option.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>  net/Kconfig     | 20 ++++++++++----------
>>  net/lwip/tftp.c |  2 ++
>>  2 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/Kconfig b/net/Kconfig
>> index 952690d677d..7e5406bb923 100644
>> --- a/net/Kconfig
>> +++ b/net/Kconfig
>> @@ -69,16 +69,6 @@ config SYS_FAULT_ECHO_LINK_DOWN
>>           this option is active, then CONFIG_SYS_FAULT_MII_ADDR also needs to
>>           be configured.
>>
>> -config TFTP_BLOCKSIZE
>> -       int "TFTP block size"
>> -       default 1468
>> -       help
>> -         Default TFTP block size.
>> -         The MTU is typically 1500 for ethernet, so a TFTP block of
>> -         1468 (MTU minus eth.hdrs) provides a good throughput with
>> -         almost-MTU block sizes.
>> -         You can also activate CONFIG_IP_DEFRAG to set a larger block.
>> -
> 
> Why do you have to move the config option?

It is moved from the 'if NET' block to the 'if NET || NET_LWIP' block since
it now applies to both.

Thanks,
Ilias Apalodimas July 30, 2024, 9:57 a.m. UTC | #3
On Mon, 29 Jul 2024 at 18:18, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Hi Ilias,
>
> On 7/29/24 14:29, Ilias Apalodimas wrote:
> > Hi Jerome,
> >
> > On Thu, 25 Jul 2024 at 15:59, Jerome Forissier
> > <jerome.forissier@linaro.org> wrote:
> >>
> >> Add support for setting the TFTP block size. The default value (1468)
> >> is fine for Ethernet and allows a better throughput than the TFTP
> >> default (512), if the server supports the blksize option of course.
> >>
> >> I tested this change with qemu_arm64_lwip_defconfig. The throughput is
> >> now 875 KiB/s vs. 313 KiB/s before. That is still a low number, but I
> >> think we can't expect more without implementing the windowsize option.
> >>
> >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> >> ---
> >>  net/Kconfig     | 20 ++++++++++----------
> >>  net/lwip/tftp.c |  2 ++
> >>  2 files changed, 12 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/net/Kconfig b/net/Kconfig
> >> index 952690d677d..7e5406bb923 100644
> >> --- a/net/Kconfig
> >> +++ b/net/Kconfig
> >> @@ -69,16 +69,6 @@ config SYS_FAULT_ECHO_LINK_DOWN
> >>           this option is active, then CONFIG_SYS_FAULT_MII_ADDR also needs to
> >>           be configured.
> >>
> >> -config TFTP_BLOCKSIZE
> >> -       int "TFTP block size"
> >> -       default 1468
> >> -       help
> >> -         Default TFTP block size.
> >> -         The MTU is typically 1500 for ethernet, so a TFTP block of
> >> -         1468 (MTU minus eth.hdrs) provides a good throughput with
> >> -         almost-MTU block sizes.
> >> -         You can also activate CONFIG_IP_DEFRAG to set a larger block.
> >> -
> >
> > Why do you have to move the config option?
>
> It is moved from the 'if NET' block to the 'if NET || NET_LWIP' block since
> it now applies to both.
>
> Thanks,
> --
> Jerome
>
> >
> > Cheers
> > /Ilias
> >>  config TFTP_PORT
> >>         bool "Set TFTP UDP source/destination ports via the environment"
> >>         help
> >> @@ -263,4 +253,14 @@ config SYS_RX_ETH_BUFFER
> >>           since all buffers can be full shortly after enabling the interface on
> >>           high Ethernet traffic.
> >>
> >> +config TFTP_BLOCKSIZE
> >> +       int "TFTP block size"
> >> +       default 1468
> >> +       help
> >> +         Default TFTP block size.
> >> +         The MTU is typically 1500 for ethernet, so a TFTP block of
> >> +         1468 (MTU minus eth.hdrs) provides a good throughput with
> >> +         almost-MTU block sizes.
> >> +         You can also activate CONFIG_IP_DEFRAG to set a larger block.
> >> +
> >>  endif   # if NET || NET_LWIP
> >> diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c
> >> index 78de0bd0dba..1fe948c84ec 100644
> >> --- a/net/lwip/tftp.c
> >> +++ b/net/lwip/tftp.c
> >> @@ -136,6 +136,8 @@ static int tftp_loop(struct udevice *udev, ulong addr, char *fname,
> >>         if (!(err == ERR_OK || err == ERR_USE))
> >>                 log_err("tftp_init_client err: %d\n", err);
> >>
> >> +       tftp_client_set_blksize(CONFIG_TFTP_BLOCKSIZE);
> >> +
> >>         ctx.start_time = get_timer(0);
> >>         err = tftp_get(&ctx, &srvip, srvport, fname, TFTP_MODE_OCTET);
> >>         /* might return different errors, like routing problems */
> >> --
> >> 2.40.1
> >>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff mbox series

Patch

diff --git a/net/Kconfig b/net/Kconfig
index 952690d677d..7e5406bb923 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -69,16 +69,6 @@  config SYS_FAULT_ECHO_LINK_DOWN
 	  this option is active, then CONFIG_SYS_FAULT_MII_ADDR also needs to
 	  be configured.
 
-config TFTP_BLOCKSIZE
-	int "TFTP block size"
-	default 1468
-	help
-	  Default TFTP block size.
-	  The MTU is typically 1500 for ethernet, so a TFTP block of
-	  1468 (MTU minus eth.hdrs) provides a good throughput with
-	  almost-MTU block sizes.
-	  You can also activate CONFIG_IP_DEFRAG to set a larger block.
-
 config TFTP_PORT
 	bool "Set TFTP UDP source/destination ports via the environment"
 	help
@@ -263,4 +253,14 @@  config SYS_RX_ETH_BUFFER
 	  since all buffers can be full shortly after enabling the interface on
 	  high Ethernet traffic.
 
+config TFTP_BLOCKSIZE
+	int "TFTP block size"
+	default 1468
+	help
+	  Default TFTP block size.
+	  The MTU is typically 1500 for ethernet, so a TFTP block of
+	  1468 (MTU minus eth.hdrs) provides a good throughput with
+	  almost-MTU block sizes.
+	  You can also activate CONFIG_IP_DEFRAG to set a larger block.
+
 endif   # if NET || NET_LWIP
diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c
index 78de0bd0dba..1fe948c84ec 100644
--- a/net/lwip/tftp.c
+++ b/net/lwip/tftp.c
@@ -136,6 +136,8 @@  static int tftp_loop(struct udevice *udev, ulong addr, char *fname,
 	if (!(err == ERR_OK || err == ERR_USE))
 		log_err("tftp_init_client err: %d\n", err);
 
+	tftp_client_set_blksize(CONFIG_TFTP_BLOCKSIZE);
+
 	ctx.start_time = get_timer(0);
 	err = tftp_get(&ctx, &srvip, srvport, fname, TFTP_MODE_OCTET);
 	/* might return different errors, like routing problems */