Re: [PATCH v7 07/20] usb-backend: include usbredirparser

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

 



Hi,

On Wed, Sep 18, 2019 at 06:17:16AM -0400, Frediano Ziglio wrote:
> > 
> > From: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> > 
> > This patch introduces the usage of usbredirparser in
> > SpiceUsbBackendChannel.
> > 
> > The focus of this patch is to the code path of real devices. As we
> > don't know beforehand if a SpiceUsbBackendChannel will be used by real
> > or emulated devices, an instance of usbredirparser must be initialized
> > with the usbredirhost's first hello message.
> > 
> > This is a must to the current design on supporting emulated devices.
> > This will be extended in the next patch to match remote's usbredirhost
> > capabilities and providing support to emulated devices.
> > 
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> 
> Please add my signed off.

Sure

> 
> > ---
> >  src/usb-backend.c | 223 ++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 218 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/usb-backend.c b/src/usb-backend.c
> > index 68faaae..ef29901 100644
> > --- a/src/usb-backend.c
> > +++ b/src/usb-backend.c
> > @@ -45,6 +45,7 @@
> >  #include "usbutil.h"
> >  
> >  #define LOUD_DEBUG(x, ...)
> > +#define USBREDIR_CALLBACK_NOT_IMPLEMENTED() spice_debug("%s not implemented
> > - FIXME", __func__)
> >  
> >  struct _SpiceUsbDevice
> >  {
> > @@ -82,6 +83,7 @@ struct _SpiceUsbBackend
> >  struct _SpiceUsbBackendChannel
> >  {
> >      struct usbredirhost *usbredirhost;
> > +    struct usbredirparser *parser;
> >      uint8_t *read_buf;
> >      int read_buf_size;
> >      struct usbredirfilter_rule *rules;
> > @@ -630,11 +632,48 @@ static void usbredir_log(void *user_data, int level,
> > const char *msg)
> >      }
> >  }
> >  
> > +static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch);
> > +
> >  static int usbredir_write_callback(void *user_data, uint8_t *data, int
> >  count)
> >  {
> >      SpiceUsbBackendChannel *ch = user_data;
> >      int res;
> >      SPICE_DEBUG("%s ch %p, %d bytes", __FUNCTION__, ch, count);
> > +
> > +    // handle first packet (HELLO) creating parser from capabilities
> > +    if (SPICE_UNLIKELY(ch->parser == NULL)) {
> > +        // Here we return 0 from this function to keep the packet in
> > +        // the queue. The packet will then be sent to the guest.
> > +        // We are initializing SpiceUsbBackendChannel, the
> > +        // SpiceUsbredirChannel is not ready to receive packets.
> > +
> > +        // we are still initializing the host
> > +        if (ch->usbredirhost == NULL) {
> > +            return 0;
> > +        }
> > +
> > +        ch->parser = create_parser(ch);
> > +        if (!ch->parser) {
> > +            return 0;
> > +        }
> > +
> > +        /* hello is short header (12) + hello struct (64) */
> > +        const int hello_size = 12 + sizeof(struct usb_redir_hello_header);
> > +        g_assert(count >= hello_size + 4);
> > +        g_assert(SPICE_ALIGNED_CAST(struct usb_redir_header *, data)->type
> > == usb_redir_hello);
> > +
> > +        const uint32_t flags =
> > +            usbredirparser_fl_write_cb_owns_buffer |
> > usbredirparser_fl_usb_host |
> > +            usbredirparser_fl_no_hello;
> > +
> > +        usbredirparser_init(ch->parser,
> > +                            PACKAGE_STRING,
> > +                            SPICE_ALIGNED_CAST(uint32_t *, data +
> > hello_size),
> > +                            (count - hello_size) / sizeof(uint32_t),
> > +                            flags);
> > +
> > +        return 0;
> > +    }
> >      res = spice_usbredir_write(ch->usbredir_channel, data, count);
> >      return res;
> >  }
> > @@ -701,6 +740,165 @@ GError *spice_usb_backend_get_error_details(int
> > error_code, gchar *desc)
> >      return err;
> >  }
> >  
> > +static void
> > +usbredir_control_packet(void *priv, uint64_t id, struct
> > usb_redir_control_packet_header *h,
> > +                        uint8_t *data, int data_len)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_bulk_packet(void *priv, uint64_t id, struct
> > usb_redir_bulk_packet_header *h,
> > +                     uint8_t *data, int data_len)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void usbredir_device_reset(void *priv)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_interface_info(void *priv, struct usb_redir_interface_info_header
> > *interface_info)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_interface_ep_info(void *priv, struct usb_redir_ep_info_header
> > *ep_info)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_set_configuration(void *priv, uint64_t id,
> > +                           struct usb_redir_set_configuration_header
> > *set_configuration)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void usbredir_get_configuration(void *priv, uint64_t id)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_set_alt_setting(void *priv, uint64_t id, struct
> > usb_redir_set_alt_setting_header *s)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_get_alt_setting(void *priv, uint64_t id, struct
> > usb_redir_get_alt_setting_header *s)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void usbredir_cancel_data(void *priv, uint64_t id)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void usbredir_filter_reject(void *priv)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +/* Note that the ownership of the rules array is passed on to the callback.
> > */
> > +static void
> > +usbredir_filter_filter(void *priv, struct usbredirfilter_rule *r, int count)
> > +{
> > +    SpiceUsbBackendChannel *ch = priv;
> > +    SPICE_DEBUG("%s ch %p %d filters", __FUNCTION__, ch, count);
> > +
> > +    free(ch->rules);
> > +    ch->rules = r;
> > +    ch->rules_count = count;
> > +    if (count) {
> > +        int i;
> > +        for (i = 0; i < count; i++) {
> > +            SPICE_DEBUG("%s class %d, %X:%X",
> > +                r[i].allow ? "allowed" : "denied", r[i].device_class,
> > +                (uint32_t)r[i].vendor_id, (uint32_t)r[i].product_id);
> > +        }
> > +    }
> > +}
> > +
> > +static void usbredir_device_disconnect_ack(void *priv)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void
> > +usbredir_hello(void *priv, struct usb_redir_hello_header *hello)
> > +{
> > +    USBREDIR_CALLBACK_NOT_IMPLEMENTED();
> > +}
> > +
> > +static void initialize_parser(SpiceUsbBackendChannel *ch)
> > +{
> > +    uint32_t flags, caps[USB_REDIR_CAPS_SIZE] = { 0 };
> > +
> > +    g_assert(ch->usbredirhost == NULL);
> > +
> > +    flags = usbredirparser_fl_write_cb_owns_buffer |
> > usbredirparser_fl_usb_host;
> > +
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version);
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_filter);
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_device_disconnect_ack);
> > +    usbredirparser_caps_set_cap(caps,
> > usb_redir_cap_ep_info_max_packet_size);
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_64bits_ids);
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_32bits_bulk_length);
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_receiving);
> > +    usbredirparser_caps_set_cap(caps, usb_redir_cap_bulk_streams);
> > +
> > +    usbredirparser_init(ch->parser, PACKAGE_STRING, caps,
> > USB_REDIR_CAPS_SIZE, flags);
> > +}
> > +
> > +/*
> > +    We can initialize the usbredirparser with HELLO enabled only in case
> > +    the libusb is not active and the usbredirhost does not function.
> > +    Then the parser sends session HELLO and receives server's response.
> > +    Otherwise (usbredirparser initialized with HELLO disabled):
> > +    - the usbredirhost sends session HELLO
> > +    - we look into it to know set of capabilities we shall initialize
> > +      the parser with
> > +    - when we receive server's response to HELLO we provide it also to
> > +      parser to let it synchronize with server's capabilities
> > +*/
> > +static struct usbredirparser *create_parser(SpiceUsbBackendChannel *ch)
> > +{
> > +    struct usbredirparser *parser = usbredirparser_create();
> > +
> > +    g_return_val_if_fail(parser != NULL, NULL);
> > +
> > +    parser->priv = ch;
> > +    parser->log_func = usbredir_log;
> > +    parser->read_func = usbredir_read_callback;
> > +    parser->write_func = usbredir_write_callback;
> > +    parser->reset_func = usbredir_device_reset;
> > +    parser->set_configuration_func = usbredir_set_configuration;
> > +    parser->get_configuration_func = usbredir_get_configuration;
> > +    parser->set_alt_setting_func = usbredir_set_alt_setting;
> > +    parser->get_alt_setting_func = usbredir_get_alt_setting;
> > +    parser->cancel_data_packet_func = usbredir_cancel_data;
> > +    parser->control_packet_func = usbredir_control_packet;
> > +    parser->bulk_packet_func = usbredir_bulk_packet;
> > +    parser->alloc_lock_func = usbredir_alloc_lock;
> > +    parser->lock_func = usbredir_lock_lock;
> > +    parser->unlock_func = usbredir_unlock_lock;
> > +    parser->free_lock_func = usbredir_free_lock;
> > +    parser->hello_func = usbredir_hello;
> > +    parser->filter_reject_func = usbredir_filter_reject;
> > +    parser->device_disconnect_ack_func = usbredir_device_disconnect_ack;
> > +    parser->interface_info_func = usbredir_interface_info;
> > +    parser->ep_info_func = usbredir_interface_ep_info;
> > +    parser->filter_filter_func = usbredir_filter_filter;
> > +
> > +    return parser;
> > +}
> > +
> >  void spice_usb_backend_return_write_data(SpiceUsbBackendChannel *ch, void
> >  *data)
> >  {
> >      if (ch->usbredirhost) {
> > @@ -799,11 +997,22 @@ spice_usb_backend_channel_new(SpiceUsbBackend *be,
> >              spice_util_get_debug() ? usbredirparser_debug :
> >              usbredirparser_warning,
> >              usbredirhost_fl_write_cb_owns_buffer);
> >          g_warn_if_fail(ch->usbredirhost != NULL);
> > -    }
> > -    if (ch->usbredirhost) {
> > -        usbredirhost_set_buffered_output_size_cb(ch->usbredirhost,
> > usbredir_buffered_output_size_callback);
> > +        if (ch->usbredirhost) {
> > +            usbredirhost_set_buffered_output_size_cb(ch->usbredirhost,
> > usbredir_buffered_output_size_callback);
> > +            // force flush of HELLO packet and creation of parser
> > +            usbredirhost_write_guest_data(ch->usbredirhost);
> > +        }
> >      } else {
> > -        g_free(ch);
> > +        // no physical device support, only emulated, create the
> > +        // parser
> > +        ch->parser = create_parser(ch);
> > +        if (ch->parser != NULL) {
> > +            initialize_parser(ch);
> > +        }
> > +    }
> > +
> > +    if (!ch->parser) {
> > +        spice_usb_backend_channel_delete(ch);
> >          ch = NULL;
> >      }
> >  
> > @@ -828,9 +1037,13 @@ void
> > spice_usb_backend_channel_delete(SpiceUsbBackendChannel *ch)
> >      if (ch->usbredirhost) {
> >          usbredirhost_close(ch->usbredirhost);
> >      }
> > +    if (ch->parser) {
> > +        usbredirparser_destroy(ch->parser);
> > +    }
> >  
> >      if (ch->rules) {
> > -        g_free(ch->rules);
> > +        /* rules were allocated by usbredirparser */
> > +        free(ch->rules);
> >      }
> >  
> >      g_free(ch);
> 
> Fine for me, I would wait Yuri opinion (on merging my changes and
> split this patch, same comment for next patch).

Ok, thanks!

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]