You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/05/06 13:08:22 UTC

[GitHub] [mynewt-newtmgr] sw opened a new issue #185: Flash alignment mismatch leads to large overhead with Zephyr mcumgr

sw opened a new issue #185:
URL: https://github.com/apache/mynewt-newtmgr/issues/185


   We are using Nordic's fork of mcumgr (https://github.com/nrfconnect/sdk-mcumgr) but I believe this also affects upstream https://github.com/apache/mynewt-mcumgr.
   
   The Zephyr port rounds the write size down to the flash write block size:
   https://github.com/apache/mynewt-mcumgr/blob/798f7fe28bb32335ba25d1c180808cd88d73a83c/cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c#L571-L576
   
   This smaller size is then reported back in the response. However, the mcumgr takes some time to detect this mismatch, and keeps sending frames with incorrect offsets.
   
   This leads to a large overhead of data transmitted on the serial port.
   
   This happens with the first frame, which is sent with 0x127 bytes of data; the receiver rounds this down to 0x124.
   mcumgr CLI then tries to continue at offset 0x127, then offset 0x27b, until it realises something is wrong and restarts at 0x124.
   
   The next 64kbyte then go through without a hitch (no offset problems).
   
   At the 64k block border though, the problem re-occurs, and from then on every fragment is essentially sent 4 times: 3 times with a wrong offset which is discarded.
   
   The image handling in newtmgr should be improved:
   * to detect a wrong offset sooner, rather than sending up to 3 frames with wrong offset
   * to only send a multiple of a common flash block size, for example a multiple of 4 bytes.
   
   Attached is a log file from `mcumgr image upload file.bin -l debug`:
   [log.txt](https://github.com/apache/mynewt-newtmgr/files/6434672/log.txt)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-newtmgr] LockeKK edited a comment on issue #185: Flash alignment mismatch leads to large overhead with Zephyr mcumgr

Posted by GitBox <gi...@apache.org>.
LockeKK edited a comment on issue #185:
URL: https://github.com/apache/mynewt-newtmgr/issues/185#issuecomment-894907940


   I found the exact the same issue. My solution is to round the chunk length as 4-byte-aligned, then bootloader won't wait for valid offset check. It squeezed the time from minutes to 17 sec for 340KB binary.
   [diff.patch.txt](https://github.com/apache/mynewt-newtmgr/files/6951861/diff.patch.txt)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-newtmgr] LockeKK commented on issue #185: Flash alignment mismatch leads to large overhead with Zephyr mcumgr

Posted by GitBox <gi...@apache.org>.
LockeKK commented on issue #185:
URL: https://github.com/apache/mynewt-newtmgr/issues/185#issuecomment-894907940


   I found the exact issue. My solution is like this. It squeezed the time from minutes to 17 sec for 340KB binary.
   
   diff --git a/nmxact/nmserial/serial_xport.go b/nmxact/nmserial/serial_xport.go
   index 1c97fc0..164d496 100644
   --- a/nmxact/nmserial/serial_xport.go
   +++ b/nmxact/nmserial/serial_xport.go
   @@ -287,7 +287,7 @@ func (sx *SerialXport) Tx(bytes []byte) error {
                    * carriage return (and possibly LF 2 bytes), */
    
                   /* all totaled, 124 bytes should work */
   -               writeLen := util.Min(124, totlen-written)
   +               writeLen := util.Min(1024, totlen-written)
    
                   writeBytes := base64Data[written : written+writeLen]
                   sx.txRaw(writeBytes)
   diff --git a/nmxact/xact/image.go b/nmxact/xact/image.go
   index b3b332c..95ee2d7 100644
   --- a/nmxact/xact/image.go
   +++ b/nmxact/xact/image.go
   @@ -46,6 +46,7 @@ const IMAGE_UPLOAD_START_WS = 1
    const IMAGE_UPLOAD_DEF_MAX_WS = 5
    const IMAGE_UPLOAD_STATUS_EXPECTED = 0
    const IMAGE_UPLOAD_STATUS_RQ = 1
   +const IMAGE_UPLOAD_CHUNK_ALIGNMENT = 4
    
    type ImageUploadProgressFn func(c *ImageUploadCmd, r *nmp.ImageUploadRsp)
    type ImageUploadCmd struct {
   @@ -150,6 +151,14 @@ func findChunkLen(s sesn.Sesn, hash []byte, upgrade bool, data []byte,
                   chunklen -= overflow
           }
    
   +       //Ensure the chunk length is always aligned as bootloader expected
   +       if chunklen > IMAGE_UPLOAD_CHUNK_ALIGNMENT {
   +               remain_bytes := chunklen % IMAGE_UPLOAD_CHUNK_ALIGNMENT
   +               if remain_bytes != 0 {
   +                       chunklen -= remain_bytes
   +               }
   +       }
   +
           return chunklen, nil
    }
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-newtmgr] sw commented on issue #185: Flash alignment mismatch leads to large overhead with Zephyr mcumgr

Posted by GitBox <gi...@apache.org>.
sw commented on issue #185:
URL: https://github.com/apache/mynewt-newtmgr/issues/185#issuecomment-920868644


   Using
   ```go
   writeLen := util.Min(512, totlen-written)
   ```
   seems to be optimal, as the MTU size is also limited elsewhere. The value 1024 suggested by @LockeKK will therefore not result in larger frames.
   
   This requires setting `SMP_SHELL_RX_BUF_SIZE` to 512 on Zephyr.
   
   This also fixes the issue with the 20ms delay as reported in #180, as that `Sleep` call no longer happens.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-newtmgr] sw commented on issue #185: Flash alignment mismatch leads to large overhead with Zephyr mcumgr

Posted by GitBox <gi...@apache.org>.
sw commented on issue #185:
URL: https://github.com/apache/mynewt-newtmgr/issues/185#issuecomment-834220928


   This seems to have gotten worse in 1.9.0 compared to 1.8.0


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-newtmgr] dachalco commented on issue #185: Flash alignment mismatch leads to large overhead with Zephyr mcumgr

Posted by GitBox <gi...@apache.org>.
dachalco commented on issue #185:
URL: https://github.com/apache/mynewt-newtmgr/issues/185#issuecomment-897931092


   +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org