diff mbox series

[LEDE-DEV] bcm53xx: use iflag=skip_bytes for dd command during sysupgrade

Message ID 20171223221425.21080-1-zajec5@gmail.com
State Accepted
Delegated to: Rafał Miłecki
Headers show
Series [LEDE-DEV] bcm53xx: use iflag=skip_bytes for dd command during sysupgrade | expand

Commit Message

Rafał Miłecki Dec. 23, 2017, 10:14 p.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

Since BusyBox 1.25.0 dd command supports iflag=skip_bytes which allows
skipping requested amount of bytes without reducing blocksize. Thanks to
this we can leave default blocksize and let dd work more efficiently.

On Netgear R6250 "dd skip=58 iflag=skip_bytes" can be 5 times faster
than "dd bs=58 skip=1" when extracting TRX out of CHK.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 target/linux/bcm53xx/base-files/lib/upgrade/platform.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Etienne Champetier Dec. 23, 2017, 11:26 p.m. UTC | #1 
Hi Rafal,

2017-12-23 14:14 GMT-08:00 Rafał Miłecki <zajec5@gmail.com>:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Since BusyBox 1.25.0 dd command supports iflag=skip_bytes which allows
> skipping requested amount of bytes without reducing blocksize. Thanks to
> this we can leave default blocksize and let dd work more efficiently.
>
> On Netgear R6250 "dd skip=58 iflag=skip_bytes" can be 5 times faster
> than "dd bs=58 skip=1" when extracting TRX out of CHK.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  target/linux/bcm53xx/base-files/lib/upgrade/platform.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
> index 1a33e3a447..06451f17fd 100644
> --- a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
> @@ -274,11 +274,11 @@ platform_pre_upgrade() {
>  platform_trx_from_chk_cmd() {
>         local header_len=$((0x$(get_magic_long_at "$1" 4)))
>
> -       echo -n dd bs=$header_len skip=1
> +       echo -n dd skip=$header_len iflag=skip_bytes
>  }
>
>  platform_trx_from_cybertan_cmd() {
> -       echo -n dd bs=32 skip=1
> +       echo -n dd skip=32 iflag=skip_bytes

haven't checked, but would "tail -c+32" work (or maybe "tail -c+33") works ?
I know I add a similar old dd version to handle in the past (but
nothing to do with OpenWRT) and "tail" was enough in the end

Regards
Etienne


>  }
>
>  platform_img_from_safeloader() {
> --
> 2.11.0
>
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
Rafał Miłecki Dec. 25, 2017, 1:18 p.m. UTC | #2 
On 2017-12-24 00:26, Etienne Champetier wrote:
> 2017-12-23 14:14 GMT-08:00 Rafał Miłecki <zajec5@gmail.com>:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> Since BusyBox 1.25.0 dd command supports iflag=skip_bytes which allows
>> skipping requested amount of bytes without reducing blocksize. Thanks 
>> to
>> this we can leave default blocksize and let dd work more efficiently.
>> 
>> On Netgear R6250 "dd skip=58 iflag=skip_bytes" can be 5 times faster
>> than "dd bs=58 skip=1" when extracting TRX out of CHK.
>> 
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  target/linux/bcm53xx/base-files/lib/upgrade/platform.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh 
>> b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>> index 1a33e3a447..06451f17fd 100644
>> --- a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>> +++ b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>> @@ -274,11 +274,11 @@ platform_pre_upgrade() {
>>  platform_trx_from_chk_cmd() {
>>         local header_len=$((0x$(get_magic_long_at "$1" 4)))
>> 
>> -       echo -n dd bs=$header_len skip=1
>> +       echo -n dd skip=$header_len iflag=skip_bytes
>>  }
>> 
>>  platform_trx_from_cybertan_cmd() {
>> -       echo -n dd bs=32 skip=1
>> +       echo -n dd skip=32 iflag=skip_bytes
> 
> haven't checked, but would "tail -c+32" work (or maybe "tail -c+33") 
> works ?
> I know I add a similar old dd version to handle in the past (but
> nothing to do with OpenWRT) and "tail" was enough in the end

I didn't test it, but I believe we could use tail somehow for that.

The question is: why should we?

We use "dd" in general for a lot of sysupgrade-related operations. It
has more fearures like controlling a block size which we may want to use
in a future for more optimizations. It can be used to "cut" a part of
file from the middle of it.

I don't see any real advantage of switching to tail.
Etienne Champetier Dec. 25, 2017, 6:57 p.m. UTC | #3 
Hi Rafal,

2017-12-25 5:18 GMT-08:00 Rafał Miłecki <rafal@milecki.pl>:
> On 2017-12-24 00:26, Etienne Champetier wrote:
>>
>> 2017-12-23 14:14 GMT-08:00 Rafał Miłecki <zajec5@gmail.com>:
>>>
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Since BusyBox 1.25.0 dd command supports iflag=skip_bytes which allows
>>> skipping requested amount of bytes without reducing blocksize. Thanks to
>>> this we can leave default blocksize and let dd work more efficiently.
>>>
>>> On Netgear R6250 "dd skip=58 iflag=skip_bytes" can be 5 times faster
>>> than "dd bs=58 skip=1" when extracting TRX out of CHK.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>  target/linux/bcm53xx/base-files/lib/upgrade/platform.sh | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>>> b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>>> index 1a33e3a447..06451f17fd 100644
>>> --- a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>>> +++ b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
>>> @@ -274,11 +274,11 @@ platform_pre_upgrade() {
>>>  platform_trx_from_chk_cmd() {
>>>         local header_len=$((0x$(get_magic_long_at "$1" 4)))
>>>
>>> -       echo -n dd bs=$header_len skip=1
>>> +       echo -n dd skip=$header_len iflag=skip_bytes
>>>  }
>>>
>>>  platform_trx_from_cybertan_cmd() {
>>> -       echo -n dd bs=32 skip=1
>>> +       echo -n dd skip=32 iflag=skip_bytes
>>
>>
>> haven't checked, but would "tail -c+32" work (or maybe "tail -c+33") works
>> ?
>> I know I add a similar old dd version to handle in the past (but
>> nothing to do with OpenWRT) and "tail" was enough in the end
>
>
> I didn't test it, but I believe we could use tail somehow for that.
>
> The question is: why should we?
>
> We use "dd" in general for a lot of sysupgrade-related operations. It
> has more fearures like controlling a block size which we may want to use
> in a future for more optimizations. It can be used to "cut" a part of
> file from the middle of it.
>
> I don't see any real advantage of switching to tail.

Just checked and lede 17.01 also has busybox 1.25+, so no need for tail indeed
diff mbox series

Patch

diff --git a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
index 1a33e3a447..06451f17fd 100644
--- a/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
+++ b/target/linux/bcm53xx/base-files/lib/upgrade/platform.sh
@@ -274,11 +274,11 @@  platform_pre_upgrade() {
 platform_trx_from_chk_cmd() {
 	local header_len=$((0x$(get_magic_long_at "$1" 4)))
 
-	echo -n dd bs=$header_len skip=1
+	echo -n dd skip=$header_len iflag=skip_bytes
 }
 
 platform_trx_from_cybertan_cmd() {
-	echo -n dd bs=32 skip=1
+	echo -n dd skip=32 iflag=skip_bytes
 }
 
 platform_img_from_safeloader() {