[mynewt-newtmgr] branch master updated: Fix not working MTU exchange on newer versions of mac

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

janc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-newtmgr.git


The following commit(s) were added to refs/heads/master by this push:
     new 5fa2e0f  Fix not working MTU exchange on newer versions of mac
5fa2e0f is described below

commit 5fa2e0fbd6ab3e19c3cc8f431939dc62ee7d11d9
Author: Michal Gorecki <mi...@codecoup.pl>
AuthorDate: Fri Dec 9 14:18:40 2022 +0100

    Fix not working MTU exchange on newer versions of mac
    
    It seems like on newer versions of mac (e.g. Monterey)
    fix with looping three times to get right MTU
    doesn't work. Also for some strange reason default TxMTU
    is set to 17 instead of 23. Now user can set MTU from
    the command line (-m <MTU value>).
    If MTU is not set in the command line and after performing
    three loops MTU is still <23 we hardcode it to 185.
---
 newtmgr/bll/bll_common.go | 30 +++++++++++++++++++++++++-----
 newtmgr/cli/commands.go   |  6 ++++++
 newtmgr/nmutil/nmutil.go  |  1 +
 nmxact/xact/image.go      |  5 +++++
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/newtmgr/bll/bll_common.go b/newtmgr/bll/bll_common.go
index 02be87d..e26bc60 100644
--- a/newtmgr/bll/bll_common.go
+++ b/newtmgr/bll/bll_common.go
@@ -22,6 +22,8 @@
 package bll
 
 import (
+	"fmt"
+	"mynewt.apache.org/newtmgr/newtmgr/nmutil"
 	"runtime"
 	"time"
 
@@ -38,8 +40,8 @@ func exchangeMtu(cln ble.Client, preferredMtu uint16) (uint16, error) {
 	// macOS, the request to exchange MTU doesn't actually do anything.  The
 	// BLE library relies on the assumption that the OS already exchanged MTUs
 	// on its own.  If this assumption is incorrect, the number that was
-	// returned is the default out of date value (23).  In this case, sleep and
-	// retry.
+	// returned is the default out of date value (23 or for some strange reason,
+	// on some versions of mac 17). In this case, sleep and retry.
 	var mtu int
 	for i := 0; i < 3; i++ {
 		var err error
@@ -53,18 +55,36 @@ func exchangeMtu(cln ble.Client, preferredMtu uint16) (uint16, error) {
 			break
 		}
 
-		// If macOS returned a value other than 23, then MTU exchange has
+		// If macOS returned a value higher than 23, then MTU exchange has
 		// completed.
-		if mtu != bledefs.BLE_ATT_MTU_DFLT {
+		if mtu > bledefs.BLE_ATT_MTU_DFLT {
 			break
 		}
 
 		// Otherwise, give the OS some time to perform the exchange.
-		log.Debugf("macOS reports an MTU of 23.  " +
+		log.Debugf("macOS reports an MTU of <=23.  " +
 			"Assume exchange hasn't completed; wait and requery.")
 		time.Sleep(time.Second)
 	}
 
+	// On some versions of mac (e.g. monterey) workaround with
+	// looping three times seems to not work anymore. This allows to use
+	// "-m <mtu value>" flag to set MTU from command line.
+	if nmutil.MtuOverride != 0 {
+		if nmutil.MtuOverride < 23 {
+			return 0, fmt.Errorf("MTU should be at least 23")
+		} else {
+			mtu = nmutil.MtuOverride
+		}
+	}
+
+	// If no MTU value was specified in the command line
+	// and performing three loops didn't help in performing exchange, MTU is still invalid (< 23).
+	// In this case we hardcode it to 185.
+	if mtu < bledefs.BLE_ATT_MTU_DFLT {
+		mtu = 185
+	}
+
 	log.Debugf("Exchanged MTU; ATT MTU = %d", mtu)
 	return uint16(mtu), nil
 }
diff --git a/newtmgr/cli/commands.go b/newtmgr/cli/commands.go
index 1f37c0d..eee832b 100644
--- a/newtmgr/cli/commands.go
+++ b/newtmgr/cli/commands.go
@@ -21,6 +21,7 @@ package cli
 
 import (
 	"fmt"
+	"runtime"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
@@ -59,6 +60,11 @@ func Commands() *cobra.Command {
 		},
 	}
 
+	if runtime.GOOS == "darwin" {
+		nmCmd.PersistentFlags().IntVarP(&nmutil.MtuOverride, "mtu-ovrd", "m", 0,
+			"Override MTU in case it can't be negotiated on Mac computers (slice out of range error)")
+	}
+
 	nmCmd.PersistentFlags().StringVarP(&nmutil.ConnProfile, "conn", "c", "",
 		"connection profile to use")
 
diff --git a/newtmgr/nmutil/nmutil.go b/newtmgr/nmutil/nmutil.go
index 908d7b0..5a13e1c 100644
--- a/newtmgr/nmutil/nmutil.go
+++ b/newtmgr/nmutil/nmutil.go
@@ -40,6 +40,7 @@ var Tries int
 var ConnProfile string
 var DeviceName string
 var BleWriteRsp bool
+var MtuOverride int
 var ConnType string
 var ConnString string
 var ConnExtra string
diff --git a/nmxact/xact/image.go b/nmxact/xact/image.go
index f2acfe3..26f7dcf 100644
--- a/nmxact/xact/image.go
+++ b/nmxact/xact/image.go
@@ -148,6 +148,11 @@ func findChunkLen(s sesn.Sesn, hash []byte, upgrade bool, data []byte,
 		// Encoded length is larger than MTU, we need to make chunk shorter
 		overflow := len(enc) - s.MtuOut()
 		chunklen -= overflow
+		if chunklen <= 0 {
+			return 0, fmt.Errorf("Cannot create image upload request; "+
+				"MTU too low to fit any image data; max-payload-size=%d chunklen %d",
+				s.MtuOut(), chunklen)
+		}
 	}
 
 	return chunklen, nil