[BlueZ] mesh: Update the behavior of --io option

Commit Message

Inga Stotland May 5, 2023, 7:39 p.m. UTC

This aligns the behavior of command line option --io to
add new "auto" value and modify the behavior of "generic"
value:

*auto* - Use first available controller: via MGMT interface
if kernel supports it, otherwise, via raw HCI socket (i.e.,
default to *generic*).

*generic:[hci]<index>* - Use generic HCI io on interface hci<index>

The default value is now *auto*, whereas *generic* is used
only if the specific HCI controller is explicitly specified.
---
 mesh/bluetooth-meshd.rst.in | 11 ++++++----
 mesh/main.c                 | 40 +++++++++++--------------------------
 mesh/mesh-io-generic.c      |  3 +++
 mesh/mesh-io.c              | 17 +++++++++++-----
 mesh/mesh-mgmt.c            |  5 +++++
 mesh/mesh-mgmt.h            |  1 +
 6 files changed, 40 insertions(+), 37 deletions(-)

Comments

[e-mail] May 5, 2023, 9:19 p.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=745379

---Test result---

Test Summary:
CheckPatch                    PASS      0.51 seconds
GitLint                       PASS      0.31 seconds
BuildEll                      PASS      31.27 seconds
BluezMake                     PASS      953.63 seconds
MakeCheck                     PASS      12.38 seconds
MakeDistcheck                 PASS      179.73 seconds
CheckValgrind                 PASS      291.13 seconds
CheckSmatch                   PASS      381.59 seconds
bluezmakeextell               PASS      113.70 seconds
IncrementalBuild              PASS      758.30 seconds
ScanBuild                     WARNING   1182.91 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
mesh/main.c:161:3: warning: Value stored to 'optarg' is never read
                optarg += strlen("auto");
                ^         ~~~~~~~~~~~~~~
1 warning generated.



---
Regards,
Linux Bluetooth

[e-mail] May 12, 2023, 6:30 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Brian Gix <[e-mail]>:

On Fri,  5 May 2023 12:39:31 -0700 you wrote:
> This aligns the behavior of command line option --io to
> add new "auto" value and modify the behavior of "generic"
> value:
> 
> *auto* - Use first available controller: via MGMT interface
> if kernel supports it, otherwise, via raw HCI socket (i.e.,
> default to *generic*).
> 
> [...]

Here is the summary with links:
  - [BlueZ] mesh: Update the behavior of --io option
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=93d0d8b2fc69

You are awesome, thank you!

Patch

diff --git a/mesh/bluetooth-meshd.rst.in b/mesh/bluetooth-meshd.rst.in
index 06cdb69da..761536711 100644
--- a/mesh/bluetooth-meshd.rst.in
+++ b/mesh/bluetooth-meshd.rst.in
@@ -36,14 +36,17 @@  OPTIONS
 -i <type>, --io <type>
     Specifies I/O interface type:
 
-    *hci<index>* - Use generic HCI io on interface hci<index>,
-    or, if no idex is specified, the first available one.
+    *auto* - Use first available controller: via MGMT interface
+    if kernel supports it, otherwise, via raw HCI socket.
+
+    *generic:[hci]<index>* - Use generic HCI io on interface
+    hci<index>.
 
     *unit:<fd_path>*- Specifies open file descriptor for
     daemon testing.
 
-    By default, if no type is specified, uses generic I/O
-    on the first available HCI interface.
+    By default, if no type is specified, uses auto I/O
+    on the first available controller.
 
 -c <file>, --config <file>
     Specifies an explicit config file path instead of relying on the
diff --git a/mesh/main.c b/mesh/main.c
index 3bca020a0..145bcfa98 100644
--- a/mesh/main.c
+++ b/mesh/main.c
@@ -48,6 +48,12 @@  static const struct option main_options[] = {
 	{ }
 };
 
+static const char *io_usage =
+	       "\t(auto | generic:[hci]<index> | unit:<fd_path>)\n"
+	       "\t\tauto - Use first available controller (MGMT or raw HCI)\n"
+	       "\t\tgeneric - Use raw HCI io on interface hci<index>\n"
+	       "\t\tunit - Use test IO (for automatic testing only)\n";
+
 static void usage(void)
 {
 	fprintf(stderr,
@@ -55,18 +61,14 @@  static void usage(void)
 	       "\tbluetooth-meshd [options]\n");
 	fprintf(stderr,
 		"Options:\n"
-	       "\t--io <io>         Use specified io (default: generic)\n"
+	       "\t--io <io>         Use specified io (default: auto)\n"
 	       "\t--config          Daemon configuration directory\n"
 	       "\t--storage         Mesh node(s) configuration directory\n"
 	       "\t--nodetach        Run in foreground\n"
 	       "\t--debug           Enable debug output\n"
 	       "\t--dbus-debug      Enable D-Bus debugging\n"
 	       "\t--help            Show %s information\n", __func__);
-	fprintf(stderr,
-	       "io:\n"
-	       "\t([hci]<index> | generic[:[hci]<index>] | unit:<fd_path>)\n"
-	       "\t\tUse generic HCI io on interface hci<index>, or the first\n"
-	       "\t\tavailable one\n");
+	fprintf(stderr, "\n\t io: %s", io_usage);
 }
 
 static void do_debug(const char *str, void *user_data)
@@ -157,21 +159,8 @@  static bool parse_io(const char *optarg, enum mesh_io_type *type, void **opts)
 		*opts = index;
 
 		optarg += strlen("auto");
-		if (!*optarg) {
-			*index = MGMT_INDEX_NONE;
-			return true;
-		}
-
-		if (*optarg != ':')
-			return false;
-
-		optarg++;
-
-		if (sscanf(optarg, "hci%d", index) == 1)
-			return true;
-
-		if (sscanf(optarg, "%d", index) == 1)
-			return true;
+		*index = MGMT_INDEX_NONE;
+		return true;
 
 		return false;
 	} else if (strstr(optarg, "generic") == optarg) {
@@ -181,12 +170,7 @@  static bool parse_io(const char *optarg, enum mesh_io_type *type, void **opts)
 		*opts = index;
 
 		optarg += strlen("generic");
-		if (!*optarg) {
-			*index = MGMT_INDEX_NONE;
-			return true;
-		}
-
-		if (*optarg != ':')
+		if (!*optarg || *optarg != ':')
 			return false;
 
 		optarg++;
@@ -291,7 +275,7 @@  int main(int argc, char *argv[])
 		io = l_strdup_printf("auto");
 
 	if (!parse_io(io, &io_type, &io_opts)) {
-		l_error("Invalid io: %s", io);
+		l_error("Invalid io: %s\n%s", io, io_usage);
 		status = EXIT_FAILURE;
 		goto done;
 	}
diff --git a/mesh/mesh-io-generic.c b/mesh/mesh-io-generic.c
index 93a56275b..00932ade7 100644
--- a/mesh/mesh-io-generic.c
+++ b/mesh/mesh-io-generic.c
@@ -380,6 +380,9 @@  static void hci_init(void *user_data)
 	if (io->pvt->hci)
 		bt_hci_unref(io->pvt->hci);
 
+	/* Clear controller HCI list to suppress mgmt interface warnings */
+	mesh_mgmt_clear();
+
 	io->pvt->hci = bt_hci_new_user_channel(io->index);
 	if (!io->pvt->hci) {
 		l_error("Failed to start mesh io (hci %u): %s", io->index,
diff --git a/mesh/mesh-io.c b/mesh/mesh-io.c
index 48e3f4226..b953bf4cf 100644
--- a/mesh/mesh-io.c
+++ b/mesh/mesh-io.c
@@ -72,12 +72,20 @@  static void ctl_alert(int index, bool up, bool pwr, bool mesh, void *user_data)
 	enum mesh_io_type type = L_PTR_TO_UINT(user_data);
 	const struct mesh_io_api *api = NULL;
 
-	l_warn("up:%d pwr: %d mesh: %d", up, pwr, mesh);
+	l_warn("index %u up:%d pwr: %d mesh: %d", index, up, pwr, mesh);
 
 	/* If specific IO controller requested, honor it */
-	if (default_io->favored_index != MGMT_INDEX_NONE &&
-					default_io->favored_index != index)
-		return;
+	if (default_io->favored_index != MGMT_INDEX_NONE) {
+		if (default_io->favored_index != index)
+			return;
+
+		if (!up | pwr) {
+			l_warn("HCI%u failed to start generic IO %s",
+				index, pwr ? ": already powered on" : "");
+			if (default_io->ready)
+				default_io->ready(default_io->user_data, false);
+		}
+	}
 
 	if (!up && default_io->index == index) {
 		/* Our controller has disappeared */
@@ -104,7 +112,6 @@  static void ctl_alert(int index, bool up, bool pwr, bool mesh, void *user_data)
 		default_io->index = index;
 		default_io->api = api;
 		api->init(default_io, &index, default_io->user_data);
-
 		l_queue_foreach(default_io->rx_regs, refresh_rx, default_io);
 	}
 }
diff --git a/mesh/mesh-mgmt.c b/mesh/mesh-mgmt.c
index d37aeb5ac..fd21a168a 100644
--- a/mesh/mesh-mgmt.c
+++ b/mesh/mesh-mgmt.c
@@ -271,3 +271,8 @@  bool mesh_mgmt_unregister(unsigned int id)
 {
 	return mgmt_unregister(mgmt_mesh, id);
 }
+
+void mesh_mgmt_clear(void)
+{
+	l_queue_clear(ctl_list, l_free);
+}
diff --git a/mesh/mesh-mgmt.h b/mesh/mesh-mgmt.h
index a3cd72faf..570282297 100644
--- a/mesh/mesh-mgmt.h
+++ b/mesh/mesh-mgmt.h
@@ -22,3 +22,4 @@  unsigned int mesh_mgmt_register(uint16_t event, uint16_t index,
 				void *user_data, mgmt_destroy_func_t destroy);
 bool mesh_mgmt_unregister(unsigned int id);
 void mesh_mgmt_destroy(void);
+void mesh_mgmt_clear(void);