public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usbip: tools: Add usbip host driver availability check
@ 2026-03-03  8:17 Zongmin Zhou
  2026-03-10 22:28 ` Shuah Khan
  2026-03-11 12:13 ` [PATCH] " Greg KH
  0 siblings, 2 replies; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-03  8:17 UTC (permalink / raw)
  To: valentina.manea.m, shuah, i; +Cc: linux-usb, linux-kernel, Zongmin Zhou

From: Zongmin Zhou <zhouzongmin@kylinos.cn>

Currently, usbip_generic_driver_open() doesn't verify that the required
kernel module (usbip-host or usbip-vudc) is actually loaded.
The function returns success even when no driver is present,
leading to usbipd daemon run success without driver loaded.

So add a check function to ensure usbip host driver has been loaded.

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
 tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
 tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
 tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
 tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
 4 files changed, 37 insertions(+)

diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
index 927a151fa9aa..6da000fab26d 100644
--- a/tools/usb/usbip/libsrc/usbip_device_driver.c
+++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
@@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
 struct usbip_host_driver device_driver = {
 	.edev_list = LIST_HEAD_INIT(device_driver.edev_list),
 	.udev_subsystem = "udc",
+	.bus_type = "platform",
+	.drv_name = USBIP_DEVICE_DRV_NAME,
 	.ops = {
 		.open = usbip_device_driver_open,
 		.close = usbip_generic_driver_close,
diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
index ca78aa368476..900370095b61 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.c
+++ b/tools/usb/usbip/libsrc/usbip_host_common.c
@@ -164,6 +164,31 @@ static void usbip_exported_device_destroy(struct list_head *devs)
 	}
 }
 
+/* Check if the usbip host driver is available in sysfs */
+static int check_driver_available(struct usbip_host_driver *hdriver)
+{
+	char driver_path[SYSFS_PATH_MAX];
+	struct stat st;
+	int ret;
+
+	if (!hdriver->drv_name || !hdriver->bus_type)
+		return 0;
+
+	//Check if the usbip-host or usbip-vudc driver directory exists in sysfs.
+	snprintf(driver_path, sizeof(driver_path), "%s/%s/%s/%s/%s",
+		SYSFS_MNT_PATH, SYSFS_BUS_NAME, hdriver->bus_type,
+		SYSFS_DRIVERS_NAME, hdriver->drv_name);
+
+	ret = stat(driver_path, &st);
+	if (ret == 0 && S_ISDIR(st.st_mode)) {
+		dbg("driver '%s' is available", hdriver->drv_name);
+		return 1;
+	}
+
+	return 0;
+}
+
+
 int usbip_generic_driver_open(struct usbip_host_driver *hdriver)
 {
 	int rc;
@@ -174,6 +199,12 @@ int usbip_generic_driver_open(struct usbip_host_driver *hdriver)
 		return -1;
 	}
 
+	//Check if the required driver is actually available.
+	if (!check_driver_available(hdriver)) {
+		err("please load '%s' kernel module", hdriver->drv_name);
+		goto err;
+	}
+
 	rc = refresh_exported_devices(hdriver);
 	if (rc < 0)
 		goto err;
diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h b/tools/usb/usbip/libsrc/usbip_host_common.h
index f46967c0aa18..cf9bd60846cf 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.h
+++ b/tools/usb/usbip/libsrc/usbip_host_common.h
@@ -40,6 +40,8 @@ struct usbip_host_driver {
 	/* list of exported device */
 	struct list_head edev_list;
 	const char *udev_subsystem;
+	const char *bus_type;
+	const char *drv_name;
 	struct usbip_host_driver_ops ops;
 };
 
diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
index 573e73ec36bd..e8f9d2ee2497 100644
--- a/tools/usb/usbip/libsrc/usbip_host_driver.c
+++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
@@ -41,6 +41,8 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
 struct usbip_host_driver host_driver = {
 	.edev_list = LIST_HEAD_INIT(host_driver.edev_list),
 	.udev_subsystem = "usb",
+	.bus_type = "usb",
+	.drv_name = USBIP_HOST_DRV_NAME,
 	.ops = {
 		.open = usbip_host_driver_open,
 		.close = usbip_generic_driver_close,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] usbip: tools: Add usbip host driver availability check
  2026-03-03  8:17 [PATCH] usbip: tools: Add usbip host driver availability check Zongmin Zhou
@ 2026-03-10 22:28 ` Shuah Khan
  2026-03-12  2:17   ` Zongmin Zhou
  2026-03-11 12:13 ` [PATCH] " Greg KH
  1 sibling, 1 reply; 15+ messages in thread
From: Shuah Khan @ 2026-03-10 22:28 UTC (permalink / raw)
  To: Zongmin Zhou, valentina.manea.m, shuah, i
  Cc: linux-usb, linux-kernel, Zongmin Zhou, Shuah Khan

On 3/3/26 01:17, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> Currently, usbip_generic_driver_open() doesn't verify that the required
> kernel module (usbip-host or usbip-vudc) is actually loaded.
> The function returns success even when no driver is present,
> leading to usbipd daemon run success without driver loaded.

Doesn't usbip_generic_driver_open() try to refresh exported
device list and fail? It returns error if it can't find fetch
them.

usbipd starts and the when usbip_host is loaded it works correctly.
Doesn't it?

> 
> So add a check function to ensure usbip host driver has been loaded.
> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
>   tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
>   tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
>   tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
>   tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
>   4 files changed, 37 insertions(+)
> 
> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
> index 927a151fa9aa..6da000fab26d 100644
> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
> @@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>   struct usbip_host_driver device_driver = {
>   	.edev_list = LIST_HEAD_INIT(device_driver.edev_list),
>   	.udev_subsystem = "udc",
> +	.bus_type = "platform",

Why are we adding this here - user-space shouldn't need to
know what kind of driver this is?

> +	.drv_name = USBIP_DEVICE_DRV_NAME,

And the name?

>   	.ops = {
>   		.open = usbip_device_driver_open,
>   		.close = usbip_generic_driver_close,
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
> index ca78aa368476..900370095b61 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
> @@ -164,6 +164,31 @@ static void usbip_exported_device_destroy(struct list_head *devs)
>   	}
>   }
>   
> +/* Check if the usbip host driver is available in sysfs */
> +static int check_driver_available(struct usbip_host_driver *hdriver)
> +{
> +	char driver_path[SYSFS_PATH_MAX];
> +	struct stat st;
> +	int ret;
> +
> +	if (!hdriver->drv_name || !hdriver->bus_type)
> +		return 0;
> +
> +	//Check if the usbip-host or usbip-vudc driver directory exists in sysfs.
> +	snprintf(driver_path, sizeof(driver_path), "%s/%s/%s/%s/%s",
> +		SYSFS_MNT_PATH, SYSFS_BUS_NAME, hdriver->bus_type,
> +		SYSFS_DRIVERS_NAME, hdriver->drv_name);
> +
> +	ret = stat(driver_path, &st);
> +	if (ret == 0 && S_ISDIR(st.st_mode)) {
> +		dbg("driver '%s' is available", hdriver->drv_name);
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +
>   int usbip_generic_driver_open(struct usbip_host_driver *hdriver)
>   {
>   	int rc;
> @@ -174,6 +199,12 @@ int usbip_generic_driver_open(struct usbip_host_driver *hdriver)
>   		return -1;
>   	}
>   
> +	//Check if the required driver is actually available.
> +	if (!check_driver_available(hdriver)) {
> +		err("please load '%s' kernel module", hdriver->drv_name);
> +		goto err;
> +	}
> +
>   	rc = refresh_exported_devices(hdriver);
>   	if (rc < 0)
>   		goto err;
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h b/tools/usb/usbip/libsrc/usbip_host_common.h
> index f46967c0aa18..cf9bd60846cf 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.h
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.h
> @@ -40,6 +40,8 @@ struct usbip_host_driver {
>   	/* list of exported device */
>   	struct list_head edev_list;
>   	const char *udev_subsystem;
> +	const char *bus_type;
> +	const char *drv_name;
>   	struct usbip_host_driver_ops ops;
>   };
>   
> diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
> index 573e73ec36bd..e8f9d2ee2497 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
> @@ -41,6 +41,8 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
>   struct usbip_host_driver host_driver = {
>   	.edev_list = LIST_HEAD_INIT(host_driver.edev_list),
>   	.udev_subsystem = "usb",
> +	.bus_type = "usb",
> +	.drv_name = USBIP_HOST_DRV_NAME,

>   	.ops = {
>   		.open = usbip_host_driver_open,
>   		.close = usbip_generic_driver_close,

I need more to justify these changes.

thanks,
-- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] usbip: tools: Add usbip host driver availability check
  2026-03-03  8:17 [PATCH] usbip: tools: Add usbip host driver availability check Zongmin Zhou
  2026-03-10 22:28 ` Shuah Khan
@ 2026-03-11 12:13 ` Greg KH
  2026-03-12  2:17   ` Zongmin Zhou
  1 sibling, 1 reply; 15+ messages in thread
From: Greg KH @ 2026-03-11 12:13 UTC (permalink / raw)
  To: Zongmin Zhou
  Cc: valentina.manea.m, shuah, i, linux-usb, linux-kernel,
	Zongmin Zhou

On Tue, Mar 03, 2026 at 04:17:20PM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> Currently, usbip_generic_driver_open() doesn't verify that the required
> kernel module (usbip-host or usbip-vudc) is actually loaded.
> The function returns success even when no driver is present,
> leading to usbipd daemon run success without driver loaded.
> 
> So add a check function to ensure usbip host driver has been loaded.
> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
>  tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
>  tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
>  tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
>  tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
>  4 files changed, 37 insertions(+)
> 
> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
> index 927a151fa9aa..6da000fab26d 100644
> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
> @@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>  struct usbip_host_driver device_driver = {
>  	.edev_list = LIST_HEAD_INIT(device_driver.edev_list),
>  	.udev_subsystem = "udc",
> +	.bus_type = "platform",

I think the "bus type" is changing in newer kernel versions, so be
careful about attempting to hard-code anything like this.  Devices can
move around between bus types just fine, that is not how they should
ever be referenced.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] usbip: tools: Add usbip host driver availability check
  2026-03-10 22:28 ` Shuah Khan
@ 2026-03-12  2:17   ` Zongmin Zhou
  2026-03-23 19:17     ` Shuah Khan
  0 siblings, 1 reply; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-12  2:17 UTC (permalink / raw)
  To: Shuah Khan, valentina.manea.m, shuah, i, Greg KH
  Cc: linux-usb, linux-kernel, Zongmin Zhou


On 2026/3/11 06:28, Shuah Khan wrote:
> On 3/3/26 01:17, Zongmin Zhou wrote:
>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>
>> Currently, usbip_generic_driver_open() doesn't verify that the required
>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>> The function returns success even when no driver is present,
>> leading to usbipd daemon run success without driver loaded.
>
> Doesn't usbip_generic_driver_open() try to refresh exported
> device list and fail? It returns error if it can't find fetch
> them.
>
> usbipd starts and the when usbip_host is loaded it works correctly.
> Doesn't it?
Actually, refresh_exported_devices() does not return an error
when the driver is not loaded,it consistently returns 0.
It only results in hdriver->ndevs being set to 0 if no exportable usbip 
devices are found.
Consequently, if the driver is missing, usbipd will start successfully 
in silence,
but subsequent usbip attach operations will fail.
The lack of explicit error messages makes it difficult for users to 
troubleshoot the root cause.
By adding a check to verify if the driver is loaded during the usbip 
daemon startup,
we can prevent these silent exceptions and ensure users are alerted to
the missing kernel module before they attempt to use the service.
>>
>> So add a check function to ensure usbip host driver has been loaded.
>>
>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>> ---
>>   tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
>>   tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
>>   tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
>>   tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
>>   4 files changed, 37 insertions(+)
>>
>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c 
>> b/tools/usb/usbip/libsrc/usbip_device_driver.c
>> index 927a151fa9aa..6da000fab26d 100644
>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>> @@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct 
>> usbip_host_driver *hdriver)
>>   struct usbip_host_driver device_driver = {
>>       .edev_list = LIST_HEAD_INIT(device_driver.edev_list),
>>       .udev_subsystem = "udc",
>> +    .bus_type = "platform",
>
> Why are we adding this here - user-space shouldn't need to
> know what kind of driver this is?
The reason I added the bus_type and drv_name fields is to
construct the specific sysfs paths required for the availability check.
Although usbip-host and usbip-vudc share the same 
usbip_generic_driver_open() interface ,
they operate on different bus types and have different driver names.
These fields allow the generic open function to dynamically verify
the correct driver path in sysfs based on the specific driver type being 
initialized.
If you prefer not to expose these in the this structure,
I'm happy to adjust further based on your suggestions.
>
>> +    .drv_name = USBIP_DEVICE_DRV_NAME,
>
> And the name?
>
>>       .ops = {
>>           .open = usbip_device_driver_open,
>>           .close = usbip_generic_driver_close,
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c 
>> b/tools/usb/usbip/libsrc/usbip_host_common.c
>> index ca78aa368476..900370095b61 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
>> @@ -164,6 +164,31 @@ static void usbip_exported_device_destroy(struct 
>> list_head *devs)
>>       }
>>   }
>>   +/* Check if the usbip host driver is available in sysfs */
>> +static int check_driver_available(struct usbip_host_driver *hdriver)
>> +{
>> +    char driver_path[SYSFS_PATH_MAX];
>> +    struct stat st;
>> +    int ret;
>> +
>> +    if (!hdriver->drv_name || !hdriver->bus_type)
>> +        return 0;
>> +
>> +    //Check if the usbip-host or usbip-vudc driver directory exists 
>> in sysfs.
>> +    snprintf(driver_path, sizeof(driver_path), "%s/%s/%s/%s/%s",
>> +        SYSFS_MNT_PATH, SYSFS_BUS_NAME, hdriver->bus_type,
>> +        SYSFS_DRIVERS_NAME, hdriver->drv_name);
>> +
>> +    ret = stat(driver_path, &st);
>> +    if (ret == 0 && S_ISDIR(st.st_mode)) {
>> +        dbg("driver '%s' is available", hdriver->drv_name);
>> +        return 1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>>   int usbip_generic_driver_open(struct usbip_host_driver *hdriver)
>>   {
>>       int rc;
>> @@ -174,6 +199,12 @@ int usbip_generic_driver_open(struct 
>> usbip_host_driver *hdriver)
>>           return -1;
>>       }
>>   +    //Check if the required driver is actually available.
>> +    if (!check_driver_available(hdriver)) {
>> +        err("please load '%s' kernel module", hdriver->drv_name);
>> +        goto err;
>> +    }
>> +
>>       rc = refresh_exported_devices(hdriver);
>>       if (rc < 0)
>>           goto err;
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h 
>> b/tools/usb/usbip/libsrc/usbip_host_common.h
>> index f46967c0aa18..cf9bd60846cf 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_common.h
>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.h
>> @@ -40,6 +40,8 @@ struct usbip_host_driver {
>>       /* list of exported device */
>>       struct list_head edev_list;
>>       const char *udev_subsystem;
>> +    const char *bus_type;
>> +    const char *drv_name;
>>       struct usbip_host_driver_ops ops;
>>   };
>>   diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c 
>> b/tools/usb/usbip/libsrc/usbip_host_driver.c
>> index 573e73ec36bd..e8f9d2ee2497 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
>> @@ -41,6 +41,8 @@ static int usbip_host_driver_open(struct 
>> usbip_host_driver *hdriver)
>>   struct usbip_host_driver host_driver = {
>>       .edev_list = LIST_HEAD_INIT(host_driver.edev_list),
>>       .udev_subsystem = "usb",
>> +    .bus_type = "usb",
>> +    .drv_name = USBIP_HOST_DRV_NAME,
>
>>       .ops = {
>>           .open = usbip_host_driver_open,
>>           .close = usbip_generic_driver_close,
>
> I need more to justify these changes.
>
> thanks,
> -- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] usbip: tools: Add usbip host driver availability check
  2026-03-11 12:13 ` [PATCH] " Greg KH
@ 2026-03-12  2:17   ` Zongmin Zhou
  0 siblings, 0 replies; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-12  2:17 UTC (permalink / raw)
  To: Greg KH; +Cc: valentina.manea.m, shuah, i, linux-usb, linux-kernel,
	Zongmin Zhou


On 2026/3/11 20:13, Greg KH wrote:
> On Tue, Mar 03, 2026 at 04:17:20PM +0800, Zongmin Zhou wrote:
>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>
>> Currently, usbip_generic_driver_open() doesn't verify that the required
>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>> The function returns success even when no driver is present,
>> leading to usbipd daemon run success without driver loaded.
>>
>> So add a check function to ensure usbip host driver has been loaded.
>>
>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>> ---
>>   tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
>>   tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
>>   tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
>>   tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
>>   4 files changed, 37 insertions(+)
>>
>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
>> index 927a151fa9aa..6da000fab26d 100644
>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>> @@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>>   struct usbip_host_driver device_driver = {
>>   	.edev_list = LIST_HEAD_INIT(device_driver.edev_list),
>>   	.udev_subsystem = "udc",
>> +	.bus_type = "platform",
> I think the "bus type" is changing in newer kernel versions, so be
> careful about attempting to hard-code anything like this.  Devices can
> move around between bus types just fine, that is not how they should
> ever be referenced.
Thanks a lot for your valuable feedback! I fully understand your 
concerns on bus type hardcoding.
I sincerely apologize for not fully considering the compatibility of 
kernel version changes
and the design consistency of the project in the initial patch.

In the current usbip codebase (libsrc/usbip_common.h),
macros such as SYSFS_BUS_TYPE (defined as "usb") have long been
used to manage the sysfs directory names related to bus type,
and functions like write_sysfs_attribute rely on these macros to access 
the driver's sysfs interface.
So, can I follow the existing design plan? add macros in usbip_common.h
to unify the bus type definition of different drivers.
This way, any future kernel bus type changes can be adapted by only 
modifying the macro, not scattered code.

Looking forward to your reply.
Best regards.
>
> thanks,
>
> greg k-h


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] usbip: tools: Add usbip host driver availability check
  2026-03-12  2:17   ` Zongmin Zhou
@ 2026-03-23 19:17     ` Shuah Khan
  2026-03-25  2:26       ` [PATCH v2] " Zongmin Zhou
  0 siblings, 1 reply; 15+ messages in thread
From: Shuah Khan @ 2026-03-23 19:17 UTC (permalink / raw)
  To: Zongmin Zhou, valentina.manea.m, shuah, i, Greg KH
  Cc: linux-usb, linux-kernel, Zongmin Zhou, Shuah Khan

On 3/11/26 20:17, Zongmin Zhou wrote:
> 
> On 2026/3/11 06:28, Shuah Khan wrote:
>> On 3/3/26 01:17, Zongmin Zhou wrote:
>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>
>>> Currently, usbip_generic_driver_open() doesn't verify that the required
>>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>>> The function returns success even when no driver is present,
>>> leading to usbipd daemon run success without driver loaded.
>>
>> Doesn't usbip_generic_driver_open() try to refresh exported
>> device list and fail? It returns error if it can't find fetch
>> them.
>>
>> usbipd starts and the when usbip_host is loaded it works correctly.
>> Doesn't it?
> Actually, refresh_exported_devices() does not return an error
> when the driver is not loaded,it consistently returns 0.
> It only results in hdriver->ndevs being set to 0 if no exportable usbip devices are found.
> Consequently, if the driver is missing, usbipd will start successfully in silence,
> but subsequent usbip attach operations will fail.
> The lack of explicit error messages makes it difficult for users to troubleshoot the root cause.
> By adding a check to verify if the driver is loaded during the usbip daemon startup,
> we can prevent these silent exceptions and ensure users are alerted to
> the missing kernel module before they attempt to use the service.
>>>
>>> So add a check function to ensure usbip host driver has been loaded.
>>>
>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>> ---
>>>   tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
>>>   tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
>>>   tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
>>>   tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
>>>   4 files changed, 37 insertions(+)
>>>
>>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>> index 927a151fa9aa..6da000fab26d 100644
>>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>> @@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>>>   struct usbip_host_driver device_driver = {
>>>       .edev_list = LIST_HEAD_INIT(device_driver.edev_list),
>>>       .udev_subsystem = "udc",
>>> +    .bus_type = "platform",
>>
>> Why are we adding this here - user-space shouldn't need to
>> know what kind of driver this is?
> The reason I added the bus_type and drv_name fields is to
> construct the specific sysfs paths required for the availability check.
> Although usbip-host and usbip-vudc share the same usbip_generic_driver_open() interface ,
> they operate on different bus types and have different driver names.
> These fields allow the generic open function to dynamically verify
> the correct driver path in sysfs based on the specific driver type being initialized.
> If you prefer not to expose these in the this structure,
> I'm happy to adjust further based on your suggestions.

Have you tried a simple system() e.g below:

if (system("/usr/bin/lsmod | grep usbip")) instead of adding
all of this code?

Take a look at other examples of driver checks in cpupower
check_msr_driver() for example.

thanks,
-- Shuah



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-23 19:17     ` Shuah Khan
@ 2026-03-25  2:26       ` Zongmin Zhou
  2026-03-25  8:58         ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-25  2:26 UTC (permalink / raw)
  To: skhan; +Cc: gregkh, i, linux-kernel, linux-usb, valentina.manea.m,
	Zongmin Zhou

From: Zongmin Zhou <zhouzongmin@kylinos.cn>

Currently, usbip_generic_driver_open() doesn't verify that the required
kernel module (usbip-host or usbip-vudc) is actually loaded.
The function returns success even when no driver is present,
leading to usbipd daemon run success without driver loaded.

So add a check function to ensure usbip host driver has been loaded.

Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
Changes in v2:
- Use system calls directly instead of checking sysfs dir.

 tools/usb/usbip/libsrc/usbip_device_driver.c | 7 +++++--
 tools/usb/usbip/libsrc/usbip_host_driver.c   | 8 ++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
index 927a151fa9aa..45ab647ef241 100644
--- a/tools/usb/usbip/libsrc/usbip_device_driver.c
+++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
@@ -136,10 +136,13 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
 	hdriver->ndevs = 0;
 	INIT_LIST_HEAD(&hdriver->edev_list);
 
-	ret = usbip_generic_driver_open(hdriver);
-	if (ret)
+	if (system("/sbin/lsmod | grep -q usbip_vudc")){
 		err("please load " USBIP_CORE_MOD_NAME ".ko and "
 		    USBIP_DEVICE_DRV_NAME ".ko!");
+		return -1;
+	}
+
+	ret = usbip_generic_driver_open(hdriver);
 
 	return ret;
 }
diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
index 573e73ec36bd..f0ac941d4f6e 100644
--- a/tools/usb/usbip/libsrc/usbip_host_driver.c
+++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
@@ -31,10 +31,14 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
 	hdriver->ndevs = 0;
 	INIT_LIST_HEAD(&hdriver->edev_list);
 
-	ret = usbip_generic_driver_open(hdriver);
-	if (ret)
+	if (system("/sbin/lsmod | grep -q usbip_host")){
 		err("please load " USBIP_CORE_MOD_NAME ".ko and "
 		    USBIP_HOST_DRV_NAME ".ko!");
+		return -1;
+	}
+
+	ret = usbip_generic_driver_open(hdriver);
+
 	return ret;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-25  2:26       ` [PATCH v2] " Zongmin Zhou
@ 2026-03-25  8:58         ` Greg KH
  2026-03-26  3:10           ` Zongmin Zhou
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2026-03-25  8:58 UTC (permalink / raw)
  To: Zongmin Zhou
  Cc: skhan, i, linux-kernel, linux-usb, valentina.manea.m,
	Zongmin Zhou

On Wed, Mar 25, 2026 at 10:26:34AM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> Currently, usbip_generic_driver_open() doesn't verify that the required
> kernel module (usbip-host or usbip-vudc) is actually loaded.
> The function returns success even when no driver is present,
> leading to usbipd daemon run success without driver loaded.
> 
> So add a check function to ensure usbip host driver has been loaded.
> 
> Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
> Changes in v2:
> - Use system calls directly instead of checking sysfs dir.
> 
>  tools/usb/usbip/libsrc/usbip_device_driver.c | 7 +++++--
>  tools/usb/usbip/libsrc/usbip_host_driver.c   | 8 ++++++--
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
> index 927a151fa9aa..45ab647ef241 100644
> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
> @@ -136,10 +136,13 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>  	hdriver->ndevs = 0;
>  	INIT_LIST_HEAD(&hdriver->edev_list);
>  
> -	ret = usbip_generic_driver_open(hdriver);
> -	if (ret)
> +	if (system("/sbin/lsmod | grep -q usbip_vudc")){

What happens if the module is built into the kernel?

>  		err("please load " USBIP_CORE_MOD_NAME ".ko and "
>  		    USBIP_DEVICE_DRV_NAME ".ko!");
> +		return -1;
> +	}
> +
> +	ret = usbip_generic_driver_open(hdriver);
>  
>  	return ret;
>  }
> diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
> index 573e73ec36bd..f0ac941d4f6e 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
> @@ -31,10 +31,14 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
>  	hdriver->ndevs = 0;
>  	INIT_LIST_HEAD(&hdriver->edev_list);
>  
> -	ret = usbip_generic_driver_open(hdriver);
> -	if (ret)
> +	if (system("/sbin/lsmod | grep -q usbip_host")){

Same here, what happens if it is built in?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-25  8:58         ` Greg KH
@ 2026-03-26  3:10           ` Zongmin Zhou
  2026-03-26  8:43             ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-26  3:10 UTC (permalink / raw)
  To: Greg KH; +Cc: skhan, i, linux-kernel, linux-usb, valentina.manea.m,
	Zongmin Zhou


On 2026/3/25 16:58, Greg KH wrote:
> On Wed, Mar 25, 2026 at 10:26:34AM +0800, Zongmin Zhou wrote:
>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>
>> Currently, usbip_generic_driver_open() doesn't verify that the required
>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>> The function returns success even when no driver is present,
>> leading to usbipd daemon run success without driver loaded.
>>
>> So add a check function to ensure usbip host driver has been loaded.
>>
>> Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>> ---
>> Changes in v2:
>> - Use system calls directly instead of checking sysfs dir.
>>
>>   tools/usb/usbip/libsrc/usbip_device_driver.c | 7 +++++--
>>   tools/usb/usbip/libsrc/usbip_host_driver.c   | 8 ++++++--
>>   2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
>> index 927a151fa9aa..45ab647ef241 100644
>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>> @@ -136,10 +136,13 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>>   	hdriver->ndevs = 0;
>>   	INIT_LIST_HEAD(&hdriver->edev_list);
>>   
>> -	ret = usbip_generic_driver_open(hdriver);
>> -	if (ret)
>> +	if (system("/sbin/lsmod | grep -q usbip_vudc")){
> What happens if the module is built into the kernel?
>
>>   		err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>   		    USBIP_DEVICE_DRV_NAME ".ko!");
>> +		return -1;
>> +	}
>> +
>> +	ret = usbip_generic_driver_open(hdriver);
>>   
>>   	return ret;
>>   }
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
>> index 573e73ec36bd..f0ac941d4f6e 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
>> @@ -31,10 +31,14 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
>>   	hdriver->ndevs = 0;
>>   	INIT_LIST_HEAD(&hdriver->edev_list);
>>   
>> -	ret = usbip_generic_driver_open(hdriver);
>> -	if (ret)
>> +	if (system("/sbin/lsmod | grep -q usbip_host")){
> Same here, what happens if it is built in?
Thank you for pointing this out.
I apologize for not considering the built-in module case.

You are right that using lsmod | grep would incorrectly fail when 
usbip_host is built into the kernel (CONFIG_USBIP_HOST=y).
Usbip has always been built as a loadable module (.ko) by default, which 
led to this oversight.

To address this issue, would the following approach be acceptable?
1. Uses /sys/module/usbip_host to check wehther had been loaded, which 
exists for both loadable modules and built-in drivers.
2. Attempts to load the module via modprobe if it is not already loaded.

for example:
static int probe_usbip_host_driver(void)
{
      struct stat st;
      return !stat("/sys/module/usbip_host", &st);
}

static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
{
      // 1. Check if driver is already loaded/built-in
      if (!probe_usbip_host_driver()) {
          // 2. Try to load the module
          system("/sbin/modprobe usbip-host > /dev/null 2>&1");
         // 3. Check again
           if (!probe_usbip_host_driver()) {
               err("please load " USBIP_CORE_MOD_NAME ".ko and "
                   USBIP_HOST_DRV_NAME ".ko!");
               return -1;
}
}

       // Continue with normal flow...
       ret = usbip_generic_driver_open(hdriver);
       return ret;
}

Does this look reasonable to you? I would be happy to submit a v3 patch 
with this approach if you agree.
>
> thanks,
>
> greg k-h


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-26  3:10           ` Zongmin Zhou
@ 2026-03-26  8:43             ` Greg KH
  2026-03-26 18:43               ` Shuah Khan
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2026-03-26  8:43 UTC (permalink / raw)
  To: Zongmin Zhou
  Cc: skhan, i, linux-kernel, linux-usb, valentina.manea.m,
	Zongmin Zhou

On Thu, Mar 26, 2026 at 11:10:02AM +0800, Zongmin Zhou wrote:
> 
> On 2026/3/25 16:58, Greg KH wrote:
> > On Wed, Mar 25, 2026 at 10:26:34AM +0800, Zongmin Zhou wrote:
> > > From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > > 
> > > Currently, usbip_generic_driver_open() doesn't verify that the required
> > > kernel module (usbip-host or usbip-vudc) is actually loaded.
> > > The function returns success even when no driver is present,
> > > leading to usbipd daemon run success without driver loaded.
> > > 
> > > So add a check function to ensure usbip host driver has been loaded.
> > > 
> > > Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
> > > Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > > ---
> > > Changes in v2:
> > > - Use system calls directly instead of checking sysfs dir.
> > > 
> > >   tools/usb/usbip/libsrc/usbip_device_driver.c | 7 +++++--
> > >   tools/usb/usbip/libsrc/usbip_host_driver.c   | 8 ++++++--
> > >   2 files changed, 11 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
> > > index 927a151fa9aa..45ab647ef241 100644
> > > --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
> > > +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
> > > @@ -136,10 +136,13 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
> > >   	hdriver->ndevs = 0;
> > >   	INIT_LIST_HEAD(&hdriver->edev_list);
> > > -	ret = usbip_generic_driver_open(hdriver);
> > > -	if (ret)
> > > +	if (system("/sbin/lsmod | grep -q usbip_vudc")){
> > What happens if the module is built into the kernel?
> > 
> > >   		err("please load " USBIP_CORE_MOD_NAME ".ko and "
> > >   		    USBIP_DEVICE_DRV_NAME ".ko!");
> > > +		return -1;
> > > +	}
> > > +
> > > +	ret = usbip_generic_driver_open(hdriver);
> > >   	return ret;
> > >   }
> > > diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
> > > index 573e73ec36bd..f0ac941d4f6e 100644
> > > --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
> > > +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
> > > @@ -31,10 +31,14 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
> > >   	hdriver->ndevs = 0;
> > >   	INIT_LIST_HEAD(&hdriver->edev_list);
> > > -	ret = usbip_generic_driver_open(hdriver);
> > > -	if (ret)
> > > +	if (system("/sbin/lsmod | grep -q usbip_host")){
> > Same here, what happens if it is built in?
> Thank you for pointing this out.
> I apologize for not considering the built-in module case.
> 
> You are right that using lsmod | grep would incorrectly fail when usbip_host
> is built into the kernel (CONFIG_USBIP_HOST=y).
> Usbip has always been built as a loadable module (.ko) by default, which led
> to this oversight.
> 
> To address this issue, would the following approach be acceptable?

Wait, what "issue" are you trying to fix here?  Why can't you just check
for opening the correct device node when the host opens the file and if
that fails, report an error?  Doesn't that happen today already?

> 1. Uses /sys/module/usbip_host to check wehther had been loaded, which
> exists for both loadable modules and built-in drivers.
> 2. Attempts to load the module via modprobe if it is not already loaded.

Don't do module autoloading, that might surprise users that previously
were not expecting it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-26  8:43             ` Greg KH
@ 2026-03-26 18:43               ` Shuah Khan
  2026-03-27  8:39                 ` Zongmin Zhou
  0 siblings, 1 reply; 15+ messages in thread
From: Shuah Khan @ 2026-03-26 18:43 UTC (permalink / raw)
  To: Greg KH, Zongmin Zhou
  Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou,
	Shuah Khan

On 3/26/26 02:43, Greg KH wrote:
> On Thu, Mar 26, 2026 at 11:10:02AM +0800, Zongmin Zhou wrote:
>>
>> On 2026/3/25 16:58, Greg KH wrote:
>>> On Wed, Mar 25, 2026 at 10:26:34AM +0800, Zongmin Zhou wrote:
>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>
>>>> Currently, usbip_generic_driver_open() doesn't verify that the required
>>>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>>>> The function returns success even when no driver is present,
>>>> leading to usbipd daemon run success without driver loaded.
>>>>
>>>> So add a check function to ensure usbip host driver has been loaded.
>>>>
>>>> Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>> ---
>>>> Changes in v2:
>>>> - Use system calls directly instead of checking sysfs dir.
>>>>
>>>>    tools/usb/usbip/libsrc/usbip_device_driver.c | 7 +++++--
>>>>    tools/usb/usbip/libsrc/usbip_host_driver.c   | 8 ++++++--
>>>>    2 files changed, 11 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>>> index 927a151fa9aa..45ab647ef241 100644
>>>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>>>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>>> @@ -136,10 +136,13 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>>>>    	hdriver->ndevs = 0;
>>>>    	INIT_LIST_HEAD(&hdriver->edev_list);
>>>> -	ret = usbip_generic_driver_open(hdriver);
>>>> -	if (ret)
>>>> +	if (system("/sbin/lsmod | grep -q usbip_vudc")){
>>> What happens if the module is built into the kernel?
>>>
>>>>    		err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>>>    		    USBIP_DEVICE_DRV_NAME ".ko!");
>>>> +		return -1;
>>>> +	}
>>>> +
>>>> +	ret = usbip_generic_driver_open(hdriver);
>>>>    	return ret;
>>>>    }
>>>> diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c b/tools/usb/usbip/libsrc/usbip_host_driver.c
>>>> index 573e73ec36bd..f0ac941d4f6e 100644
>>>> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
>>>> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
>>>> @@ -31,10 +31,14 @@ static int usbip_host_driver_open(struct usbip_host_driver *hdriver)
>>>>    	hdriver->ndevs = 0;
>>>>    	INIT_LIST_HEAD(&hdriver->edev_list);
>>>> -	ret = usbip_generic_driver_open(hdriver);
>>>> -	if (ret)
>>>> +	if (system("/sbin/lsmod | grep -q usbip_host")){
>>> Same here, what happens if it is built in?
>> Thank you for pointing this out.
>> I apologize for not considering the built-in module case.
>>
>> You are right that using lsmod | grep would incorrectly fail when usbip_host
>> is built into the kernel (CONFIG_USBIP_HOST=y).
>> Usbip has always been built as a loadable module (.ko) by default, which led
>> to this oversight.
>>
>> To address this issue, would the following approach be acceptable?
> 
> Wait, what "issue" are you trying to fix here?  Why can't you just check
> for opening the correct device node when the host opens the file and if
> that fails, report an error?  Doesn't that happen today already?
> 

The problem Zongmin is trying fix ish when usbipd starts, it looks for
exported if any - if it doesn't find any it assumes there aren't any
exported and doesn't detect that usbip_host driver isn't loaded.

refresh_exported_devices() doesn't have the logic and it shouldn't
include that logic because this hook is called periodically to
refresh the list of exported devices. Starting usbipd and loading
the driver are distinct steps and without any dependencies.

This patch he is trying to add that detection so a message can be printed
to say "load the driver".

A message can be added easily to cover two cases:

1. usbip_host driver isn't loaded
2. There are no exported devices.

refresh_exported_devices() will not find any devices in both
of the above scenarios. It isn't an error if it can't find
any exported devices.

An informational message when refresh_exported_devices()
when it doesn't find any devices could help users.

Zongmin,

Would adding a message that says
"Check if usbip_host driver is loaded or export devices"
solve the problem of hard to debug problem you are addressing here?

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-26 18:43               ` Shuah Khan
@ 2026-03-27  8:39                 ` Zongmin Zhou
  2026-03-27 17:51                   ` Shuah Khan
  0 siblings, 1 reply; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-27  8:39 UTC (permalink / raw)
  To: Shuah Khan, Greg KH
  Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou


On 2026/3/27 02:43, Shuah Khan wrote:
> On 3/26/26 02:43, Greg KH wrote:
>> On Thu, Mar 26, 2026 at 11:10:02AM +0800, Zongmin Zhou wrote:
>>>
>>> On 2026/3/25 16:58, Greg KH wrote:
>>>> On Wed, Mar 25, 2026 at 10:26:34AM +0800, Zongmin Zhou wrote:
>>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>>
>>>>> Currently, usbip_generic_driver_open() doesn't verify that the 
>>>>> required
>>>>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>>>>> The function returns success even when no driver is present,
>>>>> leading to usbipd daemon run success without driver loaded.
>>>>>
>>>>> So add a check function to ensure usbip host driver has been loaded.
>>>>>
>>>>> Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
>>>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Use system calls directly instead of checking sysfs dir.
>>>>>
>>>>>    tools/usb/usbip/libsrc/usbip_device_driver.c | 7 +++++--
>>>>>    tools/usb/usbip/libsrc/usbip_host_driver.c   | 8 ++++++--
>>>>>    2 files changed, 11 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c 
>>>>> b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>>>> index 927a151fa9aa..45ab647ef241 100644
>>>>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>>>>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>>>> @@ -136,10 +136,13 @@ static int usbip_device_driver_open(struct 
>>>>> usbip_host_driver *hdriver)
>>>>>        hdriver->ndevs = 0;
>>>>>        INIT_LIST_HEAD(&hdriver->edev_list);
>>>>> -    ret = usbip_generic_driver_open(hdriver);
>>>>> -    if (ret)
>>>>> +    if (system("/sbin/lsmod | grep -q usbip_vudc")){
>>>> What happens if the module is built into the kernel?
>>>>
>>>>>            err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>>>>                USBIP_DEVICE_DRV_NAME ".ko!");
>>>>> +        return -1;
>>>>> +    }
>>>>> +
>>>>> +    ret = usbip_generic_driver_open(hdriver);
>>>>>        return ret;
>>>>>    }
>>>>> diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c 
>>>>> b/tools/usb/usbip/libsrc/usbip_host_driver.c
>>>>> index 573e73ec36bd..f0ac941d4f6e 100644
>>>>> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
>>>>> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
>>>>> @@ -31,10 +31,14 @@ static int usbip_host_driver_open(struct 
>>>>> usbip_host_driver *hdriver)
>>>>>        hdriver->ndevs = 0;
>>>>>        INIT_LIST_HEAD(&hdriver->edev_list);
>>>>> -    ret = usbip_generic_driver_open(hdriver);
>>>>> -    if (ret)
>>>>> +    if (system("/sbin/lsmod | grep -q usbip_host")){
>>>> Same here, what happens if it is built in?
>>> Thank you for pointing this out.
>>> I apologize for not considering the built-in module case.
>>>
>>> You are right that using lsmod | grep would incorrectly fail when 
>>> usbip_host
>>> is built into the kernel (CONFIG_USBIP_HOST=y).
>>> Usbip has always been built as a loadable module (.ko) by default, 
>>> which led
>>> to this oversight.
>>>
>>> To address this issue, would the following approach be acceptable?
>>
>> Wait, what "issue" are you trying to fix here?  Why can't you just check
>> for opening the correct device node when the host opens the file and if
>> that fails, report an error?  Doesn't that happen today already?
>>
>
> The problem Zongmin is trying fix ish when usbipd starts, it looks for
> exported if any - if it doesn't find any it assumes there aren't any
> exported and doesn't detect that usbip_host driver isn't loaded.
>
> refresh_exported_devices() doesn't have the logic and it shouldn't
> include that logic because this hook is called periodically to
> refresh the list of exported devices. Starting usbipd and loading
> the driver are distinct steps and without any dependencies.
>
> This patch he is trying to add that detection so a message can be printed
> to say "load the driver".
>
> A message can be added easily to cover two cases:
>
> 1. usbip_host driver isn't loaded
> 2. There are no exported devices.
>
> refresh_exported_devices() will not find any devices in both
> of the above scenarios. It isn't an error if it can't find
> any exported devices.
>
> An informational message when refresh_exported_devices()
> when it doesn't find any devices could help users.
>
> Zongmin,
>
> Would adding a message that says
> "Check if usbip_host driver is loaded or export devices"
> solve the problem of hard to debug problem you are addressing here?
>
Shuah,

Your suggestion makes sense.
Adding an informational message when no devices are found would be a simple
and clean solution that helps users debug without being intrusive.

However, I plan to add the info() message in usbip_generic_driver_open() 
instead of
refresh_exported_devices(), because:
- usbip_generic_driver_open() is called only once at initialization.
- refresh_exported_devices() is called periodically to refresh the 
exported device list.
- When the server has no exported devices, having zero devices
   is normal and not worth frequent info messages.

Theoretically, we only need to prompt once at startup. Is my 
understanding correct?

I'll also remove the existing error messages like below,
since they cannot accurately determine whether the driver is loaded:

if (ret)
     err("please load " USBIP_CORE_MOD_NAME ".ko and "
         USBIP_HOST_DRV_NAME ".ko!");

Does this approach look acceptable?

Best regards,
--Zongmin Zhou
> thanks,
> -- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-27  8:39                 ` Zongmin Zhou
@ 2026-03-27 17:51                   ` Shuah Khan
  2026-03-27 18:29                     ` Shuah Khan
  0 siblings, 1 reply; 15+ messages in thread
From: Shuah Khan @ 2026-03-27 17:51 UTC (permalink / raw)
  To: Zongmin Zhou, Greg KH
  Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou,
	Shuah Khan

On 3/27/26 02:39, Zongmin Zhou wrote:
> 
> On 2026/3/27 02:43, Shuah Khan wrote:
>> On 3/26/26 02:43, Greg KH wrote:

[removed text]

>>
>> The problem Zongmin is trying fix ish when usbipd starts, it looks for
>> exported if any - if it doesn't find any it assumes there aren't any
>> exported and doesn't detect that usbip_host driver isn't loaded.
>>
>> refresh_exported_devices() doesn't have the logic and it shouldn't
>> include that logic because this hook is called periodically to
>> refresh the list of exported devices. Starting usbipd and loading
>> the driver are distinct steps and without any dependencies.
>>
>> This patch he is trying to add that detection so a message can be printed
>> to say "load the driver".
>>
>> A message can be added easily to cover two cases:
>>
>> 1. usbip_host driver isn't loaded
>> 2. There are no exported devices.
>>
>> refresh_exported_devices() will not find any devices in both
>> of the above scenarios. It isn't an error if it can't find
>> any exported devices.
>>
>> An informational message when refresh_exported_devices()
>> when it doesn't find any devices could help users.
>>
>> Zongmin,
>>
>> Would adding a message that says
>> "Check if usbip_host driver is loaded or export devices"
>> solve the problem of hard to debug problem you are addressing here?
>>
> Shuah,
> 
> Your suggestion makes sense.
> Adding an informational message when no devices are found would be a simple
> and clean solution that helps users debug without being intrusive.
> 
> However, I plan to add the info() message in usbip_generic_driver_open() instead of
> refresh_exported_devices(), because:
> - usbip_generic_driver_open() is called only once at initialization.
> - refresh_exported_devices() is called periodically to refresh the exported device list.

refresh_exported_devices() isn't called periodically - it is called
from usbip_host_driver op: refresh_device_list and it will be called
whenever usbipd (host side) calls it whenever it receives a request from
user-space via process_request()

For example running "usbip list -l" command will trigger a run of
refresh_exported_devices() via usbip_host_driver op: refresh_device_list

I don't think it will that noisy to add a message to refresh_exported_devices()
when device list is zero. Currently the logic doesn't detect device list zero.
You have add that condition to print informational message.


> - When the server has no exported devices, having zero devices
>    is normal and not worth frequent info messages.

That is correct. Zero exported devices isn't an error and this could
persist until devices are exported.

> 
> Theoretically, we only need to prompt once at startup. Is my understanding correct?> > I'll also remove the existing error messages like below,
> since they cannot accurately determine whether the driver is loaded:
> 
> if (ret)
>      err("please load " USBIP_CORE_MOD_NAME ".ko and "
>          USBIP_HOST_DRV_NAME ".ko!");

Leave this one alone, because it gets called from a couple of places.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-27 17:51                   ` Shuah Khan
@ 2026-03-27 18:29                     ` Shuah Khan
  2026-03-30  3:10                       ` Zongmin Zhou
  0 siblings, 1 reply; 15+ messages in thread
From: Shuah Khan @ 2026-03-27 18:29 UTC (permalink / raw)
  To: Zongmin Zhou
  Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou,
	Shuah Khan, Greg KH

On 3/27/26 11:51, Shuah Khan wrote:
> On 3/27/26 02:39, Zongmin Zhou wrote:
>>
>> On 2026/3/27 02:43, Shuah Khan wrote:
>>> On 3/26/26 02:43, Greg KH wrote:
> 
> [removed text]
> 
>>>
>>> The problem Zongmin is trying fix ish when usbipd starts, it looks for
>>> exported if any - if it doesn't find any it assumes there aren't any
>>> exported and doesn't detect that usbip_host driver isn't loaded.
>>>
>>> refresh_exported_devices() doesn't have the logic and it shouldn't
>>> include that logic because this hook is called periodically to
>>> refresh the list of exported devices. Starting usbipd and loading
>>> the driver are distinct steps and without any dependencies.
>>>
>>> This patch he is trying to add that detection so a message can be printed
>>> to say "load the driver".
>>>
>>> A message can be added easily to cover two cases:
>>>
>>> 1. usbip_host driver isn't loaded
>>> 2. There are no exported devices.
>>>
>>> refresh_exported_devices() will not find any devices in both
>>> of the above scenarios. It isn't an error if it can't find
>>> any exported devices.
>>>
>>> An informational message when refresh_exported_devices()
>>> when it doesn't find any devices could help users.
>>>
>>> Zongmin,
>>>
>>> Would adding a message that says
>>> "Check if usbip_host driver is loaded or export devices"
>>> solve the problem of hard to debug problem you are addressing here?
>>>
>> Shuah,
>>
>> Your suggestion makes sense.
>> Adding an informational message when no devices are found would be a simple
>> and clean solution that helps users debug without being intrusive.
>>
>> However, I plan to add the info() message in usbip_generic_driver_open() instead of
>> refresh_exported_devices(), because:
>> - usbip_generic_driver_open() is called only once at initialization.
>> - refresh_exported_devices() is called periodically to refresh the exported device list.
> 
> refresh_exported_devices() isn't called periodically - it is called
> from usbip_host_driver op: refresh_device_list and it will be called
> whenever usbipd (host side) calls it whenever it receives a request from
> user-space via process_request()
> 
> For example running "usbip list -l" command will trigger a run of
> refresh_exported_devices() via usbip_host_driver op: refresh_device_list
> 
> I don't think it will that noisy to add a message to refresh_exported_devices()
> when device list is zero. Currently the logic doesn't detect device list zero.
> You have add that condition to print informational message.
> 
> 
>> - When the server has no exported devices, having zero devices
>>    is normal and not worth frequent info messages.
> 
> That is correct. Zero exported devices isn't an error and this could
> persist until devices are exported.
> 
>>
>> Theoretically, we only need to prompt once at startup. Is my understanding correct?> > I'll also remove the existing error messages like below,
>> since they cannot accurately determine whether the driver is loaded:
>>
>> if (ret)
>>      err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>          USBIP_HOST_DRV_NAME ".ko!");
> 
> Leave this one alone, because it gets called from a couple of places.
> 

Better yet, why not change the usbip instead - usbip_list for example.
This is the one that prints the device list and the change can be made
there when the list is zero to say, "Check if driver is loaded and
exported devices"

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
  2026-03-27 18:29                     ` Shuah Khan
@ 2026-03-30  3:10                       ` Zongmin Zhou
  0 siblings, 0 replies; 15+ messages in thread
From: Zongmin Zhou @ 2026-03-30  3:10 UTC (permalink / raw)
  To: Shuah Khan
  Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou,
	Greg KH


On 2026/3/28 02:29, Shuah Khan wrote:
> On 3/27/26 11:51, Shuah Khan wrote:
>> On 3/27/26 02:39, Zongmin Zhou wrote:
>>>
>>> On 2026/3/27 02:43, Shuah Khan wrote:
>>>> On 3/26/26 02:43, Greg KH wrote:
>>
>> [removed text]
>>
>>>>
>>>> The problem Zongmin is trying fix ish when usbipd starts, it looks for
>>>> exported if any - if it doesn't find any it assumes there aren't any
>>>> exported and doesn't detect that usbip_host driver isn't loaded.
>>>>
>>>> refresh_exported_devices() doesn't have the logic and it shouldn't
>>>> include that logic because this hook is called periodically to
>>>> refresh the list of exported devices. Starting usbipd and loading
>>>> the driver are distinct steps and without any dependencies.
>>>>
>>>> This patch he is trying to add that detection so a message can be 
>>>> printed
>>>> to say "load the driver".
>>>>
>>>> A message can be added easily to cover two cases:
>>>>
>>>> 1. usbip_host driver isn't loaded
>>>> 2. There are no exported devices.
>>>>
>>>> refresh_exported_devices() will not find any devices in both
>>>> of the above scenarios. It isn't an error if it can't find
>>>> any exported devices.
>>>>
>>>> An informational message when refresh_exported_devices()
>>>> when it doesn't find any devices could help users.
>>>>
>>>> Zongmin,
>>>>
>>>> Would adding a message that says
>>>> "Check if usbip_host driver is loaded or export devices"
>>>> solve the problem of hard to debug problem you are addressing here?
>>>>
>>> Shuah,
>>>
>>> Your suggestion makes sense.
>>> Adding an informational message when no devices are found would be a 
>>> simple
>>> and clean solution that helps users debug without being intrusive.
>>>
>>> However, I plan to add the info() message in 
>>> usbip_generic_driver_open() instead of
>>> refresh_exported_devices(), because:
>>> - usbip_generic_driver_open() is called only once at initialization.
>>> - refresh_exported_devices() is called periodically to refresh the 
>>> exported device list.
>>
>> refresh_exported_devices() isn't called periodically - it is called
>> from usbip_host_driver op: refresh_device_list and it will be called
>> whenever usbipd (host side) calls it whenever it receives a request from
>> user-space via process_request()
>>
>> For example running "usbip list -l" command will trigger a run of
>> refresh_exported_devices() via usbip_host_driver op: refresh_device_list
>>
>> I don't think it will that noisy to add a message to 
>> refresh_exported_devices()
>> when device list is zero. Currently the logic doesn't detect device 
>> list zero.
>> You have add that condition to print informational message.
>>
>>
>>> - When the server has no exported devices, having zero devices
>>>    is normal and not worth frequent info messages.
>>
>> That is correct. Zero exported devices isn't an error and this could
>> persist until devices are exported.
>>
>>>
>>> Theoretically, we only need to prompt once at startup. Is my 
>>> understanding correct?> > I'll also remove the existing error 
>>> messages like below,
>>> since they cannot accurately determine whether the driver is loaded:
>>>
>>> if (ret)
>>>      err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>>          USBIP_HOST_DRV_NAME ".ko!");
>>
>> Leave this one alone, because it gets called from a couple of places.
In usbip_generic_driver_open(), the only path that triggers this message 
is a failure of udev_new().
This function fails due to system-level issues like memory exhaustion, 
not because usbip driver module is missing.

Furthermore, since refresh_exported_devices() doesn't practically return 
an error here,
the message is never seen during actual driver loading failures.
So I think it’s better to remove this inaccurate hint to avoid confusing 
the users.

This is the reason why I previously wanted to remove it.
>>
>
> Better yet, why not change the usbip instead - usbip_list for example.
> This is the one that prints the device list and the change can be made
> there when the list is zero to say, "Check if driver is loaded and
> exported devices"
I think placing the check/message in refresh_exported_devices() would be 
more effective.
This function covers all scenarios where the device list is refreshed, 
including:
usbipd startup, usbip list -r, and usbip attach operations.

This way, users receive helpful feedback automatically without needing 
to explicitly run usbip list -r.
>
> thanks,
> -- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-03-30  3:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03  8:17 [PATCH] usbip: tools: Add usbip host driver availability check Zongmin Zhou
2026-03-10 22:28 ` Shuah Khan
2026-03-12  2:17   ` Zongmin Zhou
2026-03-23 19:17     ` Shuah Khan
2026-03-25  2:26       ` [PATCH v2] " Zongmin Zhou
2026-03-25  8:58         ` Greg KH
2026-03-26  3:10           ` Zongmin Zhou
2026-03-26  8:43             ` Greg KH
2026-03-26 18:43               ` Shuah Khan
2026-03-27  8:39                 ` Zongmin Zhou
2026-03-27 17:51                   ` Shuah Khan
2026-03-27 18:29                     ` Shuah Khan
2026-03-30  3:10                       ` Zongmin Zhou
2026-03-11 12:13 ` [PATCH] " Greg KH
2026-03-12  2:17   ` Zongmin Zhou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox