[U-Boot] [PATCH v3] dfu: Introduction of the "dfu_hash_algo" env variable for checksum method setting

[Tom Rini]

On Fri, May 09, 2014 at 10:31:54AM +0200, Wolfgang Denk wrote:
> Dear Lukasz,
> 
> In message <20140509085203.31133238 at amdc2363> you wrote:
> > 
> > For automated tests I use MD5 and compare this value before sending
> > data to target via DFU and after I read it. This testing is done purely
> > on HOST machine.
> 
> This is unsufficient.  You should always verify the image on the
> target after the download has completed.

True.  But this patch doesn't really change what you would have to do,
and arguably make it easier.

> > Participants have agreed, that we shall optionally enable crc32 (or
> > other algorithm) calculation. 
> 
> If this is the default now, it should remain the default.

Keep in mind what this current default is.  We say "here was the CRC32".
We do not compare it with an expected value nor do we have the ability
to since we're not passed from the host what the value was.

> > 2. The current crc32 implementation is painfully slow (although I have
> > only L1 enabled on my target). 
> 
> This is an unrelated problem then, which should excluded from this
> discussion here.

Agreed.

> > 3. With large files (like rootfs images) we store data (to medium) with
> > 32 MiB chunks, which means that when we calculate complete crc32 the
> > image is already written to its final destination.
> 
> You can still detect if the download was corrupted, report a proper
> error and initiate a re-download.  This would at least give you a
> chance to react to corrupted data.  Just closing the eyes and hoping
> no errors will ever happen has always been a bad strategy.

Before and after this change, only if the console is being monitored by
some script.  We do not nor are we given an expected hash so we cannot
say data was corrupted.

> > 4. This patch also allows some flexibility: by setting the env variable
> > we can decide which algorithm to use (crc32, sha1, etc). It is
> > appealing since we use the hash_* code anyway.
> 
> Agreed.  This was not my point.
> 
> What I complained about is the change in behaviour.  I asked to make
> the existing behaviour the default, so unaware users will not be
> affected. Only if you intentionally want some other behaviour you can
> then enable this by setting the env variable.

Well, looking at mainline usage of DFU, Lukasz is speaking for about
half of the users / implementors.  Since Denx is working with the other
half, can you shed some light on actual use vs theoretical
possibilities?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140512/e1ebdeff/attachment.pgp>