diff mbox series

[OpenWrt-Devel,RFC] ath79: clarify purpose of factory image

Message ID 20200326155654.48317-1-freifunk@adrianschmutzler.de
State Under Review
Delegated to: Adrian Schmutzler
Headers show
Series [OpenWrt-Devel,RFC] ath79: clarify purpose of factory image | expand

Commit Message

Adrian Schmutzler March 26, 2020, 3:56 p.m. UTC
While the purpose of a factory image in general is to flash a
device with vendor OS "directly", some vagueness has evolved over
the years with respect to additional uses of these images.

One common case is when a device supports TFTP recovery.
Particularly with TP-Link devices in ar71xx/ath79, it is common
that the factory image can be flashed via TFTP without any additional
measures. In contrast, on some ramips devices the same procedure might
overwrite your u-boot partition and make the device unbootable.
However, in both cases you might only have a factory.bin which
won't reveal any further information just by itself.

To improve the situation at least a bit, this commit tries to
clarify the image names by introducing the following three schemes:

factory.bin - used from vendor OS, _not_ suitable for TFTP
factory-tftp.bin - used from vendor OS, _also_ suitable for TFTP
tftp.bin - can _not_ be used from vendor OS, but can be used via TFTP

Since factory.bin and tftp.bin are already used widely, this will
keep the impact relatively small by only adding the "combined"
factory-tftp.bin image name. No additional images are built, just
the name of the existing one is slightly adjusted for matching cases.
Despite, the name change as an indicator for the new TFTP capability
will have to be added manually, so in case of uncertainty the image
name will indicate the lesser functionality by default.

Thus, this patch introduces the factory-tftp.bin name for all devices
where TFTP flashing instructions are indicated by the commit message,
and for all TP-Link devices with v1 image/header or tplink-safeloader.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

This is meant as a base for discussion. I plan to do the same for
ramips later if this sees positive resonance.

Feel free to add information about devices I overlooked.

This is not even build-tested.
---
 target/linux/ath79/image/common-tp-link.mk  |  6 ++---
 target/linux/ath79/image/generic-tp-link.mk |  2 +-
 target/linux/ath79/image/generic.mk         |  4 +--
 target/linux/ath79/image/tiny-tp-link.mk    | 28 ++++++++++-----------
 4 files changed, 20 insertions(+), 20 deletions(-)

Comments

Jo-Philipp Wich March 26, 2020, 4:31 p.m. UTC | #1
Hi,

+1 from me. I think the approach makes sense.

~ Jo
Henrique de Moraes Holschuh March 26, 2020, 4:32 p.m. UTC | #2
On 26/03/2020 12:56, Adrian Schmutzler wrote:
> While the purpose of a factory image in general is to flash a
> device with vendor OS "directly", some vagueness has evolved over
> the years with respect to additional uses of these images.
> 
> One common case is when a device supports TFTP recovery.
> Particularly with TP-Link devices in ar71xx/ath79, it is common
> that the factory image can be flashed via TFTP without any additional
> measures. In contrast, on some ramips devices the same procedure might
> overwrite your u-boot partition and make the device unbootable.
> However, in both cases you might only have a factory.bin which
> won't reveal any further information just by itself.
> 
> To improve the situation at least a bit, this commit tries to
> clarify the image names by introducing the following three schemes:
> 
> factory.bin - used from vendor OS, _not_ suitable for TFTP
> factory-tftp.bin - used from vendor OS, _also_ suitable for TFTP
> tftp.bin - can _not_ be used from vendor OS, but can be used via TFTP

The name "tftp-recovery" (maybe "tftp_recovery") has already seen some 
use on images built for the purposes of being used for TFTP, maybe it 
would be better to keep that naming?
Adrian Schmutzler March 26, 2020, 4:42 p.m. UTC | #3
Hi,

> > tftp.bin - can _not_ be used from vendor OS, but can be used via TFTP
> 
> The name "tftp-recovery" (maybe "tftp_recovery") has already seen some
> use on images built for the purposes of being used for TFTP, maybe it would
> be better to keep that naming?

It looks like tftp.bin is used for ath79 and tftp-recovery.bin is used for ramips, while the latter are a few more (see grep below).

I'd definitely have that unified, though I have a weak tendency towards the shorter version (tftp.bin). Maybe somebody else has a taste on this?

Best

Adrian

---

adsc@buildfff:/data/openwrt$ grep -rn "tftp.*\.bin" target/linux/ | sort
target/linux/ar71xx/image/legacy.mk:345:                ) > $(call imgname,$(1),$(2))-tftp.bin; \
target/linux/ar71xx/image/legacy.mk:375:                ) > $(call imgname,$(1),$(2))-tftp.bin; \
target/linux/ath79/image/generic.mk:1190:  IMAGES += tftp.bin
target/linux/ath79/image/generic.mk:1191:  IMAGE/tftp.bin := $$(IMAGE/sysupgrade.bin) | yuncore-tftp-header-16m
target/linux/ath79/image/generic.mk:1201:  IMAGES += tftp.bin
target/linux/ath79/image/generic.mk:1202:  IMAGE/tftp.bin := $$(IMAGE/sysupgrade.bin) | yuncore-tftp-header-16m
target/linux/ath79/image/generic.mk:1212:  IMAGES += tftp.bin
target/linux/ath79/image/generic.mk:1213:  IMAGE/tftp.bin := $$(IMAGE/sysupgrade.bin) | yuncore-tftp-header-16m
target/linux/ath79/image/generic.mk:201:  IMAGES += factory.bin tftp.bin
target/linux/ath79/image/generic.mk:206:  IMAGE/tftp.bin := $$(IMAGE/default) | buffalo-tftp-header
target/linux/ath79/image/generic.mk:224:  IMAGES += factory.bin tftp.bin
target/linux/ath79/image/generic.mk:229:  IMAGE/tftp.bin := $$(IMAGE/default) | buffalo-tftp-header
target/linux/ath79/image/generic.mk:243:  IMAGES += factory.bin tftp.bin
target/linux/ath79/image/generic.mk:248:  IMAGE/tftp.bin := $$(IMAGE/default) | buffalo-tftp-header
target/linux/ath79/image/generic.mk:259:  IMAGES += factory.bin tftp.bin
target/linux/ath79/image/generic.mk:264:  IMAGE/tftp.bin := $$(IMAGE/default) | buffalo-tftp-header
target/linux/ath79/image/tiny.mk:13:  IMAGE/tftp.bin := $$(IMAGE/default) | buffalo-tftp-header
target/linux/ath79/image/tiny.mk:8:  IMAGES += factory.bin tftp.bin
target/linux/layerscape/README:109:  => tftp a0000000 <firmware_name>-firmware.bin
target/linux/layerscape/README:123:  => tftp a0000000 <firmware_name>-firmware.bin
target/linux/layerscape/README:142:  => tftp 96000000 <firmware_name>-firmware.bin
target/linux/layerscape/README:78:  => tftp a0000000 <firmware_name>-firmware.bin
target/linux/layerscape/README:89:  => tftp a0000000 <firmware_name>-firmware.bin
target/linux/layerscape/README:99:  => tftp a0000000 <firmware_name>-firmware.bin
target/linux/ramips/image/mt76x8.mk:258:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:259:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:287:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:288:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:338:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:339:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:353:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:354:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:366:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:367:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:379:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:380:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:392:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:393:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:420:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:421:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:434:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:435:  IMAGE/tftp-recovery.bin := pad-extra 64k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:449:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:450:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/mt76x8.mk:465:  IMAGES := sysupgrade.bin tftp-recovery.bin
target/linux/ramips/image/mt76x8.mk:466:  IMAGE/tftp-recovery.bin := pad-extra 128k | $$(IMAGE/factory.bin)
target/linux/ramips/image/rt305x.mk:303:  IMAGES += tftp.bin
target/linux/ramips/image/rt305x.mk:304:  IMAGE/tftp.bin := $$(sysupgrade_bin) | check-size | \
Bjørn Mork March 26, 2020, 5:18 p.m. UTC | #4
Adrian Schmutzler <freifunk@adrianschmutzler.de> writes:

> diff --git a/target/linux/ath79/image/generic-tp-link.mk b/target/linux/ath79/image/generic-tp-link.mk
> index 4c925cf850..0e2a56a6d5 100644
> --- a/target/linux/ath79/image/generic-tp-link.mk
> +++ b/target/linux/ath79/image/generic-tp-link.mk
> @@ -166,7 +166,7 @@ define Device/tplink_archer-c7-v2
>  	ath10k-firmware-qca988x-ct
>    TPLINK_HWID := 0xc7000002
>    SUPPORTED_DEVICES += archer-c7
> -  IMAGES += factory-us.bin factory-eu.bin
> +  IMAGES += factory-tftp-us.bin factory-tftp-eu.bin
>    IMAGE/factory-us.bin := tplink-v1-image factory -C US
>    IMAGE/factory-eu.bin := tplink-v1-image factory -C EU
>  endef

Forgot to update the image definitions here?


Bjørn
Chuanhong Guo March 27, 2020, 12:41 p.m. UTC | #5
Hi!

On Thu, Mar 26, 2020 at 11:57 PM Adrian Schmutzler
<freifunk@adrianschmutzler.de> wrote:
>
> While the purpose of a factory image in general is to flash a
> device with vendor OS "directly", some vagueness has evolved over
> the years with respect to additional uses of these images.

I think factory image is supposed to be packaged in exactly the
same way as vendor firmware upgrade package, which can be
flashed however vendor ones can be flashed. (e.g. firmware
upgrade page in web interface or user-friendly firmware recovery
methods designed by vendors.)

> One common case is when a device supports TFTP recovery.
> Particularly with TP-Link devices in ar71xx/ath79, it is common
> that the factory image can be flashed via TFTP without any additional
> measures.

These tftp recovery methods are designed by vendors and are
supposed to take their own firmware upgrade packages as well.

> In contrast, on some ramips devices the same procedure might
> overwrite your u-boot partition and make the device unbootable.

This only happens when vendors don't provide an official tftp
recovery method and users are at on their own risk by trying
to use factory images with these undocumented bootloader
features.

> However, in both cases you might only have a factory.bin which
> won't reveal any further information just by itself.
>
> To improve the situation at least a bit, this commit tries to
> clarify the image names by introducing the following three schemes:
>
> factory.bin - used from vendor OS, _not_ suitable for TFTP
> factory-tftp.bin - used from vendor OS, _also_ suitable for TFTP

I think there's no need to differentiate these two variants.
We should instead make it clear that factory should be
used the same way as vendor fw image and name other
images by their use cases like tftp/recovery etc.

> tftp.bin - can _not_ be used from vendor OS, but can be used via TFTP
>
> Since factory.bin and tftp.bin are already used widely, this will
> keep the impact relatively small by only adding the "combined"
> factory-tftp.bin image name. No additional images are built, just
> the name of the existing one is slightly adjusted for matching cases.
> Despite, the name change as an indicator for the new TFTP capability
> will have to be added manually, so in case of uncertainty the image
> name will indicate the lesser functionality by default.
>
> Thus, this patch introduces the factory-tftp.bin name for all devices
> where TFTP flashing instructions are indicated by the commit message,
> and for all TP-Link devices with v1 image/header or tplink-safeloader.

TP-Link doesn't provide an official tftp recovery method for
their earlier devices (most devices with tp-link v1 headers).
In order to unbrick these devices, one would need to attach
serial console and use u-boot command line to manually
erase a certain area on flash and write new firmware image
to it. I think it doesn't worth to mark these factory images as
'tftp capable'.

BTW I have no idea about how tplink-safeloader stuff works.
Enrico Mioso March 27, 2020, 1:04 p.m. UTC | #6
Hello all!!

I do agree about this idea, but I think there is the general need for more clarity. When adding support for a device, it could be good if the author could elaborate on:
- how a device can be recovered: detailing recovery methods and nuances, so the user knows upfront how risky is an operation and prepare with the needed tools in advance
- stating what generated images can be used for when it's not obvious: (e.g.: NETGEAR NMRP + tftp) and so on.

The situation changes a lot per device, so elaborating a little bit more may be a good idea. Infact I think I saw some device where you can install OpenWRt only via tftp.

Regarding TP-Link: I can confirm that ramips TP-Link TL-MR200 <sarcasm> NICELY </sarcams> overwrites it's u-boot when proceeding with a tftp recovery.
TP-Link RE450 doesn't provide for recovery at all: an UART is needed.
diff mbox series

Patch

diff --git a/target/linux/ath79/image/common-tp-link.mk b/target/linux/ath79/image/common-tp-link.mk
index 328eaaed30..ed636ed7fd 100644
--- a/target/linux/ath79/image/common-tp-link.mk
+++ b/target/linux/ath79/image/common-tp-link.mk
@@ -17,9 +17,9 @@  define Device/tplink-v1
   LOADER_TYPE := gz
   KERNEL := kernel-bin | append-dtb | lzma
   KERNEL_INITRAMFS := kernel-bin | append-dtb | lzma | tplink-v1-header
-  IMAGES += factory.bin
+  IMAGES += factory-tftp.bin
   IMAGE/sysupgrade.bin := tplink-v1-image sysupgrade | append-metadata
-  IMAGE/factory.bin := tplink-v1-image factory
+  IMAGE/factory-tftp.bin := tplink-v1-image factory
 endef
 
 define Device/tplink-v2
@@ -80,7 +80,7 @@  define Device/tplink-safeloader
   KERNEL := kernel-bin | append-dtb | lzma | tplink-v1-header -O
   IMAGE/sysupgrade.bin := append-rootfs | tplink-safeloader sysupgrade | \
 	append-metadata | check-size
-  IMAGE/factory.bin := append-rootfs | tplink-safeloader factory
+  IMAGE/factory-tftp.bin := append-rootfs | tplink-safeloader factory
 endef
 
 define Device/tplink-safeloader-uimage
diff --git a/target/linux/ath79/image/generic-tp-link.mk b/target/linux/ath79/image/generic-tp-link.mk
index 4c925cf850..0e2a56a6d5 100644
--- a/target/linux/ath79/image/generic-tp-link.mk
+++ b/target/linux/ath79/image/generic-tp-link.mk
@@ -166,7 +166,7 @@  define Device/tplink_archer-c7-v2
 	ath10k-firmware-qca988x-ct
   TPLINK_HWID := 0xc7000002
   SUPPORTED_DEVICES += archer-c7
-  IMAGES += factory-us.bin factory-eu.bin
+  IMAGES += factory-tftp-us.bin factory-tftp-eu.bin
   IMAGE/factory-us.bin := tplink-v1-image factory -C US
   IMAGE/factory-eu.bin := tplink-v1-image factory -C EU
 endef
diff --git a/target/linux/ath79/image/generic.mk b/target/linux/ath79/image/generic.mk
index aac89e9269..53cdd04c1e 100644
--- a/target/linux/ath79/image/generic.mk
+++ b/target/linux/ath79/image/generic.mk
@@ -574,8 +574,8 @@  define Device/engenius_epg5000
   DEVICE_MODEL := EPG5000
   DEVICE_PACKAGES := ath10k-firmware-qca988x-ct kmod-ath10k-ct kmod-usb2
   IMAGE_SIZE := 14656k
-  IMAGES += factory.dlf
-  IMAGE/factory.dlf := append-kernel | pad-to $$$$(BLOCKSIZE) | \
+  IMAGES += factory-tftp.dlf
+  IMAGE/factory-tftp.dlf := append-kernel | pad-to $$$$(BLOCKSIZE) | \
 	append-rootfs | pad-rootfs | check-size | \
 	senao-header -r 0x101 -p 0x71 -t 2
   SUPPORTED_DEVICES += epg5000
diff --git a/target/linux/ath79/image/tiny-tp-link.mk b/target/linux/ath79/image/tiny-tp-link.mk
index dc91a74ae1..d0bb119923 100644
--- a/target/linux/ath79/image/tiny-tp-link.mk
+++ b/target/linux/ath79/image/tiny-tp-link.mk
@@ -279,9 +279,9 @@  define Device/tplink_tl-wr841-v11
   DEVICE_VARIANT := v11
   TPLINK_HWID := 0x08410011
   SUPPORTED_DEVICES += tl-wr841n-v11
-  IMAGES += factory-us.bin factory-eu.bin
-  IMAGE/factory-us.bin := tplink-v1-image factory -C US
-  IMAGE/factory-eu.bin := tplink-v1-image factory -C EU
+  IMAGES += factory-tftp-us.bin factory-tftp-eu.bin
+  IMAGE/factory-tftp-us.bin := tplink-v1-image factory -C US
+  IMAGE/factory-tftp-eu.bin := tplink-v1-image factory -C EU
 endef
 TARGET_DEVICES += tplink_tl-wr841-v11
 
@@ -292,9 +292,9 @@  define Device/tplink_tl-wr841-v12
   DEVICE_VARIANT := v12
   TPLINK_HWID := 0x08410012
   SUPPORTED_DEVICES += tl-wr841n-v11
-  IMAGES += factory-us.bin factory-eu.bin
-  IMAGE/factory-us.bin := tplink-v1-image factory -C US
-  IMAGE/factory-eu.bin := tplink-v1-image factory -C EU
+  IMAGES += factory-tftp-us.bin factory-tftp-eu.bin
+  IMAGE/factory-tftp-us.bin := tplink-v1-image factory -C US
+  IMAGE/factory-tftp-eu.bin := tplink-v1-image factory -C EU
 endef
 TARGET_DEVICES += tplink_tl-wr841-v12
 
@@ -315,10 +315,10 @@  define Device/tplink_tl-wr940n-v4
   DEVICE_VARIANT := v4
   TPLINK_HWID := 0x09400004
   SUPPORTED_DEVICES += tl-wr940n-v4
-  IMAGES += factory-us.bin factory-eu.bin factory-br.bin
-  IMAGE/factory-us.bin := tplink-v1-image factory -C US
-  IMAGE/factory-eu.bin := tplink-v1-image factory -C EU
-  IMAGE/factory-br.bin := tplink-v1-image factory -C BR
+  IMAGES += factory-tftp-us.bin factory-tftp-eu.bin factory-tftp-br.bin
+  IMAGE/factory-tftp-us.bin := tplink-v1-image factory -C US
+  IMAGE/factory-tftp-eu.bin := tplink-v1-image factory -C EU
+  IMAGE/factory-tftp-br.bin := tplink-v1-image factory -C BR
 endef
 TARGET_DEVICES += tplink_tl-wr940n-v4
 
@@ -329,10 +329,10 @@  define Device/tplink_tl-wr940n-v6
   DEVICE_VARIANT := v6
   TPLINK_HWID := 0x09400006
   SUPPORTED_DEVICES += tl-wr940n-v6
-  IMAGES += factory-us.bin factory-eu.bin factory-br.bin
-  IMAGE/factory-us.bin := tplink-v1-image factory -C US
-  IMAGE/factory-eu.bin := tplink-v1-image factory -C EU
-  IMAGE/factory-br.bin := tplink-v1-image factory -C BR
+  IMAGES += factory-tftp-us.bin factory-tftp-eu.bin factory-tftp-br.bin
+  IMAGE/factory-tftp-us.bin := tplink-v1-image factory -C US
+  IMAGE/factory-tftp-eu.bin := tplink-v1-image factory -C EU
+  IMAGE/factory-tftp-br.bin := tplink-v1-image factory -C BR
 endef
 TARGET_DEVICES += tplink_tl-wr940n-v6