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

Groups > linux.kernel > #1402860

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

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

Hi,

Michal Nazarewicz <mina86@mina86.com> writes:
> On Tue, May 17 2016, Changbin Du wrote:
>>> There appears to be no kfifo support for iov_iter though, so I just went
>>> with a simple buffer.
>>> 
>>> I haven’t looked at the patch too carefully so this is an RFC rather
>>> than an actual patch at this point.  It does compile at least.
>>> 
>>> Regardless, the more I thin about it, the more I’m under the impression
>>> that the whole rounding up in f_fs was a mistake.  And the more I’m
>>> leaning towards ignoring the excess data set by the host.
>>> 
>>> ---------- >8 ----------------------------------------------------------
>>> Subject: usb: gadget: f_fs: buffer data from ‘oversized’ OUT requests
>>> 
>>> f_fs rounds up read(2) requests to a multiple of a max packet size
>>> which means that host may provide more data than user has space for.
>>> So far, the excess data has been silently ignored.
>>> 
>>> This introduces a buffer for a tail of such requests so that they are
>>> returned on next read instead of being ignored.
>>> 
>>
>> Congratulations finally reach an agreement,
>
> To be honest, if it was up to me, I would rip request size rounding up
> out of the code.

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.

>> thanks Alan Stern and Michal.
>> Here just have a comment - the buffered data need be dropped when the
>> epfile is closed, because it means the session is terminated.
>
> I blame that on sleep deprivation.  Another issue is what to do when
> endpoint is disabled.  Should the buffer be cleared as soon as the
> endpoint is disabled?  Or maybe when the endpoint is enabled again?  Or
> maybe it should never be cleared?
>
> If the buffer is cleared when endpoint is disabled, we again silently
> drop data.  On the other hand, if we don’t do that, read() on the
> endpoint will may succeed even if the configuration is disabled which
> may be surprising for users.

tough decision... but seems like clearing the buffer as soon as ep is
disabled is the way to go.

-- 
balbi

Back to linux.kernel | Previous | Next — Previous 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