mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/5] tftp: skip memory allocation when seeking to current position
@ 2026-03-12 14:44 Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 2/5] tftp: warn when seek-heavy access wastes network bandwidth Ahmad Fatoum
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-03-12 14:44 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

We currently allocate 1K of memory, only to discard it when seeking to
the same offset, we are currently at.

Restructure tftp_lseek() to use early returns for the unsupported
backward-seek and early-exit also for the no-op case of new pos being
the same as the old pos.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/tftp.c | 63 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index a454306b4b5a..dd4804041cde 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -902,42 +902,45 @@ static int tftp_read(struct file *f, void *buf, size_t insize)
 
 static int tftp_lseek(struct file *f, loff_t pos)
 {
-	/* We cannot seek backwards without reloading or caching the file */
+	int ret = 0;
+	char *buf;
 	loff_t f_pos = f->f_pos;
 
-	if (pos >= f_pos) {
-		int ret = 0;
-		char *buf = xmalloc(1024);
-
-		while (pos > f_pos) {
-			size_t len = min_t(size_t, 1024, pos - f_pos);
-
-			ret = tftp_read(f, buf, len);
-
-			if (!ret)
-				/* EOF, so the desired pos is invalid. */
-				ret = -EINVAL;
-			if (ret < 0)
-				goto out_free;
-
-			f_pos += ret;
-		}
-
-out_free:
-		free(buf);
-		if (ret < 0) {
-			/*
-			 * Update f->pos even if the overall request
-			 * failed since we can't move backwards
-			 */
-			f->f_pos = f_pos;
-			return ret;
-		}
+	/* We cannot seek backwards without reloading or caching the file */
+	if (pos < f_pos)
+		return -ENOSYS;
 
+	if (pos == f_pos)
 		return 0;
+
+	buf = xmalloc(1024);
+
+	while (pos > f_pos) {
+		size_t len = min_t(size_t, 1024, pos - f_pos);
+
+		ret = tftp_read(f, buf, len);
+
+		if (!ret)
+			/* EOF, so the desired pos is invalid. */
+			ret = -EINVAL;
+		if (ret < 0)
+			goto out_free;
+
+		f_pos += ret;
 	}
 
-	return -ENOSYS;
+out_free:
+	free(buf);
+	if (ret < 0) {
+		/*
+		 * Update f->pos even if the overall request
+		 * failed since we can't move backwards
+		 */
+		f->f_pos = f_pos;
+		return ret;
+	}
+
+	return 0;
 }
 
 static const struct inode_operations tftp_file_inode_operations;
-- 
2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/5] tftp: warn when seek-heavy access wastes network bandwidth
  2026-03-12 14:44 [PATCH 1/5] tftp: skip memory allocation when seeking to current position Ahmad Fatoum
@ 2026-03-12 14:44 ` Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 3/5] tftp: emulate backwards seek by reopening the file Ahmad Fatoum
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-03-12 14:44 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Patterns that work ok-ish on local file systems like opening a file
repeatedly and/or seeking to different offsets can have catastrophic
performance on TFTP, because every seek must read and discard data up to
the desired offset.

Add a lightweight heuristic accumulating the total bytes discarded by
seeks and emit a one-time warning, once we exceed 4 MiB of wasted reads.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/tftp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/tftp.c b/fs/tftp.c
index dd4804041cde..e5a276ed5cae 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -902,6 +902,7 @@ static int tftp_read(struct file *f, void *buf, size_t insize)
 
 static int tftp_lseek(struct file *f, loff_t pos)
 {
+	static loff_t seek_discard_total;
 	int ret = 0;
 	char *buf;
 	loff_t f_pos = f->f_pos;
@@ -931,6 +932,13 @@ static int tftp_lseek(struct file *f, loff_t pos)
 
 out_free:
 	free(buf);
+
+	seek_discard_total += f_pos - f->f_pos;
+	if (seek_discard_total > SZ_4M)
+		pr_warn_once("excessive seeking (%lld bytes discarded so far);"
+			     " consider copying file to RAM first?\n",
+			     seek_discard_total);
+
 	if (ret < 0) {
 		/*
 		 * Update f->pos even if the overall request
-- 
2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/5] tftp: emulate backwards seek by reopening the file
  2026-03-12 14:44 [PATCH 1/5] tftp: skip memory allocation when seeking to current position Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 2/5] tftp: warn when seek-heavy access wastes network bandwidth Ahmad Fatoum
@ 2026-03-12 14:44 ` Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 4/5] libfile: drop TFTP-specific hack in open_fdt() Ahmad Fatoum
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-03-12 14:44 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

TFTP is a streaming protocol with no native seek support.
We emulate forward seeks are emulated by reading and discarding data.
Backwards seeks were previously rejected with -ENOSYS, breaking higher
level code that assumes seeking to be possible.

Given that we have a heuristic in-place for detecting excessive data
discards through seeking, let's implement backwards seeking to allow the
pattern of reading the start of a buffer to detect type at one location
and then reading everything at once at a later time.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/tftp.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index e5a276ed5cae..8683c3841c68 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -822,6 +822,9 @@ static int tftp_close(struct inode *inode, struct file *f)
 {
 	struct file_priv *priv = f->private_data;
 
+	if (!priv)
+		return 0;
+
 	return tftp_do_close(priv);
 }
 
@@ -831,6 +834,9 @@ static int tftp_write(struct file *f, const void *inbuf, size_t insize)
 	size_t size, now;
 	int ret;
 
+	if (!priv)
+		return -EREMOTEIO;
+
 	pr_vdebug("%s: %zu\n", __func__, insize);
 
 	size = insize;
@@ -866,6 +872,9 @@ static int tftp_read(struct file *f, void *buf, size_t insize)
 	size_t outsize = 0, now;
 	int ret = 0;
 
+	if (!priv)
+		return -EREMOTEIO;
+
 	pr_vdebug("%s %zu\n", __func__, insize);
 
 	while (insize) {
@@ -902,14 +911,34 @@ static int tftp_read(struct file *f, void *buf, size_t insize)
 
 static int tftp_lseek(struct file *f, loff_t pos)
 {
+	struct file_priv *priv = f->private_data;
 	static loff_t seek_discard_total;
 	int ret = 0;
 	char *buf;
 	loff_t f_pos = f->f_pos;
 
+	if (!priv)
+		return -EREMOTEIO;
+
 	/* We cannot seek backwards without reloading or caching the file */
-	if (pos < f_pos)
-		return -ENOSYS;
+	if (pos < f_pos) {
+		/* We can reopen read streams, but not write streams */
+		if (priv->push)
+			return -ENOSYS;
+
+		tftp_do_close(priv);
+
+		priv = tftp_do_open(&f->fsdev->dev, f->f_flags,
+				    f->f_path.dentry, false);
+		if (IS_ERR(priv)) {
+			f->private_data = NULL;
+			return PTR_ERR(priv);
+		}
+
+		f->private_data = priv;
+		f->f_pos = 0;
+		f_pos = 0;
+	}
 
 	if (pos == f_pos)
 		return 0;
-- 
2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 4/5] libfile: drop TFTP-specific hack in open_fdt()
  2026-03-12 14:44 [PATCH 1/5] tftp: skip memory allocation when seeking to current position Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 2/5] tftp: warn when seek-heavy access wastes network bandwidth Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 3/5] tftp: emulate backwards seek by reopening the file Ahmad Fatoum
@ 2026-03-12 14:44 ` Ahmad Fatoum
  2026-03-12 14:44 ` [PATCH 5/5] fs: fix relative path resolution when CWD is on a TFTP mount Ahmad Fatoum
  2026-03-17 10:21 ` [PATCH 1/5] tftp: skip memory allocation when seeking to current position Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-03-12 14:44 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Now that TFTP internally does the equivalent of closing and reopening
a file when seeking back, switch to using that in open_fdt() instead
of open-coding it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/libfile.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/libfile.c b/lib/libfile.c
index f5cea7b94998..0670ef0584cb 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -976,7 +976,7 @@ int open_fdt(const char *filename, size_t *size)
 	if (ret)
 		goto err;
 
-	ret = pread_full(fd, &fdt_hdr, sizeof(fdt_hdr), 0);
+	ret = read_full(fd, &fdt_hdr, sizeof(fdt_hdr));
 	if (ret >= 0 && ret < sizeof(fdt_hdr))
 		ret = -EILSEQ;
 	if (ret < 0)
@@ -988,13 +988,12 @@ int open_fdt(const char *filename, size_t *size)
 		goto err;
 	}
 
-	close(fd);
-
-	/* HACK: TFTP doesn't support backwards seeking, so reopen afresh */
-	fd = open(filename, O_RDONLY);
-	if (fd >= 0)
-		*size = fdt_size;
+	if (lseek(fd, 0, SEEK_SET) != 0) {
+		ret = -errno;
+		goto err;
+	}
 
+        *size = fdt_size;
 	return fd;
 err:
 	close(fd);
-- 
2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 5/5] fs: fix relative path resolution when CWD is on a TFTP mount
  2026-03-12 14:44 [PATCH 1/5] tftp: skip memory allocation when seeking to current position Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2026-03-12 14:44 ` [PATCH 4/5] libfile: drop TFTP-specific hack in open_fdt() Ahmad Fatoum
@ 2026-03-12 14:44 ` Ahmad Fatoum
  2026-03-17 10:21 ` [PATCH 1/5] tftp: skip memory allocation when seeking to current position Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2026-03-12 14:44 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum, Claude Opus 4.6

When the current working directory is on a TFTP mount, relative paths
containing subdirectories (e.g. 'subdir/file') fail because
link_path_walk() tries to look up the first component as a directory,
which TFTP does not support.

The in-loop dentry_is_tftp() check already handles separator switching
for absolute paths and './'-prefixed relative paths, but misses the
case where the starting dentry itself is a TFTP dentry.

Check for a TFTP dentry before entering the walk loop and switch the
separator to 0x1 upfront, so the entire relative path is collapsed
into a single TFTP filename.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/fs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 6a73a5baa26e..0a151607dd7d 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2140,6 +2140,14 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 	if (!*name)
 		return 0;
 
+	/*
+	 * If we are starting from a TFTP dentry (e.g. CWD is on a TFTP
+	 * mount), switch to the TFTP separator immediately so the first
+	 * component isn't mistakenly looked up as a directory.
+	 */
+	if (dentry_is_tftp(nd->path.dentry))
+		separator = 0x1;
+
 	/* At this point we know we have a real path component. */
 	for(;;) {
 		int len;
-- 
2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/5] tftp: skip memory allocation when seeking to current position
  2026-03-12 14:44 [PATCH 1/5] tftp: skip memory allocation when seeking to current position Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2026-03-12 14:44 ` [PATCH 5/5] fs: fix relative path resolution when CWD is on a TFTP mount Ahmad Fatoum
@ 2026-03-17 10:21 ` Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2026-03-17 10:21 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Thu, 12 Mar 2026 15:44:21 +0100, Ahmad Fatoum wrote:
> We currently allocate 1K of memory, only to discard it when seeking to
> the same offset, we are currently at.
> 
> Restructure tftp_lseek() to use early returns for the unsupported
> backward-seek and early-exit also for the no-op case of new pos being
> the same as the old pos.
> 
> [...]

Applied, thanks!

[1/5] tftp: skip memory allocation when seeking to current position
      https://git.pengutronix.de/cgit/barebox/commit/?id=4eb2b9845bd4 (link may not be stable)
[2/5] tftp: warn when seek-heavy access wastes network bandwidth
      https://git.pengutronix.de/cgit/barebox/commit/?id=076a94b6e083 (link may not be stable)
[3/5] tftp: emulate backwards seek by reopening the file
      https://git.pengutronix.de/cgit/barebox/commit/?id=ac2ff2fe4306 (link may not be stable)
[4/5] libfile: drop TFTP-specific hack in open_fdt()
      https://git.pengutronix.de/cgit/barebox/commit/?id=ea481907e334 (link may not be stable)
[5/5] fs: fix relative path resolution when CWD is on a TFTP mount
      https://git.pengutronix.de/cgit/barebox/commit/?id=0528a6a5585a (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-17 10:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-12 14:44 [PATCH 1/5] tftp: skip memory allocation when seeking to current position Ahmad Fatoum
2026-03-12 14:44 ` [PATCH 2/5] tftp: warn when seek-heavy access wastes network bandwidth Ahmad Fatoum
2026-03-12 14:44 ` [PATCH 3/5] tftp: emulate backwards seek by reopening the file Ahmad Fatoum
2026-03-12 14:44 ` [PATCH 4/5] libfile: drop TFTP-specific hack in open_fdt() Ahmad Fatoum
2026-03-12 14:44 ` [PATCH 5/5] fs: fix relative path resolution when CWD is on a TFTP mount Ahmad Fatoum
2026-03-17 10:21 ` [PATCH 1/5] tftp: skip memory allocation when seeking to current position Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox