Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1403477

Re: [PATCH] usb: gadget: f_fs: report error if excess data received

From Michal Nazarewicz <mina86@mina86.com>
Newsgroups linux.kernel
Subject Re: [PATCH] usb: gadget: f_fs: report error if excess data received
Date 2016-05-19 09:40 +0200
Message-ID <rAqIN-5bD-3@gated-at.bofh.it> (permalink)
References (5 earlier) <rzDoJ-7ab-3@gated-at.bofh.it> <rA6h3-ix-3@gated-at.bofh.it> <rA6K6-JL-9@gated-at.bofh.it> <rA9RE-2KE-29@gated-at.bofh.it> <rAmlP-2iB-1@gated-at.bofh.it>
Organization http://mina86.com/

Show all headers | View raw


On Thu, May 19 2016, Changbin Du wrote:
>> On Wed, May 18 2016, Felipe Balbi wrote:
>> > we've been through this before. This needs to be done at the gadget
>> > layer. Gadget driver can over-allocate ahead of time if
>> > gadget->quirk_ep_out_aligned_size is true, then we avoid memcpy() at
>> > the UDC driver level.
>> 
>> Right, all right, so let’s look at it from a regular USB function point
>> of view.  If a USB function allocates a request which is not aligned,
>> UDC will align the buffer and *drop* excess data.  Seeing how ugly

> Do you mean UDC driver align the buffer? I searched the code, currently
> only DWC3 needs buffer size to be aligned to MaxPacketSize on ep out.
> And the align is done in f_fs driver.

I thought that was what was happening based on Felipe’s comment about
avoiding memcpy.  I looked at the code now and dunno what actually
happens.

>> f_fs’s code is becoming, I’m now leaning to letting to f_fs do the same
>> thing: if user space makes an unaligned read, f_fs aligns the buffer and
>> then drops excess data.
>> 
>> Any arguments for f_fs to not drop the data apply to UDC, so they should
>> behave identically.
>> 
> I'd prefer fail the request at all, and it is better done in HW.
> Because per the USB Spec that device can return NAK if a function was
> unable to accept data From the host.  The DWC3 has not been design as
> this, if software fail the transfer, it is a little weird for host.
>
> So, now we have 3 choices:
> 1) buffer the excess data
> 2) fail the transfer

You mean fail when more data has been sent (i.e. drop the whole packet)
or fail at entry to read() if the buffer is not aligned?

> 3) drop the excess data, then print an warning message
>
> Which one do you prefer?

I think f_fs should mimic whatever happens if unaligned request is
queued on dwc3.  As far as I understand, this is not 1.

I’ll be travelling again on Friday so I’ll finish up the patch doing 1
so we will have a choice between 1 (my patch) and 3 (your patch).

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-13 08:00 +0200
  RE: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-13 08:40 +0200
    RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-13 12:40 +0200
    RE: [PATCH] usb: gadget: f_fs: report error if excess data received Alan Stern <stern@rowland.harvard.edu> - 2016-05-13 16:30 +0200
      Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-14 22:40 +0200
      RE: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-16 15:10 +0200
        Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-16 15:10 +0200
          Re: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-16 15:20 +0200
            Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-16 21:10 +0200
              RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-17 05:00 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-18 11:50 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-18 12:20 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-18 15:40 +0200
                RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-19 05:00 +0200
                Re: [PATCH] usb: gadget: f_fs: report error if excess data received Michal Nazarewicz <mina86@mina86.com> - 2016-05-19 09:40 +0200
                RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-19 11:00 +0200
                RE: [PATCH] usb: gadget: f_fs: report error if excess data received "Du, Changbin" <changbin.du@intel.com> - 2016-05-19 04:40 +0200

csiph-web