Re: [PATCH v13 03/10] usbip: exporting devices: new connect operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/21/2016 11:48 PM, Nobuo Iwata wrote:
> Implementation of new connect operation. This is linked as a part of 
> usbip commnad. With this patch, usbip command has following operations.
> 
>  bind
>  unbind
>  list (local/remote)
>  attach
>  detach
>  port
>  connect ... this patch

I am finding it hard to follow these changes without the server and client
side details. A few comments below:

I want know a clear use-case definition of why the current client pull
model to import devices doesn't work. I understand firewalls pose some
issues. However, it doesn't sound right that server initiates export
and contacts the client. What happens when client doesn't have access
to the information on the server.

Can client use the same import over vpn connection or some sort of
authorized mechanism to access to the server?

> 
> In device side node, this binds a device internally, sends an export 
> request and receives export response. The definition of the request and 
> response are defined in original code: tools/usb/usbip/usbip_network.h 
> but was not used. They are corresponding to NEW-3 in following diagram. 
> 
> EXISTING) - invites devices from application(vhci)-side
>          +------+                                 +------------------+
>  device--+ STUB |                                 | application/VHCI |
>          +------+                                 +------------------+
>  1) usbipd ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip bind
>                     <--- list bound devices ---   4) usbip list --remote
>                     <--- import a device ------   5) usbip attach
>  = = =
>                        X disconnected             6) usbip detach
>  7) usbip unbind
> 
> NEW) - dedicates devices from device(stb)-side
>          +------+                                 +------------------+
>  device--+ STUB |                                 | application/VHCI |
>          +------+                                 +------------------+
>                                               1) usbipa ... start daemon
>  = = =
>  2) usbip list --local
>  3) usbip connect     --- export a device ------>
>  = = =
>  4) usbip disconnect  --- un-export a device --->
> 
> Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> ---
>  tools/usb/usbip/src/Makefile.am     |   3 +-
>  tools/usb/usbip/src/usbip.c         |   9 +-
>  tools/usb/usbip/src/usbip.h         |   5 +-
>  tools/usb/usbip/src/usbip_connect.c | 228 ++++++++++++++++++++++++++++
>  4 files changed, 242 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/usb/usbip/src/Makefile.am b/tools/usb/usbip/src/Makefile.am
> index e81a4eb..0947476 100644
> --- a/tools/usb/usbip/src/Makefile.am
> +++ b/tools/usb/usbip/src/Makefile.am
> @@ -6,6 +6,7 @@ sbin_PROGRAMS := usbip usbipd
>  
>  usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
>  		 usbip_attach.c usbip_detach.c usbip_list.c \
> -		 usbip_bind.c usbip_unbind.c usbip_port.c
> +		 usbip_bind.c usbip_unbind.c usbip_port.c \
> +		 usbip_connect.c
>  
>  usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
> diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c
> index d7599d9..584d7d5 100644
> --- a/tools/usb/usbip/src/usbip.c
> +++ b/tools/usb/usbip/src/usbip.c
> @@ -2,7 +2,8 @@
>   * command structure borrowed from udev
>   * (git://git.kernel.org/pub/scm/linux/hotplug/udev.git)
>   *
> - * Copyright (C) 2011 matt mooney <mfm@xxxxxxxxxxxxx>
> + * Copyright (C) 2015 Nobuo Iwata
> + *               2011 matt mooney <mfm@xxxxxxxxxxxxx>
>   *               2005-2007 Takahiro Hirofuchi
>   *
>   * This program is free software: you can redistribute it and/or modify
> @@ -76,6 +77,12 @@ static const struct command cmds[] = {
>  		.usage = usbip_detach_usage
>  	},
>  	{
> +		.name  = "connect",
> +		.fn    = usbip_connect,
> +		.help  = "Connect a USB device to a remote computer",
> +		.usage = usbip_connect_usage
> +	},
> +	{
>  		.name  = "list",
>  		.fn    = usbip_list,
>  		.help  = "List exportable or local USB devices",
> diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h
> index c296910..f365353 100644
> --- a/tools/usb/usbip/src/usbip.h
> +++ b/tools/usb/usbip/src/usbip.h
> @@ -1,5 +1,6 @@
>  /*
> - * Copyright (C) 2011 matt mooney <mfm@xxxxxxxxxxxxx>
> + * Copyright (C) 2015 Nobuo Iwata

Please don't add new Copyright to an existing file. Also the above
doesn't look right without email address. Please treat this as a
global comment for the Copyright changes in this patch series.


> + *               2011 matt mooney <mfm@xxxxxxxxxxxxx>
>   *               2005-2007 Takahiro Hirofuchi
>   *
>   * This program is free software: you can redistribute it and/or modify
> @@ -30,12 +31,14 @@ int usbip_list(int argc, char *argv[]);
>  int usbip_bind(int argc, char *argv[]);
>  int usbip_unbind(int argc, char *argv[]);
>  int usbip_port_show(int argc, char *argv[]);
> +int usbip_connect(int argc, char *argv[]);
>  
>  void usbip_attach_usage(void);
>  void usbip_detach_usage(void);
>  void usbip_list_usage(void);
>  void usbip_bind_usage(void);
>  void usbip_unbind_usage(void);
> +void usbip_connect_usage(void);
>  
>  int usbip_bind_device(char *busid);
>  int usbip_unbind_device(char *busid);
> diff --git a/tools/usb/usbip/src/usbip_connect.c b/tools/usb/usbip/src/usbip_connect.c
> new file mode 100644
> index 0000000..bbecc7e
> --- /dev/null
> +++ b/tools/usb/usbip/src/usbip_connect.c
> @@ -0,0 +1,228 @@
> +/*
> + * Copyright (C) 2015 Nobuo Iwata

Same as above.

> + *               2011 matt mooney <mfm@xxxxxxxxxxxxx>
> + *               2005-2007 Takahiro Hirofuchi
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <sys/stat.h>
> +
> +#include <limits.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <getopt.h>
> +#include <unistd.h>
> +
> +#include "usbip_host_driver.h"
> +#include "usbip_host_common.h"
> +#include "usbip_device_driver.h"
> +#include "usbip_common.h"
> +#include "usbip_network.h"
> +#include "usbip.h"
> +
> +static struct usbip_host_driver *driver = &host_driver;
> +
> +static const char usbip_connect_usage_string[] =
> +	"usbip connect <args>\n"
> +	"    -r, --remote=<host>    Address of a remote computer\n"
> +	"    -b, --busid=<busid>    Bus ID of a device to be connected\n"
> +	"    -d, --device           Run with an alternate driver, e.g. vUDC\n";
> +
> +void usbip_connect_usage(void)
> +{
> +	printf("usage: %s", usbip_connect_usage_string);
> +}
> +
> +static int send_export_device(int sockfd, struct usbip_usb_device *udev)
> +{
> +	int rc;
> +	struct op_export_request request;
> +	struct op_export_reply   reply;
> +	uint16_t code = OP_REP_EXPORT;
> +
> +	memset(&request, 0, sizeof(request));
> +	memset(&reply, 0, sizeof(reply));
> +
> +	/* send a request */
> +	rc = usbip_net_send_op_common(sockfd, OP_REQ_EXPORT, 0);
> +	if (rc < 0) {
> +		err("send op_common");
> +		return -1;
> +	}
> +
> +	memcpy(&request.udev, udev, sizeof(struct usbip_usb_device));
> +
> +	PACK_OP_EXPORT_REQUEST(0, &request);
> +
> +	rc = usbip_net_send(sockfd, (void *) &request, sizeof(request));
> +	if (rc < 0) {
> +		err("send op_export_request");
> +		return -1;
> +	}
> +
> +	/* receive a reply */
> +	rc = usbip_net_recv_op_common(sockfd, &code);
> +	if (rc < 0) {
> +		err("recv op_common");
> +		return -1;
> +	}
> +
> +	rc = usbip_net_recv(sockfd, (void *) &reply, sizeof(reply));
> +	if (rc < 0) {
> +		err("recv op_export_reply");
> +		return -1;
> +	}
> +
> +	PACK_OP_EXPORT_REPLY(0, &reply);
> +
> +	/* check the reply */
> +	if (reply.returncode) {
> +		err("recv error return %d", reply.returncode);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int export_device(char *busid, int sockfd)
> +{
> +	int rc;
> +	struct usbip_exported_device *edev;
> +
> +	rc = usbip_driver_open(driver);
> +	if (rc) {
> +		err("open driver");
> +		return -1;
> +	}
> +
> +	rc = usbip_refresh_device_list(driver);
> +	if (rc < 0) {
> +		err("could not refresh device list");
> +		usbip_driver_close(driver);
> +		return -1;
> +	}
> +
> +	edev = usbip_get_device(driver, busid);
> +	if (edev == NULL) {
> +		err("find device");
> +		usbip_driver_close(driver);
> +		return -1;
> +	}
> +
> +	rc = send_export_device(sockfd, &edev->udev);
> +	if (rc < 0) {
> +		err("send export");
> +		usbip_driver_close(driver);
> +		return -1;
> +	}
> +
> +	rc = usbip_export_device(edev, sockfd);
> +	if (rc < 0) {
> +		err("export device");
> +		usbip_driver_close(driver);
> +		return -1;
> +	}
> +
> +	usbip_driver_close(driver);
> +
> +	return 0;
> +}
> +
> +static int connect_device(char *host, char *busid, int bind)
> +{
> +	int sockfd;
> +	int rc;
> +
> +	if (bind) {
> +		rc = usbip_bind_device(busid);
> +		if (rc) {
> +			err("bind");
> +			goto err_out;
> +		}
> +	}
> +
> +	sockfd = usbip_net_tcp_connect(host, usbip_port_string);
> +	if (sockfd < 0) {
> +		err("tcp connect");
> +		goto err_unbind_device;
> +	}
> +
> +	rc = export_device(busid, sockfd);
> +	if (rc < 0) {
> +		err("export");
> +		goto err_close_conn;
> +	}
> +
> +	close(sockfd);
> +
> +	return 0;
> +err_close_conn:
> +	close(sockfd);
> +err_unbind_device:
> +	if (bind)
> +		usbip_unbind_device(busid);
> +err_out:
> +	return -1;
> +}
> +
> +int usbip_connect(int argc, char *argv[])
> +{
> +	static const struct option opts[] = {
> +		{ "remote", required_argument, NULL, 'r' },
> +		{ "busid",  required_argument, NULL, 'b' },
> +		{ "device", no_argument,       NULL, 'd' },
> +		{ NULL, 0,  NULL, 0 }
> +	};
> +	char *host = NULL;
> +	char *busid = NULL;
> +	int opt;
> +	int bind = 1;
> +	int ret = -1;
> +
> +	for (;;) {
> +		opt = getopt_long(argc, argv, "r:b:d", opts, NULL);
> +
> +		if (opt == -1)
> +			break;
> +
> +		switch (opt) {
> +		case 'r':
> +			host = optarg;
> +			break;
> +		case 'b':
> +			busid = optarg;
> +			break;
> +		case 'd':
> +			driver = &device_driver;
> +			bind = 0;
> +			break;
> +		default:
> +			goto err_out;
> +		}
> +	}
> +
> +	if (!host || !busid)
> +		goto err_out;
> +
> +	ret = connect_device(host, busid, bind);
> +	goto out;
> +
> +err_out:
> +	usbip_connect_usage();
> +out:
> +	return ret;
> +}
> 

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America(Silicon Valley)
shuah.kh@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux